From mboxrd@z Thu Jan 1 00:00:00 1970 From: vikasm Date: Fri, 1 May 2015 15:32:05 -0700 Subject: [U-Boot] [PATCH] serial: fdt: add device tree support for pl01x In-Reply-To: References: <1430516932-31542-1-git-send-email-vikas.manocha@st.com> Message-ID: <5543FEE5.3010207@st.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 wrote: >> This patch adds device tree support for arm pl010/pl011 driver. >> >> Signed-off-by: Vikas Manocha >> --- >> 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 >> #include >> #include "serial_pl01x_internal.h" >> +#include >> + >> +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? > >> .probe = pl01x_serial_probe, >> .ops = &pl01x_serial_ops, >> .flags = DM_FLAG_PRE_RELOC, >> -- >> 1.7.9.5 >> > Regards, > Simon