From de67fa80c66992b13dd018ec18e8c91156522c18 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Fri, 21 Oct 2022 15:39:28 -0500 Subject: [PATCH 1/5] dt-bindings: memory-controllers: arm,pl353-smc: Extend to support 'arm,pl354' SMC Add support for the Arm PL354 static memory controller to the existing Arm PL353 binding. Both are different configurations of the same IP with support for different types of memory interfaces. The 'arm,pl354' binding has already been in use upstream for a long time in Arm development boards. The existing users have only the controller without any child devices, so drop the required address properties (ranges, #address-cells, #size-cells). The schema for 'ranges' is too constrained as the order is not important and the PL354 has 8 chipselects (And the PL353 actually has up to 8 too). The clocks aren't really correct in either case. There's 1 bus clock and then a clock for each of the 2 memory interfaces. Signed-off-by: Rob Herring Link: https://lore.kernel.org/r/20221021203928.286169-1-robh@kernel.org Signed-off-by: Krzysztof Kozlowski --- ...{arm,pl353-smc.yaml => arm,pl35x-smc.yaml} | 80 ++++++++++++------- 1 file changed, 53 insertions(+), 27 deletions(-) rename Documentation/devicetree/bindings/memory-controllers/{arm,pl353-smc.yaml => arm,pl35x-smc.yaml} (65%) diff --git a/Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml b/Documentation/devicetree/bindings/memory-controllers/arm,pl35x-smc.yaml similarity index 65% rename from Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml rename to Documentation/devicetree/bindings/memory-controllers/arm,pl35x-smc.yaml index 01c9acf9275d..bd23257fe021 100644 --- a/Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml +++ b/Documentation/devicetree/bindings/memory-controllers/arm,pl35x-smc.yaml @@ -1,26 +1,31 @@ # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 --- -$id: http://devicetree.org/schemas/memory-controllers/arm,pl353-smc.yaml# +$id: http://devicetree.org/schemas/memory-controllers/arm,pl35x-smc.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: ARM PL353 Static Memory Controller (SMC) device-tree bindings +title: Arm PL35x Series Static Memory Controller (SMC) maintainers: - Miquel Raynal - Naga Sureshkumar Relli -description: - The PL353 Static Memory Controller is a bus where you can connect two kinds +description: | + The PL35x Static Memory Controller is a bus where you can connect two kinds of memory interfaces, which are NAND and memory mapped interfaces (such as - SRAM or NOR). + SRAM or NOR) depending on the specific configuration. + + The TRM is available here: + https://documentation-service.arm.com/static/5e8e2524fd977155116a58aa # We need a select here so we don't match all nodes with 'arm,primecell' select: properties: compatible: contains: - const: arm,pl353-smc-r2p1 + enum: + - arm,pl353-smc-r2p1 + - arm,pl354 required: - compatible @@ -30,7 +35,9 @@ properties: compatible: items: - - const: arm,pl353-smc-r2p1 + - enum: + - arm,pl353-smc-r2p1 + - arm,pl354 - const: arm,primecell "#address-cells": @@ -46,30 +53,25 @@ properties: The three chip select regions are defined in 'ranges'. clocks: - items: - - description: clock for the memory device bus - - description: main clock of the SMC + minItems: 1 + maxItems: 2 clock-names: - items: - - const: memclk - - const: apb_pclk + minItems: 1 + maxItems: 2 ranges: minItems: 1 - description: | - Memory bus areas for interacting with the devices. Reflects - the memory layout with four integer values following: - 0 - items: - - description: NAND bank 0 - - description: NOR/SRAM bank 0 - - description: NOR/SRAM bank 1 + maxItems: 8 - interrupts: true + interrupts: + minItems: 1 + items: + - description: Combined or Memory interface 0 IRQ + - description: Memory interface 1 IRQ patternProperties: - "@[0-3],[a-f0-9]+$": + "@[0-7],[a-f0-9]+$": type: object description: | The child device node represents the controller connected to the SMC @@ -87,7 +89,7 @@ patternProperties: - description: | Chip-select ID, as in the parent range property. minimum: 0 - maximum: 2 + maximum: 7 - description: | Offset of the memory region requested by the device. - description: | @@ -102,12 +104,36 @@ required: - reg - clock-names - clocks - - "#address-cells" - - "#size-cells" - - ranges additionalProperties: false +allOf: + - if: + properties: + compatible: + contains: + const: arm,pl354 + then: + properties: + clocks: + # According to TRM, really should be 3 clocks + maxItems: 1 + + clock-names: + const: apb_pclk + + else: + properties: + clocks: + items: + - description: clock for the memory device bus + - description: main clock of the SMC + + clock-names: + items: + - const: memclk + - const: apb_pclk + examples: - | smcc: memory-controller@e000e000 { From 3821e96a01d658e770074331b56cec88c169a418 Mon Sep 17 00:00:00 2001 From: Lukas Bulwahn Date: Mon, 31 Oct 2022 12:02:23 +0100 Subject: [PATCH 2/5] MAINTAINERS: arm,pl353-smc: correct dt-binding path Commit de67fa80c669 ("dt-bindings: memory-controllers: arm,pl353-smc: Extend to support 'arm,pl354' SMC") renames the arm,pl353-smc.yaml memory-controller dt-binding, but misses to adjust its reference in MAINTAINERS. Signed-off-by: Lukas Bulwahn Link: https://lore.kernel.org/r/20221031110223.30203-1-lukas.bulwahn@gmail.com Signed-off-by: Krzysztof Kozlowski --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index cf0f18502372..599a84f600c9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1685,7 +1685,7 @@ M: Miquel Raynal M: Naga Sureshkumar Relli L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) S: Maintained -F: Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml +F: Documentation/devicetree/bindings/memory-controllers/arm,pl35x-smc.yaml F: drivers/memory/pl353-smc.c ARM PRIMECELL CLCD PL110 DRIVER From 89aed3cd5cb951113b766cddd9c2df43cfbdafd5 Mon Sep 17 00:00:00 2001 From: Benedikt Niedermayr Date: Wed, 2 Nov 2022 14:30:46 +0100 Subject: [PATCH 3/5] memory: omap-gpmc: wait pin additions 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 WAITPINPOLARITY 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 Link: https://lore.kernel.org/r/20221102133047.1654449-2-benedikt.niedermayr@siemens.com Reviewed-by: Roger Quadros Signed-off-by: Krzysztof Kozlowski --- 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 */ From 1f1e46b83b7db08c8db31816c857e27da84d4ca3 Mon Sep 17 00:00:00 2001 From: Benedikt Niedermayr Date: Wed, 2 Nov 2022 14:30:47 +0100 Subject: [PATCH 4/5] dt-bindings: memory-controllers: ti,gpmc: add wait-pin polarity 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 WAITPINPOLARITY bit in the GPMC_CONFIG register by setting the "ti,wait-pin-polarity" dt-property. Signed-off-by: Benedikt Niedermayr Reviewed-by: Rob Herring Reviewed-by: Roger Quadros Link: https://lore.kernel.org/r/20221102133047.1654449-3-benedikt.niedermayr@siemens.com Signed-off-by: Krzysztof Kozlowski --- .../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 From 8dd7e4af585331dda004e92ed0739c3609e37177 Mon Sep 17 00:00:00 2001 From: Benedikt Niedermayr Date: Wed, 9 Nov 2022 11:24:54 +0100 Subject: [PATCH 5/5] memory: omap-gpmc: fix coverity issue "Control flow issues" Assign a big positive integer instead of an negative integer to an u32 variable. Also remove the check for ">= 0" which doesn't make sense for unsigned integers. Reported-by: coverity-bot Addresses-Coverity-ID: 1527139 ("Control flow issues") Fixes: 89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions") Signed-off-by: Benedikt Niedermayr Reviewed-by: Roger Quadros Link: https://lore.kernel.org/r/20221109102454.174320-1-benedikt.niedermayr@siemens.com Signed-off-by: Krzysztof Kozlowski --- drivers/memory/omap-gpmc.c | 2 +- include/linux/platform_data/gpmc-omap.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c index e427572712e2..57d9f91fe89b 100644 --- a/drivers/memory/omap-gpmc.c +++ b/drivers/memory/omap-gpmc.c @@ -1045,7 +1045,7 @@ EXPORT_SYMBOL(gpmc_cs_free); static bool gpmc_is_valid_waitpin(u32 waitpin) { - return waitpin >= 0 && waitpin < gpmc_nr_waitpins; + return waitpin < gpmc_nr_waitpins; } static int gpmc_alloc_waitpin(struct gpmc_device *gpmc, diff --git a/include/linux/platform_data/gpmc-omap.h b/include/linux/platform_data/gpmc-omap.h index 296b080c5c67..dcca6c5e23bb 100644 --- a/include/linux/platform_data/gpmc-omap.h +++ b/include/linux/platform_data/gpmc-omap.h @@ -137,11 +137,11 @@ struct gpmc_device_timings { #define GPMC_MUX_AD 2 /* Addr-Data multiplex */ /* Wait pin polarity values */ -#define GPMC_WAITPINPOLARITY_INVALID -1 +#define GPMC_WAITPINPOLARITY_INVALID UINT_MAX #define GPMC_WAITPINPOLARITY_ACTIVE_LOW 0 #define GPMC_WAITPINPOLARITY_ACTIVE_HIGH 1 -#define GPMC_WAITPIN_INVALID -1 +#define GPMC_WAITPIN_INVALID UINT_MAX struct gpmc_settings { bool burst_wrap; /* enables wrap bursting */