linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC v1 0/8] intel-nand-controller: Fixes, cleanups and questions
@ 2022-06-28 11:27 Martin Blumenstingl
  2022-06-28 11:27 ` [PATCH RFC v1 1/8] dt-bindings: mtd: intel: lgm-nand: Fix compatible string Martin Blumenstingl
                   ` (8 more replies)
  0 siblings, 9 replies; 11+ messages in thread
From: Martin Blumenstingl @ 2022-06-28 11:27 UTC (permalink / raw)
  To: linux-mtd, devicetree
  Cc: linux-kernel, tlanger, rtanwar, miquel.raynal, richard, vigneshr,
	Martin Blumenstingl

Hello,

I am trying to replace the xway_nand driver (which is still using the
legacy NAND API) with the intel-nand-controller driver. The Intel LGM
IP (for which intel-nand-controller was implemented) uses a newer
version of the EBU NAND and HSNAND IP found in Lantiq XWAY SoCs. The
most notable change is the addition of HSNAND Intel LGM SoCs (it's not
clear to me if/which Lantiq SoCs also have this DMA engine).

While testing my changes on a Lantiq xRX200 SoC I came across some
issues with the intel-nand-controller driver. The problems I found are:
1) Mismatch between dt-bindings and driver implementation (compatible
   string, patch #1 and patch #4) and hardware capabilities (number of
   CS lines, patch #1).
2) The driver reads the CS (chip select) line from the NAND controller's
   reg property. In the dt-bindings example this is 0xe0f00000. I don't
   understand how this can even work on Intel SoCs. My understanding is
   that it must be read from the NAND chip (child node).
3) A few smaller code cleanups to make the driver easier to understand
   (patches #5 to #8)
4) I tried to understand the timing parameter calculation code but found
   that it probably doesn't work on the Intel LGM SoCs either. The
   dt-bindings example use clock ID 125 which is LGM_GCLK_EBU. So far
   this is fine because EBU is the actual IP block for the NAND
   interface. However, drivers/clk/x86/clk-lgm.c defines this clock as
   a gate without a parent, so it's rate (as read by Linux) is always 0.
   The intel-nand-controller driver then tries to calculate:
     rate = clk_get_rate(ctrl->clk) / HZ_PER_MHZ
   (rate will be 0 because clk_get_rate() returns 0) and then:
     DIV_ROUND_UP(USEC_PER_SEC, rate)
   (this then tries to divide by zero)

Before this series is applied it would be great to have these questions
answered:
- My understanding is that the chip select line (reg property of the
  NAND controller's child node) refers to the chip select line of the
  controller. Let's say we have a controller with two CS lines. A NAND
  flash chip (which a single chip select line) is connected to the
  second CS line of the controller. Is my understanding correct that
  the NAND chip device-tree node should get reg = <1> in this case?
- Who from Maxlinear (who took over Intel's AnyWAN division, which
  previously worked on the drivers for the Intel LGM SoCs) can send a
  patch to correct the LGM_GCLK_EBU clock rate in
  drivers/clk/x86/clk-lgm.c? Or is LGM dead and the various drivers
  should be removed instead?
- Who from Maxlinear can provide insights into which clock is connected
  to the EBU NAND controller on Lantiq XWAY (Danube, xRX100, xRX200,
  xRX300) SoCs as well as newer GRX350/GRX550 SoCs so that I can make
  the intel-nand-controller work without hardcoded timing settings on
  the XWAY SoCs?

Due to the severity of issues 2) and 4) above I am targeting linux-next
with this series. In my opinion there's no point in backporting these
fixes to a driver which has been broken since it was upstreamed.


Best regards,
Martin


Martin Blumenstingl (8):
  dt-bindings: mtd: intel: lgm-nand: Fix compatible string
  dt-bindings: mtd: intel: lgm-nand: Fix maximum chip select value
  mtd: rawnand: intel: Read the chip-select line from the correct OF
    node
  mtd: rawnand: intel: Remove undocumented compatible string
  mtd: rawnand: intel: Don't re-define NAND_DATA_IFACE_CHECK_ONLY
  mtd: rawnand: intel: Remove unused nand_pa member from ebu_nand_cs
  mtd: rawnand: intel: Remove unused clk_rate member from struct
    ebu_nand
  mtd: rawnand: intel: Use devm_platform_ioremap_resource_byname()

 .../bindings/mtd/intel,lgm-nand.yaml          |  8 +++---
 drivers/mtd/nand/raw/intel-nand-controller.c  | 28 +++++++++----------
 2 files changed, 17 insertions(+), 19 deletions(-)

