AW: [PATCH v2 2/3] rtc: rx6110: add ACPI bindings to I2C

[johannes-hahn@xxxxxxxxxxx]

Hello Andy,

when comparing the ACPI IDs used in rtc-ds1307.c, which is already on mainline 

https://elixir.bootlin.com/linux/latest/source/drivers/rtc/rtc-ds1307.c#L1141

for example. Every ID listed there is also not formatted the ACPI ID , PNP ID way defined in the ACPI spec.

How about that ?

Best Regards,
Johannes

-----Ursprüngliche Nachricht-----
Von: Henning Schild <henning.schild@xxxxxxxxxxx> 
Gesendet: Montag, 16. November 2020 16:30
An: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
Cc: Claudius Heine <ch@xxxxxxx>; Alessandro Zummo <a.zummo@xxxxxxxxxxxx>; Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx>; linux-rtc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Hahn, Johannes (DI FA CTR PLC PRC3) <johannes-hahn@xxxxxxxxxxx>; Zeh, Werner (DI MC MTS R&D HW 1) <werner.zeh@xxxxxxxxxxx>
Betreff: Re: [PATCH v2 2/3] rtc: rx6110: add ACPI bindings to I2C

Am Mon, 16 Nov 2020 16:46:31 +0200
schrieb Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>:

> On Thu, Nov 12, 2020 at 02:07:33PM +0100, Claudius Heine wrote:
> > From: Johannes Hahn <johannes-hahn@xxxxxxxxxxx>
> > 
> > This allows the RX6110 driver to be automatically assigned to the 
> > right device on the I2C bus.
> 
> Before adding new ACPI ID, can you provide an evidence (either from 
> vendor of the component, or a real snapshot of DSDT from device on
> market) that this is real ID?
> 
> Before that happens, NAK.
> 
> P.S. Seems to me that this is kinda cargo cult patch because proposed 
> ID is against ACPI and PNP registry and ACPI specification.

In fact we pushed it in coreboot and Linux at the same time.

https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Freview.coreboot.org%2Fc%2Fcoreboot%2F%2B%2F47235&amp;data=04%7C01%7Cjohannes-hahn%40siemens.com%7C21c9e1fe99274df7951a08d88a448af5%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C637411374276831534%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=7EVdO%2F77LNyvux0y3m9nEf2HZO%2BDm2WkWMfxzaJUoto%3D&amp;reserved=0

That is the evidence. But in case this is wrong we can probably still change coreboot, even though the patches have been merged there already.

Maybe you can go into detail where you see the violations and maybe even suggest fixes that come to mind.

Henning