Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/6] mfd: ab8500-core: Remove unused ab8500-gpio IRQ ranges
From: Linus Walleij @ 2012-10-24 17:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1351089926-32161-2-git-send-email-lee.jones@linaro.org>

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

* [PATCH v2 2/2] USB: doc: Binding document for ehci-platform driver
From: Florian Fainelli @ 2012-10-24 17:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <Pine.LNX.4.44L0.1210241249010.1282-100000@iolanthe.rowland.org>

On Wednesday 24 October 2012 12:54:05 Alan Stern wrote:
> On Wed, 24 Oct 2012, Stephen Warren wrote:
> 
> > Device tree files always need a completely specific value in the
> > compatible property, even when less-specific/more-generic values are
> > also present. So for example, the Linux driver might only care about the
> > following existing:
> > 
> > compatible = "usb-ehci".
> > 
> > However, any actual EHCI controller is always some specific model, so
> > the compatible value must define which specific model it is, e.g.:
> > 
> > compatible = "nvidia,tegra20-ehci", "usb-ehci".
> > 
> > Now lets say the Tegra30 EHCI controller is identical to the Tegra20
> > controller. That needs to be explicitly represented too:
> > 
> > compatible = "nvidia,tegra30-ehci", "nvidia,tegra20-ehci", "usb-ehci".
> > 
> > In that case, the driver might still only declare support for
> > "nvidia,tegra20-ehci", but "nvidia,tegra30-ehci" is added to be explicit
> > about the actual HW model.
> > 
> > This doesn't continue forever though; you typically only list the
> > specific HW model, the base specific model it's compatible with, and any
> > generic value needed. So, a hypothetical Tegra40 EHCI controller would be:
> > 
> > compatible = "nvidia,tegra40-ehci", "nvidia,tegra20-ehci", "usb-ehci".
> > 
> > Given that, there is always something to key any newly required quirk
> > off, so even if the need for a quirk is found later, the device tree
> > already contains the information needed to trigger it.
> 
> Okay.  It appears that quite a few .dts/.dtsi files mention "usb-ehci", 
> for no apparent reason (given that the drivers don't list it):
> 
> [stern at iolanthe usb-3.6]$ find arch -name '*.dts*' | xargs grep usb-ehci
> arch/mips/cavium-octeon/octeon_3xxx.dts:                                compatible = "cavium,octeon-6335-ehci","usb-ehci";
> arch/mips/cavium-octeon/octeon_68xx.dts:                                compatible = "cavium,octeon-6335-ehci","usb-ehci";
> arch/arm/boot/dts/tegra20.dtsi:         compatible = "nvidia,tegra20-ehci", "usb-ehci";
> arch/arm/boot/dts/tegra20.dtsi:         compatible = "nvidia,tegra20-ehci", "usb-ehci";
> arch/arm/boot/dts/tegra20.dtsi:         compatible = "nvidia,tegra20-ehci", "usb-ehci";
> arch/arm/boot/dts/at91sam9x5.dtsi:                      compatible = "atmel,at91sam9g45-ehci", "usb-ehci";
> arch/arm/boot/dts/spear13xx.dtsi:                       compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/spear13xx.dtsi:                       compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/spear3xx.dtsi:                        compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/spear600.dtsi:                        compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/spear600.dtsi:                        compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/at91sam9g45.dtsi:                     compatible = "atmel,at91sam9g45-ehci", "usb-ehci";
> arch/powerpc/boot/dts/wii.dts:                  compatible = "nintendo,hollywood-usb-ehci",
> arch/powerpc/boot/dts/wii.dts:                                  "usb-ehci";
> arch/powerpc/boot/dts/sequoia.dts:                      compatible = "ibm,usb-ehci-440epx", "usb-ehci";
> arch/powerpc/boot/dts/canyonlands.dts:                  compatible = "ibm,usb-ehci-460ex", "usb-ehci";
> 
> Is there a real reason that I'm not aware of?  Or can all these entries
> safely be removed?

Apart from the powerpc entries for which a real driver exists, the others were
probably added in the hope that sooner or later, someone will come up with
a matching driver, and the corresponding dts file would not even have to be
updated to benefit from this.
--
Florian

^ permalink raw reply

* [PATCH v3] pwm: vt8500: Update vt8500 PWM driver support
From: Tony Prisk @ 2012-10-24 17:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121024054114.GA9787@avionic-0098.mockup.avionic-design.de>

On Wed, 2012-10-24 at 07:41 +0200, Thierry Reding wrote:
> On Wed, Oct 24, 2012 at 04:46:58PM +1300, Tony Prisk wrote:
> > This patch updates pwm-vt8500.c to support devicetree probing and
> > make use of the common clock subsystem.
> > 
> > A binding document describing the PWM controller found on
> > arch-vt8500 is also included.
> > 
> > Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
> > ---
> > v2/v3:
> > Fix errors/coding style as pointed out by Thierry Reding.
> > 
> >  .../devicetree/bindings/pwm/vt8500-pwm.txt         |   17 ++++
> >  drivers/pwm/pwm-vt8500.c                           |   86 ++++++++++++++------
> >  2 files changed, 80 insertions(+), 23 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/pwm/vt8500-pwm.txt
> 
> Looking real good now. One last comment though and I think I'm ready to
> take this...
> 
> > +	err = clk_enable(vt8500->clk);
> > +	if (err < 0)
> > +		dev_err(chip->dev, "failed to enable clock\n");
> > +		return -EBUSY;
> > +	};
> 
> Why do you return EBUSY instead of err?

Because I didn't notice this when I 'fixed' it - I changed the other one
to return err/ret, missed this one. Will fix.
> 
> Thierry
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCHv2] Input: omap4-keypad: Add pinctrl support
From: Dmitry Torokhov @ 2012-10-24 17:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121024165215.GA32220@arwen.pp.htv.fi>

On Wednesday, October 24, 2012 07:52:16 PM Felipe Balbi wrote:
> Hi,
> 
> On Wed, Oct 24, 2012 at 09:14:29AM -0700, Dmitry Torokhov wrote:
> 
> <snip>
> 
> > > > No, I guess we ihave different understanding of what "directly use"
> > > > means.
> > > > This particular driver does directly use interrupts: it requests it
> > > > and
> > > > performs some actions when interrupt arrives. Same goes for IO memory
> > > > -
> > > > it gets requested, then we access it. With pinctrl we do not do
> > > > anything
> > > > - we just ask another layer to configure it and that is it.
> > > 
> > > this is true for almost anything we do:
> > > 
> > > - we ask another layer to allocate memory for us
> > > - we ask another layer to call our ISR once the IRQ line is asserted
> > > - we ask another layer to handle the input events we just received
> > > - we ask another layer to transfer data through DMA for us
> > > - we ask another layer to turn regulators on and off.
> > 
> > But we are _directly_ _using_ all of these. You allocate memory and you
> > (the driver) stuff data into that memory. You ask for DMA and you take
> > the DMAed data and work with it. Not so with pinctrl in omap keypad and
> > other drivers I have seen so far.
> 
> of course we are. If we don't mux the pins to their correct setting, we
> can't realy use the HW. So while you don't see any SW control of the
> requested pins, we're still making use of them.

So we make use of CPU too, and the main power supply, and memory chips.

> 
> > > and so on. This is just how abstractions work, we group common parts in
> > > a framework so that users don't need to know the details, but still need
> > > to tell the framework when to fiddle with those resources.
> > > 
> > > > I have seen just in a few days 3 or 4 drivers having exactly the same
> > > > change - call to devm_pinctrl_get_select_default(), and I guess I will
> > > > receive the same patches for the rest of input drivers shortly.
> > > > This suggests that the operation is done at the wrong level. Do the
> > > > pin configuration as you parse DT data, the same way you set up i2c
> > > > devices registers in of_i2c.c, and leave the individual drivers that
> > > > do
> > > > not care about specifics alone.
> > > 
> > > Makes no sense to hide that from drivers. The idea here is that driver
> > > should know when it needs its pins to muxed correctly.
> > 
> > The driver also needs memory controller to be initialized, gpio chip be
> > ready and registered, DMA subsystem ready, input core reade, etc, etc,
> > etc. You however do not have every driver explicitly initialize any of
> > that; you expect certain working environment to be already operable. The
> > driver does manage resources it controls, it has ultimate knowledge
> > about, pin configuration is normally is not it. We just need to know
> > that we wired/muxed properly, otherwise we won't work. So please let
> > parent layers deal with it.
> > 
> > > 95% of the time
> > > it will be done during probe() but then again, so what ?
> > > 
> > > doing that when parsing DT, or on bus notifiers is just plain wrong.
> > > Drivers should be required to handle all of their resources.
> > 
> > All of _their_ resources, exactly. They do not own nor control pins so
> > they should not be bothered with them either. Look, when you see that
> 
> except that they *own* the pins. Now that the muxer has been setup
> properly, this particular IP owns the pins.
> 
> > potentially _every_ driver in the system needs to set up the same object
> > that it doe snot use otherwise you should realize that individual driver
> > is not the proper place to do that.
> 
> fair enough, but IMHO, we're not there yet. We can't make that claim
> yet. Besides, we don't know what's the default pin state in a system. It
> might be that certain pins start out in a way which consumes less power
> due to the internal construction of the SoC. If we set pins up before
> driver probes, and probe fails or the driver is never really used, then
> we could be falling into a situation where we're wasting power.

So what about moving this into bus code - have bus's probe() request
default pin config before probe, revert to original setup when unbinding
or probe fails. You can even plug PM switching into bus code as well.

> 
> Granted, you can undo everything you did before,

Right, the same way as we undo every other initialization when something
goes wrong.

> but I guess keeping
> track of everything we setup before probe() just to remove a couple of
> lines from drivers is wrong.

If it was just about a couple lines in a couple of drivers that would
be fine. But the way I see it eventually every driver will need to do
this.

> 
> > > > > That's why it is named "get_select_default" and not "get" only.
> > > > > This API ensure that the pin will be set to the default state, and
> > > > > this
> > > > > is all we need to do during the probe.
> > > > 
> > > > Why during the probe and not by default? Realistically, the driver
> > > > will
> > > > be loaded as long as there is a matching device and pins will need to
> > > > be
> > > > configured.
> > > 
> > > likewise memory will be allocated when matching happens, IRQs will be
> > > allocated, regulators will be turned on. So why don't we do all that by
> > > default ? Because it is wrong.
> > 
> > No, because we do not know how. The generic layer does not know the ISR
> > to install, how much memory to allocate, etc. Having regulator turned on
> > before getting to probe might not be a bad idea.
> 
> what if your driver never probes ? Will you really leave regulators on
> consuming extra, valuable power ?

If we do it right in probe() we won't consume unless the dirver is bound.

> 
> > > > > There is no point to change the mux if the driver is not probed,
> > > > > because
> > > > > potentially the pin can be use be another driver.
> > > > 
> > > > What other driver would use it? Who would chose what driver to load?
> > > 
> > > Well, you _do_ know that on a SoC we have a limited amount of pins
> > > right ?
> > > 
> > > Considering the amont of features which are packed inside a single die,
> > > it's not farfetched to assume we will have a lot less pins then we
> > > actually need, so we need muxers behind each pin in order to choose
> > > which functionality we want.
> > > 
> > > If it happens that keypad's pins are shared with another IP (e.g. GPIO),
> > > we need to give the final user (a OEM/ODM) the choice of using those
> > > pins as either keypad or GPIOs, thus the need for pinctrl framework and
> > > the calls in the drivers.
> > 
> > Right, so please walk me through, step by step, how an OEM/ODM woudl
> > select a particular configuration. Do you expect it to happen at
> > runtime, or do you expect relevant data be put in DT?
> 
> It depends, I've seen both happening, really. Also note that DT
> migration is still not complete, meaning that most (all ?) OEM/ODMs are
> still using the legacy board-file-based approach and it will still take
> them a few years to move to DT-based boot.
> 
> Another point to consider is community boards such as beaglebone which
> have tens of different "capes" to support. Depending on the cape, pins
> might have to be remuxed, so instead of adding all that code to platform
> support, just leave it all in drivers. Depending on the "cape" different
> drivers will probe() and those drivers should know how to mux pins for
> themselves.
> 
> Note that these are only the two easy examples that came to my mind, I'm
> sure we can discuss this for a long, but is it valid ? For a single line
> of code ?

Multiply by hundred of drivers - yes.

-- 
Dmitry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121024/f5b1dd36/attachment.sig>

^ permalink raw reply

* [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver
From: Linus Walleij @ 2012-10-24 17:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1351089926-32161-3-git-send-email-lee.jones@linaro.org>

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

* [PATCHv2] Input: omap4-keypad: Add pinctrl support
From: Dmitry Torokhov @ 2012-10-24 17:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CACRpkdai3NocZQnW6YRF+k6GXUSx3P_Ge=F-tkuivrFm6tBbLg@mail.gmail.com>

On Wednesday, October 24, 2012 06:51:47 PM Linus Walleij wrote:
> On Wed, Oct 24, 2012 at 6:14 PM, Dmitry Torokhov
> 
> <dmitry.torokhov@gmail.com> wrote:
> > On Wed, Oct 24, 2012 at 11:37:04AM +0300, Felipe Balbi wrote:
> >> - we ask another layer to allocate memory for us
> >> - we ask another layer to call our ISR once the IRQ line is asserted
> >> - we ask another layer to handle the input events we just received
> >> - we ask another layer to transfer data through DMA for us
> >> - we ask another layer to turn regulators on and off.
> > 
> > But we are _directly_ _using_ all of these. You allocate memory and you
> > (the driver) stuff data into that memory. You ask for DMA and you take
> > the DMAed data and work with it. Not so with pinctrl in omap keypad and
> > other drivers I have seen so far.
> 
> Consult:
> drivers/tty/serial/amba-pl011.c

OK.

> drivers/spi/spi-pl022.c

Default/sleep transitions could be moved into bus code.

> drivers/i2c/busses/i2c-nomadik.c

Don't see pinctrl in linux-next.

 
> for more complex pinctrl use cases. These are my dogfood drivers ...
> Most of these will request more than one state and switch the driver
> between these different states at runtime, in these examples for power
> saving there are states named "default", "sleep" and in the I2C driver
> also "idle".
> 
> These examples are more typical to how the ux500 platform will
> look, also the SKE input driver will move the devise to sleep/default
> states but we need to merge PM code before we can do that.

I do not say that no drivers should ever touch pinctrl, just that most
of them do not have to if you have other layers to the right thing for
them.

Thanks.

-- 
Dmitry

^ permalink raw reply

* [PATCH 0/2] clk: ux500: Make mtu driver use apb_pclock
From: Linus Walleij @ 2012-10-24 17:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1351080821-18660-1-git-send-email-ulf.hansson@stericsson.com>

On Wed, Oct 24, 2012 at 2:13 PM, Ulf Hansson <ulf.hansson@stericsson.com> wrote:

> From: Ulf Hansson <ulf.hansson@linaro.org>
>
> The apb clock was before the "common" clock driver for ux500 was merged,
> handled internally by the clock driver. Now this clock needs to be managed
> from the mtu driver as a separate clock.
>
> This patches is based in 3.7 rc2.
>
> It is important the "ARM nomadik patch" is merged together with the
> clock patch to not break boot. Therefore I suggest this series to go
> through Mike Turquettes clock tree.

OK go ahead:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply

* [PATCHv2] ARM: Push selects for TWD/SCU into machine entries
From: Stephen Boyd @ 2012-10-24 17:24 UTC (permalink / raw)
  To: linux-arm-kernel

The TWD and SCU configs are selected by default as long as
MSM_SCORPIONMP is false and/or MCT is false. Implementing the
logic this way certainly saves lines in the Kconfig but it
precludes those machines which select MSM_SCORPIONMP or MCT from
participating in the single zImage effort because when those
machines are combined with other SMP capable machines the TWD and
SCU are no longer selected by default.

Push the select out to the machine entries so that we can compile
these machines together and still select the appropriate configs.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Cc: David Brown <davidb@codeaurora.org>
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Pawel Moll <pawel.moll@arm.com>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Russell King <linux@arm.linux.org.uk>
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Shiraz Hashim <shiraz.hashim@st.com>
Acked-by: Simon Horman <horms@verge.net.au>
Cc: Srinidhi Kasagar <srinidhi.kasagar@stericsson.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Tony Lindgren <tony@atomide.com>
Acked-by: Viresh Kumar <viresh.linux@gmail.com>
---

Changes since v1:
 * Dropped OMAP5 hunk
 * alphabetical order

 arch/arm/Kconfig               | 8 ++++++--
 arch/arm/mach-exynos/Kconfig   | 2 ++
 arch/arm/mach-highbank/Kconfig | 1 +
 arch/arm/mach-imx/Kconfig      | 3 ++-
 arch/arm/mach-msm/Kconfig      | 7 ++-----
 arch/arm/mach-omap2/Kconfig    | 2 ++
 arch/arm/mach-realview/Kconfig | 8 ++++++++
 arch/arm/mach-vexpress/Kconfig | 2 ++
 arch/arm/plat-spear/Kconfig    | 2 ++
 9 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index fe90e60..53112d0 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -633,6 +633,8 @@ config ARCH_TEGRA
 	select GENERIC_GPIO
 	select HAVE_CLK
 	select HAVE_SMP
+	select HAVE_ARM_SCU if SMP
+	select HAVE_ARM_TWD if LOCAL_TIMERS
 	select MIGHT_HAVE_CACHE_L2X0
 	select SPARSE_IRQ
 	select USE_OF
@@ -677,6 +679,8 @@ config ARCH_SHMOBILE
 	bool "Renesas SH-Mobile / R-Mobile"
 	select CLKDEV_LOOKUP
 	select GENERIC_CLOCKEVENTS
+	select HAVE_ARM_SCU if SMP
+	select HAVE_ARM_TWD if LOCAL_TIMERS
 	select HAVE_CLK
 	select HAVE_MACH_CLKDEV
 	select HAVE_SMP
@@ -875,6 +879,8 @@ config ARCH_U8500
 	select CPU_V7
 	select GENERIC_CLOCKEVENTS
 	select HAVE_SMP
+	select HAVE_ARM_SCU if SMP
+	select HAVE_ARM_TWD if LOCAL_TIMERS
 	select MIGHT_HAVE_CACHE_L2X0
 	help
 	  Support for ST-Ericsson's Ux500 architecture
@@ -1490,7 +1496,6 @@ config SMP
 	depends on GENERIC_CLOCKEVENTS
 	depends on HAVE_SMP
 	depends on MMU
-	select HAVE_ARM_SCU if !ARCH_MSM_SCORPIONMP
 	select USE_GENERIC_SMP_HELPERS
 	help
 	  This enables support for systems with more than one CPU. If you have
@@ -1604,7 +1609,6 @@ config LOCAL_TIMERS
 	bool "Use local timer interrupts"
 	depends on SMP
 	default y
-	select HAVE_ARM_TWD if (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT)
 	help
 	  Enable support for local timers on SMP platforms, rather then the
 	  legacy IPI broadcast method.  Local timers allows the system
diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index da55107..a8176d4 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -14,6 +14,7 @@ menu "SAMSUNG EXYNOS SoCs Support"
 config ARCH_EXYNOS4
 	bool "SAMSUNG EXYNOS4"
 	default y
+	select HAVE_ARM_SCU if SMP
 	select HAVE_SMP
 	select MIGHT_HAVE_CACHE_L2X0
 	help
@@ -21,6 +22,7 @@ config ARCH_EXYNOS4
 
 config ARCH_EXYNOS5
 	bool "SAMSUNG EXYNOS5"
+	select HAVE_ARM_SCU if SMP
 	select HAVE_SMP
 	help
 	  Samsung EXYNOS5 (Cortex-A15) SoC based systems
diff --git a/arch/arm/mach-highbank/Kconfig b/arch/arm/mach-highbank/Kconfig
index 0e1d0a4..d87ac5b 100644
--- a/arch/arm/mach-highbank/Kconfig
+++ b/arch/arm/mach-highbank/Kconfig
@@ -10,6 +10,7 @@ config ARCH_HIGHBANK
 	select CPU_V7
 	select GENERIC_CLOCKEVENTS
 	select HAVE_ARM_SCU
+	select HAVE_ARM_TWD if LOCAL_TIMERS
 	select HAVE_SMP
 	select SPARSE_IRQ
 	select USE_OF
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 892631f..3887fe2 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -833,7 +833,8 @@ config SOC_IMX6Q
 	select ARM_GIC
 	select COMMON_CLK
 	select CPU_V7
-	select HAVE_ARM_SCU
+	select HAVE_ARM_SCU if SMP
+	select HAVE_ARM_TWD if LOCAL_TIMERS
 	select HAVE_CAN_FLEXCAN if CAN
 	select HAVE_IMX_GPC
 	select HAVE_IMX_MMDC
diff --git a/arch/arm/mach-msm/Kconfig b/arch/arm/mach-msm/Kconfig
index b619085..fceb093 100644
--- a/arch/arm/mach-msm/Kconfig
+++ b/arch/arm/mach-msm/Kconfig
@@ -44,10 +44,10 @@ endchoice
 
 config ARCH_MSM8X60
 	bool "MSM8X60"
-	select ARCH_MSM_SCORPIONMP
 	select ARM_GIC
 	select CPU_V7
 	select GPIO_MSM_V2
+	select HAVE_SMP
 	select MSM_GPIOMUX
 	select MSM_SCM if SMP
 	select MSM_V2_TLMM
@@ -55,9 +55,9 @@ config ARCH_MSM8X60
 
 config ARCH_MSM8960
 	bool "MSM8960"
-	select ARCH_MSM_SCORPIONMP
 	select ARM_GIC
 	select CPU_V7
+	select HAVE_SMP
 	select MSM_GPIOMUX
 	select MSM_SCM if SMP
 	select MSM_V2_TLMM
@@ -68,9 +68,6 @@ config MSM_HAS_DEBUG_UART_HS
 
 config MSM_SOC_REV_A
 	bool
-config  ARCH_MSM_SCORPIONMP
-	bool
-	select HAVE_SMP
 
 config  ARCH_MSM_ARM11
 	bool
diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index 2a1a898..3221a20 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -62,6 +62,8 @@ config ARCH_OMAP4
 	select CACHE_L2X0
 	select CPU_V7
 	select HAVE_SMP
+	select HAVE_ARM_SCU if SMP
+	select HAVE_ARM_TWD if LOCAL_TIMERS
 	select LOCAL_TIMERS if SMP
 	select OMAP_INTERCONNECT
 	select PL310_ERRATA_588369
diff --git a/arch/arm/mach-realview/Kconfig b/arch/arm/mach-realview/Kconfig
index 14c1d47..d210c0f9 100644
--- a/arch/arm/mach-realview/Kconfig
+++ b/arch/arm/mach-realview/Kconfig
@@ -12,6 +12,8 @@ config REALVIEW_EB_A9MP
 	bool "Support Multicore Cortex-A9 Tile"
 	depends on MACH_REALVIEW_EB
 	select CPU_V7
+	select HAVE_ARM_SCU if SMP
+	select HAVE_ARM_TWD if LOCAL_TIMERS
 	select HAVE_SMP
 	select MIGHT_HAVE_CACHE_L2X0
 	help
@@ -23,6 +25,8 @@ config REALVIEW_EB_ARM11MP
 	depends on MACH_REALVIEW_EB
 	select ARCH_HAS_BARRIERS if SMP
 	select CPU_V6K
+	select HAVE_ARM_SCU if SMP
+	select HAVE_ARM_TWD if LOCAL_TIMERS
 	select HAVE_SMP
 	select MIGHT_HAVE_CACHE_L2X0
 	help
@@ -43,6 +47,8 @@ config MACH_REALVIEW_PB11MP
 	select ARCH_HAS_BARRIERS if SMP
 	select ARM_GIC
 	select CPU_V6K
+	select HAVE_ARM_SCU if SMP
+	select HAVE_ARM_TWD if LOCAL_TIMERS
 	select HAVE_PATA_PLATFORM
 	select HAVE_SMP
 	select MIGHT_HAVE_CACHE_L2X0
@@ -85,6 +91,8 @@ config MACH_REALVIEW_PBX
 	bool "Support RealView(R) Platform Baseboard Explore"
 	select ARCH_SPARSEMEM_ENABLE if CPU_V7 && !REALVIEW_HIGH_PHYS_OFFSET
 	select ARM_GIC
+	select HAVE_ARM_SCU if SMP
+	select HAVE_ARM_TWD if LOCAL_TIMERS
 	select HAVE_PATA_PLATFORM
 	select HAVE_SMP
 	select MIGHT_HAVE_CACHE_L2X0
diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
index c952960..96b1101 100644
--- a/arch/arm/mach-vexpress/Kconfig
+++ b/arch/arm/mach-vexpress/Kconfig
@@ -8,6 +8,8 @@ config ARCH_VEXPRESS
 	select COMMON_CLK
 	select CPU_V7
 	select GENERIC_CLOCKEVENTS
+	select HAVE_ARM_SCU if SMP
+	select HAVE_ARM_TWD if LOCAL_TIMERS
 	select HAVE_CLK
 	select HAVE_PATA_PLATFORM
 	select HAVE_SMP
diff --git a/arch/arm/plat-spear/Kconfig b/arch/arm/plat-spear/Kconfig
index f8db7b2..a53936e 100644
--- a/arch/arm/plat-spear/Kconfig
+++ b/arch/arm/plat-spear/Kconfig
@@ -12,6 +12,8 @@ config ARCH_SPEAR13XX
 	bool "ST SPEAr13xx with Device Tree"
 	select ARM_GIC
 	select CPU_V7
+	select HAVE_ARM_SCU if SMP
+	select HAVE_ARM_TWD if LOCAL_TIMERS
 	select HAVE_SMP
 	select MIGHT_HAVE_CACHE_L2X0
 	select PINCTRL
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH] ARM: PMU: fix runtime PM enable
From: Will Deacon @ 2012-10-24 17:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <508803DF.7020902@ti.com>

