RE: [PATCh v3 04/14] ASoC: rsnd: Add RZ/G3E SoC probing and register map

[John Madieu]

Hi Mark, Kuninori,

Thanks for your reviews.

> -----Original Message-----
> From: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> Sent: Friday, April 3, 2026 2:36 AM
> To: Mark Brown <broonie@xxxxxxxxxx>
> Subject: Re: [PATCh v3 04/14] ASoC: rsnd: Add RZ/G3E SoC probing and
> register map
> 
> 
> Hi John, Mark
> 
> Thank you for the patch
> 
> > > @@ -629,7 +632,9 @@ struct rsnd_priv {
> > >  #define RSND_GEN4	(4 << 0)
> > >  #define RSND_SOC_MASK	(0xFF << 4)
> > >  #define RSND_SOC_E	(1 << 4) /* E1/E2/E3 */
> > > -
> > > +#define RSND_RZ_MASK	(0xFF << 8)
> >
> > This overlaps with RSND_SOC_MASK.  That might be intentional but I'm
> > really not sure and it's all kind of unclear.
> 
> Indeed.
> But it seems the strange is RSND_SOC_MASK, it should be
> 
> 	- #define RSND_SOC_MASK	(0xFF << 4)
> 	+ #define RSND_SOC_MASK	(0xF  << 4)
> 
> I think it is good timing to have comment to clarify flag.
> 
> 	0x....DCBA
> 	A:	R-Car GenX
> 	B:	R-Car GenX identifier (if needed)
> 	C:	RZ series
> 	D:	RZ series  identifier (if needed)
> 

Agreed for the overlap. I'll fix RSND_SOC_MASK to (0xF << 4), keep
RSND_RZ_MASK as (0xF << 8) for nibble C, and add RSND_RZ_ID_MASK
(0xF << 12) for nibble D. I'll add the comment block documenting the
nibble layout as Kuninori suggested.

/*
 * Flag layout: 0x....DCBA
 *
 * A (bits  3: 0): R-Car generation (Gen1/Gen2,Gen3,Gen4)
 * B (bits  7: 4): R-Car SoC variant (e.g. SOC_E for E1/E2/E3)
 * C (bits 11: 8): RZ series generation
 * D (bits 14:12): RZ series SoC indentifier (e.g. RZG3E)
 *
 * Bits 16+ are used for capability flags
 *
 */

I keep the existing shift-based style rather than GENMASK()/FIELD_PREP()
for consistency with the driver's convention. Please let me know if you'd
prefer otherwise.

Regards,
John