Linux-RTC Archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2]  Adding I2C support to RX6110 RTC
@ 2020-11-04 10:26 Claudius Heine
  2020-11-04 10:26 ` [PATCH 1/2] rtc: rx6110: add i2c support Claudius Heine
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Claudius Heine @ 2020-11-04 10:26 UTC (permalink / raw
  To: Alessandro Zummo, Alexandre Belloni, linux-rtc, linux-kernel
  Cc: Henning Schild, Johannes Hahn, Claudius Heine

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] rtc: rx6110: add i2c support
  2020-11-04 10:26 [PATCH 0/2] Adding I2C support to RX6110 RTC Claudius Heine
@ 2020-11-04 10:26 ` Claudius Heine
  2020-11-05 22:19   ` Alexandre Belloni
  2020-11-04 10:26 ` [PATCH 2/2] rtc: rx6110: add ACPI bindings to I2C Claudius Heine
  2020-11-05 22:14 ` [PATCH 0/2] Adding I2C support to RX6110 RTC Alexandre Belloni
  2 siblings, 1 reply; 10+ messages in thread
From: Claudius Heine @ 2020-11-04 10:26 UTC (permalink / raw
  To: Alessandro Zummo, Alexandre Belloni, linux-rtc, linux-kernel
  Cc: Henning Schild, Johannes Hahn, Claudius Heine

The RX6110 also supports I2C, so this patch adds support for it to the
driver.

This also renames the SPI specific functions and variables to include
`_spi_` in their names.

Signed-off-by: Claudius Heine <ch@denx.de>
Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 drivers/rtc/Kconfig      |  20 +++---
 drivers/rtc/rtc-rx6110.c | 143 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 146 insertions(+), 17 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 65ad9d0b47ab..1fe411ffb19c 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -817,15 +817,6 @@ config RTC_DRV_RX4581
 	  This driver can also be built as a module. If so the module
 	  will be called rtc-rx4581.
 
-config RTC_DRV_RX6110
-	tristate "Epson RX-6110"
-	select REGMAP_SPI
-	help
-	  If you say yes here you will get support for the Epson RX-6610.
-
-	  This driver can also be built as a module. If so the module
-	  will be called rtc-rx6110.
-
 config RTC_DRV_RS5C348
 	tristate "Ricoh RS5C348A/B"
 	help
@@ -936,6 +927,17 @@ config RTC_DRV_RV3029_HWMON
 	  Say Y here if you want to expose temperature sensor data on
 	  rtc-rv3029.
 
+config RTC_DRV_RX6110
+	tristate "Epson RX-6110"
+	depends on RTC_I2C_AND_SPI
+	select REGMAP_SPI if SPI_MASTER
+	select REGMAP_I2C if I2C
+	help
+	  If you say yes here you will get support for the Epson RX-6610.
+
+	  This driver can also be built as a module. If so the module
+	  will be called rtc-rx6110.
+
 comment "Platform RTC drivers"
 
 # this 'CMOS' RTC driver is arch dependent because it requires
diff --git a/drivers/rtc/rtc-rx6110.c b/drivers/rtc/rtc-rx6110.c
index 3a9eb7043f01..ca9f486236d4 100644
--- a/drivers/rtc/rtc-rx6110.c
+++ b/drivers/rtc/rtc-rx6110.c
@@ -16,6 +16,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/spi/spi.h>
+#include <linux/i2c.h>
 
 /* RX-6110 Register definitions */
 #define RX6110_REG_SEC		0x10
@@ -310,6 +311,7 @@ static const struct rtc_class_ops rx6110_rtc_ops = {
 	.set_time = rx6110_set_time,
 };
 
+#ifdef CONFIG_SPI_MASTER
 static struct regmap_config regmap_spi_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
@@ -318,10 +320,10 @@ static struct regmap_config regmap_spi_config = {
 };
 
 /**
- * rx6110_probe - initialize rtc driver
+ * rx6110_spi_probe - initialize rtc driver
  * @spi: pointer to spi device
  */
-static int rx6110_probe(struct spi_device *spi)
+static int rx6110_spi_probe(struct spi_device *spi)
 {
 	struct rx6110_data *rx6110;
 	int err;
@@ -362,11 +364,11 @@ static int rx6110_probe(struct spi_device *spi)
 	return 0;
 }
 
-static const struct spi_device_id rx6110_id[] = {
+static const struct spi_device_id rx6110_spi_id[] = {
 	{ "rx6110", 0 },
 	{ }
 };
-MODULE_DEVICE_TABLE(spi, rx6110_id);
+MODULE_DEVICE_TABLE(spi, rx6110_spi_id);
 
 static const struct of_device_id rx6110_spi_of_match[] = {
 	{ .compatible = "epson,rx6110" },
@@ -374,16 +376,141 @@ static const struct of_device_id rx6110_spi_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, rx6110_spi_of_match);
 
-static struct spi_driver rx6110_driver = {
+static struct spi_driver rx6110_spi_driver = {
 	.driver = {
 		.name = RX6110_DRIVER_NAME,
 		.of_match_table = of_match_ptr(rx6110_spi_of_match),
 	},
-	.probe		= rx6110_probe,
-	.id_table	= rx6110_id,
+	.probe		= rx6110_spi_probe,
+	.id_table	= rx6110_spi_id,
 };
 
-module_spi_driver(rx6110_driver);
+static int rx6110_spi_register(void)
+{
+	return spi_register_driver(&rx6110_spi_driver);
+}
+
+static void rx6110_spi_unregister(void)
+{
+	spi_unregister_driver(&rx6110_spi_driver);
+}
+#else
+static int rx6110_spi_register(void)
+{
+	return 0;
+}
+
+static void rx6110_spi_unregister(void)
+{
+}
+#endif /* CONFIG_SPI_MASTER */
+
+#ifdef CONFIG_I2C
+static struct regmap_config regmap_i2c_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = RX6110_REG_IRQ,
+	.read_flag_mask = 0x80,
+};
+
+static int rx6110_i2c_probe(struct i2c_client *client,
+			    const struct i2c_device_id *id)
+{
+	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
+	struct rx6110_data *rx6110;
+	int err;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA
+				| I2C_FUNC_SMBUS_I2C_BLOCK)) {
+		dev_err(&adapter->dev,
+			"doesn't support required functionality\n");
+		return -EIO;
+	}
+
+	rx6110 = devm_kzalloc(&client->dev, sizeof(*rx6110), GFP_KERNEL);
+	if (!rx6110)
+		return -ENOMEM;
+
+	rx6110->regmap = devm_regmap_init_i2c(client, &regmap_i2c_config);
+	if (IS_ERR(rx6110->regmap)) {
+		dev_err(&client->dev, "regmap init failed for rtc rx6110\n");
+		return PTR_ERR(rx6110->regmap);
+	}
+
+	i2c_set_clientdata(client, rx6110);
+
+	rx6110->rtc = devm_rtc_device_register(&client->dev,
+					       client->name,
+					       &rx6110_rtc_ops, THIS_MODULE);
+
+	if (IS_ERR(rx6110->rtc))
+		return PTR_ERR(rx6110->rtc);
+
+	err = rx6110_init(rx6110);
+	if (err)
+		return err;
+
+	rx6110->rtc->max_user_freq = 1;
+
+	return 0;
+}
+
+static const struct i2c_device_id rx6110_i2c_id[] = {
+	{ "rx6110", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, rx6110_i2c_id);
+
+static struct i2c_driver rx6110_i2c_driver = {
+	.driver = {
+		.name = RX6110_DRIVER_NAME,
+	},
+	.probe		= rx6110_i2c_probe,
+	.id_table	= rx6110_i2c_id,
+};
+
+static int rx6110_i2c_register(void)
+{
+	return i2c_add_driver(&rx6110_i2c_driver);
+}
+
+static void rx6110_i2c_unregister(void)
+{
+	i2c_del_driver(&rx6110_i2c_driver);
+}
+#else
+static int rx6110_i2c_register(void)
+{
+	return 0;
+}
+
+static void rx6110_i2c_unregister(void)
+{
+}
+#endif /* CONFIG_I2C */
+
+static int __init rx6110_module_init(void)
+{
+	int ret;
+
+	ret = rx6110_spi_register();
+	if (ret)
+		return ret;
+
+	ret = rx6110_i2c_register();
+	if (ret)
+		rx6110_spi_unregister();
+
+	return ret;
+}
+module_init(rx6110_module_init);
+
+static void __exit rx6110_module_exit(void)
+{
+	rx6110_spi_unregister();
+	rx6110_i2c_unregister();
+}
+module_exit(rx6110_module_exit);
 
 MODULE_AUTHOR("Val Krutov <val.krutov@erd.epson.com>");
 MODULE_DESCRIPTION("RX-6110 SA RTC driver");
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] rtc: rx6110: add ACPI bindings to I2C
  2020-11-04 10:26 [PATCH 0/2] Adding I2C support to RX6110 RTC Claudius Heine
  2020-11-04 10:26 ` [PATCH 1/2] rtc: rx6110: add i2c support Claudius Heine
