[PATCH 1/5 v4] i2c/gpio: add DT support

[PATCH 1/5 v4] i2c/gpio: add DT support

[Jean-Christophe PLAGNIOL-VILLARD]

update to use devm_kzalloc for private data

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
Acked-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Acked-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Cc: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
---
v4:

	fix timeout
	use gpio_is_valid
 .../devicetree/bindings/gpio/gpio_i2c.txt          |   32 +++++++
 drivers/i2c/busses/i2c-gpio.c                      |   94 +++++++++++++++----
 2 files changed, 106 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio_i2c.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio_i2c.txt b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt
new file mode 100644
index 0000000..4f8ec94
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt
@@ -0,0 +1,32 @@
+Device-Tree bindings for i2c gpio driver
+
+Required properties:
+	- compatible = "i2c-gpio";
+	- gpios: sda and scl gpio
+
+
+Optional properties:
+	- i2c-gpio,sda-open-drain: sda as open drain
+	- i2c-gpio,scl-open-drain: scl as open drain
+	- i2c-gpio,scl-output-only: scl as output only
+	- i2c-gpio,delay-us: delay between GPIO operations (may depend on each platform)
+	- i2c-gpio,timeout-ms: timeout to get data
+
+Example nodes:
+
+i2c@0 {
+	compatible = "i2c-gpio";
+	gpios = <&pioA 23 0 /* sda */
+		 &pioA 24 0 /* scl */
+		>;
+	i2c-gpio,sda-open-drain;
+	i2c-gpio,scl-open-drain;
+	i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	rv3029c2@56 {
+		compatible = "rv3029c2";
+		reg = <0x56>;
+	};
+};
diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index a651779..c0330a4 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -14,8 +14,15 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/platform_device.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/of_i2c.h>
 
-#include <asm/gpio.h>
+struct i2c_gpio_private_data {
+	struct i2c_adapter adap;
+	struct i2c_algo_bit_data bit_data;
+	struct i2c_gpio_platform_data pdata;
+};
 
 /* Toggle SDA by changing the direction of the pin */
 static void i2c_gpio_setsda_dir(void *data, int state)
@@ -78,24 +85,62 @@ static int i2c_gpio_getscl(void *data)
 	return gpio_get_value(pdata->scl_pin);
 }
 
+static int __devinit of_i2c_gpio_probe(struct device_node *np,
+			     struct i2c_gpio_platform_data *pdata)
+{
+	u32 reg;
+
+	if (of_gpio_count(np) < 2)
+		return -ENODEV;
+
+	pdata->sda_pin = of_get_gpio(np, 0);
+	pdata->scl_pin = of_get_gpio(np, 1);
+
+	if (!gpio_is_valid(pdata->sda_pin) || !gpio_is_valid(pdata->scl_pin)) {
+		pr_err("%s: invalid GPIO pins, sda=%d/scl=%d\n",
+		       np->full_name, pdata->sda_pin, pdata->scl_pin);
+		return -ENODEV;
+	}
+
+	of_property_read_u32(np, "i2c-gpio,delay-us", &pdata->udelay);
+
+	if (!of_property_read_u32(np, "i2c-gpio,timeout-ms", &reg))
+		pdata->timeout = msecs_to_jiffies(reg);
+
+	pdata->sda_is_open_drain =
+		of_property_read_bool(np, "i2c-gpio,sda-open-drain");
+	pdata->scl_is_open_drain =
+		of_property_read_bool(np, "i2c-gpio,scl-open-drain");
+	pdata->scl_is_output_only =
+		of_property_read_bool(np, "i2c-gpio,scl-output-only");
+
+	return 0;
+}
+
 static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 {
+	struct i2c_gpio_private_data *priv;
 	struct i2c_gpio_platform_data *pdata;
 	struct i2c_algo_bit_data *bit_data;
 	struct i2c_adapter *adap;
 	int ret;
 
-	pdata = pdev->dev.platform_data;
-	if (!pdata)
-		return -ENXIO;
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	adap = &priv->adap;
+	bit_data = &priv->bit_data;
+	pdata = &priv->pdata;
 
-	ret = -ENOMEM;
-	adap = kzalloc(sizeof(struct i2c_adapter), GFP_KERNEL);
-	if (!adap)
-		goto err_alloc_adap;
-	bit_data = kzalloc(sizeof(struct i2c_algo_bit_data), GFP_KERNEL);
-	if (!bit_data)
-		goto err_alloc_bit_data;
+	if (pdev->dev.of_node) {
+		ret = of_i2c_gpio_probe(pdev->dev.of_node, pdata);
+		if (ret)
+			return ret;
+	} else {
+		if (!pdev->dev.platform_data)
+			return -ENXIO;
+		memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
+	}
 
 	ret = gpio_request(pdata->sda_pin, "sda");
 	if (ret)
@@ -143,6 +188,7 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 	adap->algo_data = bit_data;
 	adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
 	adap->dev.parent = &pdev->dev;
+	adap->dev.of_node = pdev->dev.of_node;
 
 	/*
 	 * If "dev->id" is negative we consider it as zero.
@@ -154,7 +200,9 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_add_bus;
 
-	platform_set_drvdata(pdev, adap);
+	of_i2c_register_devices(adap);
+
+	platform_set_drvdata(pdev, priv);
 
 	dev_info(&pdev->dev, "using pins %u (SDA) and %u (SCL%s)\n",
 		 pdata->sda_pin, pdata->scl_pin,
@@ -168,34 +216,40 @@ err_add_bus:
 err_request_scl:
 	gpio_free(pdata->sda_pin);
 err_request_sda:
-	kfree(bit_data);
-err_alloc_bit_data:
-	kfree(adap);
-err_alloc_adap:
 	return ret;
 }
 
 static int __devexit i2c_gpio_remove(struct platform_device *pdev)
 {
+	struct i2c_gpio_private_data *priv;
 	struct i2c_gpio_platform_data *pdata;
 	struct i2c_adapter *adap;
 
-	adap = platform_get_drvdata(pdev);
-	pdata = pdev->dev.platform_data;
+	priv = platform_get_drvdata(pdev);
+	adap = &priv->adap;
+	pdata = &priv->pdata;
 
 	i2c_del_adapter(adap);
 	gpio_free(pdata->scl_pin);
 	gpio_free(pdata->sda_pin);
-	kfree(adap->algo_data);
-	kfree(adap);
 
 	return 0;
 }
 
+#if defined(CONFIG_OF)
+static const struct of_device_id i2c_gpio_dt_ids[] = {
+	{ .compatible = "i2c-gpio", },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, i2c_gpio_dt_ids);
+#endif
+
 static struct platform_driver i2c_gpio_driver = {
 	.driver		= {
 		.name	= "i2c-gpio",
 		.owner	= THIS_MODULE,
+		.of_match_table	= of_match_ptr(i2c_gpio_dt_ids),
 	},
 	.probe		= i2c_gpio_probe,
 	.remove		= __devexit_p(i2c_gpio_remove),
-- 
1.7.7

Re: [PATCH 1/5 v4] i2c/gpio: add DT support

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 759 bytes --]

On Tue, Mar 13, 2012 at 01:33:40PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:

> update to use devm_kzalloc for private data

Hmm, you need really to work on the commit messages, not everybody is
familiar with DT and its implications:

"To achieve DT support, we need to populate a custom platform_data in a
private struct from DT information. To simplify code, the adapter and
algorithm are also put into the private struct."

Something like this?

> ---
> v4:
> 
> 	fix timeout

You need to work on the changelogs as well :)

> 	use gpio_is_valid

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 1/5 v4] i2c/gpio: add DT support

[Jean-Christophe PLAGNIOL-VILLARD]

On 15:08 Tue 13 Mar     , Wolfram Sang wrote:
> On Tue, Mar 13, 2012 at 01:33:40PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> 
> > update to use devm_kzalloc for private data
> 
> Hmm, you need really to work on the commit messages, not everybody is
> familiar with DT and its implications:
> 
> "To achieve DT support, we need to populate a custom platform_data in a
> private struct from DT information. To simplify code, the adapter and
> algorithm are also put into the private struct."
> 
ok I put this can I get the Ack ?

I need to send the pull asap

Best Regards,
J.

Re: [PATCH 1/5 v4] i2c/gpio: add DT support

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 918 bytes --]

On Tue, Mar 13, 2012 at 05:47:56PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 15:08 Tue 13 Mar     , Wolfram Sang wrote:
> > On Tue, Mar 13, 2012 at 01:33:40PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > 
> > > update to use devm_kzalloc for private data
> > 
> > Hmm, you need really to work on the commit messages, not everybody is
> > familiar with DT and its implications:
> > 
> > "To achieve DT support, we need to populate a custom platform_data in a
> > private struct from DT information. To simplify code, the adapter and
> > algorithm are also put into the private struct."
> > 
> ok I put this can I get the Ack ?

Yes.

> I need to send the pull asap

Please, no rush. Makes it too easy for bugs to slip in...

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 1/5 v5] i2c/gpio: add DT support

[Jean-Christophe PLAGNIOL-VILLARD]

To achieve DT support, we need to populate a custom platform_data in a
private struct from DT information. To simplify code, the adapter and
algorithm are also put into the private struct.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
Acked-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Acked-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Acked-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/gpio/gpio_i2c.txt          |   32 +++++++
 drivers/i2c/busses/i2c-gpio.c                      |   94 +++++++++++++++----
 2 files changed, 106 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio_i2c.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio_i2c.txt b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt
new file mode 100644
index 0000000..4f8ec94
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt
@@ -0,0 +1,32 @@
+Device-Tree bindings for i2c gpio driver
+
+Required properties:
+	- compatible = "i2c-gpio";
+	- gpios: sda and scl gpio
+
+
+Optional properties:
+	- i2c-gpio,sda-open-drain: sda as open drain
+	- i2c-gpio,scl-open-drain: scl as open drain
+	- i2c-gpio,scl-output-only: scl as output only
+	- i2c-gpio,delay-us: delay between GPIO operations (may depend on each platform)
+	- i2c-gpio,timeout-ms: timeout to get data
+
+Example nodes:
+
+i2c@0 {
+	compatible = "i2c-gpio";
+	gpios = <&pioA 23 0 /* sda */
+		 &pioA 24 0 /* scl */
+		>;
+	i2c-gpio,sda-open-drain;
+	i2c-gpio,scl-open-drain;
+	i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	rv3029c2@56 {
+		compatible = "rv3029c2";
+		reg = <0x56>;
+	};
+};
diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index a651779..c0330a4 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -14,8 +14,15 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/platform_device.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/of_i2c.h>
 
