On Mon, 18 Aug 2008 03:27:39 +0200, Herbert Poetzl wrote:
> On Sun, Aug 17, 2008 at 10:45:11PM +0200, Jean Delvare wrote:
> > On Fri, 15 Aug 2008 22:50:12 +0200, Herbert Poetzl wrote:
> > > (...)
> > > +static void port_dor_out(unsigned char d)
> > > +{
> > > + outb(d ^ LO_INV, base + FOFF_DOR);
> > > +}
> > > +
> > > +static unsigned char port_dir_in(void)
> > > +{
> > > + return inb(base + FOFF_DIR) ^ LI_INV;
> > > +}
> >
> > These two functions are certainly worth inlining.
>
> okay, static inline fine?
Yes.
> > (...)
> > In general, "remove" functions should avoid accessing globals. You
> > should always be able to get access to everything you need based on the
> > device that is passed into parameter. Just use platform_get_drvdata
> > (and use platform_set_drvdata in the "probe" function.)
>
> hmm, is that something you plan to change in the
> i2c-parport-light driver too, if so, please point
> me to the patch, I'll gladly adjust it to match that
In fact, I was counting on you to fix your driver and then send a patch
cleaning up i2c-parport-light in a similar way ;)
> > (...)
> > This lacks a warning that this driver is meant for do-it-yourself
> > hardware and most people can safely say no.
> >
> > Out of curiosity, what would happen if someone was to run this driver
> > with actual floppy disk drives connected?
>
> I'd say the drive(s) will spin up a little, but
> that's about all that can happen, the driver doesn't
> do anything 'bad' with the floppy controller
No risk that the relatively fast motor on/off switching could damage it?
> the other way round is probably more a problem, as
> the floppy driver will not handle the changing input
> line that gracefully, but that is a problem users
> of this kind of hardware should be able to deal with
Correct, but that's still worth a warning in the documentation. Better
safe than sorry.
> > (...)
> > Your patch misses Documentation/i2c/busses/i2c-floppy explaining
> > how to build the hardware, things to pay attention to, etc.
>
> okay, something like the i2c-parport-light is fine?
> (including the pinout description of course)
Yes.
--
Jean Delvare
_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c
