linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] ux500 fixes bound for the -rcs
@ 2012-10-24 14:45 Lee Jones
  2012-10-24 14:45 ` [PATCH 1/6] mfd: ab8500-core: Remove unused ab8500-gpio IRQ ranges Lee Jones
                   ` (6 more replies)
  0 siblings, 7 replies; 27+ messages in thread
From: Lee Jones @ 2012-10-24 14:45 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel; +Cc: arnd, linus.walleij

This patch-set contains a bunch of fixes which are bound for the
v3.6 Release Candidates. Each of them provides a fix for something
which went wrong during the merge window. Without them we either
can't build the kernel, have no GPIOs, only have one running CPU
core, see unnecessary WARN()s, or individual devices fail at to be
functional.

 arch/arm/boot/dts/dbx5x0.dtsi     |   17 ++++++-
 arch/arm/mach-ux500/cpu-db8500.c  |    1 +
 arch/arm/mach-ux500/cpu.c         |    1 +
 drivers/mfd/ab8500-core.c         |   36 --------------
 drivers/pinctrl/pinctrl-nomadik.c |   96 ++++++++++++++++++-------------------
 5 files changed, 66 insertions(+), 85 deletions(-)


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

* [PATCH 1/6] mfd: ab8500-core: Remove unused ab8500-gpio IRQ ranges
  2012-10-24 14:45 [PATCH 0/6] ux500 fixes bound for the -rcs Lee Jones
@ 2012-10-24 14:45 ` Lee Jones
  2012-10-24 17:37   ` Linus Walleij
  2012-10-24 14:45 ` [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver Lee Jones
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 27+ messages in thread
From: Lee Jones @ 2012-10-24 14:45 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: arnd, linus.walleij, Lee Jones, Samuel Ortiz

The IRQ ranges provided in ab8500-core to be passed on to the
ab8500-gpio driver are not only redundant, but they are also
causing a warning in the boot log. These IRQ ranges, like any
other MFD related IRQ resource are passed though MFD core for
automatic conversion to virtual IRQs; however, MFD core does
not support IRQ mapping of IRQ ranges. Let's just remove them.

Cc: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/ab8500-core.c |   36 ------------------------------------
 1 file changed, 36 deletions(-)

diff --git a/drivers/mfd/ab8500-core.c b/drivers/mfd/ab8500-core.c
index d4008db..2a69dc2 100644
--- a/drivers/mfd/ab8500-core.c
+++ b/drivers/mfd/ab8500-core.c
@@ -591,38 +591,6 @@ int ab8500_suspend(struct ab8500 *ab8500)
 		return 0;
 }
 
-/* AB8500 GPIO Resources */
-static struct resource __devinitdata ab8500_gpio_resources[] = {
-	{
-		.name	= "GPIO_INT6",
-		.start	= AB8500_INT_GPIO6R,
-		.end	= AB8500_INT_GPIO41F,
-		.flags	= IORESOURCE_IRQ,
-	}
-};
-
-/* AB9540 GPIO Resources */
-static struct resource __devinitdata ab9540_gpio_resources[] = {
-	{
-		.name	= "GPIO_INT6",
-		.start	= AB8500_INT_GPIO6R,
-		.end	= AB8500_INT_GPIO41F,
-		.flags	= IORESOURCE_IRQ,
-	},
-	{
-		.name	= "GPIO_INT14",
-		.start	= AB9540_INT_GPIO50R,
-		.end	= AB9540_INT_GPIO54R,
-		.flags	= IORESOURCE_IRQ,
-	},
-	{
-		.name	= "GPIO_INT15",
-		.start	= AB9540_INT_GPIO50F,
-		.end	= AB9540_INT_GPIO54F,
-		.flags	= IORESOURCE_IRQ,
-	}
-};
-
 static struct resource __devinitdata ab8500_gpadc_resources[] = {
 	{
 		.name	= "HW_CONV_END",
@@ -1065,8 +1033,6 @@ static struct mfd_cell __devinitdata ab8500_devs[] = {
 	{
 		.name = "ab8500-gpio",
 		.of_compatible = "stericsson,ab8500-gpio",
-		.num_resources = ARRAY_SIZE(ab8500_gpio_resources),
-		.resources = ab8500_gpio_resources,
 	},
 	{
 		.name = "ab8500-usb",
@@ -1083,8 +1049,6 @@ static struct mfd_cell __devinitdata ab8500_devs[] = {
 static struct mfd_cell __devinitdata ab9540_devs[] = {
 	{
 		.name = "ab8500-gpio",
-		.num_resources = ARRAY_SIZE(ab9540_gpio_resources),
-		.resources = ab9540_gpio_resources,
 	},
 	{
 		.name = "ab9540-usb",
-- 
1.7.9.5


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

* [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver
  2012-10-24 14:45 [PATCH 0/6] ux500 fixes bound for the -rcs Lee Jones
  2012-10-24 14:45 ` [PATCH 1/6] mfd: ab8500-core: Remove unused ab8500-gpio IRQ ranges Lee Jones
@ 2012-10-24 14:45 ` Lee Jones
  2012-10-24 17:32   ` Linus Walleij
  2012-10-25 12:41   ` Linus Walleij
  2012-10-24 14:45 ` [PATCH 3/6] ARM: ux500: Fix build error relating to IRQCHIP_SKIP_SET_WAKE Lee Jones
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 27+ messages in thread
From: Lee Jones @ 2012-10-24 14:45 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel; +Cc: arnd, linus.walleij, Lee Jones

The clock framework has changed somewhat and it's now better to
invoke clock_prepare_enable() and clk_disable_unprepare() rather
than the legacy clk_enable() and clk_disable() calls. This patch
converts the Nomadik Pin Control driver to the new framework.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/pinctrl/pinctrl-nomadik.c |   96 ++++++++++++++++++-------------------
 1 file changed, 48 insertions(+), 48 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
index 01aea1c..570c753 100644
--- a/drivers/pinctrl/pinctrl-nomadik.c
+++ b/drivers/pinctrl/pinctrl-nomadik.c
@@ -448,7 +448,7 @@ static void nmk_gpio_glitch_slpm_init(unsigned int *slpm)
 		if (!chip)
 			break;
 
-		clk_enable(chip->clk);
+		clk_prepare_enable(chip->clk);
 
 		slpm[i] = readl(chip->addr + NMK_GPIO_SLPC);
 		writel(temp, chip->addr + NMK_GPIO_SLPC);
@@ -467,7 +467,7 @@ static void nmk_gpio_glitch_slpm_restore(unsigned int *slpm)
 
 		writel(slpm[i], chip->addr + NMK_GPIO_SLPC);
 
-		clk_disable(chip->clk);
+		clk_disable_unprepare(chip->clk);
 	}
 }
 
@@ -512,12 +512,12 @@ static int __nmk_config_pins(pin_cfg_t *cfgs, int num, bool sleep)
 			break;
 		}
 
-		clk_enable(nmk_chip->clk);
+		clk_prepare_enable(nmk_chip->clk);
 		spin_lock(&nmk_chip->lock);
 		__nmk_config_pin(nmk_chip, pin % NMK_GPIO_PER_CHIP,
 				 cfgs[i], sleep, glitch ? slpm : NULL);
 		spin_unlock(&nmk_chip->lock);
-		clk_disable(nmk_chip->clk);
+		clk_disable_unprepare(nmk_chip->clk);
 	}
 
 	if (glitch)
@@ -602,7 +602,7 @@ int nmk_gpio_set_slpm(int gpio, enum nmk_gpio_slpm mode)
 	if (!nmk_chip)
 		return -EINVAL;
 
-	clk_enable(nmk_chip->clk);
+	clk_prepare_enable(nmk_chip->clk);
 	spin_lock_irqsave(&nmk_gpio_slpm_lock, flags);
 	spin_lock(&nmk_chip->lock);
 
@@ -610,7 +610,7 @@ int nmk_gpio_set_slpm(int gpio, enum nmk_gpio_slpm mode)
 
 	spin_unlock(&nmk_chip->lock);
 	spin_unlock_irqrestore(&nmk_gpio_slpm_lock, flags);
-	clk_disable(nmk_chip->clk);
+	clk_disable_unprepare(nmk_chip->clk);
 
 	return 0;
 }
@@ -637,11 +637,11 @@ int nmk_gpio_set_pull(int gpio, enum nmk_gpio_pull pull)
 	if (!nmk_chip)
 		return -EINVAL;
 
-	clk_enable(nmk_chip->clk);
+	clk_prepare_enable(nmk_chip->clk);
 	spin_lock_irqsave(&nmk_chip->lock, flags);
 	__nmk_gpio_set_pull(nmk_chip, gpio % NMK_GPIO_PER_CHIP, pull);
 	spin_unlock_irqrestore(&nmk_chip->lock, flags);
-	clk_disable(nmk_chip->clk);
+	clk_disable_unprepare(nmk_chip->clk);
 
 	return 0;
 }
@@ -665,11 +665,11 @@ int nmk_gpio_set_mode(int gpio, int gpio_mode)
 	if (!nmk_chip)
 		return -EINVAL;
 
-	clk_enable(nmk_chip->clk);
+	clk_prepare_enable(nmk_chip->clk);
 	spin_lock_irqsave(&nmk_chip->lock, flags);
 	__nmk_gpio_set_mode(nmk_chip, gpio % NMK_GPIO_PER_CHIP, gpio_mode);
 	spin_unlock_irqrestore(&nmk_chip->lock, flags);
-	clk_disable(nmk_chip->clk);
+	clk_disable_unprepare(nmk_chip->clk);
 
 	return 0;
 }
@@ -686,12 +686,12 @@ int nmk_gpio_get_mode(int gpio)
 
 	bit = 1 << (gpio % NMK_GPIO_PER_CHIP);
 
-	clk_enable(nmk_chip->clk);
+	clk_prepare_enable(nmk_chip->clk);
 
 	afunc = readl(nmk_chip->addr + NMK_GPIO_AFSLA) & bit;
 	bfunc = readl(nmk_chip->addr + NMK_GPIO_AFSLB) & bit;
 
-	clk_disable(nmk_chip->clk);
+	clk_disable_unprepare(nmk_chip->clk);
 
 	return (afunc ? NMK_GPIO_ALT_A : 0) | (bfunc ? NMK_GPIO_ALT_B : 0);
 }
@@ -712,9 +712,9 @@ static void nmk_gpio_irq_ack(struct irq_data *d)
 	if (!nmk_chip)
 		return;
 
-	clk_enable(nmk_chip->clk);
+	clk_prepare_enable(nmk_chip->clk);
 	writel(nmk_gpio_get_bitmask(d->hwirq), nmk_chip->addr + NMK_GPIO_IC);
-	clk_disable(nmk_chip->clk);
+	clk_disable_unprepare(nmk_chip->clk);
 }
 
 enum nmk_gpio_irq_type {
@@ -788,7 +788,7 @@ static int nmk_gpio_irq_maskunmask(struct irq_data *d, bool enable)
 	if (!nmk_chip)
 		return -EINVAL;
 
-	clk_enable(nmk_chip->clk);
+	clk_prepare_enable(nmk_chip->clk);
 	spin_lock_irqsave(&nmk_gpio_slpm_lock, flags);
 	spin_lock(&nmk_chip->lock);
 
@@ -799,7 +799,7 @@ static int nmk_gpio_irq_maskunmask(struct irq_data *d, bool enable)
 
 	spin_unlock(&nmk_chip->lock);
 	spin_unlock_irqrestore(&nmk_gpio_slpm_lock, flags);
-	clk_disable(nmk_chip->clk);
+	clk_disable_unprepare(nmk_chip->clk);
 
 	return 0;
 }
@@ -825,7 +825,7 @@ static int nmk_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
 		return -EINVAL;
 	bitmask = nmk_gpio_get_bitmask(d->hwirq);
 
-	clk_enable(nmk_chip->clk);
+	clk_prepare_enable(nmk_chip->clk);
 	spin_lock_irqsave(&nmk_gpio_slpm_lock, flags);
 	spin_lock(&nmk_chip->lock);
 
@@ -839,7 +839,7 @@ static int nmk_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
 
 	spin_unlock(&nmk_chip->lock);
 	spin_unlock_irqrestore(&nmk_gpio_slpm_lock, flags);
-	clk_disable(nmk_chip->clk);
+	clk_disable_unprepare(nmk_chip->clk);
 
 	return 0;
 }
@@ -861,7 +861,7 @@ static int nmk_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 	if (type & IRQ_TYPE_LEVEL_LOW)
 		return -EINVAL;
 
-	clk_enable(nmk_chip->clk);
+	clk_prepare_enable(nmk_chip->clk);
 	spin_lock_irqsave(&nmk_chip->lock, flags);
 
 	if (enabled)
@@ -885,7 +885,7 @@ static int nmk_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 		__nmk_gpio_irq_modify(nmk_chip, d->hwirq, WAKE, true);
 
 	spin_unlock_irqrestore(&nmk_chip->lock, flags);
-	clk_disable(nmk_chip->clk);
+	clk_disable_unprepare(nmk_chip->clk);
 
 	return 0;
 }
@@ -894,7 +894,7 @@ static unsigned int nmk_gpio_irq_startup(struct irq_data *d)
 {
 	struct nmk_gpio_chip *nmk_chip = irq_data_get_irq_chip_data(d);
 
-	clk_enable(nmk_chip->clk);
+	clk_prepare_enable(nmk_chip->clk);
 	nmk_gpio_irq_unmask(d);
 	return 0;
 }
@@ -904,7 +904,7 @@ static void nmk_gpio_irq_shutdown(struct irq_data *d)
 	struct nmk_gpio_chip *nmk_chip = irq_data_get_irq_chip_data(d);
 
 	nmk_gpio_irq_mask(d);
-	clk_disable(nmk_chip->clk);
+	clk_disable_unprepare(nmk_chip->clk);
 }
 
 static struct irq_chip nmk_gpio_irq_chip = {
@@ -943,9 +943,9 @@ static void nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 	struct nmk_gpio_chip *nmk_chip = irq_get_handler_data(irq);
 	u32 status;
 
-	clk_enable(nmk_chip->clk);
+	clk_prepare_enable(nmk_chip->clk);
 	status = readl(nmk_chip->addr + NMK_GPIO_IS);
-	clk_disable(nmk_chip->clk);
+	clk_disable_unprepare(nmk_chip->clk);
 
 	__nmk_gpio_irq_handler(irq, desc, status);
 }
@@ -998,11 +998,11 @@ static int nmk_gpio_make_input(struct gpio_chip *chip, unsigned offset)
 	struct nmk_gpio_chip *nmk_chip =
 		container_of(chip, struct nmk_gpio_chip, chip);
 
-	clk_enable(nmk_chip->clk);
+	clk_prepare_enable(nmk_chip->clk);
 
 	writel(1 << offset, nmk_chip->addr + NMK_GPIO_DIRC);
 
-	clk_disable(nmk_chip->clk);
+	clk_disable_unprepare(nmk_chip->clk);
 
 	return 0;
 }
@@ -1014,11 +1014,11 @@ static int nmk_gpio_get_input(struct gpio_chip *chip, unsigned offset)
 	u32 bit = 1 << offset;
 	int value;
 
-	clk_enable(nmk_chip->clk);
+	clk_prepare_enable(nmk_chip->clk);
 
 	value = (readl(nmk_chip->addr + NMK_GPIO_DAT) & bit) != 0;
 
-	clk_disable(nmk_chip->clk);
+	clk_disable_unprepare(nmk_chip->clk);
 
 	return value;
 }
@@ -1029,11 +1029,11 @@ static void nmk_gpio_set_output(struct gpio_chip *chip, unsigned offset,
 	struct nmk_gpio_chip *nmk_chip =
 		container_of(chip, struct nmk_gpio_chip, chip);
 
-	clk_enable(nmk_chip->clk);
+	clk_prepare_enable(nmk_chip->clk);
 
 	__nmk_gpio_set_output(nmk_chip, offset, val);
 
-	clk_disable(nmk_chip->clk);
+	clk_disable_unprepare(nmk_chip->clk);
 }
 
 static int nmk_gpio_make_output(struct gpio_chip *chip, unsigned offset,
@@ -1042,11 +1042,11 @@ static int nmk_gpio_make_output(struct gpio_chip *chip, unsigned offset,
 	struct nmk_gpio_chip *nmk_chip =
 		container_of(chip, struct nmk_gpio_chip, chip);
 
-	clk_enable(nmk_chip->clk);
+	clk_prepare_enable(nmk_chip->clk);
 
 	__nmk_gpio_make_output(nmk_chip, offset, val);
 
-	clk_disable(nmk_chip->clk);
+	clk_disable_unprepare(nmk_chip->clk);
 
 	return 0;
 }
@@ -1080,7 +1080,7 @@ static void nmk_gpio_dbg_show_one(struct seq_file *s, struct gpio_chip *chip,
 		[NMK_GPIO_ALT_C]	= "altC",
 	};
 
-	clk_enable(nmk_chip->clk);
+	clk_prepare_enable(nmk_chip->clk);
 	is_out = !!(readl(nmk_chip->addr + NMK_GPIO_DIR) & bit);
 	pull = !(readl(nmk_chip->addr + NMK_GPIO_PDIS) & bit);
 	mode = nmk_gpio_get_mode(gpio);
@@ -1118,7 +1118,7 @@ static void nmk_gpio_dbg_show_one(struct seq_file *s, struct gpio_chip *chip,
 				   ? " wakeup" : "");
 		}
 	}
-	clk_disable(nmk_chip->clk);
+	clk_disable_unprepare(nmk_chip->clk);
 }
 
 static void nmk_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
@@ -1164,7 +1164,7 @@ void nmk_gpio_clocks_enable(void)
 		if (!chip)
 			continue;
 
-		clk_enable(chip->clk);
+		clk_prepare_enable(chip->clk);
 	}
 }
 
@@ -1178,7 +1178,7 @@ void nmk_gpio_clocks_disable(void)
 		if (!chip)
 			continue;
 
-		clk_disable(chip->clk);
+		clk_disable_unprepare(chip->clk);
 	}
 }
 
@@ -1201,14 +1201,14 @@ void nmk_gpio_wakeups_suspend(void)
 		if (!chip)
 			break;
 
-		clk_enable(chip->clk);
+		clk_prepare_enable(chip->clk);
 
 		writel(chip->rwimsc & chip->real_wake,
 		       chip->addr + NMK_GPIO_RWIMSC);
 		writel(chip->fwimsc & chip->real_wake,
 		       chip->addr + NMK_GPIO_FWIMSC);
 
-		clk_disable(chip->clk);
+		clk_disable_unprepare(chip->clk);
 	}
 }
 
@@ -1222,12 +1222,12 @@ void nmk_gpio_wakeups_resume(void)
 		if (!chip)
 			break;
 
-		clk_enable(chip->clk);
+		clk_prepare_enable(chip->clk);
 
 		writel(chip->rwimsc, chip->addr + NMK_GPIO_RWIMSC);
 		writel(chip->fwimsc, chip->addr + NMK_GPIO_FWIMSC);
 
-		clk_disable(chip->clk);
+		clk_disable_unprepare(chip->clk);
 	}
 }
 
@@ -1367,9 +1367,9 @@ static int __devinit nmk_gpio_probe(struct platform_device *dev)
 	chip->dev = &dev->dev;
 	chip->owner = THIS_MODULE;
 
-	clk_enable(nmk_chip->clk);
+	clk_prepare_enable(nmk_chip->clk);
 	nmk_chip->lowemi = readl_relaxed(nmk_chip->addr + NMK_GPIO_LOWEMI);
-	clk_disable(nmk_chip->clk);
+	clk_disable_unprepare(nmk_chip->clk);
 
 #ifdef CONFIG_OF_GPIO
 	chip->of_node = np;
@@ -1580,7 +1580,7 @@ static int nmk_pmx_enable(struct pinctrl_dev *pctldev, unsigned function,
 		nmk_chip = container_of(chip, struct nmk_gpio_chip, chip);
 		dev_dbg(npct->dev, "setting pin %d to altsetting %d\n", g->pins[i], g->altsetting);
 
-		clk_enable(nmk_chip->clk);
+		clk_prepare_enable(nmk_chip->clk);
 		bit = g->pins[i] % NMK_GPIO_PER_CHIP;
 		/*
 		 * If the pin is switching to altfunc, and there was an
@@ -1593,7 +1593,7 @@ static int nmk_pmx_enable(struct pinctrl_dev *pctldev, unsigned function,
 
 		__nmk_gpio_set_mode_safe(nmk_chip, bit,
 			(g->altsetting & NMK_GPIO_ALT_C), glitch);
-		clk_disable(nmk_chip->clk);
+		clk_disable_unprepare(nmk_chip->clk);
 
 		/*
 		 * Call PRCM GPIOCR config function in case ALTC
@@ -1657,11 +1657,11 @@ int nmk_gpio_request_enable(struct pinctrl_dev *pctldev,
 
 	dev_dbg(npct->dev, "enable pin %u as GPIO\n", offset);
 
-	clk_enable(nmk_chip->clk);
+	clk_prepare_enable(nmk_chip->clk);
 	bit = offset % NMK_GPIO_PER_CHIP;
 	/* There is no glitch when converting any pin to GPIO */
 	__nmk_gpio_set_mode(nmk_chip, bit, NMK_GPIO_ALT_GPIO);
-	clk_disable(nmk_chip->clk);
+	clk_disable_unprepare(nmk_chip->clk);
 
 	return 0;
 }
@@ -1773,7 +1773,7 @@ int nmk_pin_config_set(struct pinctrl_dev *pctldev,
 		output ? (val ? "high" : "low") : "",
 		lowemi ? "on" : "off" );
 
-	clk_enable(nmk_chip->clk);
+	clk_prepare_enable(nmk_chip->clk);
 	bit = pin % NMK_GPIO_PER_CHIP;
 	if (gpiomode)
 		/* No glitch when going to GPIO mode */
@@ -1788,7 +1788,7 @@ int nmk_pin_config_set(struct pinctrl_dev *pctldev,
 	__nmk_gpio_set_lowemi(nmk_chip, bit, lowemi);
 
 	__nmk_gpio_set_slpm(nmk_chip, bit, slpm);
-	clk_disable(nmk_chip->clk);
+	clk_disable_unprepare(nmk_chip->clk);
 	return 0;
 }
 
-- 
1.7.9.5


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

* [PATCH 3/6] ARM: ux500: Fix build error relating to IRQCHIP_SKIP_SET_WAKE
  2012-10-24 14:45 [PATCH 0/6] ux500 fixes bound for the -rcs Lee Jones
  2012-10-24 14:45 ` [PATCH 1/6] mfd: ab8500-core: Remove unused ab8500-gpio IRQ ranges Lee Jones
  2012-10-24 14:45 ` [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver Lee Jones
@ 2012-10-24 14:45 ` Lee Jones
  2012-10-24 14:45 ` [PATCH 4/6] ARM: ux500: Specify AMBA Primecell IDs for Nomadik I2C in DT Lee Jones
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 27+ messages in thread
From: Lee Jones @ 2012-10-24 14:45 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel; +Cc: arnd, linus.walleij, Lee Jones

This patch fixes the build error below:

arch/arm/mach-ux500/cpu.c: In function ‘ux500_init_irq’:
arch/arm/mach-ux500/cpu.c:55:2: error: invalid use of undefined type ‘struct irq_chip’
arch/arm/mach-ux500/cpu.c:55:24: error: ‘IRQCHIP_SKIP_SET_WAKE’ undeclared (first use in this function)
arch/arm/mach-ux500/cpu.c:55:24: note: each undeclared identifier is reported only once for each function it appears in
arch/arm/mach-ux500/cpu.c:55:48: error: ‘IRQCHIP_MASK_ON_SUSPEND’ undeclared (first use in this function)

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/mach-ux500/cpu.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-ux500/cpu.c b/arch/arm/mach-ux500/cpu.c
index fb25f4e..721e7b4 100644
--- a/arch/arm/mach-ux500/cpu.c
+++ b/arch/arm/mach-ux500/cpu.c
@@ -16,6 +16,7 @@
 #include <linux/stat.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
+#include <linux/irq.h>
 #include <linux/platform_data/clk-ux500.h>
 
 #include <asm/hardware/gic.h>
-- 
1.7.9.5


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

* [PATCH 4/6] ARM: ux500: Specify AMBA Primecell IDs for Nomadik I2C in DT
  2012-10-24 14:45 [PATCH 0/6] ux500 fixes bound for the -rcs Lee Jones
                   ` (2 preceding siblings ...)
  2012-10-24 14:45 ` [PATCH 3/6] ARM: ux500: Fix build error relating to IRQCHIP_SKIP_SET_WAKE Lee Jones
@ 2012-10-24 14:45 ` Lee Jones
  2012-10-24 17:47   ` Linus Walleij
  2012-10-24 14:45 ` [PATCH 5/6] ARM: ux500: Correct SDI5 address and add some format changes Lee Jones
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 27+ messages in thread
From: Lee Jones @ 2012-10-24 14:45 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel; +Cc: arnd, linus.walleij, Lee Jones

Now the Nomadik I2C driver has been converted to an AMBA one, we
are required to provide the Primecell IDs via platform code. When
booting with DT enabled these have to be specified in the device
nodes. We do that here.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/dbx5x0.dtsi |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index 536df5d..4ed1b4b 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -452,6 +452,8 @@
 			compatible = "stericsson,db8500-i2c", "st,nomadik-i2c", "arm,primecell";
 			reg = <0x80004000 0x1000>;
 			interrupts = <0 21 0x4>;
+			arm,primecell-periphid = <0x180024>;
+
 			#address-cells = <1>;
 			#size-cells = <0>;
 			v-i2c-supply = <&db8500_vape_reg>;
@@ -463,6 +465,8 @@
 			compatible = "stericsson,db8500-i2c", "st,nomadik-i2c", "arm,primecell";
 			reg = <0x80122000 0x1000>;
 			interrupts = <0 22 0x4>;
+			arm,primecell-periphid = <0x180024>;
+
 			#address-cells = <1>;
 			#size-cells = <0>;
 			v-i2c-supply = <&db8500_vape_reg>;
@@ -474,6 +478,8 @@
 			compatible = "stericsson,db8500-i2c", "st,nomadik-i2c", "arm,primecell";
 			reg = <0x80128000 0x1000>;
 			interrupts = <0 55 0x4>;
+			arm,primecell-periphid = <0x180024>;
+
 			#address-cells = <1>;
 			#size-cells = <0>;
 			v-i2c-supply = <&db8500_vape_reg>;
@@ -485,6 +491,8 @@
 			compatible = "stericsson,db8500-i2c", "st,nomadik-i2c", "arm,primecell";
 			reg = <0x80110000 0x1000>;
 			interrupts = <0 12 0x4>;
+			arm,primecell-periphid = <0x180024>;
+
 			#address-cells = <1>;
 			#size-cells = <0>;
 			v-i2c-supply = <&db8500_vape_reg>;
@@ -496,6 +504,8 @@
 			compatible = "stericsson,db8500-i2c", "st,nomadik-i2c", "arm,primecell";
 			reg = <0x8012a000 0x1000>;
 			interrupts = <0 51 0x4>;
+			arm,primecell-periphid = <0x180024>;
+
 			#address-cells = <1>;
 			#size-cells = <0>;
 			v-i2c-supply = <&db8500_vape_reg>;
-- 
1.7.9.5


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

* [PATCH 5/6] ARM: ux500: Correct SDI5 address and add some format changes
  2012-10-24 14:45 [PATCH 0/6] ux500 fixes bound for the -rcs Lee Jones
                   ` (3 preceding siblings ...)
  2012-10-24 14:45 ` [PATCH 4/6] ARM: ux500: Specify AMBA Primecell IDs for Nomadik I2C in DT Lee Jones
@ 2012-10-24 14:45 ` Lee Jones
  2012-10-24 17:48   ` Linus Walleij
  2012-10-24 14:45 ` [PATCH 6/6] ARM: ux500: Convert DT_MACHINE_START to use SMP operations Lee Jones
  2012-10-24 17:46 ` [PATCH 0/6] ux500 fixes bound for the -rcs Linus Walleij
  6 siblings, 1 reply; 27+ messages in thread
From: Lee Jones @ 2012-10-24 14:45 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel; +Cc: arnd, linus.walleij, Lee Jones

Here we fix a simple copy and paste error and bring some node
spaces back into line with the remainder of the tree.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/dbx5x0.dtsi |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index 4ed1b4b..7ce45fc 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -552,33 +552,38 @@
 			interrupts = <0 60 0x4>;
 			status = "disabled";
 		};
+
 		sdi1_per2@80118000 {
 			compatible = "arm,pl18x", "arm,primecell";
 			reg = <0x80118000 0x1000>;
 			interrupts = <0 50 0x4>;
 			status = "disabled";
 		};
+
 		sdi2_per3@80005000 {
 			compatible = "arm,pl18x", "arm,primecell";
 			reg = <0x80005000 0x1000>;
 			interrupts = <0 41 0x4>;
 			status = "disabled";
 		};
+
 		sdi3_per2@80119000 {
 			compatible = "arm,pl18x", "arm,primecell";
 			reg = <0x80119000 0x1000>;
 			interrupts = <0 59 0x4>;
 			status = "disabled";
 		};
+
 		sdi4_per2@80114000 {
 			compatible = "arm,pl18x", "arm,primecell";
 			reg = <0x80114000 0x1000>;
 			interrupts = <0 99 0x4>;
 			status = "disabled";
 		};
+
 		sdi5_per3@80008000 {
 			compatible = "arm,pl18x", "arm,primecell";
-			reg = <0x80114000 0x1000>;
+			reg = <0x80008000 0x1000>;
 			interrupts = <0 100 0x4>;
 			status = "disabled";
 		};
-- 
1.7.9.5


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

* [PATCH 6/6] ARM: ux500: Convert DT_MACHINE_START to use SMP operations
  2012-10-24 14:45 [PATCH 0/6] ux500 fixes bound for the -rcs Lee Jones
                   ` (4 preceding siblings ...)
  2012-10-24 14:45 ` [PATCH 5/6] ARM: ux500: Correct SDI5 address and add some format changes Lee Jones
@ 2012-10-24 14:45 ` Lee Jones
  2012-10-24 17:48   ` Linus Walleij
  2012-10-24 17:46 ` [PATCH 0/6] ux500 fixes bound for the -rcs Linus Walleij
  6 siblings, 1 reply; 27+ messages in thread
From: Lee Jones @ 2012-10-24 14:45 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel; +Cc: arnd, linus.walleij, Lee Jones

The Device Tree machine description for the ux5x0 was moved
recently and as a consequence missed the addition of SMP
operations. Without this patch SMP doesn't work and only one
CPU is present after booting.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/mach-ux500/cpu-db8500.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
index c65e7e3..4a0c40a 100644
--- a/arch/arm/mach-ux500/cpu-db8500.c
+++ b/arch/arm/mach-ux500/cpu-db8500.c
@@ -339,6 +339,7 @@ static const char * stericsson_dt_platform_compat[] = {
 };
 
 DT_MACHINE_START(U8500_DT, "ST-Ericsson Ux5x0 platform (Device Tree Support)")
+	.smp            = smp_ops(ux500_smp_ops),
 	.map_io		= u8500_map_io,
 	.init_irq	= ux500_init_irq,
 	/* we re-use nomadik timer here */
-- 
1.7.9.5


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

* Re: [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver
  2012-10-24 14:45 ` [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver Lee Jones
@ 2012-10-24 17:32   ` Linus Walleij
  2012-10-25  7:31     ` Lee Jones
  2012-10-25 12:41   ` Linus Walleij
  1 sibling, 1 reply; 27+ messages in thread
From: Linus Walleij @ 2012-10-24 17:32 UTC (permalink / raw)
  To: Lee Jones, Ulf Hansson
  Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij

On Wed, Oct 24, 2012 at 4:45 PM, Lee Jones <lee.jones@linaro.org> wrote:

> The clock framework has changed somewhat and it's now better to
> invoke clock_prepare_enable() and clk_disable_unprepare() rather
> than the legacy clk_enable() and clk_disable() calls. This patch
> converts the Nomadik Pin Control driver to the new framework.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

(...)
> -               clk_enable(chip->clk);
> +               clk_prepare_enable(chip->clk);
(...)
> -               clk_disable(chip->clk);
> +               clk_disable_unprepare(chip->clk);

(Repeated for each occurence.)

Is this *really* causing a regression? I mean the driver
begin like this in nmk_gpio_probe():

        clk = devm_clk_get(&dev->dev, NULL);
        if (IS_ERR(clk)) {
                ret = PTR_ERR(clk);
                goto out;
        }
        clk_prepare(clk);

Then it leaves the clock prepared. So the clock is always
prepared. You would only need to enable/disable it at times.

And the semantics of the clk_enable/clk_disable call pair
is such that it is fastpath and should be real quick, and that
is exactly why we're using it repeatedly like that. Inserting
clk_unprepare() effectively could make the whole driver a
lot slower, so convince me on this one. ...

I suspect the real bug (if there is one) must be in the clock
implementation.

Yours,
Linus Walleij

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

* Re: [PATCH 1/6] mfd: ab8500-core: Remove unused ab8500-gpio IRQ ranges
  2012-10-24 14:45 ` [PATCH 1/6] mfd: ab8500-core: Remove unused ab8500-gpio IRQ ranges Lee Jones
@ 2012-10-24 17:37   ` Linus Walleij
  0 siblings, 0 replies; 27+ messages in thread
From: Linus Walleij @ 2012-10-24 17:37 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij, Samuel Ortiz

On Wed, Oct 24, 2012 at 4:45 PM, Lee Jones <lee.jones@linaro.org> wrote:

> The IRQ ranges provided in ab8500-core to be passed on to the
> ab8500-gpio driver are not only redundant, but they are also
> causing a warning in the boot log. These IRQ ranges, like any
> other MFD related IRQ resource are passed though MFD core for
> automatic conversion to virtual IRQs; however, MFD core does
> not support IRQ mapping of IRQ ranges. Let's just remove them.
>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Hey! That riddes the pesky boot warning.
Tested-by: Linus Walleij <linus.walleij@linaro.org>

Sam, please pick this into the MFD tree for the -rc series!

Yours,
Linus Walleij

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

* Re: [PATCH 0/6] ux500 fixes bound for the -rcs
  2012-10-24 14:45 [PATCH 0/6] ux500 fixes bound for the -rcs Lee Jones
                   ` (5 preceding siblings ...)
  2012-10-24 14:45 ` [PATCH 6/6] ARM: ux500: Convert DT_MACHINE_START to use SMP operations Lee Jones
@ 2012-10-24 17:46 ` Linus Walleij
  6 siblings, 0 replies; 27+ messages in thread
From: Linus Walleij @ 2012-10-24 17:46 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij

On Wed, Oct 24, 2012 at 4:45 PM, Lee Jones <lee.jones@linaro.org> wrote:

> This patch-set contains a bunch of fixes which are bound for the
> v3.6 Release Candidates. Each of them provides a fix for something
> which went wrong during the merge window. Without them we either
> can't build the kernel, have no GPIOs, only have one running CPU
> core, see unnecessary WARN()s, or individual devices fail at to be
> functional.
>
>  arch/arm/boot/dts/dbx5x0.dtsi     |   17 ++++++-
>  arch/arm/mach-ux500/cpu-db8500.c  |    1 +
>  arch/arm/mach-ux500/cpu.c         |    1 +

I provide my Acked-by for patches: 3,4,5,6 of this series.

I tried to apply them to my ux500 fixes branch but it appears
there are dependencies in the .dtsi file and it does not apply
so I guess it's better if you send these to the ARM SoC tree
yourself.

Patches 1,2 need to go through respective subtrees and
I need review comments on the pinctrl patch.

Yours,
Linus Walleij

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

* Re: [PATCH 4/6] ARM: ux500: Specify AMBA Primecell IDs for Nomadik I2C in DT
  2012-10-24 14:45 ` [PATCH 4/6] ARM: ux500: Specify AMBA Primecell IDs for Nomadik I2C in DT Lee Jones
@ 2012-10-24 17:47   ` Linus Walleij
  0 siblings, 0 replies; 27+ messages in thread
From: Linus Walleij @ 2012-10-24 17:47 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij

On Wed, Oct 24, 2012 at 4:45 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Now the Nomadik I2C driver has been converted to an AMBA one, we
> are required to provide the Primecell IDs via platform code. When
> booting with DT enabled these have to be specified in the device
> nodes. We do that here.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

I suspect this should go to stable as well.

Yours,
Linus Walleij

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

* Re: [PATCH 5/6] ARM: ux500: Correct SDI5 address and add some format changes
  2012-10-24 14:45 ` [PATCH 5/6] ARM: ux500: Correct SDI5 address and add some format changes Lee Jones
@ 2012-10-24 17:48   ` Linus Walleij
  0 siblings, 0 replies; 27+ messages in thread
From: Linus Walleij @ 2012-10-24 17:48 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij

On Wed, Oct 24, 2012 at 4:45 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Here we fix a simple copy and paste error and bring some node
> spaces back into line with the remainder of the tree.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

* Re: [PATCH 6/6] ARM: ux500: Convert DT_MACHINE_START to use SMP operations
  2012-10-24 14:45 ` [PATCH 6/6] ARM: ux500: Convert DT_MACHINE_START to use SMP operations Lee Jones
@ 2012-10-24 17:48   ` Linus Walleij
  0 siblings, 0 replies; 27+ messages in thread
From: Linus Walleij @ 2012-10-24 17:48 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij

On Wed, Oct 24, 2012 at 4:45 PM, Lee Jones <lee.jones@linaro.org> wrote:

> The Device Tree machine description for the ux5x0 was moved
> recently and as a consequence missed the addition of SMP
> operations. Without this patch SMP doesn't work and only one
> CPU is present after booting.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

* Re: [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver
  2012-10-24 17:32   ` Linus Walleij
@ 2012-10-25  7:31     ` Lee Jones
  2012-10-25  7:57       ` Linus Walleij
  0 siblings, 1 reply; 27+ messages in thread
From: Lee Jones @ 2012-10-25  7:31 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Ulf Hansson, linux-arm-kernel, linux-kernel, arnd, linus.walleij

On Wed, 24 Oct 2012, Linus Walleij wrote:

> On Wed, Oct 24, 2012 at 4:45 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > The clock framework has changed somewhat and it's now better to
> > invoke clock_prepare_enable() and clk_disable_unprepare() rather
> > than the legacy clk_enable() and clk_disable() calls. This patch
> > converts the Nomadik Pin Control driver to the new framework.
> >
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> (...)
> > -               clk_enable(chip->clk);
> > +               clk_prepare_enable(chip->clk);
> (...)
> > -               clk_disable(chip->clk);
> > +               clk_disable_unprepare(chip->clk);
> 
> (Repeated for each occurence.)
> 
> Is this *really* causing a regression? I mean the driver
> begin like this in nmk_gpio_probe():
> 
>         clk = devm_clk_get(&dev->dev, NULL);
>         if (IS_ERR(clk)) {
>                 ret = PTR_ERR(clk);
>                 goto out;
>         }
>         clk_prepare(clk);
> 
> Then it leaves the clock prepared. So the clock is always
> prepared. You would only need to enable/disable it at times.
> 
> And the semantics of the clk_enable/clk_disable call pair
> is such that it is fastpath and should be real quick, and that
> is exactly why we're using it repeatedly like that. Inserting
> clk_unprepare() effectively could make the whole driver a
> lot slower, so convince me on this one. ...
> 
> I suspect the real bug (if there is one) must be in the clock
> implementation.

This certainly doesn't fix the bug we spoke about. I believe Ulf
is still working on that one.

So do you want me to remove this patch?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver
  2012-10-25  7:31     ` Lee Jones
@ 2012-10-25  7:57       ` Linus Walleij
  2012-10-25  8:23         ` Lee Jones
  0 siblings, 1 reply; 27+ messages in thread
From: Linus Walleij @ 2012-10-25  7:57 UTC (permalink / raw)
  To: Lee Jones
  Cc: Linus Walleij, Ulf Hansson, linux-arm-kernel, linux-kernel, arnd

On 10/25/2012 09:31 AM, Lee Jones wrote:
>
> This certainly doesn't fix the bug we spoke about. I believe Ulf
> is still working on that one.
>
> So do you want me to remove this patch?
>

Yeah drop it for now.

Yours,
Linus Walleij

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

* Re: [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver
  2012-10-25  7:57       ` Linus Walleij
@ 2012-10-25  8:23         ` Lee Jones
  2012-10-25  9:29           ` Ulf Hansson
  2012-10-25 12:07           ` Linus Walleij
  0 siblings, 2 replies; 27+ messages in thread
From: Lee Jones @ 2012-10-25  8:23 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Linus Walleij, Ulf Hansson, linux-arm-kernel, linux-kernel, arnd

On Thu, 25 Oct 2012, Linus Walleij wrote:

> On 10/25/2012 09:31 AM, Lee Jones wrote:
> >
> >This certainly doesn't fix the bug we spoke about. I believe Ulf
> >is still working on that one.
> >
> >So do you want me to remove this patch?
> >
> 
> Yeah drop it for now.

Actually, a quick question before I do:

If it's better/faster to prepare the clock and keep it prepared
while you do clk_enable/clk_disable, why don't we do that in all
drivers? Why do we bother to prepare/unprepare each time if all
it does is take up cycles?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver
  2012-10-25  8:23         ` Lee Jones
@ 2012-10-25  9:29           ` Ulf Hansson
  2012-10-25  9:44             ` Lee Jones
  2012-10-25 12:33             ` Linus Walleij
  2012-10-25 12:07           ` Linus Walleij
  1 sibling, 2 replies; 27+ messages in thread
From: Ulf Hansson @ 2012-10-25  9:29 UTC (permalink / raw)
  To: Lee Jones
  Cc: Linus Walleij, Linus Walleij, linux-arm-kernel, linux-kernel, arnd

On 25 October 2012 10:23, Lee Jones <lee.jones@linaro.org> wrote:
> On Thu, 25 Oct 2012, Linus Walleij wrote:
>
>> On 10/25/2012 09:31 AM, Lee Jones wrote:
>> >
>> >This certainly doesn't fix the bug we spoke about. I believe Ulf
>> >is still working on that one.
>> >
>> >So do you want me to remove this patch?
>> >
>>
>> Yeah drop it for now.
>
> Actually, a quick question before I do:
>
> If it's better/faster to prepare the clock and keep it prepared
> while you do clk_enable/clk_disable, why don't we do that in all
> drivers? Why do we bother to prepare/unprepare each time if all
> it does is take up cycles?
>

Depending on clock type, a clk_disable is actually not going to "gate"
the clock, that might happen only in unprepare. This depends on if the
clock is a fast or slow clock.
To save as much power as possible, in general, we should do both
disable and unprepare. Although it will be device driver dependent
were it is most convenient to do this things.
Sometimes it is possible to group them sometimes not.

Kind regards
Ulf Hansson

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

* Re: [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver
  2012-10-25  9:29           ` Ulf Hansson
@ 2012-10-25  9:44             ` Lee Jones
  2012-10-25 12:33             ` Linus Walleij
  1 sibling, 0 replies; 27+ messages in thread
From: Lee Jones @ 2012-10-25  9:44 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Linus Walleij, Linus Walleij, linux-arm-kernel, linux-kernel, arnd

On Thu, 25 Oct 2012, Ulf Hansson wrote:

> On 25 October 2012 10:23, Lee Jones <lee.jones@linaro.org> wrote:
> > On Thu, 25 Oct 2012, Linus Walleij wrote:
> >
> >> On 10/25/2012 09:31 AM, Lee Jones wrote:
> >> >
> >> >This certainly doesn't fix the bug we spoke about. I believe Ulf
> >> >is still working on that one.
> >> >
> >> >So do you want me to remove this patch?
> >> >
> >>
> >> Yeah drop it for now.
> >
> > Actually, a quick question before I do:
> >
> > If it's better/faster to prepare the clock and keep it prepared
> > while you do clk_enable/clk_disable, why don't we do that in all
> > drivers? Why do we bother to prepare/unprepare each time if all
> > it does is take up cycles?
> >
> 
> Depending on clock type, a clk_disable is actually not going to "gate"
> the clock, that might happen only in unprepare. This depends on if the
> clock is a fast or slow clock.
> To save as much power as possible, in general, we should do both
> disable and unprepare. Although it will be device driver dependent
> were it is most convenient to do this things.
> Sometimes it is possible to group them sometimes not.

And in this case, it's better to ... ?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver
  2012-10-25  8:23         ` Lee Jones
  2012-10-25  9:29           ` Ulf Hansson
@ 2012-10-25 12:07           ` Linus Walleij
  1 sibling, 0 replies; 27+ messages in thread
From: Linus Walleij @ 2012-10-25 12:07 UTC (permalink / raw)
  To: Lee Jones
  Cc: Linus Walleij, Ulf Hansson, linux-arm-kernel, linux-kernel, arnd

On Thu, Oct 25, 2012 at 10:23 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Thu, 25 Oct 2012, Linus Walleij wrote:
>>
>> Yeah drop it for now.
>
> Actually, a quick question before I do:
>
> If it's better/faster to prepare the clock and keep it prepared
> while you do clk_enable/clk_disable,

It is generally faster that is why we call it fastpath.

E.g. if the clock hardware can do this in IRQ context by just
chaninging one bit in a quickly written register from 1->0 and
then the clock goes off from some silicon.

Whether it's "better" or not is a transcendental question,
as it requires a ruler to measure betterness.

> why don't we do that in all
> drivers? Why do we bother to prepare/unprepare each time if all
> it does is take up cycles?

Usually to save power.

Albeit saving power may be at odds with gaining the maximum
performance and/or latency.

So depending on the demands and use case the answer to
whether or not you want to do this will be different.

That's for the clock API.

In the ux500 case specifically, you can drill down to the
clock implementation and ask the question whether or not
we want to do this for this instance of the pin controller
in this case, I'll leave that for Ulf to answer... but remember
that this driver is also used for the Nomadik NHK8815.

Yours,
Linus Walleij

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

* Re: [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver
  2012-10-25  9:29           ` Ulf Hansson
  2012-10-25  9:44             ` Lee Jones
@ 2012-10-25 12:33             ` Linus Walleij
  1 sibling, 0 replies; 27+ messages in thread
From: Linus Walleij @ 2012-10-25 12:33 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Lee Jones, Linus Walleij, linux-arm-kernel, linux-kernel, arnd

On Thu, Oct 25, 2012 at 11:29 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:

> Depending on clock type, a clk_disable is actually not going to "gate"
> the clock, that might happen only in unprepare. This depends on if the
> clock is a fast or slow clock.

Hm thats interesting. Now I need to drill down into this.
So looking at it in this case:

clk_disable() from the GPIO block of the pin controller will
hit "gpio.0", "gpio.1" etc in drivers/clk/ux500/u8500_clk.c.

These are PRCC (Programmable Clock Controller) clocks
registered using clk_reg_prcc_pclk() from clk-prcc.c.

pclk:s are using the clk_prcc_pclk_ops and these point to
clk_prcc_pclk_enable()/clk_prcc_pclk_disable() for
enable/disable respectively.

These will just write a PRCC register.

And prepare() and unprepare() are not implemented for
this clock.

So far we can conclude that clk_enable()/disable()
will indeed achieve the desired effect of gating the
clock to the GPIO block per se, so we are saving
some power for sure.

However the prepare()/unprepare() calls will of course
also accumulate upwards and in e.g. the example of
"gpio.0" and "gpio.1" the parent is "per1clk" which is the
clock for the entire peripheral group.

(At this point I can stick in a reminder of the idea to
restructure the device tree in peripheral groups, because
that exercise will certainly pay off the day we try to encode
clocks in the device tree, I think the point can be clearly
seen as we proceed...)

This "per1clk" is registered using clk_req_prcmu_gate()
from clk-prcmu.c.

Looking at that clock type we find it's a plain software
dummy for the clk_enable()/clk_disable() path.
The real action is happening in the prepare()/unprepare()
path.

So to be able to shut down the entire peripheral cluster,
indeed both functions need to be called.

So in accordance with this we can see that the patch
should be applied, in some form.

However it is not removing the initial clk_prepare()
so the entire patch will be pointless, the prepare count
will currently always be > 0.

I'll mangle Lee's patch a bit, hold on..

Yours,
Linus Walleij

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

* Re: [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver
  2012-10-24 14:45 ` [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver Lee Jones
  2012-10-24 17:32   ` Linus Walleij
@ 2012-10-25 12:41   ` Linus Walleij
  2012-10-25 13:07     ` Lee Jones
  2012-10-25 13:13     ` Linus Walleij
  1 sibling, 2 replies; 27+ messages in thread
From: Linus Walleij @ 2012-10-25 12:41 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij, Ulf Hansson

On Wed, Oct 24, 2012 at 4:45 PM, Lee Jones <lee.jones@linaro.org> wrote:

> The clock framework has changed somewhat and it's now better to
> invoke clock_prepare_enable() and clk_disable_unprepare() rather
> than the legacy clk_enable() and clk_disable() calls. This patch
> converts the Nomadik Pin Control driver to the new framework.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

I was convinced that this is a good change but no regression,
so applied to the devel branch for 3.8.

I also removed the initial clk_prepare() so the reference count
may actually go down to 0 for the GPIO block and the peripheral
cluster eventually gets relaxed.

Thanks!
Linus Walleij

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

* Re: [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver
  2012-10-25 12:41   ` Linus Walleij
@ 2012-10-25 13:07     ` Lee Jones
  2012-10-25 13:13     ` Linus Walleij
  1 sibling, 0 replies; 27+ messages in thread
From: Lee Jones @ 2012-10-25 13:07 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij, Ulf Hansson

On Thu, 25 Oct 2012, Linus Walleij wrote:

> On Wed, Oct 24, 2012 at 4:45 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > The clock framework has changed somewhat and it's now better to
> > invoke clock_prepare_enable() and clk_disable_unprepare() rather
> > than the legacy clk_enable() and clk_disable() calls. This patch
> > converts the Nomadik Pin Control driver to the new framework.
> >
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> I was convinced that this is a good change but no regression,
> so applied to the devel branch for 3.8.
> 
> I also removed the initial clk_prepare() so the reference count
> may actually go down to 0 for the GPIO block and the peripheral
> cluster eventually gets relaxed.

Nice. Thanks Linus.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver
  2012-10-25 12:41   ` Linus Walleij
  2012-10-25 13:07     ` Lee Jones
@ 2012-10-25 13:13     ` Linus Walleij
  2012-10-25 15:51       ` Lee Jones
  1 sibling, 1 reply; 27+ messages in thread
From: Linus Walleij @ 2012-10-25 13:13 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij, Ulf Hansson

On Thu, Oct 25, 2012 at 2:41 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Oct 24, 2012 at 4:45 PM, Lee Jones <lee.jones@linaro.org> wrote:
>
>> The clock framework has changed somewhat and it's now better to
>> invoke clock_prepare_enable() and clk_disable_unprepare() rather
>> than the legacy clk_enable() and clk_disable() calls. This patch
>> converts the Nomadik Pin Control driver to the new framework.
>>
>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>
> I was convinced that this is a good change but no regression,
> so applied to the devel branch for 3.8.
>
> I also removed the initial clk_prepare() so the reference count
> may actually go down to 0 for the GPIO block and the peripheral
> cluster eventually gets relaxed.

Famous last words!

The good news is that this actually works, and the refcount
*does* go down to zero and gate off entire peripheral
clusters.

However that was not good because something vital in
some peripheral cluster died and killed the system :-D

Lee, could to to track down the reason and fix it so the patch
can be applied?

The only thing you need to do is to remove the superfluous
clk_prepare() right after the devm_clk_get() that hogs each
peripheral cluster.

Probably some driver is needing a clk_get() or a clk_get_sys() is
needs to be added somewhere to bring up some vital cluster,
or there may be some out-of-tree driver needed to bring up the
cluster properly I have no clue... Maybe some cluster just
cannot be declocked like that.

Yours,
Linus Walleij

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

* Re: [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver
  2012-10-25 13:13     ` Linus Walleij
@ 2012-10-25 15:51       ` Lee Jones
  2012-10-25 16:03         ` Linus Walleij
  0 siblings, 1 reply; 27+ messages in thread
From: Lee Jones @ 2012-10-25 15:51 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij, Ulf Hansson

On Thu, 25 Oct 2012, Linus Walleij wrote:

> On Thu, Oct 25, 2012 at 2:41 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Wed, Oct 24, 2012 at 4:45 PM, Lee Jones <lee.jones@linaro.org> wrote:
> >
> >> The clock framework has changed somewhat and it's now better to
> >> invoke clock_prepare_enable() and clk_disable_unprepare() rather
> >> than the legacy clk_enable() and clk_disable() calls. This patch
> >> converts the Nomadik Pin Control driver to the new framework.
> >>
> >> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >
> > I was convinced that this is a good change but no regression,
> > so applied to the devel branch for 3.8.
> >
> > I also removed the initial clk_prepare() so the reference count
> > may actually go down to 0 for the GPIO block and the peripheral
> > cluster eventually gets relaxed.
> 
> Famous last words!
> 
> The good news is that this actually works, and the refcount
> *does* go down to zero and gate off entire peripheral
> clusters.
> 
> However that was not good because something vital in
> some peripheral cluster died and killed the system :-D
> 
> Lee, could to to track down the reason and fix it so the patch
> can be applied?
> 
> The only thing you need to do is to remove the superfluous
> clk_prepare() right after the devm_clk_get() that hogs each
> peripheral cluster.
> 
> Probably some driver is needing a clk_get() or a clk_get_sys() is
> needs to be added somewhere to bring up some vital cluster,
> or there may be some out-of-tree driver needed to bring up the
> cluster properly I have no clue... Maybe some cluster just
> cannot be declocked like that.

I leave work in 10 mins and won't be coding again for ~2.5 weeks.
So if this is something you could squeeze in and fix-up, I'd be
very grateful.

Kind regards,
Lee

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver
  2012-10-25 15:51       ` Lee Jones
@ 2012-10-25 16:03         ` Linus Walleij
  2012-11-14 13:18           ` Lee Jones
  0 siblings, 1 reply; 27+ messages in thread
From: Linus Walleij @ 2012-10-25 16:03 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij, Ulf Hansson

On Thu, Oct 25, 2012 at 5:51 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Thu, 25 Oct 2012, Linus Walleij wrote:

>> Probably some driver is needing a clk_get() or a clk_get_sys() is
>> needs to be added somewhere to bring up some vital cluster,
>> or there may be some out-of-tree driver needed to bring up the
>> cluster properly I have no clue... Maybe some cluster just
>> cannot be declocked like that.
>
> I leave work in 10 mins and won't be coding again for ~2.5 weeks.
> So if this is something you could squeeze in and fix-up, I'd be
> very grateful.

I'll try. It doesn't look right that a clk_prepare() in the pinctrl
driver is saving the day for somebody else...

Yours,
Linus Walleij

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

* Re: [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver
  2012-10-25 16:03         ` Linus Walleij
@ 2012-11-14 13:18           ` Lee Jones
  2012-11-14 14:37             ` Linus Walleij
  0 siblings, 1 reply; 27+ messages in thread
From: Lee Jones @ 2012-11-14 13:18 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij, Ulf Hansson

On Thu, 25 Oct 2012, Linus Walleij wrote:

> On Thu, Oct 25, 2012 at 5:51 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Thu, 25 Oct 2012, Linus Walleij wrote:
> 
> >> Probably some driver is needing a clk_get() or a clk_get_sys() is
> >> needs to be added somewhere to bring up some vital cluster,
> >> or there may be some out-of-tree driver needed to bring up the
> >> cluster properly I have no clue... Maybe some cluster just
> >> cannot be declocked like that.
> >
> > I leave work in 10 mins and won't be coding again for ~2.5 weeks.
> > So if this is something you could squeeze in and fix-up, I'd be
> > very grateful.
> 
> I'll try. It doesn't look right that a clk_prepare() in the pinctrl
> driver is saving the day for somebody else...

Hi Linus,

Did you apply this in the end?

Kind regards,
Lee

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver
  2012-11-14 13:18           ` Lee Jones
@ 2012-11-14 14:37             ` Linus Walleij
  0 siblings, 0 replies; 27+ messages in thread
From: Linus Walleij @ 2012-11-14 14:37 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij, Ulf Hansson

On Wed, Nov 14, 2012 at 2:18 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Thu, 25 Oct 2012, Linus Walleij wrote:
>
>> On Thu, Oct 25, 2012 at 5:51 PM, Lee Jones <lee.jones@linaro.org> wrote:
>> > On Thu, 25 Oct 2012, Linus Walleij wrote:
>>
>> >> Probably some driver is needing a clk_get() or a clk_get_sys() is
>> >> needs to be added somewhere to bring up some vital cluster,
>> >> or there may be some out-of-tree driver needed to bring up the
>> >> cluster properly I have no clue... Maybe some cluster just
>> >> cannot be declocked like that.
>> >
>> > I leave work in 10 mins and won't be coding again for ~2.5 weeks.
>> > So if this is something you could squeeze in and fix-up, I'd be
>> > very grateful.
>>
>> I'll try. It doesn't look right that a clk_prepare() in the pinctrl
>> driver is saving the day for somebody else...
>
> Hi Linus,
>
> Did you apply this in the end?

No I can't. If I fix the bug in the patch (removing the surplus
clk_prepare() in the probe function) the thing regresses the
entire kernel, so I have no patch which actually is working.

It needs to be root-caused and fixed properly...

Yours,
Linus Walleij

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

end of thread, other threads:[~2012-11-14 14:37 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-24 14:45 [PATCH 0/6] ux500 fixes bound for the -rcs Lee Jones
2012-10-24 14:45 ` [PATCH 1/6] mfd: ab8500-core: Remove unused ab8500-gpio IRQ ranges Lee Jones
2012-10-24 17:37   ` Linus Walleij
2012-10-24 14:45 ` [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver Lee Jones
2012-10-24 17:32   ` Linus Walleij
2012-10-25  7:31     ` Lee Jones
2012-10-25  7:57       ` Linus Walleij
2012-10-25  8:23         ` Lee Jones
2012-10-25  9:29           ` Ulf Hansson
2012-10-25  9:44             ` Lee Jones
2012-10-25 12:33             ` Linus Walleij
2012-10-25 12:07           ` Linus Walleij
2012-10-25 12:41   ` Linus Walleij
2012-10-25 13:07     ` Lee Jones
2012-10-25 13:13     ` Linus Walleij
2012-10-25 15:51       ` Lee Jones
2012-10-25 16:03         ` Linus Walleij
2012-11-14 13:18           ` Lee Jones
2012-11-14 14:37             ` Linus Walleij
2012-10-24 14:45 ` [PATCH 3/6] ARM: ux500: Fix build error relating to IRQCHIP_SKIP_SET_WAKE Lee Jones
2012-10-24 14:45 ` [PATCH 4/6] ARM: ux500: Specify AMBA Primecell IDs for Nomadik I2C in DT Lee Jones
2012-10-24 17:47   ` Linus Walleij
2012-10-24 14:45 ` [PATCH 5/6] ARM: ux500: Correct SDI5 address and add some format changes Lee Jones
2012-10-24 17:48   ` Linus Walleij
2012-10-24 14:45 ` [PATCH 6/6] ARM: ux500: Convert DT_MACHINE_START to use SMP operations Lee Jones
2012-10-24 17:48   ` Linus Walleij
2012-10-24 17:46 ` [PATCH 0/6] ux500 fixes bound for the -rcs Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).