Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1292413 > unrolled thread

[PATCH v5] rtc: support DS1302 RTC on ICP DAS LP-8x4x

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v5] rtc: support DS1302 RTC on ICP DAS LP-8x4x Sergei Ianovich <ynvich@gmail.com> - 2015-12-15 18:50 +0100
    Re: [PATCH v5] rtc: support DS1302 RTC on ICP DAS LP-8x4x Rob Herring <robh@kernel.org> - 2015-12-20 04:40 +0100
      Re: [PATCH v5] rtc: support DS1302 RTC on ICP DAS LP-8x4x Sergei Ianovich <ynvich@gmail.com> - 2015-12-20 13:20 +0100
        Re: [PATCH v5] rtc: support DS1302 RTC on ICP DAS LP-8x4x Rob Herring <robh@kernel.org> - 2015-12-22 19:20 +0100
          Re: [PATCH v5] rtc: support DS1302 RTC on ICP DAS LP-8x4x Sergei Ianovich <ynvich@gmail.com> - 2015-12-24 12:10 +0100
          Re: [PATCH v5] rtc: support DS1302 RTC on ICP DAS LP-8x4x Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2015-12-24 12:10 +0100

#1292413 — [PATCH v5] rtc: support DS1302 RTC on ICP DAS LP-8x4x

Signed-off-by: Sergei Ianovich <ynvich@gmail.com>
CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
   v4..v5
   * drop THIS_MODULE from struct platform driver
   * use "dallas" for vendor name per vendor-prefixes.txt

   v3..v4
   * move DTS bindings to a different patch

   v2..v3
   * use usleep_range instead of custom nsleep
   * number change (07/16 -> 09/21)

   v0..v2
   * use device tree
   * use devm helpers where possible

 .../devicetree/bindings/rtc/rtc-ds1302.txt         |  14 +++
 drivers/rtc/Kconfig                                |   2 +-
 drivers/rtc/rtc-ds1302.c                           | 100 ++++++++++++++++++++-
 3 files changed, 113 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/rtc/rtc-ds1302.txt

diff --git a/Documentation/devicetree/bindings/rtc/rtc-ds1302.txt b/Documentation/devicetree/bindings/rtc/rtc-ds1302.txt
new file mode 100644
index 0000000..810613b
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/rtc-ds1302.txt
@@ -0,0 +1,14 @@
+* Dallas Semiconductor DS-1302 RTC
+
+Simple device which could be used to store date/time between reboots.
+
+Required properties:
+- compatible : Should be "dallas,rtc-ds1302"
+- reg : Should be address and size of IO memory region
+
+Examples:
+
+rtc@40900000 {
+	compatible = "dallas,rtc-ds1302";
+	reg = <0x1700901c 0x1>;
+};
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 2a52424..cf36483 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -803,7 +803,7 @@ config RTC_DRV_DS1286
 
 config RTC_DRV_DS1302
 	tristate "Dallas DS1302"
-	depends on SH_SECUREEDGE5410
+	depends on SH_SECUREEDGE5410 || (ARCH_PXA && HIGH_RES_TIMERS)
 	help
 	  If you say yes here you get support for the Dallas DS1302 RTC chips.
 
diff --git a/drivers/rtc/rtc-ds1302.c b/drivers/rtc/rtc-ds1302.c
index 6bef7a5..bd68214 100644
--- a/drivers/rtc/rtc-ds1302.c
+++ b/drivers/rtc/rtc-ds1302.c
@@ -50,7 +50,7 @@
 #define ds1302_set_tx()
 #define ds1302_set_rx()
 
-static inline int ds1302_hw_init(void)
+static inline int ds1302_hw_init(struct platform_device *pdev)
 {
 	return 0;
 }
@@ -86,6 +86,101 @@ static inline int ds1302_rxbit(void)
 	return !!(get_dp() & RTC_IODATA);
 }
 
+#elif defined(CONFIG_ARCH_PXA) && defined(CONFIG_HIGH_RES_TIMERS)
+
+#include <linux/delay.h>
+#include <linux/of.h>
+
+#define	RTC_CE		0x01
+#define	RTC_CLK		0x02
+#define	RTC_nWE		0x04
+#define	RTC_IODATA	0x08
+
+static unsigned long ds1302_state;
+
+static void *mem;
+
+static inline int ds1302_hw_init(struct platform_device *pdev)
+{
+	struct resource *r;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r)
+		return -ENODEV;
+
+	mem = devm_ioremap_resource(&pdev->dev, r);
+	if (!mem)
+		return -EFAULT;
+
+	return 0;
+}
+
+static inline void ds1302_reset(void)
+{
+	ds1302_state = 0;
+	iowrite8(ds1302_state, mem);
+	usleep_range(4, 5);
+}
+
+static inline void ds1302_clock(void)
+{
+	usleep_range(1, 2);
+	ds1302_state |= RTC_CLK;
+	iowrite8(ds1302_state, mem);
+	usleep_range(1, 2);
+	ds1302_state &= ~RTC_CLK;
+	iowrite8(ds1302_state, mem);
+}
+
+static inline void ds1302_start(void)
+{
+	ds1302_state &= ~RTC_CLK;
+	ds1302_state |= RTC_CE;
+	iowrite8(ds1302_state, mem);
+	usleep_range(3, 4);
+}
+
+static inline void ds1302_stop(void)
+{
+	ds1302_state &= ~RTC_CE;
+	iowrite8(ds1302_state, mem);
+}
+
+static inline void ds1302_set_tx(void)
+{
+	ds1302_state &= ~RTC_nWE;
+	iowrite8(ds1302_state, mem);
+}
+
+static inline void ds1302_set_rx(void)
+{
+	ds1302_state |= RTC_nWE;
+	iowrite8(ds1302_state, mem);
+}
+
+static inline void ds1302_txbit(int bit)
+{
+	if (bit)
+		ds1302_state |= RTC_IODATA;
+	else
+		ds1302_state &= ~RTC_IODATA;
+	iowrite8(ds1302_state, mem);
+}
+
+static inline int ds1302_rxbit(void)
+{
+	return ioread8(mem) & 0x1;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id ds1302_dt_ids[] = {
+	{ .compatible = "dallas,rtc-ds1302" },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, ds1302_dt_ids);
+#endif
+
 #else
 #error "Add support for your platform"
 #endif
@@ -216,7 +311,7 @@ static int __init ds1302_rtc_probe(struct platform_device *pdev)
 {
 	struct rtc_device *rtc;
 
-	if (ds1302_hw_init()) {
+	if (ds1302_hw_init(pdev)) {
 		dev_err(&pdev->dev, "Failed to init communication channel");
 		return -EINVAL;
 	}
@@ -244,6 +339,7 @@ static int __init ds1302_rtc_probe(struct platform_device *pdev)
 static struct platform_driver ds1302_platform_driver = {
 	.driver		= {
 		.name	= DRV_NAME,
+		.of_match_table = of_match_ptr(ds1302_dt_ids),
 	},
 };
 
-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1295514

On Tue, Dec 15, 2015 at 08:45:23PM +0300, Sergei Ianovich wrote:

Nothing in this is specific to ICP, so the subject should be updated.

> Signed-off-by: Sergei Ianovich <ynvich@gmail.com>
> CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
>    v4..v5
>    * drop THIS_MODULE from struct platform driver
>    * use "dallas" for vendor name per vendor-prefixes.txt
> 
>    v3..v4
>    * move DTS bindings to a different patch
> 
>    v2..v3
>    * use usleep_range instead of custom nsleep
>    * number change (07/16 -> 09/21)
> 
>    v0..v2
>    * use device tree
>    * use devm helpers where possible
> 
>  .../devicetree/bindings/rtc/rtc-ds1302.txt         |  14 +++
>  drivers/rtc/Kconfig                                |   2 +-
>  drivers/rtc/rtc-ds1302.c                           | 100 ++++++++++++++++++++-
>  3 files changed, 113 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/rtc/rtc-ds1302.txt
> 
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-ds1302.txt b/Documentation/devicetree/bindings/rtc/rtc-ds1302.txt
> new file mode 100644
> index 0000000..810613b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/rtc-ds1302.txt
> @@ -0,0 +1,14 @@
> +* Dallas Semiconductor DS-1302 RTC
> +
> +Simple device which could be used to store date/time between reboots.
> +
> +Required properties:
> +- compatible : Should be "dallas,rtc-ds1302"
> +- reg : Should be address and size of IO memory region

This device is a SPI (or SPI like?) interface. So you have some sort of 
of FPGA logic in between the cpu and ds1302. The DT should have a node 
for the controller and then the ds1302 as a child of it. A full blown 
SPI driver may be overkill here, but that's a separate discussion from 
the DT binding.

Rob

> +
> +Examples:
> +
> +rtc@40900000 {
> +	compatible = "dallas,rtc-ds1302";
> +	reg = <0x1700901c 0x1>;
> +};
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1295577

On Sat, 2015-12-19 at 21:38 -0600, Rob Herring wrote:
> On Tue, Dec 15, 2015 at 08:45:23PM +0300, Sergei Ianovich wrote:
> 
> Nothing in this is specific to ICP, so the subject should be updated.
> 
> > Signed-off-by: Sergei Ianovich <ynvich@gmail.com>
> > CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > ---
> >    v4..v5
> >    * drop THIS_MODULE from struct platform driver
> >    * use "dallas" for vendor name per vendor-prefixes.txt
> > 
> >    v3..v4
> >    * move DTS bindings to a different patch
> > 
> >    v2..v3
> >    * use usleep_range instead of custom nsleep
> >    * number change (07/16 -> 09/21)
> > 
> >    v0..v2
> >    * use device tree
> >    * use devm helpers where possible
> > 
> >  .../devicetree/bindings/rtc/rtc-ds1302.txt         |  14 +++
> >  drivers/rtc/Kconfig                                |   2 +-
> >  drivers/rtc/rtc-ds1302.c                           | 100
> > ++++++++++++++++++++-
> >  3 files changed, 113 insertions(+), 3 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/rtc/rtc-
> > ds1302.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/rtc/rtc-ds1302.txt
> > b/Documentation/devicetree/bindings/rtc/rtc-ds1302.txt
> > new file mode 100644
> > index 0000000..810613b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rtc/rtc-ds1302.txt
> > @@ -0,0 +1,14 @@
> > +* Dallas Semiconductor DS-1302 RTC
> > +
> > +Simple device which could be used to store date/time between
> > reboots.
> > +
> > +Required properties:
> > +- compatible : Should be "dallas,rtc-ds1302"
> > +- reg : Should be address and size of IO memory region
> 
> This device is a SPI (or SPI like?) interface. So you have some sort
> of 
> of FPGA logic in between the cpu and ds1302. The DT should have a node
> for the controller and then the ds1302 as a child of it. A full blown 
> SPI driver may be overkill here, but that's a separate discussion from
> the DT binding.

Below is the quote from the actual DT of LP-8x4x:
>                 fpga@5 {
>                         compatible = "simple-bus";
>                         #address-cells = <1>;
>                         #size-cells = <1>;
>                         ranges = <0 5 0x3000000 0x10000>;
>                         interrupt-parent = <&fpgairq>;
> 
>                         rtc@901c {
>                                 compatible = "dallas,rtc-ds1302";
>                                 reg = <0x901c 0x1>;
>                                 status = "okay";
>                         };

You are right about the topology. ds1302 is a half-duplex SPI device.
Does this mean I should rewrite the driver to handle the chip as a slave
SPI device, and then provide a master SPI functionality at the FPGA?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296961

On Sun, Dec 20, 2015 at 6:14 AM, Sergei Ianovich <ynvich@gmail.com> wrote:
> On Sat, 2015-12-19 at 21:38 -0600, Rob Herring wrote:
>> On Tue, Dec 15, 2015 at 08:45:23PM +0300, Sergei Ianovich wrote:

[...]

>> > diff --git a/Documentation/devicetree/bindings/rtc/rtc-ds1302.txt
>> > b/Documentation/devicetree/bindings/rtc/rtc-ds1302.txt
>> > new file mode 100644
>> > index 0000000..810613b
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/rtc/rtc-ds1302.txt
>> > @@ -0,0 +1,14 @@
>> > +* Dallas Semiconductor DS-1302 RTC
>> > +
>> > +Simple device which could be used to store date/time between
>> > reboots.
>> > +
>> > +Required properties:
>> > +- compatible : Should be "dallas,rtc-ds1302"
>> > +- reg : Should be address and size of IO memory region
>>
>> This device is a SPI (or SPI like?) interface. So you have some sort
>> of
>> of FPGA logic in between the cpu and ds1302. The DT should have a node
>> for the controller and then the ds1302 as a child of it. A full blown
>> SPI driver may be overkill here, but that's a separate discussion from
>> the DT binding.
>
> Below is the quote from the actual DT of LP-8x4x:
>>                 fpga@5 {
>>                         compatible = "simple-bus";
>>                         #address-cells = <1>;
>>                         #size-cells = <1>;
>>                         ranges = <0 5 0x3000000 0x10000>;
>>                         interrupt-parent = <&fpgairq>;
>>
>>                         rtc@901c {
>>                                 compatible = "dallas,rtc-ds1302";

This node should have a LP-8x4x specific compatible and then have a
child node for ds1302 that follows the SPI binding.

>>                                 reg = <0x901c 0x1>;
>>                                 status = "okay";
>>                         };
>
> You are right about the topology. ds1302 is a half-duplex SPI device.
> Does this mean I should rewrite the driver to handle the chip as a slave
> SPI device, and then provide a master SPI functionality at the FPGA?

Well, the binding should reflect that, whether the driver needs to be
re-written is somewhat a separate question. That should probably have
been done for the DS1302 driver originally and it is not too fair for
the 2nd person to fix it. You could just have a single driver bound to
the controller node which is aware of the DS1302 being the slave
device (ignoring that part of the DT for now).

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1297814

On Thu, 2015-12-24 at 12:04 +0100, Alexandre Belloni wrote:
> On 22/12/2015 at 12:16:41 -0600, Rob Herring wrote :
> > Well, the binding should reflect that, whether the driver needs to
> > be
> > re-written is somewhat a separate question. That should probably
> > have
> > been done for the DS1302 driver originally and it is not too fair
> > for
> > the 2nd person to fix it. You could just have a single driver bound
> > to
> > the controller node which is aware of the DS1302 being the slave
> > device (ignoring that part of the DT for now).
> > 
> 
> I agree with Rob here. I won't require that you fix the driver but it
> would be better to have a proper DT binding from the beginning so that
> when the driver is fixed it will still work with the previous device
> trees.

No problem. I'll fix the driver. That's how it should be done.

What should be done with SECUREEDGE support?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1297819

Hi,

On 22/12/2015 at 12:16:41 -0600, Rob Herring wrote :
> Well, the binding should reflect that, whether the driver needs to be
> re-written is somewhat a separate question. That should probably have
> been done for the DS1302 driver originally and it is not too fair for
> the 2nd person to fix it. You could just have a single driver bound to
> the controller node which is aware of the DS1302 being the slave
> device (ignoring that part of the DT for now).
> 

I agree with Rob here. I won't require that you fix the driver but it
would be better to have a proper DT binding from the beginning so that
when the driver is fixed it will still work with the previous device
trees.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web