2018-07-18 09:41:16

by Giulio Benetti

[permalink] [raw]
Subject: [PATCH v9 2/4] rtc: ds1307: support m41t11 variant

The m41t11 variant is very similar to the already supported m41t00 and
m41t0, but it has also 56 bytes of NVRAM.

Add it to driver taking into account NVRAM section.

Signed-off-by: Giulio Benetti <[email protected]>
---
.../devicetree/bindings/rtc/rtc-ds1307.txt | 1 +
drivers/rtc/rtc-ds1307.c | 14 ++++++++++++++
2 files changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
index d28d6e7f6ae8..ce6469c1a516 100644
--- a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
@@ -13,6 +13,7 @@ Required properties:
"maxim,ds3231",
"st,m41t0",
"st,m41t00",
+ "st,m41t11",
"microchip,mcp7940x",
"microchip,mcp7941x",
"pericom,pt7c4338",
diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index aa4a161bae98..0162a600ff1b 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -44,6 +44,7 @@ enum ds_type {
ds_3231,
m41t0,
m41t00,
+ m41t11,
mcp794xx,
rx_8025,
rx_8130,
@@ -227,6 +228,11 @@ static const struct chip_desc chips[last_ds_type] = {
.irq_handler = rx8130_irq,
.rtc_ops = &rx8130_rtc_ops,
},
+ [m41t11] = {
+ /* this is battery backed SRAM */
+ .nvram_offset = 8,
+ .nvram_size = 56,
+ },
[mcp794xx] = {
.alarm = 1,
/* this is battery backed SRAM */
@@ -249,6 +255,7 @@ static const struct i2c_device_id ds1307_id[] = {
{ "ds3231", ds_3231 },
{ "m41t0", m41t0 },
{ "m41t00", m41t00 },
+ { "m41t11", m41t11 },
{ "mcp7940x", mcp794xx },
{ "mcp7941x", mcp794xx },
{ "pt7c4338", ds_1307 },
@@ -305,6 +312,10 @@ static const struct of_device_id ds1307_of_match[] = {
.compatible = "st,m41t00",
.data = (void *)m41t00
},
+ {
+ .compatible = "st,m41t11",
+ .data = (void *)m41t11
+ },
{
.compatible = "microchip,mcp7940x",
.data = (void *)mcp794xx
@@ -347,6 +358,7 @@ static const struct acpi_device_id ds1307_acpi_ids[] = {
{ .id = "DS3231", .driver_data = ds_3231 },
{ .id = "M41T0", .driver_data = m41t0 },
{ .id = "M41T00", .driver_data = m41t00 },
+ { .id = "M41T11", .driver_data = m41t11 },
{ .id = "MCP7940X", .driver_data = mcp794xx },
{ .id = "MCP7941X", .driver_data = mcp794xx },
{ .id = "PT7C4338", .driver_data = ds_1307 },
@@ -1576,6 +1588,7 @@ static int ds1307_probe(struct i2c_client *client,
case ds_1307:
case m41t0:
case m41t00:
+ case m41t11:
/* clock halted? turn it on, so clock can tick. */
if (tmp & DS1307_BIT_CH) {
regmap_write(ds1307->regmap, DS1307_REG_SECS, 0);
@@ -1641,6 +1654,7 @@ static int ds1307_probe(struct i2c_client *client,
case ds_1340:
case m41t0:
case m41t00:
+ case m41t11:
/*
* NOTE: ignores century bits; fix before deploying
* systems that will run through year 2100.
--
2.17.1



2018-07-18 09:41:16

by Giulio Benetti

[permalink] [raw]
Subject: [PATCH v9 4/4] rtc: ds1307: add frequency_test_enable sysfs attribute to check tick on m41txx

On m41txx you can enable open-drain OUT pin to check if offset is ok.
Enabling OUT pin with frequency_test_enable attribute, OUT pin will tick
512 times faster than 1s tick base.

Enable or Disable FT bit on CONTROL register if freq_test is 1 or 0.

Signed-off-by: Giulio Benetti <[email protected]>
---
drivers/rtc/rtc-ds1307.c | 96 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 96 insertions(+)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index c6d871456f25..3bccb742c983 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1050,6 +1050,96 @@ static int m41txx_rtc_set_offset(struct device *dev, long offset)
ctrl_reg);
}

+static ssize_t frequency_test_enable_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ds1307 *ds1307 = dev_get_drvdata(dev);
+ bool freq_test_en = 0;
+ int ret;
+
+ ret = kstrtobool(buf, &freq_test_en);
+ if (ret == -EINVAL) {
+ dev_err(dev, "Failed to store RTC Frequency Test attribute\n");
+ return ret;
+ }
+
+ regmap_update_bits(ds1307->regmap, M41TXX_REG_CONTROL, M41TXX_BIT_FT,
+ freq_test_en ? M41TXX_BIT_FT : 0);
+
+ return count;
+}
+
+static ssize_t frequency_test_enable_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ds1307 *ds1307 = dev_get_drvdata(dev);
+ unsigned int ctrl_reg;
+
+ regmap_read(ds1307->regmap, M41TXX_REG_CONTROL, &ctrl_reg);
+
+ if (ctrl_reg & M41TXX_BIT_FT)
+ return scnprintf(buf, PAGE_SIZE, "on\n");
+ else
+ return scnprintf(buf, PAGE_SIZE, "off\n");
+}
+
+static DEVICE_ATTR_RW(frequency_test_enable);
+
+static struct attribute *rtc_freq_test_attrs[] = {
+ &dev_attr_frequency_test_enable.attr,
+ NULL,
+};
+
+static const struct attribute_group rtc_freq_test_attr_group = {
+ .attrs = rtc_freq_test_attrs,
+};
+
+static void rtc_calib_remove_sysfs_group(void *_dev)
+{
+ struct device *dev = _dev;
+
+ sysfs_remove_group(&dev->kobj, &rtc_freq_test_attr_group);
+}
+
+static int ds1307_add_frequency_test(struct ds1307 *ds1307)
+{
+ int err = 0;
+
+ switch (ds1307->type) {
+ case m41t0:
+ case m41t00:
+ case m41t11:
+ /* Export sysfs entries */
+ err = sysfs_create_group(&(ds1307->dev)->kobj,
+ &rtc_freq_test_attr_group);
+ if (err) {
+ dev_err(ds1307->dev,
+ "Failed to create sysfs group: %d\n",
+ err);
+ return err;
+ }
+
+ err = devm_add_action_or_reset(ds1307->dev,
+ rtc_calib_remove_sysfs_group,
+ ds1307->dev);
+ if (err) {
+ dev_err(ds1307->dev,
+ "Failed to add sysfs cleanup action: %d\n",
+ err);
+ sysfs_remove_group(&(ds1307->dev)->kobj,
+ &rtc_freq_test_attr_group);
+ return err;
+ }
+ break;
+ default:
+ break;
+ }
+
+ return err;
+}
+
/*----------------------------------------------------------------------*/

static int ds1307_nvram_read(void *priv, unsigned int offset, void *val,
@@ -1792,6 +1882,12 @@ static int ds1307_probe(struct i2c_client *client,
if (err)
return err;

+ err = ds1307_add_frequency_test(ds1307);
+ if (err) {
+ rtc_device_unregister(ds1307->rtc);
+ return err;
+ }
+
if (chip->nvram_size) {
struct nvmem_config nvmem_cfg = {
.name = "ds1307_nvram",
--
2.17.1


2018-07-18 09:41:16

by Giulio Benetti

[permalink] [raw]
Subject: [PATCH v9 3/4] rtc: ds1307: add offset sysfs for mt41txx chips.

m41txx chips can hold a calibration value to get correct clock bias.

Add offset handling (ranging between -63ppm and 126ppm) via sysfs.

Signed-off-by: Giulio Benetti <[email protected]>
---
V8 => V9:
* fix GENMASK() to get 0x1f, it must be GENMASK(4, 0)
drivers/rtc/rtc-ds1307.c | 77 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 77 insertions(+)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 0162a600ff1b..c6d871456f25 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -114,6 +114,20 @@ enum ds_type {
# define RX8025_BIT_VDET 0x40
# define RX8025_BIT_XST 0x20

+#define M41TXX_REG_CONTROL 0x07
+# define M41TXX_BIT_OUT BIT(7)
+# define M41TXX_BIT_FT BIT(6)
+# define M41TXX_BIT_CALIB_SIGN BIT(5)
+# define M41TXX_M_CALIBRATION GENMASK(4, 0)
+
+/* negative offset step is -2.034ppm */
+#define M41TXX_NEG_OFFSET_STEP_PPB 2034
+/* positive offset step is +4.068ppm */
+#define M41TXX_POS_OFFSET_STEP_PPB 4068
+/* Min and max values supported with 'offset' interface by M41TXX */
+#define M41TXX_MIN_OFFSET ((-31) * M41TXX_NEG_OFFSET_STEP_PPB)
+#define M41TXX_MAX_OFFSET ((31) * M41TXX_POS_OFFSET_STEP_PPB)
+
struct ds1307 {
enum ds_type type;
unsigned long flags;
@@ -146,6 +160,9 @@ struct chip_desc {

static int ds1307_get_time(struct device *dev, struct rtc_time *t);
static int ds1307_set_time(struct device *dev, struct rtc_time *t);
+static int ds1337_read_alarm(struct device *dev, struct rtc_wkalrm *t);
+static int ds1337_set_alarm(struct device *dev, struct rtc_wkalrm *t);
+static int ds1307_alarm_irq_enable(struct device *dev, unsigned int enabled);
static u8 do_trickle_setup_ds1339(struct ds1307 *, u32 ohms, bool diode);
static irqreturn_t rx8130_irq(int irq, void *dev_id);
static int rx8130_read_alarm(struct device *dev, struct rtc_wkalrm *t);
@@ -155,6 +172,8 @@ static irqreturn_t mcp794xx_irq(int irq, void *dev_id);
static int mcp794xx_read_alarm(struct device *dev, struct rtc_wkalrm *t);
static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t);
static int mcp794xx_alarm_irq_enable(struct device *dev, unsigned int enabled);
+static int m41txx_rtc_read_offset(struct device *dev, long *offset);
+static int m41txx_rtc_set_offset(struct device *dev, long offset);

static const struct rtc_class_ops rx8130_rtc_ops = {
.read_time = ds1307_get_time,
@@ -172,6 +191,16 @@ static const struct rtc_class_ops mcp794xx_rtc_ops = {
.alarm_irq_enable = mcp794xx_alarm_irq_enable,
};

+static const struct rtc_class_ops m41txx_rtc_ops = {
+ .read_time = ds1307_get_time,
+ .set_time = ds1307_set_time,
+ .read_alarm = ds1337_read_alarm,
+ .set_alarm = ds1337_set_alarm,
+ .alarm_irq_enable = ds1307_alarm_irq_enable,
+ .read_offset = m41txx_rtc_read_offset,
+ .set_offset = m41txx_rtc_set_offset,
+};
+
static const struct chip_desc chips[last_ds_type] = {
[ds_1307] = {
.nvram_offset = 8,
@@ -228,10 +257,17 @@ static const struct chip_desc chips[last_ds_type] = {
.irq_handler = rx8130_irq,
.rtc_ops = &rx8130_rtc_ops,
},
+ [m41t0] = {
+ .rtc_ops = &m41txx_rtc_ops,
+ },
+ [m41t00] = {
+ .rtc_ops = &m41txx_rtc_ops,
+ },
[m41t11] = {
/* this is battery backed SRAM */
.nvram_offset = 8,
.nvram_size = 56,
+ .rtc_ops = &m41txx_rtc_ops,
},
[mcp794xx] = {
.alarm = 1,
@@ -973,6 +1009,47 @@ static int mcp794xx_alarm_irq_enable(struct device *dev, unsigned int enabled)
enabled ? MCP794XX_BIT_ALM0_EN : 0);
}

+static int m41txx_rtc_read_offset(struct device *dev, long *offset)
+{
+ struct ds1307 *ds1307 = dev_get_drvdata(dev);
+ unsigned int ctrl_reg;
+ u8 val;
+
+ regmap_read(ds1307->regmap, M41TXX_REG_CONTROL, &ctrl_reg);
+
+ val = ctrl_reg & M41TXX_M_CALIBRATION;
+
+ /* check if positive */
+ if (ctrl_reg & M41TXX_BIT_CALIB_SIGN)
+ *offset = (val * M41TXX_POS_OFFSET_STEP_PPB);
+ else
+ *offset = -(val * M41TXX_NEG_OFFSET_STEP_PPB);
+
+ return 0;
+}
+
+static int m41txx_rtc_set_offset(struct device *dev, long offset)
+{
+ struct ds1307 *ds1307 = dev_get_drvdata(dev);
+ unsigned int ctrl_reg;
+
+ if ((offset < M41TXX_MIN_OFFSET) || (offset > M41TXX_MAX_OFFSET))
+ return -ERANGE;
+
+ if (offset >= 0) {
+ ctrl_reg = DIV_ROUND_CLOSEST(offset,
+ M41TXX_POS_OFFSET_STEP_PPB);
+ ctrl_reg |= M41TXX_BIT_CALIB_SIGN;
+ } else {
+ ctrl_reg = DIV_ROUND_CLOSEST(abs(offset),
+ M41TXX_NEG_OFFSET_STEP_PPB);
+ }
+
+ return regmap_update_bits(ds1307->regmap, M41TXX_REG_CONTROL,
+ M41TXX_M_CALIBRATION | M41TXX_BIT_CALIB_SIGN,
+ ctrl_reg);
+}
+
/*----------------------------------------------------------------------*/

static int ds1307_nvram_read(void *priv, unsigned int offset, void *val,
--
2.17.1


2018-07-18 09:42:15

by Giulio Benetti

[permalink] [raw]
Subject: [PATCH v9 1/4] rtc: ds1307: fix data pointer to m41t0

data field points to m41t00, instead it should point to m41t0.
Driver works correctly because on both cases(m41t0 and m41t00) chip_desc
are equal.

Point to right enum m41t0 instead of m41t00.

Signed-off-by: Giulio Benetti <[email protected]>
---
drivers/rtc/rtc-ds1307.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index e9ec4160d7f6..aa4a161bae98 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -299,7 +299,7 @@ static const struct of_device_id ds1307_of_match[] = {
},
{
.compatible = "st,m41t0",
- .data = (void *)m41t00
+ .data = (void *)m41t0
},
{
.compatible = "st,m41t00",
--
2.17.1


2018-07-18 12:50:09

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v9 4/4] rtc: ds1307: add frequency_test_enable sysfs attribute to check tick on m41txx

On Wed, Jul 18, 2018 at 12:40 PM, Giulio Benetti
<[email protected]> wrote:
> On m41txx you can enable open-drain OUT pin to check if offset is ok.
> Enabling OUT pin with frequency_test_enable attribute, OUT pin will tick
> 512 times faster than 1s tick base.
>
> Enable or Disable FT bit on CONTROL register if freq_test is 1 or 0.

> +static ssize_t frequency_test_enable_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct ds1307 *ds1307 = dev_get_drvdata(dev);


> + bool freq_test_en = 0;

First of all, initialization of boolean using int is not very best style.
Second, this is not needed.

> + int ret;
> +
> + ret = kstrtobool(buf, &freq_test_en);

> + if (ret == -EINVAL) {

What kind of other error code you may expect and why they are not
considered as no error?

> + dev_err(dev, "Failed to store RTC Frequency Test attribute\n");
> + return ret;
> + }
> +
> + regmap_update_bits(ds1307->regmap, M41TXX_REG_CONTROL, M41TXX_BIT_FT,
> + freq_test_en ? M41TXX_BIT_FT : 0);
> +
> + return count;
> +}
> +
> +static ssize_t frequency_test_enable_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct ds1307 *ds1307 = dev_get_drvdata(dev);
> + unsigned int ctrl_reg;
> +
> + regmap_read(ds1307->regmap, M41TXX_REG_CONTROL, &ctrl_reg);
> +

> + if (ctrl_reg & M41TXX_BIT_FT)
> + return scnprintf(buf, PAGE_SIZE, "on\n");
> + else
> + return scnprintf(buf, PAGE_SIZE, "off\n");

Entire condition might be one line.

> +}
> +

> +static int ds1307_add_frequency_test(struct ds1307 *ds1307)
> +{

> + int err = 0;

Redundant assignment. See below.

> +
> + switch (ds1307->type) {
> + case m41t0:
> + case m41t00:
> + case m41t11:
> + /* Export sysfs entries */
> + err = sysfs_create_group(&(ds1307->dev)->kobj,
> + &rtc_freq_test_attr_group);
> + if (err) {
> + dev_err(ds1307->dev,
> + "Failed to create sysfs group: %d\n",
> + err);
> + return err;
> + }
> +
> + err = devm_add_action_or_reset(ds1307->dev,
> + rtc_calib_remove_sysfs_group,
> + ds1307->dev);
> + if (err) {
> + dev_err(ds1307->dev,
> + "Failed to add sysfs cleanup action: %d\n",
> + err);
> + sysfs_remove_group(&(ds1307->dev)->kobj,
> + &rtc_freq_test_attr_group);
> + return err;
> + }
> + break;
> + default:
> + break;
> + }
> +

> + return err;

return 0;

--
With Best Regards,
Andy Shevchenko

2018-07-18 14:07:34

by Giulio Benetti

[permalink] [raw]
Subject: Re: [PATCH v9 4/4] rtc: ds1307: add frequency_test_enable sysfs attribute to check tick on m41txx

Hi Andy,

Il 18/07/2018 14:47, Andy Shevchenko ha scritto:
> On Wed, Jul 18, 2018 at 12:40 PM, Giulio Benetti
> <[email protected]> wrote:
>> On m41txx you can enable open-drain OUT pin to check if offset is ok.
>> Enabling OUT pin with frequency_test_enable attribute, OUT pin will tick
>> 512 times faster than 1s tick base.
>>
>> Enable or Disable FT bit on CONTROL register if freq_test is 1 or 0.
>
>> +static ssize_t frequency_test_enable_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct ds1307 *ds1307 = dev_get_drvdata(dev);
>
>
>> + bool freq_test_en = 0;
>
> First of all, initialization of boolean using int is not very best style.
> Second, this is not needed.

Ok I remove initialization.

>
>> + int ret;
>> +
>> + ret = kstrtobool(buf, &freq_test_en);
>
>> + if (ret == -EINVAL) {
>
> What kind of other error code you may expect and why they are not
> considered as no error?

According to documentation, kstrtobool can only fail with -EINVAL.
See:
https://elixir.bootlin.com/linux/v4.18-rc5/source/lib/kstrtox.c#L332

>
>> + dev_err(dev, "Failed to store RTC Frequency Test attribute\n");
>> + return ret;
>> + }
>> +
>> + regmap_update_bits(ds1307->regmap, M41TXX_REG_CONTROL, M41TXX_BIT_FT,
>> + freq_test_en ? M41TXX_BIT_FT : 0);
>> +
>> + return count;
>> +}
>> +
>> +static ssize_t frequency_test_enable_show(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct ds1307 *ds1307 = dev_get_drvdata(dev);
>> + unsigned int ctrl_reg;
>> +
>> + regmap_read(ds1307->regmap, M41TXX_REG_CONTROL, &ctrl_reg);
>> +
>
>> + if (ctrl_reg & M41TXX_BIT_FT)
>> + return scnprintf(buf, PAGE_SIZE, "on\n");
>> + else
>> + return scnprintf(buf, PAGE_SIZE, "off\n");
>
> Entire condition might be one line.

Right, I change it.

>
>> +}
>> +
>
>> +static int ds1307_add_frequency_test(struct ds1307 *ds1307)
>> +{
>
>> + int err = 0;
>
> Redundant assignment. See below.

Ok.

>
>> +
>> + switch (ds1307->type) {
>> + case m41t0:
>> + case m41t00:
>> + case m41t11:
>> + /* Export sysfs entries */
>> + err = sysfs_create_group(&(ds1307->dev)->kobj,
>> + &rtc_freq_test_attr_group);
>> + if (err) {
>> + dev_err(ds1307->dev,
>> + "Failed to create sysfs group: %d\n",
>> + err);
>> + return err;
>> + }
>> +
>> + err = devm_add_action_or_reset(ds1307->dev,
>> + rtc_calib_remove_sysfs_group,
>> + ds1307->dev);
>> + if (err) {
>> + dev_err(ds1307->dev,
>> + "Failed to add sysfs cleanup action: %d\n",
>> + err);
>> + sysfs_remove_group(&(ds1307->dev)->kobj,
>> + &rtc_freq_test_attr_group);
>> + return err;
>> + }
>> + break;
>> + default:
>> + break;
>> + }
>> +
>
>> + return err;
>
> return 0;

Ok.

Thanks

Giulio

2018-07-18 14:10:50

by Giulio Benetti

[permalink] [raw]
Subject: [PATCH v10 3/4] rtc: ds1307: add offset sysfs for mt41txx chips.

m41txx chips can hold a calibration value to get correct clock bias.

Add offset handling (ranging between -63ppm and 126ppm) via sysfs.

Signed-off-by: Giulio Benetti <[email protected]>
---
drivers/rtc/rtc-ds1307.c | 77 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 77 insertions(+)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 0162a600ff1b..c6d871456f25 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -114,6 +114,20 @@ enum ds_type {
# define RX8025_BIT_VDET 0x40
# define RX8025_BIT_XST 0x20

+#define M41TXX_REG_CONTROL 0x07
+# define M41TXX_BIT_OUT BIT(7)
+# define M41TXX_BIT_FT BIT(6)
+# define M41TXX_BIT_CALIB_SIGN BIT(5)
+# define M41TXX_M_CALIBRATION GENMASK(4, 0)
+
+/* negative offset step is -2.034ppm */
+#define M41TXX_NEG_OFFSET_STEP_PPB 2034
+/* positive offset step is +4.068ppm */
+#define M41TXX_POS_OFFSET_STEP_PPB 4068
+/* Min and max values supported with 'offset' interface by M41TXX */
+#define M41TXX_MIN_OFFSET ((-31) * M41TXX_NEG_OFFSET_STEP_PPB)
+#define M41TXX_MAX_OFFSET ((31) * M41TXX_POS_OFFSET_STEP_PPB)
+
struct ds1307 {
enum ds_type type;
unsigned long flags;
@@ -146,6 +160,9 @@ struct chip_desc {

static int ds1307_get_time(struct device *dev, struct rtc_time *t);
static int ds1307_set_time(struct device *dev, struct rtc_time *t);
+static int ds1337_read_alarm(struct device *dev, struct rtc_wkalrm *t);
+static int ds1337_set_alarm(struct device *dev, struct rtc_wkalrm *t);
+static int ds1307_alarm_irq_enable(struct device *dev, unsigned int enabled);
static u8 do_trickle_setup_ds1339(struct ds1307 *, u32 ohms, bool diode);
static irqreturn_t rx8130_irq(int irq, void *dev_id);
static int rx8130_read_alarm(struct device *dev, struct rtc_wkalrm *t);
@@ -155,6 +172,8 @@ static irqreturn_t mcp794xx_irq(int irq, void *dev_id);
static int mcp794xx_read_alarm(struct device *dev, struct rtc_wkalrm *t);
static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t);
static int mcp794xx_alarm_irq_enable(struct device *dev, unsigned int enabled);
+static int m41txx_rtc_read_offset(struct device *dev, long *offset);
+static int m41txx_rtc_set_offset(struct device *dev, long offset);

static const struct rtc_class_ops rx8130_rtc_ops = {
.read_time = ds1307_get_time,
@@ -172,6 +191,16 @@ static const struct rtc_class_ops mcp794xx_rtc_ops = {
.alarm_irq_enable = mcp794xx_alarm_irq_enable,
};

+static const struct rtc_class_ops m41txx_rtc_ops = {
+ .read_time = ds1307_get_time,
+ .set_time = ds1307_set_time,
+ .read_alarm = ds1337_read_alarm,
+ .set_alarm = ds1337_set_alarm,
+ .alarm_irq_enable = ds1307_alarm_irq_enable,
+ .read_offset = m41txx_rtc_read_offset,
+ .set_offset = m41txx_rtc_set_offset,
+};
+
static const struct chip_desc chips[last_ds_type] = {
[ds_1307] = {
.nvram_offset = 8,
@@ -228,10 +257,17 @@ static const struct chip_desc chips[last_ds_type] = {
.irq_handler = rx8130_irq,
.rtc_ops = &rx8130_rtc_ops,
},
+ [m41t0] = {
+ .rtc_ops = &m41txx_rtc_ops,
+ },
+ [m41t00] = {
+ .rtc_ops = &m41txx_rtc_ops,
+ },
[m41t11] = {
/* this is battery backed SRAM */
.nvram_offset = 8,
.nvram_size = 56,
+ .rtc_ops = &m41txx_rtc_ops,
},
[mcp794xx] = {
.alarm = 1,
@@ -973,6 +1009,47 @@ static int mcp794xx_alarm_irq_enable(struct device *dev, unsigned int enabled)
enabled ? MCP794XX_BIT_ALM0_EN : 0);
}

+static int m41txx_rtc_read_offset(struct device *dev, long *offset)
+{
+ struct ds1307 *ds1307 = dev_get_drvdata(dev);
+ unsigned int ctrl_reg;
+ u8 val;
+
+ regmap_read(ds1307->regmap, M41TXX_REG_CONTROL, &ctrl_reg);
+
+ val = ctrl_reg & M41TXX_M_CALIBRATION;
+
+ /* check if positive */
+ if (ctrl_reg & M41TXX_BIT_CALIB_SIGN)
+ *offset = (val * M41TXX_POS_OFFSET_STEP_PPB);
+ else
+ *offset = -(val * M41TXX_NEG_OFFSET_STEP_PPB);
+
+ return 0;
+}
+
+static int m41txx_rtc_set_offset(struct device *dev, long offset)
+{
+ struct ds1307 *ds1307 = dev_get_drvdata(dev);
+ unsigned int ctrl_reg;
+
+ if ((offset < M41TXX_MIN_OFFSET) || (offset > M41TXX_MAX_OFFSET))
+ return -ERANGE;
+
+ if (offset >= 0) {
+ ctrl_reg = DIV_ROUND_CLOSEST(offset,
+ M41TXX_POS_OFFSET_STEP_PPB);
+ ctrl_reg |= M41TXX_BIT_CALIB_SIGN;
+ } else {
+ ctrl_reg = DIV_ROUND_CLOSEST(abs(offset),
+ M41TXX_NEG_OFFSET_STEP_PPB);
+ }
+
+ return regmap_update_bits(ds1307->regmap, M41TXX_REG_CONTROL,
+ M41TXX_M_CALIBRATION | M41TXX_BIT_CALIB_SIGN,
+ ctrl_reg);
+}
+
/*----------------------------------------------------------------------*/

static int ds1307_nvram_read(void *priv, unsigned int offset, void *val,
--
2.17.1


2018-07-18 14:10:51

by Giulio Benetti

[permalink] [raw]
Subject: [PATCH v10 1/4] rtc: ds1307: fix data pointer to m41t0

data field points to m41t00, instead it should point to m41t0.
Driver works correctly because on both cases(m41t0 and m41t00) chip_desc
are equal.

Point to right enum m41t0 instead of m41t00.

Signed-off-by: Giulio Benetti <[email protected]>
---
drivers/rtc/rtc-ds1307.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index e9ec4160d7f6..aa4a161bae98 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -299,7 +299,7 @@ static const struct of_device_id ds1307_of_match[] = {
},
{
.compatible = "st,m41t0",
- .data = (void *)m41t00
+ .data = (void *)m41t0
},
{
.compatible = "st,m41t00",
--
2.17.1


2018-07-18 14:11:35

by Giulio Benetti

[permalink] [raw]
Subject: [PATCH v10 2/4] rtc: ds1307: support m41t11 variant

The m41t11 variant is very similar to the already supported m41t00 and
m41t0, but it has also 56 bytes of NVRAM.

Add it to driver taking into account NVRAM section.

Signed-off-by: Giulio Benetti <[email protected]>
---
.../devicetree/bindings/rtc/rtc-ds1307.txt | 1 +
drivers/rtc/rtc-ds1307.c | 14 ++++++++++++++
2 files changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
index d28d6e7f6ae8..ce6469c1a516 100644
--- a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
@@ -13,6 +13,7 @@ Required properties:
"maxim,ds3231",
"st,m41t0",
"st,m41t00",
+ "st,m41t11",
"microchip,mcp7940x",
"microchip,mcp7941x",
"pericom,pt7c4338",
diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index aa4a161bae98..0162a600ff1b 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -44,6 +44,7 @@ enum ds_type {
ds_3231,
m41t0,
m41t00,
+ m41t11,
mcp794xx,
rx_8025,
rx_8130,
@@ -227,6 +228,11 @@ static const struct chip_desc chips[last_ds_type] = {
.irq_handler = rx8130_irq,
.rtc_ops = &rx8130_rtc_ops,
},
+ [m41t11] = {
+ /* this is battery backed SRAM */
+ .nvram_offset = 8,
+ .nvram_size = 56,
+ },
[mcp794xx] = {
.alarm = 1,
/* this is battery backed SRAM */
@@ -249,6 +255,7 @@ static const struct i2c_device_id ds1307_id[] = {
{ "ds3231", ds_3231 },
{ "m41t0", m41t0 },
{ "m41t00", m41t00 },
+ { "m41t11", m41t11 },
{ "mcp7940x", mcp794xx },
{ "mcp7941x", mcp794xx },
{ "pt7c4338", ds_1307 },
@@ -305,6 +312,10 @@ static const struct of_device_id ds1307_of_match[] = {
.compatible = "st,m41t00",
.data = (void *)m41t00
},
+ {
+ .compatible = "st,m41t11",
+ .data = (void *)m41t11
+ },
{
.compatible = "microchip,mcp7940x",
.data = (void *)mcp794xx
@@ -347,6 +358,7 @@ static const struct acpi_device_id ds1307_acpi_ids[] = {
{ .id = "DS3231", .driver_data = ds_3231 },
{ .id = "M41T0", .driver_data = m41t0 },
{ .id = "M41T00", .driver_data = m41t00 },
+ { .id = "M41T11", .driver_data = m41t11 },
{ .id = "MCP7940X", .driver_data = mcp794xx },
{ .id = "MCP7941X", .driver_data = mcp794xx },
{ .id = "PT7C4338", .driver_data = ds_1307 },
@@ -1576,6 +1588,7 @@ static int ds1307_probe(struct i2c_client *client,
case ds_1307:
case m41t0:
case m41t00:
+ case m41t11:
/* clock halted? turn it on, so clock can tick. */
if (tmp & DS1307_BIT_CH) {
regmap_write(ds1307->regmap, DS1307_REG_SECS, 0);
@@ -1641,6 +1654,7 @@ static int ds1307_probe(struct i2c_client *client,
case ds_1340:
case m41t0:
case m41t00:
+ case m41t11:
/*
* NOTE: ignores century bits; fix before deploying
* systems that will run through year 2100.
--
2.17.1


2018-07-18 14:11:39

by Giulio Benetti

[permalink] [raw]
Subject: [PATCH v10 4/4] rtc: ds1307: add frequency_test_enable sysfs attribute to check tick on m41txx

On m41txx you can enable open-drain OUT pin to check if offset is ok.
Enabling OUT pin with frequency_test_enable attribute, OUT pin will tick
512 times faster than 1s tick base.

Enable or Disable FT bit on CONTROL register if freq_test is 1 or 0.

Signed-off-by: Giulio Benetti <[email protected]>
---
drivers/rtc/rtc-ds1307.c | 94 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 94 insertions(+)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index c6d871456f25..2efe55f43774 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1050,6 +1050,94 @@ static int m41txx_rtc_set_offset(struct device *dev, long offset)
ctrl_reg);
}

+static ssize_t frequency_test_enable_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ds1307 *ds1307 = dev_get_drvdata(dev);
+ bool freq_test_en;
+ int ret;
+
+ ret = kstrtobool(buf, &freq_test_en);
+ if (ret == -EINVAL) {
+ dev_err(dev, "Failed to store RTC Frequency Test attribute\n");
+ return ret;
+ }
+
+ regmap_update_bits(ds1307->regmap, M41TXX_REG_CONTROL, M41TXX_BIT_FT,
+ freq_test_en ? M41TXX_BIT_FT : 0);
+
+ return count;
+}
+
+static ssize_t frequency_test_enable_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ds1307 *ds1307 = dev_get_drvdata(dev);
+ unsigned int ctrl_reg;
+
+ regmap_read(ds1307->regmap, M41TXX_REG_CONTROL, &ctrl_reg);
+
+ return scnprintf(buf, PAGE_SIZE, (ctrl_reg & M41TXX_BIT_FT) ? "on\n" :
+ "off\n");
+}
+
+static DEVICE_ATTR_RW(frequency_test_enable);
+
+static struct attribute *rtc_freq_test_attrs[] = {
+ &dev_attr_frequency_test_enable.attr,
+ NULL,
+};
+
+static const struct attribute_group rtc_freq_test_attr_group = {
+ .attrs = rtc_freq_test_attrs,
+};
+
+static void rtc_calib_remove_sysfs_group(void *_dev)
+{
+ struct device *dev = _dev;
+
+ sysfs_remove_group(&dev->kobj, &rtc_freq_test_attr_group);
+}
+
+static int ds1307_add_frequency_test(struct ds1307 *ds1307)
+{
+ int err;
+
+ switch (ds1307->type) {
+ case m41t0:
+ case m41t00:
+ case m41t11:
+ /* Export sysfs entries */
+ err = sysfs_create_group(&(ds1307->dev)->kobj,
+ &rtc_freq_test_attr_group);
+ if (err) {
+ dev_err(ds1307->dev,
+ "Failed to create sysfs group: %d\n",
+ err);
+ return err;
+ }
+
+ err = devm_add_action_or_reset(ds1307->dev,
+ rtc_calib_remove_sysfs_group,
+ ds1307->dev);
+ if (err) {
+ dev_err(ds1307->dev,
+ "Failed to add sysfs cleanup action: %d\n",
+ err);
+ sysfs_remove_group(&(ds1307->dev)->kobj,
+ &rtc_freq_test_attr_group);
+ return err;
+ }
+ break;
+ default:
+ break;
+ }
+
+ return 0;
+}
+
/*----------------------------------------------------------------------*/

static int ds1307_nvram_read(void *priv, unsigned int offset, void *val,
@@ -1792,6 +1880,12 @@ static int ds1307_probe(struct i2c_client *client,
if (err)
return err;

+ err = ds1307_add_frequency_test(ds1307);
+ if (err) {
+ rtc_device_unregister(ds1307->rtc);
+ return err;
+ }
+
if (chip->nvram_size) {
struct nvmem_config nvmem_cfg = {
.name = "ds1307_nvram",
--
2.17.1


2018-07-18 14:12:44

by Giulio Benetti

[permalink] [raw]
Subject: Re: [PATCH v10 4/4] rtc: ds1307: add frequency_test_enable sysfs attribute to check tick on m41txx

Add Andy in Cc

Il 18/07/2018 16:09, Giulio Benetti ha scritto:
> On m41txx you can enable open-drain OUT pin to check if offset is ok.
> Enabling OUT pin with frequency_test_enable attribute, OUT pin will tick
> 512 times faster than 1s tick base.
>
> Enable or Disable FT bit on CONTROL register if freq_test is 1 or 0.
>
> Signed-off-by: Giulio Benetti <[email protected]>
> ---
> drivers/rtc/rtc-ds1307.c | 94 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 94 insertions(+)
>
> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> index c6d871456f25..2efe55f43774 100644
> --- a/drivers/rtc/rtc-ds1307.c
> +++ b/drivers/rtc/rtc-ds1307.c
> @@ -1050,6 +1050,94 @@ static int m41txx_rtc_set_offset(struct device *dev, long offset)
> ctrl_reg);
> }
>
> +static ssize_t frequency_test_enable_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct ds1307 *ds1307 = dev_get_drvdata(dev);
> + bool freq_test_en;
> + int ret;
> +
> + ret = kstrtobool(buf, &freq_test_en);
> + if (ret == -EINVAL) {
> + dev_err(dev, "Failed to store RTC Frequency Test attribute\n");
> + return ret;
> + }
> +
> + regmap_update_bits(ds1307->regmap, M41TXX_REG_CONTROL, M41TXX_BIT_FT,
> + freq_test_en ? M41TXX_BIT_FT : 0);
> +
> + return count;
> +}
> +
> +static ssize_t frequency_test_enable_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct ds1307 *ds1307 = dev_get_drvdata(dev);
> + unsigned int ctrl_reg;
> +
> + regmap_read(ds1307->regmap, M41TXX_REG_CONTROL, &ctrl_reg);
> +
> + return scnprintf(buf, PAGE_SIZE, (ctrl_reg & M41TXX_BIT_FT) ? "on\n" :
> + "off\n");
> +}
> +
> +static DEVICE_ATTR_RW(frequency_test_enable);
> +
> +static struct attribute *rtc_freq_test_attrs[] = {
> + &dev_attr_frequency_test_enable.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group rtc_freq_test_attr_group = {
> + .attrs = rtc_freq_test_attrs,
> +};
> +
> +static void rtc_calib_remove_sysfs_group(void *_dev)
> +{
> + struct device *dev = _dev;
> +
> + sysfs_remove_group(&dev->kobj, &rtc_freq_test_attr_group);
> +}
> +
> +static int ds1307_add_frequency_test(struct ds1307 *ds1307)
> +{
> + int err;
> +
> + switch (ds1307->type) {
> + case m41t0:
> + case m41t00:
> + case m41t11:
> + /* Export sysfs entries */
> + err = sysfs_create_group(&(ds1307->dev)->kobj,
> + &rtc_freq_test_attr_group);
> + if (err) {
> + dev_err(ds1307->dev,
> + "Failed to create sysfs group: %d\n",
> + err);
> + return err;
> + }
> +
> + err = devm_add_action_or_reset(ds1307->dev,
> + rtc_calib_remove_sysfs_group,
> + ds1307->dev);
> + if (err) {
> + dev_err(ds1307->dev,
> + "Failed to add sysfs cleanup action: %d\n",
> + err);
> + sysfs_remove_group(&(ds1307->dev)->kobj,
> + &rtc_freq_test_attr_group);
> + return err;
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> /*----------------------------------------------------------------------*/
>
> static int ds1307_nvram_read(void *priv, unsigned int offset, void *val,
> @@ -1792,6 +1880,12 @@ static int ds1307_probe(struct i2c_client *client,
> if (err)
> return err;
>
> + err = ds1307_add_frequency_test(ds1307);
> + if (err) {
> + rtc_device_unregister(ds1307->rtc);
> + return err;
> + }
> +
> if (chip->nvram_size) {
> struct nvmem_config nvmem_cfg = {
> .name = "ds1307_nvram",
>


--
Giulio Benetti
CTO

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale ? 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642

2018-07-18 16:27:06

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v9 4/4] rtc: ds1307: add frequency_test_enable sysfs attribute to check tick on m41txx

On Wed, Jul 18, 2018 at 5:06 PM, Giulio Benetti
<[email protected]> wrote:

>>> + int ret;
>>> +
>>> + ret = kstrtobool(buf, &freq_test_en);
>>> + if (ret == -EINVAL) {

>> What kind of other error code you may expect and why they are not
>> considered as no error?

> According to documentation, kstrtobool can only fail with -EINVAL.
> See:
> https://elixir.bootlin.com/linux/v4.18-rc5/source/lib/kstrtox.c#L332

That's correct. So, what do you want to show by explicitly mentioned
an error code in the condition?

--
With Best Regards,
Andy Shevchenko

2018-07-18 16:27:56

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v10 4/4] rtc: ds1307: add frequency_test_enable sysfs attribute to check tick on m41txx

On Wed, Jul 18, 2018 at 5:09 PM, Giulio Benetti
<[email protected]> wrote:
> On m41txx you can enable open-drain OUT pin to check if offset is ok.
> Enabling OUT pin with frequency_test_enable attribute, OUT pin will tick
> 512 times faster than 1s tick base.
>
> Enable or Disable FT bit on CONTROL register if freq_test is 1 or 0.

Taking into consideration the amount of patches per hour you need to
slow down a bit.

--
With Best Regards,
Andy Shevchenko

2018-07-18 17:41:07

by Giulio Benetti

[permalink] [raw]
Subject: Re: [PATCH v10 4/4] rtc: ds1307: add frequency_test_enable sysfs attribute to check tick on m41txx

Il 18/07/2018 18:26, Andy Shevchenko ha scritto:
> On Wed, Jul 18, 2018 at 5:09 PM, Giulio Benetti
> <[email protected]> wrote:
>> On m41txx you can enable open-drain OUT pin to check if offset is ok.
>> Enabling OUT pin with frequency_test_enable attribute, OUT pin will tick
>> 512 times faster than 1s tick base.
>>
>> Enable or Disable FT bit on CONTROL register if freq_test is 1 or 0.
>
> Taking into consideration the amount of patches per hour you need to
> slow down a bit.
>

You're right, I'm really sorry about it.

Giulio

2018-07-19 18:21:43

by Giulio Benetti

[permalink] [raw]
Subject: Re: [PATCH v9 4/4] rtc: ds1307: add frequency_test_enable sysfs attribute to check tick on m41txx

Hi Andy,

Il 18/07/2018 18:26, Andy Shevchenko ha scritto:
> On Wed, Jul 18, 2018 at 5:06 PM, Giulio Benetti
> <[email protected]> wrote:
>
>>>> + int ret;
>>>> +
>>>> + ret = kstrtobool(buf, &freq_test_en);
>>>> + if (ret == -EINVAL) {
>
>>> What kind of other error code you may expect and why they are not
>>> considered as no error?
>
>> According to documentation, kstrtobool can only fail with -EINVAL.
>> See:
>> https://elixir.bootlin.com/linux/v4.18-rc5/source/lib/kstrtox.c#L332
>
> That's correct. So, what do you want to show by explicitly mentioned
> an error code in the condition?
>

So you mean I should check for negative values instead of specific
error, right?

Thanks

Giulio

2018-07-20 12:55:13

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v9 4/4] rtc: ds1307: add frequency_test_enable sysfs attribute to check tick on m41txx

On Thu, Jul 19, 2018 at 9:19 PM, Giulio Benetti
<[email protected]> wrote:
> Hi Andy,
>
> Il 18/07/2018 18:26, Andy Shevchenko ha scritto:
>>
>> On Wed, Jul 18, 2018 at 5:06 PM, Giulio Benetti
>> <[email protected]> wrote:
>>
>>>>> + int ret;
>>>>> +
>>>>> + ret = kstrtobool(buf, &freq_test_en);
>>>>> + if (ret == -EINVAL) {
>>
>>
>>>> What kind of other error code you may expect and why they are not
>>>> considered as no error?
>>
>>
>>> According to documentation, kstrtobool can only fail with -EINVAL.
>>> See:
>>> https://elixir.bootlin.com/linux/v4.18-rc5/source/lib/kstrtox.c#L332
>>
>>
>> That's correct. So, what do you want to show by explicitly mentioned
>> an error code in the condition?
>>
>
> So you mean I should check for negative values instead of specific error,
> right?

Learn from the existing code.

As you found kstrtobool() returns either 0 (no error), or -EINVAL (the
only possible error code for now).
Checking ret for non-zero is enough.
Thus,

if (ret) {
...
return ret;
}

should do a job.

--
With Best Regards,
Andy Shevchenko

2018-07-20 13:07:05

by Giulio Benetti

[permalink] [raw]
Subject: Re: [PATCH v9 4/4] rtc: ds1307: add frequency_test_enable sysfs attribute to check tick on m41txx

Il 20/07/2018 14:53, Andy Shevchenko ha scritto:
> On Thu, Jul 19, 2018 at 9:19 PM, Giulio Benetti
> <[email protected]> wrote:
>> Hi Andy,
>>
>> Il 18/07/2018 18:26, Andy Shevchenko ha scritto:
>>>
>>> On Wed, Jul 18, 2018 at 5:06 PM, Giulio Benetti
>>> <[email protected]> wrote:
>>>
>>>>>> + int ret;
>>>>>> +
>>>>>> + ret = kstrtobool(buf, &freq_test_en);
>>>>>> + if (ret == -EINVAL) {
>>>
>>>
>>>>> What kind of other error code you may expect and why they are not
>>>>> considered as no error?
>>>
>>>
>>>> According to documentation, kstrtobool can only fail with -EINVAL.
>>>> See:
>>>> https://elixir.bootlin.com/linux/v4.18-rc5/source/lib/kstrtox.c#L332
>>>
>>>
>>> That's correct. So, what do you want to show by explicitly mentioned
>>> an error code in the condition?
>>>
>>
>> So you mean I should check for negative values instead of specific error,
>> right?
>
> Learn from the existing code.
>
> As you found kstrtobool() returns either 0 (no error), or -EINVAL (the
> only possible error code for now).
> Checking ret for non-zero is enough.
> Thus,
>
> if (ret) {
> ...
> return ret;
> }
>
> should do a job.
>

Oh, I was pretty sure I've found an example of kstrtobool() in the way
I've used, instead no.
Thanks for pointing me.
I correct it.

Best regards
Giulio

2018-07-20 13:43:59

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v10 2/4] rtc: ds1307: support m41t11 variant

On Wed, Jul 18, 2018 at 04:09:43PM +0200, Giulio Benetti wrote:
> The m41t11 variant is very similar to the already supported m41t00 and
> m41t0, but it has also 56 bytes of NVRAM.
>
> Add it to driver taking into account NVRAM section.
>
> Signed-off-by: Giulio Benetti <[email protected]>
> ---
> .../devicetree/bindings/rtc/rtc-ds1307.txt | 1 +

Please add acks when posting new versions.

> drivers/rtc/rtc-ds1307.c | 14 ++++++++++++++
> 2 files changed, 15 insertions(+)

2018-07-20 13:51:47

by Giulio Benetti

[permalink] [raw]
Subject: Re: [PATCH v10 2/4] rtc: ds1307: support m41t11 variant

Il 20/07/2018 15:43, Rob Herring ha scritto:
> On Wed, Jul 18, 2018 at 04:09:43PM +0200, Giulio Benetti wrote:
>> The m41t11 variant is very similar to the already supported m41t00 and
>> m41t0, but it has also 56 bytes of NVRAM.
>>
>> Add it to driver taking into account NVRAM section.
>>
>> Signed-off-by: Giulio Benetti <[email protected]>
>> ---
>> .../devicetree/bindings/rtc/rtc-ds1307.txt | 1 +
>
> Please add acks when posting new versions.
>
>> drivers/rtc/rtc-ds1307.c | 14 ++++++++++++++
>> 2 files changed, 15 insertions(+)

Thank you for pointing me that.
v11 patchset will include them all.

Giulio