Re: [PATCH v2] ASoC: tas5805m: fix pdn polarity

[John Keeping <john@metanate.com>]

On Wed, Mar 09, 2022 at 09:55:40PM +0000, Mark Brown wrote:
> On Wed, Mar 09, 2022 at 08:16:07PM +0000, John Keeping wrote:
> > On Wed, Mar 09, 2022 at 04:28:30PM +0000, Mark Brown wrote:
> 
> > > I think the device tree binding needs to be clarified here to be
> > > explicit about this since there's obviously some room for user confusion
> > > here.  We can probably get away with a change at this point since it's
> > > not hit a release but we do need to try to avoid the situation where any
> > > other implementations use active high polarity for the bindings.
> 
> > Taking a quick survey of the other devices that have a pdn-gpios
> > property:
> 
> > - tvp5150 is correct with the driver setting 0 to make the device active
> 
> > - tas571x also sets 0 to make the device active
> 
> > - ak4375 uses the opposite sense setting PDN = 1 to make the device
> >   active; this has no in-tree users and was merged as part of v5.17-rc1
> >   so it's not in a released kernel yet
> 
> Sure, I still think it would be good to update the binding document to
> clarify things as part of your patch - the binding currently just has it
> as the "pdn" pin not the /pdn pin or anything.

I've been thinking about this but I can't really think what to say.
tas571x's binding says:

	GPIO specifier for the TAS571x's active low powerdown line

Is that the sort of wording you have in mind?

To me it seems like a general principle that the GPIO_ACTIVE_{HIGH,LOW}
flags should be used to indicate how the pin works so that the driver
consistently uses logical levels regardless of how the hardware is
wired.

From the driver point of view pdn-gpios is effectively reset-gpios by
another name and it's pretty consistent that setting a reset GPIO to 1
means the device is inaccessible.

Maybe this just means I'm approaching this "down" from the software
abstraction more than "up" from the hardware.