[PATCH v2 8/9] mtd: fsl-quadspi: disable AHB buffer prefetch
From: Yunhui Cui A-009282: QuadSPI: QuadSPI data pre-fetch can result in incorrect data Affects: QuadSPI Description: With AHB buffer prefetch enabled, the QuadSPI may return incorrect data on the AHB interface. The buffer pre-fetch is enabled if the fetch size as configured either in the LUT or in the BUFxCR register is greater than 8 bytes. Impact: Only 64 bit read allowed. Workaround: Keep the read data size to 64 bits (8 Bytes), which disables the prefetch on the AHB buffer, and prevents this issue from occurring. Signed-off-by: Yunhui Cui --- drivers/mtd/spi-nor/fsl-quadspi.c | 33 - 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c index fea18b6..6a022e7 100644 --- a/drivers/mtd/spi-nor/fsl-quadspi.c +++ b/drivers/mtd/spi-nor/fsl-quadspi.c @@ -41,6 +41,8 @@ #define QUADSPI_QUIRK_TKT253890(1 << 2) /* Controller cannot wake up from wait mode, TKT245618 */ #define QUADSPI_QUIRK_TKT245618 (1 << 3) +/*Errata A-009282: disable the AHB buffer prefetch */ +#define QUADSPI_QUIRK_ADASZ_8BYTE_LIMIT (1 << 4) /* QSPI_AMBA_BASE is internally added by SOC design */ #define QUADSPI_AMBA_BASE_INTERNAL (0x1) @@ -270,7 +272,7 @@ static struct fsl_qspi_devtype_data ls1021a_data = { .rxfifo = 128, .txfifo = 64, .ahb_buf_size = 1024, - .driver_data = 0, + .driver_data = QUADSPI_QUIRK_ADASZ_8BYTE_LIMIT, }; static struct fsl_qspi_devtype_data ls2080a_data = { @@ -278,7 +280,8 @@ static struct fsl_qspi_devtype_data ls2080a_data = { .rxfifo = 128, .txfifo = 64, .ahb_buf_size = 1024, - .driver_data = QUADSPI_AMBA_BASE_INTERNAL, + .driver_data = QUADSPI_AMBA_BASE_INTERNAL + | QUADSPI_QUIRK_ADASZ_8BYTE_LIMIT, }; #define FSL_QSPI_MAX_CHIP 4 @@ -328,6 +331,11 @@ static inline int has_added_amba_base_internal(struct fsl_qspi *q) return q->devtype_data->driver_data & QUADSPI_AMBA_BASE_INTERNAL; } +static inline int needs_disable_ahb_prefetch(struct fsl_qspi *q) +{ + return q->devtype_data->driver_data & QUADSPI_QUIRK_ADASZ_8BYTE_LIMIT; +} + /* * R/W functions for big- or little-endian registers: * The qSPI controller's endian is independent of the CPU core's endian. @@ -757,14 +765,21 @@ static void fsl_qspi_init_abh_read(struct fsl_qspi *q) qspi_writel(q, QUADSPI_BUFXCR_INVALID_MSTRID, base + QUADSPI_BUF0CR); qspi_writel(q, QUADSPI_BUFXCR_INVALID_MSTRID, base + QUADSPI_BUF1CR); qspi_writel(q, QUADSPI_BUFXCR_INVALID_MSTRID, base + QUADSPI_BUF2CR); - /* -* Set ADATSZ with the maximum AHB buffer size to improve the -* read performance. -*/ - qspi_writel(q, QUADSPI_BUF3CR_ALLMST_MASK | - ((q->devtype_data->ahb_buf_size / 8) - << QUADSPI_BUF3CR_ADATSZ_SHIFT), + + if (needs_disable_ahb_prefetch(q)) { + qspi_writel(q, QUADSPI_BUF3CR_ALLMST_MASK | + (1 << QUADSPI_BUF3CR_ADATSZ_SHIFT), base + QUADSPI_BUF3CR); + } else { + /* +* Set ADATSZ with the maximum AHB buffer size to improve the +* read performance. + */ + qspi_writel(q, QUADSPI_BUF3CR_ALLMST_MASK | + ((q->devtype_data->ahb_buf_size / 8) + << QUADSPI_BUF3CR_ADATSZ_SHIFT), + base + QUADSPI_BUF3CR); + } /* We only use the buffer3 */ qspi_writel(q, 0, base + QUADSPI_BUF0IND); -- 2.1.0.27.g96db324
[PATCH v2 5/9] mtd: spi-nor: fsl-quadspi:Support qspi for ls2080a
There is a hardware feature that qspi_amba_base is added internally by SOC design on ls2080a. So as to software, the driver need support to the feature. Signed-off-by: Yunhui Cui Signed-off-by: Yunhui Cui --- drivers/mtd/spi-nor/fsl-quadspi.c | 24 ++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c index 16ebabbd..5d9d192 100644 --- a/drivers/mtd/spi-nor/fsl-quadspi.c +++ b/drivers/mtd/spi-nor/fsl-quadspi.c @@ -41,6 +41,8 @@ #define QUADSPI_QUIRK_TKT253890(1 << 2) /* Controller cannot wake up from wait mode, TKT245618 */ #define QUADSPI_QUIRK_TKT245618 (1 << 3) +/* QSPI_AMBA_BASE is internally added by SOC design */ +#define QUADSPI_AMBA_BASE_INTERNAL (0x1) /* The registers */ #define QUADSPI_MCR0x00 @@ -217,6 +219,7 @@ enum fsl_qspi_devtype { FSL_QUADSPI_IMX7D, FSL_QUADSPI_IMX6UL, FSL_QUADSPI_LS1021A, + FSL_QUADSPI_LS2080A, }; struct fsl_qspi_devtype_data { @@ -270,6 +273,14 @@ static struct fsl_qspi_devtype_data ls1021a_data = { .driver_data = 0, }; +static struct fsl_qspi_devtype_data ls2080a_data = { + .devtype = FSL_QUADSPI_LS2080A, + .rxfifo = 128, + .txfifo = 64, + .ahb_buf_size = 1024, + .driver_data = QUADSPI_AMBA_BASE_INTERNAL, +}; + #define FSL_QSPI_MAX_CHIP 4 struct fsl_qspi { struct spi_nor nor[FSL_QSPI_MAX_CHIP]; @@ -312,6 +323,11 @@ static inline int needs_wakeup_wait_mode(struct fsl_qspi *q) return q->devtype_data->driver_data & QUADSPI_QUIRK_TKT245618; } +static inline int has_added_amba_base_internal(struct fsl_qspi *q) +{ + return q->devtype_data->driver_data & QUADSPI_AMBA_BASE_INTERNAL; +} + /* * R/W functions for big- or little-endian registers: * The qSPI controller's endian is independent of the CPU core's endian. @@ -558,8 +574,11 @@ fsl_qspi_runcmd(struct fsl_qspi *q, u8 cmd, unsigned int addr, int len) /* save the reg */ reg = qspi_readl(q, base + QUADSPI_MCR); - qspi_writel(q, q->memmap_phy + q->chip_base_addr + addr, - base + QUADSPI_SFAR); + if (has_added_amba_base_internal(q)) + qspi_writel(q, q->chip_base_addr + addr, base + QUADSPI_SFAR); + else + qspi_writel(q, q->memmap_phy + q->chip_base_addr + addr, + base + QUADSPI_SFAR); qspi_writel(q, QUADSPI_RBCT_WMRK_MASK | QUADSPI_RBCT_RXBRD_USEIPS, base + QUADSPI_RBCT); qspi_writel(q, reg | QUADSPI_MCR_CLR_RXF_MASK, base + QUADSPI_MCR); @@ -849,6 +868,7 @@ static const struct of_device_id fsl_qspi_dt_ids[] = { { .compatible = "fsl,imx7d-qspi", .data = (void *)&imx7d_data, }, { .compatible = "fsl,imx6ul-qspi", .data = (void *)&imx6ul_data, }, { .compatible = "fsl,ls1021a-qspi", .data = (void *)&ls1021a_data, }, + { .compatible = "fsl,ls2080a-qspi", .data = (void *)&ls2080a_data, }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, fsl_qspi_dt_ids); -- 2.1.0.27.g96db324
[PATCH v2 4/9] mtd: spi-nor: fsl-quadspi: extend support for some special requerment.
From: Yunhui Cui Add extra info in LUT table to support some special requerments. Spansion S25FS-S family flash need some special operations. Signed-off-by: Yunhui Cui --- drivers/mtd/spi-nor/fsl-quadspi.c | 44 +-- include/linux/mtd/spi-nor.h | 4 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c index 09adaa4..16ebabbd 100644 --- a/drivers/mtd/spi-nor/fsl-quadspi.c +++ b/drivers/mtd/spi-nor/fsl-quadspi.c @@ -205,6 +205,9 @@ #define SEQID_RDCR 9 #define SEQID_EN4B 10 #define SEQID_BRWR 11 +#define SEQID_RDAR 12 +#define SEQID_WRAR 13 + #define QUADSPI_MIN_IOMAP SZ_4M @@ -470,6 +473,28 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q) qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_BRWR), base + QUADSPI_LUT(lut_base)); + /* +* Read any device register. +* Used for Spansion S25FS-S family flash only. +*/ + lut_base = SEQID_RDAR * 4; + qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_SPANSION_RDAR) | + LUT1(ADDR, PAD1, ADDR24BIT), + base + QUADSPI_LUT(lut_base)); + qspi_writel(q, LUT0(DUMMY, PAD1, 8) | LUT1(FSL_READ, PAD1, 1), + base + QUADSPI_LUT(lut_base + 1)); + + /* +* Write any device register. +* Used for Spansion S25FS-S family flash only. +*/ + lut_base = SEQID_WRAR * 4; + qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_SPANSION_WRAR) | + LUT1(ADDR, PAD1, ADDR24BIT), + base + QUADSPI_LUT(lut_base)); + qspi_writel(q, LUT0(FSL_WRITE, PAD1, 1), + base + QUADSPI_LUT(lut_base + 1)); + fsl_qspi_lock_lut(q); } @@ -477,9 +502,15 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q) static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd) { switch (cmd) { + case SPINOR_OP_READ4_1_1_4: case SPINOR_OP_READ_1_1_4: case SPINOR_OP_READ_FAST: + case SPINOR_OP_READ4_FAST: return SEQID_READ; + case SPINOR_OP_SPANSION_RDAR: + return SEQID_RDAR; + case SPINOR_OP_SPANSION_WRAR: + return SEQID_WRAR; case SPINOR_OP_WREN: return SEQID_WREN; case SPINOR_OP_WRDI: @@ -491,6 +522,7 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd) case SPINOR_OP_CHIP_ERASE: return SEQID_CHIP_ERASE; case SPINOR_OP_PP: + case SPINOR_OP_PP_4B: return SEQID_PP; case SPINOR_OP_RDID: return SEQID_RDID; @@ -830,8 +862,12 @@ static int fsl_qspi_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len) { int ret; struct fsl_qspi *q = nor->priv; + u32 to = 0; + + if (opcode == SPINOR_OP_SPANSION_RDAR) + memcpy(&to, nor->cmd_buf, 4); - ret = fsl_qspi_runcmd(q, opcode, 0, len); + ret = fsl_qspi_runcmd(q, opcode, to, len); if (ret) return ret; @@ -843,9 +879,13 @@ static int fsl_qspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len) { struct fsl_qspi *q = nor->priv; int ret; + u32 to = 0; + + if (opcode == SPINOR_OP_SPANSION_WRAR) + memcpy(&to, nor->cmd_buf, 4); if (!buf) { - ret = fsl_qspi_runcmd(q, opcode, 0, 1); + ret = fsl_qspi_runcmd(q, opcode, to, 1); if (ret) return ret; diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 3c36113..fd0631b 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -74,6 +74,10 @@ /* Used for Spansion flashes only. */ #define SPINOR_OP_BRWR 0x17/* Bank register write */ +/* Used for Spansion S25FS-S family flash only. */ +#define SPINOR_OP_SPANSION_RDAR0x65/* Read any device register */ +#define SPINOR_OP_SPANSION_WRAR0x71/* Write any device register */ + /* Used for Micron flashes only. */ #define SPINOR_OP_RD_EVCR 0x65/* Read EVCR register */ #define SPINOR_OP_WD_EVCR 0x61/* Write EVCR register */ -- 2.1.0.27.g96db324
Re: Nokia N900 retention mode in v4.6, camera buttons fun
On Thursday 21 April 2016 23:28:08 Pavel Machek wrote: > Hi! > > > > CONFIG_HSI breaks power management completely, so power management > > > with modem will be another topic. > > > > Sebastian, any idea why power management does not work for HSI? > > > > > > > > In the meantime, I found what is causing the rention mode to break > > > > > > for > > > > > > me: CONFIG_HSI (aka wireless modem support). With HSI off, it seems > > > > > > to work. > > > > > > > > > > > > I still get problems with the camera button, in config similar to > > > > > > defconfig. For some reason, I'm even getting (autorepeating) ^@ on > > > > > > console. As long as I hold camera button down, I even get it into > > > > > > off > > > > > > mode for brief period. > > > > > > > > > > Ok, if I turn off CONFIG_KEYBOARD_GPIO, I get it into off > > > > > mode... once per screen blank, for about a second. (Does > > > > > CONFIG_KEYBOARD_GPIO also cause > > > > > problems for you?) > > > > > > > > > > Any idea why it enters off mode only once after each screenblank? > > > > > > > > After disabling CONFIG_PROVE_LOCKING, loading the LCD modules, and > > > > blanking the screen, my n900 hits off mode just fine about once > > > > a second. Sounds like you still have some extra devices enabled > > > > causing it. > > > > > > I checked again... also with vanilla 4.6-rc2 to double check... same > > > effect. > > > > > > Aha, got it... cat-ing /sys/kernel/debug/pm_debug/count breaks the > > > off mode. If I don't do that (tm), it seems to work way better. > > > > So what is result? Is power management working for CONFIG_KEYBOARD_GPIO? > > camera and unlock button GPIOs seem to break the powermanagement, > too. I disabled it for now. So this sounds like a problem in gpio keyboard driver. Maybe you can ping maintainers of that driver? > Next hint I got from Sebastian was that I may need to enable power > management in /sys. > > pavel@n900:/my/tui/ofone$ cat > /sys/devices/platform/6800.ocp/48058000.ssi-controller/power/runtime_status > active > pavel@n900:/my/tui/ofone$ cat > /sys/devices/platform/6800.ocp/48058000.ssi-controller/power/autosuspend_delay_ms > cat: > /sys/devices/platform/6800.ocp/48058000.ssi-controller/power/autosuspend_delay_ms: > Input/output error > root@n900:/my/tui/ofone# cat > /sys/devices/platform/6800.ocp/48058000.ssi-controller/power/control > auto > > I could not get it to sleep :-(. > > Best regards, > Pavel > Maybe down phonet0 interface and other hsi/ssi interfaces? -- Pali Rohár pali.ro...@gmail.com
RE: [PATCH 8/9] mtd: fsl-quadspi: disable AHB buffer prefetch
> -Original Message- > From: Han Xu > Sent: Friday, April 22, 2016 12:49 PM > To: Yunhui Cui; Yunhui Cui; dw...@infradead.org; > computersforpe...@gmail.com; han...@freescale.com > Cc: linux-kernel@vger.kernel.org; linux-...@lists.infradead.org; linux- > arm-ker...@lists.infradead.org; Yao Yuan > Subject: Re: [PATCH 8/9] mtd: fsl-quadspi: disable AHB buffer prefetch > > > > > From: Yunhui Cui > Sent: Thursday, April 21, 2016 9:52 PM > To: Han Xu; Yunhui Cui; dw...@infradead.org; computersforpe...@gmail.com; > han...@freescale.com > Cc: linux-kernel@vger.kernel.org; linux-...@lists.infradead.org; linux- > arm-ker...@lists.infradead.org; Yao Yuan > Subject: RE: [PATCH 8/9] mtd: fsl-quadspi: disable AHB buffer prefetch > > > -Original Message- > > From: Han Xu > > Sent: Thursday, April 21, 2016 11:48 PM > > To: Yunhui Cui; Yunhui Cui; dw...@infradead.org; > > computersforpe...@gmail.com; han...@freescale.com > > Cc: linux-kernel@vger.kernel.org; linux-...@lists.infradead.org; > > linux- arm-ker...@lists.infradead.org; Yao Yuan > > Subject: Re: [PATCH 8/9] mtd: fsl-quadspi: disable AHB buffer prefetch > > > > > > > > > > From: Yunhui Cui > > Sent: Thursday, April 21, 2016 3:41 AM > > To: Han Xu; Yunhui Cui; dw...@infradead.org; > > computersforpe...@gmail.com; han...@freescale.com > > Cc: linux-kernel@vger.kernel.org; linux-...@lists.infradead.org; > > linux- arm-ker...@lists.infradead.org; Yao Yuan > > Subject: RE: [PATCH 8/9] mtd: fsl-quadspi: disable AHB buffer prefetch > > > > On Thu, Apr 24, 2016 at 06:37:01 AM +0800, Han Xu wrote: > > > > > > From: Yunhui Cui > > > Sent: Wednesday, April 13, 2016 9:50 PM > > > To: dw...@infradead.org; computersforpe...@gmail.com; > > > han...@freescale.com > > > Cc: linux-kernel@vger.kernel.org; linux-...@lists.infradead.org; > > > linux- arm-ker...@lists.infradead.org; Yao Yuan; Yunhui Cui > > > Subject: [PATCH 8/9] mtd: fsl-quadspi: disable AHB buffer prefetch > > > > > > From: Yunhui Cui > > > > > > A-009282: QuadSPI: QuadSPI data pre-fetch can result in incorrect > > > data > > > Affects: QuadSPI > > > Description: With AHB buffer prefetch enabled, the QuadSPI may > > > return incorrect data on the AHB interface. The buffer pre-fetch is > > > enabled if the fetch size as configured either in the LUT or in the > > > BUFxCR register is greater than 8 bytes. > > > Impact: Only 64 bit read allowed. > > > Workaround: Keep the read data size to 64 bits (8 Bytes), which > > > disables the prefetch on the AHB buffer, and prevents this issue > > > from > > occurring. > > > > > > Signed-off-by: Yunhui Cui > > > --- > > > drivers/mtd/spi-nor/fsl-quadspi.c | 29 > > > +++-- > > > 1 file changed, 23 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c > > > b/drivers/mtd/spi-nor/fsl- quadspi.c index fea18b6..d9f3e50 100644 > > > --- a/drivers/mtd/spi-nor/fsl-quadspi.c > > > +++ b/drivers/mtd/spi-nor/fsl-quadspi.c > > > @@ -752,19 +752,36 @@ static void fsl_qspi_init_abh_read(struct > > > fsl_qspi > > > *q) { > > > void __iomem *base = q->iobase; > > > int seqid; > > > + const struct fsl_qspi_devtype_data *devtype_data = > > > + q->devtype_data; > > > > > > /* AHB configuration for access buffer 0/1/2 .*/ > > > qspi_writel(q, QUADSPI_BUFXCR_INVALID_MSTRID, base + > > > QUADSPI_BUF0CR); > > > qspi_writel(q, QUADSPI_BUFXCR_INVALID_MSTRID, base + > > > QUADSPI_BUF1CR); > > > qspi_writel(q, QUADSPI_BUFXCR_INVALID_MSTRID, base + > > > QUADSPI_BUF2CR); > > > + > > > /* > > > -* Set ADATSZ with the maximum AHB buffer size to improve the > > > -* read performance. > > > +* Errata: A-009282: QuadSPI data prefetch may result in > > > incorrect data > > > +* Workaround: Keep the read data size to 64 bits (8 bytes). > > > +* This disables the prefetch on the AHB buffer and > > > +* prevents this issue from occurring. > > > */ > > > - qspi_writel(q, QUADSPI_BUF3CR_ALLMST_MASK | > > > - ((q->devtype_data->ahb_buf_size / 8) > > > - << QUADSPI_BUF3CR_ADATSZ_SHIFT), > > > - base + QUADSPI_BUF3CR); > > > + if (devtype_data->devtype == FSL_QUADSPI_LS2080A || > > > + devtype_data->devtype == FSL_QUADSPI_LS1021A) { > > > > > > Use QUIRK, not SoC name. > > [Yunhui] This is a SoC errata, we need distinguish according to the > > devtype_data->devtype > > > > It's the same thing and we have already done that before. > > > [Yunhui] your mean that I should add such as the following code: > #define QUADSPI_QUIRK_ERRATA_9282 (1 << 4) > static inline int has_errata_num_a009282(struct fsl_qspi *q) > { > return q->devtype_data->driver_data & > QUADSPI_QU
Re: [PATCH v2] PM / Runtime: Only force-resume device if it has been force-suspended
On 21 April 2016 at 23:07, Laurent Pinchart wrote: > Hi Rafael, > > On Thursday 21 Apr 2016 23:02:06 Rafael J. Wysocki wrote: >> On Thu, Apr 21, 2016 at 10:57 PM, Laurent Pinchart wrote: >> > On Thursday 21 Apr 2016 21:52:56 Rafael J. Wysocki wrote: >> >> On Thursday, April 21, 2016 02:52:55 AM Laurent Pinchart wrote: >> >>> The pm_runtime_force_suspend() and pm_runtime_force_resume() helpers >> >>> are designed to help driver being RPM-centric by offering an easy way to >> >>> manage runtime PM state during system suspend and resume. The first >> >>> function will force the device into runtime suspend at system suspend >> >>> time, while the second one will perform the reverse operation at system >> >>> resume time. >> >>> >> >>> However, the pm_runtime_force_resume() really forces resume, regardless >> >>> of whether the device was running or already suspended before the call >> >>> to pm_runtime_force_suspend(). This results in devices being runtime >> >>> resumed at system resume time when they shouldn't. >> >>> >> >>> Fix this by recording whether the device has been forcefully suspended >> >>> in pm_runtime_force_suspend() and condition resume in >> >>> pm_runtime_force_resume() to that state. >> >>> >> >>> All current users of pm_runtime_force_resume() call the function >> >>> unconditionally in their system resume handler (some actually set it as >> >>> the resume handler), all after calling pm_runtime_force_suspend() at >> >>> system suspend time. The change in behaviour should thus be safe. >> >>> >> >>> Signed-off-by: Laurent Pinchart >> >>> >> >>> Reviewed-by: Kevin Hilman >> >> >> >> Ulf, any comments? >> > >> > Ulf has proposed a different approach in "[PATCH] PM / Runtime: Defer >> > resuming of the device in pm_runtime_force_resume()". I agree that using >> > usage_count is better than introducing a new state flag in struct >> > dev_pm_info, with a caveat: it doesn't work properly :-). We would have >> > to fix genpd first, as commented in a reply to Ulf's patch. >> >> OK, thanks! >> >> Since I'd prefer to avoid adding more state flags too, I'll let you >> guys noodle around this for a while more. :-) > > Let's see what we can do in a reasonable time frame. We could decide to merge > this patch as a temporary fix until the genpd rework is complete. Subsystems/driver that uses pm_runtime_force_suspend|resume() don't necessarily need to have their devices attached to a genpd. In such cases, $subject patch will be an improvement by itself. Me personally would rather skip the intermediate step you propose, as I prefer to properly change genpd with what is needed. Moreover, I am already working on that so it shouldn't take too long before I can post some patches. Kind regards Uffe
Re: [PATCH 2/5] x86, KASLR: Drop CONFIG_RANDOMIZE_BASE_MAX_OFFSET
* Kees Cook wrote: > >> + Since the kernel is built using 2GB addressing, > > > > Does that try to refer to the 1G kernel and 1G fixmap pagetable > > mappings? I.e., level2_kernel_pgt and level2_fixmap_pgt in > > arch/x86/kernel/head_64.S? > > The "2GB addressing" part is in reference to: > >-mcmodel=kernel >Generate code for the kernel code model. The kernel runs in the >negative 2 GB of the address space. This model has to be used for >Linux kernel code. On x86-64 this is a special GCC compiler small memory model, it is called the 'kernel code model', which is rather generic and no 'real name' ever stuck. Due to RIP-relative addressing and the sign-extension of 48 bit virtual addresses, this allows nearly as compact kernel code and (static) kernel data definitions as a 32-bit kernel would allow. The (positive) 0-4GB virtual memory range has similar advantages, but is of course frequently used by user-space code. Negative addresses are reserved for the kernel only. Thanks, Ingo
Integer overflow in target_core_device.c
Linux kernel commit 8a9ebe717a133ba7bc90b06047f43cc6b8bcb8b3 attrib->max_unmap_lba_count = (q->limits.max_discard_sectors << 9) Since max_discard_sectors is 32-bit, there may be integer overflow, making wrong max_unmap_lba_count. For example, LVM Thin provisioning reports that it have 16 GB maximal discard block. Exactly the same bug in DRBD9, I have already reported. -- Segmentation fault
Re: [PATCH 7/8] gpio: stmpe: Add STMPE1600 support
On 04/20/2016 04:53 PM, Linus Walleij wrote: On Tue, Apr 19, 2016 at 2:18 PM, wrote: From: Patrice Chotard The particularities of this variant are: - GPIO_XXX_LSB and GPIO_XXX_MSB memory locations are inverted compared to other variants. - There is no Edge detection, Rising Edge and Falling Edge registers. - IRQ flags are cleared when read, no need to write in Status register. Signed-off-by: Amelie DELAUNAY Signed-off-by: Patrice Chotard - u8 reg = stmpe->regs[STMPE_IDX_GPMR_LSB] - (offset / 8); + u8 reg; u8 mask = 1 << (offset % 8); int ret; + if (stmpe->partnum == STMPE1600) + reg = stmpe->regs[STMPE_IDX_GPMR_LSB] + (offset / 8); + else + reg = stmpe->regs[STMPE_IDX_GPMR_LSB] - (offset / 8); This construct is a bit hard to grasp. Can we think of something more intuitive? Maybe using more code lines but easier to understand. Subtracting the offset is just totally unintuitive in the first place, the STMPE1600 arrangement is much more intuitive. I would prefer if we address the LSB+MSB register explicitly instead of adding or subtracting 1 to the LSB register to get to the MSB register. + if (stmpe->partnum == STMPE1600) + reg = stmpe->regs[which] + (offset / 8); + else + reg = stmpe->regs[which] - (offset / 8); Same. + if (stmpe->partnum == STMPE1600) + reg = stmpe->regs[STMPE_IDX_GPDR_LSB] + (offset / 8); + else + reg = stmpe->regs[STMPE_IDX_GPDR_LSB] - (offset / 8); Same. + if (stmpe->partnum == STMPE1600) + reg = stmpe->regs[STMPE_IDX_GPDR_LSB] + (offset / 8); + else + reg = stmpe->regs[STMPE_IDX_GPDR_LSB] - (offset / 8); Same. + stmpe_reg_write(stmpe, + stmpe->regs[regmap[i]] + j, + new); + else + stmpe_reg_write(stmpe, + stmpe->regs[regmap[i]] - j, + new); This is also unintuitively backwards. + if (stmpe->partnum == STMPE1600) + dir_reg = stmpe->regs[STMPE_IDX_GPDR_LSB] + (offset / 8); + else + dir_reg = stmpe->regs[STMPE_IDX_GPDR_LSB] - (offset / 8); Same. + if (stmpe->partnum == STMPE1600) + statmsbreg = stmpe->regs[STMPE_IDX_ISGPIOR_LSB]; + else + statmsbreg = stmpe->regs[STMPE_IDX_ISGPIOR_MSB]; And this kind of points at the problem. Can we write this in some way that make it super-clear which register we're using and why? Ok i will rework all these points Thanks Patrice Yours, Linus Walleij
Re: [PATCHv1 0/6] leds: pca9653x: support inverted outputs and cleanups
Hi Ricardo, On 20-04-16 11:17, Ricardo Ribalda Delgado wrote: Hello again On Wed, Apr 20, 2016 at 11:06 AM, Olliver Schinagl wrote: The devil is in the details :) :) Saving mode2 sounds like a good compromise then. But I still believe that we should limit the lock to ledout. No matter what we do, we cannot have two leds blinking at different frequencies on the same chip. So to save a mutex a little bit, we take the risk that nobody else enables the blink or if they do, enable it in the same way? If it saves so much, then I guess its worth the risk I suppose? Give me a day to go through the chip doc and see if I can find a good compromise, that at least warranties that the leds that are enable stay enabled ;) Right, I also went over the datasheet, and I think we can simplyfy two things. For one, yes, move the mode2 register completly to the probe section. Set the DMBLINK led to always 1. It does not get cleared, I was wrong. We have to set it to as with 0 we do not get any blinking at all (grpfreq gets ignored). Furthermore, we should change: -gdc = (time_on * 256) / period; + gdc = 0x00; Because the calculation does not make sense. GDC is the global brightness/pwm/dimming control. It is used to uniformly change the blink rate on all the linked leds. "General brightness for the 16 outputs is controlled through 256 linear steps to FFh" I don't think that is the intention of the gdc is it? Looking at the original gdc code, it thus sets the global BRIGHTNESS based on the period/on_time. I don't think that is what we expect when we enable blink. From my understanding, the grppwm is super-imposed, thus by setting gdc to 0, we do not superimpose anything and the original brightness is retained. (If i'm wrong here, we need to set gdc to 0xff. Because of this, I even recommend removing gdc all together, which saves another i2c write. Or am I wrong here? Olliver Regards!
[PATCH v2 9/9] mtd: fsl-quadspi: add multi flash chip R/W on ls2080a
From: Yunhui Cui There is a hardware feature that qspi_amba_base is added internally by SOC design on ls2080a. so memmap_phy need not be added in driver. If memmap_phy is added, the flash A1 addr space is [0, memmap_phy] which far more than flash size. The AMBA memory will be divided into four parts and assign to every chipselect. Every channel will has two valid chipselects. Signed-off-by: Yunhui Cui --- drivers/mtd/spi-nor/fsl-quadspi.c | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c index 6a022e7..b415663 100644 --- a/drivers/mtd/spi-nor/fsl-quadspi.c +++ b/drivers/mtd/spi-nor/fsl-quadspi.c @@ -736,11 +736,17 @@ static void fsl_qspi_set_map_addr(struct fsl_qspi *q) { int nor_size = q->nor_size; void __iomem *base = q->iobase; + u32 mem_base; - qspi_writel(q, nor_size + q->memmap_phy, base + QUADSPI_SFA1AD); - qspi_writel(q, nor_size * 2 + q->memmap_phy, base + QUADSPI_SFA2AD); - qspi_writel(q, nor_size * 3 + q->memmap_phy, base + QUADSPI_SFB1AD); - qspi_writel(q, nor_size * 4 + q->memmap_phy, base + QUADSPI_SFB2AD); + if (has_added_amba_base_internal(q)) + mem_base = 0x0; + else + mem_base = q->memmap_phy; + + qspi_writel(q, nor_size + mem_base, base + QUADSPI_SFA1AD); + qspi_writel(q, nor_size * 2 + mem_base, base + QUADSPI_SFA2AD); + qspi_writel(q, nor_size * 3 + mem_base, base + QUADSPI_SFB1AD); + qspi_writel(q, nor_size * 4 + mem_base, base + QUADSPI_SFB2AD); } /* -- 2.1.0.27.g96db324
Re: [PATCH V3 04/11] CRIS v32: nand: set ECC algorithm explicitly
On Sun, Apr 17, 2016 at 10:53:00PM +0200, Rafał Miłecki wrote: > This is part of process deprecating NAND_ECC_SOFT_BCH (and switching to > enum nand_ecc_algo). > > Signed-off-by: Rafał Miłecki Acked-by: Jesper Nilsson /^JN - Jesper Nilsson -- Jesper Nilsson -- jesper.nils...@axis.com
[PULL] clockevents fixes for 4.6
Hi Thomas, Ingo, Please pull this fix for the tango xtal driver. * Fix the boot hang on Sigma Design Tango4 by inverting the error check condition at init time (Daniel Lezcano) The following changes since commit b7c8b4aac6ea6746b1c49fda0a0563a07203dd26: clocksource/drivers/pistachio: Correct output format of PTR_ERR() (2016-03-23 12:30:40 +0100) are available in the git repository at: http://git.linaro.org/people/daniel.lezcano/linux.git clockevents/4.6-fixes for you to fetch changes up to 16eeed7e5558a3dcf30f75526a896b2632f299f9: clocksource/drivers/tango-xtal: Fix boot hang due to incorrect test (2016-04-22 09:22:37 +0200) Daniel Lezcano (1): clocksource/drivers/tango-xtal: Fix boot hang due to incorrect test drivers/clocksource/tango_xtal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] clocksource/drivers/tango-xtal: Fix boot hang due to incorrect test
Commit 0881841f7e78 introduced a regression by inverting a test check after calling clocksource_mmio_init(). That results on the system to hang at boot time. Fix it by inverting the test again. Fixes: 0881841f7e78 ("Replace code by clocksource_mmio_init") Reported-by: Marc Gonzalez Signed-off-by: Daniel Lezcano --- drivers/clocksource/tango_xtal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clocksource/tango_xtal.c b/drivers/clocksource/tango_xtal.c index 2bcecaf..c407c47 100644 --- a/drivers/clocksource/tango_xtal.c +++ b/drivers/clocksource/tango_xtal.c @@ -42,7 +42,7 @@ static void __init tango_clocksource_init(struct device_node *np) ret = clocksource_mmio_init(xtal_in_cnt, "tango-xtal", xtal_freq, 350, 32, clocksource_mmio_readl_up); - if (!ret) { + if (ret) { pr_err("%s: registration failed\n", np->full_name); return; } -- 1.9.1
Re: [PATCH] kvm: x86: do not leak guest xcr0 into host interrupt handlers
Hi Paolo and David, 2016-03-31 3:24 GMT+08:00 David Matlack : > An interrupt handler that uses the fpu can kill a KVM VM, if it runs > under the following conditions: > - the guest's xcr0 register is loaded on the cpu > - the guest's fpu context is not loaded > - the host is using eagerfpu > > Note that the guest's xcr0 register and fpu context are not loaded as > part of the atomic world switch into "guest mode". They are loaded by > KVM while the cpu is still in "host mode". > > Usage of the fpu in interrupt context is gated by irq_fpu_usable(). The > interrupt handler will look something like this: > > if (irq_fpu_usable()) { > kernel_fpu_begin(); > > [... code that uses the fpu ...] > > kernel_fpu_end(); > } > > As long as the guest's fpu is not loaded and the host is using eager > fpu, irq_fpu_usable() returns true (interrupted_kernel_fpu_idle() > returns true). The interrupt handler proceeds to use the fpu with > the guest's xcr0 live. > > kernel_fpu_begin() saves the current fpu context. If this uses > XSAVE[OPT], it may leave the xsave area in an undesirable state. > According to the SDM, during XSAVE bit i of XSTATE_BV is not modified > if bit i is 0 in xcr0. So it's possible that XSTATE_BV[i] == 1 and > xcr0[i] == 0 following an XSAVE. How XSAVE save bit i since SDM mentioned that "XSAVE saves state component i if and only if RFBM[i] = 1. "? RFBM[i] will be 0 if XSTATE_BV[i] == 1 && guest xcr0[i] == 0. Regards, Wanpeng Li > > kernel_fpu_end() restores the fpu context. Now if any bit i in > XSTATE_BV == 1 while xcr0[i] == 0, XRSTOR generates a #GP. The > fault is trapped and SIGSEGV is delivered to the current process. > > Only pre-4.2 kernels appear to be vulnerable to this sequence of > events. Commit 653f52c ("kvm,x86: load guest FPU context more eagerly") > from 4.2 forces the guest's fpu to always be loaded on eagerfpu hosts. > > This patch fixes the bug by keeping the host's xcr0 loaded outside > of the interrupts-disabled region where KVM switches into guest mode. > > Cc: sta...@vger.kernel.org > Suggested-by: Andy Lutomirski > Signed-off-by: David Matlack > --- > arch/x86/kvm/x86.c | 10 -- > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index e260ccb..8df1167 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -700,7 +700,6 @@ static int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 > index, u64 xcr) > if ((xcr0 & XFEATURE_MASK_AVX512) != XFEATURE_MASK_AVX512) > return 1; > } > - kvm_put_guest_xcr0(vcpu); > vcpu->arch.xcr0 = xcr0; > > if ((xcr0 ^ old_xcr0) & XFEATURE_MASK_EXTEND) > @@ -6590,8 +6589,6 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) > kvm_x86_ops->prepare_guest_switch(vcpu); > if (vcpu->fpu_active) > kvm_load_guest_fpu(vcpu); > - kvm_load_guest_xcr0(vcpu); > - > vcpu->mode = IN_GUEST_MODE; > > srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx); > @@ -6607,6 +6604,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) > > local_irq_disable(); > > + kvm_load_guest_xcr0(vcpu); > + > if (vcpu->mode == EXITING_GUEST_MODE || vcpu->requests > || need_resched() || signal_pending(current)) { > vcpu->mode = OUTSIDE_GUEST_MODE; > @@ -6667,6 +,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) > vcpu->mode = OUTSIDE_GUEST_MODE; > smp_wmb(); > > + kvm_put_guest_xcr0(vcpu); > + > /* Interrupt is enabled by handle_external_intr() */ > kvm_x86_ops->handle_external_intr(vcpu); > > @@ -7314,7 +7315,6 @@ void kvm_load_guest_fpu(struct kvm_vcpu *vcpu) > * and assume host would use all available bits. > * Guest xcr0 would be loaded later. > */ > - kvm_put_guest_xcr0(vcpu); > vcpu->guest_fpu_loaded = 1; > __kernel_fpu_begin(); > __copy_kernel_to_fpregs(&vcpu->arch.guest_fpu.state); > @@ -7323,8 +7323,6 @@ void kvm_load_guest_fpu(struct kvm_vcpu *vcpu) > > void kvm_put_guest_fpu(struct kvm_vcpu *vcpu) > { > - kvm_put_guest_xcr0(vcpu); > - > if (!vcpu->guest_fpu_loaded) { > vcpu->fpu_counter = 0; > return; > -- > 2.8.0.rc3.226.g39d4020 > -- Regards, Wanpeng Li
[PATCH] qla1280: Reduce can_queue to 32
It was reported in https://bugzilla.redhat.com/show_bug.cgi?id=1321033, that the qla1280 driver sets the scsi_host_template's can_queue field to 0xf which results in an allocation failure when allocating the block layer tags for the driver's queues like the one shown below: [4.804166] scsi host0: QLogic QLA1040 PCI to SCSI Host Adapter Firmware version: 7.65.06, Driver version 3.27.1 [4.804174] [ cut here ] [4.804184] WARNING: CPU: 2 PID: 305 at mm/page_alloc.c:2989 alloc_pages_nodemask+0xae8/0xbc0() [4.804186] Modules linked in: amdkfd amd_iommu_v2 radeon i2c_algo_bit m_kms_helper ttm drm megaraid_sas serio_raw 8021q garp bnx2 stp llc mrp nhme qla1280(+) fjes [4.804208] CPU: 2 PID: 305 Comm: systemd-udevd Not tainted 4.6-201.fc22.x86_64 #1 [4.804210] Hardware name: Google Enterprise Search Appliance/0DT021, OS 1.1.2 08/14/2006 [4.804212] 0286 2f01064c 88042985b710 ff813b542e [4.804216] 81a75024 88042985b748 ff810a40f2 [4.804220] 000b 00 [4.804223] Call Trace: [4.804231] [] dump_stack+0x63/0x85 [4.804236] [] warn_slowpath_common+0x82/0xc0 [4.804239] [] warn_slowpath_null+0x1a/0x20 [4.804242] [] __alloc_pages_nodemask+0xae8/0xbc0 [4.804247] [] ? _raw_spin_unlock_irqrestore+0xe/0x10 [4.804251] [] ? irq_work_queue+0x8e/0xa0 [4.804256] [] ? console_unlock+0x20a/0x540 [4.804262] [] alloc_pages_current+0x8c/0x110 [4.804265] [] alloc_kmem_pages+0x19/0x90 [4.804268] [] kmalloc_order_trace+0x2e/0xe0 [4.804272] [] __kmalloc+0x232/0x260 [4.804277] [] init_tag_map+0x3d/0xc0 [4.804290] [] __blk_queue_init_tags+0x45/0x80 [4.804293] [] blk_init_tags+0x14/0x20 [4.804298] [] scsi_add_host_with_dma+0x80/0x300 [4.804305] [] qla1280_probe_one+0x683/0x9ef [qla1280] [4.804309] [] local_pci_probe+0x45/0xa0 [4.804312] [] pci_device_probe+0xfd/0x140 [4.804316] [] driver_probe_device+0x222/0x490 [4.804319] [] __driver_attach+0x84/0x90 [4.804321] [] ? driver_probe_device+0x490/0x490 [4.804324] [] bus_for_each_dev+0x6c/0xc0 [4.804326] [] driver_attach+0x1e/0x20 [4.804328] [] bus_add_driver+0x1eb/0x280 [4.804331] [] ? 0xa0015000 [4.804333] [] driver_register+0x60/0xe0 [4.804336] [] __pci_register_driver+0x4c/0x50 [4.804339] [] qla1280_init+0x1ce/0x1000 [qla1280] [4.804341] [] ? 0xa0015000 [4.804345] [] do_one_initcall+0xb3/0x200 [4.804348] [] ? kmem_cache_alloc_trace+0x196/0x210 [4.804352] [] ? do_init_module+0x27/0x1cb [4.804354] [] do_init_module+0x5f/0x1cb [4.804358] [] load_module+0x2040/0x2680 [4.804360] [] ? __symbol_put+0x60/0x60 [4.804363] [] SYSC_init_module+0x149/0x190 [4.804366] [] SyS_init_module+0xe/0x10 [4.804369] [] entry_SYSCALL_64_fastpath+0x12/0x71 [4.804371] ---[ end trace 0ea3b625f86705f7 ]--- [4.804581] qla1280: probe of :11:04.0 failed with error -12 In qla1280_set_defaults() the maximum queue depth is set to 32 so adopt the scsi_host_template to it as well. Signed-off-by: Johannes Thumshirn Cc: Laura Abbott Cc: Michael Reed Cc: sta...@vger.kernel.org --- drivers/scsi/qla1280.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/qla1280.c b/drivers/scsi/qla1280.c index 5d0ec42..6bd748e 100644 --- a/drivers/scsi/qla1280.c +++ b/drivers/scsi/qla1280.c @@ -4214,7 +4214,7 @@ static struct scsi_host_template qla1280_driver_template = { .eh_bus_reset_handler = qla1280_eh_bus_reset, .eh_host_reset_handler = qla1280_eh_adapter_reset, .bios_param = qla1280_biosparam, - .can_queue = 0xf, + .can_queue = 32, .this_id= -1, .sg_tablesize = SG_ALL, .use_clustering = ENABLE_CLUSTERING, -- 2.8.1
[PATCH] doc: correct Documentation/x86/x86_64/mm.txt
Correct the size of the module mapping space and the maximum available physical memory size of current processors. Signed-off-by: Juergen Gross --- Documentation/x86/x86_64/mm.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/x86/x86_64/mm.txt b/Documentation/x86/x86_64/mm.txt index c518dce..5aa7383 100644 --- a/Documentation/x86/x86_64/mm.txt +++ b/Documentation/x86/x86_64/mm.txt @@ -19,7 +19,7 @@ ff00 - ff7f (=39 bits) %esp fixup stacks ffef - (=64 GB) EFI region mapping space ... unused hole ... 8000 - a000 (=512 MB) kernel text mapping, from phys 0 -a000 - ff5f (=1525 MB) module mapping space +a000 - ff5f (=1526 MB) module mapping space ff60 - ffdf (=8 MB) vsyscalls ffe0 - (=2 MB) unused hole @@ -31,8 +31,8 @@ vmalloc space is lazily synchronized into the different PML4 pages of the processes using the page fault handler, with init_level4_pgt as reference. -Current X86-64 implementations only support 40 bits of address space, -but we support up to 46 bits. This expands into MBZ space in the page tables. +Current X86-64 implementations support up to 46 bits of address space (64 TB), +which is our current limit. This expands into MBZ space in the page tables. We map EFI runtime services in the 'efi_pgd' PGD in a 64Gb large virtual memory window (this size is arbitrary, it can be raised later if needed). -- 2.6.6
Re: [PATCH v7 5/8] [media] vcodec: mediatek: Add Mediatek V4L2 Video Encoder Driver
Hi, [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.6-rc4 next-20160421] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Tiffany-Lin/Add-MT8173-Video-Encoder-Driver-and-VPU-Driver/20160422-123111 base: git://linuxtv.org/media_tree.git master config: i386-allyesconfig (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=i386 All warnings (new ones prefixed by >>): drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c: In function 'mtk_venc_encode_header': >> drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c:896:43: warning: format >> '%lx' expects argument of type 'long unsigned int', but argument 9 has type >> 'size_t {aka unsigned int}' [-Wformat=] drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c: In function 'mtk_venc_worker': drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c:1046:43: warning: format '%lx' expects argument of type 'long unsigned int', but argument 7 has type 'size_t {aka unsigned int}' [-Wformat=] drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c:1046:43: warning: format '%lx' expects argument of type 'long unsigned int', but argument 10 has type 'size_t {aka unsigned int}' [-Wformat=] drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c:1046:43: warning: format '%lx' expects argument of type 'long unsigned int', but argument 13 has type 'size_t {aka unsigned int}' [-Wformat=] vim +896 drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c 880 struct mtk_vcodec_ctx *ctx = priv; 881 int ret; 882 struct vb2_buffer *dst_buf; 883 struct mtk_vcodec_mem bs_buf; 884 struct venc_done_result enc_result; 885 886 dst_buf = v4l2_m2m_dst_buf_remove(ctx->m2m_ctx); 887 if (!dst_buf) { 888 mtk_v4l2_debug(1, "No dst buffer"); 889 return -EINVAL; 890 } 891 892 bs_buf.va = vb2_plane_vaddr(dst_buf, 0); 893 bs_buf.dma_addr = vb2_dma_contig_plane_dma_addr(dst_buf, 0); 894 bs_buf.size = (size_t)dst_buf->planes[0].length; 895 > 896 mtk_v4l2_debug(1, 897 "[%d] buf idx=%d va=0x%p dma_addr=0x%llx size=0x%lx", 898 ctx->idx, 899 dst_buf->index, bs_buf.va, 900 (u64)bs_buf.dma_addr, 901 bs_buf.size); 902 903 ret = venc_if_encode(ctx, 904 VENC_START_OPT_ENCODE_SEQUENCE_HEADER, --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
[PATCH] ARM: EXYNOS: Properly skip unitialized parent clock in power domain on
We want to skip reparenting a clock on turning on power domain, if we do not have the parent yet. The parent is obtained when turning the domain off. However due to a typo, the loop is continued on IS_ERR() of clock being reparented, not on the IS_ERR() of the parent. Theoretically this could lead to OOPS on first turn on of a power domain, if there was no turn off before. Practically that should never happen because all power domains are turned on by default (reset value, bootloader does not turn off them usually) so the first action will be always turn off. Fixes: 29e5eea06bc1 ("ARM: EXYNOS: Get current parent clock for power domain on/off") Reported-by: Vladimir Zapolskiy Signed-off-by: Krzysztof Kozlowski --- arch/arm/mach-exynos/pm_domains.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mach-exynos/pm_domains.c b/arch/arm/mach-exynos/pm_domains.c index 7c21760f590f..875a2bab64f6 100644 --- a/arch/arm/mach-exynos/pm_domains.c +++ b/arch/arm/mach-exynos/pm_domains.c @@ -92,7 +92,7 @@ static int exynos_pd_power(struct generic_pm_domain *domain, bool power_on) if (IS_ERR(pd->clk[i])) break; - if (IS_ERR(pd->clk[i])) + if (IS_ERR(pd->pclk[i])) continue; /* Skip on first power up */ if (clk_set_parent(pd->clk[i], pd->pclk[i])) pr_err("%s: error setting parent to clock%d\n", -- 1.9.1
Is the page always in swapcache?
Hi Matainers, In the file mm/migrate.c 316 int migrate_page_move_mapping(struct address_space *mapping, 317 struct page *newpage, struct page *page, 318 struct buffer_head *head, enum migrate_mode mode, 319 int extra_count) 320 { 321 struct zone *oldzone, *newzone; 322 int dirty; 323 int expected_count = 1 + extra_count; 324 void **pslot; 344 345 pslot = radix_tree_lookup_slot(&mapping->page_tree, 346 page_index(page)); 347 348 expected_count += 1 + page_has_private(page); 349 if (page_count(page) != expected_count || 350 radix_tree_deref_slot_protected(pslot, &mapping->tree_lock) != page) { ... 353 } In the line 345, Is the page is always in the swap-cache? I got the follow crash issue with compaction. [ 4433.467956s][2016:04:13 11:06:41][pid:324,cpu2,lmkd]Unable to handle kernel NULL pointer dereference at virtual address [ 4433.467987s][2016:04:13 11:06:41][pid:324,cpu2,lmkd]pgd = ffc0b46f9000 [ 4433.467987s][] *pgd= [ 4433.468017s][2016:04:13 11:06:41][pid:324,cpu2,lmkd]Internal error: Oops: 9605 [#1] PREEMPT SMP [ 4433.468048s]Modules linked in: [ 4433.468078s][2016:04:13 11:06:41][pid:324,cpu2,lmkd]CPU: 2 PID: 324 Comm: lmkd Tainted: GW3.10.94-g0daa20e #1 [ 4433.468109s][2016:04:13 11:06:41][pid:324,cpu2,lmkd]task: ffc0b7783980 ti: ffc0b46c8000 task.ti: ffc0b46c8000 [ 4433.468139s][2016:04:13 11:06:41][pid:324,cpu2,lmkd]PC is at migrate_page_move_mapping.part.28+0x7c/0x21c [ 4433.468170s][2016:04:13 11:06:41][pid:324,cpu2,lmkd]LR is at migrate_page_move_mapping.part.28+0x50/0x21c [ 4433.468170s][2016:04:13 11:06:41][pid:324,cpu2,lmkd]pc : [] lr : [] pstate: 6185 [ 4433.468200s][2016:04:13 11:06:41][pid:324,cpu2,lmkd]sp : ffc0b46cbb20 [ 4433.468200s]x29: ffc0b46cbb20 x28: fff5 [ 4433.468231s]x27: ffc0bb20b000 x26: [ 4433.468261s]x25: x24: 0001 [ 4433.468292s]x23: ffc0013be1a8 x22: [ 4433.468322s]x21: 0002 x20: ffc0bdbfc900 [ 4433.468353s]x19: ffc0bb20b000 x18: [ 4433.468353s]x17: x16: [ 4433.468383s]x15: x14: fff8fff8 [ 4433.468414s]x13: fff8fff8 x12: fff8fff8 [ 4433.468444s]x11: fff8fff8 x10: [ 4433.468475s]x9 : d61f0220913be210 x8 : ffc0ba60ee80 [ 4433.468475s]x7 : 0006 x6 : 0001 [ 4433.468505s]x5 : 01440006010b x4 : 0001 [ 4433.468536s]x3 : x2 : 01440006010b [ 4433.468566s]x1 : 01440006010b x0 : 0002 [ 4433.468597s][2016:04:13 11:06:41][pid:324,cpu2,lmkd] [ 4433.468597s]PC: 0xffc000192838: [ 4433.468597s]2838 d65f03c0 a9bb7bfd 910003fd a90363f7 91006017 2a0403f8 a90153f3 a9025bf5 [ 4433.468658s]2858 aa0203f3 aa0003f5 aa1703e0 aa0103f4 a9046bf9 aa0303f9 942c336b f9400260 [ 4433.468719s]2878 910022b5 37880ba0 f9400a61 aa1503e0 9407a477 aa0003fa f9400262 aa1303e0 [ 4433.468780s]2898 f9400261 f274045f 1a9f07f6 11000ad5 37800a81 b9401c00 6b0002bf 540008a1 [ 4433.468811s]28b8 f9400340 eb00027f 54000841 d5033bbf 5281 91007263 885f7c60 6b15001f [ 4433.468872s]28d8 5461 88027c61 3582 d5033bbf 6b0002bf 540006e1 35d8 b4b9 [ 4433.468933s]28f8 aa1903e0 978f 53001c00 34000600 f9400280 378009c0 91007282 885f7c40 [ 4433.468994s]2918 11000400 88017c40 35a1 f9400260 37880420 12000295 37000734 d5033abf
Re: [PATCH 1/5] x86, KASLR: Update description for decompressor worst case size
* Kees Cook wrote: > +# > +# Getting to provably safe in-place decompression is hard. Worst case > +# behaviours need be analyzed. Here let's take decompressing gzip-compressed > +# kernel as example to illustrate it: > +# > +# The file layout of gzip compressed kernel is as follows. For more > +# information, please refer to RFC 1951 and RFC 1952. > +# > +#magic[2] > +#method[1] > +#flags[1] > +#timestamp[4] > +#extraflags[1] > +#os[1] > +#compressed data blocks[N] > +#crc[4] orig_len[4] > +# > +# resulting in 18 bytes of non compressed data overhead. > +# > +# Files divided into blocks > +# 1 bit (last block flag) > +# 2 bits (block type) > +# > +# 1 block occurs every 32K -1 bytes or when there 50% compression > +# has been achieved. The smallest block type encoding is always used. Yeah, so I incorporated Boris's comments into this text, and started cleaning it up, but then stopped because it became increasingly more difficult as the formulation is ambiguous and vague in places. I'll apply the patch as I edited it, but please let's do another pass to make this section actually readable. I know you didn't write it, but let's try to improve it nevertheless if we are moving it around, especially as we are changing and cleaning up this logic with subsequent patches, ok? Here's the current text as I've edited it: # # Getting to provably safe in-place decompression is hard. Worst case # behaviours need to be analyzed. Here let's take the decompression of # a gzip-compressed kernel as example, to illustrate it: # # The file layout of gzip compressed kernel is: # #magic[2] #method[1] #flags[1] #timestamp[4] #extraflags[1] #os[1] #compressed data blocks[N] #crc[4] orig_len[4] # # ... resulting in +18 bytes overhead of uncompressed data. # # (For more information, please refer to RFC 1951 and RFC 1952.) # # Files divided into blocks # 1 bit (last block flag) # 2 bits (block type) # # 1 block occurs every 32K -1 bytes or when there 50% compression # has been achieved. The smallest block type encoding is always used. # # stored: #32 bits length in bytes. # # fixed: #magic fixed tree. #symbols. # # dynamic: #dynamic tree encoding. #symbols. # # # The buffer for decompression in place is the length of the uncompressed # data, plus a small amount extra to keep the algorithm safe. The # compressed data is placed at the end of the buffer. The output pointer # is placed at the start of the buffer and the input pointer is placed # where the compressed data starts. Problems will occur when the output # pointer overruns the input pointer. # # The output pointer can only overrun the input pointer if the input # pointer is moving faster than the output pointer. A condition only # triggered by data whose compressed form is larger than the uncompressed # form. # # The worst case at the block level is a growth of the compressed data # of 5 bytes per 32767 bytes. # # The worst case internal to a compressed block is very hard to figure. # The worst case can at least be bounded by having one bit that represents # 32764 bytes and then all of the rest of the bytes representing the very # very last byte. # # All of which is enough to compute an amount of extra data that is required # to be safe. To avoid problems at the block level allocating 5 extra bytes # per 32767 bytes of data is sufficient. To avoid problems internal to a # block adding an extra 32767 bytes (the worst case uncompressed block size) # is sufficient, to ensure that in the worst case the decompressed data for # block will stop the byte before the compressed data for a block begins. # To avoid problems with the compressed data's meta information an extra 18 # bytes are needed. Leading to the formula: # # extra_bytes = (uncompressed_size >> 12) + 32768 + 18 + decompressor_size # # Adding 8 bytes per 32K is a bit excessive but much easier to calculate. # Adding 32768 instead of 32767 just makes for round numbers. # Adding the decompressor_size is necessary as it musht live after all # of the data as well. Last I measured the decompressor is about 14K. # 10K of actual data and 4K of bss. # # Above analysis is for decompressing gzip compressed kernel only. Up to # now 6 different decompressor are supported all together. And among them # xz stores data in chunks and has maximum chunk of 64K. Hence safety # margin should be updated to cover all decompressors so that we don't # need to deal with each of them separately. Please check # the description in lib/decompressor_xxx.c for specific information. # # extra_bytes = (uncompressed_size >> 12) + 65536 + 128 # # Note that this calculation, which results in z_extract_offset (below), # is currently generated in compressed/mkpiggy.c The bit where I got lost is right at the beginning: # # Files divided into blocks # 1 bit (last block flag) # 2 bits (block type) # # 1 block occurs every 32K -1 bytes or when there 50% compr
Re: [PATCH] asus-laptop: correct error handling in asus_read_brightness()
On Fri, Apr 22, 2016 at 02:09:22AM +0300, Andy Shevchenko wrote: > On Sat, Apr 16, 2016 at 3:27 AM, Giedrius Statkevičius > wrote: > > It is possible that acpi_evaluate_integer might fail and value would not be > > set to any value so correct this defect by returning 0 in case of an > > error. This is also the correct thing to return because the backlight > > subsystem will print the old value of brightness in this case. > > > > Signed-off-by: Giedrius Statkevičius > > --- > > drivers/platform/x86/asus-laptop.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/platform/x86/asus-laptop.c > > b/drivers/platform/x86/asus-laptop.c > > index 9a69734..15f1311 100644 > > --- a/drivers/platform/x86/asus-laptop.c > > +++ b/drivers/platform/x86/asus-laptop.c > > @@ -775,8 +775,10 @@ static int asus_read_brightness(struct > > backlight_device *bd) > > > > rv = acpi_evaluate_integer(asus->handle, METHOD_BRIGHTNESS_GET, > >NULL, &value); > > - if (ACPI_FAILURE(rv)) > > + if (ACPI_FAILURE(rv)) { > > pr_warn("Error reading brightness\n"); > > + return 0; > > + } > > This looks like a workaround. > I suppose the real fix is to return here an error code and fix all callers, > like > drivers/video/backlight/backlight.c. > It just fixes the behaviour according to the current code in that file. I suppose that would be nice but I don't think it would make any difference because the backlight core code still prints out ->props.brightness in case ops->get_brightness fails. Just the difference would be that now actual error messages are printed in the drivers themselves instead of generic messages from the backlight core. Anyway, I think the current behaviour is more useful because the drivers know better about what has failed.
Re: [PATCH 2/2 v5] irqchip/Layerscape: Add SCFG MSI controller support
On 22/04/16 06:33, Leo Li wrote: > On Mon, Mar 7, 2016 at 3:50 AM, Marc Zyngier wrote: >> On Mon, 7 Mar 2016 11:36:22 +0800 >> Minghuan Lian wrote: >> >>> Some kind of NXP Layerscape SoC provides a MSI >>> implementation which uses two SCFG registers MSIIR and >>> MSIR to support 32 MSI interrupts for each PCIe controller. >>> The patch is to support it. >>> >>> Signed-off-by: Minghuan Lian >> >> Acked-by: Marc Zyngier >> >> The DT binding still needs an Ack from the DT maintainers though (cc'd). > > Marc, > > Who will be responsible to pick this driver? I see you are also one > of the maintainers for irqchip. Can you pick up the driver? The > binding has already gotten ACKed by the device tree maintainer. Can you point me to this Ack? I can't see any trace of it in my Inbox. Thanks, M. -- Jazz is not dead. It just smells funny...
Re: [PATCH 0/5] x86, boot: clean up KASLR code (step 2)
Another small request, I've been doing this to the previous patches: sed -i 's/x86, KASLR: /x86\/KASLR: /g' sed -i 's/x86, boot: /x86\/boot: /g' Could you please apply the regular x86/subsys title format for future patches? Thanks! Ingo
Re: [PATCH] ARM64: dts: rockchip: add core dtsi file for RK3399 SoCs
On 22/04/16 02:50, jay.xu wrote: > Hi Marc: > > On 2016年04月22日 05:12, Marc Zyngier wrote: >> On Thu, 21 Apr 2016 22:24:09 +0200 >> Heiko Stübner wrote: >> >>> Am Donnerstag, 21. April 2016, 12:30:18 schrieb Marc Zyngier: On Thu, 21 Apr 2016 18:47:20 +0800 "Huang, Tao" wrote: > Hi, Mark: > > On 2016年04月21日 18:19, Mark Rutland wrote: >> On Thu, Apr 21, 2016 at 11:58:12AM +0800, Jianqun Xu wrote: >>> + cpu_l0: cpu@0 { >>> + device_type = "cpu"; >>> + compatible = "arm,cortex-a53", "arm,armv8"; >>> + reg = <0x0 0x0>; >>> + enable-method = "psci"; >>> + #cooling-cells = <2>; /* min followed by max */ >>> + clocks = <&cru ARMCLKL>; >>> + }; >>> + cpu_b0: cpu@100 { >>> + device_type = "cpu"; >>> + compatible = "arm,cortex-a72", "arm,armv8"; >>> + reg = <0x0 0x100>; >>> + enable-method = "psci"; >>> + #cooling-cells = <2>; /* min followed by max */ >>> + clocks = <&cru ARMCLKB>; >>> + }; >>> + >>> + arm-pmu { >>> + compatible = "arm,armv8-pmuv3"; >>> + interrupts = ; >>> + }; >> This is wrong, and must go. There should be a separate node for the PMU >> of each microarchitecture, with the appropriate compatible string to >> represent that (see the juno dts). > You are right. The first version we wrote is: > pmu_a53 { > > compatible = "arm,cortex-a53-pmu"; > interrupts = ; > interrupt-affinity = <&cpu_l0>, > > <&cpu_l1>, > <&cpu_l2>, > <&cpu_l3>; > > }; > > pmu_a72 { > > compatible = "arm,cortex-a72-pmu"; > interrupts = ; > interrupt-affinity = <&cpu_b0>, > > <&cpu_b1>; > > }; > > but unfortunately, the arm pmu driver do not support PPI in two cluster > well, > so we have to replace with this implementation. > >> In this case things are messier as the same PPI number is being used >> across clusters. Marc (Cc'd) has been working on PPI partitions, which >> should allow us to support that. > Great! So what we can do right now? Wait this feature, and delete > arm-pmu node? I'd rather you have a look at the patches, test them with your HW, and comment on what doesn't work! >>> I would think we could do it in two tracks, testing and fixing but also >>> letting >>> the rk3399 devicetrees move forward without the pmu at first :-) . >> Where would the fun be then? ;-) > thanks for your advices, and I will try to test the percpu-partition > patches. > > by the way, do you think it's better to let the dtsi be reviewed first, > then the percpu-partition patches could be tested by more people ? Up to you. As long as what is in the DT is correct and Acked by the DT maintainers, I'm fine with it. Thanks, M. -- Jazz is not dead. It just smells funny...
Re: [PATCH v1 4/5] perf: Introduce address range filtering
On Thu, Apr 21, 2016 at 06:17:02PM +0300, Alexander Shishkin wrote: > +struct addr_filter_setup_data { > + struct perf_event *event; > + unsigned long *offs; > + unsigned long gen; > +}; > + > +static int __perf_event_addr_filters_setup(void *info) > +{ > + struct addr_filter_setup_data *id = info; > + struct perf_event *event = id->event; > + struct perf_addr_filters_head *ifh = perf_event_addr_filters(event); > + unsigned long flags; > + > + if (READ_ONCE(event->state) != PERF_EVENT_STATE_ACTIVE) > + return -EAGAIN; > + > + /* matches smp_wmb() in event_sched_in() */ > + smp_rmb(); > + > + /* > + * There is a window with interrupts enabled before we get here, > + * so we need to check again lest we try to stop another cpu's event. > + */ > + if (READ_ONCE(event->oncpu) != smp_processor_id()) > + return -EAGAIN; > + > + raw_spin_lock_irqsave(&ifh->lock, flags); Since we only ever use this from cpu_function_call() IRQs are guaranteed off already, you even rely on that in the above ->oncpu test, so the _irqsave() thing is entirely redundant, no? > + /* > + * In case of generations' mismatch, we don't have to do anything for > + * this instance any, there will be another one with the *right* gen. > + * If called to clear filters, always let it through. > + */ > + if (id->gen == event->addr_filters_gen || !id->offs) > + event->pmu->addr_filters_setup(event, id->offs, PERF_EF_RELOAD); > + raw_spin_unlock_irqrestore(&ifh->lock, flags); > + > + return 0; > +} > + > +static int perf_event_addr_filters_setup(struct perf_event *event, > +unsigned long *offs, > +unsigned long gen) > +{ > + struct addr_filter_setup_data id = { > + .event = event, > + .offs = offs, > + .gen= gen, > + }; > + struct perf_addr_filters_head *ifh = perf_event_addr_filters(event); > + unsigned long flags; > + int ret = 0; > + > + /* > + * We can't use event_function_call() here, because that would > + * require ctx::mutex, but one of our callers is called with > + * mm::mmap_sem down, which would cause an inversion, see bullet > + * (2) in put_event(). > + */ > + do { > + if (READ_ONCE(event->state) != PERF_EVENT_STATE_ACTIVE) { > + raw_spin_lock_irqsave(&ifh->lock, flags); And here, like in all the other sites, IRQs must be enabled, no? > + /* see __perf_event_addr_filters_setup */ How is this not racy? The event could have gotten enabled between that test and getting here, right? > + if (gen == event->addr_filters_gen || !offs) > + event->pmu->addr_filters_setup(event, offs, 0); > + raw_spin_unlock_irqrestore(&ifh->lock, flags); > + > + if (READ_ONCE(event->state) != PERF_EVENT_STATE_ACTIVE) > + break; I r confused, calling ->addr_filters_setup() on an active event, from the wrong cpu, is badness, no? Is this something the callback has to handle? > + /* otherwise, fall through to the cross-call */ > + } > + > + /* matches smp_wmb() in event_sched_in() */ > + smp_rmb(); > + > + ret = cpu_function_call(READ_ONCE(event->oncpu), > + __perf_event_addr_filters_setup, &id); > + } while (ret == -EAGAIN); > + > + return ret; > +} This whole thing seems rather full of tricky :/ > @@ -6398,6 +6629,7 @@ void perf_event_mmap(struct vm_area_struct *vma) > /* .flags (attr_mmap2 only) */ > }; > > + perf_addr_filters_adjust(vma); > perf_event_mmap_event(&mmap_event); > } And this is the 'offending' site that requires all the tricky.. > +/* > + * Calculate event's address filters' ranges based on the > + * task's existing mappings; if any of the existing mappings > + * match the filters, update event's hw configuration and > + * restart it if it's running. > + */ > +static void perf_event_addr_filters_apply(struct perf_event *event) > +{ > + struct perf_addr_filters_head *ifh = perf_event_addr_filters(event); > + struct perf_addr_filter *filter; > + struct task_struct *task = READ_ONCE(event->ctx->task); > + struct mm_struct *mm = NULL; > + unsigned int restart = 0, count = 0; > + unsigned long *offs, flags, gen; > + > + offs = event->hw.addr_filters_offs; > + > + /* > + * We may observe TASK_TOMBSTONE, which means that the event tear-down > + * will stop on the parent's child_mutex that our caller is also holding > + */ > + if (task == TASK_TOMBSTONE) > + return; > + > + mm = get_task_mm(event->ctx->task); > + if (!mm) > +
Re: [PATCH 1/2] clocksource: sp804: Add support for OX810SE 24bit timer width
On Fri, Apr 01, 2016 at 04:22:38PM +0200, Neil Armstrong wrote: > In order to support the Dual-Timer on the Oxford Semiconductor OX810SE SoC, > implement variable counter width, keeping 32bit as default width. > Add new compatible string oxsemi,ox810se-rps-timer in order to select > the 24bit counter width. > > Signed-off-by: Neil Armstrong > --- > drivers/clocksource/timer-sp804.c | 107 > -- > include/clocksource/timer-sp804.h | 42 --- > 2 files changed, 102 insertions(+), 47 deletions(-) I will take those patches but this driver really deserves a cleanup.
Re: [PATCH] scripts: kconfig: implement a sort method
Hi, Randy Dunlap writes: > On 04/21/16 13:07, Felipe Balbi wrote: >> With a growing amount of Kernel configuration, it's >> getting ever more difficult to find anything on >> menuconfig. Because of that, implement mergesort for >> kconfig to make it a little easier for anybody >> building kernels. > > Hi, > > Please explain the problem and the solution better. I don't > get it. it's unclear to me what you don't understand from description above. Try to find and enable some random driver on menuconfig. Here's a suggestion: "Maxim Semiconductor MAX77843 PMIC Support" In any case, the idea is the following: menuconfig has too many options, they are unsorted -> sort them -- balbi signature.asc Description: PGP signature
Re: [PATCH 4/5] x86, boot: Make memcpy handle overlaps
* Kees Cook wrote: > Two uses of memcpy (screen scrolling and ELF parsing) were handling > overlapping memory areas. While there were no explicitly noticed bugs > here (yet), it is best to fix this so that the copying will always be > safe. > > Instead of making a new memmove function that might collide with other > memmove definitions in the decompressors, this just makes the compressed > boot's copy of memcpy overlap safe. > > Reported-by: Yinghai Lu > Suggested-by: Lasse Collin > Signed-off-by: Kees Cook > --- > arch/x86/boot/compressed/misc.c | 4 +--- > arch/x86/boot/compressed/string.c | 22 -- > 2 files changed, 21 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/boot/compressed/string.c > b/arch/x86/boot/compressed/string.c > index 00e788be1db9..1e10e40f49dd 100644 > --- a/arch/x86/boot/compressed/string.c > +++ b/arch/x86/boot/compressed/string.c > @@ -1,7 +1,7 @@ > #include "../string.c" > > #ifdef CONFIG_X86_32 I've applied this patch, but could you please also do another patch that adds a comment block to the top of this special version of compressed/string.c, which explains why this file exists and what its purpose is? Also: +/* + * This memcpy is overlap safe (i.e. it is memmove without conflicting + * with other definitions of memmove from the various decompressors. + */ +void *memcpy(void *dest, const void *src, size_t n) I'd not name it memcpy() if its semantics are not the same as the regular kernel memcpy() - that will only cause confusion later on. I'd try to name it memmove() and would fix the memmove() hacks in decompressors: lib/decompress_unxz.c:#ifndef memmove lib/decompress_unxz.c:void *memmove(void *dest, const void *src, size_t size) lib/decompress_unxz.c: * Since we need memmove anyway, would use it as memcpy too. lib/decompress_unxz.c:# define memcpy memmove any strong reason this cannot be done? Some other decompressors seem to avoid memmove() intentionally: lib/decompress_bunzip2.c:*by 256 in any case, using memmove here would lib/decompress_unlzo.c: * of the buffer. This way memmove() isn't needed which lib/decompress_unlzo.c: * Use a loop to avoid memmove() dependency. Thanks, Ingo
Re: [PATCH v7 6/8] [media] vcodec: mediatek: Add Mediatek VP8 Video Encoder Driver
Hi, [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.6-rc4 next-20160421] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Tiffany-Lin/Add-MT8173-Video-Encoder-Driver-and-VPU-Driver/20160422-123111 base: git://linuxtv.org/media_tree.git master config: i386-allyesconfig (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=i386 All warnings (new ones prefixed by >>): drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c: In function 'vp8_enc_alloc_work_buf': >> drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c:214:35: warning: format >> '%lx' expects argument of type 'long unsigned int', but argument 7 has type >> 'size_t {aka unsigned int}' [-Wformat=] drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c: In function 'vp8_enc_compose_one_frame': >> drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c:279:10: warning: format >> '%ld' expects argument of type 'long int', but argument 4 has type 'size_t >> {aka unsigned int}' [-Wformat=] mtk_vcodec_err(inst, "bitstream buf size is too small(%ld)", ^ vim +214 drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c 208 tmp_va = vpu_mapping_dm_addr(inst->vpu_inst.dev, 209 wb[i].vpua); 210 memcpy(inst->work_bufs[i].va, tmp_va, wb[i].size); 211 } 212 wb[i].iova = inst->work_bufs[i].dma_addr; 213 > 214 mtk_vcodec_debug(inst, 215 "work_bufs[%d] va=0x%p,iova=0x%p,size=0x%lx", 216 i, inst->work_bufs[i].va, 217 (void *)inst->work_bufs[i].dma_addr, 218 inst->work_bufs[i].size); 219 } 220 221 mtk_vcodec_debug_leave(inst); 222 223 return ret; 224 225 err_alloc: 226 vp8_enc_free_work_buf(inst); 227 228 return ret; 229 } 230 231 static unsigned int vp8_enc_wait_venc_done(struct venc_vp8_inst *inst) 232 { 233 unsigned int irq_status = 0; 234 struct mtk_vcodec_ctx *ctx = (struct mtk_vcodec_ctx *)inst->ctx; 235 236 if (!mtk_vcodec_wait_for_done_ctx(ctx, MTK_INST_IRQ_RECEIVED, 237WAIT_INTR_TIMEOUT_MS)) { 238 irq_status = ctx->irq_status; 239 mtk_vcodec_debug(inst, "isr return %x", irq_status); 240 } 241 return irq_status; 242 } 243 244 /* 245 * Compose ac_tag, bitstream header and bitstream payload into 246 * one bitstream buffer. 247 */ 248 static int vp8_enc_compose_one_frame(struct venc_vp8_inst *inst, 249 struct mtk_vcodec_mem *bs_buf, 250 unsigned int *bs_size) 251 { 252 unsigned int not_key; 253 u32 bs_frm_size; 254 u32 bs_hdr_len; 255 unsigned int ac_tag_size; 256 u8 ac_tag[MAX_AC_TAG_SIZE]; 257 258 bs_frm_size = vp8_enc_read_reg(inst, VENC_BITSTREAM_FRAME_SIZE); 259 bs_hdr_len = vp8_enc_read_reg(inst, VENC_BITSTREAM_HEADER_LEN); 260 261 /* if a frame is key frame, not_key is 0 */ 262 not_key = !inst->vpu_inst.is_key_frm; 263 *(u32 *)ac_tag = __cpu_to_le32((bs_hdr_len << 5) | 0x10 | not_key); 264 /* key frame */ 265 if (not_key == 0) { 266 ac_tag_size = MAX_AC_TAG_SIZE; 267 ac_tag[3] = 0x9d; 268 ac_tag[4] = 0x01; 269 ac_tag[5] = 0x2a; 270 ac_tag[6] = inst->vsi->config.pic_w; 271 ac_tag[7] = inst->vsi->config.pic_w >> 8; 272 ac_tag[8] = inst->vsi->config.pic_h; 273 ac_tag[9] = inst->vsi->config.pic_h >> 8; 274 } else { 275 ac_tag_size = 3; 276 } 277 278 if (bs_buf->size < bs_hdr_len + bs_frm_size + ac_tag_size) { > 279 mtk_vcodec_err(inst, "bitstream buf size is too > small(%ld)", 280 bs_buf->size); 281 return -EINVAL; 282 } --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
Re: [regression] linux318, linux41 - kernel stack is corrupted
Hi, I just hit the same with 4.1.22 on Gentoo. 4.1.21 are working fine. On 22.04.2016 at 08:46, Greg Kroah-Hartman wrote: ... You are going to have to be a bit more specific here... What is the oops message? How do you reproduce this? Does it also happen on 4.6-rc4? Can you run 'git bisect' to find the offending patch? Greg have you seen screenshots linked by Philip? -- Sebastian M. Bobrecki
RE: [PATCH 2/2 v5] irqchip/Layerscape: Add SCFG MSI controller support
Hi Marc, Please see the link: https://patchwork.kernel.org/patch/8649241/ Rob Herring has given the ACK. I have submitted the v6 patch: https://patchwork.kernel.org/patch/8649251/ Please apply the latest the patch after you review. Thank you very much. Regard, Minghuan > -Original Message- > From: Marc Zyngier [mailto:marc.zyng...@arm.com] > Sent: Friday, April 22, 2016 3:43 PM > To: Leo Li > Cc: Minghuan Lian ; > linux-arm-ker...@lists.infradead.org; lkml ; > Thomas Gleixner ; Jason Cooper > ; Roy Zang ; Mingkai Hu > ; Stuart Yoder ; Yang-Leo Li > ; Rob Herring ; Mark Rutland > > Subject: Re: [PATCH 2/2 v5] irqchip/Layerscape: Add SCFG MSI controller > support > > On 22/04/16 06:33, Leo Li wrote: > > On Mon, Mar 7, 2016 at 3:50 AM, Marc Zyngier > wrote: > >> On Mon, 7 Mar 2016 11:36:22 +0800 > >> Minghuan Lian wrote: > >> > >>> Some kind of NXP Layerscape SoC provides a MSI > >>> implementation which uses two SCFG registers MSIIR and > >>> MSIR to support 32 MSI interrupts for each PCIe controller. > >>> The patch is to support it. > >>> > >>> Signed-off-by: Minghuan Lian > >> > >> Acked-by: Marc Zyngier > >> > >> The DT binding still needs an Ack from the DT maintainers though (cc'd). > > > > Marc, > > > > Who will be responsible to pick this driver? I see you are also one > > of the maintainers for irqchip. Can you pick up the driver? The > > binding has already gotten ACKed by the device tree maintainer. > > Can you point me to this Ack? I can't see any trace of it in my Inbox. > > Thanks, > > M. > -- > Jazz is not dead. It just smells funny...
Re: [PATCH v7 00/24] i2c mux cleanup and locking update
Hi Wolfram, Wolfram Sang wrote: > This was the diff of v6: > > > 32 files changed, 1277 insertions(+), 915 deletions(-) > > This is v7: > > > 32 files changed, 1225 insertions(+), 916 deletions(-) > > So, we gained a little overall. And while the individual drivers have a > few lines more now, I still think it is more readable. > > So, thanks for doing that! Most, if not all, of these ~50 lines gained in v7 come from removing i2c_mux_reserve_adapters and realloc. I.e., I think we might even have lost a few lines with the i2c_mux_one_adapter removal, but not that many. If the doc patch is taken out of the equation, we gain new functionality, fix a bug and lose lines of code. Feels good! However, this is not why I'm writing this message... > I'll give people some time for testing. I'll have a further look, too. > Hopefully, I can pick up patches 1-14 by the end of the week. The reason for this message is that I just realized a couple of things. First, you have suggested to merge patches 1-14 about now, and I assumed that implied merging with Linus for 4.6, but on rereading I realize that you have consistently targeted 4.7. I must be sloppy, and that had escaped me. Is it really necessary to be that cautious? Maybe I'm overly confident, but is 1-14 really such a big deal that it has to float in next for a whole cycle? The problem with waiting until 4.8 with the rest of the series is that it will likely go stale, e.g. patch 22 ([media] rtl2832: change the i2c gate to be mux-locked) touches a ton of register accesses in that driver since it removes a regmap wrapper that is rendered obsolete. Expecting that patch to work for 4.8 is overly optimistic, and while patching things up is (probably) easy, it also renders any previous testing suspect. That said, maybe I'm just impatient and reckless? Second, should we not add 15-24 to the next branch now as well to get testing going asap, even if we do not intend to merge it just yet or even in that exact form? Or is that abusing the next branch? Third, should we deprecate the old i2c_add_mux_adapter, so that new users do not crop up behind our backs in the transition? Or not bother? Fourth, I forgot to change patch 8 (iio: imu: inv_mpu6050: convert to use an explicit i2c mux core) to not change i2c_get_clientdata() -> dev_get_drvdata() as requested by Jonathan Cameron. How should I handle that? There are also some new Tested-by tags that I have added to my local branch but have not pushed anywhere. I'm ready to push all that to a new branch once you are ready to take it. Cheers, Peter
[tip:smp/urgent] cpu/hotplug: Fix rollback during error-out in __cpu_disable()
Commit-ID: 3b9d6da67e11ca8f78fde887918983523a36b0fa Gitweb: http://git.kernel.org/tip/3b9d6da67e11ca8f78fde887918983523a36b0fa Author: Sebastian Andrzej Siewior AuthorDate: Fri, 8 Apr 2016 14:40:15 +0200 Committer: Thomas Gleixner CommitDate: Fri, 22 Apr 2016 09:49:49 +0200 cpu/hotplug: Fix rollback during error-out in __cpu_disable() The recent introduction of the hotplug thread which invokes the callbacks on the plugged cpu, cased the following regression: If takedown_cpu() fails, then we run into several issues: 1) The rollback of the target cpu states is not invoked. That leaves the smp threads and the hotplug thread in disabled state. 2) notify_online() is executed due to a missing skip_onerr flag. That causes that both CPU_DOWN_FAILED and CPU_ONLINE notifications are invoked which confuses quite some notifiers. 3) The CPU_DOWN_FAILED notification is not invoked on the target CPU. That's not an issue per se, but it is inconsistent and in consequence blocks the patches which rely on these states being invoked on the target CPU and not on the controlling cpu. It also does not preserve the strict call order on rollback which is problematic for the ongoing state machine conversion as well. To fix this we add a rollback flag to the remote callback machinery and invoke the rollback including the CPU_DOWN_FAILED notification on the remote cpu. Further mark the notify online state with 'skip_onerr' so we don't get a double invokation. This workaround will go away once we moved the unplug invocation to the target cpu itself. [ tglx: Massaged changelog and moved the CPU_DOWN_FAILED notifiaction to the target cpu ] Fixes: 4cb28ced23c4 ("cpu/hotplug: Create hotplug threads") Reported-by: Heiko Carstens Signed-off-by: Sebastian Andrzej Siewior Cc: linux-s...@vger.kernel.org Cc: r...@linutronix.de Cc: Martin Schwidefsky Cc: Anna-Maria Gleixner Link: http://lkml.kernel.org/r/20160408124015.ga21...@linutronix.de Signed-off-by: Thomas Gleixner --- kernel/cpu.c | 33 ++--- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/kernel/cpu.c b/kernel/cpu.c index 6ea42e8..3e3f6e4 100644 --- a/kernel/cpu.c +++ b/kernel/cpu.c @@ -36,6 +36,7 @@ * @target:The target state * @thread:Pointer to the hotplug thread * @should_run:Thread should execute + * @rollback: Perform a rollback * @cb_stat: The state for a single callback (install/uninstall) * @cb:Single callback function (install/uninstall) * @result:Result of the operation @@ -47,6 +48,7 @@ struct cpuhp_cpu_state { #ifdef CONFIG_SMP struct task_struct *thread; boolshould_run; + boolrollback; enum cpuhp_statecb_state; int (*cb)(unsigned int cpu); int result; @@ -301,6 +303,11 @@ static int cpu_notify(unsigned long val, unsigned int cpu) return __cpu_notify(val, cpu, -1, NULL); } +static void cpu_notify_nofail(unsigned long val, unsigned int cpu) +{ + BUG_ON(cpu_notify(val, cpu)); +} + /* Notifier wrappers for transitioning to state machine */ static int notify_prepare(unsigned int cpu) { @@ -477,6 +484,16 @@ static void cpuhp_thread_fun(unsigned int cpu) } else { ret = cpuhp_invoke_callback(cpu, st->cb_state, st->cb); } + } else if (st->rollback) { + BUG_ON(st->state < CPUHP_AP_ONLINE_IDLE); + + undo_cpu_down(cpu, st, cpuhp_ap_states); + /* +* This is a momentary workaround to keep the notifier users +* happy. Will go away once we got rid of the notifiers. +*/ + cpu_notify_nofail(CPU_DOWN_FAILED, cpu); + st->rollback = false; } else { /* Cannot happen */ BUG_ON(st->state < CPUHP_AP_ONLINE_IDLE); @@ -636,11 +653,6 @@ static inline void check_for_tasks(int dead_cpu) read_unlock(&tasklist_lock); } -static void cpu_notify_nofail(unsigned long val, unsigned int cpu) -{ - BUG_ON(cpu_notify(val, cpu)); -} - static int notify_down_prepare(unsigned int cpu) { int err, nr_calls = 0; @@ -721,9 +733,10 @@ static int takedown_cpu(unsigned int cpu) */ err = stop_machine(take_cpu_down, NULL, cpumask_of(cpu)); if (err) { - /* CPU didn't die: tell everyone. Can't complain. */ - cpu_notify_nofail(CPU_DOWN_FAILED, cpu); + /* CPU refused to die */ irq_unlock_sparse(); + /* Unpark the hotplug thread so we can rollback there */ + kthread_unpark(per_cpu_ptr(&cpuhp_state, cpu)->thread); return err; } BUG_ON(cpu_online(cpu)); @@ -832,6 +845,11 @@ static int __ref _cpu_down(unsigned int cpu, int t
Re: [PATCH 1/2] clocksource: sp804: Add support for OX810SE 24bit timer width
On Fri, 22 Apr 2016, Daniel Lezcano wrote: > On Fri, Apr 01, 2016 at 04:22:38PM +0200, Neil Armstrong wrote: > > In order to support the Dual-Timer on the Oxford Semiconductor OX810SE SoC, > > implement variable counter width, keeping 32bit as default width. > > Add new compatible string oxsemi,ox810se-rps-timer in order to select > > the 24bit counter width. > > > > Signed-off-by: Neil Armstrong > > --- > > drivers/clocksource/timer-sp804.c | 107 > > -- > > include/clocksource/timer-sp804.h | 42 --- > > 2 files changed, 102 insertions(+), 47 deletions(-) > > I will take those patches but this driver really deserves a cleanup. If it deserves a cleanup, then this should happen _BEFORE_ we add new functionality to it. Thanks, tglx
Re: [PATCH 4/5] x86, boot: Make memcpy handle overlaps
* Kees Cook wrote: > Two uses of memcpy (screen scrolling and ELF parsing) were handling > overlapping memory areas. While there were no explicitly noticed bugs > here (yet), it is best to fix this so that the copying will always be > safe. > > Instead of making a new memmove function that might collide with other > memmove definitions in the decompressors, this just makes the compressed > boot's copy of memcpy overlap safe. Btw., I changed all mentions of function calls to include a '()', i.e.: Subject: x86/boot: Make memcpy() handle overlaps From: Kees Cook Date: Wed, 20 Apr 2016 13:55:45 -0700 Two uses of memcpy() (screen scrolling and ELF parsing) were handling overlapping memory areas. While there were no explicitly noticed bugs here (yet), it is best to fix this so that the copying will always be safe. Instead of making a new memmove() function that might collide with other memmove() definitions in the decompressors, this just makes the compressed boot code's copy of memcpy() overlap-safe. Please try to do this in future changelogs and patch titles, all references to function calls should use parentheses, and all references to variables or parameters should be escaped with '...' when it's not abundantly clear what they are - this makes for much easier reading. So just to mention an extreme (made up) example, which of these two commit titles is less confusing to read: Change out parameter of function to buffer to avoid confusion or: Change 'out' parameter of function() to 'buffer' to avoid confusion ? I know which one I'd pick! ;-) Thanks, Ingo
Re: [regression] linux318, linux41 - kernel stack is corrupted
On Fri, Apr 22, 2016 at 09:47:04AM +0200, Sebastian M. Bobrecki wrote: > Hi, > > I just hit the same with 4.1.22 on Gentoo. 4.1.21 are working fine. > > On 22.04.2016 at 08:46, Greg Kroah-Hartman wrote: > > ... > > You are going to have to be a bit more specific here... > > What is the oops message? How do you reproduce this? Does it also > > happen on 4.6-rc4? > > > > Can you run 'git bisect' to find the offending patch? > > > Greg have you seen screenshots linked by Philip? I saw no such screenshots in the email.
Re: [PATCH 2/2 v5] irqchip/Layerscape: Add SCFG MSI controller support
On 22/04/16 08:53, Minghuan Lian wrote: > Hi Marc, > > Please see the link: > https://patchwork.kernel.org/patch/8649241/ > > Rob Herring has given the ACK. > > I have submitted the v6 patch: https://patchwork.kernel.org/patch/8649251/ > Please apply the latest the patch after you review. Thanks. I'll queue that for 4.7 together with Rob's ack. Thanks, M. -- Jazz is not dead. It just smells funny...
Re: [PATCH 1/5] perf tools: fix incorrect ordering of callchain entries
On 19/04/16 11:56, Chris Phlipot wrote: > The existing implentation implementation of thread__resolve_callchain, Remove 'implentation' > under certain circumstanes, can assemble callchain entries in the 'circumstanes' -> 'circumstances' > incorrect order. > > A the callchain entries are resolved incorrectly for a sample when all > of the following conditions are met: > > 1. callchain_param.order is set to ORDER_CALLER > > 2. thread__resolve_callchain_sample is able to resolve callchain entries >for the sample. > > 3. unwind__get_entries is also able to resolve callchain entries for the >sample. > > The fix is accomplished by reversing the order in which > thread__resolve_callchain_sample and unwind__get_entries are called > when callchain_param.order is set to ORDER_CALLER. Can you give an example of the commands you used and what the call chain looked like before and after. Also please run ./scripts/checkpatch.pl > > Unwind specific code from thread__resolve_callchain is also moved into a > new static function to improve readability of the fix. > > Signed-off-by: Chris Phlipot > --- > tools/perf/util/machine.c | 57 > ++- > 1 file changed, 41 insertions(+), 16 deletions(-) > > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c > index 0c4dabc..dd086c8 100644 > --- a/tools/perf/util/machine.c > +++ b/tools/perf/util/machine.c > @@ -1806,8 +1806,6 @@ static int thread__resolve_callchain_sample(struct > thread *thread, > int skip_idx = -1; > int first_call = 0; > > - callchain_cursor_reset(cursor); > - > if (has_branch_callstack(evsel)) { > err = resolve_lbr_callchain_sample(thread, cursor, sample, > parent, > root_al, max_stack); > @@ -1918,20 +1916,12 @@ static int unwind_entry(struct unwind_entry *entry, > void *arg) > entry->map, entry->sym); > } > > -int thread__resolve_callchain(struct thread *thread, > - struct callchain_cursor *cursor, > - struct perf_evsel *evsel, > - struct perf_sample *sample, > - struct symbol **parent, > - struct addr_location *root_al, > - int max_stack) > -{ > - int ret = thread__resolve_callchain_sample(thread, cursor, evsel, > -sample, parent, > -root_al, max_stack); > - if (ret) > - return ret; > - > +static int thread__resolve_callchain_unwind(struct thread *thread, > +struct callchain_cursor *cursor, > + struct perf_evsel *evsel, > + struct perf_sample *sample, > + int max_stack) > +{ > /* Can we do dwarf post unwind? */ > if (!((evsel->attr.sample_type & PERF_SAMPLE_REGS_USER) && > (evsel->attr.sample_type & PERF_SAMPLE_STACK_USER))) > @@ -1944,7 +1934,42 @@ int thread__resolve_callchain(struct thread *thread, > > return unwind__get_entries(unwind_entry, cursor, > thread, sample, max_stack); > +} > + > +int thread__resolve_callchain(struct thread *thread, > + struct callchain_cursor *cursor, > + struct perf_evsel *evsel, > + struct perf_sample *sample, > + struct symbol **parent, > + struct addr_location *root_al, > + int max_stack) > +{ > + int ret = 0; > + callchain_cursor_reset(&callchain_cursor); > + > + if (callchain_param.order == ORDER_CALLEE) { > +ret = thread__resolve_callchain_sample(thread, cursor, > + evsel, sample, > +parent, root_al, > +max_stack); > + if (ret) > + return ret; > +ret = thread__resolve_callchain_unwind(thread, cursor, > + evsel, sample, > +max_stack); > + } else { > +ret = thread__resolve_callchain_unwind(thread, cursor, > + evsel, sample, > +max_stack); > + if (ret) > + return ret; > +ret = thread__resolve_callchain_sample(thread, cursor, > + evsel, sample, > +
Re: [PATCH 2/5] perf script: extend db-export api to include callchains for samples
On 19/04/16 11:56, Chris Phlipot wrote: > The current implementation of the python database export API only > includes call path information when using some form of call/return > tracing, but is unable to do so when sampling. > > The following API extensions allow exporting of data collected by > perf record when using --call-graph. > > The additions to the python api include the following: > - add call_path_id to sample_table to allow association of samples > with call paths > > - add dso_id to call_path_table to more closely align the data with that > of a callchain_node The call_paths table already has symbol_id which belongs uniquely to a DSO, so why do we need dso_id as well? > > db-export and trace-script-python were both extended to accomidate the API accomidate -> accommodate > changes listed above. > > Thread-stack's functionality was expanded to allow storage and exporting > of callchains that result from individual samples. > > - Introduced a new static function (thread_stack__process_callchain) to > resolve call paths using the existing callchain resolution provided by > thread__resolve_callchain > > - The existing call_path tree in call_return_processor is used for storing > the data from the resolved callchain. > > - Call_return_processor was also extended to allow the ability to export > full call paths in addtion to the existing individual call/return pairs, > since call/return pairs are not available when doing sampling. Why do you need a callback? Seems like the only thing you need from thread-stack.c is the call path tree. You could move that to its own .c/.h files and then process the call chain in db-export.c Also a list of changes like the one above heavily implies you are not obeying the one patch == one change rule. Please try to make patches that only do one thing and also run checkpatch. If you don't mind, I'll let you respond to my comments before I comment on any other patches.
Re: [PATCH v1 2/5] perf/x86/intel/pt: IP filtering register/cpuid bits
On Thu, 21 Apr 2016, Borislav Petkov wrote: > On Thu, Apr 21, 2016 at 08:55:38PM +0200, Thomas Gleixner wrote: > > I have to disagree here. The MSRs itself can really go into msr-index.h > > while > > the bit definitions might go elsewhere. What's wrong with having all MSRs > > at a > > central place? > > Same reason as for pci_ids.h - to contain only MSRs which are used in > multiple compilation units. That's really not the same thing. pci ids are issued by a gazillion of vendors for a bazillion of different devices. There is no consistent view for them. MSRs on the other hand are x86 specific registers nicely defined in the SDM/APM and having at least the MSR defines in a single header makes a lot of sense. Thanks, tglx
Re: [PATCH v5] irqchip, gicv3-its, numa: Enable workaround for Cavium thunderx erratum 23144
On 21/04/16 18:40, Robert Richter wrote: > On 15.04.16 21:30:05, Robert Richter wrote: >> From: Ganapatrao Kulkarni >> >> The erratum fixes the hang of ITS SYNC command by avoiding inter node >> io and collections/cpu mapping on thunderx dual-socket platform. >> >> This fix is only applicable for Cavium's ThunderX dual-socket platform. >> >> This is based on NUMA v16 series. >> Message-Id: <1460155828-8690-1-git-send-email-ddaney.c...@gmail.com> > > Will, Catalin, > > after NUMA base patches went into linux-next (many thanks), please > consider this pass 1.x errata workaround for inclusion too. I'll queue it as a fix once the NUMA patches are merged into mainline. Thanks, M. -- Jazz is not dead. It just smells funny...
Re: [PATCH 1/5] perf tools: fix incorrect ordering of callchain entries
+Jiri since he wrote the original code On 22/04/16 10:55, Adrian Hunter wrote: > On 19/04/16 11:56, Chris Phlipot wrote: >> The existing implentation implementation of thread__resolve_callchain, > > Remove 'implentation' > >> under certain circumstanes, can assemble callchain entries in the > > 'circumstanes' -> 'circumstances' > >> incorrect order. >> >> A the callchain entries are resolved incorrectly for a sample when all >> of the following conditions are met: >> >> 1. callchain_param.order is set to ORDER_CALLER >> >> 2. thread__resolve_callchain_sample is able to resolve callchain entries >>for the sample. >> >> 3. unwind__get_entries is also able to resolve callchain entries for the >>sample. >> >> The fix is accomplished by reversing the order in which >> thread__resolve_callchain_sample and unwind__get_entries are called >> when callchain_param.order is set to ORDER_CALLER. > > Can you give an example of the commands you used and what the call chain > looked like before and after. > > Also please run ./scripts/checkpatch.pl > >> >> Unwind specific code from thread__resolve_callchain is also moved into a >> new static function to improve readability of the fix. >> >> Signed-off-by: Chris Phlipot >> --- >> tools/perf/util/machine.c | 57 >> ++- >> 1 file changed, 41 insertions(+), 16 deletions(-) >> >> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c >> index 0c4dabc..dd086c8 100644 >> --- a/tools/perf/util/machine.c >> +++ b/tools/perf/util/machine.c >> @@ -1806,8 +1806,6 @@ static int thread__resolve_callchain_sample(struct >> thread *thread, >> int skip_idx = -1; >> int first_call = 0; >> >> -callchain_cursor_reset(cursor); >> - >> if (has_branch_callstack(evsel)) { >> err = resolve_lbr_callchain_sample(thread, cursor, sample, >> parent, >> root_al, max_stack); >> @@ -1918,20 +1916,12 @@ static int unwind_entry(struct unwind_entry *entry, >> void *arg) >> entry->map, entry->sym); >> } >> >> -int thread__resolve_callchain(struct thread *thread, >> - struct callchain_cursor *cursor, >> - struct perf_evsel *evsel, >> - struct perf_sample *sample, >> - struct symbol **parent, >> - struct addr_location *root_al, >> - int max_stack) >> -{ >> -int ret = thread__resolve_callchain_sample(thread, cursor, evsel, >> - sample, parent, >> - root_al, max_stack); >> -if (ret) >> -return ret; >> - >> +static int thread__resolve_callchain_unwind(struct thread *thread, >> +struct callchain_cursor *cursor, >> +struct perf_evsel *evsel, >> +struct perf_sample *sample, >> +int max_stack) >> +{ >> /* Can we do dwarf post unwind? */ >> if (!((evsel->attr.sample_type & PERF_SAMPLE_REGS_USER) && >>(evsel->attr.sample_type & PERF_SAMPLE_STACK_USER))) >> @@ -1944,7 +1934,42 @@ int thread__resolve_callchain(struct thread *thread, >> >> return unwind__get_entries(unwind_entry, cursor, >> thread, sample, max_stack); >> +} >> + >> +int thread__resolve_callchain(struct thread *thread, >> + struct callchain_cursor *cursor, >> + struct perf_evsel *evsel, >> + struct perf_sample *sample, >> + struct symbol **parent, >> + struct addr_location *root_al, >> + int max_stack) >> +{ >> +int ret = 0; >> +callchain_cursor_reset(&callchain_cursor); >> + >> +if (callchain_param.order == ORDER_CALLEE) { >> +ret = thread__resolve_callchain_sample(thread, cursor, >> + evsel, sample, >> + parent, root_al, >> + max_stack); >> +if (ret) >> +return ret; >> +ret = thread__resolve_callchain_unwind(thread, cursor, >> + evsel, sample, >> + max_stack); >> +} else { >> +ret = thread__resolve_callchain_unwind(thread, cursor, >> + evsel, sample, >> + max_stack); >> +if (ret) >> +return ret; >> +ret = thread__re
Re: [PATCH v7 7/8] [media] vcodec: mediatek: Add Mediatek H264 Video Encoder Driver
Hi, [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.6-rc4 next-20160421] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Tiffany-Lin/Add-MT8173-Video-Encoder-Driver-and-VPU-Driver/20160422-123111 base: git://linuxtv.org/media_tree.git master config: i386-allyesconfig (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=i386 All warnings (new ones prefixed by >>): drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c: In function 'h264_enc_alloc_work_buf': >> drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c:299:35: warning: >> format '%lx' expects argument of type 'long unsigned int', but argument 7 >> has type 'size_t {aka unsigned int}' [-Wformat=] vim +299 drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c 283 * This RC_CODE is pre-allocated by VPU and saved in VPU 284 * addr. So we need use memcpy to copy RC_CODE from VPU 285 * addr into IO virtual addr in 'iova' field for reg 286 * setting in VPU side. 287 */ 288 if (i == VENC_H264_VPU_WORK_BUF_RC_CODE) { 289 void *tmp_va; 290 291 tmp_va = vpu_mapping_dm_addr(inst->vpu_inst.dev, 292 wb[i].vpua); 293 memcpy(inst->work_bufs[i].va, tmp_va, 294 wb[i].size); 295 } 296 } 297 wb[i].iova = inst->work_bufs[i].dma_addr; 298 > 299 mtk_vcodec_debug(inst, 300 "work_buf[%d] va=0x%p iova=0x%p size=0x%lx", 301 i, inst->work_bufs[i].va, 302 (void *)inst->work_bufs[i].dma_addr, 303 inst->work_bufs[i].size); 304 } 305 306 /* the pps_buf is used by AP side only */ 307 inst->pps_buf.size = 128; --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
Re: [regression] linux318, linux41 - kernel stack is corrupted
W dniu 22.04.2016 o 09:55, Greg Kroah-Hartman pisze: On Fri, Apr 22, 2016 at 09:47:04AM +0200, Sebastian M. Bobrecki wrote: Hi, I just hit the same with 4.1.22 on Gentoo. 4.1.21 are working fine. On 22.04.2016 at 08:46, Greg Kroah-Hartman wrote: ... You are going to have to be a bit more specific here... What is the oops message? How do you reproduce this? Does it also happen on 4.6-rc4? Can you run 'git bisect' to find the offending patch? Greg have you seen screenshots linked by Philip? I saw no such screenshots in the email. They are here: https://github.com/manjaro/packages-core/issues/36 -- Sebastian M. Bobrecki
Re: [PATCH V2 04/14] irqdomain: Fix handling of type settings for existing mappings
On 20/04/16 12:03, Jon Hunter wrote: > When mapping an IRQ, it is possible that a mapping for the IRQ already > exists. If mapping does exist then there are the following issues with > regard to the handling of the IRQ type settings ... > 1. If the domain is part of a hierarchy, then: >a. We do not check that the type settings for the existing mapping > match those of the new mapping. >b. We do not check to see if the type settings have been programmed > yet (and they might not have been) and so we may never set the > type. > 2. If the domain is NOT part of a hierarchy, we will overwrite the >current type settings programmed if they are different from the >previous mapping. Please note that irq_create_mapping() >calls irq_find_mapping() to check if a mapping already exists. > > Although, it may be unlikely that the type settings for a shared > interrupt would not match, nonetheless we should check for this. > Therefore, to fix this check if a mapping exists (regardless of whether > the domain is part of a hierarchy or not) and if it does then: > 1. Return the IRQ number if the type settings match or are not >specified. > 2. Program the type settings and return the IRQ number if the type >settings have not been programmed yet. > 3. Otherwise if the type setting do not match, then print a warning >and don't return the IRQ number. > > Furthermore, add a warning if the type return by irq_domain_translate() > has bits outside the sense mask set and then clear these bits. If these > bits are not cleared then this will cause the comparision of the type > settings for an existing mapping to fail with that of the new mapping > even if the sense bit themselves match. The reason being is that the > existing type settings are read by calling irq_get_trigger_type() which > will clear any bits outside the sense mask. This will allow us to detect > irqchips that are not correctly clearing these bits and fix them. > > Signed-off-by: Jon Hunter Reviewed-by: Marc Zyngier M. -- Jazz is not dead. It just smells funny...
Re: [GIT PULL] phy: for 4.6 -rc
On Sun, Apr 17, 2016 at 12:27:09PM +0530, Kishon Vijay Abraham I wrote: > Hi Greg, > > Please find the pull request for 4.6 -rc cycle below. It consists of > couple of fixes to move the rockchip emmc phy and rockchip display phy > as a child device of GRF. Both these PHY drivers were merged in the > recent merge window and requires their dt bindings to be fixed. > > Consider merging this in the -rc cycle. Let me know if I have to change > something. Pulled and pushed out, thanks. greg k-h
[PATCH 0/7] Make debugobjects fixup functions return bool type
From: "Du, Changbin" Hello, I am going to introduce debugobjects infrastructure to USB subsystem. But before this, I found the code of debugobjects could be improved. This patchset will make fixup functions return bool type instead of int. Because fixup only need report success or no. boolean is the 'real' type. The MAINTAINERS file doesn't include lib/debugobjects.c, so I can only send to Andrew Morton (a...@linux-foundation.org) as a last resort. :) I will probaly have 2 other patches which send later. Du, Changbin (7): debugobjects: make fixup functions return bool instead of int debugobjects: correct the usage of fixup call results workqueue: update debugobjects fixup callbacks return type timer: update debugobjects fixup callbacks return type rcu: update debugobjects fixup callbacks return type percpu_counter: update debugobjects fixup callbacks return type Documentation: update debugobjects doc Documentation/DocBook/debugobjects.tmpl | 26 ++- include/linux/debugobjects.h| 15 ++- kernel/rcu/update.c | 6 ++--- kernel/time/hrtimer.c | 18 ++--- kernel/time/timer.c | 30 +++--- kernel/workqueue.c | 20 +++ lib/debugobjects.c | 45 - lib/percpu_counter.c| 6 ++--- 8 files changed, 84 insertions(+), 82 deletions(-) -- 2.7.4
[PATCH 1/7] debugobjects: make fixup functions return bool instead of int
From: "Du, Changbin" The object debugging infrastructure core provides some fixup callbacks for the subsystem who use it. These callbacks are called from the debug code whenever a problem in debug_object_init is detected. And debugobjects core suppose them returns 1 when the fixup was successful, otherwise 0. So the return type is boolean. A bad thing is that debug_object_fixup use the return value for arithmetic operation. It confused me that what is the reall return type. Reading over the whole code, I found some place do use the return value incorrectly(see next patch). So why use bool type instead? Signed-off-by: Du, Changbin --- include/linux/debugobjects.h | 15 --- lib/debugobjects.c | 43 +-- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/include/linux/debugobjects.h b/include/linux/debugobjects.h index 98ffcbd..a899f10 100644 --- a/include/linux/debugobjects.h +++ b/include/linux/debugobjects.h @@ -39,7 +39,8 @@ struct debug_obj { * @debug_hint:function returning address, which have associated * kernel symbol, to allow identify the object * @fixup_init:fixup function, which is called when the init check - * fails + * fails. All fixup functions must return true if fixup + * was successful, otherwise return false * @fixup_activate:fixup function, which is called when the activate check * fails * @fixup_destroy: fixup function, which is called when the destroy check @@ -51,12 +52,12 @@ struct debug_obj { */ struct debug_obj_descr { const char *name; - void *(*debug_hint) (void *addr); - int (*fixup_init) (void *addr, enum debug_obj_state state); - int (*fixup_activate) (void *addr, enum debug_obj_state state); - int (*fixup_destroy)(void *addr, enum debug_obj_state state); - int (*fixup_free) (void *addr, enum debug_obj_state state); - int (*fixup_assert_init)(void *addr, enum debug_obj_state state); + void *(*debug_hint)(void *addr); + bool (*fixup_init)(void *addr, enum debug_obj_state state); + bool (*fixup_activate)(void *addr, enum debug_obj_state state); + bool (*fixup_destroy)(void *addr, enum debug_obj_state state); + bool (*fixup_free)(void *addr, enum debug_obj_state state); + bool (*fixup_assert_init)(void *addr, enum debug_obj_state state); }; #ifdef CONFIG_DEBUG_OBJECTS diff --git a/lib/debugobjects.c b/lib/debugobjects.c index 519b5a1..a9cee16 100644 --- a/lib/debugobjects.c +++ b/lib/debugobjects.c @@ -269,16 +269,15 @@ static void debug_print_object(struct debug_obj *obj, char *msg) * Try to repair the damage, so we have a better chance to get useful * debug output. */ -static int -debug_object_fixup(int (*fixup)(void *addr, enum debug_obj_state state), +static bool +debug_object_fixup(bool (*fixup)(void *addr, enum debug_obj_state state), void * addr, enum debug_obj_state state) { - int fixed = 0; - - if (fixup) - fixed = fixup(addr, state); - debug_objects_fixups += fixed; - return fixed; + if (fixup && fixup(addr, state)) { + debug_objects_fixups++; + return true; + } + return false; } static void debug_object_is_on_stack(void *addr, int onstack) @@ -797,7 +796,7 @@ static __initdata struct debug_obj_descr descr_type_test; * fixup_init is called when: * - an active object is initialized */ -static int __init fixup_init(void *addr, enum debug_obj_state state) +static bool __init fixup_init(void *addr, enum debug_obj_state state) { struct self_test *obj = addr; @@ -805,9 +804,9 @@ static int __init fixup_init(void *addr, enum debug_obj_state state) case ODEBUG_STATE_ACTIVE: debug_object_deactivate(obj, &descr_type_test); debug_object_init(obj, &descr_type_test); - return 1; + return true; default: - return 0; + return false; } } @@ -816,7 +815,7 @@ static int __init fixup_init(void *addr, enum debug_obj_state state) * - an active object is activated * - an unknown object is activated (might be a statically initialized object) */ -static int __init fixup_activate(void *addr, enum debug_obj_state state) +static bool __init fixup_activate(void *addr, enum debug_obj_state state) { struct self_test *obj = addr; @@ -825,17 +824,17 @@ static int __init fixup_activate(void *addr, enum debug_obj_state state) if (obj->static_init == 1) { debug_object_init(obj, &descr_type_test); debug_object_activate(obj, &descr_type_test); - return 0; + return false; }
[PATCH 4/7] timer: update debugobjects fixup callbacks return type
From: "Du, Changbin" Update the return type to use bool instead of int, corresponding to cheange (debugobjects: make fixup functions return bool instead of int). Signed-off-by: Du, Changbin --- kernel/time/hrtimer.c | 18 +- kernel/time/timer.c | 30 +++--- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c index fa0b983..f962a58 100644 --- a/kernel/time/hrtimer.c +++ b/kernel/time/hrtimer.c @@ -334,7 +334,7 @@ static void *hrtimer_debug_hint(void *addr) * fixup_init is called when: * - an active object is initialized */ -static int hrtimer_fixup_init(void *addr, enum debug_obj_state state) +static bool hrtimer_fixup_init(void *addr, enum debug_obj_state state) { struct hrtimer *timer = addr; @@ -342,9 +342,9 @@ static int hrtimer_fixup_init(void *addr, enum debug_obj_state state) case ODEBUG_STATE_ACTIVE: hrtimer_cancel(timer); debug_object_init(timer, &hrtimer_debug_descr); - return 1; + return true; default: - return 0; + return false; } } @@ -353,19 +353,19 @@ static int hrtimer_fixup_init(void *addr, enum debug_obj_state state) * - an active object is activated * - an unknown object is activated (might be a statically initialized object) */ -static int hrtimer_fixup_activate(void *addr, enum debug_obj_state state) +static bool hrtimer_fixup_activate(void *addr, enum debug_obj_state state) { switch (state) { case ODEBUG_STATE_NOTAVAILABLE: WARN_ON_ONCE(1); - return 0; + return false; case ODEBUG_STATE_ACTIVE: WARN_ON(1); default: - return 0; + return false; } } @@ -373,7 +373,7 @@ static int hrtimer_fixup_activate(void *addr, enum debug_obj_state state) * fixup_free is called when: * - an active object is freed */ -static int hrtimer_fixup_free(void *addr, enum debug_obj_state state) +static bool hrtimer_fixup_free(void *addr, enum debug_obj_state state) { struct hrtimer *timer = addr; @@ -381,9 +381,9 @@ static int hrtimer_fixup_free(void *addr, enum debug_obj_state state) case ODEBUG_STATE_ACTIVE: hrtimer_cancel(timer); debug_object_free(timer, &hrtimer_debug_descr); - return 1; + return true; default: - return 0; + return false; } } diff --git a/kernel/time/timer.c b/kernel/time/timer.c index 73164c3..be33481 100644 --- a/kernel/time/timer.c +++ b/kernel/time/timer.c @@ -493,7 +493,7 @@ static void *timer_debug_hint(void *addr) * fixup_init is called when: * - an active object is initialized */ -static int timer_fixup_init(void *addr, enum debug_obj_state state) +static bool timer_fixup_init(void *addr, enum debug_obj_state state) { struct timer_list *timer = addr; @@ -501,9 +501,9 @@ static int timer_fixup_init(void *addr, enum debug_obj_state state) case ODEBUG_STATE_ACTIVE: del_timer_sync(timer); debug_object_init(timer, &timer_debug_descr); - return 1; + return true; default: - return 0; + return false; } } @@ -518,7 +518,7 @@ static void stub_timer(unsigned long data) * - an active object is activated * - an unknown object is activated (might be a statically initialized object) */ -static int timer_fixup_activate(void *addr, enum debug_obj_state state) +static bool timer_fixup_activate(void *addr, enum debug_obj_state state) { struct timer_list *timer = addr; @@ -534,18 +534,18 @@ static int timer_fixup_activate(void *addr, enum debug_obj_state state) timer->entry.next == TIMER_ENTRY_STATIC) { debug_object_init(timer, &timer_debug_descr); debug_object_activate(timer, &timer_debug_descr); - return 0; + return false; } else { setup_timer(timer, stub_timer, 0); - return 1; + return true; } - return 0; + return false; case ODEBUG_STATE_ACTIVE: WARN_ON(1); default: - return 0; + return false; } } @@ -553,7 +553,7 @@ static int timer_fixup_activate(void *addr, enum debug_obj_state state) * fixup_free is called when: * - an active object is freed */ -static int timer_fixup_free(void *addr, enum debug_obj_state state) +static bool timer_fixup_free(void *addr, enum debug_obj_state state) { struct timer_list *timer = addr; @@ -561,9 +561,9 @@ static int timer_fixup_free(void *addr, enum debug_obj_state state) ca
[PATCH 5/7] rcu: update debugobjects fixup callbacks return type
From: "Du, Changbin" Update the return type to use bool instead of int, corresponding to cheange (debugobjects: make fixup functions return bool instead of int). Signed-off-by: Du, Changbin --- kernel/rcu/update.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c index ca828b4..66b75c3 100644 --- a/kernel/rcu/update.c +++ b/kernel/rcu/update.c @@ -386,7 +386,7 @@ void destroy_rcu_head(struct rcu_head *head) * - an unknown object is activated (might be a statically initialized object) * Activation is performed internally by call_rcu(). */ -static int rcuhead_fixup_activate(void *addr, enum debug_obj_state state) +static bool rcuhead_fixup_activate(void *addr, enum debug_obj_state state) { struct rcu_head *head = addr; @@ -399,9 +399,9 @@ static int rcuhead_fixup_activate(void *addr, enum debug_obj_state state) */ debug_object_init(head, &rcuhead_debug_descr); debug_object_activate(head, &rcuhead_debug_descr); - return 0; + return false; default: - return 1; + return true; } } -- 2.7.4
[PATCH 7/7] Documentation: update debugobjects doc
From: "Du, Changbin" Update documentation creangponding to change(debugobjects: make fixup functions return bool instead of int). Signed-off-by: Du, Changbin --- Documentation/DocBook/debugobjects.tmpl | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/Documentation/DocBook/debugobjects.tmpl b/Documentation/DocBook/debugobjects.tmpl index 24979f6..7e4f34f 100644 --- a/Documentation/DocBook/debugobjects.tmpl +++ b/Documentation/DocBook/debugobjects.tmpl @@ -316,8 +316,8 @@ - The function returns 1 when the fixup was successful, - otherwise 0. The return value is used to update the + The function returns true when the fixup was successful, + otherwise false. The return value is used to update the statistics. @@ -341,8 +341,8 @@ - The function returns 1 when the fixup was successful, - otherwise 0. The return value is used to update the + The function returns true when the fixup was successful, + otherwise false. The return value is used to update the statistics. @@ -359,7 +359,8 @@ statically initialized object or not. In case it is it calls debug_object_init() and debug_object_activate() to make the object known to the tracker and marked active. In this case - the function should return 0 because this is not a real fixup. + the function should return false because this is not a real + fixup. @@ -376,8 +377,8 @@ - The function returns 1 when the fixup was successful, - otherwise 0. The return value is used to update the + The function returns true when the fixup was successful, + otherwise false. The return value is used to update the statistics. @@ -397,8 +398,8 @@ - The function returns 1 when the fixup was successful, - otherwise 0. The return value is used to update the + The function returns true when the fixup was successful, + otherwise false. The return value is used to update the statistics. @@ -414,8 +415,8 @@ debug bucket. - The function returns 1 when the fixup was successful, - otherwise 0. The return value is used to update the + The function returns true when the fixup was successful, + otherwise false. The return value is used to update the statistics. @@ -427,7 +428,8 @@ case. The fixup function should check if this is a legitimate case of a statically initialized object or not. In this case only debug_object_init() should be called to make the object known to - the tracker. Then the function should return 0 because this is not + the tracker. Then the function should return false because this + is not a real fixup. -- 2.7.4
Re: [PATCH 9/9] arm64: dts: rockchip: move the rk3368 thermal data into rk3368.dtsi
Am Montag, 18. April 2016, 11:36:01 schrieb Caesar Wang: > In order to be standard to manage for rockchip SoCs, move the thermal > data into rk3368 dtsi, we needn't to add a new file for thermal. > > Fixes commit f990238f859e > ("arm64: dts: rockchip: Add main thermal info to rk3368.dtsi") > > Signed-off-by: Caesar Wang > Cc: Zhang Rui > Cc: Eduardo Valentin > Cc: Heiko Stuebner I'm pondering this change for some days now and am still undecided :-) On the one hand you're right ... this doesn't need to be its own file on the other hand it creates churn, but I'm inclined to pick it up. Can you at least provide another patch doing the same for rk3288, so that we really standardize on one form for Rockchip SoCs? Thanks Heiko > > --- > > arch/arm64/boot/dts/rockchip/rk3368-thermal.dtsi | 112 > --- arch/arm64/boot/dts/rockchip/rk3368.dtsi | > 66 - 2 files changed, 65 insertions(+), 113 deletions(-) > delete mode 100644 arch/arm64/boot/dts/rockchip/rk3368-thermal.dtsi > > diff --git a/arch/arm64/boot/dts/rockchip/rk3368-thermal.dtsi > b/arch/arm64/boot/dts/rockchip/rk3368-thermal.dtsi deleted file mode 100644 > index a10010f..000 > --- a/arch/arm64/boot/dts/rockchip/rk3368-thermal.dtsi > +++ /dev/null > @@ -1,112 +0,0 @@ > -/* > - * Device Tree Source for RK3368 SoC thermal > - * > - * Copyright (c) 2015, Fuzhou Rockchip Electronics Co., Ltd > - * Caesar Wang > - * > - * 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 as > - * published by the Free Software Foundation; either version 2 of the > - * License, or (at your option) any later version. > - * > - * 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. > - */ > - > -#include > - > -cpu_thermal: cpu_thermal { > - polling-delay-passive = <100>; /* milliseconds */ > - polling-delay = <5000>; /* milliseconds */ > - > - thermal-sensors = <&tsadc 0>; > - > - trips { > - cpu_alert0: cpu_alert0 { > - temperature = <75000>; /* millicelsius */ > - hysteresis = <2000>; /* millicelsius */ > - type = "passive"; > - }; > - cpu_alert1: cpu_alert1 { > - temperature = <8>; /* millicelsius */ > - hysteresis = <2000>; /* millicelsius */ > - type = "passive"; > - }; > - cpu_crit: cpu_crit { > - temperature = <95000>; /* millicelsius */ > - hysteresis = <2000>; /* millicelsius */ > - type = "critical"; > - }; > - }; > - > - cooling-maps { > - map0 { > - trip = <&cpu_alert0>; > - cooling-device = > - <&cpu_b0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; > - }; > - map1 { > - trip = <&cpu_alert1>; > - cooling-device = > - <&cpu_l0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; > - }; > - }; > -}; > - > -gpu_thermal: gpu_thermal { > - polling-delay-passive = <100>; /* milliseconds */ > - polling-delay = <5000>; /
[PATCH 6/7] percpu_counter: update debugobjects fixup callbacks return type
From: "Du, Changbin" Update the return type to use bool instead of int, corresponding to cheange (debugobjects: make fixup functions return bool instead of int). Signed-off-by: Du, Changbin --- lib/percpu_counter.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/percpu_counter.c b/lib/percpu_counter.c index f051d69..72d3611 100644 --- a/lib/percpu_counter.c +++ b/lib/percpu_counter.c @@ -19,7 +19,7 @@ static DEFINE_SPINLOCK(percpu_counters_lock); static struct debug_obj_descr percpu_counter_debug_descr; -static int percpu_counter_fixup_free(void *addr, enum debug_obj_state state) +static bool percpu_counter_fixup_free(void *addr, enum debug_obj_state state) { struct percpu_counter *fbc = addr; @@ -27,9 +27,9 @@ static int percpu_counter_fixup_free(void *addr, enum debug_obj_state state) case ODEBUG_STATE_ACTIVE: percpu_counter_destroy(fbc); debug_object_free(fbc, &percpu_counter_debug_descr); - return 1; + return true; default: - return 0; + return false; } } -- 2.7.4
[PATCH 3/7] workqueue: update debugobjects fixup callbacks return type
From: "Du, Changbin" Update the return type to use bool instead of int, corresponding to cheange (debugobjects: make fixup functions return bool instead of int) Signed-off-by: Du, Changbin --- kernel/workqueue.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 2232ae3..c3c7e69 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -437,7 +437,7 @@ static void *work_debug_hint(void *addr) * fixup_init is called when: * - an active object is initialized */ -static int work_fixup_init(void *addr, enum debug_obj_state state) +static bool work_fixup_init(void *addr, enum debug_obj_state state) { struct work_struct *work = addr; @@ -445,9 +445,9 @@ static int work_fixup_init(void *addr, enum debug_obj_state state) case ODEBUG_STATE_ACTIVE: cancel_work_sync(work); debug_object_init(work, &work_debug_descr); - return 1; + return true; default: - return 0; + return false; } } @@ -456,7 +456,7 @@ static int work_fixup_init(void *addr, enum debug_obj_state state) * - an active object is activated * - an unknown object is activated (might be a statically initialized object) */ -static int work_fixup_activate(void *addr, enum debug_obj_state state) +static bool work_fixup_activate(void *addr, enum debug_obj_state state) { struct work_struct *work = addr; @@ -471,16 +471,16 @@ static int work_fixup_activate(void *addr, enum debug_obj_state state) if (test_bit(WORK_STRUCT_STATIC_BIT, work_data_bits(work))) { debug_object_init(work, &work_debug_descr); debug_object_activate(work, &work_debug_descr); - return 0; + return false; } WARN_ON_ONCE(1); - return 0; + return false; case ODEBUG_STATE_ACTIVE: WARN_ON(1); default: - return 0; + return false; } } @@ -488,7 +488,7 @@ static int work_fixup_activate(void *addr, enum debug_obj_state state) * fixup_free is called when: * - an active object is freed */ -static int work_fixup_free(void *addr, enum debug_obj_state state) +static bool work_fixup_free(void *addr, enum debug_obj_state state) { struct work_struct *work = addr; @@ -496,9 +496,9 @@ static int work_fixup_free(void *addr, enum debug_obj_state state) case ODEBUG_STATE_ACTIVE: cancel_work_sync(work); debug_object_free(work, &work_debug_descr); - return 1; + return true; default: - return 0; + return false; } } -- 2.7.4
[PATCH 2/7] debugobjects: correct the usage of fixup call results
From: "Du, Changbin" If debug_object_fixup() return non-zero when problem has been fixed. But the code got it backwards, it taks 0 as fixup successfully. So fix it. Signed-off-by: Du, Changbin --- lib/debugobjects.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/debugobjects.c b/lib/debugobjects.c index a9cee16..2f07c8c 100644 --- a/lib/debugobjects.c +++ b/lib/debugobjects.c @@ -415,7 +415,7 @@ int debug_object_activate(void *addr, struct debug_obj_descr *descr) state = obj->state; raw_spin_unlock_irqrestore(&db->lock, flags); ret = debug_object_fixup(descr->fixup_activate, addr, state); - return ret ? -EINVAL : 0; + return ret ? 0 : -EINVAL; case ODEBUG_STATE_DESTROYED: debug_print_object(obj, "activate"); -- 2.7.4
Re: [PATCH V2 06/14] irqdomain: Don't set type when mapping an IRQ
Hi Jon, On 21/04/16 16:45, Jon Hunter wrote: > > On 20/04/16 12:03, Jon Hunter wrote: >> Some IRQ chips, such as GPIO controllers or secondary level interrupt >> controllers, may require require additional runtime power management >> control to ensure they are accessible. For such IRQ chips, it makes sense >> to enable the IRQ chip when interrupts are requested and disabled them >> again once all interrupts have been freed. >> >> When mapping an IRQ, the IRQ type settings are read and then programmed. >> The mapping of the IRQ happens before the IRQ is requested and so the >> programming of the type settings occurs before the IRQ is requested. This >> is a problem for IRQ chips that require additional power management >> control because they may not be accessible yet. Therefore, when mapping >> the IRQ, don't program the type settings, just save them and then program >> these saved settings when the IRQ is requested (so long as if they are not >> overridden via the call to request the IRQ). >> >> Add a stub function for irq_domain_free_irqs() to avoid any compilation >> errors when CONFIG_IRQ_DOMAIN_HIERARCHY is not selected. >> >> Signed-off-by: Jon Hunter >> --- >> include/linux/irqdomain.h | 3 +++ >> kernel/irq/irqdomain.c| 17 + >> 2 files changed, 16 insertions(+), 4 deletions(-) > > [snip] > >> -/* Set type if specified and different than the current one */ >> -if (type != IRQ_TYPE_NONE && >> -type != irq_get_trigger_type(virq)) >> -irq_set_irq_type(virq, type); >> +irq_data = irq_get_irq_data(virq); >> +if (!irq_data) { >> +if (irq_domain_is_hierarchy(domain)) >> +irq_domain_free_irqs(virq, 1); >> +else >> +irq_dispose_mapping(virq); >> +return 0; >> +} >> + >> +/* Store trigger type */ >> +irqd_set_trigger_type(irq_data, type); >> + > > I appear to have missed another place for saving the irq type > which I had change in this version. Next time I will triple > check! Should have been ... > > > diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h > index 2aed04396210..fc66876d1965 100644 > --- a/include/linux/irqdomain.h > +++ b/include/linux/irqdomain.h > @@ -440,6 +440,9 @@ static inline int irq_domain_alloc_irqs(struct irq_domain > *domain, > return -1; > } > > +static inline void irq_domain_free_irqs(unsigned int virq, > + unsigned int nr_irqs) { } > + > static inline bool irq_domain_is_hierarchy(struct irq_domain *domain) > { > return false; > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c > index 3d6ef5527b71..46ecf5d468b2 100644 > --- a/kernel/irq/irqdomain.c > +++ b/kernel/irq/irqdomain.c > @@ -569,6 +569,7 @@ static void of_phandle_args_to_fwspec(struct > of_phandle_args *irq_data, > unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec) > { > struct irq_domain *domain; > + struct irq_data *irq_data; > irq_hw_number_t hwirq; > unsigned int type = IRQ_TYPE_NONE; > int virq; > @@ -619,7 +620,11 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec > *fwspec) > * it now and return the interrupt number. > */ > if (IRQ_TYPE_NONE == irq_get_trigger_type(virq)) { > - irq_set_irq_type(virq, type); > + irq_data = irq_get_irq_data(virq); > + if (!irq_data) > + return 0; Can you point out in which circumstances irq_data can be NULL? If we can lookup the interrupt in the domain, it'd better exist somewhere. Or am I missing something obvious? Thanks, M. -- Jazz is not dead. It just smells funny...
Re: [regression] linux318, linux41 - kernel stack is corrupted
On Fri, Apr 22, 2016 at 10:10:59AM +0200, Sebastian M. Bobrecki wrote: > W dniu 22.04.2016 o 09:55, Greg Kroah-Hartman pisze: > > On Fri, Apr 22, 2016 at 09:47:04AM +0200, Sebastian M. Bobrecki wrote: > > > Hi, > > > > > > I just hit the same with 4.1.22 on Gentoo. 4.1.21 are working fine. > > > > > > On 22.04.2016 at 08:46, Greg Kroah-Hartman wrote: > > > > ... > > > > You are going to have to be a bit more specific here... > > > > What is the oops message? How do you reproduce this? Does it also > > > > happen on 4.6-rc4? > > > > > > > > Can you run 'git bisect' to find the offending patch? > > > > > > > Greg have you seen screenshots linked by Philip? > > I saw no such screenshots in the email. > They are here: https://github.com/manjaro/packages-core/issues/36 Looks like an acpi thermal patch got backported incorrectly, again, 'git bisect' is going to help out the best here. thanks, greg k-h
Re: [PATCH 2/8] drm/udl: Change drm_fb_helper_sys_*() calls to sys_*()
On Thu, Apr 21, 2016 at 08:18:48PM +0200, Noralf Trønnes wrote: > > Den 21.04.2016 09:28, skrev Daniel Vetter: > >On Wed, Apr 20, 2016 at 08:15:30PM +0200, Noralf Trønnes wrote: > >>Den 20.04.2016 19:42, skrev Daniel Vetter: > >>>On Wed, Apr 20, 2016 at 05:25:23PM +0200, Noralf Trønnes wrote: > Now that drm_fb_helper gets deferred io support, the > drm_fb_helper_sys_{fillrect,copyarea,imageblit} functions will schedule > the worker that calls the deferred_io callback. This will break this > driver so use the sys_{fillrect,copyarea,imageblit} functions directly. > > Signed-off-by: Noralf Trønnes > >>>I think this intermediately breaks the build, if you disable fbdev > >>>support. That's now supported in the fbdev helpers core generically across > >>>all drivers. > >>> > >>>Not sure how to best fix this up, since the only way would be to squash > >>>these patches, plus generic deferred io plus the conversion patches for > >>>udl/qxl all into one. Tricky. > >>Yes you're right, I missed that. > >>How about this: > >>#ifdef CONFIG_FB > >> sys_fillrect(info, rect); > >>#endif > >> > >>The later patch will then remove this ugliness... > >Yeah I think we have to bite the bullet and take this temporary ugliness > >:( > > Turns out the #ifdef isn't necessary since FB is always selected. > > Both udl and qxl have this: > select DRM_KMS_HELPER > select DRM_KMS_FB_HELPER > > And then we have: > > config DRM_KMS_HELPER > tristate > depends on DRM > > config DRM_KMS_FB_HELPER > bool > depends on DRM_KMS_HELPER > select FB > ... > select FB_SYS_FILLRECT > select FB_SYS_COPYAREA > select FB_SYS_IMAGEBLIT Hm ... the thing that actually builds fbdev emulation is DRM_FBDEV_EMULATION, and you can disable that. Otoh the select FB stuff seems to be at the wrong level and probably should be moved. But indeed I tried doing this and it's an impossible config. I guess I need to type a patch to ditch all these selects from drivers ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 4/8] drm/fb-helper: Add fb_deferred_io support
On Thu, Apr 21, 2016 at 08:54:45PM +0200, Noralf Trønnes wrote: > > Den 20.04.2016 17:25, skrev Noralf Trønnes: > >This adds deferred io support if CONFIG_FB_DEFERRED_IO is enabled. > >Accumulated fbdev framebuffer changes are signaled using the callback > >(struct drm_framebuffer_funcs *)->dirty() > > > >The drm_fb_helper_sys_*() functions will accumulate changes and > >schedule fb_info.deferred_work _if_ fb_info.fbdefio is set. > >This worker is used by the deferred io mmap code to signal that it > >has been collecting page faults. The page faults and/or other changes > >are then merged into a drm_clip_rect and passed to the framebuffer > >dirty() function. > > > >The driver is responsible for setting up the fb_info.fbdefio structure > >and calling fb_deferred_io_init() using the provided callback: > >(struct fb_deferred_io).deferred_io = drm_fb_helper_deferred_io; > > > >Signed-off-by: Noralf Trønnes > >--- > > drivers/gpu/drm/drm_fb_helper.c | 119 > > +++- > > include/drm/drm_fb_helper.h | 15 + > > 2 files changed, 133 insertions(+), 1 deletion(-) > > > >diff --git a/drivers/gpu/drm/drm_fb_helper.c > >b/drivers/gpu/drm/drm_fb_helper.c > > [...] > > >+#ifdef CONFIG_FB_DEFERRED_IO > >+/** > >+ * drm_fb_helper_deferred_io() - (struct fb_deferred_io *)->deferred_io > >callback > >+ * function > >+ * > >+ * This function always runs in process context (struct delayed_work) and > >calls > >+ * the (struct drm_framebuffer_funcs)->dirty function with the collected > >+ * damage. There's no need to worry about the possibility that the > >fb_sys_*() > >+ * functions could be running in atomic context. They only schedule the > >+ * delayed worker which then calls this deferred_io callback. > >+ */ > >+void drm_fb_helper_deferred_io(struct fb_info *info, > >+ struct list_head *pagelist) > >+{ > >+struct drm_fb_helper *helper = info->par; > >+unsigned long start, end, min, max; > >+struct drm_clip_rect clip; > >+unsigned long flags; > >+struct page *page; > >+ > >+if (!helper->fb->funcs->dirty) > >+return; > >+ > >+spin_lock_irqsave(&helper->dirty_lock, flags); > >+clip = helper->dirty_clip; > >+drm_clip_rect_reset(&helper->dirty_clip); > >+spin_unlock_irqrestore(&helper->dirty_lock, flags); > >+ > >+min = ULONG_MAX; > >+max = 0; > >+list_for_each_entry(page, pagelist, lru) { > >+start = page->index << PAGE_SHIFT; > >+end = start + PAGE_SIZE - 1; > >+min = min(min, start); > >+max = max(max, end); > >+} > >+ > >+if (min < max) { > >+struct drm_clip_rect mmap_clip; > >+ > >+mmap_clip.x1 = 0; > >+mmap_clip.x2 = info->var.xres; > >+mmap_clip.y1 = min / info->fix.line_length; > >+mmap_clip.y2 = min_t(u32, max / info->fix.line_length, > >+ info->var.yres); > >+drm_clip_rect_merge(&clip, &mmap_clip, 1, 0, 0, 0); > >+} > >+ > >+if (!drm_clip_rect_is_empty(&clip)) > >+helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip, 1); > >+} > >+EXPORT_SYMBOL(drm_fb_helper_deferred_io); > > There is one thing I have wondered about when it comes to deferred io and > long run times for the defio worker with my displays: > > Userspace writes to some pages then the deferred io worker kicks off and > runs for 100ms holding the page list mutex. While this is happening, > userspace writes to a new page triggering a page_mkwrite. Now this > function has to wait for the mutex to be released. > > Who is actually waiting here and is there a problem that this can last > for 100ms? No idea at all - I haven't looked that closely at fbdev defio. But one reason we have an explicit ioctl in drm to flush out frontbuffer rendering is exactly that flushing could take some time, and should only be done once userspace has completed some rendering. Not right in the middle of an op. I guess fix up your userspace to use dumb drm fb + drm dirtyfb ioctl? Otherwise you'll get to improve fbdev defio, and fbdev is deprecated subsystem and a real horror show. I highly recommend against touching it ;-) Cheers, Daniel > > Excerpt from drivers/video/fbdev/core/fb_defio.c: > > /* vm_ops->page_mkwrite handler */ > static int fb_deferred_io_mkwrite(struct vm_area_struct *vma, > struct vm_fault *vmf) > { > ... > /* this is a callback we get when userspace first tries to > write to the page. we schedule a workqueue. that workqueue > will eventually mkclean the touched pages and execute the > deferred framebuffer IO. then if userspace touches a page > again, we repeat the same scheme */ > ... > /* protect against the workqueue changing the page list */ > mutex_lock(&fbdefio->lock); > ... > /* > * We want the page to remain locked from ->page_mkwrite until >
Re: [PATCH 0/8] Input: atmel_mxt_ts - output raw touch diagnostic data via V4L
Hi Nick, On 04/21/2016 11:31 AM, Nick Dyer wrote: > This is a series of patches to add diagnostic data support to the Atmel > maXTouch driver. It's a rewrite of the previous implementation which output > via > debugfs: it now uses a V4L2 device in a similar way to the sur40 driver. > > There are significant performance advantages to putting this code into the > driver. The algorithm for retrieving the data has been fairly consistent > across > a range of chips, with the exception of the mXT1386 series (see patch). > > We have a utility which can read the data and display it in a useful format: > https://github.com/ndyer/heatmap/commits/heatmap-v4l > > These patches are also available from > https://github.com/ndyer/linux/commits/diagnostic-v4l > > Any feedback appreciated. FYI: we're working on a new buffer type for meta data: https://patchwork.linuxtv.org/patch/33938/ https://patchwork.linuxtv.org/patch/33939/ This would be an excellent fit for you. I expect that this new feature would be merged soon (for 4.7 or 4.8 at the latest) since it looks all pretty good to me. So let's wait for this to be merged and then you can migrate to the new buffer type. Regards, Hans
Re: [PATCH v12 1/2] kernel.h: add u64_to_user_ptr()
On Thu, Apr 21, 2016 at 12:38:49PM -0300, Gustavo Padovan wrote: > From: Gustavo Padovan > > This function had copies in 3 different files. Unify them in kernel.h. > > Cc: Joe Perches > Cc: Andrew Morton > Cc: David Airlie > Cc: Daniel Vetter > Cc: Rob Clark > Signed-off-by: Gustavo Padovan Ack for i915 parts for merging through any suitable tree. But since this mostly touches drm I'd propose we pull this in through drm-misc. Joe? -Daniel > > --- > v2: add typecheck() (comment from Maarten Lankhorst) > > v3: make u64_to_user_ptr() a macro (comment from Joe Perches) > --- > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 11 +++ > drivers/gpu/drm/i915/i915_drv.h | 5 - > drivers/gpu/drm/i915/i915_gem.c | 14 +++--- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 14 +++--- > drivers/gpu/drm/msm/msm_gem_submit.c | 11 +++ > include/linux/kernel.h | 7 +++ > 6 files changed, 27 insertions(+), 35 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > index 236ada9..afdd55d 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > @@ -28,11 +28,6 @@ > #define BO_LOCKED 0x4000 > #define BO_PINNED 0x2000 > > -static inline void __user *to_user_ptr(u64 address) > -{ > - return (void __user *)(uintptr_t)address; > -} > - > static struct etnaviv_gem_submit *submit_create(struct drm_device *dev, > struct etnaviv_gpu *gpu, size_t nr) > { > @@ -347,21 +342,21 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, > void *data, > cmdbuf->exec_state = args->exec_state; > cmdbuf->ctx = file->driver_priv; > > - ret = copy_from_user(bos, to_user_ptr(args->bos), > + ret = copy_from_user(bos, u64_to_user_ptr(args->bos), >args->nr_bos * sizeof(*bos)); > if (ret) { > ret = -EFAULT; > goto err_submit_cmds; > } > > - ret = copy_from_user(relocs, to_user_ptr(args->relocs), > + ret = copy_from_user(relocs, u64_to_user_ptr(args->relocs), >args->nr_relocs * sizeof(*relocs)); > if (ret) { > ret = -EFAULT; > goto err_submit_cmds; > } > > - ret = copy_from_user(stream, to_user_ptr(args->stream), > + ret = copy_from_user(stream, u64_to_user_ptr(args->stream), >args->stream_size); > if (ret) { > ret = -EFAULT; > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 1048093..bb624cc 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3576,11 +3576,6 @@ static inline i915_reg_t i915_vgacntrl_reg(struct > drm_device *dev) > return VGACNTRL; > } > > -static inline void __user *to_user_ptr(u64 address) > -{ > - return (void __user *)(uintptr_t)address; > -} > - > static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m) > { > unsigned long j = msecs_to_jiffies(m); > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index dabc089..2889716 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -324,7 +324,7 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj, > { > struct drm_device *dev = obj->base.dev; > void *vaddr = obj->phys_handle->vaddr + args->offset; > - char __user *user_data = to_user_ptr(args->data_ptr); > + char __user *user_data = u64_to_user_ptr(args->data_ptr); > int ret = 0; > > /* We manually control the domain here and pretend that it > @@ -605,7 +605,7 @@ i915_gem_shmem_pread(struct drm_device *dev, > int needs_clflush = 0; > struct sg_page_iter sg_iter; > > - user_data = to_user_ptr(args->data_ptr); > + user_data = u64_to_user_ptr(args->data_ptr); > remain = args->size; > > obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj); > @@ -692,7 +692,7 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data, > return 0; > > if (!access_ok(VERIFY_WRITE, > -to_user_ptr(args->data_ptr), > +u64_to_user_ptr(args->data_ptr), > args->size)) > return -EFAULT; > > @@ -783,7 +783,7 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev, > if (ret) > goto out_unpin; > > - user_data = to_user_ptr(args->data_ptr); > + user_data = u64_to_user_ptr(args->data_ptr); > remain = args->size; > > offset = i915_gem_obj_ggtt_offset(obj) + args->offset; > @@ -907,7 +907,7 @@ i915_gem_shmem_pwrite(struct drm_device *dev, > int needs_clflush_before = 0; > struct sg_page_iter sg_iter; > > - user_data = to_user_ptr(args->data_ptr); > + us
[PATCH 3/4] soc: qcom: spm: Use const and __initconst for qcom_cpuidle_ops
From: Jisheng Zhang The qcom_cpuidle_ops structures is not over-written, so add "const" qualifier and replace __initdata with __initconst. Signed-off-by: Jisheng Zhang Signed-off-by: Daniel Lezcano Acked-by: Andy Gross --- drivers/soc/qcom/spm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c index 5548a31..1fcbb22 100644 --- a/drivers/soc/qcom/spm.c +++ b/drivers/soc/qcom/spm.c @@ -274,7 +274,7 @@ check_spm: return per_cpu(cpu_spm_drv, cpu) ? 0 : -ENXIO; } -static struct cpuidle_ops qcom_cpuidle_ops __initdata = { +static const struct cpuidle_ops qcom_cpuidle_ops __initconst = { .suspend = qcom_idle_enter, .init = qcom_cpuidle_init, }; -- 1.9.1
[PATCH 4/4] drivers: firmware: psci: use const and __initconst for psci_cpuidle_ops
From: Jisheng Zhang The psci_cpuidle_ops structures is not over-written, so add "const" qualifier and replace __initdata with __initconst. Acked-by: Lorenzo Pieralisi Signed-off-by: Jisheng Zhang Signed-off-by: Daniel Lezcano --- drivers/firmware/psci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c index 11bfee8..eb8134a 100644 --- a/drivers/firmware/psci.c +++ b/drivers/firmware/psci.c @@ -355,7 +355,7 @@ int psci_cpu_suspend_enter(unsigned long index) /* ARM specific CPU idle operations */ #ifdef CONFIG_ARM -static struct cpuidle_ops psci_cpuidle_ops __initdata = { +static const struct cpuidle_ops psci_cpuidle_ops __initconst = { .suspend = psci_cpu_suspend_enter, .init = psci_dt_cpu_init_idle, }; -- 1.9.1
[PATCH 2/4] ARM: cpuidle: constify return value of arm_cpuidle_get_ops()
From: Jisheng Zhang arm_cpuidle_read_ops() just copies '*ops' to cpuidle_ops[cpu], so the structure '*ops' is not modified at all. The comment is also updated accordingly. Signed-off-by: Jisheng Zhang Signed-off-by: Daniel Lezcano --- arch/arm/kernel/cpuidle.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c index 703926e..a44b268e 100644 --- a/arch/arm/kernel/cpuidle.c +++ b/arch/arm/kernel/cpuidle.c @@ -70,7 +70,7 @@ int arm_cpuidle_suspend(int index) * * Returns a struct cpuidle_ops pointer, NULL if not found. */ -static struct cpuidle_ops *__init arm_cpuidle_get_ops(const char *method) +static const struct cpuidle_ops *__init arm_cpuidle_get_ops(const char *method) { struct of_cpuidle_method *m = __cpuidle_method_of_table; @@ -88,7 +88,7 @@ static struct cpuidle_ops *__init arm_cpuidle_get_ops(const char *method) * * Get the method name defined in the 'enable-method' property, retrieve the * associated cpuidle_ops and do a struct copy. This copy is needed because all - * cpuidle_ops are tagged __initdata and will be unloaded after the init + * cpuidle_ops are tagged __initconst and will be unloaded after the init * process. * * Return 0 on sucess, -ENOENT if no 'enable-method' is defined, -EOPNOTSUPP if @@ -97,7 +97,7 @@ static struct cpuidle_ops *__init arm_cpuidle_get_ops(const char *method) static int __init arm_cpuidle_read_ops(struct device_node *dn, int cpu) { const char *enable_method; - struct cpuidle_ops *ops; + const struct cpuidle_ops *ops; enable_method = of_get_property(dn, "enable-method", NULL); if (!enable_method) -- 1.9.1
[PATCH 1/4] ARM: cpuidle: add const qualifier to cpuidle_ops member in structures
From: Jisheng Zhang The core code does not modify smp_operations structures. To clarify it, this patch adds 'const' qualifier to the 'ops' member of struct of_cpuidle_method. This change allows each arm cpuidle code to add 'const' qualifier to its cpuidle_ops structure. Signed-off-by: Jisheng Zhang Signed-off-by: Daniel Lezcano --- arch/arm/include/asm/cpuidle.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h index 3848259..baefe1d 100644 --- a/arch/arm/include/asm/cpuidle.h +++ b/arch/arm/include/asm/cpuidle.h @@ -36,7 +36,7 @@ struct cpuidle_ops { struct of_cpuidle_method { const char *method; - struct cpuidle_ops *ops; + const struct cpuidle_ops *ops; }; #define CPUIDLE_METHOD_OF_DECLARE(name, _method, _ops) \ -- 1.9.1
Re: [PATCH 1/7] debugobjects: make fixup functions return bool instead of int
On Fri, 22 Apr 2016, changbin...@intel.com wrote: > From: "Du, Changbin" > > The object debugging infrastructure core provides some fixup callbacks > for the subsystem who use it. These callbacks are called from the debug > code whenever a problem in debug_object_init is detected. And > debugobjects core suppose them returns 1 when the fixup was successful, > otherwise 0. So the return type is boolean. > > A bad thing is that debug_object_fixup use the return value for > arithmetic operation. It confused me that what is the reall return What's bad about that? The fact that it's used for arithmethic operation or that it confused you? > Reading over the whole code, I found some place do use the return value > incorrectly(see next patch). So why use bool type instead? Patches which fix a problem need to come first not in the middle of a revamp series. > + bool (*fixup_init)(void *addr, enum debug_obj_state state); > + bool (*fixup_activate)(void *addr, enum debug_obj_state state); > + bool (*fixup_destroy)(void *addr, enum debug_obj_state state); > + bool (*fixup_free)(void *addr, enum debug_obj_state state); > + bool (*fixup_assert_init)(void *addr, enum debug_obj_state state); So this change will introduce a gazillion of compile warnings because the callbacks in the various usage sites are still having 'int' return type. Thanks, tglx
Re: [tip:x86/asm] x86/entry: Rename is_{ia32,x32}_task() to in_{ia32,x32}_syscall()
* Andy Lutomirski wrote: > On Tue, Apr 19, 2016 at 4:15 AM, Ingo Molnar wrote: > > > > * tip-bot for Dmitry Safonov wrote: > > > >> Commit-ID: abfb9498ee1327f534df92a7ecaea81a85913bae > >> Gitweb: > >> http://git.kernel.org/tip/abfb9498ee1327f534df92a7ecaea81a85913bae > >> Author: Dmitry Safonov > >> AuthorDate: Mon, 18 Apr 2016 16:43:43 +0300 > >> Committer: Ingo Molnar > >> CommitDate: Tue, 19 Apr 2016 10:44:52 +0200 > >> > >> x86/entry: Rename is_{ia32,x32}_task() to in_{ia32,x32}_syscall() > > > > Btw., I'm not _entirely_ happy about the 'IA32' name, but went with this > > name for > > lack of a better alternative. > > > > So we have 4 system call modes: > > > > - 64-bit native > > - 32-bit addresses with 64-bit arguments (x32) > > - 32-bit compat syscall (x86-32 compatibility on x86-64) > > - 32-bit native > > > > and we have 2 bits of data that are per system call properties: > > > > - TS_COMPAT in thread_info->status is set/cleared dynamically by the compat > >syscall entry code > > > > - a high bit in pt_regs->orig_ax tells us whether it's an x32 system call. > > > > So I'd suggest the following renames to harmonize these concepts: > > > > - CONFIG_IA32_EMULATION => CONFIG_X86_32_ABI > >this lines up nicely with: CONFIG_X86_X32_ABI > > I think I'd prefer a different interpretation: CONFIG_X86_32_ABI is > set if CONFIG_IA32_EMULATION is set *or* CONFIG_X86_32 is set. There > is a lot of code that manually looks for that, because what it > actually cares about is "do we support 32-bit syscalls". Also, with > the syscall cleanups I've been doing, a lot of the code is shared > between native 32-bit and 32-on-64 compat, so the distinction between > those two modes is slowly shrinking. > > in_ia32_syscall() is consistent with that idea: it returns true on > native 32-bit kernels. Ok, so how about: - rename CONFIG_IA32_EMULATION => CONFIG_X86_32_COMPAT - introduce CONFIG_X86_32_ABI to separate the 'convenience' functionality of CONFIG_IA32_EMULATION from the ABI meaning. - rename is_ia32_syscall() -> is_x86_32_syscall() - rename is_x32_syscall() -> is_x86_x32_syscall() - is_compat_syscall() remains as-is. ? In the long run I'd like to get rid of two naming variants: - Fix all names that use 'IA32' that refer to 32-bit compat functionality, and only name the things 'IA32' that are truly Intel specific. - Fix all names that use 'emulation' when they really refer to 32-bit compat. We don't actually emulate anything, we are just calling convention compatible with very little overhead. The CPU is a fully 32-bit/64-bit dual mode hardware, there's nothing that is emulated. Calling it IA32_EMULATION was a misnomer. Thanks, Ingo
Re: linux-next: build failure after merge of the sound-asoc tree
Am Freitag, den 22.04.2016, 12:03 +1000 schrieb Stephen Rothwell: > Hi all, > > After merging the sound-asoc tree, today's linux-next build (x86_64 > allmodconfig) failed like this: > > In file included from include/linux/notifier.h:13:0, > from include/linux/memory_hotplug.h:6, > from include/linux/mmzone.h:744, > from include/linux/gfp.h:5, > from include/linux/kmod.h:22, > from include/linux/module.h:13, > from sound/soc/codecs/hdmi-codec.c:15: > sound/soc/codecs/hdmi-codec.c: In function 'hdmi_eld_ctl_get': > sound/soc/codecs/hdmi-codec.c:68:17: error: 'struct hdmi_codec_priv' has no > member named 'eld_lock' > mutex_lock(&hcp->eld_lock); > ^ > include/linux/mutex.h:146:44: note: in definition of macro 'mutex_lock' > #define mutex_lock(lock) mutex_lock_nested(lock, 0) > ^ > sound/soc/codecs/hdmi-codec.c:70:19: error: 'struct hdmi_codec_priv' has no > member named 'eld_lock' > mutex_unlock(&hcp->eld_lock); >^ > > Caused by commit > > 81151cfb6bfe ("ASoC: hdmi-codec: Add ELD control") > > I have used the sound-asoc tree from next-20160421 for today. Sorry, the mutex_lock slipped through when reordering the patches. That should be part of the "Use HDMI notifications to add jack support" patch instead. regards Philipp 8< From: Philipp Zabel Subject: [PATCH] fixup! ASoC: hdmi-codec: Add ELD control --- sound/soc/codecs/hdmi-codec.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c index c78333b..8e36e88 100644 --- a/sound/soc/codecs/hdmi-codec.c +++ b/sound/soc/codecs/hdmi-codec.c @@ -65,9 +65,7 @@ static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol, struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component); - mutex_lock(&hcp->eld_lock); memcpy(ucontrol->value.bytes.data, hcp->eld, sizeof(hcp->eld)); - mutex_unlock(&hcp->eld_lock); return 0; } -- 2.8.0.rc3
Re: [PATCH V2 03/14] irqchip: Mask the non-type/sense bits when translating an IRQ
On 20/04/16 12:03, Jon Hunter wrote: > The firmware parameter that contains the IRQ sense bits may also contain > other data. When return the IRQ type, bits outside of these sense bits > should be masked. If these bits are not masked and > irq_create_fwspec_mapping() is called to map an IRQ, then the comparison > of the type returned from irq_domain_translate() will never match > that returned by irq_get_trigger_type() (because this function masks the > none sense bits) and so we will always call irq_set_irq_type() to program > the type even if it was not really necessary. > > Currently, the downside to this is unnecessarily re-programmming the type > but nevertheless this should be avoided. > > The Tegra LIC and TI Crossbar irqchips all have client instances (from > reviewing the device-tree sources) where bits outside the IRQ sense bits > are set, but do not mask these bits. Therefore, ensure these bits are > masked for these irqchips. > > Signed-off-by: Jon Hunter Acked-by: Marc Zyngier M. -- Jazz is not dead. It just smells funny...
Re: [PATCH v11 3/3] printk: make printk.synchronous param rw
On Fri 2016-04-22 10:28:26, Sergey Senozhatsky wrote: > Hello, > > On (04/21/16 13:07), Petr Mladek wrote: > [..] > > Please, what is the purpose of "printk_initcall_done" then? If I get > > this correctly, it will always be true when printk_sync_set() is > > called. > > well, this is a bit ugly, yes. kernel_param_ops defines ->set callback > as printk_sync_set(). and this ->set callback is getting called from 2 > different paths (but it's really about underlying __init_printk_kthread()). > > __init_printk_kthread() can be executed from: Ah, I see and feel shame. It is actually explained in the comment above printk_initcall_done declaration. Well, the explanation confused me a bit ;-) I suggest to change it sligtly: /* * printk_sync_set() can be called from two places: * * - early from start_kernel()->parse_args(). But we can't kthread_run() * at this stage, so we just set the param value. The actual * initalization happens later, from the late_initcall(). * * - even later from user space via sysfs knob. We can kthread_run() * there if needed. */ Or we could write this even more explicitely: /* * Prevent starting kthreads from start_kernel()->parse_args(). It is not * possible at this stage. Instead do so via the inticall. */ static bool printk_initcall_done; In each case, I would move the comment and the declaration right above the printk_sync_set(). > alternatively, I had this idea to re-define ->set() callback in > init_printk_kthread(). > > IOW, by default we use param_set_bool(), so parse_args() will not cause any > problems: > > static /*** can't 'const' anymore ***/ struct kernel_param_ops > param_ops_printk_sync = { > .set = param_set_bool, > .get = param_get_bool, > }; > > and change it to printk_sync_set() in: > > static __init int init_printk_kthread(void) > { > param_ops_printk_sync.set = printk_sync_set; > return __init_printk_kthread(); > } > > but I think that this bool flag is simpler to understand after all. In addition, there would be problems to handle a possible change via the sysfs knob. The bool flag looks much better to me :-) Thanks a lot for detailed explanation. Best Regards, Petr
[PATCH] MIPS: Allow R6 compact branch policy to be left unspecified
It turns out that some toolchains which support MIPS R6 don't support the -mcompact-branches flag to specify compact branch behaviour. Default to not providing the -mcompact-branch option to the compiler such that we can build with such toolchains. Signed-off-by: Paul Burton Reported-by: kbuild test robot Fixes: c1a0e9bc885d ("MIPS: Allow compact branch policy to be changed") Cc: stable # v4.4+ --- arch/mips/Kconfig.debug | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/arch/mips/Kconfig.debug b/arch/mips/Kconfig.debug index f0e314c..e91b3d1 100644 --- a/arch/mips/Kconfig.debug +++ b/arch/mips/Kconfig.debug @@ -117,7 +117,15 @@ if CPU_MIPSR6 choice prompt "Compact branch policy" - default MIPS_COMPACT_BRANCHES_OPTIMAL + default MIPS_COMPACT_BRANCHES_DEFAULT + +config MIPS_COMPACT_BRANCHES_DEFAULT + bool "Toolchain Default (don't specify)" + help + Don't pass the -mcompact-branches flag to the compiler, allowing it + to use its default (generally "optimal"). This is particularly + useful for early R6-supporting toolchains which don't support the + -mcompact-branches flag. config MIPS_COMPACT_BRANCHES_NEVER bool "Never (force delay slot branches)" -- 2.8.0
Re: [PATCH] mm: make fault_around_bytes configurable
On 04/22/2016 05:31 AM, Andrew Morton wrote: On Mon, 18 Apr 2016 20:47:16 +0530 Vinayak Menon wrote: Mapping pages around fault is found to cause performance degradation in certain use cases. The test performed here is launch of 10 apps one by one, doing something with the app each time, and then repeating the same sequence once more, on an ARM 64-bit Android device with 2GB of RAM. The time taken to launch the apps is found to be better when fault around feature is disabled by setting fault_around_bytes to page size (4096 in this case). Well that's one workload, and a somewhat strange one. What is the effect on other workloads (of which there are a lot!). This workload emulates the way a user would use his mobile device, opening an application, using it for some time, switching to next, and then coming back to the same application later. Another stat which shows significant degradation on Android with fault_around is device boot up time. I have not tried any other workload other than these. The tests were done on 3.18 kernel. 4 extra vmstat counters were added for debugging. pgpgoutclean accounts the clean pages reclaimed via __delete_from_page_cache. pageref_activate, pageref_activate_vm_exec, and pageref_keep accounts the mapped file pages activated and retained by page_check_references. === Without swap === 3.18 3.18-fault_around_bytes=4096 --- workingset_refault691100 664339 workingset_activate 210379 179139 pgpgin4676096 4492780 pgpgout 163967 96711 pgpgoutclean 1090664 990659 pgalloc_dma 3463111 3328299 pgfree3502365 3363866 pgactivate568134 238570 pgdeactivate 752260 392138 pageref_activate 315078 121705 pageref_activate_vm_exec 162940 55815 pageref_keep 141354 51011 pgmajfault2486323633 pgrefill_dma 1116370 544042 pgscan_kswapd_dma 1735186 1234622 pgsteal_kswapd_dma1121769 1005725 pgscan_direct_dma 129661090 pgsteal_direct_dma6209 967 slabs_scanned 1539849 977351 pageoutrun1260 1333 allocstall47 7 === With swap === 3.18 3.18-fault_around_bytes=4096 --- workingset_refault597687 878109 workingset_activate 167169 254037 pgpgin4035424 5157348 pgpgout 162151 85231 pgpgoutclean 928587 1225029 pswpin4603317100 pswpout 237952 127686 pgalloc_dma 3305034 3542614 pgfree3354989 3592132 pgactivate626468 355275 pgdeactivate 990205 771902 pageref_activate 294780 157106 pageref_activate_vm_exec 141722 63469 pageref_keep 121931 63028 pgmajfault6781845643 pgrefill_dma 1324023 977192 pgscan_kswapd_dma 1825267 1720322 pgsteal_kswapd_dma1181882 1365500 pgscan_direct_dma 419579622 pgsteal_direct_dma251366759 slabs_scanned 689575 542705 pageoutrun1234 1538 allocstall110 26 Looks like with fault_around, there is more pressure on reclaim because of the presence of more mapped pages, resulting in more IO activity, more faults, more swapping, and allocstalls. A few of those things did get a bit worse? I think some numbers (like workingset, pgpgin, pgpgoutclean etc) looks better with fault_around because, increased number of mapped pages is resulting in less number of file pages being reclaimed (pageref_activate, pageref_activate_vm_exec, pageref_keep above), but increased swapping. Latency numbers are far bad with fault_around_bytes + swap, possibly because of increased swapping, decrease in kswapd efficiency and increase in allocstalls. So the problem looks to be that unwanted pages are mapped around the fault and page_check_references is unaware of this. Do you have any data on actual wall-time changes? How much faster do things become with the patch? If it is "0.1%" then I'd say "umm, no". === Without swap 3.18 3.18-fault_around_bytes=4096 Avg launch latency1695ms 1300ms (23.3%) Max launch latency5097ms 3135ms (38.49%) Make
Re: [PATCH V2 06/14] irqdomain: Don't set type when mapping an IRQ
On 22/04/16 09:22, Marc Zyngier wrote: > Hi Jon, > > On 21/04/16 16:45, Jon Hunter wrote: >> >> On 20/04/16 12:03, Jon Hunter wrote: >>> Some IRQ chips, such as GPIO controllers or secondary level interrupt >>> controllers, may require require additional runtime power management >>> control to ensure they are accessible. For such IRQ chips, it makes sense >>> to enable the IRQ chip when interrupts are requested and disabled them >>> again once all interrupts have been freed. >>> >>> When mapping an IRQ, the IRQ type settings are read and then programmed. >>> The mapping of the IRQ happens before the IRQ is requested and so the >>> programming of the type settings occurs before the IRQ is requested. This >>> is a problem for IRQ chips that require additional power management >>> control because they may not be accessible yet. Therefore, when mapping >>> the IRQ, don't program the type settings, just save them and then program >>> these saved settings when the IRQ is requested (so long as if they are not >>> overridden via the call to request the IRQ). >>> >>> Add a stub function for irq_domain_free_irqs() to avoid any compilation >>> errors when CONFIG_IRQ_DOMAIN_HIERARCHY is not selected. >>> >>> Signed-off-by: Jon Hunter >>> --- >>> include/linux/irqdomain.h | 3 +++ >>> kernel/irq/irqdomain.c| 17 + >>> 2 files changed, 16 insertions(+), 4 deletions(-) >> >> [snip] >> >>> - /* Set type if specified and different than the current one */ >>> - if (type != IRQ_TYPE_NONE && >>> - type != irq_get_trigger_type(virq)) >>> - irq_set_irq_type(virq, type); >>> + irq_data = irq_get_irq_data(virq); >>> + if (!irq_data) { >>> + if (irq_domain_is_hierarchy(domain)) >>> + irq_domain_free_irqs(virq, 1); >>> + else >>> + irq_dispose_mapping(virq); >>> + return 0; >>> + } >>> + >>> + /* Store trigger type */ >>> + irqd_set_trigger_type(irq_data, type); >>> + >> >> I appear to have missed another place for saving the irq type >> which I had change in this version. Next time I will triple >> check! Should have been ... >> >> >> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h >> index 2aed04396210..fc66876d1965 100644 >> --- a/include/linux/irqdomain.h >> +++ b/include/linux/irqdomain.h >> @@ -440,6 +440,9 @@ static inline int irq_domain_alloc_irqs(struct >> irq_domain *domain, >> return -1; >> } >> >> +static inline void irq_domain_free_irqs(unsigned int virq, >> + unsigned int nr_irqs) { } >> + >> static inline bool irq_domain_is_hierarchy(struct irq_domain *domain) >> { >> return false; >> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c >> index 3d6ef5527b71..46ecf5d468b2 100644 >> --- a/kernel/irq/irqdomain.c >> +++ b/kernel/irq/irqdomain.c >> @@ -569,6 +569,7 @@ static void of_phandle_args_to_fwspec(struct >> of_phandle_args *irq_data, >> unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec) >> { >> struct irq_domain *domain; >> + struct irq_data *irq_data; >> irq_hw_number_t hwirq; >> unsigned int type = IRQ_TYPE_NONE; >> int virq; >> @@ -619,7 +620,11 @@ unsigned int irq_create_fwspec_mapping(struct >> irq_fwspec *fwspec) >> * it now and return the interrupt number. >> */ >> if (IRQ_TYPE_NONE == irq_get_trigger_type(virq)) { >> - irq_set_irq_type(virq, type); >> + irq_data = irq_get_irq_data(virq); >> + if (!irq_data) >> + return 0; > > Can you point out in which circumstances irq_data can be NULL? If we can > lookup the interrupt in the domain, it'd better exist somewhere. Or am I > missing something obvious? To be honest, I was not 100% sure if this could be possible if we find an existing mapping or if there were any paths that could race. So if we can guarantee that it is not NULL here, I can drop this and simplify the change. By the way, is there any sort of reference count for mapping an IRQ so that if irq_create_fwspec_mapping() is called more than once for an IRQ, it is not freed on the first call to irq_dispose_mapping()? I assume there is but could not see where this is handled. Cheers Jon
Re: [PATCH net-next] hv_netvsc: Fix the list processing for network change event
Haiyang Zhang writes: > RNDIS_STATUS_NETWORK_CHANGE event is handled as two "half events" -- > media disconnect & connect. The second half should be added to the list > head, not to the tail. So all events are processed in normal order. > Thanks, this matters when we get some other events in between these two halves. Reviewed-by: Vitaly Kuznetsov > Signed-off-by: Haiyang Zhang > Reviewed-by: K. Y. Srinivasan > --- > drivers/net/hyperv/netvsc_drv.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c > index bfdb568a..ba3f3f3 100644 > --- a/drivers/net/hyperv/netvsc_drv.c > +++ b/drivers/net/hyperv/netvsc_drv.c > @@ -1125,7 +1125,7 @@ static void netvsc_link_change(struct work_struct *w) > netif_tx_stop_all_queues(net); > event->event = RNDIS_STATUS_MEDIA_CONNECT; > spin_lock_irqsave(&ndev_ctx->lock, flags); > - list_add_tail(&event->list, &ndev_ctx->reconfig_events); > + list_add(&event->list, &ndev_ctx->reconfig_events); > spin_unlock_irqrestore(&ndev_ctx->lock, flags); > reschedule = true; > } -- Vitaly
Re: [PATCH V2 02/14] irqchip/gic: WARN if setting the interrupt type for a PPI fails
On 20/04/16 12:03, Jon Hunter wrote: > Setting the interrupt type for private peripheral interrupts (PPIs) may > not be supported by a given GIC because it is IMPLEMENTATION DEFINED > whether this is allowed. There is no way to know if setting the type is > supported for a given GIC and so the value written is read back to > verify it matches the desired configuration. If it does not match then > an error is return. > > There are cases where the interrupt configuration read from firmware > (such as a device-tree blob), has been incorrect and hence > gic_configure_irq() has returned an error. This error has gone > undetected because the error code returned was ignored but the interrupt > still worked fine because the configuration for the interrupt could not > be overwritten. > > Given that this has done undetected and that failing to set the > configuration for a PPI may not be a catastrophic, don't return an error > but WARN if we fail to configure a PPI. This will allows us to fix up > any places in the kernel where we should be checking the return status > and maintain backward compatibility with firmware images that may have > incorrect PPI configurations. > > Signed-off-by: Jon Hunter Acked-by: Marc Zyngier M. -- Jazz is not dead. It just smells funny...
[PATCH v4] i2c: designware-platdrv: fix unbalanced clk enable and prepare
If i2c_dw_probe() fails, we should disable and unprepare the clock, otherwise the clock enable and prepare is left unbalanced. In dw_i2c_plat_remove(), we'd better to not rely on runtime PM to disable and unprepare the clock since CONFIG_PM may be disabled when configuring the kernel. So we explicitly disable and unprepare the clock in dw_i2c_plat_remove() rather than implicitly rely on pm_runtime_put_sync(). To keep the device usage count balanced, we call pm_runtime_put_noidle() to decrease the usage count. Signed-off-by: Jisheng Zhang --- Since v3: - use runtime PM rather than rpm in commit msg - remove duplicated "(" in commit msg Since v2: - s/clk/clock - describe why use pm_runtime_put_noidle() Since v1: - fix commit msg: "not rely on rpm" rather than "rely on rpm" - call i2c_dw_plat_prepare_clk after pm_rumtime_disable() drivers/i2c/busses/i2c-designware-platdrv.c | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c index d656657..a771781 100644 --- a/drivers/i2c/busses/i2c-designware-platdrv.c +++ b/drivers/i2c/busses/i2c-designware-platdrv.c @@ -253,8 +253,11 @@ static int dw_i2c_plat_probe(struct platform_device *pdev) } r = i2c_dw_probe(dev); - if (r && !dev->pm_runtime_disabled) - pm_runtime_disable(&pdev->dev); + if (r) { + if (!dev->pm_runtime_disabled) + pm_runtime_disable(&pdev->dev); + i2c_dw_plat_prepare_clk(dev, false); + } return r; } @@ -264,15 +267,16 @@ static int dw_i2c_plat_remove(struct platform_device *pdev) struct dw_i2c_dev *dev = platform_get_drvdata(pdev); pm_runtime_get_sync(&pdev->dev); + pm_runtime_dont_use_autosuspend(&pdev->dev); + if (!dev->pm_runtime_disabled) + pm_runtime_disable(&pdev->dev); + pm_runtime_put_noidle(&pdev->dev); i2c_del_adapter(&dev->adapter); i2c_dw_disable(dev); - pm_runtime_dont_use_autosuspend(&pdev->dev); - pm_runtime_put_sync(&pdev->dev); - if (!dev->pm_runtime_disabled) - pm_runtime_disable(&pdev->dev); + i2c_dw_plat_prepare_clk(dev, false); return 0; } -- 2.8.1
RE: [PATCH 1/7] debugobjects: make fixup functions return bool instead of int
Hi, > On Fri, 22 Apr 2016, changbin...@intel.com wrote: > > From: "Du, Changbin" > > > > The object debugging infrastructure core provides some fixup callbacks > > for the subsystem who use it. These callbacks are called from the debug > > code whenever a problem in debug_object_init is detected. And > > debugobjects core suppose them returns 1 when the fixup was successful, > > otherwise 0. So the return type is boolean. > > > > A bad thing is that debug_object_fixup use the return value for > > arithmetic operation. It confused me that what is the reall return > > What's bad about that? The fact that it's used for arithmethic operation or > that it confused you? > It confused me because this is not a common usage. I was confused that what does he fixup function return? A countable value? But doc says return fixed or not! if (fixup) fixed = fixup(addr, state); debug_objects_fixups += fixed; In common,for int return 0 indicates success, negative for fail, positive for something countable. So I think it is better follow this rule. Here is not of countable, it is Boolean. So why not this? if (fixup && fixup(addr, state)) debug_objects_fixups++; > > Reading over the whole code, I found some place do use the return value > > incorrectly(see next patch). So why use bool type instead? > > Patches which fix a problem need to come first not in the middle of a revamp > series. > Thanks, I am first know of this. > > + bool (*fixup_init)(void *addr, enum debug_obj_state state); > > + bool (*fixup_activate)(void *addr, enum debug_obj_state state); > > + bool (*fixup_destroy)(void *addr, enum debug_obj_state state); > > + bool (*fixup_free)(void *addr, enum debug_obj_state state); > > + bool (*fixup_assert_init)(void *addr, enum debug_obj_state state); > > So this change will introduce a gazillion of compile warnings because the > callbacks in the various usage sites are still having 'int' return type. > No, I modified all the code who use debugojects API. > Thanks, > > tglx Thanks, Du, Changbin
Re: [kbuild-all] mipsel-linux-gnu-gcc: error: unrecognized command line option '-mcompact-branches=optimal'
On Thu, Apr 21, 2016 at 10:10:51PM +0100, Maciej W. Rozycki wrote: > On Thu, 21 Apr 2016, Ralf Baechle wrote: > > > > I don't think it makes sense as the compiler won't support MIPSr6 code > > > anyway, so first it'll bail out on `-march=mips32r6', and if we go even > > > further and disable that too, then GAS will probably break somewhere on > > > inline asm and GCC will produce code which does not make sense otherwise. > > > > GCC 5.2.0 claims to support mips32r6 and mips64r6. It's just the option > > -mcompact-branches which seem to have been added later only. > > Ah, I see -- I didn't track the timeline of support for this compiler's > option and I took it from an earlier response that the compiler does not > support R6 at all. > > In that case however it looks to me like these `-mcompact-branches=' > options (all the three we support) need to be wrapped into `$(call > cc-option,...)'. An alternative that it could be argued better fits the principle of least surprise is to add an extra option to the Kconfig choice that simply leaves -mcompact-branches unspecified. I just submitted a patch to do so [1]. > They do not affect any functionality and they are an > optimisation choice only anyway (and therefore I wonder why they've been > placed in arch/mips/Kconfig.debug rather than arch/mips/Kconfig). They're in Kconfig.debug because debug is exactly what they've been useful for - given that compact branches are new to R6 it's been useful in debugging systems, both hardware & simulators, to sometimes not use them. It's also been useful to force their use attempting to work around the compiler bug that [2] works around differently (bug 2179 on DMZ bugzilla). On the other hand I can't think of a reason we'd want to specify compact branch policy that isn't for debug - I'd expect for performance optimisation we're more likely to rely upon the toolchain using a sensible policy if the kernel is built for a specific CPU (eg. perhaps -mcpu=p6600 prefers non-compact branches & -mcpu=m6250 prefers all compact branches, or similar). Thanks, Paul [1] https://patchwork.linux-mips.org/patch/13165/ [2] https://patchwork.linux-mips.org/patch/12556/
Re: [PATCH v5] irqchip, gicv3-its, numa: Enable workaround for Cavium thunderx erratum 23144
On Fri, Apr 22, 2016 at 09:01:05AM +0100, Marc Zyngier wrote: > On 21/04/16 18:40, Robert Richter wrote: > > On 15.04.16 21:30:05, Robert Richter wrote: > >> From: Ganapatrao Kulkarni > >> > >> The erratum fixes the hang of ITS SYNC command by avoiding inter node > >> io and collections/cpu mapping on thunderx dual-socket platform. > >> > >> This fix is only applicable for Cavium's ThunderX dual-socket platform. > >> > >> This is based on NUMA v16 series. > >> Message-Id: <1460155828-8690-1-git-send-email-ddaney.c...@gmail.com> > > > > Will, Catalin, > > > > after NUMA base patches went into linux-next (many thanks), please > > consider this pass 1.x errata workaround for inclusion too. > > I'll queue it as a fix once the NUMA patches are merged into mainline. Yeah, this can probably go in as -rc1 material if you're happy with it. Will
Re: [PATCH 2/7] mfd: lp8788: Use devm_mfd_add_devices and devm_regmap_add_irq_chip
On Friday 22 April 2016 04:45 AM, Kim, Milo wrote: Hi Laxman, On 4/21/2016 9:25 PM, Laxman Dewangan wrote: Use devm_mfd_add_devices() for adding MFD child devices and devm_regmap_add_irq_chip() for IRQ chip registration. This patch doesn't include the code regarding devm_mfd_add_devices(). Could you check it again? Or am I missing any previous patches? Sigh.. yaah, it is missed. Dont know how I missed it in my grep result. I will recycle the patch. But I like to get review other patches also for V2. External code review is more stronger then self code review.
Re: [PATCH v7 1/9] clk: mediatek: Refine the makefile to support multiple clock drivers
On 14/04/16 10:11, James Liao wrote: Add a Kconfig to define clock configuration for each SoC, and modify the Makefile to build drivers that only selected in config. Signed-off-by: Shunli Wang Signed-off-by: James Liao Tested-by: John Crispin --- Reviewed-by: Matthias Brugger drivers/clk/Kconfig | 1 + drivers/clk/mediatek/Kconfig | 23 +++ drivers/clk/mediatek/Makefile | 6 +++--- 3 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 drivers/clk/mediatek/Kconfig diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 16f7d33..d94302e 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -199,6 +199,7 @@ config COMMON_CLK_PXA source "drivers/clk/bcm/Kconfig" source "drivers/clk/hisilicon/Kconfig" +source "drivers/clk/mediatek/Kconfig" source "drivers/clk/mvebu/Kconfig" source "drivers/clk/qcom/Kconfig" source "drivers/clk/samsung/Kconfig" diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig new file mode 100644 index 000..dc224e6 --- /dev/null +++ b/drivers/clk/mediatek/Kconfig @@ -0,0 +1,23 @@ +# +# MediaTek SoC drivers +# +config COMMON_CLK_MEDIATEK + bool + ---help--- + Mediatek SoCs' clock support. + +config COMMON_CLK_MT8135 + bool "Clock driver for Mediatek MT8135" + depends on COMMON_CLK + select COMMON_CLK_MEDIATEK + default ARCH_MEDIATEK + ---help--- + This driver supports Mediatek MT8135 clocks. + +config COMMON_CLK_MT8173 + bool "Clock driver for Mediatek MT8173" + depends on COMMON_CLK + select COMMON_CLK_MEDIATEK + default ARCH_MEDIATEK + ---help--- + This driver supports Mediatek MT8173 clocks. diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile index 95fdfac..32e7222 100644 --- a/drivers/clk/mediatek/Makefile +++ b/drivers/clk/mediatek/Makefile @@ -1,4 +1,4 @@ -obj-y += clk-mtk.o clk-pll.o clk-gate.o clk-apmixed.o +obj-$(CONFIG_COMMON_CLK_MEDIATEK) += clk-mtk.o clk-pll.o clk-gate.o clk-apmixed.o obj-$(CONFIG_RESET_CONTROLLER) += reset.o -obj-y += clk-mt8135.o -obj-y += clk-mt8173.o +obj-$(CONFIG_COMMON_CLK_MT8135) += clk-mt8135.o +obj-$(CONFIG_COMMON_CLK_MT8173) += clk-mt8173.o
Re: [PATCH v2 3/7] regulator: rk808: Migrate to regulator core's simplified DT parsing code
On 21.04.2016 18:01, Mark Brown wrote: > On Thu, Apr 21, 2016 at 03:12:37PM +0200, Wadim Egorov wrote: > >> +static int rk808_set_suspend_voltage(struct regulator_dev *rdev, int uv) >> +{ >> +unsigned int reg; >> +int sel = regulator_map_voltage_linear(rdev, uv, uv); >> + >> +if (sel < 0) >> +return -EINVAL; >> + >> +reg = rdev->desc->vsel_reg + RK808_SLP_REG_OFFSET; >> + >> +return regmap_update_bits(rdev->regmap, reg, >> + rdev->desc->vsel_mask, >> + sel); >> +} > This is fine but is adding a new feature and not part of the refactoring > that the changelog talked about so should be in a separate commit. This is not really a new feature. rk808_set_suspend_voltage() was using regulator_map_voltage_linear_range(). I have just renamed the function to rk808_set_suspend_voltage_range() and added an ops struct for ranges. Yes, I have also added rk808_set_suspend_voltage(), but this is just a split for the two types of ops that the driver needs now. This was needed, because the driver used only linear ranges. IMO it should be a part of the refactoring.
Re: [PATCH v7 3/9] clk: mediatek: Add dt-bindings for MT2701 clocks
On 14/04/16 10:11, James Liao wrote: From: Shunli Wang Add MT2701 clock dt-bindings, include topckgen, apmixedsys, infracfg, pericfg and subsystem clocks. Signed-off-by: Shunli Wang Signed-off-by: James Liao Tested-by: John Crispin Reviewed-by: Matthias Brugger --- include/dt-bindings/clock/mt2701-clk.h | 486 + 1 file changed, 486 insertions(+) create mode 100644 include/dt-bindings/clock/mt2701-clk.h diff --git a/include/dt-bindings/clock/mt2701-clk.h b/include/dt-bindings/clock/mt2701-clk.h new file mode 100644 index 000..2062c67 --- /dev/null +++ b/include/dt-bindings/clock/mt2701-clk.h @@ -0,0 +1,486 @@ +/* + * Copyright (c) 2014 MediaTek Inc. + * Author: Shunli Wang + * + * 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. + */ + +#ifndef _DT_BINDINGS_CLK_MT2701_H +#define _DT_BINDINGS_CLK_MT2701_H + +/* TOPCKGEN */ +#define CLK_TOP_SYSPLL 1 +#define CLK_TOP_SYSPLL_D2 2 +#define CLK_TOP_SYSPLL_D3 3 +#define CLK_TOP_SYSPLL_D5 4 +#define CLK_TOP_SYSPLL_D7 5 +#define CLK_TOP_SYSPLL1_D2 6 +#define CLK_TOP_SYSPLL1_D4 7 +#define CLK_TOP_SYSPLL1_D8 8 +#define CLK_TOP_SYSPLL1_D169 +#define CLK_TOP_SYSPLL2_D2 10 +#define CLK_TOP_SYSPLL2_D4 11 +#define CLK_TOP_SYSPLL2_D8 12 +#define CLK_TOP_SYSPLL3_D2 13 +#define CLK_TOP_SYSPLL3_D4 14 +#define CLK_TOP_SYSPLL4_D2 15 +#define CLK_TOP_SYSPLL4_D4 16 +#define CLK_TOP_UNIVPLL17 +#define CLK_TOP_UNIVPLL_D2 18 +#define CLK_TOP_UNIVPLL_D3 19 +#define CLK_TOP_UNIVPLL_D5 20 +#define CLK_TOP_UNIVPLL_D7 21 +#define CLK_TOP_UNIVPLL_D2622 +#define CLK_TOP_UNIVPLL_D5223 +#define CLK_TOP_UNIVPLL_D108 24 +#define CLK_TOP_USB_PHY48M 25 +#define CLK_TOP_UNIVPLL1_D226 +#define CLK_TOP_UNIVPLL1_D427 +#define CLK_TOP_UNIVPLL1_D828 +#define CLK_TOP_UNIVPLL2_D229 +#define CLK_TOP_UNIVPLL2_D430 +#define CLK_TOP_UNIVPLL2_D831 +#define CLK_TOP_UNIVPLL2_D16 32 +#define CLK_TOP_UNIVPLL2_D32 33 +#define CLK_TOP_UNIVPLL3_D234 +#define CLK_TOP_UNIVPLL3_D435 +#define CLK_TOP_UNIVPLL3_D836 +#define CLK_TOP_MSDCPLL37 +#define CLK_TOP_MSDCPLL_D2 38 +#define CLK_TOP_MSDCPLL_D4 39 +#define CLK_TOP_MSDCPLL_D8 40 +#define CLK_TOP_MMPLL 41 +#define CLK_TOP_MMPLL_D2 42 +#define CLK_TOP_DMPLL 43 +#define CLK_TOP_DMPLL_D2 44 +#define CLK_TOP_DMPLL_D4 45 +#define CLK_TOP_DMPLL_X2 46 +#define CLK_TOP_TVDPLL 47 +#define CLK_TOP_TVDPLL_D2 48 +#define CLK_TOP_TVDPLL_D4 49 +#define CLK_TOP_TVD2PLL50 +#define CLK_TOP_TVD2PLL_D2 51 +#define CLK_TOP_HADDS2PLL_98M 52 +#define CLK_TOP_HADDS2PLL_294M 53 +#define CLK_TOP_HADDS2_FB 54 +#define CLK_TOP_MIPIPLL_D2 55 +#define CLK_TOP_MIPIPLL_D4 56 +#define CLK_TOP_HDMIPLL57 +#define CLK_TOP_HDMIPLL_D2 58 +#define CLK_TOP_HDMIPLL_D3 59 +#define CLK_TOP_HDMI_SCL_RX60 +#define CLK_TOP_HDMI_0_PIX340M 61 +#define CLK_TOP_HDMI_0_DEEP340M62 +#define CLK_TOP_HDMI_0_PLL340M 63 +#define CLK_TOP_AUD1PLL_98M64 +#define CLK_TOP_AUD2PLL_90M65 +#define CLK_TOP_AUDPLL 66 +#define CLK_TOP_AUDPLL_D4 67 +#define CLK_TOP_AUDPLL_D8 68 +#define CLK_TOP_AUDPLL_D16 69 +#define CLK_TOP_AUDPLL_D24 70 +#define CLK_TOP_ETHPLL_500M71 +#define CLK_TOP_VDECPLL72 +#def
Re: [PATCH 2/7] debugobjects: correct the usage of fixup call results
On Fri, 22 Apr 2016, changbin...@intel.com wrote: > From: "Du, Changbin" > > If debug_object_fixup() return non-zero when problem has been > fixed. But the code got it backwards, it taks 0 as fixup > successfully. So fix it. Wrong. > @@ -415,7 +415,7 @@ int debug_object_activate(void *addr, struct > debug_obj_descr *descr) > state = obj->state; > raw_spin_unlock_irqrestore(&db->lock, flags); > ret = debug_object_fixup(descr->fixup_activate, addr, > state); > - return ret ? -EINVAL : 0; > + return ret ? 0 : -EINVAL; So you need to look at the fixup_activate() callbacks. timer_fixup_activate() The only state handled there is ODEBUG_STATE_NOTAVAILABLE. This can happen for two reasons: 1) timer is statically allocated and initialized. That's a legitimate reason and it does not count as a fixup 2) timer has not been initialized, so we have no idea what to do with it. We set it up with a dummy callback and return 1 because that is a fixup. So the return check for ret != 0 is correct. #2 is invalid hrtimer_fixup_activate() There is not much we can do about it. work_fixup_activate() That's similar to timer_fixup_activate(). We need to handle the statically allocated work gracefully. rcuhead_fixup_activate() Handles the ODEBUG_STATE_NOTAVAILABLE case by tracking it. Not a fixup, returns 0. The other states are invalid and there is not much we can do about that. Returns 1. I agree that this is not really intuitive, but it's correct as it is. I'm happy to take patches which make it simpler to understand. Just blindly changing everything to bool does not fall into that category. Thanks, tglx
[PATCH v3] sched/cpufreq: optimize cpufreq update kicker to avoid update multiple times
From: Wanpeng Li Sometimes delta_exec is 0 due to update_curr() is called multiple times, this is captured by: u64 delta_exec = rq_clock_task(rq) - curr->se.exec_start; This patch optimizes the cpufreq update kicker by bailing out when nothing changed, it will benefit the upcoming schedutil, since otherwise it will (over)react to the special util/max combination. Signed-off-by: Wanpeng Li --- v1 -> v2: * add From: head v2 -> v3: * update changelog kernel/sched/deadline.c | 8 kernel/sched/rt.c | 8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c index affd97e..8f9b5af 100644 --- a/kernel/sched/deadline.c +++ b/kernel/sched/deadline.c @@ -717,10 +717,6 @@ static void update_curr_dl(struct rq *rq) if (!dl_task(curr) || !on_dl_rq(dl_se)) return; - /* Kick cpufreq (see the comment in linux/cpufreq.h). */ - if (cpu_of(rq) == smp_processor_id()) - cpufreq_trigger_update(rq_clock(rq)); - /* * Consumed budget is computed considering the time as * observed by schedulable tasks (excluding time spent @@ -736,6 +732,10 @@ static void update_curr_dl(struct rq *rq) return; } + /* kick cpufreq (see the comment in linux/cpufreq.h). */ + if (cpu_of(rq) == smp_processor_id()) + cpufreq_trigger_update(rq_clock(rq)); + schedstat_set(curr->se.statistics.exec_max, max(curr->se.statistics.exec_max, delta_exec)); diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c index c41ea7a..19e1306 100644 --- a/kernel/sched/rt.c +++ b/kernel/sched/rt.c @@ -953,14 +953,14 @@ static void update_curr_rt(struct rq *rq) if (curr->sched_class != &rt_sched_class) return; - /* Kick cpufreq (see the comment in linux/cpufreq.h). */ - if (cpu_of(rq) == smp_processor_id()) - cpufreq_trigger_update(rq_clock(rq)); - delta_exec = rq_clock_task(rq) - curr->se.exec_start; if (unlikely((s64)delta_exec <= 0)) return; + /* Kick cpufreq (see the comment in linux/cpufreq.h). */ + if (cpu_of(rq) == smp_processor_id()) + cpufreq_trigger_update(rq_clock(rq)); + schedstat_set(curr->se.statistics.exec_max, max(curr->se.statistics.exec_max, delta_exec)); -- 1.9.1
Re: [PATCH resend 0/3] mm: allow arch to override lowmem_page_address
On Thu, Apr 21, 2016 at 04:51:38PM -0700, Andrew Morton wrote: > On Mon, 18 Apr 2016 18:04:54 +0200 Ard Biesheuvel > wrote: > > > These patches allow the arch to define the page_to_virt() conversion that > > is used in lowmem_page_address(). This is desirable for arm64, where this > > conversion is trivial when CONFIG_SPARSEMEM_VMEMMAP is enabled, while > > breaking it up into __va(PFN_PHYS(page_to_pfn(page))), as is done currently > > in lowmem_page_address(), will force the use of a virt-to-phys() conversion > > and back again, which always involves a memory access on arm64, since the > > start of physical memory is not a compile time constant. > > > > I have split off these patches from my series 'arm64: optimize virt_to_page > > and page_address' which I sent out 3 weeks ago, and resending them in the > > hope that they can be picked up (with Will's ack on #3) to be merged via > > the mm tree. > > > > I have cc'ed the nios2 and openrisc maintainers on previous versions, and > > cc'ing them again now. I have dropped both of the arch specific mailing > > lists, since one is defunct and the other is subscriber only. > > > > Andrew, is this something you would be pulling to pick up (assuming that you > > agree with the contents)? Thanks. > > Looks OK to me and apart from the trivial openrisc/nios2 changes it's > obviously a no-op for all-but-arm. So I suggest you include these > patches in the appropriate arm tree. > > Acked-by: Andrew Morton Cracking, thanks Andrew. I'll queue these three in the arm64 tree and get them into -next. Will
Re: [PATCH v2 05/13] ARM: dts: db600c: add pmic regulator supplies
On 19/04/16 20:10, Stephen Boyd wrote: On 04/12/2016 02:33 AM, Srinivas Kandagatla wrote: This patch adds pmic regulator supplies connected on the board. Rest of the invidual regulators would be added as and when required by the devices. Signed-off-by: Srinivas Kandagatla Acked-by: Bjorn Andersson --- arch/arm/boot/dts/qcom-apq8064-arrow-db600c.dts | 62 + 1 file changed, 62 insertions(+) diff --git a/arch/arm/boot/dts/qcom-apq8064-arrow-db600c.dts b/arch/arm/boot/dts/qcom-apq8064-arrow-db600c.dts index 57d4500..6695b00 100644 --- a/arch/arm/boot/dts/qcom-apq8064-arrow-db600c.dts +++ b/arch/arm/boot/dts/qcom-apq8064-arrow-db600c.dts @@ -9,7 +9,69 @@ serial1 = &gsbi1_serial; }; + regulators { + compatible = "simple-bus"; + vph: regulator-fixed@1 { + compatible = "regulator-fixed"; + regulator-min-microvolt = <450>; + regulator-max-microvolt = <450>; + regulator-name = "VPH"; + regulator-type = "voltage"; + regulator-boot-on; + }; + }; Just curious why we added the vph supply? Is that for some framework requirement? We haven't done this on other boards, although we probably should if there's a good reason for it. This is an on board 12V TO 4.5V @5.5A DC/DC convertor for PMIC VPH power. Yep we should do something similar on the other boards too. Without this probably you would notice some error messages from rpm_regulators about missing supply nodes. thanks, srini
Re: [PATCH 0/2] mtd: nand: omap2: Fix SDMA support for NAND DMA prefetch
On Fri, 15 Apr 2016 15:28:57 -0500 Franklin S Cooper Jr wrote: > NAND DMA prefetch for SDMA based devices has been broken for awhile. This > patchset fixes it so SOCs that use the SDMA can make use of the NAND > DMA prefetch. > > I've decided to split this patchset from the slightly larger patchset that > included EDMA support. Adding EDMA support will be added in a later > patchset. > > Tested using the latest master on: > am37 gp evm > > Due to a silicon issue Dra7 SoCs are unable to use NAND DMA prefetch. Applied, thanks. Boris > > Cooper Jr., Franklin (2): > mtd: nand: omap2: Start dma request before enabling prefetch > mtd: nand: omap2: Fix high memory dma prefetch transfer > > drivers/mtd/nand/omap2.c | 22 +++--- > 1 file changed, 7 insertions(+), 15 deletions(-) > -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCH 4/4] mtd: nandsim: add __init attribute
On Tue, 19 Apr 2016 14:33:35 +0200 Julia Lawall wrote: > Add __init attribute on functions that are only called from other __init > functions and that are not inlined, at least with gcc version 4.8.4 on an > x86 machine with allyesconfig. Currently, the functions are put in the > .text.unlikely segment. Declaring them as __init will cause them to be > put in the .init.text and to disappear after initialization. > > The result of objdump -x on the functions before the change is as follows: > > 059a l F .text.unlikely 0239 alloc_device > 034e l F .text.unlikely 002e get_partition_name > 07d3 l F .text.unlikely 05da init_nandsim > > And after the change it is as follows: > > 0029 l F .init.text 0234 alloc_device > l F .init.text 0029 get_partition_name > 025d l F .init.text 05d5 init_nandsim > > Done with the help of Coccinelle. The semantic patch checks for local > static non-init functions that are called from an __init function and are > not called from any other function. > > Signed-off-by: Julia Lawall > > --- > drivers/mtd/nand/nandsim.c |6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Applied, thanks. Boris -- Boris Brezillon, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
Re: [PATCH 00/12] mtd: get rid of of_mtd.{c,h} and of_get_nand_xx()
On Fri, 1 Apr 2016 14:54:20 +0200 Boris Brezillon wrote: > Hello, > > of_mtd.{h,c} are providing the of_get_nand_xxx() helpers to help NAND > controller drivers parse some generic NAND DT properties. > An infrastructure has recently been added to NAND core to automatically > parse those properties when nand_scan_ident() is called, thus removing > the need for NAND controller drivers to manually parse them. > > This series modifies drivers still making use of those helpers to rely > on NAND core initialization, and get rid of the of_mtd.{c,h} files by > moving the of_get_nand_xx() helpers into nand_base.c. Applied remaining patches. > > Best Regards, > > Boris > > Boris Brezillon (12): > mtd: nand: remove unneeded of_mtd.h inclusions > mtd: nand: atmel: rely on generic DT parsing done in nand_scan_ident() > mtd: nand: omap2: rely on generic DT parsing done in nand_scan_ident() > mtd: nand: brcm: rely on generic DT parsing done in nand_scan_ident() > mtd: nand: davinci: rely on generic DT parsing done in > nand_scan_ident() > mtd: nand: gpmi: rely on generic DT parsing done in nand_scan_ident() > mtd: nand: hisi504: rely on generic DT parsing done in > nand_scan_ident() > mtd: nand: lpc32xx: rely on generic DT parsing done in > nand_scan_ident() > mtd: nand: mxc: rely on generic DT parsing done in nand_scan_ident() > mtd: nand: pxa3xx: rely on generic DT parsing done in > nand_scan_ident() > mtd: nand: sh_flctl: rely on generic DT parsing done in > nand_scan_ident() > mtd: nand: move of_get_nand_xxx() helpers into nand_base.c > > drivers/memory/omap-gpmc.c | 7 -- > drivers/mtd/nand/atmel_nand.c | 133 +++- > drivers/mtd/nand/brcmnand/brcmnand.c | 5 +- > drivers/mtd/nand/davinci_nand.c| 85 +- > drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 21 +++-- > drivers/mtd/nand/hisi504_nand.c| 14 +-- > drivers/mtd/nand/jz4780_nand.c | 1 - > drivers/mtd/nand/lpc32xx_mlc.c | 1 - > drivers/mtd/nand/lpc32xx_slc.c | 24 ++--- > drivers/mtd/nand/mxc_nand.c| 50 +-- > drivers/mtd/nand/nand_base.c | 99 - > drivers/mtd/nand/omap2.c | 9 +- > drivers/mtd/nand/pxa3xx_nand.c | 28 +++--- > drivers/mtd/nand/qcom_nandc.c | 1 - > drivers/mtd/nand/sh_flctl.c| 31 +++ > drivers/mtd/nand/sunxi_nand.c | 1 - > drivers/mtd/nand/vf610_nfc.c | 1 - > drivers/of/Makefile| 1 - > drivers/of/of_mtd.c| 155 > - > include/linux/of_mtd.h | 56 > 20 files changed, 291 insertions(+), 432 deletions(-) > delete mode 100644 drivers/of/of_mtd.c > delete mode 100644 include/linux/of_mtd.h > -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
RE: [PATCH 1/7] debugobjects: make fixup functions return bool instead of int
On Fri, 22 Apr 2016, Du, Changbin wrote: > > On Fri, 22 Apr 2016, changbin...@intel.com wrote: > > > A bad thing is that debug_object_fixup use the return value for > > > arithmetic operation. It confused me that what is the reall return > > > > What's bad about that? The fact that it's used for arithmethic operation or > > that it confused you? > > > It confused me because this is not a common usage. I was confused that what > does he fixup function return? A countable value? But doc says return fixed > or not! It says return 0 for not fixed up and 1 for fixed up. The activate fixup is special and it has been written this way to handle the static initialization case. > if (fixup) > fixed = fixup(addr, state); > debug_objects_fixups += fixed; > In common,for int return 0 indicates success, negative for fail, positive > for something countable. So I think it is better follow this rule. Here is > not of countable, it is Boolean. Yes, it's common for most of the code. This code has been deliberately been written differently. I'm not opposed to change that and improve it, but just slapping bool on it does not really make any difference. > So why not this? > if (fixup && fixup(addr, state)) > debug_objects_fixups++; There is no problem with that per se. > > > + bool (*fixup_init)(void *addr, enum debug_obj_state state); > > > + bool (*fixup_activate)(void *addr, enum debug_obj_state state); > > > + bool (*fixup_destroy)(void *addr, enum debug_obj_state state); > > > + bool (*fixup_free)(void *addr, enum debug_obj_state state); > > > + bool (*fixup_assert_init)(void *addr, enum debug_obj_state state); > > > > So this change will introduce a gazillion of compile warnings because the > > callbacks in the various usage sites are still having 'int' return type. > > > No, I modified all the code who use debugojects API. You do that in the later patches. But patches must be compilable and functional on their own. Compiling this one will emit a gazillion of "initialization from incompatible pointer type" warnings. Thanks, tglx
Re: [PATCH] media: vb2: Fix regression on poll() for RW mode
Hi Ricardo, On 04/21/2016 11:15 AM, Ricardo Ribalda Delgado wrote: > When using a device is read/write mode, vb2 does not handle properly the > first select/poll operation. It allways return POLLERR. > > The reason for this is that when this code has been refactored, some of > the operations have changed their order, and now fileio emulator is not > started by poll, due to a previous check. > > Reported-by: Dimitrios Katsaros > Cc: Junghak Sung > Cc: sta...@vger.kernel.org > Fixes: 49d8ab9feaf2 ("media] media: videobuf2: Separate vb2_poll()") > Signed-off-by: Ricardo Ribalda Delgado > --- > drivers/media/v4l2-core/videobuf2-core.c | 8 > drivers/media/v4l2-core/videobuf2-v4l2.c | 8 > 2 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/media/v4l2-core/videobuf2-core.c > b/drivers/media/v4l2-core/videobuf2-core.c > index 5d016f496e0e..199c65dbe330 100644 > --- a/drivers/media/v4l2-core/videobuf2-core.c > +++ b/drivers/media/v4l2-core/videobuf2-core.c > @@ -2298,6 +2298,14 @@ unsigned int vb2_core_poll(struct vb2_queue *q, struct > file *file, > return POLLERR; > > /* > + * For compatibility with vb1: if QBUF hasn't been called yet, then > + * return POLLERR as well. This only affects capture queues, output > + * queues will always initialize waiting_for_buffers to false. > + */ > + if (q->waiting_for_buffers && (req_events & (POLLIN | POLLRDNORM))) > + return POLLERR; The problem I have with this is that this should be specific to V4L2. The only reason we do this is that we had to stay backwards compatible with vb1. This is the reason this code was placed in videobuf2-v4l2.c. But you are correct that this causes a regression, and I see no other choice but to put it in core.c. That said, I would still only honor this when called from v4l2, so I suggest that a new flag 'check_waiting_for_buffers' is added that is only set in vb2_queue_init in videobuf2-v4l2.c. So the test above becomes: if (q->check_waiting_for_buffers && q->waiting_for_buffers && (req_events & (POLLIN | POLLRDNORM))) It's not ideal, but at least this keeps this v4l2 specific. Regards, Hans > + > + /* >* For output streams you can call write() as long as there are fewer >* buffers queued than there are buffers available. >*/ > diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c > b/drivers/media/v4l2-core/videobuf2-v4l2.c > index 91f552124050..c9bad9ef2104 100644 > --- a/drivers/media/v4l2-core/videobuf2-v4l2.c > +++ b/drivers/media/v4l2-core/videobuf2-v4l2.c > @@ -818,14 +818,6 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file > *file, poll_table *wait) > poll_wait(file, &fh->wait, wait); > } > > - /* > - * For compatibility with vb1: if QBUF hasn't been called yet, then > - * return POLLERR as well. This only affects capture queues, output > - * queues will always initialize waiting_for_buffers to false. > - */ > - if (q->waiting_for_buffers && (req_events & (POLLIN | POLLRDNORM))) > - return POLLERR; > - > return res | vb2_core_poll(q, file, wait); > } > EXPORT_SYMBOL_GPL(vb2_poll); >
Re: linux-next: build failure after merge of the sound-asoc tree
On Fri, Apr 22, 2016 at 10:40:11AM +0200, Philipp Zabel wrote: > 8< > From: Philipp Zabel > Subject: [PATCH] fixup! ASoC: hdmi-codec: Add ELD control Please don't bury patches in the middle of e-mails. signature.asc Description: PGP signature
Re: linux-next: build failure after merge of the net-next tree
On Wed, Apr 13, 2016 at 11:15:13AM -0400, David Miller wrote: > From: Stephen Rothwell > > After merging the net-next tree, today's linux-next build (arm > > allmodconfig) failed like thisi (this has actually been failing for a > > few days, now): > > ERROR: "__bad_udelay" [drivers/net/ethernet/intel/ixgbe/ixgbe.ko] undefined! > > Caused by commit > > 49425dfc7451 ("ixgbe: Add support for x550em_a 10G MAC type") > > arm only allows udelay()s up to 2 milliseconds. This commit > > adds a 5 ms udelay in ixgbe_acquire_swfw_sync_x550em_a() in > > drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c. > Jeff, please have your folks look into this. Probably just a simple > conversion to mdelay(). This is still present, it's been breaking ARM allmodconfig builds for about two weeks now. signature.asc Description: PGP signature
Re: linux-next: build failure after merge of the sound-asoc tree
On Fri, Apr 22, 2016 at 10:40:11AM +0200, Philipp Zabel wrote: > 8< > From: Philipp Zabel > Subject: [PATCH] fixup! ASoC: hdmi-codec: Add ELD control Please also use subject lines that match the style for the subsystem. signature.asc Description: PGP signature
Re: [PATCH 2/3] ASoC: simple-card: Add support jack detection via codec
On Fri, Apr 22, 2016 at 09:17:46AM +0800, Xing Zheng wrote: > Hi Mark, the other question here, how do we make sense to use the > simple-card to call the "snd_soc_dai_set_pll" if the codec (like da7219) > requires do this? What we should do there is move the CODEC clocking to be represented using the clock API and then there are common clock bindings that already exist for configuring clocks. signature.asc Description: PGP signature
Re: [PATCH v4 2/2] KVM: move vcpu id checking to archs
Hi, Greg One confusion. There are 5 kvm_arch_vcpu_create() while in this patch you changed 2 of them. Some particular reason? On Thu, Apr 21, 2016 at 04:20:53PM +0200, Greg Kurz wrote: >Commit 338c7dbadd26 ("KVM: Improve create VCPU parameter (CVE-2013-4587)") >introduced a check to prevent potential kernel memory corruption in case >the vcpu id is too great. > >Unfortunately this check assumes vcpu ids grow in sequence with a common >difference of 1, which is wrong: archs are free to use vcpu id as they fit. >For example, QEMU originated vcpu ids for PowerPC cpus running in boot3s_hv >mode, can grow with a common difference of 2, 4 or 8: if KVM_MAX_VCPUS is >1024, guests may be limited down to 128 vcpus on POWER8. > >This means the check does not belong here and should be moved to some arch >specific function: kvm_arch_vcpu_create() looks like a good candidate. > >ARM and s390 already have such a check. > >I could not spot any path in the PowerPC or common KVM code where a vcpu >id is used as described in the above commit: I believe PowerPC can live >without this check. > >In the end, this patch simply moves the check to MIPS and x86. While here, >we also update the documentation to dissociate vcpu ids from the maximum >number of vcpus per virtual machine. > >Acked-by: James Hogan >Acked-by: Cornelia Huck >Signed-off-by: Greg Kurz >--- >v4: - updated subject for more clarity on what the patch does >- added James's and Connie's A-b tags >- updated documentation > > Documentation/virtual/kvm/api.txt |7 +++ > arch/mips/kvm/mips.c |7 ++- > arch/x86/kvm/x86.c|3 +++ > virt/kvm/kvm_main.c |3 --- > 4 files changed, 12 insertions(+), 8 deletions(-) > >diff --git a/Documentation/virtual/kvm/api.txt >b/Documentation/virtual/kvm/api.txt >index 4d0542c5206b..486a1d783b82 100644 >--- a/Documentation/virtual/kvm/api.txt >+++ b/Documentation/virtual/kvm/api.txt >@@ -199,11 +199,10 @@ Type: vm ioctl > Parameters: vcpu id (apic id on x86) > Returns: vcpu fd on success, -1 on error > >-This API adds a vcpu to a virtual machine. The vcpu id is a small integer >-in the range [0, max_vcpus). >+This API adds a vcpu to a virtual machine. The vcpu id is a positive integer. > >-The recommended max_vcpus value can be retrieved using the KVM_CAP_NR_VCPUS of >-the KVM_CHECK_EXTENSION ioctl() at run-time. >+The recommended maximum number of vcpus (max_vcpus) can be retrieved using the >+KVM_CAP_NR_VCPUS of the KVM_CHECK_EXTENSION ioctl() at run-time. > The maximum possible value for max_vcpus can be retrieved using the > KVM_CAP_MAX_VCPUS of the KVM_CHECK_EXTENSION ioctl() at run-time. > >diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c >index 70ef1a43c114..0278ea146db5 100644 >--- a/arch/mips/kvm/mips.c >+++ b/arch/mips/kvm/mips.c >@@ -248,9 +248,14 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, >unsigned int id) > int err, size, offset; > void *gebase; > int i; >+ struct kvm_vcpu *vcpu; > >- struct kvm_vcpu *vcpu = kzalloc(sizeof(struct kvm_vcpu), GFP_KERNEL); >+ if (id >= KVM_MAX_VCPUS) { >+ err = -EINVAL; >+ goto out; >+ } > >+ vcpu = kzalloc(sizeof(struct kvm_vcpu), GFP_KERNEL); > if (!vcpu) { > err = -ENOMEM; > goto out; >diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >index 9b7798c7b210..7738202edcce 100644 >--- a/arch/x86/kvm/x86.c >+++ b/arch/x86/kvm/x86.c >@@ -7358,6 +7358,9 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, > { > struct kvm_vcpu *vcpu; > >+ if (id >= KVM_MAX_VCPUS) >+ return ERR_PTR(-EINVAL); >+ > if (check_tsc_unstable() && atomic_read(&kvm->online_vcpus) != 0) > printk_once(KERN_WARNING > "kvm: SMP vm created on host with unstable TSC; " >diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c >index 4fd482fb9260..6b6cca3cb488 100644 >--- a/virt/kvm/kvm_main.c >+++ b/virt/kvm/kvm_main.c >@@ -2272,9 +2272,6 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 >id) > int r; > struct kvm_vcpu *vcpu; > >- if (id >= KVM_MAX_VCPUS) >- return -EINVAL; >- > vcpu = kvm_arch_vcpu_create(kvm, id); > if (IS_ERR(vcpu)) > return PTR_ERR(vcpu); > >-- >To unsubscribe from this list: send the line "unsubscribe kvm" in >the body of a message to majord...@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html -- Richard Yang\nHelp you, Help me
Re: [PATCH v4 2/2] KVM: move vcpu id checking to archs
Hi Radim ! On Thu, 21 Apr 2016 19:36:11 +0200 Radim Krčmář wrote: > 2016-04-21 18:45+0200, Greg Kurz: > > On Thu, 21 Apr 2016 18:00:19 +0200 > > Radim Krčmář wrote: > >> 2016-04-21 16:20+0200, Greg Kurz: > >> > Commit 338c7dbadd26 ("KVM: Improve create VCPU parameter > >> > (CVE-2013-4587)") > >> > introduced a check to prevent potential kernel memory corruption in case > >> > the vcpu id is too great. > >> > > >> > Unfortunately this check assumes vcpu ids grow in sequence with a common > >> > difference of 1, which is wrong: archs are free to use vcpu id as they > >> > fit. > >> > For example, QEMU originated vcpu ids for PowerPC cpus running in > >> > boot3s_hv > >> > mode, can grow with a common difference of 2, 4 or 8: if KVM_MAX_VCPUS is > >> > 1024, guests may be limited down to 128 vcpus on POWER8. > >> > > >> > This means the check does not belong here and should be moved to some > >> > arch > >> > specific function: kvm_arch_vcpu_create() looks like a good candidate. > >> > > >> > ARM and s390 already have such a check. > >> > > >> > I could not spot any path in the PowerPC or common KVM code where a vcpu > >> > id is used as described in the above commit: I believe PowerPC can live > >> > without this check. > >> > > >> > In the end, this patch simply moves the check to MIPS and x86. While > >> > here, > >> > we also update the documentation to dissociate vcpu ids from the maximum > >> > number of vcpus per virtual machine. > >> > > >> > Acked-by: James Hogan > >> > Acked-by: Cornelia Huck > >> > Signed-off-by: Greg Kurz > >> > --- > >> > v4: - updated subject for more clarity on what the patch does > >> > - added James's and Connie's A-b tags > >> > - updated documentation > >> > > >> > Documentation/virtual/kvm/api.txt |7 +++ > >> > arch/mips/kvm/mips.c |7 ++- > >> > arch/x86/kvm/x86.c|3 +++ > >> > virt/kvm/kvm_main.c |3 --- > >> > 4 files changed, 12 insertions(+), 8 deletions(-) > >> > > >> > diff --git a/Documentation/virtual/kvm/api.txt > >> > b/Documentation/virtual/kvm/api.txt > >> > index 4d0542c5206b..486a1d783b82 100644 > >> > --- a/Documentation/virtual/kvm/api.txt > >> > +++ b/Documentation/virtual/kvm/api.txt > >> > @@ -199,11 +199,10 @@ Type: vm ioctl > >> > Parameters: vcpu id (apic id on x86) > >> > Returns: vcpu fd on success, -1 on error > >> > > >> > -This API adds a vcpu to a virtual machine. The vcpu id is a small > >> > integer > >> > -in the range [0, max_vcpus). > >> > +This API adds a vcpu to a virtual machine. The vcpu id is a positive > >> > integer. > >> > >> Userspace won't be able to tell if KVM_CREATE_VCPU failed because it > >> provided too high vcpu_id to an old KVM or because new KVM failed in > >> other areas. Not a huge problem (because I expect that userspace will > >> die on both), but a new KVM_CAP would be able to disambiguate it. > >> > >> Toggleable capability doesn't seem necessary and only PowerPC changes, > >> so the capability could be arch specific ... I think that a generic one > >> makes more sense, though. > >> > > > > I'm not sure userspace can disambiguate all the cases where KVM_CREATE_VCPU > > returns EINVAL already... and, FWIW, QEMU simply exits if it gets an error. > > > > Yes, userspace cannot disambiguate, but would have the option of not > doing something that is destined to fail, like with KVM_CAP_MAX_VCPU. > It makes sense indeed. > > So I understand your concern but would we have a user for this ? > > I think so, new userspace on pre-patch KVM is the most likely one. > > Userspace cannot tell that KVM doesn't support the extension and > behaving like on patched KVM would result in a failure with cryptic > error message, because KVM only returns EINVAL. > This is already the case with or without the patch... which only changes things for PowerPC userspace. And in the case of QEMU, we're already violating the spec with the way we compute vcpu ids. > Btw. PowerPC QEMU tries vcpu_id >= KVM_MAX_VCPUS and fails, instead of > recognizing that the user wanted too much? > No. The error is caught in generic code and QEMU exits for all archs. And BTW, how would QEMU guess that vcpu id is too high ? I see at least three paths that can return EINVAL... > >> Userspace also doesn't know the vcpu id limit anymore, and it might > >> care. What do you think about returning the arch-specific limit (or the > >> highest positive integer) as int in KVM_CAP_MAX_VCPU_ID? > >> > > > > This is partly true: only arch agnostic code would be lost. > > > > Moreover this is a problem for powerpc only at the moment and userspace code > > can compute the vcpu_id limit out of KVM_CAP_MAX_VCPUS and KVM_CAP_PPC_SMT. > > > > How would that work on KVM without this patch? > It doesn't work for PowerPC :) KVM_CAP_MAX_VCPUS indicates we can can start, say, 1024 vcpus and KVM_CAP_PPC_SMT indicates the host has 8 threads p
Re: [PATCH v4 2/2] KVM: move vcpu id checking to archs
On Fri, 22 Apr 2016 17:21:03 +0800 Wei Yang wrote: > Hi, Greg > Hi Wei ! > One confusion. > > There are 5 kvm_arch_vcpu_create() while in this patch you changed 2 of them. > Some particular reason? > Yes and the reason is given in the changelog: - ARM and s390 already have such a check - PowerPC can live without this check - this patch simply moves the check to MIPS and x86 Does it clarify ? Cheers. -- Greg > On Thu, Apr 21, 2016 at 04:20:53PM +0200, Greg Kurz wrote: > >Commit 338c7dbadd26 ("KVM: Improve create VCPU parameter (CVE-2013-4587)") > >introduced a check to prevent potential kernel memory corruption in case > >the vcpu id is too great. > > > >Unfortunately this check assumes vcpu ids grow in sequence with a common > >difference of 1, which is wrong: archs are free to use vcpu id as they fit. > >For example, QEMU originated vcpu ids for PowerPC cpus running in boot3s_hv > >mode, can grow with a common difference of 2, 4 or 8: if KVM_MAX_VCPUS is > >1024, guests may be limited down to 128 vcpus on POWER8. > > > >This means the check does not belong here and should be moved to some arch > >specific function: kvm_arch_vcpu_create() looks like a good candidate. > > > >ARM and s390 already have such a check. > > > >I could not spot any path in the PowerPC or common KVM code where a vcpu > >id is used as described in the above commit: I believe PowerPC can live > >without this check. > > > >In the end, this patch simply moves the check to MIPS and x86. While here, > >we also update the documentation to dissociate vcpu ids from the maximum > >number of vcpus per virtual machine. > > > >Acked-by: James Hogan > >Acked-by: Cornelia Huck > >Signed-off-by: Greg Kurz > >--- > >v4: - updated subject for more clarity on what the patch does > >- added James's and Connie's A-b tags > >- updated documentation > > > > Documentation/virtual/kvm/api.txt |7 +++ > > arch/mips/kvm/mips.c |7 ++- > > arch/x86/kvm/x86.c|3 +++ > > virt/kvm/kvm_main.c |3 --- > > 4 files changed, 12 insertions(+), 8 deletions(-) > > > >diff --git a/Documentation/virtual/kvm/api.txt > >b/Documentation/virtual/kvm/api.txt > >index 4d0542c5206b..486a1d783b82 100644 > >--- a/Documentation/virtual/kvm/api.txt > >+++ b/Documentation/virtual/kvm/api.txt > >@@ -199,11 +199,10 @@ Type: vm ioctl > > Parameters: vcpu id (apic id on x86) > > Returns: vcpu fd on success, -1 on error > > > >-This API adds a vcpu to a virtual machine. The vcpu id is a small integer > >-in the range [0, max_vcpus). > >+This API adds a vcpu to a virtual machine. The vcpu id is a positive > >integer. > > > >-The recommended max_vcpus value can be retrieved using the KVM_CAP_NR_VCPUS > >of > >-the KVM_CHECK_EXTENSION ioctl() at run-time. > >+The recommended maximum number of vcpus (max_vcpus) can be retrieved using > >the > >+KVM_CAP_NR_VCPUS of the KVM_CHECK_EXTENSION ioctl() at run-time. > > The maximum possible value for max_vcpus can be retrieved using the > > KVM_CAP_MAX_VCPUS of the KVM_CHECK_EXTENSION ioctl() at run-time. > > > >diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c > >index 70ef1a43c114..0278ea146db5 100644 > >--- a/arch/mips/kvm/mips.c > >+++ b/arch/mips/kvm/mips.c > >@@ -248,9 +248,14 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, > >unsigned int id) > > int err, size, offset; > > void *gebase; > > int i; > >+struct kvm_vcpu *vcpu; > > > >-struct kvm_vcpu *vcpu = kzalloc(sizeof(struct kvm_vcpu), GFP_KERNEL); > >+if (id >= KVM_MAX_VCPUS) { > >+err = -EINVAL; > >+goto out; > >+} > > > >+vcpu = kzalloc(sizeof(struct kvm_vcpu), GFP_KERNEL); > > if (!vcpu) { > > err = -ENOMEM; > > goto out; > >diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > >index 9b7798c7b210..7738202edcce 100644 > >--- a/arch/x86/kvm/x86.c > >+++ b/arch/x86/kvm/x86.c > >@@ -7358,6 +7358,9 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, > > { > > struct kvm_vcpu *vcpu; > > > >+if (id >= KVM_MAX_VCPUS) > >+return ERR_PTR(-EINVAL); > >+ > > if (check_tsc_unstable() && atomic_read(&kvm->online_vcpus) != 0) > > printk_once(KERN_WARNING > > "kvm: SMP vm created on host with unstable TSC; " > >diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > >index 4fd482fb9260..6b6cca3cb488 100644 > >--- a/virt/kvm/kvm_main.c > >+++ b/virt/kvm/kvm_main.c > >@@ -2272,9 +2272,6 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, > >u32 id) > > int r; > > struct kvm_vcpu *vcpu; > > > >-if (id >= KVM_MAX_VCPUS) > >-return -EINVAL; > >- > > vcpu = kvm_arch_vcpu_create(kvm, id); > > if (IS_ERR(vcpu)) > > return PTR_ERR(vcpu); > > > >-- > >To unsubscribe from this list: send the line "unsubscribe kvm" in > >the body of a message to majord...@vger.kernel.org > >More majordomo info at http://vger.kern