| Message ID | 20241018015826.2925075-1-fj5100bi@fujitsu.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | i2c: designware: Add ACPI HID for DWAPB I2C controller on Fujitsu MONAKA | expand |
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
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?
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
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?
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
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.
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 },
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(+)