| Message ID | 1306393409-16153-1-git-send-email-Priyanka.Jain@freescale.com |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, May 26, 2011 at 12:33:29PM +0530, Priyanka Jain wrote: > PT7C4338 chip is being manufactured by Pericom Technology Inc. > It is a serial real-time clock which provides: > 1)Low-power clock/calendar. > 2)Programmable square-wave output. > It has 56 bytes of nonvolatile RAM. > Its register set is same as that of rtc device: DS1307. > > Signed-off-by: Priyanka Jain <Priyanka.Jain@freescale.com> So it is identical to ds1307? Then why not name your platform_device simply 'ds1307'? Regards, Wolfram
> -----Original Message----- > From: linuxppc-dev-bounces+priyanka.jain=freescale.com@lists.ozlabs.org > [mailto:linuxppc-dev- > bounces+priyanka.jain=freescale.com@lists.ozlabs.org] On Behalf Of > Wolfram Sang > Sent: Thursday, May 26, 2011 2:40 PM > To: Jain Priyanka-B32167 > Cc: a.zummo@towertech.it; akpm@linux-foundation.org; linuxppc- > dev@lists.ozlabs.org; rtc-linux@googlegroups.com; p_gortmaker@yahoo.com > Subject: Re: [PATCH] Add support for pt7c4338 (rtc device) in rtc-ds1307 > driver > > On Thu, May 26, 2011 at 12:33:29PM +0530, Priyanka Jain wrote: > > > PT7C4338 chip is being manufactured by Pericom Technology Inc. > > It is a serial real-time clock which provides: > > 1)Low-power clock/calendar. > > 2)Programmable square-wave output. > > It has 56 bytes of nonvolatile RAM. > > Its register set is same as that of rtc device: DS1307. > > > > Signed-off-by: Priyanka Jain <Priyanka.Jain@freescale.com> > > So it is identical to ds1307? Then why not name your platform_device > simply 'ds1307'? Yes, It is possible to directly use platform device 'ds1307' in device tree. But I have one query of how to capture the information that pericom, pt7c4338 device is compatible with dallas, ds1307. Can it be done in somewhere in some document or some code. Actually for pt7c4338 device driver, I actually started by asking if there is any already existing rtc device driver which is compatible with pericom pt7c4338 device on mailing list and as there was no answer or help then, I actually ended up writing device driver for that and then on the suggestion using ds1307 device driver for this. Thanks Priyanka > > Regards, > > Wolfram > > -- > Pengutronix e.K. | Wolfram Sang > | > Industrial Linux Solutions | http://www.pengutronix.de/ > |
Hi, Priyanka, > Yes, It is possible to directly use platform device 'ds1307' in device > tree. Great, thanks for testing. > But I have one query of how to capture the information that pericom, > pt7c4338 device is compatible with dallas, ds1307. Can it be done in > somewhere in some document or some code. Actually for pt7c4338 device Yes, it should definately be documented in the source. > driver, I actually started by asking if there is any already existing > rtc device driver which is compatible with pericom pt7c4338 device on > mailing list and as there was no answer or help then, I actually ended > up writing device driver for that The first place where this should be mentioned is the datasheet of the pt-chip, so you might ask the producer to add this information (don't expect much to happen, though). For such generic devices, it is then useful to look for similar register sets in existing drivers to find a duplicate. Please don't forget that such lists are voluntary, there is no guarantee that mails get replied to. > and then on the suggestion using ds1307 device driver for this. IIRC I asked you explicitly for the differences between the chips. If there are none, you can use the driver directly, right? :) Regards, Wolfram
On Mon, May 30, 2011 at 3:24 AM, Wolfram Sang <w.sang@pengutronix.de> wrote: > The first place where this should be mentioned is the datasheet of the > pt-chip, so you might ask the producer to add this information (don't > expect much to happen, though). It's true that the data sheet does not mention that it's identical to the DS1307, but that's still no excuse for not noticing it and writing a whole driver for it. :-( > IIRC I asked you explicitly for the differences between the chips. If > there are none, you can use the driver directly, right? :) Yes. The device tree node for the PT7C4338 device should just say /* The board has a PT7C4338, which is compatible with the DS1307 */ compatible = "dallas,ds1307"; And that's it.
> > IIRC I asked you explicitly for the differences between the chips. If > > there are none, you can use the driver directly, right? :) > > Yes. The device tree node for the PT7C4338 device should just say > > /* The board has a PT7C4338, which is compatible with the DS1307 */ > compatible = "dallas,ds1307"; > > And that's it. I'd also suggest to add a comment to the id_table in rtc-ds1307.c, so the chip name can be grepped for.
On Mon, May 30, 2011 at 02:29:58PM +0000, Tabi Timur-B04825 wrote: > On Mon, May 30, 2011 at 3:24 AM, Wolfram Sang <w.sang@pengutronix.de> wrote: > > > The first place where this should be mentioned is the datasheet of the > > pt-chip, so you might ask the producer to add this information (don't > > expect much to happen, though). > > It's true that the data sheet does not mention that it's identical to > the DS1307, but that's still no excuse for not noticing it and writing > a whole driver for it. :-( > > > IIRC I asked you explicitly for the differences between the chips. If > > there are none, you can use the driver directly, right? :) > > Yes. The device tree node for the PT7C4338 device should just say > > /* The board has a PT7C4338, which is compatible with the DS1307 */ > compatible = "dallas,ds1307"; While it seems to be 100% compatible, there could be chip-specific bugs or some interesting features that are hidden behind "reserved" bits and registers. So I think device tree should not lie about the chip model. Doing 'compatible = "pericom,pt7c4338", "dallas,ds1307"' is perfectly fine. Note that today the several compatible entries approach gives you almost nothing, as you will need to add pt7c4338 entry into the driver anyway. I tried to improve this, i.e. make linux do OF-matching on the most generic compatible entry (the last one): http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg21196.html It was received coldly though: http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg22041.html http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg21273.html
On Mon, May 30, 2011 at 08:57:45PM +0400, Anton Vorontsov wrote: > On Mon, May 30, 2011 at 02:29:58PM +0000, Tabi Timur-B04825 wrote: > > On Mon, May 30, 2011 at 3:24 AM, Wolfram Sang <w.sang@pengutronix.de> wrote: > > > > > The first place where this should be mentioned is the datasheet of the > > > pt-chip, so you might ask the producer to add this information (don't > > > expect much to happen, though). > > > > It's true that the data sheet does not mention that it's identical to > > the DS1307, but that's still no excuse for not noticing it and writing > > a whole driver for it. :-( > > > > > IIRC I asked you explicitly for the differences between the chips. If > > > there are none, you can use the driver directly, right? :) > > > > Yes. The device tree node for the PT7C4338 device should just say > > > > /* The board has a PT7C4338, which is compatible with the DS1307 */ > > compatible = "dallas,ds1307"; > > While it seems to be 100% compatible, there could be chip-specific > bugs or some interesting features that are hidden behind "reserved" > bits and registers. > > So I think device tree should not lie about the chip model. Doing > 'compatible = "pericom,pt7c4338", "dallas,ds1307"' is perfectly fine. Correct. It's fine (and encouraged) to claim compatibility, but the node should always specify the exact part in the compatible list. > > Note that today the several compatible entries approach gives you > almost nothing, as you will need to add pt7c4338 entry into the driver > anyway. > > I tried to improve this, i.e. make linux do OF-matching on the most > generic compatible entry (the last one): > > http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg21196.html > > It was received coldly though: > > http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg22041.html > http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg21273.html On that note, device-tree-style of_match_table binding now works for i2c devices, so the problems you were having with that thread should now be solved. The of_find_i2c_driver() approach was only ever a heuristic to get things working in the short term. of_match_table is is better in the long run. g.
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index b8f4e9e..c6045dd 100644 --- a/drivers/rtc/Kconfig +++ b/drivers/rtc/Kconfig @@ -126,13 +126,13 @@ comment "I2C RTC drivers" if I2C config RTC_DRV_DS1307 - tristate "Dallas/Maxim DS1307/37/38/39/40, ST M41T00, EPSON RX-8025" + tristate "Dallas/Maxim DS1307/37/38/39/40, ST M41T00, EPSON RX-8025, PT7C4338" help If you say yes here you get support for various compatible RTC chips (often with battery backup) connected with I2C. This driver should handle DS1307, DS1337, DS1338, DS1339, DS1340, ST M41T00, - EPSON RX-8025 and probably other chips. In some cases the RTC - must already have been initialized (by manufacturing or a + EPSON RX-8025, PT7C4338 and probably other chips. In some cases + the RTC must already have been initialized (by manufacturing or a bootloader). The first seven registers on these chips hold an RTC, and other diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c index 4724ba3..8436f16 100644 --- a/drivers/rtc/rtc-ds1307.c +++ b/drivers/rtc/rtc-ds1307.c @@ -4,6 +4,8 @@ * Copyright (C) 2005 James Chapman (ds1337 core) * Copyright (C) 2006 David Brownell * Copyright (C) 2009 Matthias Fuchs (rx8025 support) + * Copyright (C) 2011 Priyanka Jain (Priyanka.Jain@freescale.com) + * (pt7c4338 support) * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as @@ -34,6 +36,7 @@ enum ds_type { ds_1388, ds_3231, m41t00, + pt7c4338, rx_8025, // rs5c372 too? different address... }; @@ -137,6 +140,8 @@ static const struct chip_desc chips[] = { }, [m41t00] = { }, +[pt7c4338] = { +}, [rx_8025] = { }, }; @@ -149,6 +154,7 @@ static const struct i2c_device_id ds1307_id[] = { { "ds1340", ds_1340 }, { "ds3231", ds_3231 }, { "m41t00", m41t00 }, + { "pt7c4338", pt7c4338 }, { "rx8025", rx_8025 }, { } }; @@ -769,6 +775,7 @@ read_rtc: switch (ds1307->type) { case ds_1307: case m41t00: + case pt7c4338: /* clock halted? turn it on, so clock can tick. */ if (tmp & DS1307_BIT_CH) { i2c_smbus_write_byte_data(client, DS1307_REG_SECS, 0);
PT7C4338 chip is being manufactured by Pericom Technology Inc. It is a serial real-time clock which provides: 1)Low-power clock/calendar. 2)Programmable square-wave output. It has 56 bytes of nonvolatile RAM. Its register set is same as that of rtc device: DS1307. Signed-off-by: Priyanka Jain <Priyanka.Jain@freescale.com> --- Changes : This patch will supersede patch: "RTC driver(Linux) for PT7C4338 chip" Incorporting Wolfram Sang's comments to reuse ds1307 driver. drivers/rtc/Kconfig | 6 +++--- drivers/rtc/rtc-ds1307.c | 7 +++++++ 2 files changed, 10 insertions(+), 3 deletions(-)