[U-Boot] [PATCH] sunxi: support fuse cmd to read/write fuse

[André Przywara]

On 27/01/18 15:20, Jun Nie wrote:
> Support fuse cmd to read/write fuse. Power supply for fuse
> should be ready, name is VDD_EFUSE in some schematic.

Mmh, in general I am not sure it is a good idea to expose this so easily
to the user. I guess a clueless user can easily brick his board by
typing something at the "fuse write" command. I understand that one has
to manually enable the fuse command first to allow access, but this is
still quite a high risk, especially since a lot of the fuses are not
documented.
Would love to hear opinions from others about that topic.

Also I would have hoped for a bit more documentation.
How do those banks/words from the write command map to the fuses, for
instance? What fuses are available and useful? What are the implications
of writing the secure boot fuse, for instance?

And do we know how access to the fuses is affected by the exception
level / mode we are in? My understanding is that SID access is secure
only, which would make it inaccessible for the 64-bit boards where
U-Boot runs in EL2. But then again at least the A64 does not seem to
care about this (unless the secure boot fuse is set).

More inline ...

> Signed-off-by: Jun Nie <jun.nie at linaro.org>
> ---
>  arch/arm/mach-sunxi/cpu_info.c | 60 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 
> diff --git a/arch/arm/mach-sunxi/cpu_info.c b/arch/arm/mach-sunxi/cpu_info.c
> index 25a5ec2..30bc2bf 100644
> --- a/arch/arm/mach-sunxi/cpu_info.c
> +++ b/arch/arm/mach-sunxi/cpu_info.c
> @@ -133,6 +133,30 @@ uint32_t sun8i_efuse_read(uint32_t offset)
>  	reg_val = readl(SUNXI_SIDC_BASE + SIDC_RDKEY);
>  	return reg_val;
>  }
> +
> +uint32_t sun8i_efuse_write(u32 offset, u32 val)
> +{
> +	u32 reg_val;
> +
> +	writel(val, SUNXI_SIDC_BASE + SIDC_RDKEY);
> +
> +	reg_val = readl(SUNXI_SIDC_BASE + SIDC_PRCTL);
> +	reg_val &= ~(((0x1ff) << 16) | 0x3);
> +	reg_val |= (offset << 16);

Would be good to put names to those magic values.
I understand this is from the code as I sent you ;-), but still ...

> +	writel(reg_val, SUNXI_SIDC_BASE + SIDC_PRCTL);
> +
> +	reg_val &= ~(((0xff) << 8) | 0x3);
> +	reg_val |= (SIDC_OP_LOCK << 8) | 0x1;
> +	writel(reg_val, SUNXI_SIDC_BASE + SIDC_PRCTL);
> +
> +	while (readl(SUNXI_SIDC_BASE + SIDC_PRCTL) & 0x1)
> +		;

shall we have a timeout or limited retries here?

> +
> +	reg_val &= ~(((0x1ff) << 16) | ((0xff) << 8) | 0x3);
> +	writel(reg_val, SUNXI_SIDC_BASE + SIDC_PRCTL);
> +
> +	return 0;
> +}
>  #endif
>  
>  int sunxi_get_sid(unsigned int *sid)
> @@ -164,3 +188,39 @@ int sunxi_get_sid(unsigned int *sid)
>  	return -ENODEV;
>  #endif
>  }
> +
> +int fuse_read(u32 bank, u32 word, u32 *sid)
> +{
> +#ifdef CONFIG_MACH_SUN8I_H3
> +	*sid = sun8i_efuse_read(word);
> +#elif defined SUNXI_SID_BASE
> +	*sid = readl((ulong)SUNXI_SID_BASE + word);
> +#else
> +	return -ENODEV;
> +#endif
> +	return 0;
> +}
> +
> +int fuse_prog(u32 bank, u32 word, u32 val)
> +{

I would feel better if we have the write access protected by a separate
Kconfig symbol. So without this being defined either nothing happens or
the user gets a warning.

> +#ifdef CONFIG_MACH_SUN8I_H3

I guess this applies to more than the H3, namely the A64 and A83T,
possibly also H5 and others?

> +	return sun8i_efuse_write(word, val);
> +#elif defined SUNXI_SID_BASE
> +	writel(val, (ulong)SUNXI_SID_BASE + word);

Are you sure that works? If I read [1] correctly, you always have to use
a special algorithm to burn a fuse, a simple MMIO write access would not
suffice.
The algorithm seems to be different for older SoCs (pre-H3).

> +#else
> +	return -ENODEV;
> +#endif
> +	return 0;
> +}
> +
> +int fuse_sense(u32 bank, u32 word, u32 *val)
> +{
> +	/* We do not support sensing :-( */

Isn't sense the only thing we actually implement? At least this is my
understanding from reading doc/README.fuse.

Cheers,
Andre.


> +	return -EINVAL;
> +}
> +
> +int fuse_override(u32 bank, u32 word, u32 val)
> +{
> +	/* We do not support overriding :-( */
> +	return -EINVAL;
> +}
>