Messages in this thread | | | Date | Wed, 12 Jan 2022 13:05:02 +0000 | Subject | Re: [PATCH v6 08/10] ACPI / scan: Create platform device for CLSA0100 and CSC3551 ACPI nodes | From | Lucas tanure <> |
| |
On 12/20/21 17:24, Stefan Binding wrote: > Hi, > >> -----Original Message----- >> From: Hans de Goede <hdegoede@redhat.com> >> Sent: 17 December 2021 18:27 >> To: Rafael J. Wysocki <rafael@kernel.org>; Lucas Tanure >> <tanureal@opensource.cirrus.com>; Stefan Binding >> <sbinding@opensource.cirrus.com> >> Cc: Len Brown <lenb@kernel.org>; Mark Gross <markgross@kernel.org>; >> Liam Girdwood <lgirdwood@gmail.com>; Jaroslav Kysela <perex@perex.cz>; >> Mark Brown <broonie@kernel.org>; Takashi Iwai <tiwai@suse.com>; >> moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM... >> <alsa-devel@alsa-project.org>; ACPI Devel Maling List <linux- >> acpi@vger.kernel.org>; patches@opensource.cirrus.com; Platform Driver >> <platform-driver-x86@vger.kernel.org>; Linux Kernel Mailing List <linux- >> kernel@vger.kernel.org> >> Subject: Re: [PATCH v6 08/10] ACPI / scan: Create platform device for >> CLSA0100 and CSC3551 ACPI nodes >> >> Hi, >> >> On 12/17/21 18:19, Rafael J. Wysocki wrote: >>> On Fri, Dec 17, 2021 at 12:57 PM Lucas Tanure >>> <tanureal@opensource.cirrus.com> wrote: >>>> >>>> The ACPI device with CLSA0100 or CSC3551 is a sound card >>>> with multiple instances of CS35L41 connectec by I2C to >>> >>> "connected" I suppose? >>> >>>> the main CPU. >>>> >>>> We add an ID to the i2c_multi_instantiate_ids list to enumerate >>>> all I2C slaves correctly. >>>> >>>> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> >>> >>> This requires an ACK from Hans. >>> >>> If you receive one, please feel free to add my ACK to it too. >> >> One problem which I see here is that this change conflicts with >> this series: >> >> https://lore.kernel.org/all/20211210154050.3713-1- >> sbinding@opensource.cirrus.com/ >> >> I have reviewing that series on my todo list. >> >> One interesting question for you (Rafael) about that series is >> that i2c-multi-instantiate.c, which after the series also handles >> spi devices,is being moved to drivers/acpi . >> >> This is fine with me, but I wonder if it would not be better >> to keep it under drivers/platform/x86 ? Since the new SPI >> use-cases are also all on x86 laptops AFAICT. >> >> But back to this series, as said the 2 series conflict, since >> both are being submitted by @opensource.cirrus.com people, >> it would be good if the Cirrus folks can decide in which >> order these series should be merged. >> >> It might be best to just move this one patch to the other series? >> Thus removing the conflict between the 2 series. >> >> Regards, >> >> Hans >> > > We don’t really have a preference which order these two chains > should be merged in. We would rebase the other chain if one > got merged first. > If pushed for an answer, maybe: > https://lore.kernel.org/all/20211210154050.3713-1-sbinding@opensource.cirrus.com/ > should be merged first? > > Thanks, > Stefan > >> >> >>>> --- >>>> drivers/acpi/scan.c | 3 +++ >>>> drivers/platform/x86/i2c-multi-instantiate.c | 11 +++++++++++ >>>> 2 files changed, 14 insertions(+) >>>> >>>> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c >>>> index b7a6b982226e..8740cfa11f59 100644 >>>> --- a/drivers/acpi/scan.c >>>> +++ b/drivers/acpi/scan.c >>>> @@ -1712,8 +1712,11 @@ static bool >> acpi_device_enumeration_by_parent(struct acpi_device *device) >>>> static const struct acpi_device_id i2c_multi_instantiate_ids[] = { >>>> {"BSG1160", }, >>>> {"BSG2150", }, >>>> + {"CSC3551", }, >>>> {"INT33FE", }, >>>> {"INT3515", }, >>>> + /* Non-conforming _HID for Cirrus Logic already released */ >>>> + {"CLSA0100", }, >>>> {} >>>> }; >>>> >>>> diff --git a/drivers/platform/x86/i2c-multi-instantiate.c >> b/drivers/platform/x86/i2c-multi-instantiate.c >>>> index 4956a1df5b90..a889789b966c 100644 >>>> --- a/drivers/platform/x86/i2c-multi-instantiate.c >>>> +++ b/drivers/platform/x86/i2c-multi-instantiate.c >>>> @@ -147,6 +147,14 @@ static const struct i2c_inst_data int3515_data[] = >> { >>>> {} >>>> }; >>>> >>>> +static const struct i2c_inst_data cs35l41_hda[] = { >>>> + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, >>>> + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, >>>> + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, >>>> + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, >>>> + {} >>>> +}; >>>> + >>>> /* >>>> * Note new device-ids must also be added to i2c_multi_instantiate_ids in >>>> * drivers/acpi/scan.c: acpi_device_enumeration_by_parent(). >>>> @@ -154,7 +162,10 @@ static const struct i2c_inst_data int3515_data[] = >> { >>>> static const struct acpi_device_id i2c_multi_inst_acpi_ids[] = { >>>> { "BSG1160", (unsigned long)bsg1160_data }, >>>> { "BSG2150", (unsigned long)bsg2150_data }, >>>> + { "CSC3551", (unsigned long)cs35l41_hda }, >>>> { "INT3515", (unsigned long)int3515_data }, >>>> + /* Non-conforming _HID for Cirrus Logic already released */ >>>> + { "CLSA0100", (unsigned long)cs35l41_hda }, >>>> { } >>>> }; >>>> MODULE_DEVICE_TABLE(acpi, i2c_multi_inst_acpi_ids); >>>> -- >>>> 2.34.1 >>>> >>> > > As the ic2-multi-instantiate patch chain is still being worked out, we would like to submit a new chain for CLSA0100 id and a few fixes for the HDA cs35l41 driver. And to avoid conflicts the ic2-multi-instantiate patch chain will wait for this new patch chain to be merged.
Thanks, Lucas Tanure
| |