All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/3] omap-gpmc wait pin additions
@ 2022-09-15  9:13 B. Niedermayr
  2022-09-15  9:13 ` [PATCH v4 1/3] memory: omap-gpmc: allow shared wait pins B. Niedermayr
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: B. Niedermayr @ 2022-09-15  9:13 UTC (permalink / raw)
  To: linux-omap, devicetree; +Cc: rogerq, tony, krzysztof.kozlowski, robh+dt

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, but the maintainers list
    stays the same!
  * 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


Benedikt Niedermayr (3):
  memory: omap-gpmc: allow shared wait pins
  memory: omap-gpmc: add support for wait pin polarity
  dt-bindings: memory-controllers: gpmc-child: add wait-pin polarity

 .../memory-controllers/ti,gpmc-child.yaml     |  7 +++++
 drivers/memory/omap-gpmc.c                    | 30 +++++++++++++++++--
 include/linux/platform_data/gpmc-omap.h       |  6 ++++
 3 files changed, 41 insertions(+), 2 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 1/3] memory: omap-gpmc: allow shared wait pins
  2022-09-15  9:13 [PATCH v4 0/3] omap-gpmc wait pin additions B. Niedermayr
@ 2022-09-15  9:13 ` B. Niedermayr
  2022-09-16  8:50   ` Roger Quadros
  2022-09-15  9:13 ` [PATCH v4 2/3] memory: omap-gpmc: add support for wait pin polarity B. Niedermayr
  2022-09-15  9:13 ` [PATCH v4 3/3] dt-bindings: memory-controllers: gpmc-child: add wait-pin polarity B. Niedermayr
  2 siblings, 1 reply; 8+ messages in thread
From: B. Niedermayr @ 2022-09-15  9:13 UTC (permalink / raw)
  To: linux-omap, devicetree; +Cc: rogerq, tony, krzysztof.kozlowski, robh+dt

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

Newer kernels refuse to probe when using the same wait pin for
different chipselect regions.

But this may be a usecase when connecting for example FPGA or ASIC
modules to the gpmc, which only got one wait pin installed.

Signed-off-by: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
---
 drivers/memory/omap-gpmc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index d9bf1c2ac319..e3674a15b934 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -2221,9 +2221,13 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
 							 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);
-			goto err;
+			if (ret == -EBUSY) {
+				dev_info(&pdev->dev, "shared wait-pin: %d\n", wait_pin);
+			} else {
+				dev_err(&pdev->dev, "invalid wait-pin: %d\n", wait_pin);
+				goto err;
+			}
 		}
 	}
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 2/3] memory: omap-gpmc: add support for wait pin polarity
  2022-09-15  9:13 [PATCH v4 0/3] omap-gpmc wait pin additions B. Niedermayr
  2022-09-15  9:13 ` [PATCH v4 1/3] memory: omap-gpmc: allow shared wait pins B. Niedermayr
@ 2022-09-15  9:13 ` B. Niedermayr
  2022-09-16  8:46   ` Roger Quadros
  2022-09-16  8:55   ` Roger Quadros
  2022-09-15  9:13 ` [PATCH v4 3/3] dt-bindings: memory-controllers: gpmc-child: add wait-pin polarity B. Niedermayr
  2 siblings, 2 replies; 8+ messages in thread
From: B. Niedermayr @ 2022-09-15  9:13 UTC (permalink / raw)
  To: linux-omap, devicetree; +Cc: rogerq, tony, krzysztof.kozlowski, robh+dt

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

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 "gpmc,wait-pin-polarity" dt-property.

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

diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index e3674a15b934..66dd7dd80653 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -132,6 +132,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)
@@ -1881,6 +1882,21 @@ int gpmc_cs_program_settings(int cs, struct gpmc_settings *p)
 
 	gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, config1);
 
