On Tue, Jan 19, 2010 at 03:46:20AM -0500, Mike Frysinger wrote:
> On Tue, Jan 19, 2010 at 03:26, Dmitry Torokhov wrote:
> > On Mon, Jan 18, 2010 at 11:52:44PM -0500, Mike Frysinger wrote:
> >> struct ad7879 {
> >> - bus_device *bus;
> >> + struct ad7879_bus_ops bops;
> >
> > Could you change it to 'const struct ad7879_bus_ops *bops;' and add a
> > separate pointer to private transport data so that bus ops are truly
> > shared between instances (if any).
>
> i can make the struct containing only the funcs const, but not the
> struct that contains the irq/client since i dynamically assign the
> contents in the probe func
Maybe it should be 2 structs? One for the ops and one opaque to the
ad7879 core containing transport-private data?
>
> > Also I dont see the reason for having the following typedefs:
>
> it made sense in the other drivers i copied this format from, and in
> an earlier version of the code, but they arent needed anymore, so i'll
> drop em
>
> >> +typedef int (ad7879_read_t) (void *bus_data, u8 reg);
> >> +typedef int (ad7879_multi_read_t) (void *bus_data, u8 first_reg, u8
> >> count, u16 *buf);
> >> +typedef int (ad7879_write_t) (void *bus_data, u8 reg, u16 val);
> >> +
> >> +struct ad7879_bus_ops {
> >> + void *bus_data;
> >> + int irq;
> >> + ad7879_read_t *read;
> >> + ad7879_multi_read_t *multi_read;
> >> + ad7879_write_t *write;
> >> +};
> >> +
> >> +int ad7879_disable(struct device *dev);
> >> +int ad7879_enable(struct device *dev);
> >> +int ad7879_probe(struct device *dev, struct ad7879_bus_ops *bops, u8
> >> devid, u16 bustype);
> >> +int ad7879_remove(struct device *dev);
> >
> > Hmm, I liked when these accepted "struct ad7879" better.
>
> i toyed with that a bit, but the generated code was worse because the
> drvdata helpers are externals
Hm, it would only make overall difference when you have both transports
loaded, right? Oh well, I don't really have strong preference either
way.
--
Dmitry
_______________________________________________
Uclinux-dist-devel mailing list
[email protected]
https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel
