Messages in this thread | | | From | Bartlomiej Zolnierkiewicz <> | Subject | Re: arm-lh7a40x IDE support in 2.6.6 | Date | Sat, 15 May 2004 00:19:46 +0200 |
| |
On Friday 14 of May 2004 23:33, Marc Singer wrote:
> > > 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. ;-) > > I get the feeling that you're blaming me and others for making the IDE > code a mess. Might I suggest that this isn't a very productive tack?
Yep, sorry.
> The most helpful thing to do is to a) provide best-practice examples, > and b) to include some documentation. I'm not talking about anything > extensive, but statements like > > "All references to linux/ide.h must reside in the ide tree." > > Are pretty darn helpful.
OK, will do (this can take a while). Can I ask you to review it later? I think your comments will be very valuable.
> > > > - 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. > > Most of the ARM arch's use it. Perhaps all of them need a good once > over.
Since some time I have a patch killing <asm/arch-*/ide.h>. :)
> > > > > 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. > > So then we break anyone who is using the selectproc as a pre-select > proc? I don't understand what you mean here. There are several > drivers that use this function. How do you propose that we provide > both types of behavior with one entry point?
You can add last line of SELECT_DRIVE() to all ->selectproc() implementations and add 'else' to SELECT_DRIVE().
> > > 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. > > Right. We agree. I am talking about the core code. Not the ide > code. The core is what supports the CPU.
Good.
Cheers, Bartlomiej
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |