[PATCH 01/17] clk: sunxi: sunxi-ng: convert to devm_platform_ioremap_resource

[PATCH 01/17] clk: sunxi: sunxi-ng: convert to devm_platform_ioremap_resource

[Yangtao Li]

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 drivers/clk/sunxi-ng/ccu-sun50i-a64.c    | 4 +---
 drivers/clk/sunxi-ng/ccu-sun50i-h6.c     | 4 +---
 drivers/clk/sunxi-ng/ccu-sun8i-a83t.c    | 4 +---
 drivers/clk/sunxi-ng/ccu-sun8i-de2.c     | 4 +---
 drivers/clk/sunxi-ng/ccu-sun8i-r40.c     | 4 +---
 drivers/clk/sunxi-ng/ccu-sun9i-a80-de.c  | 4 +---
 drivers/clk/sunxi-ng/ccu-sun9i-a80-usb.c | 4 +---
 drivers/clk/sunxi-ng/ccu-sun9i-a80.c     | 4 +---
 drivers/clk/sunxi/clk-mod0.c             | 4 +---
 drivers/clk/sunxi/clk-sun6i-apb0-gates.c | 4 +---
 drivers/clk/sunxi/clk-sun6i-apb0.c       | 4 +---
 drivers/clk/sunxi/clk-sun6i-ar100.c      | 4 +---
 drivers/clk/sunxi/clk-sun8i-apb0.c       | 4 +---
 13 files changed, 13 insertions(+), 39 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
