* [RFT v2 05/10] ARM: exynos: Define EINT_WAKEUP_MASK registers for S5Pv210 and Exynos5433
From: Krzysztof Kozlowski @ 2018-07-23 17:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180723175302.22535-1-krzk@kernel.org>
S5Pv210 and Exynos5433/Exynos7 have different address of
EINT_WAKEUP_MASK register. Rename existing S5P_EINT_WAKEUP_MASK to
avoid confusion and add new ones.
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Tomasz Figa <tomasz.figa@gmail.com>
Cc: Sylwester Nawrocki <snawrocki@kernel.org>
Acked-by: Tomasz Figa <tomasz.figa@gmail.com>
---
arch/arm/mach-exynos/suspend.c | 2 +-
include/linux/soc/samsung/exynos-regs-pmu.h | 6 +++++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
index 0ec52f442b97..991938e19e17 100644
--- a/arch/arm/mach-exynos/suspend.c
+++ b/arch/arm/mach-exynos/suspend.c
@@ -272,7 +272,7 @@ static int exynos5420_cpu_suspend(unsigned long arg)
static void exynos_pm_set_wakeup_mask(void)
{
/* Set wake-up mask registers */
- pmu_raw_writel(exynos_get_eint_wake_mask(), S5P_EINT_WAKEUP_MASK);
+ pmu_raw_writel(exynos_get_eint_wake_mask(), EXYNOS_EINT_WAKEUP_MASK);
pmu_raw_writel(exynos_irqwake_intmask & ~(1 << 31), S5P_WAKEUP_MASK);
}
diff --git a/include/linux/soc/samsung/exynos-regs-pmu.h b/include/linux/soc/samsung/exynos-regs-pmu.h
index 66dcb9ec273a..eb0d240df7a7 100644
--- a/include/linux/soc/samsung/exynos-regs-pmu.h
+++ b/include/linux/soc/samsung/exynos-regs-pmu.h
@@ -42,7 +42,7 @@
#define EXYNOS_SWRESET 0x0400
#define S5P_WAKEUP_STAT 0x0600
-#define S5P_EINT_WAKEUP_MASK 0x0604
+#define EXYNOS_EINT_WAKEUP_MASK 0x0604
#define S5P_WAKEUP_MASK 0x0608
#define S5P_WAKEUP_MASK2 0x0614
@@ -180,6 +180,9 @@
#define S5P_CORE_WAKEUP_FROM_LOCAL_CFG (0x3 << 8)
#define S5P_CORE_AUTOWAKEUP_EN (1 << 31)
+/* Only for S5Pv210 */
+#define S5PV210_EINT_WAKEUP_MASK 0xC004
+
/* Only for EXYNOS4210 */
#define S5P_CMU_CLKSTOP_LCD1_LOWPWR 0x1154
#define S5P_CMU_RESET_LCD1_LOWPWR 0x1174
@@ -641,6 +644,7 @@
| EXYNOS5420_KFC_USE_STANDBY_WFI3)
/* For EXYNOS5433 */
+#define EXYNOS5433_EINT_WAKEUP_MASK (0x060C)
#define EXYNOS5433_USBHOST30_PHY_CONTROL (0x0728)
#define EXYNOS5433_PAD_RETENTION_AUD_OPTION (0x3028)
#define EXYNOS5433_PAD_RETENTION_MMC2_OPTION (0x30C8)
--
2.14.1
^ permalink raw reply related
* [RFT v2 05/10] ARM: exynos: Define EINT_WAKEUP_MASK registers for S5Pv210 and Exynos5433
From: Krzysztof Kozlowski @ 2018-07-23 17:52 UTC (permalink / raw)
To: Tomasz Figa, Krzysztof Kozlowski, Sylwester Nawrocki,
Linus Walleij, Rob Herring, Mark Rutland, Kukjin Kim,
Russell King, Kyungmin Park, linux-arm-kernel, linux-samsung-soc,
linux-gpio, devicetree, linux-kernel
Cc: Paweł Chmiel, Sylwester Nawrocki, Chanwoo Choi, Alim Akhtar,
Pankaj Dubey
In-Reply-To: <20180723175302.22535-1-krzk@kernel.org>
S5Pv210 and Exynos5433/Exynos7 have different address of
EINT_WAKEUP_MASK register. Rename existing S5P_EINT_WAKEUP_MASK to
avoid confusion and add new ones.
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Tomasz Figa <tomasz.figa@gmail.com>
Cc: Sylwester Nawrocki <snawrocki@kernel.org>
Acked-by: Tomasz Figa <tomasz.figa@gmail.com>
---
arch/arm/mach-exynos/suspend.c | 2 +-
include/linux/soc/samsung/exynos-regs-pmu.h | 6 +++++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
index 0ec52f442b97..991938e19e17 100644
--- a/arch/arm/mach-exynos/suspend.c
+++ b/arch/arm/mach-exynos/suspend.c
@@ -272,7 +272,7 @@ static int exynos5420_cpu_suspend(unsigned long arg)
static void exynos_pm_set_wakeup_mask(void)
{
/* Set wake-up mask registers */
- pmu_raw_writel(exynos_get_eint_wake_mask(), S5P_EINT_WAKEUP_MASK);
+ pmu_raw_writel(exynos_get_eint_wake_mask(), EXYNOS_EINT_WAKEUP_MASK);
pmu_raw_writel(exynos_irqwake_intmask & ~(1 << 31), S5P_WAKEUP_MASK);
}
diff --git a/include/linux/soc/samsung/exynos-regs-pmu.h b/include/linux/soc/samsung/exynos-regs-pmu.h
index 66dcb9ec273a..eb0d240df7a7 100644
--- a/include/linux/soc/samsung/exynos-regs-pmu.h
+++ b/include/linux/soc/samsung/exynos-regs-pmu.h
@@ -42,7 +42,7 @@
#define EXYNOS_SWRESET 0x0400
#define S5P_WAKEUP_STAT 0x0600
-#define S5P_EINT_WAKEUP_MASK 0x0604
+#define EXYNOS_EINT_WAKEUP_MASK 0x0604
#define S5P_WAKEUP_MASK 0x0608
#define S5P_WAKEUP_MASK2 0x0614
@@ -180,6 +180,9 @@
#define S5P_CORE_WAKEUP_FROM_LOCAL_CFG (0x3 << 8)
#define S5P_CORE_AUTOWAKEUP_EN (1 << 31)
+/* Only for S5Pv210 */
+#define S5PV210_EINT_WAKEUP_MASK 0xC004
+
/* Only for EXYNOS4210 */
#define S5P_CMU_CLKSTOP_LCD1_LOWPWR 0x1154
#define S5P_CMU_RESET_LCD1_LOWPWR 0x1174
@@ -641,6 +644,7 @@
| EXYNOS5420_KFC_USE_STANDBY_WFI3)
/* For EXYNOS5433 */
+#define EXYNOS5433_EINT_WAKEUP_MASK (0x060C)
#define EXYNOS5433_USBHOST30_PHY_CONTROL (0x0728)
#define EXYNOS5433_PAD_RETENTION_AUD_OPTION (0x3028)
#define EXYNOS5433_PAD_RETENTION_MMC2_OPTION (0x30C8)
--
2.14.1
^ permalink raw reply related
* Re: Regression with crc32c selection?
From: David Sterba @ 2018-07-23 16:50 UTC (permalink / raw)
To: Holger Hoffstätte; +Cc: linux-btrfs
In-Reply-To: <ca274e95-538c-0c0e-e184-3542b4524afe@applied-asynchrony.com>
On Mon, Jul 23, 2018 at 04:13:26PM +0200, Holger Hoffstätte wrote:
> While backporting a bunch of fixes to my own 4.16.x tree
> (4.17 had a few too many bugs for my taste) I also ended up merging:
Curious, bugs in btrfs or the whole 4.17 kernel? And if bugs, real
breakage or backported fixes?
^ permalink raw reply
* [RFT v2 04/10] pinctrl: samsung: Add dedicated compatible for S5Pv210 wakeup interrupts
From: Krzysztof Kozlowski @ 2018-07-23 17:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180723175302.22535-1-krzk@kernel.org>
The S5Pv210 external wakeup interrupts differ from Exynos therefore
separate compatible is needed. Duplicate existing flavor specific data
from exynos4210_wkup_irq_chip and add new compatible for S5Pv210.
At this point this new compatible does not bring anything new and works
exactly as existing "samsung,exynos4210-wakeup-eint".
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Tomasz Figa <tomasz.figa@gmail.com>
Cc: Sylwester Nawrocki <snawrocki@kernel.org>
Acked-by: Tomasz Figa <tomasz.figa@gmail.com>
---
.../devicetree/bindings/pinctrl/samsung-pinctrl.txt | 2 ++
drivers/pinctrl/samsung/pinctrl-exynos.c | 18 ++++++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
index f7700c9e0d0b..843a6cbf4774 100644
--- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
@@ -161,6 +161,8 @@ B. External Wakeup Interrupts: For supporting external wakeup interrupts, a
found on Samsung S3C2412 and S3C2413 SoCs,
- samsung,s3c64xx-wakeup-eint: represents wakeup interrupt controller
found on Samsung S3C64xx SoCs,
+ - samsung,s5pv210-wakeup-eint: represents wakeup interrupt controller
+ found on Samsung S5Pv210 SoCs,
- samsung,exynos4210-wakeup-eint: represents wakeup interrupt controller
found on Samsung Exynos4210 and S5PC110/S5PV210 SoCs.
- samsung,exynos7-wakeup-eint: represents wakeup interrupt controller
diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c
index a263ddd94945..29d86d704b0c 100644
--- a/drivers/pinctrl/samsung/pinctrl-exynos.c
+++ b/drivers/pinctrl/samsung/pinctrl-exynos.c
@@ -346,6 +346,22 @@ static int exynos_wkup_irq_set_wake(struct irq_data *irqd, unsigned int on)
/*
* irq_chip for wakeup interrupts
*/
+static const struct exynos_irq_chip s5pv210_wkup_irq_chip __initconst = {
+ .chip = {
+ .name = "s5pv210_wkup_irq_chip",
+ .irq_unmask = exynos_irq_unmask,
+ .irq_mask = exynos_irq_mask,
+ .irq_ack = exynos_irq_ack,
+ .irq_set_type = exynos_irq_set_type,
+ .irq_set_wake = exynos_wkup_irq_set_wake,
+ .irq_request_resources = exynos_irq_request_resources,
+ .irq_release_resources = exynos_irq_release_resources,
+ },
+ .eint_con = EXYNOS_WKUP_ECON_OFFSET,
+ .eint_mask = EXYNOS_WKUP_EMASK_OFFSET,
+ .eint_pend = EXYNOS_WKUP_EPEND_OFFSET,
+};
+
static const struct exynos_irq_chip exynos4210_wkup_irq_chip __initconst = {
.chip = {
.name = "exynos4210_wkup_irq_chip",
@@ -380,6 +396,8 @@ static const struct exynos_irq_chip exynos7_wkup_irq_chip __initconst = {
/* list of external wakeup controllers supported */
static const struct of_device_id exynos_wkup_irq_ids[] = {
+ { .compatible = "samsung,s5pv210-wakeup-eint",
+ .data = &s5pv210_wkup_irq_chip },
{ .compatible = "samsung,exynos4210-wakeup-eint",
.data = &exynos4210_wkup_irq_chip },
{ .compatible = "samsung,exynos7-wakeup-eint",
--
2.14.1
^ permalink raw reply related
* [RFT v2 04/10] pinctrl: samsung: Add dedicated compatible for S5Pv210 wakeup interrupts
From: Krzysztof Kozlowski @ 2018-07-23 17:52 UTC (permalink / raw)
To: Tomasz Figa, Krzysztof Kozlowski, Sylwester Nawrocki,
Linus Walleij, Rob Herring, Mark Rutland, Kukjin Kim,
Russell King, Kyungmin Park, linux-arm-kernel, linux-samsung-soc,
linux-gpio, devicetree, linux-kernel
Cc: Paweł Chmiel, Sylwester Nawrocki, Chanwoo Choi, Alim Akhtar,
Pankaj Dubey
In-Reply-To: <20180723175302.22535-1-krzk@kernel.org>
The S5Pv210 external wakeup interrupts differ from Exynos therefore
separate compatible is needed. Duplicate existing flavor specific data
from exynos4210_wkup_irq_chip and add new compatible for S5Pv210.
At this point this new compatible does not bring anything new and works
exactly as existing "samsung,exynos4210-wakeup-eint".
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Tomasz Figa <tomasz.figa@gmail.com>
Cc: Sylwester Nawrocki <snawrocki@kernel.org>
Acked-by: Tomasz Figa <tomasz.figa@gmail.com>
---
.../devicetree/bindings/pinctrl/samsung-pinctrl.txt | 2 ++
drivers/pinctrl/samsung/pinctrl-exynos.c | 18 ++++++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
index f7700c9e0d0b..843a6cbf4774 100644
--- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
@@ -161,6 +161,8 @@ B. External Wakeup Interrupts: For supporting external wakeup interrupts, a
found on Samsung S3C2412 and S3C2413 SoCs,
- samsung,s3c64xx-wakeup-eint: represents wakeup interrupt controller
found on Samsung S3C64xx SoCs,
+ - samsung,s5pv210-wakeup-eint: represents wakeup interrupt controller
+ found on Samsung S5Pv210 SoCs,
- samsung,exynos4210-wakeup-eint: represents wakeup interrupt controller
found on Samsung Exynos4210 and S5PC110/S5PV210 SoCs.
- samsung,exynos7-wakeup-eint: represents wakeup interrupt controller
diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c
index a263ddd94945..29d86d704b0c 100644
--- a/drivers/pinctrl/samsung/pinctrl-exynos.c
+++ b/drivers/pinctrl/samsung/pinctrl-exynos.c
@@ -346,6 +346,22 @@ static int exynos_wkup_irq_set_wake(struct irq_data *irqd, unsigned int on)
/*
* irq_chip for wakeup interrupts
*/
+static const struct exynos_irq_chip s5pv210_wkup_irq_chip __initconst = {
+ .chip = {
+ .name = "s5pv210_wkup_irq_chip",
+ .irq_unmask = exynos_irq_unmask,
+ .irq_mask = exynos_irq_mask,
+ .irq_ack = exynos_irq_ack,
+ .irq_set_type = exynos_irq_set_type,
+ .irq_set_wake = exynos_wkup_irq_set_wake,
+ .irq_request_resources = exynos_irq_request_resources,
+ .irq_release_resources = exynos_irq_release_resources,
+ },
+ .eint_con = EXYNOS_WKUP_ECON_OFFSET,
+ .eint_mask = EXYNOS_WKUP_EMASK_OFFSET,
+ .eint_pend = EXYNOS_WKUP_EPEND_OFFSET,
+};
+
static const struct exynos_irq_chip exynos4210_wkup_irq_chip __initconst = {
.chip = {
.name = "exynos4210_wkup_irq_chip",
@@ -380,6 +396,8 @@ static const struct exynos_irq_chip exynos7_wkup_irq_chip __initconst = {
/* list of external wakeup controllers supported */
static const struct of_device_id exynos_wkup_irq_ids[] = {
+ { .compatible = "samsung,s5pv210-wakeup-eint",
+ .data = &s5pv210_wkup_irq_chip },
{ .compatible = "samsung,exynos4210-wakeup-eint",
.data = &exynos4210_wkup_irq_chip },
{ .compatible = "samsung,exynos7-wakeup-eint",
--
2.14.1
^ permalink raw reply related
* [RFT v2 03/10] pinctrl: samsung: Document hidden requirement about one external wakeup
From: Krzysztof Kozlowski @ 2018-07-23 17:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180723175302.22535-1-krzk@kernel.org>
Hardware (S5Pv210 and all Exynos SoCs) provides only 32 external
interrupts which can wakeup device from deep sleep modes. On S5Pv210
these are gph0-gph3. On all Exynos designs these are gpx0-gpx3.
There is only one 32-bit register for controlling the external wakeup
interrupt mask (masking and unmasking waking capability of these
interrupts).
This lead to implementation in pinctrl driver and machine code which was
using static memory for storing the mask value and not caring about
multiple devices of pin controller... because only one pin controller
device will be handling this.
Since each pin controller node in Device Tree maps onto one device, this
corresponds to hidden assumption in parsing the Device Tree: external
wakeup interrupts can be defined only once. Make this assumption an
explicit requirement.
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Tomasz Figa <tomasz.figa@gmail.com>
Cc: Sylwester Nawrocki <snawrocki@kernel.org>
Acked-by: Tomasz Figa <tomasz.figa@gmail.com>
---
Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
index 5e00a21de2bf..f7700c9e0d0b 100644
--- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
@@ -145,8 +145,13 @@ A. External GPIO Interrupts: For supporting external gpio interrupts, the
B. External Wakeup Interrupts: For supporting external wakeup interrupts, a
child node representing the external wakeup interrupt controller should be
- included in the pin-controller device node. This child node should include
- the following properties.
+ included in the pin-controller device node.
+
+ Only one pin-controller device node can include external wakeup interrupts
+ child node (in other words, only one External Wakeup Interrupts
+ pin-controller is supported).
+
+ This child node should include following properties:
- compatible: identifies the type of the external wakeup interrupt controller
The possible values are:
--
2.14.1
^ permalink raw reply related
* [RFT v2 03/10] pinctrl: samsung: Document hidden requirement about one external wakeup
From: Krzysztof Kozlowski @ 2018-07-23 17:52 UTC (permalink / raw)
To: Tomasz Figa, Krzysztof Kozlowski, Sylwester Nawrocki,
Linus Walleij, Rob Herring, Mark Rutland, Kukjin Kim,
Russell King, Kyungmin Park, linux-arm-kernel, linux-samsung-soc,
linux-gpio, devicetree, linux-kernel
Cc: Paweł Chmiel, Sylwester Nawrocki, Chanwoo Choi, Alim Akhtar,
Pankaj Dubey
In-Reply-To: <20180723175302.22535-1-krzk@kernel.org>
Hardware (S5Pv210 and all Exynos SoCs) provides only 32 external
interrupts which can wakeup device from deep sleep modes. On S5Pv210
these are gph0-gph3. On all Exynos designs these are gpx0-gpx3.
There is only one 32-bit register for controlling the external wakeup
interrupt mask (masking and unmasking waking capability of these
interrupts).
This lead to implementation in pinctrl driver and machine code which was
using static memory for storing the mask value and not caring about
multiple devices of pin controller... because only one pin controller
device will be handling this.
Since each pin controller node in Device Tree maps onto one device, this
corresponds to hidden assumption in parsing the Device Tree: external
wakeup interrupts can be defined only once. Make this assumption an
explicit requirement.
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Tomasz Figa <tomasz.figa@gmail.com>
Cc: Sylwester Nawrocki <snawrocki@kernel.org>
Acked-by: Tomasz Figa <tomasz.figa@gmail.com>
---
Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
index 5e00a21de2bf..f7700c9e0d0b 100644
--- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
@@ -145,8 +145,13 @@ A. External GPIO Interrupts: For supporting external gpio interrupts, the
B. External Wakeup Interrupts: For supporting external wakeup interrupts, a
child node representing the external wakeup interrupt controller should be
- included in the pin-controller device node. This child node should include
- the following properties.
+ included in the pin-controller device node.
+
+ Only one pin-controller device node can include external wakeup interrupts
+ child node (in other words, only one External Wakeup Interrupts
+ pin-controller is supported).
+
+ This child node should include following properties:
- compatible: identifies the type of the external wakeup interrupt controller
The possible values are:
--
2.14.1
^ permalink raw reply related
* [RFT v2 02/10] pinctrl: samsung: Document suspend and resume members
From: Krzysztof Kozlowski @ 2018-07-23 17:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180723175302.22535-1-krzk@kernel.org>
Add missing documentation for suspend and resume members of struct
samsung_pin_ctrl and samsung_pinctrl_drv_data.
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Tomasz Figa <tomasz.figa@gmail.com>
Cc: Sylwester Nawrocki <snawrocki@kernel.org>
Acked-by: Tomasz Figa <tomasz.figa@gmail.com>
---
drivers/pinctrl/samsung/pinctrl-samsung.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h
index f0cda9424dfe..aac16cc8362a 100644
--- a/drivers/pinctrl/samsung/pinctrl-samsung.h
+++ b/drivers/pinctrl/samsung/pinctrl-samsung.h
@@ -223,6 +223,10 @@ struct samsung_retention_data {
* interrupts for the controller.
* @eint_wkup_init: platform specific callback to setup the external wakeup
* interrupts for the controller.
+ * @suspend: platform specific suspend callback, executed during pin controller
+ * device suspend, see samsung_pinctrl_suspend()
+ * @resume: platform specific resume callback, executed during pin controller
+ * device suspend, see samsung_pinctrl_resume()
*/
struct samsung_pin_ctrl {
const struct samsung_pin_bank_data *pin_banks;
@@ -255,6 +259,10 @@ struct samsung_pin_ctrl {
* @pin_base: starting system wide pin number.
* @nr_pins: number of pins supported by the controller.
* @retention_ctrl: retention control runtime data.
+ * @suspend: platform specific suspend callback, executed during pin controller
+ * device suspend, see samsung_pinctrl_suspend()
+ * @resume: platform specific resume callback, executed during pin controller
+ * device suspend, see samsung_pinctrl_resume()
*/
struct samsung_pinctrl_drv_data {
struct list_head node;
--
2.14.1
^ permalink raw reply related
* [RFT v2 02/10] pinctrl: samsung: Document suspend and resume members
From: Krzysztof Kozlowski @ 2018-07-23 17:52 UTC (permalink / raw)
To: Tomasz Figa, Krzysztof Kozlowski, Sylwester Nawrocki,
Linus Walleij, Rob Herring, Mark Rutland, Kukjin Kim,
Russell King, Kyungmin Park, linux-arm-kernel, linux-samsung-soc,
linux-gpio, devicetree, linux-kernel
Cc: Paweł Chmiel, Sylwester Nawrocki, Chanwoo Choi, Alim Akhtar,
Pankaj Dubey
In-Reply-To: <20180723175302.22535-1-krzk@kernel.org>
Add missing documentation for suspend and resume members of struct
samsung_pin_ctrl and samsung_pinctrl_drv_data.
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Tomasz Figa <tomasz.figa@gmail.com>
Cc: Sylwester Nawrocki <snawrocki@kernel.org>
Acked-by: Tomasz Figa <tomasz.figa@gmail.com>
---
drivers/pinctrl/samsung/pinctrl-samsung.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h
index f0cda9424dfe..aac16cc8362a 100644
--- a/drivers/pinctrl/samsung/pinctrl-samsung.h
+++ b/drivers/pinctrl/samsung/pinctrl-samsung.h
@@ -223,6 +223,10 @@ struct samsung_retention_data {
* interrupts for the controller.
* @eint_wkup_init: platform specific callback to setup the external wakeup
* interrupts for the controller.
+ * @suspend: platform specific suspend callback, executed during pin controller
+ * device suspend, see samsung_pinctrl_suspend()
+ * @resume: platform specific resume callback, executed during pin controller
+ * device suspend, see samsung_pinctrl_resume()
*/
struct samsung_pin_ctrl {
const struct samsung_pin_bank_data *pin_banks;
@@ -255,6 +259,10 @@ struct samsung_pin_ctrl {
* @pin_base: starting system wide pin number.
* @nr_pins: number of pins supported by the controller.
* @retention_ctrl: retention control runtime data.
+ * @suspend: platform specific suspend callback, executed during pin controller
+ * device suspend, see samsung_pinctrl_suspend()
+ * @resume: platform specific resume callback, executed during pin controller
+ * device suspend, see samsung_pinctrl_resume()
*/
struct samsung_pinctrl_drv_data {
struct list_head node;
--
2.14.1
^ permalink raw reply related
* [RFT v2 01/10] pinctrl: samsung: Define suspend and resume callbacks for all banks and SoCs
From: Krzysztof Kozlowski @ 2018-07-23 17:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180723175302.22535-1-krzk@kernel.org>
Suspend and resume callbacks in Exynos/S5Pv210 pin controller drivers,
save and restore state of registers. This operations should be done for
all banks which have external interrupts (as denoted by using
EXYNOS_PIN_BANK_EINTG/EINTW macros).
Add all banks of Exynos5260 and Exynos5420. This is necessary step for
supporting suspend to RAM on these SoCs.
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Tomasz Figa <tomasz.figa@gmail.com>
Cc: Sylwester Nawrocki <snawrocki@kernel.org>
Acked-by: Tomasz Figa <tomasz.figa@gmail.com>
---
drivers/pinctrl/samsung/pinctrl-exynos-arm.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm.c
index d82820fc349a..44c6b753f692 100644
--- a/drivers/pinctrl/samsung/pinctrl-exynos-arm.c
+++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm.c
@@ -616,16 +616,22 @@ static const struct samsung_pin_ctrl exynos5260_pin_ctrl[] __initconst = {
.nr_banks = ARRAY_SIZE(exynos5260_pin_banks0),
.eint_gpio_init = exynos_eint_gpio_init,
.eint_wkup_init = exynos_eint_wkup_init,
+ .suspend = exynos_pinctrl_suspend,
+ .resume = exynos_pinctrl_resume,
}, {
/* pin-controller instance 1 data */
.pin_banks = exynos5260_pin_banks1,
.nr_banks = ARRAY_SIZE(exynos5260_pin_banks1),
.eint_gpio_init = exynos_eint_gpio_init,
+ .suspend = exynos_pinctrl_suspend,
+ .resume = exynos_pinctrl_resume,
}, {
/* pin-controller instance 2 data */
.pin_banks = exynos5260_pin_banks2,
.nr_banks = ARRAY_SIZE(exynos5260_pin_banks2),
.eint_gpio_init = exynos_eint_gpio_init,
+ .suspend = exynos_pinctrl_suspend,
+ .resume = exynos_pinctrl_resume,
},
};
@@ -842,30 +848,40 @@ static const struct samsung_pin_ctrl exynos5420_pin_ctrl[] __initconst = {
.nr_banks = ARRAY_SIZE(exynos5420_pin_banks0),
.eint_gpio_init = exynos_eint_gpio_init,
.eint_wkup_init = exynos_eint_wkup_init,
+ .suspend = exynos_pinctrl_suspend,
+ .resume = exynos_pinctrl_resume,
.retention_data = &exynos5420_retention_data,
}, {
/* pin-controller instance 1 data */
.pin_banks = exynos5420_pin_banks1,
.nr_banks = ARRAY_SIZE(exynos5420_pin_banks1),
.eint_gpio_init = exynos_eint_gpio_init,
+ .suspend = exynos_pinctrl_suspend,
+ .resume = exynos_pinctrl_resume,
.retention_data = &exynos5420_retention_data,
}, {
/* pin-controller instance 2 data */
.pin_banks = exynos5420_pin_banks2,
.nr_banks = ARRAY_SIZE(exynos5420_pin_banks2),
.eint_gpio_init = exynos_eint_gpio_init,
+ .suspend = exynos_pinctrl_suspend,
+ .resume = exynos_pinctrl_resume,
.retention_data = &exynos5420_retention_data,
}, {
/* pin-controller instance 3 data */
.pin_banks = exynos5420_pin_banks3,
.nr_banks = ARRAY_SIZE(exynos5420_pin_banks3),
.eint_gpio_init = exynos_eint_gpio_init,
+ .suspend = exynos_pinctrl_suspend,
+ .resume = exynos_pinctrl_resume,
.retention_data = &exynos5420_retention_data,
}, {
/* pin-controller instance 4 data */
.pin_banks = exynos5420_pin_banks4,
.nr_banks = ARRAY_SIZE(exynos5420_pin_banks4),
.eint_gpio_init = exynos_eint_gpio_init,
+ .suspend = exynos_pinctrl_suspend,
+ .resume = exynos_pinctrl_resume,
.retention_data = &exynos4_audio_retention_data,
},
};
--
2.14.1
^ permalink raw reply related
* [RFT v2 01/10] pinctrl: samsung: Define suspend and resume callbacks for all banks and SoCs
From: Krzysztof Kozlowski @ 2018-07-23 17:52 UTC (permalink / raw)
To: Tomasz Figa, Krzysztof Kozlowski, Sylwester Nawrocki,
Linus Walleij, Rob Herring, Mark Rutland, Kukjin Kim,
Russell King, Kyungmin Park, linux-arm-kernel, linux-samsung-soc,
linux-gpio, devicetree, linux-kernel
Cc: Paweł Chmiel, Sylwester Nawrocki, Chanwoo Choi, Alim Akhtar,
Pankaj Dubey
In-Reply-To: <20180723175302.22535-1-krzk@kernel.org>
Suspend and resume callbacks in Exynos/S5Pv210 pin controller drivers,
save and restore state of registers. This operations should be done for
all banks which have external interrupts (as denoted by using
EXYNOS_PIN_BANK_EINTG/EINTW macros).
Add all banks of Exynos5260 and Exynos5420. This is necessary step for
supporting suspend to RAM on these SoCs.
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Tomasz Figa <tomasz.figa@gmail.com>
Cc: Sylwester Nawrocki <snawrocki@kernel.org>
Acked-by: Tomasz Figa <tomasz.figa@gmail.com>
---
drivers/pinctrl/samsung/pinctrl-exynos-arm.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm.c
index d82820fc349a..44c6b753f692 100644
--- a/drivers/pinctrl/samsung/pinctrl-exynos-arm.c
+++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm.c
@@ -616,16 +616,22 @@ static const struct samsung_pin_ctrl exynos5260_pin_ctrl[] __initconst = {
.nr_banks = ARRAY_SIZE(exynos5260_pin_banks0),
.eint_gpio_init = exynos_eint_gpio_init,
.eint_wkup_init = exynos_eint_wkup_init,
+ .suspend = exynos_pinctrl_suspend,
+ .resume = exynos_pinctrl_resume,
}, {
/* pin-controller instance 1 data */
.pin_banks = exynos5260_pin_banks1,
.nr_banks = ARRAY_SIZE(exynos5260_pin_banks1),
.eint_gpio_init = exynos_eint_gpio_init,
+ .suspend = exynos_pinctrl_suspend,
+ .resume = exynos_pinctrl_resume,
}, {
/* pin-controller instance 2 data */
.pin_banks = exynos5260_pin_banks2,
.nr_banks = ARRAY_SIZE(exynos5260_pin_banks2),
.eint_gpio_init = exynos_eint_gpio_init,
+ .suspend = exynos_pinctrl_suspend,
+ .resume = exynos_pinctrl_resume,
},
};
@@ -842,30 +848,40 @@ static const struct samsung_pin_ctrl exynos5420_pin_ctrl[] __initconst = {
.nr_banks = ARRAY_SIZE(exynos5420_pin_banks0),
.eint_gpio_init = exynos_eint_gpio_init,
.eint_wkup_init = exynos_eint_wkup_init,
+ .suspend = exynos_pinctrl_suspend,
+ .resume = exynos_pinctrl_resume,
.retention_data = &exynos5420_retention_data,
}, {
/* pin-controller instance 1 data */
.pin_banks = exynos5420_pin_banks1,
.nr_banks = ARRAY_SIZE(exynos5420_pin_banks1),
.eint_gpio_init = exynos_eint_gpio_init,
+ .suspend = exynos_pinctrl_suspend,
+ .resume = exynos_pinctrl_resume,
.retention_data = &exynos5420_retention_data,
}, {
/* pin-controller instance 2 data */
.pin_banks = exynos5420_pin_banks2,
.nr_banks = ARRAY_SIZE(exynos5420_pin_banks2),
.eint_gpio_init = exynos_eint_gpio_init,
+ .suspend = exynos_pinctrl_suspend,
+ .resume = exynos_pinctrl_resume,
.retention_data = &exynos5420_retention_data,
}, {
/* pin-controller instance 3 data */
.pin_banks = exynos5420_pin_banks3,
.nr_banks = ARRAY_SIZE(exynos5420_pin_banks3),
.eint_gpio_init = exynos_eint_gpio_init,
+ .suspend = exynos_pinctrl_suspend,
+ .resume = exynos_pinctrl_resume,
.retention_data = &exynos5420_retention_data,
}, {
/* pin-controller instance 4 data */
.pin_banks = exynos5420_pin_banks4,
.nr_banks = ARRAY_SIZE(exynos5420_pin_banks4),
.eint_gpio_init = exynos_eint_gpio_init,
+ .suspend = exynos_pinctrl_suspend,
+ .resume = exynos_pinctrl_resume,
.retention_data = &exynos4_audio_retention_data,
},
};
--
2.14.1
^ permalink raw reply related
* [RFT v2 00/10] pinctrl: samsung: Remove ugly hack for sharing eint_wakeup_mask
From: Krzysztof Kozlowski @ 2018-07-23 17:52 UTC (permalink / raw)
To: linux-arm-kernel
Hi All,
Changes since v1
================
1. Add Tomasz's ack.
2. Reword description in patch 6/10.
Tests
=====
This is both request for comments and requests for tests. Only basic
tests were done, including suspend to RAM on Odroid U3 (Exynos4412)
with max7768 RTC wakeup. Please kindly test it with devices capable of
suspending and resuming. I am mostly thinking about S5Pv210-based (Aria),
Trats, Trats2 and TM2 (Exynos5433). Existing platforms should not be
broken however changing external interrupt wakeup mask was not done
on Exynos5433.
Description
===========
The Exynos/S5Pv210 machine suspend code needs to write the external
interrupt mask during suspend. The mask is controlled by pin controller
driver: the exynos_wkup_irq_set_wake() in IRQ chip for these wakeup
interrupts.
Therefore pinctrl driver code exposed an exynos_get_eint_wake_mask()
function which was later used as an extern in machine code.
This is quite ugly way of combining driver and machine code,
not portable triggering Sparse and GCC warnings.
This might break suspend capability S5Pv210 on with older DTBs (thus
breaks DTB compatibility), however:
1. just "might" because in case of using older DTB, the wakeup mask
will not be changed during suspend and default reset value (all
interrupts non-masked) should work,
2. mainline support for S5Pv210 with DTB is limited and suspend
to RAM already might be broken.
Dependencies
============
1. The first seven patches should be taken through one tree,
preferably samsung-pinctrl,
2. The DTS patch (7/10) for S5Pv210 should go into next cycle,
3. The remaining patches (8-10) should go after all previous,
so probably another release cycle.
Best regards,
Krzysztof
Krzysztof Kozlowski (10):
pinctrl: samsung: Define suspend and resume callbacks for all banks
and SoCs
pinctrl: samsung: Document suspend and resume members
pinctrl: samsung: Document hidden requirement about one external
wakeup
pinctrl: samsung: Add dedicated compatible for S5Pv210 wakeup
interrupts
ARM: exynos: Define EINT_WAKEUP_MASK registers for S5Pv210 and
Exynos5433
pinctrl: samsung: Write external wakeup interrupt mask
ARM: dts: s5pv210: Switch to S5Pv210 specific pinctrl wakeup
compatible
ARM: s5pv210: Remove legacy setting of external wakeup interrupts
ARM: exynos: Remove legacy setting of external wakeup interrupts
pinctrl: samsung: Remove legacy API for handling external wakeup
interrupts mask
.../bindings/pinctrl/samsung-pinctrl.txt | 11 ++-
arch/arm/boot/dts/s5pv210.dtsi | 2 +-
arch/arm/mach-exynos/common.h | 2 -
arch/arm/mach-exynos/suspend.c | 16 +++--
arch/arm/mach-s5pv210/common.h | 1 -
arch/arm/mach-s5pv210/pm.c | 16 +++--
drivers/pinctrl/samsung/pinctrl-exynos-arm.c | 16 +++++
drivers/pinctrl/samsung/pinctrl-exynos.c | 78 +++++++++++++++++++---
drivers/pinctrl/samsung/pinctrl-samsung.h | 11 +++
include/linux/soc/samsung/exynos-regs-pmu.h | 8 ++-
10 files changed, 136 insertions(+), 25 deletions(-)
--
2.14.1
^ permalink raw reply
* [RFT v2 00/10] pinctrl: samsung: Remove ugly hack for sharing eint_wakeup_mask
From: Krzysztof Kozlowski @ 2018-07-23 17:52 UTC (permalink / raw)
To: Tomasz Figa, Krzysztof Kozlowski, Sylwester Nawrocki,
Linus Walleij, Rob Herring, Mark Rutland, Kukjin Kim,
Russell King, Kyungmin Park, linux-arm-kernel, linux-samsung-soc,
linux-gpio, devicetree, linux-kernel
Cc: Paweł Chmiel, Sylwester Nawrocki, Chanwoo Choi, Alim Akhtar,
Pankaj Dubey
Hi All,
Changes since v1
================
1. Add Tomasz's ack.
2. Reword description in patch 6/10.
Tests
=====
This is both request for comments and requests for tests. Only basic
tests were done, including suspend to RAM on Odroid U3 (Exynos4412)
with max7768 RTC wakeup. Please kindly test it with devices capable of
suspending and resuming. I am mostly thinking about S5Pv210-based (Aria),
Trats, Trats2 and TM2 (Exynos5433). Existing platforms should not be
broken however changing external interrupt wakeup mask was not done
on Exynos5433.
Description
===========
The Exynos/S5Pv210 machine suspend code needs to write the external
interrupt mask during suspend. The mask is controlled by pin controller
driver: the exynos_wkup_irq_set_wake() in IRQ chip for these wakeup
interrupts.
Therefore pinctrl driver code exposed an exynos_get_eint_wake_mask()
function which was later used as an extern in machine code.
This is quite ugly way of combining driver and machine code,
not portable triggering Sparse and GCC warnings.
This might break suspend capability S5Pv210 on with older DTBs (thus
breaks DTB compatibility), however:
1. just "might" because in case of using older DTB, the wakeup mask
will not be changed during suspend and default reset value (all
interrupts non-masked) should work,
2. mainline support for S5Pv210 with DTB is limited and suspend
to RAM already might be broken.
Dependencies
============
1. The first seven patches should be taken through one tree,
preferably samsung-pinctrl,
2. The DTS patch (7/10) for S5Pv210 should go into next cycle,
3. The remaining patches (8-10) should go after all previous,
so probably another release cycle.
Best regards,
Krzysztof
Krzysztof Kozlowski (10):
pinctrl: samsung: Define suspend and resume callbacks for all banks
and SoCs
pinctrl: samsung: Document suspend and resume members
pinctrl: samsung: Document hidden requirement about one external
wakeup
pinctrl: samsung: Add dedicated compatible for S5Pv210 wakeup
interrupts
ARM: exynos: Define EINT_WAKEUP_MASK registers for S5Pv210 and
Exynos5433
pinctrl: samsung: Write external wakeup interrupt mask
ARM: dts: s5pv210: Switch to S5Pv210 specific pinctrl wakeup
compatible
ARM: s5pv210: Remove legacy setting of external wakeup interrupts
ARM: exynos: Remove legacy setting of external wakeup interrupts
pinctrl: samsung: Remove legacy API for handling external wakeup
interrupts mask
.../bindings/pinctrl/samsung-pinctrl.txt | 11 ++-
arch/arm/boot/dts/s5pv210.dtsi | 2 +-
arch/arm/mach-exynos/common.h | 2 -
arch/arm/mach-exynos/suspend.c | 16 +++--
arch/arm/mach-s5pv210/common.h | 1 -
arch/arm/mach-s5pv210/pm.c | 16 +++--
drivers/pinctrl/samsung/pinctrl-exynos-arm.c | 16 +++++
drivers/pinctrl/samsung/pinctrl-exynos.c | 78 +++++++++++++++++++---
drivers/pinctrl/samsung/pinctrl-samsung.h | 11 +++
include/linux/soc/samsung/exynos-regs-pmu.h | 8 ++-
10 files changed, 136 insertions(+), 25 deletions(-)
--
2.14.1
^ permalink raw reply
* Re: [PATCH v2 00/21] dom0less step1: boot multiple domains from device tree
From: Stefano Stabellini @ 2018-07-23 17:52 UTC (permalink / raw)
To: Julien Grall
Cc: Stefano Stabellini, andrii_anisov, andrew.cooper3, xen-devel,
jbeulich, nd
In-Reply-To: <5411a218-46e1-5f16-64db-eae36476bb81@arm.com>
On Mon, 23 Jul 2018, Julien Grall wrote:
> On 18/07/18 18:48, Stefano Stabellini wrote:
> > On Wed, 18 Jul 2018, Julien Grall wrote:
> > > Hi Stefano,
> > >
> > > On 13/07/18 21:54, Stefano Stabellini wrote:
> > > > On Thu, 12 Jul 2018, Julien Grall wrote:
> > > > > Hi,
> > > > >
> > > > > Would it be possible to provide a branch with the patch applied? It
> > > > > would
> > > > > be
> > > > > nice to have that for every version, so I can easily know on which
> > > > > version
> > > > > of
> > > > > you are based and avoid spending time trying to apply it :).
> > > >
> > > > Makes sense, I'll do from next time
> > >
> > > Could you provide one from this version? So I can review some of your
> > > patches
> > > more easily.
> >
> > http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git
> > dom0less-v2
>
> Thanks. I will have a look at the vpl011 patches. I think the rest is either
> reviewed or will require changes based on other comments.
>
> Let me know if I missed anything.
No, I think it is OK. Thank you.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply
* Re: [Qemu-devel] [PATCH] ppc/xics: fix ICP reset path
From: Greg Kurz @ 2018-07-23 17:51 UTC (permalink / raw)
To: Cédric Le Goater
Cc: qemu-devel, qemu-ppc, David Gibson, Bharata B Rao,
Satheesh Rajendran
In-Reply-To: <821367d0-c3ef-43bd-071c-b2ff50dae7bd@kaod.org>
On Mon, 23 Jul 2018 13:36:36 +0200
Cédric Le Goater <clg@kaod.org> wrote:
> On 07/12/2018 12:01 PM, Greg Kurz wrote:
> > Recent cleanup in commit a028dd423ee6 dropped the ICPStateClass::reset
> > handler. It is now up to child ICP classes to call the DeviceClass::reset
> > handler of the parent class, thanks to device_class_set_parent_reset().
> > This is a better object programming pattern,
>
> I think that the device_class_set_parent* routines just obfuscate a little
> more the object programming approach of QEMU.
>
Really ?
> > but unfortunately it causes QEMU to crash during CPU hotplug:
>
> I guess I did not try that :/ Would it be complex to add a spapr unit test
> for CPU hotplug ? It would be useful.
>
Actually, there's one in tests/cpu-plug-test.c but it runs with
accel=qtest, and thus it doesn't exercise the KVM ICP code.
I've patched it to use accel=kvm:tcg on pseries: it would have caught
the SEGV indeed. There's another problem though: QEMU sometimes stays
stuck in kvm_vcpu_ioctl() in 'disk sleep' state, causing the test
to hang until someone kills QEMU with SIGKILL.
Not sure yet what's happening...
> > (qemu) device_add host-spapr-cpu-core,id=core1,core-id=1
> > Segmentation fault (core dumped)
> >
> > When the hotplug path tries to reset the ICP device, we end up calling:
> >
> > static void icp_kvm_reset(DeviceState *dev)
> > {
> > ICPStateClass *icpc = ICP_GET_CLASS(dev);
> >
> > icpc->parent_reset(dev);
> >
> > but icpc->parent_reset is NULL... This happens because icp_kvm_class_init()
> > calls:
> >
> > device_class_set_parent_reset(dc, icp_kvm_reset,
> > &icpc->parent_reset);
> >
> > but dc->reset, ie, DeviceClass::reset for the TYPE_ICP type, is
> > itself NULL.>
> > This patch hence sets DeviceClass::reset for the TYPE_ICP type to
> > point to icp_reset(). It then registers a reset handler that calls
> > DeviceClass::reset. If the ICP subtype has configured its own reset
> > handler with device_class_set_parent_reset(), this ensures it will
> > be called first and it can then call ICPStateClass::parent_reset
> > safely. This fixes the reset path for the TYPE_KVM_ICP type, which
> > is the only subtype that defines its own reset function.
>
> So, now, isn't ICP reset called twice on KVM ?
>
No. We have one path for cold plugged CPUS:
#0 icp_reset (dev=0x113db100) at /home/greg/Work/qemu/qemu-spapr/hw/intc/xics.c:296
#1 0x0000000010146d0c in icp_kvm_reset (dev=0x113db100) at /home/greg/Work/qemu/qemu-spapr/hw/intc/xics_kvm.c:121
#2 0x00000000101434b0 in icp_reset_handler (dev=0x113db100) at /home/greg/Work/qemu/qemu-spapr/hw/intc/xics.c:310
#3 0x00000000104c3744 in qemu_devices_reset () at /home/greg/Work/qemu/qemu-spapr/hw/core/reset.c:69
#4 0x00000000101d61c8 in spapr_machine_reset () at /home/greg/Work/qemu/qemu-spapr/hw/ppc/spapr.c:1639
#5 0x00000000103de654 in qemu_system_reset (reason=SHUTDOWN_CAUSE_NONE) at /home/greg/Work/qemu/qemu-spapr/vl.c:1645
...
and one path for hot plugged CPUs:
#0 icp_reset (dev=0x11268e40) at /home/greg/Work/qemu/qemu-spapr/hw/intc/xics.c:296
#1 0x0000000010146d0c in icp_kvm_reset (dev=0x11268e40) at /home/greg/Work/qemu/qemu-spapr/hw/intc/xics_kvm.c:121
#2 0x00000000104be300 in device_reset (dev=0x11268e40) at /home/greg/Work/qemu/qemu-spapr/hw/core/qdev.c:1082
#3 0x00000000104bd62c in device_set_realized (obj=0x11268e40, value=true, errp=0x7fffffffcfa8) at /home/greg/Work/qemu/qemu-spapr/hw/core/qdev.c:867
...
> Anyway, we can sort that out if necessary.
>
> Thanks,
>
> C.
>
> > Reported-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
> > Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> > Fixes: a028dd423ee6dfd091a8c63028240832bf10f671
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> > hw/intc/xics.c | 14 +++++++++++---
> > 1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> > index b9f1a3c97214..c90c893228dc 100644
> > --- a/hw/intc/xics.c
> > +++ b/hw/intc/xics.c
> > @@ -291,7 +291,7 @@ static const VMStateDescription vmstate_icp_server = {
> > },
> > };
> >
> > -static void icp_reset(void *dev)
> > +static void icp_reset(DeviceState *dev)
> > {
> > ICPState *icp = ICP(dev);
> >
> > @@ -303,6 +303,13 @@ static void icp_reset(void *dev)
> > qemu_set_irq(icp->output, 0);
> > }
> >
> > +static void icp_reset_handler(void *dev)
> > +{
> > + DeviceClass *dc = DEVICE_GET_CLASS(dev);
> > +
> > + dc->reset(dev);
> > +}
> > +
> > static void icp_realize(DeviceState *dev, Error **errp)
> > {
> > ICPState *icp = ICP(dev);
> > @@ -345,7 +352,7 @@ static void icp_realize(DeviceState *dev, Error **errp)
> > return;
> > }
> >
> > - qemu_register_reset(icp_reset, dev);
> > + qemu_register_reset(icp_reset_handler, dev);
> > vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp);
> > }
> >
> > @@ -354,7 +361,7 @@ static void icp_unrealize(DeviceState *dev, Error **errp)
> > ICPState *icp = ICP(dev);
> >
> > vmstate_unregister(NULL, &vmstate_icp_server, icp);
> > - qemu_unregister_reset(icp_reset, dev);
> > + qemu_unregister_reset(icp_reset_handler, dev);
> > }
> >
> > static void icp_class_init(ObjectClass *klass, void *data)
> > @@ -363,6 +370,7 @@ static void icp_class_init(ObjectClass *klass, void *data)
> >
> > dc->realize = icp_realize;
> > dc->unrealize = icp_unrealize;
> > + dc->reset = icp_reset;
> > }
> >
> > static const TypeInfo icp_info = {
> >
>
^ permalink raw reply
* Re: [PATCH v07 2/9] hotplug/cpu: Add operation queuing function
From: Nathan Fontenot @ 2018-07-23 17:51 UTC (permalink / raw)
To: Michael Bringmann, linuxppc-dev; +Cc: John Allen, Tyrel Datwyler, Thomas Falcon
In-Reply-To: <a2c023e9-6997-d1a3-0110-fa8c788e64ee@linux.vnet.ibm.com>
On 07/13/2018 03:18 PM, Michael Bringmann wrote:
> migration/dlpar: This patch adds function dlpar_queue_action()
> which will queued up information about a CPU/Memory 'readd'
> operation according to resource type, action code, and DRC index.
> At a subsequent point, the list of operations can be run/played
> in series. Examples of such oprations include 'readd' of CPU
> and Memory blocks identified as having changed their associativity
> during an LPAR migration event. >
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> Changes in patch:
> -- Correct drc_index before adding to pseries_hp_errorlog struct
> -- Correct text of notice
> -- Revise queuing model to save up all of the DLPAR actions for
> later execution.
> -- Restore list init statement missing from patch
> -- Move call to apply queued operations into 'mobility.c'
> -- Compress some code
> -- Rename some of queueing function APIs
> -- Revise implementation to push execution of queued operations
> to a workqueue task.
> -- Cleanup reference to outdated queuing operation.
> ---
> arch/powerpc/include/asm/rtas.h | 2 +
> arch/powerpc/platforms/pseries/dlpar.c | 61 +++++++++++++++++++++++++++++
> arch/powerpc/platforms/pseries/mobility.c | 4 ++
> arch/powerpc/platforms/pseries/pseries.h | 2 +
> 4 files changed, 69 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
> index 71e393c..4f601c7 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -310,12 +310,14 @@ struct pseries_hp_errorlog {
> struct { __be32 count, index; } ic;
> char drc_name[1];
> } _drc_u;
> + struct list_head list;
> };
>
> #define PSERIES_HP_ELOG_RESOURCE_CPU 1
> #define PSERIES_HP_ELOG_RESOURCE_MEM 2
> #define PSERIES_HP_ELOG_RESOURCE_SLOT 3
> #define PSERIES_HP_ELOG_RESOURCE_PHB 4
> +#define PSERIES_HP_ELOG_RESOURCE_PMT 5
>
> #define PSERIES_HP_ELOG_ACTION_ADD 1
> #define PSERIES_HP_ELOG_ACTION_REMOVE 2
> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
> index a0b20c0..7264b8e 100644
> --- a/arch/powerpc/platforms/pseries/dlpar.c
> +++ b/arch/powerpc/platforms/pseries/dlpar.c
> @@ -25,6 +25,7 @@
> #include <asm/prom.h>
> #include <asm/machdep.h>
> #include <linux/uaccess.h>
> +#include <linux/delay.h>
> #include <asm/rtas.h>
>
> static struct workqueue_struct *pseries_hp_wq;
> @@ -329,6 +330,8 @@ int dlpar_release_drc(u32 drc_index)
> return 0;
> }
>
> +static int dlpar_pmt(struct pseries_hp_errorlog *work);
> +
> static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
> {
> int rc;
> @@ -357,6 +360,9 @@ static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
> case PSERIES_HP_ELOG_RESOURCE_CPU:
> rc = dlpar_cpu(hp_elog);
> break;
> + case PSERIES_HP_ELOG_RESOURCE_PMT:
> + rc = dlpar_pmt(hp_elog);
> + break;
> default:
> pr_warn_ratelimited("Invalid resource (%d) specified\n",
> hp_elog->resource);
> @@ -407,6 +413,61 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
> }
> }
>
> +LIST_HEAD(dlpar_delayed_list);
> +
> +int dlpar_queue_action(int resource, int action, u32 drc_index)
> +{
> + struct pseries_hp_errorlog *hp_errlog;
> +
> + hp_errlog = kmalloc(sizeof(struct pseries_hp_errorlog), GFP_KERNEL);
> + if (!hp_errlog)
> + return -ENOMEM;
> +
> + hp_errlog->resource = resource;
> + hp_errlog->action = action;
> + hp_errlog->id_type = PSERIES_HP_ELOG_ID_DRC_INDEX;
> + hp_errlog->_drc_u.drc_index = cpu_to_be32(drc_index);
> +
> + list_add_tail(&hp_errlog->list, &dlpar_delayed_list);
> +
> + return 0;
> +}
> +
> +static int dlpar_pmt(struct pseries_hp_errorlog *work)
> +{
> + struct list_head *pos, *q;
> +
> + ssleep(15);
> +
> + list_for_each_safe(pos, q, &dlpar_delayed_list) {
> + struct pseries_hp_errorlog *tmp;
> +
> + tmp = list_entry(pos, struct pseries_hp_errorlog, list);
> + handle_dlpar_errorlog(tmp);
> +
> + list_del(pos);
> + kfree(tmp);
> +
> + ssleep(10);
> + }
> +
> + return 0;
> +}
> +
> +int dlpar_queued_actions_run(void)
> +{
> + if (!list_empty(&dlpar_delayed_list)) {
> + struct pseries_hp_errorlog hp_errlog;
> +
> + hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_PMT;
> + hp_errlog.action = 0;
> + hp_errlog.id_type = 0;
> +
> + queue_hotplug_event(&hp_errlog, 0, 0); > + }
> + return 0;
> +}
I'm a bit confused by this. Is there a reason this needs to queue a
hotplug event instead of just walking the list as is done in dlpar_pmt?
-Nathan
> +
> static int dlpar_parse_resource(char **cmd, struct pseries_hp_errorlog *hp_elog)
> {
> char *arg;
> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> index f6364d9..d0d1cae 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -378,6 +378,10 @@ static ssize_t migration_store(struct class *class,
> return rc;
>
> post_mobility_fixup();
> +
> + /* Apply any necessary changes identified during fixup */
> + dlpar_queued_actions_run();
> +
> return count;
> }
>
> diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
> index 60db2ee..72ca996 100644
> --- a/arch/powerpc/platforms/pseries/pseries.h
> +++ b/arch/powerpc/platforms/pseries/pseries.h
> @@ -61,6 +61,8 @@ extern struct device_node *dlpar_configure_connector(__be32,
>
> void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
> struct completion *hotplug_done, int *rc);
> +int dlpar_queue_action(int resource, int action, u32 drc_index);
> +int dlpar_queued_actions_run(void);
> #ifdef CONFIG_MEMORY_HOTPLUG
> int dlpar_memory(struct pseries_hp_errorlog *hp_elog);
> #else
>
^ permalink raw reply
* Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields
From: Rob Herring @ 2018-07-23 17:51 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Andrew Jeffery, Mark Rutland, devicetree, Greg Kroah-Hartman,
Eugene.Cho, a.amelkin, linux-kernel@vger.kernel.org, Joel Stanley,
stewart, OpenBMC Maillist,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <c69d0e7c3a95d83da87b566a62619462671a233b.camel@kernel.crashing.org>
On Wed, Jul 18, 2018 at 5:58 PM Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> On Wed, 2018-07-18 at 13:50 -0600, Rob Herring wrote:
> >
> > > So Rob, I think that's precisely where the disconnect is.
> > >
> > > I think we all (well hopefully) agree that those few tunables don't fit
> > > in any existing subystem and aren't likely to ever do (famous last
> > > words...).
> > >
> > > Where we disagree is we want to make this parametrized via the DT, and
> > > you want us to hard wire the list in some kind of SoC driver for a
> > > given SoC family/version.
> > >
> > > The reason I think hard wiring the list in the driver is not a great
> > > solution is that that list in itself is prone to variations, possibly
> > > fairly often, between boards, vendors, versions of boards, etc...
> >
> > Can we separate the list of what's enabled from the details of the
> > registers? Even if we put all this into DT, we may still want to have
> > some separation for dts file structure. Some portion of this has to be
> > SoC specific because you are simply exposing SoC registers.
>
> Not sure I understand what you mean by "what's enabled".
For example with VGA muxing, you can separate the details of the register field location and whether you want that tunable or not for a given platform.
> The goal here is to expose register "fields" (nor raw values, some
> registers need locking for RMW etc... the kernel would handle that via
> syscon locking I expect), so basically named "tunables" to userspace.
>
> Userspace is the one that knows the values for a given system.
>
> > > We can't know for sure every SoC tunable (out of the gazillions in
> > > those chips) are going to be needed for a given system. We know which
> > > ones we do use for ours, and that's a couple of handfuls, but it could
> > > be that Dell need a slightly different set, and so might Yadro, or so
> > > might our next board revision for that matter.
> >
> > That's very hand wavy. Do you have some concrete examples (i.e. dts
> > files) showing the differences.
>
> We could list what we have on the pile for some of our IBM systems
> today, but we would need Dell and Yadro to chime in with what they
> need.
>
> I still, from experience with that stuff and gut feeling, am pretty
> sure it's going to be a moving list, and updating the kernel driver
> constantly isn't going to fly.
>
> > One problem I'm having with this is you are still going to need per
> > board specifics in userspace.
>
> Yes. Userspace is ultimately the one that knows what needs to be done
> on a given machine.
>
> > You may be moving some of details out,
> > but moving to DT is not going to completely eliminate that.
>
> We aren't trying to either. We are trying to make sure we don't need to
> change the *kernel* all the time, in part bcs we are pushing hard for
> OpenBMC vendors to use upstream with, if possible, no vendor changes.
>
> So userspace has to know the board specific tunables anyways. Today in
> many systems, it does that by whacking /dev/mem. I guess you can
> understand why that is bad :-)
>
> > I agree
> > that not using /dev/mem is a good thing, but there are several ways
> > you could do that independent of any DT binding.
>
> Such as ? The only other option is to have one or more kernel drivers
> that have built-in the precise and complete list of those tunables that
> need to be exposed.
UIO is one way. That at least would restrict things to a specific address range(s) compared to /dev/mem, but would still possibly leave other registers exposed.
Another option would be with your proposed driver to provide a userspace interface to register the register fields you need. Not sure if allowing userspace to be able to specify that is a security concern (though allowing a DT update would be the same).
> It's not impossible, but I worry that it's not going to scale terribly
> well, and that vendors will constantly "fork" that driver to add
> different things to that list.
>
> I might be wrong here, so I'd like for Eugene (Dell) and Alexander
> (Yadro) to chime in, but experience with BMCs has shown that we
> regularily , as we add a feature or rewrite something, need to find
> another new magic SoC tunable the HW manufacturer hid somewhere that
> will make our stuff work.
>
> > > Now, updating the device-tree in the board flash with whatever vendor
> > > specific information is needed is a LOT easier than getting the kernel
> > > driver constantly updated. The device-tree after all is there to
> > > reflect among other things system specific ways in which the SoC is
> > > wired and configured. This is rather close...
> >
> > Sadly, updating my kernel is easier than updating my PC firmware
> > (though packaged firmware on my current laptop changes that). I can
> > assure you that ARM boards are generally much worse in that regard.
> > BTW, you may want to pay attention to EBBR[1][2]. Not sure how much
> > you care for BMCs, but there may be some interest.
>
> You are conflating your host kernel and your BMC here. The BMC kernel
> is part of the "firmware", as is its DTS and the BMC userspace.
>
> (Again this isn't the host DTS, this is the BMC DTS).
Yes, I get that. My point was just about what's easier to do actual updates for. But mainline vs. forks of the kernel is a different thing.
> They get all updated together. My point isn't about the ease or
> difficulty for a *user* to udpate their BMC, in that case the solutions
> above are equivalent.
>
> The point is from a system vendor perspective. A system vendor using
> OpenBMC will *customize* their BMC build in various ways. Typically
> they *will* have a custom DT since this is what represents their
> specific system and they will have some specific userspace bits.
>
> However, we are trying very hard for them *not* to fork the kernel, and
> if possible move OpenBMC towards a fully upstream kernel (still working
> on getting all the SoC drivers cleaned up and pushed up but it's moving
> in the right direction).
>
> So from a vendor perspective, such as us IBM, we *alread* have custom
> DTs and customized userspace, that's part of the normal flow of
> deploying a BMC.
>
> However, we are trying *NOT* to have a custom kernel (and we don't, at
> the moment there is an OpenBMC kernel accross vendors, though it's not
> *yet* fully upstream).
So if you are only trying to avoid kernel changes, then what is the advantage of putting this in DT and passing it thru to userspace. Why not just put all
>
> So if the solution proposed is prone to requiring frequent changes to a
> kernel driver, that solution makes the above a lot more difficult and
> will encourage vendors to keep forking kernels.
>
> This is what we are trying to avoid.
>
> Cheers,
> Ben.
>
> > Rob
> >
> > [1] https://github.com/ARM-software/ebbr
> > [2] https://lists.linaro.org/pipermail/boot-architecture/
^ permalink raw reply
* Re: rte_mbuf library likely()/unlikely()
From: Honnappa Nagarahalli @ 2018-07-23 17:51 UTC (permalink / raw)
To: Morten Brørup, Olivier Matz; +Cc: dev@dpdk.org
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35B421EE@smartserver.smartshare.dk>
Do you see any performance improvements with these changes?
-----Original Message-----
From: dev <dev-bounces@dpdk.org> On Behalf Of Morten Brørup
Sent: Monday, July 23, 2018 8:54 AM
To: Olivier Matz <olivier.matz@6wind.com>
Cc: dev@dpdk.org
Subject: [dpdk-dev] rte_mbuf library likely()/unlikely()
Hi Olivier,
I noticed that __rte_pktmbuf_read() could do with an unlikely(), so I went through the entire library. Here are my suggested modifications.
diff -bu rte_mbuf.c.orig rte_mbuf.c
--- rte_mbuf.c.orig 2018-07-23 15:13:22.000000000 +0200
+++ rte_mbuf.c 2018-07-23 15:32:53.000000000 +0200
@@ -173,19 +173,19 @@
{
unsigned int nb_segs, pkt_len;
- if (m == NULL)
+ if (unlikely(m == NULL))
rte_panic("mbuf is NULL\n");
/* generic checks */
- if (m->pool == NULL)
+ if (unlikely(m->pool == NULL))
rte_panic("bad mbuf pool\n");
- if (m->buf_iova == 0)
+ if (unlikely(m->buf_iova == 0))
rte_panic("bad IO addr\n");
- if (m->buf_addr == NULL)
+ if (unlikely(m->buf_addr == NULL))
rte_panic("bad virt addr\n");
uint16_t cnt = rte_mbuf_refcnt_read(m);
- if ((cnt == 0) || (cnt == UINT16_MAX))
+ if (unlikely((cnt == 0) || (cnt == UINT16_MAX)))
rte_panic("bad ref cnt\n");
/* nothing to check for sub-segments */
@@ -193,7 +193,7 @@
return;
/* data_len is supposed to be not more than pkt_len */
- if (m->data_len > m->pkt_len)
+ if (unlikely(m->data_len > m->pkt_len))
rte_panic("bad data_len\n");
nb_segs = m->nb_segs;
@@ -204,9 +204,9 @@
pkt_len -= m->data_len;
} while ((m = m->next) != NULL);
- if (nb_segs)
+ if (unlikely(nb_segs))
rte_panic("bad nb_segs\n");
- if (pkt_len)
+ if (unlikely(pkt_len))
rte_panic("bad pkt_len\n");
}
@@ -249,7 +249,7 @@
const struct rte_mbuf *seg = m;
uint32_t buf_off = 0, copy_len;
- if (off + len > rte_pktmbuf_pkt_len(m))
+ if (unlikely(off + len > rte_pktmbuf_pkt_len(m)))
return NULL;
while (off >= rte_pktmbuf_data_len(seg)) {
@@ -257,7 +257,7 @@
seg = seg->next;
}
- if (off + len <= rte_pktmbuf_data_len(seg))
+ if (likely(off + len <= rte_pktmbuf_data_len(seg)))
return rte_pktmbuf_mtod_offset(seg, char *, off);
/* rare case: header is split among several segments */
@@ -344,7 +344,7 @@
unsigned int i;
int ret;
- if (buflen == 0)
+ if (unlikely(buflen == 0))
return -1;
buf[0] = '\0';
@@ -355,9 +355,9 @@
if (name == NULL)
name = rx_flags[i].default_name;
ret = snprintf(buf, buflen, "%s ", name);
- if (ret < 0)
+ if (unlikely(ret < 0))
return -1;
- if ((size_t)ret >= buflen)
+ if (unlikely((size_t)ret >= buflen))
return -1;
buf += ret;
buflen -= ret;
@@ -440,7 +440,7 @@
unsigned int i;
int ret;
- if (buflen == 0)
+ if (unlikely(buflen == 0))
return -1;
buf[0] = '\0';
@@ -451,9 +451,9 @@
if (name == NULL)
name = tx_flags[i].default_name;
ret = snprintf(buf, buflen, "%s ", name);
- if (ret < 0)
+ if (unlikely(ret < 0))
return -1;
- if ((size_t)ret >= buflen)
+ if (unlikely((size_t)ret >= buflen))
return -1;
buf += ret;
buflen -= ret;
diff -bu rte_mbuf.h.orig rte_mbuf.h
--- rte_mbuf.h.orig 2018-07-23 15:13:26.000000000 +0200
+++ rte_mbuf.h 2018-07-23 15:24:25.000000000 +0200
@@ -1007,7 +1007,7 @@
{
struct rte_mbuf *m;
- if (rte_mempool_get(mp, (void **)&m) < 0)
+ if (unlikely(rte_mempool_get(mp, (void **)&m) < 0))
return NULL;
MBUF_RAW_ALLOC_CHECK(m);
return m;
@@ -1268,7 +1268,7 @@
static inline struct rte_mbuf *rte_pktmbuf_alloc(struct rte_mempool *mp)
{
struct rte_mbuf *m;
- if ((m = rte_mbuf_raw_alloc(mp)) != NULL)
+ if (likely((m = rte_mbuf_raw_alloc(mp)) != NULL))
rte_pktmbuf_reset(m);
return m;
}
@@ -1696,7 +1696,7 @@
{
struct rte_mbuf *m_next;
- if (m != NULL)
+ if (likely(m != NULL))
__rte_mbuf_sanity_check(m, 1);
while (m != NULL) {
@@ -2099,7 +2099,7 @@
struct rte_mbuf *cur_tail;
/* Check for number-of-segments-overflow */
- if (head->nb_segs + tail->nb_segs > RTE_MBUF_MAX_NB_SEGS)
+ if (unlikely(head->nb_segs + tail->nb_segs > RTE_MBUF_MAX_NB_SEGS))
return -EOVERFLOW;
/* Chain 'tail' onto the old tail */
@@ -2147,28 +2147,28 @@
m->outer_l3_len;
/* Headers are fragmented */
- if (rte_pktmbuf_data_len(m) < inner_l3_offset + m->l3_len + m->l4_len)
+ if (unlikely(rte_pktmbuf_data_len(m) < inner_l3_offset + m->l3_len + m->l4_len))
return -ENOTSUP;
/* IP checksum can be counted only for IPv4 packet */
- if ((ol_flags & PKT_TX_IP_CKSUM) && (ol_flags & PKT_TX_IPV6))
+ if (unlikely((ol_flags & PKT_TX_IP_CKSUM) && (ol_flags & PKT_TX_IPV6)))
return -EINVAL;
/* IP type not set when required */
if (ol_flags & (PKT_TX_L4_MASK | PKT_TX_TCP_SEG))
- if (!(ol_flags & (PKT_TX_IPV4 | PKT_TX_IPV6)))
+ if (unlikely(!(ol_flags & (PKT_TX_IPV4 | PKT_TX_IPV6))))
return -EINVAL;
/* Check requirements for TSO packet */
if (ol_flags & PKT_TX_TCP_SEG)
- if ((m->tso_segsz == 0) ||
+ if (unlikely((m->tso_segsz == 0) ||
((ol_flags & PKT_TX_IPV4) &&
- !(ol_flags & PKT_TX_IP_CKSUM)))
+ !(ol_flags & PKT_TX_IP_CKSUM))))
return -EINVAL;
/* PKT_TX_OUTER_IP_CKSUM set for non outer IPv4 packet. */
- if ((ol_flags & PKT_TX_OUTER_IP_CKSUM) &&
- !(ol_flags & PKT_TX_OUTER_IPV4))
+ if (unlikely((ol_flags & PKT_TX_OUTER_IP_CKSUM) &&
+ !(ol_flags & PKT_TX_OUTER_IPV4)))
return -EINVAL;
return 0;
diff -bu rte_mbuf_ptype.c.orig rte_mbuf_ptype.c
--- rte_mbuf_ptype.c.orig 2018-07-23 15:45:49.000000000 +0200
+++ rte_mbuf_ptype.c 2018-07-23 15:44:59.000000000 +0200
@@ -118,15 +118,15 @@
{
int ret;
- if (buflen == 0)
+ if (unlikely(buflen == 0))
return -1;
buf[0] = '\0';
if ((ptype & RTE_PTYPE_ALL_MASK) == RTE_PTYPE_UNKNOWN) {
ret = snprintf(buf, buflen, "UNKNOWN");
- if (ret < 0)
+ if (unlikely(ret < 0))
return -1;
- if ((size_t)ret >= buflen)
+ if (unlikely((size_t)ret >= buflen))
return -1;
return 0;
}
@@ -134,9 +134,9 @@
if ((ptype & RTE_PTYPE_L2_MASK) != 0) {
ret = snprintf(buf, buflen, "%s ",
rte_get_ptype_l2_name(ptype));
- if (ret < 0)
+ if (unlikely(ret < 0))
return -1;
- if ((size_t)ret >= buflen)
+ if (unlikely((size_t)ret >= buflen))
return -1;
buf += ret;
buflen -= ret;
@@ -144,9 +144,9 @@
if ((ptype & RTE_PTYPE_L3_MASK) != 0) {
ret = snprintf(buf, buflen, "%s ",
rte_get_ptype_l3_name(ptype));
- if (ret < 0)
+ if (unlikely(ret < 0))
return -1;
- if ((size_t)ret >= buflen)
+ if (unlikely((size_t)ret >= buflen))
return -1;
buf += ret;
buflen -= ret;
@@ -154,9 +154,9 @@
if ((ptype & RTE_PTYPE_L4_MASK) != 0) {
ret = snprintf(buf, buflen, "%s ",
rte_get_ptype_l4_name(ptype));
- if (ret < 0)
+ if (unlikely(ret < 0))
return -1;
- if ((size_t)ret >= buflen)
+ if (unlikely((size_t)ret >= buflen))
return -1;
buf += ret;
buflen -= ret;
@@ -164,9 +164,9 @@
if ((ptype & RTE_PTYPE_TUNNEL_MASK) != 0) {
ret = snprintf(buf, buflen, "%s ",
rte_get_ptype_tunnel_name(ptype));
- if (ret < 0)
+ if (unlikely(ret < 0))
return -1;
- if ((size_t)ret >= buflen)
+ if (unlikely((size_t)ret >= buflen))
return -1;
buf += ret;
buflen -= ret;
@@ -174,9 +174,9 @@
if ((ptype & RTE_PTYPE_INNER_L2_MASK) != 0) {
ret = snprintf(buf, buflen, "%s ",
rte_get_ptype_inner_l2_name(ptype));
- if (ret < 0)
+ if (unlikely(ret < 0))
return -1;
- if ((size_t)ret >= buflen)
+ if (unlikely((size_t)ret >= buflen))
return -1;
buf += ret;
buflen -= ret;
@@ -184,9 +184,9 @@
if ((ptype & RTE_PTYPE_INNER_L3_MASK) != 0) {
ret = snprintf(buf, buflen, "%s ",
rte_get_ptype_inner_l3_name(ptype));
- if (ret < 0)
+ if (unlikely(ret < 0))
return -1;
- if ((size_t)ret >= buflen)
+ if (unlikely((size_t)ret >= buflen))
return -1;
buf += ret;
buflen -= ret;
@@ -194,9 +194,9 @@
if ((ptype & RTE_PTYPE_INNER_L4_MASK) != 0) {
ret = snprintf(buf, buflen, "%s ",
rte_get_ptype_inner_l4_name(ptype));
- if (ret < 0)
+ if (unlikely(ret < 0))
return -1;
- if ((size_t)ret >= buflen)
+ if (unlikely((size_t)ret >= buflen))
return -1;
buf += ret;
buflen -= ret;
Med venlig hilsen / kind regards
Morten Brørup
CTO
SmartShare Systems A/S
Tonsbakken 16-18
DK-2740 Skovlunde
Denmark
Office +45 70 20 00 93
Direct +45 89 93 50 22
Mobile +45 25 40 82 12
mb@smartsharesystems.com <mailto:mb@smartsharesystems.com>
www.smartsharesystems.com <https://www.smartsharesystems.com/>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply
* Re: [PATCH 1/1] i2c: rcar: handle RXDMA HW behaviour on Gen3
From: Wolfram Sang @ 2018-07-23 17:50 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Wolfram Sang, Linux I2C, Linux-Renesas, Yoshihiro Shimoda
In-Reply-To: <20180629075404.ivnaauv2hdir32ig@ninjato>
[-- Attachment #1: Type: text/plain, Size: 943 bytes --]
On Fri, Jun 29, 2018 at 09:54:04AM +0200, Wolfram Sang wrote:
>
> > > + /* Gen3 needs a reset before allowing RXDMA once */
> > > + if (priv->devtype == I2C_RCAR_GEN3) {
> >
> > So on R-Car Gen3 the device is always reset, even if no reads will be done?
> > Or is that something you cannot check for at this point?
>
> You got a point. I copied the behaviour from the BSP to do this as early
> as possible. Still, it might be worthwhile to merge this into
> rcar_i2c_request_dma() and move the call to it to the front. Will check.
I will apply this patch as is because it is easier to backport to stable
this way.
You are still right, we can do better and not issue a reset any time. To
get there, though, I want two kinds of refactoring beforehand. And I
think it is best to do this on top of this patch and refactor it along.
BSP backporters can then decide if they want the refactored version or
not.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] drm/fb_helper: Add drm_fb_helper_output_poll_changed_with_rpm()
From: Lyude Paul @ 2018-07-23 17:50 UTC (permalink / raw)
To: Lukas Wunner
Cc: nouveau, Gustavo Padovan, Maarten Lankhorst, Sean Paul,
David Airlie, Ben Skeggs, Daniel Vetter, Ville Syrjälä,
dri-devel, linux-kernel
In-Reply-To: <20180721093955.GA32572@wunner.de>
On Sat, 2018-07-21 at 11:39 +0200, Lukas Wunner wrote:
> On Thu, Jul 19, 2018 at 08:08:15PM -0400, Lyude Paul wrote:
> > On Thu, 2018-07-19 at 09:49 +0200, Lukas Wunner wrote:
> > > On Wed, Jul 18, 2018 at 04:56:39PM -0400, Lyude Paul wrote:
> > > > When DP MST hubs get confused, they can occasionally stop responding
> > > > for
> > > > a good bit of time up until the point where the DRM driver manages to
> > > > do the right DPCD accesses to get it to start responding again. In a
> > > > worst case scenario however, this process can take upwards of 10+
> > > > seconds.
> > > >
> > > > Currently we use the default output_poll_changed handler
> > > > drm_fb_helper_output_poll_changed() to handle output polling, which
> > > > doesn't happen to grab any power references on the device when
> > > > polling.
> > > > If we're unlucky enough to have a hub (such as Lenovo's infamous
> > > > laptop
> > > > docks for the P5x/P7x series) that's easily startled and confused,
> > > > this
> > > > can lead to a pretty nasty deadlock situation that looks like this:
> > > >
> > > > - Hotplug event from hub happens, we enter
> > > > drm_fb_helper_output_poll_changed() and start communicating with the
> > > > hub
> > > > - While we're in drm_fb_helper_output_poll_changed() and attempting to
> > > > communicate with the hub, we end up confusing it and cause it to
> > > > stop
> > > > responding for at least 10 seconds
> > > > - After 5 seconds of being in drm_fb_helper_output_poll_changed(), the
> > > > pm core attempts to put the GPU into autosuspend, which ends up
> > > > calling drm_kms_helper_poll_disable()
> > > > - While the runtime PM core is waiting in
> > > > drm_kms_helper_poll_disable()
> > > > for the output poll to finish, we end up finally detecting an MST
> > > > display
> > > > - We notice the new display and tries to enable it, which triggers
> > > > an atomic commit which triggers a call to pm_runtime_get_sync()
> > > > - the output poll thread deadlocks the pm core waiting for the pm core
> > > > to finish the autosuspend request while the pm core waits for the
> > > > output poll thread to finish
> > >
> > > The correct fix is to call pm_runtime_get_sync() *conditionally* in
> > > the atomic commit which enables the display, using the same conditional
> > > as d61a5c106351, i.e. if (!drm_kms_helper_is_poll_worker()).
>
> First of all, I was mistaken when I wrote above that a check for
> !drm_kms_helper_is_poll_worker() would solve the problem. Sorry!
> It doesn't because the call to pm_runtime_get_sync() is not happening
> in output_poll_execute() but in drm_dp_mst_link_probe_work().
>
> Looking once more at the three stack traces you've provided, we've got:
> - output_poll_execute() stuck waiting for fb_helper->lock
> which is held by drm_dp_mst_link_probe_work()
> - rpm_suspend() stuck waiting for output_poll_execute() to finish
> - drm_dp_mst_link_probe_work() stuck waiting in rpm_resume()
>
> For the moment we can ignore the first task, i.e. output_poll_execute(),
> and focus on the latter two.
>
> As said I'm unfamiliar with MST but browsing through drm_dp_mst_topology.c
> I notice that drm_dp_mst_link_probe_work() is the ->work element in
> drm_dp_mst_topology_mgr() and is queued on HPD. I further notice that
> the work item is flushed on ->runtime_suspend:
>
> nouveau_pmops_runtime_suspend()
> nouveau_do_suspend()
> nouveau_display_suspend()
> nouveau_display_fini()
> disp->fini() == nv50_display_fini()
> nv50_mstm_fini()
> drm_dp_mst_topology_mgr_suspend()
> flush_work(&mgr->work);
>
> And before the work item is flushed, the HPD source is quiesced.
>
> So it looks like drm_dp_mst_link_probe_work() can only ever run
> while the GPU is runtime resumed, it never runs while the GPU is
> runtime suspended. This means that you don't have to acquire any
> runtime PM references in or below drm_dp_mst_link_probe_work().
> Au contraire, you must not acquire any because it will deadlock while
> the GPU is runtime suspending. If there are functions which are
> called from drm_dp_mst_link_probe_work() as well as from other contexts,
> and those other contexts need a runtime PM ref to be acquired,
> you need to acquire the runtime PM ref conditionally on not being
> drm_dp_mst_link_probe_work() (using the current_work() technique).
>
> Alternatively, move acquisition of the runtime PM ref further up in
> the call chain to those other contexts.
>
>
> > Anyway-that's why your explanation doesn't make sense: the deadlock is
> > happening because we're calling pm_runtime_get_sync(). If we were to
> > make that call conditional (e.g. drm_kms_helper_is_poll_worker()),
> > all that would mean is that we wouldn't grab any runtime power reference
> > and the GPU would immediately suspend once the atomic commit finished,
> > as the suspend request in Thread 5 would finally get unblocked and thus
> > ----suspend.
>
> Right, that seems to be a bug nouveau_pmops_runtime_suspend():
>
> If a display is plugged in while the GPU is about to runtime suspend,
> the display may be lit up by output_poll_execute() but the GPU will
> then nevertheless be powered off.
>
> I guess after calling drm_kms_helper_poll_disable() we should re-check
> if a crtc has been activated. This should have bumped the runtime PM
> refcount and have_disp_power_ref should be true. In that case, the
> nouveau_pmops_runtime_suspend() should return -EBUSY to abort the
> runtime_suspend.
>
> The same check seems necessary after flushing drm_dp_mst_link_probe_work():
> If the work item lit up a new display, all previous suspend steps need
> to be unwound and -EBUSY needs to be returned to the PM core.
>
> Communication with an MST hub exceeding the autosuspend timeout is
> just one scenario where this bug manifests itself.
>
> BTW, drm_kms_helper_poll_disable() seems to be called twice in the
> runtime_suspend code path, once in nouveau_pmops_runtime_suspend()
> and a second time in nouveau_display_fini().
>
> A stupid question, I notice that nv50_display_fini() calls nv50_mstm_fini()
> only if encoder_type != DRM_MODE_ENCODER_DPMST. Why isn't that == ?
Because there's a difference between DPMST connectors and encoders vs. the
rest of the device's encoders. Every DP MST topology will take up a single
"physical" DP connector on the device that will be marked as disconnected.
This connector also owns the "mstm" (MST manager, referred to as the
drm_dp_mst_topology_mgr in DRM), which through the callbacks nouveau provides
is responsible for creating the fake DP MST ports and encoders. All of these
fake ports will have DPMST encoders as opposed to the physical DP ports, which
will have TMDS encoders. Hence-mstms are only on physical connectors with
TMDS, not fake connectors with DPMST.
>
> Thanks,
>
> Lukas
--
Cheers,
Lyude Paul
^ permalink raw reply
* Re: [PATCH 1/2] drm/fb_helper: Add drm_fb_helper_output_poll_changed_with_rpm()
From: Lyude Paul @ 2018-07-23 17:50 UTC (permalink / raw)
To: Lukas Wunner
Cc: David Airlie, nouveau, linux-kernel, dri-devel, Daniel Vetter,
Ben Skeggs
In-Reply-To: <20180721093955.GA32572@wunner.de>
On Sat, 2018-07-21 at 11:39 +0200, Lukas Wunner wrote:
> On Thu, Jul 19, 2018 at 08:08:15PM -0400, Lyude Paul wrote:
> > On Thu, 2018-07-19 at 09:49 +0200, Lukas Wunner wrote:
> > > On Wed, Jul 18, 2018 at 04:56:39PM -0400, Lyude Paul wrote:
> > > > When DP MST hubs get confused, they can occasionally stop responding
> > > > for
> > > > a good bit of time up until the point where the DRM driver manages to
> > > > do the right DPCD accesses to get it to start responding again. In a
> > > > worst case scenario however, this process can take upwards of 10+
> > > > seconds.
> > > >
> > > > Currently we use the default output_poll_changed handler
> > > > drm_fb_helper_output_poll_changed() to handle output polling, which
> > > > doesn't happen to grab any power references on the device when
> > > > polling.
> > > > If we're unlucky enough to have a hub (such as Lenovo's infamous
> > > > laptop
> > > > docks for the P5x/P7x series) that's easily startled and confused,
> > > > this
> > > > can lead to a pretty nasty deadlock situation that looks like this:
> > > >
> > > > - Hotplug event from hub happens, we enter
> > > > drm_fb_helper_output_poll_changed() and start communicating with the
> > > > hub
> > > > - While we're in drm_fb_helper_output_poll_changed() and attempting to
> > > > communicate with the hub, we end up confusing it and cause it to
> > > > stop
> > > > responding for at least 10 seconds
> > > > - After 5 seconds of being in drm_fb_helper_output_poll_changed(), the
> > > > pm core attempts to put the GPU into autosuspend, which ends up
> > > > calling drm_kms_helper_poll_disable()
> > > > - While the runtime PM core is waiting in
> > > > drm_kms_helper_poll_disable()
> > > > for the output poll to finish, we end up finally detecting an MST
> > > > display
> > > > - We notice the new display and tries to enable it, which triggers
> > > > an atomic commit which triggers a call to pm_runtime_get_sync()
> > > > - the output poll thread deadlocks the pm core waiting for the pm core
> > > > to finish the autosuspend request while the pm core waits for the
> > > > output poll thread to finish
> > >
> > > The correct fix is to call pm_runtime_get_sync() *conditionally* in
> > > the atomic commit which enables the display, using the same conditional
> > > as d61a5c106351, i.e. if (!drm_kms_helper_is_poll_worker()).
>
> First of all, I was mistaken when I wrote above that a check for
> !drm_kms_helper_is_poll_worker() would solve the problem. Sorry!
> It doesn't because the call to pm_runtime_get_sync() is not happening
> in output_poll_execute() but in drm_dp_mst_link_probe_work().
>
> Looking once more at the three stack traces you've provided, we've got:
> - output_poll_execute() stuck waiting for fb_helper->lock
> which is held by drm_dp_mst_link_probe_work()
> - rpm_suspend() stuck waiting for output_poll_execute() to finish
> - drm_dp_mst_link_probe_work() stuck waiting in rpm_resume()
>
> For the moment we can ignore the first task, i.e. output_poll_execute(),
> and focus on the latter two.
>
> As said I'm unfamiliar with MST but browsing through drm_dp_mst_topology.c
> I notice that drm_dp_mst_link_probe_work() is the ->work element in
> drm_dp_mst_topology_mgr() and is queued on HPD. I further notice that
> the work item is flushed on ->runtime_suspend:
>
> nouveau_pmops_runtime_suspend()
> nouveau_do_suspend()
> nouveau_display_suspend()
> nouveau_display_fini()
> disp->fini() == nv50_display_fini()
> nv50_mstm_fini()
> drm_dp_mst_topology_mgr_suspend()
> flush_work(&mgr->work);
>
> And before the work item is flushed, the HPD source is quiesced.
>
> So it looks like drm_dp_mst_link_probe_work() can only ever run
> while the GPU is runtime resumed, it never runs while the GPU is
> runtime suspended. This means that you don't have to acquire any
> runtime PM references in or below drm_dp_mst_link_probe_work().
> Au contraire, you must not acquire any because it will deadlock while
> the GPU is runtime suspending. If there are functions which are
> called from drm_dp_mst_link_probe_work() as well as from other contexts,
> and those other contexts need a runtime PM ref to be acquired,
> you need to acquire the runtime PM ref conditionally on not being
> drm_dp_mst_link_probe_work() (using the current_work() technique).
>
> Alternatively, move acquisition of the runtime PM ref further up in
> the call chain to those other contexts.
>
>
> > Anyway-that's why your explanation doesn't make sense: the deadlock is
> > happening because we're calling pm_runtime_get_sync(). If we were to
> > make that call conditional (e.g. drm_kms_helper_is_poll_worker()),
> > all that would mean is that we wouldn't grab any runtime power reference
> > and the GPU would immediately suspend once the atomic commit finished,
> > as the suspend request in Thread 5 would finally get unblocked and thus
> > ----suspend.
>
> Right, that seems to be a bug nouveau_pmops_runtime_suspend():
>
> If a display is plugged in while the GPU is about to runtime suspend,
> the display may be lit up by output_poll_execute() but the GPU will
> then nevertheless be powered off.
>
> I guess after calling drm_kms_helper_poll_disable() we should re-check
> if a crtc has been activated. This should have bumped the runtime PM
> refcount and have_disp_power_ref should be true. In that case, the
> nouveau_pmops_runtime_suspend() should return -EBUSY to abort the
> runtime_suspend.
>
> The same check seems necessary after flushing drm_dp_mst_link_probe_work():
> If the work item lit up a new display, all previous suspend steps need
> to be unwound and -EBUSY needs to be returned to the PM core.
>
> Communication with an MST hub exceeding the autosuspend timeout is
> just one scenario where this bug manifests itself.
>
> BTW, drm_kms_helper_poll_disable() seems to be called twice in the
> runtime_suspend code path, once in nouveau_pmops_runtime_suspend()
> and a second time in nouveau_display_fini().
>
> A stupid question, I notice that nv50_display_fini() calls nv50_mstm_fini()
> only if encoder_type != DRM_MODE_ENCODER_DPMST. Why isn't that == ?
Because there's a difference between DPMST connectors and encoders vs. the
rest of the device's encoders. Every DP MST topology will take up a single
"physical" DP connector on the device that will be marked as disconnected.
This connector also owns the "mstm" (MST manager, referred to as the
drm_dp_mst_topology_mgr in DRM), which through the callbacks nouveau provides
is responsible for creating the fake DP MST ports and encoders. All of these
fake ports will have DPMST encoders as opposed to the physical DP ports, which
will have TMDS encoders. Hence-mstms are only on physical connectors with
TMDS, not fake connectors with DPMST.
>
> Thanks,
>
> Lukas
--
Cheers,
Lyude Paul
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH 2/2] cpufreq: Fix a circular lock dependency problem
From: Waiman Long @ 2018-07-23 17:49 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar, Thomas Gleixner, Peter Zijlstra,
Ingo Molnar
Cc: linux-kernel, linux-pm, Paul E. McKenney, Greg Kroah-Hartman,
Konrad Rzeszutek Wilk, Waiman Long
In-Reply-To: <1532368179-15263-1-git-send-email-longman@redhat.com>
With lockdep turned on, the following circular lock dependency problem
was reported:
[ 57.470040] ======================================================
[ 57.502900] WARNING: possible circular locking dependency detected
[ 57.535208] 4.18.0-0.rc3.1.el8+7.x86_64+debug #1 Tainted: G
[ 57.577761] ------------------------------------------------------
[ 57.609714] tuned/1505 is trying to acquire lock:
[ 57.633808] 00000000559deec5 (cpu_hotplug_lock.rw_sem){++++}, at: store+0x27/0x120
[ 57.672880]
[ 57.672880] but task is already holding lock:
[ 57.702184] 000000002136ca64 (kn->count#118){++++}, at: kernfs_fop_write+0x1d0/0x410
[ 57.742176]
[ 57.742176] which lock already depends on the new lock.
[ 57.742176]
[ 57.785220]
[ 57.785220] the existing dependency chain (in reverse order) is:
:
[ 58.932512] other info that might help us debug this:
[ 58.932512]
[ 58.973344] Chain exists of:
[ 58.973344] cpu_hotplug_lock.rw_sem --> subsys mutex#5 --> kn->count#118
[ 58.973344]
[ 59.030795] Possible unsafe locking scenario:
[ 59.030795]
[ 59.061248] CPU0 CPU1
[ 59.085377] ---- ----
[ 59.108160] lock(kn->count#118);
[ 59.124935] lock(subsys mutex#5);
[ 59.156330] lock(kn->count#118);
[ 59.186088] lock(cpu_hotplug_lock.rw_sem);
[ 59.208541]
[ 59.208541] *** DEADLOCK ***
In the cpufreq_register_driver() function, the lock sequence is:
cpus_read_lock --> kn->count
For the cpufreq sysfs store method, the lock sequence is:
kn->count --> cpus_read_lock
These sequences are actually safe as they are taking a share lock on
cpu_hotplug_lock. However, the current lockdep code doesn't check for
share locking when detecting circular lock dependency. Fixing that
could be a substantial effort.
Instead, we can work around this problem by using cpus_read_trylock()
in the store method which is much simpler. The chance of not getting
the read lock is extremely small. If that happens, the userspace
application that writes the sysfs file will get an error.
Signed-off-by: Waiman Long <longman@redhat.com>
---
drivers/cpufreq/cpufreq.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index b0dfd32..9cf02d7 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -922,8 +922,22 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
struct cpufreq_policy *policy = to_policy(kobj);
struct freq_attr *fattr = to_attr(attr);
ssize_t ret = -EINVAL;
+ int retries = 3;
- cpus_read_lock();
+ /*
+ * cpus_read_trylock() is used here to work around a circular lock
+ * dependency problem with respect to the cpufreq_register_driver().
+ * With a simple retry loop, the chance of not able to get the
+ * read lock is extremely small.
+ */
+ while (!cpus_read_trylock()) {
+ if (retries-- <= 0)
+ return -EBUSY;
+ /*
+ * Sleep for about 50ms and retry again.
+ */
+ msleep(50);
+ }
if (cpu_online(policy->cpu)) {
down_write(&policy->rwsem);
--
1.8.3.1
^ permalink raw reply related
* [PATCH 1/2] cpu/hotplug: Add a cpus_read_trylock() function
From: Waiman Long @ 2018-07-23 17:49 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar, Thomas Gleixner, Peter Zijlstra,
Ingo Molnar
Cc: linux-kernel, linux-pm, Paul E. McKenney, Greg Kroah-Hartman,
Konrad Rzeszutek Wilk, Waiman Long
In-Reply-To: <1532368179-15263-1-git-send-email-longman@redhat.com>
There are use cases where it can be useful to have a cpus_read_trylock()
function to work around circular lock dependency problem involving
the cpu_hotplug_lock.
Signed-off-by: Waiman Long <longman@redhat.com>
---
include/linux/cpu.h | 2 ++
kernel/cpu.c | 6 ++++++
2 files changed, 8 insertions(+)
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index a97a63e..e850bfe 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -103,6 +103,7 @@ static inline void cpu_maps_update_done(void)
extern void cpus_write_unlock(void);
extern void cpus_read_lock(void);
extern void cpus_read_unlock(void);
+extern int cpus_read_trylock(void);
extern void lockdep_assert_cpus_held(void);
extern void cpu_hotplug_disable(void);
extern void cpu_hotplug_enable(void);
@@ -115,6 +116,7 @@ static inline void cpus_write_lock(void) { }
static inline void cpus_write_unlock(void) { }
static inline void cpus_read_lock(void) { }
static inline void cpus_read_unlock(void) { }
+static inline int cpus_read_trylock(void) { return true; }
static inline void lockdep_assert_cpus_held(void) { }
static inline void cpu_hotplug_disable(void) { }
static inline void cpu_hotplug_enable(void) { }
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 0db8938..307486b 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -290,6 +290,12 @@ void cpus_read_lock(void)
}
EXPORT_SYMBOL_GPL(cpus_read_lock);
+int cpus_read_trylock(void)
+{
+ return percpu_down_read_trylock(&cpu_hotplug_lock);
+}
+EXPORT_SYMBOL_GPL(cpus_read_trylock);
+
void cpus_read_unlock(void)
{
percpu_up_read(&cpu_hotplug_lock);
--
1.8.3.1
^ permalink raw reply related
* [PATCH 0/2] cpufreq: Fix a circular lock dependency problem
From: Waiman Long @ 2018-07-23 17:49 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar, Thomas Gleixner, Peter Zijlstra,
Ingo Molnar
Cc: linux-kernel, linux-pm, Paul E. McKenney, Greg Kroah-Hartman,
Konrad Rzeszutek Wilk, Waiman Long
This patchset works around a circular lock dependency issue in the
cpufreq driver reported by lockdep. The two locks involved are the
cpu_hotplup_lock and the reference count of a sysfs file.
The cpufreq_register_driver() function uses the lock sequence:
cpus_read_lock --> kn->count
Whereas the cpufreq sysfs store method uses the sequence:
kn->count --> cpus_read_lock
This is not really an issue as a shared lock is used on the
cpu_hotplup_lock. However, the lockdep code isn't able to handle
shared locking. So one way to work around this is to define a
cpus_read_trylock() function and uses it in the store method instead.
Waiman Long (2):
cpu/hotplug: Add a cpus_read_trylock() function
cpufreq: Fix a circular lock dependency problem
drivers/cpufreq/cpufreq.c | 16 +++++++++++++++-
include/linux/cpu.h | 2 ++
kernel/cpu.c | 6 ++++++
3 files changed, 23 insertions(+), 1 deletion(-)
--
1.8.3.1
^ permalink raw reply
* Re: [PATCH net-next] selftests: forwarding: gre_multipath: Drop IPv6 tests
From: David Miller @ 2018-07-23 16:47 UTC (permalink / raw)
To: petrm; +Cc: netdev, linux-kselftest, shuah, idosch, dsahern
In-Reply-To: <cf0e380baf6a6aa5cf384017e8d69fec79750cfe.1532341813.git.petrm@mellanox.com>
From: Petr Machata <petrm@mellanox.com>
Date: Mon, 23 Jul 2018 12:33:08 +0200
> Support for device-only IPv6 multipath next hops was dropped in
> commit 33bd5ac54dc4 ("net/ipv6: Revert attempt to simplify route replace
> and append") and as of commit b5d2d75e079a ("net/ipv6: Do not allow
> device only routes via the multipath API"), attempts to add a next hop
> like that yield an explicit diagnostic.
>
> Correspondingly, drop the IPv6 parts of GRE multipath test that are
> supposed to test that code.
>
> Signed-off-by: Petr Machata <petrm@mellanox.com>
Applied, thank you.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.