[PATCH 0/1] Motorola CPCAP PMIC RTC
Hi, Here is a driver for the RTC found inside of the Motorola Droid 4 based on linux-next 2017021. I tried to set & get the time using hwclock and used ./tools/testing/selftests/timers/rtctest.c: $ ./rtctest RTC Driver Test Example. Counting 5 update (1/sec) interrupts from reading /dev/rtc0: 1 2 3 4 5 Again, from using select(2) on /dev/rtc: 1 2 3 4 5 Current RTC date/time is 20-2-2017, 07:11:22. Alarm time now set to 07:11:27. Waiting 5 seconds for alarm... okay. Alarm rang. Periodic IRQ rate is 1Hz. Counting 20 interrupts at: 2Hz: 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 4Hz: 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 8Hz: 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 16Hz:1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 32Hz:1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 64Hz:1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 *** Test complete *** I did not include a patch for omap4-droid4-xt894.dts, since the CPCAP DT entry has not yet been added. The following DT snippet can be used for testing on droid4: { cpcap_rtc: rtc { compatible = "motorola,cpcap-rtc"; interrupt-parent = <>; interrupts = <39 IRQ_TYPE_NONE>, <26 IRQ_TYPE_NONE>; }; }; -- Sebastian Sebastian Reichel (1): rtc: cpcap: new rtc driver .../devicetree/bindings/rtc/cpcap-rtc.txt | 13 + drivers/rtc/Kconfig| 7 + drivers/rtc/Makefile | 1 + drivers/rtc/rtc-cpcap.c| 318 + 4 files changed, 339 insertions(+) create mode 100644 Documentation/devicetree/bindings/rtc/cpcap-rtc.txt create mode 100644 drivers/rtc/rtc-cpcap.c -- 2.11.0
[PATCH 0/1] Motorola CPCAP PMIC RTC
Hi, Here is a driver for the RTC found inside of the Motorola Droid 4 based on linux-next 2017021. I tried to set & get the time using hwclock and used ./tools/testing/selftests/timers/rtctest.c: $ ./rtctest RTC Driver Test Example. Counting 5 update (1/sec) interrupts from reading /dev/rtc0: 1 2 3 4 5 Again, from using select(2) on /dev/rtc: 1 2 3 4 5 Current RTC date/time is 20-2-2017, 07:11:22. Alarm time now set to 07:11:27. Waiting 5 seconds for alarm... okay. Alarm rang. Periodic IRQ rate is 1Hz. Counting 20 interrupts at: 2Hz: 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 4Hz: 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 8Hz: 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 16Hz:1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 32Hz:1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 64Hz:1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 *** Test complete *** I did not include a patch for omap4-droid4-xt894.dts, since the CPCAP DT entry has not yet been added. The following DT snippet can be used for testing on droid4: { cpcap_rtc: rtc { compatible = "motorola,cpcap-rtc"; interrupt-parent = <>; interrupts = <39 IRQ_TYPE_NONE>, <26 IRQ_TYPE_NONE>; }; }; -- Sebastian Sebastian Reichel (1): rtc: cpcap: new rtc driver .../devicetree/bindings/rtc/cpcap-rtc.txt | 13 + drivers/rtc/Kconfig| 7 + drivers/rtc/Makefile | 1 + drivers/rtc/rtc-cpcap.c| 318 + 4 files changed, 339 insertions(+) create mode 100644 Documentation/devicetree/bindings/rtc/cpcap-rtc.txt create mode 100644 drivers/rtc/rtc-cpcap.c -- 2.11.0
[PATCH 1/1] rtc: cpcap: new rtc driver
This driver supports the Motorola CPCAP PMIC found on some of Motorola's mobile phones, such as the Droid 4. Signed-off-by: Sebastian Reichel--- .../devicetree/bindings/rtc/cpcap-rtc.txt | 13 + drivers/rtc/Kconfig| 7 + drivers/rtc/Makefile | 1 + drivers/rtc/rtc-cpcap.c| 318 + 4 files changed, 339 insertions(+) create mode 100644 Documentation/devicetree/bindings/rtc/cpcap-rtc.txt create mode 100644 drivers/rtc/rtc-cpcap.c diff --git a/Documentation/devicetree/bindings/rtc/cpcap-rtc.txt b/Documentation/devicetree/bindings/rtc/cpcap-rtc.txt new file mode 100644 index ..2709c32baf2c --- /dev/null +++ b/Documentation/devicetree/bindings/rtc/cpcap-rtc.txt @@ -0,0 +1,13 @@ +Motorola CPCAP PMIC RTC + + +Requires node properties: +- compatible: should contain "motorola,cpcap-rtc" +- interrupts: An interrupt specifier for alarm and 1 Hz irq + +Example: + +cpcap_rtc: rtc { + compatible = "motorola,cpcap-rtc"; + interrupts = <39 IRQ_TYPE_NONE>, <26 IRQ_TYPE_NONE>; +}; diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index ee1b0e9dde79..050bec749fae 100644 --- a/drivers/rtc/Kconfig +++ b/drivers/rtc/Kconfig @@ -1731,6 +1731,13 @@ config RTC_DRV_STM32 This driver can also be built as a module, if so, the module will be called "rtc-stm32". +config RTC_DRV_CPCAP + depends on MFD_CPCAP + tristate "Motorola CPCAP RTC" + help + Say y here for CPCAP rtc found on some Motorola phones + and tablets such as Droid 4. + comment "HID Sensor RTC drivers" config RTC_DRV_HID_SENSOR_TIME diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile index f07297b1460a..13857d2fce09 100644 --- a/drivers/rtc/Makefile +++ b/drivers/rtc/Makefile @@ -40,6 +40,7 @@ obj-$(CONFIG_RTC_DRV_BQ32K) += rtc-bq32k.o obj-$(CONFIG_RTC_DRV_BQ4802) += rtc-bq4802.o obj-$(CONFIG_RTC_DRV_CMOS) += rtc-cmos.o obj-$(CONFIG_RTC_DRV_COH901331)+= rtc-coh901331.o +obj-$(CONFIG_RTC_DRV_CPCAP)+= rtc-cpcap.o obj-$(CONFIG_RTC_DRV_DA9052) += rtc-da9052.o obj-$(CONFIG_RTC_DRV_DA9055) += rtc-da9055.o obj-$(CONFIG_RTC_DRV_DA9063) += rtc-da9063.o diff --git a/drivers/rtc/rtc-cpcap.c b/drivers/rtc/rtc-cpcap.c new file mode 100644 index ..815beca843ce --- /dev/null +++ b/drivers/rtc/rtc-cpcap.c @@ -0,0 +1,318 @@ +/* + * Motorola CPCAP PMIC RTC driver + * + * Based on cpcap-regulator.c from Motorola Linux kernel tree + * Copyright (C) 2009 Motorola, Inc. + * + * Rewritten for mainline kernel + * - use DT + * - use regmap + * - use standard interrupt framework + * - use managed device resources + * - remove custom "secure clock daemon" helpers + * + * Copyright (C) 2017 Sebastian Reichel + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define SECS_PER_DAY 86400 +#define DAY_MASK 0x7FFF +#define TOD1_MASK 0x00FF +#define TOD2_MASK 0x01FF + +struct cpcap_time { + int day; + int tod1; + int tod2; +}; + +struct cpcap_rtc { + struct regmap *regmap; + struct rtc_device *rtc_dev; + u16 vendor; + int alarm_irq; + bool alarm_enabled; + int update_irq; + bool update_enabled; +}; + +static void cpcap2rtc_time(struct rtc_time *rtc, struct cpcap_time *cpcap) +{ + unsigned long int tod; + unsigned long int time; + + tod = (cpcap->tod1 & TOD1_MASK) | ((cpcap->tod2 & TOD2_MASK) << 8); + time = tod + ((cpcap->day & DAY_MASK) * SECS_PER_DAY); + + rtc_time_to_tm(time, rtc); +} + +static void rtc2cpcap_time(struct cpcap_time *cpcap, struct rtc_time *rtc) +{ + unsigned long time; + + rtc_tm_to_time(rtc, ); + + cpcap->day = time / SECS_PER_DAY; + time %= SECS_PER_DAY; + cpcap->tod2 = (time >> 8) & TOD2_MASK; + cpcap->tod1 = time & TOD1_MASK; +} + +static int cpcap_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled) +{ + struct cpcap_rtc *rtc = dev_get_drvdata(dev); + + if (rtc->alarm_enabled == enabled) + return 0; + + if (enabled) + enable_irq(rtc->alarm_irq); + else + disable_irq(rtc->alarm_irq); + + rtc->alarm_enabled = !!enabled; + + return 0; +} + +static int cpcap_rtc_read_time(struct device *dev, struct rtc_time *tm) +{ +
[PATCH 1/1] rtc: cpcap: new rtc driver
This driver supports the Motorola CPCAP PMIC found on some of Motorola's mobile phones, such as the Droid 4. Signed-off-by: Sebastian Reichel --- .../devicetree/bindings/rtc/cpcap-rtc.txt | 13 + drivers/rtc/Kconfig| 7 + drivers/rtc/Makefile | 1 + drivers/rtc/rtc-cpcap.c| 318 + 4 files changed, 339 insertions(+) create mode 100644 Documentation/devicetree/bindings/rtc/cpcap-rtc.txt create mode 100644 drivers/rtc/rtc-cpcap.c diff --git a/Documentation/devicetree/bindings/rtc/cpcap-rtc.txt b/Documentation/devicetree/bindings/rtc/cpcap-rtc.txt new file mode 100644 index ..2709c32baf2c --- /dev/null +++ b/Documentation/devicetree/bindings/rtc/cpcap-rtc.txt @@ -0,0 +1,13 @@ +Motorola CPCAP PMIC RTC + + +Requires node properties: +- compatible: should contain "motorola,cpcap-rtc" +- interrupts: An interrupt specifier for alarm and 1 Hz irq + +Example: + +cpcap_rtc: rtc { + compatible = "motorola,cpcap-rtc"; + interrupts = <39 IRQ_TYPE_NONE>, <26 IRQ_TYPE_NONE>; +}; diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index ee1b0e9dde79..050bec749fae 100644 --- a/drivers/rtc/Kconfig +++ b/drivers/rtc/Kconfig @@ -1731,6 +1731,13 @@ config RTC_DRV_STM32 This driver can also be built as a module, if so, the module will be called "rtc-stm32". +config RTC_DRV_CPCAP + depends on MFD_CPCAP + tristate "Motorola CPCAP RTC" + help + Say y here for CPCAP rtc found on some Motorola phones + and tablets such as Droid 4. + comment "HID Sensor RTC drivers" config RTC_DRV_HID_SENSOR_TIME diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile index f07297b1460a..13857d2fce09 100644 --- a/drivers/rtc/Makefile +++ b/drivers/rtc/Makefile @@ -40,6 +40,7 @@ obj-$(CONFIG_RTC_DRV_BQ32K) += rtc-bq32k.o obj-$(CONFIG_RTC_DRV_BQ4802) += rtc-bq4802.o obj-$(CONFIG_RTC_DRV_CMOS) += rtc-cmos.o obj-$(CONFIG_RTC_DRV_COH901331)+= rtc-coh901331.o +obj-$(CONFIG_RTC_DRV_CPCAP)+= rtc-cpcap.o obj-$(CONFIG_RTC_DRV_DA9052) += rtc-da9052.o obj-$(CONFIG_RTC_DRV_DA9055) += rtc-da9055.o obj-$(CONFIG_RTC_DRV_DA9063) += rtc-da9063.o diff --git a/drivers/rtc/rtc-cpcap.c b/drivers/rtc/rtc-cpcap.c new file mode 100644 index ..815beca843ce --- /dev/null +++ b/drivers/rtc/rtc-cpcap.c @@ -0,0 +1,318 @@ +/* + * Motorola CPCAP PMIC RTC driver + * + * Based on cpcap-regulator.c from Motorola Linux kernel tree + * Copyright (C) 2009 Motorola, Inc. + * + * Rewritten for mainline kernel + * - use DT + * - use regmap + * - use standard interrupt framework + * - use managed device resources + * - remove custom "secure clock daemon" helpers + * + * Copyright (C) 2017 Sebastian Reichel + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define SECS_PER_DAY 86400 +#define DAY_MASK 0x7FFF +#define TOD1_MASK 0x00FF +#define TOD2_MASK 0x01FF + +struct cpcap_time { + int day; + int tod1; + int tod2; +}; + +struct cpcap_rtc { + struct regmap *regmap; + struct rtc_device *rtc_dev; + u16 vendor; + int alarm_irq; + bool alarm_enabled; + int update_irq; + bool update_enabled; +}; + +static void cpcap2rtc_time(struct rtc_time *rtc, struct cpcap_time *cpcap) +{ + unsigned long int tod; + unsigned long int time; + + tod = (cpcap->tod1 & TOD1_MASK) | ((cpcap->tod2 & TOD2_MASK) << 8); + time = tod + ((cpcap->day & DAY_MASK) * SECS_PER_DAY); + + rtc_time_to_tm(time, rtc); +} + +static void rtc2cpcap_time(struct cpcap_time *cpcap, struct rtc_time *rtc) +{ + unsigned long time; + + rtc_tm_to_time(rtc, ); + + cpcap->day = time / SECS_PER_DAY; + time %= SECS_PER_DAY; + cpcap->tod2 = (time >> 8) & TOD2_MASK; + cpcap->tod1 = time & TOD1_MASK; +} + +static int cpcap_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled) +{ + struct cpcap_rtc *rtc = dev_get_drvdata(dev); + + if (rtc->alarm_enabled == enabled) + return 0; + + if (enabled) + enable_irq(rtc->alarm_irq); + else + disable_irq(rtc->alarm_irq); + + rtc->alarm_enabled = !!enabled; + + return 0; +} + +static int cpcap_rtc_read_time(struct device *dev, struct rtc_time *tm) +{ + struct cpcap_rtc *rtc; +
Re: [GIT PULL] Block pull request for- 4.11-rc1
On Sun, Feb 19, 2017 at 06:15:41PM -0700, Jens Axboe wrote: > I don't think that's a regression in this series, it just triggers more easily > with this series. The BLOCK_PC removal fixes aren't touching device life times > at all. Yes. > That said, we will look into this again, of course. Christoph, any idea? No idea really - this seems so far away from the code touched, and there are no obvious signs for a memory scamble from another object touched that I think if it really bisects down to that issue it must be a timing issue. But reading Bart's message again: Did you actually bisect it down to the is commit? Or just test the whole tree? Between the 4.10-rc5 merge and all the block tree there might a few more likely suspects like the scsi bdi lifetime fixes that James mentioned.
Re: [GIT PULL] Block pull request for- 4.11-rc1
On Sun, Feb 19, 2017 at 06:15:41PM -0700, Jens Axboe wrote: > I don't think that's a regression in this series, it just triggers more easily > with this series. The BLOCK_PC removal fixes aren't touching device life times > at all. Yes. > That said, we will look into this again, of course. Christoph, any idea? No idea really - this seems so far away from the code touched, and there are no obvious signs for a memory scamble from another object touched that I think if it really bisects down to that issue it must be a timing issue. But reading Bart's message again: Did you actually bisect it down to the is commit? Or just test the whole tree? Between the 4.10-rc5 merge and all the block tree there might a few more likely suspects like the scsi bdi lifetime fixes that James mentioned.
Re: [PATCH v2] drm/rockchip: cdn-dp: Fix error handling
On 2017年02月20日 15:08, Christophe JAILLET wrote: It is likely that both 'clk_disable_unprepare()' should be called if 'pm_runtime_get_sync()' fails. Add a new label for that, because 'err_set_rate' is not meaningful in this case. Add a missing call to 'pm_runtime_put()'. Fixes: 1a0f7ed3abe2 ("drm/rockchip: cdn-dp: add cdn DP support for rk3399") Signed-off-by: Christophe JAILLETlooks good for me Reviewed-by: Mark Yao --- V2: rename label add missing call to 'pm_runtime_put_sync()' in error path --- drivers/gpu/drm/rockchip/cdn-dp-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c index 9ab67a670885..0fe1ec8b8fb1 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c @@ -111,7 +111,7 @@ static int cdn_dp_clk_enable(struct cdn_dp_device *dp) ret = pm_runtime_get_sync(dp->dev); if (ret < 0) { DRM_DEV_ERROR(dp->dev, "cannot get pm runtime %d\n", ret); - goto err_pclk; + goto err_pm_runtime_get; } reset_control_assert(dp->core_rst); @@ -133,6 +133,8 @@ static int cdn_dp_clk_enable(struct cdn_dp_device *dp) return 0; err_set_rate: + pm_runtime_put(dp->dev); +err_pm_runtime_get: clk_disable_unprepare(dp->core_clk); err_core_clk: clk_disable_unprepare(dp->pclk); -- Mark Yao
Re: [PATCH v2] drm/rockchip: cdn-dp: Fix error handling
On 2017年02月20日 15:08, Christophe JAILLET wrote: It is likely that both 'clk_disable_unprepare()' should be called if 'pm_runtime_get_sync()' fails. Add a new label for that, because 'err_set_rate' is not meaningful in this case. Add a missing call to 'pm_runtime_put()'. Fixes: 1a0f7ed3abe2 ("drm/rockchip: cdn-dp: add cdn DP support for rk3399") Signed-off-by: Christophe JAILLET looks good for me Reviewed-by: Mark Yao --- V2: rename label add missing call to 'pm_runtime_put_sync()' in error path --- drivers/gpu/drm/rockchip/cdn-dp-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c index 9ab67a670885..0fe1ec8b8fb1 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c @@ -111,7 +111,7 @@ static int cdn_dp_clk_enable(struct cdn_dp_device *dp) ret = pm_runtime_get_sync(dp->dev); if (ret < 0) { DRM_DEV_ERROR(dp->dev, "cannot get pm runtime %d\n", ret); - goto err_pclk; + goto err_pm_runtime_get; } reset_control_assert(dp->core_rst); @@ -133,6 +133,8 @@ static int cdn_dp_clk_enable(struct cdn_dp_device *dp) return 0; err_set_rate: + pm_runtime_put(dp->dev); +err_pm_runtime_get: clk_disable_unprepare(dp->core_clk); err_core_clk: clk_disable_unprepare(dp->pclk); -- Mark Yao
Re: [PATCH 1/2] staging: ks7010: Unnecessary parentheses are removed to make coder nicer.
Hi Arushi, [auto build test ERROR on staging/staging-testing] [also build test ERROR on v4.10 next-20170217] [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/Arushi-Singhal/staging-ks7010-Unnecessary-parentheses-are-removed-to-make-coder-nicer/20170220-145647 config: i386-randconfig-x012-201708 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers/staging/ks7010/ks_hostif.c: In function 'get_current_ap': >> drivers/staging/ks7010/ks_hostif.c:116:5: error: incompatible types when >> assigning to type 'struct local_ap_t *' from type 'struct local_ap_t' ap = priv->current_ap; ^ vim +116 drivers/staging/ks7010/ks_hostif.c 110 struct local_ap_t *ap; 111 union iwreq_data wrqu; 112 struct net_device *netdev = priv->net_dev; 113 int rc = 0; 114 115 DPRINTK(3, "\n"); > 116 ap = priv->current_ap; 117 118 if ((priv->connect_status & CONNECT_STATUS_MASK) == DISCONNECT_STATUS) { 119 memset(ap, 0, sizeof(struct local_ap_t)); --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH 1/2] staging: ks7010: Unnecessary parentheses are removed to make coder nicer.
Hi Arushi, [auto build test ERROR on staging/staging-testing] [also build test ERROR on v4.10 next-20170217] [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/Arushi-Singhal/staging-ks7010-Unnecessary-parentheses-are-removed-to-make-coder-nicer/20170220-145647 config: i386-randconfig-x012-201708 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers/staging/ks7010/ks_hostif.c: In function 'get_current_ap': >> drivers/staging/ks7010/ks_hostif.c:116:5: error: incompatible types when >> assigning to type 'struct local_ap_t *' from type 'struct local_ap_t' ap = priv->current_ap; ^ vim +116 drivers/staging/ks7010/ks_hostif.c 110 struct local_ap_t *ap; 111 union iwreq_data wrqu; 112 struct net_device *netdev = priv->net_dev; 113 int rc = 0; 114 115 DPRINTK(3, "\n"); > 116 ap = priv->current_ap; 117 118 if ((priv->connect_status & CONNECT_STATUS_MASK) == DISCONNECT_STATUS) { 119 memset(ap, 0, sizeof(struct local_ap_t)); --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCHv3 2/2] arch: Rename CONFIG_DEBUG_RODATA and CONFIG_DEBUG_MODULE_RONX
* Kees Cookwrote: > On Thu, Feb 16, 2017 at 2:25 PM, Pavel Machek wrote: > > Hi! > > > >> > >> -config DEBUG_RODATA > >> +config STRICT_KERNEL_RWX > >> bool "Make kernel text and rodata read-only" if > >> ARCH_OPTIONAL_KERNEL_RWX > >> depends on ARCH_HAS_STRICT_KERNEL_RWX > >> default !ARCH_OPTIONAL_KERNEL_RWX || > > > > Debug features are expected to have runtime cost, so kconfig help is > > silent about those. But there are runtime costs, right? It would be > > nice to mention them in the help text... > > It depends on the architecture. The prior help text for arm said: > > The tradeoff is that each region is padded to section-size (1MiB) > boundaries (because their permissions are different and splitting > the 1M pages into 4K ones causes TLB performance problems), which > can waste memory. > > parisc (somewhat inaccurately) said: > > This option may have a slight performance impact because a > portion of the kernel code won't be covered by a TLB anymore. > > IIUC, arm64 does what parisc is hinting at: mappings at the end are > broken down to PAGE_SIZE. On x86, IIUC, there's actually no change to > TLB performance due to how the mappings are already set up. BTW., a good strategy with RAM sizes above say 4GB would be to just round up to the next large-TLB boundary (2MB) and waste 0-2MB of RAM - which is only 0.05% of 4GB of RAM. On most workloads, especially with SSDs it's probably a positive RAM vs. performance trade-off. Thanks, Ingo
Re: [PATCHv3 2/2] arch: Rename CONFIG_DEBUG_RODATA and CONFIG_DEBUG_MODULE_RONX
* Kees Cook wrote: > On Thu, Feb 16, 2017 at 2:25 PM, Pavel Machek wrote: > > Hi! > > > >> > >> -config DEBUG_RODATA > >> +config STRICT_KERNEL_RWX > >> bool "Make kernel text and rodata read-only" if > >> ARCH_OPTIONAL_KERNEL_RWX > >> depends on ARCH_HAS_STRICT_KERNEL_RWX > >> default !ARCH_OPTIONAL_KERNEL_RWX || > > > > Debug features are expected to have runtime cost, so kconfig help is > > silent about those. But there are runtime costs, right? It would be > > nice to mention them in the help text... > > It depends on the architecture. The prior help text for arm said: > > The tradeoff is that each region is padded to section-size (1MiB) > boundaries (because their permissions are different and splitting > the 1M pages into 4K ones causes TLB performance problems), which > can waste memory. > > parisc (somewhat inaccurately) said: > > This option may have a slight performance impact because a > portion of the kernel code won't be covered by a TLB anymore. > > IIUC, arm64 does what parisc is hinting at: mappings at the end are > broken down to PAGE_SIZE. On x86, IIUC, there's actually no change to > TLB performance due to how the mappings are already set up. BTW., a good strategy with RAM sizes above say 4GB would be to just round up to the next large-TLB boundary (2MB) and waste 0-2MB of RAM - which is only 0.05% of 4GB of RAM. On most workloads, especially with SSDs it's probably a positive RAM vs. performance trade-off. Thanks, Ingo
[ANNOUNCE] linux-4.10-ck1 / MuQSS CPU scheduler 0.152
These are patches designed to improve system responsiveness and interactivity with specific emphasis on the desktop, but configurable for any workload. The patchset is mainly centred around the Multiple Queue Skiplist Scheduler, MuQSS. -ck1 patches: http://ck.kolivas.org/patches/4.0/4.10/4.10-ck1/ Git tree: https://github.com/ckolivas/linux/tree/4.10-ck Ubuntu 16.10 packages: http://ck.kolivas.org/patches/4.0/4.10/4.10-ck1/Ubuntu16.10/ Patch for those struggling with the nvidia driver and 4.10: http://ck.kolivas.org/patches/4.0/4.10/nvidia-375.39-linux-4.10.patch Full patch list: 0001-MuQSS-150-pre-merge.patch 0002-Remove-quick-ramp-up-which-may-have-been-keeping-CPU.patch 0003-Fix-iso-tick-calculation.patch 0004-Prevent-negative-value-causing-further-overflow-in-t.patch 0005-Demote-no-cpumask-message-to-info.patch 0006-Bump-muqss-version-to-0.152.patch 0007-Make-preemptible-kernel-default.patch 0008-Expose-vmsplit-option-for-our-poor-32bit-users.patch 0009-Implement-min-and-msec-hrtimeout-un-interruptible-sc.patch 0010-Special-case-calls-of-schedule_timeout-1-to-use-the-.patch 0011-Convert-msleep-to-use-hrtimers-when-active.patch 0012-Replace-all-schedule-timeout-1-with-schedule_min_hrt.patch 0013-Change-all-schedule_timeout-with-msecs_to_jiffies-po.patch 0014-Replace-all-calls-to-schedule_timeout_interruptible-.patch 0015-Replace-all-calls-to-schedule_timeout_uninterruptibl.patch 0016-Fix-build-for-disabled-highres-timers-with-hrtimeout.patch 0017-Make-hrtimer-granularity-and-minimum-hrtimeout-confi.patch 0018-Make-threaded-IRQs-optionally-the-default-which-can-.patch 0019-Reinstate-default-Hz-of-100-in-combination-with-MuQS.patch 0020-Don-t-use-hrtimer-overlay-when-pm_freezing-since-som.patch 0021-Make-writeback-throttling-default-enabled.patch 0022-Swap-sucks.patch 0023-BFQ-version-8.patch 0024-Make-BFQ-default-IO-scheduler.patch 0025-Add-ck1-version.patch For a brief description of each patch without trawling the git tree, each patch can be found as a quilt series here: http://ck.kolivas.org/patches/4.0/4.10/4.10-ck1/patches/ Hacking blog: http://ck-hack.blogspot.com/ Simple homepage: http://kernel.kolivas.org Enjoy! お楽しみ下さい -- -ck
[ANNOUNCE] linux-4.10-ck1 / MuQSS CPU scheduler 0.152
These are patches designed to improve system responsiveness and interactivity with specific emphasis on the desktop, but configurable for any workload. The patchset is mainly centred around the Multiple Queue Skiplist Scheduler, MuQSS. -ck1 patches: http://ck.kolivas.org/patches/4.0/4.10/4.10-ck1/ Git tree: https://github.com/ckolivas/linux/tree/4.10-ck Ubuntu 16.10 packages: http://ck.kolivas.org/patches/4.0/4.10/4.10-ck1/Ubuntu16.10/ Patch for those struggling with the nvidia driver and 4.10: http://ck.kolivas.org/patches/4.0/4.10/nvidia-375.39-linux-4.10.patch Full patch list: 0001-MuQSS-150-pre-merge.patch 0002-Remove-quick-ramp-up-which-may-have-been-keeping-CPU.patch 0003-Fix-iso-tick-calculation.patch 0004-Prevent-negative-value-causing-further-overflow-in-t.patch 0005-Demote-no-cpumask-message-to-info.patch 0006-Bump-muqss-version-to-0.152.patch 0007-Make-preemptible-kernel-default.patch 0008-Expose-vmsplit-option-for-our-poor-32bit-users.patch 0009-Implement-min-and-msec-hrtimeout-un-interruptible-sc.patch 0010-Special-case-calls-of-schedule_timeout-1-to-use-the-.patch 0011-Convert-msleep-to-use-hrtimers-when-active.patch 0012-Replace-all-schedule-timeout-1-with-schedule_min_hrt.patch 0013-Change-all-schedule_timeout-with-msecs_to_jiffies-po.patch 0014-Replace-all-calls-to-schedule_timeout_interruptible-.patch 0015-Replace-all-calls-to-schedule_timeout_uninterruptibl.patch 0016-Fix-build-for-disabled-highres-timers-with-hrtimeout.patch 0017-Make-hrtimer-granularity-and-minimum-hrtimeout-confi.patch 0018-Make-threaded-IRQs-optionally-the-default-which-can-.patch 0019-Reinstate-default-Hz-of-100-in-combination-with-MuQS.patch 0020-Don-t-use-hrtimer-overlay-when-pm_freezing-since-som.patch 0021-Make-writeback-throttling-default-enabled.patch 0022-Swap-sucks.patch 0023-BFQ-version-8.patch 0024-Make-BFQ-default-IO-scheduler.patch 0025-Add-ck1-version.patch For a brief description of each patch without trawling the git tree, each patch can be found as a quilt series here: http://ck.kolivas.org/patches/4.0/4.10/4.10-ck1/patches/ Hacking blog: http://ck-hack.blogspot.com/ Simple homepage: http://kernel.kolivas.org Enjoy! お楽しみ下さい -- -ck
Re: [PATCHv2 4/5] perf stat: Add -a as a default target
On Sat, Feb 18, 2017 at 06:52:25PM +0100, Borislav Petkov wrote: > On Fri, Feb 17, 2017 at 06:48:13PM +0100, Boris Petkov wrote: > > LGTM. > > > > Acked-by: me > > Well, it looks good but actually trying it is a different story. For > example: > > $ ./perf stat -e amd_nb/event=0xe0,umask=0x1f/ sleep 1 > > still says because argc is not 0. > > So how about the below diff instead? > > $ ./perf stat -e amd_nb/event=0xe0,umask=0x1f/ > > without args dumps the usage message and > > $ ./perf stat -e amd_nb/event=0xe0,umask=0x1f/ sleep 1 > > actually does the system-wide thing: > > Performance counter stats for 'system wide': > >196,469 amd_nb/event=0xe0,umask=0x1f/ > >1.001815180 seconds time elapsed > > Hmmm? ugh, I thought it was too easy ;-) it looks good to me thanks, jirka
Re: [PATCHv2 4/5] perf stat: Add -a as a default target
On Sat, Feb 18, 2017 at 06:52:25PM +0100, Borislav Petkov wrote: > On Fri, Feb 17, 2017 at 06:48:13PM +0100, Boris Petkov wrote: > > LGTM. > > > > Acked-by: me > > Well, it looks good but actually trying it is a different story. For > example: > > $ ./perf stat -e amd_nb/event=0xe0,umask=0x1f/ sleep 1 > > still says because argc is not 0. > > So how about the below diff instead? > > $ ./perf stat -e amd_nb/event=0xe0,umask=0x1f/ > > without args dumps the usage message and > > $ ./perf stat -e amd_nb/event=0xe0,umask=0x1f/ sleep 1 > > actually does the system-wide thing: > > Performance counter stats for 'system wide': > >196,469 amd_nb/event=0xe0,umask=0x1f/ > >1.001815180 seconds time elapsed > > Hmmm? ugh, I thought it was too easy ;-) it looks good to me thanks, jirka
[PATCH v2] drm/rockchip: cdn-dp: Fix error handling
It is likely that both 'clk_disable_unprepare()' should be called if 'pm_runtime_get_sync()' fails. Add a new label for that, because 'err_set_rate' is not meaningful in this case. Add a missing call to 'pm_runtime_put()'. Fixes: 1a0f7ed3abe2 ("drm/rockchip: cdn-dp: add cdn DP support for rk3399") Signed-off-by: Christophe JAILLET--- V2: rename label add missing call to 'pm_runtime_put_sync()' in error path --- drivers/gpu/drm/rockchip/cdn-dp-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c index 9ab67a670885..0fe1ec8b8fb1 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c @@ -111,7 +111,7 @@ static int cdn_dp_clk_enable(struct cdn_dp_device *dp) ret = pm_runtime_get_sync(dp->dev); if (ret < 0) { DRM_DEV_ERROR(dp->dev, "cannot get pm runtime %d\n", ret); - goto err_pclk; + goto err_pm_runtime_get; } reset_control_assert(dp->core_rst); @@ -133,6 +133,8 @@ static int cdn_dp_clk_enable(struct cdn_dp_device *dp) return 0; err_set_rate: + pm_runtime_put(dp->dev); +err_pm_runtime_get: clk_disable_unprepare(dp->core_clk); err_core_clk: clk_disable_unprepare(dp->pclk); -- 2.9.3
[PATCH v2] drm/rockchip: cdn-dp: Fix error handling
It is likely that both 'clk_disable_unprepare()' should be called if 'pm_runtime_get_sync()' fails. Add a new label for that, because 'err_set_rate' is not meaningful in this case. Add a missing call to 'pm_runtime_put()'. Fixes: 1a0f7ed3abe2 ("drm/rockchip: cdn-dp: add cdn DP support for rk3399") Signed-off-by: Christophe JAILLET --- V2: rename label add missing call to 'pm_runtime_put_sync()' in error path --- drivers/gpu/drm/rockchip/cdn-dp-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c index 9ab67a670885..0fe1ec8b8fb1 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c @@ -111,7 +111,7 @@ static int cdn_dp_clk_enable(struct cdn_dp_device *dp) ret = pm_runtime_get_sync(dp->dev); if (ret < 0) { DRM_DEV_ERROR(dp->dev, "cannot get pm runtime %d\n", ret); - goto err_pclk; + goto err_pm_runtime_get; } reset_control_assert(dp->core_rst); @@ -133,6 +133,8 @@ static int cdn_dp_clk_enable(struct cdn_dp_device *dp) return 0; err_set_rate: + pm_runtime_put(dp->dev); +err_pm_runtime_get: clk_disable_unprepare(dp->core_clk); err_core_clk: clk_disable_unprepare(dp->pclk); -- 2.9.3
[PATCH v10 1/3] dt-bindings: Add support for samsung s6e3ha2 panel binding
The Samsung s6e3ha2 is a 5.7" 1440x2560 AMOLED panel connected using MIPI-DSI interfaces. Signed-off-by: Donghwa LeeSigned-off-by: Hyungwon Hwang Signed-off-by: Hoegeun Kwon Reviewed-by: Andrzej Hajda Acked-by: Rob Herring --- .../bindings/display/panel/samsung,s6e3ha2.txt | 28 ++ 1 file changed, 28 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt diff --git a/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt new file mode 100644 index 000..18854f4 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt @@ -0,0 +1,28 @@ +Samsung S6E3HA2 5.7" 1440x2560 AMOLED panel + +Required properties: + - compatible: "samsung,s6e3ha2" + - reg: the virtual channel number of a DSI peripheral + - vdd3-supply: I/O voltage supply + - vci-supply: voltage supply for analog circuits + - reset-gpios: a GPIO spec for the reset pin (active low) + - enable-gpios: a GPIO spec for the panel enable pin (active high) + +Optional properties: + - te-gpios: a GPIO spec for the tearing effect synchronization signal +gpio pin (active high) + +Example: + { + ... + + panel@0 { + compatible = "samsung,s6e3ha2"; + reg = <0>; + vdd3-supply = <_reg>; + vci-supply = <_reg>; + reset-gpios = < 0 GPIO_ACTIVE_LOW>; + enable-gpios = < 5 GPIO_ACTIVE_HIGH>; + te-gpios = < 3 GPIO_ACTIVE_HIGH>; + }; +}; -- 1.9.1
[PATCH v10 0/3] Add support for the S6E3HA2 panel on TM2 board
Dear Thierry, I understand that your opinion is: It is better to handle the error every time it is input to the register, rather than error handling at once in the struct using error. This not only makes the code easier to maintain, but also reduces unnecessary computation. So I modified the panel driver to code-by-code error handling. If this is not your opinion, could you tell me what your opinion? Best Regards, Hoegeun Changes for V10: - Fixed code-by-code error handling. Changes for V9: - Fixed the te-gpio to optional in bindings Changes for V8: - Applied below two patches: (drm/exynos) : drm/exynos: mic: Add mode_set callback function : drm/exynos: mic: Fix parse_dt function - The dt-binding patch and driver patch were divided. - Rebase these patches on samsung SoC tree[1] and tm2 touckey patch[2]. Change for V7: - Fixed the mode_set callback function of mic device driver. because the mic register is initialized when entering suspend mode, so should set the reg value whenever pre_enable is called. Changes for V6: - Fixed the parse_dt function of dsi device driver. - Removed OF graph of panel in DT and DT binding document. - Fixed the s6e3ha2 panel device driver. - Fixed from number size to ARRAY_SIZE(). - Fixed error handling in mipi_dsi_dcs_* functions. - Fixed the clock of display_mode. - Removed unnecessary casting and error log. Change for V5: - The V5 has only one fix in V4 below. - Removed the enable check of the mic driver in mode_set callback, because mode_set should be performed every time. Changes for V4: - Removed display-timings in devicetree, the display-timings has been fixed to be provided by the device driver. - Added the mode_set callback function into exynos_drm_mic, because the exynos_drm_mic driver can not parse a videomode struct by removing the display-timings from the devicetree. Changes for V3: - In the DT binding document, made it clearly that the panel is a child node of dsi. - Fix reset-gpio active from high to low. - Is the OF graph saying related to patch2? Althogh the panel is a child of dsi, I think OF graph necessary. because if a remote-endpoint is not specified, the dsi also panel is not probed. - The display-timings has been fixed to be provided by the device driver. however, I think display-timings is necessary in dts. because if dts does not have display-timings, dsi will not load. Changes for V2: - Fixed the samsung,s6e3ha2.txt DT document. - Added active high or low after the description of the GPIOs. - Removed the reg and added a description of the virtual channel number of a DSI peripheral. Depends on: [1] https://git.kernel.org/cgit/linux/kernel/git/krzk/linux.git/ (for-next branch) [2] https://patchwork.kernel.org/patch/9504131/ - ("arm64: dts: exynos: Add tm2 touchkey node") Hoegeun Kwon (2): dt-bindings: Add support for samsung s6e3ha2 panel binding drm/panel: Add support for S6E3HA2 panel driver on TM2 board Hyungwon Hwang (1): arm64: dts: exynos: Add support for S6E3HA2 panel device on TM2 board .../bindings/display/panel/samsung,s6e3ha2.txt | 28 + arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 12 + drivers/gpu/drm/panel/Kconfig | 6 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c | 739 + 5 files changed, 786 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c -- 1.9.1
[PATCH v10 2/3] drm/panel: Add support for S6E3HA2 panel driver on TM2 board
This patch add support for MIPI-DSI based S6E3HA2 AMOLED panel driver. This panel has 1440x2560 resolution in 5.7-inch physical panel in the TM2 device. Signed-off-by: Donghwa LeeSigned-off-by: Hyungwon Hwang Signed-off-by: Hoegeun Kwon Tested-by: Chanwoo Choi Reviewed-by: Andrzej Hajda --- drivers/gpu/drm/panel/Kconfig | 6 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c | 739 ++ 3 files changed, 746 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 62aba97..d913c83 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -52,6 +52,12 @@ config DRM_PANEL_PANASONIC_VVX10F034N00 WUXGA (1920x1200) Novatek NT1397-based DSI panel as found in some Xperia Z2 tablets +config DRM_PANEL_SAMSUNG_S6E3HA2 + tristate "Samsung S6E3HA2 DSI video mode panel" + depends on OF + depends on DRM_MIPI_DSI + select VIDEOMODE_HELPERS + config DRM_PANEL_SAMSUNG_S6E8AA0 tristate "Samsung S6E8AA0 DSI video mode panel" depends on OF diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index a5c7ec0..1d483b0 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -3,6 +3,7 @@ obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += panel-panasonic-vvx10f034n00.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o +obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c new file mode 100644 index 000..4cc08d7 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c @@ -0,0 +1,739 @@ +/* + * MIPI-DSI based s6e3ha2 AMOLED 5.7 inch panel driver. + * + * Copyright (c) 2016 Samsung Electronics Co., Ltd. + * Donghwa Lee + * Hyungwon Hwang + * Hoegeun Kwon + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include + +#define S6E3HA2_MIN_BRIGHTNESS 0 +#define S6E3HA2_MAX_BRIGHTNESS 100 +#define S6E3HA2_DEFAULT_BRIGHTNESS 80 + +#define S6E3HA2_NUM_GAMMA_STEPS46 +#define S6E3HA2_GAMMA_CMD_CNT 35 +#define S6E3HA2_VINT_STATUS_MAX10 + +static const u8 gamma_tbl[S6E3HA2_NUM_GAMMA_STEPS][S6E3HA2_GAMMA_CMD_CNT] = { + { 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x82, 0x83, + 0x85, 0x88, 0x8b, 0x8b, 0x84, 0x88, 0x82, 0x82, 0x89, 0x86, 0x8c, + 0x94, 0x84, 0xb1, 0xaf, 0x8e, 0xcf, 0xad, 0xc9, 0x00, 0x00, 0x00, + 0x00, 0x00 }, + { 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x84, 0x84, + 0x85, 0x87, 0x8b, 0x8a, 0x84, 0x88, 0x82, 0x82, 0x89, 0x86, 0x8a, + 0x93, 0x84, 0xb0, 0xae, 0x8e, 0xc9, 0xa8, 0xc5, 0x00, 0x00, 0x00, + 0x00, 0x00 }, + { 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x83, 0x83, + 0x85, 0x86, 0x8a, 0x8a, 0x84, 0x88, 0x81, 0x84, 0x8a, 0x88, 0x8a, + 0x91, 0x84, 0xb1, 0xae, 0x8b, 0xd5, 0xb2, 0xcc, 0x00, 0x00, 0x00, + 0x00, 0x00 }, + { 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x83, 0x83, + 0x85, 0x86, 0x8a, 0x8a, 0x84, 0x87, 0x81, 0x84, 0x8a, 0x87, 0x8a, + 0x91, 0x85, 0xae, 0xac, 0x8a, 0xc3, 0xa3, 0xc0, 0x00, 0x00, 0x00, + 0x00, 0x00 }, + { 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x88, 0x86, 0x87, 0x85, 0x85, + 0x86, 0x85, 0x88, 0x89, 0x84, 0x89, 0x82, 0x84, 0x87, 0x85, 0x8b, + 0x91, 0x88, 0xad, 0xab, 0x8a, 0xb7, 0x9b, 0xb6, 0x00, 0x00, 0x00, + 0x00, 0x00 }, + { 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x83, 0x83, + 0x85, 0x86, 0x89, 0x8a, 0x84, 0x89, 0x83, 0x83, 0x86, 0x84, 0x8b, + 0x90, 0x84, 0xb0, 0xae, 0x8b, 0xce, 0xad, 0xc8, 0x00, 0x00, 0x00, + 0x00, 0x00 }, + { 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x83, 0x83, + 0x85, 0x87, 0x89, 0x8a, 0x83, 0x87, 0x82, 0x85, 0x88, 0x87, 0x89, + 0x8f, 0x84, 0xac, 0xaa, 0x89, 0xb1, 0x98, 0xaf, 0x00, 0x00, 0x00, + 0x00, 0x00 }, + { 0x00, 0xb8, 0x00,
[PATCH v10 1/3] dt-bindings: Add support for samsung s6e3ha2 panel binding
The Samsung s6e3ha2 is a 5.7" 1440x2560 AMOLED panel connected using MIPI-DSI interfaces. Signed-off-by: Donghwa Lee Signed-off-by: Hyungwon Hwang Signed-off-by: Hoegeun Kwon Reviewed-by: Andrzej Hajda Acked-by: Rob Herring --- .../bindings/display/panel/samsung,s6e3ha2.txt | 28 ++ 1 file changed, 28 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt diff --git a/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt new file mode 100644 index 000..18854f4 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt @@ -0,0 +1,28 @@ +Samsung S6E3HA2 5.7" 1440x2560 AMOLED panel + +Required properties: + - compatible: "samsung,s6e3ha2" + - reg: the virtual channel number of a DSI peripheral + - vdd3-supply: I/O voltage supply + - vci-supply: voltage supply for analog circuits + - reset-gpios: a GPIO spec for the reset pin (active low) + - enable-gpios: a GPIO spec for the panel enable pin (active high) + +Optional properties: + - te-gpios: a GPIO spec for the tearing effect synchronization signal +gpio pin (active high) + +Example: + { + ... + + panel@0 { + compatible = "samsung,s6e3ha2"; + reg = <0>; + vdd3-supply = <_reg>; + vci-supply = <_reg>; + reset-gpios = < 0 GPIO_ACTIVE_LOW>; + enable-gpios = < 5 GPIO_ACTIVE_HIGH>; + te-gpios = < 3 GPIO_ACTIVE_HIGH>; + }; +}; -- 1.9.1
[PATCH v10 0/3] Add support for the S6E3HA2 panel on TM2 board
Dear Thierry, I understand that your opinion is: It is better to handle the error every time it is input to the register, rather than error handling at once in the struct using error. This not only makes the code easier to maintain, but also reduces unnecessary computation. So I modified the panel driver to code-by-code error handling. If this is not your opinion, could you tell me what your opinion? Best Regards, Hoegeun Changes for V10: - Fixed code-by-code error handling. Changes for V9: - Fixed the te-gpio to optional in bindings Changes for V8: - Applied below two patches: (drm/exynos) : drm/exynos: mic: Add mode_set callback function : drm/exynos: mic: Fix parse_dt function - The dt-binding patch and driver patch were divided. - Rebase these patches on samsung SoC tree[1] and tm2 touckey patch[2]. Change for V7: - Fixed the mode_set callback function of mic device driver. because the mic register is initialized when entering suspend mode, so should set the reg value whenever pre_enable is called. Changes for V6: - Fixed the parse_dt function of dsi device driver. - Removed OF graph of panel in DT and DT binding document. - Fixed the s6e3ha2 panel device driver. - Fixed from number size to ARRAY_SIZE(). - Fixed error handling in mipi_dsi_dcs_* functions. - Fixed the clock of display_mode. - Removed unnecessary casting and error log. Change for V5: - The V5 has only one fix in V4 below. - Removed the enable check of the mic driver in mode_set callback, because mode_set should be performed every time. Changes for V4: - Removed display-timings in devicetree, the display-timings has been fixed to be provided by the device driver. - Added the mode_set callback function into exynos_drm_mic, because the exynos_drm_mic driver can not parse a videomode struct by removing the display-timings from the devicetree. Changes for V3: - In the DT binding document, made it clearly that the panel is a child node of dsi. - Fix reset-gpio active from high to low. - Is the OF graph saying related to patch2? Althogh the panel is a child of dsi, I think OF graph necessary. because if a remote-endpoint is not specified, the dsi also panel is not probed. - The display-timings has been fixed to be provided by the device driver. however, I think display-timings is necessary in dts. because if dts does not have display-timings, dsi will not load. Changes for V2: - Fixed the samsung,s6e3ha2.txt DT document. - Added active high or low after the description of the GPIOs. - Removed the reg and added a description of the virtual channel number of a DSI peripheral. Depends on: [1] https://git.kernel.org/cgit/linux/kernel/git/krzk/linux.git/ (for-next branch) [2] https://patchwork.kernel.org/patch/9504131/ - ("arm64: dts: exynos: Add tm2 touchkey node") Hoegeun Kwon (2): dt-bindings: Add support for samsung s6e3ha2 panel binding drm/panel: Add support for S6E3HA2 panel driver on TM2 board Hyungwon Hwang (1): arm64: dts: exynos: Add support for S6E3HA2 panel device on TM2 board .../bindings/display/panel/samsung,s6e3ha2.txt | 28 + arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 12 + drivers/gpu/drm/panel/Kconfig | 6 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c | 739 + 5 files changed, 786 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c -- 1.9.1
[PATCH v10 2/3] drm/panel: Add support for S6E3HA2 panel driver on TM2 board
This patch add support for MIPI-DSI based S6E3HA2 AMOLED panel driver. This panel has 1440x2560 resolution in 5.7-inch physical panel in the TM2 device. Signed-off-by: Donghwa Lee Signed-off-by: Hyungwon Hwang Signed-off-by: Hoegeun Kwon Tested-by: Chanwoo Choi Reviewed-by: Andrzej Hajda --- drivers/gpu/drm/panel/Kconfig | 6 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c | 739 ++ 3 files changed, 746 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 62aba97..d913c83 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -52,6 +52,12 @@ config DRM_PANEL_PANASONIC_VVX10F034N00 WUXGA (1920x1200) Novatek NT1397-based DSI panel as found in some Xperia Z2 tablets +config DRM_PANEL_SAMSUNG_S6E3HA2 + tristate "Samsung S6E3HA2 DSI video mode panel" + depends on OF + depends on DRM_MIPI_DSI + select VIDEOMODE_HELPERS + config DRM_PANEL_SAMSUNG_S6E8AA0 tristate "Samsung S6E8AA0 DSI video mode panel" depends on OF diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index a5c7ec0..1d483b0 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -3,6 +3,7 @@ obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += panel-panasonic-vvx10f034n00.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o +obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c new file mode 100644 index 000..4cc08d7 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c @@ -0,0 +1,739 @@ +/* + * MIPI-DSI based s6e3ha2 AMOLED 5.7 inch panel driver. + * + * Copyright (c) 2016 Samsung Electronics Co., Ltd. + * Donghwa Lee + * Hyungwon Hwang + * Hoegeun Kwon + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include + +#define S6E3HA2_MIN_BRIGHTNESS 0 +#define S6E3HA2_MAX_BRIGHTNESS 100 +#define S6E3HA2_DEFAULT_BRIGHTNESS 80 + +#define S6E3HA2_NUM_GAMMA_STEPS46 +#define S6E3HA2_GAMMA_CMD_CNT 35 +#define S6E3HA2_VINT_STATUS_MAX10 + +static const u8 gamma_tbl[S6E3HA2_NUM_GAMMA_STEPS][S6E3HA2_GAMMA_CMD_CNT] = { + { 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x82, 0x83, + 0x85, 0x88, 0x8b, 0x8b, 0x84, 0x88, 0x82, 0x82, 0x89, 0x86, 0x8c, + 0x94, 0x84, 0xb1, 0xaf, 0x8e, 0xcf, 0xad, 0xc9, 0x00, 0x00, 0x00, + 0x00, 0x00 }, + { 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x84, 0x84, + 0x85, 0x87, 0x8b, 0x8a, 0x84, 0x88, 0x82, 0x82, 0x89, 0x86, 0x8a, + 0x93, 0x84, 0xb0, 0xae, 0x8e, 0xc9, 0xa8, 0xc5, 0x00, 0x00, 0x00, + 0x00, 0x00 }, + { 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x83, 0x83, + 0x85, 0x86, 0x8a, 0x8a, 0x84, 0x88, 0x81, 0x84, 0x8a, 0x88, 0x8a, + 0x91, 0x84, 0xb1, 0xae, 0x8b, 0xd5, 0xb2, 0xcc, 0x00, 0x00, 0x00, + 0x00, 0x00 }, + { 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x83, 0x83, + 0x85, 0x86, 0x8a, 0x8a, 0x84, 0x87, 0x81, 0x84, 0x8a, 0x87, 0x8a, + 0x91, 0x85, 0xae, 0xac, 0x8a, 0xc3, 0xa3, 0xc0, 0x00, 0x00, 0x00, + 0x00, 0x00 }, + { 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x88, 0x86, 0x87, 0x85, 0x85, + 0x86, 0x85, 0x88, 0x89, 0x84, 0x89, 0x82, 0x84, 0x87, 0x85, 0x8b, + 0x91, 0x88, 0xad, 0xab, 0x8a, 0xb7, 0x9b, 0xb6, 0x00, 0x00, 0x00, + 0x00, 0x00 }, + { 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x83, 0x83, + 0x85, 0x86, 0x89, 0x8a, 0x84, 0x89, 0x83, 0x83, 0x86, 0x84, 0x8b, + 0x90, 0x84, 0xb0, 0xae, 0x8b, 0xce, 0xad, 0xc8, 0x00, 0x00, 0x00, + 0x00, 0x00 }, + { 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x83, 0x83, + 0x85, 0x87, 0x89, 0x8a, 0x83, 0x87, 0x82, 0x85, 0x88, 0x87, 0x89, + 0x8f, 0x84, 0xac, 0xaa, 0x89, 0xb1, 0x98, 0xaf, 0x00, 0x00, 0x00, + 0x00, 0x00 }, + { 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x83, 0x83, + 0x85, 0x86, 0x88, 0x89, 0x84, 0x88, 0x83, 0x82, 0x85, 0x84, 0x8c, + 0x91, 0x86, 0xac, 0xaa, 0x89, 0xc2, 0xa5, 0xbd, 0x00, 0x00,
[PATCH v10 3/3] arm64: dts: exynos: Add support for S6E3HA2 panel device on TM2 board
From: Hyungwon HwangThis patch add the panel device tree node for S6E3HA2 display controller to TM2 dts. Signed-off-by: Hyungwon Hwang Signed-off-by: Andrzej Hajda Signed-off-by: Chanwoo Choi Signed-off-by: Hoegeun Kwon Tested-by: Chanwoo Choi --- arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 12 1 file changed, 12 insertions(+) diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts index dea0a6f..db3fed2 100644 --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts @@ -52,6 +52,18 @@ assigned-clock-rates = <25000>, <4>; }; + { + panel@0 { + compatible = "samsung,s6e3ha2"; + reg = <0>; + vdd3-supply = <_reg>; + vci-supply = <_reg>; + reset-gpios = < 0 GPIO_ACTIVE_LOW>; + enable-gpios = < 5 GPIO_ACTIVE_HIGH>; + te-gpios = < 3 GPIO_ACTIVE_HIGH>; + }; +}; + _9 { status = "okay"; -- 1.9.1
[PATCH v10 3/3] arm64: dts: exynos: Add support for S6E3HA2 panel device on TM2 board
From: Hyungwon Hwang This patch add the panel device tree node for S6E3HA2 display controller to TM2 dts. Signed-off-by: Hyungwon Hwang Signed-off-by: Andrzej Hajda Signed-off-by: Chanwoo Choi Signed-off-by: Hoegeun Kwon Tested-by: Chanwoo Choi --- arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 12 1 file changed, 12 insertions(+) diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts index dea0a6f..db3fed2 100644 --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts @@ -52,6 +52,18 @@ assigned-clock-rates = <25000>, <4>; }; + { + panel@0 { + compatible = "samsung,s6e3ha2"; + reg = <0>; + vdd3-supply = <_reg>; + vci-supply = <_reg>; + reset-gpios = < 0 GPIO_ACTIVE_LOW>; + enable-gpios = < 5 GPIO_ACTIVE_HIGH>; + te-gpios = < 3 GPIO_ACTIVE_HIGH>; + }; +}; + _9 { status = "okay"; -- 1.9.1
Re: [RFC] perf/sdt: Directly record SDT event with 'perf record'
* Ravi Bangoriawrote: > All events from 'perf list', except SDT events, can be directly recorded > with 'perf record'. But, the flow is little different for SDT events. > Probe point for SDT event needs to be created using 'perf probe' before > recording it using 'perf record'. > > As suggested by Ingo[1], it's better to make this process simple by > creating probe points automatically with 'perf record' for SDT events. > > This patch disables 'perf probe' on SDT events to simplify usage. It > enables recording SDT event only with 'perf record'. > > This removes all those 'multiple events with same name' issues by not > allowing manual probe creation to user. When there are multiple events > with same name, 'perf record' will record all of them (in line with > other tools supporting SDT (systemtap)). > > I know 'perf probe' for SDT events has already became interface and > people are using it. But, doing this change will make user interface very > easy and also it will make tool behaviour consistent. Also, it won't > require any changes in uprobe_events structure (suggested by Masami[2]). So I like the automatism you implemented for 'perf record', but why not keep the 'perf probe' flow as well, if people got used to it? It's not like computer software is bad at sorting apart and handling the two cases properly, right? Thanks, Ingo
Re: [RFC] perf/sdt: Directly record SDT event with 'perf record'
* Ravi Bangoria wrote: > All events from 'perf list', except SDT events, can be directly recorded > with 'perf record'. But, the flow is little different for SDT events. > Probe point for SDT event needs to be created using 'perf probe' before > recording it using 'perf record'. > > As suggested by Ingo[1], it's better to make this process simple by > creating probe points automatically with 'perf record' for SDT events. > > This patch disables 'perf probe' on SDT events to simplify usage. It > enables recording SDT event only with 'perf record'. > > This removes all those 'multiple events with same name' issues by not > allowing manual probe creation to user. When there are multiple events > with same name, 'perf record' will record all of them (in line with > other tools supporting SDT (systemtap)). > > I know 'perf probe' for SDT events has already became interface and > people are using it. But, doing this change will make user interface very > easy and also it will make tool behaviour consistent. Also, it won't > require any changes in uprobe_events structure (suggested by Masami[2]). So I like the automatism you implemented for 'perf record', but why not keep the 'perf probe' flow as well, if people got used to it? It's not like computer software is bad at sorting apart and handling the two cases properly, right? Thanks, Ingo
[PATCH 2/2] arm: dts: imx6q: Add Engicam i.CoreM6 Quad/Dual OpenFrame Cap 10.1 initial support
From: Jagan Tekii.CoreM6 Quad/Dual OpenFrame modules are "system on modules plus openframe display carriers" which are good solution for develop user friendly graphic user interface. General features: CPU NXP i.MX6Q rev1.2 at 792 MHz RAM 1GB, 32, 64 bit, DDR3-800/1066 NAND SLC,512MB LVDS Display TFT 10.1" industrial, 1280x800 resolution Backlight LED backlight, brightness 350 Cd/m2 Power supply 15 to 30 Vdc Cc: Domenico Acri Cc: Matteo Lisi Cc: Michael Trimarchi Cc: Shawn Guo Signed-off-by: Jagan Teki --- arch/arm/boot/dts/Makefile| 1 + arch/arm/boot/dts/imx6q-icore-ofcap10.dts | 76 +++ 2 files changed, 77 insertions(+) create mode 100644 arch/arm/boot/dts/imx6q-icore-ofcap10.dts diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 0118084..458ec09 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -398,6 +398,7 @@ dtb-$(CONFIG_SOC_IMX6Q) += \ imx6q-h100.dtb \ imx6q-hummingboard.dtb \ imx6q-icore.dtb \ + imx6q-icore-ofcap10.dtb \ imx6q-icore-rqs.dtb \ imx6q-marsboard.dtb \ imx6q-mccmon6.dtb \ diff --git a/arch/arm/boot/dts/imx6q-icore-ofcap10.dts b/arch/arm/boot/dts/imx6q-icore-ofcap10.dts new file mode 100644 index 000..70600e2 --- /dev/null +++ b/arch/arm/boot/dts/imx6q-icore-ofcap10.dts @@ -0,0 +1,76 @@ +/* + * Copyright (C) 2016 Amarula Solutions B.V. + * Copyright (C) 2016 Engicam S.r.l. + * + * This file is dual-licensed: you can use it either under the terms + * of the GPL or the X11 license, at your option. Note that this dual + * licensing only applies to this file, and not this project as a + * whole. + * + * a) This file is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This file is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * Or, alternatively, + * + * b) Permission is hereby granted, free of charge, to any person + * obtaining a copy of this software and associated documentation + * files (the "Software"), to deal in the Software without + * restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following + * conditions: + * + * The above copyright notice and this permission notice shall be + * included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + */ + +/dts-v1/; + +#include "imx6q.dtsi" +#include "imx6qdl-icore.dtsi" + +/ { + model = "Engicam i.CoreM6 Quad/Dual OpenFrame Capacitive touch 10.1 Kit"; + compatible = "engicam,imx6-icore", "fsl,imx6q"; +}; + + { + status = "okay"; + + lvds-channel@0 { + fsl,data-mapping = "spwg"; + fsl,data-width = <24>; + status = "okay"; + + display-timings { +native-mode = <>; +timing1: hsd100pxn1 { +clock-frequency = <6000>; +hactive = <1280>; +vactive = <800>; +hback-porch = <40>; +hfront-porch = <40>; +vback-porch = <10>; +vfront-porch = <3>; +hsync-len = <80>; +vsync-len = <10>; +}; +}; + }; +}; -- 1.9.1
[PATCH 1/2] arm: dts: imx6qdl-icore: Add backlight support for lvds
From: Jagan TekiThis patch add support for lvds backlight on i.CoreM6 QDL variant boards. Cc: Domenico Acri Cc: Matteo Lisi Cc: Michael Trimarchi Cc: Shawn Guo Signed-off-by: Jagan Teki --- arch/arm/boot/dts/imx6qdl-icore.dtsi | 20 1 file changed, 20 insertions(+) diff --git a/arch/arm/boot/dts/imx6qdl-icore.dtsi b/arch/arm/boot/dts/imx6qdl-icore.dtsi index 55bebfc..7dda608 100644 --- a/arch/arm/boot/dts/imx6qdl-icore.dtsi +++ b/arch/arm/boot/dts/imx6qdl-icore.dtsi @@ -48,6 +48,14 @@ reg = <0x1000 0x8000>; }; + backlight { + compatible = "pwm-backlight"; + pwms = < 0 10>; + brightness-levels = <0 4 8 16 32 64 128 255>; + default-brightness-level = <7>; + status = "okay"; + }; + reg_3p3v: regulator-3p3v { compatible = "regulator-fixed"; regulator-name = "3P3V"; @@ -136,6 +144,12 @@ status = "okay"; }; + { + pinctrl-names = "default"; + pinctrl-0 = <_pwm3>; + status = "okay"; +}; + { pinctrl-names = "default"; pinctrl-0 = <_uart4>; @@ -246,6 +260,12 @@ >; }; + pinctrl_pwm3: pwm3grp-1 { + fsl,pins = < + MX6QDL_PAD_SD4_DAT1__PWM3_OUT 0x1b0b1 + >; + }; + pinctrl_usbotg: usbotggrp { fsl,pins = < MX6QDL_PAD_GPIO_1__USB_OTG_ID 0x17059 -- 1.9.1
[PATCH 2/2] arm: dts: imx6q: Add Engicam i.CoreM6 Quad/Dual OpenFrame Cap 10.1 initial support
From: Jagan Teki i.CoreM6 Quad/Dual OpenFrame modules are "system on modules plus openframe display carriers" which are good solution for develop user friendly graphic user interface. General features: CPU NXP i.MX6Q rev1.2 at 792 MHz RAM 1GB, 32, 64 bit, DDR3-800/1066 NAND SLC,512MB LVDS Display TFT 10.1" industrial, 1280x800 resolution Backlight LED backlight, brightness 350 Cd/m2 Power supply 15 to 30 Vdc Cc: Domenico Acri Cc: Matteo Lisi Cc: Michael Trimarchi Cc: Shawn Guo Signed-off-by: Jagan Teki --- arch/arm/boot/dts/Makefile| 1 + arch/arm/boot/dts/imx6q-icore-ofcap10.dts | 76 +++ 2 files changed, 77 insertions(+) create mode 100644 arch/arm/boot/dts/imx6q-icore-ofcap10.dts diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 0118084..458ec09 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -398,6 +398,7 @@ dtb-$(CONFIG_SOC_IMX6Q) += \ imx6q-h100.dtb \ imx6q-hummingboard.dtb \ imx6q-icore.dtb \ + imx6q-icore-ofcap10.dtb \ imx6q-icore-rqs.dtb \ imx6q-marsboard.dtb \ imx6q-mccmon6.dtb \ diff --git a/arch/arm/boot/dts/imx6q-icore-ofcap10.dts b/arch/arm/boot/dts/imx6q-icore-ofcap10.dts new file mode 100644 index 000..70600e2 --- /dev/null +++ b/arch/arm/boot/dts/imx6q-icore-ofcap10.dts @@ -0,0 +1,76 @@ +/* + * Copyright (C) 2016 Amarula Solutions B.V. + * Copyright (C) 2016 Engicam S.r.l. + * + * This file is dual-licensed: you can use it either under the terms + * of the GPL or the X11 license, at your option. Note that this dual + * licensing only applies to this file, and not this project as a + * whole. + * + * a) This file is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This file is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * Or, alternatively, + * + * b) Permission is hereby granted, free of charge, to any person + * obtaining a copy of this software and associated documentation + * files (the "Software"), to deal in the Software without + * restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following + * conditions: + * + * The above copyright notice and this permission notice shall be + * included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + */ + +/dts-v1/; + +#include "imx6q.dtsi" +#include "imx6qdl-icore.dtsi" + +/ { + model = "Engicam i.CoreM6 Quad/Dual OpenFrame Capacitive touch 10.1 Kit"; + compatible = "engicam,imx6-icore", "fsl,imx6q"; +}; + + { + status = "okay"; + + lvds-channel@0 { + fsl,data-mapping = "spwg"; + fsl,data-width = <24>; + status = "okay"; + + display-timings { +native-mode = <>; +timing1: hsd100pxn1 { +clock-frequency = <6000>; +hactive = <1280>; +vactive = <800>; +hback-porch = <40>; +hfront-porch = <40>; +vback-porch = <10>; +vfront-porch = <3>; +hsync-len = <80>; +vsync-len = <10>; +}; +}; + }; +}; -- 1.9.1
[PATCH 1/2] arm: dts: imx6qdl-icore: Add backlight support for lvds
From: Jagan Teki This patch add support for lvds backlight on i.CoreM6 QDL variant boards. Cc: Domenico Acri Cc: Matteo Lisi Cc: Michael Trimarchi Cc: Shawn Guo Signed-off-by: Jagan Teki --- arch/arm/boot/dts/imx6qdl-icore.dtsi | 20 1 file changed, 20 insertions(+) diff --git a/arch/arm/boot/dts/imx6qdl-icore.dtsi b/arch/arm/boot/dts/imx6qdl-icore.dtsi index 55bebfc..7dda608 100644 --- a/arch/arm/boot/dts/imx6qdl-icore.dtsi +++ b/arch/arm/boot/dts/imx6qdl-icore.dtsi @@ -48,6 +48,14 @@ reg = <0x1000 0x8000>; }; + backlight { + compatible = "pwm-backlight"; + pwms = < 0 10>; + brightness-levels = <0 4 8 16 32 64 128 255>; + default-brightness-level = <7>; + status = "okay"; + }; + reg_3p3v: regulator-3p3v { compatible = "regulator-fixed"; regulator-name = "3P3V"; @@ -136,6 +144,12 @@ status = "okay"; }; + { + pinctrl-names = "default"; + pinctrl-0 = <_pwm3>; + status = "okay"; +}; + { pinctrl-names = "default"; pinctrl-0 = <_uart4>; @@ -246,6 +260,12 @@ >; }; + pinctrl_pwm3: pwm3grp-1 { + fsl,pins = < + MX6QDL_PAD_SD4_DAT1__PWM3_OUT 0x1b0b1 + >; + }; + pinctrl_usbotg: usbotggrp { fsl,pins = < MX6QDL_PAD_GPIO_1__USB_OTG_ID 0x17059 -- 1.9.1
Re: [Outreachy kernel] [PATCH 2/2] staging: ks7010: Unnecessary parentheses are removed.
On Mon, 20 Feb 2017, Arushi Singhal wrote: > Unnecessary parentheses should be avoided as reported by checkpatch.pl. > Remove unnecessary parentheses, as reported by checkpatch as are nicer > to read.For example:- > It's often nicer to read if &(foo[0]) is converted to foo like: > memcpy(&(ap->bssid[0]), &(ap_info->bssid[0]), ETH_ALEN); > memcpy(ap->bssid, ap_info->bssid, ETH_ALEN); The commit message is not well presented. One has the impression that all of the changes are related to 0 array elements. It would be better to split the patch into two: one for removing parentheses, and one for making the [0] -> x change. They don't really have anything to do with each other. julia > > Signed-off-by: Arushi Singhal> --- > drivers/staging/ks7010/ks_hostif.c | 20 ++-- > drivers/staging/ks7010/ks_wlan_net.c | 20 ++-- > 2 files changed, 20 insertions(+), 20 deletions(-) > > diff --git a/drivers/staging/ks7010/ks_hostif.c > b/drivers/staging/ks7010/ks_hostif.c > index ba283ab741a7..2d5ec57c5cfd 100644 > --- a/drivers/staging/ks7010/ks_hostif.c > +++ b/drivers/staging/ks7010/ks_hostif.c > @@ -239,19 +239,19 @@ int get_ap_information(struct ks_wlan_private *priv, > struct ap_info_t *ap_info, > *(bp + 1)); > ap->ssid.size = SSID_MAX_SIZE; > } > - memcpy(&(ap->ssid.body[0]), bp + 2, ap->ssid.size); > + memcpy(ap->ssid.body, bp + 2, ap->ssid.size); > break; > case 1: /* rate */ > case 50:/* ext rate */ > if ((*(bp + 1) + ap->rate_set.size) <= > RATE_SET_MAX_SIZE) { > - memcpy(&(ap->rate_set.body[ap->rate_set.size]), > + memcpy(>rate_set.body[ap->rate_set.size], > bp + 2, *(bp + 1)); > ap->rate_set.size += *(bp + 1); > } else { > DPRINTK(1, "size over :: rate size=%d\n", > (*(bp + 1) + ap->rate_set.size)); > - memcpy(&(ap->rate_set.body[ap->rate_set.size]), > + memcpy(>rate_set.body[ap->rate_set.size], > bp + 2, > RATE_SET_MAX_SIZE - ap->rate_set.size); > ap->rate_set.size += > @@ -269,7 +269,7 @@ int get_ap_information(struct ks_wlan_private *priv, > struct ap_info_t *ap_info, > *(bp + 1)); > ap->rsn_ie.size = RSN_IE_BODY_MAX; > } > - memcpy(&(ap->rsn_ie.body[0]), bp + 2, ap->rsn_ie.size); > + memcpy(ap->rsn_ie.body, bp + 2, ap->rsn_ie.size); > break; > case 221: /* WPA */ > if (!memcmp(bp + 2, "\x00\x50\xf2\x01", 4)) { /* WPA > OUI check */ > @@ -282,7 +282,7 @@ int get_ap_information(struct ks_wlan_private *priv, > struct ap_info_t *ap_info, > *(bp + 1)); > ap->wpa_ie.size = RSN_IE_BODY_MAX; > } > - memcpy(&(ap->wpa_ie.body[0]), bp + 2, > + memcpy(ap->wpa_ie.body, bp + 2, > ap->wpa_ie.size); > } > break; > @@ -832,8 +832,8 @@ void hostif_scan_indication(struct ks_wlan_private *priv) > if (priv->scan_ind_count != 0) { > for (i = 0; i < priv->aplist.size; i++) { /* bssid check > */ > if (!memcmp > - (&(ap_info->bssid[0]), > - &(priv->aplist.ap[i].bssid[0]), ETH_ALEN)) { > + (ap_info->bssid, > + >aplist.ap[i].bssid[0], ETH_ALEN)) { > if (ap_info->frame_type == > FRAME_TYPE_PROBE_RESP) > get_ap_information(priv, ap_info, > @@ -2652,7 +2652,7 @@ int hostif_init(struct ks_wlan_private *priv) > > priv->aplist.size = 0; > for (i = 0; i < LOCAL_APLIST_MAX; i++) > - memset(&(priv->aplist.ap[i]), 0, sizeof(struct local_ap_t)); > + memset(>aplist.ap[i], 0, sizeof(struct local_ap_t)); > priv->infra_status = 0; > priv->current_rate = 4; > priv->connect_status = DISCONNECT_STATUS; > @@ -2675,12 +2675,12 @@ int hostif_init(struct ks_wlan_private *priv) > INIT_WORK(>ks_wlan_wakeup_task, ks_wlan_hw_wakeup_task); > > /* WPA */ > - memset(&(priv->wpa), 0, sizeof(priv->wpa)); > +
Re: [Outreachy kernel] [PATCH 2/2] staging: ks7010: Unnecessary parentheses are removed.
On Mon, 20 Feb 2017, Arushi Singhal wrote: > Unnecessary parentheses should be avoided as reported by checkpatch.pl. > Remove unnecessary parentheses, as reported by checkpatch as are nicer > to read.For example:- > It's often nicer to read if &(foo[0]) is converted to foo like: > memcpy(&(ap->bssid[0]), &(ap_info->bssid[0]), ETH_ALEN); > memcpy(ap->bssid, ap_info->bssid, ETH_ALEN); The commit message is not well presented. One has the impression that all of the changes are related to 0 array elements. It would be better to split the patch into two: one for removing parentheses, and one for making the [0] -> x change. They don't really have anything to do with each other. julia > > Signed-off-by: Arushi Singhal > --- > drivers/staging/ks7010/ks_hostif.c | 20 ++-- > drivers/staging/ks7010/ks_wlan_net.c | 20 ++-- > 2 files changed, 20 insertions(+), 20 deletions(-) > > diff --git a/drivers/staging/ks7010/ks_hostif.c > b/drivers/staging/ks7010/ks_hostif.c > index ba283ab741a7..2d5ec57c5cfd 100644 > --- a/drivers/staging/ks7010/ks_hostif.c > +++ b/drivers/staging/ks7010/ks_hostif.c > @@ -239,19 +239,19 @@ int get_ap_information(struct ks_wlan_private *priv, > struct ap_info_t *ap_info, > *(bp + 1)); > ap->ssid.size = SSID_MAX_SIZE; > } > - memcpy(&(ap->ssid.body[0]), bp + 2, ap->ssid.size); > + memcpy(ap->ssid.body, bp + 2, ap->ssid.size); > break; > case 1: /* rate */ > case 50:/* ext rate */ > if ((*(bp + 1) + ap->rate_set.size) <= > RATE_SET_MAX_SIZE) { > - memcpy(&(ap->rate_set.body[ap->rate_set.size]), > + memcpy(>rate_set.body[ap->rate_set.size], > bp + 2, *(bp + 1)); > ap->rate_set.size += *(bp + 1); > } else { > DPRINTK(1, "size over :: rate size=%d\n", > (*(bp + 1) + ap->rate_set.size)); > - memcpy(&(ap->rate_set.body[ap->rate_set.size]), > + memcpy(>rate_set.body[ap->rate_set.size], > bp + 2, > RATE_SET_MAX_SIZE - ap->rate_set.size); > ap->rate_set.size += > @@ -269,7 +269,7 @@ int get_ap_information(struct ks_wlan_private *priv, > struct ap_info_t *ap_info, > *(bp + 1)); > ap->rsn_ie.size = RSN_IE_BODY_MAX; > } > - memcpy(&(ap->rsn_ie.body[0]), bp + 2, ap->rsn_ie.size); > + memcpy(ap->rsn_ie.body, bp + 2, ap->rsn_ie.size); > break; > case 221: /* WPA */ > if (!memcmp(bp + 2, "\x00\x50\xf2\x01", 4)) { /* WPA > OUI check */ > @@ -282,7 +282,7 @@ int get_ap_information(struct ks_wlan_private *priv, > struct ap_info_t *ap_info, > *(bp + 1)); > ap->wpa_ie.size = RSN_IE_BODY_MAX; > } > - memcpy(&(ap->wpa_ie.body[0]), bp + 2, > + memcpy(ap->wpa_ie.body, bp + 2, > ap->wpa_ie.size); > } > break; > @@ -832,8 +832,8 @@ void hostif_scan_indication(struct ks_wlan_private *priv) > if (priv->scan_ind_count != 0) { > for (i = 0; i < priv->aplist.size; i++) { /* bssid check > */ > if (!memcmp > - (&(ap_info->bssid[0]), > - &(priv->aplist.ap[i].bssid[0]), ETH_ALEN)) { > + (ap_info->bssid, > + >aplist.ap[i].bssid[0], ETH_ALEN)) { > if (ap_info->frame_type == > FRAME_TYPE_PROBE_RESP) > get_ap_information(priv, ap_info, > @@ -2652,7 +2652,7 @@ int hostif_init(struct ks_wlan_private *priv) > > priv->aplist.size = 0; > for (i = 0; i < LOCAL_APLIST_MAX; i++) > - memset(&(priv->aplist.ap[i]), 0, sizeof(struct local_ap_t)); > + memset(>aplist.ap[i], 0, sizeof(struct local_ap_t)); > priv->infra_status = 0; > priv->current_rate = 4; > priv->connect_status = DISCONNECT_STATUS; > @@ -2675,12 +2675,12 @@ int hostif_init(struct ks_wlan_private *priv) > INIT_WORK(>ks_wlan_wakeup_task, ks_wlan_hw_wakeup_task); > > /* WPA */ > - memset(&(priv->wpa), 0, sizeof(priv->wpa)); > + memset(>wpa, 0, sizeof(priv->wpa)); >
Re: [PATCH] drm/rockchip: cdn-dp: Fix error handling
On 2017年02月20日 14:41, Christophe JAILLET wrote: Le 20/02/2017 à 02:40, Mark yao a écrit : On 2017年02月20日 00:59, Christophe JAILLET wrote: It is likely that both 'clk_disable_unprepare()' should be called if 'pm_runtime_get_sync()' fails. Add a new label for that, because 'err_set_rate' is not meaningful in this case. Fixes: 1a0f7ed3abe2 ("drm/rockchip: cdn-dp: add cdn DP support for rk3399") Signed-off-by: Christophe JAILLET--- Not sure but a 'pm_runtime_get_sync()' is maybe also required in the 'err_set_rate' path. --- drivers/gpu/drm/rockchip/cdn-dp-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c index 9ab67a670885..0fe1ec8b8fb1 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c @@ -111,7 +111,7 @@ static int cdn_dp_clk_enable(struct cdn_dp_device *dp) ret = pm_runtime_get_sync(dp->dev); if (ret < 0) { DRM_DEV_ERROR(dp->dev, "cannot get pm runtime %d\n", ret); -goto err_pclk; +goto err_sync; I think the name err_pm_runtime_get is better. err_sync is not a clear name for the pm_runtime_get_sync. I will change it. } reset_control_assert(dp->core_rst); @@ -133,6 +133,7 @@ static int cdn_dp_clk_enable(struct cdn_dp_device *dp) return 0; err_set_rate: +err_sync: miss pm_runtime_put, it should be: I am wondering if 'pm_runtime_put_sync' should be added, instead. We want to revert the 'pm_runtime_get_sync' of line 111. According to the naming of the function, the _sync version looks more logical to me. Using ccoccinelle shows that 2/3 of functions calling both 'pm_runtime_get_sync' and 'pm_runtime_get[_sync]' and using the _sync variant. pm_runtime_get_sync will block until hardware actually done power configure, we need make sure power is enable before use the hardware, So we should use pm_runtime_get_sync at power on. At power off time, use pm_runtime_put is enough, it can be async, no need block. Thanks. Which semantic is the correct one? err_set_rate: pm_runtime_put(dp->dev); err_pm_runtime_get: clk_disable_unprepare(dp->core_clk); err_core_clk: clk_disable_unprepare(dp->core_clk); err_core_clk: clk_disable_unprepare(dp->pclk); -- Mark Yao
Re: [PATCH] drm/rockchip: cdn-dp: Fix error handling
On 2017年02月20日 14:41, Christophe JAILLET wrote: Le 20/02/2017 à 02:40, Mark yao a écrit : On 2017年02月20日 00:59, Christophe JAILLET wrote: It is likely that both 'clk_disable_unprepare()' should be called if 'pm_runtime_get_sync()' fails. Add a new label for that, because 'err_set_rate' is not meaningful in this case. Fixes: 1a0f7ed3abe2 ("drm/rockchip: cdn-dp: add cdn DP support for rk3399") Signed-off-by: Christophe JAILLET --- Not sure but a 'pm_runtime_get_sync()' is maybe also required in the 'err_set_rate' path. --- drivers/gpu/drm/rockchip/cdn-dp-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c index 9ab67a670885..0fe1ec8b8fb1 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c @@ -111,7 +111,7 @@ static int cdn_dp_clk_enable(struct cdn_dp_device *dp) ret = pm_runtime_get_sync(dp->dev); if (ret < 0) { DRM_DEV_ERROR(dp->dev, "cannot get pm runtime %d\n", ret); -goto err_pclk; +goto err_sync; I think the name err_pm_runtime_get is better. err_sync is not a clear name for the pm_runtime_get_sync. I will change it. } reset_control_assert(dp->core_rst); @@ -133,6 +133,7 @@ static int cdn_dp_clk_enable(struct cdn_dp_device *dp) return 0; err_set_rate: +err_sync: miss pm_runtime_put, it should be: I am wondering if 'pm_runtime_put_sync' should be added, instead. We want to revert the 'pm_runtime_get_sync' of line 111. According to the naming of the function, the _sync version looks more logical to me. Using ccoccinelle shows that 2/3 of functions calling both 'pm_runtime_get_sync' and 'pm_runtime_get[_sync]' and using the _sync variant. pm_runtime_get_sync will block until hardware actually done power configure, we need make sure power is enable before use the hardware, So we should use pm_runtime_get_sync at power on. At power off time, use pm_runtime_put is enough, it can be async, no need block. Thanks. Which semantic is the correct one? err_set_rate: pm_runtime_put(dp->dev); err_pm_runtime_get: clk_disable_unprepare(dp->core_clk); err_core_clk: clk_disable_unprepare(dp->core_clk); err_core_clk: clk_disable_unprepare(dp->pclk); -- Mark Yao
[PATCH 2/2] staging: ks7010: Unnecessary parentheses are removed.
Unnecessary parentheses should be avoided as reported by checkpatch.pl. Remove unnecessary parentheses, as reported by checkpatch as are nicer to read.For example:- It's often nicer to read if &(foo[0]) is converted to foo like: memcpy(&(ap->bssid[0]), &(ap_info->bssid[0]), ETH_ALEN); memcpy(ap->bssid, ap_info->bssid, ETH_ALEN); Signed-off-by: Arushi Singhal--- drivers/staging/ks7010/ks_hostif.c | 20 ++-- drivers/staging/ks7010/ks_wlan_net.c | 20 ++-- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index ba283ab741a7..2d5ec57c5cfd 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -239,19 +239,19 @@ int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, *(bp + 1)); ap->ssid.size = SSID_MAX_SIZE; } - memcpy(&(ap->ssid.body[0]), bp + 2, ap->ssid.size); + memcpy(ap->ssid.body, bp + 2, ap->ssid.size); break; case 1: /* rate */ case 50:/* ext rate */ if ((*(bp + 1) + ap->rate_set.size) <= RATE_SET_MAX_SIZE) { - memcpy(&(ap->rate_set.body[ap->rate_set.size]), + memcpy(>rate_set.body[ap->rate_set.size], bp + 2, *(bp + 1)); ap->rate_set.size += *(bp + 1); } else { DPRINTK(1, "size over :: rate size=%d\n", (*(bp + 1) + ap->rate_set.size)); - memcpy(&(ap->rate_set.body[ap->rate_set.size]), + memcpy(>rate_set.body[ap->rate_set.size], bp + 2, RATE_SET_MAX_SIZE - ap->rate_set.size); ap->rate_set.size += @@ -269,7 +269,7 @@ int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, *(bp + 1)); ap->rsn_ie.size = RSN_IE_BODY_MAX; } - memcpy(&(ap->rsn_ie.body[0]), bp + 2, ap->rsn_ie.size); + memcpy(ap->rsn_ie.body, bp + 2, ap->rsn_ie.size); break; case 221: /* WPA */ if (!memcmp(bp + 2, "\x00\x50\xf2\x01", 4)) { /* WPA OUI check */ @@ -282,7 +282,7 @@ int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, *(bp + 1)); ap->wpa_ie.size = RSN_IE_BODY_MAX; } - memcpy(&(ap->wpa_ie.body[0]), bp + 2, + memcpy(ap->wpa_ie.body, bp + 2, ap->wpa_ie.size); } break; @@ -832,8 +832,8 @@ void hostif_scan_indication(struct ks_wlan_private *priv) if (priv->scan_ind_count != 0) { for (i = 0; i < priv->aplist.size; i++) { /* bssid check */ if (!memcmp - (&(ap_info->bssid[0]), -&(priv->aplist.ap[i].bssid[0]), ETH_ALEN)) { + (ap_info->bssid, +>aplist.ap[i].bssid[0], ETH_ALEN)) { if (ap_info->frame_type == FRAME_TYPE_PROBE_RESP) get_ap_information(priv, ap_info, @@ -2652,7 +2652,7 @@ int hostif_init(struct ks_wlan_private *priv) priv->aplist.size = 0; for (i = 0; i < LOCAL_APLIST_MAX; i++) - memset(&(priv->aplist.ap[i]), 0, sizeof(struct local_ap_t)); + memset(>aplist.ap[i], 0, sizeof(struct local_ap_t)); priv->infra_status = 0; priv->current_rate = 4; priv->connect_status = DISCONNECT_STATUS; @@ -2675,12 +2675,12 @@ int hostif_init(struct ks_wlan_private *priv) INIT_WORK(>ks_wlan_wakeup_task, ks_wlan_hw_wakeup_task); /* WPA */ - memset(&(priv->wpa), 0, sizeof(priv->wpa)); + memset(>wpa, 0, sizeof(priv->wpa)); priv->wpa.rsn_enabled = 0; priv->wpa.mic_failure.failure = 0; priv->wpa.mic_failure.last_failure_time = 0; priv->wpa.mic_failure.stop = 0; - memset(&(priv->pmklist), 0, sizeof(priv->pmklist)); + memset(>pmklist, 0, sizeof(priv->pmklist)); INIT_LIST_HEAD(>pmklist.head); for (i = 0; i < PMK_LIST_MAX; i++)
[PATCH 2/2] staging: ks7010: Unnecessary parentheses are removed.
Unnecessary parentheses should be avoided as reported by checkpatch.pl. Remove unnecessary parentheses, as reported by checkpatch as are nicer to read.For example:- It's often nicer to read if &(foo[0]) is converted to foo like: memcpy(&(ap->bssid[0]), &(ap_info->bssid[0]), ETH_ALEN); memcpy(ap->bssid, ap_info->bssid, ETH_ALEN); Signed-off-by: Arushi Singhal --- drivers/staging/ks7010/ks_hostif.c | 20 ++-- drivers/staging/ks7010/ks_wlan_net.c | 20 ++-- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index ba283ab741a7..2d5ec57c5cfd 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -239,19 +239,19 @@ int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, *(bp + 1)); ap->ssid.size = SSID_MAX_SIZE; } - memcpy(&(ap->ssid.body[0]), bp + 2, ap->ssid.size); + memcpy(ap->ssid.body, bp + 2, ap->ssid.size); break; case 1: /* rate */ case 50:/* ext rate */ if ((*(bp + 1) + ap->rate_set.size) <= RATE_SET_MAX_SIZE) { - memcpy(&(ap->rate_set.body[ap->rate_set.size]), + memcpy(>rate_set.body[ap->rate_set.size], bp + 2, *(bp + 1)); ap->rate_set.size += *(bp + 1); } else { DPRINTK(1, "size over :: rate size=%d\n", (*(bp + 1) + ap->rate_set.size)); - memcpy(&(ap->rate_set.body[ap->rate_set.size]), + memcpy(>rate_set.body[ap->rate_set.size], bp + 2, RATE_SET_MAX_SIZE - ap->rate_set.size); ap->rate_set.size += @@ -269,7 +269,7 @@ int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, *(bp + 1)); ap->rsn_ie.size = RSN_IE_BODY_MAX; } - memcpy(&(ap->rsn_ie.body[0]), bp + 2, ap->rsn_ie.size); + memcpy(ap->rsn_ie.body, bp + 2, ap->rsn_ie.size); break; case 221: /* WPA */ if (!memcmp(bp + 2, "\x00\x50\xf2\x01", 4)) { /* WPA OUI check */ @@ -282,7 +282,7 @@ int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, *(bp + 1)); ap->wpa_ie.size = RSN_IE_BODY_MAX; } - memcpy(&(ap->wpa_ie.body[0]), bp + 2, + memcpy(ap->wpa_ie.body, bp + 2, ap->wpa_ie.size); } break; @@ -832,8 +832,8 @@ void hostif_scan_indication(struct ks_wlan_private *priv) if (priv->scan_ind_count != 0) { for (i = 0; i < priv->aplist.size; i++) { /* bssid check */ if (!memcmp - (&(ap_info->bssid[0]), -&(priv->aplist.ap[i].bssid[0]), ETH_ALEN)) { + (ap_info->bssid, +>aplist.ap[i].bssid[0], ETH_ALEN)) { if (ap_info->frame_type == FRAME_TYPE_PROBE_RESP) get_ap_information(priv, ap_info, @@ -2652,7 +2652,7 @@ int hostif_init(struct ks_wlan_private *priv) priv->aplist.size = 0; for (i = 0; i < LOCAL_APLIST_MAX; i++) - memset(&(priv->aplist.ap[i]), 0, sizeof(struct local_ap_t)); + memset(>aplist.ap[i], 0, sizeof(struct local_ap_t)); priv->infra_status = 0; priv->current_rate = 4; priv->connect_status = DISCONNECT_STATUS; @@ -2675,12 +2675,12 @@ int hostif_init(struct ks_wlan_private *priv) INIT_WORK(>ks_wlan_wakeup_task, ks_wlan_hw_wakeup_task); /* WPA */ - memset(&(priv->wpa), 0, sizeof(priv->wpa)); + memset(>wpa, 0, sizeof(priv->wpa)); priv->wpa.rsn_enabled = 0; priv->wpa.mic_failure.failure = 0; priv->wpa.mic_failure.last_failure_time = 0; priv->wpa.mic_failure.stop = 0; - memset(&(priv->pmklist), 0, sizeof(priv->pmklist)); + memset(>pmklist, 0, sizeof(priv->pmklist)); INIT_LIST_HEAD(>pmklist.head); for (i = 0; i < PMK_LIST_MAX; i++)
Re: [PATCH 3/4] iio: accel: adxl345: Add SPI support
On Sun, Feb 19, 2017 at 01:12:50PM +, Jonathan Cameron wrote: > On 16/02/17 10:02, Eva Rachel Retuya wrote: > > Add SPI driver for initializing spi regmap for the adxl345 core driver. > > The driver supports the same functionality as I2C namely the x, y, z and > > scale readings. > > > > Signed-off-by: Eva Rachel Retuya> Looks nice. I didn't know about the readmask stuff in regmap so > always good to see something new. > Me too. The SPI reads were unusual, and the datasheet mentions some sort of mask to enable proper consecutive register reads. Grepped the mask and discovered this. Since then, the reads are no longer incorrect :) Eva > Jonathan > > --- > > drivers/iio/accel/Kconfig | 13 +++ > > drivers/iio/accel/Makefile | 1 + > > drivers/iio/accel/adxl345_spi.c | 75 > > + > > 3 files changed, 89 insertions(+) > > create mode 100644 drivers/iio/accel/adxl345_spi.c > > > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > > index 5bdd87f..d474fed 100644 > > --- a/drivers/iio/accel/Kconfig > > +++ b/drivers/iio/accel/Kconfig > > @@ -21,6 +21,19 @@ config ADXL345_I2C > > To compile this driver as a module, choose M here: the > > module will be called adxl345_i2c. > > > > +config ADXL345_SPI > > + tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer SPI > > Driver" > > + depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m) > > + depends on SPI > > + select ADXL345 > > + select REGMAP_SPI > > + help > > + Say Y here if you want to build support for the Analog Devices > > + ADXL345 3-axis digital accelerometer. > > + > > + To compile this driver as a module, choose M here: the > > + module will be called adxl345_spi. > > + > > config BMA180 > > tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver" > > depends on I2C > > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile > > index 3f4a6d6..31fba19 100644 > > --- a/drivers/iio/accel/Makefile > > +++ b/drivers/iio/accel/Makefile > > @@ -5,6 +5,7 @@ > > # When adding new entries keep the list in alphabetical order > > obj-$(CONFIG_ADXL345) += adxl345_core.o > > obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o > > +obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o > > obj-$(CONFIG_BMA180) += bma180.o > > obj-$(CONFIG_BMA220) += bma220_spi.o > > obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o > > diff --git a/drivers/iio/accel/adxl345_spi.c > > b/drivers/iio/accel/adxl345_spi.c > > new file mode 100644 > > index 000..5fcd1fa > > --- /dev/null > > +++ b/drivers/iio/accel/adxl345_spi.c > > @@ -0,0 +1,75 @@ > > +/* > > + * ADXL345 3-Axis Digital Accelerometer > > + * > > + * Copyright (c) 2017 Eva Rachel Retuya > > + * > > + * This file is subject to the terms and conditions of version 2 of > > + * the GNU General Public License. See the file COPYING in the main > > + * directory of this archive for more details. > > + * > > + * SPI driver for ADXL345 > > + */ > > + > > +#include > > +#include > > +#include > > + > > +#include "adxl345.h" > > + > > +#define ADXL345_MAX_SPI_FREQ_HZ500 > > + > > +static const struct regmap_config adxl345_spi_regmap_config = { > > + .reg_bits = 8, > > + .val_bits = 8, > > +/* Setting bits 7 and 6 enables multiple-byte read */ > > + .read_flag_mask = BIT(7) | BIT(6), > Nice. I didn't know about that one ;) > > +}; > > + > > +static int adxl345_spi_probe(struct spi_device *spi) > > +{ > > + struct regmap *regmap; > > + const struct spi_device_id *id = spi_get_device_id(spi); > > + > > + /* Bail out if max_speed_hz exceeds 5 MHz */ > > + if (spi->max_speed_hz > ADXL345_MAX_SPI_FREQ_HZ) { > > + dev_err(>dev, "SPI CLK, %d Hz exceeds 5 MHz\n", > > + spi->max_speed_hz); > > + return -EINVAL; > > + } > > + > > + regmap = devm_regmap_init_spi(spi, _spi_regmap_config); > > + if (IS_ERR(regmap)) { > > + dev_err(>dev, "Error initializing spi regmap: %d\n", > > + (int)PTR_ERR(regmap)); > > + return PTR_ERR(regmap); > > + } > > + > > + return adxl345_common_probe(>dev, regmap, id->name); > > +} > > + > > +static int adxl345_spi_remove(struct spi_device *spi) > > +{ > > + return adxl345_common_remove(>dev); > > +} > > + > > +static const struct spi_device_id adxl345_spi_id[] = { > > + { "adxl345", 0 }, > > + { } > > +}; > > + > > +MODULE_DEVICE_TABLE(spi, adxl345_spi_id); > > + > > +static struct spi_driver adxl345_spi_driver = { > > + .driver = { > > + .name = "adxl345_spi", > > + }, > > + .probe = adxl345_spi_probe, > > + .remove = adxl345_spi_remove, > > + .id_table = adxl345_spi_id, > > +}; > > + > > +module_spi_driver(adxl345_spi_driver); > > + > > +MODULE_AUTHOR("Eva Rachel Retuya "); > > +MODULE_DESCRIPTION("ADXL345 3-Axis Digital
Re: [PATCH 3/4] iio: accel: adxl345: Add SPI support
On Sun, Feb 19, 2017 at 01:12:50PM +, Jonathan Cameron wrote: > On 16/02/17 10:02, Eva Rachel Retuya wrote: > > Add SPI driver for initializing spi regmap for the adxl345 core driver. > > The driver supports the same functionality as I2C namely the x, y, z and > > scale readings. > > > > Signed-off-by: Eva Rachel Retuya > Looks nice. I didn't know about the readmask stuff in regmap so > always good to see something new. > Me too. The SPI reads were unusual, and the datasheet mentions some sort of mask to enable proper consecutive register reads. Grepped the mask and discovered this. Since then, the reads are no longer incorrect :) Eva > Jonathan > > --- > > drivers/iio/accel/Kconfig | 13 +++ > > drivers/iio/accel/Makefile | 1 + > > drivers/iio/accel/adxl345_spi.c | 75 > > + > > 3 files changed, 89 insertions(+) > > create mode 100644 drivers/iio/accel/adxl345_spi.c > > > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > > index 5bdd87f..d474fed 100644 > > --- a/drivers/iio/accel/Kconfig > > +++ b/drivers/iio/accel/Kconfig > > @@ -21,6 +21,19 @@ config ADXL345_I2C > > To compile this driver as a module, choose M here: the > > module will be called adxl345_i2c. > > > > +config ADXL345_SPI > > + tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer SPI > > Driver" > > + depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m) > > + depends on SPI > > + select ADXL345 > > + select REGMAP_SPI > > + help > > + Say Y here if you want to build support for the Analog Devices > > + ADXL345 3-axis digital accelerometer. > > + > > + To compile this driver as a module, choose M here: the > > + module will be called adxl345_spi. > > + > > config BMA180 > > tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver" > > depends on I2C > > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile > > index 3f4a6d6..31fba19 100644 > > --- a/drivers/iio/accel/Makefile > > +++ b/drivers/iio/accel/Makefile > > @@ -5,6 +5,7 @@ > > # When adding new entries keep the list in alphabetical order > > obj-$(CONFIG_ADXL345) += adxl345_core.o > > obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o > > +obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o > > obj-$(CONFIG_BMA180) += bma180.o > > obj-$(CONFIG_BMA220) += bma220_spi.o > > obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o > > diff --git a/drivers/iio/accel/adxl345_spi.c > > b/drivers/iio/accel/adxl345_spi.c > > new file mode 100644 > > index 000..5fcd1fa > > --- /dev/null > > +++ b/drivers/iio/accel/adxl345_spi.c > > @@ -0,0 +1,75 @@ > > +/* > > + * ADXL345 3-Axis Digital Accelerometer > > + * > > + * Copyright (c) 2017 Eva Rachel Retuya > > + * > > + * This file is subject to the terms and conditions of version 2 of > > + * the GNU General Public License. See the file COPYING in the main > > + * directory of this archive for more details. > > + * > > + * SPI driver for ADXL345 > > + */ > > + > > +#include > > +#include > > +#include > > + > > +#include "adxl345.h" > > + > > +#define ADXL345_MAX_SPI_FREQ_HZ500 > > + > > +static const struct regmap_config adxl345_spi_regmap_config = { > > + .reg_bits = 8, > > + .val_bits = 8, > > +/* Setting bits 7 and 6 enables multiple-byte read */ > > + .read_flag_mask = BIT(7) | BIT(6), > Nice. I didn't know about that one ;) > > +}; > > + > > +static int adxl345_spi_probe(struct spi_device *spi) > > +{ > > + struct regmap *regmap; > > + const struct spi_device_id *id = spi_get_device_id(spi); > > + > > + /* Bail out if max_speed_hz exceeds 5 MHz */ > > + if (spi->max_speed_hz > ADXL345_MAX_SPI_FREQ_HZ) { > > + dev_err(>dev, "SPI CLK, %d Hz exceeds 5 MHz\n", > > + spi->max_speed_hz); > > + return -EINVAL; > > + } > > + > > + regmap = devm_regmap_init_spi(spi, _spi_regmap_config); > > + if (IS_ERR(regmap)) { > > + dev_err(>dev, "Error initializing spi regmap: %d\n", > > + (int)PTR_ERR(regmap)); > > + return PTR_ERR(regmap); > > + } > > + > > + return adxl345_common_probe(>dev, regmap, id->name); > > +} > > + > > +static int adxl345_spi_remove(struct spi_device *spi) > > +{ > > + return adxl345_common_remove(>dev); > > +} > > + > > +static const struct spi_device_id adxl345_spi_id[] = { > > + { "adxl345", 0 }, > > + { } > > +}; > > + > > +MODULE_DEVICE_TABLE(spi, adxl345_spi_id); > > + > > +static struct spi_driver adxl345_spi_driver = { > > + .driver = { > > + .name = "adxl345_spi", > > + }, > > + .probe = adxl345_spi_probe, > > + .remove = adxl345_spi_remove, > > + .id_table = adxl345_spi_id, > > +}; > > + > > +module_spi_driver(adxl345_spi_driver); > > + > > +MODULE_AUTHOR("Eva Rachel Retuya "); > > +MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer SPI driver"); > > +MODULE_LICENSE("GPL v2"); > > >
Re: [PATCH 2/4] iio: accel: adxl345: Split driver into core and I2C
On Sun, Feb 19, 2017 at 01:11:29PM +, Jonathan Cameron wrote: > On 16/02/17 10:02, Eva Rachel Retuya wrote: > > Move I2C-specific code into its own file and rely on regmap to access > > registers. The core code provides access to x, y, z and scale readings. > > > > Signed-off-by: Eva Rachel Retuya> A few minor comment inline. This unwound the comment about client->dev > in the previous patch, but I'd prefer to see it done there as then the > changes in here will be easier to see (with move detection hopefully working!) > > Jonathan > > --- > > drivers/iio/accel/Kconfig| 8 +- > > drivers/iio/accel/Makefile | 3 +- > > drivers/iio/accel/adxl345.c | 213 > > --- > > drivers/iio/accel/adxl345.h | 18 > > drivers/iio/accel/adxl345_core.c | 182 + > > drivers/iio/accel/adxl345_i2c.c | 70 + > This is a case where allowing git to notice the moves would have lead > to easier to read patches (unless this was sufficiently complex that git > couldn't follow it ;) Hello Jonathan, Thanks for the feedback. I amended the relevant commits as you say and git detects the file adxl345.c -> adxl345_core.c as 78% renamed. The reason for that percentage is that I did some probe/remove renames and some other deletions here and there. Should I commit them separately in order to get the move detection to work? > > 6 files changed, 278 insertions(+), 216 deletions(-) > > delete mode 100644 drivers/iio/accel/adxl345.c > > create mode 100644 drivers/iio/accel/adxl345.h > > create mode 100644 drivers/iio/accel/adxl345_core.c > > create mode 100644 drivers/iio/accel/adxl345_i2c.c > > > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > > index 26b8614..5bdd87f 100644 > > --- a/drivers/iio/accel/Kconfig > > +++ b/drivers/iio/accel/Kconfig > > @@ -6,16 +6,20 @@ > > menu "Accelerometers" > > > > config ADXL345 > > - tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer Driver" > > + tristate > > + > > +config ADXL345_I2C > > + tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer I2C > > Driver" > > depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m) > > depends on I2C > > + select ADXL345 > > select REGMAP_I2C > > help > > Say Y here if you want to build support for the Analog Devices > > ADXL345 3-axis digital accelerometer. > > > > To compile this driver as a module, choose M here: the > > - module will be called adxl345. > > + module will be called adxl345_i2c. > There are a couple of ways to do this. I personally kind of prefer the > way the bmc150 does it as it gives a cleaner set of options in Kconfig. > > Look at the various ways it is done and if you still prefer this one then > fair enough (it's how we always used to do it ;) Ack. I've seen it but went this way just to be explicit. I agree with doing it cleaner and will reflect it on v2. Eva > > > > config BMA180 > > tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver" > > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile > > index 618488d..3f4a6d6 100644 > > --- a/drivers/iio/accel/Makefile > > +++ b/drivers/iio/accel/Makefile > > @@ -3,7 +3,8 @@ > > # > > > > # When adding new entries keep the list in alphabetical order > > -obj-$(CONFIG_ADXL345) += adxl345.o > > +obj-$(CONFIG_ADXL345) += adxl345_core.o > > +obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o > > obj-$(CONFIG_BMA180) += bma180.o > > obj-$(CONFIG_BMA220) += bma220_spi.o > > obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o > > diff --git a/drivers/iio/accel/adxl345.c b/drivers/iio/accel/adxl345.c > > deleted file mode 100644 > > index a1fdf60..000 > > --- a/drivers/iio/accel/adxl345.c > > +++ /dev/null > > @@ -1,213 +0,0 @@ > > -/* > > - * ADXL345 3-Axis Digital Accelerometer > > - * > > - * Copyright (c) 2017 Eva Rachel Retuya > > - * > > - * This file is subject to the terms and conditions of version 2 of > > - * the GNU General Public License. See the file COPYING in the main > > - * directory of this archive for more details. > > - * > > - * IIO driver for ADXL345 > > - * 7-bit I2C slave address: 0x1D (ALT ADDRESS pin tied to VDDIO) or > > - * 0x53 (ALT ADDRESS pin grounded) > > - */ > > - > > -#include > > -#include > > -#include > > - > > -#include > > - > > -#define ADXL345_REG_DEVID 0x00 > > -#define ADXL345_REG_POWER_CTL 0x2D > > -#define ADXL345_REG_DATA_FORMAT0x31 > > -#define ADXL345_REG_DATAX0 0x32 > > -#define ADXL345_REG_DATAY0 0x34 > > -#define ADXL345_REG_DATAZ0 0x36 > > - > > -#define ADXL345_POWER_CTL_MEASURE BIT(3) > > -#define ADXL345_POWER_CTL_STANDBY 0x00 > > - > > -#define ADXL345_DATA_FORMAT_FULL_RES BIT(3) /* Up to 13-bits > > resolution */ > > -#define ADXL345_DATA_FORMAT_2G 0 > > -#define
Re: [PATCH] drm/rockchip: cdn-dp: Fix error handling
Le 20/02/2017 à 02:40, Mark yao a écrit : On 2017年02月20日 00:59, Christophe JAILLET wrote: It is likely that both 'clk_disable_unprepare()' should be called if 'pm_runtime_get_sync()' fails. Add a new label for that, because 'err_set_rate' is not meaningful in this case. Fixes: 1a0f7ed3abe2 ("drm/rockchip: cdn-dp: add cdn DP support for rk3399") Signed-off-by: Christophe JAILLET--- Not sure but a 'pm_runtime_get_sync()' is maybe also required in the 'err_set_rate' path. --- drivers/gpu/drm/rockchip/cdn-dp-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c index 9ab67a670885..0fe1ec8b8fb1 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c @@ -111,7 +111,7 @@ static int cdn_dp_clk_enable(struct cdn_dp_device *dp) ret = pm_runtime_get_sync(dp->dev); if (ret < 0) { DRM_DEV_ERROR(dp->dev, "cannot get pm runtime %d\n", ret); -goto err_pclk; +goto err_sync; I think the name err_pm_runtime_get is better. err_sync is not a clear name for the pm_runtime_get_sync. I will change it. } reset_control_assert(dp->core_rst); @@ -133,6 +133,7 @@ static int cdn_dp_clk_enable(struct cdn_dp_device *dp) return 0; err_set_rate: +err_sync: miss pm_runtime_put, it should be: I am wondering if 'pm_runtime_put_sync' should be added, instead. We want to revert the 'pm_runtime_get_sync' of line 111. According to the naming of the function, the _sync version looks more logical to me. Using ccoccinelle shows that 2/3 of functions calling both 'pm_runtime_get_sync' and 'pm_runtime_get[_sync]' and using the _sync variant. Which semantic is the correct one? err_set_rate: pm_runtime_put(dp->dev); err_pm_runtime_get: clk_disable_unprepare(dp->core_clk); err_core_clk: clk_disable_unprepare(dp->core_clk); err_core_clk: clk_disable_unprepare(dp->pclk);
Re: [PATCH 2/4] iio: accel: adxl345: Split driver into core and I2C
On Sun, Feb 19, 2017 at 01:11:29PM +, Jonathan Cameron wrote: > On 16/02/17 10:02, Eva Rachel Retuya wrote: > > Move I2C-specific code into its own file and rely on regmap to access > > registers. The core code provides access to x, y, z and scale readings. > > > > Signed-off-by: Eva Rachel Retuya > A few minor comment inline. This unwound the comment about client->dev > in the previous patch, but I'd prefer to see it done there as then the > changes in here will be easier to see (with move detection hopefully working!) > > Jonathan > > --- > > drivers/iio/accel/Kconfig| 8 +- > > drivers/iio/accel/Makefile | 3 +- > > drivers/iio/accel/adxl345.c | 213 > > --- > > drivers/iio/accel/adxl345.h | 18 > > drivers/iio/accel/adxl345_core.c | 182 + > > drivers/iio/accel/adxl345_i2c.c | 70 + > This is a case where allowing git to notice the moves would have lead > to easier to read patches (unless this was sufficiently complex that git > couldn't follow it ;) Hello Jonathan, Thanks for the feedback. I amended the relevant commits as you say and git detects the file adxl345.c -> adxl345_core.c as 78% renamed. The reason for that percentage is that I did some probe/remove renames and some other deletions here and there. Should I commit them separately in order to get the move detection to work? > > 6 files changed, 278 insertions(+), 216 deletions(-) > > delete mode 100644 drivers/iio/accel/adxl345.c > > create mode 100644 drivers/iio/accel/adxl345.h > > create mode 100644 drivers/iio/accel/adxl345_core.c > > create mode 100644 drivers/iio/accel/adxl345_i2c.c > > > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > > index 26b8614..5bdd87f 100644 > > --- a/drivers/iio/accel/Kconfig > > +++ b/drivers/iio/accel/Kconfig > > @@ -6,16 +6,20 @@ > > menu "Accelerometers" > > > > config ADXL345 > > - tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer Driver" > > + tristate > > + > > +config ADXL345_I2C > > + tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer I2C > > Driver" > > depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m) > > depends on I2C > > + select ADXL345 > > select REGMAP_I2C > > help > > Say Y here if you want to build support for the Analog Devices > > ADXL345 3-axis digital accelerometer. > > > > To compile this driver as a module, choose M here: the > > - module will be called adxl345. > > + module will be called adxl345_i2c. > There are a couple of ways to do this. I personally kind of prefer the > way the bmc150 does it as it gives a cleaner set of options in Kconfig. > > Look at the various ways it is done and if you still prefer this one then > fair enough (it's how we always used to do it ;) Ack. I've seen it but went this way just to be explicit. I agree with doing it cleaner and will reflect it on v2. Eva > > > > config BMA180 > > tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver" > > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile > > index 618488d..3f4a6d6 100644 > > --- a/drivers/iio/accel/Makefile > > +++ b/drivers/iio/accel/Makefile > > @@ -3,7 +3,8 @@ > > # > > > > # When adding new entries keep the list in alphabetical order > > -obj-$(CONFIG_ADXL345) += adxl345.o > > +obj-$(CONFIG_ADXL345) += adxl345_core.o > > +obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o > > obj-$(CONFIG_BMA180) += bma180.o > > obj-$(CONFIG_BMA220) += bma220_spi.o > > obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o > > diff --git a/drivers/iio/accel/adxl345.c b/drivers/iio/accel/adxl345.c > > deleted file mode 100644 > > index a1fdf60..000 > > --- a/drivers/iio/accel/adxl345.c > > +++ /dev/null > > @@ -1,213 +0,0 @@ > > -/* > > - * ADXL345 3-Axis Digital Accelerometer > > - * > > - * Copyright (c) 2017 Eva Rachel Retuya > > - * > > - * This file is subject to the terms and conditions of version 2 of > > - * the GNU General Public License. See the file COPYING in the main > > - * directory of this archive for more details. > > - * > > - * IIO driver for ADXL345 > > - * 7-bit I2C slave address: 0x1D (ALT ADDRESS pin tied to VDDIO) or > > - * 0x53 (ALT ADDRESS pin grounded) > > - */ > > - > > -#include > > -#include > > -#include > > - > > -#include > > - > > -#define ADXL345_REG_DEVID 0x00 > > -#define ADXL345_REG_POWER_CTL 0x2D > > -#define ADXL345_REG_DATA_FORMAT0x31 > > -#define ADXL345_REG_DATAX0 0x32 > > -#define ADXL345_REG_DATAY0 0x34 > > -#define ADXL345_REG_DATAZ0 0x36 > > - > > -#define ADXL345_POWER_CTL_MEASURE BIT(3) > > -#define ADXL345_POWER_CTL_STANDBY 0x00 > > - > > -#define ADXL345_DATA_FORMAT_FULL_RES BIT(3) /* Up to 13-bits > > resolution */ > > -#define ADXL345_DATA_FORMAT_2G 0 > > -#define ADXL345_DATA_FORMAT_4G 1 > > -#define
Re: [PATCH] drm/rockchip: cdn-dp: Fix error handling
Le 20/02/2017 à 02:40, Mark yao a écrit : On 2017年02月20日 00:59, Christophe JAILLET wrote: It is likely that both 'clk_disable_unprepare()' should be called if 'pm_runtime_get_sync()' fails. Add a new label for that, because 'err_set_rate' is not meaningful in this case. Fixes: 1a0f7ed3abe2 ("drm/rockchip: cdn-dp: add cdn DP support for rk3399") Signed-off-by: Christophe JAILLET --- Not sure but a 'pm_runtime_get_sync()' is maybe also required in the 'err_set_rate' path. --- drivers/gpu/drm/rockchip/cdn-dp-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c index 9ab67a670885..0fe1ec8b8fb1 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c @@ -111,7 +111,7 @@ static int cdn_dp_clk_enable(struct cdn_dp_device *dp) ret = pm_runtime_get_sync(dp->dev); if (ret < 0) { DRM_DEV_ERROR(dp->dev, "cannot get pm runtime %d\n", ret); -goto err_pclk; +goto err_sync; I think the name err_pm_runtime_get is better. err_sync is not a clear name for the pm_runtime_get_sync. I will change it. } reset_control_assert(dp->core_rst); @@ -133,6 +133,7 @@ static int cdn_dp_clk_enable(struct cdn_dp_device *dp) return 0; err_set_rate: +err_sync: miss pm_runtime_put, it should be: I am wondering if 'pm_runtime_put_sync' should be added, instead. We want to revert the 'pm_runtime_get_sync' of line 111. According to the naming of the function, the _sync version looks more logical to me. Using ccoccinelle shows that 2/3 of functions calling both 'pm_runtime_get_sync' and 'pm_runtime_get[_sync]' and using the _sync variant. Which semantic is the correct one? err_set_rate: pm_runtime_put(dp->dev); err_pm_runtime_get: clk_disable_unprepare(dp->core_clk); err_core_clk: clk_disable_unprepare(dp->core_clk); err_core_clk: clk_disable_unprepare(dp->pclk);
Re: [PATCH v6 1/3] perf: add PERF_RECORD_NAMESPACES to include namespaces related info
Hi Eric, On Thursday 16 February 2017 04:55 PM, Eric W. Biederman wrote: +/* + * The maximum size of the name of each namespace + */ +#define NS_NAME_SIZE 8 + +struct perf_ns_link_info { + charname[NS_NAME_SIZE]; + __u64 dev; + __u64 ino; +}; Ugh. I missed the name the first time around. That really looks like useless clutter. You already know the index so the name doesn't add any information, so unless I am missing something that name will just slow down the perf kernel implementation with useless work. The userspace reader can have the information just as reliably by looking at the index and indexing into a table. The set of namespaces changes slowly enough that this is not likely to be a problem in practice. Especially as perf is released with the kernel. Plus who knows how long the name of the next namespace is going to be. Agreed. Will drop name field from the structure and use an indexing table to get names in userspace.. Thanks Hari
Re: [PATCH v6 1/3] perf: add PERF_RECORD_NAMESPACES to include namespaces related info
Hi Eric, On Thursday 16 February 2017 04:55 PM, Eric W. Biederman wrote: +/* + * The maximum size of the name of each namespace + */ +#define NS_NAME_SIZE 8 + +struct perf_ns_link_info { + charname[NS_NAME_SIZE]; + __u64 dev; + __u64 ino; +}; Ugh. I missed the name the first time around. That really looks like useless clutter. You already know the index so the name doesn't add any information, so unless I am missing something that name will just slow down the perf kernel implementation with useless work. The userspace reader can have the information just as reliably by looking at the index and indexing into a table. The set of namespaces changes slowly enough that this is not likely to be a problem in practice. Especially as perf is released with the kernel. Plus who knows how long the name of the next namespace is going to be. Agreed. Will drop name field from the structure and use an indexing table to get names in userspace.. Thanks Hari
[GIT PULL] power-supply changes for 4.11
Hi Linus, Everything in my pull-request has been in linux-next for three weeks. There are overlappings with mfd & arm, but no problems were reported by Stephen, so I assume the immutable branches worked as expected. -- Sebastian The following changes since commit 7ce7d89f48834cefece7804d38fc5d85382edf77: Linux 4.10-rc1 (2016-12-25 16:13:08 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git tags/for-v4.11 for you to fetch changes up to 744cc304a18f1c9de4f3215fbe93fe878f934179: power: supply: add AC power supply driver for AXP20X and AXP22X PMICs (2017-01-29 23:15:18 +0100) power supply and reset changes for the v4.11 series * New drivers - sbs-charger driver - max14656_charger_detector - axp20x_ac_power * New chip/feature support - axp20x_usb_power: add AXP223 support - tps65217: add usb charger support - qcom_smbb: support otg regulator - at91-reset: add samx7 support * Dropped drivers - intel_mid_battery (platform was dropped) * Fixes - at91-poweroff: avoid wearing off LPDDR memory - replace deprecated extcon API - lots of cleanup and style fixes - misc. minor functionality fixes Alexander Kurz (2): dt-bindings: power: supply: Add max14656_charger_detector power: supply: Add support for MAX14656 USB charger detector Alexandre Belloni (3): ARM: at91: define LPDDR types power: reset: at91-poweroff: timely shutdown LPDDR memories power: reset: at91-reset: remove leftover platform_device_id Andy Shevchenko (1): power: supply: remove Intel Moorestown battery support Arnd Bergmann (1): power: supply: qcom_smbb: add regulator dependency Bird, Tim (2): dt-bindings: power: supply: Add otg regulator binding power: supply: qcom_smbb: Add otg regulator for control of vbus Chanwoo Choi (2): power: supply: axp288_charger: Replace the extcon API power: supply: qcom_smbb: Replace the deprecated extcon API Chris Lapa (12): power: supply: bq27xxx: move overtemp tests to a switch statement. power: supply: bq27xxx: rename BQ27500 allow for deprecation in future. power: supply: bq27xxx: rename BQ27510 allow for deprecation in future. power: supply: bq27xxx: adds specific support for bq27500/1 revision. power: supply: bq27xxx: adds specific support for bq27510-g1 revision. power: supply: bq27xxx: adds specific support for bq27510-g2 revision. power: supply: bq27xxx: adds specific support for bq27510-g3 revision. power: supply: bq27xxx: adds specific support for bq27520-g1 revision. power: supply: bq27xxx: adds specific support for bq27520-g2 revision. power: supply: bq27xxx: adds specific support for bq27520-g3 revision. power: supply: bq27xxx: adds specific support for bq27520-g4 revision. power: supply: bq27xxx: adds device tree binding documentation. Colin Ian King (3): power: supply: wm97xx_battery: remove redundant 2nd null check on pdata power: supply: fix spelling mistake: supply: "Celcius" -> "Celsius" power: supply: bq2415x: check for NULL acpi_id to avoid null pointer dereference Gustavo A. R. Silva (2): power: supply: ab8500_btemp: Compress return logic into one line. power: supply: pcf50633-charger: Compress return logic into one line. Hans de Goede (18): power: supply: axp288_charger: Make charger_init_hw_regs propagate i2c errors power: supply: axp288_charger: Drop platform_data dependency power: supply: axp288_fuel_gauge: Drop platform_data dependency power: supply: axp288_fuel_gauge: Fix fuel_gauge_reg_readb return on error power: supply: axp288_fuel_gauge: Read 15 bit values 2 registers at a time power: supply: axp288_fuel_gauge: Read 12 bit values 2 registers at a time power: supply: axp288_charger: Use devm_power_supply_register power: supply: axp288_charger: Register extcon notifers after power_supply power: supply: axp288_charger: Move init_hw_regs call before supply registration power: supply: axp288_charger: Actually get and use the USB_HOST extcon device power: supply: axp288_charger: Handle charger type changing without disconnect power: supply: axp288_charger: Some minor cleanups power: supply: axp288_charger: Get and process initial hardware-state power: supply: axp288_charger: Fix wrong regmap_update_bits power: supply: axp288_charger: Remove unnecessary irq?_en register writes power: supply: axp288_charger: Fix the module not auto-loading power: supply: axp288_charger: Use one notifier_block per extcon cable power: supply: axp288_fuel_gauge: Remove unnecessary irq?_en register writes Javier Martinez Canillas (1): power: supply: max14656: Export I2C and OF device ID as module
[GIT PULL] power-supply changes for 4.11
Hi Linus, Everything in my pull-request has been in linux-next for three weeks. There are overlappings with mfd & arm, but no problems were reported by Stephen, so I assume the immutable branches worked as expected. -- Sebastian The following changes since commit 7ce7d89f48834cefece7804d38fc5d85382edf77: Linux 4.10-rc1 (2016-12-25 16:13:08 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git tags/for-v4.11 for you to fetch changes up to 744cc304a18f1c9de4f3215fbe93fe878f934179: power: supply: add AC power supply driver for AXP20X and AXP22X PMICs (2017-01-29 23:15:18 +0100) power supply and reset changes for the v4.11 series * New drivers - sbs-charger driver - max14656_charger_detector - axp20x_ac_power * New chip/feature support - axp20x_usb_power: add AXP223 support - tps65217: add usb charger support - qcom_smbb: support otg regulator - at91-reset: add samx7 support * Dropped drivers - intel_mid_battery (platform was dropped) * Fixes - at91-poweroff: avoid wearing off LPDDR memory - replace deprecated extcon API - lots of cleanup and style fixes - misc. minor functionality fixes Alexander Kurz (2): dt-bindings: power: supply: Add max14656_charger_detector power: supply: Add support for MAX14656 USB charger detector Alexandre Belloni (3): ARM: at91: define LPDDR types power: reset: at91-poweroff: timely shutdown LPDDR memories power: reset: at91-reset: remove leftover platform_device_id Andy Shevchenko (1): power: supply: remove Intel Moorestown battery support Arnd Bergmann (1): power: supply: qcom_smbb: add regulator dependency Bird, Tim (2): dt-bindings: power: supply: Add otg regulator binding power: supply: qcom_smbb: Add otg regulator for control of vbus Chanwoo Choi (2): power: supply: axp288_charger: Replace the extcon API power: supply: qcom_smbb: Replace the deprecated extcon API Chris Lapa (12): power: supply: bq27xxx: move overtemp tests to a switch statement. power: supply: bq27xxx: rename BQ27500 allow for deprecation in future. power: supply: bq27xxx: rename BQ27510 allow for deprecation in future. power: supply: bq27xxx: adds specific support for bq27500/1 revision. power: supply: bq27xxx: adds specific support for bq27510-g1 revision. power: supply: bq27xxx: adds specific support for bq27510-g2 revision. power: supply: bq27xxx: adds specific support for bq27510-g3 revision. power: supply: bq27xxx: adds specific support for bq27520-g1 revision. power: supply: bq27xxx: adds specific support for bq27520-g2 revision. power: supply: bq27xxx: adds specific support for bq27520-g3 revision. power: supply: bq27xxx: adds specific support for bq27520-g4 revision. power: supply: bq27xxx: adds device tree binding documentation. Colin Ian King (3): power: supply: wm97xx_battery: remove redundant 2nd null check on pdata power: supply: fix spelling mistake: supply: "Celcius" -> "Celsius" power: supply: bq2415x: check for NULL acpi_id to avoid null pointer dereference Gustavo A. R. Silva (2): power: supply: ab8500_btemp: Compress return logic into one line. power: supply: pcf50633-charger: Compress return logic into one line. Hans de Goede (18): power: supply: axp288_charger: Make charger_init_hw_regs propagate i2c errors power: supply: axp288_charger: Drop platform_data dependency power: supply: axp288_fuel_gauge: Drop platform_data dependency power: supply: axp288_fuel_gauge: Fix fuel_gauge_reg_readb return on error power: supply: axp288_fuel_gauge: Read 15 bit values 2 registers at a time power: supply: axp288_fuel_gauge: Read 12 bit values 2 registers at a time power: supply: axp288_charger: Use devm_power_supply_register power: supply: axp288_charger: Register extcon notifers after power_supply power: supply: axp288_charger: Move init_hw_regs call before supply registration power: supply: axp288_charger: Actually get and use the USB_HOST extcon device power: supply: axp288_charger: Handle charger type changing without disconnect power: supply: axp288_charger: Some minor cleanups power: supply: axp288_charger: Get and process initial hardware-state power: supply: axp288_charger: Fix wrong regmap_update_bits power: supply: axp288_charger: Remove unnecessary irq?_en register writes power: supply: axp288_charger: Fix the module not auto-loading power: supply: axp288_charger: Use one notifier_block per extcon cable power: supply: axp288_fuel_gauge: Remove unnecessary irq?_en register writes Javier Martinez Canillas (1): power: supply: max14656: Export I2C and OF device ID as module
[PATCH v2] x86/mce: Don't participate in rendezvous process once nmi_shootdown_cpus() was made
We met an issue for kdump: after kdump kernel boots up, and there comes a broadcasted mce in first kernel, the other cpus remaining in first kernel will enter the old mce handler of first kernel, then timeout and panic due to MCE synchronization, finally reset the kdump cpus. This patch lets cpus stay quiet after nmi_shootdown_cpus(), so before crash cpu shots them down or after kdump boots, they should not do anything except clearing MCG_STATUS in case of broadcasted mce. This is useful for kdump to let the vmcore dumping perform as hard as it can. Previous efforts: https://patchwork.kernel.org/patch/6167631/ https://lists.gt.net/linux/kernel/2146557 Cc: Naoya HoriguchiSuggested-by: Borislav Petkov Signed-off-by: Xunlei Pang --- v1->v2: Using crashing_cpu according to Borislav's suggestion. arch/x86/include/asm/reboot.h| 1 + arch/x86/kernel/cpu/mcheck/mce.c | 6 -- arch/x86/kernel/reboot.c | 16 +++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/arch/x86/include/asm/reboot.h b/arch/x86/include/asm/reboot.h index 2cb1cc2..ec8657b6 100644 --- a/arch/x86/include/asm/reboot.h +++ b/arch/x86/include/asm/reboot.h @@ -26,5 +26,6 @@ struct machine_ops { typedef void (*nmi_shootdown_cb)(int, struct pt_regs*); void nmi_shootdown_cpus(nmi_shootdown_cb callback); void run_crash_ipi_callback(struct pt_regs *regs); +bool cpus_shotdown(void); #endif /* _ASM_X86_REBOOT_H */ diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c index 8e9725c..3b56710 100644 --- a/arch/x86/kernel/cpu/mcheck/mce.c +++ b/arch/x86/kernel/cpu/mcheck/mce.c @@ -49,6 +49,7 @@ #include #include #include +#include #include "mce-internal.h" @@ -1128,8 +1129,9 @@ void do_machine_check(struct pt_regs *regs, long error_code) */ int lmce = 1; - /* If this CPU is offline, just bail out. */ - if (cpu_is_offline(smp_processor_id())) { + /* If nmi shootdown happened or this CPU is offline, just bail out. */ + if (cpus_shotdown() || + cpu_is_offline(smp_processor_id())) { u64 mcgstatus; mcgstatus = mce_rdmsrl(MSR_IA32_MCG_STATUS); diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c index e244c19..b301c8d 100644 --- a/arch/x86/kernel/reboot.c +++ b/arch/x86/kernel/reboot.c @@ -752,7 +752,7 @@ void machine_crash_shutdown(struct pt_regs *regs) #if defined(CONFIG_SMP) /* This keeps a track of which one is crashing cpu. */ -static int crashing_cpu; +static int crashing_cpu = -1; static nmi_shootdown_cb shootdown_callback; static atomic_t waiting_for_crash_ipi; @@ -852,6 +852,14 @@ void nmi_panic_self_stop(struct pt_regs *regs) } } +bool cpus_shotdown(void) +{ + if (crashing_cpu != -1) + return true; + + return false; +} + #else /* !CONFIG_SMP */ void nmi_shootdown_cpus(nmi_shootdown_cb callback) { @@ -861,4 +869,10 @@ void nmi_shootdown_cpus(nmi_shootdown_cb callback) void run_crash_ipi_callback(struct pt_regs *regs) { } + +bool cpus_shotdown(void) +{ + return false; +} + #endif -- 1.8.3.1
[PATCH v2] x86/mce: Don't participate in rendezvous process once nmi_shootdown_cpus() was made
We met an issue for kdump: after kdump kernel boots up, and there comes a broadcasted mce in first kernel, the other cpus remaining in first kernel will enter the old mce handler of first kernel, then timeout and panic due to MCE synchronization, finally reset the kdump cpus. This patch lets cpus stay quiet after nmi_shootdown_cpus(), so before crash cpu shots them down or after kdump boots, they should not do anything except clearing MCG_STATUS in case of broadcasted mce. This is useful for kdump to let the vmcore dumping perform as hard as it can. Previous efforts: https://patchwork.kernel.org/patch/6167631/ https://lists.gt.net/linux/kernel/2146557 Cc: Naoya Horiguchi Suggested-by: Borislav Petkov Signed-off-by: Xunlei Pang --- v1->v2: Using crashing_cpu according to Borislav's suggestion. arch/x86/include/asm/reboot.h| 1 + arch/x86/kernel/cpu/mcheck/mce.c | 6 -- arch/x86/kernel/reboot.c | 16 +++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/arch/x86/include/asm/reboot.h b/arch/x86/include/asm/reboot.h index 2cb1cc2..ec8657b6 100644 --- a/arch/x86/include/asm/reboot.h +++ b/arch/x86/include/asm/reboot.h @@ -26,5 +26,6 @@ struct machine_ops { typedef void (*nmi_shootdown_cb)(int, struct pt_regs*); void nmi_shootdown_cpus(nmi_shootdown_cb callback); void run_crash_ipi_callback(struct pt_regs *regs); +bool cpus_shotdown(void); #endif /* _ASM_X86_REBOOT_H */ diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c index 8e9725c..3b56710 100644 --- a/arch/x86/kernel/cpu/mcheck/mce.c +++ b/arch/x86/kernel/cpu/mcheck/mce.c @@ -49,6 +49,7 @@ #include #include #include +#include #include "mce-internal.h" @@ -1128,8 +1129,9 @@ void do_machine_check(struct pt_regs *regs, long error_code) */ int lmce = 1; - /* If this CPU is offline, just bail out. */ - if (cpu_is_offline(smp_processor_id())) { + /* If nmi shootdown happened or this CPU is offline, just bail out. */ + if (cpus_shotdown() || + cpu_is_offline(smp_processor_id())) { u64 mcgstatus; mcgstatus = mce_rdmsrl(MSR_IA32_MCG_STATUS); diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c index e244c19..b301c8d 100644 --- a/arch/x86/kernel/reboot.c +++ b/arch/x86/kernel/reboot.c @@ -752,7 +752,7 @@ void machine_crash_shutdown(struct pt_regs *regs) #if defined(CONFIG_SMP) /* This keeps a track of which one is crashing cpu. */ -static int crashing_cpu; +static int crashing_cpu = -1; static nmi_shootdown_cb shootdown_callback; static atomic_t waiting_for_crash_ipi; @@ -852,6 +852,14 @@ void nmi_panic_self_stop(struct pt_regs *regs) } } +bool cpus_shotdown(void) +{ + if (crashing_cpu != -1) + return true; + + return false; +} + #else /* !CONFIG_SMP */ void nmi_shootdown_cpus(nmi_shootdown_cb callback) { @@ -861,4 +869,10 @@ void nmi_shootdown_cpus(nmi_shootdown_cb callback) void run_crash_ipi_callback(struct pt_regs *regs) { } + +bool cpus_shotdown(void) +{ + return false; +} + #endif -- 1.8.3.1
Re: [Xen-devel] [PATCH v2] xen, input: try to read screen resolution for xen-kbdfront
On 17/02/17 20:47, Dmitry Torokhov wrote: > On Mon, Jan 30, 2017 at 01:27:29PM +0200, Oleksandr Andrushchenko wrote: >> On 01/30/2017 01:23 PM, Juergen Gross wrote: >>> On 27/01/17 17:10, Dmitry Torokhov wrote: On January 27, 2017 12:31:19 AM PST, Juergen Grosswrote: > On 27/01/17 09:26, Oleksandr Andrushchenko wrote: >> On 01/27/2017 10:14 AM, Juergen Gross wrote: >>> On 27/01/17 08:53, Oleksandr Andrushchenko wrote: On 01/27/2017 09:46 AM, Juergen Gross wrote: > On 27/01/17 08:21, Oleksandr Andrushchenko wrote: >> On 01/27/2017 09:12 AM, Juergen Gross wrote: >>> Instead of using the default resolution of 800*600 for the > pointing >>> device of xen-kbdfront try to read the resolution of the > (virtual) >>> framebuffer device. Use the default as fallback only. >>> >>> Signed-off-by: Juergen Gross >>> --- >>> V2: get framebuffer resolution only if CONFIG_FB (Dmitry > Torokhov) >>> --- >>> drivers/input/misc/xen-kbdfront.c | 15 --- >>> 1 file changed, 12 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/input/misc/xen-kbdfront.c >>> b/drivers/input/misc/xen-kbdfront.c >>> index 3900875..3aae9b4 100644 >>> --- a/drivers/input/misc/xen-kbdfront.c >>> +++ b/drivers/input/misc/xen-kbdfront.c >>> @@ -16,6 +16,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> #include >>> #include >>> @@ -108,7 +109,7 @@ static irqreturn_t input_handler(int rq, > void >>> *dev_id) >>> static int xenkbd_probe(struct xenbus_device *dev, >>> const struct xenbus_device_id *id) >>> { >>> -int ret, i; >>> +int ret, i, width, height; >>> unsigned int abs; >>> struct xenkbd_info *info; >>> struct input_dev *kbd, *ptr; >>> @@ -173,9 +174,17 @@ static int xenkbd_probe(struct > xenbus_device >>> *dev, >>> ptr->id.product = 0xfffe; >>> if (abs) { >>> +width = XENFB_WIDTH; >>> +height = XENFB_HEIGHT; >>> +#ifdef CONFIG_FB >>> +if (registered_fb[0]) { >> This still will not help if FB gets registered after kbd+ptr > Hmm, so you think I should add a call to fb_register_client() to > get > events for new registered framebuffer devices? yes, but also pay attention to CONFIG_FB_NOTIFY: you may still end up w/o notification. >>> Okay, that's not worse than today. >> agree > This would probably work. I'll have a try. > > > Thanks, > > Juergen My bigger concern here is that we try to tie keyboard and pointer > device to the framebuffer. IMO, these are independent parts of the system > and the relation depends on the use-case. One can have graphics enabled w/o > framebuffer at all, e.g. DRM/KMS + OpenGLES + Weston + kbd + ptr... >>> Again: that's a use case which will work as today. The current > defaults >>> are being used. >>> >>> The question is whether we should add a module parameter switching > off >>> the automatic adaption of the resolution as there might be use cases >>> where we don't want this feature. >> I think for those who doesn't want this resolution there is >> still a possibility to change it on backend's XenbusStateConnected >> So, no need for module parameter, IMO > Fine. > > I'll send V3 soon. How about you do the axis adjustment from userspace (udev rule), and leave kernel as is? >>> Hmm, is this a good idea? >>> >>> I'd need a udev rule to trigger when either the pointing device or a >>> new frame buffer is showing up. In both cases I need to read the >>> geometry of the frame buffer (in case it exists) and set the geometry >>> of the pointing device (in case it exists) to the same values. This >>> seems to be much more complicated than the required changes in the >>> driver. >>> >>> I could be wrong, of course, especially as I'm no expert in writing >>> udev rules. :-) >> And you may also end up with thin Dom0 w/o udev at all... > > So what piece of software is using resolution of this input device and > why it has to match screen resolution? What happens when framebuffer is > registered after input device is created? I see that in the later > version of the patch you hook the notifier and change values, but how > users would know about that? The problem showed up in out internal QA: the installer is being tested automatically in a virtual machine. The output of the (virtual) framebuffer is captured
Re: [Xen-devel] [PATCH v2] xen, input: try to read screen resolution for xen-kbdfront
On 17/02/17 20:47, Dmitry Torokhov wrote: > On Mon, Jan 30, 2017 at 01:27:29PM +0200, Oleksandr Andrushchenko wrote: >> On 01/30/2017 01:23 PM, Juergen Gross wrote: >>> On 27/01/17 17:10, Dmitry Torokhov wrote: On January 27, 2017 12:31:19 AM PST, Juergen Gross wrote: > On 27/01/17 09:26, Oleksandr Andrushchenko wrote: >> On 01/27/2017 10:14 AM, Juergen Gross wrote: >>> On 27/01/17 08:53, Oleksandr Andrushchenko wrote: On 01/27/2017 09:46 AM, Juergen Gross wrote: > On 27/01/17 08:21, Oleksandr Andrushchenko wrote: >> On 01/27/2017 09:12 AM, Juergen Gross wrote: >>> Instead of using the default resolution of 800*600 for the > pointing >>> device of xen-kbdfront try to read the resolution of the > (virtual) >>> framebuffer device. Use the default as fallback only. >>> >>> Signed-off-by: Juergen Gross >>> --- >>> V2: get framebuffer resolution only if CONFIG_FB (Dmitry > Torokhov) >>> --- >>> drivers/input/misc/xen-kbdfront.c | 15 --- >>> 1 file changed, 12 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/input/misc/xen-kbdfront.c >>> b/drivers/input/misc/xen-kbdfront.c >>> index 3900875..3aae9b4 100644 >>> --- a/drivers/input/misc/xen-kbdfront.c >>> +++ b/drivers/input/misc/xen-kbdfront.c >>> @@ -16,6 +16,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> #include >>> #include >>> @@ -108,7 +109,7 @@ static irqreturn_t input_handler(int rq, > void >>> *dev_id) >>> static int xenkbd_probe(struct xenbus_device *dev, >>> const struct xenbus_device_id *id) >>> { >>> -int ret, i; >>> +int ret, i, width, height; >>> unsigned int abs; >>> struct xenkbd_info *info; >>> struct input_dev *kbd, *ptr; >>> @@ -173,9 +174,17 @@ static int xenkbd_probe(struct > xenbus_device >>> *dev, >>> ptr->id.product = 0xfffe; >>> if (abs) { >>> +width = XENFB_WIDTH; >>> +height = XENFB_HEIGHT; >>> +#ifdef CONFIG_FB >>> +if (registered_fb[0]) { >> This still will not help if FB gets registered after kbd+ptr > Hmm, so you think I should add a call to fb_register_client() to > get > events for new registered framebuffer devices? yes, but also pay attention to CONFIG_FB_NOTIFY: you may still end up w/o notification. >>> Okay, that's not worse than today. >> agree > This would probably work. I'll have a try. > > > Thanks, > > Juergen My bigger concern here is that we try to tie keyboard and pointer > device to the framebuffer. IMO, these are independent parts of the system > and the relation depends on the use-case. One can have graphics enabled w/o > framebuffer at all, e.g. DRM/KMS + OpenGLES + Weston + kbd + ptr... >>> Again: that's a use case which will work as today. The current > defaults >>> are being used. >>> >>> The question is whether we should add a module parameter switching > off >>> the automatic adaption of the resolution as there might be use cases >>> where we don't want this feature. >> I think for those who doesn't want this resolution there is >> still a possibility to change it on backend's XenbusStateConnected >> So, no need for module parameter, IMO > Fine. > > I'll send V3 soon. How about you do the axis adjustment from userspace (udev rule), and leave kernel as is? >>> Hmm, is this a good idea? >>> >>> I'd need a udev rule to trigger when either the pointing device or a >>> new frame buffer is showing up. In both cases I need to read the >>> geometry of the frame buffer (in case it exists) and set the geometry >>> of the pointing device (in case it exists) to the same values. This >>> seems to be much more complicated than the required changes in the >>> driver. >>> >>> I could be wrong, of course, especially as I'm no expert in writing >>> udev rules. :-) >> And you may also end up with thin Dom0 w/o udev at all... > > So what piece of software is using resolution of this input device and > why it has to match screen resolution? What happens when framebuffer is > registered after input device is created? I see that in the later > version of the patch you hook the notifier and change values, but how > users would know about that? The problem showed up in out internal QA: the installer is being tested automatically in a virtual machine. The output of the (virtual) framebuffer is captured automatically and verified to match
[PATCH 0/2] Revert works for the mapping of cpuid <-> nodeid
Currently, We make the mapping of "cpuid <-> nodeid" fixed at the booting time. It keeps consistent with the WorkQueue and avoids some bugs which may be caused by the dynamic assignment. As we know, It is implemented by the patches as follows: 2532fc318d, f7c28833c2, 8f54969dc8, 8ad893faf2, dc6db24d24, which depend on ACPI table. Simply speaking: Step 1. Make the "Logical CPU ID <-> Processor ID/UID" fixed Using MADT: We generate the logical CPU IDs by the Local APIC/x2APIC IDs orderly and get the mapping of Processor ID/UID <-> Local Apic ID directly in MADT. So, we get the mapping of *Processor ID/UID <-> Local Apic ID <-> Logical CPU ID* Step 2. Make the "Processor ID/UID <-> Node ID(_PXM)" fixed Using DSDT: The maaping of "Processor ID/UID <-> Node ID(_PXM)" is ready-made in each entities. we just use it directly. So, at last we get the maaping of *Node ID <-> Logical CPU ID* according to step1 and step2: *Node ID(_PXM) <-> Processor ID/UID <-> Local Apic ID <-> Logical CPU ID* But, The ACPI table is unreliable and it is very risky that we use the entity which isn't related to a physical device at booting time. Here has already two bugs we found. 1. Duplicated Processor IDs in DSDT. It has been fixed by commit 8e089eaa19, fd74da217d. 2. The _PXM in DSDT is inconsistent with the one in MADT. It may cause the bug, which is shown in: https://lkml.org/lkml/2017/2/12/200 There may be more later. We shouldn't just only fix them everytime, we should solve this problem from the source to avoid such problems happend again and again. Now, a simple and easy way is found, we revert our patches. Do the Step 2 at hot-plug time, not at booting time where we did some useless work. It also can make the mapping of "cpuid <-> nodeid" fixed and avoid excessive use of the ACPI table. We have tested them in our box: Fujitsu PQ2000 with 2 nodes for hot-plug. To Xiaolong: Please help me to test it in the special machine. Dou Liyang (2): Revert"x86/acpi: Set persistent cpuid <-> nodeid mapping Revert"x86, acpi, cpu-hotplug: Enable MADT APIs to return disabled apicid" arch/x86/kernel/acpi/boot.c | 2 +- drivers/acpi/acpi_processor.c | 5 -- drivers/acpi/bus.c| 1 - drivers/acpi/processor_core.c | 133 +++--- include/linux/acpi.h | 3 - 5 files changed, 23 insertions(+), 121 deletions(-) -- 2.5.5
[PATCH 0/2] Revert works for the mapping of cpuid <-> nodeid
Currently, We make the mapping of "cpuid <-> nodeid" fixed at the booting time. It keeps consistent with the WorkQueue and avoids some bugs which may be caused by the dynamic assignment. As we know, It is implemented by the patches as follows: 2532fc318d, f7c28833c2, 8f54969dc8, 8ad893faf2, dc6db24d24, which depend on ACPI table. Simply speaking: Step 1. Make the "Logical CPU ID <-> Processor ID/UID" fixed Using MADT: We generate the logical CPU IDs by the Local APIC/x2APIC IDs orderly and get the mapping of Processor ID/UID <-> Local Apic ID directly in MADT. So, we get the mapping of *Processor ID/UID <-> Local Apic ID <-> Logical CPU ID* Step 2. Make the "Processor ID/UID <-> Node ID(_PXM)" fixed Using DSDT: The maaping of "Processor ID/UID <-> Node ID(_PXM)" is ready-made in each entities. we just use it directly. So, at last we get the maaping of *Node ID <-> Logical CPU ID* according to step1 and step2: *Node ID(_PXM) <-> Processor ID/UID <-> Local Apic ID <-> Logical CPU ID* But, The ACPI table is unreliable and it is very risky that we use the entity which isn't related to a physical device at booting time. Here has already two bugs we found. 1. Duplicated Processor IDs in DSDT. It has been fixed by commit 8e089eaa19, fd74da217d. 2. The _PXM in DSDT is inconsistent with the one in MADT. It may cause the bug, which is shown in: https://lkml.org/lkml/2017/2/12/200 There may be more later. We shouldn't just only fix them everytime, we should solve this problem from the source to avoid such problems happend again and again. Now, a simple and easy way is found, we revert our patches. Do the Step 2 at hot-plug time, not at booting time where we did some useless work. It also can make the mapping of "cpuid <-> nodeid" fixed and avoid excessive use of the ACPI table. We have tested them in our box: Fujitsu PQ2000 with 2 nodes for hot-plug. To Xiaolong: Please help me to test it in the special machine. Dou Liyang (2): Revert"x86/acpi: Set persistent cpuid <-> nodeid mapping Revert"x86, acpi, cpu-hotplug: Enable MADT APIs to return disabled apicid" arch/x86/kernel/acpi/boot.c | 2 +- drivers/acpi/acpi_processor.c | 5 -- drivers/acpi/bus.c| 1 - drivers/acpi/processor_core.c | 133 +++--- include/linux/acpi.h | 3 - 5 files changed, 23 insertions(+), 121 deletions(-) -- 2.5.5
[PATCH 1/2] Revert"x86/acpi: Set persistent cpuid <-> nodeid mapping when booting"
Currently, We make the mapping of "cpuid <-> nodeid" fixed at the booting time. It keeps consistent with the WorkQueue and avoids some bugs which may be caused by the dynamic assignment. But, The ACPI table is unreliable and it is very risky that we use the entity which isn't related to a physical device at booting time. Now, we revert our patches. Do the last mapping of "cpuid <-> nodeid" at hot-plug time, not at booting time where we did some useless work. It also can make the mapping of "cpuid <-> nodeid" fixed and avoid excessive use of the ACPI table. The patch revert the commit dc6db24d24: "x86/acpi: Set persistent cpuid <-> nodeid mapping when booting". Signed-off-by: Dou Liyang--- arch/x86/kernel/acpi/boot.c | 2 +- drivers/acpi/acpi_processor.c | 5 --- drivers/acpi/bus.c| 1 - drivers/acpi/processor_core.c | 73 --- include/linux/acpi.h | 3 -- 5 files changed, 1 insertion(+), 83 deletions(-) diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index 64422f8..32846a2 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -709,7 +709,7 @@ static void __init acpi_set_irq_model_ioapic(void) #ifdef CONFIG_ACPI_HOTPLUG_CPU #include -int acpi_map_cpu2node(acpi_handle handle, int cpu, int physid) +static int acpi_map_cpu2node(acpi_handle handle, int cpu, int physid) { #ifdef CONFIG_ACPI_NUMA int nid; diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c index 3de3b6b..f43a586 100644 --- a/drivers/acpi/acpi_processor.c +++ b/drivers/acpi/acpi_processor.c @@ -182,11 +182,6 @@ int __weak arch_register_cpu(int cpu) void __weak arch_unregister_cpu(int cpu) {} -int __weak acpi_map_cpu2node(acpi_handle handle, int cpu, int physid) -{ - return -ENODEV; -} - static int acpi_processor_hotadd_init(struct acpi_processor *pr) { unsigned long long sta; diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 95855cb..d4455e4 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -1207,7 +1207,6 @@ static int __init acpi_init(void) acpi_wakeup_device_init(); acpi_debugger_init(); acpi_setup_sb_notify_handler(); - acpi_set_processor_mapping(); return 0; } diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index 611a558..a843862 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -278,79 +278,6 @@ int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id) } EXPORT_SYMBOL_GPL(acpi_get_cpuid); -#ifdef CONFIG_ACPI_HOTPLUG_CPU -static bool __init -map_processor(acpi_handle handle, phys_cpuid_t *phys_id, int *cpuid) -{ - int type, id; - u32 acpi_id; - acpi_status status; - acpi_object_type acpi_type; - unsigned long long tmp; - union acpi_object object = { 0 }; - struct acpi_buffer buffer = { sizeof(union acpi_object), }; - - status = acpi_get_type(handle, _type); - if (ACPI_FAILURE(status)) - return false; - - switch (acpi_type) { - case ACPI_TYPE_PROCESSOR: - status = acpi_evaluate_object(handle, NULL, NULL, ); - if (ACPI_FAILURE(status)) - return false; - acpi_id = object.processor.proc_id; - - /* validate the acpi_id */ - if(acpi_processor_validate_proc_id(acpi_id)) - return false; - break; - case ACPI_TYPE_DEVICE: - status = acpi_evaluate_integer(handle, "_UID", NULL, ); - if (ACPI_FAILURE(status)) - return false; - acpi_id = tmp; - break; - default: - return false; - } - - type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0; - - *phys_id = __acpi_get_phys_id(handle, type, acpi_id, false); - id = acpi_map_cpuid(*phys_id, acpi_id); - - if (id < 0) - return false; - *cpuid = id; - return true; -} - -static acpi_status __init -set_processor_node_mapping(acpi_handle handle, u32 lvl, void *context, - void **rv) -{ - phys_cpuid_t phys_id; - int cpu_id; - - if (!map_processor(handle, _id, _id)) - return AE_ERROR; - - acpi_map_cpu2node(handle, cpu_id, phys_id); - return AE_OK; -} - -void __init acpi_set_processor_mapping(void) -{ - /* Set persistent cpu <-> node mapping for all processors. */ - acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT, - ACPI_UINT32_MAX, set_processor_node_mapping, - NULL, NULL, NULL); -} -#else -void __init acpi_set_processor_mapping(void) {} -#endif /* CONFIG_ACPI_HOTPLUG_CPU */ - #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC static int get_ioapic_id(struct acpi_subtable_header *entry, u32 gsi_base,
[PATCH 2/2] Revert"x86, acpi, cpu-hotplug: Enable MADT APIs to return disabled apicid"
After we never do the last mapping of "cpuid <-> nodeid" at booting time. we also no need to enable MADT APIs to return disabled apicid. So, The patch work for reverting the commit 8ad893faf2: "x86, acpi, cpu-hotplug: Enable MADT APIs to return disabled apicid" Signed-off-by: Dou Liyang--- drivers/acpi/processor_core.c | 60 --- 1 file changed, 22 insertions(+), 38 deletions(-) diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index a843862..b933061 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -32,12 +32,12 @@ static struct acpi_table_madt *get_madt_table(void) } static int map_lapic_id(struct acpi_subtable_header *entry, -u32 acpi_id, phys_cpuid_t *apic_id, bool ignore_disabled) +u32 acpi_id, phys_cpuid_t *apic_id) { struct acpi_madt_local_apic *lapic = container_of(entry, struct acpi_madt_local_apic, header); - if (ignore_disabled && !(lapic->lapic_flags & ACPI_MADT_ENABLED)) + if (!(lapic->lapic_flags & ACPI_MADT_ENABLED)) return -ENODEV; if (lapic->processor_id != acpi_id) @@ -48,13 +48,12 @@ static int map_lapic_id(struct acpi_subtable_header *entry, } static int map_x2apic_id(struct acpi_subtable_header *entry, - int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id, - bool ignore_disabled) + int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id) { struct acpi_madt_local_x2apic *apic = container_of(entry, struct acpi_madt_local_x2apic, header); - if (ignore_disabled && !(apic->lapic_flags & ACPI_MADT_ENABLED)) + if (!(apic->lapic_flags & ACPI_MADT_ENABLED)) return -ENODEV; if (device_declaration && (apic->uid == acpi_id)) { @@ -66,13 +65,12 @@ static int map_x2apic_id(struct acpi_subtable_header *entry, } static int map_lsapic_id(struct acpi_subtable_header *entry, - int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id, - bool ignore_disabled) + int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id) { struct acpi_madt_local_sapic *lsapic = container_of(entry, struct acpi_madt_local_sapic, header); - if (ignore_disabled && !(lsapic->lapic_flags & ACPI_MADT_ENABLED)) + if (!(lsapic->lapic_flags & ACPI_MADT_ENABLED)) return -ENODEV; if (device_declaration) { @@ -89,13 +87,12 @@ static int map_lsapic_id(struct acpi_subtable_header *entry, * Retrieve the ARM CPU physical identifier (MPIDR) */ static int map_gicc_mpidr(struct acpi_subtable_header *entry, - int device_declaration, u32 acpi_id, phys_cpuid_t *mpidr, - bool ignore_disabled) + int device_declaration, u32 acpi_id, phys_cpuid_t *mpidr) { struct acpi_madt_generic_interrupt *gicc = container_of(entry, struct acpi_madt_generic_interrupt, header); - if (ignore_disabled && !(gicc->flags & ACPI_MADT_ENABLED)) + if (!(gicc->flags & ACPI_MADT_ENABLED)) return -ENODEV; /* device_declaration means Device object in DSDT, in the @@ -112,7 +109,7 @@ static int map_gicc_mpidr(struct acpi_subtable_header *entry, } static phys_cpuid_t map_madt_entry(struct acpi_table_madt *madt, - int type, u32 acpi_id, bool ignore_disabled) + int type, u32 acpi_id) { unsigned long madt_end, entry; phys_cpuid_t phys_id = PHYS_CPUID_INVALID; /* CPU hardware ID */ @@ -130,20 +127,16 @@ static phys_cpuid_t map_madt_entry(struct acpi_table_madt *madt, struct acpi_subtable_header *header = (struct acpi_subtable_header *)entry; if (header->type == ACPI_MADT_TYPE_LOCAL_APIC) { - if (!map_lapic_id(header, acpi_id, _id, - ignore_disabled)) + if (!map_lapic_id(header, acpi_id, _id)) break; } else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC) { - if (!map_x2apic_id(header, type, acpi_id, _id, - ignore_disabled)) + if (!map_x2apic_id(header, type, acpi_id, _id)) break; } else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC) { - if (!map_lsapic_id(header, type, acpi_id, _id, - ignore_disabled)) + if (!map_lsapic_id(header, type, acpi_id, _id)) break; } else if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) { - if (!map_gicc_mpidr(header, type,
[PATCH 1/2] Revert"x86/acpi: Set persistent cpuid <-> nodeid mapping when booting"
Currently, We make the mapping of "cpuid <-> nodeid" fixed at the booting time. It keeps consistent with the WorkQueue and avoids some bugs which may be caused by the dynamic assignment. But, The ACPI table is unreliable and it is very risky that we use the entity which isn't related to a physical device at booting time. Now, we revert our patches. Do the last mapping of "cpuid <-> nodeid" at hot-plug time, not at booting time where we did some useless work. It also can make the mapping of "cpuid <-> nodeid" fixed and avoid excessive use of the ACPI table. The patch revert the commit dc6db24d24: "x86/acpi: Set persistent cpuid <-> nodeid mapping when booting". Signed-off-by: Dou Liyang --- arch/x86/kernel/acpi/boot.c | 2 +- drivers/acpi/acpi_processor.c | 5 --- drivers/acpi/bus.c| 1 - drivers/acpi/processor_core.c | 73 --- include/linux/acpi.h | 3 -- 5 files changed, 1 insertion(+), 83 deletions(-) diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index 64422f8..32846a2 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -709,7 +709,7 @@ static void __init acpi_set_irq_model_ioapic(void) #ifdef CONFIG_ACPI_HOTPLUG_CPU #include -int acpi_map_cpu2node(acpi_handle handle, int cpu, int physid) +static int acpi_map_cpu2node(acpi_handle handle, int cpu, int physid) { #ifdef CONFIG_ACPI_NUMA int nid; diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c index 3de3b6b..f43a586 100644 --- a/drivers/acpi/acpi_processor.c +++ b/drivers/acpi/acpi_processor.c @@ -182,11 +182,6 @@ int __weak arch_register_cpu(int cpu) void __weak arch_unregister_cpu(int cpu) {} -int __weak acpi_map_cpu2node(acpi_handle handle, int cpu, int physid) -{ - return -ENODEV; -} - static int acpi_processor_hotadd_init(struct acpi_processor *pr) { unsigned long long sta; diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 95855cb..d4455e4 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -1207,7 +1207,6 @@ static int __init acpi_init(void) acpi_wakeup_device_init(); acpi_debugger_init(); acpi_setup_sb_notify_handler(); - acpi_set_processor_mapping(); return 0; } diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index 611a558..a843862 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -278,79 +278,6 @@ int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id) } EXPORT_SYMBOL_GPL(acpi_get_cpuid); -#ifdef CONFIG_ACPI_HOTPLUG_CPU -static bool __init -map_processor(acpi_handle handle, phys_cpuid_t *phys_id, int *cpuid) -{ - int type, id; - u32 acpi_id; - acpi_status status; - acpi_object_type acpi_type; - unsigned long long tmp; - union acpi_object object = { 0 }; - struct acpi_buffer buffer = { sizeof(union acpi_object), }; - - status = acpi_get_type(handle, _type); - if (ACPI_FAILURE(status)) - return false; - - switch (acpi_type) { - case ACPI_TYPE_PROCESSOR: - status = acpi_evaluate_object(handle, NULL, NULL, ); - if (ACPI_FAILURE(status)) - return false; - acpi_id = object.processor.proc_id; - - /* validate the acpi_id */ - if(acpi_processor_validate_proc_id(acpi_id)) - return false; - break; - case ACPI_TYPE_DEVICE: - status = acpi_evaluate_integer(handle, "_UID", NULL, ); - if (ACPI_FAILURE(status)) - return false; - acpi_id = tmp; - break; - default: - return false; - } - - type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0; - - *phys_id = __acpi_get_phys_id(handle, type, acpi_id, false); - id = acpi_map_cpuid(*phys_id, acpi_id); - - if (id < 0) - return false; - *cpuid = id; - return true; -} - -static acpi_status __init -set_processor_node_mapping(acpi_handle handle, u32 lvl, void *context, - void **rv) -{ - phys_cpuid_t phys_id; - int cpu_id; - - if (!map_processor(handle, _id, _id)) - return AE_ERROR; - - acpi_map_cpu2node(handle, cpu_id, phys_id); - return AE_OK; -} - -void __init acpi_set_processor_mapping(void) -{ - /* Set persistent cpu <-> node mapping for all processors. */ - acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT, - ACPI_UINT32_MAX, set_processor_node_mapping, - NULL, NULL, NULL); -} -#else -void __init acpi_set_processor_mapping(void) {} -#endif /* CONFIG_ACPI_HOTPLUG_CPU */ - #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC static int get_ioapic_id(struct acpi_subtable_header *entry, u32 gsi_base, u64
[PATCH 2/2] Revert"x86, acpi, cpu-hotplug: Enable MADT APIs to return disabled apicid"
After we never do the last mapping of "cpuid <-> nodeid" at booting time. we also no need to enable MADT APIs to return disabled apicid. So, The patch work for reverting the commit 8ad893faf2: "x86, acpi, cpu-hotplug: Enable MADT APIs to return disabled apicid" Signed-off-by: Dou Liyang --- drivers/acpi/processor_core.c | 60 --- 1 file changed, 22 insertions(+), 38 deletions(-) diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index a843862..b933061 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -32,12 +32,12 @@ static struct acpi_table_madt *get_madt_table(void) } static int map_lapic_id(struct acpi_subtable_header *entry, -u32 acpi_id, phys_cpuid_t *apic_id, bool ignore_disabled) +u32 acpi_id, phys_cpuid_t *apic_id) { struct acpi_madt_local_apic *lapic = container_of(entry, struct acpi_madt_local_apic, header); - if (ignore_disabled && !(lapic->lapic_flags & ACPI_MADT_ENABLED)) + if (!(lapic->lapic_flags & ACPI_MADT_ENABLED)) return -ENODEV; if (lapic->processor_id != acpi_id) @@ -48,13 +48,12 @@ static int map_lapic_id(struct acpi_subtable_header *entry, } static int map_x2apic_id(struct acpi_subtable_header *entry, - int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id, - bool ignore_disabled) + int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id) { struct acpi_madt_local_x2apic *apic = container_of(entry, struct acpi_madt_local_x2apic, header); - if (ignore_disabled && !(apic->lapic_flags & ACPI_MADT_ENABLED)) + if (!(apic->lapic_flags & ACPI_MADT_ENABLED)) return -ENODEV; if (device_declaration && (apic->uid == acpi_id)) { @@ -66,13 +65,12 @@ static int map_x2apic_id(struct acpi_subtable_header *entry, } static int map_lsapic_id(struct acpi_subtable_header *entry, - int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id, - bool ignore_disabled) + int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id) { struct acpi_madt_local_sapic *lsapic = container_of(entry, struct acpi_madt_local_sapic, header); - if (ignore_disabled && !(lsapic->lapic_flags & ACPI_MADT_ENABLED)) + if (!(lsapic->lapic_flags & ACPI_MADT_ENABLED)) return -ENODEV; if (device_declaration) { @@ -89,13 +87,12 @@ static int map_lsapic_id(struct acpi_subtable_header *entry, * Retrieve the ARM CPU physical identifier (MPIDR) */ static int map_gicc_mpidr(struct acpi_subtable_header *entry, - int device_declaration, u32 acpi_id, phys_cpuid_t *mpidr, - bool ignore_disabled) + int device_declaration, u32 acpi_id, phys_cpuid_t *mpidr) { struct acpi_madt_generic_interrupt *gicc = container_of(entry, struct acpi_madt_generic_interrupt, header); - if (ignore_disabled && !(gicc->flags & ACPI_MADT_ENABLED)) + if (!(gicc->flags & ACPI_MADT_ENABLED)) return -ENODEV; /* device_declaration means Device object in DSDT, in the @@ -112,7 +109,7 @@ static int map_gicc_mpidr(struct acpi_subtable_header *entry, } static phys_cpuid_t map_madt_entry(struct acpi_table_madt *madt, - int type, u32 acpi_id, bool ignore_disabled) + int type, u32 acpi_id) { unsigned long madt_end, entry; phys_cpuid_t phys_id = PHYS_CPUID_INVALID; /* CPU hardware ID */ @@ -130,20 +127,16 @@ static phys_cpuid_t map_madt_entry(struct acpi_table_madt *madt, struct acpi_subtable_header *header = (struct acpi_subtable_header *)entry; if (header->type == ACPI_MADT_TYPE_LOCAL_APIC) { - if (!map_lapic_id(header, acpi_id, _id, - ignore_disabled)) + if (!map_lapic_id(header, acpi_id, _id)) break; } else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC) { - if (!map_x2apic_id(header, type, acpi_id, _id, - ignore_disabled)) + if (!map_x2apic_id(header, type, acpi_id, _id)) break; } else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC) { - if (!map_lsapic_id(header, type, acpi_id, _id, - ignore_disabled)) + if (!map_lsapic_id(header, type, acpi_id, _id)) break; } else if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) { - if (!map_gicc_mpidr(header, type, acpi_id, _id, -
Re: [PATCH] Make EN2 pin optional in the TRF7970A driver
Hello all, Am 13.02.2017 um 22:31 schrieb Rob Herring: On Mon, Feb 13, 2017 at 12:38 AM, Heiko Schocherwrote: Hello Rob, Am 10.02.2017 um 16:51 schrieb Rob Herring: On Tue, Feb 07, 2017 at 06:22:04AM +0100, Heiko Schocher wrote: From: Guan Ben Make the EN2 pin optional. This is useful for boards, which have this pin fix wired, for example to ground. Signed-off-by: Guan Ben Signed-off-by: Mark Jonas Signed-off-by: Heiko Schocher --- .../devicetree/bindings/net/nfc/trf7970a.txt | 4 ++-- drivers/nfc/trf7970a.c | 26 -- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt index 32b35a0..5889a3d 100644 --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt @@ -5,8 +5,8 @@ Required properties: - spi-max-frequency: Maximum SPI frequency (<= 200). - interrupt-parent: phandle of parent interrupt handler. - interrupts: A single interrupt specifier. -- ti,enable-gpios: Two GPIO entries used for 'EN' and 'EN2' pins on the - TRF7970A. +- ti,enable-gpios: One or two GPIO entries used for 'EN' and 'EN2' pins on the + TRF7970A. EN2 is optional. Could EN ever be optional/fixed? If so, perhaps deprecate this property and do 2 properties, one for each pin. The hardware I have has the EN2 pin fix connected to ground. Looking into http://www.ti.com/lit/ds/slos743k/slos743k.pdf page 19 table 6-3 and 6-4 the EN2 pin is a don;t core if EN = 1. If EN = 0 EN2 pin selects between Power Down and Sleep Mode ... I see no reason why this is not possible/allowed ... Hmm.. I do not like the idea of deprecating the "ti,enable-gpios" property into 2 seperate properties ... but if this would be a reason for not accepting this patch, I can do this ... How should I name the 2 new properties? I guess if this ever happens, then we just add "ti,enable2-gpios" and ti,enable-gpios continues to point to EN. We don't need to deprecate anything (or maybe just deprecate having both GPIOs on single property). In that case, Acked-by: Rob Herring gentle ping. Are there any more comments to this patch? Is it acceptable as it is? Thanks! bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Re: [PATCH] Make EN2 pin optional in the TRF7970A driver
Hello all, Am 13.02.2017 um 22:31 schrieb Rob Herring: On Mon, Feb 13, 2017 at 12:38 AM, Heiko Schocher wrote: Hello Rob, Am 10.02.2017 um 16:51 schrieb Rob Herring: On Tue, Feb 07, 2017 at 06:22:04AM +0100, Heiko Schocher wrote: From: Guan Ben Make the EN2 pin optional. This is useful for boards, which have this pin fix wired, for example to ground. Signed-off-by: Guan Ben Signed-off-by: Mark Jonas Signed-off-by: Heiko Schocher --- .../devicetree/bindings/net/nfc/trf7970a.txt | 4 ++-- drivers/nfc/trf7970a.c | 26 -- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt index 32b35a0..5889a3d 100644 --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt @@ -5,8 +5,8 @@ Required properties: - spi-max-frequency: Maximum SPI frequency (<= 200). - interrupt-parent: phandle of parent interrupt handler. - interrupts: A single interrupt specifier. -- ti,enable-gpios: Two GPIO entries used for 'EN' and 'EN2' pins on the - TRF7970A. +- ti,enable-gpios: One or two GPIO entries used for 'EN' and 'EN2' pins on the + TRF7970A. EN2 is optional. Could EN ever be optional/fixed? If so, perhaps deprecate this property and do 2 properties, one for each pin. The hardware I have has the EN2 pin fix connected to ground. Looking into http://www.ti.com/lit/ds/slos743k/slos743k.pdf page 19 table 6-3 and 6-4 the EN2 pin is a don;t core if EN = 1. If EN = 0 EN2 pin selects between Power Down and Sleep Mode ... I see no reason why this is not possible/allowed ... Hmm.. I do not like the idea of deprecating the "ti,enable-gpios" property into 2 seperate properties ... but if this would be a reason for not accepting this patch, I can do this ... How should I name the 2 new properties? I guess if this ever happens, then we just add "ti,enable2-gpios" and ti,enable-gpios continues to point to EN. We don't need to deprecate anything (or maybe just deprecate having both GPIOs on single property). In that case, Acked-by: Rob Herring gentle ping. Are there any more comments to this patch? Is it acceptable as it is? Thanks! bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Re: [next-20170217] WARN @/arch/powerpc/include/asm/xics.h:124 .icp_hv_eoi+0x40/0x140
>> While booting next-20170217 on a POWER6 box, I ran into following >> warning. This is a full system lpar. Previous next tree was good. >> I will try a bisect tomorrow. > > Do you have CONFIG_DEBUG_SHIRQ=y ? > Yes. CONFIG_DEBUG_SHIRQ is enabled. As suggested by you reverting following commit allows a clean boot. f91f694540f3 ("genirq: Reenable shared irq debugging in request_*_irq()”) >> ipr: IBM Power RAID SCSI Device Driver version: 2.6.3 (October 17, 2015) >> ipr 0200:00:01.0: Found IOA with IRQ: 305 >> [ cut here ] >> WARNING: CPU: 12 PID: 1 at ./arch/powerpc/include/asm/xics.h:124 >> .icp_hv_eoi+0x40/0x140 >> Modules linked in: >> CPU: 12 PID: 1 Comm: swapper/14 Not tainted >> 4.10.0-rc8-next-20170217-autotest #1 >> task: c002b2a4a580 task.stack: c002b2a5c000 >> NIP: c00731b0 LR: c01389f8 CTR: c0073170 >> REGS: c002b2a5f050 TRAP: 0700 Not tainted >> (4.10.0-rc8-next-20170217-autotest) >> MSR: 80029032>> CR: 28004082 XER: 2004 >> CFAR: c01389e0 SOFTE: 0 >> GPR00: c01389f8 c002b2a5f2d0 c1025800 c002b203f498 >> GPR04: 0064 0131 >> GPR08: 0001 c000d3104cb8 0009b1f8 >> GPR12: 48004082 cedc2400 c000dad0 >> GPR16: 3c007efc c0a9e848 >> GPR20: d8008008 c002af4d47f0 c11efda8 c0a9ea10 >> GPR24: c0a9e848 c002af4d4fb8 >> GPR28: c002b203f498 c0ef8928 c002b203f400 >> NIP [c00731b0] .icp_hv_eoi+0x40/0x140 >> LR [c01389f8] .handle_fasteoi_irq+0x1e8/0x270 >> Call Trace: >> [c002b2a5f2d0] [c002b2a5f360] 0xc002b2a5f360 (unreliable) >> [c002b2a5f360] [c01389f8] .handle_fasteoi_irq+0x1e8/0x270 >> [c002b2a5f3e0] [c0136a08] .request_threaded_irq+0x298/0x370 >> [c002b2a5f490] [c05895c0] .ipr_probe_ioa+0x1110/0x1390 >> [c002b2a5f5c0] [c058d030] .ipr_probe+0x30/0x3e0 >> [c002b2a5f670] [c0466860] .local_pci_probe+0x60/0x130 >> [c002b2a5f710] [c0467658] .pci_device_probe+0x148/0x1e0 >> [c002b2a5f7c0] [c0527524] .driver_probe_device+0x2d4/0x5b0 >> [c002b2a5f860] [c052796c] .__driver_attach+0x16c/0x190 >> [c002b2a5f8f0] [c05242c4] .bus_for_each_dev+0x84/0xf0 >> [c002b2a5f990] [c0526af4] .driver_attach+0x24/0x40 >> [c002b2a5fa00] [c0526318] .bus_add_driver+0x2a8/0x370 >> [c002b2a5faa0] [c0528a5c] .driver_register+0x8c/0x170 >> [c002b2a5fb20] [c0465a54] .__pci_register_driver+0x44/0x60 >> [c002b2a5fb90] [c0b8efc8] .ipr_init+0x58/0x70 >> [c002b2a5fc10] [c000d20c] .do_one_initcall+0x5c/0x1c0 >> [c002b2a5fce0] [c0b44738] .kernel_init_freeable+0x280/0x360 >> [c002b2a5fdb0] [c000daec] .kernel_init+0x1c/0x130 >> [c002b2a5fe30] [c000baa0] .ret_from_kernel_thread+0x58/0xb8 >> Instruction dump: >> f8010010 f821ff71 80e3000c 7c0004ac e94d0030 3d02ffbc 3928f4b8 7d295214 >> 81090004 3948 7d484378 79080fe2 <0b08> 2fa8 40de0050 91490004 >> ---[ end trace 5e18ae409f46392c ]--- >> ipr 0200:00:01.0: Initializing IOA. >> >> Thanks >> -Sachin >
Re: [next-20170217] WARN @/arch/powerpc/include/asm/xics.h:124 .icp_hv_eoi+0x40/0x140
>> While booting next-20170217 on a POWER6 box, I ran into following >> warning. This is a full system lpar. Previous next tree was good. >> I will try a bisect tomorrow. > > Do you have CONFIG_DEBUG_SHIRQ=y ? > Yes. CONFIG_DEBUG_SHIRQ is enabled. As suggested by you reverting following commit allows a clean boot. f91f694540f3 ("genirq: Reenable shared irq debugging in request_*_irq()”) >> ipr: IBM Power RAID SCSI Device Driver version: 2.6.3 (October 17, 2015) >> ipr 0200:00:01.0: Found IOA with IRQ: 305 >> [ cut here ] >> WARNING: CPU: 12 PID: 1 at ./arch/powerpc/include/asm/xics.h:124 >> .icp_hv_eoi+0x40/0x140 >> Modules linked in: >> CPU: 12 PID: 1 Comm: swapper/14 Not tainted >> 4.10.0-rc8-next-20170217-autotest #1 >> task: c002b2a4a580 task.stack: c002b2a5c000 >> NIP: c00731b0 LR: c01389f8 CTR: c0073170 >> REGS: c002b2a5f050 TRAP: 0700 Not tainted >> (4.10.0-rc8-next-20170217-autotest) >> MSR: 80029032 >> CR: 28004082 XER: 2004 >> CFAR: c01389e0 SOFTE: 0 >> GPR00: c01389f8 c002b2a5f2d0 c1025800 c002b203f498 >> GPR04: 0064 0131 >> GPR08: 0001 c000d3104cb8 0009b1f8 >> GPR12: 48004082 cedc2400 c000dad0 >> GPR16: 3c007efc c0a9e848 >> GPR20: d8008008 c002af4d47f0 c11efda8 c0a9ea10 >> GPR24: c0a9e848 c002af4d4fb8 >> GPR28: c002b203f498 c0ef8928 c002b203f400 >> NIP [c00731b0] .icp_hv_eoi+0x40/0x140 >> LR [c01389f8] .handle_fasteoi_irq+0x1e8/0x270 >> Call Trace: >> [c002b2a5f2d0] [c002b2a5f360] 0xc002b2a5f360 (unreliable) >> [c002b2a5f360] [c01389f8] .handle_fasteoi_irq+0x1e8/0x270 >> [c002b2a5f3e0] [c0136a08] .request_threaded_irq+0x298/0x370 >> [c002b2a5f490] [c05895c0] .ipr_probe_ioa+0x1110/0x1390 >> [c002b2a5f5c0] [c058d030] .ipr_probe+0x30/0x3e0 >> [c002b2a5f670] [c0466860] .local_pci_probe+0x60/0x130 >> [c002b2a5f710] [c0467658] .pci_device_probe+0x148/0x1e0 >> [c002b2a5f7c0] [c0527524] .driver_probe_device+0x2d4/0x5b0 >> [c002b2a5f860] [c052796c] .__driver_attach+0x16c/0x190 >> [c002b2a5f8f0] [c05242c4] .bus_for_each_dev+0x84/0xf0 >> [c002b2a5f990] [c0526af4] .driver_attach+0x24/0x40 >> [c002b2a5fa00] [c0526318] .bus_add_driver+0x2a8/0x370 >> [c002b2a5faa0] [c0528a5c] .driver_register+0x8c/0x170 >> [c002b2a5fb20] [c0465a54] .__pci_register_driver+0x44/0x60 >> [c002b2a5fb90] [c0b8efc8] .ipr_init+0x58/0x70 >> [c002b2a5fc10] [c000d20c] .do_one_initcall+0x5c/0x1c0 >> [c002b2a5fce0] [c0b44738] .kernel_init_freeable+0x280/0x360 >> [c002b2a5fdb0] [c000daec] .kernel_init+0x1c/0x130 >> [c002b2a5fe30] [c000baa0] .ret_from_kernel_thread+0x58/0xb8 >> Instruction dump: >> f8010010 f821ff71 80e3000c 7c0004ac e94d0030 3d02ffbc 3928f4b8 7d295214 >> 81090004 3948 7d484378 79080fe2 <0b08> 2fa8 40de0050 91490004 >> ---[ end trace 5e18ae409f46392c ]--- >> ipr 0200:00:01.0: Initializing IOA. >> >> Thanks >> -Sachin >
Re: [PATCH repost] ptr_ring: fix race conditions when resizing
On Sun, Feb 19, 2017 at 12:52:48AM -0500, David Miller wrote: > From: "Michael S. Tsirkin"> Date: Sun, 19 Feb 2017 07:17:17 +0200 > > > Dave, could you merge this before 4.10? If not - I can try. > > I just sent my last pull request to Linus, please merge it to > him directly. > > Thanks. I missed the opportunity. Could you pls queue the fix for next release and copy stable? -- MST
Re: [PATCH repost] ptr_ring: fix race conditions when resizing
On Sun, Feb 19, 2017 at 12:52:48AM -0500, David Miller wrote: > From: "Michael S. Tsirkin" > Date: Sun, 19 Feb 2017 07:17:17 +0200 > > > Dave, could you merge this before 4.10? If not - I can try. > > I just sent my last pull request to Linus, please merge it to > him directly. > > Thanks. I missed the opportunity. Could you pls queue the fix for next release and copy stable? -- MST
Re: [PATCH V2 4/6] PM / domain: Register for PM QOS performance notifier
On 17-02-17, 15:54, Kevin Hilman wrote: > Viresh Kumarwrites: > > > Some platforms have the capability to configure the performance state of > > their Power Domains. The performance levels are represented by positive > > integer values, a lower value represents lower performance state. > > > > This patch registers the power domain framework for PM QOS performance > > notifier in order to manage performance state of power domains. > > It seems to me it doesm't just register, but actually keeps track of the > performance_state by always tracking the max. Yes. Will update the commit log to make sure it is clear. > > This also allows the power domain drivers to implement a > > ->set_performance_state() callback, which will be called by the power > > domain core from the notifier routine. > > > > Signed-off-by: Viresh Kumar > > --- > > drivers/base/power/domain.c | 98 > > - > > include/linux/pm_domain.h | 5 +++ > > 2 files changed, 101 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > > index a73d79670a64..1158a07f92de 100644 > > --- a/drivers/base/power/domain.c > > +++ b/drivers/base/power/domain.c > > @@ -367,6 +367,88 @@ static int genpd_dev_pm_qos_notifier(struct > > notifier_block *nb, > > return NOTIFY_DONE; > > } > > > > +static void update_domain_performance_state(struct generic_pm_domain > > *genpd, > > + int depth) > > +{ > > + struct generic_pm_domain_data *pd_data; > > + struct generic_pm_domain *subdomain; > > + struct pm_domain_data *pdd; > > + unsigned int state = 0; > > + struct gpd_link *link; > > + > > + /* Traverse all devices within the domain */ > > + list_for_each_entry(pdd, >dev_list, list_node) { > > + pd_data = to_gpd_data(pdd); > > + > > + if (pd_data->performance_state > state) > > + state = pd_data->performance_state; > > + } > > This seems to only update the state if it's bigger. Maybe I'm missing > something here, but it seems like won't be able to lower the > performance_state after it's been raised? 'state' is initialized to 0 to begin with and then the above loop finds the highest state requested so far. The below code (after the below loop) then changes the state if it isn't equal to the previous one. That is, it would update the state if the new target state is lower than the current one. Or am I missing a bug in there ? > > + /* Traverse all subdomains within the domain */ > > + list_for_each_entry(link, >master_links, master_node) { > > + subdomain = link->slave; > > + > > + if (subdomain->performance_state > state) > > + state = subdomain->performance_state; > > + } > > So subdomains are always assumed to influence the performance_state of > the parent domains? Is that always the case? It is true for the hardware we have to work on right now, but I would assume that being the most common case. > I suspect this should be > probably be a reasonable default assumption, but maybe controlled with a > flag. Yeah, maybe we can have a flag to ignore a subdomain while doing the calculations, but I would rather defer that to the first platform that needs it and leave it as is for now. I am afraid that code may never get used and maybe we should wait for a real case first. > > + if (genpd->performance_state == state) > > + return; > > + > > + genpd->performance_state = state; > > + > > + if (genpd->set_performance_state) { > > + genpd->set_performance_state(genpd, state); > > + return; > > + } > > So is zero not a valid performance_state? Zero is a *valid* performance state. Are you getting confused with the above 'if' block which checks for a valid set_performance_state() callback and not the performance level ? > > + /* Propagate only if this domain has a single parent */ > > Why? This limitation should be explained in the cover letter and > changelog. I would also expect some sort of WARN here since this could > otherwise be a rather silent failures. I think this limitation can just be removed, but I am confused a bit. I thought that support for multiple parent domains isn't yet there, isn't it? And that few people are trying to add it in kernel and the stuff is still under review. Like this thread: https://lkml.org/lkml/2016/11/22/792 -- viresh
Re: [PATCH V2 4/6] PM / domain: Register for PM QOS performance notifier
On 17-02-17, 15:54, Kevin Hilman wrote: > Viresh Kumar writes: > > > Some platforms have the capability to configure the performance state of > > their Power Domains. The performance levels are represented by positive > > integer values, a lower value represents lower performance state. > > > > This patch registers the power domain framework for PM QOS performance > > notifier in order to manage performance state of power domains. > > It seems to me it doesm't just register, but actually keeps track of the > performance_state by always tracking the max. Yes. Will update the commit log to make sure it is clear. > > This also allows the power domain drivers to implement a > > ->set_performance_state() callback, which will be called by the power > > domain core from the notifier routine. > > > > Signed-off-by: Viresh Kumar > > --- > > drivers/base/power/domain.c | 98 > > - > > include/linux/pm_domain.h | 5 +++ > > 2 files changed, 101 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > > index a73d79670a64..1158a07f92de 100644 > > --- a/drivers/base/power/domain.c > > +++ b/drivers/base/power/domain.c > > @@ -367,6 +367,88 @@ static int genpd_dev_pm_qos_notifier(struct > > notifier_block *nb, > > return NOTIFY_DONE; > > } > > > > +static void update_domain_performance_state(struct generic_pm_domain > > *genpd, > > + int depth) > > +{ > > + struct generic_pm_domain_data *pd_data; > > + struct generic_pm_domain *subdomain; > > + struct pm_domain_data *pdd; > > + unsigned int state = 0; > > + struct gpd_link *link; > > + > > + /* Traverse all devices within the domain */ > > + list_for_each_entry(pdd, >dev_list, list_node) { > > + pd_data = to_gpd_data(pdd); > > + > > + if (pd_data->performance_state > state) > > + state = pd_data->performance_state; > > + } > > This seems to only update the state if it's bigger. Maybe I'm missing > something here, but it seems like won't be able to lower the > performance_state after it's been raised? 'state' is initialized to 0 to begin with and then the above loop finds the highest state requested so far. The below code (after the below loop) then changes the state if it isn't equal to the previous one. That is, it would update the state if the new target state is lower than the current one. Or am I missing a bug in there ? > > + /* Traverse all subdomains within the domain */ > > + list_for_each_entry(link, >master_links, master_node) { > > + subdomain = link->slave; > > + > > + if (subdomain->performance_state > state) > > + state = subdomain->performance_state; > > + } > > So subdomains are always assumed to influence the performance_state of > the parent domains? Is that always the case? It is true for the hardware we have to work on right now, but I would assume that being the most common case. > I suspect this should be > probably be a reasonable default assumption, but maybe controlled with a > flag. Yeah, maybe we can have a flag to ignore a subdomain while doing the calculations, but I would rather defer that to the first platform that needs it and leave it as is for now. I am afraid that code may never get used and maybe we should wait for a real case first. > > + if (genpd->performance_state == state) > > + return; > > + > > + genpd->performance_state = state; > > + > > + if (genpd->set_performance_state) { > > + genpd->set_performance_state(genpd, state); > > + return; > > + } > > So is zero not a valid performance_state? Zero is a *valid* performance state. Are you getting confused with the above 'if' block which checks for a valid set_performance_state() callback and not the performance level ? > > + /* Propagate only if this domain has a single parent */ > > Why? This limitation should be explained in the cover letter and > changelog. I would also expect some sort of WARN here since this could > otherwise be a rather silent failures. I think this limitation can just be removed, but I am confused a bit. I thought that support for multiple parent domains isn't yet there, isn't it? And that few people are trying to add it in kernel and the stuff is still under review. Like this thread: https://lkml.org/lkml/2016/11/22/792 -- viresh
linux-next: Tree for Feb 20
Hi all, Changes since 20170217: The pci tree gained a conflict (probably) against Linus' tree. The kspp tree gained a conflict against the net-next tree. The tip tree gained a conflict against the net-next tree. The target-bva tree gained conflicts against the target-updates tree. Non-merge commits (relative to Linus' tree): 9790 10485 files changed, 445345 insertions(+), 205077 deletions(-) I have created today's linux-next tree at git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (patches at http://www.kernel.org/pub/linux/kernel/next/ ). If you are tracking the linux-next tree using git, you should not use "git pull" to do so as that will try to merge the new linux-next release with the old one. You should use "git fetch" and checkout or reset to the new master. You can see which trees have been included by looking in the Next/Trees file in the source. There are also quilt-import.log and merge.log files in the Next directory. Between each merge, the tree was built with a ppc64_defconfig for powerpc and an allmodconfig (with CONFIG_BUILD_DOCSRC=n) for x86_64, a multi_v7_defconfig for arm and a native build of tools/perf. After the final fixups (if any), I do an x86_64 modules_install followed by builds for x86_64 allnoconfig, powerpc allnoconfig (32 and 64 bit), ppc44x_defconfig, allyesconfig and pseries_le_defconfig and i386, sparc and sparc64 defconfig. Below is a summary of the state of the merge. I am currently merging 254 trees (counting Linus' and 37 trees of bug fix patches pending for the current merge release). Stats about the size of the tree over time can be seen at http://neuling.org/linux-next-size.html . Status of my local build tests will be at http://kisskb.ellerman.id.au/linux-next . If maintainers want to give advice about cross compilers/configs that work, we are always open to add more builds. Thanks to Randy Dunlap for doing many randconfig builds. And to Paul Gortmaker for triage and bug fixes. -- Cheers, Stephen Rothwell $ git checkout master $ git reset --hard stable Merging origin/master (137d01df511b Fix missing sanity check in /dev/sg) Merging fixes/master (30066ce675d3 Merge branch 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6) Merging kbuild-current/rc-fixes (c7858bf16c0b asm-prototypes: Clear any CPP defines before declaring the functions) Merging arc-current/for-curr (8ba605b607b7 ARC: [plat-*] ARC_HAS_COH_CACHES no longer relevant) Merging arm-current/fixes (9e3440481845 ARM: 8658/1: uaccess: fix zeroing of 64-bit get_user()) Merging m68k-current/for-linus (ad595b77c4a8 m68k/atari: Use seq_puts() in atari_get_hardware_list()) Merging metag-fixes/fixes (35d04077ad96 metag: Only define atomic_dec_if_positive conditionally) Merging powerpc-fixes/fixes (3f91a89d424a powerpc/64: Disable use of radix under a hypervisor) Merging sparc/master (f9a42e0d58cf Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc) Merging fscrypt-current/for-stable (42d97eb0ade3 fscrypt: fix renaming and linking special files) Merging net/master (00ea1ceebe0d ipv6: release dst on error in ip6_dst_lookup_tail) Merging ipsec/master (e3dc847a5f85 vti6: Don't report path MTU below IPV6_MIN_MTU.) Merging netfilter/master (f95d7a46bc57 netfilter: ctnetlink: Fix regression in CTA_HELP processing) Merging ipvs/master (045169816b31 Merge branch 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6) Merging wireless-drivers/master (52f5631a4c05 rtlwifi: rtl8192ce: Fix loading of incorrect firmware) Merging mac80211/master (0c8ef291d976 Merge branch 'rhashtable-allocation-failure-during-insertion') Merging sound-current/for-linus (af677166cf63 ALSA: hda - adding a new NV HDMI/DP codec ID in the driver) Merging pci-current/for-linus (afe3e4d11bdf PCI/PME: Restore pcie_pme_driver.remove) Merging driver-core.current/driver-core-linus (49def1853334 Linux 4.10-rc4) Merging tty.current/tty-linus (49def1853334 Linux 4.10-rc4) Merging usb.current/usb-linus (d5adbfcd5f7b Linux 4.10-rc7) Merging usb-gadget-fixes/fixes (efe357f4633a usb: dwc2: host: fix Wmaybe-uninitialized warning) Merging usb-serial-fixes/usb-linus (d07830db1bdb USB: serial: pl2303: add ATEN device ID) Merging usb-chipidea-fixes/ci-for-usb-stable (c7fbb09b2ea1 usb: chipidea: move the lock initialization to core file) Merging phy/fixes (7ce7d89f4883 Linux 4.10-rc1) Merging staging.current/staging-linus (d5adbfcd5f7b Linux 4.10-rc7) Merging char-misc.current/char-misc-linus (d5adbfcd5f7b Linux 4.10-rc7) Merging input-current/for-linus (722c5ac708b4 Input: elan_i2c - add ELAN0605 to the ACPI table) Merging crypto-current/master (7c2cf1c4615c crypto: chcr - Fix key length for RFC4106) Merging ide/master (da095587e6be Revert "ide: Fix interface autodetection in legacy IDE driver (trial #2)") Merging vfio-fixes/for-linus (930a42ded3fe vfio/spapr_tce: Set window
linux-next: Tree for Feb 20
Hi all, Changes since 20170217: The pci tree gained a conflict (probably) against Linus' tree. The kspp tree gained a conflict against the net-next tree. The tip tree gained a conflict against the net-next tree. The target-bva tree gained conflicts against the target-updates tree. Non-merge commits (relative to Linus' tree): 9790 10485 files changed, 445345 insertions(+), 205077 deletions(-) I have created today's linux-next tree at git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (patches at http://www.kernel.org/pub/linux/kernel/next/ ). If you are tracking the linux-next tree using git, you should not use "git pull" to do so as that will try to merge the new linux-next release with the old one. You should use "git fetch" and checkout or reset to the new master. You can see which trees have been included by looking in the Next/Trees file in the source. There are also quilt-import.log and merge.log files in the Next directory. Between each merge, the tree was built with a ppc64_defconfig for powerpc and an allmodconfig (with CONFIG_BUILD_DOCSRC=n) for x86_64, a multi_v7_defconfig for arm and a native build of tools/perf. After the final fixups (if any), I do an x86_64 modules_install followed by builds for x86_64 allnoconfig, powerpc allnoconfig (32 and 64 bit), ppc44x_defconfig, allyesconfig and pseries_le_defconfig and i386, sparc and sparc64 defconfig. Below is a summary of the state of the merge. I am currently merging 254 trees (counting Linus' and 37 trees of bug fix patches pending for the current merge release). Stats about the size of the tree over time can be seen at http://neuling.org/linux-next-size.html . Status of my local build tests will be at http://kisskb.ellerman.id.au/linux-next . If maintainers want to give advice about cross compilers/configs that work, we are always open to add more builds. Thanks to Randy Dunlap for doing many randconfig builds. And to Paul Gortmaker for triage and bug fixes. -- Cheers, Stephen Rothwell $ git checkout master $ git reset --hard stable Merging origin/master (137d01df511b Fix missing sanity check in /dev/sg) Merging fixes/master (30066ce675d3 Merge branch 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6) Merging kbuild-current/rc-fixes (c7858bf16c0b asm-prototypes: Clear any CPP defines before declaring the functions) Merging arc-current/for-curr (8ba605b607b7 ARC: [plat-*] ARC_HAS_COH_CACHES no longer relevant) Merging arm-current/fixes (9e3440481845 ARM: 8658/1: uaccess: fix zeroing of 64-bit get_user()) Merging m68k-current/for-linus (ad595b77c4a8 m68k/atari: Use seq_puts() in atari_get_hardware_list()) Merging metag-fixes/fixes (35d04077ad96 metag: Only define atomic_dec_if_positive conditionally) Merging powerpc-fixes/fixes (3f91a89d424a powerpc/64: Disable use of radix under a hypervisor) Merging sparc/master (f9a42e0d58cf Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc) Merging fscrypt-current/for-stable (42d97eb0ade3 fscrypt: fix renaming and linking special files) Merging net/master (00ea1ceebe0d ipv6: release dst on error in ip6_dst_lookup_tail) Merging ipsec/master (e3dc847a5f85 vti6: Don't report path MTU below IPV6_MIN_MTU.) Merging netfilter/master (f95d7a46bc57 netfilter: ctnetlink: Fix regression in CTA_HELP processing) Merging ipvs/master (045169816b31 Merge branch 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6) Merging wireless-drivers/master (52f5631a4c05 rtlwifi: rtl8192ce: Fix loading of incorrect firmware) Merging mac80211/master (0c8ef291d976 Merge branch 'rhashtable-allocation-failure-during-insertion') Merging sound-current/for-linus (af677166cf63 ALSA: hda - adding a new NV HDMI/DP codec ID in the driver) Merging pci-current/for-linus (afe3e4d11bdf PCI/PME: Restore pcie_pme_driver.remove) Merging driver-core.current/driver-core-linus (49def1853334 Linux 4.10-rc4) Merging tty.current/tty-linus (49def1853334 Linux 4.10-rc4) Merging usb.current/usb-linus (d5adbfcd5f7b Linux 4.10-rc7) Merging usb-gadget-fixes/fixes (efe357f4633a usb: dwc2: host: fix Wmaybe-uninitialized warning) Merging usb-serial-fixes/usb-linus (d07830db1bdb USB: serial: pl2303: add ATEN device ID) Merging usb-chipidea-fixes/ci-for-usb-stable (c7fbb09b2ea1 usb: chipidea: move the lock initialization to core file) Merging phy/fixes (7ce7d89f4883 Linux 4.10-rc1) Merging staging.current/staging-linus (d5adbfcd5f7b Linux 4.10-rc7) Merging char-misc.current/char-misc-linus (d5adbfcd5f7b Linux 4.10-rc7) Merging input-current/for-linus (722c5ac708b4 Input: elan_i2c - add ELAN0605 to the ACPI table) Merging crypto-current/master (7c2cf1c4615c crypto: chcr - Fix key length for RFC4106) Merging ide/master (da095587e6be Revert "ide: Fix interface autodetection in legacy IDE driver (trial #2)") Merging vfio-fixes/for-linus (930a42ded3fe vfio/spapr_tce: Set window
Re: [PATCH v3] locking/pvqspinlock: Relax cmpxchg's to improve performance on some archs
(Really add Will this time ...) On Mon, Feb 20, 2017 at 12:53:58PM +0800, Boqun Feng wrote: > On Mon, Feb 20, 2017 at 05:20:52AM +0100, Andrea Parri wrote: > > On Fri, Feb 17, 2017 at 03:43:40PM -0500, Waiman Long wrote: > > > All the locking related cmpxchg's in the following functions are > > > replaced with the _acquire variants: > > > - pv_queued_spin_steal_lock() > > > - trylock_clear_pending() > > > > > > This change should help performance on architectures that use LL/SC. > > > > > > On a 2-core 16-thread Power8 system with pvqspinlock explicitly > > > enabled, the performance of a locking microbenchmark with and without > > > this patch on a 4.10-rc8 kernel with Xinhui's PPC qspinlock patch > > > were as follows: > > > > > > # of thread w/o patchwith patch % Change > > > --- --- > > >4 4053.3 Mop/s 4223.7 Mop/s +4.2% > > >8 3310.4 Mop/s 3406.0 Mop/s +2.9% > > > 12 2576.4 Mop/s 2674.6 Mop/s +3.8% > > > > > > Signed-off-by: Waiman Long> > > --- > > > > > > v2->v3: > > > - Reduce scope by relaxing cmpxchg's in fast path only. > > > > > > v1->v2: > > > - Add comments in changelog and code for the rationale of the change. > > > > > > kernel/locking/qspinlock_paravirt.h | 15 +-- > > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > > > diff --git a/kernel/locking/qspinlock_paravirt.h > > > b/kernel/locking/qspinlock_paravirt.h > > > index e6b2f7a..a59dc05 100644 > > > --- a/kernel/locking/qspinlock_paravirt.h > > > +++ b/kernel/locking/qspinlock_paravirt.h > > > @@ -72,7 +72,7 @@ static inline bool pv_queued_spin_steal_lock(struct > > > qspinlock *lock) > > > struct __qspinlock *l = (void *)lock; > > > > > > if (!(atomic_read(>val) & _Q_LOCKED_PENDING_MASK) && > > > - (cmpxchg(>locked, 0, _Q_LOCKED_VAL) == 0)) { > > > + (cmpxchg_acquire(>locked, 0, _Q_LOCKED_VAL) == 0)) { > > > qstat_inc(qstat_pv_lock_stealing, true); > > > return true; > > > } > > > @@ -101,16 +101,16 @@ static __always_inline void clear_pending(struct > > > qspinlock *lock) > > > > > > /* > > > * The pending bit check in pv_queued_spin_steal_lock() isn't a memory > > > - * barrier. Therefore, an atomic cmpxchg() is used to acquire the lock > > > - * just to be sure that it will get it. > > > + * barrier. Therefore, an atomic cmpxchg_acquire() is used to acquire the > > > + * lock just to be sure that it will get it. > > > */ > > > static __always_inline int trylock_clear_pending(struct qspinlock *lock) > > > { > > > struct __qspinlock *l = (void *)lock; > > > > > > return !READ_ONCE(l->locked) && > > > -(cmpxchg(>locked_pending, _Q_PENDING_VAL, _Q_LOCKED_VAL) > > > - == _Q_PENDING_VAL); > > > +(cmpxchg_acquire(>locked_pending, _Q_PENDING_VAL, > > > + _Q_LOCKED_VAL) == _Q_PENDING_VAL); > > > } > > > #else /* _Q_PENDING_BITS == 8 */ > > > static __always_inline void set_pending(struct qspinlock *lock) > > > @@ -138,7 +138,7 @@ static __always_inline int > > > trylock_clear_pending(struct qspinlock *lock) > > >*/ > > > old = val; > > > new = (val & ~_Q_PENDING_MASK) | _Q_LOCKED_VAL; > > > - val = atomic_cmpxchg(>val, old, new); > > > + val = atomic_cmpxchg_acquire(>val, old, new); > > > > > > if (val == old) > > > return 1; > > > @@ -361,6 +361,9 @@ static void pv_kick_node(struct qspinlock *lock, > > > struct mcs_spinlock *node) > > >* observe its next->locked value and advance itself. > > >* > > >* Matches with smp_store_mb() and cmpxchg() in pv_wait_node() > > > + * > > > + * We can't used relaxed form of cmpxchg here as the loading of > > > + * pn->state can happen before setting next->locked in some archs. > > >*/ > > > if (cmpxchg(>state, vcpu_halted, vcpu_hashed) != vcpu_halted) > > > > Hi Waiman. > > > > cmpxchg() does not guarantee the (here implied) smp_mb(), in general; c.f., > > e.g., arch/arm64/include/asm/atomic_ll_sc.h, where cmpxchg() get "compiled" > > to something like: > > > > _loop: ldxr; eor; cbnz _exit; stlxr; cbnz _loop; dmb ish; _exit: > > > > Yes, sorry for be late for this one. > > So Waiman, the fact is that in this case, we want the following code > sequence: > > CPU 0 CPU 1 > = > {pn->state = vcpu_running, node->locked = 0} > > smp_store_smb(>state, vcpu_halted): > WRITE_ONCE(pn->state, vcpu_halted); > smp_mb(); > r1 = READ_ONCE(node->locked); > > arch_mcs_spin_unlock_contented(); > WRITE_ONCE(node->locked, 1) > > cmpxchg(>state,
Re: [PATCH v3] locking/pvqspinlock: Relax cmpxchg's to improve performance on some archs
(Really add Will this time ...) On Mon, Feb 20, 2017 at 12:53:58PM +0800, Boqun Feng wrote: > On Mon, Feb 20, 2017 at 05:20:52AM +0100, Andrea Parri wrote: > > On Fri, Feb 17, 2017 at 03:43:40PM -0500, Waiman Long wrote: > > > All the locking related cmpxchg's in the following functions are > > > replaced with the _acquire variants: > > > - pv_queued_spin_steal_lock() > > > - trylock_clear_pending() > > > > > > This change should help performance on architectures that use LL/SC. > > > > > > On a 2-core 16-thread Power8 system with pvqspinlock explicitly > > > enabled, the performance of a locking microbenchmark with and without > > > this patch on a 4.10-rc8 kernel with Xinhui's PPC qspinlock patch > > > were as follows: > > > > > > # of thread w/o patchwith patch % Change > > > --- --- > > >4 4053.3 Mop/s 4223.7 Mop/s +4.2% > > >8 3310.4 Mop/s 3406.0 Mop/s +2.9% > > > 12 2576.4 Mop/s 2674.6 Mop/s +3.8% > > > > > > Signed-off-by: Waiman Long > > > --- > > > > > > v2->v3: > > > - Reduce scope by relaxing cmpxchg's in fast path only. > > > > > > v1->v2: > > > - Add comments in changelog and code for the rationale of the change. > > > > > > kernel/locking/qspinlock_paravirt.h | 15 +-- > > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > > > diff --git a/kernel/locking/qspinlock_paravirt.h > > > b/kernel/locking/qspinlock_paravirt.h > > > index e6b2f7a..a59dc05 100644 > > > --- a/kernel/locking/qspinlock_paravirt.h > > > +++ b/kernel/locking/qspinlock_paravirt.h > > > @@ -72,7 +72,7 @@ static inline bool pv_queued_spin_steal_lock(struct > > > qspinlock *lock) > > > struct __qspinlock *l = (void *)lock; > > > > > > if (!(atomic_read(>val) & _Q_LOCKED_PENDING_MASK) && > > > - (cmpxchg(>locked, 0, _Q_LOCKED_VAL) == 0)) { > > > + (cmpxchg_acquire(>locked, 0, _Q_LOCKED_VAL) == 0)) { > > > qstat_inc(qstat_pv_lock_stealing, true); > > > return true; > > > } > > > @@ -101,16 +101,16 @@ static __always_inline void clear_pending(struct > > > qspinlock *lock) > > > > > > /* > > > * The pending bit check in pv_queued_spin_steal_lock() isn't a memory > > > - * barrier. Therefore, an atomic cmpxchg() is used to acquire the lock > > > - * just to be sure that it will get it. > > > + * barrier. Therefore, an atomic cmpxchg_acquire() is used to acquire the > > > + * lock just to be sure that it will get it. > > > */ > > > static __always_inline int trylock_clear_pending(struct qspinlock *lock) > > > { > > > struct __qspinlock *l = (void *)lock; > > > > > > return !READ_ONCE(l->locked) && > > > -(cmpxchg(>locked_pending, _Q_PENDING_VAL, _Q_LOCKED_VAL) > > > - == _Q_PENDING_VAL); > > > +(cmpxchg_acquire(>locked_pending, _Q_PENDING_VAL, > > > + _Q_LOCKED_VAL) == _Q_PENDING_VAL); > > > } > > > #else /* _Q_PENDING_BITS == 8 */ > > > static __always_inline void set_pending(struct qspinlock *lock) > > > @@ -138,7 +138,7 @@ static __always_inline int > > > trylock_clear_pending(struct qspinlock *lock) > > >*/ > > > old = val; > > > new = (val & ~_Q_PENDING_MASK) | _Q_LOCKED_VAL; > > > - val = atomic_cmpxchg(>val, old, new); > > > + val = atomic_cmpxchg_acquire(>val, old, new); > > > > > > if (val == old) > > > return 1; > > > @@ -361,6 +361,9 @@ static void pv_kick_node(struct qspinlock *lock, > > > struct mcs_spinlock *node) > > >* observe its next->locked value and advance itself. > > >* > > >* Matches with smp_store_mb() and cmpxchg() in pv_wait_node() > > > + * > > > + * We can't used relaxed form of cmpxchg here as the loading of > > > + * pn->state can happen before setting next->locked in some archs. > > >*/ > > > if (cmpxchg(>state, vcpu_halted, vcpu_hashed) != vcpu_halted) > > > > Hi Waiman. > > > > cmpxchg() does not guarantee the (here implied) smp_mb(), in general; c.f., > > e.g., arch/arm64/include/asm/atomic_ll_sc.h, where cmpxchg() get "compiled" > > to something like: > > > > _loop: ldxr; eor; cbnz _exit; stlxr; cbnz _loop; dmb ish; _exit: > > > > Yes, sorry for be late for this one. > > So Waiman, the fact is that in this case, we want the following code > sequence: > > CPU 0 CPU 1 > = > {pn->state = vcpu_running, node->locked = 0} > > smp_store_smb(>state, vcpu_halted): > WRITE_ONCE(pn->state, vcpu_halted); > smp_mb(); > r1 = READ_ONCE(node->locked); > > arch_mcs_spin_unlock_contented(); > WRITE_ONCE(node->locked, 1) > > cmpxchg(>state, > vcpu_halted,
Re: [PATCH v3] locking/pvqspinlock: Relax cmpxchg's to improve performance on some archs
On Mon, Feb 20, 2017 at 05:20:52AM +0100, Andrea Parri wrote: > On Fri, Feb 17, 2017 at 03:43:40PM -0500, Waiman Long wrote: > > All the locking related cmpxchg's in the following functions are > > replaced with the _acquire variants: > > - pv_queued_spin_steal_lock() > > - trylock_clear_pending() > > > > This change should help performance on architectures that use LL/SC. > > > > On a 2-core 16-thread Power8 system with pvqspinlock explicitly > > enabled, the performance of a locking microbenchmark with and without > > this patch on a 4.10-rc8 kernel with Xinhui's PPC qspinlock patch > > were as follows: > > > > # of thread w/o patchwith patch % Change > > --- --- > >4 4053.3 Mop/s 4223.7 Mop/s +4.2% > >8 3310.4 Mop/s 3406.0 Mop/s +2.9% > > 12 2576.4 Mop/s 2674.6 Mop/s +3.8% > > > > Signed-off-by: Waiman Long> > --- > > > > v2->v3: > > - Reduce scope by relaxing cmpxchg's in fast path only. > > > > v1->v2: > > - Add comments in changelog and code for the rationale of the change. > > > > kernel/locking/qspinlock_paravirt.h | 15 +-- > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > diff --git a/kernel/locking/qspinlock_paravirt.h > > b/kernel/locking/qspinlock_paravirt.h > > index e6b2f7a..a59dc05 100644 > > --- a/kernel/locking/qspinlock_paravirt.h > > +++ b/kernel/locking/qspinlock_paravirt.h > > @@ -72,7 +72,7 @@ static inline bool pv_queued_spin_steal_lock(struct > > qspinlock *lock) > > struct __qspinlock *l = (void *)lock; > > > > if (!(atomic_read(>val) & _Q_LOCKED_PENDING_MASK) && > > - (cmpxchg(>locked, 0, _Q_LOCKED_VAL) == 0)) { > > + (cmpxchg_acquire(>locked, 0, _Q_LOCKED_VAL) == 0)) { > > qstat_inc(qstat_pv_lock_stealing, true); > > return true; > > } > > @@ -101,16 +101,16 @@ static __always_inline void clear_pending(struct > > qspinlock *lock) > > > > /* > > * The pending bit check in pv_queued_spin_steal_lock() isn't a memory > > - * barrier. Therefore, an atomic cmpxchg() is used to acquire the lock > > - * just to be sure that it will get it. > > + * barrier. Therefore, an atomic cmpxchg_acquire() is used to acquire the > > + * lock just to be sure that it will get it. > > */ > > static __always_inline int trylock_clear_pending(struct qspinlock *lock) > > { > > struct __qspinlock *l = (void *)lock; > > > > return !READ_ONCE(l->locked) && > > - (cmpxchg(>locked_pending, _Q_PENDING_VAL, _Q_LOCKED_VAL) > > - == _Q_PENDING_VAL); > > + (cmpxchg_acquire(>locked_pending, _Q_PENDING_VAL, > > + _Q_LOCKED_VAL) == _Q_PENDING_VAL); > > } > > #else /* _Q_PENDING_BITS == 8 */ > > static __always_inline void set_pending(struct qspinlock *lock) > > @@ -138,7 +138,7 @@ static __always_inline int trylock_clear_pending(struct > > qspinlock *lock) > > */ > > old = val; > > new = (val & ~_Q_PENDING_MASK) | _Q_LOCKED_VAL; > > - val = atomic_cmpxchg(>val, old, new); > > + val = atomic_cmpxchg_acquire(>val, old, new); > > > > if (val == old) > > return 1; > > @@ -361,6 +361,9 @@ static void pv_kick_node(struct qspinlock *lock, struct > > mcs_spinlock *node) > > * observe its next->locked value and advance itself. > > * > > * Matches with smp_store_mb() and cmpxchg() in pv_wait_node() > > +* > > +* We can't used relaxed form of cmpxchg here as the loading of > > +* pn->state can happen before setting next->locked in some archs. > > */ > > if (cmpxchg(>state, vcpu_halted, vcpu_hashed) != vcpu_halted) > > Hi Waiman. > > cmpxchg() does not guarantee the (here implied) smp_mb(), in general; c.f., > e.g., arch/arm64/include/asm/atomic_ll_sc.h, where cmpxchg() get "compiled" > to something like: > > _loop: ldxr; eor; cbnz _exit; stlxr; cbnz _loop; dmb ish; _exit: > Yes, sorry for be late for this one. So Waiman, the fact is that in this case, we want the following code sequence: CPU 0 CPU 1 = {pn->state = vcpu_running, node->locked = 0} smp_store_smb(>state, vcpu_halted): WRITE_ONCE(pn->state, vcpu_halted); smp_mb(); r1 = READ_ONCE(node->locked); arch_mcs_spin_unlock_contented(); WRITE_ONCE(node->locked, 1) cmpxchg(>state, vcpu_halted, vcpu_hashed); never ends up in: r1 == 0 && cmpxchg fail(i.e. the read part of cmpxchg reads the value vcpu_running). We can have such a guarantee if cmpxchg has a smp_mb() before its load part, which is true for PPC.
Re: [PATCH v3] locking/pvqspinlock: Relax cmpxchg's to improve performance on some archs
On Mon, Feb 20, 2017 at 05:20:52AM +0100, Andrea Parri wrote: > On Fri, Feb 17, 2017 at 03:43:40PM -0500, Waiman Long wrote: > > All the locking related cmpxchg's in the following functions are > > replaced with the _acquire variants: > > - pv_queued_spin_steal_lock() > > - trylock_clear_pending() > > > > This change should help performance on architectures that use LL/SC. > > > > On a 2-core 16-thread Power8 system with pvqspinlock explicitly > > enabled, the performance of a locking microbenchmark with and without > > this patch on a 4.10-rc8 kernel with Xinhui's PPC qspinlock patch > > were as follows: > > > > # of thread w/o patchwith patch % Change > > --- --- > >4 4053.3 Mop/s 4223.7 Mop/s +4.2% > >8 3310.4 Mop/s 3406.0 Mop/s +2.9% > > 12 2576.4 Mop/s 2674.6 Mop/s +3.8% > > > > Signed-off-by: Waiman Long > > --- > > > > v2->v3: > > - Reduce scope by relaxing cmpxchg's in fast path only. > > > > v1->v2: > > - Add comments in changelog and code for the rationale of the change. > > > > kernel/locking/qspinlock_paravirt.h | 15 +-- > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > diff --git a/kernel/locking/qspinlock_paravirt.h > > b/kernel/locking/qspinlock_paravirt.h > > index e6b2f7a..a59dc05 100644 > > --- a/kernel/locking/qspinlock_paravirt.h > > +++ b/kernel/locking/qspinlock_paravirt.h > > @@ -72,7 +72,7 @@ static inline bool pv_queued_spin_steal_lock(struct > > qspinlock *lock) > > struct __qspinlock *l = (void *)lock; > > > > if (!(atomic_read(>val) & _Q_LOCKED_PENDING_MASK) && > > - (cmpxchg(>locked, 0, _Q_LOCKED_VAL) == 0)) { > > + (cmpxchg_acquire(>locked, 0, _Q_LOCKED_VAL) == 0)) { > > qstat_inc(qstat_pv_lock_stealing, true); > > return true; > > } > > @@ -101,16 +101,16 @@ static __always_inline void clear_pending(struct > > qspinlock *lock) > > > > /* > > * The pending bit check in pv_queued_spin_steal_lock() isn't a memory > > - * barrier. Therefore, an atomic cmpxchg() is used to acquire the lock > > - * just to be sure that it will get it. > > + * barrier. Therefore, an atomic cmpxchg_acquire() is used to acquire the > > + * lock just to be sure that it will get it. > > */ > > static __always_inline int trylock_clear_pending(struct qspinlock *lock) > > { > > struct __qspinlock *l = (void *)lock; > > > > return !READ_ONCE(l->locked) && > > - (cmpxchg(>locked_pending, _Q_PENDING_VAL, _Q_LOCKED_VAL) > > - == _Q_PENDING_VAL); > > + (cmpxchg_acquire(>locked_pending, _Q_PENDING_VAL, > > + _Q_LOCKED_VAL) == _Q_PENDING_VAL); > > } > > #else /* _Q_PENDING_BITS == 8 */ > > static __always_inline void set_pending(struct qspinlock *lock) > > @@ -138,7 +138,7 @@ static __always_inline int trylock_clear_pending(struct > > qspinlock *lock) > > */ > > old = val; > > new = (val & ~_Q_PENDING_MASK) | _Q_LOCKED_VAL; > > - val = atomic_cmpxchg(>val, old, new); > > + val = atomic_cmpxchg_acquire(>val, old, new); > > > > if (val == old) > > return 1; > > @@ -361,6 +361,9 @@ static void pv_kick_node(struct qspinlock *lock, struct > > mcs_spinlock *node) > > * observe its next->locked value and advance itself. > > * > > * Matches with smp_store_mb() and cmpxchg() in pv_wait_node() > > +* > > +* We can't used relaxed form of cmpxchg here as the loading of > > +* pn->state can happen before setting next->locked in some archs. > > */ > > if (cmpxchg(>state, vcpu_halted, vcpu_hashed) != vcpu_halted) > > Hi Waiman. > > cmpxchg() does not guarantee the (here implied) smp_mb(), in general; c.f., > e.g., arch/arm64/include/asm/atomic_ll_sc.h, where cmpxchg() get "compiled" > to something like: > > _loop: ldxr; eor; cbnz _exit; stlxr; cbnz _loop; dmb ish; _exit: > Yes, sorry for be late for this one. So Waiman, the fact is that in this case, we want the following code sequence: CPU 0 CPU 1 = {pn->state = vcpu_running, node->locked = 0} smp_store_smb(>state, vcpu_halted): WRITE_ONCE(pn->state, vcpu_halted); smp_mb(); r1 = READ_ONCE(node->locked); arch_mcs_spin_unlock_contented(); WRITE_ONCE(node->locked, 1) cmpxchg(>state, vcpu_halted, vcpu_hashed); never ends up in: r1 == 0 && cmpxchg fail(i.e. the read part of cmpxchg reads the value vcpu_running). We can have such a guarantee if cmpxchg has a smp_mb() before its load part, which is true for PPC. But semantically,
Re: [PATCH v2 2/3] watchdog: sama5d4: Fix setting timeout when watchdog is disabled
On 02/19/2017 03:52 PM, Alexandre Belloni wrote: On 19/02/2017 at 08:57:35 -0800, Guenter Roeck wrote: That means if the watchdog is running, the timeout would not be updated. It should be updated no matter if it is running or not. No, it is enabling the watchdog, then changing WDV and WDD and finally disabling the watchdog if necessary. So, WDV and WDD are always changed. You are correct. Sorry for the noise. Seems odd that the watchdog must be _running_ to change the timeout. Usually, if there is a restriction, it is the opposite. I hope this doesn't cause race conditions, where the watchdog fires immediately after being enabled due to a low timeout. While it is difficult to reproduce, I can confirm it races and sometimes reset the SoC without any good reason. It doesn't matter whether it is disabled or not Outch :-(. I've raised the issue at Atmel last Thursday so I don't have any answer yet. Please keep us in the loop. Thanks, Guenter
Re: [PATCH v2 2/3] watchdog: sama5d4: Fix setting timeout when watchdog is disabled
On 02/19/2017 03:52 PM, Alexandre Belloni wrote: On 19/02/2017 at 08:57:35 -0800, Guenter Roeck wrote: That means if the watchdog is running, the timeout would not be updated. It should be updated no matter if it is running or not. No, it is enabling the watchdog, then changing WDV and WDD and finally disabling the watchdog if necessary. So, WDV and WDD are always changed. You are correct. Sorry for the noise. Seems odd that the watchdog must be _running_ to change the timeout. Usually, if there is a restriction, it is the opposite. I hope this doesn't cause race conditions, where the watchdog fires immediately after being enabled due to a low timeout. While it is difficult to reproduce, I can confirm it races and sometimes reset the SoC without any good reason. It doesn't matter whether it is disabled or not Outch :-(. I've raised the issue at Atmel last Thursday so I don't have any answer yet. Please keep us in the loop. Thanks, Guenter
Re: [PATCH] powerpc/xmon: Fix an unexpected xmon onoff state change
Pan Xinhuiwrites: > 在 2017/2/17 14:05, Michael Ellerman 写道: >> Pan Xinhui writes: >>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c >>> index 9c0e17c..f6e5c3d 100644 >>> --- a/arch/powerpc/xmon/xmon.c >>> +++ b/arch/powerpc/xmon/xmon.c >>> @@ -76,6 +76,7 @@ static int xmon_gate; >>> #endif /* CONFIG_SMP */ >>> >>> static unsigned long in_xmon __read_mostly = 0; >>> +static int xmon_off = !IS_ENABLED(CONFIG_XMON_DEFAULT); >> >> I think the logic would probably clearer if we invert this to become >> xmon_on. >> > yep, make sense. > >>> @@ -3266,16 +3269,16 @@ static int __init setup_xmon_sysrq(void) >>> __initcall(setup_xmon_sysrq); >>> #endif /* CONFIG_MAGIC_SYSRQ */ >>> >>> -static int __initdata xmon_early, xmon_off; >>> +static int __initdata xmon_early; >>> >>> static int __init early_parse_xmon(char *p) >>> { >>> if (!p || strncmp(p, "early", 5) == 0) { >>> /* just "xmon" is equivalent to "xmon=early" */ >>> - xmon_init(1); >>> xmon_early = 1; >>> + xmon_off = 0; >>> } else if (strncmp(p, "on", 2) == 0) >>> - xmon_init(1); >>> + xmon_off = 0; >> >> You've just changed the timing of when xmon gets enabled for the above >> two cases, from here which is called very early, to xmon_setup() which >> is called much later in boot. >> >> That effectively disables xmon for most of the boot, which we do not >> want to do. >> > Although it is not often that kernel got stucked during boot. I hope you're joking! :) cheers
Re: [PATCH] powerpc/xmon: Fix an unexpected xmon onoff state change
Pan Xinhui writes: > 在 2017/2/17 14:05, Michael Ellerman 写道: >> Pan Xinhui writes: >>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c >>> index 9c0e17c..f6e5c3d 100644 >>> --- a/arch/powerpc/xmon/xmon.c >>> +++ b/arch/powerpc/xmon/xmon.c >>> @@ -76,6 +76,7 @@ static int xmon_gate; >>> #endif /* CONFIG_SMP */ >>> >>> static unsigned long in_xmon __read_mostly = 0; >>> +static int xmon_off = !IS_ENABLED(CONFIG_XMON_DEFAULT); >> >> I think the logic would probably clearer if we invert this to become >> xmon_on. >> > yep, make sense. > >>> @@ -3266,16 +3269,16 @@ static int __init setup_xmon_sysrq(void) >>> __initcall(setup_xmon_sysrq); >>> #endif /* CONFIG_MAGIC_SYSRQ */ >>> >>> -static int __initdata xmon_early, xmon_off; >>> +static int __initdata xmon_early; >>> >>> static int __init early_parse_xmon(char *p) >>> { >>> if (!p || strncmp(p, "early", 5) == 0) { >>> /* just "xmon" is equivalent to "xmon=early" */ >>> - xmon_init(1); >>> xmon_early = 1; >>> + xmon_off = 0; >>> } else if (strncmp(p, "on", 2) == 0) >>> - xmon_init(1); >>> + xmon_off = 0; >> >> You've just changed the timing of when xmon gets enabled for the above >> two cases, from here which is called very early, to xmon_setup() which >> is called much later in boot. >> >> That effectively disables xmon for most of the boot, which we do not >> want to do. >> > Although it is not often that kernel got stucked during boot. I hope you're joking! :) cheers
Re: [PATCH] ipmi: bt-bmc: Use a regmap for register access
Hi Cory, On Tue, 2016-12-06 at 16:02 +0100, Cédric Le Goater wrote: > [ this is a resend bc of some mailing list issues] > > On 12/06/2016 03:57 AM, Andrew Jeffery wrote: > > The registers for the bt-bmc device live under the Aspeed LPC > > controller. Devicetree bindings have recently been introduced for the > > LPC controller where the "host" portion of the LPC register space is > > described as a syscon device. Future devicetrees describing the bt-bmc > > device should nest its node under the appropriate "simple-mfd", "syscon" > > compatible node. > > > > This change allows the bt-bmc driver to function with both syscon and > > non-syscon- based devicetree descriptions by always using a regmap for > > register access, either retrieved from the parent syscon device or > > instantiated if none exists. > > > > The patch has been tested on an OpenPOWER Palmetto machine, successfully > > booting, rebooting and powering down the host. > > > > Signed-off-by: Andrew Jeffery> > It would be nice to have an example of the associated binding. > I did not see it. A part from that : > > Reviewed-by: Cédric Le Goater Will this make it into 4.11? Cheers, Andrew signature.asc Description: This is a digitally signed message part
Re: [PATCH] ipmi: bt-bmc: Use a regmap for register access
Hi Cory, On Tue, 2016-12-06 at 16:02 +0100, Cédric Le Goater wrote: > [ this is a resend bc of some mailing list issues] > > On 12/06/2016 03:57 AM, Andrew Jeffery wrote: > > The registers for the bt-bmc device live under the Aspeed LPC > > controller. Devicetree bindings have recently been introduced for the > > LPC controller where the "host" portion of the LPC register space is > > described as a syscon device. Future devicetrees describing the bt-bmc > > device should nest its node under the appropriate "simple-mfd", "syscon" > > compatible node. > > > > This change allows the bt-bmc driver to function with both syscon and > > non-syscon- based devicetree descriptions by always using a regmap for > > register access, either retrieved from the parent syscon device or > > instantiated if none exists. > > > > The patch has been tested on an OpenPOWER Palmetto machine, successfully > > booting, rebooting and powering down the host. > > > > Signed-off-by: Andrew Jeffery > > It would be nice to have an example of the associated binding. > I did not see it. A part from that : > > Reviewed-by: Cédric Le Goater Will this make it into 4.11? Cheers, Andrew signature.asc Description: This is a digitally signed message part
Re: [PATCH 06/35] powerpc: Convert remaining uses of pr_warning to pr_warn
On Mon, 2017-02-20 at 15:40 +1100, Michael Ellerman wrote: > Joe Percheswrites: > > > To enable eventual removal of pr_warning > > > > This makes pr_warn use consistent for arch/powerpc > > > > Prior to this patch, there were 36 uses of pr_warning and > > 217 uses of pr_warn in arch/powerpc > > > > Signed-off-by: Joe Perches > > Can I take this via the powerpc tree, or do you want to merge them as a > series? Well, I expect it'd be better if you merge it.
Re: [PATCH 06/35] powerpc: Convert remaining uses of pr_warning to pr_warn
On Mon, 2017-02-20 at 15:40 +1100, Michael Ellerman wrote: > Joe Perches writes: > > > To enable eventual removal of pr_warning > > > > This makes pr_warn use consistent for arch/powerpc > > > > Prior to this patch, there were 36 uses of pr_warning and > > 217 uses of pr_warn in arch/powerpc > > > > Signed-off-by: Joe Perches > > Can I take this via the powerpc tree, or do you want to merge them as a > series? Well, I expect it'd be better if you merge it.
Re: [PATCH 06/35] powerpc: Convert remaining uses of pr_warning to pr_warn
Joe Percheswrites: > To enable eventual removal of pr_warning > > This makes pr_warn use consistent for arch/powerpc > > Prior to this patch, there were 36 uses of pr_warning and > 217 uses of pr_warn in arch/powerpc > > Signed-off-by: Joe Perches Can I take this via the powerpc tree, or do you want to merge them as a series? cheers
Re: [PATCH 06/35] powerpc: Convert remaining uses of pr_warning to pr_warn
Joe Perches writes: > To enable eventual removal of pr_warning > > This makes pr_warn use consistent for arch/powerpc > > Prior to this patch, there were 36 uses of pr_warning and > 217 uses of pr_warn in arch/powerpc > > Signed-off-by: Joe Perches Can I take this via the powerpc tree, or do you want to merge them as a series? cheers
Re: [RFC 1/1] shiftfs: uid/gid shifting bind mount
James Bottomleywrites: > On Fri, 2017-02-17 at 14:57 +1300, Eric W. Biederman wrote: >> I think I am missing something but I completely do not understand >> that subthread that says use file marks and perform the work in the >> vfs. The problem is that fundamentally we need multiple mappings and >> I don't see a mark on a file (even an inherited mark) providing the >> mapping so I don't see the point. > > The point of the mark is that it's a statement by the system > administrator that the underlying subtree is safe to be mounted by an > unprivileged container in the containers user view (i.e. with > current_user_ns() == s_user_ns). For the unprivileged container > there's no real arbitrary s_user_ns use case because the unprivileged > container must prove it can set up the mapping, so it would likely > always be mounting from within a user_ns with the mapping it wanted. As a statement that it is ok for the unprivileged mapping code to operate that seems reasonable. I don't currently the need for such an ok from the system adminstrator, but if you need it a flag that propagates to children and child directories seems reasonable. Eric
Re: [RFC 1/1] shiftfs: uid/gid shifting bind mount
James Bottomley writes: > On Fri, 2017-02-17 at 14:57 +1300, Eric W. Biederman wrote: >> I think I am missing something but I completely do not understand >> that subthread that says use file marks and perform the work in the >> vfs. The problem is that fundamentally we need multiple mappings and >> I don't see a mark on a file (even an inherited mark) providing the >> mapping so I don't see the point. > > The point of the mark is that it's a statement by the system > administrator that the underlying subtree is safe to be mounted by an > unprivileged container in the containers user view (i.e. with > current_user_ns() == s_user_ns). For the unprivileged container > there's no real arbitrary s_user_ns use case because the unprivileged > container must prove it can set up the mapping, so it would likely > always be mounting from within a user_ns with the mapping it wanted. As a statement that it is ok for the unprivileged mapping code to operate that seems reasonable. I don't currently the need for such an ok from the system adminstrator, but if you need it a flag that propagates to children and child directories seems reasonable. Eric
Re: [PATCH] drm/bridge: analogix_dp: Don't return -EBUSY when msg->size is 0 in aux transaction
On Mon, Feb 20, 2017 at 1:04 PM, Zain Wangwrote: > 在 2017/2/20 10:40, Tomasz Figa 写道: >> On Mon, Feb 13, 2017 at 6:27 PM, zain wang wrote: >>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c >>> b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c >>> index 303083a..5384aca 100644 >>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c >>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c >>> @@ -1162,5 +1162,5 @@ ssize_t analogix_dp_transfer(struct >>> analogix_dp_device *dp, >>> (msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_NATIVE_READ) >>> msg->reply = DP_AUX_NATIVE_REPLY_ACK; >>> >>> - return num_transferred > 0 ? num_transferred : -EBUSY; >>> + return (num_transferred == msg->size) ? num_transferred : -EBUSY; >> >> I might be missing something but, looking at the code, I don't see any >> possibility of num_transferred ever being different than msg->size. To >> be honest, it doesn't seem to even make any sense keeping the local >> variable there, because msg->size can be simply always returned, as >> errors are handled by jumping to aux_error label. > > Yeah, I agree with you. > The better way to fix this issue is to revert the changes > https://patchwork.kernel.org/patch/9411711/ > (returning num_transferred directly may be better here) I think it's still not enough to clean up the code completely. It should be just enough to remove num_transferred completely and simply return msg->size at the end. Best regards, Tomasz
Re: [PATCH] drm/bridge: analogix_dp: Don't return -EBUSY when msg->size is 0 in aux transaction
On Mon, Feb 20, 2017 at 1:04 PM, Zain Wang wrote: > 在 2017/2/20 10:40, Tomasz Figa 写道: >> On Mon, Feb 13, 2017 at 6:27 PM, zain wang wrote: >>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c >>> b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c >>> index 303083a..5384aca 100644 >>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c >>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c >>> @@ -1162,5 +1162,5 @@ ssize_t analogix_dp_transfer(struct >>> analogix_dp_device *dp, >>> (msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_NATIVE_READ) >>> msg->reply = DP_AUX_NATIVE_REPLY_ACK; >>> >>> - return num_transferred > 0 ? num_transferred : -EBUSY; >>> + return (num_transferred == msg->size) ? num_transferred : -EBUSY; >> >> I might be missing something but, looking at the code, I don't see any >> possibility of num_transferred ever being different than msg->size. To >> be honest, it doesn't seem to even make any sense keeping the local >> variable there, because msg->size can be simply always returned, as >> errors are handled by jumping to aux_error label. > > Yeah, I agree with you. > The better way to fix this issue is to revert the changes > https://patchwork.kernel.org/patch/9411711/ > (returning num_transferred directly may be better here) I think it's still not enough to clean up the code completely. It should be just enough to remove num_transferred completely and simply return msg->size at the end. Best regards, Tomasz
Re: [PATCH] switchtec: cleanup cdev init
On 19/02/17 02:43 PM, Dan Williams wrote: > Is this race present for all file operations? I've only seen it with > mmap() and late faults. So if these other drivers do not support mmap > it's not clear they can trigger the failure. I don't see how it's related to mmap only. I think there's a number of variations on this but the race I see happens if you try to take a reference to the device in the open/close handlers of a char device file. If a driver puts the last remaining reference in the release handler, then the device structure will be freed, and on returning from the release handler, the char_dev code will try to put the last reference to the cdev structure which may have already been free'd. There needs to be a way for the cdev structure to take a reference on the device structure so it doesn't get freed and the kobj.parent trick seems to accomplish this fairly well. Really, in any situation where there's a cdev and a device in the same structure, the life cycles of the two become linked but their reference counts are not and that is the problem here. I feel like a number of developers have made the same realization independently so this would probably be a good thing to clean up. See these commits for examples spanning around 5 years: 4a215aa Input: fix use-after-free introduced with dynamic minor changes 0d5b7da iio: Prevent race between IIO chardev opening and IIO device ba0ef85 tpm: Fix initialization of the cdev Also, seems both my brother and I have independently looked into this exact issue: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg25692.html So, I think it would be a good idea to clean it up with Dan's proposed API cleanup. If I have some time tomorrow, I may throw together a patch set with that patch and the changes to the instances around the kernel. Logan
Re: [PATCH] switchtec: cleanup cdev init
On 19/02/17 02:43 PM, Dan Williams wrote: > Is this race present for all file operations? I've only seen it with > mmap() and late faults. So if these other drivers do not support mmap > it's not clear they can trigger the failure. I don't see how it's related to mmap only. I think there's a number of variations on this but the race I see happens if you try to take a reference to the device in the open/close handlers of a char device file. If a driver puts the last remaining reference in the release handler, then the device structure will be freed, and on returning from the release handler, the char_dev code will try to put the last reference to the cdev structure which may have already been free'd. There needs to be a way for the cdev structure to take a reference on the device structure so it doesn't get freed and the kobj.parent trick seems to accomplish this fairly well. Really, in any situation where there's a cdev and a device in the same structure, the life cycles of the two become linked but their reference counts are not and that is the problem here. I feel like a number of developers have made the same realization independently so this would probably be a good thing to clean up. See these commits for examples spanning around 5 years: 4a215aa Input: fix use-after-free introduced with dynamic minor changes 0d5b7da iio: Prevent race between IIO chardev opening and IIO device ba0ef85 tpm: Fix initialization of the cdev Also, seems both my brother and I have independently looked into this exact issue: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg25692.html So, I think it would be a good idea to clean it up with Dan's proposed API cleanup. If I have some time tomorrow, I may throw together a patch set with that patch and the changes to the instances around the kernel. Logan
Re: [PATCH v3] locking/pvqspinlock: Relax cmpxchg's to improve performance on some archs
On Fri, Feb 17, 2017 at 03:43:40PM -0500, Waiman Long wrote: > All the locking related cmpxchg's in the following functions are > replaced with the _acquire variants: > - pv_queued_spin_steal_lock() > - trylock_clear_pending() > > This change should help performance on architectures that use LL/SC. > > On a 2-core 16-thread Power8 system with pvqspinlock explicitly > enabled, the performance of a locking microbenchmark with and without > this patch on a 4.10-rc8 kernel with Xinhui's PPC qspinlock patch > were as follows: > > # of thread w/o patchwith patch % Change > --- --- >4 4053.3 Mop/s 4223.7 Mop/s +4.2% >8 3310.4 Mop/s 3406.0 Mop/s +2.9% > 12 2576.4 Mop/s 2674.6 Mop/s +3.8% > > Signed-off-by: Waiman Long> --- > > v2->v3: > - Reduce scope by relaxing cmpxchg's in fast path only. > > v1->v2: > - Add comments in changelog and code for the rationale of the change. > > kernel/locking/qspinlock_paravirt.h | 15 +-- > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/kernel/locking/qspinlock_paravirt.h > b/kernel/locking/qspinlock_paravirt.h > index e6b2f7a..a59dc05 100644 > --- a/kernel/locking/qspinlock_paravirt.h > +++ b/kernel/locking/qspinlock_paravirt.h > @@ -72,7 +72,7 @@ static inline bool pv_queued_spin_steal_lock(struct > qspinlock *lock) > struct __qspinlock *l = (void *)lock; > > if (!(atomic_read(>val) & _Q_LOCKED_PENDING_MASK) && > - (cmpxchg(>locked, 0, _Q_LOCKED_VAL) == 0)) { > + (cmpxchg_acquire(>locked, 0, _Q_LOCKED_VAL) == 0)) { > qstat_inc(qstat_pv_lock_stealing, true); > return true; > } > @@ -101,16 +101,16 @@ static __always_inline void clear_pending(struct > qspinlock *lock) > > /* > * The pending bit check in pv_queued_spin_steal_lock() isn't a memory > - * barrier. Therefore, an atomic cmpxchg() is used to acquire the lock > - * just to be sure that it will get it. > + * barrier. Therefore, an atomic cmpxchg_acquire() is used to acquire the > + * lock just to be sure that it will get it. > */ > static __always_inline int trylock_clear_pending(struct qspinlock *lock) > { > struct __qspinlock *l = (void *)lock; > > return !READ_ONCE(l->locked) && > -(cmpxchg(>locked_pending, _Q_PENDING_VAL, _Q_LOCKED_VAL) > - == _Q_PENDING_VAL); > +(cmpxchg_acquire(>locked_pending, _Q_PENDING_VAL, > + _Q_LOCKED_VAL) == _Q_PENDING_VAL); > } > #else /* _Q_PENDING_BITS == 8 */ > static __always_inline void set_pending(struct qspinlock *lock) > @@ -138,7 +138,7 @@ static __always_inline int trylock_clear_pending(struct > qspinlock *lock) >*/ > old = val; > new = (val & ~_Q_PENDING_MASK) | _Q_LOCKED_VAL; > - val = atomic_cmpxchg(>val, old, new); > + val = atomic_cmpxchg_acquire(>val, old, new); > > if (val == old) > return 1; > @@ -361,6 +361,9 @@ static void pv_kick_node(struct qspinlock *lock, struct > mcs_spinlock *node) >* observe its next->locked value and advance itself. >* >* Matches with smp_store_mb() and cmpxchg() in pv_wait_node() > + * > + * We can't used relaxed form of cmpxchg here as the loading of > + * pn->state can happen before setting next->locked in some archs. >*/ > if (cmpxchg(>state, vcpu_halted, vcpu_hashed) != vcpu_halted) Hi Waiman. cmpxchg() does not guarantee the (here implied) smp_mb(), in general; c.f., e.g., arch/arm64/include/asm/atomic_ll_sc.h, where cmpxchg() get "compiled" to something like: _loop: ldxr; eor; cbnz _exit; stlxr; cbnz _loop; dmb ish; _exit: Andrea > return; > -- > 1.8.3.1 >
Re: [PATCH v3] locking/pvqspinlock: Relax cmpxchg's to improve performance on some archs
On Fri, Feb 17, 2017 at 03:43:40PM -0500, Waiman Long wrote: > All the locking related cmpxchg's in the following functions are > replaced with the _acquire variants: > - pv_queued_spin_steal_lock() > - trylock_clear_pending() > > This change should help performance on architectures that use LL/SC. > > On a 2-core 16-thread Power8 system with pvqspinlock explicitly > enabled, the performance of a locking microbenchmark with and without > this patch on a 4.10-rc8 kernel with Xinhui's PPC qspinlock patch > were as follows: > > # of thread w/o patchwith patch % Change > --- --- >4 4053.3 Mop/s 4223.7 Mop/s +4.2% >8 3310.4 Mop/s 3406.0 Mop/s +2.9% > 12 2576.4 Mop/s 2674.6 Mop/s +3.8% > > Signed-off-by: Waiman Long > --- > > v2->v3: > - Reduce scope by relaxing cmpxchg's in fast path only. > > v1->v2: > - Add comments in changelog and code for the rationale of the change. > > kernel/locking/qspinlock_paravirt.h | 15 +-- > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/kernel/locking/qspinlock_paravirt.h > b/kernel/locking/qspinlock_paravirt.h > index e6b2f7a..a59dc05 100644 > --- a/kernel/locking/qspinlock_paravirt.h > +++ b/kernel/locking/qspinlock_paravirt.h > @@ -72,7 +72,7 @@ static inline bool pv_queued_spin_steal_lock(struct > qspinlock *lock) > struct __qspinlock *l = (void *)lock; > > if (!(atomic_read(>val) & _Q_LOCKED_PENDING_MASK) && > - (cmpxchg(>locked, 0, _Q_LOCKED_VAL) == 0)) { > + (cmpxchg_acquire(>locked, 0, _Q_LOCKED_VAL) == 0)) { > qstat_inc(qstat_pv_lock_stealing, true); > return true; > } > @@ -101,16 +101,16 @@ static __always_inline void clear_pending(struct > qspinlock *lock) > > /* > * The pending bit check in pv_queued_spin_steal_lock() isn't a memory > - * barrier. Therefore, an atomic cmpxchg() is used to acquire the lock > - * just to be sure that it will get it. > + * barrier. Therefore, an atomic cmpxchg_acquire() is used to acquire the > + * lock just to be sure that it will get it. > */ > static __always_inline int trylock_clear_pending(struct qspinlock *lock) > { > struct __qspinlock *l = (void *)lock; > > return !READ_ONCE(l->locked) && > -(cmpxchg(>locked_pending, _Q_PENDING_VAL, _Q_LOCKED_VAL) > - == _Q_PENDING_VAL); > +(cmpxchg_acquire(>locked_pending, _Q_PENDING_VAL, > + _Q_LOCKED_VAL) == _Q_PENDING_VAL); > } > #else /* _Q_PENDING_BITS == 8 */ > static __always_inline void set_pending(struct qspinlock *lock) > @@ -138,7 +138,7 @@ static __always_inline int trylock_clear_pending(struct > qspinlock *lock) >*/ > old = val; > new = (val & ~_Q_PENDING_MASK) | _Q_LOCKED_VAL; > - val = atomic_cmpxchg(>val, old, new); > + val = atomic_cmpxchg_acquire(>val, old, new); > > if (val == old) > return 1; > @@ -361,6 +361,9 @@ static void pv_kick_node(struct qspinlock *lock, struct > mcs_spinlock *node) >* observe its next->locked value and advance itself. >* >* Matches with smp_store_mb() and cmpxchg() in pv_wait_node() > + * > + * We can't used relaxed form of cmpxchg here as the loading of > + * pn->state can happen before setting next->locked in some archs. >*/ > if (cmpxchg(>state, vcpu_halted, vcpu_hashed) != vcpu_halted) Hi Waiman. cmpxchg() does not guarantee the (here implied) smp_mb(), in general; c.f., e.g., arch/arm64/include/asm/atomic_ll_sc.h, where cmpxchg() get "compiled" to something like: _loop: ldxr; eor; cbnz _exit; stlxr; cbnz _loop; dmb ish; _exit: Andrea > return; > -- > 1.8.3.1 >
Re: [PATCH v6 1/3] perf: add PERF_RECORD_NAMESPACES to include namespaces related info
Hi Peter, On Thursday 16 February 2017 03:58 PM, Peter Zijlstra wrote: On Wed, Feb 08, 2017 at 02:01:24PM +0530, Hari Bathini wrote: With the advert of container technologies like docker, that depend on namespaces for isolation, there is a need for tracing support for namespaces. This patch introduces new PERF_RECORD_NAMESPACES event for tracing based on namespaces related info. This event records the device and inode numbers for every namespace of all processes. While device number is same for all namespaces currently, that may 'While device numbers are the same ..." ? change in future, to avoid the need for a namespace of namespaces. Unfinished sentence Unnecessary comma spoiled the sentence, I guess. Recording device number along with inode number will take care of such scenario. More words on why this is so? Because I've no clue. This should have read: "Each namespace has a combination of device and inode numbers. Though every namespace has the same device number currently, that may change in future to avoid the need for a namespace of namespaces. Considering such possibility, record both device and inode numbers separately for each namespace." Will update changelog accordingly.. @@ -6584,6 +6590,135 @@ void perf_event_comm(struct task_struct *task, bool exec) } /* + * namespaces tracking + */ + +struct namespaces_event_id { + struct perf_event_headerheader; + + u32 pid; + u32 tid; + u32 nr_namespaces; + struct perf_ns_link_infolink_info[NAMESPACES_MAX]; +}; Contains the same hole and makes the record depend on architecture alignment requirements. Ugh! Let me change nr_namespaces type to u64 and also drop name field in perf_ns_link_info struct. +static void perf_fill_ns_link_info(struct perf_ns_link_info *ns_link_info, + struct task_struct *task, + const struct proc_ns_operations *ns_ops) +{ + struct path ns_path; + struct inode *ns_inode; + void *error; + + error = ns_get_path(_path, task, ns_ops); + if (!error) { + snprintf(ns_link_info->name, NS_NAME_SIZE, +"%s", ns_path.dentry->d_iname); + + ns_inode = ns_path.dentry->d_inode; + ns_link_info->dev = new_encode_dev(ns_inode->i_sb->s_dev); + ns_link_info->ino = ns_inode->i_ino; + } +} + +static void perf_event_namespaces_output(struct perf_event *event, +void *data) +{ + struct perf_namespaces_event *namespaces_event = data; + struct perf_output_handle handle; + struct perf_sample_data sample; + struct namespaces_event_id *ei; + struct task_struct *task = namespaces_event->task; + int ret; + + if (!perf_event_namespaces_match(event)) + return; + + ei = _event->event_id; + perf_event_header__init_id(>header, , event); + ret = perf_output_begin(, event, ei->header.size); + if (ret) + return; + + ei->pid = perf_event_pid(event, task); + ei->tid = perf_event_tid(event, task); + + ei->nr_namespaces = NAMESPACES_MAX; + + perf_fill_ns_link_info(>link_info[MNT_NS_INDEX], + task, _operations); + +#ifdef CONFIG_USER_NS + perf_fill_ns_link_info(>link_info[USER_NS_INDEX], + task, _operations); +#endif +#ifdef CONFIG_NET_NS + perf_fill_ns_link_info(>link_info[NET_NS_INDEX], + task, _operations); +#endif +#ifdef CONFIG_UTS_NS + perf_fill_ns_link_info(>link_info[UTS_NS_INDEX], + task, _operations); +#endif +#ifdef CONFIG_IPC_NS + perf_fill_ns_link_info(>link_info[IPC_NS_INDEX], + task, _operations); +#endif +#ifdef CONFIG_PID_NS + perf_fill_ns_link_info(>link_info[PID_NS_INDEX], + task, _operations); +#endif +#ifdef CONFIG_CGROUPS + perf_fill_ns_link_info(>link_info[CGROUP_NS_INDEX], + task, _operations); +#endif Two points here, since you've given these thingies a string identifier, do you still need to rely on their positional index? If not, you could generate smaller events if we lack some of these CONFIG knobs. Dropping name field. So, I don't think this applies now.. Secondly, does anything in here depend on @event? Afaict you're generating the exact same information for each event. The reason we set {pid,tid} for each event is because of PID_NS, we report the pid number as per the namespace associated with the event. But from what I can tell, there is no such namespace of namespaces dependency here and this should live in the function below. Right. I will move it to perf_event_namespaces()
Re: [PATCH v6 1/3] perf: add PERF_RECORD_NAMESPACES to include namespaces related info
Hi Peter, On Thursday 16 February 2017 03:58 PM, Peter Zijlstra wrote: On Wed, Feb 08, 2017 at 02:01:24PM +0530, Hari Bathini wrote: With the advert of container technologies like docker, that depend on namespaces for isolation, there is a need for tracing support for namespaces. This patch introduces new PERF_RECORD_NAMESPACES event for tracing based on namespaces related info. This event records the device and inode numbers for every namespace of all processes. While device number is same for all namespaces currently, that may 'While device numbers are the same ..." ? change in future, to avoid the need for a namespace of namespaces. Unfinished sentence Unnecessary comma spoiled the sentence, I guess. Recording device number along with inode number will take care of such scenario. More words on why this is so? Because I've no clue. This should have read: "Each namespace has a combination of device and inode numbers. Though every namespace has the same device number currently, that may change in future to avoid the need for a namespace of namespaces. Considering such possibility, record both device and inode numbers separately for each namespace." Will update changelog accordingly.. @@ -6584,6 +6590,135 @@ void perf_event_comm(struct task_struct *task, bool exec) } /* + * namespaces tracking + */ + +struct namespaces_event_id { + struct perf_event_headerheader; + + u32 pid; + u32 tid; + u32 nr_namespaces; + struct perf_ns_link_infolink_info[NAMESPACES_MAX]; +}; Contains the same hole and makes the record depend on architecture alignment requirements. Ugh! Let me change nr_namespaces type to u64 and also drop name field in perf_ns_link_info struct. +static void perf_fill_ns_link_info(struct perf_ns_link_info *ns_link_info, + struct task_struct *task, + const struct proc_ns_operations *ns_ops) +{ + struct path ns_path; + struct inode *ns_inode; + void *error; + + error = ns_get_path(_path, task, ns_ops); + if (!error) { + snprintf(ns_link_info->name, NS_NAME_SIZE, +"%s", ns_path.dentry->d_iname); + + ns_inode = ns_path.dentry->d_inode; + ns_link_info->dev = new_encode_dev(ns_inode->i_sb->s_dev); + ns_link_info->ino = ns_inode->i_ino; + } +} + +static void perf_event_namespaces_output(struct perf_event *event, +void *data) +{ + struct perf_namespaces_event *namespaces_event = data; + struct perf_output_handle handle; + struct perf_sample_data sample; + struct namespaces_event_id *ei; + struct task_struct *task = namespaces_event->task; + int ret; + + if (!perf_event_namespaces_match(event)) + return; + + ei = _event->event_id; + perf_event_header__init_id(>header, , event); + ret = perf_output_begin(, event, ei->header.size); + if (ret) + return; + + ei->pid = perf_event_pid(event, task); + ei->tid = perf_event_tid(event, task); + + ei->nr_namespaces = NAMESPACES_MAX; + + perf_fill_ns_link_info(>link_info[MNT_NS_INDEX], + task, _operations); + +#ifdef CONFIG_USER_NS + perf_fill_ns_link_info(>link_info[USER_NS_INDEX], + task, _operations); +#endif +#ifdef CONFIG_NET_NS + perf_fill_ns_link_info(>link_info[NET_NS_INDEX], + task, _operations); +#endif +#ifdef CONFIG_UTS_NS + perf_fill_ns_link_info(>link_info[UTS_NS_INDEX], + task, _operations); +#endif +#ifdef CONFIG_IPC_NS + perf_fill_ns_link_info(>link_info[IPC_NS_INDEX], + task, _operations); +#endif +#ifdef CONFIG_PID_NS + perf_fill_ns_link_info(>link_info[PID_NS_INDEX], + task, _operations); +#endif +#ifdef CONFIG_CGROUPS + perf_fill_ns_link_info(>link_info[CGROUP_NS_INDEX], + task, _operations); +#endif Two points here, since you've given these thingies a string identifier, do you still need to rely on their positional index? If not, you could generate smaller events if we lack some of these CONFIG knobs. Dropping name field. So, I don't think this applies now.. Secondly, does anything in here depend on @event? Afaict you're generating the exact same information for each event. The reason we set {pid,tid} for each event is because of PID_NS, we report the pid number as per the namespace associated with the event. But from what I can tell, there is no such namespace of namespaces dependency here and this should live in the function below. Right. I will move it to perf_event_namespaces()
Re: [PATCH] Add pidfs filesystem
Alexey Gladkovwrites: > The pidfs filesystem contains a subset of the /proc file system which > contains only information about the processes. My summary of your motivation. It hurts when I create a container with a processes with uid 0 inside of it. This generates lots of hacks to attempt to limit uid 0. My answer: Don't run a container with a real uid 0 inside of it. Any reasonable permission check will on proc files will keep you safe if your container does not have a real uid 0 in it. That said I am not opposed in principle to a pidfs. And the idea of using overlay for this purpose is intriguing. I have not looked at it in enough detail comment on the technical merits. Eric > Some of the container virtualization systems are mounted /proc inside > the container. This is done in most cases to operate with information > about the processes. Knowing that /proc filesystem is not fully > virtualized they are mounted on top of dangerous places empty files or > directories (for exmaple /proc/kcore, /sys/firmware, etc.). > > The structure of this filesystem is dynamic and any module can create a > new object which will not necessarily be virtualized. There are > proprietary modules that aren't in the mainline whose work we can not > verify. > > This opens up a potential threat to the system. The developers of the > virtualization system can't predict all dangerous places in /proc by > definition. > > A more effective solution would be to mount into the container only what > is necessary and ignore the rest. > > Right now there is the opportunity to pass in the container any port of > the /proc filesystem using mount --bind expect the pids. > > This patch allows to mount only the part of /proc related to pids > without rest objects. Since this is an addon to /proc, flags applied to > /proc have an effect on this pidfs filesystem. > > Why not implement it as another flag to /proc ? > > The /proc flags is stored in the pid_namespace and are global for > namespace. It means that if you add a flag to hide all except the pids, > then it will act on all mounted instances of /proc. > > Originally the idea was that the container will be mounted only pidfs > and additional required files will be mounted on top using the > overlayfs. But I found out that /proc does not support overlayfs and > does not allow to mount anything on top or under it. > > My question is whether it's possible to add overlayfs support for /proc? > > Cc: Kirill A. Shutemov > Signed-off-by: Alexey Gladkov > --- > Documentation/filesystems/pidfs.txt | 16 > fs/proc/Kconfig | 8 > fs/proc/inode.c | 8 +++- > fs/proc/internal.h | 2 + > fs/proc/root.c | 76 > ++--- > fs/proc/self.c | 6 +++ > fs/proc/thread_self.c | 6 +++ > include/linux/pid_namespace.h | 5 +++ > 8 files changed, 119 insertions(+), 8 deletions(-) > create mode 100644 Documentation/filesystems/pidfs.txt > > diff --git a/Documentation/filesystems/pidfs.txt > b/Documentation/filesystems/pidfs.txt > new file mode 100644 > index 000..ce958a5 > --- /dev/null > +++ b/Documentation/filesystems/pidfs.txt > @@ -0,0 +1,16 @@ > +The PIDFS Filesystem > + > + > +The pidfs filesystem contains a subset of the /proc file system which > contains > +only information about the processes. The link self points to the process > +reading the file system. All other special files and directories in /proc are > +not available in this filesystem. > + > +The pidfs is not an independent filesystem, its implementation shares code > +with /proc. > + > +All mount options applicable to /proc filesystem are also applicable > +to pidfs filesystem. For example, access to the information in /proc/[pid] > +directories can be restricted using hidepid option. > + > +To get more information about the processes read the proc.txt > diff --git a/fs/proc/Kconfig b/fs/proc/Kconfig > index 1ade120..fa568f6 100644 > --- a/fs/proc/Kconfig > +++ b/fs/proc/Kconfig > @@ -43,6 +43,14 @@ config PROC_VMCORE > help > Exports the dump image of crashed kernel in ELF format. > > +config PROC_PIDFS > + bool "pidfs file system support" > + depends on PROC_FS > + default n > + help > + The pidfs filesystem contains a subset of the /proc file system > + which contains only information only about the processes. > + > config PROC_SYSCTL > bool "Sysctl support (/proc/sys)" if EXPERT > depends on PROC_FS > diff --git a/fs/proc/inode.c b/fs/proc/inode.c > index 783bc19..1be65b4 100644 > --- a/fs/proc/inode.c > +++ b/fs/proc/inode.c > @@ -474,12 +474,16 @@ struct inode *proc_get_inode(struct super_block *sb, > struct proc_dir_entry *de) > int proc_fill_super(struct super_block *s, void *data, int
Re: [PATCH] Add pidfs filesystem
Alexey Gladkov writes: > The pidfs filesystem contains a subset of the /proc file system which > contains only information about the processes. My summary of your motivation. It hurts when I create a container with a processes with uid 0 inside of it. This generates lots of hacks to attempt to limit uid 0. My answer: Don't run a container with a real uid 0 inside of it. Any reasonable permission check will on proc files will keep you safe if your container does not have a real uid 0 in it. That said I am not opposed in principle to a pidfs. And the idea of using overlay for this purpose is intriguing. I have not looked at it in enough detail comment on the technical merits. Eric > Some of the container virtualization systems are mounted /proc inside > the container. This is done in most cases to operate with information > about the processes. Knowing that /proc filesystem is not fully > virtualized they are mounted on top of dangerous places empty files or > directories (for exmaple /proc/kcore, /sys/firmware, etc.). > > The structure of this filesystem is dynamic and any module can create a > new object which will not necessarily be virtualized. There are > proprietary modules that aren't in the mainline whose work we can not > verify. > > This opens up a potential threat to the system. The developers of the > virtualization system can't predict all dangerous places in /proc by > definition. > > A more effective solution would be to mount into the container only what > is necessary and ignore the rest. > > Right now there is the opportunity to pass in the container any port of > the /proc filesystem using mount --bind expect the pids. > > This patch allows to mount only the part of /proc related to pids > without rest objects. Since this is an addon to /proc, flags applied to > /proc have an effect on this pidfs filesystem. > > Why not implement it as another flag to /proc ? > > The /proc flags is stored in the pid_namespace and are global for > namespace. It means that if you add a flag to hide all except the pids, > then it will act on all mounted instances of /proc. > > Originally the idea was that the container will be mounted only pidfs > and additional required files will be mounted on top using the > overlayfs. But I found out that /proc does not support overlayfs and > does not allow to mount anything on top or under it. > > My question is whether it's possible to add overlayfs support for /proc? > > Cc: Kirill A. Shutemov > Signed-off-by: Alexey Gladkov > --- > Documentation/filesystems/pidfs.txt | 16 > fs/proc/Kconfig | 8 > fs/proc/inode.c | 8 +++- > fs/proc/internal.h | 2 + > fs/proc/root.c | 76 > ++--- > fs/proc/self.c | 6 +++ > fs/proc/thread_self.c | 6 +++ > include/linux/pid_namespace.h | 5 +++ > 8 files changed, 119 insertions(+), 8 deletions(-) > create mode 100644 Documentation/filesystems/pidfs.txt > > diff --git a/Documentation/filesystems/pidfs.txt > b/Documentation/filesystems/pidfs.txt > new file mode 100644 > index 000..ce958a5 > --- /dev/null > +++ b/Documentation/filesystems/pidfs.txt > @@ -0,0 +1,16 @@ > +The PIDFS Filesystem > + > + > +The pidfs filesystem contains a subset of the /proc file system which > contains > +only information about the processes. The link self points to the process > +reading the file system. All other special files and directories in /proc are > +not available in this filesystem. > + > +The pidfs is not an independent filesystem, its implementation shares code > +with /proc. > + > +All mount options applicable to /proc filesystem are also applicable > +to pidfs filesystem. For example, access to the information in /proc/[pid] > +directories can be restricted using hidepid option. > + > +To get more information about the processes read the proc.txt > diff --git a/fs/proc/Kconfig b/fs/proc/Kconfig > index 1ade120..fa568f6 100644 > --- a/fs/proc/Kconfig > +++ b/fs/proc/Kconfig > @@ -43,6 +43,14 @@ config PROC_VMCORE > help > Exports the dump image of crashed kernel in ELF format. > > +config PROC_PIDFS > + bool "pidfs file system support" > + depends on PROC_FS > + default n > + help > + The pidfs filesystem contains a subset of the /proc file system > + which contains only information only about the processes. > + > config PROC_SYSCTL > bool "Sysctl support (/proc/sys)" if EXPERT > depends on PROC_FS > diff --git a/fs/proc/inode.c b/fs/proc/inode.c > index 783bc19..1be65b4 100644 > --- a/fs/proc/inode.c > +++ b/fs/proc/inode.c > @@ -474,12 +474,16 @@ struct inode *proc_get_inode(struct super_block *sb, > struct proc_dir_entry *de) > int proc_fill_super(struct super_block *s, void *data, int silent) > { > struct pid_namespace *ns = get_pid_ns(s->s_fs_info); > +
Re: [PATCH v2] MAINTAINERS: cpufreq: add bmips-cpufreq.c
On 19 February 2017 at 19:29, Viresh Kumarwrote: > On 17-02-17, 12:27, Markus Mayer wrote: >> From: Markus Mayer >> >> Add maintainer information for bmips-cpufreq.c. >> >> Signed-off-by: Markus Mayer >> Acked-by: Florian Fainelli >> --- >> >> This is based on PM's linux-next from today (February 17). >> >> This patch could be squashed into patch 3/4 of the original series if that >> is acceptable (see [1]) or it can remain separate. >> >> [1] https://lkml.org/lkml/2017/2/7/775 >> >> Changes in v2: >> - added bcm-kernel-feedback-l...@broadcom.com >> >> MAINTAINERS | 7 +++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 107c10e..d4ac248 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -2692,6 +2692,13 @@ F: drivers/irqchip/irq-brcmstb* >> F: include/linux/bcm963xx_nvram.h >> F: include/linux/bcm963xx_tag.h >> >> +BROADCOM BMIPS CPUFREQ DRIVER >> +M: Markus Mayer >> +M: bcm-kernel-feedback-l...@broadcom.com > > Isn't this a list as well? Shouldn't this be L: ? We used to do that, but then there was a discussion at some point in the past (I don't have the link handy for it at this point), where it was suggested that we use M: for the Broadcom list, because it is a private, closed list (i.e. you can't just go and subscribe). It does reach a bunch of folks at Broadcom, however, who work on upstreaming. Therefore, the reasoning was that it fits the "maintainer" category better than the "list" category (where people might expect to be able to subscribe or find a public archive). >> +L: linux...@vger.kernel.org >> +S: Maintained >> +F: drivers/cpufreq/bmips-cpufreq.c >> + >> BROADCOM TG3 GIGABIT ETHERNET DRIVER >> M: Siva Reddy Kallam >> M: Prashant Sreedharan > > -- > viresh
Re: [PATCH v2] MAINTAINERS: cpufreq: add bmips-cpufreq.c
On 19 February 2017 at 19:29, Viresh Kumar wrote: > On 17-02-17, 12:27, Markus Mayer wrote: >> From: Markus Mayer >> >> Add maintainer information for bmips-cpufreq.c. >> >> Signed-off-by: Markus Mayer >> Acked-by: Florian Fainelli >> --- >> >> This is based on PM's linux-next from today (February 17). >> >> This patch could be squashed into patch 3/4 of the original series if that >> is acceptable (see [1]) or it can remain separate. >> >> [1] https://lkml.org/lkml/2017/2/7/775 >> >> Changes in v2: >> - added bcm-kernel-feedback-l...@broadcom.com >> >> MAINTAINERS | 7 +++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 107c10e..d4ac248 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -2692,6 +2692,13 @@ F: drivers/irqchip/irq-brcmstb* >> F: include/linux/bcm963xx_nvram.h >> F: include/linux/bcm963xx_tag.h >> >> +BROADCOM BMIPS CPUFREQ DRIVER >> +M: Markus Mayer >> +M: bcm-kernel-feedback-l...@broadcom.com > > Isn't this a list as well? Shouldn't this be L: ? We used to do that, but then there was a discussion at some point in the past (I don't have the link handy for it at this point), where it was suggested that we use M: for the Broadcom list, because it is a private, closed list (i.e. you can't just go and subscribe). It does reach a bunch of folks at Broadcom, however, who work on upstreaming. Therefore, the reasoning was that it fits the "maintainer" category better than the "list" category (where people might expect to be able to subscribe or find a public archive). >> +L: linux...@vger.kernel.org >> +S: Maintained >> +F: drivers/cpufreq/bmips-cpufreq.c >> + >> BROADCOM TG3 GIGABIT ETHERNET DRIVER >> M: Siva Reddy Kallam >> M: Prashant Sreedharan > > -- > viresh
Re: [PATCH] drm/bridge: analogix_dp: Don't return -EBUSY when msg->size is 0 in aux transaction
Hi Tomasz, 在 2017/2/20 10:40, Tomasz Figa 写道: Hi Zain, On Mon, Feb 13, 2017 at 6:27 PM, zain wangwrote: The analogix_dp_transfer() will return -EBUSY if num_transferred is zero. But sometimes we will send a bare address packet to start the transaction, like drm_dp_i2c_xfer() show: .. /* Send a bare address packet to start the transaction. * Zero sized messages specify an address only (bare * address) transaction. */ msg.buffer = NULL; msg.size = 0; err = drm_dp_i2c_do_msg(aux, ); .. In this case, the msg->size is zero, so the num_transferred will be zero too. We can't return -EBUSY here, let's we return num_transferred if num_transferred equals msg->size. Please see my question inline. Signed-off-by: zain wang --- drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index 303083a..5384aca 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -1162,5 +1162,5 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, (msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_NATIVE_READ) msg->reply = DP_AUX_NATIVE_REPLY_ACK; - return num_transferred > 0 ? num_transferred : -EBUSY; + return (num_transferred == msg->size) ? num_transferred : -EBUSY; I might be missing something but, looking at the code, I don't see any possibility of num_transferred ever being different than msg->size. To be honest, it doesn't seem to even make any sense keeping the local variable there, because msg->size can be simply always returned, as errors are handled by jumping to aux_error label. Yeah, I agree with you. The better way to fix this issue is to revert the changes https://patchwork.kernel.org/patch/9411711/ (returning num_transferred directly may be better here) Maybe we can revert the changes above with some new comment. @Sean, How do you think about Tomasz's comment? Thanks Zain Best regards, Tomasz
Re: [PATCH] drm/bridge: analogix_dp: Don't return -EBUSY when msg->size is 0 in aux transaction
Hi Tomasz, 在 2017/2/20 10:40, Tomasz Figa 写道: Hi Zain, On Mon, Feb 13, 2017 at 6:27 PM, zain wang wrote: The analogix_dp_transfer() will return -EBUSY if num_transferred is zero. But sometimes we will send a bare address packet to start the transaction, like drm_dp_i2c_xfer() show: .. /* Send a bare address packet to start the transaction. * Zero sized messages specify an address only (bare * address) transaction. */ msg.buffer = NULL; msg.size = 0; err = drm_dp_i2c_do_msg(aux, ); .. In this case, the msg->size is zero, so the num_transferred will be zero too. We can't return -EBUSY here, let's we return num_transferred if num_transferred equals msg->size. Please see my question inline. Signed-off-by: zain wang --- drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index 303083a..5384aca 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -1162,5 +1162,5 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, (msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_NATIVE_READ) msg->reply = DP_AUX_NATIVE_REPLY_ACK; - return num_transferred > 0 ? num_transferred : -EBUSY; + return (num_transferred == msg->size) ? num_transferred : -EBUSY; I might be missing something but, looking at the code, I don't see any possibility of num_transferred ever being different than msg->size. To be honest, it doesn't seem to even make any sense keeping the local variable there, because msg->size can be simply always returned, as errors are handled by jumping to aux_error label. Yeah, I agree with you. The better way to fix this issue is to revert the changes https://patchwork.kernel.org/patch/9411711/ (returning num_transferred directly may be better here) Maybe we can revert the changes above with some new comment. @Sean, How do you think about Tomasz's comment? Thanks Zain Best regards, Tomasz
[PATCH net-next V2] virito-net: set queues after reset during xdp_set
We set queues before reset which will cause a crash[1]. This is because is_xdp_raw_buffer_queue() depends on the old xdp queue pairs number to do the correct detection. So fix this by - passing xdp queue pairs and current queue pairs to virtnet_reset() - change vi->xdp_qp after reset but before refill, to make sure both free_unused_bufs() and refill can make correct detection of XDP. - remove the duplicated queue pairs setting before virtnet_reset() since we will do it inside virtnet_reset() [1] [ 74.328168] general protection fault: [#1] SMP [ 74.328625] Modules linked in: nfsd xfs libcrc32c virtio_net virtio_pci [ 74.329117] CPU: 0 PID: 2849 Comm: xdp2 Not tainted 4.10.0-rc7+ #499 [ 74.329577] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.10.1-0-g8891697-prebuilt.qemu-project.org 04/01/2014 [ 74.330424] task: 88007a894000 task.stack: c90004388000 [ 74.330844] RIP: 0010:skb_release_head_state+0x28/0x80 [ 74.331298] RSP: 0018:c9000438b8d0 EFLAGS: 00010206 [ 74.331676] RAX: RBX: 88007ad96300 RCX: [ 74.332217] RDX: 88007fc137a8 RSI: 88007fc0db28 RDI: 0001bf0001be [ 74.332758] RBP: c9000438b8d8 R08: 0005008f R09: 05f9 [ 74.333274] R10: 88007d001700 R11: 820a8a4d R12: 88007ad96300 [ 74.333787] R13: 0002 R14: 880036604000 R15: 77ff8000 [ 74.334308] FS: 7fc70d8a7b40() GS:88007fc0() knlGS: [ 74.334891] CS: 0010 DS: ES: CR0: 80050033 [ 74.335314] CR2: 7fff4144a710 CR3: 7ab56000 CR4: 003406f0 [ 74.335830] DR0: DR1: DR2: [ 74.336373] DR3: DR6: fffe0ff0 DR7: 0400 [ 74.336895] Call Trace: [ 74.337086] skb_release_all+0xd/0x30 [ 74.337356] consume_skb+0x2c/0x90 [ 74.337607] free_unused_bufs+0x1ff/0x270 [virtio_net] [ 74.337988] ? vp_synchronize_vectors+0x3b/0x60 [virtio_pci] [ 74.338398] virtnet_xdp+0x21e/0x440 [virtio_net] [ 74.338741] dev_change_xdp_fd+0x101/0x140 [ 74.339048] do_setlink+0xcf4/0xd20 [ 74.339304] ? symcmp+0xf/0x20 [ 74.339529] ? mls_level_isvalid+0x52/0x60 [ 74.339828] ? mls_range_isvalid+0x43/0x50 [ 74.340135] ? nla_parse+0xa0/0x100 [ 74.340400] rtnl_setlink+0xd4/0x120 [ 74.340664] ? cpumask_next_and+0x30/0x50 [ 74.340966] rtnetlink_rcv_msg+0x7f/0x1f0 [ 74.341259] ? sock_has_perm+0x59/0x60 [ 74.341586] ? napi_consume_skb+0xe2/0x100 [ 74.342010] ? rtnl_newlink+0x890/0x890 [ 74.342435] netlink_rcv_skb+0x92/0xb0 [ 74.342846] rtnetlink_rcv+0x23/0x30 [ 74.343277] netlink_unicast+0x162/0x210 [ 74.343677] netlink_sendmsg+0x2db/0x390 [ 74.343968] sock_sendmsg+0x33/0x40 [ 74.344233] SYSC_sendto+0xee/0x160 [ 74.344482] ? SYSC_bind+0xb0/0xe0 [ 74.344806] ? sock_alloc_file+0x92/0x110 [ 74.345106] ? fd_install+0x20/0x30 [ 74.345360] ? sock_map_fd+0x3f/0x60 [ 74.345586] SyS_sendto+0x9/0x10 [ 74.345790] entry_SYSCALL_64_fastpath+0x1a/0xa9 [ 74.346086] RIP: 0033:0x7fc70d1b8f6d [ 74.346312] RSP: 002b:7fff4144a708 EFLAGS: 0246 ORIG_RAX: 002c [ 74.346785] RAX: ffda RBX: RCX: 7fc70d1b8f6d [ 74.347244] RDX: 002c RSI: 7fff4144a720 RDI: 0003 [ 74.347683] RBP: 0003 R08: R09: [ 74.348544] R10: R11: 0246 R12: 7fff4144bd90 [ 74.349082] R13: 0002 R14: 0002 R15: 7fff4144cda0 [ 74.349607] Code: 00 00 00 55 48 89 e5 53 48 89 fb 48 8b 7f 58 48 85 ff 74 0e 40 f6 c7 01 74 3d 48 c7 43 58 00 00 00 00 48 8b 7b 68 48 85 ff 74 05 ff 0f 74 20 48 8b 43 60 48 85 c0 74 14 65 8b 15 f3 ab 8d 7e [ 74.351008] RIP: skb_release_head_state+0x28/0x80 RSP: c9000438b8d0 [ 74.351625] ---[ end trace fe6e19fd11cfc80b ]--- Fixes: 2de2f7f40ef9 ("virtio_net: XDP support for adjust_head") Cc: John FastabendSigned-off-by: Jason Wang --- Changes from V1: - passing xdp_qp and curr_qp to virtnet_reset() to refill correctly - remove the duplicated queue pair setting after virtnet_reset() --- drivers/net/virtio_net.c | 28 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 11e2853..d12e5d6 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1737,7 +1737,7 @@ static int virtnet_restore_up(struct virtio_device *vdev) return err; } -static int virtnet_reset(struct virtnet_info *vi) +static int virtnet_reset(struct virtnet_info *vi, int curr_qp, int xdp_qp) { struct virtio_device *dev = vi->vdev; int ret; @@ -1755,10 +1755,11 @@ static int virtnet_reset(struct virtnet_info *vi) if (ret) goto err;
[PATCH net-next V2] virito-net: set queues after reset during xdp_set
We set queues before reset which will cause a crash[1]. This is because is_xdp_raw_buffer_queue() depends on the old xdp queue pairs number to do the correct detection. So fix this by - passing xdp queue pairs and current queue pairs to virtnet_reset() - change vi->xdp_qp after reset but before refill, to make sure both free_unused_bufs() and refill can make correct detection of XDP. - remove the duplicated queue pairs setting before virtnet_reset() since we will do it inside virtnet_reset() [1] [ 74.328168] general protection fault: [#1] SMP [ 74.328625] Modules linked in: nfsd xfs libcrc32c virtio_net virtio_pci [ 74.329117] CPU: 0 PID: 2849 Comm: xdp2 Not tainted 4.10.0-rc7+ #499 [ 74.329577] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.10.1-0-g8891697-prebuilt.qemu-project.org 04/01/2014 [ 74.330424] task: 88007a894000 task.stack: c90004388000 [ 74.330844] RIP: 0010:skb_release_head_state+0x28/0x80 [ 74.331298] RSP: 0018:c9000438b8d0 EFLAGS: 00010206 [ 74.331676] RAX: RBX: 88007ad96300 RCX: [ 74.332217] RDX: 88007fc137a8 RSI: 88007fc0db28 RDI: 0001bf0001be [ 74.332758] RBP: c9000438b8d8 R08: 0005008f R09: 05f9 [ 74.333274] R10: 88007d001700 R11: 820a8a4d R12: 88007ad96300 [ 74.333787] R13: 0002 R14: 880036604000 R15: 77ff8000 [ 74.334308] FS: 7fc70d8a7b40() GS:88007fc0() knlGS: [ 74.334891] CS: 0010 DS: ES: CR0: 80050033 [ 74.335314] CR2: 7fff4144a710 CR3: 7ab56000 CR4: 003406f0 [ 74.335830] DR0: DR1: DR2: [ 74.336373] DR3: DR6: fffe0ff0 DR7: 0400 [ 74.336895] Call Trace: [ 74.337086] skb_release_all+0xd/0x30 [ 74.337356] consume_skb+0x2c/0x90 [ 74.337607] free_unused_bufs+0x1ff/0x270 [virtio_net] [ 74.337988] ? vp_synchronize_vectors+0x3b/0x60 [virtio_pci] [ 74.338398] virtnet_xdp+0x21e/0x440 [virtio_net] [ 74.338741] dev_change_xdp_fd+0x101/0x140 [ 74.339048] do_setlink+0xcf4/0xd20 [ 74.339304] ? symcmp+0xf/0x20 [ 74.339529] ? mls_level_isvalid+0x52/0x60 [ 74.339828] ? mls_range_isvalid+0x43/0x50 [ 74.340135] ? nla_parse+0xa0/0x100 [ 74.340400] rtnl_setlink+0xd4/0x120 [ 74.340664] ? cpumask_next_and+0x30/0x50 [ 74.340966] rtnetlink_rcv_msg+0x7f/0x1f0 [ 74.341259] ? sock_has_perm+0x59/0x60 [ 74.341586] ? napi_consume_skb+0xe2/0x100 [ 74.342010] ? rtnl_newlink+0x890/0x890 [ 74.342435] netlink_rcv_skb+0x92/0xb0 [ 74.342846] rtnetlink_rcv+0x23/0x30 [ 74.343277] netlink_unicast+0x162/0x210 [ 74.343677] netlink_sendmsg+0x2db/0x390 [ 74.343968] sock_sendmsg+0x33/0x40 [ 74.344233] SYSC_sendto+0xee/0x160 [ 74.344482] ? SYSC_bind+0xb0/0xe0 [ 74.344806] ? sock_alloc_file+0x92/0x110 [ 74.345106] ? fd_install+0x20/0x30 [ 74.345360] ? sock_map_fd+0x3f/0x60 [ 74.345586] SyS_sendto+0x9/0x10 [ 74.345790] entry_SYSCALL_64_fastpath+0x1a/0xa9 [ 74.346086] RIP: 0033:0x7fc70d1b8f6d [ 74.346312] RSP: 002b:7fff4144a708 EFLAGS: 0246 ORIG_RAX: 002c [ 74.346785] RAX: ffda RBX: RCX: 7fc70d1b8f6d [ 74.347244] RDX: 002c RSI: 7fff4144a720 RDI: 0003 [ 74.347683] RBP: 0003 R08: R09: [ 74.348544] R10: R11: 0246 R12: 7fff4144bd90 [ 74.349082] R13: 0002 R14: 0002 R15: 7fff4144cda0 [ 74.349607] Code: 00 00 00 55 48 89 e5 53 48 89 fb 48 8b 7f 58 48 85 ff 74 0e 40 f6 c7 01 74 3d 48 c7 43 58 00 00 00 00 48 8b 7b 68 48 85 ff 74 05 ff 0f 74 20 48 8b 43 60 48 85 c0 74 14 65 8b 15 f3 ab 8d 7e [ 74.351008] RIP: skb_release_head_state+0x28/0x80 RSP: c9000438b8d0 [ 74.351625] ---[ end trace fe6e19fd11cfc80b ]--- Fixes: 2de2f7f40ef9 ("virtio_net: XDP support for adjust_head") Cc: John Fastabend Signed-off-by: Jason Wang --- Changes from V1: - passing xdp_qp and curr_qp to virtnet_reset() to refill correctly - remove the duplicated queue pair setting after virtnet_reset() --- drivers/net/virtio_net.c | 28 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 11e2853..d12e5d6 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1737,7 +1737,7 @@ static int virtnet_restore_up(struct virtio_device *vdev) return err; } -static int virtnet_reset(struct virtnet_info *vi) +static int virtnet_reset(struct virtnet_info *vi, int curr_qp, int xdp_qp) { struct virtio_device *dev = vi->vdev; int ret; @@ -1755,10 +1755,11 @@ static int virtnet_reset(struct virtnet_info *vi) if (ret) goto err; + vi->xdp_queue_pairs = xdp_qp;