Add support for NXP LPC18xx EEPROM using nvmem

Add the devicetree binding document for NXP LPC18xx EEPROM memory.

Signed-off-by Ariel D'Alessandro <[email protected]>

---
 .../devicetree/bindings/nvmem/lpc1857-eeprom.txt   | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/lpc1857-eeprom.txt

Documentation/devicetree/bindings/nvmem/lpc1857-eeprom.txt new file +3 -0

new file mode 100644
index 0000000..01cde0d
@@ -0,0 +1,26 @@
+* NXP LPC18xx EEPROM memory NVMEM driver
+
+Required properties:

Hi Ariel,

Quoted 21 lines · Ariel D'Alessandro

Add the devicetree binding document for NXP LPC18xx EEPROM memory.

Signed-off-by Ariel D'Alessandro <[email protected]>

---
 .../devicetree/bindings/nvmem/lpc1857-eeprom.txt   | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/lpc1857-eeprom.txt

Documentation/devicetree/bindings/nvmem/lpc1857-eeprom.txt new file +7 -0

new file mode 100644
index 0000000..01cde0d
@@ -0,0 +1,26 @@
+* NXP LPC18xx EEPROM memory NVMEM driver
+
+Required properties:
+  - compatible: Should be "nxp,lpc1857-eeprom"
+  - reg: Must contain an entry with the physical base address and length
+    for each entry in reg-names.
+  - reg-names: Must include the following entries.

Documentation/devicetree/bindings/nvmem/lpc1857-eeprom.txt line 13

Quoted 6 lines

Documentation/devicetree/bindings/nvmem/lpc1857-eeprom.txt new file +6 -0

+    - reg: EEPROM registers.
+    - mem: EEPROM address space.
+  - clocks: Must contain an entry for each entry in clock-names.
+  - clock-names: Must include the following entries.
+    - eeprom: EEPROM operating clock.
+  - interrupts: Should contain EEPROM interrupt.

You should document the 'reset' property as well.

Documentation/devicetree/bindings/nvmem/lpc1857-eeprom.txt line 26

Quoted 13 lines

Documentation/devicetree/bindings/nvmem/lpc1857-eeprom.txt new file +12 -0

+Example:
+
+  eeprom: eeprom at 4000e000 {
+    compatible = "nxp,lpc1857-eeprom";
+    reg = <0x4000e000 0x1000>,
+          <0x20040000 0x4000>;
+    reg-names = "reg", "mem";
+    clocks = <&ccu1 CLK_CPU_EEPROM>;
+    clock-names = "eeprom";
+    resets = <&rgu 27>;
+    interrupts = <4>;
+  };

Other than that this looks good to me.

Acked-by Joachim Eastwood <[email protected]>

Add the devicetree binding document for NXP LPC18xx EEPROM memory.

Signed-off-by Ariel D'Alessandro <[email protected]>

---
 .../devicetree/bindings/nvmem/lpc1857-eeprom.txt   | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/lpc1857-eeprom.txt

Documentation/devicetree/bindings/nvmem/lpc1857-eeprom.txt new file +26 -1

new file mode 100644
index 0000000..01cde0d
@@ -0,0 +1,26 @@
+* NXP LPC18xx EEPROM memory NVMEM driver
+
+Required properties:
+  - compatible: Should be "nxp,lpc1857-eeprom"
+  - reg: Must contain an entry with the physical base address and length
+    for each entry in reg-names.
+  - reg-names: Must include the following entries.
+    - reg: EEPROM registers.
+    - mem: EEPROM address space.
+  - clocks: Must contain an entry for each entry in clock-names.
+  - clock-names: Must include the following entries.
+    - eeprom: EEPROM operating clock.
+  - interrupts: Should contain EEPROM interrupt.
+
+Example:
+
+  eeprom: eeprom at 4000e000 {
+    compatible = "nxp,lpc1857-eeprom";
+    reg = <0x4000e000 0x1000>,
+          <0x20040000 0x4000>;
+    reg-names = "reg", "mem";
+    clocks = <&ccu1 CLK_CPU_EEPROM>;
+    clock-names = "eeprom";
+    resets = <&rgu 27>;
+    interrupts = <4>;
+  };
--

2.6.1

This commit adds support for NXP LPC18xx EEPROM memory found in NXP LPC185x/3x and LPC435x/3x/2x/1x devices.

EEPROM size is 16384 bytes and it can be entirely read and written/erased with 1 word (4 bytes) granularity. The last page (128 bytes) contains the EEPROM initialization data and is not writable.

Erase/program time is less than 3ms. The EEPROM device requires a ~1500 kHz clock (min 800 kHz, max 1600 kHz) that is generated dividing the system bus clock by the division factor, contained in the divider register (minus 1 encoded).

Signed-off-by Ariel D'Alessandro <[email protected]>

---
 drivers/nvmem/Kconfig          |   9 ++
 drivers/nvmem/Makefile         |   2 +
 drivers/nvmem/lpc18xx_eeprom.c | 266 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 277 insertions(+)
 create mode 100644 drivers/nvmem/lpc18xx_eeprom.c

drivers/nvmem/lpc18xx_eeprom.c new file +5 -0

+static int lpc18xx_eeprom_gather_write(void *context, const void *reg,
+                                      size_t reg_size, const void *val,
+                                      size_t val_size)
+{
+       struct lpc18xx_eeprom_dev *eeprom = context;

drivers/nvmem/lpc18xx_eeprom.c new file +5 -0

+static int lpc18xx_eeprom_read(void *context, const void *reg, size_t reg_size,
+                              void *val, size_t val_size)
+{
+       struct lpc18xx_eeprom_dev *eeprom = context;
+       unsigned int offset = *(u32 *)reg;

drivers/nvmem/lpc18xx_eeprom.c new file +48 -0

+       /*
+        * Clock rate is generated by dividing the system bus clock by the
+        * division factor, contained in the divider register (minus 1 encoded).
+        */
+       clk_rate = clk_get_rate(eeprom->clk);
+       clk_rate = DIV_ROUND_UP(clk_rate, LPC18XX_EEPROM_CLOCK_HZ) - 1;
+       lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_CLKDIV, clk_rate);
+
+       /*
+        * Writing a single word to the page will start the erase/program cycle
+        * automatically
+        */
+       lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_AUTOPROG,
+                             LPC18XX_EEPROM_AUTOPROG_WORD);
+
+       lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_PWRDWN,
+                             LPC18XX_EEPROM_PWRDWN_NO);
+
+       lpc18xx_regmap_config.max_register = resource_size(res) - 1;
+       lpc18xx_regmap_config.writeable_reg = lpc18xx_eeprom_writeable_reg;
+       lpc18xx_regmap_config.readable_reg = lpc18xx_eeprom_readable_reg;
+
+       regmap = devm_regmap_init(dev, &lpc18xx_eeprom_bus, eeprom,
+                                 &lpc18xx_regmap_config);
+       if (IS_ERR(regmap)) {
+               dev_err(dev, "regmap init failed: %ld\n", PTR_ERR(regmap));
+               ret = PTR_ERR(regmap);
+               goto err_clk;
+       }
+
+       lpc18xx_nvmem_config.dev = dev;
+
+       eeprom->nvmem = nvmem_register(&lpc18xx_nvmem_config);
+       if (IS_ERR(eeprom->nvmem)) {
+               ret = PTR_ERR(eeprom->nvmem);
+               goto err_clk;
+       }
+
+       platform_set_drvdata(pdev, eeprom);
+
+       return 0;
+
+err_clk:
+       clk_disable_unprepare(eeprom->clk);
+
+       return ret;
+}
+

Hi Ariel,

Quoted 25 lines · Ariel D'Alessandro

This commit adds support for NXP LPC18xx EEPROM memory found in NXP LPC185x/3x and LPC435x/3x/2x/1x devices.

EEPROM size is 16384 bytes and it can be entirely read and written/erased with 1 word (4 bytes) granularity. The last page (128 bytes) contains the EEPROM initialization data and is not writable.

Erase/program time is less than 3ms. The EEPROM device requires a ~1500 kHz clock (min 800 kHz, max 1600 kHz) that is generated dividing the system bus clock by the division factor, contained in the divider register (minus 1 encoded).

Signed-off-by Ariel D'Alessandro <[email protected]>

---
  drivers/nvmem/Kconfig          |   9 ++
  drivers/nvmem/Makefile         |   2 +
  drivers/nvmem/lpc18xx_eeprom.c | 266 +++++++++++++++++++++++++++++++++++++++++
  3 files changed, 277 insertions(+)

create mode 100644 drivers/nvmem/lpc18xx_eeprom.c

(quoted diff) +5 -0

+static int lpc18xx_eeprom_gather_write(void *context, const void *reg,
+                                      size_t reg_size, const void *val,
+                                      size_t val_size)
+{
+       struct lpc18xx_eeprom_dev *eeprom = context;

Review

Quoted 10 lines

(quoted diff) +10 -0

+       unsigned int offset = *(u32 *)reg;
+
+       /* 3 ms of erase/program time between each writing */
+       while (val_size) {
+               writel(*(u32 *)val, eeprom->mem_base + offset);
+               usleep_range(3000, 4000);
+               val_size -= eeprom->val_bytes;
+               val += eeprom->val_bytes;
+               offset += eeprom->val_bytes;
+       }

What happens here if 'val_size' is less than 4 or not dividable by 4? Same thing for 'offset'.

I tested the driver from sysfs by writing strings into the nvmem-file with echo. Writing a string not dividable by 4 seems to hang the system.

Quoted 6 lines

(quoted diff) +5 -0

+static int lpc18xx_eeprom_read(void *context, const void *reg, size_t reg_size,
+                              void *val, size_t val_size)
+{
+       struct lpc18xx_eeprom_dev *eeprom = context;
+       unsigned int offset = *(u32 *)reg;

Review

Quoted 10 lines

(quoted diff) +10 -0

+
+       while (val_size) {
+               *(u32 *)val = readl(eeprom->mem_base + offset);
+               val_size -= eeprom->val_bytes;
+               val += eeprom->val_bytes;
+               offset += eeprom->val_bytes;
+       }
+
+       return 0;
+}

Same comments as for lpc18xx_eeprom_gather_write().

Review

Quoted 5 lines

(quoted diff) +5 -0

+static const struct of_device_id lpc18xx_eeprom_of_match[] = {
+       { .compatible = "nxp,lpc1857-eeprom" },
+       { },
+};
+MODULE_DEVICE_TABLE(of, lpc18xx_eeprom_of_match);

nit: It's usual to place of_device_id struct just above the platform_driver struct.

Review

Quoted 2 lines

(quoted diff) +2 -0

+       eeprom->val_bytes = lpc18xx_regmap_config.val_bits / 8;
+       eeprom->reg_bytes = lpc18xx_regmap_config.reg_bits / 8;

There is a BITS_PER_BYTE define in bitops.h that you might want to use here.

Quoted 48 lines

(quoted diff) +48 -0

+       /*
+        * Clock rate is generated by dividing the system bus clock by the
+        * division factor, contained in the divider register (minus 1 encoded).
+        */
+       clk_rate = clk_get_rate(eeprom->clk);
+       clk_rate = DIV_ROUND_UP(clk_rate, LPC18XX_EEPROM_CLOCK_HZ) - 1;
+       lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_CLKDIV, clk_rate);
+
+       /*
+        * Writing a single word to the page will start the erase/program cycle
+        * automatically
+        */
+       lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_AUTOPROG,
+                             LPC18XX_EEPROM_AUTOPROG_WORD);
+
+       lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_PWRDWN,
+                             LPC18XX_EEPROM_PWRDWN_NO);
+
+       lpc18xx_regmap_config.max_register = resource_size(res) - 1;
+       lpc18xx_regmap_config.writeable_reg = lpc18xx_eeprom_writeable_reg;
+       lpc18xx_regmap_config.readable_reg = lpc18xx_eeprom_readable_reg;
+
+       regmap = devm_regmap_init(dev, &lpc18xx_eeprom_bus, eeprom,
+                                 &lpc18xx_regmap_config);
+       if (IS_ERR(regmap)) {
+               dev_err(dev, "regmap init failed: %ld\n", PTR_ERR(regmap));
+               ret = PTR_ERR(regmap);
+               goto err_clk;
+       }
+
+       lpc18xx_nvmem_config.dev = dev;
+
+       eeprom->nvmem = nvmem_register(&lpc18xx_nvmem_config);
+       if (IS_ERR(eeprom->nvmem)) {
+               ret = PTR_ERR(eeprom->nvmem);
+               goto err_clk;
+       }
+
+       platform_set_drvdata(pdev, eeprom);
+
+       return 0;
+
+err_clk:
+       clk_disable_unprepare(eeprom->clk);
+
+       return ret;
+}
+

Review

Quoted 10 lines

(quoted diff) +10 -0

+static int lpc18xx_eeprom_remove(struct platform_device *pdev)
+{
+       struct lpc18xx_eeprom_dev *eeprom = platform_get_drvdata(pdev);
+
+       lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_PWRDWN,
+                             LPC18XX_EEPROM_PWRDWN_YES);
+
+       clk_disable_unprepare(eeprom->clk);
+
+       return nvmem_unregister(eeprom->nvmem);

Normally you do tear down in the reverse order of initialization.

Consider what happens here when you power down and disable the clock while there still are nvmem users of the eeprom.

regards,
Joachim Eastwood

Quoted 44 lines

Hi Ariel,

Quoted 26 lines · Ariel D'Alessandro

This commit adds support for NXP LPC18xx EEPROM memory found in NXP LPC185x/3x and LPC435x/3x/2x/1x devices.

EEPROM size is 16384 bytes and it can be entirely read and written/erased with 1 word (4 bytes) granularity. The last page (128 bytes) contains the EEPROM initialization data and is not writable.

Erase/program time is less than 3ms. The EEPROM device requires a ~1500 kHz clock (min 800 kHz, max 1600 kHz) that is generated dividing the system bus clock by the division factor, contained in the divider register (minus 1 encoded).

Signed-off-by Ariel D'Alessandro <[email protected]>

---
  drivers/nvmem/Kconfig          |   9 ++
  drivers/nvmem/Makefile         |   2 +
  drivers/nvmem/lpc18xx_eeprom.c | 266
+++++++++++++++++++++++++++++++++++++++++
  3 files changed, 277 insertions(+)

create mode 100644 drivers/nvmem/lpc18xx_eeprom.c

(quoted diff) +5 -0

