[PATCH 1/4] rtc: ds1307: fix data pointer to m41t0

[PATCH 1/4] rtc: ds1307: fix data pointer to m41t0

[Giulio Benetti]

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 <giulio.benetti@micronovasrl.com>
---
 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 a13e59edff53..32aadcbc377f 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -298,7 +298,7 @@ static const struct of_device_id ds1307_of_match[] = {
 	},
 	{
 		.compatible = "st,m41t0",
-		.data = (void *)m41t00
+		.data = (void *)m41t0
 	},
 	{
 		.compatible = "st,m41t00",
-- 
2.17.0

[PATCH 2/4] rtc: ds1307: support m41t11 variant

[Giulio Benetti]

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 <giulio.benetti@micronovasrl.com>
---
 .../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 32aadcbc377f..0ab0c166da83 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,
@@ -226,6 +227,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 */
@@ -248,6 +254,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 },
@@ -304,6 +311,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
@@ -346,6 +357,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 },
@@ -1574,6 +1586,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);
@@ -1639,6 +1652,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.0

[PATCH 3/4] rtc: ds1307: add calibration of_ for mt41txx chips.

[Giulio Benetti]

m41txx chips can hold a calibration value to get really near to real
tick value.

Add calibration property(ranging between (-31) and 31), so on every probe
calibration value will be written to rtc.
This is because ic could loose supply due to low battery.

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 .../devicetree/bindings/rtc/rtc-ds1307.txt    |  2 ++
 drivers/rtc/rtc-ds1307.c                      | 33 +++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
index ce6469c1a516..d3d70a5495c5 100644
--- a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
@@ -34,6 +34,8 @@ Optional properties:
 - trickle-diode-disable : ds1339, ds1340 and ds 1388 only
 	Do not use internal trickle charger diode
 	Should be given if internal trickle charger diode should be disabled
