regmap: add support for address cell 2

PATCH Superseded Kever Yang 3 participants 5 messages May 5, 2017 – Jun 20, 2017 Library code

lore

Share & tools

lore

Kever Yang

author

2017-05-05 02:39 UTC #0 lore raw

PATCH v3 1/1 regmap: add support for address cell 2

ARM64 is using 64bit address which address cell is 2 instead of 1, update to support it when of-platdata enabled.

Signed-off-by Kever Yang <[email protected]>

---

Changes in v3:
- move of_plat_get_number() into lib/of_plat.c

Changes in v2:
- rename the fdtdec_get_number() to of_plat_get_number()

 drivers/core/regmap.c |  9 +++++++++
 include/of_plat.h     | 22 ++++++++++++++++++++++
 lib/Makefile          |  3 +++
 lib/of_plat.c         | 17 +++++++++++++++++
 4 files changed, 51 insertions(+)
 create mode 100644 include/of_plat.h
 create mode 100644 lib/of_plat.c

drivers/core/regmap.c +9 -0

index 3bec3df..c03279e 100644
@@ -12,6 +12,7 @@
 #include <malloc.h>
 #include <mapmem.h>
 #include <regmap.h>
+#include <of_plat.h>
 
 #include <asm/io.h>

@@ -49,11 +50,19 @@ int regmap_init_mem_platdata(struct udevice *dev, u32 *reg, int count,
 	if (!map)
 		return -ENOMEM;
 
+#ifdef CONFIG_PHYS_64BIT
+	map->base = of_plat_get_number(reg, 2);
+	for (range = map->range; count > 0; reg += 4, range++, count--) {
+		range->start = of_plat_get_number(reg, 2);
+		range->size = of_plat_get_number(reg + 2, 2);
+	}
+#else
 	map->base = *reg;
 	for (range = map->range; count > 0; reg += 2, range++, count--) {
 		range->start = *reg;
 		range->size = reg[1];
 	}
+#endif
 
 	*mapp = map;

include/of_plat.h new file +22 -0

new file mode 100644
index 0000000..0badabb
@@ -0,0 +1,22 @@
+/*
+ * (C) Copyright 2017 Rockchip Electronics Co., Ltd
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#ifndef __of_plat_h
+#define __of_plat_h
+
+#include "libfdt_env.h"
+/**
+ * Get a variable-sized number from a property
+ *
+ * This reads a number from one or more cells.
+ *
+ * @param ptr	Pointer to property
+ * @param cells	Number of cells containing the number
+ * @return the value in the cells
+ */
+u64 of_plat_get_number(const fdt32_t *ptr, unsigned int cells);
+
+#endif

lib/Makefile +3 -0

index 23e9f1e..a988965 100644
@@ -55,6 +55,9 @@ obj-$(CONFIG_$(SPL_)OF_CONTROL) += fdtdec_common.o
 obj-$(CONFIG_$(SPL_)OF_CONTROL) += fdtdec.o
 endif
 
+ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_SPL_OF_PLATDATA),yy)
+obj-$(CONFIG_$(SPL_)OF_CONTROL) += of_plat.o
+endif
 ifdef CONFIG_SPL_BUILD
 obj-$(CONFIG_SPL_YMODEM_SUPPORT) += crc16.o
 obj-$(CONFIG_SPL_NET_SUPPORT) += net_utils.o

lib/of_plat.c new file +17 -0

new file mode 100644
index 0000000..cf1cd1e
@@ -0,0 +1,17 @@
+/*
+ * (C) Copyright 2017 Rockchip Electronics Co., Ltd
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#include "of_plat.h"
+
+u64 of_plat_get_number(const u32 *ptr, unsigned int cells)
+{
+	u64 number = 0;
+
+	while (cells--)
+		number = (number << 32) | (*ptr++);
+
+	return number;
+}

Signature · 1 line

1.9.1

Heiko Stuebner

2017-05-05 13:10 UTC #1 lore raw

PATCH v3 1/1 regmap: add support for address cell 2

Hi Kever,

Am Freitag, 5. Mai 2017, 10:39:35 CEST schrieb Kever Yang:

Quoted 4 lines

ARM64 is using 64bit address which address cell is 2 instead of 1, update to support it when of-platdata enabled.
Signed-off-by Kever Yang <[email protected]>

This helps make OF_PLATDATA work on my firefly-rk3399 so yay :-), but I don't think it's that easy to solve, see below:

Quoted 29 lines

---

Changes in v3:
- move of_plat_get_number() into lib/of_plat.c

Changes in v2:
- rename the fdtdec_get_number() to of_plat_get_number()

 drivers/core/regmap.c |  9 +++++++++
 include/of_plat.h     | 22 ++++++++++++++++++++++
 lib/Makefile          |  3 +++
 lib/of_plat.c         | 17 +++++++++++++++++
 4 files changed, 51 insertions(+)
 create mode 100644 include/of_plat.h
 create mode 100644 lib/of_plat.c

drivers/core/regmap.c +1 -0

index 3bec3df..c03279e 100644
@@ -12,6 +12,7 @@
 #include <malloc.h>
 #include <mapmem.h>
 #include <regmap.h>
+#include <of_plat.h>
 
 #include <asm/io.h>

drivers/core/regmap.c int regmap_init_mem_platdata(struct udevice *dev, u32 *reg, int count, · line 58

Quoted 9 lines

drivers/core/regmap.c +6 -0

@@ -49,11 +50,19 @@ int regmap_init_mem_platdata(struct udevice *dev, u32 *reg, int count,
 	if (!map)
 		return -ENOMEM;
 
+#ifdef CONFIG_PHYS_64BIT
+	map->base = of_plat_get_number(reg, 2);
+	for (range = map->range; count > 0; reg += 4, range++, count--) {
+		range->start = of_plat_get_number(reg, 2);
+		range->size = of_plat_get_number(reg + 2, 2);
+	}

I may just be missing something, but how can you be sure that the cell-size is always 2?

For example, there were discussions about 64bit platforms not really needing to add all the 0x0 elements, when the whole io-registers are well below the 4GB mark and for example at least one sunxi also uses this, see for example:
allwinner/sun50i-a64.dtsi, altera/socfpga_stratix10.dtsi, broadcom/bcm283x.dtsi and problably more using #address-cells = <1>, #size-cells = <1> for their memory mapped io.

And from what I've seen dtoc simply converts the reg property and just ignores #address-cells and #size-cells (or I'm overlooking something).

Possible solutions that come to mind would be make dtoc also convert #address-cells and #size-cells, making regmap and everybody check it or alternatively make dtoc convert regs to cell-size 2 in all cases when CONFIG_PHYS_64BIT is set.

Heiko

Kever Yang

author

2017-05-08 01:45 UTC #2 lore raw

PATCH v3 1/1 regmap: add support for address cell 2

Hi Heiko,

Thanks for your comments.

Quoted 59 lines · Heiko Stuebner

Hi Kever,
Am Freitag, 5. Mai 2017, 10:39:35 CEST schrieb Kever Yang:
Quoted 4 lines
ARM64 is using 64bit address which address cell is 2 instead of 1, update to support it when of-platdata enabled.
Signed-off-by Kever Yang <[email protected]>
This helps make OF_PLATDATA work on my firefly-rk3399 so yay :-), but I don't think it's that easy to solve, see below:
Quoted 29 lines
---
Changes in v3:
- move of_plat_get_number() into lib/of_plat.c
Changes in v2:
- rename the fdtdec_get_number() to of_plat_get_number()
  drivers/core/regmap.c |  9 +++++++++
  include/of_plat.h     | 22 ++++++++++++++++++++++
  lib/Makefile          |  3 +++
  lib/of_plat.c         | 17 +++++++++++++++++
  4 files changed, 51 insertions(+)
create mode 100644 include/of_plat.h
create mode 100644 lib/of_plat.c
drivers/core/regmap.c +1 -0
index 3bec3df..c03279e 100644
@@ -12,6 +12,7 @@
  #include <malloc.h>
  #include <mapmem.h>
  #include <regmap.h>
+#include <of_plat.h>
 
  #include <asm/io.h>
 
drivers/core/regmap.c int regmap_init_mem_platdata(struct udevice *dev, u32 *reg, int count, · line 58
Quoted 9 lines
drivers/core/regmap.c +6 -0
@@ -49,11 +50,19 @@ int regmap_init_mem_platdata(struct udevice *dev, u32 *reg, int count,
  	if (!map)
  		return -ENOMEM;
 
+#ifdef CONFIG_PHYS_64BIT
+	map->base = of_plat_get_number(reg, 2);
+	for (range = map->range; count > 0; reg += 4, range++, count--) {
+		range->start = of_plat_get_number(reg, 2);
+		range->size = of_plat_get_number(reg + 2, 2);
+	}
I may just be missing something, but how can you be sure that the cell-size is always 2?
For example, there were discussions about 64bit platforms not really needing to add all the 0x0 elements, when the whole io-registers are well below the 4GB mark and for example at least one sunxi also uses this, see for example:
allwinner/sun50i-a64.dtsi, altera/socfpga_stratix10.dtsi, broadcom/bcm283x.dtsi and problably more using #address-cells = <1>, #size-cells = <1> for their memory mapped io.

Quoted 7 lines

And from what I've seen dtoc simply converts the reg property and just ignores #address-cells and #size-cells (or I'm overlooking something).
Possible solutions that come to mind would be make dtoc also convert #address-cells and #size-cells, making regmap and everybody check it or alternatively make dtoc convert regs to cell-size 2 in all cases when CONFIG_PHYS_64BIT is set.

@Simon, could you take a look about this issue, I really not good at dtoc and libfdt, I think maybe we can re-use fdtdec_get_number() if dtoc do not do the fdt32_to_cpu() int dtoc?

Thanks,
- Kever

Quoted 5 lines

Heiko

Simon Glass

2017-06-19 04:11 UTC #3 lore raw

PATCH v3 1/1 regmap: add support for address cell 2

Hi Kever,

Quoted 78 lines · Kever Yang

Hi Heiko,
Thanks for your comments.
Quoted 62 lines · Heiko Stuebner
Hi Kever,
Am Freitag, 5. Mai 2017, 10:39:35 CEST schrieb Kever Yang:
Quoted 5 lines
ARM64 is using 64bit address which address cell is 2 instead of 1, update to support it when of-platdata enabled.
Signed-off-by Kever Yang <[email protected]>
This helps make OF_PLATDATA work on my firefly-rk3399 so yay :-), but I don't think it's that easy to solve, see below:
Quoted 31 lines
---
Changes in v3:
- move of_plat_get_number() into lib/of_plat.c
Changes in v2:
- rename the fdtdec_get_number() to of_plat_get_number()
  drivers/core/regmap.c |  9 +++++++++
  include/of_plat.h     | 22 ++++++++++++++++++++++
  lib/Makefile          |  3 +++
  lib/of_plat.c         | 17 +++++++++++++++++
  4 files changed, 51 insertions(+)
create mode 100644 include/of_plat.h
create mode 100644 lib/of_plat.c
drivers/core/regmap.c +1 -0
index 3bec3df..c03279e 100644
@@ -12,6 +12,7 @@
  #include <malloc.h>
  #include <mapmem.h>
  #include <regmap.h>
+#include <of_plat.h>
    #include <asm/io.h>
  @@ -49,11 +50,19 @@ int regmap_init_mem_platdata(struct udevice *dev,
 u32 *reg, int count,
        if (!map)
                return -ENOMEM;
+#ifdef CONFIG_PHYS_64BIT
drivers/core/regmap.c int regmap_init_mem_platdata(struct udevice *dev, u32 *reg, int count, · line 58
Quoted 5 lines
drivers/core/regmap.c +5 -0
+       map->base = of_plat_get_number(reg, 2);
+       for (range = map->range; count > 0; reg += 4, range++, count--) {
+               range->start = of_plat_get_number(reg, 2);
+               range->size = of_plat_get_number(reg + 2, 2);
+       }
I may just be missing something, but how can you be sure that the cell-size
is always 2?
For example, there were discussions about 64bit platforms not really needing to add all the 0x0 elements, when the whole io-registers are well below the 4GB mark and for example at least one sunxi also uses this, see for example:
allwinner/sun50i-a64.dtsi, altera/socfpga_stratix10.dtsi, broadcom/bcm283x.dtsi and problably more using #address-cells = <1>, #size-cells = <1> for their memory mapped io.
Quoted 7 lines
And from what I've seen dtoc simply converts the reg property and just ignores #address-cells and #size-cells (or I'm overlooking something).
Possible solutions that come to mind would be make dtoc also convert #address-cells and #size-cells, making regmap and everybody check it or alternatively make dtoc convert regs to cell-size 2 in all cases when CONFIG_PHYS_64BIT is set.

Quoted 3 lines

@Simon, could you take a look about this issue, I really not good at dtoc and libfdt, I think maybe we can re-use fdtdec_get_number() if dtoc do not do the fdt32_to_cpu() int dtoc?

I think the best solution is the second one above. However dtoc is a bit of a pain to change at the moment. I've sent a series to clean it up.

I'll hopefully take a look at the above in the next few weeks.

Regards,
Simon

Simon Glass

2017-06-20 18:26 UTC #4 lore raw

PATCH v3 1/1 regmap: add support for address cell 2

Hi Kever,

Quoted 86 lines · Simon Glass

Hi Kever,
Quoted 78 lines · Kever Yang
Hi Heiko,
Thanks for your comments.
Quoted 62 lines · Heiko Stuebner
Hi Kever,
Am Freitag, 5. Mai 2017, 10:39:35 CEST schrieb Kever Yang:
Quoted 5 lines
ARM64 is using 64bit address which address cell is 2 instead of 1, update to support it when of-platdata enabled.
Signed-off-by Kever Yang <[email protected]>
This helps make OF_PLATDATA work on my firefly-rk3399 so yay :-), but I don't think it's that easy to solve, see below:
Quoted 31 lines
---
Changes in v3:
- move of_plat_get_number() into lib/of_plat.c
Changes in v2:
- rename the fdtdec_get_number() to of_plat_get_number()
  drivers/core/regmap.c |  9 +++++++++
  include/of_plat.h     | 22 ++++++++++++++++++++++
  lib/Makefile          |  3 +++
  lib/of_plat.c         | 17 +++++++++++++++++
  4 files changed, 51 insertions(+)
create mode 100644 include/of_plat.h
create mode 100644 lib/of_plat.c
drivers/core/regmap.c +1 -0
index 3bec3df..c03279e 100644
@@ -12,6 +12,7 @@
  #include <malloc.h>
  #include <mapmem.h>
  #include <regmap.h>
+#include <of_plat.h>
    #include <asm/io.h>
  @@ -49,11 +50,19 @@ int regmap_init_mem_platdata(struct udevice *dev,
 u32 *reg, int count,
        if (!map)
                return -ENOMEM;
+#ifdef CONFIG_PHYS_64BIT
drivers/core/regmap.c int regmap_init_mem_platdata(struct udevice *dev, u32 *reg, int count, · line 58
Quoted 5 lines
drivers/core/regmap.c +5 -0
+       map->base = of_plat_get_number(reg, 2);
+       for (range = map->range; count > 0; reg += 4, range++, count--) {
+               range->start = of_plat_get_number(reg, 2);
+               range->size = of_plat_get_number(reg + 2, 2);
+       }
I may just be missing something, but how can you be sure that the cell-size
is always 2?
For example, there were discussions about 64bit platforms not really needing to add all the 0x0 elements, when the whole io-registers are well below the 4GB mark and for example at least one sunxi also uses this, see for example:
allwinner/sun50i-a64.dtsi, altera/socfpga_stratix10.dtsi, broadcom/bcm283x.dtsi and problably more using #address-cells = <1>, #size-cells = <1> for their memory mapped io.
Quoted 7 lines
And from what I've seen dtoc simply converts the reg property and just ignores #address-cells and #size-cells (or I'm overlooking something).
Possible solutions that come to mind would be make dtoc also convert #address-cells and #size-cells, making regmap and everybody check it or alternatively make dtoc convert regs to cell-size 2 in all cases when CONFIG_PHYS_64BIT is set.
Quoted 3 lines
@Simon, could you take a look about this issue, I really not good at dtoc and libfdt, I think maybe we can re-use fdtdec_get_number() if dtoc do not do the fdt32_to_cpu() int dtoc?

Quoted 5 lines

I think the best solution is the second one above. However dtoc is a bit of a pain to change at the moment. I've sent a series to clean it up.
I'll hopefully take a look at the above in the next few weeks.

Just to follow up, I sent a few patches for this yesterday.

Regards,
Simon