mbox series

[RFC,0/1] Timeout error with Microchip OTPC driver on SAM9X60

Message ID 20240412140802.1571935-1-ada@thorsis.com (mailing list archive)
Headers show
Series Timeout error with Microchip OTPC driver on SAM9X60 | expand

Message

Alexander Dahl April 12, 2024, 2:08 p.m. UTC
Hei hei,

on a custom sam9x60 based board we want to access a unique ID of the
SoC.  Microchip sam-ba has a command 'readuniqueid' which returns the
content of the OTPC Product UID x Register in that case.

(On a different board with a SAMA5D2 we use the Serial Number x Register
exposed through the atmel soc driver, which is not present in the
SAM9X60 series.)

There is a driver for the OTPC of the SAMA7G5 and after comparing
register layouts it seems that one is almost identical to the one used
by SAM9X60.  So I thought just adapting the driver for SAM9X60 should be
easy.  (At least as a start, the driver has no support for that UID
register, but I suppose it would be the right place to implement it.)

However it does not work.  I used the patch attached with
additional debug messages on a SAM9X60-Curiosity board.  (That patch is
not meant for inclusion, just for showing what I've tried.)

On probe the function mchp_otpc_init_packets_list() returns with
ETIMEDOUT, which it can only do if mchp_otpc_prepare_read() returns with
timeout and that can only happen if read_poll_timeout() times out on
reading the Status Register.  Poking that register with `devmem
0xeff0000c 32` gives 0x00000040 which means "A packet read is on-going".

Kinda stuck here.  Any ideas?

Greets and have a nice weekend everyone
Alex

Alexander Dahl (1):
  nvmem: microchip-otpc: Add support for SAM9X60

 .../dts/microchip/at91-sam9x60_curiosity.dts     |  4 ++++
 arch/arm/boot/dts/microchip/sam9x60.dtsi         |  7 +++++++
 drivers/nvmem/microchip-otpc.c                   | 16 +++++++++++++---
 3 files changed, 24 insertions(+), 3 deletions(-)


base-commit: fec50db7033ea478773b159e0e2efb135270e3b7

Comments

Claudiu Beznea April 24, 2024, 7:32 a.m. UTC | #1
Hi, Alexander,

On 12.04.2024 17:08, Alexander Dahl wrote:
> Hei hei,
> 
> on a custom sam9x60 based board we want to access a unique ID of the
> SoC.  Microchip sam-ba has a command 'readuniqueid' which returns the
> content of the OTPC Product UID x Register in that case.
> 
> (On a different board with a SAMA5D2 we use the Serial Number x Register
> exposed through the atmel soc driver, which is not present in the
> SAM9X60 series.)
> 
> There is a driver for the OTPC of the SAMA7G5 and after comparing
> register layouts it seems that one is almost identical to the one used
> by SAM9X60.  So I thought just adapting the driver for SAM9X60 should be
> easy.  (At least as a start, the driver has no support for that UID
> register, but I suppose it would be the right place to implement it.)
> 
> However it does not work.  I used the patch attached with
> additional debug messages on a SAM9X60-Curiosity board.  (That patch is
> not meant for inclusion, just for showing what I've tried.)
> 
> On probe the function mchp_otpc_init_packets_list() returns with
> ETIMEDOUT, which it can only do if mchp_otpc_prepare_read() returns with
> timeout and that can only happen if read_poll_timeout() times out on
> reading the Status Register.  Poking that register with `devmem
> 0xeff0000c 32` gives 0x00000040 which means "A packet read is on-going".


Would it be possible that the OTP memory is not properly initialized and
the algorithm to initialized the packet list to confuse the hardware?

I see in the datasheet the following: "The initial value of the OTP memory
is ‘0’ but the memory may contain some “defective” bits already set to the
value ‘1’."

Otherwise, from the top of my mind I don't have any idea on what might happen.

Thank you,
Claudiu Beznea

> 
> Kinda stuck here.  Any ideas?
> 
> Greets and have a nice weekend everyone
> Alex
> 
> Alexander Dahl (1):
>   nvmem: microchip-otpc: Add support for SAM9X60
> 
>  .../dts/microchip/at91-sam9x60_curiosity.dts     |  4 ++++
>  arch/arm/boot/dts/microchip/sam9x60.dtsi         |  7 +++++++
>  drivers/nvmem/microchip-otpc.c                   | 16 +++++++++++++---
>  3 files changed, 24 insertions(+), 3 deletions(-)
> 
> 
> base-commit: fec50db7033ea478773b159e0e2efb135270e3b7
Alexander Dahl July 1, 2024, 7:40 a.m. UTC | #2
Hei hei,

Am Wed, Apr 24, 2024 at 10:32:02AM +0300 schrieb claudiu beznea:
> Hi, Alexander,
> 
> On 12.04.2024 17:08, Alexander Dahl wrote:
> > Hei hei,
> > 
> > on a custom sam9x60 based board we want to access a unique ID of the
> > SoC.  Microchip sam-ba has a command 'readuniqueid' which returns the
> > content of the OTPC Product UID x Register in that case.
> > 
> > (On a different board with a SAMA5D2 we use the Serial Number x Register
> > exposed through the atmel soc driver, which is not present in the
> > SAM9X60 series.)
> > 
> > There is a driver for the OTPC of the SAMA7G5 and after comparing
> > register layouts it seems that one is almost identical to the one used
> > by SAM9X60.  So I thought just adapting the driver for SAM9X60 should be
> > easy.  (At least as a start, the driver has no support for that UID
> > register, but I suppose it would be the right place to implement it.)
> > 
> > However it does not work.  I used the patch attached with
> > additional debug messages on a SAM9X60-Curiosity board.  (That patch is
> > not meant for inclusion, just for showing what I've tried.)
> > 
> > On probe the function mchp_otpc_init_packets_list() returns with
> > ETIMEDOUT, which it can only do if mchp_otpc_prepare_read() returns with
> > timeout and that can only happen if read_poll_timeout() times out on
> > reading the Status Register.  Poking that register with `devmem
> > 0xeff0000c 32` gives 0x00000040 which means "A packet read is on-going".
> 
> 
> Would it be possible that the OTP memory is not properly initialized and
> the algorithm to initialized the packet list to confuse the hardware?
> 
> I see in the datasheet the following: "The initial value of the OTP memory
> is ‘0’ but the memory may contain some “defective” bits already set to the
> value ‘1’."

Meanwhile we looked deeper into this driver.  I just drop here what we
found so far, because it's vacation time, and this will probably not
be picked up again this month because vacation etc.  I wanna get it
out of my head for now. ;-)

Note: most of this should also apply to sama7g5, from comparing data
sheets the OTPC is the same.

1. the call to read_poll_timeout() most probably has its sleep and
timeout paramaters swapped.

2. the read access to OTPC_CR register is not necessary, the datasheet
says the register is "write only".  It may even trigger an interrupt
for reading a write only register.  Furthermore the WPCTEN bit in the
OTPC Write Protection Mode register (OTPC_WPMR) is not checked before
accessing OTPC_CR as suggested by the datasheet.

3. the errata sheet for the SAM9X60 SoC (DS80000846) has a section on
the OTPC which recommends a fixup before the first write operation,
with code example:
https://ww1.microchip.com/downloads/aemDocuments/documents/MPU32/ProductDocuments/Errata/SAM9X60-Device-Silicon-Errata-and-Data-Sheet-Clarif-DS80000846.pdf
The same thing is implemented in 'atmel-software-package' which has
(at least had in the past) the code for the binary applets used in the
'sam-ba' tool:
https://github.com/atmelcorp/atmel-software-package/blame/master/drivers/nvm/otp/otpc.c#L86
I bet this fixup should be implemented in this driver here as well?

So far for now.

Greets
Alex

> 
> Otherwise, from the top of my mind I don't have any idea on what might happen.
> 
> Thank you,
> Claudiu Beznea
> 
> > 
> > Kinda stuck here.  Any ideas?
> > 
> > Greets and have a nice weekend everyone
> > Alex
> > 
> > Alexander Dahl (1):
> >   nvmem: microchip-otpc: Add support for SAM9X60
> > 
> >  .../dts/microchip/at91-sam9x60_curiosity.dts     |  4 ++++
> >  arch/arm/boot/dts/microchip/sam9x60.dtsi         |  7 +++++++
> >  drivers/nvmem/microchip-otpc.c                   | 16 +++++++++++++---
> >  3 files changed, 24 insertions(+), 3 deletions(-)
> > 
> > 
> > base-commit: fec50db7033ea478773b159e0e2efb135270e3b7
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Alexander Dahl Aug. 13, 2024, 8:55 a.m. UTC | #3
Hello Claudiu,

Am Wed, Apr 24, 2024 at 10:32:02AM +0300 schrieb claudiu beznea:
> Hi, Alexander,
> 
> On 12.04.2024 17:08, Alexander Dahl wrote:
> > Hei hei,
> > 
> > on a custom sam9x60 based board we want to access a unique ID of the
> > SoC.  Microchip sam-ba has a command 'readuniqueid' which returns the
> > content of the OTPC Product UID x Register in that case.
> > 
> > (On a different board with a SAMA5D2 we use the Serial Number x Register
> > exposed through the atmel soc driver, which is not present in the
> > SAM9X60 series.)
> > 
> > There is a driver for the OTPC of the SAMA7G5 and after comparing
> > register layouts it seems that one is almost identical to the one used
> > by SAM9X60.  So I thought just adapting the driver for SAM9X60 should be
> > easy.  (At least as a start, the driver has no support for that UID
> > register, but I suppose it would be the right place to implement it.)
> > 
> > However it does not work.  I used the patch attached with
> > additional debug messages on a SAM9X60-Curiosity board.  (That patch is
> > not meant for inclusion, just for showing what I've tried.)
> > 
> > On probe the function mchp_otpc_init_packets_list() returns with
> > ETIMEDOUT, which it can only do if mchp_otpc_prepare_read() returns with
> > timeout and that can only happen if read_poll_timeout() times out on
> > reading the Status Register.  Poking that register with `devmem
> > 0xeff0000c 32` gives 0x00000040 which means "A packet read is on-going".
> 
> 
> Would it be possible that the OTP memory is not properly initialized and
> the algorithm to initialized the packet list to confuse the hardware?
> 
> I see in the datasheet the following: "The initial value of the OTP memory
> is ‘0’ but the memory may contain some “defective” bits already set to the
> value ‘1’."

I think this might be possible?  SAM-BA also stumbles here, but the
SoC is like shipped by the vendor, no OTPC writes ever from my side.
When calling this …

    $ sam-ba -p serial -d sam9x60:0:1 -t 5 -a bootconfig -c readcfg:bcp-otp

… I get this on serial debug output:

    Applet 'BootConfig' from SAM-BA Applets Framework 3.8 (v3.8).
    -E- Cannot read Boot Config Packet.
    -E- Invalid parameter for read config: index 0

Question is: how should the driver behave in this case?  Fail like it
does now?  Or load in some kind of safe state with "empty" nvmem?
This is especially interesting with regard to a new question below.

> Otherwise, from the top of my mind I don't have any idea on what might happen.

I have some debug code here, and digging deeper into this currently to
see what's really happening.  While at it, a new question came up:

There's this OTP memory which the driver tries to expose as NVMEM.
However what I really want to do is getting access to the OTPC Product
UID x Registers, which are not OTP memory but plain registers inside
of the address space of the OTPC here.  Should this be exposed as a
second nvmem device then, or handled by a different driver?  How would
accessing the same register space from different drivers be handled
then?

Greets
Alex

> 
> Thank you,
> Claudiu Beznea
> 
> > 
> > Kinda stuck here.  Any ideas?
> > 
> > Greets and have a nice weekend everyone
> > Alex
> > 
> > Alexander Dahl (1):
> >   nvmem: microchip-otpc: Add support for SAM9X60
> > 
> >  .../dts/microchip/at91-sam9x60_curiosity.dts     |  4 ++++
> >  arch/arm/boot/dts/microchip/sam9x60.dtsi         |  7 +++++++
> >  drivers/nvmem/microchip-otpc.c                   | 16 +++++++++++++---
> >  3 files changed, 24 insertions(+), 3 deletions(-)
> > 
> > 
> > base-commit: fec50db7033ea478773b159e0e2efb135270e3b7
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Claudiu Beznea Aug. 20, 2024, 11:59 a.m. UTC | #4
Hi, Alexander,

On 13.08.2024 11:55, Alexander Dahl wrote:
> Hello Claudiu,
> 
> Am Wed, Apr 24, 2024 at 10:32:02AM +0300 schrieb claudiu beznea:
>> Hi, Alexander,
>>
>> On 12.04.2024 17:08, Alexander Dahl wrote:
>>> Hei hei,
>>>
>>> on a custom sam9x60 based board we want to access a unique ID of the
>>> SoC.  Microchip sam-ba has a command 'readuniqueid' which returns the
>>> content of the OTPC Product UID x Register in that case.
>>>
>>> (On a different board with a SAMA5D2 we use the Serial Number x Register
>>> exposed through the atmel soc driver, which is not present in the
>>> SAM9X60 series.)
>>>
>>> There is a driver for the OTPC of the SAMA7G5 and after comparing
>>> register layouts it seems that one is almost identical to the one used
>>> by SAM9X60.  So I thought just adapting the driver for SAM9X60 should be
>>> easy.  (At least as a start, the driver has no support for that UID
>>> register, but I suppose it would be the right place to implement it.)
>>>
>>> However it does not work.  I used the patch attached with
>>> additional debug messages on a SAM9X60-Curiosity board.  (That patch is
>>> not meant for inclusion, just for showing what I've tried.)
>>>
>>> On probe the function mchp_otpc_init_packets_list() returns with
>>> ETIMEDOUT, which it can only do if mchp_otpc_prepare_read() returns with
>>> timeout and that can only happen if read_poll_timeout() times out on
>>> reading the Status Register.  Poking that register with `devmem
>>> 0xeff0000c 32` gives 0x00000040 which means "A packet read is on-going".
>>
>>
>> Would it be possible that the OTP memory is not properly initialized and
>> the algorithm to initialized the packet list to confuse the hardware?
>>
>> I see in the datasheet the following: "The initial value of the OTP memory
>> is ‘0’ but the memory may contain some “defective” bits already set to the
>> value ‘1’."
> 
> I think this might be possible?  SAM-BA also stumbles here, but the
> SoC is like shipped by the vendor, no OTPC writes ever from my side.

I'm not sure how the SAM9X60 OTP memory is shipped by vendor. If I remember
correctly the user must flash the bootconfig packet depending on his needs.

> When calling this …
> 
>     $ sam-ba -p serial -d sam9x60:0:1 -t 5 -a bootconfig -c readcfg:bcp-otp
> 
> … I get this on serial debug output:
> 
>     Applet 'BootConfig' from SAM-BA Applets Framework 3.8 (v3.8).
>     -E- Cannot read Boot Config Packet.
>     -E- Invalid parameter for read config: index 0
> 
> Question is: how should the driver behave in this case?  Fail like it
> does now?  Or load in some kind of safe state with "empty" nvmem?

The driver cannot know if the memory is defective or not. Not that I've
know. It expects the memory to be in a proper state.

E.g. On SAMA7G5 the OTP memory has at lest the temperature sensor
calibration data packet. And if I remember correctly, it also has the
bootconfig packet on the first position.

Thank you,
Claudiu Beznea

> This is especially interesting with regard to a new question below.
> 
>> Otherwise, from the top of my mind I don't have any idea on what might happen.
> 
> I have some debug code here, and digging deeper into this currently to
> see what's really happening.  While at it, a new question came up:
> 
> There's this OTP memory which the driver tries to expose as NVMEM.
> However what I really want to do is getting access to the OTPC Product
> UID x Registers, which are not OTP memory but plain registers inside
> of the address space of the OTPC here.  Should this be exposed as a
> second nvmem device then, or handled by a different driver?  How would
> accessing the same register space from different drivers be handled
> then?
> 
> Greets
> Alex
> 
>>
>> Thank you,
>> Claudiu Beznea
>>
>>>
>>> Kinda stuck here.  Any ideas?
>>>
>>> Greets and have a nice weekend everyone
>>> Alex
>>>
>>> Alexander Dahl (1):
>>>   nvmem: microchip-otpc: Add support for SAM9X60
>>>
>>>  .../dts/microchip/at91-sam9x60_curiosity.dts     |  4 ++++
>>>  arch/arm/boot/dts/microchip/sam9x60.dtsi         |  7 +++++++
>>>  drivers/nvmem/microchip-otpc.c                   | 16 +++++++++++++---
>>>  3 files changed, 24 insertions(+), 3 deletions(-)
>>>
>>>
>>> base-commit: fec50db7033ea478773b159e0e2efb135270e3b7
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Alexander Dahl Aug. 20, 2024, 1:48 p.m. UTC | #5
Hello Claudiu,

Am Tue, Aug 20, 2024 at 02:59:03PM +0300 schrieb claudiu beznea:
> Hi, Alexander,
> 
> On 13.08.2024 11:55, Alexander Dahl wrote:
> > Hello Claudiu,
> > 
> > Am Wed, Apr 24, 2024 at 10:32:02AM +0300 schrieb claudiu beznea:
> >> Hi, Alexander,
> >>
> >> On 12.04.2024 17:08, Alexander Dahl wrote:
> >>> Hei hei,
> >>>
> >>> on a custom sam9x60 based board we want to access a unique ID of the
> >>> SoC.  Microchip sam-ba has a command 'readuniqueid' which returns the
> >>> content of the OTPC Product UID x Register in that case.
> >>>
> >>> (On a different board with a SAMA5D2 we use the Serial Number x Register
> >>> exposed through the atmel soc driver, which is not present in the
> >>> SAM9X60 series.)
> >>>
> >>> There is a driver for the OTPC of the SAMA7G5 and after comparing
> >>> register layouts it seems that one is almost identical to the one used
> >>> by SAM9X60.  So I thought just adapting the driver for SAM9X60 should be
> >>> easy.  (At least as a start, the driver has no support for that UID
> >>> register, but I suppose it would be the right place to implement it.)
> >>>
> >>> However it does not work.  I used the patch attached with
> >>> additional debug messages on a SAM9X60-Curiosity board.  (That patch is
> >>> not meant for inclusion, just for showing what I've tried.)
> >>>
> >>> On probe the function mchp_otpc_init_packets_list() returns with
> >>> ETIMEDOUT, which it can only do if mchp_otpc_prepare_read() returns with
> >>> timeout and that can only happen if read_poll_timeout() times out on
> >>> reading the Status Register.  Poking that register with `devmem
> >>> 0xeff0000c 32` gives 0x00000040 which means "A packet read is on-going".
> >>
> >>
> >> Would it be possible that the OTP memory is not properly initialized and
> >> the algorithm to initialized the packet list to confuse the hardware?
> >>
> >> I see in the datasheet the following: "The initial value of the OTP memory
> >> is ‘0’ but the memory may contain some “defective” bits already set to the
> >> value ‘1’."
> > 
> > I think this might be possible?  SAM-BA also stumbles here, but the
> > SoC is like shipped by the vendor, no OTPC writes ever from my side.
> 
> I'm not sure how the SAM9X60 OTP memory is shipped by vendor. If I remember
> correctly the user must flash the bootconfig packet depending on his needs.

User can do that, but does not need to if the defaults are sufficient.
On sam9x60-curiosity it just boots with defaults from SD card or raw
NAND flash.  No bootconfig packets are set here.

> > When calling this …
> > 
> >     $ sam-ba -p serial -d sam9x60:0:1 -t 5 -a bootconfig -c readcfg:bcp-otp
> > 
> > … I get this on serial debug output:
> > 
> >     Applet 'BootConfig' from SAM-BA Applets Framework 3.8 (v3.8).
> >     -E- Cannot read Boot Config Packet.
> >     -E- Invalid parameter for read config: index 0
> > 
> > Question is: how should the driver behave in this case?  Fail like it
> > does now?  Or load in some kind of safe state with "empty" nvmem?
> 
> The driver cannot know if the memory is defective or not. Not that I've
> know. It expects the memory to be in a proper state.

Datasheet says:

    After each reset, the OTPC parses the User area to check its
    content. The header of each packet will be read, depending on the
    header value some actions can be triggered:
    • If the header is corrupted, the init sequence is interrupted and
    the OTPC_ISR.COERR bit is set.

The COERR bit in OTPC_ISR can be checked in the driver of course.  Not
sure if that adds any real value however.  Maybe issue a warning in
that case?

> E.g. On SAMA7G5 the OTP memory has at lest the temperature sensor
> calibration data packet. And if I remember correctly, it also has the
> bootconfig packet on the first position.

Nice to know, however I don't have a board featuring the sama7g5 at hand.

Greets
Alex

> 
> Thank you,
> Claudiu Beznea
> 
> > This is especially interesting with regard to a new question below.
> > 
> >> Otherwise, from the top of my mind I don't have any idea on what might happen.
> > 
> > I have some debug code here, and digging deeper into this currently to
> > see what's really happening.  While at it, a new question came up:
> > 
> > There's this OTP memory which the driver tries to expose as NVMEM.
> > However what I really want to do is getting access to the OTPC Product
> > UID x Registers, which are not OTP memory but plain registers inside
> > of the address space of the OTPC here.  Should this be exposed as a
> > second nvmem device then, or handled by a different driver?  How would
> > accessing the same register space from different drivers be handled
> > then?
> > 
> > Greets
> > Alex
> > 
> >>
> >> Thank you,
> >> Claudiu Beznea
> >>
> >>>
> >>> Kinda stuck here.  Any ideas?
> >>>
> >>> Greets and have a nice weekend everyone
> >>> Alex
> >>>
> >>> Alexander Dahl (1):
> >>>   nvmem: microchip-otpc: Add support for SAM9X60
> >>>
> >>>  .../dts/microchip/at91-sam9x60_curiosity.dts     |  4 ++++
> >>>  arch/arm/boot/dts/microchip/sam9x60.dtsi         |  7 +++++++
> >>>  drivers/nvmem/microchip-otpc.c                   | 16 +++++++++++++---
> >>>  3 files changed, 24 insertions(+), 3 deletions(-)
> >>>
> >>>
> >>> base-commit: fec50db7033ea478773b159e0e2efb135270e3b7
> >>
> >> _______________________________________________
> >> linux-arm-kernel mailing list
> >> linux-arm-kernel@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>