* [alsa-devel] Right place for defining ACPI GPIO mapping for codec
@ 2020-01-06 10:21 Yauhen Kharuzhy
2020-01-06 11:09 ` Hans de Goede
0 siblings, 1 reply; 3+ messages in thread
From: Yauhen Kharuzhy @ 2020-01-06 10:21 UTC (permalink / raw)
To: alsa-devel, Hans de Goede
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?
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
{
}
}
--
Yauhen Kharuzhy
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [alsa-devel] Right place for defining ACPI GPIO mapping for codec
2020-01-06 10:21 [alsa-devel] Right place for defining ACPI GPIO mapping for codec Yauhen Kharuzhy
@ 2020-01-06 11:09 ` Hans de Goede
2020-01-06 22:52 ` Yauhen Kharuzhy
0 siblings, 1 reply; 3+ messages in thread
From: Hans de Goede @ 2020-01-06 11:09 UTC (permalink / raw)
To: Yauhen Kharuzhy, alsa-devel
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
> {
> }
> }
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [alsa-devel] Right place for defining ACPI GPIO mapping for codec
2020-01-06 11:09 ` Hans de Goede
@ 2020-01-06 22:52 ` Yauhen Kharuzhy
0 siblings, 0 replies; 3+ messages in thread
From: Yauhen Kharuzhy @ 2020-01-06 22:52 UTC (permalink / raw)
To: Hans de Goede; +Cc: alsa-devel
On Mon, Jan 06, 2020 at 12:09:32PM +0100, Hans de Goede wrote:
> 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
OK, thanks for the answer. This sounds not ideal but reasonable, I will go
by such way.
--
Yauhen Kharuzhy
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-01-07 10:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-06 10:21 [alsa-devel] Right place for defining ACPI GPIO mapping for codec Yauhen Kharuzhy
2020-01-06 11:09 ` Hans de Goede
2020-01-06 22:52 ` Yauhen Kharuzhy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.