[PATCH v9 0/2] gpmc wait pin additions

[PATCH v9 0/2] gpmc wait pin additions

[B. Niedermayr]

From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>

Currently it is not possible to configure the WAIT0PINPOLARITY and
WAIT1PINPOLARITY bits of the GPMC_CONFIG register directly via
device tree properties.

It is also not possible to use the same wait-pin for different
cs-regions.

While the current implementation may fullfill most usecases, it may not
be sufficient for more complex setups (e.g. FPGA/ASIC interfaces), where
more complex interfacing options where possible.

For example interfacing an ASIC which offers multiple cs-regions but
only one waitpin the current driver and dt-bindings are not sufficient.

While using the same waitpin for different cs-regions worked for older
kernels (4.14) the omap-gpmc.c driver refused to probe (-EBUSY) with
newer kernels (>5.10).

Changes since v1:
  * Rebase against recent 6.0.0-rc3 kernel
  * Updated eMail recipients list
Changes since v2:
  * Remove the gpmc register configuration out of the gpiochip
    callbacks. In this case the memory interface configuration
    is not done via gpio bindings.
  * Some minor code fixes
  * Changed git commit descriptions
Change since v3:
  * Use a uint32 dt-property instean a boolean one
  * If the dt-property is not set, then don't touch the
    GPMC_CONFIG register
  * Changed git commit descriptions
Changes since v4:
  * Use checkpatch with "--strict" option
  * Moved wait-pin sanity checks to gpmc_read_settings_dt()
  * Always assign WAITPINPOLARITY_DEFAULT on error cases
  * Track waitpin allocation within gpmc for determine
    allocation origin
Changes since v5:
  * Tracking of wait-pin allocations with polarity change detection
    * Introduced a new struct gpmc_waitpin
  * Add GPMC_* to global header definitions
  * Don't allow GPMC_WAITPINPOLARITY_DEFAULT when parsing dt-properties
  * Squashed wait-pin-polarity and shared-wait-pin patches, since they
    should not be separated
Changes since v6:
  * Move wait-pin allocation into gpmc_probe()
  * Fix s/gpmc/GPMC/ in commit description
  * use ti,wait-pin-polarity instead of gpmc,wait-pin-polarity
  * Refactored if clause in gpmc_alloc_waitpin()
  * Revert values for GPMC_WAITPINPOLARITY_ACTIVE_LOW and
    GPMC_WAITPINPOLARITY_ACTIVE_HIGH.
    Use the exact same values which are written into the register.
Changes since v7:
  * Renamed GPMC_WAITPINPOLARITY_DEFAULT to GPMC_WAITPINPOLARITY_INVALID
  * Call gpiochip_request_own_desc() only on first wait-pin allocation
  * Fixed use of old "gpmc,wait-pin-polarity" property.
Changes since v8:
  * Fixed rebase against v6.0
  * Add correct patch series version to each patch

Benedikt Niedermayr (2):
  memory: omap-gpmc: wait pin additions
  dt-bindings: memory-controllers: gpmc-child: add wait-pin polarity

 .../memory-controllers/ti,gpmc-child.yaml     |   7 +
 drivers/memory/omap-gpmc.c                    | 122 ++++++++++++++++--
 include/linux/platform_data/gpmc-omap.h       |   8 ++
 3 files changed, 124 insertions(+), 13 deletions(-)

--
2.25.1

[PATCH v9 1/2] memory: omap-gpmc: wait pin additions

[B. Niedermayr]

From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>

This patch introduces support for setting the wait-pin polarity as well
as using the same wait-pin for different CS regions.

The waitpin polarity can be configured via the WAITPIN<X>POLARITY bits
in the GPMC_CONFIG register. This is currently not supported by the
driver. This patch adds support for setting the required register bits
with the "ti,wait-pin-polarity" dt-property.

The wait-pin can also be shared between different CS regions for special
usecases. Therefore GPMC must keep track of wait-pin allocations, so it
knows that either GPMC itself or another driver has the ownership.

Signed-off-by: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
---
 drivers/memory/omap-gpmc.c              | 122 +++++++++++++++++++++---
 include/linux/platform_data/gpmc-omap.h |   8 ++
 2 files changed, 117 insertions(+), 13 deletions(-)

diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index 2351f2708da2..e427572712e2 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -134,6 +134,7 @@
 #define GPMC_CONFIG_DEV_SIZE	0x00000002
 #define GPMC_CONFIG_DEV_TYPE	0x00000003
 
+#define GPMC_CONFIG_WAITPINPOLARITY(pin)	(BIT(pin) << 8)
 #define GPMC_CONFIG1_WRAPBURST_SUPP     (1 << 31)
 #define GPMC_CONFIG1_READMULTIPLE_SUPP  (1 << 30)
 #define GPMC_CONFIG1_READTYPE_ASYNC     (0 << 29)
@@ -229,6 +230,12 @@ struct omap3_gpmc_regs {
 	struct gpmc_cs_config cs_context[GPMC_CS_NUM];
 };
 
+struct gpmc_waitpin {
+	u32 pin;
+	u32 polarity;
+	struct gpio_desc *desc;
+};
+
 struct gpmc_device {
 	struct device *dev;
 	int irq;
@@ -236,6 +243,7 @@ struct gpmc_device {
 	struct gpio_chip gpio_chip;
 	struct notifier_block nb;
 	struct omap3_gpmc_regs context;
+	struct gpmc_waitpin *waitpins;
 	int nirqs;
 	unsigned int is_suspended:1;
 	struct resource *data;
@@ -1035,6 +1043,62 @@ void gpmc_cs_free(int cs)
 }
 EXPORT_SYMBOL(gpmc_cs_free);
 
+static bool gpmc_is_valid_waitpin(u32 waitpin)
+{
+	return waitpin >= 0 && waitpin < gpmc_nr_waitpins;
+}
+
+static int gpmc_alloc_waitpin(struct gpmc_device *gpmc,
+			      struct gpmc_settings *p)
+{
+	int ret;
+	struct gpmc_waitpin *waitpin;
+	struct gpio_desc *waitpin_desc;
+
+	if (!gpmc_is_valid_waitpin(p->wait_pin))
+		return -EINVAL;
+
+	waitpin = &gpmc->waitpins[p->wait_pin];
+
+	if (!waitpin->desc) {
+		/* Reserve the GPIO for wait pin usage.
+		 * GPIO polarity doesn't matter here. Wait pin polarity
+		 * is set in GPMC_CONFIG register.
+		 */
+		waitpin_desc = gpiochip_request_own_desc(&gpmc->gpio_chip,
+							 p->wait_pin, "WAITPIN",
+							 GPIO_ACTIVE_HIGH,
+							 GPIOD_IN);
+
+		ret = PTR_ERR(waitpin_desc);
+		if (IS_ERR(waitpin_desc) && ret != -EBUSY)
+			return ret;
+
+		/* New wait pin */
+		waitpin->desc = waitpin_desc;
+		waitpin->pin = p->wait_pin;
+		waitpin->polarity = p->wait_pin_polarity;
+	} else {
+		/* Shared wait pin */
+		if (p->wait_pin_polarity != waitpin->polarity ||
+		    p->wait_pin != waitpin->pin) {
+			dev_err(gpmc->dev,
+				"shared-wait-pin: invalid configuration\n");
+			return -EINVAL;
+		}
+		dev_info(gpmc->dev, "shared wait-pin: %d\n", waitpin->pin);
+	}
+
+	return 0;
+}
+
+static void gpmc_free_waitpin(struct gpmc_device *gpmc,
+			      int wait_pin)
+{
+	if (gpmc_is_valid_waitpin(wait_pin))
+		gpiochip_free_own_desc(gpmc->waitpins[wait_pin].desc);
+}
+
 /**
  * gpmc_configure - write request to configure gpmc
  * @cmd: command type
@@ -1886,6 +1950,17 @@ int gpmc_cs_program_settings(int cs, struct gpmc_settings *p)
 
 	gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, config1);
 
+	if (p->wait_pin_polarity != GPMC_WAITPINPOLARITY_INVALID) {
+		config1 = gpmc_read_reg(GPMC_CONFIG);
+
+		if (p->wait_pin_polarity == GPMC_WAITPINPOLARITY_ACTIVE_LOW)
+			config1 &= ~GPMC_CONFIG_WAITPINPOLARITY(p->wait_pin);
+		else if (p->wait_pin_polarity == GPMC_WAITPINPOLARITY_ACTIVE_HIGH)
+			config1 |= GPMC_CONFIG_WAITPINPOLARITY(p->wait_pin);
+
+		gpmc_write_reg(GPMC_CONFIG, config1);
+	}
+
 	return 0;
 }
 
@@ -1975,7 +2050,25 @@ void gpmc_read_settings_dt(struct device_node *np, struct gpmc_settings *p)
 				__func__);
 	}
 
+	p->wait_pin = GPMC_WAITPIN_INVALID;
+	p->wait_pin_polarity = GPMC_WAITPINPOLARITY_INVALID;
+
 	if (!of_property_read_u32(np, "gpmc,wait-pin", &p->wait_pin)) {
+		if (!gpmc_is_valid_waitpin(p->wait_pin)) {
+			pr_err("%s: Invalid wait-pin (%d)\n", __func__, p->wait_pin);
+			p->wait_pin = GPMC_WAITPIN_INVALID;
+		}
+
+		if (!of_property_read_u32(np, "ti,wait-pin-polarity",
+					  &p->wait_pin_polarity)) {
+			if (p->wait_pin_polarity != GPMC_WAITPINPOLARITY_ACTIVE_HIGH &&
+			    p->wait_pin_polarity != GPMC_WAITPINPOLARITY_ACTIVE_LOW) {
+				pr_err("%s: Invalid wait-pin-polarity (%d)\n",
+				       __func__, p->wait_pin_polarity);
+				p->wait_pin_polarity = GPMC_WAITPINPOLARITY_INVALID;
+				}
+		}
+
 		p->wait_on_read = of_property_read_bool(np,
 							"gpmc,wait-on-read");
 		p->wait_on_write = of_property_read_bool(np,
@@ -2080,7 +2173,6 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
 	const char *name;
 	int ret, cs;
 	u32 val;
-	struct gpio_desc *waitpin_desc = NULL;
 	struct gpmc_device *gpmc = platform_get_drvdata(pdev);
 
 	if (of_property_read_u32(child, "reg", &cs) < 0) {
@@ -2208,17 +2300,9 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
 
 	/* Reserve wait pin if it is required and valid */
 	if (gpmc_s.wait_on_read || gpmc_s.wait_on_write) {
-		unsigned int wait_pin = gpmc_s.wait_pin;
-
-		waitpin_desc = gpiochip_request_own_desc(&gpmc->gpio_chip,
-							 wait_pin, "WAITPIN",
-							 GPIO_ACTIVE_HIGH,
-							 GPIOD_IN);
-		if (IS_ERR(waitpin_desc)) {
-			dev_err(&pdev->dev, "invalid wait-pin: %d\n", wait_pin);
-			ret = PTR_ERR(waitpin_desc);
+		ret = gpmc_alloc_waitpin(gpmc, &gpmc_s);
+		if (ret < 0)
 			goto err;
-		}
 	}
 
 	gpmc_cs_show_timings(cs, "before gpmc_cs_program_settings");
@@ -2260,7 +2344,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
 	ret = -ENODEV;
 
 err_cs:
-	gpiochip_free_own_desc(waitpin_desc);
+	gpmc_free_waitpin(gpmc, gpmc_s.wait_pin);
 err:
 	gpmc_cs_free(cs);
 
@@ -2489,7 +2573,7 @@ static int omap_gpmc_context_notifier(struct notifier_block *nb,
 
 static int gpmc_probe(struct platform_device *pdev)
 {
-	int rc;
+	int rc, i;
 	u32 l;
 	struct resource *res;
 	struct gpmc_device *gpmc;
@@ -2545,6 +2629,15 @@ static int gpmc_probe(struct platform_device *pdev)
 		gpmc_nr_waitpins = GPMC_NR_WAITPINS;
 	}
 
+	gpmc->waitpins = devm_kzalloc(&pdev->dev,
+				      gpmc_nr_waitpins * sizeof(struct gpmc_waitpin),
+				      GFP_KERNEL);
+	if (!gpmc->waitpins)
+		return -ENOMEM;
+
+	for (i = 0; i < gpmc_nr_waitpins; i++)
+		gpmc->waitpins[i].pin = GPMC_WAITPIN_INVALID;
+
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
 
@@ -2598,9 +2691,12 @@ static int gpmc_probe(struct platform_device *pdev)
 
 static int gpmc_remove(struct platform_device *pdev)
 {
+	int i;
 	struct gpmc_device *gpmc = platform_get_drvdata(pdev);
 
 	cpu_pm_unregister_notifier(&gpmc->nb);
+	for (i = 0; i < gpmc_nr_waitpins; i++)
+		gpmc_free_waitpin(gpmc, i);
 	gpmc_free_irq(gpmc);
 	gpmc_mem_exit();
 	pm_runtime_put_sync(&pdev->dev);
diff --git a/include/linux/platform_data/gpmc-omap.h b/include/linux/platform_data/gpmc-omap.h
index c9cc4e32435d..296b080c5c67 100644
--- a/include/linux/platform_data/gpmc-omap.h
+++ b/include/linux/platform_data/gpmc-omap.h
@@ -136,6 +136,13 @@ struct gpmc_device_timings {
 #define GPMC_MUX_AAD			1	/* Addr-Addr-Data multiplex */
 #define GPMC_MUX_AD			2	/* Addr-Data multiplex */
 
+/* Wait pin polarity values */
+#define GPMC_WAITPINPOLARITY_INVALID -1
+#define GPMC_WAITPINPOLARITY_ACTIVE_LOW 0
+#define GPMC_WAITPINPOLARITY_ACTIVE_HIGH 1
+
+#define GPMC_WAITPIN_INVALID -1
+
 struct gpmc_settings {
 	bool burst_wrap;	/* enables wrap bursting */
 	bool burst_read;	/* enables read page/burst mode */
@@ -149,6 +156,7 @@ struct gpmc_settings {
 	u32 device_width;	/* device bus width (8 or 16 bit) */
 	u32 mux_add_data;	/* multiplex address & data */
 	u32 wait_pin;		/* wait-pin to be used */
+	u32 wait_pin_polarity;
 };
 
 /* Data for each chip select */
-- 
2.25.1

[PATCH v9 2/2] dt-bindings: memory-controllers: gpmc-child: add wait-pin polarity

[B. Niedermayr]

From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>

The GPMC controller has the ability to configure the polarity for the
wait pin. The current properties do not allow this configuration.
This binding directly configures the WAITPIN<X>POLARITY bit
in the GPMC_CONFIG register by setting the "ti,wait-pin-polarity"
dt-property.

Signed-off-by: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/memory-controllers/ti,gpmc-child.yaml         | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc-child.yaml b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc-child.yaml
index 6e3995bb1630..4a257fac577e 100644
--- a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc-child.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc-child.yaml
@@ -230,6 +230,13 @@ properties:
       Wait-pin used by client. Must be less than "gpmc,num-waitpins".
     $ref: /schemas/types.yaml#/definitions/uint32
 
+  ti,wait-pin-polarity:
+    description: |
+      Set the desired polarity for the selected wait pin.
+      0 for active low, 1 for active high.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1]
+
   gpmc,wait-on-read:
     description: Enables wait monitoring on reads.
     type: boolean
-- 
2.25.1

Re: [PATCH v9 0/2] gpmc wait pin additions

[Krzysztof Kozlowski]

On 02/11/2022 09:30, B. Niedermayr wrote:
> From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
> 
> Currently it is not possible to configure the WAIT0PINPOLARITY and
> WAIT1PINPOLARITY bits of the GPMC_CONFIG register directly via
> device tree properties.
> 
> It is also not possible to use the same wait-pin for different
> cs-regions.
> 
> While the current implementation may fullfill most usecases, it may not
> be sufficient for more complex setups (e.g. FPGA/ASIC interfaces), where
> more complex interfacing options where possible.
> 
> For example interfacing an ASIC which offers multiple cs-regions but
> only one waitpin the current driver and dt-bindings are not sufficient.
> 
> While using the same waitpin for different cs-regions worked for older
> kernels (4.14) the omap-gpmc.c driver refused to probe (-EBUSY) with
> newer kernels (>5.10).

This is a friendly reminder during the review process.

It looks like you received a tag and forgot to add it.

If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions. However, there's no need to repost patches *only* to add the
tags. The upstream maintainer will do that for acks received on the
version they apply.

https://elixir.bootlin.com/linux/v5.17/source/Documentation/process/submitting-patches.rst#L540

If a tag was not added on purpose, please state why and what changed.

Best regards,
Krzysztof

Re: [PATCH v9 0/2] gpmc wait pin additions

[Krzysztof Kozlowski]

On Wed, 2 Nov 2022 14:30:45 +0100, B. Niedermayr wrote:
> From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
> 
> Currently it is not possible to configure the WAIT0PINPOLARITY and
> WAIT1PINPOLARITY bits of the GPMC_CONFIG register directly via
> device tree properties.
> 
> It is also not possible to use the same wait-pin for different
> cs-regions.
> 
> [...]

Applied, thanks!

[1/2] memory: omap-gpmc: wait pin additions
      https://git.kernel.org/krzk/linux-mem-ctrl/c/89aed3cd5cb951113b766cddd9c2df43cfbdafd5
[2/2] dt-bindings: memory-controllers: gpmc-child: add wait-pin polarity
      https://git.kernel.org/krzk/linux-mem-ctrl/c/1f1e46b83b7db08c8db31816c857e27da84d4ca3

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Re: [PATCH v9 0/2] gpmc wait pin additions

[Niedermayr, BENEDIKT]

On Wed, 2022-11-02 at 10:03 -0400, Krzysztof Kozlowski wrote:
> On Wed, 2 Nov 2022 14:30:45 +0100, B. Niedermayr wrote:
> > From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
> > 
> > Currently it is not possible to configure the WAIT0PINPOLARITY and
> > WAIT1PINPOLARITY bits of the GPMC_CONFIG register directly via
> > device tree properties.
> > 
> > It is also not possible to use the same wait-pin for different
> > cs-regions.
> > 
> > [...]
> 
> Applied, thanks!
> 
> [1/2] memory: omap-gpmc: wait pin additions
>       
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fkrzk%2Flinux-mem-ctrl%2Fc%2F89aed3cd5cb951113b766cddd9c2df43cfbdafd5&amp;data=05%7C01%7Cbenedikt.niedermayr%40siemens.com%7Ca77b0a7ae09b48a3d87c08dabcdb1434%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C638029946370825368%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=cfNaOFmcLbfhLdDQY4OVe6onsRGA13B%2BqUs0uLylr%2BI%3D&amp;reserved=0
> [2/2] dt-bindings: memory-controllers: gpmc-child: add wait-pin polarity
>       
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fkrzk%2Flinux-mem-ctrl%2Fc%2F1f1e46b83b7db08c8db31816c857e27da84d4ca3&amp;data=05%7C01%7Cbenedikt.niedermayr%40siemens.com%7Ca77b0a7ae09b48a3d87c08dabcdb1434%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C638029946370825368%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=PaOpjoM6aVT4rpxYZCvwIM8c%2Bzsfyl8dTZ9sxGdtGeY%3D&amp;reserved=0
> 
> Best regards,
Thank you!

Cheers, 
Benedikt

Re: [PATCH v9 0/2] gpmc wait pin additions

[Niedermayr, BENEDIKT]

On Wed, 2022-11-02 at 10:02 -0400, Krzysztof Kozlowski wrote:
> On 02/11/2022 09:30, B. Niedermayr wrote:
> > From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
> > 
> > Currently it is not possible to configure the WAIT0PINPOLARITY and
> > WAIT1PINPOLARITY bits of the GPMC_CONFIG register directly via
> > device tree properties.
> > 
> > It is also not possible to use the same wait-pin for different
> > cs-regions.
> > 
> > While the current implementation may fullfill most usecases, it may not
> > be sufficient for more complex setups (e.g. FPGA/ASIC interfaces), where
> > more complex interfacing options where possible.
> > 
> > For example interfacing an ASIC which offers multiple cs-regions but
> > only one waitpin the current driver and dt-bindings are not sufficient.
> > 
> > While using the same waitpin for different cs-regions worked for older
> > kernels (4.14) the omap-gpmc.c driver refused to probe (-EBUSY) with
> > newer kernels (>5.10).
> 
> This is a friendly reminder during the review process.
> 
> It looks like you received a tag and forgot to add it.

Thanks for the hint.

Was everything OK with v9 (except that I resendet the tagged patch)? Until v8 I wasn't aware of that. I thought I added the tag for v9. 

Cheers,
Benedikt

Re: [PATCH v9 0/2] gpmc wait pin additions

[Krzysztof Kozlowski]

On 03/11/2022 04:13, Niedermayr, BENEDIKT wrote:
> On Wed, 2022-11-02 at 10:02 -0400, Krzysztof Kozlowski wrote:
>> On 02/11/2022 09:30, B. Niedermayr wrote:
>>> From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
>>>
>>> Currently it is not possible to configure the WAIT0PINPOLARITY and
>>> WAIT1PINPOLARITY bits of the GPMC_CONFIG register directly via
>>> device tree properties.
>>>
>>> It is also not possible to use the same wait-pin for different
>>> cs-regions.
>>>
>>> While the current implementation may fullfill most usecases, it may not
>>> be sufficient for more complex setups (e.g. FPGA/ASIC interfaces), where
>>> more complex interfacing options where possible.
>>>
>>> For example interfacing an ASIC which offers multiple cs-regions but
>>> only one waitpin the current driver and dt-bindings are not sufficient.
>>>
>>> While using the same waitpin for different cs-regions worked for older
>>> kernels (4.14) the omap-gpmc.c driver refused to probe (-EBUSY) with
>>> newer kernels (>5.10).
>>
>> This is a friendly reminder during the review process.
>>
>> It looks like you received a tag and forgot to add it.
> 
> Thanks for the hint.
> 
> Was everything OK with v9 (except that I resendet the tagged patch)? Until v8 I wasn't aware of that. I thought I added the tag for v9. 

You did not add the tag you received.

Best regards,
Krzysztof

Re: [PATCH v9 1/2] memory: omap-gpmc: wait pin additions

[Tony Lindgren]

Hi,

* B. Niedermayr <benedikt.niedermayr@siemens.com> [221102 13:21]:
> From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
> 
> This patch introduces support for setting the wait-pin polarity as well
> as using the same wait-pin for different CS regions.

Looks like Linux next commit 89aed3cd5cb9 ("memory: omap-gpmc: wait pin
additions") breaks the old smsc911x using devices somehow for nfsroot.

Reverting this commit makes things work again. Any ideas?

Regards,

Tony

Re: [PATCH v9 1/2] memory: omap-gpmc: wait pin additions

[Niedermayr, BENEDIKT]

Hi Tony,

On Wed, 2022-12-07 at 15:51 +0200, Tony Lindgren wrote:
> Hi,
> 
> * B. Niedermayr <benedikt.niedermayr@siemens.com> [221102 13:21]:
> > From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
> > 
> > This patch introduces support for setting the wait-pin polarity as well
> > as using the same wait-pin for different CS regions.
> 
> Looks like Linux next commit 89aed3cd5cb9 ("memory: omap-gpmc: wait pin
> additions") breaks the old smsc911x using devices somehow for nfsroot.
> 
Can you explain how this breaking change looks like, in bit more detail?
I'm a bit confused since the changes on omap-gpmc have nothing in common with
smsc911x. 
 
> Reverting this commit makes things work again. Any ideas?
> 
> Regards,
> 
> Tony

cheers,
Benedikt

Re: [PATCH v9 1/2] memory: omap-gpmc: wait pin additions

[Tony Lindgren]

* Niedermayr, BENEDIKT <benedikt.niedermayr@siemens.com> [221207 14:52]:
> Hi Tony,
> 
> On Wed, 2022-12-07 at 15:51 +0200, Tony Lindgren wrote:
> > Hi,
> > 
> > * B. Niedermayr <benedikt.niedermayr@siemens.com> [221102 13:21]:
> > > From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
> > > 
> > > This patch introduces support for setting the wait-pin polarity as well
> > > as using the same wait-pin for different CS regions.
> > 
> > Looks like Linux next commit 89aed3cd5cb9 ("memory: omap-gpmc: wait pin
> > additions") breaks the old smsc911x using devices somehow for nfsroot.
> > 
> Can you explain how this breaking change looks like, in bit more detail?
> I'm a bit confused since the changes on omap-gpmc have nothing in common with
> smsc911x. 

The smsc911x device is on gpmc. It's not found with this change.
See arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi for the configuration.

Regards,

Tony

Re: [PATCH v9 1/2] memory: omap-gpmc: wait pin additions

[Niedermayr, BENEDIKT]

Hi Tony,
On Wed, 2022-12-07 at 17:06 +0200, Tony Lindgren wrote:
> * Niedermayr, BENEDIKT <benedikt.niedermayr@siemens.com> [221207 14:52]:
> > Hi Tony,
> > 
> > On Wed, 2022-12-07 at 15:51 +0200, Tony Lindgren wrote:
> > > Hi,
> > > 
> > > * B. Niedermayr <benedikt.niedermayr@siemens.com> [221102 13:21]:
> > > > From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
> > > > 
> > > > This patch introduces support for setting the wait-pin polarity as well
> > > > as using the same wait-pin for different CS regions.
> > > 
> > > Looks like Linux next commit 89aed3cd5cb9 ("memory: omap-gpmc: wait pin
> > > additions") breaks the old smsc911x using devices somehow for nfsroot.
> > > 
> > Can you explain how this breaking change looks like, in bit more detail?
> > I'm a bit confused since the changes on omap-gpmc have nothing in common with
> > smsc911x. 
> 
> The smsc911x device is on gpmc. It's not found with this change.
> See arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi for the configuration.
Thanks for the configuration example. 

I found the cause of this bug.
At least when "gpmc_cs_program_settings: invalid wait-pin (-1)" is printed in the kernel log.

Now I'm not sure where to send the bugfix patch (linux-next, linux-omap, both?). 

> 
> Regards,
> 
> Tony

cheers,
benedikt

Re: [PATCH v9 1/2] memory: omap-gpmc: wait pin additions

[Tony Lindgren]

* Niedermayr, BENEDIKT <benedikt.niedermayr@siemens.com> [221207 17:29]:
> I found the cause of this bug.
> At least when "gpmc_cs_program_settings: invalid wait-pin (-1)" is printed in the kernel log.

OK

> Now I'm not sure where to send the bugfix patch (linux-next, linux-omap, both?). 

Please send a fix as against current Linux next as a separate patch as
your earlier patches have been already applied for the merge window.

If dts changes are also needed, let's first try to fix the driver to handle
the invalid wait-pin case. Then we can patch the dts files separately as
needed.

Regards,

Tony

Re: [PATCH v9 1/2] memory: omap-gpmc: wait pin additions

[Niedermayr, BENEDIKT]

Hi Tony,

On Thu, 2022-12-08 at 07:49 +0200, Tony Lindgren wrote:
> * Niedermayr, BENEDIKT <benedikt.niedermayr@siemens.com> [221207 17:29]:
> > I found the cause of this bug.
> > At least when "gpmc_cs_program_settings: invalid wait-pin (-1)" is
> > printed in the kernel log.
> 
> OK
> 
> > Now I'm not sure where to send the bugfix patch (linux-next, linux-omap, 
> > both?). 
> 
> Please send a fix as against current Linux next as a separate patch as
> your earlier patches have been already applied for the merge window.

OK. Thanks!

> If dts changes are also needed, let's first try to fix the driver to
> handle
> the invalid wait-pin case. Then we can patch the dts files separately as
> needed.
No need for dts changes. One concern for the wait-pin implementation was to
not break existing dts's where the wait-pin is not used.   

> 
> Regards,
> 
> Tony

cheers,
Benedikt

Re: [PATCH v9 1/2] memory: omap-gpmc: wait pin additions

[Tony Lindgren]

* Niedermayr, BENEDIKT <benedikt.niedermayr@siemens.com> [221208 15:55]:
> No need for dts changes. One concern for the wait-pin implementation was to
> not break existing dts's where the wait-pin is not used.   

OK thanks.

Tony