[PATCH 1/2 v2] mtd: maps: Leave assigned complex mappings

[PATCH 1/2 v2] mtd: maps: Leave assigned complex mappings

[Linus Walleij]

The simple_map_init() may need to be called with some
function pointers already assigned for complex mappings,
just bail out if complex handlers have already been
assigned.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Rebase on latest MTD development branch
- Use a new approach as the code changed under me
---
 drivers/mtd/maps/map_funcs.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/maps/map_funcs.c b/drivers/mtd/maps/map_funcs.c
index 3f268370eeca..60e132293e1c 100644
--- a/drivers/mtd/maps/map_funcs.c
+++ b/drivers/mtd/maps/map_funcs.c
@@ -31,6 +31,10 @@ static void __xipram simple_map_copy_to(struct map_info *map, unsigned long to,
 
 void simple_map_init(struct map_info *map)
 {
+	/* Complex map functions already assigned */
+	if (map->read)
+		return;
+
 	BUG_ON(!map_bankwidth_supported(map->bankwidth));
 
 	map->read = simple_map_read;
-- 
2.19.1

[PATCH 2/2 v2] mtd: physmap_of_gemini: Handle pin control

[Linus Walleij]

This enables the complex mapping for the Gemini and kicks in
custom read/write functions that will wrap the existing
simple functions in calls to enable/disable the parallel
flash pins using pin controls.

This is necessary on some hardware such as the D-Link
DIR-685 where all flash pins are patched in/out at the same
time, but some of the flash pins are in practice unused by
the flash and have anyway been reused as GPIO.

This concerns specifically CE1 on the Gemini. There is only
one flash chip, so only CE0 is used, and the line for CE1
has been reused as chip select for the emulated SPI port
connected to the display. If we try to use the same lines
for flash and GPIO at the same time, one of them will loose:
the GPIO line will disappear because it gets disconnected
from the pin when the flash group is muxed out.

Fix this by introducing two pin control states named simply
"enabled" and "disabled" and only enable the flash lines
when absolutely necessary (during read/write/copy). This
way, they are available for GPIO at all other times and
the display works.

Collect all the state variables in a struct named
struct gemini_flash and allocate this struct at probe
time.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Rebase on latest MTD development branch
---
 drivers/mtd/maps/Kconfig          |   1 +
 drivers/mtd/maps/physmap-gemini.c | 110 +++++++++++++++++++++++++++++-
 2 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig
index 2edd8bcdbe1c..e0cf869c8544 100644
--- a/drivers/mtd/maps/Kconfig
+++ b/drivers/mtd/maps/Kconfig
@@ -88,6 +88,7 @@ config MTD_PHYSMAP_GEMINI
 	bool "Cortina Gemini OF-based physical memory map handling"
 	depends on MTD_PHYSMAP_OF
 	depends on MFD_SYSCON
+	select MTD_COMPLEX_MAPPINGS
 	default ARCH_GEMINI
 	help
 	  This provides some extra DT physmap parsing for the Gemini
diff --git a/drivers/mtd/maps/physmap-gemini.c b/drivers/mtd/maps/physmap-gemini.c
index 1cf128a0526d..60775b208fc9 100644
--- a/drivers/mtd/maps/physmap-gemini.c
+++ b/drivers/mtd/maps/physmap-gemini.c
@@ -10,9 +10,11 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/mtd/map.h>
+#include <linux/mtd/xip.h>
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
 #include <linux/bitops.h>
+#include <linux/pinctrl/consumer.h>
 #include "physmap-gemini.h"
 
 /*
@@ -44,6 +46,82 @@
 
 #define FLASH_PARALLEL_HIGH_PIN_CNT	(1 << 20)	/* else low pin cnt */
 
+static const struct of_device_id syscon_match[] = {
+	{ .compatible = "cortina,gemini-syscon" },
+	{ },
+};
+
+struct gemini_flash {
+	struct device *dev;
+	struct pinctrl *p;
+	struct pinctrl_state *enabled_state;
+	struct pinctrl_state *disabled_state;
+};
+
+/* Static local state */
+static struct gemini_flash *gf;
+
+static void gemini_flash_enable_pins(void)
+{
+	int ret;
+
+	if (IS_ERR(gf->enabled_state))
+		return;
+	ret = pinctrl_select_state(gf->p, gf->enabled_state);
+	if (ret)
+		dev_err(gf->dev, "failed to enable pins\n");
+}
+
+static void gemini_flash_disable_pins(void)
+{
+	int ret;
+
+	if (IS_ERR(gf->disabled_state))
+		return;
+	ret = pinctrl_select_state(gf->p, gf->disabled_state);
+	if (ret)
+		dev_err(gf->dev, "failed to disable pins\n");
+}
+
+static map_word __xipram gemini_flash_map_read(struct map_info *map,
+					       unsigned long ofs)
+{
+	map_word __xipram ret;
+
+	gemini_flash_enable_pins();
+	ret = inline_map_read(map, ofs);
+	gemini_flash_disable_pins();
+
+	return ret;
+}
+
+static void __xipram gemini_flash_map_write(struct map_info *map,
+					    const map_word datum,
+					    unsigned long ofs)
+{
+	gemini_flash_enable_pins();
+	inline_map_write(map, datum, ofs);
+	gemini_flash_disable_pins();
+}
+
+static void __xipram gemini_flash_map_copy_from(struct map_info *map,
+						void *to, unsigned long from,
+						ssize_t len)
+{
+	gemini_flash_enable_pins();
+	inline_map_copy_from(map, to, from, len);
+	gemini_flash_disable_pins();
+}
+
+static void __xipram gemini_flash_map_copy_to(struct map_info *map,
+					      unsigned long to,
+					      const void *from, ssize_t len)
+{
+	gemini_flash_enable_pins();
+	inline_map_copy_to(map, to, from, len);
+	gemini_flash_disable_pins();
+}
+
 int of_flash_probe_gemini(struct platform_device *pdev,
 			  struct device_node *np,
 			  struct map_info *map)
@@ -57,6 +135,11 @@ int of_flash_probe_gemini(struct platform_device *pdev,
 	if (!of_device_is_compatible(np, "cortina,gemini-flash"))
 		return 0;
 
+	gf = devm_kzalloc(dev, sizeof(*gf), GFP_KERNEL);
+	if (!gf)
+		return -ENOMEM;
+	gf->dev = dev;
+
 	rmap = syscon_regmap_lookup_by_phandle(np, "syscon");
 	if (IS_ERR(rmap)) {
 		dev_err(dev, "no syscon\n");
@@ -91,7 +174,32 @@ int of_flash_probe_gemini(struct platform_device *pdev,
 				 map->bankwidth * 8);
 	}
 
-	dev_info(&pdev->dev, "initialized Gemini-specific physmap control\n");
+	gf->p = devm_pinctrl_get(dev);
+	if (IS_ERR(gf->p)) {
+		dev_err(dev, "no pinctrl handle\n");
+		ret = PTR_ERR(gf->p);
+		return ret;
+	}
+
+	gf->enabled_state = pinctrl_lookup_state(gf->p, "enabled");
+	if (IS_ERR(gf->enabled_state))
+		dev_err(dev, "no enabled pin control state\n");
+
+	gf->disabled_state = pinctrl_lookup_state(gf->p, "disabled");
+	if (IS_ERR(gf->enabled_state)) {
+		dev_err(dev, "no disabled pin control state\n");
+	} else {
+		ret = pinctrl_select_state(gf->p, gf->disabled_state);
+		if (ret)
+			dev_err(gf->dev, "failed to disable pins\n");
+	}
+
+	map->read = gemini_flash_map_read;
+	map->write = gemini_flash_map_write;
+	map->copy_from = gemini_flash_map_copy_from;
+	map->copy_to = gemini_flash_map_copy_to;
+
+	dev_info(dev, "initialized Gemini-specific physmap control\n");
 
 	return 0;
 }
-- 
2.19.1

Re: [PATCH 2/2 v2] mtd: physmap_of_gemini: Handle pin control

[Boris Brezillon]

On Mon, 12 Nov 2018 22:07:29 +0100
Linus Walleij <linus.walleij@linaro.org> wrote:

> This enables the complex mapping for the Gemini and kicks in
> custom read/write functions that will wrap the existing
> simple functions in calls to enable/disable the parallel
> flash pins using pin controls.
> 
> This is necessary on some hardware such as the D-Link
> DIR-685 where all flash pins are patched in/out at the same
> time, but some of the flash pins are in practice unused by
> the flash and have anyway been reused as GPIO.
> 
> This concerns specifically CE1 on the Gemini. There is only
> one flash chip, so only CE0 is used, and the line for CE1
> has been reused as chip select for the emulated SPI port
> connected to the display. If we try to use the same lines
> for flash and GPIO at the same time, one of them will loose:
> the GPIO line will disappear because it gets disconnected
> from the pin when the flash group is muxed out.
> 
> Fix this by introducing two pin control states named simply
> "enabled" and "disabled" and only enable the flash lines
> when absolutely necessary (during read/write/copy). This
> way, they are available for GPIO at all other times and
> the display works.

Okay, so your GPIOs might be unavailable when the flash is accessed?
How does that work in practice? Is CE1 muxed as a GPIO on demand? What
happens if the flash and SPI bus are accessed at the same time? Do you
get -EBUSY? Not sure all MTD users retry when they get -EBUSY...

> 
> Collect all the state variables in a struct named
> struct gemini_flash and allocate this struct at probe
> time.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Rebase on latest MTD development branch
> ---
>  drivers/mtd/maps/Kconfig          |   1 +
>  drivers/mtd/maps/physmap-gemini.c | 110 +++++++++++++++++++++++++++++-
>  2 files changed, 110 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig
> index 2edd8bcdbe1c..e0cf869c8544 100644
> --- a/drivers/mtd/maps/Kconfig
> +++ b/drivers/mtd/maps/Kconfig
> @@ -88,6 +88,7 @@ config MTD_PHYSMAP_GEMINI
>  	bool "Cortina Gemini OF-based physical memory map handling"
>  	depends on MTD_PHYSMAP_OF
>  	depends on MFD_SYSCON
> +	select MTD_COMPLEX_MAPPINGS
>  	default ARCH_GEMINI
>  	help
>  	  This provides some extra DT physmap parsing for the Gemini
> diff --git a/drivers/mtd/maps/physmap-gemini.c b/drivers/mtd/maps/physmap-gemini.c
> index 1cf128a0526d..60775b208fc9 100644
> --- a/drivers/mtd/maps/physmap-gemini.c
> +++ b/drivers/mtd/maps/physmap-gemini.c
> @@ -10,9 +10,11 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/mtd/map.h>
> +#include <linux/mtd/xip.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/regmap.h>
>  #include <linux/bitops.h>
> +#include <linux/pinctrl/consumer.h>
>  #include "physmap-gemini.h"
>  
>  /*
> @@ -44,6 +46,82 @@
>  
>  #define FLASH_PARALLEL_HIGH_PIN_CNT	(1 << 20)	/* else low pin cnt */
>  
> +static const struct of_device_id syscon_match[] = {
> +	{ .compatible = "cortina,gemini-syscon" },
> +	{ },
> +};
> +
> +struct gemini_flash {
> +	struct device *dev;
> +	struct pinctrl *p;
> +	struct pinctrl_state *enabled_state;
> +	struct pinctrl_state *disabled_state;
> +};
> +
> +/* Static local state */
> +static struct gemini_flash *gf;
> +
> +static void gemini_flash_enable_pins(void)
> +{
> +	int ret;
> +
> +	if (IS_ERR(gf->enabled_state))
> +		return;
> +	ret = pinctrl_select_state(gf->p, gf->enabled_state);
> +	if (ret)
> +		dev_err(gf->dev, "failed to enable pins\n");
> +}
> +
> +static void gemini_flash_disable_pins(void)
> +{
> +	int ret;
> +
> +	if (IS_ERR(gf->disabled_state))
> +		return;
> +	ret = pinctrl_select_state(gf->p, gf->disabled_state);
> +	if (ret)
> +		dev_err(gf->dev, "failed to disable pins\n");
> +}
> +
> +static map_word __xipram gemini_flash_map_read(struct map_info *map,
> +					       unsigned long ofs)
> +{
> +	map_word __xipram ret;
> +
> +	gemini_flash_enable_pins();
> +	ret = inline_map_read(map, ofs);
> +	gemini_flash_disable_pins();
> +
> +	return ret;
> +}
> +
> +static void __xipram gemini_flash_map_write(struct map_info *map,
> +					    const map_word datum,
> +					    unsigned long ofs)
> +{
> +	gemini_flash_enable_pins();
> +	inline_map_write(map, datum, ofs);
> +	gemini_flash_disable_pins();
> +}
> +
> +static void __xipram gemini_flash_map_copy_from(struct map_info *map,
> +						void *to, unsigned long from,
> +						ssize_t len)
> +{
> +	gemini_flash_enable_pins();
> +	inline_map_copy_from(map, to, from, len);
> +	gemini_flash_disable_pins();
> +}
> +
> +static void __xipram gemini_flash_map_copy_to(struct map_info *map,
> +					      unsigned long to,
> +					      const void *from, ssize_t len)
> +{
> +	gemini_flash_enable_pins();
> +	inline_map_copy_to(map, to, from, len);
> +	gemini_flash_disable_pins();
> +}
> +
>  int of_flash_probe_gemini(struct platform_device *pdev,
>  			  struct device_node *np,
>  			  struct map_info *map)
> @@ -57,6 +135,11 @@ int of_flash_probe_gemini(struct platform_device *pdev,
>  	if (!of_device_is_compatible(np, "cortina,gemini-flash"))
>  		return 0;
>  
> +	gf = devm_kzalloc(dev, sizeof(*gf), GFP_KERNEL);
> +	if (!gf)
> +		return -ENOMEM;
> +	gf->dev = dev;
> +
>  	rmap = syscon_regmap_lookup_by_phandle(np, "syscon");
>  	if (IS_ERR(rmap)) {
>  		dev_err(dev, "no syscon\n");
> @@ -91,7 +174,32 @@ int of_flash_probe_gemini(struct platform_device *pdev,
>  				 map->bankwidth * 8);
>  	}
>  
> -	dev_info(&pdev->dev, "initialized Gemini-specific physmap control\n");
> +	gf->p = devm_pinctrl_get(dev);
> +	if (IS_ERR(gf->p)) {
> +		dev_err(dev, "no pinctrl handle\n");
> +		ret = PTR_ERR(gf->p);
> +		return ret;
> +	}
> +
> +	gf->enabled_state = pinctrl_lookup_state(gf->p, "enabled");
> +	if (IS_ERR(gf->enabled_state))
> +		dev_err(dev, "no enabled pin control state\n");
> +
> +	gf->disabled_state = pinctrl_lookup_state(gf->p, "disabled");
> +	if (IS_ERR(gf->enabled_state)) {
> +		dev_err(dev, "no disabled pin control state\n");
> +	} else {
> +		ret = pinctrl_select_state(gf->p, gf->disabled_state);
> +		if (ret)
> +			dev_err(gf->dev, "failed to disable pins\n");
> +	}
> +
> +	map->read = gemini_flash_map_read;
> +	map->write = gemini_flash_map_write;
> +	map->copy_from = gemini_flash_map_copy_from;
> +	map->copy_to = gemini_flash_map_copy_to;
> +
> +	dev_info(dev, "initialized Gemini-specific physmap control\n");
>  
>  	return 0;
>  }

Re: [PATCH 2/2 v2] mtd: physmap_of_gemini: Handle pin control

[Linus Walleij]

On Tue, Nov 20, 2018 at 3:36 PM Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> On Mon, 12 Nov 2018 22:07:29 +0100
> Linus Walleij <linus.walleij@linaro.org> wrote:
>
> > This enables the complex mapping for the Gemini and kicks in
> > custom read/write functions that will wrap the existing
> > simple functions in calls to enable/disable the parallel
> > flash pins using pin controls.
> >
> > This is necessary on some hardware such as the D-Link
> > DIR-685 where all flash pins are patched in/out at the same
> > time, but some of the flash pins are in practice unused by
> > the flash and have anyway been reused as GPIO.
> >
> > This concerns specifically CE1 on the Gemini. There is only
> > one flash chip, so only CE0 is used, and the line for CE1
> > has been reused as chip select for the emulated SPI port
> > connected to the display. If we try to use the same lines
> > for flash and GPIO at the same time, one of them will loose:
> > the GPIO line will disappear because it gets disconnected
> > from the pin when the flash group is muxed out.
> >
> > Fix this by introducing two pin control states named simply
> > "enabled" and "disabled" and only enable the flash lines
> > when absolutely necessary (during read/write/copy). This
> > way, they are available for GPIO at all other times and
> > the display works.
>
> Okay, so your GPIOs might be unavailable when the flash is accessed?
>
> How does that work in practice? Is CE1 muxed as a GPIO
> on demand?

Yes, well the actual pin will go into the state of CE1
which default to a low state, the effect is
hardware-defined.

This is not a recommendable type of electronics
engineering if that is what you wonder... it's a way to
patch over design mistakes in the design with
software. (Which is what the vendor tree does.)

In one case (SQ201) the LED connected to them will light up
since it is active low. So the LED goes on during flash
access and gives a kind of "flash access flicker" on top
of whatever it was indicating before.

That LED is marked "power" and I put a heartbeat on it
so it will be brief interruptions of steady light in the
heartbeat. The "only" problem is in UX. The vendor
device exhibits the same strange behavior.

> What happens if the flash and SPI bus are
> accessed at the same time? Do you
> get -EBUSY? Not sure all MTD users retry when they
> get -EBUSY...

The way I set it up flash access "always win", so
no worries for MTD.

If I simulate SPI or any other slow bus on the GPIO
at the same time as we have muxed it out for
flash access, it will have to do error recovery,
because the bytes will not get through.

Ideally I would want to patch the GPIO library to
redirect the access to this line to enable/disable
CE1 so the pin is driven from the flash controller
instead. But since that is essentially an address
bit it looked pretty tricky.

In practice the use cases do not overlap, so
we are fine, I guess if problems still occur the
path of least resistance would simply be to have the
display driver using this SPI line delay and
retry a few times. It's an ugly solution perfectly
fitting the ugly electronics IMO.

Yours,
Linus Walleij

Re: [PATCH 2/2 v2] mtd: physmap_of_gemini: Handle pin control

[Boris Brezillon]

On Thu, 22 Nov 2018 15:10:47 +0100
Linus Walleij <linus.walleij@linaro.org> wrote:

> On Tue, Nov 20, 2018 at 3:36 PM Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> > On Mon, 12 Nov 2018 22:07:29 +0100
> > Linus Walleij <linus.walleij@linaro.org> wrote:
> >  
> > > This enables the complex mapping for the Gemini and kicks in
> > > custom read/write functions that will wrap the existing
> > > simple functions in calls to enable/disable the parallel
> > > flash pins using pin controls.
> > >
> > > This is necessary on some hardware such as the D-Link
> > > DIR-685 where all flash pins are patched in/out at the same
> > > time, but some of the flash pins are in practice unused by
> > > the flash and have anyway been reused as GPIO.
> > >
> > > This concerns specifically CE1 on the Gemini. There is only
> > > one flash chip, so only CE0 is used, and the line for CE1
> > > has been reused as chip select for the emulated SPI port
> > > connected to the display. If we try to use the same lines
> > > for flash and GPIO at the same time, one of them will loose:
> > > the GPIO line will disappear because it gets disconnected
> > > from the pin when the flash group is muxed out.
> > >
> > > Fix this by introducing two pin control states named simply
> > > "enabled" and "disabled" and only enable the flash lines
> > > when absolutely necessary (during read/write/copy). This
> > > way, they are available for GPIO at all other times and
> > > the display works.  
> >
> > Okay, so your GPIOs might be unavailable when the flash is accessed?
> >
> > How does that work in practice? Is CE1 muxed as a GPIO
> > on demand?  
> 
> Yes, well the actual pin will go into the state of CE1
> which default to a low state, the effect is
> hardware-defined.
> 
> This is not a recommendable type of electronics
> engineering if that is what you wonder... it's a way to
> patch over design mistakes in the design with
> software. (Which is what the vendor tree does.)
> 
> In one case (SQ201) the LED connected to them will light up
> since it is active low. So the LED goes on during flash
> access and gives a kind of "flash access flicker" on top
> of whatever it was indicating before.
> 
> That LED is marked "power" and I put a heartbeat on it
> so it will be brief interruptions of steady light in the
> heartbeat. The "only" problem is in UX. The vendor
> device exhibits the same strange behavior.
> 
> > What happens if the flash and SPI bus are
> > accessed at the same time? Do you
> > get -EBUSY? Not sure all MTD users retry when they
> > get -EBUSY...  
> 
> The way I set it up flash access "always win", so
> no worries for MTD.
> 
> If I simulate SPI or any other slow bus on the GPIO
> at the same time as we have muxed it out for
> flash access, it will have to do error recovery,
> because the bytes will not get through.

Do you mean that the GPIO controller does not request the pins it's
using? If it does, the pinctrl framework should complain, when the pins
are already assigned to another dev.

> 
> Ideally I would want to patch the GPIO library to
> redirect the access to this line to enable/disable
> CE1 so the pin is driven from the flash controller
> instead. But since that is essentially an address
> bit it looked pretty tricky.
> 
> In practice the use cases do not overlap, so
> we are fine, I guess if problems still occur the
> path of least resistance would simply be to have the
> display driver using this SPI line delay and
> retry a few times. It's an ugly solution perfectly
> fitting the ugly electronics IMO.

If the flash and SPI bus are never accessed concurrently and things are
controlled from userspace, maybe it's cleaner to bind/unbind devices on
demand, and let each driver mux the conflicting pins as they expect it
to be muxed.

Re: [PATCH 2/2 v2] mtd: physmap_of_gemini: Handle pin control

[Linus Walleij]

On Fri, Nov 30, 2018 at 12:17 PM Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> On Thu, 22 Nov 2018 15:10:47 +0100 Linus Walleij <linus.walleij@linaro.org> wrote:

> Do you mean that the GPIO controller does not request the pins it's
> using? If it does, the pinctrl framework should complain, when the pins
> are already assigned to another dev.

That depends on whether the pin controller is "strict" or
not. Several pin controllers actually allow the pin to be used
for some device and GPIO at the same time.

Gemini is not strict, there is a field in struct pinmux_ops
that controls strictness and this depends on the hardware
and use cases.

I would prefer if the world was simpler, but sadly hardware
engineers doing this kind of stuff don't know what that
means.

> If the flash and SPI bus are never accessed concurrently and things are
> controlled from userspace, maybe it's cleaner to bind/unbind devices on
> demand, and let each driver mux the conflicting pins as they expect it
> to be muxed.

These are in-kernel use cases.

The problem is not entirely new: if we want a pin
to be absolutely exclusively under control of a certain
hardware block, or even another CPU, we use a hardware
semaphore, see for example how STM32 does it:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/commit/?h=devel&id=97cfb6cd34f2a3dab03dce5f885333c213bbec8a

The same can of course be done using software, somehow,
with something like a global mutex that the SPI host and MTD
need to take respectively. (I imagine this would be retrieved as
a resource like any other like regulator, clock etc).

However implementing that when there isn't even a clash in
reality, just in theory, and only if I construct specific test programs
to make this happen ... I dunno it's just a bit overly ambitious.
This router is already working fine, so...

Yours,
Linus Walleij

Re: [PATCH 1/2 v2] mtd: maps: Leave assigned complex mappings

[Boris Brezillon]

Hi Linus,

On Mon, 12 Nov 2018 22:07:28 +0100
Linus Walleij <linus.walleij@linaro.org> wrote:

> The simple_map_init() may need to be called with some
> function pointers already assigned for complex mappings,
> just bail out if complex handlers have already been
> assigned.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Rebase on latest MTD development branch
> - Use a new approach as the code changed under me
> ---
>  drivers/mtd/maps/map_funcs.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/mtd/maps/map_funcs.c b/drivers/mtd/maps/map_funcs.c
> index 3f268370eeca..60e132293e1c 100644
> --- a/drivers/mtd/maps/map_funcs.c
> +++ b/drivers/mtd/maps/map_funcs.c
> @@ -31,6 +31,10 @@ static void __xipram simple_map_copy_to(struct map_info *map, unsigned long to,
>  
>  void simple_map_init(struct map_info *map)
>  {
> +	/* Complex map functions already assigned */
> +	if (map->read)
> +		return;
> +
>  	BUG_ON(!map_bankwidth_supported(map->bankwidth));
>  
>  	map->read = simple_map_read;

Can we move that to the physmap driver?

--->8---
diff --git a/drivers/mtd/maps/physmap-core.c b/drivers/mtd/maps/physmap-core.c
index e8c3b250d842..043c7de39757 100644
--- a/drivers/mtd/maps/physmap-core.c
+++ b/drivers/mtd/maps/physmap-core.c
@@ -514,10 +514,16 @@ static int physmap_flash_probe(struct platform_device *dev)
                        err = physmap_addr_gpios_map_init(&info->maps[i]);
                        if (err)
                                goto err_out;
-               } else {
-                       simple_map_init(&info->maps[i]);
                }
 
+               /*
+                * Only use the simple_map implementation if map hooks are not
+                * implemented. Since map->read() is mandatory checking for its
+                * presence is enough.
+                */
+               if (map->read)
+                       simple_map_init(&info->maps[i]);
+
                if (info->probe_type) {
                        info->mtds[i] = do_map_probe(info->probe_type,
                                                     &info->maps[i]);