[RESEND PATCH v3] rtc: ds1307: add trickle charger device tree binding

[RESEND PATCH v3] rtc: ds1307: add trickle charger device tree binding

[Matti Vaittinen]

Hi dt bindings maintainers (and others interested in device-tree bindings),

   I would like to get this included in your tree. Do you think there is
   still something I could improve/change in order to get this accepted?
   Or do you think I should address this to someone else?









Some DS13XX devices have "trickle chargers". Introduce a device tree binding
for specifying the trickle charger configuration for ds1339.

Signed-off-by: Matti Vaittinen <matti.vaittinen@nsn.com>
---
 .../devicetree/bindings/i2c/trivial-devices.txt    |  1 -
 .../devicetree/bindings/rtc/dallas,ds1339.txt      | 18 ++++++
 drivers/rtc/rtc-ds1307.c                           | 66 ++++++++++++++++++++--
 3 files changed, 80 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/rtc/dallas,ds1339.txt

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index 6af570e..e9206a4 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -35,7 +35,6 @@ catalyst,24c32		i2c serial eeprom
 cirrus,cs42l51		Cirrus Logic CS42L51 audio codec
 dallas,ds1307		64 x 8, Serial, I2C Real-Time Clock
 dallas,ds1338		I2C RTC with 56-Byte NV RAM
-dallas,ds1339		I2C Serial Real-Time Clock
 dallas,ds1340		I2C RTC with Trickle Charger
 dallas,ds1374		I2C, 32-Bit Binary Counter Watchdog RTC with Trickle Charger and Reset Input/Output
 dallas,ds1631		High-Precision Digital Thermometer
diff --git a/Documentation/devicetree/bindings/rtc/dallas,ds1339.txt b/Documentation/devicetree/bindings/rtc/dallas,ds1339.txt
new file mode 100644
index 0000000..fba65b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/dallas,ds1339.txt
@@ -0,0 +1,18 @@
+* Dallas DS1339		I2C Serial Real-Time Clock
+
+Required properties:
+- compatible: Should contain "dallas,ds1339".
+- reg: I2C address for chip
+
+Optional properties:
+- trickle-resistor-ohms : Selected resistor for trickle charger
+	Values usable for ds1339 are 250, 2000, 4000
+	Should be given if trickle charger should be enabled
+- trickle-diode-enable : Use internal trickle charger diode
+	Should be given if internal trickle charger diode should be enabled
+Example:
+	ds1339: rtc@68 {
+		compatible = "dallas,ds1339";
+		trickle-resistor-ohms = <250>;
+		reg = <0x68>;
+	};
diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index f03d5ba..b297c2c 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -126,9 +126,14 @@ struct chip_desc {
 	u16			nvram_offset;
 	u16			nvram_size;
 	u16			trickle_charger_reg;
+	u8			trickle_charger_setup;
+	u8			(*do_trickle_setup)(struct i2c_client *, uint32_t, bool);
 };
 
