Hi Sam, On Fri, 10 Nov 2023 at 11:29, Sam Protsenko <semen.protse...@linaro.org> wrote: > > Hi Simon, > > On Tue, Nov 7, 2023 at 10:26 PM Simon Glass <s...@chromium.org> wrote: > > > > Hi Sam, > > > > On Tue, 7 Nov 2023 at 12:06, Sam Protsenko <semen.protse...@linaro.org> > > wrote: > > > > > > Use dev_read_u8_default() instead of fdtdec_get_int() to read the "id" > > > property from device tree, as suggested in [1]. dev_* API is already > > > used in this driver, so there is no reason to stick to fdtdec_* API. > > > This also fixes checkpatch warning: > > > > > > WARNING: Use the livetree API (dev_read_...) > > > > > > [1] doc/develop/driver-model/livetree.rst > > > > > > Signed-off-by: Sam Protsenko <semen.protse...@linaro.org> > > > --- > > > drivers/serial/serial_s5p.c | 5 +---- > > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > > > diff --git a/drivers/serial/serial_s5p.c b/drivers/serial/serial_s5p.c > > > index 177215535676..c57bdd059ea6 100644 > > > --- a/drivers/serial/serial_s5p.c > > > +++ b/drivers/serial/serial_s5p.c > > > @@ -20,8 +20,6 @@ > > > #include <serial.h> > > > #include <clk.h> > > > > > > -DECLARE_GLOBAL_DATA_PTR; > > > - > > > enum { > > > PORT_S5P = 0, > > > PORT_S5L > > > @@ -220,8 +218,7 @@ static int s5p_serial_of_to_plat(struct udevice *dev) > > > > > > plat->reg = (struct s5p_uart *)addr; > > > plat->reg_width = dev_read_u32_default(dev, "reg-io-width", 1); > > > - plat->port_id = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), > > > - "id", dev_seq(dev)); > > > + plat->port_id = dev_read_u8_default(dev, "id", dev_seq(dev)); > > > > > > if (port_type == PORT_S5L) { > > > plat->rx_fifo_count_shift = S5L_RX_FIFO_COUNT_SHIFT; > > > -- > > > 2.39.2 > > > > > > > Really this property should not be needed anymore. Can you figure out > > how to drop it? > > > > The 'port_id' seems to be needed for ARCH_EXYNOS4 boards. Because > Exynos4 doesn't have proper DM clocks, it uses 'id' property to get > corresponding UART clock frequency from its mach code. > > Here is what's happening in the serial driver in case of Exynos4: > > 1. get_uart_clk(port_id) is called > 2. which in turn calls exynos4_get_uart_clk(port_id) > 3. which uses "port_id" value to read corresponding bits of of > CLK_SRC_PERIL0 register > 4. those bits are used to get corresponding PLL clock's frequency > 5. which is in turn used to calculate UART clock rate > 6. calculated rate is returned by get_uart_clk() to serial driver > > So I don't see any *easy* way we can get rid of that id property. > > The proper way of doing that would require converting Exynos4 clock > code to CCF (enabling CONFIG_CLK_EXYNOS). Which of course also means > implementing clocks in dts, akin to kernel's exynos4.dtsi. Then it'll > be possible to get rid of 'id' property.
That sounds good! > > Maybe I'm missing something, please let me know. An easy way in the meantime would be to look at the compatible / reg property, e.g. here is a sketch: static int get_id(ofnode node) { ulong addr = (ulong)plat->reg; if (ofnode_device_is_compatible(node, "samsung,exynos4210-uart")) { return (addr >> 16) & 0xf; ... reg = <0x13800000 0x3c>; id = <0>; }; serail_1: serial@13810000 { compatible = "samsung,exynos4210-uart"; reg = <0x13810000 0x3c>; id = <1>; }; serial_2: serial@13820000 { compatible = "samsung,exynos4210-uart"; reg = <0x13820000 0x3c>; id = <2>; }; serial_3: serial@13830000 { compatible = "samsung,exynos4210-uart"; reg = <0x13830000 0x3c>; id = <3>; }; serial_4: serial@13840000 { compatible = "samsung,exynos4210-uart"; reg = <0x13840000 0x3c>; id = <4>; Regards, Simon