On Wed, Oct 24, 2012 at 04:06:07PM +0100, Jon Hunter wrote:
> On 10/24/2012 09:32 AM, Will Deacon wrote:
> > Hmmm, now I start to wonder whether your original idea of having separate
> > callbacks for enable/disable irq and resume/suspend doesn't make more sense.
> > Then the CTI magic can go in the irq management code and be totally separate
> > to the PM stuff.
> > 
> > What do you reckon?
> 
> The resume/suspend calls really replaced the enable/disable irq
> callbacks. That still seems like a good approach given that we need
> runtime PM for OMAP and PMU.

Ok, perhaps splitting it up isn't worth it then. I'm still not convinced
either way.

> > Nah, we should be able to fix this in the platdata, I'd just rather have
> > function pointers instead of state variables in there.
> 
> Well, we could pass a pointer to pm_runtime_enable() function in the
> platdata.

What do other drivers do? Grepping around, I see calls to pm_runtime_enable
made in various drivers and, given that you pass the device in there, what's
the problem with us just calling that unconditionally from perf? I know you
said that will work for OMAP, but I'm trying to understand the effect that
has on PM-aware platforms that don't require this for the PMU (since this
seems to be per-device).

Will

^ permalink raw reply

* [PATCH] ARM: dts: OMAP: Move interrupt-parent to the root node to avoid duplication
From: Hiremath, Vaibhav @ 2012-10-24 17:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5087BCA6.1020602@ti.com>

On Wed, Oct 24, 2012 at 15:32:14, Cousson, Benoit wrote:
> The interrupt-parent attribute does not have to be added in each
> node since the DT framework will check for the parent as well to get it.
> 
> Create an interrupt-parent for OMAP2, OMAP3, AM33xx and remove the
> attributes from every nodes that were using it.
> 
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Vaibhav Hiremath <hvaibhav@ti.com>
> Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Cc: Sebastien Guiriec <s-guiriec@ti.com>
> ---
> 
> This patch was triggered by the thread about Seb's patch [1].
> So let's clean the current OMAP/AM stuff right now to be consistent.
> 
> The patch applies on top of the for_3.8/dts [2] branch.
> 
> Regards,
> Benoit
> 
> [1] https://lkml.org/lkml/2012/10/24/85
> [2] git://git.kernel.org/pub/scm/linux/kernel/git/bcousson/linux-omap-dt.git
> 
> 
>  arch/arm/boot/dts/am33xx.dtsi   |   17 +----------------
>  arch/arm/boot/dts/omap2.dtsi    |    1 +
>  arch/arm/boot/dts/omap2420.dtsi |    2 --
>  arch/arm/boot/dts/omap2430.dtsi |    5 -----
>  arch/arm/boot/dts/omap3.dtsi    |    6 +-----
>  arch/arm/boot/dts/omap4.dtsi    |    6 ------
>  arch/arm/boot/dts/omap5.dtsi    |    5 -----
>  7 files changed, 3 insertions(+), 39 deletions(-)
> 

Looks good to me.

Acked-by: Vaibhav Hiremath <hvaibhav@ti.com>

Thanks,
Vaibhav

> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index 64c2efe..4709269 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -12,6 +12,7 @@
>  
>  / {
>  	compatible = "ti,am33xx";
> +	interrupt-parent = <&intc>;
>  
>  	aliases {
>  		serial0 = &uart1;
> @@ -94,7 +95,6 @@
>  			interrupt-controller;
>  			#interrupt-cells = <1>;
>  			reg = <0x44e07000 0x1000>;
> -			interrupt-parent = <&intc>;
>  			interrupts = <96>;
>  		};
>  
> @@ -106,7 +106,6 @@
>  			interrupt-controller;
>  			#interrupt-cells = <1>;
>  			reg = <0x4804c000 0x1000>;
> -			interrupt-parent = <&intc>;
>  			interrupts = <98>;
>  		};
>  
> @@ -118,7 +117,6 @@
>  			interrupt-controller;
>  			#interrupt-cells = <1>;
>  			reg = <0x481ac000 0x1000>;
> -			interrupt-parent = <&intc>;
>  			interrupts = <32>;
>  		};
>  
> @@ -130,7 +128,6 @@
>  			interrupt-controller;
>  			#interrupt-cells = <1>;
>  			reg = <0x481ae000 0x1000>;
> -			interrupt-parent = <&intc>;
>  			interrupts = <62>;
>  		};
>  
> @@ -139,7 +136,6 @@
>  			ti,hwmods = "uart1";
>  			clock-frequency = <48000000>;
>  			reg = <0x44e09000 0x2000>;
> -			interrupt-parent = <&intc>;
>  			interrupts = <72>;
>  			status = "disabled";
>  		};
> @@ -149,7 +145,6 @@
>  			ti,hwmods = "uart2";
>  			clock-frequency = <48000000>;
>  			reg = <0x48022000 0x2000>;
> -			interrupt-parent = <&intc>;
>  			interrupts = <73>;
>  			status = "disabled";
>  		};
> @@ -159,7 +154,6 @@
>  			ti,hwmods = "uart3";
>  			clock-frequency = <48000000>;
>  			reg = <0x48024000 0x2000>;
> -			interrupt-parent = <&intc>;
>  			interrupts = <74>;
>  			status = "disabled";
>  		};
> @@ -169,7 +163,6 @@
>  			ti,hwmods = "uart4";
>  			clock-frequency = <48000000>;
>  			reg = <0x481a6000 0x2000>;
> -			interrupt-parent = <&intc>;
>  			interrupts = <44>;
>  			status = "disabled";
>  		};
> @@ -179,7 +172,6 @@
>  			ti,hwmods = "uart5";
>  			clock-frequency = <48000000>;
>  			reg = <0x481a8000 0x2000>;
> -			interrupt-parent = <&intc>;
>  			interrupts = <45>;
>  			status = "disabled";
>  		};
> @@ -189,7 +181,6 @@
>  			ti,hwmods = "uart6";
>  			clock-frequency = <48000000>;
>  			reg = <0x481aa000 0x2000>;
> -			interrupt-parent = <&intc>;
>  			interrupts = <46>;
>  			status = "disabled";
>  		};
> @@ -200,7 +191,6 @@
>  			#size-cells = <0>;
>  			ti,hwmods = "i2c1";
>  			reg = <0x44e0b000 0x1000>;
> -			interrupt-parent = <&intc>;
>  			interrupts = <70>;
>  			status = "disabled";
>  		};
> @@ -211,7 +201,6 @@
>  			#size-cells = <0>;
>  			ti,hwmods = "i2c2";
>  			reg = <0x4802a000 0x1000>;
> -			interrupt-parent = <&intc>;
>  			interrupts = <71>;
>  			status = "disabled";
>  		};
> @@ -222,7 +211,6 @@
>  			#size-cells = <0>;
>  			ti,hwmods = "i2c3";
>  			reg = <0x4819c000 0x1000>;
> -			interrupt-parent = <&intc>;
>  			interrupts = <30>;
>  			status = "disabled";
>  		};
> @@ -231,7 +219,6 @@
>  			compatible = "ti,omap3-wdt";
>  			ti,hwmods = "wd_timer2";
>  			reg = <0x44e35000 0x1000>;
> -			interrupt-parent = <&intc>;
>  			interrupts = <91>;
>  		};
>  
> @@ -240,7 +227,6 @@
>  			ti,hwmods = "d_can0";
>  			reg = <0x481cc000 0x2000>;
>  			interrupts = <52>;
> -			interrupt-parent = <&intc>;
>  			status = "disabled";
>  		};
>  
> @@ -249,7 +235,6 @@
>  			ti,hwmods = "d_can1";
>  			reg = <0x481d0000 0x2000>;
>  			interrupts = <55>;
> -			interrupt-parent = <&intc>;
>  			status = "disabled";
>  		};
>  	};
> diff --git a/arch/arm/boot/dts/omap2.dtsi b/arch/arm/boot/dts/omap2.dtsi
> index 581cb08..f366482 100644
> --- a/arch/arm/boot/dts/omap2.dtsi
> +++ b/arch/arm/boot/dts/omap2.dtsi
> @@ -12,6 +12,7 @@
>  
>  / {
>  	compatible = "ti,omap2430", "ti,omap2420", "ti,omap2";
> +	interrupt-parent = <&intc>;
>  
>  	aliases {
>  		serial0 = &uart1;
> diff --git a/arch/arm/boot/dts/omap2420.dtsi b/arch/arm/boot/dts/omap2420.dtsi
> index bfd76b4..4d5ce91 100644
> --- a/arch/arm/boot/dts/omap2420.dtsi
> +++ b/arch/arm/boot/dts/omap2420.dtsi
> @@ -30,7 +30,6 @@
>  			interrupts = <59>, /* TX interrupt */
>  				     <60>; /* RX interrupt */
>  			interrupt-names = "tx", "rx";
> -			interrupt-parent = <&intc>;
>  			ti,hwmods = "mcbsp1";
>  		};
>  
> @@ -41,7 +40,6 @@
>  			interrupts = <62>, /* TX interrupt */
>  				     <63>; /* RX interrupt */
>  			interrupt-names = "tx", "rx";
> -			interrupt-parent = <&intc>;
>  			ti,hwmods = "mcbsp2";
>  		};
>  	};
> diff --git a/arch/arm/boot/dts/omap2430.dtsi b/arch/arm/boot/dts/omap2430.dtsi
> index 4565d97..fa84532 100644
> --- a/arch/arm/boot/dts/omap2430.dtsi
> +++ b/arch/arm/boot/dts/omap2430.dtsi
> @@ -32,7 +32,6 @@
>  				     <60>, /* RX interrupt */
>  				     <61>; /* RX overflow interrupt */
>  			interrupt-names = "common", "tx", "rx", "rx_overflow";
> -			interrupt-parent = <&intc>;
>  			ti,buffer-size = <128>;
>  			ti,hwmods = "mcbsp1";
>  		};
> @@ -45,7 +44,6 @@
>  				     <62>, /* TX interrupt */
>  				     <63>; /* RX interrupt */
>  			interrupt-names = "common", "tx", "rx";
> -			interrupt-parent = <&intc>;
>  			ti,buffer-size = <128>;
>  			ti,hwmods = "mcbsp2";
>  		};
> @@ -58,7 +56,6 @@
>  				     <89>, /* TX interrupt */
>  				     <90>; /* RX interrupt */
>  			interrupt-names = "common", "tx", "rx";
> -			interrupt-parent = <&intc>;
>  			ti,buffer-size = <128>;
>  			ti,hwmods = "mcbsp3";
>  		};
> @@ -71,7 +68,6 @@
>  				     <54>, /* TX interrupt */
>  				     <55>; /* RX interrupt */
>  			interrupt-names = "common", "tx", "rx";
> -			interrupt-parent = <&intc>;
>  			ti,buffer-size = <128>;
>  			ti,hwmods = "mcbsp4";
>  		};
> @@ -84,7 +80,6 @@
>  				     <81>, /* TX interrupt */
>  				     <82>; /* RX interrupt */
>  			interrupt-names = "common", "tx", "rx";
> -			interrupt-parent = <&intc>;
>  			ti,buffer-size = <128>;
>  			ti,hwmods = "mcbsp5";
>  		};
> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> index f38ea87..955cbdc 100644
> --- a/arch/arm/boot/dts/omap3.dtsi
> +++ b/arch/arm/boot/dts/omap3.dtsi
> @@ -12,6 +12,7 @@
>  
>  / {
>  	compatible = "ti,omap3430", "ti,omap3";
> +	interrupt-parent = <&intc>;
>  
>  	aliases {
>  		serial0 = &uart1;
> @@ -240,7 +241,6 @@
>  				     <59>, /* TX interrupt */
>  				     <60>; /* RX interrupt */
>  			interrupt-names = "common", "tx", "rx";
> -			interrupt-parent = <&intc>;
>  			ti,buffer-size = <128>;
>  			ti,hwmods = "mcbsp1";
>  		};
> @@ -255,7 +255,6 @@
>  				     <63>, /* RX interrupt */
>  				     <4>;  /* Sidetone */
>  			interrupt-names = "common", "tx", "rx", "sidetone";
> -			interrupt-parent = <&intc>;
>  			ti,buffer-size = <1280>;
>  			ti,hwmods = "mcbsp2";
>  		};
> @@ -270,7 +269,6 @@
>  				     <90>, /* RX interrupt */
>  				     <5>;  /* Sidetone */
>  			interrupt-names = "common", "tx", "rx", "sidetone";
> -			interrupt-parent = <&intc>;
>  			ti,buffer-size = <128>;
>  			ti,hwmods = "mcbsp3";
>  		};
> @@ -283,7 +281,6 @@
>  				     <54>, /* TX interrupt */
>  				     <55>; /* RX interrupt */
>  			interrupt-names = "common", "tx", "rx";
> -			interrupt-parent = <&intc>;
>  			ti,buffer-size = <128>;
>  			ti,hwmods = "mcbsp4";
>  		};
> @@ -296,7 +293,6 @@
>  				     <81>, /* TX interrupt */
>  				     <82>; /* RX interrupt */
>  			interrupt-names = "common", "tx", "rx";
> -			interrupt-parent = <&intc>;
>  			ti,buffer-size = <128>;
>  			ti,hwmods = "mcbsp5";
>  		};
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index 812461e..2ab6e68 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -340,7 +340,6 @@
>  			      <0x49032000 0x7f>; /* L3 Interconnect */
>  			reg-names = "mpu", "dma";
>  			interrupts = <0 112 0x4>;
> -			interrupt-parent = <&gic>;
>  			ti,hwmods = "mcpdm";
>  		};
>  
> @@ -350,7 +349,6 @@
>  			      <0x4902e000 0x7f>; /* L3 Interconnect */
>  			reg-names = "mpu", "dma";
>  			interrupts = <0 114 0x4>;
> -			interrupt-parent = <&gic>;
>  			ti,hwmods = "dmic";
>  		};
>  
> @@ -361,7 +359,6 @@
>  			reg-names = "mpu", "dma";
>  			interrupts = <0 17 0x4>;
>  			interrupt-names = "common";
> -			interrupt-parent = <&gic>;
>  			ti,buffer-size = <128>;
>  			ti,hwmods = "mcbsp1";
>  		};
> @@ -373,7 +370,6 @@
>  			reg-names = "mpu", "dma";
>  			interrupts = <0 22 0x4>;
>  			interrupt-names = "common";
> -			interrupt-parent = <&gic>;
>  			ti,buffer-size = <128>;
>  			ti,hwmods = "mcbsp2";
>  		};
> @@ -385,7 +381,6 @@
>  			reg-names = "mpu", "dma";
>  			interrupts = <0 23 0x4>;
>  			interrupt-names = "common";
> -			interrupt-parent = <&gic>;
>  			ti,buffer-size = <128>;
>  			ti,hwmods = "mcbsp3";
>  		};
> @@ -396,7 +391,6 @@
>  			reg-names = "mpu";
>  			interrupts = <0 16 0x4>;
>  			interrupt-names = "common";
> -			interrupt-parent = <&gic>;
>  			ti,buffer-size = <128>;
>  			ti,hwmods = "mcbsp4";
>  		};
> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
> index 42c78be..61a4f2e 100644
> --- a/arch/arm/boot/dts/omap5.dtsi
> +++ b/arch/arm/boot/dts/omap5.dtsi
> @@ -287,7 +287,6 @@
>  			      <0x49032000 0x7f>; /* L3 Interconnect */
>  			reg-names = "mpu", "dma";
>  			interrupts = <0 112 0x4>;
> -			interrupt-parent = <&gic>;
>  			ti,hwmods = "mcpdm";
>  		};
>  
> @@ -297,7 +296,6 @@
>  			      <0x4902e000 0x7f>; /* L3 Interconnect */
>  			reg-names = "mpu", "dma";
>  			interrupts = <0 114 0x4>;
> -			interrupt-parent = <&gic>;
>  			ti,hwmods = "dmic";
>  		};
>  
> @@ -308,7 +306,6 @@
>  			reg-names = "mpu", "dma";
>  			interrupts = <0 17 0x4>;
>  			interrupt-names = "common";
> -			interrupt-parent = <&gic>;
>  			ti,buffer-size = <128>;
>  			ti,hwmods = "mcbsp1";
>  		};
> @@ -320,7 +317,6 @@
>  			reg-names = "mpu", "dma";
>  			interrupts = <0 22 0x4>;
>  			interrupt-names = "common";
> -			interrupt-parent = <&gic>;
>  			ti,buffer-size = <128>;
>  			ti,hwmods = "mcbsp2";
>  		};
> @@ -332,7 +328,6 @@
>  			reg-names = "mpu", "dma";
>  			interrupts = <0 23 0x4>;
>  			interrupt-names = "common";
> -			interrupt-parent = <&gic>;
>  			ti,buffer-size = <128>;
>  			ti,hwmods = "mcbsp3";
>  		};
> -- 
> 1.7.0.4
> 

