Hi,
this patch introduces I2C support to the RX6110 RTC driver and also adds
an ACPI identifier to it.
Since we are also pushing the coreboot changes for the ACPI table
upstream in parallel, we are free to name this ACPI entry however we
like it seems. So any feedback on that would be welcome ;)
kind regards,
Claudius
Claudius Heine (1):
rtc: rx6110: add i2c support
Johannes Hahn (1):
rtc: rx6110: add ACPI bindings to I2C
drivers/rtc/Kconfig | 20 ++---
drivers/rtc/rtc-rx6110.c | 155 +++++++++++++++++++++++++++++++++++++--
2 files changed, 158 insertions(+), 17 deletions(-)
--
2.20.1
From: Johannes Hahn <[email protected]>
This allows the RX6110 driver to be automatically assigned to the right
device on the I2C bus.
Signed-off-by: Johannes Hahn <[email protected]>
Signed-off-by: Claudius Heine <[email protected]>
Signed-off-by: Henning Schild <[email protected]>
---
drivers/rtc/rtc-rx6110.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/rtc/rtc-rx6110.c b/drivers/rtc/rtc-rx6110.c
index ca9f486236d4..6c0c7e5a7065 100644
--- a/drivers/rtc/rtc-rx6110.c
+++ b/drivers/rtc/rtc-rx6110.c
@@ -13,6 +13,7 @@
#include <linux/of_gpio.h>
#include <linux/regmap.h>
#include <linux/rtc.h>
+#include <linux/acpi.h>
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/spi/spi.h>
@@ -455,6 +456,14 @@ static int rx6110_i2c_probe(struct i2c_client *client,
return 0;
}
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id rx6110_i2c_acpi_match[] = {
+ { "RX6110SA", },
+ { },
+};
+MODULE_DEVICE_TABLE(acpi, rx6110_i2c_acpi_match);
+#endif
+
static const struct i2c_device_id rx6110_i2c_id[] = {
{ "rx6110", 0 },
{ }
@@ -464,6 +473,9 @@ MODULE_DEVICE_TABLE(i2c, rx6110_i2c_id);
static struct i2c_driver rx6110_i2c_driver = {
.driver = {
.name = RX6110_DRIVER_NAME,
+#ifdef CONFIG_ACPI
+ .acpi_match_table = ACPI_PTR(rx6110_i2c_acpi_match),
+#endif
},
.probe = rx6110_i2c_probe,
.id_table = rx6110_i2c_id,
--
2.20.1
Hello Claudius!
It has been a while ;)
On 04/11/2020 11:26:27+0100, Claudius Heine wrote:
> Hi,
>
> this patch introduces I2C support to the RX6110 RTC driver and also adds
> an ACPI identifier to it.
>
> Since we are also pushing the coreboot changes for the ACPI table
> upstream in parallel, we are free to name this ACPI entry however we
> like it seems. So any feedback on that would be welcome ;)
>
I don't care too much about ACPI so if you are really looking for advice
there, I guess you should ask seom of the ACPI guys (but I guess you are
free to choose whatever you want).
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Hi Alex,
On 2020-11-05 23:14, Alexandre Belloni wrote:
> Hello Claudius!
>
> It has been a while ;)
yeah, lots of downstream stuff for me :/
>
> On 04/11/2020 11:26:27+0100, Claudius Heine wrote:
>> Hi,
>>
>> this patch introduces I2C support to the RX6110 RTC driver and also adds
>> an ACPI identifier to it.
>>
>> Since we are also pushing the coreboot changes for the ACPI table
>> upstream in parallel, we are free to name this ACPI entry however we
>> like it seems. So any feedback on that would be welcome ;)
>>
>
> I don't care too much about ACPI so if you are really looking for advice
> there, I guess you should ask seom of the ACPI guys (but I guess you are
> free to choose whatever you want).
As Henning said, we are also getting feedback from the coreboot people.
See you hopefully soon again, when all this sitting at home is over.
regards,
Claudius
On 06/11/2020 08:40:34+0100, Henning Schild wrote:
> Hi,
>
> Am Thu, 5 Nov 2020 23:14:51 +0100
> schrieb Alexandre Belloni <[email protected]>:
>
> > Hello Claudius!
> >
> > It has been a while ;)
> >
> > On 04/11/2020 11:26:27+0100, Claudius Heine wrote:
> > > Hi,
> > >
> > > this patch introduces I2C support to the RX6110 RTC driver and also
> > > adds an ACPI identifier to it.
> > >
> > > Since we are also pushing the coreboot changes for the ACPI table
> > > upstream in parallel, we are free to name this ACPI entry however we
> > > like it seems. So any feedback on that would be welcome ;)
> > >
> >
> > I don't care too much about ACPI so if you are really looking for
> > advice there, I guess you should ask seom of the ACPI guys (but I
> > guess you are free to choose whatever you want).
> >
>
> This is the coreboot stuff currently under review.
>
> https://review.coreboot.org/c/coreboot/+/47235
>
I can't really comment on the patch, however another part is worrying:
if VLF is set, coreboot is resetting the time to a valid value (user
defined or the build date). This is nasty because this hides the event
from the kernel and ulimately, userspace has no way of knowing whether
the RTC date is the real date or just a dummy date.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Hi,
Am Thu, 5 Nov 2020 23:14:51 +0100
schrieb Alexandre Belloni <[email protected]>:
> Hello Claudius!
>
> It has been a while ;)
>
> On 04/11/2020 11:26:27+0100, Claudius Heine wrote:
> > Hi,
> >
> > this patch introduces I2C support to the RX6110 RTC driver and also
> > adds an ACPI identifier to it.
> >
> > Since we are also pushing the coreboot changes for the ACPI table
> > upstream in parallel, we are free to name this ACPI entry however we
> > like it seems. So any feedback on that would be welcome ;)
> >
>
> I don't care too much about ACPI so if you are really looking for
> advice there, I guess you should ask seom of the ACPI guys (but I
> guess you are free to choose whatever you want).
>
This is the coreboot stuff currently under review.
https://review.coreboot.org/c/coreboot/+/47235
Henning
Am Fri, 6 Nov 2020 08:59:08 +0100
schrieb Alexandre Belloni <[email protected]>:
> On 06/11/2020 08:40:34+0100, Henning Schild wrote:
> > Hi,
> >
> > Am Thu, 5 Nov 2020 23:14:51 +0100
> > schrieb Alexandre Belloni <[email protected]>:
> >
> > > Hello Claudius!
> > >
> > > It has been a while ;)
> > >
> > > On 04/11/2020 11:26:27+0100, Claudius Heine wrote:
> > > > Hi,
> > > >
> > > > this patch introduces I2C support to the RX6110 RTC driver and
> > > > also adds an ACPI identifier to it.
> > > >
> > > > Since we are also pushing the coreboot changes for the ACPI
> > > > table upstream in parallel, we are free to name this ACPI entry
> > > > however we like it seems. So any feedback on that would be
> > > > welcome ;)
> > >
> > > I don't care too much about ACPI so if you are really looking for
> > > advice there, I guess you should ask seom of the ACPI guys (but I
> > > guess you are free to choose whatever you want).
> > >
> >
> > This is the coreboot stuff currently under review.
> >
> > https://review.coreboot.org/c/coreboot/+/47235
> >
>
> I can't really comment on the patch, however another part is worrying:
> if VLF is set, coreboot is resetting the time to a valid value (user
> defined or the build date). This is nasty because this hides the event
> from the kernel and ulimately, userspace has no way of knowing whether
> the RTC date is the real date or just a dummy date.
Is that worrying problem part of the patch, or just a general
observation looking at their driver?
I think in the patches we should focus on whether I2C and ACPI support
should be added, and how.
Henning
On 06/11/2020 09:57:56+0100, Henning Schild wrote:
> Am Fri, 6 Nov 2020 08:59:08 +0100
> schrieb Alexandre Belloni <[email protected]>:
>
> > On 06/11/2020 08:40:34+0100, Henning Schild wrote:
> > > Hi,
> > >
> > > Am Thu, 5 Nov 2020 23:14:51 +0100
> > > schrieb Alexandre Belloni <[email protected]>:
> > >
> > > > Hello Claudius!
> > > >
> > > > It has been a while ;)
> > > >
> > > > On 04/11/2020 11:26:27+0100, Claudius Heine wrote:
> > > > > Hi,
> > > > >
> > > > > this patch introduces I2C support to the RX6110 RTC driver and
> > > > > also adds an ACPI identifier to it.
> > > > >
> > > > > Since we are also pushing the coreboot changes for the ACPI
> > > > > table upstream in parallel, we are free to name this ACPI entry
> > > > > however we like it seems. So any feedback on that would be
> > > > > welcome ;)
> > > >
> > > > I don't care too much about ACPI so if you are really looking for
> > > > advice there, I guess you should ask seom of the ACPI guys (but I
> > > > guess you are free to choose whatever you want).
> > > >
> > >
> > > This is the coreboot stuff currently under review.
> > >
> > > https://review.coreboot.org/c/coreboot/+/47235
> > >
> >
> > I can't really comment on the patch, however another part is worrying:
> > if VLF is set, coreboot is resetting the time to a valid value (user
> > defined or the build date). This is nasty because this hides the event
> > from the kernel and ulimately, userspace has no way of knowing whether
> > the RTC date is the real date or just a dummy date.
>
> Is that worrying problem part of the patch, or just a general
> observation looking at their driver?
>
It is a separate observation on their driver.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com