mbox series

[v9,0/2] gpmc wait pin additions

Message ID 20221102133047.1654449-1-benedikt.niedermayr@siemens.com (mailing list archive)
Headers show
Series gpmc wait pin additions | expand

Message

Benedikt Niedermayr Nov. 2, 2022, 1:30 p.m. UTC
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

Comments

Krzysztof Kozlowski Nov. 2, 2022, 2:02 p.m. UTC | #1
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
Krzysztof Kozlowski Nov. 2, 2022, 2:03 p.m. UTC | #2
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,
Benedikt Niedermayr Nov. 3, 2022, 8:07 a.m. UTC | #3
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
Benedikt Niedermayr Nov. 3, 2022, 8:13 a.m. UTC | #4
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
Krzysztof Kozlowski Nov. 3, 2022, 12:37 p.m. UTC | #5
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