[alsa-devel] [PATCH 2/4] soundwire: cdns: fix check for number of messages

[Vinod Koul]

On 28-03-19, 14:58, Srinivas Kandagatla wrote:
> 
> 
> On 28/03/2019 14:03, Pierre-Louis Bossart wrote:
> > > ---
> > >   drivers/soundwire/cadence_master.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/soundwire/cadence_master.c
> > > b/drivers/soundwire/cadence_master.c
> > > index cb6a331f448a..d41adbc57918 100644
> > > --- a/drivers/soundwire/cadence_master.c
> > > +++ b/drivers/soundwire/cadence_master.c
> > > @@ -434,7 +434,7 @@ cdns_xfer_msg_defer(struct sdw_bus *bus,
> > >       int cmd = 0, ret;
> > >       /* for defer only 1 message is supported */
> > > -    if (msg->len > 1)
> > 
> > I am not sure this is a typo.
> > 
> 
> Am bit confused with defer data-structure itself.
> 
> I was assuming that defer->length is number of sdw messages in the
> structure. If its same as message lenght then sdw_msg->len and
> sdw_defer->length are totally redundant. May be we should get rid of lenght
> field in defer to avoid confusion?

Sorry Srini, I didnt pay much attention to it last time we discussed. So
relooking at the code, we send msg as a message to trf (defered or not)
and then copy length few lines down, but I agree this is not most useful
and should be cleaned up.

> > IRRC the hardware only defer e.g. a single write that can perform bank
> > switches and writes at specific times indicated with an SSP. What's more
> okay got it.
> Does that mean that candence controller can only support 1 byte defer
> transfers?

I think so, Pierre can confirm.

> > is that the defer structure is actually modified below this code (not
> > shown in the diff) to set defer>length = msg->len, so testing before the
> > value is set looks more suspicious than the current code.
> 
> removing the length field in defer should get rid of such instances.

Right!

-- 
~Vinod