* [PATCH v3 0/4] Add NXP pcf85263 real-time clock support @ 2018-12-06 8:55 Biju Das 2018-12-06 8:55 ` [PATCH v3 1/4] dt-bindings: rtc: pcf85363: Document pcf85263 real-time clock Biju Das ` (3 more replies) 0 siblings, 4 replies; 22+ messages in thread From: Biju Das @ 2018-12-06 8:55 UTC (permalink / raw) To: Alessandro Zummo, Alexandre Belloni, Geert Uytterhoeven, Rob Herring, Mark Rutland Cc: Biju Das, linux-rtc, devicetree, Simon Horman, Chris Paterson, Fabrizio Castro, linux-renesas-soc This patch set aims to add support for NXP pcf85263 real-time clock. pcf85263 rtc is compatible with pcf85363 rtc except that pcf85363 has 64 bytes additional RAM. 1 byte of nvmem is supported in pcf85263 and is exposed through sysfs. The details of pcf85363 and pcf85263 can be found in the below data sheets. https://www.nxp.com/docs/en/data-sheet/PCF85363A.pdf https://www.nxp.com/docs/en/data-sheet/PCF85263A.pdf This patch is tested against linux-next. V1-->V2 * Incorporated simon's review comment for binding patch. * Incorporated Geert and Alexandre's review comments. V2-->V3 * Incorporated Geert's review comments. Biju Das (4): dt-bindings: rtc: pcf85363: Document pcf85263 real-time clock rtc: pcf85363: Add support for NXP pcf85263 rtc ARM: shmobile: Enable NXP pcf85363 rtc in shmobile_defconfig ARM: dts: iwg23s-sbc: Enable RTC Documentation/devicetree/bindings/rtc/pcf85363.txt | 4 +- arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts | 18 +++++ arch/arm/configs/shmobile_defconfig | 1 + drivers/rtc/rtc-pcf85363.c | 87 +++++++++++++++++----- 4 files changed, 90 insertions(+), 20 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 1/4] dt-bindings: rtc: pcf85363: Document pcf85263 real-time clock 2018-12-06 8:55 [PATCH v3 0/4] Add NXP pcf85263 real-time clock support Biju Das @ 2018-12-06 8:55 ` Biju Das 2018-12-06 9:13 ` Geert Uytterhoeven 2018-12-06 8:55 ` [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc Biju Das ` (2 subsequent siblings) 3 siblings, 1 reply; 22+ messages in thread From: Biju Das @ 2018-12-06 8:55 UTC (permalink / raw) To: Alessandro Zummo, Alexandre Belloni, Rob Herring, Mark Rutland Cc: Biju Das, linux-rtc, devicetree, Simon Horman, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro, linux-renesas-soc The pcf85263 RTC is compatible with the pcf85363 RTC. The difference between the pcf85263 and pcf85363 RTC is that the latter has 64 bytes more RAM. This renders them incompatible from a DT point of view. Signed-off-by: Biju Das <biju.das@bp.renesas.com> --- V1-->V2 * Incorporated Simon's review comment. V2-->V3 * No Change --- Documentation/devicetree/bindings/rtc/pcf85363.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/rtc/pcf85363.txt b/Documentation/devicetree/bindings/rtc/pcf85363.txt index 76fdabc..94adc1c 100644 --- a/Documentation/devicetree/bindings/rtc/pcf85363.txt +++ b/Documentation/devicetree/bindings/rtc/pcf85363.txt @@ -1,8 +1,8 @@ -NXP PCF85363 Real Time Clock +NXP PCF85263/PCF85363 Real Time Clock ============================ Required properties: -- compatible: Should contain "nxp,pcf85363". +- compatible: Should contain "nxp,pcf85263" or "nxp,pcf85363". - reg: I2C address for chip. Optional properties: -- 2.7.4 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v3 1/4] dt-bindings: rtc: pcf85363: Document pcf85263 real-time clock 2018-12-06 8:55 ` [PATCH v3 1/4] dt-bindings: rtc: pcf85363: Document pcf85263 real-time clock Biju Das @ 2018-12-06 9:13 ` Geert Uytterhoeven 0 siblings, 0 replies; 22+ messages in thread From: Geert Uytterhoeven @ 2018-12-06 9:13 UTC (permalink / raw) To: Biju Das Cc: Alessandro Zummo, Alexandre Belloni, Rob Herring, Mark Rutland, linux-rtc, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Simon Horman, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro, Linux-Renesas On Thu, Dec 6, 2018 at 10:04 AM Biju Das <biju.das@bp.renesas.com> wrote: > The pcf85263 RTC is compatible with the pcf85363 RTC. > > The difference between the pcf85263 and pcf85363 RTC is that the latter has > 64 bytes more RAM. This renders them incompatible from a DT point of view. > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc 2018-12-06 8:55 [PATCH v3 0/4] Add NXP pcf85263 real-time clock support Biju Das 2018-12-06 8:55 ` [PATCH v3 1/4] dt-bindings: rtc: pcf85363: Document pcf85263 real-time clock Biju Das @ 2018-12-06 8:55 ` Biju Das 2018-12-06 9:39 ` Geert Uytterhoeven 2018-12-06 8:55 ` [PATCH v3 3/4] ARM: shmobile: Enable NXP pcf85363 rtc in shmobile_defconfig Biju Das 2018-12-06 8:55 ` [PATCH v3 4/4] ARM: dts: iwg23s-sbc: Enable RTC Biju Das 3 siblings, 1 reply; 22+ messages in thread From: Biju Das @ 2018-12-06 8:55 UTC (permalink / raw) To: Alessandro Zummo, Geert Uytterhoeven, Alexandre Belloni Cc: Biju Das, linux-rtc, Simon Horman, Chris Paterson, Fabrizio Castro, linux-renesas-soc Add support for NXP pcf85263 real-time clock. pcf85263 rtc is compatible with pcf85363,except that pcf85363 has additional 64 bytes of RAM. 1 byte of nvmem is supported and exposed in sysfs (# is the instance number,starting with 0): /sys/bus/nvmem/devices/pcf85x63-#/nvmem Signed-off-by: Biju Das <biju.das@bp.renesas.com> --- V1-->V2 * Incorporated Alexandre and Geert's review comment. V2-->V3 * Incorporated Geert's review comment. --- drivers/rtc/rtc-pcf85363.c | 87 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 69 insertions(+), 18 deletions(-) diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c index c04a1ed..6a0a994 100644 --- a/drivers/rtc/rtc-pcf85363.c +++ b/drivers/rtc/rtc-pcf85363.c @@ -120,6 +120,11 @@ struct pcf85363 { struct regmap *regmap; }; +struct pcf85x63_config { + struct regmap_config regmap; + unsigned int num_nvram; +}; + static int pcf85363_rtc_read_time(struct device *dev, struct rtc_time *tm) { struct pcf85363 *pcf85363 = dev_get_drvdata(dev); @@ -311,25 +316,68 @@ static int pcf85363_nvram_write(void *priv, unsigned int offset, void *val, val, bytes); } -static const struct regmap_config regmap_config = { - .reg_bits = 8, - .val_bits = 8, - .max_register = 0x7f, +static int pcf85x63_nvram_read(void *priv, unsigned int offset, void *val, + size_t bytes) +{ + struct pcf85363 *pcf85363 = priv; + + return regmap_read(pcf85363->regmap, CTRL_RAMBYTE, val); +} + +static int pcf85x63_nvram_write(void *priv, unsigned int offset, void *val, + size_t bytes) +{ + struct pcf85363 *pcf85363 = priv; + + return regmap_write(pcf85363->regmap, CTRL_RAMBYTE, + *((unsigned int *)val)); +} + +static const struct pcf85x63_config pcf_85263_config = { + { + .reg_bits = 8, + .val_bits = 8, + .max_register = 0x2f, + }, + 1 +}; + +static const struct pcf85x63_config pcf_85363_config = { + { + .reg_bits = 8, + .val_bits = 8, + .max_register = 0x7f, + }, + 2 }; static int pcf85363_probe(struct i2c_client *client, const struct i2c_device_id *id) { struct pcf85363 *pcf85363; - struct nvmem_config nvmem_cfg = { - .name = "pcf85363-", - .word_size = 1, - .stride = 1, - .size = NVRAM_SIZE, - .reg_read = pcf85363_nvram_read, - .reg_write = pcf85363_nvram_write, + const struct pcf85x63_config *config = &pcf_85363_config; + const void *data = of_device_get_match_data(&client->dev); + static struct nvmem_config nvmem_cfg[] = { + { + .name = "pcf85x63-", + .word_size = 1, + .stride = 1, + .size = 1, + .reg_read = pcf85x63_nvram_read, + .reg_write = pcf85x63_nvram_write, + }, { + .name = "pcf85363-", + .word_size = 1, + .stride = 1, + .size = NVRAM_SIZE, + .reg_read = pcf85363_nvram_read, + .reg_write = pcf85363_nvram_write, + }, }; - int ret; + int ret, i; + + if (data) + config = data; if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) return -ENODEV; @@ -339,7 +387,7 @@ static int pcf85363_probe(struct i2c_client *client, if (!pcf85363) return -ENOMEM; - pcf85363->regmap = devm_regmap_init_i2c(client, ®map_config); + pcf85363->regmap = devm_regmap_init_i2c(client, &config->regmap); if (IS_ERR(pcf85363->regmap)) { dev_err(&client->dev, "regmap allocation failed\n"); return PTR_ERR(pcf85363->regmap); @@ -370,15 +418,18 @@ static int pcf85363_probe(struct i2c_client *client, ret = rtc_register_device(pcf85363->rtc); - nvmem_cfg.priv = pcf85363; - rtc_nvmem_register(pcf85363->rtc, &nvmem_cfg); + for (i = 0; i < config->num_nvram; i++) { + nvmem_cfg[i].priv = pcf85363; + rtc_nvmem_register(pcf85363->rtc, &nvmem_cfg[i]); + } return ret; } static const struct of_device_id dev_ids[] = { - { .compatible = "nxp,pcf85363" }, - {} + { .compatible = "nxp,pcf85263", .data = &pcf_85263_config }, + { .compatible = "nxp,pcf85363", .data = &pcf_85363_config }, + { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, dev_ids); @@ -393,5 +444,5 @@ static struct i2c_driver pcf85363_driver = { module_i2c_driver(pcf85363_driver); MODULE_AUTHOR("Eric Nelson"); -MODULE_DESCRIPTION("pcf85363 I2C RTC driver"); +MODULE_DESCRIPTION("pcf85263/pcf85363 I2C RTC driver"); MODULE_LICENSE("GPL"); -- 2.7.4 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc 2018-12-06 8:55 ` [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc Biju Das @ 2018-12-06 9:39 ` Geert Uytterhoeven 2018-12-06 15:24 ` Biju Das 0 siblings, 1 reply; 22+ messages in thread From: Geert Uytterhoeven @ 2018-12-06 9:39 UTC (permalink / raw) To: Biju Das Cc: Alessandro Zummo, Geert Uytterhoeven, Alexandre Belloni, linux-rtc, Simon Horman, Chris Paterson, Fabrizio Castro, Linux-Renesas, Srinivas Kandagatla Hi Biju, CC nvmem maintainer On Thu, Dec 6, 2018 at 10:04 AM Biju Das <biju.das@bp.renesas.com> wrote: > Add support for NXP pcf85263 real-time clock. pcf85263 rtc is compatible > with pcf85363,except that pcf85363 has additional 64 bytes of RAM. > > 1 byte of nvmem is supported and exposed in sysfs (# is the instance > number,starting with 0): /sys/bus/nvmem/devices/pcf85x63-#/nvmem > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > --- > V1-->V2 > * Incorporated Alexandre and Geert's review comment. > V2-->V3 > * Incorporated Geert's review comment. Thanks for the update! > --- a/drivers/rtc/rtc-pcf85363.c > +++ b/drivers/rtc/rtc-pcf85363.c > @@ -120,6 +120,11 @@ struct pcf85363 { > struct regmap *regmap; > }; > > +struct pcf85x63_config { > + struct regmap_config regmap; > + unsigned int num_nvram; > +}; > + > static int pcf85363_rtc_read_time(struct device *dev, struct rtc_time *tm) > { > struct pcf85363 *pcf85363 = dev_get_drvdata(dev); > @@ -311,25 +316,68 @@ static int pcf85363_nvram_write(void *priv, unsigned int offset, void *val, > val, bytes); > } > > -static const struct regmap_config regmap_config = { > - .reg_bits = 8, > - .val_bits = 8, > - .max_register = 0x7f, > +static int pcf85x63_nvram_read(void *priv, unsigned int offset, void *val, > + size_t bytes) Given bytes should be 1, val should be a pointer to a single byte... What if bytes == 0? > +{> + struct pcf85363 *pcf85363 = priv; > + > + return regmap_read(pcf85363->regmap, CTRL_RAMBYTE, val); However, regmap_read() has an unsigned int output parameter! So it's writing too many bytes, and only writing the actual data byte to the correct address on little-endian systems. Hence you need to use an intermediate variable to convert from unsigned int to byte. > +} > + > +static int pcf85x63_nvram_write(void *priv, unsigned int offset, void *val, > + size_t bytes) > +{ > + struct pcf85363 *pcf85363 = priv; > + > + return regmap_write(pcf85363->regmap, CTRL_RAMBYTE, > + *((unsigned int *)val)); Likewise for writing. > +} BTW, while the nvmem_device_{read,write}() public API is documented, the nvmem_device.reg_{read,write}() driver API isn't. And the behavior might be confusing. E.g. * Return: length of successful bytes read on success and negative * error code on error. The public API seems to assume the driver API returns zero on success, and replaces the zero by the number of bytes requested. If the requested number of bytes is too large, a zero success would be converted to a value that's larger than the actual number of bytes transferred! However, the driver API can return a smaller (positive) number, which matches "standard" read/write() APIs. > +static const struct pcf85x63_config pcf_85263_config = { > + { > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = 0x2f, > + }, > + 1 The "1" looks funny. Please use C99 initializers for all struct members. > +}; > + > +static const struct pcf85x63_config pcf_85363_config = { > + { > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = 0x7f, > + }, > + 2 Likewise. The rest looks good to me, so Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc 2018-12-06 9:39 ` Geert Uytterhoeven @ 2018-12-06 15:24 ` Biju Das 2018-12-06 15:37 ` Geert Uytterhoeven 0 siblings, 1 reply; 22+ messages in thread From: Biju Das @ 2018-12-06 15:24 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Alessandro Zummo, Geert Uytterhoeven, Alexandre Belloni, linux-rtc, Simon Horman, Chris Paterson, Fabrizio Castro, Linux-Renesas, Srinivas Kandagatla Hi Geert, Thanks for feedback. > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc > > Hi Biju, > > CC nvmem maintainer > > On Thu, Dec 6, 2018 at 10:04 AM Biju Das <biju.das@bp.renesas.com> wrote: > > Add support for NXP pcf85263 real-time clock. pcf85263 rtc is > > compatible with pcf85363,except that pcf85363 has additional 64 bytes of > RAM. > > > > 1 byte of nvmem is supported and exposed in sysfs (# is the instance > > number,starting with 0): /sys/bus/nvmem/devices/pcf85x63-#/nvmem > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > --- > > V1-->V2 > > * Incorporated Alexandre and Geert's review comment. > > V2-->V3 > > * Incorporated Geert's review comment. > > Thanks for the update! > > > --- a/drivers/rtc/rtc-pcf85363.c > > +++ b/drivers/rtc/rtc-pcf85363.c > > @@ -120,6 +120,11 @@ struct pcf85363 { > > struct regmap *regmap; > > }; > > > > +struct pcf85x63_config { > > + struct regmap_config regmap; > > + unsigned int num_nvram; > > +}; > > + > > static int pcf85363_rtc_read_time(struct device *dev, struct rtc_time > > *tm) { > > struct pcf85363 *pcf85363 = dev_get_drvdata(dev); @@ -311,25 > > +316,68 @@ static int pcf85363_nvram_write(void *priv, unsigned int > offset, void *val, > > val, bytes); } > > > > -static const struct regmap_config regmap_config = { > > - .reg_bits = 8, > > - .val_bits = 8, > > - .max_register = 0x7f, > > +static int pcf85x63_nvram_read(void *priv, unsigned int offset, void *val, > > + size_t bytes) > > Given bytes should be 1, val should be a pointer to a single byte... > What if bytes == 0? I doubt we get "bytes==0" because of the checks in " drivers/nvmem/core.c" Function " bin_attr_nvmem_read/ bin_attr_nvmem_write". > > +{> + struct pcf85363 *pcf85363 = priv; > > + > > + return regmap_read(pcf85363->regmap, CTRL_RAMBYTE, val); > > However, regmap_read() has an unsigned int output parameter! > So it's writing too many bytes, and only writing the actual data byte to the > correct address on little-endian systems. > Hence you need to use an intermediate variable to convert from unsigned int > to byte. OK. Will use an intermediate integer variable. > > +} > > + > > +static int pcf85x63_nvram_write(void *priv, unsigned int offset, void *val, > > + size_t bytes) { > > + struct pcf85363 *pcf85363 = priv; > > + > > + return regmap_write(pcf85363->regmap, CTRL_RAMBYTE, > > + *((unsigned int *)val)); > > Likewise for writing. > > > +} > > BTW, while the nvmem_device_{read,write}() public API is documented, the > nvmem_device.reg_{read,write}() driver API isn't. > And the behavior might be confusing. > > E.g. > * Return: length of successful bytes read on success and negative > * error code on error. > > The public API seems to assume the driver API returns zero on success, and > replaces the zero by the number of bytes requested. > If the requested number of bytes is too large, a zero success would be > converted to a value that's larger than the actual number of bytes > transferred! > However, the driver API can return a smaller (positive) number, which > matches "standard" read/write() APIs. > > > +static const struct pcf85x63_config pcf_85263_config = { > > + { > > + .reg_bits = 8, > > + .val_bits = 8, > > + .max_register = 0x2f, > > + }, > > + 1 > > The "1" looks funny. Please use C99 initializers for all struct members. OK will fix this. > > +}; > > + > > +static const struct pcf85x63_config pcf_85363_config = { > > + { > > + .reg_bits = 8, > > + .val_bits = 8, > > + .max_register = 0x7f, > > + }, > > + 2 > > Likewise. OK will fix this. Regards, Biju > The rest looks good to me, so > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux- > m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds [https://www2.renesas.eu/media/email/unicef.jpg] This Christmas, instead of sending out cards, Renesas Electronics Europe have decided to support Unicef with a donation. For further details click here<https://www.unicef.org/> to find out about the valuable work they do, helping children all over the world. We would like to take this opportunity to wish you a Merry Christmas and a prosperous New Year. Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc 2018-12-06 15:24 ` Biju Das @ 2018-12-06 15:37 ` Geert Uytterhoeven 2018-12-06 15:49 ` Biju Das 0 siblings, 1 reply; 22+ messages in thread From: Geert Uytterhoeven @ 2018-12-06 15:37 UTC (permalink / raw) To: Biju Das Cc: Alessandro Zummo, Geert Uytterhoeven, Alexandre Belloni, linux-rtc, Simon Horman, Chris Paterson, Fabrizio Castro, Linux-Renesas, Srinivas Kandagatla Hi Biju, On Thu, Dec 6, 2018 at 4:24 PM Biju Das <biju.das@bp.renesas.com> wrote: > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc > > CC nvmem maintainer > > > > On Thu, Dec 6, 2018 at 10:04 AM Biju Das <biju.das@bp.renesas.com> wrote: > > > Add support for NXP pcf85263 real-time clock. pcf85263 rtc is > > > compatible with pcf85363,except that pcf85363 has additional 64 bytes of > > RAM. > > > --- a/drivers/rtc/rtc-pcf85363.c > > > +++ b/drivers/rtc/rtc-pcf85363.c > > > @@ -120,6 +120,11 @@ struct pcf85363 { > > > struct regmap *regmap; > > > }; > > > > > > +struct pcf85x63_config { > > > + struct regmap_config regmap; > > > + unsigned int num_nvram; > > > +}; > > > + > > > static int pcf85363_rtc_read_time(struct device *dev, struct rtc_time > > > *tm) { > > > struct pcf85363 *pcf85363 = dev_get_drvdata(dev); @@ -311,25 > > > +316,68 @@ static int pcf85363_nvram_write(void *priv, unsigned int > > offset, void *val, > > > val, bytes); } > > > > > > -static const struct regmap_config regmap_config = { > > > - .reg_bits = 8, > > > - .val_bits = 8, > > > - .max_register = 0x7f, > > > +static int pcf85x63_nvram_read(void *priv, unsigned int offset, void *val, > > > + size_t bytes) > > > > Given bytes should be 1, val should be a pointer to a single byte... > > What if bytes == 0? > > I doubt we get "bytes==0" because of the checks in " drivers/nvmem/core.c" > Function " bin_attr_nvmem_read/ bin_attr_nvmem_write". Depends. There are other functions calling nvmem_reg_{read,write}(), e.g. nvmem_device_{read,write}(). > > > > +{> + struct pcf85363 *pcf85363 = priv; > > > + > > > + return regmap_read(pcf85363->regmap, CTRL_RAMBYTE, val); > > > > However, regmap_read() has an unsigned int output parameter! > > So it's writing too many bytes, and only writing the actual data byte to the > > correct address on little-endian systems. > > Hence you need to use an intermediate variable to convert from unsigned int > > to byte. > > OK. Will use an intermediate integer variable. > > > > +} > > > + > > > +static int pcf85x63_nvram_write(void *priv, unsigned int offset, void *val, > > > + size_t bytes) { > > > + struct pcf85363 *pcf85363 = priv; > > > + > > > + return regmap_write(pcf85363->regmap, CTRL_RAMBYTE, > > > + *((unsigned int *)val)); > > > > Likewise for writing. > > > > > +} > > > > BTW, while the nvmem_device_{read,write}() public API is documented, the > > nvmem_device.reg_{read,write}() driver API isn't. > > And the behavior might be confusing. > > > > E.g. > > * Return: length of successful bytes read on success and negative > > * error code on error. > > > > The public API seems to assume the driver API returns zero on success, and > > replaces the zero by the number of bytes requested. > > If the requested number of bytes is too large, a zero success would be > > converted to a value that's larger than the actual number of bytes > > transferred! > > However, the driver API can return a smaller (positive) number, which > > matches "standard" read/write() APIs. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc 2018-12-06 15:37 ` Geert Uytterhoeven @ 2018-12-06 15:49 ` Biju Das 2018-12-06 16:52 ` Alexandre Belloni 0 siblings, 1 reply; 22+ messages in thread From: Biju Das @ 2018-12-06 15:49 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Alessandro Zummo, Geert Uytterhoeven, Alexandre Belloni, linux-rtc, Simon Horman, Chris Paterson, Fabrizio Castro, Linux-Renesas, Srinivas Kandagatla Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc > > Hi Biju, > > On Thu, Dec 6, 2018 at 4:24 PM Biju Das <biju.das@bp.renesas.com> wrote: > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP > > > pcf85263 rtc CC nvmem maintainer > > > > > > On Thu, Dec 6, 2018 at 10:04 AM Biju Das <biju.das@bp.renesas.com> > wrote: > > > > Add support for NXP pcf85263 real-time clock. pcf85263 rtc is > > > > compatible with pcf85363,except that pcf85363 has additional 64 > > > > bytes of > > > RAM. > > > > > --- a/drivers/rtc/rtc-pcf85363.c > > > > +++ b/drivers/rtc/rtc-pcf85363.c > > > > @@ -120,6 +120,11 @@ struct pcf85363 { > > > > struct regmap *regmap; > > > > }; > > > > > > > > +struct pcf85x63_config { > > > > + struct regmap_config regmap; > > > > + unsigned int num_nvram; > > > > +}; > > > > + > > > > static int pcf85363_rtc_read_time(struct device *dev, struct > > > > rtc_time > > > > *tm) { > > > > struct pcf85363 *pcf85363 = dev_get_drvdata(dev); @@ > > > > -311,25 > > > > +316,68 @@ static int pcf85363_nvram_write(void *priv, unsigned > > > > +int > > > offset, void *val, > > > > val, bytes); } > > > > > > > > -static const struct regmap_config regmap_config = { > > > > - .reg_bits = 8, > > > > - .val_bits = 8, > > > > - .max_register = 0x7f, > > > > +static int pcf85x63_nvram_read(void *priv, unsigned int offset, void > *val, > > > > + size_t bytes) > > > > > > Given bytes should be 1, val should be a pointer to a single byte... > > > What if bytes == 0? > > > > I doubt we get "bytes==0" because of the checks in " > drivers/nvmem/core.c" > > Function " bin_attr_nvmem_read/ bin_attr_nvmem_write". > > Depends. There are other functions calling nvmem_reg_{read,write}(), e.g. > nvmem_device_{read,write}(). OK. In that case, I will return (-EINVAL) for "bytes !=1" > > > > > > +{> + struct pcf85363 *pcf85363 = priv; > > > > + > > > > + return regmap_read(pcf85363->regmap, CTRL_RAMBYTE, val); > > > > > > However, regmap_read() has an unsigned int output parameter! > > > So it's writing too many bytes, and only writing the actual data > > > byte to the correct address on little-endian systems. > > > Hence you need to use an intermediate variable to convert from > > > unsigned int to byte. > > > > OK. Will use an intermediate integer variable. > > > > > > +} > > > > + > > > > +static int pcf85x63_nvram_write(void *priv, unsigned int offset, void > *val, > > > > + size_t bytes) { > > > > + struct pcf85363 *pcf85363 = priv; > > > > + > > > > + return regmap_write(pcf85363->regmap, CTRL_RAMBYTE, > > > > + *((unsigned int *)val)); > > > > > > Likewise for writing. > > > > > > > +} > > > > > > BTW, while the nvmem_device_{read,write}() public API is documented, > > > the > > > nvmem_device.reg_{read,write}() driver API isn't. > > > And the behavior might be confusing. > > > > > > E.g. > > > * Return: length of successful bytes read on success and negative > > > * error code on error. > > > > > > The public API seems to assume the driver API returns zero on > > > success, and replaces the zero by the number of bytes requested. > > > If the requested number of bytes is too large, a zero success would > > > be converted to a value that's larger than the actual number of > > > bytes transferred! > > > However, the driver API can return a smaller (positive) number, > > > which matches "standard" read/write() APIs. Regards, Biju [https://www2.renesas.eu/media/email/unicef.jpg] This Christmas, instead of sending out cards, Renesas Electronics Europe have decided to support Unicef with a donation. For further details click here<https://www.unicef.org/> to find out about the valuable work they do, helping children all over the world. We would like to take this opportunity to wish you a Merry Christmas and a prosperous New Year. Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc 2018-12-06 15:49 ` Biju Das @ 2018-12-06 16:52 ` Alexandre Belloni 2018-12-07 9:34 ` Biju Das 0 siblings, 1 reply; 22+ messages in thread From: Alexandre Belloni @ 2018-12-06 16:52 UTC (permalink / raw) To: Biju Das Cc: Geert Uytterhoeven, Alessandro Zummo, Geert Uytterhoeven, linux-rtc, Simon Horman, Chris Paterson, Fabrizio Castro, Linux-Renesas, Srinivas Kandagatla On 06/12/2018 15:49:57+0000, Biju Das wrote: > Hi Geert, > > Thanks for the feedback. > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc > > > > Hi Biju, > > > > On Thu, Dec 6, 2018 at 4:24 PM Biju Das <biju.das@bp.renesas.com> wrote: > > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP > > > > pcf85263 rtc CC nvmem maintainer > > > > > > > > On Thu, Dec 6, 2018 at 10:04 AM Biju Das <biju.das@bp.renesas.com> > > wrote: > > > > > Add support for NXP pcf85263 real-time clock. pcf85263 rtc is > > > > > compatible with pcf85363,except that pcf85363 has additional 64 > > > > > bytes of > > > > RAM. > > > > > > > --- a/drivers/rtc/rtc-pcf85363.c > > > > > +++ b/drivers/rtc/rtc-pcf85363.c > > > > > @@ -120,6 +120,11 @@ struct pcf85363 { > > > > > struct regmap *regmap; > > > > > }; > > > > > > > > > > +struct pcf85x63_config { > > > > > + struct regmap_config regmap; > > > > > + unsigned int num_nvram; > > > > > +}; > > > > > + > > > > > static int pcf85363_rtc_read_time(struct device *dev, struct > > > > > rtc_time > > > > > *tm) { > > > > > struct pcf85363 *pcf85363 = dev_get_drvdata(dev); @@ > > > > > -311,25 > > > > > +316,68 @@ static int pcf85363_nvram_write(void *priv, unsigned > > > > > +int > > > > offset, void *val, > > > > > val, bytes); } > > > > > > > > > > -static const struct regmap_config regmap_config = { > > > > > - .reg_bits = 8, > > > > > - .val_bits = 8, > > > > > - .max_register = 0x7f, > > > > > +static int pcf85x63_nvram_read(void *priv, unsigned int offset, void > > *val, > > > > > + size_t bytes) > > > > > > > > Given bytes should be 1, val should be a pointer to a single byte... > > > > What if bytes == 0? > > > > > > I doubt we get "bytes==0" because of the checks in " > > drivers/nvmem/core.c" > > > Function " bin_attr_nvmem_read/ bin_attr_nvmem_write". > > > > Depends. There are other functions calling nvmem_reg_{read,write}(), e.g. > > nvmem_device_{read,write}(). > > OK. In that case, I will return (-EINVAL) for "bytes !=1" > I think it is probably better to ensure the nvmem core never passes an invalid number of bytes. All the ther RTC drivers make that assumption. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc 2018-12-06 16:52 ` Alexandre Belloni @ 2018-12-07 9:34 ` Biju Das 2018-12-07 9:45 ` Geert Uytterhoeven 0 siblings, 1 reply; 22+ messages in thread From: Biju Das @ 2018-12-07 9:34 UTC (permalink / raw) To: Alexandre Belloni Cc: Geert Uytterhoeven, Alessandro Zummo, Geert Uytterhoeven, linux-rtc, Simon Horman, Chris Paterson, Fabrizio Castro, Linux-Renesas, Srinivas Kandagatla Hi Alexandre, Thanks for the feedback. > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc > > On 06/12/2018 15:49:57+0000, Biju Das wrote: > > Hi Geert, > > > > Thanks for the feedback. > > > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP > > > pcf85263 rtc > > > > > > Hi Biju, > > > > > > On Thu, Dec 6, 2018 at 4:24 PM Biju Das <biju.das@bp.renesas.com> > wrote: > > > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP > > > > > pcf85263 rtc CC nvmem maintainer > > > > > > > > > > On Thu, Dec 6, 2018 at 10:04 AM Biju Das > > > > > <biju.das@bp.renesas.com> > > > wrote: > > > > > > Add support for NXP pcf85263 real-time clock. pcf85263 rtc is > > > > > > compatible with pcf85363,except that pcf85363 has additional > > > > > > 64 bytes of > > > > > RAM. > > > > > > > > > --- a/drivers/rtc/rtc-pcf85363.c > > > > > > +++ b/drivers/rtc/rtc-pcf85363.c > > > > > > @@ -120,6 +120,11 @@ struct pcf85363 { > > > > > > struct regmap *regmap; > > > > > > }; > > > > > > > > > > > > +struct pcf85x63_config { > > > > > > + struct regmap_config regmap; > > > > > > + unsigned int num_nvram; }; > > > > > > + > > > > > > static int pcf85363_rtc_read_time(struct device *dev, struct > > > > > > rtc_time > > > > > > *tm) { > > > > > > struct pcf85363 *pcf85363 = dev_get_drvdata(dev); @@ > > > > > > -311,25 > > > > > > +316,68 @@ static int pcf85363_nvram_write(void *priv, > > > > > > +unsigned int > > > > > offset, void *val, > > > > > > val, bytes); } > > > > > > > > > > > > -static const struct regmap_config regmap_config = { > > > > > > - .reg_bits = 8, > > > > > > - .val_bits = 8, > > > > > > - .max_register = 0x7f, > > > > > > +static int pcf85x63_nvram_read(void *priv, unsigned int > > > > > > +offset, void > > > *val, > > > > > > + size_t bytes) > > > > > > > > > > Given bytes should be 1, val should be a pointer to a single byte... > > > > > What if bytes == 0? > > > > > > > > I doubt we get "bytes==0" because of the checks in " > > > drivers/nvmem/core.c" > > > > Function " bin_attr_nvmem_read/ bin_attr_nvmem_write". > > > > > > Depends. There are other functions calling nvmem_reg_{read,write}(), > e.g. > > > nvmem_device_{read,write}(). > > > > OK. In that case, I will return (-EINVAL) for "bytes !=1" > > > > I think it is probably better to ensure the nvmem core never passes an invalid > number of bytes. All the ther RTC drivers make that assumption. In that case, I will do following checks in nvmem_device_{read,write}() before calling nvmem_reg_{read,write}(), nvmem_device_read /* Stop the user from reading */ if (offset >= nvmem->size) return 0; if (bytes == 0) return -EINVAL; if (offset + bytes > nvmem->size) bytes = nvmem->size - offset; nvmem_device_write /* Stop the user from writing */ if (offset >= nvmem->size) return -EFBIG; if (bytes == 0) return -EINVAL; if (offset + bytes > nvmem->size) bytes = nvmem->size - offset; regards, Biju [https://www2.renesas.eu/media/email/unicef.jpg] This Christmas, instead of sending out cards, Renesas Electronics Europe have decided to support Unicef with a donation. For further details click here<https://www.unicef.org/> to find out about the valuable work they do, helping children all over the world. We would like to take this opportunity to wish you a Merry Christmas and a prosperous New Year. Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc 2018-12-07 9:34 ` Biju Das @ 2018-12-07 9:45 ` Geert Uytterhoeven 2018-12-07 10:16 ` Biju Das 0 siblings, 1 reply; 22+ messages in thread From: Geert Uytterhoeven @ 2018-12-07 9:45 UTC (permalink / raw) To: Biju Das Cc: Alexandre Belloni, Alessandro Zummo, Geert Uytterhoeven, linux-rtc, Simon Horman, Chris Paterson, Fabrizio Castro, Linux-Renesas, Srinivas Kandagatla Hi Biju, On Fri, Dec 7, 2018 at 10:34 AM Biju Das <biju.das@bp.renesas.com> wrote: > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc > > > > On 06/12/2018 15:49:57+0000, Biju Das wrote: > > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP > > > > pcf85263 rtc > > > > On Thu, Dec 6, 2018 at 4:24 PM Biju Das <biju.das@bp.renesas.com> > > wrote: > > > > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP > > > > > > pcf85263 rtc CC nvmem maintainer > > > > > > Given bytes should be 1, val should be a pointer to a single byte... > > > > > > What if bytes == 0? > > > > > > > > > > I doubt we get "bytes==0" because of the checks in " > > > > drivers/nvmem/core.c" > > > > > Function " bin_attr_nvmem_read/ bin_attr_nvmem_write". > > > > > > > > Depends. There are other functions calling nvmem_reg_{read,write}(), > > e.g. > > > > nvmem_device_{read,write}(). > > > > > > OK. In that case, I will return (-EINVAL) for "bytes !=1" > > > > I think it is probably better to ensure the nvmem core never passes an invalid > > number of bytes. All the ther RTC drivers make that assumption. > > In that case, I will do following checks in nvmem_device_{read,write}() before calling nvmem_reg_{read,write}(), > > nvmem_device_read > > /* Stop the user from reading */ > if (offset >= nvmem->size) > return 0; > > if (bytes == 0) > return -EINVAL; Why not 0? > > if (offset + bytes > nvmem->size) This might overflow, please use check_add_overflow(). > bytes = nvmem->size - offset; > > nvmem_device_write > > /* Stop the user from writing */ > if (offset >= nvmem->size) > return -EFBIG; ENOSPC? + same comments as for read. Oh, and offset is unsigned int instead of loff_t. Nobody's envisioning nvmem devices larger than 4 GiB? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc 2018-12-07 9:45 ` Geert Uytterhoeven @ 2018-12-07 10:16 ` Biju Das 2018-12-07 10:18 ` Geert Uytterhoeven 0 siblings, 1 reply; 22+ messages in thread From: Biju Das @ 2018-12-07 10:16 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Alexandre Belloni, Alessandro Zummo, Geert Uytterhoeven, linux-rtc, Simon Horman, Chris Paterson, Fabrizio Castro, Linux-Renesas, Srinivas Kandagatla Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc > > Hi Biju, > > On Fri, Dec 7, 2018 at 10:34 AM Biju Das <biju.das@bp.renesas.com> wrote: > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP > > > pcf85263 rtc > > > > > > On 06/12/2018 15:49:57+0000, Biju Das wrote: > > > > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP > > > > > pcf85263 rtc > > > > > > On Thu, Dec 6, 2018 at 4:24 PM Biju Das > > > > > <biju.das@bp.renesas.com> > > > wrote: > > > > > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for > > > > > > > NXP > > > > > > > pcf85263 rtc CC nvmem maintainer Given bytes should be 1, > > > > > > > val should be a pointer to a single byte... > > > > > > > What if bytes == 0? > > > > > > > > > > > > I doubt we get "bytes==0" because of the checks in " > > > > > drivers/nvmem/core.c" > > > > > > Function " bin_attr_nvmem_read/ bin_attr_nvmem_write". > > > > > > > > > > Depends. There are other functions calling > > > > > nvmem_reg_{read,write}(), > > > e.g. > > > > > nvmem_device_{read,write}(). > > > > > > > > OK. In that case, I will return (-EINVAL) for "bytes !=1" > > > > > > I think it is probably better to ensure the nvmem core never passes > > > an invalid number of bytes. All the ther RTC drivers make that > assumption. > > > > In that case, I will do following checks in > > nvmem_device_{read,write}() before calling nvmem_reg_{read,write}(), > > > > nvmem_device_read > > > > /* Stop the user from reading */ > > if (offset >= nvmem->size) > > return 0; > > > > if (bytes == 0) > > return -EINVAL; > > Why not 0? Ok. Will merge with above check. if ((offset >= nvmem->size) && (bytes == 0)) return 0; > > > > if (offset + bytes > nvmem->size) > > This might overflow, please use check_add_overflow(). Will use check_add_overflow() and then the result is compared with nvmem->size, if the check operation is successful. > > bytes = nvmem->size - offset; > > > > nvmem_device_write > > > > /* Stop the user from writing */ > > if (offset >= nvmem->size) > > return -EFBIG; > > ENOSPC? OK, Will change it to ENOSPC. > + same comments as for read. > > Oh, and offset is unsigned int instead of loff_t. > Nobody's envisioning nvmem devices larger than 4 GiB? Regards, Biju [https://www2.renesas.eu/media/email/unicef.jpg] This Christmas, instead of sending out cards, Renesas Electronics Europe have decided to support Unicef with a donation. For further details click here<https://www.unicef.org/> to find out about the valuable work they do, helping children all over the world. We would like to take this opportunity to wish you a Merry Christmas and a prosperous New Year. Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc 2018-12-07 10:16 ` Biju Das @ 2018-12-07 10:18 ` Geert Uytterhoeven 0 siblings, 0 replies; 22+ messages in thread From: Geert Uytterhoeven @ 2018-12-07 10:18 UTC (permalink / raw) To: Biju Das Cc: Alexandre Belloni, Alessandro Zummo, Geert Uytterhoeven, linux-rtc, Simon Horman, Chris Paterson, Fabrizio Castro, Linux-Renesas, Srinivas Kandagatla Hi Biju, On Fri, Dec 7, 2018 at 11:16 AM Biju Das <biju.das@bp.renesas.com> wrote: > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc > > On Fri, Dec 7, 2018 at 10:34 AM Biju Das <biju.das@bp.renesas.com> wrote: > > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP > > > > pcf85263 rtc > > > > I think it is probably better to ensure the nvmem core never passes > > > > an invalid number of bytes. All the ther RTC drivers make that > > assumption. > > > > > > In that case, I will do following checks in > > > nvmem_device_{read,write}() before calling nvmem_reg_{read,write}(), > > > > > > nvmem_device_read > > > > > > /* Stop the user from reading */ > > > if (offset >= nvmem->size) > > > return 0; > > > > > > if (bytes == 0) > > > return -EINVAL; > > > > Why not 0? > > Ok. Will merge with above check. > > if ((offset >= nvmem->size) && (bytes == 0)) "||" ;-) > return 0; Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 3/4] ARM: shmobile: Enable NXP pcf85363 rtc in shmobile_defconfig 2018-12-06 8:55 [PATCH v3 0/4] Add NXP pcf85263 real-time clock support Biju Das 2018-12-06 8:55 ` [PATCH v3 1/4] dt-bindings: rtc: pcf85363: Document pcf85263 real-time clock Biju Das 2018-12-06 8:55 ` [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc Biju Das @ 2018-12-06 8:55 ` Biju Das 2018-12-06 9:41 ` Geert Uytterhoeven 2018-12-06 8:55 ` [PATCH v3 4/4] ARM: dts: iwg23s-sbc: Enable RTC Biju Das 3 siblings, 1 reply; 22+ messages in thread From: Biju Das @ 2018-12-06 8:55 UTC (permalink / raw) To: Simon Horman Cc: Biju Das, Magnus Damm, Russell King, Alexandre Belloni, linux-rtc, linux-renesas-soc, linux-arm-kernel, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro The iWave RZ/G1C SBC supports RTC (NXP pcf85263). To increase hardware support enable the driver in the shmobile_defconfig multiplatform configuration. Signed-off-by: Biju Das <biju.das@bp.renesas.com> --- V1-->V2 * No change. V2-->V3 * No change. --- arch/arm/configs/shmobile_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/shmobile_defconfig b/arch/arm/configs/shmobile_defconfig index 9e5a5ad..fdac4e4 100644 --- a/arch/arm/configs/shmobile_defconfig +++ b/arch/arm/configs/shmobile_defconfig @@ -177,6 +177,7 @@ CONFIG_LEDS_CLASS=y CONFIG_LEDS_GPIO=y CONFIG_RTC_CLASS=y CONFIG_RTC_DRV_RS5C372=y +CONFIG_RTC_DRV_PCF85363=y CONFIG_RTC_DRV_BQ32K=y CONFIG_RTC_DRV_S35390A=y CONFIG_RTC_DRV_RX8581=y -- 2.7.4 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v3 3/4] ARM: shmobile: Enable NXP pcf85363 rtc in shmobile_defconfig 2018-12-06 8:55 ` [PATCH v3 3/4] ARM: shmobile: Enable NXP pcf85363 rtc in shmobile_defconfig Biju Das @ 2018-12-06 9:41 ` Geert Uytterhoeven 0 siblings, 0 replies; 22+ messages in thread From: Geert Uytterhoeven @ 2018-12-06 9:41 UTC (permalink / raw) To: Biju Das Cc: Simon Horman, Magnus Damm, Russell King, Alexandre Belloni, linux-rtc, Linux-Renesas, Linux ARM, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro On Thu, Dec 6, 2018 at 10:04 AM Biju Das <biju.das@bp.renesas.com> wrote: > > The iWave RZ/G1C SBC supports RTC (NXP pcf85263). To increase hardware > support enable the driver in the shmobile_defconfig multiplatform > configuration. > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 4/4] ARM: dts: iwg23s-sbc: Enable RTC 2018-12-06 8:55 [PATCH v3 0/4] Add NXP pcf85263 real-time clock support Biju Das ` (2 preceding siblings ...) 2018-12-06 8:55 ` [PATCH v3 3/4] ARM: shmobile: Enable NXP pcf85363 rtc in shmobile_defconfig Biju Das @ 2018-12-06 8:55 ` Biju Das 2018-12-06 9:55 ` Geert Uytterhoeven 3 siblings, 1 reply; 22+ messages in thread From: Biju Das @ 2018-12-06 8:55 UTC (permalink / raw) To: Rob Herring, Mark Rutland Cc: Biju Das, Simon Horman, Magnus Damm, Alexandre Belloni, linux-rtc, linux-renesas-soc, devicetree, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro Enable NXP pcf85263 real time clock for the iWave SBC based on RZ/G1C. Signed-off-by: Biju Das <biju.das@bp.renesas.com> --- V1-->V2 * No change V2-->V3 * No change --- arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts b/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts index 40b7f98..77d1824 100644 --- a/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts +++ b/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts @@ -84,12 +84,30 @@ clock-frequency = <20000000>; }; +&i2c3 { + pinctrl-0 = <&i2c3_pins>; + pinctrl-names = "default"; + + status = "okay"; + clock-frequency = <400000>; + + rtc@51 { + compatible = "nxp,pcf85263"; + reg = <0x51>; + }; +}; + &pfc { avb_pins: avb { groups = "avb_mdio", "avb_gmii_tx_rx"; function = "avb"; }; + i2c3_pins: i2c3 { + groups = "i2c3_c"; + function = "i2c3"; + }; + mmc_pins_uhs: mmc_uhs { groups = "mmc_data8", "mmc_ctrl"; function = "mmc"; -- 2.7.4 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v3 4/4] ARM: dts: iwg23s-sbc: Enable RTC 2018-12-06 8:55 ` [PATCH v3 4/4] ARM: dts: iwg23s-sbc: Enable RTC Biju Das @ 2018-12-06 9:55 ` Geert Uytterhoeven 2018-12-06 12:40 ` Biju Das 0 siblings, 1 reply; 22+ messages in thread From: Geert Uytterhoeven @ 2018-12-06 9:55 UTC (permalink / raw) To: Biju Das Cc: Rob Herring, Mark Rutland, Simon Horman, Magnus Damm, Alexandre Belloni, linux-rtc, Linux-Renesas, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro Hi Biju, On Thu, Dec 6, 2018 at 10:04 AM Biju Das <biju.das@bp.renesas.com> wrote: > Enable NXP pcf85263 real time clock for the iWave SBC based on RZ/G1C. > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- a/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts > +++ b/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts > @@ -84,12 +84,30 @@ > clock-frequency = <20000000>; > }; > > +&i2c3 { > + pinctrl-0 = <&i2c3_pins>; > + pinctrl-names = "default"; > + > + status = "okay"; > + clock-frequency = <400000>; > + > + rtc@51 { > + compatible = "nxp,pcf85263"; > + reg = <0x51>; You might want to enable the optional interrupt: interrupt-parent = <&gpio0>; interrupts = <2 IRQ_TYPE_LEVEL_LOW>; > + }; Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH v3 4/4] ARM: dts: iwg23s-sbc: Enable RTC 2018-12-06 9:55 ` Geert Uytterhoeven @ 2018-12-06 12:40 ` Biju Das 2018-12-06 12:59 ` Geert Uytterhoeven 0 siblings, 1 reply; 22+ messages in thread From: Biju Das @ 2018-12-06 12:40 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Rob Herring, Mark Rutland, Simon Horman, Magnus Damm, Alexandre Belloni, linux-rtc, Linux-Renesas, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH v3 4/4] ARM: dts: iwg23s-sbc: Enable RTC > > Hi Biju, > > On Thu, Dec 6, 2018 at 10:04 AM Biju Das <biju.das@bp.renesas.com> wrote: > > Enable NXP pcf85263 real time clock for the iWave SBC based on RZ/G1C. > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > --- a/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts > > +++ b/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts > > @@ -84,12 +84,30 @@ > > clock-frequency = <20000000>; > > }; > > > > +&i2c3 { > > + pinctrl-0 = <&i2c3_pins>; > > + pinctrl-names = "default"; > > + > > + status = "okay"; > > + clock-frequency = <400000>; > > + > > + rtc@51 { > > + compatible = "nxp,pcf85263"; > > + reg = <0x51>; > > You might want to enable the optional interrupt: I have enabled this but unfortunately it is generating 100000 of gpio interrupts during boot. The reason is, by default this pin is configured as function(Power on reset/at u-boot). Currently there is no function available in kernel to convert a pin from function to gpio (Similar to the issue Fab is facing for display hot plug interrupt) May be we can add optional interrupt at a later stage, once we have a solution for converting pin from function to gpio. Please share your opinion on this. Regards, Biju [https://www2.renesas.eu/media/email/unicef.jpg] This Christmas, instead of sending out cards, Renesas Electronics Europe have decided to support Unicef with a donation. For further details click here<https://www.unicef.org/> to find out about the valuable work they do, helping children all over the world. We would like to take this opportunity to wish you a Merry Christmas and a prosperous New Year. Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 4/4] ARM: dts: iwg23s-sbc: Enable RTC 2018-12-06 12:40 ` Biju Das @ 2018-12-06 12:59 ` Geert Uytterhoeven 2018-12-10 11:52 ` Simon Horman 0 siblings, 1 reply; 22+ messages in thread From: Geert Uytterhoeven @ 2018-12-06 12:59 UTC (permalink / raw) To: Biju Das Cc: Rob Herring, Mark Rutland, Simon Horman, Magnus Damm, Alexandre Belloni, linux-rtc, Linux-Renesas, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro Hi Biju, On Thu, Dec 6, 2018 at 1:41 PM Biju Das <biju.das@bp.renesas.com> wrote: > > Subject: Re: [PATCH v3 4/4] ARM: dts: iwg23s-sbc: Enable RTC > > On Thu, Dec 6, 2018 at 10:04 AM Biju Das <biju.das@bp.renesas.com> wrote: > > > Enable NXP pcf85263 real time clock for the iWave SBC based on RZ/G1C. > > > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > > --- a/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts > > > +++ b/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts > > > @@ -84,12 +84,30 @@ > > > clock-frequency = <20000000>; > > > }; > > > > > > +&i2c3 { > > > + pinctrl-0 = <&i2c3_pins>; > > > + pinctrl-names = "default"; > > > + > > > + status = "okay"; > > > + clock-frequency = <400000>; > > > + > > > + rtc@51 { > > > + compatible = "nxp,pcf85263"; > > > + reg = <0x51>; > > > > You might want to enable the optional interrupt: > > I have enabled this but unfortunately it is generating 100000 of gpio interrupts during boot. Oh, the DT bindings claim interrupt support hasn't been implement yet ;-) > The reason is, by default this pin is configured as function(Power on reset/at u-boot). > Currently there is no function available in kernel to convert a pin from function to gpio (Similar to the issue Fab is facing for display hot plug interrupt) > > May be we can add optional interrupt at a later stage, once we have a solution for converting pin from function to gpio. > > Please share your opinion on this. IC. In that case, please postpone describing the interrupt until the issue is fixed. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 4/4] ARM: dts: iwg23s-sbc: Enable RTC 2018-12-06 12:59 ` Geert Uytterhoeven @ 2018-12-10 11:52 ` Simon Horman 2018-12-10 11:56 ` Geert Uytterhoeven 0 siblings, 1 reply; 22+ messages in thread From: Simon Horman @ 2018-12-10 11:52 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Biju Das, Rob Herring, Mark Rutland, Magnus Damm, Alexandre Belloni, linux-rtc, Linux-Renesas, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro On Thu, Dec 06, 2018 at 01:59:58PM +0100, Geert Uytterhoeven wrote: > Hi Biju, > > On Thu, Dec 6, 2018 at 1:41 PM Biju Das <biju.das@bp.renesas.com> wrote: > > > Subject: Re: [PATCH v3 4/4] ARM: dts: iwg23s-sbc: Enable RTC > > > > On Thu, Dec 6, 2018 at 10:04 AM Biju Das <biju.das@bp.renesas.com> wrote: > > > > Enable NXP pcf85263 real time clock for the iWave SBC based on RZ/G1C. > > > > > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > > > > --- a/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts > > > > +++ b/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts > > > > @@ -84,12 +84,30 @@ > > > > clock-frequency = <20000000>; > > > > }; > > > > > > > > +&i2c3 { > > > > + pinctrl-0 = <&i2c3_pins>; > > > > + pinctrl-names = "default"; > > > > + > > > > + status = "okay"; > > > > + clock-frequency = <400000>; > > > > + > > > > + rtc@51 { > > > > + compatible = "nxp,pcf85263"; > > > > + reg = <0x51>; > > > > > > You might want to enable the optional interrupt: > > > > I have enabled this but unfortunately it is generating 100000 of gpio interrupts during boot. > > Oh, the DT bindings claim interrupt support hasn't been implement yet ;-) > > > The reason is, by default this pin is configured as function(Power on reset/at u-boot). > > Currently there is no function available in kernel to convert a pin from function to gpio (Similar to the issue Fab is facing for display hot plug interrupt) > > > > May be we can add optional interrupt at a later stage, once we have a solution for converting pin from function to gpio. > > > > Please share your opinion on this. > > IC. In that case, please postpone describing the interrupt until the issue is > fixed. It feels like this patch is ready to be accepted. Geert, do you concur? ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 4/4] ARM: dts: iwg23s-sbc: Enable RTC 2018-12-10 11:52 ` Simon Horman @ 2018-12-10 11:56 ` Geert Uytterhoeven 2018-12-10 12:15 ` Simon Horman 0 siblings, 1 reply; 22+ messages in thread From: Geert Uytterhoeven @ 2018-12-10 11:56 UTC (permalink / raw) To: Simon Horman Cc: Biju Das, Rob Herring, Mark Rutland, Magnus Damm, Alexandre Belloni, linux-rtc, Linux-Renesas, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro Hi Simon, On Mon, Dec 10, 2018 at 12:52 PM Simon Horman <horms@verge.net.au> wrote: > On Thu, Dec 06, 2018 at 01:59:58PM +0100, Geert Uytterhoeven wrote: > > On Thu, Dec 6, 2018 at 1:41 PM Biju Das <biju.das@bp.renesas.com> wrote: > > > > Subject: Re: [PATCH v3 4/4] ARM: dts: iwg23s-sbc: Enable RTC > > > > > > On Thu, Dec 6, 2018 at 10:04 AM Biju Das <biju.das@bp.renesas.com> wrote: > > > > > Enable NXP pcf85263 real time clock for the iWave SBC based on RZ/G1C. > > > > > > > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > > > > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > > > > > > --- a/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts > > > > > +++ b/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts > > > > > @@ -84,12 +84,30 @@ > > > > > clock-frequency = <20000000>; > > > > > }; > > > > > > > > > > +&i2c3 { > > > > > + pinctrl-0 = <&i2c3_pins>; > > > > > + pinctrl-names = "default"; > > > > > + > > > > > + status = "okay"; > > > > > + clock-frequency = <400000>; > > > > > + > > > > > + rtc@51 { > > > > > + compatible = "nxp,pcf85263"; > > > > > + reg = <0x51>; > > > > > > > > You might want to enable the optional interrupt: > > > > > > I have enabled this but unfortunately it is generating 100000 of gpio interrupts during boot. > > > > Oh, the DT bindings claim interrupt support hasn't been implement yet ;-) > > > > > The reason is, by default this pin is configured as function(Power on reset/at u-boot). > > > Currently there is no function available in kernel to convert a pin from function to gpio (Similar to the issue Fab is facing for display hot plug interrupt) > > > > > > May be we can add optional interrupt at a later stage, once we have a solution for converting pin from function to gpio. > > > > > > Please share your opinion on this. > > > > IC. In that case, please postpone describing the interrupt until the issue is > > fixed. > > It feels like this patch is ready to be accepted. > Geert, do you concur? Yes, I agree. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 4/4] ARM: dts: iwg23s-sbc: Enable RTC 2018-12-10 11:56 ` Geert Uytterhoeven @ 2018-12-10 12:15 ` Simon Horman 0 siblings, 0 replies; 22+ messages in thread From: Simon Horman @ 2018-12-10 12:15 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Biju Das, Rob Herring, Mark Rutland, Magnus Damm, Alexandre Belloni, linux-rtc, Linux-Renesas, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro On Mon, Dec 10, 2018 at 12:56:11PM +0100, Geert Uytterhoeven wrote: > Hi Simon, > > On Mon, Dec 10, 2018 at 12:52 PM Simon Horman <horms@verge.net.au> wrote: > > On Thu, Dec 06, 2018 at 01:59:58PM +0100, Geert Uytterhoeven wrote: > > > On Thu, Dec 6, 2018 at 1:41 PM Biju Das <biju.das@bp.renesas.com> wrote: > > > > > Subject: Re: [PATCH v3 4/4] ARM: dts: iwg23s-sbc: Enable RTC > > > > > > > > On Thu, Dec 6, 2018 at 10:04 AM Biju Das <biju.das@bp.renesas.com> wrote: > > > > > > Enable NXP pcf85263 real time clock for the iWave SBC based on RZ/G1C. > > > > > > > > > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > > > > > > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > > > > > > > > --- a/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts > > > > > > +++ b/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts > > > > > > @@ -84,12 +84,30 @@ > > > > > > clock-frequency = <20000000>; > > > > > > }; > > > > > > > > > > > > +&i2c3 { > > > > > > + pinctrl-0 = <&i2c3_pins>; > > > > > > + pinctrl-names = "default"; > > > > > > + > > > > > > + status = "okay"; > > > > > > + clock-frequency = <400000>; > > > > > > + > > > > > > + rtc@51 { > > > > > > + compatible = "nxp,pcf85263"; > > > > > > + reg = <0x51>; > > > > > > > > > > You might want to enable the optional interrupt: > > > > > > > > I have enabled this but unfortunately it is generating 100000 of gpio interrupts during boot. > > > > > > Oh, the DT bindings claim interrupt support hasn't been implement yet ;-) > > > > > > > The reason is, by default this pin is configured as function(Power on reset/at u-boot). > > > > Currently there is no function available in kernel to convert a pin from function to gpio (Similar to the issue Fab is facing for display hot plug interrupt) > > > > > > > > May be we can add optional interrupt at a later stage, once we have a solution for converting pin from function to gpio. > > > > > > > > Please share your opinion on this. > > > > > > IC. In that case, please postpone describing the interrupt until the issue is > > > fixed. > > > > It feels like this patch is ready to be accepted. > > Geert, do you concur? > > Yes, I agree. Thanks, I have applied v4 (which is the same as v3). ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2018-12-10 12:16 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-06 8:55 [PATCH v3 0/4] Add NXP pcf85263 real-time clock support Biju Das 2018-12-06 8:55 ` [PATCH v3 1/4] dt-bindings: rtc: pcf85363: Document pcf85263 real-time clock Biju Das 2018-12-06 9:13 ` Geert Uytterhoeven 2018-12-06 8:55 ` [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc Biju Das 2018-12-06 9:39 ` Geert Uytterhoeven 2018-12-06 15:24 ` Biju Das 2018-12-06 15:37 ` Geert Uytterhoeven 2018-12-06 15:49 ` Biju Das 2018-12-06 16:52 ` Alexandre Belloni 2018-12-07 9:34 ` Biju Das 2018-12-07 9:45 ` Geert Uytterhoeven 2018-12-07 10:16 ` Biju Das 2018-12-07 10:18 ` Geert Uytterhoeven 2018-12-06 8:55 ` [PATCH v3 3/4] ARM: shmobile: Enable NXP pcf85363 rtc in shmobile_defconfig Biju Das 2018-12-06 9:41 ` Geert Uytterhoeven 2018-12-06 8:55 ` [PATCH v3 4/4] ARM: dts: iwg23s-sbc: Enable RTC Biju Das 2018-12-06 9:55 ` Geert Uytterhoeven 2018-12-06 12:40 ` Biju Das 2018-12-06 12:59 ` Geert Uytterhoeven 2018-12-10 11:52 ` Simon Horman 2018-12-10 11:56 ` Geert Uytterhoeven 2018-12-10 12:15 ` Simon Horman
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 NNTP newsgroup(s).