All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] imx ahci DT updates + cubox-i eSATA support
@ 2014-04-16  8:42 ` Russell King - ARM Linux
  0 siblings, 0 replies; 28+ messages in thread
From: Russell King - ARM Linux @ 2014-04-16  8:42 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: devicetree, Ian Campbell, Kumar Gala, linux-ide, Mark Rutland,
	Pawel Moll, Rob Herring, Sascha Hauer, Shawn Guo, Tejun Heo

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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 0/5] imx ahci DT updates + cubox-i eSATA support
@ 2014-04-16  8:42 ` Russell King - ARM Linux
  0 siblings, 0 replies; 28+ messages in thread
From: Russell King - ARM Linux @ 2014-04-16  8:42 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH RFC 1/5] ata: ahci_imx: warn when disabling ahci link
  2014-04-16  8:42 ` Russell King - ARM Linux
@ 2014-04-16  8:43   ` Russell King
  -1 siblings, 0 replies; 28+ messages in thread
From: Russell King @ 2014-04-16  8:43 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Tejun Heo, linux-ide

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


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH RFC 1/5] ata: ahci_imx: warn when disabling ahci link
@ 2014-04-16  8:43   ` Russell King
  0 siblings, 0 replies; 28+ messages in thread
From: Russell King @ 2014-04-16  8:43 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH RFC 2/5] ata: ahci_imx: avoid reprobing ahci_imx driver when using DT
  2014-04-16  8:42 ` Russell King - ARM Linux
@ 2014-04-16  8:43   ` Russell King
  -1 siblings, 0 replies; 28+ messages in thread
From: Russell King @ 2014-04-16  8:43 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Tejun Heo, linux-ide

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


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH RFC 2/5] ata: ahci_imx: avoid reprobing ahci_imx driver when using DT
@ 2014-04-16  8:43   ` Russell King
  0 siblings, 0 replies; 28+ messages in thread
From: Russell King @ 2014-04-16  8:43 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH RFC 3/5] ata: ahci_imx: allow hardware parameters to be specified in DT
  2014-04-16  8:42 ` Russell King - ARM Linux
@ 2014-04-16  8:43   ` Russell King
  -1 siblings, 0 replies; 28+ messages in thread
From: Russell King @ 2014-04-16  8:43 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Tejun Heo, linux-ide

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


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH RFC 3/5] ata: ahci_imx: allow hardware parameters to be specified in DT
@ 2014-04-16  8:43   ` Russell King
  0 siblings, 0 replies; 28+ messages in thread
From: Russell King @ 2014-04-16  8:43 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH RFC 4/5] ARM: cubox-i: add eSATA DT configuration
  2014-04-16  8:42 ` Russell King - ARM Linux
@ 2014-04-16  8:43     ` Russell King
  -1 siblings, 0 replies; 28+ messages in thread
From: Russell King @ 2014-04-16  8:43 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Shawn Guo, Sascha Hauer, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, devicetree-u79uwXL29TY76Z2rM5mHXA

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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH RFC 4/5] ARM: cubox-i: add eSATA DT configuration
@ 2014-04-16  8:43     ` Russell King
  0 siblings, 0 replies; 28+ messages in thread
From: Russell King @ 2014-04-16  8:43 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum
  2014-04-16  8:42 ` Russell King - ARM Linux
@ 2014-04-16  8:43   ` Russell King
  -1 siblings, 0 replies; 28+ messages in thread
From: Russell King @ 2014-04-16  8:43 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Shawn Guo, Sascha Hauer, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Tejun Heo, devicetree, linux-ide

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


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum
@ 2014-04-16  8:43   ` Russell King
  0 siblings, 0 replies; 28+ messages in thread
From: Russell King @ 2014-04-16  8:43 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum
  2014-04-16  8:43   ` Russell King
@ 2014-04-16 22:46     ` Rob Herring
  -1 siblings, 0 replies; 28+ messages in thread
From: Rob Herring @ 2014-04-16 22:46 UTC (permalink / raw)
  To: Russell King
  Cc: linux-arm-kernel, Shawn Guo, Sascha Hauer, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Tejun Heo,
	devicetree, linux-ide

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
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum
@ 2014-04-16 22:46     ` Rob Herring
  0 siblings, 0 replies; 28+ messages in thread
From: Rob Herring @ 2014-04-16 22:46 UTC (permalink / raw)
  To: linux-arm-kernel

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
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum
  2014-04-16 22:46     ` Rob Herring
@ 2014-04-16 22:57       ` Russell King - ARM Linux
  -1 siblings, 0 replies; 28+ messages in thread
From: Russell King - ARM Linux @ 2014-04-16 22:57 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-arm-kernel, Shawn Guo, Sascha Hauer, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Tejun Heo,
	devicetree, linux-ide

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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum
@ 2014-04-16 22:57       ` Russell King - ARM Linux
  0 siblings, 0 replies; 28+ messages in thread
From: Russell King - ARM Linux @ 2014-04-16 22:57 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum
  2014-04-16 22:57       ` Russell King - ARM Linux
@ 2014-04-16 23:02         ` Russell King - ARM Linux
  -1 siblings, 0 replies; 28+ messages in thread
From: Russell King - ARM Linux @ 2014-04-16 23:02 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell, linux-ide,
	Rob Herring, linux-arm-kernel, Sascha Hauer, Kumar Gala,
	Tejun Heo, Shawn Guo

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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum
@ 2014-04-16 23:02         ` Russell King - ARM Linux
  0 siblings, 0 replies; 28+ messages in thread
From: Russell King - ARM Linux @ 2014-04-16 23:02 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH RFC 2/5] ata: ahci_imx: avoid reprobing ahci_imx driver when using DT
  2014-04-16  8:43   ` Russell King
@ 2014-04-17  8:45     ` Shawn Guo
  -1 siblings, 0 replies; 28+ messages in thread
From: Shawn Guo @ 2014-04-17  8:45 UTC (permalink / raw)
  To: Russell King; +Cc: linux-arm-kernel, Tejun Heo, linux-ide

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


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH RFC 2/5] ata: ahci_imx: avoid reprobing ahci_imx driver when using DT
@ 2014-04-17  8:45     ` Shawn Guo
  0 siblings, 0 replies; 28+ messages in thread
From: Shawn Guo @ 2014-04-17  8:45 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH RFC 2/5] ata: ahci_imx: avoid reprobing ahci_imx driver when using DT
  2014-04-17  8:45     ` Shawn Guo
@ 2014-04-17 18:37       ` Russell King - ARM Linux
  -1 siblings, 0 replies; 28+ messages in thread
From: Russell King - ARM Linux @ 2014-04-17 18:37 UTC (permalink / raw)
  To: Shawn Guo; +Cc: linux-arm-kernel, Tejun Heo, linux-ide

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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH RFC 2/5] ata: ahci_imx: avoid reprobing ahci_imx driver when using DT
@ 2014-04-17 18:37       ` Russell King - ARM Linux
  0 siblings, 0 replies; 28+ messages in thread
From: Russell King - ARM Linux @ 2014-04-17 18:37 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum
  2014-04-16 23:02         ` Russell King - ARM Linux
@ 2014-06-06 16:40           ` Russell King - ARM Linux
  -1 siblings, 0 replies; 28+ messages in thread
From: Russell King - ARM Linux @ 2014-06-06 16:40 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell, linux-ide,
	Rob Herring, Shawn Guo, Sascha Hauer, Kumar Gala, Tejun Heo,
	linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum
@ 2014-06-06 16:40           ` Russell King - ARM Linux
  0 siblings, 0 replies; 28+ messages in thread
From: Russell King - ARM Linux @ 2014-06-06 16:40 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum
  2014-06-06 16:40           ` Russell King - ARM Linux
@ 2014-06-06 19:50             ` Rob Herring
  -1 siblings, 0 replies; 28+ messages in thread
From: Rob Herring @ 2014-06-06 19:50 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell, linux-ide,
	Rob Herring, Shawn Guo, Sascha Hauer, Kumar Gala, Tejun Heo,
	linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum
@ 2014-06-06 19:50             ` Rob Herring
  0 siblings, 0 replies; 28+ messages in thread
From: Rob Herring @ 2014-06-06 19:50 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum
  2014-06-06 19:50             ` Rob Herring
@ 2014-06-17  9:47               ` Russell King - ARM Linux
  -1 siblings, 0 replies; 28+ messages in thread
From: Russell King - ARM Linux @ 2014-06-17  9:47 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell, linux-ide,
	Rob Herring, Shawn Guo, Sascha Hauer, Kumar Gala, Tejun Heo,
	linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum
@ 2014-06-17  9:47               ` Russell King - ARM Linux
  0 siblings, 0 replies; 28+ messages in thread
From: Russell King - ARM Linux @ 2014-06-17  9:47 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2014-06-17  9:47 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-16  8:42 [PATCH 0/5] imx ahci DT updates + cubox-i eSATA support Russell King - ARM Linux
2014-04-16  8:42 ` Russell King - ARM Linux
2014-04-16  8:43 ` [PATCH RFC 1/5] ata: ahci_imx: warn when disabling ahci link Russell King
2014-04-16  8:43   ` Russell King
2014-04-16  8:43 ` [PATCH RFC 2/5] ata: ahci_imx: avoid reprobing ahci_imx driver when using DT Russell King
2014-04-16  8:43   ` Russell King
2014-04-17  8:45   ` Shawn Guo
2014-04-17  8:45     ` Shawn Guo
2014-04-17 18:37     ` Russell King - ARM Linux
2014-04-17 18:37       ` Russell King - ARM Linux
2014-04-16  8:43 ` [PATCH RFC 3/5] ata: ahci_imx: allow hardware parameters to be specified in DT Russell King
2014-04-16  8:43   ` Russell King
     [not found] ` <20140416084227.GD24070-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>
2014-04-16  8:43   ` [PATCH RFC 4/5] ARM: cubox-i: add eSATA DT configuration Russell King
2014-04-16  8:43     ` Russell King
2014-04-16  8:43 ` [PATCH RFC 5/5] ahci_imx: add disable for spread-spectrum Russell King
2014-04-16  8:43   ` Russell King
2014-04-16 22:46   ` Rob Herring
2014-04-16 22:46     ` Rob Herring
2014-04-16 22:57     ` Russell King - ARM Linux
2014-04-16 22:57       ` Russell King - ARM Linux
2014-04-16 23:02       ` Russell King - ARM Linux
2014-04-16 23:02         ` Russell King - ARM Linux
2014-06-06 16:40         ` Russell King - ARM Linux
2014-06-06 16:40           ` Russell King - ARM Linux
2014-06-06 19:50           ` Rob Herring
2014-06-06 19:50             ` Rob Herring
2014-06-17  9:47             ` Russell King - ARM Linux
2014-06-17  9:47               ` Russell King - ARM Linux

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.