RE: [PATCH v9 08/14] hw/nvram: NPCM7xx OTP device model

[address@hidden]

Hi,

Inside


> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Sent: Thursday, December 22, 2022 5:03 PM
> To: Havard Skinnemoen <hskinnemoen@google.com>; peter.maydell@linaro.org
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; IS20 Avi Fishman
> <Avi.Fishman@nuvoton.com>; CS20 KFTing <KFTING@nuvoton.com>; Alexander
> Bulekov <alxndr@bu.edu>; Shengtan Mao <stmao@google.com>; Hao Wu
> <wuhaotsh@google.com>; Chris Rauer <crauer@google.com>; CS20 KFTing
> <KFTING@nuvoton.com>
> Subject: Re: [PATCH v9 08/14] hw/nvram: NPCM7xx OTP device model
>
> Hi,
>
> (old patch)
>
> On 11/9/20 07:20, Havard Skinnemoen wrote:
> > This supports reading and writing OTP fuses and keys. Only fuse reading
> > has been tested. Protection is not implemented.
> >
> > Reviewed-by: Avi Fishman <avi.fishman@nuvoton.com>
> > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > Tested-by: Alexander Bulekov <alxndr@bu.edu>
> > Signed-off-by: Havard Skinnemoen <hskinnemoen@google.com>
> > ---
> >   include/hw/arm/npcm7xx.h       |   3 +
> >   include/hw/nvram/npcm7xx_otp.h |  79 ++++++
> >   hw/arm/npcm7xx.c               |  29 +++
> >   hw/nvram/npcm7xx_otp.c         | 440 +++++++++++++++++++++++++++++++++
> >   hw/nvram/meson.build           |   1 +
> >   5 files changed, 552 insertions(+)
> >   create mode 100644 include/hw/nvram/npcm7xx_otp.h
> >   create mode 100644 hw/nvram/npcm7xx_otp.c
>
> > +/**
> > + * npcm7xx_otp_array_write - ECC encode and write data to OTP array.
> > + * @s: OTP module.
> > + * @data: Data to be encoded and written.
> > + * @offset: Offset of first byte to be written in the OTP array.
> > + * @len: Number of bytes before ECC encoding.
> > + *
> > + * Each nibble of data is encoded into a byte, so the number of bytes 
> > written
> > + * to the array will be @len * 2.
> > + */
> > +extern void npcm7xx_otp_array_write(NPCM7xxOTPState *s, const void
> *data,
> > +                                    unsigned int offset, unsigned int len);
>
> > +static void npcm7xx_init_fuses(NPCM7xxState *s)
> > +{
> > +    NPCM7xxClass *nc = NPCM7XX_GET_CLASS(s);
> > +    uint32_t value;
> > +
> > +    /*
> > +     * The initial mask of disabled modules indicates the chip derivative 
> > (e.g.
> > +     * NPCM750 or NPCM730).
> > +     */
> > +    value = tswap32(nc->disabled_modules);
>
> In which endianness do you want this 32-bit fuse value to be written?

It should be little endian, I am not sure why there is a swap here.
Unless the nc->disabled_modules for some reason is coming swapped so we swap it 
back.

>
> I suppose you used a little-endian host, so we want it big-endian in
> the OTP? In that case it would be better to use cpu_to_be32(), to
> be able to use the OTP on a big-endian host such s390x.

So according to what I said then use cpu_to_le32()

>
> > +    npcm7xx_otp_array_write(&s->fuse_array, &value,
> NPCM7XX_FUSE_DERIVATIVE,
> > +                            sizeof(value));
> > +}
>
> For completeness:
>
>  > +static void npcm730_class_init(ObjectClass *oc, void *data)
>  > +{
>  > +    NPCM7xxClass *nc = NPCM7XX_CLASS(oc);
>  > +
>  > +    /* NPCM730 is optimized for data center use, so no graphics, etc. */
>  > +    nc->disabled_modules = 0x00300395;
>  > +    nc->num_cpus = 2;
>  > +}
>  > +
>  > +static void npcm750_class_init(ObjectClass *oc, void *data)
>  > +{
>  > +    NPCM7xxClass *nc = NPCM7XX_CLASS(oc);
>  > +
>  > +    /* NPCM750 has 2 cores and a full set of peripherals */
>  > +    nc->disabled_modules = 0x00000000;
>  > +    nc->num_cpus = 2;
>  > +}
>
> Thanks,
>
> Phil.
________________________________
________________________________
 The privileged confidential information contained in this email is intended 
for use only by the addressees as indicated by the original sender of this 
email. If you are not the addressee indicated in this email or are not 
responsible for delivery of the email to such a person, please kindly reply to 
the sender indicating this fact and delete all copies of it from your computer 
and network server immediately. Your cooperation is highly appreciated. It is 
advised that any unauthorized use of confidential information of Nuvoton is 
strictly prohibited; and any information in this email irrelevant to the 
official business of Nuvoton shall be deemed as neither given nor endorsed by 
Nuvoton.