+	if (p->wait_on_read || p->wait_on_write) {
+		config1 = gpmc_read_reg(GPMC_CONFIG);
+
+		if (p->wait_pin_polarity == WAITPINPOLARITY_ACTIVE_LOW)
+			config1 &= ~GPMC_CONFIG_WAITPINPOLARITY(p->wait_pin);
+		else if (p->wait_pin_polarity == WAITPINPOLARITY_ACTIVE_HIGH)
+			config1 |= GPMC_CONFIG_WAITPINPOLARITY(p->wait_pin);
+		else if (p->wait_pin_polarity != WAITPINPOLARITY_DEFAULT)
+			pr_err("%s: invalid wait-pin-polarity (pin: %d, polarity: %d)\n",
+				__func__, p->wait_pin, p->wait_pin_polarity);
+
+		gpmc_write_reg(GPMC_CONFIG, config1);
+	}
+
+
 	return 0;
 }
 
@@ -1981,6 +1997,12 @@ void gpmc_read_settings_dt(struct device_node *np, struct gpmc_settings *p)
 	}
 
 	if (!of_property_read_u32(np, "gpmc,wait-pin", &p->wait_pin)) {
+
+		p->wait_pin_polarity = WAITPINPOLARITY_DEFAULT;
+		of_property_read_u32(np,
+			"gpmc,wait-pin-polarity",
+			&p->wait_pin_polarity);
+
 		p->wait_on_read = of_property_read_bool(np,
 							"gpmc,wait-on-read");
 		p->wait_on_write = of_property_read_bool(np,
diff --git a/include/linux/platform_data/gpmc-omap.h b/include/linux/platform_data/gpmc-omap.h
index c9cc4e32435d..c46c28069c31 100644
--- a/include/linux/platform_data/gpmc-omap.h
+++ b/include/linux/platform_data/gpmc-omap.h
@@ -136,6 +136,11 @@ 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 WAITPINPOLARITY_DEFAULT -1
+#define WAITPINPOLARITY_ACTIVE_LOW 0
+#define WAITPINPOLARITY_ACTIVE_HIGH 1
+
 struct gpmc_settings {
 	bool burst_wrap;	/* enables wrap bursting */
 	bool burst_read;	/* enables read page/burst mode */
@@ -149,6 +154,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;	/* wait-pin polarity */
 };
 
 /* Data for each chip select */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 3/3] dt-bindings: memory-controllers: gpmc-child: add wait-pin polarity
  2022-09-15  9:13 [PATCH v4 0/3] omap-gpmc wait pin additions B. Niedermayr
  2022-09-15  9:13 ` [PATCH v4 1/3] memory: omap-gpmc: allow shared wait pins B. Niedermayr
  2022-09-15  9:13 ` [PATCH v4 2/3] memory: omap-gpmc: add support for wait pin polarity B. Niedermayr
@ 2022-09-15  9:13 ` B. Niedermayr
  2 siblings, 0 replies; 8+ messages in thread
From: B. Niedermayr @ 2022-09-15  9:13 UTC (permalink / raw)
  To: linux-omap, devicetree; +Cc: rogerq, tony, krzysztof.kozlowski, robh+dt

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 gpmc,wait-pin-polarity
dt-property.

Signed-off-by: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
---
 .../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..8e541acdb1ff 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
 
+  gpmc,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.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/3] memory: omap-gpmc: add support for wait pin polarity
  2022-09-15  9:13 ` [PATCH v4 2/3] memory: omap-gpmc: add support for wait pin polarity B. Niedermayr
@ 2022-09-16  8:46   ` Roger Quadros
  2022-09-16  8:55   ` Roger Quadros
  1 sibling, 0 replies; 8+ messages in thread
From: Roger Quadros @ 2022-09-16  8:46 UTC (permalink / raw)
  To: B. Niedermayr, linux-omap, devicetree; +Cc: tony, krzysztof.kozlowski, robh+dt

Hi,

On 15/09/2022 12:13, B. Niedermayr wrote:
> From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
> 
> 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 "gpmc,wait-pin-polarity" dt-property.
> 
> Signed-off-by: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
> ---
>  drivers/memory/omap-gpmc.c              | 22 ++++++++++++++++++++++
>  include/linux/platform_data/gpmc-omap.h |  6 ++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
> index e3674a15b934..66dd7dd80653 100644
> --- a/drivers/memory/omap-gpmc.c
> +++ b/drivers/memory/omap-gpmc.c
> @@ -132,6 +132,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)
> @@ -1881,6 +1882,21 @@ int gpmc_cs_program_settings(int cs, struct gpmc_settings *p)
>  
>  	gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, config1);
>  
> +	if (p->wait_on_read || p->wait_on_write) {
> +		config1 = gpmc_read_reg(GPMC_CONFIG);
> +
> +		if (p->wait_pin_polarity == WAITPINPOLARITY_ACTIVE_LOW)
> +			config1 &= ~GPMC_CONFIG_WAITPINPOLARITY(p->wait_pin);
> +		else if (p->wait_pin_polarity == WAITPINPOLARITY_ACTIVE_HIGH)
> +			config1 |= GPMC_CONFIG_WAITPINPOLARITY(p->wait_pin);
> +		else if (p->wait_pin_polarity != WAITPINPOLARITY_DEFAULT)
> +			pr_err("%s: invalid wait-pin-polarity (pin: %d, polarity: %d)\n",
> +				__func__, p->wait_pin, p->wait_pin_polarity);

We could avoid the print here. Instead the check for correct polarity
value and printing an error message on invalid values needs to be done
when we read the DT property in gpmc_read_settings_dt()

> +
> +		gpmc_write_reg(GPMC_CONFIG, config1);

How about avoiding the read/write altogether if
p->wait_pin_polarity == WAITPINPOLARITY_DEFAULT?

> +	}
> +
> +
>  	return 0;
>  }
>  
> @@ -1981,6 +1997,12 @@ void gpmc_read_settings_dt(struct device_node *np, struct gpmc_settings *p)
>  	}
>  
>  	if (!of_property_read_u32(np, "gpmc,wait-pin", &p->wait_pin)) {
> +
New line not required.

> +		p->wait_pin_polarity = WAITPINPOLARITY_DEFAULT;

This should be moved before this 'if' block. Else we will have cases
where WAITPINPOLARITY_DEFAULT is not set when there was no "gpmc,wait-pin"
property in the DT node.

> +		of_property_read_u32(np,
> +			"gpmc,wait-pin-polarity",
> +			&p->wait_pin_polarity);
> +

Please sanity check p->wait_pin_polarity here.
If invalid value, print error and set to WAITPINPOLARITY_DEFAULT.

>  		p->wait_on_read = of_property_read_bool(np,
>  							"gpmc,wait-on-read");
>  		p->wait_on_write = of_property_read_bool(np,
> diff --git a/include/linux/platform_data/gpmc-omap.h b/include/linux/platform_data/gpmc-omap.h
> index c9cc4e32435d..c46c28069c31 100644
> --- a/include/linux/platform_data/gpmc-omap.h
> +++ b/include/linux/platform_data/gpmc-omap.h
> @@ -136,6 +136,11 @@ 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 WAITPINPOLARITY_DEFAULT -1
> +#define WAITPINPOLARITY_ACTIVE_LOW 0
> +#define WAITPINPOLARITY_ACTIVE_HIGH 1
> +
>  struct gpmc_settings {
>  	bool burst_wrap;	/* enables wrap bursting */
>  	bool burst_read;	/* enables read page/burst mode */
> @@ -149,6 +154,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;	/* wait-pin polarity */
>  };
>  
>  /* Data for each chip select */

cheers,
-roger

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 1/3] memory: omap-gpmc: allow shared wait pins
  2022-09-15  9:13 ` [PATCH v4 1/3] memory: omap-gpmc: allow shared wait pins B. Niedermayr
@ 2022-09-16  8:50   ` Roger Quadros
  0 siblings, 0 replies; 8+ messages in thread
From: Roger Quadros @ 2022-09-16  8:50 UTC (permalink / raw)
  To: B. Niedermayr, linux-omap, devicetree; +Cc: tony, krzysztof.kozlowski, robh+dt

Hi,

On 15/09/2022 12:13, B. Niedermayr wrote:
> From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
> 
> Newer kernels refuse to probe when using the same wait pin for
> different chipselect regions.
> 
> But this may be a usecase when connecting for example FPGA or ASIC
> modules to the gpmc, which only got one wait pin installed.
> 
> Signed-off-by: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
> ---
>  drivers/memory/omap-gpmc.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
> index d9bf1c2ac319..e3674a15b934 100644
> --- a/drivers/memory/omap-gpmc.c
> +++ b/drivers/memory/omap-gpmc.c
> @@ -2221,9 +2221,13 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>  							 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);
> -			goto err;
> +			if (ret == -EBUSY) {
> +				dev_info(&pdev->dev, "shared wait-pin: %d\n", wait_pin);

What if this GPI was taken by someone other than us for wait-pin usage?
e.g. some other device driver for GPI usage?

In that case that GPIO functionality will break if we now start using it for wait-pin usage. Right?

One solution could be keep track if we reserved that pin for wait-pin usage
and if so sharing the wait-pin for wait-pin usage is fine.

> +			} else {
> +				dev_err(&pdev->dev, "invalid wait-pin: %d\n", wait_pin);
> +				goto err;
> +			}
>  		}
>  	}
>  

cheers,
-roger

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/3] memory: omap-gpmc: add support for wait pin polarity
  2022-09-15  9:13 ` [PATCH v4 2/3] memory: omap-gpmc: add support for wait pin polarity B. Niedermayr
  2022-09-16  8:46   ` Roger Quadros
@ 2022-09-16  8:55   ` Roger Quadros
  2022-09-16  9:17     ` Niedermayr, BENEDIKT
  1 sibling, 1 reply; 8+ messages in thread
From: Roger Quadros @ 2022-09-16  8:55 UTC (permalink / raw)
  To: B. Niedermayr, linux-omap, devicetree; +Cc: tony, krzysztof.kozlowski, robh+dt



On 15/09/2022 12:13, B. Niedermayr wrote:
> From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
> 
> 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 "gpmc,wait-pin-polarity" dt-property.
> 
> Signed-off-by: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
> ---
>  drivers/memory/omap-gpmc.c              | 22 ++++++++++++++++++++++
>  include/linux/platform_data/gpmc-omap.h |  6 ++++++
>  2 files changed, 28 insertions(+)


./scripts/checkpatch.pl --strict

-------------------------------------------------------------
0002-memory-omap-gpmc-add-support-for-wait-pin-polarity.patch
-------------------------------------------------------------
CHECK: Alignment should match open parenthesis
#42: FILE: drivers/memory/omap-gpmc.c:1899:
+			pr_err("%s: invalid wait-pin-polarity (pin: %d, polarity: %d)\n",
+				__func__, p->wait_pin, p->wait_pin_polarity);

CHECK: Please don't use multiple blank lines
#47: FILE: drivers/memory/omap-gpmc.c:1904:
+
+

CHECK: Blank lines aren't necessary after an open brace '{'
#55: FILE: drivers/memory/omap-gpmc.c:1995:
 	if (!of_property_read_u32(np, "gpmc,wait-pin", &p->wait_pin)) {
+

CHECK: Alignment should match open parenthesis
#58: FILE: drivers/memory/omap-gpmc.c:1998:
+		of_property_read_u32(np,
+			"gpmc,wait-pin-polarity",

total: 0 errors, 0 warnings, 4 checks, 58 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

> 
> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
> index e3674a15b934..66dd7dd80653 100644
> --- a/drivers/memory/omap-gpmc.c
> +++ b/drivers/memory/omap-gpmc.c
> @@ -132,6 +132,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)
> @@ -1881,6 +1882,21 @@ int gpmc_cs_program_settings(int cs, struct gpmc_settings *p)
>  
>  	gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, config1);
>  
> +	if (p->wait_on_read || p->wait_on_write) {
> +		config1 = gpmc_read_reg(GPMC_CONFIG);
> +
> +		if (p->wait_pin_polarity == WAITPINPOLARITY_ACTIVE_LOW)
> +			config1 &= ~GPMC_CONFIG_WAITPINPOLARITY(p->wait_pin);
> +		else if (p->wait_pin_polarity == WAITPINPOLARITY_ACTIVE_HIGH)
> +			config1 |= GPMC_CONFIG_WAITPINPOLARITY(p->wait_pin);
> +		else if (p->wait_pin_polarity != WAITPINPOLARITY_DEFAULT)
> +			pr_err("%s: invalid wait-pin-polarity (pin: %d, polarity: %d)\n",
> +				__func__, p->wait_pin, p->wait_pin_polarity);
> +
> +		gpmc_write_reg(GPMC_CONFIG, config1);
> +	}
> +
> +
>  	return 0;
>  }
>  
> @@ -1981,6 +1997,12 @@ void gpmc_read_settings_dt(struct device_node *np, struct gpmc_settings *p)
>  	}
>  
>  	if (!of_property_read_u32(np, "gpmc,wait-pin", &p->wait_pin)) {
> +
> +		p->wait_pin_polarity = WAITPINPOLARITY_DEFAULT;
> +		of_property_read_u32(np,
> +			"gpmc,wait-pin-polarity",
> +			&p->wait_pin_polarity);
> +
>  		p->wait_on_read = of_property_read_bool(np,
>  							"gpmc,wait-on-read");
>  		p->wait_on_write = of_property_read_bool(np,
> diff --git a/include/linux/platform_data/gpmc-omap.h b/include/linux/platform_data/gpmc-omap.h
> index c9cc4e32435d..c46c28069c31 100644
> --- a/include/linux/platform_data/gpmc-omap.h
> +++ b/include/linux/platform_data/gpmc-omap.h
> @@ -136,6 +136,11 @@ 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 WAITPINPOLARITY_DEFAULT -1
> +#define WAITPINPOLARITY_ACTIVE_LOW 0
> +#define WAITPINPOLARITY_ACTIVE_HIGH 1
> +
>  struct gpmc_settings {
>  	bool burst_wrap;	/* enables wrap bursting */
>  	bool burst_read;	/* enables read page/burst mode */
> @@ -149,6 +154,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;	/* wait-pin polarity */
>  };
>  
>  /* Data for each chip select */

cheers,
-roger

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/3] memory: omap-gpmc: add support for wait pin polarity
  2022-09-16  8:55   ` Roger Quadros
@ 2022-09-16  9:17     ` Niedermayr, BENEDIKT
  0 siblings, 0 replies; 8+ messages in thread
From: Niedermayr, BENEDIKT @ 2022-09-16  9:17 UTC (permalink / raw)
  To: rogerq, devicetree, linux-omap; +Cc: tony, krzysztof.kozlowski, robh+dt

On Fri, 2022-09-16 at 11:55 +0300, Roger Quadros wrote:
> 
> On 15/09/2022 12:13, B. Niedermayr wrote:
> > From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
> > 
> > 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 "gpmc,wait-pin-polarity" dt-property.
> > 
> > Signed-off-by: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
> > ---
> >  drivers/memory/omap-gpmc.c              | 22 ++++++++++++++++++++++
> >  include/linux/platform_data/gpmc-omap.h |  6 ++++++
> >  2 files changed, 28 insertions(+)
> 
> ./scripts/checkpatch.pl --strict
Sorry, I wasn't aware of that option.
It's my first patchseries I try to get upstream.

But thanks for the review and feedback!

> 
> -------------------------------------------------------------
> 0002-memory-omap-gpmc-add-support-for-wait-pin-polarity.patch
> -------------------------------------------------------------
> CHECK: Alignment should match open parenthesis
> #42: FILE: drivers/memory/omap-gpmc.c:1899:
> +			pr_err("%s: invalid wait-pin-polarity (pin: %d, polarity: %d)\n",
> +				__func__, p->wait_pin, p->wait_pin_polarity);
> 
> CHECK: Please don't use multiple blank lines
> #47: FILE: drivers/memory/omap-gpmc.c:1904:
> +
> +
> 
> CHECK: Blank lines aren't necessary after an open brace '{'
> #55: FILE: drivers/memory/omap-gpmc.c:1995:
>  	if (!of_property_read_u32(np, "gpmc,wait-pin", &p->wait_pin)) {
> +
> 
> CHECK: Alignment should match open parenthesis
> #58: FILE: drivers/memory/omap-gpmc.c:1998:
> +		of_property_read_u32(np,
> +			"gpmc,wait-pin-polarity",
> 
> total: 0 errors, 0 warnings, 4 checks, 58 lines checked
> 
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or --fix-inplace.
> 
> > diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
> > index e3674a15b934..66dd7dd80653 100644
> > --- a/drivers/memory/omap-gpmc.c
> > +++ b/drivers/memory/omap-gpmc.c
> > @@ -132,6 +132,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)
> > @@ -1881,6 +1882,21 @@ int gpmc_cs_program_settings(int cs, struct gpmc_settings *p)
> >  
> >  	gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, config1);
> >  
> > +	if (p->wait_on_read || p->wait_on_write) {
> > +		config1 = gpmc_read_reg(GPMC_CONFIG);
> > +
> > +		if (p->wait_pin_polarity == WAITPINPOLARITY_ACTIVE_LOW)
> > +			config1 &= ~GPMC_CONFIG_WAITPINPOLARITY(p->wait_pin);
> > +		else if (p->wait_pin_polarity == WAITPINPOLARITY_ACTIVE_HIGH)
> > +			config1 |= GPMC_CONFIG_WAITPINPOLARITY(p->wait_pin);
> > +		else if (p->wait_pin_polarity != WAITPINPOLARITY_DEFAULT)
> > +			pr_err("%s: invalid wait-pin-polarity (pin: %d, polarity: %d)\n",
> > +				__func__, p->wait_pin, p->wait_pin_polarity);
> > +
> > +		gpmc_write_reg(GPMC_CONFIG, config1);
> > +	}
> > +
> > +
> >  	return 0;
> >  }
> >  
> > @@ -1981,6 +1997,12 @@ void gpmc_read_settings_dt(struct device_node *np, struct gpmc_settings *p)
> >  	}
> >  
> >  	if (!of_property_read_u32(np, "gpmc,wait-pin", &p->wait_pin)) {
> > +
> > +		p->wait_pin_polarity = WAITPINPOLARITY_DEFAULT;
> > +		of_property_read_u32(np,
> > +			"gpmc,wait-pin-polarity",
> > +			&p->wait_pin_polarity);
> > +
> >  		p->wait_on_read = of_property_read_bool(np,
> >  							"gpmc,wait-on-read");
> >  		p->wait_on_write = of_property_read_bool(np,
> > diff --git a/include/linux/platform_data/gpmc-omap.h b/include/linux/platform_data/gpmc-omap.h
> > index c9cc4e32435d..c46c28069c31 100644
> > --- a/include/linux/platform_data/gpmc-omap.h
> > +++ b/include/linux/platform_data/gpmc-omap.h
> > @@ -136,6 +136,11 @@ 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 WAITPINPOLARITY_DEFAULT -1
> > +#define WAITPINPOLARITY_ACTIVE_LOW 0
> > +#define WAITPINPOLARITY_ACTIVE_HIGH 1
> > +
> >  struct gpmc_settings {
> >  	bool burst_wrap;	/* enables wrap bursting */
> >  	bool burst_read;	/* enables read page/burst mode */
> > @@ -149,6 +154,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;	/* wait-pin polarity */
> >  };
> >  
> >  /* Data for each chip select */
> 
> cheers,
> -roger

cheers,
benedikt


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-09-16  9:18 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-15  9:13 [PATCH v4 0/3] omap-gpmc wait pin additions B. Niedermayr
2022-09-15  9:13 ` [PATCH v4 1/3] memory: omap-gpmc: allow shared wait pins B. Niedermayr
2022-09-16  8:50   ` Roger Quadros
2022-09-15  9:13 ` [PATCH v4 2/3] memory: omap-gpmc: add support for wait pin polarity B. Niedermayr
2022-09-16  8:46   ` Roger Quadros
2022-09-16  8:55   ` Roger Quadros
2022-09-16  9:17     ` Niedermayr, BENEDIKT
2022-09-15  9:13 ` [PATCH v4 3/3] dt-bindings: memory-controllers: gpmc-child: add wait-pin polarity B. Niedermayr

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.