| 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
| ||
|
Message-ID: <51847629489852d493af928580c5a2de@walle.cc>
Date: Thu, 13 Apr 2023 18:08:18 +0200
From: Michael Walle <michael@...le.cc>
To: "Vaibhaav Ram T.L" <vaibhaavram.tl@...rochip.com>
Cc: gregkh@...uxfoundation.org, arnd@...db.de,
kumaravel.thiagarajan@...rochip.com,
tharunkumar.pasumarthi@...rochip.com, linux-kernel@...r.kernel.org,
linux-gpio@...r.kernel.org, UNGLinuxDriver@...rochip.com
Subject: Re: [PATCH v9 char-misc-next 1/2] misc: microchip: pci1xxxx: Add
support to read and write into PCI1XXXX OTP via NVMEM sysfs
Am 2023-04-13 12:53, schrieb Vaibhaav Ram T.L:
> From: Kumaravel Thiagarajan <kumaravel.thiagarajan@...rochip.com>
>
> Microchip's pci1xxxx is an unmanaged PCIe3.1a switch for consumer,
> industrial, and automotive applications. This switch integrates OTP
> and EEPROM to enable customization of the part in the field.
> This patch adds support to read and write into PCI1XXXX OTP
> via NVMEM sysfs
Thanks for working on that. Now this driver is less of an oddball
one.
> Signed-off-by: Kumaravel Thiagarajan
> <kumaravel.thiagarajan@...rochip.com>
> Co-developed-by: Tharun Kumar P <tharunkumar.pasumarthi@...rochip.com>
> Signed-off-by: Tharun Kumar P <tharunkumar.pasumarthi@...rochip.com>
> Co-developed-by: Vaibhaav Ram T.L <vaibhaavram.tl@...rochip.com>
> Signed-off-by: Vaibhaav Ram T.L <vaibhaavram.tl@...rochip.com>
> ---
> MAINTAINERS | 2 +
> drivers/misc/mchp_pci1xxxx/Kconfig | 1 +
> drivers/misc/mchp_pci1xxxx/Makefile | 2 +-
> .../misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c | 301 ++++++++++++++++++
> 4 files changed, 305 insertions(+), 1 deletion(-)
> create mode 100644 drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a33261b4592f..58e4b9d6ed61 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13721,12 +13721,14 @@
> F: Documentation/devicetree/bindings/mtd/atmel-nand.txt
> F: drivers/mtd/nand/raw/atmel/*
>
> MICROCHIP PCI1XXXX GP DRIVER
> +M: Vaibhaav Ram T.L <vaibhaavram.tl@...rochip.com>
> M: Kumaravel Thiagarajan <kumaravel.thiagarajan@...rochip.com>
> L: linux-gpio@...r.kernel.org
> S: Supported
> F: drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
> F: drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.h
> F: drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c
> +F: drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c
>
> MICROCHIP OTPC DRIVER
> M: Claudiu Beznea <claudiu.beznea@...rochip.com>
> diff --git a/drivers/misc/mchp_pci1xxxx/Kconfig
> b/drivers/misc/mchp_pci1xxxx/Kconfig
> index 4abb47de7219..64e457581fb4 100644
> --- a/drivers/misc/mchp_pci1xxxx/Kconfig
> +++ b/drivers/misc/mchp_pci1xxxx/Kconfig
> @@ -2,6 +2,7 @@ config GP_PCI1XXXX
> tristate "Microchip PCI1XXXX PCIe to GPIO Expander + OTP/EEPROM
> manager"
> depends on PCI
> depends on GPIOLIB
> + depends on NVMEM_SYSFS
> select GPIOLIB_IRQCHIP
> select AUXILIARY_BUS
> help
> diff --git a/drivers/misc/mchp_pci1xxxx/Makefile
> b/drivers/misc/mchp_pci1xxxx/Makefile
> index fc4615cfe28b..ae31251dab37 100644
> --- a/drivers/misc/mchp_pci1xxxx/Makefile
> +++ b/drivers/misc/mchp_pci1xxxx/Makefile
> @@ -1 +1 @@
> -obj-$(CONFIG_GP_PCI1XXXX) := mchp_pci1xxxx_gp.o mchp_pci1xxxx_gpio.o
> +obj-$(CONFIG_GP_PCI1XXXX) := mchp_pci1xxxx_gp.o mchp_pci1xxxx_gpio.o
> mchp_pci1xxxx_otpe2p.o
> diff --git a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c
> b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c
> new file mode 100644
> index 000000000000..23f44ad6773e
> --- /dev/null
> +++ b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c
> @@ -0,0 +1,301 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2022-2023 Microchip Technology Inc.
> +// PCI1xxxx OTP/EEPROM driver
> +
> +#include <linux/auxiliary_bus.h>
> +#include <linux/bio.h>
> +#include <linux/device.h>
> +#include <linux/iopoll.h>
> +#include <linux/kthread.h>
Is this needed? I don't see any threads. Also bio.h. Please double check
your includes.
> +#include <linux/module.h>
> +#include <linux/nvmem-provider.h>
> +
> +#include "mchp_pci1xxxx_gp.h"
> +
> +#define AUX_DRIVER_NAME "PCI1xxxxOTPE2P"
> +#define OTP_NAME "pci1xxxx_otp"
> +
> +#define PERI_PF3_SYSTEM_REG_ADDR_BASE 0x2000
> +#define PERI_PF3_SYSTEM_REG_LENGTH 0x4000
> +
> +#define OTP_SIZE_BYTES 8192
> +
> +#define CONFIG_REG_ADDR_BASE 0
> +#define OTP_REG_ADDR_BASE 0x1000
> +
> +#define MMAP_OTP_OFFSET(x) (OTP_REG_ADDR_BASE + x)
> +#define MMAP_CFG_OFFSET(x) (CONFIG_REG_ADDR_BASE + x)
> +
> +#define STATUS_READ_DELAY_US 1
> +#define STATUS_READ_TIMEOUT_US 20000
> +
> +#define OTP_ADDR_HIGH_OFFSET 0x04
> +#define OTP_ADDR_LOW_OFFSET 0x08
> +#define OTP_PRGM_DATA_OFFSET 0x10
> +#define OTP_PRGM_MODE_OFFSET 0x14
> +#define OTP_RD_DATA_OFFSET 0x18
> +#define OTP_FUNC_CMD_OFFSET 0x20
> +#define OTP_CMD_GO_OFFSET 0x28
> +#define OTP_PASS_FAIL_OFFSET 0x2C
> +#define OTP_STATUS_OFFSET 0x30
> +
> +#define OTP_FUNC_RD_BIT BIT(0)
> +#define OTP_FUNC_PGM_BIT BIT(1)
> +#define OTP_CMD_GO_BIT BIT(0)
> +#define OTP_STATUS_BUSY_BIT BIT(0)
> +#define OTP_PGM_MODE_BYTE_BIT BIT(0)
> +#define OTP_FAIL_BIT BIT(0)
> +
> +#define OTP_PWR_DN_BIT BIT(0)
> +#define OTP_PWR_DN_OFFSET 0x00
> +
> +#define CFG_SYS_LOCK_OFFSET 0xA0
> +#define CFG_SYS_LOCK_PF3 BIT(5)
> +
> +#define BYTE_LOW (GENMASK(7, 0))
> +#define BYTE_HIGH (GENMASK(12, 8))
> +
> +struct pci1xxxx_otp_eeprom_device {
> + struct auxiliary_device *pdev;
> + void __iomem *reg_base;
> + struct nvmem_config nvmem_config_otp;
> + struct nvmem_device *nvmem_otp;
> +};
> +
> +static int set_sys_lock(struct pci1xxxx_otp_eeprom_device *priv)
> +{
> + void __iomem *sys_lock = priv->reg_base +
> + MMAP_CFG_OFFSET(CFG_SYS_LOCK_OFFSET);
> + u8 data;
> +
> + writel(CFG_SYS_LOCK_PF3, sys_lock);
> + data = readl(sys_lock);
> + if (data != CFG_SYS_LOCK_PF3)
> + return -EPERM;
> +
> + return 0;
> +}
> +
> +static void release_sys_lock(struct pci1xxxx_otp_eeprom_device *priv)
> +{
> + void __iomem *sys_lock = priv->reg_base +
> + MMAP_CFG_OFFSET(CFG_SYS_LOCK_OFFSET);
> + writel(0, sys_lock);
> +}
> +
> +static void otp_device_set_address(struct pci1xxxx_otp_eeprom_device
> *priv,
> + u16 address)
> +{
> + u16 lo, hi;
> +
> + lo = address & BYTE_LOW;
> + hi = (address & BYTE_HIGH) >> 8;
> + writew(lo, priv->reg_base + MMAP_OTP_OFFSET(OTP_ADDR_LOW_OFFSET));
> + writew(hi, priv->reg_base + MMAP_OTP_OFFSET(OTP_ADDR_HIGH_OFFSET));
> +}
> +
> +static int pci1xxxx_otp_read(void *priv_t, unsigned int off,
> + void *buf_t, size_t count)
> +{
> + struct pci1xxxx_otp_eeprom_device *priv = priv_t;
> + char *buf = buf_t;
> + void __iomem *rb;
> + u32 regval;
> + u32 byte;
> + int ret;
> + u8 data;
> +
> + if (priv != NULL)
> + rb = priv->reg_base;
> + else
> + return -ENODEV;
Unneeded check, priv cannot be NULL, right?
> +
> + ret = set_sys_lock(priv);
> + if (ret)
> + return ret;
> +
> + for (byte = 0; byte < count; byte++) {
> + otp_device_set_address(priv, (u16)(off + byte));
> + data = readl(rb + MMAP_OTP_OFFSET(OTP_FUNC_CMD_OFFSET));
> + writel(data | OTP_FUNC_RD_BIT,
> + rb + MMAP_OTP_OFFSET(OTP_FUNC_CMD_OFFSET));
> + data = readl(rb + MMAP_OTP_OFFSET(OTP_CMD_GO_OFFSET));
> + writel(data | OTP_CMD_GO_BIT,
> + rb + MMAP_OTP_OFFSET(OTP_CMD_GO_OFFSET));
> +
> + ret = read_poll_timeout(readl, regval,
> + !(regval & OTP_STATUS_BUSY_BIT),
> + STATUS_READ_DELAY_US,
> + STATUS_READ_TIMEOUT_US, true,
> + rb + MMAP_OTP_OFFSET(OTP_STATUS_OFFSET));
> +
> + data = readl(rb + MMAP_OTP_OFFSET(OTP_PASS_FAIL_OFFSET));
> + if (ret < 0 || data & OTP_FAIL_BIT)
> + break;
No error handling?
> +
> + buf[byte] = readl(rb + MMAP_OTP_OFFSET(OTP_RD_DATA_OFFSET));
> + }
> + release_sys_lock(priv);
> +
> + return byte;
> +}
> +
> +static int pci1xxxx_otp_write(void *priv_t, unsigned int off,
> + void *value_t, size_t count)
> +{
> + struct pci1xxxx_otp_eeprom_device *priv = priv_t;
> + char *value = value_t;
> + void __iomem *rb;
> + u32 regval;
> + u32 byte;
> + int ret;
> + u8 data;
> +
> + if (priv != NULL)
> + rb = priv->reg_base;
> + else
> + return -ENODEV;
Same as above.
> +
> + ret = set_sys_lock(priv);
> + if (ret)
> + return ret;
> +
> + for (byte = 0; byte < count; byte++) {
> + otp_device_set_address(priv, (u16)(off + byte));
> +
> + /*
> + * Set OTP_PGM_MODE_BYTE command bit in OTP_PRGM_MODE register
> + * to enable Byte programming
> + */
> + data = readl(rb + MMAP_OTP_OFFSET(OTP_PRGM_MODE_OFFSET));
> + writel(data | OTP_PGM_MODE_BYTE_BIT,
> + rb + MMAP_OTP_OFFSET(OTP_PRGM_MODE_OFFSET));
> + writel(*(value + byte), rb + MMAP_OTP_OFFSET(OTP_PRGM_DATA_OFFSET));
> + data = readl(rb + MMAP_OTP_OFFSET(OTP_FUNC_CMD_OFFSET));
> + writel(data | OTP_FUNC_PGM_BIT,
> + rb + MMAP_OTP_OFFSET(OTP_FUNC_CMD_OFFSET));
> + data = readl(rb + MMAP_OTP_OFFSET(OTP_CMD_GO_OFFSET));
> + writel(data | OTP_CMD_GO_BIT,
> + rb + MMAP_OTP_OFFSET(OTP_CMD_GO_OFFSET));
> +
> + ret = read_poll_timeout(readl, regval,
> + !(regval & OTP_STATUS_BUSY_BIT),
> + STATUS_READ_DELAY_US,
> + STATUS_READ_TIMEOUT_US, true,
> + rb + MMAP_OTP_OFFSET(OTP_STATUS_OFFSET));
> +
> + data = readl(rb + MMAP_OTP_OFFSET(OTP_PASS_FAIL_OFFSET));
> + if (ret < 0 || data & OTP_FAIL_BIT)
> + break;
also here.
> + }
> + release_sys_lock(priv);
> +
> + return byte;
> +}
> +
> +static int pci1xxxx_otp_eeprom_probe(struct auxiliary_device *aux_dev,
> + const struct auxiliary_device_id *id)
> +{
> + struct auxiliary_device_wrapper *aux_dev_wrapper;
> + struct pci1xxxx_otp_eeprom_device *priv;
> + struct gp_aux_data_type *pdata;
> + int ret;
> + u8 data;
> +
> + aux_dev_wrapper = container_of(aux_dev, struct
> auxiliary_device_wrapper,
> + aux_dev);
> + pdata = &aux_dev_wrapper->gp_aux_data;
> + if (!pdata)
> + return dev_err_probe(&aux_dev->dev, -EINVAL,
> + "Invalid data in aux_dev_wrapper->gp_aux_data\n");
> +
> + priv = devm_kzalloc(&aux_dev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return dev_err_probe(&aux_dev->dev, -ENOMEM,
> + "Memory Allocation Failed\n");
Just return -ENOMEM. No need for that message.
-michael
Powered by blists - more mailing lists