Message ID | 1430516932-31542-1-git-send-email-vikas.manocha@st.com |
---|---|
State | Changes Requested |
Delegated to: | Simon Glass |
Headers | show |
+Masahiro, for my of_match_ptr() comment below. Hi Vikas, On 1 May 2015 at 15:48, Vikas Manocha <vikas.manocha@st.com> wrote: > This patch adds device tree support for arm pl010/pl011 driver. > > Signed-off-by: Vikas Manocha <vikas.manocha@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}, > + {} > +}; > + > +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); > + 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. > + > + 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. For the first one you can do .of_match = of_match_ptr(pl01x_serial_id), 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
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.manocha@st.com> wrote: >> This patch adds device tree support for arm pl010/pl011 driver. >> >> Signed-off-by: Vikas Manocha <vikas.manocha@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? > >> .probe = pl01x_serial_probe, >> .ops = &pl01x_serial_ops, >> .flags = DM_FLAG_PRE_RELOC, >> -- >> 1.7.9.5 >> > Regards, > Simon
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.manocha@st.com> wrote: >>> This patch adds device tree support for arm pl010/pl011 driver. >>> >>> Signed-off-by: Vikas Manocha <vikas.manocha@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
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"}, + {} +}; + +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); + if(!strcmp(type_pl01x, "arm,pl011")) + plat->type = TYPE_PL011; + else if(!strcmp(type_pl01x, "arm,pl010")) + plat->type = TYPE_PL010; + else + return -EINVAL; + + 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 .probe = pl01x_serial_probe, .ops = &pl01x_serial_ops, .flags = DM_FLAG_PRE_RELOC,
This patch adds device tree support for arm pl010/pl011 driver. Signed-off-by: Vikas Manocha <vikas.manocha@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