| Message ID | 1411515008-4483-10-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
On 09/23/2014 05:30 PM, Simon Glass wrote: > Since we currently don't have device tree available in SPL, add platform > data so the uart works. > diff --git a/drivers/serial/serial_tegra.c b/drivers/serial/serial_tegra.c > +#ifdef CONFIG_OF_CONTROL > static const struct udevice_id tegra_serial_ids[] = { > { .compatible = "nvidia,tegra20-uart" }, > { } > @@ -26,13 +27,28 @@ static int tegra_serial_ofdata_to_platdata(struct udevice *dev) > > return 0; > } > +#else > +struct ns16550_platdata tegra_serial = { > + .base = CONFIG_SYS_NS16550_COM1, > + .reg_shift = 2, > + .clock = V_NS16550_CLK, > +}; > + > +U_BOOT_DEVICE(ns16550_serial) = { > + "serial_tegra20", &tegra_serial > +}; > +#endif If we're going to have to add "hard-coded" instantiation of devices to the code to support SPL, I rather wonder what the point is of having DT support in these (or honestly, any) drivers at all. Why not just switch directly to always using the hard-coded instantiation. Then, we can drop all the DT parsing code from the drivers, simplify the code, and completely avoid all the annoying DT ABI and binding consistency issues. > U_BOOT_DRIVER(serial_ns16550) = { > .name = "serial_tegra20", > .id = UCLASS_SERIAL, > +#ifdef CONFIG_OF_CONTROL > .of_match = tegra_serial_ids, > .ofdata_to_platdata = tegra_serial_ofdata_to_platdata, > .platdata_auto_alloc_size = sizeof(struct ns16550_platdata), > +#endif > .priv_auto_alloc_size = sizeof(struct NS16550), > .probe = ns16550_serial_probe, > .ops = &ns16550_serial_ops, > + .flags = DM_FLAG_PRE_RELOC, > };
Hi Stephen, On 24 September 2014 09:42, Stephen Warren <swarren@wwwdotorg.org> wrote: > > On 09/23/2014 05:30 PM, Simon Glass wrote: >> >> Since we currently don't have device tree available in SPL, add platform >> data so the uart works. > > >> diff --git a/drivers/serial/serial_tegra.c b/drivers/serial/serial_tegra.c > > >> +#ifdef CONFIG_OF_CONTROL >> static const struct udevice_id tegra_serial_ids[] = { >> { .compatible = "nvidia,tegra20-uart" }, >> { } >> @@ -26,13 +27,28 @@ static int tegra_serial_ofdata_to_platdata(struct udevice *dev) >> >> return 0; >> } >> +#else >> +struct ns16550_platdata tegra_serial = { >> + .base = CONFIG_SYS_NS16550_COM1, >> + .reg_shift = 2, >> + .clock = V_NS16550_CLK, >> +}; >> + >> +U_BOOT_DEVICE(ns16550_serial) = { >> + "serial_tegra20", &tegra_serial >> +}; >> +#endif > > > If we're going to have to add "hard-coded" instantiation of devices to the code to support SPL, I rather wonder what the point is of having DT support in these (or honestly, any) drivers at all. Why not just switch directly to always using the hard-coded instantiation. Then, we can drop all the DT parsing code from the drivers, simplify the code, and completely avoid all the annoying DT ABI and binding consistency issues. This is a temporary state along the way to perfection. It just replaces the existing CONFIG options which currently feed into the driver, which are currently not in the device tree in SPL. At present we only use device tree for U-Boot, not for SPL. I don't see the point of duplicating all the device tree information in a different format. I also feel that the binding inconsistencies are reducing now. Your issue with the UART clock was resolved (on OMAP the clock is in the binding but not on Tegra). Also this isn't really hard-coded. Both CONFIG_SYS_NS16550_COM1 and V_NS16550_CLK are actually board-specific. I've just moved it out of the ns16550 driver and into a tegra file. Regards, Simon
diff --git a/drivers/serial/serial_tegra.c b/drivers/serial/serial_tegra.c index 7eb70e1..b9227f0 100644 --- a/drivers/serial/serial_tegra.c +++ b/drivers/serial/serial_tegra.c @@ -9,6 +9,7 @@ #include <ns16550.h> #include <serial.h> +#ifdef CONFIG_OF_CONTROL static const struct udevice_id tegra_serial_ids[] = { { .compatible = "nvidia,tegra20-uart" }, { } @@ -26,13 +27,28 @@ static int tegra_serial_ofdata_to_platdata(struct udevice *dev) return 0; } +#else +struct ns16550_platdata tegra_serial = { + .base = CONFIG_SYS_NS16550_COM1, + .reg_shift = 2, + .clock = V_NS16550_CLK, +}; + +U_BOOT_DEVICE(ns16550_serial) = { + "serial_tegra20", &tegra_serial +}; +#endif + U_BOOT_DRIVER(serial_ns16550) = { .name = "serial_tegra20", .id = UCLASS_SERIAL, +#ifdef CONFIG_OF_CONTROL .of_match = tegra_serial_ids, .ofdata_to_platdata = tegra_serial_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct ns16550_platdata), +#endif .priv_auto_alloc_size = sizeof(struct NS16550), .probe = ns16550_serial_probe, .ops = &ns16550_serial_ops, + .flags = DM_FLAG_PRE_RELOC, };
Since we currently don't have device tree available in SPL, add platform data so the uart works. Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/serial/serial_tegra.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)