Re: [PATCH v2] soundwire: SDCA: add helper macro to access controls

[Vinod Koul <vkoul@kernel.org>]

On 17-08-20, 10:14, Pierre-Louis Bossart wrote:
> 
> > > The upcoming SDCA (SoundWire Device Class Audio) specification defines
> > > a hiearchical encoding to interface with Class-defined capabilities,
> > 
> > typo hiearchical
> 
> ok
> 
> > > based on which audio function, entity, control and channel being used.
> > 
> > Can you please elaborate on what do these terms refer to?
> > 
> > Also can we have some documentation for this and how Linux is going to
> > use it..
> 
> These are concepts in the SDCA draft spec, and that should be the reference.
> We worked with MIPI so that this spec will be available with a click-through
> agreement when ratified, for now it's only available to contributors per
> MIPI bylaws.
> 
> If you do not have access to this specification, then that's a real problem.
> Maybe you need to let Bard take care of this part as a co-maintainer?
> 
> The goal with this macro is to enable a first set of codecs drivers using
> these concepts to be released upstream. All you need to know at this point
> is that controls are defined in a hierarchical way and accessed with a
> read/write transaction from/to the address created with the following macro.

Hmmm, if we cannot get some kind of Documentation of what it means and
review the code, then I do not see a point in getting this into kernel.

As kernel community we would like to see some form of Documentation
associated with the patches on what this means.

If that is not possible due to MIPI regulations, maybe deferring
this would make sense

Thanks
> 
> > > +/* v1.2 device - SDCA address mapping */
> > > +#define SDW_SDCA_CTL(fun, ent, ctl, ch)		(BIT(30) |			\
> > > +						 (((fun) & 0x7) << 22) |	\
> > > +						 (((ent) & 0x40) << 15) |	\
> > > +						 (((ent) & 0x3f) << 7) |	\
> > > +						 (((ctl) & 0x30) << 15) |	\
> > > +						 (((ctl) & 0x0f) << 3) |	\
> > > +						 (((ch) & 0x38) << 12) |	\
> > > +						 ((ch) & 0x07))
> > > +
> > 
> > how about adding an underscore to the arguments here:
> > 
> > #define SDW_SDCA_CTL(_fun, _ent, _ctl, _ch)
> > and so on..
> 
> I checked the SoundWire defines and the vast majority of the macros don't
> use underscores, and when they do there's no consistency between 1 or 2
> underscores.

-- 
~Vinod