-- 
2.36.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH RFC v1 1/8] dt-bindings: mtd: intel: lgm-nand: Fix compatible string
  2022-06-28 11:27 [PATCH RFC v1 0/8] intel-nand-controller: Fixes, cleanups and questions Martin Blumenstingl
@ 2022-06-28 11:27 ` Martin Blumenstingl
  2022-06-28 13:15   ` Rob Herring
  2022-06-28 11:27 ` [PATCH RFC v1 2/8] dt-bindings: mtd: intel: lgm-nand: Fix maximum chip select value Martin Blumenstingl
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 11+ messages in thread
From: Martin Blumenstingl @ 2022-06-28 11:27 UTC (permalink / raw)
  To: linux-mtd, devicetree
  Cc: linux-kernel, tlanger, rtanwar, miquel.raynal, richard, vigneshr,
	Martin Blumenstingl

The driver which was added at the same time as the dt-bindings uses the
compatible string "intel,lgm-ebunand". Use the same compatible string
also in the dt-bindings.

Fixes: 2f9cea8eae44f5 ("dt-bindings: mtd: Add Nand Flash Controller support for Intel LGM SoC")
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml b/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
index 30e0c66ab0eb..763ee3e1faf3 100644
--- a/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
+++ b/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/mtd/intel,lgm-nand.yaml#
+$id: http://devicetree.org/schemas/mtd/intel,lgm-ebunand.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title: Intel LGM SoC NAND Controller Device Tree Bindings
@@ -14,7 +14,7 @@ maintainers:
 
 properties:
   compatible:
-    const: intel,lgm-nand
+    const: intel,lgm-ebunand
 
   reg:
     maxItems: 6
@@ -75,7 +75,7 @@ additionalProperties: false
 examples:
   - |
     nand-controller@e0f00000 {
-      compatible = "intel,lgm-nand";
+      compatible = "intel,lgm-ebunand";
       reg = <0xe0f00000 0x100>,
             <0xe1000000 0x300>,
             <0xe1400000 0x8000>,
-- 
2.36.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH RFC v1 2/8] dt-bindings: mtd: intel: lgm-nand: Fix maximum chip select value
  2022-06-28 11:27 [PATCH RFC v1 0/8] intel-nand-controller: Fixes, cleanups and questions Martin Blumenstingl
  2022-06-28 11:27 ` [PATCH RFC v1 1/8] dt-bindings: mtd: intel: lgm-nand: Fix compatible string Martin Blumenstingl
@ 2022-06-28 11:27 ` Martin Blumenstingl
  2022-06-28 11:27 ` [PATCH RFC v1 3/8] mtd: rawnand: intel: Read the chip-select line from the correct OF node Martin Blumenstingl
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Martin Blumenstingl @ 2022-06-28 11:27 UTC (permalink / raw)
  To: linux-mtd, devicetree
  Cc: linux-kernel, tlanger, rtanwar, miquel.raynal, richard, vigneshr,
	Martin Blumenstingl

The Intel LGM NAND IP only supports two chip selects: There's only two
CS and ADDR_SEL register sets. Fix the maximum allowed chip select value
according to the dt-bindings.

Fixes: 2f9cea8eae44f5 ("dt-bindings: mtd: Add Nand Flash Controller support for Intel LGM SoC")
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml b/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
index 763ee3e1faf3..04f26196c4c1 100644
--- a/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
+++ b/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
@@ -51,7 +51,7 @@ patternProperties:
     properties:
       reg:
         minimum: 0
-        maximum: 7
+        maximum: 1
 
       nand-ecc-mode: true
 
-- 
2.36.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH RFC v1 3/8] mtd: rawnand: intel: Read the chip-select line from the correct OF node
  2022-06-28 11:27 [PATCH RFC v1 0/8] intel-nand-controller: Fixes, cleanups and questions Martin Blumenstingl
  2022-06-28 11:27 ` [PATCH RFC v1 1/8] dt-bindings: mtd: intel: lgm-nand: Fix compatible string Martin Blumenstingl
  2022-06-28 11:27 ` [PATCH RFC v1 2/8] dt-bindings: mtd: intel: lgm-nand: Fix maximum chip select value Martin Blumenstingl