+- calibration: m41t0, m41t00, m41t11 only
+	Set calibration value to correct external bias, ranging between (-31) and 31.
 
 Example:
 	rtc1: ds1339@68 {
diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 0ab0c166da83..9cda52589c0f 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -114,6 +114,12 @@ enum ds_type {
 #	define RX8025_BIT_VDET		0x40
 #	define RX8025_BIT_XST		0x20
 
+#define M41TXX_REG_CONTROL	0x07
+#	define M41TXX_BIT_OUT		0x80
+#	define M41TXX_BIT_FT		0x40
+#	define M41TXX_BIT_CALIB_SIGN	0x20
+#	define M41TXX_M_CALIBRATION	0x1f
+
 struct ds1307 {
 	enum ds_type		type;
 	unsigned long		flags;
@@ -1397,6 +1403,7 @@ static int ds1307_probe(struct i2c_client *client,
 	unsigned char		regs[8];
 	struct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);
 	u8			trickle_charger_setup = 0;
+	s32			calib;
 
 	ds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL);
 	if (!ds1307)
@@ -1460,6 +1467,32 @@ static int ds1307_probe(struct i2c_client *client,
 	if (chip->alarm && of_property_read_bool(client->dev.of_node,
 						 "wakeup-source"))
 		ds1307_can_wakeup_device = true;
+
+	/* retrieve calibration value if provided */
+	if (!of_property_read_s32(client->dev.of_node, "calibration",
+				 &calib)) {
+		switch (ds1307->type) {
+		case m41t0:
+		case m41t00:
+		case m41t11:
+		{
+			/*
+			 * Set calibration value every power-on since rtc
+			 * could have shut off(low battery)
+			 */
+			u8 out_byte = abs(calib) & M41TXX_M_CALIBRATION;
+
+			if (calib >= 0)
+				out_byte |= M41TXX_BIT_CALIB_SIGN;
+
+			regmap_write(ds1307->regmap, M41TXX_REG_CONTROL,
+					out_byte);
+		}
+			break;
+		default:
+			break;
+		}
+	}
 #endif
 
 	switch (ds1307->type) {
-- 
2.17.0

Re: [PATCH 3/4] rtc: ds1307: add calibration of_ for mt41txx chips.

[Rob Herring]

On Tue, May 8, 2018 at 9:56 AM, Giulio Benetti
<giulio.benetti@micronovasrl.com> wrote:
> m41txx chips can hold a calibration value to get really near to real
> tick value.

Typo in the subject.

> Add calibration property(ranging between (-31) and 31), so on every probe
> calibration value will be written to rtc.
> This is because ic could loose supply due to low battery.
>
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
>  .../devicetree/bindings/rtc/rtc-ds1307.txt    |  2 ++

Put all the binding changes in separate patch (or maybe 2: new
compatible and new properties).

>  drivers/rtc/rtc-ds1307.c                      | 33 +++++++++++++++++++
>  2 files changed, 35 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
> index ce6469c1a516..d3d70a5495c5 100644
> --- a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
> +++ b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
> @@ -34,6 +34,8 @@ Optional properties:
>  - trickle-diode-disable : ds1339, ds1340 and ds 1388 only
>         Do not use internal trickle charger diode
>         Should be given if internal trickle charger diode should be disabled
> +- calibration: m41t0, m41t00, m41t11 only

Needs a vendor prefix.

Calibration of what and what are the units?

> +       Set calibration value to correct external bias, ranging between (-31) and 31.
>
>  Example:
>         rtc1: ds1339@68 {

Re: [PATCH 3/4] rtc: ds1307: add calibration of_ for mt41txx chips.

[Giulio Benetti]

Hi Rob,

Il 08/05/2018 19:40, Rob Herring ha scritto:
> On Tue, May 8, 2018 at 9:56 AM, Giulio Benetti
> <giulio.benetti@micronovasrl.com> wrote:
>> m41txx chips can hold a calibration value to get really near to real
>> tick value.
> 
> Typo in the subject.
> 
>> Add calibration property(ranging between (-31) and 31), so on every probe
>> calibration value will be written to rtc.
>> This is because ic could loose supply due to low battery.
>>
>> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
>> ---
>>   .../devicetree/bindings/rtc/rtc-ds1307.txt    |  2 ++
> 
> Put all the binding changes in separate patch (or maybe 2: new
> compatible and new properties).
> 
>>   drivers/rtc/rtc-ds1307.c                      | 33 +++++++++++++++++++
>>   2 files changed, 35 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
>> index ce6469c1a516..d3d70a5495c5 100644
>> --- a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
>> +++ b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
>> @@ -34,6 +34,8 @@ Optional properties:
>>   - trickle-diode-disable : ds1339, ds1340 and ds 1388 only
>>          Do not use internal trickle charger diode
>>          Should be given if internal trickle charger diode should be disabled
>> +- calibration: m41t0, m41t00, m41t11 only
> 
> Needs a vendor prefix.
> 
> Calibration of what and what are the units?
> 
>> +       Set calibration value to correct external bias, ranging between (-31) and 31.
>>
>>   Example:
>>          rtc1: ds1339@68 {

This is not a problem anymore since I've used standard rtc sysfs 
"offset" instead of a dt-property as Alexandre pointed me out.

Thanks.

-- 
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

Re: [PATCH 3/4] rtc: ds1307: add calibration of_ for mt41txx chips.

[Alexandre Belloni]

Hi,

There is a sysfs interface for calibration, please use it. this is
especially useful since the calibration value probably depend on
temperature so it has to be updated dynamically to be really useful.

See https://elixir.bootlin.com/linux/v4.16.7/source/Documentation/ABI/testing/sysfs-class-rtc#L68
and https://elixir.bootlin.com/linux/v4.16.7/source/drivers/rtc/interface.c#L999

On 08/05/2018 16:56:10+0200, Giulio Benetti wrote:
> m41txx chips can hold a calibration value to get really near to real
> tick value.
> 
> Add calibration property(ranging between (-31) and 31), so on every probe
> calibration value will be written to rtc.
> This is because ic could loose supply due to low battery.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
>  .../devicetree/bindings/rtc/rtc-ds1307.txt    |  2 ++
>  drivers/rtc/rtc-ds1307.c                      | 33 +++++++++++++++++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
> index ce6469c1a516..d3d70a5495c5 100644
> --- a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
> +++ b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
> @@ -34,6 +34,8 @@ Optional properties:
>  - trickle-diode-disable : ds1339, ds1340 and ds 1388 only
>  	Do not use internal trickle charger diode
>  	Should be given if internal trickle charger diode should be disabled
> +- calibration: m41t0, m41t00, m41t11 only
> +	Set calibration value to correct external bias, ranging between (-31) and 31.
>  
>  Example:
>  	rtc1: ds1339@68 {
> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> index 0ab0c166da83..9cda52589c0f 100644
> --- a/drivers/rtc/rtc-ds1307.c
> +++ b/drivers/rtc/rtc-ds1307.c
> @@ -114,6 +114,12 @@ enum ds_type {
>  #	define RX8025_BIT_VDET		0x40
>  #	define RX8025_BIT_XST		0x20
>  
> +#define M41TXX_REG_CONTROL	0x07
> +#	define M41TXX_BIT_OUT		0x80
> +#	define M41TXX_BIT_FT		0x40
> +#	define M41TXX_BIT_CALIB_SIGN	0x20
> +#	define M41TXX_M_CALIBRATION	0x1f
> +
>  struct ds1307 {
>  	enum ds_type		type;
>  	unsigned long		flags;
> @@ -1397,6 +1403,7 @@ static int ds1307_probe(struct i2c_client *client,
>  	unsigned char		regs[8];
>  	struct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);
>  	u8			trickle_charger_setup = 0;
> +	s32			calib;
>  
>  	ds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL);
>  	if (!ds1307)
> @@ -1460,6 +1467,32 @@ static int ds1307_probe(struct i2c_client *client,
>  	if (chip->alarm && of_property_read_bool(client->dev.of_node,
>  						 "wakeup-source"))
>  		ds1307_can_wakeup_device = true;
> +
> +	/* retrieve calibration value if provided */
> +	if (!of_property_read_s32(client->dev.of_node, "calibration",
> +				 &calib)) {
> +		switch (ds1307->type) {
> +		case m41t0:
> +		case m41t00:
> +		case m41t11:
> +		{
> +			/*
> +			 * Set calibration value every power-on since rtc
> +			 * could have shut off(low battery)
> +			 */
> +			u8 out_byte = abs(calib) & M41TXX_M_CALIBRATION;
> +
> +			if (calib >= 0)
> +				out_byte |= M41TXX_BIT_CALIB_SIGN;
> +
> +			regmap_write(ds1307->regmap, M41TXX_REG_CONTROL,
> +					out_byte);
> +		}
> +			break;
> +		default:
> +			break;
> +		}
> +	}
>  #endif
>  
>  	switch (ds1307->type) {
> -- 
> 2.17.0
> 

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 3/4] rtc: ds1307: add calibration of_ for mt41txx chips.

[Giulio Benetti]

Hi Alexandre,

Il 08/05/2018 21:19, Alexandre Belloni ha scritto:
> Hi,
> 
> There is a sysfs interface for calibration, please use it. this is
> especially useful since the calibration value probably depend on
> temperature so it has to be updated dynamically to be really useful.
> 
> See https://elixir.bootlin.com/linux/v4.16.7/source/Documentation/ABI/testing/sysfs-class-rtc#L68
> and https://elixir.bootlin.com/linux/v4.16.7/source/drivers/rtc/interface.c#L999
> 

Used sysfs now and just submitted V2 patchset.
Thanks for pointing me those links.

-- 
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

[PATCH 4/4] rtc: ds1307: add frequency-test property to check calibration on m41txx

[Giulio Benetti]

On m41txx you can enable open-drain OUT pin to check if calibration is ok.
Enabling OUT pin with frequency-test bool property, OUT pin will tick
512 times faster than 1s tick base.

Enable FT bit on CONTROL register if calibration is active.

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 Documentation/devicetree/bindings/rtc/rtc-ds1307.txt | 3 +++
 drivers/rtc/rtc-ds1307.c                             | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
index d3d70a5495c5..72a5f8cdc306 100644
--- a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
@@ -36,6 +36,9 @@ Optional properties:
 	Should be given if internal trickle charger diode should be disabled
 - calibration: m41t0, m41t00, m41t11 only
 	Set calibration value to correct external bias, ranging between (-31) and 31.
+- frequency-test: m41t0, m41t00, m41t11 only
+	Enable open-drain OUT pin 512 times faster than 1 second tick.
+	This allows to check calibration value.
 
 Example:
 	rtc1: ds1339@68 {
diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 9cda52589c0f..a727ced157df 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1485,6 +1485,10 @@ static int ds1307_probe(struct i2c_client *client,
 			if (calib >= 0)
 				out_byte |= M41TXX_BIT_CALIB_SIGN;
 
+			if (of_property_read_bool(client->dev.of_node,
+					"frequency-test"))
+				out_byte |= M41TXX_BIT_FT;
+
 			regmap_write(ds1307->regmap, M41TXX_REG_CONTROL,
 					out_byte);
 		}
-- 
2.17.0

Re: [PATCH 4/4] rtc: ds1307: add frequency-test property to check calibration on m41txx

[Rob Herring]

On Tue, May 8, 2018 at 9:56 AM, Giulio Benetti
<giulio.benetti@micronovasrl.com> wrote:
> On m41txx you can enable open-drain OUT pin to check if calibration is ok.
> Enabling OUT pin with frequency-test bool property, OUT pin will tick
> 512 times faster than 1s tick base.
>
> Enable FT bit on CONTROL register if calibration is active.

A DT is (or should be) a pretty static thing, so I don't think this
belongs. A kernel command line option (via module params) or a
sysfs/debugfs file would make more sense IMO.

Rob

Re: [PATCH 4/4] rtc: ds1307: add frequency-test property to check calibration on m41txx

[Giulio Benetti]

Hi Rob,

Il 08/05/2018 19:44, Rob Herring ha scritto:
> On Tue, May 8, 2018 at 9:56 AM, Giulio Benetti
> <giulio.benetti@micronovasrl.com> wrote:
>> On m41txx you can enable open-drain OUT pin to check if calibration is ok.
>> Enabling OUT pin with frequency-test bool property, OUT pin will tick
>> 512 times faster than 1s tick base.
>>
>> Enable FT bit on CONTROL register if calibration is active.
> 
> A DT is (or should be) a pretty static thing, so I don't think this
> belongs. A kernel command line option (via module params) or a
> sysfs/debugfs file would make more sense IMO.

Totally right, it's very useful having it accessible via sysfs.
Already changed and just submitted V2 patchset.
Thanks

-- 
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