Hello Masahiro, On 05/01/2015 03:32 PM, vikasm wrote: > Thanks Simon, > > On 05/01/2015 03:02 PM, Simon Glass wrote: >> +Masahiro, for my of_match_ptr() comment below. >> >> Hi Vikas, >> >> On 1 May 2015 at 15:48, Vikas Manocha <vikas.mano...@st.com> wrote: >>> This patch adds device tree support for arm pl010/pl011 driver. >>> >>> Signed-off-by: Vikas Manocha <vikas.mano...@st.com> >>> --- >>> doc/device-tree-bindings/serial/pl01x.txt | 7 +++++ >>> drivers/serial/serial_pl01x.c | 41 >>> ++++++++++++++++++++++++++++- >>> 2 files changed, 47 insertions(+), 1 deletion(-) >>> create mode 100644 doc/device-tree-bindings/serial/pl01x.txt >>> >>> diff --git a/doc/device-tree-bindings/serial/pl01x.txt >>> b/doc/device-tree-bindings/serial/pl01x.txt >>> new file mode 100644 >>> index 0000000..61c27d1 >>> --- /dev/null >>> +++ b/doc/device-tree-bindings/serial/pl01x.txt >>> @@ -0,0 +1,7 @@ >>> +* ARM AMBA Primecell PL011 & PL010 serial UART >>> + >>> +Required properties: >>> +- compatible: must be "arm,primecell", "arm,pl011" or "arm,pl010" >>> +- reg: exactly one register range with length 0x1000 >>> +- clock: input clock frequency for the UART (used to calculate the baud >>> + rate divisor) >>> diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c >>> index 2124161..ea22cfd 100644 >>> --- a/drivers/serial/serial_pl01x.c >>> +++ b/drivers/serial/serial_pl01x.c >>> @@ -20,6 +20,9 @@ >>> #include <dm/platform_data/serial_pl01x.h> >>> #include <linux/compiler.h> >>> #include "serial_pl01x_internal.h" >>> +#include <fdtdec.h> >>> + >>> +DECLARE_GLOBAL_DATA_PTR; >>> >>> #ifndef CONFIG_DM_SERIAL >>> >>> @@ -28,7 +31,6 @@ static enum pl01x_type pl01x_type __attribute__ >>> ((section(".data"))); >>> static struct pl01x_regs *base_regs __attribute__ ((section(".data"))); >>> #define NUM_PORTS (sizeof(port)/sizeof(port[0])) >>> >>> -DECLARE_GLOBAL_DATA_PTR; >>> #endif >>> >>> static int pl01x_putc(struct pl01x_regs *regs, char c) >>> @@ -351,9 +353,46 @@ static const struct dm_serial_ops pl01x_serial_ops = { >>> .setbrg = pl01x_serial_setbrg, >>> }; >>> >>> +#ifdef CONFIG_OF_CONTROL >>> +static const struct udevice_id pl01x_serial_id[] ={ >>> + {.compatible = "arm,pl011"}, >>> + {.compatible = "arm,pl010"}, >> You can use: >> >> {.compatible = "arm,pl011", .data = TYPE_PL011}, > Thanks for the suggestion. > >>> + {} >>> +}; >>> + >>> +static int pl01x_serial_ofdata_to_platdata(struct udevice *dev) >>> +{ >>> + struct pl01x_serial_platdata *plat = dev_get_platdata(dev); >>> + fdt_addr_t addr; >>> + const char* type_pl01x; >>> + >>> + addr = fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg"); >>> + if (addr == FDT_ADDR_T_NONE) >>> + return -EINVAL; >>> + >>> + plat->base = addr; >>> + plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "clock", >>> 1); >>> + type_pl01x = fdt_getprop(gd->fdt_blob, dev->of_offset, \ >>> + "compatible", NULL); >> plat->type = dev_get_driver_data(dev); > completely agree, it would make the code much cleaner. > >>> + if(!strcmp(type_pl01x, "arm,pl011")) >>> + plat->type = TYPE_PL011; >>> + else if(!strcmp(type_pl01x, "arm,pl010")) >>> + plat->type = TYPE_PL010; >>> + else >>> + return -EINVAL; >> Should be able to drop this line. > yes, all the above block. > >>> + >>> + return 0; >>> +} >>> +#endif >>> + >>> U_BOOT_DRIVER(serial_pl01x) = { >>> .name = "serial_pl01x", >>> .id = UCLASS_SERIAL, >>> +#ifdef CONFIG_OF_CONTROL >>> + .of_match = pl01x_serial_id, >>> + .ofdata_to_platdata = pl01x_serial_ofdata_to_platdata, >>> + .platdata_auto_alloc_size = sizeof(struct pl01x_serial_platdata), >>> +#endif >> Would be good to get rid of the #ifdef. >> >> I think you can put the last one outside the #ifdef, since >> device_bind() will do the right thing if there is already platform >> data. > ok. > >> For the first one you can do .of_match = of_match_ptr(pl01x_serial_id), > ok, i will check it. > > Rgds, > Vikas > >> But the middle line (ofdata_to_platdata) does need an #ifdef I think. >> Perhaps we could create something similar to of_match_ptr() for this >> case?
Any suggestion on this point..can we use of_match_ptr() ? Rgds, Vikas >> >>> .probe = pl01x_serial_probe, >>> .ops = &pl01x_serial_ops, >>> .flags = DM_FLAG_PRE_RELOC, >>> -- >>> 1.7.9.5 >>> >> Regards, >> Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot