[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 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 1/5] ata: ahci_imx: warn when disabling ahci link

[Russell King]

When the AHCI link is disabled, it can't be re-enabled except by
resetting the entire SoC.  Rather than doing this silently print
some kernel messages to inform the user, along with how to avoid
this.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/ata/ahci_imx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
index 497c7abe1c7d..6a56a561a80b 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -160,6 +160,10 @@ static void ahci_imx_error_handler(struct ata_port *ap)
 	writel(reg_val | PORT_PHY_CTL_PDDQ_LOC, mmio + PORT_PHY_CTL);
 	imx_sata_disable(hpriv);
 	imxpriv->no_device = true;
+
+	dev_info(ap->dev, "no device found, disabling link.\n");
+	dev_info(ap->dev, "pass " MODULE_PARAM_PREFIX
+		 ".hotplug=1 to enable hotplug\n");
 }
 
 static int ahci_imx_softreset(struct ata_link *link, unsigned int *class,
-- 
1.8.3.1

[PATCH RFC 1/5] ata: ahci_imx: warn when disabling ahci link

[Russell King]

When the AHCI link is disabled, it can't be re-enabled except by
resetting the entire SoC.  Rather than doing this silently print
some kernel messages to inform the user, along with how to avoid
this.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/ata/ahci_imx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
index 497c7abe1c7d..6a56a561a80b 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -160,6 +160,10 @@ static void ahci_imx_error_handler(struct ata_port *ap)
 	writel(reg_val | PORT_PHY_CTL_PDDQ_LOC, mmio + PORT_PHY_CTL);
 	imx_sata_disable(hpriv);
 	imxpriv->no_device = true;
+
+	dev_info(ap->dev, "no device found, disabling link.\n");
+	dev_info(ap->dev, "pass " MODULE_PARAM_PREFIX
+		 ".hotplug=1 to enable hotplug\n");
 }
 
 static int ahci_imx_softreset(struct ata_link *link, unsigned int *class,
-- 
1.8.3.1

[PATCH RFC 2/5] ata: ahci_imx: avoid reprobing ahci_imx driver when using DT

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

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 <rmk+kernel@arm.linux.org.uk>
---
 drivers/ata/ahci_imx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
index 6a56a561a80b..26ee412127b3 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -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;
-- 
1.8.3.1

[PATCH RFC 2/5] ata: ahci_imx: avoid reprobing ahci_imx driver when using DT

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

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 <rmk+kernel@arm.linux.org.uk>
---
 drivers/ata/ahci_imx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
index 6a56a561a80b..26ee412127b3 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -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;
-- 
1.8.3.1

Re: [PATCH RFC 2/5] ata: ahci_imx: avoid reprobing ahci_imx driver when using DT

[Shawn Guo]

On Wed, Apr 16, 2014 at 09:43:30AM +0100, Russell King wrote:
> 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.

Shawn

> 
> 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 <rmk+kernel@arm.linux.org.uk>
> ---
>  drivers/ata/ahci_imx.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
> index 6a56a561a80b..26ee412127b3 100644
> --- a/drivers/ata/ahci_imx.c
> +++ b/drivers/ata/ahci_imx.c
> @@ -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;
> -- 
> 1.8.3.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 

[PATCH RFC 2/5] ata: ahci_imx: avoid reprobing ahci_imx driver when using DT

[Shawn Guo]

On Wed, Apr 16, 2014 at 09:43:30AM +0100, Russell King wrote:
> 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.

Shawn

> 
> 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 <rmk+kernel@arm.linux.org.uk>
> ---
>  drivers/ata/ahci_imx.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
> index 6a56a561a80b..26ee412127b3 100644
> --- a/drivers/ata/ahci_imx.c
> +++ b/drivers/ata/ahci_imx.c
> @@ -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;
> -- 
> 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
> 
> 

Re: [PATCH RFC 2/5] ata: ahci_imx: avoid reprobing ahci_imx driver when using DT

[Russell King - ARM Linux]

On Thu, Apr 17, 2014 at 04:45:44PM +0800, Shawn Guo wrote:
> On Wed, Apr 16, 2014 at 09:43:30AM +0100, Russell King wrote:
> > 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.

Thanks, I've moved this patch.

-- 
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 2/5] ata: ahci_imx: avoid reprobing ahci_imx driver when using DT

[Russell King - ARM Linux]

On Thu, Apr 17, 2014 at 04:45:44PM +0800, Shawn Guo wrote:
> On Wed, Apr 16, 2014 at 09:43:30AM +0100, Russell King wrote:
> > 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.

Thanks, I've moved this patch.

-- 
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 3/5] ata: ahci_imx: allow hardware parameters to be specified in DT

[Russell King]

Various SATA phy parameters are board specific, and therefore need to
be configured.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/ata/ahci_imx.c | 168 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 160 insertions(+), 8 deletions(-)

diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
index 26ee412127b3..1251d719cc73 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -46,6 +46,7 @@ struct imx_ahci_priv {
 	struct regmap *gpr;
 	bool no_device;
 	bool first_time;
+	u32 phy_params;
 };
 
 static int ahci_imx_hotplug;
@@ -90,14 +91,7 @@ static int imx_sata_enable(struct ahci_host_priv *hpriv)
 				   IMX6Q_GPR13_SATA_TX_LVL_MASK |
 				   IMX6Q_GPR13_SATA_MPLL_CLK_EN |
 				   IMX6Q_GPR13_SATA_TX_EDGE_RATE,
-				   IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB |
-				   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 |
-				   IMX6Q_GPR13_SATA_TX_ATTEN_9_16 |
-				   IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB |
-				   IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
+				   imxpriv->phy_params);
 		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
 				   IMX6Q_GPR13_SATA_MPLL_CLK_EN,
 				   IMX6Q_GPR13_SATA_MPLL_CLK_EN);
@@ -204,6 +198,152 @@ static const struct of_device_id imx_ahci_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, imx_ahci_of_match);
 
+struct reg_value {
+	u32 of_value;
+	u32 reg_value;
+};
+
+struct reg_property {
+	const char *name;
+	const struct reg_value *values;
+	size_t num_values;
+	u32 def_value;
+};
+
+static const struct reg_value gpr13_tx_level[] = {
+	{  937, IMX6Q_GPR13_SATA_TX_LVL_0_937_V },
+	{  947, IMX6Q_GPR13_SATA_TX_LVL_0_947_V },
+	{  957, IMX6Q_GPR13_SATA_TX_LVL_0_957_V },
+	{  966, IMX6Q_GPR13_SATA_TX_LVL_0_966_V },
+	{  976, IMX6Q_GPR13_SATA_TX_LVL_0_976_V },
+	{  986, IMX6Q_GPR13_SATA_TX_LVL_0_986_V },
+	{  996, IMX6Q_GPR13_SATA_TX_LVL_0_996_V },
+	{ 1005, IMX6Q_GPR13_SATA_TX_LVL_1_005_V },
+	{ 1015, IMX6Q_GPR13_SATA_TX_LVL_1_015_V },
+	{ 1025, IMX6Q_GPR13_SATA_TX_LVL_1_025_V },
+	{ 1035, IMX6Q_GPR13_SATA_TX_LVL_1_035_V },
+	{ 1045, IMX6Q_GPR13_SATA_TX_LVL_1_045_V },
+	{ 1054, IMX6Q_GPR13_SATA_TX_LVL_1_054_V },
+	{ 1064, IMX6Q_GPR13_SATA_TX_LVL_1_064_V },
+	{ 1074, IMX6Q_GPR13_SATA_TX_LVL_1_074_V },
+	{ 1084, IMX6Q_GPR13_SATA_TX_LVL_1_084_V },
+	{ 1094, IMX6Q_GPR13_SATA_TX_LVL_1_094_V },
+	{ 1104, IMX6Q_GPR13_SATA_TX_LVL_1_104_V },
+	{ 1113, IMX6Q_GPR13_SATA_TX_LVL_1_113_V },
+	{ 1123, IMX6Q_GPR13_SATA_TX_LVL_1_123_V },
+	{ 1133, IMX6Q_GPR13_SATA_TX_LVL_1_133_V },
+	{ 1143, IMX6Q_GPR13_SATA_TX_LVL_1_143_V },
+	{ 1152, IMX6Q_GPR13_SATA_TX_LVL_1_152_V },
+	{ 1162, IMX6Q_GPR13_SATA_TX_LVL_1_162_V },
+	{ 1172, IMX6Q_GPR13_SATA_TX_LVL_1_172_V },
+	{ 1182, IMX6Q_GPR13_SATA_TX_LVL_1_182_V },
+	{ 1191, IMX6Q_GPR13_SATA_TX_LVL_1_191_V },
+	{ 1201, IMX6Q_GPR13_SATA_TX_LVL_1_201_V },
+	{ 1211, IMX6Q_GPR13_SATA_TX_LVL_1_211_V },
+	{ 1221, IMX6Q_GPR13_SATA_TX_LVL_1_221_V },
+	{ 1230, IMX6Q_GPR13_SATA_TX_LVL_1_230_V },
+	{ 1240, IMX6Q_GPR13_SATA_TX_LVL_1_240_V }
+};
+
+static const struct reg_value gpr13_tx_boost[] = {
+	{    0, IMX6Q_GPR13_SATA_TX_BOOST_0_00_DB },
+	{  370, IMX6Q_GPR13_SATA_TX_BOOST_0_37_DB },
+	{  740, IMX6Q_GPR13_SATA_TX_BOOST_0_74_DB },
+	{  111, IMX6Q_GPR13_SATA_TX_BOOST_1_11_DB },
+	{  148, IMX6Q_GPR13_SATA_TX_BOOST_1_48_DB },
+	{  185, IMX6Q_GPR13_SATA_TX_BOOST_1_85_DB },
+	{  222, IMX6Q_GPR13_SATA_TX_BOOST_2_22_DB },
+	{  259, IMX6Q_GPR13_SATA_TX_BOOST_2_59_DB },
+	{  296, IMX6Q_GPR13_SATA_TX_BOOST_2_96_DB },
+	{  333, IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB },
+	{  370, IMX6Q_GPR13_SATA_TX_BOOST_3_70_DB },
+	{  407, IMX6Q_GPR13_SATA_TX_BOOST_4_07_DB },
+	{  444, IMX6Q_GPR13_SATA_TX_BOOST_4_44_DB },
+	{  481, IMX6Q_GPR13_SATA_TX_BOOST_4_81_DB },
+	{  528, IMX6Q_GPR13_SATA_TX_BOOST_5_28_DB },
+	{  575, IMX6Q_GPR13_SATA_TX_BOOST_5_75_DB }
+};
+
+static const struct reg_value gpr13_tx_atten[] = {
+	{  8, IMX6Q_GPR13_SATA_TX_ATTEN_8_16 },
+	{  9, IMX6Q_GPR13_SATA_TX_ATTEN_9_16 },
+	{ 10, IMX6Q_GPR13_SATA_TX_ATTEN_10_16 },
+	{ 12, IMX6Q_GPR13_SATA_TX_ATTEN_12_16 },
+	{ 14, IMX6Q_GPR13_SATA_TX_ATTEN_14_16 },
+	{ 16, IMX6Q_GPR13_SATA_TX_ATTEN_16_16 },
+};
+
+static const struct reg_value gpr13_rx_eq[] = {
+	{  500, IMX6Q_GPR13_SATA_RX_EQ_VAL_0_5_DB },
+	{ 1000, IMX6Q_GPR13_SATA_RX_EQ_VAL_1_0_DB },
+	{ 1500, IMX6Q_GPR13_SATA_RX_EQ_VAL_1_5_DB },
+	{ 2000, IMX6Q_GPR13_SATA_RX_EQ_VAL_2_0_DB },
+	{ 2500, IMX6Q_GPR13_SATA_RX_EQ_VAL_2_5_DB },
+	{ 3000, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB },
+	{ 3500, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_5_DB },
+	{ 4000, IMX6Q_GPR13_SATA_RX_EQ_VAL_4_0_DB },
+};
+
+static const struct reg_property gpr13_props[] = {
+	{
+		.name = "fsl,transmit-level-mV",
+		.values = gpr13_tx_level,
+		.num_values = ARRAY_SIZE(gpr13_tx_level),
+		.def_value = IMX6Q_GPR13_SATA_TX_LVL_1_025_V,
+	}, {
+		.name = "fsl,transmit-boost-mdB",
+		.values = gpr13_tx_boost,
+		.num_values = ARRAY_SIZE(gpr13_tx_boost),
+		.def_value = IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB,
+	}, {
+		.name = "fsl,transmit-atten-16ths",
+		.values = gpr13_tx_atten,
+		.num_values = ARRAY_SIZE(gpr13_tx_atten),
+		.def_value = IMX6Q_GPR13_SATA_TX_ATTEN_9_16,
+	}, {
+		.name = "fsl,receive-eq-mdB",
+		.values = gpr13_rx_eq,
+		.num_values = ARRAY_SIZE(gpr13_rx_eq),
+		.def_value = IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB,
+	},
+};
+
+static u32 imx_ahci_parse_props(struct device *dev,
+				const struct reg_property *prop, size_t num)
+{
+	struct device_node *np = dev->of_node;
+	u32 reg_value = 0;
+	int i, j;
+
+	for (i = 0; i < num; i++, prop++) {
+		u32 of_val;
+
+		if (of_property_read_u32(np, prop->name, &of_val)) {
+			dev_info(dev, "%s not specified, using %08x\n",
+				prop->name, prop->def_value);
+			reg_value |= prop->def_value;
+			continue;
+		}
+
+		for (j = 0; j < prop->num_values; j++) {
+			if (prop->values[j].of_value == of_val) {
+				dev_info(dev, "%s value %u, using %08x\n",
+					prop->name, of_val, prop->values[j].reg_value);
+				reg_value |= prop->values[j].reg_value;
+				break;
+			}
+		}
+
+		if (j == prop->num_values) {
+			dev_err(dev, "DT property %s is not a valid value\n",
+				prop->name);
+			reg_value |= prop->def_value;
+		}
+	}
+
+	return reg_value;
+}
+
 static int imx_ahci_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -235,6 +375,8 @@ static int imx_ahci_probe(struct platform_device *pdev)
 	}
 
 	if (imxpriv->type == AHCI_IMX6Q) {
+		u32 reg_value;
+
 		imxpriv->gpr = syscon_regmap_lookup_by_compatible(
 							"fsl,imx6q-iomuxc-gpr");
 		if (IS_ERR(imxpriv->gpr)) {
@@ -242,6 +384,16 @@ static int imx_ahci_probe(struct platform_device *pdev)
 				"failed to find fsl,imx6q-iomux-gpr regmap\n");
 			return PTR_ERR(imxpriv->gpr);
 		}
+
+		reg_value = imx_ahci_parse_props(dev, gpr13_props,
+						 ARRAY_SIZE(gpr13_props));
+
+		imxpriv->phy_params =
+				   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;
 	}
 
 	hpriv = ahci_platform_get_resources(pdev);
-- 
1.8.3.1

[PATCH RFC 3/5] ata: ahci_imx: allow hardware parameters to be specified in DT

[Russell King]

Various SATA phy parameters are board specific, and therefore need to
be configured.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/ata/ahci_imx.c | 168 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 160 insertions(+), 8 deletions(-)

diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
index 26ee412127b3..1251d719cc73 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -46,6 +46,7 @@ struct imx_ahci_priv {
 	struct regmap *gpr;
 	bool no_device;
 	bool first_time;
+	u32 phy_params;
 };
 
 static int ahci_imx_hotplug;
@@ -90,14 +91,7 @@ static int imx_sata_enable(struct ahci_host_priv *hpriv)
 				   IMX6Q_GPR13_SATA_TX_LVL_MASK |
 				   IMX6Q_GPR13_SATA_MPLL_CLK_EN |
 				   IMX6Q_GPR13_SATA_TX_EDGE_RATE,
-				   IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB |
-				   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 |
-				   IMX6Q_GPR13_SATA_TX_ATTEN_9_16 |
-				   IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB |
-				   IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
+				   imxpriv->phy_params);
 		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
 				   IMX6Q_GPR13_SATA_MPLL_CLK_EN,
 				   IMX6Q_GPR13_SATA_MPLL_CLK_EN);
@@ -204,6 +198,152 @@ static const struct of_device_id imx_ahci_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, imx_ahci_of_match);
 
