| 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
| ||
|
Date: Tue, 30 Aug 2022 13:08:03 +0100
From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To: Horatiu Vultur <horatiu.vultur@...rochip.com>,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
UNGLinuxDriver@...rochip.com
Subject: Re: [PATCH v2 2/2] nvmem: lan9662-otp: add support.
On 25/08/2022 21:40, Horatiu Vultur wrote:
> Add support for OTP controller available on LAN9662. The OTPC controls
> the access to a non-volatile memory. The size of the memory is 8KB.
> The OTPC can access the memory based on an offset.
> Implement both the read and the write functionality.
>
> Signed-off-by: Horatiu Vultur <horatiu.vultur@...rochip.com>
> ---
> drivers/nvmem/Kconfig | 8 ++
> drivers/nvmem/Makefile | 2 +
> drivers/nvmem/lan9662-otpc.c | 249 +++++++++++++++++++++++++++++++++++
> 3 files changed, 259 insertions(+)
> create mode 100644 drivers/nvmem/lan9662-otpc.c
>
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index 967d0084800e..c9929ec35a39 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -84,6 +84,14 @@ config NVMEM_LPC18XX_OTP
> To compile this driver as a module, choose M here: the module
> will be called nvmem_lpc18xx_otp.
>
> +config NVMEM_LAN9662_OTPC
> + tristate "Microchip LAN9662 OTP controller support"
> + depends on SOC_LAN966 || COMPILE_TEST
> + depends on HAS_IOMEM
> + help
> + This driver enables the OTP controller available on Microchip LAN9662
> + SoCs. It controlls the access to the OTP memory connected to it.
> +
s/controlls/controls/
> config NVMEM_MXS_OCOTP
> tristate "Freescale MXS On-Chip OTP Memory Support"
> depends on ARCH_MXS || COMPILE_TEST
> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> index 00e136a0a123..e1baface2c53 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -21,6 +21,8 @@ obj-$(CONFIG_NVMEM_LPC18XX_EEPROM) += nvmem_lpc18xx_eeprom.o
> nvmem_lpc18xx_eeprom-y := lpc18xx_eeprom.o
> obj-$(CONFIG_NVMEM_LPC18XX_OTP) += nvmem_lpc18xx_otp.o
> nvmem_lpc18xx_otp-y := lpc18xx_otp.o
> +obj-$(CONFIG_NVMEM_LAN9662_OTPC) += nvmem-lan9662-otpc.o
> +nvmem-lan9662-otpc-y := lan9662-otpc.o
> obj-$(CONFIG_NVMEM_MXS_OCOTP) += nvmem-mxs-ocotp.o
> nvmem-mxs-ocotp-y := mxs-ocotp.o
> obj-$(CONFIG_NVMEM_NINTENDO_OTP) += nvmem-nintendo-otp.o
> diff --git a/drivers/nvmem/lan9662-otpc.c b/drivers/nvmem/lan9662-otpc.c
> new file mode 100644
> index 000000000000..302a5bae04dc
> --- /dev/null
> +++ b/drivers/nvmem/lan9662-otpc.c
> @@ -0,0 +1,249 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-provider.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#define OTP_OTP_PWR_DN(t) (t + 0x00)
> +#define OTP_OTP_PWR_DN_OTP_PWRDN_N BIT(0)
> +#define OTP_OTP_ADDR_HI(t) (t + 0x04)
> +#define OTP_OTP_ADDR_LO(t) (t + 0x08)
> +#define OTP_OTP_PRGM_DATA(t) (t + 0x10)
> +#define OTP_OTP_PRGM_MODE(t) (t + 0x14)
> +#define OTP_OTP_PRGM_MODE_OTP_PGM_MODE_BYTE BIT(0)
> +#define OTP_OTP_RD_DATA(t) (t + 0x18)
> +#define OTP_OTP_FUNC_CMD(t) (t + 0x20)
> +#define OTP_OTP_FUNC_CMD_OTP_PROGRAM BIT(1)
> +#define OTP_OTP_FUNC_CMD_OTP_READ BIT(0)
> +#define OTP_OTP_CMD_GO(t) (t + 0x28)
> +#define OTP_OTP_CMD_GO_OTP_GO BIT(0)
> +#define OTP_OTP_PASS_FAIL(t) (t + 0x2c)
> +#define OTP_OTP_PASS_FAIL_OTP_READ_PROHIBITED BIT(3)
> +#define OTP_OTP_PASS_FAIL_OTP_WRITE_PROHIBITED BIT(2)
> +#define OTP_OTP_PASS_FAIL_OTP_FAIL BIT(0)
> +#define OTP_OTP_STATUS(t) (t + 0x30)
> +#define OTP_OTP_STATUS_OTP_CPUMPEN BIT(1)
> +#define OTP_OTP_STATUS_OTP_BUSY BIT(0)
> +
> +#define OTP_MEM_SIZE 8192
> +#define OTP_SLEEP_US 10
> +#define OTP_TIMEOUT_US 500000
> +
> +struct lan9662_otp {
> + struct device *dev;
> + void __iomem *base;
> +};
> +
> +static inline void lan9662_writel(void __iomem *addr, u32 val)
> +{
> + writel(val, addr);
> +}
> +
> +static inline u32 lan9662_readl(void __iomem *addr)
> +{
> + return readl(addr);
> +}
> +
Why these boiler plate functions?
> +static inline void lan9662_clrbits(void __iomem *addr, u32 clear)
> +{
> + writel(readl(addr) & ~clear, addr);
> +}
> +
> +static inline void lan9662_setbits(void __iomem *addr, u32 set)
> +{
> + writel(readl(addr) | set, addr);
> +}
These two functions are called just once and I see no point in having a
wrapper function for this, instead you could use them directly or use
./include/linux/bitfield.h helper macros.
> +
> +static bool lan9662_otp_wait_flag_clear(void __iomem *reg, u32 flag)
> +{
> + u32 val;
> +
> + return readl_poll_timeout(reg, val, !(val & flag),
> + OTP_SLEEP_US, OTP_TIMEOUT_US);
> +}
> +
> +static int lan9662_otp_power(struct lan9662_otp *otp, bool up)
> +{
> + if (up) {
> + lan9662_clrbits(OTP_OTP_PWR_DN(otp->base),
> + OTP_OTP_PWR_DN_OTP_PWRDN_N);
> + if (lan9662_otp_wait_flag_clear(OTP_OTP_STATUS(otp->base),
> + OTP_OTP_STATUS_OTP_CPUMPEN))
> + return -ETIMEDOUT;
> + } else {
> + lan9662_setbits(OTP_OTP_PWR_DN(otp->base),
> + OTP_OTP_PWR_DN_OTP_PWRDN_N);
> + }
> +
> + return 0;
> +}
> +
> +static int lan9662_otp_execute(struct lan9662_otp *otp)
> +{
> + if (lan9662_otp_wait_flag_clear(OTP_OTP_CMD_GO(otp->base),
> + OTP_OTP_CMD_GO_OTP_GO))
> + return -ETIMEDOUT;
> +
> + if (lan9662_otp_wait_flag_clear(OTP_OTP_STATUS(otp->base),
> + OTP_OTP_STATUS_OTP_BUSY))
> + return -ETIMEDOUT;
> +
> + return 0;
> +}
> +
> +static void lan9662_otp_set_address(struct lan9662_otp *otp, u32 offset)
> +{
> + WARN_ON(offset >= OTP_MEM_SIZE);
> +
would we ever hit this condition? looks like unecessary check.
> + lan9662_writel(OTP_OTP_ADDR_HI(otp->base), 0xff & (offset >> 8));
> + lan9662_writel(OTP_OTP_ADDR_LO(otp->base), 0xff & offset);
> +}
> +
> +static int lan9662_otp_read_byte(struct lan9662_otp *otp, u32 offset, u8 *dst)
> +{
> + u32 pass;
> + int rc;
> +
> + lan9662_otp_set_address(otp, offset);
> + lan9662_writel(OTP_OTP_FUNC_CMD(otp->base),
> + OTP_OTP_FUNC_CMD_OTP_READ);
> + lan9662_writel(OTP_OTP_CMD_GO(otp->base),
> + OTP_OTP_CMD_GO_OTP_GO);
Can be wrapped into single line.
> + rc = lan9662_otp_execute(otp);
> + if (!rc) {
> + pass = lan9662_readl(OTP_OTP_PASS_FAIL(otp->base));
> + if (pass & OTP_OTP_PASS_FAIL_OTP_READ_PROHIBITED)
> + return -EACCES;
> + *dst = (u8) lan9662_readl(OTP_OTP_RD_DATA(otp->base));
> + }
> + return rc;
> +}
> +
...
thanks,
srini
> +
> +static const struct of_device_id lan9662_otp_match[] = {
> + { .compatible = "microchip,lan9662-otp", },
> + { .compatible = "microchip,lan9668-otp", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, lan9662_otp_match);
> +
> +static struct platform_driver lan9662_otp_driver = {
> + .probe = lan9662_otp_probe,
> + .driver = {
> + .name = "lan9662-otp",
> + .of_match_table = lan9662_otp_match,
> + },
> +};
> +module_platform_driver(lan9662_otp_driver);
> +
> +MODULE_AUTHOR("Horatiu Vultur <horatiu.vultur@...rochip.com>");
> +MODULE_DESCRIPTION("lan9662 OTP driver");
> +MODULE_LICENSE("GPL");
Powered by blists - more mailing lists