Re: [alsa-devel] [PATCH v4 06/15] soundwire: Add IO transfer

[Vinod Koul]

On Tue, Dec 05, 2017 at 08:48:03AM -0600, Pierre-Louis Bossart wrote:
> On 12/5/17 7:43 AM, Pierre-Louis Bossart wrote:
> >On 12/5/17 12:31 AM, Vinod Koul wrote:
> >>On Sun, Dec 03, 2017 at 09:01:41PM -0600, Pierre-Louis Bossart wrote:

> >>>>>>+static inline int do_transfer(struct sdw_bus *bus, struct
> >>>>>>sdw_msg *msg)
> >>>>>>+{
> >>>>>>+    int retry = bus->prop.err_threshold;
> >>>>>>+    enum sdw_command_response resp;
> >>>>>>+    int ret = 0, i;
> >>>>>>+
> >>>>>>+    for (i = 0; i <= retry; i++) {
> >>>>>>+        resp = bus->ops->xfer_msg(bus, msg);
> >>>>>>+        ret = find_response_code(resp);
> >>>>>>+
> >>>>>>+        /* if cmd is ok or ignored return */
> >>>>>>+        if (ret == 0 || ret == -ENODATA)
> >>>>>
> >>>>>Can you document why you don't retry on a CMD_IGNORED? I know
> >>>>>there was a
> >>>>>reason, I just can't remember it.
> >>>>
> >>>>CMD_IGNORED can be okay on broadcast. User of this API can retry all
> >>>>they
> >>>>want!
> >>>
> >>>So you retry if this is a CMD_FAILED but let higher levels retry for
> >>>CMD_IGNORED, sorry I don't see the logic.
> >>
> >>Yes that is right.
> >>
> >>If I am doing a broadcast read, lets say for Device Id registers, why in
> >>the
> >>world would I want to retry? CMD_IGNORED is a valid response and
> >>required to
> >>stop enumeration cycle in that case.

Above is the clarfication

> >>But if I am not expecting a CMD_IGNORED response, I can very well go
> >>ahead
> >>and retry from caller. The context is with caller and they can choose to
> >>do
> >>appropriate handling.
> >>
> >>And I have clarified this couple of times to you already, not sure how
> >>many
> >>more times I would have to do that.
> >
> >Until you clarify what you are doing.

Let me try again, I think u missed that part of my reply above

If I am doing a broadcast read, lets say for Device Id registers,
why in the world would I want to retry? CMD_IGNORED is a valid response
and required to stop enumeration cycle in that case.

> >There is ONE case where IGNORED is a valid answer (reading the Prepare not
> >finished bits), and it should not only be documented but analyzed in more
> >details.
> I meant Read SCP_DevID registers from Device0... prepare bits should never
> return a CMD_IGNORED

Precisely as I pointed out above.

> >For a write an IGNORED is never OK.

Agreed, but then transfer does both read and write. Write call can treat it
as error and read call leaves it upto caller.

Does that sound okay for you?

-- 
~Vinod