mbox series

[v2,0/6] Support Spi in i2c-multi-instantiate driver

Message ID 20211210154050.3713-1-sbinding@opensource.cirrus.com
Headers show
Series Support Spi in i2c-multi-instantiate driver | expand

Message

Stefan Binding Dec. 10, 2021, 3:40 p.m. UTC 
Add support for SPI bus in the ic2-multi-instantiate driver as
upcoming laptops will need to multi instantiate SPI devices from
a single device node, which has multiple SpiSerialBus entries at
the ACPI table.

With the new SPI support, i2c-multi-instantiate becomes
bus-multi-instantiate and is moved to the ACPI folder.

The intention is to support the SPI bus by re-using the current
I2C multi instantiate, instead of creating a new SPI multi
instantiate, to make it possible for peripherals that can be
controlled by I2C or SPI to have the same HID at the ACPI table.

The new driver (Bus multi instantiate, bmi) checks for the
hard-coded bus type and returns -ENODEV in case of zero devices
found for that bus. In the case of automatic bus detection, 
the driver will give preference to I2C.

The expectation is for a device node in the ACPI table to have
multiple I2cSerialBus only or multiple SpiSerialBus only, not
a mix of both; and for the case where there are both entries in
one device node, only the I2C ones would be probed.

This new bus multi instantiate will be used in CS35L41 HDA new
driver, being upstreamed:
https://lkml.org/lkml/2021/11/23/723

Changes since V1:
 - Added Cover Letter
 - Split SPI patch into move, rename, reorganize and add
   SPI support
 - Hard coded BUS type for better decison making at bmi_probe
 - Driver moved to acpi folder
 - Change to use acpi_spi_find_controller_by_adev
 - New shared function bmi_get_irq for I2C and SPI


Lucas Tanure (3):
  platform/x86: i2c-multi-instantiate: Move it to drivers/acpi folder
  ACPI: i2c-multi-instantiate: Rename it for a generic bus driver name
  ACPI: bus-multi-instantiate: Reorganize I2C functions

Stefan Binding (3):
  spi: Export acpi_spi_find_controller_by_adev to be used externally
  spi: Make spi_alloc_device and spi_add_device public again
  ACPI: bus-multi-instantiate: Add SPI support

 MAINTAINERS                                  |   4 +-
 drivers/acpi/Kconfig                         |  11 +
 drivers/acpi/Makefile                        |   1 +
 drivers/acpi/bus-multi-instantiate.c         | 500 +++++++++++++++++++
 drivers/acpi/scan.c                          |  19 +-
 drivers/platform/x86/Kconfig                 |  11 -
 drivers/platform/x86/Makefile                |   1 -
 drivers/platform/x86/i2c-multi-instantiate.c | 174 -------
 drivers/spi/spi.c                            |   9 +-
 include/linux/spi/spi.h                      |  22 +
 10 files changed, 552 insertions(+), 200 deletions(-)
 create mode 100644 drivers/acpi/bus-multi-instantiate.c
 delete mode 100644 drivers/platform/x86/i2c-multi-instantiate.c

Comments

Hans de Goede Dec. 10, 2021, 4:54 p.m. UTC | #1 
Hi Stefan,

On 12/10/21 16:40, Stefan Binding wrote:
> Add support for SPI bus in the ic2-multi-instantiate driver as
> upcoming laptops will need to multi instantiate SPI devices from
> a single device node, which has multiple SpiSerialBus entries at
> the ACPI table.
> 
> With the new SPI support, i2c-multi-instantiate becomes
> bus-multi-instantiate and is moved to the ACPI folder.
> 
> The intention is to support the SPI bus by re-using the current
> I2C multi instantiate, instead of creating a new SPI multi
> instantiate, to make it possible for peripherals that can be
> controlled by I2C or SPI to have the same HID at the ACPI table.
> 
> The new driver (Bus multi instantiate, bmi) checks for the
> hard-coded bus type and returns -ENODEV in case of zero devices
> found for that bus. In the case of automatic bus detection, 
> the driver will give preference to I2C.
> 
> The expectation is for a device node in the ACPI table to have
> multiple I2cSerialBus only or multiple SpiSerialBus only, not
> a mix of both; and for the case where there are both entries in
> one device node, only the I2C ones would be probed.
> 
> This new bus multi instantiate will be used in CS35L41 HDA new
> driver, being upstreamed:
> https://lkml.org/lkml/2021/11/23/723

Unfortunately you never really answered my questions about v1
of this series:

