* OMAP baseline test results for v3.7-rc1
From: Kevin Hilman @ 2012-10-23 19:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAORVsuUkU3iTTFcXfYJ=_FntjiTR33Gs4z9OsNEn2Cxq7Ef-qA@mail.gmail.com>
Jean Pihet <jean.pihet@newoldbits.com> writes:
> Hi,
>
> On Sat, Oct 20, 2012 at 8:14 AM, Paul Walmsley <paul@pwsan.com> wrote:
>> Hi Jean
>>
>> On Fri, 19 Oct 2012, Paul Walmsley wrote:
>>
>>> On Thu, 18 Oct 2012, Paul Walmsley wrote:
>>>
>>> > Here are some basic OMAP test results for Linux v3.7-rc1.
>>> > Logs and other details at http://www.pwsan.com/omap/testlogs/test_v3.7-rc1/
>>
>> ...
>>
>>> > Failing tests: needing investigation
>>> > ------------------------------------
>>> >
>>> > Boot tests:
>>>
>>> * 3530ES3 Beagle: I2C timeouts during userspace init
>>> - May be related to the threaded IRQ conversion of the I2C driver
>>> - Unknown cause
>>
>> This one turned out to be caused by:
>>
>> commit 3db11feffc1ad2ab9dea27789e6b5b3032827adc
>> Author: Jean Pihet <jean.pihet@newoldbits.com>
>> Date: Thu Sep 20 18:08:03 2012 +0200
>>
>> ARM: OMAP: convert I2C driver to PM QoS for MPU latency constraints
>>
>>
>> Reverting this commit causes the problem to go away, but since the OMAP PM
>> constraint code was removed as well, it's unlikely that a simple revert is
>> the right thing to do.
>>
>> Jean could you please investigate and fix this?
> I tried the latest l-o with omap2plus defconfig on my Beagleboard B5
> (ES2.1) and could not reproduce the problem.
> I do not have the I2C error messages at boot, nor at user space start
> up. I tried to read/write the TWL RTC, successfully.
>
> Another difference is the bootloader images. I have the following:
> - Texas Instruments X-Loader 1.4.2 (Feb 3 2009 - 15:34:17)
> - U-Boot 2009.01-dirty (Feb 19 2009 - 12:22:31)
> Could you send your bootloader images?
>
> I noticed you have I2C error messages in U-Boot, could that be the
> cause of the I2C lock-up?
FWIW, I have a relatively recent mainline u-boot+SPL:
U-Boot SPL 2012.04.01 (Jul 03 2012 - 07:07:04)
U-Boot 2012.04.01 (Jul 03 2012 - 07:07:04)
And I see I2C error messages from u-boot too:
timed out in wait_for_pin: I2C_STAT=0
I2C read: I/O error
I think these are normal/expected since u-boot may be looking for
expansion cards.
Kevin
^ permalink raw reply
* OMAP baseline test results for v3.7-rc1
From: Paul Walmsley @ 2012-10-23 19:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAORVsuUkU3iTTFcXfYJ=_FntjiTR33Gs4z9OsNEn2Cxq7Ef-qA@mail.gmail.com>
Hi
On Mon, 22 Oct 2012, Jean Pihet wrote:
> I tried the latest l-o with omap2plus defconfig on my Beagleboard B5
> (ES2.1) and could not reproduce the problem.
> I do not have the I2C error messages at boot, nor at user space start
> up. I tried to read/write the TWL RTC, successfully.
>
> Another difference is the bootloader images. I have the following:
> - Texas Instruments X-Loader 1.4.2 (Feb 3 2009 - 15:34:17)
> - U-Boot 2009.01-dirty (Feb 19 2009 - 12:22:31)
> Could you send your bootloader images?
Just sent you a link via private E-mail.
> On the PM QoS side the commit 3db11fef moves the I2C code from the
> OMAP PM no-op layer to the PM QoS for CPU and DMA latency, which
> influences the cpuidle states. However CPU_IDLE is not set in
> omap2plus_defconfig so there should not be any effect.
> Do you have CPU_IDLE enabled?
No, it's omap2plus_defconfig.
- Paul
^ permalink raw reply
* OMAP baseline test results for v3.7-rc1
From: Paul Walmsley @ 2012-10-23 19:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAORVsuWFzH3UmXCaY=iqmYsE72KOecLo40tG9BcTUBirJQ5iLA@mail.gmail.com>
On Mon, 22 Oct 2012, Jean Pihet wrote:
> On Mon, Oct 22, 2012 at 6:12 PM, Jean Pihet <jean.pihet@newoldbits.com> wrote:
>
> > Do you have CPU_IDLE enabled?
> FYI the issue is not present with CPU_IDLE enabled.
Hmm, how can you tell? I thought you weren't able to reproduce it with
CPU_IDLE disabled either?
- Paul
^ permalink raw reply
* OMAP baseline test results for v3.7-rc1
From: Jean Pihet @ 2012-10-23 19:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1210231918500.27211@utopia.booyaka.com>
On Tue, Oct 23, 2012 at 9:19 PM, Paul Walmsley <paul@pwsan.com> wrote:
> On Mon, 22 Oct 2012, Jean Pihet wrote:
>
>> On Mon, Oct 22, 2012 at 6:12 PM, Jean Pihet <jean.pihet@newoldbits.com> wrote:
>>
>> > Do you have CPU_IDLE enabled?
>> FYI the issue is not present with CPU_IDLE enabled.
>
> Hmm, how can you tell? I thought you weren't able to reproduce it with
> CPU_IDLE disabled either?
I could not reproduce the issue, with and without CPU_IDLE enabled.
What puzzles me is that the PM QoS code only has influence on the
states chosen by cpuidle, so the change should not have any impact
with CPU_IDLE enabled. I reallt need to reproduce the issue.
Let me try with the same setup as yours (bootloader images,
omap2pus_defconfig, angstrom roots).
Regards,
Jean
>
>
> - Paul
^ permalink raw reply
* [PATCH 4/4] ARM: EXYNOS: Kconfig: Remove dependencies on particular SoCs from DT machines
From: Tomasz Figa @ 2012-10-23 19:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <020a01cdb122$2d888ae0$8899a0a0$%kim@samsung.com>
On Tuesday 23 of October 2012 22:27:41 Kukjin Kim wrote:
> Tomasz Figa wrote:
> > MACH_EXYNOS{4,5}_DT are used for whole SoC lines, so they should
> > depend
> > on ARCH_EXYNOS{4,5} rather than on particular SoCs.
> >
> > Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >
> > arch/arm/mach-exynos/Kconfig | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/arch/arm/mach-exynos/Kconfig
> > b/arch/arm/mach-exynos/Kconfig index 6ea95f0..2e82ce7 100644
> > --- a/arch/arm/mach-exynos/Kconfig
> > +++ b/arch/arm/mach-exynos/Kconfig
> > @@ -404,7 +404,6 @@ comment "Flattened Device Tree based board for
> > EXYNOS SoCs"
> >
> > config MACH_EXYNOS4_DT
> >
> > bool "Samsung Exynos4 Machine using device tree"
> > depends on ARCH_EXYNOS4
> >
> > - select SOC_EXYNOS4210
> >
> > select USE_OF
> > select ARM_AMBA
> > select HAVE_SAMSUNG_KEYPAD if INPUT_KEYBOARD
> >
> > @@ -419,7 +418,6 @@ config MACH_EXYNOS4_DT
> >
> > config MACH_EXYNOS5_DT
> >
> > bool "SAMSUNG EXYNOS5 Machine using device tree"
> > depends on ARCH_EXYNOS5
> >
> > - select SOC_EXYNOS5250
> >
> > select USE_OF
> > select ARM_AMBA
> > help
> >
> > --
> > 1.7.12
>
> Hmm...your comment is correct, but we need to think again its selecting
> order, between ARCH name, SoC and board. In addition, in case of
> MACH_XXX_DT depending on SoC is rather to be supposed...
With device tree the situation is a bit different than with boards,
because mach-exynos4-dt does not limit the scope to a single SoC, but
rather to the whole Exynos4 arch/family.
Making MACH_EXYNOS4_DT select all SOC_EXYNOS4* would remove the ability to
enable/disable support for particular Exynos4 SoCs, so in my opinion it is
not a good option.
Selecting ARCH_EXYNOS4 is not enough for the kernel to work, because at
least one SoC must be enabled.
So I think that letting the user select the SoCs he want to be supported
and making MACH_EXYNOS4_DT depend on at least one of Exynos4 SoCs (which
is implied by ARCH_EXYNOS4 enabled) is the most reasonable variant.
Btw. Maybe this could go in pair with something like
static char const *exynos4_dt_compat[] __initdata = {
+#ifdef CONFIG_SOC_EXYNOS4210
"samsung,exynos4210",
+#endif
/* and so on for any Exynos4 SoCs added in future... */
NULL
};
to make the kernel reject booting DT-enabled boards with unsupported SoCs.
What do you think?
Best regards,
Tomasz Figa
^ permalink raw reply
* [PATCH v2 2/2] USB: doc: Binding document for ehci-platform driver
From: Alan Stern @ 2012-10-23 19:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5086E62D.8040407@wwwdotorg.org>
On Tue, 23 Oct 2012, Stephen Warren wrote:
> > Nothing intrinsically distinguishes this class of hardware. The only
> > thing these devices have in common is that they can be managed by
> > Linux's ehci-platform driver.
>
> I don't agree. They're all EHCI USB controllers (or all EHCI USB
> controllers that don't require custom drivers due to quirks), which is a
> much more general statement than anything related to which driver Linux
> uses for them.
That's true, but it doesn't distinguish these devices. There are other
EHCI controllers with no quirks that nevertheless can't be handled by
ehci-platform because they have requirements (_not_ quirks) that
ehci-platform is unable to deal with currently -- clocks or power
supplies or special register settings or PHYs or etc.
> > In essense, we're trying to say that the HW is compatible with a
> > particular driver rather than with some other type of HW.
>
> Using a compatible value in order to pick a specific driver in Linux is
> explicitly against the device tree design principles; DT is supposed to
> represent just the hardware.
>
> > Maybe DT
> > wasn't intended to provide this kind of information, but it seems like
> > a logical thing to do. Unless DT already offers some other way to
> > accomplish the same thing?
>
> The typical way is for the Linux driver to declare that is supports a
> variety of different HW models.
>
> Admittedly this is annoying when there are many HW models that otherwise
> wouldn't need any driver changes, hence the desire to (additionally)
> include some generic value in the compatible field, but again, what that
> generic value is should be driven by the HW itself, not the SW that's
> using it.
Okay, fine. But then what should the binding documentation specify for
the compatible value? A list of all the HW models that the driver
currently supports?
Alan Stern
^ permalink raw reply
* OMAP baseline test results for v3.7-rc2
From: Paul Walmsley @ 2012-10-23 19:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121022163825.GA4730@atomide.com>
On Mon, 22 Oct 2012, Tony Lindgren wrote:
> * Jon Hunter <jon-hunter@ti.com> [121022 09:30]:
> > On 10/20/2012 04:26 PM, Paul Walmsley wrote:
> > >
> > > * 2430sdp: vfp_reload_hw oops during MMC initialization
> > > - Kernel attempts to save FP registers that don't exist; fix posted:
> > > - http://www.spinics.net/lists/arm-kernel/msg200646.html
>
> Has that one been posted to RMK's patch system?
Just did that, let's see what happens.
> > > * AM335x Beaglebone: omap2plus_defconfig kernels don't boot
> > > - due to a GPMC bug
> > > - Apparently fixed by http://www.spinics.net/lists/arm-kernel/msg200787.html
> >
> > This is now addressed and I have verified it is booting on v3.7-rc2. The
> > following patch address this boot problem ...
> >
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=8119024ef7363591fd958ec89ebfaee7c18209e3
>
> That's in v3.7-rc2 already, is there some other problem too?
Will try a re-test.
- Paul
^ permalink raw reply
* [PATCH 1/2] ARM: dt: tegra: Add sdhci regulators
From: Marc Dietrich @ 2012-10-23 19:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5086E503.8090604@wwwdotorg.org>
On Tuesday 23 October 2012 12:42:11 Stephen Warren wrote:
> On 10/23/2012 12:07 PM, Marc Dietrich wrote:
> > Pavan,
> >
> > On Tuesday 23 October 2012 12:48:59 Pavan Kunapuli wrote:
> >> Adding vmmc and vmmcq supplies for sdhci nodes
> >> in tegra dt files.
> >
> > <...>
> >
> >> diff --git a/arch/arm/boot/dts/tegra20-paz00.dts
> >> b/arch/arm/boot/dts/tegra20-paz00.dts index 6a93d14..e161b65 100644
> >> --- a/arch/arm/boot/dts/tegra20-paz00.dts
> >> +++ b/arch/arm/boot/dts/tegra20-paz00.dts
> >> @@ -422,13 +422,17 @@
> >>
> >> status = "okay";
> >> cd-gpios = <&gpio 173 0>; /* gpio PV5 */
> >> wp-gpios = <&gpio 57 0>; /* gpio PH1 */
> >>
> >> - power-gpios = <&gpio 169 0>; /* gpio PV1 */
> >>
> >> bus-width = <4>;
> >>
> >> + vmmc-supply = <&vddio_sd_reg>;
> >> + vqmmc-supply = <&vddio_sd_reg>;
> >>
> >> };
> >>
> >> sdhci at c8000600 {
> >>
> >> status = "okay";
> >> bus-width = <8>;
> >>
> >> + vmmc-supply = <&vddio_sdmmc_reg>;
> >> + vqmmc-supply = <&vddio_sdmmc_reg>;
> >
> > to make it better, this should be:
> >
> > vmmc-supply = <&vcore_mmc_reg>; and
> > vqmmc-supply = <&vddio_nand_reg>;
> >
> > with vcore_mmc_reg is ldo5 and vddio_nand_reg is the fixed regulator
> > below.
>
> I think for the eMMC (not SD card), the core supply is actually +3vs_s3,
> and hence fixed; it looks like LDO5 used to be used, but they changed
> it. See the note for the VCC connections on the eMMC chip.
yes, you are right. The schematic had a comment that this was changed.
> > By doing so, I get "sdhci-tegra sdhci-tegra.3: could not set regulator OCR
> > (-1)". MMC subsys wants to set ldo5 to 3.3V but it is fixed. I guess
> > that's
> > harmless for now.
>
> What I mention above might end up fixing that?
yes, it does.
So the original patch is just fine. Sorry for the noise.
Marc
^ permalink raw reply
* [PATCH 1/2] i2c: mux: Add dt support to i2c-mux-gpio driver
From: Peter Korsgaard @ 2012-10-23 19:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1350910413-23925-2-git-send-email-maxime.ripard@free-electrons.com>
>>>>> "MR" == Maxime Ripard <maxime.ripard@free-electrons.com> writes:
Hi Maxime,
I'm fine with the idea, but a few comments:
MR> Allow the i2c-mux-gpio to be used by a device tree enabled device. The
MR> bindings are inspired by the one found in the i2c-mux-pinctrl driver.
MR> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
MR> ---
MR> .../devicetree/bindings/i2c/i2c-mux-gpio.txt | 81 ++++++++++
MR> drivers/i2c/muxes/i2c-mux-gpio.c | 169 +++++++++++++++-----
MR> 2 files changed, 211 insertions(+), 39 deletions(-)
MR> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
MR> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
MR> new file mode 100644
MR> index 0000000..335fc4e
MR> --- /dev/null
MR> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
MR> @@ -0,0 +1,81 @@
MR> +GPIO-based I2C Bus Mux
MR> +
MR> +This binding describes an I2C bus multiplexer that uses GPIOs to
MR> +route the I2C signals.
MR> +
MR> + +-----+ +-----+
MR> + | dev | | dev |
MR> + +------------+ +-----+ +-----+
MR> + | SoC | | |
MR> + | | /--------+--------+
MR> + | +------+ | +------+ child bus A, on GPIO value set to 0
MR> + | | I2C |-|--| Mux |
MR> + | +------+ | +--+---+ child bus B, on GPIO value set to 1
MR> + | | | \----------+--------+--------+
MR> + | +------+ | | | | |
MR> + | | GPIO |-|-----+ +-----+ +-----+ +-----+
MR> + | +------+ | | dev | | dev | | dev |
MR> + +------------+ +-----+ +-----+ +-----+
MR> +
MR> +Required properties:
MR> +- compatible: i2c-mux-gpio
MR> +- i2c-parent: The phandle of the I2C bus that this multiplexer's master-side
MR> + port is connected to.
MR> +- mux-gpios: list of gpios to use to control the muxer
s/to use to/used to/
MR> +* Standard I2C mux properties. See mux.txt in this directory.
MR> +* I2C child bus nodes. See mux.txt in this directory.
MR> +
MR> +Optional properties:
MR> +- idle-state: value to set to the muxer when idle. When no value is
How about 'bitmask defining mux state when idle' instead?
MR> + given, it defaults to the last value used.
MR> +
MR> +For each i2c child node, an I2C child bus will be created. They will
MR> +be numbered based on the reg property of each node.
As far as I can see they are dynamically assigned numbers in the order
they are listed in the dt.
MR> +
MR> +Whenever an access is made to a device on a child bus, the value set
MR> +in the revelant node's reg property will be output using the list of
MR> +GPIOs, the first in the list holding the most-significant value.
Isn't it the other way around, E.G. first gpio in mux-gpios controlled
by LSB of reg property, next gpio by lsb+1, ..?
MR> +
MR> +If an idle state is defined, using the idle-state (optional) property,
MR> +whenever an access is not being made to a device on a child bus, the
MR> +idle value will be programmed into the GPIOs.
s/idle value will be programmed into the GPIOS/GPIOS set according to
the idle value bitmask/
MR> +
MR> +If an idle state is not defined, the most recently used value will be
MR> +left programmed into hardware whenever no access is being made of a
s/of a/to a/
MR> +device on a child bus.
MR> +
MR> +Example:
MR> + i2cmux {
MR> + compatible = "i2c-mux-gpio";
MR> + #address-cells = <1>;
MR> + #size-cells = <0>;
MR> + mux-gpios = <&gpio1 22 0 &gpio1 23 0>;
MR> + i2c-parent = <&i2c1>;
MR> +
MR> + i2c at 1 {
MR> + reg = <1>;
MR> + #address-cells = <1>;
MR> + #size-cells = <0>;
MR> +
MR> + ssd1307: oled at 3c {
MR> + compatible = "solomon,ssd1307fb-i2c";
MR> + reg = <0x3c>;
MR> + pwms = <&pwm 4 3000>;
MR> + reset-gpios = <&gpio2 7 1>;
MR> + reset-active-low;
MR> + };
MR> + };
MR> +
MR> + i2c at 3 {
MR> + reg = <3>;
MR> + #address-cells = <1>;
MR> + #size-cells = <0>;
MR> +
MR> + pca9555: pca9555 at 20 {
MR> + compatible = "nxp,pca9555";
MR> + gpio-controller;
MR> + #gpio-cells = <2>;
MR> + reg = <0x20>;
MR> + };
MR> + };
MR> + };
MR> diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
MR> index 566a675..7ebef01 100644
MR> --- a/drivers/i2c/muxes/i2c-mux-gpio.c
MR> +++ b/drivers/i2c/muxes/i2c-mux-gpio.c
MR> @@ -16,11 +16,13 @@
MR> #include <linux/module.h>
MR> #include <linux/slab.h>
MR> #include <linux/gpio.h>
MR> +#include <linux/of_i2c.h>
MR> +#include <linux/of_gpio.h>
MR> struct gpiomux {
MR> struct i2c_adapter *parent;
MR> struct i2c_adapter **adap; /* child busses */
MR> - struct i2c_mux_gpio_platform_data data;
MR> + struct i2c_mux_gpio_platform_data *data;
Why this change? I don't see why it is needed and the patch would be a
lot easier to review without all the s/.data/->data/ noise.
MR> unsigned gpio_base;
MR> };
MR> @@ -28,8 +30,8 @@ static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val)
MR> {
MR> int i;
MR> - for (i = 0; i < mux->data.n_gpios; i++)
MR> - gpio_set_value(mux->gpio_base + mux->data.gpios[i],
MR> + for (i = 0; i < mux->data->n_gpios; i++)
MR> + gpio_set_value(mux->gpio_base + mux->data->gpios[i],
MR> val & (1 << i));
MR> }
MR> @@ -37,7 +39,7 @@ static int i2c_mux_gpio_select(struct i2c_adapter *adap, void *data, u32 chan)
MR> {
MR> struct gpiomux *mux = data;
MR> - i2c_mux_gpio_set(mux, mux->data.values[chan]);
MR> + i2c_mux_gpio_set(mux, mux->data->values[chan]);
MR> return 0;
MR> }
MR> @@ -46,7 +48,7 @@ static int i2c_mux_gpio_deselect(struct i2c_adapter *adap, void *data, u32 chan)
MR> {
MR> struct gpiomux *mux = data;
MR> - i2c_mux_gpio_set(mux, mux->data.idle);
MR> + i2c_mux_gpio_set(mux, mux->data->idle);
MR> return 0;
MR> }
MR> @@ -57,29 +59,118 @@ static int __devinit match_gpio_chip_by_label(struct gpio_chip *chip,
MR> return !strcmp(chip->label, data);
MR> }
MR> +#ifdef CONFIG_OF
MR> +static int __devinit i2c_mux_gpio_probe_dt(struct gpiomux *mux,
MR> + struct platform_device *pdev)
MR> +{
MR> + struct device_node *np = pdev->dev.of_node;
MR> + struct device_node *adapter_np, *child;
MR> + struct i2c_adapter *adapter;
MR> + unsigned *values, *gpios;
MR> + int i = 0;
MR> +
MR> + if (!np)
MR> + return 0;
MR> +
MR> + mux->data = devm_kzalloc(&pdev->dev, sizeof(*mux->data),
MR> + GFP_KERNEL);
MR> + if (!mux->data) {
MR> + dev_err(&pdev->dev, "Cannot allocate platform_data");
MR> + return -ENOMEM;
MR> + }
MR> +
MR> + adapter_np = of_parse_phandle(np, "i2c-parent", 0);
MR> + if (!adapter_np) {
MR> + dev_err(&pdev->dev, "Cannot parse i2c-parent\n");
MR> + return -ENODEV;
MR> + }
MR> + adapter = of_find_i2c_adapter_by_node(adapter_np);
MR> + if (!adapter) {
MR> + dev_err(&pdev->dev, "Cannot find parent bus\n");
MR> + return -ENODEV;
MR> + }
MR> + mux->data->parent = i2c_adapter_id(adapter);
MR> + put_device(&adapter->dev);
MR> +
MR> + mux->data->n_values = of_get_child_count(np);
MR> +
MR> + values = devm_kzalloc(&pdev->dev,
MR> + sizeof(*mux->data->values) * mux->data->n_values,
MR> + GFP_KERNEL);
MR> + if (!values) {
MR> + dev_err(&pdev->dev, "Cannot allocate values array");
MR> + return -ENOMEM;
MR> + }
MR> +
MR> + for_each_child_of_node(np, child) {
MR> + of_property_read_u32(child, "reg", values + i);
MR> + i++;
MR> + }
MR> + mux->data->values = values;
MR> +
MR> + if (of_property_read_u32(np, "idle-state", &mux->data->idle))
MR> + mux->data->idle = I2C_MUX_GPIO_NO_IDLE;
MR> +
MR> + mux->data->n_gpios = of_gpio_named_count(np, "mux-gpios");
MR> + if (mux->data->n_gpios < 0) {
MR> + dev_err(&pdev->dev, "Missing mux-gpios property in the DT.\n");
MR> + return -EINVAL;
MR> + }
MR> +
MR> + gpios = devm_kzalloc(&pdev->dev,
MR> + sizeof(*mux->data->gpios) * mux->data->n_gpios,
MR> + GFP_KERNEL);
MR> + if (!gpios) {
MR> + dev_err(&pdev->dev, "Cannot allocate gpios array");
MR> + return -ENOMEM;
MR> + }
MR> +
MR> + for (i = 0; i < mux->data->n_gpios; i++)
MR> + gpios[i] = of_get_named_gpio(np, "mux-gpios", i);
MR> +
MR> + mux->data->gpios = gpios;
MR> +
MR> + return 0;
MR> +}
MR> +#else
MR> +static int __devinit i2c_mux_gpio_probe_dt(struct gpiomux *mux,
MR> + struct platform_device *pdev)
MR> +{
MR> + return 0;
MR> +}
MR> +#endif
MR> +
MR> static int __devinit i2c_mux_gpio_probe(struct platform_device *pdev)
MR> {
MR> struct gpiomux *mux;
MR> - struct i2c_mux_gpio_platform_data *pdata;
MR> struct i2c_adapter *parent;
MR> int (*deselect) (struct i2c_adapter *, void *, u32);
MR> unsigned initial_state, gpio_base;
MR> int i, ret;
MR> - pdata = pdev->dev.platform_data;
MR> - if (!pdata) {
MR> - dev_err(&pdev->dev, "Missing platform data\n");
MR> - return -ENODEV;
MR> + mux = devm_kzalloc(&pdev->dev, sizeof(*mux), GFP_KERNEL);
MR> + if (!mux) {
MR> + dev_err(&pdev->dev, "Cannot allocate gpiomux structure");
MR> + return -ENOMEM;
MR> + }
MR> +
MR> + platform_set_drvdata(pdev, mux);
MR> +
MR> + mux->data = pdev->dev.platform_data;
MR> + if (!mux->data) {
MR> + ret = i2c_mux_gpio_probe_dt(mux, pdev);
MR> + if (ret < 0)
MR> + return ret;
MR> }
MR> /*
MR> * If a GPIO chip name is provided, the GPIO pin numbers provided are
MR> * relative to its base GPIO number. Otherwise they are absolute.
MR> */
MR> - if (pdata->gpio_chip) {
MR> + if (mux->data->gpio_chip) {
MR> struct gpio_chip *gpio;
MR> - gpio = gpiochip_find(pdata->gpio_chip,
MR> + gpio = gpiochip_find(mux->data->gpio_chip,
MR> match_gpio_chip_by_label);
MR> if (!gpio)
MR> return -EPROBE_DEFER;
MR> @@ -89,49 +180,44 @@ static int __devinit i2c_mux_gpio_probe(struct platform_device *pdev)
MR> gpio_base = 0;
MR> }
MR> - parent = i2c_get_adapter(pdata->parent);
MR> + parent = i2c_get_adapter(mux->data->parent);
MR> if (!parent) {
MR> dev_err(&pdev->dev, "Parent adapter (%d) not found\n",
MR> - pdata->parent);
MR> + mux->data->parent);
MR> return -ENODEV;
MR> }
MR> - mux = devm_kzalloc(&pdev->dev, sizeof(*mux), GFP_KERNEL);
MR> - if (!mux) {
MR> - ret = -ENOMEM;
MR> - goto alloc_failed;
MR> - }
MR> -
mux-> parent = parent;
MR> - mux->data = *pdata;
mux-> gpio_base = gpio_base;
MR> +
mux-> adap = devm_kzalloc(&pdev->dev,
MR> - sizeof(*mux->adap) * pdata->n_values,
MR> + sizeof(*mux->adap) * mux->data->n_values,
MR> GFP_KERNEL);
MR> if (!mux->adap) {
MR> + dev_err(&pdev->dev, "Cannot allocate i2c_adapter structure");
MR> ret = -ENOMEM;
MR> goto alloc_failed;
MR> }
MR> - if (pdata->idle != I2C_MUX_GPIO_NO_IDLE) {
MR> - initial_state = pdata->idle;
MR> + if (mux->data->idle != I2C_MUX_GPIO_NO_IDLE) {
MR> + initial_state = mux->data->idle;
MR> deselect = i2c_mux_gpio_deselect;
MR> } else {
MR> - initial_state = pdata->values[0];
MR> + initial_state = mux->data->values[0];
MR> deselect = NULL;
MR> }
MR> - for (i = 0; i < pdata->n_gpios; i++) {
MR> - ret = gpio_request(gpio_base + pdata->gpios[i], "i2c-mux-gpio");
MR> + for (i = 0; i < mux->data->n_gpios; i++) {
MR> + ret = gpio_request(gpio_base + mux->data->gpios[i], "i2c-mux-gpio");
MR> if (ret)
MR> goto err_request_gpio;
MR> - gpio_direction_output(gpio_base + pdata->gpios[i],
MR> + gpio_direction_output(gpio_base + mux->data->gpios[i],
MR> initial_state & (1 << i));
MR> }
MR> - for (i = 0; i < pdata->n_values; i++) {
MR> - u32 nr = pdata->base_nr ? (pdata->base_nr + i) : 0;
MR> - unsigned int class = pdata->classes ? pdata->classes[i] : 0;
MR> + for (i = 0; i < mux->data->n_values; i++) {
MR> + u32 nr = mux->data->base_nr ? (mux->data->base_nr + i) : 0;
MR> + unsigned int class = mux->data->classes ? mux->data->classes[i] : 0;
mux-> adap[i] = i2c_add_mux_adapter(parent, &pdev->dev, mux, nr,
MR> i, class,
MR> @@ -144,19 +230,17 @@ static int __devinit i2c_mux_gpio_probe(struct platform_device *pdev)
MR> }
MR> dev_info(&pdev->dev, "%d port mux on %s adapter\n",
MR> - pdata->n_values, parent->name);
MR> -
MR> - platform_set_drvdata(pdev, mux);
MR> + mux->data->n_values, parent->name);
MR> return 0;
MR> add_adapter_failed:
MR> for (; i > 0; i--)
MR> i2c_del_mux_adapter(mux->adap[i - 1]);
MR> - i = pdata->n_gpios;
MR> + i = mux->data->n_gpios;
MR> err_request_gpio:
MR> for (; i > 0; i--)
MR> - gpio_free(gpio_base + pdata->gpios[i - 1]);
MR> + gpio_free(gpio_base + mux->data->gpios[i - 1]);
MR> alloc_failed:
MR> i2c_put_adapter(parent);
MR> @@ -168,11 +252,11 @@ static int __devexit i2c_mux_gpio_remove(struct platform_device *pdev)
MR> struct gpiomux *mux = platform_get_drvdata(pdev);
MR> int i;
MR> - for (i = 0; i < mux->data.n_values; i++)
MR> + for (i = 0; i < mux->data->n_values; i++)
MR> i2c_del_mux_adapter(mux->adap[i]);
MR> - for (i = 0; i < mux->data.n_gpios; i++)
MR> - gpio_free(mux->gpio_base + mux->data.gpios[i]);
MR> + for (i = 0; i < mux->data->n_gpios; i++)
MR> + gpio_free(mux->gpio_base + mux->data->gpios[i]);
MR> platform_set_drvdata(pdev, NULL);
MR> i2c_put_adapter(mux->parent);
MR> @@ -180,12 +264,19 @@ static int __devexit i2c_mux_gpio_remove(struct platform_device *pdev)
MR> return 0;
MR> }
MR> +static const struct of_device_id i2c_mux_gpio_of_match[] __devinitconst = {
MR> + { .compatible = "i2c-mux-gpio", },
MR> + {},
MR> +};
MR> +MODULE_DEVICE_TABLE(of, i2c_mux_gpio_of_match);
MR> +
MR> static struct platform_driver i2c_mux_gpio_driver = {
MR> .probe = i2c_mux_gpio_probe,
MR> .remove = __devexit_p(i2c_mux_gpio_remove),
MR> .driver = {
MR> .owner = THIS_MODULE,
MR> .name = "i2c-mux-gpio",
MR> + .of_match_table = of_match_ptr(i2c_mux_gpio_of_match),
MR> },
MR> };
MR> --
MR> 1.7.9.5
--
Sorry about disclaimer - It's out of my control.
Bye, Peter Korsgaard
DISCLAIMER:
Unless indicated otherwise, the information contained in this message is privileged and confidential, and is intended only for the use of the addressee(s) named above and others who have been specifically authorized to receive it. If you are not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this message and/or attachments is strictly prohibited. The company accepts no liability for any damage caused by any virus transmitted by this email. Furthermore, the company does not warrant a proper and complete transmission of this information, nor does it accept liability for any delays. If you have received this message in error, please contact the sender and delete the message. Thank you.
^ permalink raw reply
* [PATCHv2] Input: omap4-keypad: Add pinctrl support
From: Dmitry Torokhov @ 2012-10-23 20:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <508660D4.9030507@ti.com>
On Tue, Oct 23, 2012 at 11:18:12AM +0200, Benoit Cousson wrote:
> Hi Dimitry,
>
> On 10/22/2012 05:50 PM, Dmitry Torokhov wrote:
> > Hi Sourav,
> >
> > On Mon, Oct 22, 2012 at 06:43:00PM +0530, Sourav Poddar wrote:
> >> Adapt keypad to use pinctrl framework.
> >>
> >> Tested on omap4430 sdp with 3.7-rc1 kernel.
> >
> > I do not see anything in the driver that would directly use pinctrl. Is
> > there a better place to select default pin configuration; maybe when
> > instantiating platform device?
>
> Why?
> The devm_pinctrl_get_select_default is using the pinctrl.
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.
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.
> 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.
>
> 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?
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH v2 2/2] USB: doc: Binding document for ehci-platform driver
From: Rob Herring @ 2012-10-23 20:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <Pine.LNX.4.44L0.1210231522230.1635-100000@iolanthe.rowland.org>
On 10/23/2012 02:33 PM, Alan Stern wrote:
> On Tue, 23 Oct 2012, Stephen Warren wrote:
>
>>> Nothing intrinsically distinguishes this class of hardware. The only
>>> thing these devices have in common is that they can be managed by
>>> Linux's ehci-platform driver.
>>
>> I don't agree. They're all EHCI USB controllers (or all EHCI USB
>> controllers that don't require custom drivers due to quirks), which is a
>> much more general statement than anything related to which driver Linux
>> uses for them.
>
> That's true, but it doesn't distinguish these devices. There are other
> EHCI controllers with no quirks that nevertheless can't be handled by
> ehci-platform because they have requirements (_not_ quirks) that
> ehci-platform is unable to deal with currently -- clocks or power
> supplies or special register settings or PHYs or etc.
But what if the h/w has quirks you haven't yet discovered? You need the
compatible property to be specific now, so if there are any future
driver changes needed the DT does not have to change (as it may be in
firmware which you can't change).
>>> In essense, we're trying to say that the HW is compatible with a
>>> particular driver rather than with some other type of HW.
>>
>> Using a compatible value in order to pick a specific driver in Linux is
>> explicitly against the device tree design principles; DT is supposed to
>> represent just the hardware.
>>
>>> Maybe DT
>>> wasn't intended to provide this kind of information, but it seems like
>>> a logical thing to do. Unless DT already offers some other way to
>>> accomplish the same thing?
>>
>> The typical way is for the Linux driver to declare that is supports a
>> variety of different HW models.
>>
>> Admittedly this is annoying when there are many HW models that otherwise
>> wouldn't need any driver changes, hence the desire to (additionally)
>> include some generic value in the compatible field, but again, what that
>> generic value is should be driven by the HW itself, not the SW that's
>> using it.
>
> Okay, fine. But then what should the binding documentation specify for
> the compatible value? A list of all the HW models that the driver
> currently supports?
The binding docs should be independent of the driver. There may be
fields the driver ignores. So you need to document all defined
compatible strings. It is fine if the driver only has "usb-ehci", but it
is not fine for the DT to only have that.
Rob
^ permalink raw reply
* [PATCH v2 2/4] zynq: move static peripheral mappings
From: Rob Herring @ 2012-10-23 20:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201210231450.11540.arnd@arndb.de>
On 10/23/2012 09:50 AM, Arnd Bergmann wrote:
> On Monday 22 October 2012, Josh Cartwright wrote:
>> Shifting them up into the vmalloc region prevents the following warning,
>> when booting a zynq qemu target with more than 512mb of RAM:
>>
>> BUG: mapping for 0xe0000000 at 0xe0000000 out of vmalloc space
>>
>> In addition, it allows for reuse of these mappings when the proper
>> drivers issue requests via ioremap().
>>
>> Signed-off-by: Josh Cartwright <josh.cartwright@ni.com>
>
> This looks like a bug fix that should be backported to older kernels,
> so it would be good to add 'Cc: stable at vger.kernel.org' below your
> Signed-off-by.
>
>
>> diff --git a/arch/arm/mach-zynq/include/mach/zynq_soc.h b/arch/arm/mach-zynq/include/mach/zynq_soc.h
>> index d0d3f8f..ae3b236 100644
>> --- a/arch/arm/mach-zynq/include/mach/zynq_soc.h
>> +++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h
>> @@ -15,33 +15,37 @@
>> #ifndef __MACH_XILINX_SOC_H__
>> #define __MACH_XILINX_SOC_H__
>>
>> +#include <asm/pgtable.h>
>> +
>> #define PERIPHERAL_CLOCK_RATE 2500000
>>
>> -/* For now, all mappings are flat (physical = virtual)
>> +/* Static peripheral mappings are mapped at the top of the
>> + * vmalloc region
>> */
>> -#define UART0_PHYS 0xE0000000
>> -#define UART0_VIRT UART0_PHYS
>> +#define UART0_PHYS 0xE0000000
>> +#define UART0_SIZE SZ_4K
>> +#define UART0_VIRT (VMALLOC_END - UART0_SIZE)
>
> There are plans to move the uart location into a fixed virtual
> address in the future, but it hasn't been decided yet.
> It will still need a fixed mapping though, just to a different
> address.
>
>> -#define TTC0_PHYS 0xF8001000
>> -#define TTC0_VIRT TTC0_PHYS
>> +#define TTC0_PHYS 0xF8001000
>> +#define TTC0_SIZE SZ_4K
>> +#define TTC0_VIRT (UART0_VIRT - TTC0_SIZE)
>
> It's quite likely that this does not have to be a fixed mapping
> any more. Just have a look at how drivers/clocksource/dw_apb_timer_of.c
> calls of_iomap() to get the address.
>
>> -#define PL310_L2CC_PHYS 0xF8F02000
>> -#define PL310_L2CC_VIRT PL310_L2CC_PHYS
>> +#define PL310_L2CC_PHYS 0xF8F02000
>> +#define PL310_L2CC_SIZE SZ_4K
>> +#define PL310_L2CC_VIRT (TTC0_VIRT - PL310_L2CC_SIZE)
>
> This address would not need a fixed mapping by calling l2x0_of_init
> rather than l2x0_init.
>
>> -#define SCU_PERIPH_PHYS 0xF8F00000
>> -#define SCU_PERIPH_VIRT SCU_PERIPH_PHYS
>> +#define SCU_PERIPH_PHYS 0xF8F00000
>> +#define SCU_PERIPH_SIZE SZ_8K
>> +#define SCU_PERIPH_VIRT (PL310_L2CC_VIRT - SCU_PERIPH_SIZE)
>
> And your patch 3 already obsoletes this mapping.
Actually, it's probably still needed. The smp platform code typically
reads the number of cores from the SCU and the mapping has to be in
place before ioremap is up. I don't think there is an architected way to
get the number of cores, but it would be nice to avoid this early SCU
access. We could also mandate getting the core count from DT instead.
Also, the physical address can be read with this on A9's:
asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base));
Rob
^ permalink raw reply
* [PATCH v2 2/4] zynq: move static peripheral mappings
From: Nick Bowler @ 2012-10-23 20:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121023162641.GG20593@beefymiracle.amer.corp.natinst.com>
On 2012-10-23 11:26 -0500, Josh Cartwright wrote:
> On Tue, Oct 23, 2012 at 02:50:11PM +0000, Arnd Bergmann wrote:
> > On Monday 22 October 2012, Josh Cartwright wrote:
> > > Shifting them up into the vmalloc region prevents the following warning,
> > > when booting a zynq qemu target with more than 512mb of RAM:
> > >
> > > BUG: mapping for 0xe0000000 at 0xe0000000 out of vmalloc space
> > >
> > > In addition, it allows for reuse of these mappings when the proper
> > > drivers issue requests via ioremap().
> > >
> > > Signed-off-by: Josh Cartwright <josh.cartwright@ni.com>
> >
> > This looks like a bug fix that should be backported to older kernels,
> > so it would be good to add 'Cc: stable at vger.kernel.org' below your
> > Signed-off-by.
>
> Will-do, thanks.
Just FYI, I sent a patch to fix the same bug a while back
https://patchwork.kernel.org/patch/1156361/
together with other patches to fix early printk on the ZC702 serial
console. Admittedly, I dropped the ball on these as other issues
came up so I was away from the Zynq for a while.
However, I'm now getting back on the Zynq and have a bunch of patches to
make it all work on the ZC702 board. I've respun the ZC702 early boot
fixes against newer git but they're obviously going to conflict with
this series. Should I resend them anyway?
> > > -#define TTC0_PHYS 0xF8001000
> > > -#define TTC0_VIRT TTC0_PHYS
> > > +#define TTC0_PHYS 0xF8001000
> > > +#define TTC0_SIZE SZ_4K
> > > +#define TTC0_VIRT (UART0_VIRT - TTC0_SIZE)
> >
> > It's quite likely that this does not have to be a fixed mapping
> > any more. Just have a look at how drivers/clocksource/dw_apb_timer_of.c
> > calls of_iomap() to get the address.
>
> Yes, this is already on my list of plans. The in-tree TTC driver
> unfortunately doesn't yet support device tree bindings. Are you
> comfortable waiting on the DT-ification of the TTC in a follow-up
> patchset?
I also have a DT binding for the TTC driver, I can send that.
Cheers,
--
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)
^ permalink raw reply
* [PATCH] ARM: PMU: fix runtime PM enable
From: Jon Hunter @ 2012-10-23 20:31 UTC (permalink / raw)
To: linux-arm-kernel
Commit 7be2958 (ARM: PMU: Add runtime PM Support) updated the ARM PMU code to
use runtime PM which was prototyped and validated on the OMAP devices. In this
commit, there is no call pm_runtime_enable() and for OMAP devices
pm_runtime_enable() is currently being called from the OMAP PMU code when the
PMU device is created. However, there are two problems with this:
1. For any other ARM device wishing to use runtime PM for PMU they will need
to call pm_runtime_enable() for runtime PM to work.
2. When booting with device-tree and using device-tree to create the PMU
device, pm_runtime_enable() needs to be called from within the ARM PERF
driver as we are no longer calling any device specific code to create the
device. Hence, PMU does not work on OMAP devices that use the runtime PM
callbacks when using device-tree to create the PMU device.
Therefore, add a new platform data variable to indicate whether runtime PM is
used and if so call pm_runtime_enable() when the PMU device is registered. Note
that devices using runtime PM may not use the runtime_resume/suspend callbacks
and so we cannot use the presence of these handlers in the platform data to
determine whether we should call pm_runtime_enable().
Tested with PERF on OMAP2420, OMAP3430 and OMAP4460.
Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
arch/arm/include/asm/pmu.h | 1 +
arch/arm/kernel/perf_event.c | 6 ++++++
arch/arm/mach-omap2/pmu.c | 9 +++++----
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/arch/arm/include/asm/pmu.h b/arch/arm/include/asm/pmu.h
index a26170d..50a6c3b 100644
--- a/arch/arm/include/asm/pmu.h
+++ b/arch/arm/include/asm/pmu.h
@@ -36,6 +36,7 @@
struct arm_pmu_platdata {
irqreturn_t (*handle_irq)(int irq, void *dev,
irq_handler_t pmu_handler);
+ bool use_runtime_pm;
int (*runtime_resume)(struct device *dev);
int (*runtime_suspend)(struct device *dev);
};
diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
index 93971b1..a8c5ddf 100644
--- a/arch/arm/kernel/perf_event.c
+++ b/arch/arm/kernel/perf_event.c
@@ -515,7 +515,13 @@ static void __init armpmu_init(struct arm_pmu *armpmu)
int armpmu_register(struct arm_pmu *armpmu, char *name, int type)
{
+ struct platform_device *plat_device = armpmu->plat_device;
+ struct arm_pmu_platdata *plat = dev_get_platdata(&plat_device->dev);
+
armpmu_init(armpmu);
+ if (plat && plat->use_runtime_pm)
+ pm_runtime_enable(&armpmu->plat_device->dev);
+
pr_info("enabled with %s PMU driver, %d counters available\n",
armpmu->name, armpmu->num_events);
return perf_pmu_register(&armpmu->pmu, name, type);
diff --git a/arch/arm/mach-omap2/pmu.c b/arch/arm/mach-omap2/pmu.c
index 2a79176..1a3d4d7 100644
--- a/arch/arm/mach-omap2/pmu.c
+++ b/arch/arm/mach-omap2/pmu.c
@@ -22,6 +22,7 @@ static char *omap2_pmu_oh_names[] = {"mpu"};
static char *omap3_pmu_oh_names[] = {"mpu", "debugss"};
static char *omap4430_pmu_oh_names[] = {"l3_main_3", "l3_instr", "debugss"};
static struct platform_device *omap_pmu_dev;
+static struct arm_pmu_platdata omap_pmu_data;
/**
* omap2_init_pmu - creates and registers PMU platform device
@@ -49,16 +50,14 @@ static int __init omap2_init_pmu(unsigned oh_num, char *oh_names[])
}
}
- omap_pmu_dev = omap_device_build_ss(dev_name, -1, oh, oh_num, NULL, 0,
- NULL, 0, 0);
+ omap_pmu_dev = omap_device_build_ss(dev_name, -1, oh, oh_num,
+ &omap_pmu_data, sizeof(omap_pmu_data), NULL, 0, 0);
WARN(IS_ERR(omap_pmu_dev), "Can't build omap_device for %s.\n",
dev_name);
if (IS_ERR(omap_pmu_dev))
return PTR_ERR(omap_pmu_dev);
- pm_runtime_enable(&omap_pmu_dev->dev);
-
return 0;
}
@@ -85,9 +84,11 @@ static int __init omap_init_pmu(void)
} else if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
oh_num = ARRAY_SIZE(omap3_pmu_oh_names);
oh_names = omap3_pmu_oh_names;
+ omap_pmu_data.use_runtime_pm = true;
} else {
oh_num = ARRAY_SIZE(omap2_pmu_oh_names);
oh_names = omap2_pmu_oh_names;
+ omap_pmu_data.use_runtime_pm = false;
}
return omap2_init_pmu(oh_num, oh_names);
--
1.7.9.5
^ permalink raw reply related
* [PATCH] ARM: decompressor: clear SCTLR.A bit for v7 cores
From: Gregory CLEMENT @ 2012-10-23 20:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.02.1210111103230.16518@xanadu.home>
On 10/11/2012 05:44 PM, Nicolas Pitre wrote:
> On Thu, 11 Oct 2012, Russell King - ARM Linux wrote:
>
>> On Thu, Oct 11, 2012 at 08:31:47AM -0500, Rob Herring wrote:
>>> On 10/11/2012 08:09 AM, Russell King - ARM Linux wrote:
>>>> On Thu, Oct 11, 2012 at 07:43:22AM -0500, Rob Herring wrote:
>>>>> The contents of this were already reviewed on this thread, so I sent this
>>>>> to the patch system and this was Russell's reply:
>>>>
>>>> So that's why I couldn't find it - the mailing list thread has a different
>>>> subject line to the patch. Don't do that. Given the amount of list
>>>> traffic we have today, that's as good as not having been posted at all.
>>>>
>>>>>> NAK for two reasons.
>>>>>>
>>>>>> 1. It hasn't been on the list (I can't find a match for "clear SCTLR.A"
>>>>>> in my mailbox)
>>>>>>
>>>>>> 2. The behaviour of unaligned accesses vary depending on CPU. Some
>>>>>> fix-up the access, others load the word and then rotate it. If we have
>>>>>> decompressors which perform unaligned accesses, we need to fix this
>>>>>> properly to avoid the CPU specific behaviour, rather than tweaking
>>>>>> control bits to hide the problem.
>>>>>
>>>>> I'm simply matching the behavior of the kernel itself. The A bit is cleared
>>>>> for v7 kernels and compilers only generate unaligned accesses for v7.
>>>>> Without this the initial state of the A bit is undefined as a bootloader
>>>>> could have cleared it already. We should document the required state or set
>>>>> it to what we want.
>>>>
>>>> Irrespective of this, (2) still stands. Unaligned accesses in the
>>>> decompressor without a fixup (which will be very hard to provide)
>>>> will return different data depending on the CPU as I mention in point
>>>> 2.
>>>
>>> This only affects v7 cores. It should not vary for v7 cores as unaligned
>>> access is a required feature. So how is it going to vary on v7 CPUs?
>>> We've got bigger problems if there are v7 cores that don't handle
>>> unaligned accesses.
>>
>> Rob,
>>
>> Your patch may only affect v7 cores, but you've raised the issue of the
>> decompressor performing unaligned accesses in general. Shall I re-repeat
>> my point over that or is the problem here going to finally sink in?
>
> The decompressor is not performing direct unaligned accesses. It uses
> the get_unaligned() and put_unaligned() accessors. That means that
> we're in control of how this is happening.
>
> So let's talk about the how. On pre ARMv7, those accesses are performed
> with a series of byte accesses. When compiling for ARMv7, gcc knows and
> that the hardware can do unaligned accesses, and it does optimize its
> output by using ldr/str instructions. But the A bit has to be cleared
> in that case, and only in that case. This is why the patch clears the A
> bit only for ARMv7.
>
> So this patch is only setting up the hardware to match gcc's
> expectations when generating code from the use of get_unaligned() and
> put_unaligned() when optimizing for ARMv7.
>
> As always, any code doing unaligned access and _not_ using those
> accessors is broken.
So is there a chance that this patch will be applied for 3.7?
Currently I can't boot anymore Armada XP or Armada 370, if kernel is
compressed in LZO. It's annoying.
Russell, did Nicolas manage to convince you?
If not, what should be the solution to fix this issue?
Thanks,
Gregory
^ permalink raw reply
* [PATCH v2 0/4] zynq subarch cleanups
From: Rob Herring @ 2012-10-23 20:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201210231441.29107.arnd@arndb.de>
On 10/23/2012 09:41 AM, Arnd Bergmann wrote:
> On Monday 22 October 2012, Josh Cartwright wrote:
>> Hey all-
>>
>> Things have been relatively quiet on the Zynq front lately. This
>> patchset does a bit of cleanup of the Zynq subarchitecture. It was the
>> necessary set of things I had to do to get a zynq target booting with
>> the upstream qemu model. It removes some unused clock infrastructure,
>> adds DT support for the GIC, and moves around peripheral mappings.
>
> Ok, thanks for these patches. As we have moved on for some of these issues,
> I'll comment on how to go even further. On a global scale, I wonder
> if there are any obstacles for enabling CONFIG_MULTIPLATFORM on Zynq
> as we have done for most of the other recently added platform.
IIRC, the only thing I saw preventing it was the custom clk usage.
Rob
^ permalink raw reply
* [PATCH v2 2/4] zynq: move static peripheral mappings
From: Josh Cartwright @ 2012-10-23 20:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5086F973.6050309@gmail.com>
On Tue, Oct 23, 2012 at 03:09:23PM -0500, Rob Herring wrote:
> On 10/23/2012 09:50 AM, Arnd Bergmann wrote:
> > On Monday 22 October 2012, Josh Cartwright wrote:
> >> -#define SCU_PERIPH_PHYS 0xF8F00000
> >> -#define SCU_PERIPH_VIRT SCU_PERIPH_PHYS
> >> +#define SCU_PERIPH_PHYS 0xF8F00000
> >> +#define SCU_PERIPH_SIZE SZ_8K
> >> +#define SCU_PERIPH_VIRT (PL310_L2CC_VIRT - SCU_PERIPH_SIZE)
> >
> > And your patch 3 already obsoletes this mapping.
>
> Actually, it's probably still needed. The smp platform code typically
> reads the number of cores from the SCU and the mapping has to be in
> place before ioremap is up. I don't think there is an architected way to
> get the number of cores, but it would be nice to avoid this early SCU
> access. We could also mandate getting the core count from DT instead.
>
> Also, the physical address can be read with this on A9's:
>
> asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base));
For the sake of the zynq cleanups, I think it may still make sense to
remove the SCU peripheral mappings for now. By the time we're ready to
push in SMP support for zynq, maybe we can tackle the problem of how to
solve the SCU mapping problem generically.
If we're already considering a architecture-agnostic way to maintain the
early uart mapping, would it make sense to handle this in a similar way?
Thanks,
Josh
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121023/518ac82a/attachment.sig>
^ permalink raw reply
* OMAP baseline test results for v3.7-rc2
From: Kevin Hilman @ 2012-10-23 21:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121023131153.GE1855@beef>
Matt Porter <mporter@ti.com> writes:
[...]
> Ok, very quick update...no need to mess around with the eeprom. I just
> received the official word on what will be supported. Since A2 is
> pre-release, simply go to http://beagleboard.org/support/rma and fill
> out the form to have it replaced with the current revision (A6).
>
> This applies to *anyone* that received a pre-release A2 board.
Hmm, doesn't seem the RMA people are aware of this. They just rejected
my request since it's not a "hardware related issue":
Kevin,
We only work with hardware related issue here. BeagleBone revision A2
was not officially released. We recommend you to work with the
community for support in Uboot. You can post your questions at the
BeagleBoard Google Groups:
https://groups.google.com/forum/?fromgroups=#!forum/beagleboard
Regards,
RMA Team
^ permalink raw reply
* OMAP baseline test results for v3.7-rc2
From: Matt Porter @ 2012-10-23 21:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <877gqg99zk.fsf@deeprootsystems.com>
On Tue, Oct 23, 2012 at 02:03:43PM -0700, Kevin Hilman wrote:
> Matt Porter <mporter@ti.com> writes:
>
> [...]
>
> > Ok, very quick update...no need to mess around with the eeprom. I just
> > received the official word on what will be supported. Since A2 is
> > pre-release, simply go to http://beagleboard.org/support/rma and fill
> > out the form to have it replaced with the current revision (A6).
> >
> > This applies to *anyone* that received a pre-release A2 board.
>
> Hmm, doesn't seem the RMA people are aware of this. They just rejected
> my request since it's not a "hardware related issue":
>
> Kevin,
>
> We only work with hardware related issue here. BeagleBone revision A2
> was not officially released. We recommend you to work with the
> community for support in Uboot. You can post your questions at the
> BeagleBoard Google Groups:
> https://groups.google.com/forum/?fromgroups=#!forum/beagleboard
>
> Regards,
> RMA Team
That's sad considering the beagleboard.org/BeagleBone hardware designer
specifically said they would be replaced via this route. *sigh*. Ok, I'll
go back and share this with him.
-Matt
^ permalink raw reply
* [PATCH v2 2/4] zynq: move static peripheral mappings
From: Nick Bowler @ 2012-10-23 21:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121023205304.GJ20593@beefymiracle.amer.corp.natinst.com>
On 2012-10-23 15:53 -0500, Josh Cartwright wrote:
> On Tue, Oct 23, 2012 at 03:09:23PM -0500, Rob Herring wrote:
> > On 10/23/2012 09:50 AM, Arnd Bergmann wrote:
> > > On Monday 22 October 2012, Josh Cartwright wrote:
> > >> -#define SCU_PERIPH_PHYS 0xF8F00000
> > >> -#define SCU_PERIPH_VIRT SCU_PERIPH_PHYS
> > >> +#define SCU_PERIPH_PHYS 0xF8F00000
> > >> +#define SCU_PERIPH_SIZE SZ_8K
> > >> +#define SCU_PERIPH_VIRT (PL310_L2CC_VIRT - SCU_PERIPH_SIZE)
> > >
> > > And your patch 3 already obsoletes this mapping.
> >
> > Actually, it's probably still needed. The smp platform code typically
> > reads the number of cores from the SCU and the mapping has to be in
> > place before ioremap is up. I don't think there is an architected way to
> > get the number of cores, but it would be nice to avoid this early SCU
> > access. We could also mandate getting the core count from DT instead.
> >
> > Also, the physical address can be read with this on A9's:
> >
> > asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base));
>
> For the sake of the zynq cleanups, I think it may still make sense to
> remove the SCU peripheral mappings for now. By the time we're ready to
> push in SMP support for zynq, maybe we can tackle the problem of how to
> solve the SCU mapping problem generically.
Then the static mapping can be removed if and when the we "solve the SCU
mapping problem generically". There's no point in removing it until
then since it doesn't cause any actual problems, does it?
Cheers,
--
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)
^ permalink raw reply
* [PATCH v2 2/4] zynq: move static peripheral mappings
From: Josh Cartwright @ 2012-10-23 21:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121023211742.GA14047@elliptictech.com>
On Tue, Oct 23, 2012 at 05:17:42PM -0400, Nick Bowler wrote:
> On 2012-10-23 15:53 -0500, Josh Cartwright wrote:
> > On Tue, Oct 23, 2012 at 03:09:23PM -0500, Rob Herring wrote:
> > > On 10/23/2012 09:50 AM, Arnd Bergmann wrote:
> > > > On Monday 22 October 2012, Josh Cartwright wrote:
> > > >> -#define SCU_PERIPH_PHYS 0xF8F00000
> > > >> -#define SCU_PERIPH_VIRT SCU_PERIPH_PHYS
> > > >> +#define SCU_PERIPH_PHYS 0xF8F00000
> > > >> +#define SCU_PERIPH_SIZE SZ_8K
> > > >> +#define SCU_PERIPH_VIRT (PL310_L2CC_VIRT - SCU_PERIPH_SIZE)
> > > >
> > > > And your patch 3 already obsoletes this mapping.
> > >
> > > Actually, it's probably still needed. The smp platform code typically
> > > reads the number of cores from the SCU and the mapping has to be in
> > > place before ioremap is up. I don't think there is an architected way to
> > > get the number of cores, but it would be nice to avoid this early SCU
> > > access. We could also mandate getting the core count from DT instead.
> > >
> > > Also, the physical address can be read with this on A9's:
> > >
> > > asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base));
> >
> > For the sake of the zynq cleanups, I think it may still make sense to
> > remove the SCU peripheral mappings for now. By the time we're ready to
> > push in SMP support for zynq, maybe we can tackle the problem of how to
> > solve the SCU mapping problem generically.
>
> Then the static mapping can be removed if and when the we "solve the SCU
> mapping problem generically". There's no point in removing it until
> then since it doesn't cause any actual problems, does it?
That's also fine with me as well. I'm not strongly opinionated, and not
convinced it really matters much.
Thanks,
Josh
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121023/2fb200b7/attachment.sig>
^ permalink raw reply
* [RFC PATCH] dt: describe base reset signal binding
From: Stephen Warren @ 2012-10-23 21:45 UTC (permalink / raw)
To: linux-arm-kernel
From: Stephen Warren <swarren@nvidia.com>
This binding is intended to represent the hardware reset signals present
internally in most IC (SoC, FPGA, ...) designs.
Such a binding would allow the creation of a "reset subsystem", which
could replace APIs such as the following Tegra-specific API:
void tegra_periph_reset_deassert(struct clk *c);
void tegra_periph_reset_assert(struct clk *c);
(Note that at present, Tegra couples reset assertion with the clock for
the affected peripheral module. However, reset and clocking are two
separate, yet admittedly related, concepts).
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
What do people think of this? Does it sound like a good idea to go ahead
with a reset subsystem? Should we simply add a new API to the common clock
subsystem instead (and assume that reset and clock domains match 1:1).
Should this be implemented as part of the generic power management domains;
see include/linux/pm_domain.h instead?
Documentation/devicetree/bindings/reset/reset.txt | 75 +++++++++++++++++++++
1 files changed, 75 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/reset/reset.txt
diff --git a/Documentation/devicetree/bindings/reset/reset.txt b/Documentation/devicetree/bindings/reset/reset.txt
new file mode 100644
index 0000000..31db6ff
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/reset.txt
@@ -0,0 +1,75 @@
+= Reset Signal Device Tree Bindings =
+
+This binding is intended to represent the hardware reset signals present
+internally in most IC (SoC, FPGA, ...) designs. Reset signals for whole
+standalone chips are most likely better represented as GPIOs, although there
+are likely to be exceptions to this rule.
+
+Hardware blocks typically receive a reset signal. This signal is generated by
+a reset provider (e.g. power management or clock module) and received by a
+reset consumer (the module being reset, or a module managing when a sub-
+ordinate module is reset). This binding exists to represent the provider and
+consumer, and provide a way to couple the two together.
+
+A reset signal is represented by the phandle of the provider, plus a reset
+specifier - a list of DT cells that represents the reset signal within the
+provider. The length (number of cells) and semantics of the reset specifier
+are dictated by the binding of the reset provider, although common schemes
+are described below.
+
+A word on where to place reset signal consumers in device tree: It is possible
+in hardware for a reset signal to affect multiple logically separate HW blocks
+at once. In this case, it would be unwise to represent this reset signal in
+the DT node of each affected HW block, since if activated, an unrelated block
+may be reset. Instead, reset signals should be represented in the DT node
+where it makes most sense to control it; this may be a bus node if all
+children of the bus are affected by the reset signal, or an individual HW
+block node for dedicated reset signals. The intent of this binding is to give
+appropriate software access to the reset signals in order to manage the HW,
+rather than to slavishly enumerate the reset signal that affects each HW
+block.
+
+= Reset providers =
+
+Required properties:
+#reset-cells: Number of cells in a reset specifier; Typically 0 for nodes
+ with a single reset output and 1 for nodes with multiple
+ reset outputs.
+
+For example:
+
+ rst: reset-controller {
+ #reset-cells = <1>;
+ };
+
+= Reset consumers =
+
+Required properties:
+resets: List of phandle and reset specifier pairs, one pair
+ for each reset signal that affects the device, or that the
+ device manages. Note: if the reset provider specifies '0' for
+ #reset-cells, then only the phandle portion of the pair will
+ appear.
+
+Optional properties:
+reset-names: List of reset signal name strings sorted in the same order as
+ the resets property. Consumers drivers will use reset-names to
+ match reset signal names with reset specifiers.
+
+For example:
+
+ device {
+ resets = <&rst 20>;
+ reset-names = "reset";
+ };
+
+This represents a device with a single reset signal named "reset".
+
+ bus {
+ resets = <&rst 10> <&rst 11> <&rst 12> <&rst 11>;
+ reset-names = "i2s1", "i2s2", "dma", "mixer";
+ };
+
+This represents a bus that controls the reset signal of each of four sub-
+ordinate devices. Consider for example a bus that fails to operate unless no
+child device has reset asserted.
--
1.7.0.4
^ permalink raw reply related
* [PATCH] gpio/omap: fix off-mode bug: clear debounce clock enable mask on disable
From: Kevin Hilman @ 2012-10-23 22:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121023190914.GA853@arwen.pp.htv.fi>
Felipe Balbi <balbi@ti.com> writes:
> Hi,
>
> On Tue, Oct 23, 2012 at 11:09:31AM -0700, Kevin Hilman wrote:
>> From: Kevin Hilman <khilman@ti.com>
>>
>> When debounce clocks are disabled, 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.
>> Then, enabling off mode causing bogus state to be restored, leaving
>> GPIO debounce enabled which then prevented the CORE powerdomain from
>> transitioning.
>>
>> Reported-by: Paul Walmsley <paul@pwsan.com>
>> Cc: Igor Grinberg <grinberg@compulab.co.il>
>> Signed-off-by: Kevin Hilman <khilman@ti.com>
>
> looks like this deserves a Cc: stable at vger.kernel.org tag.
>
Agreed. I think this goes all the way back to v3.5, but would've only
been seen on boards using a request/gpio_set_debounce/free sequence
combined with off-mode.
Linus, feel free to add the Cc: stable when commiting. Thanks.
>> ---
>> Applies on v3.7-rc2, targetted for v3.7.
>>
>> 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..dee2856 100644
>> --- a/drivers/gpio/gpio-omap.c
>> +++ b/drivers/gpio/gpio-omap.c
>> @@ -187,6 +187,7 @@ static inline void _gpio_dbck_disable(struct gpio_bank *bank)
>> * to detect events and generate interrupts at least on OMAP3.
>> */
>> __raw_writel(0, bank->base + bank->regs->debounce_en);
>> + bank->dbck_enable_mask = 0;
>
> shouldn't omap_gpio_restore_context() check for dbck_enabled instead of
> the mask ? I mean:
>
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 94cbc84..b3a39a7 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -1371,7 +1371,7 @@ static void omap_gpio_restore_context(struct gpio_bank *bank)
> bank->base + bank->regs->dataout);
> __raw_writel(bank->context.oe, bank->base + bank->regs->direction);
>
> - if (bank->dbck_enable_mask) {
> + if (bank->dbck_enabled) {
> __raw_writel(bank->context.debounce, bank->base +
> bank->regs->debounce);
> __raw_writel(bank->context.debounce_en,
>
> the outcome would be the same, so it doesn't really matter. Just that,
> at least to me, it would look better.
I tried your version, and unfortunately, the outcome is not the same,
but don't plan to look into why. $SUBJECT version is targetted and
tested. If you want to cleanup the cosmetics here, please do in a
subsequent patch. This driver could certainly benefit from more
readability cleanups.
> No strong feelings though.
Good. I'll take that as an Ack. :)
Kevin
^ permalink raw reply
* [PATCH 1/7] ARM: OMAP2xxx: hwmod: Convert SHAM crypto device data to hwmod
From: Mark A. Greer @ 2012-10-23 22:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1210221935320.13464@utopia.booyaka.com>
On Mon, Oct 22, 2012 at 07:49:47PM +0000, Paul Walmsley wrote:
> On Mon, 22 Oct 2012, Mark A. Greer wrote:
>
> > On Sat, Oct 20, 2012 at 07:40:19PM +0000, Paul Walmsley wrote:
> >
> > > > static void omap_init_sham(void)
> > > > {
> > > > - if (cpu_is_omap24xx()) {
> > > > - sham_device.resource = omap2_sham_resources;
> > > > - sham_device.num_resources = omap2_sham_resources_sz;
> > > > + if (cpu_is_omap24xx() &&
> > > > + (omap2_cm_read_mod_reg(CORE_MOD, OMAP24XX_CM_IDLEST4) &
> > > > + OMAP24XX_ST_SHA_MASK)) {
> > >
> > > Hmm. Not sure I understand the purpose of this CM read. Don't we want to
> > > initialize this device unconditionally?
> >
> > No, the device doesn't exist on all parts.
>
> It should exist on all OMAP2xxx, AFAIK. (Whether some bootloader
> firewalled it off is another matter, of course.)
>
> > This is the only way to tell if its there (AFAIK).
>
> Hmm. I don't think the CM_IDLEST bits work that way, in the general case.
> They just indicate whether the PRCM considers that IP block to be
> currently accessible. So for example if the clocks are disabled to an IP
> block, the CM_IDLEST bit would indicate that it's not accessible. The
> software could then enable the clocks, and the CM_IDLEST bit would
> indicate that it is accessible. We use this in the clock framework and
> hwmod code after enabling clocks to wait until the system considers the IP
> block accessible. See for example
>
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/clock.c;h=961ac8f7e13d8c84a1cbb4587255ea685520bd18;hb=HEAD#l80
>
> ...
>
> Now it's possible that on some TI chips, the CM_IDLEST bit is tied to the
> SIdleAck signal originating from the IP block. (I've been told that on
> other OMAPs, CM_IDLEST is actually tied to the SIdleReq signal originating
> from the PRCM, which is not terribly useful...) So if it's tied to the
> SIdleAck signal, and the PRCM clocks are enabled, then it might provide an
> indication of whether the IP block exists on that chip. But ultimately
> the IP block might still be firewalled off even if it appears to exist to
> the PRCM. So for 3xxx, I'd suggest just adding it to the appropriate
> GP-specific hwmod init lists, such as omap3xxx_gp_hwmod_ocp_ifs.
Paul, thank you for the clear explanation.
I've talked to some hardware guys and I have come to the following
conclusions:
a) What you say above about CM_IDLEST is correct.
b) There seems to be no good way, in general, to tell if the SHA
(or AES or RNG or ...) module exists.
c) The best thing to do is what you suggest and add the SHA hwmod
data for all omap2's and omap3 GP's.
I will respin my patches with everyone's comments addressed later today.
Thanks everyone.
Mark
--
^ permalink raw reply
* [RFC] Energy/power monitoring within the kernel
From: Guenter Roeck @ 2012-10-23 22:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1351013449.9070.5.camel@hornet>
On Tue, Oct 23, 2012 at 06:30:49PM +0100, Pawel Moll wrote:
> Greetings All,
>
> 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...
>
Only driver supporting "energy" output so far is ibmaem, and the reported energy
is supposed to be cumulative, as in energy = power * time. Do you mean power,
possibly ?
> 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. The second
> most frequent request is about providing it to a "energy aware"
> cpufreq governor.
>
Anything energy related would have to be along the line of "do something after a
certain amount of work has been performed", which at least at the surface does
not make much sense to me, unless you mean something along the line of a
process scheduler which schedules a process not based on time slices but based
on energy consumed, ie if you want to define a time slice not in milli-seconds
but in Joule.
If so, I would argue that a similar behavior could be achieved by varying the
duration of time slices with the current CPU speed, or simply by using cycle
count instead of time as time slice parameter. Not that I am sure if such an
approach would really be of interest for anyone.
Or do you really mean power, not energy, such as in "reduce CPU speed if its
power consumption is above X Watt" ?
> I've came up with three (non-mutually exclusive) options. I will
> appreciate any other ideas and comments (including "it makes not sense
> whatsoever" ones, with justification). Of course I am more than willing
> to spend time on prototyping anything that seems reasonable and propose
> patches.
>
>
>
> === Option 1: Trace event ===
>
> This seems to be the "cheapest" option. Simply defining a trace event
> that can be generated by a hwmon (or any other) driver makes the
> interesting data immediately available to any ftrace/perf user. Of
> course it doesn't really help with the cpufreq case, but seems to be
> a good place to start with.
>
> The question is how to define it... I've came up with two prototypes:
>
> = Generic hwmon trace event =
>
> This one allows any driver to generate a trace event whenever any
> "hwmon attribute" (measured value) gets updated. The rate at which the
> updates happen can be controlled by already existing "update_interval"
> attribute.
>
> 8<-------------------------------------------
> TRACE_EVENT(hwmon_attr_update,
> TP_PROTO(struct device *dev, struct attribute *attr, long long input),
> TP_ARGS(dev, attr, input),
>
> TP_STRUCT__entry(
> __string( dev, dev_name(dev))
> __string( attr, attr->name)
> __field( long long, input)
> ),
>
> TP_fast_assign(
> __assign_str(dev, dev_name(dev));
> __assign_str(attr, attr->name);
> __entry->input = input;
> ),
>
> TP_printk("%s %s %lld", __get_str(dev), __get_str(attr), __entry->input)
> );
> 8<-------------------------------------------
>
> It generates such ftrace message:
>
> <...>212.673126: hwmon_attr_update: hwmon4 temp1_input 34361
>
> One issue with this is that some external knowledge is required to
> relate a number to a processor core. Or maybe it's not an issue at all
> because it should be left for the user(space)?
>
> = CPU power/energy/temperature trace event =
>
> This one is designed to emphasize the relation between the measured
> value (whether it is energy, temperature or any other physical
> phenomena, really) and CPUs, so it is quite specific (too specific?)
>
> 8<-------------------------------------------
> TRACE_EVENT(cpus_environment,
> TP_PROTO(const struct cpumask *cpus, long long value, char unit),
> TP_ARGS(cpus, value, unit),
>
> TP_STRUCT__entry(
> __array( unsigned char, cpus, sizeof(struct cpumask))
> __field( long long, value)
> __field( char, unit)
> ),
>
> TP_fast_assign(
> memcpy(__entry->cpus, cpus, sizeof(struct cpumask));
> __entry->value = value;
> __entry->unit = unit;
> ),
>
> TP_printk("cpus %s %lld[%c]",
> __print_cpumask((struct cpumask *)__entry->cpus),
> __entry->value, __entry->unit)
> );
> 8<-------------------------------------------
>
> And the equivalent ftrace message is:
>
> <...>127.063107: cpus_environment: cpus 0,1,2,3 34361[C]
>
> It's a cpumask, not just single cpu id, because the sensor may measure
> the value per set of CPUs, eg. a temperature of the whole silicon die
> (so all the cores) or an energy consumed by a subset of cores (this
> is my particular use case - two meters monitor a cluster of two
> processors and a cluster of three processors, all working as a SMP
> system).
>
> Of course the cpus __array could be actually a special __cpumask field
> type (I've just hacked the __print_cpumask so far). And I've just
> realised that the unit field should actually be a string to allow unit
> prefixes to be specified (the above should obviously be "34361[mC]"
> not "[C]"). Also - excuse the "cpus_environment" name - this was the
> best I was able to come up with at the time and I'm eager to accept
> any alternative suggestions :-)
>
I am not sure how this would be expected to work. hwmon is, by its very nature,
a passive subsystem: It doesn't do anything unless data is explicitly requested
from it. It does not update an attribute unless that attribute is read.
That does not seem to fit well with the idea of tracing - which assumes
that some activity is happening, ultimately, all by itself, presumably
periodically. The idea to have a user space application read hwmon data only
for it to trigger trace events does not seem to be very compelling to me.
An exception is if a monitoring device suppports interrupts, and if its driver
actually implements those interrupts. This is, however, not the case for most of
the current drivers (if any), mostly because interrupt support for hardware
monitoring devices is very platform dependent and thus difficult to implement.
>
> === Option 2: hwmon perf PMU ===
>
> Although the trace event makes it possible to obtain interesting
> information using perf, the user wouldn't be able to treat the
> energy meter as a normal data source. In particular there would
> be no way of creating a group of events consisting eg. of a
> "normal" leader (eg. cache miss event) triggering energy meter
> read. The only way to get this done is to implement a perf PMU
> backend providing "environmental data" to the user.
>
> = High-level hwmon API and PMU =
>
> Current hwmon subsystem does not provide any abstraction for the
> measured values and requires particular drivers to create specified
> sysfs attributes than used by userspace libsensors. This makes
> the framework ultimately flexible and ultimately hard to access
> from within the kernel...
>
> What could be done here is some (simple) API to register the
> measured values with the hwmon core which would result in creating
> equivalent sysfs attributes automagically, but also allow a
> in-kernel API for values enumeration and access. That way the core
> could also register a "hwmon PMU" with the perf framework providing
> data from all "compliant" drivers.
>
> = A driver-specific PMU =
>
> Of course a particular driver could register its own perf PMU on its
> own. It's certainly an option, just very suboptimal in my opinion.
> Or maybe not? Maybe the task is so specialized that it makes sense?
>
We had a couple of attempts to provide an in-kernel API. Unfortunately,
the result was, at least so far, more complexity on the driver side.
So the difficulty is really to define an API which is really simple, and does
not just complicate driver development for a (presumably) rare use case.
Guenter
>
>
> === Option 3: CPU power(energy) monitoring framework ===
>
> And last but not least, maybe the problem deserves some dedicated
> API? Something that would take providers and feed their data into
> interested parties, in particular a perf PMU implementation and
> cpufreq governors?
>
> Maybe it could be an extension to the thermal framework? It already
> gives some meaning to a physical phenomena. Adding other, related ones
> like energy, and relating it to cpu cores could make some sense.
>
>
>
> I've tried to gather all potentially interested audience in the To:
> list, but if I missed anyone - please, do let them (and/or me) know.
>
> Best regards and thanks for participation in the discussion!
>
> Pawel
>
>
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox