[rtc-linux] Re: [PATCH] Add support for pt7c4338 (rtc device) in rtc-ds1307 driver

[Grant Likely]

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 at 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.