index 49bd7a4c015c..087880b20c15 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
@@ -923,12 +923,10 @@ static const struct sunxi_ccu_desc sun50i_a64_ccu_desc = {
 
 static int sun50i_a64_ccu_probe(struct platform_device *pdev)
 {
-	struct resource *res;
 	void __iomem *reg;
 	u32 val;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(&pdev->dev, res);
+	reg = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-h6.c b/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
index f2497d0a4683..00fa8ff2f51b 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
@@ -1183,13 +1183,11 @@ static const u32 usb2_clk_regs[] = {
 
 static int sun50i_h6_ccu_probe(struct platform_device *pdev)
 {
-	struct resource *res;
 	void __iomem *reg;
 	u32 val;
 	int i;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(&pdev->dev, res);
+	reg = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c b/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
index 2b434521c5cc..b2aaa99ad23f 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
@@ -887,12 +887,10 @@ static void sun8i_a83t_cpu_pll_fixup(void __iomem *reg)
 
 static int sun8i_a83t_ccu_probe(struct platform_device *pdev)
 {
-	struct resource *res;
 	void __iomem *reg;
 	u32 val;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(&pdev->dev, res);
+	reg = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-de2.c b/drivers/clk/sunxi-ng/ccu-sun8i-de2.c
index d9668493c3f9..f48f61297551 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-de2.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-de2.c
@@ -251,7 +251,6 @@ static const struct sunxi_ccu_desc sun8i_v3s_de2_clk_desc = {
 
 static int sunxi_de2_clk_probe(struct platform_device *pdev)
 {
-	struct resource *res;
 	struct clk *bus_clk, *mod_clk;
 	struct reset_control *rstc;
 	void __iomem *reg;
@@ -262,8 +261,7 @@ static int sunxi_de2_clk_probe(struct platform_device *pdev)
 	if (!ccu_desc)
 		return -EINVAL;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(&pdev->dev, res);
+	reg = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-r40.c b/drivers/clk/sunxi-ng/ccu-sun8i-r40.c
index 897490800102..b456f85fd8da 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-r40.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-r40.c
@@ -1294,14 +1294,12 @@ static struct regmap_config sun8i_r40_ccu_regmap_config = {
 
 static int sun8i_r40_ccu_probe(struct platform_device *pdev)
 {
-	struct resource *res;
 	struct regmap *regmap;
 	void __iomem *reg;
 	u32 val;
 	int ret;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(&pdev->dev, res);
+	reg = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
diff --git a/drivers/clk/sunxi-ng/ccu-sun9i-a80-de.c b/drivers/clk/sunxi-ng/ccu-sun9i-a80-de.c
index 6616e8114f62..94552199d54d 100644
--- a/drivers/clk/sunxi-ng/ccu-sun9i-a80-de.c
+++ b/drivers/clk/sunxi-ng/ccu-sun9i-a80-de.c
@@ -203,14 +203,12 @@ static const struct sunxi_ccu_desc sun9i_a80_de_clk_desc = {
 
 static int sun9i_a80_de_clk_probe(struct platform_device *pdev)
 {
-	struct resource *res;
 	struct clk *bus_clk;
 	struct reset_control *rstc;
 	void __iomem *reg;
 	int ret;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(&pdev->dev, res);
+	reg = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
diff --git a/drivers/clk/sunxi-ng/ccu-sun9i-a80-usb.c b/drivers/clk/sunxi-ng/ccu-sun9i-a80-usb.c
index 4b4a507d04ed..adac159a5f63 100644
--- a/drivers/clk/sunxi-ng/ccu-sun9i-a80-usb.c
+++ b/drivers/clk/sunxi-ng/ccu-sun9i-a80-usb.c
@@ -92,13 +92,11 @@ static const struct sunxi_ccu_desc sun9i_a80_usb_clk_desc = {
 
 static int sun9i_a80_usb_clk_probe(struct platform_device *pdev)
 {
-	struct resource *res;
 	struct clk *bus_clk;
 	void __iomem *reg;
 	int ret;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(&pdev->dev, res);
+	reg = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
diff --git a/drivers/clk/sunxi-ng/ccu-sun9i-a80.c b/drivers/clk/sunxi-ng/ccu-sun9i-a80.c
index dcac1391767f..8295667e2252 100644
--- a/drivers/clk/sunxi-ng/ccu-sun9i-a80.c
+++ b/drivers/clk/sunxi-ng/ccu-sun9i-a80.c
@@ -1213,12 +1213,10 @@ static void sun9i_a80_cpu_pll_fixup(void __iomem *reg)
 
 static int sun9i_a80_ccu_probe(struct platform_device *pdev)
 {
-	struct resource *res;
 	void __iomem *reg;
 	u32 val;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(&pdev->dev, res);
+	reg = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
diff --git a/drivers/clk/sunxi/clk-mod0.c b/drivers/clk/sunxi/clk-mod0.c
index 0cca91e075a5..cdb98937f584 100644
--- a/drivers/clk/sunxi/clk-mod0.c
+++ b/drivers/clk/sunxi/clk-mod0.c
@@ -88,14 +88,12 @@ CLK_OF_DECLARE_DRIVER(sun4i_a10_mod0, "allwinner,sun4i-a10-mod0-clk",
 static int sun4i_a10_mod0_clk_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
-	struct resource *r;
 	void __iomem *reg;
 
 	if (!np)
 		return -ENODEV;
 
-	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(&pdev->dev, r);
+	reg = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
diff --git a/drivers/clk/sunxi/clk-sun6i-apb0-gates.c b/drivers/clk/sunxi/clk-sun6i-apb0-gates.c
index a165e7172346..abae2813c59e 100644
--- a/drivers/clk/sunxi/clk-sun6i-apb0-gates.c
+++ b/drivers/clk/sunxi/clk-sun6i-apb0-gates.c
@@ -41,7 +41,6 @@ static int sun6i_a31_apb0_gates_clk_probe(struct platform_device *pdev)
 	const struct gates_data *data;
 	const char *clk_parent;
 	const char *clk_name;
-	struct resource *r;
 	void __iomem *reg;
 	int ngates;
 	int i;
@@ -55,8 +54,7 @@ static int sun6i_a31_apb0_gates_clk_probe(struct platform_device *pdev)
 		return -ENODEV;
 	data = device->data;
 
-	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(&pdev->dev, r);
+	reg = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
diff --git a/drivers/clk/sunxi/clk-sun6i-apb0.c b/drivers/clk/sunxi/clk-sun6i-apb0.c
index 10f70c35c265..f80c67bafe38 100644
--- a/drivers/clk/sunxi/clk-sun6i-apb0.c
+++ b/drivers/clk/sunxi/clk-sun6i-apb0.c
@@ -32,12 +32,10 @@ static int sun6i_a31_apb0_clk_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	const char *clk_name = np->name;
 	const char *clk_parent;
-	struct resource *r;
 	void __iomem *reg;
 	struct clk *clk;
 
-	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(&pdev->dev, r);
+	reg = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
diff --git a/drivers/clk/sunxi/clk-sun6i-ar100.c b/drivers/clk/sunxi/clk-sun6i-ar100.c
index e1b7d0929cf7..ffc297732f8f 100644
--- a/drivers/clk/sunxi/clk-sun6i-ar100.c
+++ b/drivers/clk/sunxi/clk-sun6i-ar100.c
@@ -71,12 +71,10 @@ static DEFINE_SPINLOCK(sun6i_ar100_lock);
 static int sun6i_a31_ar100_clk_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
-	struct resource *r;
 	void __iomem *reg;
 	struct clk *clk;
 
-	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(&pdev->dev, r);
+	reg = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
diff --git a/drivers/clk/sunxi/clk-sun8i-apb0.c b/drivers/clk/sunxi/clk-sun8i-apb0.c
index fc5d6e3b77d1..f605ecca879f 100644
--- a/drivers/clk/sunxi/clk-sun8i-apb0.c
+++ b/drivers/clk/sunxi/clk-sun8i-apb0.c
@@ -87,12 +87,10 @@ CLK_OF_DECLARE_DRIVER(sun8i_a23_apb0, "allwinner,sun8i-a23-apb0-clk",
 static int sun8i_a23_apb0_clk_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
-	struct resource *r;
 	void __iomem *reg;
 	struct clk *clk;
 
-	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(&pdev->dev, r);
+	reg = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
-- 
2.17.1

[PATCH 02/17] clk: qcom: convert to devm_platform_ioremap_resource

[Yangtao Li]

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 drivers/clk/qcom/a53-pll.c  | 4 +---
 drivers/clk/qcom/common.c   | 8 ++------
 drivers/clk/qcom/hfpll.c    | 4 +---
 drivers/clk/qcom/kpss-xcc.c | 4 +---
 4 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/clk/qcom/a53-pll.c b/drivers/clk/qcom/a53-pll.c
index 45cfc57bff92..42d14b6861d4 100644
--- a/drivers/clk/qcom/a53-pll.c
+++ b/drivers/clk/qcom/a53-pll.c
@@ -38,7 +38,6 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct regmap *regmap;
-	struct resource *res;
 	struct clk_pll *pll;
 	void __iomem *base;
 	struct clk_init_data init = { };
@@ -48,8 +47,7 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
 	if (!pll)
 		return -ENOMEM;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(dev, res);
+	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index 60d2a78d1395..0932e019dd12 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -73,11 +73,9 @@ struct regmap *
 qcom_cc_map(struct platform_device *pdev, const struct qcom_cc_desc *desc)
 {
 	void __iomem *base;
-	struct resource *res;
 	struct device *dev = &pdev->dev;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(dev, res);
+	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return ERR_CAST(base);
 
@@ -313,11 +311,9 @@ int qcom_cc_probe_by_index(struct platform_device *pdev, int index,
 			   const struct qcom_cc_desc *desc)
 {
 	struct regmap *regmap;
-	struct resource *res;
 	void __iomem *base;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, index);
-	base = devm_ioremap_resource(&pdev->dev, res);
+	base = devm_platform_ioremap_resource(pdev, index);
 	if (IS_ERR(base))
 		return -ENOMEM;
 
diff --git a/drivers/clk/qcom/hfpll.c b/drivers/clk/qcom/hfpll.c
index a6de7101430c..de7df848dcfb 100644
--- a/drivers/clk/qcom/hfpll.c
+++ b/drivers/clk/qcom/hfpll.c
@@ -47,7 +47,6 @@ static const struct regmap_config hfpll_regmap_config = {
 
 static int qcom_hfpll_probe(struct platform_device *pdev)
 {
-	struct resource *res;
 	struct device *dev = &pdev->dev;
 	void __iomem *base;
 	struct regmap *regmap;
@@ -62,8 +61,7 @@ static int qcom_hfpll_probe(struct platform_device *pdev)
 	if (!h)
 		return -ENOMEM;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(dev, res);
+	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
diff --git a/drivers/clk/qcom/kpss-xcc.c b/drivers/clk/qcom/kpss-xcc.c
index 8590b5edd19d..4fec1f9142b8 100644
--- a/drivers/clk/qcom/kpss-xcc.c
+++ b/drivers/clk/qcom/kpss-xcc.c
@@ -33,7 +33,6 @@ static int kpss_xcc_driver_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *id;
 	struct clk *clk;
-	struct resource *res;
 	void __iomem *base;
 	const char *name;
 
@@ -41,8 +40,7 @@ static int kpss_xcc_driver_probe(struct platform_device *pdev)
 	if (!id)
 		return -ENODEV;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(&pdev->dev, res);
+	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-- 
2.17.1

[PATCH 03/17] clk: samsung: convert to devm_platform_ioremap_resource

[Yangtao Li]

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 drivers/clk/samsung/clk-exynos-audss.c   | 4 +---
 drivers/clk/samsung/clk-exynos4412-isp.c | 4 +---
 drivers/clk/samsung/clk-exynos5433.c     | 4 +---
 drivers/clk/samsung/clk-s5pv210-audss.c  | 4 +---
 4 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos-audss.c b/drivers/clk/samsung/clk-exynos-audss.c
index 42b5d32c6cc7..9cc127a162ad 100644
--- a/drivers/clk/samsung/clk-exynos-audss.c
+++ b/drivers/clk/samsung/clk-exynos-audss.c
@@ -129,7 +129,6 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 	struct clk *pll_ref, *pll_in, *cdclk, *sclk_audio, *sclk_pcm_in;
 	const struct exynos_audss_clk_drvdata *variant;
 	struct clk_hw **clk_table;
-	struct resource *res;
 	struct device *dev = &pdev->dev;
 	int i, ret = 0;
 
@@ -137,8 +136,7 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 	if (!variant)
 		return -EINVAL;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg_base = devm_ioremap_resource(dev, res);
+	reg_base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg_base))
 		return PTR_ERR(reg_base);
 
diff --git a/drivers/clk/samsung/clk-exynos4412-isp.c b/drivers/clk/samsung/clk-exynos4412-isp.c
index 4b9e73608c21..20f5129f7212 100644
--- a/drivers/clk/samsung/clk-exynos4412-isp.c
+++ b/drivers/clk/samsung/clk-exynos4412-isp.c
@@ -110,11 +110,9 @@ static int __init exynos4x12_isp_clk_probe(struct platform_device *pdev)
 	struct samsung_clk_provider *ctx;
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
-	struct resource *res;
 	void __iomem *reg_base;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg_base = devm_ioremap_resource(dev, res);
+	reg_base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg_base)) {
 		dev_err(dev, "failed to map registers\n");
 		return PTR_ERR(reg_base);
diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c
index 4b1aa9382ad2..b0d48cddfd6e 100644
--- a/drivers/clk/samsung/clk-exynos5433.c
+++ b/drivers/clk/samsung/clk-exynos5433.c
@@ -5557,7 +5557,6 @@ static int __init exynos5433_cmu_probe(struct platform_device *pdev)
 	struct exynos5433_cmu_data *data;
 	struct samsung_clk_provider *ctx;
 	struct device *dev = &pdev->dev;
-	struct resource *res;
 	void __iomem *reg_base;
 	int i;
 
@@ -5570,8 +5569,7 @@ static int __init exynos5433_cmu_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	ctx = &data->ctx;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg_base = devm_ioremap_resource(dev, res);
+	reg_base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg_base))
 		return PTR_ERR(reg_base);
 
diff --git a/drivers/clk/samsung/clk-s5pv210-audss.c b/drivers/clk/samsung/clk-s5pv210-audss.c
index 14985ebd043b..503bd8a10d8f 100644
--- a/drivers/clk/samsung/clk-s5pv210-audss.c
+++ b/drivers/clk/samsung/clk-s5pv210-audss.c
@@ -63,15 +63,13 @@ static struct syscore_ops s5pv210_audss_clk_syscore_ops = {
 static int s5pv210_audss_clk_probe(struct platform_device *pdev)
 {
 	int i, ret = 0;
-	struct resource *res;
 	const char *mout_audss_p[2];
 	const char *mout_i2s_p[3];
 	const char *hclk_p;
 	struct clk_hw **clk_table;
 	struct clk *hclk, *pll_ref, *pll_in, *cdclk, *sclk_audio;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg_base = devm_ioremap_resource(&pdev->dev, res);
+	reg_base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg_base)) {
 		dev_err(&pdev->dev, "failed to map audss registers\n");
 		return PTR_ERR(reg_base);
-- 
2.17.1

Re: [PATCH 03/17] clk: samsung: convert to devm_platform_ioremap_resource

[Chanwoo Choi]

Hi,

On 12/10/19 4:57 AM, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
> 
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> ---
>  drivers/clk/samsung/clk-exynos-audss.c   | 4 +---
>  drivers/clk/samsung/clk-exynos4412-isp.c | 4 +---
>  drivers/clk/samsung/clk-exynos5433.c     | 4 +---
>  drivers/clk/samsung/clk-s5pv210-audss.c  | 4 +---
>  4 files changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/clk/samsung/clk-exynos-audss.c b/drivers/clk/samsung/clk-exynos-audss.c
> index 42b5d32c6cc7..9cc127a162ad 100644
> --- a/drivers/clk/samsung/clk-exynos-audss.c
> +++ b/drivers/clk/samsung/clk-exynos-audss.c
> @@ -129,7 +129,6 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
>  	struct clk *pll_ref, *pll_in, *cdclk, *sclk_audio, *sclk_pcm_in;
>  	const struct exynos_audss_clk_drvdata *variant;
>  	struct clk_hw **clk_table;
> -	struct resource *res;
>  	struct device *dev = &pdev->dev;
>  	int i, ret = 0;
>  
> @@ -137,8 +136,7 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
>  	if (!variant)
>  		return -EINVAL;
>  
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	reg_base = devm_ioremap_resource(dev, res);
> +	reg_base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(reg_base))
>  		return PTR_ERR(reg_base);
>  
> diff --git a/drivers/clk/samsung/clk-exynos4412-isp.c b/drivers/clk/samsung/clk-exynos4412-isp.c
> index 4b9e73608c21..20f5129f7212 100644
> --- a/drivers/clk/samsung/clk-exynos4412-isp.c
> +++ b/drivers/clk/samsung/clk-exynos4412-isp.c
> @@ -110,11 +110,9 @@ static int __init exynos4x12_isp_clk_probe(struct platform_device *pdev)
>  	struct samsung_clk_provider *ctx;
>  	struct device *dev = &pdev->dev;
>  	struct device_node *np = dev->of_node;
> -	struct resource *res;
>  	void __iomem *reg_base;
>  
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	reg_base = devm_ioremap_resource(dev, res);
> +	reg_base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(reg_base)) {
>  		dev_err(dev, "failed to map registers\n");
>  		return PTR_ERR(reg_base);
> diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c
> index 4b1aa9382ad2..b0d48cddfd6e 100644
> --- a/drivers/clk/samsung/clk-exynos5433.c
> +++ b/drivers/clk/samsung/clk-exynos5433.c
> @@ -5557,7 +5557,6 @@ static int __init exynos5433_cmu_probe(struct platform_device *pdev)
>  	struct exynos5433_cmu_data *data;
>  	struct samsung_clk_provider *ctx;
>  	struct device *dev = &pdev->dev;
> -	struct resource *res;
>  	void __iomem *reg_base;
>  	int i;
>  
> @@ -5570,8 +5569,7 @@ static int __init exynos5433_cmu_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	ctx = &data->ctx;
>  
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	reg_base = devm_ioremap_resource(dev, res);
> +	reg_base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(reg_base))
>  		return PTR_ERR(reg_base);
>  
> diff --git a/drivers/clk/samsung/clk-s5pv210-audss.c b/drivers/clk/samsung/clk-s5pv210-audss.c
> index 14985ebd043b..503bd8a10d8f 100644
> --- a/drivers/clk/samsung/clk-s5pv210-audss.c
> +++ b/drivers/clk/samsung/clk-s5pv210-audss.c
> @@ -63,15 +63,13 @@ static struct syscore_ops s5pv210_audss_clk_syscore_ops = {
>  static int s5pv210_audss_clk_probe(struct platform_device *pdev)
>  {
>  	int i, ret = 0;
> -	struct resource *res;
>  	const char *mout_audss_p[2];
>  	const char *mout_i2s_p[3];
>  	const char *hclk_p;
>  	struct clk_hw **clk_table;
>  	struct clk *hclk, *pll_ref, *pll_in, *cdclk, *sclk_audio;
>  
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	reg_base = devm_ioremap_resource(&pdev->dev, res);
> +	reg_base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(reg_base)) {
>  		dev_err(&pdev->dev, "failed to map audss registers\n");
>  		return PTR_ERR(reg_base);
> 

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

[PATCH 04/17] clk: mediatek: convert to devm_platform_ioremap_resource

[Yangtao Li]

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 drivers/clk/mediatek/clk-mt2701.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt2701.c b/drivers/clk/mediatek/clk-mt2701.c
index 695be0f77427..a6aea6646118 100644
--- a/drivers/clk/mediatek/clk-mt2701.c
+++ b/drivers/clk/mediatek/clk-mt2701.c
@@ -668,9 +668,8 @@ static int mtk_topckgen_init(struct platform_device *pdev)
 	struct clk_onecell_data *clk_data;
 	void __iomem *base;
 	struct device_node *node = pdev->dev.of_node;
-	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
-	base = devm_ioremap_resource(&pdev->dev, res);
+	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
@@ -889,9 +888,8 @@ static int mtk_pericfg_init(struct platform_device *pdev)
 	void __iomem *base;
 	int r;
 	struct device_node *node = pdev->dev.of_node;
-	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
-	base = devm_ioremap_resource(&pdev->dev, res);
+	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-- 
2.17.1

[PATCH 05/17] clk: hisilicon: convert to devm_platform_ioremap_resource

[Yangtao Li]

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 drivers/clk/hisilicon/clk-hi3660-stub.c | 6 +-----
 drivers/clk/hisilicon/clk.c             | 7 +------
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/hisilicon/clk-hi3660-stub.c b/drivers/clk/hisilicon/clk-hi3660-stub.c
index 3a653d54bee0..7e44d6e2a307 100644
--- a/drivers/clk/hisilicon/clk-hi3660-stub.c
+++ b/drivers/clk/hisilicon/clk-hi3660-stub.c
@@ -124,7 +124,6 @@ static struct clk_hw *hi3660_stub_clk_hw_get(struct of_phandle_args *clkspec,
 static int hi3660_stub_clk_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct resource *res;
 	unsigned int i;
 	int ret;
 
@@ -139,10 +138,7 @@ static int hi3660_stub_clk_probe(struct platform_device *pdev)
 	if (IS_ERR(stub_clk_chan.mbox))
 		return PTR_ERR(stub_clk_chan.mbox);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
-		return -EINVAL;
-	freq_reg = devm_ioremap(dev, res->start, resource_size(res));
+	freq_reg = devm_platform_ioremap_resource(pdev, 0);
 	if (!freq_reg)
 		return -ENOMEM;
 
diff --git a/drivers/clk/hisilicon/clk.c b/drivers/clk/hisilicon/clk.c
index 54d9fdc93599..8c521495d9e5 100644
--- a/drivers/clk/hisilicon/clk.c
+++ b/drivers/clk/hisilicon/clk.c
@@ -27,18 +27,13 @@ struct hisi_clock_data *hisi_clk_alloc(struct platform_device *pdev,
 						int nr_clks)
 {
 	struct hisi_clock_data *clk_data;
-	struct resource *res;
 	struct clk **clk_table;
 
 	clk_data = devm_kmalloc(&pdev->dev, sizeof(*clk_data), GFP_KERNEL);
 	if (!clk_data)
 		return NULL;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
-		return NULL;
-	clk_data->base = devm_ioremap(&pdev->dev,
-				res->start, resource_size(res));
+	clk_data->base = devm_platform_ioremap_resource(pdev, 0);
 	if (!clk_data->base)
 		return NULL;
 
-- 
2.17.1

[PATCH 06/17] clk: tegra: convert to devm_platform_ioremap_resource

[Yangtao Li]

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 drivers/clk/tegra/clk-dfll.c | 34 ++++------------------------------
 1 file changed, 4 insertions(+), 30 deletions(-)

diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
index c051d92c2bbf..070a2957e119 100644
--- a/drivers/clk/tegra/clk-dfll.c
+++ b/drivers/clk/tegra/clk-dfll.c
@@ -1935,7 +1935,6 @@ static int dfll_fetch_common_params(struct tegra_dfll *td)
 int tegra_dfll_register(struct platform_device *pdev,
 			struct tegra_dfll_soc_data *soc)
 {
-	struct resource *mem;
 	struct tegra_dfll *td;
 	int ret;
 
@@ -1985,51 +1984,26 @@ int tegra_dfll_register(struct platform_device *pdev,
 		return ret;
 	}
 
-	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!mem) {
-		dev_err(td->dev, "no control register resource\n");
-		return -ENODEV;
-	}
-
-	td->base = devm_ioremap(td->dev, mem->start, resource_size(mem));
+	td->base = devm_platform_ioremap_resource(pdev, 0);
 	if (!td->base) {
 		dev_err(td->dev, "couldn't ioremap DFLL control registers\n");
 		return -ENODEV;
 	}
 
-	mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	if (!mem) {
-		dev_err(td->dev, "no i2c_base resource\n");
-		return -ENODEV;
-	}
-
-	td->i2c_base = devm_ioremap(td->dev, mem->start, resource_size(mem));
+	td->i2c_base = devm_platform_ioremap_resource(pdev, 1);
 	if (!td->i2c_base) {
 		dev_err(td->dev, "couldn't ioremap i2c_base resource\n");
 		return -ENODEV;
 	}
 
-	mem = platform_get_resource(pdev, IORESOURCE_MEM, 2);
-	if (!mem) {
-		dev_err(td->dev, "no i2c_controller_base resource\n");
-		return -ENODEV;
-	}
-
-	td->i2c_controller_base = devm_ioremap(td->dev, mem->start,
-					       resource_size(mem));
+	td->i2c_controller_base = devm_platform_ioremap_resource(pdev, 2);
 	if (!td->i2c_controller_base) {
 		dev_err(td->dev,
 			"couldn't ioremap i2c_controller_base resource\n");
 		return -ENODEV;
 	}
 
-	mem = platform_get_resource(pdev, IORESOURCE_MEM, 3);
-	if (!mem) {
-		dev_err(td->dev, "no lut_base resource\n");
-		return -ENODEV;
-	}
-
-	td->lut_base = devm_ioremap(td->dev, mem->start, resource_size(mem));
+	td->lut_base = devm_platform_ioremap_resource(pdev, 3);
 	if (!td->lut_base) {
 		dev_err(td->dev,
 			"couldn't ioremap lut_base resource\n");
-- 
2.17.1

Re: [PATCH 06/17] clk: tegra: convert to devm_platform_ioremap_resource

[Peter De Schrijver]

On Mon, Dec 09, 2019 at 07:57:38PM +0000, Yangtao Li wrote:
> External email: Use caution opening links or attachments
> 
> 

Acked-by:  Peter De Schrijver <pdeschrijver@nvidia.com>

> Use devm_platform_ioremap_resource() to simplify code.
> 
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> ---
>  drivers/clk/tegra/clk-dfll.c | 34 ++++------------------------------
>  1 file changed, 4 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
> index c051d92c2bbf..070a2957e119 100644
> --- a/drivers/clk/tegra/clk-dfll.c
> +++ b/drivers/clk/tegra/clk-dfll.c
> @@ -1935,7 +1935,6 @@ static int dfll_fetch_common_params(struct tegra_dfll *td)
>  int tegra_dfll_register(struct platform_device *pdev,
>                         struct tegra_dfll_soc_data *soc)
>  {
> -       struct resource *mem;
>         struct tegra_dfll *td;
>         int ret;
> 
> @@ -1985,51 +1984,26 @@ int tegra_dfll_register(struct platform_device *pdev,
>                 return ret;
>         }
> 
> -       mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -       if (!mem) {
> -               dev_err(td->dev, "no control register resource\n");
> -               return -ENODEV;
> -       }
> -
> -       td->base = devm_ioremap(td->dev, mem->start, resource_size(mem));
> +       td->base = devm_platform_ioremap_resource(pdev, 0);
>         if (!td->base) {
>                 dev_err(td->dev, "couldn't ioremap DFLL control registers\n");
>                 return -ENODEV;
>         }
> 
> -       mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> -       if (!mem) {
> -               dev_err(td->dev, "no i2c_base resource\n");
> -               return -ENODEV;
> -       }
> -
> -       td->i2c_base = devm_ioremap(td->dev, mem->start, resource_size(mem));
> +       td->i2c_base = devm_platform_ioremap_resource(pdev, 1);
>         if (!td->i2c_base) {
>                 dev_err(td->dev, "couldn't ioremap i2c_base resource\n");
>                 return -ENODEV;
>         }
> 
> -       mem = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> -       if (!mem) {
> -               dev_err(td->dev, "no i2c_controller_base resource\n");
> -               return -ENODEV;
> -       }
> -
> -       td->i2c_controller_base = devm_ioremap(td->dev, mem->start,
> -                                              resource_size(mem));
> +       td->i2c_controller_base = devm_platform_ioremap_resource(pdev, 2);
>         if (!td->i2c_controller_base) {
>                 dev_err(td->dev,
>                         "couldn't ioremap i2c_controller_base resource\n");
>                 return -ENODEV;
>         }
> 
> -       mem = platform_get_resource(pdev, IORESOURCE_MEM, 3);
> -       if (!mem) {
> -               dev_err(td->dev, "no lut_base resource\n");
> -               return -ENODEV;
> -       }
> -
> -       td->lut_base = devm_ioremap(td->dev, mem->start, resource_size(mem));
> +       td->lut_base = devm_platform_ioremap_resource(pdev, 3);
>         if (!td->lut_base) {
>                 dev_err(td->dev,
>                         "couldn't ioremap lut_base resource\n");
> --
> 2.17.1
> 

[PATCH 07/17] clk: mvebu: convert to devm_platform_ioremap_resource

[Yangtao Li]

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 drivers/clk/mvebu/armada-37xx-periph.c | 4 +---
 drivers/clk/mvebu/armada-37xx-tbg.c    | 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/mvebu/armada-37xx-periph.c b/drivers/clk/mvebu/armada-37xx-periph.c
index f5746f9ea929..0d03878920ac 100644
--- a/drivers/clk/mvebu/armada-37xx-periph.c
+++ b/drivers/clk/mvebu/armada-37xx-periph.c
@@ -725,7 +725,6 @@ static int armada_3700_periph_clock_probe(struct platform_device *pdev)
 	const struct clk_periph_data *data;
 	struct device *dev = &pdev->dev;
 	int num_periph = 0, i, ret;
-	struct resource *res;
 
 	data = of_device_get_match_data(dev);
 	if (!data)
@@ -746,8 +745,7 @@ static int armada_3700_periph_clock_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	driver_data->hw_data->num = num_periph;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	driver_data->reg = devm_ioremap_resource(dev, res);
+	driver_data->reg = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(driver_data->reg))
 		return PTR_ERR(driver_data->reg);
 
diff --git a/drivers/clk/mvebu/armada-37xx-tbg.c b/drivers/clk/mvebu/armada-37xx-tbg.c
index 585a02e0b330..5d86912fbb6e 100644
--- a/drivers/clk/mvebu/armada-37xx-tbg.c
+++ b/drivers/clk/mvebu/armada-37xx-tbg.c
@@ -84,7 +84,6 @@ static int armada_3700_tbg_clock_probe(struct platform_device *pdev)
 	struct clk_hw_onecell_data *hw_tbg_data;
 	struct device *dev = &pdev->dev;
 	const char *parent_name;
-	struct resource *res;
 	struct clk *parent;
 	void __iomem *reg;
 	int i, ret;
@@ -105,8 +104,7 @@ static int armada_3700_tbg_clock_probe(struct platform_device *pdev)
 	parent_name = __clk_get_name(parent);
 	clk_put(parent);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(dev, res);
+	reg = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
-- 
2.17.1

[PATCH 08/17] clk: imx: convert to devm_platform_ioremap_resource

[Yangtao Li]

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 drivers/clk/imx/clk-imx8qxp-lpcg.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8qxp-lpcg.c b/drivers/clk/imx/clk-imx8qxp-lpcg.c
index c0aff7ca6374..10ae712447c6 100644
--- a/drivers/clk/imx/clk-imx8qxp-lpcg.c
+++ b/drivers/clk/imx/clk-imx8qxp-lpcg.c
@@ -164,7 +164,6 @@ static int imx8qxp_lpcg_clk_probe(struct platform_device *pdev)
 	struct clk_hw_onecell_data *clk_data;
 	const struct imx8qxp_ss_lpcg *ss_lpcg;
 	const struct imx8qxp_lpcg_data *lpcg;
-	struct resource *res;
 	struct clk_hw **clks;
 	void __iomem *base;
 	int i;
@@ -173,10 +172,7 @@ static int imx8qxp_lpcg_clk_probe(struct platform_device *pdev)
 	if (!ss_lpcg)
 		return -ENODEV;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
-		return -EINVAL;
-	base = devm_ioremap(dev, res->start, resource_size(res));
+	base = devm_platform_ioremap_resource(pdev, 0);
 	if (!base)
 		return -ENOMEM;
 
-- 
2.17.1

Re: [PATCH 08/17] clk: imx: convert to devm_platform_ioremap_resource

[Leonard Crestez]

On 09.12.2019 21:58, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
> 
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> ---
>   drivers/clk/imx/clk-imx8qxp-lpcg.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-imx8qxp-lpcg.c b/drivers/clk/imx/clk-imx8qxp-lpcg.c
> index c0aff7ca6374..10ae712447c6 100644
> --- a/drivers/clk/imx/clk-imx8qxp-lpcg.c
> +++ b/drivers/clk/imx/clk-imx8qxp-lpcg.c
> @@ -164,7 +164,6 @@ static int imx8qxp_lpcg_clk_probe(struct platform_device *pdev)
>   	struct clk_hw_onecell_data *clk_data;
>   	const struct imx8qxp_ss_lpcg *ss_lpcg;
>   	const struct imx8qxp_lpcg_data *lpcg;
> -	struct resource *res;
>   	struct clk_hw **clks;
>   	void __iomem *base;
>   	int i;
> @@ -173,10 +172,7 @@ static int imx8qxp_lpcg_clk_probe(struct platform_device *pdev)
>   	if (!ss_lpcg)
>   		return -ENODEV;
>   
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (!res)
> -		return -EINVAL;
> -	base = devm_ioremap(dev, res->start, resource_size(res));
> +	base = devm_platform_ioremap_resource(pdev, 0);
>   	if (!base)
>   		return -ENOMEM;

This breaks imx8qxp-mek boot by causing most peripherals (like uart) to 
fail to probe.

The old and new paths are not equivalent: devm_platform_ioremap_resource
calls devm_ioremap_resource which differs from devm_ioremap by also 
calling devm_request_mem_region.

This prevents other mappings in the area and imx8qxp-lpcg nodes map 
whole hardware "subsystems" and overlap most peripherals. For example:

                  adma_lpcg: clock-controller@59000000 {
                          compatible = "fsl,imx8qxp-lpcg-adma";
                          reg = <0x59000000 0x2000000>;
                          #clock-cells = <1>;
		};

                 adma_lpuart0: serial@5a060000 {
			reg = <0x5a060000 0x1000>;
			...
		};

I don't know if this issue affects any other platforms (imx8 lpcg 
bindings are unusual) but if you found this with an automated tool 
perhaps it should be adjusted?

By my count it's the 4th time this incorrect cleanup was posted.

Previously: https://lkml.org/lkml/2019/12/4/487

--
Regards,
Leonard

Re: [PATCH 08/17] clk: imx: convert to devm_platform_ioremap_resource

[Thierry Reding]

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

On Mon, Dec 09, 2019 at 08:44:39PM +0000, Leonard Crestez wrote:
> On 09.12.2019 21:58, Yangtao Li wrote:
> > Use devm_platform_ioremap_resource() to simplify code.
> > 
> > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> > ---
> >   drivers/clk/imx/clk-imx8qxp-lpcg.c | 6 +-----
> >   1 file changed, 1 insertion(+), 5 deletions(-)
> > 
> > diff --git a/drivers/clk/imx/clk-imx8qxp-lpcg.c b/drivers/clk/imx/clk-imx8qxp-lpcg.c
> > index c0aff7ca6374..10ae712447c6 100644
> > --- a/drivers/clk/imx/clk-imx8qxp-lpcg.c
> > +++ b/drivers/clk/imx/clk-imx8qxp-lpcg.c
> > @@ -164,7 +164,6 @@ static int imx8qxp_lpcg_clk_probe(struct platform_device *pdev)
> >   	struct clk_hw_onecell_data *clk_data;
> >   	const struct imx8qxp_ss_lpcg *ss_lpcg;
> >   	const struct imx8qxp_lpcg_data *lpcg;
> > -	struct resource *res;
> >   	struct clk_hw **clks;
> >   	void __iomem *base;
> >   	int i;
> > @@ -173,10 +172,7 @@ static int imx8qxp_lpcg_clk_probe(struct platform_device *pdev)
> >   	if (!ss_lpcg)
> >   		return -ENODEV;
> >   
> > -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > -	if (!res)
> > -		return -EINVAL;
> > -	base = devm_ioremap(dev, res->start, resource_size(res));
> > +	base = devm_platform_ioremap_resource(pdev, 0);
> >   	if (!base)
> >   		return -ENOMEM;
> 
> This breaks imx8qxp-mek boot by causing most peripherals (like uart) to 
> fail to probe.
> 
> The old and new paths are not equivalent: devm_platform_ioremap_resource
> calls devm_ioremap_resource which differs from devm_ioremap by also 
> calling devm_request_mem_region.
> 
> This prevents other mappings in the area and imx8qxp-lpcg nodes map 
> whole hardware "subsystems" and overlap most peripherals. For example:
> 
>                   adma_lpcg: clock-controller@59000000 {
>                           compatible = "fsl,imx8qxp-lpcg-adma";
>                           reg = <0x59000000 0x2000000>;
>                           #clock-cells = <1>;
> 		};
> 
>                  adma_lpuart0: serial@5a060000 {
> 			reg = <0x5a060000 0x1000>;
> 			...
> 		};

The whole point of doing a request_mem_region() is to avoid having
multiple drivers trample on each others' mappings. What you do above
doesn't look right. Why does that clock controller need access to 32
MiB of I/O memory space?

That said, there are legitimate reasons for sharing mappings across
drivers, so I agree that automated conversions like this should be done
very carefully. The difficulty is that there are cases where drivers
simply omitted that request_mem_region() by mistake and where the
conversion can be correct (and in fact an improvement), but we can't
make the assumption blindly.

Thierry

> I don't know if this issue affects any other platforms (imx8 lpcg 
> bindings are unusual) but if you found this with an automated tool 
> perhaps it should be adjusted?
> 
> By my count it's the 4th time this incorrect cleanup was posted.
> 
> Previously: https://lkml.org/lkml/2019/12/4/487
> 
> --
> Regards,
> Leonard

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

Re: [PATCH 08/17] clk: imx: convert to devm_platform_ioremap_resource

[Andreas Färber]

Hi Thierry,

Am 10.12.19 um 14:21 schrieb Thierry Reding:
> On Mon, Dec 09, 2019 at 08:44:39PM +0000, Leonard Crestez wrote:
>> On 09.12.2019 21:58, Yangtao Li wrote:
>>> Use devm_platform_ioremap_resource() to simplify code.
>>>
>>> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
>>> ---
>>>   drivers/clk/imx/clk-imx8qxp-lpcg.c | 6 +-----
>>>   1 file changed, 1 insertion(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/clk/imx/clk-imx8qxp-lpcg.c b/drivers/clk/imx/clk-imx8qxp-lpcg.c
>>> index c0aff7ca6374..10ae712447c6 100644
>>> --- a/drivers/clk/imx/clk-imx8qxp-lpcg.c
>>> +++ b/drivers/clk/imx/clk-imx8qxp-lpcg.c
>>> @@ -164,7 +164,6 @@ static int imx8qxp_lpcg_clk_probe(struct platform_device *pdev)
>>>   	struct clk_hw_onecell_data *clk_data;
>>>   	const struct imx8qxp_ss_lpcg *ss_lpcg;
>>>   	const struct imx8qxp_lpcg_data *lpcg;
>>> -	struct resource *res;
>>>   	struct clk_hw **clks;
>>>   	void __iomem *base;
>>>   	int i;
>>> @@ -173,10 +172,7 @@ static int imx8qxp_lpcg_clk_probe(struct platform_device *pdev)
>>>   	if (!ss_lpcg)
>>>   		return -ENODEV;
>>>   
>>> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> -	if (!res)
>>> -		return -EINVAL;
>>> -	base = devm_ioremap(dev, res->start, resource_size(res));
>>> +	base = devm_platform_ioremap_resource(pdev, 0);
>>>   	if (!base)
>>>   		return -ENOMEM;
>> This breaks imx8qxp-mek boot by causing most peripherals (like uart) to 
>> fail to probe.
>>
>> The old and new paths are not equivalent: devm_platform_ioremap_resource
>> calls devm_ioremap_resource which differs from devm_ioremap by also 
>> calling devm_request_mem_region.
>>
>> This prevents other mappings in the area and imx8qxp-lpcg nodes map 
>> whole hardware "subsystems" and overlap most peripherals. For example:
>>
>>                   adma_lpcg: clock-controller@59000000 {
>>                           compatible = "fsl,imx8qxp-lpcg-adma";
>>                           reg = <0x59000000 0x2000000>;
>>                           #clock-cells = <1>;
>> 		};
>>
>>                  adma_lpuart0: serial@5a060000 {
>> 			reg = <0x5a060000 0x1000>;
>> 			...
>> 		};
> The whole point of doing a request_mem_region() is to avoid having
> multiple drivers trample on each others' mappings. What you do above
> doesn't look right. Why does that clock controller need access to 32
> MiB of I/O memory space?

I have similar cases with Realtek where registers are simply not grouped
into convenient blocks but spread across large memory regions.

Also, Fabien and I had a review discussion about the very same topic of
suggesting these functions which implicitly do a request_mem_region(),
so this might need some better documentation for raising awareness?

For some cases I posted patches to convert those to syscon / simple-mfd,
but clk in particular is a difficult one: clks still express their
parents by names rather than pointers, and having clks spread across
drivers has implications on needing to expose non-leaf clocks in the DT
bindings, as well as needing to mess with __clk_get_name() for actually
getting the name to use as parent when obtaining the clk from DT.
Imagine a lonely clk gate register and the PLLs or other gates they are
supplied from residing far away. It's really ugly either way...

That said, the Actions Semi patch 16/17 that I'm CC'ed for looks okay.

Regards,
Andreas

> 
> That said, there are legitimate reasons for sharing mappings across
> drivers, so I agree that automated conversions like this should be done
> very carefully. The difficulty is that there are cases where drivers
> simply omitted that request_mem_region() by mistake and where the
> conversion can be correct (and in fact an improvement), but we can't
> make the assumption blindly.
> 
> Thierry
> 
>> I don't know if this issue affects any other platforms (imx8 lpcg 
>> bindings are unusual) but if you found this with an automated tool 
>> perhaps it should be adjusted?
>>
>> By my count it's the 4th time this incorrect cleanup was posted.
>>
>> Previously: https://lkml.org/lkml/2019/12/4/487
>>
>> --
>> Regards,
>> Leonard


-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

Re: [PATCH 08/17] clk: imx: convert to devm_platform_ioremap_resource

[mripard]

On Tue, Dec 10, 2019 at 03:57:50PM +0100, Andreas Färber wrote:
> For some cases I posted patches to convert those to syscon / simple-mfd,
> but clk in particular is a difficult one: clks still express their
> parents by names rather than pointers,

It's no longer the case since 5.2. clk_init_data can now take either
the parent name, a clk_hw, or one of the names in clock-names.

maxime

Re: [PATCH 08/17] clk: imx: convert to devm_platform_ioremap_resource

[Paul Walmsley]

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

On Tue, 10 Dec 2019, Andreas Färber wrote:

> I have similar cases with Realtek where registers are simply not grouped
> into convenient blocks but spread across large memory regions.

At the hardware level, registers are grouped into IP blocks, to simply 
both design integration and address decoding.  Not knowing which Realtek 
device you're referring to, most likely it's the same situation as with 
the IMX8M TRM, where the DT data doesn't match the underlying reality of 
the hardware.  In those cases the best approach is usually to just fix the 
DT data.


- Paul

Re: [PATCH 08/17] clk: imx: convert to devm_platform_ioremap_resource

[Andreas Färber]

Am 11.12.19 um 18:57 schrieb Paul Walmsley:
> On Tue, 10 Dec 2019, Andreas Färber wrote:
> 
>> I have similar cases with Realtek where registers are simply not grouped
>> into convenient blocks but spread across large memory regions.
> 
> At the hardware level, registers are grouped into IP blocks, to simply 
> both design integration and address decoding.

Reality shows, not all vendors/chips always care about simplification.

Blocks do have names, but they don't always group registers of the same
kind, as Linux expects it - be it for historical backwards compatibility
reasons or because Linux/Android wasn't their main use case in the past.
Firmware developers won't care where their registers are located.

>  Not knowing which Realtek 
> device you're referring to,

Arm based RTD1195 and RTD1295/RTD1395/RTD1619/RTD1319 SoC families,
which I maintain.

> most likely it's the same situation as with 
> the IMX8M TRM, where the DT data doesn't match the underlying reality of 
> the hardware.  In those cases the best approach is usually to just fix the 
> DT data.

No, you're not reading me. My DT data matches the hardware as far as I
know it. You can be really happy that you can login to get NXP manuals;
for other vendors, manuals simply don't exist and we have to deduce DT
from register names/offsets ourselves. Reality is messy!

Just please accept that hardware does not always allow for unique
contiguous memory reservations, and we therefore cannot force these
types of reservations onto everybody.

There might be an opportunity for a new helper with even longer name
that does the expected combination of actions. But is it worth it?
People seem to have stopped giving motivations for their patches in
commit message or cover letter, so it remains entirely unclear how else
one might satisfy the submitter's goals while keeping your code working.
(Also referring to unjustified style-only cleanups popping up lately.)
"to simplify code" is not much to go on, it sounds like a style cleanup
without any practical error avoidance benefits nor an API to be dropped.

Note that I did not receive any cover letter accompanying this patch,
but was CC'ed on plenty of other patches like this one that I'm not
maintainer of, leading me to assume that none was sent.

Alternatively one could do the reservations decoupled from DT inside the
driver, but again not using this suggested helper.

From what I read on other such patches, apparently some Coccinelle build
target emits warnings when it matches some pattern for potential
refactoring, which people then set out to resolve, without understanding
the code they touch or being able to actually test it. That's probably
the root cause that someone would need to tackle - whitelisting
fully-intentional usages of certain APIs to protect against unwarranted
refactorings, or otherwise making sure that people don't get inspired to
in their best intentions break other people's code. I assume kbuild bot
doesn't send out such cocci warnings to us maintainers for good reasons.

A completely fragmented DT with either dozens of reg entries for single
registers or distinct compatible strings for individual registers, to
give them their own DT nodes, is not really handy, compared to one or
two larger clk nodes that handle reg offsets under the hood, without
impacting public DT bindings (e.g., bumping reg's maxItems, clk header).

If you care about modeling this, you're welcome to participate in patch
review @ DTML/LAKML/LRSML. So far there's largely been a yawning silence
in response to my patches introducing syscon and simple-mfd as cleanups,
before things get worse as we add to the DT. Following an unreviewed clk
RFC of mine two years back, there's now been a clk patchset from Realtek
that got a load of review comments from me, waiting for a v2.

If you don't care, then please don't lecture us about how you think
other people's hardware should ideally be like. That's not helpful.

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

Re: [PATCH 08/17] clk: imx: convert to devm_platform_ioremap_resource

[Paul Walmsley]

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

On Wed, 11 Dec 2019, Andreas Färber wrote:

> Blocks do have names, but they don't always group registers of the same
> kind, as Linux expects it

Linux does not expect that all of the registers in the same IP block are 
of the same kind.  That's part of the reason why Linux frameworks exist.  
To consider clocks as the present example, you're welcome to register 
local IP block clock control registers in the local IP block driver via 
the clock framework.  There's no need for a separate clock driver with an 
overlapping address range, or anything like that.

This is nothing new with Realtek.  IP blocks that contain many different 
kinds of registers have had Linux driver support without requiring 
overlapping register address ranges long before Realtek ARM SoCs 
appeared.

> Just please accept that hardware does not always allow for unique 
> contiguous memory reservations

Hardware designs do in fact mandate unique contiguous memory reservations, 
otherwise address decoding would be indeterministic.  What they don't 
mandate is that all of the registers in that region be all of one kind. 
It's certainly possible to have an SoC with one giant IP block with all 
registers mixed together.  Even in that case, it is still incorrect to 
have multiple DT entries with overlapping register address ranges.

It sounds like you're thinking of the difficulties of figuring out how to 
structure the software driver support for those mixed IP block as a Linux 
driver: where it would fit in the tree, what frameworks it would need to 
register with, and who would maintain it.  Those issues certainly merit 
careful thought and consideration.  They aren't related to multiple 
overlapping address ranges.


- Paul

Re: [PATCH 08/17] clk: imx: convert to devm_platform_ioremap_resource

[Andreas Färber]

Am 11.12.19 um 23:49 schrieb Paul Walmsley:
> On Wed, 11 Dec 2019, Andreas Färber wrote:
> 
>> Blocks do have names, but they don't always group registers of the same
>> kind, as Linux expects it
> 
> Linux does not expect that all of the registers in the same IP block are 
> of the same kind.  That's part of the reason why Linux frameworks exist.  
> To consider clocks as the present example, you're welcome to register 
> local IP block clock control registers in the local IP block driver via 
> the clock framework.  There's no need for a separate clock driver with an 
> overlapping address range, or anything like that.

If I throw random code into drivers/mfd/ it will not get proper review.

We rely on clk drivers going into drivers/clk/, even if I could
theoretically register clks also from other parts of the code base -
which will then require complex Kconfig dependencies or #ifdef'ery, not
to mention the nightmares of collecting Acks and figuring out through
whose tree which patches go.

> 
> This is nothing new with Realtek.

As this NXP patch proves. :)

>  IP blocks that contain many different 
> kinds of registers have had Linux driver support without requiring 
> overlapping register address ranges long before Realtek ARM SoCs 
> appeared.

Hey, you're the one that's trying to pin this on Realtek, not me!
STM32 RCC is another example I know, also Allwinner, etc. My point was
precisely that this is - for good or bad - a rather common scenario that
we need to deal with.

>> Just please accept that hardware does not always allow for unique 
>> contiguous memory reservations
> 
> Hardware designs do in fact mandate unique contiguous memory reservations,
> otherwise address decoding would be indeterministic.

Are you not understanding what I'm saying or intentionally gas-lighting?
A contiguous memory _reservation_ is a range of memory like <0xdead0000
0x100> that the kernel (software!) blocks other drivers (software!) from
reserving. This has _nothing_ to do with hardware address line decoding.
It's still about devm_platform_ioremap_resource() and related APIs. Do a
`cat /proc/iomem` to see, e.g., "98007800-9800781f : serial" reservation
in successful case; as mentioned by Leonard, an unsuccessful reservation
usually causes the driver to fail to probe and thus be unavailable.

>  What they don't 
> mandate is that all of the registers in that region be all of one kind. 
> It's certainly possible to have an SoC with one giant IP block with all 
> registers mixed together.  Even in that case, it is still incorrect to 
> have multiple DT entries with overlapping register address ranges.

Says who? Since when? Can we maybe agree that incorrect != invalid?

> It sounds like you're thinking of the difficulties of figuring out how to 
> structure the software driver support for those mixed IP block as a Linux 
> driver:

Yes, these are Linux kernel mailing lists and patches after all... I
don't design hardware, that's why I said we need to live with the flawed
reality of the actual hardware we get.

> where it would fit in the tree, what frameworks it would need to 
> register with, and who would maintain it.  Those issues certainly merit 
> careful thought and consideration.  They aren't related to multiple 
> overlapping address ranges.

Oh they are. Overlapping address ranges of DT nodes are a _result_ of
unexpected hardware design involving blocks not clearly separated the
same way as Linux subsystems (to distinguish from "frameworks") are.

The DT should describe the hardware blocks as they were designed, but on
the other hand, we need to describe it in a way that Linux drivers can
actually bind against the relevant parts and that those drivers can
operate efficiently. There is no ioremap-all-regs helper that I'm aware
of, for instance, as that would result in __iomem base addresses to be
stored per reg entry; compare that to just one for an overlapping range.

Example:

clk@f00 {
	reg = <0xf00 0x100>;
}

reset@f0f {
	reg = <0xf0c 0x4>;
};

This should be a valid DT example today, as long as the clk driver
doesn't mess with the reset register embedded within its range. In this
case they can't both reserve their ranges as they would mutually cause
each other to fail to probe, depending on probe order.

As I wrote, turning this into

clk@f00 {
	reg = <0xf00 0xc>, <0xf10 0xe0>;
};

reset@f0f {
	reg = <0xf0c 0x4>;
};

is helping no one and makes things much more complex, especially when
the number of carve-outs grows or is not predetermined, as I noted about
some of my cases. Thus I disagree with you about the overlapping ranges.

DT needs to be designed forward-looking rather than just around the
handful of registers we might read/write today, not just to relieve Rob
from excessive reviews.

My solution was to do

syscon@f00 {
	reg = <0xf00 0x100>;
	ranges = ...;

	clk@0 {
		reg = <0x0 0x100>;
	};

	reset@c {
		reg = <0xc 0x4>;
	};
};

https://patchwork.kernel.org/cover/11269453/
https://patchwork.kernel.org/cover/11269971/ (and more in my tree)

which clearly models the blocks and shares a syscon for most children,
other than pre-existing 8250 UART, I²C, etc. drivers using platform
helpers such as the one discussed here.

What we lose with syscon is reservations, i.e. /proc/iomem neither
showing the full syscon nor the drivers using parts of it, unless we
explicitly reserve the memory (syscon does the ioremap for us, so no
need for this devm_platform_ioremap_resource helper there).

Also please keep in mind that we actually want to get to the point where
new systems are booting and usable. At least in the Arm world we do have
hardware at plenty to boot Linux. Dying in DT-beauty then is
counter-productive; we also need to come to timely compromises for not
blocking other work. clk drivers don't need to be platform_drivers like
here and thus can coexist easier with other drivers (e.g., syscon
without child), but I clearly contradict the generality in which you
appear to rule out overlapping memory ranges, be it for siblings or for
parent/child.

Hiding overlaps in an mfd driver does not strike me as better than
openly declaring them - if the mfd components are not dynamic, then I
understood simple-mfd were the way to go, which requires some reg(s),
which then for convenience may overlap if there's no clear boundaries.

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

Re: [PATCH 08/17] clk: imx: convert to devm_platform_ioremap_resource

[Thierry Reding]

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

On Thu, Dec 12, 2019 at 06:38:31AM +0100, Andreas Färber wrote:
> Am 11.12.19 um 23:49 schrieb Paul Walmsley:
> > On Wed, 11 Dec 2019, Andreas Färber wrote:
> > 
> >> Blocks do have names, but they don't always group registers of the same
> >> kind, as Linux expects it
> > 
> > Linux does not expect that all of the registers in the same IP block are 
> > of the same kind.  That's part of the reason why Linux frameworks exist.  
> > To consider clocks as the present example, you're welcome to register 
> > local IP block clock control registers in the local IP block driver via 
> > the clock framework.  There's no need for a separate clock driver with an 
> > overlapping address range, or anything like that.
> 
> If I throw random code into drivers/mfd/ it will not get proper review.

I don't think that's what Paul was suggesting. MFD is something that can
help in these cases, but often isn't required.

> We rely on clk drivers going into drivers/clk/, even if I could
> theoretically register clks also from other parts of the code base -
> which will then require complex Kconfig dependencies or #ifdef'ery, not
> to mention the nightmares of collecting Acks and figuring out through
> whose tree which patches go.

That's a process issue and isn't actually that bad in my experience.

> > This is nothing new with Realtek.
> 
> As this NXP patch proves. :)
> 
> >  IP blocks that contain many different 
> > kinds of registers have had Linux driver support without requiring 
> > overlapping register address ranges long before Realtek ARM SoCs 
> > appeared.
> 
> Hey, you're the one that's trying to pin this on Realtek, not me!
> STM32 RCC is another example I know, also Allwinner, etc. My point was
> precisely that this is - for good or bad - a rather common scenario that
> we need to deal with.
> 
> >> Just please accept that hardware does not always allow for unique 
> >> contiguous memory reservations
> > 
> > Hardware designs do in fact mandate unique contiguous memory reservations,
> > otherwise address decoding would be indeterministic.
> 
> Are you not understanding what I'm saying or intentionally gas-lighting?
> A contiguous memory _reservation_ is a range of memory like <0xdead0000
> 0x100> that the kernel (software!) blocks other drivers (software!) from
> reserving. This has _nothing_ to do with hardware address line decoding.
> It's still about devm_platform_ioremap_resource() and related APIs. Do a
> `cat /proc/iomem` to see, e.g., "98007800-9800781f : serial" reservation
> in successful case; as mentioned by Leonard, an unsuccessful reservation
> usually causes the driver to fail to probe and thus be unavailable.

I think what Paul is saying here is that even in hardware you do in fact
have these contiguous address regions assigned for each block. This is
just because you need to have an address decoder somewhere that forwards
addresses from the CPU to the various IP blocks. These address decoders
work on contiguous ranges, so by definitions registers within the same
region go to the same IP block.

> >  What they don't 
> > mandate is that all of the registers in that region be all of one kind. 
> > It's certainly possible to have an SoC with one giant IP block with all 
> > registers mixed together.  Even in that case, it is still incorrect to 
> > have multiple DT entries with overlapping register address ranges.
> 
> Says who? Since when? Can we maybe agree that incorrect != invalid?

We always represent each hardware blocks with one device tree node.
Given the above, if you have multiple nodes with overlapping register
ranges, you're describing one hardware block with multiple nodes and
that's when these kinds of issues come up.

> > It sounds like you're thinking of the difficulties of figuring out how to 
> > structure the software driver support for those mixed IP block as a Linux 
> > driver:
> 
> Yes, these are Linux kernel mailing lists and patches after all... I
> don't design hardware, that's why I said we need to live with the flawed
> reality of the actual hardware we get.

I don't think the hardware is necessarily flawed. It might not be
structured in a way that reflects the Linux kernel's subsystem
structure, but that's quite common. That's not a problem to solve at the
device tree level. The device tree should describe the hardware. It's
the job of device drivers to make the hardware available to the kernel
and its subsystems.

> > where it would fit in the tree, what frameworks it would need to 
> > register with, and who would maintain it.  Those issues certainly merit 
> > careful thought and consideration.  They aren't related to multiple 
> > overlapping address ranges.
> 
> Oh they are. Overlapping address ranges of DT nodes are a _result_ of
> unexpected hardware design involving blocks not clearly separated the
> same way as Linux subsystems (to distinguish from "frameworks") are.
> 
> The DT should describe the hardware blocks as they were designed, but on
> the other hand, we need to describe it in a way that Linux drivers can
> actually bind against the relevant parts and that those drivers can
> operate efficiently.

That's exactly where the misconception is. DT should not at all be
concerned about the operating system's internal structuring. While I
think the way that Linux is structure is great, not all operating
systems may work the same way. Other operating systems may not have
separate frameworks for clocks and resets. So if you start writing
device trees with a specific operating system in mind, you're likely
going to end up with one device tree per device per operating system.
That's not what we want. One device tree per device, independent of
the operating system, that's the goal.

>                      There is no ioremap-all-regs helper that I'm aware
> of, for instance, as that would result in __iomem base addresses to be
> stored per reg entry; compare that to just one for an overlapping range.

> 
> Example:
> 
> clk@f00 {
> 	reg = <0xf00 0x100>;
> }
> 
> reset@f0f {
> 	reg = <0xf0c 0x4>;
> };
> 
> This should be a valid DT example today, as long as the clk driver
> doesn't mess with the reset register embedded within its range. In this
> case they can't both reserve their ranges as they would mutually cause
> each other to fail to probe, depending on probe order.

It's certainly not invalid DT from a syntax point of view. But that
doesn't necessarily mean that it's a correct description of the
hardware.

> 
> As I wrote, turning this into
> 
> clk@f00 {
> 	reg = <0xf00 0xc>, <0xf10 0xe0>;
> };
> 
> reset@f0f {
> 	reg = <0xf0c 0x4>;
> };
> 
> is helping no one and makes things much more complex, especially when
> the number of carve-outs grows or is not predetermined, as I noted about
> some of my cases. Thus I disagree with you about the overlapping ranges.

Again, you're trying to split the hardware up based on Linux' frameworks
rather than by the actual hardware blocks.

> DT needs to be designed forward-looking rather than just around the
> handful of registers we might read/write today, not just to relieve Rob
> from excessive reviews.
> 
> My solution was to do
> 
> syscon@f00 {
> 	reg = <0xf00 0x100>;
> 	ranges = ...;
> 
> 	clk@0 {
> 		reg = <0x0 0x100>;
> 	};
> 
> 	reset@c {
> 		reg = <0xc 0x4>;
> 	};
> };

That's starting to look more like it. But still, why even bother with
the separate clk and reset nodes? This block clearly provides both
controls for clocks and controls for resets. The fact that Linux has two
separate frameworks for these types of resources is secondary.

The right thing to do here is to have one driver for the whole block and
then register its device with both the clock and reset frameworks. That
way you don't need two separate nodes. You only have these nodes so that
you can get two separate devices that a clock driver and a reset driver
can bind to, respectively. There's nothing in the frameworks that would
prevent the same driver from registering to both frameworks.

In other words, it's perfectly fine to have a single device/driver be a
clock provider and reset provider at the same time.

> https://patchwork.kernel.org/cover/11269453/
> https://patchwork.kernel.org/cover/11269971/ (and more in my tree)
> 
> which clearly models the blocks and shares a syscon for most children,
> other than pre-existing 8250 UART, I²C, etc. drivers using platform
> helpers such as the one discussed here.
> 
> What we lose with syscon is reservations, i.e. /proc/iomem neither
> showing the full syscon nor the drivers using parts of it, unless we
> explicitly reserve the memory (syscon does the ioremap for us, so no
> need for this devm_platform_ioremap_resource helper there).
> 
> Also please keep in mind that we actually want to get to the point where
> new systems are booting and usable. At least in the Arm world we do have
> hardware at plenty to boot Linux. Dying in DT-beauty then is
> counter-productive; we also need to come to timely compromises for not
> blocking other work. clk drivers don't need to be platform_drivers like
> here and thus can coexist easier with other drivers (e.g., syscon
> without child), but I clearly contradict the generality in which you
> appear to rule out overlapping memory ranges, be it for siblings or for
> parent/child.

I think the common misconception that we need separate drivers for each
framework is what's counter-productive. If the same device provides
different types of resources, then we should just have one driver
register with whatever the frameworks are that expose these resources.
It becomes really quite simple once you shed that misconception.

Thierry

> Hiding overlaps in an mfd driver does not strike me as better than
> openly declaring them - if the mfd components are not dynamic, then I
> understood simple-mfd were the way to go, which requires some reg(s),
> which then for convenience may overlap if there's no clear boundaries.
> 
> Regards,
> Andreas
> 
> -- 
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer
> HRB 36809 (AG Nürnberg)

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

Re: [PATCH 08/17] clk: imx: convert to devm_platform_ioremap_resource

[Paul Walmsley]

+ Dong

On Tue, 10 Dec 2019, Thierry Reding wrote:

> On Mon, Dec 09, 2019 at 08:44:39PM +0000, Leonard Crestez wrote:
>
> > This breaks imx8qxp-mek boot by causing most peripherals (like uart) to 
> > fail to probe.
> > 
> > The old and new paths are not equivalent: devm_platform_ioremap_resource
> > calls devm_ioremap_resource which differs from devm_ioremap by also 
> > calling devm_request_mem_region.
> > 
> > This prevents other mappings in the area and imx8qxp-lpcg nodes map 
> > whole hardware "subsystems" and overlap most peripherals. For example:
> > 
> >                   adma_lpcg: clock-controller@59000000 {
> >                           compatible = "fsl,imx8qxp-lpcg-adma";
> >                           reg = <0x59000000 0x2000000>;
> >                           #clock-cells = <1>;
> > 		};
> > 
> >                  adma_lpuart0: serial@5a060000 {
> > 			reg = <0x5a060000 0x1000>;
> > 			...
> > 		};
> 
> The whole point of doing a request_mem_region() is to avoid having
> multiple drivers trample on each others' mappings. What you do above
> doesn't look right. Why does that clock controller need access to 32
> MiB of I/O memory space?

Thierry's right; your DT data looks broken.  Unfortunately the IMX8M TRM 
requires some sort of signup to download, so I can't easily read it, but 
based on files like this:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/imx/clk-imx8qxp-lpcg.h

it looks like what should be in the DT instead is either a large set of 
smaller register ranges, or a set of separate DT nodes/drivers: one per 
peripheral LPCG instance.  That should avoid the overlapping memory range 
issue.


- Paul

Re: [PATCH 08/17] clk: imx: convert to devm_platform_ioremap_resource

[Leonard Crestez]

On 11.12.2019 19:51, Paul Walmsley wrote:
> On Tue, 10 Dec 2019, Thierry Reding wrote:
>> On Mon, Dec 09, 2019 at 08:44:39PM +0000, Leonard Crestez wrote:
>>
>>> This breaks imx8qxp-mek boot by causing most peripherals (like uart) to
>>> fail to probe.
>>>
>>> The old and new paths are not equivalent: devm_platform_ioremap_resource
>>> calls devm_ioremap_resource which differs from devm_ioremap by also
>>> calling devm_request_mem_region.
>>>
>>> This prevents other mappings in the area and imx8qxp-lpcg nodes map
>>> whole hardware "subsystems" and overlap most peripherals. For example:
>>>
>>>                    adma_lpcg: clock-controller@59000000 {
>>>                            compatible = "fsl,imx8qxp-lpcg-adma";
>>>                            reg = <0x59000000 0x2000000>;
>>>                            #clock-cells = <1>;
>>> 		};
>>>
>>>                   adma_lpuart0: serial@5a060000 {
>>> 			reg = <0x5a060000 0x1000>;
>>> 			...
>>> 		};
>>
>> The whole point of doing a request_mem_region() is to avoid having
>> multiple drivers trample on each others' mappings. What you do above
>> doesn't look right. Why does that clock controller need access to 32
>> MiB of I/O memory space?
> 
> Thierry's right; your DT data looks broken.  Unfortunately the IMX8M TRM
> requires some sort of signup to download, so I can't easily read it, but
> based on files like this:
> 
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fdrivers%2Fclk%2Fimx%2Fclk-imx8qxp-lpcg.h&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7Cf0e68024888e4aed777e08d77e62c5df%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637116835018036168&amp;sdata=z70QUYwknXfvc4zhyOrT7dcLPq5eryv4U7HoGMzI4ZE%3D&amp;reserved=0
> 
> it looks like what should be in the DT instead is either a large set of
> smaller register ranges, or a set of separate DT nodes/drivers: one per
> peripheral LPCG instance.  That should avoid the overlapping memory range
> issue.

Yes, current imx8 clock DT bindings are definitely odd. I only objected 
to the cleanup because it breaks boot.

This series converts imx8 LPCG to use separate DT nodes for each 
peripheral LPCG instance and makes other improvements, it's just been 
stuck in review:

https://patchwork.kernel.org/cover/11248249/

Having a large number of clock-providing DT nodes is also odd but it 
closely aligns with SOC hardware architecture. The clock gating bits 
were deliberately split into many 64k areas so that access can be 
controlled for each peripheral using very coarse-grained access control 
methods.

For example hypervisors can control guest access on page boundaries but 
not to individual bits in a byte.

This design makes it easy to securely grant control for a peripheral and 
all of its clock bits to an M4 core or virtualization guest without 
additional layers of software indirection.

--
Regards,
Leonard

Re: [PATCH 08/17] clk: imx: convert to devm_platform_ioremap_resource

[Frank Lee]

On Tue, Dec 10, 2019 at 4:44 AM Leonard Crestez <leonard.crestez@nxp.com> wrote:
>
> On 09.12.2019 21:58, Yangtao Li wrote:
> > Use devm_platform_ioremap_resource() to simplify code.
> >
> > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> > ---
> >   drivers/clk/imx/clk-imx8qxp-lpcg.c | 6 +-----
> >   1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/drivers/clk/imx/clk-imx8qxp-lpcg.c b/drivers/clk/imx/clk-imx8qxp-lpcg.c
> > index c0aff7ca6374..10ae712447c6 100644
> > --- a/drivers/clk/imx/clk-imx8qxp-lpcg.c
> > +++ b/drivers/clk/imx/clk-imx8qxp-lpcg.c
> > @@ -164,7 +164,6 @@ static int imx8qxp_lpcg_clk_probe(struct platform_device *pdev)
> >       struct clk_hw_onecell_data *clk_data;
> >       const struct imx8qxp_ss_lpcg *ss_lpcg;
> >       const struct imx8qxp_lpcg_data *lpcg;
> > -     struct resource *res;
> >       struct clk_hw **clks;
> >       void __iomem *base;
> >       int i;
> > @@ -173,10 +172,7 @@ static int imx8qxp_lpcg_clk_probe(struct platform_device *pdev)
> >       if (!ss_lpcg)
> >               return -ENODEV;
> >
> > -     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > -     if (!res)
> > -             return -EINVAL;
> > -     base = devm_ioremap(dev, res->start, resource_size(res));
> > +     base = devm_platform_ioremap_resource(pdev, 0);
> >       if (!base)
> >               return -ENOMEM;
>
> This breaks imx8qxp-mek boot by causing most peripherals (like uart) to
> fail to probe.
>
> The old and new paths are not equivalent: devm_platform_ioremap_resource
> calls devm_ioremap_resource which differs from devm_ioremap by also
> calling devm_request_mem_region.
>
> This prevents other mappings in the area and imx8qxp-lpcg nodes map
> whole hardware "subsystems" and overlap most peripherals. For example:
>
>                   adma_lpcg: clock-controller@59000000 {
>                           compatible = "fsl,imx8qxp-lpcg-adma";
>                           reg = <0x59000000 0x2000000>;
>                           #clock-cells = <1>;
>                 };

It's a bit surprising that the clock driver does such a large mapping.

So, it is best to check the dts reg configuration of the driver.


Yours,
Yangtao

>
>                  adma_lpuart0: serial@5a060000 {
>                         reg = <0x5a060000 0x1000>;
>                         ...
>                 };
>
> I don't know if this issue affects any other platforms (imx8 lpcg
> bindings are unusual) but if you found this with an automated tool
> perhaps it should be adjusted?
>
> By my count it's the 4th time this incorrect cleanup was posted.
>
> Previously: https://lkml.org/lkml/2019/12/4/487
>
> --
> Regards,
> Leonard

[PATCH 09/17] clk: sifive: convert to devm_platform_ioremap_resource

[Yangtao Li]

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 drivers/clk/sifive/fu540-prci.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c
index 6282ee2f361c..ececee273d32 100644
--- a/drivers/clk/sifive/fu540-prci.c
+++ b/drivers/clk/sifive/fu540-prci.c
@@ -582,7 +582,6 @@ static int __prci_register_clocks(struct device *dev, struct __prci_data *pd)
 static int sifive_fu540_prci_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct resource *res;
 	struct __prci_data *pd;
 	int r;
 
@@ -590,8 +589,7 @@ static int sifive_fu540_prci_probe(struct platform_device *pdev)
 	if (!pd)
 		return -ENOMEM;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	pd->va = devm_ioremap_resource(dev, res);
+	pd->va = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(pd->va))
 		return PTR_ERR(pd->va);
 
-- 
2.17.1

[PATCH 10/17] clk: axi-clkgen: convert to devm_platform_ioremap_resource

[Yangtao Li]

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 drivers/clk/clk-axi-clkgen.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/clk/clk-axi-clkgen.c b/drivers/clk/clk-axi-clkgen.c
index 96f351785b41..8d0c32e81e78 100644
--- a/drivers/clk/clk-axi-clkgen.c
+++ b/drivers/clk/clk-axi-clkgen.c
@@ -412,7 +412,6 @@ static int axi_clkgen_probe(struct platform_device *pdev)
 	struct clk_init_data init;
 	const char *parent_names[2];
 	const char *clk_name;
-	struct resource *mem;
 	unsigned int i;
 	int ret;
 
@@ -427,8 +426,7 @@ static int axi_clkgen_probe(struct platform_device *pdev)
 	if (!axi_clkgen)
 		return -ENOMEM;
 
-	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	axi_clkgen->base = devm_ioremap_resource(&pdev->dev, mem);
+	axi_clkgen->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(axi_clkgen->base))
 		return PTR_ERR(axi_clkgen->base);
 
-- 
2.17.1

[PATCH 11/17] clk: milbeaut: convert to devm_platform_ioremap_resource

[Yangtao Li]

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 drivers/clk/clk-milbeaut.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/clk/clk-milbeaut.c b/drivers/clk/clk-milbeaut.c
index 80b9d78493bc..05f09fbfb80a 100644
--- a/drivers/clk/clk-milbeaut.c
+++ b/drivers/clk/clk-milbeaut.c
@@ -560,14 +560,12 @@ static void m10v_reg_mux_pre(const struct m10v_clk_mux_factors *factors,
 static int m10v_clk_probe(struct platform_device *pdev)
 {
 	int id;
-	struct resource *res;
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
 	void __iomem *base;
 	const char *parent_name;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(dev, res);
+	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-- 
2.17.1

[PATCH 12/17] clk: socfpga: convert to devm_platform_ioremap_resource

[Yangtao Li]

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 drivers/clk/socfpga/clk-s10.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/clk/socfpga/clk-s10.c b/drivers/clk/socfpga/clk-s10.c
index 993f3a73c71e..85055fc56404 100644
--- a/drivers/clk/socfpga/clk-s10.c
+++ b/drivers/clk/socfpga/clk-s10.c
@@ -276,11 +276,9 @@ static struct stratix10_clock_data *__socfpga_s10_clk_init(struct platform_devic
 	struct device *dev = &pdev->dev;
 	struct stratix10_clock_data *clk_data;
 	struct clk **clk_table;
-	struct resource *res;
 	void __iomem *base;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(dev, res);
+	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base)) {
 		pr_err("%s: failed to map clock registers\n", __func__);
 		return ERR_CAST(base);
-- 
2.17.1

Re: [PATCH 12/17] clk: socfpga: convert to devm_platform_ioremap_resource

[Dinh Nguyen]

On 12/9/19 1:57 PM, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
> 
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> ---
>  drivers/clk/socfpga/clk-s10.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/socfpga/clk-s10.c b/drivers/clk/socfpga/clk-s10.c
> index 993f3a73c71e..85055fc56404 100644
> --- a/drivers/clk/socfpga/clk-s10.c
> +++ b/drivers/clk/socfpga/clk-s10.c
> @@ -276,11 +276,9 @@ static struct stratix10_clock_data *__socfpga_s10_clk_init(struct platform_devic
>  	struct device *dev = &pdev->dev;
>  	struct stratix10_clock_data *clk_data;
>  	struct clk **clk_table;
> -	struct resource *res;
>  	void __iomem *base;
>  
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	base = devm_ioremap_resource(dev, res);
> +	base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(base)) {
>  		pr_err("%s: failed to map clock registers\n", __func__);
>  		return ERR_CAST(base);
> 

Tested-by: Dinh Nguyen <dinguyen@kernel.org>

[PATCH 13/17] clk: gemini: convert to devm_platform_ioremap_resource

[Yangtao Li]

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 drivers/clk/clk-gemini.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/clk/clk-gemini.c b/drivers/clk/clk-gemini.c
index b51069e794ff..0184ff9bd206 100644
--- a/drivers/clk/clk-gemini.c
+++ b/drivers/clk/clk-gemini.c
@@ -276,7 +276,6 @@ static int gemini_clk_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
 	unsigned int mult, div;
-	struct resource *res;
 	u32 val;
 	int ret;
 	int i;
@@ -286,8 +285,7 @@ static int gemini_clk_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	/* Remap the system controller for the exclusive register */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(dev, res);
+	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-- 
2.17.1

[PATCH 14/17] clk: axm5516: convert to devm_platform_ioremap_resource

[Yangtao Li]

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 drivers/clk/clk-axm5516.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/clk/clk-axm5516.c b/drivers/clk/clk-axm5516.c
index 07e80fe8c310..3ab02b62b645 100644
--- a/drivers/clk/clk-axm5516.c
+++ b/drivers/clk/clk-axm5516.c
@@ -541,14 +541,12 @@ MODULE_DEVICE_TABLE(of, axmclk_match_table);
 static int axmclk_probe(struct platform_device *pdev)
 {
 	void __iomem *base;
-	struct resource *res;
 	int i, ret;
 	struct device *dev = &pdev->dev;
 	struct regmap *regmap;
 	size_t num_clks;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(dev, res);
+	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-- 
2.17.1

[PATCH 15/17] clk: bm1880: convert to devm_platform_ioremap_resource

[Yangtao Li]

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 drivers/clk/clk-bm1880.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/clk-bm1880.c b/drivers/clk/clk-bm1880.c
index 4cd175afce9b..9fa0ca1fe833 100644
--- a/drivers/clk/clk-bm1880.c
+++ b/drivers/clk/clk-bm1880.c
@@ -893,16 +893,13 @@ static int bm1880_clk_probe(struct platform_device *pdev)
 	struct bm1880_clock_data *clk_data;
 	void __iomem *pll_base, *sys_base;
 	struct device *dev = &pdev->dev;
-	struct resource *res;
 	int num_clks, i;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	pll_base = devm_ioremap_resource(&pdev->dev, res);
+	pll_base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(pll_base))
 		return PTR_ERR(pll_base);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	sys_base = devm_ioremap_resource(&pdev->dev, res);
+	sys_base = devm_platform_ioremap_resource(pdev, 1);
 	if (IS_ERR(sys_base))
 		return PTR_ERR(sys_base);
 
-- 
2.17.1

[PATCH 16/17] clk: actions: convert to devm_platform_ioremap_resource

[Yangtao Li]

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 drivers/clk/actions/owl-common.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/clk/actions/owl-common.c b/drivers/clk/actions/owl-common.c
index 4de97cc7cb54..9e98e8a92ad7 100644
--- a/drivers/clk/actions/owl-common.c
+++ b/drivers/clk/actions/owl-common.c
@@ -43,10 +43,8 @@ int owl_clk_regmap_init(struct platform_device *pdev,
 {
 	void __iomem *base;
 	struct regmap *regmap;
-	struct resource *res;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(&pdev->dev, res);
+	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-- 
2.17.1

[PATCH 17/17] ARC: clk: convert to devm_platform_ioremap_resource

[Yangtao Li]

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 drivers/clk/clk-hsdk-pll.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/clk/clk-hsdk-pll.c b/drivers/clk/clk-hsdk-pll.c
index 97d1e8c35b71..155f2af65361 100644
--- a/drivers/clk/clk-hsdk-pll.c
+++ b/drivers/clk/clk-hsdk-pll.c
@@ -299,7 +299,6 @@ static const struct clk_ops hsdk_pll_ops = {
 static int hsdk_pll_clk_probe(struct platform_device *pdev)
 {
 	int ret;
-	struct resource *mem;
 	const char *parent_name;
 	unsigned int num_parents;
 	struct hsdk_pll_clk *pll_clk;
@@ -310,8 +309,7 @@ static int hsdk_pll_clk_probe(struct platform_device *pdev)
 	if (!pll_clk)
 		return -ENOMEM;
 
-	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	pll_clk->regs = devm_ioremap_resource(dev, mem);
+	pll_clk->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(pll_clk->regs))
 		return PTR_ERR(pll_clk->regs);
 
-- 
2.17.1

Re: [PATCH 01/17] clk: sunxi: sunxi-ng: convert to devm_platform_ioremap_resource

[Chen-Yu Tsai]

On Tue, Dec 10, 2019 at 3:58 AM Yangtao Li <tiny.windzz@gmail.com> wrote:
>
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> ---
>  drivers/clk/sunxi-ng/ccu-sun50i-a64.c    | 4 +---
>  drivers/clk/sunxi-ng/ccu-sun50i-h6.c     | 4 +---
>  drivers/clk/sunxi-ng/ccu-sun8i-a83t.c    | 4 +---
>  drivers/clk/sunxi-ng/ccu-sun8i-de2.c     | 4 +---
>  drivers/clk/sunxi-ng/ccu-sun8i-r40.c     | 4 +---
>  drivers/clk/sunxi-ng/ccu-sun9i-a80-de.c  | 4 +---
>  drivers/clk/sunxi-ng/ccu-sun9i-a80-usb.c | 4 +---
>  drivers/clk/sunxi-ng/ccu-sun9i-a80.c     | 4 +---
>  drivers/clk/sunxi/clk-mod0.c             | 4 +---
>  drivers/clk/sunxi/clk-sun6i-apb0-gates.c | 4 +---
>  drivers/clk/sunxi/clk-sun6i-apb0.c       | 4 +---
>  drivers/clk/sunxi/clk-sun6i-ar100.c      | 4 +---
>  drivers/clk/sunxi/clk-sun8i-apb0.c       | 4 +---
>  13 files changed, 13 insertions(+), 39 deletions(-)

Reviewed-by: Chen-Yu Tsai <wens@csie.org>

Re: [PATCH 01/17] clk: sunxi: sunxi-ng: convert to devm_platform_ioremap_resource

[Stephen Boyd]

Quoting Yangtao Li (2019-12-09 11:57:33)
> Use devm_platform_ioremap_resource() to simplify code.
> 
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>

Please add a cover letter, resend this series picking up any acks and
dropping any patches that were rejected by maintainers of the drivers.
There was significant discussion on one patch that was rejected, so I'll
only pick up patches for the next cycle that are acked/reviewed-by
relevant folks.