Re: [alsa-devel] Right place for defining ACPI GPIO mapping for codec

6 Jan 2020


      Hi,
On 06-01-2020 11:21, Yauhen Kharuzhy wrote:
...
Hello,
I am working now to get sound working at Lenovo Yoga Book tablet. It is
Intel CherryTrail-based device and has RT5677 sound codec.
The RT5677 codec driver uses two GPIOs to reset and enable the codec,
with names 'realtek,reset' and 'realtek,pow-ldo2'. The ACPI definition lacks a
  _DSD section with GPIO name->CRS ID definition, so I need to manually
define this mapping somewhere using existing
devm_acpi_dev_add_driver_gpios() method (various devices has various _CRS
  definitions order and content, so this is cannot be placed into
rt5677.c codec driver).
The most obvious place for this is ASoC machine driver for my device, but the
codec driver is binded to the ACPI device and initializes before machine
driver.
Can somebody advice how to define such GPIOs mapping for codec in my
case?
Hmm, so normally I would say to move the devm_gpiod_get_optional calls
into the component probe part of the codec driver (rt5677_probe) which
does run after the machine driver, but it seems that the GPIOs must
be driven to the correct values before we can do any i2c transfers.
You could move everything below (and including) the devm_gpiod_get_optional calls
from rt5677_i2c_probe to the top of rt5677_probe, but then you will also
be moving these calls:
rt5677_init_gpio(i2c);
         ret = rt5677_init_irq(i2c);
Which register a GPIO chip themselves, which may be a dependency for probing
other bits of the sound stack so moving those 2 calls is a bad idea.
This means that the codec-driver itself and specifically the  rt5677_i2c_probe()
function is pretty much the only remaining place where you can add the
devm_acpi_dev_add_driver_gpios() call. Note that you may also need to set some
pdata settings. For the rt5640 and rt5651 drivers we set some device properties
from the machine driver and check those in the component probe function. But
rt5677 already depends on various props inside the i2c probe function.
Note that taking care of machine specific bits in the codec driver is not
unheard of, the rt5645.c also does this and includes a DMI table for this
even though typically this would be more appropriate for the machine driver.
So in this case given the constraints I think it is fine to de a DMI match
and add the devm_acpi_dev_add_driver_gpios() call based on that to the
codec driver itself, like we are doing in sound/soc/codecs/rt5645.c, you
can then also set some of the pdata based on the DMI match as needed.
For now I would not worry about making this generic, my suggestion would
be to add a "rt5677_lenovo_yogabook_fixup" function which stars with the
DMI check (and bails if it fails) and then does whatever you need wrt
both the devm_acpi_dev_add_driver_gpios() call and the pdata settings.
And then add a call to rt5677_lenovo_yogabook_fixup() directly under the
rt5677_read_device_properties() call in rt5677_i2c_probe().
We can worry about making the X86 machine specific fixups more generic
when we need to add them for a second X86 device.
Regards,
Hans
...
The ACPI definition of sound device is below. Second I2C address
definition is for TS3A277E jack detector. The first GPIO in the _CRS is
codec reset, the second is pow-ldo2.
Device (RTEK)
{
     Name (_ADR, Zero)  // _ADR: Address
     Name (_HID, "10EC5677")  // _HID: Hardware ID
     Name (_CID, "10EC5677")  // _CID: Compatible ID
     Name (_DDN, "Realtek IIS Audio Codec")  // _DDN: DOS Device Name
     Name (_SUB, "17AA7005")  // _SUB: Subsystem ID
     Name (_UID, One)  // _UID: Unique ID
     Name (_PR0, Package (0x01)  // _PR0: Power Resources for D0
     {
         CLK3
     })
     Name (CHAN, Package (0x02)
     {
         One,
         0x0124F800
     })
     Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
     {
         Name (SBUF, ResourceTemplate ()
         {
             I2cSerialBusV2 (0x002C, ControllerInitiated, 0x000186A0,
                 AddressingMode7Bit, "\_SB.PCI0.I2C1",
                 0x00, ResourceConsumer, , Exclusive,
                 )
             I2cSerialBusV2 (0x003B, ControllerInitiated, 0x000186A0,
                 AddressingMode7Bit, "\_SB.PCI0.I2C1",
                 0x00, ResourceConsumer, , Exclusive,
                 )
             GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
                 "\_SB.GPO3", 0x00, ResourceConsumer, ,
                 )
                 {   // Pin list
                     0x0019
                 }
             GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
                 "\_SB.GPO3", 0x00, ResourceConsumer, ,
                 )
                 {   // Pin list
                     0x0012
                 }
             GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
                 "\_SB.GPO3", 0x00, ResourceConsumer, ,
                 )
                 {   // Pin list
                     0x0030
                 }
             GpioInt (Edge, ActiveLow, Exclusive, PullNone, 0x0000,
                 "\_SB.GPO0", 0x00, ResourceConsumer, ,
                 )
                 {   // Pin list
                     0x005B
                 }
             GpioInt (Edge, ActiveLow, Exclusive, PullNone, 0x0000,
                 "\_SB.GPO0", 0x00, ResourceConsumer, ,
                 )
                 {   // Pin list
                     0x004D
                 }
             SpiSerialBusV2 (0x0001, PolarityLow, FourWireMode, 0x08,
                 ControllerInitiated, 0x003D0900, ClockPolarityHigh,
                 ClockPhaseSecond, "\_SB.PCI0.SPI1",
                 0x00, ResourceConsumer, , Exclusive,
                 )
         })
         Return (SBUF) /* _SB_.PCI0.I2C1.RTEK._CRS.SBUF */
     }
 Method (_STA, 0, NotSerialized)  // _STA: Status
 {
     Return (0x0F)
 }

 Method (_DIS, 0, NotSerialized)  // _DIS: Disable Device
 {
 }

}

Re: [alsa-devel] Right place for defining ACPI GPIO mapping for codec

Hans de Goede