All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] nvmem: lan9662-otpc: add support
@ 2022-08-31  6:42 Horatiu Vultur
  2022-08-31  6:42 ` [PATCH v3 1/2] dt-bindings: lan9662-otpc: document Lan9662 OTPC Horatiu Vultur
  2022-08-31  6:42 ` [PATCH v3 2/2] nvmem: lan9662-otp: add support Horatiu Vultur
  0 siblings, 2 replies; 11+ messages in thread
From: Horatiu Vultur @ 2022-08-31  6:42 UTC (permalink / raw)
  To: devicetree, linux-kernel
  Cc: srinivas.kandagatla, robh+dt, krzysztof.kozlowski+dt,
	UNGLinuxDriver, Horatiu Vultur

Add support for lan9662 OTP controller that is available on lan9662.
The driver gives access to non-volatile memory. It allows both write
and read access to it.

v2->v3:
- fix dt-bindings, make sure that make dt_binding_check passes
- remove lan9662_writel/readl/clrbits/setbits and use writel/readl
- remove WARN_ON(offset >= OTP_MEM_SIZE) as it can't happen

v1->v2:
- remove unneed quotes for $ref: nvmem.yaml#
- rename lan966x to lan9662 as not to have any wildcards
- remove compatible string syscon

Horatiu Vultur (2):
  dt-bindings: lan9662-otpc: document Lan9662 OTPC
  nvmem: lan9662-otp: add support.

 .../nvmem/microchip,lan9662-otpc.yaml         |  45 ++++
 drivers/nvmem/Kconfig                         |   8 +
 drivers/nvmem/Makefile                        |   2 +
 drivers/nvmem/lan9662-otpc.c                  | 223 ++++++++++++++++++
 4 files changed, 278 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/microchip,lan9662-otpc.yaml
 create mode 100644 drivers/nvmem/lan9662-otpc.c

-- 
2.33.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/2] dt-bindings: lan9662-otpc: document Lan9662 OTPC
  2022-08-31  6:42 [PATCH v3 0/2] nvmem: lan9662-otpc: add support Horatiu Vultur
@ 2022-08-31  6:42 ` Horatiu Vultur
  2022-08-31  7:28   ` Krzysztof Kozlowski
  2022-08-31  6:42 ` [PATCH v3 2/2] nvmem: lan9662-otp: add support Horatiu Vultur
  1 sibling, 1 reply; 11+ messages in thread
From: Horatiu Vultur @ 2022-08-31  6:42 UTC (permalink / raw)
  To: devicetree, linux-kernel
  Cc: srinivas.kandagatla, robh+dt, krzysztof.kozlowski+dt,
	UNGLinuxDriver, Horatiu Vultur

Document Lan9662 OTP controller.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 .../nvmem/microchip,lan9662-otpc.yaml         | 45 +++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/microchip,lan9662-otpc.yaml

diff --git a/Documentation/devicetree/bindings/nvmem/microchip,lan9662-otpc.yaml b/Documentation/devicetree/bindings/nvmem/microchip,lan9662-otpc.yaml
new file mode 100644
index 000000000000..def20733cf60
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/microchip,lan9662-otpc.yaml
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/nvmem/microchip,lan9662-otpc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip LAN9662 OTP Controller (OTPC)
+
+maintainers:
+  - Horatiu Vultur <horatiu.vultur@microchip.com>
+
+description: |
+  OTP controller drives a NVMEM memory where system specific data
+  (e.g. hardware configuration settings, chip identifiers) or
+  user specific data could be stored.
+
+allOf:
+  - $ref: nvmem.yaml#
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - const: microchip,lan9662-otpc
+          - const: microchip,lan9668-otpc
+      - enum:
+          - microchip,lan9662-otpc
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    otpc: otp@e0021000 {
+        compatible = "microchip,lan9662-otpc";
+        reg = <0xe0021000 0x300>;
+    };
+
+...
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/2] nvmem: lan9662-otp: add support.
  2022-08-31  6:42 [PATCH v3 0/2] nvmem: lan9662-otpc: add support Horatiu Vultur
  2022-08-31  6:42 ` [PATCH v3 1/2] dt-bindings: lan9662-otpc: document Lan9662 OTPC Horatiu Vultur
@ 2022-08-31  6:42 ` Horatiu Vultur
  2022-08-31  7:29   ` Krzysztof Kozlowski
  1 sibling, 1 reply; 11+ messages in thread
From: Horatiu Vultur @ 2022-08-31  6:42 UTC (permalink / raw)
  To: devicetree, linux-kernel
  Cc: srinivas.kandagatla, robh+dt, krzysztof.kozlowski+dt,
	UNGLinuxDriver, Horatiu Vultur

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@microchip.com>
---
 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 <horatiu.vultur@microchip.com>");
+MODULE_DESCRIPTION("lan9662 OTP driver");
+MODULE_LICENSE("GPL");
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] dt-bindings: lan9662-otpc: document Lan9662 OTPC
  2022-08-31  6:42 ` [PATCH v3 1/2] dt-bindings: lan9662-otpc: document Lan9662 OTPC Horatiu Vultur
@ 2022-08-31  7:28   ` Krzysztof Kozlowski
  2022-08-31 10:44     ` Horatiu Vultur
  0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-31  7:28 UTC (permalink / raw)
  To: Horatiu Vultur, devicetree, linux-kernel
  Cc: srinivas.kandagatla, robh+dt, krzysztof.kozlowski+dt, UNGLinuxDriver

On 31/08/2022 09:42, Horatiu Vultur wrote:
> Document Lan9662 OTP controller.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  .../nvmem/microchip,lan9662-otpc.yaml         | 45 +++++++++++++++++++
>  1 file changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/microchip,lan9662-otpc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/microchip,lan9662-otpc.yaml b/Documentation/devicetree/bindings/nvmem/microchip,lan9662-otpc.yaml
> new file mode 100644
> index 000000000000..def20733cf60
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/microchip,lan9662-otpc.yaml
> @@ -0,0 +1,45 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/nvmem/microchip,lan9662-otpc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip LAN9662 OTP Controller (OTPC)
> +
> +maintainers:
> +  - Horatiu Vultur <horatiu.vultur@microchip.com>
> +
> +description: |
> +  OTP controller drives a NVMEM memory where system specific data
> +  (e.g. hardware configuration settings, chip identifiers) or
> +  user specific data could be stored.
> +
> +allOf:
> +  - $ref: nvmem.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - const: microchip,lan9662-otpc
> +          - const: microchip,lan9668-otpc
> +      - enum:
> +          - microchip,lan9662-otpc

This is not what I wrote and this does not make sense. You now listed
twice 9662 and 9668 does not have its entry.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] nvmem: lan9662-otp: add support.
  2022-08-31  6:42 ` [PATCH v3 2/2] nvmem: lan9662-otp: add support Horatiu Vultur
@ 2022-08-31  7:29   ` Krzysztof Kozlowski
  2022-08-31 10:52     ` Horatiu Vultur
  0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-31  7:29 UTC (permalink / raw)
  To: Horatiu Vultur, devicetree, linux-kernel
  Cc: srinivas.kandagatla, robh+dt, krzysztof.kozlowski+dt, UNGLinuxDriver

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] dt-bindings: lan9662-otpc: document Lan9662 OTPC
  2022-08-31  7:28   ` Krzysztof Kozlowski
@ 2022-08-31 10:44     ` Horatiu Vultur
  2022-08-31 12:45       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 11+ messages in thread
From: Horatiu Vultur @ 2022-08-31 10:44 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: devicetree, linux-kernel, srinivas.kandagatla, robh+dt,
	krzysztof.kozlowski+dt, UNGLinuxDriver

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

Hi Krzysztof,

> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - items:
> > +          - const: microchip,lan9662-otpc
> > +          - const: microchip,lan9668-otpc
> > +      - enum:
> > +          - microchip,lan9662-otpc
> 
> This is not what I wrote and this does not make sense. You now listed
> twice 9662 and 9668 does not have its entry.

As you figured it out, I am quite noob at these bindings.
The only difference between what you wrote and what I wrote is the order
under items. So the order matters?

> 
> Best regards,
> Krzysztof

-- 
/Horatiu

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] nvmem: lan9662-otp: add support.
  2022-08-31  7:29   ` Krzysztof Kozlowski
@ 2022-08-31 10:52     ` Horatiu Vultur
  2022-08-31 12:52       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 11+ messages in thread
From: Horatiu Vultur @ 2022-08-31 10:52 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: devicetree, linux-kernel, srinivas.kandagatla, robh+dt,
	krzysztof.kozlowski+dt, UNGLinuxDriver

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] dt-bindings: lan9662-otpc: document Lan9662 OTPC
  2022-08-31 10:44     ` Horatiu Vultur
@ 2022-08-31 12:45       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-31 12:45 UTC (permalink / raw)
  To: Horatiu Vultur
  Cc: devicetree, linux-kernel, srinivas.kandagatla, robh+dt,
	krzysztof.kozlowski+dt, UNGLinuxDriver

On 31/08/2022 13:44, Horatiu Vultur wrote:
> The 08/31/2022 10:28, Krzysztof Kozlowski wrote:
> 
> Hi Krzysztof,
> 
>>> +
>>> +properties:
>>> +  compatible:
>>> +    oneOf:
>>> +      - items:
>>> +          - const: microchip,lan9662-otpc
>>> +          - const: microchip,lan9668-otpc
>>> +      - enum:
>>> +          - microchip,lan9662-otpc
>>
>> This is not what I wrote and this does not make sense. You now listed
>> twice 9662 and 9668 does not have its entry.
> 
> As you figured it out, I am quite noob at these bindings.
> The only difference between what you wrote and what I wrote is the order
> under items. So the order matters?

Yes, because it is not an enum but list. What you wrote is:
compatible = "microchip,lan9662-otpc", "microchip,lan9668-otpc"

and you would see the problems if you tested it with lan9668.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] nvmem: lan9662-otp: add support.
  2022-08-31 10:52     ` Horatiu Vultur
@ 2022-08-31 12:52       ` Krzysztof Kozlowski
  2022-08-31 14:52         ` Horatiu Vultur
  0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-31 12:52 UTC (permalink / raw)
  To: Horatiu Vultur
  Cc: devicetree, linux-kernel, srinivas.kandagatla, robh+dt,
	krzysztof.kozlowski+dt, UNGLinuxDriver

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] nvmem: lan9662-otp: add support.
  2022-08-31 12:52       ` Krzysztof Kozlowski
@ 2022-08-31 14:52         ` Horatiu Vultur
  2022-08-31 15:17           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 11+ messages in thread
From: Horatiu Vultur @ 2022-08-31 14:52 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: devicetree, linux-kernel, srinivas.kandagatla, robh+dt,
	krzysztof.kozlowski+dt, UNGLinuxDriver

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] nvmem: lan9662-otp: add support.
  2022-08-31 14:52         ` Horatiu Vultur
@ 2022-08-31 15:17           ` Krzysztof Kozlowski
  0 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-31 15:17 UTC (permalink / raw)
  To: Horatiu Vultur
  Cc: devicetree, linux-kernel, srinivas.kandagatla, robh+dt,
	krzysztof.kozlowski+dt, UNGLinuxDriver

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-08-31 15:17 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-31  6:42 [PATCH v3 0/2] nvmem: lan9662-otpc: add support Horatiu Vultur
2022-08-31  6:42 ` [PATCH v3 1/2] dt-bindings: lan9662-otpc: document Lan9662 OTPC Horatiu Vultur
2022-08-31  7:28   ` Krzysztof Kozlowski
2022-08-31 10:44     ` Horatiu Vultur
2022-08-31 12:45       ` Krzysztof Kozlowski
2022-08-31  6:42 ` [PATCH v3 2/2] nvmem: lan9662-otp: add support Horatiu Vultur
2022-08-31  7:29   ` Krzysztof Kozlowski
2022-08-31 10:52     ` Horatiu Vultur
2022-08-31 12:52       ` Krzysztof Kozlowski
2022-08-31 14:52         ` Horatiu Vultur
2022-08-31 15:17           ` Krzysztof Kozlowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.