[PATCH 1/4] ASoC/SoundWire: rt1316: Add RT1316 SDCA vendor-specific driver

Mark Brown broonie at kernel.org
Thu Dec 3 13:38:02 CET 2020


On Wed, Dec 02, 2020 at 10:38:42PM +0800, Bard Liao wrote:

This looks mostly good, a few small things below - if I apply please fix
incrementally:

> +static const char * const rt1316_xu24_bypass_ctl[] = {
> +	"Not Bypass",
> +	"Bypass",
> +};

Why is this not a Switch control?  If there's no strong reason please
submit an incremental patch converting it to one.

> +/* RT1316 SDCA function topology */
> +#define FUN_SMART_AMP 0x04

Full marks for picking this constant!

> +/* RT1316 SDCA channel */
> +#define CH_L 0x01
> +#define CH_R 0x02
> +
> +/* Power State */
> +#define PS0 0x00
> +#define PS3 0x03
> +
> +/* Mute Control */
> +#define UNMUTE 0x00
> +#define MUTE 0x01

A bunch of these could use namespacing, at least MUTE doesn't seem to be
used.

> +static const struct reg_default rt1316_reg_defaults[] = {
> +	{ 0x3004, 0x00 },

This should be in the driver, not the header file.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20201203/c880818e/attachment.sig>


More information about the Alsa-devel mailing list