From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Andr=c3=a9_Przywara?= Date: Thu, 26 Mar 2020 17:06:02 +0000 Subject: [PATCH 2/7] uart: pl011: Add proper DM clock support In-Reply-To: References: <20200325144702.16288-1-andre.przywara@arm.com> <20200325144702.16288-3-andre.przywara@arm.com> Message-ID: <4f82e12b-7607-224e-9162-66bd656bf2ae@arm.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 26/03/2020 16:20, Simon Glass wrote: Hi Simon, > On Wed, 25 Mar 2020 at 08:47, 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 >> --- >> drivers/serial/serial_pl01x.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> 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 >> #include >> +#include >> #include >> #include >> #include >> @@ -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); > > is this needed? This is to provide the existing behaviour as a fallback. Some SoCs have a complex clock providing the baud rate clock (HiKey 960, FSL LS2080a), which U-Boot doesn't suport. I'd rather not break them, but also don't really want to provide a clock driver ;-) Also this mimics the !DM_SERIAL behaviour, which sets this clock rate based on Kconfig, again as a fallback. I needed that because I think the clock driver wasn't ready that early. It's a bit hard to confirm without serial output ;-) So the order should be: - If there is a clocks property and we support that clock provider (fixed-clock), then use that value. - If not, check for a "clock" property in the DT node and use that value. - If there is no "clock property", use the Kconfig variable. Just written the other way around in the code. Does this make sense? Cheers, Andre. >> + 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"); >> >> -- >> 2.14.5 >> > > Regards, > Simon >