Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH/RFT v2 11/17] USB: OHCI: make ohci-da8xx a separate driver
From: David Lechner @ 2016-10-25 16:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKXjFTM8RwsAut8cD0n0foxueSjR4VB4k4WJizXo9SyaW3viOA@mail.gmail.com>

On 10/25/2016 11:21 AM, Axel Haslam wrote:
> On Tue, Oct 25, 2016 at 6:12 PM, David Lechner <david@lechnology.com> wrote:
>> On 10/25/2016 02:39 AM, Axel Haslam wrote:
>>>
>>> On Tue, Oct 25, 2016 at 2:38 AM, David Lechner <david@lechnology.com>
>>> wrote:
>>>>
>>>> On 10/24/2016 11:46 AM, ahaslam at baylibre.com wrote:
>>>>>
>>>>>
>>>>> -#ifndef CONFIG_ARCH_DAVINCI_DA8XX
>>>>> -#error "This file is DA8xx bus glue.  Define
>>>>> CONFIG_ARCH_DAVINCI_DA8XX."
>>>>> -#endif
>>>>> +#include "ohci.h"
>>>>> +
>>>>> +#define DRIVER_DESC "OHCI DA8XX driver"
>>>>> +
>>>>> +static const char hcd_name[] = "ohci-da8xx";
>>>>
>>>>
>>>>
>>>> why static const char instead of #define? This is only used one time in a
>>>> pr_info, so it seems kind of pointless anyway.
>>>
>>>
>>> Other drivers are using static const for the same variable.
>>> i think static const is preferred over #define because #define doet give a
>>> type.
>>> If you dont mind ill keep it static const.
>>>
>>
>> If this string was used in this file more than one place, I would agree with
>> you, but currently it is only used as the argument of a pr_info(). The
>> string "ohci-da8xx" could just be included in the fmt string instead of
>> using "%s".
>
> I think the purpose was to use it in the .name of the platform_driver
> structure, too. only that not everybody is doing that, i looked at some bad
>  examples :(
>
> would you agree to keep it if we use it in .name too?
>
> -Axel
>
>>

Yes, that will make more sense. ;-)

^ permalink raw reply

* [PATCH] tty/serial: at91: fix hardware handshake on Atmel platforms
From: Alexandre Belloni @ 2016-10-25 16:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161025161135.7316-1-richard.genoud@gmail.com>

Hi,

On 25/10/2016 at 18:11:35 +0200, Richard Genoud wrote :
> commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when
> hardware handshake is enabled"), despite its title, broke hardware
> handshake on *every* Atmel platforms.
> 
> The only one partially working is the SAMA5D2.
> 

[...]

> Changes since v4:
>  - the mctrl_gpio_use_rtscts() is gone since it was atmel_serial
>  specific. (so patch 1 is gone)
>  - patches 2 and 3 have been merged together since it didn't make
>  a lot of sense to correct the GPIO case in one separate patch.
>  - ATMEL_US_USMODE_HWHS is now unset for platform with PDC
> 
> Changes since v3:
>  - remove superfluous #include <linux/err.h> (thanks to Uwe)
>  - rebase on next-20160930
> 
> Changes since v2:
>  - remove IS_ERR_OR_NULL() test in patch 1/3 as Uwe suggested.
>  - fix typos in patch 2/3
>  - rebase on next-20160927
>  - simplify the logic in patch 3/3.
> 
> Changes since v1:
>  - Correct patch 1 with the error found by kbuild.
>  - Add Alexandre's Acked-by on patch 2
>  - Rewrite patch 3 logic in the light of the on-going discussion
>    with Cyrille and Alexandre.
> 

The changelog has to go after the --- marker.

> * the list may not be exhaustive
> 
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>

Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

> ---
>  drivers/tty/serial/atmel_serial.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
>  I think this should go in the stable tree since it fixes the flow
>  control broken since v4.0.
>  But It won't compile on versions before 4.9rc1 because:
>  function atmel_use_fifo was introduced in 4.4.12 / 4.7
>  variable atmel_port was introduced in 4.9rc1
> 
>  That's why I didn't add the Cc stable in the email body.
> 
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index fd8aa1f4ba78..2c7c45904ba7 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -2132,11 +2132,28 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>  		mode |= ATMEL_US_USMODE_RS485;
>  	} else if (termios->c_cflag & CRTSCTS) {
>  		/* RS232 with hardware handshake (RTS/CTS) */
> -		if (atmel_use_dma_rx(port) && !atmel_use_fifo(port)) {
> -			dev_info(port->dev, "not enabling hardware flow control because DMA is used");
> -			termios->c_cflag &= ~CRTSCTS;
> -		} else {
> +		if (atmel_use_fifo(port) &&
> +		    !mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_CTS)) {
> +			/*
> +			 * with ATMEL_US_USMODE_HWHS set, the controller will
> +			 * be able to drive the RTS pin high/low when the RX
> +			 * FIFO is above RXFTHRES/below RXFTHRES2.
> +			 * It will also disable the transmitter when the CTS
> +			 * pin is high.
> +			 * This mode is not activated if CTS pin is a GPIO
> +			 * because in this case, the transmitter is always
> +			 * disabled.
> +			 * If the RTS pin is a GPIO, the controller won't be
> +			 * able to drive it according to the FIFO thresholds,
> +			 * but it will be handled by the driver.
> +			 */
>  			mode |= ATMEL_US_USMODE_HWHS;
> +		} else {
> +			/*
> +			 * For platforms without FIFO, the flow control is
> +			 * handled by the driver.
> +			 */
> +			mode |= ATMEL_US_USMODE_NORMAL;
>  		}
>  	} else {
>  		/* RS232 without hadware handshake */

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH/RFT v2 11/17] USB: OHCI: make ohci-da8xx a separate driver
From: Axel Haslam @ 2016-10-25 16:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <21019d11-3b01-3e0e-60bc-c4873cf20237@lechnology.com>

On Tue, Oct 25, 2016 at 6:12 PM, David Lechner <david@lechnology.com> wrote:
> On 10/25/2016 02:39 AM, Axel Haslam wrote:
>>
>> On Tue, Oct 25, 2016 at 2:38 AM, David Lechner <david@lechnology.com>
>> wrote:
>>>
>>> On 10/24/2016 11:46 AM, ahaslam at baylibre.com wrote:
>>>>
>>>>
>>>> -#ifndef CONFIG_ARCH_DAVINCI_DA8XX
>>>> -#error "This file is DA8xx bus glue.  Define
>>>> CONFIG_ARCH_DAVINCI_DA8XX."
>>>> -#endif
>>>> +#include "ohci.h"
>>>> +
>>>> +#define DRIVER_DESC "OHCI DA8XX driver"
>>>> +
>>>> +static const char hcd_name[] = "ohci-da8xx";
>>>
>>>
>>>
>>> why static const char instead of #define? This is only used one time in a
>>> pr_info, so it seems kind of pointless anyway.
>>
>>
>> Other drivers are using static const for the same variable.
>> i think static const is preferred over #define because #define doet give a
>> type.
>> If you dont mind ill keep it static const.
>>
>
> If this string was used in this file more than one place, I would agree with
> you, but currently it is only used as the argument of a pr_info(). The
> string "ohci-da8xx" could just be included in the fmt string instead of
> using "%s".

I think the purpose was to use it in the .name of the platform_driver
structure, too. only that not everybody is doing that, i looked at some bad
 examples :(

would you agree to keep it if we use it in .name too?

-Axel

>

^ permalink raw reply

* [PATCH] ARM: DT: stm32: move dma translation to board files
From: Alexandre TORGUE @ 2016-10-25 16:16 UTC (permalink / raw)
  To: linux-arm-kernel

stm32f469-disco and stm32f429-eval boards use SDRAM start address remapping
(to @0) to boost performances. A DMA translation through "dma-ranges"
property was needed for other masters than the M4 CPU.
stm32f429-disco doesn't use remapping so doesn't need this DMA translation.
This patches moves this DMA translation definition from stm32f429 soc file
to board files.

Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com>

diff --git a/arch/arm/boot/dts/stm32429i-eval.dts b/arch/arm/boot/dts/stm32429i-eval.dts
index 13c7cd2..a763c15 100644
--- a/arch/arm/boot/dts/stm32429i-eval.dts
+++ b/arch/arm/boot/dts/stm32429i-eval.dts
@@ -82,6 +82,10 @@
 		};
 	};
 
