Linux GPIO subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH] gpio: aspeed: Pass irqchip when adding gpiochip
From: Andrew Jeffery @ 2019-08-13  4:36 UTC (permalink / raw)
  To: Linus Walleij, linux-gpio
  Cc: Bartosz Golaszewski, Joel Stanley, Benjamin Herrenschmidt,
	Thierry Reding
In-Reply-To: <20190809125515.19094-1-linus.walleij@linaro.org>



On Fri, 9 Aug 2019, at 22:25, Linus Walleij wrote:
> We need to convert all old gpio irqchips to pass the irqchip
> setup along when adding the gpio_chip. For more info see
> drivers/gpio/TODO.
> 
> For chained irqchips this is a pretty straight-forward
> conversion.
> 
> Cc: Joel Stanley <joel@jms.id.au>
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Hi Aspeed folks, it'd be great if you could test/review this
> patch.

Used it to exercise our WIP GPIO model for qemu. After fixing the qemu bugs it
worked fine there. Booted it on a Witherspoon and things worked as expected
there too. From a brief inspection it looks fine to me.

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
Tested-by: Andrew Jeffery <andrew@aj.id.au>

> ---
>  drivers/gpio/gpio-aspeed.c | 60 ++++++++++++++++++--------------------
>  1 file changed, 29 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
> index 13d80bfbc3b6..9defe25d4721 100644
> --- a/drivers/gpio/gpio-aspeed.c
> +++ b/drivers/gpio/gpio-aspeed.c
> @@ -711,32 +711,6 @@ static void set_irq_valid_mask(struct aspeed_gpio *gpio)
>  	}
>  }
>  
> -static int aspeed_gpio_setup_irqs(struct aspeed_gpio *gpio,
> -		struct platform_device *pdev)
> -{
> -	int rc;
> -
> -	rc = platform_get_irq(pdev, 0);
> -	if (rc < 0)
> -		return rc;
> -
> -	gpio->irq = rc;
> -
> -	set_irq_valid_mask(gpio);
> -
> -	rc = gpiochip_irqchip_add(&gpio->chip, &aspeed_gpio_irqchip,
> -			0, handle_bad_irq, IRQ_TYPE_NONE);
> -	if (rc) {
> -		dev_info(&pdev->dev, "Could not add irqchip\n");
> -		return rc;
> -	}
> -
> -	gpiochip_set_chained_irqchip(&gpio->chip, &aspeed_gpio_irqchip,
> -				     gpio->irq, aspeed_gpio_irq_handler);
> -
> -	return 0;
> -}
> -
>  static int aspeed_gpio_reset_tolerance(struct gpio_chip *chip,
>  					unsigned int offset, bool enable)
>  {
> @@ -1189,7 +1163,6 @@ static int __init aspeed_gpio_probe(struct 
> platform_device *pdev)
>  	gpio->chip.set_config = aspeed_gpio_set_config;
>  	gpio->chip.label = dev_name(&pdev->dev);
>  	gpio->chip.base = -1;
> -	gpio->chip.irq.need_valid_mask = true;
>  
>  	/* Allocate a cache of the output registers */
>  	banks = gpio->config->nr_gpios >> 5;
> @@ -1212,16 +1185,41 @@ static int __init aspeed_gpio_probe(struct 
> platform_device *pdev)
>  		aspeed_gpio_change_cmd_source(gpio, bank, 3, GPIO_CMDSRC_ARM);
>  	}
>  
> -	rc = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
> -	if (rc < 0)
> -		return rc;
> +	/* Optionally set up an irqchip if there is an IRQ */
> +	rc = platform_get_irq(pdev, 0);
> +	if (rc > 0) {
> +		struct gpio_irq_chip *girq;
> +
> +		gpio->irq = rc;
> +		girq = &gpio->chip.irq;
> +		girq->chip = &aspeed_gpio_irqchip;
> +		girq->parent_handler = aspeed_gpio_irq_handler;
> +		girq->num_parents = 1;
> +		girq->parents = devm_kcalloc(&pdev->dev, 1,
> +					     sizeof(*girq->parents),
> +					     GFP_KERNEL);
> +		if (!girq->parents)
> +			return -ENOMEM;
> +		girq->parents[0] = gpio->irq;
> +		girq->default_type = IRQ_TYPE_NONE;
> +		girq->handler = handle_bad_irq;
> +		girq->need_valid_mask = true;
> +	}
>  
>  	gpio->offset_timer =
>  		devm_kzalloc(&pdev->dev, gpio->chip.ngpio, GFP_KERNEL);
>  	if (!gpio->offset_timer)
>  		return -ENOMEM;
>  
> -	return aspeed_gpio_setup_irqs(gpio, pdev);
> +	rc = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
> +	if (rc < 0)
> +		return rc;
> +
> +	/* Now the valid mask is allocated */
> +	if (gpio->irq)
> +		set_irq_valid_mask(gpio);
> +
> +	return 0;
>  }
>  
>  static struct platform_driver aspeed_gpio_driver = {
> -- 
> 2.21.0
> 
>

^ permalink raw reply

* Re: [PATCH 3/7] dma: iop-adma: use correct printk format strings
From: Vinod Koul @ 2019-08-13  4:33 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: soc, Russell King, Dan Williams, Linus Walleij,
	Bartosz Golaszewski, linux-arm-kernel, linux-kernel, dmaengine,
	linux-gpio, linux-i2c
In-Reply-To: <20190809163334.489360-3-arnd@arndb.de>

On 09-08-19, 18:33, Arnd Bergmann wrote:
> When compile-testing on other architectures, we get lots of warnings
> about incorrect format strings, like:
> 
>    drivers/dma/iop-adma.c: In function 'iop_adma_alloc_slots':
>    drivers/dma/iop-adma.c:307:6: warning: format '%x' expects argument of type 'unsigned int', but argument 6 has type 'dma_addr_t {aka long long unsigned int}' [-Wformat=]
> 
>    drivers/dma/iop-adma.c: In function 'iop_adma_prep_dma_memcpy':
> >> drivers/dma/iop-adma.c:518:40: warning: format '%u' expects argument of type 'unsigned int', but argument 5 has type 'size_t {aka long unsigned int}' [-Wformat=]
> 
> Use %zu for printing size_t as required, and cast the dma_addr_t
> arguments to 'u64' for printing with %llx. Ideally this should use
> the %pad format string, but that requires an lvalue argument that
> doesn't work here.

Please change title to "dmaengine: iop-adma: use correct printk format strings"

After that:

Acked-by: Vinod Koul <vkoul@kernel.org>

> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/dma/iop-adma.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/dma/iop-adma.c b/drivers/dma/iop-adma.c
> index 7857b54770d1..aebdd671651a 100644
> --- a/drivers/dma/iop-adma.c
> +++ b/drivers/dma/iop-adma.c
> @@ -117,9 +117,9 @@ static void __iop_adma_slot_cleanup(struct iop_adma_chan *iop_chan)
>  	list_for_each_entry_safe(iter, _iter, &iop_chan->chain,
>  					chain_node) {
>  		pr_debug("\tcookie: %d slot: %d busy: %d "
> -			"this_desc: %#x next_desc: %#x ack: %d\n",
> +			"this_desc: %#x next_desc: %#llx ack: %d\n",
>  			iter->async_tx.cookie, iter->idx, busy,
> -			iter->async_tx.phys, iop_desc_get_next_desc(iter),
> +			iter->async_tx.phys, (u64)iop_desc_get_next_desc(iter),
>  			async_tx_test_ack(&iter->async_tx));
>  		prefetch(_iter);
>  		prefetch(&_iter->async_tx);
> @@ -307,9 +307,9 @@ iop_adma_alloc_slots(struct iop_adma_chan *iop_chan, int num_slots,
>  				int i;
>  				dev_dbg(iop_chan->device->common.dev,
>  					"allocated slot: %d "
> -					"(desc %p phys: %#x) slots_per_op %d\n",
> +					"(desc %p phys: %#llx) slots_per_op %d\n",
>  					iter->idx, iter->hw_desc,
> -					iter->async_tx.phys, slots_per_op);
> +					(u64)iter->async_tx.phys, slots_per_op);
>  
>  				/* pre-ack all but the last descriptor */
>  				if (num_slots != slots_per_op)
> @@ -517,7 +517,7 @@ iop_adma_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dma_dest,
>  		return NULL;
>  	BUG_ON(len > IOP_ADMA_MAX_BYTE_COUNT);
>  
> -	dev_dbg(iop_chan->device->common.dev, "%s len: %u\n",
> +	dev_dbg(iop_chan->device->common.dev, "%s len: %zu\n",
>  		__func__, len);
>  
>  	spin_lock_bh(&iop_chan->lock);
> @@ -550,7 +550,7 @@ iop_adma_prep_dma_xor(struct dma_chan *chan, dma_addr_t dma_dest,
>  	BUG_ON(len > IOP_ADMA_XOR_MAX_BYTE_COUNT);
>  
>  	dev_dbg(iop_chan->device->common.dev,
> -		"%s src_cnt: %d len: %u flags: %lx\n",
> +		"%s src_cnt: %d len: %zu flags: %lx\n",
>  		__func__, src_cnt, len, flags);
>  
>  	spin_lock_bh(&iop_chan->lock);
> @@ -583,7 +583,7 @@ iop_adma_prep_dma_xor_val(struct dma_chan *chan, dma_addr_t *dma_src,
>  	if (unlikely(!len))
>  		return NULL;
>  
> -	dev_dbg(iop_chan->device->common.dev, "%s src_cnt: %d len: %u\n",
> +	dev_dbg(iop_chan->device->common.dev, "%s src_cnt: %d len: %zu\n",
>  		__func__, src_cnt, len);
>  
>  	spin_lock_bh(&iop_chan->lock);
> @@ -621,7 +621,7 @@ iop_adma_prep_dma_pq(struct dma_chan *chan, dma_addr_t *dst, dma_addr_t *src,
>  	BUG_ON(len > IOP_ADMA_XOR_MAX_BYTE_COUNT);
>  
>  	dev_dbg(iop_chan->device->common.dev,
> -		"%s src_cnt: %d len: %u flags: %lx\n",
> +		"%s src_cnt: %d len: %zu flags: %lx\n",
>  		__func__, src_cnt, len, flags);
>  
>  	if (dmaf_p_disabled_continue(flags))
> @@ -684,7 +684,7 @@ iop_adma_prep_dma_pq_val(struct dma_chan *chan, dma_addr_t *pq, dma_addr_t *src,
>  		return NULL;
>  	BUG_ON(len > IOP_ADMA_XOR_MAX_BYTE_COUNT);
>  
> -	dev_dbg(iop_chan->device->common.dev, "%s src_cnt: %d len: %u\n",
> +	dev_dbg(iop_chan->device->common.dev, "%s src_cnt: %d len: %zu\n",
>  		__func__, src_cnt, len);
>  
>  	spin_lock_bh(&iop_chan->lock);
> -- 
> 2.20.0

-- 
~Vinod

^ permalink raw reply

* Re: [PATCH 2/7] dma: iop-adma: include prefetch.h
From: Vinod Koul @ 2019-08-13  4:33 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: soc, Russell King, Dan Williams, Linus Walleij,
	Bartosz Golaszewski, linux-arm-kernel, linux-kernel, dmaengine,
	linux-gpio, linux-i2c, kbuild test robot
In-Reply-To: <20190809163334.489360-2-arnd@arndb.de>

On 09-08-19, 18:33, Arnd Bergmann wrote:
> Compile-testing this driver fails on m68k without the
> extra header inclusion.

Please change title to "dmaengine: iop-adma: include prefetch.h"

After that:

Acked-by: Vinod Koul <vkoul@kernel.org>

> Reported-by: kbuild test robot <lkp@intel.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/dma/iop-adma.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/dma/iop-adma.c b/drivers/dma/iop-adma.c
> index c6c0143670d9..7857b54770d1 100644
> --- a/drivers/dma/iop-adma.c
> +++ b/drivers/dma/iop-adma.c
> @@ -16,6 +16,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/interrupt.h>
>  #include <linux/platform_device.h>
> +#include <linux/prefetch.h>
>  #include <linux/memory.h>
>  #include <linux/ioport.h>
>  #include <linux/raid/pq.h>
> -- 
> 2.20.0

-- 
~Vinod

^ permalink raw reply

* [PATCH v2] arm64: dts: ls1088a: fix gpio node
From: Hui Song @ 2019-08-13  2:04 UTC (permalink / raw)
  To: Shawn Guo, Li Yang, Rob Herring, Mark Rutland, Linus Walleij,
	Bartosz Golaszewski
  Cc: linux-arm-kernel, devicetree, linux-kernel, linux-gpio, Song Hui

From: Song Hui <hui.song_1@nxp.com>

Update the nodes to include little-endian
property to be consistent with the hardware
and add ls1088a gpio specify compatible.

Signed-off-by: Song Hui <hui.song_1@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
index 20f5ebd..d58d203 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
@@ -269,43 +269,47 @@
 		};
 
 		gpio0: gpio@2300000 {
-			compatible = "fsl,qoriq-gpio";
+			compatible = "fsl,ls1088a-gpio", "fsl,qoriq-gpio";
 			reg = <0x0 0x2300000 0x0 0x10000>;
 			interrupts = <0 36 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
+			little-endian;
 		};
 
 		gpio1: gpio@2310000 {
-			compatible = "fsl,qoriq-gpio";
+			compatible = "fsl,ls1088a-gpio", "fsl,qoriq-gpio";
 			reg = <0x0 0x2310000 0x0 0x10000>;
 			interrupts = <0 36 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
+			little-endian;
 		};
 
 		gpio2: gpio@2320000 {
-			compatible = "fsl,qoriq-gpio";
+			compatible = "fsl,ls1088a-gpio", "fsl,qoriq-gpio";
 			reg = <0x0 0x2320000 0x0 0x10000>;
 			interrupts = <0 37 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
+			little-endian;
 		};
 
 		gpio3: gpio@2330000 {
-			compatible = "fsl,qoriq-gpio";
+			compatible = "fsl,ls1088a-gpio", "fsl,qoriq-gpio";
 			reg = <0x0 0x2330000 0x0 0x10000>;
 			interrupts = <0 37 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
+			little-endian;
 		};
 
 		ifc: ifc@2240000 {
-- 
2.9.5


^ permalink raw reply related

* [PATCH v2] gpiolib: Take MUX usage into account
From: Ramon Fried @ 2019-08-13  1:42 UTC (permalink / raw)
  To: linus.walleij, bgolaszewski, stefan.wahren; +Cc: linux-gpio, linux-kernel

From: Stefan Wahren <stefan.wahren@i2se.com>

The user space like gpioinfo only see the GPIO usage but not the
MUX usage (e.g. I2C or SPI usage) of a pin. As a user we want to know which
pin is free/safe to use. So take the MUX usage of strict pinmux controllers
into account to get a more realistic view for ioctl GPIO_GET_LINEINFO_IOCTL.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
Tested-by: Ramon Fried <rfried.dev@gmail.com>
Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
---
v2: Address review from linus:
* ** Please notive logic was reversed **
* renamed pinctrl_gpio_is_in_use() to pinctrl_gpio_can_use_line()
* renamed pinmux_is_in_use() to pinmux_can_be_used_for_gpio()
* changed dev_err to dev_dbg (Linus suggested removing it altogether, I
  find it better to keep it for debug).

 drivers/gpio/gpiolib.c           |  3 ++-
 drivers/pinctrl/core.c           | 28 ++++++++++++++++++++++++++++
 drivers/pinctrl/pinmux.c         | 27 +++++++++++++++++++++++++++
 drivers/pinctrl/pinmux.h         |  8 ++++++++
 include/linux/pinctrl/consumer.h |  6 ++++++
 5 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f497003f119c..52937bf8e514 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1084,7 +1084,8 @@ static long gpio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		    test_bit(FLAG_IS_HOGGED, &desc->flags) ||
 		    test_bit(FLAG_USED_AS_IRQ, &desc->flags) ||
 		    test_bit(FLAG_EXPORT, &desc->flags) ||
-		    test_bit(FLAG_SYSFS, &desc->flags))
+		    test_bit(FLAG_SYSFS, &desc->flags) ||
+		    !pinctrl_gpio_can_use_line(chip->base + lineinfo.line_offset))
 			lineinfo.flags |= GPIOLINE_FLAG_KERNEL;
 		if (test_bit(FLAG_IS_OUT, &desc->flags))
 			lineinfo.flags |= GPIOLINE_FLAG_IS_OUT;
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index b70df27874d1..2bbd8ee93507 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -736,6 +736,34 @@ int pinctrl_get_group_selector(struct pinctrl_dev *pctldev,
 	return -EINVAL;
 }
 
+bool pinctrl_gpio_can_use_line(unsigned gpio)
+{
+	struct pinctrl_dev *pctldev;
+	struct pinctrl_gpio_range *range;
+	bool result;
+	int pin;
+
+	/*
+	 * Try to obtain GPIO range, if it fails
+	 * we're probably dealing with GPIO driver
+	 * without a backing pin controller - bail out.
+	 */
+	if (pinctrl_get_device_gpio_range(gpio, &pctldev, &range))
+		return true;
+
+	mutex_lock(&pctldev->mutex);
+
+	/* Convert to the pin controllers number space */
+	pin = gpio_to_pin(range, gpio);
+
+	result = pinmux_can_be_used_for_gpio(pctldev, pin);
+
+	mutex_unlock(&pctldev->mutex);
+
+	return result;
+}
+EXPORT_SYMBOL_GPL(pinctrl_gpio_can_use_line);
+
 /**
  * pinctrl_gpio_request() - request a single pin to be used as GPIO
  * @gpio: the GPIO pin number from the GPIO subsystem number space
diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index 020e54f843f9..7e42a5738d82 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -70,6 +70,33 @@ int pinmux_validate_map(const struct pinctrl_map *map, int i)
 	return 0;
 }
 
+/**
+ * pinmux_can_be_used_for_gpio() - check if a specific pin
+ *	is either muxed to a different function or used as gpio.
+ *
+ * @pin: the pin number in the global pin space
+ *
+ * Controllers not defined as strict will always return true,
+ * menaning that the gpio can be used.
+ */
+bool pinmux_can_be_used_for_gpio(struct pinctrl_dev *pctldev, unsigned pin)
+{
+	struct pin_desc *desc = pin_desc_get(pctldev, pin);
+	const struct pinmux_ops *ops = pctldev->desc->pmxops;
+
+	if (!desc) {
+		dev_dbg(pctldev->dev,
+			"pin %u is not registered so it cannot be requested\n",
+			pin);
+		return true;
+	}
+
+	if (ops->strict && desc->mux_usecount)
+		return false;
+
+	return !(ops->strict && !!desc->gpio_owner);
+}
+
 /**
  * pin_request() - request a single pin to be muxed in, typically for GPIO
  * @pin: the pin number in the global pin space
diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
index 794cb3a003ff..78c3a31be882 100644
--- a/drivers/pinctrl/pinmux.h
+++ b/drivers/pinctrl/pinmux.h
@@ -15,6 +15,8 @@ int pinmux_check_ops(struct pinctrl_dev *pctldev);
 
 int pinmux_validate_map(const struct pinctrl_map *map, int i);
 
+bool pinmux_can_be_used_for_gpio(struct pinctrl_dev *pctldev, unsigned pin);
+
 int pinmux_request_gpio(struct pinctrl_dev *pctldev,
 			struct pinctrl_gpio_range *range,
 			unsigned pin, unsigned gpio);
@@ -42,6 +44,12 @@ static inline int pinmux_validate_map(const struct pinctrl_map *map, int i)
 	return 0;
 }
 
+static inline bool pinmux_can_be_used_for_gpio(struct pinctrl_dev *pctldev,
+					       unsigned pin)
+{
+	return true;
+}
+
 static inline int pinmux_request_gpio(struct pinctrl_dev *pctldev,
 			struct pinctrl_gpio_range *range,
 			unsigned pin, unsigned gpio)
diff --git a/include/linux/pinctrl/consumer.h b/include/linux/pinctrl/consumer.h
index 86720a5a384f..7f8c7d9583d3 100644
--- a/include/linux/pinctrl/consumer.h
+++ b/include/linux/pinctrl/consumer.h
@@ -24,6 +24,7 @@ struct device;
 #ifdef CONFIG_PINCTRL
 
 /* External interface to pin control */
+extern bool pinctrl_gpio_can_use_line(unsigned gpio);
 extern int pinctrl_gpio_request(unsigned gpio);
 extern void pinctrl_gpio_free(unsigned gpio);
 extern int pinctrl_gpio_direction_input(unsigned gpio);
@@ -61,6 +62,11 @@ static inline int pinctrl_pm_select_idle_state(struct device *dev)
 
 #else /* !CONFIG_PINCTRL */
 
+static inline bool pinctrl_gpio_can_use_line(unsigned gpio)
+{
+	return true;
+}
+
 static inline int pinctrl_gpio_request(unsigned gpio)
 {
 	return 0;
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH 04/18] ARM: dts: bcm283x: Define MMC interfaces at board level
From: Stefan Wahren @ 2019-08-12 21:03 UTC (permalink / raw)
  To: Eric Anholt, Florian Fainelli, Ray Jui, Scott Branden,
	Nicolas Saenz Julienne, Matthias Brugger, Rob Herring,
	Mark Rutland, Linus Walleij, Michael Turquette, Stephen Boyd,
	Ulf Hansson, Adrian Hunter
  Cc: linux-mmc, linux-gpio, bcm-kernel-feedback-list, linux-rpi-kernel,
	linux-arm-kernel
In-Reply-To: <1563774880-8061-5-git-send-email-wahrenst@gmx.net>

Am 22.07.19 um 07:54 schrieb Stefan Wahren:
> Starting with RPi 4 this is the first board, which doesn't use sdhost
> as default SD interface. So the MMC interfaces should be defined finally at
> board level. Since all boards using sdhci already does this, we can drop the
> pinctrl part from bcm2835-rpi.dtsi.
>
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Applied to bcm2835-dt-next

^ permalink raw reply

* Re: [PATCH 05/18] ARM: dts: bcm283x: Define memory at board level
From: Stefan Wahren @ 2019-08-12 21:02 UTC (permalink / raw)
  To: Eric Anholt, Florian Fainelli, Ray Jui, Scott Branden,
	Nicolas Saenz Julienne, Matthias Brugger, Rob Herring,
	Mark Rutland, Linus Walleij, Michael Turquette, Stephen Boyd,
	Ulf Hansson, Adrian Hunter
  Cc: linux-mmc, linux-gpio, bcm-kernel-feedback-list, linux-rpi-kernel,
	linux-arm-kernel
In-Reply-To: <1563774880-8061-6-git-send-email-wahrenst@gmx.net>

Am 22.07.19 um 07:54 schrieb Stefan Wahren:
> Now with the varity of several RPi boards, the memory should be defined
> at board level. This step gives us the chance to fix the memory size
> of the RPi 1 B+, Zero (incl. W) and Compute Module 1.
>
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Applied to bcm2835-dt-next

^ permalink raw reply

* RE: [PATCH] arm64: dts: ls1028a: fix gpio nodes
From: Leo Li @ 2019-08-12 21:01 UTC (permalink / raw)
  To: Shawn Guo, Hui Song
  Cc: Rob Herring, Mark Rutland, Linus Walleij, Bartosz Golaszewski,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org
In-Reply-To: <20190812134716.GF27041@X250>



> -----Original Message-----
> From: Shawn Guo <shawnguo@kernel.org>
> Sent: Monday, August 12, 2019 8:47 AM
> To: Hui Song <hui.song_1@nxp.com>; Leo Li <leoyang.li@nxp.com>
> Cc: Rob Herring <robh+dt@kernel.org>; Mark Rutland
> <mark.rutland@arm.com>; Linus Walleij <linus.walleij@linaro.org>; Bartosz
> Golaszewski <bgolaszewski@baylibre.com>; linux-arm-
> kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-gpio@vger.kernel.org
> Subject: Re: [PATCH] arm64: dts: ls1028a: fix gpio nodes
> 
> On Mon, Aug 05, 2019 at 02:57:00PM +0800, Hui Song wrote:
> > From: Song Hui <hui.song_1@nxp.com>
> >
> > Update the nodes to include little-endian property to be consistent
> > with the hardware.
> >
> > Signed-off-by: Song Hui <hui.song_1@nxp.com>
> 
> @Leo, looks good?

Acked-by: Li Yang <leoyang.li@nxp.com>

> 
> Shawn
> 
> > ---
> >  arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> > b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> > index aef5b06..7ccbbfc 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> > @@ -277,33 +277,36 @@
> >  		};
> >
> >  		gpio1: gpio@2300000 {
> > -			compatible = "fsl,qoriq-gpio";
> > +			compatible = "fsl,ls1028a-gpio","fsl,qoriq-gpio";
> >  			reg = <0x0 0x2300000 0x0 0x10000>;
> >  			interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
> >  			gpio-controller;
> >  			#gpio-cells = <2>;
> >  			interrupt-controller;
> >  			#interrupt-cells = <2>;
> > +			little-endian;
> >  		};
> >
> >  		gpio2: gpio@2310000 {
> > -			compatible = "fsl,qoriq-gpio";
> > +			compatible = "fsl,ls1028a-gpio","fsl,qoriq-gpio";
> >  			reg = <0x0 0x2310000 0x0 0x10000>;
> >  			interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
> >  			gpio-controller;
> >  			#gpio-cells = <2>;
> >  			interrupt-controller;
> >  			#interrupt-cells = <2>;
> > +			little-endian;
> >  		};
> >
> >  		gpio3: gpio@2320000 {
> > -			compatible = "fsl,qoriq-gpio";
> > +			compatible = "fsl,ls1028a-gpio","fsl,qoriq-gpio";
> >  			reg = <0x0 0x2320000 0x0 0x10000>;
> >  			interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
> >  			gpio-controller;
> >  			#gpio-cells = <2>;
> >  			interrupt-controller;
> >  			#interrupt-cells = <2>;
> > +			little-endian;
> >  		};
> >
> >  		usb0: usb@3100000 {
> > --
> > 2.9.5
> >

^ permalink raw reply

* Re: [PATCH v8 14/21] clk: tegra210: Add suspend and resume support
From: Dmitry Osipenko @ 2019-08-12 20:28 UTC (permalink / raw)
  To: Sowjanya Komatineni
  Cc: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
	linus.walleij, stefan, mark.rutland, pdeschrijver, pgaikwad,
	sboyd, linux-clk, linux-gpio, jckuo, josephl, talho, linux-tegra,
	linux-kernel, mperttunen, spatra, robh+dt, devicetree, rjw,
	viresh.kumar, linux-pm
In-Reply-To: <cd685e84-c0de-6142-597e-f7c77604350e@nvidia.com>

12.08.2019 22:03, Sowjanya Komatineni пишет:
> 
> On 8/12/19 11:19 AM, Dmitry Osipenko wrote:
>> 12.08.2019 20:28, Sowjanya Komatineni пишет:
>>> On 8/12/19 9:25 AM, Dmitry Osipenko wrote:
>>>> 11.08.2019 22:15, Sowjanya Komatineni пишет:
>>>>> On 8/11/19 10:39 AM, Dmitry Osipenko wrote:
>>>>>> 09.08.2019 21:40, Sowjanya Komatineni пишет:
>>>>>>> On 8/9/19 11:18 AM, Dmitry Osipenko wrote:
>>>>>>>> 09.08.2019 19:19, Sowjanya Komatineni пишет:
>>>>>>>>> On 8/9/19 6:56 AM, Dmitry Osipenko wrote:
>>>>>>>>>> 09.08.2019 2:46, Sowjanya Komatineni пишет:
>>>>>>>>>>> This patch adds support for clk: tegra210: suspend-resume.
>>>>>>>>>>>
>>>>>>>>>>> All the CAR controller settings are lost on suspend when core
>>>>>>>>>>> power goes off.
>>>>>>>>>>>
>>>>>>>>>>> This patch has implementation for saving and restoring all PLLs
>>>>>>>>>>> and clocks context during system suspend and resume to have the
>>>>>>>>>>> clocks back to same state for normal operation.
>>>>>>>>>>>
>>>>>>>>>>> Clock driver suspend and resume are registered as syscore_ops as clocks
>>>>>>>>>>> restore need to happen before the other drivers resume to have all their
>>>>>>>>>>> clocks back to the same state as before suspend.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>>>>>>> ---
>>>>>>>>>>>      drivers/clk/tegra/clk-tegra210.c | 103 +++++++++++++++++++++++++++++++++++++--
>>>>>>>>>>>      drivers/clk/tegra/clk.c          |  64 ++++++++++++++++++++++++
>>>>>>>>>>>      drivers/clk/tegra/clk.h          |   3 ++
>>>>>>>>>>>      3 files changed, 166 insertions(+), 4 deletions(-)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>>>> index 998bf60b219a..8dd6f4f4debb 100644
>>>>>>>>>>> --- a/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>>>> @@ -9,13 +9,13 @@
>>>>>>>>>>>      #include <linux/clkdev.h>
>>>>>>>>>>>      #include <linux/of.h>
>>>>>>>>>>>      #include <linux/of_address.h>
>>>>>>>>>>> +#include <linux/syscore_ops.h>
>>>>>>>>>>>      #include <linux/delay.h>
>>>>>>>>>>>      #include <linux/export.h>
>>>>>>>>>>>      #include <linux/mutex.h>
>>>>>>>>>>>      #include <linux/clk/tegra.h>
>>>>>>>>>>>      #include <dt-bindings/clock/tegra210-car.h>
>>>>>>>>>>>      #include <dt-bindings/reset/tegra210-car.h>
>>>>>>>>>>> -#include <linux/iopoll.h>
>>>>>>>>>>>      #include <linux/sizes.h>
>>>>>>>>>>>      #include <soc/tegra/pmc.h>
>>>>>>>>>>>      @@ -220,11 +220,15 @@
>>>>>>>>>>>      #define CLK_M_DIVISOR_SHIFT 2
>>>>>>>>>>>      #define CLK_M_DIVISOR_MASK 0x3
>>>>>>>>>>>      +#define CLK_MASK_ARM    0x44
>>>>>>>>>>> +#define MISC_CLK_ENB    0x48
>>>>>>>>>>> +
>>>>>>>>>>>      #define RST_DFLL_DVCO 0x2f4
>>>>>>>>>>>      #define DVFS_DFLL_RESET_SHIFT 0
>>>>>>>>>>>        #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
>>>>>>>>>>>      #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
>>>>>>>>>>> +#define CPU_SOFTRST_CTRL 0x380
>>>>>>>>>>>        #define LVL2_CLK_GATE_OVRA 0xf8
>>>>>>>>>>>      #define LVL2_CLK_GATE_OVRC 0x3a0
>>>>>>>>>>> @@ -2825,6 +2829,7 @@ static int tegra210_enable_pllu(void)
>>>>>>>>>>>          struct tegra_clk_pll_freq_table *fentry;
>>>>>>>>>>>          struct tegra_clk_pll pllu;
>>>>>>>>>>>          u32 reg;
>>>>>>>>>>> +    int ret;
>>>>>>>>>>>            for (fentry = pll_u_freq_table; fentry->input_rate; fentry++) {
>>>>>>>>>>>              if (fentry->input_rate == pll_ref_freq)
>>>>>>>>>>> @@ -2853,9 +2858,14 @@ static int tegra210_enable_pllu(void)
>>>>>>>>>>>          reg |= PLL_ENABLE;
>>>>>>>>>>>          writel(reg, clk_base + PLLU_BASE);
>>>>>>>>>>>      -    readl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,
>>>>>>>>>>> -                      reg & PLL_BASE_LOCK, 2, 1000);
>>>>>>>>>>> -    if (!(reg & PLL_BASE_LOCK)) {
>>>>>>>>>>> +    /*
>>>>>>>>>>> +     * During clocks resume, same PLLU init and enable sequence get
>>>>>>>>>>> +     * executed. So, readx_poll_timeout_atomic can't be used here as it
>>>>>>>>>>> +     * uses ktime_get() and timekeeping resume doesn't happen by that
>>>>>>>>>>> +     * time. So, using tegra210_wait_for_mask for PLL LOCK.
>>>>>>>>>>> +     */
>>>>>>>>>>> +    ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
>>>>>>>>>>> +    if (ret) {
>>>>>>>>>>>              pr_err("Timed out waiting for PLL_U to lock\n");
>>>>>>>>>>>              return -ETIMEDOUT;
>>>>>>>>>>>          }
>>>>>>>>>>> @@ -3288,6 +3298,84 @@ static void tegra210_disable_cpu_clock(u32 cpu)
>>>>>>>>>>>      }
>>>>>>>>>>>        #ifdef CONFIG_PM_SLEEP
>>>>>>>>>>> +/*
>>>>>>>>>>> + * This array lists mask values for each peripheral clk bank
>>>>>>>>>>> + * to mask out reserved bits during the clocks state restore
>>>>>>>>>>> + * on SC7 resume to prevent accidental writes to these reserved
>>>>>>>>>>> + * bits.
>>>>>>>>>>> + */
>>>>>>>>>>> +static u32 periph_clk_rsvd_mask[TEGRA210_CAR_BANK_COUNT] = {
>>>>>>>>>> Should be more natural to have a "valid_mask" instead of "rsvd_mask".
>>>>>>>>>>
>>>>>>>>>> What's actually wrong with touching of the reserved bits? They must be NO-OP.. or the
>>>>>>>>>> reserved bits are actually some kind of "secret" bits? If those bits have some
>>>>>>>>>> use-case
>>>>>>>>>> outside of Silicon HW (like FPGA simulation), then this doesn't matter for upstream
>>>>>>>>>> and you
>>>>>>>>>> have to keep the workaround locally in the downstream kernel or whatever.
>>>>>>>>> Will rename as valid_mask.
>>>>>>>>>
>>>>>>>>> some bits in these registers are undefined and is not good to write to these bits as
>>>>>>>>> they
>>>>>>>>> can cause pslverr.
>>>>>>>> Okay, it should be explained in the comment.
>>>>>>>>
>>>>>>>> Is it possible to disable trapping of changing the undefined bits?
>>>>>>> No its internal to design
>>>>>> Okay.
>>>>>>
>>>>>> Also, what about to move the valid_mask into struct tegra_clk_periph_regs?
>>>>> No, we cannot move to tegra_clk_periph_regs as its in tegra/clk.c and is common for all
>>>>> tegra.
>>>>>
>>>>> Reserved bits are different on tegra chips so should come from Tegra chip specific clock
>>>>> driver like
>>>>>
>>>>> clk-tegra210 for Tegra210.
>>>> Could you please check whether the reserved bits are RAZ (read as zero)?
>>>>
>>>> [snip]
>>> yes all reserved bits of clk_enb register is 0. This should not be set to 1.
>>>
>>> As I will be changing to variable name to valid_mask instead of reserved mask, will also
>>> change values to valid mask so it can be used directly to write to clk_enb for enabling all
>>> peripherals clks.
>>>
>> It looks to me that the tegra_clk_periph_force_on() could be made local to the
>> clk-tegra210.c and then the raw clk_enb values could be written directly instead of having
>> the clk_enb[] array, probably that will be a bit cleaner
> 
> All CLK_OUT_ENB* registers are already defined in clk driver and also periph_regs includes
> all of these to use.
> 
> To write value to enable all clocks directly without array, it need total 7 individual
> register writes for Tegra210. Also when suspend/resume is implemented for other prior
> tegras, they need to do same in tegra clock driver.
> 
> Reason I had this in clock driver is, this can be used by all tegra clock drivers and just
> can pass valid clocks values.
> 
> But doing individual register write with direct hard code values in corresponding tegra
> clock driver is preferred still, will update so in next revision and will move all the
> CLK_OUT_ENB* register defines to tegra/clk.h
> 
> Currently RST_DEVICES & CLK_OUT_ENB are all in tegra/clk.c

Yes, it should be a bit more clear to share these defines. Also, please define the "valid"
bitmasks with something like TEGRA210_DEVICES_MASK_L.

^ permalink raw reply

* Re: [PATCH] gpio: hlwd: Pass irqchip when adding gpiochip
From: Jonathan Neuschäfer @ 2019-08-12 19:06 UTC (permalink / raw)
  To: Jonathan Neuschäfer
  Cc: Linus Walleij, linux-gpio, Bartosz Golaszewski, Thierry Reding
In-Reply-To: <20190810103523.GC1966@latitude>

[-- Attachment #1: Type: text/plain, Size: 904 bytes --]

On Sat, Aug 10, 2019 at 12:35:23PM +0200, Jonathan Neuschäfer wrote:
> On Fri, Aug 09, 2019 at 04:00:05PM +0200, Linus Walleij wrote:
> > We need to convert all old gpio irqchips to pass the irqchip
> > setup along when adding the gpio_chip. For more info see
> > drivers/gpio/TODO.
> > 
> > For chained irqchips this is a pretty straight-forward
> > conversion.
> > 
> > Cc: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> > Cc: Thierry Reding <treding@nvidia.com>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> 
> Unfortunately, there seems to have been an unrelated regression between
> 5.0 and now affecting the Wii, so I can't easily test this patch right
> now. But it looks good (esp. with git show -b).
> 
> Reviewed-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>

Ok, now I also tested it:

Tested-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v8 14/21] clk: tegra210: Add suspend and resume support
From: Sowjanya Komatineni @ 2019-08-12 19:03 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
	linus.walleij, stefan, mark.rutland, pdeschrijver, pgaikwad,
	sboyd, linux-clk, linux-gpio, jckuo, josephl, talho, linux-tegra,
	linux-kernel, mperttunen, spatra, robh+dt, devicetree, rjw,
	viresh.kumar, linux-pm
In-Reply-To: <1779e92b-fa4d-68ab-9218-51970eee1ec5@gmail.com>


On 8/12/19 11:19 AM, Dmitry Osipenko wrote:
> 12.08.2019 20:28, Sowjanya Komatineni пишет:
>> On 8/12/19 9:25 AM, Dmitry Osipenko wrote:
>>> 11.08.2019 22:15, Sowjanya Komatineni пишет:
>>>> On 8/11/19 10:39 AM, Dmitry Osipenko wrote:
>>>>> 09.08.2019 21:40, Sowjanya Komatineni пишет:
>>>>>> On 8/9/19 11:18 AM, Dmitry Osipenko wrote:
>>>>>>> 09.08.2019 19:19, Sowjanya Komatineni пишет:
>>>>>>>> On 8/9/19 6:56 AM, Dmitry Osipenko wrote:
>>>>>>>>> 09.08.2019 2:46, Sowjanya Komatineni пишет:
>>>>>>>>>> This patch adds support for clk: tegra210: suspend-resume.
>>>>>>>>>>
>>>>>>>>>> All the CAR controller settings are lost on suspend when core
>>>>>>>>>> power goes off.
>>>>>>>>>>
>>>>>>>>>> This patch has implementation for saving and restoring all PLLs
>>>>>>>>>> and clocks context during system suspend and resume to have the
>>>>>>>>>> clocks back to same state for normal operation.
>>>>>>>>>>
>>>>>>>>>> Clock driver suspend and resume are registered as syscore_ops as clocks
>>>>>>>>>> restore need to happen before the other drivers resume to have all their
>>>>>>>>>> clocks back to the same state as before suspend.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>>>>>> ---
>>>>>>>>>>      drivers/clk/tegra/clk-tegra210.c | 103 +++++++++++++++++++++++++++++++++++++--
>>>>>>>>>>      drivers/clk/tegra/clk.c          |  64 ++++++++++++++++++++++++
>>>>>>>>>>      drivers/clk/tegra/clk.h          |   3 ++
>>>>>>>>>>      3 files changed, 166 insertions(+), 4 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>>> index 998bf60b219a..8dd6f4f4debb 100644
>>>>>>>>>> --- a/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>>> +++ b/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>>> @@ -9,13 +9,13 @@
>>>>>>>>>>      #include <linux/clkdev.h>
>>>>>>>>>>      #include <linux/of.h>
>>>>>>>>>>      #include <linux/of_address.h>
>>>>>>>>>> +#include <linux/syscore_ops.h>
>>>>>>>>>>      #include <linux/delay.h>
>>>>>>>>>>      #include <linux/export.h>
>>>>>>>>>>      #include <linux/mutex.h>
>>>>>>>>>>      #include <linux/clk/tegra.h>
>>>>>>>>>>      #include <dt-bindings/clock/tegra210-car.h>
>>>>>>>>>>      #include <dt-bindings/reset/tegra210-car.h>
>>>>>>>>>> -#include <linux/iopoll.h>
>>>>>>>>>>      #include <linux/sizes.h>
>>>>>>>>>>      #include <soc/tegra/pmc.h>
>>>>>>>>>>      @@ -220,11 +220,15 @@
>>>>>>>>>>      #define CLK_M_DIVISOR_SHIFT 2
>>>>>>>>>>      #define CLK_M_DIVISOR_MASK 0x3
>>>>>>>>>>      +#define CLK_MASK_ARM    0x44
>>>>>>>>>> +#define MISC_CLK_ENB    0x48
>>>>>>>>>> +
>>>>>>>>>>      #define RST_DFLL_DVCO 0x2f4
>>>>>>>>>>      #define DVFS_DFLL_RESET_SHIFT 0
>>>>>>>>>>        #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
>>>>>>>>>>      #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
>>>>>>>>>> +#define CPU_SOFTRST_CTRL 0x380
>>>>>>>>>>        #define LVL2_CLK_GATE_OVRA 0xf8
>>>>>>>>>>      #define LVL2_CLK_GATE_OVRC 0x3a0
>>>>>>>>>> @@ -2825,6 +2829,7 @@ static int tegra210_enable_pllu(void)
>>>>>>>>>>          struct tegra_clk_pll_freq_table *fentry;
>>>>>>>>>>          struct tegra_clk_pll pllu;
>>>>>>>>>>          u32 reg;
>>>>>>>>>> +    int ret;
>>>>>>>>>>            for (fentry = pll_u_freq_table; fentry->input_rate; fentry++) {
>>>>>>>>>>              if (fentry->input_rate == pll_ref_freq)
>>>>>>>>>> @@ -2853,9 +2858,14 @@ static int tegra210_enable_pllu(void)
>>>>>>>>>>          reg |= PLL_ENABLE;
>>>>>>>>>>          writel(reg, clk_base + PLLU_BASE);
>>>>>>>>>>      -    readl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,
>>>>>>>>>> -                      reg & PLL_BASE_LOCK, 2, 1000);
>>>>>>>>>> -    if (!(reg & PLL_BASE_LOCK)) {
>>>>>>>>>> +    /*
>>>>>>>>>> +     * During clocks resume, same PLLU init and enable sequence get
>>>>>>>>>> +     * executed. So, readx_poll_timeout_atomic can't be used here as it
>>>>>>>>>> +     * uses ktime_get() and timekeeping resume doesn't happen by that
>>>>>>>>>> +     * time. So, using tegra210_wait_for_mask for PLL LOCK.
>>>>>>>>>> +     */
>>>>>>>>>> +    ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
>>>>>>>>>> +    if (ret) {
>>>>>>>>>>              pr_err("Timed out waiting for PLL_U to lock\n");
>>>>>>>>>>              return -ETIMEDOUT;
>>>>>>>>>>          }
>>>>>>>>>> @@ -3288,6 +3298,84 @@ static void tegra210_disable_cpu_clock(u32 cpu)
>>>>>>>>>>      }
>>>>>>>>>>        #ifdef CONFIG_PM_SLEEP
>>>>>>>>>> +/*
>>>>>>>>>> + * This array lists mask values for each peripheral clk bank
>>>>>>>>>> + * to mask out reserved bits during the clocks state restore
>>>>>>>>>> + * on SC7 resume to prevent accidental writes to these reserved
>>>>>>>>>> + * bits.
>>>>>>>>>> + */
>>>>>>>>>> +static u32 periph_clk_rsvd_mask[TEGRA210_CAR_BANK_COUNT] = {
>>>>>>>>> Should be more natural to have a "valid_mask" instead of "rsvd_mask".
>>>>>>>>>
>>>>>>>>> What's actually wrong with touching of the reserved bits? They must be NO-OP.. or the
>>>>>>>>> reserved bits are actually some kind of "secret" bits? If those bits have some use-case
>>>>>>>>> outside of Silicon HW (like FPGA simulation), then this doesn't matter for upstream
>>>>>>>>> and you
>>>>>>>>> have to keep the workaround locally in the downstream kernel or whatever.
>>>>>>>> Will rename as valid_mask.
>>>>>>>>
>>>>>>>> some bits in these registers are undefined and is not good to write to these bits as
>>>>>>>> they
>>>>>>>> can cause pslverr.
>>>>>>> Okay, it should be explained in the comment.
>>>>>>>
>>>>>>> Is it possible to disable trapping of changing the undefined bits?
>>>>>> No its internal to design
>>>>> Okay.
>>>>>
>>>>> Also, what about to move the valid_mask into struct tegra_clk_periph_regs?
>>>> No, we cannot move to tegra_clk_periph_regs as its in tegra/clk.c and is common for all
>>>> tegra.
>>>>
>>>> Reserved bits are different on tegra chips so should come from Tegra chip specific clock
>>>> driver like
>>>>
>>>> clk-tegra210 for Tegra210.
>>> Could you please check whether the reserved bits are RAZ (read as zero)?
>>>
>>> [snip]
>> yes all reserved bits of clk_enb register is 0. This should not be set to 1.
>>
>> As I will be changing to variable name to valid_mask instead of reserved mask, will also
>> change values to valid mask so it can be used directly to write to clk_enb for enabling all
>> peripherals clks.
>>
> It looks to me that the tegra_clk_periph_force_on() could be made local to the
> clk-tegra210.c and then the raw clk_enb values could be written directly instead of having
> the clk_enb[] array, probably that will be a bit cleaner

All CLK_OUT_ENB* registers are already defined in clk driver and also 
periph_regs includes all of these to use.

To write value to enable all clocks directly without array, it need 
total 7 individual register writes for Tegra210. Also when 
suspend/resume is implemented for other prior tegras, they need to do 
same in tegra clock driver.

Reason I had this in clock driver is, this can be used by all tegra 
clock drivers and just can pass valid clocks values.

But doing individual register write with direct hard code values in 
corresponding tegra clock driver is preferred still, will update so in 
next revision and will move all the CLK_OUT_ENB* register defines to 
tegra/clk.h

Currently RST_DEVICES & CLK_OUT_ENB are all in tegra/clk.c

> .

^ permalink raw reply

* Re: [PATCH v8 14/21] clk: tegra210: Add suspend and resume support
From: Dmitry Osipenko @ 2019-08-12 18:19 UTC (permalink / raw)
  To: Sowjanya Komatineni
  Cc: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
	linus.walleij, stefan, mark.rutland, pdeschrijver, pgaikwad,
	sboyd, linux-clk, linux-gpio, jckuo, josephl, talho, linux-tegra,
	linux-kernel, mperttunen, spatra, robh+dt, devicetree, rjw,
	viresh.kumar, linux-pm
In-Reply-To: <931b027d-fdf3-220b-167a-4177fa917781@nvidia.com>

12.08.2019 20:28, Sowjanya Komatineni пишет:
> 
> On 8/12/19 9:25 AM, Dmitry Osipenko wrote:
>> 11.08.2019 22:15, Sowjanya Komatineni пишет:
>>> On 8/11/19 10:39 AM, Dmitry Osipenko wrote:
>>>> 09.08.2019 21:40, Sowjanya Komatineni пишет:
>>>>> On 8/9/19 11:18 AM, Dmitry Osipenko wrote:
>>>>>> 09.08.2019 19:19, Sowjanya Komatineni пишет:
>>>>>>> On 8/9/19 6:56 AM, Dmitry Osipenko wrote:
>>>>>>>> 09.08.2019 2:46, Sowjanya Komatineni пишет:
>>>>>>>>> This patch adds support for clk: tegra210: suspend-resume.
>>>>>>>>>
>>>>>>>>> All the CAR controller settings are lost on suspend when core
>>>>>>>>> power goes off.
>>>>>>>>>
>>>>>>>>> This patch has implementation for saving and restoring all PLLs
>>>>>>>>> and clocks context during system suspend and resume to have the
>>>>>>>>> clocks back to same state for normal operation.
>>>>>>>>>
>>>>>>>>> Clock driver suspend and resume are registered as syscore_ops as clocks
>>>>>>>>> restore need to happen before the other drivers resume to have all their
>>>>>>>>> clocks back to the same state as before suspend.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>>>>> ---
>>>>>>>>>     drivers/clk/tegra/clk-tegra210.c | 103 +++++++++++++++++++++++++++++++++++++--
>>>>>>>>>     drivers/clk/tegra/clk.c          |  64 ++++++++++++++++++++++++
>>>>>>>>>     drivers/clk/tegra/clk.h          |   3 ++
>>>>>>>>>     3 files changed, 166 insertions(+), 4 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>> index 998bf60b219a..8dd6f4f4debb 100644
>>>>>>>>> --- a/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>> +++ b/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>> @@ -9,13 +9,13 @@
>>>>>>>>>     #include <linux/clkdev.h>
>>>>>>>>>     #include <linux/of.h>
>>>>>>>>>     #include <linux/of_address.h>
>>>>>>>>> +#include <linux/syscore_ops.h>
>>>>>>>>>     #include <linux/delay.h>
>>>>>>>>>     #include <linux/export.h>
>>>>>>>>>     #include <linux/mutex.h>
>>>>>>>>>     #include <linux/clk/tegra.h>
>>>>>>>>>     #include <dt-bindings/clock/tegra210-car.h>
>>>>>>>>>     #include <dt-bindings/reset/tegra210-car.h>
>>>>>>>>> -#include <linux/iopoll.h>
>>>>>>>>>     #include <linux/sizes.h>
>>>>>>>>>     #include <soc/tegra/pmc.h>
>>>>>>>>>     @@ -220,11 +220,15 @@
>>>>>>>>>     #define CLK_M_DIVISOR_SHIFT 2
>>>>>>>>>     #define CLK_M_DIVISOR_MASK 0x3
>>>>>>>>>     +#define CLK_MASK_ARM    0x44
>>>>>>>>> +#define MISC_CLK_ENB    0x48
>>>>>>>>> +
>>>>>>>>>     #define RST_DFLL_DVCO 0x2f4
>>>>>>>>>     #define DVFS_DFLL_RESET_SHIFT 0
>>>>>>>>>       #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
>>>>>>>>>     #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
>>>>>>>>> +#define CPU_SOFTRST_CTRL 0x380
>>>>>>>>>       #define LVL2_CLK_GATE_OVRA 0xf8
>>>>>>>>>     #define LVL2_CLK_GATE_OVRC 0x3a0
>>>>>>>>> @@ -2825,6 +2829,7 @@ static int tegra210_enable_pllu(void)
>>>>>>>>>         struct tegra_clk_pll_freq_table *fentry;
>>>>>>>>>         struct tegra_clk_pll pllu;
>>>>>>>>>         u32 reg;
>>>>>>>>> +    int ret;
>>>>>>>>>           for (fentry = pll_u_freq_table; fentry->input_rate; fentry++) {
>>>>>>>>>             if (fentry->input_rate == pll_ref_freq)
>>>>>>>>> @@ -2853,9 +2858,14 @@ static int tegra210_enable_pllu(void)
>>>>>>>>>         reg |= PLL_ENABLE;
>>>>>>>>>         writel(reg, clk_base + PLLU_BASE);
>>>>>>>>>     -    readl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,
>>>>>>>>> -                      reg & PLL_BASE_LOCK, 2, 1000);
>>>>>>>>> -    if (!(reg & PLL_BASE_LOCK)) {
>>>>>>>>> +    /*
>>>>>>>>> +     * During clocks resume, same PLLU init and enable sequence get
>>>>>>>>> +     * executed. So, readx_poll_timeout_atomic can't be used here as it
>>>>>>>>> +     * uses ktime_get() and timekeeping resume doesn't happen by that
>>>>>>>>> +     * time. So, using tegra210_wait_for_mask for PLL LOCK.
>>>>>>>>> +     */
>>>>>>>>> +    ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
>>>>>>>>> +    if (ret) {
>>>>>>>>>             pr_err("Timed out waiting for PLL_U to lock\n");
>>>>>>>>>             return -ETIMEDOUT;
>>>>>>>>>         }
>>>>>>>>> @@ -3288,6 +3298,84 @@ static void tegra210_disable_cpu_clock(u32 cpu)
>>>>>>>>>     }
>>>>>>>>>       #ifdef CONFIG_PM_SLEEP
>>>>>>>>> +/*
>>>>>>>>> + * This array lists mask values for each peripheral clk bank
>>>>>>>>> + * to mask out reserved bits during the clocks state restore
>>>>>>>>> + * on SC7 resume to prevent accidental writes to these reserved
>>>>>>>>> + * bits.
>>>>>>>>> + */
>>>>>>>>> +static u32 periph_clk_rsvd_mask[TEGRA210_CAR_BANK_COUNT] = {
>>>>>>>> Should be more natural to have a "valid_mask" instead of "rsvd_mask".
>>>>>>>>
>>>>>>>> What's actually wrong with touching of the reserved bits? They must be NO-OP.. or the
>>>>>>>> reserved bits are actually some kind of "secret" bits? If those bits have some use-case
>>>>>>>> outside of Silicon HW (like FPGA simulation), then this doesn't matter for upstream
>>>>>>>> and you
>>>>>>>> have to keep the workaround locally in the downstream kernel or whatever.
>>>>>>> Will rename as valid_mask.
>>>>>>>
>>>>>>> some bits in these registers are undefined and is not good to write to these bits as
>>>>>>> they
>>>>>>> can cause pslverr.
>>>>>> Okay, it should be explained in the comment.
>>>>>>
>>>>>> Is it possible to disable trapping of changing the undefined bits?
>>>>> No its internal to design
>>>> Okay.
>>>>
>>>> Also, what about to move the valid_mask into struct tegra_clk_periph_regs?
>>> No, we cannot move to tegra_clk_periph_regs as its in tegra/clk.c and is common for all
>>> tegra.
>>>
>>> Reserved bits are different on tegra chips so should come from Tegra chip specific clock
>>> driver like
>>>
>>> clk-tegra210 for Tegra210.
>> Could you please check whether the reserved bits are RAZ (read as zero)?
>>
>> [snip]
> 
> yes all reserved bits of clk_enb register is 0. This should not be set to 1.
> 
> As I will be changing to variable name to valid_mask instead of reserved mask, will also
> change values to valid mask so it can be used directly to write to clk_enb for enabling all
> peripherals clks.
> 

It looks to me that the tegra_clk_periph_force_on() could be made local to the
clk-tegra210.c and then the raw clk_enb values could be written directly instead of having
the clk_enb[] array, probably that will be a bit cleaner.

^ permalink raw reply

* Re: [PATCH] pinctrl: intel: baytrail: Pass irqchip when adding gpiochip
From: Andy Shevchenko @ 2019-08-12 17:52 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-gpio, Mika Westerberg, Thierry Reding
In-Reply-To: <20190812141446.1090-1-linus.walleij@linaro.org>

On Mon, Aug 12, 2019 at 04:14:46PM +0200, Linus Walleij wrote:
> We need to convert all old gpio irqchips to pass the irqchip
> setup along when adding the gpio_chip. For more info see
> drivers/gpio/TODO.
> 
> For chained irqchips this is a pretty straight-forward
> conversion.

Pushed to my review and testing queue, thanks!

> 
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Andy: when you're happy with this you can either supply an
> ACK and I will merge it or you can merge it into your tree
> for a later pull request, just tell me what you prefer.
> ---
>  drivers/pinctrl/intel/pinctrl-baytrail.c | 38 ++++++++++++++----------
>  1 file changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c b/drivers/pinctrl/intel/pinctrl-baytrail.c
> index e5a112a8e067..57faddca3710 100644
> --- a/drivers/pinctrl/intel/pinctrl-baytrail.c
> +++ b/drivers/pinctrl/intel/pinctrl-baytrail.c
> @@ -1533,6 +1533,28 @@ static int byt_gpio_probe(struct byt_gpio *vg)
>  	if (!vg->saved_context)
>  		return -ENOMEM;
>  #endif
> +
> +	/* set up interrupts  */
> +	irq_rc = platform_get_resource(vg->pdev, IORESOURCE_IRQ, 0);
> +	if (irq_rc && irq_rc->start) {
> +		struct gpio_irq_chip *girq;
> +
> +		byt_gpio_irq_init_hw(vg);
> +
> +		girq = &gc->irq;
> +		girq->chip = &byt_irqchip;
> +		girq->parent_handler = byt_gpio_irq_handler;
> +		girq->num_parents = 1;
> +		girq->parents = devm_kcalloc(&vg->pdev->dev, 1,
> +					     sizeof(*girq->parents),
> +					     GFP_KERNEL);
> +		if (!girq->parents)
> +			return -ENOMEM;
> +		girq->parents[0] = (unsigned int)irq_rc->start;
> +		girq->default_type = IRQ_TYPE_NONE;
> +		girq->handler = handle_bad_irq;
> +	}
> +
>  	ret = devm_gpiochip_add_data(&vg->pdev->dev, gc, vg);
>  	if (ret) {
>  		dev_err(&vg->pdev->dev, "failed adding byt-gpio chip\n");
> @@ -1546,22 +1568,6 @@ static int byt_gpio_probe(struct byt_gpio *vg)
>  		return ret;
>  	}
>  
> -	/* set up interrupts  */
> -	irq_rc = platform_get_resource(vg->pdev, IORESOURCE_IRQ, 0);
> -	if (irq_rc && irq_rc->start) {
> -		byt_gpio_irq_init_hw(vg);
> -		ret = gpiochip_irqchip_add(gc, &byt_irqchip, 0,
> -					   handle_bad_irq, IRQ_TYPE_NONE);
> -		if (ret) {
> -			dev_err(&vg->pdev->dev, "failed to add irqchip\n");
> -			return ret;
> -		}
> -
> -		gpiochip_set_chained_irqchip(gc, &byt_irqchip,
> -					     (unsigned)irq_rc->start,
> -					     byt_gpio_irq_handler);
> -	}
> -
>  	return ret;
>  }
>  
> -- 
> 2.21.0
> 

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v3] pinctrl: intel: Allow to request locked pads
From: Mika Westerberg @ 2019-08-12 17:53 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-gpio, Linus Walleij
In-Reply-To: <20190812161401.87589-1-andriy.shevchenko@linux.intel.com>

On Mon, Aug 12, 2019 at 07:14:01PM +0300, Andy Shevchenko wrote:
> Some firmwares would like to protect pads from being modified by OS
> and at the same time provide them to OS as a resource. So, the driver
> in such circumstances may request pad and may not change its state.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

^ permalink raw reply

* Re: [PATCH] gpio: merrifield: Pass irqchip when adding gpiochip
From: Andy Shevchenko @ 2019-08-12 17:38 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-gpio, Bartosz Golaszewski, Mika Westerberg, David Cohen,
	Thierry Reding
In-Reply-To: <20190812082331.22674-1-linus.walleij@linaro.org>

On Mon, Aug 12, 2019 at 10:23:31AM +0200, Linus Walleij wrote:
> We need to convert all old gpio irqchips to pass the irqchip
> setup along when adding the gpio_chip. For more info see
> drivers/gpio/TODO.
> 
> For chained irqchips this is a pretty straight-forward
> conversion.

All three Intel GPIO changes with kcalloc() fix I was commenting about
pushed to my review and testing queue. Thanks!

> 
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: David Cohen <david.a.cohen@linux.intel.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Andy: when you're happy with this you can either supply an
> ACK and I will merge it or you can merge it into your tree
> for a later pull request, just tell me what you prefer.
> ---
>  drivers/gpio/gpio-merrifield.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-merrifield.c b/drivers/gpio/gpio-merrifield.c
> index 3302125e5265..299277951791 100644
> --- a/drivers/gpio/gpio-merrifield.c
> +++ b/drivers/gpio/gpio-merrifield.c
> @@ -397,6 +397,7 @@ static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id
>  {
>  	const struct mrfld_gpio_pinrange *range;
>  	const char *pinctrl_dev_name;
> +	struct gpio_irq_chip *girq;
>  	struct mrfld_gpio *priv;
>  	u32 gpio_base, irq_base;
>  	void __iomem *base;
> @@ -444,6 +445,21 @@ static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id
>  
>  	raw_spin_lock_init(&priv->lock);
>  
> +	girq = &priv->chip.irq;
> +	girq->chip = &mrfld_irqchip;
> +	girq->parent_handler = mrfld_irq_handler;
> +	girq->num_parents = 1;
> +	girq->parents = devm_kcalloc(&pdev->dev, 1,
> +				     sizeof(*girq->parents),
> +				     GFP_KERNEL);
> +	if (!girq->parents)
> +		return -ENOMEM;
> +	girq->parents[0] = pdev->irq;
> +	girq->default_type = IRQ_TYPE_NONE;
> +	girq->handler = handle_bad_irq;
> +
> +	mrfld_irq_init_hw(priv);
> +
>  	pci_set_drvdata(pdev, priv);
>  	retval = devm_gpiochip_add_data(&pdev->dev, &priv->chip, priv);
>  	if (retval) {
> @@ -465,18 +481,6 @@ static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id
>  		}
>  	}
>  
> -	retval = gpiochip_irqchip_add(&priv->chip, &mrfld_irqchip, irq_base,
> -				      handle_bad_irq, IRQ_TYPE_NONE);
> -	if (retval) {
> -		dev_err(&pdev->dev, "could not connect irqchip to gpiochip\n");
> -		return retval;
> -	}
> -
> -	mrfld_irq_init_hw(priv);
> -
> -	gpiochip_set_chained_irqchip(&priv->chip, &mrfld_irqchip, pdev->irq,
> -				     mrfld_irq_handler);
> -
>  	return 0;
>  }
>  
> -- 
> 2.21.0
> 

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v8 14/21] clk: tegra210: Add suspend and resume support
From: Sowjanya Komatineni @ 2019-08-12 17:28 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
	linus.walleij, stefan, mark.rutland, pdeschrijver, pgaikwad,
	sboyd, linux-clk, linux-gpio, jckuo, josephl, talho, linux-tegra,
	linux-kernel, mperttunen, spatra, robh+dt, devicetree, rjw,
	viresh.kumar, linux-pm
In-Reply-To: <ca90bd2b-8088-8b46-2816-95e58a4811b8@gmail.com>


On 8/12/19 9:25 AM, Dmitry Osipenko wrote:
> 11.08.2019 22:15, Sowjanya Komatineni пишет:
>> On 8/11/19 10:39 AM, Dmitry Osipenko wrote:
>>> 09.08.2019 21:40, Sowjanya Komatineni пишет:
>>>> On 8/9/19 11:18 AM, Dmitry Osipenko wrote:
>>>>> 09.08.2019 19:19, Sowjanya Komatineni пишет:
>>>>>> On 8/9/19 6:56 AM, Dmitry Osipenko wrote:
>>>>>>> 09.08.2019 2:46, Sowjanya Komatineni пишет:
>>>>>>>> This patch adds support for clk: tegra210: suspend-resume.
>>>>>>>>
>>>>>>>> All the CAR controller settings are lost on suspend when core
>>>>>>>> power goes off.
>>>>>>>>
>>>>>>>> This patch has implementation for saving and restoring all PLLs
>>>>>>>> and clocks context during system suspend and resume to have the
>>>>>>>> clocks back to same state for normal operation.
>>>>>>>>
>>>>>>>> Clock driver suspend and resume are registered as syscore_ops as clocks
>>>>>>>> restore need to happen before the other drivers resume to have all their
>>>>>>>> clocks back to the same state as before suspend.
>>>>>>>>
>>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>>>> ---
>>>>>>>>     drivers/clk/tegra/clk-tegra210.c | 103 +++++++++++++++++++++++++++++++++++++--
>>>>>>>>     drivers/clk/tegra/clk.c          |  64 ++++++++++++++++++++++++
>>>>>>>>     drivers/clk/tegra/clk.h          |   3 ++
>>>>>>>>     3 files changed, 166 insertions(+), 4 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
>>>>>>>> index 998bf60b219a..8dd6f4f4debb 100644
>>>>>>>> --- a/drivers/clk/tegra/clk-tegra210.c
>>>>>>>> +++ b/drivers/clk/tegra/clk-tegra210.c
>>>>>>>> @@ -9,13 +9,13 @@
>>>>>>>>     #include <linux/clkdev.h>
>>>>>>>>     #include <linux/of.h>
>>>>>>>>     #include <linux/of_address.h>
>>>>>>>> +#include <linux/syscore_ops.h>
>>>>>>>>     #include <linux/delay.h>
>>>>>>>>     #include <linux/export.h>
>>>>>>>>     #include <linux/mutex.h>
>>>>>>>>     #include <linux/clk/tegra.h>
>>>>>>>>     #include <dt-bindings/clock/tegra210-car.h>
>>>>>>>>     #include <dt-bindings/reset/tegra210-car.h>
>>>>>>>> -#include <linux/iopoll.h>
>>>>>>>>     #include <linux/sizes.h>
>>>>>>>>     #include <soc/tegra/pmc.h>
>>>>>>>>     @@ -220,11 +220,15 @@
>>>>>>>>     #define CLK_M_DIVISOR_SHIFT 2
>>>>>>>>     #define CLK_M_DIVISOR_MASK 0x3
>>>>>>>>     +#define CLK_MASK_ARM    0x44
>>>>>>>> +#define MISC_CLK_ENB    0x48
>>>>>>>> +
>>>>>>>>     #define RST_DFLL_DVCO 0x2f4
>>>>>>>>     #define DVFS_DFLL_RESET_SHIFT 0
>>>>>>>>       #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
>>>>>>>>     #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
>>>>>>>> +#define CPU_SOFTRST_CTRL 0x380
>>>>>>>>       #define LVL2_CLK_GATE_OVRA 0xf8
>>>>>>>>     #define LVL2_CLK_GATE_OVRC 0x3a0
>>>>>>>> @@ -2825,6 +2829,7 @@ static int tegra210_enable_pllu(void)
>>>>>>>>         struct tegra_clk_pll_freq_table *fentry;
>>>>>>>>         struct tegra_clk_pll pllu;
>>>>>>>>         u32 reg;
>>>>>>>> +    int ret;
>>>>>>>>           for (fentry = pll_u_freq_table; fentry->input_rate; fentry++) {
>>>>>>>>             if (fentry->input_rate == pll_ref_freq)
>>>>>>>> @@ -2853,9 +2858,14 @@ static int tegra210_enable_pllu(void)
>>>>>>>>         reg |= PLL_ENABLE;
>>>>>>>>         writel(reg, clk_base + PLLU_BASE);
>>>>>>>>     -    readl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,
>>>>>>>> -                      reg & PLL_BASE_LOCK, 2, 1000);
>>>>>>>> -    if (!(reg & PLL_BASE_LOCK)) {
>>>>>>>> +    /*
>>>>>>>> +     * During clocks resume, same PLLU init and enable sequence get
>>>>>>>> +     * executed. So, readx_poll_timeout_atomic can't be used here as it
>>>>>>>> +     * uses ktime_get() and timekeeping resume doesn't happen by that
>>>>>>>> +     * time. So, using tegra210_wait_for_mask for PLL LOCK.
>>>>>>>> +     */
>>>>>>>> +    ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
>>>>>>>> +    if (ret) {
>>>>>>>>             pr_err("Timed out waiting for PLL_U to lock\n");
>>>>>>>>             return -ETIMEDOUT;
>>>>>>>>         }
>>>>>>>> @@ -3288,6 +3298,84 @@ static void tegra210_disable_cpu_clock(u32 cpu)
>>>>>>>>     }
>>>>>>>>       #ifdef CONFIG_PM_SLEEP
>>>>>>>> +/*
>>>>>>>> + * This array lists mask values for each peripheral clk bank
>>>>>>>> + * to mask out reserved bits during the clocks state restore
>>>>>>>> + * on SC7 resume to prevent accidental writes to these reserved
>>>>>>>> + * bits.
>>>>>>>> + */
>>>>>>>> +static u32 periph_clk_rsvd_mask[TEGRA210_CAR_BANK_COUNT] = {
>>>>>>> Should be more natural to have a "valid_mask" instead of "rsvd_mask".
>>>>>>>
>>>>>>> What's actually wrong with touching of the reserved bits? They must be NO-OP.. or the
>>>>>>> reserved bits are actually some kind of "secret" bits? If those bits have some use-case
>>>>>>> outside of Silicon HW (like FPGA simulation), then this doesn't matter for upstream
>>>>>>> and you
>>>>>>> have to keep the workaround locally in the downstream kernel or whatever.
>>>>>> Will rename as valid_mask.
>>>>>>
>>>>>> some bits in these registers are undefined and is not good to write to these bits as they
>>>>>> can cause pslverr.
>>>>> Okay, it should be explained in the comment.
>>>>>
>>>>> Is it possible to disable trapping of changing the undefined bits?
>>>> No its internal to design
>>> Okay.
>>>
>>> Also, what about to move the valid_mask into struct tegra_clk_periph_regs?
>> No, we cannot move to tegra_clk_periph_regs as its in tegra/clk.c and is common for all tegra.
>>
>> Reserved bits are different on tegra chips so should come from Tegra chip specific clock
>> driver like
>>
>> clk-tegra210 for Tegra210.
> Could you please check whether the reserved bits are RAZ (read as zero)?
>
> [snip]

yes all reserved bits of clk_enb register is 0. This should not be set to 1.

As I will be changing to variable name to valid_mask instead of reserved 
mask, will also change values to valid mask so it can be used directly 
to write to clk_enb for enabling all peripherals clks.


^ permalink raw reply

* Re: [PATCH v8 14/21] clk: tegra210: Add suspend and resume support
From: Dmitry Osipenko @ 2019-08-12 16:25 UTC (permalink / raw)
  To: Sowjanya Komatineni
  Cc: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
	linus.walleij, stefan, mark.rutland, pdeschrijver, pgaikwad,
	sboyd, linux-clk, linux-gpio, jckuo, josephl, talho, linux-tegra,
	linux-kernel, mperttunen, spatra, robh+dt, devicetree, rjw,
	viresh.kumar, linux-pm
In-Reply-To: <805a825e-f19d-d056-83eb-8ed1cb1c089c@nvidia.com>

11.08.2019 22:15, Sowjanya Komatineni пишет:
> 
> On 8/11/19 10:39 AM, Dmitry Osipenko wrote:
>> 09.08.2019 21:40, Sowjanya Komatineni пишет:
>>> On 8/9/19 11:18 AM, Dmitry Osipenko wrote:
>>>> 09.08.2019 19:19, Sowjanya Komatineni пишет:
>>>>> On 8/9/19 6:56 AM, Dmitry Osipenko wrote:
>>>>>> 09.08.2019 2:46, Sowjanya Komatineni пишет:
>>>>>>> This patch adds support for clk: tegra210: suspend-resume.
>>>>>>>
>>>>>>> All the CAR controller settings are lost on suspend when core
>>>>>>> power goes off.
>>>>>>>
>>>>>>> This patch has implementation for saving and restoring all PLLs
>>>>>>> and clocks context during system suspend and resume to have the
>>>>>>> clocks back to same state for normal operation.
>>>>>>>
>>>>>>> Clock driver suspend and resume are registered as syscore_ops as clocks
>>>>>>> restore need to happen before the other drivers resume to have all their
>>>>>>> clocks back to the same state as before suspend.
>>>>>>>
>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>>> ---
>>>>>>>    drivers/clk/tegra/clk-tegra210.c | 103 +++++++++++++++++++++++++++++++++++++--
>>>>>>>    drivers/clk/tegra/clk.c          |  64 ++++++++++++++++++++++++
>>>>>>>    drivers/clk/tegra/clk.h          |   3 ++
>>>>>>>    3 files changed, 166 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
>>>>>>> index 998bf60b219a..8dd6f4f4debb 100644
>>>>>>> --- a/drivers/clk/tegra/clk-tegra210.c
>>>>>>> +++ b/drivers/clk/tegra/clk-tegra210.c
>>>>>>> @@ -9,13 +9,13 @@
>>>>>>>    #include <linux/clkdev.h>
>>>>>>>    #include <linux/of.h>
>>>>>>>    #include <linux/of_address.h>
>>>>>>> +#include <linux/syscore_ops.h>
>>>>>>>    #include <linux/delay.h>
>>>>>>>    #include <linux/export.h>
>>>>>>>    #include <linux/mutex.h>
>>>>>>>    #include <linux/clk/tegra.h>
>>>>>>>    #include <dt-bindings/clock/tegra210-car.h>
>>>>>>>    #include <dt-bindings/reset/tegra210-car.h>
>>>>>>> -#include <linux/iopoll.h>
>>>>>>>    #include <linux/sizes.h>
>>>>>>>    #include <soc/tegra/pmc.h>
>>>>>>>    @@ -220,11 +220,15 @@
>>>>>>>    #define CLK_M_DIVISOR_SHIFT 2
>>>>>>>    #define CLK_M_DIVISOR_MASK 0x3
>>>>>>>    +#define CLK_MASK_ARM    0x44
>>>>>>> +#define MISC_CLK_ENB    0x48
>>>>>>> +
>>>>>>>    #define RST_DFLL_DVCO 0x2f4
>>>>>>>    #define DVFS_DFLL_RESET_SHIFT 0
>>>>>>>      #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
>>>>>>>    #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
>>>>>>> +#define CPU_SOFTRST_CTRL 0x380
>>>>>>>      #define LVL2_CLK_GATE_OVRA 0xf8
>>>>>>>    #define LVL2_CLK_GATE_OVRC 0x3a0
>>>>>>> @@ -2825,6 +2829,7 @@ static int tegra210_enable_pllu(void)
>>>>>>>        struct tegra_clk_pll_freq_table *fentry;
>>>>>>>        struct tegra_clk_pll pllu;
>>>>>>>        u32 reg;
>>>>>>> +    int ret;
>>>>>>>          for (fentry = pll_u_freq_table; fentry->input_rate; fentry++) {
>>>>>>>            if (fentry->input_rate == pll_ref_freq)
>>>>>>> @@ -2853,9 +2858,14 @@ static int tegra210_enable_pllu(void)
>>>>>>>        reg |= PLL_ENABLE;
>>>>>>>        writel(reg, clk_base + PLLU_BASE);
>>>>>>>    -    readl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,
>>>>>>> -                      reg & PLL_BASE_LOCK, 2, 1000);
>>>>>>> -    if (!(reg & PLL_BASE_LOCK)) {
>>>>>>> +    /*
>>>>>>> +     * During clocks resume, same PLLU init and enable sequence get
>>>>>>> +     * executed. So, readx_poll_timeout_atomic can't be used here as it
>>>>>>> +     * uses ktime_get() and timekeeping resume doesn't happen by that
>>>>>>> +     * time. So, using tegra210_wait_for_mask for PLL LOCK.
>>>>>>> +     */
>>>>>>> +    ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
>>>>>>> +    if (ret) {
>>>>>>>            pr_err("Timed out waiting for PLL_U to lock\n");
>>>>>>>            return -ETIMEDOUT;
>>>>>>>        }
>>>>>>> @@ -3288,6 +3298,84 @@ static void tegra210_disable_cpu_clock(u32 cpu)
>>>>>>>    }
>>>>>>>      #ifdef CONFIG_PM_SLEEP
>>>>>>> +/*
>>>>>>> + * This array lists mask values for each peripheral clk bank
>>>>>>> + * to mask out reserved bits during the clocks state restore
>>>>>>> + * on SC7 resume to prevent accidental writes to these reserved
>>>>>>> + * bits.
>>>>>>> + */
>>>>>>> +static u32 periph_clk_rsvd_mask[TEGRA210_CAR_BANK_COUNT] = {
>>>>>> Should be more natural to have a "valid_mask" instead of "rsvd_mask".
>>>>>>
>>>>>> What's actually wrong with touching of the reserved bits? They must be NO-OP.. or the
>>>>>> reserved bits are actually some kind of "secret" bits? If those bits have some use-case
>>>>>> outside of Silicon HW (like FPGA simulation), then this doesn't matter for upstream
>>>>>> and you
>>>>>> have to keep the workaround locally in the downstream kernel or whatever.
>>>>> Will rename as valid_mask.
>>>>>
>>>>> some bits in these registers are undefined and is not good to write to these bits as they
>>>>> can cause pslverr.
>>>> Okay, it should be explained in the comment.
>>>>
>>>> Is it possible to disable trapping of changing the undefined bits?
>>> No its internal to design
>> Okay.
>>
>> Also, what about to move the valid_mask into struct tegra_clk_periph_regs?
> 
> No, we cannot move to tegra_clk_periph_regs as its in tegra/clk.c and is common for all tegra.
> 
> Reserved bits are different on tegra chips so should come from Tegra chip specific clock
> driver like
> 
> clk-tegra210 for Tegra210.

Could you please check whether the reserved bits are RAZ (read as zero)?

[snip]

^ permalink raw reply

* [PATCH v3] pinctrl: intel: Allow to request locked pads
From: Andy Shevchenko @ 2019-08-12 16:14 UTC (permalink / raw)
  To: Mika Westerberg, linux-gpio, Linus Walleij; +Cc: Andy Shevchenko

Some firmwares would like to protect pads from being modified by OS
and at the same time provide them to OS as a resource. So, the driver
in such circumstances may request pad and may not change its state.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
v3: Describe enum in kernel-doc
    (note a warning which is fixed in a separate patch against kernel-doc)
    Add Linus' Rb tag
 drivers/pinctrl/intel/pinctrl-intel.c | 69 ++++++++++++++++++++-------
 1 file changed, 52 insertions(+), 17 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index c949df07cbdf..104dfaa78cb8 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -220,47 +220,71 @@ static bool intel_pad_acpi_mode(struct intel_pinctrl *pctrl, unsigned int pin)
 	return !(readl(hostown) & BIT(gpp_offset));
 }
 
-static bool intel_pad_locked(struct intel_pinctrl *pctrl, unsigned int pin)
+/**
+ * enum - Locking variants of the pad configuration
+ *
+ * @PAD_UNLOCKED:	pad is fully controlled by the configuration registers
+ * @PAD_LOCKED:		pad configuration registers, except TX state, are locked
+ * @PAD_LOCKED_TX:	pad configuration TX state is locked
+ * @PAD_LOCKED_FULL:	pad configuration registers are locked completely
+ *
+ * Locking is considered as read-only mode for corresponding registers and
+ * their respective fields. That said, TX state bit is locked separately from
+ * the main locking scheme.
+ */
+enum {
+	PAD_UNLOCKED	= 0,
+	PAD_LOCKED	= 1,
+	PAD_LOCKED_TX	= 2,
+	PAD_LOCKED_FULL	= PAD_LOCKED | PAD_LOCKED_TX,
+};
+
+static int intel_pad_locked(struct intel_pinctrl *pctrl, unsigned int pin)
 {
 	struct intel_community *community;
 	const struct intel_padgroup *padgrp;
 	unsigned int offset, gpp_offset;
 	u32 value;
+	int ret = PAD_UNLOCKED;
 
 	community = intel_get_community(pctrl, pin);
 	if (!community)
-		return true;
+		return PAD_LOCKED_FULL;
 	if (!community->padcfglock_offset)
-		return false;
+		return PAD_UNLOCKED;
 
 	padgrp = intel_community_get_padgroup(community, pin);
 	if (!padgrp)
-		return true;
+		return PAD_LOCKED_FULL;
 
 	gpp_offset = padgroup_offset(padgrp, pin);
 
 	/*
 	 * If PADCFGLOCK and PADCFGLOCKTX bits are both clear for this pad,
 	 * the pad is considered unlocked. Any other case means that it is
-	 * either fully or partially locked and we don't touch it.
+	 * either fully or partially locked.
 	 */
-	offset = community->padcfglock_offset + padgrp->reg_num * 8;
+	offset = community->padcfglock_offset + 0 + padgrp->reg_num * 8;
 	value = readl(community->regs + offset);
 	if (value & BIT(gpp_offset))
-		return true;
+		ret |= PAD_LOCKED;
 
 	offset = community->padcfglock_offset + 4 + padgrp->reg_num * 8;
 	value = readl(community->regs + offset);
 	if (value & BIT(gpp_offset))
-		return true;
+		ret |= PAD_LOCKED_TX;
 
-	return false;
+	return ret;
+}
+
+static bool intel_pad_is_unlocked(struct intel_pinctrl *pctrl, unsigned int pin)
+{
+	return (intel_pad_locked(pctrl, pin) & PAD_LOCKED) == PAD_UNLOCKED;
 }
 
 static bool intel_pad_usable(struct intel_pinctrl *pctrl, unsigned int pin)
 {
-	return intel_pad_owned_by_host(pctrl, pin) &&
-		!intel_pad_locked(pctrl, pin);
+	return intel_pad_owned_by_host(pctrl, pin) && intel_pad_is_unlocked(pctrl, pin);
 }
 
 static int intel_get_groups_count(struct pinctrl_dev *pctldev)
@@ -294,7 +318,8 @@ static void intel_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
 	struct intel_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
 	void __iomem *padcfg;
 	u32 cfg0, cfg1, mode;
-	bool locked, acpi;
+	int locked;
+	bool acpi;
 
 	if (!intel_pad_owned_by_host(pctrl, pin)) {
 		seq_puts(s, "not available");
@@ -322,11 +347,16 @@ static void intel_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
 
 	if (locked || acpi) {
 		seq_puts(s, " [");
-		if (locked) {
+		if (locked)
 			seq_puts(s, "LOCKED");
-			if (acpi)
-				seq_puts(s, ", ");
-		}
+		if ((locked & PAD_LOCKED_FULL) == PAD_LOCKED_TX)
+			seq_puts(s, " tx");
+		else if ((locked & PAD_LOCKED_FULL) == PAD_LOCKED_FULL)
+			seq_puts(s, " full");
+
+		if (locked && acpi)
+			seq_puts(s, ", ");
+
 		if (acpi)
 			seq_puts(s, "ACPI");
 		seq_puts(s, "]");
@@ -448,11 +478,16 @@ static int intel_gpio_request_enable(struct pinctrl_dev *pctldev,
 
 	raw_spin_lock_irqsave(&pctrl->lock, flags);
 
-	if (!intel_pad_usable(pctrl, pin)) {
+	if (!intel_pad_owned_by_host(pctrl, pin)) {
 		raw_spin_unlock_irqrestore(&pctrl->lock, flags);
 		return -EBUSY;
 	}
 
+	if (!intel_pad_is_unlocked(pctrl, pin)) {
+		raw_spin_unlock_irqrestore(&pctrl->lock, flags);
+		return 0;
+	}
+
 	padcfg0 = intel_get_padcfg(pctrl, pin, PADCFG0);
 	intel_gpio_set_gpio_mode(padcfg0);
 	/* Disable TX buffer and enable RX (this will be input) */
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH] pinctrl: bcm-iproc: Pass irqchip when adding gpiochip
From: Scott Branden @ 2019-08-12 16:00 UTC (permalink / raw)
  To: Linus Walleij, linux-gpio
  Cc: Bartosz Golaszewski, Pramod Kumar, Ray Jui, Scott Branden,
	Thierry Reding
In-Reply-To: <20190812132554.18313-1-linus.walleij@linaro.org>


On 2019-08-12 6:25 a.m., Linus Walleij wrote:
> We need to convert all old gpio irqchips to pass the irqchip
> setup along when adding the gpio_chip. For more info see
> drivers/gpio/TODO.
>
> For chained irqchips this is a pretty straight-forward
> conversion.
>
> Cc: Pramod Kumar <pramodku@broadcom.com>
> Cc: Ray Jui <rjui@broadcom.com>
> Cc: Scott Branden <sbranden@broadcom.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Scott Branden <scott.branden@broadcom.com>
> ---
>   drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 33 ++++++++++++++----------
>   1 file changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
> index 18ff01727e0e..ee01306c62fa 100644
> --- a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
> +++ b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
> @@ -780,6 +780,25 @@ static int iproc_gpio_probe(struct platform_device *pdev)
>   	chip->pinmux_is_supported = of_property_read_bool(dev->of_node,
>   							"gpio-ranges");
>   
> +	/* optional GPIO interrupt support */
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq) {
> +		struct gpio_irq_chip *girq;
> +
> +		girq = &gc->irq;
> +		girq->chip = &iproc_gpio_irq_chip;
> +		girq->parent_handler = iproc_gpio_irq_handler;
> +		girq->num_parents = 1;
> +		girq->parents = devm_kcalloc(dev, 1,
> +					     sizeof(*girq->parents),
> +					     GFP_KERNEL);
> +		if (!girq->parents)
> +			return -ENOMEM;
> +		girq->parents[0] = irq;
> +		girq->default_type = IRQ_TYPE_NONE;
> +		girq->handler = handle_simple_irq;
> +	}
> +
>   	ret = gpiochip_add_data(gc, chip);
>   	if (ret < 0) {
>   		dev_err(dev, "unable to add GPIO chip\n");
> @@ -804,20 +823,6 @@ static int iproc_gpio_probe(struct platform_device *pdev)
>   		}
>   	}
>   
> -	/* optional GPIO interrupt support */
> -	irq = platform_get_irq(pdev, 0);
> -	if (irq) {
> -		ret = gpiochip_irqchip_add(gc, &iproc_gpio_irq_chip, 0,
> -					   handle_simple_irq, IRQ_TYPE_NONE);
> -		if (ret) {
> -			dev_err(dev, "no GPIO irqchip\n");
> -			goto err_rm_gpiochip;
> -		}
> -
> -		gpiochip_set_chained_irqchip(gc, &iproc_gpio_irq_chip, irq,
> -					     iproc_gpio_irq_handler);
> -	}
> -
>   	return 0;
>   
>   err_rm_gpiochip:

^ permalink raw reply

* Re: [PATCH] pinctrl: bcm-iproc: Use SPDX header
From: Scott Branden @ 2019-08-12 15:59 UTC (permalink / raw)
  To: Linus Walleij, linux-gpio; +Cc: Pramod Kumar, Ray Jui, Scott Branden
In-Reply-To: <20190812130401.22769-1-linus.walleij@linaro.org>

Hi Linus,

On 2019-08-12 6:04 a.m., Linus Walleij wrote:
> This convert the BCM IPROC driver to use the SPDX header
> for indicating GPL v2.0 only licensing.
>
> Cc: Pramod Kumar <pramodku@broadcom.com>
> Cc: Ray Jui <rjui@broadcom.com>
> Cc: Scott Branden <sbranden@broadcom.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>   drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 12 +-----------
>   1 file changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
> index b70058caee50..18ff01727e0e 100644
> --- a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
> +++ b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
> @@ -1,17 +1,7 @@
> +// SPDX-License-Identifier: GPL-2.0-only
>   /*
>    * Copyright (C) 2014-2017 Broadcom
>    *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License as
> - * published by the Free Software Foundation version 2.
> - *
> - * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> - * kind, whether express or implied; without even the implied warranty
> - * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
> - */
> -
> -/*

Please leave the file description comment separate from the license 
header/copyright notices.

ie. leave the above 3 lines intact.

>    * This file contains the Broadcom Iproc GPIO driver that supports 3
>    * GPIO controllers on Iproc including the ASIU GPIO controller, the
>    * chipCommonG GPIO controller, and the always-on GPIO controller. Basic

^ permalink raw reply

* Re: [PATCH v1 2/3] arm64: dts: fix gpio node
From: Shawn Guo @ 2019-08-12 15:27 UTC (permalink / raw)
  To: Hui Song
  Cc: Li Yang, Rob Herring, Mark Rutland, Linus Walleij,
	Bartosz Golaszewski, linux-arm-kernel, devicetree, linux-kernel,
	linux-gpio
In-Reply-To: <20190808101628.36782-2-hui.song_1@nxp.com>

On Thu, Aug 08, 2019 at 06:16:27PM +0800, Hui Song wrote:
> From: Song Hui <hui.song_1@nxp.com>
> 
> Update the nodes to include little-endian
> property to be consistent with the hardware
> and add ls1088a gpio specify compatible.
> 
> Signed-off-by: Song Hui <hui.song_1@nxp.com>

The prefix should be more specific, like 'arm64: dts: ls1088a: ...'

Shawn

> ---
>  arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> index 20f5ebd..d58d203 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> @@ -269,43 +269,47 @@
>  		};
>  
>  		gpio0: gpio@2300000 {
> -			compatible = "fsl,qoriq-gpio";
> +			compatible = "fsl,ls1088a-gpio", "fsl,qoriq-gpio";
>  			reg = <0x0 0x2300000 0x0 0x10000>;
>  			interrupts = <0 36 IRQ_TYPE_LEVEL_HIGH>;
>  			gpio-controller;
>  			#gpio-cells = <2>;
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
> +			little-endian;
>  		};
>  
>  		gpio1: gpio@2310000 {
> -			compatible = "fsl,qoriq-gpio";
> +			compatible = "fsl,ls1088a-gpio", "fsl,qoriq-gpio";
>  			reg = <0x0 0x2310000 0x0 0x10000>;
>  			interrupts = <0 36 IRQ_TYPE_LEVEL_HIGH>;
>  			gpio-controller;
>  			#gpio-cells = <2>;
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
> +			little-endian;
>  		};
>  
>  		gpio2: gpio@2320000 {
> -			compatible = "fsl,qoriq-gpio";
> +			compatible = "fsl,ls1088a-gpio", "fsl,qoriq-gpio";
>  			reg = <0x0 0x2320000 0x0 0x10000>;
>  			interrupts = <0 37 IRQ_TYPE_LEVEL_HIGH>;
>  			gpio-controller;
>  			#gpio-cells = <2>;
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
> +			little-endian;
>  		};
>  
>  		gpio3: gpio@2330000 {
> -			compatible = "fsl,qoriq-gpio";
> +			compatible = "fsl,ls1088a-gpio", "fsl,qoriq-gpio";
>  			reg = <0x0 0x2330000 0x0 0x10000>;
>  			interrupts = <0 37 IRQ_TYPE_LEVEL_HIGH>;
>  			gpio-controller;
>  			#gpio-cells = <2>;
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
> +			little-endian;
>  		};
>  
>  		ifc: ifc@2240000 {
> -- 
> 2.9.5
> 

^ permalink raw reply

* [PATCH] pinctrl: intel: baytrail: Pass irqchip when adding gpiochip
From: Linus Walleij @ 2019-08-12 14:14 UTC (permalink / raw)
  To: linux-gpio
  Cc: Linus Walleij, Andy Shevchenko, Mika Westerberg, Thierry Reding

We need to convert all old gpio irqchips to pass the irqchip
setup along when adding the gpio_chip. For more info see
drivers/gpio/TODO.

For chained irqchips this is a pretty straight-forward
conversion.

Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Thierry Reding <treding@nvidia.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Andy: when you're happy with this you can either supply an
ACK and I will merge it or you can merge it into your tree
for a later pull request, just tell me what you prefer.
---
 drivers/pinctrl/intel/pinctrl-baytrail.c | 38 ++++++++++++++----------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c b/drivers/pinctrl/intel/pinctrl-baytrail.c
index e5a112a8e067..57faddca3710 100644
--- a/drivers/pinctrl/intel/pinctrl-baytrail.c
+++ b/drivers/pinctrl/intel/pinctrl-baytrail.c
@@ -1533,6 +1533,28 @@ static int byt_gpio_probe(struct byt_gpio *vg)
 	if (!vg->saved_context)
 		return -ENOMEM;
 #endif
+
+	/* set up interrupts  */
+	irq_rc = platform_get_resource(vg->pdev, IORESOURCE_IRQ, 0);
+	if (irq_rc && irq_rc->start) {
+		struct gpio_irq_chip *girq;
+
+		byt_gpio_irq_init_hw(vg);
+
+		girq = &gc->irq;
+		girq->chip = &byt_irqchip;
+		girq->parent_handler = byt_gpio_irq_handler;
+		girq->num_parents = 1;
+		girq->parents = devm_kcalloc(&vg->pdev->dev, 1,
+					     sizeof(*girq->parents),
+					     GFP_KERNEL);
+		if (!girq->parents)
+			return -ENOMEM;
+		girq->parents[0] = (unsigned int)irq_rc->start;
+		girq->default_type = IRQ_TYPE_NONE;
+		girq->handler = handle_bad_irq;
+	}
+
 	ret = devm_gpiochip_add_data(&vg->pdev->dev, gc, vg);
 	if (ret) {
 		dev_err(&vg->pdev->dev, "failed adding byt-gpio chip\n");
@@ -1546,22 +1568,6 @@ static int byt_gpio_probe(struct byt_gpio *vg)
 		return ret;
 	}
 
-	/* set up interrupts  */
-	irq_rc = platform_get_resource(vg->pdev, IORESOURCE_IRQ, 0);
-	if (irq_rc && irq_rc->start) {
-		byt_gpio_irq_init_hw(vg);
-		ret = gpiochip_irqchip_add(gc, &byt_irqchip, 0,
-					   handle_bad_irq, IRQ_TYPE_NONE);
-		if (ret) {
-			dev_err(&vg->pdev->dev, "failed to add irqchip\n");
-			return ret;
-		}
-
-		gpiochip_set_chained_irqchip(gc, &byt_irqchip,
-					     (unsigned)irq_rc->start,
-					     byt_gpio_irq_handler);
-	}
-
 	return ret;
 }
 
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH] arm64: dts: ls1028a: fix gpio nodes
From: Shawn Guo @ 2019-08-12 13:47 UTC (permalink / raw)
  To: Hui Song, Li Yang
  Cc: Rob Herring, Mark Rutland, Linus Walleij, Bartosz Golaszewski,
	linux-arm-kernel, devicetree, linux-kernel, linux-gpio
In-Reply-To: <20190805065700.7601-1-hui.song_1@nxp.com>

On Mon, Aug 05, 2019 at 02:57:00PM +0800, Hui Song wrote:
> From: Song Hui <hui.song_1@nxp.com>
> 
> Update the nodes to include little-endian
> property to be consistent with the hardware.
> 
> Signed-off-by: Song Hui <hui.song_1@nxp.com>

@Leo, looks good?

Shawn

> ---
>  arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> index aef5b06..7ccbbfc 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> @@ -277,33 +277,36 @@
>  		};
>  
>  		gpio1: gpio@2300000 {
> -			compatible = "fsl,qoriq-gpio";
> +			compatible = "fsl,ls1028a-gpio","fsl,qoriq-gpio";
>  			reg = <0x0 0x2300000 0x0 0x10000>;
>  			interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
>  			gpio-controller;
>  			#gpio-cells = <2>;
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
> +			little-endian;
>  		};
>  
>  		gpio2: gpio@2310000 {
> -			compatible = "fsl,qoriq-gpio";
> +			compatible = "fsl,ls1028a-gpio","fsl,qoriq-gpio";
>  			reg = <0x0 0x2310000 0x0 0x10000>;
>  			interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
>  			gpio-controller;
>  			#gpio-cells = <2>;
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
> +			little-endian;
>  		};
>  
>  		gpio3: gpio@2320000 {
> -			compatible = "fsl,qoriq-gpio";
> +			compatible = "fsl,ls1028a-gpio","fsl,qoriq-gpio";
>  			reg = <0x0 0x2320000 0x0 0x10000>;
>  			interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
>  			gpio-controller;
>  			#gpio-cells = <2>;
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
> +			little-endian;
>  		};
>  
>  		usb0: usb@3100000 {
> -- 
> 2.9.5
> 

^ permalink raw reply

* [PATCH] pinctrl: bcm-iproc: Pass irqchip when adding gpiochip
From: Linus Walleij @ 2019-08-12 13:25 UTC (permalink / raw)
  To: linux-gpio
  Cc: Bartosz Golaszewski, Linus Walleij, Pramod Kumar, Ray Jui,
	Scott Branden, Thierry Reding

We need to convert all old gpio irqchips to pass the irqchip
setup along when adding the gpio_chip. For more info see
drivers/gpio/TODO.

For chained irqchips this is a pretty straight-forward
conversion.

Cc: Pramod Kumar <pramodku@broadcom.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Scott Branden <sbranden@broadcom.com>
Cc: Thierry Reding <treding@nvidia.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 33 ++++++++++++++----------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
index 18ff01727e0e..ee01306c62fa 100644
--- a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
+++ b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
@@ -780,6 +780,25 @@ static int iproc_gpio_probe(struct platform_device *pdev)
 	chip->pinmux_is_supported = of_property_read_bool(dev->of_node,
 							"gpio-ranges");
 
+	/* optional GPIO interrupt support */
+	irq = platform_get_irq(pdev, 0);
+	if (irq) {
+		struct gpio_irq_chip *girq;
+
+		girq = &gc->irq;
+		girq->chip = &iproc_gpio_irq_chip;
+		girq->parent_handler = iproc_gpio_irq_handler;
+		girq->num_parents = 1;
+		girq->parents = devm_kcalloc(dev, 1,
+					     sizeof(*girq->parents),
+					     GFP_KERNEL);
+		if (!girq->parents)
+			return -ENOMEM;
+		girq->parents[0] = irq;
+		girq->default_type = IRQ_TYPE_NONE;
+		girq->handler = handle_simple_irq;
+	}
+
 	ret = gpiochip_add_data(gc, chip);
 	if (ret < 0) {
 		dev_err(dev, "unable to add GPIO chip\n");
@@ -804,20 +823,6 @@ static int iproc_gpio_probe(struct platform_device *pdev)
 		}
 	}
 
-	/* optional GPIO interrupt support */
-	irq = platform_get_irq(pdev, 0);
-	if (irq) {
-		ret = gpiochip_irqchip_add(gc, &iproc_gpio_irq_chip, 0,
-					   handle_simple_irq, IRQ_TYPE_NONE);
-		if (ret) {
-			dev_err(dev, "no GPIO irqchip\n");
-			goto err_rm_gpiochip;
-		}
-
-		gpiochip_set_chained_irqchip(gc, &iproc_gpio_irq_chip, irq,
-					     iproc_gpio_irq_handler);
-	}
-
 	return 0;
 
 err_rm_gpiochip:
-- 
2.21.0


^ permalink raw reply related

* [PATCH] pinctrl: bcm-iproc: Use SPDX header
From: Linus Walleij @ 2019-08-12 13:04 UTC (permalink / raw)
  To: linux-gpio; +Cc: Linus Walleij, Pramod Kumar, Ray Jui, Scott Branden

This convert the BCM IPROC driver to use the SPDX header
for indicating GPL v2.0 only licensing.

Cc: Pramod Kumar <pramodku@broadcom.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Scott Branden <sbranden@broadcom.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
index b70058caee50..18ff01727e0e 100644
--- a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
+++ b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
@@ -1,17 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (C) 2014-2017 Broadcom
  *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation version 2.
- *
- * This program is distributed "as is" WITHOUT ANY WARRANTY of any
- * kind, whether express or implied; without even the implied warranty
- * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-/*
  * This file contains the Broadcom Iproc GPIO driver that supports 3
  * GPIO controllers on Iproc including the ASIU GPIO controller, the
  * chipCommonG GPIO controller, and the always-on GPIO controller. Basic
-- 
2.21.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox