| Message ID | 20200407111236.14930-3-andre.przywara@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Arm Juno board OF_CONTROL upgrade | expand |
On Tue, Apr 07, 2020 at 12:12:31PM +0100, Andre Przywara wrote: > Even though the PL011 UART driver claims to be DM compliant, it does not > really a good job with parsing DT nodes. U-Boot seems to adhere to a > non-standard binding, either requiring to have a "skip-init" property in > the node, or to have an extra "clock" property holding the base > *frequency* value for the baud rate generator. > DTs in the U-Boot tree seem to have been hacked to match this > requirement. > > The official binding does not mention any of these properties, instead > recommends a standard "clocks" property to point to the baud base clock. > > Some boards use simple "fixed-clock" providers, which U-Boot readily > supports, so let's add some simple DM clock code to the PL011 driver to > learn the rate of the first clock, as described by the official binding. > > These clock nodes seem to be not ready very early in the boot process, > so provide a fallback value, by re-using the already existing > CONFIG_PL011_CLOCK variable. > > Signed-off-by: Andre Przywara <andre.przywara at arm.com> > Reviewed-by: Simon Glass <sjg at chromium.org> This patch breaks rpi_3*/rpi_4/rpi_arm64, please fix, thanks!
On 24/04/2020 15:51, Tom Rini wrote: Hi Tom, > On Tue, Apr 07, 2020 at 12:12:31PM +0100, Andre Przywara wrote: > >> Even though the PL011 UART driver claims to be DM compliant, it does not >> really a good job with parsing DT nodes. U-Boot seems to adhere to a >> non-standard binding, either requiring to have a "skip-init" property in >> the node, or to have an extra "clock" property holding the base >> *frequency* value for the baud rate generator. >> DTs in the U-Boot tree seem to have been hacked to match this >> requirement. >> >> The official binding does not mention any of these properties, instead >> recommends a standard "clocks" property to point to the baud base clock. >> >> Some boards use simple "fixed-clock" providers, which U-Boot readily >> supports, so let's add some simple DM clock code to the PL011 driver to >> learn the rate of the first clock, as described by the official binding. >> >> These clock nodes seem to be not ready very early in the boot process, >> so provide a fallback value, by re-using the already existing >> CONFIG_PL011_CLOCK variable. >> >> Signed-off-by: Andre Przywara <andre.przywara at arm.com> >> Reviewed-by: Simon Glass <sjg at chromium.org> > > This patch breaks rpi_3*/rpi_4/rpi_arm64, please fix, thanks! Doh, thanks for the heads up and apologies for not spotting this myself. I fixed this up by catching a possibly undefined CONFIG_PL011_CLOCK. Looking deeper into this code I think we can get rid of the unofficial skip-init property as well, but that's another patch. Will send a (rebased) v3 shortly. Thanks! Andre.
diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c index 2a5f256184..1ab0ccadb2 100644 --- a/drivers/serial/serial_pl01x.c +++ b/drivers/serial/serial_pl01x.c @@ -12,6 +12,7 @@ #include <common.h> #include <dm.h> +#include <clk.h> #include <errno.h> #include <watchdog.h> #include <asm/io.h> @@ -340,14 +341,21 @@ static const struct udevice_id pl01x_serial_id[] ={ int pl01x_serial_ofdata_to_platdata(struct udevice *dev) { struct pl01x_serial_platdata *plat = dev_get_platdata(dev); + struct clk clk; fdt_addr_t addr; + int ret; addr = devfdt_get_addr(dev); if (addr == FDT_ADDR_T_NONE) return -EINVAL; plat->base = addr; - plat->clock = dev_read_u32_default(dev, "clock", 1); + plat->clock = dev_read_u32_default(dev, "clock", CONFIG_PL011_CLOCK); + ret = clk_get_by_index(dev, 0, &clk); + if (!ret) { + clk_enable(&clk); + plat->clock = clk_get_rate(&clk); + } plat->type = dev_get_driver_data(dev); plat->skip_init = dev_read_bool(dev, "skip-init");