+	soc {
+		dma-ranges = <0xc0000000 0x0 0x10000000>;
+	};
+
 	usbotg_hs_phy: usbphy {
 		#phy-cells = <0>;
 		compatible = "usb-nop-xceiv";
diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index 0596d60..3a1cfdd 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -59,8 +59,6 @@
 	};
 
 	soc {
-		dma-ranges = <0xc0000000 0x0 0x10000000>;
-
 		timer2: timer at 40000000 {
 			compatible = "st,stm32-timer";
 			reg = <0x40000000 0x400>;
diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
index 9e73656..c2213c0 100644
--- a/arch/arm/boot/dts/stm32f469-disco.dts
+++ b/arch/arm/boot/dts/stm32f469-disco.dts
@@ -64,6 +64,10 @@
 	aliases {
 		serial0 = &usart3;
 	};
+
+	soc {
+		dma-ranges = <0xc0000000 0x0 0x10000000>;
+	};
 };
 
 &clk_hse {
-- 
1.9.1

^ permalink raw reply related

* [PATCH/RFT v2 11/17] USB: OHCI: make ohci-da8xx a separate driver
From: David Lechner @ 2016-10-25 16:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKXjFTPvEXPRyq0HZiF7mcHWFaFoH7MjFqEKnTyuSCUOB=U7Aw@mail.gmail.com>

On 10/25/2016 02:39 AM, Axel Haslam wrote:
> On Tue, Oct 25, 2016 at 2:38 AM, David Lechner <david@lechnology.com> wrote:
>> On 10/24/2016 11:46 AM, ahaslam at baylibre.com wrote:
>>>
>>> -#ifndef CONFIG_ARCH_DAVINCI_DA8XX
>>> -#error "This file is DA8xx bus glue.  Define CONFIG_ARCH_DAVINCI_DA8XX."
>>> -#endif
>>> +#include "ohci.h"
>>> +
>>> +#define DRIVER_DESC "OHCI DA8XX driver"
>>> +
>>> +static const char hcd_name[] = "ohci-da8xx";
>>
>>
>> why static const char instead of #define? This is only used one time in a
>> pr_info, so it seems kind of pointless anyway.
>
> Other drivers are using static const for the same variable.
> i think static const is preferred over #define because #define doet give a type.
> If you dont mind ill keep it static const.
>

If this string was used in this file more than one place, I would agree 
with you, but currently it is only used as the argument of a pr_info(). 
The string "ohci-da8xx" could just be included in the fmt string instead 
of using "%s".

^ permalink raw reply

* [PATCH] tty/serial: at91: fix hardware handshake on Atmel platforms
From: Richard Genoud @ 2016-10-25 16:11 UTC (permalink / raw)
  To: linux-arm-kernel

commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when
hardware handshake is enabled"), despite its title, broke hardware
handshake on *every* Atmel platforms.

The only one partially working is the SAMA5D2.

To understand why, one has to understand the flag ATMEL_US_USMODE_HWHS
first:
Before commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management
when hardware handshake is enabled"), this flag was never set.
Thus, the CTS/RTS where only handled by serial_core (and everything
worked just fine).

This commit introduced the use of the ATMEL_US_USMODE_HWHS flag,
enabling it for all boards when the user space enables flow control.

When the ATMEL_US_USMODE_HWHS is set, the Atmel USART controller
handles a part of the flow control job:
- disable the transmitter when the CTS pin gets high.
- drive the RTS pin high when the DMA buffer transfer is completed or
  PDC RX buffer full or RX FIFO is beyond threshold. (depending on the
  controller version).

NB: This feature is *not* mandatory for the flow control to work.

Now, the specifics of the ATMEL_US_USMODE_HWHS flag:

- For platforms with DMAC and no FIFOs (sam9x25, sam9x35, sama5D3,
sama5D4, sam9g15, sam9g25, sam9g35)* this feature simply doesn't work.
( source: https://lkml.org/lkml/2016/9/7/598 )
Tested it on sam9g35, the RTS pins always stays up, even when RXEN=1
or a new DMA transfer descriptor is set.
=> ATMEL_US_USMODE_HWHS should not be used for those platforms

- For platforms with a PDC (sam926{0,1,3}, sam9g10, sam9g20, sam9g45,
sam9g46)*, there's another kind of problem. Once the flag
ATMEL_US_USMODE_HWHS is set, the RTS pin can't be driven anymore via
RTSEN/RTSDIS in USART Control Register. The RTS pin can only be driven
by enabling/disabling the receiver or setting RCR=RNCR=0 in the PDC
(Receive (Next) Counter Register).
=> Doing this is beyond the scope of this patch and could add other
bugs, so the original (and working) behaviour should be set for those
platforms (meaning ATMEL_US_USMODE_HWHS flag should be unset).

- For platforms with a FIFO (sama5d2)*, the RTS pin is driven according
to the RX FIFO thresholds, and can be also driven by RTSEN/RTSDIS in
USART Control Register. No problem here.
(This was the use case of commit 1cf6e8fc8341 ("tty/serial: at91: fix
RTS line management when hardware handshake is enabled"))
NB: If the CTS pin declared as a GPIO in the DTS, (for instance
cts-gpios = <&pioA PIN_PB31 GPIO_ACTIVE_LOW>), the transmitter will be
disabled.
=> ATMEL_US_USMODE_HWHS flag can be set for this platform ONLY IF the
CTS pin is not a GPIO.

So, the only case when ATMEL_US_USMODE_HWHS can be enabled is when
(atmel_use_fifo(port) &&
 !mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_CTS))

Tested on all Atmel USART controller flavours:
 AT91SAM9G35-CM, AT91SAM9G20-EK and SAMA5D2xplained
      ^^^^           ^^^^               ^^^^
  (DMAC flavour), (PDC flavour) and (FIFO flavour)

Changes since v4:
 - the mctrl_gpio_use_rtscts() is gone since it was atmel_serial
 specific. (so patch 1 is gone)
 - patches 2 and 3 have been merged together since it didn't make
 a lot of sense to correct the GPIO case in one separate patch.
 - ATMEL_US_USMODE_HWHS is now unset for platform with PDC

Changes since v3:
 - remove superfluous #include <linux/err.h> (thanks to Uwe)
 - rebase on next-20160930

Changes since v2:
 - remove IS_ERR_OR_NULL() test in patch 1/3 as Uwe suggested.
 - fix typos in patch 2/3
 - rebase on next-20160927
 - simplify the logic in patch 3/3.

Changes since v1:
 - Correct patch 1 with the error found by kbuild.
 - Add Alexandre's Acked-by on patch 2
 - Rewrite patch 3 logic in the light of the on-going discussion
   with Cyrille and Alexandre.

* the list may not be exhaustive

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/tty/serial/atmel_serial.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

 I think this should go in the stable tree since it fixes the flow
 control broken since v4.0.
 But It won't compile on versions before 4.9rc1 because:
 function atmel_use_fifo was introduced in 4.4.12 / 4.7
 variable atmel_port was introduced in 4.9rc1

 That's why I didn't add the Cc stable in the email body.


diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index fd8aa1f4ba78..2c7c45904ba7 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -2132,11 +2132,28 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 		mode |= ATMEL_US_USMODE_RS485;
 	} else if (termios->c_cflag & CRTSCTS) {
 		/* RS232 with hardware handshake (RTS/CTS) */
-		if (atmel_use_dma_rx(port) && !atmel_use_fifo(port)) {
-			dev_info(port->dev, "not enabling hardware flow control because DMA is used");
-			termios->c_cflag &= ~CRTSCTS;
-		} else {
+		if (atmel_use_fifo(port) &&
+		    !mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_CTS)) {
+			/*
+			 * with ATMEL_US_USMODE_HWHS set, the controller will
+			 * be able to drive the RTS pin high/low when the RX
+			 * FIFO is above RXFTHRES/below RXFTHRES2.
+			 * It will also disable the transmitter when the CTS
+			 * pin is high.
+			 * This mode is not activated if CTS pin is a GPIO
+			 * because in this case, the transmitter is always
+			 * disabled.
+			 * If the RTS pin is a GPIO, the controller won't be
+			 * able to drive it according to the FIFO thresholds,
+			 * but it will be handled by the driver.
+			 */
 			mode |= ATMEL_US_USMODE_HWHS;
+		} else {
+			/*
+			 * For platforms without FIFO, the flow control is
+			 * handled by the driver.
+			 */
+			mode |= ATMEL_US_USMODE_NORMAL;
 		}
 	} else {
 		/* RS232 without hadware handshake */

^ permalink raw reply related

* [PATCH/RFT v2 07/17] ARM: davinci: da8xx: Enable the usb20 "per" clk on phy_clk_enable
From: David Lechner @ 2016-10-25 16:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <833b6b60-4504-dac7-ccbb-bc08c985ad01@ti.com>

On 10/25/2016 05:12 AM, Sekhar Nori wrote:
> On Monday 24 October 2016 10:16 PM, ahaslam at baylibre.com wrote:
>> diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx.c
>> index 9e41a7f..982e105 100644
>> --- a/arch/arm/mach-davinci/usb-da8xx.c
>> +++ b/arch/arm/mach-davinci/usb-da8xx.c
>> @@ -53,11 +53,19 @@ int __init da8xx_register_usb_refclkin(int rate)
>>
>>  static void usb20_phy_clk_enable(struct clk *clk)
>>  {
>> +	struct clk *usb20_clk;
>>  	u32 val;
>>  	u32 timeout = 500000; /* 500 msec */
>>
>>  	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
>>
>> +	usb20_clk = clk_get(NULL, "usb20");
>
> We should not be using a NULL device pointer here. Can you pass the musb
> device pointer available in the same file? Also, da850_clks[] in da850.c
> needs to be fixed to add the matching device name.

This clock can be used for usb 1.1 PHY even when musb is not being used, 
so I don't think we can depend on having a musb device here.

Also, in a previous review, it was decided that the usb clocks should 
*not* be added to da850_clks[] [1]. Instead, they are dynamically 
registered elsewhere.


[1]: http://www.gossamer-threads.com/lists/linux/kernel/2396533

>
>> +	if (IS_ERR(usb20_clk)) {
>> +		pr_err("could not get usb20 clk\n");
>> +		return;
>> +	}
>
> Thanks,
> Sekhar
>

^ permalink raw reply

* [PATCH 3/4] dt-bindings: Update domain-idle-state binding to use correct compatibles
From: Sudeep Holla @ 2016-10-25 15:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477409199-52182-4-git-send-email-lina.iyer@linaro.org>



On 25/10/16 16:26, Lina Iyer wrote:
> Update domain-idle-state binding to use "domain-idle-state" compatible
> from Documentation/devicetree/bindings/arm/idle-states.txt.
>
> Cc: <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Suggested-by: Sudeep Holla <sudeep.holla@arm.com>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> ---
>  Documentation/devicetree/bindings/power/power_domain.txt | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index e165036..6fb53a3 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -30,8 +30,9 @@ Optional properties:
>     available in the next section.
>
>  - domain-idle-states : A phandle of an idle-state that shall be soaked into a
> -                generic domain power state. The idle state definitions are
> -                compatible with arm,idle-state specified in [1].
> +                generic domain power state. The idle state definitions must be
> +                compatible with "domain-idle-state"

I would reword the below a bit different so that it's flexible to be
reused without "arm,idle-state".

> as well as
> +                "arm,idle-state" as defined in [1].

'Idle states that are "arm,idle-state" compatible are generally
"domain-idle-state" compatible as well if it's a PM domain.'

or something like that in line with what's in patch 2/4.

That would give us the scope of reuse of "domain-idle-state" in device
for future. Also it aligns with your patch 4/4.

Otherwise, it looks good.

-- 
Regards,
Sudeep

^ permalink raw reply

* [PATCH] ARM: socfpga_defconfig: Enable HIGHMEM
From: dinguyen at opensource.altera.com @ 2016-10-25 15:59 UTC (permalink / raw)
  To: linux-arm-kernel

From: Dinh Nguyen <dinguyen@opensource.altera.com>

All of the SoCFPGA boards have at least 1GB of RAM, so enabling HIGHMEM
is necessary to avoid the following warning:

[    0.000000] Truncating RAM at 0x00000000-0x40000000 to -0x30000000
[    0.000000] Consider using a HIGHMEM enabled kernel.

Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
---
 arch/arm/configs/socfpga_defconfig |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/socfpga_defconfig b/arch/arm/configs/socfpga_defconfig
index 4660506..f5b9bc5 100644
--- a/arch/arm/configs/socfpga_defconfig
+++ b/arch/arm/configs/socfpga_defconfig
@@ -25,6 +25,7 @@ CONFIG_PCIE_ALTERA_MSI=y
 CONFIG_SMP=y
 CONFIG_NR_CPUS=2
 CONFIG_AEABI=y
+CONFIG_HIGHMEM=y
 CONFIG_ZBOOT_ROM_TEXT=0x0
 CONFIG_ZBOOT_ROM_BSS=0x0
 CONFIG_VFP=y
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH/RFT v2 02/17] ARM: davinci: da8xx: Add CFGCHIP syscon platform declaration.
From: David Lechner @ 2016-10-25 15:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <758f206c-d0bd-6f70-da1e-42c88d6dd1f0@ti.com>

Hi Sekhar,

On 10/25/2016 05:17 AM, Sekhar Nori wrote:
> On Tuesday 25 October 2016 03:07 PM, Axel Haslam wrote:
>> Hi Sekar,
>>
>> On Tue, Oct 25, 2016 at 10:10 AM, Sekhar Nori <nsekhar@ti.com> wrote:
>>> On Monday 24 October 2016 10:16 PM, ahaslam at baylibre.com wrote:
>>>> From: David Lechner <david@lechnology.com>
>>>>
>>>> The CFGCHIP registers are used by a number of devices, so using a syscon
>>>> device to share them. The first consumer of this will by the phy-da8xx-usb
>>>> driver.
>>>>
>>>> Signed-off-by: David Lechner <david@lechnology.com>
>>>> [Axel: minor fix: change id to -1]
>>>
>>> Can you please clarify this change? There could be other syscon devices
>>> on the chip for other common registers. Why use the singular device-id?
>>>
>>
>> in the case of non DT boot, the phy driver is looking for "syscon" :
>>
>> d_phy->regmap = syscon_regmap_lookup_by_pdevname("syscon");
>>
>> if we register the syscon driver with id = 0, the actual name of the syscon
>> device will be "syscon.0" and the phy driver will fail to probe, because
>> the strncmp match in the syscon driver (syscon_match_pdevname)
>> will fail.
>>
>> should i change the phy driver instead?
>
> Yes, please. Forcing only one syscon region for the whole chip will be
> too restrictive, I am pretty sure.
>
> Thanks,
> Sekhar
>

In the previous review, you requested that this be changed to -1 [1].

If we change it back to 0, it will also require reverting a patch to the 
phy driver that has already been merged[2].

[1]: http://www.gossamer-threads.com/lists/linux/kernel/2435807?page=last
[2]: http://www.gossamer-threads.com/lists/linux/kernel/2518804

^ permalink raw reply

* [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz
From: John Syne @ 2016-10-25 15:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161025063847.GD8574@dell>


> On Oct 24, 2016, at 11:38 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> On Mon, 24 Oct 2016, John Syne wrote:
>>> On Oct 24, 2016, at 11:01 PM, John Syne <john3909@gmail.com> wrote:
>>>> On Oct 24, 2016, at 10:52 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>>>> 
>>>> On Tuesday 25 October 2016 02:28 AM, John Syne wrote:
>>>>>>> On Oct 23, 2016, at 11:02 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>>>>>>> 
>>>>>>> Increase ADC reference clock from 3MHz to 24MHz so that the
>>>>>>> sampling rates goes up from 100K samples per second to 800K
>>>>>>> samples per second on AM335x and AM437x SoC.
>>>>>>> 
>>>>>>> Also increase opendelay for touchscreen configuration to
>>>>>>> equalize the increase in ADC reference clock frequency,
>>>>>>> which results in the same amount touch events reported via
>>>>>>> evtest on AM335x GP EVM.
>>>>>>> 
>>>>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>>>>>>> ---
>>>>>>> 
>>>>>>> This patch depends on ADC DMA patch series [1]
>>>>>>> 
>>>>>>> Without DMA support, when ADC ref clock is set at 24MHz, I am
>>>>>>> seeing fifo overflow as CPU is not able to pull the ADC samples.
>>>>>>> This answers that DMA support is must for ADC to consume the
>>>>>>> samples generated at 24MHz with no open, step delay or
>>>>>>> averaging with patch [2].
>>>>>>> 
>>>>>>> Measured the performance with the iio_generic_buffer with the
>>>>>>> patch [3] applied
>>>>>>> 
>>>>>>> [1] - http://www.spinics.net/lists/devicetree/msg145045.html
>>>>>>> [2] - http://pastebin.ubuntu.com/23357935/
>>>>>>> [3] - http://pastebin.ubuntu.com/23357939/
>>>>>>> 
>>>>>>> ---
>>>>>>> include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>> 
>>>>>>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>>>>>>> index b9a53e0..96c4207 100644
>>>>>>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>>>>>>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>>>>>>> @@ -90,7 +90,7 @@
>>>>>>> /* Delay register */
>>>>>>> #define STEPDELAY_OPEN_MASK	(0x3FFFF << 0)
>>>>>>> #define STEPDELAY_OPEN(val)	((val) << 0)
>>>>>>> -#define STEPCONFIG_OPENDLY	STEPDELAY_OPEN(0x098)
>>>>> Wouldn?t this be better to add this to the devicetree?
>>>>> 
>>>>> 	ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
>>>>> 	ti,chan-step-opendelay = <0x500 0x500 0x500 0x500 0x500 0x500 0x500>;
>>>>> 	ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
>>>> 
>>>> For a touch screen, there is not need to change in these parameter
>>>> settings, so my opinion is to keep it as is. Or am I missing something?
>>> I was thinking that if you are using this driver as an ADC, you may want the flexibility to make these changes in the DT. I?m doing this by connecting sensors to the ADC inputs. I?m not using this driver for a touchscreen. 
>> 
>> Here is a DT overlay were this gets using on the BeagleBoneBlack.  
>> 
>> https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-ADC-00A0.dts
>> 
>> Besides, these DT features are already implemented in the driver so it is just a matter of adding these entries to the am33xx.dtsi & am4372.dtsi, which you modified in this patch series.
> 
> This looks like configuration, no?
> 
> DT should be used to describe the hardware.
You may be right, but how is this different to setting the baud rate on a serial channel or sampling rate on a audio channel? Looking through the DT, there are many configuration settings, so I?m not sure what is the correct way to handle this. Surely it is better to handle this in DT vs hard coding these settings?

Regards,
John
> 
> -- 
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org ? Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH] drivers: iommu: constify iommu_gather_ops structures
From: Julia Lawall @ 2016-10-25 15:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477409624-30949-1-git-send-email-bhumirks@gmail.com>

On Tue, 25 Oct 2016, Bhumika Goyal wrote:

> Check for iommu_gather_ops structures that are only stored in the tlb
> field of an io_pgtable_cfg structure. The tlb field is of type
> const struct iommu_gather_ops *, so iommu_gather_ops structures
> having this property can be declared as const.
> Done using Coccinelle:
>
> @r1 disable optional_qualifier @
> identifier i;
> position p;
> @@
> static struct iommu_gather_ops i at p = {...};
>
> @ok1@
> identifier r1.i;
> position p;
> struct io_pgtable_cfg q;
> @@
> q.tlb=&i at p;
>
> @bad@
> position p!={r1.p,ok1.p};
> identifier r1.i;
> @@
> i at p
>
> @depends on !bad disable optional_qualifier@
> identifier r1.i;
> @@
> static
> +const
> struct iommu_gather_ops i={...};
>
> @depends on !bad disable optional_qualifier@
> identifier r1.i;
> @@
> +const
> struct iommu_gather_ops i;
>
> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>

Acked-by: Julia Lawall <julia.lawall@lip6.fr>

> ---
>  drivers/iommu/io-pgtable-arm-v7s.c | 2 +-
>  drivers/iommu/ipmmu-vmsa.c         | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c
> index f50e51c..1465bbc 100644
> --- a/drivers/iommu/io-pgtable-arm-v7s.c
> +++ b/drivers/iommu/io-pgtable-arm-v7s.c
> @@ -741,7 +741,7 @@ static void dummy_tlb_sync(void *cookie)
>  	WARN_ON(cookie != cfg_cookie);
>  }
>
> -static struct iommu_gather_ops dummy_tlb_ops = {
> +static const struct iommu_gather_ops dummy_tlb_ops = {
>  	.tlb_flush_all	= dummy_tlb_flush_all,
>  	.tlb_add_flush	= dummy_tlb_add_flush,
>  	.tlb_sync	= dummy_tlb_sync,
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index ace331d..b8bcf18 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -283,7 +283,7 @@ static void ipmmu_tlb_add_flush(unsigned long iova, size_t size,
>  	/* The hardware doesn't support selective TLB flush. */
>  }
>
> -static struct iommu_gather_ops ipmmu_gather_ops = {
> +static const struct iommu_gather_ops ipmmu_gather_ops = {
>  	.tlb_flush_all = ipmmu_tlb_flush_all,
>  	.tlb_add_flush = ipmmu_tlb_add_flush,
>  	.tlb_sync = ipmmu_tlb_flush_all,
> --
> 1.9.1
>
>

^ permalink raw reply

* [PATCH v3] drivers: psci: PSCI checker module
From: Lorenzo Pieralisi @ 2016-10-25 15:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161020145115.6326-1-kevin.brodsky@arm.com>

[ +Paul, James ]

Left most of the patch content in place so that they can follow it even
if they weren't CC'ed.

On Thu, Oct 20, 2016 at 03:51:15PM +0100, Kevin Brodsky wrote:
> On arm and arm64, PSCI is one of the possible firmware interfaces
> used for power management. This includes both turning CPUs on and off,
> and suspending them (entering idle states).
> 
> This patch adds a PSCI checker module that enables basic testing of
> PSCI operations during startup. There are two main tests: CPU
> hotplugging and suspending.
> 
> In the hotplug tests, the hotplug API is used to turn off and on again
> all CPUs in the system, and then all CPUs in each cluster, checking
> the consistency of the return codes.
> 
> In the suspend tests, a high-priority thread is created on each core
> and uses low-level cpuidle functionalities to enter suspend, in all
> the possible states and multiple times. This should allow a maximum
> number of CPUs to enter the same sleep state at the same or slightly
> different time.
> 
> In essence, the suspend tests use a principle similar to that of the
> intel_powerclamp driver (drivers/thermal/intel_powerclamp.c), but the
> threads are only kept for the duration of the test (they are already
> gone when userspace is started).
> 
> While in theory power management PSCI functions (CPU_{ON,OFF,SUSPEND})
> could be directly called, this proved too difficult as it would imply
> the duplication of all the logic used by the kernel to allow for a
> clean shutdown/bringup/suspend of the CPU (the deepest sleep states
> implying potentially the shutdown of the CPU).
> 
> Note that this file cannot be compiled as a loadable module, since it
> uses a number of non-exported identifiers (essentially for
> PSCI-specific checks and direct use of cpuidle) and relies on the
> absence of userspace to avoid races when calling hotplug and cpuidle
> functions.
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Kevin Hilman <khilman@kernel.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
> ---

[...]

> +static int find_clusters(const struct cpumask *cpus,
> +			 const struct cpumask **clusters)
> +{
> +	unsigned int nb = 0;
> +	cpumask_var_t tmp;
> +
> +	if (!alloc_cpumask_var(&tmp, GFP_KERNEL))
> +		return -ENOMEM;
> +	cpumask_copy(tmp, cpus);
> +
> +	while (!cpumask_empty(tmp)) {
> +		const struct cpumask *cluster =
> +			topology_core_cpumask(cpumask_any(tmp));
> +
> +		clusters[nb++] = cluster;
> +		cpumask_andnot(tmp, tmp, cluster);
> +	}
> +
> +	free_cpumask_var(tmp);
> +	return nb;
> +}
> +
> +/*
> + * offlined_cpus is a temporary array but passing it as an argument avoids
> + * multiple allocations.
> + */
> +static unsigned int down_and_up_cpus(const struct cpumask *cpus,
> +				     struct cpumask *offlined_cpus)
> +{
> +	int cpu;
> +	int err = 0;
> +
> +	cpumask_clear(offlined_cpus);
> +
> +	/* Try to power down all CPUs in the mask. */
> +	for_each_cpu(cpu, cpus) {
> +		int ret = cpu_down(cpu);
> +
> +		/*
> +		 * cpu_down() checks the number of online CPUs before the TOS
> +		 * resident CPU.
> +		 */
> +		if (cpumask_weight(offlined_cpus) + 1 == nb_available_cpus) {
> +			if (ret != -EBUSY) {
> +				pr_err("Unexpected return code %d while trying "
> +				       "to power down last online CPU %d\n",
> +				       ret, cpu);
> +				++err;
> +			}
> +		} else if (cpu == tos_resident_cpu) {
> +			if (ret != -EPERM) {
> +				pr_err("Unexpected return code %d while trying "
> +				       "to power down TOS resident CPU %d\n",
> +				       ret, cpu);
> +				++err;
> +			}
> +		} else if (ret != 0) {
> +			pr_err("Error occurred (%d) while trying "
> +			       "to power down CPU %d\n", ret, cpu);
> +			++err;
> +		}
> +
> +		if (ret == 0)
> +			cpumask_set_cpu(cpu, offlined_cpus);
> +	}
> +
> +	/* Try to power up all the CPUs that have been offlined. */
> +	for_each_cpu(cpu, offlined_cpus) {
> +		int ret = cpu_up(cpu);
> +
> +		if (ret != 0) {
> +			pr_err("Error occurred (%d) while trying "
> +			       "to power up CPU %d\n", ret, cpu);
> +			++err;
> +		} else {
> +			cpumask_clear_cpu(cpu, offlined_cpus);
> +		}
> +	}
> +
> +	/*
> +	 * Something went bad at some point and some CPUs could not be turned
> +	 * back on.
> +	 */
> +	WARN_ON(!cpumask_empty(offlined_cpus) ||
> +		num_online_cpus() != nb_available_cpus);
> +
> +	return err;
> +}
> +
> +static int hotplug_tests(void)
> +{
> +	int err;
> +	cpumask_var_t offlined_cpus;
> +	int i, nb_cluster;
> +	const struct cpumask **clusters;
> +	char *page_buf;
> +
> +	err = -ENOMEM;
> +	if (!alloc_cpumask_var(&offlined_cpus, GFP_KERNEL))
> +		return err;
> +	/* We may have up to nb_available_cpus clusters. */
> +	clusters = kmalloc_array(nb_available_cpus, sizeof(*clusters),
> +				 GFP_KERNEL);
> +	if (!clusters)
> +		goto out_free_cpus;
> +	page_buf = (char *)__get_free_page(GFP_KERNEL);
> +	if (!page_buf)
> +		goto out_free_clusters;
> +
> +	err = 0;
> +	nb_cluster = find_clusters(cpu_online_mask, clusters);
> +
> +	/*
> +	 * Of course the last CPU cannot be powered down and cpu_down() should
> +	 * refuse doing that.
> +	 */
> +	pr_info("Trying to turn off and on again all CPUs\n");
> +	err += down_and_up_cpus(cpu_online_mask, offlined_cpus);
> +
> +	/*
> +	 * Take down CPUs by cluster this time. When the last CPU is turned
> +	 * off, the cluster itself should shut down.
> +	 */
> +	for (i = 0; i < nb_cluster; ++i) {
> +		int cluster_id =
> +			topology_physical_package_id(cpumask_any(clusters[i]));
> +		ssize_t len = cpumap_print_to_pagebuf(true, page_buf,
> +						      clusters[i]);
> +		/* Remove trailing newline. */
> +		page_buf[len - 1] = '\0';
> +		pr_info("Trying to turn off and on again cluster %d "
> +			"(CPUs %s)\n", cluster_id, page_buf);
> +		err += down_and_up_cpus(clusters[i], offlined_cpus);
> +	}
> +
> +	free_page((unsigned long)page_buf);
> +out_free_clusters:
> +	kfree(clusters);
> +out_free_cpus:
> +	free_cpumask_var(offlined_cpus);
> +	return err;
> +}
> +
> +static void dummy_callback(unsigned long ignored) {}
> +
> +static int suspend_cpu(int index, bool broadcast)
> +{
> +	int ret;
> +
> +	arch_cpu_idle_enter();
> +
> +	if (broadcast) {
> +		/*
> +		 * The local timer will be shut down, we need to enter tick
> +		 * broadcast.
> +		 */
> +		ret = tick_broadcast_enter();
> +		if (ret) {
> +			/*
> +			 * In the absence of hardware broadcast mechanism,
> +			 * this CPU might be used to broadcast wakeups, which
> +			 * may be why entering tick broadcast has failed.
> +			 * There is little the kernel can do to work around
> +			 * that, so enter WFI instead (idle state 0).
> +			 */
> +			cpu_do_idle();
> +			ret = 0;
> +			goto out_arch_exit;
> +		}
> +	}
> +
> +	/*
> +	 * Replicate the common ARM cpuidle enter function
> +	 * (arm_enter_idle_state).
> +	 */
> +	ret = CPU_PM_CPU_IDLE_ENTER(arm_cpuidle_suspend, index);
> +
> +	if (broadcast)
> +		tick_broadcast_exit();
> +
> +out_arch_exit:
> +	arch_cpu_idle_exit();
> +
> +	return ret;
> +}
> +
> +static int suspend_test_thread(void *arg)
> +{
> +	int cpu = (long)arg;
> +	int i, nb_suspend = 0, nb_shallow_sleep = 0, nb_err = 0;
> +	struct sched_param sched_priority = { .sched_priority = MAX_RT_PRIO-1 };
> +	struct cpuidle_device *dev;
> +	struct cpuidle_driver *drv;
> +	/* No need for an actual callback, we just want to wake up the CPU. */
> +	struct timer_list wakeup_timer =
> +		TIMER_INITIALIZER(dummy_callback, 0, 0);
> +
> +	/* Wait for the main thread to give the start signal. */
> +	wait_for_completion(&suspend_threads_started);
> +
> +	/* Set maximum priority to preempt all other threads on this CPU. */
> +	if (sched_setscheduler_nocheck(current, SCHED_FIFO, &sched_priority))
> +		pr_warn("Failed to set suspend thread scheduler on CPU %d\n",
> +			cpu);
> +
> +	dev = this_cpu_read(cpuidle_devices);
> +	drv = cpuidle_get_cpu_driver(dev);
> +
> +	pr_info("CPU %d entering suspend cycles, states 1 through %d\n",
> +		cpu, drv->state_count - 1);
> +
> +	for (i = 0; i < NUM_SUSPEND_CYCLE; ++i) {
> +		int index;
> +		/*
> +		 * Test all possible states, except 0 (which is usually WFI and
> +		 * doesn't use PSCI).
> +		 */
> +		for (index = 1; index < drv->state_count; ++index) {
> +			struct cpuidle_state *state = &drv->states[index];
> +			bool broadcast = state->flags & CPUIDLE_FLAG_TIMER_STOP;
> +			int ret;
> +
> +			/*
> +			 * Set the timer to wake this CPU up in some time (which
> +			 * should be largely sufficient for entering suspend).
> +			 * If the local tick is disabled when entering suspend,
> +			 * suspend_cpu() takes care of switching to a broadcast
> +			 * tick, so the timer will still wake us up.
> +			 */
> +			mod_timer(&wakeup_timer, jiffies +
> +				  usecs_to_jiffies(state->target_residency));
> +
> +			/* IRQs must be disabled during suspend operations. */
> +			local_irq_disable();
> +
> +			ret = suspend_cpu(index, broadcast);
> +
> +			/*
> +			 * We have woken up. Re-enable IRQs to handle any
> +			 * pending interrupt, do not wait until the end of the
> +			 * loop.
> +			 */
> +			local_irq_enable();
> +
> +			if (ret == index) {
> +				++nb_suspend;
> +			} else if (ret >= 0) {
> +				/* We did not enter the expected state. */
> +				++nb_shallow_sleep;
> +			} else {
> +				pr_err("Failed to suspend CPU %d: error %d "
> +				       "(requested state %d, cycle %d)\n",
> +				       cpu, ret, index, i);
> +				++nb_err;
> +			}
> +		}
> +	}
> +
> +	/*
> +	 * Disable the timer to make sure that the timer will not trigger
> +	 * later.
> +	 */
> +	del_timer(&wakeup_timer);
> +
> +	if (atomic_dec_return_relaxed(&nb_active_threads) == 0)
> +		complete(&suspend_threads_done);
> +
> +	/* Give up on RT scheduling and wait for termination. */
> +	sched_priority.sched_priority = 0;
> +	if (sched_setscheduler_nocheck(current, SCHED_NORMAL, &sched_priority))
> +		pr_warn("Failed to set suspend thread scheduler on CPU %d\n",
> +			cpu);
> +	for (;;) {
> +		/* Needs to be set first to avoid missing a wakeup. */
> +		set_current_state(TASK_INTERRUPTIBLE);
> +		if (kthread_should_stop()) {
> +			__set_current_state(TASK_RUNNING);
> +			break;
> +		}
> +		schedule();
> +	}
> +
> +	pr_info("CPU %d suspend test results: success %d, shallow states %d, errors %d\n",
> +		cpu, nb_suspend, nb_shallow_sleep, nb_err);
> +
> +	return nb_err;
> +}
> +
> +static int suspend_tests(void)
> +{
> +	int i, cpu, err = 0;
> +	struct task_struct **threads;
> +	int nb_threads = 0;
> +
> +	threads = kmalloc_array(nb_available_cpus, sizeof(*threads),
> +				GFP_KERNEL);
> +	if (!threads)
> +		return -ENOMEM;
> +
> +	for_each_online_cpu(cpu) {
> +		struct task_struct *thread;
> +		/* Check that cpuidle is available on that CPU. */
> +		struct cpuidle_device *dev = per_cpu(cpuidle_devices, cpu);
> +		struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
> +
> +		if (cpuidle_not_available(drv, dev)) {
> +			pr_warn("cpuidle not available on CPU %d, ignoring\n",
> +				cpu);
> +			continue;
> +		}
> +
> +		thread = kthread_create_on_cpu(suspend_test_thread,
> +					       (void *)(long)cpu, cpu,
> +					       "psci_suspend_test");
> +		if (IS_ERR(thread))
> +			pr_err("Failed to create kthread on CPU %d\n", cpu);
> +		else
> +			threads[nb_threads++] = thread;
> +	}
> +	if (nb_threads < 1) {
> +		kfree(threads);
> +		return -ENODEV;
> +	}
> +
> +	atomic_set(&nb_active_threads, nb_threads);
> +
> +	/*
> +	 * Stop cpuidle to prevent the idle tasks from entering a deep sleep
> +	 * mode, as it might interfere with the suspend threads on other CPUs.
> +	 * This does not prevent the suspend threads from using cpuidle (only
> +	 * the idle tasks check this status).
> +	 */
> +	cpuidle_pause();
> +
> +	/*
> +	 * Wake up the suspend threads. To avoid the main thread being preempted
> +	 * before all the threads have been unparked, the suspend threads will
> +	 * wait for the completion of suspend_threads_started.
> +	 */
> +	for (i = 0; i < nb_threads; ++i)
> +		wake_up_process(threads[i]);
> +	complete_all(&suspend_threads_started);
> +
> +	wait_for_completion(&suspend_threads_done);
> +
> +	cpuidle_resume();
> +
> +	/* Stop and destroy all threads, get return status. */
> +	for (i = 0; i < nb_threads; ++i)
> +		err += kthread_stop(threads[i]);
> +
> +	kfree(threads);
> +	return err;
> +}
> +
> +static int __init psci_checker(void)
> +{
> +	int ret;
> +
> +	/*
> +	 * Since we're in an initcall, we assume that all the CPUs that all
> +	 * CPUs that can be onlined have been onlined.
> +	 *
> +	 * The tests assume that hotplug is enabled but nobody else is using it,
> +	 * otherwise the results will be unpredictable. However, since there
> +	 * is no userspace yet in initcalls, that should be fine.

I do not think it is. If you run a kernel with, say,
CONFIG_LOCK_TORTURE_TEST, cpus may disappear from the radar while
running the PSCI checker test itself; that at least would confuse the
checker, and that's just an example.

I added Paul to check what are the assumptions behind the torture test
hotplug tests, in particular if there are any implicit assumptions for
it to work (ie it is the only kernel test hotplugging cpus in and out
(?)), what I know is that the PSCI checker assumptions are not correct.

There is something simple you can do to get this "fixed".

You can use the new API James implemented for hibernate,
that allows you to disable (ie PSCI CPU OFF) all "secondary" cpus
other than the primary one passed in as parameter:

freeze_secondary_cpus(int primary);

that function will _cpu_down() all online cpus other than "primary"
in one go, without any interference allowed from other bits of the
kernel. It requires an enable_nonboot_cpus() counterpart, and you
can do that for every online cpus you detect (actually you can even
avoid using the online cpu mask and use the present mask to carry
out the test). If there is a resident trusted OS you can just
trigger the test with primary == tos_resident_cpu, since all
others are bound to fail (well, you can run them and check they
do fail, it is a checker after all).

You would lose the capability of hotplugging a "cluster" at a time, but
I do not think it is a big problem, the test above would cover it
and more importantly, it is safe to execute.

Or we can augment the torture test API to restrict the cpumask
it actually uses to offline/online cpus (I am referring to
torture_onoff(), kernel/torture.c).

Further comments appreciated since I am not sure I grokked the
assumption the torture tests make about hotplugging cpus in and out,
I will go through the commits logs again to find more info.

Thanks !
Lorenzo

> +	 */
> +	nb_available_cpus = num_online_cpus();
> +
> +	/* Check PSCI operations are set up and working. */
> +	ret = psci_ops_check();
> +	if (ret)
> +		return ret;
> +
> +	pr_info("PSCI checker started using %u CPUs\n", nb_available_cpus);
> +
> +	pr_info("Starting hotplug tests\n");
> +	ret = hotplug_tests();
> +	if (ret == 0)
> +		pr_info("Hotplug tests passed OK\n");
> +	else if (ret > 0)
> +		pr_err("%d error(s) encountered in hotplug tests\n", ret);
> +	else {
> +		pr_err("Out of memory\n");
> +		return ret;
> +	}
> +
> +	pr_info("Starting suspend tests (%d cycles per state)\n",
> +		NUM_SUSPEND_CYCLE);
> +	ret = suspend_tests();
> +	if (ret == 0)
> +		pr_info("Suspend tests passed OK\n");
> +	else if (ret > 0)
> +		pr_err("%d error(s) encountered in suspend tests\n", ret);
> +	else {
> +		switch (ret) {
> +		case -ENOMEM:
> +			pr_err("Out of memory\n");
> +			break;
> +		case -ENODEV:
> +			pr_warn("Could not start suspend tests on any CPU\n");
> +			break;
> +		}
> +	}
> +
> +	pr_info("PSCI checker completed\n");
> +	return ret < 0 ? ret : 0;
> +}
> +late_initcall(psci_checker);
> -- 
> 2.10.0
> 

^ permalink raw reply

* [PATCH 5/5] ARM: dts: Add LEGO MINDSTORTMS EV3 dts
From: David Lechner @ 2016-10-25 15:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <0b7cf7a1-e234-2868-dfde-347c92c23829@ti.com>

On 10/25/2016 05:58 AM, Sekhar Nori wrote:
> On Tuesday 25 October 2016 02:50 AM, David Lechner wrote:
>> On 10/24/2016 02:50 PM, David Lechner wrote:
>>> On 10/24/2016 10:50 AM, David Lechner wrote:
>>>> On 10/24/2016 06:58 AM, Sekhar Nori wrote:
>>>>> On Saturday 22 October 2016 12:06 AM, David Lechner wrote:
>>>>>
>>>>>> +&ehrpwm1 {
>>>>>> +    status = "disabled";
>>>>>
>>>>> Hmm, disabled? Can you add this node when you actually use it?
>>>>
>>>> Not sure why I have this disabled. Like the gpios, the pwms can be used
>>>> via sysfs, so I would like to leave them.
>>>>
>>>
>>> Now I remember why these are disabled. The clock matching is broken.
>>> Only the first ehrpwm and the first ecap get clocks. The others fail.
>>>
>>> I can change these to "okay". It will just result in a kernel error
>>> message until the clocks are fixed.
>>>
>>
>> correction: it is not the clocks that are broken. it is the device names.
>>
>> In  arch/arm/mach-davinci/da8xx-dt.c, we have...
>>
>>
>>     OF_DEV_AUXDATA("ti,da850-ehrpwm", 0x01f00000, "ehrpwm", NULL),
>>     OF_DEV_AUXDATA("ti,da850-ehrpwm", 0x01f02000, "ehrpwm", NULL),
>>     OF_DEV_AUXDATA("ti,da850-ecap", 0x01f06000, "ecap", NULL),
>>     OF_DEV_AUXDATA("ti,da850-ecap", 0x01f07000, "ecap", NULL),
>>     OF_DEV_AUXDATA("ti,da850-ecap", 0x01f08000, "ecap", NULL),
>>
>>
>> Which causes each device to have the same device node name. This causes
>> sysfs errors because it is trying to register a second device at the
>> same sysfs path.
>>
>> If you change the names here, then the device do not work because the
>> clock lookup fails.
>
> Yeah, this is incorrect (I should have caught it in review). The device
> id should have been present in the lookup. Can you fix auxdata and clock
> lookup too and send a separate patch? Its probably a v4.9-rc candidate.
>
> Thanks,
> Sekhar
>

Yes, I can do this.

^ permalink raw reply

* [PATCH 3/5] staging/vchi: Fix some pointer math for 64-bit.
From: Eric Anholt @ 2016-10-25 15:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161025075515.GA29765@kroah.com>

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

> On Mon, Oct 17, 2016 at 12:44:04PM -0700, Eric Anholt wrote:
>> These were throwing warnings on aarch64, and all are trivially
>> converted to longs.
>> 
>> Signed-off-by: Eric Anholt <eric@anholt.net>
>> ---
>>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 6 +++---
>>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c     | 5 +++--
>>  2 files changed, 6 insertions(+), 5 deletions(-)
>
> This didnt apply anymore as I think I took the other fixups, sorry.

No problem, I'm happy to see Michael's stuff getting merged.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161025/36d2d692/attachment-0001.sig>

^ permalink raw reply

* [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz
From: John Syne @ 2016-10-25 15:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <7024bec3-2ebe-db82-12e2-6d6bcc664c6b@ti.com>


> On Oct 24, 2016, at 11:37 PM, Vignesh R <vigneshr@ti.com> wrote:
> 
> 
> 
> On Tuesday 25 October 2016 11:46 AM, John Syne wrote:
>> 
>>> On Oct 24, 2016, at 11:01 PM, John Syne <john3909@gmail.com> wrote:
>>> 
>>>> 
>>>> On Oct 24, 2016, at 10:52 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>>>> 
> [...]
>>>>>>> 
>>>>>>> ---
>>>>>>> include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>> 
>>>>>>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>>>>>>> index b9a53e0..96c4207 100644
>>>>>>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>>>>>>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>>>>>>> @@ -90,7 +90,7 @@
>>>>>>> /* Delay register */
>>>>>>> #define STEPDELAY_OPEN_MASK	(0x3FFFF << 0)
>>>>>>> #define STEPDELAY_OPEN(val)	((val) << 0)
>>>>>>> -#define STEPCONFIG_OPENDLY	STEPDELAY_OPEN(0x098)
>>>>> Wouldn?t this be better to add this to the devicetree?
>>>>> 
>>>>> 	ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
>>>>> 	ti,chan-step-opendelay = <0x500 0x500 0x500 0x500 0x500 0x500 0x500>;
>>>>> 	ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
>>>> 
>>>> For a touch screen, there is not need to change in these parameter
>>>> settings, so my opinion is to keep it as is. Or am I missing something?
>>> I was thinking that if you are using this driver as an ADC, you may want the flexibility to make these changes in the DT. I?m doing this by connecting sensors to the ADC inputs. I?m not using this driver for a touchscreen.
>> 
> 
> ti_am335x_adc driver already supports above DT parameters and its upto
> the user to adjust these parameters as required.
> 
>> Here is a DT overlay were this gets using on the BeagleBoneBlack.  
>> 
>> https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-ADC-00A0.dts
>> 
>> Besides, these DT features are already implemented in the driver so it is just a matter of adding these entries to the am33xx.dtsi & am4372.dtsi, which you modified in this patch series.
>> 
> 
> Touchscreen driver (ti_am335x_tsc.c) does not support above DT parameters.
This patch series also modifies ti_am335x_adc.c

https://github.com/analogdevicesinc/linux/blob/master/drivers/iio/adc/ti_am335x_adc.c#L447

Regards,
John
> 
> -- 
> Regards
> Vignesh

^ permalink raw reply

* [PATCH 3/3] ARM: dts: socfpga: Enable QSPI on the Cyclone5 sockit
From: Graham Moore @ 2016-10-25 15:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <c62baad6-2528-a9df-6b09-3dba66785102@opensource.altera.com>

On 10/20/2016 09:12 AM, Dinh Nguyen wrote:
>
>
> On 10/20/2016 02:19 AM, Steffen Trumtrar wrote:
>
>>> +		cdns,tslch-ns = <4>;
>>> +
>>> +		partition at qspi-boot {
>>> +			/* 8MB for raw data. */
>>> +			label = "Flash 0 Raw Data";
>>> +			reg = <0x0 0x800000>;
>>> +		};
>>> +
>>> +		partition at qspi-rootfs {
>>> +			/* 120MB for jffs2 data. */
>>> +			label = "Flash 0 jffs2 Filesystem";
>>> +			reg = <0x800000 0x7800000>;
>>> +		};
>>> +	};
>>> +};
>>> +
>>
>> What is the current preferred way of handling the partitions?
>> This doesn't fit my Sockit configuration for example. So I would always
>> have to patch the devicetree.
>
> I'm not 100% sure on this. Graham, do you have any insight?
>>

Well, strictly speaking, these partitions are only for the socdk, the 
Altera dev kit.  Our sample designs and file systems expect this layout.

Therefore, these partitions are not required for any other dev kits, and 
can probably be left out.

Or, Steffen, if you have a standard layout you'd like to see, then put 
that in there.

I think some people will have to patch the layout regardless.

-Graham

^ permalink raw reply

* [PATCH] drivers: iommu: constify iommu_gather_ops structures
From: Bhumika Goyal @ 2016-10-25 15:33 UTC (permalink / raw)
  To: linux-arm-kernel

Check for iommu_gather_ops structures that are only stored in the tlb
field of an io_pgtable_cfg structure. The tlb field is of type
const struct iommu_gather_ops *, so iommu_gather_ops structures
having this property can be declared as const.
Done using Coccinelle:

@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct iommu_gather_ops i at p = {...};

@ok1@
identifier r1.i;
position p;
struct io_pgtable_cfg q;
@@
q.tlb=&i at p;

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i at p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
static
+const
struct iommu_gather_ops i={...};

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct iommu_gather_ops i;

Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
 drivers/iommu/io-pgtable-arm-v7s.c | 2 +-
 drivers/iommu/ipmmu-vmsa.c         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c
index f50e51c..1465bbc 100644
--- a/drivers/iommu/io-pgtable-arm-v7s.c
+++ b/drivers/iommu/io-pgtable-arm-v7s.c
@@ -741,7 +741,7 @@ static void dummy_tlb_sync(void *cookie)
 	WARN_ON(cookie != cfg_cookie);
 }
 
-static struct iommu_gather_ops dummy_tlb_ops = {
+static const struct iommu_gather_ops dummy_tlb_ops = {
 	.tlb_flush_all	= dummy_tlb_flush_all,
 	.tlb_add_flush	= dummy_tlb_add_flush,
 	.tlb_sync	= dummy_tlb_sync,
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index ace331d..b8bcf18 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -283,7 +283,7 @@ static void ipmmu_tlb_add_flush(unsigned long iova, size_t size,
 	/* The hardware doesn't support selective TLB flush. */
 }
 
-static struct iommu_gather_ops ipmmu_gather_ops = {
+static const struct iommu_gather_ops ipmmu_gather_ops = {
 	.tlb_flush_all = ipmmu_tlb_flush_all,
 	.tlb_add_flush = ipmmu_tlb_add_flush,
 	.tlb_sync = ipmmu_tlb_flush_all,
-- 
1.9.1

^ permalink raw reply related

* [PATCH 4/9] pinctrl: meson: allow gpio to request irq
From: Jerome Brunet @ 2016-10-25 15:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <bbc268c8-35c3-206a-46b1-62cebef174b2@arm.com>

On Tue, 2016-10-25 at 15:47 +0100, Marc Zyngier wrote:
>?
> > > But why are those number different? Why don't you use the same
> > > namespace? If gpio == hwirq, all your problems are already
> > > solved. If
> > > you don't find the mapping in the irqdomain, then there is no
> > > irq,
> > > end
> > > of story. What am I missing?
> > > 
> > 
> > There is a few problems to guarantee that gpio == hwirq.
> > 1. We have 2 instances of pinctrl, to guarantee that the linux gpio
> > number == hwirq, we would have to guarantee the order in which they
> > are
> > probed. At least this my understanding?
> 
> Maybe I wasn't clear enough, and my use of gpio is probably wrong. So
> Linux has a gpio number, which is obviously an abstract number (just
> like the Linux irq number). But the pad number, in the context of
> given
> SoC, is constant. So we have:
> 
> 	pad->gpio
> 	hwirq->irq
> 
> Why can't you have pad == hwirq, always? This is already what you
> have
> in the irqchip driver. This would simplify a lot of things.

The meson pinctrl driver is designed so that, the pad numbering (or the
equivalent), is linear within a bank. This makes the code simpler for a
lot of things in the meson-pinctrl driver (like finding the appropriate
register and bit). Because of this, and the fact that some gpio might
not have an irq, pad == hwirq cannot hold.

In addition the pad numbering starts from 0 on each gpiochip.

The solution for that is to have the first and last irq number (which
is actually the mux setting of the controller) in the data of each gpio
bank, as described in the Datasheet.

> 
> > 
> > 2. Inside each instance, we may have banks that can't have irq. We
> > even
> > have a bank on meson8b which has a mixed state (the last pins don't
> > have irqs, while the first ones do). those banks/pins are still
> > valid
> > gpios with gpio numbers. This introduce a shift between the gpio
> > numbering and the hwirq.
> > 
> > The point of this calculation is take the offset given to the
> > 'to_irq'
> > callback, remove the gpio bank base number and add irq base number.
> > There is a few trick added to handled the case in 2.
> 
> You seem to assume linear mappings, which is pretty dangerous.

That's the way the meson pinctrl driver works (linear numbering in each
banks) and the HW is described in DS. Why would it be dangerous ?

> 
> > 
> > 
> > In addition, to call 'irq_find_mapping', we would first have to
> > create
> > the mapping, in the probe I suppose. This would call the allocate
> > callback of the domain, in which we can allocate only 8 interrupts.
> > 
> > That's why I create the mapping in the .to_irq callback.
> 
> Is gpio_to_irq() supposed to allocate an interrupt? Or merely to
> report
> the existence of a mapping?

Linus, please correct me if I'm wrong,
.to_irq gets the linux gpio number and returns the linux virtual irq
numbers, 0 if there is no interrupt.

So could either find or create the mapping.
   A. With the hardware at hand, and the limited upstream interrupt
      number, i don't see how we could create the mapping beforehand.?

> 
> > 
> > 
> > > 
> > > > 
> > > > 
> > > > 
> > > > Even if I implement an another irqdomain at the gpio level, I
> > > > would
> > > > still have to perform this kind of calculation, one way or the
> > > > other.
> > > > 
> > > > > 
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > Which is problematic since quite a few GPIO drivers now
> > > > > > need to use them.
> > > > > > 
> > > > > > I will review his slides, in the meantime I would say:
> > > > > > whatever
> > > > > > Marc ACKs is fine with me. I trust this guy 100%. So I
> > > > > > guess I
> > > > > > could ask that he ACK the entire chain of patches
> > > > > > from GIC->specialchip->GPIO.
> > > > 
> > > > Actually this discussion go me thinking about another issue we
> > > > have
> > > > with this hardware.
> > > > We are looking for a way to implement support for
> > > > IRQ_TYPE_EDGE_BOTH
> > > > (needed for things like gpio-keys or mmc card detect).?
> > > > The controller can do each edge but not both at the same time.
> > > > I'm thinking that implementing another irqdomain at the gpio
> > > > level
> > > > would allow to properly check the pad level in the EOI callback
> > > > then
> > > > set the next expected edge type accordingly (using
> > > > 'irq_chip_set_type_parent')
> > > > 
> > > > Would it be acceptable ?
> > > 
> > > I really don't see what another irqdomain brings to the table.
> > > This
> > > is
> > > not a separate piece of HW, so the hwirq:irq mapping is still the
> > > same.
> > > I fail to see what the benefit is.
> > 
> > The separate piece of hw is the gpio itself.?
> > The irq-controller (in irqchip) can't read the gpio state because
> > it is
> > simply another device.
> > 
> > The domain I'm thinking about wouldn't do much, I reckon.?
> > It would just register an irqchip which would only implement eoi.
> > For everything else, it would rely the parent controller.
> > From your explanation, I understood this is the purpose of
> > hierarchy
> > domains, For each hw in the chain to handle only what it can, and
> > rely
> > on its parent for the rest.
> 
> Right. But that doesn't make it more reliable, see below.
> 
> > 
> > 
> > > 
> > > 
> > > > 
> > > > 
> > > > It looks a few other drivers deal with IRQ_TYPE_EDGE_BOTH in a
> > > > similar
> > > > way (gpio/gpio-omap.c, gpio/gpio-dwapb.c)
> > > 
> > > Being already done doesn't make it reliable. If the line goes low
> > > between latching the rising edge and reprogramming the trigger,
> > > you've
> > > lost at least *two* interrupts (the falling edge and the
> > > following
> > > rising edge).
> > 
> > If a 'usual' controller support IRQ_TYPE_EDGE_BOTH and the line
> > goes
> > low between latching rising edge and handling the interrupt,
> > wouldn't
> > you miss the falling edge anyway ? The signal is just going too
> > fast
> > the HW to handle everything.
> 
> That's the role of the HW to ensure that you don't loose any
> interrupt,
> up to the sampling frequency of the controller. Doing it in SW is
> always
> going to be a wonderful case of "it mostly work".
> 
> > 
> > For the second rising edge, I disagree, it would not be lost, since
> > we
> > would read the pad state, get a low level, and reprogram the
> > controller
> > for another rising edge.
> 
> You always have a race between checking your level and switching the
> configuration, which is likely to be slow anyway. In the meantime,
> the
> level has changed and you're dead.
> 
> Anyway, I suggest you focus on getting something simple up and
> running,
> and postpone the funky (read broken) stuff for later, once you have
> something that looks vaguely sane (because we're not quite there
> yet).
> 
> Thanks,
> 
> 	M.

^ permalink raw reply

* [PATCH 4/4] PM / Domains: Use domain-idle-state compatible
From: Lina Iyer @ 2016-10-25 15:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477409199-52182-1-git-send-email-lina.iyer@linaro.org>

Use the more specific "domain-idle-states" compatible property to read
idle state properties from DT.

Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 drivers/base/power/domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index aac656a..07ed835 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2048,7 +2048,7 @@ out:
 EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
 
 static const struct of_device_id idle_state_match[] = {
-	{ .compatible = "arm,idle-state", },
+	{ .compatible = "domain-idle-state", },
 	{ }
 };
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH 3/4] dt-bindings: Update domain-idle-state binding to use correct compatibles
From: Lina Iyer @ 2016-10-25 15:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477409199-52182-1-git-send-email-lina.iyer@linaro.org>

Update domain-idle-state binding to use "domain-idle-state" compatible
from Documentation/devicetree/bindings/arm/idle-states.txt.

Cc: <devicetree@vger.kernel.org>
Cc: Rob Herring <robh@kernel.org>
Suggested-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 Documentation/devicetree/bindings/power/power_domain.txt | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index e165036..6fb53a3 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -30,8 +30,9 @@ Optional properties:
    available in the next section.
 
 - domain-idle-states : A phandle of an idle-state that shall be soaked into a
-                generic domain power state. The idle state definitions are
-                compatible with arm,idle-state specified in [1].
+                generic domain power state. The idle state definitions must be
+                compatible with "domain-idle-state" as well as
+                "arm,idle-state" as defined in [1].
   The domain-idle-state property reflects the idle state of this PM domain and
   not the idle states of the devices or sub-domains in the PM domain. Devices
   and sub-domains have their own idle-states independent of the parent
@@ -85,7 +86,7 @@ Example 3:
 	};
 
 	DOMAIN_RET: state at 0 {
-		compatible = "arm,idle-state";
+		compatible = "domain-idle-state", "arm,idle-state";
 		reg = <0x0>;
 		entry-latency-us = <1000>;
 		exit-latency-us = <2000>;
@@ -93,7 +94,7 @@ Example 3:
 	};
 
 	DOMAIN_PWR_DN: state at 1 {
-		compatible = "arm,idle-state";
+		compatible = "domain-idle-state", "arm,idle-state";
 		reg = <0x1>;
 		entry-latency-us = <5000>;
 		exit-latency-us = <8000>;
-- 
2.7.4

^ permalink raw reply related

* [PATCH 2/4] dt-bindings: add domain-idle-state compatible to arm, idle-state
From: Lina Iyer @ 2016-10-25 15:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477409199-52182-1-git-send-email-lina.iyer@linaro.org>

CPU's idle states are defined by the arm,idle-state compatible flag. PM
domains that can contains devices and other domains also has similar
definition for its idle state. Reuse the definition of arm,idle-state
for PM domains by allowing an addition compatible string
("domain-idle-state") to denote idle states that are specific to PM
Domains.

Cc: <devicetree@vger.kernel.org>
Cc: Rob Herring <robh@kernel.org>
Suggested-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 Documentation/devicetree/bindings/arm/idle-states.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/idle-states.txt b/Documentation/devicetree/bindings/arm/idle-states.txt
index b8e41c1..4b0ff59 100644
--- a/Documentation/devicetree/bindings/arm/idle-states.txt
+++ b/Documentation/devicetree/bindings/arm/idle-states.txt
@@ -271,6 +271,9 @@ follows:
 		Usage: Required
 		Value type: <stringlist>
 		Definition: Must be "arm,idle-state".
+			    Additionally, nodes that are used to describe a
+			    idle-state of PM domain must also define
+			    "domain-idle-state" as compatible string.
 
 	- local-timer-stop
 		Usage: See definition
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/4] PM / doc: Update device documentation for devices in IRQ safe PM domains
From: Lina Iyer @ 2016-10-25 15:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477409199-52182-1-git-send-email-lina.iyer@linaro.org>

Update documentation to reflect the changes made to support IRQ safe PM
domains.

Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Signed-off-by: Rafael J. Wysocki <rjw@rjwysocki.net>
---
 Documentation/power/devices.txt | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/power/devices.txt b/Documentation/power/devices.txt
index 8ba6625..73ddea3 100644
--- a/Documentation/power/devices.txt
+++ b/Documentation/power/devices.txt
@@ -607,7 +607,9 @@ individually.  Instead, a set of devices sharing a power resource can be put
 into a low-power state together at the same time by turning off the shared
 power resource.  Of course, they also need to be put into the full-power state
 together, by turning the shared power resource on.  A set of devices with this
-property is often referred to as a power domain.
+property is often referred to as a power domain. A power domain may also be
+nested inside another power domain. The nested domain is referred to as the
+sub-domain of the parent domain.
 
 Support for power domains is provided through the pm_domain field of struct
 device.  This field is a pointer to an object of type struct dev_pm_domain,
@@ -629,6 +631,16 @@ support for power domains into subsystem-level callbacks, for example by
 modifying the platform bus type.  Other platforms need not implement it or take
 it into account in any way.
 
+Devices may be defined as IRQ-safe which indicates to the PM core that their
+runtime PM callbacks may be invoked with disabled interrupts (see
+Documentation/power/runtime_pm.txt for more information).  If an IRQ-safe
+device belongs to a PM domain, the runtime PM of the domain will be
+disallowed, unless the domain itself is defined as IRQ-safe. However, it
+makes sense to define a PM domain as IRQ-safe only if all the devices in it
+are IRQ-safe. Moreover, if an IRQ-safe domain has a parent domain, the runtime
+PM of the parent is only allowed if the parent itself is IRQ-safe too with the
+additional restriction that all child domains of an IRQ-safe parent must also
+be IRQ-safe.
 
 Device Low Power (suspend) States
 ---------------------------------
-- 
2.7.4

^ permalink raw reply related

* [PATCH 0/4] PM / Domains: DT bindings for domain idle states and doc updates
From: Lina Iyer @ 2016-10-25 15:26 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Rafael,

This patchset adds the following on top of the patches [0-7] of v3[1], that you
have already queued up for linux-next.

- New DT compatible for PM Domain idle states.
- Update documentation for PM domain idle state
(Pls. note that this new compatible is yet to be approved by DT reviewers)
- Update device/domain documentation per your suggestion

Thanks,
Lina

[1]. http://www.spinics.net/lists/arm-kernel/msg536181.html

Lina Iyer (4):
  PM / doc: Update device documentation for devices in IRQ safe PM
    domains
  dt-bindings: add domain-idle-state compatible to arm,idle-state
  dt-bindings: Update domain-idle-state binding to use correct
    compatibles
  PM / Domains: Use domain-idle-state compatible

 Documentation/devicetree/bindings/arm/idle-states.txt    |  3 +++
 Documentation/devicetree/bindings/power/power_domain.txt |  9 +++++----
 Documentation/power/devices.txt                          | 14 +++++++++++++-
 drivers/base/power/domain.c                              |  2 +-
 4 files changed, 22 insertions(+), 6 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH 1/2] iommu/arm-smmu: Don't inadvertently reject multiple SMMUv3s
From: Hanjun Guo @ 2016-10-25 15:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5cf1acbf9c42cc99e5cc0dacb50b7a92c3bd0feb.1476702234.git.robin.murphy@arm.com>

On 2016/10/17 19:06, Robin Murphy wrote:
> We now delay installing our per-bus iommu_ops until we know an SMMU has
> successfully probed, as they don't serve much purpose beforehand, and
> doing so also avoids fights between multiple IOMMU drivers in a single
> kernel. However, the upshot of passing the return value of bus_set_iommu()
> back from our probe function is that if there happens to be more than
> one SMMUv3 device in a system, the second and subsequent probes will
> wind up returning -EBUSY to the driver core and getting torn down again.
>
> There are essentially 3 cases in which bus_set_iommu() returns nonzero:
> 1. The bus already has iommu_ops installed
> 2. One of the add_device callbacks from the initial notifier failed
> 3. Allocating or installing the notifier itself failed
>
> The first two are down to devices other than the SMMU in question, so
> shouldn't abort an otherwise-successful SMMU probe, whilst the third is
> indicative of the kind of catastrophic system failure which isn't going
> to get much further anyway. Consequently, there is little harm in
> ignoring the return value either way.
>
> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>

Tested-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

^ 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