[U-Boot] [PATCH v2 09/11] drivers: serial_sifive: Skip baudrate config if no input clock

[Auer, Lukas]

On Mon, 2019-01-21 at 12:39 +0000, Auer, Lukas wrote:
> On Sun, 2019-01-20 at 17:07 -0800, Atish Patra wrote:
> > On 1/20/19 12:22 PM, Auer, Lukas wrote:
> > > Hi Anup,
> > > 
> > > On Fri, 2019-01-18 at 11:19 +0000, Anup Patel wrote:
> > > > From: Atish Patra <atish.patra at wdc.com>
> > > > 
> > > > It is possible that input clock is not available because clk
> > > > device was not available and 'clock-frequency' DT property is
> > > > also not available.
> > > 
> > > Why would the clock device not be available?
> > > I suspect the problem is that the clock driver is not probed pre-
> > > relocation. Adding DM_FLAG_PRE_RELOC to the driver flags would
> > > fix
> > > this.
> > > 
> > 
> > The problem is serial driver gets probed first before clock driver
> > even 
> > if we add DM_FLAG_PRE_RELOC.
> 
> That makes sense. I just browsed through the code to see what other
> boards do here. The serial driver for the MPC83xx SoC series directly
> probes the clock driver (see get_serial_clock in
> driver/clk/mpc83xx_clk.c called from the serial driver). Maybe we
> should do something similar here?
> 

What are your thoughts on this, can you add something like it?

Thanks,
Lukas

> > > > In this case, instead of failing we should just skip baudrate
> > > > config by returning zero.
> > > 
> > > Won't this cause issues if an incorrect baudrate is set?
> > > 
> > It might be possible that baudrate is configured by earlier boot
> > stage.
> > Thus, any early information can be displayed in console by the
> > serial 
> > driver even if it is not configured correctly by u-boot.
> > 
> 
> Yes, it is very likely not going to be a problem, but if we can fix
> it
> it would be great :)
> 
> Thanks,
> Lukas
> 
> > Regards,
> > Atish
> > > Thanks,
> > > Lukas
> > > 
> > > > Signed-off-by: Atish Patra <atish.patra at wdc.com>
> > > > Signed-off-by: Anup Patel <anup.patel at wdc.com>
> > > > Reviewed-by: Alexander Graf <agraf at suse.de>
> > > > ---
> > > >   drivers/serial/serial_sifive.c | 32 ++++++++++++++++---------
> > > > --
> > > > -----
> > > >   1 file changed, 16 insertions(+), 16 deletions(-)
> > > > 
> > > > diff --git a/drivers/serial/serial_sifive.c
> > > > b/drivers/serial/serial_sifive.c
> > > > index ea4d35d48c..537bc7a975 100644
> > > > --- a/drivers/serial/serial_sifive.c
> > > > +++ b/drivers/serial/serial_sifive.c
> > > > @@ -99,27 +99,27 @@ static int _sifive_serial_getc(struct
> > > > uart_sifive
> > > > *regs)
> > > >   
> > > >   static int sifive_serial_setbrg(struct udevice *dev, int
> > > > baudrate)
> > > >   {
> > > > -	int err;
> > > > +	int ret;
> > > >   	struct clk clk;
> > > >   	struct sifive_uart_platdata *platdata =
> > > > dev_get_platdata(dev);
> > > > +	u32 clock = 0;
> > > >   
> > > > -	err = clk_get_by_index(dev, 0, &clk);
> > > > -	if (!err) {
> > > > -		err = clk_get_rate(&clk);
> > > > -		if (!IS_ERR_VALUE(err))
> > > > -			platdata->clock = err;
> > > > -	} else if (err != -ENOENT && err != -ENODEV && err !=
> > > > -ENOSYS)
> > > > {
> > > > +	ret = clk_get_by_index(dev, 0, &clk);
> > > > +	if (IS_ERR_VALUE(ret)) {
> > > >   		debug("SiFive UART failed to get clock\n");
> > > > -		return err;
> > > > -	}
> > > > -
> > > > -	if (!platdata->clock)
> > > > -		platdata->clock = dev_read_u32_default(dev,
> > > > "clock-
> > > > frequency", 0);
> > > > -	if (!platdata->clock) {
> > > > -		debug("SiFive UART clock not defined\n");
> > > > -		return -EINVAL;
> > > > +		ret = dev_read_u32(dev, "clock-frequency",
> > > > &clock);
> > > > +		if (IS_ERR_VALUE(ret)) {
> > > > +			debug("SiFive UART clock not
> > > > defined\n");
> > > > +			return 0;
> > > > +		}
> > > > +	} else {
> > > > +		clock = clk_get_rate(&clk);
> > > > +		if (IS_ERR_VALUE(clock)) {
> > > > +			debug("SiFive UART clock get rate
> > > > failed\n");
> > > > +			return 0;
> > > > +		}
> > > >   	}
> > > > -
> > > > +	platdata->clock = clock;
> > > >   	_sifive_serial_setbrg(platdata->regs, platdata->clock,
> > > > baudrate);
> > > >   
> > > >   	return 0;
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot