Re: [PATCH 3/4] soundwire: SDCA: add helper macro to access controls

2020-08-26 Thread Vinod Koul
On 26-08-20, 10:00, Pierre-Louis Bossart wrote:
> 
> 
> > > +/* v1.2 device - SDCA address mapping */
> > 
> > Can you please add description of bits used by each field here,
> > something like we have done for DevId
> 
> were you referring to something like this?
> 
>  * Spec definition
>  *   Register Bit Contents
>  *   DevId_0 [7:4]47:44   sdw_version
>  *   DevId_0 [3:0]43:40   unique_id
>  *   DevId_1  39:32   mfg_id [15:8]
>  *   DevId_2  31:24   mfg_id [7:0]
>  *   DevId_3  23:16   part_id [15:8]
>  *   DevId_4  15:08   part_id [7:0]
>  *   DevId_5  07:00   class_id

Correct

> > 
> > > +#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))
> > 
> > GENMASK() for the bitmaps here please. Also it would look very neat by
> > using FIELD_PREP() here, you can skip the bit shifts and they would be
> > done by FIELD_PREP() for you.
> 
> ok.

FWIW I am testing changes to do the conversion for subsystem to use nice
stuff in bitfield.h


-- 
~Vinod


Re: [PATCH 3/4] soundwire: SDCA: add helper macro to access controls

2020-08-26 Thread Pierre-Louis Bossart





+/* v1.2 device - SDCA address mapping */


Can you please add description of bits used by each field here,
something like we have done for DevId


were you referring to something like this?

 * Spec definition
 *   Register   Bit Contents
 *   DevId_0 [7:4]  47:44   sdw_version
 *   DevId_0 [3:0]  43:40   unique_id
 *   DevId_139:32   mfg_id [15:8]
 *   DevId_231:24   mfg_id [7:0]
 *   DevId_323:16   part_id [15:8]
 *   DevId_415:08   part_id [7:0]
 *   DevId_507:00   class_id




+#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))


GENMASK() for the bitmaps here please. Also it would look very neat by
using FIELD_PREP() here, you can skip the bit shifts and they would be
done by FIELD_PREP() for you.


ok.


Re: [PATCH 3/4] soundwire: SDCA: add helper macro to access controls

2020-08-26 Thread Vinod Koul
On 25-08-20, 12:16, Pierre-Louis Bossart wrote:
> The upcoming SDCA (SoundWire Device Class Audio) specification defines
> a hierarchical encoding to interface with Class-defined capabilities.
> 
> 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.
> 
> A block of 64 MBytes of register addresses are allocated to SDCA
> controls, starting at address 0x4000. The 26 LSBs which identify
> individual controls are set based on the following variables:
> 
> - Function Number. An SCDA device can be split in up to 8 independent
>   Functions. Each of these Functions is described in the SDCA
>   specification, e.g. Smart Amplifier, Smart Microphone, Simple
>   Microphone, Jack codec, HID, etc.
> 
> - Entity Number.  Within each Function,  an Entity is  an identifiable
>   block.  Up   to  127  Entities   are  connected  in   a  pre-defined
>   graph  (similar to  USB), with  Entity0 reserved  for Function-level
>   configurations.  In  contrast  to  USB, the  SDCA  spec  pre-defines
>   Function Types, topologies, and allowed  options, i.e. the degree of
>   freedom  is not  unlimited to  limit  the possibility  of errors  in
>   descriptors leading to software quirks.
> 
> - Control Selector. Within each Entity, the SDCA specification defines
>   48 controls such as Mute, Gain, AGC, etc, and 16 implementation
>   defined ones. Some Control Selectors might be used for low-level
>   platform setup, and other exposed to applications and users. Note
>   that the same Control Selector capability, e.g. Latency control,
>   might be located at different offsets in different entities, the
>   Control Selector mapping is Entity-specific.
> 
> - Control Number. Some Control Selectors allow channel-specific values
>   to be set, with up to 64 channels allowed. This is mostly used for
>   volume control.
> 
> - Current/Next values. Some Control Selectors are
>   'Dual-Ranked'. Software may either update the Current value directly
>   for immediate effect. Alternatively, software may write into the
>   'Next' values and update the SoundWire 1.2 'Commit Groups' register
>   to copy 'Next' values into 'Current' ones in a synchronized
>   manner. This is different from bank switching which is typically
>   used to change the bus configuration only.
> 
> - MBQ. the Multi-Byte Quantity bit is used to provide atomic updates
>   when accessing more that one byte, for example a 16-bit volume
>   control would be updated consistently, the intermediate values
>   mixing old MSB with new LSB are not applied.
> 
> These 6 parameters are used to build a 32-bit address to access the
> desired Controls. Because of address range, paging is required, but
> the most often used parameter values are placed in the lower 16 bits
> of the address. This helps to keep the paging registers constant while
> updating Controls for a specific Device/Function.
> 
> Reviewed-by: Rander Wang 
> Reviewed-by: Guennadi Liakhovetski 
> Reviewed-by: Kai Vehmanen 
> Signed-off-by: Pierre-Louis Bossart 
> ---
>  include/linux/soundwire/sdw_registers.h | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/include/linux/soundwire/sdw_registers.h 
> b/include/linux/soundwire/sdw_registers.h
> index 5d3c271af7d1..906dadda7387 100644
> --- a/include/linux/soundwire/sdw_registers.h
> +++ b/include/linux/soundwire/sdw_registers.h
> @@ -305,4 +305,17 @@
>  #define SDW_CASC_PORT_MASK_INTSTAT3  1
>  #define SDW_CASC_PORT_REG_OFFSET_INTSTAT32
>  
> +/* v1.2 device - SDCA address mapping */

