Re: [EXTERNAL] Re: [PATCH v6 1/4] ASoc: PCM6240: Create PCM6240 Family driver code

From: Mark Brown
Date: Wed Feb 28 2024 - 07:46:52 EST


On Wed, Feb 28, 2024 at 04:20:16AM +0000, Ding, Shenghao wrote:
> > From: Mark Brown <broonie@xxxxxxxxxx>
> > On Fri, Feb 23, 2024 at 08:38:04PM +0800, Shenghao Ding wrote:

> > > +static const char *const pcmdev_ctrl_name[] = {
> > > + "%s-i2c-%d-dev%d-ch%d-ana-gain",
> > > + "%s-i2c-%d-dev%d-ch%d-digi-gain",
> > > + "%s-i2c-%d-dev%d-ch%d-fine-gain",
> > > +};

> > So, I see why you're doing this naming thing for the per-device controls
> > - the device can (and is designed to) allow controlling multiple amps with a
> > single I2C write. However this is resulting in something that's really awkward
> > from an ALSA point of view, the names that are being generated are very
> > much not idiomatic for control names and probably aren't going to be
> > terribly meaningful for end users since they're not associated with where the

> For only single one device, all these seems redundant and makes uncomfortable
> Many of my customers will have such cases as
> Four pieces of Pcm6240s and three pieces of pcm6360s in the same i2c bus.
> The chip name can help them identify the chip and set the right chip and the right channel.

The problem is not with having the names, the problem is that the names
look nothing like how normal ALSA names for deduplicating controls look
- normally these are specified by the board and are human legible rather
than based on the numbers of buses inside the system.

> > I think it would make sense to do a version the driver with no user visible
> > controls (or at least none that need this name generation stuff) and then
> > make the controls an incremental patch, the driver would obviously need
> > both bits to be properly useful but it'd mean that the simple bit could get
> > reviewed and possibly merged separately to the complicated bit which would
> > probably be easier.

> You means to remove the kcontrols first. After the patch was accepted, add these
> Kcontrols, right?

Yes, or even just split the controls out so they're a later patch in the
series - no need to wait before sending the code, just sort it after the
rest of the driver.

Attachment: signature.asc
Description: PGP signature