@ 2022-06-28 11:27 ` Martin Blumenstingl
  2022-06-28 11:27 ` [PATCH RFC v1 4/8] mtd: rawnand: intel: Remove undocumented compatible string Martin Blumenstingl
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Martin Blumenstingl @ 2022-06-28 11:27 UTC (permalink / raw)
  To: linux-mtd, devicetree
  Cc: linux-kernel, tlanger, rtanwar, miquel.raynal, richard, vigneshr,
	Martin Blumenstingl

The chip select has to be read from the flash node which is a child node
of the NAND controller.

Fixes: 0b1039f016e8a3 ("mtd: rawnand: Add NAND controller support on Intel LGM SoC")
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/mtd/nand/raw/intel-nand-controller.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/intel-nand-controller.c b/drivers/mtd/nand/raw/intel-nand-controller.c
index e91b879b32bd..3df3f32423f9 100644
--- a/drivers/mtd/nand/raw/intel-nand-controller.c
+++ b/drivers/mtd/nand/raw/intel-nand-controller.c
@@ -16,6 +16,7 @@
 #include <linux/mtd/rawnand.h>
 #include <linux/mtd/nand.h>
 
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
@@ -580,6 +581,7 @@ static int ebu_nand_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct ebu_nand_controller *ebu_host;
+	struct device_node *chip_np;
 	struct nand_chip *nand;
 	struct mtd_info *mtd;
 	struct resource *res;
@@ -604,7 +606,12 @@ static int ebu_nand_probe(struct platform_device *pdev)
 	if (IS_ERR(ebu_host->hsnand))
 		return PTR_ERR(ebu_host->hsnand);
 
-	ret = device_property_read_u32(dev, "reg", &cs);
+	chip_np = of_get_next_child(dev->of_node, NULL);
+	if (!chip_np)
+		return dev_err_probe(dev, -EINVAL,
+				     "Could not find child node for the NAND chip\n");
+
+	ret = of_property_read_u32(chip_np, "reg", &cs);
 	if (ret) {
 		dev_err(dev, "failed to get chip select: %d\n", ret);
 		return ret;
@@ -660,7 +667,7 @@ static int ebu_nand_probe(struct platform_device *pdev)
 	writel(ebu_host->cs[cs].addr_sel | EBU_ADDR_MASK(5) | EBU_ADDR_SEL_REGEN,
 	       ebu_host->ebu + EBU_ADDR_SEL(cs));
 
-	nand_set_flash_node(&ebu_host->chip, dev->of_node);
+	nand_set_flash_node(&ebu_host->chip, chip_np);
 
 	mtd = nand_to_mtd(&ebu_host->chip);
 	if (!mtd->name) {
-- 
2.36.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH RFC v1 4/8] mtd: rawnand: intel: Remove undocumented compatible string
  2022-06-28 11:27 [PATCH RFC v1 0/8] intel-nand-controller: Fixes, cleanups and questions Martin Blumenstingl
                   ` (2 preceding siblings ...)
  2022-06-28 11:27 ` [PATCH RFC v1 3/8] mtd: rawnand: intel: Read the chip-select line from the correct OF node Martin Blumenstingl
@ 2022-06-28 11:27 ` Martin Blumenstingl
  2022-06-28 11:27 ` [PATCH RFC v1 5/8] mtd: rawnand: intel: Don't re-define NAND_DATA_IFACE_CHECK_ONLY Martin Blumenstingl
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Martin Blumenstingl @ 2022-06-28 11:27 UTC (permalink / raw)
  To: linux-mtd, devicetree
  Cc: linux-kernel, tlanger, rtanwar, miquel.raynal, richard, vigneshr,
	Martin Blumenstingl

The "intel,nand-controller" compatible string is not part of the
dt-bindings. Remove it from the driver as it's not supposed to be used
without any documentation for it.

Fixes: 0b1039f016e8a3 ("mtd: rawnand: Add NAND controller support on Intel LGM SoC")
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/mtd/nand/raw/intel-nand-controller.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/intel-nand-controller.c b/drivers/mtd/nand/raw/intel-nand-controller.c
index 3df3f32423f9..056835fd4562 100644
--- a/drivers/mtd/nand/raw/intel-nand-controller.c
+++ b/drivers/mtd/nand/raw/intel-nand-controller.c
@@ -723,7 +723,6 @@ static int ebu_nand_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id ebu_nand_match[] = {
-	{ .compatible = "intel,nand-controller" },
 	{ .compatible = "intel,lgm-ebunand" },
 	{}
 };
-- 
2.36.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH RFC v1 5/8] mtd: rawnand: intel: Don't re-define NAND_DATA_IFACE_CHECK_ONLY
  2022-06-28 11:27 [PATCH RFC v1 0/8] intel-nand-controller: Fixes, cleanups and questions Martin Blumenstingl
                   ` (3 preceding siblings ...)
  2022-06-28 11:27 ` [PATCH RFC v1 4/8] mtd: rawnand: intel: Remove undocumented compatible string Martin Blumenstingl