-#include <asm/gpio.h>
+struct i2c_gpio_private_data {
+	struct i2c_adapter adap;
+	struct i2c_algo_bit_data bit_data;
+	struct i2c_gpio_platform_data pdata;
+};
 
 /* Toggle SDA by changing the direction of the pin */
 static void i2c_gpio_setsda_dir(void *data, int state)
@@ -78,24 +85,62 @@ static int i2c_gpio_getscl(void *data)
 	return gpio_get_value(pdata->scl_pin);
 }
 
+static int __devinit of_i2c_gpio_probe(struct device_node *np,
+			     struct i2c_gpio_platform_data *pdata)
+{
+	u32 reg;
+
+	if (of_gpio_count(np) < 2)
+		return -ENODEV;
+
+	pdata->sda_pin = of_get_gpio(np, 0);
+	pdata->scl_pin = of_get_gpio(np, 1);
+
+	if (!gpio_is_valid(pdata->sda_pin) || !gpio_is_valid(pdata->scl_pin)) {
+		pr_err("%s: invalid GPIO pins, sda=%d/scl=%d\n",
+		       np->full_name, pdata->sda_pin, pdata->scl_pin);
+		return -ENODEV;
+	}
+
+	of_property_read_u32(np, "i2c-gpio,delay-us", &pdata->udelay);
+
+	if (!of_property_read_u32(np, "i2c-gpio,timeout-ms", &reg))
+		pdata->timeout = msecs_to_jiffies(reg);
+
+	pdata->sda_is_open_drain =
+		of_property_read_bool(np, "i2c-gpio,sda-open-drain");
+	pdata->scl_is_open_drain =
+		of_property_read_bool(np, "i2c-gpio,scl-open-drain");
+	pdata->scl_is_output_only =
+		of_property_read_bool(np, "i2c-gpio,scl-output-only");
+
+	return 0;
+}
+
 static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 {
+	struct i2c_gpio_private_data *priv;
 	struct i2c_gpio_platform_data *pdata;
 	struct i2c_algo_bit_data *bit_data;
 	struct i2c_adapter *adap;
 	int ret;
 
-	pdata = pdev->dev.platform_data;
-	if (!pdata)
-		return -ENXIO;
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	adap = &priv->adap;
+	bit_data = &priv->bit_data;
+	pdata = &priv->pdata;
 
-	ret = -ENOMEM;
-	adap = kzalloc(sizeof(struct i2c_adapter), GFP_KERNEL);
-	if (!adap)
-		goto err_alloc_adap;
-	bit_data = kzalloc(sizeof(struct i2c_algo_bit_data), GFP_KERNEL);
-	if (!bit_data)
-		goto err_alloc_bit_data;
+	if (pdev->dev.of_node) {
+		ret = of_i2c_gpio_probe(pdev->dev.of_node, pdata);
+		if (ret)
+			return ret;
+	} else {
+		if (!pdev->dev.platform_data)
+			return -ENXIO;
+		memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
+	}
 
 	ret = gpio_request(pdata->sda_pin, "sda");
 	if (ret)
@@ -143,6 +188,7 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 	adap->algo_data = bit_data;
 	adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
 	adap->dev.parent = &pdev->dev;
+	adap->dev.of_node = pdev->dev.of_node;
 
 	/*
 	 * If "dev->id" is negative we consider it as zero.
@@ -154,7 +200,9 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_add_bus;
 
-	platform_set_drvdata(pdev, adap);
+	of_i2c_register_devices(adap);
+
+	platform_set_drvdata(pdev, priv);
 
 	dev_info(&pdev->dev, "using pins %u (SDA) and %u (SCL%s)\n",
 		 pdata->sda_pin, pdata->scl_pin,
@@ -168,34 +216,40 @@ err_add_bus:
 err_request_scl:
 	gpio_free(pdata->sda_pin);
 err_request_sda:
-	kfree(bit_data);
-err_alloc_bit_data:
-	kfree(adap);
-err_alloc_adap:
 	return ret;
 }
 
 static int __devexit i2c_gpio_remove(struct platform_device *pdev)
 {
+	struct i2c_gpio_private_data *priv;
 	struct i2c_gpio_platform_data *pdata;
 	struct i2c_adapter *adap;
 
-	adap = platform_get_drvdata(pdev);
-	pdata = pdev->dev.platform_data;
+	priv = platform_get_drvdata(pdev);
+	adap = &priv->adap;
+	pdata = &priv->pdata;
 
 	i2c_del_adapter(adap);
 	gpio_free(pdata->scl_pin);
 	gpio_free(pdata->sda_pin);
-	kfree(adap->algo_data);
-	kfree(adap);
 
 	return 0;
 }
 
+#if defined(CONFIG_OF)
+static const struct of_device_id i2c_gpio_dt_ids[] = {
+	{ .compatible = "i2c-gpio", },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, i2c_gpio_dt_ids);
+#endif
+
 static struct platform_driver i2c_gpio_driver = {
 	.driver		= {
 		.name	= "i2c-gpio",
 		.owner	= THIS_MODULE,
+		.of_match_table	= of_match_ptr(i2c_gpio_dt_ids),
 	},
 	.probe		= i2c_gpio_probe,
 	.remove		= __devexit_p(i2c_gpio_remove),
-- 
1.7.7

[PATCH 2/5 v4] ARM: at91: sam9g20 add i2c DT support

[Jean-Christophe PLAGNIOL-VILLARD]

For now on use i2c-gpio driver on the same pin as the hardware IP.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
Acked-by: Rob Herring <rob.herring@calxeda.com>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: devicetree-discuss@lists.ozlabs.org
---
 arch/arm/boot/dts/at91sam9g20.dtsi |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/at91sam9g20.dtsi b/arch/arm/boot/dts/at91sam9g20.dtsi
index 4b0dc99..a885a30 100644
--- a/arch/arm/boot/dts/at91sam9g20.dtsi
+++ b/arch/arm/boot/dts/at91sam9g20.dtsi
@@ -189,4 +189,17 @@
 			status = "disabled";
 		};
 	};
+
+	i2c@0 {
+		compatible = "i2c-gpio";
+		gpios = <&pioA 23 0 /* sda */
+			 &pioA 24 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
 };
-- 
1.7.7

[PATCH 3/5 v4] ARM: at91: usb_a9g20 add DT i2c support

[Jean-Christophe PLAGNIOL-VILLARD]

Use i2c-gpio and enable rv3029 RTC.

Enable the rtc in the sam9g20 defconfig.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
Acked-by: Rob Herring <rob.herring@calxeda.com>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: devicetree-discuss@lists.ozlabs.org
---
 arch/arm/boot/dts/usb_a9g20.dts        |    9 +++++++++
 arch/arm/configs/at91sam9g20_defconfig |    3 +++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/usb_a9g20.dts b/arch/arm/boot/dts/usb_a9g20.dts
index 71d83ef..0ea90b5 100644
--- a/arch/arm/boot/dts/usb_a9g20.dts
+++ b/arch/arm/boot/dts/usb_a9g20.dts
@@ -97,4 +97,13 @@
 			gpio-key,wakeup;
 		};
 	};
+
+	i2c@0 {
+		status = "okay";
+
+		rv3029c2@56 {
+			compatible = "rv3029c2";
+			reg = <0x56>;
+		};
+	};
 };
diff --git a/arch/arm/configs/at91sam9g20_defconfig b/arch/arm/configs/at91sam9g20_defconfig
index 9123568..994d331 100644
--- a/arch/arm/configs/at91sam9g20_defconfig
+++ b/arch/arm/configs/at91sam9g20_defconfig
@@ -74,6 +74,8 @@ CONFIG_LEGACY_PTY_COUNT=16
 CONFIG_SERIAL_ATMEL=y
 CONFIG_SERIAL_ATMEL_CONSOLE=y
 CONFIG_HW_RANDOM=y
+CONFIG_I2C=y
+CONFIG_I2C_GPIO=y
 CONFIG_SPI=y
 CONFIG_SPI_ATMEL=y
 CONFIG_SPI_SPIDEV=y
@@ -105,6 +107,7 @@ CONFIG_LEDS_TRIGGERS=y
 CONFIG_LEDS_TRIGGER_TIMER=y
 CONFIG_LEDS_TRIGGER_HEARTBEAT=y
 CONFIG_RTC_CLASS=y
+CONFIG_RTC_DRV_RV3029C2=y
 CONFIG_RTC_DRV_AT91SAM9=y
 CONFIG_EXT2_FS=y
 CONFIG_MSDOS_FS=y
-- 
1.7.7

[PATCH 4/5 v4] ARM: at91: sam9g45 add i2c DT support

[Jean-Christophe PLAGNIOL-VILLARD]

For now on use i2c-gpio driver on the same pin as the hardware IP.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
Acked-by: Rob Herring <rob.herring@calxeda.com>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: devicetree-discuss@lists.ozlabs.org
---
 arch/arm/boot/dts/at91sam9g45.dtsi |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index d79021b..92fe5a5 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi
@@ -197,4 +197,17 @@
 			status = "disabled";
 		};
 	};
+
+	i2c@0 {
+		compatible = "i2c-gpio";
+		gpios = <&pioA 20 0 /* sda */
+			 &pioA 21 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <5>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
 };
-- 
1.7.7

[PATCH 5/5 v4] ARM: at91: sam9x5 add i2c DT support

[Jean-Christophe PLAGNIOL-VILLARD]

For now on use i2c-gpio driver on the same pin as the hardware IP.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
Acked-by: Rob Herring <rob.herring@calxeda.com>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: devicetree-discuss@lists.ozlabs.org
---
 arch/arm/boot/dts/at91sam9x5.dtsi |   39 +++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi
index c294657..f0104f4 100644
--- a/arch/arm/boot/dts/at91sam9x5.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5.dtsi
@@ -188,4 +188,43 @@
 			status = "disabled";
 		};
 	};
