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@kernel.org 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.