[PATCH 0/7] memory: renesas-rpc-if: Rebind and s2ram fixes

[PATCH 0/7] memory: renesas-rpc-if: Rebind and s2ram fixes

[Geert Uytterhoeven]

	Hi all,

The Renesas RPC-IF provides either HyperFlash or SPI host access.
To handle this, three drivers are used:
  1. The RPC-IF core diver,
  2. An HyperFlash child driver,
  3. An SPI child driver.

Currently this driver collection has the following issues:
  1. After manually unbinding the child driver, rebinding the child
     driver fails with -EBUSY,
  2. During PSCI system suspend, the SoC may be powered down, losing
     RPC-IF register state, and causing data corruption after resume.

This patch series aims to fix this:
  - Patches 1-4 contain preparatory cleanups and improvements,
  - Patch 5 fixes unbind/rebind,
  - Patch 6 cleans up the internal API between the RPC-IF core diver,
    and the HF and SPI child drivers, and thus touches the MTD/HYPERBUS
    and SPI subsystems, too,
  - Patch 7 adds system suspend/resume support to the RPC-IF core
    driver.

This has been tested on the Salvator-XS (HyperFlash) and Falcon (QSPI
FLASH) development boards.

At least with HyperFlash, successful RPC-IF operation after s2ram is
still not guaranteed (more details below).
I do not have physical access to a board that uses the RPC-IF in SPI
mode, so I could not test s2ram with RPC-SPI.  I am wondering if it
suffers from similar problems, or if these are purely related to
HyperFlash?

Findings:

  - Sometimes RPC-HF still works after resume from s2ram

  - Sometimes RPC-HF read data is corrupted after resume from s2ram:

      - Data read looks like (for each block of 16 bytes at offset i):
          - 8 bytes of data stored at offset (i % 262144) * 256,
	  - 8 bytes duplicate of the above.

      - After that, unbind/rebind fails:

          # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/unbind
	  # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/bind
	  rpc-if-hyperflash rpc-if-hyperflash: probing of hyperbus device failed

      - After doing s2ram again, rebind (usually) succeeds again, and
	reading from HF returns the expected data again:

	  # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/bind
	  rpc-if-hyperflash: Found 1 x16 devices at 0x0 in 16-bit bank. Manufacturer ID 0x000001 Chip ID 0x007000

      - When doing unbind before s2ram, rebind after resume usually
	works (better success rate than without unbind), but not always.

Things I have tried:

  - Always resetting the device in rpcif_hw_init(), like is done on
    RZ/G2L, does not make a difference.

  - Dumping the full RPC register space before/after s2ram, but there
    does not seem to be any relation between register contents (which
    vary) and successful operation.

  - Adding HF calibration like hbmc-am654 (and never setting the
    controller's calibrated flag) does not help: either calibration
    succeeds with 5 passes on 5 tries, or fails with 0 passes on 25
    tries.

  - Browsing the TF/A and U-Boot sources also didn't help.

Thanks for your comments!

Geert Uytterhoeven (7):
  memory: renesas-rpc-if: Always use dev in rpcif_sw_init()
  memory: renesas-rpc-if: Add dev helper to rpcif_probe()
  memory: renesas-rpc-if: Improve Runtime PM handling
  memory: renesas-rpc-if: Split-off private data from struct rpcif
  memory: renesas-rpc-if: Move resource acquisition to .probe()
  memory: renesas-rpc-if: Pass device instead of rpcif to rpcif_*()
  memory: renesas-rpc-if: Reinitialize registers during system resume

 drivers/memory/renesas-rpc-if.c | 167 +++++++++++++++++++++-----------
 drivers/mtd/hyperbus/rpc-if.c   |  18 ++--
 drivers/spi/spi-rpc-if.c        |  14 +--
 include/memory/renesas-rpc-if.h |  32 ++----
 4 files changed, 137 insertions(+), 94 deletions(-)

-- 
2.25.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH 0/7] memory: renesas-rpc-if: Rebind and s2ram fixes

[Geert Uytterhoeven]

	Hi all,

The Renesas RPC-IF provides either HyperFlash or SPI host access.
To handle this, three drivers are used:
  1. The RPC-IF core diver,
  2. An HyperFlash child driver,
  3. An SPI child driver.

Currently this driver collection has the following issues:
  1. After manually unbinding the child driver, rebinding the child
     driver fails with -EBUSY,
  2. During PSCI system suspend, the SoC may be powered down, losing
     RPC-IF register state, and causing data corruption after resume.

This patch series aims to fix this:
  - Patches 1-4 contain preparatory cleanups and improvements,
  - Patch 5 fixes unbind/rebind,
  - Patch 6 cleans up the internal API between the RPC-IF core diver,
    and the HF and SPI child drivers, and thus touches the MTD/HYPERBUS
    and SPI subsystems, too,
  - Patch 7 adds system suspend/resume support to the RPC-IF core
    driver.

This has been tested on the Salvator-XS (HyperFlash) and Falcon (QSPI
FLASH) development boards.

At least with HyperFlash, successful RPC-IF operation after s2ram is
still not guaranteed (more details below).
I do not have physical access to a board that uses the RPC-IF in SPI
mode, so I could not test s2ram with RPC-SPI.  I am wondering if it
suffers from similar problems, or if these are purely related to
HyperFlash?

Findings:

  - Sometimes RPC-HF still works after resume from s2ram

  - Sometimes RPC-HF read data is corrupted after resume from s2ram:

      - Data read looks like (for each block of 16 bytes at offset i):
          - 8 bytes of data stored at offset (i % 262144) * 256,
	  - 8 bytes duplicate of the above.

      - After that, unbind/rebind fails:

          # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/unbind
	  # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/bind
	  rpc-if-hyperflash rpc-if-hyperflash: probing of hyperbus device failed

      - After doing s2ram again, rebind (usually) succeeds again, and
	reading from HF returns the expected data again:

	  # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/bind
	  rpc-if-hyperflash: Found 1 x16 devices at 0x0 in 16-bit bank. Manufacturer ID 0x000001 Chip ID 0x007000

      - When doing unbind before s2ram, rebind after resume usually
	works (better success rate than without unbind), but not always.

Things I have tried:

  - Always resetting the device in rpcif_hw_init(), like is done on
    RZ/G2L, does not make a difference.

  - Dumping the full RPC register space before/after s2ram, but there
    does not seem to be any relation between register contents (which
    vary) and successful operation.

  - Adding HF calibration like hbmc-am654 (and never setting the
    controller's calibrated flag) does not help: either calibration
    succeeds with 5 passes on 5 tries, or fails with 0 passes on 25
    tries.

  - Browsing the TF/A and U-Boot sources also didn't help.

Thanks for your comments!

Geert Uytterhoeven (7):
  memory: renesas-rpc-if: Always use dev in rpcif_sw_init()
  memory: renesas-rpc-if: Add dev helper to rpcif_probe()
  memory: renesas-rpc-if: Improve Runtime PM handling
  memory: renesas-rpc-if: Split-off private data from struct rpcif
  memory: renesas-rpc-if: Move resource acquisition to .probe()
  memory: renesas-rpc-if: Pass device instead of rpcif to rpcif_*()
  memory: renesas-rpc-if: Reinitialize registers during system resume

 drivers/memory/renesas-rpc-if.c | 167 +++++++++++++++++++++-----------
 drivers/mtd/hyperbus/rpc-if.c   |  18 ++--
 drivers/spi/spi-rpc-if.c        |  14 +--
 include/memory/renesas-rpc-if.h |  32 ++----
 4 files changed, 137 insertions(+), 94 deletions(-)

-- 
2.25.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 1/7] memory: renesas-rpc-if: Always use dev in rpcif_sw_init()

[Geert Uytterhoeven]

rpcif_sw_init() already has the "dev" parameter pointing to the right
device structure, so there is no need to take a detour through the
platform device.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/memory/renesas-rpc-if.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index 4316988d791a527f..829b962e8f034bdd 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -263,22 +263,21 @@ int rpcif_sw_init(struct rpcif *rpc, struct device *dev)
 	if (IS_ERR(rpc->base))
 		return PTR_ERR(rpc->base);
 
-	rpc->regmap = devm_regmap_init(&pdev->dev, NULL, rpc, &rpcif_regmap_config);
+	rpc->regmap = devm_regmap_init(dev, NULL, rpc, &rpcif_regmap_config);
 	if (IS_ERR(rpc->regmap)) {
-		dev_err(&pdev->dev,
-			"failed to init regmap for rpcif, error %ld\n",
+		dev_err(dev, "failed to init regmap for rpcif, error %ld\n",
 			PTR_ERR(rpc->regmap));
 		return	PTR_ERR(rpc->regmap);
 	}
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dirmap");
-	rpc->dirmap = devm_ioremap_resource(&pdev->dev, res);
+	rpc->dirmap = devm_ioremap_resource(dev, res);
 	if (IS_ERR(rpc->dirmap))
 		return PTR_ERR(rpc->dirmap);
 	rpc->size = resource_size(res);
 
 	rpc->type = (uintptr_t)of_device_get_match_data(dev);
-	rpc->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+	rpc->rstc = devm_reset_control_get_exclusive(dev, NULL);
 
 	return PTR_ERR_OR_ZERO(rpc->rstc);
 }
-- 
2.25.1

[PATCH 1/7] memory: renesas-rpc-if: Always use dev in rpcif_sw_init()

[Geert Uytterhoeven]

rpcif_sw_init() already has the "dev" parameter pointing to the right
device structure, so there is no need to take a detour through the
platform device.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/memory/renesas-rpc-if.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index 4316988d791a527f..829b962e8f034bdd 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -263,22 +263,21 @@ int rpcif_sw_init(struct rpcif *rpc, struct device *dev)
 	if (IS_ERR(rpc->base))
 		return PTR_ERR(rpc->base);
 
-	rpc->regmap = devm_regmap_init(&pdev->dev, NULL, rpc, &rpcif_regmap_config);
+	rpc->regmap = devm_regmap_init(dev, NULL, rpc, &rpcif_regmap_config);
 	if (IS_ERR(rpc->regmap)) {
-		dev_err(&pdev->dev,
-			"failed to init regmap for rpcif, error %ld\n",
+		dev_err(dev, "failed to init regmap for rpcif, error %ld\n",
 			PTR_ERR(rpc->regmap));
 		return	PTR_ERR(rpc->regmap);
 	}
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dirmap");
-	rpc->dirmap = devm_ioremap_resource(&pdev->dev, res);
+	rpc->dirmap = devm_ioremap_resource(dev, res);
 	if (IS_ERR(rpc->dirmap))
 		return PTR_ERR(rpc->dirmap);
 	rpc->size = resource_size(res);
 
 	rpc->type = (uintptr_t)of_device_get_match_data(dev);
-	rpc->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+	rpc->rstc = devm_reset_control_get_exclusive(dev, NULL);
 
 	return PTR_ERR_OR_ZERO(rpc->rstc);
 }
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 2/7] memory: renesas-rpc-if: Add dev helper to rpcif_probe()

[Geert Uytterhoeven]

Add a helper variable pointing to the device structure, to avoid going
through the platform device every time.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/memory/renesas-rpc-if.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index 829b962e8f034bdd..2421a820e3c5880c 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -666,14 +666,15 @@ EXPORT_SYMBOL(rpcif_dirmap_read);
 
 static int rpcif_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct platform_device *vdev;
 	struct device_node *flash;
 	const char *name;
 	int ret;
 
-	flash = of_get_next_child(pdev->dev.of_node, NULL);
+	flash = of_get_next_child(dev->of_node, NULL);
 	if (!flash) {
-		dev_warn(&pdev->dev, "no flash node found\n");
+		dev_warn(dev, "no flash node found\n");
 		return -ENODEV;
 	}
 
@@ -683,7 +684,7 @@ static int rpcif_probe(struct platform_device *pdev)
 		name = "rpc-if-hyperflash";
 	} else	{
 		of_node_put(flash);
-		dev_warn(&pdev->dev, "unknown flash type\n");
+		dev_warn(dev, "unknown flash type\n");
 		return -ENODEV;
 	}
 	of_node_put(flash);
@@ -691,7 +692,7 @@ static int rpcif_probe(struct platform_device *pdev)
 	vdev = platform_device_alloc(name, pdev->id);
 	if (!vdev)
 		return -ENOMEM;
-	vdev->dev.parent = &pdev->dev;
+	vdev->dev.parent = dev;
 	platform_set_drvdata(pdev, vdev);
 
 	ret = platform_device_add(vdev);
-- 
2.25.1

[PATCH 2/7] memory: renesas-rpc-if: Add dev helper to rpcif_probe()

[Geert Uytterhoeven]

Add a helper variable pointing to the device structure, to avoid going
through the platform device every time.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/memory/renesas-rpc-if.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index 829b962e8f034bdd..2421a820e3c5880c 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -666,14 +666,15 @@ EXPORT_SYMBOL(rpcif_dirmap_read);
 
 static int rpcif_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct platform_device *vdev;
 	struct device_node *flash;
 	const char *name;
 	int ret;
 
-	flash = of_get_next_child(pdev->dev.of_node, NULL);
+	flash = of_get_next_child(dev->of_node, NULL);
 	if (!flash) {
-		dev_warn(&pdev->dev, "no flash node found\n");
+		dev_warn(dev, "no flash node found\n");
 		return -ENODEV;
 	}
 
@@ -683,7 +684,7 @@ static int rpcif_probe(struct platform_device *pdev)
 		name = "rpc-if-hyperflash";
 	} else	{
 		of_node_put(flash);
-		dev_warn(&pdev->dev, "unknown flash type\n");
+		dev_warn(dev, "unknown flash type\n");
 		return -ENODEV;
 	}
 	of_node_put(flash);
@@ -691,7 +692,7 @@ static int rpcif_probe(struct platform_device *pdev)
 	vdev = platform_device_alloc(name, pdev->id);
 	if (!vdev)
 		return -ENOMEM;
-	vdev->dev.parent = &pdev->dev;
+	vdev->dev.parent = dev;
 	platform_set_drvdata(pdev, vdev);
 
 	ret = platform_device_add(vdev);
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 3/7] memory: renesas-rpc-if: Improve Runtime PM handling

[Geert Uytterhoeven]