@ 2022-06-28 11:27 ` Martin Blumenstingl
  2022-06-28 11:27 ` [PATCH RFC v1 6/8] mtd: rawnand: intel: Remove unused nand_pa member from ebu_nand_cs Martin Blumenstingl
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Martin Blumenstingl @ 2022-06-28 11:27 UTC (permalink / raw)
  To: linux-mtd, devicetree
  Cc: linux-kernel, tlanger, rtanwar, miquel.raynal, richard, vigneshr,
	Martin Blumenstingl

NAND_DATA_IFACE_CHECK_ONLY is already defined in
include/linux/mtd/rawnand.h which is also included by the driver. Drop
the re-definition from the intel-nand-controller driver.

Fixes: 0b1039f016e8a3 ("mtd: rawnand: Add NAND controller support on Intel LGM SoC")
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/mtd/nand/raw/intel-nand-controller.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/intel-nand-controller.c b/drivers/mtd/nand/raw/intel-nand-controller.c
index 056835fd4562..3df16d5ecae8 100644
--- a/drivers/mtd/nand/raw/intel-nand-controller.c
+++ b/drivers/mtd/nand/raw/intel-nand-controller.c
@@ -100,8 +100,6 @@
 
 #define HSNAND_ECC_OFFSET	0x008
 
-#define NAND_DATA_IFACE_CHECK_ONLY	-1
-
 #define MAX_CS	2
 
 #define USEC_PER_SEC	1000000L
-- 
2.36.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH RFC v1 6/8] mtd: rawnand: intel: Remove unused nand_pa member from ebu_nand_cs
  2022-06-28 11:27 [PATCH RFC v1 0/8] intel-nand-controller: Fixes, cleanups and questions Martin Blumenstingl
                   ` (4 preceding siblings ...)
  2022-06-28 11:27 ` [PATCH RFC v1 5/8] mtd: rawnand: intel: Don't re-define NAND_DATA_IFACE_CHECK_ONLY Martin Blumenstingl
@ 2022-06-28 11:27 ` Martin Blumenstingl
  2022-06-28 11:27 ` [PATCH RFC v1 7/8] mtd: rawnand: intel: Remove unused clk_rate member from struct ebu_nand Martin Blumenstingl
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Martin Blumenstingl @ 2022-06-28 11:27 UTC (permalink / raw)
  To: linux-mtd, devicetree
  Cc: linux-kernel, tlanger, rtanwar, miquel.raynal, richard, vigneshr,
	Martin Blumenstingl

The nand_pa member from struct ebu_nand_cs is only written but never
read. Remove this unused and unneeded member.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/mtd/nand/raw/intel-nand-controller.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/intel-nand-controller.c b/drivers/mtd/nand/raw/intel-nand-controller.c
index 3df16d5ecae8..de4f85368988 100644
--- a/drivers/mtd/nand/raw/intel-nand-controller.c
+++ b/drivers/mtd/nand/raw/intel-nand-controller.c
@@ -106,7 +106,6 @@
 
 struct ebu_nand_cs {
 	void __iomem *chipaddr;
-	dma_addr_t nand_pa;
 	u32 addr_sel;
 };
 
@@ -626,7 +625,6 @@ static int ebu_nand_probe(struct platform_device *pdev)
 	ebu_host->cs[cs].chipaddr = devm_ioremap_resource(dev, res);
 	if (IS_ERR(ebu_host->cs[cs].chipaddr))
 		return PTR_ERR(ebu_host->cs[cs].chipaddr);
-	ebu_host->cs[cs].nand_pa = res->start;
 
 	ebu_host->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(ebu_host->clk))
-- 
2.36.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH RFC v1 7/8] mtd: rawnand: intel: Remove unused clk_rate member from struct ebu_nand
  2022-06-28 11:27 [PATCH RFC v1 0/8] intel-nand-controller: Fixes, cleanups and questions Martin Blumenstingl
                   ` (5 preceding siblings ...)
  2022-06-28 11:27 ` [PATCH RFC v1 6/8] mtd: rawnand: intel: Remove unused nand_pa member from ebu_nand_cs Martin Blumenstingl
@ 2022-06-28 11:27 ` Martin Blumenstingl
  2022-06-28 11:27 ` [PATCH RFC v1 8/8] mtd: rawnand: intel: Use devm_platform_ioremap_resource_byname() Martin Blumenstingl
  2022-06-28 14:38 ` [PATCH RFC v1 0/8] intel-nand-controller: Fixes, cleanups and questions Miquel Raynal
  8 siblings, 0 replies; 11+ messages in thread
From: Martin Blumenstingl @ 2022-06-28 11:27 UTC (permalink / raw)
  To: linux-mtd, devicetree
  Cc: linux-kernel, tlanger, rtanwar, miquel.raynal, richard, vigneshr,
	Martin Blumenstingl

The clk_rate member from struct ebu_nand is only written but never read.
Remove this unused and unneeded member.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/mtd/nand/raw/intel-nand-controller.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/intel-nand-controller.c b/drivers/mtd/nand/raw/intel-nand-controller.c
index de4f85368988..e486db11ecc3 100644
--- a/drivers/mtd/nand/raw/intel-nand-controller.c
+++ b/drivers/mtd/nand/raw/intel-nand-controller.c
@@ -118,7 +118,6 @@ struct ebu_nand_controller {
 	struct dma_chan *dma_tx;
 	struct dma_chan *dma_rx;
 	struct completion dma_access_complete;
-	unsigned long clk_rate;
 	struct clk *clk;
 	u32 nd_para0;
 	u8 cs_num;
@@ -636,7 +635,6 @@ static int ebu_nand_probe(struct platform_device *pdev)
 		dev_err(dev, "failed to enable clock: %d\n", ret);
 		return ret;
 	}
-	ebu_host->clk_rate = clk_get_rate(ebu_host->clk);
 
 	ebu_host->dma_tx = dma_request_chan(dev, "tx");
 	if (IS_ERR(ebu_host->dma_tx)) {
-- 
2.36.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH RFC v1 8/8] mtd: rawnand: intel: Use devm_platform_ioremap_resource_byname()
  2022-06-28 11:27 [PATCH RFC v1 0/8] intel-nand-controller: Fixes, cleanups and questions Martin Blumenstingl
                   ` (6 preceding siblings ...)
  2022-06-28 11:27 ` [PATCH RFC v1 7/8] mtd: rawnand: intel: Remove unused clk_rate member from struct ebu_nand Martin Blumenstingl
@ 2022-06-28 11:27 ` Martin Blumenstingl
  2022-06-28 14:38 ` [PATCH RFC v1 0/8] intel-nand-controller: Fixes, cleanups and questions Miquel Raynal
  8 siblings, 0 replies; 11+ messages in thread
From: Martin Blumenstingl @ 2022-06-28 11:27 UTC (permalink / raw)
  To: linux-mtd, devicetree
  Cc: linux-kernel, tlanger, rtanwar, miquel.raynal, richard, vigneshr,
	Martin Blumenstingl

Switch from open-coded platform_get_resource_byname() and
devm_ioremap_resource() to devm_platform_ioremap_resource_byname() where
possible to simplify the code.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/mtd/nand/raw/intel-nand-controller.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/raw/intel-nand-controller.c b/drivers/mtd/nand/raw/intel-nand-controller.c
index e486db11ecc3..d4a0987e93ac 100644
--- a/drivers/mtd/nand/raw/intel-nand-controller.c
+++ b/drivers/mtd/nand/raw/intel-nand-controller.c
@@ -592,13 +592,11 @@ static int ebu_nand_probe(struct platform_device *pdev)
 	ebu_host->dev = dev;
 	nand_controller_init(&ebu_host->controller);
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ebunand");
-	ebu_host->ebu = devm_ioremap_resource(&pdev->dev, res);
+	ebu_host->ebu = devm_platform_ioremap_resource_byname(pdev, "ebunand");
 	if (IS_ERR(ebu_host->ebu))
 		return PTR_ERR(ebu_host->ebu);
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hsnand");
-	ebu_host->hsnand = devm_ioremap_resource(&pdev->dev, res);
+	ebu_host->hsnand = devm_platform_ioremap_resource_byname(pdev, "hsnand");
 	if (IS_ERR(ebu_host->hsnand))
 		return PTR_ERR(ebu_host->hsnand);
 
@@ -620,8 +618,8 @@ static int ebu_nand_probe(struct platform_device *pdev)
 	ebu_host->cs_num = cs;
 
 	resname = devm_kasprintf(dev, GFP_KERNEL, "nand_cs%d", cs);
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, resname);
-	ebu_host->cs[cs].chipaddr = devm_ioremap_resource(dev, res);
+	ebu_host->cs[cs].chipaddr = devm_platform_ioremap_resource_byname(pdev,
+									  resname);
 	if (IS_ERR(ebu_host->cs[cs].chipaddr))
 		return PTR_ERR(ebu_host->cs[cs].chipaddr);
 
-- 
2.36.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH RFC v1 1/8] dt-bindings: mtd: intel: lgm-nand: Fix compatible string
  2022-06-28 11:27 ` [PATCH RFC v1 1/8] dt-bindings: mtd: intel: lgm-nand: Fix compatible string Martin Blumenstingl
@ 2022-06-28 13:15   ` Rob Herring
  0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2022-06-28 13:15 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: miquel.raynal, richard, tlanger, devicetree, linux-mtd, vigneshr,
	linux-kernel, rtanwar

On Tue, 28 Jun 2022 13:27:24 +0200, Martin Blumenstingl wrote:
> The driver which was added at the same time as the dt-bindings uses the
> compatible string "intel,lgm-ebunand". Use the same compatible string
> also in the dt-bindings.
> 
> Fixes: 2f9cea8eae44f5 ("dt-bindings: mtd: Add Nand Flash Controller support for Intel LGM SoC")
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml: $id: relative path/filename doesn't match actual path or filename
	expected: http://devicetree.org/schemas/mtd/intel,lgm-nand.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH RFC v1 0/8] intel-nand-controller: Fixes, cleanups and questions
  2022-06-28 11:27 [PATCH RFC v1 0/8] intel-nand-controller: Fixes, cleanups and questions Martin Blumenstingl
                   ` (7 preceding siblings ...)
  2022-06-28 11:27 ` [PATCH RFC v1 8/8] mtd: rawnand: intel: Use devm_platform_ioremap_resource_byname() Martin Blumenstingl
@ 2022-06-28 14:38 ` Miquel Raynal
  8 siblings, 0 replies; 11+ messages in thread
From: Miquel Raynal @ 2022-06-28 14:38 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-mtd, devicetree, linux-kernel, tlanger, rtanwar, richard, vigneshr

Hi Martin,

martin.blumenstingl@googlemail.com wrote on Tue, 28 Jun 2022 13:27:23
+0200:

> Hello,
> 
> I am trying to replace the xway_nand driver (which is still using the
> legacy NAND API) with the intel-nand-controller driver. The Intel LGM
> IP (for which intel-nand-controller was implemented) uses a newer
> version of the EBU NAND and HSNAND IP found in Lantiq XWAY SoCs. The
> most notable change is the addition of HSNAND Intel LGM SoCs (it's not
> clear to me if/which Lantiq SoCs also have this DMA engine).
> 
> While testing my changes on a Lantiq xRX200 SoC I came across some
> issues with the intel-nand-controller driver. The problems I found are:
> 1) Mismatch between dt-bindings and driver implementation (compatible
>    string, patch #1 and patch #4) and hardware capabilities (number of
>    CS lines, patch #1).
> 2) The driver reads the CS (chip select) line from the NAND controller's
>    reg property. In the dt-bindings example this is 0xe0f00000. I don't
>    understand how this can even work on Intel SoCs. My understanding is
>    that it must be read from the NAND chip (child node).