https://lore.kernel.org/platform-driver-x86/a1f546c2-5c63-573a-c032-603c792f3f7c@redhat.com/

So looking at the linked CS35L41 HDA series there is a single
ACPI device node with a HID of CLSA0100 which describes
two CS35L41 amplifiers connected over I2C ?

I assume you are doing this work because there are also designs
where there is a similar CLSA0100 ACPI device which also describes
two CS35L41 amplifiers but then connected over SPI ?

It would really help if you can:

1. Answer my questions from v1
2. Provide a concrete example of a device where these changes will
be necessary to make things work, preferably with a link to an
actual ACPI DSDT of that device.

Until you can better clarify why this is necessary, this series
gets a nack from me. The i2c-mult-instantiate code is a hack to
deal with some rather sub-optimal choices made in DSDTs used on
devices shipped with Windows and unless absolutely necessary
I would rather not see this get expanded to SPI.

Regards,

Hans
Mark Brown Dec. 10, 2021, 5:59 p.m. UTC | #2 
On Fri, Dec 10, 2021 at 03:40:47PM +0000, Stefan Binding wrote:
> From: Lucas Tanure <tanureal@opensource.cirrus.com>
> 
> Moving I2C multi instantiate driver to drivers/acpi folder for
> upcoming conversion into a generic bus multi instantiate
> driver for SPI and I2C
> 
> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
> ---

You've not provided a Signed-off-by for this so people can't do anything
with it, please see Documentation/process/submitting-patches.rst for
details on what this is and why it's important.
Stefan Binding Dec. 13, 2021, 11:40 a.m. UTC | #3 
Hi,

> -----Original Message-----
> From: Hans de Goede <hdegoede@redhat.com>
> Sent: 10 December 2021 16:55
> To: Stefan Binding <sbinding@opensource.cirrus.com>; Mark Brown
> <broonie@kernel.org>; Rafael J . Wysocki <rafael@kernel.org>; Len Brown
> <lenb@kernel.org>; Mark Gross <markgross@kernel.org>
> Cc: linux-kernel@vger.kernel.org; linux-spi@vger.kernel.org; linux-
> acpi@vger.kernel.org; platform-driver-x86@vger.kernel.org;
> patches@opensource.cirrus.com
> Subject: Re: [PATCH v2 0/6] Support Spi in i2c-multi-instantiate driver
> 
> Hi Stefan,
> 
> On 12/10/21 16:40, Stefan Binding wrote:
> > Add support for SPI bus in the ic2-multi-instantiate driver as
> > upcoming laptops will need to multi instantiate SPI devices from a
> > single device node, which has multiple SpiSerialBus entries at the
> > ACPI table.
> >
> > With the new SPI support, i2c-multi-instantiate becomes
> > bus-multi-instantiate and is moved to the ACPI folder.
> >
> > The intention is to support the SPI bus by re-using the current I2C
> > multi instantiate, instead of creating a new SPI multi instantiate, to
> > make it possible for peripherals that can be controlled by I2C or SPI
> > to have the same HID at the ACPI table.
> >
> > The new driver (Bus multi instantiate, bmi) checks for the hard-coded
> > bus type and returns -ENODEV in case of zero devices found for that
> > bus. In the case of automatic bus detection, the driver will give
> > preference to I2C.
> >
> > The expectation is for a device node in the ACPI table to have
> > multiple I2cSerialBus only or multiple SpiSerialBus only, not a mix of
> > both; and for the case where there are both entries in one device
> > node, only the I2C ones would be probed.
> >
> > This new bus multi instantiate will be used in CS35L41 HDA new driver,
> > being upstreamed:
> > https://lkml.org/lkml/2021/11/23/723
> 
> Unfortunately you never really answered my questions about v1 of this
> series:
> 
> https://lore.kernel.org/platform-driver-x86/a1f546c2-5c63-573a-c032-
> 603c792f3f7c@redhat.com/
> 
> So looking at the linked CS35L41 HDA series there is a single ACPI device node
> with a HID of CLSA0100 which describes two CS35L41 amplifiers connected
> over I2C ?

Yes, the related series uses HID CLSA0100, which contains 2 I2C devices inside a
single node. This ID was mistakenly used for this laptop, and instead CSC3551 
has been used for subsequent laptops.

> 
> I assume you are doing this work because there are also designs where there
> is a similar CLSA0100 ACPI device which also describes two CS35L41 amplifiers
> but then connected over SPI ?

