Re: [PATCH] Add hook for custom xfer function in PATA Platform driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Graeme Russ wrote:

I don't think ide-platform matters in this case.
  You've just supported a patch restoring feature parity between
pata_platform and ide-platfrom WRT IRQ flags. ;-)
Because it was trivial to do.
  Yes, but this needs to be *done* too, preferrably by the author of the
original patch and simultaneously with it. We can't accept a single patch
that only touches pata_platform.
I'm happy to expand the patch,  but why would we implement such a thing in
a
legacy driver.

  Because the drivers are intended to be interchangeable.

I thought the whole purpose of marking a driver 'legacy' was to no longer
enhance its features and encourage people to migrate away from it so that
one day it can be removed entirely.

I also think I have a massive understanding gap with the idea of
interchangeability between IDE and ATA (please excuse me for that) - I
thought that the pata_platform driver is designed to be 'hard-wired' for
embedded systems (i.e. the board developer explicitly sets up address
ranges etc used for initialization in the board init function). I didn't
think interchangeability with another driver platform was a goal.

The 'ide_platform' driver handles exactly the same platform device that 'pata_platform' does, so that the drivers can be swapped over without changing anything in the code. They are quite coupled together, so a change in one driver should ensue a parallel change in the other one; or at least, we should explicitly mark the new 'pata_platform' driver feature as unsupported in the 'ide_platform' driver -- one way or another, the latter driver needs to be aware of it.

Odds are that nobody would ever use it.
  As I said a choice of libata over IDE might not be so obvious advantage as
some people tend to think. The ide-platfrom is still used in embedded, given
that it's actually quite young driver.

If I had developed
my code prior to pata_platform and used the old IDE driver instead I would
of
  Can't parse "would of" -- did you mean "would have"? :-)

Yes, (Aussie English) ;-)

Strange, I've also encountered that with non-Australian speakers. Must be some form of English slang...

patched it then (and it may have been included in pata_platform).

  You'll be surprised to know that pata_platfrom actually *predates*
ide-platfrom. ;-)

Yes, I am

Now you should see that there were a need in the IDE driver still, despite the libata one already existed. So MontaVista had to write it (although some form of IDE platform driver had existed for some time out of tree -- it was called something like ide-cf, and was written exectly to support various kinds of CF encountered on the embedded boards).

Sure - or support it in both. I've no problem with both supporting that
function, just with legacy code blocking progress.
  Well, with 8250 we still don't allow for overriding things at the
board level, via the platform data callbacks.
Oh yes we do. Platform specific drivers can directly override the access
operators and they do some truely horrible platform specific stuff in the
overridden operators.

  That's something new to me. Looking at <linux/serial_8250.h>, you're
right. But this got added only in January, due to Cavium.

And the great thing - nobody else has to know what the board vendors have
been on..

  Well, mainly SoC vendors...

Exactly - This is good driver design. Implement the fundamentals for the
majority of use-cases and provide overrides to allow individual
implementations which can do anything weird and wonderful without
polluting
the code space of everyone else.

  You should have looked at 8250.c first -- there's still lot of pollution
left because old accessors were all left in place, including the unfortunate
Alchemy UART's ones. :-)

I've had a quick look at 8250 and, yes, the serial_in / serial_out overrides
are exactly what I have proposed for PATA Platform. I do not see how this
could be considered bad driver design IMHO.

As I've said, the platform data overrides were a recent addition, before that the driver was "polluted" by e.g. the Alchemy UART accessors (which were result of merging support for this "not very compatible" UART from a separate driver -- which just blindly copied most of 8250.c). This stuff still pollutes 8250.c, although it should be hidden by the overrides now...

As I said before, provide
baseline functionality for the majority (80%), designed-in extensibility
for the minority (15%) and force the last 5% into writing their own driver
as a lesson to them for poor hardware design decisions ;)

   It's only not clear where to draw a line between those 15% and 5%... :-)

Regards,

Graeme

MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux