Re: [PATCH v3 3/3] nvmem: stm32: add TAMP nvmem driver

From: Patrice CHOTARD

Date: Wed Nov 04 2026 - 08:46:28 EST

On 10/17/26 19:57, Krzysztof Kozlowski wrote:
> On Fri, Oct 16, 2026 at 05:26:05PM +0200, Patrice Chotard wrote:
>> +static int stm32_tamp_nvram_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct stm32_tamp_nvram_priv *priv;
>> + struct resource *res;
>> + struct device_node *of_node;
>> + const struct of_device_id *of_id;
>> + void __iomem *base_addr;
>> + int *zones_end;
>> +
>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + priv->dev = &pdev->dev;
>> +
>> + of_node = dev_of_node(dev);
>> + if (!of_node) {
>> + dev_err(dev, "Can't get of node of the device\n");
>> + return -ENODEV;
>
> Entire dev_of_node and if() look not necessary.

Ok

>
>> + }
>> +
>> + of_id = of_match_node(stm32_tamp_nvram_of_match, of_node);
>> + if (!of_id) {
>> + dev_err(dev, "Can't get of_id of the device\n");
>> + return -ENODEV;
>> + }
>> +
>> + priv->data = (struct stm32_tamp_nvram_plat *)of_id->data;
>
> Use wrapper - device_get_match_data or something similar (don't remember
> exactly).

Ok>
>> + if (!priv->data) {
>> + dev_err(dev, "Can't get platform data of the node\n");
>> + return -ENODEV;
>> + }
>> +
>> + priv->config_regmap = syscon_node_to_regmap(dev->parent->of_node);
>> + if (IS_ERR(priv->config_regmap))
>> + return dev_err_probe(dev, PTR_ERR(priv->config_regmap),
>> + "Can't get TAMP syscon regmap\n");
>> +
>> + priv->idx_bkpreg_zones_end = stm32_tamp_nvram_get_backup_zones(priv);
>> + if (IS_ERR(priv->idx_bkpreg_zones_end))
>> + return dev_err_probe(dev, PTR_ERR(priv->idx_bkpreg_zones_end),
>> + "Can't determine protection zone\n");
>> +
>> + zones_end = priv->idx_bkpreg_zones_end;
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +
>
> Drop blank line

Ok>
>> + if (!res) {
>> + dev_err(dev, "Can't get resource\n");
>> + return -ENODEV;
>> + }
>> +
>> + base_addr = devm_ioremap_resource(dev, res);
>
> You cannot use wrapper over these two? And I am a bit confused - device
> takes syscon from the parent, so how does it map also another memory?

This part will be reworked. On patch 2 of DT series [1], sashiko made a similar
remark regarding the overlapping parent syscon's memory region.
I will address this point in next revision.

[1] https://patchwork.kernel.org/project/linux-arm-kernel/patch/20260930-upstream_tamp_nvram_dts_part-v2-2-bd73158ce46b@xxxxxxxxxxx/