Yes, there are several laptop designs which use an equivalent ACPI which describes
2 or 4 CS35L41 amplifiers which are connected either via I2C or via SPI.
Both designs use the same ACPI design and have 2-4 devices (either I2C or SPI)
defined inside a single ACPI node for HID CSC3551.
Note that the devices inside the node can be either SPI or I2C, but never SPI
and I2C.

> 
> It would really help if you can:
> 
> 1. Answer my questions from v1

I hope my colleague Lucas has answered these questions now.

> 2. Provide a concrete example of a device where these changes will be
> necessary to make things work, preferably with a link to an actual ACPI DSDT
> of that device.

This is the CSC3551 node for a laptop with 4 SPI nodes, with a shared IRQ:

 Scope (_SB.PC00.SPI0)
    {
        Device (GSPK)
        {
            Name (_HID, "CSC3551")  // _HID: Hardware ID
            Method (AUID, 0, NotSerialized)
            {
                Return ("103C89C3")
            }

            Method (_SUB, 0, NotSerialized)  // _SUB: Subsystem ID
            {
                Return (AUID ())
            }

            Name (_UID, One)  // _UID: Unique ID
            Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
            {
                Name (RBUF, ResourceTemplate ()
                {
                    SpiSerialBusV2 (0x0000, PolarityLow, FourWireMode, 0x08,
                        ControllerInitiated, 0x003D0900, ClockPolarityLow,
                        ClockPhaseFirst, "\\_SB.PC00.SPI0",
                        0x00, ResourceConsumer, , Exclusive,
                        )
                    SpiSerialBusV2 (0x0001, PolarityLow, FourWireMode, 0x08,
                        ControllerInitiated, 0x003D0900, ClockPolarityLow,
                        ClockPhaseFirst, "\\_SB.PC00.SPI0",
                        0x00, ResourceConsumer, , Exclusive,
                        )
                    SpiSerialBusV2 (0x0002, PolarityLow, FourWireMode, 0x08,
                        ControllerInitiated, 0x003D0900, ClockPolarityLow,
                        ClockPhaseFirst, "\\_SB.PC00.SPI0",
                        0x00, ResourceConsumer, , Exclusive,
                        )
                    SpiSerialBusV2 (0x0003, PolarityLow, FourWireMode, 0x08,
                        ControllerInitiated, 0x003D0900, ClockPolarityLow,
                        ClockPhaseFirst, "\\_SB.PC00.SPI0",
                        0x00, ResourceConsumer, , Exclusive,
                        )
                    GpioIo (Exclusive, PullUp, 0x0000, 0x0000, IoRestrictionOutputOnly,
                        "\\_SB.GPI0", 0x00, ResourceConsumer, ,
                        )
                        {   // Pin list
                            0x0000
                        }
                    GpioIo (Exclusive, PullUp, 0x0000, 0x0000, IoRestrictionOutputOnly,
                        "\\_SB.GPI0", 0x00, ResourceConsumer, ,
                        )
                        {   // Pin list
                            0x0000
                        }
                    GpioIo (Exclusive, PullUp, 0x0000, 0x0000, IoRestrictionOutputOnly,
                        "\\_SB.GPI0", 0x00, ResourceConsumer, ,
                        )
                        {   // Pin list
                            0x0000
                        }
                    GpioIo (Exclusive, PullDown, 0x0000, 0x0000, IoRestrictionOutputOnly,
                        "\\_SB.GPI0", 0x00, ResourceConsumer, ,
                        )
                        {   // Pin list
                            0x0000
                        }
                    GpioIo (Shared, PullUp, 0x0064, 0x0000, IoRestrictionInputOnly,
                        "\\_SB.GPI0", 0x00, ResourceConsumer, ,
                        )
                        {   // Pin list
                            0x0000
                        }
                    GpioInt (Edge, ActiveBoth, Shared, PullUp, 0x0064,
                        "\\_SB.GPI0", 0x00, ResourceConsumer, ,
                        )
                        {   // Pin list
                            0x0000
                        }
                })
                CreateWordField (RBUF, 0xCA, ACS1)
                CreateWordField (RBUF, 0xA7, ACS2)
                CreateWordField (RBUF, 0xED, ACS3)
                CreateWordField (RBUF, 0x0110, ARST)
                CreateWordField (RBUF, 0x0133, AINT)
                CreateWordField (RBUF, 0x0156, AIN2)
                ACS1 = GNUM (0x090E0016)
                ACS2 = GNUM (0x090E0017)
                ACS3 = GNUM (0x090C0006)
                ARST = GNUM (0x09070017)
                AINT = GNUM (0x09070013)
                AIN2 = GNUM (0x09070013)
                Return (RBUF) /* \_SB_.PC00.SPI0.GSPK._CRS.RBUF */
            }

            Method (_STA, 0, NotSerialized)  // _STA: Status
            {
                Return (0x0F)
            }

            Method (_DIS, 0, NotSerialized)  // _DIS: Disable Device
            {
            }
            Name (_DSD, Package ()  // _DSD: Device-Specific Data
            {
                ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */,
                Package ()
                {
                    Package () { "cirrus,dev-index", Package () { 0, 1, 2, 3 } },
                    Package ()
                    {
                        "reset-gpios", Package ()
                        {
                            ^GSPK, 3, 0, 0,
                            ^GSPK, 3, 0, 0,
                            ^GSPK, 3, 0, 0,
                            ^GSPK, 3, 0, 0,
                        },
                    },
                    Package () { "cirrus,speaker-position",     Package () { 1, 0, 1, 0 } },
                    Package () { "cirrus,gpio1-func",           Package () { 3, 3, 3, 3 } },
                    Package () { "cirrus,gpio2-func",           Package () { 2, 2, 2, 2 } },
                    Package () { "cirrus,boost-ind-nanohenry",  Package () { 1000, 1000, 1000, 1000 } },
                    Package () { "cirrus,boost-peak-milliamp",  Package () { 4500, 4500, 4500, 4500 } },
                    Package () { "cirrus,boost-cap-microfarad", Package () { 24, 24, 24, 24 } },
                }
            })
        }
    }

