Re: [RFC PATCH] soundwire: bus: Add flag to mark DPN_BlockCtrl1 as readonly

[Srinivas Kandagatla]

Thanks for the inputs

On 10/03/2020 15:53, Pierre-Louis Bossart wrote:
> Hi Srinivas,
>
> > > Â> My recommendation would be to add a DisCo property stating the
> > > WordLength value can be used by the bus code but not written to the 
> > > Slave device registers.
> >
> > Does something like "mipi-sdw-read-only-wordlength" as slave property, 
> > make sense?
>
> The properties can be handled at two levels.
>
> First, you'd want to change include/linux/soundwire/sdw.h, and add a new 
> field in
>
> struct sdw_dpn_prop {
>  ÂÂÂÂu32 num;
>  ÂÂÂÂu32 max_word;
>  ÂÂÂÂu32 min_word;
>  ÂÂÂÂu32 num_words;
>  ÂÂÂÂu32 *words;
> +ÂÂÂÂÂÂ bool read_only_wordlength;
>
> Once this is added, along with the code that bypasses the programming of 
> DPn_BlockCtrl1, the implementation has two choices:
>
> a) hard-code the field value in the codec driver.

This totally works for me.

> b) read the property from firmware with the DisCo helpers.

I would defer adding this for now till there is a real users for this.

> There is no requirement that all properties be read from firmware, and 
> if you look at existing code base sdw_slave_read_prop() is currently 
> unused, each codec implements its own .read_prop() callback.
>
> We really wanted to be pragmatic, and give the possibility to either 
> override bad firmware or extend incomplete firmware to avoid coupling OS 
> and firmware too much. If you foresee cases where this implementation 
> might vary and firmware distribution is not a problem, then a property 
> read would make sense.
>
> Just once procedural reminder that all 'mipi-sdw' properties are handled 
> by the MIPI software WG, so we'd need to have this property added in a 
> formal MIPI document update.
>
> I suggest you talk with Lior first on this.

Sure, I will talk to him.

> Hope this helps
> -Pierre