diff mbox

mtd: physmap_of: fixup gemini/versatile dependencies

Message ID 20170209020937.62339-1-computersforpeace@gmail.com
State Accepted
Commit 4f04f68e1598087a8110e8946bfb321eebe115c9
Headers show

Commit Message

Brian Norris Feb. 9, 2017, 2:09 a.m. UTC
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 <linus.walleij@linaro.org>
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 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(-)

Comments

Brian Norris Feb. 10, 2017, 2:38 a.m. UTC | #1
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.

On Wed, Feb 08, 2017 at 06:09:37PM -0800, Brian Norris wrote:
> 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 <linus.walleij@linaro.org>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
>  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(-)
> 
> diff --git a/drivers/mtd/maps/Makefile b/drivers/mtd/maps/Makefile
> index 2fec1e0c2371..aef1846b4de2 100644
> --- a/drivers/mtd/maps/Makefile
> +++ b/drivers/mtd/maps/Makefile
> @@ -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
> diff --git a/drivers/mtd/maps/physmap_of_gemini.c b/drivers/mtd/maps/physmap_of_gemini.c
> index e99db772143b..9d371cd728ea 100644
> --- a/drivers/mtd/maps/physmap_of_gemini.c
> +++ b/drivers/mtd/maps/physmap_of_gemini.c
> @@ -115,4 +115,3 @@ int of_flash_probe_gemini(struct platform_device *pdev,
>  
>  	return 0;
>  }
> -EXPORT_SYMBOL_GPL(of_flash_probe_gemini);
> diff --git a/drivers/mtd/maps/physmap_of_versatile.c b/drivers/mtd/maps/physmap_of_versatile.c
> index 0f39b2a015f4..8c6ccded9be8 100644
> --- a/drivers/mtd/maps/physmap_of_versatile.c
> +++ b/drivers/mtd/maps/physmap_of_versatile.c
> @@ -252,4 +252,3 @@ int of_flash_probe_versatile(struct platform_device *pdev,
>  
>  	return 0;
>  }
> -EXPORT_SYMBOL_GPL(of_flash_probe_versatile);
> -- 
> 2.11.0.483.g087da7b7c-goog
>
Linus Walleij Feb. 10, 2017, 12:19 p.m. UTC | #2
On Thu, Feb 9, 2017 at 3:09 AM, Brian Norris
<computersforpeace@gmail.com> wrote:

> 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 <linus.walleij@linaro.org>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

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 Feb. 10, 2017, 6:03 p.m. UTC | #3
On Fri, Feb 10, 2017 at 01:19:15PM +0100, Linus Walleij wrote:
> On Thu, Feb 9, 2017 at 3:09 AM, Brian Norris
> <computersforpeace@gmail.com> wrote:
> 
> > 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 <linus.walleij@linaro.org>
> > Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> 
> 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.

> 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.

Sounds good.

> And by the way: thanks for merging this!

No problem.

Brian
diff mbox

Patch

diff --git a/drivers/mtd/maps/Makefile b/drivers/mtd/maps/Makefile
index 2fec1e0c2371..aef1846b4de2 100644
--- a/drivers/mtd/maps/Makefile
+++ b/drivers/mtd/maps/Makefile
@@ -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
diff --git a/drivers/mtd/maps/physmap_of_gemini.c b/drivers/mtd/maps/physmap_of_gemini.c
index e99db772143b..9d371cd728ea 100644
--- a/drivers/mtd/maps/physmap_of_gemini.c
+++ b/drivers/mtd/maps/physmap_of_gemini.c
@@ -115,4 +115,3 @@  int of_flash_probe_gemini(struct platform_device *pdev,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(of_flash_probe_gemini);
diff --git a/drivers/mtd/maps/physmap_of_versatile.c b/drivers/mtd/maps/physmap_of_versatile.c
index 0f39b2a015f4..8c6ccded9be8 100644
--- a/drivers/mtd/maps/physmap_of_versatile.c
+++ b/drivers/mtd/maps/physmap_of_versatile.c
@@ -252,4 +252,3 @@  int of_flash_probe_versatile(struct platform_device *pdev,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(of_flash_probe_versatile);