This is just our node from the DSDT, we are working to obtain and share the full DSDT, if still required.

> 
> Until you can better clarify why this is necessary, this series gets a nack from
> me. The i2c-mult-instantiate code is a hack to deal with some rather sub-
> optimal choices made in DSDTs used on devices shipped with Windows and
> unless absolutely necessary I would rather not see this get expanded to SPI.
> 
> Regards,
> 
> Hans

Thanks,

Stefan
Hans de Goede Dec. 15, 2021, 10:22 a.m. UTC | #4 
Hi,

On 12/13/21 12:40, Stefan Binding wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Hans de Goede <hdegoede@redhat.com>
>> Sent: 10 December 2021 16:55
>> To: Stefan Binding <sbinding@opensource.cirrus.com>; Mark Brown
>> <broonie@kernel.org>; Rafael J . Wysocki <rafael@kernel.org>; Len Brown
>> <lenb@kernel.org>; Mark Gross <markgross@kernel.org>
>> Cc: linux-kernel@vger.kernel.org; linux-spi@vger.kernel.org; linux-
>> acpi@vger.kernel.org; platform-driver-x86@vger.kernel.org;
>> patches@opensource.cirrus.com
>> Subject: Re: [PATCH v2 0/6] Support Spi in i2c-multi-instantiate driver
>>
>> Hi Stefan,
>>
>> On 12/10/21 16:40, Stefan Binding wrote:
>>> Add support for SPI bus in the ic2-multi-instantiate driver as
>>> upcoming laptops will need to multi instantiate SPI devices from a
>>> single device node, which has multiple SpiSerialBus entries at the
>>> ACPI table.
>>>
>>> With the new SPI support, i2c-multi-instantiate becomes
>>> bus-multi-instantiate and is moved to the ACPI folder.
>>>
>>> The intention is to support the SPI bus by re-using the current I2C
>>> multi instantiate, instead of creating a new SPI multi instantiate, to
>>> make it possible for peripherals that can be controlled by I2C or SPI
>>> to have the same HID at the ACPI table.
>>>
>>> The new driver (Bus multi instantiate, bmi) checks for the hard-coded
>>> bus type and returns -ENODEV in case of zero devices found for that
>>> bus. In the case of automatic bus detection, the driver will give
>>> preference to I2C.
>>>
>>> The expectation is for a device node in the ACPI table to have
>>> multiple I2cSerialBus only or multiple SpiSerialBus only, not a mix of
>>> both; and for the case where there are both entries in one device
>>> node, only the I2C ones would be probed.
>>>
>>> This new bus multi instantiate will be used in CS35L41 HDA new driver,
>>> being upstreamed:
>>> https://lkml.org/lkml/2021/11/23/723
>>
>> Unfortunately you never really answered my questions about v1 of this
>> series:
>>
>> https://lore.kernel.org/platform-driver-x86/a1f546c2-5c63-573a-c032-
>> 603c792f3f7c@redhat.com/
>>
>> So looking at the linked CS35L41 HDA series there is a single ACPI device node
>> with a HID of CLSA0100 which describes two CS35L41 amplifiers connected
>> over I2C ?
> 
> Yes, the related series uses HID CLSA0100, which contains 2 I2C devices inside a
> single node. This ID was mistakenly used for this laptop, and instead CSC3551 
> has been used for subsequent laptops.
> 
>>
>> I assume you are doing this work because there are also designs where there
>> is a similar CLSA0100 ACPI device which also describes two CS35L41 amplifiers
>> but then connected over SPI ?
> 
> Yes, there are several laptop designs which use an equivalent ACPI which describes
> 2 or 4 CS35L41 amplifiers which are connected either via I2C or via SPI.
> Both designs use the same ACPI design and have 2-4 devices (either I2C or SPI)
> defined inside a single ACPI node for HID CSC3551.
> Note that the devices inside the node can be either SPI or I2C, but never SPI
> and I2C.
> 
>>
>> It would really help if you can:
>>
>> 1. Answer my questions from v1
> 
> I hope my colleague Lucas has answered these questions now.