+struct reg_value {
+	u32 of_value;
+	u32 reg_value;
+};
+
+struct reg_property {
+	const char *name;
+	const struct reg_value *values;
+	size_t num_values;
+	u32 def_value;
+};
+
+static const struct reg_value gpr13_tx_level[] = {
+	{  937, IMX6Q_GPR13_SATA_TX_LVL_0_937_V },
+	{  947, IMX6Q_GPR13_SATA_TX_LVL_0_947_V },
+	{  957, IMX6Q_GPR13_SATA_TX_LVL_0_957_V },
+	{  966, IMX6Q_GPR13_SATA_TX_LVL_0_966_V },
+	{  976, IMX6Q_GPR13_SATA_TX_LVL_0_976_V },
+	{  986, IMX6Q_GPR13_SATA_TX_LVL_0_986_V },
+	{  996, IMX6Q_GPR13_SATA_TX_LVL_0_996_V },
+	{ 1005, IMX6Q_GPR13_SATA_TX_LVL_1_005_V },
+	{ 1015, IMX6Q_GPR13_SATA_TX_LVL_1_015_V },
+	{ 1025, IMX6Q_GPR13_SATA_TX_LVL_1_025_V },
+	{ 1035, IMX6Q_GPR13_SATA_TX_LVL_1_035_V },
+	{ 1045, IMX6Q_GPR13_SATA_TX_LVL_1_045_V },
+	{ 1054, IMX6Q_GPR13_SATA_TX_LVL_1_054_V },
+	{ 1064, IMX6Q_GPR13_SATA_TX_LVL_1_064_V },
+	{ 1074, IMX6Q_GPR13_SATA_TX_LVL_1_074_V },
+	{ 1084, IMX6Q_GPR13_SATA_TX_LVL_1_084_V },
+	{ 1094, IMX6Q_GPR13_SATA_TX_LVL_1_094_V },
+	{ 1104, IMX6Q_GPR13_SATA_TX_LVL_1_104_V },
+	{ 1113, IMX6Q_GPR13_SATA_TX_LVL_1_113_V },
+	{ 1123, IMX6Q_GPR13_SATA_TX_LVL_1_123_V },
+	{ 1133, IMX6Q_GPR13_SATA_TX_LVL_1_133_V },
+	{ 1143, IMX6Q_GPR13_SATA_TX_LVL_1_143_V },
+	{ 1152, IMX6Q_GPR13_SATA_TX_LVL_1_152_V },
+	{ 1162, IMX6Q_GPR13_SATA_TX_LVL_1_162_V },
+	{ 1172, IMX6Q_GPR13_SATA_TX_LVL_1_172_V },
+	{ 1182, IMX6Q_GPR13_SATA_TX_LVL_1_182_V },
+	{ 1191, IMX6Q_GPR13_SATA_TX_LVL_1_191_V },
+	{ 1201, IMX6Q_GPR13_SATA_TX_LVL_1_201_V },
+	{ 1211, IMX6Q_GPR13_SATA_TX_LVL_1_211_V },
+	{ 1221, IMX6Q_GPR13_SATA_TX_LVL_1_221_V },
+	{ 1230, IMX6Q_GPR13_SATA_TX_LVL_1_230_V },
+	{ 1240, IMX6Q_GPR13_SATA_TX_LVL_1_240_V }
+};
+
+static const struct reg_value gpr13_tx_boost[] = {
+	{    0, IMX6Q_GPR13_SATA_TX_BOOST_0_00_DB },
+	{  370, IMX6Q_GPR13_SATA_TX_BOOST_0_37_DB },
+	{  740, IMX6Q_GPR13_SATA_TX_BOOST_0_74_DB },
+	{  111, IMX6Q_GPR13_SATA_TX_BOOST_1_11_DB },
+	{  148, IMX6Q_GPR13_SATA_TX_BOOST_1_48_DB },
+	{  185, IMX6Q_GPR13_SATA_TX_BOOST_1_85_DB },
+	{  222, IMX6Q_GPR13_SATA_TX_BOOST_2_22_DB },
+	{  259, IMX6Q_GPR13_SATA_TX_BOOST_2_59_DB },
+	{  296, IMX6Q_GPR13_SATA_TX_BOOST_2_96_DB },
+	{  333, IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB },
+	{  370, IMX6Q_GPR13_SATA_TX_BOOST_3_70_DB },
+	{  407, IMX6Q_GPR13_SATA_TX_BOOST_4_07_DB },
+	{  444, IMX6Q_GPR13_SATA_TX_BOOST_4_44_DB },
+	{  481, IMX6Q_GPR13_SATA_TX_BOOST_4_81_DB },
+	{  528, IMX6Q_GPR13_SATA_TX_BOOST_5_28_DB },
+	{  575, IMX6Q_GPR13_SATA_TX_BOOST_5_75_DB }
+};
+
+static const struct reg_value gpr13_tx_atten[] = {
+	{  8, IMX6Q_GPR13_SATA_TX_ATTEN_8_16 },
+	{  9, IMX6Q_GPR13_SATA_TX_ATTEN_9_16 },
+	{ 10, IMX6Q_GPR13_SATA_TX_ATTEN_10_16 },
+	{ 12, IMX6Q_GPR13_SATA_TX_ATTEN_12_16 },
+	{ 14, IMX6Q_GPR13_SATA_TX_ATTEN_14_16 },
+	{ 16, IMX6Q_GPR13_SATA_TX_ATTEN_16_16 },
+};
+
+static const struct reg_value gpr13_rx_eq[] = {
+	{  500, IMX6Q_GPR13_SATA_RX_EQ_VAL_0_5_DB },
+	{ 1000, IMX6Q_GPR13_SATA_RX_EQ_VAL_1_0_DB },
+	{ 1500, IMX6Q_GPR13_SATA_RX_EQ_VAL_1_5_DB },
+	{ 2000, IMX6Q_GPR13_SATA_RX_EQ_VAL_2_0_DB },
+	{ 2500, IMX6Q_GPR13_SATA_RX_EQ_VAL_2_5_DB },
+	{ 3000, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB },
+	{ 3500, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_5_DB },
+	{ 4000, IMX6Q_GPR13_SATA_RX_EQ_VAL_4_0_DB },
+};
+
+static const struct reg_property gpr13_props[] = {
+	{
+		.name = "fsl,transmit-level-mV",
+		.values = gpr13_tx_level,
+		.num_values = ARRAY_SIZE(gpr13_tx_level),
+		.def_value = IMX6Q_GPR13_SATA_TX_LVL_1_025_V,
+	}, {
+		.name = "fsl,transmit-boost-mdB",
+		.values = gpr13_tx_boost,
+		.num_values = ARRAY_SIZE(gpr13_tx_boost),
+		.def_value = IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB,
+	}, {
+		.name = "fsl,transmit-atten-16ths",
+		.values = gpr13_tx_atten,
+		.num_values = ARRAY_SIZE(gpr13_tx_atten),
+		.def_value = IMX6Q_GPR13_SATA_TX_ATTEN_9_16,
+	}, {
+		.name = "fsl,receive-eq-mdB",
+		.values = gpr13_rx_eq,
+		.num_values = ARRAY_SIZE(gpr13_rx_eq),
+		.def_value = IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB,
+	},
+};
+
+static u32 imx_ahci_parse_props(struct device *dev,
+				const struct reg_property *prop, size_t num)
+{
+	struct device_node *np = dev->of_node;
+	u32 reg_value = 0;
+	int i, j;
+
+	for (i = 0; i < num; i++, prop++) {
+		u32 of_val;
+
+		if (of_property_read_u32(np, prop->name, &of_val)) {
+			dev_info(dev, "%s not specified, using %08x\n",
+				prop->name, prop->def_value);
+			reg_value |= prop->def_value;
+			continue;
+		}
+
+		for (j = 0; j < prop->num_values; j++) {
+			if (prop->values[j].of_value == of_val) {
+				dev_info(dev, "%s value %u, using %08x\n",
+					prop->name, of_val, prop->values[j].reg_value);
+				reg_value |= prop->values[j].reg_value;
+				break;
+			}
+		}
+
+		if (j == prop->num_values) {
+			dev_err(dev, "DT property %s is not a valid value\n",
+				prop->name);
+			reg_value |= prop->def_value;
+		}
+	}
+
+	return reg_value;
+}
+
 static int imx_ahci_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -235,6 +375,8 @@ static int imx_ahci_probe(struct platform_device *pdev)
 	}
 
 	if (imxpriv->type == AHCI_IMX6Q) {
+		u32 reg_value;
+
 		imxpriv->gpr = syscon_regmap_lookup_by_compatible(
 							"fsl,imx6q-iomuxc-gpr");
 		if (IS_ERR(imxpriv->gpr)) {
@@ -242,6 +384,16 @@ static int imx_ahci_probe(struct platform_device *pdev)
 				"failed to find fsl,imx6q-iomux-gpr regmap\n");
 			return PTR_ERR(imxpriv->gpr);
 		}
+
+		reg_value = imx_ahci_parse_props(dev, gpr13_props,
+						 ARRAY_SIZE(gpr13_props));
+
+		imxpriv->phy_params =
+				   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;
 	}
 
 	hpriv = ahci_platform_get_resources(pdev);
-- 
1.8.3.1

[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 4/5] ARM: cubox-i: add eSATA DT configuration

[Russell King]

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 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

[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

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

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

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

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

[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

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

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