@ 2020-11-04 10:26 ` Claudius Heine
  2020-11-05 22:14 ` [PATCH 0/2] Adding I2C support to RX6110 RTC Alexandre Belloni
  2 siblings, 0 replies; 10+ messages in thread
From: Claudius Heine @ 2020-11-04 10:26 UTC (permalink / raw
  To: Alessandro Zummo, Alexandre Belloni, linux-rtc, linux-kernel
  Cc: Henning Schild, Johannes Hahn, Claudius Heine

From: Johannes Hahn <johannes-hahn@siemens.com>

This allows the RX6110 driver to be automatically assigned to the right
device on the I2C bus.

Signed-off-by: Johannes Hahn <johannes-hahn@siemens.com>
Signed-off-by: Claudius Heine <ch@denx.de>
Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/2]  Adding I2C support to RX6110 RTC
  2020-11-04 10:26 [PATCH 0/2] Adding I2C support to RX6110 RTC Claudius Heine
  2020-11-04 10:26 ` [PATCH 1/2] rtc: rx6110: add i2c support Claudius Heine
  2020-11-04 10:26 ` [PATCH 2/2] rtc: rx6110: add ACPI bindings to I2C Claudius Heine
@ 2020-11-05 22:14 ` Alexandre Belloni
  2020-11-06  7:40   ` Henning Schild
  2020-11-06  7:51   ` Claudius Heine
  2 siblings, 2 replies; 10+ messages in thread
From: Alexandre Belloni @ 2020-11-05 22:14 UTC (permalink / raw
  To: Claudius Heine
  Cc: Alessandro Zummo, linux-rtc, linux-kernel, Henning Schild,
	Johannes Hahn

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] rtc: rx6110: add i2c support
  2020-11-04 10:26 ` [PATCH 1/2] rtc: rx6110: add i2c support Claudius Heine
@ 2020-11-05 22:19   ` Alexandre Belloni
  0 siblings, 0 replies; 10+ messages in thread
From: Alexandre Belloni @ 2020-11-05 22:19 UTC (permalink / raw
  To: Claudius Heine
  Cc: Alessandro Zummo, linux-rtc, linux-kernel, Henning Schild,
	Johannes Hahn

On 04/11/2020 11:26:28+0100, Claudius Heine wrote:
> +static int rx6110_i2c_probe(struct i2c_client *client,
> +			    const struct i2c_device_id *id)
> +{
> +	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
> +	struct rx6110_data *rx6110;
> +	int err;
> +
> +	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA
> +				| I2C_FUNC_SMBUS_I2C_BLOCK)) {
> +		dev_err(&adapter->dev,
> +			"doesn't support required functionality\n");
> +		return -EIO;
> +	}
> +
> +	rx6110 = devm_kzalloc(&client->dev, sizeof(*rx6110), GFP_KERNEL);
> +	if (!rx6110)
> +		return -ENOMEM;
> +
> +	rx6110->regmap = devm_regmap_init_i2c(client, &regmap_i2c_config);
> +	if (IS_ERR(rx6110->regmap)) {
> +		dev_err(&client->dev, "regmap init failed for rtc rx6110\n");
> +		return PTR_ERR(rx6110->regmap);
> +	}
> +
> +	i2c_set_clientdata(client, rx6110);
> +
> +	rx6110->rtc = devm_rtc_device_register(&client->dev,
> +					       client->name,
> +					       &rx6110_rtc_ops, THIS_MODULE);
> +
> +	if (IS_ERR(rx6110->rtc))
> +		return PTR_ERR(rx6110->rtc);
> +
> +	err = rx6110_init(rx6110);
> +	if (err)
> +		return err;
> +
> +	rx6110->rtc->max_user_freq = 1;
> +

I would prefer if you could have a common function doing the RTC
registration and init for both i2c and spi. It wouldn't do much yet but
ideally, it would set the RTC range too and it would be better to not
have to duplicate that.


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/2]  Adding I2C support to RX6110 RTC
  2020-11-05 22:14 ` [PATCH 0/2] Adding I2C support to RX6110 RTC Alexandre Belloni
@ 2020-11-06  7:40   ` Henning Schild
  2020-11-06  7:59     ` Alexandre Belloni
  2020-11-06  7:51   ` Claudius Heine
  1 sibling, 1 reply; 10+ messages in thread
From: Henning Schild @ 2020-11-06  7:40 UTC (permalink / raw
  To: Alexandre Belloni
  Cc: Claudius Heine, Alessandro Zummo, linux-rtc, linux-kernel,
	Johannes Hahn, werner.zeh

Hi,

Am Thu, 5 Nov 2020 23:14:51 +0100
schrieb Alexandre Belloni <alexandre.belloni@bootlin.com>:

> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/2] Adding I2C support to RX6110 RTC
  2020-11-05 22:14 ` [PATCH 0/2] Adding I2C support to RX6110 RTC Alexandre Belloni
  2020-11-06  7:40   ` Henning Schild
@ 2020-11-06  7:51   ` Claudius Heine
  1 sibling, 0 replies; 10+ messages in thread
From: Claudius Heine @ 2020-11-06  7:51 UTC (permalink / raw
  To: Alexandre Belloni
  Cc: Alessandro Zummo, linux-rtc, linux-kernel, Henning Schild,
	Johannes Hahn

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/2]  Adding I2C support to RX6110 RTC
  2020-11-06  7:40   ` Henning Schild
@ 2020-11-06  7:59     ` Alexandre Belloni
  2020-11-06  8:57       ` Henning Schild
  0 siblings, 1 reply; 10+ messages in thread
From: Alexandre Belloni @ 2020-11-06  7:59 UTC (permalink / raw
  To: Henning Schild
  Cc: Claudius Heine, Alessandro Zummo, linux-rtc, linux-kernel,
	Johannes Hahn, werner.zeh

On 06/11/2020 08:40:34+0100, Henning Schild wrote:
> Hi,
> 
> Am Thu, 5 Nov 2020 23:14:51 +0100
> schrieb Alexandre Belloni <alexandre.belloni@bootlin.com>:
> 
> > 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/2]  Adding I2C support to RX6110 RTC
  2020-11-06  7:59     ` Alexandre Belloni
@ 2020-11-06  8:57       ` Henning Schild
  2020-11-06  9:08         ` Alexandre Belloni
  0 siblings, 1 reply; 10+ messages in thread
From: Henning Schild @ 2020-11-06  8:57 UTC (permalink / raw
  To: Alexandre Belloni
  Cc: Claudius Heine, Alessandro Zummo, linux-rtc, linux-kernel,
	Johannes Hahn, werner.zeh

Am Fri, 6 Nov 2020 08:59:08 +0100
schrieb Alexandre Belloni <alexandre.belloni@bootlin.com>:

> On 06/11/2020 08:40:34+0100, Henning Schild wrote:
> > Hi,
> > 
> > Am Thu, 5 Nov 2020 23:14:51 +0100
> > schrieb Alexandre Belloni <alexandre.belloni@bootlin.com>:
> >   
> > > 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/2]  Adding I2C support to RX6110 RTC
  2020-11-06  8:57       ` Henning Schild
@ 2020-11-06  9:08         ` Alexandre Belloni
  0 siblings, 0 replies; 10+ messages in thread
From: Alexandre Belloni @ 2020-11-06  9:08 UTC (permalink / raw
  To: Henning Schild
  Cc: Claudius Heine, Alessandro Zummo, linux-rtc, linux-kernel,
	Johannes Hahn, werner.zeh

On 06/11/2020 09:57:56+0100, Henning Schild wrote:
> Am Fri, 6 Nov 2020 08:59:08 +0100
> schrieb Alexandre Belloni <alexandre.belloni@bootlin.com>:
> 
> > On 06/11/2020 08:40:34+0100, Henning Schild wrote:
> > > Hi,
> > > 
> > > Am Thu, 5 Nov 2020 23:14:51 +0100
> > > schrieb Alexandre Belloni <alexandre.belloni@bootlin.com>:
> > >   
> > > > 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-11-06  9:09 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-04 10:26 [PATCH 0/2] Adding I2C support to RX6110 RTC Claudius Heine
2020-11-04 10:26 ` [PATCH 1/2] rtc: rx6110: add i2c support Claudius Heine
2020-11-05 22:19   ` Alexandre Belloni
2020-11-04 10:26 ` [PATCH 2/2] rtc: rx6110: add ACPI bindings to I2C Claudius Heine
2020-11-05 22:14 ` [PATCH 0/2] Adding I2C support to RX6110 RTC Alexandre Belloni
2020-11-06  7:40   ` Henning Schild
2020-11-06  7:59     ` Alexandre Belloni
2020-11-06  8:57       ` Henning Schild
2020-11-06  9:08         ` Alexandre Belloni
2020-11-06  7:51   ` Claudius Heine

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).