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

[Srinivas Kandagatla <srinivas.kandagatla@linaro.org>]

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