Yes

> 3) A few smaller code cleanups to make the driver easier to understand
>    (patches #5 to #8)
> 4) I tried to understand the timing parameter calculation code but found
>    that it probably doesn't work on the Intel LGM SoCs either. The
>    dt-bindings example use clock ID 125 which is LGM_GCLK_EBU. So far
>    this is fine because EBU is the actual IP block for the NAND
>    interface. However, drivers/clk/x86/clk-lgm.c defines this clock as
>    a gate without a parent, so it's rate (as read by Linux) is always 0.
>    The intel-nand-controller driver then tries to calculate:
>      rate = clk_get_rate(ctrl->clk) / HZ_PER_MHZ
>    (rate will be 0 because clk_get_rate() returns 0) and then:
>      DIV_ROUND_UP(USEC_PER_SEC, rate)
>    (this then tries to divide by zero)
> 
> Before this series is applied it would be great to have these questions
> answered:
> - My understanding is that the chip select line (reg property of the
>   NAND controller's child node) refers to the chip select line of the
>   controller. Let's say we have a controller with two CS lines. A NAND
>   flash chip (which a single chip select line) is connected to the
>   second CS line of the controller. Is my understanding correct that
>   the NAND chip device-tree node should get reg = <1> in this case?

Yes

> - Who from Maxlinear (who took over Intel's AnyWAN division, which
>   previously worked on the drivers for the Intel LGM SoCs) can send a
>   patch to correct the LGM_GCLK_EBU clock rate in
>   drivers/clk/x86/clk-lgm.c? Or is LGM dead and the various drivers
>   should be removed instead?
> - Who from Maxlinear can provide insights into which clock is connected
>   to the EBU NAND controller on Lantiq XWAY (Danube, xRX100, xRX200,
>   xRX300) SoCs as well as newer GRX350/GRX550 SoCs so that I can make
>   the intel-nand-controller work without hardcoded timing settings on
>   the XWAY SoCs?
> 
> Due to the severity of issues 2) and 4) above I am targeting linux-next
> with this series. In my opinion there's no point in backporting these
> fixes to a driver which has been broken since it was upstreamed.

The changes look good to me, please resend with the bindings file name
fixed and we should be good.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

end of thread, other threads:[~2022-06-28 14:39 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-28 11:27 [PATCH RFC v1 0/8] intel-nand-controller: Fixes, cleanups and questions Martin Blumenstingl
2022-06-28 11:27 ` [PATCH RFC v1 1/8] dt-bindings: mtd: intel: lgm-nand: Fix compatible string Martin Blumenstingl
2022-06-28 13:15   ` Rob Herring
2022-06-28 11:27 ` [PATCH RFC v1 2/8] dt-bindings: mtd: intel: lgm-nand: Fix maximum chip select value Martin Blumenstingl
2022-06-28 11:27 ` [PATCH RFC v1 3/8] mtd: rawnand: intel: Read the chip-select line from the correct OF node Martin Blumenstingl
2022-06-28 11:27 ` [PATCH RFC v1 4/8] mtd: rawnand: intel: Remove undocumented compatible string Martin Blumenstingl
2022-06-28 11:27 ` [PATCH RFC v1 5/8] mtd: rawnand: intel: Don't re-define NAND_DATA_IFACE_CHECK_ONLY Martin Blumenstingl
2022-06-28 11:27 ` [PATCH RFC v1 6/8] mtd: rawnand: intel: Remove unused nand_pa member from ebu_nand_cs Martin Blumenstingl
2022-06-28 11:27 ` [PATCH RFC v1 7/8] mtd: rawnand: intel: Remove unused clk_rate member from struct ebu_nand Martin Blumenstingl
2022-06-28 11:27 ` [PATCH RFC v1 8/8] mtd: rawnand: intel: Use devm_platform_ioremap_resource_byname() Martin Blumenstingl
2022-06-28 14:38 ` [PATCH RFC v1 0/8] intel-nand-controller: Fixes, cleanups and questions Miquel Raynal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).