^ permalink raw reply

* [PATCHv2] Input: omap4-keypad: Add pinctrl support
From: Linus Walleij @ 2012-10-24 17:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121024165749.GB32220@arwen.pp.htv.fi>

On Wed, Oct 24, 2012 at 6:57 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Wed, Oct 24, 2012 at 09:18:01AM -0700, Dmitry Torokhov wrote:
>> OK, so with drivers/base/, have you considered doing default pinctrl
>> selection in bus's probe() methods? Yo would select the default
>> configuration before starting probing the device and maybe select idle
>> when probe fails or device is unbound? That would still keep the link
>> between device object and pinctrl and there less busses than device
>> drivers out there.
>
> it starts to become confusing after a while. I mean, there's a reason
> why all drivers explictly call pm_runtim_enable(), right ?
>
> From a first thought, one could think of just yanking that into bus'
> probe() as you may suggest, but sometimes the device is already enabled,
> so we need extra tricks:
>
> pm_runtime_set_active();
> pm_runtime_enable();
> pm_runtime_get();
>
> the same could happen with pinctrl eventually. What if a device needs to
> do something else (an errata fix as an example) before requesting
> pinctrl's default state ?

I can confirm this. Just the ordering between enabling/disabling
resources like clock/pins/powerdomain screw things up for us
and even if we can surely centralize parts of this code as such
into the drivers/base or pm_* namespace we would still need
explicit calls from the driver.

I'm thinking that maybe the best helpers are actually static
inline functions in the <linux/pinctrl/consumer.h> header
rather than moving anything into drivers/base/* if the
code duplication is the real problem.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH v5 3/5] ARM: EXYNOS: Enable PMUs for exynos4
From: Olof Johansson @ 2012-10-24 17:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1351056894-5790-4-git-send-email-chanho61.park@samsung.com>

Hi,

On Tue, Oct 23, 2012 at 10:34 PM, Chanho Park <chanho61.park@samsung.com> wrote:
> This patch defines irq numbers of ARM performance monitoring unit for exynos4.
> Firs of all, we need to fix IRQ_PMU correctly and to split pmu initialization
> of exynos from plat-samsung for easily defining it.
>
> The number of CPU cores and PMU irq numbers are vary according to soc types.
> So, we need to identify each soc type using soc_is_xxx function and to define
> the pmu irqs dynamically. For example, the exynos4412 has 4 cpu cores and pmus.

I wonder if it's worth doing this complexity on the non-DT case for exynos4?

I wish there was more focus on the Samsung platforms for getting the
DT support up to par with non-DT so you can avoid having to add new
platform devices like these in the first place.


Thanks,

-Olof

^ permalink raw reply

* [PATCHv2] Input: omap4-keypad: Add pinctrl support
From: Linus Walleij @ 2012-10-24 17:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121024165215.GA32220@arwen.pp.htv.fi>

On Wed, Oct 24, 2012 at 6:52 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Wed, Oct 24, 2012 at 09:14:29AM -0700, Dmitry Torokhov wrote:

>> All of _their_ resources, exactly. They do not own nor control pins so
>> they should not be bothered with them either. Look, when you see that
>
> except that they *own* the pins. Now that the muxer has been setup
> properly, this particular IP owns the pins.

This is true. It is then also reflected in the device model.
And in debugfs, which is very helpful when debugging. If I
cat
/sys/kernel/debug/pinctrl/pinctrl-db8500/pins

I can see a list of all pins on this ASIC and what device
is using it. It is all due to the fact that each driver use
[devm_]pinctrl_get(&dev) ad the struct device * pointer
is used with dev_name() to print the corresponding
device name.

When using hogs it just says the name of the pin
controller and the pins aren't really connected to their
real consumers :-P

Example:

Pinmux settings per pin
Format: pin (name): mux_owner gpio_owner hog?
pin 0 (GPIO0_AJ5): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 1 (GPIO1_AJ3): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 2 (GPIO2_AH4): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 3 (GPIO3_AH3): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 4 (GPIO4_AH6): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 5 (GPIO5_AG6): (MUX UNCLAIMED) DB8500:5
pin 6 (GPIO6_AF6): pinctrl-db8500 (GPIO UNCLAIMED) function ipgpio
group ipgpio0_c_1
pin 7 (GPIO7_AG5): pinctrl-db8500 (GPIO UNCLAIMED) function ipgpio
group ipgpio1_c_1
pin 8 (GPIO8_AD5): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 9 (GPIO9_AE4): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 10 (GPIO10_AF5): nmk-i2c.2 (GPIO UNCLAIMED) function i2c2 group i2c2_b_2
pin 11 (GPIO11_AG4): nmk-i2c.2 (GPIO UNCLAIMED) function i2c2 group i2c2_b_2
pin 12 (GPIO12_AC4): pinctrl-db8500 (GPIO UNCLAIMED) function msp0
group msp0txrx_a_1
pin 13 (GPIO13_AF3): pinctrl-db8500 (GPIO UNCLAIMED) function msp0
group msp0tfstck_a_1
pin 14 (GPIO14_AE3): pinctrl-db8500 (GPIO UNCLAIMED) function msp0
group msp0tfstck_a_1
pin 15 (GPIO15_AC3): pinctrl-db8500 (GPIO UNCLAIMED) function msp0
group msp0txrx_a_1
pin 16 (GPIO16_AD3): nmk-i2c.1 (GPIO UNCLAIMED) function i2c1 group i2c1_b_2
pin 17 (GPIO17_AD4): nmk-i2c.1 (GPIO UNCLAIMED) function i2c1 group i2c1_b_2

As you can see pins 6,7,12-15 are using hogs. Sure I can see
the name of the function but that is just a string albeit
helpful.

Pins 10,11,16,17 are requested directly from the i2c driver
and shows up connected to its device.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH v2] gpio/omap: fix off-mode bug: clear debounce clock enable mask on free/reset
From: Kevin Hilman @ 2012-10-24 17:10 UTC (permalink / raw)
  To: linux-arm-kernel

From: Kevin Hilman <khilman@ti.com>

When a GPIO bank is freed or shutdown, ensure that the banks
dbck_enable_mask is cleared also.  Otherwise, context restore on
subsequent off-mode transition will restore previous value from the
shadow copies (bank->context.debounce*) leading to mismatch state
between driver state and hardware state.

This was discovered when board code was doing

  gpio_request_one()
  gpio_set_debounce()
  gpio_free()

which was leaving the GPIO debounce settings in a confused state.  If
that GPIO bank is subsequently used with off-mode enabled, bogus state
would be restored, leaving GPIO debounce enabled which then prevented
the CORE powerdomain from transitioning.

To fix, ensure that bank->dbck_enable_mask is cleared when the bank
is freed/shutdown so debounce state doesn't persist.

Special thanks to Grazvydas Ignotas for pointing out a bug in an
earlier version that would've disabled debounce on any runtime PM
transition.

Reported-by: Paul Walmsley <paul@pwsan.com>
Cc: Igor Grinberg <grinberg@compulab.co.il>
Cc: Grazvydas Ignotas <notasas@gmail.com>
Signed-off-by: Kevin Hilman <khilman@ti.com>
---
v2: only clear mask in free/shutdown, not in runtime PM paths, 
    clarified changelog   
Applies on v3.7-rc2.

 drivers/gpio/gpio-omap.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 94cbc84..113b167 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -539,6 +539,7 @@ static void _reset_gpio(struct gpio_bank *bank, int gpio)
 	_set_gpio_irqenable(bank, gpio, 0);
 	_clear_gpio_irqstatus(bank, gpio);
 	_set_gpio_triggering(bank, GPIO_INDEX(bank, gpio), IRQ_TYPE_NONE);
+	bank->dbck_enable_mask = 0;
 }
 
 /* Use disable_irq_wake() and enable_irq_wake() functions from drivers */
