[PATCH v2 2/3] nvmem: syscon: Add syscon backed nvmem driver
Marek Vasut
marex at denx.de
Tue May 23 20:30:46 PDT 2023
On 5/18/23 16:22, Krzysztof Kozlowski wrote:
[...]
>> +++ b/drivers/nvmem/nvmem-syscon.c
>> @@ -0,0 +1,105 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2022 Marek Vasut <marex at denx.de>
>> + *
>> + * Based on snvs_lpgpr.c .
>> + */
>> +
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/nvmem-provider.h>
>> +#include <linux/of_device.h>
>> +#include <linux/regmap.h>
>> +
>> +struct nvmem_syscon_priv {
>> + struct device_d *dev;
>> + struct regmap *regmap;
>> + struct nvmem_config cfg;
>> + unsigned int off;
>> +};
>> +
>> +static int nvmem_syscon_write(void *context, unsigned int offset, void *val,
>> + size_t bytes)
>> +{
>> + struct nvmem_syscon_priv *priv = context;
>> +
>> + return regmap_bulk_write(priv->regmap, priv->off + offset,
>> + val, bytes / 4);
>> +}
>> +
>> +static int nvmem_syscon_read(void *context, unsigned int offset, void *val,
>> + size_t bytes)
>> +{
>> + struct nvmem_syscon_priv *priv = context;
>> +
>> + return regmap_bulk_read(priv->regmap, priv->off + offset,
>> + val, bytes / 4);
>> +}
>> +
>> +static int nvmem_syscon_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct device_node *node = dev->of_node;
>> + struct device_node *syscon_node;
>> + struct nvmem_syscon_priv *priv;
>> + struct nvmem_device *nvmem;
>> + struct nvmem_config *cfg;
>> + int ret;
>> +
>> + if (!node)
>> + return -ENOENT;
>> +
>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + ret = of_property_read_u32_index(node, "reg", 0, &priv->off);
>> + if (ret)
>> + return ret;
>> +
>> + ret = of_property_read_u32_index(node, "reg", 1, &priv->cfg.size);
>> + if (ret)
>> + return ret;
>> +
>> + syscon_node = of_get_parent(node);
>
> This does not look correct. You hard-code dependency that it must be a
> child of syscon node. This is weird requirement and not explained in the
> bindings.
>
> Why this cannot be then generic MMIO node? Why it has to be a child of
> syscon?
Because I already have a syscon node and I want to expose only a subset
of it to userspace (bootcounter in my case) . See 1/3, I replied there,
let's continue the discussion there.
I fixed the rest in prep for V3, sorry for the horrid delays in replies.
More information about the linux-arm-kernel
mailing list