imx ahci DT updates + cubox-i eSATA support

Avoid matching our child platform device, which can happen as we copy the DT node to the child device. This allows the child platform device to be matched against the ahci-imx driver and should this happen, it will cause another platform device to be allocated, resulting in failure.

Ideally, we shouldn't be copying the of_node, but this is unavoidable as prevents ahci_platform() obtaining its resources.

Signed-off-by Russell King <[email protected]>

---
 drivers/ata/ahci_imx.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/ata/ahci_imx.c +4 -0

index 6a56a561a80b..26ee412127b3 100644
@@ -213,6 +213,10 @@ static int imx_ahci_probe(struct platform_device *pdev)
 	unsigned int reg_val;
 	int ret;
 
+	/* Prevent our child ahci device coming back to us */
+	if (!strcmp(dev_name(&pdev->dev), "ahci"))
+		return -ENODEV;
+
 	of_id = of_match_device(imx_ahci_of_match, dev);
 	if (!of_id)
 		return -EINVAL;

Signature · 7 lines

1.8.3.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 4 lines · Russell King

Avoid matching our child platform device, which can happen as we copy the DT node to the child device. This allows the child platform device to be matched against the ahci-imx driver and should this happen, it will cause another platform device to be allocated, resulting in failure.

This is not the case anymore, since commit 90870d7 (ahci-imx: Port to library-ised ahci_platform) is merged into v3.15-rc1.

Spread-spectrum doesn't work with Cubox-i hardware, so we have to disable this feature. Add a DT property so that platforms can indicate that this feature should not be enabled.

Having it as a negative property keeps existing DT files working.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/boot/dts/imx6q-cubox-i.dts |  1 +
 drivers/ata/ahci_imx.c              | 14 +++++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

arch/arm/boot/dts/imx6q-cubox-i.dts +1 -0

index 941365d7ee65..9efd8b0c8011 100644
@@ -16,4 +16,5 @@
        fsl,transmit-level-mV = <1104>;
        fsl,transmit-boost-mdB = <0>;
        fsl,transmit-atten-16ths = <9>;
+       fsl,no-spread-spectrum;
 };

drivers/ata/ahci_imx.c +13 -2

index 1251d719cc73..f3221b8bfafa 100644
@@ -208,6 +208,7 @@ struct reg_property {
        const struct reg_value *values;
        size_t num_values;
        u32 def_value;
+       u32 set_value;
 };
 
 static const struct reg_value gpr13_tx_level[] = {

@@ -305,6 +306,10 @@ static const struct reg_property gpr13_props[] = {
                .values = gpr13_rx_eq,
                .num_values = ARRAY_SIZE(gpr13_rx_eq),
                .def_value = IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB,
+       }, {
+               .name = "fsl,no-spread-spectrum",
+               .def_value = IMX6Q_GPR13_SATA_MPLL_SS_EN,
+               .set_value = 0,
        },
 };
 

@@ -318,6 +323,14 @@ static u32 imx_ahci_parse_props(struct device *dev,
        for (i = 0; i < num; i++, prop++) {
                u32 of_val;
 
+               if (prop->num_values == 0) {
+                       if (of_property_read_bool(np, prop->name))
+                               reg_value |= prop->set_value;
+                       else
+                               reg_value |= prop->def_value;
+                       continue;
+               }
+
                if (of_property_read_u32(np, prop->name, &of_val)) {
                        dev_info(dev, "%s not specified, using %08x\n",
                                prop->name, prop->def_value);

@@ -392,7 +405,6 @@ static int imx_ahci_probe(struct platform_device *pdev)
                                   IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M |
                                   IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F |
                                   IMX6Q_GPR13_SATA_SPD_MODE_3P0G |
-                                  IMX6Q_GPR13_SATA_MPLL_SS_EN |
                                   reg_value;
        }
 
--

1.8.3.1

Quoted 3 lines · Russell King

Spread-spectrum doesn't work with Cubox-i hardware, so we have to disable this feature. Add a DT property so that platforms can indicate that this feature should not be enabled.

This is for spread-spectrum tx or rx? Transmit SS is optional to support, but the receiver must support SS. Otherwise random drives won't work which makes for a good user experience. Is this really a board quirk rather than a Si issue?

Quoted 10 lines · Rob Herring

Quoted 3 lines · Russell King

Spread-spectrum doesn't work with Cubox-i hardware, so we have to disable this feature. Add a DT property so that platforms can indicate that this feature should not be enabled.

This is for spread-spectrum tx or rx? Transmit SS is optional to support, but the receiver must support SS. Otherwise random drives won't work which makes for a good user experience. Is this really a board quirk rather than a Si issue?

No idea. This bit controls clock generation, and one reason given to disable it is if the reference clock being supplied is already spread spectrum. I don't think that applies here. It doesn't say which clock(s) this is applied to - I would guess it's the transmit clock.

All I know is that with SS enabled, the drive is not detected, and SolidRun's original port disables SS. Disabling SS allows the external drive to be detected.

I have no capability to check the eye pattern, so I've no idea if there's a problem with the electrical setup which stops SS from working. All I know is with the parameters I give here (which are those which SolidRun's original port uses) and with SS disabled, it works.

Quoted 21 lines · Russell King - ARM Linux

Quoted 10 lines · Rob Herring

Quoted 3 lines · Russell King

Spread-spectrum doesn't work with Cubox-i hardware, so we have to disable this feature. Add a DT property so that platforms can indicate that this feature should not be enabled.

This is for spread-spectrum tx or rx? Transmit SS is optional to support, but the receiver must support SS. Otherwise random drives won't work which makes for a good user experience. Is this really a board quirk rather than a Si issue?

No idea. This bit controls clock generation, and one reason given to disable it is if the reference clock being supplied is already spread spectrum. I don't think that applies here. It doesn't say which clock(s) this is applied to - I would guess it's the transmit clock.

All I know is that with SS enabled, the drive is not detected, and SolidRun's original port disables SS. Disabling SS allows the external drive to be detected.

Quoted 5 lines

I have no capability to check the eye pattern, so I've no idea if there's a problem with the electrical setup which stops SS from working. All I know is with the parameters I give here (which are those which SolidRun's original port uses) and with SS disabled, it works.

I'll correct that - it _is_ detected with SS enabled, but things go awry very quickly with errors, which then result in corrupted IDENTIFY responses, the link dropping back to 1.5Gbps, more errors and corruption and eventually the SATA layer gives up and declares the port dead.

Quoted 28 lines · Russell King - ARM Linux

Quoted 21 lines · Russell King - ARM Linux

Quoted 10 lines · Rob Herring

Quoted 3 lines · Russell King

Spread-spectrum doesn't work with Cubox-i hardware, so we have to disable this feature. Add a DT property so that platforms can indicate that this feature should not be enabled.

This is for spread-spectrum tx or rx? Transmit SS is optional to support, but the receiver must support SS. Otherwise random drives won't work which makes for a good user experience. Is this really a board quirk rather than a Si issue?

No idea. This bit controls clock generation, and one reason given to disable it is if the reference clock being supplied is already spread spectrum. I don't think that applies here. It doesn't say which clock(s) this is applied to - I would guess it's the transmit clock.

All I know is that with SS enabled, the drive is not detected, and SolidRun's original port disables SS. Disabling SS allows the external drive to be detected.

Quoted 5 lines

I have no capability to check the eye pattern, so I've no idea if there's a problem with the electrical setup which stops SS from working. All I know is with the parameters I give here (which are those which SolidRun's original port uses) and with SS disabled, it works.

Quoted 4 lines

I'll correct that - it _is_ detected with SS enabled, but things go awry very quickly with errors, which then result in corrupted IDENTIFY responses, the link dropping back to 1.5Gbps, more errors and corruption and eventually the SATA layer gives up and declares the port dead.

Rob,

Can I take your lack of reply on this as meaning that you don't have any objections against these new DT properties for this driver - if so, can I have your ack for these properties please?

Yes, they are okay. I'll ack the documentation once that is done.