mtd: physmap_of: fixup gemini/versatile dependencies

PATCH In review Brian Norris 2 participants 4 messages Feb 9, 2017 – Feb 10, 2017 Memory technology devices (MTD)

Share & tools

Brian Norris

author

2017-02-09 02:11 UTC #0 lore raw

PATCH 1/1 mtd: physmap_of: fixup gemini/versatile dependencies

physmap_of sort of depends on the gemini and versatile modules (when they're enabled), but this isn't expressed in Kconfig. Let's just merge the modules all together, when enabled. Then we can avoid exporting a few symbols, and the versatile and gemini code can now be modular again (the below commit accidentally made them built-in only).

Resolves errors like this:

ERROR: "of_flash_probe_versatile" [drivers/mtd/maps/physmap_of.ko] undefined!
ERROR: "of_flash_probe_gemini" [drivers/mtd/maps/physmap_of.ko] undefined!

Fixes 56ff337ea433 ("mtd: physmap_of: add a hook for Gemini flash probing")

Cc Linus Walleij <[email protected]>

Signed-off-by Brian Norris <[email protected]>

---
 drivers/mtd/maps/Makefile               | 8 ++++++--
 drivers/mtd/maps/physmap_of_gemini.c    | 1 -
 drivers/mtd/maps/physmap_of_versatile.c | 1 -
 3 files changed, 6 insertions(+), 4 deletions(-)

drivers/mtd/maps/Makefile +6 -2

index 2fec1e0c2371..aef1846b4de2 100644
@@ -17,9 +17,13 @@ obj-$(CONFIG_MTD_CK804XROM)	+= ck804xrom.o
 obj-$(CONFIG_MTD_TSUNAMI)	+= tsunami_flash.o
 obj-$(CONFIG_MTD_PXA2XX)	+= pxa2xx-flash.o
 obj-$(CONFIG_MTD_PHYSMAP)	+= physmap.o
+ifdef CONFIG_MTD_PHYSMAP_OF_VERSATILE
+physmap_of-objs += physmap_of_versatile.o
+endif
+ifdef CONFIG_MTD_PHYSMAP_OF_GEMINI
+physmap_of-objs += physmap_of_gemini.o
+endif
 obj-$(CONFIG_MTD_PHYSMAP_OF)	+= physmap_of.o
-obj-$(CONFIG_MTD_PHYSMAP_OF_VERSATILE)	+= physmap_of_versatile.o
-obj-$(CONFIG_MTD_PHYSMAP_OF_GEMINI)	+= physmap_of_gemini.o
 obj-$(CONFIG_MTD_PISMO)		+= pismo.o
 obj-$(CONFIG_MTD_PMC_MSP_EVM)   += pmcmsp-flash.o
 obj-$(CONFIG_MTD_PCMCIA)	+= pcmciamtd.o

drivers/mtd/maps/physmap_of_gemini.c +0 -1

index e99db772143b..9d371cd728ea 100644
@@ -115,4 +115,3 @@ int of_flash_probe_gemini(struct platform_device *pdev,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(of_flash_probe_gemini);

drivers/mtd/maps/physmap_of_versatile.c +0 -1

index 0f39b2a015f4..8c6ccded9be8 100644
@@ -252,4 +252,3 @@ int of_flash_probe_versatile(struct platform_device *pdev,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(of_flash_probe_versatile);

Signature · 1 line

2.11.0.483.g087da7b7c-goog

Brian Norris

author

2017-02-10 02:38 UTC #1 lore raw

Hi Linus,

Any thoughts? It's either revert your patches, or apply something like this. I'm going to need to decide that in the next <24 hours or so.

Quoted 61 lines · Brian Norris

physmap_of sort of depends on the gemini and versatile modules (when they're enabled), but this isn't expressed in Kconfig. Let's just merge the modules all together, when enabled. Then we can avoid exporting a few symbols, and the versatile and gemini code can now be modular again (the below commit accidentally made them built-in only).
Resolves errors like this:
ERROR: "of_flash_probe_versatile" [drivers/mtd/maps/physmap_of.ko] undefined!
ERROR: "of_flash_probe_gemini" [drivers/mtd/maps/physmap_of.ko] undefined!
Fixes 56ff337ea433 ("mtd: physmap_of: add a hook for Gemini flash probing")
Cc Linus Walleij <[email protected]>
Signed-off-by Brian Norris <[email protected]>
---
 drivers/mtd/maps/Makefile               | 8 ++++++--
 drivers/mtd/maps/physmap_of_gemini.c    | 1 -
 drivers/mtd/maps/physmap_of_versatile.c | 1 -
 3 files changed, 6 insertions(+), 4 deletions(-)
drivers/mtd/maps/Makefile +6 -2
index 2fec1e0c2371..aef1846b4de2 100644
@@ -17,9 +17,13 @@ obj-$(CONFIG_MTD_CK804XROM)	+= ck804xrom.o
 obj-$(CONFIG_MTD_TSUNAMI)	+= tsunami_flash.o
 obj-$(CONFIG_MTD_PXA2XX)	+= pxa2xx-flash.o
 obj-$(CONFIG_MTD_PHYSMAP)	+= physmap.o
+ifdef CONFIG_MTD_PHYSMAP_OF_VERSATILE
+physmap_of-objs += physmap_of_versatile.o
+endif
+ifdef CONFIG_MTD_PHYSMAP_OF_GEMINI
+physmap_of-objs += physmap_of_gemini.o
+endif
 obj-$(CONFIG_MTD_PHYSMAP_OF)	+= physmap_of.o
-obj-$(CONFIG_MTD_PHYSMAP_OF_VERSATILE)	+= physmap_of_versatile.o
-obj-$(CONFIG_MTD_PHYSMAP_OF_GEMINI)	+= physmap_of_gemini.o
 obj-$(CONFIG_MTD_PISMO)		+= pismo.o
 obj-$(CONFIG_MTD_PMC_MSP_EVM)   += pmcmsp-flash.o
 obj-$(CONFIG_MTD_PCMCIA)	+= pcmciamtd.o
drivers/mtd/maps/physmap_of_gemini.c +0 -1
index e99db772143b..9d371cd728ea 100644
@@ -115,4 +115,3 @@ int of_flash_probe_gemini(struct platform_device *pdev,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(of_flash_probe_gemini);
drivers/mtd/maps/physmap_of_versatile.c +0 -1
index 0f39b2a015f4..8c6ccded9be8 100644
@@ -252,4 +252,3 @@ int of_flash_probe_versatile(struct platform_device *pdev,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(of_flash_probe_versatile);
Signature · 1 line
2.11.0.483.g087da7b7c-goog

Linus Walleij

Reviewed-by on v1 1/1

2017-02-10 12:19 UTC #2 lore raw

In reply to Brian Norris

Quoted 6 lines · Brian Norris

physmap_of sort of depends on the gemini and versatile modules (when they're enabled), but this isn't expressed in Kconfig. Let's just merge the modules all together, when enabled. Then we can avoid exporting a few symbols, and the versatile and gemini code can now be modular again (the below commit accidentally made them built-in only).

Quoted 8 lines

Resolves errors like this:
ERROR: "of_flash_probe_versatile" [drivers/mtd/maps/physmap_of.ko] undefined!
ERROR: "of_flash_probe_gemini" [drivers/mtd/maps/physmap_of.ko] undefined!
Fixes 56ff337ea433 ("mtd: physmap_of: add a hook for Gemini flash probing")
Cc Linus Walleij <[email protected]>
Signed-off-by Brian Norris <[email protected]>

Reviewed-by Linus Walleij <[email protected]>

Sorry for not responding quicker.

I hacked up this not entirely optimal solution to the same problem but I think what you're doing is better:

config MTD_PHYSMAP_OF_VERSATILE

-       bool "Support ARM Versatile physmap OF"
-       depends on MTD_PHYSMAP_OF
-       depends on MFD_SYSCON
+       bool "ARM Versatile OF-based physical memory map handling"
+       depends on MTD_PHYSMAP_OF=y
+       depends on MFD_SYSCON=y

default y if (ARCH_INTEGRATOR || ARCH_VERSATILE || ARCH_REALVIEW) help
This provides some extra DT physmap parsing for the ARM Versatile platforms, basically to add a VPP (write protection) callback so the flash can be taken out of write protection.

+config MTD_PHYSMAP_OF_GEMINI

+       bool "Cortina Gemini OF-based physical memory map handling"
+       depends on MTD_PHYSMAP_OF=y
+       depends on MFD_SYSCON=y
+       default ARCH_GEMINI
+       help
+         This provides some extra DT physmap parsing for the Gemini
+         platforms, some detection and setting up parallel mode on the
+         external interface.

I.e. I made the modules be only accessible when compiled-in.

Either solution is fine with me, but someone else might be annoyed by my solution so let's go with yours.

And by the way: thanks for merging this!

Yours,
Linus Walleij

Brian Norris

author Applied

2017-02-10 18:03 UTC #3 lore raw

Quoted 14 lines · Linus Walleij

Quoted 11 lines · Brian Norris
physmap_of sort of depends on the gemini and versatile modules (when they're enabled), but this isn't expressed in Kconfig. Let's just merge the modules all together, when enabled. Then we can avoid exporting a few symbols, and the versatile and gemini code can now be modular again (the below commit accidentally made them built-in only).
Resolves errors like this:
ERROR: "of_flash_probe_versatile" [drivers/mtd/maps/physmap_of.ko] undefined!
ERROR: "of_flash_probe_gemini" [drivers/mtd/maps/physmap_of.ko] undefined!

Quoted 7 lines

Quoted 3 lines
Fixes 56ff337ea433 ("mtd: physmap_of: add a hook for Gemini flash probing")
Cc Linus Walleij <[email protected]>
Signed-off-by Brian Norris <[email protected]>
Reviewed-by Linus Walleij <[email protected]>
Sorry for not responding quicker.

Eh, no problem. It's mostly my fault for batch processing within the last week before the merge window :)

I've applied my solution to l2-mtd.git. Thanks for reviewing.

Quoted 26 lines

I hacked up this not entirely optimal solution to the same problem but I think what you're doing is better:

config MTD_PHYSMAP_OF_VERSATILE

(quoted diff) +12 -3

-       bool "Support ARM Versatile physmap OF"
-       depends on MTD_PHYSMAP_OF
-       depends on MFD_SYSCON
+       bool "ARM Versatile OF-based physical memory map handling"
+       depends on MTD_PHYSMAP_OF=y
+       depends on MFD_SYSCON=y
        default y if (ARCH_INTEGRATOR || ARCH_VERSATILE || ARCH_REALVIEW)
        help
          This provides some extra DT physmap parsing for the ARM Versatile
          platforms, basically to add a VPP (write protection) callback so
          the flash can be taken out of write protection.
 
+config MTD_PHYSMAP_OF_GEMINI
+       bool "Cortina Gemini OF-based physical memory map handling"
+       depends on MTD_PHYSMAP_OF=y
+       depends on MFD_SYSCON=y
+       default ARCH_GEMINI
+       help
+         This provides some extra DT physmap parsing for the Gemini
+         platforms, some detection and setting up parallel mode on the
+         external interface.

Quoted 4 lines

I.e. I made the modules be only accessible when compiled-in.
Either solution is fine with me, but someone else might be annoyed by my solution so let's go with yours.

Sounds good.

Quoted 1 line

And by the way: thanks for merging this!

No problem.

Brian