| Message ID | 20190212214022.9446-1-stuart.menefy@mathembedded.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | tty: serial: samsung: Enable baud clock during initialisation | expand |
Hi Stuart, On 2019-02-12 22:40, Stuart Menefy wrote: > The Exynos 5260, like the 5433, appears to require baud clock as > well as pclk to be running before accessing any of the registers, > otherwise an external abort is raised. > > The serial driver already enables baud clock when required, but only > if it knows which clock is baud clock. On older SoCs baud clock may be > selected from a number of possible clocks so to support this the driver > only selects which clock to use for baud clock when a port is opened, > at which point the desired baud rate is known and the best clock can be > selected. > > The result is that there are a number of circumstances in which > registers are accessed without first explicitly enabling baud clock: > - while the driver is being initialised > - the initial parts of opening a port for the first time > - when resuming if the port hasn't been already opened > > The 5433 overcomes this currently by marking the baud clock as > CLK_IGNORE_UNUSED, so the clock is always enabled, however > for the 5260 I've been trying to avoid this. > > This change adds code to pick the first available clock to use > as baud clock and enables it while initialising the driver. > > This code wouldn't be sufficient on a SoC which supports > multiple possible baud clock sources _and_ requires the > correct baud clock to be enabled before accessing any of the > serial port registers (in particular the register which selects > which clock to use as the baud clock). As far as I know > such hardware doesn't exist. I think that we can use a simpler approach. The driver can simply keep baud clock enabled together with pclk always when there is only one baud clock source (s3c24xx_serial_drv_data.num_clks == 1). This will work both on Exynos 5260 and 5433. > Signed-off-by: Stuart Menefy <stuart.menefy@mathembedded.com> > --- > drivers/tty/serial/samsung.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 42 insertions(+) > > diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c > index 9fc3559f80d9..83fd51607741 100644 > --- a/drivers/tty/serial/samsung.c > +++ b/drivers/tty/serial/samsung.c > @@ -1694,6 +1694,42 @@ s3c24xx_serial_cpufreq_deregister(struct s3c24xx_uart_port *port) > } > #endif > > +static int s3c24xx_serial_enable_baudclk(struct s3c24xx_uart_port *ourport) > +{ > + struct device *dev = ourport->port.dev; > + struct s3c24xx_uart_info *info = ourport->info; > + char clk_name[MAX_CLK_NAME_LENGTH]; > + unsigned int clk_sel; > + struct clk *clk; > + int clk_num; > + int ret; > + > + clk_sel = ourport->cfg->clk_sel ? : info->def_clk_sel; > + for (clk_num = 0; clk_num < info->num_clks; clk_num++) { > + if (!(clk_sel & (1 << clk_num))) > + continue; > + > + sprintf(clk_name, "clk_uart_baud%d", clk_num); > + clk = clk_get(dev, clk_name); > + if (IS_ERR(clk)) > + continue; > + > + ret = clk_prepare_enable(clk); > + if (ret) { > + clk_put(clk); > + continue; > + } > + > + ourport->baudclk = clk; > + ourport->baudclk_rate = clk_get_rate(clk); > + s3c24xx_serial_setsource(&ourport->port, clk_num); > + > + return 0; > + } > + > + return -EINVAL; > +} > + > /* s3c24xx_serial_init_port > * > * initialise a single serial port from the platform device given > @@ -1788,6 +1824,10 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport, > goto err; > } > > + ret = s3c24xx_serial_enable_baudclk(ourport); > + if (ret) > + pr_warn("uart: failed to enable baudclk\n"); > + > /* Keep all interrupts masked and cleared */ > if (s3c24xx_serial_has_interrupt_mask(port)) { > wr_regl(port, S3C64XX_UINTM, 0xf); > @@ -1901,6 +1941,8 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) > * and keeps the clock enabled in this case. > */ > clk_disable_unprepare(ourport->clk); > + if (!IS_ERR(ourport->baudclk)) > + clk_disable_unprepare(ourport->baudclk); > > ret = s3c24xx_serial_cpufreq_register(ourport); > if (ret < 0) Best regards
diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c index 9fc3559f80d9..83fd51607741 100644 --- a/drivers/tty/serial/samsung.c +++ b/drivers/tty/serial/samsung.c @@ -1694,6 +1694,42 @@ s3c24xx_serial_cpufreq_deregister(struct s3c24xx_uart_port *port) } #endif +static int s3c24xx_serial_enable_baudclk(struct s3c24xx_uart_port *ourport) +{ + struct device *dev = ourport->port.dev; + struct s3c24xx_uart_info *info = ourport->info; + char clk_name[MAX_CLK_NAME_LENGTH]; + unsigned int clk_sel; + struct clk *clk; + int clk_num; + int ret; + + clk_sel = ourport->cfg->clk_sel ? : info->def_clk_sel; + for (clk_num = 0; clk_num < info->num_clks; clk_num++) { + if (!(clk_sel & (1 << clk_num))) + continue; + + sprintf(clk_name, "clk_uart_baud%d", clk_num); + clk = clk_get(dev, clk_name); + if (IS_ERR(clk)) + continue; + + ret = clk_prepare_enable(clk); + if (ret) { + clk_put(clk); + continue; + } + + ourport->baudclk = clk; + ourport->baudclk_rate = clk_get_rate(clk); + s3c24xx_serial_setsource(&ourport->port, clk_num); + + return 0; + } + + return -EINVAL; +} + /* s3c24xx_serial_init_port * * initialise a single serial port from the platform device given @@ -1788,6 +1824,10 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport, goto err; } + ret = s3c24xx_serial_enable_baudclk(ourport); + if (ret) + pr_warn("uart: failed to enable baudclk\n"); + /* Keep all interrupts masked and cleared */ if (s3c24xx_serial_has_interrupt_mask(port)) { wr_regl(port, S3C64XX_UINTM, 0xf); @@ -1901,6 +1941,8 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) * and keeps the clock enabled in this case. */ clk_disable_unprepare(ourport->clk); + if (!IS_ERR(ourport->baudclk)) + clk_disable_unprepare(ourport->baudclk); ret = s3c24xx_serial_cpufreq_register(ourport); if (ret < 0)
The Exynos 5260, like the 5433, appears to require baud clock as well as pclk to be running before accessing any of the registers, otherwise an external abort is raised. The serial driver already enables baud clock when required, but only if it knows which clock is baud clock. On older SoCs baud clock may be selected from a number of possible clocks so to support this the driver only selects which clock to use for baud clock when a port is opened, at which point the desired baud rate is known and the best clock can be selected. The result is that there are a number of circumstances in which registers are accessed without first explicitly enabling baud clock: - while the driver is being initialised - the initial parts of opening a port for the first time - when resuming if the port hasn't been already opened The 5433 overcomes this currently by marking the baud clock as CLK_IGNORE_UNUSED, so the clock is always enabled, however for the 5260 I've been trying to avoid this. This change adds code to pick the first available clock to use as baud clock and enables it while initialising the driver. This code wouldn't be sufficient on a SoC which supports multiple possible baud clock sources _and_ requires the correct baud clock to be enabled before accessing any of the serial port registers (in particular the register which selects which clock to use as the baud clock). As far as I know such hardware doesn't exist. Signed-off-by: Stuart Menefy <stuart.menefy@mathembedded.com> --- drivers/tty/serial/samsung.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)