2002-01-14 13:54:31

by Ian Molton

[permalink] [raw]
Subject: IDE code

Hi.

in the ide code the functions ide_[input|output]_data() seem to have become
polluted by a test for a 'helper function' to be used to read data instead
of inb() and friends.

It is my opinion that the functions should not have the test and
subroutine, as it violates the 'layering' in that they no longer just 'get
the data' but now also test for ... etc. blah blah.

I propose that we should be using the already existing function pointers to
allow the CALLERS of ide_[input|output]_data() to perform the condition,
and branch if appropriate to a driver provided alternative function.

This would mean replacing the handful (literally) of ide_input_data() and
ide_output_data() calls with a macro (say, IDE_INPUT_DATA()) which performs
the condition, or, if not needed, can be defined away as a straight call to
ide_input_data() etc.

As the writer of an ide card driver that needs this functionality (it uses
a multiply mapped register concept on ARM hardware to take advantage of its
multiple load/store instructions for PIO (the hardware cannot do DMA, its
15 years old :)), I would appreciate input on wether or not anyone else
feels this change should / should not be made?


2002-01-14 17:17:22

by Bjorn Wesen

[permalink] [raw]
Subject: Re: IDE code

On Mon, 14 Jan 2002, Ian Molton wrote:
> in the ide code the functions ide_[input|output]_data() seem to have become
> polluted by a test for a 'helper function' to be used to read data instead
> of inb() and friends.

I made that change, and I thought about letting the caller check it, but
this was a much cleaner way to fix it overall given that it needed to go
in the 2.4 stable series. Actually, the caller having to check driver
details is not very nice at all. It has to do this for the dmaproc because
the DMA and PIO modes are fundamentally different, but in this case it's
probably better if the caller does not have to bother.

Now, what is lacking in the current implementation is that the caller
should use the hw ptr directly instead of first calling the ide_xx_data
which subsequently checks for an override, and ide_xx_data could be put as
default in that ptr. But that would require changing in a lot of places in
a very mission critical piece of code - it can be fixed during 2.5 of
course.

> As the writer of an ide card driver that needs this functionality (it uses
> a multiply mapped register concept on ARM hardware to take advantage of its
> multiple load/store instructions for PIO (the hardware cannot do DMA, its
> 15 years old :)), I would appreciate input on wether or not anyone else
> feels this change should / should not be made?

In what way, exactly, does the current mechanism stop you from doing this?

/BW