| Message ID | cover.1704788539.git.ysato@users.sourceforge.jp |
|---|---|
| Headers | show |
| Series | Device Tree support for SH7751 based board | expand |
On Tue, 09 Jan 2024, Yoshinori Sato wrote: > Various parameters of SM501 can be set using platform_data, > so parameters cannot be passed in the DeviceTree target. > Expands the parameters set in platform_data so that they can be > specified using DeviceTree properties. > > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp> > --- > drivers/mfd/sm501.c | 436 ++++++++++++++++++++++++++++++++++ How has this grown from 99 lines to 436 lines? Most of it almost certainly needs moving (back?) out to the leaf drivers. A great deal of the properties parsed in here are only relevant to a single device (display for instance). Please move all non-generic handling out to the relevant subsystems. > drivers/video/fbdev/sm501fb.c | 106 +++++++++ > 2 files changed, 542 insertions(+)
On Tue, Jan 9, 2024 at 9:23 AM Yoshinori Sato <ysato@users.sourceforge.jp> wrote: > R4 is caller saved in SH ABI. > Save it so it doesn't get corrupted until it's needed for initialization. > > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp> My Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> on v3 is still valid. Gr{oetje,eeting}s, Geert
Hi Sato-san, On Tue, Jan 9, 2024 at 9:23 AM Yoshinori Sato <ysato@users.sourceforge.jp> wrote: > Targets that support OF should be treated as one board. > > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp> Thanks for your patch! > --- a/arch/sh/Kconfig > +++ b/arch/sh/Kconfig > @@ -710,6 +710,7 @@ choice > prompt "Kernel command line" > optional > default CMDLINE_OVERWRITE > + depends on !OF || USE_BUILTIN_DTB This is still useful in the generic OF case. I think it would be good to model this similar to what arm/arm64/riscv are using (from bootloader / extend / force). > help > Setting this option allows the kernel command line arguments > to be set. > diff --git a/arch/sh/boards/Kconfig b/arch/sh/boards/Kconfig > index 109bec4dad94..e7e52779ef62 100644 > --- a/arch/sh/boards/Kconfig > +++ b/arch/sh/boards/Kconfig > @@ -19,16 +19,9 @@ config SH_DEVICE_TREE > select TIMER_OF > select COMMON_CLK > select GENERIC_CALIBRATE_DELAY > - > -config SH_JCORE_SOC > - bool "J-Core SoC" > - select SH_DEVICE_TREE > - select CLKSRC_JCORE_PIT > - select JCORE_AIC > - depends on CPU_J2 > - help > - Select this option to include drivers core components of the > - J-Core SoC, including interrupt controllers and timers. > + select GENERIC_IRQ_CHIP > + select SYS_SUPPORTS_PCI > + select GENERIC_PCI_IOMAP if PCI > > config SH_SOLUTION_ENGINE > bool "SolutionEngine" > @@ -293,6 +286,7 @@ config SH_LANDISK > bool "LANDISK" > depends on CPU_SUBTYPE_SH7751R > select HAVE_PCI > + select SYS_SUPPORTS_PCI > help > I-O DATA DEVICE, INC. "LANDISK Series" support. > > @@ -369,6 +363,16 @@ config SH_APSH4AD0A > help > Select AP-SH4AD-0A if configuring for an ALPHAPROJECT AP-SH4AD-0A. > > +config SH_OF_BOARD > + bool "General Open Firmware boards" > + select SH_DEVICE_TREE > + select CLKSRC_JCORE_PIT if CPU_J2 > + select JCORE_AIC if CPU_J2 Please move these selects to CPU_J2 instead... > + select HAVE_PCI if CPU_SUBTYPE_SH7751R ... and this to CPU_SUBTYPE_SH7751R, else it will become a long unmaintainable list soon... > + help > + This board means general OF supported targets. > + > + Gr{oetje,eeting}s, Geert
Hi Saton-san, Thanks for your patch! Please drop the period at the end of the one-line summary. On Tue, Jan 9, 2024 at 9:23 AM Yoshinori Sato <ysato@users.sourceforge.jp> wrote: > Allows initialization as CLOCKSOURCE. Please explain why this is needed. E.g. Add support for early registration using TIMER_OF_DECLARE(), so the timer can be used as a clocksource on SoCs that do not have any other suitable timer. > > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp> > --- a/drivers/clocksource/sh_tmu.c > +++ b/drivers/clocksource/sh_tmu.c > @@ -148,8 +151,8 @@ static int __sh_tmu_enable(struct sh_tmu_channel *ch) > /* enable clock */ > ret = clk_enable(ch->tmu->clk); > if (ret) { > - dev_err(&ch->tmu->pdev->dev, "ch%u: cannot enable clock\n", > - ch->index); > + pr_err("%s ch%u: cannot enable clock\n", > + ch->tmu->name, ch->index); Please wrap the line after, not before, "ch->tmu->name,". > return ret; > } > > @@ -324,14 +332,14 @@ static int sh_tmu_register_clocksource(struct sh_tmu_channel *ch, > cs->mask = CLOCKSOURCE_MASK(32); > cs->flags = CLOCK_SOURCE_IS_CONTINUOUS; > > - dev_info(&ch->tmu->pdev->dev, "ch%u: used as clock source\n", > - ch->index); > + pr_info("%s ch%u: used as clock source\n", > + ch->tmu->name, ch->index); No need to wrap this line at all. > > clocksource_register_hz(cs, ch->tmu->rate); > return 0; > } > > -static struct sh_tmu_channel *ced_to_sh_tmu(struct clock_event_device *ced) > +static inline struct sh_tmu_channel *ced_to_sh_tmu(struct clock_event_device *ced) > { > return container_of(ced, struct sh_tmu_channel, ced); > } > @@ -364,8 +372,8 @@ static int sh_tmu_clock_event_set_state(struct clock_event_device *ced, > if (clockevent_state_oneshot(ced) || clockevent_state_periodic(ced)) > sh_tmu_disable(ch); > > - dev_info(&ch->tmu->pdev->dev, "ch%u: used for %s clock events\n", > - ch->index, periodic ? "periodic" : "oneshot"); > + pr_info("%s ch%u: used for %s clock events\n", > + ch->tmu->name, ch->index, periodic ? "periodic" : "oneshot"); Please wrap the line after, not before, "ch->tmu->name,". > sh_tmu_clock_event_start(ch, periodic); > return 0; > } > @@ -403,7 +411,8 @@ static void sh_tmu_clock_event_resume(struct clock_event_device *ced) > } > > static void sh_tmu_register_clockevent(struct sh_tmu_channel *ch, > - const char *name) > + const char *name, > + struct device_node *np) "np" is unused in this function, hence this change is unneeded. (Hey, I already said that in my review of v3) > { > struct clock_event_device *ced = &ch->ced; > int ret; > @@ -417,30 +426,32 @@ static void sh_tmu_register_clockevent(struct sh_tmu_channel *ch, > ced->set_state_shutdown = sh_tmu_clock_event_shutdown; > ced->set_state_periodic = sh_tmu_clock_event_set_periodic; > ced->set_state_oneshot = sh_tmu_clock_event_set_oneshot; > - ced->suspend = sh_tmu_clock_event_suspend; > - ced->resume = sh_tmu_clock_event_resume; > - > - dev_info(&ch->tmu->pdev->dev, "ch%u: used for clock events\n", > - ch->index); > + if (ch->tmu->pdev) { > + ced->suspend = sh_tmu_clock_event_suspend; > + ced->resume = sh_tmu_clock_event_resume; > + } > + pr_info("%s ch%u: used for clock events\n", > + ch->tmu->name, ch->index); No need to wrap this line at all. > > clockevents_config_and_register(ced, ch->tmu->rate, 0x300, 0xffffffff); > > ret = request_irq(ch->irq, sh_tmu_interrupt, > IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING, > - dev_name(&ch->tmu->pdev->dev), ch); > + ch->tmu->name, ch); > if (ret) { > - dev_err(&ch->tmu->pdev->dev, "ch%u: failed to request irq %d\n", > - ch->index, ch->irq); > + pr_err("%s ch%u: failed to request irq %d\n", > + ch->tmu->name, ch->index, ch->irq); Please wrap the line after, not before, "ch->tmu->name,". > return; > } > } > > static int sh_tmu_register(struct sh_tmu_channel *ch, const char *name, > + struct device_node *np, np is unneeded. > bool clockevent, bool clocksource) > { > if (clockevent) { > ch->tmu->has_clockevent = true; > - sh_tmu_register_clockevent(ch, name); > + sh_tmu_register_clockevent(ch, name, np); > } else if (clocksource) { > ch->tmu->has_clocksource = true; > sh_tmu_register_clocksource(ch, name); > @@ -465,53 +477,59 @@ static int sh_tmu_channel_setup(struct sh_tmu_channel *ch, unsigned int index, > else > ch->base = tmu->mapbase + 8 + ch->index * 12; > > - ch->irq = platform_get_irq(tmu->pdev, index); > + if (tmu->pdev) > + ch->irq = platform_get_irq(tmu->pdev, index); > + else > + ch->irq = of_irq_get(np, index); You can use of_irq_get() unconditionally. > if (ch->irq < 0) > return ch->irq; > > ch->cs_enabled = false; > ch->enable_count = 0; > > - return sh_tmu_register(ch, dev_name(&tmu->pdev->dev), > + return sh_tmu_register(ch, tmu->name, np, No need to pass np. > clockevent, clocksource); > } > > -static int sh_tmu_map_memory(struct sh_tmu_device *tmu) > +static int sh_tmu_map_memory(struct sh_tmu_device *tmu, struct device_node *np) > { > struct resource *res; > > - res = platform_get_resource(tmu->pdev, IORESOURCE_MEM, 0); > - if (!res) { > - dev_err(&tmu->pdev->dev, "failed to get I/O memory\n"); > - return -ENXIO; > - } > + if (tmu->pdev) { > + res = platform_get_resource(tmu->pdev, IORESOURCE_MEM, 0); > + if (!res) { > + pr_err("sh_tmu failed to get I/O memory\n"); > + return -ENXIO; > + } > + > + tmu->mapbase = ioremap(res->start, resource_size(res)); > + } else > + tmu->mapbase = of_iomap(np, 0); You can use of_iomap() unconditionally. > > - tmu->mapbase = ioremap(res->start, resource_size(res)); > if (tmu->mapbase == NULL) > return -ENXIO; > > return 0; > } > > -static int sh_tmu_parse_dt(struct sh_tmu_device *tmu) > +static int sh_tmu_parse_dt(struct sh_tmu_device *tmu, struct device_node *np) > { > - struct device_node *np = tmu->pdev->dev.of_node; > - > tmu->model = SH_TMU; > tmu->num_channels = 3; > > of_property_read_u32(np, "#renesas,channels", &tmu->num_channels); > > if (tmu->num_channels != 2 && tmu->num_channels != 3) { > - dev_err(&tmu->pdev->dev, "invalid number of channels %u\n", > - tmu->num_channels); > + pr_err("%s: invalid number of channels %u\n", > + tmu->name, tmu->num_channels); Please wrap the line after, not before, "ch->tmu->name,". > return -EINVAL; > } > > return 0; > } > > -static int sh_tmu_setup(struct sh_tmu_device *tmu, struct platform_device *pdev) > +static int sh_tmu_setup(struct sh_tmu_device *tmu, > + struct platform_device *pdev, struct device_node *np) > { > unsigned int i; > int ret; > @@ -531,14 +554,17 @@ static int sh_tmu_setup(struct sh_tmu_device *tmu, struct platform_device *pdev) > tmu->model = id->driver_data; > tmu->num_channels = hweight8(cfg->channels_mask); > } else { > - dev_err(&tmu->pdev->dev, "missing platform data\n"); > + pr_err("%s missing platform data\n", tmu->name); > return -ENXIO; > } > > /* Get hold of clock. */ > - tmu->clk = clk_get(&tmu->pdev->dev, "fck"); > + if (pdev) > + tmu->clk = clk_get(&tmu->pdev->dev, "fck"); > + else > + tmu->clk = of_clk_get(np, 0); You can use of_clk_get() unconditionally. > if (IS_ERR(tmu->clk)) { > - dev_err(&tmu->pdev->dev, "cannot get clock\n"); > + pr_err("%s: cannot get clock\n", tmu->name); > return PTR_ERR(tmu->clk); > } > > @@ -665,12 +711,17 @@ static void __exit sh_tmu_exit(void) > platform_driver_unregister(&sh_tmu_device_driver); > } > > +subsys_initcall(sh_tmu_init); > +module_exit(sh_tmu_exit); > +#endif > + > #ifdef CONFIG_SUPERH > +#ifdef CONFIG_SH_DEVICE_TREE > +TIMER_OF_DECLARE(sh_tmu, "renesas,tmu", sh_tmu_of_register); Probably this TIMER_OF_DECLARE() should be done unconditionally, like is done in drivers/clocksource/renesas-ostm.c. I gave that a try on R-Mobile A1, which also has TMU, but it didn't seem to work (timer not firing?). So I suspect there are some missing clk_enable() calls. In the case of the platform driver, these are handled using pm_runtime_get_sync(). > +#else > sh_early_platform_init("earlytimer", &sh_tmu_device_driver); > #endif > - > -subsys_initcall(sh_tmu_init); > -module_exit(sh_tmu_exit); > +#endif > > MODULE_AUTHOR("Magnus Damm"); > MODULE_DESCRIPTION("SuperH TMU Timer Driver"); Gr{oetje,eeting}s, Geert
Hi Sato-san, On Tue, Jan 9, 2024 at 9:24 AM Yoshinori Sato <ysato@users.sourceforge.jp> wrote: > divider and gate only support 32-bit registers. > Older hardware uses narrower registers, so I want to be able to handle > 8-bit and 16-bit wide registers. > > Seven clk_divider flags are used, and if I add flags for 8bit access and > 16bit access, 8bit will not be enough, so I expanded it to u16. > > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp> Thanks for your patch! > --- a/drivers/clk/clk-gate.c > +++ b/drivers/clk/clk-gate.c > @@ -143,6 +161,18 @@ struct clk_hw *__clk_hw_register_gate(struct device *dev, > return ERR_PTR(-EINVAL); > } Please add a check for invalid CLK_GATE_HIWORD_MASK and register width combinations: if (clk_gate_flags & (CLK_GATE_REG_16BIT | CLK_GATE_REG_8BIT)) { pr_err("HIWORD_MASK needs 32-bit registers\n"); return ERR_PTR(-EINVAL); } > } > + if (clk_gate_flags & CLK_GATE_REG_16BIT) { > + if (bit_idx > 15) { > + pr_err("gate bit exceeds 16 bits\n"); > + return ERR_PTR(-EINVAL); > + } > + } > + if (clk_gate_flags & CLK_GATE_REG_8BIT) { > + if (bit_idx > 7) { > + pr_err("gate bit exceeds 8 bits\n"); > + return ERR_PTR(-EINVAL); > + } > + } > > /* allocate the gate */ > gate = kzalloc(sizeof(*gate), GFP_KERNEL); > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > index ace3a4ce2fc9..e2dfc1f083f4 100644 > --- a/include/linux/clk-provider.h > +++ b/include/linux/clk-provider.h > @@ -508,12 +508,16 @@ void of_fixed_clk_setup(struct device_node *np); > * CLK_GATE_BIG_ENDIAN - by default little endian register accesses are used for > * the gate register. Setting this flag makes the register accesses big > * endian. > + * CLK_GATE_REG_8BIT - by default 32bit register accesses are used for > + * the gate register. Setting this flag makes the register accesses 8bit. > + * CLK_GATE_REG_16BIT - by default 32bit register accesses are used for > + * the gate register. Setting this flag makes the register accesses 16bit. > */ > struct clk_gate { > struct clk_hw hw; > void __iomem *reg; > u8 bit_idx; > - u8 flags; > + u32 flags; There is no need to increase the size of the flags field for the gate clock. > spinlock_t *lock; > }; > > @@ -522,6 +526,8 @@ struct clk_gate { > #define CLK_GATE_SET_TO_DISABLE BIT(0) > #define CLK_GATE_HIWORD_MASK BIT(1) > #define CLK_GATE_BIG_ENDIAN BIT(2) > +#define CLK_GATE_REG_8BIT BIT(3) > +#define CLK_GATE_REG_16BIT BIT(4) > > extern const struct clk_ops clk_gate_ops; > struct clk_hw *__clk_hw_register_gate(struct device *dev, The rest LGTM. Gr{oetje,eeting}s, Geert
Hi Sato-san, On Tue, Jan 9, 2024 at 9:24 AM Yoshinori Sato <ysato@users.sourceforge.jp> wrote: > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp> Thanks for your patch! Please enhance the one-line summary, e.g. sh: j2_mimas_v2: Update CPU compatible value For the actual changes: Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
Hi Sato-san, On Tue, Jan 9, 2024 at 9:24 AM Yoshinori Sato <ysato@users.sourceforge.jp> wrote: > Renesas SH7750 and SH7751 series CPG driver. > This driver supported frequency control and clock gating. > > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp> Thanks for your patch! > --- a/drivers/clk/renesas/Kconfig > +++ b/drivers/clk/renesas/Kconfig > @@ -193,6 +196,10 @@ config CLK_SH73A0 > select CLK_RENESAS_CPG_MSTP > select CLK_RENESAS_DIV6 > > +config CLK_SH7750 > + bool "SH7750/7751 family clock support" if COMPILE_TEST > + help > + This is a driver for SH7750 / SH7751 CPG. This is a duplicate of the below. Please drop it. > > # Family > config CLK_RCAR_CPG_LIB > @@ -223,6 +230,11 @@ config CLK_RZG2L > bool "Renesas RZ/{G2L,G2UL,G3S,V2L} family clock support" if COMPILE_TEST > select RESET_CONTROLLER > > +config CLK_SH7750 > + bool "Renesas SH7750/7751 family clock support" if COMPILE_TEST > + help > + This is a driver for SH7750 / SH7751 CPG. > + > # Generic > config CLK_RENESAS_CPG_MSSR > bool "CPG/MSSR clock support" if COMPILE_TEST > --- /dev/null > +++ b/drivers/clk/renesas/clk-sh7750.c > +static int register_pll(struct device_node *node, struct cpg_priv *cpg) > +{ > + const char *clk_name = node->name; > + const char *parent_name; > + struct clk_init_data init = { > + .name = PLLOUT, > + .ops = &pll_ops, > + .flags = 0, > + .num_parents = 1, > + }; > + int ret; > + > + parent_name = of_clk_get_parent_name(node, 0); > + init.parent_names = &parent_name; > + cpg->hw.init = &init; > + > + ret = of_clk_hw_register(node, &cpg->hw); > + if (ret < 0) { > + pr_err("%s: failed to register %s pll clock (%d)\n", > + __func__, clk_name, ret); > + return ret; > + } > + if (ret < 0) > + pr_err("%s: failed to add provider %s (%d)\n", > + __func__, clk_name, ret); Bogus check and error message. > + return ret; > +} > +static int register_div(struct device_node *node, struct cpg_priv *cpg) > +{ > + static const char * const divout[] = { > + "fck", "bck", "ick", > + }; > + static const char * const stbcrout[] = { > + "sci_clk", "rtc_clk", "tmu012_clk", /* STBCR */ > + "scif_clk", "dmac_clk", /* STBCR */ > + "ubc_clk", "sq_clk", /* STBCR2 */ > + }; > + static const char * const clkstpout[] = { > + "intc_clk", "tmu34_clk", "pcic_clk", /* CLKSTP00 */ > + }; > + > + unsigned int i; > + int ret; > + struct clk_hw_onecell_data *data; > + struct clk_hw *reg_hw; > + int num_clk = ARRAY_SIZE(divout) + ARRAY_SIZE(stbcrout) + ARRAY_SIZE(clkstpout); > + > + data = kzalloc(struct_size(data, hws, num_clk + 1), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + num_clk = 0; > + for (i = 0; i < ARRAY_SIZE(divout); i++) { > + reg_hw = __clk_hw_register_divider(NULL, node, divout[i], > + PLLOUT, NULL, NULL, > + 0, cpg->frqcr, i * 3, 3, > + CLK_DIVIDER_REG_16BIT, > + (i == 0) ? pdiv_table : div_table, > + &cpg->clklock); > + if (IS_ERR(reg_hw)) { > + ret = PTR_ERR(reg_hw); > + goto error; > + } > + data->hws[num_clk++] = reg_hw; > + } > + for (i = 0; i < ARRAY_SIZE(stbcrout); i++) { > + u32 off = (i < 5) ? STBCR : STBCR2; > + > + if (i >= 5 && !(cpg->feat & MSTP_CR2)) > + break; Alternatively, you could set the maximum loop counter upfront n = cpg->feat & MSTP_CR2 ? ARRAY_SIZE(stbcrout) : 5; for (i = 0; i < n; i++) ... > + reg_hw = __clk_hw_register_gate(NULL, node, stbcrout[i], > + divout[0], NULL, NULL, > + 0, cpg->frqcr + off, i % 5, > + CLK_GATE_REG_8BIT | CLK_GATE_SET_TO_DISABLE, > + &cpg->clklock); > + if (IS_ERR(reg_hw)) { > + ret = PTR_ERR(reg_hw); > + goto error; > + } > + data->hws[num_clk++] = reg_hw; > + } > + if (cpg->feat & MSTP_CLKSTP) { > + for (i = 0; i < ARRAY_SIZE(clkstpout); i++) { > + if (i == 2 && !(cpg->feat & MSTP_CSTP2)) > + continue; Set maximum loop counter upfront? > + reg_hw = clk_hw_register_clkstp(node, clkstpout[i], > + divout[0], cpg->clkstp00, > + i, &cpg->clklock); > + if (IS_ERR(reg_hw)) { > + ret = PTR_ERR(reg_hw); > + goto error; > + } > + data->hws[num_clk++] = reg_hw; > + } > + } > + data->num = num_clk; > + ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, data); > + if (ret < 0) > + goto error; > + return 0; > + > +error: > + pr_err("%pOF: failed to register clock (%d)\n", > + node, ret); > + for (num_clk--; num_clk >= 0; num_clk--) > + kfree(data->hws[num_clk]); > + kfree(data); > + return ret; > +} > + > +static struct cpg_priv *sh7750_cpg_setup(struct device_node *node, u32 feat) > +{ > + unsigned int num_parents; > + u32 mode; > + struct cpg_priv *cpg; > + int ret = 0; > + > + num_parents = of_clk_get_parent_count(node); > + if (num_parents < 1) { > + pr_err("%s: no parent found", node->name); > + return ERR_PTR(-ENODEV); > + } Do you need num_parents? > + > + of_property_read_u32_index(node, "renesas,mode", 0, &mode); mode may be used uninitialized, if "renesas,mode" is missing. > + if (mode >= 7) { > + pr_err("%s: Invalid clock mode setting (%u)\n", > + node->name, mode); > + return ERR_PTR(-EINVAL); > + } > + > + cpg = kzalloc(sizeof(struct cpg_priv), GFP_KERNEL); > + if (!cpg) > + return ERR_PTR(-ENOMEM); > + > + cpg->frqcr = of_iomap(node, 0); > + if (cpg->frqcr == NULL) { > + pr_err("%pOF: failed to map divide register", node); > + ret = -ENODEV; > + goto cpg_free; > + } > + > + if (feat & MSTP_CLKSTP) { > + cpg->clkstp00 = of_iomap(node, 1); > + if (cpg->clkstp00 == NULL) { > + pr_err("%pOF: failed to map clkstp00 register", node); > + ret = -ENODEV; > + goto unmap_frqcr; > + } > + } > + cpg->feat = feat; > + cpg->mode = mode; > + > + ret = register_pll(node, cpg); > + if (ret < 0) > + goto unmap_clkstp00; > + > + ret = register_div(node, cpg); > + if (ret < 0) > + goto unmap_clkstp00; > + Perhaps "cpg_data = cpg;" here, and return an error code instead? ... > + return cpg; > + > +unmap_clkstp00: > + iounmap(cpg->clkstp00); > +unmap_frqcr: > + iounmap(cpg->frqcr); > +cpg_free: > + kfree(cpg); > + return ERR_PTR(ret); > +} > + > +static void __init sh7750_cpg_init(struct device_node *node) > +{ > + cpg_data = sh7750_cpg_setup(node, cpg_feature[CPG_SH7750]); > + if (IS_ERR(cpg_data)) > + cpg_data = NULL; ... then all cpg_data handling can be removed here... > +} > +static int sh7750_cpg_probe(struct platform_device *pdev) > +{ > + u32 feature; > + > + if (cpg_data) > + return 0; > + feature = *(u32 *)of_device_get_match_data(&pdev->dev); > + cpg_data = sh7750_cpg_setup(pdev->dev.of_node, feature); > + if (IS_ERR(cpg_data)) > + return PTR_ERR(cpg_data); > + return 0; ... and this can be simplified to return sh7750_cpg_setup(...); > +} Gr{oetje,eeting}s, Geert
On Tue, Jan 9, 2024 at 9:24 AM Yoshinori Sato <ysato@users.sourceforge.jp> wrote: > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
This is an updated version of something I wrote about 7 years ago. Minimum support for R2D-plus and LANDISK. I think R2D-1 will work if you add AX88796 to dts. And board-specific functions and SCI's SPI functions are not supported. You can get it working with qemu found here. https://gitlab.com/yoshinori.sato/qemu/-/tree/landisk v6 changes. - pci-sh7751: merge register define. - pci-sh7751: use 'dma-ranges' property. - pci-sh7751: rename general PCI properties. - sm501 and sm501fb: Re-design Device Tree properties. - sh/kernel/setup: cleanup command line setup. - irq-sh7751.c: some cleanup. v5 changes. - pci-sh7751: revert header changes. and some fix in previuous driver. - sh/kernel/iomap.c: Use SH io functions. - sm501 and sm501fb: re-write DT support. v4 changes. - cpg-sh7750: use clk-divider and clk-gate. - pci-sh7751: unified header files to old PCI driver. - irq-renesas-sh7751: IPR registers direct mapping. - irq-renesas-sh7751irl: useful register bit mapping. - sm501 and sm501fb: re-write dt parser. - j2_minus: fix build error. - dt-binding schema: fix some errors. - *.dts: cleanup. v3 changes. - Rewrite clk drivers. - Added sh_tmu to OF support. - Cleanup PCI stuff. - Update sm501 and sm501fb OF support. - Update devicetree and documents. v2 changes. - Rebasing v6,6-rc1 - re-write irqchip driver. - Add binding documents. - Cleanup review comment. Yoshinori Sato (37): sh: passing FDT address to kernel startup. sh: Kconfig unified OF supported targets. sh: Enable OF support for build and configuration. dt-bindings: interrupt-controller: Add header for Renesas SH3/4 INTC. sh: GENERIC_IRQ_CHIP support for CONFIG_OF=y sh: kernel/setup Update DT support. sh: Fix COMMON_CLK support in CONFIG_OF=y. clocksource: sh_tmu: CLOCKSOURCE support. dt-bindings: timer: renesas,tmu: add renesas,tmu-sh7750 sh: Common PCI Framework driver support. pci: pci-sh7751: Add SH7751 PCI driver dt-bindings: pci: pci-sh7751: Add SH7751 PCI dt-bindings: clock: sh7750-cpg: Add renesas,sh7750-cpg header. clk: Compatible with narrow registers clk: renesas: Add SH7750/7751 CPG Driver irqchip: Add SH7751 INTC driver dt-bindings: interrupt-controller: renesas,sh7751-intc: Add json-schema irqchip: SH7751 external interrupt encoder with enable gate. dt-bindings: interrupt-controller: renesas,sh7751-irl-ext: Add json-schema serial: sh-sci: fix SH4 OF support. dt-bindings: serial: renesas,scif: Add scif-sh7751. dt-bindings: display: smi,sm501: SMI SM501 binding json-schema mfd: sm501: Convert platform_data to OF property dt-binding: sh: cpus: Add SH CPUs json-schema dt-bindings: vendor-prefixes: Add iodata dt-bindings: vendor-prefixes: Add smi dt-bindings: ata: ata-generic: Add new targets dt-bindings: soc: renesas: sh: Add SH7751 based target sh: SH7751R SoC Internal peripheral definition dtsi. sh: add RTS7751R2D Plus DTS sh: Add IO DATA LANDISK dts sh: Add IO DATA USL-5P dts sh: j2_mimas_v2.dts update sh: Add dtbs target support. sh: RTS7751R2D Plus OF defconfig sh: LANDISK OF defconfig sh: j2_defconfig: update .../devicetree/bindings/ata/ata-generic.yaml | 2 + .../bindings/clock/renesas,sh7750-cpg.yaml | 103 ++++ .../bindings/display/smi,sm501.yaml | 417 +++++++++++++++ .../renesas,sh7751-intc.yaml | 105 ++++ .../renesas,sh7751-irl-ext.yaml | 72 +++ .../bindings/pci/renesas,sh7751-pci.yaml | 150 ++++++ .../bindings/serial/renesas,scif.yaml | 1 + .../devicetree/bindings/sh/cpus.yaml | 74 +++ .../devicetree/bindings/soc/renesas/sh.yaml | 32 ++ .../bindings/timer/renesas,tmu.yaml | 67 ++- .../devicetree/bindings/vendor-prefixes.yaml | 4 + arch/sh/Kconfig | 12 +- arch/sh/boards/Kconfig | 24 +- arch/sh/boards/of-generic.c | 28 +- arch/sh/boot/compressed/head_32.S | 5 +- arch/sh/boot/dts/Makefile | 5 + arch/sh/boot/dts/j2_mimas_v2.dts | 2 +- arch/sh/boot/dts/landisk.dts | 75 +++ arch/sh/boot/dts/rts7751r2dplus.dts | 180 +++++++ arch/sh/boot/dts/sh7751r.dtsi | 152 ++++++ arch/sh/boot/dts/usl-5p.dts | 83 +++ arch/sh/configs/j2_defconfig | 11 +- arch/sh/configs/landisk-of_defconfig | 104 ++++ arch/sh/configs/rts7751r2dplus-of_defconfig | 80 +++ arch/sh/drivers/Makefile | 2 + arch/sh/include/asm/io.h | 8 + arch/sh/include/asm/irq.h | 10 +- arch/sh/include/asm/pci.h | 4 + arch/sh/kernel/cpu/Makefile | 6 +- arch/sh/kernel/cpu/irq/imask.c | 17 + arch/sh/kernel/cpu/sh4/Makefile | 3 + arch/sh/kernel/iomap.c | 18 + arch/sh/kernel/setup.c | 33 +- arch/sh/kernel/time.c | 12 + drivers/clk/clk-divider.c | 56 +- drivers/clk/clk-gate.c | 56 +- drivers/clk/renesas/Kconfig | 16 +- drivers/clk/renesas/Makefile | 1 + drivers/clk/renesas/clk-sh7750.c | 498 ++++++++++++++++++ drivers/clocksource/sh_tmu.c | 161 ++++-- drivers/irqchip/Kconfig | 15 + drivers/irqchip/Makefile | 3 + drivers/irqchip/irq-renesas-sh7751.c | 313 +++++++++++ drivers/irqchip/irq-renesas-sh7751irl.c | 228 ++++++++ drivers/mfd/sm501.c | 436 +++++++++++++++ drivers/pci/controller/Kconfig | 9 + drivers/pci/controller/Makefile | 1 + drivers/pci/controller/pci-sh7751.c | 392 ++++++++++++++ drivers/tty/serial/Kconfig | 2 +- drivers/tty/serial/sh-sci.c | 6 +- drivers/video/fbdev/sm501fb.c | 106 ++++ include/dt-bindings/clock/sh7750-cpg.h | 26 + .../renesas,sh7751-intc.h | 19 + include/linux/clk-provider.h | 22 +- include/linux/sh_intc.h | 7 +- 55 files changed, 4096 insertions(+), 178 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/renesas,sh7750-cpg.yaml create mode 100644 Documentation/devicetree/bindings/display/smi,sm501.yaml create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-intc.yaml create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml create mode 100644 Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml create mode 100644 Documentation/devicetree/bindings/sh/cpus.yaml create mode 100644 Documentation/devicetree/bindings/soc/renesas/sh.yaml create mode 100644 arch/sh/boot/dts/landisk.dts create mode 100644 arch/sh/boot/dts/rts7751r2dplus.dts create mode 100644 arch/sh/boot/dts/sh7751r.dtsi create mode 100644 arch/sh/boot/dts/usl-5p.dts create mode 100644 arch/sh/configs/landisk-of_defconfig create mode 100644 arch/sh/configs/rts7751r2dplus-of_defconfig create mode 100644 drivers/clk/renesas/clk-sh7750.c create mode 100644 drivers/irqchip/irq-renesas-sh7751.c create mode 100644 drivers/irqchip/irq-renesas-sh7751irl.c create mode 100644 drivers/pci/controller/pci-sh7751.c create mode 100644 include/dt-bindings/clock/sh7750-cpg.h create mode 100644 include/dt-bindings/interrupt-controller/renesas,sh7751-intc.h