[U-Boot] [PATCH v3 16/25] riscv: Update supports_extension() to use desc from cpu driver

[Auer, Lukas]

Hi Bin,

On Wed, 2018-12-12 at 15:02 +0800, Bin Meng wrote:
> Hi Lukas,
> 
> On Wed, Dec 12, 2018 at 7:40 AM Auer, Lukas
> <lukas.auer at aisec.fraunhofer.de> wrote:
> > 
> > Hi Bin,
> > 
> > On Tue, 2018-12-11 at 01:34 -0800, Bin Meng wrote:
> > > This updates supports_extension() implementation to use the desc
> > > string from the cpu driver whenever possible, which avoids the
> > > reading of misa CSR for S-mode U-Boot.
> > > 
> > > Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> > > 
> > > ---
> > > 
> > > Changes in v3:
> > > - new patch to update supports_extension() to use desc from cpu
> > > driver
> > > 
> > > Changes in v2: None
> > > 
> > >  arch/riscv/cpu/cpu.c | 16 ++++++++++++++++
> > >  1 file changed, 16 insertions(+)
> > > 
> > > diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
> > > index d3c59da..704ae70 100644
> > > --- a/arch/riscv/cpu/cpu.c
> > > +++ b/arch/riscv/cpu/cpu.c
> > > @@ -5,8 +5,10 @@
> > > 
> > >  #include <common.h>
> > >  #include <cpu.h>
> > > +#include <dm.h>
> > >  #include <log.h>
> > >  #include <asm/csr.h>
> > > +#include <dm/uclass-internal.h>
> > > 
> > >  /*
> > >   * prior_stage_fdt_address must be stored in the data section
> > > since
> > > it is used
> > > @@ -16,7 +18,21 @@ phys_addr_t prior_stage_fdt_address
> > > __attribute__((section(".data")));
> > > 
> > >  static inline bool supports_extension(char ext)
> > >  {
> > > +#ifdef CONFIG_CPU
> > > +     struct udevice *dev;
> > > +     char desc[32];
> > > +
> > > +     uclass_find_first_device(UCLASS_CPU, &dev);
> > > +     if (!cpu_get_desc(dev, desc, sizeof(desc))) {
> > > +             /* skip the first 4 characters (rv32|rv64) */
> > > +             if (strchr(desc + 4, ext))
> > > +                     return true;
> > > +     }
> > > +
> > > +     return false;
> > > +#else
> > >       return csr_read(misa) & (1 << (ext - 'a'));
> > 
> > This should not be used in supervisor mode, so that we don't need a
> > patched BBL. I think it is better to use #elif
> > defined(CONFIG_RISCV_MMODE) here.
> > 
> 
> OK, will do in v4.
> 
> > There should also be a warning message to the user if
> > supports_extension() does not have a way to determine the available
> > extensions (supervisor-mode and no CONFIG_CPU or CONFIG_CPU, but no
> > device found). This would help to prevent difficult-to-debug errors
> > caused by part of the initialization missing.
> > 
> 
> Will add a warning message for S-mode without CPU. I don't think we
> should warn CONFIG_CPU but no device found case as it has already
> been
> covered in riscv_cpu_probe().
> 

You are right, I missed that.

Thanks,
Lukas