+static int lpc18xx_eeprom_gather_write(void *context, const void *reg,
+                                      size_t reg_size, const void *val,
+                                      size_t val_size)
+{
+       struct lpc18xx_eeprom_dev *eeprom = context;

Review

Quoted 10 lines

(quoted diff) +10 -0

+       unsigned int offset = *(u32 *)reg;
+
+       /* 3 ms of erase/program time between each writing */
+       while (val_size) {
+               writel(*(u32 *)val, eeprom->mem_base + offset);
+               usleep_range(3000, 4000);
+               val_size -= eeprom->val_bytes;
+               val += eeprom->val_bytes;
+               offset += eeprom->val_bytes;
+       }

What happens here if 'val_size' is less than 4 or not dividable by 4? Same thing for 'offset'.

Quoted 4 lines

I tested the driver from sysfs by writing strings into the nvmem-file with echo. Writing a string not dividable by 4 seems to hang the system.

I think I know the issue here:
Could you try this patch:

-------------------------------->cut<---------------------------------- From 8cae10eff8ea8da9c5a8058ff75abeeddd8a8224 Mon Sep 17 00:00:00 2001
From: Srinivas Kandagatla <[email protected]>
Date: Mon, 26 Oct 2015 13:30:24 +0000
Subject: [PATCH] nvmem: core: return error for non word aligned bytes

nvmem providers have restrictions on register strides, so return error code when users attempt to read/write buffers with sizes which are not aligned to the word boundary.

Without this patch the userspace would continue to try as it does not get any error from the nvmem core, resulting in a hang.

Signed-off-by Srinivas Kandagatla <[email protected]>

---
 drivers/nvmem/core.c | 6 ++++++
 1 file changed, 6 insertions(+)

drivers/nvmem/core.c +3 -0

index 6fd4e5a..9d11d98 100644
@@ -70,6 +70,9 @@ static ssize_t bin_attr_nvmem_read(struct file *filp,
 struct kobject *kobj,
     if (pos >= nvmem->size)
         return 0;
 
+    if (count < nvmem->word_size)
+        return -EINVAL;
+
     if (pos + count > nvmem->size)
         count = nvmem->size - pos;

Srinivas,

El 26/10/15 a las 10:37, Srinivas Kandagatla escribi?:

Quoted 93 lines · Joachim Eastwood

Quoted 44 lines

Hi Ariel,

Quoted 26 lines · Ariel D'Alessandro

This commit adds support for NXP LPC18xx EEPROM memory found in NXP LPC185x/3x and LPC435x/3x/2x/1x devices.

EEPROM size is 16384 bytes and it can be entirely read and written/erased with 1 word (4 bytes) granularity. The last page (128 bytes) contains the EEPROM initialization data and is not writable.

Erase/program time is less than 3ms. The EEPROM device requires a ~1500 kHz clock (min 800 kHz, max 1600 kHz) that is generated dividing the system bus clock by the division factor, contained in the divider register (minus 1 encoded).

Signed-off-by Ariel D'Alessandro <[email protected]>

---
  drivers/nvmem/Kconfig          |   9 ++
  drivers/nvmem/Makefile         |   2 +
  drivers/nvmem/lpc18xx_eeprom.c | 266
+++++++++++++++++++++++++++++++++++++++++
  3 files changed, 277 insertions(+)

create mode 100644 drivers/nvmem/lpc18xx_eeprom.c

(quoted diff) +5 -0

+static int lpc18xx_eeprom_gather_write(void *context, const void *reg,
+                                      size_t reg_size, const void *val,
+                                      size_t val_size)
+{
+       struct lpc18xx_eeprom_dev *eeprom = context;

Review

Quoted 10 lines

(quoted diff) +10 -0

+       unsigned int offset = *(u32 *)reg;
+
+       /* 3 ms of erase/program time between each writing */
+       while (val_size) {
+               writel(*(u32 *)val, eeprom->mem_base + offset);
+               usleep_range(3000, 4000);
+               val_size -= eeprom->val_bytes;
+               val += eeprom->val_bytes;
+               offset += eeprom->val_bytes;
+       }

What happens here if 'val_size' is less than 4 or not dividable by 4? Same thing for 'offset'.

Quoted 4 lines

I tested the driver from sysfs by writing strings into the nvmem-file with echo. Writing a string not dividable by 4 seems to hang the system.

I think I know the issue here:
Could you try this patch:

-------------------------------->cut<---------------------------------- From 8cae10eff8ea8da9c5a8058ff75abeeddd8a8224 Mon Sep 17 00:00:00 2001
From: Srinivas Kandagatla <[email protected]>
Date: Mon, 26 Oct 2015 13:30:24 +0000
Subject: [PATCH] nvmem: core: return error for non word aligned bytes

nvmem providers have restrictions on register strides, so return error code when users attempt to read/write buffers with sizes which are not aligned to the word boundary.

Without this patch the userspace would continue to try as it does not get any error from the nvmem core, resulting in a hang.

Signed-off-by Srinivas Kandagatla <[email protected]>

---
 drivers/nvmem/core.c | 6 ++++++
 1 file changed, 6 insertions(+)

drivers/nvmem/core.c +3 -0

index 6fd4e5a..9d11d98 100644
@@ -70,6 +70,9 @@ static ssize_t bin_attr_nvmem_read(struct file *filp,
 struct kobject *kobj,
     if (pos >= nvmem->size)
         return 0;
 
+    if (count < nvmem->word_size)
+        return -EINVAL;
+
     if (pos + count > nvmem->size)
         count = nvmem->size - pos;

@@ -95,6 +98,9 @@ static ssize_t bin_attr_nvmem_write(struct file *filp,
 struct kobject *kobj,
     if (pos >= nvmem->size)
         return 0;
 

drivers/nvmem/core.c static ssize_t bin_attr_nvmem_write(struct file *filp, · line 106

Quoted 6 lines

drivers/nvmem/core.c +3 -0

+    if (count < nvmem->word_size)
+        return -EINVAL;
+
     if (pos + count > nvmem->size)
         count = nvmem->size - pos;

Patch looks good to me. I think that it solves the issue. I don't have the board here right now, so I'll check it ASAP and give some feedback.

Quoted 38 lines

Quoted 38 lines

Quoted 38 lines

Quoted 38 lines

-------------------------------->cut<---------------------------------- From 8cae10eff8ea8da9c5a8058ff75abeeddd8a8224 Mon Sep 17 00:00:00 2001
From: Srinivas Kandagatla<[email protected]>
Date: Mon, 26 Oct 2015 13:30:24 +0000
Subject: [PATCH] nvmem: core: return error for non word aligned bytes

nvmem providers have restrictions on register strides, so return error code when users attempt to read/write buffers with sizes which are not aligned to the word boundary.

Without this patch the userspace would continue to try as it does not get any error from the nvmem core, resulting in a hang.

Signed-off-by Srinivas Kandagatla<[email protected]>

---
 drivers/nvmem/core.c | 6 ++++++
 1 file changed, 6 insertions(+)

drivers/nvmem/core.c +3 -0

index 6fd4e5a..9d11d98 100644
@@ -70,6 +70,9 @@ static ssize_t bin_attr_nvmem_read(struct file *filp,
 struct kobject *kobj,
     if (pos >= nvmem->size)
         return 0;
 
+    if (count < nvmem->word_size)
+        return -EINVAL;
+
     if (pos + count > nvmem->size)
         count = nvmem->size - pos;

@@ -95,6 +98,9 @@ static ssize_t bin_attr_nvmem_write(struct file *filp,
 struct kobject *kobj,
     if (pos >= nvmem->size)
         return 0;
 

Quoted 10 lines

Quoted 10 lines

Quoted 6 lines

drivers/nvmem/core.c static ssize_t bin_attr_nvmem_write(struct file *filp, · line 106

Quoted 6 lines

drivers/nvmem/core.c +3 -0

+    if (count < nvmem->word_size)
+        return -EINVAL;
+
     if (pos + count > nvmem->size)
         count = nvmem->size - pos;

Patch looks good to me. I think that it solves the issue. I don't have the board here right now, so I'll check it ASAP and give some feedback.

Finally tested this. As it seemed, it solved the issue. Are you submitting a patch for this?

Thanks again.

Quoted 51 lines · Ariel D'Alessandro

Quoted 51 lines

Quoted 51 lines

Quoted 49 lines

Quoted 49 lines

Quoted 3 lines

Quoted 3 lines

-------------------------------->cut<----------------------------------

From 8cae10eff8ea8da9c5a8058ff75abeeddd8a8224 Mon Sep 17 00:00:00 2001
From: Srinivas Kandagatla<[email protected]>
Date: Mon, 26 Oct 2015 13:30:24 +0000
Subject: [PATCH] nvmem: core: return error for non word aligned bytes

nvmem providers have restrictions on register strides, so return error code when users attempt to read/write buffers with sizes which are not aligned to the word boundary.

Without this patch the userspace would continue to try as it does not get any error from the nvmem core, resulting in a hang.

Signed-off-by Srinivas Kandagatla<[email protected]>

---
 drivers/nvmem/core.c | 6 ++++++
 1 file changed, 6 insertions(+)

drivers/nvmem/core.c +3 -0

index 6fd4e5a..9d11d98 100644
@@ -70,6 +70,9 @@ static ssize_t bin_attr_nvmem_read(struct file
 *filp,
 struct kobject *kobj,
     if (pos >= nvmem->size)
         return 0;
 
+    if (count < nvmem->word_size)
+        return -EINVAL;
+
     if (pos + count > nvmem->size)

         count = nvmem->size - pos;

drivers/nvmem/core.c +3 -0

@@ -95,6 +98,9 @@ static ssize_t bin_attr_nvmem_write(struct file
 *filp,
 struct kobject *kobj,
     if (pos >= nvmem->size)
         return 0;
 
+    if (count < nvmem->word_size)
+        return -EINVAL;
+
     if (pos + count > nvmem->size)

         count = nvmem->size - pos;

Quoted 6 lines

Quoted 3 lines

Quoted 3 lines

Patch looks good to me. I think that it solves the issue. I don't have the board here right now, so I'll check it ASAP and give some feedback.

Finally tested this. As it seemed, it solved the issue. Are you submitting a patch for this?

Yes, I will send a patch to the mailing list.

Hi Ariel,

Quoted 29 lines · Ariel D'Alessandro

This commit adds support for NXP LPC18xx EEPROM memory found in NXP LPC185x/3x and LPC435x/3x/2x/1x devices.

EEPROM size is 16384 bytes and it can be entirely read and written/erased with 1 word (4 bytes) granularity. The last page (128 bytes) contains the EEPROM initialization data and is not writable.

Erase/program time is less than 3ms. The EEPROM device requires a ~1500 kHz clock (min 800 kHz, max 1600 kHz) that is generated dividing the system bus clock by the division factor, contained in the divider register (minus 1 encoded).

Signed-off-by Ariel D'Alessandro <[email protected]>

---
 drivers/nvmem/Kconfig          |   9 ++
 drivers/nvmem/Makefile         |   2 +
 drivers/nvmem/lpc18xx_eeprom.c | 266 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 277 insertions(+)
 create mode 100644 drivers/nvmem/lpc18xx_eeprom.c

drivers/nvmem/lpc18xx_eeprom.c new file +9 -0

+static int lpc18xx_eeprom_gather_write(void *context, const void *reg,
+                                      size_t reg_size, const void *val,
+                                      size_t val_size)
+{
+       struct lpc18xx_eeprom_dev *eeprom = context;
+       unsigned int offset = *(u32 *)reg;
+
+       /* 3 ms of erase/program time between each writing */
+       while (val_size) {

drivers/nvmem/lpc18xx_eeprom.c line 67

Quoted 6 lines

drivers/nvmem/lpc18xx_eeprom.c new file +6 -0

+               writel(*(u32 *)val, eeprom->mem_base + offset);
+               usleep_range(3000, 4000);
+               val_size -= eeprom->val_bytes;
+               val += eeprom->val_bytes;
+               offset += eeprom->val_bytes;
+       }

What happens here if 'val_size' is less than 4 or not dividable by 4?

Same thing for 'offset'.

I tested the driver from sysfs by writing strings into the nvmem-file with echo. Writing a string not dividable by 4 seems to hang the system.

Quoted 11 lines

drivers/nvmem/lpc18xx_eeprom.c new file +9 -0

+static int lpc18xx_eeprom_read(void *context, const void *reg, size_t reg_size,
+                              void *val, size_t val_size)
+{
+       struct lpc18xx_eeprom_dev *eeprom = context;
+       unsigned int offset = *(u32 *)reg;
+
+       while (val_size) {
+               *(u32 *)val = readl(eeprom->mem_base + offset);
+               val_size -= eeprom->val_bytes;

drivers/nvmem/lpc18xx_eeprom.c line 70

Quoted 6 lines

drivers/nvmem/lpc18xx_eeprom.c new file +6 -0

+               val += eeprom->val_bytes;
+               offset += eeprom->val_bytes;
+       }
+
+       return 0;
+}

Same comments as for lpc18xx_eeprom_gather_write().

drivers/nvmem/lpc18xx_eeprom.c line 139

Quoted 7 lines

drivers/nvmem/lpc18xx_eeprom.c new file +5 -0

+static const struct of_device_id lpc18xx_eeprom_of_match[] = {
+       { .compatible = "nxp,lpc1857-eeprom" },
+       { },
+};
+MODULE_DEVICE_TABLE(of, lpc18xx_eeprom_of_match);

nit: It's usual to place of_device_id struct just above the platform_driver struct.

drivers/nvmem/lpc18xx_eeprom.c line 191

Quoted 4 lines

drivers/nvmem/lpc18xx_eeprom.c new file +2 -0

+       eeprom->val_bytes = lpc18xx_regmap_config.val_bits / 8;
+       eeprom->reg_bytes = lpc18xx_regmap_config.reg_bits / 8;

There is a BITS_PER_BYTE define in bitops.h that you might want to use here.

Quoted 50 lines

drivers/nvmem/lpc18xx_eeprom.c new file +48 -0

+       /*
+        * Clock rate is generated by dividing the system bus clock by the
+        * division factor, contained in the divider register (minus 1 encoded).
+        */
+       clk_rate = clk_get_rate(eeprom->clk);
+       clk_rate = DIV_ROUND_UP(clk_rate, LPC18XX_EEPROM_CLOCK_HZ) - 1;
+       lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_CLKDIV, clk_rate);
+
+       /*
+        * Writing a single word to the page will start the erase/program cycle
+        * automatically
+        */
+       lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_AUTOPROG,
+                             LPC18XX_EEPROM_AUTOPROG_WORD);
+
+       lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_PWRDWN,
+                             LPC18XX_EEPROM_PWRDWN_NO);
+
+       lpc18xx_regmap_config.max_register = resource_size(res) - 1;
+       lpc18xx_regmap_config.writeable_reg = lpc18xx_eeprom_writeable_reg;
+       lpc18xx_regmap_config.readable_reg = lpc18xx_eeprom_readable_reg;
+
+       regmap = devm_regmap_init(dev, &lpc18xx_eeprom_bus, eeprom,
+                                 &lpc18xx_regmap_config);
+       if (IS_ERR(regmap)) {
+               dev_err(dev, "regmap init failed: %ld\n", PTR_ERR(regmap));
+               ret = PTR_ERR(regmap);
+               goto err_clk;
+       }
+
+       lpc18xx_nvmem_config.dev = dev;
+
+       eeprom->nvmem = nvmem_register(&lpc18xx_nvmem_config);
+       if (IS_ERR(eeprom->nvmem)) {
+               ret = PTR_ERR(eeprom->nvmem);
+               goto err_clk;
+       }
+
+       platform_set_drvdata(pdev, eeprom);
+
+       return 0;
+
+err_clk:
+       clk_disable_unprepare(eeprom->clk);
+
+       return ret;
+}
+

drivers/nvmem/lpc18xx_eeprom.c line 250

Quoted 10 lines

drivers/nvmem/lpc18xx_eeprom.c new file +10 -0

+static int lpc18xx_eeprom_remove(struct platform_device *pdev)
+{
+       struct lpc18xx_eeprom_dev *eeprom = platform_get_drvdata(pdev);
+
+       lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_PWRDWN,
+                             LPC18XX_EEPROM_PWRDWN_YES);
+
+       clk_disable_unprepare(eeprom->clk);
+
+       return nvmem_unregister(eeprom->nvmem);

Normally you do tear down in the reverse order of initialization.

Consider what happens here when you power down and disable the clock while there still are nvmem users of the eeprom.

Joachim,

El 24/10/15 a las 19:04, Joachim Eastwood escribi?:

Quoted 33 lines

Hi Ariel,

Quoted 29 lines · Ariel D'Alessandro

This commit adds support for NXP LPC18xx EEPROM memory found in NXP LPC185x/3x and LPC435x/3x/2x/1x devices.

EEPROM size is 16384 bytes and it can be entirely read and written/erased with 1 word (4 bytes) granularity. The last page (128 bytes) contains the EEPROM initialization data and is not writable.

Erase/program time is less than 3ms. The EEPROM device requires a ~1500 kHz clock (min 800 kHz, max 1600 kHz) that is generated dividing the system bus clock by the division factor, contained in the divider register (minus 1 encoded).

Signed-off-by Ariel D'Alessandro <[email protected]>

---
 drivers/nvmem/Kconfig          |   9 ++
 drivers/nvmem/Makefile         |   2 +
 drivers/nvmem/lpc18xx_eeprom.c | 266 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 277 insertions(+)
 create mode 100644 drivers/nvmem/lpc18xx_eeprom.c

drivers/nvmem/lpc18xx_eeprom.c new file +9 -0

+static int lpc18xx_eeprom_gather_write(void *context, const void *reg,
+                                      size_t reg_size, const void *val,
+                                      size_t val_size)
+{
+       struct lpc18xx_eeprom_dev *eeprom = context;
+       unsigned int offset = *(u32 *)reg;
+
+       /* 3 ms of erase/program time between each writing */
+       while (val_size) {

Quoted 8 lines

drivers/nvmem/lpc18xx_eeprom.c line 67

Quoted 6 lines

drivers/nvmem/lpc18xx_eeprom.c new file +6 -0

+               writel(*(u32 *)val, eeprom->mem_base + offset);
+               usleep_range(3000, 4000);
+               val_size -= eeprom->val_bytes;
+               val += eeprom->val_bytes;
+               offset += eeprom->val_bytes;
+       }

What happens here if 'val_size' is less than 4 or not dividable by 4?

AFAIK, before calling to gather_write, the caller ensures that 'val_size' % map->format.val_bytes == 0

Like nvmem_device_write() do this in
drivers/base/regmap/regmap.c:regmap_raw_write()

Then, if val_size == 0, nothing will happen.

Quoted 1 line

Same thing for 'offset'.

Let me check this one.

This commit adds support for NXP LPC18xx EEPROM memory found in NXP

LPC185x/3x and LPC435x/3x/2x/1x devices.

EEPROM size is 16384 bytes and it can be entirely read and written/erased with 1 word (4 bytes) granularity. The last page (128 bytes) contains the EEPROM initialization data and is not writable.

Erase/program time is less than 3ms. The EEPROM device requires a ~1500 kHz clock (min 800 kHz, max 1600 kHz) that is generated dividing the system bus clock by the division factor, contained in the divider register (minus 1 encoded).

Signed-off-by Ariel D'Alessandro <[email protected]>

---
  drivers/nvmem/Kconfig          |   9 ++
  drivers/nvmem/Makefile         |   2 +
  drivers/nvmem/lpc18xx_eeprom.c | 266 +++++++++++++++++++++++++++++++++++++++++
  3 files changed, 277 insertions(+)

create mode 100644 drivers/nvmem/lpc18xx_eeprom.c

drivers/nvmem/Kconfig +9 -0

index bc4ea58..6ff1b50 100644
@@ -25,6 +25,15 @@ config NVMEM_IMX_OCOTP
  	  This driver can also be built as a module. If so, the module
  	  will be called nvmem-imx-ocotp.
 
+config NVMEM_LPC18XX_EEPROM
+	tristate "NXP LPC18XX EEPROM Memory Support"
+	depends on ARCH_LPC18XX || COMPILE_TEST
+	help
+	  Say Y here to include support for NXP LPC18xx EEPROM memory found in
+	  NXP LPC185x/3x and LPC435x/3x/2x/1x devices.
+	  To compile this driver as a module, choose M here: the module
+	  will be called nvmem_lpc18xx_eeprom.
+
  config NVMEM_MXS_OCOTP
  	tristate "Freescale MXS On-Chip OTP Memory Support"
  	depends on ARCH_MXS || COMPILE_TEST

drivers/nvmem/Makefile +2 -0

index 95dde3f..c14a556 100644
@@ -8,6 +8,8 @@ nvmem_core-y			:= core.o
  # Devices
  obj-$(CONFIG_NVMEM_IMX_OCOTP)	+= nvmem-imx-ocotp.o
  nvmem-imx-ocotp-y		:= imx-ocotp.o
+obj-$(CONFIG_NVMEM_LPC18XX_EEPROM)	+= nvmem_lpc18xx_eeprom.o
+nvmem_lpc18xx_eeprom-y	:= lpc18xx_eeprom.o
  obj-$(CONFIG_NVMEM_MXS_OCOTP)	+= nvmem-mxs-ocotp.o
  nvmem-mxs-ocotp-y		:= mxs-ocotp.o
  obj-$(CONFIG_QCOM_QFPROM)	+= nvmem_qfprom.o

drivers/nvmem/lpc18xx_eeprom.c new file +8 -0

new file mode 100644
index 0000000..ccdda66
@@ -0,0 +1,266 @@
+/*
+ * NXP LPC18xx/LPC43xx EEPROM memory NVMEM driver
+ *
+ * Copyright (c) 2015 Ariel D'Alessandro <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.

drivers/nvmem/lpc18xx_eeprom.c new file +23 -0

+#include <linux/regmap.h>
+#include <linux/reset.h>
+
+/* Registers */
+#define LPC18XX_EEPROM_AUTOPROG		0x00c
+#define LPC18XX_EEPROM_AUTOPROG_WORD	0x1
+
+#define LPC18XX_EEPROM_CLKDIV		0x014
+
+#define LPC18XX_EEPROM_PWRDWN		0x018
+#define LPC18XX_EEPROM_PWRDWN_NO	0x0
+#define LPC18XX_EEPROM_PWRDWN_YES	0x1
+
+/* Fixed page size (bytes) */
+#define LPC18XX_EEPROM_PAGE_SIZE	0x80
+
+/* EEPROM device requires a ~1500 kHz clock (min 800 kHz, max 1600 kHz) */
+#define LPC18XX_EEPROM_CLOCK_HZ		1500000
+
+struct lpc18xx_eeprom_dev {
+	struct clk *clk;
+	void __iomem *reg_base;
+	void __iomem *mem_base;

drivers/nvmem/lpc18xx_eeprom.c new file +19 -0

+
+static int lpc18xx_eeprom_gather_write(void *context, const void *reg,
+				       size_t reg_size, const void *val,
+				       size_t val_size)
+{
+	struct lpc18xx_eeprom_dev *eeprom = context;
+	unsigned int offset = *(u32 *)reg;
+
+	/* 3 ms of erase/program time between each writing */
+	while (val_size) {
+		writel(*(u32 *)val, eeprom->mem_base + offset);
+		usleep_range(3000, 4000);
+		val_size -= eeprom->val_bytes;
+		val += eeprom->val_bytes;
+		offset += eeprom->val_bytes;
+	}
+
+	return 0;
+}

drivers/nvmem/lpc18xx_eeprom.c new file +119 -0

+	return lpc18xx_eeprom_gather_write(context, data, eeprom->reg_bytes,
+					   data + offset, count - offset);
+}
+
+static int lpc18xx_eeprom_read(void *context, const void *reg, size_t reg_size,
+			       void *val, size_t val_size)
+{
+	struct lpc18xx_eeprom_dev *eeprom = context;
+	unsigned int offset = *(u32 *)reg;
+
+	while (val_size) {
+		*(u32 *)val = readl(eeprom->mem_base + offset);
+		val_size -= eeprom->val_bytes;
+		val += eeprom->val_bytes;
+		offset += eeprom->val_bytes;
+	}
+
+	return 0;
+}
+
+static struct regmap_bus lpc18xx_eeprom_bus = {
+	.write = lpc18xx_eeprom_write,
+	.gather_write = lpc18xx_eeprom_gather_write,
+	.read = lpc18xx_eeprom_read,
+	.reg_format_endian_default = REGMAP_ENDIAN_NATIVE,
+	.val_format_endian_default = REGMAP_ENDIAN_NATIVE,
+};
+
+static struct regmap_config lpc18xx_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+};
+
+static bool lpc18xx_eeprom_writeable_reg(struct device *dev, unsigned int reg)
+{
+	/*
+	 * The last page contains the EEPROM initialization data and is not
+	 * writable.
+	 */
+	return reg <= lpc18xx_regmap_config.max_register -
+						LPC18XX_EEPROM_PAGE_SIZE;
+}
+
+static bool lpc18xx_eeprom_readable_reg(struct device *dev, unsigned int reg)
+{
+	return reg <= lpc18xx_regmap_config.max_register;
+}
+
+static struct nvmem_config lpc18xx_nvmem_config = {
+	.name = "lpc18xx-eeprom",
+	.owner = THIS_MODULE,
+};
+
+static const struct of_device_id lpc18xx_eeprom_of_match[] = {
+	{ .compatible = "nxp,lpc1857-eeprom" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, lpc18xx_eeprom_of_match);
+
+static int lpc18xx_eeprom_probe(struct platform_device *pdev)
+{
+	struct lpc18xx_eeprom_dev *eeprom;
+	struct device *dev = &pdev->dev;
+	struct reset_control *rst;
+	unsigned long clk_rate;
+	struct regmap *regmap;
+	struct resource *res;
+	int ret;
+
+	eeprom = devm_kzalloc(dev, sizeof(*eeprom), GFP_KERNEL);
+	if (!eeprom)
+		return -ENOMEM;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
+	eeprom->reg_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(eeprom->reg_base))
+		return PTR_ERR(eeprom->reg_base);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mem");
+	eeprom->mem_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(eeprom->mem_base))
+		return PTR_ERR(eeprom->mem_base);
+
+	eeprom->clk = devm_clk_get(&pdev->dev, "eeprom");
+	if (IS_ERR(eeprom->clk)) {
+		dev_err(&pdev->dev, "failed to get eeprom clock\n");
+		return PTR_ERR(eeprom->clk);
+	}
+
+	ret = clk_prepare_enable(eeprom->clk);
+	if (ret < 0) {
+		dev_err(dev, "failed to prepare/enable eeprom clk: %d\n", ret);
+		return ret;
+	}
+
+	rst = devm_reset_control_get(dev, NULL);
+	if (IS_ERR(rst)) {
+		dev_err(dev, "failed to get reset: %ld\n", PTR_ERR(rst));
+		ret = PTR_ERR(rst);
+		goto err_clk;
+	}
+
+	ret = reset_control_deassert(rst);
+	if (ret < 0) {
+		dev_err(dev, "failed to deassert reset: %d\n", ret);
+		goto err_clk;
+	}
+
+	eeprom->val_bytes = lpc18xx_regmap_config.val_bits / 8;
+	eeprom->reg_bytes = lpc18xx_regmap_config.reg_bits / 8;
+
+	/*
+	 * Clock rate is generated by dividing the system bus clock by the
+	 * division factor, contained in the divider register (minus 1 encoded).
+	 */
+	clk_rate = clk_get_rate(eeprom->clk);
+	clk_rate = DIV_ROUND_UP(clk_rate, LPC18XX_EEPROM_CLOCK_HZ) - 1;
+	lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_CLKDIV, clk_rate);

drivers/nvmem/lpc18xx_eeprom.c new file +16 -0

+
+	lpc18xx_regmap_config.max_register = resource_size(res) - 1;
+	lpc18xx_regmap_config.writeable_reg = lpc18xx_eeprom_writeable_reg;
+	lpc18xx_regmap_config.readable_reg = lpc18xx_eeprom_readable_reg;
+
+	regmap = devm_regmap_init(dev, &lpc18xx_eeprom_bus, eeprom,
+				  &lpc18xx_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(dev, "regmap init failed: %ld\n", PTR_ERR(regmap));
+		ret = PTR_ERR(regmap);
+		goto err_clk;
+	}
+
+	lpc18xx_nvmem_config.dev = dev;
+
+	eeprom->nvmem = nvmem_register(&lpc18xx_nvmem_config);

drivers/nvmem/lpc18xx_eeprom.c new file +5 -0

+	clk_disable_unprepare(eeprom->clk);
+
+	return ret;
+}
+

drivers/nvmem/lpc18xx_eeprom.c new file +16 -0

+}
+
+static struct platform_driver lpc18xx_eeprom_driver = {
+	.probe = lpc18xx_eeprom_probe,
+	.remove = lpc18xx_eeprom_remove,
+	.driver = {
+		.name = "lpc18xx-eeprom",
+		.of_match_table = lpc18xx_eeprom_of_match,
+	},
+};
+
+module_platform_driver(lpc18xx_eeprom_driver);
+
+MODULE_AUTHOR("Ariel D'Alessandro <[email protected]>");
+MODULE_DESCRIPTION("NXP LPC18xx EEPROM memory Driver");
+MODULE_LICENSE("GPL v2");

Quoted 1 line

This commit adds support for NXP LPC18xx EEPROM memory found in NXP

s/commit/patch

Quoted 21 lines

LPC185x/3x and LPC435x/3x/2x/1x devices.

EEPROM size is 16384 bytes and it can be entirely read and written/erased with 1 word (4 bytes) granularity. The last page (128 bytes) contains the EEPROM initialization data and is not writable.

Erase/program time is less than 3ms. The EEPROM device requires a ~1500 kHz clock (min 800 kHz, max 1600 kHz) that is generated dividing the system bus clock by the division factor, contained in the divider register (minus 1 encoded).

Signed-off-by Ariel D'Alessandro <[email protected]>

---
  drivers/nvmem/Kconfig          |   9 ++
  drivers/nvmem/Makefile         |   2 +
  drivers/nvmem/lpc18xx_eeprom.c | 266
+++++++++++++++++++++++++++++++++++++++++
  3 files changed, 277 insertions(+)

create mode 100644 drivers/nvmem/lpc18xx_eeprom.c

Quoted 18 lines

drivers/nvmem/lpc18xx_eeprom.c new file +6 -0

new file mode 100644
index 0000000..ccdda66
@@ -0,0 +1,266 @@
+/*
+ * NXP LPC18xx/LPC43xx EEPROM memory NVMEM driver
+ *
+ * Copyright (c) 2015 Ariel D'Alessandro <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or

modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */

drivers/nvmem/lpc18xx_eeprom.c line 18

Quoted 9 lines

drivers/nvmem/lpc18xx_eeprom.c new file +9 -0

+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/nvmem-provider.h>
+#include <linux/of_device.h>

Why do you need above?

Also you should probably include

#include <linux/platform_device.h>

Quoted 31 lines

drivers/nvmem/lpc18xx_eeprom.c new file +17 -0

+#include <linux/regmap.h>
+#include <linux/reset.h>
+
+/* Registers */
+#define LPC18XX_EEPROM_AUTOPROG        0x00c
+#define LPC18XX_EEPROM_AUTOPROG_WORD    0x1
+
+#define LPC18XX_EEPROM_CLKDIV        0x014
+
+#define LPC18XX_EEPROM_PWRDWN        0x018
+#define LPC18XX_EEPROM_PWRDWN_NO    0x0
+#define LPC18XX_EEPROM_PWRDWN_YES    0x1
+
+/* Fixed page size (bytes) */
+#define LPC18XX_EEPROM_PAGE_SIZE    0x80
+
+/* EEPROM device requires a ~1500 kHz clock (min 800 kHz, max 1600

kHz) */

drivers/nvmem/lpc18xx_eeprom.c new file +11 -0

+#define LPC18XX_EEPROM_CLOCK_HZ        1500000
+
+struct lpc18xx_eeprom_dev {
+    struct clk *clk;
+    void __iomem *reg_base;
+    void __iomem *mem_base;
+    struct nvmem_device *nvmem;
+    unsigned reg_bytes;
+    unsigned val_bytes;
+};
+

drivers/nvmem/lpc18xx_eeprom.c line 51

Quoted 6 lines

+static inline void lpc18xx_eeprom_writel(struct lpc18xx_eeprom_dev *eeprom,

drivers/nvmem/lpc18xx_eeprom.c new file +4 -0

+                     u32 reg, u32 val)
+{
+    writel(val, eeprom->reg_base + reg);
+}

I don't have a strong feeling but, I see no point to have a wrapper for writel which is only used in probe function.

Quoted 24 lines

drivers/nvmem/lpc18xx_eeprom.c new file +21 -0

+
+static int lpc18xx_eeprom_gather_write(void *context, const void *reg,
+                       size_t reg_size, const void *val,
+                       size_t val_size)
+{
+    struct lpc18xx_eeprom_dev *eeprom = context;
+    unsigned int offset = *(u32 *)reg;
+
+    /* 3 ms of erase/program time between each writing */
+    while (val_size) {
+        writel(*(u32 *)val, eeprom->mem_base + offset);
+        usleep_range(3000, 4000);
+        val_size -= eeprom->val_bytes;
+        val += eeprom->val_bytes;
+        offset += eeprom->val_bytes;
+    }
+
+    return 0;
+}
+
+static int lpc18xx_eeprom_write(void *context, const void *data,

size_t count)
+{

drivers/nvmem/lpc18xx_eeprom.c line 80

Quoted 7 lines

drivers/nvmem/lpc18xx_eeprom.c new file +7 -0

+    struct lpc18xx_eeprom_dev *eeprom = context;
+    unsigned int offset = eeprom->reg_bytes;
+
+    if (count <= offset)
+        return -EINVAL;
+
+

unnecessary new line.

Quoted 5 lines

drivers/nvmem/lpc18xx_eeprom.c new file +4 -0

+    return lpc18xx_eeprom_gather_write(context, data, eeprom->reg_bytes,
+                       data + offset, count - offset);
+}
+

Quoted 65 lines

drivers/nvmem/lpc18xx_eeprom.c new file +56 -0

+
+static int lpc18xx_eeprom_probe(struct platform_device *pdev)
+{
+    struct lpc18xx_eeprom_dev *eeprom;
+    struct device *dev = &pdev->dev;
+    struct reset_control *rst;
+    unsigned long clk_rate;
+    struct regmap *regmap;
+    struct resource *res;
+    int ret;
+
+    eeprom = devm_kzalloc(dev, sizeof(*eeprom), GFP_KERNEL);
+    if (!eeprom)
+        return -ENOMEM;
+
+    res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
+    eeprom->reg_base = devm_ioremap_resource(dev, res);
+    if (IS_ERR(eeprom->reg_base))
+        return PTR_ERR(eeprom->reg_base);
+
+    res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mem");
+    eeprom->mem_base = devm_ioremap_resource(dev, res);
+    if (IS_ERR(eeprom->mem_base))
+        return PTR_ERR(eeprom->mem_base);
+
+    eeprom->clk = devm_clk_get(&pdev->dev, "eeprom");
+    if (IS_ERR(eeprom->clk)) {
+        dev_err(&pdev->dev, "failed to get eeprom clock\n");
+        return PTR_ERR(eeprom->clk);
+    }
+
+    ret = clk_prepare_enable(eeprom->clk);
+    if (ret < 0) {
+        dev_err(dev, "failed to prepare/enable eeprom clk: %d\n", ret);
+        return ret;
+    }
+
+    rst = devm_reset_control_get(dev, NULL);
+    if (IS_ERR(rst)) {
+        dev_err(dev, "failed to get reset: %ld\n", PTR_ERR(rst));
+        ret = PTR_ERR(rst);
+        goto err_clk;
+    }
+
+    ret = reset_control_deassert(rst);
+    if (ret < 0) {
+        dev_err(dev, "failed to deassert reset: %d\n", ret);
+        goto err_clk;
+    }
+
+    eeprom->val_bytes = lpc18xx_regmap_config.val_bits / 8;
+    eeprom->reg_bytes = lpc18xx_regmap_config.reg_bits / 8;
+
+    /*
+     * Clock rate is generated by dividing the system bus clock by the
+     * division factor, contained in the divider register (minus 1

encoded).

drivers/nvmem/lpc18xx_eeprom.c new file +7 -0

+     */
+    clk_rate = clk_get_rate(eeprom->clk);
+    clk_rate = DIV_ROUND_UP(clk_rate, LPC18XX_EEPROM_CLOCK_HZ) - 1;
+    lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_CLKDIV, clk_rate);
+
+    /*
+     * Writing a single word to the page will start the erase/program

cycle

drivers/nvmem/lpc18xx_eeprom.c line 209

Quoted 7 lines

drivers/nvmem/lpc18xx_eeprom.c new file +7 -0

+     * automatically
+     */
+    lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_AUTOPROG,
+                  LPC18XX_EEPROM_AUTOPROG_WORD);
+
+    lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_PWRDWN,
+                  LPC18XX_EEPROM_PWRDWN_NO);

Any reason not power up/dowm dynamically in
lpc18xx_eeprom_write()/lpc18xx_eeprom_read().

This can potentially save some power.

Quoted 21 lines

drivers/nvmem/lpc18xx_eeprom.c new file +20 -0

+
+    lpc18xx_regmap_config.max_register = resource_size(res) - 1;
+    lpc18xx_regmap_config.writeable_reg = lpc18xx_eeprom_writeable_reg;
+    lpc18xx_regmap_config.readable_reg = lpc18xx_eeprom_readable_reg;
+
+    regmap = devm_regmap_init(dev, &lpc18xx_eeprom_bus, eeprom,
+                  &lpc18xx_regmap_config);
+    if (IS_ERR(regmap)) {
+        dev_err(dev, "regmap init failed: %ld\n", PTR_ERR(regmap));
+        ret = PTR_ERR(regmap);
+        goto err_clk;
+    }
+
+    lpc18xx_nvmem_config.dev = dev;
+
+    eeprom->nvmem = nvmem_register(&lpc18xx_nvmem_config);
+    if (IS_ERR(eeprom->nvmem)) {
+        ret = PTR_ERR(eeprom->nvmem);
+        goto err_clk;
+    }

drivers/nvmem/lpc18xx_eeprom.c line 235

Quoted 6 lines

drivers/nvmem/lpc18xx_eeprom.c new file +6 -0

+
+    platform_set_drvdata(pdev, eeprom);
+
+    return 0;
+
+err_clk:

Should this error path also include power down/reset assert the eeprom?

Quoted 10 lines

drivers/nvmem/lpc18xx_eeprom.c new file +9 -0

+    clk_disable_unprepare(eeprom->clk);
+
+    return ret;
+}
+
+static int lpc18xx_eeprom_remove(struct platform_device *pdev)
+{
+    struct lpc18xx_eeprom_dev *eeprom = platform_get_drvdata(pdev);
+

drivers/nvmem/lpc18xx_eeprom.c line 250

Quoted 6 lines

drivers/nvmem/lpc18xx_eeprom.c new file +6 -0

+    lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_PWRDWN,
+                  LPC18XX_EEPROM_PWRDWN_YES);
+
+    clk_disable_unprepare(eeprom->clk);
+
+    return nvmem_unregister(eeprom->nvmem);

Same comment as Joachim, should the reset be asserted too?

El 26/10/15 a las 11:23, Srinivas Kandagatla escribi?:

Quoted 5 lines · Ariel D'Alessandro

Quoted 1 line

This commit adds support for NXP LPC18xx EEPROM memory found in NXP

s/commit/patch

OK.

This commit adds support for NXP LPC18xx EEPROM memory found in NXP LPC185x/3x and LPC435x/3x/2x/1x devices.

EEPROM size is 16384 bytes and it can be entirely read and written/erased with 1 word (4 bytes) granularity. The last page (128 bytes) contains the EEPROM initialization data and is not writable.

Erase/program time is less than 3ms. The EEPROM device requires a ~1500 kHz clock (min 800 kHz, max 1600 kHz) that is generated dividing the system bus clock by the division factor, contained in the divider register (minus 1 encoded).

Signed-off-by Ariel D'Alessandro <[email protected]>

---
 drivers/nvmem/Kconfig          |   9 ++
 drivers/nvmem/Makefile         |   2 +
 drivers/nvmem/lpc18xx_eeprom.c | 266 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 277 insertions(+)
 create mode 100644 drivers/nvmem/lpc18xx_eeprom.c

[...]

drivers/nvmem/lpc18xx_eeprom.c new file +2 -0

+
+static int lpc18xx_eeprom_gather_write(void *context, const void *reg,

drivers/nvmem/lpc18xx_eeprom.c new file +7 -0

+		val_size -= eeprom->val_bytes;
+		val += eeprom->val_bytes;
+		offset += eeprom->val_bytes;
+	}
+
+	return 0;
+}

Hi Ariel,

Am 19.10.2015 um 19:32 schrieb Ariel D'Alessandro:

Quoted 27 lines

This commit adds support for NXP LPC18xx EEPROM memory found in NXP LPC185x/3x and LPC435x/3x/2x/1x devices.

EEPROM size is 16384 bytes and it can be entirely read and written/erased with 1 word (4 bytes) granularity. The last page (128 bytes) contains the EEPROM initialization data and is not writable.

Erase/program time is less than 3ms. The EEPROM device requires a ~1500 kHz clock (min 800 kHz, max 1600 kHz) that is generated dividing the system bus clock by the division factor, contained in the divider register (minus 1 encoded).

Signed-off-by Ariel D'Alessandro <[email protected]>

---
 drivers/nvmem/Kconfig          |   9 ++
 drivers/nvmem/Makefile         |   2 +
 drivers/nvmem/lpc18xx_eeprom.c | 266 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 277 insertions(+)
 create mode 100644 drivers/nvmem/lpc18xx_eeprom.c

[...]

drivers/nvmem/lpc18xx_eeprom.c new file +7 -0

+
+static int lpc18xx_eeprom_gather_write(void *context, const void *reg,
+				       size_t reg_size, const void *val,
+				       size_t val_size)
+{
+	struct lpc18xx_eeprom_dev *eeprom = context;
+	unsigned int offset = *(u32 *)reg;

drivers/nvmem/lpc18xx_eeprom.c line 63

Quoted 5 lines

drivers/nvmem/lpc18xx_eeprom.c new file +5 -0

+
+	/* 3 ms of erase/program time between each writing */
+	while (val_size) {
+		writel(*(u32 *)val, eeprom->mem_base + offset);
+		usleep_range(3000, 4000);

i think it would be good to verify that the EEPROM write operation has really finished.

Ariel D'Alessandro <[email protected]> hat am 16. November 2015 um 16:29 geschrieben:

Hi Stefan,

Sorry for the delay.

El 03/11/15 a las 05:20, Stefan Wahren escribi?:

Quoted 31 lines

Hi Ariel,

Am 19.10.2015 um 19:32 schrieb Ariel D'Alessandro:

Quoted 28 lines

This commit adds support for NXP LPC18xx EEPROM memory found in NXP LPC185x/3x and LPC435x/3x/2x/1x devices.

EEPROM size is 16384 bytes and it can be entirely read and written/erased with 1 word (4 bytes) granularity. The last page (128 bytes) contains the EEPROM initialization data and is not writable.

Erase/program time is less than 3ms. The EEPROM device requires a ~1500 kHz clock (min 800 kHz, max 1600 kHz) that is generated dividing the system bus clock by the division factor, contained in the divider register (minus 1 encoded).

Signed-off-by Ariel D'Alessandro <[email protected]>

---
drivers/nvmem/Kconfig | 9 ++
drivers/nvmem/Makefile | 2 +
drivers/nvmem/lpc18xx_eeprom.c | 266
+++++++++++++++++++++++++++++++++++++++++

3 files changed, 277 insertions(+)

create mode 100644 drivers/nvmem/lpc18xx_eeprom.c
[...]

drivers/nvmem/lpc18xx_eeprom.c new file +7 -0

+
+static int lpc18xx_eeprom_gather_write(void *context, const void *reg,
+ size_t reg_size, const void *val,
+ size_t val_size)
+{
+ struct lpc18xx_eeprom_dev *eeprom = context;
+ unsigned int offset = *(u32 *)reg;

Quoted 8 lines

Quoted 5 lines

+
+ /* 3 ms of erase/program time between each writing */
+ while (val_size) {
+ writel(*(u32 *)val, eeprom->mem_base + offset);
+ usleep_range(3000, 4000);

i think it would be good to verify that the EEPROM write operation has really finished.

I'm not sure what are you proposing. Why could the write operation not finish?

--
Ariel D'Alessandro, VanguardiaSur
www.vanguardiasur.com.ar

Hi Ariel,

Quoted 49 lines

Ariel D'Alessandro <[email protected]> hat am 16. November 2015 um 16:29 geschrieben:

Hi Stefan,

Sorry for the delay.

El 03/11/15 a las 05:20, Stefan Wahren escribi?:

Quoted 31 lines

Hi Ariel,

Am 19.10.2015 um 19:32 schrieb Ariel D'Alessandro:

Quoted 28 lines

This commit adds support for NXP LPC18xx EEPROM memory found in NXP LPC185x/3x and LPC435x/3x/2x/1x devices.

EEPROM size is 16384 bytes and it can be entirely read and written/erased with 1 word (4 bytes) granularity. The last page (128 bytes) contains the EEPROM initialization data and is not writable.

Erase/program time is less than 3ms. The EEPROM device requires a ~1500 kHz clock (min 800 kHz, max 1600 kHz) that is generated dividing the system bus clock by the division factor, contained in the divider register (minus 1 encoded).

Signed-off-by Ariel D'Alessandro <[email protected]>

---
drivers/nvmem/Kconfig | 9 ++
drivers/nvmem/Makefile | 2 +
drivers/nvmem/lpc18xx_eeprom.c | 266
+++++++++++++++++++++++++++++++++++++++++

3 files changed, 277 insertions(+)

create mode 100644 drivers/nvmem/lpc18xx_eeprom.c
[...]

drivers/nvmem/lpc18xx_eeprom.c new file +7 -0

+
+static int lpc18xx_eeprom_gather_write(void *context, const void *reg,
+ size_t reg_size, const void *val,
+ size_t val_size)
+{
+ struct lpc18xx_eeprom_dev *eeprom = context;
+ unsigned int offset = *(u32 *)reg;

Quoted 8 lines

Quoted 5 lines

+
+ /* 3 ms of erase/program time between each writing */
+ while (val_size) {
+ writel(*(u32 *)val, eeprom->mem_base + offset);
+ usleep_range(3000, 4000);

i think it would be good to verify that the EEPROM write operation has really finished.

Quoted 2 lines

I'm not sure what are you proposing. Why could the write operation not finish?

it's always good to keep in sync with the hardware. Here is an extract of chapter
"46.6.2.1 Writing and erase/programming" from the datasheet [1]:

During programming, the EEPROM is not available for other operations. To
prevent
undesired loss in performance which would be caused by stalling the bus, the
EEPROM
instead generates an error for AHB read/writes and APB writes when programming
is
busy. In order to prevent the error response, the program operation finished
interrupt
can be enabled or the interrupt status bit can be polled.

Please blame me if it doesn't apply.

It's only a suggestion: How about checking the interrupt status bit for END_OF_PROG after the 3 ms sleep?

Add node for the NXP LPC18xx EEPROM memory which can be found in NXP LPC185x/3x and LPC435x/3x/2x/1x devices.

Signed-off-by Ariel D'Alessandro <[email protected]>

Enable NVMEM driver for NXP LPC18xx EEPROM, which can be found in NXP LPC185x/3x and LPC435x/3x/2x/1x devices.

Signed-off-by Ariel D'Alessandro <[email protected]>