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@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
