devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v13 0/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC
@ 2020-09-17  0:33 Ramuthevar,Vadivel MuruganX
  2020-09-17  0:33 ` [PATCH v13 1/2] dt-bindings: mtd: Add Nand Flash Controller support for " Ramuthevar,Vadivel MuruganX
       [not found] ` <20200917003308.57038-3-vadivel.muruganx.ramuthevar@linux.intel.com>
  0 siblings, 2 replies; 4+ messages in thread
From: Ramuthevar,Vadivel MuruganX @ 2020-09-17  0:33 UTC (permalink / raw)
  To: miquel.raynal, linux-kernel
  Cc: linux-mtd, richard, vigneshr, boris.brezillon,
	christophe.kerello, piotrs, robert.jarzmik, brendanhiggins,
	devicetree, tglx, hauke.mehrtens, robh+dt, linux-mips, arnd,
	andriy.shevchenko, cheol.yong.kim, qi-ming.wu,
	Ramuthevar,Vadivel MuruganX

This patch adds the new IP of Nand Flash Controller(NFC) support
on Intel's Lightning Mountain(LGM) SoC.

DMA is used for burst data transfer operation, also DMA HW supports
aligned 32bit memory address and aligned data access by default.
DMA burst of 8 supported. Data register used to support the read/write
operation from/to device.

NAND controller also supports in-built HW ECC engine.

NAND controller driver implements ->exec_op() to replace legacy hooks,
these specific call-back method to execute NAND operations.

Thanks Miquel, Boris, Andy, Arnd and Rob for the review comments and suggestions.
---
v13:
  - Address Miquel Raynal review comments
  - update the return type with variable 'ret'
  - handle err check statement properly
  - change the naming convention aligned with recently changed the naming 
    around the data interface
    data structure and function names
  - replace by div 8 instead of <<4 in ecc calculation better code readability
  - handle check_only properly like existing drivers
v12-resend:
  - No Change
v12:
  - address Miquel Raynal review comments update
  - add/modify the comments for better understanding
  - handle the check_only variable
  - update the ecc function based on the existing drivers
  - add newline
  - verify that mtd->name is set after nand_set_flash_node()
  - add the check WARN_ON(ret);
v11-resend:
  - Rebase to v5.8-rc1
v11:
  - No Change
v10:
  - No Change
v9:
  - No change
v8:
  - fix the kbuild bot warnings
  - correct the typo's
v7:
  - indentation issue is fixed
  - add error check for retrieve the resource from dt
v6:
  - update EBU_ADDR_SELx register base value build it from DT
  - Add tabs in in Kconfig
v5:
  - replace by 'HSNAND_CLE_OFFS | HSNAND_CS_OFFS' to NAND_WRITE_CMD and NAND_WRITE_ADDR
  - remove the unused macros
  - update EBU_ADDR_MASK(x) macro
  - update the EBU_ADDR_SELx register values to be written
v4:
  - add ebu_nand_cs structure for multiple-CS support
  - mask/offset encoding for 0x51 value
  - update macro HSNAND_CTL_ENABLE_ECC
  - drop the op argument and un-used macros.
  - updated the datatype and macros
  - add function disable nand module
  - remove ebu_host->dma_rx = NULL;
  - rename MMIO address range variables to ebu and hsnand
  - implement ->setup_data_interface()
  - update label err_cleanup_nand and err_cleanup_dma
  - add return value check in the nand_remove function
  - add/remove tabs and spaces as per coding standard
  - encoded CS ids by reg property
v3:
  - Add depends on MACRO in Kconfig
  - file name update in Makefile
  - file name update to intel-nand-controller
  - modification of MACRO divided like EBU, HSNAND and NAND
  - add NAND_ALE_OFFS, NAND_CLE_OFFS and NAND_CS_OFFS
  - rename lgm_ to ebu_ and _va suffix is removed in the whole file
  - rename structure and varaibles as per review comments.
  - remove lgm_read_byte(), lgm_dev_ready() and cmd_ctrl() un-used function
  - update in exec_op() as per review comments
  - rename function lgm_dma_exit() by lgm_dma_cleanup()
  - hardcoded magic value  for base and offset replaced by MACRO defined
  - mtd_device_unregister() + nand_cleanup() instead of nand_release()
v2:
  - implement the ->exec_op() to replaces the legacy hook-up.
  - update the commit message
  - add MIPS maintainers and xway_nand driver author in CC
v1:
 - initial version


dt-bindings: mtd: Add Nand Flash Controller support for Intel LGM SoC
---
v13:
  - No chnage
v12-Resend:
  - No Change
v12:
  - No change
v11-resend:
  - No change
v11:
  - Fixed the compatible issue with example
10:
  - fix bot errors
v9:
  - Rob's review comments address
  - dual licensed
  - compatible change
  - add reg-names
  - drop clock-names and clock-cells
  - correct typo's
v8:
  No change
v7:
  - Rob's review comments addressed
  - dt-schema build issue fixed with upgraded dt-schema
v6:
  - Rob's review comments addressed in YAML file
  - add addr_sel0 and addr_sel1 reg-names in YAML example
v5:
  - add the example in YAML file
v4:
  - No change
v3:
  - No change
v2:
  YAML compatible string update to intel, lgm-nand-controller
v1:
  - initial version

Ramuthevar Vadivel Murugan (2):
  dt-bindings: mtd: Add Nand Flash Controller support for Intel LGM SoC
  mtd: rawnand: Add NAND controller support on Intel LGM SoC

 .../devicetree/bindings/mtd/intel,lgm-nand.yaml    |  99 +++
 drivers/mtd/nand/raw/Kconfig                       |   8 +
 drivers/mtd/nand/raw/Makefile                      |   1 +
 drivers/mtd/nand/raw/intel-nand-controller.c       | 760 +++++++++++++++++++++
 4 files changed, 868 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
 create mode 100644 drivers/mtd/nand/raw/intel-nand-controller.c

-- 
2.11.0


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

* [PATCH v13 1/2] dt-bindings: mtd: Add Nand Flash Controller support for Intel LGM SoC
  2020-09-17  0:33 [PATCH v13 0/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC Ramuthevar,Vadivel MuruganX
@ 2020-09-17  0:33 ` Ramuthevar,Vadivel MuruganX
       [not found] ` <20200917003308.57038-3-vadivel.muruganx.ramuthevar@linux.intel.com>
  1 sibling, 0 replies; 4+ messages in thread
From: Ramuthevar,Vadivel MuruganX @ 2020-09-17  0:33 UTC (permalink / raw)
  To: miquel.raynal, linux-kernel
  Cc: linux-mtd, richard, vigneshr, boris.brezillon,
	christophe.kerello, piotrs, robert.jarzmik, brendanhiggins,
	devicetree, tglx, hauke.mehrtens, robh+dt, linux-mips, arnd,
	andriy.shevchenko, cheol.yong.kim, qi-ming.wu,
	Ramuthevar Vadivel Murugan

From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>

Add YAML file for dt-bindings to support NAND Flash Controller
on Intel's Lightning Mountain SoC.

Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/mtd/intel,lgm-nand.yaml    | 99 ++++++++++++++++++++++
 1 file changed, 99 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml

diff --git a/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml b/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
new file mode 100644
index 000000000000..313daec4d783
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
@@ -0,0 +1,99 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/intel,lgm-nand.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intel LGM SoC NAND Controller Device Tree Bindings
+
+allOf:
+  - $ref: "nand-controller.yaml"
+
+maintainers:
+  - Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
+
+properties:
+  compatible:
+    const: intel,lgm-nand
+
+  reg:
+    maxItems: 6
+
+  reg-names:
+    items:
+       - const: ebunand
+       - const: hsnand
+       - const: nand_cs0
+       - const: nand_cs1
+       - const: addr_sel0
+       - const: addr_sel1
+
+  clocks:
+    maxItems: 1
+
+  dmas:
+    maxItems: 2
+
+  dma-names:
+    items:
+      - const: tx
+      - const: rx
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  "^nand@[a-f0-9]+$":
+    type: object
+    properties:
+      reg:
+        minimum: 0
+        maximum: 7
+
+      nand-ecc-mode: true
+
+      nand-ecc-algo:
+        const: hw
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - clocks
+  - dmas
+  - dma-names
+  - "#address-cells"
+  - "#size-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    nand-controller@e0f00000 {
+      compatible = "intel,lgm-nand";
+      reg = <0xe0f00000 0x100>,
+            <0xe1000000 0x300>,
+            <0xe1400000 0x8000>,
+            <0xe1c00000 0x1000>,
+            <0x17400000 0x4>,
+            <0x17c00000 0x4>;
+      reg-names = "ebunand", "hsnand", "nand_cs0", "nand_cs1",
+        "addr_sel0", "addr_sel1";
+      clocks = <&cgu0 125>;
+      dmas = <&dma0 8>, <&dma0 9>;
+      dma-names = "tx", "rx";
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      nand@0 {
+        reg = <0>;
+        nand-ecc-mode = "hw";
+      };
+    };
+
+...
-- 
2.11.0


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

