[PATCH 0/5] imx ahci DT updates + cubox-i eSATA support

[PATCH 0/5] imx ahci DT updates + cubox-i eSATA support

[Russell King - ARM Linux]

The following series adds several DT properties to the iMX ahci driver,
which are necessary to configure the electrical characteristics of the
SATA interface.

The required electrical characteristics are board dependent, so the
existing solution where the parameters are hard-coded for the first
board(s) which came along is completely rediculous, and cause their
own set of problems: we have to default to these parameters when no
properties are given, and it means we have to use negative properties
to turn stuff off rather than positive properties to enable features.

Yes, I know that the required Documentation/devicetree file is missing,
I couldn't find the existing file to update for this driver with the
new properties. :)

 arch/arm/boot/dts/imx6q-cubox-i.dts |   4 +
 drivers/ata/ahci_imx.c              | 188 ++++++++++++++++++++++++++++++++++--
 2 files changed, 184 insertions(+), 8 deletions(-)

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH RFC 4/5] ARM: cubox-i: add eSATA DT configuration

[Russell King]

Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
---
 arch/arm/boot/dts/imx6q-cubox-i.dts | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/imx6q-cubox-i.dts b/arch/arm/boot/dts/imx6q-cubox-i.dts
index bc5f31e3e892..941365d7ee65 100644
--- a/arch/arm/boot/dts/imx6q-cubox-i.dts
+++ b/arch/arm/boot/dts/imx6q-cubox-i.dts
@@ -13,4 +13,7 @@
 
 &sata {
 	status = "okay";
+	fsl,transmit-level-mV = <1104>;
+	fsl,transmit-boost-mdB = <0>;
+	fsl,transmit-atten-16ths = <9>;
 };
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum

[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.

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

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/boot/dts/imx6q-cubox-i.dts |  1 +
 drivers/ata/ahci_imx.c              | 14 +++++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6q-cubox-i.dts b/arch/arm/boot/dts/imx6q-cubox-i.dts
index 941365d7ee65..9efd8b0c8011 100644
--- a/arch/arm/boot/dts/imx6q-cubox-i.dts
+++ b/arch/arm/boot/dts/imx6q-cubox-i.dts
@@ -16,4 +16,5 @@
 	fsl,transmit-level-mV = <1104>;
 	fsl,transmit-boost-mdB = <0>;
 	fsl,transmit-atten-16ths = <9>;
+	fsl,no-spread-spectrum;
 };
diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
index 1251d719cc73..f3221b8bfafa 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -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

Re: [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum

[Rob Herring]

On Wed, Apr 16, 2014 at 3:43 AM, Russell King
<rmk+kernel@arm.linux.org.uk> wrote:
> 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?

Rob

>
> Having it as a negative property keeps existing DT files working.
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>  arch/arm/boot/dts/imx6q-cubox-i.dts |  1 +
>  drivers/ata/ahci_imx.c              | 14 +++++++++++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/imx6q-cubox-i.dts b/arch/arm/boot/dts/imx6q-cubox-i.dts
> index 941365d7ee65..9efd8b0c8011 100644
> --- a/arch/arm/boot/dts/imx6q-cubox-i.dts
> +++ b/arch/arm/boot/dts/imx6q-cubox-i.dts
> @@ -16,4 +16,5 @@
>         fsl,transmit-level-mV = <1104>;
>         fsl,transmit-boost-mdB = <0>;
>         fsl,transmit-atten-16ths = <9>;
> +       fsl,no-spread-spectrum;
>  };
> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
> index 1251d719cc73..f3221b8bfafa 100644
> --- a/drivers/ata/ahci_imx.c
> +++ b/drivers/ata/ahci_imx.c
> @@ -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
>

Re: [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum

[Russell King - ARM Linux]

On Wed, Apr 16, 2014 at 05:46:47PM -0500, Rob Herring wrote:
> On Wed, Apr 16, 2014 at 3:43 AM, Russell King
> <rmk+kernel@arm.linux.org.uk> wrote:
> > 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.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum

[Russell King - ARM Linux]

On Wed, Apr 16, 2014 at 11:57:21PM +0100, Russell King - ARM Linux wrote:
> On Wed, Apr 16, 2014 at 05:46:47PM -0500, Rob Herring wrote:
> > On Wed, Apr 16, 2014 at 3:43 AM, Russell King
> > <rmk+kernel@arm.linux.org.uk> wrote:
> > > 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.

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.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum

[Russell King - ARM Linux]

On Thu, Apr 17, 2014 at 12:02:26AM +0100, Russell King - ARM Linux wrote:
> On Wed, Apr 16, 2014 at 11:57:21PM +0100, Russell King - ARM Linux wrote:
> > On Wed, Apr 16, 2014 at 05:46:47PM -0500, Rob Herring wrote:
> > > On Wed, Apr 16, 2014 at 3:43 AM, Russell King
> > > <rmk+kernel@arm.linux.org.uk> wrote:
> > > > 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.
> 
> 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?

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum

[Rob Herring]

On Fri, Jun 6, 2014 at 11:40 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Apr 17, 2014 at 12:02:26AM +0100, Russell King - ARM Linux wrote:
>> On Wed, Apr 16, 2014 at 11:57:21PM +0100, Russell King - ARM Linux wrote:
>> > On Wed, Apr 16, 2014 at 05:46:47PM -0500, Rob Herring wrote:
>> > > On Wed, Apr 16, 2014 at 3:43 AM, Russell King
>> > > <rmk+kernel@arm.linux.org.uk> wrote:
>> > > > 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.
>>
>> 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.

Rob

Re: [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum

[Russell King - ARM Linux]

On Fri, Jun 06, 2014 at 02:50:38PM -0500, Rob Herring wrote:
> Yes, they are okay. I'll ack the documentation once that is done.

When someone documents the existing implmentation, then I'll add to
it - I've not really looked into what the differences are between
the compatible strings, so that's something I can't document.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.