-- 
1.8.0

^ permalink raw reply related

* [PATCHv2] Input: omap4-keypad: Add pinctrl support
From: Linus Walleij @ 2012-10-24 17:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121024161801.GB16350@core.coreip.homeip.net>

On Wed, Oct 24, 2012 at 6:18 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, Oct 24, 2012 at 02:54:23PM +0200, Linus Walleij wrote:
>>
>> A third alternative as outlined is to use notifiers and some
>> resource core in drivers/base/*
>
> OK, so with drivers/base/, have you considered doing default pinctrl
> selection in bus's probe() methods? Yo would select the default
> configuration before starting probing the device and maybe select idle
> when probe fails or device is unbound? That would still keep the link
> between device object and pinctrl and there less busses than device
> drivers out there.

One of our major important busses is the AMBA (PrimeCell) bus.

As it happens, this bus actually already do limited resource handling
by requesting the silicon block clock for the device, which is necessary
to perform auto-probing on the bus level. (You won't be able to read
the auto-detect registers unless the silicon is clocked...)

When it comes to pin control is turns out in the AMBA drivers we
have that we need to do more complex stuff than just select a
default configuration. (I assure this is not just for fun, it is saving
considerable amounts of power).

So the examples I outlined just in the previous mail:
drivers/tty/serial/amba-pl011.c
drivers/spi/spi-pl022.c
drivers/i2c/busses/i2c-nomadik.c

Hm it turns out that Wolfram has not yet merged the i2c patch,
here it is:
http://marc.info/?l=linux-i2c&m=134986995731695&w=2

There are complex state switches involved. It can arguably be
centralized, but then it needs to go into drivers/base/[power]
or similar, not into a specific piece of bus code, because the
needs won't be any different for e.g. a platform device.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCHv2] Input: omap4-keypad: Add pinctrl support
From: Felipe Balbi @ 2012-10-24 16:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121024161801.GB16350@core.coreip.homeip.net>

Hi,

On Wed, Oct 24, 2012 at 09:18:01AM -0700, Dmitry Torokhov wrote:
> On Wed, Oct 24, 2012 at 02:54:23PM +0200, Linus Walleij wrote:
> > On Tue, Oct 23, 2012 at 10:02 PM, Dmitry Torokhov
> > <dmitry.torokhov@gmail.com> wrote:
> > 
> > > I have seen just in a few days 3 or 4 drivers having exactly the same
> > > change - call to devm_pinctrl_get_select_default(), and I guess I will
> > > receive the same patches for the rest of input drivers shortly.
> > > This suggests that the operation is done at the wrong level. Do the
> > > pin configuration as you parse DT data, the same way you set up i2c
> > > devices registers in of_i2c.c, and leave the individual drivers that do
> > > not care about specifics alone.
> > 
> > Exactly this can be done with pinctrl hogs.
> > 
> > The problem with that is that it removes the cross-reference
> > between the device and it's pinctrl handle (also from the device
> > tree). Instead the pinctrl handle gets referenced to the pin controller
> > itself. So from a modelling perpective this looks a bit ugly.
> > 
> > So we have two kinds of ugly:
> > 
> > - Sprinke devm_pinctrl_get_select_default() over all drivers
> >   which makes pinctrl handles properly reference their devices
> > 
> > - Use hogs and loose coupling between pinctrl handles and their
> >   devices
> > 
> > A third alternative as outlined is to use notifiers and some
> > resource core in drivers/base/*
> 
> OK, so with drivers/base/, have you considered doing default pinctrl
> selection in bus's probe() methods? Yo would select the default
> configuration before starting probing the device and maybe select idle
> when probe fails or device is unbound? That would still keep the link
> between device object and pinctrl and there less busses than device
> drivers out there.

it starts to become confusing after a while. I mean, there's a reason
why all drivers explictly call pm_runtim_enable(), right ?

>From a first thought, one could think of just yanking that into bus'
probe() as you may suggest, but sometimes the device is already enabled,
so we need extra tricks:

pm_runtime_set_active();
pm_runtime_enable();
pm_runtime_get();

the same could happen with pinctrl eventually. What if a device needs to
do something else (an errata fix as an example) before requesting
pinctrl's default state ?

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121024/5a06cca9/attachment.sig>

^ permalink raw reply

* [PATCH v4 0/7] uio_pruss cleanup and platform support
From: Matt Porter @ 2012-10-24 16:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5072B190.1060200@ti.com>

On Mon, Oct 08, 2012 at 04:27:20PM +0530, Sekhar Nori wrote:
> On 10/5/2012 10:34 PM, Matt Porter wrote:
> 
> > This series enables uio_pruss on DA850 and removes use of the
> > private SRAM API by the driver. The driver previously was not
> > enabled by any platform and the private SRAM API was accessing
> > an invalid SRAM bank.
> > 
> > It is regression tested on AM180x EVM with suspend/resume due
> > to the new use of the shared SRAM for both PM and PRUSS. The
> > uio_pruss driver is tested on the same platform using the
> > PRU_memAccessPRUDataRam and PRU_memAccessL3andDDR examples from
> > the PRU userspace tools available from http://www.ti.com/tool/sprc940
> 
> I applied patches 2/7, 3/7 and 6/7 of this series for v3.8. I have some
> comments on the board patch. Rest of the patches depend on acceptance of
> 1/7 so I will take them only after that is accepted.

Ok, Hans has accepted 1/7, will you take the entire series through the
Davinci tree?

-Matt

^ permalink raw reply

* [PATCH v2 2/2] USB: doc: Binding document for ehci-platform driver
From: Alan Stern @ 2012-10-24 16:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <50881727.10601@wwwdotorg.org>

On Wed, 24 Oct 2012, Stephen Warren wrote:

> Device tree files always need a completely specific value in the
> compatible property, even when less-specific/more-generic values are
> also present. So for example, the Linux driver might only care about the
> following existing:
> 
> compatible = "usb-ehci".
> 
> However, any actual EHCI controller is always some specific model, so
> the compatible value must define which specific model it is, e.g.:
> 
> compatible = "nvidia,tegra20-ehci", "usb-ehci".
> 
> Now lets say the Tegra30 EHCI controller is identical to the Tegra20
> controller. That needs to be explicitly represented too:
> 
> compatible = "nvidia,tegra30-ehci", "nvidia,tegra20-ehci", "usb-ehci".
> 
> In that case, the driver might still only declare support for
> "nvidia,tegra20-ehci", but "nvidia,tegra30-ehci" is added to be explicit
> about the actual HW model.
> 
> This doesn't continue forever though; you typically only list the
> specific HW model, the base specific model it's compatible with, and any
> generic value needed. So, a hypothetical Tegra40 EHCI controller would be:
> 
> compatible = "nvidia,tegra40-ehci", "nvidia,tegra20-ehci", "usb-ehci".
> 
> Given that, there is always something to key any newly required quirk
> off, so even if the need for a quirk is found later, the device tree
> already contains the information needed to trigger it.

Okay.  It appears that quite a few .dts/.dtsi files mention "usb-ehci", 
for no apparent reason (given that the drivers don't list it):

[stern at iolanthe usb-3.6]$ find arch -name '*.dts*' | xargs grep usb-ehci
arch/mips/cavium-octeon/octeon_3xxx.dts:                                compatible = "cavium,octeon-6335-ehci","usb-ehci";
arch/mips/cavium-octeon/octeon_68xx.dts:                                compatible = "cavium,octeon-6335-ehci","usb-ehci";
arch/arm/boot/dts/tegra20.dtsi:         compatible = "nvidia,tegra20-ehci", "usb-ehci";
arch/arm/boot/dts/tegra20.dtsi:         compatible = "nvidia,tegra20-ehci", "usb-ehci";
arch/arm/boot/dts/tegra20.dtsi:         compatible = "nvidia,tegra20-ehci", "usb-ehci";
arch/arm/boot/dts/at91sam9x5.dtsi:                      compatible = "atmel,at91sam9g45-ehci", "usb-ehci";
arch/arm/boot/dts/spear13xx.dtsi:                       compatible = "st,spear600-ehci", "usb-ehci";
arch/arm/boot/dts/spear13xx.dtsi:                       compatible = "st,spear600-ehci", "usb-ehci";
arch/arm/boot/dts/spear3xx.dtsi:                        compatible = "st,spear600-ehci", "usb-ehci";
arch/arm/boot/dts/spear600.dtsi:                        compatible = "st,spear600-ehci", "usb-ehci";
arch/arm/boot/dts/spear600.dtsi:                        compatible = "st,spear600-ehci", "usb-ehci";
arch/arm/boot/dts/at91sam9g45.dtsi:                     compatible = "atmel,at91sam9g45-ehci", "usb-ehci";
arch/powerpc/boot/dts/wii.dts:                  compatible = "nintendo,hollywood-usb-ehci",
arch/powerpc/boot/dts/wii.dts:                                  "usb-ehci";
arch/powerpc/boot/dts/sequoia.dts:                      compatible = "ibm,usb-ehci-440epx", "usb-ehci";
arch/powerpc/boot/dts/canyonlands.dts:                  compatible = "ibm,usb-ehci-460ex", "usb-ehci";

Is there a real reason that I'm not aware of?  Or can all these entries
safely be removed?

Alan Stern

^ permalink raw reply

* [PATCHv2] Input: omap4-keypad: Add pinctrl support
From: Felipe Balbi @ 2012-10-24 16:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121024161429.GA16350@core.coreip.homeip.net>

Hi,

On Wed, Oct 24, 2012 at 09:14:29AM -0700, Dmitry Torokhov wrote:

<snip>

> > > No, I guess we ihave different understanding of what "directly use" means.
> > > This particular driver does directly use interrupts: it requests it and
> > > performs some actions when interrupt arrives. Same goes for IO memory -
> > > it gets requested, then we access it. With pinctrl we do not do anything
> > > - we just ask another layer to configure it and that is it.
> > 
> > this is true for almost anything we do:
> > 
> > - we ask another layer to allocate memory for us
> > - we ask another layer to call our ISR once the IRQ line is asserted
> > - we ask another layer to handle the input events we just received
> > - we ask another layer to transfer data through DMA for us
> > - we ask another layer to turn regulators on and off.
> 
> But we are _directly_ _using_ all of these. You allocate memory and you
> (the driver) stuff data into that memory. You ask for DMA and you take
> the DMAed data and work with it. Not so with pinctrl in omap keypad and
> other drivers I have seen so far.

of course we are. If we don't mux the pins to their correct setting, we
can't realy use the HW. So while you don't see any SW control of the
requested pins, we're still making use of them.

> > and so on. This is just how abstractions work, we group common parts in
> > a framework so that users don't need to know the details, but still need
> > to tell the framework when to fiddle with those resources.
> > 
> > > I have seen just in a few days 3 or 4 drivers having exactly the same
> > > change - call to devm_pinctrl_get_select_default(), and I guess I will
> > > receive the same patches for the rest of input drivers shortly.
> > > This suggests that the operation is done at the wrong level. Do the
> > > pin configuration as you parse DT data, the same way you set up i2c
> > > devices registers in of_i2c.c, and leave the individual drivers that do
> > > not care about specifics alone.
> > 
> > Makes no sense to hide that from drivers. The idea here is that driver
> > should know when it needs its pins to muxed correctly.
> 
> The driver also needs memory controller to be initialized, gpio chip be
> ready and registered, DMA subsystem ready, input core reade, etc, etc,
> etc. You however do not have every driver explicitly initialize any of
> that; you expect certain working environment to be already operable. The
> driver does manage resources it controls, it has ultimate knowledge
> about, pin configuration is normally is not it. We just need to know
> that we wired/muxed properly, otherwise we won't work. So please let
> parent layers deal with it.
> 
> > 95% of the time
> > it will be done during probe() but then again, so what ?
> > 
> > doing that when parsing DT, or on bus notifiers is just plain wrong.
> > Drivers should be required to handle all of their resources.
> 
> All of _their_ resources, exactly. They do not own nor control pins so
> they should not be bothered with them either. Look, when you see that

except that they *own* the pins. Now that the muxer has been setup
properly, this particular IP owns the pins.

> potentially _every_ driver in the system needs to set up the same object
> that it doe snot use otherwise you should realize that individual driver
> is not the proper place to do that.

fair enough, but IMHO, we're not there yet. We can't make that claim
yet. Besides, we don't know what's the default pin state in a system. It
might be that certain pins start out in a way which consumes less power
due to the internal construction of the SoC. If we set pins up before
driver probes, and probe fails or the driver is never really used, then
we could be falling into a situation where we're wasting power.

Granted, you can undo everything you did before, but I guess keeping
track of everything we setup before probe() just to remove a couple of
lines from drivers is wrong.

> > > > That's why it is named "get_select_default" and not "get" only.
> > > > This API ensure that the pin will be set to the default state, and this
> > > > is all we need to do during the probe.
> > > 
> > > Why during the probe and not by default? Realistically, the driver will
> > > be loaded as long as there is a matching device and pins will need to be
> > > configured.
> > 
> > likewise memory will be allocated when matching happens, IRQs will be
> > allocated, regulators will be turned on. So why don't we do all that by
> > default ? Because it is wrong.
> 
> No, because we do not know how. The generic layer does not know the ISR
> to install, how much memory to allocate, etc. Having regulator turned on
> before getting to probe might not be a bad idea.

what if your driver never probes ? Will you really leave regulators on
consuming extra, valuable power ?

> > > > There is no point to change the mux if the driver is not probed, because
> > > > potentially the pin can be use be another driver.
> > > 
> > > What other driver would use it? Who would chose what driver to load?
> > 
> > Well, you _do_ know that on a SoC we have a limited amount of pins
> > right ?
> > 
> > Considering the amont of features which are packed inside a single die,
> > it's not farfetched to assume we will have a lot less pins then we
> > actually need, so we need muxers behind each pin in order to choose
> > which functionality we want.
> > 
> > If it happens that keypad's pins are shared with another IP (e.g. GPIO),
> > we need to give the final user (a OEM/ODM) the choice of using those
> > pins as either keypad or GPIOs, thus the need for pinctrl framework and
> > the calls in the drivers.
> 
> Right, so please walk me through, step by step, how an OEM/ODM woudl
> select a particular configuration. Do you expect it to happen at
> runtime, or do you expect relevant data be put in DT?

It depends, I've seen both happening, really. Also note that DT
migration is still not complete, meaning that most (all ?) OEM/ODMs are
still using the legacy board-file-based approach and it will still take
them a few years to move to DT-based boot.

Another point to consider is community boards such as beaglebone which
have tens of different "capes" to support. Depending on the cape, pins
might have to be remuxed, so instead of adding all that code to platform
support, just leave it all in drivers. Depending on the "cape" different
drivers will probe() and those drivers should know how to mux pins for
themselves.

Note that these are only the two easy examples that came to my mind, I'm
sure we can discuss this for a long, but is it valid ? For a single line
of code ?

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121024/1f1b2220/attachment.sig>

^ permalink raw reply

* [RFC] Energy/power monitoring within the kernel
From: Pawel Moll @ 2012-10-24 16:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4317776.evLpJapyim@hammer82.arch.suse.de>

On Wed, 2012-10-24 at 01:40 +0100, Thomas Renninger wrote:
> > More and more of people are getting interested in the subject of power
> > (energy) consumption monitoring. We have some external tools like
> > "battery simulators", energy probes etc., but some targets can measure
> > their power usage on their own.
> > 
> > Traditionally such data should be exposed to the user via hwmon sysfs
> > interface, and that's exactly what I did for "my" platform - I have
> > a /sys/class/hwmon/hwmon*/device/energy*_input and this was good
> > enough to draw pretty graphs in userspace. Everyone was happy...
> > 
> > Now I am getting new requests to do more with this data. In particular
> > I'm asked how to add such information to ftrace/perf output.
> Why? What is the gain?
> 
> Perf events can be triggered at any point in the kernel.
> A cpufreq event is triggered when the frequency gets changed.
> CPU idle events are triggered when the kernel requests to enter an idle state
> or exits one.
> 
> When would you trigger a thermal or a power event?
> There is the possibility of (critical) thermal limits.
> But if I understand this correctly you want this for debugging and
> I guess you have everything interesting one can do with temperature
> values:
>   - read the temperature
>   - draw some nice graphs from the results
> 
> Hm, I guess I know what you want to do:
> In your temperature/energy graph, you want to have some dots
> when relevant HW states (frequency, sleep states,  DDR power,...)
> changed. Then you are able to see the effects over a timeline.
> 
> So you have to bring the existing frequency/idle perf events together
> with temperature readings
> 
> Cleanest solution could be to enhance the exisiting userspace apps
> (pytimechart/perf timechart) and let them add another line
> (temperature/energy), but the data would not come from perf, but
> from sysfs/hwmon.
> Not sure whether this works out with the timechart tools.
> Anyway, this sounds like a userspace only problem.

