[PATCH] docs: i2c: piix4: Add ACPI section

[PATCH] docs: i2c: piix4: Add ACPI section

[Konstantin Aladyshev]

Provide information how to reference I2C busses created by the PIIX4
chip driver from the ACPI code.

Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
---
 Documentation/i2c/busses/i2c-piix4.rst | 56 ++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/Documentation/i2c/busses/i2c-piix4.rst b/Documentation/i2c/busses/i2c-piix4.rst
index 07fe6f6f4b18..2a00158b508a 100644
--- a/Documentation/i2c/busses/i2c-piix4.rst
+++ b/Documentation/i2c/busses/i2c-piix4.rst
@@ -109,3 +109,59 @@ which can easily get corrupted due to a state machine bug. These are mostly
 Thinkpad laptops, but desktop systems may also be affected. We have no list
 of all affected systems, so the only safe solution was to prevent access to
 the SMBus on all IBM systems (detected using DMI data.)
+
+
+Description in the ACPI code
+----------------------------
+
+Device driver for the PIIX4 chip creates a separate I2C bus for each of its ports::
+
+    $ i2cdetect -l
+    ...
+    i2c-7   unknown         SMBus PIIX4 adapter port 0 at 0b00      N/A
+    i2c-8   unknown         SMBus PIIX4 adapter port 2 at 0b00      N/A
+    i2c-9   unknown         SMBus PIIX4 adapter port 1 at 0b20      N/A
+    ...
+
+Therefore if you want to access one of these busses in the ACPI code, you need to
+declare port subdevices inside the PIIX device::
+
+    Scope (\_SB_.PCI0.SMBS)
+    {
+        Name (_ADR, 0x00140000)
+
+        Device (SMB0) {
+            Name (_ADR, 0)
+        }
+        Device (SMB1) {
+            Name (_ADR, 1)
+        }
+        Device (SMB2) {
+            Name (_ADR, 2)
+        }
+    }
+
+As an example of usage here is the ACPI snippet code that would assign jc42 driver
+to the 0x1C device on the I2C bus created by the PIIX port 0::
+
+    Device (JC42) {
+        Name (_HID, "PRP0001")
+        Name (_DDN, "JC42 Temperature sensor")
+        Name (_CRS, ResourceTemplate () {
+            I2cSerialBusV2 (
+                0x001c,
+                ControllerInitiated,
+                100000,
+                AddressingMode7Bit,
+                "\\_SB.PCI0.SMBS.SMB0",
+                0
+            )
+        })
+
+        Name (_DSD, Package () {
+            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+            Package () {
+                Package () { "compatible", Package() { "jedec,jc-42.4-temp" } },
+            }
+        })
+    }
-- 
2.43.0

Re: [PATCH] docs: i2c: piix4: Add ACPI section

[Andy Shevchenko]

On Mon, Nov 11, 2024 at 02:56:52PM +0300, Konstantin Aladyshev wrote:
> Provide information how to reference I2C busses created by the PIIX4
> chip driver from the ACPI code.

...

> +Therefore if you want to access one of these busses in the ACPI code, you need to
> +declare port subdevices inside the PIIX device::
> +
> +    Scope (\_SB_.PCI0.SMBS)
> +    {
> +        Name (_ADR, 0x00140000)
> +
> +        Device (SMB0) {
> +            Name (_ADR, 0)
> +        }
> +        Device (SMB1) {
> +            Name (_ADR, 1)
> +        }
> +        Device (SMB2) {
> +            Name (_ADR, 2)
> +        }
> +    }

You need to elaborate that some of this data may be already present in the BIOS
DSDT (you give your example as it seems most common so far) and hence requires
an additional per-port addresses. With that you should add a note that this
will require to load SSDT quite in advance to make sure that the driver will
see these changes before its ->probe().

...

The rest is LGTM.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] docs: i2c: piix4: Add ACPI section

[Andy Shevchenko]

On Mon, Nov 11, 2024 at 02:52:57PM +0200, Andy Shevchenko wrote:
> On Mon, Nov 11, 2024 at 02:56:52PM +0300, Konstantin Aladyshev wrote:
> > Provide information how to reference I2C busses created by the PIIX4
> > chip driver from the ACPI code.

...

> > +Therefore if you want to access one of these busses in the ACPI code, you need to
> > +declare port subdevices inside the PIIX device::
> > +
> > +    Scope (\_SB_.PCI0.SMBS)
> > +    {
> > +        Name (_ADR, 0x00140000)
> > +
> > +        Device (SMB0) {
> > +            Name (_ADR, 0)
> > +        }
> > +        Device (SMB1) {
> > +            Name (_ADR, 1)
> > +        }
> > +        Device (SMB2) {
> > +            Name (_ADR, 2)
> > +        }
> > +    }
> 
> You need to elaborate that some of this data may be already present in the BIOS
> DSDT (you give your example as it seems most common so far) and hence requires
> an additional per-port addresses. With that you should add a note that this
> will require to load SSDT quite in advance to make sure that the driver will
> see these changes before its ->probe().
> 
> ...
> 
> The rest is LGTM.

Also Cc new version to Andi Shyti who is I2C host driver maintainer.
It's probably he who is going to apply the change.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2] docs: i2c: piix4: Add ACPI section

[Konstantin Aladyshev]

Provide information how to reference I2C busses created by the PIIX4
chip driver from the ACPI code.

Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
---
 Documentation/i2c/busses/i2c-piix4.rst | 62 ++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/Documentation/i2c/busses/i2c-piix4.rst b/Documentation/i2c/busses/i2c-piix4.rst
index 07fe6f6f4b18..90447dff7419 100644
--- a/Documentation/i2c/busses/i2c-piix4.rst
+++ b/Documentation/i2c/busses/i2c-piix4.rst
@@ -109,3 +109,65 @@ which can easily get corrupted due to a state machine bug. These are mostly
 Thinkpad laptops, but desktop systems may also be affected. We have no list
 of all affected systems, so the only safe solution was to prevent access to
 the SMBus on all IBM systems (detected using DMI data.)
+
+
+Description in the ACPI code
+----------------------------
+
+Device driver for the PIIX4 chip creates a separate I2C bus for each of its ports::
+
+    $ i2cdetect -l
+    ...
+    i2c-7   unknown         SMBus PIIX4 adapter port 0 at 0b00      N/A
+    i2c-8   unknown         SMBus PIIX4 adapter port 2 at 0b00      N/A
+    i2c-9   unknown         SMBus PIIX4 adapter port 1 at 0b20      N/A
+    ...
+
+Therefore if you want to access one of these busses in the ACPI code, port subdevices
+are needed to be declared inside the PIIX device::
+
+    Scope (\_SB_.PCI0.SMBS)
+    {
+        Name (_ADR, 0x00140000)
+
+        Device (SMB0) {
+            Name (_ADR, 0)
+        }
+        Device (SMB1) {
+            Name (_ADR, 1)
+        }
+        Device (SMB2) {
+            Name (_ADR, 2)
+        }
+    }
+
+If it is not the case for your UEFI firmware and you don't have access to the source
+code, you can use ACPI SSDT Overlays to provide the missing parts. Just keep in mind
+that in this case you would need to load your extra SSDT table before the piix4 driver
+start, i.e. you should provide SSDT via initrd or EFI variable methods and not via
+configfs.
+
+As an example of usage here is the ACPI snippet code that would assign jc42 driver
+to the 0x1C device on the I2C bus created by the PIIX port 0::
+
+    Device (JC42) {
+        Name (_HID, "PRP0001")
+        Name (_DDN, "JC42 Temperature sensor")
+        Name (_CRS, ResourceTemplate () {
+            I2cSerialBusV2 (
+                0x001c,
+                ControllerInitiated,
+                100000,
+                AddressingMode7Bit,
+                "\\_SB.PCI0.SMBS.SMB0",
+                0
+            )
+        })
+
+        Name (_DSD, Package () {
+            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+            Package () {
+                Package () { "compatible", Package() { "jedec,jc-42.4-temp" } },
+            }
+        })
+    }
-- 
2.43.0

Re: [PATCH v2] docs: i2c: piix4: Add ACPI section

[Andy Shevchenko]

+Cc: Andi

On Mon, Nov 11, 2024 at 05:02:31PM +0300, Konstantin Aladyshev wrote:
> Provide information how to reference I2C busses created by the PIIX4
> chip driver from the ACPI code.

So, this is not ideal, but it's a good start so far.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
We may amend this later on.

Thanks!

> Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
> ---

This place is for changelog and comments, if any. Since this is a v2 of a
standalone patch, the changelog is good to have. No need to resend now,
you can just reply to your original v2 with the missing changelog.

>  Documentation/i2c/busses/i2c-piix4.rst | 62 ++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] docs: i2c: piix4: Add ACPI section

[Andi Shyti]

Hi Konstantin,

thanks for your driver, just few grammatical thoughts.

...

> +If it is not the case for your UEFI firmware and you don't have access to the source

Not a grammatical error, but it sounds a bit more linear:

/If it is not/If this is not/

> +code, you can use ACPI SSDT Overlays to provide the missing parts. Just keep in mind
> +that in this case you would need to load your extra SSDT table before the piix4 driver
> +start, i.e. you should provide SSDT via initrd or EFI variable methods and not via

/start/starts/

If you agree, I would go ahead and merge this.

Andi

> +configfs.

Re: [PATCH v2] docs: i2c: piix4: Add ACPI section

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 2907 bytes --]

On Mon, Nov 11, 2024 at 05:02:31PM +0300, Konstantin Aladyshev wrote:
> diff --git a/Documentation/i2c/busses/i2c-piix4.rst b/Documentation/i2c/busses/i2c-piix4.rst
> index 07fe6f6f4b18..90447dff7419 100644
> --- a/Documentation/i2c/busses/i2c-piix4.rst
> +++ b/Documentation/i2c/busses/i2c-piix4.rst
> @@ -109,3 +109,65 @@ which can easily get corrupted due to a state machine bug. These are mostly
>  Thinkpad laptops, but desktop systems may also be affected. We have no list
>  of all affected systems, so the only safe solution was to prevent access to
>  the SMBus on all IBM systems (detected using DMI data.)
> +
> +
> +Description in the ACPI code
> +----------------------------
> +
> +Device driver for the PIIX4 chip creates a separate I2C bus for each of its ports::
> +
> +    $ i2cdetect -l
> +    ...
> +    i2c-7   unknown         SMBus PIIX4 adapter port 0 at 0b00      N/A
> +    i2c-8   unknown         SMBus PIIX4 adapter port 2 at 0b00      N/A
> +    i2c-9   unknown         SMBus PIIX4 adapter port 1 at 0b20      N/A
> +    ...
> +
> +Therefore if you want to access one of these busses in the ACPI code, port subdevices
> +are needed to be declared inside the PIIX device::
> +
> +    Scope (\_SB_.PCI0.SMBS)
> +    {
> +        Name (_ADR, 0x00140000)
> +
> +        Device (SMB0) {
> +            Name (_ADR, 0)
> +        }
> +        Device (SMB1) {
> +            Name (_ADR, 1)
> +        }
> +        Device (SMB2) {
> +            Name (_ADR, 2)
> +        }
> +    }
> +
> +If it is not the case for your UEFI firmware and you don't have access to the source
> +code, you can use ACPI SSDT Overlays to provide the missing parts. Just keep in mind
> +that in this case you would need to load your extra SSDT table before the piix4 driver
> +start, i.e. you should provide SSDT via initrd or EFI variable methods and not via
> +configfs.
> +
> +As an example of usage here is the ACPI snippet code that would assign jc42 driver
> +to the 0x1C device on the I2C bus created by the PIIX port 0::
> +
> +    Device (JC42) {
> +        Name (_HID, "PRP0001")
> +        Name (_DDN, "JC42 Temperature sensor")
> +        Name (_CRS, ResourceTemplate () {
> +            I2cSerialBusV2 (
> +                0x001c,
> +                ControllerInitiated,
> +                100000,
> +                AddressingMode7Bit,
> +                "\\_SB.PCI0.SMBS.SMB0",
> +                0
> +            )
> +        })
> +
> +        Name (_DSD, Package () {
> +            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> +            Package () {
> +                Package () { "compatible", Package() { "jedec,jc-42.4-temp" } },
> +            }
> +        })
> +    }

Looks good, thanks!

Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2] docs: i2c: piix4: Add ACPI section

[Andi Shyti]

Hi Konstantin,

On Mon, Nov 11, 2024 at 05:02:31PM +0300, Konstantin Aladyshev wrote:
> Provide information how to reference I2C busses created by the PIIX4
> chip driver from the ACPI code.
> 
> Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>

I merged the patch into i2c/i2c-host with the changes I
suggested and I also wrapped the lines to 80 characters to keep a
uniform style throughout the doc file.

Andi

Re: [PATCH v2] docs: i2c: piix4: Add ACPI section

[Konstantin Aladyshev]

Hi Andi!

Sorry, I just didn't have time to fix it yesterday. Of course I don't
mind your changes. Thanks for the help!

Best regards,
Konstantin Aladyshev

On Thu, Nov 14, 2024 at 1:50 AM Andi Shyti <andi.shyti@kernel.org> wrote:
>
> Hi Konstantin,
>
> On Mon, Nov 11, 2024 at 05:02:31PM +0300, Konstantin Aladyshev wrote:
> > Provide information how to reference I2C busses created by the PIIX4
> > chip driver from the ACPI code.
> >
> > Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
>
> I merged the patch into i2c/i2c-host with the changes I
> suggested and I also wrapped the lines to 80 characters to keep a
> uniform style throughout the doc file.
>
> Andi