linux-kernel - Re: [PATCH] nvmem: add QTI SDAM driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 19 Dec 2019 10:38:32 +0000
From:   Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To:     Shyam Kumar Thella <sthella@...eaurora.org>
Cc:     agross@...nel.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH] nvmem: add QTI SDAM driver



On 17/12/2019 09:20, Shyam Kumar Thella wrote:
> QTI SDAM driver allows PMIC peripherals to access the shared memory
> that is available on QTI PMICs.
> 
> Change-Id: I40005646ab1fbba9e0e4aa68e0a61cfbc7b51ba6
> Signed-off-by: Shyam Kumar Thella <sthella@...eaurora.org>
> ---
>   drivers/nvmem/Kconfig          |   8 ++
>   drivers/nvmem/Makefile         |   1 +
>   drivers/nvmem/qcom-spmi-sdam.c | 197 +++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 206 insertions(+)
Not repeating comments from Bjorn,
Apart from that I have twos comment.

1>
Any reason why there is no Device tree bindings documented for this driver?

>   create mode 100644 drivers/nvmem/qcom-spmi-sdam.c
> 
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index 73567e9..35efab1 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -109,6 +109,14 @@ config QCOM_QFPROM
>   	  This driver can also be built as a module. If so, the module
>   	  will be called nvmem_qfprom.
>   
> +config NVMEM_SPMI_SDAM
> +	tristate "SPMI SDAM Support"
> +	depends on SPMI
> +	help
> +	  This driver supports the Shared Direct Access Memory Module on
> +	  Qualcomm Technologies, Inc. PMICs. It provides the clients
> +	  an interface to read/write to the SDAM module's shared memory.
> +
>   config ROCKCHIP_EFUSE
>   	tristate "Rockchip eFuse Support"
>   	depends on ARCH_ROCKCHIP || COMPILE_TEST
> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> index 9e66782..877a0b0 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_MTK_EFUSE)		+= nvmem_mtk-efuse.o
>   nvmem_mtk-efuse-y		:= mtk-efuse.o
>   obj-$(CONFIG_QCOM_QFPROM)	+= nvmem_qfprom.o
>   nvmem_qfprom-y			:= qfprom.o
> +obj-$(CONFIG_NVMEM_SPMI_SDAM)	+= qcom-spmi-sdam.o
>   obj-$(CONFIG_ROCKCHIP_EFUSE)	+= nvmem_rockchip_efuse.o
>   nvmem_rockchip_efuse-y		:= rockchip-efuse.o
>   obj-$(CONFIG_ROCKCHIP_OTP)	+= nvmem-rockchip-otp.o
> diff --git a/drivers/nvmem/qcom-spmi-sdam.c b/drivers/nvmem/qcom-spmi-sdam.c
> new file mode 100644
> index 0000000..e80a446
> --- /dev/null
> +++ b/drivers/nvmem/qcom-spmi-sdam.c
> @@ -0,0 +1,197 @@

...
> +
> +static int sdam_probe(struct platform_device *pdev)
> +{
> +	struct sdam_chip *sdam;
> +	struct nvmem_device *nvmem;
> +	struct nvmem_config *sdam_config;
> +	unsigned int val = 0;
> +	int rc;
> +
> +	sdam = devm_kzalloc(&pdev->dev, sizeof(*sdam), GFP_KERNEL);
> +	if (!sdam)
> +		return -ENOMEM;
> +
> +	sdam_config = devm_kzalloc(&pdev->dev, sizeof(*sdam_config),
> +							GFP_KERNEL);
> +	if (!sdam_config)
> +		return -ENOMEM;
> +
> +	sdam->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!sdam->regmap) {
> +		pr_err("Failed to get regmap handle\n");
> +		return -ENXIO;
> +	}
> +
> +	rc = of_property_read_u32(pdev->dev.of_node, "reg", &sdam->base);
> +	if (rc < 0) {
> +		pr_err("Failed to get SDAM base, rc=%d\n", rc);
> +		return -EINVAL;
> +	}
> +
> +	rc = regmap_read(sdam->regmap, sdam->base + SDAM_SIZE, &val);
> +	if (rc < 0) {
> +		pr_err("Failed to read SDAM_SIZE rc=%d\n", rc);
> +		return -EINVAL;
> +	}
> +	sdam->size = val * 32;
> +
> +	sdam_config->dev = &pdev->dev;
> +	sdam_config->name = "spmi_sdam";
> +	sdam_config->id = pdev->id;
> +	sdam_config->owner = THIS_MODULE,
> +	sdam_config->stride = 1;
> +	sdam_config->word_size = 1;
> +	sdam_config->reg_read = sdam_read;
> +	sdam_config->reg_write = sdam_write;
> +	sdam_config->priv = sdam;
> +
> +	nvmem = nvmem_register(sdam_config);
2>
May be devm_nvmem_register() here would  be better.


--srini