[PATCH] i2c: designware: Add ACPI HID for DWAPB I2C controller on Fujitsu MONAKA

[PATCH] i2c: designware: Add ACPI HID for DWAPB I2C controller on Fujitsu MONAKA

[Yoshihiro Furudera]

This patch enables DWAPB I2C controller support on Fujitsu MONAKA.

Signed-off-by: Yoshihiro Furudera <fj5100bi@fujitsu.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 2d0c7348e491..c04af315a4f9 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -351,6 +351,7 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = {
 	{ "AMDI0019", ACCESS_INTR_MASK | ARBITRATION_SEMAPHORE },
 	{ "AMDI0510", 0 },
 	{ "APMC0D0F", 0 },
+	{ "FUJI200B", 0 },
 	{ "HISI02A1", 0 },
 	{ "HISI02A2", 0 },
 	{ "HISI02A3", 0 },
-- 
2.34.1

Re: [PATCH] i2c: designware: Add ACPI HID for DWAPB I2C controller on Fujitsu MONAKA

[Andy Shevchenko]

On Fri, Oct 18, 2024 at 01:58:26AM +0000, Yoshihiro Furudera wrote:
> This patch enables DWAPB I2C controller support on Fujitsu MONAKA.

s/This patch enables/Enable/

Also please give more details:
1) is this ID already present in the wild
   (in the products that one may just go and buy)?
if so, mention the example of the product.

2) provide an excerpt from DSDT for the Device object that uses this _HID.

Otherwise the vendor ID is legit [1] and code wise patch is okay.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

[1]: https://uefi.org/ACPI_ID_List?acpi_search=fuji

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH] i2c: designware: Add ACPI HID for DWAPB I2C controller on Fujitsu MONAKA

[Yoshihiro Furudera (Fujitsu)]

Hi, Andy Shevchenko
Thanks for you review/comments.

> On Fri, Oct 18, 2024 at 01:58:26AM +0000, Yoshihiro Furudera wrote:
> > This patch enables DWAPB I2C controller support on Fujitsu MONAKA.
> 
> s/This patch enables/Enable/

Understood.

> 
> Also please give more details:
> 1) is this ID already present in the wild
>    (in the products that one may just go and buy)?
> if so, mention the example of the product.

Not available at this time.
It is planned to be implemented in the MONAKA server
scheduled for shipment in 2027.

> 
> 2) provide an excerpt from DSDT for the Device object that uses this _HID.

The DSDT information obtained when verified using
an in-house simulator is presented below.

DefinitionBlock ("", "SSDT", 2, "FUJ   ", "NGDC    ", 0x00000001)
{
    Scope (\_SB)
    {
        Device (SMB0)
        {
            Name (_HID, "FUJI200B")  // _HID: Hardware ID
            Name (_UID, Zero)  // _UID: Unique ID
            Method (_STA, 0, NotSerialized)  // _STA: Status
            {
                Return (0x0F)
            }
 
            Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
            {
                Memory32Fixed (ReadWrite,
                    0x2A4B0000,         // Address Base
                    0x00010000,         // Address Length
                    )
                Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, )
                {
                    0x00000159,
                }
            })
            Device (SSIF)
            {
                Name (_HID, EisaId ("IPI0001"))  // _HID: Hardware ID
                Name (_UID, Zero)  // _UID: Unique ID
                Name (_STR, Unicode ("IPMI_SSIF"))  // _STR: Description String
                Method (_IFT, 0, NotSerialized)  // _IFT: IPMI Interface Type
                {
                    Return (0x04)
                }
 
                Method (_ADR, 0, NotSerialized)  // _ADR: Address
                {
                    Return (0x20)
                }
 
                Method (_SRV, 0, NotSerialized)  // _SRV: IPMI Spec Revision
                {
                    Return (0x0200)
                }
            }
        }
    }
}

> 
> Otherwise the vendor ID is legit [1] and code wise patch is okay.
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> [1]: https://uefi.org/ACPI_ID_List?acpi_search=fuji
> 
> --
> With Best Regards,
> Andy Shevchenko
>

Best Regards,
Yoshihiro Furudera