Convert from the deprecated pm_runtime_get_sync() to the new
pm_runtime_resume_and_get(), and add error checking.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/memory/renesas-rpc-if.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index 2421a820e3c5880c..55dbb3854b09ad43 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -300,12 +300,13 @@ static void rpcif_rzg2l_timing_adjust_sdr(struct rpcif *rpc)
 int rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
 {
 	u32 dummy;
+	int ret;
 
-	pm_runtime_get_sync(rpc->dev);
+	ret = pm_runtime_resume_and_get(rpc->dev);
+	if (ret)
+		return ret;
 
 	if (rpc->type == RPCIF_RZ_G2L) {
-		int ret;
-
 		ret = reset_control_reset(rpc->rstc);
 		if (ret)
 			return ret;
@@ -469,7 +470,9 @@ int rpcif_manual_xfer(struct rpcif *rpc)
 	u32 smenr, smcr, pos = 0, max = rpc->bus_size == 2 ? 8 : 4;
 	int ret = 0;
 
-	pm_runtime_get_sync(rpc->dev);
+	ret = pm_runtime_resume_and_get(rpc->dev);
+	if (ret < 0)
+		return ret;
 
 	regmap_update_bits(rpc->regmap, RPCIF_PHYCNT,
 			   RPCIF_PHYCNT_CAL, RPCIF_PHYCNT_CAL);
@@ -636,11 +639,14 @@ ssize_t rpcif_dirmap_read(struct rpcif *rpc, u64 offs, size_t len, void *buf)
 {
 	loff_t from = offs & (rpc->size - 1);
 	size_t size = rpc->size - from;
+	int ret;
 
 	if (len > size)
 		len = size;
 
-	pm_runtime_get_sync(rpc->dev);
+	ret = pm_runtime_resume_and_get(rpc->dev);
+	if (ret < 0)
+		return ret;
 
 	regmap_update_bits(rpc->regmap, RPCIF_CMNCR, RPCIF_CMNCR_MD, 0);
 	regmap_write(rpc->regmap, RPCIF_DRCR, 0);
-- 
2.25.1

[PATCH 3/7] memory: renesas-rpc-if: Improve Runtime PM handling

[Geert Uytterhoeven]

Convert from the deprecated pm_runtime_get_sync() to the new
pm_runtime_resume_and_get(), and add error checking.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/memory/renesas-rpc-if.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index 2421a820e3c5880c..55dbb3854b09ad43 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -300,12 +300,13 @@ static void rpcif_rzg2l_timing_adjust_sdr(struct rpcif *rpc)
 int rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
 {
 	u32 dummy;
+	int ret;
 
-	pm_runtime_get_sync(rpc->dev);
+	ret = pm_runtime_resume_and_get(rpc->dev);
+	if (ret)
+		return ret;
 
 	if (rpc->type == RPCIF_RZ_G2L) {
-		int ret;
-
 		ret = reset_control_reset(rpc->rstc);
 		if (ret)
 			return ret;
@@ -469,7 +470,9 @@ int rpcif_manual_xfer(struct rpcif *rpc)
 	u32 smenr, smcr, pos = 0, max = rpc->bus_size == 2 ? 8 : 4;
 	int ret = 0;
 
-	pm_runtime_get_sync(rpc->dev);
+	ret = pm_runtime_resume_and_get(rpc->dev);
+	if (ret < 0)
+		return ret;
 
 	regmap_update_bits(rpc->regmap, RPCIF_PHYCNT,
 			   RPCIF_PHYCNT_CAL, RPCIF_PHYCNT_CAL);
@@ -636,11 +639,14 @@ ssize_t rpcif_dirmap_read(struct rpcif *rpc, u64 offs, size_t len, void *buf)
 {
 	loff_t from = offs & (rpc->size - 1);
 	size_t size = rpc->size - from;
+	int ret;
 
 	if (len > size)
 		len = size;
 
-	pm_runtime_get_sync(rpc->dev);
+	ret = pm_runtime_resume_and_get(rpc->dev);
+	if (ret < 0)
+		return ret;
 
 	regmap_update_bits(rpc->regmap, RPCIF_CMNCR, RPCIF_CMNCR_MD, 0);
 	regmap_write(rpc->regmap, RPCIF_DRCR, 0);
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 4/7] memory: renesas-rpc-if: Split-off private data from struct rpcif

[Geert Uytterhoeven]

The rpcif structure is used as a common data structure, shared by the
RPC-IF core driver and by the HyperBus and SPI child drivers.
This poses several problems:
  - Most structure members describe private core driver state, which
    should not be accessible by the child drivers,
  - The structure's lifetime is controlled by the child drivers,
    complicating use by the core driver.

Fix this by moving the private core driver state to its own structure,
managed by the RPC-IF core driver, and store it in the core driver's
private data field.  This requires absorbing the child's platform
device, as that was stored in the driver's private data field before.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/memory/renesas-rpc-if.c | 75 +++++++++++++++++++++++++--------
 include/memory/renesas-rpc-if.h | 16 -------
 2 files changed, 57 insertions(+), 34 deletions(-)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index 55dbb3854b09ad43..78e10a7300411191 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -162,14 +162,36 @@ static const struct regmap_access_table rpcif_volatile_table = {
 	.n_yes_ranges	= ARRAY_SIZE(rpcif_volatile_ranges),
 };
 
+struct rpcif_priv {
+	struct device *dev;
+	void __iomem *base;
+	void __iomem *dirmap;
+	struct regmap *regmap;
+	struct reset_control *rstc;
+	struct platform_device *vdev;
+	size_t size;
+	enum rpcif_type type;
+	enum rpcif_data_dir dir;
+	u8 bus_size;
+	u8 xfer_size;
+	void *buffer;
+	u32 xferlen;
+	u32 smcr;
+	u32 smadr;
+	u32 command;		/* DRCMR or SMCMR */
+	u32 option;		/* DROPR or SMOPR */
+	u32 enable;		/* DRENR or SMENR */
+	u32 dummy;		/* DRDMCR or SMDMCR */
+	u32 ddr;		/* DRDRENR or SMDRENR */
+};
 
 /*
  * Custom accessor functions to ensure SM[RW]DR[01] are always accessed with
- * proper width.  Requires rpcif.xfer_size to be correctly set before!
+ * proper width.  Requires rpcif_priv.xfer_size to be correctly set before!
  */
 static int rpcif_reg_read(void *context, unsigned int reg, unsigned int *val)
 {
-	struct rpcif *rpc = context;
+	struct rpcif_priv *rpc = context;
 
 	switch (reg) {
 	case RPCIF_SMRDR0:
@@ -205,7 +227,7 @@ static int rpcif_reg_read(void *context, unsigned int reg, unsigned int *val)
 
 static int rpcif_reg_write(void *context, unsigned int reg, unsigned int val)
 {
-	struct rpcif *rpc = context;
+	struct rpcif_priv *rpc = context;
 
 	switch (reg) {
 	case RPCIF_SMWDR0:
@@ -252,13 +274,12 @@ static const struct regmap_config rpcif_regmap_config = {
 	.volatile_table	= &rpcif_volatile_table,
 };
 
-int rpcif_sw_init(struct rpcif *rpc, struct device *dev)
+int rpcif_sw_init(struct rpcif *rpcif, struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
+	struct rpcif_priv *rpc = dev_get_drvdata(dev);
 	struct resource *res;
 
-	rpc->dev = dev;
-
 	rpc->base = devm_platform_ioremap_resource_byname(pdev, "regs");
 	if (IS_ERR(rpc->base))
 		return PTR_ERR(rpc->base);
@@ -278,12 +299,17 @@ int rpcif_sw_init(struct rpcif *rpc, struct device *dev)
 
 	rpc->type = (uintptr_t)of_device_get_match_data(dev);
 	rpc->rstc = devm_reset_control_get_exclusive(dev, NULL);
+	if (IS_ERR(rpc->rstc))
+		return PTR_ERR(rpc->rstc);
 
-	return PTR_ERR_OR_ZERO(rpc->rstc);
+	rpcif->dev = dev;
+	rpcif->dirmap = rpc->dirmap;
+	rpcif->size = rpc->size;
+	return 0;
 }
 EXPORT_SYMBOL(rpcif_sw_init);
 
-static void rpcif_rzg2l_timing_adjust_sdr(struct rpcif *rpc)
+static void rpcif_rzg2l_timing_adjust_sdr(struct rpcif_priv *rpc)
 {
 	regmap_write(rpc->regmap, RPCIF_PHYWR, 0xa5390000);
 	regmap_write(rpc->regmap, RPCIF_PHYADD, 0x80000000);
@@ -297,8 +323,9 @@ static void rpcif_rzg2l_timing_adjust_sdr(struct rpcif *rpc)
 	regmap_write(rpc->regmap, RPCIF_PHYADD, 0x80000032);
 }
 
-int rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
+int rpcif_hw_init(struct rpcif *rpcif, bool hyperflash)
 {
+	struct rpcif_priv *rpc = dev_get_drvdata(rpcif->dev);
 	u32 dummy;
 	int ret;
 
@@ -357,7 +384,7 @@ int rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
 }
 EXPORT_SYMBOL(rpcif_hw_init);
 
-static int wait_msg_xfer_end(struct rpcif *rpc)
+static int wait_msg_xfer_end(struct rpcif_priv *rpc)
 {
 	u32 sts;
 
@@ -366,7 +393,7 @@ static int wait_msg_xfer_end(struct rpcif *rpc)
 					USEC_PER_SEC);
 }
 
-static u8 rpcif_bits_set(struct rpcif *rpc, u32 nbytes)
+static u8 rpcif_bits_set(struct rpcif_priv *rpc, u32 nbytes)
 {
 	if (rpc->bus_size == 2)
 		nbytes /= 2;
@@ -379,9 +406,11 @@ static u8 rpcif_bit_size(u8 buswidth)
 	return buswidth > 4 ? 2 : ilog2(buswidth);
 }
 
-void rpcif_prepare(struct rpcif *rpc, const struct rpcif_op *op, u64 *offs,
+void rpcif_prepare(struct rpcif *rpcif, const struct rpcif_op *op, u64 *offs,
 		   size_t *len)
 {
+	struct rpcif_priv *rpc = dev_get_drvdata(rpcif->dev);
+
 	rpc->smcr = 0;
 	rpc->smadr = 0;
 	rpc->enable = 0;
@@ -465,8 +494,9 @@ void rpcif_prepare(struct rpcif *rpc, const struct rpcif_op *op, u64 *offs,
 }
 EXPORT_SYMBOL(rpcif_prepare);
 
-int rpcif_manual_xfer(struct rpcif *rpc)
+int rpcif_manual_xfer(struct rpcif *rpcif)
 {
+	struct rpcif_priv *rpc = dev_get_drvdata(rpcif->dev);
 	u32 smenr, smcr, pos = 0, max = rpc->bus_size == 2 ? 8 : 4;
 	int ret = 0;
 
@@ -588,7 +618,7 @@ int rpcif_manual_xfer(struct rpcif *rpc)
 err_out:
 	if (reset_control_reset(rpc->rstc))
 		dev_err(rpc->dev, "Failed to reset HW\n");
-	rpcif_hw_init(rpc, rpc->bus_size == 2);
+	rpcif_hw_init(rpcif, rpc->bus_size == 2);
 	goto exit;
 }
 EXPORT_SYMBOL(rpcif_manual_xfer);
@@ -635,8 +665,9 @@ static void memcpy_fromio_readw(void *to,
 	}
 }
 
-ssize_t rpcif_dirmap_read(struct rpcif *rpc, u64 offs, size_t len, void *buf)
+ssize_t rpcif_dirmap_read(struct rpcif *rpcif, u64 offs, size_t len, void *buf)
 {
+	struct rpcif_priv *rpc = dev_get_drvdata(rpcif->dev);
 	loff_t from = offs & (rpc->size - 1);
 	size_t size = rpc->size - from;
 	int ret;
@@ -675,6 +706,7 @@ static int rpcif_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct platform_device *vdev;
 	struct device_node *flash;
+	struct rpcif_priv *rpc;
 	const char *name;
 	int ret;
 
@@ -695,11 +727,18 @@ static int rpcif_probe(struct platform_device *pdev)
 	}
 	of_node_put(flash);
 
+	rpc = devm_kzalloc(dev, sizeof(*rpc), GFP_KERNEL);
+	if (!rpc)
+		return -ENOMEM;
+
 	vdev = platform_device_alloc(name, pdev->id);
 	if (!vdev)
 		return -ENOMEM;
 	vdev->dev.parent = dev;
-	platform_set_drvdata(pdev, vdev);
+
+	rpc->dev = dev;
+	rpc->vdev = vdev;
+	platform_set_drvdata(pdev, rpc);
 
 	ret = platform_device_add(vdev);
 	if (ret) {
@@ -712,9 +751,9 @@ static int rpcif_probe(struct platform_device *pdev)
 
 static int rpcif_remove(struct platform_device *pdev)
 {
-	struct platform_device *vdev = platform_get_drvdata(pdev);
+	struct rpcif_priv *rpc = platform_get_drvdata(pdev);
 
-	platform_device_unregister(vdev);
+	platform_device_unregister(rpc->vdev);
 
 	return 0;
 }
diff --git a/include/memory/renesas-rpc-if.h b/include/memory/renesas-rpc-if.h
index 9c0ad64b8d292d49..ddf94356752d3315 100644
--- a/include/memory/renesas-rpc-if.h
+++ b/include/memory/renesas-rpc-if.h
@@ -64,24 +64,8 @@ enum rpcif_type {
 
 struct rpcif {
 	struct device *dev;
-	void __iomem *base;
 	void __iomem *dirmap;
-	struct regmap *regmap;
-	struct reset_control *rstc;
 	size_t size;
-	enum rpcif_type type;
-	enum rpcif_data_dir dir;
-	u8 bus_size;
-	u8 xfer_size;
-	void *buffer;
-	u32 xferlen;
-	u32 smcr;
-	u32 smadr;
-	u32 command;		/* DRCMR or SMCMR */
-	u32 option;		/* DROPR or SMOPR */
-	u32 enable;		/* DRENR or SMENR */
-	u32 dummy;		/* DRDMCR or SMDMCR */
-	u32 ddr;		/* DRDRENR or SMDRENR */
 };
 
 int rpcif_sw_init(struct rpcif *rpc, struct device *dev);
-- 
2.25.1

[PATCH 4/7] memory: renesas-rpc-if: Split-off private data from struct rpcif

[Geert Uytterhoeven]

The rpcif structure is used as a common data structure, shared by the
RPC-IF core driver and by the HyperBus and SPI child drivers.
This poses several problems:
  - Most structure members describe private core driver state, which
    should not be accessible by the child drivers,
  - The structure's lifetime is controlled by the child drivers,
    complicating use by the core driver.

Fix this by moving the private core driver state to its own structure,
managed by the RPC-IF core driver, and store it in the core driver's
private data field.  This requires absorbing the child's platform
device, as that was stored in the driver's private data field before.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/memory/renesas-rpc-if.c | 75 +++++++++++++++++++++++++--------
 include/memory/renesas-rpc-if.h | 16 -------
 2 files changed, 57 insertions(+), 34 deletions(-)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index 55dbb3854b09ad43..78e10a7300411191 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -162,14 +162,36 @@ static const struct regmap_access_table rpcif_volatile_table = {
 	.n_yes_ranges	= ARRAY_SIZE(rpcif_volatile_ranges),
 };
 
+struct rpcif_priv {
+	struct device *dev;
+	void __iomem *base;
+	void __iomem *dirmap;
+	struct regmap *regmap;
+	struct reset_control *rstc;
+	struct platform_device *vdev;
+	size_t size;
+	enum rpcif_type type;
+	enum rpcif_data_dir dir;
+	u8 bus_size;
+	u8 xfer_size;
+	void *buffer;
+	u32 xferlen;
+	u32 smcr;
+	u32 smadr;
+	u32 command;		/* DRCMR or SMCMR */
+	u32 option;		/* DROPR or SMOPR */
+	u32 enable;		/* DRENR or SMENR */
+	u32 dummy;		/* DRDMCR or SMDMCR */
+	u32 ddr;		/* DRDRENR or SMDRENR */
+};
 
 /*
  * Custom accessor functions to ensure SM[RW]DR[01] are always accessed with
- * proper width.  Requires rpcif.xfer_size to be correctly set before!
+ * proper width.  Requires rpcif_priv.xfer_size to be correctly set before!
  */
 static int rpcif_reg_read(void *context, unsigned int reg, unsigned int *val)
 {
-	struct rpcif *rpc = context;
+	struct rpcif_priv *rpc = context;
 
 	switch (reg) {
 	case RPCIF_SMRDR0:
@@ -205,7 +227,7 @@ static int rpcif_reg_read(void *context, unsigned int reg, unsigned int *val)
 
 static int rpcif_reg_write(void *context, unsigned int reg, unsigned int val)
 {
-	struct rpcif *rpc = context;
+	struct rpcif_priv *rpc = context;
 
 	switch (reg) {
 	case RPCIF_SMWDR0:
@@ -252,13 +274,12 @@ static const struct regmap_config rpcif_regmap_config = {
 	.volatile_table	= &rpcif_volatile_table,
 };
 
-int rpcif_sw_init(struct rpcif *rpc, struct device *dev)
+int rpcif_sw_init(struct rpcif *rpcif, struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
+	struct rpcif_priv *rpc = dev_get_drvdata(dev);
 	struct resource *res;
 
-	rpc->dev = dev;
-
 	rpc->base = devm_platform_ioremap_resource_byname(pdev, "regs");
 	if (IS_ERR(rpc->base))
 		return PTR_ERR(rpc->base);
@@ -278,12 +299,17 @@ int rpcif_sw_init(struct rpcif *rpc, struct device *dev)
 
 	rpc->type = (uintptr_t)of_device_get_match_data(dev);
 	rpc->rstc = devm_reset_control_get_exclusive(dev, NULL);
+	if (IS_ERR(rpc->rstc))
+		return PTR_ERR(rpc->rstc);
 
-	return PTR_ERR_OR_ZERO(rpc->rstc);
+	rpcif->dev = dev;
+	rpcif->dirmap = rpc->dirmap;
+	rpcif->size = rpc->size;
+	return 0;
 }
 EXPORT_SYMBOL(rpcif_sw_init);
 
-static void rpcif_rzg2l_timing_adjust_sdr(struct rpcif *rpc)
+static void rpcif_rzg2l_timing_adjust_sdr(struct rpcif_priv *rpc)
 {
 	regmap_write(rpc->regmap, RPCIF_PHYWR, 0xa5390000);
 	regmap_write(rpc->regmap, RPCIF_PHYADD, 0x80000000);
@@ -297,8 +323,9 @@ static void rpcif_rzg2l_timing_adjust_sdr(struct rpcif *rpc)
 	regmap_write(rpc->regmap, RPCIF_PHYADD, 0x80000032);
 }
 
-int rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
+int rpcif_hw_init(struct rpcif *rpcif, bool hyperflash)
 {
+	struct rpcif_priv *rpc = dev_get_drvdata(rpcif->dev);
 	u32 dummy;
 	int ret;
 
@@ -357,7 +384,7 @@ int rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
 }
 EXPORT_SYMBOL(rpcif_hw_init);
 
-static int wait_msg_xfer_end(struct rpcif *rpc)
+static int wait_msg_xfer_end(struct rpcif_priv *rpc)
 {
 	u32 sts;
 
@@ -366,7 +393,7 @@ static int wait_msg_xfer_end(struct rpcif *rpc)
 					USEC_PER_SEC);
 }
 
-static u8 rpcif_bits_set(struct rpcif *rpc, u32 nbytes)
+static u8 rpcif_bits_set(struct rpcif_priv *rpc, u32 nbytes)
 {
 	if (rpc->bus_size == 2)
 		nbytes /= 2;
@@ -379,9 +406,11 @@ static u8 rpcif_bit_size(u8 buswidth)
 	return buswidth > 4 ? 2 : ilog2(buswidth);
 }
 
-void rpcif_prepare(struct rpcif *rpc, const struct rpcif_op *op, u64 *offs,
+void rpcif_prepare(struct rpcif *rpcif, const struct rpcif_op *op, u64 *offs,
 		   size_t *len)
 {
+	struct rpcif_priv *rpc = dev_get_drvdata(rpcif->dev);
+
 	rpc->smcr = 0;
 	rpc->smadr = 0;
 	rpc->enable = 0;
@@ -465,8 +494,9 @@ void rpcif_prepare(struct rpcif *rpc, const struct rpcif_op *op, u64 *offs,
 }
 EXPORT_SYMBOL(rpcif_prepare);
 
-int rpcif_manual_xfer(struct rpcif *rpc)
+int rpcif_manual_xfer(struct rpcif *rpcif)
 {
+	struct rpcif_priv *rpc = dev_get_drvdata(rpcif->dev);
 	u32 smenr, smcr, pos = 0, max = rpc->bus_size == 2 ? 8 : 4;
 	int ret = 0;
 
@@ -588,7 +618,7 @@ int rpcif_manual_xfer(struct rpcif *rpc)
 err_out:
 	if (reset_control_reset(rpc->rstc))
 		dev_err(rpc->dev, "Failed to reset HW\n");
-	rpcif_hw_init(rpc, rpc->bus_size == 2);
+	rpcif_hw_init(rpcif, rpc->bus_size == 2);
 	goto exit;
 }
 EXPORT_SYMBOL(rpcif_manual_xfer);
@@ -635,8 +665,9 @@ static void memcpy_fromio_readw(void *to,
 	}
 }
 
-ssize_t rpcif_dirmap_read(struct rpcif *rpc, u64 offs, size_t len, void *buf)
+ssize_t rpcif_dirmap_read(struct rpcif *rpcif, u64 offs, size_t len, void *buf)
 {
+	struct rpcif_priv *rpc = dev_get_drvdata(rpcif->dev);
 	loff_t from = offs & (rpc->size - 1);
 	size_t size = rpc->size - from;
 	int ret;
@@ -675,6 +706,7 @@ static int rpcif_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct platform_device *vdev;
 	struct device_node *flash;
+	struct rpcif_priv *rpc;
 	const char *name;
 	int ret;
 
@@ -695,11 +727,18 @@ static int rpcif_probe(struct platform_device *pdev)
 	}
 	of_node_put(flash);
 
+	rpc = devm_kzalloc(dev, sizeof(*rpc), GFP_KERNEL);
+	if (!rpc)
+		return -ENOMEM;
+
 	vdev = platform_device_alloc(name, pdev->id);
 	if (!vdev)
 		return -ENOMEM;
 	vdev->dev.parent = dev;
-	platform_set_drvdata(pdev, vdev);
+
+	rpc->dev = dev;
+	rpc->vdev = vdev;
+	platform_set_drvdata(pdev, rpc);
 
 	ret = platform_device_add(vdev);
 	if (ret) {
@@ -712,9 +751,9 @@ static int rpcif_probe(struct platform_device *pdev)
 
 static int rpcif_remove(struct platform_device *pdev)
 {
-	struct platform_device *vdev = platform_get_drvdata(pdev);
+	struct rpcif_priv *rpc = platform_get_drvdata(pdev);
 
-	platform_device_unregister(vdev);
+	platform_device_unregister(rpc->vdev);
 
 	return 0;
 }
diff --git a/include/memory/renesas-rpc-if.h b/include/memory/renesas-rpc-if.h
index 9c0ad64b8d292d49..ddf94356752d3315 100644
--- a/include/memory/renesas-rpc-if.h
+++ b/include/memory/renesas-rpc-if.h
@@ -64,24 +64,8 @@ enum rpcif_type {
 
 struct rpcif {
 	struct device *dev;
-	void __iomem *base;
 	void __iomem *dirmap;
-	struct regmap *regmap;
-	struct reset_control *rstc;
 	size_t size;
-	enum rpcif_type type;
-	enum rpcif_data_dir dir;
-	u8 bus_size;
-	u8 xfer_size;
-	void *buffer;
-	u32 xferlen;
-	u32 smcr;
-	u32 smadr;
-	u32 command;		/* DRCMR or SMCMR */
-	u32 option;		/* DROPR or SMOPR */
-	u32 enable;		/* DRENR or SMENR */
-	u32 dummy;		/* DRDMCR or SMDMCR */
-	u32 ddr;		/* DRDRENR or SMDRENR */
 };
 
 int rpcif_sw_init(struct rpcif *rpc, struct device *dev);
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 5/7] memory: renesas-rpc-if: Move resource acquisition to .probe()

[Geert Uytterhoeven]

While the acquired resources are tied to the lifetime of the RPC-IF core
device (through the use of managed resource functions), the actual
resource acquisition is triggered from the HyperBus and SPI child
drivers.  Due to this mismatch, unbinding and rebinding the child
drivers manually fails with -EBUSY:

    # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/unbind
    # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/bind
    rpc-if ee200000.spi: can't request region for resource [mem 0xee200000-0xee2001ff]
    rpc-if-hyperflash: probe of rpc-if-hyperflash failed with error -16

Fix this by moving all resource acquisition to the core driver's probe
routine.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/memory/renesas-rpc-if.c | 47 ++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 24 deletions(-)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index 78e10a7300411191..ef0336cbb4c196fb 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -276,31 +276,7 @@ static const struct regmap_config rpcif_regmap_config = {
 
 int rpcif_sw_init(struct rpcif *rpcif, struct device *dev)
 {
-	struct platform_device *pdev = to_platform_device(dev);
 	struct rpcif_priv *rpc = dev_get_drvdata(dev);
-	struct resource *res;
-
-	rpc->base = devm_platform_ioremap_resource_byname(pdev, "regs");
-	if (IS_ERR(rpc->base))
-		return PTR_ERR(rpc->base);
-
-	rpc->regmap = devm_regmap_init(dev, NULL, rpc, &rpcif_regmap_config);
-	if (IS_ERR(rpc->regmap)) {
-		dev_err(dev, "failed to init regmap for rpcif, error %ld\n",
-			PTR_ERR(rpc->regmap));
-		return	PTR_ERR(rpc->regmap);
-	}
-
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dirmap");
-	rpc->dirmap = devm_ioremap_resource(dev, res);
-	if (IS_ERR(rpc->dirmap))
-		return PTR_ERR(rpc->dirmap);
-	rpc->size = resource_size(res);
-
-	rpc->type = (uintptr_t)of_device_get_match_data(dev);
-	rpc->rstc = devm_reset_control_get_exclusive(dev, NULL);
-	if (IS_ERR(rpc->rstc))
-		return PTR_ERR(rpc->rstc);
 
 	rpcif->dev = dev;
 	rpcif->dirmap = rpc->dirmap;
@@ -707,6 +683,7 @@ static int rpcif_probe(struct platform_device *pdev)
 	struct platform_device *vdev;
 	struct device_node *flash;
 	struct rpcif_priv *rpc;
+	struct resource *res;
 	const char *name;
 	int ret;
 
@@ -731,6 +708,28 @@ static int rpcif_probe(struct platform_device *pdev)
 	if (!rpc)
 		return -ENOMEM;
 
+	rpc->base = devm_platform_ioremap_resource_byname(pdev, "regs");
+	if (IS_ERR(rpc->base))
+		return PTR_ERR(rpc->base);
+
+	rpc->regmap = devm_regmap_init(dev, NULL, rpc, &rpcif_regmap_config);
+	if (IS_ERR(rpc->regmap)) {
+		dev_err(dev, "failed to init regmap for rpcif, error %ld\n",
+			PTR_ERR(rpc->regmap));
+		return	PTR_ERR(rpc->regmap);
+	}
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dirmap");
+	rpc->dirmap = devm_ioremap_resource(dev, res);
+	if (IS_ERR(rpc->dirmap))
+		return PTR_ERR(rpc->dirmap);
+	rpc->size = resource_size(res);
+
+	rpc->type = (uintptr_t)of_device_get_match_data(dev);
+	rpc->rstc = devm_reset_control_get_exclusive(dev, NULL);
+	if (IS_ERR(rpc->rstc))
+		return PTR_ERR(rpc->rstc);
+
 	vdev = platform_device_alloc(name, pdev->id);
 	if (!vdev)
 		return -ENOMEM;
-- 
2.25.1

[PATCH 5/7] memory: renesas-rpc-if: Move resource acquisition to .probe()

[Geert Uytterhoeven]

While the acquired resources are tied to the lifetime of the RPC-IF core
device (through the use of managed resource functions), the actual
resource acquisition is triggered from the HyperBus and SPI child
drivers.  Due to this mismatch, unbinding and rebinding the child
drivers manually fails with -EBUSY:

    # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/unbind
    # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/bind
    rpc-if ee200000.spi: can't request region for resource [mem 0xee200000-0xee2001ff]
    rpc-if-hyperflash: probe of rpc-if-hyperflash failed with error -16

Fix this by moving all resource acquisition to the core driver's probe
routine.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/memory/renesas-rpc-if.c | 47 ++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 24 deletions(-)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index 78e10a7300411191..ef0336cbb4c196fb 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -276,31 +276,7 @@ static const struct regmap_config rpcif_regmap_config = {
 
 int rpcif_sw_init(struct rpcif *rpcif, struct device *dev)
 {
-	struct platform_device *pdev = to_platform_device(dev);
 	struct rpcif_priv *rpc = dev_get_drvdata(dev);
-	struct resource *res;
-
-	rpc->base = devm_platform_ioremap_resource_byname(pdev, "regs");
-	if (IS_ERR(rpc->base))
-		return PTR_ERR(rpc->base);
-
-	rpc->regmap = devm_regmap_init(dev, NULL, rpc, &rpcif_regmap_config);
-	if (IS_ERR(rpc->regmap)) {
-		dev_err(dev, "failed to init regmap for rpcif, error %ld\n",
-			PTR_ERR(rpc->regmap));
-		return	PTR_ERR(rpc->regmap);
-	}
-
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dirmap");
-	rpc->dirmap = devm_ioremap_resource(dev, res);
-	if (IS_ERR(rpc->dirmap))
-		return PTR_ERR(rpc->dirmap);
-	rpc->size = resource_size(res);
-
-	rpc->type = (uintptr_t)of_device_get_match_data(dev);
-	rpc->rstc = devm_reset_control_get_exclusive(dev, NULL);
-	if (IS_ERR(rpc->rstc))
-		return PTR_ERR(rpc->rstc);
 
 	rpcif->dev = dev;
 	rpcif->dirmap = rpc->dirmap;
@@ -707,6 +683,7 @@ static int rpcif_probe(struct platform_device *pdev)
 	struct platform_device *vdev;
 	struct device_node *flash;
 	struct rpcif_priv *rpc;
+	struct resource *res;
 	const char *name;
 	int ret;
 
@@ -731,6 +708,28 @@ static int rpcif_probe(struct platform_device *pdev)
 	if (!rpc)
 		return -ENOMEM;
 
+	rpc->base = devm_platform_ioremap_resource_byname(pdev, "regs");
+	if (IS_ERR(rpc->base))
+		return PTR_ERR(rpc->base);
+
+	rpc->regmap = devm_regmap_init(dev, NULL, rpc, &rpcif_regmap_config);
+	if (IS_ERR(rpc->regmap)) {
+		dev_err(dev, "failed to init regmap for rpcif, error %ld\n",
+			PTR_ERR(rpc->regmap));
+		return	PTR_ERR(rpc->regmap);
+	}
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dirmap");
+	rpc->dirmap = devm_ioremap_resource(dev, res);
+	if (IS_ERR(rpc->dirmap))
+		return PTR_ERR(rpc->dirmap);
+	rpc->size = resource_size(res);
+
+	rpc->type = (uintptr_t)of_device_get_match_data(dev);
+	rpc->rstc = devm_reset_control_get_exclusive(dev, NULL);
+	if (IS_ERR(rpc->rstc))
+		return PTR_ERR(rpc->rstc);
+
 	vdev = platform_device_alloc(name, pdev->id);
 	if (!vdev)
 		return -ENOMEM;
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 6/7] memory: renesas-rpc-if: Pass device instead of rpcif to rpcif_*()

[Geert Uytterhoeven]

Most rpcif_*() API functions do not need access to any other fields in
the rpcif structure than the device pointer.  Simplify dependencies by
passing the device pointer instead.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/memory/renesas-rpc-if.c | 32 ++++++++++++++++----------------
 drivers/mtd/hyperbus/rpc-if.c   | 18 +++++++++---------
 drivers/spi/spi-rpc-if.c        | 14 +++++++-------
 include/memory/renesas-rpc-if.h | 16 ++++++++--------
 4 files changed, 40 insertions(+), 40 deletions(-)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index ef0336cbb4c196fb..ec76e603ad24d214 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -299,13 +299,13 @@ static void rpcif_rzg2l_timing_adjust_sdr(struct rpcif_priv *rpc)
 	regmap_write(rpc->regmap, RPCIF_PHYADD, 0x80000032);
 }
 
-int rpcif_hw_init(struct rpcif *rpcif, bool hyperflash)
+int rpcif_hw_init(struct device *dev, bool hyperflash)
 {
-	struct rpcif_priv *rpc = dev_get_drvdata(rpcif->dev);
+	struct rpcif_priv *rpc = dev_get_drvdata(dev);
 	u32 dummy;
 	int ret;
 
-	ret = pm_runtime_resume_and_get(rpc->dev);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret)
 		return ret;
 
@@ -352,7 +352,7 @@ int rpcif_hw_init(struct rpcif *rpcif, bool hyperflash)
 	regmap_write(rpc->regmap, RPCIF_SSLDR, RPCIF_SSLDR_SPNDL(7) |
 		     RPCIF_SSLDR_SLNDL(7) | RPCIF_SSLDR_SCKDL(7));
 
-	pm_runtime_put(rpc->dev);
+	pm_runtime_put(dev);
 
 	rpc->bus_size = hyperflash ? 2 : 1;
 
@@ -382,10 +382,10 @@ static u8 rpcif_bit_size(u8 buswidth)
 	return buswidth > 4 ? 2 : ilog2(buswidth);
 }
 
-void rpcif_prepare(struct rpcif *rpcif, const struct rpcif_op *op, u64 *offs,
+void rpcif_prepare(struct device *dev, const struct rpcif_op *op, u64 *offs,
 		   size_t *len)
 {
-	struct rpcif_priv *rpc = dev_get_drvdata(rpcif->dev);
+	struct rpcif_priv *rpc = dev_get_drvdata(dev);
 
 	rpc->smcr = 0;
 	rpc->smadr = 0;
@@ -470,13 +470,13 @@ void rpcif_prepare(struct rpcif *rpcif, const struct rpcif_op *op, u64 *offs,
 }
 EXPORT_SYMBOL(rpcif_prepare);
 
-int rpcif_manual_xfer(struct rpcif *rpcif)
+int rpcif_manual_xfer(struct device *dev)
 {
-	struct rpcif_priv *rpc = dev_get_drvdata(rpcif->dev);
+	struct rpcif_priv *rpc = dev_get_drvdata(dev);
 	u32 smenr, smcr, pos = 0, max = rpc->bus_size == 2 ? 8 : 4;
 	int ret = 0;
 
-	ret = pm_runtime_resume_and_get(rpc->dev);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0)
 		return ret;
 
@@ -588,13 +588,13 @@ int rpcif_manual_xfer(struct rpcif *rpcif)
 	}
 
 exit:
-	pm_runtime_put(rpc->dev);
+	pm_runtime_put(dev);
 	return ret;
 
 err_out:
 	if (reset_control_reset(rpc->rstc))
-		dev_err(rpc->dev, "Failed to reset HW\n");
-	rpcif_hw_init(rpcif, rpc->bus_size == 2);
+		dev_err(dev, "Failed to reset HW\n");
+	rpcif_hw_init(dev, rpc->bus_size == 2);
 	goto exit;
 }
 EXPORT_SYMBOL(rpcif_manual_xfer);
@@ -641,9 +641,9 @@ static void memcpy_fromio_readw(void *to,
 	}
 }
 
-ssize_t rpcif_dirmap_read(struct rpcif *rpcif, u64 offs, size_t len, void *buf)
+ssize_t rpcif_dirmap_read(struct device *dev, u64 offs, size_t len, void *buf)
 {
-	struct rpcif_priv *rpc = dev_get_drvdata(rpcif->dev);
+	struct rpcif_priv *rpc = dev_get_drvdata(dev);
 	loff_t from = offs & (rpc->size - 1);
 	size_t size = rpc->size - from;
 	int ret;
@@ -651,7 +651,7 @@ ssize_t rpcif_dirmap_read(struct rpcif *rpcif, u64 offs, size_t len, void *buf)
 	if (len > size)
 		len = size;
 
-	ret = pm_runtime_resume_and_get(rpc->dev);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0)
 		return ret;
 
@@ -671,7 +671,7 @@ ssize_t rpcif_dirmap_read(struct rpcif *rpcif, u64 offs, size_t len, void *buf)
 	else
 		memcpy_fromio(buf, rpc->dirmap + from, len);
 
-	pm_runtime_put(rpc->dev);
+	pm_runtime_put(dev);
 
 	return len;
 }
diff --git a/drivers/mtd/hyperbus/rpc-if.c b/drivers/mtd/hyperbus/rpc-if.c
index d00d302434030b20..41734e337ac00e40 100644
--- a/drivers/mtd/hyperbus/rpc-if.c
+++ b/drivers/mtd/hyperbus/rpc-if.c
@@ -56,7 +56,7 @@ static void rpcif_hb_prepare_read(struct rpcif *rpc, void *to,
 	op.data.nbytes = len;
 	op.data.buf.in = to;
 
-	rpcif_prepare(rpc, &op, NULL, NULL);
+	rpcif_prepare(rpc->dev, &op, NULL, NULL);
 }
 
 static void rpcif_hb_prepare_write(struct rpcif *rpc, unsigned long to,
@@ -70,7 +70,7 @@ static void rpcif_hb_prepare_write(struct rpcif *rpc, unsigned long to,
 	op.data.nbytes = len;
 	op.data.buf.out = from;
 
-	rpcif_prepare(rpc, &op, NULL, NULL);
+	rpcif_prepare(rpc->dev, &op, NULL, NULL);
 }
 
 static u16 rpcif_hb_read16(struct hyperbus_device *hbdev, unsigned long addr)
@@ -81,7 +81,7 @@ static u16 rpcif_hb_read16(struct hyperbus_device *hbdev, unsigned long addr)
 
 	rpcif_hb_prepare_read(&hyperbus->rpc, &data, addr, 2);
 
-	rpcif_manual_xfer(&hyperbus->rpc);
+	rpcif_manual_xfer(hyperbus->rpc.dev);
 
 	return data.x[0];
 }
@@ -94,7 +94,7 @@ static void rpcif_hb_write16(struct hyperbus_device *hbdev, unsigned long addr,
 
 	rpcif_hb_prepare_write(&hyperbus->rpc, addr, &data, 2);
 
-	rpcif_manual_xfer(&hyperbus->rpc);
+	rpcif_manual_xfer(hyperbus->rpc.dev);
 }
 
 static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
@@ -105,7 +105,7 @@ static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
 
 	rpcif_hb_prepare_read(&hyperbus->rpc, to, from, len);
 
-	rpcif_dirmap_read(&hyperbus->rpc, from, len, to);
+	rpcif_dirmap_read(hyperbus->rpc.dev, from, len, to);
 }
 
 static const struct hyperbus_ops rpcif_hb_ops = {
@@ -130,9 +130,9 @@ static int rpcif_hb_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, hyperbus);
 
-	rpcif_enable_rpm(&hyperbus->rpc);
+	rpcif_enable_rpm(hyperbus->rpc.dev);
 
-	error = rpcif_hw_init(&hyperbus->rpc, true);
+	error = rpcif_hw_init(hyperbus->rpc.dev, true);
 	if (error)
 		goto out_disable_rpm;
 
@@ -150,7 +150,7 @@ static int rpcif_hb_probe(struct platform_device *pdev)
 	return 0;
 
 out_disable_rpm:
-	rpcif_disable_rpm(&hyperbus->rpc);
+	rpcif_disable_rpm(hyperbus->rpc.dev);
 	return error;
 }
 
@@ -160,7 +160,7 @@ static int rpcif_hb_remove(struct platform_device *pdev)
 
 	hyperbus_unregister_device(&hyperbus->hbdev);
 
-	rpcif_disable_rpm(&hyperbus->rpc);
+	rpcif_disable_rpm(hyperbus->rpc.dev);
 
 	return 0;
 }
diff --git a/drivers/spi/spi-rpc-if.c b/drivers/spi/spi-rpc-if.c
index 24ec1c83f379ceec..5063587d2c724c7c 100644
--- a/drivers/spi/spi-rpc-if.c
+++ b/drivers/spi/spi-rpc-if.c
@@ -58,7 +58,7 @@ static void rpcif_spi_mem_prepare(struct spi_device *spi_dev,
 		rpc_op.data.dir = RPCIF_NO_DATA;
 	}
 
-	rpcif_prepare(rpc, &rpc_op, offs, len);
+	rpcif_prepare(rpc->dev, &rpc_op, offs, len);
 }
 
 static bool rpcif_spi_mem_supports_op(struct spi_mem *mem,
@@ -86,7 +86,7 @@ static ssize_t rpcif_spi_mem_dirmap_read(struct spi_mem_dirmap_desc *desc,
 
 	rpcif_spi_mem_prepare(desc->mem->spi, &desc->info.op_tmpl, &offs, &len);
 
-	return rpcif_dirmap_read(rpc, offs, len, buf);
+	return rpcif_dirmap_read(rpc->dev, offs, len, buf);
 }
 
 static int rpcif_spi_mem_dirmap_create(struct spi_mem_dirmap_desc *desc)
@@ -117,7 +117,7 @@ static int rpcif_spi_mem_exec_op(struct spi_mem *mem,
 
 	rpcif_spi_mem_prepare(mem->spi, op, NULL, NULL);
 
-	return rpcif_manual_xfer(rpc);
+	return rpcif_manual_xfer(rpc->dev);
 }
 
 static const struct spi_controller_mem_ops rpcif_spi_mem_ops = {
@@ -147,7 +147,7 @@ static int rpcif_spi_probe(struct platform_device *pdev)
 
 	ctlr->dev.of_node = parent->of_node;
 
-	rpcif_enable_rpm(rpc);
+	rpcif_enable_rpm(rpc->dev);
 
 	ctlr->num_chipselect = 1;
 	ctlr->mem_ops = &rpcif_spi_mem_ops;
@@ -156,7 +156,7 @@ static int rpcif_spi_probe(struct platform_device *pdev)
 	ctlr->mode_bits = SPI_CPOL | SPI_CPHA | SPI_TX_QUAD | SPI_RX_QUAD;
 	ctlr->flags = SPI_CONTROLLER_HALF_DUPLEX;
 
-	error = rpcif_hw_init(rpc, false);
+	error = rpcif_hw_init(rpc->dev, false);
 	if (error)
 		goto out_disable_rpm;
 
@@ -169,7 +169,7 @@ static int rpcif_spi_probe(struct platform_device *pdev)
 	return 0;
 
 out_disable_rpm:
-	rpcif_disable_rpm(rpc);
+	rpcif_disable_rpm(rpc->dev);
 	return error;
 }
 
@@ -179,7 +179,7 @@ static int rpcif_spi_remove(struct platform_device *pdev)
 	struct rpcif *rpc = spi_controller_get_devdata(ctlr);
 
 	spi_unregister_controller(ctlr);
-	rpcif_disable_rpm(rpc);
+	rpcif_disable_rpm(rpc->dev);
 
 	return 0;
 }
diff --git a/include/memory/renesas-rpc-if.h b/include/memory/renesas-rpc-if.h
index ddf94356752d3315..d2130c2c8c82fbd5 100644
--- a/include/memory/renesas-rpc-if.h
+++ b/include/memory/renesas-rpc-if.h
@@ -69,20 +69,20 @@ struct rpcif {
 };
 
 int rpcif_sw_init(struct rpcif *rpc, struct device *dev);
-int rpcif_hw_init(struct rpcif *rpc, bool hyperflash);
-void rpcif_prepare(struct rpcif *rpc, const struct rpcif_op *op, u64 *offs,
+int rpcif_hw_init(struct device *dev, bool hyperflash);
+void rpcif_prepare(struct device *dev, const struct rpcif_op *op, u64 *offs,
 		   size_t *len);
-int rpcif_manual_xfer(struct rpcif *rpc);
-ssize_t rpcif_dirmap_read(struct rpcif *rpc, u64 offs, size_t len, void *buf);
+int rpcif_manual_xfer(struct device *dev);
+ssize_t rpcif_dirmap_read(struct device *dev, u64 offs, size_t len, void *buf);
 
-static inline void rpcif_enable_rpm(struct rpcif *rpc)
+static inline void rpcif_enable_rpm(struct device *dev)
 {
-	pm_runtime_enable(rpc->dev);
+	pm_runtime_enable(dev);
 }
 
-static inline void rpcif_disable_rpm(struct rpcif *rpc)
+static inline void rpcif_disable_rpm(struct device *dev)
 {
-	pm_runtime_disable(rpc->dev);
+	pm_runtime_disable(dev);
 }
 
 #endif // __RENESAS_RPC_IF_H
-- 
2.25.1

[PATCH 6/7] memory: renesas-rpc-if: Pass device instead of rpcif to rpcif_*()

[Geert Uytterhoeven]

Most rpcif_*() API functions do not need access to any other fields in
the rpcif structure than the device pointer.  Simplify dependencies by
passing the device pointer instead.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/memory/renesas-rpc-if.c | 32 ++++++++++++++++----------------
 drivers/mtd/hyperbus/rpc-if.c   | 18 +++++++++---------
 drivers/spi/spi-rpc-if.c        | 14 +++++++-------
 include/memory/renesas-rpc-if.h | 16 ++++++++--------
 4 files changed, 40 insertions(+), 40 deletions(-)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index ef0336cbb4c196fb..ec76e603ad24d214 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -299,13 +299,13 @@ static void rpcif_rzg2l_timing_adjust_sdr(struct rpcif_priv *rpc)
 	regmap_write(rpc->regmap, RPCIF_PHYADD, 0x80000032);
 }
 
-int rpcif_hw_init(struct rpcif *rpcif, bool hyperflash)
+int rpcif_hw_init(struct device *dev, bool hyperflash)
 {
-	struct rpcif_priv *rpc = dev_get_drvdata(rpcif->dev);
+	struct rpcif_priv *rpc = dev_get_drvdata(dev);
 	u32 dummy;
 	int ret;
 
-	ret = pm_runtime_resume_and_get(rpc->dev);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret)
 		return ret;
 
@@ -352,7 +352,7 @@ int rpcif_hw_init(struct rpcif *rpcif, bool hyperflash)
 	regmap_write(rpc->regmap, RPCIF_SSLDR, RPCIF_SSLDR_SPNDL(7) |
 		     RPCIF_SSLDR_SLNDL(7) | RPCIF_SSLDR_SCKDL(7));
 
-	pm_runtime_put(rpc->dev);
+	pm_runtime_put(dev);
 
 	rpc->bus_size = hyperflash ? 2 : 1;
 
@@ -382,10 +382,10 @@ static u8 rpcif_bit_size(u8 buswidth)
 	return buswidth > 4 ? 2 : ilog2(buswidth);
 }
 
-void rpcif_prepare(struct rpcif *rpcif, const struct rpcif_op *op, u64 *offs,
+void rpcif_prepare(struct device *dev, const struct rpcif_op *op, u64 *offs,
 		   size_t *len)
 {
-	struct rpcif_priv *rpc = dev_get_drvdata(rpcif->dev);
+	struct rpcif_priv *rpc = dev_get_drvdata(dev);
 
 	rpc->smcr = 0;
 	rpc->smadr = 0;
@@ -470,13 +470,13 @@ void rpcif_prepare(struct rpcif *rpcif, const struct rpcif_op *op, u64 *offs,
 }
 EXPORT_SYMBOL(rpcif_prepare);
 
-int rpcif_manual_xfer(struct rpcif *rpcif)
+int rpcif_manual_xfer(struct device *dev)
 {
-	struct rpcif_priv *rpc = dev_get_drvdata(rpcif->dev);
+	struct rpcif_priv *rpc = dev_get_drvdata(dev);
 	u32 smenr, smcr, pos = 0, max = rpc->bus_size == 2 ? 8 : 4;
 	int ret = 0;
 
-	ret = pm_runtime_resume_and_get(rpc->dev);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0)
 		return ret;
 
@@ -588,13 +588,13 @@ int rpcif_manual_xfer(struct rpcif *rpcif)
 	}
 
 exit:
-	pm_runtime_put(rpc->dev);
+	pm_runtime_put(dev);
 	return ret;
 
 err_out:
 	if (reset_control_reset(rpc->rstc))
-		dev_err(rpc->dev, "Failed to reset HW\n");
-	rpcif_hw_init(rpcif, rpc->bus_size == 2);
+		dev_err(dev, "Failed to reset HW\n");
+	rpcif_hw_init(dev, rpc->bus_size == 2);
 	goto exit;
 }
 EXPORT_SYMBOL(rpcif_manual_xfer);
@@ -641,9 +641,9 @@ static void memcpy_fromio_readw(void *to,
 	}
 }
 
-ssize_t rpcif_dirmap_read(struct rpcif *rpcif, u64 offs, size_t len, void *buf)
+ssize_t rpcif_dirmap_read(struct device *dev, u64 offs, size_t len, void *buf)
 {
-	struct rpcif_priv *rpc = dev_get_drvdata(rpcif->dev);
+	struct rpcif_priv *rpc = dev_get_drvdata(dev);
 	loff_t from = offs & (rpc->size - 1);
 	size_t size = rpc->size - from;
 	int ret;
@@ -651,7 +651,7 @@ ssize_t rpcif_dirmap_read(struct rpcif *rpcif, u64 offs, size_t len, void *buf)
 	if (len > size)
 		len = size;
 
-	ret = pm_runtime_resume_and_get(rpc->dev);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0)
 		return ret;
 
@@ -671,7 +671,7 @@ ssize_t rpcif_dirmap_read(struct rpcif *rpcif, u64 offs, size_t len, void *buf)
 	else
 		memcpy_fromio(buf, rpc->dirmap + from, len);
 
-	pm_runtime_put(rpc->dev);
+	pm_runtime_put(dev);
 
 	return len;
 }
diff --git a/drivers/mtd/hyperbus/rpc-if.c b/drivers/mtd/hyperbus/rpc-if.c
index d00d302434030b20..41734e337ac00e40 100644
--- a/drivers/mtd/hyperbus/rpc-if.c
+++ b/drivers/mtd/hyperbus/rpc-if.c
@@ -56,7 +56,7 @@ static void rpcif_hb_prepare_read(struct rpcif *rpc, void *to,
 	op.data.nbytes = len;
 	op.data.buf.in = to;
 
-	rpcif_prepare(rpc, &op, NULL, NULL);
+	rpcif_prepare(rpc->dev, &op, NULL, NULL);
 }
 
 static void rpcif_hb_prepare_write(struct rpcif *rpc, unsigned long to,
@@ -70,7 +70,7 @@ static void rpcif_hb_prepare_write(struct rpcif *rpc, unsigned long to,
 	op.data.nbytes = len;
 	op.data.buf.out = from;
 
-	rpcif_prepare(rpc, &op, NULL, NULL);
+	rpcif_prepare(rpc->dev, &op, NULL, NULL);
 }
 
 static u16 rpcif_hb_read16(struct hyperbus_device *hbdev, unsigned long addr)
@@ -81,7 +81,7 @@ static u16 rpcif_hb_read16(struct hyperbus_device *hbdev, unsigned long addr)
 
 	rpcif_hb_prepare_read(&hyperbus->rpc, &data, addr, 2);
 
-	rpcif_manual_xfer(&hyperbus->rpc);
+	rpcif_manual_xfer(hyperbus->rpc.dev);
 
 	return data.x[0];
 }
@@ -94,7 +94,7 @@ static void rpcif_hb_write16(struct hyperbus_device *hbdev, unsigned long addr,
 
 	rpcif_hb_prepare_write(&hyperbus->rpc, addr, &data, 2);
 
-	rpcif_manual_xfer(&hyperbus->rpc);
+	rpcif_manual_xfer(hyperbus->rpc.dev);
 }
 
 static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
@@ -105,7 +105,7 @@ static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
 
 	rpcif_hb_prepare_read(&hyperbus->rpc, to, from, len);
 
-	rpcif_dirmap_read(&hyperbus->rpc, from, len, to);
+	rpcif_dirmap_read(hyperbus->rpc.dev, from, len, to);
 }
 
 static const struct hyperbus_ops rpcif_hb_ops = {
@@ -130,9 +130,9 @@ static int rpcif_hb_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, hyperbus);
 
-	rpcif_enable_rpm(&hyperbus->rpc);
+	rpcif_enable_rpm(hyperbus->rpc.dev);
 
-	error = rpcif_hw_init(&hyperbus->rpc, true);
+	error = rpcif_hw_init(hyperbus->rpc.dev, true);
 	if (error)
 		goto out_disable_rpm;
 
@@ -150,7 +150,7 @@ static int rpcif_hb_probe(struct platform_device *pdev)
 	return 0;
 
 out_disable_rpm:
-	rpcif_disable_rpm(&hyperbus->rpc);
+	rpcif_disable_rpm(hyperbus->rpc.dev);
 	return error;
 }
 
@@ -160,7 +160,7 @@ static int rpcif_hb_remove(struct platform_device *pdev)
 
 	hyperbus_unregister_device(&hyperbus->hbdev);
 
-	rpcif_disable_rpm(&hyperbus->rpc);
+	rpcif_disable_rpm(hyperbus->rpc.dev);
 
 	return 0;
 }
diff --git a/drivers/spi/spi-rpc-if.c b/drivers/spi/spi-rpc-if.c
index 24ec1c83f379ceec..5063587d2c724c7c 100644
--- a/drivers/spi/spi-rpc-if.c
+++ b/drivers/spi/spi-rpc-if.c
@@ -58,7 +58,7 @@ static void rpcif_spi_mem_prepare(struct spi_device *spi_dev,
 		rpc_op.data.dir = RPCIF_NO_DATA;
 	}
 
-	rpcif_prepare(rpc, &rpc_op, offs, len);
+	rpcif_prepare(rpc->dev, &rpc_op, offs, len);
 }
 
 static bool rpcif_spi_mem_supports_op(struct spi_mem *mem,
@@ -86,7 +86,7 @@ static ssize_t rpcif_spi_mem_dirmap_read(struct spi_mem_dirmap_desc *desc,
 
 	rpcif_spi_mem_prepare(desc->mem->spi, &desc->info.op_tmpl, &offs, &len);
 
-	return rpcif_dirmap_read(rpc, offs, len, buf);
+	return rpcif_dirmap_read(rpc->dev, offs, len, buf);
 }
 
 static int rpcif_spi_mem_dirmap_create(struct spi_mem_dirmap_desc *desc)
@@ -117,7 +117,7 @@ static int rpcif_spi_mem_exec_op(struct spi_mem *mem,
 
 	rpcif_spi_mem_prepare(mem->spi, op, NULL, NULL);
 
-	return rpcif_manual_xfer(rpc);
+	return rpcif_manual_xfer(rpc->dev);
 }
 
 static const struct spi_controller_mem_ops rpcif_spi_mem_ops = {
@@ -147,7 +147,7 @@ static int rpcif_spi_probe(struct platform_device *pdev)
 
 	ctlr->dev.of_node = parent->of_node;
 
-	rpcif_enable_rpm(rpc);
+	rpcif_enable_rpm(rpc->dev);
 
 	ctlr->num_chipselect = 1;
 	ctlr->mem_ops = &rpcif_spi_mem_ops;
@@ -156,7 +156,7 @@ static int rpcif_spi_probe(struct platform_device *pdev)
 	ctlr->mode_bits = SPI_CPOL | SPI_CPHA | SPI_TX_QUAD | SPI_RX_QUAD;
 	ctlr->flags = SPI_CONTROLLER_HALF_DUPLEX;
 
-	error = rpcif_hw_init(rpc, false);
+	error = rpcif_hw_init(rpc->dev, false);
 	if (error)
 		goto out_disable_rpm;
 
@@ -169,7 +169,7 @@ static int rpcif_spi_probe(struct platform_device *pdev)
 	return 0;
 
 out_disable_rpm:
-	rpcif_disable_rpm(rpc);
+	rpcif_disable_rpm(rpc->dev);
 	return error;
 }
 
@@ -179,7 +179,7 @@ static int rpcif_spi_remove(struct platform_device *pdev)
 	struct rpcif *rpc = spi_controller_get_devdata(ctlr);
 
 	spi_unregister_controller(ctlr);
-	rpcif_disable_rpm(rpc);
+	rpcif_disable_rpm(rpc->dev);
 
 	return 0;
 }
diff --git a/include/memory/renesas-rpc-if.h b/include/memory/renesas-rpc-if.h
index ddf94356752d3315..d2130c2c8c82fbd5 100644
--- a/include/memory/renesas-rpc-if.h
+++ b/include/memory/renesas-rpc-if.h
@@ -69,20 +69,20 @@ struct rpcif {
 };
 
 int rpcif_sw_init(struct rpcif *rpc, struct device *dev);
-int rpcif_hw_init(struct rpcif *rpc, bool hyperflash);
-void rpcif_prepare(struct rpcif *rpc, const struct rpcif_op *op, u64 *offs,
+int rpcif_hw_init(struct device *dev, bool hyperflash);
+void rpcif_prepare(struct device *dev, const struct rpcif_op *op, u64 *offs,
 		   size_t *len);
-int rpcif_manual_xfer(struct rpcif *rpc);
-ssize_t rpcif_dirmap_read(struct rpcif *rpc, u64 offs, size_t len, void *buf);
+int rpcif_manual_xfer(struct device *dev);
+ssize_t rpcif_dirmap_read(struct device *dev, u64 offs, size_t len, void *buf);
 
-static inline void rpcif_enable_rpm(struct rpcif *rpc)
+static inline void rpcif_enable_rpm(struct device *dev)
 {
-	pm_runtime_enable(rpc->dev);
+	pm_runtime_enable(dev);
 }
 
-static inline void rpcif_disable_rpm(struct rpcif *rpc)
+static inline void rpcif_disable_rpm(struct device *dev)
 {
-	pm_runtime_disable(rpc->dev);
+	pm_runtime_disable(dev);
 }
 
 #endif // __RENESAS_RPC_IF_H
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 7/7] memory: renesas-rpc-if: Reinitialize registers during system resume

[Geert Uytterhoeven]

During PSCI system suspend, R-Car Gen3 SoCs may be powered down, and
thus the RPC-IF register state may be lost.  Consequently, when using
the RPC-IF after system resume, data corruption may happen.

Fix this by reinitializing the hardware state during system resume.
As this requires resuming the RPC-IF core device, this can only be done
when the device is under active control of the HyperBus or SPI child
driver.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/memory/renesas-rpc-if.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index ec76e603ad24d214..2a49d4511c8296c5 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -757,6 +757,20 @@ static int rpcif_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int rpcif_resume(struct device *dev)
+{
+	struct rpcif_priv *rpc = dev_get_drvdata(dev);
+
+	if (!pm_runtime_enabled(dev)) {
+		/* Not yet activated or deactivated by child device */
+		return 0;
+	}
+
+	return rpcif_hw_init(dev, rpc->bus_size == 2);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(rpcif_pm_ops, NULL, rpcif_resume);
+
 static const struct of_device_id rpcif_of_match[] = {
 	{ .compatible = "renesas,rcar-gen3-rpc-if", .data = (void *)RPCIF_RCAR_GEN3 },
 	{ .compatible = "renesas,rzg2l-rpc-if", .data = (void *)RPCIF_RZ_G2L },
@@ -770,6 +784,7 @@ static struct platform_driver rpcif_driver = {
 	.driver = {
 		.name =	"rpc-if",
 		.of_match_table = rpcif_of_match,
+		.pm = pm_sleep_ptr(&rpcif_pm_ops),
 	},
 };
 module_platform_driver(rpcif_driver);
-- 
2.25.1

[PATCH 7/7] memory: renesas-rpc-if: Reinitialize registers during system resume

[Geert Uytterhoeven]

During PSCI system suspend, R-Car Gen3 SoCs may be powered down, and
thus the RPC-IF register state may be lost.  Consequently, when using
the RPC-IF after system resume, data corruption may happen.

Fix this by reinitializing the hardware state during system resume.
As this requires resuming the RPC-IF core device, this can only be done
when the device is under active control of the HyperBus or SPI child
driver.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/memory/renesas-rpc-if.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index ec76e603ad24d214..2a49d4511c8296c5 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -757,6 +757,20 @@ static int rpcif_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int rpcif_resume(struct device *dev)
+{
+	struct rpcif_priv *rpc = dev_get_drvdata(dev);
+
+	if (!pm_runtime_enabled(dev)) {
+		/* Not yet activated or deactivated by child device */
+		return 0;
+	}
+
+	return rpcif_hw_init(dev, rpc->bus_size == 2);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(rpcif_pm_ops, NULL, rpcif_resume);
+
 static const struct of_device_id rpcif_of_match[] = {
 	{ .compatible = "renesas,rcar-gen3-rpc-if", .data = (void *)RPCIF_RCAR_GEN3 },
 	{ .compatible = "renesas,rzg2l-rpc-if", .data = (void *)RPCIF_RZ_G2L },
@@ -770,6 +784,7 @@ static struct platform_driver rpcif_driver = {
 	.driver = {
 		.name =	"rpc-if",
 		.of_match_table = rpcif_of_match,
+		.pm = pm_sleep_ptr(&rpcif_pm_ops),
 	},
 };
 module_platform_driver(rpcif_driver);
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 5/7] memory: renesas-rpc-if: Move resource acquisition to .probe()

[Krzysztof Kozlowski]

On 27/06/2022 17:31, Geert Uytterhoeven wrote:
> While the acquired resources are tied to the lifetime of the RPC-IF core
> device (through the use of managed resource functions), the actual
> resource acquisition is triggered from the HyperBus and SPI child
> drivers.  Due to this mismatch, unbinding and rebinding the child
> drivers manually fails with -EBUSY:
> 
>     # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/unbind
>     # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/bind
>     rpc-if ee200000.spi: can't request region for resource [mem 0xee200000-0xee2001ff]
>     rpc-if-hyperflash: probe of rpc-if-hyperflash failed with error -16
> 
> Fix this by moving all resource acquisition to the core driver's probe
> routine.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

This looks like a fix, so how about putting it as first in the series,
so backporting is easy/automatic? Plus a fixes tag?

Best regards,
Krzysztof

Re: [PATCH 5/7] memory: renesas-rpc-if: Move resource acquisition to .probe()

[Krzysztof Kozlowski]

On 27/06/2022 17:31, Geert Uytterhoeven wrote:
> While the acquired resources are tied to the lifetime of the RPC-IF core
> device (through the use of managed resource functions), the actual
> resource acquisition is triggered from the HyperBus and SPI child
> drivers.  Due to this mismatch, unbinding and rebinding the child
> drivers manually fails with -EBUSY:
> 
>     # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/unbind
>     # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/bind
>     rpc-if ee200000.spi: can't request region for resource [mem 0xee200000-0xee2001ff]
>     rpc-if-hyperflash: probe of rpc-if-hyperflash failed with error -16
> 
> Fix this by moving all resource acquisition to the core driver's probe
> routine.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

This looks like a fix, so how about putting it as first in the series,
so backporting is easy/automatic? Plus a fixes tag?

Best regards,
Krzysztof

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 5/7] memory: renesas-rpc-if: Move resource acquisition to .probe()

[Geert Uytterhoeven]

Hi Krzysztof,

On Wed, Jun 29, 2022 at 7:44 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 27/06/2022 17:31, Geert Uytterhoeven wrote:
> > While the acquired resources are tied to the lifetime of the RPC-IF core
> > device (through the use of managed resource functions), the actual
> > resource acquisition is triggered from the HyperBus and SPI child
> > drivers.  Due to this mismatch, unbinding and rebinding the child
> > drivers manually fails with -EBUSY:
> >
> >     # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/unbind
> >     # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/bind
> >     rpc-if ee200000.spi: can't request region for resource [mem 0xee200000-0xee2001ff]
> >     rpc-if-hyperflash: probe of rpc-if-hyperflash failed with error -16
> >
> > Fix this by moving all resource acquisition to the core driver's probe
> > routine.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> This looks like a fix, so how about putting it as first in the series,
> so backporting is easy/automatic?

Unfortunately that is not as simple as it sounds, as this really
depends on patch 4.
I agree patches 1-3 could be moved later, if you think it is worthwhile.

> Plus a fixes tag?

It's been like that since forever.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 5/7] memory: renesas-rpc-if: Move resource acquisition to .probe()

[Geert Uytterhoeven]

Hi Krzysztof,

On Wed, Jun 29, 2022 at 7:44 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 27/06/2022 17:31, Geert Uytterhoeven wrote:
> > While the acquired resources are tied to the lifetime of the RPC-IF core
> > device (through the use of managed resource functions), the actual
> > resource acquisition is triggered from the HyperBus and SPI child
> > drivers.  Due to this mismatch, unbinding and rebinding the child
> > drivers manually fails with -EBUSY:
> >
> >     # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/unbind
> >     # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/bind
> >     rpc-if ee200000.spi: can't request region for resource [mem 0xee200000-0xee2001ff]
> >     rpc-if-hyperflash: probe of rpc-if-hyperflash failed with error -16
> >
> > Fix this by moving all resource acquisition to the core driver's probe
> > routine.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> This looks like a fix, so how about putting it as first in the series,
> so backporting is easy/automatic?

Unfortunately that is not as simple as it sounds, as this really
depends on patch 4.
I agree patches 1-3 could be moved later, if you think it is worthwhile.

> Plus a fixes tag?

It's been like that since forever.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 5/7] memory: renesas-rpc-if: Move resource acquisition to .probe()

[Krzysztof Kozlowski]

On 29/06/2022 19:55, Geert Uytterhoeven wrote:
> Hi Krzysztof,
> 
> On Wed, Jun 29, 2022 at 7:44 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>> On 27/06/2022 17:31, Geert Uytterhoeven wrote:
>>> While the acquired resources are tied to the lifetime of the RPC-IF core
>>> device (through the use of managed resource functions), the actual
>>> resource acquisition is triggered from the HyperBus and SPI child
>>> drivers.  Due to this mismatch, unbinding and rebinding the child
>>> drivers manually fails with -EBUSY:
>>>
>>>     # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/unbind
>>>     # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/bind
>>>     rpc-if ee200000.spi: can't request region for resource [mem 0xee200000-0xee2001ff]
>>>     rpc-if-hyperflash: probe of rpc-if-hyperflash failed with error -16
>>>
>>> Fix this by moving all resource acquisition to the core driver's probe
>>> routine.
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>
>> This looks like a fix, so how about putting it as first in the series,
>> so backporting is easy/automatic?
> 
> Unfortunately that is not as simple as it sounds, as this really
> depends on patch 4.

You sure? Except rebasing I don't see that. rpcif_sw_init() received the
rpcif so it had access to all fields.


> I agree patches 1-3 could be moved later, if you think it is worthwhile.

This would not be enough, it has to be first patch to be backportable.

Best regards,
Krzysztof

Re: [PATCH 5/7] memory: renesas-rpc-if: Move resource acquisition to .probe()

[Krzysztof Kozlowski]

On 29/06/2022 19:55, Geert Uytterhoeven wrote:
> Hi Krzysztof,
> 
> On Wed, Jun 29, 2022 at 7:44 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>> On 27/06/2022 17:31, Geert Uytterhoeven wrote:
>>> While the acquired resources are tied to the lifetime of the RPC-IF core
>>> device (through the use of managed resource functions), the actual
>>> resource acquisition is triggered from the HyperBus and SPI child
>>> drivers.  Due to this mismatch, unbinding and rebinding the child
>>> drivers manually fails with -EBUSY:
>>>
>>>     # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/unbind
>>>     # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/bind
>>>     rpc-if ee200000.spi: can't request region for resource [mem 0xee200000-0xee2001ff]
>>>     rpc-if-hyperflash: probe of rpc-if-hyperflash failed with error -16
>>>
>>> Fix this by moving all resource acquisition to the core driver's probe
>>> routine.
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>
>> This looks like a fix, so how about putting it as first in the series,
>> so backporting is easy/automatic?
> 
> Unfortunately that is not as simple as it sounds, as this really
> depends on patch 4.

You sure? Except rebasing I don't see that. rpcif_sw_init() received the
rpcif so it had access to all fields.


> I agree patches 1-3 could be moved later, if you think it is worthwhile.

This would not be enough, it has to be first patch to be backportable.

Best regards,
Krzysztof

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 6/7] memory: renesas-rpc-if: Pass device instead of rpcif to rpcif_*()

[Krzysztof Kozlowski]

On 27/06/2022 17:31, Geert Uytterhoeven wrote:
> Most rpcif_*() API functions do not need access to any other fields in
> the rpcif structure than the device pointer.  Simplify dependencies by
> passing the device pointer instead.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/memory/renesas-rpc-if.c | 32 ++++++++++++++++----------------
>  drivers/mtd/hyperbus/rpc-if.c   | 18 +++++++++---------
>  drivers/spi/spi-rpc-if.c        | 14 +++++++-------
>  include/memory/renesas-rpc-if.h | 16 ++++++++--------
>  4 files changed, 40 insertions(+), 40 deletions(-)
> 

I need some acks here.

Best regards,
Krzysztof

Re: [PATCH 6/7] memory: renesas-rpc-if: Pass device instead of rpcif to rpcif_*()

[Krzysztof Kozlowski]

On 27/06/2022 17:31, Geert Uytterhoeven wrote:
> Most rpcif_*() API functions do not need access to any other fields in
> the rpcif structure than the device pointer.  Simplify dependencies by
> passing the device pointer instead.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/memory/renesas-rpc-if.c | 32 ++++++++++++++++----------------
>  drivers/mtd/hyperbus/rpc-if.c   | 18 +++++++++---------
>  drivers/spi/spi-rpc-if.c        | 14 +++++++-------
>  include/memory/renesas-rpc-if.h | 16 ++++++++--------
>  4 files changed, 40 insertions(+), 40 deletions(-)
> 

I need some acks here.

Best regards,
Krzysztof

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 5/7] memory: renesas-rpc-if: Move resource acquisition to .probe()

[Geert Uytterhoeven]

Hi Krzysztof,

On Wed, Jun 29, 2022 at 8:26 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 29/06/2022 19:55, Geert Uytterhoeven wrote:
> > On Wed, Jun 29, 2022 at 7:44 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >> On 27/06/2022 17:31, Geert Uytterhoeven wrote:
> >>> While the acquired resources are tied to the lifetime of the RPC-IF core
> >>> device (through the use of managed resource functions), the actual
> >>> resource acquisition is triggered from the HyperBus and SPI child
> >>> drivers.  Due to this mismatch, unbinding and rebinding the child
> >>> drivers manually fails with -EBUSY:
> >>>
> >>>     # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/unbind
> >>>     # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/bind
> >>>     rpc-if ee200000.spi: can't request region for resource [mem 0xee200000-0xee2001ff]
> >>>     rpc-if-hyperflash: probe of rpc-if-hyperflash failed with error -16
> >>>
> >>> Fix this by moving all resource acquisition to the core driver's probe
> >>> routine.
> >>>
> >>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>
> >> This looks like a fix, so how about putting it as first in the series,
> >> so backporting is easy/automatic?
> >
> > Unfortunately that is not as simple as it sounds, as this really
> > depends on patch 4.
>
> You sure? Except rebasing I don't see that. rpcif_sw_init() received the
> rpcif so it had access to all fields.

Yes I am, don't be misguided by the name of the local variable.
The rpcif structure is allocated by the HF or SPI child driver,
and thus not available in the RPC core driver's .probe() function.
The rpc_priv structure (as of patch 4) is allocated by the RPC core driver.

> > I agree patches 1-3 could be moved later, if you think it is worthwhile.
>
> This would not be enough, it has to be first patch to be backportable.

I can make it second? ;-)
Note that that still precludes (easily) backporting s2ram support.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 5/7] memory: renesas-rpc-if: Move resource acquisition to .probe()

[Geert Uytterhoeven]

Hi Krzysztof,

On Wed, Jun 29, 2022 at 8:26 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 29/06/2022 19:55, Geert Uytterhoeven wrote:
> > On Wed, Jun 29, 2022 at 7:44 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >> On 27/06/2022 17:31, Geert Uytterhoeven wrote:
> >>> While the acquired resources are tied to the lifetime of the RPC-IF core
> >>> device (through the use of managed resource functions), the actual
> >>> resource acquisition is triggered from the HyperBus and SPI child
> >>> drivers.  Due to this mismatch, unbinding and rebinding the child
> >>> drivers manually fails with -EBUSY:
> >>>
> >>>     # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/unbind
> >>>     # echo rpc-if-hyperflash > /sys/bus/platform/drivers/rpc-if-hyperflash/bind
> >>>     rpc-if ee200000.spi: can't request region for resource [mem 0xee200000-0xee2001ff]
> >>>     rpc-if-hyperflash: probe of rpc-if-hyperflash failed with error -16
> >>>
> >>> Fix this by moving all resource acquisition to the core driver's probe
> >>> routine.
> >>>
> >>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>
> >> This looks like a fix, so how about putting it as first in the series,
> >> so backporting is easy/automatic?
> >
> > Unfortunately that is not as simple as it sounds, as this really
> > depends on patch 4.
>
> You sure? Except rebasing I don't see that. rpcif_sw_init() received the
> rpcif so it had access to all fields.

Yes I am, don't be misguided by the name of the local variable.
The rpcif structure is allocated by the HF or SPI child driver,
and thus not available in the RPC core driver's .probe() function.
The rpc_priv structure (as of patch 4) is allocated by the RPC core driver.

> > I agree patches 1-3 could be moved later, if you think it is worthwhile.
>
> This would not be enough, it has to be first patch to be backportable.

I can make it second? ;-)
Note that that still precludes (easily) backporting s2ram support.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 6/7] memory: renesas-rpc-if: Pass device instead of rpcif to rpcif_*()

[Miquel Raynal]

Hi Krzysztof,

geert+renesas@glider.be wrote on Mon, 27 Jun 2022 17:31:13 +0200:

> Most rpcif_*() API functions do not need access to any other fields in
> the rpcif structure than the device pointer.  Simplify dependencies by
> passing the device pointer instead.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/memory/renesas-rpc-if.c | 32 ++++++++++++++++----------------
>  drivers/mtd/hyperbus/rpc-if.c   | 18 +++++++++---------

[...]

> diff --git a/drivers/mtd/hyperbus/rpc-if.c b/drivers/mtd/hyperbus/rpc-if.c
> index d00d302434030b20..41734e337ac00e40 100644
> --- a/drivers/mtd/hyperbus/rpc-if.c
> +++ b/drivers/mtd/hyperbus/rpc-if.c
> @@ -56,7 +56,7 @@ static void rpcif_hb_prepare_read(struct rpcif *rpc, void *to,
>  	op.data.nbytes = len;
>  	op.data.buf.in = to;
>  
> -	rpcif_prepare(rpc, &op, NULL, NULL);
> +	rpcif_prepare(rpc->dev, &op, NULL, NULL);
>  }
>  
>  static void rpcif_hb_prepare_write(struct rpcif *rpc, unsigned long to,
> @@ -70,7 +70,7 @@ static void rpcif_hb_prepare_write(struct rpcif *rpc, unsigned long to,
>  	op.data.nbytes = len;
>  	op.data.buf.out = from;
>  
> -	rpcif_prepare(rpc, &op, NULL, NULL);
> +	rpcif_prepare(rpc->dev, &op, NULL, NULL);
>  }
>  
>  static u16 rpcif_hb_read16(struct hyperbus_device *hbdev, unsigned long addr)
> @@ -81,7 +81,7 @@ static u16 rpcif_hb_read16(struct hyperbus_device *hbdev, unsigned long addr)
>  
>  	rpcif_hb_prepare_read(&hyperbus->rpc, &data, addr, 2);
>  
> -	rpcif_manual_xfer(&hyperbus->rpc);
> +	rpcif_manual_xfer(hyperbus->rpc.dev);
>  
>  	return data.x[0];
>  }
> @@ -94,7 +94,7 @@ static void rpcif_hb_write16(struct hyperbus_device *hbdev, unsigned long addr,
>  
>  	rpcif_hb_prepare_write(&hyperbus->rpc, addr, &data, 2);
>  
> -	rpcif_manual_xfer(&hyperbus->rpc);
> +	rpcif_manual_xfer(hyperbus->rpc.dev);
>  }
>  
>  static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
> @@ -105,7 +105,7 @@ static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
>  
>  	rpcif_hb_prepare_read(&hyperbus->rpc, to, from, len);
>  
> -	rpcif_dirmap_read(&hyperbus->rpc, from, len, to);
> +	rpcif_dirmap_read(hyperbus->rpc.dev, from, len, to);
>  }
>  
>  static const struct hyperbus_ops rpcif_hb_ops = {
> @@ -130,9 +130,9 @@ static int rpcif_hb_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, hyperbus);
>  
> -	rpcif_enable_rpm(&hyperbus->rpc);
> +	rpcif_enable_rpm(hyperbus->rpc.dev);
>  
> -	error = rpcif_hw_init(&hyperbus->rpc, true);
> +	error = rpcif_hw_init(hyperbus->rpc.dev, true);
>  	if (error)
>  		goto out_disable_rpm;
>  
> @@ -150,7 +150,7 @@ static int rpcif_hb_probe(struct platform_device *pdev)
>  	return 0;
>  
>  out_disable_rpm:
> -	rpcif_disable_rpm(&hyperbus->rpc);
> +	rpcif_disable_rpm(hyperbus->rpc.dev);
>  	return error;
>  }

