Signed-off-by: Sergei Ianovich <[email protected]>
CC: Alexandre Belloni <[email protected]>
---
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
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 <[email protected]>
> CC: Alexandre Belloni <[email protected]>
> ---
> 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>;
> +};
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 <[email protected]>
> > CC: Alexandre Belloni <[email protected]>
> > ---
> > 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?
On Sun, Dec 20, 2015 at 6:14 AM, Sergei Ianovich <[email protected]> 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
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
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?