diff mbox series

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

Message ID 20241018015826.2925075-1-fj5100bi@fujitsu.com
State New
Headers show
Series i2c: designware: Add ACPI HID for DWAPB I2C controller on Fujitsu MONAKA | expand

Commit Message

Yoshihiro Furudera (Fujitsu) Oct. 18, 2024, 1:58 a.m. UTC
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(+)

Comments

Yoshihiro Furudera (Fujitsu) Oct. 21, 2024, 7:22 a.m. UTC | #1
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
'Andy Shevchenko' Oct. 21, 2024, 8:25 a.m. UTC | #2
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?
Yoshihiro Furudera (Fujitsu) Oct. 22, 2024, 1:14 a.m. UTC | #3
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
'Andy Shevchenko' Oct. 22, 2024, 1:12 p.m. UTC | #4
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?
Yoshihiro Furudera (Fujitsu) Oct. 23, 2024, 4:40 a.m. UTC | #5
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
diff mbox series

Patch

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 },