Yes, thank you .

>> 2. Provide a concrete example of a device where these changes will be
>> necessary to make things work, preferably with a link to an actual ACPI DSDT
>> of that device.
> 
> This is the CSC3551 node for a laptop with 4 SPI nodes, with a shared IRQ:
> 
>  Scope (_SB.PC00.SPI0)
>     {
>         Device (GSPK)
>         {
>             Name (_HID, "CSC3551")  // _HID: Hardware ID
>             Method (AUID, 0, NotSerialized)
>             {
>                 Return ("103C89C3")
>             }
> 
>             Method (_SUB, 0, NotSerialized)  // _SUB: Subsystem ID
>             {
>                 Return (AUID ())
>             }
> 
>             Name (_UID, One)  // _UID: Unique ID
>             Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
>             {
>                 Name (RBUF, ResourceTemplate ()
>                 {
>                     SpiSerialBusV2 (0x0000, PolarityLow, FourWireMode, 0x08,
>                         ControllerInitiated, 0x003D0900, ClockPolarityLow,
>                         ClockPhaseFirst, "\\_SB.PC00.SPI0",
>                         0x00, ResourceConsumer, , Exclusive,
>                         )
>                     SpiSerialBusV2 (0x0001, PolarityLow, FourWireMode, 0x08,
>                         ControllerInitiated, 0x003D0900, ClockPolarityLow,
>                         ClockPhaseFirst, "\\_SB.PC00.SPI0",
>                         0x00, ResourceConsumer, , Exclusive,
>                         )
>                     SpiSerialBusV2 (0x0002, PolarityLow, FourWireMode, 0x08,
>                         ControllerInitiated, 0x003D0900, ClockPolarityLow,
>                         ClockPhaseFirst, "\\_SB.PC00.SPI0",
>                         0x00, ResourceConsumer, , Exclusive,
>                         )
>                     SpiSerialBusV2 (0x0003, PolarityLow, FourWireMode, 0x08,
>                         ControllerInitiated, 0x003D0900, ClockPolarityLow,
>                         ClockPhaseFirst, "\\_SB.PC00.SPI0",
>                         0x00, ResourceConsumer, , Exclusive,
>                         )
>                     GpioIo (Exclusive, PullUp, 0x0000, 0x0000, IoRestrictionOutputOnly,
>                         "\\_SB.GPI0", 0x00, ResourceConsumer, ,
>                         )
>                         {   // Pin list
>                             0x0000
>                         }
>                     GpioIo (Exclusive, PullUp, 0x0000, 0x0000, IoRestrictionOutputOnly,
>                         "\\_SB.GPI0", 0x00, ResourceConsumer, ,
>                         )
>                         {   // Pin list
>                             0x0000
>                         }
>                     GpioIo (Exclusive, PullUp, 0x0000, 0x0000, IoRestrictionOutputOnly,
>                         "\\_SB.GPI0", 0x00, ResourceConsumer, ,
>                         )
>                         {   // Pin list
>                             0x0000
>                         }
>                     GpioIo (Exclusive, PullDown, 0x0000, 0x0000, IoRestrictionOutputOnly,
>                         "\\_SB.GPI0", 0x00, ResourceConsumer, ,
>                         )
>                         {   // Pin list
>                             0x0000
>                         }
>                     GpioIo (Shared, PullUp, 0x0064, 0x0000, IoRestrictionInputOnly,
>                         "\\_SB.GPI0", 0x00, ResourceConsumer, ,
>                         )
>                         {   // Pin list
>                             0x0000
>                         }
>                     GpioInt (Edge, ActiveBoth, Shared, PullUp, 0x0064,
>                         "\\_SB.GPI0", 0x00, ResourceConsumer, ,
>                         )
>                         {   // Pin list
>                             0x0000
>                         }
>                 })
>                 CreateWordField (RBUF, 0xCA, ACS1)
>                 CreateWordField (RBUF, 0xA7, ACS2)
>                 CreateWordField (RBUF, 0xED, ACS3)
>                 CreateWordField (RBUF, 0x0110, ARST)
>                 CreateWordField (RBUF, 0x0133, AINT)
>                 CreateWordField (RBUF, 0x0156, AIN2)
>                 ACS1 = GNUM (0x090E0016)
>                 ACS2 = GNUM (0x090E0017)
>                 ACS3 = GNUM (0x090C0006)
>                 ARST = GNUM (0x09070017)
>                 AINT = GNUM (0x09070013)
>                 AIN2 = GNUM (0x09070013)
>                 Return (RBUF) /* \_SB_.PC00.SPI0.GSPK._CRS.RBUF */
>             }
> 
>             Method (_STA, 0, NotSerialized)  // _STA: Status
>             {
>                 Return (0x0F)
>             }
> 
>             Method (_DIS, 0, NotSerialized)  // _DIS: Disable Device
>             {
>             }
>             Name (_DSD, Package ()  // _DSD: Device-Specific Data
>             {
>                 ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */,
>                 Package ()
>                 {
>                     Package () { "cirrus,dev-index", Package () { 0, 1, 2, 3 } },
>                     Package ()
>                     {
>                         "reset-gpios", Package ()
>                         {
>                             ^GSPK, 3, 0, 0,
>                             ^GSPK, 3, 0, 0,
>                             ^GSPK, 3, 0, 0,
>                             ^GSPK, 3, 0, 0,
>                         },
>                     },
>                     Package () { "cirrus,speaker-position",     Package () { 1, 0, 1, 0 } },
>                     Package () { "cirrus,gpio1-func",           Package () { 3, 3, 3, 3 } },
>                     Package () { "cirrus,gpio2-func",           Package () { 2, 2, 2, 2 } },
>                     Package () { "cirrus,boost-ind-nanohenry",  Package () { 1000, 1000, 1000, 1000 } },
>                     Package () { "cirrus,boost-peak-milliamp",  Package () { 4500, 4500, 4500, 4500 } },
>                     Package () { "cirrus,boost-cap-microfarad", Package () { 24, 24, 24, 24 } },
>                 }
>             })
>         }
>     }
> 
> This is just our node from the DSDT, we are working to obtain and share the full DSDT, if still required.

