Re: [PATCH v3 06/13] nvmem: microchip-otpc: add tag-based packet lookup
From: Claudiu Beznea
Date: Sat Jul 25 2026 - 10:27:18 EST
Hi, Varshini,
On 7/22/26 08:50, Varshini.Rajendran@xxxxxxxxxxxxx wrote:
Hi Claudiu,
Thanks for taking the time to review.
On 22/07/26 12:44 am, Claudiu Beznea wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know
the content is safe
Hi, Varshini,
On 6/30/26 12:35, Varshini Rajendran wrote:
Add support for accessing OTP packets by their 4-byte ASCII tag while
preserving backward compatibility with the existing ID-based lookup.
The OTP memory layout can vary across devices and may change over time,
making the packet ID approach unreliable when the memory map is not
known in advance. The packet tag provides a reliable way to identify
and access packets without prior knowledge of the OTP memory layout.
Two offset encoding are now supported:
1. Legacy ID-based: offset = OTP_PKT(id) = id * 4
Used in DT as: reg = <OTP_PKT(1) 76>;
2. TAG-based: offset = 4-byte ASCII packet tag
Used in DT as: reg = <0x41435354 0x4c>; (tag "ACST")
I think this:
The driver resolves offsets matching valid legacy selectors (multiples
of 4 within the packet count) through ID lookup, falling back to tag
lookup for other values. This ensures existing device trees continue
to work while enabling new tag-based access.
should fall in a different patch?
You mean moving the "invoking the new tag-based method with the legacy
method as a fallback" part alone? Only add the functions in this patch
and invoking them in the next one - Did I get it right?
Ah, apologies for confusion, I referred to the wrong section. I wanted to refer to this section:
The driver also validates OTP memory accessibility and emulation mode
status. When the boot packet is not configured, emulation mode allows
access to the other packets. When both are not available an
informational message is logged.
That is handled through this code:
+
+ tmp = readl_relaxed(otpc->base + MCHP_OTPC_MR);
+ emul_enable = tmp & MCHP_OTPC_MR_EMUL;
+ if (emul_enable)
+ dev_info(otpc->dev, "Emulation mode enabled\n");
+
ret = mchp_otpc_init_packets_list(otpc, &size);
if (ret)
return ret;
+ if (!size) {
+ dev_warn(otpc->dev, "Cannot access OTP memory\n");
+ if (!emul_enable)
+ dev_info(otpc->dev, "Boot packet not programmed and emulation mode disabled\n");
+ }
+
This I think should go in a different patch.
During probe, packet meta data including the tag is read and cached.
The driver also validates OTP memory accessibility and emulation mode
status. When the boot packet is not configured, emulation mode allows
access to the other packets. When both are not available an
informational message is logged.
The stride of the nvmem memory is set to 1 in order to support tag based
offsets, comment in the header file is updated accordingly.
Signed-off-by: Varshini Rajendran <varshini.rajendran@xxxxxxxxxxxxx>
---
drivers/nvmem/microchip-otpc.c | 143 ++++++++++++++++--
.../nvmem/microchip,sama7g5-otpc.h | 4 +-
2 files changed, 136 insertions(+), 11 deletions(-)
diff --git a/drivers/nvmem/microchip-otpc.c b/drivers/nvmem/microchip-
otpc.c
index df979e8549fd..bf8589048e17 100644
--- a/drivers/nvmem/microchip-otpc.c
+++ b/drivers/nvmem/microchip-otpc.c
@@ -18,16 +18,20 @@
#define MCHP_OTPC_CR_READ BIT(6)
#define MCHP_OTPC_MR (0x4)
#define MCHP_OTPC_MR_ADDR GENMASK(31, 16)
+#define MCHP_OTPC_MR_EMUL BIT(7)
#define MCHP_OTPC_AR (0x8)
#define MCHP_OTPC_SR (0xc)
#define MCHP_OTPC_SR_READ BIT(6)
#define MCHP_OTPC_HR (0x20)
#define MCHP_OTPC_HR_SIZE GENMASK(15, 8)
+#define MCHP_OTPC_HR_PACKET_TYPE GENMASK(2, 0)
Nit: in SAMA7D65 manual this is simply packet. Maybe rename it:
MCHP_OTPC_HR_PACKET to match the manual.
#define MCHP_OTPC_DR (0x24)
#define MCHP_OTPC_NAME "mchp-otpc"
#define MCHP_OTPC_SIZE (11 * 1024)
+#define PACKET_TYPE_REGULAR 1
I would move this close to MCHP_OTPC_HR_PACKET_TYPE and name it
something like:
MCHP_OTPC_HR_PACKET_REGULAR to match the datasheet.
+
/**
* struct mchp_otpc - OTPC private data structure
* @base: base address
@@ -47,11 +51,15 @@ struct mchp_otpc {
* @list: list head
* @id: packet ID
* @offset: packet offset (in words) in OTP memory
+ * @type: type of the packet
+ * @tag: 4-byte ASCII tag of the packet
*/
struct mchp_otpc_packet {
struct list_head list;
u32 id;
u32 offset;
+ u32 type;
This can be dropped for now since it's used only in the initialization
path.
+ u32 tag;
};
static struct mchp_otpc_packet *mchp_otpc_id_to_packet(struct
mchp_otpc *otpc,
@@ -70,6 +78,56 @@ static struct mchp_otpc_packet
*mchp_otpc_id_to_packet(struct mchp_otpc *otpc,
return NULL;
}
+/**
+ * mchp_otpc_tag_to_packet() - find packet by tag
+ * @otpc: OTPC private data
+ * @tag: 4-byte ASCII tag to search for
+ *
+ * Return: pointer to packet if found, NULL otherwise
+ */
I'm not sure we need this description since the function is simple
enough. I
would drop it.
+static struct mchp_otpc_packet *mchp_otpc_tag_to_packet(struct
mchp_otpc *otpc,
+ u32 tag)
+{
+ struct mchp_otpc_packet *packet;
+
+ list_for_each_entry(packet, &otpc->packets, list) {
+ if (packet->tag == tag)
+ return packet;
+ }
+
+ return NULL;
+}
+
+/**
+ * mchp_otpc_resolve_packet() - resolve offset to packet
+ * @otpc: OTPC private data
+ * @off: NVMEM offset (legacy ID-based or TAG-based)
+ *
+ * Legacy offsets (multiples of 4 within valid ID range) are resolved
+ * through ID lookup. Other offsets are treated as 4-byte ASCII tags.
+ *
+ * Return: pointer to packet if found, NULL otherwise
+ */
Same here.
+static struct mchp_otpc_packet *mchp_otpc_resolve_packet(struct
mchp_otpc *otpc,
+ u32 off)
+{
+ /*
+ * Legacy id based packet access: offset = id * 4
+ * Inside the driver we use continuous unsigned integer numbers
+ * for packet id, thus divide off by 4 before passing it to
+ * mchp_otpc_id_to_packet().
+ */
+ u32 id = off / 4;
+
+ if (!(off % 4) && id < otpc->npackets)
The tag can be anything, no? Can't the tag satisfy this condition and the
execution to wrongly use mchp_otpc_id_to_packet() ?
The tag is a FourCC code. So the minimum value would be 0x20202020 which
way too big than the number of packets the OTP could hold. So this
condition pretty much never fails. If you want me to add this
"0x20202020" value as an additional check I can do that.
Please mention FourCC in patch description.
Also, I think one would manage to access a packet with a tag that is not in FourCC format (tag >= 0x20202020). E.g. if the memory footprint is:
Header 0
-----------
Payload 0:
tag0 = 0x20202020
-----------
Header 1
-----------
Payload 1:
tag1 = 0x00002020
------------
Header 2
------------
Payload 2:
tag2 = 0x00000004
-----------
Header 3
-----------
Payload 3:
tag3 = 0x00002020
-----------
Header 4
-----------
Payload 4:
tag3 = 0x00002020
Since the initialization function mchp_otpc_read_packet_tag() don't validate the FourCC format with the minimal FourCC value, it will create a list with 5 packets as follows:
packet0:
- id = 0
- tag = 0x20202020
packet1:
- id = 0
- tag = 0x00002020
packet2:
- id = 2
- tag = 0x00000004
packet3:
- id = 3
- tag = 0x00002020
packet4:
- id = 4
- tag = 0x00002020
Thus, reaching mchp_otpc_read() with off=0x00002020 will allow the user to get packet1 using tag method (even though the user flashed multiple packets with the same tag; I'm not sure if we should validate this).
At the same time if off=0x00000004 (corresponding to packet2) the code will return the payload of packet1 as it will resolve the packet using mchp_otpc_id_to_packet().
Please correct me if I'm wrong.
Thank you,
Claudiu