On 8/28/08, David Brownell <[EMAIL PROTECTED]> wrote:
> On Wednesday 27 August 2008, Jon Smirl wrote:
> > On 8/27/08, David Brownell <[EMAIL PROTECTED]> wrote:
> > > On Tuesday 26 August 2008, Trent Piepho wrote:
> > > > Lots of tv cards have eeproms with configuration information. They
> use an
> > > > I2C driver called tveeprom that exports this:
> > > >
> > > > int tveeprom_read(struct i2c_client *c, unsigned char *eedata, int
> len)
> > >
> > >
> > > That presumes the driver somehow has access to that I2C client.
> >
> > How about using bus_find_device? PowerPC uses it to locate the i2c
> > device using pointers in the device tree.
>
>
> Still making too many assumptions for my taste ... like using
> I2C, and in this case also OF. What I want is an approach that
> can work with all the drivers with EEPROM and NVRAM support
> that I've happenened on so far:
>
>
> drivers/i2c/chips/at24.c
>
> drivers/spi/chips/at25.c
> drivers/rtc/rtc-cmos.c
> drivers/rtc/rtc-ds1305.c
> drivers/rtc/rtc-ds1307.c
> drivers/rtc/rtc-ds1511.c
> drivers/rtc/rtc-ds1553.c
> drivers/rtc/rtc-ds1742.c
> drivers/rtc/rtc-m48t59.c
> drivers/rtc/rtc-stk17ta8.c
>
> All those could easily export a (renamed) "struct at24_iface *" so
> their persistent (and updatable) memory could be exported to kernel
> code using the very same (simple) interface.
>
> Discussion about how to use a different interface than what's shown
> in the $SUBJECT patch seems to want to promote (a) each of those ten
> drivers having a *different* interface exposed by EXPORT_SYMBOL(),
> or else don't support in-kernel access at all, and (b) a bunch of
> extra glue code to support it, like the OF-specific bits you showed
> below and then going to the code that knows to use those bits to
> get which device, and how to ask those devices for their data.
>
> Moreover, IMO the basic question is still whether there is a good
> reason not to build on the $SUBJECT patch. (So far: no.)
>
> Sure it *could* be done differently -- especially if think (a) is
> good but being able to reuse interfaces is bad -- but it's like
> spending so much time choosing what color to paint the bikeshed
> that the bikeshed itself never gets built...
There are two parts to the puzzle.
1) A common way to export the interface. That can be addressed by
applying the technique in the initial patch.
2) How do the drivers find each other. Device trees already have a way
for drivers to find each other. This is an example of a audio codec
that lives on both the i2c and i2s bus. The codec-handle is used to
retrieve the tas0 node pointer. of_find_i2c_device_by_node then
searches the archdata for that pointer.
struct i2c_client *of_find_i2c_device_by_node(struct device_node
*node) could be generalized by having it search multiple buses for the
device. dev->archdata.of_node is just a cookie, it is only used to
match against. The pointer could be a void*.
I didn't like setup call part of the proposal. It is building another
way for drivers to find each other. How can you generate an equivalent
to the archdata.of_node cookie for other platforms? Another problem
with the setup callback, how do you tell identical devices apart?
[EMAIL PROTECTED] { /* PSC2 in i2s mode */
compatible =
"fsl,mpc5200b-psc-i2s","fsl,mpc5200-psc-i2s";
cell-index = <1>;
reg = <0x2200 0x100>;
interrupts = <0x2 0x2 0x0>;
interrupt-parent = <&mpc5200_pic>;
codec-handle = <&tas0>;
};
[EMAIL PROTECTED] {
#address-cells = <1>;
#size-cells = <0>;
compatible =
"fsl,mpc5200b-i2c","fsl,mpc5200-i2c","fsl-i2c";
cell-index = <0>;
reg = <0x3d00 0x40>;
interrupts = <0x2 0xf 0x0>;
interrupt-parent = <&mpc5200_pic>;
fsl5200-clocking;
tas0:[EMAIL PROTECTED] {
compatible = "ti,tas5504";
reg = <0x1b>;
};
clock0:[EMAIL PROTECTED] {
compatible = "maxim,max9485";
reg = <0x68>;
};
};
>
> - Dave
>
> p.s. The only nontrivial technical issue I have with $PATCH is that
> "struct at24_iface" needs to cope better with "rmmod at24".
> An optional teardown(...) call would suffice.
>
>
>
> > +static int of_dev_node_match(struct device *dev, void *data)
> > +{
> > + return dev->archdata.of_node == data;
> > +}
> > +
> > +struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
> > +{
> > + struct device *dev;
> > +
> > + dev = bus_find_device(&i2c_bus_type, NULL, node,
> > + of_dev_node_match);
> > + if (!dev)
> > + return NULL;
> > +
> > + return to_i2c_client(dev);
> > +}
> > +EXPORT_SYMBOL(of_find_i2c_device_by_node);
> > +
>
>
--
Jon Smirl
[EMAIL PROTECTED]
_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c
