Re: [PATCH v2] mmc: sdhci-esdhc-imx: allow 1.8V modes without 100/200MHz pinctrl states
On 05.07.2018 15:10, Ulf Hansson wrote:
> On 4 July 2018 at 17:07, Stefan Agner wrote:
>> If pinctrl nodes for 100/200MHz are missing, the controller should
>> not select any mode which need signal frequencies 100MHz or higher.
>> To prevent such speed modes the driver currently uses the quirk flag
>> SDHCI_QUIRK2_NO_1_8_V. This works nicely for SD cards since 1.8V
>> signaling is required for all faster modes and slower modes use 3.3V
>> signaling only.
>>
>> However, there are eMMC modes which use 1.8V signaling and run below
>> 100MHz, e.g. DDR52 at 1.8V. With using SDHCI_QUIRK2_NO_1_8_V this
>> mode is prevented. When using a fixed 1.8V regulator as vqmmc-supply
>> the stack has no valid mode to use. In this tenuous situation the
>> kernel continuously prints voltage switching errors:
>> mmc1: Switching to 3.3V signalling voltage failed
>>
>> Avoid using SDHCI_QUIRK2_NO_1_8_V and prevent faster modes by
>> altering the SDHCI capability register. With that the stack is able
>> to select 1.8V modes even if no faster pinctrl states are available:
>> # cat /sys/kernel/debug/mmc1/ios
>> ...
>> timing spec:8 (mmc DDR52)
>> signal voltage: 1 (1.80 V)
>> ...
>>
>> Link: http://lkml.kernel.org/r/20180628081331.13051-1-ste...@agner.ch
>> Signed-off-by: Stefan Agner
>
> Thanks, applied for next! Let's see if this turns out okay, then let's
> make it a fix and add a stable tag.
>
> BTW, would you mind looking up the commit it fixes? Or if there is a
> certain stable release we should target.
>
The quirk SDHCI_QUIRK2_NO_1_8_V has been used if pinctrl were missing
since support has been added for additional pinctrl states (back around
3.13).
Fixes: ad93220de7da ("mmc: sdhci-esdhc-imx: change pinctrl state
according to uhs mode")
I guess it won't apply on older kernels since the code which applies the
quirk has been moved around.
--
Stefan
> Kind regards
> Uffe
>
>> ---
>> drivers/mmc/host/sdhci-esdhc-imx.c | 21 +
>> 1 file changed, 9 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
>> b/drivers/mmc/host/sdhci-esdhc-imx.c
>> index 20a420b765b3..e96d969ab2c3 100644
>> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
>> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
>> @@ -312,6 +312,15 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int
>> reg)
>>
>> if (imx_data->socdata->flags & ESDHC_FLAG_HS400)
>> val |= SDHCI_SUPPORT_HS400;
>> +
>> + /*
>> +* Do not advertise faster UHS modes if there are no
>> +* pinctrl states for 100MHz/200MHz.
>> +*/
>> + if (IS_ERR_OR_NULL(imx_data->pins_100mhz) ||
>> + IS_ERR_OR_NULL(imx_data->pins_200mhz))
>> + val &= ~(SDHCI_SUPPORT_SDR50 |
>> SDHCI_SUPPORT_DDR50
>> +| SDHCI_SUPPORT_SDR104 |
>> SDHCI_SUPPORT_HS400);
>> }
>> }
>>
>> @@ -1157,18 +1166,6 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
>> ESDHC_PINCTRL_STATE_100MHZ);
>> imx_data->pins_200mhz =
>> pinctrl_lookup_state(imx_data->pinctrl,
>> ESDHC_PINCTRL_STATE_200MHZ);
>> - if (IS_ERR(imx_data->pins_100mhz) ||
>> - IS_ERR(imx_data->pins_200mhz)) {
>> - dev_warn(mmc_dev(host->mmc),
>> - "could not get ultra high speed state, work
>> on normal mode\n");
>> - /*
>> -* fall back to not supporting uhs by specifying no
>> -* 1.8v quirk
>> -*/
>> - host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
>> - }
>> - } else {
>> - host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
>> }
>>
>> /* call to generic mmc_of_parse to support additional capabilities */
>> --
>> 2.18.0
>>
Re: [PATCH v2] mmc: sdhci-esdhc-imx: allow 1.8V modes without 100/200MHz pinctrl states
On 05.07.2018 15:10, Ulf Hansson wrote:
> On 4 July 2018 at 17:07, Stefan Agner wrote:
>> If pinctrl nodes for 100/200MHz are missing, the controller should
>> not select any mode which need signal frequencies 100MHz or higher.
>> To prevent such speed modes the driver currently uses the quirk flag
>> SDHCI_QUIRK2_NO_1_8_V. This works nicely for SD cards since 1.8V
>> signaling is required for all faster modes and slower modes use 3.3V
>> signaling only.
>>
>> However, there are eMMC modes which use 1.8V signaling and run below
>> 100MHz, e.g. DDR52 at 1.8V. With using SDHCI_QUIRK2_NO_1_8_V this
>> mode is prevented. When using a fixed 1.8V regulator as vqmmc-supply
>> the stack has no valid mode to use. In this tenuous situation the
>> kernel continuously prints voltage switching errors:
>> mmc1: Switching to 3.3V signalling voltage failed
>>
>> Avoid using SDHCI_QUIRK2_NO_1_8_V and prevent faster modes by
>> altering the SDHCI capability register. With that the stack is able
>> to select 1.8V modes even if no faster pinctrl states are available:
>> # cat /sys/kernel/debug/mmc1/ios
>> ...
>> timing spec:8 (mmc DDR52)
>> signal voltage: 1 (1.80 V)
>> ...
>>
>> Link: http://lkml.kernel.org/r/20180628081331.13051-1-ste...@agner.ch
>> Signed-off-by: Stefan Agner
>
> Thanks, applied for next! Let's see if this turns out okay, then let's
> make it a fix and add a stable tag.
>
> BTW, would you mind looking up the commit it fixes? Or if there is a
> certain stable release we should target.
>
The quirk SDHCI_QUIRK2_NO_1_8_V has been used if pinctrl were missing
since support has been added for additional pinctrl states (back around
3.13).
Fixes: ad93220de7da ("mmc: sdhci-esdhc-imx: change pinctrl state
according to uhs mode")
I guess it won't apply on older kernels since the code which applies the
quirk has been moved around.
--
Stefan
> Kind regards
> Uffe
>
>> ---
>> drivers/mmc/host/sdhci-esdhc-imx.c | 21 +
>> 1 file changed, 9 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
>> b/drivers/mmc/host/sdhci-esdhc-imx.c
>> index 20a420b765b3..e96d969ab2c3 100644
>> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
>> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
>> @@ -312,6 +312,15 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int
>> reg)
>>
>> if (imx_data->socdata->flags & ESDHC_FLAG_HS400)
>> val |= SDHCI_SUPPORT_HS400;
>> +
>> + /*
>> +* Do not advertise faster UHS modes if there are no
>> +* pinctrl states for 100MHz/200MHz.
>> +*/
>> + if (IS_ERR_OR_NULL(imx_data->pins_100mhz) ||
>> + IS_ERR_OR_NULL(imx_data->pins_200mhz))
>> + val &= ~(SDHCI_SUPPORT_SDR50 |
>> SDHCI_SUPPORT_DDR50
>> +| SDHCI_SUPPORT_SDR104 |
>> SDHCI_SUPPORT_HS400);
>> }
>> }
>>
>> @@ -1157,18 +1166,6 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
>> ESDHC_PINCTRL_STATE_100MHZ);
>> imx_data->pins_200mhz =
>> pinctrl_lookup_state(imx_data->pinctrl,
>> ESDHC_PINCTRL_STATE_200MHZ);
>> - if (IS_ERR(imx_data->pins_100mhz) ||
>> - IS_ERR(imx_data->pins_200mhz)) {
>> - dev_warn(mmc_dev(host->mmc),
>> - "could not get ultra high speed state, work
>> on normal mode\n");
>> - /*
>> -* fall back to not supporting uhs by specifying no
>> -* 1.8v quirk
>> -*/
>> - host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
>> - }
>> - } else {
>> - host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
>> }
>>
>> /* call to generic mmc_of_parse to support additional capabilities */
>> --
>> 2.18.0
>>
[PATCH v4 5/6] ARM: dts: Add generic interconnect target module node for MCAN
The ti-sysc driver provides support for manipulating the idle modes
and interconnect level resets.
Add the generic interconnect target module node for MCAN to support
the same.
CC: Tony Lindgren
Signed-off-by: Faiz Abbas
---
arch/arm/boot/dts/dra76x.dtsi | 18 ++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/boot/dts/dra76x.dtsi b/arch/arm/boot/dts/dra76x.dtsi
index bfc82636999c..5157cc431574 100644
--- a/arch/arm/boot/dts/dra76x.dtsi
+++ b/arch/arm/boot/dts/dra76x.dtsi
@@ -11,6 +11,24 @@
/ {
compatible = "ti,dra762", "ti,dra7";
+ ocp {
+ target-module@42c01900 {
+ compatible = "ti,sysc-dra7-mcan", "ti,sysc";
+ ranges = <0x0 0x42c0 0x2000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = <0x42c01900 0x4>,
+ <0x42c01904 0x4>,
+ <0x42c01908 0x4>;
+ reg-names = "rev", "sysc", "syss";
+ ti,sysc-mask = <(SYSC_OMAP4_SOFTRESET |
+SYSC_DRA7_MCAN_ENAWAKEUP)>;
+ ti,syss-mask = <1>;
+ clocks = <_clkctrl DRA7_ADC_CLKCTRL 0>;
+ clock-names = "fck";
+ };
+ };
+
};
/* MCAN interrupts are hard-wired to irqs 67, 68 */
--
2.17.0
[PATCH v4 5/6] ARM: dts: Add generic interconnect target module node for MCAN
The ti-sysc driver provides support for manipulating the idle modes
and interconnect level resets.
Add the generic interconnect target module node for MCAN to support
the same.
CC: Tony Lindgren
Signed-off-by: Faiz Abbas
---
arch/arm/boot/dts/dra76x.dtsi | 18 ++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/boot/dts/dra76x.dtsi b/arch/arm/boot/dts/dra76x.dtsi
index bfc82636999c..5157cc431574 100644
--- a/arch/arm/boot/dts/dra76x.dtsi
+++ b/arch/arm/boot/dts/dra76x.dtsi
@@ -11,6 +11,24 @@
/ {
compatible = "ti,dra762", "ti,dra7";
+ ocp {
+ target-module@42c01900 {
+ compatible = "ti,sysc-dra7-mcan", "ti,sysc";
+ ranges = <0x0 0x42c0 0x2000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = <0x42c01900 0x4>,
+ <0x42c01904 0x4>,
+ <0x42c01908 0x4>;
+ reg-names = "rev", "sysc", "syss";
+ ti,sysc-mask = <(SYSC_OMAP4_SOFTRESET |
+SYSC_DRA7_MCAN_ENAWAKEUP)>;
+ ti,syss-mask = <1>;
+ clocks = <_clkctrl DRA7_ADC_CLKCTRL 0>;
+ clock-names = "fck";
+ };
+ };
+
};
/* MCAN interrupts are hard-wired to irqs 67, 68 */
--
2.17.0
[PATCH v4 4/6] bus: ti-sysc: Add support for software reset
Add support for the software reset of a target interconnect
module using its sysconfig and sysstatus registers.
Signed-off-by: Faiz Abbas
---
drivers/bus/ti-sysc.c | 39 +++
1 file changed, 39 insertions(+)
diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
index ad1cd6888757..ac65a4f336d5 100644
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -23,11 +23,14 @@
#include
#include
#include
+#include
#include
#include
+#define MAX_MODULE_SOFTRESET_WAIT 1
+
static const char * const reg_names[] = { "rev", "sysc", "syss", };
enum sysc_clocks {
@@ -88,6 +91,11 @@ struct sysc {
struct delayed_work idle_work;
};
+void sysc_write(struct sysc *ddata, int offset, u32 value)
+{
+ writel_relaxed(value, ddata->module_va + offset);
+}
+
static u32 sysc_read(struct sysc *ddata, int offset)
{
if (ddata->cfg.quirks & SYSC_QUIRK_16BIT) {
@@ -943,6 +951,26 @@ static void sysc_init_revision_quirks(struct sysc *ddata)
}
}
+static int sysc_reset(struct sysc *ddata)
+{
+ int offset = ddata->offsets[SYSC_SYSCONFIG];
+ int val = sysc_read(ddata, offset);
+
+ val |= (0x1 << ddata->cap->regbits->srst_shift);
+ sysc_write(ddata, offset, val);
+
+ /* Poll on reset status */
+ if (ddata->cfg.syss_mask) {
+ offset = ddata->offsets[SYSC_SYSSTATUS];
+
+ return readl_poll_timeout(ddata->module_va + offset, val,
+ (val & ddata->cfg.syss_mask) == 0x0,
+ 100, MAX_MODULE_SOFTRESET_WAIT);
+ }
+
+ return 0;
+}
+
/* At this point the module is configured enough to read the revision */
static int sysc_init_module(struct sysc *ddata)
{
@@ -960,6 +988,17 @@ static int sysc_init_module(struct sysc *ddata)
return 0;
}
+ if (!(ddata->cfg.quirks & SYSC_QUIRK_NO_RESET_ON_INIT) &&
+ !ddata->legacy_mode) {
+ error = sysc_reset(ddata);
+ if (error) {
+ dev_err(ddata->dev, "Reset failed with %d\n", error);
+ pm_runtime_put_sync(ddata->dev);
+
+ return error;
+ }
+ }
+
ddata->revision = sysc_read_revision(ddata);
pm_runtime_put_sync(ddata->dev);
--
2.17.0
[PATCH v4 4/6] bus: ti-sysc: Add support for software reset
Add support for the software reset of a target interconnect
module using its sysconfig and sysstatus registers.
Signed-off-by: Faiz Abbas
---
drivers/bus/ti-sysc.c | 39 +++
1 file changed, 39 insertions(+)
diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
index ad1cd6888757..ac65a4f336d5 100644
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -23,11 +23,14 @@
#include
#include
#include
+#include
#include
#include
+#define MAX_MODULE_SOFTRESET_WAIT 1
+
static const char * const reg_names[] = { "rev", "sysc", "syss", };
enum sysc_clocks {
@@ -88,6 +91,11 @@ struct sysc {
struct delayed_work idle_work;
};
+void sysc_write(struct sysc *ddata, int offset, u32 value)
+{
+ writel_relaxed(value, ddata->module_va + offset);
+}
+
static u32 sysc_read(struct sysc *ddata, int offset)
{
if (ddata->cfg.quirks & SYSC_QUIRK_16BIT) {
@@ -943,6 +951,26 @@ static void sysc_init_revision_quirks(struct sysc *ddata)
}
}
+static int sysc_reset(struct sysc *ddata)
+{
+ int offset = ddata->offsets[SYSC_SYSCONFIG];
+ int val = sysc_read(ddata, offset);
+
+ val |= (0x1 << ddata->cap->regbits->srst_shift);
+ sysc_write(ddata, offset, val);
+
+ /* Poll on reset status */
+ if (ddata->cfg.syss_mask) {
+ offset = ddata->offsets[SYSC_SYSSTATUS];
+
+ return readl_poll_timeout(ddata->module_va + offset, val,
+ (val & ddata->cfg.syss_mask) == 0x0,
+ 100, MAX_MODULE_SOFTRESET_WAIT);
+ }
+
+ return 0;
+}
+
/* At this point the module is configured enough to read the revision */
static int sysc_init_module(struct sysc *ddata)
{
@@ -960,6 +988,17 @@ static int sysc_init_module(struct sysc *ddata)
return 0;
}
+ if (!(ddata->cfg.quirks & SYSC_QUIRK_NO_RESET_ON_INIT) &&
+ !ddata->legacy_mode) {
+ error = sysc_reset(ddata);
+ if (error) {
+ dev_err(ddata->dev, "Reset failed with %d\n", error);
+ pm_runtime_put_sync(ddata->dev);
+
+ return error;
+ }
+ }
+
ddata->revision = sysc_read_revision(ddata);
pm_runtime_put_sync(ddata->dev);
--
2.17.0
[PATCH v4 2/6] clk: ti: dra7: Add clkctrl clock data for the mcan clocks
Add clkctrl data for the m_can clocks and register it within the
clkctrl driver
Acked-by: Rob Herring
CC: Tero Kristo
Signed-off-by: Faiz Abbas
---
drivers/clk/ti/clk-7xx.c | 1 +
include/dt-bindings/clock/dra7.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c
index fb249a1637a5..71a122b2dc67 100644
--- a/drivers/clk/ti/clk-7xx.c
+++ b/drivers/clk/ti/clk-7xx.c
@@ -708,6 +708,7 @@ static const struct omap_clkctrl_reg_data
dra7_wkupaon_clkctrl_regs[] __initcons
{ DRA7_COUNTER_32K_CLKCTRL, NULL, 0, "wkupaon_iclk_mux" },
{ DRA7_UART10_CLKCTRL, dra7_uart10_bit_data, CLKF_SW_SUP,
"wkupaon_cm:clk:0060:24" },
{ DRA7_DCAN1_CLKCTRL, dra7_dcan1_bit_data, CLKF_SW_SUP,
"wkupaon_cm:clk:0068:24" },
+ { DRA7_ADC_CLKCTRL, NULL, CLKF_SW_SUP, "mcan_clk"},
{ 0 },
};
diff --git a/include/dt-bindings/clock/dra7.h b/include/dt-bindings/clock/dra7.h
index 5e1061b15aed..d7549c57cac3 100644
--- a/include/dt-bindings/clock/dra7.h
+++ b/include/dt-bindings/clock/dra7.h
@@ -168,5 +168,6 @@
#define DRA7_COUNTER_32K_CLKCTRL DRA7_CLKCTRL_INDEX(0x50)
#define DRA7_UART10_CLKCTRLDRA7_CLKCTRL_INDEX(0x80)
#define DRA7_DCAN1_CLKCTRL DRA7_CLKCTRL_INDEX(0x88)
+#define DRA7_ADC_CLKCTRL DRA7_CLKCTRL_INDEX(0xa0)
#endif
--
2.17.0
[PATCH v4 6/6] ARM: dts: dra76x: Add MCAN node
From: Franklin S Cooper Jr
Add support for the MCAN peripheral which supports both classic
CAN messages along with the new CAN-FD message.
Add MCAN node to evm and enable it with a maximum datarate of 5 mbps
Signed-off-by: Faiz Abbas
---
arch/arm/boot/dts/dra76-evm.dts | 6 ++
arch/arm/boot/dts/dra76x.dtsi | 13 +
2 files changed, 19 insertions(+)
diff --git a/arch/arm/boot/dts/dra76-evm.dts b/arch/arm/boot/dts/dra76-evm.dts
index c07f0051844d..0ac533784b11 100644
--- a/arch/arm/boot/dts/dra76-evm.dts
+++ b/arch/arm/boot/dts/dra76-evm.dts
@@ -436,3 +436,9 @@
phys = <_phy>, <_phy>;
phy-names = "pcie-phy0", "pcie-phy1";
};
+
+_can0 {
+ can-transceiver {
+ max-bitrate = <500>;
+ };
+};
diff --git a/arch/arm/boot/dts/dra76x.dtsi b/arch/arm/boot/dts/dra76x.dtsi
index 5157cc431574..613e4dc0ed3e 100644
--- a/arch/arm/boot/dts/dra76x.dtsi
+++ b/arch/arm/boot/dts/dra76x.dtsi
@@ -26,6 +26,19 @@
ti,syss-mask = <1>;
clocks = <_clkctrl DRA7_ADC_CLKCTRL 0>;
clock-names = "fck";
+
+ m_can0: mcan@1a00 {
+ compatible = "bosch,m_can";
+ reg = <0x1a00 0x4000>, <0x0 0x18FC>;
+ reg-names = "m_can", "message_ram";
+ interrupt-parent = <>;
+ interrupts = ,
+;
+ interrupt-names = "int0", "int1";
+ clocks = <_clk>, <_iclk_div>;
+ clock-names = "cclk", "hclk";
+ bosch,mram-cfg = <0x0 0 0 32 0 0 1 1>;
+ };
};
};
--
2.17.0
[PATCH v4 2/6] clk: ti: dra7: Add clkctrl clock data for the mcan clocks
Add clkctrl data for the m_can clocks and register it within the
clkctrl driver
Acked-by: Rob Herring
CC: Tero Kristo
Signed-off-by: Faiz Abbas
---
drivers/clk/ti/clk-7xx.c | 1 +
include/dt-bindings/clock/dra7.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c
index fb249a1637a5..71a122b2dc67 100644
--- a/drivers/clk/ti/clk-7xx.c
+++ b/drivers/clk/ti/clk-7xx.c
@@ -708,6 +708,7 @@ static const struct omap_clkctrl_reg_data
dra7_wkupaon_clkctrl_regs[] __initcons
{ DRA7_COUNTER_32K_CLKCTRL, NULL, 0, "wkupaon_iclk_mux" },
{ DRA7_UART10_CLKCTRL, dra7_uart10_bit_data, CLKF_SW_SUP,
"wkupaon_cm:clk:0060:24" },
{ DRA7_DCAN1_CLKCTRL, dra7_dcan1_bit_data, CLKF_SW_SUP,
"wkupaon_cm:clk:0068:24" },
+ { DRA7_ADC_CLKCTRL, NULL, CLKF_SW_SUP, "mcan_clk"},
{ 0 },
};
diff --git a/include/dt-bindings/clock/dra7.h b/include/dt-bindings/clock/dra7.h
index 5e1061b15aed..d7549c57cac3 100644
--- a/include/dt-bindings/clock/dra7.h
+++ b/include/dt-bindings/clock/dra7.h
@@ -168,5 +168,6 @@
#define DRA7_COUNTER_32K_CLKCTRL DRA7_CLKCTRL_INDEX(0x50)
#define DRA7_UART10_CLKCTRLDRA7_CLKCTRL_INDEX(0x80)
#define DRA7_DCAN1_CLKCTRL DRA7_CLKCTRL_INDEX(0x88)
+#define DRA7_ADC_CLKCTRL DRA7_CLKCTRL_INDEX(0xa0)
#endif
--
2.17.0
[PATCH v4 6/6] ARM: dts: dra76x: Add MCAN node
From: Franklin S Cooper Jr
Add support for the MCAN peripheral which supports both classic
CAN messages along with the new CAN-FD message.
Add MCAN node to evm and enable it with a maximum datarate of 5 mbps
Signed-off-by: Faiz Abbas
---
arch/arm/boot/dts/dra76-evm.dts | 6 ++
arch/arm/boot/dts/dra76x.dtsi | 13 +
2 files changed, 19 insertions(+)
diff --git a/arch/arm/boot/dts/dra76-evm.dts b/arch/arm/boot/dts/dra76-evm.dts
index c07f0051844d..0ac533784b11 100644
--- a/arch/arm/boot/dts/dra76-evm.dts
+++ b/arch/arm/boot/dts/dra76-evm.dts
@@ -436,3 +436,9 @@
phys = <_phy>, <_phy>;
phy-names = "pcie-phy0", "pcie-phy1";
};
+
+_can0 {
+ can-transceiver {
+ max-bitrate = <500>;
+ };
+};
diff --git a/arch/arm/boot/dts/dra76x.dtsi b/arch/arm/boot/dts/dra76x.dtsi
index 5157cc431574..613e4dc0ed3e 100644
--- a/arch/arm/boot/dts/dra76x.dtsi
+++ b/arch/arm/boot/dts/dra76x.dtsi
@@ -26,6 +26,19 @@
ti,syss-mask = <1>;
clocks = <_clkctrl DRA7_ADC_CLKCTRL 0>;
clock-names = "fck";
+
+ m_can0: mcan@1a00 {
+ compatible = "bosch,m_can";
+ reg = <0x1a00 0x4000>, <0x0 0x18FC>;
+ reg-names = "m_can", "message_ram";
+ interrupt-parent = <>;
+ interrupts = ,
+;
+ interrupt-names = "int0", "int1";
+ clocks = <_clk>, <_iclk_div>;
+ clock-names = "cclk", "hclk";
+ bosch,mram-cfg = <0x0 0 0 32 0 0 1 1>;
+ };
};
};
--
2.17.0
[PATCH v4 3/6] bus: ti-sysc: Add support for using ti-sysc for MCAN on dra76x
The dra76x MCAN generic interconnect module has a its own
format for the bits in the control registers.
Therefore add a new module type, new regbits and new capabilities
specific to the MCAN module.
Acked-by: Rob Herring
CC: Tony Lindgren
Signed-off-by: Faiz Abbas
---
.../devicetree/bindings/bus/ti-sysc.txt| 1 +
drivers/bus/ti-sysc.c | 18 ++
include/dt-bindings/bus/ti-sysc.h | 2 ++
include/linux/platform_data/ti-sysc.h | 1 +
4 files changed, 22 insertions(+)
diff --git a/Documentation/devicetree/bindings/bus/ti-sysc.txt
b/Documentation/devicetree/bindings/bus/ti-sysc.txt
index d8ed5b780ed9..91dc2333af01 100644
--- a/Documentation/devicetree/bindings/bus/ti-sysc.txt
+++ b/Documentation/devicetree/bindings/bus/ti-sysc.txt
@@ -36,6 +36,7 @@ Required standard properties:
"ti,sysc-omap-aes"
"ti,sysc-mcasp"
"ti,sysc-usb-host-fs"
+ "ti,sysc-dra7-mcan"
- reg shall have register areas implemented for the interconnect
target module in question such as revision, sysc and syss
diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
index 1cc29629d238..ad1cd6888757 100644
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -1552,6 +1552,23 @@ static const struct sysc_capabilities
sysc_omap4_usb_host_fs = {
.regbits = _regbits_omap4_usb_host_fs,
};
+static const struct sysc_regbits sysc_regbits_dra7_mcan = {
+ .dmadisable_shift = -ENODEV,
+ .midle_shift = -ENODEV,
+ .sidle_shift = -ENODEV,
+ .clkact_shift = -ENODEV,
+ .enwkup_shift = 4,
+ .srst_shift = 0,
+ .emufree_shift = -ENODEV,
+ .autoidle_shift = -ENODEV,
+};
+
+static const struct sysc_capabilities sysc_dra7_mcan = {
+ .type = TI_SYSC_DRA7_MCAN,
+ .sysc_mask = SYSC_DRA7_MCAN_ENAWAKEUP | SYSC_OMAP4_SOFTRESET,
+ .regbits = _regbits_dra7_mcan,
+};
+
static int sysc_init_pdata(struct sysc *ddata)
{
struct ti_sysc_platform_data *pdata = dev_get_platdata(ddata->dev);
@@ -1743,6 +1760,7 @@ static const struct of_device_id sysc_match[] = {
{ .compatible = "ti,sysc-mcasp", .data = _omap4_mcasp, },
{ .compatible = "ti,sysc-usb-host-fs",
.data = _omap4_usb_host_fs, },
+ { .compatible = "ti,sysc-dra7-mcan", .data = _dra7_mcan, },
{ },
};
MODULE_DEVICE_TABLE(of, sysc_match);
diff --git a/include/dt-bindings/bus/ti-sysc.h
b/include/dt-bindings/bus/ti-sysc.h
index 2c005376ac0e..7138384e2ef9 100644
--- a/include/dt-bindings/bus/ti-sysc.h
+++ b/include/dt-bindings/bus/ti-sysc.h
@@ -15,6 +15,8 @@
/* SmartReflex sysc found on 36xx and later */
#define SYSC_OMAP3_SR_ENAWAKEUP(1 << 26)
+#define SYSC_DRA7_MCAN_ENAWAKEUP (1 << 4)
+
/* SYSCONFIG STANDBYMODE/MIDLEMODE/SIDLEMODE supported by hardware */
#define SYSC_IDLE_FORCE0
#define SYSC_IDLE_NO 1
diff --git a/include/linux/platform_data/ti-sysc.h
b/include/linux/platform_data/ti-sysc.h
index 990aad477458..2efa3470a451 100644
--- a/include/linux/platform_data/ti-sysc.h
+++ b/include/linux/platform_data/ti-sysc.h
@@ -14,6 +14,7 @@ enum ti_sysc_module_type {
TI_SYSC_OMAP4_SR,
TI_SYSC_OMAP4_MCASP,
TI_SYSC_OMAP4_USB_HOST_FS,
+ TI_SYSC_DRA7_MCAN,
};
struct ti_sysc_cookie {
--
2.17.0
Re: [PATCH 2/2] tools/memory-model: Add write ordering by release-acquire and by locks
On Wed, 4 Jul 2018, Will Deacon wrote:
> Hi Alan,
>
> On Tue, Jul 03, 2018 at 01:28:17PM -0400, Alan Stern wrote:
> > On Mon, 25 Jun 2018, Andrea Parri wrote:
> >
> > > On Fri, Jun 22, 2018 at 07:30:08PM +0100, Will Deacon wrote:
> > > > > > I think the second example would preclude us using LDAPR for
> > > > > > load-acquire,
> > >
> > > > I don't think it's a moot point. We want new architectures to implement
> > > > acquire/release efficiently, and it's not unlikely that they will have
> > > > acquire loads that are similar in semantics to LDAPR. This patch
> > > > prevents
> > > > them from doing so,
> > >
> > > By this same argument, you should not be a "big fan" of rfi-rel-acq in
> > > ppo ;)
> > > consider, e.g., the two litmus tests below: what am I missing?
> >
> > This is an excellent point, which seems to have gotten lost in the
> > shuffle. I'd like to see your comments.
>
> Yeah, sorry. Loads going on at the moment. You could ask herd instead of me
> though ;)
Indeed; and the answer was as expected. Sometimes one gains additional
insights by asking a person, though.
> > In essence, if you're using release-acquire instructions that only
> > provide RCpc consistency, does store-release followed by load-acquire
> > of the same address provide read-read ordering? In theory it doesn't
> > have to, because if the value from the store-release is forwarded to
> > the load-acquire then:
> >
> > LOAD A
> > STORE-RELEASE X, v
> > LOAD-ACQUIRE X
> > LOAD B
> >
> > could be executed by the CPU in the order:
> >
> > LOAD-ACQUIRE X
> > LOAD B
> > LOAD A
> > STORE-RELEASE X, v
> >
> > thereby accessing A and B out of program order without violating the
> > requirements on the release or the acquire.
> >
> > Of course PPC doesn't allow this, but should we rule it out entirely?
>
> This would be allowed if LOAD-ACQUIRE was implemented using LDAPR on Arm.
> I don't think we should be ruling out architectures using RCpc
> acquire/release primitives, because doing so just feels like an artifact of
> most architectures building these out of fences today.
>
> It's funny really, because from an Arm-perspective I don't plan to stray
> outside of RCsc, but I feel like other weak architectures aren't being
> well represented here. If we just care about x86, Arm and Power (and assume
> that Power doesn't plan to implement RCpc acquire/release instructions)
> then we're good to tighten things up. But I fear that RISC-V should probably
> be more engaged (adding Daniel) and who knows about MIPS or these other
> random architectures popping up on linux-arch.
I don't object to having weak versions of acquire/release in the LKMM.
Perhaps the stronger versions could be kept in the hardware model
(which has not been published and is not in the kernel source), but
even that might be a bad idea in view of what RISC-V is liable to do.
> > > C MP+fencewmbonceonce+pooncerelease-rfireleaseacquire-poacquireonce
> > >
> > > {}
> > >
> > > P0(int *x, int *y)
> > > {
> > > WRITE_ONCE(*x, 1);
> > > smp_wmb();
> > > WRITE_ONCE(*y, 1);
> > > }
> > >
> > > P1(int *x, int *y, int *z)
> > > {
> > > r0 = READ_ONCE(*y);
> > > smp_store_release(z, 1);
> > > r1 = smp_load_acquire(z);
> > > r2 = READ_ONCE(*x);
> > > }
> > >
> > > exists (1:r0=1 /\ 1:r1=1 /\ 1:r2=0)
> > >
> > >
> > > AArch64 MP+dmb.st+popl-rfilq-poqp
> > > "DMB.STdWW Rfe PodRWPL RfiLQ PodRRQP Fre"
> > > Generator=diyone7 (version 7.49+02(dev))
> > > Prefetch=0:x=F,0:y=W,1:y=F,1:x=T
> > > Com=Rf Fr
> > > Orig=DMB.STdWW Rfe PodRWPL RfiLQ PodRRQP Fre
> > > {
> > > 0:X1=x; 0:X3=y;
> > > 1:X1=y; 1:X3=z; 1:X6=x;
> > > }
> > > P0 | P1;
> > > MOV W0,#1 | LDR W0,[X1] ;
> > > STR W0,[X1] | MOV W2,#1 ;
> > > DMB ST | STLR W2,[X3] ;
> > > MOV W2,#1 | LDAPR W4,[X3] ;
> > > STR W2,[X3] | LDR W5,[X6] ;
> > > exists
> > > (1:X0=1 /\ 1:X4=1 /\ 1:X5=0)
>
> (you can also run this yourself, since 'Q' is supported in the .cat file
> I contributed to herdtools7)
>
> Test MP+dmb.sy+popl-rfilq-poqp Allowed
> States 4
> 1:X0=0; 1:X4=1; 1:X5=0;
> 1:X0=0; 1:X4=1; 1:X5=1;
> 1:X0=1; 1:X4=1; 1:X5=0;
> 1:X0=1; 1:X4=1; 1:X5=1;
> Ok
> Witnesses
> Positive: 1 Negative: 3
> Condition exists (1:X0=1 /\ 1:X4=1 /\ 1:X5=0)
> Observation MP+dmb.sy+popl-rfilq-poqp Sometimes 1 3
> Time MP+dmb.sy+popl-rfilq-poqp 0.01
> Hash=61858b7b59a6310d869f99cd05718f96
>
> > There's also read-write ordering, in the form of the LB pattern:
> >
> > P0(int *x, int *y, int *z)
> > {
> > r0 = READ_ONCE(*x);
> > smp_store_release(z, 1);
> > r1 = smp_load_acquire(z);
> > WRITE_ONCE(*y, 1);
> > }
> >
> > P1(int *x, int *y)
> > {
> > r2 = READ_ONCE(*y);
> > smp_mp();
> > WRITE_ONCE(*x, 1);
> > }
> >
> > exists (0:r0=1 /\ 1:r2=1)
>
> The access types are irrelevant to the acquire/release primitives, so yes
> that's also allowed.
>
> > Would this be allowed if smp_load_acquire() was
[PATCH v4 1/6] ARM: dts: dra762: Add MCAN clock support
From: Lokesh Vutla
MCAN is clocked by H14 divider of DPLL_GMAC. Unlike other
DPLL dividers this DPLL_GMAC H14 divider is controlled by
control module. Adding support for these clocks.
Signed-off-by: Lokesh Vutla
Signed-off-by: Faiz Abbas
---
arch/arm/boot/dts/dra76x.dtsi | 33 +
1 file changed, 33 insertions(+)
diff --git a/arch/arm/boot/dts/dra76x.dtsi b/arch/arm/boot/dts/dra76x.dtsi
index 1c88c581ff18..bfc82636999c 100644
--- a/arch/arm/boot/dts/dra76x.dtsi
+++ b/arch/arm/boot/dts/dra76x.dtsi
@@ -17,3 +17,36 @@
_mpu {
ti,irqs-skip = <10 67 68 133 139 140>;
};
+
+_conf_clocks {
+ dpll_gmac_h14x2_ctrl_ck: dpll_gmac_h14x2_ctrl_ck@3fc {
+ #clock-cells = <0>;
+ compatible = "ti,divider-clock";
+ clocks = <_gmac_x2_ck>;
+ ti,max-div = <63>;
+ reg = <0x03fc>;
+ ti,bit-shift=<20>;
+ ti,latch-bit=<26>;
+ assigned-clocks = <_gmac_h14x2_ctrl_ck>;
+ assigned-clock-rates = <8000>;
+ };
+
+ dpll_gmac_h14x2_ctrl_mux_ck: dpll_gmac_h14x2_ctrl_mux_ck@3fc {
+ #clock-cells = <0>;
+ compatible = "ti,mux-clock";
+ clocks = <_gmac_ck>, <_gmac_h14x2_ctrl_ck>;
+ reg = <0x3fc>;
+ ti,bit-shift = <29>;
+ ti,latch-bit=<26>;
+ assigned-clocks = <_gmac_h14x2_ctrl_mux_ck>;
+ assigned-clock-parents = <_gmac_h14x2_ctrl_ck>;
+ };
+
+ mcan_clk: mcan_clk@3fc {
+ #clock-cells = <0>;
+ compatible = "ti,gate-clock";
+ clocks = <_gmac_h14x2_ctrl_mux_ck>;
+ ti,bit-shift = <27>;
+ reg = <0x3fc>;
+ };
+};
--
2.17.0
Re: [PATCH 2/2] tools/memory-model: Add write ordering by release-acquire and by locks
On Wed, 4 Jul 2018, Will Deacon wrote:
> Hi Alan,
>
> On Tue, Jul 03, 2018 at 01:28:17PM -0400, Alan Stern wrote:
> > On Mon, 25 Jun 2018, Andrea Parri wrote:
> >
> > > On Fri, Jun 22, 2018 at 07:30:08PM +0100, Will Deacon wrote:
> > > > > > I think the second example would preclude us using LDAPR for
> > > > > > load-acquire,
> > >
> > > > I don't think it's a moot point. We want new architectures to implement
> > > > acquire/release efficiently, and it's not unlikely that they will have
> > > > acquire loads that are similar in semantics to LDAPR. This patch
> > > > prevents
> > > > them from doing so,
> > >
> > > By this same argument, you should not be a "big fan" of rfi-rel-acq in
> > > ppo ;)
> > > consider, e.g., the two litmus tests below: what am I missing?
> >
> > This is an excellent point, which seems to have gotten lost in the
> > shuffle. I'd like to see your comments.
>
> Yeah, sorry. Loads going on at the moment. You could ask herd instead of me
> though ;)
Indeed; and the answer was as expected. Sometimes one gains additional
insights by asking a person, though.
> > In essence, if you're using release-acquire instructions that only
> > provide RCpc consistency, does store-release followed by load-acquire
> > of the same address provide read-read ordering? In theory it doesn't
> > have to, because if the value from the store-release is forwarded to
> > the load-acquire then:
> >
> > LOAD A
> > STORE-RELEASE X, v
> > LOAD-ACQUIRE X
> > LOAD B
> >
> > could be executed by the CPU in the order:
> >
> > LOAD-ACQUIRE X
> > LOAD B
> > LOAD A
> > STORE-RELEASE X, v
> >
> > thereby accessing A and B out of program order without violating the
> > requirements on the release or the acquire.
> >
> > Of course PPC doesn't allow this, but should we rule it out entirely?
>
> This would be allowed if LOAD-ACQUIRE was implemented using LDAPR on Arm.
> I don't think we should be ruling out architectures using RCpc
> acquire/release primitives, because doing so just feels like an artifact of
> most architectures building these out of fences today.
>
> It's funny really, because from an Arm-perspective I don't plan to stray
> outside of RCsc, but I feel like other weak architectures aren't being
> well represented here. If we just care about x86, Arm and Power (and assume
> that Power doesn't plan to implement RCpc acquire/release instructions)
> then we're good to tighten things up. But I fear that RISC-V should probably
> be more engaged (adding Daniel) and who knows about MIPS or these other
> random architectures popping up on linux-arch.
I don't object to having weak versions of acquire/release in the LKMM.
Perhaps the stronger versions could be kept in the hardware model
(which has not been published and is not in the kernel source), but
even that might be a bad idea in view of what RISC-V is liable to do.
> > > C MP+fencewmbonceonce+pooncerelease-rfireleaseacquire-poacquireonce
> > >
> > > {}
> > >
> > > P0(int *x, int *y)
> > > {
> > > WRITE_ONCE(*x, 1);
> > > smp_wmb();
> > > WRITE_ONCE(*y, 1);
> > > }
> > >
> > > P1(int *x, int *y, int *z)
> > > {
> > > r0 = READ_ONCE(*y);
> > > smp_store_release(z, 1);
> > > r1 = smp_load_acquire(z);
> > > r2 = READ_ONCE(*x);
> > > }
> > >
> > > exists (1:r0=1 /\ 1:r1=1 /\ 1:r2=0)
> > >
> > >
> > > AArch64 MP+dmb.st+popl-rfilq-poqp
> > > "DMB.STdWW Rfe PodRWPL RfiLQ PodRRQP Fre"
> > > Generator=diyone7 (version 7.49+02(dev))
> > > Prefetch=0:x=F,0:y=W,1:y=F,1:x=T
> > > Com=Rf Fr
> > > Orig=DMB.STdWW Rfe PodRWPL RfiLQ PodRRQP Fre
> > > {
> > > 0:X1=x; 0:X3=y;
> > > 1:X1=y; 1:X3=z; 1:X6=x;
> > > }
> > > P0 | P1;
> > > MOV W0,#1 | LDR W0,[X1] ;
> > > STR W0,[X1] | MOV W2,#1 ;
> > > DMB ST | STLR W2,[X3] ;
> > > MOV W2,#1 | LDAPR W4,[X3] ;
> > > STR W2,[X3] | LDR W5,[X6] ;
> > > exists
> > > (1:X0=1 /\ 1:X4=1 /\ 1:X5=0)
>
> (you can also run this yourself, since 'Q' is supported in the .cat file
> I contributed to herdtools7)
>
> Test MP+dmb.sy+popl-rfilq-poqp Allowed
> States 4
> 1:X0=0; 1:X4=1; 1:X5=0;
> 1:X0=0; 1:X4=1; 1:X5=1;
> 1:X0=1; 1:X4=1; 1:X5=0;
> 1:X0=1; 1:X4=1; 1:X5=1;
> Ok
> Witnesses
> Positive: 1 Negative: 3
> Condition exists (1:X0=1 /\ 1:X4=1 /\ 1:X5=0)
> Observation MP+dmb.sy+popl-rfilq-poqp Sometimes 1 3
> Time MP+dmb.sy+popl-rfilq-poqp 0.01
> Hash=61858b7b59a6310d869f99cd05718f96
>
> > There's also read-write ordering, in the form of the LB pattern:
> >
> > P0(int *x, int *y, int *z)
> > {
> > r0 = READ_ONCE(*x);
> > smp_store_release(z, 1);
> > r1 = smp_load_acquire(z);
> > WRITE_ONCE(*y, 1);
> > }
> >
> > P1(int *x, int *y)
> > {
> > r2 = READ_ONCE(*y);
> > smp_mp();
> > WRITE_ONCE(*x, 1);
> > }
> >
> > exists (0:r0=1 /\ 1:r2=1)
>
> The access types are irrelevant to the acquire/release primitives, so yes
> that's also allowed.
>
> > Would this be allowed if smp_load_acquire() was
[PATCH v4 1/6] ARM: dts: dra762: Add MCAN clock support
From: Lokesh Vutla
MCAN is clocked by H14 divider of DPLL_GMAC. Unlike other
DPLL dividers this DPLL_GMAC H14 divider is controlled by
control module. Adding support for these clocks.
Signed-off-by: Lokesh Vutla
Signed-off-by: Faiz Abbas
---
arch/arm/boot/dts/dra76x.dtsi | 33 +
1 file changed, 33 insertions(+)
diff --git a/arch/arm/boot/dts/dra76x.dtsi b/arch/arm/boot/dts/dra76x.dtsi
index 1c88c581ff18..bfc82636999c 100644
--- a/arch/arm/boot/dts/dra76x.dtsi
+++ b/arch/arm/boot/dts/dra76x.dtsi
@@ -17,3 +17,36 @@
_mpu {
ti,irqs-skip = <10 67 68 133 139 140>;
};
+
+_conf_clocks {
+ dpll_gmac_h14x2_ctrl_ck: dpll_gmac_h14x2_ctrl_ck@3fc {
+ #clock-cells = <0>;
+ compatible = "ti,divider-clock";
+ clocks = <_gmac_x2_ck>;
+ ti,max-div = <63>;
+ reg = <0x03fc>;
+ ti,bit-shift=<20>;
+ ti,latch-bit=<26>;
+ assigned-clocks = <_gmac_h14x2_ctrl_ck>;
+ assigned-clock-rates = <8000>;
+ };
+
+ dpll_gmac_h14x2_ctrl_mux_ck: dpll_gmac_h14x2_ctrl_mux_ck@3fc {
+ #clock-cells = <0>;
+ compatible = "ti,mux-clock";
+ clocks = <_gmac_ck>, <_gmac_h14x2_ctrl_ck>;
+ reg = <0x3fc>;
+ ti,bit-shift = <29>;
+ ti,latch-bit=<26>;
+ assigned-clocks = <_gmac_h14x2_ctrl_mux_ck>;
+ assigned-clock-parents = <_gmac_h14x2_ctrl_ck>;
+ };
+
+ mcan_clk: mcan_clk@3fc {
+ #clock-cells = <0>;
+ compatible = "ti,gate-clock";
+ clocks = <_gmac_h14x2_ctrl_mux_ck>;
+ ti,bit-shift = <27>;
+ reg = <0x3fc>;
+ };
+};
--
2.17.0
[PATCH v4 3/6] bus: ti-sysc: Add support for using ti-sysc for MCAN on dra76x
The dra76x MCAN generic interconnect module has a its own
format for the bits in the control registers.
Therefore add a new module type, new regbits and new capabilities
specific to the MCAN module.
Acked-by: Rob Herring
CC: Tony Lindgren
Signed-off-by: Faiz Abbas
---
.../devicetree/bindings/bus/ti-sysc.txt| 1 +
drivers/bus/ti-sysc.c | 18 ++
include/dt-bindings/bus/ti-sysc.h | 2 ++
include/linux/platform_data/ti-sysc.h | 1 +
4 files changed, 22 insertions(+)
diff --git a/Documentation/devicetree/bindings/bus/ti-sysc.txt
b/Documentation/devicetree/bindings/bus/ti-sysc.txt
index d8ed5b780ed9..91dc2333af01 100644
--- a/Documentation/devicetree/bindings/bus/ti-sysc.txt
+++ b/Documentation/devicetree/bindings/bus/ti-sysc.txt
@@ -36,6 +36,7 @@ Required standard properties:
"ti,sysc-omap-aes"
"ti,sysc-mcasp"
"ti,sysc-usb-host-fs"
+ "ti,sysc-dra7-mcan"
- reg shall have register areas implemented for the interconnect
target module in question such as revision, sysc and syss
diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
index 1cc29629d238..ad1cd6888757 100644
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -1552,6 +1552,23 @@ static const struct sysc_capabilities
sysc_omap4_usb_host_fs = {
.regbits = _regbits_omap4_usb_host_fs,
};
+static const struct sysc_regbits sysc_regbits_dra7_mcan = {
+ .dmadisable_shift = -ENODEV,
+ .midle_shift = -ENODEV,
+ .sidle_shift = -ENODEV,
+ .clkact_shift = -ENODEV,
+ .enwkup_shift = 4,
+ .srst_shift = 0,
+ .emufree_shift = -ENODEV,
+ .autoidle_shift = -ENODEV,
+};
+
+static const struct sysc_capabilities sysc_dra7_mcan = {
+ .type = TI_SYSC_DRA7_MCAN,
+ .sysc_mask = SYSC_DRA7_MCAN_ENAWAKEUP | SYSC_OMAP4_SOFTRESET,
+ .regbits = _regbits_dra7_mcan,
+};
+
static int sysc_init_pdata(struct sysc *ddata)
{
struct ti_sysc_platform_data *pdata = dev_get_platdata(ddata->dev);
@@ -1743,6 +1760,7 @@ static const struct of_device_id sysc_match[] = {
{ .compatible = "ti,sysc-mcasp", .data = _omap4_mcasp, },
{ .compatible = "ti,sysc-usb-host-fs",
.data = _omap4_usb_host_fs, },
+ { .compatible = "ti,sysc-dra7-mcan", .data = _dra7_mcan, },
{ },
};
MODULE_DEVICE_TABLE(of, sysc_match);
diff --git a/include/dt-bindings/bus/ti-sysc.h
b/include/dt-bindings/bus/ti-sysc.h
index 2c005376ac0e..7138384e2ef9 100644
--- a/include/dt-bindings/bus/ti-sysc.h
+++ b/include/dt-bindings/bus/ti-sysc.h
@@ -15,6 +15,8 @@
/* SmartReflex sysc found on 36xx and later */
#define SYSC_OMAP3_SR_ENAWAKEUP(1 << 26)
+#define SYSC_DRA7_MCAN_ENAWAKEUP (1 << 4)
+
/* SYSCONFIG STANDBYMODE/MIDLEMODE/SIDLEMODE supported by hardware */
#define SYSC_IDLE_FORCE0
#define SYSC_IDLE_NO 1
diff --git a/include/linux/platform_data/ti-sysc.h
b/include/linux/platform_data/ti-sysc.h
index 990aad477458..2efa3470a451 100644
--- a/include/linux/platform_data/ti-sysc.h
+++ b/include/linux/platform_data/ti-sysc.h
@@ -14,6 +14,7 @@ enum ti_sysc_module_type {
TI_SYSC_OMAP4_SR,
TI_SYSC_OMAP4_MCASP,
TI_SYSC_OMAP4_USB_HOST_FS,
+ TI_SYSC_DRA7_MCAN,
};
struct ti_sysc_cookie {
--
2.17.0
Re: [PATCH 06/19] mmc: mmci: add variant properties to define cpsm & cmdresp bits
On 12 June 2018 at 15:14, Ludovic Barre wrote:
> From: Ludovic Barre
>
> This patch adds command variant properties to define
> cpsm enable bit and responses.
> Needed to support the STM32 variant (shift of cpsm bit,
> specific definition of commands response).
>
> Signed-off-by: Ludovic Barre
> ---
> drivers/mmc/host/mmci.c | 47 +++
> drivers/mmc/host/mmci.h | 8
> 2 files changed, 51 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 801c86b..52562fc 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -51,6 +51,10 @@ static unsigned int fmax = 515633;
> static struct variant_data variant_arm = {
> .fifosize = 16 * 4,
> .fifohalfsize = 8 * 4,
> + .cmdreg_cpsm_enable = MCI_CPSM_ENABLE,
> + .cmdreg_lrsp_crc= MCI_CPSM_RESPONSE | MCI_CPSM_LONGRSP,
> + .cmdreg_srsp_crc= MCI_CPSM_RESPONSE,
> + .cmdreg_srsp= MCI_CPSM_RESPONSE,
Do these really needs to be a superset of each other?
Maybe it becomes easier for the STM32 variant later though...
[...]
> @@ -603,16 +639,19 @@ mmci_start_command(struct mmci_host *host, struct
> mmc_command *cmd, u32 c)
While at it, would you mind folding in a cleanup patch removing the
"u32 c" as in-parameter? It's currently always set to "0" by the
caller, so it's not needed.
> dev_dbg(mmc_dev(host->mmc), "op %02x arg %08x flags %08x\n",
> cmd->opcode, cmd->arg, cmd->flags);
>
> - if (readl(base + MMCICOMMAND) & MCI_CPSM_ENABLE) {
> + if (readl(base + MMCICOMMAND) & host->variant->cmdreg_cpsm_enable) {
> writel(0, base + MMCICOMMAND);
> mmci_reg_delay(host);
> }
>
> - c |= cmd->opcode | MCI_CPSM_ENABLE;
> + c |= cmd->opcode | host->variant->cmdreg_cpsm_enable;
> if (cmd->flags & MMC_RSP_PRESENT) {
> if (cmd->flags & MMC_RSP_136)
> - c |= MCI_CPSM_LONGRSP;
> - c |= MCI_CPSM_RESPONSE;
> + c |= host->variant->cmdreg_lrsp_crc;
This looks weird. Probably because of the naming of the variant data.
Perhaps rename to "cmdreg_lrsp", thus skipping the "_crc" suffix?
> + else if (cmd->flags & MMC_RSP_CRC)
> + c |= host->variant->cmdreg_srsp_crc;
Why is here an else? We can have both MMC_RSP_136 and MMC_RSP_CRC bits
set, right!?
> + else
> + c |= host->variant->cmdreg_srsp;
What do you think about using a switch-case, perhaps with fall-through
- and then adding those bits that are needed for each response
bit+variant instead? Could that make this more readable?
> }
> if (/*interrupt*/0)
> c |= MCI_CPSM_INTERRUPT;
> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
> index 7265ca6..e173305 100644
> --- a/drivers/mmc/host/mmci.h
> +++ b/drivers/mmc/host/mmci.h
> @@ -239,6 +239,10 @@ struct mmci_host;
> * @clkreg_enable: enable value for MMCICLOCK register
> * @clkreg_8bit_bus_enable: enable value for 8 bit bus
> * @clkreg_neg_edge_enable: enable value for inverted data/cmd output
> + * @cmdreg_cpsm_enable: enable value for CPSM
> + * @cmdreg_lrsp_crc: enable value for long response with crc
> + * @cmdreg_srsp_crc: enable value for short response with crc
> + * @cmdreg_srsp: enable value for short response without crc
> * @datalength_bits: number of bits in the MMCIDATALENGTH register
> * @fifosize: number of bytes that can be written when MMCI_TXFIFOEMPTY
> * is asserted (likewise for RX)
> @@ -282,6 +286,10 @@ struct variant_data {
> unsigned intclkreg_enable;
> unsigned intclkreg_8bit_bus_enable;
> unsigned intclkreg_neg_edge_enable;
> + unsigned intcmdreg_cpsm_enable;
> + unsigned intcmdreg_lrsp_crc;
> + unsigned intcmdreg_srsp_crc;
> + unsigned intcmdreg_srsp;
> unsigned intdatalength_bits;
> unsigned intfifosize;
> unsigned intfifohalfsize;
> --
> 2.7.4
>
Kind regards
Uffe
Re: [PATCH 06/19] mmc: mmci: add variant properties to define cpsm & cmdresp bits
On 12 June 2018 at 15:14, Ludovic Barre wrote:
> From: Ludovic Barre
>
> This patch adds command variant properties to define
> cpsm enable bit and responses.
> Needed to support the STM32 variant (shift of cpsm bit,
> specific definition of commands response).
>
> Signed-off-by: Ludovic Barre
> ---
> drivers/mmc/host/mmci.c | 47 +++
> drivers/mmc/host/mmci.h | 8
> 2 files changed, 51 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 801c86b..52562fc 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -51,6 +51,10 @@ static unsigned int fmax = 515633;
> static struct variant_data variant_arm = {
> .fifosize = 16 * 4,
> .fifohalfsize = 8 * 4,
> + .cmdreg_cpsm_enable = MCI_CPSM_ENABLE,
> + .cmdreg_lrsp_crc= MCI_CPSM_RESPONSE | MCI_CPSM_LONGRSP,
> + .cmdreg_srsp_crc= MCI_CPSM_RESPONSE,
> + .cmdreg_srsp= MCI_CPSM_RESPONSE,
Do these really needs to be a superset of each other?
Maybe it becomes easier for the STM32 variant later though...
[...]
> @@ -603,16 +639,19 @@ mmci_start_command(struct mmci_host *host, struct
> mmc_command *cmd, u32 c)
While at it, would you mind folding in a cleanup patch removing the
"u32 c" as in-parameter? It's currently always set to "0" by the
caller, so it's not needed.
> dev_dbg(mmc_dev(host->mmc), "op %02x arg %08x flags %08x\n",
> cmd->opcode, cmd->arg, cmd->flags);
>
> - if (readl(base + MMCICOMMAND) & MCI_CPSM_ENABLE) {
> + if (readl(base + MMCICOMMAND) & host->variant->cmdreg_cpsm_enable) {
> writel(0, base + MMCICOMMAND);
> mmci_reg_delay(host);
> }
>
> - c |= cmd->opcode | MCI_CPSM_ENABLE;
> + c |= cmd->opcode | host->variant->cmdreg_cpsm_enable;
> if (cmd->flags & MMC_RSP_PRESENT) {
> if (cmd->flags & MMC_RSP_136)
> - c |= MCI_CPSM_LONGRSP;
> - c |= MCI_CPSM_RESPONSE;
> + c |= host->variant->cmdreg_lrsp_crc;
This looks weird. Probably because of the naming of the variant data.
Perhaps rename to "cmdreg_lrsp", thus skipping the "_crc" suffix?
> + else if (cmd->flags & MMC_RSP_CRC)
> + c |= host->variant->cmdreg_srsp_crc;
Why is here an else? We can have both MMC_RSP_136 and MMC_RSP_CRC bits
set, right!?
> + else
> + c |= host->variant->cmdreg_srsp;
What do you think about using a switch-case, perhaps with fall-through
- and then adding those bits that are needed for each response
bit+variant instead? Could that make this more readable?
> }
> if (/*interrupt*/0)
> c |= MCI_CPSM_INTERRUPT;
> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
> index 7265ca6..e173305 100644
> --- a/drivers/mmc/host/mmci.h
> +++ b/drivers/mmc/host/mmci.h
> @@ -239,6 +239,10 @@ struct mmci_host;
> * @clkreg_enable: enable value for MMCICLOCK register
> * @clkreg_8bit_bus_enable: enable value for 8 bit bus
> * @clkreg_neg_edge_enable: enable value for inverted data/cmd output
> + * @cmdreg_cpsm_enable: enable value for CPSM
> + * @cmdreg_lrsp_crc: enable value for long response with crc
> + * @cmdreg_srsp_crc: enable value for short response with crc
> + * @cmdreg_srsp: enable value for short response without crc
> * @datalength_bits: number of bits in the MMCIDATALENGTH register
> * @fifosize: number of bytes that can be written when MMCI_TXFIFOEMPTY
> * is asserted (likewise for RX)
> @@ -282,6 +286,10 @@ struct variant_data {
> unsigned intclkreg_enable;
> unsigned intclkreg_8bit_bus_enable;
> unsigned intclkreg_neg_edge_enable;
> + unsigned intcmdreg_cpsm_enable;
> + unsigned intcmdreg_lrsp_crc;
> + unsigned intcmdreg_srsp_crc;
> + unsigned intcmdreg_srsp;
> unsigned intdatalength_bits;
> unsigned intfifosize;
> unsigned intfifohalfsize;
> --
> 2.7.4
>
Kind regards
Uffe
Re: [PATCH] c6x: switch to NO_BOOTMEM
On Wed, 2018-07-04 at 16:25 +0300, Mike Rapoport wrote:
> Any comments on this?
>
> On Wed, Jun 27, 2018 at 03:09:20PM +0300, Mike Rapoport wrote:
> > The c6x is already using memblock and does most of early memory
> > reservations with it, so it was only a matter of removing the bootmem
> > initialization and handover of the memory from memblock to bootmem.
> >
> > Signed-off-by: Mike Rapoport
> > ---
> > arch/c6x/Kconfig| 1 +
> > arch/c6x/kernel/setup.c | 26 +-
> > 2 files changed, 2 insertions(+), 25 deletions(-)
> >
> > diff --git a/arch/c6x/Kconfig b/arch/c6x/Kconfig
> > index bf59855628ac..054c7c963180 100644
> > --- a/arch/c6x/Kconfig
> > +++ b/arch/c6x/Kconfig
> > @@ -14,6 +14,7 @@ config C6X
> > select GENERIC_IRQ_SHOW
> > select HAVE_ARCH_TRACEHOOK
> > select HAVE_MEMBLOCK
> > + select NO_BOOTMEM
> > select SPARSE_IRQ
> > select IRQ_DOMAIN
> > select OF
> > diff --git a/arch/c6x/kernel/setup.c b/arch/c6x/kernel/setup.c
> > index 786e36e2f61d..cc74cb9d349b 100644
> > --- a/arch/c6x/kernel/setup.c
> > +++ b/arch/c6x/kernel/setup.c
> > @@ -296,7 +296,6 @@ notrace void __init machine_init(unsigned long dt_ptr)
> >
> > void __init setup_arch(char **cmdline_p)
> > {
> > - int bootmap_size;
> > struct memblock_region *reg;
> >
> > printk(KERN_INFO "Initializing kernel\n");
> > @@ -353,16 +352,6 @@ void __init setup_arch(char **cmdline_p)
> > init_mm.end_data = memory_start;
> > init_mm.brk= memory_start;
> >
> > - /*
> > -* Give all the memory to the bootmap allocator, tell it to put the
> > -* boot mem_map at the start of memory
> > -*/
> > - bootmap_size = init_bootmem_node(NODE_DATA(0),
> > -memory_start >> PAGE_SHIFT,
> > -PAGE_OFFSET >> PAGE_SHIFT,
> > -memory_end >> PAGE_SHIFT);
> > - memblock_reserve(memory_start, bootmap_size);
> > -
> > unflatten_device_tree();
> >
> > c6x_cache_init();
> > @@ -397,22 +386,9 @@ void __init setup_arch(char **cmdline_p)
> > /* Initialize the coherent memory allocator */
> > coherent_mem_init(dma_start, dma_size);
> >
> > - /*
> > -* Free all memory as a starting point.
> > -*/
> > - free_bootmem(PAGE_OFFSET, memory_end - PAGE_OFFSET);
> > -
> > - /*
> > -* Then reserve memory which is already being used.
> > -*/
> > - for_each_memblock(reserved, reg) {
> > - pr_debug("reserved - 0x%08x-0x%08x\n",
> > -(u32) reg->base, (u32) reg->size);
> > - reserve_bootmem(reg->base, reg->size, BOOTMEM_DEFAULT);
> > - }
> > -
> > max_low_pfn = PFN_DOWN(memory_end);
> > min_low_pfn = PFN_UP(memory_start);
> > + max_pfn = max_low_pfn;
> > max_mapnr = max_low_pfn - min_low_pfn;
> >
> > /* Get kmalloc into gear */
> > --
> > 2.7.4
> >
>
>
Thanks. Looks fine. I'll pull it into c6x tree for next merge window.
Re: [PATCH] c6x: switch to NO_BOOTMEM
On Wed, 2018-07-04 at 16:25 +0300, Mike Rapoport wrote:
> Any comments on this?
>
> On Wed, Jun 27, 2018 at 03:09:20PM +0300, Mike Rapoport wrote:
> > The c6x is already using memblock and does most of early memory
> > reservations with it, so it was only a matter of removing the bootmem
> > initialization and handover of the memory from memblock to bootmem.
> >
> > Signed-off-by: Mike Rapoport
> > ---
> > arch/c6x/Kconfig| 1 +
> > arch/c6x/kernel/setup.c | 26 +-
> > 2 files changed, 2 insertions(+), 25 deletions(-)
> >
> > diff --git a/arch/c6x/Kconfig b/arch/c6x/Kconfig
> > index bf59855628ac..054c7c963180 100644
> > --- a/arch/c6x/Kconfig
> > +++ b/arch/c6x/Kconfig
> > @@ -14,6 +14,7 @@ config C6X
> > select GENERIC_IRQ_SHOW
> > select HAVE_ARCH_TRACEHOOK
> > select HAVE_MEMBLOCK
> > + select NO_BOOTMEM
> > select SPARSE_IRQ
> > select IRQ_DOMAIN
> > select OF
> > diff --git a/arch/c6x/kernel/setup.c b/arch/c6x/kernel/setup.c
> > index 786e36e2f61d..cc74cb9d349b 100644
> > --- a/arch/c6x/kernel/setup.c
> > +++ b/arch/c6x/kernel/setup.c
> > @@ -296,7 +296,6 @@ notrace void __init machine_init(unsigned long dt_ptr)
> >
> > void __init setup_arch(char **cmdline_p)
> > {
> > - int bootmap_size;
> > struct memblock_region *reg;
> >
> > printk(KERN_INFO "Initializing kernel\n");
> > @@ -353,16 +352,6 @@ void __init setup_arch(char **cmdline_p)
> > init_mm.end_data = memory_start;
> > init_mm.brk= memory_start;
> >
> > - /*
> > -* Give all the memory to the bootmap allocator, tell it to put the
> > -* boot mem_map at the start of memory
> > -*/
> > - bootmap_size = init_bootmem_node(NODE_DATA(0),
> > -memory_start >> PAGE_SHIFT,
> > -PAGE_OFFSET >> PAGE_SHIFT,
> > -memory_end >> PAGE_SHIFT);
> > - memblock_reserve(memory_start, bootmap_size);
> > -
> > unflatten_device_tree();
> >
> > c6x_cache_init();
> > @@ -397,22 +386,9 @@ void __init setup_arch(char **cmdline_p)
> > /* Initialize the coherent memory allocator */
> > coherent_mem_init(dma_start, dma_size);
> >
> > - /*
> > -* Free all memory as a starting point.
> > -*/
> > - free_bootmem(PAGE_OFFSET, memory_end - PAGE_OFFSET);
> > -
> > - /*
> > -* Then reserve memory which is already being used.
> > -*/
> > - for_each_memblock(reserved, reg) {
> > - pr_debug("reserved - 0x%08x-0x%08x\n",
> > -(u32) reg->base, (u32) reg->size);
> > - reserve_bootmem(reg->base, reg->size, BOOTMEM_DEFAULT);
> > - }
> > -
> > max_low_pfn = PFN_DOWN(memory_end);
> > min_low_pfn = PFN_UP(memory_start);
> > + max_pfn = max_low_pfn;
> > max_mapnr = max_low_pfn - min_low_pfn;
> >
> > /* Get kmalloc into gear */
> > --
> > 2.7.4
> >
>
>
Thanks. Looks fine. I'll pull it into c6x tree for next merge window.
Re: [PATCH v2 5/6] mm: track gup pages with page->dma_pinned_* fields
On Wed, 4 Jul 2018, Jan Kara wrote: > > So this seems unsolvable without having the caller specify that it knows the > > page type, and that it is therefore safe to decrement > > page->dma_pinned_count. > > I was hoping I'd found a way, but clearly I haven't. :) > > Well, I think the misconception is that "pinned" is a fundamental property > of a page. It is not. "pinned" is a property of a page reference (i.e., a > kind of reference that can be used for DMA access) and page gets into > "pinned" state if it has any reference of "pinned" type. And when you > realize this, it is obvious that you just have to have a special api for > getting and dropping references of this "pinned" type. For getting we > already have get_user_pages(), for putting we have to create the api... Maybe we can do something by creating a special "pinned" bit in the pte? If it is a RDMA reference then set that pinned bit there. Thus any of the references could cause a pin. Since the page struct does not contain that information we therefore have to scan through the ptes to figure out if a page is pinned? If so then we would not need a special function for dropping the reference. References to a page can also be created from devices mmu. Maybe we could at some point start to manage them in a similar way to the page tables of the processor? The mmu notifiers are a bit awkward if we need closer mm integration.
Re: [PATCH v2 5/6] mm: track gup pages with page->dma_pinned_* fields
On Wed, 4 Jul 2018, Jan Kara wrote: > > So this seems unsolvable without having the caller specify that it knows the > > page type, and that it is therefore safe to decrement > > page->dma_pinned_count. > > I was hoping I'd found a way, but clearly I haven't. :) > > Well, I think the misconception is that "pinned" is a fundamental property > of a page. It is not. "pinned" is a property of a page reference (i.e., a > kind of reference that can be used for DMA access) and page gets into > "pinned" state if it has any reference of "pinned" type. And when you > realize this, it is obvious that you just have to have a special api for > getting and dropping references of this "pinned" type. For getting we > already have get_user_pages(), for putting we have to create the api... Maybe we can do something by creating a special "pinned" bit in the pte? If it is a RDMA reference then set that pinned bit there. Thus any of the references could cause a pin. Since the page struct does not contain that information we therefore have to scan through the ptes to figure out if a page is pinned? If so then we would not need a special function for dropping the reference. References to a page can also be created from devices mmu. Maybe we could at some point start to manage them in a similar way to the page tables of the processor? The mmu notifiers are a bit awkward if we need closer mm integration.
Re: [PATCHv4 11/12] sched/core: Disable SD_ASYM_CPUCAPACITY for root_domains without asymmetry
On Thu, Jul 05, 2018 at 02:31:43PM +0100, Quentin Perret wrote:
> Hi Morten,
>
> On Wednesday 04 Jul 2018 at 11:17:49 (+0100), Morten Rasmussen wrote:
> > diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> > index 71330e0e41db..29c186961345 100644
> > --- a/kernel/sched/topology.c
> > +++ b/kernel/sched/topology.c
> > @@ -1160,6 +1160,26 @@ sd_init(struct sched_domain_topology_level *tl,
> > sd_id = cpumask_first(sched_domain_span(sd));
> >
> > /*
> > +* Check if cpu_map eclipses cpu capacity asymmetry.
> > +*/
> > +
> > + if (sd->flags & SD_ASYM_CPUCAPACITY) {
> > + int i;
> > + bool disable = true;
> > + long capacity = arch_scale_cpu_capacity(NULL, sd_id);
> > +
> > + for_each_cpu(i, sched_domain_span(sd)) {
> > + if (capacity != arch_scale_cpu_capacity(NULL, i)) {
> > + disable = false;
> > + break;
> > + }
> > + }
> > +
> > + if (disable)
> > + sd->flags &= ~SD_ASYM_CPUCAPACITY;
> > + }
> > +
> > + /*
> > * Convert topological properties into behaviour.
> > */
>
> If SD_ASYM_CPUCAPACITY means that some CPUs have different
> arch_scale_cpu_capacity() values, we could also automatically _set_
> the flag in sd_init() no ? Why should we let the arch set it and just
> correct it later ?
>
> I understand the moment at which we know the capacities of CPUs varies
> from arch to arch, but the arch code could just call
> rebuild_sched_domain when the capacities of CPUs change and let the
> scheduler detect things automatically. I mean, even if the arch code
> sets the flag in its topology level table, it will have to rebuild
> the sched domains anyway ...
>
> What do you think ?
We could as well set the flag here so the architecture doesn't have to
do it. It is a bit more complicated though for few reasons:
1. Detecting when to disable the flag is a lot simpler than checking
which level is should be set on. You basically have to work you way up
from the lowest topology level until you get to a level spanning all the
capacities available in the system to figure out where the flag should
be set. I don't think this fits easily with how we build the
sched_domain hierarchy. It can of course be done.
2. As you say, we still need the arch code (or cpufreq?) to rebuild the
whole thing once we know that the capacities have been determined. That
currently implies implementing arch_update_cpu_topology() which is
arch-specific. So we would need some arch code to make rebuild happen at
the right point in time. If the rebuild should be triggering the rebuild
we need another way to force a full rebuild. This can also be done.
3. Detecting the flag in generic kernel/sched/* code means that all
architectures will pay the for the overhead when building/rebuilding the
sched_domain hierarchy, and all architectures that sets the cpu
capacities to asymmetric will set the flag whether they like it or not.
I'm not sure if this is a problem.
In the end it is really about how much of this we want in generic code
and how much we hide in arch/, and if we dare to touch the sched_domain
build code ;-)
Morten
Re: [PATCHv4 11/12] sched/core: Disable SD_ASYM_CPUCAPACITY for root_domains without asymmetry
On Thu, Jul 05, 2018 at 02:31:43PM +0100, Quentin Perret wrote:
> Hi Morten,
>
> On Wednesday 04 Jul 2018 at 11:17:49 (+0100), Morten Rasmussen wrote:
> > diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> > index 71330e0e41db..29c186961345 100644
> > --- a/kernel/sched/topology.c
> > +++ b/kernel/sched/topology.c
> > @@ -1160,6 +1160,26 @@ sd_init(struct sched_domain_topology_level *tl,
> > sd_id = cpumask_first(sched_domain_span(sd));
> >
> > /*
> > +* Check if cpu_map eclipses cpu capacity asymmetry.
> > +*/
> > +
> > + if (sd->flags & SD_ASYM_CPUCAPACITY) {
> > + int i;
> > + bool disable = true;
> > + long capacity = arch_scale_cpu_capacity(NULL, sd_id);
> > +
> > + for_each_cpu(i, sched_domain_span(sd)) {
> > + if (capacity != arch_scale_cpu_capacity(NULL, i)) {
> > + disable = false;
> > + break;
> > + }
> > + }
> > +
> > + if (disable)
> > + sd->flags &= ~SD_ASYM_CPUCAPACITY;
> > + }
> > +
> > + /*
> > * Convert topological properties into behaviour.
> > */
>
> If SD_ASYM_CPUCAPACITY means that some CPUs have different
> arch_scale_cpu_capacity() values, we could also automatically _set_
> the flag in sd_init() no ? Why should we let the arch set it and just
> correct it later ?
>
> I understand the moment at which we know the capacities of CPUs varies
> from arch to arch, but the arch code could just call
> rebuild_sched_domain when the capacities of CPUs change and let the
> scheduler detect things automatically. I mean, even if the arch code
> sets the flag in its topology level table, it will have to rebuild
> the sched domains anyway ...
>
> What do you think ?
We could as well set the flag here so the architecture doesn't have to
do it. It is a bit more complicated though for few reasons:
1. Detecting when to disable the flag is a lot simpler than checking
which level is should be set on. You basically have to work you way up
from the lowest topology level until you get to a level spanning all the
capacities available in the system to figure out where the flag should
be set. I don't think this fits easily with how we build the
sched_domain hierarchy. It can of course be done.
2. As you say, we still need the arch code (or cpufreq?) to rebuild the
whole thing once we know that the capacities have been determined. That
currently implies implementing arch_update_cpu_topology() which is
arch-specific. So we would need some arch code to make rebuild happen at
the right point in time. If the rebuild should be triggering the rebuild
we need another way to force a full rebuild. This can also be done.
3. Detecting the flag in generic kernel/sched/* code means that all
architectures will pay the for the overhead when building/rebuilding the
sched_domain hierarchy, and all architectures that sets the cpu
capacities to asymmetric will set the flag whether they like it or not.
I'm not sure if this is a problem.
In the end it is really about how much of this we want in generic code
and how much we hide in arch/, and if we dare to touch the sched_domain
build code ;-)
Morten
RE: Weird SysRq's during reboot
Panic over. It seems that my OnApp control server, when it loses contact with a compute node, sends an snmptrap command to force a reboot of that node. The trap script on the server uses sysrq-trigger, first "s", then "u", then "b". Please accept my apologies if you wasted any time on this. Regards, Alan Griffiths Bytehouse -Original Message- From: Alan [mailto:a...@bytehouse.co.uk] Sent: 05 July 2018 11:19 To: 'linux-kernel@vger.kernel.org' Subject: Weird SysRq's during reboot Hello everyone, this is my first time posting to this list so please be gentle. I have a server, hardware spec: HP DL360p G8 2 x Intel Xeon E5-2620 Hex-Core 2GHz processors 150GB Ram Embedded HP Ethernet 1Gb 4-port 331FLR Adapter Embedded Smart Array P420i Controller PCI-E Slot 1 HP Ethernet 10Gb 2-port 530SFP+ Adapter which I am trying to configure as an OnApp compute engine. Recently I have seen the following weird behaviour: 1. Login on the server as root 2. Enter a 'shutdown -r now' command 3. Watch the reboot on the iLO console 4. System reboots except that right at the end, after the login prompt appears on the console, I see: SysRq: Emergency Sync Emergency Sync complete SysRq : Emergency Remount R/O And then the system reboots again. 5. The second reboot always works without the SysRq. Looking in /var/log/messages I see, just prior to the second reboot (*): Jul 5 10:30:17 hv08 kernel: Ebtables v2.0 registered Jul 5 10:30:17 hv08 kernel: Bridge firewalling registered Jul 5 10:30:17 hv08 kernel: arp_tables: (C) 2002 David S. Miller Jul 5 10:30:18 hv08 kernel: lo: Disabled Privacy Extensions *Jul 5 10:30:27 hv08 kernel: SysRq : Emergency Sync *Jul 5 10:30:27 hv08 kernel: Emergency Sync complete *Jul 5 10:30:28 hv08 kernel: SysRq : Emergency Remount R/O Jul 5 10:33:43 hv08 kernel: imklog 5.8.10, log source = /proc/kmsg started. Jul 5 10:33:43 hv08 rsyslogd: [origin software="rsyslogd" swVersion="5.8.10" x-pid="2491" x-info="http://www.rsyslog.com;] start Jul 5 10:33:43 hv08 kernel: Initializing cgroup subsys cpuset Jul 5 10:33:43 hv08 kernel: Initializing cgroup subsys cpu Jul 5 10:33:43 hv08 kernel: Linux version 2.6.32-696.30.1.el6.x86_64 (mockbu...@c1bl.rdu2.centos.org) (gcc version 4.4.7 20120313 (Red Hat 4.4.7-18) (GCC) ) #1 SMP Tue May 22 03:28:18 UTC 2018 Jul 5 10:33:43 hv08 kernel: Command line: ro root=UUID=eafc533f-c5cb-4c89-bc0d-33125ab7f9d9 nomodeset rd_NO_LUKS KEYBOARDTYPE=pc KEYTABLE=uk LANG=en_US.UTF-8 rd_NO_MD SYSFONT=latarcyrheb-sun16 crash kernel=auto rd_NO_LVM rd_NO_DM rhgb quiet I have tried booting the 2.6.32-696.28.1.el6.x86_64 and the 2.6.32-696.el6.x86_64 kernels with the same result. The problem does not occur if I boot from the Centos 6.9 liveDVD iso which also uses kernel version 2.6.32-696.el6.x86_64. So, I am hoping someone here can answer any of these questions for me: A. What have I done to make this happen? B. Is there a way I can fix it? C. If not is there some other way to stop it happening? Thank you for taking the time to read this email. I shall look forward to hearing from you. Please CC me on any replies. Regards, Alan Griffiths Bytehouse
RE: Weird SysRq's during reboot
Panic over. It seems that my OnApp control server, when it loses contact with a compute node, sends an snmptrap command to force a reboot of that node. The trap script on the server uses sysrq-trigger, first "s", then "u", then "b". Please accept my apologies if you wasted any time on this. Regards, Alan Griffiths Bytehouse -Original Message- From: Alan [mailto:a...@bytehouse.co.uk] Sent: 05 July 2018 11:19 To: 'linux-kernel@vger.kernel.org' Subject: Weird SysRq's during reboot Hello everyone, this is my first time posting to this list so please be gentle. I have a server, hardware spec: HP DL360p G8 2 x Intel Xeon E5-2620 Hex-Core 2GHz processors 150GB Ram Embedded HP Ethernet 1Gb 4-port 331FLR Adapter Embedded Smart Array P420i Controller PCI-E Slot 1 HP Ethernet 10Gb 2-port 530SFP+ Adapter which I am trying to configure as an OnApp compute engine. Recently I have seen the following weird behaviour: 1. Login on the server as root 2. Enter a 'shutdown -r now' command 3. Watch the reboot on the iLO console 4. System reboots except that right at the end, after the login prompt appears on the console, I see: SysRq: Emergency Sync Emergency Sync complete SysRq : Emergency Remount R/O And then the system reboots again. 5. The second reboot always works without the SysRq. Looking in /var/log/messages I see, just prior to the second reboot (*): Jul 5 10:30:17 hv08 kernel: Ebtables v2.0 registered Jul 5 10:30:17 hv08 kernel: Bridge firewalling registered Jul 5 10:30:17 hv08 kernel: arp_tables: (C) 2002 David S. Miller Jul 5 10:30:18 hv08 kernel: lo: Disabled Privacy Extensions *Jul 5 10:30:27 hv08 kernel: SysRq : Emergency Sync *Jul 5 10:30:27 hv08 kernel: Emergency Sync complete *Jul 5 10:30:28 hv08 kernel: SysRq : Emergency Remount R/O Jul 5 10:33:43 hv08 kernel: imklog 5.8.10, log source = /proc/kmsg started. Jul 5 10:33:43 hv08 rsyslogd: [origin software="rsyslogd" swVersion="5.8.10" x-pid="2491" x-info="http://www.rsyslog.com;] start Jul 5 10:33:43 hv08 kernel: Initializing cgroup subsys cpuset Jul 5 10:33:43 hv08 kernel: Initializing cgroup subsys cpu Jul 5 10:33:43 hv08 kernel: Linux version 2.6.32-696.30.1.el6.x86_64 (mockbu...@c1bl.rdu2.centos.org) (gcc version 4.4.7 20120313 (Red Hat 4.4.7-18) (GCC) ) #1 SMP Tue May 22 03:28:18 UTC 2018 Jul 5 10:33:43 hv08 kernel: Command line: ro root=UUID=eafc533f-c5cb-4c89-bc0d-33125ab7f9d9 nomodeset rd_NO_LUKS KEYBOARDTYPE=pc KEYTABLE=uk LANG=en_US.UTF-8 rd_NO_MD SYSFONT=latarcyrheb-sun16 crash kernel=auto rd_NO_LVM rd_NO_DM rhgb quiet I have tried booting the 2.6.32-696.28.1.el6.x86_64 and the 2.6.32-696.el6.x86_64 kernels with the same result. The problem does not occur if I boot from the Centos 6.9 liveDVD iso which also uses kernel version 2.6.32-696.el6.x86_64. So, I am hoping someone here can answer any of these questions for me: A. What have I done to make this happen? B. Is there a way I can fix it? C. If not is there some other way to stop it happening? Thank you for taking the time to read this email. I shall look forward to hearing from you. Please CC me on any replies. Regards, Alan Griffiths Bytehouse
Re: [PATCH 2/2] tools/memory-model: Add write ordering by release-acquire and by locks
On Wed, Jul 04, 2018 at 01:11:04PM +0100, Will Deacon wrote:
> Hi Alan,
>
> On Tue, Jul 03, 2018 at 01:28:17PM -0400, Alan Stern wrote:
> > On Mon, 25 Jun 2018, Andrea Parri wrote:
> >
> > > On Fri, Jun 22, 2018 at 07:30:08PM +0100, Will Deacon wrote:
> > > > > > I think the second example would preclude us using LDAPR for
> > > > > > load-acquire,
> > >
> > > > I don't think it's a moot point. We want new architectures to implement
> > > > acquire/release efficiently, and it's not unlikely that they will have
> > > > acquire loads that are similar in semantics to LDAPR. This patch
> > > > prevents
> > > > them from doing so,
> > >
> > > By this same argument, you should not be a "big fan" of rfi-rel-acq in
> > > ppo ;)
> > > consider, e.g., the two litmus tests below: what am I missing?
> >
> > This is an excellent point, which seems to have gotten lost in the
> > shuffle. I'd like to see your comments.
>
> Yeah, sorry. Loads going on at the moment. You could ask herd instead of me
> though ;)
>
> > In essence, if you're using release-acquire instructions that only
> > provide RCpc consistency, does store-release followed by load-acquire
> > of the same address provide read-read ordering? In theory it doesn't
> > have to, because if the value from the store-release is forwarded to
> > the load-acquire then:
> >
> > LOAD A
> > STORE-RELEASE X, v
> > LOAD-ACQUIRE X
> > LOAD B
> >
> > could be executed by the CPU in the order:
> >
> > LOAD-ACQUIRE X
> > LOAD B
> > LOAD A
> > STORE-RELEASE X, v
> >
> > thereby accessing A and B out of program order without violating the
> > requirements on the release or the acquire.
> >
> > Of course PPC doesn't allow this, but should we rule it out entirely?
>
> This would be allowed if LOAD-ACQUIRE was implemented using LDAPR on Arm.
> I don't think we should be ruling out architectures using RCpc
> acquire/release primitives, because doing so just feels like an artifact of
> most architectures building these out of fences today.
>
> It's funny really, because from an Arm-perspective I don't plan to stray
> outside of RCsc, but I feel like other weak architectures aren't being
> well represented here. If we just care about x86, Arm and Power (and assume
> that Power doesn't plan to implement RCpc acquire/release instructions)
> then we're good to tighten things up. But I fear that RISC-V should probably
> be more engaged (adding Daniel) and who knows about MIPS or these other
> random architectures popping up on linux-arch.
>
> > > C MP+fencewmbonceonce+pooncerelease-rfireleaseacquire-poacquireonce
> > >
> > > {}
> > >
> > > P0(int *x, int *y)
> > > {
> > > WRITE_ONCE(*x, 1);
> > > smp_wmb();
> > > WRITE_ONCE(*y, 1);
> > > }
> > >
> > > P1(int *x, int *y, int *z)
> > > {
> > > r0 = READ_ONCE(*y);
> > > smp_store_release(z, 1);
> > > r1 = smp_load_acquire(z);
> > > r2 = READ_ONCE(*x);
> > > }
> > >
> > > exists (1:r0=1 /\ 1:r1=1 /\ 1:r2=0)
> > >
> > >
> > > AArch64 MP+dmb.st+popl-rfilq-poqp
> > > "DMB.STdWW Rfe PodRWPL RfiLQ PodRRQP Fre"
> > > Generator=diyone7 (version 7.49+02(dev))
> > > Prefetch=0:x=F,0:y=W,1:y=F,1:x=T
> > > Com=Rf Fr
> > > Orig=DMB.STdWW Rfe PodRWPL RfiLQ PodRRQP Fre
> > > {
> > > 0:X1=x; 0:X3=y;
> > > 1:X1=y; 1:X3=z; 1:X6=x;
> > > }
> > > P0 | P1;
> > > MOV W0,#1 | LDR W0,[X1] ;
> > > STR W0,[X1] | MOV W2,#1 ;
> > > DMB ST | STLR W2,[X3] ;
> > > MOV W2,#1 | LDAPR W4,[X3] ;
> > > STR W2,[X3] | LDR W5,[X6] ;
> > > exists
> > > (1:X0=1 /\ 1:X4=1 /\ 1:X5=0)
>
> (you can also run this yourself, since 'Q' is supported in the .cat file
> I contributed to herdtools7)
>
> Test MP+dmb.sy+popl-rfilq-poqp Allowed
> States 4
> 1:X0=0; 1:X4=1; 1:X5=0;
> 1:X0=0; 1:X4=1; 1:X5=1;
> 1:X0=1; 1:X4=1; 1:X5=0;
> 1:X0=1; 1:X4=1; 1:X5=1;
> Ok
> Witnesses
> Positive: 1 Negative: 3
> Condition exists (1:X0=1 /\ 1:X4=1 /\ 1:X5=0)
> Observation MP+dmb.sy+popl-rfilq-poqp Sometimes 1 3
> Time MP+dmb.sy+popl-rfilq-poqp 0.01
> Hash=61858b7b59a6310d869f99cd05718f96
>
> > There's also read-write ordering, in the form of the LB pattern:
> >
> > P0(int *x, int *y, int *z)
> > {
> > r0 = READ_ONCE(*x);
> > smp_store_release(z, 1);
> > r1 = smp_load_acquire(z);
> > WRITE_ONCE(*y, 1);
> > }
> >
> > P1(int *x, int *y)
> > {
> > r2 = READ_ONCE(*y);
> > smp_mp();
> > WRITE_ONCE(*x, 1);
> > }
> >
> > exists (0:r0=1 /\ 1:r2=1)
>
> The access types are irrelevant to the acquire/release primitives, so yes
> that's also allowed.
>
> > Would this be allowed if smp_load_acquire() was implemented with LDAPR?
> > If the answer is yes then we will have to remove the rfi-rel-acq and
> > rel-rf-acq-po relations from the memory model entirely.
>
> I don't understand what you mean by "rfi-rel-acq-po", and I assume you mean
> rel-rfi-acq-po for the other? Sounds like I'm confused here.
[Your reply about 1/2 suggests that you've figured this out now, IAC
Re: [PATCH 2/2] tools/memory-model: Add write ordering by release-acquire and by locks
On Wed, Jul 04, 2018 at 01:11:04PM +0100, Will Deacon wrote:
> Hi Alan,
>
> On Tue, Jul 03, 2018 at 01:28:17PM -0400, Alan Stern wrote:
> > On Mon, 25 Jun 2018, Andrea Parri wrote:
> >
> > > On Fri, Jun 22, 2018 at 07:30:08PM +0100, Will Deacon wrote:
> > > > > > I think the second example would preclude us using LDAPR for
> > > > > > load-acquire,
> > >
> > > > I don't think it's a moot point. We want new architectures to implement
> > > > acquire/release efficiently, and it's not unlikely that they will have
> > > > acquire loads that are similar in semantics to LDAPR. This patch
> > > > prevents
> > > > them from doing so,
> > >
> > > By this same argument, you should not be a "big fan" of rfi-rel-acq in
> > > ppo ;)
> > > consider, e.g., the two litmus tests below: what am I missing?
> >
> > This is an excellent point, which seems to have gotten lost in the
> > shuffle. I'd like to see your comments.
>
> Yeah, sorry. Loads going on at the moment. You could ask herd instead of me
> though ;)
>
> > In essence, if you're using release-acquire instructions that only
> > provide RCpc consistency, does store-release followed by load-acquire
> > of the same address provide read-read ordering? In theory it doesn't
> > have to, because if the value from the store-release is forwarded to
> > the load-acquire then:
> >
> > LOAD A
> > STORE-RELEASE X, v
> > LOAD-ACQUIRE X
> > LOAD B
> >
> > could be executed by the CPU in the order:
> >
> > LOAD-ACQUIRE X
> > LOAD B
> > LOAD A
> > STORE-RELEASE X, v
> >
> > thereby accessing A and B out of program order without violating the
> > requirements on the release or the acquire.
> >
> > Of course PPC doesn't allow this, but should we rule it out entirely?
>
> This would be allowed if LOAD-ACQUIRE was implemented using LDAPR on Arm.
> I don't think we should be ruling out architectures using RCpc
> acquire/release primitives, because doing so just feels like an artifact of
> most architectures building these out of fences today.
>
> It's funny really, because from an Arm-perspective I don't plan to stray
> outside of RCsc, but I feel like other weak architectures aren't being
> well represented here. If we just care about x86, Arm and Power (and assume
> that Power doesn't plan to implement RCpc acquire/release instructions)
> then we're good to tighten things up. But I fear that RISC-V should probably
> be more engaged (adding Daniel) and who knows about MIPS or these other
> random architectures popping up on linux-arch.
>
> > > C MP+fencewmbonceonce+pooncerelease-rfireleaseacquire-poacquireonce
> > >
> > > {}
> > >
> > > P0(int *x, int *y)
> > > {
> > > WRITE_ONCE(*x, 1);
> > > smp_wmb();
> > > WRITE_ONCE(*y, 1);
> > > }
> > >
> > > P1(int *x, int *y, int *z)
> > > {
> > > r0 = READ_ONCE(*y);
> > > smp_store_release(z, 1);
> > > r1 = smp_load_acquire(z);
> > > r2 = READ_ONCE(*x);
> > > }
> > >
> > > exists (1:r0=1 /\ 1:r1=1 /\ 1:r2=0)
> > >
> > >
> > > AArch64 MP+dmb.st+popl-rfilq-poqp
> > > "DMB.STdWW Rfe PodRWPL RfiLQ PodRRQP Fre"
> > > Generator=diyone7 (version 7.49+02(dev))
> > > Prefetch=0:x=F,0:y=W,1:y=F,1:x=T
> > > Com=Rf Fr
> > > Orig=DMB.STdWW Rfe PodRWPL RfiLQ PodRRQP Fre
> > > {
> > > 0:X1=x; 0:X3=y;
> > > 1:X1=y; 1:X3=z; 1:X6=x;
> > > }
> > > P0 | P1;
> > > MOV W0,#1 | LDR W0,[X1] ;
> > > STR W0,[X1] | MOV W2,#1 ;
> > > DMB ST | STLR W2,[X3] ;
> > > MOV W2,#1 | LDAPR W4,[X3] ;
> > > STR W2,[X3] | LDR W5,[X6] ;
> > > exists
> > > (1:X0=1 /\ 1:X4=1 /\ 1:X5=0)
>
> (you can also run this yourself, since 'Q' is supported in the .cat file
> I contributed to herdtools7)
>
> Test MP+dmb.sy+popl-rfilq-poqp Allowed
> States 4
> 1:X0=0; 1:X4=1; 1:X5=0;
> 1:X0=0; 1:X4=1; 1:X5=1;
> 1:X0=1; 1:X4=1; 1:X5=0;
> 1:X0=1; 1:X4=1; 1:X5=1;
> Ok
> Witnesses
> Positive: 1 Negative: 3
> Condition exists (1:X0=1 /\ 1:X4=1 /\ 1:X5=0)
> Observation MP+dmb.sy+popl-rfilq-poqp Sometimes 1 3
> Time MP+dmb.sy+popl-rfilq-poqp 0.01
> Hash=61858b7b59a6310d869f99cd05718f96
>
> > There's also read-write ordering, in the form of the LB pattern:
> >
> > P0(int *x, int *y, int *z)
> > {
> > r0 = READ_ONCE(*x);
> > smp_store_release(z, 1);
> > r1 = smp_load_acquire(z);
> > WRITE_ONCE(*y, 1);
> > }
> >
> > P1(int *x, int *y)
> > {
> > r2 = READ_ONCE(*y);
> > smp_mp();
> > WRITE_ONCE(*x, 1);
> > }
> >
> > exists (0:r0=1 /\ 1:r2=1)
>
> The access types are irrelevant to the acquire/release primitives, so yes
> that's also allowed.
>
> > Would this be allowed if smp_load_acquire() was implemented with LDAPR?
> > If the answer is yes then we will have to remove the rfi-rel-acq and
> > rel-rf-acq-po relations from the memory model entirely.
>
> I don't understand what you mean by "rfi-rel-acq-po", and I assume you mean
> rel-rfi-acq-po for the other? Sounds like I'm confused here.
[Your reply about 1/2 suggests that you've figured this out now, IAC
Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button
Am 05.07.2018 um 14:12 schrieb Takashi Iwai: > On Thu, 05 Jul 2018 12:41:03 +0200, > Rafael J. Wysocki wrote: >> >> On Thursday, July 5, 2018 11:50:11 AM CEST Takashi Iwai wrote: >>> On Thu, 05 Jul 2018 11:34:59 +0200, >>> Rafael J. Wysocki wrote: Hi, On Thu, Jul 5, 2018 at 9:05 AM, Takashi Iwai wrote: > Hi, > > we've got a regression report since 4.17 about the behavior of > power-off with the power button. When a machine is powered off with > the power button on desktop, it reboots after a few seconds instead of > power down. > > The manual power down via "systemctl poweroff" works fine, so it's > possibly some spurious wakeup by the power button action, and some > ACPI-related change is suspected. > The regression still remains in 4.18-rc3. There are only a few ACPI commits directly related to power management between 4.16 and 4.17 and none of them looks particularly suspicious. >>> >>> OK, interesting. >>> It looks like the power button state may not be cleared sufficiently after it's been pressed which is now visible for some reason. >>> >>> Hmm, where can such a state remain? Since it happens after the >>> machine turned off, some (ACPI) wakeup bits? >> >> Basically, yes. >> >> It looks like a GPE may remain active which then triggers wakeup after >> shutdown. >> >> On a hunch, I'm wondering if reverting commit >> >> 18996f2db918 ACPICA: Events: Stop unconditionally clearing ACPI IRQs during >> suspend/resume >> >> (may not revert clearly, though) makes any difference. > > OK, I'm building a 4.17.x test kernel with that revert, in OBS > home:tiwai:bsc1099930 repo. > > Thomas, could you try later the kernel in > http://download.opensuse.org/repositories/home:/tiwai:/bsc1099930/standard/ > ? It'll take an hour or so until the build finishes. With your new built kernel 4.17.4-1.g6f23755-default the power button works again, so the revert solved the problem Thanks Thomas
Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button
Am 05.07.2018 um 14:12 schrieb Takashi Iwai: > On Thu, 05 Jul 2018 12:41:03 +0200, > Rafael J. Wysocki wrote: >> >> On Thursday, July 5, 2018 11:50:11 AM CEST Takashi Iwai wrote: >>> On Thu, 05 Jul 2018 11:34:59 +0200, >>> Rafael J. Wysocki wrote: Hi, On Thu, Jul 5, 2018 at 9:05 AM, Takashi Iwai wrote: > Hi, > > we've got a regression report since 4.17 about the behavior of > power-off with the power button. When a machine is powered off with > the power button on desktop, it reboots after a few seconds instead of > power down. > > The manual power down via "systemctl poweroff" works fine, so it's > possibly some spurious wakeup by the power button action, and some > ACPI-related change is suspected. > The regression still remains in 4.18-rc3. There are only a few ACPI commits directly related to power management between 4.16 and 4.17 and none of them looks particularly suspicious. >>> >>> OK, interesting. >>> It looks like the power button state may not be cleared sufficiently after it's been pressed which is now visible for some reason. >>> >>> Hmm, where can such a state remain? Since it happens after the >>> machine turned off, some (ACPI) wakeup bits? >> >> Basically, yes. >> >> It looks like a GPE may remain active which then triggers wakeup after >> shutdown. >> >> On a hunch, I'm wondering if reverting commit >> >> 18996f2db918 ACPICA: Events: Stop unconditionally clearing ACPI IRQs during >> suspend/resume >> >> (may not revert clearly, though) makes any difference. > > OK, I'm building a 4.17.x test kernel with that revert, in OBS > home:tiwai:bsc1099930 repo. > > Thomas, could you try later the kernel in > http://download.opensuse.org/repositories/home:/tiwai:/bsc1099930/standard/ > ? It'll take an hour or so until the build finishes. With your new built kernel 4.17.4-1.g6f23755-default the power button works again, so the revert solved the problem Thanks Thomas
Re: [PATCH] sched/cputime: Ensure correct utime and stime proportion
On 7/5/18 9:42 PM, Peter Zijlstra wrote:
> On Thu, Jul 05, 2018 at 09:21:15PM +0800, Xunlei Pang wrote:
>> On 7/5/18 6:46 PM, Peter Zijlstra wrote:
>>> On Wed, Jun 27, 2018 at 08:22:42PM +0800, Xunlei Pang wrote:
tick-based whole utime is utime_0, tick-based whole stime
is stime_0, scheduler time is rtime_0.
>>>
For a long time, the process runs mainly in userspace with
run-sleep patterns, and because two different clocks, it
is possible to have the following condition:
rtime_0 < utime_0 (as with little stime_0)
>>>
>>> I don't follow... what?
>>>
>>> Why are you, and why do you think it makes sense to, compare rtime_0
>>> against utime_0 ?
>>>
>>> The [us]time_0 are, per your earlier definition, ticks. They're not an
>>> actual measure of time. Do not compare the two, that makes no bloody
>>> sense.
>>>
>>
>> [us]time_0 is task_struct:utime{stime}, I cited directly from
>> cputime_adjust(), both in nanoseconds. I assumed "rtime_0 < utime_0"
>> here to simple the following proof to help explain the problem we met.
>
> In the !VIRT_CPU_ACCOUNTING case they (task_struct::[us]time) are not
> actual durations. Yes, the happen to be accounted in multiples of
> TICK_NSEC and thereby happen to carry a [ns] unit, but they are not
> durations, they are samples.
>
> (we just happen to store them in a [ns] unit because for
> VIRT_CPU_ACCOUNTING they are in fact durations)
>
> If 'rtime < utime' is not a valid assumption to build a problem on for
> !VIRT_CPU_ACCOUNTING.
>
It is rtime < utime + stime, that is the imprecise tick-based run time
may be larger than precise sum_exec_runtime scheduler-based run time, it
can happen with some frequent run-sleep patterns.
Because stime is usually very small, so it is possible to have rtime <
utime.
>
> So please try again, so far you're not making any sense.
>
I also had a reproducer to verify this patch, can attach it tomorrow.
Re: [PATCH] sched/cputime: Ensure correct utime and stime proportion
On 7/5/18 9:42 PM, Peter Zijlstra wrote:
> On Thu, Jul 05, 2018 at 09:21:15PM +0800, Xunlei Pang wrote:
>> On 7/5/18 6:46 PM, Peter Zijlstra wrote:
>>> On Wed, Jun 27, 2018 at 08:22:42PM +0800, Xunlei Pang wrote:
tick-based whole utime is utime_0, tick-based whole stime
is stime_0, scheduler time is rtime_0.
>>>
For a long time, the process runs mainly in userspace with
run-sleep patterns, and because two different clocks, it
is possible to have the following condition:
rtime_0 < utime_0 (as with little stime_0)
>>>
>>> I don't follow... what?
>>>
>>> Why are you, and why do you think it makes sense to, compare rtime_0
>>> against utime_0 ?
>>>
>>> The [us]time_0 are, per your earlier definition, ticks. They're not an
>>> actual measure of time. Do not compare the two, that makes no bloody
>>> sense.
>>>
>>
>> [us]time_0 is task_struct:utime{stime}, I cited directly from
>> cputime_adjust(), both in nanoseconds. I assumed "rtime_0 < utime_0"
>> here to simple the following proof to help explain the problem we met.
>
> In the !VIRT_CPU_ACCOUNTING case they (task_struct::[us]time) are not
> actual durations. Yes, the happen to be accounted in multiples of
> TICK_NSEC and thereby happen to carry a [ns] unit, but they are not
> durations, they are samples.
>
> (we just happen to store them in a [ns] unit because for
> VIRT_CPU_ACCOUNTING they are in fact durations)
>
> If 'rtime < utime' is not a valid assumption to build a problem on for
> !VIRT_CPU_ACCOUNTING.
>
It is rtime < utime + stime, that is the imprecise tick-based run time
may be larger than precise sum_exec_runtime scheduler-based run time, it
can happen with some frequent run-sleep patterns.
Because stime is usually very small, so it is possible to have rtime <
utime.
>
> So please try again, so far you're not making any sense.
>
I also had a reproducer to verify this patch, can attach it tomorrow.
[PATCH v2 1/2] slimbus: stream: add stream support
This patch adds support to SLIMbus stream apis for slimbus device.
SLIMbus streaming involves adding support to Data Channel Management and
channel Reconfiguration Messages to slim core plus few stream apis.
>From slim device side the apis are very simple mostly inline with other
stream apis.
Currently it only supports Isochronous and Push/Pull transport protocols,
which are sufficient for audio use cases.
Signed-off-by: Srinivas Kandagatla
---
Documentation/driver-api/slimbus.rst | 5 +
drivers/slimbus/Makefile | 2 +-
drivers/slimbus/core.c | 2 +
drivers/slimbus/slimbus.h| 189 ++
drivers/slimbus/stream.c | 477 +++
include/linux/slimbus.h | 46
6 files changed, 720 insertions(+), 1 deletion(-)
create mode 100644 drivers/slimbus/stream.c
diff --git a/Documentation/driver-api/slimbus.rst
b/Documentation/driver-api/slimbus.rst
index a97449cf603a..410eec79b2a1 100644
--- a/Documentation/driver-api/slimbus.rst
+++ b/Documentation/driver-api/slimbus.rst
@@ -125,3 +125,8 @@ Messaging APIs:
~~~
.. kernel-doc:: drivers/slimbus/messaging.c
:export:
+
+Streaming APIs:
+~~~
+.. kernel-doc:: drivers/slimbus/stream.c
+ :export:
diff --git a/drivers/slimbus/Makefile b/drivers/slimbus/Makefile
index c78c6e16c2df..d9aa011b6804 100644
--- a/drivers/slimbus/Makefile
+++ b/drivers/slimbus/Makefile
@@ -3,7 +3,7 @@
# Makefile for kernel SLIMbus framework.
#
obj-$(CONFIG_SLIMBUS) += slimbus.o
-slimbus-y := core.o messaging.o sched.o
+slimbus-y := core.o messaging.o sched.o stream.o
#Controllers
obj-$(CONFIG_SLIM_QCOM_CTRL) += slim-qcom-ctrl.o
diff --git a/drivers/slimbus/core.c b/drivers/slimbus/core.c
index 88248a4ecad9..95b00d28ad6e 100644
--- a/drivers/slimbus/core.c
+++ b/drivers/slimbus/core.c
@@ -114,6 +114,8 @@ static int slim_add_device(struct slim_controller *ctrl,
sbdev->dev.release = slim_dev_release;
sbdev->dev.driver = NULL;
sbdev->ctrl = ctrl;
+ INIT_LIST_HEAD(>stream_list);
+ spin_lock_init(>stream_list_lock);
if (node)
sbdev->dev.of_node = of_node_get(node);
diff --git a/drivers/slimbus/slimbus.h b/drivers/slimbus/slimbus.h
index 63229e8cd050..6dbc2b320704 100644
--- a/drivers/slimbus/slimbus.h
+++ b/drivers/slimbus/slimbus.h
@@ -45,9 +45,20 @@
#define SLIM_MSG_MC_ASSIGN_LOGICAL_ADDRESS 0x2
#define SLIM_MSG_MC_REPORT_ABSENT0xF
+/* Data channel management messages */
+#define SLIM_MSG_MC_CONNECT_SOURCE 0x10
+#define SLIM_MSG_MC_CONNECT_SINK 0x11
+#define SLIM_MSG_MC_DISCONNECT_PORT0x14
+#define SLIM_MSG_MC_CHANGE_CONTENT 0x18
+
/* Clock pause Reconfiguration messages */
#define SLIM_MSG_MC_BEGIN_RECONFIGURATION0x40
#define SLIM_MSG_MC_NEXT_PAUSE_CLOCK 0x4A
+#define SLIM_MSG_MC_NEXT_DEFINE_CHANNEL 0x50
+#define SLIM_MSG_MC_NEXT_DEFINE_CONTENT 0x51
+#define SLIM_MSG_MC_NEXT_ACTIVATE_CHANNEL0x54
+#define SLIM_MSG_MC_NEXT_DEACTIVATE_CHANNEL 0x55
+#define SLIM_MSG_MC_NEXT_REMOVE_CHANNEL 0x58
#define SLIM_MSG_MC_RECONFIGURE_NOW 0x5F
/*
@@ -69,7 +80,15 @@
/* Standard values per SLIMbus spec needed by controllers and devices */
#define SLIM_MAX_CLK_GEAR 10
#define SLIM_MIN_CLK_GEAR 1
+#define SLIM_SLOT_LEN_BITS 4
+
+/* Indicate that the frequency of the flow and the bus frequency are locked */
+#define SLIM_CHANNEL_CONTENT_FLBIT(7)
+/* Standard values per SLIMbus spec needed by controllers and devices */
+#define SLIM_CL_PER_SUPERFRAME 6144
+#define SLIM_SLOTS_PER_SUPERFRAME (SLIM_CL_PER_SUPERFRAME >> 2)
+#define SLIM_SL_PER_SUPERFRAME (SLIM_CL_PER_SUPERFRAME >> 2)
/* Manager's logical address is set to 0xFF per spec */
#define SLIM_LA_MANAGER 0xFF
@@ -167,6 +186,170 @@ struct slim_sched {
struct mutexm_reconf;
};
+/**
+ * enum slim_port_direction: SLIMbus port direction
+ *
+ * @SLIM_PORT_SINK: SLIMbus port is a sink
+ * @SLIM_PORT_SOURCE: SLIMbus port is a source
+ */
+enum slim_port_direction {
+ SLIM_PORT_SINK = 0,
+ SLIM_PORT_SOURCE,
+};
+/**
+ * enum slim_port_state: SLIMbus Port/Endpoint state machine
+ * according to SLIMbus Spec 2.0
+ * @SLIM_PORT_DISCONNECTED: SLIMbus port is disconnected
+ * entered from Unconfigure/configured state after
+ * DISCONNECT_PORT or REMOVE_CHANNEL core command
+ * @SLIM_PORT_UNCONFIGURED: SLIMbus port is in unconfigured state.
+ * entered from disconnect state after CONNECT_SOURCE/SINK core command
+ * @SLIM_PORT_CONFIGURED: SLIMbus port is in configured state.
+ * entered from unconfigured state after DEFINE_CHANNEL, DEFINE_CONTENT
+ * and ACTIVATE_CHANNEL core commands. Ready
[PATCH v2 2/2] slimbus: ngd: add stream support
This patch adds support to stream support, this involve implementing
user specific implementation of Data channel management and channel
management SLIMbus messages.
Signed-off-by: Srinivas Kandagatla
---
drivers/slimbus/qcom-ngd-ctrl.c | 149 +++-
1 file changed, 147 insertions(+), 2 deletions(-)
diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
index 8554e3f43522..8be4d6786c61 100644
--- a/drivers/slimbus/qcom-ngd-ctrl.c
+++ b/drivers/slimbus/qcom-ngd-ctrl.c
@@ -603,7 +603,9 @@ static void qcom_slim_ngd_rx(struct qcom_slim_ngd_ctrl
*ctrl, u8 *buf)
if (mc == SLIM_MSG_MC_REPLY_INFORMATION ||
mc == SLIM_MSG_MC_REPLY_VALUE || (mc == SLIM_USR_MC_ADDR_REPLY &&
- mt == SLIM_MSG_MT_SRC_REFERRED_USER)) {
+ mt == SLIM_MSG_MT_SRC_REFERRED_USER) ||
+ (mc == SLIM_USR_MC_GENERIC_ACK &&
+mt == SLIM_MSG_MT_SRC_REFERRED_USER)) {
slim_msg_response(>ctrl, [4], buf[3], len - 4);
pm_runtime_mark_last_busy(ctrl->dev);
}
@@ -766,10 +768,14 @@ static int qcom_slim_ngd_xfer_msg(struct slim_controller
*sctrl,
{
struct qcom_slim_ngd_ctrl *ctrl = dev_get_drvdata(sctrl->dev);
DECLARE_COMPLETION_ONSTACK(tx_sent);
- int ret, timeout;
+ DECLARE_COMPLETION_ONSTACK(done);
+ int ret, timeout, i;
+ u8 wbuf[SLIM_MSGQ_BUF_LEN];
+ u8 rbuf[SLIM_MSGQ_BUF_LEN];
u32 *pbuf;
u8 *puc;
u8 la = txn->la;
+ bool usr_msg = false;
if (txn->mc & SLIM_MSG_CLK_PAUSE_SEQ_FLG)
return -EPROTONOSUPPORT;
@@ -794,6 +800,48 @@ static int qcom_slim_ngd_xfer_msg(struct slim_controller
*sctrl,
return -ENOMEM;
}
+ if (txn->mt == SLIM_MSG_MT_CORE &&
+ (txn->mc == SLIM_MSG_MC_CONNECT_SOURCE ||
+ txn->mc == SLIM_MSG_MC_CONNECT_SINK ||
+ txn->mc == SLIM_MSG_MC_DISCONNECT_PORT)) {
+ txn->mt = SLIM_MSG_MT_DEST_REFERRED_USER;
+ switch (txn->mc) {
+ case SLIM_MSG_MC_CONNECT_SOURCE:
+ txn->mc = SLIM_USR_MC_CONNECT_SRC;
+ break;
+ case SLIM_MSG_MC_CONNECT_SINK:
+ txn->mc = SLIM_USR_MC_CONNECT_SINK;
+ break;
+ case SLIM_MSG_MC_DISCONNECT_PORT:
+ txn->mc = SLIM_USR_MC_DISCONNECT_PORT;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ usr_msg = true;
+ i = 0;
+ wbuf[i++] = txn->la;
+ la = SLIM_LA_MGR;
+ wbuf[i++] = txn->msg->wbuf[0];
+ if (txn->mc != SLIM_USR_MC_DISCONNECT_PORT)
+ wbuf[i++] = txn->msg->wbuf[1];
+
+ txn->comp =
+ ret = slim_alloc_txn_tid(sctrl, txn);
+ if (ret) {
+ dev_err(ctrl->dev, "Unable to allocate TID\n");
+ return ret;
+ }
+
+ wbuf[i++] = txn->tid;
+
+ txn->msg->num_bytes = i;
+ txn->msg->wbuf = wbuf;
+ txn->msg->rbuf = rbuf;
+ txn->rl = txn->msg->num_bytes + 4;
+ }
+
/* HW expects length field to be excluded */
txn->rl--;
puc = (u8 *)pbuf;
@@ -830,6 +878,15 @@ static int qcom_slim_ngd_xfer_msg(struct slim_controller
*sctrl,
return -ETIMEDOUT;
}
+ if (usr_msg) {
+ timeout = wait_for_completion_timeout(, HZ);
+ if (!timeout) {
+ dev_err(sctrl->dev, "TX timed out:MC:0x%x,mt:0x%x",
+ txn->mc, txn->mt);
+ return -ETIMEDOUT;
+ }
+ }
+
return 0;
}
@@ -856,6 +913,93 @@ static int qcom_slim_ngd_xfer_msg_sync(struct
slim_controller *ctrl,
return 0;
}
+static int qcom_slim_ngd_enable_stream(struct slim_stream_runtime *rt)
+{
+ struct slim_device *sdev = rt->dev;
+ struct slim_controller *ctrl = sdev->ctrl;
+ struct slim_val_inf msg = {0};
+ u8 wbuf[SLIM_MSGQ_BUF_LEN];
+ u8 rbuf[SLIM_MSGQ_BUF_LEN];
+ struct slim_msg_txn txn = {0,};
+ int i, ret;
+
+ txn.mt = SLIM_MSG_MT_DEST_REFERRED_USER;
+ txn.dt = SLIM_MSG_DEST_LOGICALADDR;
+ txn.la = SLIM_LA_MGR;
+ txn.ec = 0;
+ txn.msg =
+ txn.msg->num_bytes = 0;
+ txn.msg->wbuf = wbuf;
+ txn.msg->rbuf = rbuf;
+
+ for (i = 0; i < rt->num_ports; i++) {
+ struct slim_port *port = >ports[i];
+
+ if (txn.msg->num_bytes == 0) {
+ int seg_interval = SLIM_SLOTS_PER_SUPERFRAME/rt->ratem;
+ int exp;
+
+ wbuf[txn.msg->num_bytes++] = sdev->laddr;
+
[PATCH v2 1/2] slimbus: stream: add stream support
This patch adds support to SLIMbus stream apis for slimbus device.
SLIMbus streaming involves adding support to Data Channel Management and
channel Reconfiguration Messages to slim core plus few stream apis.
>From slim device side the apis are very simple mostly inline with other
stream apis.
Currently it only supports Isochronous and Push/Pull transport protocols,
which are sufficient for audio use cases.
Signed-off-by: Srinivas Kandagatla
---
Documentation/driver-api/slimbus.rst | 5 +
drivers/slimbus/Makefile | 2 +-
drivers/slimbus/core.c | 2 +
drivers/slimbus/slimbus.h| 189 ++
drivers/slimbus/stream.c | 477 +++
include/linux/slimbus.h | 46
6 files changed, 720 insertions(+), 1 deletion(-)
create mode 100644 drivers/slimbus/stream.c
diff --git a/Documentation/driver-api/slimbus.rst
b/Documentation/driver-api/slimbus.rst
index a97449cf603a..410eec79b2a1 100644
--- a/Documentation/driver-api/slimbus.rst
+++ b/Documentation/driver-api/slimbus.rst
@@ -125,3 +125,8 @@ Messaging APIs:
~~~
.. kernel-doc:: drivers/slimbus/messaging.c
:export:
+
+Streaming APIs:
+~~~
+.. kernel-doc:: drivers/slimbus/stream.c
+ :export:
diff --git a/drivers/slimbus/Makefile b/drivers/slimbus/Makefile
index c78c6e16c2df..d9aa011b6804 100644
--- a/drivers/slimbus/Makefile
+++ b/drivers/slimbus/Makefile
@@ -3,7 +3,7 @@
# Makefile for kernel SLIMbus framework.
#
obj-$(CONFIG_SLIMBUS) += slimbus.o
-slimbus-y := core.o messaging.o sched.o
+slimbus-y := core.o messaging.o sched.o stream.o
#Controllers
obj-$(CONFIG_SLIM_QCOM_CTRL) += slim-qcom-ctrl.o
diff --git a/drivers/slimbus/core.c b/drivers/slimbus/core.c
index 88248a4ecad9..95b00d28ad6e 100644
--- a/drivers/slimbus/core.c
+++ b/drivers/slimbus/core.c
@@ -114,6 +114,8 @@ static int slim_add_device(struct slim_controller *ctrl,
sbdev->dev.release = slim_dev_release;
sbdev->dev.driver = NULL;
sbdev->ctrl = ctrl;
+ INIT_LIST_HEAD(>stream_list);
+ spin_lock_init(>stream_list_lock);
if (node)
sbdev->dev.of_node = of_node_get(node);
diff --git a/drivers/slimbus/slimbus.h b/drivers/slimbus/slimbus.h
index 63229e8cd050..6dbc2b320704 100644
--- a/drivers/slimbus/slimbus.h
+++ b/drivers/slimbus/slimbus.h
@@ -45,9 +45,20 @@
#define SLIM_MSG_MC_ASSIGN_LOGICAL_ADDRESS 0x2
#define SLIM_MSG_MC_REPORT_ABSENT0xF
+/* Data channel management messages */
+#define SLIM_MSG_MC_CONNECT_SOURCE 0x10
+#define SLIM_MSG_MC_CONNECT_SINK 0x11
+#define SLIM_MSG_MC_DISCONNECT_PORT0x14
+#define SLIM_MSG_MC_CHANGE_CONTENT 0x18
+
/* Clock pause Reconfiguration messages */
#define SLIM_MSG_MC_BEGIN_RECONFIGURATION0x40
#define SLIM_MSG_MC_NEXT_PAUSE_CLOCK 0x4A
+#define SLIM_MSG_MC_NEXT_DEFINE_CHANNEL 0x50
+#define SLIM_MSG_MC_NEXT_DEFINE_CONTENT 0x51
+#define SLIM_MSG_MC_NEXT_ACTIVATE_CHANNEL0x54
+#define SLIM_MSG_MC_NEXT_DEACTIVATE_CHANNEL 0x55
+#define SLIM_MSG_MC_NEXT_REMOVE_CHANNEL 0x58
#define SLIM_MSG_MC_RECONFIGURE_NOW 0x5F
/*
@@ -69,7 +80,15 @@
/* Standard values per SLIMbus spec needed by controllers and devices */
#define SLIM_MAX_CLK_GEAR 10
#define SLIM_MIN_CLK_GEAR 1
+#define SLIM_SLOT_LEN_BITS 4
+
+/* Indicate that the frequency of the flow and the bus frequency are locked */
+#define SLIM_CHANNEL_CONTENT_FLBIT(7)
+/* Standard values per SLIMbus spec needed by controllers and devices */
+#define SLIM_CL_PER_SUPERFRAME 6144
+#define SLIM_SLOTS_PER_SUPERFRAME (SLIM_CL_PER_SUPERFRAME >> 2)
+#define SLIM_SL_PER_SUPERFRAME (SLIM_CL_PER_SUPERFRAME >> 2)
/* Manager's logical address is set to 0xFF per spec */
#define SLIM_LA_MANAGER 0xFF
@@ -167,6 +186,170 @@ struct slim_sched {
struct mutexm_reconf;
};
+/**
+ * enum slim_port_direction: SLIMbus port direction
+ *
+ * @SLIM_PORT_SINK: SLIMbus port is a sink
+ * @SLIM_PORT_SOURCE: SLIMbus port is a source
+ */
+enum slim_port_direction {
+ SLIM_PORT_SINK = 0,
+ SLIM_PORT_SOURCE,
+};
+/**
+ * enum slim_port_state: SLIMbus Port/Endpoint state machine
+ * according to SLIMbus Spec 2.0
+ * @SLIM_PORT_DISCONNECTED: SLIMbus port is disconnected
+ * entered from Unconfigure/configured state after
+ * DISCONNECT_PORT or REMOVE_CHANNEL core command
+ * @SLIM_PORT_UNCONFIGURED: SLIMbus port is in unconfigured state.
+ * entered from disconnect state after CONNECT_SOURCE/SINK core command
+ * @SLIM_PORT_CONFIGURED: SLIMbus port is in configured state.
+ * entered from unconfigured state after DEFINE_CHANNEL, DEFINE_CONTENT
+ * and ACTIVATE_CHANNEL core commands. Ready
[PATCH v2 2/2] slimbus: ngd: add stream support
This patch adds support to stream support, this involve implementing
user specific implementation of Data channel management and channel
management SLIMbus messages.
Signed-off-by: Srinivas Kandagatla
---
drivers/slimbus/qcom-ngd-ctrl.c | 149 +++-
1 file changed, 147 insertions(+), 2 deletions(-)
diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
index 8554e3f43522..8be4d6786c61 100644
--- a/drivers/slimbus/qcom-ngd-ctrl.c
+++ b/drivers/slimbus/qcom-ngd-ctrl.c
@@ -603,7 +603,9 @@ static void qcom_slim_ngd_rx(struct qcom_slim_ngd_ctrl
*ctrl, u8 *buf)
if (mc == SLIM_MSG_MC_REPLY_INFORMATION ||
mc == SLIM_MSG_MC_REPLY_VALUE || (mc == SLIM_USR_MC_ADDR_REPLY &&
- mt == SLIM_MSG_MT_SRC_REFERRED_USER)) {
+ mt == SLIM_MSG_MT_SRC_REFERRED_USER) ||
+ (mc == SLIM_USR_MC_GENERIC_ACK &&
+mt == SLIM_MSG_MT_SRC_REFERRED_USER)) {
slim_msg_response(>ctrl, [4], buf[3], len - 4);
pm_runtime_mark_last_busy(ctrl->dev);
}
@@ -766,10 +768,14 @@ static int qcom_slim_ngd_xfer_msg(struct slim_controller
*sctrl,
{
struct qcom_slim_ngd_ctrl *ctrl = dev_get_drvdata(sctrl->dev);
DECLARE_COMPLETION_ONSTACK(tx_sent);
- int ret, timeout;
+ DECLARE_COMPLETION_ONSTACK(done);
+ int ret, timeout, i;
+ u8 wbuf[SLIM_MSGQ_BUF_LEN];
+ u8 rbuf[SLIM_MSGQ_BUF_LEN];
u32 *pbuf;
u8 *puc;
u8 la = txn->la;
+ bool usr_msg = false;
if (txn->mc & SLIM_MSG_CLK_PAUSE_SEQ_FLG)
return -EPROTONOSUPPORT;
@@ -794,6 +800,48 @@ static int qcom_slim_ngd_xfer_msg(struct slim_controller
*sctrl,
return -ENOMEM;
}
+ if (txn->mt == SLIM_MSG_MT_CORE &&
+ (txn->mc == SLIM_MSG_MC_CONNECT_SOURCE ||
+ txn->mc == SLIM_MSG_MC_CONNECT_SINK ||
+ txn->mc == SLIM_MSG_MC_DISCONNECT_PORT)) {
+ txn->mt = SLIM_MSG_MT_DEST_REFERRED_USER;
+ switch (txn->mc) {
+ case SLIM_MSG_MC_CONNECT_SOURCE:
+ txn->mc = SLIM_USR_MC_CONNECT_SRC;
+ break;
+ case SLIM_MSG_MC_CONNECT_SINK:
+ txn->mc = SLIM_USR_MC_CONNECT_SINK;
+ break;
+ case SLIM_MSG_MC_DISCONNECT_PORT:
+ txn->mc = SLIM_USR_MC_DISCONNECT_PORT;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ usr_msg = true;
+ i = 0;
+ wbuf[i++] = txn->la;
+ la = SLIM_LA_MGR;
+ wbuf[i++] = txn->msg->wbuf[0];
+ if (txn->mc != SLIM_USR_MC_DISCONNECT_PORT)
+ wbuf[i++] = txn->msg->wbuf[1];
+
+ txn->comp =
+ ret = slim_alloc_txn_tid(sctrl, txn);
+ if (ret) {
+ dev_err(ctrl->dev, "Unable to allocate TID\n");
+ return ret;
+ }
+
+ wbuf[i++] = txn->tid;
+
+ txn->msg->num_bytes = i;
+ txn->msg->wbuf = wbuf;
+ txn->msg->rbuf = rbuf;
+ txn->rl = txn->msg->num_bytes + 4;
+ }
+
/* HW expects length field to be excluded */
txn->rl--;
puc = (u8 *)pbuf;
@@ -830,6 +878,15 @@ static int qcom_slim_ngd_xfer_msg(struct slim_controller
*sctrl,
return -ETIMEDOUT;
}
+ if (usr_msg) {
+ timeout = wait_for_completion_timeout(, HZ);
+ if (!timeout) {
+ dev_err(sctrl->dev, "TX timed out:MC:0x%x,mt:0x%x",
+ txn->mc, txn->mt);
+ return -ETIMEDOUT;
+ }
+ }
+
return 0;
}
@@ -856,6 +913,93 @@ static int qcom_slim_ngd_xfer_msg_sync(struct
slim_controller *ctrl,
return 0;
}
+static int qcom_slim_ngd_enable_stream(struct slim_stream_runtime *rt)
+{
+ struct slim_device *sdev = rt->dev;
+ struct slim_controller *ctrl = sdev->ctrl;
+ struct slim_val_inf msg = {0};
+ u8 wbuf[SLIM_MSGQ_BUF_LEN];
+ u8 rbuf[SLIM_MSGQ_BUF_LEN];
+ struct slim_msg_txn txn = {0,};
+ int i, ret;
+
+ txn.mt = SLIM_MSG_MT_DEST_REFERRED_USER;
+ txn.dt = SLIM_MSG_DEST_LOGICALADDR;
+ txn.la = SLIM_LA_MGR;
+ txn.ec = 0;
+ txn.msg =
+ txn.msg->num_bytes = 0;
+ txn.msg->wbuf = wbuf;
+ txn.msg->rbuf = rbuf;
+
+ for (i = 0; i < rt->num_ports; i++) {
+ struct slim_port *port = >ports[i];
+
+ if (txn.msg->num_bytes == 0) {
+ int seg_interval = SLIM_SLOTS_PER_SUPERFRAME/rt->ratem;
+ int exp;
+
+ wbuf[txn.msg->num_bytes++] = sdev->laddr;
+
[PATCH v2 0/2] slimbus: Add Stream Support
Thanks to Vinod and Stephen for reviewing v1 patchset This v2 patchset adds basic stream support for SLIMbus devices and controllers. Mostly inspired by soundwire stream patches. But slimbus stream is much simpler compared to soundwire >From slim_device side, we have below 6 new apis. slim_stream_allocate() - allocating runtime slim stream slim_stream_prepare() - to configure runtime stream with config slim_stream_enable() - enable channels/ports for data slim_stream_disable() - disable channels/ports. slim_stream_unprepare() - un configure runtime stream slim_stream_free() - free all associated memory. >From Controller side: Data channel Management and reconfiguration messages are applicable for all the controllers which are inline with SLIMbus specs. However light weight controller like NGD which have user specific implementation of some messages need to be invoked instead of standard message commands. For usecases like this simple enable/disable stream apis are provided. Assumptions: 1> Current design assumes that the channel and ports are statically allocated to the device during SoC integration, which is the case with all the Qualcomm SoCs. 2> One-to-One mapping between Port and Channel, SLIMBus versions earlier than 2.0 has only one endpoint per port. Current patchset can be extended to support more than one endpoints per port. 3> Only audio usecase, This patchset only supports Isochronous and Push/Pull transport protocols, which are sufficient for audio use cases. 4> DSP does all the data handling for the allocated channels. Which is true for Qcom SoCs. TODO: Bandwidth management. Dependency: This patchset has dependency on the NGD driver https://patchwork.kernel.org/patch/10474959/ Tested this patchset with WCD9335 codec playback and capture on DB820c using 4.18-rc1 with qdsp6 + slim tx patches merged in sound.git. I have pushed my working branch to [1] incase someone want to try. [1]:https://git.linaro.org/people/srinivas.kandagatla/linux.git/log/?h=slimbus-ngd Changes since v1 (https://lwn.net/Articles/758100/): - removed redundant state machine checks for audio usecases, suggested by Vinod. - fixed few typos. - removed irqsave/restore spinlock variant spotted by Vinod. - general cleanups suggested by Stephen and Vinod. thanks, srini Srinivas Kandagatla (2): slimbus: stream: add stream support slimbus: ngd: add stream support Documentation/driver-api/slimbus.rst | 5 + drivers/slimbus/Makefile | 2 +- drivers/slimbus/core.c | 2 + drivers/slimbus/qcom-ngd-ctrl.c | 149 ++- drivers/slimbus/slimbus.h| 189 ++ drivers/slimbus/stream.c | 477 +++ include/linux/slimbus.h | 46 7 files changed, 867 insertions(+), 3 deletions(-) create mode 100644 drivers/slimbus/stream.c -- 2.16.2
[PATCH v2 0/2] slimbus: Add Stream Support
Thanks to Vinod and Stephen for reviewing v1 patchset This v2 patchset adds basic stream support for SLIMbus devices and controllers. Mostly inspired by soundwire stream patches. But slimbus stream is much simpler compared to soundwire >From slim_device side, we have below 6 new apis. slim_stream_allocate() - allocating runtime slim stream slim_stream_prepare() - to configure runtime stream with config slim_stream_enable() - enable channels/ports for data slim_stream_disable() - disable channels/ports. slim_stream_unprepare() - un configure runtime stream slim_stream_free() - free all associated memory. >From Controller side: Data channel Management and reconfiguration messages are applicable for all the controllers which are inline with SLIMbus specs. However light weight controller like NGD which have user specific implementation of some messages need to be invoked instead of standard message commands. For usecases like this simple enable/disable stream apis are provided. Assumptions: 1> Current design assumes that the channel and ports are statically allocated to the device during SoC integration, which is the case with all the Qualcomm SoCs. 2> One-to-One mapping between Port and Channel, SLIMBus versions earlier than 2.0 has only one endpoint per port. Current patchset can be extended to support more than one endpoints per port. 3> Only audio usecase, This patchset only supports Isochronous and Push/Pull transport protocols, which are sufficient for audio use cases. 4> DSP does all the data handling for the allocated channels. Which is true for Qcom SoCs. TODO: Bandwidth management. Dependency: This patchset has dependency on the NGD driver https://patchwork.kernel.org/patch/10474959/ Tested this patchset with WCD9335 codec playback and capture on DB820c using 4.18-rc1 with qdsp6 + slim tx patches merged in sound.git. I have pushed my working branch to [1] incase someone want to try. [1]:https://git.linaro.org/people/srinivas.kandagatla/linux.git/log/?h=slimbus-ngd Changes since v1 (https://lwn.net/Articles/758100/): - removed redundant state machine checks for audio usecases, suggested by Vinod. - fixed few typos. - removed irqsave/restore spinlock variant spotted by Vinod. - general cleanups suggested by Stephen and Vinod. thanks, srini Srinivas Kandagatla (2): slimbus: stream: add stream support slimbus: ngd: add stream support Documentation/driver-api/slimbus.rst | 5 + drivers/slimbus/Makefile | 2 +- drivers/slimbus/core.c | 2 + drivers/slimbus/qcom-ngd-ctrl.c | 149 ++- drivers/slimbus/slimbus.h| 189 ++ drivers/slimbus/stream.c | 477 +++ include/linux/slimbus.h | 46 7 files changed, 867 insertions(+), 3 deletions(-) create mode 100644 drivers/slimbus/stream.c -- 2.16.2
Re: [PATCH v2 1/1] mmc: sunxi: Disable irq during pm_suspend
On 05/07/18 13:07, Ulf Hansson wrote:
> On 5 July 2018 at 13:40, Marc Zyngier wrote:
>> On 05/07/18 12:12, Ulf Hansson wrote:
>>> On 4 July 2018 at 22:29, Marc Zyngier wrote:
On Wed, 4 Jul 2018 15:34:36 +0200
Ulf Hansson wrote:
> On 4 July 2018 at 13:34, Marc Zyngier wrote:
>> On 04/07/18 11:50, Ulf Hansson wrote:
>>> + Marc
>>>
>>> On 4 July 2018 at 08:28, Stefan Mavrodiev wrote:
When mmc host controller enters suspend state, the clocks are
disabled, but irqs are not. For some reason the irqchip emits
false interrupts, which causes system lock loop.
Debug log is:
...
sunxi-mmc 1c11000.mmc: setting clk to 5200, rounded 5120
sunxi-mmc 1c11000.mmc: enabling the clock
sunxi-mmc 1c11000.mmc: cmd 13(814d) arg 1 ie 0xbbc6 len 0
sunxi-mmc 1c11000.mmc: irq: rq (ptrval) mi 0004 idi
sunxi-mmc 1c11000.mmc: cmd 6(8146) arg 3210101 ie 0xbbc6 len 0
sunxi-mmc 1c11000.mmc: irq: rq (ptrval) mi 0004 idi
sunxi-mmc 1c11000.mmc: cmd 13(814d) arg 1 ie 0xbbc6 len 0
sunxi-mmc 1c11000.mmc: irq: rq (ptrval) mi 0004 idi
mmc1: new DDR MMC card at address 0001
mmcblk1: mmc1:0001 AGND3R 14.6 GiB
mmcblk1boot0: mmc1:0001 AGND3R partition 1 4.00 MiB
mmcblk1boot1: mmc1:0001 AGND3R partition 2 4.00 MiB
sunxi-mmc 1c11000.mmc: cmd 18(80003352) arg 0 ie 0xfbc2 len 409
sunxi-mmc 1c11000.mmc: irq: rq (ptrval) mi 4000 idi 0002
mmcblk1: p1
sunxi-mmc 1c11000.mmc: irq: rq (null) mi idi
sunxi-mmc 1c11000.mmc: irq: rq (null) mi idi
sunxi-mmc 1c11000.mmc: irq: rq (null) mi idi
sunxi-mmc 1c11000.mmc: irq: rq (null) mi idi
and so on...
This issue apears on eMMC cards, routed on MMC2 slot. The patch is
tested with A20-OLinuXino-MICRO/LIME/LIME2 boards.
Fixes: 9a8e1e8cc2c0 ("mmc: sunxi: Add runtime_pm support")
Signed-off-by: Stefan Mavrodiev
---
Changes in v2:
- Add comment why disable_irq() is necessary
---
drivers/mmc/host/sunxi-mmc.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/mmc/host/sunxi-mmc.c
b/drivers/mmc/host/sunxi-mmc.c
index e747259..8e7f3e3 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -1446,6 +1446,7 @@ static int sunxi_mmc_runtime_resume(struct
device *dev)
sunxi_mmc_init_host(host);
sunxi_mmc_set_bus_width(host, mmc->ios.bus_width);
sunxi_mmc_set_clk(host, >ios);
+ enable_irq(host->irq);
return 0;
}
@@ -1455,6 +1456,12 @@ static int sunxi_mmc_runtime_suspend(struct
device *dev)
struct mmc_host *mmc = dev_get_drvdata(dev);
struct sunxi_mmc_host *host = mmc_priv(mmc);
+ /*
+* When clocks are off, it's possible receiving
+* fake interrupts, which will stall the system.
+* Disabling the irq will prevent this.
+*/
+ disable_irq(host->irq);
>>>
>>> No, this doesn't work for shared IRQs.
>>
>> Well, in this case, it does work, because that interrupt line cannot be
>> shared with anything else, if I understand how the SoC is wired: each
>> MMC controller has a dedicated interrupt line to the GIC, and it isn't
>> shared with anything (that's on the A20 though, and I don't know about
>> other SoCs integrating the same IP).
>
> That's the problem. This may work on some SoCs but not on others.
>
>>
>>>
sunxi_mmc_reset_host(host);
sunxi_mmc_disable(host);
--
2.7.4
>>>
>>> The only option today is to use free_irq() in runtime suspend and then
>>> re-request the irq to re-install the handler at runtime resume.
>>>
>>> That's not an optimal solution, which is pointed out in the below
>>> discussion as well. Moreover, it has also turned out using free_irq()
>>> is also problematic in cases threaded handlers are used.
>>>
>>> Here's the link to the discussion, it's not the only one I know of, so
>>> this is common problem.
>>> https://lkml.org/lkml/2016/1/28/213
>>>
>>> Care to have a hack on the "common" solution, which in principle means
>>> adding APIs to genirq that can disable/enable handlers from being
>>> called, rather than the entire IRQ line.
>>
>> That doesn't work.
Re: [PATCH v2 1/1] mmc: sunxi: Disable irq during pm_suspend
On 05/07/18 13:07, Ulf Hansson wrote:
> On 5 July 2018 at 13:40, Marc Zyngier wrote:
>> On 05/07/18 12:12, Ulf Hansson wrote:
>>> On 4 July 2018 at 22:29, Marc Zyngier wrote:
On Wed, 4 Jul 2018 15:34:36 +0200
Ulf Hansson wrote:
> On 4 July 2018 at 13:34, Marc Zyngier wrote:
>> On 04/07/18 11:50, Ulf Hansson wrote:
>>> + Marc
>>>
>>> On 4 July 2018 at 08:28, Stefan Mavrodiev wrote:
When mmc host controller enters suspend state, the clocks are
disabled, but irqs are not. For some reason the irqchip emits
false interrupts, which causes system lock loop.
Debug log is:
...
sunxi-mmc 1c11000.mmc: setting clk to 5200, rounded 5120
sunxi-mmc 1c11000.mmc: enabling the clock
sunxi-mmc 1c11000.mmc: cmd 13(814d) arg 1 ie 0xbbc6 len 0
sunxi-mmc 1c11000.mmc: irq: rq (ptrval) mi 0004 idi
sunxi-mmc 1c11000.mmc: cmd 6(8146) arg 3210101 ie 0xbbc6 len 0
sunxi-mmc 1c11000.mmc: irq: rq (ptrval) mi 0004 idi
sunxi-mmc 1c11000.mmc: cmd 13(814d) arg 1 ie 0xbbc6 len 0
sunxi-mmc 1c11000.mmc: irq: rq (ptrval) mi 0004 idi
mmc1: new DDR MMC card at address 0001
mmcblk1: mmc1:0001 AGND3R 14.6 GiB
mmcblk1boot0: mmc1:0001 AGND3R partition 1 4.00 MiB
mmcblk1boot1: mmc1:0001 AGND3R partition 2 4.00 MiB
sunxi-mmc 1c11000.mmc: cmd 18(80003352) arg 0 ie 0xfbc2 len 409
sunxi-mmc 1c11000.mmc: irq: rq (ptrval) mi 4000 idi 0002
mmcblk1: p1
sunxi-mmc 1c11000.mmc: irq: rq (null) mi idi
sunxi-mmc 1c11000.mmc: irq: rq (null) mi idi
sunxi-mmc 1c11000.mmc: irq: rq (null) mi idi
sunxi-mmc 1c11000.mmc: irq: rq (null) mi idi
and so on...
This issue apears on eMMC cards, routed on MMC2 slot. The patch is
tested with A20-OLinuXino-MICRO/LIME/LIME2 boards.
Fixes: 9a8e1e8cc2c0 ("mmc: sunxi: Add runtime_pm support")
Signed-off-by: Stefan Mavrodiev
---
Changes in v2:
- Add comment why disable_irq() is necessary
---
drivers/mmc/host/sunxi-mmc.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/mmc/host/sunxi-mmc.c
b/drivers/mmc/host/sunxi-mmc.c
index e747259..8e7f3e3 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -1446,6 +1446,7 @@ static int sunxi_mmc_runtime_resume(struct
device *dev)
sunxi_mmc_init_host(host);
sunxi_mmc_set_bus_width(host, mmc->ios.bus_width);
sunxi_mmc_set_clk(host, >ios);
+ enable_irq(host->irq);
return 0;
}
@@ -1455,6 +1456,12 @@ static int sunxi_mmc_runtime_suspend(struct
device *dev)
struct mmc_host *mmc = dev_get_drvdata(dev);
struct sunxi_mmc_host *host = mmc_priv(mmc);
+ /*
+* When clocks are off, it's possible receiving
+* fake interrupts, which will stall the system.
+* Disabling the irq will prevent this.
+*/
+ disable_irq(host->irq);
>>>
>>> No, this doesn't work for shared IRQs.
>>
>> Well, in this case, it does work, because that interrupt line cannot be
>> shared with anything else, if I understand how the SoC is wired: each
>> MMC controller has a dedicated interrupt line to the GIC, and it isn't
>> shared with anything (that's on the A20 though, and I don't know about
>> other SoCs integrating the same IP).
>
> That's the problem. This may work on some SoCs but not on others.
>
>>
>>>
sunxi_mmc_reset_host(host);
sunxi_mmc_disable(host);
--
2.7.4
>>>
>>> The only option today is to use free_irq() in runtime suspend and then
>>> re-request the irq to re-install the handler at runtime resume.
>>>
>>> That's not an optimal solution, which is pointed out in the below
>>> discussion as well. Moreover, it has also turned out using free_irq()
>>> is also problematic in cases threaded handlers are used.
>>>
>>> Here's the link to the discussion, it's not the only one I know of, so
>>> this is common problem.
>>> https://lkml.org/lkml/2016/1/28/213
>>>
>>> Care to have a hack on the "common" solution, which in principle means
>>> adding APIs to genirq that can disable/enable handlers from being
>>> called, rather than the entire IRQ line.
>>
>> That doesn't work.
Re: [PATCHv3 0/5] pinctrl fixes for generic functions and groups
On Thu, Jul 5, 2018 at 12:10 PM, Tony Lindgren wrote: > Here is a resend of fixes for a race issues for generic group and > functions reported by H. Nikolaus Schaller . > Reviewed-by: Andy Shevchenko I think for ACPI case we would use generic helpers, so this change is quite useful to not screw up with new stuff. > Regards, > > Tony > > > Changes since v2: > > - Added Nikolaus to Cc as Reported-by did not do it > > Changes since v1: > > - Check if a function or group already exists as suggested by > Andy Shevchenko > > - Make sure we always have a valid name for functions and groups > as suggested by Christ van Willegen > > - Prettify rza1 variables as suggested by Jacopo Mondi > > Tony Lindgren (5): > pinctrl: core: Return selector to the pinctrl driver > pinctrl: pinmux: Return selector to the pinctrl driver > pinctrl: single: Fix group and function selector use > pinctrl: rza1: Fix selector use for groups and functions > pinctrl: core: Remove broken remove_last group and pinmux functions > > drivers/pinctrl/core.c | 35 ++-- > drivers/pinctrl/core.h | 6 --- > drivers/pinctrl/pinctrl-rza1.c | 24 + > drivers/pinctrl/pinctrl-single.c | 91 +++- > drivers/pinctrl/pinmux.c | 16 -- > drivers/pinctrl/pinmux.h | 7 --- > 6 files changed, 112 insertions(+), 67 deletions(-) > > -- > 2.17.1 -- With Best Regards, Andy Shevchenko
Re: [PATCH 10/19] mmc: mmci: add variant property to allow remain data
On 12 June 2018 at 15:14, Ludovic Barre wrote:
> From: Ludovic Barre
>
> This patch adds a boolean property to read remaining data.
> Needed to support the STM32 sdmmc variant. MMCIDATACNT
> register should be read only after the data transfer is complete.
> When reading after an error event the read data count value may be
> different from the real number of data bytes transferred.
>
> Signed-off-by: Ludovic Barre
> ---
> drivers/mmc/host/mmci.c | 17 +++--
> drivers/mmc/host/mmci.h | 3 +++
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 83c3572..abddad7 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -58,6 +58,7 @@ static struct variant_data variant_arm = {
> .datalength_bits= 16,
> .datactrl_blocksz = 11,
> .datactrl_dpsm_enable = MCI_DPSM_ENABLE,
> + .datacnt_remain = true,
According to the description in the changelog, it sounds like
MMCIDATACNT is broken (or useless for the STM32 variant). I suggest to
rename the new variant to something along those lines instead.
Like "datacnt_broken", then you only need to set it to true for the
STM32 variant and legacy variants can remain as is.
> .pwrreg_powerup = MCI_PWR_UP,
> .f_max = 1,
> .reversed_irq_handling = true,
> @@ -78,6 +79,7 @@ static struct variant_data variant_arm_extended_fifo = {
> .datalength_bits= 16,
> .datactrl_blocksz = 11,
> .datactrl_dpsm_enable = MCI_DPSM_ENABLE,
> + .datacnt_remain = true,
> .pwrreg_powerup = MCI_PWR_UP,
> .f_max = 1,
> .mmcimask1 = true,
> @@ -98,6 +100,7 @@ static struct variant_data variant_arm_extended_fifo_hwfc
> = {
> .datalength_bits= 16,
> .datactrl_blocksz = 11,
> .datactrl_dpsm_enable = MCI_DPSM_ENABLE,
> + .datacnt_remain = true,
> .pwrreg_powerup = MCI_PWR_UP,
> .f_max = 1,
> .mmcimask1 = true,
> @@ -120,6 +123,7 @@ static struct variant_data variant_u300 = {
> .datactrl_blocksz = 11,
> .datactrl_dpsm_enable = MCI_DPSM_ENABLE,
> .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,
> + .datacnt_remain = true,
> .st_sdio= true,
> .pwrreg_powerup = MCI_PWR_ON,
> .f_max = 1,
> @@ -146,6 +150,7 @@ static struct variant_data variant_nomadik = {
> .datactrl_blocksz = 11,
> .datactrl_dpsm_enable = MCI_DPSM_ENABLE,
> .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,
> + .datacnt_remain = true,
> .st_sdio= true,
> .st_clkdiv = true,
> .pwrreg_powerup = MCI_PWR_ON,
> @@ -175,6 +180,7 @@ static struct variant_data variant_ux500 = {
> .datactrl_blocksz = 11,
> .datactrl_dpsm_enable = MCI_DPSM_ENABLE,
> .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,
> + .datacnt_remain = true,
> .st_sdio= true,
> .st_clkdiv = true,
> .pwrreg_powerup = MCI_PWR_ON,
> @@ -209,6 +215,7 @@ static struct variant_data variant_ux500v2 = {
> .datactrl_blocksz = 11,
> .datactrl_dpsm_enable = MCI_DPSM_ENABLE,
> .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,
> + .datacnt_remain = true,
> .st_sdio= true,
> .st_clkdiv = true,
> .blksz_datactrl16 = true,
> @@ -244,6 +251,7 @@ static struct variant_data variant_stm32 = {
> .datactrl_blocksz = 11,
> .datactrl_dpsm_enable = MCI_DPSM_ENABLE,
> .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,
> + .datacnt_remain = true,
> .st_sdio= true,
> .st_clkdiv = true,
> .pwrreg_powerup = MCI_PWR_ON,
> @@ -270,6 +278,7 @@ static struct variant_data variant_qcom = {
> .datalength_bits= 24,
> .datactrl_blocksz = 11,
> .datactrl_dpsm_enable = MCI_DPSM_ENABLE,
> + .datacnt_remain = true,
> .pwrreg_powerup = MCI_PWR_UP,
> .f_max = 20800,
> .explicit_mclk_control = true,
> @@ -711,8 +720,12 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data
> *data,
> * can be as much as a FIFO-worth of data ahead. This
> * matters for FIFO overruns only.
> */
> - remain = readl(host->base + MMCIDATACNT);
> - success = data->blksz * data->blocks - remain;
> + if (host->variant->datacnt_remain) {
> + remain =
Re: [PATCHv3 0/5] pinctrl fixes for generic functions and groups
On Thu, Jul 5, 2018 at 12:10 PM, Tony Lindgren wrote: > Here is a resend of fixes for a race issues for generic group and > functions reported by H. Nikolaus Schaller . > Reviewed-by: Andy Shevchenko I think for ACPI case we would use generic helpers, so this change is quite useful to not screw up with new stuff. > Regards, > > Tony > > > Changes since v2: > > - Added Nikolaus to Cc as Reported-by did not do it > > Changes since v1: > > - Check if a function or group already exists as suggested by > Andy Shevchenko > > - Make sure we always have a valid name for functions and groups > as suggested by Christ van Willegen > > - Prettify rza1 variables as suggested by Jacopo Mondi > > Tony Lindgren (5): > pinctrl: core: Return selector to the pinctrl driver > pinctrl: pinmux: Return selector to the pinctrl driver > pinctrl: single: Fix group and function selector use > pinctrl: rza1: Fix selector use for groups and functions > pinctrl: core: Remove broken remove_last group and pinmux functions > > drivers/pinctrl/core.c | 35 ++-- > drivers/pinctrl/core.h | 6 --- > drivers/pinctrl/pinctrl-rza1.c | 24 + > drivers/pinctrl/pinctrl-single.c | 91 +++- > drivers/pinctrl/pinmux.c | 16 -- > drivers/pinctrl/pinmux.h | 7 --- > 6 files changed, 112 insertions(+), 67 deletions(-) > > -- > 2.17.1 -- With Best Regards, Andy Shevchenko
Re: [PATCH 10/19] mmc: mmci: add variant property to allow remain data
On 12 June 2018 at 15:14, Ludovic Barre wrote:
> From: Ludovic Barre
>
> This patch adds a boolean property to read remaining data.
> Needed to support the STM32 sdmmc variant. MMCIDATACNT
> register should be read only after the data transfer is complete.
> When reading after an error event the read data count value may be
> different from the real number of data bytes transferred.
>
> Signed-off-by: Ludovic Barre
> ---
> drivers/mmc/host/mmci.c | 17 +++--
> drivers/mmc/host/mmci.h | 3 +++
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 83c3572..abddad7 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -58,6 +58,7 @@ static struct variant_data variant_arm = {
> .datalength_bits= 16,
> .datactrl_blocksz = 11,
> .datactrl_dpsm_enable = MCI_DPSM_ENABLE,
> + .datacnt_remain = true,
According to the description in the changelog, it sounds like
MMCIDATACNT is broken (or useless for the STM32 variant). I suggest to
rename the new variant to something along those lines instead.
Like "datacnt_broken", then you only need to set it to true for the
STM32 variant and legacy variants can remain as is.
> .pwrreg_powerup = MCI_PWR_UP,
> .f_max = 1,
> .reversed_irq_handling = true,
> @@ -78,6 +79,7 @@ static struct variant_data variant_arm_extended_fifo = {
> .datalength_bits= 16,
> .datactrl_blocksz = 11,
> .datactrl_dpsm_enable = MCI_DPSM_ENABLE,
> + .datacnt_remain = true,
> .pwrreg_powerup = MCI_PWR_UP,
> .f_max = 1,
> .mmcimask1 = true,
> @@ -98,6 +100,7 @@ static struct variant_data variant_arm_extended_fifo_hwfc
> = {
> .datalength_bits= 16,
> .datactrl_blocksz = 11,
> .datactrl_dpsm_enable = MCI_DPSM_ENABLE,
> + .datacnt_remain = true,
> .pwrreg_powerup = MCI_PWR_UP,
> .f_max = 1,
> .mmcimask1 = true,
> @@ -120,6 +123,7 @@ static struct variant_data variant_u300 = {
> .datactrl_blocksz = 11,
> .datactrl_dpsm_enable = MCI_DPSM_ENABLE,
> .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,
> + .datacnt_remain = true,
> .st_sdio= true,
> .pwrreg_powerup = MCI_PWR_ON,
> .f_max = 1,
> @@ -146,6 +150,7 @@ static struct variant_data variant_nomadik = {
> .datactrl_blocksz = 11,
> .datactrl_dpsm_enable = MCI_DPSM_ENABLE,
> .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,
> + .datacnt_remain = true,
> .st_sdio= true,
> .st_clkdiv = true,
> .pwrreg_powerup = MCI_PWR_ON,
> @@ -175,6 +180,7 @@ static struct variant_data variant_ux500 = {
> .datactrl_blocksz = 11,
> .datactrl_dpsm_enable = MCI_DPSM_ENABLE,
> .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,
> + .datacnt_remain = true,
> .st_sdio= true,
> .st_clkdiv = true,
> .pwrreg_powerup = MCI_PWR_ON,
> @@ -209,6 +215,7 @@ static struct variant_data variant_ux500v2 = {
> .datactrl_blocksz = 11,
> .datactrl_dpsm_enable = MCI_DPSM_ENABLE,
> .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,
> + .datacnt_remain = true,
> .st_sdio= true,
> .st_clkdiv = true,
> .blksz_datactrl16 = true,
> @@ -244,6 +251,7 @@ static struct variant_data variant_stm32 = {
> .datactrl_blocksz = 11,
> .datactrl_dpsm_enable = MCI_DPSM_ENABLE,
> .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,
> + .datacnt_remain = true,
> .st_sdio= true,
> .st_clkdiv = true,
> .pwrreg_powerup = MCI_PWR_ON,
> @@ -270,6 +278,7 @@ static struct variant_data variant_qcom = {
> .datalength_bits= 24,
> .datactrl_blocksz = 11,
> .datactrl_dpsm_enable = MCI_DPSM_ENABLE,
> + .datacnt_remain = true,
> .pwrreg_powerup = MCI_PWR_UP,
> .f_max = 20800,
> .explicit_mclk_control = true,
> @@ -711,8 +720,12 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data
> *data,
> * can be as much as a FIFO-worth of data ahead. This
> * matters for FIFO overruns only.
> */
> - remain = readl(host->base + MMCIDATACNT);
> - success = data->blksz * data->blocks - remain;
> + if (host->variant->datacnt_remain) {
> + remain =
Re: [PATCH] KVM: Switch 'requests' to be 64-bit (explicitly)
On Sun, Apr 15, 2018 at 12:26:44AM +0200, KarimAllah Ahmed wrote:
> Switch 'requests' to be explicitly 64-bit and update BUILD_BUG_ON check to
> use the size of "requests" instead of the hard-coded '32'.
>
> That gives us a bit more room again for arch-specific requests as we
> already ran out of space for x86 due to the hard-coded check.
>
> Cc: Paolo Bonzini
> Cc: Radim Krčmář
> Cc: k...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: KarimAllah Ahmed
> ---
> include/linux/kvm_host.h | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 6930c63..fe4f46b 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -129,7 +129,7 @@ static inline bool is_error_page(struct page *page)
> #define KVM_REQUEST_ARCH_BASE 8
>
> #define KVM_ARCH_REQ_FLAGS(nr, flags) ({ \
> - BUILD_BUG_ON((unsigned)(nr) >= 32 - KVM_REQUEST_ARCH_BASE); \
> + BUILD_BUG_ON((unsigned)(nr) >= (sizeof(((struct kvm_vcpu
> *)0)->requests) * 8) - KVM_REQUEST_ARCH_BASE); \
> (unsigned)(((nr) + KVM_REQUEST_ARCH_BASE) | (flags)); \
> })
> #define KVM_ARCH_REQ(nr) KVM_ARCH_REQ_FLAGS(nr, 0)
> @@ -223,7 +223,7 @@ struct kvm_vcpu {
> int vcpu_id;
> int srcu_idx;
> int mode;
> - unsigned long requests;
> + u64 requests;
The usual thing to do for bitmaps is something like:
#define KVM_REQUEST_NR (KVM_REQUEST_ARCH_BASE + KVM_REQUEST_ARCH_NR)
unsigned long requests[BITS_TO_LONGS(NR_KVM_REQUESTS)];
> unsigned long guest_debug;
>
> int pre_pcpu;
> @@ -1122,7 +1122,7 @@ static inline void kvm_make_request(int req, struct
> kvm_vcpu *vcpu)
>* caller. Paired with the smp_mb__after_atomic in kvm_check_request.
>*/
> smp_wmb();
> - set_bit(req & KVM_REQUEST_MASK, >requests);
> + set_bit(req & KVM_REQUEST_MASK, (void *)>requests);
... which wouldn't require a void cast to make the bit API functions
happy (as these expect a pointer to the first unsigned long in the
bitmap).
Thanks,
Mark.
Re: [PATCH] KVM: Switch 'requests' to be 64-bit (explicitly)
On Sun, Apr 15, 2018 at 12:26:44AM +0200, KarimAllah Ahmed wrote:
> Switch 'requests' to be explicitly 64-bit and update BUILD_BUG_ON check to
> use the size of "requests" instead of the hard-coded '32'.
>
> That gives us a bit more room again for arch-specific requests as we
> already ran out of space for x86 due to the hard-coded check.
>
> Cc: Paolo Bonzini
> Cc: Radim Krčmář
> Cc: k...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: KarimAllah Ahmed
> ---
> include/linux/kvm_host.h | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 6930c63..fe4f46b 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -129,7 +129,7 @@ static inline bool is_error_page(struct page *page)
> #define KVM_REQUEST_ARCH_BASE 8
>
> #define KVM_ARCH_REQ_FLAGS(nr, flags) ({ \
> - BUILD_BUG_ON((unsigned)(nr) >= 32 - KVM_REQUEST_ARCH_BASE); \
> + BUILD_BUG_ON((unsigned)(nr) >= (sizeof(((struct kvm_vcpu
> *)0)->requests) * 8) - KVM_REQUEST_ARCH_BASE); \
> (unsigned)(((nr) + KVM_REQUEST_ARCH_BASE) | (flags)); \
> })
> #define KVM_ARCH_REQ(nr) KVM_ARCH_REQ_FLAGS(nr, 0)
> @@ -223,7 +223,7 @@ struct kvm_vcpu {
> int vcpu_id;
> int srcu_idx;
> int mode;
> - unsigned long requests;
> + u64 requests;
The usual thing to do for bitmaps is something like:
#define KVM_REQUEST_NR (KVM_REQUEST_ARCH_BASE + KVM_REQUEST_ARCH_NR)
unsigned long requests[BITS_TO_LONGS(NR_KVM_REQUESTS)];
> unsigned long guest_debug;
>
> int pre_pcpu;
> @@ -1122,7 +1122,7 @@ static inline void kvm_make_request(int req, struct
> kvm_vcpu *vcpu)
>* caller. Paired with the smp_mb__after_atomic in kvm_check_request.
>*/
> smp_wmb();
> - set_bit(req & KVM_REQUEST_MASK, >requests);
> + set_bit(req & KVM_REQUEST_MASK, (void *)>requests);
... which wouldn't require a void cast to make the bit API functions
happy (as these expect a pointer to the first unsigned long in the
bitmap).
Thanks,
Mark.
Re: [PATCH] perf: disable parallelism for 'make clean'
On Thu, Jul 05, 2018 at 03:15:27PM +0200, Rasmus Villemoes wrote: > The Yocto build system does a 'make clean' when rebuilding due to > changed dependencies, and that consistently fails for me (causing the > whole BSP build to fail) with errors such as > > | find: '[...]/perf/1.0-r9/perf-1.0/plugin_mac80211.so': No such file or > directory > | find: '[...]/perf/1.0-r9/perf-1.0/plugin_mac80211.so': No such file or > directory > | find: find: > '[...]/perf/1.0-r9/perf-1.0/libtraceevent.a''[...]/perf/1.0-r9/perf-1.0/libtraceevent.a': > No such file or directory: No such file or directory > | > [...] > | find: cannot delete > '/mnt/xfs/devel/pil/yocto/tmp-glibc/work/wandboard-oe-linux-gnueabi/perf/1.0-r9/perf-1.0/util/.pstack.o.cmd': > No such file or directory > > Apparently (despite the comment), 'make clean' ends up launching > multiple sub-makes that all want to remove the same things - perhaps > this only happens in combination with a O=... parameter. In any case, we > don't lose much by explicitly disabling the parallelism for the clean > target, and it makes automated builds much more reliable. > > Signed-off-by: Rasmus Villemoes Acked-by: Jiri Olsa thanks, jirka
Re: [PATCH] perf: disable parallelism for 'make clean'
On Thu, Jul 05, 2018 at 03:15:27PM +0200, Rasmus Villemoes wrote: > The Yocto build system does a 'make clean' when rebuilding due to > changed dependencies, and that consistently fails for me (causing the > whole BSP build to fail) with errors such as > > | find: '[...]/perf/1.0-r9/perf-1.0/plugin_mac80211.so': No such file or > directory > | find: '[...]/perf/1.0-r9/perf-1.0/plugin_mac80211.so': No such file or > directory > | find: find: > '[...]/perf/1.0-r9/perf-1.0/libtraceevent.a''[...]/perf/1.0-r9/perf-1.0/libtraceevent.a': > No such file or directory: No such file or directory > | > [...] > | find: cannot delete > '/mnt/xfs/devel/pil/yocto/tmp-glibc/work/wandboard-oe-linux-gnueabi/perf/1.0-r9/perf-1.0/util/.pstack.o.cmd': > No such file or directory > > Apparently (despite the comment), 'make clean' ends up launching > multiple sub-makes that all want to remove the same things - perhaps > this only happens in combination with a O=... parameter. In any case, we > don't lose much by explicitly disabling the parallelism for the clean > target, and it makes automated builds much more reliable. > > Signed-off-by: Rasmus Villemoes Acked-by: Jiri Olsa thanks, jirka
Re: [PATCH 05/19] mmc: mmci: allow to overwrite clock/power procedure to specific variant
On 12 June 2018 at 15:14, Ludovic Barre wrote:
> From: Ludovic Barre
>
> A specific variant could have different power or clock procedures.
> This patch allows to overwrite the default mmci_set_clkreg and
> mmci_set_pwrreg for a specific variant.
>
> Signed-off-by: Ludovic Barre
> ---
> drivers/mmc/host/mmci.c | 96
> +
> drivers/mmc/host/mmci.h | 7
> 2 files changed, 64 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index ede95b7..801c86b 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -374,6 +374,52 @@ static void mmci_set_clkreg(struct mmci_host *host,
> unsigned int desired)
> mmci_write_clkreg(host, clk);
> }
>
> +static void mmci_set_pwrreg(struct mmci_host *host, unsigned char power_mode,
> + unsigned int pwr)
> +{
> + struct variant_data *variant = host->variant;
> + struct mmc_host *mmc = host->mmc;
> +
> + switch (power_mode) {
> + case MMC_POWER_OFF:
> + if (!IS_ERR(mmc->supply.vmmc))
> + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
> +
> + if (!IS_ERR(mmc->supply.vqmmc) && host->vqmmc_enabled) {
> + regulator_disable(mmc->supply.vqmmc);
> + host->vqmmc_enabled = false;
> + }
> +
> + break;
> + case MMC_POWER_UP:
> + if (!IS_ERR(mmc->supply.vmmc))
> + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
> + mmc->ios.vdd);
> +
> + /*
> +* The ST Micro variant doesn't have the PL180s MCI_PWR_UP
> +* and instead uses MCI_PWR_ON so apply whatever value is
> +* configured in the variant data.
> +*/
> + pwr |= variant->pwrreg_powerup;
> +
> + break;
> + case MMC_POWER_ON:
> + if (!IS_ERR(mmc->supply.vqmmc) && !host->vqmmc_enabled) {
> + if (regulator_enable(mmc->supply.vqmmc) < 0)
> + dev_err(mmc_dev(mmc),
> + "failed to enable vqmmc regulator\n");
> + else
> + host->vqmmc_enabled = true;
> + }
> +
> + pwr |= MCI_PWR_ON;
> + break;
> + }
> +
> + mmci_write_pwrreg(host, pwr);
> +}
> +
> static void
> mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
> {
> @@ -1031,7 +1077,7 @@ static void mmci_set_ios(struct mmc_host *mmc, struct
> mmc_ios *ios)
> {
> struct mmci_host *host = mmc_priv(mmc);
> struct variant_data *variant = host->variant;
> - u32 pwr = 0;
> + unsigned int pwr = 0;
?
> unsigned long flags;
> int ret;
>
> @@ -1039,42 +1085,6 @@ static void mmci_set_ios(struct mmc_host *mmc, struct
> mmc_ios *ios)
> host->plat->ios_handler(mmc_dev(mmc), ios))
> dev_err(mmc_dev(mmc), "platform ios_handler
> failed\n");
>
> - switch (ios->power_mode) {
> - case MMC_POWER_OFF:
> - if (!IS_ERR(mmc->supply.vmmc))
> - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
> -
> - if (!IS_ERR(mmc->supply.vqmmc) && host->vqmmc_enabled) {
> - regulator_disable(mmc->supply.vqmmc);
> - host->vqmmc_enabled = false;
> - }
> -
> - break;
> - case MMC_POWER_UP:
> - if (!IS_ERR(mmc->supply.vmmc))
> - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
> ios->vdd);
> -
> - /*
> -* The ST Micro variant doesn't have the PL180s MCI_PWR_UP
> -* and instead uses MCI_PWR_ON so apply whatever value is
> -* configured in the variant data.
> -*/
> - pwr |= variant->pwrreg_powerup;
> -
> - break;
> - case MMC_POWER_ON:
> - if (!IS_ERR(mmc->supply.vqmmc) && !host->vqmmc_enabled) {
> - ret = regulator_enable(mmc->supply.vqmmc);
> - if (ret < 0)
> - dev_err(mmc_dev(mmc),
> - "failed to enable vqmmc regulator\n");
> - else
> - host->vqmmc_enabled = true;
> - }
> -
> - pwr |= MCI_PWR_ON;
> - break;
> - }
This above looks like pure re-factoring. Please make the above change
a separate patch.
>
> if (variant->signal_direction && ios->power_mode != MMC_POWER_OFF) {
> /*
> @@ -1126,8 +1136,16 @@ static void mmci_set_ios(struct mmc_host *mmc, struct
> mmc_ios *ios)
>
> spin_lock_irqsave(>lock,
Re: [PATCH 05/19] mmc: mmci: allow to overwrite clock/power procedure to specific variant
On 12 June 2018 at 15:14, Ludovic Barre wrote:
> From: Ludovic Barre
>
> A specific variant could have different power or clock procedures.
> This patch allows to overwrite the default mmci_set_clkreg and
> mmci_set_pwrreg for a specific variant.
>
> Signed-off-by: Ludovic Barre
> ---
> drivers/mmc/host/mmci.c | 96
> +
> drivers/mmc/host/mmci.h | 7
> 2 files changed, 64 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index ede95b7..801c86b 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -374,6 +374,52 @@ static void mmci_set_clkreg(struct mmci_host *host,
> unsigned int desired)
> mmci_write_clkreg(host, clk);
> }
>
> +static void mmci_set_pwrreg(struct mmci_host *host, unsigned char power_mode,
> + unsigned int pwr)
> +{
> + struct variant_data *variant = host->variant;
> + struct mmc_host *mmc = host->mmc;
> +
> + switch (power_mode) {
> + case MMC_POWER_OFF:
> + if (!IS_ERR(mmc->supply.vmmc))
> + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
> +
> + if (!IS_ERR(mmc->supply.vqmmc) && host->vqmmc_enabled) {
> + regulator_disable(mmc->supply.vqmmc);
> + host->vqmmc_enabled = false;
> + }
> +
> + break;
> + case MMC_POWER_UP:
> + if (!IS_ERR(mmc->supply.vmmc))
> + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
> + mmc->ios.vdd);
> +
> + /*
> +* The ST Micro variant doesn't have the PL180s MCI_PWR_UP
> +* and instead uses MCI_PWR_ON so apply whatever value is
> +* configured in the variant data.
> +*/
> + pwr |= variant->pwrreg_powerup;
> +
> + break;
> + case MMC_POWER_ON:
> + if (!IS_ERR(mmc->supply.vqmmc) && !host->vqmmc_enabled) {
> + if (regulator_enable(mmc->supply.vqmmc) < 0)
> + dev_err(mmc_dev(mmc),
> + "failed to enable vqmmc regulator\n");
> + else
> + host->vqmmc_enabled = true;
> + }
> +
> + pwr |= MCI_PWR_ON;
> + break;
> + }
> +
> + mmci_write_pwrreg(host, pwr);
> +}
> +
> static void
> mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
> {
> @@ -1031,7 +1077,7 @@ static void mmci_set_ios(struct mmc_host *mmc, struct
> mmc_ios *ios)
> {
> struct mmci_host *host = mmc_priv(mmc);
> struct variant_data *variant = host->variant;
> - u32 pwr = 0;
> + unsigned int pwr = 0;
?
> unsigned long flags;
> int ret;
>
> @@ -1039,42 +1085,6 @@ static void mmci_set_ios(struct mmc_host *mmc, struct
> mmc_ios *ios)
> host->plat->ios_handler(mmc_dev(mmc), ios))
> dev_err(mmc_dev(mmc), "platform ios_handler
> failed\n");
>
> - switch (ios->power_mode) {
> - case MMC_POWER_OFF:
> - if (!IS_ERR(mmc->supply.vmmc))
> - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
> -
> - if (!IS_ERR(mmc->supply.vqmmc) && host->vqmmc_enabled) {
> - regulator_disable(mmc->supply.vqmmc);
> - host->vqmmc_enabled = false;
> - }
> -
> - break;
> - case MMC_POWER_UP:
> - if (!IS_ERR(mmc->supply.vmmc))
> - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
> ios->vdd);
> -
> - /*
> -* The ST Micro variant doesn't have the PL180s MCI_PWR_UP
> -* and instead uses MCI_PWR_ON so apply whatever value is
> -* configured in the variant data.
> -*/
> - pwr |= variant->pwrreg_powerup;
> -
> - break;
> - case MMC_POWER_ON:
> - if (!IS_ERR(mmc->supply.vqmmc) && !host->vqmmc_enabled) {
> - ret = regulator_enable(mmc->supply.vqmmc);
> - if (ret < 0)
> - dev_err(mmc_dev(mmc),
> - "failed to enable vqmmc regulator\n");
> - else
> - host->vqmmc_enabled = true;
> - }
> -
> - pwr |= MCI_PWR_ON;
> - break;
> - }
This above looks like pure re-factoring. Please make the above change
a separate patch.
>
> if (variant->signal_direction && ios->power_mode != MMC_POWER_OFF) {
> /*
> @@ -1126,8 +1136,16 @@ static void mmci_set_ios(struct mmc_host *mmc, struct
> mmc_ios *ios)
>
> spin_lock_irqsave(>lock,
[PATCH] parport: ieee1284: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/parport/ieee1284.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/parport/ieee1284.c b/drivers/parport/ieee1284.c index 2d1a5c7..f12b9da 100644 --- a/drivers/parport/ieee1284.c +++ b/drivers/parport/ieee1284.c @@ -267,7 +267,7 @@ static void parport_ieee1284_terminate (struct parport *port) port->ieee1284.phase = IEEE1284_PH_FWD_IDLE; } - /* fall-though.. */ + /* fall through */ default: /* Terminate from all other modes. */ @@ -615,6 +615,7 @@ ssize_t parport_write (struct parport *port, const void *buffer, size_t len) case IEEE1284_MODE_NIBBLE: case IEEE1284_MODE_BYTE: parport_negotiate (port, IEEE1284_MODE_COMPAT); + /* fall through */ case IEEE1284_MODE_COMPAT: DPRINTK (KERN_DEBUG "%s: Using compatibility mode\n", port->name); -- 2.7.4
[PATCH] parport: ieee1284: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/parport/ieee1284.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/parport/ieee1284.c b/drivers/parport/ieee1284.c index 2d1a5c7..f12b9da 100644 --- a/drivers/parport/ieee1284.c +++ b/drivers/parport/ieee1284.c @@ -267,7 +267,7 @@ static void parport_ieee1284_terminate (struct parport *port) port->ieee1284.phase = IEEE1284_PH_FWD_IDLE; } - /* fall-though.. */ + /* fall through */ default: /* Terminate from all other modes. */ @@ -615,6 +615,7 @@ ssize_t parport_write (struct parport *port, const void *buffer, size_t len) case IEEE1284_MODE_NIBBLE: case IEEE1284_MODE_BYTE: parport_negotiate (port, IEEE1284_MODE_COMPAT); + /* fall through */ case IEEE1284_MODE_COMPAT: DPRINTK (KERN_DEBUG "%s: Using compatibility mode\n", port->name); -- 2.7.4
Re: [PATCH] sched/cputime: Ensure correct utime and stime proportion
On Thu, Jul 05, 2018 at 09:21:15PM +0800, Xunlei Pang wrote:
> On 7/5/18 6:46 PM, Peter Zijlstra wrote:
> > On Wed, Jun 27, 2018 at 08:22:42PM +0800, Xunlei Pang wrote:
> >> tick-based whole utime is utime_0, tick-based whole stime
> >> is stime_0, scheduler time is rtime_0.
> >
> >> For a long time, the process runs mainly in userspace with
> >> run-sleep patterns, and because two different clocks, it
> >> is possible to have the following condition:
> >> rtime_0 < utime_0 (as with little stime_0)
> >
> > I don't follow... what?
> >
> > Why are you, and why do you think it makes sense to, compare rtime_0
> > against utime_0 ?
> >
> > The [us]time_0 are, per your earlier definition, ticks. They're not an
> > actual measure of time. Do not compare the two, that makes no bloody
> > sense.
> >
>
> [us]time_0 is task_struct:utime{stime}, I cited directly from
> cputime_adjust(), both in nanoseconds. I assumed "rtime_0 < utime_0"
> here to simple the following proof to help explain the problem we met.
In the !VIRT_CPU_ACCOUNTING case they (task_struct::[us]time) are not
actual durations. Yes, the happen to be accounted in multiples of
TICK_NSEC and thereby happen to carry a [ns] unit, but they are not
durations, they are samples.
(we just happen to store them in a [ns] unit because for
VIRT_CPU_ACCOUNTING they are in fact durations)
If 'rtime < utime' is not a valid assumption to build a problem on for
!VIRT_CPU_ACCOUNTING.
So please try again, so far you're not making any sense.
Re: [PATCH] sched/cputime: Ensure correct utime and stime proportion
On Thu, Jul 05, 2018 at 09:21:15PM +0800, Xunlei Pang wrote:
> On 7/5/18 6:46 PM, Peter Zijlstra wrote:
> > On Wed, Jun 27, 2018 at 08:22:42PM +0800, Xunlei Pang wrote:
> >> tick-based whole utime is utime_0, tick-based whole stime
> >> is stime_0, scheduler time is rtime_0.
> >
> >> For a long time, the process runs mainly in userspace with
> >> run-sleep patterns, and because two different clocks, it
> >> is possible to have the following condition:
> >> rtime_0 < utime_0 (as with little stime_0)
> >
> > I don't follow... what?
> >
> > Why are you, and why do you think it makes sense to, compare rtime_0
> > against utime_0 ?
> >
> > The [us]time_0 are, per your earlier definition, ticks. They're not an
> > actual measure of time. Do not compare the two, that makes no bloody
> > sense.
> >
>
> [us]time_0 is task_struct:utime{stime}, I cited directly from
> cputime_adjust(), both in nanoseconds. I assumed "rtime_0 < utime_0"
> here to simple the following proof to help explain the problem we met.
In the !VIRT_CPU_ACCOUNTING case they (task_struct::[us]time) are not
actual durations. Yes, the happen to be accounted in multiples of
TICK_NSEC and thereby happen to carry a [ns] unit, but they are not
durations, they are samples.
(we just happen to store them in a [ns] unit because for
VIRT_CPU_ACCOUNTING they are in fact durations)
If 'rtime < utime' is not a valid assumption to build a problem on for
!VIRT_CPU_ACCOUNTING.
So please try again, so far you're not making any sense.
Re: [PATCH v2 3/4] RISC-V: Add definiion of extract symbol's index and type for 32-bit
Palmer Dabbelt 於 2018年7月5日 週四 上午4:58寫道： > > On Fri, 29 Jun 2018 09:53:49 PDT (-0700), zong...@gmail.com wrote: > > Christoph Hellwig 於 2018年6月29日 週五 下午3:12寫道： > >> > >> On Mon, Jun 25, 2018 at 04:49:39PM +0800, Zong Li wrote: > >> > Use generic marco to get the index and type of symbol. > >> > >> Why do we even need this in a uapi header? Shouldn't ELF_RISCV_R_SYM > >> and ELF_RISCV_R_TYPE move to module.c? In that case we could just > >> use CONFIG_64BIT. > > > > Make sense. I even think the all definitions can be move to > > include/asm/elf.h. > > How do you think about that? > > Sorry I've been slow about reviewing this, but I think just in the interest of > getting a sane RV32I port up and running quickly (so we can try to get glibc > in > shape) I'm going to take this for now. > > Do you mind submitting a cleanup patch? I will send a patch to clean up. Thanks everyone to review this patchset.
Re: [PATCH v2 3/4] RISC-V: Add definiion of extract symbol's index and type for 32-bit
Palmer Dabbelt 於 2018年7月5日 週四 上午4:58寫道： > > On Fri, 29 Jun 2018 09:53:49 PDT (-0700), zong...@gmail.com wrote: > > Christoph Hellwig 於 2018年6月29日 週五 下午3:12寫道： > >> > >> On Mon, Jun 25, 2018 at 04:49:39PM +0800, Zong Li wrote: > >> > Use generic marco to get the index and type of symbol. > >> > >> Why do we even need this in a uapi header? Shouldn't ELF_RISCV_R_SYM > >> and ELF_RISCV_R_TYPE move to module.c? In that case we could just > >> use CONFIG_64BIT. > > > > Make sense. I even think the all definitions can be move to > > include/asm/elf.h. > > How do you think about that? > > Sorry I've been slow about reviewing this, but I think just in the interest of > getting a sane RV32I port up and running quickly (so we can try to get glibc > in > shape) I'm going to take this for now. > > Do you mind submitting a cleanup patch? I will send a patch to clean up. Thanks everyone to review this patchset.
Re: [PATCH v3 1/2] mm/sparse: add sparse_init_nid()
On 07/02/2018 01:29 PM, Pavel Tatashin wrote:
> On Mon, Jul 2, 2018 at 4:00 PM Dave Hansen wrote:
>>> + unsigned long size = sizeof(struct page) * PAGES_PER_SECTION;
>>> + unsigned long pnum, map_index = 0;
>>> + void *vmemmap_buf_start;
>>> +
>>> + size = ALIGN(size, PMD_SIZE) * map_count;
>>> + vmemmap_buf_start = __earlyonly_bootmem_alloc(nid, size,
>>> + PMD_SIZE,
>>> + __pa(MAX_DMA_ADDRESS));
>>
>> Let's not repeat the mistakes of the previous version of the code.
>> Please explain why we are aligning this. Also,
>> __earlyonly_bootmem_alloc()->memblock_virt_alloc_try_nid_raw() claims to
>> be aligning the size. Do we also need to do it here?
>>
>> Yes, I know the old code did this, but this is the cost of doing a
>> rewrite. :)
>
> Actually, I was thinking about this particular case when I was
> rewriting this code. Here we align size before multiplying by
> map_count aligns after memblock_virt_alloc_try_nid_raw(). So, we must
> have both as they are different.
That's a good point that they do different things.
But, which behavior of the two different things is the one we _want_?
>>> + if (vmemmap_buf_start) {
>>> + vmemmap_buf = vmemmap_buf_start;
>>> + vmemmap_buf_end = vmemmap_buf_start + size;
>>> + }
>>
>> It would be nice to call out that these are globals that other code
>> picks up.
>
> I do not like these globals, they should have specific functions that
> access them only, something:
> static struct {
> buffer;
> buffer_end;
> } vmemmap_buffer;
> vmemmap_buffer_init() allocate buffer
> vmemmap_buffer_alloc() return NULL if buffer is empty
> vmemmap_buffer_fini()
>
> Call vmemmap_buffer_init() and vmemmap_buffer_fini() from
> sparse_populate_node() and
> vmemmap_buffer_alloc() from vmemmap_alloc_block_buf().
>
> But, it should be a separate patch. If you would like I can add it to
> this series, or submit separately.
Seems like a nice cleanup, but I don't think it needs to be done here.
>>> + * Return map for pnum section. sparse_populate_node() has populated
>>> memory map
>>> + * in this node, we simply do pnum to struct page conversion.
>>> + */
>>> +struct page * __init sparse_populate_node_section(struct page *map_base,
>>> + unsigned long map_index,
>>> + unsigned long pnum,
>>> + int nid)
>>> +{
>>> + return pfn_to_page(section_nr_to_pfn(pnum));
>>> +}
>>
>> What is up with all of the unused arguments to this function?
>
> Because the same function is called from non-vmemmap sparse code.
That's probably good to call out in the patch description if not there
already.
>>> diff --git a/mm/sparse.c b/mm/sparse.c
>>> index d18e2697a781..c18d92b8ab9b 100644
>>> --- a/mm/sparse.c
>>> +++ b/mm/sparse.c
>>> @@ -456,6 +456,43 @@ void __init sparse_mem_maps_populate_node(struct page
>>> **map_map,
>>> __func__);
>>> }
>>> }
>>> +
>>> +static unsigned long section_map_size(void)
>>> +{
>>> + return PAGE_ALIGN(sizeof(struct page) * PAGES_PER_SECTION);
>>> +}
>>
>> Seems like if we have this, we should use it wherever possible, like
>> sparse_populate_node().
>
> It is used in sparse_populate_node():
>
> 401 struct page * __init sparse_populate_node(unsigned long pnum_begin,
> 406 return memblock_virt_alloc_try_nid_raw(section_map_size()
> * map_count,
> 407PAGE_SIZE,
> __pa(MAX_DMA_ADDRESS),
> 408
> BOOTMEM_ALLOC_ACCESSIBLE, nid);
I missed the PAGE_ALIGN() until now. That really needs a comment
calling out how it's not really the map size but the *allocation* size
of a single section's map.
It probably also needs a name like section_memmap_allocation_size() or
something to differentiate it from the *used* size.
>>> +/*
>>> + * Try to allocate all struct pages for this node, if this fails, we will
>>> + * be allocating one section at a time in sparse_populate_node_section().
>>> + */
>>> +struct page * __init sparse_populate_node(unsigned long pnum_begin,
>>> + unsigned long pnum_end,
>>> + unsigned long map_count,
>>> + int nid)
>>> +{
>>> + return memblock_virt_alloc_try_nid_raw(section_map_size() * map_count,
>>> +PAGE_SIZE,
>>> __pa(MAX_DMA_ADDRESS),
>>> +BOOTMEM_ALLOC_ACCESSIBLE, nid);
>>> +}
>>> +
>>> +/*
>>> + * Return map for pnum section. map_base is not NULL if we could allocate
>>> map
>>> + * for this node together. Otherwise we allocate one section at a time.
>>> + * map_index is the index of pnum in this node counting only present
>>> sections.
>>> + */
>>> +struct page *
Re: [PATCH v3 1/2] mm/sparse: add sparse_init_nid()
On 07/02/2018 01:29 PM, Pavel Tatashin wrote:
> On Mon, Jul 2, 2018 at 4:00 PM Dave Hansen wrote:
>>> + unsigned long size = sizeof(struct page) * PAGES_PER_SECTION;
>>> + unsigned long pnum, map_index = 0;
>>> + void *vmemmap_buf_start;
>>> +
>>> + size = ALIGN(size, PMD_SIZE) * map_count;
>>> + vmemmap_buf_start = __earlyonly_bootmem_alloc(nid, size,
>>> + PMD_SIZE,
>>> + __pa(MAX_DMA_ADDRESS));
>>
>> Let's not repeat the mistakes of the previous version of the code.
>> Please explain why we are aligning this. Also,
>> __earlyonly_bootmem_alloc()->memblock_virt_alloc_try_nid_raw() claims to
>> be aligning the size. Do we also need to do it here?
>>
>> Yes, I know the old code did this, but this is the cost of doing a
>> rewrite. :)
>
> Actually, I was thinking about this particular case when I was
> rewriting this code. Here we align size before multiplying by
> map_count aligns after memblock_virt_alloc_try_nid_raw(). So, we must
> have both as they are different.
That's a good point that they do different things.
But, which behavior of the two different things is the one we _want_?
>>> + if (vmemmap_buf_start) {
>>> + vmemmap_buf = vmemmap_buf_start;
>>> + vmemmap_buf_end = vmemmap_buf_start + size;
>>> + }
>>
>> It would be nice to call out that these are globals that other code
>> picks up.
>
> I do not like these globals, they should have specific functions that
> access them only, something:
> static struct {
> buffer;
> buffer_end;
> } vmemmap_buffer;
> vmemmap_buffer_init() allocate buffer
> vmemmap_buffer_alloc() return NULL if buffer is empty
> vmemmap_buffer_fini()
>
> Call vmemmap_buffer_init() and vmemmap_buffer_fini() from
> sparse_populate_node() and
> vmemmap_buffer_alloc() from vmemmap_alloc_block_buf().
>
> But, it should be a separate patch. If you would like I can add it to
> this series, or submit separately.
Seems like a nice cleanup, but I don't think it needs to be done here.
>>> + * Return map for pnum section. sparse_populate_node() has populated
>>> memory map
>>> + * in this node, we simply do pnum to struct page conversion.
>>> + */
>>> +struct page * __init sparse_populate_node_section(struct page *map_base,
>>> + unsigned long map_index,
>>> + unsigned long pnum,
>>> + int nid)
>>> +{
>>> + return pfn_to_page(section_nr_to_pfn(pnum));
>>> +}
>>
>> What is up with all of the unused arguments to this function?
>
> Because the same function is called from non-vmemmap sparse code.
That's probably good to call out in the patch description if not there
already.
>>> diff --git a/mm/sparse.c b/mm/sparse.c
>>> index d18e2697a781..c18d92b8ab9b 100644
>>> --- a/mm/sparse.c
>>> +++ b/mm/sparse.c
>>> @@ -456,6 +456,43 @@ void __init sparse_mem_maps_populate_node(struct page
>>> **map_map,
>>> __func__);
>>> }
>>> }
>>> +
>>> +static unsigned long section_map_size(void)
>>> +{
>>> + return PAGE_ALIGN(sizeof(struct page) * PAGES_PER_SECTION);
>>> +}
>>
>> Seems like if we have this, we should use it wherever possible, like
>> sparse_populate_node().
>
> It is used in sparse_populate_node():
>
> 401 struct page * __init sparse_populate_node(unsigned long pnum_begin,
> 406 return memblock_virt_alloc_try_nid_raw(section_map_size()
> * map_count,
> 407PAGE_SIZE,
> __pa(MAX_DMA_ADDRESS),
> 408
> BOOTMEM_ALLOC_ACCESSIBLE, nid);
I missed the PAGE_ALIGN() until now. That really needs a comment
calling out how it's not really the map size but the *allocation* size
of a single section's map.
It probably also needs a name like section_memmap_allocation_size() or
something to differentiate it from the *used* size.
>>> +/*
>>> + * Try to allocate all struct pages for this node, if this fails, we will
>>> + * be allocating one section at a time in sparse_populate_node_section().
>>> + */
>>> +struct page * __init sparse_populate_node(unsigned long pnum_begin,
>>> + unsigned long pnum_end,
>>> + unsigned long map_count,
>>> + int nid)
>>> +{
>>> + return memblock_virt_alloc_try_nid_raw(section_map_size() * map_count,
>>> +PAGE_SIZE,
>>> __pa(MAX_DMA_ADDRESS),
>>> +BOOTMEM_ALLOC_ACCESSIBLE, nid);
>>> +}
>>> +
>>> +/*
>>> + * Return map for pnum section. map_base is not NULL if we could allocate
>>> map
>>> + * for this node together. Otherwise we allocate one section at a time.
>>> + * map_index is the index of pnum in this node counting only present
>>> sections.
>>> + */
>>> +struct page *
Re: [PATCH v7 00/11] track CPU utilization
Hi Peter On Thu, 5 Jul 2018 at 14:36, Peter Zijlstra wrote: > > > OK, this looks good I suppose. Rafael, are you OK with me taking these? > > I have the below on top because I once again forgot how it all worked; > does this work for you Vincent? Yes looks good to me Thanks > > --- > Subject: sched/cpufreq: Clarify sugov_get_util() > > Add a few comments (hopefully) clarifying some of the magic in > sugov_get_util(). > > Signed-off-by: Peter Zijlstra (Intel) > --- > cpufreq_schedutil.c | 69 > ++-- > 1 file changed, 51 insertions(+), 18 deletions(-) >
Re: [PATCH v7 00/11] track CPU utilization
Hi Peter On Thu, 5 Jul 2018 at 14:36, Peter Zijlstra wrote: > > > OK, this looks good I suppose. Rafael, are you OK with me taking these? > > I have the below on top because I once again forgot how it all worked; > does this work for you Vincent? Yes looks good to me Thanks > > --- > Subject: sched/cpufreq: Clarify sugov_get_util() > > Add a few comments (hopefully) clarifying some of the magic in > sugov_get_util(). > > Signed-off-by: Peter Zijlstra (Intel) > --- > cpufreq_schedutil.c | 69 > ++-- > 1 file changed, 51 insertions(+), 18 deletions(-) >
Re: [PATCHv4 11/12] sched/core: Disable SD_ASYM_CPUCAPACITY for root_domains without asymmetry
Hi Morten,
On Wednesday 04 Jul 2018 at 11:17:49 (+0100), Morten Rasmussen wrote:
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 71330e0e41db..29c186961345 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -1160,6 +1160,26 @@ sd_init(struct sched_domain_topology_level *tl,
> sd_id = cpumask_first(sched_domain_span(sd));
>
> /*
> + * Check if cpu_map eclipses cpu capacity asymmetry.
> + */
> +
> + if (sd->flags & SD_ASYM_CPUCAPACITY) {
> + int i;
> + bool disable = true;
> + long capacity = arch_scale_cpu_capacity(NULL, sd_id);
> +
> + for_each_cpu(i, sched_domain_span(sd)) {
> + if (capacity != arch_scale_cpu_capacity(NULL, i)) {
> + disable = false;
> + break;
> + }
> + }
> +
> + if (disable)
> + sd->flags &= ~SD_ASYM_CPUCAPACITY;
> + }
> +
> + /*
>* Convert topological properties into behaviour.
>*/
If SD_ASYM_CPUCAPACITY means that some CPUs have different
arch_scale_cpu_capacity() values, we could also automatically _set_
the flag in sd_init() no ? Why should we let the arch set it and just
correct it later ?
I understand the moment at which we know the capacities of CPUs varies
from arch to arch, but the arch code could just call
rebuild_sched_domain when the capacities of CPUs change and let the
scheduler detect things automatically. I mean, even if the arch code
sets the flag in its topology level table, it will have to rebuild
the sched domains anyway ...
What do you think ?
Thanks,
Quentin
Re: [PATCHv4 11/12] sched/core: Disable SD_ASYM_CPUCAPACITY for root_domains without asymmetry
Hi Morten,
On Wednesday 04 Jul 2018 at 11:17:49 (+0100), Morten Rasmussen wrote:
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 71330e0e41db..29c186961345 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -1160,6 +1160,26 @@ sd_init(struct sched_domain_topology_level *tl,
> sd_id = cpumask_first(sched_domain_span(sd));
>
> /*
> + * Check if cpu_map eclipses cpu capacity asymmetry.
> + */
> +
> + if (sd->flags & SD_ASYM_CPUCAPACITY) {
> + int i;
> + bool disable = true;
> + long capacity = arch_scale_cpu_capacity(NULL, sd_id);
> +
> + for_each_cpu(i, sched_domain_span(sd)) {
> + if (capacity != arch_scale_cpu_capacity(NULL, i)) {
> + disable = false;
> + break;
> + }
> + }
> +
> + if (disable)
> + sd->flags &= ~SD_ASYM_CPUCAPACITY;
> + }
> +
> + /*
>* Convert topological properties into behaviour.
>*/
If SD_ASYM_CPUCAPACITY means that some CPUs have different
arch_scale_cpu_capacity() values, we could also automatically _set_
the flag in sd_init() no ? Why should we let the arch set it and just
correct it later ?
I understand the moment at which we know the capacities of CPUs varies
from arch to arch, but the arch code could just call
rebuild_sched_domain when the capacities of CPUs change and let the
scheduler detect things automatically. I mean, even if the arch code
sets the flag in its topology level table, it will have to rebuild
the sched domains anyway ...
What do you think ?
Thanks,
Quentin
Re: [PATCH v6 21/21] s390: doc: detailed specifications for AP virtualization
On 07/04/2018 12:31 PM, Boris Fiuczynski wrote: On 07/03/2018 06:36 PM, Tony Krowiak wrote: On 07/02/2018 07:10 PM, Halil Pasic wrote: On 06/29/2018 11:11 PM, Tony Krowiak wrote: This patch provides documentation describing the AP architecture and design concepts behind the virtualization of AP devices. It also includes an example of how to configure AP devices for exclusive use of KVM guests. Signed-off-by: Tony Krowiak I don't like the design of external interfaces except for: * cpu model features, and * reset handling. In particular: 1) The architecture is such that authorizing access (via APM, AQM and ADM) to an AP queue that is currently not configured (e.g. the card not physically plugged, or just configured off). That seems to be a perfectly normal use case. Your assign operations however enforce that the resource is bound to your driver, and thus the existence of the resource in the host. It is clear: we need to avoid passing trough resources to guests that are not dedicated for this purpose (e.g. a queue utilized by zcrypt). But IMHO we need a different mechanism. Interesting that you wait until v6 to bring this up. I agree, this is a normal use case, but there is currently no mechanism in the AP bus for drivers to reserve devices that are not yet configured. There is proposed solution in the works, but until such time that is available the only choice is to disallow assignment of AP queues to a guest that are not bound to the vfio_ap device driver. 2) I see no benefit in deferring the exclusivity check to vfio_ap_mdev_open(). The downside is however pretty obvious: management software is notified about a 'bad configuration' only at an attempted guest start-up. And your current QEMU patches are not very helpful in conveying this piece of information. It only becomes a 'bad configuration' if the two guests are started concurrently. Is there value in being able to configure two mediated devices with the same queue if the intent is to never run two guests using those mediated devices simultaneously? If so, then the only time the exclusivity check can be done is when the guest opens the mediated device. If not, then we can certainly prevent multiple mediated devices from being assigned the same queue. In my view, while a mediated device is used by a guest, it is not a guest and can be configured any way an administrator prefers. If we get concurrence that doing an exclusivity check when an adapter or domain is assigned to the mediated device, I'll make that change. I've talked with Boris, and AFAIR he said this is not acceptable to him (@Boris can you confirm). Then I suggest Boris participate in the review and explain why. [To make things a bit easier I am not going to address the aspect of not-currently-exiting host resources.] Your current implementation does provide active configurations that work with existing host resources. These need to be bound to the vfio_ap driver. Libvirt allows to define objects (e.g. domains or networks). These are just definitions and do NOT bind any resources. The defined resources are bound once the definition is started. Currently I am assuming that an ap matrix device is defined in libvirt outside of a libvirt domain (an ap definition). The mediated device of the ap matrix device is used in a libvirt domain by referencing it via its UID. When a libvirt domain is started the mediated device should exist and be configured correctly as every other host resource. Therefore there needs to be something new in libvirt that allows one to define, start, stop and undefine an ap matrix device. After a define the ap definition for an ap matrix device would exist in libvirt only. Once you start the ap definition the result should be a well configured ready to be used mediated device representing the ap definition which can be used configuration-error free by a libvirt domain. Please not that the start of an ap definition is independent from the start of a libvirt domain using the ap definition. Can you explain to me how that can be accomplished? I can make a similar case for the mediated devices. Mediated devices play no role in guest configuration until a vfio-ap device is specified on the QEMU command line when starting a guest. In other words, a mediated device configuration is independent from the start of a guest using the mediated device. To answer your question then, if there are two or more mediated devices with the same APQN(s) assigned, then only start one libvirt domain that uses one of these mediated devices. This begs the question: Does libvirt preclude one from defining a domain that uses a host device (of any kind) that must be dedicated to a single guest? If not, then isn't it incumbent upon the administrator to ensure he doesn't start two guests with the same dedicated host device? Wouldn't that same logic apply to AP devices? Having said that, I have no problem disallowing
Re: [PATCH v6 21/21] s390: doc: detailed specifications for AP virtualization
On 07/04/2018 12:31 PM, Boris Fiuczynski wrote: On 07/03/2018 06:36 PM, Tony Krowiak wrote: On 07/02/2018 07:10 PM, Halil Pasic wrote: On 06/29/2018 11:11 PM, Tony Krowiak wrote: This patch provides documentation describing the AP architecture and design concepts behind the virtualization of AP devices. It also includes an example of how to configure AP devices for exclusive use of KVM guests. Signed-off-by: Tony Krowiak I don't like the design of external interfaces except for: * cpu model features, and * reset handling. In particular: 1) The architecture is such that authorizing access (via APM, AQM and ADM) to an AP queue that is currently not configured (e.g. the card not physically plugged, or just configured off). That seems to be a perfectly normal use case. Your assign operations however enforce that the resource is bound to your driver, and thus the existence of the resource in the host. It is clear: we need to avoid passing trough resources to guests that are not dedicated for this purpose (e.g. a queue utilized by zcrypt). But IMHO we need a different mechanism. Interesting that you wait until v6 to bring this up. I agree, this is a normal use case, but there is currently no mechanism in the AP bus for drivers to reserve devices that are not yet configured. There is proposed solution in the works, but until such time that is available the only choice is to disallow assignment of AP queues to a guest that are not bound to the vfio_ap device driver. 2) I see no benefit in deferring the exclusivity check to vfio_ap_mdev_open(). The downside is however pretty obvious: management software is notified about a 'bad configuration' only at an attempted guest start-up. And your current QEMU patches are not very helpful in conveying this piece of information. It only becomes a 'bad configuration' if the two guests are started concurrently. Is there value in being able to configure two mediated devices with the same queue if the intent is to never run two guests using those mediated devices simultaneously? If so, then the only time the exclusivity check can be done is when the guest opens the mediated device. If not, then we can certainly prevent multiple mediated devices from being assigned the same queue. In my view, while a mediated device is used by a guest, it is not a guest and can be configured any way an administrator prefers. If we get concurrence that doing an exclusivity check when an adapter or domain is assigned to the mediated device, I'll make that change. I've talked with Boris, and AFAIR he said this is not acceptable to him (@Boris can you confirm). Then I suggest Boris participate in the review and explain why. [To make things a bit easier I am not going to address the aspect of not-currently-exiting host resources.] Your current implementation does provide active configurations that work with existing host resources. These need to be bound to the vfio_ap driver. Libvirt allows to define objects (e.g. domains or networks). These are just definitions and do NOT bind any resources. The defined resources are bound once the definition is started. Currently I am assuming that an ap matrix device is defined in libvirt outside of a libvirt domain (an ap definition). The mediated device of the ap matrix device is used in a libvirt domain by referencing it via its UID. When a libvirt domain is started the mediated device should exist and be configured correctly as every other host resource. Therefore there needs to be something new in libvirt that allows one to define, start, stop and undefine an ap matrix device. After a define the ap definition for an ap matrix device would exist in libvirt only. Once you start the ap definition the result should be a well configured ready to be used mediated device representing the ap definition which can be used configuration-error free by a libvirt domain. Please not that the start of an ap definition is independent from the start of a libvirt domain using the ap definition. Can you explain to me how that can be accomplished? I can make a similar case for the mediated devices. Mediated devices play no role in guest configuration until a vfio-ap device is specified on the QEMU command line when starting a guest. In other words, a mediated device configuration is independent from the start of a guest using the mediated device. To answer your question then, if there are two or more mediated devices with the same APQN(s) assigned, then only start one libvirt domain that uses one of these mediated devices. This begs the question: Does libvirt preclude one from defining a domain that uses a host device (of any kind) that must be dedicated to a single guest? If not, then isn't it incumbent upon the administrator to ensure he doesn't start two guests with the same dedicated host device? Wouldn't that same logic apply to AP devices? Having said that, I have no problem disallowing
Re: [PATCHv3 0/5] pinctrl fixes for generic functions and groups
Hi Tony and Linus, > Am 05.07.2018 um 11:10 schrieb Tony Lindgren : > > Here is a resend of fixes for a race issues for generic group and > functions reported by H. Nikolaus Schaller . Thanks for adding me. I have tested these patches on our OMAP3 based device where the problem was quite reproducible and testable by adding some printk. With these patches it seems now to be correct. Each group selector is assigned only once, even if driver probing fails and is redone later. Code review of the call paths has convinced us that we now have proper mutex around the radix tree function calls and races are no longer a problem. So you can add my Tested-By: H. Nikolaus Schaller BR and thanks, Nikolaus > > Regards, > > Tony > > > Changes since v2: > > - Added Nikolaus to Cc as Reported-by did not do it > > Changes since v1: > > - Check if a function or group already exists as suggested by > Andy Shevchenko > > - Make sure we always have a valid name for functions and groups > as suggested by Christ van Willegen > > - Prettify rza1 variables as suggested by Jacopo Mondi > > Tony Lindgren (5): > pinctrl: core: Return selector to the pinctrl driver > pinctrl: pinmux: Return selector to the pinctrl driver > pinctrl: single: Fix group and function selector use > pinctrl: rza1: Fix selector use for groups and functions > pinctrl: core: Remove broken remove_last group and pinmux functions > > drivers/pinctrl/core.c | 35 ++-- > drivers/pinctrl/core.h | 6 --- > drivers/pinctrl/pinctrl-rza1.c | 24 + > drivers/pinctrl/pinctrl-single.c | 91 +++- > drivers/pinctrl/pinmux.c | 16 -- > drivers/pinctrl/pinmux.h | 7 --- > 6 files changed, 112 insertions(+), 67 deletions(-) > > -- > 2.17.1
Re: [PATCHv3 0/5] pinctrl fixes for generic functions and groups
Hi Tony and Linus, > Am 05.07.2018 um 11:10 schrieb Tony Lindgren : > > Here is a resend of fixes for a race issues for generic group and > functions reported by H. Nikolaus Schaller . Thanks for adding me. I have tested these patches on our OMAP3 based device where the problem was quite reproducible and testable by adding some printk. With these patches it seems now to be correct. Each group selector is assigned only once, even if driver probing fails and is redone later. Code review of the call paths has convinced us that we now have proper mutex around the radix tree function calls and races are no longer a problem. So you can add my Tested-By: H. Nikolaus Schaller BR and thanks, Nikolaus > > Regards, > > Tony > > > Changes since v2: > > - Added Nikolaus to Cc as Reported-by did not do it > > Changes since v1: > > - Check if a function or group already exists as suggested by > Andy Shevchenko > > - Make sure we always have a valid name for functions and groups > as suggested by Christ van Willegen > > - Prettify rza1 variables as suggested by Jacopo Mondi > > Tony Lindgren (5): > pinctrl: core: Return selector to the pinctrl driver > pinctrl: pinmux: Return selector to the pinctrl driver > pinctrl: single: Fix group and function selector use > pinctrl: rza1: Fix selector use for groups and functions > pinctrl: core: Remove broken remove_last group and pinmux functions > > drivers/pinctrl/core.c | 35 ++-- > drivers/pinctrl/core.h | 6 --- > drivers/pinctrl/pinctrl-rza1.c | 24 + > drivers/pinctrl/pinctrl-single.c | 91 +++- > drivers/pinctrl/pinmux.c | 16 -- > drivers/pinctrl/pinmux.h | 7 --- > 6 files changed, 112 insertions(+), 67 deletions(-) > > -- > 2.17.1
Re: [PATCH] arm64: dts: marvell: fix CP110 ICU node size
Hi,
On jeu., juil. 05 2018, Gregory CLEMENT wrote:
> Hi Miquel,
>
> On jeu., juil. 05 2018, Miquel Raynal wrote:
>
>> commit 2f872ddcdb1e8e2186162616cea4581b8403849d upstream.
>>
>> ICU size in CP110 is not 0x10 but at least 0x440 bytes long (from the
>> specification).
>>
>> Fixes: 6ef84a827c37 ("arm64: dts: marvell: enable GICP and ICU on Armada
>> 7K/8K")
>> Cc: sta...@vger.kernel.org # 4.14.x
>> Signed-off-by: Miquel Raynal
>> Reviewed-by: Thomas Petazzoni
>> Signed-off-by: Gregory CLEMENT
>
> See
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2f872ddcdb1e8e2186162616cea4581b8403849d
>
> it is already in the linux kernel.
>
> Gregory
I missed the comment below and the fact it was for the stable branch.
Sorryt for the noise!
Gregory
>
>> ---
>>
>> Backport to 4.14 stable tree: the original patch did not applied
>> because of the effort of DT de-duplication that merged two files in one.
>>
>>
>> arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi | 2 +-
>> arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
>> b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
>> index 9a7b63cd63a3..7c19f32d0f44 100644
>> --- a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
>> +++ b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
>> @@ -169,7 +169,7 @@
>>
>> cpm_icu: interrupt-controller@1e {
>> compatible = "marvell,cp110-icu";
>> -reg = <0x1e 0x10>;
>> +reg = <0x1e 0x440>;
>> #interrupt-cells = <3>;
>> interrupt-controller;
>> msi-parent = <>;
>> diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
>> b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
>> index faf28633a309..150e97a1dfea 100644
>> --- a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
>> +++ b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
>> @@ -169,7 +169,7 @@
>>
>> cps_icu: interrupt-controller@1e {
>> compatible = "marvell,cp110-icu";
>> -reg = <0x1e 0x10>;
>> +reg = <0x1e 0x440>;
>> #interrupt-cells = <3>;
>> interrupt-controller;
>> msi-parent = <>;
>> --
>> 2.14.1
>>
>
> --
> Gregory Clement, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> http://bootlin.com
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
Re: [PATCH] arm64: dts: marvell: fix CP110 ICU node size
Hi,
On jeu., juil. 05 2018, Gregory CLEMENT wrote:
> Hi Miquel,
>
> On jeu., juil. 05 2018, Miquel Raynal wrote:
>
>> commit 2f872ddcdb1e8e2186162616cea4581b8403849d upstream.
>>
>> ICU size in CP110 is not 0x10 but at least 0x440 bytes long (from the
>> specification).
>>
>> Fixes: 6ef84a827c37 ("arm64: dts: marvell: enable GICP and ICU on Armada
>> 7K/8K")
>> Cc: sta...@vger.kernel.org # 4.14.x
>> Signed-off-by: Miquel Raynal
>> Reviewed-by: Thomas Petazzoni
>> Signed-off-by: Gregory CLEMENT
>
> See
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2f872ddcdb1e8e2186162616cea4581b8403849d
>
> it is already in the linux kernel.
>
> Gregory
I missed the comment below and the fact it was for the stable branch.
Sorryt for the noise!
Gregory
>
>> ---
>>
>> Backport to 4.14 stable tree: the original patch did not applied
>> because of the effort of DT de-duplication that merged two files in one.
>>
>>
>> arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi | 2 +-
>> arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
>> b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
>> index 9a7b63cd63a3..7c19f32d0f44 100644
>> --- a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
>> +++ b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
>> @@ -169,7 +169,7 @@
>>
>> cpm_icu: interrupt-controller@1e {
>> compatible = "marvell,cp110-icu";
>> -reg = <0x1e 0x10>;
>> +reg = <0x1e 0x440>;
>> #interrupt-cells = <3>;
>> interrupt-controller;
>> msi-parent = <>;
>> diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
>> b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
>> index faf28633a309..150e97a1dfea 100644
>> --- a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
>> +++ b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
>> @@ -169,7 +169,7 @@
>>
>> cps_icu: interrupt-controller@1e {
>> compatible = "marvell,cp110-icu";
>> -reg = <0x1e 0x10>;
>> +reg = <0x1e 0x440>;
>> #interrupt-cells = <3>;
>> interrupt-controller;
>> msi-parent = <>;
>> --
>> 2.14.1
>>
>
> --
> Gregory Clement, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> http://bootlin.com
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
Re: [PATCH] sched/fair: Avoid divide by zero when rebalancing domains
On Thu, 05 Jul, at 11:10:42AM, Valentin Schneider wrote:
> Hi,
>
> On 04/07/18 15:24, Matt Fleming wrote:
> > It's possible that the CPU doing nohz idle balance hasn't had its own
> > load updated for many seconds. This can lead to huge deltas between
> > rq->avg_stamp and rq->clock when rebalancing, and has been seen to
> > cause the following crash:
> >
> > divide error: [#1] SMP
> > Call Trace:
> > [] update_sd_lb_stats+0xe8/0x560
> > [] find_busiest_group+0x2d/0x4b0
> > [] load_balance+0x170/0x950
> > [] rebalance_domains+0x13f/0x290
> > [] __do_softirq+0xec/0x300
> > [] irq_exit+0xfa/0x110
> > [] reschedule_interrupt+0xc9/0xd0
> >
>
> Do you have some sort of reproducer for that crash? If not I guess I can cook
> something up with a quiet userspace & rt-app, though I've never seen that one
> on arm64.
Unfortunately no, I don't have a reproduction case. Would love to have
one to verify the patch though.
> > Make sure we update the rq clock and load before balancing.
> >
> > Cc: Ingo Molnar
> > Cc: Mike Galbraith
> > Cc: Peter Zijlstra
> > Signed-off-by: Matt Fleming
> > ---
> > kernel/sched/fair.c | 10 ++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 2f0a0be4d344..2c81662c858a 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -9597,6 +9597,16 @@ static bool _nohz_idle_balance(struct rq *this_rq,
> > unsigned int flags,
> > */
> > smp_mb();
> >
> > + /*
> > +* Ensure this_rq's clock and load are up-to-date before we
> > +* rebalance since it's possible that they haven't been
> > +* updated for multiple schedule periods, i.e. many seconds.
> > +*/
> > + raw_spin_lock_irq(_rq->lock);
> > + update_rq_clock(this_rq);
> > + cpu_load_update_idle(this_rq);
> > + raw_spin_unlock_irq(_rq->lock);
> > +
>
> I'm failing to understand why the updates further down below are seemingly
> not enough. After we've potentially done
>
> update_rq_clock(rq);
> cpu_load_update_idle(rq);
>
> for all nohz cpus != this_cpu, we still end up doing:
>
> if (idle != CPU_NEWLY_IDLE) {
> update_blocked_averages(this_cpu);
> has_blocked_load |= this_rq->has_blocked_load;
> }
>
> which should properly update this_rq's clock and load before we attempt to do
> any balancing on it.
But cpu_load_update_idle() and update_blocked_averages() are not the same
thing.
Re: [PATCH] sched/fair: Avoid divide by zero when rebalancing domains
On Thu, 05 Jul, at 11:10:42AM, Valentin Schneider wrote:
> Hi,
>
> On 04/07/18 15:24, Matt Fleming wrote:
> > It's possible that the CPU doing nohz idle balance hasn't had its own
> > load updated for many seconds. This can lead to huge deltas between
> > rq->avg_stamp and rq->clock when rebalancing, and has been seen to
> > cause the following crash:
> >
> > divide error: [#1] SMP
> > Call Trace:
> > [] update_sd_lb_stats+0xe8/0x560
> > [] find_busiest_group+0x2d/0x4b0
> > [] load_balance+0x170/0x950
> > [] rebalance_domains+0x13f/0x290
> > [] __do_softirq+0xec/0x300
> > [] irq_exit+0xfa/0x110
> > [] reschedule_interrupt+0xc9/0xd0
> >
>
> Do you have some sort of reproducer for that crash? If not I guess I can cook
> something up with a quiet userspace & rt-app, though I've never seen that one
> on arm64.
Unfortunately no, I don't have a reproduction case. Would love to have
one to verify the patch though.
> > Make sure we update the rq clock and load before balancing.
> >
> > Cc: Ingo Molnar
> > Cc: Mike Galbraith
> > Cc: Peter Zijlstra
> > Signed-off-by: Matt Fleming
> > ---
> > kernel/sched/fair.c | 10 ++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 2f0a0be4d344..2c81662c858a 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -9597,6 +9597,16 @@ static bool _nohz_idle_balance(struct rq *this_rq,
> > unsigned int flags,
> > */
> > smp_mb();
> >
> > + /*
> > +* Ensure this_rq's clock and load are up-to-date before we
> > +* rebalance since it's possible that they haven't been
> > +* updated for multiple schedule periods, i.e. many seconds.
> > +*/
> > + raw_spin_lock_irq(_rq->lock);
> > + update_rq_clock(this_rq);
> > + cpu_load_update_idle(this_rq);
> > + raw_spin_unlock_irq(_rq->lock);
> > +
>
> I'm failing to understand why the updates further down below are seemingly
> not enough. After we've potentially done
>
> update_rq_clock(rq);
> cpu_load_update_idle(rq);
>
> for all nohz cpus != this_cpu, we still end up doing:
>
> if (idle != CPU_NEWLY_IDLE) {
> update_blocked_averages(this_cpu);
> has_blocked_load |= this_rq->has_blocked_load;
> }
>
> which should properly update this_rq's clock and load before we attempt to do
> any balancing on it.
But cpu_load_update_idle() and update_blocked_averages() are not the same
thing.
[PATCH] Input: i8042 - add Lenovo LaVie Z to the i8042 reset list
The Lenovo LaVie Z laptop requires i8042 to be reset in order to
consistently detect its Elantech touchpad. The nomux and kbdreset
quirks are not sufficient.
It's possible the other LaVie Z models from NEC require this as well.
Cc: sta...@vger.kernel.org
Signed-off-by: Chen-Yu Tsai
---
drivers/input/serio/i8042-x86ia64io.h | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/input/serio/i8042-x86ia64io.h
b/drivers/input/serio/i8042-x86ia64io.h
index b353d494ad40..136f6e7bf797 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -527,6 +527,13 @@ static const struct dmi_system_id __initconst
i8042_dmi_nomux_table[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "N24_25BU"),
},
},
+ {
+ /* Lenovo LaVie Z */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo LaVie Z"),
+ },
+ },
{ }
};
--
2.18.0
[PATCH] Input: i8042 - add Lenovo LaVie Z to the i8042 reset list
The Lenovo LaVie Z laptop requires i8042 to be reset in order to
consistently detect its Elantech touchpad. The nomux and kbdreset
quirks are not sufficient.
It's possible the other LaVie Z models from NEC require this as well.
Cc: sta...@vger.kernel.org
Signed-off-by: Chen-Yu Tsai
---
drivers/input/serio/i8042-x86ia64io.h | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/input/serio/i8042-x86ia64io.h
b/drivers/input/serio/i8042-x86ia64io.h
index b353d494ad40..136f6e7bf797 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -527,6 +527,13 @@ static const struct dmi_system_id __initconst
i8042_dmi_nomux_table[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "N24_25BU"),
},
},
+ {
+ /* Lenovo LaVie Z */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo LaVie Z"),
+ },
+ },
{ }
};
--
2.18.0
Re: [PATCH] arm64: dts: marvell: fix CP110 ICU node size
Hi Miquel,
On jeu., juil. 05 2018, Miquel Raynal wrote:
> commit 2f872ddcdb1e8e2186162616cea4581b8403849d upstream.
>
> ICU size in CP110 is not 0x10 but at least 0x440 bytes long (from the
> specification).
>
> Fixes: 6ef84a827c37 ("arm64: dts: marvell: enable GICP and ICU on Armada
> 7K/8K")
> Cc: sta...@vger.kernel.org # 4.14.x
> Signed-off-by: Miquel Raynal
> Reviewed-by: Thomas Petazzoni
> Signed-off-by: Gregory CLEMENT
See
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2f872ddcdb1e8e2186162616cea4581b8403849d
it is already in the linux kernel.
Gregory
> ---
>
> Backport to 4.14 stable tree: the original patch did not applied
> because of the effort of DT de-duplication that merged two files in one.
>
>
> arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi | 2 +-
> arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
> b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
> index 9a7b63cd63a3..7c19f32d0f44 100644
> --- a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
> @@ -169,7 +169,7 @@
>
> cpm_icu: interrupt-controller@1e {
> compatible = "marvell,cp110-icu";
> - reg = <0x1e 0x10>;
> + reg = <0x1e 0x440>;
> #interrupt-cells = <3>;
> interrupt-controller;
> msi-parent = <>;
> diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
> b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
> index faf28633a309..150e97a1dfea 100644
> --- a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
> @@ -169,7 +169,7 @@
>
> cps_icu: interrupt-controller@1e {
> compatible = "marvell,cp110-icu";
> - reg = <0x1e 0x10>;
> + reg = <0x1e 0x440>;
> #interrupt-cells = <3>;
> interrupt-controller;
> msi-parent = <>;
> --
> 2.14.1
>
--
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
Re: [PATCH] arm64: dts: marvell: fix CP110 ICU node size
Hi Miquel,
On jeu., juil. 05 2018, Miquel Raynal wrote:
> commit 2f872ddcdb1e8e2186162616cea4581b8403849d upstream.
>
> ICU size in CP110 is not 0x10 but at least 0x440 bytes long (from the
> specification).
>
> Fixes: 6ef84a827c37 ("arm64: dts: marvell: enable GICP and ICU on Armada
> 7K/8K")
> Cc: sta...@vger.kernel.org # 4.14.x
> Signed-off-by: Miquel Raynal
> Reviewed-by: Thomas Petazzoni
> Signed-off-by: Gregory CLEMENT
See
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2f872ddcdb1e8e2186162616cea4581b8403849d
it is already in the linux kernel.
Gregory
> ---
>
> Backport to 4.14 stable tree: the original patch did not applied
> because of the effort of DT de-duplication that merged two files in one.
>
>
> arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi | 2 +-
> arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
> b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
> index 9a7b63cd63a3..7c19f32d0f44 100644
> --- a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
> @@ -169,7 +169,7 @@
>
> cpm_icu: interrupt-controller@1e {
> compatible = "marvell,cp110-icu";
> - reg = <0x1e 0x10>;
> + reg = <0x1e 0x440>;
> #interrupt-cells = <3>;
> interrupt-controller;
> msi-parent = <>;
> diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
> b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
> index faf28633a309..150e97a1dfea 100644
> --- a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
> @@ -169,7 +169,7 @@
>
> cps_icu: interrupt-controller@1e {
> compatible = "marvell,cp110-icu";
> - reg = <0x1e 0x10>;
> + reg = <0x1e 0x440>;
> #interrupt-cells = <3>;
> interrupt-controller;
> msi-parent = <>;
> --
> 2.14.1
>
--
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
Re: [lkp-robot] [sched/fair] fbd5188493: WARNING:inconsistent_lock_state
On Thu, 05 Jul, at 11:52:21AM, Dietmar Eggemann wrote:
>
> Moving the code from _nohz_idle_balance to nohz_idle_balance let it disappear:
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 02be51c9dcc1..070924f07c68 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9596,16 +9596,6 @@ static bool _nohz_idle_balance(struct rq *this_rq,
> unsigned int flags,
> */
> smp_mb();
>
> - /*
> -* Ensure this_rq's clock and load are up-to-date before we
> -* rebalance since it's possible that they haven't been
> -* updated for multiple schedule periods, i.e. many seconds.
> -*/
> - raw_spin_lock_irq(_rq->lock);
> - update_rq_clock(this_rq);
> - cpu_load_update_idle(this_rq);
> - raw_spin_unlock_irq(_rq->lock);
> -
> for_each_cpu(balance_cpu, nohz.idle_cpus_mask) {
> if (balance_cpu == this_cpu || !idle_cpu(balance_cpu))
> continue;
> @@ -9701,6 +9691,16 @@ static bool nohz_idle_balance(struct rq *this_rq, enum
> cpu_idle_type idle)
> if (!(flags & NOHZ_KICK_MASK))
> return false;
>
> + /*
> +* Ensure this_rq's clock and load are up-to-date before we
> +* rebalance since it's possible that they haven't been
> +* updated for multiple schedule periods, i.e. many seconds.
> +*/
> + raw_spin_lock_irq(_rq->lock);
> + update_rq_clock(this_rq);
> + cpu_load_update_idle(this_rq);
> + raw_spin_unlock_irq(_rq->lock);
> +
> _nohz_idle_balance(this_rq, flags, idle);
>
> return true;
>
Hmm.. it still looks to me like we should be saving and restoring IRQs
since this can be called from IRQ context, no?
The patch was a forward-port from one of our SLE kernels, and I messed
up the IRQ flag balancing for the v4.18-rc3 code :-(
Re: [lkp-robot] [sched/fair] fbd5188493: WARNING:inconsistent_lock_state
On Thu, 05 Jul, at 11:52:21AM, Dietmar Eggemann wrote:
>
> Moving the code from _nohz_idle_balance to nohz_idle_balance let it disappear:
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 02be51c9dcc1..070924f07c68 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9596,16 +9596,6 @@ static bool _nohz_idle_balance(struct rq *this_rq,
> unsigned int flags,
> */
> smp_mb();
>
> - /*
> -* Ensure this_rq's clock and load are up-to-date before we
> -* rebalance since it's possible that they haven't been
> -* updated for multiple schedule periods, i.e. many seconds.
> -*/
> - raw_spin_lock_irq(_rq->lock);
> - update_rq_clock(this_rq);
> - cpu_load_update_idle(this_rq);
> - raw_spin_unlock_irq(_rq->lock);
> -
> for_each_cpu(balance_cpu, nohz.idle_cpus_mask) {
> if (balance_cpu == this_cpu || !idle_cpu(balance_cpu))
> continue;
> @@ -9701,6 +9691,16 @@ static bool nohz_idle_balance(struct rq *this_rq, enum
> cpu_idle_type idle)
> if (!(flags & NOHZ_KICK_MASK))
> return false;
>
> + /*
> +* Ensure this_rq's clock and load are up-to-date before we
> +* rebalance since it's possible that they haven't been
> +* updated for multiple schedule periods, i.e. many seconds.
> +*/
> + raw_spin_lock_irq(_rq->lock);
> + update_rq_clock(this_rq);
> + cpu_load_update_idle(this_rq);
> + raw_spin_unlock_irq(_rq->lock);
> +
> _nohz_idle_balance(this_rq, flags, idle);
>
> return true;
>
Hmm.. it still looks to me like we should be saving and restoring IRQs
since this can be called from IRQ context, no?
The patch was a forward-port from one of our SLE kernels, and I messed
up the IRQ flag balancing for the v4.18-rc3 code :-(
Re: [PATCH] sched/cputime: Ensure correct utime and stime proportion
On 7/5/18 6:46 PM, Peter Zijlstra wrote:
> On Wed, Jun 27, 2018 at 08:22:42PM +0800, Xunlei Pang wrote:
>> tick-based whole utime is utime_0, tick-based whole stime
>> is stime_0, scheduler time is rtime_0.
>
>> For a long time, the process runs mainly in userspace with
>> run-sleep patterns, and because two different clocks, it
>> is possible to have the following condition:
>> rtime_0 < utime_0 (as with little stime_0)
>
> I don't follow... what?
>
> Why are you, and why do you think it makes sense to, compare rtime_0
> against utime_0 ?
>
> The [us]time_0 are, per your earlier definition, ticks. They're not an
> actual measure of time. Do not compare the two, that makes no bloody
> sense.
>
[us]time_0 is task_struct:utime{stime}, I cited directly from
cputime_adjust(), both in nanoseconds. I assumed "rtime_0 < utime_0"
here to simple the following proof to help explain the problem we met.
Re: [PATCH] sched/cputime: Ensure correct utime and stime proportion
On 7/5/18 6:46 PM, Peter Zijlstra wrote:
> On Wed, Jun 27, 2018 at 08:22:42PM +0800, Xunlei Pang wrote:
>> tick-based whole utime is utime_0, tick-based whole stime
>> is stime_0, scheduler time is rtime_0.
>
>> For a long time, the process runs mainly in userspace with
>> run-sleep patterns, and because two different clocks, it
>> is possible to have the following condition:
>> rtime_0 < utime_0 (as with little stime_0)
>
> I don't follow... what?
>
> Why are you, and why do you think it makes sense to, compare rtime_0
> against utime_0 ?
>
> The [us]time_0 are, per your earlier definition, ticks. They're not an
> actual measure of time. Do not compare the two, that makes no bloody
> sense.
>
[us]time_0 is task_struct:utime{stime}, I cited directly from
cputime_adjust(), both in nanoseconds. I assumed "rtime_0 < utime_0"
here to simple the following proof to help explain the problem we met.
[PATCH v12 2/2] Add the missing information for the oom report
From: yuzhoujian
The current oom report doesn't display victim's memcg context during the
global OOM situation. While this information is not strictly needed, it
can be really helpful for containerized environments to locate which
container has lost a process. Now that we have a single line for the oom
context, we can trivially add both the oom memcg (this can be either
global_oom or a specific memcg which hits its hard limits) and task_memcg
which is the victim's memcg.
Signed-off-by: yuzhoujian
---
include/linux/memcontrol.h | 14 +++---
mm/memcontrol.c| 36 ++--
mm/oom_kill.c | 10 ++
3 files changed, 39 insertions(+), 21 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 6c6fb116e925..96a73f989101 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -491,8 +491,10 @@ void mem_cgroup_handle_over_high(void);
unsigned long mem_cgroup_get_max(struct mem_cgroup *memcg);
-void mem_cgroup_print_oom_info(struct mem_cgroup *memcg,
- struct task_struct *p);
+void mem_cgroup_print_oom_context(struct mem_cgroup *memcg,
+ struct task_struct *p);
+
+void mem_cgroup_print_oom_meminfo(struct mem_cgroup *memcg);
static inline void mem_cgroup_oom_enable(void)
{
@@ -903,7 +905,13 @@ static inline unsigned long mem_cgroup_get_max(struct
mem_cgroup *memcg)
}
static inline void
-mem_cgroup_print_oom_info(struct mem_cgroup *memcg, struct task_struct *p)
+mem_cgroup_print_oom_context(struct mem_cgroup *memcg,
+ struct task_struct *p)
+{
+}
+
+static inline void
+mem_cgroup_print_oom_meminfo(struct mem_cgroup *memcg)
{
}
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index e6f0d5ef320a..18deea974cfd 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1119,32 +1119,40 @@ static const char *const memcg1_stat_names[] = {
#define K(x) ((x) << (PAGE_SHIFT-10))
/**
- * mem_cgroup_print_oom_info: Print OOM information relevant to memory
controller.
- * @memcg: The memory cgroup that went over limit
+ * mem_cgroup_print_oom_context: Print OOM context information relevant to
+ * memory controller.
+ * @memcg: The origin memory cgroup that went over limit
* @p: Task that is going to be killed
*
* NOTE: @memcg and @p's mem_cgroup can be different when hierarchy is
* enabled
*/
-void mem_cgroup_print_oom_info(struct mem_cgroup *memcg, struct task_struct *p)
+void mem_cgroup_print_oom_context(struct mem_cgroup *memcg, struct task_struct
*p)
{
- struct mem_cgroup *iter;
- unsigned int i;
+ struct cgroup *origin_cgrp, *kill_cgrp;
rcu_read_lock();
-
+ if (memcg) {
+ pr_cont(",oom_memcg=");
+ pr_cont_cgroup_path(memcg->css.cgroup);
+ } else
+ pr_cont(",global_oom");
if (p) {
- pr_info("Task in ");
+ pr_cont(",task_memcg=");
pr_cont_cgroup_path(task_cgroup(p, memory_cgrp_id));
- pr_cont(" killed as a result of limit of ");
- } else {
- pr_info("Memory limit reached of cgroup ");
}
-
- pr_cont_cgroup_path(memcg->css.cgroup);
- pr_cont("\n");
-
rcu_read_unlock();
+}
+
+/**
+ * mem_cgroup_print_oom_meminfo: Print OOM memory information relevant to
+ * memory controller.
+ * @memcg: The memory cgroup that went over limit
+ */
+void mem_cgroup_print_oom_meminfo(struct mem_cgroup *memcg)
+{
+ struct mem_cgroup *iter;
+ unsigned int i;
pr_info("memory: usage %llukB, limit %llukB, failcnt %lu\n",
K((u64)page_counter_read(>memory)),
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index c38f224b0d9e..9e80f6c2eb2e 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -430,13 +430,15 @@ static void dump_header(struct oom_control *oc, struct
task_struct *p)
dump_stack();
/* one line summary of the oom killer context. */
- pr_info("oom-kill:constraint=%s,nodemask=%*pbl,task=%s,pid=%5d,uid=%5d",
+ pr_info("oom-kill:constraint=%s,nodemask=%*pbl",
oom_constraint_text[oc->constraint],
- nodemask_pr_args(oc->nodemask),
- p->comm, p->pid, from_kuid(_user_ns, task_uid(p)));
+ nodemask_pr_args(oc->nodemask));
+ mem_cgroup_print_oom_context(oc->memcg, p);
cpuset_print_current_mems_allowed();
+ pr_cont(",task=%s,pid=%5d,uid=%5d\n", p->comm, p->pid,
+ from_kuid(_user_ns, task_uid(p)));
if (is_memcg_oom(oc))
- mem_cgroup_print_oom_info(oc->memcg, p);
+ mem_cgroup_print_oom_meminfo(oc->memcg);
else {
show_mem(SHOW_MEM_FILTER_NODES, oc->nodemask);
if (is_dump_unreclaim_slabs())
--
2.14.1
[PATCH v12 1/2] Reorganize the oom report in dump_header
From: yuzhoujian
OOM report contains several sections. The first one is the allocation
context that has triggered the OOM. Then we have cpuset context
followed by the stack trace of the OOM path. Followed by the oom
eligible tasks and the information about the chosen oom victim.
One thing that makes parsing more awkward than necessary is that we do
not have a single and easily parsable line about the oom context. This
patch is reorganizing the oom report to
1) who invoked oom and what was the allocation request
[ 126.168182] panic invoked oom-killer:
gfp_mask=0x6280ca(GFP_HIGHUSER_MOVABLE|__GFP_ZERO), order=0, oom_score_adj=0
2) OOM stack trace
[ 126.169806] CPU: 23 PID: 8668 Comm: panic Not tainted 4.18.0-rc3+ #42
[ 126.170494] Hardware name: Inspur SA5212M4/YZMB-00370-107, BIOS
4.1.10 11/14/2016
[ 126.171197] Call Trace:
[ 126.171901] dump_stack+0x5a/0x73
[ 126.172593] dump_header+0x58/0x2dc
[ 126.173294] oom_kill_process+0x228/0x420
[ 126.173999] ? oom_badness+0x2a/0x130
[ 126.174705] out_of_memory+0x11a/0x4a0
[ 126.175415] __alloc_pages_slowpath+0x7cc/0xa1e
[ 126.176128] ? __alloc_pages_slowpath+0x194/0xa1e
[ 126.176853] ? page_counter_try_charge+0x54/0xc0
[ 126.177580] __alloc_pages_nodemask+0x277/0x290
[ 126.178319] alloc_pages_vma+0x73/0x180
[ 126.179058] do_anonymous_page+0xed/0x5a0
[ 126.179825] __handle_mm_fault+0xbb3/0xe70
[ 126.180566] handle_mm_fault+0xfa/0x210
[ 126.181313] __do_page_fault+0x233/0x4c0
[ 126.182063] do_page_fault+0x32/0x140
[ 126.182812] ? page_fault+0x8/0x30
[ 126.183560] page_fault+0x1e/0x30
3) oom context (contrains and the chosen victim).
[ 126.190619]
oom-kill:constraint=CONSTRAINT_NONE,nodemask=(null),cpuset=/,mems_allowed=0-1,task=panic,pid=10235,uid=
0
An admin can easily get the full oom context at a single line which
makes parsing much easier.
Signed-off-by: yuzhoujian
---
Changes since v11:
- move the array of const char oom_constraint_text to oom_kill.c
- add the cpuset information in the one line output.
Changes since v10:
- divide the patch v8 into two parts. One part is to add the array of const
char and put enum
oom_constaint into oom.h; the other adds a new func to print the missing
information for the system-
wide oom report.
Changes since v9:
- divide the patch v8 into two parts. One part is to move enum oom_constraint
into memcontrol.h; the
other refactors the output info in the dump_header.
- replace orgin_memcg and kill_memcg with oom_memcg and task_memcg resptively.
Changes since v8:
- add the constraint in the oom_control structure.
- put enum oom_constraint and constraint array into the oom.h file.
- simplify the description for mem_cgroup_print_oom_context.
Changes since v7:
- add the constraint parameter to dump_header and oom_kill_process.
- remove the static char array in the mem_cgroup_print_oom_context, and
invoke pr_cont_cgroup_path to print memcg' name.
- combine the patchset v6 into one.
Changes since v6:
- divide the patch v5 into two parts. One part is to add an array of const char
and
put enum oom_constraint into the memcontrol.h; the other refactors the output
in the dump_header.
- limit the memory usage for the static char array by using NAME_MAX in the
mem_cgroup_print_oom_context.
- eliminate the spurious spaces in the oom's output and fix the spelling of
"constrain".
Changes since v5:
- add an array of const char for each constraint.
- replace all of the pr_cont with a single line print of the pr_info.
- put enum oom_constraint into the memcontrol.c file for printing oom
constraint.
Changes since v4:
- rename the helper's name to mem_cgroup_print_oom_context.
- rename the mem_cgroup_print_oom_info to mem_cgroup_print_oom_meminfo.
- add the constrain info in the dump_header.
Changes since v3:
- rename the helper's name to mem_cgroup_print_oom_memcg_name.
- add the rcu lock held to the helper.
- remove the print info of memcg's name in mem_cgroup_print_oom_info.
Changes since v2:
- add the mem_cgroup_print_memcg_name helper to print the memcg's
name which contains the task that will be killed by the oom-killer.
Changes since v1:
- replace adding mem_cgroup_print_oom_info with printing the memcg's
name only.
include/linux/oom.h| 10 ++
kernel/cgroup/cpuset.c | 4 ++--
mm/oom_kill.c | 36
mm/page_alloc.c| 4 ++--
4 files changed, 34 insertions(+), 20 deletions(-)
diff --git a/include/linux/oom.h b/include/linux/oom.h
index 6adac113e96d..3e5e01619bc8 100644
--- a/include/linux/oom.h
+++ b/include/linux/oom.h
@@ -15,6 +15,13 @@ struct notifier_block;
struct mem_cgroup;
struct task_struct;
+enum oom_constraint {
+ CONSTRAINT_NONE,
+ CONSTRAINT_CPUSET,
+ CONSTRAINT_MEMORY_POLICY,
+
[PATCH v12 2/2] Add the missing information for the oom report
From: yuzhoujian
The current oom report doesn't display victim's memcg context during the
global OOM situation. While this information is not strictly needed, it
can be really helpful for containerized environments to locate which
container has lost a process. Now that we have a single line for the oom
context, we can trivially add both the oom memcg (this can be either
global_oom or a specific memcg which hits its hard limits) and task_memcg
which is the victim's memcg.
Signed-off-by: yuzhoujian
---
include/linux/memcontrol.h | 14 +++---
mm/memcontrol.c| 36 ++--
mm/oom_kill.c | 10 ++
3 files changed, 39 insertions(+), 21 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 6c6fb116e925..96a73f989101 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -491,8 +491,10 @@ void mem_cgroup_handle_over_high(void);
unsigned long mem_cgroup_get_max(struct mem_cgroup *memcg);
-void mem_cgroup_print_oom_info(struct mem_cgroup *memcg,
- struct task_struct *p);
+void mem_cgroup_print_oom_context(struct mem_cgroup *memcg,
+ struct task_struct *p);
+
+void mem_cgroup_print_oom_meminfo(struct mem_cgroup *memcg);
static inline void mem_cgroup_oom_enable(void)
{
@@ -903,7 +905,13 @@ static inline unsigned long mem_cgroup_get_max(struct
mem_cgroup *memcg)
}
static inline void
-mem_cgroup_print_oom_info(struct mem_cgroup *memcg, struct task_struct *p)
+mem_cgroup_print_oom_context(struct mem_cgroup *memcg,
+ struct task_struct *p)
+{
+}
+
+static inline void
+mem_cgroup_print_oom_meminfo(struct mem_cgroup *memcg)
{
}
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index e6f0d5ef320a..18deea974cfd 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1119,32 +1119,40 @@ static const char *const memcg1_stat_names[] = {
#define K(x) ((x) << (PAGE_SHIFT-10))
/**
- * mem_cgroup_print_oom_info: Print OOM information relevant to memory
controller.
- * @memcg: The memory cgroup that went over limit
+ * mem_cgroup_print_oom_context: Print OOM context information relevant to
+ * memory controller.
+ * @memcg: The origin memory cgroup that went over limit
* @p: Task that is going to be killed
*
* NOTE: @memcg and @p's mem_cgroup can be different when hierarchy is
* enabled
*/
-void mem_cgroup_print_oom_info(struct mem_cgroup *memcg, struct task_struct *p)
+void mem_cgroup_print_oom_context(struct mem_cgroup *memcg, struct task_struct
*p)
{
- struct mem_cgroup *iter;
- unsigned int i;
+ struct cgroup *origin_cgrp, *kill_cgrp;
rcu_read_lock();
-
+ if (memcg) {
+ pr_cont(",oom_memcg=");
+ pr_cont_cgroup_path(memcg->css.cgroup);
+ } else
+ pr_cont(",global_oom");
if (p) {
- pr_info("Task in ");
+ pr_cont(",task_memcg=");
pr_cont_cgroup_path(task_cgroup(p, memory_cgrp_id));
- pr_cont(" killed as a result of limit of ");
- } else {
- pr_info("Memory limit reached of cgroup ");
}
-
- pr_cont_cgroup_path(memcg->css.cgroup);
- pr_cont("\n");
-
rcu_read_unlock();
+}
+
+/**
+ * mem_cgroup_print_oom_meminfo: Print OOM memory information relevant to
+ * memory controller.
+ * @memcg: The memory cgroup that went over limit
+ */
+void mem_cgroup_print_oom_meminfo(struct mem_cgroup *memcg)
+{
+ struct mem_cgroup *iter;
+ unsigned int i;
pr_info("memory: usage %llukB, limit %llukB, failcnt %lu\n",
K((u64)page_counter_read(>memory)),
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index c38f224b0d9e..9e80f6c2eb2e 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -430,13 +430,15 @@ static void dump_header(struct oom_control *oc, struct
task_struct *p)
dump_stack();
/* one line summary of the oom killer context. */
- pr_info("oom-kill:constraint=%s,nodemask=%*pbl,task=%s,pid=%5d,uid=%5d",
+ pr_info("oom-kill:constraint=%s,nodemask=%*pbl",
oom_constraint_text[oc->constraint],
- nodemask_pr_args(oc->nodemask),
- p->comm, p->pid, from_kuid(_user_ns, task_uid(p)));
+ nodemask_pr_args(oc->nodemask));
+ mem_cgroup_print_oom_context(oc->memcg, p);
cpuset_print_current_mems_allowed();
+ pr_cont(",task=%s,pid=%5d,uid=%5d\n", p->comm, p->pid,
+ from_kuid(_user_ns, task_uid(p)));
if (is_memcg_oom(oc))
- mem_cgroup_print_oom_info(oc->memcg, p);
+ mem_cgroup_print_oom_meminfo(oc->memcg);
else {
show_mem(SHOW_MEM_FILTER_NODES, oc->nodemask);
if (is_dump_unreclaim_slabs())
--
2.14.1
[PATCH v12 1/2] Reorganize the oom report in dump_header
From: yuzhoujian
OOM report contains several sections. The first one is the allocation
context that has triggered the OOM. Then we have cpuset context
followed by the stack trace of the OOM path. Followed by the oom
eligible tasks and the information about the chosen oom victim.
One thing that makes parsing more awkward than necessary is that we do
not have a single and easily parsable line about the oom context. This
patch is reorganizing the oom report to
1) who invoked oom and what was the allocation request
[ 126.168182] panic invoked oom-killer:
gfp_mask=0x6280ca(GFP_HIGHUSER_MOVABLE|__GFP_ZERO), order=0, oom_score_adj=0
2) OOM stack trace
[ 126.169806] CPU: 23 PID: 8668 Comm: panic Not tainted 4.18.0-rc3+ #42
[ 126.170494] Hardware name: Inspur SA5212M4/YZMB-00370-107, BIOS
4.1.10 11/14/2016
[ 126.171197] Call Trace:
[ 126.171901] dump_stack+0x5a/0x73
[ 126.172593] dump_header+0x58/0x2dc
[ 126.173294] oom_kill_process+0x228/0x420
[ 126.173999] ? oom_badness+0x2a/0x130
[ 126.174705] out_of_memory+0x11a/0x4a0
[ 126.175415] __alloc_pages_slowpath+0x7cc/0xa1e
[ 126.176128] ? __alloc_pages_slowpath+0x194/0xa1e
[ 126.176853] ? page_counter_try_charge+0x54/0xc0
[ 126.177580] __alloc_pages_nodemask+0x277/0x290
[ 126.178319] alloc_pages_vma+0x73/0x180
[ 126.179058] do_anonymous_page+0xed/0x5a0
[ 126.179825] __handle_mm_fault+0xbb3/0xe70
[ 126.180566] handle_mm_fault+0xfa/0x210
[ 126.181313] __do_page_fault+0x233/0x4c0
[ 126.182063] do_page_fault+0x32/0x140
[ 126.182812] ? page_fault+0x8/0x30
[ 126.183560] page_fault+0x1e/0x30
3) oom context (contrains and the chosen victim).
[ 126.190619]
oom-kill:constraint=CONSTRAINT_NONE,nodemask=(null),cpuset=/,mems_allowed=0-1,task=panic,pid=10235,uid=
0
An admin can easily get the full oom context at a single line which
makes parsing much easier.
Signed-off-by: yuzhoujian
---
Changes since v11:
- move the array of const char oom_constraint_text to oom_kill.c
- add the cpuset information in the one line output.
Changes since v10:
- divide the patch v8 into two parts. One part is to add the array of const
char and put enum
oom_constaint into oom.h; the other adds a new func to print the missing
information for the system-
wide oom report.
Changes since v9:
- divide the patch v8 into two parts. One part is to move enum oom_constraint
into memcontrol.h; the
other refactors the output info in the dump_header.
- replace orgin_memcg and kill_memcg with oom_memcg and task_memcg resptively.
Changes since v8:
- add the constraint in the oom_control structure.
- put enum oom_constraint and constraint array into the oom.h file.
- simplify the description for mem_cgroup_print_oom_context.
Changes since v7:
- add the constraint parameter to dump_header and oom_kill_process.
- remove the static char array in the mem_cgroup_print_oom_context, and
invoke pr_cont_cgroup_path to print memcg' name.
- combine the patchset v6 into one.
Changes since v6:
- divide the patch v5 into two parts. One part is to add an array of const char
and
put enum oom_constraint into the memcontrol.h; the other refactors the output
in the dump_header.
- limit the memory usage for the static char array by using NAME_MAX in the
mem_cgroup_print_oom_context.
- eliminate the spurious spaces in the oom's output and fix the spelling of
"constrain".
Changes since v5:
- add an array of const char for each constraint.
- replace all of the pr_cont with a single line print of the pr_info.
- put enum oom_constraint into the memcontrol.c file for printing oom
constraint.
Changes since v4:
- rename the helper's name to mem_cgroup_print_oom_context.
- rename the mem_cgroup_print_oom_info to mem_cgroup_print_oom_meminfo.
- add the constrain info in the dump_header.
Changes since v3:
- rename the helper's name to mem_cgroup_print_oom_memcg_name.
- add the rcu lock held to the helper.
- remove the print info of memcg's name in mem_cgroup_print_oom_info.
Changes since v2:
- add the mem_cgroup_print_memcg_name helper to print the memcg's
name which contains the task that will be killed by the oom-killer.
Changes since v1:
- replace adding mem_cgroup_print_oom_info with printing the memcg's
name only.
include/linux/oom.h| 10 ++
kernel/cgroup/cpuset.c | 4 ++--
mm/oom_kill.c | 36
mm/page_alloc.c| 4 ++--
4 files changed, 34 insertions(+), 20 deletions(-)
diff --git a/include/linux/oom.h b/include/linux/oom.h
index 6adac113e96d..3e5e01619bc8 100644
--- a/include/linux/oom.h
+++ b/include/linux/oom.h
@@ -15,6 +15,13 @@ struct notifier_block;
struct mem_cgroup;
struct task_struct;
+enum oom_constraint {
+ CONSTRAINT_NONE,
+ CONSTRAINT_CPUSET,
+ CONSTRAINT_MEMORY_POLICY,
+
[PATCH] perf: disable parallelism for 'make clean'
The Yocto build system does a 'make clean' when rebuilding due to changed dependencies, and that consistently fails for me (causing the whole BSP build to fail) with errors such as | find: '[...]/perf/1.0-r9/perf-1.0/plugin_mac80211.so': No such file or directory | find: '[...]/perf/1.0-r9/perf-1.0/plugin_mac80211.so': No such file or directory | find: find: '[...]/perf/1.0-r9/perf-1.0/libtraceevent.a''[...]/perf/1.0-r9/perf-1.0/libtraceevent.a': No such file or directory: No such file or directory | [...] | find: cannot delete '/mnt/xfs/devel/pil/yocto/tmp-glibc/work/wandboard-oe-linux-gnueabi/perf/1.0-r9/perf-1.0/util/.pstack.o.cmd': No such file or directory Apparently (despite the comment), 'make clean' ends up launching multiple sub-makes that all want to remove the same things - perhaps this only happens in combination with a O=... parameter. In any case, we don't lose much by explicitly disabling the parallelism for the clean target, and it makes automated builds much more reliable. Signed-off-by: Rasmus Villemoes --- Another option is to add -ignore_readdir_race to all find commands, but that's a lot more invasive and only works properly together with -delete with a very recent GNU find (https://savannah.gnu.org/bugs/?52981). tools/perf/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/perf/Makefile b/tools/perf/Makefile index 225454416ed5..7902a5681fc8 100644 --- a/tools/perf/Makefile +++ b/tools/perf/Makefile @@ -84,10 +84,10 @@ endif # has_clean endif # MAKECMDGOALS # -# The clean target is not really parallel, don't print the jobs info: +# Explicitly disable parallelism for the clean target. # clean: - $(make) + $(make) -j1 # # The build-test target is not really parallel, don't print the jobs info, -- 2.16.4
[PATCH] perf: disable parallelism for 'make clean'
The Yocto build system does a 'make clean' when rebuilding due to changed dependencies, and that consistently fails for me (causing the whole BSP build to fail) with errors such as | find: '[...]/perf/1.0-r9/perf-1.0/plugin_mac80211.so': No such file or directory | find: '[...]/perf/1.0-r9/perf-1.0/plugin_mac80211.so': No such file or directory | find: find: '[...]/perf/1.0-r9/perf-1.0/libtraceevent.a''[...]/perf/1.0-r9/perf-1.0/libtraceevent.a': No such file or directory: No such file or directory | [...] | find: cannot delete '/mnt/xfs/devel/pil/yocto/tmp-glibc/work/wandboard-oe-linux-gnueabi/perf/1.0-r9/perf-1.0/util/.pstack.o.cmd': No such file or directory Apparently (despite the comment), 'make clean' ends up launching multiple sub-makes that all want to remove the same things - perhaps this only happens in combination with a O=... parameter. In any case, we don't lose much by explicitly disabling the parallelism for the clean target, and it makes automated builds much more reliable. Signed-off-by: Rasmus Villemoes --- Another option is to add -ignore_readdir_race to all find commands, but that's a lot more invasive and only works properly together with -delete with a very recent GNU find (https://savannah.gnu.org/bugs/?52981). tools/perf/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/perf/Makefile b/tools/perf/Makefile index 225454416ed5..7902a5681fc8 100644 --- a/tools/perf/Makefile +++ b/tools/perf/Makefile @@ -84,10 +84,10 @@ endif # has_clean endif # MAKECMDGOALS # -# The clean target is not really parallel, don't print the jobs info: +# Explicitly disable parallelism for the clean target. # clean: - $(make) + $(make) -j1 # # The build-test target is not really parallel, don't print the jobs info, -- 2.16.4
[PATCH] nvme-rdma: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/nvme/host/rdma.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c index 518c5b0..9935234 100644 --- a/drivers/nvme/host/rdma.c +++ b/drivers/nvme/host/rdma.c @@ -1574,6 +1574,7 @@ static int nvme_rdma_cm_handler(struct rdma_cm_id *cm_id, case RDMA_CM_EVENT_CONNECT_ERROR: case RDMA_CM_EVENT_UNREACHABLE: nvme_rdma_destroy_queue_ib(queue); + /* fall through */ case RDMA_CM_EVENT_ADDR_ERROR: dev_dbg(queue->ctrl->ctrl.device, "CM error event %d\n", ev->event); -- 2.7.4
[PATCH] nvme-rdma: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/nvme/host/rdma.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c index 518c5b0..9935234 100644 --- a/drivers/nvme/host/rdma.c +++ b/drivers/nvme/host/rdma.c @@ -1574,6 +1574,7 @@ static int nvme_rdma_cm_handler(struct rdma_cm_id *cm_id, case RDMA_CM_EVENT_CONNECT_ERROR: case RDMA_CM_EVENT_UNREACHABLE: nvme_rdma_destroy_queue_ib(queue); + /* fall through */ case RDMA_CM_EVENT_ADDR_ERROR: dev_dbg(queue->ctrl->ctrl.device, "CM error event %d\n", ev->event); -- 2.7.4
Re: [PATCH] mmc: sdhci-of-dwcmshc: add SDHCI OF Synopsys DWC MSHC dirver
On 4 July 2018 at 11:45, Jisheng Zhang wrote:
> Add a driver for SDHCI OF Synopsys DesignWare Cores Mobile Storage
> Host Controller.
>
> Signed-off-by: Jisheng Zhang
> ---
> .../bindings/mmc/sdhci-of-dwcmshc.txt | 20 +++
Please split DT docs into separate patches.
Ehh, just make checkpatch not to complain, then I am happy with this patch!
Kind regards
Uffe
> drivers/mmc/host/Kconfig | 10 ++
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/sdhci-of-dwcmshc.c | 117 ++
> 4 files changed, 148 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-of-dwcmshc.txt
> create mode 100644 drivers/mmc/host/sdhci-of-dwcmshc.c
>
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-of-dwcmshc.txt
> b/Documentation/devicetree/bindings/mmc/sdhci-of-dwcmshc.txt
> new file mode 100644
> index ..ee4253b33be2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-of-dwcmshc.txt
> @@ -0,0 +1,20 @@
> +* Synopsys DesignWare Cores Mobile Storage Host Controller
> +
> +Required properties:
> +- compatible: should be one of the following:
> +"snps,dwcmshc-sdhci"
> +- reg: offset and length of the register set for the device.
> +- interrupts: a single interrupt specifier.
> +- clocks: Array of clocks required for SDHCI; requires at least one for
> +core clock.
> +- clock-names: Array of names corresponding to clocks property; shall be
> +"core" for core clock and "bus" for optional bus clock.
> +
> +Example:
> + sdhci2: sdhci@aa {
> + compatible = "snps,dwcmshc-sdhci";
> + reg = <0xaa 0x1000>;
> + interrupts = ;
> + clocks = <>;
> + bus-width = <8>;
> + }
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 0581c199c996..23beb9ec6dbf 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -176,6 +176,16 @@ config MMC_SDHCI_OF_HLWD
>
> If unsure, say N.
>
> +config MMC_SDHCI_OF_DWCMSHC
> + tristate "SDHCI OF support for the Synopsys DWC MSHC"
> + depends on MMC_SDHCI_PLTFM
> + depends on OF
> + depends on COMMON_CLK
> + help
> + This selects Synopsys DesignWare Cores Mobile Storage Controller.
> + If you have a controller with this interface, say Y or M here.
> + If unsure, say N.
> +
> config MMC_SDHCI_CADENCE
> tristate "SDHCI support for the Cadence SD/SDIO/eMMC controller"
> depends on MMC_SDHCI_PLTFM
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index 85dc1322c3de..a18fbba1b97e 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -82,6 +82,7 @@ obj-$(CONFIG_MMC_SDHCI_OF_ARASAN) += sdhci-of-arasan.o
> obj-$(CONFIG_MMC_SDHCI_OF_AT91)+= sdhci-of-at91.o
> obj-$(CONFIG_MMC_SDHCI_OF_ESDHC) += sdhci-of-esdhc.o
> obj-$(CONFIG_MMC_SDHCI_OF_HLWD)+= sdhci-of-hlwd.o
> +obj-$(CONFIG_MMC_SDHCI_OF_DWCMSHC) += sdhci-of-dwcmshc.o
> obj-$(CONFIG_MMC_SDHCI_BCM_KONA) += sdhci-bcm-kona.o
> obj-$(CONFIG_MMC_SDHCI_IPROC) += sdhci-iproc.o
> obj-$(CONFIG_MMC_SDHCI_MSM)+= sdhci-msm.o
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c
> b/drivers/mmc/host/sdhci-of-dwcmshc.c
> new file mode 100644
> index ..93b613cd7c33
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Synopsys DesignWare Cores Mobile Storage Host Controller
> + *
> + * Copyright (C) 2018 Synaptics Incorporated
> + *
> + * Author: Jisheng Zhang
> + */
> +
> +#include
> +#include
> +#include
> +
> +#include "sdhci-pltfm.h"
> +
> +struct dwcmshc_priv {
> + /* bus clock */
> + struct clk *bus_clk;
> +};
> +
> +static const struct sdhci_ops sdhci_dwcmshc_ops = {
> + .set_clock = sdhci_set_clock,
> + .set_bus_width = sdhci_set_bus_width,
> + .set_uhs_signaling = sdhci_set_uhs_signaling,
> + .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> + .reset = sdhci_reset,
> +};
> +
> +static const struct sdhci_pltfm_data sdhci_dwcmshc_pdata = {
> + .ops = _dwcmshc_ops,
> + .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> +};
> +
> +static int dwcmshc_probe(struct platform_device *pdev)
> +{
> + struct sdhci_pltfm_host *pltfm_host;
> + struct sdhci_host *host;
> + struct dwcmshc_priv *priv;
> + int err;
> +
> + host = sdhci_pltfm_init(pdev, _dwcmshc_pdata,
> + sizeof(struct dwcmshc_priv));
> + if (IS_ERR(host))
> + return PTR_ERR(host);
> +
> + pltfm_host = sdhci_priv(host);
> + priv = sdhci_pltfm_priv(pltfm_host);
> +
> + pltfm_host->clk = devm_clk_get(>dev, "core");
> + if
Re: [PATCH] mmc: sdhci-of-dwcmshc: add SDHCI OF Synopsys DWC MSHC dirver
On 4 July 2018 at 11:45, Jisheng Zhang wrote:
> Add a driver for SDHCI OF Synopsys DesignWare Cores Mobile Storage
> Host Controller.
>
> Signed-off-by: Jisheng Zhang
> ---
> .../bindings/mmc/sdhci-of-dwcmshc.txt | 20 +++
Please split DT docs into separate patches.
Ehh, just make checkpatch not to complain, then I am happy with this patch!
Kind regards
Uffe
> drivers/mmc/host/Kconfig | 10 ++
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/sdhci-of-dwcmshc.c | 117 ++
> 4 files changed, 148 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-of-dwcmshc.txt
> create mode 100644 drivers/mmc/host/sdhci-of-dwcmshc.c
>
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-of-dwcmshc.txt
> b/Documentation/devicetree/bindings/mmc/sdhci-of-dwcmshc.txt
> new file mode 100644
> index ..ee4253b33be2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-of-dwcmshc.txt
> @@ -0,0 +1,20 @@
> +* Synopsys DesignWare Cores Mobile Storage Host Controller
> +
> +Required properties:
> +- compatible: should be one of the following:
> +"snps,dwcmshc-sdhci"
> +- reg: offset and length of the register set for the device.
> +- interrupts: a single interrupt specifier.
> +- clocks: Array of clocks required for SDHCI; requires at least one for
> +core clock.
> +- clock-names: Array of names corresponding to clocks property; shall be
> +"core" for core clock and "bus" for optional bus clock.
> +
> +Example:
> + sdhci2: sdhci@aa {
> + compatible = "snps,dwcmshc-sdhci";
> + reg = <0xaa 0x1000>;
> + interrupts = ;
> + clocks = <>;
> + bus-width = <8>;
> + }
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 0581c199c996..23beb9ec6dbf 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -176,6 +176,16 @@ config MMC_SDHCI_OF_HLWD
>
> If unsure, say N.
>
> +config MMC_SDHCI_OF_DWCMSHC
> + tristate "SDHCI OF support for the Synopsys DWC MSHC"
> + depends on MMC_SDHCI_PLTFM
> + depends on OF
> + depends on COMMON_CLK
> + help
> + This selects Synopsys DesignWare Cores Mobile Storage Controller.
> + If you have a controller with this interface, say Y or M here.
> + If unsure, say N.
> +
> config MMC_SDHCI_CADENCE
> tristate "SDHCI support for the Cadence SD/SDIO/eMMC controller"
> depends on MMC_SDHCI_PLTFM
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index 85dc1322c3de..a18fbba1b97e 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -82,6 +82,7 @@ obj-$(CONFIG_MMC_SDHCI_OF_ARASAN) += sdhci-of-arasan.o
> obj-$(CONFIG_MMC_SDHCI_OF_AT91)+= sdhci-of-at91.o
> obj-$(CONFIG_MMC_SDHCI_OF_ESDHC) += sdhci-of-esdhc.o
> obj-$(CONFIG_MMC_SDHCI_OF_HLWD)+= sdhci-of-hlwd.o
> +obj-$(CONFIG_MMC_SDHCI_OF_DWCMSHC) += sdhci-of-dwcmshc.o
> obj-$(CONFIG_MMC_SDHCI_BCM_KONA) += sdhci-bcm-kona.o
> obj-$(CONFIG_MMC_SDHCI_IPROC) += sdhci-iproc.o
> obj-$(CONFIG_MMC_SDHCI_MSM)+= sdhci-msm.o
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c
> b/drivers/mmc/host/sdhci-of-dwcmshc.c
> new file mode 100644
> index ..93b613cd7c33
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Synopsys DesignWare Cores Mobile Storage Host Controller
> + *
> + * Copyright (C) 2018 Synaptics Incorporated
> + *
> + * Author: Jisheng Zhang
> + */
> +
> +#include
> +#include
> +#include
> +
> +#include "sdhci-pltfm.h"
> +
> +struct dwcmshc_priv {
> + /* bus clock */
> + struct clk *bus_clk;
> +};
> +
> +static const struct sdhci_ops sdhci_dwcmshc_ops = {
> + .set_clock = sdhci_set_clock,
> + .set_bus_width = sdhci_set_bus_width,
> + .set_uhs_signaling = sdhci_set_uhs_signaling,
> + .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> + .reset = sdhci_reset,
> +};
> +
> +static const struct sdhci_pltfm_data sdhci_dwcmshc_pdata = {
> + .ops = _dwcmshc_ops,
> + .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> +};
> +
> +static int dwcmshc_probe(struct platform_device *pdev)
> +{
> + struct sdhci_pltfm_host *pltfm_host;
> + struct sdhci_host *host;
> + struct dwcmshc_priv *priv;
> + int err;
> +
> + host = sdhci_pltfm_init(pdev, _dwcmshc_pdata,
> + sizeof(struct dwcmshc_priv));
> + if (IS_ERR(host))
> + return PTR_ERR(host);
> +
> + pltfm_host = sdhci_priv(host);
> + priv = sdhci_pltfm_priv(pltfm_host);
> +
> + pltfm_host->clk = devm_clk_get(>dev, "core");
> + if
Re: [PATCH] mmc: sdhci: do not try to use 3.3V signaling if not supported
On 5 July 2018 at 14:18, Stefan Agner wrote:
> For eMMC devices it is valid to only support 1.8V signaling. When
> vqmmc is set to a fixed 1.8V regulator the stack tries to set 3.3V
> initially and prints the following warning:
>mmc1: Switching to 3.3V signalling voltage failed
>
> Clear the MMC_SIGNAL_VOLTAGE_330 flag in case 3.3V is signaling is
> not available. This prevents the stack from even trying to use
> 3.3V signaling and avoids the above warning.
>
> Signed-off-by: Stefan Agner
Thanks, applied for next!
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 1c828e0e9905..a7b5602ef6f7 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -3734,14 +3734,21 @@ int sdhci_setup_host(struct sdhci_host *host)
> mmc_gpio_get_cd(host->mmc) < 0)
> mmc->caps |= MMC_CAP_NEEDS_POLL;
>
> - /* If vqmmc regulator and no 1.8V signalling, then there's no UHS */
> if (!IS_ERR(mmc->supply.vqmmc)) {
> ret = regulator_enable(mmc->supply.vqmmc);
> +
> + /* If vqmmc provides no 1.8V signalling, then there's no UHS
> */
> if (!regulator_is_supported_voltage(mmc->supply.vqmmc,
> 170,
> 195))
> host->caps1 &= ~(SDHCI_SUPPORT_SDR104 |
> SDHCI_SUPPORT_SDR50 |
> SDHCI_SUPPORT_DDR50);
> +
> + /* In eMMC case vqmmc might be a fixed 1.8V regulator */
> + if (!regulator_is_supported_voltage(mmc->supply.vqmmc,
> 270,
> + 360))
> + host->flags &= ~SDHCI_SIGNALING_330;
> +
> if (ret) {
> pr_warn("%s: Failed to enable vqmmc regulator: %d\n",
> mmc_hostname(mmc), ret);
> --
> 2.18.0
>
Re: [PATCH] mmc: sdhci: do not try to use 3.3V signaling if not supported
On 5 July 2018 at 14:18, Stefan Agner wrote:
> For eMMC devices it is valid to only support 1.8V signaling. When
> vqmmc is set to a fixed 1.8V regulator the stack tries to set 3.3V
> initially and prints the following warning:
>mmc1: Switching to 3.3V signalling voltage failed
>
> Clear the MMC_SIGNAL_VOLTAGE_330 flag in case 3.3V is signaling is
> not available. This prevents the stack from even trying to use
> 3.3V signaling and avoids the above warning.
>
> Signed-off-by: Stefan Agner
Thanks, applied for next!
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 1c828e0e9905..a7b5602ef6f7 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -3734,14 +3734,21 @@ int sdhci_setup_host(struct sdhci_host *host)
> mmc_gpio_get_cd(host->mmc) < 0)
> mmc->caps |= MMC_CAP_NEEDS_POLL;
>
> - /* If vqmmc regulator and no 1.8V signalling, then there's no UHS */
> if (!IS_ERR(mmc->supply.vqmmc)) {
> ret = regulator_enable(mmc->supply.vqmmc);
> +
> + /* If vqmmc provides no 1.8V signalling, then there's no UHS
> */
> if (!regulator_is_supported_voltage(mmc->supply.vqmmc,
> 170,
> 195))
> host->caps1 &= ~(SDHCI_SUPPORT_SDR104 |
> SDHCI_SUPPORT_SDR50 |
> SDHCI_SUPPORT_DDR50);
> +
> + /* In eMMC case vqmmc might be a fixed 1.8V regulator */
> + if (!regulator_is_supported_voltage(mmc->supply.vqmmc,
> 270,
> + 360))
> + host->flags &= ~SDHCI_SIGNALING_330;
> +
> if (ret) {
> pr_warn("%s: Failed to enable vqmmc regulator: %d\n",
> mmc_hostname(mmc), ret);
> --
> 2.18.0
>
Re: [PATCH v2] mmc: sdhci-esdhc-imx: allow 1.8V modes without 100/200MHz pinctrl states
On 4 July 2018 at 17:07, Stefan Agner wrote:
> If pinctrl nodes for 100/200MHz are missing, the controller should
> not select any mode which need signal frequencies 100MHz or higher.
> To prevent such speed modes the driver currently uses the quirk flag
> SDHCI_QUIRK2_NO_1_8_V. This works nicely for SD cards since 1.8V
> signaling is required for all faster modes and slower modes use 3.3V
> signaling only.
>
> However, there are eMMC modes which use 1.8V signaling and run below
> 100MHz, e.g. DDR52 at 1.8V. With using SDHCI_QUIRK2_NO_1_8_V this
> mode is prevented. When using a fixed 1.8V regulator as vqmmc-supply
> the stack has no valid mode to use. In this tenuous situation the
> kernel continuously prints voltage switching errors:
> mmc1: Switching to 3.3V signalling voltage failed
>
> Avoid using SDHCI_QUIRK2_NO_1_8_V and prevent faster modes by
> altering the SDHCI capability register. With that the stack is able
> to select 1.8V modes even if no faster pinctrl states are available:
> # cat /sys/kernel/debug/mmc1/ios
> ...
> timing spec:8 (mmc DDR52)
> signal voltage: 1 (1.80 V)
> ...
>
> Link: http://lkml.kernel.org/r/20180628081331.13051-1-ste...@agner.ch
> Signed-off-by: Stefan Agner
Thanks, applied for next! Let's see if this turns out okay, then let's
make it a fix and add a stable tag.
BTW, would you mind looking up the commit it fixes? Or if there is a
certain stable release we should target.
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci-esdhc-imx.c | 21 +
> 1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 20a420b765b3..e96d969ab2c3 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -312,6 +312,15 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int
> reg)
>
> if (imx_data->socdata->flags & ESDHC_FLAG_HS400)
> val |= SDHCI_SUPPORT_HS400;
> +
> + /*
> +* Do not advertise faster UHS modes if there are no
> +* pinctrl states for 100MHz/200MHz.
> +*/
> + if (IS_ERR_OR_NULL(imx_data->pins_100mhz) ||
> + IS_ERR_OR_NULL(imx_data->pins_200mhz))
> + val &= ~(SDHCI_SUPPORT_SDR50 |
> SDHCI_SUPPORT_DDR50
> +| SDHCI_SUPPORT_SDR104 |
> SDHCI_SUPPORT_HS400);
> }
> }
>
> @@ -1157,18 +1166,6 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
> ESDHC_PINCTRL_STATE_100MHZ);
> imx_data->pins_200mhz =
> pinctrl_lookup_state(imx_data->pinctrl,
> ESDHC_PINCTRL_STATE_200MHZ);
> - if (IS_ERR(imx_data->pins_100mhz) ||
> - IS_ERR(imx_data->pins_200mhz)) {
> - dev_warn(mmc_dev(host->mmc),
> - "could not get ultra high speed state, work
> on normal mode\n");
> - /*
> -* fall back to not supporting uhs by specifying no
> -* 1.8v quirk
> -*/
> - host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
> - }
> - } else {
> - host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
> }
>
> /* call to generic mmc_of_parse to support additional capabilities */
> --
> 2.18.0
>
Re: [PATCH v2] mmc: sdhci-esdhc-imx: allow 1.8V modes without 100/200MHz pinctrl states
On 4 July 2018 at 17:07, Stefan Agner wrote:
> If pinctrl nodes for 100/200MHz are missing, the controller should
> not select any mode which need signal frequencies 100MHz or higher.
> To prevent such speed modes the driver currently uses the quirk flag
> SDHCI_QUIRK2_NO_1_8_V. This works nicely for SD cards since 1.8V
> signaling is required for all faster modes and slower modes use 3.3V
> signaling only.
>
> However, there are eMMC modes which use 1.8V signaling and run below
> 100MHz, e.g. DDR52 at 1.8V. With using SDHCI_QUIRK2_NO_1_8_V this
> mode is prevented. When using a fixed 1.8V regulator as vqmmc-supply
> the stack has no valid mode to use. In this tenuous situation the
> kernel continuously prints voltage switching errors:
> mmc1: Switching to 3.3V signalling voltage failed
>
> Avoid using SDHCI_QUIRK2_NO_1_8_V and prevent faster modes by
> altering the SDHCI capability register. With that the stack is able
> to select 1.8V modes even if no faster pinctrl states are available:
> # cat /sys/kernel/debug/mmc1/ios
> ...
> timing spec:8 (mmc DDR52)
> signal voltage: 1 (1.80 V)
> ...
>
> Link: http://lkml.kernel.org/r/20180628081331.13051-1-ste...@agner.ch
> Signed-off-by: Stefan Agner
Thanks, applied for next! Let's see if this turns out okay, then let's
make it a fix and add a stable tag.
BTW, would you mind looking up the commit it fixes? Or if there is a
certain stable release we should target.
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci-esdhc-imx.c | 21 +
> 1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 20a420b765b3..e96d969ab2c3 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -312,6 +312,15 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int
> reg)
>
> if (imx_data->socdata->flags & ESDHC_FLAG_HS400)
> val |= SDHCI_SUPPORT_HS400;
> +
> + /*
> +* Do not advertise faster UHS modes if there are no
> +* pinctrl states for 100MHz/200MHz.
> +*/
> + if (IS_ERR_OR_NULL(imx_data->pins_100mhz) ||
> + IS_ERR_OR_NULL(imx_data->pins_200mhz))
> + val &= ~(SDHCI_SUPPORT_SDR50 |
> SDHCI_SUPPORT_DDR50
> +| SDHCI_SUPPORT_SDR104 |
> SDHCI_SUPPORT_HS400);
> }
> }
>
> @@ -1157,18 +1166,6 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
> ESDHC_PINCTRL_STATE_100MHZ);
> imx_data->pins_200mhz =
> pinctrl_lookup_state(imx_data->pinctrl,
> ESDHC_PINCTRL_STATE_200MHZ);
> - if (IS_ERR(imx_data->pins_100mhz) ||
> - IS_ERR(imx_data->pins_200mhz)) {
> - dev_warn(mmc_dev(host->mmc),
> - "could not get ultra high speed state, work
> on normal mode\n");
> - /*
> -* fall back to not supporting uhs by specifying no
> -* 1.8v quirk
> -*/
> - host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
> - }
> - } else {
> - host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
> }
>
> /* call to generic mmc_of_parse to support additional capabilities */
> --
> 2.18.0
>
Re: [PATCH] mmc: sdhci-xenon: mark expected switch fall-through
On 4 July 2018 at 00:56, Gustavo A. R. Silva wrote: > In preparation to enabling -Wimplicit-fallthrough, mark switch cases > where we are expecting to fall through. > > Warning level 2 was used: -Wimplicit-fallthrough=2 > > Signed-off-by: Gustavo A. R. Silva Thanks, applied for next! Kind regards Uffe > --- > drivers/mmc/host/sdhci-xenon-phy.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/mmc/host/sdhci-xenon-phy.c > b/drivers/mmc/host/sdhci-xenon-phy.c > index a35804b..c335052 100644 > --- a/drivers/mmc/host/sdhci-xenon-phy.c > +++ b/drivers/mmc/host/sdhci-xenon-phy.c > @@ -526,6 +526,7 @@ static bool xenon_emmc_phy_slow_mode(struct sdhci_host > *host, > ret = true; > break; > } > + /* else: fall through */ > default: > reg &= ~XENON_TIMING_ADJUST_SLOW_MODE; > ret = false; > -- > 2.7.4 >
Re: [PATCH] mmc: sdhci-esdhc-imx: disable clocks before changing frequency
On 5 July 2018 at 14:15, Stefan Agner wrote:
> In the uSDHC case (e.g. i.MX 6) clocks only get disabled if frequency
> is set to 0. However, it could be that the stack asks for a frequency
> change while clocks are on. In that case the function clears the
> divider registers (by clearing ESDHC_CLOCK_MASK) while the clock is
> enabled! This causes a short period of time where the clock is
> undivided (on a i.MX 6DL a clock of 196MHz has been measured).
>
> For older IP variants the driver disables clock by clearing some bits
> in ESDHC_SYSTEM_CONTROL.
>
> Make sure to disable card clock before chainging frequency for uSDHC
> IP variants too. Also fix indent to make disable/enable clock look
> alike.
>
> Signed-off-by: Stefan Agner
Thanks, applied for next!
Please tell if you want this for fixes and if I should add a stable tag.
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci-esdhc-imx.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 85fd5a8b0b6d..aa48f4b2541a 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -708,14 +708,14 @@ static inline void esdhc_pltfm_set_clock(struct
> sdhci_host *host,
> int div = 1;
> u32 temp, val;
>
> + if (esdhc_is_usdhc(imx_data)) {
> + val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
> + writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> + host->ioaddr + ESDHC_VENDOR_SPEC);
> + }
> +
> if (clock == 0) {
> host->mmc->actual_clock = 0;
> -
> - if (esdhc_is_usdhc(imx_data)) {
> - val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
> - writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> - host->ioaddr + ESDHC_VENDOR_SPEC);
> - }
> return;
> }
>
> @@ -761,7 +761,7 @@ static inline void esdhc_pltfm_set_clock(struct
> sdhci_host *host,
> if (esdhc_is_usdhc(imx_data)) {
> val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
> writel(val | ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> - host->ioaddr + ESDHC_VENDOR_SPEC);
> + host->ioaddr + ESDHC_VENDOR_SPEC);
> }
>
> mdelay(1);
> --
> 2.18.0
>
Re: [PATCH v2] mmc: sdhci-of-esdhc: set proper dma mask for ls104x chips
On 4 July 2018 at 13:34, Laurentiu Tudor wrote:
> SDHCI controller in ls1043a and ls1046a generate 40-bit wide addresses
> when doing DMA. Make sure that the corresponding dma mask is correctly
> configured.
>
> Context: when enabling smmu on these chips the following problem is
> encountered: the smmu input address size is 48 bits so the dma mappings
> for sdhci end up 48-bit wide. However, on these chips sdhci only use
> 40-bits of that address size when doing dma.
> So you end up with a 48-bit address translation in smmu but the device
> generates transactions with clipped 40-bit addresses, thus smmu context
> faults are triggered. Setting up the correct dma mask fixes this
> situation.
>
> Signed-off-by: Laurentiu Tudor
Thanks, re-placed v1 with v2 on my next branch.
Kind regards
Uffe
> ---
> Changes in v2:
> - updated commit log with some context
>
> drivers/mmc/host/sdhci-of-esdhc.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-esdhc.c
> b/drivers/mmc/host/sdhci-of-esdhc.c
> index 4ffa6b173a21..8332f56e6c0d 100644
> --- a/drivers/mmc/host/sdhci-of-esdhc.c
> +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> @@ -22,6 +22,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include "sdhci-pltfm.h"
> #include "sdhci-esdhc.h"
> @@ -427,6 +428,11 @@ static void esdhc_of_adma_workaround(struct sdhci_host
> *host, u32 intmask)
> static int esdhc_of_enable_dma(struct sdhci_host *host)
> {
> u32 value;
> + struct device *dev = mmc_dev(host->mmc);
> +
> + if (of_device_is_compatible(dev->of_node, "fsl,ls1043a-esdhc") ||
> + of_device_is_compatible(dev->of_node, "fsl,ls1046a-esdhc"))
> + dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
>
> value = sdhci_readl(host, ESDHC_DMA_SYSCTL);
> value |= ESDHC_DMA_SNOOP;
> --
> 2.17.1
>
Re: [PATCH] mmc: sdhci-xenon: mark expected switch fall-through
On 4 July 2018 at 00:56, Gustavo A. R. Silva wrote: > In preparation to enabling -Wimplicit-fallthrough, mark switch cases > where we are expecting to fall through. > > Warning level 2 was used: -Wimplicit-fallthrough=2 > > Signed-off-by: Gustavo A. R. Silva Thanks, applied for next! Kind regards Uffe > --- > drivers/mmc/host/sdhci-xenon-phy.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/mmc/host/sdhci-xenon-phy.c > b/drivers/mmc/host/sdhci-xenon-phy.c > index a35804b..c335052 100644 > --- a/drivers/mmc/host/sdhci-xenon-phy.c > +++ b/drivers/mmc/host/sdhci-xenon-phy.c > @@ -526,6 +526,7 @@ static bool xenon_emmc_phy_slow_mode(struct sdhci_host > *host, > ret = true; > break; > } > + /* else: fall through */ > default: > reg &= ~XENON_TIMING_ADJUST_SLOW_MODE; > ret = false; > -- > 2.7.4 >
Re: [PATCH] mmc: sdhci-esdhc-imx: disable clocks before changing frequency
On 5 July 2018 at 14:15, Stefan Agner wrote:
> In the uSDHC case (e.g. i.MX 6) clocks only get disabled if frequency
> is set to 0. However, it could be that the stack asks for a frequency
> change while clocks are on. In that case the function clears the
> divider registers (by clearing ESDHC_CLOCK_MASK) while the clock is
> enabled! This causes a short period of time where the clock is
> undivided (on a i.MX 6DL a clock of 196MHz has been measured).
>
> For older IP variants the driver disables clock by clearing some bits
> in ESDHC_SYSTEM_CONTROL.
>
> Make sure to disable card clock before chainging frequency for uSDHC
> IP variants too. Also fix indent to make disable/enable clock look
> alike.
>
> Signed-off-by: Stefan Agner
Thanks, applied for next!
Please tell if you want this for fixes and if I should add a stable tag.
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci-esdhc-imx.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 85fd5a8b0b6d..aa48f4b2541a 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -708,14 +708,14 @@ static inline void esdhc_pltfm_set_clock(struct
> sdhci_host *host,
> int div = 1;
> u32 temp, val;
>
> + if (esdhc_is_usdhc(imx_data)) {
> + val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
> + writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> + host->ioaddr + ESDHC_VENDOR_SPEC);
> + }
> +
> if (clock == 0) {
> host->mmc->actual_clock = 0;
> -
> - if (esdhc_is_usdhc(imx_data)) {
> - val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
> - writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> - host->ioaddr + ESDHC_VENDOR_SPEC);
> - }
> return;
> }
>
> @@ -761,7 +761,7 @@ static inline void esdhc_pltfm_set_clock(struct
> sdhci_host *host,
> if (esdhc_is_usdhc(imx_data)) {
> val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
> writel(val | ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> - host->ioaddr + ESDHC_VENDOR_SPEC);
> + host->ioaddr + ESDHC_VENDOR_SPEC);
> }
>
> mdelay(1);
> --
> 2.18.0
>
Re: [PATCH v2] mmc: sdhci-of-esdhc: set proper dma mask for ls104x chips
On 4 July 2018 at 13:34, Laurentiu Tudor wrote:
> SDHCI controller in ls1043a and ls1046a generate 40-bit wide addresses
> when doing DMA. Make sure that the corresponding dma mask is correctly
> configured.
>
> Context: when enabling smmu on these chips the following problem is
> encountered: the smmu input address size is 48 bits so the dma mappings
> for sdhci end up 48-bit wide. However, on these chips sdhci only use
> 40-bits of that address size when doing dma.
> So you end up with a 48-bit address translation in smmu but the device
> generates transactions with clipped 40-bit addresses, thus smmu context
> faults are triggered. Setting up the correct dma mask fixes this
> situation.
>
> Signed-off-by: Laurentiu Tudor
Thanks, re-placed v1 with v2 on my next branch.
Kind regards
Uffe
> ---
> Changes in v2:
> - updated commit log with some context
>
> drivers/mmc/host/sdhci-of-esdhc.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-esdhc.c
> b/drivers/mmc/host/sdhci-of-esdhc.c
> index 4ffa6b173a21..8332f56e6c0d 100644
> --- a/drivers/mmc/host/sdhci-of-esdhc.c
> +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> @@ -22,6 +22,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include "sdhci-pltfm.h"
> #include "sdhci-esdhc.h"
> @@ -427,6 +428,11 @@ static void esdhc_of_adma_workaround(struct sdhci_host
> *host, u32 intmask)
> static int esdhc_of_enable_dma(struct sdhci_host *host)
> {
> u32 value;
> + struct device *dev = mmc_dev(host->mmc);
> +
> + if (of_device_is_compatible(dev->of_node, "fsl,ls1043a-esdhc") ||
> + of_device_is_compatible(dev->of_node, "fsl,ls1046a-esdhc"))
> + dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
>
> value = sdhci_readl(host, ESDHC_DMA_SYSCTL);
> value |= ESDHC_DMA_SNOOP;
> --
> 2.17.1
>
Re: [PATCH] USB: serial: cp210x: Implement GPIO support for CP2102N
On Fri, Jun 22, 2018 at 04:10:21PM +0100, Martyn Welch wrote: > On Wed, 2018-06-20 at 19:41 +, Karoly Pados wrote: > > Here I argue the following multiple ways. First, I say that claiming > > that a pin which is used as an input is actually an output is not > > only confusing, but also much less correct than thinking of it as an > > input pin with a weak pullup to prevent floating signals. Second, > > the pullups - while not explicitly listed in the datasheet - can be > > calculated from what is there, and for the cp2105 are typically > > 132k, for the cp2102n even higher around 165k. These are pretty > > weak pullups, so weak that they won't matter for the vast majority > > of applications as people rarely use pull-ups or pull- downs higher > > than 100k (not never, but rarely). So claiming that it can result in > > false expectation, while not completely wrong, is favoring the > > needs of a few instead of the much more common practice. > > > > Lastly, and maybe most importantly, I argue that calling everything > > an "output" pin only in name does not actually avoid any design > > errors, as the same circuit that would case a false reading in one > > case would also cause the same false reading in the other, and the > > circuits are usually developed before the software. So it'll be too > > late anyway by the time somebody realizes such a mistake. But on > > the contrary, it opens up more opportunities for errors, because > > now you are open to software bugs that ignore a pin's direction > > because everything's an output either way even when it really > > isn't, and think that they can treat it as as open-drain while for > > some reason it is in push-pull mode. Worse, even if it is in > > open-drain mode, it will only work with a specific output values - > > it must be high, which is not the default. With my proposal, > > setting a pin's direction to "input" will make sure it cannot be > > actively driven by the chip, avoiding such "misunderstandings" and > > errors, and similar measures are also in place for the push-pull > > pins. > > Yeah, I'll go with that. :-) Sounds good to me too. Thanks to both of you for spelling this out. > > The only problem I can see is if there isn't a way for the cp2105 to > > query the reset values of the pins (maybe there is, I just haven't > > looked into it). Then I don't know how the direction could be > > determined for an open-drain pin during initialization. But this is > > solved for the cp2102n, and then it is a device-specific issue for > > the cp2105, which shouldn't be forced onto other devices if we > > otherwise decide the approach to be inferior. > > I'm pretty sure there is a way to determine the pin state, though > unfortunately I no longer have access to the HW to be able to test... If this isn't (yet) possible, it's never wrong to continue treating the cp2105 pins as (open-drain) outputs. Thanks, Johan
Re: [PATCH] USB: serial: cp210x: Implement GPIO support for CP2102N
On Fri, Jun 22, 2018 at 04:10:21PM +0100, Martyn Welch wrote: > On Wed, 2018-06-20 at 19:41 +, Karoly Pados wrote: > > Here I argue the following multiple ways. First, I say that claiming > > that a pin which is used as an input is actually an output is not > > only confusing, but also much less correct than thinking of it as an > > input pin with a weak pullup to prevent floating signals. Second, > > the pullups - while not explicitly listed in the datasheet - can be > > calculated from what is there, and for the cp2105 are typically > > 132k, for the cp2102n even higher around 165k. These are pretty > > weak pullups, so weak that they won't matter for the vast majority > > of applications as people rarely use pull-ups or pull- downs higher > > than 100k (not never, but rarely). So claiming that it can result in > > false expectation, while not completely wrong, is favoring the > > needs of a few instead of the much more common practice. > > > > Lastly, and maybe most importantly, I argue that calling everything > > an "output" pin only in name does not actually avoid any design > > errors, as the same circuit that would case a false reading in one > > case would also cause the same false reading in the other, and the > > circuits are usually developed before the software. So it'll be too > > late anyway by the time somebody realizes such a mistake. But on > > the contrary, it opens up more opportunities for errors, because > > now you are open to software bugs that ignore a pin's direction > > because everything's an output either way even when it really > > isn't, and think that they can treat it as as open-drain while for > > some reason it is in push-pull mode. Worse, even if it is in > > open-drain mode, it will only work with a specific output values - > > it must be high, which is not the default. With my proposal, > > setting a pin's direction to "input" will make sure it cannot be > > actively driven by the chip, avoiding such "misunderstandings" and > > errors, and similar measures are also in place for the push-pull > > pins. > > Yeah, I'll go with that. :-) Sounds good to me too. Thanks to both of you for spelling this out. > > The only problem I can see is if there isn't a way for the cp2105 to > > query the reset values of the pins (maybe there is, I just haven't > > looked into it). Then I don't know how the direction could be > > determined for an open-drain pin during initialization. But this is > > solved for the cp2102n, and then it is a device-specific issue for > > the cp2105, which shouldn't be forced onto other devices if we > > otherwise decide the approach to be inferior. > > I'm pretty sure there is a way to determine the pin state, though > unfortunately I no longer have access to the HW to be able to test... If this isn't (yet) possible, it's never wrong to continue treating the cp2105 pins as (open-drain) outputs. Thanks, Johan
