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

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

[Bard Liao]

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The upcoming SDCA (SoundWire Device Class Audio) specification defines
a hiearchical encoding to interface with Class-defined capabilities,
based on which audio function, entity, control and channel being used.

The specification is not yet accessible to the general public but this
information is released with explicit permission from the MIPI Board
to avoid delays with SDCA support on Linux platforms.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---

Changelog:

v2:
 - add SDW_SDCA_MBQ_CTL

---
 include/linux/soundwire/sdw_registers.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/soundwire/sdw_registers.h b/include/linux/soundwire/sdw_registers.h
index 5d3c271af7d1..4517c0f65b4f 100644
--- a/include/linux/soundwire/sdw_registers.h
+++ b/include/linux/soundwire/sdw_registers.h
@@ -305,4 +305,16 @@
 #define SDW_CASC_PORT_MASK_INTSTAT3		1
 #define SDW_CASC_PORT_REG_OFFSET_INTSTAT3	2
 
+/* 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))
+
+#define SDW_SDCA_MBQ_CTL(reg)			((reg) | BIT(13))
+
 #endif /* __SDW_REGISTERS_H */
-- 
2.17.1

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

[Vinod Koul]

On 17-08-20, 04:10, Bard Liao wrote:
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
> The upcoming SDCA (SoundWire Device Class Audio) specification defines
> a hiearchical encoding to interface with Class-defined capabilities,

typo hiearchical

> 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..

> 
> The specification is not yet accessible to the general public but this
> information is released with explicit permission from the MIPI Board
> to avoid delays with SDCA support on Linux platforms.
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> ---
> 
> Changelog:
> 
> v2:
>  - add SDW_SDCA_MBQ_CTL
> 
> ---
>  include/linux/soundwire/sdw_registers.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/linux/soundwire/sdw_registers.h b/include/linux/soundwire/sdw_registers.h
> index 5d3c271af7d1..4517c0f65b4f 100644
> --- a/include/linux/soundwire/sdw_registers.h
> +++ b/include/linux/soundwire/sdw_registers.h
> @@ -305,4 +305,16 @@
>  #define SDW_CASC_PORT_MASK_INTSTAT3		1
>  #define SDW_CASC_PORT_REG_OFFSET_INTSTAT3	2
>  
> +/* 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..

-- 
~Vinod

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

[Pierre-Louis Bossart]

>> 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.

>> +/* 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.

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

[Vinod Koul]

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