>
>> + if (IS_ERR(base_addr)) {
>> + dev_err(dev, "Can't remap resource\n");
>> + return PTR_ERR(base_addr);
>> + }
>> +
>> + priv->cfg.name = "stm32-tamp-nvram";
>> + priv->cfg.word_size = 1;
>> + priv->cfg.stride = 1;
>> + priv->cfg.dev = dev;
>> + priv->cfg.priv = priv;
>> + priv->cfg.owner = THIS_MODULE;
>> + priv->cfg.type = NVMEM_TYPE_BATTERY_BACKED;
>> + priv->cfg.size = resource_size(res);
>> + priv->cfg.reg_read = stm32_tamp_nvram_read;
>> + priv->cfg.reg_write = stm32_tamp_nvram_write;
>> +
>> + if (priv->data->nb_zones == NB_ZONES_STM32MP2) {
>> + priv->bkpreg_access = stm32_tamp_get_access_rights_mp2x(priv);
>> +
>> + if (IS_ERR(priv->bkpreg_access))
>> + return PTR_ERR(priv->bkpreg_access);
>> +
>> + dev_dbg(dev, "\n"
>> + "Zone 1-RIF1 %3d - %3d %c%c\n"
>> + "Zone 1-RIF2 %3d - %3d %c%c\n"
>> + "Zone 2-RIF1 %3d - %3d %c%c\n"
>> + "Zone 2-RIF2 %3d - %3d %c%c\n"
>> + "Zone 3-RIF1 %3d - %3d %c%c\n"
>> + "Zone 3-RIF0 %3d - %3d %c%c\n"
>> + "Zone 3-RIF2 %3d - %3d %c%c\n",
>> + 0,
>> + zones_end[BKPREG_PROTECTION_ZONE_1_RIF1],
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_1_RIF1],
>> + true) ? 'R' : '-',
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_1_RIF1],
>> + false) ? 'W' : '-',
>> + zones_end[BKPREG_PROTECTION_ZONE_1_RIF1] + 1,
>> + zones_end[BKPREG_PROTECTION_ZONE_1_RIF2],
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_1_RIF2],
>> + true) ? 'R' : '-',
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_1_RIF2],
>> + false) ? 'W' : '-',
>> + zones_end[BKPREG_PROTECTION_ZONE_1_RIF2] + 1,
>> + zones_end[BKPREG_PROTECTION_ZONE_2_RIF1],
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_2_RIF1],
>> + true) ? 'R' : '-',
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_2_RIF1],
>> + false) ? 'W' : '-',
>> + zones_end[BKPREG_PROTECTION_ZONE_2_RIF1] + 1,
>> + zones_end[BKPREG_PROTECTION_ZONE_2_RIF2],
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_2_RIF2],
>> + true) ? 'R' : '-',
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_2_RIF2],
>> + false) ? 'W' : '-',
>> + zones_end[BKPREG_PROTECTION_ZONE_2_RIF2] + 1,
>> + zones_end[BKPREG_PROTECTION_ZONE_3_RIF1],
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_3_RIF1],
>> + true) ? 'R' : '-',
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_3_RIF1],
>> + false) ? 'W' : '-',
>> + zones_end[BKPREG_PROTECTION_ZONE_3_RIF1] + 1,
>> + zones_end[BKPREG_PROTECTION_ZONE_3_RIF0],
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_3_RIF0],
>> + true) ? 'R' : '-',
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_3_RIF0],
>> + false) ? 'W' : '-',
>> + zones_end[BKPREG_PROTECTION_ZONE_3_RIF0] + 1,
>> + zones_end[BKPREG_PROTECTION_ZONE_3_RIF2],
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_3_RIF2],
>> + true) ? 'R' : '-',
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_3_RIF2],
>> + false) ? 'W' : '-'
>> + );
>> + } else if (priv->data->nb_zones == NB_ZONES_STM32MP1) {
>> + priv->bkpreg_access = stm32mp1_tamp_bkpreg_access;
>> +
>> + dev_dbg(dev, "\n"
>> + "Zone 1 %3d - %3d %c%c\n"
>> + "Zone 2 %3d - %3d %c%c\n"
>> + "Zone 3 %3d - %3d %c%c\n",
>> + 0,
>> + zones_end[BKPREG_PROTECTION_ZONE_1],
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_1],
>> + true) ? 'R' : '-',
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_1],
>> + false) ? 'W' : '-',
>> + zones_end[BKPREG_PROTECTION_ZONE_1] + 1,
>> + zones_end[BKPREG_PROTECTION_ZONE_2],
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_2],
>> + true) ? 'R' : '-',
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_2],
>> + false) ? 'W' : '-',
>> + zones_end[BKPREG_PROTECTION_ZONE_2] + 1,
>> + zones_end[BKPREG_PROTECTION_ZONE_3],
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_3],
>> + true) ? 'R' : '-',
>> + stm32_tamp_nvram_rights(priv,
>> + zones_end[BKPREG_PROTECTION_ZONE_3],
>> + false) ? 'W' : '-');
>> + }
>> +
>> + platform_set_drvdata(pdev, priv);
>> + priv->bkpregs_regmap = devm_regmap_init_mmio(dev, base_addr,
>> + priv->data->bkpregs_regmap_cfg);
>> + if (IS_ERR(priv->bkpregs_regmap)) {
>> + dev_err(dev, "failed to init regmap: %ld\n", PTR_ERR(priv->bkpregs_regmap));
>> + return PTR_ERR(priv->bkpregs_regmap);
>
> return dev_err_probe

Ok

Thanks
Patrice

>
>
> Best regards,
> Krzysztof
>