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;elsereturn -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