This will only apply on top of mtd/next, because that
rpcif_disable_rpm() balance call was very recently contributed by Geert:
https://lore.kernel.org/linux-mtd/f3070e1af480cb252ae183d479a593dbbf947685.1655457790.git.geert+renesas@glider.be/

So we need to first share an immutable tag on the current mtd/next
branch. Richard, that is my vacation gift for you :)

Otherwise,

Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

Re: [PATCH 6/7] memory: renesas-rpc-if: Pass device instead of rpcif to rpcif_*()

[Miquel Raynal]

Hi Krzysztof,

geert+renesas@glider.be wrote on Mon, 27 Jun 2022 17:31:13 +0200:

> Most rpcif_*() API functions do not need access to any other fields in
> the rpcif structure than the device pointer.  Simplify dependencies by
> passing the device pointer instead.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/memory/renesas-rpc-if.c | 32 ++++++++++++++++----------------
>  drivers/mtd/hyperbus/rpc-if.c   | 18 +++++++++---------

[...]

> diff --git a/drivers/mtd/hyperbus/rpc-if.c b/drivers/mtd/hyperbus/rpc-if.c
> index d00d302434030b20..41734e337ac00e40 100644
> --- a/drivers/mtd/hyperbus/rpc-if.c
> +++ b/drivers/mtd/hyperbus/rpc-if.c
> @@ -56,7 +56,7 @@ static void rpcif_hb_prepare_read(struct rpcif *rpc, void *to,
>  	op.data.nbytes = len;
>  	op.data.buf.in = to;
>  
> -	rpcif_prepare(rpc, &op, NULL, NULL);
> +	rpcif_prepare(rpc->dev, &op, NULL, NULL);
>  }
>  
>  static void rpcif_hb_prepare_write(struct rpcif *rpc, unsigned long to,
> @@ -70,7 +70,7 @@ static void rpcif_hb_prepare_write(struct rpcif *rpc, unsigned long to,
>  	op.data.nbytes = len;
>  	op.data.buf.out = from;
>  
> -	rpcif_prepare(rpc, &op, NULL, NULL);
> +	rpcif_prepare(rpc->dev, &op, NULL, NULL);
>  }
>  
>  static u16 rpcif_hb_read16(struct hyperbus_device *hbdev, unsigned long addr)
> @@ -81,7 +81,7 @@ static u16 rpcif_hb_read16(struct hyperbus_device *hbdev, unsigned long addr)
>  
>  	rpcif_hb_prepare_read(&hyperbus->rpc, &data, addr, 2);
>  
> -	rpcif_manual_xfer(&hyperbus->rpc);
> +	rpcif_manual_xfer(hyperbus->rpc.dev);
>  
>  	return data.x[0];
>  }
> @@ -94,7 +94,7 @@ static void rpcif_hb_write16(struct hyperbus_device *hbdev, unsigned long addr,
>  
>  	rpcif_hb_prepare_write(&hyperbus->rpc, addr, &data, 2);
>  
> -	rpcif_manual_xfer(&hyperbus->rpc);
> +	rpcif_manual_xfer(hyperbus->rpc.dev);
>  }
>  
>  static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
> @@ -105,7 +105,7 @@ static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
>  
>  	rpcif_hb_prepare_read(&hyperbus->rpc, to, from, len);
>  
> -	rpcif_dirmap_read(&hyperbus->rpc, from, len, to);
> +	rpcif_dirmap_read(hyperbus->rpc.dev, from, len, to);
>  }
>  
>  static const struct hyperbus_ops rpcif_hb_ops = {
> @@ -130,9 +130,9 @@ static int rpcif_hb_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, hyperbus);
>  
> -	rpcif_enable_rpm(&hyperbus->rpc);
> +	rpcif_enable_rpm(hyperbus->rpc.dev);
>  
> -	error = rpcif_hw_init(&hyperbus->rpc, true);
> +	error = rpcif_hw_init(hyperbus->rpc.dev, true);
>  	if (error)
>  		goto out_disable_rpm;
>  
> @@ -150,7 +150,7 @@ static int rpcif_hb_probe(struct platform_device *pdev)
>  	return 0;
>  
>  out_disable_rpm:
> -	rpcif_disable_rpm(&hyperbus->rpc);
> +	rpcif_disable_rpm(hyperbus->rpc.dev);
>  	return error;
>  }

This will only apply on top of mtd/next, because that
rpcif_disable_rpm() balance call was very recently contributed by Geert:
https://lore.kernel.org/linux-mtd/f3070e1af480cb252ae183d479a593dbbf947685.1655457790.git.geert+renesas@glider.be/

So we need to first share an immutable tag on the current mtd/next
branch. Richard, that is my vacation gift for you :)

Otherwise,

Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 6/7] memory: renesas-rpc-if: Pass device instead of rpcif to rpcif_*()

[Krzysztof Kozlowski]

On 30/06/2022 09:25, Miquel Raynal wrote:
> Hi Krzysztof,
> 
> geert+renesas@glider.be wrote on Mon, 27 Jun 2022 17:31:13 +0200:
> 
>> Most rpcif_*() API functions do not need access to any other fields in
>> the rpcif structure than the device pointer.  Simplify dependencies by
>> passing the device pointer instead.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>>  drivers/memory/renesas-rpc-if.c | 32 ++++++++++++++++----------------
>>  drivers/mtd/hyperbus/rpc-if.c   | 18 +++++++++---------
> 
> [...]
> 
>> diff --git a/drivers/mtd/hyperbus/rpc-if.c b/drivers/mtd/hyperbus/rpc-if.c
>> index d00d302434030b20..41734e337ac00e40 100644
>> --- a/drivers/mtd/hyperbus/rpc-if.c
>> +++ b/drivers/mtd/hyperbus/rpc-if.c
>> @@ -56,7 +56,7 @@ static void rpcif_hb_prepare_read(struct rpcif *rpc, void *to,
>>  	op.data.nbytes = len;
>>  	op.data.buf.in = to;
>>  
>> -	rpcif_prepare(rpc, &op, NULL, NULL);
>> +	rpcif_prepare(rpc->dev, &op, NULL, NULL);
>>  }
>>  
>>  static void rpcif_hb_prepare_write(struct rpcif *rpc, unsigned long to,
>> @@ -70,7 +70,7 @@ static void rpcif_hb_prepare_write(struct rpcif *rpc, unsigned long to,
>>  	op.data.nbytes = len;
>>  	op.data.buf.out = from;
>>  
>> -	rpcif_prepare(rpc, &op, NULL, NULL);
>> +	rpcif_prepare(rpc->dev, &op, NULL, NULL);
>>  }
>>  
>>  static u16 rpcif_hb_read16(struct hyperbus_device *hbdev, unsigned long addr)
>> @@ -81,7 +81,7 @@ static u16 rpcif_hb_read16(struct hyperbus_device *hbdev, unsigned long addr)
>>  
>>  	rpcif_hb_prepare_read(&hyperbus->rpc, &data, addr, 2);
>>  
>> -	rpcif_manual_xfer(&hyperbus->rpc);
>> +	rpcif_manual_xfer(hyperbus->rpc.dev);
>>  
>>  	return data.x[0];
>>  }
>> @@ -94,7 +94,7 @@ static void rpcif_hb_write16(struct hyperbus_device *hbdev, unsigned long addr,
>>  
>>  	rpcif_hb_prepare_write(&hyperbus->rpc, addr, &data, 2);
>>  
>> -	rpcif_manual_xfer(&hyperbus->rpc);
>> +	rpcif_manual_xfer(hyperbus->rpc.dev);
>>  }
>>  
>>  static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
>> @@ -105,7 +105,7 @@ static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
>>  
>>  	rpcif_hb_prepare_read(&hyperbus->rpc, to, from, len);
>>  
>> -	rpcif_dirmap_read(&hyperbus->rpc, from, len, to);
>> +	rpcif_dirmap_read(hyperbus->rpc.dev, from, len, to);
>>  }
>>  
>>  static const struct hyperbus_ops rpcif_hb_ops = {
>> @@ -130,9 +130,9 @@ static int rpcif_hb_probe(struct platform_device *pdev)
>>  
>>  	platform_set_drvdata(pdev, hyperbus);
>>  
>> -	rpcif_enable_rpm(&hyperbus->rpc);
>> +	rpcif_enable_rpm(hyperbus->rpc.dev);
>>  
>> -	error = rpcif_hw_init(&hyperbus->rpc, true);
>> +	error = rpcif_hw_init(hyperbus->rpc.dev, true);
>>  	if (error)
>>  		goto out_disable_rpm;
>>  
>> @@ -150,7 +150,7 @@ static int rpcif_hb_probe(struct platform_device *pdev)
>>  	return 0;
>>  
>>  out_disable_rpm:
>> -	rpcif_disable_rpm(&hyperbus->rpc);
>> +	rpcif_disable_rpm(hyperbus->rpc.dev);
>>  	return error;
>>  }
> 
> This will only apply on top of mtd/next, because that
> rpcif_disable_rpm() balance call was very recently contributed by Geert:
> https://lore.kernel.org/linux-mtd/f3070e1af480cb252ae183d479a593dbbf947685.1655457790.git.geert+renesas@glider.be/
> 
> So we need to first share an immutable tag on the current mtd/next
> branch. Richard, that is my vacation gift for you :)

I don't want entire mtd/next. I could take Renesas hyperbus specific
commits. Another solution is me to rebase on some rcX, if that commit
was sent as fix for current cycle.
The third option is to simply resolve a conflict - which should looks
pretty easy and compile-testable. In that case the commit should be
rebased on my v5.19-rc1.

Best regards,
Krzysztof

Re: [PATCH 6/7] memory: renesas-rpc-if: Pass device instead of rpcif to rpcif_*()

[Krzysztof Kozlowski]

On 30/06/2022 09:25, Miquel Raynal wrote:
> Hi Krzysztof,
> 
> geert+renesas@glider.be wrote on Mon, 27 Jun 2022 17:31:13 +0200:
> 
>> Most rpcif_*() API functions do not need access to any other fields in
>> the rpcif structure than the device pointer.  Simplify dependencies by
>> passing the device pointer instead.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>>  drivers/memory/renesas-rpc-if.c | 32 ++++++++++++++++----------------
>>  drivers/mtd/hyperbus/rpc-if.c   | 18 +++++++++---------
> 
> [...]
> 
>> diff --git a/drivers/mtd/hyperbus/rpc-if.c b/drivers/mtd/hyperbus/rpc-if.c
>> index d00d302434030b20..41734e337ac00e40 100644
>> --- a/drivers/mtd/hyperbus/rpc-if.c
>> +++ b/drivers/mtd/hyperbus/rpc-if.c
>> @@ -56,7 +56,7 @@ static void rpcif_hb_prepare_read(struct rpcif *rpc, void *to,
>>  	op.data.nbytes = len;
>>  	op.data.buf.in = to;
>>  
>> -	rpcif_prepare(rpc, &op, NULL, NULL);
>> +	rpcif_prepare(rpc->dev, &op, NULL, NULL);
>>  }
>>  
>>  static void rpcif_hb_prepare_write(struct rpcif *rpc, unsigned long to,
>> @@ -70,7 +70,7 @@ static void rpcif_hb_prepare_write(struct rpcif *rpc, unsigned long to,
>>  	op.data.nbytes = len;
>>  	op.data.buf.out = from;
>>  
>> -	rpcif_prepare(rpc, &op, NULL, NULL);
>> +	rpcif_prepare(rpc->dev, &op, NULL, NULL);
>>  }
>>  
>>  static u16 rpcif_hb_read16(struct hyperbus_device *hbdev, unsigned long addr)
>> @@ -81,7 +81,7 @@ static u16 rpcif_hb_read16(struct hyperbus_device *hbdev, unsigned long addr)
>>  
>>  	rpcif_hb_prepare_read(&hyperbus->rpc, &data, addr, 2);
>>  
>> -	rpcif_manual_xfer(&hyperbus->rpc);
>> +	rpcif_manual_xfer(hyperbus->rpc.dev);
>>  
>>  	return data.x[0];
>>  }
>> @@ -94,7 +94,7 @@ static void rpcif_hb_write16(struct hyperbus_device *hbdev, unsigned long addr,
>>  
>>  	rpcif_hb_prepare_write(&hyperbus->rpc, addr, &data, 2);
>>  
>> -	rpcif_manual_xfer(&hyperbus->rpc);
>> +	rpcif_manual_xfer(hyperbus->rpc.dev);
>>  }
>>  
>>  static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
>> @@ -105,7 +105,7 @@ static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
>>  
>>  	rpcif_hb_prepare_read(&hyperbus->rpc, to, from, len);
>>  
>> -	rpcif_dirmap_read(&hyperbus->rpc, from, len, to);
>> +	rpcif_dirmap_read(hyperbus->rpc.dev, from, len, to);
>>  }
>>  
>>  static const struct hyperbus_ops rpcif_hb_ops = {
>> @@ -130,9 +130,9 @@ static int rpcif_hb_probe(struct platform_device *pdev)
>>  
>>  	platform_set_drvdata(pdev, hyperbus);
>>  
>> -	rpcif_enable_rpm(&hyperbus->rpc);
>> +	rpcif_enable_rpm(hyperbus->rpc.dev);
>>  
>> -	error = rpcif_hw_init(&hyperbus->rpc, true);
>> +	error = rpcif_hw_init(hyperbus->rpc.dev, true);
>>  	if (error)
>>  		goto out_disable_rpm;
>>  
>> @@ -150,7 +150,7 @@ static int rpcif_hb_probe(struct platform_device *pdev)
>>  	return 0;
>>  
>>  out_disable_rpm:
>> -	rpcif_disable_rpm(&hyperbus->rpc);
>> +	rpcif_disable_rpm(hyperbus->rpc.dev);
>>  	return error;
>>  }
> 
> This will only apply on top of mtd/next, because that
> rpcif_disable_rpm() balance call was very recently contributed by Geert:
> https://lore.kernel.org/linux-mtd/f3070e1af480cb252ae183d479a593dbbf947685.1655457790.git.geert+renesas@glider.be/
> 
> So we need to first share an immutable tag on the current mtd/next
> branch. Richard, that is my vacation gift for you :)

I don't want entire mtd/next. I could take Renesas hyperbus specific
commits. Another solution is me to rebase on some rcX, if that commit
was sent as fix for current cycle.
The third option is to simply resolve a conflict - which should looks
pretty easy and compile-testable. In that case the commit should be
rebased on my v5.19-rc1.

Best regards,
Krzysztof

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 5/7] memory: renesas-rpc-if: Move resource acquisition to .probe()

[Krzysztof Kozlowski]

On 29/06/2022 20:48, Geert Uytterhoeven wrote:
>>
>> You sure? Except rebasing I don't see that. rpcif_sw_init() received the
>> rpcif so it had access to all fields.
> 
> Yes I am, don't be misguided by the name of the local variable.
> The rpcif structure is allocated by the HF or SPI child driver,
> and thus not available in the RPC core driver's .probe() function.
> The rpc_priv structure (as of patch 4) is allocated by the RPC core driver.
> 
>>> I agree patches 1-3 could be moved later, if you think it is worthwhile.
>>
>> This would not be enough, it has to be first patch to be backportable.
> 
> I can make it second? ;-)

Why? The point is that this commit should have Fixes or Cc-stable tag.
If you make it depending on other non-backportable commit, stable folks
cannot pull it automatically.

> Note that that still precludes (easily) backporting s2ram support.

But S2R is a feature so it won't be backported...

Best regards,
Krzysztof

Re: [PATCH 5/7] memory: renesas-rpc-if: Move resource acquisition to .probe()

[Krzysztof Kozlowski]

On 29/06/2022 20:48, Geert Uytterhoeven wrote:
>>
>> You sure? Except rebasing I don't see that. rpcif_sw_init() received the
>> rpcif so it had access to all fields.
> 
> Yes I am, don't be misguided by the name of the local variable.
> The rpcif structure is allocated by the HF or SPI child driver,
> and thus not available in the RPC core driver's .probe() function.
> The rpc_priv structure (as of patch 4) is allocated by the RPC core driver.
> 
>>> I agree patches 1-3 could be moved later, if you think it is worthwhile.
>>
>> This would not be enough, it has to be first patch to be backportable.
> 
> I can make it second? ;-)

Why? The point is that this commit should have Fixes or Cc-stable tag.
If you make it depending on other non-backportable commit, stable folks
cannot pull it automatically.

> Note that that still precludes (easily) backporting s2ram support.

But S2R is a feature so it won't be backported...

Best regards,
Krzysztof

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 6/7] memory: renesas-rpc-if: Pass device instead of rpcif to rpcif_*()

[Miquel Raynal]

Hi Krzysztof,

krzysztof.kozlowski@linaro.org wrote on Thu, 30 Jun 2022 10:46:51 +0200:

> On 30/06/2022 09:25, Miquel Raynal wrote:
> > Hi Krzysztof,
> > 
> > geert+renesas@glider.be wrote on Mon, 27 Jun 2022 17:31:13 +0200:
> >   
> >> Most rpcif_*() API functions do not need access to any other fields in
> >> the rpcif structure than the device pointer.  Simplify dependencies by
> >> passing the device pointer instead.
> >>
> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> ---
> >>  drivers/memory/renesas-rpc-if.c | 32 ++++++++++++++++----------------
> >>  drivers/mtd/hyperbus/rpc-if.c   | 18 +++++++++---------  
> > 
> > [...]
> >   
> >> diff --git a/drivers/mtd/hyperbus/rpc-if.c b/drivers/mtd/hyperbus/rpc-if.c
> >> index d00d302434030b20..41734e337ac00e40 100644
> >> --- a/drivers/mtd/hyperbus/rpc-if.c
> >> +++ b/drivers/mtd/hyperbus/rpc-if.c
> >> @@ -56,7 +56,7 @@ static void rpcif_hb_prepare_read(struct rpcif *rpc, void *to,
> >>  	op.data.nbytes = len;
> >>  	op.data.buf.in = to;
> >>  
> >> -	rpcif_prepare(rpc, &op, NULL, NULL);
> >> +	rpcif_prepare(rpc->dev, &op, NULL, NULL);
> >>  }
> >>  
> >>  static void rpcif_hb_prepare_write(struct rpcif *rpc, unsigned long to,
> >> @@ -70,7 +70,7 @@ static void rpcif_hb_prepare_write(struct rpcif *rpc, unsigned long to,
> >>  	op.data.nbytes = len;
> >>  	op.data.buf.out = from;
> >>  
> >> -	rpcif_prepare(rpc, &op, NULL, NULL);
> >> +	rpcif_prepare(rpc->dev, &op, NULL, NULL);
> >>  }
> >>  
> >>  static u16 rpcif_hb_read16(struct hyperbus_device *hbdev, unsigned long addr)
> >> @@ -81,7 +81,7 @@ static u16 rpcif_hb_read16(struct hyperbus_device *hbdev, unsigned long addr)
> >>  
> >>  	rpcif_hb_prepare_read(&hyperbus->rpc, &data, addr, 2);
> >>  
> >> -	rpcif_manual_xfer(&hyperbus->rpc);
> >> +	rpcif_manual_xfer(hyperbus->rpc.dev);
> >>  
> >>  	return data.x[0];
> >>  }
> >> @@ -94,7 +94,7 @@ static void rpcif_hb_write16(struct hyperbus_device *hbdev, unsigned long addr,
> >>  
> >>  	rpcif_hb_prepare_write(&hyperbus->rpc, addr, &data, 2);
> >>  
> >> -	rpcif_manual_xfer(&hyperbus->rpc);
> >> +	rpcif_manual_xfer(hyperbus->rpc.dev);
> >>  }
> >>  
> >>  static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
> >> @@ -105,7 +105,7 @@ static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
> >>  
> >>  	rpcif_hb_prepare_read(&hyperbus->rpc, to, from, len);
> >>  
> >> -	rpcif_dirmap_read(&hyperbus->rpc, from, len, to);
> >> +	rpcif_dirmap_read(hyperbus->rpc.dev, from, len, to);
> >>  }
> >>  
> >>  static const struct hyperbus_ops rpcif_hb_ops = {
> >> @@ -130,9 +130,9 @@ static int rpcif_hb_probe(struct platform_device *pdev)
> >>  
> >>  	platform_set_drvdata(pdev, hyperbus);
> >>  
> >> -	rpcif_enable_rpm(&hyperbus->rpc);
> >> +	rpcif_enable_rpm(hyperbus->rpc.dev);
> >>  
> >> -	error = rpcif_hw_init(&hyperbus->rpc, true);
> >> +	error = rpcif_hw_init(hyperbus->rpc.dev, true);
> >>  	if (error)
> >>  		goto out_disable_rpm;
> >>  
> >> @@ -150,7 +150,7 @@ static int rpcif_hb_probe(struct platform_device *pdev)
> >>  	return 0;
> >>  
> >>  out_disable_rpm:
> >> -	rpcif_disable_rpm(&hyperbus->rpc);
> >> +	rpcif_disable_rpm(hyperbus->rpc.dev);
> >>  	return error;
> >>  }  
> > 
> > This will only apply on top of mtd/next, because that
> > rpcif_disable_rpm() balance call was very recently contributed by Geert:
> > https://lore.kernel.org/linux-mtd/f3070e1af480cb252ae183d479a593dbbf947685.1655457790.git.geert+renesas@glider.be/
> > 
> > So we need to first share an immutable tag on the current mtd/next
> > branch. Richard, that is my vacation gift for you :)  
> 
> I don't want entire mtd/next. I could take Renesas hyperbus specific
> commits.

I applied this patch a week ago and did not remembered there was a
possibly conflicting series aside.

> Another solution is me to rebase on some rcX, if that commit
> was sent as fix for current cycle.

Unfortunately, no.

> The third option is to simply resolve a conflict - which should looks
> pretty easy and compile-testable. In that case the commit should be
> rebased on my v5.19-rc1.

I believe it should work. As this is mostly a cleanup, you can also
wait 5.20-rc1.

Thanks,
Miquèl

Re: [PATCH 6/7] memory: renesas-rpc-if: Pass device instead of rpcif to rpcif_*()

[Miquel Raynal]

Hi Krzysztof,

krzysztof.kozlowski@linaro.org wrote on Thu, 30 Jun 2022 10:46:51 +0200:

> On 30/06/2022 09:25, Miquel Raynal wrote:
> > Hi Krzysztof,
> > 
> > geert+renesas@glider.be wrote on Mon, 27 Jun 2022 17:31:13 +0200:
> >   
> >> Most rpcif_*() API functions do not need access to any other fields in
> >> the rpcif structure than the device pointer.  Simplify dependencies by
> >> passing the device pointer instead.
> >>
> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> ---
> >>  drivers/memory/renesas-rpc-if.c | 32 ++++++++++++++++----------------
> >>  drivers/mtd/hyperbus/rpc-if.c   | 18 +++++++++---------  
> > 
> > [...]
> >   
> >> diff --git a/drivers/mtd/hyperbus/rpc-if.c b/drivers/mtd/hyperbus/rpc-if.c
> >> index d00d302434030b20..41734e337ac00e40 100644
> >> --- a/drivers/mtd/hyperbus/rpc-if.c
> >> +++ b/drivers/mtd/hyperbus/rpc-if.c
> >> @@ -56,7 +56,7 @@ static void rpcif_hb_prepare_read(struct rpcif *rpc, void *to,
> >>  	op.data.nbytes = len;
> >>  	op.data.buf.in = to;
> >>  
> >> -	rpcif_prepare(rpc, &op, NULL, NULL);
> >> +	rpcif_prepare(rpc->dev, &op, NULL, NULL);
> >>  }
> >>  
> >>  static void rpcif_hb_prepare_write(struct rpcif *rpc, unsigned long to,
> >> @@ -70,7 +70,7 @@ static void rpcif_hb_prepare_write(struct rpcif *rpc, unsigned long to,
> >>  	op.data.nbytes = len;
> >>  	op.data.buf.out = from;
> >>  
> >> -	rpcif_prepare(rpc, &op, NULL, NULL);
> >> +	rpcif_prepare(rpc->dev, &op, NULL, NULL);
> >>  }
> >>  
> >>  static u16 rpcif_hb_read16(struct hyperbus_device *hbdev, unsigned long addr)
> >> @@ -81,7 +81,7 @@ static u16 rpcif_hb_read16(struct hyperbus_device *hbdev, unsigned long addr)
> >>  
> >>  	rpcif_hb_prepare_read(&hyperbus->rpc, &data, addr, 2);
> >>  
> >> -	rpcif_manual_xfer(&hyperbus->rpc);
> >> +	rpcif_manual_xfer(hyperbus->rpc.dev);
> >>  
> >>  	return data.x[0];
> >>  }
> >> @@ -94,7 +94,7 @@ static void rpcif_hb_write16(struct hyperbus_device *hbdev, unsigned long addr,
> >>  
> >>  	rpcif_hb_prepare_write(&hyperbus->rpc, addr, &data, 2);
> >>  
> >> -	rpcif_manual_xfer(&hyperbus->rpc);
> >> +	rpcif_manual_xfer(hyperbus->rpc.dev);
> >>  }
> >>  
> >>  static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
> >> @@ -105,7 +105,7 @@ static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
> >>  
> >>  	rpcif_hb_prepare_read(&hyperbus->rpc, to, from, len);
> >>  
> >> -	rpcif_dirmap_read(&hyperbus->rpc, from, len, to);
> >> +	rpcif_dirmap_read(hyperbus->rpc.dev, from, len, to);
> >>  }
> >>  
> >>  static const struct hyperbus_ops rpcif_hb_ops = {
> >> @@ -130,9 +130,9 @@ static int rpcif_hb_probe(struct platform_device *pdev)
> >>  
> >>  	platform_set_drvdata(pdev, hyperbus);
> >>  
> >> -	rpcif_enable_rpm(&hyperbus->rpc);
> >> +	rpcif_enable_rpm(hyperbus->rpc.dev);
> >>  
> >> -	error = rpcif_hw_init(&hyperbus->rpc, true);
> >> +	error = rpcif_hw_init(hyperbus->rpc.dev, true);
> >>  	if (error)
> >>  		goto out_disable_rpm;
> >>  
> >> @@ -150,7 +150,7 @@ static int rpcif_hb_probe(struct platform_device *pdev)
> >>  	return 0;
> >>  
> >>  out_disable_rpm:
> >> -	rpcif_disable_rpm(&hyperbus->rpc);
> >> +	rpcif_disable_rpm(hyperbus->rpc.dev);
> >>  	return error;
> >>  }  
> > 
> > This will only apply on top of mtd/next, because that
> > rpcif_disable_rpm() balance call was very recently contributed by Geert:
> > https://lore.kernel.org/linux-mtd/f3070e1af480cb252ae183d479a593dbbf947685.1655457790.git.geert+renesas@glider.be/
> > 
> > So we need to first share an immutable tag on the current mtd/next
> > branch. Richard, that is my vacation gift for you :)  
> 
> I don't want entire mtd/next. I could take Renesas hyperbus specific
> commits.