+
+	i2c@0 {
+		compatible = "i2c-gpio";
+		gpios = <&pioA 30 0 /* sda */
+			 &pioA 31 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
+
+	i2c@1 {
+		compatible = "i2c-gpio";
+		gpios = <&pioC 0 0 /* sda */
+			 &pioC 1 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
+
+	i2c@2 {
+		compatible = "i2c-gpio";
+		gpios = <&pioB 4 0 /* sda */
+			 &pioB 5 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
 };
-- 
1.7.7

[PATCH v1 0/7] mtd: spi-nor: Add the DDR quad read support

[Huang Shijie]

 (0) This patch set depends on the patch:
	http://lists.infradead.org/pipermail/linux-mtd/2014-April/053308.html

  
 (1) This patch set tries to add the DDR quad read support for the SPI
     NOR framework, and it also adds the DDR quad read support for FREESCALE
     quadspi controller driver.

 (2) Test this patch set with Spansion s25fl128s, the performance with mtd_speedtest.ko:
     =================================================
     mtd_speedtest: MTD device: 0
     mtd_speedtest: not NAND flash, assume page size is 512 bytes.
     mtd_speedtest: MTD device size 16777216, eraseblock size 65536, page size 512,
                    count of eraseblocks 256, pages per eraseblock 128, OOB size 0
     mtd_speedtest: testing eraseblock write speed
     mtd_speedtest: eraseblock write speed is 665 KiB/s
     mtd_speedtest: testing eraseblock read speed
     mtd_speedtest: eraseblock read speed is 49799 KiB/s
     mtd_speedtest: testing page write speed
     mtd_speedtest: page write speed is 662 KiB/s
     mtd_speedtest: testing page read speed
     mtd_speedtest: page read speed is 24236 KiB/s
     mtd_speedtest: testing 2 page write speed
     mtd_speedtest: 2 page write speed is 657 KiB/s
     mtd_speedtest: testing 2 page read speed
     mtd_speedtest: 2 page read speed is 32637 KiB/s
     mtd_speedtest: Testing erase speed
     mtd_speedtest: erase speed is 518 KiB/s
     mtd_speedtest: Testing 2x multi-block erase speed
     mtd_speedtest: 2x multi-block erase speed is 506 KiB/s
     mtd_speedtest: Testing 4x multi-block erase speed
     mtd_speedtest: 4x multi-block erase speed is 503 KiB/s
     mtd_speedtest: Testing 8x multi-block erase speed
     mtd_speedtest: 8x multi-block erase speed is 501 KiB/s
     mtd_speedtest: Testing 16x multi-block erase speed
     mtd_speedtest: 16x multi-block erase speed is 498 KiB/s
     mtd_speedtest: Testing 32x multi-block erase speed
     mtd_speedtest: 32x multi-block erase speed is 496 KiB/s
     mtd_speedtest: Testing 64x multi-block erase speed
     mtd_speedtest: 64x multi-block erase speed is 495 KiB/s
     mtd_speedtest: finished
     =================================================

  (3) Conclusion:
     The DDR quad read could be 49799 KiB/s.
     
Changlog:
  About this patch set:
     For patch 1, please see the old discusstion:
     http://lists.infradead.org/pipermail/linux-mtd/2014-April/053370.html

     For patch 2, please see the old discusstion:
     http://lists.infradead.org/pipermail/linux-mtd/2014-April/053374.html

     
Huang Shijie (7):
  mtd: spi-nor: fix the wrong dummy value
  mtd: spi-nor: add DDR quad read support
  Documentation: mtd: add a new document for SPI NOR flash
  Documentation: fsl-quadspi: update the document
  mtd: fsl-quadspi: get the dummy cycles for DDR Quad read from the DT
    property
  mtd: fsl-quadspi: use the information stored in spi-nor{}
  mtd: fsl-quadspi: add the DDR quad read support

 .../devicetree/bindings/mtd/fsl-quadspi.txt        |   16 +++
 .../devicetree/bindings/mtd/spi-nor-flash.txt      |    7 +
 drivers/mtd/devices/m25p80.c                       |    5 +-
 drivers/mtd/spi-nor/fsl-quadspi.c                  |  121 +++++++++++++-------
 drivers/mtd/spi-nor/spi-nor.c                      |   52 ++++++++-
 include/linux/mtd/spi-nor.h                        |    8 +-
 6 files changed, 162 insertions(+), 47 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/spi-nor-flash.txt

-- 
1.7.2.rc3

[PATCH v1 1/7] mtd: spi-nor: fix the wrong dummy value

[Huang Shijie]

For the DDR Quad read, the dummy cycles maybe 3 or 6 which is less then 8.
The dummy cycles is actually 8 for SPI fast/dual/quad read.

This patch makes preparations for the DDR quad read, it fixes the wrong dummy
value for both the spi-nor.c and m25p80.c.

Signed-off-by: Huang Shijie <b32955-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 drivers/mtd/devices/m25p80.c  |    5 ++++-
 drivers/mtd/spi-nor/spi-nor.c |    2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 1557d8f..693e25f 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -128,9 +128,12 @@ static int m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
 	struct spi_device *spi = flash->spi;
 	struct spi_transfer t[2];
 	struct spi_message m;
-	int dummy = nor->read_dummy;
+	unsigned int dummy = nor->read_dummy;
 	int ret;
 
+	/* convert the dummy cycles to the number of byte */
+	dummy /= 8;
+
 	/* Wait till previous write/erase is done. */
 	ret = nor->wait_till_ready(nor);
 	if (ret)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index f76f3fc..1a12f81 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -77,7 +77,7 @@ static inline int spi_nor_read_dummy_cycles(struct spi_nor *nor)
 	case SPI_NOR_FAST:
 	case SPI_NOR_DUAL:
 	case SPI_NOR_QUAD:
-		return 1;
+		return 8;
 	case SPI_NOR_NORMAL:
 		return 0;
 	}
-- 
1.7.2.rc3

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

Re: [PATCH v1 1/7] mtd: spi-nor: fix the wrong dummy value

[Marek Vasut]

On Wednesday, April 23, 2014 at 12:16:49 PM, Huang Shijie wrote:
> For the DDR Quad read, the dummy cycles maybe 3 or 6 which is less then 8.
> The dummy cycles is actually 8 for SPI fast/dual/quad read.
> 
> This patch makes preparations for the DDR quad read, it fixes the wrong
> dummy value for both the spi-nor.c and m25p80.c.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>

This patch is actually V2, right ?

> ---
>  drivers/mtd/devices/m25p80.c  |    5 ++++-
>  drivers/mtd/spi-nor/spi-nor.c |    2 +-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 1557d8f..693e25f 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -128,9 +128,12 @@ static int m25p80_read(struct spi_nor *nor, loff_t
> from, size_t len, struct spi_device *spi = flash->spi;
>  	struct spi_transfer t[2];
>  	struct spi_message m;
> -	int dummy = nor->read_dummy;
> +	unsigned int dummy = nor->read_dummy;
>  	int ret;
> 
> +	/* convert the dummy cycles to the number of byte */

'bytes', plural ...
[...]
Best regards,
Marek Vasut

Re: [PATCH v1 1/7] mtd: spi-nor: fix the wrong dummy value

[Huang Shijie]

On Wed, Apr 23, 2014 at 09:41:26PM +0200, Marek Vasut wrote:
> On Wednesday, April 23, 2014 at 12:16:49 PM, Huang Shijie wrote:
> > For the DDR Quad read, the dummy cycles maybe 3 or 6 which is less then 8.
> > The dummy cycles is actually 8 for SPI fast/dual/quad read.
> > 
> > This patch makes preparations for the DDR quad read, it fixes the wrong
> > dummy value for both the spi-nor.c and m25p80.c.
> > 
> > Signed-off-by: Huang Shijie <b32955-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> 
> This patch is actually V2, right ?
yes.

I mentioned it in the cover letter. Since other patches are the V1.
I did not change it to v2.
> 
> > ---
> >  drivers/mtd/devices/m25p80.c  |    5 ++++-
> >  drivers/mtd/spi-nor/spi-nor.c |    2 +-
> >  2 files changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> > index 1557d8f..693e25f 100644
> > --- a/drivers/mtd/devices/m25p80.c
> > +++ b/drivers/mtd/devices/m25p80.c
> > @@ -128,9 +128,12 @@ static int m25p80_read(struct spi_nor *nor, loff_t
> > from, size_t len, struct spi_device *spi = flash->spi;
> >  	struct spi_transfer t[2];
> >  	struct spi_message m;
> > -	int dummy = nor->read_dummy;
> > +	unsigned int dummy = nor->read_dummy;
> >  	int ret;
> > 
> > +	/* convert the dummy cycles to the number of byte */
> 
> 'bytes', plural ...
I will change it in the next version.

thanks.

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

Re: [PATCH v1 1/7] mtd: spi-nor: fix the wrong dummy value

[Marek Vasut]

On Thursday, April 24, 2014 at 06:50:29 AM, Huang Shijie wrote:
> On Wed, Apr 23, 2014 at 09:41:26PM +0200, Marek Vasut wrote:
> > On Wednesday, April 23, 2014 at 12:16:49 PM, Huang Shijie wrote:
> > > For the DDR Quad read, the dummy cycles maybe 3 or 6 which is less then
> > > 8. The dummy cycles is actually 8 for SPI fast/dual/quad read.
> > > 
> > > This patch makes preparations for the DDR quad read, it fixes the wrong
> > > dummy value for both the spi-nor.c and m25p80.c.
> > > 
> > > Signed-off-by: Huang Shijie <b32955@freescale.com>
> > 
> > This patch is actually V2, right ?
> 
> yes.
> 
> I mentioned it in the cover letter. Since other patches are the V1.
> I did not change it to v2.

So next one is V3 ;-)

Best regards,
Marek Vasut

[PATCH v1 2/7] mtd: spi-nor: add DDR quad read support

[Huang Shijie]

This patch adds the DDR quad read support by the following:

  [1] add SPI_NOR_DDR_QUAD read mode.

  [2] add DDR Quad read opcodes:
       SPINOR_OP_READ_1_4_4_D / SPINOR_OP_READ4_1_4_4_D

  [3] add set_ddr_quad_mode() to initialize for the DDR quad read.
      Currently it only works for Spansion NOR.

  [3] set the dummy with 8 for DDR quad read.
      The m25p80.c can not support the DDR quad read, the SPI NOR controller
      can set the dummy value in its driver, such as fsl-quadspi.c.

Test this patch for Spansion s25fl128s NOR flash.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/spi-nor/spi-nor.c |   50 +++++++++++++++++++++++++++++++++++++++-
 include/linux/mtd/spi-nor.h   |    8 +++++-
 2 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 1a12f81..e2f69db 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -74,6 +74,15 @@ static int read_cr(struct spi_nor *nor)
 static inline int spi_nor_read_dummy_cycles(struct spi_nor *nor)
 {
 	switch (nor->flash_read) {
+	case SPI_NOR_DDR_QUAD:
+		/*
+		 * The m25p80.c can not support the DDR quad read.
+		 * We set the dummy cycles to 8 by default. If the SPI NOR
+		 * controller driver has already set it before call the
+		 * spi_nor_scan(), we just keep it as it is.
+		 */
+		if (nor->read_dummy)
+			return nor->read_dummy;
 	case SPI_NOR_FAST:
 	case SPI_NOR_DUAL:
 	case SPI_NOR_QUAD:
@@ -402,6 +411,7 @@ struct flash_info {
 #define	SECT_4K_PMC		0x10	/* SPINOR_OP_BE_4K_PMC works uniformly */
 #define	SPI_NOR_DUAL_READ	0x20    /* Flash supports Dual Read */
 #define	SPI_NOR_QUAD_READ	0x40    /* Flash supports Quad Read */
+#define	SPI_NOR_DDR_QUAD_READ	0x80    /* Flash supports DDR Quad Read */
 };
 
 #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
@@ -846,6 +856,24 @@ static int spansion_quad_enable(struct spi_nor *nor)
 	return 0;
 }
 
+static int set_ddr_quad_mode(struct spi_nor *nor, u32 jedec_id)
+{
+	int status;
+
+	switch (JEDEC_MFR(jedec_id)) {
+	case CFI_MFR_AMD: /* Spansion, actually */
+		status = spansion_quad_enable(nor);
+		if (status) {
+			dev_err(nor->dev,
+				"Spansion DDR quad-read not enabled\n");
+			return -EINVAL;
+		}
+		return status;
+	default:
+		return -EINVAL;
+	}
+}
+
 static int set_quad_mode(struct spi_nor *nor, u32 jedec_id)
 {
 	int status;
@@ -1016,8 +1044,15 @@ int spi_nor_scan(struct spi_nor *nor, const struct spi_device_id *id,
 	if (info->flags & SPI_NOR_NO_FR)
 		nor->flash_read = SPI_NOR_NORMAL;
 
-	/* Quad/Dual-read mode takes precedence over fast/normal */
-	if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
+	/* DDR Quad/Quad/Dual-read mode takes precedence over fast/normal */
+	if (mode == SPI_NOR_DDR_QUAD && info->flags & SPI_NOR_DDR_QUAD_READ) {
+		ret = set_ddr_quad_mode(nor, info->jedec_id);
+		if (ret) {
+			dev_err(dev, "DDR quad mode not supported\n");
+			return ret;
+		}
+		nor->flash_read = SPI_NOR_DDR_QUAD;
+	} else if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
 		ret = set_quad_mode(nor, info->jedec_id);
 		if (ret) {
 			dev_err(dev, "quad mode not supported\n");
@@ -1030,6 +1065,14 @@ int spi_nor_scan(struct spi_nor *nor, const struct spi_device_id *id,
 
 	/* Default commands */
 	switch (nor->flash_read) {
+	case SPI_NOR_DDR_QUAD:
+		if (JEDEC_MFR(info->jedec_id) == CFI_MFR_AMD) { /* Spansion */
+			nor->read_opcode = SPINOR_OP_READ_1_4_4_D;
+		} else {
+			dev_err(dev, "DDR Quad Read is not supported.\n");
+			return -EINVAL;
+		}
+		break;
 	case SPI_NOR_QUAD:
 		nor->read_opcode = SPINOR_OP_READ_1_1_4;
 		break;
@@ -1057,6 +1100,9 @@ int spi_nor_scan(struct spi_nor *nor, const struct spi_device_id *id,
 		if (JEDEC_MFR(info->jedec_id) == CFI_MFR_AMD) {
 			/* Dedicated 4-byte command set */
 			switch (nor->flash_read) {
+			case SPI_NOR_DDR_QUAD:
+				nor->read_opcode = SPINOR_OP_READ4_1_4_4_D;
+				break;
 			case SPI_NOR_QUAD:
 				nor->read_opcode = SPINOR_OP_READ4_1_1_4;
 				break;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 5324184..fea7769 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -12,10 +12,11 @@
 
 /*
  * Note on opcode nomenclature: some opcodes have a format like
- * SPINOR_OP_FUNCTION{4,}_x_y_z. The numbers x, y, and z stand for the number
+ * SPINOR_OP_FUNCTION{4,}_x_y_z{_D}. The numbers x, y, and z stand for the number
  * of I/O lines used for the opcode, address, and data (respectively). The
  * FUNCTION has an optional suffix of '4', to represent an opcode which
- * requires a 4-byte (32-bit) address.
+ * requires a 4-byte (32-bit) address. The suffix of 'D' stands for the
+ * DDR mode.
  */
 
 /* Flash opcodes. */
@@ -26,6 +27,7 @@
 #define SPINOR_OP_READ_FAST	0x0b	/* Read data bytes (high frequency) */
 #define SPINOR_OP_READ_1_1_2	0x3b	/* Read data bytes (Dual SPI) */
 #define SPINOR_OP_READ_1_1_4	0x6b	/* Read data bytes (Quad SPI) */
+#define SPINOR_OP_READ_1_4_4_D	0xed	/* Read data bytes (DDR Quad SPI) */
 #define SPINOR_OP_PP		0x02	/* Page program (up to 256 bytes) */
 #define SPINOR_OP_BE_4K		0x20	/* Erase 4KiB block */
 #define SPINOR_OP_BE_4K_PMC	0xd7	/* Erase 4KiB block on PMC chips */
@@ -40,6 +42,7 @@
 #define SPINOR_OP_READ4_FAST	0x0c	/* Read data bytes (high frequency) */
 #define SPINOR_OP_READ4_1_1_2	0x3c	/* Read data bytes (Dual SPI) */
 #define SPINOR_OP_READ4_1_1_4	0x6c	/* Read data bytes (Quad SPI) */
+#define SPINOR_OP_READ4_1_4_4_D	0xee	/* Read data bytes (DDR Quad SPI) */
 #define SPINOR_OP_PP_4B		0x12	/* Page program (up to 256 bytes) */
 #define SPINOR_OP_SE_4B		0xdc	/* Sector erase (usually 64KiB) */
 
@@ -74,6 +77,7 @@ enum read_mode {
 	SPI_NOR_FAST,
 	SPI_NOR_DUAL,
 	SPI_NOR_QUAD,
+	SPI_NOR_DDR_QUAD,
 };
 
 /**
-- 
1.7.2.rc3

Re: [PATCH v1 2/7] mtd: spi-nor: add DDR quad read support

[Marek Vasut]

On Wednesday, April 23, 2014 at 12:16:50 PM, Huang Shijie wrote:
> This patch adds the DDR quad read support by the following:
> 
>   [1] add SPI_NOR_DDR_QUAD read mode.
> 
>   [2] add DDR Quad read opcodes:
>        SPINOR_OP_READ_1_4_4_D / SPINOR_OP_READ4_1_4_4_D
> 
>   [3] add set_ddr_quad_mode() to initialize for the DDR quad read.
>       Currently it only works for Spansion NOR.
> 
>   [3] set the dummy with 8 for DDR quad read.
>       The m25p80.c can not support the DDR quad read, the SPI NOR
> controller can set the dummy value in its driver, such as fsl-quadspi.c.
> 
> Test this patch for Spansion s25fl128s NOR flash.
> 
> Signed-off-by: Huang Shijie <b32955-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
>  drivers/mtd/spi-nor/spi-nor.c |   50
> +++++++++++++++++++++++++++++++++++++++- include/linux/mtd/spi-nor.h   |  
>  8 +++++-
>  2 files changed, 54 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 1a12f81..e2f69db 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -74,6 +74,15 @@ static int read_cr(struct spi_nor *nor)
>  static inline int spi_nor_read_dummy_cycles(struct spi_nor *nor)
>  {
>  	switch (nor->flash_read) {
> +	case SPI_NOR_DDR_QUAD:
> +		/*
> +		 * The m25p80.c can not support the DDR quad read.
> +		 * We set the dummy cycles to 8 by default. If the SPI NOR
> +		 * controller driver has already set it before call the
> +		 * spi_nor_scan(), we just keep it as it is.
> +		 */
> +		if (nor->read_dummy)
> +			return nor->read_dummy;

Can the controller set this variable to zero ?

[...]

> +static int set_ddr_quad_mode(struct spi_nor *nor, u32 jedec_id)
> +{
> +	int status;
> +
> +	switch (JEDEC_MFR(jedec_id)) {
> +	case CFI_MFR_AMD: /* Spansion, actually */
> +		status = spansion_quad_enable(nor);
> +		if (status) {
> +			dev_err(nor->dev,
> +				"Spansion DDR quad-read not enabled\n");
> +			return -EINVAL;

Can't you just return status here as well to propagate the failure?
[...]
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 2/7] mtd: spi-nor: add DDR quad read support

[Huang Shijie]

On Wed, Apr 23, 2014 at 09:45:57PM +0200, Marek Vasut wrote:
> On Wednesday, April 23, 2014 at 12:16:50 PM, Huang Shijie wrote:
> > This patch adds the DDR quad read support by the following:
> > 
> >   [1] add SPI_NOR_DDR_QUAD read mode.
> > 
> >   [2] add DDR Quad read opcodes:
> >        SPINOR_OP_READ_1_4_4_D / SPINOR_OP_READ4_1_4_4_D
> > 
> >   [3] add set_ddr_quad_mode() to initialize for the DDR quad read.
> >       Currently it only works for Spansion NOR.
> > 
> >   [3] set the dummy with 8 for DDR quad read.
> >       The m25p80.c can not support the DDR quad read, the SPI NOR
> > controller can set the dummy value in its driver, such as fsl-quadspi.c.
> > 
> > Test this patch for Spansion s25fl128s NOR flash.
> > 
> > Signed-off-by: Huang Shijie <b32955@freescale.com>
> > ---
> >  drivers/mtd/spi-nor/spi-nor.c |   50
> > +++++++++++++++++++++++++++++++++++++++- include/linux/mtd/spi-nor.h   |  
> >  8 +++++-
> >  2 files changed, 54 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> > index 1a12f81..e2f69db 100644
> > --- a/drivers/mtd/spi-nor/spi-nor.c
> > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > @@ -74,6 +74,15 @@ static int read_cr(struct spi_nor *nor)
> >  static inline int spi_nor_read_dummy_cycles(struct spi_nor *nor)
> >  {
> >  	switch (nor->flash_read) {
> > +	case SPI_NOR_DDR_QUAD:
> > +		/*
> > +		 * The m25p80.c can not support the DDR quad read.
> > +		 * We set the dummy cycles to 8 by default. If the SPI NOR
> > +		 * controller driver has already set it before call the
> > +		 * spi_nor_scan(), we just keep it as it is.
> > +		 */
> > +		if (nor->read_dummy)
> > +			return nor->read_dummy;
> 
> Can the controller set this variable to zero ?

The default value of this variable is zero.  It it meaningless to set zero.
The DDR Quad read definitely will use a non-zero dummy.
> 
> [...]
> 
> > +static int set_ddr_quad_mode(struct spi_nor *nor, u32 jedec_id)
> > +{
> > +	int status;
> > +
> > +	switch (JEDEC_MFR(jedec_id)) {
> > +	case CFI_MFR_AMD: /* Spansion, actually */
> > +		status = spansion_quad_enable(nor);
> > +		if (status) {
> > +			dev_err(nor->dev,
> > +				"Spansion DDR quad-read not enabled\n");
> > +			return -EINVAL;
> 
> Can't you just return status here as well to propagate the failure?

ok. 

thanks
Huang Shijie

Re: [PATCH v1 2/7] mtd: spi-nor: add DDR quad read support

[Marek Vasut]

On Thursday, April 24, 2014 at 06:53:34 AM, Huang Shijie wrote:
> On Wed, Apr 23, 2014 at 09:45:57PM +0200, Marek Vasut wrote:
> > On Wednesday, April 23, 2014 at 12:16:50 PM, Huang Shijie wrote:
> > > This patch adds the DDR quad read support by the following:
> > >   [1] add SPI_NOR_DDR_QUAD read mode.
> > >   
> > >   [2] add DDR Quad read opcodes:
> > >        SPINOR_OP_READ_1_4_4_D / SPINOR_OP_READ4_1_4_4_D
> > >   
> > >   [3] add set_ddr_quad_mode() to initialize for the DDR quad read.
> > >   
> > >       Currently it only works for Spansion NOR.
> > >   
> > >   [3] set the dummy with 8 for DDR quad read.
> > >   
> > >       The m25p80.c can not support the DDR quad read, the SPI NOR
> > > 
> > > controller can set the dummy value in its driver, such as
> > > fsl-quadspi.c.
> > > 
> > > Test this patch for Spansion s25fl128s NOR flash.
> > > 
> > > Signed-off-by: Huang Shijie <b32955-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > > ---
> > > 
> > >  drivers/mtd/spi-nor/spi-nor.c |   50
> > > 
> > > +++++++++++++++++++++++++++++++++++++++- include/linux/mtd/spi-nor.h  
> > > |
> > > 
> > >  8 +++++-
> > >  2 files changed, 54 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > > b/drivers/mtd/spi-nor/spi-nor.c index 1a12f81..e2f69db 100644
> > > --- a/drivers/mtd/spi-nor/spi-nor.c
> > > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > > @@ -74,6 +74,15 @@ static int read_cr(struct spi_nor *nor)
> > > 
> > >  static inline int spi_nor_read_dummy_cycles(struct spi_nor *nor)
> > >  {
> > >  
> > >  	switch (nor->flash_read) {
> > > 
> > > +	case SPI_NOR_DDR_QUAD:
> > > +		/*
> > > +		 * The m25p80.c can not support the DDR quad read.
> > > +		 * We set the dummy cycles to 8 by default. If the SPI NOR
> > > +		 * controller driver has already set it before call the
> > > +		 * spi_nor_scan(), we just keep it as it is.
> > > +		 */
> > > +		if (nor->read_dummy)
> > > +			return nor->read_dummy;
> > 
> > Can the controller set this variable to zero ?
> 
> The default value of this variable is zero.  It it meaningless to set zero.
> The DDR Quad read definitely will use a non-zero dummy.

Can you back this by some documentation please ?

I mean, I know I am a hardass here, but please understand I'd like to understand 
the decisions that are being made.
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 2/7] mtd: spi-nor: add DDR quad read support

[Huang Shijie]

On Thu, Apr 24, 2014 at 03:43:51PM +0200, Marek Vasut wrote:
> On Thursday, April 24, 2014 at 06:53:34 AM, Huang Shijie wrote:
> > On Wed, Apr 23, 2014 at 09:45:57PM +0200, Marek Vasut wrote:
> > > On Wednesday, April 23, 2014 at 12:16:50 PM, Huang Shijie wrote:
> > > > This patch adds the DDR quad read support by the following:
> > > >   [1] add SPI_NOR_DDR_QUAD read mode.
> > > >   
> > > >   [2] add DDR Quad read opcodes:
> > > >        SPINOR_OP_READ_1_4_4_D / SPINOR_OP_READ4_1_4_4_D
> > > >   
> > > >   [3] add set_ddr_quad_mode() to initialize for the DDR quad read.
> > > >   
> > > >       Currently it only works for Spansion NOR.
> > > >   
> > > >   [3] set the dummy with 8 for DDR quad read.
> > > >   
> > > >       The m25p80.c can not support the DDR quad read, the SPI NOR
> > > > 
> > > > controller can set the dummy value in its driver, such as
> > > > fsl-quadspi.c.
> > > > 
> > > > Test this patch for Spansion s25fl128s NOR flash.
> > > > 
> > > > Signed-off-by: Huang Shijie <b32955-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > > > ---
> > > > 
> > > >  drivers/mtd/spi-nor/spi-nor.c |   50
> > > > 
> > > > +++++++++++++++++++++++++++++++++++++++- include/linux/mtd/spi-nor.h  
> > > > |
> > > > 
> > > >  8 +++++-
> > > >  2 files changed, 54 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > > > b/drivers/mtd/spi-nor/spi-nor.c index 1a12f81..e2f69db 100644
> > > > --- a/drivers/mtd/spi-nor/spi-nor.c
> > > > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > > > @@ -74,6 +74,15 @@ static int read_cr(struct spi_nor *nor)
> > > > 
> > > >  static inline int spi_nor_read_dummy_cycles(struct spi_nor *nor)
> > > >  {
> > > >  
> > > >  	switch (nor->flash_read) {
> > > > 
> > > > +	case SPI_NOR_DDR_QUAD:
> > > > +		/*
> > > > +		 * The m25p80.c can not support the DDR quad read.
> > > > +		 * We set the dummy cycles to 8 by default. If the SPI NOR
> > > > +		 * controller driver has already set it before call the
> > > > +		 * spi_nor_scan(), we just keep it as it is.
> > > > +		 */
> > > > +		if (nor->read_dummy)
> > > > +			return nor->read_dummy;
> > > 
> > > Can the controller set this variable to zero ?
> > 
> > The default value of this variable is zero.  It it meaningless to set zero.
> > The DDR Quad read definitely will use a non-zero dummy.
> 
> Can you back this by some documentation please ?

I can not find a document for this. :(

But i have decided to parse out the DT node in here, not in the driver.


thanks
Huang Shijie

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

[PATCH v1 3/7] Documentation: mtd: add a new document for SPI NOR flash

[Huang Shijie]

We need a DT property to store the dummy cycles for DDR Quad read.
This is a common feature for the SPI NOR flash, such as Spansion and Micron
chips.

Add this file to describe this specific SPI NOR flash features which will
be referred by the SPI NOR flash drivers.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 .../devicetree/bindings/mtd/spi-nor-flash.txt      |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/spi-nor-flash.txt

diff --git a/Documentation/devicetree/bindings/mtd/spi-nor-flash.txt b/Documentation/devicetree/bindings/mtd/spi-nor-flash.txt
new file mode 100644
index 0000000..aba4d54
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/spi-nor-flash.txt
@@ -0,0 +1,7 @@
+This file defines some DT properties for specific SPI NOR flash features.
+The SPI NOR controller drivers may refer to this file, such as fsl-quadspi.txt
+
+Optional properties:
+  - spi-nor,ddr-quad-read-dummy: The dummy cycles used by the DDR Quad read.
+                                 Please refer to the chip's datasheet. This
+                                 property can be 4 or 6 which is less then 8.
-- 
1.7.2.rc3

[PATCH v1 4/7] Documentation: fsl-quadspi: update the document

[Huang Shijie]

The patch updates the document by adding more information to describe the
DT proporties used by the Freescale Quadspi driver and the childs nodes.

For the child node for SPI NOR flash, we add the required property
("spi-max-frequency"), and refer to spi-nor-flash.txt for the optional
properties.

Signed-off-by: Huang Shijie <b32955-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 .../devicetree/bindings/mtd/fsl-quadspi.txt        |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/fsl-quadspi.txt b/Documentation/devicetree/bindings/mtd/fsl-quadspi.txt
index 823d134..7e1dbaf 100644
--- a/Documentation/devicetree/bindings/mtd/fsl-quadspi.txt
+++ b/Documentation/devicetree/bindings/mtd/fsl-quadspi.txt
@@ -1,5 +1,11 @@
 * Freescale Quad Serial Peripheral Interface(QuadSPI)
 
+The QuadSPI controller acts as the SPI master. It is described with a node
+for the controller and a set of child nodes for each SPI NOR flash.
+
+Part I - The DT node for the controller:
+------------------------------
+
 Required properties:
   - compatible : Should be "fsl,vf610-qspi"
   - reg : the first contains the register location and length,
@@ -18,6 +24,16 @@ Optional properties:
 			      bus, you should enable this property.
 			      (Please check the board's schematic.)
 
+Part II - The DT nodes for each SPI NOR flash
+------------------------------
+Required properties:
+- spi-max-frequency : Maximum frequency of the SPI bus the chip can operate at
+
+Optional properties:
+  Please refer to the Documentation/devicetree/bindings/mtd/spi-nor-flash.txt
+  If you set the "spi-nor,ddr-quad-read-dummy", it means you enable the DDR
+  quad read feature for the driver.
+
 Example:
 
 qspi0: quadspi@40044000 {
-- 
1.7.2.rc3

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

[PATCH v1 5/7] mtd: fsl-quadspi: get the dummy cycles for DDR Quad read from the DT property

[Huang Shijie]

Check the "spi-nor,ddr-quad-read-dummy" DT property to get the
dummy cycles for DDR quad read.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 8d659a2..15bdeb9 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -883,6 +883,7 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 	for_each_available_child_of_node(dev->of_node, np) {
 		const struct spi_device_id *id;
 		char modalias[40];
+		u32 dummy = 0;
 
 		/* skip the holes */
 		if (!has_second_chip)
@@ -918,6 +919,12 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 		if (ret < 0)
 			goto map_failed;
 
+		/* Set the dummy cycles for the DDR Quad Read */
+		ret = of_property_read_u32(np, "spi-nor,ddr-quad-read-dummy",
+				&dummy);
+		if (!ret && dummy > 0 && dummy < 8)
+			nor->read_dummy = dummy;
+
 		/* set the chip address for READID */
 		fsl_qspi_set_base_addr(q, nor);
 
-- 
1.7.2.rc3

Re: [PATCH v1 5/7] mtd: fsl-quadspi: get the dummy cycles for DDR Quad read from the DT property

[Marek Vasut]

On Wednesday, April 23, 2014 at 12:16:53 PM, Huang Shijie wrote:
> Check the "spi-nor,ddr-quad-read-dummy" DT property to get the
> dummy cycles for DDR quad read.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c
> b/drivers/mtd/spi-nor/fsl-quadspi.c index 8d659a2..15bdeb9 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -883,6 +883,7 @@ static int fsl_qspi_probe(struct platform_device *pdev)
>  	for_each_available_child_of_node(dev->of_node, np) {
>  		const struct spi_device_id *id;
>  		char modalias[40];
> +		u32 dummy = 0;
> 
>  		/* skip the holes */
>  		if (!has_second_chip)
> @@ -918,6 +919,12 @@ static int fsl_qspi_probe(struct platform_device
> *pdev) if (ret < 0)
>  			goto map_failed;
> 
> +		/* Set the dummy cycles for the DDR Quad Read */
> +		ret = of_property_read_u32(np, "spi-nor,ddr-quad-read-dummy",
> +				&dummy);
> +		if (!ret && dummy > 0 && dummy < 8)
> +			nor->read_dummy = dummy;

Is there any reason for the upper limit on number of dummy cycles ?

Best regards,
Marek Vasut

Re: [PATCH v1 5/7] mtd: fsl-quadspi: get the dummy cycles for DDR Quad read from the DT property

[Huang Shijie]

On Wed, Apr 23, 2014 at 09:48:50PM +0200, Marek Vasut wrote:
> On Wednesday, April 23, 2014 at 12:16:53 PM, Huang Shijie wrote:
> > Check the "spi-nor,ddr-quad-read-dummy" DT property to get the
> > dummy cycles for DDR quad read.
> > 
> > Signed-off-by: Huang Shijie <b32955-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > ---
> >  drivers/mtd/spi-nor/fsl-quadspi.c |    7 +++++++
> >  1 files changed, 7 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c
> > b/drivers/mtd/spi-nor/fsl-quadspi.c index 8d659a2..15bdeb9 100644
> > --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> > +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> > @@ -883,6 +883,7 @@ static int fsl_qspi_probe(struct platform_device *pdev)
> >  	for_each_available_child_of_node(dev->of_node, np) {
> >  		const struct spi_device_id *id;
> >  		char modalias[40];
> > +		u32 dummy = 0;
> > 
> >  		/* skip the holes */
> >  		if (!has_second_chip)
> > @@ -918,6 +919,12 @@ static int fsl_qspi_probe(struct platform_device
> > *pdev) if (ret < 0)
> >  			goto map_failed;
> > 
> > +		/* Set the dummy cycles for the DDR Quad Read */
> > +		ret = of_property_read_u32(np, "spi-nor,ddr-quad-read-dummy",
> > +				&dummy);
> > +		if (!ret && dummy > 0 && dummy < 8)
> > +			nor->read_dummy = dummy;
> 
> Is there any reason for the upper limit on number of dummy cycles ?
I actually tested two kinds of NOR flash, the Spansion s25fl128s and Micron N25q256a.
The dummy cycles are all less then 8.

but i admit the upper limit here is not proper.
I can remove it in the next version.

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

Re: [PATCH v1 5/7] mtd: fsl-quadspi: get the dummy cycles for DDR Quad read from the DT property

[Marek Vasut]

On Thursday, April 24, 2014 at 06:58:40 AM, Huang Shijie wrote:
> On Wed, Apr 23, 2014 at 09:48:50PM +0200, Marek Vasut wrote:
> > On Wednesday, April 23, 2014 at 12:16:53 PM, Huang Shijie wrote:
> > > Check the "spi-nor,ddr-quad-read-dummy" DT property to get the
> > > dummy cycles for DDR quad read.
> > > 
> > > Signed-off-by: Huang Shijie <b32955-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > > ---
> > > 
> > >  drivers/mtd/spi-nor/fsl-quadspi.c |    7 +++++++
> > >  1 files changed, 7 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c
> > > b/drivers/mtd/spi-nor/fsl-quadspi.c index 8d659a2..15bdeb9 100644
> > > --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> > > +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> > > @@ -883,6 +883,7 @@ static int fsl_qspi_probe(struct platform_device
> > > *pdev)
> > > 
> > >  	for_each_available_child_of_node(dev->of_node, np) {
> > >  	
> > >  		const struct spi_device_id *id;
> > >  		char modalias[40];
> > > 
> > > +		u32 dummy = 0;
> > > 
> > >  		/* skip the holes */
> > >  		if (!has_second_chip)
> > > 
> > > @@ -918,6 +919,12 @@ static int fsl_qspi_probe(struct platform_device
> > > *pdev) if (ret < 0)
> > > 
> > >  			goto map_failed;
> > > 
> > > +		/* Set the dummy cycles for the DDR Quad Read */
> > > +		ret = of_property_read_u32(np, "spi-nor,ddr-quad-read-dummy",
> > > +				&dummy);
> > > +		if (!ret && dummy > 0 && dummy < 8)
> > > +			nor->read_dummy = dummy;
> > 
> > Is there any reason for the upper limit on number of dummy cycles ?
> 
> I actually tested two kinds of NOR flash, the Spansion s25fl128s and Micron
> N25q256a. The dummy cycles are all less then 8.

I'm all for having a limit if you can justify why number 8 is the upper limit. 
If there's no justification for it, let's remove it.

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

Re: [PATCH v1 5/7] mtd: fsl-quadspi: get the dummy cycles for DDR Quad read from the DT property

[Huang Shijie]

On Thu, Apr 24, 2014 at 03:41:54PM +0200, Marek Vasut wrote:
> On Thursday, April 24, 2014 at 06:58:40 AM, Huang Shijie wrote:
> > On Wed, Apr 23, 2014 at 09:48:50PM +0200, Marek Vasut wrote:
> > > On Wednesday, April 23, 2014 at 12:16:53 PM, Huang Shijie wrote:
> > > > Check the "spi-nor,ddr-quad-read-dummy" DT property to get the
> > > > dummy cycles for DDR quad read.
> > > > 
> > > > Signed-off-by: Huang Shijie <b32955@freescale.com>
> > > > ---
> > > > 
> > > >  drivers/mtd/spi-nor/fsl-quadspi.c |    7 +++++++
> > > >  1 files changed, 7 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c
> > > > b/drivers/mtd/spi-nor/fsl-quadspi.c index 8d659a2..15bdeb9 100644
> > > > --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> > > > +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> > > > @@ -883,6 +883,7 @@ static int fsl_qspi_probe(struct platform_device
> > > > *pdev)
> > > > 
> > > >  	for_each_available_child_of_node(dev->of_node, np) {
> > > >  	
> > > >  		const struct spi_device_id *id;
> > > >  		char modalias[40];
> > > > 
> > > > +		u32 dummy = 0;
> > > > 
> > > >  		/* skip the holes */
> > > >  		if (!has_second_chip)
> > > > 
> > > > @@ -918,6 +919,12 @@ static int fsl_qspi_probe(struct platform_device
> > > > *pdev) if (ret < 0)
> > > > 
> > > >  			goto map_failed;
> > > > 
> > > > +		/* Set the dummy cycles for the DDR Quad Read */
> > > > +		ret = of_property_read_u32(np, "spi-nor,ddr-quad-read-dummy",
> > > > +				&dummy);
> > > > +		if (!ret && dummy > 0 && dummy < 8)
> > > > +			nor->read_dummy = dummy;
> > > 
> > > Is there any reason for the upper limit on number of dummy cycles ?
> > 
> > I actually tested two kinds of NOR flash, the Spansion s25fl128s and Micron
> > N25q256a. The dummy cycles are all less then 8.
> 
> I'm all for having a limit if you can justify why number 8 is the upper limit. 
> If there's no justification for it, let's remove it.
ok.

thanks
Huang Shijie

[PATCH v1 6/7] mtd: fsl-quadspi: use the information stored in spi-nor{}

[Huang Shijie]

We can get the read/write/erase opcode from the spi nor framework now.
What's more is that we can get the correct dummy cycles.

This patch uses the information stored in the spi_nor{} to remove the
hardcode in the fsl_qspi_init_lut().

Signed-off-by: Huang Shijie <b32955-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 drivers/mtd/spi-nor/fsl-quadspi.c |   57 ++++++++++++------------------------
 1 files changed, 19 insertions(+), 38 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 15bdeb9..0a2901e 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -280,8 +280,10 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 {
 	void __iomem *base = q->iobase;
 	int rxfifo = q->devtype_data->rxfifo;
+	struct spi_nor *nor = &q->nor[0];
+	u8 addrlen = (nor->addr_width == 3) ? ADDR24BIT : ADDR32BIT;
 	u32 lut_base;
-	u8 cmd, addrlen, dummy;
+	u8 op, dm;
 	int i;
 
 	fsl_qspi_unlock_lut(q);
@@ -292,40 +294,29 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 
 	/* Quad Read */
 	lut_base = SEQID_QUAD_READ * 4;
-
-	if (q->nor_size <= SZ_16M) {
-		cmd = SPINOR_OP_READ_1_1_4;
-		addrlen = ADDR24BIT;
-		dummy = 8;
-	} else {
-		/* use the 4-byte address */
-		cmd = SPINOR_OP_READ_1_1_4;
-		addrlen = ADDR32BIT;
-		dummy = 8;
+	op = nor->read_opcode;
+	dm = nor->read_dummy;
+	if (nor->flash_read == SPI_NOR_QUAD) {
+		if (op == SPINOR_OP_READ_1_1_4 || op == SPINOR_OP_READ4_1_1_4) {
+			/* read mode : 1-1-4 */
+			writel(LUT0(CMD, PAD1, op) | LUT1(ADDR, PAD1, addrlen),
+				base + QUADSPI_LUT(lut_base));
+
+			writel(LUT0(DUMMY, PAD1, dm) | LUT1(READ, PAD4, rxfifo),
+				base + QUADSPI_LUT(lut_base + 1));
+		} else {
+			dev_err(nor->dev,
+				"Unsupported cmd:%.2x\n", nor->read_opcode);
+		}
 	}
 
-	writel(LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
-			base + QUADSPI_LUT(lut_base));
-	writel(LUT0(DUMMY, PAD1, dummy) | LUT1(READ, PAD4, rxfifo),
-			base + QUADSPI_LUT(lut_base + 1));
-
 	/* Write enable */
 	lut_base = SEQID_WREN * 4;
 	writel(LUT0(CMD, PAD1, SPINOR_OP_WREN), base + QUADSPI_LUT(lut_base));
 
 	/* Page Program */
 	lut_base = SEQID_PP * 4;
-
-	if (q->nor_size <= SZ_16M) {
-		cmd = SPINOR_OP_PP;
-		addrlen = ADDR24BIT;
-	} else {
-		/* use the 4-byte address */
-		cmd = SPINOR_OP_PP;
-		addrlen = ADDR32BIT;
-	}
-
-	writel(LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
+	writel(LUT0(CMD, PAD1, nor->program_opcode) | LUT1(ADDR, PAD1, addrlen),
 			base + QUADSPI_LUT(lut_base));
 	writel(LUT0(WRITE, PAD1, 0), base + QUADSPI_LUT(lut_base + 1));
 
@@ -336,17 +327,7 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 
 	/* Erase a sector */
 	lut_base = SEQID_SE * 4;
-
-	if (q->nor_size <= SZ_16M) {
-		cmd = SPINOR_OP_SE;
-		addrlen = ADDR24BIT;
-	} else {
-		/* use the 4-byte address */
-		cmd = SPINOR_OP_SE;
-		addrlen = ADDR32BIT;
-	}
-
-	writel(LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
+	writel(LUT0(CMD, PAD1, nor->erase_opcode) | LUT1(ADDR, PAD1, addrlen),
 			base + QUADSPI_LUT(lut_base));
 
 	/* Erase the whole chip */
-- 
1.7.2.rc3

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

[PATCH v1 7/7] mtd: fsl-quadspi: add the DDR quad read support

[Huang Shijie]

Add the DDR quad read support for the fsl-quadspi driver.

 (1) Test this patch with imx6sx-sdb board (Spansion s25fl128s)
     The clock rate is 66MHz.

 (2) The information of NOR flash:
     -----------------------------------------------
     root@imx6qdlsolo:~# mtdinfo /dev/mtd0
     mtd0
     Name:                           21e4000.qspi
     Type:                           nor
     Eraseblock size:                65536 bytes, 64.0 KiB
     Amount of eraseblocks:          256 (16777216 bytes, 16.0 MiB)
     Minimum input/output unit size: 1 byte
     Sub-page size:                  1 byte
     Character device major/minor:   90:0
     Bad blocks are allowed:         false
     Device is writable:             true
     -----------------------------------------------

 (3) Test this patch set with UBIFS & bonnie++:
     -----------------------------------------------
	ubiattach /dev/ubi_ctrl -m 0
	ubimkvol /dev/ubi0 -N test -m
	mount -t ubifs ubi0:test tmp
	bonnie++ -d tmp -u 0 -s 10 -r 5
     -----------------------------------------------

 (4) Test this patch with mtd_speedtest.ko

     root@imx6qdlsolo:~# insmod mtd_speedtest.ko dev=0
     =================================================
     mtd_speedtest: MTD device: 0
     mtd_speedtest: not NAND flash, assume page size is 512 bytes.
     mtd_speedtest: MTD device size 16777216, eraseblock size 65536, page size 512,
                    count of eraseblocks 256, pages per eraseblock 128, OOB size 0
     mtd_speedtest: testing eraseblock write speed
     mtd_speedtest: eraseblock write speed is 665 KiB/s
     mtd_speedtest: testing eraseblock read speed
     mtd_speedtest: eraseblock read speed is 49799 KiB/s
     mtd_speedtest: testing page write speed
     mtd_speedtest: page write speed is 662 KiB/s
     mtd_speedtest: testing page read speed
     mtd_speedtest: page read speed is 24236 KiB/s
     mtd_speedtest: testing 2 page write speed
     mtd_speedtest: 2 page write speed is 657 KiB/s
     mtd_speedtest: testing 2 page read speed
     mtd_speedtest: 2 page read speed is 32637 KiB/s
     mtd_speedtest: Testing erase speed
     mtd_speedtest: erase speed is 518 KiB/s
     mtd_speedtest: Testing 2x multi-block erase speed
     mtd_speedtest: 2x multi-block erase speed is 506 KiB/s
     mtd_speedtest: Testing 4x multi-block erase speed
     mtd_speedtest: 4x multi-block erase speed is 503 KiB/s
     mtd_speedtest: Testing 8x multi-block erase speed
     mtd_speedtest: 8x multi-block erase speed is 501 KiB/s
     mtd_speedtest: Testing 16x multi-block erase speed
     mtd_speedtest: 16x multi-block erase speed is 498 KiB/s
     mtd_speedtest: Testing 32x multi-block erase speed
     mtd_speedtest: 32x multi-block erase speed is 496 KiB/s
     mtd_speedtest: Testing 64x multi-block erase speed
     mtd_speedtest: 64x multi-block erase speed is 495 KiB/s
     mtd_speedtest: finished
     =================================================

  (5) Conclusion:
     The DDR quad read could be 49799 KiB/s.

Signed-off-by: Huang Shijie <b32955-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 drivers/mtd/spi-nor/fsl-quadspi.c |   59 ++++++++++++++++++++++++++++++++++--
 1 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 0a2901e..fcb963f 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -29,6 +29,9 @@
 
 /* The registers */
 #define QUADSPI_MCR			0x00
+#define MX6SX_QUADSPI_MCR_TX_DDR_DELAY_EN_SHIFT	29
+#define MX6SX_QUADSPI_MCR_TX_DDR_DELAY_EN_MASK	\
+				(1 << MX6SX_QUADSPI_MCR_TX_DDR_DELAY_EN_SHIFT)
 #define QUADSPI_MCR_RESERVED_SHIFT	16
 #define QUADSPI_MCR_RESERVED_MASK	(0xF << QUADSPI_MCR_RESERVED_SHIFT)
 #define QUADSPI_MCR_MDIS_SHIFT		14
@@ -292,7 +295,7 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 	for (i = 0; i < QUADSPI_LUT_NUM; i++)
 		writel(0, base + QUADSPI_LUT_BASE + i * 4);
 
-	/* Quad Read */
+	/* Quad Read and DDR Quad Read*/
 	lut_base = SEQID_QUAD_READ * 4;
 	op = nor->read_opcode;
 	dm = nor->read_dummy;
@@ -308,6 +311,24 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 			dev_err(nor->dev,
 				"Unsupported cmd:%.2x\n", nor->read_opcode);
 		}
+	} else if (nor->flash_read == SPI_NOR_DDR_QUAD) {
+		if (op == SPINOR_OP_READ_1_4_4_D ||
+			 op == SPINOR_OP_READ4_1_4_4_D) {
+			/* read mode : 1-4-4, such as Spansion s25fl128s. */
+			writel(LUT0(CMD, PAD1, op)
+				| LUT1(ADDR_DDR, PAD4, addrlen),
+				base + QUADSPI_LUT(lut_base));
+
+			writel(LUT0(MODE_DDR, PAD1, 4) | LUT1(DUMMY, PAD1, dm),
+				base + QUADSPI_LUT(lut_base + 1));
+
+			writel(LUT0(READ_DDR, PAD4, rxfifo)
+				| LUT1(JMP_ON_CS, PAD1, 0),
+				base + QUADSPI_LUT(lut_base + 2));
+		} else {
+			dev_err(nor->dev,
+				"Unsupported cmd:%.2x\n", nor->read_opcode);
+		}
 	}
 
 	/* Write enable */
@@ -369,6 +390,9 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
 {
 	switch (cmd) {
+	case SPINOR_OP_READ_1_4_4_D:
+	case SPINOR_OP_READ4_1_4_4_D:
+	case SPINOR_OP_READ4_1_1_4:
 	case SPINOR_OP_READ_1_1_4:
 		return SEQID_QUAD_READ;
 	case SPINOR_OP_WREN:
@@ -558,6 +582,8 @@ static void fsl_qspi_set_map_addr(struct fsl_qspi *q)
 static void fsl_qspi_init_abh_read(struct fsl_qspi *q)
 {
 	void __iomem *base = q->iobase;
+	struct spi_nor *nor = &q->nor[0];
+	u32 reg, reg2;
 	int seqid;
 
 	/* AHB configuration for access buffer 0/1/2 .*/
@@ -572,9 +598,30 @@ static void fsl_qspi_init_abh_read(struct fsl_qspi *q)
 	writel(0, base + QUADSPI_BUF2IND);
 
 	/* Set the default lut sequence for AHB Read. */
-	seqid = fsl_qspi_get_seqid(q, q->nor[0].read_opcode);
+	seqid = fsl_qspi_get_seqid(q, nor->read_opcode);
 	writel(seqid << QUADSPI_BFGENCR_SEQID_SHIFT,
 		q->iobase + QUADSPI_BFGENCR);
+
+	/* enable the DDR quad read */
+	if (nor->flash_read == SPI_NOR_DDR_QUAD) {
+		reg = readl(q->iobase + QUADSPI_MCR);
+
+		/* Firstly, disable the module */
+		writel(reg | QUADSPI_MCR_MDIS_MASK, q->iobase + QUADSPI_MCR);
+
+		/* Set the Sampling Register for DDR */
+		reg2 = readl(q->iobase + QUADSPI_SMPR);
+		reg2 &= ~QUADSPI_SMPR_DDRSMP_MASK;
+		reg2 |= (2 << QUADSPI_SMPR_DDRSMP_SHIFT);
+		writel(reg2, q->iobase + QUADSPI_SMPR);
+
+		/* Enable the module again (enable the DDR too) */
+		reg |= QUADSPI_MCR_DDR_EN_MASK;
+		if (is_imx6sx_qspi(q))
+			reg |= MX6SX_QUADSPI_MCR_TX_DDR_DELAY_EN_MASK;
+
+		writel(reg, q->iobase + QUADSPI_MCR);
+	}
 }
 
 /* We use this function to do some basic init for spi_nor_scan(). */
@@ -863,6 +910,7 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 	/* iterate the subnodes. */
 	for_each_available_child_of_node(dev->of_node, np) {
 		const struct spi_device_id *id;
+		enum read_mode mode = SPI_NOR_QUAD;
 		char modalias[40];
 		u32 dummy = 0;
 
@@ -903,13 +951,16 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 		/* Set the dummy cycles for the DDR Quad Read */
 		ret = of_property_read_u32(np, "spi-nor,ddr-quad-read-dummy",
 				&dummy);
-		if (!ret && dummy > 0 && dummy < 8)
+		if (!ret && dummy > 0 && dummy < 8) {
 			nor->read_dummy = dummy;
+			mode = SPI_NOR_DDR_QUAD;
+			dev_dbg(dev, "The DDR quad read dummy is %d.\n", dummy);
+		}
 
 		/* set the chip address for READID */
 		fsl_qspi_set_base_addr(q, nor);
 
-		ret = spi_nor_scan(nor, id, SPI_NOR_QUAD);
+		ret = spi_nor_scan(nor, id, mode);
 		if (ret)
 			goto map_failed;
 
-- 
1.7.2.rc3

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

[PATCH] arm64: dts: rockchip: enable rng on all rk356x

[Peter Robinson]

The rk356x rng is available on both the rk3566 and rk3568
parts, the IP is all self contained within the SoCs so
it's enabled already by default on rk3568 so let's enable
it in the base rk356x.dtsi so it's enabled consistently
everywhere.

Signed-off-by: Peter Robinson <pbrobinson@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3568.dtsi      | 4 ----
 arch/arm64/boot/dts/rockchip/rk356x-base.dtsi | 1 -
 2 files changed, 5 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
index ecaefe208e3e..9dc09db5034d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
@@ -397,10 +397,6 @@ power-domain@RK3568_PD_PIPE {
 	};
 };
 
-&rng {
-	status = "okay";
-};
-
 &usb_host0_xhci {
 	phys = <&usb2phy0_otg>, <&combphy0 PHY_TYPE_USB3>;
 	phy-names = "usb2-phy", "usb3-phy";
diff --git a/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
index 62be06f3b863..2994cddb3464 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
@@ -1038,7 +1038,6 @@ rng: rng@fe388000 {
 		clocks = <&cru CLK_TRNG_NS>, <&cru HCLK_TRNG_NS>;
 		clock-names = "core", "ahb";
 		resets = <&cru SRST_TRNG_NS>;
-		status = "disabled";
 	};
 
 	i2s0_8ch: i2s@fe400000 {
-- 
2.47.1

Re: [PATCH] arm64: dts: rockchip: enable rng on all rk356x

[Dragan Simic]

Hello Peter,

On 2024-12-02 00:46, Peter Robinson wrote:
> The rk356x rng is available on both the rk3566 and rk3568
> parts, the IP is all self contained within the SoCs so
> it's enabled already by default on rk3568 so let's enable
> it in the base rk356x.dtsi so it's enabled consistently
> everywhere.

Please, go through the mailing list threads [1][2] that have led us
to the current state.  To sum it up, it isn't about what's supported
in the two RK356x SoC variants, but about the RK3566's HWRNG being
disabled because the testing showed that it produces unacceptably
low quality of random data, for some yet unknown reason.

[1] 
https://lore.kernel.org/linux-rockchip/cover.1720969799.git.daniel@makrotopia.org/T/#u
[2] 
https://lore.kernel.org/linux-rockchip/cover.1722355365.git.daniel@makrotopia.org/T/#u

> Signed-off-by: Peter Robinson <pbrobinson@gmail.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3568.dtsi      | 4 ----
>  arch/arm64/boot/dts/rockchip/rk356x-base.dtsi | 1 -
>  2 files changed, 5 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> index ecaefe208e3e..9dc09db5034d 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> @@ -397,10 +397,6 @@ power-domain@RK3568_PD_PIPE {
>  	};
>  };
> 
> -&rng {
> -	status = "okay";
> -};
> -
>  &usb_host0_xhci {
>  	phys = <&usb2phy0_otg>, <&combphy0 PHY_TYPE_USB3>;
>  	phy-names = "usb2-phy", "usb3-phy";
> diff --git a/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
> b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
> index 62be06f3b863..2994cddb3464 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
> @@ -1038,7 +1038,6 @@ rng: rng@fe388000 {
>  		clocks = <&cru CLK_TRNG_NS>, <&cru HCLK_TRNG_NS>;
>  		clock-names = "core", "ahb";
>  		resets = <&cru SRST_TRNG_NS>;
> -		status = "disabled";
>  	};
> 
>  	i2s0_8ch: i2s@fe400000 {

Re: [PATCH] arm64: dts: rockchip: enable rng on all rk356x

[Marcin Juszkiewicz]

W dniu 2.12.2024 o 04:55, Dragan Simic pisze:
 > On 2024-12-02 00:46, Peter Robinson wrote:
 >> The rk356x rng is available on both the rk3566 and rk3568
 >> parts, the IP is all self contained within the SoCs so
 >> it's enabled already by default on rk3568 so let's enable
 >> it in the base rk356x.dtsi so it's enabled consistently
 >> everywhere.
 >
 > Please, go through the mailing list threads [1][2] that have led us
 > to the current state.  To sum it up, it isn't about what's supported
 > in the two RK356x SoC variants, but about the RK3566's HWRNG being
 > disabled because the testing showed that it produces unacceptably
 > low quality of random data, for some yet unknown reason.

So maybe there should be a comment in rockchip/rk3568.dtsi so we would 
not get back to it again.

Re: [PATCH] arm64: dts: rockchip: enable rng on all rk356x

[Diederik de Haas]

[-- Attachment #1: Type: text/plain, Size: 1180 bytes --]

On Mon Dec 2, 2024 at 11:40 AM CET, Marcin Juszkiewicz wrote:
> W dniu 2.12.2024 o 04:55, Dragan Simic pisze:
>  > On 2024-12-02 00:46, Peter Robinson wrote:
>  >> The rk356x rng is available on both the rk3566 and rk3568
>  >> parts, the IP is all self contained within the SoCs so
>  >> it's enabled already by default on rk3568 so let's enable
>  >> it in the base rk356x.dtsi so it's enabled consistently
>  >> everywhere.
>  >
>  > Please, go through the mailing list threads [1][2] that have led us
>  > to the current state.  To sum it up, it isn't about what's supported
>  > in the two RK356x SoC variants, but about the RK3566's HWRNG being
>  > disabled because the testing showed that it produces unacceptably
>  > low quality of random data, for some yet unknown reason.
>
> So maybe there should be a comment in rockchip/rk3568.dtsi so we would 
> not get back to it again.

I suggested to put that in the (original) commit message:
https://lore.kernel.org/linux-rockchip/6690040.iosknibmi9@bagend/
precisely because this was quite predictable to happen.

So a +1 on a comment in the dtsi with a link to the discussion in the
commit message.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] arm64: dts: rockchip: enable rng on all rk356x

[Dragan Simic]

Hello Diederik and Marcin,

On 2024-12-02 13:01, Diederik de Haas wrote:
> On Mon Dec 2, 2024 at 11:40 AM CET, Marcin Juszkiewicz wrote:
>> W dniu 2.12.2024 o 04:55, Dragan Simic pisze:
>>  > On 2024-12-02 00:46, Peter Robinson wrote:
>>  >> The rk356x rng is available on both the rk3566 and rk3568
>>  >> parts, the IP is all self contained within the SoCs so
>>  >> it's enabled already by default on rk3568 so let's enable
>>  >> it in the base rk356x.dtsi so it's enabled consistently
>>  >> everywhere.
>>  >
>>  > Please, go through the mailing list threads [1][2] that have led us
>>  > to the current state.  To sum it up, it isn't about what's 
>> supported
>>  > in the two RK356x SoC variants, but about the RK3566's HWRNG being
>>  > disabled because the testing showed that it produces unacceptably
>>  > low quality of random data, for some yet unknown reason.
>> 
>> So maybe there should be a comment in rockchip/rk3568.dtsi so we would
>> not get back to it again.
> 
> I suggested to put that in the (original) commit message:
> https://lore.kernel.org/linux-rockchip/6690040.iosknibmi9@bagend/
> precisely because this was quite predictable to happen.
> 
> So a +1 on a comment in the dtsi with a link to the discussion in the
> commit message.

Totally agreed on adding a brief summary to the RK356x base SoC dtsi,
so I went ahead and sent a patch that adds it. [1]  I also expected
that to be described in the base dtsi back when the HWRNG support was
added, but it somehow went in without the description.

[1] 
https://lore.kernel.org/linux-rockchip/6b272e2f8f916c04b05db50df621659a5a7f29ab.1733149874.git.dsimic@manjaro.org/

Re: [PATCH] arm64: dts: rockchip: enable rng on all rk356x

[Heiko Stübner]

Am Montag, 2. Dezember 2024, 15:50:07 CET schrieb Dragan Simic:
> Hello Diederik and Marcin,
> 
> On 2024-12-02 13:01, Diederik de Haas wrote:
> > On Mon Dec 2, 2024 at 11:40 AM CET, Marcin Juszkiewicz wrote:
> >> W dniu 2.12.2024 o 04:55, Dragan Simic pisze:
> >>  > On 2024-12-02 00:46, Peter Robinson wrote:
> >>  >> The rk356x rng is available on both the rk3566 and rk3568
> >>  >> parts, the IP is all self contained within the SoCs so
> >>  >> it's enabled already by default on rk3568 so let's enable
> >>  >> it in the base rk356x.dtsi so it's enabled consistently
> >>  >> everywhere.
> >>  >
> >>  > Please, go through the mailing list threads [1][2] that have led us
> >>  > to the current state.  To sum it up, it isn't about what's 
> >> supported
> >>  > in the two RK356x SoC variants, but about the RK3566's HWRNG being
> >>  > disabled because the testing showed that it produces unacceptably
> >>  > low quality of random data, for some yet unknown reason.
> >> 
> >> So maybe there should be a comment in rockchip/rk3568.dtsi so we would
> >> not get back to it again.
> > 
> > I suggested to put that in the (original) commit message:
> > https://lore.kernel.org/linux-rockchip/6690040.iosknibmi9@bagend/
> > precisely because this was quite predictable to happen.
> > 
> > So a +1 on a comment in the dtsi with a link to the discussion in the
> > commit message.
> 
> Totally agreed on adding a brief summary to the RK356x base SoC dtsi,
> so I went ahead and sent a patch that adds it. [1]  I also expected
> that to be described in the base dtsi back when the HWRNG support was
> added, but it somehow went in without the description.

thanks a lot for doing that, so I'll drop this patch here now from my inbox.


Heiko