Re: [PATCH] i2c: designware: Add ACPI HID for DWAPB I2C controller on Fujitsu MONAKA

['Andy Shevchenko']

On Mon, Oct 21, 2024 at 07:22:55AM +0000, Yoshihiro Furudera (Fujitsu) wrote:
> > On Fri, Oct 18, 2024 at 01:58:26AM +0000, Yoshihiro Furudera wrote:

...

> > Also please give more details:
> > 1) is this ID already present in the wild
> >    (in the products that one may just go and buy)?
> > if so, mention the example of the product.
> 
> Not available at this time.
> It is planned to be implemented in the MONAKA server
> scheduled for shipment in 2027.

So, summarize that in a short sentence, like "This will be used in the MONAKA
server scheduled for shipment in 2027."

> > 2) provide an excerpt from DSDT for the Device object that uses this _HID.
> 
> The DSDT information obtained when verified using
> an in-house simulator is presented below.

Just strip it to the only significant parts, see below

     Device (SMB0)
     {
         Name (_HID, "FUJI200B")  // _HID: Hardware ID
         Name (_UID, Zero)  // _UID: Unique ID
         ...
         Name (_CRS, ResourceTemplate ()
         {
             Memory32Fixed (ReadWrite,
                 0x2A4B0000,         // Address Base
                 0x00010000,         // Address Length
                 )
             Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, )
             {
                 0x00000159,
             }
         })
         ...
     }

...

Hmm... Why Device object is called SMB0, are you sure it's the correct one?

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH] i2c: designware: Add ACPI HID for DWAPB I2C controller on Fujitsu MONAKA

[Yoshihiro Furudera (Fujitsu)]

Hi, Andy Shevchenko
Thanks for you review/comments.

> On Mon, Oct 21, 2024 at 07:22:55AM +0000, Yoshihiro Furudera (Fujitsu) wrote:
> > > On Fri, Oct 18, 2024 at 01:58:26AM +0000, Yoshihiro Furudera wrote:
> 
> ...
> 
> > > Also please give more details:
> > > 1) is this ID already present in the wild
> > >    (in the products that one may just go and buy)?
> > > if so, mention the example of the product.
> >
> > Not available at this time.
> > It is planned to be implemented in the MONAKA server scheduled for
> > shipment in 2027.
> 
> So, summarize that in a short sentence, like "This will be used in the MONAKA
> server scheduled for shipment in 2027."

Understood.
I will be more careful in the future.

> 
> > > 2) provide an excerpt from DSDT for the Device object that uses this _HID.
> >
> > The DSDT information obtained when verified using an in-house
> > simulator is presented below.
> 
> Just strip it to the only significant parts, see below
> 
>      Device (SMB0)
>      {
>          Name (_HID, "FUJI200B")  // _HID: Hardware ID
>          Name (_UID, Zero)  // _UID: Unique ID
>          ...
>          Name (_CRS, ResourceTemplate ()
>          {
>              Memory32Fixed (ReadWrite,
>                  0x2A4B0000,         // Address Base
>                  0x00010000,         // Address Length
>                  )
>              Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, )
>              {
>                  0x00000159,
>              }
>          })
>          ...
>      }

Understood.

> 
> ...
> 
> Hmm... Why Device object is called SMB0, are you sure it's the correct one?

We considered the string to be the most concise
representation of SMBus HC#0, given the general
constraint that object names should ideally be
four characters or less. We understood that,
unlike HID, SMBus object names are vendor-specific.

> 
> --
> With Best Regards,
> Andy Shevchenko
> 

Best Regards,
Yoshihiro Furudera

Re: [PATCH] i2c: designware: Add ACPI HID for DWAPB I2C controller on Fujitsu MONAKA

['Andy Shevchenko']

On Tue, Oct 22, 2024 at 01:14:18AM +0000, Yoshihiro Furudera (Fujitsu) wrote:
> > On Mon, Oct 21, 2024 at 07:22:55AM +0000, Yoshihiro Furudera (Fujitsu) wrote:
> > > > On Fri, Oct 18, 2024 at 01:58:26AM +0000, Yoshihiro Furudera wrote:

