| Message ID | 20180512101558.24375-3-ramon.fried@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | *** Qualcomm Snapdraon serial fixes*** | expand |
Hi Ramon, On 12 May 2018 at 20:15, Ramon Fried <ramon.fried@gmail.com> wrote: > Failure to set the clocks will causes data abort exception when > trying to write to AHB uart registers. > This patch ensures that we don't touch these registers if clock > setting failed. > > Signed-off-by: Ramon Fried <ramon.fried@gmail.com> > --- > drivers/serial/serial_msm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c > index 119e6b9846..250e48c996 100644 > --- a/drivers/serial/serial_msm.c > +++ b/drivers/serial/serial_msm.c > @@ -183,8 +183,8 @@ static int msm_serial_probe(struct udevice *dev) > { > struct msm_serial_data *priv = dev_get_priv(dev); > > - msm_uart_clk_init(dev); /* Ignore return value and hope clock was > - properly initialized by earlier loaders */ > + if (msm_uart_clk_init(dev)) > + return -EINVAL; Would it not be better to return the error that msm_uart_clk_init() returns, rather than -EINVAL? > > if (readl(priv->base + UARTDM_SR) & UARTDM_SR_UART_OVERRUN) > writel(UARTDM_CR_CMD_RESET_ERR, priv->base + UARTDM_CR); > -- > 2.14.1 > Regards, Simon
On Mon, May 14, 2018 at 1:00 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Ramon, > > On 12 May 2018 at 20:15, Ramon Fried <ramon.fried@gmail.com> wrote: >> Failure to set the clocks will causes data abort exception when >> trying to write to AHB uart registers. >> This patch ensures that we don't touch these registers if clock >> setting failed. >> >> Signed-off-by: Ramon Fried <ramon.fried@gmail.com> >> --- >> drivers/serial/serial_msm.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c >> index 119e6b9846..250e48c996 100644 >> --- a/drivers/serial/serial_msm.c >> +++ b/drivers/serial/serial_msm.c >> @@ -183,8 +183,8 @@ static int msm_serial_probe(struct udevice *dev) >> { >> struct msm_serial_data *priv = dev_get_priv(dev); >> >> - msm_uart_clk_init(dev); /* Ignore return value and hope clock was >> - properly initialized by earlier loaders */ >> + if (msm_uart_clk_init(dev)) >> + return -EINVAL; > > Would it not be better to return the error that msm_uart_clk_init() > returns, rather than -EINVAL? Yes. you're right. I will fix. >> >> if (readl(priv->base + UARTDM_SR) & UARTDM_SR_UART_OVERRUN) >> writel(UARTDM_CR_CMD_RESET_ERR, priv->base + UARTDM_CR); >> -- >> 2.14.1 >> > > Regards, > Simon
diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c index 119e6b9846..250e48c996 100644 --- a/drivers/serial/serial_msm.c +++ b/drivers/serial/serial_msm.c @@ -183,8 +183,8 @@ static int msm_serial_probe(struct udevice *dev) { struct msm_serial_data *priv = dev_get_priv(dev); - msm_uart_clk_init(dev); /* Ignore return value and hope clock was - properly initialized by earlier loaders */ + if (msm_uart_clk_init(dev)) + return -EINVAL; if (readl(priv->base + UARTDM_SR) & UARTDM_SR_UART_OVERRUN) writel(UARTDM_CR_CMD_RESET_ERR, priv->base + UARTDM_CR);
Failure to set the clocks will causes data abort exception when trying to write to AHB uart registers. This patch ensures that we don't touch these registers if clock setting failed. Signed-off-by: Ramon Fried <ramon.fried@gmail.com> --- drivers/serial/serial_msm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)