-static const struct chip_desc chips[last_ds_type] = {
+static u8 do_trickle_setup_ds1339(struct i2c_client *,
+				  uint32_t ohms, bool diode);
+
+static struct chip_desc chips[last_ds_type] = {
 	[ds_1307] = {
 		.nvram_offset	= 8,
 		.nvram_size	= 56,
@@ -143,6 +148,7 @@ static const struct chip_desc chips[last_ds_type] = {
 	[ds_1339] = {
 		.alarm		= 1,
 		.trickle_charger_reg = 0x10,
+		.do_trickle_setup = &do_trickle_setup_ds1339,
 	},
 	[ds_1340] = {
 		.trickle_charger_reg = 0x08,
@@ -833,15 +839,57 @@ ds1307_nvram_write(struct file *filp, struct kobject *kobj,
 	return count;
 }
 
+
 /*----------------------------------------------------------------------*/
 
+static u8 do_trickle_setup_ds1339(struct i2c_client *client,
+				  uint32_t ohms, bool diode)
+{
+	u8 setup = (diode) ? DS1307_TRICKLE_CHARGER_DIODE :
+		DS1307_TRICKLE_CHARGER_NO_DIODE;
+
+	switch (ohms) {
+	case 250:
+		setup |= DS1307_TRICKLE_CHARGER_250_OHM;
+		break;
+	case 2000:
+		setup |= DS1307_TRICKLE_CHARGER_2K_OHM;
+		break;
+	case 4000:
+		setup |= DS1307_TRICKLE_CHARGER_4K_OHM;
+		break;
+	default:
+		dev_warn(&client->dev,
+			 "Unsupported ohm value %u in dt\n", ohms);
+		return 0;
+	}
+	return setup;
+}
+
+static void ds1307_trickle_of_init(struct i2c_client *client,
+				   struct chip_desc *chip)
+{
+	uint32_t ohms = 0;
+	bool diode = false;
+
+	if (!chip->do_trickle_setup)
+		goto out;
+	if (of_property_read_u32(client->dev.of_node, "trickle-resistor-ohms" , &ohms))
+		goto out;
+	diode = of_property_read_bool(client->dev.of_node, "diode-connected");
+	chip->trickle_charger_setup = chip->do_trickle_setup(client,
+							     ohms, diode);
+out:
+	return;
+}
+
 static int ds1307_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
 	struct ds1307		*ds1307;
 	int			err = -ENODEV;
 	int			tmp;
-	const struct chip_desc	*chip = &chips[id->driver_data];
+	struct chip_desc	*chip = &chips[id->driver_data];
 	struct i2c_adapter	*adapter = to_i2c_adapter(client->dev.parent);
 	bool			want_irq = false;
 	unsigned char		*buf;
@@ -866,9 +914,19 @@ static int ds1307_probe(struct i2c_client *client,
 	ds1307->client	= client;
 	ds1307->type	= id->driver_data;
 
-	if (pdata && pdata->trickle_charger_setup && chip->trickle_charger_reg)
+	if (!pdata && client->dev.of_node)
+		ds1307_trickle_of_init(client, chip);
+	else if (pdata && pdata->trickle_charger_setup)
+		chip->trickle_charger_setup = pdata->trickle_charger_setup;
+
+	if (chip->trickle_charger_setup && chip->trickle_charger_reg) {
+		dev_dbg(&client->dev, "writing trickle charger info 0x%x to 0x%x\n",
+		    DS13XX_TRICKLE_CHARGER_MAGIC | chip->trickle_charger_setup,
+		    chip->trickle_charger_reg);
 		i2c_smbus_write_byte_data(client, chip->trickle_charger_reg,
-			DS13XX_TRICKLE_CHARGER_MAGIC | pdata->trickle_charger_setup);
+		    DS13XX_TRICKLE_CHARGER_MAGIC |
+		    chip->trickle_charger_setup);
+	}
 
 	buf = ds1307->regs;
 	if (i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) {
-- 
1.8.3.1


-- 

Re: [RESEND PATCH v3] rtc: ds1307: add trickle charger device tree binding

[Jason Cooper]

Matti,

On Mon, Sep 08, 2014 at 10:32:24AM +0300, Matti Vaittinen wrote:

>From here:

> Hi dt bindings maintainers (and others interested in device-tree bindings),
> 
>    I would like to get this included in your tree. Do you think there is
>    still something I could improve/change in order to get this accepted?
>    Or do you think I should address this to someone else?
> 
> 
> 
> 
> 
> 
> 
> 
> 

To here should go below the '---'

> Some DS13XX devices have "trickle chargers". Introduce a device tree binding
> for specifying the trickle charger configuration for ds1339.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@nsn.com>
> ---

Here.  Otherwise, it'll get included in the commit message, which I
don't think was your intention.

>  .../devicetree/bindings/i2c/trivial-devices.txt    |  1 -
>  .../devicetree/bindings/rtc/dallas,ds1339.txt      | 18 ++++++
>  drivers/rtc/rtc-ds1307.c                           | 66 ++++++++++++++++++++--
>  3 files changed, 80 insertions(+), 5 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/rtc/dallas,ds1339.txt

I would split this into two patches, one for the binding documentation,
and one for the C file changes.  You can then use
./scripts/get_maintainer.pl to see who the rtc maintainer is (nothing
jumps out from my memory).

Most likely, the rtc maintainer will take the series with the DT
maintainers Acks.  You made all the binding changes as discussed by Mark
and I, so fwiw,

Acked-by: Jason Cooper <jason@lakedaemon.net>

Just keep in mind that I'm not a DT maintainer, I've just moved a lot of
patches into that area :-P  When you respin the series to split it, just
throw my Ack on the binding docs.

thx,

Jason.

Re: [RESEND PATCH v3] rtc: ds1307: add trickle charger device tree binding

[Jason Cooper]

Matti,

On Mon, Sep 08, 2014 at 10:32:24AM +0300, Matti Vaittinen wrote:

>From here:

> Hi dt bindings maintainers (and others interested in device-tree bindings),
> 
>    I would like to get this included in your tree. Do you think there is
>    still something I could improve/change in order to get this accepted?
>    Or do you think I should address this to someone else?
> 
> 
> 
> 
> 
> 
> 
> 
> 

To here should go below the '---'

> Some DS13XX devices have "trickle chargers". Introduce a device tree binding
> for specifying the trickle charger configuration for ds1339.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen-OYasijW0DpE@public.gmane.org>
> ---

Here.  Otherwise, it'll get included in the commit message, which I
don't think was your intention.

>  .../devicetree/bindings/i2c/trivial-devices.txt    |  1 -
>  .../devicetree/bindings/rtc/dallas,ds1339.txt      | 18 ++++++
>  drivers/rtc/rtc-ds1307.c                           | 66 ++++++++++++++++++++--
>  3 files changed, 80 insertions(+), 5 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/rtc/dallas,ds1339.txt

I would split this into two patches, one for the binding documentation,
and one for the C file changes.  You can then use
./scripts/get_maintainer.pl to see who the rtc maintainer is (nothing
jumps out from my memory).

Most likely, the rtc maintainer will take the series with the DT
maintainers Acks.  You made all the binding changes as discussed by Mark
and I, so fwiw,

Acked-by: Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>

Just keep in mind that I'm not a DT maintainer, I've just moved a lot of
patches into that area :-P  When you respin the series to split it, just
throw my Ack on the binding docs.

thx,

Jason.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND PATCH v3] rtc: ds1307: add trickle charger device tree binding

[Alessandro Zummo]

On Mon, 8 Sep 2014 07:26:57 -0400
Jason Cooper <jason@lakedaemon.net> wrote:

> Most likely, the rtc maintainer will take the series with the DT
> maintainers Acks.  You made all the binding changes as discussed by Mark
> and I, so fwiw,
> 
> Acked-by: Jason Cooper <jason@lakedaemon.net>


 Acked-by: Alessandro Zummo <a.zummo@towertech.it>

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it

Re: [RESEND PATCH v3] rtc: ds1307: add trickle charger device tree binding

[Matti Vaittinen]

On Mon, Sep 08, 2014 at 07:26:57AM -0400, ext Jason Cooper wrote:
> Matti,
> 
> On Mon, Sep 08, 2014 at 10:32:24AM +0300, Matti Vaittinen wrote:
> > ---
> 
> Here.  Otherwise, it'll get included in the commit message, which I
> don't think was your intention.

Ok, thanks.

> 
> >  .../devicetree/bindings/i2c/trivial-devices.txt    |  1 -
> >  .../devicetree/bindings/rtc/dallas,ds1339.txt      | 18 ++++++
> >  drivers/rtc/rtc-ds1307.c                           | 66 ++++++++++++++++++++--
> >  3 files changed, 80 insertions(+), 5 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/rtc/dallas,ds1339.txt
> 
> I would split this into two patches, one for the binding documentation,
> and one for the C file changes.  You can then use
> ./scripts/get_maintainer.pl to see who the rtc maintainer is (nothing
> jumps out from my memory).

Splitting sounds reasonable. I will do this and also invert the default
diode setting to be connected as was suggested by Pavel in mail responce
to v1 of patch. And Alessandro is the maintainer for rtc according to
the MAINTAINERS file.

> 
> Most likely, the rtc maintainer will take the series with the DT
> maintainers Acks.  You made all the binding changes as discussed by Mark
> and I, so fwiw,
> 
> Acked-by: Jason Cooper <jason@lakedaemon.net>
> 
> Just keep in mind that I'm not a DT maintainer, I've just moved a lot of
> patches into that area :-P  When you respin the series to split it, just
> throw my Ack on the binding docs.

Thanks, although the doc will change when I invert the default diode
setting  =)

Br.
Matti

-- 
=============================================

Matti Vaittinen
Senile SW Specialist
FINLAND 

~~ When things go utterly wrong vim users can always type :help! ~~

Re: [RESEND PATCH v3] rtc: ds1307: add trickle charger device tree binding

[Matti Vaittinen]

On Mon, Sep 08, 2014 at 07:26:57AM -0400, ext Jason Cooper wrote:
> Matti,
> 
> On Mon, Sep 08, 2014 at 10:32:24AM +0300, Matti Vaittinen wrote:
> > ---
> 
> Here.  Otherwise, it'll get included in the commit message, which I
> don't think was your intention.

Ok, thanks.

> 
> >  .../devicetree/bindings/i2c/trivial-devices.txt    |  1 -
> >  .../devicetree/bindings/rtc/dallas,ds1339.txt      | 18 ++++++
> >  drivers/rtc/rtc-ds1307.c                           | 66 ++++++++++++++++++++--
> >  3 files changed, 80 insertions(+), 5 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/rtc/dallas,ds1339.txt
> 
> I would split this into two patches, one for the binding documentation,
> and one for the C file changes.  You can then use
> ./scripts/get_maintainer.pl to see who the rtc maintainer is (nothing
> jumps out from my memory).

Splitting sounds reasonable. I will do this and also invert the default
diode setting to be connected as was suggested by Pavel in mail responce
to v1 of patch. And Alessandro is the maintainer for rtc according to
the MAINTAINERS file.

> 
> Most likely, the rtc maintainer will take the series with the DT
> maintainers Acks.  You made all the binding changes as discussed by Mark
> and I, so fwiw,
> 
> Acked-by: Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>
> 
> Just keep in mind that I'm not a DT maintainer, I've just moved a lot of
> patches into that area :-P  When you respin the series to split it, just
> throw my Ack on the binding docs.

Thanks, although the doc will change when I invert the default diode
setting  =)

Br.
Matti

-- 
=============================================

Matti Vaittinen
Senile SW Specialist
FINLAND 

~~ When things go utterly wrong vim users can always type :help! ~~

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND PATCH v3] rtc: ds1307: add trickle charger device tree binding

[Jason Cooper]

On Tue, Sep 09, 2014 at 11:29:11AM +0300, Matti Vaittinen wrote:
> On Mon, Sep 08, 2014 at 07:26:57AM -0400, ext Jason Cooper wrote:
> > Matti,
> > 
> > On Mon, Sep 08, 2014 at 10:32:24AM +0300, Matti Vaittinen wrote:
> > > ---
> > 
> > Here.  Otherwise, it'll get included in the commit message, which I
> > don't think was your intention.
> 
> Ok, thanks.
> 
> > 
> > >  .../devicetree/bindings/i2c/trivial-devices.txt    |  1 -
> > >  .../devicetree/bindings/rtc/dallas,ds1339.txt      | 18 ++++++
> > >  drivers/rtc/rtc-ds1307.c                           | 66 ++++++++++++++++++++--
> > >  3 files changed, 80 insertions(+), 5 deletions(-)
> > >  create mode 100644 Documentation/devicetree/bindings/rtc/dallas,ds1339.txt
> > 
> > I would split this into two patches, one for the binding documentation,
> > and one for the C file changes.  You can then use
> > ./scripts/get_maintainer.pl to see who the rtc maintainer is (nothing
> > jumps out from my memory).
> 
> Splitting sounds reasonable. I will do this and also invert the default
> diode setting to be connected as was suggested by Pavel in mail responce
> to v1 of patch. And Alessandro is the maintainer for rtc according to
> the MAINTAINERS file.
> 
> > 
> > Most likely, the rtc maintainer will take the series with the DT
> > maintainers Acks.  You made all the binding changes as discussed by Mark
> > and I, so fwiw,
> > 
> > Acked-by: Jason Cooper <jason@lakedaemon.net>
> > 
> > Just keep in mind that I'm not a DT maintainer, I've just moved a lot of
> > patches into that area :-P  When you respin the series to split it, just
> > throw my Ack on the binding docs.
> 
> Thanks, although the doc will change when I invert the default diode
> setting  =)

Yep, I'm fine with the inversion.  Feel free to keep the Ack.

thx,

Jason.