* Re: [PATCH v13 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC
       [not found] ` <20200917003308.57038-3-vadivel.muruganx.ramuthevar@linux.intel.com>
@ 2020-09-17 13:05   ` Andy Shevchenko
  2020-09-18  6:57     ` Ramuthevar, Vadivel MuruganX
  0 siblings, 1 reply; 4+ messages in thread
From: Andy Shevchenko @ 2020-09-17 13:05 UTC (permalink / raw)
  To: Ramuthevar,Vadivel MuruganX
  Cc: miquel.raynal, linux-kernel, linux-mtd, richard, vigneshr,
	boris.brezillon, christophe.kerello, piotrs, robert.jarzmik,
	brendanhiggins, devicetree, tglx, hauke.mehrtens, robh+dt,
	linux-mips, arnd, cheol.yong.kim, qi-ming.wu

On Thu, Sep 17, 2020 at 08:33:08AM +0800, Ramuthevar,Vadivel MuruganX wrote:
> From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
> 
> This patch adds the new IP of Nand Flash Controller(NFC) support
> on Intel's Lightning Mountain(LGM) SoC.
> 
> DMA is used for burst data transfer operation, also DMA HW supports
> aligned 32bit memory address and aligned data access by default.
> DMA burst of 8 supported. Data register used to support the read/write
> operation from/to device.
> 
> NAND controller driver implements ->exec_op() to replace legacy hooks,
> these specific call-back method to execute NAND operations.

...

> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-direction.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/err.h>
> +#include <linux/init.h>

> +#include <linux/io.h>
> +#include <linux/iopoll.h>

io.h is guaranteed to be included by iopoll.h.

> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/module.h>

> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/rawnand.h>
> +#include <linux/mtd/nand_ecc.h>
> +#include <linux/mtd/nand.h>

Since mtd is a hosting framework for this driver, I would move this group of headers after more generic ones with a blank line in between.


> +#include <linux/resource.h>

And this I think is guaranteed to be included by io.h.

> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>

> +#include <linux/platform_device.h>

Dup? It's exactly the reason how alphabetical order can help.

...

> +#define EBU_ADDR_SEL(n)		(0x20 + (n) * 4)

I think 0x20 is an offset here, and better to have it as 0x020 to be consistent
with all other offsets.

...

> +#define EBU_BUSCON(n)		(0x60 + (n) * 4)

Ditto.

...

> +static void ebu_nand_setup_timing(struct ebu_nand_controller *ctrl,
> +				  const struct nand_sdr_timings *timings)
> +{
> +	unsigned int rate = clk_get_rate(ctrl->clk) / 1000000;

HZ_PER_MHZ?

> +	unsigned int period = DIV_ROUND_UP(1000000, rate);

USEC_PER_SEC?

> +	u32 trecov, thold, twrwait, trdwait;
> +	u32 reg = 0;
> +
> +	trecov = DIV_ROUND_UP(max(timings->tREA_max, timings->tREH_min),
> +			      period);
> +	reg |= EBU_BUSCON_RECOVC(trecov);
> +
> +	thold = DIV_ROUND_UP(max(timings->tDH_min, timings->tDS_min), period);
> +	reg |= EBU_BUSCON_HOLDC(thold);
> +
> +	trdwait = DIV_ROUND_UP(max(timings->tRC_min, timings->tREH_min),
> +			       period);
> +	reg |= EBU_BUSCON_WAITRDC(trdwait);
> +
> +	twrwait = DIV_ROUND_UP(max(timings->tWC_min, timings->tWH_min), period);
> +	reg |= EBU_BUSCON_WAITWRC(twrwait);
> +
> +	reg |= EBU_BUSCON_CMULT_V4 | EBU_BUSCON_BCGEN_CS | EBU_BUSCON_ALEC |
> +		EBU_BUSCON_SETUP_EN;
> +
> +	writel(reg, ctrl->ebu + EBU_BUSCON(ctrl->cs_num));
> +}

...

> +	if (oob_required) {
> +		reg = (chip->oob_poi[3] << 24) | (chip->oob_poi[2] << 16) |
> +			(chip->oob_poi[1] << 8) | chip->oob_poi[0];

get_unligned_le32()?

...

> +		reg = (chip->oob_poi[7] << 24) | (chip->oob_poi[6] << 16) |
> +			(chip->oob_poi[5] << 8) | chip->oob_poi[4];

Ditto.

...

> +	ret = readl_poll_timeout_atomic(int_sta, val,
> +					!(val & HSNAND_INT_STA_WR_C), 10, 1000);

Slightly better (logically split between lines):

	ret = readl_poll_timeout_atomic(int_sta, val, !(val & HSNAND_INT_STA_WR_C),
					10, 1000);


> +	if (ret)
> +		return ret;

...

> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ebunand");
> +	ebu_host->ebu = devm_ioremap_resource(&pdev->dev, res);

Why not to use

	ebu_host->ebu = devm_platform_ioremap_resource_byname(&pdev->dev, "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);

Ditto.

> +	if (IS_ERR(ebu_host->hsnand))
> +		return PTR_ERR(ebu_host->hsnand);

...


> +	for (i = 0; i < MAX_CS; i++) {
> +		resname = devm_kasprintf(dev, GFP_KERNEL, "nand_cs%d", i);
> +		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> +						   resname);

> +		if (!res)
> +			return -EINVAL;

Redundant check.

> +		ebu_host->cs[i].chipaddr = devm_ioremap_resource(dev, res);

Ditto (see above).

> +		ebu_host->cs[i].nand_pa = res->start;
> +		if (IS_ERR(ebu_host->cs[i].chipaddr))
> +			return PTR_ERR(ebu_host->cs[i].chipaddr);
> +	}

...

> +	ebu_host->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(ebu_host->clk)) {

> +		ret = PTR_ERR(ebu_host->clk);
> +		dev_err(dev, "failed to get clock: %d\n", ret);
> +		return ret;

	return dev_err_probe() ?

> +	}

...

> +	ebu_host->dma_tx = dma_request_chan(dev, "tx");
> +	if (IS_ERR(ebu_host->dma_tx)) {

> +		ret = PTR_ERR(ebu_host->dma_tx);
> +		dev_err(dev, "DMA tx channel request fail!.\n");
> +		goto err_cleanup_dma;

Ditto. On top why !. ???

> +	}
> +
> +	ebu_host->dma_rx = dma_request_chan(dev, "rx");
> +	if (IS_ERR(ebu_host->dma_rx)) {

> +		ret = PTR_ERR(ebu_host->dma_rx);
> +		dev_err(dev, "DMA rx channel request fail!.\n");
> +		goto err_cleanup_dma;

Ditto.

> +	}

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v13 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC
  2020-09-17 13:05   ` [PATCH v13 2/2] mtd: rawnand: Add NAND controller support on " Andy Shevchenko
@ 2020-09-18  6:57     ` Ramuthevar, Vadivel MuruganX
  0 siblings, 0 replies; 4+ messages in thread
From: Ramuthevar, Vadivel MuruganX @ 2020-09-18  6:57 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: miquel.raynal, linux-kernel, linux-mtd, richard, vigneshr,
	boris.brezillon, christophe.kerello, piotrs, robert.jarzmik,
	brendanhiggins, devicetree, tglx, hauke.mehrtens, robh+dt,
	linux-mips, arnd, cheol.yong.kim, qi-ming.wu

Hi Andy,

  Thank you for the review comments...

On 17/9/2020 9:05 pm, Andy Shevchenko wrote:
> On Thu, Sep 17, 2020 at 08:33:08AM +0800, Ramuthevar,Vadivel MuruganX wrote:
>> From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
>>
>> This patch adds the new IP of Nand Flash Controller(NFC) support
>> on Intel's Lightning Mountain(LGM) SoC.
>>
>> DMA is used for burst data transfer operation, also DMA HW supports
>> aligned 32bit memory address and aligned data access by default.
>> DMA burst of 8 supported. Data register used to support the read/write
>> operation from/to device.
>>
>> NAND controller driver implements ->exec_op() to replace legacy hooks,
>> these specific call-back method to execute NAND operations.
> 
> ...
> 
>> +#include <linux/clk.h>
>> +#include <linux/completion.h>
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-direction.h>
>> +#include <linux/dma-mapping.h>
>> +#include <linux/err.h>
>> +#include <linux/init.h>
> 
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
> 
> io.h is guaranteed to be included by iopoll.h.
Noted
> 
>> +#include <linux/kernel.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/module.h>
> 
>> +#include <linux/mtd/mtd.h>
>> +#include <linux/mtd/rawnand.h>
>> +#include <linux/mtd/nand_ecc.h>
>> +#include <linux/mtd/nand.h>
> 
> Since mtd is a hosting framework for this driver, I would move this group of headers after more generic ones with a blank line in between.
okay, noted
> 
> 
>> +#include <linux/resource.h>
> 
> And this I think is guaranteed to be included by io.h.
Sure, will check and update.
> 
>> +#include <linux/sched.h>
>> +#include <linux/slab.h>
>> +#include <linux/types.h>
> 
>> +#include <linux/platform_device.h>
> 
> Dup? It's exactly the reason how alphabetical order can help.
Good catch
> 
> ...
> 
>> +#define EBU_ADDR_SEL(n)		(0x20 + (n) * 4)
> 
> I think 0x20 is an offset here, and better to have it as 0x020 to be consistent
> with all other offsets.
> 
> ...
> 
>> +#define EBU_BUSCON(n)		(0x60 + (n) * 4)
> 
> Ditto.
Noted, will update
> 
> ...
> 
>> +static void ebu_nand_setup_timing(struct ebu_nand_controller *ctrl,
>> +				  const struct nand_sdr_timings *timings)
>> +{
>> +	unsigned int rate = clk_get_rate(ctrl->clk) / 1000000;
> 
> HZ_PER_MHZ?
yes, you're right we can use it.
> 
>> +	unsigned int period = DIV_ROUND_UP(1000000, rate);
> 
> USEC_PER_SEC?
yes, you're right we can use it.
> 
>> +	u32 trecov, thold, twrwait, trdwait;
>> +	u32 reg = 0;
>> +
>> +	trecov = DIV_ROUND_UP(max(timings->tREA_max, timings->tREH_min),
>> +			      period);
>> +	reg |= EBU_BUSCON_RECOVC(trecov);
>> +
>> +	thold = DIV_ROUND_UP(max(timings->tDH_min, timings->tDS_min), period);
>> +	reg |= EBU_BUSCON_HOLDC(thold);
>> +
>> +	trdwait = DIV_ROUND_UP(max(timings->tRC_min, timings->tREH_min),
>> +			       period);
>> +	reg |= EBU_BUSCON_WAITRDC(trdwait);
>> +
>> +	twrwait = DIV_ROUND_UP(max(timings->tWC_min, timings->tWH_min), period);
>> +	reg |= EBU_BUSCON_WAITWRC(twrwait);
>> +
>> +	reg |= EBU_BUSCON_CMULT_V4 | EBU_BUSCON_BCGEN_CS | EBU_BUSCON_ALEC |
>> +		EBU_BUSCON_SETUP_EN;
>> +
>> +	writel(reg, ctrl->ebu + EBU_BUSCON(ctrl->cs_num));
>> +}
> 
> ...
> 
>> +	if (oob_required) {
>> +		reg = (chip->oob_poi[3] << 24) | (chip->oob_poi[2] << 16) |
>> +			(chip->oob_poi[1] << 8) | chip->oob_poi[0];
> 
> get_unligned_le32()?
last time seen system crash ,so I left it.
> 
> ...
> 
>> +		reg = (chip->oob_poi[7] << 24) | (chip->oob_poi[6] << 16) |
>> +			(chip->oob_poi[5] << 8) | chip->oob_poi[4];
> 
> Ditto.
Let me double check will add it, keep the same if not
> 
> ...
> 
>> +	ret = readl_poll_timeout_atomic(int_sta, val,
>> +					!(val & HSNAND_INT_STA_WR_C), 10, 1000);
> 
> Slightly better (logically split between lines):
> 
> 	ret = readl_poll_timeout_atomic(int_sta, val, !(val & HSNAND_INT_STA_WR_C),
> 					10, 1000);
Thanks!, will update
> 
> 
>> +	if (ret)
>> +		return ret;
> 
> ...
> 
>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ebunand");
>> +	ebu_host->ebu = devm_ioremap_resource(&pdev->dev, res);
> 
> Why not to use
> 
> 	ebu_host->ebu = devm_platform_ioremap_resource_byname(&pdev->dev, "ebunand");
As Boris mtd-maintainer suggested me to split into 2 API's, thanks!.
> 
> ?
> 
>> +	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);
> 
> Ditto.
> 
>> +	if (IS_ERR(ebu_host->hsnand))
>> +		return PTR_ERR(ebu_host->hsnand);
> 
> ...
> 
> 
>> +	for (i = 0; i < MAX_CS; i++) {
>> +		resname = devm_kasprintf(dev, GFP_KERNEL, "nand_cs%d", i);
>> +		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>> +						   resname);
> 
>> +		if (!res)
>> +			return -EINVAL;
> 
> Redundant check.
> 
>> +		ebu_host->cs[i].chipaddr = devm_ioremap_resource(dev, res);
> 
> Ditto (see above).
Noted
> 
>> +		ebu_host->cs[i].nand_pa = res->start;
>> +		if (IS_ERR(ebu_host->cs[i].chipaddr))
>> +			return PTR_ERR(ebu_host->cs[i].chipaddr);
>> +	}
> 
> ...
> 
>> +	ebu_host->clk = devm_clk_get(dev, NULL);
>> +	if (IS_ERR(ebu_host->clk)) {
> 
>> +		ret = PTR_ERR(ebu_host->clk);
>> +		dev_err(dev, "failed to get clock: %d\n", ret);
>> +		return ret;
> 
> 	return dev_err_probe() ?
Noted, will add it.
> 
>> +	}
> 
> ...
> 
>> +	ebu_host->dma_tx = dma_request_chan(dev, "tx");
>> +	if (IS_ERR(ebu_host->dma_tx)) {
> 
>> +		ret = PTR_ERR(ebu_host->dma_tx);
>> +		dev_err(dev, "DMA tx channel request fail!.\n");
>> +		goto err_cleanup_dma;
> 
> Ditto. On top why !. ???
sorry missed it,Noted
> 
>> +	}
>> +
>> +	ebu_host->dma_rx = dma_request_chan(dev, "rx");
>> +	if (IS_ERR(ebu_host->dma_rx)) {
> 
>> +		ret = PTR_ERR(ebu_host->dma_rx);
>> +		dev_err(dev, "DMA rx channel request fail!.\n");
>> +		goto err_cleanup_dma;
> 
> Ditto.
Noted

Regards
Vadivel
> 
>> +	}
> 

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

end of thread, other threads:[~2020-09-18  6:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-17  0:33 [PATCH v13 0/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC Ramuthevar,Vadivel MuruganX
2020-09-17  0:33 ` [PATCH v13 1/2] dt-bindings: mtd: Add Nand Flash Controller support for " Ramuthevar,Vadivel MuruganX
     [not found] ` <20200917003308.57038-3-vadivel.muruganx.ramuthevar@linux.intel.com>
2020-09-17 13:05   ` [PATCH v13 2/2] mtd: rawnand: Add NAND controller support on " Andy Shevchenko
2020-09-18  6:57     ` Ramuthevar, Vadivel MuruganX

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