2022-08-31 07:07:31

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH v3 2/2] nvmem: lan9662-otp: add support.

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 <[email protected]>
---
drivers/nvmem/Kconfig | 8 ++
drivers/nvmem/Makefile | 2 +
drivers/nvmem/lan9662-otpc.c | 223 +++++++++++++++++++++++++++++++++++
3 files changed, 233 insertions(+)
create mode 100644 drivers/nvmem/lan9662-otpc.c

diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index 967d0084800e..9399445a2f4c 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 controls the access to the OTP memory connected to it.
+
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..655cc3bb6f53
--- /dev/null
+++ b/drivers/nvmem/lan9662-otpc.c
@@ -0,0 +1,223 @@
+// 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 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)
+{
+ void __iomem *pwrdn = OTP_OTP_PWR_DN(otp->base);
+
+ if (up) {
+ writel(readl(pwrdn) & ~OTP_OTP_PWR_DN_OTP_PWRDN_N, pwrdn);
+ if (lan9662_otp_wait_flag_clear(OTP_OTP_STATUS(otp->base),
+ OTP_OTP_STATUS_OTP_CPUMPEN))
+ return -ETIMEDOUT;
+ } else {
+ writel(readl(pwrdn) | OTP_OTP_PWR_DN_OTP_PWRDN_N, pwrdn);
+ }
+
+ 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)
+{
+ writel(0xff & (offset >> 8), OTP_OTP_ADDR_HI(otp->base));
+ writel(0xff & offset, OTP_OTP_ADDR_LO(otp->base));
+}
+
+static int lan9662_otp_read_byte(struct lan9662_otp *otp, u32 offset, u8 *dst)
+{
+ u32 pass;
+ int rc;
+
+ lan9662_otp_set_address(otp, offset);
+ writel(OTP_OTP_FUNC_CMD_OTP_READ, OTP_OTP_FUNC_CMD(otp->base));
+ writel(OTP_OTP_CMD_GO_OTP_GO, OTP_OTP_CMD_GO(otp->base));
+ rc = lan9662_otp_execute(otp);
+ if (!rc) {
+ pass = readl(OTP_OTP_PASS_FAIL(otp->base));
+ if (pass & OTP_OTP_PASS_FAIL_OTP_READ_PROHIBITED)
+ return -EACCES;
+ *dst = (u8) readl(OTP_OTP_RD_DATA(otp->base));
+ }
+ return rc;
+}
+
+static int lan9662_otp_write_byte(struct lan9662_otp *otp, u32 offset, u8 data)
+{
+ u32 pass;
+ int rc;
+
+ lan9662_otp_set_address(otp, offset);
+ writel(OTP_OTP_PRGM_MODE_OTP_PGM_MODE_BYTE, OTP_OTP_PRGM_MODE(otp->base));
+ writel(data, OTP_OTP_PRGM_DATA(otp->base));
+ writel(OTP_OTP_FUNC_CMD_OTP_PROGRAM, OTP_OTP_FUNC_CMD(otp->base));
+ writel(OTP_OTP_CMD_GO_OTP_GO, OTP_OTP_CMD_GO(otp->base));
+
+ rc = lan9662_otp_execute(otp);
+ if (!rc) {
+ pass = readl(OTP_OTP_PASS_FAIL(otp->base));
+ if (pass & OTP_OTP_PASS_FAIL_OTP_WRITE_PROHIBITED)
+ return -EACCES;
+ if (pass & OTP_OTP_PASS_FAIL_OTP_FAIL)
+ return -EIO;
+ }
+ return rc;
+}
+
+static int lan9662_otp_read(void *context, unsigned int offset,
+ void *_val, size_t bytes)
+{
+ struct lan9662_otp *otp = context;
+ u8 *val = _val;
+ uint8_t data;
+ int i, rc = 0;
+
+ lan9662_otp_power(otp, true);
+ for (i = 0; i < bytes; i++) {
+ rc = lan9662_otp_read_byte(otp, offset + i, &data);
+ if (rc < 0)
+ break;
+ *val++ = data;
+ }
+ lan9662_otp_power(otp, false);
+
+ return rc;
+}
+
+static int lan9662_otp_write(void *context, unsigned int offset,
+ void *_val, size_t bytes)
+{
+ struct lan9662_otp *otp = context;
+ u8 *val = _val;
+ u8 data, newdata;
+ int i, rc = 0;
+
+ lan9662_otp_power(otp, true);
+ for (i = 0; i < bytes; i++) {
+ /* Skip zero bytes */
+ if (val[i]) {
+ rc = lan9662_otp_read_byte(otp, offset + i, &data);
+ if (rc < 0)
+ break;
+
+ newdata = data | val[i];
+ if (newdata == data)
+ continue;
+
+ rc = lan9662_otp_write_byte(otp, offset + i,
+ newdata);
+ if (rc < 0)
+ break;
+ }
+ }
+ lan9662_otp_power(otp, false);
+
+ return rc;
+}
+
+static struct nvmem_config otp_config = {
+ .name = "lan9662-otp",
+ .stride = 1,
+ .word_size = 1,
+ .reg_read = lan9662_otp_read,
+ .reg_write = lan9662_otp_write,
+ .size = OTP_MEM_SIZE,
+};
+
+static int lan9662_otp_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct nvmem_device *nvmem;
+ struct lan9662_otp *otp;
+
+ otp = devm_kzalloc(&pdev->dev, sizeof(*otp), GFP_KERNEL);
+ if (!otp)
+ return -ENOMEM;
+
+ otp->dev = dev;
+ otp->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(otp->base))
+ return PTR_ERR(otp->base);
+
+ otp_config.priv = otp;
+ otp_config.dev = dev;
+
+ nvmem = devm_nvmem_register(dev, &otp_config);
+
+ return PTR_ERR_OR_ZERO(nvmem);
+}
+
+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 <[email protected]>");
+MODULE_DESCRIPTION("lan9662 OTP driver");
+MODULE_LICENSE("GPL");
--
2.33.0


