| Message ID | cover.1701768028.git.ysato@users.sourceforge.jp |
|---|---|
| Headers | show |
| Series | Device Tree support for SH7751 based board | expand |
On Tue, 05 Dec 2023, 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 | 99 +++++++++++++++++++++++++++++++++++ > drivers/video/fbdev/sm501fb.c | 82 +++++++++++++++++++++++++++++ > 2 files changed, 181 insertions(+) > > diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c > index 28027982cf69..f0104fdf0f34 100644 > --- a/drivers/mfd/sm501.c > +++ b/drivers/mfd/sm501.c > @@ -1370,6 +1370,99 @@ static int sm501_init_dev(struct sm501_devdata *sm) > return 0; > } > > +static void sm501_of_read_reg_init(struct device_node *np, > + const char *propname, struct sm501_reg_init *val) > +{ > + u32 u32_val[2]; Encoding the size of the variable in the variable name is odd. > + > + if (!of_property_read_u32_array(np, propname, u32_val, sizeof(u32_val))) { > + val->set = u32_val[0]; > + val->mask = u32_val[1]; Masks and register values stored as DT properties? This is generally not permitted. Please seek counsel from the DT maintainers. > + } > +} > + > +/* Read GPIO I2C configuration */ > +static int sm501_parse_dt_gpio_i2c(struct device *dev, struct sm501_platdata *plat, > + struct device_node *np) > +{ > + struct sm501_platdata_gpio_i2c *gpio_i2c_p; > + struct property *prop; > + u32 gpio_i2c[5]; Why 5? Please define all magic numbers. > + const __be32 *p; > + unsigned int i; > + u32 i2c_nr; > + > + prop = of_find_property(np, "smi,gpio-i2c", NULL); > + if (!prop) > + return 0; > + > + i2c_nr = of_property_count_u32_elems(np, "smi,gpio-i2c"); Why do you need both of these probing functions? What does of_property_count_u32_elems() return if smi,gpio-i2c doesn't exist? > + /* GPIO I2C define 5 words per channel. */ > + if (i2c_nr % 5) > + return -EINVAL; > + i2c_nr /= 5; '\n' > + plat->gpio_i2c = devm_kzalloc(dev, sizeof(*plat->gpio_i2c) * i2c_nr, > + GFP_KERNEL); > + if (!plat->gpio_i2c) > + return -ENOMEM; > + > + plat->gpio_i2c_nr = i2c_nr; > + gpio_i2c_p = plat->gpio_i2c; What's the purpose of this intermediary variable? > + > + for (; i2c_nr > 0; i2c_nr--) { You can define 'p' in here, right? > + for (i = 0; i < ARRAY_SIZE(gpio_i2c); i++) { > + p = of_prop_next_u32(prop, p, &gpio_i2c[i]); > + if (!p) > + return -EINVAL; > + } > + gpio_i2c_p->bus_num = gpio_i2c[0]; > + gpio_i2c_p->pin_sda = gpio_i2c[1]; > + gpio_i2c_p->pin_scl = gpio_i2c[2]; > + gpio_i2c_p->udelay = gpio_i2c[3]; > + gpio_i2c_p->timeout = gpio_i2c[4]; I'm not even going to ask. I'll leave this to the DT maintainers. > + gpio_i2c_p++; > + } > + return 0; > +} > + > +/* Build platform_data from OF property */ > +static int sm501_parse_dt(struct sm501_devdata *sm, struct device_node *np) > +{ > + struct sm501_platdata *plat; > + u32 u32_val; > + int ret; > + > + plat = devm_kzalloc(sm->dev, sizeof(*plat), GFP_KERNEL); > + if (!plat) > + return -ENOMEM; > + > + plat->init = devm_kzalloc(sm->dev, sizeof(*plat->init), GFP_KERNEL); Why not grab all of the memory at once? Maybe make this 'init' thing a non-pointer. > + if (!plat->init) > + return -ENOMEM; > + > + if (!of_property_read_u32(np, "smi,devices", &u32_val)) > + plat->init->devices = u32_val; What happens if you do: of_property_read_u32(np, "smi,devices", &plat->init->devices); > + if (!of_property_read_u32(np, "smi,mclk", &u32_val)) > + plat->init->mclk = u32_val; > + if (!of_property_read_u32(np, "smi,m1xclk", &u32_val)) > + plat->init->m1xclk = u32_val; > + > + sm501_of_read_reg_init(np, "smi,misc-timing", &plat->init->misc_timing); > + sm501_of_read_reg_init(np, "smi,misc-control", &plat->init->misc_control); > + sm501_of_read_reg_init(np, "smi,gpio-low", &plat->init->gpio_low); > + sm501_of_read_reg_init(np, "smi,gpio-high", &plat->init->gpio_high); > + > + if (IS_ENABLED(CONFIG_MFD_SM501_GPIO) && > + (plat->init->devices & SM501_USE_GPIO)) { That's over-bracketed, right? plat->init->devices is a bit mask of enable devices stored in DT? Okay, I think this is going to need a lot of work on the DT side. Leaving the review here for now. > + ret = sm501_parse_dt_gpio_i2c(sm->dev, plat, np); > + if (ret) > + return ret; > + } > + sm->platdata = plat; > + return 0; > +} > + > static int sm501_plat_probe(struct platform_device *dev) > { > struct sm501_devdata *sm; > @@ -1406,6 +1499,12 @@ static int sm501_plat_probe(struct platform_device *dev) > goto err_res; > } > > + if (IS_ENABLED(CONFIG_OF) && dev->dev.of_node) { > + ret = sm501_parse_dt(sm, dev->dev.of_node); > + if (ret) > + goto err_res; > + } > + > platform_set_drvdata(dev, sm); > > sm->regs = ioremap(sm->io_res->start, resource_size(sm->io_res)); > diff --git a/drivers/video/fbdev/sm501fb.c b/drivers/video/fbdev/sm501fb.c > index d6fdc1737cd2..d35285819d28 100644 > --- a/drivers/video/fbdev/sm501fb.c > +++ b/drivers/video/fbdev/sm501fb.c > @@ -1932,6 +1932,82 @@ static int sm501fb_start_one(struct sm501fb_info *info, > return 0; > } > > +#if defined(CONFIG_OF) > +/* parse CRT / panel configuration */ > +static struct sm501_platdata_fbsub *dt_fbsub(struct device *dev, > + struct device_node *np, > + const char *name) > +{ > + struct sm501_platdata_fbsub *fbsub = NULL; > + struct fb_videomode *def_mode = NULL; > + struct device_node *child; > + const void *p_edid; > + u32 flags = 0; > + u32 bpp = 0; > + int len; > + > + child = of_get_child_by_name(np, name); > + if (child == NULL) > + return NULL; > + > + p_edid = of_get_property(child, "edid", &len); > + if (p_edid && len == EDID_LENGTH) { > + struct fb_monspecs *specs; > + u8 *edid; > + > + edid = kmemdup(p_edid, EDID_LENGTH, GFP_KERNEL); > + if (edid) { > + specs = kzalloc(sizeof(*specs), GFP_KERNEL); > + if (specs) { > + fb_edid_to_monspecs(edid, specs); > + def_mode = specs->modedb; > + } > + } > + kfree(edid); > + } > + > + of_property_read_u32(child, "bpp", &bpp); > + > + /* If flags property is obtained, fbsub is returned. */ > + if (!of_property_read_u32(child, "smi,flags", &flags)) { > + fbsub = devm_kzalloc(dev, sizeof(*fbsub), GFP_KERNEL); > + if (fbsub) { > + fbsub->def_mode = def_mode; > + fbsub->def_bpp = bpp; > + fbsub->flags = flags; > + } > + } > + return fbsub; > +} > + > +/* Build platform_data from OF property */ > +static struct sm501_platdata_fb *pdata_from_dt(struct device *dev, struct device_node *np) > +{ > + enum sm501_fb_routing fb_route = SM501_FB_OWN; > + struct sm501_platdata_fb *pdata = NULL; > + struct sm501_platdata_fbsub *fb_crt; > + struct sm501_platdata_fbsub *fb_pnl; > + unsigned int flags = 0; > + > + if (of_property_read_bool(np, "route-crt-panel")) > + fb_route = SM501_FB_CRT_PANEL; > + if (of_property_read_bool(np, "swap-fb-endian")) > + flags = SM501_FBPD_SWAP_FB_ENDIAN; > + fb_crt = dt_fbsub(dev, np, "crt"); > + fb_pnl = dt_fbsub(dev, np, "panel"); > + if (fb_crt || fb_pnl) { > + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); > + if (pdata) { > + pdata->fb_route = fb_route; > + pdata->flags = flags; > + pdata->fb_crt = fb_crt; > + pdata->fb_pnl = fb_pnl; > + } > + } > + return pdata; > +} > +#endif > + > static int sm501fb_probe(struct platform_device *pdev) > { > struct sm501fb_info *info; > @@ -1974,6 +2050,12 @@ static int sm501fb_probe(struct platform_device *pdev) > if (info->edid_data) > found = 1; > } > + /* Get platform data compatible configuration */ > + if (!found) { > + info->pdata = pdata_from_dt(dev, np); > + if (info->pdata) > + found = 1; > + } > } > #endif > if (!found) { > -- > 2.39.2 >
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 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 IRL external 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 | 134 +++++ .../renesas,sh7751-intc.yaml | 105 ++++ .../renesas,sh7751-irl-ext.yaml | 83 +++ .../bindings/pci/renesas,sh7751-pci.yaml | 128 +++++ .../bindings/serial/renesas,scif.yaml | 1 + .../devicetree/bindings/sh/cpus.yaml | 73 +++ .../devicetree/bindings/soc/renesas/sh.yaml | 32 ++ .../bindings/timer/renesas,tmu.yaml | 11 +- .../devicetree/bindings/vendor-prefixes.yaml | 4 + arch/sh/Kconfig | 11 +- 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 | 74 +++ arch/sh/boot/dts/rts7751r2dplus.dts | 157 ++++++ arch/sh/boot/dts/sh7751r.dtsi | 150 ++++++ arch/sh/boot/dts/usl-5p.dts | 84 +++ arch/sh/configs/j2_defconfig | 11 +- arch/sh/configs/landisk-of_defconfig | 111 ++++ arch/sh/configs/rts7751r2dplus-of_defconfig | 93 ++++ arch/sh/drivers/Makefile | 2 + arch/sh/drivers/pci/pci.c | 6 - arch/sh/include/asm/io.h | 6 + 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 | 24 + arch/sh/kernel/setup.c | 51 +- 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 | 290 ++++++++++ drivers/irqchip/irq-renesas-sh7751irl.c | 227 ++++++++ drivers/mfd/sm501.c | 99 ++++ drivers/pci/controller/Kconfig | 9 + drivers/pci/controller/Makefile | 1 + drivers/pci/controller/pci-sh7751.c | 302 +++++++++++ drivers/pci/controller/pci-sh7751.h | 76 +++ drivers/tty/serial/Kconfig | 2 +- drivers/tty/serial/sh-sci.c | 6 +- drivers/video/fbdev/sm501fb.c | 82 +++ include/dt-bindings/clock/sh7750-cpg.h | 26 + include/dt-bindings/display/sm501.h | 25 + .../interrupt-controller/sh7751-intc.h | 21 + include/linux/clk-provider.h | 22 +- include/linux/sh_intc.h | 7 +- 58 files changed, 3396 insertions(+), 177 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 drivers/pci/controller/pci-sh7751.h create mode 100644 include/dt-bindings/clock/sh7750-cpg.h create mode 100644 include/dt-bindings/display/sm501.h create mode 100644 include/dt-bindings/interrupt-controller/sh7751-intc.h