Ok, so it is actually what I'm working on right now. Not with the
standard perf tool (there are other users of that API ;-) but indeed I'm
trying to "enrich" the data stream coming from kernel with user-space
originating values. I am a little bit concerned about effect of extra
syscalls (accessing the value and gettimeofday to generate a timestamp)
at a higher sampling rates, but most likely it won't be a problem. Can
report once I know more, if this is of interest to anyone.

Anyway, there are at least two debug/trace related use cases that can
not be satisfied that way (of course one could argue about their
usefulness):

1. ftrace-over-network (https://lwn.net/Articles/410200/) which is
particularly appealing for "embedded users", where there's virtually no
useful userspace available (think Android). Here a (functional) trace
event is embedded into a normal trace and available "for free" at the
host side.

2. perf groups - the general idea is that one event (let it be cycle
counter interrupt or even a timer) triggers read of other values (eg.
cache counter or - in this case - energy counter). The aim is to have a
regular "snapshots" of the system state. I'm not sure if the standard
perf tool can do this, but I do :-)

And last, but not least, there are the non-debug/trace clients for
energy data as discussed in other mails in this thread. Of course the
trace event won't really satisfy their needs either.

Thanks for your feedback!

Pawe?

^ permalink raw reply

* [PATCHv2] Input: omap4-keypad: Add pinctrl support
From: Linus Walleij @ 2012-10-24 16:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121024161429.GA16350@core.coreip.homeip.net>

On Wed, Oct 24, 2012 at 6:14 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, Oct 24, 2012 at 11:37:04AM +0300, Felipe Balbi wrote:

>> - we ask another layer to allocate memory for us
>> - we ask another layer to call our ISR once the IRQ line is asserted
>> - we ask another layer to handle the input events we just received
>> - we ask another layer to transfer data through DMA for us
>> - we ask another layer to turn regulators on and off.
>
> But we are _directly_ _using_ all of these. You allocate memory and you
> (the driver) stuff data into that memory. You ask for DMA and you take
> the DMAed data and work with it. Not so with pinctrl in omap keypad and
> other drivers I have seen so far.

