[PATCH v3 0/2] refine the NFC clock framework

[PATCH v3 0/2] refine the NFC clock framework

[Liang Yang]

Background firstly, 
Both EMMC and NAND have the same clock control register named 'SD_EMMC_CLOCK' which is
defined in EMMC port internally. bit0~5 of 'SD_EMMC_CLOCK' is the divider and
bit6~7 is the mux for fix pll and xtal.
Previously a common MMC sub clock framework is implemented and shared by EMMC and
NAND, but that is coupling the EMMC and NAND, although EMMC and NAND is mutually
exclusive. see the link:
[https://lore.kernel.org/all/1jy23226sa.fsf@starbuckisacylon.baylibre.com/]
Now we plan to abandon common mmc sub clock framework and recovery the series.

Changes since v2 [3]
 - use fw_name from dts, instead the wrong way using __clk_get_name
 - reg resource size change to 0x800
 - use reg-names

Changes since v1 [2]
 - use clk_parent_data instead of parent_names
 - define a reg resource instead of sd_emmc_c_clkc 

[1] https://lore.kernel.org/r/20220106033130.37623-1-liang.yang@amlogic.com
    https://lore.kernel.org/r/20220106032504.23310-1-liang.yang@amlogic.com
[2] https://lore.kernel.org/all/20220217063346.21691-1-liang.yang@amlogic.com

Liang Yang (2):
  mtd: rawnand: meson: discard the common MMC sub clock framework
  dt-bindings: nand: meson: refine Amlogic NAND controller driver

 .../bindings/mtd/amlogic,meson-nand.txt       |  60 -----------
 .../bindings/mtd/amlogic,meson-nand.yaml      |  71 ++++++++++++
 drivers/mtd/nand/raw/meson_nand.c             | 101 +++++++++---------
 3 files changed, 124 insertions(+), 108 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml

-- 
2.34.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v3 1/2] mtd: rawnand: meson: discard the common MMC sub clock framework

[Liang Yang]

EMMC and NAND have the same clock control register named 'SD_EMMC_CLOCK' which is
defined in EMMC port internally. bit0~5 of 'SD_EMMC_CLOCK' is the divider and
bit6~7 is the mux for fix pll and xtal.A common MMC and NAND sub-clock has been
implemented and can be used by the eMMC and NAND controller (which are mutually
exclusive anyway). Let's use this new clock.

Signed-off-by: Liang Yang <liang.yang@amlogic.com>
---
 drivers/mtd/nand/raw/meson_nand.c | 101 ++++++++++++++++--------------
 1 file changed, 53 insertions(+), 48 deletions(-)

diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
index ac3be92872d0..257ffc8a41ab 100644
--- a/drivers/mtd/nand/raw/meson_nand.c
+++ b/drivers/mtd/nand/raw/meson_nand.c
@@ -10,6 +10,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/interrupt.h>
 #include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/mtd/rawnand.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mfd/syscon.h>
@@ -19,6 +20,7 @@
 #include <linux/iopoll.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_address.h>
 #include <linux/sched/task_stack.h>
 
 #define NFC_REG_CMD		0x00
@@ -104,6 +106,9 @@
 
 #define PER_INFO_BYTE		8
 
+#define CLK_DIV_SHIFT		0
+#define CLK_DIV_WIDTH		6
+
 struct meson_nfc_nand_chip {
 	struct list_head node;
 	struct nand_chip nand;
@@ -151,15 +156,15 @@ struct meson_nfc {
 	struct nand_controller controller;
 	struct clk *core_clk;
 	struct clk *device_clk;
-	struct clk *phase_tx;
-	struct clk *phase_rx;
+	struct clk *nand_clk;
+	struct clk_divider nand_divider;
 
 	unsigned long clk_rate;
 	u32 bus_timing;
 
 	struct device *dev;
 	void __iomem *reg_base;
-	struct regmap *reg_clk;
+	void __iomem *sd_emmc_clock;
 	struct completion completion;
 	struct list_head chips;
 	const struct meson_nfc_data *data;
@@ -235,7 +240,7 @@ static void meson_nfc_select_chip(struct nand_chip *nand, int chip)
 	nfc->timing.tbers_max = meson_chip->tbers_max;
 
 	if (nfc->clk_rate != meson_chip->clk_rate) {
-		ret = clk_set_rate(nfc->device_clk, meson_chip->clk_rate);
+		ret = clk_set_rate(nfc->nand_clk, meson_chip->clk_rate);
 		if (ret) {
 			dev_err(nfc->dev, "failed to set clock rate\n");
 			return;
@@ -406,7 +411,6 @@ static int meson_nfc_queue_rb(struct meson_nfc *nfc, int timeout_ms)
 	cmd = NFC_CMD_RB | NFC_CMD_RB_INT
 		| nfc->param.chip_select | nfc->timing.tbers_max;
 	writel(cmd, nfc->reg_base + NFC_REG_CMD);
-
 	ret = wait_for_completion_timeout(&nfc->completion,
 					  msecs_to_jiffies(timeout_ms));
 	if (ret == 0)
@@ -985,9 +989,11 @@ static const struct mtd_ooblayout_ops meson_ooblayout_ops = {
 	.free = meson_ooblayout_free,
 };
 
+struct clk_parent_data nfc_divider_parent_data[1];
 static int meson_nfc_clk_init(struct meson_nfc *nfc)
 {
 	int ret;
+	struct clk_init_data init = {0};
 
 	/* request core clock */
 	nfc->core_clk = devm_clk_get(nfc->dev, "core");
@@ -1002,21 +1008,26 @@ static int meson_nfc_clk_init(struct meson_nfc *nfc)
 		return PTR_ERR(nfc->device_clk);
 	}
 
-	nfc->phase_tx = devm_clk_get(nfc->dev, "tx");
-	if (IS_ERR(nfc->phase_tx)) {
-		dev_err(nfc->dev, "failed to get TX clk\n");
-		return PTR_ERR(nfc->phase_tx);
-	}
-
-	nfc->phase_rx = devm_clk_get(nfc->dev, "rx");
-	if (IS_ERR(nfc->phase_rx)) {
-		dev_err(nfc->dev, "failed to get RX clk\n");
-		return PTR_ERR(nfc->phase_rx);
-	}
+	init.name = devm_kstrdup(nfc->dev, "nfc#div", GFP_KERNEL);
+	init.ops = &clk_divider_ops;
+	nfc_divider_parent_data[0].fw_name = "device";
+	init.parent_data = nfc_divider_parent_data;
+	init.num_parents = 1;
+	nfc->nand_divider.reg = nfc->sd_emmc_clock;
+	nfc->nand_divider.shift = CLK_DIV_SHIFT;
+	nfc->nand_divider.width = CLK_DIV_WIDTH;
+	nfc->nand_divider.hw.init = &init;
+	nfc->nand_divider.flags = CLK_DIVIDER_ONE_BASED |
+				  CLK_DIVIDER_ROUND_CLOSEST |
+				  CLK_DIVIDER_ALLOW_ZERO;
+
+	nfc->nand_clk = devm_clk_register(nfc->dev, &nfc->nand_divider.hw);
+	if (IS_ERR(nfc->nand_clk))
+		return PTR_ERR(nfc->nand_clk);
 
 	/* init SD_EMMC_CLOCK to sane defaults w/min clock rate */
-	regmap_update_bits(nfc->reg_clk,
-			   0, CLK_SELECT_NAND, CLK_SELECT_NAND);
+	writel(CLK_SELECT_NAND | readl(nfc->sd_emmc_clock),
+	       nfc->sd_emmc_clock);
 
 	ret = clk_prepare_enable(nfc->core_clk);
 	if (ret) {
@@ -1030,29 +1041,21 @@ static int meson_nfc_clk_init(struct meson_nfc *nfc)
 		goto err_device_clk;
 	}
 
-	ret = clk_prepare_enable(nfc->phase_tx);
-	if (ret) {
-		dev_err(nfc->dev, "failed to enable TX clock\n");
-		goto err_phase_tx;
-	}
-
-	ret = clk_prepare_enable(nfc->phase_rx);
+	ret = clk_prepare_enable(nfc->nand_clk);
 	if (ret) {
-		dev_err(nfc->dev, "failed to enable RX clock\n");
-		goto err_phase_rx;
+		dev_err(nfc->dev, "pre enable NFC divider fail\n");
+		goto err_nand_clk;
 	}
 
-	ret = clk_set_rate(nfc->device_clk, 24000000);
+	ret = clk_set_rate(nfc->nand_clk, 24000000);
 	if (ret)
-		goto err_disable_rx;
+		goto err_disable_clk;
 
 	return 0;
 
-err_disable_rx:
-	clk_disable_unprepare(nfc->phase_rx);
-err_phase_rx:
-	clk_disable_unprepare(nfc->phase_tx);
-err_phase_tx:
+err_disable_clk:
+	clk_disable_unprepare(nfc->nand_clk);
+err_nand_clk:
 	clk_disable_unprepare(nfc->device_clk);
 err_device_clk:
 	clk_disable_unprepare(nfc->core_clk);
@@ -1061,8 +1064,7 @@ static int meson_nfc_clk_init(struct meson_nfc *nfc)
 
 static void meson_nfc_disable_clk(struct meson_nfc *nfc)
 {
-	clk_disable_unprepare(nfc->phase_rx);
-	clk_disable_unprepare(nfc->phase_tx);
+	clk_disable_unprepare(nfc->nand_clk);
 	clk_disable_unprepare(nfc->device_clk);
 	clk_disable_unprepare(nfc->core_clk);
 }
@@ -1373,9 +1375,10 @@ MODULE_DEVICE_TABLE(of, meson_nfc_id_table);
 static int meson_nfc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
 	struct meson_nfc *nfc;
-	struct resource *res;
-	int ret, irq;
+	struct resource res;
+	int ret, irq, i;
 
 	nfc = devm_kzalloc(dev, sizeof(*nfc), GFP_KERNEL);
 	if (!nfc)
@@ -1388,21 +1391,23 @@ static int meson_nfc_probe(struct platform_device *pdev)
 	nand_controller_init(&nfc->controller);
 	INIT_LIST_HEAD(&nfc->chips);
 	init_completion(&nfc->completion);
-
 	nfc->dev = dev;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	nfc->reg_base = devm_ioremap_resource(dev, res);
+	i = of_property_match_string(node, "reg-names", "nfc");
+	if (of_address_to_resource(node, i, &res))
+		return -ENOENT;
+
+	nfc->reg_base = devm_ioremap_resource(dev, &res);
 	if (IS_ERR(nfc->reg_base))
 		return PTR_ERR(nfc->reg_base);
 
-	nfc->reg_clk =
-		syscon_regmap_lookup_by_phandle(dev->of_node,
-						"amlogic,mmc-syscon");
-	if (IS_ERR(nfc->reg_clk)) {
-		dev_err(dev, "Failed to lookup clock base\n");
-		return PTR_ERR(nfc->reg_clk);
-	}
+	i = of_property_match_string(node, "reg-names", "emmc");
+	if (of_address_to_resource(node, i, &res))
+		return -ENOENT;
+
+	nfc->sd_emmc_clock = devm_ioremap_resource(dev, &res);
+	if (IS_ERR(nfc->sd_emmc_clock))
+		return PTR_ERR(nfc->sd_emmc_clock);
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
-- 
2.34.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 1/2] mtd: rawnand: meson: discard the common MMC sub clock framework

[Neil Armstrong]

Hi,

On 18/03/2022 13:41, Liang Yang wrote:
> EMMC and NAND have the same clock control register named 'SD_EMMC_CLOCK' which is
> defined in EMMC port internally. bit0~5 of 'SD_EMMC_CLOCK' is the divider and
> bit6~7 is the mux for fix pll and xtal.A common MMC and NAND sub-clock has been
> implemented and can be used by the eMMC and NAND controller (which are mutually
> exclusive anyway). Let's use this new clock.
> 
> Signed-off-by: Liang Yang <liang.yang@amlogic.com>
> ---
>   drivers/mtd/nand/raw/meson_nand.c | 101 ++++++++++++++++--------------
>   1 file changed, 53 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
> index ac3be92872d0..257ffc8a41ab 100644
> --- a/drivers/mtd/nand/raw/meson_nand.c
> +++ b/drivers/mtd/nand/raw/meson_nand.c
> @@ -10,6 +10,7 @@
>   #include <linux/dma-mapping.h>
>   #include <linux/interrupt.h>
>   #include <linux/clk.h>
> +#include <linux/clk-provider.h>
>   #include <linux/mtd/rawnand.h>
>   #include <linux/mtd/mtd.h>
>   #include <linux/mfd/syscon.h>
> @@ -19,6 +20,7 @@
>   #include <linux/iopoll.h>
>   #include <linux/of.h>
>   #include <linux/of_device.h>
> +#include <linux/of_address.h>
>   #include <linux/sched/task_stack.h>
>   
>   #define NFC_REG_CMD		0x00
> @@ -104,6 +106,9 @@
>   
>   #define PER_INFO_BYTE		8
>   
> +#define CLK_DIV_SHIFT		0
> +#define CLK_DIV_WIDTH		6
> +
>   struct meson_nfc_nand_chip {
>   	struct list_head node;
>   	struct nand_chip nand;
> @@ -151,15 +156,15 @@ struct meson_nfc {
>   	struct nand_controller controller;
>   	struct clk *core_clk;
>   	struct clk *device_clk;
> -	struct clk *phase_tx;
> -	struct clk *phase_rx;
> +	struct clk *nand_clk;
> +	struct clk_divider nand_divider;
>   
>   	unsigned long clk_rate;
>   	u32 bus_timing;
>   
>   	struct device *dev;
>   	void __iomem *reg_base;
> -	struct regmap *reg_clk;
> +	void __iomem *sd_emmc_clock;
>   	struct completion completion;
>   	struct list_head chips;
>   	const struct meson_nfc_data *data;
> @@ -235,7 +240,7 @@ static void meson_nfc_select_chip(struct nand_chip *nand, int chip)
>   	nfc->timing.tbers_max = meson_chip->tbers_max;
>   
>   	if (nfc->clk_rate != meson_chip->clk_rate) {
> -		ret = clk_set_rate(nfc->device_clk, meson_chip->clk_rate);
> +		ret = clk_set_rate(nfc->nand_clk, meson_chip->clk_rate);
>   		if (ret) {
>   			dev_err(nfc->dev, "failed to set clock rate\n");
>   			return;
> @@ -406,7 +411,6 @@ static int meson_nfc_queue_rb(struct meson_nfc *nfc, int timeout_ms)
>   	cmd = NFC_CMD_RB | NFC_CMD_RB_INT
>   		| nfc->param.chip_select | nfc->timing.tbers_max;
>   	writel(cmd, nfc->reg_base + NFC_REG_CMD);
> -
>   	ret = wait_for_completion_timeout(&nfc->completion,
>   					  msecs_to_jiffies(timeout_ms));
>   	if (ret == 0)
> @@ -985,9 +989,11 @@ static const struct mtd_ooblayout_ops meson_ooblayout_ops = {
>   	.free = meson_ooblayout_free,
>   };
>   
> +struct clk_parent_data nfc_divider_parent_data[1];
>   static int meson_nfc_clk_init(struct meson_nfc *nfc)
>   {
>   	int ret;
> +	struct clk_init_data init = {0};
>   
>   	/* request core clock */
>   	nfc->core_clk = devm_clk_get(nfc->dev, "core");
> @@ -1002,21 +1008,26 @@ static int meson_nfc_clk_init(struct meson_nfc *nfc)
>   		return PTR_ERR(nfc->device_clk);
>   	}
>   
> -	nfc->phase_tx = devm_clk_get(nfc->dev, "tx");
> -	if (IS_ERR(nfc->phase_tx)) {
> -		dev_err(nfc->dev, "failed to get TX clk\n");
> -		return PTR_ERR(nfc->phase_tx);
> -	}
> -
> -	nfc->phase_rx = devm_clk_get(nfc->dev, "rx");
> -	if (IS_ERR(nfc->phase_rx)) {
> -		dev_err(nfc->dev, "failed to get RX clk\n");
> -		return PTR_ERR(nfc->phase_rx);
> -	}
> +	init.name = devm_kstrdup(nfc->dev, "nfc#div", GFP_KERNEL);
> +	init.ops = &clk_divider_ops;
> +	nfc_divider_parent_data[0].fw_name = "device";
> +	init.parent_data = nfc_divider_parent_data;
> +	init.num_parents = 1;
> +	nfc->nand_divider.reg = nfc->sd_emmc_clock;
> +	nfc->nand_divider.shift = CLK_DIV_SHIFT;
> +	nfc->nand_divider.width = CLK_DIV_WIDTH;
> +	nfc->nand_divider.hw.init = &init;
> +	nfc->nand_divider.flags = CLK_DIVIDER_ONE_BASED |
> +				  CLK_DIVIDER_ROUND_CLOSEST |
> +				  CLK_DIVIDER_ALLOW_ZERO;
> +
> +	nfc->nand_clk = devm_clk_register(nfc->dev, &nfc->nand_divider.hw);
> +	if (IS_ERR(nfc->nand_clk))
> +		return PTR_ERR(nfc->nand_clk);
>   
>   	/* init SD_EMMC_CLOCK to sane defaults w/min clock rate */
> -	regmap_update_bits(nfc->reg_clk,
> -			   0, CLK_SELECT_NAND, CLK_SELECT_NAND);
> +	writel(CLK_SELECT_NAND | readl(nfc->sd_emmc_clock),
> +	       nfc->sd_emmc_clock);
>   
>   	ret = clk_prepare_enable(nfc->core_clk);
>   	if (ret) {
> @@ -1030,29 +1041,21 @@ static int meson_nfc_clk_init(struct meson_nfc *nfc)
>   		goto err_device_clk;
>   	}
>   
> -	ret = clk_prepare_enable(nfc->phase_tx);
> -	if (ret) {
> -		dev_err(nfc->dev, "failed to enable TX clock\n");
> -		goto err_phase_tx;
> -	}
> -
> -	ret = clk_prepare_enable(nfc->phase_rx);
> +	ret = clk_prepare_enable(nfc->nand_clk);
>   	if (ret) {
> -		dev_err(nfc->dev, "failed to enable RX clock\n");
> -		goto err_phase_rx;
> +		dev_err(nfc->dev, "pre enable NFC divider fail\n");
> +		goto err_nand_clk;
>   	}
>   
> -	ret = clk_set_rate(nfc->device_clk, 24000000);
> +	ret = clk_set_rate(nfc->nand_clk, 24000000);
>   	if (ret)
> -		goto err_disable_rx;
> +		goto err_disable_clk;
>   
>   	return 0;
>   
> -err_disable_rx:
> -	clk_disable_unprepare(nfc->phase_rx);
> -err_phase_rx:
> -	clk_disable_unprepare(nfc->phase_tx);
> -err_phase_tx:
> +err_disable_clk:
> +	clk_disable_unprepare(nfc->nand_clk);
> +err_nand_clk:
>   	clk_disable_unprepare(nfc->device_clk);
>   err_device_clk:
>   	clk_disable_unprepare(nfc->core_clk);
> @@ -1061,8 +1064,7 @@ static int meson_nfc_clk_init(struct meson_nfc *nfc)
>   
>   static void meson_nfc_disable_clk(struct meson_nfc *nfc)
>   {
> -	clk_disable_unprepare(nfc->phase_rx);
> -	clk_disable_unprepare(nfc->phase_tx);
> +	clk_disable_unprepare(nfc->nand_clk);
>   	clk_disable_unprepare(nfc->device_clk);
>   	clk_disable_unprepare(nfc->core_clk);
>   }
> @@ -1373,9 +1375,10 @@ MODULE_DEVICE_TABLE(of, meson_nfc_id_table);
>   static int meson_nfc_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
>   	struct meson_nfc *nfc;
> -	struct resource *res;
> -	int ret, irq;
> +	struct resource res;
> +	int ret, irq, i;
>   
>   	nfc = devm_kzalloc(dev, sizeof(*nfc), GFP_KERNEL);
>   	if (!nfc)
> @@ -1388,21 +1391,23 @@ static int meson_nfc_probe(struct platform_device *pdev)
>   	nand_controller_init(&nfc->controller);
>   	INIT_LIST_HEAD(&nfc->chips);
>   	init_completion(&nfc->completion);
> -
>   	nfc->dev = dev;
>   
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	nfc->reg_base = devm_ioremap_resource(dev, res);
> +	i = of_property_match_string(node, "reg-names", "nfc");
> +	if (of_address_to_resource(node, i, &res))
> +		return -ENOENT;
> +
> +	nfc->reg_base = devm_ioremap_resource(dev, &res);
>   	if (IS_ERR(nfc->reg_base))
>   		return PTR_ERR(nfc->reg_base);

You can use devm_platform_ioremap_resource_byname() here which is musch simpler,
and permits dropping platform_get_resource() at the same time.

>   
> -	nfc->reg_clk =
> -		syscon_regmap_lookup_by_phandle(dev->of_node,
> -						"amlogic,mmc-syscon");
> -	if (IS_ERR(nfc->reg_clk)) {
> -		dev_err(dev, "Failed to lookup clock base\n");
> -		return PTR_ERR(nfc->reg_clk);
> -	}
> +	i = of_property_match_string(node, "reg-names", "emmc");
> +	if (of_address_to_resource(node, i, &res))
> +		return -ENOENT;
> +
> +	nfc->sd_emmc_clock = devm_ioremap_resource(dev, &res);
> +	if (IS_ERR(nfc->sd_emmc_clock))
> +		return PTR_ERR(nfc->sd_emmc_clock);

Same here.

>   
>   	irq = platform_get_irq(pdev, 0);
>   	if (irq < 0)

Thanks !
Neil

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 1/2] mtd: rawnand: meson: discard the common MMC sub clock framework

[Liang.Yang]

Hi Neil,

On 2022/3/18 22:02, Neil Armstrong wrote:
> [ EXTERNAL EMAIL ]
> 
> Hi,
> 
> On 18/03/2022 13:41, Liang Yang wrote:
>> EMMC and NAND have the same clock control register named 
>> 'SD_EMMC_CLOCK' which is
>> defined in EMMC port internally. bit0~5 of 'SD_EMMC_CLOCK' is the 
>> divider and
>> bit6~7 is the mux for fix pll and xtal.A common MMC and NAND sub-clock 
>> has been
>> implemented and can be used by the eMMC and NAND controller (which are 
>> mutually
>> exclusive anyway). Let's use this new clock.
>>
>> Signed-off-by: Liang Yang <liang.yang@amlogic.com>
>> ---
>>   drivers/mtd/nand/raw/meson_nand.c | 101 ++++++++++++++++--------------
>>   1 file changed, 53 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/raw/meson_nand.c 
>> b/drivers/mtd/nand/raw/meson_nand.c
>> index ac3be92872d0..257ffc8a41ab 100644
>> --- a/drivers/mtd/nand/raw/meson_nand.c
>> +++ b/drivers/mtd/nand/raw/meson_nand.c
>> @@ -10,6 +10,7 @@
>>   #include <linux/dma-mapping.h>
>>   #include <linux/interrupt.h>
>>   #include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>>   #include <linux/mtd/rawnand.h>
>>   #include <linux/mtd/mtd.h>
>>   #include <linux/mfd/syscon.h>
>> @@ -19,6 +20,7 @@
>>   #include <linux/iopoll.h>
>>   #include <linux/of.h>
>>   #include <linux/of_device.h>
>> +#include <linux/of_address.h>
>>   #include <linux/sched/task_stack.h>
>>   #define NFC_REG_CMD        0x00
>> @@ -104,6 +106,9 @@
>>   #define PER_INFO_BYTE        8
>> +#define CLK_DIV_SHIFT        0
>> +#define CLK_DIV_WIDTH        6
>> +
>>   struct meson_nfc_nand_chip {
>>       struct list_head node;
>>       struct nand_chip nand;
>> @@ -151,15 +156,15 @@ struct meson_nfc {
>>       struct nand_controller controller;
>>       struct clk *core_clk;
>>       struct clk *device_clk;
>> -    struct clk *phase_tx;
>> -    struct clk *phase_rx;
>> +    struct clk *nand_clk;
>> +    struct clk_divider nand_divider;
>>       unsigned long clk_rate;
>>       u32 bus_timing;
>>       struct device *dev;
>>       void __iomem *reg_base;
>> -    struct regmap *reg_clk;
>> +    void __iomem *sd_emmc_clock;
>>       struct completion completion;
>>       struct list_head chips;
>>       const struct meson_nfc_data *data;
>> @@ -235,7 +240,7 @@ static void meson_nfc_select_chip(struct nand_chip 
>> *nand, int chip)
>>       nfc->timing.tbers_max = meson_chip->tbers_max;
>>       if (nfc->clk_rate != meson_chip->clk_rate) {
>> -        ret = clk_set_rate(nfc->device_clk, meson_chip->clk_rate);
>> +        ret = clk_set_rate(nfc->nand_clk, meson_chip->clk_rate);
>>           if (ret) {
>>               dev_err(nfc->dev, "failed to set clock rate\n");
>>               return;
>> @@ -406,7 +411,6 @@ static int meson_nfc_queue_rb(struct meson_nfc 
>> *nfc, int timeout_ms)
>>       cmd = NFC_CMD_RB | NFC_CMD_RB_INT
>>           | nfc->param.chip_select | nfc->timing.tbers_max;
>>       writel(cmd, nfc->reg_base + NFC_REG_CMD);
>> -
>>       ret = wait_for_completion_timeout(&nfc->completion,
>>                         msecs_to_jiffies(timeout_ms));
>>       if (ret == 0)
>> @@ -985,9 +989,11 @@ static const struct mtd_ooblayout_ops 
>> meson_ooblayout_ops = {
>>       .free = meson_ooblayout_free,
>>   };
>> +struct clk_parent_data nfc_divider_parent_data[1];
>>   static int meson_nfc_clk_init(struct meson_nfc *nfc)
>>   {
>>       int ret;
>> +    struct clk_init_data init = {0};
>>       /* request core clock */
>>       nfc->core_clk = devm_clk_get(nfc->dev, "core");
>> @@ -1002,21 +1008,26 @@ static int meson_nfc_clk_init(struct meson_nfc 
>> *nfc)
>>           return PTR_ERR(nfc->device_clk);
>>       }
>> -    nfc->phase_tx = devm_clk_get(nfc->dev, "tx");
>> -    if (IS_ERR(nfc->phase_tx)) {
>> -        dev_err(nfc->dev, "failed to get TX clk\n");
>> -        return PTR_ERR(nfc->phase_tx);
>> -    }
>> -
>> -    nfc->phase_rx = devm_clk_get(nfc->dev, "rx");
>> -    if (IS_ERR(nfc->phase_rx)) {
>> -        dev_err(nfc->dev, "failed to get RX clk\n");
>> -        return PTR_ERR(nfc->phase_rx);
>> -    }
>> +    init.name = devm_kstrdup(nfc->dev, "nfc#div", GFP_KERNEL);
>> +    init.ops = &clk_divider_ops;
>> +    nfc_divider_parent_data[0].fw_name = "device";
>> +    init.parent_data = nfc_divider_parent_data;
>> +    init.num_parents = 1;
>> +    nfc->nand_divider.reg = nfc->sd_emmc_clock;
>> +    nfc->nand_divider.shift = CLK_DIV_SHIFT;
>> +    nfc->nand_divider.width = CLK_DIV_WIDTH;
>> +    nfc->nand_divider.hw.init = &init;
>> +    nfc->nand_divider.flags = CLK_DIVIDER_ONE_BASED |
>> +                  CLK_DIVIDER_ROUND_CLOSEST |
>> +                  CLK_DIVIDER_ALLOW_ZERO;
>> +
>> +    nfc->nand_clk = devm_clk_register(nfc->dev, &nfc->nand_divider.hw);
>> +    if (IS_ERR(nfc->nand_clk))
>> +        return PTR_ERR(nfc->nand_clk);
>>       /* init SD_EMMC_CLOCK to sane defaults w/min clock rate */
>> -    regmap_update_bits(nfc->reg_clk,
>> -               0, CLK_SELECT_NAND, CLK_SELECT_NAND);
>> +    writel(CLK_SELECT_NAND | readl(nfc->sd_emmc_clock),
>> +           nfc->sd_emmc_clock);
>>       ret = clk_prepare_enable(nfc->core_clk);
>>       if (ret) {
>> @@ -1030,29 +1041,21 @@ static int meson_nfc_clk_init(struct meson_nfc 
>> *nfc)
>>           goto err_device_clk;
>>       }
>> -    ret = clk_prepare_enable(nfc->phase_tx);
>> -    if (ret) {
>> -        dev_err(nfc->dev, "failed to enable TX clock\n");
>> -        goto err_phase_tx;
>> -    }
>> -
>> -    ret = clk_prepare_enable(nfc->phase_rx);
>> +    ret = clk_prepare_enable(nfc->nand_clk);
>>       if (ret) {
>> -        dev_err(nfc->dev, "failed to enable RX clock\n");
>> -        goto err_phase_rx;
>> +        dev_err(nfc->dev, "pre enable NFC divider fail\n");
>> +        goto err_nand_clk;
>>       }
>> -    ret = clk_set_rate(nfc->device_clk, 24000000);
>> +    ret = clk_set_rate(nfc->nand_clk, 24000000);
>>       if (ret)
>> -        goto err_disable_rx;
>> +        goto err_disable_clk;
>>       return 0;
>> -err_disable_rx:
>> -    clk_disable_unprepare(nfc->phase_rx);
>> -err_phase_rx:
>> -    clk_disable_unprepare(nfc->phase_tx);
>> -err_phase_tx:
>> +err_disable_clk:
>> +    clk_disable_unprepare(nfc->nand_clk);
>> +err_nand_clk:
>>       clk_disable_unprepare(nfc->device_clk);
>>   err_device_clk:
>>       clk_disable_unprepare(nfc->core_clk);
>> @@ -1061,8 +1064,7 @@ static int meson_nfc_clk_init(struct meson_nfc 
>> *nfc)
>>   static void meson_nfc_disable_clk(struct meson_nfc *nfc)
>>   {
>> -    clk_disable_unprepare(nfc->phase_rx);
>> -    clk_disable_unprepare(nfc->phase_tx);
>> +    clk_disable_unprepare(nfc->nand_clk);
>>       clk_disable_unprepare(nfc->device_clk);
>>       clk_disable_unprepare(nfc->core_clk);
>>   }
>> @@ -1373,9 +1375,10 @@ MODULE_DEVICE_TABLE(of, meson_nfc_id_table);
>>   static int meson_nfc_probe(struct platform_device *pdev)
>>   {
>>       struct device *dev = &pdev->dev;
>> +    struct device_node *node = dev->of_node;
>>       struct meson_nfc *nfc;
>> -    struct resource *res;
>> -    int ret, irq;
>> +    struct resource res;
>> +    int ret, irq, i;
>>       nfc = devm_kzalloc(dev, sizeof(*nfc), GFP_KERNEL);
>>       if (!nfc)
>> @@ -1388,21 +1391,23 @@ static int meson_nfc_probe(struct 
>> platform_device *pdev)
>>       nand_controller_init(&nfc->controller);
>>       INIT_LIST_HEAD(&nfc->chips);
>>       init_completion(&nfc->completion);
>> -
>>       nfc->dev = dev;
>> -    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -    nfc->reg_base = devm_ioremap_resource(dev, res);
>> +    i = of_property_match_string(node, "reg-names", "nfc");
>> +    if (of_address_to_resource(node, i, &res))
>> +        return -ENOENT;
>> +
>> +    nfc->reg_base = devm_ioremap_resource(dev, &res);
>>       if (IS_ERR(nfc->reg_base))
>>           return PTR_ERR(nfc->reg_base);
> 
> You can use devm_platform_ioremap_resource_byname() here which is musch 
> simpler,
> and permits dropping platform_get_resource() at the same time.
ok, i will do it.
> 
>> -    nfc->reg_clk =
>> -        syscon_regmap_lookup_by_phandle(dev->of_node,
>> -                        "amlogic,mmc-syscon");
>> -    if (IS_ERR(nfc->reg_clk)) {
>> -        dev_err(dev, "Failed to lookup clock base\n");
>> -        return PTR_ERR(nfc->reg_clk);
>> -    }
>> +    i = of_property_match_string(node, "reg-names", "emmc");
>> +    if (of_address_to_resource(node, i, &res))
>> +        return -ENOENT;
>> +
>> +    nfc->sd_emmc_clock = devm_ioremap_resource(dev, &res);
>> +    if (IS_ERR(nfc->sd_emmc_clock))
>> +        return PTR_ERR(nfc->sd_emmc_clock);
> 
> Same here.
ok. thanks.
> 
>>       irq = platform_get_irq(pdev, 0);
>>       if (irq < 0)
> 
> Thanks !
> Neil
> 
> .

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 1/2] mtd: rawnand: meson: discard the common MMC sub clock framework

[kernel test robot]

Hi Liang,

I love your patch! Yet something to improve:

[auto build test ERROR on mtd/nand/next]
[also build test ERROR on mtd/mtd/next mtd/mtd/fixes robh/for-next v5.17 next-20220318]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Liang-Yang/refine-the-NFC-clock-framework/20220318-204307
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next
config: m68k-randconfig-r015-20220320 (https://download.01.org/0day-ci/archive/20220321/202203211339.6SuyU3aA-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/8722c778a1a81891f7362662caf9d166442d5990
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Liang-Yang/refine-the-NFC-clock-framework/20220318-204307
        git checkout 8722c778a1a81891f7362662caf9d166442d5990
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=m68k SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   m68k-linux-ld: drivers/mtd/nand/raw/meson_nand.o: in function `meson_nfc_probe':
>> meson_nand.c:(.text+0xaae): undefined reference to `clk_divider_ops'
>> m68k-linux-ld: meson_nand.c:(.text+0xaf0): undefined reference to `devm_clk_register'

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v3 2/2] dt-bindings: nand: meson: refine Amlogic NAND controller driver

[Liang Yang]

convert txt to yaml and refine the meson NFC clock document.

Signed-off-by: Liang Yang <liang.yang@amlogic.com>
---
 .../bindings/mtd/amlogic,meson-nand.txt       | 60 ----------------
 .../bindings/mtd/amlogic,meson-nand.yaml      | 71 +++++++++++++++++++
 2 files changed, 71 insertions(+), 60 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml

diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
deleted file mode 100644
index 5794ab1147c1..000000000000
--- a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
+++ /dev/null
@@ -1,60 +0,0 @@
-Amlogic NAND Flash Controller (NFC) for GXBB/GXL/AXG family SoCs
-
-This file documents the properties in addition to those available in
-the MTD NAND bindings.
-
-Required properties:
-- compatible : contains one of:
-  - "amlogic,meson-gxl-nfc"
-  - "amlogic,meson-axg-nfc"
-- clocks     :
-	A list of phandle + clock-specifier pairs for the clocks listed
-	in clock-names.
-
-- clock-names: Should contain the following:
-	"core" - NFC module gate clock
-	"device" - device clock from eMMC sub clock controller
-	"rx" - rx clock phase
-	"tx" - tx clock phase
-
-- amlogic,mmc-syscon	: Required for NAND clocks, it's shared with SD/eMMC
-				controller port C
-
-Optional children nodes:
-Children nodes represent the available nand chips.
-
-Other properties:
-see Documentation/devicetree/bindings/mtd/nand-controller.yaml for generic bindings.
-
-Example demonstrate on AXG SoC:
-
-	sd_emmc_c_clkc: mmc@7000 {
-		compatible = "amlogic,meson-axg-mmc-clkc", "syscon";
-		reg = <0x0 0x7000 0x0 0x800>;
-	};
-
-	nand-controller@7800 {
-		compatible = "amlogic,meson-axg-nfc";
-		reg = <0x0 0x7800 0x0 0x100>;
-		#address-cells = <1>;
-		#size-cells = <0>;
-		interrupts = <GIC_SPI 34 IRQ_TYPE_EDGE_RISING>;
-
-		clocks = <&clkc CLKID_SD_EMMC_C>,
-			<&sd_emmc_c_clkc CLKID_MMC_DIV>,
-			<&sd_emmc_c_clkc CLKID_MMC_PHASE_RX>,
-			<&sd_emmc_c_clkc CLKID_MMC_PHASE_TX>;
-		clock-names = "core", "device", "rx", "tx";
-		amlogic,mmc-syscon = <&sd_emmc_c_clkc>;
-
-		pinctrl-names = "default";
-		pinctrl-0 = <&nand_pins>;
-
-		nand@0 {
-			reg = <0>;
-			#address-cells = <1>;
-			#size-cells = <1>;
-
-			nand-on-flash-bbt;
-		};
-	};
diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
new file mode 100644
index 000000000000..4de4962a40b7
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/amlogic,mmc-clkc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Amlogic NAND Flash Controller (NFC) for GXBB/GXL/AXG family SoCs
+
+maintainers:
+  - liang.yang@amlogic.com
+
+properties:
+  compatible:
+    enum:
+      - "amlogic,meson-gxl-nfc"
+      - "amlogic,meson-axg-nfc"
+
+  reg:
+    maxItems: 2
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: "core", "device"
+
+  "#clock-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - "#clock-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    nand-controller@7800 {
+	compatible = "amlogic,meson-axg-nfc";
+	reg = <0x0 0x7800 0x0 0x100>,
+	      <0x0 0x7000 0x0 0x800>;
+        reg-names = "nfc", "emmc";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	interrupts = <GIC_SPI 34 IRQ_TYPE_EDGE_RISING>;
+
+	clocks = <&clkc CLKID_SD_EMMC_C>,
+		<&clkc CLKID_FCLK_DIV2>;
+	clock-names = "core", "device";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&nand_pins>;
+
+	nand@0 {
+		reg = <0>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		nand-on-flash-bbt;
+	};
+     };
+
+...
-- 
2.34.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 2/2] dt-bindings: nand: meson: refine Amlogic NAND controller driver

[Neil Armstrong]

Hi,

On 18/03/2022 13:41, Liang Yang wrote:
> convert txt to yaml and refine the meson NFC clock document.
> 
> Signed-off-by: Liang Yang <liang.yang@amlogic.com>
> ---
>   .../bindings/mtd/amlogic,meson-nand.txt       | 60 ----------------
>   .../bindings/mtd/amlogic,meson-nand.yaml      | 71 +++++++++++++++++++
>   2 files changed, 71 insertions(+), 60 deletions(-)
>   delete mode 100644 Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
>   create mode 100644 Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
> deleted file mode 100644
> index 5794ab1147c1..000000000000
> --- a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
> +++ /dev/null
> @@ -1,60 +0,0 @@
> -Amlogic NAND Flash Controller (NFC) for GXBB/GXL/AXG family SoCs
> -
> -This file documents the properties in addition to those available in
> -the MTD NAND bindings.
> -
> -Required properties:
> -- compatible : contains one of:
> -  - "amlogic,meson-gxl-nfc"
> -  - "amlogic,meson-axg-nfc"
> -- clocks     :
> -	A list of phandle + clock-specifier pairs for the clocks listed
> -	in clock-names.
> -
> -- clock-names: Should contain the following:
> -	"core" - NFC module gate clock
> -	"device" - device clock from eMMC sub clock controller
> -	"rx" - rx clock phase
> -	"tx" - tx clock phase
> -
> -- amlogic,mmc-syscon	: Required for NAND clocks, it's shared with SD/eMMC
> -				controller port C
> -
> -Optional children nodes:
> -Children nodes represent the available nand chips.
> -
> -Other properties:
> -see Documentation/devicetree/bindings/mtd/nand-controller.yaml for generic bindings.
> -
> -Example demonstrate on AXG SoC:
> -
> -	sd_emmc_c_clkc: mmc@7000 {
> -		compatible = "amlogic,meson-axg-mmc-clkc", "syscon";
> -		reg = <0x0 0x7000 0x0 0x800>;
> -	};
> -
> -	nand-controller@7800 {
> -		compatible = "amlogic,meson-axg-nfc";
> -		reg = <0x0 0x7800 0x0 0x100>;
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -		interrupts = <GIC_SPI 34 IRQ_TYPE_EDGE_RISING>;
> -
> -		clocks = <&clkc CLKID_SD_EMMC_C>,
> -			<&sd_emmc_c_clkc CLKID_MMC_DIV>,
> -			<&sd_emmc_c_clkc CLKID_MMC_PHASE_RX>,
> -			<&sd_emmc_c_clkc CLKID_MMC_PHASE_TX>;
> -		clock-names = "core", "device", "rx", "tx";
> -		amlogic,mmc-syscon = <&sd_emmc_c_clkc>;
> -
> -		pinctrl-names = "default";
> -		pinctrl-0 = <&nand_pins>;
> -
> -		nand@0 {
> -			reg = <0>;
> -			#address-cells = <1>;
> -			#size-cells = <1>;
> -
> -			nand-on-flash-bbt;
> -		};
> -	};
> diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
> new file mode 100644
> index 000000000000..4de4962a40b7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/amlogic,mmc-clkc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Amlogic NAND Flash Controller (NFC) for GXBB/GXL/AXG family SoCs
> +
> +maintainers:
> +  - liang.yang@amlogic.com
> +
> +properties:
> +  compatible:
> +    enum:
> +      - "amlogic,meson-gxl-nfc"
> +      - "amlogic,meson-axg-nfc"
> +
> +  reg:
> +    maxItems: 2

   reg-names:
     items:
       - const: nfc
       - const: mmc

Is missing to conform to the example

> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      - const: "core", "device"

This should be:
   clock-names:
     items:
       - const: core
       - const: device

> +
> +  "#clock-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg

+ reg-names

> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - "#clock-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    nand-controller@7800 {
> +	compatible = "amlogic,meson-axg-nfc";
> +	reg = <0x0 0x7800 0x0 0x100>,
> +	      <0x0 0x7000 0x0 0x800>;
> +        reg-names = "nfc", "emmc";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	interrupts = <GIC_SPI 34 IRQ_TYPE_EDGE_RISING>;
> +
> +	clocks = <&clkc CLKID_SD_EMMC_C>,
> +		<&clkc CLKID_FCLK_DIV2>;
> +	clock-names = "core", "device";
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&nand_pins>;
> +
> +	nand@0 {
> +		reg = <0>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +
> +		nand-on-flash-bbt;
> +	};
> +     };
> +
> +...

Don't forgot to run :
make ARCH=-arm64 dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml

after installing the DT schema validator:
pip3 install git+https://github.com/devicetree-org/dt-schema.git@master

as documented at https://www.kernel.org/doc/html/v5.9/devicetree/writing-schema.html

Thanks !
Neil

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 2/2] dt-bindings: nand: meson: refine Amlogic NAND controller driver

[Liang.Yang]

Hi Neil,

On 2022/3/18 21:59, Neil Armstrong wrote:
> [ EXTERNAL EMAIL ]
> 
> Hi,
> 
> On 18/03/2022 13:41, Liang Yang wrote:
>> convert txt to yaml and refine the meson NFC clock document.
>>
>> Signed-off-by: Liang Yang <liang.yang@amlogic.com>
>> ---
>>   .../bindings/mtd/amlogic,meson-nand.txt       | 60 ----------------
>>   .../bindings/mtd/amlogic,meson-nand.yaml      | 71 +++++++++++++++++++
>>   2 files changed, 71 insertions(+), 60 deletions(-)
>>   delete mode 100644 
>> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
>>   create mode 100644 
>> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt 
>> b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
>> deleted file mode 100644
>> index 5794ab1147c1..000000000000
>> --- a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
>> +++ /dev/null
>> @@ -1,60 +0,0 @@
>> -Amlogic NAND Flash Controller (NFC) for GXBB/GXL/AXG family SoCs
>> -
>> -This file documents the properties in addition to those available in
>> -the MTD NAND bindings.
>> -
>> -Required properties:
>> -- compatible : contains one of:
>> -  - "amlogic,meson-gxl-nfc"
>> -  - "amlogic,meson-axg-nfc"
>> -- clocks     :
>> -    A list of phandle + clock-specifier pairs for the clocks listed
>> -    in clock-names.
>> -
>> -- clock-names: Should contain the following:
>> -    "core" - NFC module gate clock
>> -    "device" - device clock from eMMC sub clock controller
>> -    "rx" - rx clock phase
>> -    "tx" - tx clock phase
>> -
>> -- amlogic,mmc-syscon    : Required for NAND clocks, it's shared with 
>> SD/eMMC
>> -                controller port C
>> -
>> -Optional children nodes:
>> -Children nodes represent the available nand chips.
>> -
>> -Other properties:
>> -see Documentation/devicetree/bindings/mtd/nand-controller.yaml for 
>> generic bindings.
>> -
>> -Example demonstrate on AXG SoC:
>> -
>> -    sd_emmc_c_clkc: mmc@7000 {
>> -        compatible = "amlogic,meson-axg-mmc-clkc", "syscon";
>> -        reg = <0x0 0x7000 0x0 0x800>;
>> -    };
>> -
>> -    nand-controller@7800 {
>> -        compatible = "amlogic,meson-axg-nfc";
>> -        reg = <0x0 0x7800 0x0 0x100>;
>> -        #address-cells = <1>;
>> -        #size-cells = <0>;
>> -        interrupts = <GIC_SPI 34 IRQ_TYPE_EDGE_RISING>;
>> -
>> -        clocks = <&clkc CLKID_SD_EMMC_C>,
>> -            <&sd_emmc_c_clkc CLKID_MMC_DIV>,
>> -            <&sd_emmc_c_clkc CLKID_MMC_PHASE_RX>,
>> -            <&sd_emmc_c_clkc CLKID_MMC_PHASE_TX>;
>> -        clock-names = "core", "device", "rx", "tx";
>> -        amlogic,mmc-syscon = <&sd_emmc_c_clkc>;
>> -
>> -        pinctrl-names = "default";
>> -        pinctrl-0 = <&nand_pins>;
>> -
>> -        nand@0 {
>> -            reg = <0>;
>> -            #address-cells = <1>;
>> -            #size-cells = <1>;
>> -
>> -            nand-on-flash-bbt;
>> -        };
>> -    };
>> diff --git 
>> a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml 
>> b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
>> new file mode 100644
>> index 000000000000..4de4962a40b7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
>> @@ -0,0 +1,71 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/clock/amlogic,mmc-clkc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Amlogic NAND Flash Controller (NFC) for GXBB/GXL/AXG family SoCs
>> +
>> +maintainers:
>> +  - liang.yang@amlogic.com
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - "amlogic,meson-gxl-nfc"
>> +      - "amlogic,meson-axg-nfc"
>> +
>> +  reg:
>> +    maxItems: 2
> 
>    reg-names:
>      items:
>        - const: nfc
>        - const: mmc
> 
> Is missing to conform to the example
ok, i will add it.
> 
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 2
>> +
>> +  clock-names:
>> +    items:
>> +      - const: "core", "device"
> 
> This should be:
>    clock-names:
>      items:
>        - const: core
>        - const: device

ok, i will fix it.
> 
>> +
>> +  "#clock-cells":
>> +    const: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
> 
> + reg-names
ok
> 
>> +  - interrupts
>> +  - clocks
>> +  - clock-names
>> +  - "#clock-cells"
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    nand-controller@7800 {
>> +    compatible = "amlogic,meson-axg-nfc";
>> +    reg = <0x0 0x7800 0x0 0x100>,
>> +          <0x0 0x7000 0x0 0x800>;
>> +        reg-names = "nfc", "emmc";
>> +    #address-cells = <1>;
>> +    #size-cells = <0>;
>> +    interrupts = <GIC_SPI 34 IRQ_TYPE_EDGE_RISING>;
>> +
>> +    clocks = <&clkc CLKID_SD_EMMC_C>,
>> +        <&clkc CLKID_FCLK_DIV2>;
>> +    clock-names = "core", "device";
>> +
>> +    pinctrl-names = "default";
>> +    pinctrl-0 = <&nand_pins>;
>> +
>> +    nand@0 {
>> +        reg = <0>;
>> +        #address-cells = <1>;
>> +        #size-cells = <1>;
>> +
>> +        nand-on-flash-bbt;
>> +    };
>> +     };
>> +
>> +...
> 
> Don't forgot to run :
> make ARCH=-arm64 dt_binding_check 
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml 
> 
> 
> after installing the DT schema validator:
> pip3 install git+https://github.com/devicetree-org/dt-schema.git@master
> 
> as documented at 
> https://www.kernel.org/doc/html/v5.9/devicetree/writing-schema.html
ok
> 
> Thanks !
> Neil
> 
> .

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic