On Tue, Feb 01, 2022 at 11:07:45AM -0800, Curtis Malainey wrote: > On Tue, Feb 1, 2022 at 10:40 AM Mark Brown <broonie@xxxxxxxxxx> wrote: > > In general if there's something like a speaker amplifier that's just > > controlled via GPIO I'd expect that to be something that's set up by the > > machine driver. If the CODEC is a GPIO provider then the pattern should > Just to confirm, by provider, you mean it has on codec gpio it is > exposing to the kernel correct? Interestingly, this appears to be > counter to the max98357a.c driver. It has the SDMODE pin which can put > it into a low power state. The codec driver appears to consume this > pin from FW lookup and toggle it on trigger. I am just curious why we > would not want the codec to handle its own pins? That way the control > logic for the pin is collected into a non-chipset dependent location > that is close to the internal state of the driver. Bear in mind that I'm just reading some e-mail about a machine driver here, the most common case for a GPIO for an amplifier is something where there's a very simple analogue amplifier with just a GPIO to mute it that's been attached to a CODEC or if it's something that goes into a device that's otherwise visible to software. I don't have immediate visibility of the full set of machines and CODECs here. Like I say the state of ACPI firmware description really doesn't help here. > The board this patch fixes is not shipping, the board it breaks is > planned to ship from my understanding. Eric, feel free to correct me. > We could do a partial revert and remove the _NP fields and both boards > would work (Sujith already sent a patch for this "ASoC: amd: acp: Set > gpio_spkr_en to None for max speaker amplifer in machine driver") but > I think we should still consider a patch to stop hard coding the GPIO > as it should be available via a lookup. Are there other systems that aren't Chromebooks being covered here, and if so what state are they in? In any case what I'd expect to see here is a series transitioning the existing code to use lookups from the firmware.
Attachment:
signature.asc
Description: PGP signature
