| Message ID | 20230623141806.13388-4-quic_kbajaj@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | soc: qcom: llcc: Add support for QDU1000/QRU1000 | expand |
Hi Komal, kernel test robot noticed the following build warnings: [auto build test WARNING on robh/for-next] [also build test WARNING on linus/master v6.4-rc7 next-20230623] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Komal-Bajaj/dt-bindings-nvmem-sec-qfprom-Add-bindings-for-secure-qfprom/20230623-222054 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next patch link: https://lore.kernel.org/r/20230623141806.13388-4-quic_kbajaj%40quicinc.com patch subject: [PATCH v4 3/6] nvmem: sec-qfprom: Add Qualcomm secure QFPROM support. config: arm-allmodconfig (https://download.01.org/0day-ci/archive/20230624/202306240837.Keyvmt2I-lkp@intel.com/config) compiler: arm-linux-gnueabi-gcc (GCC) 12.3.0 reproduce: (https://download.01.org/0day-ci/archive/20230624/202306240837.Keyvmt2I-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202306240837.Keyvmt2I-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/nvmem/sec-qfprom.c:31: warning: expecting prototype for struct sec_sec_qfprom_priv. Prototype was for struct sec_qfprom_priv instead vim +31 drivers/nvmem/sec-qfprom.c 20 21 22 /** 23 * struct sec_sec_qfprom_priv - structure holding secure qfprom attributes 24 * 25 * @qfpseccorrected: iomapped memory space for secure qfprom corrected address space. 26 * @dev: qfprom device structure. 27 */ 28 struct sec_qfprom_priv { 29 phys_addr_t qfpseccorrected; 30 struct device *dev; > 31 }; 32
On Fri, Jun 23, 2023 at 07:48:03PM +0530, Komal Bajaj wrote: > For some of the Qualcomm SoC's, it is possible that > some of the fuse regions or entire qfprom region is > protected from non-secure access. In such situations, > linux will have to use secure calls to read the region. Linux > With that motivation, add secure qfprom driver. Ensuring > the address to read is word aligned since our secure I/O > only supports word size I/O. What do you mean with this last sentence? I don't see anything in your patch demanding this. Perhaps just drop this sentence? > > Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com> > --- > drivers/nvmem/Kconfig | 12 ++++ > drivers/nvmem/Makefile | 2 + > drivers/nvmem/sec-qfprom.c | 116 +++++++++++++++++++++++++++++++++++++ > 3 files changed, 130 insertions(+) > create mode 100644 drivers/nvmem/sec-qfprom.c > > diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig > index b291b27048c7..2b0dd73d899e 100644 > --- a/drivers/nvmem/Kconfig > +++ b/drivers/nvmem/Kconfig > @@ -216,6 +216,18 @@ config NVMEM_QCOM_QFPROM > This driver can also be built as a module. If so, the module > will be called nvmem_qfprom. > > +config NVMEM_QCOM_SEC_QFPROM > + tristate "QCOM SECURE QFPROM Support" > + depends on ARCH_QCOM || COMPILE_TEST > + depends on HAS_IOMEM > + select QCOM_SCM > + help > + Say y here to enable secure QFPROM support. The secure QFPROM provides access > + functions for QFPROM data to rest of the drivers via nvmem interface. > + > + This driver can also be built as a module. If so, the module will be called > + nvmem_sec_qfprom. > + > config NVMEM_RAVE_SP_EEPROM > tristate "Rave SP EEPROM Support" > depends on RAVE_SP_CORE > diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile > index f82431ec8aef..4b4bf5880488 100644 > --- a/drivers/nvmem/Makefile > +++ b/drivers/nvmem/Makefile > @@ -44,6 +44,8 @@ obj-$(CONFIG_NVMEM_NINTENDO_OTP) += nvmem-nintendo-otp.o > nvmem-nintendo-otp-y := nintendo-otp.o > obj-$(CONFIG_NVMEM_QCOM_QFPROM) += nvmem_qfprom.o > nvmem_qfprom-y := qfprom.o > +obj-$(CONFIG_NVMEM_QCOM_SEC_QFPROM) += nvmem_sec_qfprom.o > +nvmem_sec_qfprom-y := sec-qfprom.o > obj-$(CONFIG_NVMEM_RAVE_SP_EEPROM) += nvmem-rave-sp-eeprom.o > nvmem-rave-sp-eeprom-y := rave-sp-eeprom.o > obj-$(CONFIG_NVMEM_RMEM) += nvmem-rmem.o > diff --git a/drivers/nvmem/sec-qfprom.c b/drivers/nvmem/sec-qfprom.c > new file mode 100644 > index 000000000000..47b2c71593dd > --- /dev/null > +++ b/drivers/nvmem/sec-qfprom.c > @@ -0,0 +1,116 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include <linux/clk.h> Please review your include list, this and a few others doesn't seem to be used. > +#include <linux/device.h> > +#include <linux/firmware/qcom/qcom_scm.h> > +#include <linux/io.h> > +#include <linux/iopoll.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/mod_devicetable.h> > +#include <linux/nvmem-provider.h> > +#include <linux/platform_device.h> > +#include <linux/pm_domain.h> > +#include <linux/pm_runtime.h> > +#include <linux/property.h> > +#include <linux/regulator/consumer.h> > + > + > +/** > + * struct sec_sec_qfprom_priv - structure holding secure qfprom attributes > + * > + * @qfpseccorrected: iomapped memory space for secure qfprom corrected address space. > + * @dev: qfprom device structure. > + */ > +struct sec_qfprom_priv { There no risk of confusion here, so please drop the "_priv" suffix. > + phys_addr_t qfpseccorrected; Same thing here, so "base" or "addr" would suffice. > + struct device *dev; > +}; > + > +static int sec_qfprom_reg_read(void *context, unsigned int reg, void *_val, size_t bytes) > +{ > + struct sec_qfprom_priv *priv = context; > + u8 *val = _val; > + u8 *tmp; > + u32 read_val; > + unsigned int i; > + > + for (i = 0; i < bytes; i++, reg++) { > + if (i == 0 || reg % 4 == 0) { > + if (qcom_scm_io_readl(priv->qfpseccorrected + (reg & ~3), &read_val)) { > + dev_err(priv->dev, "Couldn't access fuse register\n"); > + return -EINVAL; > + } > + tmp = (u8 *)&read_val; > + } > + > + val[i] = tmp[reg & 3]; > + } > + > + return 0; > +} > + > +static void sec_qfprom_runtime_disable(void *data) > +{ > + pm_runtime_disable(data); > +} > + > +static int sec_qfprom_probe(struct platform_device *pdev) > +{ > + struct nvmem_config econfig = { > + .name = "sec-qfprom", > + .stride = 1, > + .word_size = 1, > + .id = NVMEM_DEVID_AUTO, > + .reg_read = sec_qfprom_reg_read, > + }; > + struct device *dev = &pdev->dev; > + struct resource *res; > + struct nvmem_device *nvmem; > + struct sec_qfprom_priv *priv; > + int ret; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + priv->qfpseccorrected = res->start; > + if (!priv->qfpseccorrected) > + return -ENOMEM; > + > + econfig.size = resource_size(res); > + econfig.dev = dev; > + econfig.priv = priv; > + > + priv->dev = dev; > + > + pm_runtime_enable(dev); > + ret = devm_add_action_or_reset(dev, sec_qfprom_runtime_disable, dev); > + if (ret) > + return ret; Please use devm_pm_runtime_enable(). > + > + nvmem = devm_nvmem_register(dev, &econfig); > + > + return PTR_ERR_OR_ZERO(nvmem); > +} > + > +static const struct of_device_id sec_qfprom_of_match[] = { > + { .compatible = "qcom,sec-qfprom",}, > + {/* sentinel */}, > +}; > +MODULE_DEVICE_TABLE(of, sec_qfprom_of_match); > + > +static struct platform_driver qfprom_driver = { > + .probe = sec_qfprom_probe, > + .driver = { > + .name = "qcom,sec_qfprom", No comma here please, qcom_sec_qfprom. > + .of_match_table = sec_qfprom_of_match, > + }, > +}; > +module_platform_driver(qfprom_driver); > +MODULE_DESCRIPTION("Qualcomm Secure QFPROM driver"); > +MODULE_LICENSE("GPL v2"); Please change this to "GPL". Regards, Bjorn
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index b291b27048c7..2b0dd73d899e 100644 --- a/drivers/nvmem/Kconfig +++ b/drivers/nvmem/Kconfig @@ -216,6 +216,18 @@ config NVMEM_QCOM_QFPROM This driver can also be built as a module. If so, the module will be called nvmem_qfprom. +config NVMEM_QCOM_SEC_QFPROM + tristate "QCOM SECURE QFPROM Support" + depends on ARCH_QCOM || COMPILE_TEST + depends on HAS_IOMEM + select QCOM_SCM + help + Say y here to enable secure QFPROM support. The secure QFPROM provides access + functions for QFPROM data to rest of the drivers via nvmem interface. + + This driver can also be built as a module. If so, the module will be called + nvmem_sec_qfprom. + config NVMEM_RAVE_SP_EEPROM tristate "Rave SP EEPROM Support" depends on RAVE_SP_CORE diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile index f82431ec8aef..4b4bf5880488 100644 --- a/drivers/nvmem/Makefile +++ b/drivers/nvmem/Makefile @@ -44,6 +44,8 @@ obj-$(CONFIG_NVMEM_NINTENDO_OTP) += nvmem-nintendo-otp.o nvmem-nintendo-otp-y := nintendo-otp.o obj-$(CONFIG_NVMEM_QCOM_QFPROM) += nvmem_qfprom.o nvmem_qfprom-y := qfprom.o +obj-$(CONFIG_NVMEM_QCOM_SEC_QFPROM) += nvmem_sec_qfprom.o +nvmem_sec_qfprom-y := sec-qfprom.o obj-$(CONFIG_NVMEM_RAVE_SP_EEPROM) += nvmem-rave-sp-eeprom.o nvmem-rave-sp-eeprom-y := rave-sp-eeprom.o obj-$(CONFIG_NVMEM_RMEM) += nvmem-rmem.o diff --git a/drivers/nvmem/sec-qfprom.c b/drivers/nvmem/sec-qfprom.c new file mode 100644 index 000000000000..47b2c71593dd --- /dev/null +++ b/drivers/nvmem/sec-qfprom.c @@ -0,0 +1,116 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved. + */ + +#include <linux/clk.h> +#include <linux/device.h> +#include <linux/firmware/qcom/qcom_scm.h> +#include <linux/io.h> +#include <linux/iopoll.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/mod_devicetable.h> +#include <linux/nvmem-provider.h> +#include <linux/platform_device.h> +#include <linux/pm_domain.h> +#include <linux/pm_runtime.h> +#include <linux/property.h> +#include <linux/regulator/consumer.h> + + +/** + * struct sec_sec_qfprom_priv - structure holding secure qfprom attributes + * + * @qfpseccorrected: iomapped memory space for secure qfprom corrected address space. + * @dev: qfprom device structure. + */ +struct sec_qfprom_priv { + phys_addr_t qfpseccorrected; + struct device *dev; +}; + +static int sec_qfprom_reg_read(void *context, unsigned int reg, void *_val, size_t bytes) +{ + struct sec_qfprom_priv *priv = context; + u8 *val = _val; + u8 *tmp; + u32 read_val; + unsigned int i; + + for (i = 0; i < bytes; i++, reg++) { + if (i == 0 || reg % 4 == 0) { + if (qcom_scm_io_readl(priv->qfpseccorrected + (reg & ~3), &read_val)) { + dev_err(priv->dev, "Couldn't access fuse register\n"); + return -EINVAL; + } + tmp = (u8 *)&read_val; + } + + val[i] = tmp[reg & 3]; + } + + return 0; +} + +static void sec_qfprom_runtime_disable(void *data) +{ + pm_runtime_disable(data); +} + +static int sec_qfprom_probe(struct platform_device *pdev) +{ + struct nvmem_config econfig = { + .name = "sec-qfprom", + .stride = 1, + .word_size = 1, + .id = NVMEM_DEVID_AUTO, + .reg_read = sec_qfprom_reg_read, + }; + struct device *dev = &pdev->dev; + struct resource *res; + struct nvmem_device *nvmem; + struct sec_qfprom_priv *priv; + int ret; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + priv->qfpseccorrected = res->start; + if (!priv->qfpseccorrected) + return -ENOMEM; + + econfig.size = resource_size(res); + econfig.dev = dev; + econfig.priv = priv; + + priv->dev = dev; + + pm_runtime_enable(dev); + ret = devm_add_action_or_reset(dev, sec_qfprom_runtime_disable, dev); + if (ret) + return ret; + + nvmem = devm_nvmem_register(dev, &econfig); + + return PTR_ERR_OR_ZERO(nvmem); +} + +static const struct of_device_id sec_qfprom_of_match[] = { + { .compatible = "qcom,sec-qfprom",}, + {/* sentinel */}, +}; +MODULE_DEVICE_TABLE(of, sec_qfprom_of_match); + +static struct platform_driver qfprom_driver = { + .probe = sec_qfprom_probe, + .driver = { + .name = "qcom,sec_qfprom", + .of_match_table = sec_qfprom_of_match, + }, +}; +module_platform_driver(qfprom_driver); +MODULE_DESCRIPTION("Qualcomm Secure QFPROM driver"); +MODULE_LICENSE("GPL v2");
For some of the Qualcomm SoC's, it is possible that some of the fuse regions or entire qfprom region is protected from non-secure access. In such situations, linux will have to use secure calls to read the region. With that motivation, add secure qfprom driver. Ensuring the address to read is word aligned since our secure I/O only supports word size I/O. Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com> --- drivers/nvmem/Kconfig | 12 ++++ drivers/nvmem/Makefile | 2 + drivers/nvmem/sec-qfprom.c | 116 +++++++++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+) create mode 100644 drivers/nvmem/sec-qfprom.c