imx6 eSATA

[Russell King - ARM Linux]

On Sat, Jan 18, 2014 at 06:44:27PM +0000, Russell King - ARM Linux wrote:
> So, I see we have AHCI support for SATA on the iMX6.  Great... but it
> doesn't work on the cubox-i, because the phy settings are wrong.

And another thing.  This is wonderful... really wonderful.

static int imx_ahci_probe(struct platform_device *pdev)
{  
        struct device *dev = &pdev->dev;
...
        ahci_pdev = platform_device_alloc("ahci", -1);
...
        ahci_dev = &ahci_pdev->dev; 
...
        ahci_dev->of_node = dev->of_node;

This is a hanging offence.

Here's a lesson in how DT matching works:

- A device gets declared into the device model.
- The device is offered to each driver in turn via the bus specific
  code to see whether the driver should be bound to the device.
- If there's an of_node present, the DT IDs for the driver are walked
  to compare the device with the driver's DT IDs.  If a match is found,
  the device is passed to the driver probe function.
- If there is no of_node present, and it's a platform device, the bare
  device name is compared the driver name(s) and if it matches the
  probe function is called.

Now, what does this mean for the above monstrosity?  If the driver
model happens to find the ahci_imx driver _before_ the ahci driver
while trying to bind the ahci_dev, it will find that the ahci_dev
has an of_node with a compatible string which matches this driver.
So, imx_ahci_probe() _can_ be called with the ahci_pdev that it
just created.

It doesn't take much to understand what the result will be of that.
It will try to create another ahci device... hopefully this time
erroring out.

This is utterly disgusting.  You must *never* *ever* assign an of_node
from one device to another of the same bus type.  If you need to pass
the of_node to another device, then it _must_ be done outside of the
child device's of_node pointer - in other words, it must be done using
platform data.

Alternatively, turn ahci into a library that both the original ahci
and ahci_imx drivers can use without jumping through these kinds of
games - or in this case, just get rid of that assignment - I can't
see anything in ahci.c which needs the of_node.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".