Thanks, there is no need to share the full DSDT, this is exactly what I was looking for.

With the provided info I believe that this indeed is the right approach
and I no longer have any objections against this approach.

I will try to make some time to review this series soon-ish.

Regards,

Hans
Hans de Goede Dec. 15, 2021, 10:26 a.m. UTC | #5 
Hi,

On 12/10/21 18:59, Mark Brown wrote:
> On Fri, Dec 10, 2021 at 03:40:47PM +0000, Stefan Binding wrote:
>> From: Lucas Tanure <tanureal@opensource.cirrus.com>
>>
>> Moving I2C multi instantiate driver to drivers/acpi folder for
>> upcoming conversion into a generic bus multi instantiate
>> driver for SPI and I2C
>>
>> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
>> ---
> 
> You've not provided a Signed-off-by for this so people can't do anything
> with it, please see Documentation/process/submitting-patches.rst for
> details on what this is and why it's important.

To clarify this, what I believe that Mark means here is that if you
submit a patch which was originally authored by someone else, then
you should add your own (the submitters) Signed-off-by after the
author's Signed-off-by.

The idea is that each person directly touching the patch (rather
then merging a branch which has the patch) adds its S-o-b to the
patch. You will also always see sub-system maintainers add their
own S-o-b at the end when they pick up the patch from the list
and then apply it to their own tree (and possibly resolve
conflicts or touch up things a bit).

Regards,

Hans