[GIT PULL] RISC-V additional updates for v5.4-rc1
Linus, The following changes since commit b41dae061bbd722b9d7fa828f35d22035b218e18: Merge tag 'xfs-5.4-merge-7' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux (2019-09-18 18:32:43 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux.git tags/riscv/for-v5.4-rc1-b for you to fetch changes up to c82dd6d078a2bb29d41eda032bb96d05699a524d: riscv: Avoid interrupts being erroneously enabled in handle_exception() (2019-09-20 08:42:34 -0700) RISC-V additional updates for v5.4-rc1 Some additional RISC-V updates for v5.4-rc1. This includes one significant fix: - Prevent interrupts from being unconditionally re-enabled during exception handling if they were disabled in the context in which the exception occurred Also a few other fixes: - Fix a build error when sparse memory support is manually enabled - Prevent CPUs beyond CONFIG_NR_CPUS from being enabled in early boot And a few minor improvements: - DT improvements: in the FU540 SoC DT files, improve U-Boot compatibility by adding an "ethernet0" alias, drop an unnecessary property from the DT files, and add support for the PWM device - KVM preparation: add a KVM-related macro for future RISC-V KVM support, and export some symbols required to build KVM support as modules - defconfig additions: build more drivers by default for QEMU configurations Anup Patel (2): RISC-V: Enable VIRTIO drivers in RV64 and RV32 defconfig KVM: RISC-V: Add KVM_REG_RISCV for ONE_REG interface Atish Patra (1): RISC-V: Export kernel symbols for kvm Bin Meng (2): riscv: dts: sifive: Add ethernet0 to the aliases node riscv: dts: sifive: Drop "clock-frequency" property of cpu nodes Greentime Hu (1): RISC-V: Fix building error when CONFIG_SPARSEMEM_MANUAL=y Vincent Chen (1): riscv: Avoid interrupts being erroneously enabled in handle_exception() Xiang Wang (1): arch/riscv: disable excess harts before picking main boot hart Yash Shah (1): riscv: dts: Add DT support for SiFive FU540 PWM driver arch/riscv/boot/dts/sifive/fu540-c000.dtsi | 22 +--- .../riscv/boot/dts/sifive/hifive-unleashed-a00.dts | 8 arch/riscv/configs/defconfig | 11 ++ arch/riscv/configs/rv32_defconfig | 11 ++ arch/riscv/include/asm/pgtable.h | 24 +++--- arch/riscv/kernel/entry.S | 6 +- arch/riscv/kernel/head.S | 8 +--- arch/riscv/kernel/smp.c| 1 + arch/riscv/kernel/time.c | 1 + include/uapi/linux/kvm.h | 1 + 10 files changed, 74 insertions(+), 19 deletions(-)
Re: [PATCH] mbox: qcom: avoid unused-variable warning
Quoting Geert Uytterhoeven (2019-09-26 06:07:13) > Hi Stephen, > > On Fri, Sep 20, 2019 at 11:06 PM Stephen Boyd wrote: > > Quoting Arnd Bergmann (2019-09-20 12:27:50) > > > On Fri, Sep 20, 2019 at 6:45 PM Stephen Boyd wrote: > > > > > @@ -54,11 +60,6 @@ static int qcom_apcs_ipc_probe(struct > > > > > platform_device *pdev) > > > > > void __iomem *base; > > > > > unsigned long i; > > > > > int ret; > > > > > - const struct of_device_id apcs_clk_match_table[] = { > > > > > > > > Does marking it static here work too? It would be nice to limit the > > > > scope of this variable to this function instead of making it a global. > > > > Also, it might be slightly smaller code size if that works. > > > > > > No, I just tried and the warning returned. > > It's there for the convenience for the user, so he doesn't have to add it > himself explicitly. > > > ("of/device: Nullify match table in of_match_device() for CONFIG_OF=n"), > > but it's been 5 years! Surely we can revert this change now that commit > > 219a7bc577e6 ("spi: rspi: Use of_device_get_match_data() helper") is > > merged. > > Right, the particular error case in the RSPI driver can no longer happen. > > Calling of_device_get_match_data() is the better solution anyway, as > that uses the match table stored in dev->driver->of_match_table, so > there's no need to pass the table explicitly, and conditionally. > > Hence... > > > --- a/drivers/leds/leds-pca9532.c > > +++ b/drivers/leds/leds-pca9532.c > > @@ -472,7 +472,7 @@ pca9532_of_populate_pdata(struct device *dev, struct > > device_node *np) > > int i = 0; > > const char *state; > > > > - match = of_match_device(of_pca9532_leds_match, dev); > > + match = of_match_device(of_match_ptr(of_pca9532_leds_match), dev); > > if (!match) > > return ERR_PTR(-ENODEV); > > Please convert to of_device_get_match_data() instead of adding > of_match_ptr() invocations... How is this workable? I left it as of_match_device() because the value returned may be 0 for the enum and that looks the same as NULL. > > > @@ -525,7 +525,7 @@ static int pca9532_probe(struct i2c_client *client, > > dev_err(&client->dev, "no platform data\n"); > > return -EINVAL; > > } > > - of_id = of_match_device(of_pca9532_leds_match, > > + of_id = of_match_device(of_match_ptr(of_pca9532_leds_match), > > Likewise. > > > --- a/sound/soc/jz4740/jz4740-i2s.c > > +++ b/sound/soc/jz4740/jz4740-i2s.c > > @@ -503,7 +503,7 @@ static int jz4740_i2s_dev_probe(struct platform_device > > *pdev) > > if (!i2s) > > return -ENOMEM; > > > > - match = of_match_device(jz4740_of_matches, &pdev->dev); > > + match = of_match_device(of_match_ptr(jz4740_of_matches), > > &pdev->dev); > > Given of_device_get_match_data() returns NULL, not an error code, even > this one could use of_device_get_match_data(). >
Re: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property
On Wed, Sep 25, 2019 at 03:34:47AM +, Leo Li wrote: > > > > -Original Message- > > From: Biwen Li > > Sent: Tuesday, September 24, 2019 10:30 PM > > To: Leo Li ; shawn...@kernel.org; > > robh...@kernel.org; mark.rutl...@arm.com; Ran Wang > > > > Cc: linuxppc-...@lists.ozlabs.org; linux-arm-ker...@lists.infradead.org; > > linux-kernel@vger.kernel.org; devicet...@vger.kernel.org > > Subject: RE: [v3,3/3] Documentation: dt: binding: fsl: Add > > 'fsl,ippdexpcr-alt- > > addr' property > > > > > > > > > > > > > > The 'fsl,ippdexpcr-alt-addr' property is used to handle an > > > > > > errata > > > > > > A-008646 on LS1021A > > > > > > > > > > > > Signed-off-by: Biwen Li > > > > > > --- > > > > > > Change in v3: > > > > > > - rename property name > > > > > > fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr > > > > > > > > > > > > Change in v2: > > > > > > - update desc of the property 'fsl,rcpm-scfg' > > > > > > > > > > > > Documentation/devicetree/bindings/soc/fsl/rcpm.txt | 14 > > > > > > ++ > > > > > > 1 file changed, 14 insertions(+) > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt > > > > > > b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt > > > > > > index 5a33619d881d..157dcf6da17c 100644 > > > > > > --- a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt > > > > > > +++ b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt > > > > > > @@ -34,6 +34,11 @@ Chassis Version Example Chips > > > > > > Optional properties: > > > > > > - little-endian : RCPM register block is Little Endian. Without > > > > > > it RCPM > > > > > > will be Big Endian (default case). > > > > > > + - fsl,ippdexpcr-alt-addr : Must add the property for SoC > > > > > > + LS1021A, > > > > > > > > > > You probably should mention this is related to a hardware issue on > > > > > LS1021a and only needed on LS1021a. > > > > Okay, got it, thanks, I will add this in v4. > > > > > > > > > > > + Must include n + 1 entries (n = #fsl,rcpm-wakeup-cells, such as: > > > > > > + #fsl,rcpm-wakeup-cells equal to 2, then must include 2 + 1 > > > > > > entries). > > > > > > > > > > #fsl,rcpm-wakeup-cells is the number of IPPDEXPCR registers on an SoC. > > > > > However you are defining an offset to scfg registers here. Why > > > > > these two are related? The length here should actually be related > > > > > to the #address-cells of the soc/. But since this is only needed > > > > > for LS1021, you can > > > > just make it 3. > > > > I need set the value of IPPDEXPCR resgiters from ftm_alarm0 device > > > > node(fsl,rcpm-wakeup = <&rcpm 0x0 0x2000>; > > > > 0x0 is a value for IPPDEXPCR0, 0x2000 is a value for IPPDEXPCR1). > > > > But because of the hardware issue on LS1021A, I need store the value > > > > of IPPDEXPCR registers to an alt address. So I defining an offset to > > > > scfg registers, then RCPM driver get an abosolute address from > > > > offset, RCPM driver write the value of IPPDEXPCR registers to these > > > > abosolute addresses(backup the value of IPPDEXPCR registers). > > > > > > I understand what you are trying to do. The problem is that the new > > > fsl,ippdexpcr-alt-addr property contains a phandle and an offset. The > > > size of it shouldn't be related to #fsl,rcpm-wakeup-cells. > > You maybe like this: fsl,ippdexpcr-alt-addr = <&scfg 0x51c>;/* > > SCFG_SPARECR8 */ > > No. The #address-cell for the soc/ is 2, so the offset to scfg > should be 0x0 0x51c. The total size should be 3, but it shouldn't be > coming from #fsl,rcpm-wakeup-cells like you mentioned in the binding. Um, no. #address-cells applies to reg and ranges, not some vendor specific property. You can just define how many cells you need if that's fixed. However, I suggested doing this another way in the next version of this. Rob
Re: [PATCH net] net: socionext: ave: Avoid using netdev_err() before calling register_netdev()
From: Kunihiko Hayashi Date: Thu, 26 Sep 2019 15:35:10 +0900 > Until calling register_netdev(), ndev->dev_name isn't specified, and > netdev_err() displays "(unnamed net_device)". > > ave 6500.ethernet (unnamed net_device) (uninitialized): invalid > phy-mode setting > ave: probe of 6500.ethernet failed with error -22 > > This replaces netdev_err() with dev_err() before calling register_netdev(). > > Signed-off-by: Kunihiko Hayashi Applied.
Re: [PATCH] net: phy: micrel: add Asym Pause workaround for KSZ9021
From: Hans Andersson Date: Thu, 26 Sep 2019 09:54:37 +0200 > From: Hans Andersson > > The Micrel KSZ9031 PHY may fail to establish a link when the Asymmetric > Pause capability is set. This issue is described in a Silicon Errata > (DS8691D or DS8692D), which advises to always disable the > capability. > > Micrel KSZ9021 has no errata, but has the same issue with Asymmetric Pause. > This patch apply the same workaround as the one for KSZ9031. > > Signed-off-by: Hans Andersson Applied and queued up for -stable.
Re: [PATCH] NFC: st95hf: clean up indentation issue
From: Colin King Date: Thu, 26 Sep 2019 12:13:06 +0100 > From: Colin Ian King > > The return statement is indented incorrectly, add in a missing > tab and remove an extraneous space after the return > > Signed-off-by: Colin Ian King Applied.
Re: [PATCH v3 1/5] media: dt-bindings: Add jpeg enc device tree node document
On Tue, 24 Sep 2019 15:43:00 +0800, Xia Jiang wrote: > Add jpeg enc device tree node document. > > Signed-off-by: Xia Jiang > --- > v3: change compatible to SoC specific compatible > > v2: no changes > --- > .../bindings/media/mediatek-jpeg-encoder.txt | 37 +++ > 1 file changed, 37 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt > Reviewed-by: Rob Herring
Re: [PATCH] net: ena: clean up indentation issue
From: Colin King Date: Thu, 26 Sep 2019 12:22:52 +0100 > From: Colin Ian King > > There memset is indented incorrectly, remove the extraneous tabs. > > Signed-off-by: Colin Ian King Applied.
[Patch 04/16] media: ti-vpe: Add support for SEQ_BT
From: Nikhil Devshatwar SEQ_BT indicates the buffer for bottom field needs to be processed before the top field. Simplify the field selection logic to support SEQ_BT as well. Modify the interlace flags to include any of alternate, SEQ_TB, SEQ_BT. Update other format error checking to consider SEQ_BT. Replace SEQ_TB with SEQ_XX wherever applicable. Signed-off-by: Nikhil Devshatwar Signed-off-by: Benoit Parrot --- drivers/media/platform/ti-vpe/vpe.c | 73 ++--- 1 file changed, 46 insertions(+), 27 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 0e9cb0319a92..5d0ec5f7ca25 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -328,9 +328,14 @@ struct vpe_q_data { #defineQ_DATA_MODE_TILED BIT(1) #defineQ_DATA_INTERLACED_ALTERNATE BIT(2) #defineQ_DATA_INTERLACED_SEQ_TBBIT(3) +#defineQ_DATA_INTERLACED_SEQ_BTBIT(4) + +#define Q_IS_SEQ_XX(Q_DATA_INTERLACED_SEQ_TB | \ + Q_DATA_INTERLACED_SEQ_BT) #define Q_IS_INTERLACED(Q_DATA_INTERLACED_ALTERNATE | \ - Q_DATA_INTERLACED_SEQ_TB) + Q_DATA_INTERLACED_SEQ_TB | \ + Q_DATA_INTERLACED_SEQ_BT) enum { Q_DATA_SRC = 0, @@ -1105,24 +1110,31 @@ static void add_in_dtd(struct vpe_ctx *ctx, int port) dma_addr += offset; stride = q_data->bytesperline[VPE_LUMA]; - if (q_data->flags & Q_DATA_INTERLACED_SEQ_TB) { - /* -* Use top or bottom field from same vb alternately -* f,f-1,f-2 = TBT when seq is even -* f,f-1,f-2 = BTB when seq is odd -*/ - field = (p_data->vb_index + (ctx->sequence % 2)) % 2; + /* +* field used in VPDMA desc = 0 (top) / 1 (bottom) +* Use top or bottom field from same vb alternately +* For each de-interlacing operation, f,f-1,f-2 should be one +* of TBT or BTB +*/ + if (q_data->flags & Q_DATA_INTERLACED_SEQ_TB || + q_data->flags & Q_DATA_INTERLACED_SEQ_BT) { + /* Select initial value based on format */ + if (q_data->flags & Q_DATA_INTERLACED_SEQ_BT) + field = 1; + else + field = 0; + + /* Toggle for each vb_index and each operation */ + field = (field + p_data->vb_index + ctx->sequence) % 2; if (field) { - /* -* bottom field of a SEQ_TB buffer -* Skip the top field data by -*/ int height = q_data->height / 2; int bpp = fmt->fourcc == V4L2_PIX_FMT_NV12 ? 1 : (vpdma_fmt->depth >> 3); + if (plane) height /= 2; + dma_addr += q_data->width * height * bpp; } } @@ -1177,12 +1189,14 @@ static void device_run(void *priv) struct vpe_q_data *d_q_data = &ctx->q_data[Q_DATA_DST]; struct vpe_q_data *s_q_data = &ctx->q_data[Q_DATA_SRC]; - if (ctx->deinterlacing && s_q_data->flags & Q_DATA_INTERLACED_SEQ_TB && - ctx->sequence % 2 == 0) { - /* When using SEQ_TB buffers, When using it first time, -* No need to remove the buffer as the next field is present -* in the same buffer. (so that job_ready won't fail) -* It will be removed when using bottom field + if (ctx->deinterlacing && s_q_data->flags & Q_IS_SEQ_XX && + ctx->sequence % 2 == 0) { + /* When using SEQ_XX type buffers, each buffer has two fields +* each buffer has two fields (top & bottom) +* Removing one buffer is actually getting two fields +* Alternate between two operations:- +* Even : consume one field but DO NOT REMOVE from queue +* Odd : consume other field and REMOVE from queue */ ctx->src_vbs[0] = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx); WARN_ON(ctx->src_vbs[0] == NULL); @@ -1573,8 +1587,10 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f, return -EINVAL; } - if (pix->field != V4L2_FIELD_NONE && pix->field != V4L2_FIELD_ALTERNATE - && pix->field != V4L2_F
[Patch 02/16] media: ti-vpe: vpe: Add missing null pointer checks
A few NULL pointer checks were missing. Add check with appropriate return code. Signed-off-by: Benoit Parrot --- drivers/media/platform/ti-vpe/vpe.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 5ba72445584d..56f60dbea15c 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1537,6 +1537,8 @@ static int vpe_g_fmt(struct file *file, void *priv, struct v4l2_format *f) return -EINVAL; q_data = get_q_data(ctx, f->type); + if (!q_data) + return -EINVAL; pix->width = q_data->width; pix->height = q_data->height; @@ -2001,6 +2003,8 @@ static int vpe_queue_setup(struct vb2_queue *vq, struct vpe_q_data *q_data; q_data = get_q_data(ctx, vq->type); + if (!q_data) + return -EINVAL; *nplanes = q_data->nplanes; @@ -2025,6 +2029,8 @@ static int vpe_buf_prepare(struct vb2_buffer *vb) vpe_dbg(ctx->dev, "type: %d\n", vb->vb2_queue->type); q_data = get_q_data(ctx, vb->vb2_queue->type); + if (!q_data) + return -EINVAL; num_planes = q_data->nplanes; if (vb->vb2_queue->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { @@ -2481,7 +2487,12 @@ static int vpe_probe(struct platform_device *pdev) mutex_init(&dev->dev_mutex); dev->res = platform_get_resource_byname(pdev, IORESOURCE_MEM, - "vpe_top"); + "vpe_top"); + if (!dev->res) { + dev_err(&pdev->dev, "missing 'vpe_top' resources data\n"); + return -ENODEV; + } + /* * HACK: we get resource info from device tree in the form of a list of * VPE sub blocks, the driver currently uses only the base of vpe_top -- 2.17.1
[Patch 03/16] media: ti-vpe: vpe: Remove unnecessary use of container_of
Instead of saving a pointer to the 'fh' member of struct vpe_ctx to later have to use container_of to retrieve the actual pointer to the context structure, which seems to confuse static code analysis tool anyways, just save the pointer to the actual structure and then retrieve it directly. Signed-off-by: Benoit Parrot --- drivers/media/platform/ti-vpe/vpe.c | 22 +++--- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 56f60dbea15c..0e9cb0319a92 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -900,14 +900,6 @@ static int set_srcdst_params(struct vpe_ctx *ctx) return 0; } -/* - * Return the vpe_ctx structure for a given struct file - */ -static struct vpe_ctx *file2ctx(struct file *file) -{ - return container_of(file->private_data, struct vpe_ctx, fh); -} - /* * mem2mem callbacks */ @@ -1527,7 +1519,7 @@ static int vpe_enum_fmt(struct file *file, void *priv, static int vpe_g_fmt(struct file *file, void *priv, struct v4l2_format *f) { struct v4l2_pix_format_mplane *pix = &f->fmt.pix_mp; - struct vpe_ctx *ctx = file2ctx(file); + struct vpe_ctx *ctx = file->private_data; struct vb2_queue *vq; struct vpe_q_data *q_data; int i; @@ -1689,7 +1681,7 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f, static int vpe_try_fmt(struct file *file, void *priv, struct v4l2_format *f) { - struct vpe_ctx *ctx = file2ctx(file); + struct vpe_ctx *ctx = file->private_data; struct vpe_fmt *fmt = find_format(f); if (V4L2_TYPE_IS_OUTPUT(f->type)) @@ -1762,7 +1754,7 @@ static int __vpe_s_fmt(struct vpe_ctx *ctx, struct v4l2_format *f) static int vpe_s_fmt(struct file *file, void *priv, struct v4l2_format *f) { int ret; - struct vpe_ctx *ctx = file2ctx(file); + struct vpe_ctx *ctx = file->private_data; ret = vpe_try_fmt(file, priv, f); if (ret) @@ -1847,7 +1839,7 @@ static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s) static int vpe_g_selection(struct file *file, void *fh, struct v4l2_selection *s) { - struct vpe_ctx *ctx = file2ctx(file); + struct vpe_ctx *ctx = file->private_data; struct vpe_q_data *q_data; bool use_c_rect = false; @@ -1908,7 +1900,7 @@ static int vpe_g_selection(struct file *file, void *fh, static int vpe_s_selection(struct file *file, void *fh, struct v4l2_selection *s) { - struct vpe_ctx *ctx = file2ctx(file); + struct vpe_ctx *ctx = file->private_data; struct vpe_q_data *q_data; struct v4l2_selection sel = *s; int ret; @@ -2275,7 +2267,7 @@ static int vpe_open(struct file *file) init_adb_hdrs(ctx); v4l2_fh_init(&ctx->fh, video_devdata(file)); - file->private_data = &ctx->fh; + file->private_data = ctx; hdl = &ctx->hdl; v4l2_ctrl_handler_init(hdl, 1); @@ -2360,7 +2352,7 @@ static int vpe_open(struct file *file) static int vpe_release(struct file *file) { struct vpe_dev *dev = video_drvdata(file); - struct vpe_ctx *ctx = file2ctx(file); + struct vpe_ctx *ctx = file->private_data; vpe_dbg(dev, "releasing instance %p\n", ctx); -- 2.17.1
[Patch 08/16] media: ti-vpe: vpe: fix a v4l2-compliance warning about invalid pixel format
v4l2-compliance warns with this message: warn: v4l2-test-formats.cpp(717): \ TRY_FMT cannot handle an invalid pixelformat. warn: v4l2-test-formats.cpp(718): \ This may or may not be a problem. For more information see: warn: v4l2-test-formats.cpp(719): \ http://www.mail-archive.com/linux-media@vger.kernel.org/msg56550.html ... test VIDIOC_TRY_FMT: FAIL We need to make sure that the returns a valid pixel format in all instance. Based on the v4l2 framework convention drivers must return a valid pixel format when the requested pixel format is either invalid or not supported. Signed-off-by: Benoit Parrot Reviewed-by: Tomi Valkeinen --- drivers/media/platform/ti-vpe/vpe.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 1278d457f753..d3f1ae8b72fa 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -351,20 +351,25 @@ enum { }; /* find our format description corresponding to the passed v4l2_format */ -static struct vpe_fmt *find_format(struct v4l2_format *f) +static struct vpe_fmt *__find_format(u32 fourcc) { struct vpe_fmt *fmt; unsigned int k; for (k = 0; k < ARRAY_SIZE(vpe_formats); k++) { fmt = &vpe_formats[k]; - if (fmt->fourcc == f->fmt.pix.pixelformat) + if (fmt->fourcc == fourcc) return fmt; } return NULL; } +static struct vpe_fmt *find_format(struct v4l2_format *f) +{ + return __find_format(f->fmt.pix.pixelformat); +} + /* * there is one vpe_dev structure in the driver, it is shared by * all instances. @@ -1599,9 +1604,9 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f, unsigned int stride = 0; if (!fmt || !(fmt->types & type)) { - vpe_err(ctx->dev, "Fourcc format (0x%08x) invalid.\n", + vpe_dbg(ctx->dev, "Fourcc format (0x%08x) invalid.\n", pix->pixelformat); - return -EINVAL; + fmt = __find_format(V4L2_PIX_FMT_YUYV); } if (pix->field != V4L2_FIELD_NONE && -- 2.17.1
[Patch 16/16] media: ti-vpe: vpe: don't rely on colorspace member for conversion
Up to now VPE was relying on the colorspace value of struct v4l2_format as an indication to perform color space conversion from YUV to RGB or not. Instead we should used the source/destination fourcc codes as a more reliable indication to perform color space conversion or not. Signed-off-by: Benoit Parrot --- drivers/media/platform/ti-vpe/vpe.c | 41 ++--- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index e30981cd3e8f..d002adc6263f 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -353,6 +353,32 @@ enum { Q_DATA_DST = 1, }; +static bool is_fmt_rgb(u32 fourcc) +{ + if (fourcc == V4L2_PIX_FMT_RGB24 || + fourcc == V4L2_PIX_FMT_BGR24 || + fourcc == V4L2_PIX_FMT_RGB32 || + fourcc == V4L2_PIX_FMT_BGR32 || + fourcc == V4L2_PIX_FMT_RGB565 || + fourcc == V4L2_PIX_FMT_RGB555) + return true; + + return false; +} + +/* + * This helper is only used to setup the color space converter + * the actual value returned is only to broadly differentiate between + * RGB and YUV + */ +static enum v4l2_colorspace fourcc_to_colorspace(u32 fourcc) +{ + if (is_fmt_rgb(fourcc)) + return V4L2_COLORSPACE_SRGB; + + return V4L2_COLORSPACE_SMPTE170M; +} + /* find our format description corresponding to the passed v4l2_format */ static struct vpe_fmt *__find_format(u32 fourcc) { @@ -764,11 +790,10 @@ static void set_src_registers(struct vpe_ctx *ctx) static void set_dst_registers(struct vpe_ctx *ctx) { struct vpe_mmr_adb *mmr_adb = ctx->mmr_adb.addr; - enum v4l2_colorspace clrspc = ctx->q_data[Q_DATA_DST].colorspace; struct vpe_fmt *fmt = ctx->q_data[Q_DATA_DST].fmt; u32 val = 0; - if (clrspc == V4L2_COLORSPACE_SRGB) { + if (is_fmt_rgb(fmt->fourcc)) { val |= VPE_RGB_OUT_SELECT; vpdma_set_bg_color(ctx->dev->vpdma, (struct vpdma_data_format *)fmt->vpdma_fmt[0], 0xff); @@ -912,7 +937,8 @@ static int set_srcdst_params(struct vpe_ctx *ctx) set_dei_regs(ctx); csc_set_coeff(ctx->dev->csc, &mmr_adb->csc_regs[0], - s_q_data->colorspace, d_q_data->colorspace); + fourcc_to_colorspace(s_q_data->fmt->fourcc), + fourcc_to_colorspace(d_q_data->fmt->fourcc)); sc_set_hs_coeffs(ctx->dev->sc, ctx->sc_coeff_h.addr, src_w, dst_w); sc_set_vs_coeffs(ctx->dev->sc, ctx->sc_coeff_v.addr, src_h, dst_h); @@ -1285,7 +1311,7 @@ static void device_run(void *priv) if (ctx->deinterlacing) add_out_dtd(ctx, VPE_PORT_MV_OUT); - if (d_q_data->colorspace == V4L2_COLORSPACE_SRGB) { + if (is_fmt_rgb(d_q_data->fmt->fourcc)) { add_out_dtd(ctx, VPE_PORT_RGB_OUT); } else { add_out_dtd(ctx, VPE_PORT_LUMA_OUT); @@ -1327,7 +1353,7 @@ static void device_run(void *priv) } /* sync on channel control descriptors for output ports */ - if (d_q_data->colorspace == V4L2_COLORSPACE_SRGB) { + if (is_fmt_rgb(d_q_data->fmt->fourcc)) { vpdma_add_sync_on_channel_ctd(&ctx->desc_list, VPE_CHAN_RGB_OUT); } else { @@ -1682,10 +1708,7 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f, height = pix->height; if (!pix->colorspace) { - if (fmt->fourcc == V4L2_PIX_FMT_RGB24 || - fmt->fourcc == V4L2_PIX_FMT_BGR24 || - fmt->fourcc == V4L2_PIX_FMT_RGB32 || - fmt->fourcc == V4L2_PIX_FMT_BGR32) { + if (is_fmt_rgb(fmt->fourcc)) { pix->colorspace = V4L2_COLORSPACE_SRGB; } else { if (height > 1280) /* HD */ -- 2.17.1
[Patch 15/16] media: ti-vpe: vpe: fix v4l2_compliance issue related to xfer_func
All 4 of the "colorspace" components were not originally handled. Causing issue related to xfer_func not being initialized properly. This was found with v4l2-compliance test. Signed-off-by: Benoit Parrot --- drivers/media/platform/ti-vpe/vpe.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index d7f8eb901475..e30981cd3e8f 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -324,6 +324,9 @@ struct vpe_q_data { unsigned intnplanes;/* Current number of planes */ unsigned intbytesperline[VPE_MAX_PLANES]; /* bytes per line in memory */ enum v4l2_colorspacecolorspace; + enum v4l2_ycbcr_encoding ycbcr_enc; + enum v4l2_xfer_func xfer_func; + enum v4l2_quantization quant; enum v4l2_field field; /* supported field value */ unsigned intflags; unsigned intsizeimage[VPE_MAX_PLANES]; /* image size in memory */ @@ -1576,6 +1579,9 @@ static int vpe_g_fmt(struct file *file, void *priv, struct v4l2_format *f) if (V4L2_TYPE_IS_OUTPUT(f->type)) { pix->colorspace = q_data->colorspace; + pix->xfer_func = q_data->xfer_func; + pix->ycbcr_enc = q_data->ycbcr_enc; + pix->quantization = q_data->quant; } else { struct vpe_q_data *s_q_data; @@ -1583,6 +1589,9 @@ static int vpe_g_fmt(struct file *file, void *priv, struct v4l2_format *f) s_q_data = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); pix->colorspace = s_q_data->colorspace; + pix->xfer_func = s_q_data->xfer_func; + pix->ycbcr_enc = s_q_data->ycbcr_enc; + pix->quantization = s_q_data->quant; } pix->num_planes = q_data->nplanes; @@ -1758,6 +1767,9 @@ static int __vpe_s_fmt(struct vpe_ctx *ctx, struct v4l2_format *f) q_data->width = pix->width; q_data->height = pix->height; q_data->colorspace = pix->colorspace; + q_data->xfer_func = pix->xfer_func; + q_data->ycbcr_enc = pix->ycbcr_enc; + q_data->quant = pix->quantization; q_data->field = pix->field; q_data->nplanes = pix->num_planes; -- 2.17.1
[Patch 01/16] media: ti-vpe: vpe: Fix Motion Vector vpdma stride
commit 52831a418fa6 ("[media] media: ti-vpe: vpe: allow use of user specified stride") and commit 8c1e4fa17e92 ("[media] media: ti-vpe: vpdma: add support for user specified stride") resulted in the Motion Vector stride to be the same as the image stride. This caused memory corruption in the output image as mentionned in commit 44f98adf71a8 ("[media] media: ti-vpe: vpe: Fix line stride for output motion vector"). Fixes: 52831a418fa6 ("[media] media: ti-vpe: vpe: allow use of user specified stride") Fixes: 8c1e4fa17e92 ("[media] media: ti-vpe: vpdma: add support for user specified stride") Signed-off-by: Benoit Parrot Acked-by: Nikhil Devshatwar --- drivers/media/platform/ti-vpe/vpe.c | 18 +- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 60b575bb44c4..5ba72445584d 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1013,11 +1013,14 @@ static void add_out_dtd(struct vpe_ctx *ctx, int port) dma_addr_t dma_addr; u32 flags = 0; u32 offset = 0; + u32 stride; if (port == VPE_PORT_MV_OUT) { vpdma_fmt = &vpdma_misc_fmts[VPDMA_DATA_FMT_MV]; dma_addr = ctx->mv_buf_dma[mv_buf_selector]; q_data = &ctx->q_data[Q_DATA_SRC]; + stride = ALIGN((q_data->width * vpdma_fmt->depth) >> 3, + VPDMA_STRIDE_ALIGN); } else { /* to incorporate interleaved formats */ int plane = fmt->coplanar ? p_data->vb_part : 0; @@ -1044,6 +1047,7 @@ static void add_out_dtd(struct vpe_ctx *ctx, int port) } /* Apply the offset */ dma_addr += offset; + stride = q_data->bytesperline[VPE_LUMA]; } if (q_data->flags & Q_DATA_FRAME_1D) @@ -1055,7 +1059,7 @@ static void add_out_dtd(struct vpe_ctx *ctx, int port) MAX_W, MAX_H); vpdma_add_out_dtd(&ctx->desc_list, q_data->width, - q_data->bytesperline[VPE_LUMA], &q_data->c_rect, + stride, &q_data->c_rect, vpdma_fmt, dma_addr, MAX_OUT_WIDTH_REG1, MAX_OUT_HEIGHT_REG1, p_data->channel, flags); } @@ -1074,10 +1078,13 @@ static void add_in_dtd(struct vpe_ctx *ctx, int port) dma_addr_t dma_addr; u32 flags = 0; u32 offset = 0; + u32 stride; if (port == VPE_PORT_MV_IN) { vpdma_fmt = &vpdma_misc_fmts[VPDMA_DATA_FMT_MV]; dma_addr = ctx->mv_buf_dma[mv_buf_selector]; + stride = ALIGN((q_data->width * vpdma_fmt->depth) >> 3, + VPDMA_STRIDE_ALIGN); } else { /* to incorporate interleaved formats */ int plane = fmt->coplanar ? p_data->vb_part : 0; @@ -1104,6 +,7 @@ static void add_in_dtd(struct vpe_ctx *ctx, int port) } /* Apply the offset */ dma_addr += offset; + stride = q_data->bytesperline[VPE_LUMA]; if (q_data->flags & Q_DATA_INTERLACED_SEQ_TB) { /* @@ -1139,10 +1147,10 @@ static void add_in_dtd(struct vpe_ctx *ctx, int port) if (p_data->vb_part && fmt->fourcc == V4L2_PIX_FMT_NV12) frame_height /= 2; - vpdma_add_in_dtd(&ctx->desc_list, q_data->width, -q_data->bytesperline[VPE_LUMA], &q_data->c_rect, - vpdma_fmt, dma_addr, p_data->channel, field, flags, frame_width, - frame_height, 0, 0); + vpdma_add_in_dtd(&ctx->desc_list, q_data->width, stride, +&q_data->c_rect, vpdma_fmt, dma_addr, +p_data->channel, field, flags, frame_width, +frame_height, 0, 0); } /* -- 2.17.1
[Patch 12/16] media: ti-vpe: vpe: ensure buffers are cleaned up properly in abort cases
v4l2-compliance fails with this message: fail: v4l2-test-buffers.cpp(691): ret == 0 fail: v4l2-test-buffers.cpp(974): captureBufs(node, q, m2m_q, frame_count, true) test MMAP: FAIL This caused the following Kernel Warning: WARNING: CPU: 0 PID: 961 at drivers/media/v4l2-core/videobuf2-core.c:1658 __vb2_queue_cancel+0x174/0x1d8 ... CPU: 0 PID: 961 Comm: v4l2-compliance Not tainted 4.14.62-01720-g20ecd717e87a #6 Hardware name: Generic DRA72X (Flattened Device Tree) Backtrace: [] (dump_backtrace) from [] (show_stack+0x18/0x1c) r7:0009 r6:60070013 r5: r4:c1053824 [] (show_stack) from [] (dump_stack+0x90/0xa4) [] (dump_stack) from [] (__warn+0xec/0x104) r7:0009 r6:c0c0ad50 r5: r4: [] (__warn) from [] (warn_slowpath_null+0x28/0x30) r9:0008 r8: r7:eced4808 r6:edbc9bac r5:eced4844 r4:eced4808 [] (warn_slowpath_null) from [] (__vb2_queue_cancel+0x174/0x1d8) [] (__vb2_queue_cancel) from [] (vb2_core_queue_release+0x20/0x40) r10:ecc7bd70 r9:0008 r8: r7:edb73010 r6:edbc9bac r5:eced4844 r4:eced4808 r3:0004 [] (vb2_core_queue_release) from [] (vb2_queue_release+0x10/0x14) r5:edbc9810 r4:eced4800 [] (vb2_queue_release) from [] (v4l2_m2m_ctx_release+0x1c/0x30) [] (v4l2_m2m_ctx_release) from [] (vpe_release+0x74/0xb0 [ti_vpe]) r5:edbc9810 r4:ed67a400 [] (vpe_release [ti_vpe]) from [] (v4l2_release+0x3c/0x80) r7:edb73010 r6:ed176aa0 r5:edbc9868 r4:ed5119c0 [] (v4l2_release) from [] (__fput+0x8c/0x1dc) r5:ecc7bd70 r4:ed5119c0 [] (__fput) from [] (fput+0x10/0x14) r10: r9:ed5119c0 r8:ece392d0 r7:c1059544 r6:ece38d80 r5:ece392b4 r4: [] (fput) from [] (task_work_run+0x98/0xb8) [] (task_work_run) from [] (do_exit+0x170/0xa80) r9:ece351fc r8: r7:ecde3f58 r6:e000 r5:ece351c0 r4:ece38d80 [] (do_exit) from [] (do_group_exit+0x48/0xc4) r7:00f8 [] (do_group_exit) from [] (__wake_up_parent+0x0/0x28) r7:00f8 r6:b6c6a798 r5:0001 r4:0001 [] (SyS_exit_group) from [] (ret_fast_syscall+0x0/0x4c) These warnings are caused by buffers which not properly cleaned up/release during an abort use case. In the abort cases the VPDMA desc buffers would still be mapped and the in-flight VB2 buffers would not be released properly causing a kernel warning from being generated by the videobuf2-core level. Signed-off-by: Benoit Parrot Reviewed-by: Tomi Valkeinen --- drivers/media/platform/ti-vpe/vpe.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 8ab1c3241b74..ad9d8b559cad 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1427,9 +1427,6 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data) /* the previous dst mv buffer becomes the next src mv buffer */ ctx->src_mv_buf_selector = !ctx->src_mv_buf_selector; - if (ctx->aborting) - goto finished; - s_vb = ctx->src_vbs[0]; d_vb = ctx->dst_vb; @@ -1494,6 +1491,9 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data) ctx->src_vbs[0] = NULL; ctx->dst_vb = NULL; + if (ctx->aborting) + goto finished; + ctx->bufs_completed++; if (ctx->bufs_completed < ctx->bufs_per_job && job_ready(ctx)) { device_run(ctx); @@ -2404,6 +2404,12 @@ static int vpe_release(struct file *file) mutex_lock(&dev->dev_mutex); free_mv_buffers(ctx); + + vpdma_unmap_desc_buf(dev->vpdma, &ctx->desc_list.buf); + vpdma_unmap_desc_buf(dev->vpdma, &ctx->mmr_adb); + vpdma_unmap_desc_buf(dev->vpdma, &ctx->sc_coeff_h); + vpdma_unmap_desc_buf(dev->vpdma, &ctx->sc_coeff_v); + vpdma_free_desc_list(&ctx->desc_list); vpdma_free_desc_buf(&ctx->mmr_adb); -- 2.17.1
[Patch 14/16] media: ti-vpe: Set the DMA mask and coherent mask
VPE uses VPDMA (built-in dma engine) to transfer data to and from the IP and memory. VPDMA expect 32 bits addresses. To make sure that is always the case set the DMA mask and coherent mask for the device. Signed-off-by: Benoit Parrot --- drivers/media/platform/ti-vpe/vpe.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index ad9d8b559cad..d7f8eb901475 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -2517,6 +2517,13 @@ static int vpe_probe(struct platform_device *pdev) struct vpe_dev *dev; int ret, irq, func; + ret = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); + if (ret) { + dev_err(&pdev->dev, + "32-bit consistent DMA enable failed\n"); + return ret; + } + dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL); if (!dev) return -ENOMEM; -- 2.17.1
[Patch 05/16] media: ti-vpe: Add support for NV21 format
From: Nikhil Devshatwar In NV21 format, the chroma plane is written to memory such that the U and V components are swapped for NV12. Create a new entry in the VPDMA formats to describe the correct data types used in the data descriptors. Update all checks for NV12 and add NV21 there as well. Add support for V4L2_PIX_FMT_NV21 format for both capture and output streams. Signed-off-by: Nikhil Devshatwar Signed-off-by: Benoit Parrot Reviewed-by: Tomi Valkeinen --- drivers/media/platform/ti-vpe/vpdma.c | 11 ++-- drivers/media/platform/ti-vpe/vpdma.h | 1 + drivers/media/platform/ti-vpe/vpdma_priv.h | 1 + drivers/media/platform/ti-vpe/vpe.c| 29 +- 4 files changed, 34 insertions(+), 8 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpdma.c b/drivers/media/platform/ti-vpe/vpdma.c index 53d27cd6e10a..817d287c8138 100644 --- a/drivers/media/platform/ti-vpe/vpdma.c +++ b/drivers/media/platform/ti-vpe/vpdma.c @@ -56,6 +56,11 @@ const struct vpdma_data_format vpdma_yuv_fmts[] = { .data_type = DATA_TYPE_C420, .depth = 4, }, + [VPDMA_DATA_FMT_CB420] = { + .type = VPDMA_DATA_FMT_TYPE_YUV, + .data_type = DATA_TYPE_CB420, + .depth = 4, + }, [VPDMA_DATA_FMT_YCR422] = { .type = VPDMA_DATA_FMT_TYPE_YUV, .data_type = DATA_TYPE_YCR422, @@ -825,7 +830,8 @@ void vpdma_rawchan_add_out_dtd(struct vpdma_desc_list *list, int width, channel = next_chan = raw_vpdma_chan; if (fmt->type == VPDMA_DATA_FMT_TYPE_YUV && - fmt->data_type == DATA_TYPE_C420) { + (fmt->data_type == DATA_TYPE_C420 || +fmt->data_type == DATA_TYPE_CB420)) { rect.height >>= 1; rect.top >>= 1; depth = 8; @@ -893,7 +899,8 @@ void vpdma_add_in_dtd(struct vpdma_desc_list *list, int width, channel = next_chan = chan_info[chan].num; if (fmt->type == VPDMA_DATA_FMT_TYPE_YUV && - fmt->data_type == DATA_TYPE_C420) { + (fmt->data_type == DATA_TYPE_C420 || +fmt->data_type == DATA_TYPE_CB420)) { rect.height >>= 1; rect.top >>= 1; depth = 8; diff --git a/drivers/media/platform/ti-vpe/vpdma.h b/drivers/media/platform/ti-vpe/vpdma.h index 28bc94129348..bce17329c4c9 100644 --- a/drivers/media/platform/ti-vpe/vpdma.h +++ b/drivers/media/platform/ti-vpe/vpdma.h @@ -71,6 +71,7 @@ enum vpdma_yuv_formats { VPDMA_DATA_FMT_C444, VPDMA_DATA_FMT_C422, VPDMA_DATA_FMT_C420, + VPDMA_DATA_FMT_CB420, VPDMA_DATA_FMT_YCR422, VPDMA_DATA_FMT_YC444, VPDMA_DATA_FMT_CRY422, diff --git a/drivers/media/platform/ti-vpe/vpdma_priv.h b/drivers/media/platform/ti-vpe/vpdma_priv.h index c488609bc162..d8ae3e1cd54d 100644 --- a/drivers/media/platform/ti-vpe/vpdma_priv.h +++ b/drivers/media/platform/ti-vpe/vpdma_priv.h @@ -92,6 +92,7 @@ #define DATA_TYPE_C444 0x4 #define DATA_TYPE_C422 0x5 #define DATA_TYPE_C420 0x6 +#define DATA_TYPE_CB4200x16 #define DATA_TYPE_YC4440x8 #define DATA_TYPE_YCB422 0x7 #define DATA_TYPE_YCR422 0x17 diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 5d0ec5f7ca25..f3ee9ff87927 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -248,6 +248,14 @@ static struct vpe_fmt vpe_formats[] = { &vpdma_yuv_fmts[VPDMA_DATA_FMT_C420], }, }, + { + .fourcc = V4L2_PIX_FMT_NV21, + .types = VPE_FMT_TYPE_CAPTURE | VPE_FMT_TYPE_OUTPUT, + .coplanar = 1, + .vpdma_fmt = { &vpdma_yuv_fmts[VPDMA_DATA_FMT_Y420], + &vpdma_yuv_fmts[VPDMA_DATA_FMT_CB420], + }, + }, { .fourcc = V4L2_PIX_FMT_YUYV, .types = VPE_FMT_TYPE_CAPTURE | VPE_FMT_TYPE_OUTPUT, @@ -686,7 +694,8 @@ static void set_cfg_modes(struct vpe_ctx *ctx) * Cfg Mode 1: YUV422 source, disable upsampler, DEI is de-interlacing. */ - if (fmt->fourcc == V4L2_PIX_FMT_NV12) + if (fmt->fourcc == V4L2_PIX_FMT_NV12 || + fmt->fourcc == V4L2_PIX_FMT_NV21) cfg_mode = 0; write_field(us1_reg0, cfg_mode, VPE_US_MODE_MASK, VPE_US_MODE_SHIFT); @@ -701,7 +710,8 @@ static void set_line_modes(struct vpe_ctx *ctx) struct vpe_fmt *fmt = ctx->q_data[Q_DATA_SRC].fmt; int line_mode = 1; - if (fmt->fourcc == V4L2_
[Patch 13/16] media: ti-vpe: vpdma: Use fixed type for address in descriptor
Using dma_addr_t as the type to hold address inside of a fix sized descriptor used by the vpdma firmware is prone to fail when the expected width is 32 bits and suddenly when CONFIG_LPAE is enabled the data size is now 64 bits shifted the remaining members of the descriptor in memory which confuses the firmware. Signed-off-by: Benoit Parrot --- drivers/media/platform/ti-vpe/vpdma_priv.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpdma_priv.h b/drivers/media/platform/ti-vpe/vpdma_priv.h index d8ae3e1cd54d..0bbee45338bd 100644 --- a/drivers/media/platform/ti-vpe/vpdma_priv.h +++ b/drivers/media/platform/ti-vpe/vpdma_priv.h @@ -166,11 +166,11 @@ struct vpdma_dtd { u32 xfer_length_height; u32 w1; }; - dma_addr_t start_addr; + u32 start_addr; u32 pkt_ctl; union { u32 frame_width_height; /* inbound */ - dma_addr_t desc_write_addr;/* outbound */ + u32 desc_write_addr;/* outbound */ }; union { u32 start_h_v; /* inbound */ -- 2.17.1
[Patch 10/16] media: ti-vpe: vpe: fix a v4l2-compliance failure about invalid sizeimage
v4l2-compliance fails with this message: fail: v4l2-test-formats.cpp(463): !pfmt.sizeimage fail: v4l2-test-formats.cpp(736): \ Video Capture Multiplanar is valid, \ but TRY_FMT failed to return a format test VIDIOC_TRY_FMT: FAIL This failure is causd by the driver failing to handle out range 'bytesperline' values from user space applications. VPDMA hardware is limited to 64k line stride (16 bytes aligned, so 65520 bytes). So make sure the provided or calculated 'bytesperline' is smaller than the maximum value. Signed-off-by: Benoit Parrot Reviewed-by: Tomi Valkeinen --- drivers/media/platform/ti-vpe/vpdma.h | 1 + drivers/media/platform/ti-vpe/vpe.c | 4 2 files changed, 5 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpdma.h b/drivers/media/platform/ti-vpe/vpdma.h index bce17329c4c9..393fcbb3cb40 100644 --- a/drivers/media/platform/ti-vpe/vpdma.h +++ b/drivers/media/platform/ti-vpe/vpdma.h @@ -57,6 +57,7 @@ struct vpdma_data_format { * line stride of source and dest * buffers should be 16 byte aligned */ +#define VPDMA_MAX_STRIDE 65520 /* Max line stride 16 byte aligned */ #define VPDMA_DTD_DESC_SIZE32 /* 8 words */ #define VPDMA_CFD_CTD_DESC_SIZE16 /* 4 words */ diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 7aa83026fb6c..0a7cf9c820c6 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1694,6 +1694,10 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f, if (stride > plane_fmt->bytesperline) plane_fmt->bytesperline = stride; + plane_fmt->bytesperline = clamp_t(u32, plane_fmt->bytesperline, + stride, + VPDMA_MAX_STRIDE); + plane_fmt->bytesperline = ALIGN(plane_fmt->bytesperline, VPDMA_STRIDE_ALIGN); -- 2.17.1
Re: [GIT PULL] Thermal management updates for v5.4-rc1
On Fri, Sep 27, 2019 at 6:08 AM Zhang Rui wrote: > > One thing to mention is that, all the patches have been tested in > linux-next for weeks, but there is a conflict detected, because > upstream has took commit eaf7b46083a7e34 ("docs: thermal: add it to the > driver API") from jc-docs tree while I'm keeping a wrong version of the > patch, so I just rebased my tree to fix this. Why do I have to say this EVERY single release? A conflict is not a reason to rebase. Conflicts happen. They happen a lot. I deal with them, and it's usually trivial. If you feel it's not trivial, just describe what the resolution is, rather than rebasing. Really. Rebasing for a random conflict (particularly in documentation, for chrissake!) is like using an atomic bomb to swat a fly. You have all those downsides, and there are basically _no_ upsides. It only makes for more work for me because I have to re-write this email for the millionth time, and that takes longer and is more aggravating than the conflict would have taken to just sort out. Linus
[Patch 07/16] media: ti-vpe: vpe: fix a v4l2-compliance failure causing a kernel panic
v4l2-compliance fails with this message: warn: v4l2-test-formats.cpp(717): \ TRY_FMT cannot handle an invalid pixelformat. test VIDIOC_TRY_FMT: FAIL This causes the following kernel panic: Unable to handle kernel paging request at virtual address 56595561 pgd = ecd80e00 *pgd= Internal error: Oops: 205 [#1] PREEMPT SMP ARM ... CPU: 0 PID: 930 Comm: v4l2-compliance Not tainted \ 4.14.62-01715-gc8cd67f49a19 #1 Hardware name: Generic DRA72X (Flattened Device Tree) task: ece44d80 task.stack: ecc6e000 PC is at __vpe_try_fmt+0x18c/0x2a8 [ti_vpe] LR is at 0x8 Because the driver fails to properly check the 'num_planes' values for proper ranges it ends up accessing out of bound data causing the kernel panic. Since this driver only handle single or dual plane pixel format, make sure the provided value does not exceed 2 planes. Signed-off-by: Benoit Parrot Reviewed-by: Tomi Valkeinen --- drivers/media/platform/ti-vpe/vpe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index bbbf11174e16..1278d457f753 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1650,7 +1650,7 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f, &pix->height, MIN_H, MAX_H, H_ALIGN, S_ALIGN); - if (!pix->num_planes) + if (!pix->num_planes || pix->num_planes > 2) pix->num_planes = fmt->coplanar ? 2 : 1; else if (pix->num_planes > 1 && !fmt->coplanar) pix->num_planes = 1; -- 2.17.1
[Patch 11/16] media: ti-vpe: vpe: fix a v4l2-compliance failure about frame sequence number
v4l2-compliance fails with this message: fail: v4l2-test-buffers.cpp(294): \ (int)g_sequence() < seq.last_seq + 1 fail: v4l2-test-buffers.cpp(740): \ buf.check(m2m_q, last_m2m_seq) fail: v4l2-test-buffers.cpp(974): \ captureBufs(node, q, m2m_q, frame_count, true) test MMAP: FAIL The driver is failing to update the source frame sequence number in the vb2 buffer object. Only the destination frame sequence was being updated. This is only a reporting issue if the user space app actually cares about the frame sequence number. But it is fixed nonetheless. Signed-off-by: Benoit Parrot Reviewed-by: Tomi Valkeinen --- drivers/media/platform/ti-vpe/vpe.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 0a7cf9c820c6..8ab1c3241b74 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1440,6 +1440,7 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data) d_vb->timecode = s_vb->timecode; d_vb->sequence = ctx->sequence; + s_vb->sequence = ctx->sequence; d_q_data = &ctx->q_data[Q_DATA_DST]; if (d_q_data->flags & Q_IS_INTERLACED) { -- 2.17.1
[Patch 06/16] media: ti-vpe: Set MAX height supported to 2048 pixels
From: Ram Prasad VPE's max height supported MAX_H is set to 1184 which is the padded height from VC1 decoder output. In case of 90, 270 degree rotated video processing, input to VPE will be 1080x1920, 720x1280 etc and MAX_H needs to be set correct value. Setting MAX_H to 2048 as worst case height. Signed-off-by: Ram Prasad Signed-off-by: Benoit Parrot --- drivers/media/platform/ti-vpe/vpe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index f3ee9ff87927..bbbf11174e16 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -52,7 +52,7 @@ #define MIN_W 32 #define MIN_H 32 #define MAX_W 2048 -#define MAX_H 1184 +#define MAX_H 2048 /* required alignments */ #define S_ALIGN0 /* multiple of 1 */ -- 2.17.1
[Patch 09/16] media: ti-vpe: vpe: Make sure YUYV is set as default format
v4l2-compliance fails with this message: fail: v4l2-test-formats.cpp(672): \ Video Capture Multiplanar: TRY_FMT(G_FMT) != G_FMT fail: v4l2-test-formats.cpp(672): \ Video Output Multiplanar: TRY_FMT(G_FMT) != G_FMT ... test VIDIOC_TRY_FMT: FAIL Fixes: 94ed726e8e01 ("media: ti-vpe: Add support for NV21 format") The default pixel format was setup as pointing to a specific offset in the vpe_formats table assuming it was pointing to the V4L2_PIX_FMT_YUYV entry. This became false after the addition on the NV21 format (see above commid-id) So instead of hard-coding an offset which might change over time we need to use a lookup helper instead so we know the default will always be what we intended. Signed-off-by: Benoit Parrot Reviewed-by: Tomi Valkeinen --- drivers/media/platform/ti-vpe/vpe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index d3f1ae8b72fa..7aa83026fb6c 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -2321,7 +2321,7 @@ static int vpe_open(struct file *file) v4l2_ctrl_handler_setup(hdl); s_q_data = &ctx->q_data[Q_DATA_SRC]; - s_q_data->fmt = &vpe_formats[2]; + s_q_data->fmt = __find_format(V4L2_PIX_FMT_YUYV); s_q_data->width = 1920; s_q_data->height = 1080; s_q_data->nplanes = 1; -- 2.17.1
[Patch 00/16] media: vpe: maintenance
This a collection of backlog patches I have been carrying for the VPE driver. It adds supports for SEQ_BT as well as NV21. And fixes a number of issues both through v4l2-compliance and normal usage. == v4l2-compliance SHA: 5b168dc8473911227890526bad26553d9e8ff81b, 32 bits Compliance test for vpe device /dev/video0: Driver Info: Driver name : vpe Card type: vpe Bus info : platform:vpe Driver version : 5.3.0 Capabilities : 0x84204000 Video Memory-to-Memory Multiplanar Streaming Extended Pix Format Device Capabilities Device Caps : 0x04204000 Video Memory-to-Memory Multiplanar Streaming Extended Pix Format Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second /dev/video0 open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK test for unlimited opens: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK (Not Supported) Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 0 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 1 Private Controls: 1 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK (Not Supported) test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK test VIDIOC_TRY_FMT: OK test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK test Composing: OK test Scaling: OK Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls: warn: v4l2-test-buffers.cpp(683): VIDIOC_CREATE_BUFS not supported warn: v4l2-test-buffers.cpp(683): VIDIOC_CREATE_BUFS not supported test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK test Requests: OK (Not Supported) Test input 0: Streaming ioctls: test read/write: OK (Not Supported) test blocking wait: OK Video Capture Multiplanar: Captured 58 buffers test MMAP (no poll): OK Video Capture Multiplanar: Captured 58 buffers test MMAP (select): OK Video Capture Multiplanar: Captured 58 buffers test MMAP (epoll): OK test USERPTR (no poll): OK (Not Supported) test USERPTR (select): OK (Not Supported) test DMABUF: Cannot test, specify --expbuf-device Total for vpe device /dev/video0: 51, Succeeded: 51, Failed: 0, Warnings: 2 == Benoit Parrot (13): media: ti-vpe: vpe: Fix Motion Vector vpdma stride media: ti-vpe: vpe: Add missing null pointer checks media: ti-vpe: vpe: Remove unnecessary use of container_of media: ti-vpe: vpe: fix a v4l2-compliance failure causing a kernel panic media: ti-vpe: vpe: fix a v4l2-compliance warning about invalid pixel format media: ti-vpe: vpe: Make sure YUYV is set as default format media: ti-vpe: vpe: fix a v4l2-compliance failure about invalid sizeimage media: ti-vpe: vpe: fix a v4l2-compliance failure about frame sequence number media: ti-vpe: vpe: ensure buffers are cleaned up properly in abort cases media: ti-vpe: vpdma: Use fixed type for address in descriptor media: ti-vpe: Set the DMA mask and coherent mask media: ti-vpe: vpe: fix v4l2_compliance issue related to xfer_func media: ti-vpe: vpe: don't rely on colorspace member for conversion Nikhil Devshatwar (2): media: ti-v
Re: [GIT PULL REQUEST] watchdog - v5.4 Merge window
The pull request you sent on Fri, 27 Sep 2019 09:15:27 +0200: > git://www.linux-watchdog.org/linux-watchdog.git tags/linux-watchdog-5.4-rc1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/7bccb9f10c8f36ee791769b531ed4d28f6379aae Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [GIT PULL] NTB changes for v5.4
The pull request you sent on Thu, 26 Sep 2019 13:35:22 -0400: > git://github.com/jonmason/ntb tags/ntb-5.4 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/0cd81d77d0569f1dc1e39abeea93c6184e9b5b54 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [Patch v3 5/8] media: dt-bindings: ov2659: add powerdown/reset-gpios optional property
On Tue, 24 Sep 2019 11:44:11 -0500, Benoit Parrot wrote: > Add powerdown-gpios and reset-gpios to the list of optional properties > for the OV2659 camera sensor. > > Signed-off-by: Benoit Parrot > --- > Documentation/devicetree/bindings/media/i2c/ov2659.txt | 9 + > 1 file changed, 9 insertions(+) > Reviewed-by: Rob Herring
[PATCH v6 0/7] Add support of New Amlogic temperature sensor for G12 SoCs
This patchs series add support of New Amlogic temperature sensor and minimal thermal zone for SEI510 and ODROID-N2 boards. First implementation was doing on IIO[1] but after comments i move on thermal framework. Formulas and calibration values come from amlogic. Changes since v5: - fix patch 5 and 6 send twice Changes since v4: - Move thermal-zone in soc dtsi file - Remove critical trip point and add passive one - fix commit message - use devm_platform_ioremap_resource instead of platform_get_resource Changes since v3: - Add cooling map and trip point for hot type - move compatible on g12a instead of g12 to be aligned with others - add all reviewer, sorry for this mistake Changes since v2: - fix yaml documention - remove unneeded status variable for temperature-sensor node - rework driver after Martin review - add some information in commit message Changes since v1: - fix enum vs const in documentation - fix error with thermal-sensor-cells value set to 1 instead of 0 - add some dependencies needed to add cooling-maps Dependencies : - patch 3,4 & 5: depends on Neil's patch and series : - missing dwc2 phy-names[2] - patchsets to add DVFS on G12a[3] which have deps on [4] and [5] [1] https://lore.kernel.org/linux-amlogic/20190604144714.2009-1-glaro...@baylibre.com/ [2] https://lore.kernel.org/linux-amlogic/20190625123647.26117-1-narmstr...@baylibre.com/ [3] https://lore.kernel.org/linux-amlogic/20190729132622.7566-1-narmstr...@baylibre.com/ [4] https://lore.kernel.org/linux-amlogic/20190731084019.8451-5-narmstr...@baylibre.com/ [5] https://lore.kernel.org/linux-amlogic/20190729132622.7566-3-narmstr...@baylibre.com/ Guillaume La Roque (7): dt-bindings: thermal: Add DT bindings documentation for Amlogic Thermal thermal: amlogic: Add thermal driver to support G12 SoCs arm64: dts: amlogic: g12: add temperature sensor arm64: dts: meson: g12: Add minimal thermal zone arm64: dts: amlogic: g12a: add cooling properties arm64: dts: amlogic: g12b: add cooling properties MAINTAINERS: add entry for Amlogic Thermal driver .../bindings/thermal/amlogic,thermal.yaml | 54 +++ MAINTAINERS | 9 + .../boot/dts/amlogic/meson-g12-common.dtsi| 66 arch/arm64/boot/dts/amlogic/meson-g12a.dtsi | 24 ++ arch/arm64/boot/dts/amlogic/meson-g12b.dtsi | 29 ++ drivers/thermal/Kconfig | 11 + drivers/thermal/Makefile | 1 + drivers/thermal/amlogic_thermal.c | 333 ++ 8 files changed, 527 insertions(+) create mode 100644 Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml create mode 100644 drivers/thermal/amlogic_thermal.c -- 2.17.1
[PATCH v6 5/7] arm64: dts: amlogic: g12a: add cooling properties
Add missing #colling-cells field for G12A SoC Add cooling-map for passive and hot trip point Tested-by: Christian Hewitt Tested-by: Kevin Hilman Signed-off-by: Guillaume La Roque --- arch/arm64/boot/dts/amlogic/meson-g12a.dtsi | 24 + 1 file changed, 24 insertions(+) diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi index 733a9d46fc4b..3ab6497548ca 100644 --- a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi +++ b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi @@ -18,6 +18,7 @@ reg = <0x0 0x0>; enable-method = "psci"; next-level-cache = <&l2>; + #cooling-cells = <2>; }; cpu1: cpu@1 { @@ -26,6 +27,7 @@ reg = <0x0 0x1>; enable-method = "psci"; next-level-cache = <&l2>; + #cooling-cells = <2>; }; cpu2: cpu@2 { @@ -34,6 +36,7 @@ reg = <0x0 0x2>; enable-method = "psci"; next-level-cache = <&l2>; + #cooling-cells = <2>; }; cpu3: cpu@3 { @@ -42,6 +45,7 @@ reg = <0x0 0x3>; enable-method = "psci"; next-level-cache = <&l2>; + #cooling-cells = <2>; }; l2: l2-cache0 { @@ -113,3 +117,23 @@ &sd_emmc_a { amlogic,dram-access-quirk; }; + +&cpu_thermal { + cooling-maps { + map0 { + trip = <&cpu_passive>; + cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, + <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, + <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, + <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; + }; + + map1 { + trip = <&cpu_hot>; + cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, + <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, + <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, + <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; + }; + }; +}; -- 2.17.1
[PATCH v6 4/7] arm64: dts: meson: g12: Add minimal thermal zone
Add minimal thermal zone for two temperature sensor One is located close to the DDR and the other one is located close to the PLLs (between the CPU and GPU) Acked-by: Martin Blumenstingl Tested-by: Christian Hewitt Tested-by: Kevin Hilman Signed-off-by: Guillaume La Roque --- .../boot/dts/amlogic/meson-g12-common.dtsi| 46 +++ 1 file changed, 46 insertions(+) diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi index 0660d9ef6a86..f98171949fcb 100644 --- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi +++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi @@ -12,6 +12,7 @@ #include #include #include +#include / { interrupt-parent = <&gic>; @@ -94,6 +95,50 @@ #size-cells = <2>; ranges; + thermal-zones { + cpu_thermal: cpu-thermal { + polling-delay = <1000>; + polling-delay-passive = <100>; + thermal-sensors = <&cpu_temp>; + + trips { + cpu_passive: cpu-passive { + temperature = <85000>; /* millicelsius */ + hysteresis = <2000>; /* millicelsius */ + type = "passive"; + }; + + cpu_hot: cpu-hot { + temperature = <95000>; /* millicelsius */ + hysteresis = <2000>; /* millicelsius */ + type = "hot"; + }; + + }; + }; + + ddr_thermal: ddr-thermal { + polling-delay = <1000>; + polling-delay-passive = <100>; + thermal-sensors = <&ddr_temp>; + + trips { + ddr_passive: ddr-passive { + temperature = <85000>; /* millicelsius */ + hysteresis = <2000>; /* millicelsius */ + type = "passive"; + }; + }; + + cooling-maps { + map { + trip = <&ddr_passive>; + cooling-device = <&mali THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; + }; + }; + }; + }; + ethmac: ethernet@ff3f { compatible = "amlogic,meson-axg-dwmac", "snps,dwmac-3.70a", @@ -2412,6 +2457,7 @@ assigned-clock-rates = <0>, /* Do Nothing */ <8>, <0>; /* Do Nothing */ + #cooling-cells = <2>; }; }; -- 2.17.1
[PATCH v6 7/7] MAINTAINERS: add entry for Amlogic Thermal driver
Add myself as maintainer for Amlogic Thermal driver. Reviewed-by: Neil Armstrong Signed-off-by: Guillaume La Roque --- MAINTAINERS | 9 + 1 file changed, 9 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 390c3194ee93..bdc30d740342 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -15932,6 +15932,15 @@ F: Documentation/driver-api/thermal/cpu-cooling-api.rst F: drivers/thermal/cpu_cooling.c F: include/linux/cpu_cooling.h +THERMAL DRIVER FOR AMLOGIC SOCS +M: Guillaume La Roque +L: linux...@vger.kernel.org +L: linux-amlo...@lists.infradead.org +W: http://linux-meson.com/ +S: Supported +F: drivers/thermal/amlogic_thermal.c +F: Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml + THINKPAD ACPI EXTRAS DRIVER M: Henrique de Moraes Holschuh L: ibm-acpi-de...@lists.sourceforge.net -- 2.17.1
Re: [PATCH] tools: bpf: Use !building_out_of_srctree to determine srctree
On Thu, Sep 26, 2019 at 6:14 PM Shuah Khan wrote: > > make TARGETS=bpf kselftest fails with: > > Makefile:127: tools/build/Makefile.include: No such file or directory > > When the bpf tool make is invoked from tools Makefile, srctree is > cleared and the current logic check for srctree equals to empty > string to determine srctree location from CURDIR. > > When the build in invoked from selftests/bpf Makefile, the srctree > is set to "." and the same logic used for srctree equals to empty is > needed to determine srctree. > > Check building_out_of_srctree undefined as the condition for both > cases to fix "make TARGETS=bpf kselftest" build failure. > > Signed-off-by: Shuah Khan The fix looks reasonable. Thanks! However, I am still seeing some failure: make TARGETS=bpf kselftest [...] test_verifier.c /data/users/songliubraving/kernel/linux-git/tools/testing/selftests/bpf/test_stub.o /data/users/songliubraving/kernel/linux-git/tools/testing/selftests/bpf/libbpf.a -lcap -lelf -lrt -lpthread -o /data/users/songliubraving/kernel/linux-git/tools/testing/selftests/bpf/test_verifier make[3]: test_verifier.c: Command not found Is this just a problem with my setup? Thanks, Song
[PATCH v6 6/7] arm64: dts: amlogic: g12b: add cooling properties
Add missing #colling-cells field for G12B SoC Add cooling-map for passive and hot trip point Tested-by: Christian Hewitt Tested-by: Kevin Hilman Signed-off-by: Guillaume La Roque --- arch/arm64/boot/dts/amlogic/meson-g12b.dtsi | 29 + 1 file changed, 29 insertions(+) diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12b.dtsi index 98ae8a7c8b41..4bb89bce758f 100644 --- a/arch/arm64/boot/dts/amlogic/meson-g12b.dtsi +++ b/arch/arm64/boot/dts/amlogic/meson-g12b.dtsi @@ -49,6 +49,7 @@ reg = <0x0 0x0>; enable-method = "psci"; next-level-cache = <&l2>; + #cooling-cells = <2>; }; cpu1: cpu@1 { @@ -57,6 +58,7 @@ reg = <0x0 0x1>; enable-method = "psci"; next-level-cache = <&l2>; + #cooling-cells = <2>; }; cpu100: cpu@100 { @@ -65,6 +67,7 @@ reg = <0x0 0x100>; enable-method = "psci"; next-level-cache = <&l2>; + #cooling-cells = <2>; }; cpu101: cpu@101 { @@ -73,6 +76,7 @@ reg = <0x0 0x101>; enable-method = "psci"; next-level-cache = <&l2>; + #cooling-cells = <2>; }; cpu102: cpu@102 { @@ -81,6 +85,7 @@ reg = <0x0 0x102>; enable-method = "psci"; next-level-cache = <&l2>; + #cooling-cells = <2>; }; cpu103: cpu@103 { @@ -89,6 +94,7 @@ reg = <0x0 0x103>; enable-method = "psci"; next-level-cache = <&l2>; + #cooling-cells = <2>; }; l2: l2-cache0 { @@ -219,3 +225,26 @@ &sd_emmc_a { amlogic,dram-access-quirk; }; + +&cpu_thermal { + cooling-maps { + map0 { + trip = <&cpu_passive>; + cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, +<&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, +<&cpu100 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, +<&cpu101 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, +<&cpu102 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, +<&cpu103 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; + }; + map1 { + trip = <&cpu_hot>; + cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, +<&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, +<&cpu100 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, +<&cpu101 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, +<&cpu102 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, +<&cpu103 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; + }; + }; +}; -- 2.17.1
[PATCH v6 3/7] arm64: dts: amlogic: g12: add temperature sensor
Add cpu and ddr temperature sensors for G12 Socs Reviewed-by: Martin Blumenstingl Tested-by: Christian Hewitt Tested-by: Kevin Hilman Signed-off-by: Guillaume La Roque --- .../boot/dts/amlogic/meson-g12-common.dtsi| 20 +++ 1 file changed, 20 insertions(+) diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi index 38d70ce1cfc7..0660d9ef6a86 100644 --- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi +++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi @@ -1353,6 +1353,26 @@ }; }; + cpu_temp: temperature-sensor@34800 { + compatible = "amlogic,g12a-cpu-thermal", +"amlogic,g12a-thermal"; + reg = <0x0 0x34800 0x0 0x50>; + interrupts = ; + clocks = <&clkc CLKID_TS>; + #thermal-sensor-cells = <0>; + amlogic,ao-secure = <&sec_AO>; + }; + + ddr_temp: temperature-sensor@34c00 { + compatible = "amlogic,g12a-ddr-thermal", +"amlogic,g12a-thermal"; + reg = <0x0 0x34c00 0x0 0x50>; + interrupts = ; + clocks = <&clkc CLKID_TS>; + #thermal-sensor-cells = <0>; + amlogic,ao-secure = <&sec_AO>; + }; + usb2_phy0: phy@36000 { compatible = "amlogic,g12a-usb2-phy"; reg = <0x0 0x36000 0x0 0x2000>; -- 2.17.1
[PATCH v6 2/7] thermal: amlogic: Add thermal driver to support G12 SoCs
Amlogic G12A and G12B SoCs integrate two thermal sensors with the same design. One is located close to the DDR controller and the other one is located close to the PLLs (between the CPU and GPU). The calibration data for each of the thermal sensors instance is stored in a different location within the AO region. Implement reading the temperature from each thermal sensor. The IP block has more functionality, which may be added to this driver in the future: - chip reset when the temperature exceeds a configurable threshold - up to four interrupts when the temperature has risen above a configurable threshold - up to four interrupts when the temperature has fallen below a configurable threshold Tested-by: Christian Hewitt Tested-by: Kevin Hilman Signed-off-by: Guillaume La Roque --- drivers/thermal/Kconfig | 11 + drivers/thermal/Makefile | 1 + drivers/thermal/amlogic_thermal.c | 333 ++ 3 files changed, 345 insertions(+) create mode 100644 drivers/thermal/amlogic_thermal.c diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index 9966364a6deb..0f31bb4bc372 100644 --- a/drivers/thermal/Kconfig +++ b/drivers/thermal/Kconfig @@ -348,6 +348,17 @@ config MTK_THERMAL Enable this option if you want to have support for thermal management controller present in Mediatek SoCs +config AMLOGIC_THERMAL + tristate "Amlogic Thermal Support" + default ARCH_MESON + depends on OF && ARCH_MESON + help + If you say yes here you get support for Amlogic Thermal + for G12 SoC Family. + + This driver can also be built as a module. If so, the module will + be called amlogic_thermal. + menu "Intel thermal drivers" depends on X86 || X86_INTEL_QUARK || COMPILE_TEST source "drivers/thermal/intel/Kconfig" diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile index 74a37c7f847a..baeb70bf0568 100644 --- a/drivers/thermal/Makefile +++ b/drivers/thermal/Makefile @@ -54,3 +54,4 @@ obj-$(CONFIG_MTK_THERMAL) += mtk_thermal.o obj-$(CONFIG_GENERIC_ADC_THERMAL) += thermal-generic-adc.o obj-$(CONFIG_ZX2967_THERMAL) += zx2967_thermal.o obj-$(CONFIG_UNIPHIER_THERMAL) += uniphier_thermal.o +obj-$(CONFIG_AMLOGIC_THERMAL) += amlogic_thermal.o diff --git a/drivers/thermal/amlogic_thermal.c b/drivers/thermal/amlogic_thermal.c new file mode 100644 index ..8a9e9bc421c6 --- /dev/null +++ b/drivers/thermal/amlogic_thermal.c @@ -0,0 +1,333 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Amlogic Thermal Sensor Driver + * + * Copyright (C) 2017 Huan Biao + * Copyright (C) 2019 Guillaume La Roque + * + * Register value to celsius temperature formulas: + * Read_Valm * U + * U = -, Uptat = - + * 2^16 1 + n * U + * + * Temperature = A * ( Uptat + u_efuse / 2^16 )- B + * + * A B m n : calibration parameters + * u_efuse : fused calibration value, it's a signed 16 bits value + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "thermal_core.h" + +#define TSENSOR_CFG_REG1 0x4 + #define TSENSOR_CFG_REG1_RSET_VBG BIT(12) + #define TSENSOR_CFG_REG1_RSET_ADC BIT(11) + #define TSENSOR_CFG_REG1_VCM_EN BIT(10) + #define TSENSOR_CFG_REG1_VBG_EN BIT(9) + #define TSENSOR_CFG_REG1_OUT_CTLBIT(6) + #define TSENSOR_CFG_REG1_FILTER_EN BIT(5) + #define TSENSOR_CFG_REG1_DEM_EN BIT(3) + #define TSENSOR_CFG_REG1_CH_SEL GENMASK(1, 0) + #define TSENSOR_CFG_REG1_ENABLE \ + (TSENSOR_CFG_REG1_FILTER_EN | \ +TSENSOR_CFG_REG1_VCM_EN | \ +TSENSOR_CFG_REG1_VBG_EN | \ +TSENSOR_CFG_REG1_DEM_EN | \ +TSENSOR_CFG_REG1_CH_SEL) + +#define TSENSOR_STAT0 0x40 + +#define TSENSOR_STAT9 0x64 + +#define TSENSOR_READ_TEMP_MASK GENMASK(15, 0) +#define TSENSOR_TEMP_MASK GENMASK(11, 0) + +#define TSENSOR_TRIM_SIGN_MASK BIT(15) +#define TSENSOR_TRIM_TEMP_MASK GENMASK(14, 0) +#define TSENSOR_TRIM_VERSION_MASK GENMASK(31, 24) + +#define TSENSOR_TRIM_VERSION(_version) \ + FIELD_GET(TSENSOR_TRIM_VERSION_MASK, _version) + +#define TSENSOR_TRIM_CALIB_VALID_MASK (GENMASK(3, 2) | BIT(7)) + +#define TSENSOR_CALIB_OFFSET 1 +#define TSENSOR_CALIB_SHIFT4 + +/** + * struct amlogic_thermal_soc_calib_data + * @A, B, m, n: calibration parameters + * This structure is required for configuration of amlogic thermal driver. + */ +struct amlogic_thermal_soc_calib_data { + int A; + int B; + int m; + int n; +}; + +/** + * struct amlogic_thermal_data + * @u_efuse_off: register offset to read fused calibration value + * @calibration_parameters: calibration parameters st
[PATCH v6 1/7] dt-bindings: thermal: Add DT bindings documentation for Amlogic Thermal
Adding the devicetree binding documentation for the Amlogic temperature sensor found in the Amlogic Meson G12A and G12B SoCs. Reviewed-by: Rob Herring Tested-by: Christian Hewitt Tested-by: Kevin Hilman Signed-off-by: Guillaume La Roque --- .../bindings/thermal/amlogic,thermal.yaml | 54 +++ 1 file changed, 54 insertions(+) create mode 100644 Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml diff --git a/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml new file mode 100644 index ..f761681e4c0d --- /dev/null +++ b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml @@ -0,0 +1,54 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/thermal/amlogic,thermal.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Amlogic Thermal + +maintainers: + - Guillaume La Roque + +description: Binding for Amlogic Thermal + +properties: + compatible: + items: +- enum: +- amlogic,g12a-cpu-thermal +- amlogic,g12a-ddr-thermal +- const: amlogic,g12a-thermal + + reg: +maxItems: 1 + + interrupts: +maxItems: 1 + + clocks: +maxItems: 1 + + amlogic,ao-secure: +description: phandle to the ao-secure syscon +$ref: '/schemas/types.yaml#/definitions/phandle' + + +required: + - compatible + - reg + - interrupts + - clocks + - amlogic,ao-secure + +examples: + - | +cpu_temp: temperature-sensor@ff634800 { +compatible = "amlogic,g12a-cpu-thermal", + "amlogic,g12a-thermal"; +reg = <0xff634800 0x50>; +interrupts = <0x0 0x24 0x0>; +clocks = <&clk 164>; +#thermal-sensor-cells = <0>; +amlogic,ao-secure = <&sec_AO>; +}; +... -- 2.17.1
[Patch v4 7/8] media: i2c: ov2659: Fix missing 720p register config
The initial registers sequence is only loaded at probe time. Afterward only the resolution and format specific register are modified. Care must be taken to make sure registers modified by one resolution setting are reverted back when another resolution is programmed. This was not done properly for the 720p case. Signed-off-by: Benoit Parrot --- drivers/media/i2c/ov2659.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c index 7d0baa386644..720310e0725d 100644 --- a/drivers/media/i2c/ov2659.c +++ b/drivers/media/i2c/ov2659.c @@ -412,10 +412,14 @@ static struct sensor_register ov2659_720p[] = { { REG_TIMING_YINC, 0x11 }, { REG_TIMING_VERT_FORMAT, 0x80 }, { REG_TIMING_HORIZ_FORMAT, 0x00 }, + { 0x370a, 0x12 }, { 0x3a03, 0xe8 }, { 0x3a09, 0x6f }, { 0x3a0b, 0x5d }, { 0x3a15, 0x9a }, + { REG_VFIFO_READ_START_H, 0x00 }, + { REG_VFIFO_READ_START_L, 0x80 }, + { REG_ISP_CTRL02, 0x00 }, { REG_NULL, 0x00 }, }; -- 2.17.1
[Patch v4 3/8] media: i2c: ov2659: Cleanup include file list
Several of include files listed are not explicitly needed. If they are need then they are implicitly included. Reduce the list of includes to an easier to manage list. Signed-off-by: Benoit Parrot --- drivers/media/i2c/ov2659.c | 14 -- 1 file changed, 14 deletions(-) diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c index efbe6dc720e2..f77320e8a60d 100644 --- a/drivers/media/i2c/ov2659.c +++ b/drivers/media/i2c/ov2659.c @@ -22,29 +22,15 @@ #include #include -#include -#include -#include -#include #include -#include -#include #include -#include #include -#include -#include -#include -#include #include -#include #include -#include #include #include #include -#include #include #define DRIVER_NAME "ov2659" -- 2.17.1
[Patch v4 0/8] media: i2c: ov2659: maintenance series
This patch series is a collection of patches we have been carrying for a while. It includes a few sensor register fixes which would cause visual artifacts at lower resolution and also at 720p. Also on some board the 'powerdown' and /or 'reset' pins are not tied so we need to add support for optional gpios to handle these. Since these camera are removable on some board we also need the driver to actually fail when there is no hardware present so the driver is actually removed. Also added a patch to check and propagate errors on s_stream invocation. Finally, we update the licensing statement to use SPDX licensing. Changes since v3: - Addressed review comment from Sakari about s_ctrl i2c access whne powered off Changes since v2: - Addressed review comment from Sakari - Reworked the "media: i2c: ov2659: Add powerdown/reset gpio handling" to use pm_runtime if it is available but not to make it depend on it. - Cleaned up the gpio related calls to remove the unnecessary NULL check Changes since v1: - Addressed review comment from Prabhakar - Added support for reset-gpios - Rework the power setting to use pm_runtime instead of s_power as based on discussion with Sakari it would be the prefered method - Added a patch to reduce the number explicit include files to the minimum necessary instead of the previous kitchen sink approach Benoit Parrot (8): media: i2c: ov2659: Fix for image wrap-around in lower resolution media: i2c: ov2659: Fix sensor detection to actually fail when device is not present media: i2c: ov2659: Cleanup include file list media: i2c: ov2659: fix s_stream return value media: dt-bindings: ov2659: add powerdown/reset-gpios optional property media: i2c: ov2659: Add powerdown/reset gpio handling media: i2c: ov2659: Fix missing 720p register config media: i2c: ov2659: Switch to SPDX Licensing .../devicetree/bindings/media/i2c/ov2659.txt | 9 ++ drivers/media/i2c/Kconfig | 2 +- drivers/media/i2c/ov2659.c| 138 +- 3 files changed, 112 insertions(+), 37 deletions(-) -- 2.17.1
[Patch v4 5/8] media: dt-bindings: ov2659: add powerdown/reset-gpios optional property
Add powerdown-gpios and reset-gpios to the list of optional properties for the OV2659 camera sensor. Signed-off-by: Benoit Parrot --- Documentation/devicetree/bindings/media/i2c/ov2659.txt | 9 + 1 file changed, 9 insertions(+) diff --git a/Documentation/devicetree/bindings/media/i2c/ov2659.txt b/Documentation/devicetree/bindings/media/i2c/ov2659.txt index cabc7d827dfb..92989a619f29 100644 --- a/Documentation/devicetree/bindings/media/i2c/ov2659.txt +++ b/Documentation/devicetree/bindings/media/i2c/ov2659.txt @@ -12,6 +12,12 @@ Required Properties: - clock-names: should be "xvclk". - link-frequencies: target pixel clock frequency. +Optional Properties: +- powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any. + Active high with internal pull down resistor. +- reset-gpios: reference to the GPIO connected to the resetb pin, if any. + Active low with internal pull up resistor. + For further reading on port node refer to Documentation/devicetree/bindings/media/video-interfaces.txt. @@ -27,6 +33,9 @@ Example: clocks = <&clk_ov2659 0>; clock-names = "xvclk"; + powerdown-gpios = <&gpio6 14 GPIO_ACTIVE_HIGH>; + reset-gpios = <&gpio6 15 GPIO_ACTIVE_LOW>; + port { ov2659_0: endpoint { remote-endpoint = <&vpfe_ep>; -- 2.17.1
[Patch v4 4/8] media: i2c: ov2659: fix s_stream return value
In ov2659_s_stream() return value for invoked function should be checked and propagated. Signed-off-by: Benoit Parrot --- drivers/media/i2c/ov2659.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c index f77320e8a60d..cd4625432264 100644 --- a/drivers/media/i2c/ov2659.c +++ b/drivers/media/i2c/ov2659.c @@ -1187,10 +1187,13 @@ static int ov2659_s_stream(struct v4l2_subdev *sd, int on) goto unlock; } - ov2659_set_pixel_clock(ov2659); - ov2659_set_frame_size(ov2659); - ov2659_set_format(ov2659); - ov2659_set_streaming(ov2659, 1); + ret = ov2659_set_pixel_clock(ov2659); + if (!ret) + ret = ov2659_set_frame_size(ov2659); + if (!ret) + ret = ov2659_set_format(ov2659); + if (!ret) + ov2659_set_streaming(ov2659, 1); ov2659->streaming = on; unlock: -- 2.17.1
[Patch v4 1/8] media: i2c: ov2659: Fix for image wrap-around in lower resolution
Based on recently found sensor configuration examples, it was discovered that when scaling and binning are used for the lower resolutions (i.e. 640x480, 320x240) the read offset has to be increased otherwise the image appears to be wrapped around. Signed-off-by: Benoit Parrot Signed-off-by: Jyri Sarha --- drivers/media/i2c/ov2659.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c index f4ded0669ff9..17573257097d 100644 --- a/drivers/media/i2c/ov2659.c +++ b/drivers/media/i2c/ov2659.c @@ -661,7 +661,7 @@ static struct sensor_register ov2659_vga[] = { { REG_TIMING_HORIZ_FORMAT, 0x01 }, { 0x370a, 0x52 }, { REG_VFIFO_READ_START_H, 0x00 }, - { REG_VFIFO_READ_START_L, 0x80 }, + { REG_VFIFO_READ_START_L, 0xa0 }, { REG_ISP_CTRL02, 0x10 }, { REG_NULL, 0x00 }, }; @@ -709,7 +709,7 @@ static struct sensor_register ov2659_qvga[] = { { REG_TIMING_HORIZ_FORMAT, 0x01 }, { 0x370a, 0x52 }, { REG_VFIFO_READ_START_H, 0x00 }, - { REG_VFIFO_READ_START_L, 0x80 }, + { REG_VFIFO_READ_START_L, 0xa0 }, { REG_ISP_CTRL02, 0x10 }, { REG_NULL, 0x00 }, }; -- 2.17.1
[Patch v4 2/8] media: i2c: ov2659: Fix sensor detection to actually fail when device is not present
Make sure that if the expected sensor device id register is not recognized properly the failure is propagated up so devices are not left partially initialized. Signed-off-by: Benoit Parrot Signed-off-by: Jyri Sarha --- drivers/media/i2c/ov2659.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c index 17573257097d..efbe6dc720e2 100644 --- a/drivers/media/i2c/ov2659.c +++ b/drivers/media/i2c/ov2659.c @@ -1330,11 +1330,12 @@ static int ov2659_detect(struct v4l2_subdev *sd) unsigned short id; id = OV265X_ID(pid, ver); - if (id != OV2659_ID) + if (id != OV2659_ID) { dev_err(&client->dev, "Sensor detection failed (%04X, %d)\n", id, ret); - else { + ret = -ENODEV; + } else { dev_info(&client->dev, "Found OV%04X sensor\n", id); ret = ov2659_init(sd, 0); } -- 2.17.1
[Patch v4 8/8] media: i2c: ov2659: Switch to SPDX Licensing
Switch to SPDX licensing and drop the redundant GPL text. Signed-off-by: Benoit Parrot --- drivers/media/i2c/ov2659.c | 14 +- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c index 720310e0725d..e6da23d3a555 100644 --- a/drivers/media/i2c/ov2659.c +++ b/drivers/media/i2c/ov2659.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0-only /* * Omnivision OV2659 CMOS Image Sensor driver * @@ -5,19 +6,6 @@ * * Benoit Parrot * Lad, Prabhakar - * - * This program is free software; you may redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; version 2 of the License. - * - * 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 -- 2.17.1
[Patch v4 6/8] media: i2c: ov2659: Add powerdown/reset gpio handling
On some board it is possible that the sensor 'powerdown' and or 'reset' pin might be controlled by gpio instead of being tied. To implement we add pm_runtime support which will handle the power up/down sequence when it is available otherwise the sensor will be powered on at module insertion/probe and powered off at module removal. Now originally the driver assumed that the sensor would always stay powered and keep its register setting. We cannot assume this anymore, so every time we "power up" we need to re-program the initial registers configuration first. This was previously done only at probe time. Signed-off-by: Benoit Parrot --- drivers/media/i2c/Kconfig | 2 +- drivers/media/i2c/ov2659.c | 88 +- 2 files changed, 87 insertions(+), 3 deletions(-) diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 7eee1812bba3..315c1d8bdb7b 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -634,7 +634,7 @@ config VIDEO_OV2640 config VIDEO_OV2659 tristate "OmniVision OV2659 sensor support" depends on VIDEO_V4L2 && I2C - depends on MEDIA_CAMERA_SUPPORT + depends on MEDIA_CAMERA_SUPPORT && GPIOLIB select V4L2_FWNODE help This is a Video4Linux2 sensor driver for the OmniVision diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c index cd4625432264..7d0baa386644 100644 --- a/drivers/media/i2c/ov2659.c +++ b/drivers/media/i2c/ov2659.c @@ -22,9 +22,11 @@ #include #include +#include #include #include #include +#include #include #include @@ -218,6 +220,11 @@ struct ov2659 { struct sensor_register *format_ctrl_regs; struct ov2659_pll_ctrl pll; int streaming; + /* used to control the sensor PWDN pin */ + struct gpio_desc *pwdn_gpio; + /* used to control the sensor RESETB pin */ + struct gpio_desc *resetb_gpio; + int on; }; static const struct sensor_register ov2659_init_regs[] = { @@ -1184,10 +1191,19 @@ static int ov2659_s_stream(struct v4l2_subdev *sd, int on) /* Stop Streaming Sequence */ ov2659_set_streaming(ov2659, 0); ov2659->streaming = on; + pm_runtime_put(&client->dev); goto unlock; } - ret = ov2659_set_pixel_clock(ov2659); + ret = pm_runtime_get_sync(&client->dev); + if (ret < 0) { + pm_runtime_put_noidle(&client->dev); + goto unlock; + } + + ret = ov2659_init(sd, 0); + if (!ret) + ret = ov2659_set_pixel_clock(ov2659); if (!ret) ret = ov2659_set_frame_size(ov2659); if (!ret) @@ -1229,6 +1245,9 @@ static int ov2659_s_ctrl(struct v4l2_ctrl *ctrl) struct ov2659 *ov2659 = container_of(ctrl->handler, struct ov2659, ctrls); + if (!ov2659->on) + return 0; + switch (ctrl->id) { case V4L2_CID_TEST_PATTERN: return ov2659_set_test_pattern(ov2659, ctrl->val); @@ -1246,6 +1265,43 @@ static const char * const ov2659_test_pattern_menu[] = { "Vertical Color Bars", }; +static int ov2659_power_off(struct device *dev) +{ + struct i2c_client *client = to_i2c_client(dev); + struct v4l2_subdev *sd = i2c_get_clientdata(client); + struct ov2659 *ov2659 = to_ov2659(sd); + + dev_dbg(&client->dev, "%s:\n", __func__); + + gpiod_set_value(ov2659->pwdn_gpio, 1); + + ov2659->on = false; + + return 0; +} + +static int ov2659_power_on(struct device *dev) +{ + struct i2c_client *client = to_i2c_client(dev); + struct v4l2_subdev *sd = i2c_get_clientdata(client); + struct ov2659 *ov2659 = to_ov2659(sd); + + dev_dbg(&client->dev, "%s:\n", __func__); + + gpiod_set_value(ov2659->pwdn_gpio, 0); + + if (ov2659->resetb_gpio) { + gpiod_set_value(ov2659->resetb_gpio, 1); + usleep_range(500, 1000); + gpiod_set_value(ov2659->resetb_gpio, 0); + usleep_range(3000, 5000); + } + + ov2659->on = true; + + return 0; +} + /* - * V4L2 subdev internal operations */ @@ -1326,7 +1382,6 @@ static int ov2659_detect(struct v4l2_subdev *sd) ret = -ENODEV; } else { dev_info(&client->dev, "Found OV%04X sensor\n", id); - ret = ov2659_init(sd, 0); } } @@ -1403,6 +1458,18 @@ static int ov2659_probe(struct i2c_client *client) ov2659->xvclk_frequency > 2700) return -EINVAL; + /* Optional gpio don't fail if not present */ + ov2659->pwdn_gpio = devm_gpiod_get_optional(&client->dev, "powerdown", + GPIOD_OUT_LOW); + if (IS_ERR(
Re: [PATCH 1/2] clk: sprd: Use IS_ERR() to validate the return value of syscon_regmap_lookup_by_phandle()
Quoting Baolin Wang (2019-09-26 20:50:53) > The syscon_regmap_lookup_by_phandle() will never return NULL, thus use > IS_ERR() to validate the return value instead of IS_ERR_OR_NULL(). > > Signed-off-by: Baolin Wang > --- > drivers/clk/sprd/common.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Fixes tag?
Re: [PATCH V9 5/5] mmc: host: sdhci-pci: Add Genesys Logic GL975x support
On Mon, 23 Sep 2019 at 23:39, Michael K. Johnson wrote: > > On Wed, Sep 18, 2019 at 02:07:51PM +0300, Adrian Hunter wrote: > > On 18/09/19 1:47 PM, Michael K. Johnson wrote: > > > I see that the first four patches made it into Linus's kernel > > > yesterday. Is there any chance of this final patch that actually > > > enables the hardware making it into another pull request still > > > intended for 5.4? Waiting on additional acked-by on Ben's work > > > addressing all the review comments? > ... > > > On Wed, Sep 11, 2019 at 03:23:44PM +0800, Ben Chuang wrote: > > >> From: Ben Chuang > > >> > > >> Add support for the GL9750 and GL9755 chipsets. > > >> > > >> Enable v4 mode and wait 5ms after set 1.8V signal enable for GL9750/ > > >> GL9755. Fix the value of SDHCI_MAX_CURRENT register and use the vendor > > >> tuning flow for GL9750. > > > > > > > It is OK by me: > > > > Acked-by: Adrian Hunter > > Ulf, > > Sorry to be a bother... Is anything remaining for this work to make > it into a second PR for 5.4 before the merge window closes? > > It would be really convenient for the microsd readers in > current-generation thinkpads (for instance) to have hardware support out > of the box without having to wait another kernel release cycle, if > there's nothing otherwise remaining to change. I confirmed that > it currently applies cleanly on top of Linus's kernel. I have applied this for fixes, so it will go in for 5.4, but perhaps I need to defer my PR to after rc1 as I am still on the road. Kind regards Uffe
[ANNOUNCE] 4.14.146-rt67
Hello RT Folks! I'm pleased to announce the 4.14.146-rt67 stable release. This release is just an update to the new stable 4.14.146 version and no RT specific changes have been made. You can get this release via the git tree at: git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git branch: v4.14-rt Head SHA1: e3bd8fc2e828293163262f52cab0cd4beeae7f4c Or to build 4.14.146-rt67 directly, the following patches should be applied: https://www.kernel.org/pub/linux/kernel/v4.x/linux-4.14.tar.xz https://www.kernel.org/pub/linux/kernel/v4.x/patch-4.14.146.xz https://www.kernel.org/pub/linux/kernel/projects/rt/4.14/patch-4.14.146-rt67.patch.xz Enjoy! Tom
[PATCH] drivers/clk: convert VL struct to struct_size
There are a few manually-calculated variable-length struct allocations left, this converts them to use struct_size. Signed-off-by: Stephen Kitt --- drivers/clk/at91/sckc.c | 3 +-- drivers/clk/imgtec/clk-boston.c | 3 +-- drivers/clk/ingenic/tcu.c | 3 +-- drivers/clk/mvebu/ap-cpu-clk.c | 4 ++-- drivers/clk/mvebu/cp110-system-controller.c | 4 ++-- drivers/clk/samsung/clk.c | 3 +-- drivers/clk/uniphier/clk-uniphier-core.c| 3 +-- 7 files changed, 9 insertions(+), 14 deletions(-) diff --git a/drivers/clk/at91/sckc.c b/drivers/clk/at91/sckc.c index 9bfe9a28294a..5ad6180449cb 100644 --- a/drivers/clk/at91/sckc.c +++ b/drivers/clk/at91/sckc.c @@ -478,8 +478,7 @@ static void __init of_sam9x60_sckc_setup(struct device_node *np) if (IS_ERR(slow_osc)) goto unregister_slow_rc; - clk_data = kzalloc(sizeof(*clk_data) + (2 * sizeof(struct clk_hw *)), - GFP_KERNEL); + clk_data = kzalloc(struct_size(clk_data, hws, 2), GFP_KERNEL); if (!clk_data) goto unregister_slow_osc; diff --git a/drivers/clk/imgtec/clk-boston.c b/drivers/clk/imgtec/clk-boston.c index 33ab4ff61165..b00cbd045af5 100644 --- a/drivers/clk/imgtec/clk-boston.c +++ b/drivers/clk/imgtec/clk-boston.c @@ -58,8 +58,7 @@ static void __init clk_boston_setup(struct device_node *np) cpu_div = ext_field(mmcmdiv, BOSTON_PLAT_MMCMDIV_CLK1DIV); cpu_freq = mult_frac(in_freq, mul, cpu_div); - onecell = kzalloc(sizeof(*onecell) + - (BOSTON_CLK_COUNT * sizeof(struct clk_hw *)), + onecell = kzalloc(struct_size(onecell, hws, BOSTON_CLK_COUNT), GFP_KERNEL); if (!onecell) return; diff --git a/drivers/clk/ingenic/tcu.c b/drivers/clk/ingenic/tcu.c index a1a5f9cb439e..ad7daa494fd4 100644 --- a/drivers/clk/ingenic/tcu.c +++ b/drivers/clk/ingenic/tcu.c @@ -358,8 +358,7 @@ static int __init ingenic_tcu_probe(struct device_node *np) } } - tcu->clocks = kzalloc(sizeof(*tcu->clocks) + - sizeof(*tcu->clocks->hws) * TCU_CLK_COUNT, + tcu->clocks = kzalloc(struct_size(tcu->clocks, hws, TCU_CLK_COUNT), GFP_KERNEL); if (!tcu->clocks) { ret = -ENOMEM; diff --git a/drivers/clk/mvebu/ap-cpu-clk.c b/drivers/clk/mvebu/ap-cpu-clk.c index af5e5acad370..6b394302c76a 100644 --- a/drivers/clk/mvebu/ap-cpu-clk.c +++ b/drivers/clk/mvebu/ap-cpu-clk.c @@ -274,8 +274,8 @@ static int ap_cpu_clock_probe(struct platform_device *pdev) if (!ap_cpu_clk) return -ENOMEM; - ap_cpu_data = devm_kzalloc(dev, sizeof(*ap_cpu_data) + - sizeof(struct clk_hw *) * nclusters, + ap_cpu_data = devm_kzalloc(dev, struct_size(ap_cpu_data, hws, + nclusters), GFP_KERNEL); if (!ap_cpu_data) return -ENOMEM; diff --git a/drivers/clk/mvebu/cp110-system-controller.c b/drivers/clk/mvebu/cp110-system-controller.c index 808463276145..84c8900542e4 100644 --- a/drivers/clk/mvebu/cp110-system-controller.c +++ b/drivers/clk/mvebu/cp110-system-controller.c @@ -235,8 +235,8 @@ static int cp110_syscon_common_probe(struct platform_device *pdev, if (ret) return ret; - cp110_clk_data = devm_kzalloc(dev, sizeof(*cp110_clk_data) + - sizeof(struct clk_hw *) * CP110_CLK_NUM, + cp110_clk_data = devm_kzalloc(dev, struct_size(cp110_clk_data, hws, + CP110_CLK_NUM), GFP_KERNEL); if (!cp110_clk_data) return -ENOMEM; diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c index e544a38106dd..dad31308c071 100644 --- a/drivers/clk/samsung/clk.c +++ b/drivers/clk/samsung/clk.c @@ -60,8 +60,7 @@ struct samsung_clk_provider *__init samsung_clk_init(struct device_node *np, struct samsung_clk_provider *ctx; int i; - ctx = kzalloc(sizeof(struct samsung_clk_provider) + - sizeof(*ctx->clk_data.hws) * nr_clks, GFP_KERNEL); + ctx = kzalloc(struct_size(ctx, clk_data.hws, nr_clks), GFP_KERNEL); if (!ctx) panic("could not allocate clock provider context.\n"); diff --git a/drivers/clk/uniphier/clk-uniphier-core.c b/drivers/clk/uniphier/clk-uniphier-core.c index c6aaca73cf86..12380236d7ab 100644 --- a/drivers/clk/uniphier/clk-uniphier-core.c +++ b/drivers/clk/uniphier/clk-uniphier-core.c @@ -64,8 +64,7 @@ static int uniphier_clk_probe(struct platform_device *pdev) for (p = data; p->name; p++) clk_num = max(clk_num, p->idx + 1); - hw_data = devm_kzalloc(dev, - sizeof(*hw_data) + cl
Re: [PATCH v3] nfp: abm: fix memory leak in nfp_abm_u32_knode_replace
From: Navid Emamdoost Date: Thu, 26 Sep 2019 20:51:46 -0500 > In nfp_abm_u32_knode_replace if the allocation for match fails it should > go to the error handling instead of returning. Updated other gotos to > have correct errno returned, too. > > Signed-off-by: Navid Emamdoost Applied.
Re: [PATCH] net: tap: clean up an indentation issue
From: Colin King Date: Fri, 27 Sep 2019 10:40:39 +0100 > From: Colin Ian King > > There is a statement that is indented too deeply, remove > the extraneous tab. > > Signed-off-by: Colin Ian King Applied.
Re: [PATCH v2] vsock/virtio: add support for MSG_PEEK
This is net-next material.
Re: [PATCH 1/2] Modify cpupower to schedule itself on cores it is reading MSRs from
On 9/18/19 10:34 AM, Natarajan, Janakarajan wrote: Modify cpupower to schedule itself on each of the cpus in the system and then get the APERF/MPERF register values. This is advantageous because an IPI is not generated when a read_msr() is executed on the local logical CPU thereby reducing the chance of having APERF and MPERF being out of sync. Somehow this doesn't read right. Is this that you are trying to avoid APERF and MPERF being out of sync with this change? This description is rather confusing. Signed-off-by: Janakarajan Natarajan --- .../utils/idle_monitor/mperf_monitor.c| 38 ++- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c b/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c index 44806a6dae11..8b072e39c897 100644 --- a/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c +++ b/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c @@ -10,6 +10,7 @@ #include #include #include +#include #include @@ -86,15 +87,33 @@ static int mperf_get_tsc(unsigned long long *tsc) return ret; } +static int get_aperf_mperf(int cpu, unsigned long long *aval, + unsigned long long *mval) +{ + cpu_set_t set; + int ret; + + CPU_ZERO(&set); + CPU_SET(cpu, &set); + if (sched_setaffinity(getpid(), sizeof(set), &set) == -1) { + dprint("Could not migrate to cpu: %d\n", cpu); + return 1; + } + + ret = read_msr(cpu, MSR_APERF, aval); + ret |= read_msr(cpu, MSR_MPERF, mval); + + return ret; +} + static int mperf_init_stats(unsigned int cpu) { - unsigned long long val; + unsigned long long aval, mval; int ret; - ret = read_msr(cpu, MSR_APERF, &val); - aperf_previous_count[cpu] = val; - ret |= read_msr(cpu, MSR_MPERF, &val); - mperf_previous_count[cpu] = val; + ret = get_aperf_mperf(cpu, &aval, &mval); get_aperf_mperf() could return error right? It returns 1 when sched_setaffinity() fails. Shouldn't the return value checked, instead of using aval and mval? + aperf_previous_count[cpu] = aval; + mperf_previous_count[cpu] = mval; is_valid[cpu] = !ret; return 0; @@ -102,13 +121,12 @@ static int mperf_init_stats(unsigned int cpu) static int mperf_measure_stats(unsigned int cpu) { - unsigned long long val; + unsigned long long aval, mval; int ret; - ret = read_msr(cpu, MSR_APERF, &val); - aperf_current_count[cpu] = val; - ret |= read_msr(cpu, MSR_MPERF, &val); - mperf_current_count[cpu] = val; + ret = get_aperf_mperf(cpu, &aval, &mval); Same comments as above here. + aperf_current_count[cpu] = aval; + mperf_current_count[cpu] = mval; is_valid[cpu] = !ret; return 0; thanks, -- Shuah
RE: [Intel-wired-lan] [PATCH] i40e: prevent memory leak in i40e_setup_macvlans
> -Original Message- > From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On > Behalf Of Navid Emamdoost > Sent: Wednesday, September 25, 2019 8:49 AM > Cc: net...@vger.kernel.org; k...@umn.edu; linux-kernel@vger.kernel.org; > emamd...@umn.edu; intel-wired-...@lists.osuosl.org; > smcca...@umn.edu; David S. Miller ; Navid > Emamdoost > Subject: [Intel-wired-lan] [PATCH] i40e: prevent memory leak in > i40e_setup_macvlans > > In i40e_setup_macvlans if i40e_setup_channel fails the allocated memory for > ch should be released. > > Signed-off-by: Navid Emamdoost > --- > drivers/net/ethernet/intel/i40e/i40e_main.c | 1 + > 1 file changed, 1 insertion(+) Tested-by: Andrew Bowers
Re: [PATCH] tools: bpf: Use !building_out_of_srctree to determine srctree
On 9/27/19 12:44 PM, Song Liu wrote: On Thu, Sep 26, 2019 at 6:14 PM Shuah Khan wrote: make TARGETS=bpf kselftest fails with: Makefile:127: tools/build/Makefile.include: No such file or directory When the bpf tool make is invoked from tools Makefile, srctree is cleared and the current logic check for srctree equals to empty string to determine srctree location from CURDIR. When the build in invoked from selftests/bpf Makefile, the srctree is set to "." and the same logic used for srctree equals to empty is needed to determine srctree. Check building_out_of_srctree undefined as the condition for both cases to fix "make TARGETS=bpf kselftest" build failure. Signed-off-by: Shuah Khan The fix looks reasonable. Thanks! However, I am still seeing some failure: make TARGETS=bpf kselftest [...] test_verifier.c /data/users/songliubraving/kernel/linux-git/tools/testing/selftests/bpf/test_stub.o /data/users/songliubraving/kernel/linux-git/tools/testing/selftests/bpf/libbpf.a -lcap -lelf -lrt -lpthread -o /data/users/songliubraving/kernel/linux-git/tools/testing/selftests/bpf/test_verifier make[3]: test_verifier.c: Command not found Is this just a problem with my setup? You are running into the second bpf failure because of the dependency on the latest llvm. This is known issue with bpf test and it doesn't compile on 5.4 and maybe even 5.3 You have upgrade to the bleeding edge llvm. thanks, -- Shuah
Re: [PATCH] media: i2c: adv7180: fix adv7280 BT.656-4 compatibility
Hi Tim, Sorry for taking to so long to look at this. On 2019-09-23 15:04:47 -0700, Tim Harvey wrote: > On Thu, Aug 29, 2019 at 7:29 AM Niklas Söderlund > wrote: > > > > Hi, > > > > On 2019-08-29 13:43:49 +0200, Hans Verkuil wrote: > > > Adding Niklas. > > > > > > Niklas, can you take a look at this? > > > > I'm happy to have a look at this. I'm currently moving so all my boards > > are in a box somewhere. I hope to have my lab up and running next week, > > so if this is not urgent I will look at it then. > > > > Niklas, > > Have you looked at this yet? Without this patch the ADV7280A does not > output proper BT.656. We tested this on a Gateworks Ventana GW5404-G > which uses the ADV7280A connected to the IMX6 CSI parallel bus. I'm > hoping to see this get merged and perhaps backported to older kernels. I only have access to an adv7180 so I was unable to test this patch. After reviewing the documentation I think the patch is OK if what you want is to unconditionally switch the driver from outputting BT.656-3 to outputting BT.656-4. As this change would effect a large number of compat strings (adv7280, adv7280-m, adv7281, adv7281-m, adv7281-ma, adv7282, adv7282-m) and the goal is to back port it I'm a bit reluctant to adding my tag to this patch as I'm not sure if this will break other setups. >From the documentation about the BT.656-4 register (address 0x04 bit 7): Between Revision 3 and Revision 4 of the ITU-R BT.656 standards, the ITU has changed the toggling position for the V bit within the SAV EAV codes for NTSC. The ITU-R BT.656-4 standard bit allows the user to select an output mode that is compliant with either the previous or new standard. For further information, visit the International Telecommunication Union website. Note that the standard change only affects NTSC and has no bearing on PAL. When ITU-R BT.656-4 is 0 (default), the ITU-R BT.656-3 specification is used. The V bit goes low at EAV of Line 10 and Line 273. When ITU-R BT.656-4 is 1, the ITU-R BT.656-4 specification is used. The V bit goes low at EAV of Line 20 and Line 283. Do you know what effects such a change would bring? Looking at the driver BT.656-4 seems to be set unconditionally for some adv7180 chips. > > Regards, > > Tim > > > > > > > Regards, > > > > > > Hans > > > > > > On 8/27/19 11:55 PM, Matthew Michilot wrote: > > > > From: Matthew Michilot > > > > > > > > Captured video would be out of sync when using the adv7280 with > > > > the BT.656-4 protocol. Certain registers (0x04, 0x31, 0xE6) had to > > > > be configured properly to ensure BT.656-4 compatibility. > > > > > > > > An error in the adv7280 reference manual suggested that EAV/SAV mode > > > > was enabled by default, however upon inspecting register 0x31, it was > > > > determined to be disabled by default. The manual I have [1] states that NEWAVMODE is switched off by default. I'm only asking as I would like to know if there is an error in that datasheet or not. 1. https://www.analog.com/media/en/technical-documentation/user-guides/ADV7280_7281_7282_7283_UG-637.pdf > > > > > > > > Signed-off-by: Matthew Michilot > > > > Reviewed-by: Tim Harvey > > > > --- > > > > drivers/media/i2c/adv7180.c | 15 +-- > > > > 1 file changed, 13 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c > > > > index 99697baad2ea..27da424dce76 100644 > > > > --- a/drivers/media/i2c/adv7180.c > > > > +++ b/drivers/media/i2c/adv7180.c > > > > @@ -94,6 +94,7 @@ > > > > #define ADV7180_REG_SHAP_FILTER_CTL_1 0x0017 > > > > #define ADV7180_REG_CTRL_2 0x001d > > > > #define ADV7180_REG_VSYNC_FIELD_CTL_1 0x0031 > > > > +#define ADV7180_VSYNC_FIELD_CTL_1_NEWAV 0x12 > > > > #define ADV7180_REG_MANUAL_WIN_CTL_1 0x003d > > > > #define ADV7180_REG_MANUAL_WIN_CTL_2 0x003e > > > > #define ADV7180_REG_MANUAL_WIN_CTL_3 0x003f > > > > @@ -935,10 +936,20 @@ static int adv7182_init(struct adv7180_state > > > > *state) > > > > adv7180_write(state, ADV7180_REG_EXTENDED_OUTPUT_CONTROL, > > > > 0x57); > > > > adv7180_write(state, ADV7180_REG_CTRL_2, 0xc0); > > > > } else { > > > > - if (state->chip_info->flags & ADV7180_FLAG_V2) > > > > + if (state->chip_info->flags & ADV7180_FLAG_V2) { > > > > + /* ITU-R BT.656-4 compatible */ > > > > adv7180_write(state, > > > > ADV7180_REG_EXTENDED_OUTPUT_CONTROL, > > > > - 0x17); > > > > + > > > > ADV7180_EXTENDED_OUTPUT_CONTROL_NTSCDIS); > > > > + /* Manually set NEWAVMODE */ > > > > + adv7180_write(state, > > > > + ADV7180_REG_VSYNC_FIELD_CTL_1, > > > > + ADV7180_VSYNC_FIELD_CTL_1_NEW
Re: [PATCH 1/3] ARM: dts: add Netronix E60K02 board common file
Hi Marco, On Fri, 27 Sep 2019 11:47:21 +0200 Marco Felsch wrote: > Hi Andreas, > > thanks for the patch. > thanks for the quick review. Most of your comments are clear. [...] > > + wifi_pwrseq: wifi_pwrseq { > > + compatible = "mmc-pwrseq-simple"; > > + post-power-on-delay-ms = <20>; > > + reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>; > > Can you add a pinctrl-entry here please? The general rule is to mux > things where you use it. > yes, there are many places in my patch where they are added to some parent devices. I will fix that. [...] > > + leds { > > + compatible = "gpio-leds"; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_led>; > > Please move all muxing you made here into this file or add phandles so > the dts file need to add only the muxing stuff. This applies to all > pinctrl you made here. > so you disagree with this pattern: in .dtsi some_device { pinctrl-0 = <&pinctrl_some_device>; }; and in .dts (one I sent with this patch series and the tolino/mx6sl one is not ready-cooked yet, will be part of a later series) &iomuxc { pinctrl_some_device: some_devicegrp { fsl,pins = <...>; }; }; ? > > + > > + GLED { > > + gpios = <&gpio5 7 GPIO_ACTIVE_LOW>; > > + linux,default-trigger = "timer"; > > + }; > > + }; > > + > > + gpio-keys { > > + compatible = "gpio-keys"; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_gpio_keys>; > > + power { > > + label = "Power"; > > + gpios = <&gpio5 8 GPIO_ACTIVE_LOW>; > > + linux,code = ; > > Add missing header: dt-bindings/input/input.h to use this. > I am doing this in the .dts but it is probably better to do it here because it is used here. > > + gpio-key,wakeup; > > + }; > > + cover { > > + label = "Cover"; > > + gpios = <&gpio5 12 GPIO_ACTIVE_LOW>; > > + linux,code = ; > > + linux,input-type = <0x05>;/* EV_SW */ > > In the header above EV_SW is also specified so please use it here. > This pattern is in many files. I took one as an example. It seems that 50% of devicetree files have this pattern, the other 50% do have the pattern you proposed (which I like more). So probably EV_SW was not available in former times? > > + gpio-key,wakeup; > > + }; > > + }; > > + > > +}; > > + > > + > > + > > Whitespaces > > > +&audmux { > > + pinctrl-names = "default"; > > + status = "disabled"; > > Why you mentioned a pinctrl-names here without the mux? Do we need the > status line here? The common case is that such devices are off by > default/the base dt. > yes, that things can be removed. > > +}; > > + > > +&snvs_rtc { > > + status = "disabled"; > > Same applies here. > No, seems to be an exception, it does not have a status = "disabled" in imx6sll.dtsi. > > +}; > > + > > +&i2c1 { > > + clock-frequency = <10>; > > + pinctrl-names = "default","sleep"; > > + pinctrl-0 = <&pinctrl_i2c1 &pinctrl_lm3630a_bl_gpio>; > > The &pinctrl_lm3630a_bl_gpio should be moved into the lm3630a node. > > > + pinctrl-1 = <&pinctrl_i2c1_sleep>; > > + status = "okay"; > > + > > + lm3630a: lm3630a-i2c@36 { > > please name it backlight@36 > > > + reg = <0x36>; > > + status = "ok"; > > status lines are always be the last and if it is okay you can drop it > because the default is okay. > well, I added it because the driver was not loaded but later I found out that the real reason is that module aliases were broken and forgot to remove that "ok". Regards, Andreas
Re: [PATCH] mbox: qcom: avoid unused-variable warning
Hi Stephen, On Fri, Sep 27, 2019 at 8:26 PM Stephen Boyd wrote: > Quoting Geert Uytterhoeven (2019-09-26 06:07:13) > > On Fri, Sep 20, 2019 at 11:06 PM Stephen Boyd wrote: > > > Quoting Arnd Bergmann (2019-09-20 12:27:50) > > > > On Fri, Sep 20, 2019 at 6:45 PM Stephen Boyd wrote: > > > > > > @@ -54,11 +60,6 @@ static int qcom_apcs_ipc_probe(struct > > > > > > platform_device *pdev) > > > > > > void __iomem *base; > > > > > > unsigned long i; > > > > > > int ret; > > > > > > - const struct of_device_id apcs_clk_match_table[] = { > > > > > > > > > > Does marking it static here work too? It would be nice to limit the > > > > > scope of this variable to this function instead of making it a global. > > > > > Also, it might be slightly smaller code size if that works. > > > > > > > > No, I just tried and the warning returned. > > > > It's there for the convenience for the user, so he doesn't have to add it > > himself explicitly. > > > > > ("of/device: Nullify match table in of_match_device() for CONFIG_OF=n"), > > > but it's been 5 years! Surely we can revert this change now that commit > > > 219a7bc577e6 ("spi: rspi: Use of_device_get_match_data() helper") is > > > merged. > > > > Right, the particular error case in the RSPI driver can no longer happen. > > > > Calling of_device_get_match_data() is the better solution anyway, as > > that uses the match table stored in dev->driver->of_match_table, so > > there's no need to pass the table explicitly, and conditionally. > > > > Hence... > > > > > --- a/drivers/leds/leds-pca9532.c > > > +++ b/drivers/leds/leds-pca9532.c > > > @@ -472,7 +472,7 @@ pca9532_of_populate_pdata(struct device *dev, struct > > > device_node *np) > > > int i = 0; > > > const char *state; > > > > > > - match = of_match_device(of_pca9532_leds_match, dev); > > > + match = of_match_device(of_match_ptr(of_pca9532_leds_match), dev); > > > if (!match) > > > return ERR_PTR(-ENODEV); > > > > Please convert to of_device_get_match_data() instead of adding > > of_match_ptr() invocations... > > How is this workable? I left it as of_match_device() because the value > returned may be 0 for the enum and that looks the same as NULL. This function is used for the DT case only, so there will always be a match. Hence you can do devid = (int)(uintptr_t)of_device_get_match_data(dev). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH] tools/power/cpupower: Fix initializer override in hsw_ext_cstates
On 9/27/19 10:26 AM, Nathan Chancellor wrote: When building cpupower with clang, the following warning appears: utils/idle_monitor/hsw_ext_idle.c:42:16: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides] .desc = N_("Processor Package C2"), ^~ ./utils/helpers/helpers.h:25:33: note: expanded from macro 'N_' #define N_(String) gettext_noop(String) ^~ ./utils/helpers/helpers.h:23:30: note: expanded from macro 'gettext_noop' #define gettext_noop(String) String ^~ utils/idle_monitor/hsw_ext_idle.c:41:16: note: previous initialization is here .desc = N_("Processor Package C9"), ^~ ./utils/helpers/helpers.h:25:33: note: expanded from macro 'N_' #define N_(String) gettext_noop(String) ^~ ./utils/helpers/helpers.h:23:30: note: expanded from macro 'gettext_noop' #define gettext_noop(String) String ^~ 1 warning generated. This appears to be a copy and paste or merge mistake because the name and id fields both have PC9 in them, not PC2. Remove the second assignment to fix the warning. Fixes: 7ee767b69b68 ("cpupower: Add Haswell family 0x45 specific idle monitor to show PC8,9,10 states") Link: https://github.com/ClangBuiltLinux/linux/issues/718 Signed-off-by: Nathan Chancellor --- tools/power/cpupower/utils/idle_monitor/hsw_ext_idle.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/power/cpupower/utils/idle_monitor/hsw_ext_idle.c b/tools/power/cpupower/utils/idle_monitor/hsw_ext_idle.c index 7c7451d3f494..58dbdfd4fa13 100644 --- a/tools/power/cpupower/utils/idle_monitor/hsw_ext_idle.c +++ b/tools/power/cpupower/utils/idle_monitor/hsw_ext_idle.c @@ -39,7 +39,6 @@ static cstate_t hsw_ext_cstates[HSW_EXT_CSTATE_COUNT] = { { .name = "PC9", .desc = N_("Processor Package C9"), - .desc = N_("Processor Package C2"), .id = PC9, .range = RANGE_PACKAGE, .get_count_percent = hsw_ext_get_count_percent, Looks good to me. I will queue this up for 5.4-rc2 or rc3. thanks, -- Shuah
Re: [PATCH] media: i2c: adv7180: fix adv7280 BT.656-4 compatibility
Hi Tim, On Wed, Sep 25, 2019 at 9:22 PM Tim Harvey wrote: > Have you looked at this yet? Without this patch the ADV7280A does not > output proper BT.656. We tested this on a Gateworks Ventana GW5404-G > which uses the ADV7280A connected to the IMX6 CSI parallel bus. I'm > hoping to see this get merged and perhaps backported to older kernels. In order to be backported to older kernels, please add a Fixes tag. Thanks
Re: [GIT PULL] pwm: Changes for v5.4-rc1
The pull request you sent on Fri, 27 Sep 2019 18:31:04 +0200: > git://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm.git > tags/pwm/for-5.4-rc1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/e37e3bc7e265d05d00f14079767537699cf6bd46 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [PATCH] media: i2c: adv7180: fix adv7280 BT.656-4 compatibility
On Fri, Sep 27, 2019 at 12:04 PM Niklas Söderlund wrote: > > Hi Tim, > > Sorry for taking to so long to look at this. > > On 2019-09-23 15:04:47 -0700, Tim Harvey wrote: > > On Thu, Aug 29, 2019 at 7:29 AM Niklas Söderlund > > wrote: > > > > > > Hi, > > > > > > On 2019-08-29 13:43:49 +0200, Hans Verkuil wrote: > > > > Adding Niklas. > > > > > > > > Niklas, can you take a look at this? > > > > > > I'm happy to have a look at this. I'm currently moving so all my boards > > > are in a box somewhere. I hope to have my lab up and running next week, > > > so if this is not urgent I will look at it then. > > > > > > > Niklas, > > > > Have you looked at this yet? Without this patch the ADV7280A does not > > output proper BT.656. We tested this on a Gateworks Ventana GW5404-G > > which uses the ADV7280A connected to the IMX6 CSI parallel bus. I'm > > hoping to see this get merged and perhaps backported to older kernels. > > I only have access to an adv7180 so I was unable to test this patch. > After reviewing the documentation I think the patch is OK if what you > want is to unconditionally switch the driver from outputting BT.656-3 to > outputting BT.656-4. > > As this change would effect a large number of compat strings (adv7280, > adv7280-m, adv7281, adv7281-m, adv7281-ma, adv7282, adv7282-m) and the > goal is to back port it I'm a bit reluctant to adding my tag to this > patch as I'm not sure if this will break other setups. > > From the documentation about the BT.656-4 register (address 0x04 bit 7): > > Between Revision 3 and Revision 4 of the ITU-R BT.656 standards, > the ITU has changed the toggling position for the V bit within > the SAV EAV codes for NTSC. The ITU-R BT.656-4 standard > bit allows the user to select an output mode that is compliant > with either the previous or new standard. For further information, > visit the International Telecommunication Union website. > > Note that the standard change only affects NTSC and has no > bearing on PAL. > > When ITU-R BT.656-4 is 0 (default), the ITU-R BT.656-3 > specification is used. The V bit goes low at EAV of Line 10 > and Line 273. > > When ITU-R BT.656-4 is 1, the ITU-R BT.656-4 specification is > used. The V bit goes low at EAV of Line 20 and Line 283. > > Do you know what effects such a change would bring? Looking at the > driver BT.656-4 seems to be set unconditionally for some adv7180 chips. > Niklas, Quite simply, we have a board that has an ADV7180 attached to the parallel CSI of an IMX6 that worked fine with mainline drivers then when we revised this board to attach an ADV7280A in the same way capture failed to sync. Investigation showed that the NEWAVMODE differed between the two. So if the point of the driver is to configure the variants in the same way, this patch needs to be applied. I would maintain that the adv7180 comes up with NEWAVMODE enabled and in order to be compatible we must configure the adv7282 the same. The same argument can be made for setting the V bit end position in NTSC mode - its done for the adv7180 so for compatible output it should be done for the adv7282. > > > > Regards, > > > > Tim > > > > > > > > > > Regards, > > > > > > > > Hans > > > > > > > > On 8/27/19 11:55 PM, Matthew Michilot wrote: > > > > > From: Matthew Michilot > > > > > > > > > > Captured video would be out of sync when using the adv7280 with > > > > > the BT.656-4 protocol. Certain registers (0x04, 0x31, 0xE6) had to > > > > > be configured properly to ensure BT.656-4 compatibility. > > > > > > > > > > An error in the adv7280 reference manual suggested that EAV/SAV mode > > > > > was enabled by default, however upon inspecting register 0x31, it was > > > > > determined to be disabled by default. > > The manual I have [1] states that NEWAVMODE is switched off by default. > I'm only asking as I would like to know if there is an error in that > datasheet or not. > > 1. > https://www.analog.com/media/en/technical-documentation/user-guides/ADV7280_7281_7282_7283_UG-637.pdf > Table 99 in that document shows NEVAVMODE disabled on power-up (0x31=0x02) yet Page 77 shows it enabled at power-up. Looking at an actual device we find it is indeed disabled on powerup (0x31=0x02) so Table 99 is correct, and Page 77 is not. If you look at the ADV7180 datasheet (https://www.analog.com/media/en/technical-documentation/data-sheets/ADV7180.pdf) Table 105 shows NEWAVMODE enabled by default which is also reflected in the register details on Page 91 and is what you find on an actual device. Regards, Tim -- CONFIDENTIALITY NOTICE: This email constitutes an electronic communication within the meaning of the Electronic Communications Privacy Act, 18 U.S.C. 2510, and its disclosure is strictly limited to the named recipient(s) intended by the sender of this message. This email, and any attachments, may contain confidential and/or proprietary information. If you are not a named recipient, any copyin
Re: [GIT PULL] Thermal management updates for v5.4-rc1
On Fri, 2019-09-27 at 11:34 -0700, Linus Torvalds wrote: > On Fri, Sep 27, 2019 at 6:08 AM Zhang Rui wrote: > > One thing to mention is that, all the patches have been tested in > > linux-next for weeks, but there is a conflict detected, because > > upstream has took commit eaf7b46083a7e34 ("docs: thermal: add it to > > the > > driver API") from jc-docs tree while I'm keeping a wrong version of > > the > > patch, so I just rebased my tree to fix this. > > Why do I have to say this EVERY single release? Because there are literally thousands of developers working on kernel bits here and there, and you're swatting this particular fly one developer at a time. I might suggest that you need to speak with the git people and politely ask them to add a warning to the rebase command itself so that it prints out something like: If you are doing linux kernel development, and you are doing a rebase, please read Documentation/When_Not_To_Rebase.rst before rebasing your code and sending it to Linus. You've been warned. Acknowledge receipt of warning and proceed with rebase? (y/N) You would have free reign to put one of your more monumental yet funny rants in place in the documentation. You could also have a global git config to turn off the "Don't annoy Linus with rebases" warning. But only mention that global config at the end of the kernel documentation so you know people have read it before they turn the warning off. Maybe that would help. > A conflict is not a reason to rebase. Conflicts happen. They happen a > lot. I deal with them, and it's usually trivial. > > If you feel it's not trivial, just describe what the resolution is, > rather than rebasing. Really. > > Rebasing for a random conflict (particularly in documentation, for > chrissake!) is like using an atomic bomb to swat a fly. You have all > those downsides, and there are basically _no_ upsides. It only makes > for more work for me because I have to re-write this email for the > millionth time, and that takes longer and is more aggravating than the > conflict would have taken to just sort out. > >Linus -- Doug Ledford GPG KeyID: B826A3330E572FDD Fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD signature.asc Description: This is a digitally signed message part
Re: [PATCH] mm, page_alloc: drop pointless static qualifier in build_zonelists()
On Fri, Sep 27, 2019 at 9:14 AM Kaitao Cheng wrote: > > There is no need to make the 'node_order' variable static > since new value always be assigned before use it. > > Signed-off-by: Kaitao Cheng > Signed-off-by: Muchun Song > --- > mm/page_alloc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 3334a769eb91..c473c304d09f 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -5597,7 +5597,7 @@ static void build_thisnode_zonelists(pg_data_t *pgdat) > > static void build_zonelists(pg_data_t *pgdat) > { > - static int node_order[MAX_NUMNODES]; > + int node_order[MAX_NUMNODES]; This isn't pointless. This prevents 4KB stack allocation which might overflow.
[PATCH 1/2] dt-bindings: firmware: Add bindings for Versal firmware
ZynqMP firmware driver can be used for versal also. Add versal compatible string to zynqmp firmware driver doc. Signed-off-by: Jolly Shah --- .../bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt| 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt index a4fe136..18c3aea 100644 --- a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt +++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt @@ -11,7 +11,9 @@ power management service, FPGA service and other platform management services. Required properties: - - compatible: Must contain: "xlnx,zynqmp-firmware" + - compatible: Must contain any of below: + "xlnx,zynqmp-firmware" for Zynq Ultrascale+ MPSoC + "xlnx,versal-firmware" for Versal - method: The method of calling the PM-API firmware layer. Permitted values are: - "smc" : SMC #0, following the SMCCC @@ -21,6 +23,8 @@ Required properties: Example --- +Zynq Ultrascale+ MPSoC +-- firmware { zynqmp_firmware: zynqmp-firmware { compatible = "xlnx,zynqmp-firmware"; @@ -28,3 +32,13 @@ firmware { ... }; }; + +Versal +-- +firmware { + versal_firmware: versal-firmware { + compatible = "xlnx,versal-firmware"; + method = "smc"; + ... + }; +}; -- 2.7.4
[PATCH 2/2] drivers: firmware: xilinx: Add support for versal soc
Versal is xilinx's next generation soc. This patch adds driver support required to be compatible with versal device. Signed-off-by: Jolly Shah --- drivers/firmware/xilinx/zynqmp.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c index fd3d837..75bdfaa 100644 --- a/drivers/firmware/xilinx/zynqmp.c +++ b/drivers/firmware/xilinx/zynqmp.c @@ -711,8 +711,11 @@ static int zynqmp_firmware_probe(struct platform_device *pdev) int ret; np = of_find_compatible_node(NULL, NULL, "xlnx,zynqmp"); - if (!np) - return 0; + if (!np) { + np = of_find_compatible_node(NULL, NULL, "xlnx,versal"); + if (!np) + return 0; + } of_node_put(np); ret = get_set_conduit_method(dev->of_node); @@ -770,6 +773,7 @@ static int zynqmp_firmware_remove(struct platform_device *pdev) static const struct of_device_id zynqmp_firmware_of_match[] = { {.compatible = "xlnx,zynqmp-firmware"}, + {.compatible = "xlnx,versal-firmware"}, {}, }; MODULE_DEVICE_TABLE(of, zynqmp_firmware_of_match); -- 2.7.4
[PATCH 0/2] drivers: firmware: xilinx: Add support for versal soc
Versal is xilinx's next generation soc. This patch adds driver support required to be compatible with versal device. Jolly Shah (2): dt-bindings: firmware: Add bindings for Versal firmware drivers: firmware: xilinx: Add support for versal soc .../bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt| 16 +++- drivers/firmware/xilinx/zynqmp.c | 8 ++-- 2 files changed, 21 insertions(+), 3 deletions(-) -- 2.7.4
Re: [GIT PULL] Thermal management updates for v5.4-rc1
On Fri, Sep 27, 2019 at 12:29 PM Doug Ledford wrote: > > Because there are literally thousands of developers working on kernel > bits here and there, and you're swatting this particular fly one > developer at a time. Well, at least these days it's also very clearly spelled out in the Documentation directory. And the "don't rebase" does get posted on the mailing lists each time, and I've mentioned it over the years in my release notes too. Besides, I actually only work with about a hundred top-level maintainers, not thousands. Yes, we have thousands of developers, but doing the stats over the 5.0 releases, there have been "only" 131 people sending me pull requests. Sure, more than a couple, but at the same time it's not like this is a "every developer" kind of thing, this is literally subsystem maintainers. We've got a fair number of them, but it's definitely not about thousands. I feel like I've sent that email out way more than a hundred times over the last 15+ years. .. and I don't think having git warn is right, since rebasing is perfectly fine as you are doing development. It's really just that maintainers shouldn't do it for bad reasons and at bad times. And "there was a conflict" and "yesterday" is really one of the absolute worst reasons/times around. Linus
Re: [PATCH] kasan: fix the missing underflow in memmove and memcpy with CONFIG_KASAN_GENERIC=y
On Fri, Sep 27, 2019 at 4:22 PM Walter Wu wrote: > > On Fri, 2019-09-27 at 15:07 +0200, Dmitry Vyukov wrote: > > On Fri, Sep 27, 2019 at 5:43 AM Walter Wu wrote: > > > > > > memmove() and memcpy() have missing underflow issues. > > > When -7 <= size < 0, then KASAN will miss to catch the underflow issue. > > > It looks like shadow start address and shadow end address is the same, > > > so it does not actually check anything. > > > > > > The following test is indeed not caught by KASAN: > > > > > > char *p = kmalloc(64, GFP_KERNEL); > > > memset((char *)p, 0, 64); > > > memmove((char *)p, (char *)p + 4, -2); > > > kfree((char*)p); > > > > > > It should be checked here: > > > > > > void *memmove(void *dest, const void *src, size_t len) > > > { > > > check_memory_region((unsigned long)src, len, false, _RET_IP_); > > > check_memory_region((unsigned long)dest, len, true, _RET_IP_); > > > > > > return __memmove(dest, src, len); > > > } > > > > > > We fix the shadow end address which is calculated, then generic KASAN > > > get the right shadow end address and detect this underflow issue. > > > > > > [1] https://bugzilla.kernel.org/show_bug.cgi?id=199341 > > > > > > Signed-off-by: Walter Wu > > > Reported-by: Dmitry Vyukov > > > --- > > > lib/test_kasan.c | 36 > > > mm/kasan/generic.c | 8 ++-- > > > 2 files changed, 42 insertions(+), 2 deletions(-) > > > > > > diff --git a/lib/test_kasan.c b/lib/test_kasan.c > > > index b63b367a94e8..8bd014852556 100644 > > > --- a/lib/test_kasan.c > > > +++ b/lib/test_kasan.c > > > @@ -280,6 +280,40 @@ static noinline void __init > > > kmalloc_oob_in_memset(void) > > > kfree(ptr); > > > } > > > > > > +static noinline void __init kmalloc_oob_in_memmove_underflow(void) > > > +{ > > > + char *ptr; > > > + size_t size = 64; > > > + > > > + pr_info("underflow out-of-bounds in memmove\n"); > > > + ptr = kmalloc(size, GFP_KERNEL); > > > + if (!ptr) { > > > + pr_err("Allocation failed\n"); > > > + return; > > > + } > > > + > > > + memset((char *)ptr, 0, 64); > > > + memmove((char *)ptr, (char *)ptr + 4, -2); > > > + kfree(ptr); > > > +} > > > + > > > +static noinline void __init kmalloc_oob_in_memmove_overflow(void) > > > +{ > > > + char *ptr; > > > + size_t size = 64; > > > + > > > + pr_info("overflow out-of-bounds in memmove\n"); > > > + ptr = kmalloc(size, GFP_KERNEL); > > > + if (!ptr) { > > > + pr_err("Allocation failed\n"); > > > + return; > > > + } > > > + > > > + memset((char *)ptr, 0, 64); > > > + memmove((char *)ptr + size, (char *)ptr, 2); > > > + kfree(ptr); > > > +} > > > + > > > static noinline void __init kmalloc_uaf(void) > > > { > > > char *ptr; > > > @@ -734,6 +768,8 @@ static int __init kmalloc_tests_init(void) > > > kmalloc_oob_memset_4(); > > > kmalloc_oob_memset_8(); > > > kmalloc_oob_memset_16(); > > > + kmalloc_oob_in_memmove_underflow(); > > > + kmalloc_oob_in_memmove_overflow(); > > > kmalloc_uaf(); > > > kmalloc_uaf_memset(); > > > kmalloc_uaf2(); > > > diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c > > > index 616f9dd82d12..34ca23d59e67 100644 > > > --- a/mm/kasan/generic.c > > > +++ b/mm/kasan/generic.c > > > @@ -131,9 +131,13 @@ static __always_inline bool > > > memory_is_poisoned_n(unsigned long addr, > > > size_t size) > > > { > > > unsigned long ret; > > > + void *shadow_start = kasan_mem_to_shadow((void *)addr); > > > + void *shadow_end = kasan_mem_to_shadow((void *)addr + size - 1) + > > > 1; > > > > > > - ret = memory_is_nonzero(kasan_mem_to_shadow((void *)addr), > > > - kasan_mem_to_shadow((void *)addr + size - 1) + 1); > > > + if ((long)size < 0) > > > + shadow_end = kasan_mem_to_shadow((void *)addr + size); > > > > Hi Walter, > > > > Thanks for working on this. > > > > If size<0, does it make sense to continue at all? We will still check > > 1PB of shadow memory? What happens when we pass such huge range to > > memory_is_nonzero? > > Perhaps it's better to produce an error and bail out immediately if size<0? > > I agree with what you said. when size<0, it is indeed an unreasonable > behavior, it should be blocked from continuing to do. > > > > Also, what's the failure mode of the tests? Didn't they badly corrupt > > memory? We tried to keep tests such that they produce the KASAN > > reports, but don't badly corrupt memory b/c/ we need to run all of > > them. > > Maybe we should first produce KASAN reports and then go to execute > memmove() or do nothing? It looks like it’s doing the following.or? > > void *memmove(void *dest, const void *src, size_t len) > { > + if (long(len)
[PATCH] mm/page_alloc: fix a crash in free_pages_prepare()
On architectures like s390, arch_free_page() could mark the page unused (set_page_unused()) and any access later would trigger a kernel panic. Fix it by moving arch_free_page() after all possible accessing calls. Hardware name: IBM 2964 N96 400 (z/VM 6.4.0) Krnl PSW : 0404e0018000 26c2b96e (__free_pages_ok+0x34e/0x5d8) R:0 T:1 IO:0 EX:0 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3 Krnl GPRS: 88d43af7 00484000 007c 000f 03d080012100 03d080013fc0 0010 275cca48 0100 0008 03d08001 01d0 03d0 26c2b78a 2717fdb0 Krnl Code: 26c2b95c: ec1100b30659 risbgn %r1,%r1,0,179,6 26c2b962: e3201436 pfd 2,1024(%r1) #26c2b968: d7ff10001000 xc 0(256,%r1),0(%r1) >26c2b96e: 41101100 la %r1,256(%r1) 26c2b972: a737fff8 brctg %r3,26c2b962 26c2b976: d7ff10001000 xc 0(256,%r1),0(%r1) 26c2b97c: e3100344 lg %r1,832 26c2b982: ebff1430016a asi 5168(%r1),-1 Call Trace: __free_pages_ok+0x16a/0x5d8) memblock_free_all+0x206/0x290 mem_init+0x58/0x120 start_kernel+0x2b0/0x570 startup_continue+0x6a/0xc0 INFO: lockdep is turned off. Last Breaking-Event-Address: __free_pages_ok+0x372/0x5d8 Kernel panic - not syncing: Fatal exception: panic_on_oops 00: HCPGIR450W CP entered; disabled wait PSW 00020001 8000 26A2379C Signed-off-by: Qian Cai --- mm/page_alloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 3334a769eb91..a54ff6a60649 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1175,11 +1175,11 @@ static __always_inline bool free_pages_prepare(struct page *page, debug_check_no_obj_freed(page_address(page), PAGE_SIZE << order); } - arch_free_page(page, order); if (want_init_on_free()) kernel_init_free_pages(page, 1 << order); kernel_poison_pages(page, 1 << order, 0); + arch_free_page(page, order); if (debug_pagealloc_enabled()) kernel_map_pages(page, 1 << order, 0); -- 1.8.3.1
Re: [PATCH] powerpc/vio: use simple dummy struct device as bus parent
On Fri, Sep 27, 2019 at 2:19 PM Greg Kroah-Hartman wrote: > > On Fri, Sep 27, 2019 at 09:04:02AM -0400, Dan Streetman wrote: > > The dummy vio_bus_device creates the /sys/devices/vio directory, which > > contains real vio devices under it; since it represents itself as having > > a bus = &vio_bus_type, its /sys/devices/vio/uevent does call the bus's > > .uevent function, vio_hotplug(), and as that function won't find a real > > device for the dummy vio_dev, it will return -ENODEV. > > > > One of the main users of the uevent node is udevadm, e.g. when it is called > > with 'udevadm trigger --devices'. Up until recently, it would ignore any > > errors returned when writing to devices' uevent file, but it was recently > > changed to start returning error if it gets an error writing to any uevent > > file: > > https://github.com/systemd/systemd/commit/97afc0351a96e0daa83964df33937967c75c644f > > > > since the /sys/devices/vio/uevent file has always returned ENODEV from > > any write to it, this now causes the udevadm trigger command to return > > an error. This may be fixed in udevadm to ignore ENODEV errors, but the > > vio driver should still be fixed. > > > > This patch changes the arch/powerpc/platform/pseries/vio.c 'dummy' > > parent device into a real dummy device with no .bus, so its uevent > > file will stop returning ENODEV and simply do nothing and return 0. > > > > Signed-off-by: Dan Streetman > > --- > > arch/powerpc/platforms/pseries/vio.c | 11 --- > > 1 file changed, 4 insertions(+), 7 deletions(-) > > > > diff --git a/arch/powerpc/platforms/pseries/vio.c > > b/arch/powerpc/platforms/pseries/vio.c > > index 79e2287991db..63bc16631680 100644 > > --- a/arch/powerpc/platforms/pseries/vio.c > > +++ b/arch/powerpc/platforms/pseries/vio.c > > @@ -32,11 +32,8 @@ > > #include > > #include > > > > -static struct vio_dev vio_bus_device = { /* fake "parent" device */ > > - .name = "vio", > > - .type = "", > > - .dev.init_name = "vio", > > - .dev.bus = &vio_bus_type, > > +static struct device vio_bus = { > > + .init_name = "vio", > > Eeek, no! Why are you creating a static device that will then be > reference counted? Not nice :( sorry! I'll admit that I simply copied what drivers/base/platform.c seemed to be doing. > > What's wrong with a simple call to device_create() for your "fake" > device you want to make here? That's what it is there for :) ack, will send a new patch using that. thanks! > > thanks, > > greg k-h
Re: [GIT PULL] Thermal management updates for v5.4-rc1
On Fri, Sep 27, 2019 at 9:30 PM Doug Ledford wrote: > Because there are literally thousands of developers working on kernel > bits here and there, and you're swatting this particular fly one > developer at a time. I strongly disagree. One of the golden rules of kernel development is, read what Linus writes. Especially during the merge window. Thanks, //richard
Re: [GIT PULL] Thermal management updates for v5.4-rc1
On Fri, 2019-09-27 at 12:41 -0700, Linus Torvalds wrote: > On Fri, Sep 27, 2019 at 12:29 PM Doug Ledford > wrote: > > Because there are literally thousands of developers working on > > kernel > > bits here and there, and you're swatting this particular fly one > > developer at a time. > > Well, at least these days it's also very clearly spelled out in the > Documentation directory. Yeah, but let's be fair. How many people read the documentation fully? Or even if some new maintainer read it, did it really sink in? Or will they rebase a year later not thinking about it? > And the "don't rebase" does get posted on the mailing lists each time, > and I've mentioned it over the years in my release notes too. Lots of people here and there miss those things. You never know who caught what. > Besides, I actually only work with about a hundred top-level > maintainers, not thousands. Yes, we have thousands of developers, but > doing the stats over the 5.0 releases, there have been "only" 131 > people sending me pull requests. Sure, more than a couple, but at the > same time it's not like this is a "every developer" kind of thing, > this is literally subsystem maintainers. We've got a fair number of > them, but it's definitely not about thousands. > > I feel like I've sent that email out way more than a hundred times > over the last 15+ years. I'm sure you probably have. I think I got it two or three times before I got all the nuances of which rebases were OK and which weren't :-). > .. and I don't think having git warn is right, since rebasing is > perfectly fine as you are doing development. > > It's really just that maintainers shouldn't do it for bad reasons and > at bad times. > > And "there was a conflict" and "yesterday" is really one of the > absolute worst reasons/times around. I think you send it out a lot because it doesn't get driven home in people's heads until they get yelled at. And there's more to the badness of a rebase than just annoying you when you want to see the conflicts to judge things for yourself. There are legitimate issues such as a rebase wipes out history. Or if you rebase a public tree it blows up everyone that is tracking that branch. These things apply even to when you are doing your own development. Or rebases might wipe out merge commits from other trees, depending on options. A git warning solely for you might not truly be appropriate, but I'm not at all convinced that a git warning with a more general "Here are the pros and cons of rebases, with a list of dos and donts if you are going to do a rebase" document is a bad idea, and might actually help you out too. -- Doug Ledford GPG KeyID: B826A3330E572FDD Fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD signature.asc Description: This is a digitally signed message part
Re: [PATCH v2] arm64/sve: Fix wrong free for task->thread.sve_state
Hi Dave, On 27/09/2019 17:15, Dave Martin wrote: > On Fri, Sep 27, 2019 at 11:39:49AM -0400, Masayoshi Mizuma wrote: >> From: Masayoshi Mizuma >> >> The system which has SVE feature crashed because of >> the memory pointed by task->thread.sve_state was destroyed >> by someone. >> >> That is because sve_state is freed while the forking the >> child process. The child process has the pointer of sve_state >> which is same as the parent's because the child's task_struct >> is copied from the parent's one. If the copy_process() >> fails as an error on somewhere, for example, copy_creds(), >> then the sve_state is freed even if the parent is alive. >> The flow is as follows. >> >> copy_process >> p = dup_task_struct >> => arch_dup_task_struct >> *dst = *src; // copy the entire region. >> : >> retval = copy_creds >> if (retval < 0) >> goto bad_fork_free; >> : >> bad_fork_free: >> ... >> delayed_free_task(p); >>=> free_task >> => arch_release_task_struct >> => fpsimd_release_task >> => __sve_free >>=> kfree(task->thread.sve_state); >> // free the parent's sve_state >> >> Move child's sve_state = NULL and clearing TIF_SVE flag >> to arch_dup_task_struct() so that the child doesn't free the >> parent's one. > > You could also add: > > --8<-- > There is no need to wait until copy_process() to clear TIF_SVE for > dst, becuase the thread flags for dst are initialized already by > copying the src task_struct. > > This change simplifies the code, so get rid of comments that are no > longer needed. > -->8-- > >> >> Cc: sta...@vger.kernel.org > > Since SVE only exists from v4.15, it may be helpful to specify that, > i.e., replace that Cc line with: > > Cc: # 4.15.x- > > > Otherwise, I'm happy to see this applied, but I'd like somebody to > confirm that this change definitely fixes the bug. I am working on a reproducer for this. So I should be able to test it. Cheers, -- Julien Grall
Re: [GIT PULL] Thermal management updates for v5.4-rc1
On Fri, 2019-09-27 at 21:52 +0200, Richard Weinberger wrote: > On Fri, Sep 27, 2019 at 9:30 PM Doug Ledford > wrote: > > Because there are literally thousands of developers working on > > kernel > > bits here and there, and you're swatting this particular fly one > > developer at a time. > > I strongly disagree. One of the golden rules of kernel development is, > read what Linus writes. Especially during the merge window. > > Thanks, > //richard Developers come and go. Your argument is temporally flawed. A developer might start working on the tree, read everything during a merge window, and not catch one of Linus' rebase rants prior to committing a rebase felony of their own. Besides, I don't think this rule of yours is all that universal. If Linus is off on a thread about arm64 device tree brokenness and someone does a rebase and he rants about it, I'm very likely to miss that rant. I read what I reasonably deem to be relevant to me and my work, or sometimes additional stuff that just jumps out at me. But I never learned to speed read so I don't even try to read it all and wouldn't agree to a rule that says I have to. -- Doug Ledford GPG KeyID: B826A3330E572FDD Fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD signature.asc Description: This is a digitally signed message part
Re: [PATCH] ieee802154: ca8210: prevent memory leak
Hello. On 18.09.19 00:47, Navid Emamdoost wrote: In ca8210_probe the allocated pdata needs to be assigned to spi_device->dev.platform_data before calling ca8210_get_platform_data. Othrwise when ca8210_get_platform_data fails pdata cannot be released. Signed-off-by: Navid Emamdoost --- drivers/net/ieee802154/ca8210.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c index b188fce3f641..229d70a897ca 100644 --- a/drivers/net/ieee802154/ca8210.c +++ b/drivers/net/ieee802154/ca8210.c @@ -3152,12 +3152,12 @@ static int ca8210_probe(struct spi_device *spi_device) goto error; } + priv->spi->dev.platform_data = pdata; ret = ca8210_get_platform_data(priv->spi, pdata); if (ret) { dev_crit(&spi_device->dev, "ca8210_get_platform_data failed\n"); goto error; } - priv->spi->dev.platform_data = pdata; ret = ca8210_dev_com_init(priv); if (ret) { As Harry seems to be unavailable I am taking this patch directly. This patch has been applied to the wpan tree and will be part of the next pull request to net. Thanks! regards Stefan Schmidt
Re: [PATCH RT 4/8] sched: migrate disable: Protect cpus_ptr with lock
On Fri, 2019-09-27 at 14:19 +0200, Sebastian Andrzej Siewior wrote: > On 2019-09-26 11:52:42 [-0500], Scott Wood wrote: > > Looks good, thanks! > > Thanks, just released. > Moving forward. It would be nice to have some DL-dev feedback on DL > patch. For the remaining once, could please throw Steven's > stress-test-hostplug-cpu-script? If that one does not complain I don't > see a reason why not apply the patches (since they improve performance > and do not break anything while doing so). I'd been using a quick-and-dirty script that does something similar, and ran it along with rcutorture, a kernel build, and something that randomly changes affinities -- though with these loads I have to ignore occasional RCU forward progress complaints that Paul said were expected with this version of the RCU code, XFS warnings that happen even on unmodified non-RT, and sometimes a transitory netdev timeout that is not that surprising given the heavy load and affinity stress (I get it without these patches as well, though it takes longer). I just ran Steven's script (both alone and with the other stuff above) and saw no difference. -Scott
[GIT PULL] nfsd changes for 5.4
Please pull nfsd changes from: git://linux-nfs.org/~bfields/linux.git tags/nfsd-5.4 --b. Highlights: - add a new knfsd file cache, so that we don't have to open and close on each (NFSv2/v3) READ or WRITE. This can speed up read and write in some cases. It also replaces our readahead cache. - Prevent silent data loss on write errors, by treating write errors like server reboots for the purposes of write caching, thus forcing clients to resend their writes. - Tweak the code that allocates sessions to be more forgiving, so that NFSv4.1 mounts are less likely to hang when a server already has a lot of clients. - Eliminate an arbitrary limit on NFSv4 ACL sizes; they should now be limited only by the backend filesystem and the maximum RPC size. - Allow the server to enforce use of the correct kerberos credentials when a client reclaims state after a reboot. And some miscellaneous smaller bugfixes and cleanup. Dave Wysochanski (1): SUNRPC: Track writers of the 'channel' file to improve cache_listeners_exist include/linux/sunrpc/cache.h | 6 +++--- net/sunrpc/cache.c | 12 2 files changed, 11 insertions(+), 7 deletions(-)
Re: [PATCH 1/2] dt-bindings: clock: meson: add A1 clock controller bindings
On Wed, Sep 25, 2019 at 6:45 AM Jian Hu wrote: > > Add the documentation to support Amlogic A1 clock driver, > and add A1 clock controller bindings. > > Signed-off-by: Jian Hu > Signed-off-by: Jianxin Pan > --- > .../devicetree/bindings/clock/amlogic,a1-clkc.yaml | 65 + > include/dt-bindings/clock/a1-clkc.h| 102 > + > 2 files changed, 167 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml > create mode 100644 include/dt-bindings/clock/a1-clkc.h > > diff --git a/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml > b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml > new file mode 100644 > index 000..f012eb2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml > @@ -0,0 +1,65 @@ > +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */ (GPL-2.0-only OR BSD-2-Clause) please. Rob
Re: [GIT PULL] RISC-V additional updates for v5.4-rc1
The pull request you sent on Fri, 27 Sep 2019 11:25:13 -0700 (PDT): > git://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux.git > tags/riscv/for-v5.4-rc1-b has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/568d850e3c6015acec8f854f5be97766497a676b Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [GIT PULL] Second batch of KVM changes for Linux 5.4 merge window
The pull request you sent on Fri, 27 Sep 2019 14:08:02 +0200: > https://git.kernel.org/pub/scm/virt/kvm/kvm.git tags/for-linus has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/8bbe0dec38e147a50e9dd5f585295f7e68e0f2d0 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [GIT PULL] arch/nios2 update for v5.4
The pull request you sent on Fri, 27 Sep 2019 16:50:52 +0800: > (unable to parse the git remote) has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/70570a6418be2bd28be30fda700e57a81df7282b Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [PATCH v2 2/3] KVM: x86: fix nested guest live migration with PML
On 9/27/19 4:15 AM, Paolo Bonzini wrote: > Shadow paging is fundamentally incompatible with the page-modification > log, because the GPAs in the log come from the wrong memory map. > In particular, for the EPT page-modification log, the GPAs in the log come > from L2 rather than L1. (If there was a non-EPT page-modification log, > we couldn't use it for shadow paging because it would log GVAs rather > than GPAs). > > Therefore, we need to rely on write protection to record dirty pages. > This has the side effect of bypassing PML, since writes now result in an > EPT violation vmexit. > > This is relatively easy to add to KVM, because pretty much the only place > that needs changing is spte_clear_dirty. The first access to the page > already goes through the page fault path and records the correct GPA; > it's only subsequent accesses that are wrong. Therefore, we can equip > set_spte (where the first access happens) to record that the SPTE will > have to be write protected, and then spte_clear_dirty will use this > information to do the right thing. > > Signed-off-by: Paolo Bonzini > --- Reviewed-by: Junaid Shahid
Re: [PATCH 00/22] x86, objtool: several fixes/improvements
On Thu, Jul 18, 2019 at 3:26 PM Nick Desaulniers wrote: > > On Tue, Jul 16, 2019 at 4:17 PM Josh Poimboeuf wrote: > > > > > > > 2) There's also an issue in clang where a large switch table had a > > > > > bunch > > > > >of unused (bad) entries. It's not a code correctness issue, but > > > > >hopefully it can get fixed in clang anyway. See patch 20/22 for > > > > > more > > > > >details. > > > > > > Thanks for the report, let's follow up on steps for me to reproduce. > > > > Just to clarify, there are two clang issues. Both of them were reported > > originally by Arnd, IIRC. > > > > 1) The one described above and in patch 20, where the switch table is > >mostly unused entries. Not a real bug, but it's a bit sloppy and > >wasteful, and objtool doesn't know how to interpret it. > > Thanks for the concise reports. Will follow up on these in: > https://github.com/ClangBuiltLinux/linux/issues/611 Following up on this one; in one of the test cases we determined that the default destination of an exhaustive switch wasn't getting cleaned up properly, and is being fixed in: https://reviews.llvm.org/D68131 https://bugs.llvm.org/show_bug.cgi?id=43129 I'm not sure that was the precise issue you described, or if there's more than one bug here, but hopefully it will help. > > > > > 2) The bug with the noreturn call site having a different stack size > >depending on which code path was taken. > > and: > https://github.com/ClangBuiltLinux/linux/issues/612 -- Thanks, ~Nick Desaulniers
Re: [PATCH v3 1/3] dt-bindings: dmaengine: dma-common: Change dma-channel-mask to uint32-array
On Thu, Sep 26, 2019 at 02:19:52PM +0300, Peter Ujfalusi wrote: > Make the dma-channel-mask to be usable for controllers with more than 32 > channels. > > Signed-off-by: Peter Ujfalusi > --- > Documentation/devicetree/bindings/dma/dma-common.yaml | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/dma/dma-common.yaml > b/Documentation/devicetree/bindings/dma/dma-common.yaml > index ed0a49a6f020..4527f20301ff 100644 > --- a/Documentation/devicetree/bindings/dma/dma-common.yaml > +++ b/Documentation/devicetree/bindings/dma/dma-common.yaml > @@ -25,11 +25,18 @@ properties: >Used to provide DMA controller specific information. > >dma-channel-mask: > -$ref: /schemas/types.yaml#definitions/uint32 > description: >Bitmask of available DMA channels in ascending order that are >not reserved by firmware and are available to the >kernel. i.e. first channel corresponds to LSB. > + The first item in the array is for channels 0-31, the second is for > + channels 32-63, etc. > +allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32-array > +items: > + minItems: 1 > + # Should be enough > + maxItems: 255 'items' has to be a separate sub-schema from $ref to have any effect: allOf: - $ref: /schemas/types.yaml#/definitions/uint32-array items: minItems: 1 # Should be enough maxItems: 255 Or (note the added '-'): allOf: - $ref: /schemas/types.yaml#/definitions/uint32-array - items: minItems: 1 # Should be enough maxItems: 255 The first way is my preference. Rob
Re: [Patch v4 4/8] media: i2c: ov2659: fix s_stream return value
Hi Benoit, thank you for the patch. On Fri, Sep 27, 2019 at 7:46 PM Benoit Parrot wrote: > > In ov2659_s_stream() return value for invoked function should be checked > and propagated. > > Signed-off-by: Benoit Parrot > --- > drivers/media/i2c/ov2659.c | 11 +++ > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c > index f77320e8a60d..cd4625432264 100644 > --- a/drivers/media/i2c/ov2659.c > +++ b/drivers/media/i2c/ov2659.c > @@ -1187,10 +1187,13 @@ static int ov2659_s_stream(struct v4l2_subdev *sd, > int on) > goto unlock; > } > > - ov2659_set_pixel_clock(ov2659); > - ov2659_set_frame_size(ov2659); > - ov2659_set_format(ov2659); > - ov2659_set_streaming(ov2659, 1); > + ret = ov2659_set_pixel_clock(ov2659); > + if (!ret) > + ret = ov2659_set_frame_size(ov2659); > + if (!ret) > + ret = ov2659_set_format(ov2659); > + if (!ret) > + ov2659_set_streaming(ov2659, 1); > ov2659->streaming = on; > the "ov2659->streaming = on;" should only be set if above calls succeed, otherwise we might hit -EBUSY during set_fmt. Cheers, --Prabhakar Lad > unlock: > -- > 2.17.1 >
Re: [Patch v4 1/8] media: i2c: ov2659: Fix for image wrap-around in lower resolution
On Fri, Sep 27, 2019 at 7:47 PM Benoit Parrot wrote: > > Based on recently found sensor configuration examples, it was > discovered that when scaling and binning are used for the lower > resolutions (i.e. 640x480, 320x240) the read offset has to be > increased otherwise the image appears to be wrapped around. > > Signed-off-by: Benoit Parrot > Signed-off-by: Jyri Sarha > --- > drivers/media/i2c/ov2659.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Acked-by: Lad, Prabhakar Cheers, --Prabhakar Lad > diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c > index f4ded0669ff9..17573257097d 100644 > --- a/drivers/media/i2c/ov2659.c > +++ b/drivers/media/i2c/ov2659.c > @@ -661,7 +661,7 @@ static struct sensor_register ov2659_vga[] = { > { REG_TIMING_HORIZ_FORMAT, 0x01 }, > { 0x370a, 0x52 }, > { REG_VFIFO_READ_START_H, 0x00 }, > - { REG_VFIFO_READ_START_L, 0x80 }, > + { REG_VFIFO_READ_START_L, 0xa0 }, > { REG_ISP_CTRL02, 0x10 }, > { REG_NULL, 0x00 }, > }; > @@ -709,7 +709,7 @@ static struct sensor_register ov2659_qvga[] = { > { REG_TIMING_HORIZ_FORMAT, 0x01 }, > { 0x370a, 0x52 }, > { REG_VFIFO_READ_START_H, 0x00 }, > - { REG_VFIFO_READ_START_L, 0x80 }, > + { REG_VFIFO_READ_START_L, 0xa0 }, > { REG_ISP_CTRL02, 0x10 }, > { REG_NULL, 0x00 }, > }; > -- > 2.17.1 >
Re: [Patch v4 2/8] media: i2c: ov2659: Fix sensor detection to actually fail when device is not present
On Fri, Sep 27, 2019 at 7:47 PM Benoit Parrot wrote: > > Make sure that if the expected sensor device id register > is not recognized properly the failure is propagated > up so devices are not left partially initialized. > > Signed-off-by: Benoit Parrot > Signed-off-by: Jyri Sarha > --- > drivers/media/i2c/ov2659.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > Acked-by: Lad, Prabhakar Cheers, --Prabhakar Lad > diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c > index 17573257097d..efbe6dc720e2 100644 > --- a/drivers/media/i2c/ov2659.c > +++ b/drivers/media/i2c/ov2659.c > @@ -1330,11 +1330,12 @@ static int ov2659_detect(struct v4l2_subdev *sd) > unsigned short id; > > id = OV265X_ID(pid, ver); > - if (id != OV2659_ID) > + if (id != OV2659_ID) { > dev_err(&client->dev, > "Sensor detection failed (%04X, %d)\n", > id, ret); > - else { > + ret = -ENODEV; > + } else { > dev_info(&client->dev, "Found OV%04X sensor\n", id); > ret = ov2659_init(sd, 0); > } > -- > 2.17.1 >
Re: [Patch v4 3/8] media: i2c: ov2659: Cleanup include file list
On Fri, Sep 27, 2019 at 7:47 PM Benoit Parrot wrote: > > Several of include files listed are not explicitly needed. > If they are need then they are implicitly included. > > Reduce the list of includes to an easier to manage list. > > Signed-off-by: Benoit Parrot > --- > drivers/media/i2c/ov2659.c | 14 -- > 1 file changed, 14 deletions(-) > Acked-by: Lad, Prabhakar Cheers, --Prabhakar Lad > diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c > index efbe6dc720e2..f77320e8a60d 100644 > --- a/drivers/media/i2c/ov2659.c > +++ b/drivers/media/i2c/ov2659.c > @@ -22,29 +22,15 @@ > > #include > #include > -#include > -#include > -#include > -#include > #include > -#include > -#include > #include > -#include > #include > -#include > -#include > -#include > > -#include > #include > -#include > #include > -#include > #include > #include > #include > -#include > #include > > #define DRIVER_NAME "ov2659" > -- > 2.17.1 >
Re: [Patch v4 7/8] media: i2c: ov2659: Fix missing 720p register config
On Fri, Sep 27, 2019 at 7:47 PM Benoit Parrot wrote: > > The initial registers sequence is only loaded at probe > time. Afterward only the resolution and format specific > register are modified. Care must be taken to make sure > registers modified by one resolution setting are reverted > back when another resolution is programmed. > > This was not done properly for the 720p case. > > Signed-off-by: Benoit Parrot > --- > drivers/media/i2c/ov2659.c | 4 > 1 file changed, 4 insertions(+) > Acked-by: Lad, Prabhakar Cheers, --Prabhakar Lad > diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c > index 7d0baa386644..720310e0725d 100644 > --- a/drivers/media/i2c/ov2659.c > +++ b/drivers/media/i2c/ov2659.c > @@ -412,10 +412,14 @@ static struct sensor_register ov2659_720p[] = { > { REG_TIMING_YINC, 0x11 }, > { REG_TIMING_VERT_FORMAT, 0x80 }, > { REG_TIMING_HORIZ_FORMAT, 0x00 }, > + { 0x370a, 0x12 }, > { 0x3a03, 0xe8 }, > { 0x3a09, 0x6f }, > { 0x3a0b, 0x5d }, > { 0x3a15, 0x9a }, > + { REG_VFIFO_READ_START_H, 0x00 }, > + { REG_VFIFO_READ_START_L, 0x80 }, > + { REG_ISP_CTRL02, 0x00 }, > { REG_NULL, 0x00 }, > }; > > -- > 2.17.1 >
Re: [Patch v4 5/8] media: dt-bindings: ov2659: add powerdown/reset-gpios optional property
On Fri, Sep 27, 2019 at 7:47 PM Benoit Parrot wrote: > > Add powerdown-gpios and reset-gpios to the list of optional properties > for the OV2659 camera sensor. > > Signed-off-by: Benoit Parrot > --- > Documentation/devicetree/bindings/media/i2c/ov2659.txt | 9 + > 1 file changed, 9 insertions(+) > Acked-by: Lad, Prabhakar Cheers, --Prabhakar Lad > diff --git a/Documentation/devicetree/bindings/media/i2c/ov2659.txt > b/Documentation/devicetree/bindings/media/i2c/ov2659.txt > index cabc7d827dfb..92989a619f29 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ov2659.txt > +++ b/Documentation/devicetree/bindings/media/i2c/ov2659.txt > @@ -12,6 +12,12 @@ Required Properties: > - clock-names: should be "xvclk". > - link-frequencies: target pixel clock frequency. > > +Optional Properties: > +- powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any. > + Active high with internal pull down resistor. > +- reset-gpios: reference to the GPIO connected to the resetb pin, if any. > + Active low with internal pull up resistor. > + > For further reading on port node refer to > Documentation/devicetree/bindings/media/video-interfaces.txt. > > @@ -27,6 +33,9 @@ Example: > clocks = <&clk_ov2659 0>; > clock-names = "xvclk"; > > + powerdown-gpios = <&gpio6 14 GPIO_ACTIVE_HIGH>; > + reset-gpios = <&gpio6 15 GPIO_ACTIVE_LOW>; > + > port { > ov2659_0: endpoint { > remote-endpoint = <&vpfe_ep>; > -- > 2.17.1 >
Re: [PATCH] powerpc/prom_init: Undo relocation before entering secure mode
Thiago Jung Bauermann writes: > Thiago Jung Bauermann writes: > >> The ultravisor will do an integrity check of the kernel image but we >> relocated it so the check will fail. Restore the original image by >> relocating it back to the kernel virtual base address. >> >> This works because during build vmlinux is linked with an expected virtual >> runtime address of KERNELBASE. >> >> Fixes: 6a9c930bd775 ("powerpc/prom_init: Add the ESM call to prom_init") >> Signed-off-by: Thiago Jung Bauermann > > I meant to put a Suggested-by: Paul Mackerras > > Sorry. Will add it if there's a v2. Ping? -- Thiago Jung Bauermann IBM Linux Technology Center
Re: [alsa-devel] [PATCH v2] ASoC: Intel: Skylake: prevent memory leak in snd_skl_parse_uuids
On Fri, Sep 27, 2019 at 7:39 PM Pierre-Louis Bossart wrote: > > The problem with solution #1 is freeing orphaned pointer. It will work, > > but it's simple is not okay from object life time prospective. > > ?? I don't get your point at all Andy. > Two allocations happens in a loop and if the second fails, you free the > first and then jump to free everything allocated in the previous > iterations. what am I missing? Two things: - one allocation is done with kzalloc(), while the other one with devm_kcalloc() - due to above the ordering of resources is reversed -- With Best Regards, Andy Shevchenko
Re: Do we need to correct barriering in circular-buffers.rst?
On Fri, Sep 27, 2019 at 5:49 AM Peter Zijlstra wrote: > > On Fri, Sep 27, 2019 at 11:51:07AM +0200, Andrea Parri wrote: > > > For the record, the LKMM doesn't currently model "order" derived from > > control dependencies to a _plain_ access (even if the plain access is > > a write): in particular, the following is racy (as far as the current > > LKMM is concerned): > > > > C rb > > > > { } > > > > P0(int *tail, int *data, int *head) > > { > > if (READ_ONCE(*tail)) { > > *data = 1; > > smp_wmb(); > > WRITE_ONCE(*head, 1); > > } > > } > > > > P1(int *tail, int *data, int *head) > > { > > int r0; > > int r1; > > > > r0 = READ_ONCE(*head); > > smp_rmb(); > > r1 = *data; > > smp_mb(); > > WRITE_ONCE(*tail, 1); > > } > > > > Replacing the plain "*data = 1" with "WRITE_ONCE(*data, 1)" (or doing > > s/READ_ONCE(*tail)/smp_load_acquire(tail)) suffices to avoid the race. > > Maybe I'm short of imagination this morning... but I can't currently > > see how the compiler could "break" the above scenario. > > The compiler; if sufficiently smart; is 'allowed' to change P0 into > something terrible like: > > *data = 1; > if (*tail) { > smp_wmb(); > *head = 1; > } else > *data = 0; I don't think so. This snippet has different side effects than P0. P0 never assigned *data to zero, this snippet does. P0 *may* assign *data to 1. This snippet will unconditionally assign to *data, conditionally 1 or 0. I think the REVERSE transform (from your snippet to P0) would actually be legal, but IANALL (I am not a language lawyer; haven't yet passed the BAR). > > > (assuming it knows *data was 0 from a prior store or something) Oh, in that case I'm less sure (I still don't think so, but I would love to be proven wrong, preferably with a godbolt link). I think the best would be to share a godbolt.org link to a case that's clearly broken, or cite the relevant part of the ISO C standard (which itself leaves room for interpretation), otherwise the discussion is too hypothetical. Those two things are single-handedly the best way to communicate with compiler folks. > > Using WRITE_ONCE() defeats this because volatile indicates external > visibility. Could data be declared as a pointer to volatile qualified int? > > > I also didn't spend much time thinking about it. memory-barriers.txt > > has a section "CONTROL DEPENDENCIES" dedicated to "alerting developers > > using control dependencies for ordering". That's quite a long section > > (and probably still incomplete); the last paragraph summarizes: ;-) > > Barring LTO the above works for perf because of inter-translation-unit > function calls, which imply a compiler barrier. > > Now, when the compiler inlines, it looses that sync point (and thereby > subtlely changes semantics from the non-inline variant). I suspect LTO > does the same and can cause subtle breakage through this transformation. Do you have a bug report or godbolt link for the above? I trust that you're familiar enough with the issue to be able to quickly reproduce it? These descriptions of problems are difficult for me to picture in code or generated code, and when I try to read through memory-barriers.txt my eyes start to glaze over (then something else catches fire and I have to go put that out). Having a concise test case I think would better illustrate potential issues with LTO that we'd then be able to focus on trying to fix/support. We definitely have heavy hitting language lawyers and our LTO folks are super sharp; I just don't have the necessary compiler experience just yet to be as helpful in these discussions as we need but I'm happy to bring them cases that don't work for the kernel and drive their resolution. > > > (*) Compilers do not understand control dependencies. It is therefore > > your job to ensure that they do not break your code. > > It is one the list of things I want to talk about when I finally get > relevant GCC and LLVM people in the same room ;-) > > Ideally the compiler can be taught to recognise conditionals dependent > on 'volatile' loads and disallow problematic transformations around > them. > > I've added Nick (clang) and Jose (GCC) on Cc, hopefully they can help > find the right people for us. -- Thanks, ~Nick Desaulniers
Re: [PATCH] media: i2c: adv7180: fix adv7280 BT.656-4 compatibility
Hi Tim, On 2019-09-27 12:26:40 -0700, Tim Harvey wrote: > On Fri, Sep 27, 2019 at 12:04 PM Niklas Söderlund > wrote: > > > > Hi Tim, > > > > Sorry for taking to so long to look at this. > > > > On 2019-09-23 15:04:47 -0700, Tim Harvey wrote: > > > On Thu, Aug 29, 2019 at 7:29 AM Niklas Söderlund > > > wrote: > > > > > > > > Hi, > > > > > > > > On 2019-08-29 13:43:49 +0200, Hans Verkuil wrote: > > > > > Adding Niklas. > > > > > > > > > > Niklas, can you take a look at this? > > > > > > > > I'm happy to have a look at this. I'm currently moving so all my boards > > > > are in a box somewhere. I hope to have my lab up and running next week, > > > > so if this is not urgent I will look at it then. > > > > > > > > > > Niklas, > > > > > > Have you looked at this yet? Without this patch the ADV7280A does not > > > output proper BT.656. We tested this on a Gateworks Ventana GW5404-G > > > which uses the ADV7280A connected to the IMX6 CSI parallel bus. I'm > > > hoping to see this get merged and perhaps backported to older kernels. > > > > I only have access to an adv7180 so I was unable to test this patch. > > After reviewing the documentation I think the patch is OK if what you > > want is to unconditionally switch the driver from outputting BT.656-3 to > > outputting BT.656-4. > > > > As this change would effect a large number of compat strings (adv7280, > > adv7280-m, adv7281, adv7281-m, adv7281-ma, adv7282, adv7282-m) and the > > goal is to back port it I'm a bit reluctant to adding my tag to this > > patch as I'm not sure if this will break other setups. > > > > From the documentation about the BT.656-4 register (address 0x04 bit 7): > > > > Between Revision 3 and Revision 4 of the ITU-R BT.656 standards, > > the ITU has changed the toggling position for the V bit within > > the SAV EAV codes for NTSC. The ITU-R BT.656-4 standard > > bit allows the user to select an output mode that is compliant > > with either the previous or new standard. For further information, > > visit the International Telecommunication Union website. > > > > Note that the standard change only affects NTSC and has no > > bearing on PAL. > > > > When ITU-R BT.656-4 is 0 (default), the ITU-R BT.656-3 > > specification is used. The V bit goes low at EAV of Line 10 > > and Line 273. > > > > When ITU-R BT.656-4 is 1, the ITU-R BT.656-4 specification is > > used. The V bit goes low at EAV of Line 20 and Line 283. > > > > Do you know what effects such a change would bring? Looking at the > > driver BT.656-4 seems to be set unconditionally for some adv7180 chips. > > > > Niklas, > > Quite simply, we have a board that has an ADV7180 attached to the > parallel CSI of an IMX6 that worked fine with mainline drivers then > when we revised this board to attach an ADV7280A in the same way > capture failed to sync. Investigation showed that the NEWAVMODE > differed between the two. I understand your problem, the driver configures adv7180 and adv7280 differently. > > So if the point of the driver is to configure the variants in the same > way, this patch needs to be applied. I'm not sure that is the point of the driver. As the driver today configures different compatible strings differently. Some as ITU-R BT.656-3 and some as ITU-R BT.656-4, I can only assume there is a reason for that. > > I would maintain that the adv7180 comes up with NEWAVMODE enabled and > in order to be compatible we must configure the adv7282 the same. > > The same argument can be made for setting the V bit end position in > NTSC mode - its done for the adv7180 so for compatible output it > should be done for the adv7282. I understand that this is needed to make it a drop-in replacement for the adv7180 in your use-case. But I'm not sure it is a good idea for other users of the driver. What if someone is already using a adv7282 on a board and depends on it providing ITU-R BT.656-3 and the old settings? If this patch is picked up there use-cases may break. I'm not sure what the best way forward is I'm afraid. Looking at video-interfaces.txt we have a device tree property bus-type which is used to describe the bus is a BT.656 bus but not which revision of it. I'm not really found of driver specific bus descriptions, but maybe this is a case where one might consider adding one? Hans what do you think? > > > > > > > Regards, > > > > > > Tim > > > > > > > > > > > > > Regards, > > > > > > > > > > Hans > > > > > > > > > > On 8/27/19 11:55 PM, Matthew Michilot wrote: > > > > > > From: Matthew Michilot > > > > > > > > > > > > Captured video would be out of sync when using the adv7280 with > > > > > > the BT.656-4 protocol. Certain registers (0x04, 0x31, 0xE6) had to > > > > > > be configured properly to ensure BT.656-4 compatibility. > > > > > > > > > > > > An error in the adv7280 reference manual suggested that EAV/SAV mode > > > > > > was enabled by default, however upon inspecting register 0x31, it >
Re: [PATCH v3 2/3] dt-bindings: dma: ti-edma: Document dma-channel-mask for EDMA
On Thu, Sep 26, 2019 at 02:19:53PM +0300, Peter Ujfalusi wrote: > Similarly to paRAM slots, channels can be used by other cores. > > The common dma-channel-mask property can be used for specifying the > available channels. > > Signed-off-by: Peter Ujfalusi > --- > Documentation/devicetree/bindings/dma/ti-edma.txt | 8 > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/devicetree/bindings/dma/ti-edma.txt > b/Documentation/devicetree/bindings/dma/ti-edma.txt > index 4bbc94d829c8..014187088020 100644 > --- a/Documentation/devicetree/bindings/dma/ti-edma.txt > +++ b/Documentation/devicetree/bindings/dma/ti-edma.txt > @@ -42,6 +42,11 @@ Optional properties: > - ti,edma-reserved-slot-ranges: PaRAM slot ranges which should not be used by > the driver, they are allocated to be used by for example the > DSP. See example. > +- dma-channel-mask: Mask of usable channels. > + Single uint32 for EDMA with 32 channels, array of two uint32 for > + EDMA with 64 channels. See example and > + Documentation/devicetree/bindings/dma/dma-common.yaml > + > > > -- > eDMA3 Transfer Controller > @@ -91,6 +96,9 @@ edma: edma@4900 { > ti,edma-memcpy-channels = <20 21>; > /* The following PaRAM slots are reserved: 35-44 and 100-109 */ > ti,edma-reserved-slot-ranges = <35 10>, <100 10>; > + /* The following channels are reserved: 35-44 */ > + dma-channel-mask = <0x>, /* Channel 0-31 */ > +<0xe007>; /* Channel 32-63 */ Doesn't matter yet, but you have a mismatch here with the schema. While the <> around each int or not doesn't matter for the dtb, it does for the schema. dma-channel-mask = <0x>, <0xe007>; minItems: 1 maxItems: 255 dma-channel-mask = <0x 0xe007>; items: minItems: 1 maxItems: 255 I think the latter case is slightly more logical here as you have 1 thing (a mask). If had N of something (like interrupts), then the former makes sense. Rob
WARNING in em28xx_init_extension
Hello, syzbot found the following crash on: HEAD commit:2994c077 usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git usb-fuzzer console output: https://syzkaller.appspot.com/x/log.txt?x=171c4be360 kernel config: https://syzkaller.appspot.com/x/.config?x=69ddefac6929256a dashboard link: https://syzkaller.appspot.com/bug?extid=76929be61691e7b3904b compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=137d73bd60 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14f0c8cb60 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+76929be61691e7b39...@syzkaller.appspotmail.com em28xx 1-1:0.221: Audio interface 221 found (Vendor Class) em28xx 1-1:0.221: unknown em28xx chip ID (0) em28xx 1-1:0.221: Config register raw data: 0xfffb em28xx 1-1:0.221: AC97 chip type couldn't be determined em28xx 1-1:0.221: No AC97 audio processor [ cut here ] list_add corruption. prev->next should be next (87779de0), but was 8352fdcc. (prev=8881d2ecc240). WARNING: CPU: 1 PID: 83 at lib/list_debug.c:26 __list_add_valid+0x99/0xf0 lib/list_debug.c:26 Kernel panic - not syncing: panic_on_warn set ... CPU: 1 PID: 83 Comm: kworker/1:2 Not tainted 5.3.0+ #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: usb_hub_wq hub_event Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0xca/0x13e lib/dump_stack.c:113 panic+0x2a3/0x6da kernel/panic.c:219 __warn.cold+0x20/0x4a kernel/panic.c:576 report_bug+0x262/0x2a0 lib/bug.c:186 fixup_bug arch/x86/kernel/traps.c:179 [inline] fixup_bug arch/x86/kernel/traps.c:174 [inline] do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:272 do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:291 invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1028 RIP: 0010:__list_add_valid+0x99/0xf0 lib/list_debug.c:26 Code: 48 c7 c7 60 06 db 85 e8 2a 7a 30 ff 0f 0b 48 83 c4 08 31 c0 5d 41 5c c3 48 89 f1 48 c7 c7 20 07 db 85 4c 89 e6 e8 0c 7a 30 ff <0f> 0b 31 c0 eb c5 48 89 f2 4c 89 e1 48 89 ee 48 c7 c7 a0 07 db 85 RSP: 0018:8881d93cf120 EFLAGS: 00010286 RAX: RBX: 8881d2810120 RCX: RDX: RSI: 8128d3fd RDI: ed103b279e16 RBP: 8881d2810240 R08: 8881d92a3000 R09: fbfff11f45af R10: fbfff11f45ae R11: 88fa2d77 R12: 87779de0 R13: 8881d281 R14: 8881d281012c R15: 8881d2c84400 __list_add include/linux/list.h:60 [inline] list_add_tail include/linux/list.h:93 [inline] em28xx_init_extension+0x44/0x1f0 drivers/media/usb/em28xx/em28xx-core.c:1125 em28xx_init_dev.isra.0+0xa7b/0x15d8 drivers/media/usb/em28xx/em28xx-cards.c:3520 em28xx_usb_probe.cold+0xcac/0x2516 drivers/media/usb/em28xx/em28xx-cards.c:3869 usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 really_probe+0x281/0x6d0 drivers/base/dd.c:548 driver_probe_device+0x104/0x210 drivers/base/dd.c:721 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828 bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:430 __device_attach+0x217/0x360 drivers/base/dd.c:894 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:490 device_add+0xae6/0x16f0 drivers/base/core.c:2201 usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023 generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210 usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266 really_probe+0x281/0x6d0 drivers/base/dd.c:548 driver_probe_device+0x104/0x210 drivers/base/dd.c:721 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828 bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:430 __device_attach+0x217/0x360 drivers/base/dd.c:894 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:490 device_add+0xae6/0x16f0 drivers/base/core.c:2201 usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536 hub_port_connect drivers/usb/core/hub.c:5098 [inline] hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] port_event drivers/usb/core/hub.c:5359 [inline] hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441 process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 process_scheduled_works kernel/workqueue.c:2331 [inline] worker_thread+0x7ab/0xe20 kernel/workqueue.c:2417 kthread+0x318/0x420 kernel/kthread.c:255 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 Kernel Offset: disabled Rebooting in 86400 seconds.. --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#status for how to communicate with syzbot. syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches
Re: [PATCH] mm/page_alloc: fix a crash in free_pages_prepare()
On Fri, 27 Sep 2019 15:47:03 -0400 Qian Cai wrote: > On architectures like s390, arch_free_page() could mark the page unused > (set_page_unused()) and any access later would trigger a kernel panic. > Fix it by moving arch_free_page() after all possible accessing calls. > > Hardware name: IBM 2964 N96 400 (z/VM 6.4.0) > Krnl PSW : 0404e0018000 26c2b96e > (__free_pages_ok+0x34e/0x5d8) > R:0 T:1 IO:0 EX:0 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3 > Krnl GPRS: 88d43af7 00484000 007c > 000f > 03d080012100 03d080013fc0 > 0010 > 275cca48 0100 0008 > 03d08001 > 01d0 03d0 26c2b78a > 2717fdb0 > Krnl Code: 26c2b95c: ec1100b30659 risbgn %r1,%r1,0,179,6 > 26c2b962: e3201436 pfd 2,1024(%r1) >#26c2b968: d7ff10001000 xc 0(256,%r1),0(%r1) >>26c2b96e: 41101100 la %r1,256(%r1) > 26c2b972: a737fff8 brctg %r3,26c2b962 > 26c2b976: d7ff10001000 xc 0(256,%r1),0(%r1) > 26c2b97c: e3100344 lg %r1,832 > 26c2b982: ebff1430016a asi 5168(%r1),-1 > Call Trace: > __free_pages_ok+0x16a/0x5d8) > memblock_free_all+0x206/0x290 > mem_init+0x58/0x120 > start_kernel+0x2b0/0x570 > startup_continue+0x6a/0xc0 > INFO: lockdep is turned off. > Last Breaking-Event-Address: > __free_pages_ok+0x372/0x5d8 > Kernel panic - not syncing: Fatal exception: panic_on_oops > 00: HCPGIR450W CP entered; disabled wait PSW 00020001 8000 > 26A2379C > > ... > > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -1175,11 +1175,11 @@ static __always_inline bool free_pages_prepare(struct > page *page, > debug_check_no_obj_freed(page_address(page), > PAGE_SIZE << order); > } > - arch_free_page(page, order); > if (want_init_on_free()) > kernel_init_free_pages(page, 1 << order); > > kernel_poison_pages(page, 1 << order, 0); > + arch_free_page(page, order); > if (debug_pagealloc_enabled()) > kernel_map_pages(page, 1 << order, 0); This is all fairly mature code, isn't it? What happened to make this problem pop up now? IOW, is a -stable backport needed?