pata_marvell: Undo the regressions Jeff caused

From: Alan Cox <alan@redhat.com>

I've been chasing Jeff about this for months so am now giving up and sending
it directly. Jeff added the Marvell device identifiers to the ahci driver
without making the AHCI driver handle the PATA port. This means a lot of users
can't use current kernels and in most distro cases can't even install. This has
been going on since March 2008 for the 6121 Marvell, and late 2007 for the 6145.

This was all pointed out at the time and repeatedly ignored. Bugs assigned
to Jeff about this are ignored also.

To quote Jeff in email

"Just switch the order of 'ahci' and 'pata_marvell' in
/etc/modprobe.conf, then use Fedora's tools regenerate the initrd.

See?  It's not rocket science, and the current configuration can be
easily made to work for Fedora users."

(Which isn't trivial, isn't end user, shouldn't be needed, and as it usually
breaks at install time is in fact impossible)

To quote Jeff in August 2007

"   mv-ahci-pata
Marvell 6121/6141 PATA support.  Needs fixing in the 'PATA controller
command' area before it is usable, and can go upstream."

Only he applied it all anyway later and broke everything.

The actual fix for the moment is very simple. If the user has included
the pata_marvell driver let it drive the ports. If they've only selected
for SATA support give them the AHCI driver which will run the port a fraction
faster.


Signed-off-by: Alan Cox <alan@redhat.com>
---

 drivers/ata/Kconfig |    6 ++++--
 drivers/ata/ahci.c  |    5 +++++
 2 files changed, 9 insertions(+), 2 deletions(-)


diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index ae84949..11c8c19 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -448,8 +448,10 @@ config PATA_MARVELL
 	tristate "Marvell PATA support via legacy mode"
 	depends on PCI
 	help
-	  This option enables limited support for the Marvell 88SE6145 ATA
-	  controller.
+	  This option enables limited support for the ...
[ message continues ]

Completely and utterly false.  libata-dev.git#mv-ahci-pata has not yet 
gone upstream, as is blatantly clear if you had bothered to LOOK.

It helps to look at source code before posting, Alan:

         /*
          * Temporary Marvell 6145 hack: PATA port presence
          * is asserted through the standard AHCI port
          * presence register, as bit 4 (counting from 0)
          */
         if (hpriv->flags & AHCI_HFLAG_MV_PATA) {
                 if (pdev->device == 0x6121)
                         mv = 0x3;
                 else
                         mv = 0xf;

Clearly PATA support is not applied, because the code masks out the PATA 

You omit details that hurt your case, namely that a ton of users were 
asking for AHCI support EVEN IF IT IS SATA ONLY, because the SATA 
support via pata_marvell legacy mode was so poor and problematic.

Among the pata_marvell complaints I received that were solved were:

* bloody awful error handling, with no hotplug support
* problems with suspend/resume (apparently some BIOS assumed you were in 
AHCI/enhanced mode)
* problems with speed negotiation of 3.0 Gbps SATA devices

So any claim that SATA always worked wonderfully under pata_marvell is 
specious.




this prevents users from choosing the driver that works best for them, 
including bringing back all the SATA problems just as you bring back PATA.

The attached patch is more reasonable, but neither your nor my patch 
actually address the relevant problem:  distros choosing the module.

So at this point, applying your patch would create regressions just as 
it purports to solve regressions, since Marvell AHCI is out there in 
active use as well.


How to move forward?  I recommend,

1) Applying my attached patch

2) Communicating with distros

3) See if we can get Marvell docs to someone willing to code in support. 
  Marvell has been very responsive in the past year with regards to 
'mvsas', so maybe they can help here too.

I kept hoping Marvell ...
[ message continues ]

Oh I beg your pardon - you only pushed the PCI identifiers that broke
stuff rather than the code that went with it you never finished. So does
that actually sound better "hey this was broken, not merged but I pushed
the IDs part *knowing* it would cause serious regressions" ?

Bit rich telling me to LOOK (emphasis yours) when your last email says

"Situation before ahci mod:

	Users with PATA may use pata_marvell.
	Users wanting SATA screwed."


Actually it just works. You don't get Phy diagnostics that is about all

Full SFF error handling. Warmplug but not automatic hotplug detect. But
hey at least you *can install the damned machine* and the machine that
was working in FC 8 might actually still be working.

(But you clearly didn't know this because your last email to linux-ide


Not really, its in the config script. Oh that hard for users, then tell
me why 'editing the initrd' before installing is acceptable ?

At runtime it might make sense to allow ahci.marvell=1. I can add that in
about 60 seconds if you want now you've finally been forced to stop
ignoring the issue. That would allow the distros to ship pata_marvell and
users to say "actually I want full SATA support".

Unfortunately there isn't a documented way to check the 6121 PATA port

Currently distributions can't compile in pata_marvell and remove the

A mess *you* created by repeatedly ignoring the problems that were
pointed out to you and then ignoring attempts to raise it. Meanwhile I've
been getting zillions of bug reports and emails about it. You dug the
hole and ran away with your fingers in your ears while I got all the bugs

Your patch is a no-op. No distribution of any note builds them
non-modular. You know this so I find it incredulous you propose this

Well you've been ignoring bugzilla for months so doing something about

or to provide docs on the pata port. I'm assuming its probably as trivial
as "issue one command at a time". In SFF mode the chip does all the
management itself so it's ...
[ message continues ]

> 1) Applying my attached patch

This is another way of doing it that again unlike your joke 'patch'
actually isn't a pointless no-op. Push the decision into a config
variable with module option. Also document that the PATA port is being
disabled so the end user knows who screwed up their machine and how to
undo it.

Default is pata_marvell = use this driver, no pata_marvell use AHCI, but
the default can be reversed in either direction using
ahci.marvell_enable=0/1.

--

pata_marvell: Undo multiple ahci caused regressions

From: Alan Cox <alan@redhat.com>

I've been chasing Jeff about this for months so am now giving up and sending
it directly. Jeff added the Marvell device identifiers to the ahci driver
without making the AHCI driver handle the PATA port. This means a lot of users
can't use current kernels and in most distro cases can't even install.

This has been going on since March 2008 for the 6121 Marvell, and late 2007
for the 6145!!!

This was all pointed out at the time and repeatedly ignored. Bugs assigned
to Jeff about this are ignored also.

To quote Jeff in email

"Just switch the order of 'ahci' and 'pata_marvell' in
/etc/modprobe.conf, then use Fedora's tools regenerate the initrd.

See?  It's not rocket science, and the current configuration can be
easily made to work for Fedora users."

(Which isn't trivial, isn't end user, shouldn't be needed, and as it usually
breaks at install time is in fact impossible)

To quote Jeff in August 2007

"   mv-ahci-pata
Marvell 6121/6141 PATA support.  Needs fixing in the 'PATA controller
command' area before it is usable, and can go upstream."

Only he add the ids anyway later and caused regressions, adding a further
id in March causing more regresions.

The actual fix for the moment is very simple. If the user has included
the pata_marvell driver let it drive the ports. If they've only selected
for SATA support give them the AHCI driver which will run the port a fraction
faster. Allow the user to control ...
[ message continues ]

[snip]

That's far more reasonable, and I'd be happy to queue that with your 
full email text...

Unless you object, I'll roll this up with a couple other libata fixes 
for 2.6.27-rc.

	Jeff



--

[ message continues ]

And here's a rev 3

- Fix the option so it works both ways
- We can using the 6145 detect PATA enabled so implement a facility in
pata_marvell to punt to AHCI if disabled

We can't automate that for the other devices as we don't actually know
how to check the PATA port on them.

--

pata_marvell: Undo multiple ahci caused regressions

From: Alan Cox <alan@redhat.com>

I've been chasing Jeff about this for months so am now giving up and sending
it directly. Jeff added the Marvell device identifiers to the ahci driver
without making the AHCI driver handle the PATA port. This means a lot of users
can't use current kernels and in most distro cases can't even install.

This has been going on since March 2008 for the 6121 Marvell, and late 2007
for the 6145!!!

This was all pointed out at the time and repeatedly ignored. Bugs assigned
to Jeff about this are ignored also.

To quote Jeff in email

"Just switch the order of 'ahci' and 'pata_marvell' in
/etc/modprobe.conf, then use Fedora's tools regenerate the initrd.

See?  It's not rocket science, and the current configuration can be
easily made to work for Fedora users."

(Which isn't trivial, isn't end user, shouldn't be needed, and as it usually
breaks at install time is in fact impossible)

To quote Jeff in August 2007

"   mv-ahci-pata
Marvell 6121/6141 PATA support.  Needs fixing in the 'PATA controller
command' area before it is usable, and can go upstream."

Only he add the ids anyway later and caused regressions, adding a further
id in March causing more regresions.

The actual fix for the moment is very simple. If the user has included
the pata_marvell driver let it drive the ports. If they've only selected
for SATA support give them the AHCI driver which will run the port a fraction
faster. Allow the user to control this decision via ahci.marvell_enable as
a module parameter so that distributions can ship 'it works' defaults and
smarter users (or config tools) can then flip it over it ...
[ message continues ]

Nice... superceding your rev 2 in my queue for 2.6.27-rc.


--

[ message continues ]

I happen to have one of the others:
 	Marvell Technology Group Ltd. 88SE6121 SATA II Controller (rev b1)

It is not in use, likely as a result of the issues at hand, I don't
recall exactly what transpired at the time.

I have no specs, but would be happy to run any software diagnostics
if they'd be of help
-- 
Rick Nelson
"I would rather spend 10 hours reading someone else's source code than
10 minutes listening to Musak waiting for technical support which isn't."
(By Dr. Greg Wettstein, Roger Maris Cancer Center)
--

[ message continues ]

On Wed, 3 Sep 2008 11:54:36 -0700 (PDT)

Load the pata_marvell driver and it dumps out BAR5: .... lists of values

Can you do that with a PATA device attached and with no PATA device
attached and see what changes.
--

[ message continues ]

applied.  I edited the text a bit, for formatting and clarity only (see 
attached)

[ message continues ]

>   ahci, pata_marvell: play nicely together

I can't see how that's going to work with a 6145 adapter and no PATA devices
attached. The AHCI driver will always defer to pata_marvell (unless
marvell_enable=1 is passed) and pata_marvell will defer to ahci when it doesn't
see any PATA devices.

--

[ message continues ]