On Thu, 17 Feb 2022 17:50:31 +0100
Pali Rohár <p...@kernel.org> wrote:
> On Thursday 17 February 2022 15:31:10 Marek Behún wrote:
> > On Thu, 17 Feb 2022 10:26:19 +0100
> > Pali Rohár <p...@kernel.org> wrote:
> >
> > > Only secure CM3 core can access Security OTP. It is not possible via A53
> >
> > It is not possible for the A53 core (on which U-Boot is running) to read
> > it directly.
> >
> > > core on which is running U-Boot. Marvell for this purpose defined mbox
> > > API
> >
> > For this purpose Marvell defined...
> >
> > > for sending OTP commands between CM and A53 cores.
> > ^CM3
> >
> > > Implement this Marvell mbox API via U-Boot fuse API.
> >
> > Implement these Marvell fuse reading mbox commands via ....
> >
> > > Banks 0-43 are used for accessing Security OTP (44 rows with 67 bits via
> > > 44
> > > banks and words 0-2).
> >
> > Note that of the 67 bits, the 3 upper bits are: 1 lock bit and 2
> > auxiliary bits (meant for testing during the manufacture of the SOC, as
> > I understand it).
> >
> > Also note that the lock bit and the auxiliary bits are not readable
> > via Marvell commands.
> >
> > With CZ.NIC's commands the lock bit is readable.
> >
> > > Write support is not implemented yet.
> > >
> > > Signed-off-by: Pali Rohár <p...@kernel.org>
> > > ---
> > > arch/arm/mach-mvebu/armada3700/efuse.c | 40 ++++++++++++++++++++++++--
> > > 1 file changed, 38 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/arm/mach-mvebu/armada3700/efuse.c
> > > b/arch/arm/mach-mvebu/armada3700/efuse.c
> > > index 03778f17ea49..274d9c72c073 100644
> > > --- a/arch/arm/mach-mvebu/armada3700/efuse.c
> > > +++ b/arch/arm/mach-mvebu/armada3700/efuse.c
> > > @@ -8,6 +8,7 @@
> > > #include <common.h>
> > > #include <asm/io.h>
> > > #include <linux/delay.h>
> > > +#include <mach/mbox.h>
> > > #include <mach/soc.h>
> > >
> > > #define OTP_NB_REG_BASE ((void __iomem
> > > *)MVEBU_REGISTER(0x12600))
> > > @@ -77,6 +78,42 @@ static void otp_read_parallel(void __iomem *base, u32
> > > *data, u32 count)
> > > }
> > > }
> > >
> > > +static int rwtm_otp_read(u8 row, u32 word, u32 *data)
> > > +{
> > > + u32 out[3];
> > > + u32 in[2];
> > > + int res;
> > > +
> > > + /*
> > > + * MBOX_CMD_OTP_READ_32B command is supported by Marvell fuse.bin
> > > + * firmware and also by new (yet unreleased) CZ.NIC wtmi firmware.
> >
> > Marvell's, CZ.NIC's, and drop the "(yet unreleased)", because you'll
> > need to send another patch that drops it afterwards.
> >
> > > + * But this command does not provide access to lock bit.
> > > + */
> > > + if (word < 2) {
> > > + in[0] = row;
> > > + in[1] = word * 32;
> > > + res = mbox_do_cmd(MBOX_CMD_OTP_READ_32B, in, 2, out, 2);
> > > + if (res != -ENOSYS) {
> > > + if (!res)
> > > + *data = out[0];
> > > + return res;
> > > + }
> > > + /* Fallback for old version of CZ.NIC wtmi firmware. */
> > > + }
> >
> > I am afraid this is not correct, because Marvell's firmware reads the
> > efuse without Error Correction. So it is possible for Marvell's command
> > to return different value than CZ.NIC's command.
> >
> > You need to determine whether CZ.NIC's command is supported, and use it
> > if it is, otherwise use Marvell's command. Or you need to define
> > whether and when the Error Correction is supposed to be used, or
> > something.
>
> Seems that this U-Boot fuse API is low level API, so it probably would
> be better to always read without ECC correction (which is provided by
> Marvell OTP API). As ECC is stored in other bits, it is possible to read
> everything needed for ECC correction via this API.
>
> This could simplify patch: Lock bit read via CZ.NIC API (as there is no
> other API) and other bits read via Marvell API (which is going to be
> supported also by CZ.NIC firmware).
Ok, as long as turris_mox.c reads OTP with Error Correction, fuse can
be kept low level.
Marek
> > But doing what you are doing here can make Turris MOX boards read
> > different values. I know of at least one board where serial number or
> > MAC address needs Error Correction.
> >
> > Marek