Can you please add description of bits used by each field here,
something like we have done for DevId

> +#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))

GENMASK() for the bitmaps here please. Also it would look very neat by
using FIELD_PREP() here, you can skip the bit shifts and they would be
done by FIELD_PREP() for you.

> -- 
> 2.25.1

-- 
~Vinod


Re: [PATCH 3/4] soundwire: SDCA: add helper macro to access controls

2020-08-25 Thread Bard liao



On 8/26/2020 1:16 AM, Pierre-Louis Bossart wrote:

The upcoming SDCA (SoundWire Device Class Audio) specification defines
a hierarchical encoding to interface with Class-defined capabilities.

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.

A block of 64 MBytes of register addresses are allocated to SDCA
controls, starting at address 0x4000. The 26 LSBs which identify
individual controls are set based on the following variables:

- Function Number. An SCDA device can be split in up to 8 independent
   Functions. Each of these Functions is described in the SDCA
   specification, e.g. Smart Amplifier, Smart Microphone, Simple
   Microphone, Jack codec, HID, etc.

- Entity Number.  Within each Function,  an Entity is  an identifiable
   block.  Up   to  127  Entities   are  connected  in   a  pre-defined
   graph  (similar to  USB), with  Entity0 reserved  for Function-level
   configurations.  In  contrast  to  USB, the  SDCA  spec  pre-defines
   Function Types, topologies, and allowed  options, i.e. the degree of
   freedom  is not  unlimited to  limit  the possibility  of errors  in
   descriptors leading to software quirks.

- Control Selector. Within each Entity, the SDCA specification defines
   48 controls such as Mute, Gain, AGC, etc, and 16 implementation
   defined ones. Some Control Selectors might be used for low-level
   platform setup, and other exposed to applications and users. Note
   that the same Control Selector capability, e.g. Latency control,
   might be located at different offsets in different entities, the
   Control Selector mapping is Entity-specific.

- Control Number. Some Control Selectors allow channel-specific values
   to be set, with up to 64 channels allowed. This is mostly used for
   volume control.

- Current/Next values. Some Control Selectors are
   'Dual-Ranked'. Software may either update the Current value directly
   for immediate effect. Alternatively, software may write into the
   'Next' values and update the SoundWire 1.2 'Commit Groups' register
   to copy 'Next' values into 'Current' ones in a synchronized
   manner. This is different from bank switching which is typically
   used to change the bus configuration only.

- MBQ. the Multi-Byte Quantity bit is used to provide atomic updates
   when accessing more that one byte, for example a 16-bit volume
   control would be updated consistently, the intermediate values
   mixing old MSB with new LSB are not applied.

These 6 parameters are used to build a 32-bit address to access the
desired Controls. Because of address range, paging is required, but
the most often used parameter values are placed in the lower 16 bits
of the address. This helps to keep the paging registers constant while
updating Controls for a specific Device/Function.

Reviewed-by: Rander Wang 
Reviewed-by: Guennadi Liakhovetski 
Reviewed-by: Kai Vehmanen 
Signed-off-by: Pierre-Louis Bossart 


Acked-by: Bard Liao