Consult:
drivers/tty/serial/amba-pl011.c
drivers/spi/spi-pl022.c
drivers/i2c/busses/i2c-nomadik.c

for more complex pinctrl use cases. These are my dogfood drivers ...
Most of these will request more than one state and switch the driver
between these different states at runtime, in these examples for power
saving there are states named "default", "sleep" and in the I2C driver
also "idle".

These examples are more typical to how the ux500 platform will
look, also the SKE input driver will move the devise to sleep/default
states but we need to merge PM code before we can do that.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH v2 2/2] USB: doc: Binding document for ehci-platform driver
From: Stephen Warren @ 2012-10-24 16:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <Pine.LNX.4.44L0.1210241238570.1282-100000@iolanthe.rowland.org>

On 10/24/2012 10:44 AM, Alan Stern wrote:
> On Wed, 24 Oct 2012, Stephen Warren wrote:
> 
>> We should absolutely avoid Linux-specific properties where possible.
>>
>> That said, what Linux-specific properties are you talking about? The
>> properties discussed here (has-synopsys-hc-bug, no-io-watchdog, has-tt)
>> are all purely a description of HW, aren't they.
> 
> "has-tt" is definitely a description of the HW.

OK.

> "has-synopsys-hc-bug" is too, although determining whether or not it 
> should apply to a particular controller might be difficult.  I'm 
> inclined not to include it among the properties.
> 
> "no-io-watchdog" is not the greatest name.  It describes to controllers 
> that always do generate IRQs for I/O events when they are supposed to 
> (and hence the driver doesn't need to set up a watchdog timer to detect 
> I/O completions that didn't generate an IRQ).  So while the concept is 
> HW-specific, the name refers to a driver implementation issue.  A 
> better name might be something like "reliable-IRQs".  Again, it's not 
> such an easy thing to test for.  Almost all the existing drivers leave 
> it unset.

OK, I'd be inclined to drive those last two by quirks then, since they
aren't architectural features of EHCI but rather implementation issues.
And indeed have the quirk table have a "reliable IRQs" field instead of
"no IO watchdog", to minimize the table size.

^ permalink raw reply

* [PATCH v2 2/2] USB: doc: Binding document for ehci-platform driver
From: Stephen Warren @ 2012-10-24 16:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <Pine.LNX.4.44L0.1210241233040.1282-100000@iolanthe.rowland.org>

On 10/24/2012 10:38 AM, Alan Stern wrote:
> On Wed, 24 Oct 2012, Stephen Warren wrote:
> 
>> On 10/24/2012 09:26 AM, Sebastian Andrzej Siewior wrote:
>>> On Wed, Oct 24, 2012 at 10:57:00AM -0400, Alan Stern wrote:
>>>> Under the circumstances, do we really need a new binding document for 
>>>> the ehci-platform driver?
>>
>> It seems reasonable to add the new properties to usb-ehci.txt, since
>> they do describe the HW.
>>
>>>> We should be able to use the existing 
>>>> usb-ehci binding, perhaps with some new properties added:
>>>>
>>>> 	has-synopsys-hc-bug
>>>> 	no-io-watchdog
>>>> 	has-tt
>>
>> That sounds fine to me.
>>
>> However, there is an implementation issue here. I believe the way Linux
>> searches for a driver for a particular node is:
>>
>> for every driver that's registered
>>     if the driver's supported compatible list matches the device
>>         use the driver
>>
>> (See drivers/base/platform.c:platform_match() which implements the if
>> statement above, and I assume the driver core implements the outer for
>> loop above)
> 
> Yes, it does.
> 
>> That means that if the generic driver supports compatible="usb-ehci", it
>> may "steal" device nodes that have
>> compatible="something-custom","usb-ehci", even if there's a driver
>> specifically for "something-custom". We would need to re-arrange the
>> driver matching code to:
>>
>> for each compatible value in the node:
>>     for each driver that's registered:
>>         if the driver supports the compatible value:
>>             use the driver.
> 
> Which might be difficult since the inner loop would be controlled by
> the outer code in the driver core.
> 
> How do we determine which existing drivers claim to support usb-ehci?  
> A quick search under arch/ and drivers/ turns up nothing but
> drivers/usb/host/ehci-ppc-of.c.  Changing it to a more HW-specific
> match should be easy enough, and then "usb-ehci" would be safe to use
> in ehci-platform.c.

It's not drivers that claim to support usb-ehci, but device tree files
that contain nodes that include "usb-ehci" in their compatible value,
yet need to be handled by a driver that isn't the generic USB EHCI
driver, and that driver binds to the other more specific compatible value:

> $ grep -HrnI usb-ehci arch/*/boot/dts
> arch/arm/boot/dts/spear3xx.dtsi:76:			compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/at91sam9x5.dtsi:399:			compatible = "atmel,at91sam9g45-ehci", "usb-ehci";
> arch/arm/boot/dts/spear13xx.dtsi:145:			compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/spear13xx.dtsi:152:			compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/tegra20.dtsip:215:		compatible = "nvidia,tegra20-ehci", "usb-ehci";
> arch/arm/boot/dts/tegra20.dtsip:224:		compatible = "nvidia,tegra20-ehci", "usb-ehci";
> arch/arm/boot/dts/tegra20.dtsip:232:		compatible = "nvidia,tegra20-ehci", "usb-ehci";
> arch/arm/boot/dts/spear600.dtsi:88:			compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/spear600.dtsi:96:			compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/at91sam9g45.dtsi:392:			compatible = "atmel,at91sam9g45-ehci", "usb-ehci";
> arch/powerpc/boot/dts/sequoia.dts:156:			compatible = "ibm,usb-ehci-440epx", "usb-ehci";
> arch/powerpc/boot/dts/wii.dts:120:			compatible = "nintendo,hollywood-usb-ehci",
> arch/powerpc/boot/dts/wii.dts:121:					"usb-ehci";
> arch/powerpc/boot/dts/canyonlands.dts:167:			compatible = "ibm,usb-ehci-460ex", "usb-ehci";

and then search for all those other compatible values in drivers. The
ARM instances certainly all have this issue (although perhaps it's worth
investigating if a generic could work for them instead). The PPC
instances need more investigation; the values don't show up in of match
tables but rather in code.

^ permalink raw reply

* [PATCH v2 2/2] USB: doc: Binding document for ehci-platform driver
From: Alan Stern @ 2012-10-24 16:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5088145F.1040504@wwwdotorg.org>

On Wed, 24 Oct 2012, Stephen Warren wrote:

> We should absolutely avoid Linux-specific properties where possible.
> 
> That said, what Linux-specific properties are you talking about? The
> properties discussed here (has-synopsys-hc-bug, no-io-watchdog, has-tt)
> are all purely a description of HW, aren't they.

"has-tt" is definitely a description of the HW.

"has-synopsys-hc-bug" is too, although determining whether or not it 
should apply to a particular controller might be difficult.  I'm 
inclined not to include it among the properties.

"no-io-watchdog" is not the greatest name.  It describes to controllers 
that always do generate IRQs for I/O events when they are supposed to 
(and hence the driver doesn't need to set up a watchdog timer to detect 
I/O completions that didn't generate an IRQ).  So while the concept is 
HW-specific, the name refers to a driver implementation issue.  A 
better name might be something like "reliable-IRQs".  Again, it's not 
such an easy thing to test for.  Almost all the existing drivers leave 
it unset.

Alan Stern

^ permalink raw reply

* [PATCH v2 2/2] USB: doc: Binding document for ehci-platform driver
From: Florian Fainelli @ 2012-10-24 16:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <Pine.LNX.4.44L0.1210241233040.1282-100000@iolanthe.rowland.org>

On Wednesday 24 October 2012 12:38:42 Alan Stern wrote:
> On Wed, 24 Oct 2012, Stephen Warren wrote:
> 
> > On 10/24/2012 09:26 AM, Sebastian Andrzej Siewior wrote:
> > > On Wed, Oct 24, 2012 at 10:57:00AM -0400, Alan Stern wrote:
> > >> Under the circumstances, do we really need a new binding document for 
> > >> the ehci-platform driver?
> > 
> > It seems reasonable to add the new properties to usb-ehci.txt, since
> > they do describe the HW.
> > 
> > >> We should be able to use the existing 
> > >> usb-ehci binding, perhaps with some new properties added:
> > >>
> > >> 	has-synopsys-hc-bug
> > >> 	no-io-watchdog
> > >> 	has-tt
> > 
> > That sounds fine to me.
> > 
> > However, there is an implementation issue here. I believe the way Linux
> > searches for a driver for a particular node is:
> > 
> > for every driver that's registered
> >     if the driver's supported compatible list matches the device
> >         use the driver
> > 
> > (See drivers/base/platform.c:platform_match() which implements the if
> > statement above, and I assume the driver core implements the outer for
> > loop above)
> 
> Yes, it does.
> 
> > That means that if the generic driver supports compatible="usb-ehci", it
> > may "steal" device nodes that have
> > compatible="something-custom","usb-ehci", even if there's a driver
> > specifically for "something-custom". We would need to re-arrange the
> > driver matching code to:
> > 
> > for each compatible value in the node:
> >     for each driver that's registered:
> >         if the driver supports the compatible value:
> >             use the driver.
> 
> Which might be difficult since the inner loop would be controlled by
> the outer code in the driver core.
> 
> How do we determine which existing drivers claim to support usb-ehci?  
> A quick search under arch/ and drivers/ turns up nothing but
> drivers/usb/host/ehci-ppc-of.c.  Changing it to a more HW-specific
> match should be easy enough, and then "usb-ehci" would be safe to use
> in ehci-platform.c.

As long as no one enables both ehci-platform and ehci-ppc-of at the same time
there is no problem. ehci-ppc-of should be removed in favor of ehci-platform
and make sure that the specific quirk in ehci-ppc-of also gets ported, other 
that I see no issue using "usb-ehci" as the least detailed compatible property
name.
--
Florian

^ permalink raw reply


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