Message ID | 1450201525-9137-1-git-send-email-ynvich@gmail.com |
---|---|
State | Superseded |
Headers | show |
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>; > +};
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?
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
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.
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?
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), }, };
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