Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1530723 > unrolled thread

[PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP patch bomb

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP patch bomb Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    [PATCH 09/39] mtd: nand: denali: fix erased page check code Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
      Re: [PATCH 09/39] mtd: nand: denali: fix erased page check code Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-27 16:30 +0100
        Re: [PATCH 09/39] mtd: nand: denali: fix erased page check code Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-12-02 05:40 +0100
          Re: [PATCH 09/39] mtd: nand: denali: fix erased page check code Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-12-02 09:00 +0100
    [PATCH 22/39] mtd: nand: denali_dt: remove dma-mask DT property Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
      Re: [PATCH 22/39] mtd: nand: denali_dt: remove dma-mask DT property Rob Herring <robh@kernel.org> - 2016-12-01 17:00 +0100
    [PATCH 29/39] mtd: nand: denali: refactor multi NAND fixup code in more generic way Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    [PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to a helper function Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
      Re: [PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to  a helper function Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-27 17:30 +0100
        Re: [PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to a  helper function Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-30 07:10 +0100
          Re: [PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to  a helper function Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-30 09:10 +0100
    [PATCH 15/39] mtd: nand: denali: improve readability of handle_ecc() Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
      Re: [PATCH 15/39] mtd: nand: denali: improve readability of  handle_ecc() Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-27 16:40 +0100
      Re: [PATCH 15/39] mtd: nand: denali: improve readability of  handle_ecc() Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-27 16:50 +0100
        Re: [PATCH 15/39] mtd: nand: denali: improve readability of handle_ecc() Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-12-02 05:30 +0100
          Re: [PATCH 15/39] mtd: nand: denali: improve readability of  handle_ecc() Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-12-02 09:00 +0100
    [PATCH 18/39] mtd: nand: denali: move denali_read_page_raw() above denali_read_page() Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
      Re: [PATCH 18/39] mtd: nand: denali: move denali_read_page_raw()  above denali_read_page() Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-27 17:20 +0100
        Re: [PATCH 18/39] mtd: nand: denali: move denali_read_page_raw()  above denali_read_page() Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-30 07:20 +0100
    [PATCH 37/39] mtd: nand: denali: support "nand-ecc-strength" DT property Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
      Re: [PATCH 37/39] mtd: nand: denali: support "nand-ecc-strength" DT  property Rob Herring <robh@kernel.org> - 2016-12-01 17:20 +0100
    [PATCH 23/39] mtd: nand: denali_dt: use pdev instead of ofdev for platform_device Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    [PATCH 36/39] mtd: nand: denali: allow to use SoC-specific ECC strength Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    [PATCH 02/39] mtd: nand: denali: remove unused CONFIG option and macros Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    [PATCH 30/39] mtd: nand: denali: set DEVICES_CONNECTED 1 if not set Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    [PATCH 27/39] mtd: nand: denali: do not set mtd->name Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    [PATCH 35/39] mtd: nand: denali: calculate ecc.strength and ecc.bytes generically Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
      Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Rob Herring <robh@kernel.org> - 2016-12-01 17:10 +0100
        Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-12-02 04:00 +0100
          Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Rob Herring <robh@kernel.org> - 2016-12-02 17:30 +0100
            Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-12-03 03:50 +0100
              Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Marek Vasut <marek.vasut@gmail.com> - 2016-12-03 04:00 +0100
                Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Dinh Nguyen <dinh.linux@gmail.com> - 2016-12-03 23:10 +0100
                  Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-12-05 04:40 +0100
                    Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Marek Vasut <marek.vasut@gmail.com> - 2016-12-05 04:50 +0100
                      Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-12-05 05:20 +0100
                        Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Marek Vasut <marek.vasut@gmail.com> - 2016-12-05 05:30 +0100
                          Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Dinh Nguyen <dinh.linux@gmail.com> - 2016-12-05 22:00 +0100
                            Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Marek Vasut <marek.vasut@gmail.com> - 2016-12-05 22:50 +0100
                              Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for  UniPhier SoC variants Dinh Nguyen <dinguyen@kernel.org> - 2016-12-05 23:40 +0100
    [PATCH 12/39] mtd: nand: denali: return 0 for uncorrectable ECC error Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    [PATCH 31/39] mtd: nand: denali: remove meaningless writes to read-only registers Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    [PATCH 32/39] mtd: nand: denali: remove unnecessary writes to ECC_CORRECTION Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    [PATCH 01/39] mtd: nand: allow to set only one of ECC size and ECC strength from DT Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
      Re: [PATCH 01/39] mtd: nand: allow to set only one of ECC size and  ECC strength from DT Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-27 15:00 +0100
    [PATCH 06/39] mtd: nand: denali: fix write_oob_data() function Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    [PATCH 03/39] mtd: nand: denali: remove redundant define of BANK(x) Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    [PATCH 05/39] mtd: nand: denali: fix comment of denali_nand_info::flash_mem Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    [PATCH 14/39] mtd: nand: denali: replace uint{8/16/32}_t with u{8/16/32} Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    [PATCH 24/39] mtd: nand: denali: add NEW_N_BANKS_FORMAT capability Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    [PATCH 26/39] mtd: nand: denali: call nand_set_flash_node() to set DT node Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    [PATCH 33/39] mtd: nand: denali: support 1024 byte ECC step size Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
      Re: [PATCH 33/39] mtd: nand: denali: support 1024 byte ECC step size Rob Herring <robh@kernel.org> - 2016-12-01 17:00 +0100
    [PATCH 10/39] mtd: nand: denali: remove redundant if conditional of erased_check Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
    [PATCH 38/39] mtd: nand: denali: remove Toshiba, Hynix specific fixup code Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-26 19:20 +0100
      Re: [PATCH 38/39] mtd: nand: denali: remove Toshiba, Hynix specific  fixup code Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-27 17:30 +0100
    Re: [PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP  patch bomb Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-27 16:10 +0100
      Re: [PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP  patch bomb Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-30 09:10 +0100
        Re: [PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP  patch bomb Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-30 09:20 +0100
          Re: [PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP  patch bomb Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-12-01 10:20 +0100
      Re: [PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP  patch bomb Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-30 09:20 +0100
    Re: [PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP  patch bomb Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-11-27 17:40 +0100

Page 2 of 4 — ← Prev page 1 [2] 3 4  Next page →

#1530731 — [PATCH 37/39] mtd: nand: denali: support "nand-ecc-strength" DT property

Historically, this driver tried to choose as big ECC strength as
possible, but it would be reasonable to allow DT to set a particular
ECC strength with "nand-ecc-strength" property.

Going forward, DT platforms should specify "nand-ecc-strength" or
"nand-ecc-maximize" to show the ECC strength strategy explicitly.

If nothing is specified in DT, "nand-ecc-maximize" is implied since
this was the original behavior.  It applies to PCI platforms too.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 .../devicetree/bindings/mtd/denali-nand.txt        |  5 ++++
 drivers/mtd/nand/denali.c                          | 27 +++++++++++++++++++++-
 drivers/mtd/nand/denali_pci.c                      |  2 ++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mtd/denali-nand.txt b/Documentation/devicetree/bindings/mtd/denali-nand.txt
index e9d5818..51fe195 100644
--- a/Documentation/devicetree/bindings/mtd/denali-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/denali-nand.txt
@@ -9,6 +9,11 @@ Required properties:
 Optional properties:
   - nand-ecc-step-size: must be 512 or 1024.  If not specified, default to 512.
     see nand.txt for details.
+  - nand-ecc-strength: see nand.txt for details
+  - nand-ecc-maximize: see nand.txt for details
+
+Note:
+Either nand-ecc-strength or nand-ecc-maximize should be specified.
 
 The device tree may optionally contain sub-nodes describing partitions of the
 address space. See partition.txt for more detail.
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 54c9e0c..df174ca 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1393,6 +1393,21 @@ static int denali_set_max_ecc_strength(struct denali_nand_info *denali)
 	return -EINVAL;
 }
 
+static int denali_check_ecc_strength(struct denali_nand_info *denali)
+{
+	const int *ecc_strength = denali->ecc_strength_avail;
+
+	for (; *ecc_strength; ecc_strength++) {
+		if (*ecc_strength == denali->nand.ecc.strength)
+			return 0;
+	}
+
+	dev_err(denali->dev,
+		"Specified ECC strength is not supported for this controller.\n");
+
+	return -EINVAL;
+}
+
 static int denali_ooblayout_ecc(struct mtd_info *mtd, int section,
 				struct mtd_oob_region *oobregion)
 {
@@ -1628,7 +1643,17 @@ int denali_init(struct denali_nand_info *denali)
 	if (!denali->ecc_strength_avail)
 		denali->ecc_strength_avail = denali_default_ecc_strength;
 
-	ret = denali_set_max_ecc_strength(denali);
+	if (!chip->ecc.strength && !(chip->ecc.options & NAND_ECC_MAXIMIZE)) {
+		dev_info(denali->dev,
+			"No ECC strength is specified.  Trying max ECC strength strategy\n");
+		chip->ecc.options |= NAND_ECC_MAXIMIZE;
+	}
+
+	if (chip->ecc.options & NAND_ECC_MAXIMIZE)
+		ret = denali_set_max_ecc_strength(denali);
+	else
+		ret = denali_check_ecc_strength(denali);
+
 	if (ret)
 		goto failed_req_irq;
 
diff --git a/drivers/mtd/nand/denali_pci.c b/drivers/mtd/nand/denali_pci.c
index ac84323..0064f3e 100644
--- a/drivers/mtd/nand/denali_pci.c
+++ b/drivers/mtd/nand/denali_pci.c
@@ -85,6 +85,8 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		goto failed_remap_reg;
 	}
 
+	denali->nand.ecc.options |= NAND_ECC_MAXIMIZE;
+
 	ret = denali_init(denali);
 	if (ret)
 		goto failed_remap_mem;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1534249 — Re: [PATCH 37/39] mtd: nand: denali: support "nand-ecc-strength" DT property

On Sun, Nov 27, 2016 at 03:06:23AM +0900, Masahiro Yamada wrote:
> Historically, this driver tried to choose as big ECC strength as
> possible, but it would be reasonable to allow DT to set a particular
> ECC strength with "nand-ecc-strength" property.
> 
> Going forward, DT platforms should specify "nand-ecc-strength" or
> "nand-ecc-maximize" to show the ECC strength strategy explicitly.
> 
> If nothing is specified in DT, "nand-ecc-maximize" is implied since
> this was the original behavior.  It applies to PCI platforms too.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  .../devicetree/bindings/mtd/denali-nand.txt        |  5 ++++

I'd prefer all the DT changes be in 1 patch, but

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/mtd/nand/denali.c                          | 27 +++++++++++++++++++++-
>  drivers/mtd/nand/denali_pci.c                      |  2 ++
>  3 files changed, 33 insertions(+), 1 deletion(-)

[toc] | [prev] | [next] | [standalone]

#1530733 — [PATCH 23/39] mtd: nand: denali_dt: use pdev instead of ofdev for platform_device

"pdev" is much more often used for platform_device, so this will
help the driver code look consistent across the kernel.

While we are here, fix "line over 80 characters" coding style
violations.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mtd/nand/denali_dt.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/mtd/nand/denali_dt.c b/drivers/mtd/nand/denali_dt.c
index 6a486a7..f085626 100644
--- a/drivers/mtd/nand/denali_dt.c
+++ b/drivers/mtd/nand/denali_dt.c
@@ -40,7 +40,7 @@ static const struct of_device_id denali_nand_dt_ids[] = {
 
 MODULE_DEVICE_TABLE(of, denali_nand_dt_ids);
 
-static int denali_dt_probe(struct platform_device *ofdev)
+static int denali_dt_probe(struct platform_device *pdev)
 {
 	struct resource *denali_reg, *nand_data;
 	struct denali_dt *dt;
@@ -48,38 +48,40 @@ static int denali_dt_probe(struct platform_device *ofdev)
 	struct denali_nand_info *denali;
 	int ret;
 
-	dt = devm_kzalloc(&ofdev->dev, sizeof(*dt), GFP_KERNEL);
+	dt = devm_kzalloc(&pdev->dev, sizeof(*dt), GFP_KERNEL);
 	if (!dt)
 		return -ENOMEM;
 	denali = &dt->denali;
 
-	data = of_device_get_match_data(&ofdev->dev);
+	data = of_device_get_match_data(&pdev->dev);
 	if (data)
 		denali->caps = data->caps;
 
 	denali->caps |= DENALI_CAPS_HW_ECC_FIXUP;
 
 	denali->platform = DT;
-	denali->dev = &ofdev->dev;
-	denali->irq = platform_get_irq(ofdev, 0);
+	denali->dev = &pdev->dev;
+	denali->irq = platform_get_irq(pdev, 0);
 	if (denali->irq < 0) {
-		dev_err(&ofdev->dev, "no irq defined\n");
+		dev_err(&pdev->dev, "no irq defined\n");
 		return denali->irq;
 	}
 
-	denali_reg = platform_get_resource_byname(ofdev, IORESOURCE_MEM, "denali_reg");
-	denali->flash_reg = devm_ioremap_resource(&ofdev->dev, denali_reg);
+	denali_reg = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+						  "denali_reg");
+	denali->flash_reg = devm_ioremap_resource(&pdev->dev, denali_reg);
 	if (IS_ERR(denali->flash_reg))
 		return PTR_ERR(denali->flash_reg);
 
-	nand_data = platform_get_resource_byname(ofdev, IORESOURCE_MEM, "nand_data");
-	denali->flash_mem = devm_ioremap_resource(&ofdev->dev, nand_data);
+	nand_data = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+						 "nand_data");
+	denali->flash_mem = devm_ioremap_resource(&pdev->dev, nand_data);
 	if (IS_ERR(denali->flash_mem))
 		return PTR_ERR(denali->flash_mem);
 
-	dt->clk = devm_clk_get(&ofdev->dev, NULL);
+	dt->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(dt->clk)) {
-		dev_err(&ofdev->dev, "no clk available\n");
+		dev_err(&pdev->dev, "no clk available\n");
 		return PTR_ERR(dt->clk);
 	}
 	clk_prepare_enable(dt->clk);
@@ -88,7 +90,7 @@ static int denali_dt_probe(struct platform_device *ofdev)
 	if (ret)
 		goto out_disable_clk;
 
-	platform_set_drvdata(ofdev, dt);
+	platform_set_drvdata(pdev, dt);
 	return 0;
 
 out_disable_clk:
@@ -97,9 +99,9 @@ static int denali_dt_probe(struct platform_device *ofdev)
 	return ret;
 }
 
-static int denali_dt_remove(struct platform_device *ofdev)
+static int denali_dt_remove(struct platform_device *pdev)
 {
-	struct denali_dt *dt = platform_get_drvdata(ofdev);
+	struct denali_dt *dt = platform_get_drvdata(pdev);
 
 	denali_remove(&dt->denali);
 	clk_disable_unprepare(dt->clk);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1530734 — [PATCH 36/39] mtd: nand: denali: allow to use SoC-specific ECC strength

The ecc.strength of this IP is a platform-dependent parameter.

I chose {15, 8} as the default to respect the historical reason of
this driver, but it should be overridable to use this driver for
other SoCs.

If necessary, SoCs can provide their own ecc_strength_avail.
This must be an array of supported ecc.strength in descending order,
terminated by zero.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mtd/nand/denali.c    | 5 ++++-
 drivers/mtd/nand/denali.h    | 1 +
 drivers/mtd/nand/denali_dt.c | 5 ++++-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index f2ed3f8..54c9e0c 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1373,7 +1373,7 @@ static int denali_set_max_ecc_strength(struct denali_nand_info *denali)
 	int oobsize = mtd->oobsize;
 	int ecc_size = chip->ecc.size;
 	int ecc_steps = mtd->writesize / chip->ecc.size;
-	const int *ecc_strength = denali_default_ecc_strength;
+	const int *ecc_strength = denali->ecc_strength_avail;
 	int ecc_bytes;
 
 	/* carve out the BBM area */
@@ -1625,6 +1625,9 @@ int denali_init(struct denali_nand_info *denali)
 		chip->ecc.size = denali->caps & DENALI_CAPS_ECC_SIZE_1024 ?
 								1024 : 512;
 
+	if (!denali->ecc_strength_avail)
+		denali->ecc_strength_avail = denali_default_ecc_strength;
+
 	ret = denali_set_max_ecc_strength(denali);
 	if (ret)
 		goto failed_req_irq;
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index 5209625..2892e46 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -428,6 +428,7 @@ struct denali_nand_info {
 	u32 devnum;	/* represent how many nands connected */
 	u32 bbtskipbytes;
 	u32 max_banks;
+	const int *ecc_strength_avail;
 	unsigned int caps;
 #define DENALI_CAPS_HW_ECC_FIXUP		BIT(0)
 #define DENALI_CAPS_DMA_64BIT			BIT(1)
diff --git a/drivers/mtd/nand/denali_dt.c b/drivers/mtd/nand/denali_dt.c
index f085626..aa1e032 100644
--- a/drivers/mtd/nand/denali_dt.c
+++ b/drivers/mtd/nand/denali_dt.c
@@ -30,6 +30,7 @@ struct denali_dt {
 };
 
 struct denali_dt_data {
+	const int *ecc_strength_avail;
 	unsigned int caps;
 };
 
@@ -54,8 +55,10 @@ static int denali_dt_probe(struct platform_device *pdev)
 	denali = &dt->denali;
 
 	data = of_device_get_match_data(&pdev->dev);
-	if (data)
+	if (data) {
+		denali->ecc_strength_avail = data->ecc_strength_avail;
 		denali->caps = data->caps;
+	}
 
 	denali->caps |= DENALI_CAPS_HW_ECC_FIXUP;
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1530735 — [PATCH 02/39] mtd: nand: denali: remove unused CONFIG option and macros

All of these macros are not used at all.
CONFIG_MTD_NAND_DENALI_SCRATCH_REG_ADDR is not used for anything but
defining SCRATCH_REG_ADDR.  The config option should go away as well.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mtd/nand/Kconfig  | 11 -----------
 drivers/mtd/nand/denali.h | 42 ------------------------------------------
 2 files changed, 53 deletions(-)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 7b7a887..5eb643e 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -60,17 +60,6 @@ config MTD_NAND_DENALI_DT
 	  Enable the driver for NAND flash on platforms using a Denali NAND
 	  controller as a DT device.
 
-config MTD_NAND_DENALI_SCRATCH_REG_ADDR
-        hex "Denali NAND size scratch register address"
-        default "0xFF108018"
-        depends on MTD_NAND_DENALI_PCI
-        help
-          Some platforms place the NAND chip size in a scratch register
-          because (some versions of) the driver aren't able to automatically
-          determine the size of certain chips. Set the address of the
-          scratch register here to enable this feature. On Intel Moorestown
-          boards, the scratch register is at 0xFF108018.
-
 config MTD_NAND_GPIO
 	tristate "GPIO assisted NAND Flash driver"
 	depends on GPIOLIB || COMPILE_TEST
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index ea22191..fd1ae08 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -367,33 +367,16 @@
 #define     MIN_MAX_BANK__MIN_VALUE			0x0003
 #define     MIN_MAX_BANK__MAX_VALUE			0x000c
 
-
-/* ffsdefs.h */
-#define CLEAR 0                 /*use this to clear a field instead of "fail"*/
-#define SET   1                 /*use this to set a field instead of "pass"*/
 #define FAIL 1                  /*failed flag*/
 #define PASS 0                  /*success flag*/
 #define ERR -1                  /*error flag*/
 
-/* lld.h */
-#define GOOD_BLOCK 0
-#define DEFECTIVE_BLOCK 1
-#define READ_ERROR 2
-
 #define CLK_X  5
 #define CLK_MULTI 4
 
-/* KBV - Updated to LNW scratch register address */
-#define SCRATCH_REG_ADDR    CONFIG_MTD_NAND_DENALI_SCRATCH_REG_ADDR
-#define SCRATCH_REG_SIZE    64
-
-#define GLOB_HWCTL_DEFAULT_BLKS    2048
-
 #define SUPPORT_15BITECC        1
 #define SUPPORT_8BITECC         1
 
-#define CUSTOM_CONF_PARAMS      0
-
 #define ONFI_BLOOM_TIME         1
 #define MODE5_WORKAROUND        0
 
@@ -403,31 +386,6 @@
 #define MODE_10    0x08000000
 #define MODE_11    0x0C000000
 
-
-#define DATA_TRANSFER_MODE              0
-#define PROTECTION_PER_BLOCK            1
-#define LOAD_WAIT_COUNT                 2
-#define PROGRAM_WAIT_COUNT              3
-#define ERASE_WAIT_COUNT                4
-#define INT_MONITOR_CYCLE_COUNT         5
-#define READ_BUSY_PIN_ENABLED           6
-#define MULTIPLANE_OPERATION_SUPPORT    7
-#define PRE_FETCH_MODE                  8
-#define CE_DONT_CARE_SUPPORT            9
-#define COPYBACK_SUPPORT                10
-#define CACHE_WRITE_SUPPORT             11
-#define CACHE_READ_SUPPORT              12
-#define NUM_PAGES_IN_BLOCK              13
-#define ECC_ENABLE_SELECT               14
-#define WRITE_ENABLE_2_READ_ENABLE      15
-#define ADDRESS_2_DATA                  16
-#define READ_ENABLE_2_WRITE_ENABLE      17
-#define TWO_ROW_ADDRESS_CYCLES          18
-#define MULTIPLANE_ADDRESS_RESTRICT     19
-#define ACC_CLOCKS                      20
-#define READ_WRITE_ENABLE_LOW_COUNT     21
-#define READ_WRITE_ENABLE_HIGH_COUNT    22
-
 #define ECC_SECTOR_SIZE     512
 
 struct nand_buf {
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1530736 — [PATCH 30/39] mtd: nand: denali: set DEVICES_CONNECTED 1 if not set

Currently, the driver expects DEVICE_CONNECTED is automatically set
by the hardware, but this feature is disabled in some cases.
In such cases, it is the software's responsibility to set up the
DEVICES_CONNECTED register.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mtd/nand/denali.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 71bdf2a..4b2bbcb 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1485,6 +1485,13 @@ static void denali_multidev_fixup(struct denali_nand_info *denali)
 	 */
 	denali->devnum = ioread32(denali->flash_reg + DEVICES_CONNECTED);
 
+	/*
+	 * On some SoCs, DEVICES_CONNECTED is not auto-detected. For those SoCs
+	 * DEVICES_CONNECT is left to 0.  Set 1 if it is the case.
+	 */
+	if (denali->devnum == 0)
+		denali->devnum = 1;
+
 	devnum_shift = fls(denali->devnum) - 1;
 
 	if (denali->devnum != BIT(devnum_shift)) {
@@ -1492,9 +1499,10 @@ static void denali_multidev_fixup(struct denali_nand_info *denali)
 			 "DEVICE_CONNECTED=%d is not power of 2. Some devices will not be used.\n",
 			 denali->devnum);
 		denali->devnum = BIT(devnum_shift);
-		iowrite32(denali->devnum, denali->flash_reg + DEVICES_CONNECTED);
 	}
 
+	iowrite32(denali->devnum, denali->flash_reg + DEVICES_CONNECTED);
+
 	mtd->size <<= devnum_shift;
 	mtd->erasesize <<= devnum_shift;
 	mtd->writesize <<= devnum_shift;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1530737 — [PATCH 27/39] mtd: nand: denali: do not set mtd->name

This will be filled by nand_scan_ident() later.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mtd/nand/denali.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 0f94381b..60b0858 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1509,7 +1509,6 @@ int denali_init(struct denali_nand_info *denali)
 
 	/* now that our ISR is registered, we can enable interrupts */
 	denali_set_intr_modes(denali, true);
-	mtd->name = "denali-nand";
 	nand_set_flash_node(chip, denali->dev->of_node);
 
 	/* register the driver with the NAND core subsystem */
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1530738 — [PATCH 35/39] mtd: nand: denali: calculate ecc.strength and ecc.bytes generically

Another problem of this driver is hard-coded ecc.strength and
ecc.bytes.  Currently ecc.bytes is defined as follows:

  #define ECC_8BITS	14
  #define ECC_15BITS	26

Such parameters were hard-coded because only the following two cases
are possible on Intel platforms:

 - ecc.size = 512, ecc.strength = 8    --> ecc.bytes = 14
 - ecc.size = 512, ecc.strength = 15   --> ecc.bytes = 26

However, these are actually customizable parameters, for example,
UniPhier platform supports the following:

 - ecc.size = 1024, ecc.strength = 8   --> ecc.bytes = 14
 - ecc.size = 1024, ecc.strength = 16  --> ecc.bytes = 28
 - ecc.size = 1024, ecc.strength = 24  --> ecc.bytes = 42

So, we need somehow a generic way to calculate these parameters.
Fortunately, the Denali User's Guide explains how to calculate the
ecc.bytes.  The formula is:

  ecc.bytes = 2 * CEIL(13 * ecc.strength / 16)  (for ecc.size = 512)
  ecc.bytes = 2 * CEIL(14 * ecc.strength / 16)  (for ecc.size = 1024)

This commit implements two functions denali_calc_ecc_bytes() and
denali_set_max_ecc_strength() to prepare for the next commit,
where I will allow to use SoC-dependent ecc.strength values.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mtd/nand/denali.c | 72 +++++++++++++++++++++++++++++------------------
 1 file changed, 45 insertions(+), 27 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index c17e92b..f2ed3f8 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1354,13 +1354,44 @@ static void denali_hw_init(struct denali_nand_info *denali)
 	denali_irq_init(denali);
 }
 
-/*
- * Althogh controller spec said SLC ECC is forceb to be 4bit,
- * but denali controller in MRST only support 15bit and 8bit ECC
- * correction
- */
-#define ECC_8BITS	14
-#define ECC_15BITS	26
+/* default of supported ECC strength for Intel platforms. */
+static const int denali_default_ecc_strength[] = {
+	15, 8, 0,
+};
+
+static int denali_calc_ecc_bytes(int ecc_size, int ecc_strength)
+{
+	WARN_ON(ecc_size != 512 && ecc_size != 1024);
+
+	return DIV_ROUND_UP(ecc_strength * (ecc_size == 512 ? 13 : 14), 16) * 2;
+}
+
+static int denali_set_max_ecc_strength(struct denali_nand_info *denali)
+{
+	struct nand_chip *chip = &denali->nand;
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	int oobsize = mtd->oobsize;
+	int ecc_size = chip->ecc.size;
+	int ecc_steps = mtd->writesize / chip->ecc.size;
+	const int *ecc_strength = denali_default_ecc_strength;
+	int ecc_bytes;
+
+	/* carve out the BBM area */
+	oobsize -= denali->bbtskipbytes;
+
+	for (; *ecc_strength; ecc_strength++) {
+		ecc_bytes = denali_calc_ecc_bytes(ecc_size, *ecc_strength);
+		if (oobsize >= ecc_bytes * ecc_steps) {
+			chip->ecc.strength = *ecc_strength;
+			return 0;
+		}
+	}
+
+	dev_err(denali->dev,
+		"Your NAND chip OOB is too small. No available ECC strength.\n");
+
+	return -EINVAL;
+}
 
 static int denali_ooblayout_ecc(struct mtd_info *mtd, int section,
 				struct mtd_oob_region *oobregion)
@@ -1594,27 +1625,14 @@ int denali_init(struct denali_nand_info *denali)
 		chip->ecc.size = denali->caps & DENALI_CAPS_ECC_SIZE_1024 ?
 								1024 : 512;
 
-	/*
-	 * Denali Controller only support 15bit and 8bit ECC in MRST,
-	 * so just let controller do 15bit ECC for MLC and 8bit ECC for
-	 * SLC if possible.
-	 * */
-	if (!nand_is_slc(chip) &&
-			mtd->oobsize >= denali->bbtskipbytes +
-			ECC_15BITS * (mtd->writesize / chip->ecc.size)) {
-		/* if MLC OOB size is large enough, use 15bit ECC*/
-		chip->ecc.strength = 15;
-		chip->ecc.bytes = ECC_15BITS;
-		iowrite32(15, denali->flash_reg + ECC_CORRECTION);
-	} else if (mtd->oobsize <
-		   denali->bbtskipbytes + ECC_8BITS * (mtd->writesize / chip->ecc.size)) {
-		pr_err("Your NAND chip OOB is not large enough to contain 8bit ECC correction codes");
+	ret = denali_set_max_ecc_strength(denali);
+	if (ret)
 		goto failed_req_irq;
-	} else {
-		chip->ecc.strength = 8;
-		chip->ecc.bytes = ECC_8BITS;
-		iowrite32(8, denali->flash_reg + ECC_CORRECTION);
-	}
+
+	chip->ecc.bytes = denali_calc_ecc_bytes(chip->ecc.size,
+						chip->ecc.strength);
+
+	iowrite32(chip->ecc.strength, denali->flash_reg + ECC_CORRECTION);
 
 	mtd_set_ooblayout(mtd, &denali_ooblayout_ops);
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1530739 — [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants

Add two compatible strings for UniPhier SoCs.  The revision register
on both shows revision 5.0, but they are different hardware.

Features:
 - DMA engine with 64 bit physical address support
 - 1024 byte ECC step size
 - 8 / 16 / 24 bit ECC strength
 - The n_banks format depends on SoC

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 .../devicetree/bindings/mtd/denali-nand.txt        | 10 +++++--
 drivers/mtd/nand/denali_dt.c                       | 33 ++++++++++++++++++++--
 2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/denali-nand.txt b/Documentation/devicetree/bindings/mtd/denali-nand.txt
index 51fe195..cea46e2 100644
--- a/Documentation/devicetree/bindings/mtd/denali-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/denali-nand.txt
@@ -1,13 +1,19 @@
 * Denali NAND controller
 
 Required properties:
-  - compatible : should be "denali,denali-nand-dt"
+  - compatible : should be one of the following:
+      "denali,denali-nand-dt"
+      "denali,denali-nand-uniphier-v5a"
+      "denali,denali-nand-uniphier-v5b"
   - reg : should contain registers location and length for data and reg.
   - reg-names: Should contain the reg names "nand_data" and "denali_reg"
   - interrupts : The interrupt number.
 
 Optional properties:
-  - nand-ecc-step-size: must be 512 or 1024.  If not specified, default to 512.
+  - nand-ecc-step-size: must be 512 or 1024.  If not specified, default to:
+      512  for "denali,denali-nand-dt"
+      1024 for "denali,denali-nand-uniphier-v5a"
+      1024 for "denali,denali-nand-uniphier-v5b"
     see nand.txt for details.
   - nand-ecc-strength: see nand.txt for details
   - nand-ecc-maximize: see nand.txt for details
diff --git a/drivers/mtd/nand/denali_dt.c b/drivers/mtd/nand/denali_dt.c
index aa1e032..b411889 100644
--- a/drivers/mtd/nand/denali_dt.c
+++ b/drivers/mtd/nand/denali_dt.c
@@ -34,10 +34,37 @@ struct denali_dt_data {
 	unsigned int caps;
 };
 
+static const int denali_uniphier_ecc_strength[] = {
+	24, 16, 8, 0,
+};
+
+static const struct denali_dt_data denali_uniphier_v5a_data = {
+	.ecc_strength_avail = denali_uniphier_ecc_strength,
+	.caps = DENALI_CAPS_DMA_64BIT |
+		DENALI_CAPS_ECC_SIZE_1024,
+};
+
+static const struct denali_dt_data denali_uniphier_v5b_data = {
+	.ecc_strength_avail = denali_uniphier_ecc_strength,
+	.caps = DENALI_CAPS_DMA_64BIT |
+		DENALI_CAPS_NEW_N_BANKS_FORMAT |
+		DENALI_CAPS_ECC_SIZE_1024,
+};
+
 static const struct of_device_id denali_nand_dt_ids[] = {
-		{ .compatible = "denali,denali-nand-dt" },
-		{ /* sentinel */ }
-	};
+	{
+		.compatible = "denali,denali-nand-dt",
+	},
+	{
+		.compatible = "denali,denali-nand-uniphier-v5a",
+		.data = &denali_uniphier_v5a_data,
+	},
+	{
+		.compatible = "denali,denali-nand-uniphier-v5b",
+		.data = &denali_uniphier_v5b_data,
+	},
+	{ /* sentinel */ }
+};
 
 MODULE_DEVICE_TABLE(of, denali_nand_dt_ids);
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1534243 — Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants

On Sun, Nov 27, 2016 at 03:06:25AM +0900, Masahiro Yamada wrote:
> Add two compatible strings for UniPhier SoCs.  The revision register
> on both shows revision 5.0, but they are different hardware.
> 
> Features:
>  - DMA engine with 64 bit physical address support
>  - 1024 byte ECC step size
>  - 8 / 16 / 24 bit ECC strength
>  - The n_banks format depends on SoC
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  .../devicetree/bindings/mtd/denali-nand.txt        | 10 +++++--
>  drivers/mtd/nand/denali_dt.c                       | 33 ++++++++++++++++++++--
>  2 files changed, 38 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/denali-nand.txt b/Documentation/devicetree/bindings/mtd/denali-nand.txt
> index 51fe195..cea46e2 100644
> --- a/Documentation/devicetree/bindings/mtd/denali-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/denali-nand.txt
> @@ -1,13 +1,19 @@
>  * Denali NAND controller
>  
>  Required properties:
> -  - compatible : should be "denali,denali-nand-dt"
> +  - compatible : should be one of the following:
> +      "denali,denali-nand-dt"

There are multiple things wrong with this string. denali,denali is 
redundant is one. It's also fairly useless as this IP has several 
versions and numerous configuration options IIRC. This should be 
deprecated IMO.

> +      "denali,denali-nand-uniphier-v5a"
> +      "denali,denali-nand-uniphier-v5b"

Use your vendor prefix, not denali. The 2nd denali can probably be 
dropped because it is not likely you have another kind of nand 
controller in the SoC.

>    - reg : should contain registers location and length for data and reg.
>    - reg-names: Should contain the reg names "nand_data" and "denali_reg"
>    - interrupts : The interrupt number.
>  
>  Optional properties:
> -  - nand-ecc-step-size: must be 512 or 1024.  If not specified, default to 512.
> +  - nand-ecc-step-size: must be 512 or 1024.  If not specified, default to:
> +      512  for "denali,denali-nand-dt"
> +      1024 for "denali,denali-nand-uniphier-v5a"
> +      1024 for "denali,denali-nand-uniphier-v5b"
>      see nand.txt for details.
>    - nand-ecc-strength: see nand.txt for details
>    - nand-ecc-maximize: see nand.txt for details

[toc] | [prev] | [next] | [standalone]

#1534644 — Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants

Hi Rob,
(+CC Dinh)

2016-12-02 1:05 GMT+09:00 Rob Herring <robh@kernel.org>:
> On Sun, Nov 27, 2016 at 03:06:25AM +0900, Masahiro Yamada wrote:
>> Add two compatible strings for UniPhier SoCs.  The revision register
>> on both shows revision 5.0, but they are different hardware.
>>
>> Features:
>>  - DMA engine with 64 bit physical address support
>>  - 1024 byte ECC step size
>>  - 8 / 16 / 24 bit ECC strength
>>  - The n_banks format depends on SoC
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>>  .../devicetree/bindings/mtd/denali-nand.txt        | 10 +++++--
>>  drivers/mtd/nand/denali_dt.c                       | 33 ++++++++++++++++++++--
>>  2 files changed, 38 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/denali-nand.txt b/Documentation/devicetree/bindings/mtd/denali-nand.txt
>> index 51fe195..cea46e2 100644
>> --- a/Documentation/devicetree/bindings/mtd/denali-nand.txt
>> +++ b/Documentation/devicetree/bindings/mtd/denali-nand.txt
>> @@ -1,13 +1,19 @@
>>  * Denali NAND controller
>>
>>  Required properties:
>> -  - compatible : should be "denali,denali-nand-dt"
>> +  - compatible : should be one of the following:
>> +      "denali,denali-nand-dt"
>
> There are multiple things wrong with this string. denali,denali is
> redundant is one.

One more redundancy; "-dt" is weird because
DT compatible should be a name of hardware.


> It's also fairly useless as this IP has several
> versions and numerous configuration options IIRC. This should be
> deprecated IMO.

Right.  There are several customizable parameters for this IP,
so a generic compatible string like this is probably useless.

This DT binding was added by commit 30f9f2f for Altera SOCFPGA,
A funny thing is that they upstreamed DT binding, but they did not upstream
needed changes for the Denali driver core.
So, the mainline driver has never worked on SOCFPGA
(or any of DT-based SoCs).



>> +      "denali,denali-nand-uniphier-v5a"
>> +      "denali,denali-nand-uniphier-v5b"
>
> Use your vendor prefix, not denali. The 2nd denali can probably be
> dropped because it is not likely you have another kind of nand
> controller in the SoC.

Hmm, your statement implies that a vendor prefix
belongs to an SoC vendor, not an IP vendor.
(I was not quite sure about this.)


It is unlikely to happen to have two different NAND controllers on one SoC.
But, we used a different NAND controller for our SoC family before
introducing the Denali IP.
It also implies that Socionext may use a different NAND IP in the future.
I'd like to include "denali" somewhere because it is clearly associated with
the driver name.
Also, this will give an idea what kind of _basic_ hardware is used,
even though we know various parameters are customizable.



(Plan A)
  "denali,socfpga-nand"           (for Altera SOCFPGA variant)
  "denali,uniphier-nand-v1"       (for old Socionext UniPhier family variant)
  "denali,uniphier-nand-v2"       (for new Socionext UniPhier family variant)

(Plan B)
  "altera,denali-nand"            (for Altera SOCFPGA variant)
  "socionext,denali-nand-v5a"     (for old Socionext UniPhier family variant)
  "socionext,denali-nand-v5b"     (for new Socionext UniPhier family variant)


I think Plan B is nearer to your suggestion,
and I think it is OK for Socionext (hopefully for Altera too).





-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]

#1535057 — Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants

On Thu, Dec 1, 2016 at 8:54 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Hi Rob,
> (+CC Dinh)
>
> 2016-12-02 1:05 GMT+09:00 Rob Herring <robh@kernel.org>:
>> On Sun, Nov 27, 2016 at 03:06:25AM +0900, Masahiro Yamada wrote:
>>> Add two compatible strings for UniPhier SoCs.  The revision register
>>> on both shows revision 5.0, but they are different hardware.
>>>
>>> Features:
>>>  - DMA engine with 64 bit physical address support
>>>  - 1024 byte ECC step size
>>>  - 8 / 16 / 24 bit ECC strength
>>>  - The n_banks format depends on SoC
>>>
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> ---
>>>
>>>  .../devicetree/bindings/mtd/denali-nand.txt        | 10 +++++--
>>>  drivers/mtd/nand/denali_dt.c                       | 33 ++++++++++++++++++++--
>>>  2 files changed, 38 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/denali-nand.txt b/Documentation/devicetree/bindings/mtd/denali-nand.txt
>>> index 51fe195..cea46e2 100644
>>> --- a/Documentation/devicetree/bindings/mtd/denali-nand.txt
>>> +++ b/Documentation/devicetree/bindings/mtd/denali-nand.txt
>>> @@ -1,13 +1,19 @@
>>>  * Denali NAND controller
>>>
>>>  Required properties:
>>> -  - compatible : should be "denali,denali-nand-dt"
>>> +  - compatible : should be one of the following:
>>> +      "denali,denali-nand-dt"
>>
>> There are multiple things wrong with this string. denali,denali is
>> redundant is one.
>
> One more redundancy; "-dt" is weird because
> DT compatible should be a name of hardware.
>
>
>> It's also fairly useless as this IP has several
>> versions and numerous configuration options IIRC. This should be
>> deprecated IMO.
>
> Right.  There are several customizable parameters for this IP,
> so a generic compatible string like this is probably useless.
>
> This DT binding was added by commit 30f9f2f for Altera SOCFPGA,
> A funny thing is that they upstreamed DT binding, but they did not upstream
> needed changes for the Denali driver core.
> So, the mainline driver has never worked on SOCFPGA
> (or any of DT-based SoCs).
>
>
>
>>> +      "denali,denali-nand-uniphier-v5a"
>>> +      "denali,denali-nand-uniphier-v5b"
>>
>> Use your vendor prefix, not denali. The 2nd denali can probably be
>> dropped because it is not likely you have another kind of nand
>> controller in the SoC.
>
> Hmm, your statement implies that a vendor prefix
> belongs to an SoC vendor, not an IP vendor.
> (I was not quite sure about this.)

Right. We do have some IP vendor strings, but they are usually
fallbacks. Sometimes, I guess they are useful, but IMO over time they
prove to not be useful.

> It is unlikely to happen to have two different NAND controllers on one SoC.
> But, we used a different NAND controller for our SoC family before
> introducing the Denali IP.
> It also implies that Socionext may use a different NAND IP in the future.
> I'd like to include "denali" somewhere because it is clearly associated with
> the driver name.
> Also, this will give an idea what kind of _basic_ hardware is used,
> even though we know various parameters are customizable.
>
>
>
> (Plan A)
>   "denali,socfpga-nand"           (for Altera SOCFPGA variant)
>   "denali,uniphier-nand-v1"       (for old Socionext UniPhier family variant)
>   "denali,uniphier-nand-v2"       (for new Socionext UniPhier family variant)
>
> (Plan B)
>   "altera,denali-nand"            (for Altera SOCFPGA variant)
>   "socionext,denali-nand-v5a"     (for old Socionext UniPhier family variant)
>   "socionext,denali-nand-v5b"     (for new Socionext UniPhier family variant)

Let the Altera folks worry about their stuff. At least for soft IP in
FPGA, it's a bit of a special case. The old string can remain as bad
as it is. I simply would do "socionext,uniphier-v5b-nand" (and v5a).
The fact that it is denali is part of the documentation.

Rob

[toc] | [prev] | [next] | [standalone]

#1535326 — Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants

Hi Rob,

2016-12-03 1:26 GMT+09:00 Rob Herring <robh@kernel.org>:

>>
>>
>> (Plan A)
>>   "denali,socfpga-nand"           (for Altera SOCFPGA variant)
>>   "denali,uniphier-nand-v1"       (for old Socionext UniPhier family variant)
>>   "denali,uniphier-nand-v2"       (for new Socionext UniPhier family variant)
>>
>> (Plan B)
>>   "altera,denali-nand"            (for Altera SOCFPGA variant)
>>   "socionext,denali-nand-v5a"     (for old Socionext UniPhier family variant)
>>   "socionext,denali-nand-v5b"     (for new Socionext UniPhier family variant)

> Let the Altera folks worry about their stuff. At least for soft IP in
> FPGA, it's a bit of a special case. The old string can remain as bad
> as it is.


Hmm, I am not sure if this IP would fit in FPGA
(to use it along with NIOS-II?)

(even if it happened, nothing of this IP would be customizable on users' side.
When buying the IP, SoC vendors submit a list of desired features.
Denali (now Cadence) generates the RTL according to the configuration sheet.
The function is fixed at this point. So, generic compatible would be
useless anyway.)


If we are talking about SOCFPGA,
SOCFPGA is not only FPGA. Rather "SOC" + "FPGA".
It consists of two parts:
[1] SOC part  (Cortex-A9 + various hard-wired peripherals such UART,
USB, SD, NAND, ...)
[2] FPGA part (User design logic)

The Denali NAND controller is included in [1].
So, as far as we talk about the Denali on SOCFPGA,
it is as hard-wired as Intel, Socionext's ones.


> I simply would do "socionext,uniphier-v5b-nand" (and v5a).
> The fact that it is denali is part of the documentation.
>

Let me think about this.

Socionext bought two version of Denali IP,
and we are now re-using the newer one (v5b) for several SoCs.
Socionext has some more product lines other than Uniphier SoC family,
perhaps wider re-use might happen in the future.

At first, I included "uniphier" in compatible, but I am still wondering
if such a specific string is good or not.

Also, comments from Altera engineers are appreciated.


-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]

#1535328 — Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants

On 12/03/2016 03:41 AM, Masahiro Yamada wrote:
> Hi Rob,

Hi!

> 2016-12-03 1:26 GMT+09:00 Rob Herring <robh@kernel.org>:
> 
>>>
>>>
>>> (Plan A)
>>>   "denali,socfpga-nand"           (for Altera SOCFPGA variant)
>>>   "denali,uniphier-nand-v1"       (for old Socionext UniPhier family variant)
>>>   "denali,uniphier-nand-v2"       (for new Socionext UniPhier family variant)
>>>
>>> (Plan B)
>>>   "altera,denali-nand"            (for Altera SOCFPGA variant)
>>>   "socionext,denali-nand-v5a"     (for old Socionext UniPhier family variant)
>>>   "socionext,denali-nand-v5b"     (for new Socionext UniPhier family variant)
> 
>> Let the Altera folks worry about their stuff. At least for soft IP in
>> FPGA, it's a bit of a special case. The old string can remain as bad
>> as it is.
> 
> 
> Hmm, I am not sure if this IP would fit in FPGA
> (to use it along with NIOS-II?)
> 
> (even if it happened, nothing of this IP would be customizable on users' side.
> When buying the IP, SoC vendors submit a list of desired features.
> Denali (now Cadence) generates the RTL according to the configuration sheet.
> The function is fixed at this point. So, generic compatible would be
> useless anyway.)
> 
> 
> If we are talking about SOCFPGA,
> SOCFPGA is not only FPGA. Rather "SOC" + "FPGA".
> It consists of two parts:
> [1] SOC part  (Cortex-A9 + various hard-wired peripherals such UART,
> USB, SD, NAND, ...)
> [2] FPGA part (User design logic)
> 
> The Denali NAND controller is included in [1].
> So, as far as we talk about the Denali on SOCFPGA,
> it is as hard-wired as Intel, Socionext's ones.

That's correct, the Denali NAND IP in altera socfpga is a hardware
block. You can make it available to the fabric too, but by default
it's used by the ARM part of the chip, so for this discussion, you
can forget that the FPGA part exists altogether.

I would be in favor of plan B, since it seems to be the more often
taken approach. A nice example is ci-hdrc:

$ git grep compatible drivers/usb/chipidea/

>> I simply would do "socionext,uniphier-v5b-nand" (and v5a).
>> The fact that it is denali is part of the documentation.
>>
> 
> Let me think about this.
> 
> Socionext bought two version of Denali IP,
> and we are now re-using the newer one (v5b) for several SoCs.
> Socionext has some more product lines other than Uniphier SoC family,
> perhaps wider re-use might happen in the future.
> 
> At first, I included "uniphier" in compatible, but I am still wondering
> if such a specific string is good or not.
> 
> Also, comments from Altera engineers are appreciated.

Adding a few more on Cc

-- 
Best regards,
Marek Vasut

[toc] | [prev] | [next] | [standalone]

#1535532 — Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants

Hi,

On Fri, Dec 2, 2016 at 8:49 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> On 12/03/2016 03:41 AM, Masahiro Yamada wrote:
>> Hi Rob,
>
> Hi!
>
>> 2016-12-03 1:26 GMT+09:00 Rob Herring <robh@kernel.org>:
>>
>>>>
>>>>
>>>> (Plan A)
>>>>   "denali,socfpga-nand"           (for Altera SOCFPGA variant)
>>>>   "denali,uniphier-nand-v1"       (for old Socionext UniPhier family variant)
>>>>   "denali,uniphier-nand-v2"       (for new Socionext UniPhier family variant)
>>>>
>>>> (Plan B)
>>>>   "altera,denali-nand"            (for Altera SOCFPGA variant)
>>>>   "socionext,denali-nand-v5a"     (for old Socionext UniPhier family variant)
>>>>   "socionext,denali-nand-v5b"     (for new Socionext UniPhier family variant)
>>
>>> Let the Altera folks worry about their stuff. At least for soft IP in
>>> FPGA, it's a bit of a special case. The old string can remain as bad
>>> as it is.
>>
>>
>> Hmm, I am not sure if this IP would fit in FPGA
>> (to use it along with NIOS-II?)
>>
>> (even if it happened, nothing of this IP would be customizable on users' side.
>> When buying the IP, SoC vendors submit a list of desired features.
>> Denali (now Cadence) generates the RTL according to the configuration sheet.
>> The function is fixed at this point. So, generic compatible would be
>> useless anyway.)
>>
>>
>> If we are talking about SOCFPGA,
>> SOCFPGA is not only FPGA. Rather "SOC" + "FPGA".
>> It consists of two parts:
>> [1] SOC part  (Cortex-A9 + various hard-wired peripherals such UART,
>> USB, SD, NAND, ...)
>> [2] FPGA part (User design logic)
>>
>> The Denali NAND controller is included in [1].
>> So, as far as we talk about the Denali on SOCFPGA,
>> it is as hard-wired as Intel, Socionext's ones.
>
> That's correct, the Denali NAND IP in altera socfpga is a hardware
> block. You can make it available to the fabric too, but by default
> it's used by the ARM part of the chip, so for this discussion, you
> can forget that the FPGA part exists altogether.
>
> I would be in favor of plan B, since it seems to be the more often
> taken approach. A nice example is ci-hdrc:
>
> $ git grep compatible drivers/usb/chipidea/
>
>>> I simply would do "socionext,uniphier-v5b-nand" (and v5a).
>>> The fact that it is denali is part of the documentation.
>>>
>>
>> Let me think about this.
>>
>> Socionext bought two version of Denali IP,
>> and we are now re-using the newer one (v5b) for several SoCs.
>> Socionext has some more product lines other than Uniphier SoC family,
>> perhaps wider re-use might happen in the future.
>>
>> At first, I included "uniphier" in compatible, but I am still wondering
>> if such a specific string is good or not.
>>
>> Also, comments from Altera engineers are appreciated.

Sorry, it's taken me a while to add comments. My altera email is very spotty now
that the Intel merge is completed. Please use dinguyen@kernel.org for any future
communications.

Yes, everything that is said so far for the NAND controller on the
SoCFPGA is correct. I added the binding for the controller a while
back, but unfortunately, we never added the NAND interface to the
devkit, so we did not do much in terms of enabling it.

I think the only SoCFPGA board I know that has the NAND interface active is
the TRCom board, but I have never seen that board.

I don't have any strong opinions on this matter, just as long as the
original binding
"denali,denali-nand-dt" is kept, and I think Rob was ok with keeping
that binding.

Dinh

[toc] | [prev] | [next] | [standalone]

#1535786 — Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants

Hi Dinh,


2016-12-04 7:08 GMT+09:00 Dinh Nguyen <dinh.linux@gmail.com>:
> Hi,
>
> On Fri, Dec 2, 2016 at 8:49 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> On 12/03/2016 03:41 AM, Masahiro Yamada wrote:
>>> Hi Rob,
>>
>> Hi!
>>
>>> 2016-12-03 1:26 GMT+09:00 Rob Herring <robh@kernel.org>:
>>>
>>>>>
>>>>>
>>>>> (Plan A)
>>>>>   "denali,socfpga-nand"           (for Altera SOCFPGA variant)
>>>>>   "denali,uniphier-nand-v1"       (for old Socionext UniPhier family variant)
>>>>>   "denali,uniphier-nand-v2"       (for new Socionext UniPhier family variant)
>>>>>
>>>>> (Plan B)
>>>>>   "altera,denali-nand"            (for Altera SOCFPGA variant)
>>>>>   "socionext,denali-nand-v5a"     (for old Socionext UniPhier family variant)
>>>>>   "socionext,denali-nand-v5b"     (for new Socionext UniPhier family variant)
>>>
>>>> Let the Altera folks worry about their stuff. At least for soft IP in
>>>> FPGA, it's a bit of a special case. The old string can remain as bad
>>>> as it is.
>>>
>>>
>>> Hmm, I am not sure if this IP would fit in FPGA
>>> (to use it along with NIOS-II?)
>>>
>>> (even if it happened, nothing of this IP would be customizable on users' side.
>>> When buying the IP, SoC vendors submit a list of desired features.
>>> Denali (now Cadence) generates the RTL according to the configuration sheet.
>>> The function is fixed at this point. So, generic compatible would be
>>> useless anyway.)
>>>
>>>
>>> If we are talking about SOCFPGA,
>>> SOCFPGA is not only FPGA. Rather "SOC" + "FPGA".
>>> It consists of two parts:
>>> [1] SOC part  (Cortex-A9 + various hard-wired peripherals such UART,
>>> USB, SD, NAND, ...)
>>> [2] FPGA part (User design logic)
>>>
>>> The Denali NAND controller is included in [1].
>>> So, as far as we talk about the Denali on SOCFPGA,
>>> it is as hard-wired as Intel, Socionext's ones.
>>
>> That's correct, the Denali NAND IP in altera socfpga is a hardware
>> block. You can make it available to the fabric too, but by default
>> it's used by the ARM part of the chip, so for this discussion, you
>> can forget that the FPGA part exists altogether.
>>
>> I would be in favor of plan B, since it seems to be the more often
>> taken approach. A nice example is ci-hdrc:
>>
>> $ git grep compatible drivers/usb/chipidea/
>>
>>>> I simply would do "socionext,uniphier-v5b-nand" (and v5a).
>>>> The fact that it is denali is part of the documentation.
>>>>
>>>
>>> Let me think about this.
>>>
>>> Socionext bought two version of Denali IP,
>>> and we are now re-using the newer one (v5b) for several SoCs.
>>> Socionext has some more product lines other than Uniphier SoC family,
>>> perhaps wider re-use might happen in the future.
>>>
>>> At first, I included "uniphier" in compatible, but I am still wondering
>>> if such a specific string is good or not.
>>>
>>> Also, comments from Altera engineers are appreciated.
>
> Sorry, it's taken me a while to add comments. My altera email is very spotty now
> that the Intel merge is completed. Please use dinguyen@kernel.org for any future
> communications.
>
> Yes, everything that is said so far for the NAND controller on the
> SoCFPGA is correct. I added the binding for the controller a while
> back, but unfortunately, we never added the NAND interface to the
> devkit, so we did not do much in terms of enabling it.
>
> I think the only SoCFPGA board I know that has the NAND interface active is
> the TRCom board, but I have never seen that board.
>
> I don't have any strong opinions on this matter, just as long as the
> original binding
> "denali,denali-nand-dt" is kept, and I think Rob was ok with keeping
> that binding.
>

I am proposing to add "altera,denali-nand" for Altera.
For what, do you need the generic compatible?
This IP has no default for it to fallback to.


-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]

#1535792 — Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants

On 12/05/2016 04:30 AM, Masahiro Yamada wrote:
> Hi Dinh,
> 
> 
> 2016-12-04 7:08 GMT+09:00 Dinh Nguyen <dinh.linux@gmail.com>:
>> Hi,
>>
>> On Fri, Dec 2, 2016 at 8:49 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>> On 12/03/2016 03:41 AM, Masahiro Yamada wrote:
>>>> Hi Rob,
>>>
>>> Hi!
>>>
>>>> 2016-12-03 1:26 GMT+09:00 Rob Herring <robh@kernel.org>:
>>>>
>>>>>>
>>>>>>
>>>>>> (Plan A)
>>>>>>   "denali,socfpga-nand"           (for Altera SOCFPGA variant)
>>>>>>   "denali,uniphier-nand-v1"       (for old Socionext UniPhier family variant)
>>>>>>   "denali,uniphier-nand-v2"       (for new Socionext UniPhier family variant)
>>>>>>
>>>>>> (Plan B)
>>>>>>   "altera,denali-nand"            (for Altera SOCFPGA variant)
>>>>>>   "socionext,denali-nand-v5a"     (for old Socionext UniPhier family variant)
>>>>>>   "socionext,denali-nand-v5b"     (for new Socionext UniPhier family variant)
>>>>
>>>>> Let the Altera folks worry about their stuff. At least for soft IP in
>>>>> FPGA, it's a bit of a special case. The old string can remain as bad
>>>>> as it is.
>>>>
>>>>
>>>> Hmm, I am not sure if this IP would fit in FPGA
>>>> (to use it along with NIOS-II?)
>>>>
>>>> (even if it happened, nothing of this IP would be customizable on users' side.
>>>> When buying the IP, SoC vendors submit a list of desired features.
>>>> Denali (now Cadence) generates the RTL according to the configuration sheet.
>>>> The function is fixed at this point. So, generic compatible would be
>>>> useless anyway.)
>>>>
>>>>
>>>> If we are talking about SOCFPGA,
>>>> SOCFPGA is not only FPGA. Rather "SOC" + "FPGA".
>>>> It consists of two parts:
>>>> [1] SOC part  (Cortex-A9 + various hard-wired peripherals such UART,
>>>> USB, SD, NAND, ...)
>>>> [2] FPGA part (User design logic)
>>>>
>>>> The Denali NAND controller is included in [1].
>>>> So, as far as we talk about the Denali on SOCFPGA,
>>>> it is as hard-wired as Intel, Socionext's ones.
>>>
>>> That's correct, the Denali NAND IP in altera socfpga is a hardware
>>> block. You can make it available to the fabric too, but by default
>>> it's used by the ARM part of the chip, so for this discussion, you
>>> can forget that the FPGA part exists altogether.
>>>
>>> I would be in favor of plan B, since it seems to be the more often
>>> taken approach. A nice example is ci-hdrc:
>>>
>>> $ git grep compatible drivers/usb/chipidea/
>>>
>>>>> I simply would do "socionext,uniphier-v5b-nand" (and v5a).
>>>>> The fact that it is denali is part of the documentation.
>>>>>
>>>>
>>>> Let me think about this.
>>>>
>>>> Socionext bought two version of Denali IP,
>>>> and we are now re-using the newer one (v5b) for several SoCs.
>>>> Socionext has some more product lines other than Uniphier SoC family,
>>>> perhaps wider re-use might happen in the future.
>>>>
>>>> At first, I included "uniphier" in compatible, but I am still wondering
>>>> if such a specific string is good or not.
>>>>
>>>> Also, comments from Altera engineers are appreciated.
>>
>> Sorry, it's taken me a while to add comments. My altera email is very spotty now
>> that the Intel merge is completed. Please use dinguyen@kernel.org for any future
>> communications.
>>
>> Yes, everything that is said so far for the NAND controller on the
>> SoCFPGA is correct. I added the binding for the controller a while
>> back, but unfortunately, we never added the NAND interface to the
>> devkit, so we did not do much in terms of enabling it.
>>
>> I think the only SoCFPGA board I know that has the NAND interface active is
>> the TRCom board, but I have never seen that board.
>>
>> I don't have any strong opinions on this matter, just as long as the
>> original binding
>> "denali,denali-nand-dt" is kept, and I think Rob was ok with keeping
>> that binding.
>>
> 
> I am proposing to add "altera,denali-nand" for Altera.
> For what, do you need the generic compatible?
> This IP has no default for it to fallback to.

IMO just for compatibility reasons with old DTs .
I'm also for "altera,denali-nand" or maybe "altera,socfpga-denali-nand"
to be more precise ?

-- 
Best regards,
Marek Vasut

[toc] | [prev] | [next] | [standalone]

#1535801 — Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants

Hi Marek,


2016-12-05 12:44 GMT+09:00 Marek Vasut <marek.vasut@gmail.com>:
> On 12/05/2016 04:30 AM, Masahiro Yamada wrote:
>> Hi Dinh,
>>
>>
>> 2016-12-04 7:08 GMT+09:00 Dinh Nguyen <dinh.linux@gmail.com>:
>>> Hi,
>>>
>>> On Fri, Dec 2, 2016 at 8:49 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>>> On 12/03/2016 03:41 AM, Masahiro Yamada wrote:
>>>>> Hi Rob,
>>>>
>>>> Hi!
>>>>
>>>>> 2016-12-03 1:26 GMT+09:00 Rob Herring <robh@kernel.org>:
>>>>>
>>>>>>>
>>>>>>>
>>>>>>> (Plan A)
>>>>>>>   "denali,socfpga-nand"           (for Altera SOCFPGA variant)
>>>>>>>   "denali,uniphier-nand-v1"       (for old Socionext UniPhier family variant)
>>>>>>>   "denali,uniphier-nand-v2"       (for new Socionext UniPhier family variant)
>>>>>>>
>>>>>>> (Plan B)
>>>>>>>   "altera,denali-nand"            (for Altera SOCFPGA variant)
>>>>>>>   "socionext,denali-nand-v5a"     (for old Socionext UniPhier family variant)
>>>>>>>   "socionext,denali-nand-v5b"     (for new Socionext UniPhier family variant)
>>>>>
>>>>>> Let the Altera folks worry about their stuff. At least for soft IP in
>>>>>> FPGA, it's a bit of a special case. The old string can remain as bad
>>>>>> as it is.
>>>>>
>>>>>
>>>>> Hmm, I am not sure if this IP would fit in FPGA
>>>>> (to use it along with NIOS-II?)
>>>>>
>>>>> (even if it happened, nothing of this IP would be customizable on users' side.
>>>>> When buying the IP, SoC vendors submit a list of desired features.
>>>>> Denali (now Cadence) generates the RTL according to the configuration sheet.
>>>>> The function is fixed at this point. So, generic compatible would be
>>>>> useless anyway.)
>>>>>
>>>>>
>>>>> If we are talking about SOCFPGA,
>>>>> SOCFPGA is not only FPGA. Rather "SOC" + "FPGA".
>>>>> It consists of two parts:
>>>>> [1] SOC part  (Cortex-A9 + various hard-wired peripherals such UART,
>>>>> USB, SD, NAND, ...)
>>>>> [2] FPGA part (User design logic)
>>>>>
>>>>> The Denali NAND controller is included in [1].
>>>>> So, as far as we talk about the Denali on SOCFPGA,
>>>>> it is as hard-wired as Intel, Socionext's ones.
>>>>
>>>> That's correct, the Denali NAND IP in altera socfpga is a hardware
>>>> block. You can make it available to the fabric too, but by default
>>>> it's used by the ARM part of the chip, so for this discussion, you
>>>> can forget that the FPGA part exists altogether.
>>>>
>>>> I would be in favor of plan B, since it seems to be the more often
>>>> taken approach. A nice example is ci-hdrc:
>>>>
>>>> $ git grep compatible drivers/usb/chipidea/
>>>>
>>>>>> I simply would do "socionext,uniphier-v5b-nand" (and v5a).
>>>>>> The fact that it is denali is part of the documentation.
>>>>>>
>>>>>
>>>>> Let me think about this.
>>>>>
>>>>> Socionext bought two version of Denali IP,
>>>>> and we are now re-using the newer one (v5b) for several SoCs.
>>>>> Socionext has some more product lines other than Uniphier SoC family,
>>>>> perhaps wider re-use might happen in the future.
>>>>>
>>>>> At first, I included "uniphier" in compatible, but I am still wondering
>>>>> if such a specific string is good or not.
>>>>>
>>>>> Also, comments from Altera engineers are appreciated.
>>>
>>> Sorry, it's taken me a while to add comments. My altera email is very spotty now
>>> that the Intel merge is completed. Please use dinguyen@kernel.org for any future
>>> communications.
>>>
>>> Yes, everything that is said so far for the NAND controller on the
>>> SoCFPGA is correct. I added the binding for the controller a while
>>> back, but unfortunately, we never added the NAND interface to the
>>> devkit, so we did not do much in terms of enabling it.
>>>
>>> I think the only SoCFPGA board I know that has the NAND interface active is
>>> the TRCom board, but I have never seen that board.
>>>
>>> I don't have any strong opinions on this matter, just as long as the
>>> original binding
>>> "denali,denali-nand-dt" is kept, and I think Rob was ok with keeping
>>> that binding.
>>>
>>
>> I am proposing to add "altera,denali-nand" for Altera.
>> For what, do you need the generic compatible?
>> This IP has no default for it to fallback to.
>
> IMO just for compatibility reasons with old DTs .

We generally contribute for
a "working driver" (at least, should be functional to some extent)
and "DT binding" bundled together.

However, Altera upstreamed the DT binding first
(then some parts of the DT binding turned out wrong),
but they did not upstream needed driver changes in the end.

So, the mainline driver has never worked on SOCFPGA, right?
Removing "denali,denali-nand-dt" is not breakage at all,
so I do not owe anything to them, right?



-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]

#1535804 — Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants

On 12/05/2016 05:10 AM, Masahiro Yamada wrote:
> Hi Marek,
> 
> 
> 2016-12-05 12:44 GMT+09:00 Marek Vasut <marek.vasut@gmail.com>:
>> On 12/05/2016 04:30 AM, Masahiro Yamada wrote:
>>> Hi Dinh,
>>>
>>>
>>> 2016-12-04 7:08 GMT+09:00 Dinh Nguyen <dinh.linux@gmail.com>:
>>>> Hi,
>>>>
>>>> On Fri, Dec 2, 2016 at 8:49 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>> On 12/03/2016 03:41 AM, Masahiro Yamada wrote:
>>>>>> Hi Rob,
>>>>>
>>>>> Hi!
>>>>>
>>>>>> 2016-12-03 1:26 GMT+09:00 Rob Herring <robh@kernel.org>:
>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> (Plan A)
>>>>>>>>   "denali,socfpga-nand"           (for Altera SOCFPGA variant)
>>>>>>>>   "denali,uniphier-nand-v1"       (for old Socionext UniPhier family variant)
>>>>>>>>   "denali,uniphier-nand-v2"       (for new Socionext UniPhier family variant)
>>>>>>>>
>>>>>>>> (Plan B)
>>>>>>>>   "altera,denali-nand"            (for Altera SOCFPGA variant)
>>>>>>>>   "socionext,denali-nand-v5a"     (for old Socionext UniPhier family variant)
>>>>>>>>   "socionext,denali-nand-v5b"     (for new Socionext UniPhier family variant)
>>>>>>
>>>>>>> Let the Altera folks worry about their stuff. At least for soft IP in
>>>>>>> FPGA, it's a bit of a special case. The old string can remain as bad
>>>>>>> as it is.
>>>>>>
>>>>>>
>>>>>> Hmm, I am not sure if this IP would fit in FPGA
>>>>>> (to use it along with NIOS-II?)
>>>>>>
>>>>>> (even if it happened, nothing of this IP would be customizable on users' side.
>>>>>> When buying the IP, SoC vendors submit a list of desired features.
>>>>>> Denali (now Cadence) generates the RTL according to the configuration sheet.
>>>>>> The function is fixed at this point. So, generic compatible would be
>>>>>> useless anyway.)
>>>>>>
>>>>>>
>>>>>> If we are talking about SOCFPGA,
>>>>>> SOCFPGA is not only FPGA. Rather "SOC" + "FPGA".
>>>>>> It consists of two parts:
>>>>>> [1] SOC part  (Cortex-A9 + various hard-wired peripherals such UART,
>>>>>> USB, SD, NAND, ...)
>>>>>> [2] FPGA part (User design logic)
>>>>>>
>>>>>> The Denali NAND controller is included in [1].
>>>>>> So, as far as we talk about the Denali on SOCFPGA,
>>>>>> it is as hard-wired as Intel, Socionext's ones.
>>>>>
>>>>> That's correct, the Denali NAND IP in altera socfpga is a hardware
>>>>> block. You can make it available to the fabric too, but by default
>>>>> it's used by the ARM part of the chip, so for this discussion, you
>>>>> can forget that the FPGA part exists altogether.
>>>>>
>>>>> I would be in favor of plan B, since it seems to be the more often
>>>>> taken approach. A nice example is ci-hdrc:
>>>>>
>>>>> $ git grep compatible drivers/usb/chipidea/
>>>>>
>>>>>>> I simply would do "socionext,uniphier-v5b-nand" (and v5a).
>>>>>>> The fact that it is denali is part of the documentation.
>>>>>>>
>>>>>>
>>>>>> Let me think about this.
>>>>>>
>>>>>> Socionext bought two version of Denali IP,
>>>>>> and we are now re-using the newer one (v5b) for several SoCs.
>>>>>> Socionext has some more product lines other than Uniphier SoC family,
>>>>>> perhaps wider re-use might happen in the future.
>>>>>>
>>>>>> At first, I included "uniphier" in compatible, but I am still wondering
>>>>>> if such a specific string is good or not.
>>>>>>
>>>>>> Also, comments from Altera engineers are appreciated.
>>>>
>>>> Sorry, it's taken me a while to add comments. My altera email is very spotty now
>>>> that the Intel merge is completed. Please use dinguyen@kernel.org for any future
>>>> communications.
>>>>
>>>> Yes, everything that is said so far for the NAND controller on the
>>>> SoCFPGA is correct. I added the binding for the controller a while
>>>> back, but unfortunately, we never added the NAND interface to the
>>>> devkit, so we did not do much in terms of enabling it.
>>>>
>>>> I think the only SoCFPGA board I know that has the NAND interface active is
>>>> the TRCom board, but I have never seen that board.
>>>>
>>>> I don't have any strong opinions on this matter, just as long as the
>>>> original binding
>>>> "denali,denali-nand-dt" is kept, and I think Rob was ok with keeping
>>>> that binding.
>>>>
>>>
>>> I am proposing to add "altera,denali-nand" for Altera.
>>> For what, do you need the generic compatible?
>>> This IP has no default for it to fallback to.
>>
>> IMO just for compatibility reasons with old DTs .
> 
> We generally contribute for
> a "working driver" (at least, should be functional to some extent)
> and "DT binding" bundled together.
> 
> However, Altera upstreamed the DT binding first
> (then some parts of the DT binding turned out wrong),
> but they did not upstream needed driver changes in the end.
> 
> So, the mainline driver has never worked on SOCFPGA, right?

Most likely it never worked, yes.

> Removing "denali,denali-nand-dt" is not breakage at all,
> so I do not owe anything to them, right?

I don't think I'm really qualified to answer this one. But, there is
drivers/mtd/nand/denali_dt.c , which handles this compatible string
and it's documented in
Documentation/devicetree/bindings/mtd/denali-nand.txt, so doesn't that
make it part of the ABI ? I think we should
at least keep it as a fallback, that should be pretty harmless.

-- 
Best regards,
Marek Vasut

[toc] | [prev] | [next] | [standalone]

#1536405 — Re: [PATCH 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants

On Sun, Dec 4, 2016 at 10:22 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> On 12/05/2016 05:10 AM, Masahiro Yamada wrote:
>> Hi Marek,
>>
>>
>> 2016-12-05 12:44 GMT+09:00 Marek Vasut <marek.vasut@gmail.com>:
>>> On 12/05/2016 04:30 AM, Masahiro Yamada wrote:
>>>> Hi Dinh,
>>>>
>>>>
>>>> 2016-12-04 7:08 GMT+09:00 Dinh Nguyen <dinh.linux@gmail.com>:
>>>>> Hi,
>>>>>
>>>>> On Fri, Dec 2, 2016 at 8:49 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>>> On 12/03/2016 03:41 AM, Masahiro Yamada wrote:
>>>>>>> Hi Rob,
>>>>>>
>>>>>> Hi!
>>>>>>
>>>>>>> 2016-12-03 1:26 GMT+09:00 Rob Herring <robh@kernel.org>:
>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> (Plan A)
>>>>>>>>>   "denali,socfpga-nand"           (for Altera SOCFPGA variant)
>>>>>>>>>   "denali,uniphier-nand-v1"       (for old Socionext UniPhier family variant)
>>>>>>>>>   "denali,uniphier-nand-v2"       (for new Socionext UniPhier family variant)
>>>>>>>>>
>>>>>>>>> (Plan B)
>>>>>>>>>   "altera,denali-nand"            (for Altera SOCFPGA variant)
>>>>>>>>>   "socionext,denali-nand-v5a"     (for old Socionext UniPhier family variant)
>>>>>>>>>   "socionext,denali-nand-v5b"     (for new Socionext UniPhier family variant)
>>>>>>>
>>>>>>>> Let the Altera folks worry about their stuff. At least for soft IP in
>>>>>>>> FPGA, it's a bit of a special case. The old string can remain as bad
>>>>>>>> as it is.
>>>>>>>
>>>>>>>
>>>>>>> Hmm, I am not sure if this IP would fit in FPGA
>>>>>>> (to use it along with NIOS-II?)
>>>>>>>
>>>>>>> (even if it happened, nothing of this IP would be customizable on users' side.
>>>>>>> When buying the IP, SoC vendors submit a list of desired features.
>>>>>>> Denali (now Cadence) generates the RTL according to the configuration sheet.
>>>>>>> The function is fixed at this point. So, generic compatible would be
>>>>>>> useless anyway.)
>>>>>>>
>>>>>>>
>>>>>>> If we are talking about SOCFPGA,
>>>>>>> SOCFPGA is not only FPGA. Rather "SOC" + "FPGA".
>>>>>>> It consists of two parts:
>>>>>>> [1] SOC part  (Cortex-A9 + various hard-wired peripherals such UART,
>>>>>>> USB, SD, NAND, ...)
>>>>>>> [2] FPGA part (User design logic)
>>>>>>>
>>>>>>> The Denali NAND controller is included in [1].
>>>>>>> So, as far as we talk about the Denali on SOCFPGA,
>>>>>>> it is as hard-wired as Intel, Socionext's ones.
>>>>>>
>>>>>> That's correct, the Denali NAND IP in altera socfpga is a hardware
>>>>>> block. You can make it available to the fabric too, but by default
>>>>>> it's used by the ARM part of the chip, so for this discussion, you
>>>>>> can forget that the FPGA part exists altogether.
>>>>>>
>>>>>> I would be in favor of plan B, since it seems to be the more often
>>>>>> taken approach. A nice example is ci-hdrc:
>>>>>>
>>>>>> $ git grep compatible drivers/usb/chipidea/
>>>>>>
>>>>>>>> I simply would do "socionext,uniphier-v5b-nand" (and v5a).
>>>>>>>> The fact that it is denali is part of the documentation.
>>>>>>>>
>>>>>>>
>>>>>>> Let me think about this.
>>>>>>>
>>>>>>> Socionext bought two version of Denali IP,
>>>>>>> and we are now re-using the newer one (v5b) for several SoCs.
>>>>>>> Socionext has some more product lines other than Uniphier SoC family,
>>>>>>> perhaps wider re-use might happen in the future.
>>>>>>>
>>>>>>> At first, I included "uniphier" in compatible, but I am still wondering
>>>>>>> if such a specific string is good or not.
>>>>>>>
>>>>>>> Also, comments from Altera engineers are appreciated.
>>>>>
>>>>> Sorry, it's taken me a while to add comments. My altera email is very spotty now
>>>>> that the Intel merge is completed. Please use dinguyen@kernel.org for any future
>>>>> communications.
>>>>>
>>>>> Yes, everything that is said so far for the NAND controller on the
>>>>> SoCFPGA is correct. I added the binding for the controller a while
>>>>> back, but unfortunately, we never added the NAND interface to the
>>>>> devkit, so we did not do much in terms of enabling it.
>>>>>
>>>>> I think the only SoCFPGA board I know that has the NAND interface active is
>>>>> the TRCom board, but I have never seen that board.
>>>>>
>>>>> I don't have any strong opinions on this matter, just as long as the
>>>>> original binding
>>>>> "denali,denali-nand-dt" is kept, and I think Rob was ok with keeping
>>>>> that binding.
>>>>>
>>>>
>>>> I am proposing to add "altera,denali-nand" for Altera.
>>>> For what, do you need the generic compatible?
>>>> This IP has no default for it to fallback to.
>>>
>>> IMO just for compatibility reasons with old DTs .
>>
>> We generally contribute for
>> a "working driver" (at least, should be functional to some extent)
>> and "DT binding" bundled together.
>>
>> However, Altera upstreamed the DT binding first
>> (then some parts of the DT binding turned out wrong),
>> but they did not upstream needed driver changes in the end.
>>
>> So, the mainline driver has never worked on SOCFPGA, right?
>
> Most likely it never worked, yes.
>

Right, looking through our downstream support, we may need to upstream a
few changes to make upstream driver work on SoCFPGA.

>> Removing "denali,denali-nand-dt" is not breakage at all,
>> so I do not owe anything to them, right?
>
> I don't think I'm really qualified to answer this one. But, there is
> drivers/mtd/nand/denali_dt.c , which handles this compatible string
> and it's documented in
> Documentation/devicetree/bindings/mtd/denali-nand.txt, so doesn't that
> make it part of the ABI ? I think we should
> at least keep it as a fallback, that should be pretty harmless.
>

I would like to propose "altr,denali-nand" as the binding we use to support the
driver going forward on SoCFPGA hardware. It's pretty much the same as
"altera,denali-nand", just with the correct vendor prefix.

If we can please keep, "denali,denali-nand-dt" only because SoCFPGA is using
this binding downstream, but I know that is a weak argument.

Dinh

[toc] | [prev] | [next] | [standalone]

Page 2 of 4 — ← Prev page 1 [2] 3 4  Next page →

Back to top | Article view | linux.kernel

csiph-web