I applied this patch a week ago and did not remembered there was a
possibly conflicting series aside.

> Another solution is me to rebase on some rcX, if that commit
> was sent as fix for current cycle.

Unfortunately, no.

> The third option is to simply resolve a conflict - which should looks
> pretty easy and compile-testable. In that case the commit should be
> rebased on my v5.19-rc1.

I believe it should work. As this is mostly a cleanup, you can also
wait 5.20-rc1.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 5/7] memory: renesas-rpc-if: Move resource acquisition to .probe()

[Geert Uytterhoeven]

Hi Krzysztof,

On Thu, Jun 30, 2022 at 10:48 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 29/06/2022 20:48, Geert Uytterhoeven wrote:
> >> You sure? Except rebasing I don't see that. rpcif_sw_init() received the
> >> rpcif so it had access to all fields.
> >
> > Yes I am, don't be misguided by the name of the local variable.
> > The rpcif structure is allocated by the HF or SPI child driver,
> > and thus not available in the RPC core driver's .probe() function.
> > The rpc_priv structure (as of patch 4) is allocated by the RPC core driver.
> >
> >>> I agree patches 1-3 could be moved later, if you think it is worthwhile.
> >>
> >> This would not be enough, it has to be first patch to be backportable.
> >
> > I can make it second? ;-)
>
> Why? The point is that this commit should have Fixes or Cc-stable tag.
> If you make it depending on other non-backportable commit, stable folks
> cannot pull it automatically.

Because the current driver structure does not allow us to fix the
problem in a simple way.  Hence the need for patch 4 first.

> > Note that that still precludes (easily) backporting s2ram support.
>
> But S2R is a feature so it won't be backported...

Working rebind is a feature, too?

Actually non-working s2ram is worse, as it returns corrupted data
(haven't dared to try writing after s2ram yet ;-),  while non-working
rebind means you just cannot access the device anymore.

But note there are still issues with s2ram...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 5/7] memory: renesas-rpc-if: Move resource acquisition to .probe()

[Geert Uytterhoeven]

Hi Krzysztof,

On Thu, Jun 30, 2022 at 10:48 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 29/06/2022 20:48, Geert Uytterhoeven wrote:
> >> You sure? Except rebasing I don't see that. rpcif_sw_init() received the
> >> rpcif so it had access to all fields.
> >
> > Yes I am, don't be misguided by the name of the local variable.
> > The rpcif structure is allocated by the HF or SPI child driver,
> > and thus not available in the RPC core driver's .probe() function.
> > The rpc_priv structure (as of patch 4) is allocated by the RPC core driver.
> >
> >>> I agree patches 1-3 could be moved later, if you think it is worthwhile.
> >>
> >> This would not be enough, it has to be first patch to be backportable.
> >
> > I can make it second? ;-)
>
> Why? The point is that this commit should have Fixes or Cc-stable tag.
> If you make it depending on other non-backportable commit, stable folks
> cannot pull it automatically.

Because the current driver structure does not allow us to fix the
problem in a simple way.  Hence the need for patch 4 first.

> > Note that that still precludes (easily) backporting s2ram support.
>
> But S2R is a feature so it won't be backported...

Working rebind is a feature, too?

Actually non-working s2ram is worse, as it returns corrupted data
(haven't dared to try writing after s2ram yet ;-),  while non-working
rebind means you just cannot access the device anymore.

But note there are still issues with s2ram...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 7/7] memory: renesas-rpc-if: Reinitialize registers during system resume

[Geert Uytterhoeven]

On Mon, Jun 27, 2022 at 5:31 PM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> During PSCI system suspend, R-Car Gen3 SoCs may be powered down, and
> thus the RPC-IF register state may be lost.  Consequently, when using
> the RPC-IF after system resume, data corruption may happen.
>
> Fix this by reinitializing the hardware state during system resume.
> As this requires resuming the RPC-IF core device, this can only be done
> when the device is under active control of the HyperBus or SPI child
> driver.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

For v2, I dropped this patch from the series.

Apparently this patch is not needed, nor does it have any impact on
HyperFLASH read operation on Salvator-XS with R-Car M3-N ES1.0 and
Ebisu-4D with R-Car E3 ES1.0.

On Salvator-X with R-Car M3-W ES1.0, there is a different issue causing
random bit flips (which is not solved by the Strobe Timing Adjustment
bit (STRTIM) fix for R-Car M3-W ES1.x in the BSP).

On Salvator-XS with R-Car H3-ES2.0, corruption is seen after s2ram.
TL;DR: while this patch does have impact on that, RPC operation after
s2ram is still not guaranteed, and the core issue is still not
understood.

---
For testing, I use the following command to read /dev/mtdblock1 (which
contains the BL2 bootloader) and check its checksum 100 times:

    time sha256sum -c <(yes $(cat mtdblock1.sha256sum) | head -100)

After boot and s2idle, the success rate is 100%.

1. Without this patch, the failure rate after s2ram is ca. 65%.

When splitting and comparing the data read back, some blocks of 64 KiB
(but not always the same on different runs) have been replaced by bad
data, containing either data from elsewhere, or all ones.  The latter is
probably the same symptom as the former, as the HyperFLASH does contain
blocks with all-ones data.

The data from elsewhere looks like e.g.:

    00046f10  75 75 69 64 5f 64 69 73  75 75 69 64 5f 64 69 73
|uuid_disuuid_dis|
    00046f20  64 72 27 0a 20 20 20 20  64 72 27 0a 20 20 20 20  |dr'.
  dr'.    |
    00046f30  6c 69 67 6e 65 64 20 74  6c 69 67 6e 65 64 20 74
|ligned tligned t|
    00046f40  61 64 64 72 32 20 63 6f  61 64 64 72 32 20 63 6f  |addr2
coaddr2 co|
    00046f50  0a 6d 6d 63 20 72 70 6d  0a 6d 6d 63 20 72 70 6d  |.mmc
rpm.mmc rpm|
    00046f60  20 6c 61 72 67 65 72 0a  20 6c 61 72 67 65 72 0a  |
larger. larger.|
    00046f70  6d 32 00 63 6d 6d 31 00  6d 32 00 63 6d 6d 31 00
|m2.cmm1.m2.cmm1.|
    00046f80  32 5f 64 61 74 61 34 00  32 5f 64 61 74 61 34 00
|2_data4.2_data4.|
    00046f90  31 00 47 50 5f 35 5f 31  31 00 47 50 5f 35 5f 31
|1.GP_5_11.GP_5_1|
    00046fa0  65 78 63 65 65 64 65 64  65 78 63 65 65 64 65 64
|exceededexceeded|
    00046fb0  30 34 2d 72 63 34 2d 30  30 34 2d 72 63 34 2d 30
|04-rc4-004-rc4-0|

which seems to originate from two copies of the first 8 bytes of each of
the following lines, found elsewhere in the HyperFLASH:

    006f1000  75 75 69 64 5f 64 69 73  6b 3d 00 6e 61 6d 65 3d
|uuid_disk=.name=|
    ...
    006f2000  64 72 27 0a 20 20 20 20  20 20 70 61 73 73 69 6e  |dr'.
    passin|
    ...
    006f3000  6c 69 67 6e 65 64 20 74  6f 0a 20 20 20 20 20 20
|ligned to.      |
    ...
    006f4000  61 64 64 72 32 20 63 6f  75 6e 74 00 6d 65 6d 6f  |addr2
count.memo|
    ...
    006f5000  0a 6d 6d 63 20 72 70 6d  62 20 6b 65 79 20 3c 61  |.mmc
rpmb key <a|
    ...
    006f6000  20 6c 61 72 67 65 72 0a  00 75 6e 7a 69 70 00 75  |
larger..unzip.u|
    ...
    006f7000  6d 32 00 63 6d 6d 31 00  63 6d 6d 30 00 63 73 69
|m2.cmm1.cmm0.csi|
    ...
    006f8000  32 5f 64 61 74 61 34 00  73 64 68 69 32 5f 64 61
|2_data4.sdhi2_da|
    ...
    006f9000  31 00 47 50 5f 35 5f 31  32 00 47 50 5f 35 5f 31
|1.GP_5_12.GP_5_1|
    ...
    006fa000  65 78 63 65 65 64 65 64  00 0a 25 73 3b 20 73 74
|exceeded..%s; st|
    ...
    006fb000  30 34 2d 72 63 34 2d 30  30 30 38 32 2d 67 35 34
|04-rc4-00082-g54|

For both hexdumps above, offsets are absolute FLASH offsets, not relative
partition offsets.  Still, there is some similarity (e.g. 0x46f10 vs.
0x6f1000).

2. With this patch, the failure rate of 100 reads after s2ram is either
0%, or 100%.  The same is true after subsequent s2ram operations.

Contrary to before this patch, in case of corruption, the data read back
is always the same: i.e. either the good data is read back, or the exact
same bad data is read back.  In case of bad data, some blocks of 64 KiB
have been replaced by blocks containing either data from elsewhere,
or all ones again....

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 7/7] memory: renesas-rpc-if: Reinitialize registers during system resume

[Geert Uytterhoeven]

On Mon, Jun 27, 2022 at 5:31 PM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> During PSCI system suspend, R-Car Gen3 SoCs may be powered down, and
> thus the RPC-IF register state may be lost.  Consequently, when using
> the RPC-IF after system resume, data corruption may happen.
>
> Fix this by reinitializing the hardware state during system resume.
> As this requires resuming the RPC-IF core device, this can only be done
> when the device is under active control of the HyperBus or SPI child
> driver.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

For v2, I dropped this patch from the series.

Apparently this patch is not needed, nor does it have any impact on
HyperFLASH read operation on Salvator-XS with R-Car M3-N ES1.0 and
Ebisu-4D with R-Car E3 ES1.0.

On Salvator-X with R-Car M3-W ES1.0, there is a different issue causing
random bit flips (which is not solved by the Strobe Timing Adjustment
bit (STRTIM) fix for R-Car M3-W ES1.x in the BSP).

On Salvator-XS with R-Car H3-ES2.0, corruption is seen after s2ram.
TL;DR: while this patch does have impact on that, RPC operation after
s2ram is still not guaranteed, and the core issue is still not
understood.

---
For testing, I use the following command to read /dev/mtdblock1 (which
contains the BL2 bootloader) and check its checksum 100 times:

    time sha256sum -c <(yes $(cat mtdblock1.sha256sum) | head -100)

After boot and s2idle, the success rate is 100%.

1. Without this patch, the failure rate after s2ram is ca. 65%.

When splitting and comparing the data read back, some blocks of 64 KiB
(but not always the same on different runs) have been replaced by bad
data, containing either data from elsewhere, or all ones.  The latter is
probably the same symptom as the former, as the HyperFLASH does contain
blocks with all-ones data.

The data from elsewhere looks like e.g.:

    00046f10  75 75 69 64 5f 64 69 73  75 75 69 64 5f 64 69 73
|uuid_disuuid_dis|
    00046f20  64 72 27 0a 20 20 20 20  64 72 27 0a 20 20 20 20  |dr'.
  dr'.    |
    00046f30  6c 69 67 6e 65 64 20 74  6c 69 67 6e 65 64 20 74
|ligned tligned t|
    00046f40  61 64 64 72 32 20 63 6f  61 64 64 72 32 20 63 6f  |addr2
coaddr2 co|
    00046f50  0a 6d 6d 63 20 72 70 6d  0a 6d 6d 63 20 72 70 6d  |.mmc
rpm.mmc rpm|
    00046f60  20 6c 61 72 67 65 72 0a  20 6c 61 72 67 65 72 0a  |
larger. larger.|
    00046f70  6d 32 00 63 6d 6d 31 00  6d 32 00 63 6d 6d 31 00
|m2.cmm1.m2.cmm1.|
    00046f80  32 5f 64 61 74 61 34 00  32 5f 64 61 74 61 34 00
|2_data4.2_data4.|
    00046f90  31 00 47 50 5f 35 5f 31  31 00 47 50 5f 35 5f 31
|1.GP_5_11.GP_5_1|
    00046fa0  65 78 63 65 65 64 65 64  65 78 63 65 65 64 65 64
|exceededexceeded|
    00046fb0  30 34 2d 72 63 34 2d 30  30 34 2d 72 63 34 2d 30
|04-rc4-004-rc4-0|

which seems to originate from two copies of the first 8 bytes of each of
the following lines, found elsewhere in the HyperFLASH:

    006f1000  75 75 69 64 5f 64 69 73  6b 3d 00 6e 61 6d 65 3d
|uuid_disk=.name=|
    ...
    006f2000  64 72 27 0a 20 20 20 20  20 20 70 61 73 73 69 6e  |dr'.
    passin|
    ...
    006f3000  6c 69 67 6e 65 64 20 74  6f 0a 20 20 20 20 20 20
|ligned to.      |
    ...
    006f4000  61 64 64 72 32 20 63 6f  75 6e 74 00 6d 65 6d 6f  |addr2
count.memo|
    ...
    006f5000  0a 6d 6d 63 20 72 70 6d  62 20 6b 65 79 20 3c 61  |.mmc
rpmb key <a|
    ...
    006f6000  20 6c 61 72 67 65 72 0a  00 75 6e 7a 69 70 00 75  |
larger..unzip.u|
    ...
    006f7000  6d 32 00 63 6d 6d 31 00  63 6d 6d 30 00 63 73 69
|m2.cmm1.cmm0.csi|
    ...
    006f8000  32 5f 64 61 74 61 34 00  73 64 68 69 32 5f 64 61
|2_data4.sdhi2_da|
    ...
    006f9000  31 00 47 50 5f 35 5f 31  32 00 47 50 5f 35 5f 31
|1.GP_5_12.GP_5_1|
    ...
    006fa000  65 78 63 65 65 64 65 64  00 0a 25 73 3b 20 73 74
|exceeded..%s; st|
    ...
    006fb000  30 34 2d 72 63 34 2d 30  30 30 38 32 2d 67 35 34
|04-rc4-00082-g54|

For both hexdumps above, offsets are absolute FLASH offsets, not relative
partition offsets.  Still, there is some similarity (e.g. 0x46f10 vs.
0x6f1000).

2. With this patch, the failure rate of 100 reads after s2ram is either
0%, or 100%.  The same is true after subsequent s2ram operations.

Contrary to before this patch, in case of corruption, the data read back
is always the same: i.e. either the good data is read back, or the exact
same bad data is read back.  In case of bad data, some blocks of 64 KiB
have been replaced by blocks containing either data from elsewhere,
or all ones again....

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds