Re: arm-lh7a40x IDE support in 2.6.6

[Bartlomiej Zolnierkiewicz <B.Zolnierkiewicz@elka.pw.edu.pl>]

On Friday 14 of May 2004 21:47, Marc Singer wrote:

> > [ you used 'struct ide_hwif_s' in arch-lh7a40x/ide.h to workaround this
> > 8) ]

struct hwif_s actually

> Copied from elsewhere.

superio.h, superio.c

EEK, added to TODO

> Listen.  It is not my intention to be clever.  All I want to do is get
> things working and to not break other people's code.  I'm certain that
> most people are working with the same assumptions.  Aside from some
> naive snippets I've see promulgated, the bulk of the kernel work I've
> see is sane given limited information.  Certainly, once one
> understands a sybsystem completely then the quality rises.  I hope
> you'll admit that the IDE code is overly intricate.

With changes like this nobody will ever be able to understand
IDE subsystem completely. ;-)

> > - you are setting IDE_NO_IRQ in ide_init_hwif_ports() which is used
> >   in many places in generic IDE code - anybody wanting to understand
> >   interactions with your code + generic code will have serious
> >   problems (especially if knows _nothing_ about lpd7a40x)
>
> I don't know what you mean.  I grep for that constant and found it
> nowhere except for ide-io.c and in my code.  It doesn't take much to
> find the references.

I'm talking about ide_init_hwif_ports() function.

> What is it that you want changed?
>
> > - hwif->mmio is set to 2 but resource handling is missed
>
> Can you be more specific?  Did you notice the comment?  I didn't know
> what it was for, but I set it because I thought that that was the
> right way to go.
>
> Are you talking about reserving the address space?  At the moment,

Yes.

> this is done in the arch setup.  I can certainly move it to the IDE
> driver.

OK

> > > The OUTB breaks my interface because I don't really have byte-level
> > > access to the resgisters.  So, is selectproc a pre-select procedure or
> > > should it be a substitute?
> >
> > pre-select but you can change it to be substitute if you need
> > (just remember to update all users if you decide to do this)
>
> I'll have to search the kernel to see what uses it.  Maybe the better
> way would be to define a new select proc that *is* a substitute.

Nope.

> > IMO it is better to fix it correctly than to do hacks like this
> > in lpd7a40x_ide_outb() (which is minor performance hit btw)
>
> Of course.
>
> > > Anyway, that is what I did in a nutshell.  I plan to get back to this
> > > in a week or so.  Since Russell King already integrated the lh7a40x
> > > code into the kernel, this stuff should be easy to test.
> >
> > That's what I'm talking about - it shouldn't have been integrated. :-)
>
> I think we are talking about two things here.  I agree that the
> ide-lpd7a400 code should not have been integrated.  However, the rest

OK

> of the core code is fine.  It is that core code that is necessary to
> make the test possible.

Stuff in arch-lh7a40x/ide.h is really a driver code but
abuses subsystem code instead - that's my complain.

OK, lets move things forward and just fix it.

Cheers.