| Message ID | 20250301104724.36399-1-ziyao@disroot.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Support SD/SDIO controllers on RK3528 | expand |
Hi, > + nr_clks = rockchip_clk_find_max_clk_id(rk3528_clk_branches, > + nr_branches) + 1; > + > + vo_grf = syscon_regmap_lookup_by_compatible("rockchip,rk3528-vo-grf"); > + if (!IS_ERR(vo_grf)) > + nr_clks = MAX(rockchip_clk_find_max_clk_id(rk3528_vo_clk_branches, > + nr_vo_branches) + 1, drivers/clk/rockchip/clk-rk3528.c: In function 'clk_rk3528_probe': drivers/clk/rockchip/clk-rk3528.c:1105:27: error: implicit declaration of function 'MAX'; did you mean 'MUX'? 1105 | nr_clks = MAX(rockchip_clk_find_max_clk_id(rk3528_vo_clk_branches, | ^~~ | MUX It seems that missing definition with older kernels. Thanks, Chukun
Hi, Am Samstag, 1. März 2025, 11:47:24 MEZ schrieb Yao Zi: > These clocks locate in VO and VPU GRF, serving for SD/SDIO controller > tuning purpose. Add their definitions and register them in driver if > corresponding GRF is available. (no critique, just an observation :-) ) this puts a completely new meaning on the "general register files" as dumping ground ;-) . Whoever got the idea of making sdmm/sdio tuning controls part of GRFs that are supposed display and/or video encoder parts :-D > GRFs are looked up by compatible to simplify devicetree binding. > > Signed-off-by: Yao Zi <ziyao@disroot.org> > --- > static int __init clk_rk3528_probe(struct platform_device *pdev) > { > + unsigned long nr_vpu_branches = ARRAY_SIZE(rk3528_vpu_clk_branches); > + unsigned long nr_vo_branches = ARRAY_SIZE(rk3528_vo_clk_branches); > + unsigned long nr_branches = ARRAY_SIZE(rk3528_clk_branches); > struct rockchip_clk_provider *ctx; > struct device *dev = &pdev->dev; > struct device_node *np = dev->of_node; > - unsigned long nr_branches = ARRAY_SIZE(rk3528_clk_branches); > - unsigned long nr_clks; > + struct regmap *vo_grf, *vpu_grf; > void __iomem *reg_base; > - > - nr_clks = rockchip_clk_find_max_clk_id(rk3528_clk_branches, > - nr_branches) + 1; > + unsigned long nr_clks; > > reg_base = devm_platform_ioremap_resource(pdev, 0); > if (IS_ERR(reg_base)) > return dev_err_probe(dev, PTR_ERR(reg_base), > "could not map cru region"); > > + nr_clks = rockchip_clk_find_max_clk_id(rk3528_clk_branches, > + nr_branches) + 1; > + > + vo_grf = syscon_regmap_lookup_by_compatible("rockchip,rk3528-vo-grf"); > + if (!IS_ERR(vo_grf)) for readability, please make this into something like if (!IS_ERR(vo_grf)) { nr_vo_clks = rockchip_clk_find_max_clk_id(rk3528_vo_clk_branches, nr_vo_branches) + 1; nr_clks = max(nr_vo_clks, nr_clks); } > + else if (PTR_ERR(vo_grf) != ENODEV) > + return dev_err_probe(dev, PTR_ERR(vo_grf), > + "failed to look up VO GRF\n"); > + > + vpu_grf = syscon_regmap_lookup_by_compatible("rockchip,rk3528-vpu-grf"); > + if (!IS_ERR(vpu_grf)) > + nr_clks = MAX(rockchip_clk_find_max_clk_id(rk3528_vpu_clk_branches, > + nr_vpu_branches) + 1, > + nr_clks); same here please > + else if (PTR_ERR(vpu_grf) != ENODEV) > + return dev_err_probe(dev, PTR_ERR(vpu_grf), > + "failed to look up VPU GRF\n"); > + > ctx = rockchip_clk_init(np, reg_base, nr_clks); > if (IS_ERR(ctx)) > return dev_err_probe(dev, PTR_ERR(ctx), Thanks Heiko
On Wed, Mar 05, 2025 at 11:21:48AM +0100, Heiko Stübner wrote: > Hi, > > Am Samstag, 1. März 2025, 11:47:24 MEZ schrieb Yao Zi: > > These clocks locate in VO and VPU GRF, serving for SD/SDIO controller > > tuning purpose. Add their definitions and register them in driver if > > corresponding GRF is available. > > (no critique, just an observation :-) ) > > this puts a completely new meaning on the "general register files" > as dumping ground ;-) . > > Whoever got the idea of making sdmm/sdio tuning controls part > of GRFs that are supposed display and/or video encoder parts :-D Yes, the register layout is quite weird. Additionally some USB2 phy registers locate in VO GRF as well... > > > GRFs are looked up by compatible to simplify devicetree binding. > > > > Signed-off-by: Yao Zi <ziyao@disroot.org> > > --- > > > static int __init clk_rk3528_probe(struct platform_device *pdev) > > { > > + unsigned long nr_vpu_branches = ARRAY_SIZE(rk3528_vpu_clk_branches); > > + unsigned long nr_vo_branches = ARRAY_SIZE(rk3528_vo_clk_branches); > > + unsigned long nr_branches = ARRAY_SIZE(rk3528_clk_branches); > > struct rockchip_clk_provider *ctx; > > struct device *dev = &pdev->dev; > > struct device_node *np = dev->of_node; > > - unsigned long nr_branches = ARRAY_SIZE(rk3528_clk_branches); > > - unsigned long nr_clks; > > + struct regmap *vo_grf, *vpu_grf; > > void __iomem *reg_base; > > - > > - nr_clks = rockchip_clk_find_max_clk_id(rk3528_clk_branches, > > - nr_branches) + 1; > > + unsigned long nr_clks; > > > > reg_base = devm_platform_ioremap_resource(pdev, 0); > > if (IS_ERR(reg_base)) > > return dev_err_probe(dev, PTR_ERR(reg_base), > > "could not map cru region"); > > > > + nr_clks = rockchip_clk_find_max_clk_id(rk3528_clk_branches, > > + nr_branches) + 1; > > + > > + vo_grf = syscon_regmap_lookup_by_compatible("rockchip,rk3528-vo-grf"); > > + if (!IS_ERR(vo_grf)) > > for readability, please make this into something like > if (!IS_ERR(vo_grf)) { > nr_vo_clks = rockchip_clk_find_max_clk_id(rk3528_vo_clk_branches, > nr_vo_branches) + 1; > nr_clks = max(nr_vo_clks, nr_clks); > } Thanks for the suggestion, will take it. > > + else if (PTR_ERR(vo_grf) != ENODEV) > > + return dev_err_probe(dev, PTR_ERR(vo_grf), > > + "failed to look up VO GRF\n"); > > + > > + vpu_grf = syscon_regmap_lookup_by_compatible("rockchip,rk3528-vpu-grf"); > > + if (!IS_ERR(vpu_grf)) > > + nr_clks = MAX(rockchip_clk_find_max_clk_id(rk3528_vpu_clk_branches, > > + nr_vpu_branches) + 1, > > + nr_clks); > > same here please > > > + else if (PTR_ERR(vpu_grf) != ENODEV) > > + return dev_err_probe(dev, PTR_ERR(vpu_grf), > > + "failed to look up VPU GRF\n"); > > + > > ctx = rockchip_clk_init(np, reg_base, nr_clks); > > if (IS_ERR(ctx)) > > return dev_err_probe(dev, PTR_ERR(ctx), > > Thanks > Heiko > Cheers, Yao Zi
diff --git a/drivers/clk/rockchip/clk-rk3528.c b/drivers/clk/rockchip/clk-rk3528.c index b8b577b902a0..b89440dd7448 100644 --- a/drivers/clk/rockchip/clk-rk3528.c +++ b/drivers/clk/rockchip/clk-rk3528.c @@ -10,6 +10,7 @@ #include <linux/of.h> #include <linux/of_device.h> #include <linux/platform_device.h> +#include <linux/mfd/syscon.h> #include <dt-bindings/clock/rockchip,rk3528-cru.h> @@ -1061,23 +1062,62 @@ static struct rockchip_clk_branch rk3528_clk_branches[] __initdata = { 0, 1, 1), }; +static struct rockchip_clk_branch rk3528_vo_clk_branches[] __initdata = { + MMC_GRF(SCLK_SDMMC_DRV, "sdmmc_drv", "cclk_src_sdmmc0", + RK3528_SDMMC_CON(0), 1), + MMC_GRF(SCLK_SDMMC_SAMPLE, "sdmmc_sample", "cclk_src_sdmmc0", + RK3528_SDMMC_CON(1), 1), +}; + +static struct rockchip_clk_branch rk3528_vpu_clk_branches[] __initdata = { + MMC_GRF(SCLK_SDIO0_DRV, "sdio0_drv", "cclk_src_sdio0", + RK3528_SDIO0_CON(0), 1), + MMC_GRF(SCLK_SDIO0_SAMPLE, "sdio0_sample", "cclk_src_sdio0", + RK3528_SDIO0_CON(1), 1), + MMC_GRF(SCLK_SDIO1_DRV, "sdio1_drv", "cclk_src_sdio1", + RK3528_SDIO1_CON(0), 1), + MMC_GRF(SCLK_SDIO1_SAMPLE, "sdio1_sample", "cclk_src_sdio1", + RK3528_SDIO1_CON(1), 1), +}; + static int __init clk_rk3528_probe(struct platform_device *pdev) { + unsigned long nr_vpu_branches = ARRAY_SIZE(rk3528_vpu_clk_branches); + unsigned long nr_vo_branches = ARRAY_SIZE(rk3528_vo_clk_branches); + unsigned long nr_branches = ARRAY_SIZE(rk3528_clk_branches); struct rockchip_clk_provider *ctx; struct device *dev = &pdev->dev; struct device_node *np = dev->of_node; - unsigned long nr_branches = ARRAY_SIZE(rk3528_clk_branches); - unsigned long nr_clks; + struct regmap *vo_grf, *vpu_grf; void __iomem *reg_base; - - nr_clks = rockchip_clk_find_max_clk_id(rk3528_clk_branches, - nr_branches) + 1; + unsigned long nr_clks; reg_base = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(reg_base)) return dev_err_probe(dev, PTR_ERR(reg_base), "could not map cru region"); + nr_clks = rockchip_clk_find_max_clk_id(rk3528_clk_branches, + nr_branches) + 1; + + vo_grf = syscon_regmap_lookup_by_compatible("rockchip,rk3528-vo-grf"); + if (!IS_ERR(vo_grf)) + nr_clks = MAX(rockchip_clk_find_max_clk_id(rk3528_vo_clk_branches, + nr_vo_branches) + 1, + nr_clks); + else if (PTR_ERR(vo_grf) != ENODEV) + return dev_err_probe(dev, PTR_ERR(vo_grf), + "failed to look up VO GRF\n"); + + vpu_grf = syscon_regmap_lookup_by_compatible("rockchip,rk3528-vpu-grf"); + if (!IS_ERR(vpu_grf)) + nr_clks = MAX(rockchip_clk_find_max_clk_id(rk3528_vpu_clk_branches, + nr_vpu_branches) + 1, + nr_clks); + else if (PTR_ERR(vpu_grf) != ENODEV) + return dev_err_probe(dev, PTR_ERR(vpu_grf), + "failed to look up VPU GRF\n"); + ctx = rockchip_clk_init(np, reg_base, nr_clks); if (IS_ERR(ctx)) return dev_err_probe(dev, PTR_ERR(ctx), @@ -1091,6 +1131,12 @@ static int __init clk_rk3528_probe(struct platform_device *pdev) &rk3528_cpuclk_data, rk3528_cpuclk_rates, ARRAY_SIZE(rk3528_cpuclk_rates)); rockchip_clk_register_branches(ctx, rk3528_clk_branches, nr_branches); + if (!IS_ERR(vo_grf)) + rockchip_clk_register_grf_branches(ctx, rk3528_vo_clk_branches, + vo_grf, nr_vo_branches); + if (!IS_ERR(vpu_grf)) + rockchip_clk_register_grf_branches(ctx, rk3528_vpu_clk_branches, + vpu_grf, nr_vpu_branches); rk3528_rst_init(np, reg_base); diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h index 5d82ec5facfa..02d4ca0012d7 100644 --- a/drivers/clk/rockchip/clk.h +++ b/drivers/clk/rockchip/clk.h @@ -217,6 +217,9 @@ struct clk; #define RK3528_CLKSEL_CON(x) ((x) * 0x4 + 0x300) #define RK3528_CLKGATE_CON(x) ((x) * 0x4 + 0x800) #define RK3528_SOFTRST_CON(x) ((x) * 0x4 + 0xa00) +#define RK3528_SDMMC_CON(x) ((x) * 0x4 + 0x24) +#define RK3528_SDIO0_CON(x) ((x) * 0x4 + 0x4) +#define RK3528_SDIO1_CON(x) ((x) * 0x4 + 0xc) #define RK3528_PMU_CLKSEL_CON(x) ((x) * 0x4 + 0x300 + RK3528_PMU_CRU_BASE) #define RK3528_PMU_CLKGATE_CON(x) ((x) * 0x4 + 0x800 + RK3528_PMU_CRU_BASE) #define RK3528_PCIE_CLKSEL_CON(x) ((x) * 0x4 + 0x300 + RK3528_PCIE_CRU_BASE)
These clocks locate in VO and VPU GRF, serving for SD/SDIO controller tuning purpose. Add their definitions and register them in driver if corresponding GRF is available. GRFs are looked up by compatible to simplify devicetree binding. Signed-off-by: Yao Zi <ziyao@disroot.org> --- drivers/clk/rockchip/clk-rk3528.c | 56 ++++++++++++++++++++++++++++--- drivers/clk/rockchip/clk.h | 3 ++ 2 files changed, 54 insertions(+), 5 deletions(-)