Re: [PATCH v6 08/10] ACPI / scan: Create platform device for CLSA0100 and CSC3551 ACPI nodes

[Lucas tanure]

On 12/20/21 17:24, Stefan Binding wrote:
> Hi,
>
> > -----Original Message-----
> > From: Hans de Goede <hdegoede@xxxxxxxxxx>
> > Sent: 17 December 2021 18:27
> > To: Rafael J. Wysocki <rafael@xxxxxxxxxx>; Lucas Tanure
> > <tanureal@xxxxxxxxxxxxxxxxxxxxx>; Stefan Binding
> > <sbinding@xxxxxxxxxxxxxxxxxxxxx>
> > Cc: Len Brown <lenb@xxxxxxxxxx>; Mark Gross <markgross@xxxxxxxxxx>;
> > Liam Girdwood <lgirdwood@xxxxxxxxx>; Jaroslav Kysela <perex@xxxxxxxx>;
> > Mark Brown <broonie@xxxxxxxxxx>; Takashi Iwai <tiwai@xxxxxxxx>;
> > moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...
> > <alsa-devel@xxxxxxxxxxxxxxxx>; ACPI Devel Maling List <linux-
> > acpi@xxxxxxxxxxxxxxx>; patches@xxxxxxxxxxxxxxxxxxxxx; Platform Driver
> > <platform-driver-x86@xxxxxxxxxxxxxxx>; Linux Kernel Mailing List <linux-
> > kernel@xxxxxxxxxxxxxxx>
> > 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@xxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxxxxxxxxxxxxx/
> >
> > 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@xxxxxxxxxxxxxxxxxxxxx/
> 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