2022-08-31 07:32:49

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] nvmem: lan9662-otp: add support.

On 31/08/2022 09:42, Horatiu Vultur wrote:

> +static const struct of_device_id lan9662_otp_match[] = {
> + { .compatible = "microchip,lan9662-otp", },
> + { .compatible = "microchip,lan9668-otp", },

This is still wrong, does not match your bindings at all and still
duplicates entries without driver data. One entry - 9662.

Best regards,
Krzysztof

2022-08-31 11:05:56

by Horatiu Vultur

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] nvmem: lan9662-otp: add support.

The 08/31/2022 10:29, Krzysztof Kozlowski wrote:

Hi Krzysztof,

>
> On 31/08/2022 09:42, Horatiu Vultur wrote:
>
> > +static const struct of_device_id lan9662_otp_match[] = {
> > + { .compatible = "microchip,lan9662-otp", },
> > + { .compatible = "microchip,lan9668-otp", },
>
> This is still wrong, does not match your bindings at all and still
> duplicates entries without driver data. One entry - 9662.

I have look at some other drivers, where I can see they don't have any
driver data. For example [1] and the bindings are here [2].

[1] https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/ti/cpsw_new.c#L1832
[2] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml#L23

Is this also wrong, or I still can't understand how the bindings are
working?

If I put only one entry:
---
static const struct of_device_id lan9662_otp_match[] = {
{ .compatible = "microchip,lan9662-otp", },
---

Wouldn't be a problem that the binding mentions also lan9668?

>
> Best regards,
> Krzysztof

--
/Horatiu

2022-08-31 13:31:03

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] nvmem: lan9662-otp: add support.

On 31/08/2022 13:52, Horatiu Vultur wrote:
> The 08/31/2022 10:29, Krzysztof Kozlowski wrote:
>
> Hi Krzysztof,
>
>>
>> On 31/08/2022 09:42, Horatiu Vultur wrote:
>>
>>> +static const struct of_device_id lan9662_otp_match[] = {
>>> + { .compatible = "microchip,lan9662-otp", },
>>> + { .compatible = "microchip,lan9668-otp", },
>>
>> This is still wrong, does not match your bindings at all and still
>> duplicates entries without driver data. One entry - 9662.
>
> I have look at some other drivers, where I can see they don't have any
> driver data. For example [1] and the bindings are here [2].
>
> [1] https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/ti/cpsw_new.c#L1832
> [2] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml#L23

There are plenty of poor examples in Linux kernel code and it is not a
reason to re-use their patterns...

> Is this also wrong, or I still can't understand how the bindings are
> working?

The topic here is not that much related to the bindings, but device
matching in Linux kernel.

>
> If I put only one entry:
> ---
> static const struct of_device_id lan9662_otp_match[] = {
> { .compatible = "microchip,lan9662-otp", },
> ---
>
> Wouldn't be a problem that the binding mentions also lan9668?

No. What could be the problem exactly, which you are afraid? Why
implementation should be a problem for a binding (which we try to be
mostly implementation independent)?

Best regards,
Krzysztof

2022-08-31 14:59:27

by Horatiu Vultur

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] nvmem: lan9662-otp: add support.

The 08/31/2022 15:52, Krzysztof Kozlowski wrote:
>
> On 31/08/2022 13:52, Horatiu Vultur wrote:
> > The 08/31/2022 10:29, Krzysztof Kozlowski wrote:
> >
> > Hi Krzysztof,
> >
> >>
> >> On 31/08/2022 09:42, Horatiu Vultur wrote:
> >>
> >>> +static const struct of_device_id lan9662_otp_match[] = {
> >>> + { .compatible = "microchip,lan9662-otp", },
> >>> + { .compatible = "microchip,lan9668-otp", },
> >>
> >> This is still wrong, does not match your bindings at all and still
> >> duplicates entries without driver data. One entry - 9662.
> >
> > I have look at some other drivers, where I can see they don't have any
> > driver data. For example [1] and the bindings are here [2].
> >
> > [1] https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/ti/cpsw_new.c#L1832
> > [2] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml#L23
>
> There are plenty of poor examples in Linux kernel code and it is not a
> reason to re-use their patterns...
>
> > Is this also wrong, or I still can't understand how the bindings are
> > working?
>
> The topic here is not that much related to the bindings, but device
> matching in Linux kernel.
>
> >
> > If I put only one entry:
> > ---
> > static const struct of_device_id lan9662_otp_match[] = {
> > { .compatible = "microchip,lan9662-otp", },
> > ---
> >
> > Wouldn't be a problem that the binding mentions also lan9668?
>
> No. What could be the problem exactly, which you are afraid? Why
> implementation should be a problem for a binding (which we try to be
> mostly implementation independent)?

The implementation wouldn't be a problem for the binding.
The only thing was if the binding has more compatible strings than what
the driver supports. As an example, in the binding we mention about
lan9668 but nothing in the driver.

>
> Best regards,
> Krzysztof

--
/Horatiu

2022-08-31 15:42:04

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] nvmem: lan9662-otp: add support.

On 31/08/2022 17:52, Horatiu Vultur wrote:
>>> If I put only one entry:
>>> ---
>>> static const struct of_device_id lan9662_otp_match[] = {
>>> { .compatible = "microchip,lan9662-otp", },
>>> ---
>>>
>>> Wouldn't be a problem that the binding mentions also lan9668?
>>
>> No. What could be the problem exactly, which you are afraid? Why
>> implementation should be a problem for a binding (which we try to be
>> mostly implementation independent)?
>
> The implementation wouldn't be a problem for the binding.
> The only thing was if the binding has more compatible strings than what
> the driver supports. As an example, in the binding we mention about
> lan9668 but nothing in the driver.

What is that "only thing"? What is the problem? It does not matter
really if the driver mentions or does not mention lan9668, because it's
not related...

Best regards,
Krzysztof