Re: [PATCH v3 05/16] mtd: rawnand: qcom: remove dt property nand-ecc-step-size
On 2018-05-26 14:12, Miquel Raynal wrote: Hi Abhishek, On Fri, 25 May 2018 17:51:33 +0530, Abhishek Sahuwrote: QCOM NAND controller supports only one step size (512) so nand-ecc-step-size DT property is redundant. This property can be removed and ecc step size can be assigned with 512 value. Signed-off-by: Abhishek Sahu --- * Changes from v2: NEW CHANGE 1. Removed the custom logic and used the helper fuction. drivers/mtd/nand/raw/qcom_nandc.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c index b554fb6..b538390 100644 --- a/drivers/mtd/nand/raw/qcom_nandc.c +++ b/drivers/mtd/nand/raw/qcom_nandc.c @@ -2325,15 +2325,8 @@ static int qcom_nand_host_setup(struct qcom_nand_host *host) bool wide_bus; int ecc_mode = 1; - /* -* the controller requires each step consists of 512 bytes of data. -* bail out if DT has populated a wrong step size. -*/ - if (ecc->size != NANDC_STEP_SIZE) { - dev_err(nandc->dev, "invalid ecc size\n"); - return -EINVAL; - } - + /* controller only supports 512 bytes of data in each step */ "512 bytes data steps" Thanks Miquel. Will update that. Regards, Abhishek + ecc->size = NANDC_STEP_SIZE; wide_bus = chip->options & NAND_BUSWIDTH_16 ? true : false; if (ecc->strength >= 8) { Once corrected: Acked-by: Miquel Raynal
Re: [PATCH v3 05/16] mtd: rawnand: qcom: remove dt property nand-ecc-step-size
On 2018-05-26 14:12, Miquel Raynal wrote: Hi Abhishek, On Fri, 25 May 2018 17:51:33 +0530, Abhishek Sahu wrote: QCOM NAND controller supports only one step size (512) so nand-ecc-step-size DT property is redundant. This property can be removed and ecc step size can be assigned with 512 value. Signed-off-by: Abhishek Sahu --- * Changes from v2: NEW CHANGE 1. Removed the custom logic and used the helper fuction. drivers/mtd/nand/raw/qcom_nandc.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c index b554fb6..b538390 100644 --- a/drivers/mtd/nand/raw/qcom_nandc.c +++ b/drivers/mtd/nand/raw/qcom_nandc.c @@ -2325,15 +2325,8 @@ static int qcom_nand_host_setup(struct qcom_nand_host *host) bool wide_bus; int ecc_mode = 1; - /* -* the controller requires each step consists of 512 bytes of data. -* bail out if DT has populated a wrong step size. -*/ - if (ecc->size != NANDC_STEP_SIZE) { - dev_err(nandc->dev, "invalid ecc size\n"); - return -EINVAL; - } - + /* controller only supports 512 bytes of data in each step */ "512 bytes data steps" Thanks Miquel. Will update that. Regards, Abhishek + ecc->size = NANDC_STEP_SIZE; wide_bus = chip->options & NAND_BUSWIDTH_16 ? true : false; if (ecc->strength >= 8) { Once corrected: Acked-by: Miquel Raynal
Re: [PATCH v3 03/16] dt-bindings: qcom_nandc: make nand-ecc-strength optional
On 2018-05-26 14:12, Miquel Raynal wrote: Hi Abhishek, On Fri, 25 May 2018 17:51:31 +0530, Abhishek Sahuwrote: If nand-ecc-strength specified in DT, then controller will use this ECC strength otherwise ECC strength will be calculated according to chip requirement and available OOB size. Signed-off-by: Abhishek Sahu --- * Changes from v2: NONE * Changes from v1: NEW PATCH Documentation/devicetree/bindings/mtd/qcom_nandc.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt index 73d336be..f246aa0 100644 --- a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt @@ -45,11 +45,13 @@ Required properties: number (e.g., 0, 1, 2, etc.) - #address-cells: see partition.txt - #size-cells: see partition.txt -- nand-ecc-strength: see nand.txt - nand-ecc-step-size: must be 512. see nand.txt for more details. I think you can squash the two dt-bindings commits as they are tightly related to each other. Sure Miquel. Earlier made one patch and then split into two. Will squash that and make single patch again :-) Thanks, Abhishek Optional properties: - nand-bus-width: see nand.txt +- nand-ecc-strength: see nand.txt. If not specified, then ECC strength will + be used according to chip requirement and available + OOB size. Each nandcs device node may optionally contain a 'partitions' sub-node, which further contains sub-nodes describing the flash partition mapping. See
Re: [PATCH v3 03/16] dt-bindings: qcom_nandc: make nand-ecc-strength optional
On 2018-05-26 14:12, Miquel Raynal wrote: Hi Abhishek, On Fri, 25 May 2018 17:51:31 +0530, Abhishek Sahu wrote: If nand-ecc-strength specified in DT, then controller will use this ECC strength otherwise ECC strength will be calculated according to chip requirement and available OOB size. Signed-off-by: Abhishek Sahu --- * Changes from v2: NONE * Changes from v1: NEW PATCH Documentation/devicetree/bindings/mtd/qcom_nandc.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt index 73d336be..f246aa0 100644 --- a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt @@ -45,11 +45,13 @@ Required properties: number (e.g., 0, 1, 2, etc.) - #address-cells: see partition.txt - #size-cells: see partition.txt -- nand-ecc-strength: see nand.txt - nand-ecc-step-size: must be 512. see nand.txt for more details. I think you can squash the two dt-bindings commits as they are tightly related to each other. Sure Miquel. Earlier made one patch and then split into two. Will squash that and make single patch again :-) Thanks, Abhishek Optional properties: - nand-bus-width: see nand.txt +- nand-ecc-strength: see nand.txt. If not specified, then ECC strength will + be used according to chip requirement and available + OOB size. Each nandcs device node may optionally contain a 'partitions' sub-node, which further contains sub-nodes describing the flash partition mapping. See
Re: [PATCH v3 01/16] mtd: rawnand: helper function for setting up ECC configuration
On 2018-05-26 14:12, Miquel Raynal wrote: Hi Abhishek, On Fri, 25 May 2018 17:51:29 +0530, Abhishek Sahuwrote: commit 2c8f8afa7f92 ("mtd: nand: add generic helpers to check, match, maximize ECC settings") provides generic helpers which drivers can use for setting up ECC parameters. Since same board can have different ECC strength nand chips so following is the logic for setting up ECC strength and ECC step size, which can be used by most of the drivers. 1. If both ECC step size and ECC strength are already set (usually by DT) then just check whether this setting is supported by NAND controller. 2. If NAND_ECC_MAXIMIZE is set, then select maximum ECC strength supported by NAND controller. 3. Otherwise, try to match the ECC step size and ECC strength closest to the chip's requirement. If available OOB size can't fit the chip requirement then select maximum ECC strength which can be fit with available OOB size. This patch introduces nand_ecc_choose_conf function which calls the required helper functions for the above logic. The drivers can use this single function instead of calling the 3 helper functions individually. CC: Masahiro Yamada Signed-off-by: Abhishek Sahu --- * Changes from v2: 1. Renamed function to nand_ecc_choose_conf. 2. Minor code reorganization to remove warning and 2 function calls for nand_maximize_ecc. * Changes from v1: NEW PATCH drivers/mtd/nand/raw/nand_base.c | 42 drivers/mtd/nand/raw/nand_base.c | 31 +++ include/linux/mtd/rawnand.h | 3 +++ 2 files changed, 34 insertions(+) diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index 72f3a89..e52019d 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -6249,6 +6249,37 @@ int nand_maximize_ecc(struct nand_chip *chip, } EXPORT_SYMBOL_GPL(nand_maximize_ecc); +/** + * nand_ecc_choose_conf - Set the ECC strength and ECC step size + * @chip: nand chip info structure + * @caps: ECC engine caps info structure + * @oobavail: OOB size that the ECC engine can use + * + * Choose the ECC configuration according to following logic + * + * 1. If both ECC step size and ECC strength are already set (usually by DT) + *then check if it is supported by this controller. + * 2. If NAND_ECC_MAXIMIZE is set, then select maximum ECC strength. + * 3. Otherwise, try to match the ECC step size and ECC strength closest + *to the chip's requirement. If available OOB size can't fit the chip + *requirement then fallback to the maximum ECC step size and ECC strength. + * + * On success, the chosen ECC settings are set. + */ +int nand_ecc_choose_conf(struct nand_chip *chip, +const struct nand_ecc_caps *caps, int oobavail) +{ + if (chip->ecc.size && chip->ecc.strength) + return nand_check_ecc_caps(chip, caps, oobavail); + + if (!(chip->ecc.options & NAND_ECC_MAXIMIZE) && + !nand_match_ecc_req(chip, caps, oobavail)) + return 0; + + return nand_maximize_ecc(chip, caps, oobavail); I personally don't mind if nand_maximize_ecc() is called twice in the function if it clarifies the logic. Maybe the following will be more clear for the user? Thanks Miquel. Both the implementations are fine. The above implementation (which was in Denali NAND driver) code was also clear. We can go for any of these implementation. Shall I update this ? if (chip->ecc.size && chip->ecc.strength) return nand_check_ecc_caps(chip, caps, oobavail); if (chip->ecc.options & NAND_ECC_MAXIMIZE) return nand_maximize_ecc(chip, caps, oobavail); if (!nand_match_ecc_req(chip, caps, oobavail)) return 0; return nand_maximize_ecc(chip, caps, oobavail); Also, I'm not sure we should just error out when nand_check_ecc_caps() fails. What about something more robust, like: But again, It will lead in overriding the DT ECC strength parameter. We started our discussion from that point. :-) Thanks, Abhishek int ret; if (chip->ecc.size && chip->ecc.strength) { ret = nand_check_ecc_caps(chip, caps, oobavail); if (ret) goto maximize_ecc; return 0; } if (chip->ecc.options & NAND_ECC_MAXIMIZE) goto maximize_ecc; ret = nand_match_ecc_req(chip, caps, oobavail); if (ret) goto maximize_ecc; return 0; maximize_ecc: return nand_maximize_ecc(chip, caps, oobavail); +} +EXPORT_SYMBOL_GPL(nand_ecc_choose_conf); + /* * Check if the chip configuration meet the datasheet requirements. diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h index 5dad59b..89889fa 100644 ---
Re: [PATCH v3 01/16] mtd: rawnand: helper function for setting up ECC configuration
On 2018-05-26 14:12, Miquel Raynal wrote: Hi Abhishek, On Fri, 25 May 2018 17:51:29 +0530, Abhishek Sahu wrote: commit 2c8f8afa7f92 ("mtd: nand: add generic helpers to check, match, maximize ECC settings") provides generic helpers which drivers can use for setting up ECC parameters. Since same board can have different ECC strength nand chips so following is the logic for setting up ECC strength and ECC step size, which can be used by most of the drivers. 1. If both ECC step size and ECC strength are already set (usually by DT) then just check whether this setting is supported by NAND controller. 2. If NAND_ECC_MAXIMIZE is set, then select maximum ECC strength supported by NAND controller. 3. Otherwise, try to match the ECC step size and ECC strength closest to the chip's requirement. If available OOB size can't fit the chip requirement then select maximum ECC strength which can be fit with available OOB size. This patch introduces nand_ecc_choose_conf function which calls the required helper functions for the above logic. The drivers can use this single function instead of calling the 3 helper functions individually. CC: Masahiro Yamada Signed-off-by: Abhishek Sahu --- * Changes from v2: 1. Renamed function to nand_ecc_choose_conf. 2. Minor code reorganization to remove warning and 2 function calls for nand_maximize_ecc. * Changes from v1: NEW PATCH drivers/mtd/nand/raw/nand_base.c | 42 drivers/mtd/nand/raw/nand_base.c | 31 +++ include/linux/mtd/rawnand.h | 3 +++ 2 files changed, 34 insertions(+) diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index 72f3a89..e52019d 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -6249,6 +6249,37 @@ int nand_maximize_ecc(struct nand_chip *chip, } EXPORT_SYMBOL_GPL(nand_maximize_ecc); +/** + * nand_ecc_choose_conf - Set the ECC strength and ECC step size + * @chip: nand chip info structure + * @caps: ECC engine caps info structure + * @oobavail: OOB size that the ECC engine can use + * + * Choose the ECC configuration according to following logic + * + * 1. If both ECC step size and ECC strength are already set (usually by DT) + *then check if it is supported by this controller. + * 2. If NAND_ECC_MAXIMIZE is set, then select maximum ECC strength. + * 3. Otherwise, try to match the ECC step size and ECC strength closest + *to the chip's requirement. If available OOB size can't fit the chip + *requirement then fallback to the maximum ECC step size and ECC strength. + * + * On success, the chosen ECC settings are set. + */ +int nand_ecc_choose_conf(struct nand_chip *chip, +const struct nand_ecc_caps *caps, int oobavail) +{ + if (chip->ecc.size && chip->ecc.strength) + return nand_check_ecc_caps(chip, caps, oobavail); + + if (!(chip->ecc.options & NAND_ECC_MAXIMIZE) && + !nand_match_ecc_req(chip, caps, oobavail)) + return 0; + + return nand_maximize_ecc(chip, caps, oobavail); I personally don't mind if nand_maximize_ecc() is called twice in the function if it clarifies the logic. Maybe the following will be more clear for the user? Thanks Miquel. Both the implementations are fine. The above implementation (which was in Denali NAND driver) code was also clear. We can go for any of these implementation. Shall I update this ? if (chip->ecc.size && chip->ecc.strength) return nand_check_ecc_caps(chip, caps, oobavail); if (chip->ecc.options & NAND_ECC_MAXIMIZE) return nand_maximize_ecc(chip, caps, oobavail); if (!nand_match_ecc_req(chip, caps, oobavail)) return 0; return nand_maximize_ecc(chip, caps, oobavail); Also, I'm not sure we should just error out when nand_check_ecc_caps() fails. What about something more robust, like: But again, It will lead in overriding the DT ECC strength parameter. We started our discussion from that point. :-) Thanks, Abhishek int ret; if (chip->ecc.size && chip->ecc.strength) { ret = nand_check_ecc_caps(chip, caps, oobavail); if (ret) goto maximize_ecc; return 0; } if (chip->ecc.options & NAND_ECC_MAXIMIZE) goto maximize_ecc; ret = nand_match_ecc_req(chip, caps, oobavail); if (ret) goto maximize_ecc; return 0; maximize_ecc: return nand_maximize_ecc(chip, caps, oobavail); +} +EXPORT_SYMBOL_GPL(nand_ecc_choose_conf); + /* * Check if the chip configuration meet the datasheet requirements. diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h index 5dad59b..89889fa 100644 --- a/include/linux/mtd/rawnand.h +++ b/include/linux/mtd/rawnand.h @@ -1627,6 +1627,9
[PATCH 3/3] thermal: broadcom: Add Stingray thermal driver
From: Pramod KumarThis commit adds stingray thermal driver to monitor six thermal zones temperature and trips at critical temperature. Signed-off-by: Pramod Kumar Signed-off-by: Srinath Mannam Reviewed-by: Ray Jui Reviewed-by: Scott Branden Reviewed-by: Vikram Prakash --- drivers/thermal/Kconfig | 3 +- drivers/thermal/broadcom/Kconfig | 9 ++ drivers/thermal/broadcom/Makefile | 1 + drivers/thermal/broadcom/sr-thermal.c | 151 ++ 4 files changed, 163 insertions(+), 1 deletion(-) create mode 100644 drivers/thermal/broadcom/sr-thermal.c diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index 8297988..26d39d4 100644 --- a/drivers/thermal/Kconfig +++ b/drivers/thermal/Kconfig @@ -416,7 +416,8 @@ config MTK_THERMAL controller present in Mediatek SoCs menu "Broadcom thermal drivers" -depends on ARCH_BCM || ARCH_BRCMSTB || ARCH_BCM2835 || COMPILE_TEST +depends on ARCH_BCM || ARCH_BRCMSTB || ARCH_BCM2835 || ARCH_BCM_IPROC || \ + COMPILE_TEST source "drivers/thermal/broadcom/Kconfig" endmenu diff --git a/drivers/thermal/broadcom/Kconfig b/drivers/thermal/broadcom/Kconfig index c106a15..dc9a9bd 100644 --- a/drivers/thermal/broadcom/Kconfig +++ b/drivers/thermal/broadcom/Kconfig @@ -22,3 +22,12 @@ config BCM_NS_THERMAL BCM4708, BCM4709, BCM5301x, BCM95852X, etc). It contains DMU (Device Management Unit) block with a thermal sensor that allows checking CPU temperature. + +config BCM_SR_THERMAL + tristate "Stingray thermal driver" + depends on ARCH_BCM_IPROC || COMPILE_TEST + default ARCH_BCM_IPROC + help + Support for the Stingray family of SoCs. Its different blocks like + iHost, CRMU and NITRO has thermal sensor that allows checking its + temperature. diff --git a/drivers/thermal/broadcom/Makefile b/drivers/thermal/broadcom/Makefile index fae10ec..79df69e 100644 --- a/drivers/thermal/broadcom/Makefile +++ b/drivers/thermal/broadcom/Makefile @@ -1,3 +1,4 @@ obj-$(CONFIG_BCM2835_THERMAL) += bcm2835_thermal.o obj-$(CONFIG_BRCMSTB_THERMAL) += brcmstb_thermal.o obj-$(CONFIG_BCM_NS_THERMAL) += ns-thermal.o +obj-$(CONFIG_BCM_SR_THERMAL) += sr-thermal.o diff --git a/drivers/thermal/broadcom/sr-thermal.c b/drivers/thermal/broadcom/sr-thermal.c new file mode 100644 index 000..5baaa6e --- /dev/null +++ b/drivers/thermal/broadcom/sr-thermal.c @@ -0,0 +1,151 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2018 Broadcom + */ + +#include +#include +#include +#include + +#define TMON_CRIT_TEMP 105000 /* temp in millidegree C */ + +struct sr_thermal { + struct thermal_zone_device *tz; + struct device *dev; + void __iomem *regs; + unsigned int crit_temp; +}; + +static int sr_get_temp(struct thermal_zone_device *tz, int *temp) +{ + struct sr_thermal *sr_thermal = tz->devdata; + + *temp = readl(sr_thermal->regs); + + return 0; +} + +static int sr_get_trip_type(struct thermal_zone_device *tz, int trip, + enum thermal_trip_type *type) +{ + struct sr_thermal *sr_thermal = tz->devdata; + + switch (trip) { + case 0: + *type = THERMAL_TRIP_CRITICAL; + break; + default: + dev_dbg(sr_thermal->dev, + "Driver does not support more than 1 trip point\n"); + return -EINVAL; + } + return 0; +} + +static int sr_get_trip_temp(struct thermal_zone_device *tz, int trip, int *temp) +{ + struct sr_thermal *sr_thermal = tz->devdata; + + switch (trip) { + case 0: + *temp = sr_thermal->crit_temp; + break; + default: + dev_dbg(sr_thermal->dev, + "Driver does not support more than 1 trip point\n"); + return -EINVAL; + } + return 0; +} + +static int sr_set_trip_temp(struct thermal_zone_device *tz, int trip, int temp) +{ + struct sr_thermal *sr_thermal = tz->devdata; + + switch (trip) { + case 0: + /* +* Allow the user to change critical temperature +* as per their requirement, could be for debug +* purpose, even if it's more than the recommended +* critical temperature. +*/ + sr_thermal->crit_temp = temp; + break; + default: + return -EINVAL; + } + return 0; +} + +static struct thermal_zone_device_ops sr_thermal_ops = { + .get_temp = sr_get_temp, + .get_trip_type = sr_get_trip_type, + .get_trip_temp = sr_get_trip_temp, + .set_trip_temp =
[PATCH 2/3] arm64: dts: stingray: Add Stingray Thermal DT support.
From: Pramod KumarAdd DT nodes for thermal zones memory base address to read temperature. Signed-off-by: Pramod Kumar Reviewed-by: Ray Jui Reviewed-by: Scott Branden Reviewed-by: Srinath Mannam --- .../arm64/boot/dts/broadcom/stingray/stingray.dtsi | 37 ++ 1 file changed, 37 insertions(+) diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi index 99aaff0..db1cc67 100644 --- a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi +++ b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi @@ -593,4 +593,41 @@ status = "disabled"; }; }; + + tmons { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x0 0x8f10 0x100>; + + tmon_ihost0: thermal@0 { + compatible = "brcm,sr-thermal"; + reg = <0x0 0x4>; + }; + + tmon_ihost1: thermal@4 { + compatible = "brcm,sr-thermal"; + reg = <0x4 0x4>; + }; + + tmon_ihost2: thermal@8 { + compatible = "brcm,sr-thermal"; + reg = <0x8 0x4>; + }; + + tmon_ihost3: thermal@c { + compatible = "brcm,sr-thermal"; + reg = <0xc 0x4>; + }; + + tmon_crmu: thermal@10 { + compatible = "brcm,sr-thermal"; + reg = <0x10 0x4>; + }; + + tmon_nitro: thermal@14 { + compatible = "brcm,sr-thermal"; + reg = <0x14 0x4>; + }; + }; }; -- 2.7.4
[PATCH 3/3] thermal: broadcom: Add Stingray thermal driver
From: Pramod Kumar This commit adds stingray thermal driver to monitor six thermal zones temperature and trips at critical temperature. Signed-off-by: Pramod Kumar Signed-off-by: Srinath Mannam Reviewed-by: Ray Jui Reviewed-by: Scott Branden Reviewed-by: Vikram Prakash --- drivers/thermal/Kconfig | 3 +- drivers/thermal/broadcom/Kconfig | 9 ++ drivers/thermal/broadcom/Makefile | 1 + drivers/thermal/broadcom/sr-thermal.c | 151 ++ 4 files changed, 163 insertions(+), 1 deletion(-) create mode 100644 drivers/thermal/broadcom/sr-thermal.c diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index 8297988..26d39d4 100644 --- a/drivers/thermal/Kconfig +++ b/drivers/thermal/Kconfig @@ -416,7 +416,8 @@ config MTK_THERMAL controller present in Mediatek SoCs menu "Broadcom thermal drivers" -depends on ARCH_BCM || ARCH_BRCMSTB || ARCH_BCM2835 || COMPILE_TEST +depends on ARCH_BCM || ARCH_BRCMSTB || ARCH_BCM2835 || ARCH_BCM_IPROC || \ + COMPILE_TEST source "drivers/thermal/broadcom/Kconfig" endmenu diff --git a/drivers/thermal/broadcom/Kconfig b/drivers/thermal/broadcom/Kconfig index c106a15..dc9a9bd 100644 --- a/drivers/thermal/broadcom/Kconfig +++ b/drivers/thermal/broadcom/Kconfig @@ -22,3 +22,12 @@ config BCM_NS_THERMAL BCM4708, BCM4709, BCM5301x, BCM95852X, etc). It contains DMU (Device Management Unit) block with a thermal sensor that allows checking CPU temperature. + +config BCM_SR_THERMAL + tristate "Stingray thermal driver" + depends on ARCH_BCM_IPROC || COMPILE_TEST + default ARCH_BCM_IPROC + help + Support for the Stingray family of SoCs. Its different blocks like + iHost, CRMU and NITRO has thermal sensor that allows checking its + temperature. diff --git a/drivers/thermal/broadcom/Makefile b/drivers/thermal/broadcom/Makefile index fae10ec..79df69e 100644 --- a/drivers/thermal/broadcom/Makefile +++ b/drivers/thermal/broadcom/Makefile @@ -1,3 +1,4 @@ obj-$(CONFIG_BCM2835_THERMAL) += bcm2835_thermal.o obj-$(CONFIG_BRCMSTB_THERMAL) += brcmstb_thermal.o obj-$(CONFIG_BCM_NS_THERMAL) += ns-thermal.o +obj-$(CONFIG_BCM_SR_THERMAL) += sr-thermal.o diff --git a/drivers/thermal/broadcom/sr-thermal.c b/drivers/thermal/broadcom/sr-thermal.c new file mode 100644 index 000..5baaa6e --- /dev/null +++ b/drivers/thermal/broadcom/sr-thermal.c @@ -0,0 +1,151 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2018 Broadcom + */ + +#include +#include +#include +#include + +#define TMON_CRIT_TEMP 105000 /* temp in millidegree C */ + +struct sr_thermal { + struct thermal_zone_device *tz; + struct device *dev; + void __iomem *regs; + unsigned int crit_temp; +}; + +static int sr_get_temp(struct thermal_zone_device *tz, int *temp) +{ + struct sr_thermal *sr_thermal = tz->devdata; + + *temp = readl(sr_thermal->regs); + + return 0; +} + +static int sr_get_trip_type(struct thermal_zone_device *tz, int trip, + enum thermal_trip_type *type) +{ + struct sr_thermal *sr_thermal = tz->devdata; + + switch (trip) { + case 0: + *type = THERMAL_TRIP_CRITICAL; + break; + default: + dev_dbg(sr_thermal->dev, + "Driver does not support more than 1 trip point\n"); + return -EINVAL; + } + return 0; +} + +static int sr_get_trip_temp(struct thermal_zone_device *tz, int trip, int *temp) +{ + struct sr_thermal *sr_thermal = tz->devdata; + + switch (trip) { + case 0: + *temp = sr_thermal->crit_temp; + break; + default: + dev_dbg(sr_thermal->dev, + "Driver does not support more than 1 trip point\n"); + return -EINVAL; + } + return 0; +} + +static int sr_set_trip_temp(struct thermal_zone_device *tz, int trip, int temp) +{ + struct sr_thermal *sr_thermal = tz->devdata; + + switch (trip) { + case 0: + /* +* Allow the user to change critical temperature +* as per their requirement, could be for debug +* purpose, even if it's more than the recommended +* critical temperature. +*/ + sr_thermal->crit_temp = temp; + break; + default: + return -EINVAL; + } + return 0; +} + +static struct thermal_zone_device_ops sr_thermal_ops = { + .get_temp = sr_get_temp, + .get_trip_type = sr_get_trip_type, + .get_trip_temp = sr_get_trip_temp, + .set_trip_temp = sr_set_trip_temp, +}; + +static int sr_thermal_probe(struct platform_device *pdev) +{ + struct device *dev = >dev; + struct sr_thermal *sr_thermal; + struct
[PATCH 2/3] arm64: dts: stingray: Add Stingray Thermal DT support.
From: Pramod Kumar Add DT nodes for thermal zones memory base address to read temperature. Signed-off-by: Pramod Kumar Reviewed-by: Ray Jui Reviewed-by: Scott Branden Reviewed-by: Srinath Mannam --- .../arm64/boot/dts/broadcom/stingray/stingray.dtsi | 37 ++ 1 file changed, 37 insertions(+) diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi index 99aaff0..db1cc67 100644 --- a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi +++ b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi @@ -593,4 +593,41 @@ status = "disabled"; }; }; + + tmons { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x0 0x8f10 0x100>; + + tmon_ihost0: thermal@0 { + compatible = "brcm,sr-thermal"; + reg = <0x0 0x4>; + }; + + tmon_ihost1: thermal@4 { + compatible = "brcm,sr-thermal"; + reg = <0x4 0x4>; + }; + + tmon_ihost2: thermal@8 { + compatible = "brcm,sr-thermal"; + reg = <0x8 0x4>; + }; + + tmon_ihost3: thermal@c { + compatible = "brcm,sr-thermal"; + reg = <0xc 0x4>; + }; + + tmon_crmu: thermal@10 { + compatible = "brcm,sr-thermal"; + reg = <0x10 0x4>; + }; + + tmon_nitro: thermal@14 { + compatible = "brcm,sr-thermal"; + reg = <0x14 0x4>; + }; + }; }; -- 2.7.4
[PATCH 1/3] dt-bindings: thermal: Add binding document for SR thermal
From: Pramod KumarAdd binding document for supported thermal implementation in Stingray. Signed-off-by: Pramod Kumar Reviewed-by: Ray Jui Reviewed-by: Scott Branden Reviewed-by: Srinath Mannam --- .../bindings/thermal/brcm,sr-thermal.txt | 45 ++ 1 file changed, 45 insertions(+) create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt diff --git a/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt new file mode 100644 index 000..33f9e11 --- /dev/null +++ b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt @@ -0,0 +1,45 @@ +* Broadcom Stingray Thermal + +This binding describes thermal sensors that is part of Stingray SoCs. + +Required properties: +- compatible : Must be "brcm,sr-thermal" +- reg : memory where tmon data will be available. + +Example: + tmons { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + tmon_ihost0: thermal@8f10 { + compatible = "brcm,sr-thermal"; + reg = <0x8f10 0x4>; + }; + + tmon_ihost1: thermal@8f14 { + compatible = "brcm,sr-thermal"; + reg = <0x8f14 0x4>; + }; + + tmon_ihost2: thermal@8f18 { + compatible = "brcm,sr-thermal"; + reg = <0x8f18 0x4>; + }; + + tmon_ihost3: thermal@8f1c { + compatible = "brcm,sr-thermal"; + reg = <0x8f1c 0x4>; + }; + + tmon_crmu: thermal@8f100010 { + compatible = "brcm,sr-thermal"; + reg = <0x8f100010 0x4>; + }; + + tmon_nitro: thermal@8f100014 { + compatible = "brcm,sr-thermal"; + reg = <0x8f100014 0x4>; + }; + }; -- 2.7.4
[PATCH 1/3] dt-bindings: thermal: Add binding document for SR thermal
From: Pramod Kumar Add binding document for supported thermal implementation in Stingray. Signed-off-by: Pramod Kumar Reviewed-by: Ray Jui Reviewed-by: Scott Branden Reviewed-by: Srinath Mannam --- .../bindings/thermal/brcm,sr-thermal.txt | 45 ++ 1 file changed, 45 insertions(+) create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt diff --git a/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt new file mode 100644 index 000..33f9e11 --- /dev/null +++ b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt @@ -0,0 +1,45 @@ +* Broadcom Stingray Thermal + +This binding describes thermal sensors that is part of Stingray SoCs. + +Required properties: +- compatible : Must be "brcm,sr-thermal" +- reg : memory where tmon data will be available. + +Example: + tmons { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + tmon_ihost0: thermal@8f10 { + compatible = "brcm,sr-thermal"; + reg = <0x8f10 0x4>; + }; + + tmon_ihost1: thermal@8f14 { + compatible = "brcm,sr-thermal"; + reg = <0x8f14 0x4>; + }; + + tmon_ihost2: thermal@8f18 { + compatible = "brcm,sr-thermal"; + reg = <0x8f18 0x4>; + }; + + tmon_ihost3: thermal@8f1c { + compatible = "brcm,sr-thermal"; + reg = <0x8f1c 0x4>; + }; + + tmon_crmu: thermal@8f100010 { + compatible = "brcm,sr-thermal"; + reg = <0x8f100010 0x4>; + }; + + tmon_nitro: thermal@8f100014 { + compatible = "brcm,sr-thermal"; + reg = <0x8f100014 0x4>; + }; + }; -- 2.7.4
[PATCH 0/3] Stingray thermal driver support
These patches adds the stingray thermal driver and its corresponding DT nodes with documentation. Pramod Kumar (3): dt-bindings: thermal: Add binding document for SR thermal arm64: dts: stingray: Add Stingray Thermal DT support. thermal: broadcom: Add Stingray thermal driver .../bindings/thermal/brcm,sr-thermal.txt | 45 ++ .../arm64/boot/dts/broadcom/stingray/stingray.dtsi | 37 + drivers/thermal/Kconfig| 3 +- drivers/thermal/broadcom/Kconfig | 9 ++ drivers/thermal/broadcom/Makefile | 1 + drivers/thermal/broadcom/sr-thermal.c | 151 + 6 files changed, 245 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt create mode 100644 drivers/thermal/broadcom/sr-thermal.c -- 2.7.4
[PATCH 0/3] Stingray thermal driver support
These patches adds the stingray thermal driver and its corresponding DT nodes with documentation. Pramod Kumar (3): dt-bindings: thermal: Add binding document for SR thermal arm64: dts: stingray: Add Stingray Thermal DT support. thermal: broadcom: Add Stingray thermal driver .../bindings/thermal/brcm,sr-thermal.txt | 45 ++ .../arm64/boot/dts/broadcom/stingray/stingray.dtsi | 37 + drivers/thermal/Kconfig| 3 +- drivers/thermal/broadcom/Kconfig | 9 ++ drivers/thermal/broadcom/Makefile | 1 + drivers/thermal/broadcom/sr-thermal.c | 151 + 6 files changed, 245 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt create mode 100644 drivers/thermal/broadcom/sr-thermal.c -- 2.7.4
Re: [PATCH 04/11] PM / devfreq: Remove redundant frequency adjustment from governors
Hi, On 2018년 05월 26일 05:30, Matthias Kaehlcke wrote: > The userspace and simpleondemand governor determine a target frequency and > then adjust it according to the df->min/max_freq limits that might have > been set by user space. This adjustment is redundant, it is done in > update_devfreq() for any governor, right after returning from > governor->get_target_freq(). > > Signed-off-by: Matthias Kaehlcke> --- > drivers/devfreq/governor_simpleondemand.c | 5 - > drivers/devfreq/governor_userspace.c | 16 > 2 files changed, 4 insertions(+), 17 deletions(-) > > diff --git a/drivers/devfreq/governor_simpleondemand.c > b/drivers/devfreq/governor_simpleondemand.c > index 278964783fa6..3da7554b4837 100644 > --- a/drivers/devfreq/governor_simpleondemand.c > +++ b/drivers/devfreq/governor_simpleondemand.c > @@ -84,11 +84,6 @@ static int devfreq_simple_ondemand_func(struct devfreq *df, > b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2)); > *freq = (unsigned long) b; > > - if (df->min_freq && *freq < df->min_freq) > - *freq = df->min_freq; > - if (df->max_freq && *freq > df->max_freq) > - *freq = df->max_freq; > - > return 0; > } > > diff --git a/drivers/devfreq/governor_userspace.c > b/drivers/devfreq/governor_userspace.c > index 080607c3f34d..378d84c011df 100644 > --- a/drivers/devfreq/governor_userspace.c > +++ b/drivers/devfreq/governor_userspace.c > @@ -26,19 +26,11 @@ static int devfreq_userspace_func(struct devfreq *df, > unsigned long *freq) > { > struct userspace_data *data = df->data; > > - if (data->valid) { > - unsigned long adjusted_freq = data->user_frequency; > - > - if (df->max_freq && adjusted_freq > df->max_freq) > - adjusted_freq = df->max_freq; > - > - if (df->min_freq && adjusted_freq < df->min_freq) > - adjusted_freq = df->min_freq; > - > - *freq = adjusted_freq; > - } else { > + if (data->valid) > + *freq = data->user_frequency; > + else > *freq = df->previous_freq; /* No user freq specified yet */ > - } > + > return 0; > } > > Reviewed-by: Chanwoo Choi -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH 04/11] PM / devfreq: Remove redundant frequency adjustment from governors
Hi, On 2018년 05월 26일 05:30, Matthias Kaehlcke wrote: > The userspace and simpleondemand governor determine a target frequency and > then adjust it according to the df->min/max_freq limits that might have > been set by user space. This adjustment is redundant, it is done in > update_devfreq() for any governor, right after returning from > governor->get_target_freq(). > > Signed-off-by: Matthias Kaehlcke > --- > drivers/devfreq/governor_simpleondemand.c | 5 - > drivers/devfreq/governor_userspace.c | 16 > 2 files changed, 4 insertions(+), 17 deletions(-) > > diff --git a/drivers/devfreq/governor_simpleondemand.c > b/drivers/devfreq/governor_simpleondemand.c > index 278964783fa6..3da7554b4837 100644 > --- a/drivers/devfreq/governor_simpleondemand.c > +++ b/drivers/devfreq/governor_simpleondemand.c > @@ -84,11 +84,6 @@ static int devfreq_simple_ondemand_func(struct devfreq *df, > b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2)); > *freq = (unsigned long) b; > > - if (df->min_freq && *freq < df->min_freq) > - *freq = df->min_freq; > - if (df->max_freq && *freq > df->max_freq) > - *freq = df->max_freq; > - > return 0; > } > > diff --git a/drivers/devfreq/governor_userspace.c > b/drivers/devfreq/governor_userspace.c > index 080607c3f34d..378d84c011df 100644 > --- a/drivers/devfreq/governor_userspace.c > +++ b/drivers/devfreq/governor_userspace.c > @@ -26,19 +26,11 @@ static int devfreq_userspace_func(struct devfreq *df, > unsigned long *freq) > { > struct userspace_data *data = df->data; > > - if (data->valid) { > - unsigned long adjusted_freq = data->user_frequency; > - > - if (df->max_freq && adjusted_freq > df->max_freq) > - adjusted_freq = df->max_freq; > - > - if (df->min_freq && adjusted_freq < df->min_freq) > - adjusted_freq = df->min_freq; > - > - *freq = adjusted_freq; > - } else { > + if (data->valid) > + *freq = data->user_frequency; > + else > *freq = df->previous_freq; /* No user freq specified yet */ > - } > + > return 0; > } > > Reviewed-by: Chanwoo Choi -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH v9 01/10] drivers: qcom: rpmh-rsc: add RPMH controller for QCOM SoCs
Hi, On 5/24/2018 4:15 PM, Raju P L S S S N wrote: From: Lina IyerAdd controller driver for QCOM SoCs that have hardware based shared resource management. The hardware IP known as RSC (Resource State Coordinator) houses multiple Direct Resource Voter (DRV) for different execution levels. A DRV is a unique voter on the state of a shared resource. A Trigger Control Set (TCS) is a bunch of slots that can house multiple resource state requests, that when triggered will issue those requests through an internal bus to the Resource Power Manager Hardened (RPMH) blocks. These hardware blocks are capable of adjusting clocks, voltages, etc. The resource state request from a DRV are aggregated along with state requests from other processors in the SoC and the aggregate value is applied on the resource. Some important aspects of the RPMH communication - - Requests are with some header information - Multiple requests (upto 16) may be sent through a TCS, at a time - Requests in a TCS are sent in sequence - Requests may be fire-n-forget or completion (response expected) - Multiple TCS from the same DRV may be triggered simultaneously - Cannot send a request if another request for the same addr is in progress from the same DRV - When all the requests from a TCS are complete, an IRQ is raised - The IRQ handler needs to clear the TCS before it is available for reuse - TCS configuration is specific to a DRV - Platform drivers may use DRV from different RSCs to make requests Resource state requests made when CPUs are active are called 'active' state requests. Requests made when all the CPUs are powered down (idle state) are called 'sleep' state requests. They are matched by a corresponding 'wake' state requests which puts the resources back in to previously requested active state before resuming any CPU. TCSes are dedicated for each type of requests. Active mode TCSes (AMC) are used to send requests immediately to the resource, while control TCS are used to provide specific information to the controller. Sleep and Wake TCS send sleep and wake requests, after and before the system halt respectively. Signed-off-by: Lina Iyer Signed-off-by: Raju P.L.S.S.S.N Reviewed-by: Douglas Anderson I have added Reviewed-by: Douglas Anderson tag by mistake while sending v9 patches. Didn't receive the tag. Will remove it in next spin. Thanks, Raju --- Changes in v9: - Remove EXPORT_SYMBOL Changes in v8: - introduce interrupt description in DT for all DRvs Changes in v7: - rename 'm' and 'n' variable names Changes in v6: - Remove tasklet and response object - introduce rpm_write_tcs_cmd() function - rename tcs_irq_handler() - avoid bool in structures. Fix tcs.h. Changes in v4: - lots of variable name changes as suggested by Stephen B - use of const for data pointers - fix comments and other code syntax - use of bitmap for tcs_in_use instead of atomic --- drivers/soc/qcom/Kconfig| 10 + drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/rpmh-internal.h| 69 + drivers/soc/qcom/rpmh-rsc.c | 482 include/dt-bindings/soc/qcom,rpmh-rsc.h | 14 + include/soc/qcom/tcs.h | 56 6 files changed, 632 insertions(+) create mode 100644 drivers/soc/qcom/rpmh-internal.h create mode 100644 drivers/soc/qcom/rpmh-rsc.c create mode 100644 include/dt-bindings/soc/qcom,rpmh-rsc.h create mode 100644 include/soc/qcom/tcs.h diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig index 5c4535b..1887e1b 100644 --- a/drivers/soc/qcom/Kconfig +++ b/drivers/soc/qcom/Kconfig @@ -56,6 +56,16 @@ config QCOM_RMTFS_MEM Say y here if you intend to boot the modem remoteproc. +config QCOM_RPMH + bool "Qualcomm RPM-Hardened (RPMH) Communication" + depends on ARCH_QCOM && ARM64 && OF || COMPILE_TEST + help + Support for communication with the hardened-RPM blocks in + Qualcomm Technologies Inc (QTI) SoCs. RPMH communication uses an + internal bus to transmit state requests for shared resources. A set + of hardware components aggregate requests for these resources and + help apply the aggregated state on the resource. + config QCOM_SMEM tristate "Qualcomm Shared Memory Manager (SMEM)" depends on ARCH_QCOM diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile index dcebf28..39d3a05 100644 --- a/drivers/soc/qcom/Makefile +++ b/drivers/soc/qcom/Makefile @@ -6,6 +6,7 @@ obj-$(CONFIG_QCOM_PM) += spm.o obj-$(CONFIG_QCOM_QMI_HELPERS)+= qmi_helpers.o qmi_helpers-y += qmi_encdec.o qmi_interface.o obj-$(CONFIG_QCOM_RMTFS_MEM) += rmtfs_mem.o +obj-$(CONFIG_QCOM_RPMH)+= rpmh-rsc.o
Re: [PATCH v9 01/10] drivers: qcom: rpmh-rsc: add RPMH controller for QCOM SoCs
Hi, On 5/24/2018 4:15 PM, Raju P L S S S N wrote: From: Lina Iyer Add controller driver for QCOM SoCs that have hardware based shared resource management. The hardware IP known as RSC (Resource State Coordinator) houses multiple Direct Resource Voter (DRV) for different execution levels. A DRV is a unique voter on the state of a shared resource. A Trigger Control Set (TCS) is a bunch of slots that can house multiple resource state requests, that when triggered will issue those requests through an internal bus to the Resource Power Manager Hardened (RPMH) blocks. These hardware blocks are capable of adjusting clocks, voltages, etc. The resource state request from a DRV are aggregated along with state requests from other processors in the SoC and the aggregate value is applied on the resource. Some important aspects of the RPMH communication - - Requests are with some header information - Multiple requests (upto 16) may be sent through a TCS, at a time - Requests in a TCS are sent in sequence - Requests may be fire-n-forget or completion (response expected) - Multiple TCS from the same DRV may be triggered simultaneously - Cannot send a request if another request for the same addr is in progress from the same DRV - When all the requests from a TCS are complete, an IRQ is raised - The IRQ handler needs to clear the TCS before it is available for reuse - TCS configuration is specific to a DRV - Platform drivers may use DRV from different RSCs to make requests Resource state requests made when CPUs are active are called 'active' state requests. Requests made when all the CPUs are powered down (idle state) are called 'sleep' state requests. They are matched by a corresponding 'wake' state requests which puts the resources back in to previously requested active state before resuming any CPU. TCSes are dedicated for each type of requests. Active mode TCSes (AMC) are used to send requests immediately to the resource, while control TCS are used to provide specific information to the controller. Sleep and Wake TCS send sleep and wake requests, after and before the system halt respectively. Signed-off-by: Lina Iyer Signed-off-by: Raju P.L.S.S.S.N Reviewed-by: Douglas Anderson I have added Reviewed-by: Douglas Anderson tag by mistake while sending v9 patches. Didn't receive the tag. Will remove it in next spin. Thanks, Raju --- Changes in v9: - Remove EXPORT_SYMBOL Changes in v8: - introduce interrupt description in DT for all DRvs Changes in v7: - rename 'm' and 'n' variable names Changes in v6: - Remove tasklet and response object - introduce rpm_write_tcs_cmd() function - rename tcs_irq_handler() - avoid bool in structures. Fix tcs.h. Changes in v4: - lots of variable name changes as suggested by Stephen B - use of const for data pointers - fix comments and other code syntax - use of bitmap for tcs_in_use instead of atomic --- drivers/soc/qcom/Kconfig| 10 + drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/rpmh-internal.h| 69 + drivers/soc/qcom/rpmh-rsc.c | 482 include/dt-bindings/soc/qcom,rpmh-rsc.h | 14 + include/soc/qcom/tcs.h | 56 6 files changed, 632 insertions(+) create mode 100644 drivers/soc/qcom/rpmh-internal.h create mode 100644 drivers/soc/qcom/rpmh-rsc.c create mode 100644 include/dt-bindings/soc/qcom,rpmh-rsc.h create mode 100644 include/soc/qcom/tcs.h diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig index 5c4535b..1887e1b 100644 --- a/drivers/soc/qcom/Kconfig +++ b/drivers/soc/qcom/Kconfig @@ -56,6 +56,16 @@ config QCOM_RMTFS_MEM Say y here if you intend to boot the modem remoteproc. +config QCOM_RPMH + bool "Qualcomm RPM-Hardened (RPMH) Communication" + depends on ARCH_QCOM && ARM64 && OF || COMPILE_TEST + help + Support for communication with the hardened-RPM blocks in + Qualcomm Technologies Inc (QTI) SoCs. RPMH communication uses an + internal bus to transmit state requests for shared resources. A set + of hardware components aggregate requests for these resources and + help apply the aggregated state on the resource. + config QCOM_SMEM tristate "Qualcomm Shared Memory Manager (SMEM)" depends on ARCH_QCOM diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile index dcebf28..39d3a05 100644 --- a/drivers/soc/qcom/Makefile +++ b/drivers/soc/qcom/Makefile @@ -6,6 +6,7 @@ obj-$(CONFIG_QCOM_PM) += spm.o obj-$(CONFIG_QCOM_QMI_HELPERS)+= qmi_helpers.o qmi_helpers-y += qmi_encdec.o qmi_interface.o obj-$(CONFIG_QCOM_RMTFS_MEM) += rmtfs_mem.o +obj-$(CONFIG_QCOM_RPMH)+= rpmh-rsc.o obj-$(CONFIG_QCOM_SMD_RPM)+= smd-rpm.o obj-$(CONFIG_QCOM_SMEM) +=smem.o obj-$(CONFIG_QCOM_SMEM_STATE) +=
Re: B53 DSA switch problem on Banana Pi-R1 on Fedora 26 - systemd-networkd problem
On 27.05.2018 22:31, Florian Fainelli wrote: Le 05/27/18 à 12:01, Gerhard Wiesinger a écrit : On 24.05.2018 08:22, Gerhard Wiesinger wrote: On 24.05.2018 07:29, Gerhard Wiesinger wrote: After some analysis with Florian (thnx) we found out that the current implementation is broken: https://patchwork.ozlabs.org/patch/836538/ https://github.com/torvalds/linux/commit/c499696e7901bda18385ac723b7bd27c3a4af624#diff-a2b6f8d89e18de600e873ac3ac43fa1d Florians comment: c499696e7901bda18385ac723b7bd27c3a4af624 ("net: dsa: b53: Stop using dev->cpu_port incorrectly") since it would result in no longer setting the CPU port as tagged for a specific VLAN. Easiest way for you right now is to just revert it, but this needs some more thoughts for a proper upstream change. I will think about it some more. Can confirm 4.14.18-200.fc26.armv7hl works, 4.15.x should be broken. # Kernel 4.14.x ok https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/drivers/net/dsa/b53?h=v4.14.43 # Kernel 4.15.x should be NOT ok https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/drivers/net/dsa/b53?h=v4.15.18 Kernel 4.14.18-300.fc27.armv7hl works well so far, even with FC28 update. Florian send me a patch to try for 4.16.x So does my patch make 4.16 work correctly for you now? If so, can I just submit it and copy you? I got the commands below to work with manual script commands. Afterwards I wrote systemd-networkd config where I've a strage problem when IPv6 sends a multicast broadcast from another machine to the bridge this will be sent back via the network interface, but with the source MAC of the bridge of the other machine. dmesg from the other machine: [117768.330444] br0: received packet on lan0 with own address as source address (addr:a0:36:9f:ab:cd:ef, vlan:0) [117768.334887] br0: received packet on lan0 with own address as source address (addr:a0:36:9f:ab:cd:ef, vlan:0) [117768.339281] br0: received packet on lan0 with own address as source address (addr:a0:36:9f:ab:cd:ef, vlan:0) And: If I just enter this command after e.g. a systemd-network restart everything is fine forever: # Not OK (dmesg message above is triggered on a remote computer, whole switching network gets unstable, ssh terminals close, packet loss, etc.) systemctl restart systemd-networkd # OK again when this command is entered bridge vlan add dev wan vid 102 pvid untagged brctl show, ip link, bridge vlan, bridge link commands, etc. look all the same, also /sys/class/net/br0/bridge, /sys/class/net/br1/bridge settings Systemd config correct? Any ideas? You should not have eth0.101 and eth0.102 to be enslaved in a bridge at all, this is what is causing the bridge to be confused. Remember what I wrote to you before, with the current b53 driver that does not have any tagging enabled the lanX interfaces and brX interfaces are only used for control and should not be used for passing any data. The only network device that will be passing data is eth0, which is why we need to set-up VLAN interfaces to pop/push the VLAN id accordingly. I have no idea why manual vs. systemd does not work but you can most certainly troubleshoot that by comparing the bridge/ip outputs. So is that then the correct structure? br1 - lan1 (with VID 101) - lan2 (with VID 101) - lan3 (with VID 101) - lan4 (with VID 101) brlan - eth0.101 - wlan0 (currently not active, could be optimized without bridge but for future comfort) br2 - wan (with VID 102) (could be optimized without bridge but for future comfort) - future1 brwan - eth0.102 (could be optimized without bridge but for future comfort) - future2 Ad systemd vs. manual config: As I said I didn't find any difference in the bridge/ip outputs. As they are broken (see other message) maybe something else is broken, too. Thnx. Ciao, Gerhard
Re: B53 DSA switch problem on Banana Pi-R1 on Fedora 26 - systemd-networkd problem
On 27.05.2018 22:31, Florian Fainelli wrote: Le 05/27/18 à 12:01, Gerhard Wiesinger a écrit : On 24.05.2018 08:22, Gerhard Wiesinger wrote: On 24.05.2018 07:29, Gerhard Wiesinger wrote: After some analysis with Florian (thnx) we found out that the current implementation is broken: https://patchwork.ozlabs.org/patch/836538/ https://github.com/torvalds/linux/commit/c499696e7901bda18385ac723b7bd27c3a4af624#diff-a2b6f8d89e18de600e873ac3ac43fa1d Florians comment: c499696e7901bda18385ac723b7bd27c3a4af624 ("net: dsa: b53: Stop using dev->cpu_port incorrectly") since it would result in no longer setting the CPU port as tagged for a specific VLAN. Easiest way for you right now is to just revert it, but this needs some more thoughts for a proper upstream change. I will think about it some more. Can confirm 4.14.18-200.fc26.armv7hl works, 4.15.x should be broken. # Kernel 4.14.x ok https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/drivers/net/dsa/b53?h=v4.14.43 # Kernel 4.15.x should be NOT ok https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/drivers/net/dsa/b53?h=v4.15.18 Kernel 4.14.18-300.fc27.armv7hl works well so far, even with FC28 update. Florian send me a patch to try for 4.16.x So does my patch make 4.16 work correctly for you now? If so, can I just submit it and copy you? I got the commands below to work with manual script commands. Afterwards I wrote systemd-networkd config where I've a strage problem when IPv6 sends a multicast broadcast from another machine to the bridge this will be sent back via the network interface, but with the source MAC of the bridge of the other machine. dmesg from the other machine: [117768.330444] br0: received packet on lan0 with own address as source address (addr:a0:36:9f:ab:cd:ef, vlan:0) [117768.334887] br0: received packet on lan0 with own address as source address (addr:a0:36:9f:ab:cd:ef, vlan:0) [117768.339281] br0: received packet on lan0 with own address as source address (addr:a0:36:9f:ab:cd:ef, vlan:0) And: If I just enter this command after e.g. a systemd-network restart everything is fine forever: # Not OK (dmesg message above is triggered on a remote computer, whole switching network gets unstable, ssh terminals close, packet loss, etc.) systemctl restart systemd-networkd # OK again when this command is entered bridge vlan add dev wan vid 102 pvid untagged brctl show, ip link, bridge vlan, bridge link commands, etc. look all the same, also /sys/class/net/br0/bridge, /sys/class/net/br1/bridge settings Systemd config correct? Any ideas? You should not have eth0.101 and eth0.102 to be enslaved in a bridge at all, this is what is causing the bridge to be confused. Remember what I wrote to you before, with the current b53 driver that does not have any tagging enabled the lanX interfaces and brX interfaces are only used for control and should not be used for passing any data. The only network device that will be passing data is eth0, which is why we need to set-up VLAN interfaces to pop/push the VLAN id accordingly. I have no idea why manual vs. systemd does not work but you can most certainly troubleshoot that by comparing the bridge/ip outputs. So is that then the correct structure? br1 - lan1 (with VID 101) - lan2 (with VID 101) - lan3 (with VID 101) - lan4 (with VID 101) brlan - eth0.101 - wlan0 (currently not active, could be optimized without bridge but for future comfort) br2 - wan (with VID 102) (could be optimized without bridge but for future comfort) - future1 brwan - eth0.102 (could be optimized without bridge but for future comfort) - future2 Ad systemd vs. manual config: As I said I didn't find any difference in the bridge/ip outputs. As they are broken (see other message) maybe something else is broken, too. Thnx. Ciao, Gerhard
Can I use 'signed -off-by' to define maintainers' workload?
Hi all, I am a student from Peking University. I'm not sure if it's appropriate to ask questions here. I have already tried other mailing lists, but I got no reply. I am very sorry to bother all of you. I am doing a research about the maintainers' workload in the Linux kernel community. We all know that the commits submitted by the developer will be reviewed layer by layer and eventually merged into the main repository. Most commits have one or several signed-off-by tags. The documentation from community about signed-off-by is described as follows: The sign-off is a simple line at the end of the explanation for the patch, which certifies that you wrote it or otherwise have the right to pass it on as an open-source patch. The documentation is very clear, which means that only two types of people can sign their name: 1. The author 2. The related maintainer. I want to define the maintainer's workload by this tag. There are several questions that I would like to consult you: 1) Do all the maintainers in the path from the author of the commits to the mainline repository sign their name? 2) If the answer is yes, do the workload of subsystem maintainer and the upper maintainer are the same in the code review? In other words, whether is it possible that the first maintainer who merge the commits submitted by the developer to its own repository spend a lot effort on review? The upper maintainer is based on the trust of the lower layer maintainer, and simply merges it into his/her own repository as long as there is no compiling problem and also sign their name. 3) If the answer is no, why do some maintainers sign their names, and some do not? Is it because these maintainers trust the lower layer very much and feel that it is not necessary to review it? 4) Is there any special situation that leads to signing-off-by not identifying all the maintainers in the path of the commits develiry? For example, the upper maintainer does not trust the lower layer maintainers, and he/she will contribute a new commits by himself instead of passing by, so it will not record the maintainer of the lower layer. Or because this commit contains modification to several files and each file has a specific maintainer, only one maintainer merged it to his repository and signed his name. In short, I would like to know how signed-off-by is used in the actual process. I would be grateful if you could reply to me. Thank you again! Best regards, Xin Tan
Can I use 'signed -off-by' to define maintainers' workload?
Hi all, I am a student from Peking University. I'm not sure if it's appropriate to ask questions here. I have already tried other mailing lists, but I got no reply. I am very sorry to bother all of you. I am doing a research about the maintainers' workload in the Linux kernel community. We all know that the commits submitted by the developer will be reviewed layer by layer and eventually merged into the main repository. Most commits have one or several signed-off-by tags. The documentation from community about signed-off-by is described as follows: The sign-off is a simple line at the end of the explanation for the patch, which certifies that you wrote it or otherwise have the right to pass it on as an open-source patch. The documentation is very clear, which means that only two types of people can sign their name: 1. The author 2. The related maintainer. I want to define the maintainer's workload by this tag. There are several questions that I would like to consult you: 1) Do all the maintainers in the path from the author of the commits to the mainline repository sign their name? 2) If the answer is yes, do the workload of subsystem maintainer and the upper maintainer are the same in the code review? In other words, whether is it possible that the first maintainer who merge the commits submitted by the developer to its own repository spend a lot effort on review? The upper maintainer is based on the trust of the lower layer maintainer, and simply merges it into his/her own repository as long as there is no compiling problem and also sign their name. 3) If the answer is no, why do some maintainers sign their names, and some do not? Is it because these maintainers trust the lower layer very much and feel that it is not necessary to review it? 4) Is there any special situation that leads to signing-off-by not identifying all the maintainers in the path of the commits develiry? For example, the upper maintainer does not trust the lower layer maintainers, and he/she will contribute a new commits by himself instead of passing by, so it will not record the maintainer of the lower layer. Or because this commit contains modification to several files and each file has a specific maintainer, only one maintainer merged it to his repository and signed his name. In short, I would like to know how signed-off-by is used in the actual process. I would be grateful if you could reply to me. Thank you again! Best regards, Xin Tan
Re: [PATCH 03/11] PM / devfreq: Remove check for df->max_freq == 0 from governors
Hi, On 2018년 05월 26일 05:30, Matthias Kaehlcke wrote: > Commit "PM / devfreq: Fix handling of min/max_freq == 0" ensures that > df->max_freq is not 0, remove unnecessary checks. > > Signed-off-by: Matthias Kaehlcke> --- > drivers/devfreq/governor_performance.c| 5 + > drivers/devfreq/governor_simpleondemand.c | 7 +++ > 2 files changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/devfreq/governor_performance.c > b/drivers/devfreq/governor_performance.c > index 4d23ecfbd948..1c990cb45098 100644 > --- a/drivers/devfreq/governor_performance.c > +++ b/drivers/devfreq/governor_performance.c > @@ -20,10 +20,7 @@ static int devfreq_performance_func(struct devfreq *df, >* target callback should be able to get floor value as >* said in devfreq.h >*/ > - if (!df->max_freq) > - *freq = UINT_MAX; > - else > - *freq = df->max_freq; > + *freq = df->max_freq; > return 0; > } > > diff --git a/drivers/devfreq/governor_simpleondemand.c > b/drivers/devfreq/governor_simpleondemand.c > index 28e0f2de7100..278964783fa6 100644 > --- a/drivers/devfreq/governor_simpleondemand.c > +++ b/drivers/devfreq/governor_simpleondemand.c > @@ -27,7 +27,6 @@ static int devfreq_simple_ondemand_func(struct devfreq *df, > unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD; > unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL; > struct devfreq_simple_ondemand_data *data = df->data; > - unsigned long max = (df->max_freq) ? df->max_freq : UINT_MAX; > > err = devfreq_update_stats(df); > if (err) > @@ -47,7 +46,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df, > > /* Assume MAX if it is going to be divided by zero */ > if (stat->total_time == 0) { > - *freq = max; > + *freq = df->max_freq; > return 0; > } > > @@ -60,13 +59,13 @@ static int devfreq_simple_ondemand_func(struct devfreq > *df, > /* Set MAX if it's busy enough */ > if (stat->busy_time * 100 > > stat->total_time * dfso_upthreshold) { > - *freq = max; > + *freq = df->max_freq; > return 0; > } > > /* Set MAX if we do not know the initial frequency */ > if (stat->current_frequency == 0) { > - *freq = max; > + *freq = df->max_freq; > return 0; > } > > Reviewed-by: Chanwoo Choi -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH 03/11] PM / devfreq: Remove check for df->max_freq == 0 from governors
Hi, On 2018년 05월 26일 05:30, Matthias Kaehlcke wrote: > Commit "PM / devfreq: Fix handling of min/max_freq == 0" ensures that > df->max_freq is not 0, remove unnecessary checks. > > Signed-off-by: Matthias Kaehlcke > --- > drivers/devfreq/governor_performance.c| 5 + > drivers/devfreq/governor_simpleondemand.c | 7 +++ > 2 files changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/devfreq/governor_performance.c > b/drivers/devfreq/governor_performance.c > index 4d23ecfbd948..1c990cb45098 100644 > --- a/drivers/devfreq/governor_performance.c > +++ b/drivers/devfreq/governor_performance.c > @@ -20,10 +20,7 @@ static int devfreq_performance_func(struct devfreq *df, >* target callback should be able to get floor value as >* said in devfreq.h >*/ > - if (!df->max_freq) > - *freq = UINT_MAX; > - else > - *freq = df->max_freq; > + *freq = df->max_freq; > return 0; > } > > diff --git a/drivers/devfreq/governor_simpleondemand.c > b/drivers/devfreq/governor_simpleondemand.c > index 28e0f2de7100..278964783fa6 100644 > --- a/drivers/devfreq/governor_simpleondemand.c > +++ b/drivers/devfreq/governor_simpleondemand.c > @@ -27,7 +27,6 @@ static int devfreq_simple_ondemand_func(struct devfreq *df, > unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD; > unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL; > struct devfreq_simple_ondemand_data *data = df->data; > - unsigned long max = (df->max_freq) ? df->max_freq : UINT_MAX; > > err = devfreq_update_stats(df); > if (err) > @@ -47,7 +46,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df, > > /* Assume MAX if it is going to be divided by zero */ > if (stat->total_time == 0) { > - *freq = max; > + *freq = df->max_freq; > return 0; > } > > @@ -60,13 +59,13 @@ static int devfreq_simple_ondemand_func(struct devfreq > *df, > /* Set MAX if it's busy enough */ > if (stat->busy_time * 100 > > stat->total_time * dfso_upthreshold) { > - *freq = max; > + *freq = df->max_freq; > return 0; > } > > /* Set MAX if we do not know the initial frequency */ > if (stat->current_frequency == 0) { > - *freq = max; > + *freq = df->max_freq; > return 0; > } > > Reviewed-by: Chanwoo Choi -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH 01/11] PM / devfreq: Init user limits from OPP limits, not viceversa
Hi, On 2018년 05월 26일 05:30, Matthias Kaehlcke wrote: > Commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when adding > the devfreq device") introduced the initialization of the user > limits min/max_freq from the lowest/highest available OPPs. Later > commit f1d981eaecf8 ("PM / devfreq: Use the available min/max > frequency") added scaling_min/max_freq, which actually represent > the frequencies of the lowest/highest available OPP. scaling_min/ > max_freq are initialized with the values from min/max_freq, which > is totally correct in the context, but a bit awkward to read. > > Swap the initialization and assign scaling_min/max_freq with the > OPP freqs and then the user limts min/max_freq with scaling_min/ > max_freq. > > Needless to say that this change is a NOP, intended to improve > readability. > > Signed-off-by: Matthias Kaehlcke> --- > drivers/devfreq/devfreq.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index fe2af6aa88fc..0057ef5b0a98 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -604,21 +604,21 @@ struct devfreq *devfreq_add_device(struct device *dev, > mutex_lock(>lock); > } > > - devfreq->min_freq = find_available_min_freq(devfreq); > - if (!devfreq->min_freq) { > + devfreq->scaling_min_freq = find_available_min_freq(devfreq); > + if (!devfreq->scaling_min_freq) { > mutex_unlock(>lock); > err = -EINVAL; > goto err_dev; > } > - devfreq->scaling_min_freq = devfreq->min_freq; > + devfreq->min_freq = devfreq->scaling_min_freq; > > - devfreq->max_freq = find_available_max_freq(devfreq); > - if (!devfreq->max_freq) { > + devfreq->scaling_max_freq = find_available_max_freq(devfreq); > + if (!devfreq->scaling_max_freq) { > mutex_unlock(>lock); > err = -EINVAL; > goto err_dev; > } > - devfreq->scaling_max_freq = devfreq->max_freq; > + devfreq->max_freq = devfreq->scaling_max_freq; > > dev_set_name(>dev, "devfreq%d", > atomic_inc_return(_no)); > I already replied with my Reviewed-by tag. You are missing my tag. Again, Reviewed-by: Chanwoo Choi -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
On Sat, May 26, 2018 at 09:15:05PM -0700, Guenter Roeck wrote: > Good point. Maybe it would be better to limit the warning to SMP systems > instead of (unnecessarily) fixing drivers all over the place ? No. The coherent_dma_mask is the mask used for dma_alloc_coherent and dma_alloc_attrs. It has nothing to do with cache coherent in SMP systems.
Re: [PATCH 01/11] PM / devfreq: Init user limits from OPP limits, not viceversa
Hi, On 2018년 05월 26일 05:30, Matthias Kaehlcke wrote: > Commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when adding > the devfreq device") introduced the initialization of the user > limits min/max_freq from the lowest/highest available OPPs. Later > commit f1d981eaecf8 ("PM / devfreq: Use the available min/max > frequency") added scaling_min/max_freq, which actually represent > the frequencies of the lowest/highest available OPP. scaling_min/ > max_freq are initialized with the values from min/max_freq, which > is totally correct in the context, but a bit awkward to read. > > Swap the initialization and assign scaling_min/max_freq with the > OPP freqs and then the user limts min/max_freq with scaling_min/ > max_freq. > > Needless to say that this change is a NOP, intended to improve > readability. > > Signed-off-by: Matthias Kaehlcke > --- > drivers/devfreq/devfreq.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index fe2af6aa88fc..0057ef5b0a98 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -604,21 +604,21 @@ struct devfreq *devfreq_add_device(struct device *dev, > mutex_lock(>lock); > } > > - devfreq->min_freq = find_available_min_freq(devfreq); > - if (!devfreq->min_freq) { > + devfreq->scaling_min_freq = find_available_min_freq(devfreq); > + if (!devfreq->scaling_min_freq) { > mutex_unlock(>lock); > err = -EINVAL; > goto err_dev; > } > - devfreq->scaling_min_freq = devfreq->min_freq; > + devfreq->min_freq = devfreq->scaling_min_freq; > > - devfreq->max_freq = find_available_max_freq(devfreq); > - if (!devfreq->max_freq) { > + devfreq->scaling_max_freq = find_available_max_freq(devfreq); > + if (!devfreq->scaling_max_freq) { > mutex_unlock(>lock); > err = -EINVAL; > goto err_dev; > } > - devfreq->scaling_max_freq = devfreq->max_freq; > + devfreq->max_freq = devfreq->scaling_max_freq; > > dev_set_name(>dev, "devfreq%d", > atomic_inc_return(_no)); > I already replied with my Reviewed-by tag. You are missing my tag. Again, Reviewed-by: Chanwoo Choi -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
On Sat, May 26, 2018 at 09:15:05PM -0700, Guenter Roeck wrote: > Good point. Maybe it would be better to limit the warning to SMP systems > instead of (unnecessarily) fixing drivers all over the place ? No. The coherent_dma_mask is the mask used for dma_alloc_coherent and dma_alloc_attrs. It has nothing to do with cache coherent in SMP systems.
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
On Mon, 28 May 2018, Michael Schmitz wrote: > Hi Finn, > > Am 27.05.2018 um 17:49 schrieb Finn Thain: > > On Sun, 27 May 2018, Michael Schmitz wrote: > > > >> That should have fixed the warning already ... > > > > It's still not fixed (hence my "acked-by" for Geunter's patch). > > > > Odd - does link order still matter even though the > arch_setup_dev_archdata() function from the core platform code is > declared as a weak symbol? > > I'll see what I can find out on elgar ... > Any one of the numerous patches/rfcs/suggestions that I sent will avoid the WARN splat. When I said "it's still not fixed", what I meant to say was, "it's still not fixed in mainline and no proposed fix was accepted to the best of my knowledge". -- > Cheers, > > Michael >
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
On Mon, 28 May 2018, Michael Schmitz wrote: > Hi Finn, > > Am 27.05.2018 um 17:49 schrieb Finn Thain: > > On Sun, 27 May 2018, Michael Schmitz wrote: > > > >> That should have fixed the warning already ... > > > > It's still not fixed (hence my "acked-by" for Geunter's patch). > > > > Odd - does link order still matter even though the > arch_setup_dev_archdata() function from the core platform code is > declared as a weak symbol? > > I'll see what I can find out on elgar ... > Any one of the numerous patches/rfcs/suggestions that I sent will avoid the WARN splat. When I said "it's still not fixed", what I meant to say was, "it's still not fixed in mainline and no proposed fix was accepted to the best of my knowledge". -- > Cheers, > > Michael >
Re: B53 DSA switch problem on Banana Pi-R1 on Fedora 26 - systemd-networkd problem
On 27.05.2018 22:35, Florian Fainelli wrote: Le 05/27/18 à 12:18, Gerhard Wiesinger a écrit : On 27.05.2018 21:01, Gerhard Wiesinger wrote: On 24.05.2018 08:22, Gerhard Wiesinger wrote: On 24.05.2018 07:29, Gerhard Wiesinger wrote: After some analysis with Florian (thnx) we found out that the current implementation is broken: https://patchwork.ozlabs.org/patch/836538/ https://github.com/torvalds/linux/commit/c499696e7901bda18385ac723b7bd27c3a4af624#diff-a2b6f8d89e18de600e873ac3ac43fa1d Florians comment: c499696e7901bda18385ac723b7bd27c3a4af624 ("net: dsa: b53: Stop using dev->cpu_port incorrectly") since it would result in no longer setting the CPU port as tagged for a specific VLAN. Easiest way for you right now is to just revert it, but this needs some more thoughts for a proper upstream change. I will think about it some more. Can confirm 4.14.18-200.fc26.armv7hl works, 4.15.x should be broken. # Kernel 4.14.x ok https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/drivers/net/dsa/b53?h=v4.14.43 # Kernel 4.15.x should be NOT ok https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/drivers/net/dsa/b53?h=v4.15.18 Forgot to mention: What's also strange is that the VLAN ID is very high: # 4.14.18-300.fc27.armv7hl, iproute-4.15.0-1.fc28.armv7hl ip -d link show eth0.101 | grep "vlan protocol" vlan protocol 802.1Q id 3069279796 ip -d link show eth0.102 | grep "vlan protocol" vlan protocol 802.1Q id 3068673588 On older kernels this looks ok: 4.12.8-200.fc25.armv7hl, iproute-4.11.0-1.fc25.armv7hl: ip -d link show eth0.101 | grep "vlan protocol" vlan protocol 802.1Q id 101 ip -d link show eth0.102 | grep "vlan protocol" vlan protocol 802.1Q id 102 Ideas? That is quite likely a kernel/iproute2 issue, if you configured the switch through bridge vlan to have the ports in VLAN 101 and VLAN 102 and you do indeed see frames entering eth0 with these VLAN IDs, then clearly the bridge -> switchdev -> dsa -> b53 part is working just fine and what you are seeing is some for of kernel header/netlink incompatibility. Yes, sniffing on eth0 shows the correct VLAN IDs, e.g. 101. Yes, my guess is that tools are wrong and have random values on 2 calls in different values (e.g. alsopromiscuity ) , see below Who can fix it? BTW: On FC27 same issue with same kernel version, but guess older iproute version. Ciao, Gerhard ip -d link show eth0.101 13: eth0.101@eth0:mtu 1500 qdisc noqueue master br0 state UP mode DEFAULT group default qlen 1000 link/ether 02:18:09:ab:cd:ef brd ff:ff:ff:ff:ff:ff promiscuity 3068661300 vlan protocol 802.1Q id 3068661300 bridge_slave state forwarding priority 32 cost 4 hairpin off guard off root_block off fastleave off learning on flood on port_id 0x8005 port_no 0x5 designa ted_port 3068661300 designated_cost 3068661300 designated_bridge 8000.66:5d:a2:ab:cd:ef designated_root 8000.66:5d:a2:ab:cd:ef hold_timer 0.00 message_age_tim er 0.00 forward_delay_timer 0.00 topology_change_ack 3068661300 config_pending 3068661300 proxy_arp off proxy_arp_wifi off mcast_router 3068661300 mcast_ fast_leave off mcast_flood on vlan_tunnel off addrgenmode eui64 numtxqueues 3068661300 numrxqueues 3068661300 gso_max_size 3068661300 gso_max_segs 3068661300 ip -d link show eth0.101 13: eth0.101@eth0: mtu 1500 qdisc noqueue master br0 state UP mode DEFAULT group default qlen 1000 link/ether 02:18:09:ab:cd:ef brd ff:ff:ff:ff:ff:ff promiscuity 3068735028 vlan protocol 802.1Q id 3068735028 bridge_slave state forwarding priority 32 cost 4 hairpin off guard off root_block off fastleave off learning on flood on port_id 0x8005 port_no 0x5 designa ted_port 3068735028 designated_cost 3068735028 designated_bridge 8000.66:5d:ab:cd:ef designated_root 8000.66:5d:a2:ab:cd:ef hold_timer 0.00 message_age_tim er 0.00 forward_delay_timer 0.00 topology_change_ack 3068735028 config_pending 3068735028 proxy_arp off proxy_arp_wifi off mcast_router 3068735028 mcast_ fast_leave off mcast_flood on vlan_tunnel off addrgenmode eui64 numtxqueues 3068735028 numrxqueues 3068735028 gso_max_size 3068735028 gso_max_segs 3068735028
Re: B53 DSA switch problem on Banana Pi-R1 on Fedora 26 - systemd-networkd problem
On 27.05.2018 22:35, Florian Fainelli wrote: Le 05/27/18 à 12:18, Gerhard Wiesinger a écrit : On 27.05.2018 21:01, Gerhard Wiesinger wrote: On 24.05.2018 08:22, Gerhard Wiesinger wrote: On 24.05.2018 07:29, Gerhard Wiesinger wrote: After some analysis with Florian (thnx) we found out that the current implementation is broken: https://patchwork.ozlabs.org/patch/836538/ https://github.com/torvalds/linux/commit/c499696e7901bda18385ac723b7bd27c3a4af624#diff-a2b6f8d89e18de600e873ac3ac43fa1d Florians comment: c499696e7901bda18385ac723b7bd27c3a4af624 ("net: dsa: b53: Stop using dev->cpu_port incorrectly") since it would result in no longer setting the CPU port as tagged for a specific VLAN. Easiest way for you right now is to just revert it, but this needs some more thoughts for a proper upstream change. I will think about it some more. Can confirm 4.14.18-200.fc26.armv7hl works, 4.15.x should be broken. # Kernel 4.14.x ok https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/drivers/net/dsa/b53?h=v4.14.43 # Kernel 4.15.x should be NOT ok https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/drivers/net/dsa/b53?h=v4.15.18 Forgot to mention: What's also strange is that the VLAN ID is very high: # 4.14.18-300.fc27.armv7hl, iproute-4.15.0-1.fc28.armv7hl ip -d link show eth0.101 | grep "vlan protocol" vlan protocol 802.1Q id 3069279796 ip -d link show eth0.102 | grep "vlan protocol" vlan protocol 802.1Q id 3068673588 On older kernels this looks ok: 4.12.8-200.fc25.armv7hl, iproute-4.11.0-1.fc25.armv7hl: ip -d link show eth0.101 | grep "vlan protocol" vlan protocol 802.1Q id 101 ip -d link show eth0.102 | grep "vlan protocol" vlan protocol 802.1Q id 102 Ideas? That is quite likely a kernel/iproute2 issue, if you configured the switch through bridge vlan to have the ports in VLAN 101 and VLAN 102 and you do indeed see frames entering eth0 with these VLAN IDs, then clearly the bridge -> switchdev -> dsa -> b53 part is working just fine and what you are seeing is some for of kernel header/netlink incompatibility. Yes, sniffing on eth0 shows the correct VLAN IDs, e.g. 101. Yes, my guess is that tools are wrong and have random values on 2 calls in different values (e.g. alsopromiscuity ) , see below Who can fix it? BTW: On FC27 same issue with same kernel version, but guess older iproute version. Ciao, Gerhard ip -d link show eth0.101 13: eth0.101@eth0: mtu 1500 qdisc noqueue master br0 state UP mode DEFAULT group default qlen 1000 link/ether 02:18:09:ab:cd:ef brd ff:ff:ff:ff:ff:ff promiscuity 3068661300 vlan protocol 802.1Q id 3068661300 bridge_slave state forwarding priority 32 cost 4 hairpin off guard off root_block off fastleave off learning on flood on port_id 0x8005 port_no 0x5 designa ted_port 3068661300 designated_cost 3068661300 designated_bridge 8000.66:5d:a2:ab:cd:ef designated_root 8000.66:5d:a2:ab:cd:ef hold_timer 0.00 message_age_tim er 0.00 forward_delay_timer 0.00 topology_change_ack 3068661300 config_pending 3068661300 proxy_arp off proxy_arp_wifi off mcast_router 3068661300 mcast_ fast_leave off mcast_flood on vlan_tunnel off addrgenmode eui64 numtxqueues 3068661300 numrxqueues 3068661300 gso_max_size 3068661300 gso_max_segs 3068661300 ip -d link show eth0.101 13: eth0.101@eth0: mtu 1500 qdisc noqueue master br0 state UP mode DEFAULT group default qlen 1000 link/ether 02:18:09:ab:cd:ef brd ff:ff:ff:ff:ff:ff promiscuity 3068735028 vlan protocol 802.1Q id 3068735028 bridge_slave state forwarding priority 32 cost 4 hairpin off guard off root_block off fastleave off learning on flood on port_id 0x8005 port_no 0x5 designa ted_port 3068735028 designated_cost 3068735028 designated_bridge 8000.66:5d:ab:cd:ef designated_root 8000.66:5d:a2:ab:cd:ef hold_timer 0.00 message_age_tim er 0.00 forward_delay_timer 0.00 topology_change_ack 3068735028 config_pending 3068735028 proxy_arp off proxy_arp_wifi off mcast_router 3068735028 mcast_ fast_leave off mcast_flood on vlan_tunnel off addrgenmode eui64 numtxqueues 3068735028 numrxqueues 3068735028 gso_max_size 3068735028 gso_max_segs 3068735028
RE: [PATCH 08/11] PM / devfreq: Make update_devfreq() public
>Currently update_devfreq() is only visible to devfreq governors outside >of devfreq.c. Make it public to allow drivers that adjust devfreq policies >to cause a re-evaluation of the frequency after a policy change. > >Signed-off-by: Matthias Kaehlcke>--- > drivers/devfreq/governor.h | 3 --- > include/linux/devfreq.h| 8 > 2 files changed, 8 insertions(+), 3 deletions(-) With the requirement from patch 9/11, this commit is reasonable enough. Acked-by: MyungJoo Ham
RE: [PATCH 08/11] PM / devfreq: Make update_devfreq() public
>Currently update_devfreq() is only visible to devfreq governors outside >of devfreq.c. Make it public to allow drivers that adjust devfreq policies >to cause a re-evaluation of the frequency after a policy change. > >Signed-off-by: Matthias Kaehlcke >--- > drivers/devfreq/governor.h | 3 --- > include/linux/devfreq.h| 8 > 2 files changed, 8 insertions(+), 3 deletions(-) With the requirement from patch 9/11, this commit is reasonable enough. Acked-by: MyungJoo Ham
RE: [PATCH v11 00/26] Speculative page faults
Some regression and improvements is found by LKP-tools(linux kernel performance) on V9 patch series tested on Intel 4s Skylake platform. The regression result is sorted by the metric will-it-scale.per_thread_ops. Branch: Laurent-Dufour/Speculative-page-faults/20180316-151833 (V9 patch series) Commit id: base commit: d55f34411b1b126429a823d06c3124c16283231f head commit: 0355322b3577eeab7669066df42c550a56801110 Benchmark suite: will-it-scale Download link: https://github.com/antonblanchard/will-it-scale/tree/master/tests Metrics: will-it-scale.per_process_ops=processes/nr_cpu will-it-scale.per_thread_ops=threads/nr_cpu test box: lkp-skl-4sp1(nr_cpu=192,memory=768G) THP: enable / disable nr_task: 100% 1. Regressions: a) THP enabled: testcasebasechange head metric page_fault3/ enable THP 10092 -17.5% 8323 will-it-scale.per_thread_ops page_fault2/ enable THP 8300 -17.2% 6869 will-it-scale.per_thread_ops brk1/ enable THP 957.67 -7.6% 885 will-it-scale.per_thread_ops page_fault3/ enable THP172821-5.3%163692 will-it-scale.per_process_ops signal1/ enable THP 9125-3.2% 8834 will-it-scale.per_process_ops b) THP disabled: testcasebasechange head metric page_fault3/ disable THP10107 -19.1% 8180 will-it-scale.per_thread_ops page_fault2/ disable THP 8432 -17.8% 6931 will-it-scale.per_thread_ops context_switch1/ disable THP 215389-6.8%200776 will-it-scale.per_thread_ops brk1/ disable THP 939.67 -6.6% 877.33 will-it-scale.per_thread_ops page_fault3/ disable THP 173145-4.7%165064 will-it-scale.per_process_ops signal1/ disable THP 9162-3.9% 8802 will-it-scale.per_process_ops 2. Improvements: a) THP enabled: testcasebasechange head metric malloc1/ enable THP 66.33+469.8% 383.67 will-it-scale.per_thread_ops writeseek3/ enable THP 2531 +4.5% 2646 will-it-scale.per_thread_ops signal1/ enable THP 989.33 +2.8% 1016 will-it-scale.per_thread_ops b) THP disabled: testcasebasechange head metric malloc1/ disable THP 90.33+417.3% 467.33 will-it-scale.per_thread_ops read2/ disable THP 58934+39.2% 82060 will-it-scale.per_thread_ops page_fault1/ disable THP8607+36.4% 11736 will-it-scale.per_thread_ops read1/ disable THP314063+12.7%353934 will-it-scale.per_thread_ops writeseek3/ disable THP 2452+12.5% 2759 will-it-scale.per_thread_ops signal1/ disable THP 971.33 +5.5% 1024 will-it-scale.per_thread_ops Notes: for above values in column "change", the higher value means that the related testcase result on head commit is better than that on base commit for this benchmark. Best regards Haiyan Song From: owner-linux...@kvack.org [owner-linux...@kvack.org] on behalf of Laurent Dufour [lduf...@linux.vnet.ibm.com] Sent: Thursday, May 17, 2018 7:06 PM To: a...@linux-foundation.org; mho...@kernel.org; pet...@infradead.org; kir...@shutemov.name; a...@linux.intel.com; d...@stgolabs.net; j...@suse.cz; Matthew Wilcox; khand...@linux.vnet.ibm.com; aneesh.ku...@linux.vnet.ibm.com; b...@kernel.crashing.org; m...@ellerman.id.au; pau...@samba.org; Thomas Gleixner; Ingo Molnar; h...@zytor.com; Will Deacon; Sergey Senozhatsky; sergey.senozhatsky.w...@gmail.com; Andrea Arcangeli; Alexei Starovoitov; Wang, Kemi; Daniel Jordan; David Rientjes; Jerome Glisse; Ganesh Mahendran; Minchan Kim; Punit Agrawal; vinayak menon; Yang Shi Cc: linux-kernel@vger.kernel.org; linux...@kvack.org; ha...@linux.vnet.ibm.com; npig...@gmail.com; bsinghar...@gmail.com; paul...@linux.vnet.ibm.com; Tim Chen; linuxppc-...@lists.ozlabs.org; x...@kernel.org Subject: [PATCH v11 00/26] Speculative page faults This is a port on kernel 4.17 of the work done by Peter Zijlstra to handle page fault without holding the mm semaphore [1]. The idea is to try to handle user space page faults without holding the mmap_sem. This should allow better concurrency for massively threaded process since the page fault handler will not wait for other threads memory layout change to be done, assuming that this change is done in another part of the process's memory space. This type page fault is named speculative page
RE: [PATCH v11 00/26] Speculative page faults
Some regression and improvements is found by LKP-tools(linux kernel performance) on V9 patch series tested on Intel 4s Skylake platform. The regression result is sorted by the metric will-it-scale.per_thread_ops. Branch: Laurent-Dufour/Speculative-page-faults/20180316-151833 (V9 patch series) Commit id: base commit: d55f34411b1b126429a823d06c3124c16283231f head commit: 0355322b3577eeab7669066df42c550a56801110 Benchmark suite: will-it-scale Download link: https://github.com/antonblanchard/will-it-scale/tree/master/tests Metrics: will-it-scale.per_process_ops=processes/nr_cpu will-it-scale.per_thread_ops=threads/nr_cpu test box: lkp-skl-4sp1(nr_cpu=192,memory=768G) THP: enable / disable nr_task: 100% 1. Regressions: a) THP enabled: testcasebasechange head metric page_fault3/ enable THP 10092 -17.5% 8323 will-it-scale.per_thread_ops page_fault2/ enable THP 8300 -17.2% 6869 will-it-scale.per_thread_ops brk1/ enable THP 957.67 -7.6% 885 will-it-scale.per_thread_ops page_fault3/ enable THP172821-5.3%163692 will-it-scale.per_process_ops signal1/ enable THP 9125-3.2% 8834 will-it-scale.per_process_ops b) THP disabled: testcasebasechange head metric page_fault3/ disable THP10107 -19.1% 8180 will-it-scale.per_thread_ops page_fault2/ disable THP 8432 -17.8% 6931 will-it-scale.per_thread_ops context_switch1/ disable THP 215389-6.8%200776 will-it-scale.per_thread_ops brk1/ disable THP 939.67 -6.6% 877.33 will-it-scale.per_thread_ops page_fault3/ disable THP 173145-4.7%165064 will-it-scale.per_process_ops signal1/ disable THP 9162-3.9% 8802 will-it-scale.per_process_ops 2. Improvements: a) THP enabled: testcasebasechange head metric malloc1/ enable THP 66.33+469.8% 383.67 will-it-scale.per_thread_ops writeseek3/ enable THP 2531 +4.5% 2646 will-it-scale.per_thread_ops signal1/ enable THP 989.33 +2.8% 1016 will-it-scale.per_thread_ops b) THP disabled: testcasebasechange head metric malloc1/ disable THP 90.33+417.3% 467.33 will-it-scale.per_thread_ops read2/ disable THP 58934+39.2% 82060 will-it-scale.per_thread_ops page_fault1/ disable THP8607+36.4% 11736 will-it-scale.per_thread_ops read1/ disable THP314063+12.7%353934 will-it-scale.per_thread_ops writeseek3/ disable THP 2452+12.5% 2759 will-it-scale.per_thread_ops signal1/ disable THP 971.33 +5.5% 1024 will-it-scale.per_thread_ops Notes: for above values in column "change", the higher value means that the related testcase result on head commit is better than that on base commit for this benchmark. Best regards Haiyan Song From: owner-linux...@kvack.org [owner-linux...@kvack.org] on behalf of Laurent Dufour [lduf...@linux.vnet.ibm.com] Sent: Thursday, May 17, 2018 7:06 PM To: a...@linux-foundation.org; mho...@kernel.org; pet...@infradead.org; kir...@shutemov.name; a...@linux.intel.com; d...@stgolabs.net; j...@suse.cz; Matthew Wilcox; khand...@linux.vnet.ibm.com; aneesh.ku...@linux.vnet.ibm.com; b...@kernel.crashing.org; m...@ellerman.id.au; pau...@samba.org; Thomas Gleixner; Ingo Molnar; h...@zytor.com; Will Deacon; Sergey Senozhatsky; sergey.senozhatsky.w...@gmail.com; Andrea Arcangeli; Alexei Starovoitov; Wang, Kemi; Daniel Jordan; David Rientjes; Jerome Glisse; Ganesh Mahendran; Minchan Kim; Punit Agrawal; vinayak menon; Yang Shi Cc: linux-kernel@vger.kernel.org; linux...@kvack.org; ha...@linux.vnet.ibm.com; npig...@gmail.com; bsinghar...@gmail.com; paul...@linux.vnet.ibm.com; Tim Chen; linuxppc-...@lists.ozlabs.org; x...@kernel.org Subject: [PATCH v11 00/26] Speculative page faults This is a port on kernel 4.17 of the work done by Peter Zijlstra to handle page fault without holding the mm semaphore [1]. The idea is to try to handle user space page faults without holding the mmap_sem. This should allow better concurrency for massively threaded process since the page fault handler will not wait for other threads memory layout change to be done, assuming that this change is done in another part of the process's memory space. This type page fault is named speculative page
RE: [PATCH v2] dcdbas: Add support for WSMT ACPI table
> -Original Message- > From: Warzecha, Douglas > Sent: Friday, May 25, 2018 2:02 PM > To: Stuart Hayes > Cc: Limonciello, Mario; Dominguez, Jared; linux-kernel@vger.kernel.org > Subject: RE: [PATCH v2] dcdbas: Add support for WSMT ACPI table > > > On 05/16/2018 9:06 AM, Stuart Hayes wrote: > > If the WSMT ACPI table is present and indicates that a fixed communication > > buffer should be used, use the firmware-specified buffer instead of > > allocating a buffer in memory for communications between the dcdbas driver > > and firmare. > > > > Signed-off-by: Stuart Hayes> > --- > > v2 Bumped driver version to 5.6.0-3.3 > > > > > > drivers/firmware/Kconfig | 2 +- > > drivers/firmware/dcdbas.c | 102 > > +++--- > > drivers/firmware/dcdbas.h | 11 + > > 3 files changed, 109 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index > > b7c748248e53..a2bd6092bfa1 100644 > > --- a/drivers/firmware/Kconfig > > +++ b/drivers/firmware/Kconfig > > @@ -125,7 +125,7 @@ config DELL_RBU > > > > config DCDBAS > > tristate "Dell Systems Management Base Driver" > > - depends on X86 > > + depends on X86 && ACPI > > help > > The Dell Systems Management Base Driver provides a sysfs > > interface > > for systems management software to perform System > > Management diff --git a/drivers/firmware/dcdbas.c > > b/drivers/firmware/dcdbas.c index 0bdea60c65dd..1699cfdcefe4 100644 > > --- a/drivers/firmware/dcdbas.c > > +++ b/drivers/firmware/dcdbas.c > > @@ -36,12 +36,13 @@ > > #include > > #include > > #include > > +#include > > #include > > > > #include "dcdbas.h" > > > > #define DRIVER_NAME"dcdbas" > > -#define DRIVER_VERSION "5.6.0-3.2" > > +#define DRIVER_VERSION "5.6.0-3.3" > > #define DRIVER_DESCRIPTION "Dell Systems Management Base Driver" > > > > static struct platform_device *dcdbas_pdev; @@ -49,19 +50,23 @@ static > > struct platform_device *dcdbas_pdev; static u8 *smi_data_buf; static > > dma_addr_t smi_data_buf_handle; static unsigned long smi_data_buf_size; > > +static unsigned long max_smi_data_buf_size = MAX_SMI_DATA_BUF_SIZE; > > static u32 smi_data_buf_phys_addr; > > static DEFINE_MUTEX(smi_data_lock); > > +static u8 *eps_buffer; > > > > static unsigned int host_control_action; static unsigned int > > host_control_smi_type; static unsigned int host_control_on_shutdown; > > > > +static bool wsmt_enabled; > > + > > /** > > * smi_data_buf_free: free SMI data buffer > > */ > > static void smi_data_buf_free(void) > > { > > - if (!smi_data_buf) > > + if (!smi_data_buf || wsmt_enabled) > > return; > > > > dev_dbg(_pdev->dev, "%s: phys: %x size: %lu\n", @@ -86,7 > > +91,7 @@ static int smi_data_buf_realloc(unsigned long size) > > if (smi_data_buf_size >= size) > > return 0; > > > > - if (size > MAX_SMI_DATA_BUF_SIZE) > > + if (size > max_smi_data_buf_size) > > return -EINVAL; > > > > /* new buffer is needed */ > > @@ -169,7 +174,7 @@ static ssize_t smi_data_write(struct file *filp, struct > > kobject *kobj, { > > ssize_t ret; > > > > - if ((pos + count) > MAX_SMI_DATA_BUF_SIZE) > > + if ((pos + count) > max_smi_data_buf_size) > > return -EINVAL; > > > > mutex_lock(_data_lock); > > @@ -323,7 +328,8 @@ static ssize_t smi_request_store(struct device *dev, > > break; > > case 1: > > /* Calling Interface SMI */ > > - smi_cmd->ebx = (u32) virt_to_phys(smi_cmd- > > >command_buffer); > > + smi_cmd->ebx = smi_data_buf_phys_addr > > + + offsetof(struct smi_cmd, > > command_buffer); > > ret = dcdbas_smi_request(smi_cmd); > > if (!ret) > > ret = count; > > @@ -482,6 +488,85 @@ static void dcdbas_host_control(void) > > } > > } > > > > +/* WSMT */ > > + > > +static u8 checksum(u8 *buffer, u8 length) { > > + u8 sum = 0; > > + u8 *end = buffer + length; > > + > > + while (buffer < end) > > + sum = (u8)(sum + *(buffer++)); > > + return sum; > > +} > > + > > +static inline struct smm_eps_table *check_eps_table(u8 *addr) { > > + struct smm_eps_table *eps = (struct smm_eps_table *)addr; > > + > > + if (strncmp(SMM_EPS_SIG, eps->smm_comm_buff_anchor, 4) != 0) > > + return NULL; > > + > > + if (checksum(addr, eps->length) != 0) > > + return NULL; > > + > > + return eps; > > +} > > + > > +static int dcdbas_check_wsmt(void) > > +{ > > + struct acpi_table_wsmt *wsmt = NULL; > > + struct smm_eps_table *eps = NULL; > > + u8 *addr; > > + > > + acpi_get_table(ACPI_SIG_WSMT, 0, (struct acpi_table_header > > **)); > > + if (!wsmt) > > + return 0; > > + > > + /* Check if WSMT ACPI table shows that protection is enabled */ > > + if (!(wsmt->protection_flags & >
RE: [PATCH v2] dcdbas: Add support for WSMT ACPI table
> -Original Message- > From: Warzecha, Douglas > Sent: Friday, May 25, 2018 2:02 PM > To: Stuart Hayes > Cc: Limonciello, Mario; Dominguez, Jared; linux-kernel@vger.kernel.org > Subject: RE: [PATCH v2] dcdbas: Add support for WSMT ACPI table > > > On 05/16/2018 9:06 AM, Stuart Hayes wrote: > > If the WSMT ACPI table is present and indicates that a fixed communication > > buffer should be used, use the firmware-specified buffer instead of > > allocating a buffer in memory for communications between the dcdbas driver > > and firmare. > > > > Signed-off-by: Stuart Hayes > > --- > > v2 Bumped driver version to 5.6.0-3.3 > > > > > > drivers/firmware/Kconfig | 2 +- > > drivers/firmware/dcdbas.c | 102 > > +++--- > > drivers/firmware/dcdbas.h | 11 + > > 3 files changed, 109 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index > > b7c748248e53..a2bd6092bfa1 100644 > > --- a/drivers/firmware/Kconfig > > +++ b/drivers/firmware/Kconfig > > @@ -125,7 +125,7 @@ config DELL_RBU > > > > config DCDBAS > > tristate "Dell Systems Management Base Driver" > > - depends on X86 > > + depends on X86 && ACPI > > help > > The Dell Systems Management Base Driver provides a sysfs > > interface > > for systems management software to perform System > > Management diff --git a/drivers/firmware/dcdbas.c > > b/drivers/firmware/dcdbas.c index 0bdea60c65dd..1699cfdcefe4 100644 > > --- a/drivers/firmware/dcdbas.c > > +++ b/drivers/firmware/dcdbas.c > > @@ -36,12 +36,13 @@ > > #include > > #include > > #include > > +#include > > #include > > > > #include "dcdbas.h" > > > > #define DRIVER_NAME"dcdbas" > > -#define DRIVER_VERSION "5.6.0-3.2" > > +#define DRIVER_VERSION "5.6.0-3.3" > > #define DRIVER_DESCRIPTION "Dell Systems Management Base Driver" > > > > static struct platform_device *dcdbas_pdev; @@ -49,19 +50,23 @@ static > > struct platform_device *dcdbas_pdev; static u8 *smi_data_buf; static > > dma_addr_t smi_data_buf_handle; static unsigned long smi_data_buf_size; > > +static unsigned long max_smi_data_buf_size = MAX_SMI_DATA_BUF_SIZE; > > static u32 smi_data_buf_phys_addr; > > static DEFINE_MUTEX(smi_data_lock); > > +static u8 *eps_buffer; > > > > static unsigned int host_control_action; static unsigned int > > host_control_smi_type; static unsigned int host_control_on_shutdown; > > > > +static bool wsmt_enabled; > > + > > /** > > * smi_data_buf_free: free SMI data buffer > > */ > > static void smi_data_buf_free(void) > > { > > - if (!smi_data_buf) > > + if (!smi_data_buf || wsmt_enabled) > > return; > > > > dev_dbg(_pdev->dev, "%s: phys: %x size: %lu\n", @@ -86,7 > > +91,7 @@ static int smi_data_buf_realloc(unsigned long size) > > if (smi_data_buf_size >= size) > > return 0; > > > > - if (size > MAX_SMI_DATA_BUF_SIZE) > > + if (size > max_smi_data_buf_size) > > return -EINVAL; > > > > /* new buffer is needed */ > > @@ -169,7 +174,7 @@ static ssize_t smi_data_write(struct file *filp, struct > > kobject *kobj, { > > ssize_t ret; > > > > - if ((pos + count) > MAX_SMI_DATA_BUF_SIZE) > > + if ((pos + count) > max_smi_data_buf_size) > > return -EINVAL; > > > > mutex_lock(_data_lock); > > @@ -323,7 +328,8 @@ static ssize_t smi_request_store(struct device *dev, > > break; > > case 1: > > /* Calling Interface SMI */ > > - smi_cmd->ebx = (u32) virt_to_phys(smi_cmd- > > >command_buffer); > > + smi_cmd->ebx = smi_data_buf_phys_addr > > + + offsetof(struct smi_cmd, > > command_buffer); > > ret = dcdbas_smi_request(smi_cmd); > > if (!ret) > > ret = count; > > @@ -482,6 +488,85 @@ static void dcdbas_host_control(void) > > } > > } > > > > +/* WSMT */ > > + > > +static u8 checksum(u8 *buffer, u8 length) { > > + u8 sum = 0; > > + u8 *end = buffer + length; > > + > > + while (buffer < end) > > + sum = (u8)(sum + *(buffer++)); > > + return sum; > > +} > > + > > +static inline struct smm_eps_table *check_eps_table(u8 *addr) { > > + struct smm_eps_table *eps = (struct smm_eps_table *)addr; > > + > > + if (strncmp(SMM_EPS_SIG, eps->smm_comm_buff_anchor, 4) != 0) > > + return NULL; > > + > > + if (checksum(addr, eps->length) != 0) > > + return NULL; > > + > > + return eps; > > +} > > + > > +static int dcdbas_check_wsmt(void) > > +{ > > + struct acpi_table_wsmt *wsmt = NULL; > > + struct smm_eps_table *eps = NULL; > > + u8 *addr; > > + > > + acpi_get_table(ACPI_SIG_WSMT, 0, (struct acpi_table_header > > **)); > > + if (!wsmt) > > + return 0; > > + > > + /* Check if WSMT ACPI table shows that protection is enabled */ > > + if (!(wsmt->protection_flags & > >
Re: [PATCH 3/3] s390:add missing CONFIG_STRICT_KERNEL_RWX for mark_rodata_ro
On Mon, May 28, 2018 at 11:33:55AM +0800, nixiaoming wrote: > Signed-off-by: nixiaomingPlease do not submit patches without any changelog text. I do not accept such patches, but other maintainers might be easier... greg k-h
Re: [PATCH 3/3] s390:add missing CONFIG_STRICT_KERNEL_RWX for mark_rodata_ro
On Mon, May 28, 2018 at 11:33:55AM +0800, nixiaoming wrote: > Signed-off-by: nixiaoming Please do not submit patches without any changelog text. I do not accept such patches, but other maintainers might be easier... greg k-h
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
Hi Finn, Am 27.05.2018 um 17:49 schrieb Finn Thain: > On Sun, 27 May 2018, Michael Schmitz wrote: > >> That should have fixed the warning already ... > > It's still not fixed (hence my "acked-by" for Geunter's patch). > Odd - does link order still matter even though the arch_setup_dev_archdata() function from the core platform code is declared as a weak symbol? I'll see what I can find out on elgar ... Cheers, Michael
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
Hi Finn, Am 27.05.2018 um 17:49 schrieb Finn Thain: > On Sun, 27 May 2018, Michael Schmitz wrote: > >> That should have fixed the warning already ... > > It's still not fixed (hence my "acked-by" for Geunter's patch). > Odd - does link order still matter even though the arch_setup_dev_archdata() function from the core platform code is declared as a weak symbol? I'll see what I can find out on elgar ... Cheers, Michael
RE: [PATCH 07/11] PM / devfreg: Add support policy notifiers
>Policy notifiers are called before a frequency change and may narrow >the min/max frequency range in devfreq_policy, which is used to adjust >the target frequency if it is beyond this range. > >Also add a few helpers: > - devfreq_verify_within_[dev_]limits() >- should be used by the notifiers for policy adjustments. > - dev_to_devfreq() >- lookup a devfreq strict from a device pointer > >Signed-off-by: Matthias Kaehlcke>--- > drivers/devfreq/devfreq.c | 47 +--- > include/linux/devfreq.h | 66 +++ > 2 files changed, 102 insertions(+), 11 deletions(-) Hello Matthias, Why should we have yet another notifier from an instance of devfreq? Wouldn't it better to let the current notifier (transition notifier) handle new events as well by adding possible event states to it? Anyway, is this the reason why you've separated some data of devfreq into "policy" struct? (I was wondering why while reading commit 6/11). Cheers MyungJoo
RE: [PATCH 07/11] PM / devfreg: Add support policy notifiers
>Policy notifiers are called before a frequency change and may narrow >the min/max frequency range in devfreq_policy, which is used to adjust >the target frequency if it is beyond this range. > >Also add a few helpers: > - devfreq_verify_within_[dev_]limits() >- should be used by the notifiers for policy adjustments. > - dev_to_devfreq() >- lookup a devfreq strict from a device pointer > >Signed-off-by: Matthias Kaehlcke >--- > drivers/devfreq/devfreq.c | 47 +--- > include/linux/devfreq.h | 66 +++ > 2 files changed, 102 insertions(+), 11 deletions(-) Hello Matthias, Why should we have yet another notifier from an instance of devfreq? Wouldn't it better to let the current notifier (transition notifier) handle new events as well by adding possible event states to it? Anyway, is this the reason why you've separated some data of devfreq into "policy" struct? (I was wondering why while reading commit 6/11). Cheers MyungJoo
Re: [PATCH v3 1/2] rtmutex: allow specifying a subclass for nested locking
On Thu, May 24, 2018 at 03:52:39PM +0200, Peter Rosin wrote: > Needed for annotating rt_mutex locks. > > Signed-off-by: Peter Rosin> --- > include/linux/rtmutex.h | 7 +++ > kernel/locking/rtmutex.c | 29 + > 2 files changed, 32 insertions(+), 4 deletions(-) > > diff --git a/include/linux/rtmutex.h b/include/linux/rtmutex.h > index 1b92a28dd672..6fd615a0eea9 100644 > --- a/include/linux/rtmutex.h > +++ b/include/linux/rtmutex.h > @@ -106,7 +106,14 @@ static inline int rt_mutex_is_locked(struct rt_mutex > *lock) > extern void __rt_mutex_init(struct rt_mutex *lock, const char *name, struct > lock_class_key *key); > extern void rt_mutex_destroy(struct rt_mutex *lock); > > +#ifdef CONFIG_DEBUG_LOCK_ALLOC > +extern void rt_mutex_lock_nested(struct rt_mutex *lock, unsigned int > subclass); > +#define rt_mutex_lock(lock) rt_mutex_lock_nested(lock, 0) > +#else > extern void rt_mutex_lock(struct rt_mutex *lock); > +#define rt_mutex_lock_nested(lock, subclass) rt_mutex_lock(lock) > +#endif > + > extern int rt_mutex_lock_interruptible(struct rt_mutex *lock); > extern int rt_mutex_timed_lock(struct rt_mutex *lock, > struct hrtimer_sleeper *timeout); > diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c : > } > > +static inline void __rt_mutex_lock(struct rt_mutex *lock, unsigned int > subclass) > +{ > + might_sleep(); > + > + mutex_acquire(>dep_map, subclass, 0, _RET_IP_); > + rt_mutex_fastlock(lock, TASK_UNINTERRUPTIBLE, rt_mutex_slowlock); > +} > + > +#ifdef CONFIG_DEBUG_LOCK_ALLOC > +/** > + * rt_mutex_lock_nested - lock a rt_mutex This ifdef seems consistent with other nested locking primitives, but its kind of confusing. The Kconfig.debug for DEBUG_LOCK_ALLOC says: config DEBUG_LOCK_ALLOC bool "Lock debugging: detect incorrect freeing of live locks" [...] help This feature will check whether any held lock (spinlock, rwlock, mutex or rwsem) is incorrectly freed by the kernel, via any of the memory-freeing routines (kfree(), kmem_cache_free(), free_pages(), vfree(), etc.), whether a live lock is incorrectly reinitialized via spin_lock_init()/mutex_init()/etc., or whether there is any lock held during task exit. Shouldn't this ideally be ifdef'd under PROVE_LOCKING for this and other locking primitives? Any idea what's the reason? I know PROVE_LOCKING selects DEBUG_LOCK_ALLOC but still.. thanks! - Joel
Re: [PATCH v3 1/2] rtmutex: allow specifying a subclass for nested locking
On Thu, May 24, 2018 at 03:52:39PM +0200, Peter Rosin wrote: > Needed for annotating rt_mutex locks. > > Signed-off-by: Peter Rosin > --- > include/linux/rtmutex.h | 7 +++ > kernel/locking/rtmutex.c | 29 + > 2 files changed, 32 insertions(+), 4 deletions(-) > > diff --git a/include/linux/rtmutex.h b/include/linux/rtmutex.h > index 1b92a28dd672..6fd615a0eea9 100644 > --- a/include/linux/rtmutex.h > +++ b/include/linux/rtmutex.h > @@ -106,7 +106,14 @@ static inline int rt_mutex_is_locked(struct rt_mutex > *lock) > extern void __rt_mutex_init(struct rt_mutex *lock, const char *name, struct > lock_class_key *key); > extern void rt_mutex_destroy(struct rt_mutex *lock); > > +#ifdef CONFIG_DEBUG_LOCK_ALLOC > +extern void rt_mutex_lock_nested(struct rt_mutex *lock, unsigned int > subclass); > +#define rt_mutex_lock(lock) rt_mutex_lock_nested(lock, 0) > +#else > extern void rt_mutex_lock(struct rt_mutex *lock); > +#define rt_mutex_lock_nested(lock, subclass) rt_mutex_lock(lock) > +#endif > + > extern int rt_mutex_lock_interruptible(struct rt_mutex *lock); > extern int rt_mutex_timed_lock(struct rt_mutex *lock, > struct hrtimer_sleeper *timeout); > diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c : > } > > +static inline void __rt_mutex_lock(struct rt_mutex *lock, unsigned int > subclass) > +{ > + might_sleep(); > + > + mutex_acquire(>dep_map, subclass, 0, _RET_IP_); > + rt_mutex_fastlock(lock, TASK_UNINTERRUPTIBLE, rt_mutex_slowlock); > +} > + > +#ifdef CONFIG_DEBUG_LOCK_ALLOC > +/** > + * rt_mutex_lock_nested - lock a rt_mutex This ifdef seems consistent with other nested locking primitives, but its kind of confusing. The Kconfig.debug for DEBUG_LOCK_ALLOC says: config DEBUG_LOCK_ALLOC bool "Lock debugging: detect incorrect freeing of live locks" [...] help This feature will check whether any held lock (spinlock, rwlock, mutex or rwsem) is incorrectly freed by the kernel, via any of the memory-freeing routines (kfree(), kmem_cache_free(), free_pages(), vfree(), etc.), whether a live lock is incorrectly reinitialized via spin_lock_init()/mutex_init()/etc., or whether there is any lock held during task exit. Shouldn't this ideally be ifdef'd under PROVE_LOCKING for this and other locking primitives? Any idea what's the reason? I know PROVE_LOCKING selects DEBUG_LOCK_ALLOC but still.. thanks! - Joel
Re: [PATCH v2] Revert "alx: remove WoL support"
Hi all, Just inform you a news reported by a user who confirmed the wake up issue can't be reproduce by the new kernel. https://bugzilla.kernel.org/show_bug.cgi?id=61651#c126 Guillaume de Jabrun 2018-05-27 15:12:54 UTC I am using this patch for a long time. I was experiencing the "wake up twice" bug, but with recent kernel version, I don't have this issue anymore. I can't tell which version actually fix the bug (I don't remember..). Best regards, AceLan Kao. 2018-05-21 11:18 GMT+08:00 David Miller: > From: AceLan Kao > Date: Mon, 21 May 2018 11:14:00 +0800 > >> We are willing to fix the issue, but we don't have a machine to >> reproduce it, and the WoL feature has been removed 5 years ago, it's >> hard to find those buggy machines. > > Have you bothered to ask the person who did the revert? > >> WoL is a feature that is only used by a very small group of people, >> and the wake up issue looks like only happens on some >> platforms. Which means only small part of the group of people are >> affected. > > One of those people was the wireless networking stack maintainer. > >> So, it's not a serious issue worth to remove it from alx driver. > > I disagree. > > You must fix the regression solved by the revert, before adding > WoL support back to the driver. > > I'm not going to say this again. > > Thank you. >
Re: [PATCH v2] Revert "alx: remove WoL support"
Hi all, Just inform you a news reported by a user who confirmed the wake up issue can't be reproduce by the new kernel. https://bugzilla.kernel.org/show_bug.cgi?id=61651#c126 Guillaume de Jabrun 2018-05-27 15:12:54 UTC I am using this patch for a long time. I was experiencing the "wake up twice" bug, but with recent kernel version, I don't have this issue anymore. I can't tell which version actually fix the bug (I don't remember..). Best regards, AceLan Kao. 2018-05-21 11:18 GMT+08:00 David Miller : > From: AceLan Kao > Date: Mon, 21 May 2018 11:14:00 +0800 > >> We are willing to fix the issue, but we don't have a machine to >> reproduce it, and the WoL feature has been removed 5 years ago, it's >> hard to find those buggy machines. > > Have you bothered to ask the person who did the revert? > >> WoL is a feature that is only used by a very small group of people, >> and the wake up issue looks like only happens on some >> platforms. Which means only small part of the group of people are >> affected. > > One of those people was the wireless networking stack maintainer. > >> So, it's not a serious issue worth to remove it from alx driver. > > I disagree. > > You must fix the regression solved by the revert, before adding > WoL support back to the driver. > > I'm not going to say this again. > > Thank you. >
RE: [PATCH 05/11] PM / devfreq: governors: Return device frequency limits instead of user limits
>The performance, powersave and simpleondemand governors can return >df->min/max_freq, which are the user defined frequency limits. >update_devfreq() already takes care of adjusting the target frequency >with the user limits if necessary, therefore we can return >df->scaling_min/max_freq instead, which is the min/max frequency >supported by the device at a given time (depending on the >enabled/disabled OPPs) > >Signed-off-by: Matthias Kaehlcke>--- > drivers/devfreq/governor_performance.c| 2 +- > drivers/devfreq/governor_powersave.c | 2 +- > drivers/devfreq/governor_simpleondemand.c | 6 +++--- > 3 files changed, 5 insertions(+), 5 deletions(-) > Actually, even scaling_max_freq and scaling_min_freq are covered centerally at devfreq.c:update_devfreq(); Wouldn't it be sufficient to return UINT_MAX for performance and return UINT_MIN (0) for powersave, if the purpose is to remove redundancy? In the same sense, we may return UINT_MAX for freq-increasing case for simpleondemand as well, because they are filtered centrally anyway. (This commit might be better merged to 4/11 in that case as well.) Cheers, MyungJoo
RE: [PATCH 05/11] PM / devfreq: governors: Return device frequency limits instead of user limits
>The performance, powersave and simpleondemand governors can return >df->min/max_freq, which are the user defined frequency limits. >update_devfreq() already takes care of adjusting the target frequency >with the user limits if necessary, therefore we can return >df->scaling_min/max_freq instead, which is the min/max frequency >supported by the device at a given time (depending on the >enabled/disabled OPPs) > >Signed-off-by: Matthias Kaehlcke >--- > drivers/devfreq/governor_performance.c| 2 +- > drivers/devfreq/governor_powersave.c | 2 +- > drivers/devfreq/governor_simpleondemand.c | 6 +++--- > 3 files changed, 5 insertions(+), 5 deletions(-) > Actually, even scaling_max_freq and scaling_min_freq are covered centerally at devfreq.c:update_devfreq(); Wouldn't it be sufficient to return UINT_MAX for performance and return UINT_MIN (0) for powersave, if the purpose is to remove redundancy? In the same sense, we may return UINT_MAX for freq-increasing case for simpleondemand as well, because they are filtered centrally anyway. (This commit might be better merged to 4/11 in that case as well.) Cheers, MyungJoo
Re: [PATCH v14 1/2] cpufreq: Add Kryo CPU scaling driver
On 25-05-18, 15:41, Ilia Lin wrote: > In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, > the CPU frequency subset and voltage value of each OPP varies > based on the silicon variant in use. Qualcomm Process Voltage Scaling Tables > defines the voltage and frequency value based on the msm-id in SMEM > and speedbin blown in the efuse combination. > The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC > to provide the OPP framework with required information. > This is used to determine the voltage and frequency value for each OPP of > operating-points-v2 table when it is parsed by the OPP framework. > > Signed-off-by: Ilia Lin> --- > MAINTAINERS | 7 ++ > drivers/cpufreq/Kconfig.arm | 10 ++ > drivers/cpufreq/Makefile | 1 + > drivers/cpufreq/cpufreq-dt-platdev.c | 3 + > drivers/cpufreq/qcom-cpufreq-kryo.c | 212 > +++ > 5 files changed, 233 insertions(+) > create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c Acked-by: Viresh Kumar -- viresh
Re: [PATCH v14 1/2] cpufreq: Add Kryo CPU scaling driver
On 25-05-18, 15:41, Ilia Lin wrote: > In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, > the CPU frequency subset and voltage value of each OPP varies > based on the silicon variant in use. Qualcomm Process Voltage Scaling Tables > defines the voltage and frequency value based on the msm-id in SMEM > and speedbin blown in the efuse combination. > The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC > to provide the OPP framework with required information. > This is used to determine the voltage and frequency value for each OPP of > operating-points-v2 table when it is parsed by the OPP framework. > > Signed-off-by: Ilia Lin > --- > MAINTAINERS | 7 ++ > drivers/cpufreq/Kconfig.arm | 10 ++ > drivers/cpufreq/Makefile | 1 + > drivers/cpufreq/cpufreq-dt-platdev.c | 3 + > drivers/cpufreq/qcom-cpufreq-kryo.c | 212 > +++ > 5 files changed, 233 insertions(+) > create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c Acked-by: Viresh Kumar -- viresh
RE: [PATCH 04/11] PM / devfreq: Remove redundant frequency adjustment from governors
> The userspace and simpleondemand governor determine a target frequency and > then adjust it according to the df->min/max_freq limits that might have > been set by user space. This adjustment is redundant, it is done in > update_devfreq() for any governor, right after returning from > governor->get_target_freq(). > > Signed-off-by: Matthias Kaehlcke> --- > drivers/devfreq/governor_simpleondemand.c | 5 - > drivers/devfreq/governor_userspace.c | 16 > 2 files changed, 4 insertions(+), 17 deletions(-) > Yes, indeed. Governors are no longer required to be aware of min/max freq. Acked-by: MyungJoo Ham
RE: [PATCH 04/11] PM / devfreq: Remove redundant frequency adjustment from governors
> The userspace and simpleondemand governor determine a target frequency and > then adjust it according to the df->min/max_freq limits that might have > been set by user space. This adjustment is redundant, it is done in > update_devfreq() for any governor, right after returning from > governor->get_target_freq(). > > Signed-off-by: Matthias Kaehlcke > --- > drivers/devfreq/governor_simpleondemand.c | 5 - > drivers/devfreq/governor_userspace.c | 16 > 2 files changed, 4 insertions(+), 17 deletions(-) > Yes, indeed. Governors are no longer required to be aware of min/max freq. Acked-by: MyungJoo Ham
RE: [PATCH 03/11] PM / devfreq: Remove check for df->max_freq == 0 from governors
>Commit "PM / devfreq: Fix handling of min/max_freq == 0" ensures that >df->max_freq is not 0, remove unnecessary checks. > >Signed-off-by: Matthias Kaehlcke>--- > drivers/devfreq/governor_performance.c| 5 + > drivers/devfreq/governor_simpleondemand.c | 7 +++ > 2 files changed, 4 insertions(+), 8 deletions(-) > >diff --git a/drivers/devfreq/governor_performance.c >b/drivers/devfreq/governor_performance.c >index 4d23ecfbd948..1c990cb45098 100644 > Thanks! Acked-by: MyungJoo Ham
RE: [PATCH 03/11] PM / devfreq: Remove check for df->max_freq == 0 from governors
>Commit "PM / devfreq: Fix handling of min/max_freq == 0" ensures that >df->max_freq is not 0, remove unnecessary checks. > >Signed-off-by: Matthias Kaehlcke >--- > drivers/devfreq/governor_performance.c| 5 + > drivers/devfreq/governor_simpleondemand.c | 7 +++ > 2 files changed, 4 insertions(+), 8 deletions(-) > >diff --git a/drivers/devfreq/governor_performance.c >b/drivers/devfreq/governor_performance.c >index 4d23ecfbd948..1c990cb45098 100644 > Thanks! Acked-by: MyungJoo Ham
Re: [PATCH] mm-kasan-dont-vfree-nonexistent-vm_area-fix
On Mon, 2018-05-28 at 12:13 +0800, Ian Kent wrote: > On Fri, 2018-05-25 at 20:48 -0700, Andrew Morton wrote: > > On Sat, 26 May 2018 11:31:35 +0800 kbuild test robotwrote: > > > > > Hi Andrey, > > > > > > I love your patch! Yet something to improve: > > > > > > [auto build test ERROR on mmotm/master] > > > [cannot apply to v4.17-rc6] > > > [if your patch is applied to the wrong git tree, please drop us a note to > > > help improve the system] > > > > > > url:https://github.com/0day-ci/linux/commits/Andrey-Ryabinin/mm-kasan- > > > do > > > nt-vfree-nonexistent-vm_area-fix/20180526-093255 > > > base: git://git.cmpxchg.org/linux-mmotm.git master > > > config: sparc-allyesconfig (attached as .config) > > > compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 > > > reproduce: > > > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin > > > /m > > > ake.cross -O ~/bin/make.cross > > > chmod +x ~/bin/make.cross > > > # save the attached .config to linux build tree > > > make.cross ARCH=sparc > > > > > > All errors (new ones prefixed by >>): > > > > > >fs/autofs/inode.o: In function `autofs_new_ino': > > >inode.c:(.text+0x220): multiple definition of `autofs_new_ino' > > >fs/autofs/inode.o:inode.c:(.text+0x220): first defined here > > >fs/autofs/inode.o: In function `autofs_clean_ino': > > >inode.c:(.text+0x280): multiple definition of `autofs_clean_ino' > > >fs/autofs/inode.o:inode.c:(.text+0x280): first defined here > > > > There's bot breakage here - clearly that patch didn't cause this error. > > > > Ian, this autofs glitch may still not be fixed. > > Yes, autofs-make-autofs4-Kconfig-depend-on-AUTOFS_FS.patch should have > fixed that. > > I tied a bunch of .config combinations and I was unable to find any that > lead to both CONFIG_AUTOFS_FS and CONFIG_AUTOFS4_FS being defined. Oh, autofs-make-autofs4-Kconfig-depend-on-AUTOFS_FS.patch was sent as a follow up patch which means it's still possible to have both CONFIG_AUTOFS_FS and CONFIG_AUTOFS4_FS set between autofs-create-autofs-Kconfig-and-Makefile.patch and the above patch. Perhaps all that's needed is to fold the follow up patch into autofs-create-autofs-Kconfig-and-Makefile.patch to close that possibility. I'll check that can be done without problem. > > I must be missing something else, I'll investigate. And I'll check if there's anything else I'm missing. Sorry for the inconvenience. > > Ian
Re: [PATCH] mm-kasan-dont-vfree-nonexistent-vm_area-fix
On Mon, 2018-05-28 at 12:13 +0800, Ian Kent wrote: > On Fri, 2018-05-25 at 20:48 -0700, Andrew Morton wrote: > > On Sat, 26 May 2018 11:31:35 +0800 kbuild test robot wrote: > > > > > Hi Andrey, > > > > > > I love your patch! Yet something to improve: > > > > > > [auto build test ERROR on mmotm/master] > > > [cannot apply to v4.17-rc6] > > > [if your patch is applied to the wrong git tree, please drop us a note to > > > help improve the system] > > > > > > url:https://github.com/0day-ci/linux/commits/Andrey-Ryabinin/mm-kasan- > > > do > > > nt-vfree-nonexistent-vm_area-fix/20180526-093255 > > > base: git://git.cmpxchg.org/linux-mmotm.git master > > > config: sparc-allyesconfig (attached as .config) > > > compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 > > > reproduce: > > > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin > > > /m > > > ake.cross -O ~/bin/make.cross > > > chmod +x ~/bin/make.cross > > > # save the attached .config to linux build tree > > > make.cross ARCH=sparc > > > > > > All errors (new ones prefixed by >>): > > > > > >fs/autofs/inode.o: In function `autofs_new_ino': > > >inode.c:(.text+0x220): multiple definition of `autofs_new_ino' > > >fs/autofs/inode.o:inode.c:(.text+0x220): first defined here > > >fs/autofs/inode.o: In function `autofs_clean_ino': > > >inode.c:(.text+0x280): multiple definition of `autofs_clean_ino' > > >fs/autofs/inode.o:inode.c:(.text+0x280): first defined here > > > > There's bot breakage here - clearly that patch didn't cause this error. > > > > Ian, this autofs glitch may still not be fixed. > > Yes, autofs-make-autofs4-Kconfig-depend-on-AUTOFS_FS.patch should have > fixed that. > > I tied a bunch of .config combinations and I was unable to find any that > lead to both CONFIG_AUTOFS_FS and CONFIG_AUTOFS4_FS being defined. Oh, autofs-make-autofs4-Kconfig-depend-on-AUTOFS_FS.patch was sent as a follow up patch which means it's still possible to have both CONFIG_AUTOFS_FS and CONFIG_AUTOFS4_FS set between autofs-create-autofs-Kconfig-and-Makefile.patch and the above patch. Perhaps all that's needed is to fold the follow up patch into autofs-create-autofs-Kconfig-and-Makefile.patch to close that possibility. I'll check that can be done without problem. > > I must be missing something else, I'll investigate. And I'll check if there's anything else I'm missing. Sorry for the inconvenience. > > Ian
Re: [PATCH] mm-kasan-dont-vfree-nonexistent-vm_area-fix
On Fri, 2018-05-25 at 20:48 -0700, Andrew Morton wrote: > On Sat, 26 May 2018 11:31:35 +0800 kbuild test robotwrote: > > > Hi Andrey, > > > > I love your patch! Yet something to improve: > > > > [auto build test ERROR on mmotm/master] > > [cannot apply to v4.17-rc6] > > [if your patch is applied to the wrong git tree, please drop us a note to > > help improve the system] > > > > url:https://github.com/0day-ci/linux/commits/Andrey-Ryabinin/mm-kasan-do > > nt-vfree-nonexistent-vm_area-fix/20180526-093255 > > base: git://git.cmpxchg.org/linux-mmotm.git master > > config: sparc-allyesconfig (attached as .config) > > compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 > > reproduce: > > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/m > > ake.cross -O ~/bin/make.cross > > chmod +x ~/bin/make.cross > > # save the attached .config to linux build tree > > make.cross ARCH=sparc > > > > All errors (new ones prefixed by >>): > > > >fs/autofs/inode.o: In function `autofs_new_ino': > >inode.c:(.text+0x220): multiple definition of `autofs_new_ino' > >fs/autofs/inode.o:inode.c:(.text+0x220): first defined here > >fs/autofs/inode.o: In function `autofs_clean_ino': > >inode.c:(.text+0x280): multiple definition of `autofs_clean_ino' > >fs/autofs/inode.o:inode.c:(.text+0x280): first defined here > > There's bot breakage here - clearly that patch didn't cause this error. > > Ian, this autofs glitch may still not be fixed. Yes, autofs-make-autofs4-Kconfig-depend-on-AUTOFS_FS.patch should have fixed that. I tied a bunch of .config combinations and I was unable to find any that lead to both CONFIG_AUTOFS_FS and CONFIG_AUTOFS4_FS being defined. I must be missing something else, I'll investigate. Ian
Re: [PATCH] mm-kasan-dont-vfree-nonexistent-vm_area-fix
On Fri, 2018-05-25 at 20:48 -0700, Andrew Morton wrote: > On Sat, 26 May 2018 11:31:35 +0800 kbuild test robot wrote: > > > Hi Andrey, > > > > I love your patch! Yet something to improve: > > > > [auto build test ERROR on mmotm/master] > > [cannot apply to v4.17-rc6] > > [if your patch is applied to the wrong git tree, please drop us a note to > > help improve the system] > > > > url:https://github.com/0day-ci/linux/commits/Andrey-Ryabinin/mm-kasan-do > > nt-vfree-nonexistent-vm_area-fix/20180526-093255 > > base: git://git.cmpxchg.org/linux-mmotm.git master > > config: sparc-allyesconfig (attached as .config) > > compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 > > reproduce: > > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/m > > ake.cross -O ~/bin/make.cross > > chmod +x ~/bin/make.cross > > # save the attached .config to linux build tree > > make.cross ARCH=sparc > > > > All errors (new ones prefixed by >>): > > > >fs/autofs/inode.o: In function `autofs_new_ino': > >inode.c:(.text+0x220): multiple definition of `autofs_new_ino' > >fs/autofs/inode.o:inode.c:(.text+0x220): first defined here > >fs/autofs/inode.o: In function `autofs_clean_ino': > >inode.c:(.text+0x280): multiple definition of `autofs_clean_ino' > >fs/autofs/inode.o:inode.c:(.text+0x280): first defined here > > There's bot breakage here - clearly that patch didn't cause this error. > > Ian, this autofs glitch may still not be fixed. Yes, autofs-make-autofs4-Kconfig-depend-on-AUTOFS_FS.patch should have fixed that. I tied a bunch of .config combinations and I was unable to find any that lead to both CONFIG_AUTOFS_FS and CONFIG_AUTOFS4_FS being defined. I must be missing something else, I'll investigate. Ian
RE: [PATCH 02/11] PM / devfreq: Fix handling of min/max_freq == 0
> Commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when adding the > devfreq device") initializes df->min/max_freq with the min/max OPP when > the device is added. Later commit f1d981eaecf8 ("PM / devfreq: Use the > available min/max frequency") adds df->scaling_min/max_freq and the > following to the frequency adjustment code: > > max_freq = MIN(devfreq->scaling_max_freq, devfreq->max_freq); > > With the current handling of min/max_freq this is incorrect: > > Even though df->max_freq is now initialized to a value != 0 user space > can still set it to 0, in this case max_freq would be 0 instead of > df->scaling_max_freq as intended. In consequence the frequency adjustment > is not performed: > > if (max_freq && freq > max_freq) { > freq = max_freq; > > To fix this set df->min/max freq to the min/max OPP in max/max_freq_store, > when the user passes a value of 0. This also prevents df->max_freq from > being set below the min OPP when df->min_freq is 0, and similar for > min_freq. Since it is now guaranteed that df->min/max_freq can't be 0 the > checks for this case can be removed. > > Fixes: f1d981eaecf8 ("PM / devfreq: Use the available min/max frequency") > Signed-off-by: Matthias Kaehlcke> --- > drivers/devfreq/devfreq.c | 30 ++ > 1 file changed, 18 insertions(+), 12 deletions(-) Thanks a lot! Nice Catch. Acked-by: MyungJoo Ham Cheers, MyungJoo.
RE: [PATCH 02/11] PM / devfreq: Fix handling of min/max_freq == 0
> Commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when adding the > devfreq device") initializes df->min/max_freq with the min/max OPP when > the device is added. Later commit f1d981eaecf8 ("PM / devfreq: Use the > available min/max frequency") adds df->scaling_min/max_freq and the > following to the frequency adjustment code: > > max_freq = MIN(devfreq->scaling_max_freq, devfreq->max_freq); > > With the current handling of min/max_freq this is incorrect: > > Even though df->max_freq is now initialized to a value != 0 user space > can still set it to 0, in this case max_freq would be 0 instead of > df->scaling_max_freq as intended. In consequence the frequency adjustment > is not performed: > > if (max_freq && freq > max_freq) { > freq = max_freq; > > To fix this set df->min/max freq to the min/max OPP in max/max_freq_store, > when the user passes a value of 0. This also prevents df->max_freq from > being set below the min OPP when df->min_freq is 0, and similar for > min_freq. Since it is now guaranteed that df->min/max_freq can't be 0 the > checks for this case can be removed. > > Fixes: f1d981eaecf8 ("PM / devfreq: Use the available min/max frequency") > Signed-off-by: Matthias Kaehlcke > --- > drivers/devfreq/devfreq.c | 30 ++ > 1 file changed, 18 insertions(+), 12 deletions(-) Thanks a lot! Nice Catch. Acked-by: MyungJoo Ham Cheers, MyungJoo.
[PATCH 1/3] arm64:add missing CONFIG_STRICT_KERNEL_RWX for mark_rodata_ro
Signed-off-by: nixiaoming--- arch/arm64/mm/mmu.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index 2dbb2c9..849f326 100644 --- a/arch/arm64/mm/mmu.c +++ b/arch/arm64/mm/mmu.c @@ -491,6 +491,7 @@ static void __init map_mem(pgd_t *pgdp) #endif } +#ifdef CONFIG_STRICT_KERNEL_RWX void mark_rodata_ro(void) { unsigned long section_size; @@ -505,6 +506,7 @@ void mark_rodata_ro(void) debug_checkwx(); } +#endif static void __init map_kernel_segment(pgd_t *pgdp, void *va_start, void *va_end, pgprot_t prot, struct vm_struct *vma, -- 2.10.1
[PATCH 2/3] x86:add missing CONFIG_STRICT_KERNEL_RWX for mark_rodata_ro
Signed-off-by: nixiaoming--- arch/x86/mm/init_32.c | 2 ++ arch/x86/mm/init_64.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c index c893c6a..121c567 100644 --- a/arch/x86/mm/init_32.c +++ b/arch/x86/mm/init_32.c @@ -920,6 +920,7 @@ static void mark_nxdata_nx(void) set_pages_nx(virt_to_page(start), size >> PAGE_SHIFT); } +#ifdef CONFIG_STRICT_KERNEL_RWX void mark_rodata_ro(void) { unsigned long start = PFN_ALIGN(_text); @@ -957,3 +958,4 @@ void mark_rodata_ro(void) if (__supported_pte_mask & _PAGE_NX) debug_checkwx(); } +#endif /*end of CONFIG_STRICT_KERNEL_RWX*/ diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c index 0a40060..1b7a1a7 100644 --- a/arch/x86/mm/init_64.c +++ b/arch/x86/mm/init_64.c @@ -1245,6 +1245,7 @@ void set_kernel_text_ro(void) set_memory_ro(start, (end - start) >> PAGE_SHIFT); } +#ifdef CONFIG_STRICT_KERNEL_RWX void mark_rodata_ro(void) { unsigned long start = PFN_ALIGN(_text); @@ -1298,6 +1299,7 @@ void mark_rodata_ro(void) */ pti_clone_kernel_text(); } +#endif int kern_addr_valid(unsigned long addr) { -- 2.10.1
[PATCH 1/3] arm64:add missing CONFIG_STRICT_KERNEL_RWX for mark_rodata_ro
Signed-off-by: nixiaoming --- arch/arm64/mm/mmu.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index 2dbb2c9..849f326 100644 --- a/arch/arm64/mm/mmu.c +++ b/arch/arm64/mm/mmu.c @@ -491,6 +491,7 @@ static void __init map_mem(pgd_t *pgdp) #endif } +#ifdef CONFIG_STRICT_KERNEL_RWX void mark_rodata_ro(void) { unsigned long section_size; @@ -505,6 +506,7 @@ void mark_rodata_ro(void) debug_checkwx(); } +#endif static void __init map_kernel_segment(pgd_t *pgdp, void *va_start, void *va_end, pgprot_t prot, struct vm_struct *vma, -- 2.10.1
[PATCH 2/3] x86:add missing CONFIG_STRICT_KERNEL_RWX for mark_rodata_ro
Signed-off-by: nixiaoming --- arch/x86/mm/init_32.c | 2 ++ arch/x86/mm/init_64.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c index c893c6a..121c567 100644 --- a/arch/x86/mm/init_32.c +++ b/arch/x86/mm/init_32.c @@ -920,6 +920,7 @@ static void mark_nxdata_nx(void) set_pages_nx(virt_to_page(start), size >> PAGE_SHIFT); } +#ifdef CONFIG_STRICT_KERNEL_RWX void mark_rodata_ro(void) { unsigned long start = PFN_ALIGN(_text); @@ -957,3 +958,4 @@ void mark_rodata_ro(void) if (__supported_pte_mask & _PAGE_NX) debug_checkwx(); } +#endif /*end of CONFIG_STRICT_KERNEL_RWX*/ diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c index 0a40060..1b7a1a7 100644 --- a/arch/x86/mm/init_64.c +++ b/arch/x86/mm/init_64.c @@ -1245,6 +1245,7 @@ void set_kernel_text_ro(void) set_memory_ro(start, (end - start) >> PAGE_SHIFT); } +#ifdef CONFIG_STRICT_KERNEL_RWX void mark_rodata_ro(void) { unsigned long start = PFN_ALIGN(_text); @@ -1298,6 +1299,7 @@ void mark_rodata_ro(void) */ pti_clone_kernel_text(); } +#endif int kern_addr_valid(unsigned long addr) { -- 2.10.1
[PATCH 3/3] s390:add missing CONFIG_STRICT_KERNEL_RWX for mark_rodata_ro
Signed-off-by: nixiaoming--- arch/s390/mm/init.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c index 3fa3e53..a96fc3f 100644 --- a/arch/s390/mm/init.c +++ b/arch/s390/mm/init.c @@ -116,6 +116,7 @@ void __init paging_init(void) free_area_init_nodes(max_zone_pfns); } +#ifdef CONFIG_STRICT_KERNEL_RWX void mark_rodata_ro(void) { unsigned long size = __end_ro_after_init - __start_ro_after_init; @@ -123,6 +124,7 @@ void mark_rodata_ro(void) set_memory_ro((unsigned long)__start_ro_after_init, size >> PAGE_SHIFT); pr_info("Write protected read-only-after-init data: %luk\n", size >> 10); } +#endif void __init mem_init(void) { -- 2.10.1
[PATCH 3/3] s390:add missing CONFIG_STRICT_KERNEL_RWX for mark_rodata_ro
Signed-off-by: nixiaoming --- arch/s390/mm/init.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c index 3fa3e53..a96fc3f 100644 --- a/arch/s390/mm/init.c +++ b/arch/s390/mm/init.c @@ -116,6 +116,7 @@ void __init paging_init(void) free_area_init_nodes(max_zone_pfns); } +#ifdef CONFIG_STRICT_KERNEL_RWX void mark_rodata_ro(void) { unsigned long size = __end_ro_after_init - __start_ro_after_init; @@ -123,6 +124,7 @@ void mark_rodata_ro(void) set_memory_ro((unsigned long)__start_ro_after_init, size >> PAGE_SHIFT); pr_info("Write protected read-only-after-init data: %luk\n", size >> 10); } +#endif void __init mem_init(void) { -- 2.10.1
Re: [PATCH] perf test 39 (Session topology) dumps core on s390
Hi Thomas, On 05/24/2018 07:26 PM, Thomas Richter wrote: > @@ -95,7 +98,7 @@ int test__session_topology(struct test *test > __maybe_unused, int subtest __maybe > { > char path[PATH_MAX]; > struct cpu_map *map; > - int ret = -1; > + int ret; This is failing for me: tests/topology.c: In function ‘test__session_topology’: tests/topology.c:101:6: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized] int ret; ^ cc1: all warnings being treated as errors CC util/intlist.o mv: cannot stat 'tests/.topology.o.tmp': No such file or directory /home/ravi/Workspace/linux/tools/build/Makefile.build:96: recipe for target 'tests/topology.o' failed make[4]: *** [tests/topology.o] Error 1 make[4]: *** Waiting for unfinished jobs Thanks, Ravi
Re: [PATCH] perf test 39 (Session topology) dumps core on s390
Hi Thomas, On 05/24/2018 07:26 PM, Thomas Richter wrote: > @@ -95,7 +98,7 @@ int test__session_topology(struct test *test > __maybe_unused, int subtest __maybe > { > char path[PATH_MAX]; > struct cpu_map *map; > - int ret = -1; > + int ret; This is failing for me: tests/topology.c: In function ‘test__session_topology’: tests/topology.c:101:6: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized] int ret; ^ cc1: all warnings being treated as errors CC util/intlist.o mv: cannot stat 'tests/.topology.o.tmp': No such file or directory /home/ravi/Workspace/linux/tools/build/Makefile.build:96: recipe for target 'tests/topology.o' failed make[4]: *** [tests/topology.o] Error 1 make[4]: *** Waiting for unfinished jobs Thanks, Ravi
[v2] ASoC: AMD: make channel 1 dma as circular
channel 1: SYSMEM<->ACP channel 2: ACP<->I2S Instead of waiting on period interrupt of ch 2 and then starting dma on ch1, we make ch1 dma as circular. This removes dependency of period granularity on hw pointer. Signed-off-by: Akshu AgrawalReviewed-by: Daniel Kurtz Tested-by: Daniel Kurtz --- v2: Fixed kbuild error sound/soc/amd/acp-pcm-dma.c | 74 ++--- 1 file changed, 10 insertions(+), 64 deletions(-) diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c index ac32dea..7720384 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -337,8 +337,7 @@ static void config_acp_dma(void __iomem *acp_mmio, } /* Start a given DMA channel transfer */ -static void acp_dma_start(void __iomem *acp_mmio, - u16 ch_num, bool is_circular) +static void acp_dma_start(void __iomem *acp_mmio, u16 ch_num) { u32 dma_ctrl; @@ -369,11 +368,8 @@ static void acp_dma_start(void __iomem *acp_mmio, break; } - /* enable for ACP SRAM to/from I2S DMA channel */ - if (is_circular == true) - dma_ctrl |= ACP_DMA_CNTL_0__Circular_DMA_En_MASK; - else - dma_ctrl &= ~ACP_DMA_CNTL_0__Circular_DMA_En_MASK; + /* circular for both DMA channel */ + dma_ctrl |= ACP_DMA_CNTL_0__Circular_DMA_En_MASK; acp_reg_write(dma_ctrl, acp_mmio, mmACP_DMA_CNTL_0 + ch_num); } @@ -617,7 +613,6 @@ static int acp_deinit(void __iomem *acp_mmio) /* ACP DMA irq handler routine for playback, capture usecases */ static irqreturn_t dma_irq_handler(int irq, void *arg) { - u16 dscr_idx; u32 intr_flag, ext_intr_status; struct audio_drv_data *irq_data; void __iomem *acp_mmio; @@ -634,33 +629,13 @@ static irqreturn_t dma_irq_handler(int irq, void *arg) if ((intr_flag & BIT(ACP_TO_I2S_DMA_CH_NUM)) != 0) { valid_irq = true; - if (acp_reg_read(acp_mmio, mmACP_DMA_CUR_DSCR_13) == - PLAYBACK_START_DMA_DESCR_CH13) - dscr_idx = PLAYBACK_END_DMA_DESCR_CH12; - else - dscr_idx = PLAYBACK_START_DMA_DESCR_CH12; - config_acp_dma_channel(acp_mmio, SYSRAM_TO_ACP_CH_NUM, dscr_idx, - 1, 0); - acp_dma_start(acp_mmio, SYSRAM_TO_ACP_CH_NUM, false); - snd_pcm_period_elapsed(irq_data->play_i2ssp_stream); - acp_reg_write((intr_flag & BIT(ACP_TO_I2S_DMA_CH_NUM)) << 16, acp_mmio, mmACP_EXTERNAL_INTR_STAT); } if ((intr_flag & BIT(ACP_TO_I2S_DMA_BT_INSTANCE_CH_NUM)) != 0) { valid_irq = true; - if (acp_reg_read(acp_mmio, mmACP_DMA_CUR_DSCR_9) == - PLAYBACK_START_DMA_DESCR_CH9) - dscr_idx = PLAYBACK_END_DMA_DESCR_CH8; - else - dscr_idx = PLAYBACK_START_DMA_DESCR_CH8; - config_acp_dma_channel(acp_mmio, - SYSRAM_TO_ACP_BT_INSTANCE_CH_NUM, - dscr_idx, 1, 0); - acp_dma_start(acp_mmio, SYSRAM_TO_ACP_BT_INSTANCE_CH_NUM, - false); snd_pcm_period_elapsed(irq_data->play_i2sbt_stream); acp_reg_write((intr_flag & BIT(ACP_TO_I2S_DMA_BT_INSTANCE_CH_NUM)) << 16, @@ -669,38 +644,20 @@ static irqreturn_t dma_irq_handler(int irq, void *arg) if ((intr_flag & BIT(I2S_TO_ACP_DMA_CH_NUM)) != 0) { valid_irq = true; - if (acp_reg_read(acp_mmio, mmACP_DMA_CUR_DSCR_15) == - CAPTURE_START_DMA_DESCR_CH15) - dscr_idx = CAPTURE_END_DMA_DESCR_CH14; - else - dscr_idx = CAPTURE_START_DMA_DESCR_CH14; - config_acp_dma_channel(acp_mmio, ACP_TO_SYSRAM_CH_NUM, dscr_idx, - 1, 0); - acp_dma_start(acp_mmio, ACP_TO_SYSRAM_CH_NUM, false); - + snd_pcm_period_elapsed(irq_data->capture_i2ssp_stream); acp_reg_write((intr_flag & BIT(I2S_TO_ACP_DMA_CH_NUM)) << 16, acp_mmio, mmACP_EXTERNAL_INTR_STAT); } if ((intr_flag & BIT(ACP_TO_SYSRAM_CH_NUM)) != 0) { valid_irq = true; - snd_pcm_period_elapsed(irq_data->capture_i2ssp_stream); acp_reg_write((intr_flag & BIT(ACP_TO_SYSRAM_CH_NUM)) << 16, acp_mmio, mmACP_EXTERNAL_INTR_STAT); } if ((intr_flag & BIT(I2S_TO_ACP_DMA_BT_INSTANCE_CH_NUM)) != 0) { valid_irq = true; - if (acp_reg_read(acp_mmio, mmACP_DMA_CUR_DSCR_11) == -
[v2] ASoC: AMD: make channel 1 dma as circular
channel 1: SYSMEM<->ACP channel 2: ACP<->I2S Instead of waiting on period interrupt of ch 2 and then starting dma on ch1, we make ch1 dma as circular. This removes dependency of period granularity on hw pointer. Signed-off-by: Akshu Agrawal Reviewed-by: Daniel Kurtz Tested-by: Daniel Kurtz --- v2: Fixed kbuild error sound/soc/amd/acp-pcm-dma.c | 74 ++--- 1 file changed, 10 insertions(+), 64 deletions(-) diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c index ac32dea..7720384 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -337,8 +337,7 @@ static void config_acp_dma(void __iomem *acp_mmio, } /* Start a given DMA channel transfer */ -static void acp_dma_start(void __iomem *acp_mmio, - u16 ch_num, bool is_circular) +static void acp_dma_start(void __iomem *acp_mmio, u16 ch_num) { u32 dma_ctrl; @@ -369,11 +368,8 @@ static void acp_dma_start(void __iomem *acp_mmio, break; } - /* enable for ACP SRAM to/from I2S DMA channel */ - if (is_circular == true) - dma_ctrl |= ACP_DMA_CNTL_0__Circular_DMA_En_MASK; - else - dma_ctrl &= ~ACP_DMA_CNTL_0__Circular_DMA_En_MASK; + /* circular for both DMA channel */ + dma_ctrl |= ACP_DMA_CNTL_0__Circular_DMA_En_MASK; acp_reg_write(dma_ctrl, acp_mmio, mmACP_DMA_CNTL_0 + ch_num); } @@ -617,7 +613,6 @@ static int acp_deinit(void __iomem *acp_mmio) /* ACP DMA irq handler routine for playback, capture usecases */ static irqreturn_t dma_irq_handler(int irq, void *arg) { - u16 dscr_idx; u32 intr_flag, ext_intr_status; struct audio_drv_data *irq_data; void __iomem *acp_mmio; @@ -634,33 +629,13 @@ static irqreturn_t dma_irq_handler(int irq, void *arg) if ((intr_flag & BIT(ACP_TO_I2S_DMA_CH_NUM)) != 0) { valid_irq = true; - if (acp_reg_read(acp_mmio, mmACP_DMA_CUR_DSCR_13) == - PLAYBACK_START_DMA_DESCR_CH13) - dscr_idx = PLAYBACK_END_DMA_DESCR_CH12; - else - dscr_idx = PLAYBACK_START_DMA_DESCR_CH12; - config_acp_dma_channel(acp_mmio, SYSRAM_TO_ACP_CH_NUM, dscr_idx, - 1, 0); - acp_dma_start(acp_mmio, SYSRAM_TO_ACP_CH_NUM, false); - snd_pcm_period_elapsed(irq_data->play_i2ssp_stream); - acp_reg_write((intr_flag & BIT(ACP_TO_I2S_DMA_CH_NUM)) << 16, acp_mmio, mmACP_EXTERNAL_INTR_STAT); } if ((intr_flag & BIT(ACP_TO_I2S_DMA_BT_INSTANCE_CH_NUM)) != 0) { valid_irq = true; - if (acp_reg_read(acp_mmio, mmACP_DMA_CUR_DSCR_9) == - PLAYBACK_START_DMA_DESCR_CH9) - dscr_idx = PLAYBACK_END_DMA_DESCR_CH8; - else - dscr_idx = PLAYBACK_START_DMA_DESCR_CH8; - config_acp_dma_channel(acp_mmio, - SYSRAM_TO_ACP_BT_INSTANCE_CH_NUM, - dscr_idx, 1, 0); - acp_dma_start(acp_mmio, SYSRAM_TO_ACP_BT_INSTANCE_CH_NUM, - false); snd_pcm_period_elapsed(irq_data->play_i2sbt_stream); acp_reg_write((intr_flag & BIT(ACP_TO_I2S_DMA_BT_INSTANCE_CH_NUM)) << 16, @@ -669,38 +644,20 @@ static irqreturn_t dma_irq_handler(int irq, void *arg) if ((intr_flag & BIT(I2S_TO_ACP_DMA_CH_NUM)) != 0) { valid_irq = true; - if (acp_reg_read(acp_mmio, mmACP_DMA_CUR_DSCR_15) == - CAPTURE_START_DMA_DESCR_CH15) - dscr_idx = CAPTURE_END_DMA_DESCR_CH14; - else - dscr_idx = CAPTURE_START_DMA_DESCR_CH14; - config_acp_dma_channel(acp_mmio, ACP_TO_SYSRAM_CH_NUM, dscr_idx, - 1, 0); - acp_dma_start(acp_mmio, ACP_TO_SYSRAM_CH_NUM, false); - + snd_pcm_period_elapsed(irq_data->capture_i2ssp_stream); acp_reg_write((intr_flag & BIT(I2S_TO_ACP_DMA_CH_NUM)) << 16, acp_mmio, mmACP_EXTERNAL_INTR_STAT); } if ((intr_flag & BIT(ACP_TO_SYSRAM_CH_NUM)) != 0) { valid_irq = true; - snd_pcm_period_elapsed(irq_data->capture_i2ssp_stream); acp_reg_write((intr_flag & BIT(ACP_TO_SYSRAM_CH_NUM)) << 16, acp_mmio, mmACP_EXTERNAL_INTR_STAT); } if ((intr_flag & BIT(I2S_TO_ACP_DMA_BT_INSTANCE_CH_NUM)) != 0) { valid_irq = true; - if (acp_reg_read(acp_mmio, mmACP_DMA_CUR_DSCR_11) == - CAPTURE_START_DMA_DESCR_CH11) - dscr_idx =
Re: [PATCH v2 2/5] gpio: syscon: Add gpio-syscon for rockchip
On 2018-05-24 8:18 PM, Heiko Stuebner wrote: Hi Levin, Am Donnerstag, 24. Mai 2018, 03:59:36 CEST schrieb Levin Du: Hi all, I'd like to quote reply of Robin Murphy at http://lists.infradead.org/pipermail/linux-rockchip/2018-May/020619.html I would suggest s/pin number/bit number in the associated GRF register/ here. At least in this RK3328 case there's only one pin, which isn't numbered, and if you naively considered it pin 0 of this 'bank' you'd already have the wrong number. Since we're dealing with the "random SoC-specific controls" region of the GRF as opposed to the relatively-consistent and organised pinmux parts, I don't think we should rely on any assumptions about how things are laid out. I was initially going to suggest a more specific compatible string as well, but on reflection I think the generic "rockchip,gpio-syscon" for basic "flip this single GRF bit" functionality actually is the right way to go. In the specific RK3328 GPIO_MUTE case, there look to be 4 bits in total related to this pin - the enable, value, and some pull controls (which I assume apply when the output is disabled) - if at some point in future we *did* want to start explicitly controlling the rest of them too, then would be a good time to define a separate "rockchip,rk3328-gpio-mute" binding (and probably a dedicated driver) for that specialised functionality, independently of this basic one. Shall we go the generic "rockchip,gpio-syscon" way, or the specific "rockchip,rk3328-gpio-mute" way? I prefer the former one. The property of "gpio,syscon-dev" in gpio-syscon driver should be documented. Since the gpio controller is defined in the dtsi file, which inevitably contains voodoo register addresses. But at the board level dts file, there won't be more register addresses. Past experience shows that the GRF is not really suitable for generalization, as it's more of a dumping ground where chip designers can put everything that's left over. This is especially true for GRF_SOC_CONx registers, that really only contain pretty random bits. So personally, I'd really prefer soc-specific compatibles as everywhere else, instead of trying to push stuff into the devicetree that won't hold up on future socs. On 2018-05-24 3:53 AM, Rob Herring wrote: On Wed, May 23, 2018 at 10:12 AM, Heiko Stübnerwrote: Hi Rob, Levin, sorry for being late to the party. Am Mittwoch, 23. Mai 2018, 16:43:07 CEST schrieb Rob Herring: On Tue, May 22, 2018 at 9:02 PM, Levin Du wrote: On 2018-05-23 2:02 AM, Rob Herring wrote: On Fri, May 18, 2018 at 11:52:05AM +0800, d...@t-chip.com.cn wrote: From: Levin Du Some GPIOs sit in the GRF_SOC_CON registers of Rockchip SoCs, which do not belong to the general pinctrl. Adding gpio-syscon support makes controlling regulator or LED using these special pins very easy by reusing existing drivers, such as gpio-regulator and led-gpio. Signed-off-by: Levin Du --- Changes in v2: - Rename gpio_syscon10 to gpio_mute in doc Changes in v1: - Refactured for general gpio-syscon usage for Rockchip SoCs. - Add doc rockchip,gpio-syscon.txt .../bindings/gpio/rockchip,gpio-syscon.txt | 41 ++ drivers/gpio/gpio-syscon.c | 30 2 files changed, 71 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt diff --git a/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt b/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt new file mode 100644 index 000..b1b2a67 --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt @@ -0,0 +1,41 @@ +* Rockchip GPIO support for GRF_SOC_CON registers + +Required properties: +- compatible: Should contain "rockchip,gpio-syscon". +- gpio-controller: Marks the device node as a gpio controller. +- #gpio-cells: Should be two. The first cell is the pin number and + the second cell is used to specify the gpio polarity: +0 = Active high, +1 = Active low. There's no need for this child node. Just make the parent node a gpio controller. Rob Hi Rob, it is not clear to me. Do you suggest that the grf node should be a gpio controller, like below? +grf: syscon at ff10 { +compatible = "rockchip,gpio-syscon", "rockchip,rk3328-grf", "syscon", "simple-mfd"; Yes, but drop "rockchip,gpio-syscon" and "simple-mfd". I would disagree quite a bit here. The grf are the "general register files", a bunch of registers used for quite a lot of things, and so it seems among other users, also a gpio-controller for some more random pins not controlled through the regular gpio controllers. For a more fully stocked grf, please see https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/rk3288.dtsi#n855
Re: [PATCH v2 2/5] gpio: syscon: Add gpio-syscon for rockchip
On 2018-05-24 8:18 PM, Heiko Stuebner wrote: Hi Levin, Am Donnerstag, 24. Mai 2018, 03:59:36 CEST schrieb Levin Du: Hi all, I'd like to quote reply of Robin Murphy at http://lists.infradead.org/pipermail/linux-rockchip/2018-May/020619.html I would suggest s/pin number/bit number in the associated GRF register/ here. At least in this RK3328 case there's only one pin, which isn't numbered, and if you naively considered it pin 0 of this 'bank' you'd already have the wrong number. Since we're dealing with the "random SoC-specific controls" region of the GRF as opposed to the relatively-consistent and organised pinmux parts, I don't think we should rely on any assumptions about how things are laid out. I was initially going to suggest a more specific compatible string as well, but on reflection I think the generic "rockchip,gpio-syscon" for basic "flip this single GRF bit" functionality actually is the right way to go. In the specific RK3328 GPIO_MUTE case, there look to be 4 bits in total related to this pin - the enable, value, and some pull controls (which I assume apply when the output is disabled) - if at some point in future we *did* want to start explicitly controlling the rest of them too, then would be a good time to define a separate "rockchip,rk3328-gpio-mute" binding (and probably a dedicated driver) for that specialised functionality, independently of this basic one. Shall we go the generic "rockchip,gpio-syscon" way, or the specific "rockchip,rk3328-gpio-mute" way? I prefer the former one. The property of "gpio,syscon-dev" in gpio-syscon driver should be documented. Since the gpio controller is defined in the dtsi file, which inevitably contains voodoo register addresses. But at the board level dts file, there won't be more register addresses. Past experience shows that the GRF is not really suitable for generalization, as it's more of a dumping ground where chip designers can put everything that's left over. This is especially true for GRF_SOC_CONx registers, that really only contain pretty random bits. So personally, I'd really prefer soc-specific compatibles as everywhere else, instead of trying to push stuff into the devicetree that won't hold up on future socs. On 2018-05-24 3:53 AM, Rob Herring wrote: On Wed, May 23, 2018 at 10:12 AM, Heiko Stübner wrote: Hi Rob, Levin, sorry for being late to the party. Am Mittwoch, 23. Mai 2018, 16:43:07 CEST schrieb Rob Herring: On Tue, May 22, 2018 at 9:02 PM, Levin Du wrote: On 2018-05-23 2:02 AM, Rob Herring wrote: On Fri, May 18, 2018 at 11:52:05AM +0800, d...@t-chip.com.cn wrote: From: Levin Du Some GPIOs sit in the GRF_SOC_CON registers of Rockchip SoCs, which do not belong to the general pinctrl. Adding gpio-syscon support makes controlling regulator or LED using these special pins very easy by reusing existing drivers, such as gpio-regulator and led-gpio. Signed-off-by: Levin Du --- Changes in v2: - Rename gpio_syscon10 to gpio_mute in doc Changes in v1: - Refactured for general gpio-syscon usage for Rockchip SoCs. - Add doc rockchip,gpio-syscon.txt .../bindings/gpio/rockchip,gpio-syscon.txt | 41 ++ drivers/gpio/gpio-syscon.c | 30 2 files changed, 71 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt diff --git a/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt b/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt new file mode 100644 index 000..b1b2a67 --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt @@ -0,0 +1,41 @@ +* Rockchip GPIO support for GRF_SOC_CON registers + +Required properties: +- compatible: Should contain "rockchip,gpio-syscon". +- gpio-controller: Marks the device node as a gpio controller. +- #gpio-cells: Should be two. The first cell is the pin number and + the second cell is used to specify the gpio polarity: +0 = Active high, +1 = Active low. There's no need for this child node. Just make the parent node a gpio controller. Rob Hi Rob, it is not clear to me. Do you suggest that the grf node should be a gpio controller, like below? +grf: syscon at ff10 { +compatible = "rockchip,gpio-syscon", "rockchip,rk3328-grf", "syscon", "simple-mfd"; Yes, but drop "rockchip,gpio-syscon" and "simple-mfd". I would disagree quite a bit here. The grf are the "general register files", a bunch of registers used for quite a lot of things, and so it seems among other users, also a gpio-controller for some more random pins not controlled through the regular gpio controllers. For a more fully stocked grf, please see https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/rk3288.dtsi#n855 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3399.dtsi#n1338 So the gpio controller should definitly
Re: [REGRESSION] (>= v4.12) IO w/dmcrypt causing audio underruns
On Thu, Jan 25, 2018 at 12:33:21AM -0800, vcap...@pengaru.com wrote: > On Fri, Jan 19, 2018 at 11:57:32AM +0100, Enric Balletbo Serra wrote: > > Hi Vito, > > > > 2018-01-17 23:48 GMT+01:00: > > > On Mon, Dec 18, 2017 at 10:25:33AM +0100, Enric Balletbo Serra wrote: > > >> Hi Vito, > > >> > > >> 2017-12-01 22:33 GMT+01:00 : > > >> > On Wed, Nov 29, 2017 at 10:39:19AM -0800, vcap...@pengaru.com wrote: > > >> >> Hello, > > >> >> > > >> >> Recently I noticed substantial audio dropouts when listening to MP3s > > >> >> in > > >> >> `cmus` while doing big and churny `git checkout` commands in my linux > > >> >> git > > >> >> tree. > > >> >> > > >> >> It's not something I've done much of over the last couple months so I > > >> >> hadn't noticed until yesterday, but didn't remember this being a > > >> >> problem in > > >> >> recent history. > > >> >> > > >> >> As there's quite an accumulation of similarly configured and built > > >> >> kernels > > >> >> in my grub menu, it was trivial to determine approximately when this > > >> >> began: > > >> >> > > >> >> 4.11.0: no dropouts > > >> >> 4.12.0-rc7: dropouts > > >> >> 4.14.0-rc6: dropouts (seem more substantial as well, didn't > > >> >> investigate) > > >> >> > > >> >> Watching top while this is going on in the various kernel versions, > > >> >> it's > > >> >> apparent that the kworker behavior changed. Both the priority and > > >> >> quantity > > >> >> of running kworker threads is elevated in kernels experiencing > > >> >> dropouts. > > >> >> > > >> >> Searching through the commit history for v4.11..v4.12 uncovered: > > >> >> > > >> >> commit a1b89132dc4f61071bdeaab92ea958e0953380a1 > > >> >> Author: Tim Murray > > >> >> Date: Fri Apr 21 11:11:36 2017 +0200 > > >> >> > > >> >> dm crypt: use WQ_HIGHPRI for the IO and crypt workqueues > > >> >> > > >> >> Running dm-crypt with workqueues at the standard priority results > > >> >> in IO > > >> >> competing for CPU time with standard user apps, which can lead to > > >> >> pipeline bubbles and seriously degraded performance. Move to > > >> >> using > > >> >> WQ_HIGHPRI workqueues to protect against that. > > >> >> > > >> >> Signed-off-by: Tim Murray > > >> >> Signed-off-by: Enric Balletbo i Serra > > >> >> > > >> >> Signed-off-by: Mike Snitzer > > >> >> > > >> >> --- > > >> >> > > >> >> Reverting a1b8913 from 4.14.0-rc6, my current kernel, eliminates the > > >> >> problem completely. > > >> >> > > >> >> Looking at the diff in that commit, it looks like the commit message > > >> >> isn't > > >> >> even accurate; not only is the priority of the dmcrypt workqueues > > >> >> being > > >> >> changed - they're also being made "CPU intensive" workqueues as well. > > >> >> > > >> >> This combination appears to result in both elevated scheduling > > >> >> priority and > > >> >> greater quantity of participant worker threads effectively starving > > >> >> any > > >> >> normal priority user task under periods of heavy IO on dmcrypt > > >> >> volumes. > > >> >> > > >> >> I don't know what the right solution is here. It seems to me we're > > >> >> lacking > > >> >> the appropriate mechanism for charging CPU resources consumed on > > >> >> behalf of > > >> >> user processes in kworker threads to the work-causing process. > > >> >> > > >> >> What effectively happens is my normal `git` user process is able to > > >> >> greatly amplify what share of CPU it takes from the system by > > >> >> generating IO > > >> >> on what happens to be a high-priority CPU-intensive storage volume. > > >> >> > > >> >> It looks potentially complicated to fix properly, but I suspect at > > >> >> its core > > >> >> this may be a fairly longstanding shortcoming of the page cache and > > >> >> its > > >> >> asynchronous design. Something that has been exacerbated > > >> >> substantially by > > >> >> the introduction of CPU-intensive storage subsystems like dmcrypt. > > >> >> > > >> >> If we imagine the whole stack simplified, where all the IO was being > > >> >> done > > >> >> synchronously in-band, and the dmcrypt kernel code simply ran in the > > >> >> IO-causing process context, it would be getting charged to the calling > > >> >> process and scheduled accordingly. The resource accounting and > > >> >> scheduling > > >> >> problems all emerge with the page cache, buffered IO, and async > > >> >> background > > >> >> writeback in a pool of unrelated worker threads, etc. That's how it > > >> >> appears to me anyways... > > >> >> > > >> >> The system used is a X61s Thinkpad 1.8Ghz with 840 EVO SSD, lvm on > > >> >> dmcrypt. > > >> >> The kernel .config is attached in case it's of interest. > > >> >> > > >> >> Thanks, > > >> >> Vito Caputo > > >> > > > >> > > > >> > > > >> > Ping... > > >> > > > >> > Could somebody please at least ACK receiving this so
Re: [REGRESSION] (>= v4.12) IO w/dmcrypt causing audio underruns
On Thu, Jan 25, 2018 at 12:33:21AM -0800, vcap...@pengaru.com wrote: > On Fri, Jan 19, 2018 at 11:57:32AM +0100, Enric Balletbo Serra wrote: > > Hi Vito, > > > > 2018-01-17 23:48 GMT+01:00 : > > > On Mon, Dec 18, 2017 at 10:25:33AM +0100, Enric Balletbo Serra wrote: > > >> Hi Vito, > > >> > > >> 2017-12-01 22:33 GMT+01:00 : > > >> > On Wed, Nov 29, 2017 at 10:39:19AM -0800, vcap...@pengaru.com wrote: > > >> >> Hello, > > >> >> > > >> >> Recently I noticed substantial audio dropouts when listening to MP3s > > >> >> in > > >> >> `cmus` while doing big and churny `git checkout` commands in my linux > > >> >> git > > >> >> tree. > > >> >> > > >> >> It's not something I've done much of over the last couple months so I > > >> >> hadn't noticed until yesterday, but didn't remember this being a > > >> >> problem in > > >> >> recent history. > > >> >> > > >> >> As there's quite an accumulation of similarly configured and built > > >> >> kernels > > >> >> in my grub menu, it was trivial to determine approximately when this > > >> >> began: > > >> >> > > >> >> 4.11.0: no dropouts > > >> >> 4.12.0-rc7: dropouts > > >> >> 4.14.0-rc6: dropouts (seem more substantial as well, didn't > > >> >> investigate) > > >> >> > > >> >> Watching top while this is going on in the various kernel versions, > > >> >> it's > > >> >> apparent that the kworker behavior changed. Both the priority and > > >> >> quantity > > >> >> of running kworker threads is elevated in kernels experiencing > > >> >> dropouts. > > >> >> > > >> >> Searching through the commit history for v4.11..v4.12 uncovered: > > >> >> > > >> >> commit a1b89132dc4f61071bdeaab92ea958e0953380a1 > > >> >> Author: Tim Murray > > >> >> Date: Fri Apr 21 11:11:36 2017 +0200 > > >> >> > > >> >> dm crypt: use WQ_HIGHPRI for the IO and crypt workqueues > > >> >> > > >> >> Running dm-crypt with workqueues at the standard priority results > > >> >> in IO > > >> >> competing for CPU time with standard user apps, which can lead to > > >> >> pipeline bubbles and seriously degraded performance. Move to > > >> >> using > > >> >> WQ_HIGHPRI workqueues to protect against that. > > >> >> > > >> >> Signed-off-by: Tim Murray > > >> >> Signed-off-by: Enric Balletbo i Serra > > >> >> > > >> >> Signed-off-by: Mike Snitzer > > >> >> > > >> >> --- > > >> >> > > >> >> Reverting a1b8913 from 4.14.0-rc6, my current kernel, eliminates the > > >> >> problem completely. > > >> >> > > >> >> Looking at the diff in that commit, it looks like the commit message > > >> >> isn't > > >> >> even accurate; not only is the priority of the dmcrypt workqueues > > >> >> being > > >> >> changed - they're also being made "CPU intensive" workqueues as well. > > >> >> > > >> >> This combination appears to result in both elevated scheduling > > >> >> priority and > > >> >> greater quantity of participant worker threads effectively starving > > >> >> any > > >> >> normal priority user task under periods of heavy IO on dmcrypt > > >> >> volumes. > > >> >> > > >> >> I don't know what the right solution is here. It seems to me we're > > >> >> lacking > > >> >> the appropriate mechanism for charging CPU resources consumed on > > >> >> behalf of > > >> >> user processes in kworker threads to the work-causing process. > > >> >> > > >> >> What effectively happens is my normal `git` user process is able to > > >> >> greatly amplify what share of CPU it takes from the system by > > >> >> generating IO > > >> >> on what happens to be a high-priority CPU-intensive storage volume. > > >> >> > > >> >> It looks potentially complicated to fix properly, but I suspect at > > >> >> its core > > >> >> this may be a fairly longstanding shortcoming of the page cache and > > >> >> its > > >> >> asynchronous design. Something that has been exacerbated > > >> >> substantially by > > >> >> the introduction of CPU-intensive storage subsystems like dmcrypt. > > >> >> > > >> >> If we imagine the whole stack simplified, where all the IO was being > > >> >> done > > >> >> synchronously in-band, and the dmcrypt kernel code simply ran in the > > >> >> IO-causing process context, it would be getting charged to the calling > > >> >> process and scheduled accordingly. The resource accounting and > > >> >> scheduling > > >> >> problems all emerge with the page cache, buffered IO, and async > > >> >> background > > >> >> writeback in a pool of unrelated worker threads, etc. That's how it > > >> >> appears to me anyways... > > >> >> > > >> >> The system used is a X61s Thinkpad 1.8Ghz with 840 EVO SSD, lvm on > > >> >> dmcrypt. > > >> >> The kernel .config is attached in case it's of interest. > > >> >> > > >> >> Thanks, > > >> >> Vito Caputo > > >> > > > >> > > > >> > > > >> > Ping... > > >> > > > >> > Could somebody please at least ACK receiving this so I'm not left > > >> > wondering > > >> > if my mails to lkml are somehow winding up flagged as spam, thanks! > > >> > > >> Sorry I did
[GIT PULL] nds32 fixes for 4.17
Linux 4.17-rc6 (2018-05-20 15:31:38 -0700) are available in the Git repository at: ssh://g...@gitolite.kernel.org/pub/scm/linux/kernel/git/greentime/linux.git tags/nds32-for-linus-4.17-fixes for you to fetch changes up to a30e7d1e37e8acc37c25420d93af218166cca3ae: nds32: Fix compiler warning, Wstringop-overflow, in vdso.c (2018-05-23 13:26:22 +0800) nds32 patches for 4.17 Here is the nds32 patch set based on 4.17-rc6. Contained in here are the bug fixes and building error fixes for nds32. These are the LTP20170427 testing results. hackbench01 may fail sometimes. We are still investigating this issue. Total Tests: 1902 Total Skipped Tests: 593 Total Failures: 420 Kernel Version: 4.17.0-rc6-00018-ga30e7d1e37e8 Machine Architecture: nds32 Total Tests: 1902 Total Skipped Tests: 593 Total Failures: 419 Kernel Version: 4.17.0-rc5-00018-g27288975a735 Machine Architecture: nds32 Signed-off-by: Greentime HuGreentime Hu (12): nds32: lib: To use generic lib instead of libgcc to prevent the symbol undefined issue. nds32: Fix building error when CONFIG_FREEZE is enabled. nds32: Fix building error of crypto/xor.c by adding xor.h nds32: Fix drivers/gpu/drm/udl/udl_fb.c building error by defining PAGE_SHARED nds32: Fix xfs_buf built failed by export invalidate_kernel_vmap_range and flush_kernel_vmap_range nds32: Fix the symbols undefined issue by exporting them. nds32: Fix the unknown type u8 issue. nds32: Fix build failed because arch_trace_hardirqs_off is changed to trace_hardirqs_off. nds32: Fix the allmodconfig build. To make sure CONFIG_CPU_LITTLE_ENDIAN is default y nds32: Fix the virtual address may map too much range by tlbop issue. nds32: To refine readability of INT_MASK_INITAIAL_VAL nds32: To fix a cache inconsistency issue by setting correct cacheability of NTC Nickhu (2): nds32: Renaming the file for unaligned access nds32: Fix the unaligned access handler Vincent Chen (4): nds32: Correct flush_dcache_page function nds32: Flush the cache of the page at vmaddr instead of kaddr in flush_anon_page nds32: Disable local irq before calling cpu_dcache_wb_page in copy_user_highpage nds32: Fix compiler warning, Wstringop-overflow, in vdso.c arch/nds32/Kconfig | 7 +++ arch/nds32/Kconfig.cpu | 5 +++-- arch/nds32/Makefile | 7 --- arch/nds32/include/asm/Kbuild | 2 ++ arch/nds32/include/asm/bitfield.h | 3 ++- arch/nds32/include/asm/cacheflush.h | 2 ++ arch/nds32/include/asm/io.h | 2 ++ arch/nds32/include/asm/page.h | 3 +++ arch/nds32/include/asm/pgtable.h| 1 + arch/nds32/kernel/ex-entry.S| 2 +- arch/nds32/kernel/head.S| 28 +++- arch/nds32/kernel/setup.c | 3 +++ arch/nds32/kernel/stacktrace.c | 2 ++ arch/nds32/kernel/vdso.c| 10 +- arch/nds32/lib/copy_page.S | 3 +++ arch/nds32/mm/alignment.c | 9 ++--- arch/nds32/mm/cacheflush.c | 74 -- arch/nds32/mm/init.c| 1 + 18 files changed, 130 insertions(+), 34 deletions(-)
[GIT PULL] nds32 fixes for 4.17
Linux 4.17-rc6 (2018-05-20 15:31:38 -0700) are available in the Git repository at: ssh://g...@gitolite.kernel.org/pub/scm/linux/kernel/git/greentime/linux.git tags/nds32-for-linus-4.17-fixes for you to fetch changes up to a30e7d1e37e8acc37c25420d93af218166cca3ae: nds32: Fix compiler warning, Wstringop-overflow, in vdso.c (2018-05-23 13:26:22 +0800) nds32 patches for 4.17 Here is the nds32 patch set based on 4.17-rc6. Contained in here are the bug fixes and building error fixes for nds32. These are the LTP20170427 testing results. hackbench01 may fail sometimes. We are still investigating this issue. Total Tests: 1902 Total Skipped Tests: 593 Total Failures: 420 Kernel Version: 4.17.0-rc6-00018-ga30e7d1e37e8 Machine Architecture: nds32 Total Tests: 1902 Total Skipped Tests: 593 Total Failures: 419 Kernel Version: 4.17.0-rc5-00018-g27288975a735 Machine Architecture: nds32 Signed-off-by: Greentime Hu Greentime Hu (12): nds32: lib: To use generic lib instead of libgcc to prevent the symbol undefined issue. nds32: Fix building error when CONFIG_FREEZE is enabled. nds32: Fix building error of crypto/xor.c by adding xor.h nds32: Fix drivers/gpu/drm/udl/udl_fb.c building error by defining PAGE_SHARED nds32: Fix xfs_buf built failed by export invalidate_kernel_vmap_range and flush_kernel_vmap_range nds32: Fix the symbols undefined issue by exporting them. nds32: Fix the unknown type u8 issue. nds32: Fix build failed because arch_trace_hardirqs_off is changed to trace_hardirqs_off. nds32: Fix the allmodconfig build. To make sure CONFIG_CPU_LITTLE_ENDIAN is default y nds32: Fix the virtual address may map too much range by tlbop issue. nds32: To refine readability of INT_MASK_INITAIAL_VAL nds32: To fix a cache inconsistency issue by setting correct cacheability of NTC Nickhu (2): nds32: Renaming the file for unaligned access nds32: Fix the unaligned access handler Vincent Chen (4): nds32: Correct flush_dcache_page function nds32: Flush the cache of the page at vmaddr instead of kaddr in flush_anon_page nds32: Disable local irq before calling cpu_dcache_wb_page in copy_user_highpage nds32: Fix compiler warning, Wstringop-overflow, in vdso.c arch/nds32/Kconfig | 7 +++ arch/nds32/Kconfig.cpu | 5 +++-- arch/nds32/Makefile | 7 --- arch/nds32/include/asm/Kbuild | 2 ++ arch/nds32/include/asm/bitfield.h | 3 ++- arch/nds32/include/asm/cacheflush.h | 2 ++ arch/nds32/include/asm/io.h | 2 ++ arch/nds32/include/asm/page.h | 3 +++ arch/nds32/include/asm/pgtable.h| 1 + arch/nds32/kernel/ex-entry.S| 2 +- arch/nds32/kernel/head.S| 28 +++- arch/nds32/kernel/setup.c | 3 +++ arch/nds32/kernel/stacktrace.c | 2 ++ arch/nds32/kernel/vdso.c| 10 +- arch/nds32/lib/copy_page.S | 3 +++ arch/nds32/mm/alignment.c | 9 ++--- arch/nds32/mm/cacheflush.c | 74 -- arch/nds32/mm/init.c| 1 + 18 files changed, 130 insertions(+), 34 deletions(-)
Re: [PATCH] perf tools: Fix indexing for decoder packet queue
On Fri, May 25, 2018 at 05:10:54PM -0600, Mathieu Poirier wrote: > The tail of a queue is supposed to be pointing to the next available slot > in a queue. In this implementation the tail is incremented before it is > used and as such points to the last used element, something that has the > immense advantage of centralizing tail management at a single location > and eliminating a lot of redundant code. > > But this needs to be taken into consideration on the dequeueing side where > the head also needs to be incremented before it is used, or the first > available element of the queue will be skipped. > > Signed-off-by: Mathieu Poirier> --- > tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 12 ++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > index c8b98fa22997..4d5fc374e730 100644 > --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > @@ -96,11 +96,19 @@ int cs_etm_decoder__get_packet(struct cs_etm_decoder > *decoder, > /* Nothing to do, might as well just return */ > if (decoder->packet_count == 0) > return 0; > + /* > + * The queueing process in function cs_etm_decoder__buffer_packet() > + * increments the tail *before* using it. This is somewhat counter > + * intuitive but it has the advantage of centralizing tail management > + * at a single location. Because of that we need to follow the same > + * heuristic with the head, i.e we increment it before using its > + * value. Otherwise the first element of the packet queue is not > + * used. > + */ > + decoder->head = (decoder->head + 1) & (MAX_BUFFER - 1); > > *packet = decoder->packet_buffer[decoder->head]; > > - decoder->head = (decoder->head + 1) & (MAX_BUFFER - 1); > - I tested this patch and confirmed it can work well with python decoding script: Tested-by: Leo Yan Actually, I have another idea for this fixing, seems to me the unchanged code has right logic for decoder->head, and I think this issue is more related with incorrect initialization index. So we can change the initialization index for decoder->head as below. How about you think for this? diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c index c8b98fa..b133260 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -249,7 +249,7 @@ static void cs_etm_decoder__clear_buffer(struct cs_etm_decoder *decoder) { int i; - decoder->head = 0; + decoder->head = 1; decoder->tail = 0; decoder->packet_count = 0; for (i = 0; i < MAX_BUFFER; i++) { Thanks, Leo Yan > decoder->packet_count--; > > return 1; > -- > 2.7.4 >
Re: [PATCH] perf tools: Fix indexing for decoder packet queue
On Fri, May 25, 2018 at 05:10:54PM -0600, Mathieu Poirier wrote: > The tail of a queue is supposed to be pointing to the next available slot > in a queue. In this implementation the tail is incremented before it is > used and as such points to the last used element, something that has the > immense advantage of centralizing tail management at a single location > and eliminating a lot of redundant code. > > But this needs to be taken into consideration on the dequeueing side where > the head also needs to be incremented before it is used, or the first > available element of the queue will be skipped. > > Signed-off-by: Mathieu Poirier > --- > tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 12 ++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > index c8b98fa22997..4d5fc374e730 100644 > --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > @@ -96,11 +96,19 @@ int cs_etm_decoder__get_packet(struct cs_etm_decoder > *decoder, > /* Nothing to do, might as well just return */ > if (decoder->packet_count == 0) > return 0; > + /* > + * The queueing process in function cs_etm_decoder__buffer_packet() > + * increments the tail *before* using it. This is somewhat counter > + * intuitive but it has the advantage of centralizing tail management > + * at a single location. Because of that we need to follow the same > + * heuristic with the head, i.e we increment it before using its > + * value. Otherwise the first element of the packet queue is not > + * used. > + */ > + decoder->head = (decoder->head + 1) & (MAX_BUFFER - 1); > > *packet = decoder->packet_buffer[decoder->head]; > > - decoder->head = (decoder->head + 1) & (MAX_BUFFER - 1); > - I tested this patch and confirmed it can work well with python decoding script: Tested-by: Leo Yan Actually, I have another idea for this fixing, seems to me the unchanged code has right logic for decoder->head, and I think this issue is more related with incorrect initialization index. So we can change the initialization index for decoder->head as below. How about you think for this? diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c index c8b98fa..b133260 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -249,7 +249,7 @@ static void cs_etm_decoder__clear_buffer(struct cs_etm_decoder *decoder) { int i; - decoder->head = 0; + decoder->head = 1; decoder->tail = 0; decoder->packet_count = 0; for (i = 0; i < MAX_BUFFER; i++) { Thanks, Leo Yan > decoder->packet_count--; > > return 1; > -- > 2.7.4 >
Re: [PATCH] drivers/net/phy/micrel: Fix for PHY KSZ8061 errrata: Potential link-up failure when Ethernet cable is connected slowly
> This e-mail (including any attachments) is confidential and may be > legally privileged. If you are not an intended recipient or an > authorized representative of an intended recipient, you are > prohibited from using, copying or distributing the information in > this e-mail or its attachments. If you have received this e-mail in > error, please notify the sender immediately by return e-mail and > delete all copies of this message and any attachments. Thank you. Once this has been removed, i will have a comment... Andrew
Re: [PATCH] drivers/net/phy/micrel: Fix for PHY KSZ8061 errrata: Potential link-up failure when Ethernet cable is connected slowly
> This e-mail (including any attachments) is confidential and may be > legally privileged. If you are not an intended recipient or an > authorized representative of an intended recipient, you are > prohibited from using, copying or distributing the information in > this e-mail or its attachments. If you have received this e-mail in > error, please notify the sender immediately by return e-mail and > delete all copies of this message and any attachments. Thank you. Once this has been removed, i will have a comment... Andrew
Re: [PATCH v3 8/8] drm/mediatek: add third ddp path
Hi, Stu: On Mon, 2018-05-28 at 10:26 +0800, Stu Hsieh wrote: > Hi, CK: > I've some idea as below. > > On Fri, 2018-05-25 at 13:00 +0800, CK Hu wrote: > > Hi, Stu: > > > > On Fri, 2018-05-25 at 10:34 +0800, stu.hs...@mediatek.com wrote: > > > From: Stu Hsieh> > > > > > This patch create third crtc by third ddp path > > > > > > > Apply this patch before the patch 'Add support for mediatek SOC MT2712' > > because this patch is necessary for mt2712. > > > > > Signed-off-by: Stu Hsieh > > > --- > > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 + > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > > b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > > index b32c4cc8d051..3a866e1d6af4 100644 > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > > @@ -267,6 +267,11 @@ static int mtk_drm_kms_init(struct drm_device *drm) > > > if (ret < 0) > > > goto err_component_unbind; > > > > > > + ret = mtk_drm_crtc_create(drm, private->data->third_path, > > > + private->data->third_len); > > > > I think you should prevent doing this for mt8173 and mt2701 because that > > two SoC has only two ddp path. > > Now, 8173 and 2701 have only two ddp path, there is a problem on run > time. > There are three crtc for drm resource, and there is nothing in third > crtc. > Because 8173 and 2701 have no third ddp, and the third ddp_len is zero. > So, I want to add the condition like following in mtk_crtc_create() > if (path_len == 0) > return 0; > > Then, the valur ret is zero and it would not create the null third crtc. This sounds good to me. Regards, CK > > > Regards, > Stu > > > > > Regards, > > CK > > > > > + if (ret < 0) > > > + goto err_component_unbind; > > > + > > > /* Use OVL device for all DMA memory allocations */ > > > np = private->comp_node[private->data->main_path[0]] ?: > > >private->comp_node[private->data->ext_path[0]]; > > > > > >
Re: [PATCH v3 8/8] drm/mediatek: add third ddp path
Hi, Stu: On Mon, 2018-05-28 at 10:26 +0800, Stu Hsieh wrote: > Hi, CK: > I've some idea as below. > > On Fri, 2018-05-25 at 13:00 +0800, CK Hu wrote: > > Hi, Stu: > > > > On Fri, 2018-05-25 at 10:34 +0800, stu.hs...@mediatek.com wrote: > > > From: Stu Hsieh > > > > > > This patch create third crtc by third ddp path > > > > > > > Apply this patch before the patch 'Add support for mediatek SOC MT2712' > > because this patch is necessary for mt2712. > > > > > Signed-off-by: Stu Hsieh > > > --- > > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 + > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > > b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > > index b32c4cc8d051..3a866e1d6af4 100644 > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > > @@ -267,6 +267,11 @@ static int mtk_drm_kms_init(struct drm_device *drm) > > > if (ret < 0) > > > goto err_component_unbind; > > > > > > + ret = mtk_drm_crtc_create(drm, private->data->third_path, > > > + private->data->third_len); > > > > I think you should prevent doing this for mt8173 and mt2701 because that > > two SoC has only two ddp path. > > Now, 8173 and 2701 have only two ddp path, there is a problem on run > time. > There are three crtc for drm resource, and there is nothing in third > crtc. > Because 8173 and 2701 have no third ddp, and the third ddp_len is zero. > So, I want to add the condition like following in mtk_crtc_create() > if (path_len == 0) > return 0; > > Then, the valur ret is zero and it would not create the null third crtc. This sounds good to me. Regards, CK > > > Regards, > Stu > > > > > Regards, > > CK > > > > > + if (ret < 0) > > > + goto err_component_unbind; > > > + > > > /* Use OVL device for all DMA memory allocations */ > > > np = private->comp_node[private->data->main_path[0]] ?: > > >private->comp_node[private->data->ext_path[0]]; > > > > > >
Re: [RESEND PATCH V5 00/33] block: support multipage bvec
On Sun, May 27, 2018 at 07:44:52PM -0600, Jens Axboe wrote: > On 5/27/18 1:23 AM, Ming Lei wrote: > > On Fri, May 25, 2018 at 10:30:46AM -0600, Jens Axboe wrote: > >> On 5/24/18 10:53 PM, Kent Overstreet wrote: > >>> On Fri, May 25, 2018 at 11:45:48AM +0800, Ming Lei wrote: > Hi, > > This patchset brings multipage bvec into block layer: > >>> > >>> patch series looks sane to me. goddamn that's a lot of renaming. > >> > >> Indeed... I actually objected to some of the segment -> page > >> renaming, but it's still in there. The foo2() temporary functions > >> also concern me, we all know there's nothing more permanent than a > >> temporary fixup. > > > > Jens, I remember I explained the renaming story to you in lsfmm a bit: > > > > 1) the current naming of segment is actually wrong, since every segment > > only stores one single-page vector > > > > 2) the most important part is that once multipage bvec is introduced, > > if the old _segment naming is still kept, it can be very confusing, > > especially no good name is left for the helpers of dealing with real > > segment. > > Yes, we discussed exactly this, which is why I'm surprised you went > ahead with the same approach. I told you I don't like tree wide renames, Maybe I misunderstood your point, that isn't strange given my poor english, :-) > if they can be avoided. I'd rather suffer some pain wrt page vs segments > naming, and then later do a rename (if it bothers us) once the dust has > settled on the interesting part of the changes. > > I'm very well away of our current naming and what it signifies. With > #1, you are really splitting hairs, imho. Find a decent name for > multiple segment. Chunk? OK, will try _chunk in next post. > > > For the foo2() temporary change, that is only for avoiding tree-wide > > change in one single tree, with this way, we can change sub-system one > > by one, but if you think it is good to do tree-wide conversion in one > > patch, I am fine to do it in next version. > > It's still a painful middle step. I hate the conversion too, but looks it can't be avoided since bio_for_each_segment_all() has to be changed. Could you share us what your favorite approach is for this conversion? Thanks, Ming
Re: [RESEND PATCH V5 00/33] block: support multipage bvec
On Sun, May 27, 2018 at 07:44:52PM -0600, Jens Axboe wrote: > On 5/27/18 1:23 AM, Ming Lei wrote: > > On Fri, May 25, 2018 at 10:30:46AM -0600, Jens Axboe wrote: > >> On 5/24/18 10:53 PM, Kent Overstreet wrote: > >>> On Fri, May 25, 2018 at 11:45:48AM +0800, Ming Lei wrote: > Hi, > > This patchset brings multipage bvec into block layer: > >>> > >>> patch series looks sane to me. goddamn that's a lot of renaming. > >> > >> Indeed... I actually objected to some of the segment -> page > >> renaming, but it's still in there. The foo2() temporary functions > >> also concern me, we all know there's nothing more permanent than a > >> temporary fixup. > > > > Jens, I remember I explained the renaming story to you in lsfmm a bit: > > > > 1) the current naming of segment is actually wrong, since every segment > > only stores one single-page vector > > > > 2) the most important part is that once multipage bvec is introduced, > > if the old _segment naming is still kept, it can be very confusing, > > especially no good name is left for the helpers of dealing with real > > segment. > > Yes, we discussed exactly this, which is why I'm surprised you went > ahead with the same approach. I told you I don't like tree wide renames, Maybe I misunderstood your point, that isn't strange given my poor english, :-) > if they can be avoided. I'd rather suffer some pain wrt page vs segments > naming, and then later do a rename (if it bothers us) once the dust has > settled on the interesting part of the changes. > > I'm very well away of our current naming and what it signifies. With > #1, you are really splitting hairs, imho. Find a decent name for > multiple segment. Chunk? OK, will try _chunk in next post. > > > For the foo2() temporary change, that is only for avoiding tree-wide > > change in one single tree, with this way, we can change sub-system one > > by one, but if you think it is good to do tree-wide conversion in one > > patch, I am fine to do it in next version. > > It's still a painful middle step. I hate the conversion too, but looks it can't be avoided since bio_for_each_segment_all() has to be changed. Could you share us what your favorite approach is for this conversion? Thanks, Ming
Re: [PATCHv4 1/2] ARM: imx53: add secure-reg-access support for PMU
On Tue, Feb 27, 2018 at 11:17:12AM +0100, Sebastian Reichel wrote: > Hi, > > On Tue, Feb 27, 2018 at 09:10:34AM +0800, Shawn Guo wrote: > > On Mon, Feb 26, 2018 at 02:47:41PM +0100, Sebastian Reichel wrote: > > > On Sat, Feb 24, 2018 at 03:45:44PM +0800, Shawn Guo wrote: > > > > On Mon, Feb 12, 2018 at 01:39:44PM +0100, Sebastian Reichel wrote: > > > > > On i.MX53 it is necessary to set the DBG_EN bit in the > > > > > platform GPC register to enable access to PMU counters > > > > > other than the cycle counter. > > > > > > > > > > Signed-off-by: Sebastian Reichel> > > > > --- > > > > > arch/arm/mach-imx/mach-imx53.c | 39 > > > > > ++- > > > > > 1 file changed, 38 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/arch/arm/mach-imx/mach-imx53.c > > > > > b/arch/arm/mach-imx/mach-imx53.c > > > > > index 07c2e8dca494..658e28604dca 100644 > > > > > --- a/arch/arm/mach-imx/mach-imx53.c > > > > > +++ b/arch/arm/mach-imx/mach-imx53.c > > > > > @@ -28,10 +28,47 @@ static void __init imx53_init_early(void) > > > > > mxc_set_cpu_type(MXC_CPU_MX53); > > > > > } > > > > > > > > > > +#define MXC_CORTEXA8_PLAT_GPC 0x63fa0004 > > > > > > > > The base address should be retrieved from device tree. > > > > > > DT has no entry for 0x63fa-0x63fa3fff. iMX53 TRM lists it as "ARM > > > Platform" > > > with 8 platform specific 32 bit registers. Do you think it's worth the > > > trouble > > > adding a new binding? Do you have a suggestion for a compatible value? > > > > Looking at it more closely, I feel that patching every single platform > > which needs to set up additional register for secure-reg-access support > > doesn't really scale. Can we have pmu driver do it with a phandle in > > DT pointing to the register and bit that need to be configured? > > The PMU driver used to have a feature for initialising platform > specific bits, but it is currently being removed to make the PMU > code more maintainable. My understanding is, that it's very uncommon > to require platform specific setup to get secure-reg-access working. > I.e. it is not needed for newer iMX platforms. Are you saying this is a very specific setup required by i.MX53 only? In that case, I can live with it. Shawn
Re: [PATCHv4 1/2] ARM: imx53: add secure-reg-access support for PMU
On Tue, Feb 27, 2018 at 11:17:12AM +0100, Sebastian Reichel wrote: > Hi, > > On Tue, Feb 27, 2018 at 09:10:34AM +0800, Shawn Guo wrote: > > On Mon, Feb 26, 2018 at 02:47:41PM +0100, Sebastian Reichel wrote: > > > On Sat, Feb 24, 2018 at 03:45:44PM +0800, Shawn Guo wrote: > > > > On Mon, Feb 12, 2018 at 01:39:44PM +0100, Sebastian Reichel wrote: > > > > > On i.MX53 it is necessary to set the DBG_EN bit in the > > > > > platform GPC register to enable access to PMU counters > > > > > other than the cycle counter. > > > > > > > > > > Signed-off-by: Sebastian Reichel > > > > > --- > > > > > arch/arm/mach-imx/mach-imx53.c | 39 > > > > > ++- > > > > > 1 file changed, 38 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/arch/arm/mach-imx/mach-imx53.c > > > > > b/arch/arm/mach-imx/mach-imx53.c > > > > > index 07c2e8dca494..658e28604dca 100644 > > > > > --- a/arch/arm/mach-imx/mach-imx53.c > > > > > +++ b/arch/arm/mach-imx/mach-imx53.c > > > > > @@ -28,10 +28,47 @@ static void __init imx53_init_early(void) > > > > > mxc_set_cpu_type(MXC_CPU_MX53); > > > > > } > > > > > > > > > > +#define MXC_CORTEXA8_PLAT_GPC 0x63fa0004 > > > > > > > > The base address should be retrieved from device tree. > > > > > > DT has no entry for 0x63fa-0x63fa3fff. iMX53 TRM lists it as "ARM > > > Platform" > > > with 8 platform specific 32 bit registers. Do you think it's worth the > > > trouble > > > adding a new binding? Do you have a suggestion for a compatible value? > > > > Looking at it more closely, I feel that patching every single platform > > which needs to set up additional register for secure-reg-access support > > doesn't really scale. Can we have pmu driver do it with a phandle in > > DT pointing to the register and bit that need to be configured? > > The PMU driver used to have a feature for initialising platform > specific bits, but it is currently being removed to make the PMU > code more maintainable. My understanding is, that it's very uncommon > to require platform specific setup to get secure-reg-access working. > I.e. it is not needed for newer iMX platforms. Are you saying this is a very specific setup required by i.MX53 only? In that case, I can live with it. Shawn
Re: [PATCH v3 8/8] drm/mediatek: add third ddp path
Hi, CK: I've some idea as below. On Fri, 2018-05-25 at 13:00 +0800, CK Hu wrote: > Hi, Stu: > > On Fri, 2018-05-25 at 10:34 +0800, stu.hs...@mediatek.com wrote: > > From: Stu Hsieh> > > > This patch create third crtc by third ddp path > > > > Apply this patch before the patch 'Add support for mediatek SOC MT2712' > because this patch is necessary for mt2712. > > > Signed-off-by: Stu Hsieh > > --- > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > index b32c4cc8d051..3a866e1d6af4 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > @@ -267,6 +267,11 @@ static int mtk_drm_kms_init(struct drm_device *drm) > > if (ret < 0) > > goto err_component_unbind; > > > > + ret = mtk_drm_crtc_create(drm, private->data->third_path, > > + private->data->third_len); > > I think you should prevent doing this for mt8173 and mt2701 because that > two SoC has only two ddp path. Now, 8173 and 2701 have only two ddp path, there is a problem on run time. There are three crtc for drm resource, and there is nothing in third crtc. Because 8173 and 2701 have no third ddp, and the third ddp_len is zero. So, I want to add the condition like following in mtk_crtc_create() if (path_len == 0) return 0; Then, the valur ret is zero and it would not create the null third crtc. Regards, Stu > > Regards, > CK > > > + if (ret < 0) > > + goto err_component_unbind; > > + > > /* Use OVL device for all DMA memory allocations */ > > np = private->comp_node[private->data->main_path[0]] ?: > > private->comp_node[private->data->ext_path[0]]; > >
Re: [PATCH v3 8/8] drm/mediatek: add third ddp path
Hi, CK: I've some idea as below. On Fri, 2018-05-25 at 13:00 +0800, CK Hu wrote: > Hi, Stu: > > On Fri, 2018-05-25 at 10:34 +0800, stu.hs...@mediatek.com wrote: > > From: Stu Hsieh > > > > This patch create third crtc by third ddp path > > > > Apply this patch before the patch 'Add support for mediatek SOC MT2712' > because this patch is necessary for mt2712. > > > Signed-off-by: Stu Hsieh > > --- > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > index b32c4cc8d051..3a866e1d6af4 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > @@ -267,6 +267,11 @@ static int mtk_drm_kms_init(struct drm_device *drm) > > if (ret < 0) > > goto err_component_unbind; > > > > + ret = mtk_drm_crtc_create(drm, private->data->third_path, > > + private->data->third_len); > > I think you should prevent doing this for mt8173 and mt2701 because that > two SoC has only two ddp path. Now, 8173 and 2701 have only two ddp path, there is a problem on run time. There are three crtc for drm resource, and there is nothing in third crtc. Because 8173 and 2701 have no third ddp, and the third ddp_len is zero. So, I want to add the condition like following in mtk_crtc_create() if (path_len == 0) return 0; Then, the valur ret is zero and it would not create the null third crtc. Regards, Stu > > Regards, > CK > > > + if (ret < 0) > > + goto err_component_unbind; > > + > > /* Use OVL device for all DMA memory allocations */ > > np = private->comp_node[private->data->main_path[0]] ?: > > private->comp_node[private->data->ext_path[0]]; > >
Re: [PATCH] powerpc-opal: fix spelling mistake "Uniterrupted" -> "Uninterrupted"
Colin Kingwrites: > From: Colin Ian King > > Trivial fix to spelling mistake in hmi_error_types text > > Signed-off-by: Colin Ian King Reviewed-by: Stewart Smith -- Stewart Smith OPAL Architect, IBM.
Re: [PATCH] powerpc-opal: fix spelling mistake "Uniterrupted" -> "Uninterrupted"
Colin King writes: > From: Colin Ian King > > Trivial fix to spelling mistake in hmi_error_types text > > Signed-off-by: Colin Ian King Reviewed-by: Stewart Smith -- Stewart Smith OPAL Architect, IBM.
Re: [Nouveau] [PATCH][next] drm/nouveau/disp: avoid potential overflow on shift of int value
On Sun, May 27, 2018 at 5:54 PM, Colin Kingwrote: > From: Colin Ian King > > The constant values being shifted are 32 bit integers and may potentially > overflow on the shift. Avoid this potential overflow by making them > unsigned long long values before the shift. > > Detected by CoverityScan, CID#1469383, 1469400 ("Unintentional > integer overflow") > > Signed-off-by: Colin Ian King > --- > drivers/gpu/drm/nouveau/nvkm/engine/disp/changf119.c | 2 +- > drivers/gpu/drm/nouveau/nvkm/engine/disp/channv50.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/changf119.c > b/drivers/gpu/drm/nouveau/nvkm/engine/disp/changf119.c > index 29e6dd58ac48..99b94802ed63 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/changf119.c > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/changf119.c > @@ -52,7 +52,7 @@ void > gf119_disp_chan_intr(struct nv50_disp_chan *chan, bool en) > { > struct nvkm_device *device = chan->disp->base.engine.subdev.device; > - const u64 mask = 0x0001 << chan->chid.user; > + const u64 mask = 0x0001ULL << chan->chid.user; I'm pretty sure all of these should just be u32 (below as well). The registers that this is masking are all 32-bit, more doesn't make sense. > if (!en) { > nvkm_mask(device, 0x610090, mask, 0x); > nvkm_mask(device, 0x6100a0, mask, 0x); > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/channv50.c > b/drivers/gpu/drm/nouveau/nvkm/engine/disp/channv50.c > index 57719f675eec..43ae3b092e43 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/channv50.c > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/channv50.c > @@ -166,7 +166,7 @@ void > nv50_disp_chan_intr(struct nv50_disp_chan *chan, bool en) > { > struct nvkm_device *device = chan->disp->base.engine.subdev.device; > - const u64 mask = 0x00010001 << chan->chid.user; > + const u64 mask = 0x00010001ULL << chan->chid.user; > const u64 data = en ? 0x0001 : 0x; > nvkm_mask(device, 0x610028, mask, data); > } > -- > 2.17.0 > > ___ > Nouveau mailing list > nouv...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH][next] drm/nouveau/disp: avoid potential overflow on shift of int value
On Sun, May 27, 2018 at 5:54 PM, Colin King wrote: > From: Colin Ian King > > The constant values being shifted are 32 bit integers and may potentially > overflow on the shift. Avoid this potential overflow by making them > unsigned long long values before the shift. > > Detected by CoverityScan, CID#1469383, 1469400 ("Unintentional > integer overflow") > > Signed-off-by: Colin Ian King > --- > drivers/gpu/drm/nouveau/nvkm/engine/disp/changf119.c | 2 +- > drivers/gpu/drm/nouveau/nvkm/engine/disp/channv50.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/changf119.c > b/drivers/gpu/drm/nouveau/nvkm/engine/disp/changf119.c > index 29e6dd58ac48..99b94802ed63 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/changf119.c > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/changf119.c > @@ -52,7 +52,7 @@ void > gf119_disp_chan_intr(struct nv50_disp_chan *chan, bool en) > { > struct nvkm_device *device = chan->disp->base.engine.subdev.device; > - const u64 mask = 0x0001 << chan->chid.user; > + const u64 mask = 0x0001ULL << chan->chid.user; I'm pretty sure all of these should just be u32 (below as well). The registers that this is masking are all 32-bit, more doesn't make sense. > if (!en) { > nvkm_mask(device, 0x610090, mask, 0x); > nvkm_mask(device, 0x6100a0, mask, 0x); > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/channv50.c > b/drivers/gpu/drm/nouveau/nvkm/engine/disp/channv50.c > index 57719f675eec..43ae3b092e43 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/channv50.c > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/channv50.c > @@ -166,7 +166,7 @@ void > nv50_disp_chan_intr(struct nv50_disp_chan *chan, bool en) > { > struct nvkm_device *device = chan->disp->base.engine.subdev.device; > - const u64 mask = 0x00010001 << chan->chid.user; > + const u64 mask = 0x00010001ULL << chan->chid.user; > const u64 data = en ? 0x0001 : 0x; > nvkm_mask(device, 0x610028, mask, data); > } > -- > 2.17.0 > > ___ > Nouveau mailing list > nouv...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau
[PATCH v6 1/4] random: Fix whitespace pre random-bytes work
There are a couple of whitespace issues around the function get_random_bytes_arch(). In preparation for patching this function let's clean them up. Signed-off-by: Tobin C. HardingAcked-by: Theodore Ts'o --- drivers/char/random.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index cd888d4ee605..031d18b31e0f 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -1737,7 +1737,7 @@ void get_random_bytes_arch(void *buf, int nbytes) if (!arch_get_random_long()) break; - + memcpy(p, , chunk); p += chunk; nbytes -= chunk; @@ -1748,7 +1748,6 @@ void get_random_bytes_arch(void *buf, int nbytes) } EXPORT_SYMBOL(get_random_bytes_arch); - /* * init_std_data - initialize pool with system data * -- 2.7.4
[PATCH v6 3/4] vsprintf: Use hw RNG for ptr_key
Currently we must wait for enough entropy to become available before hashed pointers can be printed. We can remove this wait by using the hw RNG if available. Use hw RNG to get keying material. Cc: Steven Rostedt (VMware)Suggested-by: Kees Cook Signed-off-by: Tobin C. Harding --- lib/vsprintf.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 23920c5ff728..1545a8aa26a9 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -1693,8 +1693,16 @@ static struct random_ready_callback random_ready = { static int __init initialize_ptr_random(void) { - int ret = add_random_ready_callback(_ready); + int key_size = sizeof(ptr_key); + int ret; + + /* Use hw RNG if available */ + if (get_random_bytes_arch(_key, key_size) == key_size) { + static_branch_disable(_filled_random_ptr_key); + return 0; + } + ret = add_random_ready_callback(_ready); if (!ret) { return 0; } else if (ret == -EALREADY) { -- 2.7.4
[PATCH v6 4/4] vsprintf: Add command line option debug_boot_weak_hash
Currently printing [hashed] pointers requires enough entropy to be available. Early in the boot sequence this may not be the case resulting in a dummy string '(ptrval)' being printed. This makes debugging the early boot sequence difficult. We can relax the requirement to use cryptographically secure hashing during debugging. This enables debugging while keeping development/production kernel behaviour the same. If new command line option debug_boot_weak_hash is enabled use cryptographically insecure hashing and hash pointer value immediately. Cc: Anna-Maria GleixnerCc: Steven Rostedt Cc: Randy Dunlap Signed-off-by: Tobin C. Harding --- Documentation/admin-guide/kernel-parameters.txt | 9 + lib/vsprintf.c | 20 2 files changed, 29 insertions(+) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index f2040d46f095..8a86d895343e 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -753,6 +753,15 @@ debug [KNL] Enable kernel debugging (events log level). + debug_boot_weak_hash + [KNL] Enable printing pointers early in the boot + sequence. If enabled, we use a weak hash instead of + siphash to hash pointers. Use this option if you need + to see pointer values during early boot (i.e you are + seeing instances of '(___ptrval___)'). + Cryptographically insecure, please do not use on + production kernels. + debug_locks_verbose= [KNL] verbose self-tests Format=<0|1> diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 1545a8aa26a9..369623205e2c 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -1670,6 +1670,20 @@ char *pointer_string(char *buf, char *end, const void *ptr, } static DEFINE_STATIC_KEY_TRUE(not_filled_random_ptr_key); + +/* Make pointers available for printing early in the boot sequence. */ +static int debug_boot_weak_hash __ro_after_init; +EXPORT_SYMBOL(debug_boot_weak_hash); + +static int __init debug_boot_weak_hash_enable(char *str) +{ + debug_boot_weak_hash = 1; + pr_info("debug_boot_weak_hash enabled\n"); + return 0; +} +early_param("debug_boot_weak_hash", debug_boot_weak_hash_enable); + +static bool have_filled_random_ptr_key __read_mostly; static siphash_key_t ptr_key __read_mostly; static void enable_ptr_key_workfn(struct work_struct *work) @@ -1721,6 +1735,12 @@ static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec) unsigned long hashval; const int default_width = 2 * sizeof(ptr); + /* When debugging early boot use non-cryptographically secure hash */ + if (unlikely(debug_boot_weak_hash)) { + hashval = hash_long((unsigned long)ptr, 32); + return pointer_string(buf, end, (const void *)hashval, spec); + } + if (static_branch_unlikely(_filled_random_ptr_key)) { spec.field_width = default_width; /* string length must be less than default_width */ -- 2.7.4
[PATCH v6 4/4] vsprintf: Add command line option debug_boot_weak_hash
Currently printing [hashed] pointers requires enough entropy to be available. Early in the boot sequence this may not be the case resulting in a dummy string '(ptrval)' being printed. This makes debugging the early boot sequence difficult. We can relax the requirement to use cryptographically secure hashing during debugging. This enables debugging while keeping development/production kernel behaviour the same. If new command line option debug_boot_weak_hash is enabled use cryptographically insecure hashing and hash pointer value immediately. Cc: Anna-Maria Gleixner Cc: Steven Rostedt Cc: Randy Dunlap Signed-off-by: Tobin C. Harding --- Documentation/admin-guide/kernel-parameters.txt | 9 + lib/vsprintf.c | 20 2 files changed, 29 insertions(+) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index f2040d46f095..8a86d895343e 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -753,6 +753,15 @@ debug [KNL] Enable kernel debugging (events log level). + debug_boot_weak_hash + [KNL] Enable printing pointers early in the boot + sequence. If enabled, we use a weak hash instead of + siphash to hash pointers. Use this option if you need + to see pointer values during early boot (i.e you are + seeing instances of '(___ptrval___)'). + Cryptographically insecure, please do not use on + production kernels. + debug_locks_verbose= [KNL] verbose self-tests Format=<0|1> diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 1545a8aa26a9..369623205e2c 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -1670,6 +1670,20 @@ char *pointer_string(char *buf, char *end, const void *ptr, } static DEFINE_STATIC_KEY_TRUE(not_filled_random_ptr_key); + +/* Make pointers available for printing early in the boot sequence. */ +static int debug_boot_weak_hash __ro_after_init; +EXPORT_SYMBOL(debug_boot_weak_hash); + +static int __init debug_boot_weak_hash_enable(char *str) +{ + debug_boot_weak_hash = 1; + pr_info("debug_boot_weak_hash enabled\n"); + return 0; +} +early_param("debug_boot_weak_hash", debug_boot_weak_hash_enable); + +static bool have_filled_random_ptr_key __read_mostly; static siphash_key_t ptr_key __read_mostly; static void enable_ptr_key_workfn(struct work_struct *work) @@ -1721,6 +1735,12 @@ static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec) unsigned long hashval; const int default_width = 2 * sizeof(ptr); + /* When debugging early boot use non-cryptographically secure hash */ + if (unlikely(debug_boot_weak_hash)) { + hashval = hash_long((unsigned long)ptr, 32); + return pointer_string(buf, end, (const void *)hashval, spec); + } + if (static_branch_unlikely(_filled_random_ptr_key)) { spec.field_width = default_width; /* string length must be less than default_width */ -- 2.7.4
[PATCH v6 1/4] random: Fix whitespace pre random-bytes work
There are a couple of whitespace issues around the function get_random_bytes_arch(). In preparation for patching this function let's clean them up. Signed-off-by: Tobin C. Harding Acked-by: Theodore Ts'o --- drivers/char/random.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index cd888d4ee605..031d18b31e0f 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -1737,7 +1737,7 @@ void get_random_bytes_arch(void *buf, int nbytes) if (!arch_get_random_long()) break; - + memcpy(p, , chunk); p += chunk; nbytes -= chunk; @@ -1748,7 +1748,6 @@ void get_random_bytes_arch(void *buf, int nbytes) } EXPORT_SYMBOL(get_random_bytes_arch); - /* * init_std_data - initialize pool with system data * -- 2.7.4
[PATCH v6 3/4] vsprintf: Use hw RNG for ptr_key
Currently we must wait for enough entropy to become available before hashed pointers can be printed. We can remove this wait by using the hw RNG if available. Use hw RNG to get keying material. Cc: Steven Rostedt (VMware) Suggested-by: Kees Cook Signed-off-by: Tobin C. Harding --- lib/vsprintf.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 23920c5ff728..1545a8aa26a9 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -1693,8 +1693,16 @@ static struct random_ready_callback random_ready = { static int __init initialize_ptr_random(void) { - int ret = add_random_ready_callback(_ready); + int key_size = sizeof(ptr_key); + int ret; + + /* Use hw RNG if available */ + if (get_random_bytes_arch(_key, key_size) == key_size) { + static_branch_disable(_filled_random_ptr_key); + return 0; + } + ret = add_random_ready_callback(_ready); if (!ret) { return 0; } else if (ret == -EALREADY) { -- 2.7.4