...

> >      Device (SMB0)
> >      {
> >          ...
> >      }

> > Hmm... Why Device object is called SMB0, are you sure it's the correct one?
> 
> We considered the string to be the most concise
> representation of SMBus HC#0, given the general
> constraint that object names should ideally be
> four characters or less. We understood that,
> unlike HID, SMBus object names are vendor-specific.

But this all about UART! How is it related to SMBus?

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH] i2c: designware: Add ACPI HID for DWAPB I2C controller on Fujitsu MONAKA

[Yoshihiro Furudera (Fujitsu)]

Thanks for the comment.

> On Tue, Oct 22, 2024 at 01:14:18AM +0000, Yoshihiro Furudera (Fujitsu) wrote:
> > > On Mon, Oct 21, 2024 at 07:22:55AM +0000, Yoshihiro Furudera (Fujitsu)
> wrote:
> > > > > On Fri, Oct 18, 2024 at 01:58:26AM +0000, Yoshihiro Furudera wrote:
> 
> ...
> 
> > >      Device (SMB0)
> > >      {
> > >          ...
> > >      }
> 
> > > Hmm... Why Device object is called SMB0, are you sure it's the correct one?
> >
> > We considered the string to be the most concise representation of
> > SMBus HC#0, given the general constraint that object names should
> > ideally be four characters or less. We understood that, unlike HID,
> > SMBus object names are vendor-specific.
> 
> But this all about UART! How is it related to SMBus?

We created the SMB0 object according to the following specifications:
 
ACPI Specification
13.2. Accessing the SMBus from ASL Code
https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/13_ACPI_System_Mgmt_Bus_Interface_Spec/accessing-the-smbus-from-asl-code.html
 
IPMI Specification
Example 4: SSIF Interface(P574)
https://www.intel.co.jp/content/www/jp/ja/products/docs/servers/ipmi/ipmi-second-gen-interface-spec-v2-rev1-1.html
 
Therefore, SMB0 does not deviate from the SMBus related specifications.

> --
> With Best Regards,
> Andy Shevchenko
> 

Best Regards,
Yoshihiro Furudera

Re: [PATCH] i2c: designware: Add ACPI HID for DWAPB I2C controller on Fujitsu MONAKA

['Andy Shevchenko']

On Wed, Oct 23, 2024 at 04:40:06AM +0000, Yoshihiro Furudera (Fujitsu) wrote:
> > On Tue, Oct 22, 2024 at 01:14:18AM +0000, Yoshihiro Furudera (Fujitsu) wrote:
> > > > On Mon, Oct 21, 2024 at 07:22:55AM +0000, Yoshihiro Furudera (Fujitsu)
> > wrote:
> > > > > > On Fri, Oct 18, 2024 at 01:58:26AM +0000, Yoshihiro Furudera wrote:

...

> > > >      Device (SMB0)
> > > >      {
> > > >          ...
> > > >      }
> > 
> > > > Hmm... Why Device object is called SMB0, are you sure it's the correct one?
> > >
> > > We considered the string to be the most concise representation of
> > > SMBus HC#0, given the general constraint that object names should
> > > ideally be four characters or less. We understood that, unlike HID,
> > > SMBus object names are vendor-specific.
> > 
> > But this all about UART! How is it related to SMBus?
> 
> We created the SMB0 object according to the following specifications:
>  
> ACPI Specification
> 13.2. Accessing the SMBus from ASL Code
> https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/13_ACPI_System_Mgmt_Bus_Interface_Spec/accessing-the-smbus-from-asl-code.html
>  
> IPMI Specification
> Example 4: SSIF Interface(P574)
> https://www.intel.co.jp/content/www/jp/ja/products/docs/servers/ipmi/ipmi-second-gen-interface-spec-v2-rev1-1.html
>  
> Therefore, SMB0 does not deviate from the SMBus related specifications.

Ah, I see now, sorry I missed that. Thank you for your patience and
elaboration. Please, update the commit message as discussed, send a new version
and I review it.


-- 
With Best Regards,
Andy Shevchenko