Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM: dma-mapping: preallocate DMA-debug hash tables in pure_initcall
From: Marek Szyprowski @ 2016-11-15 10:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <3289056.5bg7xtivbS@wuerfel>



On 2016-11-15 11:31, Arnd Bergmann wrote:
> On Tuesday, November 15, 2016 11:04:55 AM CET Marek Szyprowski wrote:
>> fs_initcall is definitely too late to initialize DMA-debug hash tables,
>> because some drivers might get probed and use DMA mapping framework
>> already in core_initcall. Late initialization of DMA-debug results in
>> false warning about accessing memory, that was not allocated, like this
>> one:
> To ask the obvious question: what driver does DMA allocations in a
> core_initcall, and have you tried to change that first?

See the attached stack trace.

Exynos IOMMU driver does that in its domain_alloc implementation (to 
allocate
first level of PTE) and I see no easy way to avoid that, as iommu 
domains are
allocated very early (as well as the whole IOMMU initialization and 
attaching
to the devices). Exynos IOMMU driver has to use DMA-mapping API for PTE
management, because the IOMMU controllers are not coherent with system 
CPU and
there is no other way to ensure proper CPU cache management.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

^ permalink raw reply

* [GIT PULL] STi DT fix for v4.9-rcs
From: Patrice Chotard @ 2016-11-15 10:38 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Arnd, Olof, Kevin

Please consider this set for inclusion into the v4.9-rc.

The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:

  Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/pchotard/sti.git tags/sti-dt-for-v4.9-rc

for you to fetch changes up to 5bf7b6e86f29f064979d7b3e6dd21c5dd1feb855:

  ARM: dts: STiH410-b2260: Fix typo in spi0 chipselect definition (2016-11-15 11:29:25 +0100)

----------------------------------------------------------------

STi DT fix:

Fix typo cs-gpio to cs-gpios

----------------------------------------------------------------
Loic Pallardy (1):
      ARM: dts: STiH410-b2260: Fix typo in spi0 chipselect definition

 arch/arm/boot/dts/stih410-b2260.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

^ permalink raw reply

* [PATCH v2 04/10] clk: rockchip: add clock controller for rk1108
From: Heiko Stuebner @ 2016-11-15 10:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479125262-24294-1-git-send-email-andy.yan@rock-chips.com>

Am Montag, 14. November 2016, 20:07:42 CET schrieb Andy Yan:
> From: Shawn Lin <shawn.lin@rock-chips.com>
> 
> Add the clock tree definition and driver for rk1108 SoC.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> Tested-by: Jacob Chen <jacob2.chen@rock-chips.com>
> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
> ---
> 
> Changes in v2:
> - fix some CodingStyle issues
> 
>  drivers/clk/rockchip/Makefile     |   1 +
>  drivers/clk/rockchip/clk-rk1108.c | 451
> ++++++++++++++++++++++++++++++++++++++ drivers/clk/rockchip/clk.h        | 
> 14 ++
>  3 files changed, 466 insertions(+)
>  create mode 100644 drivers/clk/rockchip/clk-rk1108.c
> 
> diff --git a/drivers/clk/rockchip/Makefile b/drivers/clk/rockchip/Makefile
> index b5f2c8e..16e098c 100644
> --- a/drivers/clk/rockchip/Makefile
> +++ b/drivers/clk/rockchip/Makefile
> @@ -11,6 +11,7 @@ obj-y	+= clk-mmc-phase.o
>  obj-y	+= clk-ddr.o
>  obj-$(CONFIG_RESET_CONTROLLER)	+= softrst.o
> 
> +obj-y	+= clk-rk1108.o
>  obj-y	+= clk-rk3036.o
>  obj-y	+= clk-rk3188.o
>  obj-y	+= clk-rk3228.o
> diff --git a/drivers/clk/rockchip/clk-rk1108.c
> b/drivers/clk/rockchip/clk-rk1108.c new file mode 100644
> index 0000000..e3a4f74
> --- /dev/null
> +++ b/drivers/clk/rockchip/clk-rk1108.c
> @@ -0,0 +1,451 @@
> +/*
> + * Copyright (c) 2016 Rockchip Electronics Co. Ltd.
> + * Author: Shawn Lin <shawn.lin@rock-chips.com>
> + *         Andy Yan <andy.yan@rock-chips.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/syscore_ops.h>
> +#include <dt-bindings/clock/rk1108-cru.h>
> +#include "clk.h"
> +
> +#define RK1108_GRF_SOC_STATUS0	0x480
> +
> +enum rk1108_plls {
> +	apll, dpll, gpll,
> +};
> +
> +static struct rockchip_pll_rate_table rk1108_pll_rates[] = {
> +	/* _mhz, _refdiv, _fbdiv, _postdiv1, _postdiv2, _dsmpd, _frac */
> +	RK3036_PLL_RATE(1608000000, 1, 67, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1584000000, 1, 66, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1560000000, 1, 65, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1536000000, 1, 64, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1512000000, 1, 63, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1488000000, 1, 62, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1464000000, 1, 61, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1440000000, 1, 60, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1416000000, 1, 59, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1392000000, 1, 58, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1368000000, 1, 57, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1344000000, 1, 56, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1320000000, 1, 55, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1296000000, 1, 54, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1272000000, 1, 53, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1248000000, 1, 52, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1200000000, 1, 50, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1188000000, 2, 99, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1104000000, 1, 46, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1100000000, 12, 550, 1, 1, 1, 0),
> +	RK3036_PLL_RATE(1008000000, 1, 84, 2, 1, 1, 0),
> +	RK3036_PLL_RATE(1000000000, 6, 500, 2, 1, 1, 0),
> +	RK3036_PLL_RATE( 984000000, 1, 82, 2, 1, 1, 0),
> +	RK3036_PLL_RATE( 960000000, 1, 80, 2, 1, 1, 0),
> +	RK3036_PLL_RATE( 936000000, 1, 78, 2, 1, 1, 0),
> +	RK3036_PLL_RATE( 912000000, 1, 76, 2, 1, 1, 0),
> +	RK3036_PLL_RATE( 900000000, 4, 300, 2, 1, 1, 0),
> +	RK3036_PLL_RATE( 888000000, 1, 74, 2, 1, 1, 0),
> +	RK3036_PLL_RATE( 864000000, 1, 72, 2, 1, 1, 0),
> +	RK3036_PLL_RATE( 840000000, 1, 70, 2, 1, 1, 0),
> +	RK3036_PLL_RATE( 816000000, 1, 68, 2, 1, 1, 0),
> +	RK3036_PLL_RATE( 800000000, 6, 400, 2, 1, 1, 0),
> +	RK3036_PLL_RATE( 700000000, 6, 350, 2, 1, 1, 0),
> +	RK3036_PLL_RATE( 696000000, 1, 58, 2, 1, 1, 0),
> +	RK3036_PLL_RATE( 600000000, 1, 75, 3, 1, 1, 0),
> +	RK3036_PLL_RATE( 594000000, 2, 99, 2, 1, 1, 0),
> +	RK3036_PLL_RATE( 504000000, 1, 63, 3, 1, 1, 0),
> +	RK3036_PLL_RATE( 500000000, 6, 250, 2, 1, 1, 0),
> +	RK3036_PLL_RATE( 408000000, 1, 68, 2, 2, 1, 0),
> +	RK3036_PLL_RATE( 312000000, 1, 52, 2, 2, 1, 0),
> +	RK3036_PLL_RATE( 216000000, 1, 72, 4, 2, 1, 0),
> +	RK3036_PLL_RATE(  96000000, 1, 64, 4, 4, 1, 0),
> +	{ /* sentinel */ },
> +};
> +
> +#define RK1108_DIV_CORE_MASK		0xf
> +#define RK1108_DIV_CORE_SHIFT		4
> +
> +#define RK1108_CLKSEL0(_core_peri_div)					\
> +	{									\
> +		.reg = RK1108_CLKSEL_CON(1),					\
> +		.val = HIWORD_UPDATE(_core_peri_div, RK1108_DIV_CORE_MASK,	\
> +				RK1108_DIV_CORE_SHIFT)				\
> +	}
> +
> +#define RK1108_CPUCLK_RATE(_prate, _core_peri_div)			\
> +	{								\
> +		.prate = _prate,					\
> +		.divs = {						\
> +			RK1108_CLKSEL0(_core_peri_div),		\
> +		},							\
> +	}
> +
> +static struct rockchip_cpuclk_rate_table rk1108_cpuclk_rates[] __initdata =
> { +	RK1108_CPUCLK_RATE(816000000, 4),
> +	RK1108_CPUCLK_RATE(600000000, 4),
> +	RK1108_CPUCLK_RATE(312000000, 4),
> +};
> +
> +static const struct rockchip_cpuclk_reg_data rk1108_cpuclk_data = {
> +	.core_reg = RK1108_CLKSEL_CON(0),
> +	.div_core_shift = 0,
> +	.div_core_mask = 0x1f,
> +	.mux_core_alt = 1,
> +	.mux_core_main = 0,
> +	.mux_core_shift = 8,
> +	.mux_core_mask = 0x1,
> +};
> +
> +PNAME(mux_pll_p)		= { "xin24m", "xin24m"};
> +PNAME(mux_ddrphy_p)		= { "dpll_ddr", "gpll_ddr", "apll_ddr" };
> +PNAME(mux_armclk_p)		= { "apll_core", "gpll_core", "dpll_core" };
> +PNAME(mux_pmu_1f)		= { "xin24m", "pmu_24m"};
> +PNAME(mux_usb480m_phy_p)	= { "usb480m_phy0", "usb480m_phy1" };
> +PNAME(mux_usb480m_p)		= { "usb480m_phy", "xin24m" };
> +PNAME(mux_hdmiphy_p)		= { "hdmiphy_phy", "pclk_top_pre", "xin24m" };

pmu_1f, usbphy and hdmiphy do not seem to be used in this driver, while they 
are specified in the clock documentation.

Also there is a discrepancy between your pmu_24m and pmu_24m_ena below I 
think.


The rest looks sane but I didn't check every register offset :-) .


Heiko

^ permalink raw reply

* [PATCH v5 2/3] vcodec: mediatek: Add Mediatek JPEG Decoder Driver
From: Rick Chang @ 2016-11-15 10:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <c0c2be8e-51ff-843a-6f3d-1fa0170cb209@xs4all.nl>

Hi Hans,

Thank you for your review. I will fix those problems in patch v6.

On Fri, 2016-11-11 at 16:10 +0100, Hans Verkuil wrote:
> A quick review:
> 
> On 11/08/2016 07:34 AM, Rick Chang wrote:
> > Add v4l2 driver for Mediatek JPEG Decoder
> > 
> > Signed-off-by: Rick Chang <rick.chang@mediatek.com>
> > Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
> > ---
> >  drivers/media/platform/Kconfig                   |   15 +
> >  drivers/media/platform/Makefile                  |    2 +
> >  drivers/media/platform/mtk-jpeg/Makefile         |    2 +
> >  drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c  | 1275 ++++++++++++++++++++++
> >  drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h  |  141 +++
> >  drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.c    |  417 +++++++
> >  drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.h    |   91 ++
> >  drivers/media/platform/mtk-jpeg/mtk_jpeg_parse.c |  160 +++
> >  drivers/media/platform/mtk-jpeg/mtk_jpeg_parse.h |   25 +
> >  drivers/media/platform/mtk-jpeg/mtk_jpeg_reg.h   |   58 +
> >  10 files changed, 2186 insertions(+)
> >  create mode 100644 drivers/media/platform/mtk-jpeg/Makefile
> >  create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> >  create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> >  create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.c
> >  create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.h
> >  create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_parse.c
> >  create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_parse.h
> >  create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_reg.h
> > 
> > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> > index 754edbf1..96c9887 100644
> > --- a/drivers/media/platform/Kconfig
> > +++ b/drivers/media/platform/Kconfig
> > @@ -162,6 +162,21 @@ config VIDEO_CODA
> >  	   Coda is a range of video codec IPs that supports
> >  	   H.264, MPEG-4, and other video formats.
> >  
> > +config VIDEO_MEDIATEK_JPEG
> > +	tristate "Mediatek JPEG Codec driver"
> > +	depends on MTK_IOMMU_V1 || COMPILE_TEST
> > +	depends on VIDEO_DEV && VIDEO_V4L2
> > +	depends on ARCH_MEDIATEK || COMPILE_TEST
> > +	depends on HAS_DMA
> > +	select VIDEOBUF2_DMA_CONTIG
> > +	select V4L2_MEM2MEM_DEV
> > +	---help---
> > +	  Mediatek jpeg codec driver provides HW capability to decode
> > +	  JPEG format
> > +
> > +	  To compile this driver as a module, choose M here: the
> > +	  module will be called mtk-jpeg
> > +
> >  config VIDEO_MEDIATEK_VPU
> >  	tristate "Mediatek Video Processor Unit"
> >  	depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
> > diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> > index f842933..cf701e3 100644
> > --- a/drivers/media/platform/Makefile
> > +++ b/drivers/media/platform/Makefile
> > @@ -68,3 +68,5 @@ obj-$(CONFIG_VIDEO_MEDIATEK_VPU)	+= mtk-vpu/
> >  obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC)	+= mtk-vcodec/
> >  
> >  obj-$(CONFIG_VIDEO_MEDIATEK_MDP)	+= mtk-mdp/
> > +
> > +obj-$(CONFIG_VIDEO_MEDIATEK_JPEG)	+= mtk-jpeg/
> > diff --git a/drivers/media/platform/mtk-jpeg/Makefile b/drivers/media/platform/mtk-jpeg/Makefile
> > new file mode 100644
> > index 0000000..b2e6069
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-jpeg/Makefile
> > @@ -0,0 +1,2 @@
> > +mtk_jpeg-objs := mtk_jpeg_core.o mtk_jpeg_hw.o mtk_jpeg_parse.o
> > +obj-$(CONFIG_VIDEO_MEDIATEK_JPEG) += mtk_jpeg.o
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > new file mode 100644
> > index 0000000..33ddf79
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -0,0 +1,1275 @@
> > +/*
> > + * Copyright (c) 2016 MediaTek Inc.
> > + * Author: Ming Hsiu Tsai <minghsiu.tsai@mediatek.com>
> > + *         Rick Chang <rick.chang@mediatek.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/err.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/spinlock.h>
> > +#include <media/v4l2-event.h>
> > +#include <media/v4l2-mem2mem.h>
> > +#include <media/v4l2-ioctl.h>
> > +#include <media/videobuf2-core.h>
> > +#include <media/videobuf2-dma-contig.h>
> > +#include <soc/mediatek/smi.h>
> > +#include <asm/dma-iommu.h>
> > +
> > +#include "mtk_jpeg_hw.h"
> > +#include "mtk_jpeg_core.h"
> > +#include "mtk_jpeg_parse.h"
> > +
> > +static struct mtk_jpeg_fmt mtk_jpeg_formats[] = {
> > +	{
> > +		.name		= "JPEG JFIF",
> > +		.fourcc		= V4L2_PIX_FMT_JPEG,
> > +		.colplanes	= 1,
> > +		.flags		= MTK_JPEG_FMT_FLAG_DEC_OUTPUT,
> > +	},
> > +	{
> > +		.name		= "YUV 4:2:0 non-contiguous 3-planar, Y/Cb/Cr",
> > +		.fourcc		= V4L2_PIX_FMT_YUV420M,
> > +		.h_sample	= {4, 2, 2},
> > +		.v_sample	= {4, 2, 2},
> > +		.colplanes	= 3,
> > +		.h_align	= 5,
> > +		.v_align	= 4,
> > +		.flags		= MTK_JPEG_FMT_FLAG_DEC_CAPTURE,
> > +	},
> > +	{
> > +		.name		= "YUV 4:2:2 non-contiguous 3-planar, Y/Cb/Cr",
> > +		.fourcc		= V4L2_PIX_FMT_YUV422M,
> > +		.h_sample	= {4, 2, 2},
> > +		.v_sample	= {4, 4, 4},
> > +		.colplanes	= 3,
> > +		.h_align	= 5,
> > +		.v_align	= 3,
> > +		.flags		= MTK_JPEG_FMT_FLAG_DEC_CAPTURE,
> 
> You probably don't need the name since it is filled in by the v4l2 core
> (v4l2-ioctls.c).

Ok.

> > +	},
> > +};
> > +
> > +#define MTK_JPEG_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_formats)
> > +
> > +enum {
> > +	MTK_JPEG_BUF_FLAGS_INIT			= 0,
> > +	MTK_JPEG_BUF_FLAGS_LAST_FRAME		= 1,
> > +};
> > +
> > +struct mtk_jpeg_src_buf {
> > +	struct vb2_v4l2_buffer b;
> > +	struct list_head list;
> > +	int flags;
> > +	struct mtk_jpeg_dec_param dec_param;
> > +};
> > +
> > +static int debug;
> > +module_param(debug, int, 0644);
> > +
> > +static inline struct mtk_jpeg_ctx *mtk_jpeg_fh_to_ctx(struct v4l2_fh *fh)
> > +{
> > +	return container_of(fh, struct mtk_jpeg_ctx, fh);
> > +}
> > +
> > +static inline struct mtk_jpeg_src_buf *mtk_jpeg_vb2_to_srcbuf(
> > +							struct vb2_buffer *vb)
> > +{
> > +	return container_of(to_vb2_v4l2_buffer(vb), struct mtk_jpeg_src_buf, b);
> > +}
> > +
> > +static int mtk_jpeg_querycap(struct file *file, void *priv,
> > +			     struct v4l2_capability *cap)
> > +{
> > +	struct mtk_jpeg_dev *jpeg = video_drvdata(file);
> > +
> > +	strlcpy(cap->driver, MTK_JPEG_NAME " decoder", sizeof(cap->driver));
> > +	strlcpy(cap->card, MTK_JPEG_NAME " decoder", sizeof(cap->card));
> > +	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> > +		 dev_name(jpeg->dev));
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtk_jpeg_enum_fmt(struct mtk_jpeg_fmt *mtk_jpeg_formats, int n,
> > +			     struct v4l2_fmtdesc *f, u32 type)
> > +{
> > +	int i, num = 0;
> > +
> > +	for (i = 0; i < n; ++i) {
> > +		if (mtk_jpeg_formats[i].flags & type) {
> > +			if (num == f->index)
> > +				break;
> > +			++num;
> > +		}
> > +	}
> > +
> > +	if (i >= n)
> > +		return -EINVAL;
> > +
> > +	f->pixelformat = mtk_jpeg_formats[i].fourcc;
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtk_jpeg_enum_fmt_vid_cap(struct file *file, void *priv,
> > +				     struct v4l2_fmtdesc *f)
> > +{
> > +	return mtk_jpeg_enum_fmt(mtk_jpeg_formats, MTK_JPEG_NUM_FORMATS, f,
> > +				 MTK_JPEG_FMT_FLAG_DEC_CAPTURE);
> > +}
> > +
> > +static int mtk_jpeg_enum_fmt_vid_out(struct file *file, void *priv,
> > +				     struct v4l2_fmtdesc *f)
> > +{
> > +	return mtk_jpeg_enum_fmt(mtk_jpeg_formats, MTK_JPEG_NUM_FORMATS, f,
> > +				 MTK_JPEG_FMT_FLAG_DEC_OUTPUT);
> > +}
> > +
> > +static struct mtk_jpeg_q_data *mtk_jpeg_get_q_data(struct mtk_jpeg_ctx *ctx,
> > +						   enum v4l2_buf_type type)
> > +{
> > +	if (V4L2_TYPE_IS_OUTPUT(type))
> > +		return &ctx->out_q;
> > +	else
> 
> No need for 'else'.

Ok.

> > +		return &ctx->cap_q;
> > +}
> > +
> > +static struct mtk_jpeg_fmt *mtk_jpeg_find_format(struct mtk_jpeg_ctx *ctx,
> > +						 u32 pixelformat,
> > +						 unsigned int fmt_type)
> > +{
> > +	unsigned int k, fmt_flag;
> > +
> > +	fmt_flag = (fmt_type == MTK_JPEG_FMT_TYPE_OUTPUT) ?
> > +		   MTK_JPEG_FMT_FLAG_DEC_OUTPUT :
> > +		   MTK_JPEG_FMT_FLAG_DEC_CAPTURE;
> > +
> > +	for (k = 0; k < MTK_JPEG_NUM_FORMATS; k++) {
> > +		struct mtk_jpeg_fmt *fmt = &mtk_jpeg_formats[k];
> > +
> > +		if (fmt->fourcc == pixelformat && fmt->flags & fmt_flag)
> > +			return fmt;
> > +	}
> > +
> > +	return NULL;
> > +}
> > +
> > +static void mtk_jpeg_bound_align_image(u32 *w, unsigned int wmin,
> > +				       unsigned int wmax, unsigned int walign,
> > +				       u32 *h, unsigned int hmin,
> > +				       unsigned int hmax, unsigned int halign)
> > +{
> > +	int width, height, w_step, h_step;
> > +
> > +	width = *w;
> > +	height = *h;
> > +	w_step = 1 << walign;
> > +	h_step = 1 << halign;
> > +
> > +	v4l_bound_align_image(w, wmin, wmax, walign, h, hmin, hmax, halign, 0);
> > +	if (*w < width && (*w + w_step) <= wmax)
> > +		*w += w_step;
> > +	if (*h < height && (*h + h_step) <= hmax)
> > +		*h += h_step;
> > +}
> > +
> > +static void mtk_jpeg_adjust_fmt_mplane(struct mtk_jpeg_ctx *ctx,
> > +				       struct v4l2_format *f)
> > +{
> > +	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > +	struct mtk_jpeg_q_data *q_data;
> > +	int i;
> > +
> > +	q_data = mtk_jpeg_get_q_data(ctx, f->type);
> > +
> > +	pix_mp->width = q_data->w;
> > +	pix_mp->height = q_data->h;
> > +	pix_mp->pixelformat = q_data->fmt->fourcc;
> > +	pix_mp->num_planes = q_data->fmt->colplanes;
> > +
> > +	for (i = 0; i < pix_mp->num_planes; i++) {
> > +		pix_mp->plane_fmt[i].bytesperline = q_data->bytesperline[i];
> > +		pix_mp->plane_fmt[i].sizeimage = q_data->sizeimage[i];
> > +	}
> > +}
> > +
> > +static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> > +				   struct mtk_jpeg_fmt *fmt,
> > +				   struct mtk_jpeg_ctx *ctx, int q_type)
> > +{
> > +	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > +	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > +	int i;
> > +
> > +	memset(pix_mp->reserved, 0, sizeof(pix_mp->reserved));
> > +	pix_mp->field = V4L2_FIELD_NONE;
> > +
> > +	if (ctx->state != MTK_JPEG_INIT) {
> > +		mtk_jpeg_adjust_fmt_mplane(ctx, f);
> > +		goto end;
> > +	}
> > +
> > +	pix_mp->num_planes = fmt->colplanes;
> > +	pix_mp->pixelformat = fmt->fourcc;
> > +
> > +	if (q_type == MTK_JPEG_FMT_TYPE_OUTPUT) {
> > +		struct v4l2_plane_pix_format *pfmt = &pix_mp->plane_fmt[0];
> > +
> > +		mtk_jpeg_bound_align_image(&pix_mp->width, MTK_JPEG_MIN_WIDTH,
> > +					   MTK_JPEG_MAX_WIDTH, 0,
> > +					   &pix_mp->height, MTK_JPEG_MIN_HEIGHT,
> > +					   MTK_JPEG_MAX_HEIGHT, 0);
> > +
> > +		memset(pfmt->reserved, 0, sizeof(pfmt->reserved));
> > +		pfmt->bytesperline = 0;
> > +		/* Source size must be aligned to 128 */
> > +		pfmt->sizeimage = mtk_jpeg_align(pfmt->sizeimage, 128);
> > +		if (pfmt->sizeimage == 0)
> > +			pfmt->sizeimage = MTK_JPEG_DEFAULT_SIZEIMAGE;
> > +		goto end;
> > +	}
> > +
> > +	/* type is MTK_JPEG_FMT_TYPE_CAPTURE */
> > +	mtk_jpeg_bound_align_image(&pix_mp->width, MTK_JPEG_MIN_WIDTH,
> > +				   MTK_JPEG_MAX_WIDTH, fmt->h_align,
> > +				   &pix_mp->height, MTK_JPEG_MIN_HEIGHT,
> > +				   MTK_JPEG_MAX_HEIGHT, fmt->v_align);
> > +
> > +	for (i = 0; i < fmt->colplanes; i++) {
> > +		struct v4l2_plane_pix_format *pfmt = &pix_mp->plane_fmt[i];
> > +		u32 stride = pix_mp->width * fmt->h_sample[i] / 4;
> > +		u32 h = pix_mp->height * fmt->v_sample[i] / 4;
> > +
> > +		memset(pfmt->reserved, 0, sizeof(pfmt->reserved));
> > +		pfmt->bytesperline = stride;
> > +		pfmt->sizeimage = stride * h;
> > +	}
> > +end:
> > +	v4l2_dbg(2, debug, &jpeg->v4l2_dev, "wxh:%ux%u\n",
> > +		 pix_mp->width, pix_mp->height);
> > +	for (i = 0; i < pix_mp->num_planes; i++) {
> > +		v4l2_dbg(2, debug, &jpeg->v4l2_dev,
> > +			 "plane[%d] bpl=%u, size=%u\n",
> > +			 i,
> > +			 pix_mp->plane_fmt[i].bytesperline,
> > +			 pix_mp->plane_fmt[i].sizeimage);
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int mtk_jpeg_g_fmt_vid_mplane(struct file *file, void *priv,
> > +				     struct v4l2_format *f)
> > +{
> > +	struct vb2_queue *vq;
> > +	struct mtk_jpeg_q_data *q_data = NULL;
> > +	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > +	struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> > +	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > +	int i;
> > +
> > +	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> > +	if (!vq)
> > +		return -EINVAL;
> > +
> > +	q_data = mtk_jpeg_get_q_data(ctx, f->type);
> > +
> > +	memset(pix_mp->reserved, 0, sizeof(pix_mp->reserved));
> > +	pix_mp->width = q_data->w;
> > +	pix_mp->height = q_data->h;
> > +	pix_mp->field = V4L2_FIELD_NONE;
> > +	pix_mp->pixelformat = q_data->fmt->fourcc;
> > +	pix_mp->num_planes = q_data->fmt->colplanes;
> > +	pix_mp->colorspace = ctx->colorspace;
> > +	pix_mp->ycbcr_enc = ctx->ycbcr_enc;
> > +	pix_mp->xfer_func = ctx->xfer_func;
> > +	pix_mp->quantization = ctx->quantization;
> > +
> > +	v4l2_dbg(1, debug, &jpeg->v4l2_dev, "(%d) g_fmt:%s wxh:%ux%u\n",
> > +		 f->type, q_data->fmt->name, pix_mp->width, pix_mp->height);
> > +
> > +	for (i = 0; i < pix_mp->num_planes; i++) {
> > +		struct v4l2_plane_pix_format *pfmt = &pix_mp->plane_fmt[i];
> > +
> > +		pfmt->bytesperline = q_data->bytesperline[i];
> > +		pfmt->sizeimage = q_data->sizeimage[i];
> > +		memset(pfmt->reserved, 0, sizeof(pfmt->reserved));
> > +
> > +		v4l2_dbg(1, debug, &jpeg->v4l2_dev,
> > +			 "plane[%d] bpl=%u, size=%u\n",
> > +			 i,
> > +			 pfmt->bytesperline,
> > +			 pfmt->sizeimage);
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int mtk_jpeg_try_fmt_vid_cap_mplane(struct file *file, void *priv,
> > +					   struct v4l2_format *f)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> > +	struct mtk_jpeg_fmt *fmt;
> > +
> > +	fmt = mtk_jpeg_find_format(ctx, f->fmt.pix_mp.pixelformat,
> > +				   MTK_JPEG_FMT_TYPE_CAPTURE);
> > +	if (!fmt)
> > +		fmt = ctx->cap_q.fmt;
> > +
> > +	v4l2_dbg(2, debug, &ctx->jpeg->v4l2_dev, "(%d) try_fmt:%s\n",
> > +		 f->type, fmt->name);
> > +
> > +	return mtk_jpeg_try_fmt_mplane(f, fmt, ctx, MTK_JPEG_FMT_TYPE_CAPTURE);
> > +}
> > +
> > +static int mtk_jpeg_try_fmt_vid_out_mplane(struct file *file, void *priv,
> > +					   struct v4l2_format *f)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> > +	struct mtk_jpeg_fmt *fmt;
> > +
> > +	fmt = mtk_jpeg_find_format(ctx, f->fmt.pix_mp.pixelformat,
> > +				   MTK_JPEG_FMT_TYPE_OUTPUT);
> > +	if (!fmt)
> > +		fmt = ctx->out_q.fmt;
> > +
> > +	v4l2_dbg(2, debug, &ctx->jpeg->v4l2_dev, "(%d) try_fmt:%s\n",
> > +		 f->type, fmt->name);
> > +
> > +	return mtk_jpeg_try_fmt_mplane(f, fmt, ctx, MTK_JPEG_FMT_TYPE_OUTPUT);
> > +}
> > +
> > +static int mtk_jpeg_s_fmt_mplane(struct mtk_jpeg_ctx *ctx,
> > +				 struct v4l2_format *f)
> > +{
> > +	struct vb2_queue *vq;
> > +	struct mtk_jpeg_q_data *q_data = NULL;
> > +	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > +	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > +	unsigned int f_type;
> > +	int i;
> > +
> > +	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> > +	if (!vq)
> > +		return -EINVAL;
> > +
> > +	q_data = mtk_jpeg_get_q_data(ctx, f->type);
> > +
> > +	if (vb2_is_busy(vq)) {
> > +		v4l2_err(&jpeg->v4l2_dev, "queue busy\n");
> > +		return -EBUSY;
> > +	}
> > +
> > +	f_type = V4L2_TYPE_IS_OUTPUT(f->type) ?
> > +			 MTK_JPEG_FMT_TYPE_OUTPUT : MTK_JPEG_FMT_TYPE_CAPTURE;
> > +
> > +	q_data->fmt = mtk_jpeg_find_format(ctx, pix_mp->pixelformat, f_type);
> > +	q_data->w = pix_mp->width;
> > +	q_data->h = pix_mp->height;
> > +	ctx->colorspace = pix_mp->colorspace;
> > +	ctx->ycbcr_enc = pix_mp->ycbcr_enc;
> > +	ctx->xfer_func = pix_mp->xfer_func;
> > +	ctx->quantization = pix_mp->quantization;
> > +
> > +	v4l2_dbg(1, debug, &jpeg->v4l2_dev, "(%d) s_fmt:%s wxh:%ux%u\n",
> > +		 f->type, q_data->fmt->name, q_data->w, q_data->h);
> > +
> > +	for (i = 0; i < q_data->fmt->colplanes; i++) {
> > +		q_data->bytesperline[i] = pix_mp->plane_fmt[i].bytesperline;
> > +		q_data->sizeimage[i] = pix_mp->plane_fmt[i].sizeimage;
> > +
> > +		v4l2_dbg(1, debug, &jpeg->v4l2_dev,
> > +			 "plane[%d] bpl=%u, size=%u\n",
> > +			 i, q_data->bytesperline[i], q_data->sizeimage[i]);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtk_jpeg_s_fmt_vid_out_mplane(struct file *file, void *priv,
> > +					 struct v4l2_format *f)
> > +{
> > +	int ret;
> > +
> > +	ret = mtk_jpeg_try_fmt_vid_out_mplane(file, priv, f);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return mtk_jpeg_s_fmt_mplane(mtk_jpeg_fh_to_ctx(priv), f);
> > +}
> > +
> > +static int mtk_jpeg_s_fmt_vid_cap_mplane(struct file *file, void *priv,
> > +					 struct v4l2_format *f)
> > +{
> > +	int ret;
> > +
> > +	ret = mtk_jpeg_try_fmt_vid_cap_mplane(file, priv, f);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return mtk_jpeg_s_fmt_mplane(mtk_jpeg_fh_to_ctx(priv), f);
> > +}
> > +
> > +static void mtk_jpeg_queue_src_chg_event(struct mtk_jpeg_ctx *ctx)
> > +{
> > +	static const struct v4l2_event ev_src_ch = {
> > +		.type = V4L2_EVENT_SOURCE_CHANGE,
> > +		.u.src_change.changes =
> > +		V4L2_EVENT_SRC_CH_RESOLUTION,
> > +	};
> > +
> > +	v4l2_event_queue_fh(&ctx->fh, &ev_src_ch);
> > +}
> > +
> > +static int mtk_jpeg_subscribe_event(struct v4l2_fh *fh,
> > +				    const struct v4l2_event_subscription *sub)
> > +{
> > +	switch (sub->type) {
> > +	case V4L2_EVENT_SOURCE_CHANGE:
> > +		return v4l2_src_change_event_subscribe(fh, sub);
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
> > +static int mtk_jpeg_g_selection(struct file *file, void *priv,
> > +				struct v4l2_selection *s)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> > +
> > +	if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> > +		return -EINVAL;
> > +
> > +	switch (s->target) {
> > +	case V4L2_SEL_TGT_COMPOSE:
> > +	case V4L2_SEL_TGT_COMPOSE_DEFAULT:
> > +		s->r.width = ctx->out_q.w;
> > +		s->r.height = ctx->out_q.h;
> > +		s->r.left = 0;
> > +		s->r.top = 0;
> > +		break;
> > +	case V4L2_SEL_TGT_COMPOSE_BOUNDS:
> > +	case V4L2_SEL_TGT_COMPOSE_PADDED:
> > +		s->r.width = ctx->cap_q.w;
> > +		s->r.height = ctx->cap_q.h;
> > +		s->r.left = 0;
> > +		s->r.top = 0;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int mtk_jpeg_s_selection(struct file *file, void *priv,
> > +				struct v4l2_selection *s)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> > +
> > +	if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> > +		return -EINVAL;
> > +
> > +	switch (s->target) {
> > +	case V4L2_SEL_TGT_COMPOSE:
> > +		s->r.left = 0;
> > +		s->r.top = 0;
> > +		s->r.width = ctx->out_q.w;
> > +		s->r.height = ctx->out_q.h;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int mtk_jpeg_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
> > +{
> > +	struct v4l2_fh *fh = file->private_data;
> > +	struct vb2_queue *vq;
> > +	struct vb2_buffer *vb;
> > +	struct mtk_jpeg_src_buf *jpeg_src_buf;
> > +
> > +	if (buf->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > +		goto end;
> > +
> > +	vq = v4l2_m2m_get_vq(fh->m2m_ctx, buf->type);
> > +	vb = vq->bufs[buf->index];
> > +	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(vb);
> > +	jpeg_src_buf->flags = (buf->m.planes[0].bytesused == 0) ?
> > +		MTK_JPEG_BUF_FLAGS_LAST_FRAME : MTK_JPEG_BUF_FLAGS_INIT;
> > +end:
> > +	return v4l2_m2m_qbuf(file, fh->m2m_ctx, buf);
> > +}
> > +
> > +static const struct v4l2_ioctl_ops mtk_jpeg_ioctl_ops = {
> > +	.vidioc_querycap                = mtk_jpeg_querycap,
> > +	.vidioc_enum_fmt_vid_cap_mplane = mtk_jpeg_enum_fmt_vid_cap,
> > +	.vidioc_enum_fmt_vid_out_mplane = mtk_jpeg_enum_fmt_vid_out,
> > +	.vidioc_try_fmt_vid_cap_mplane	= mtk_jpeg_try_fmt_vid_cap_mplane,
> > +	.vidioc_try_fmt_vid_out_mplane	= mtk_jpeg_try_fmt_vid_out_mplane,
> > +	.vidioc_g_fmt_vid_cap_mplane    = mtk_jpeg_g_fmt_vid_mplane,
> > +	.vidioc_g_fmt_vid_out_mplane    = mtk_jpeg_g_fmt_vid_mplane,
> > +	.vidioc_s_fmt_vid_cap_mplane    = mtk_jpeg_s_fmt_vid_cap_mplane,
> > +	.vidioc_s_fmt_vid_out_mplane    = mtk_jpeg_s_fmt_vid_out_mplane,
> > +	.vidioc_qbuf                    = mtk_jpeg_qbuf,
> > +	.vidioc_subscribe_event         = mtk_jpeg_subscribe_event,
> > +	.vidioc_g_selection		= mtk_jpeg_g_selection,
> > +	.vidioc_s_selection		= mtk_jpeg_s_selection,
> > +
> > +	.vidioc_create_bufs		= v4l2_m2m_ioctl_create_bufs,
> > +	.vidioc_prepare_buf		= v4l2_m2m_ioctl_prepare_buf,
> > +	.vidioc_reqbufs                 = v4l2_m2m_ioctl_reqbufs,
> > +	.vidioc_querybuf                = v4l2_m2m_ioctl_querybuf,
> > +	.vidioc_dqbuf                   = v4l2_m2m_ioctl_dqbuf,
> > +	.vidioc_expbuf                  = v4l2_m2m_ioctl_expbuf,
> > +	.vidioc_streamon                = v4l2_m2m_ioctl_streamon,
> > +	.vidioc_streamoff               = v4l2_m2m_ioctl_streamoff,
> > +
> > +	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
> > +};
> > +
> > +static int mtk_jpeg_queue_setup(struct vb2_queue *q,
> > +				unsigned int *num_buffers,
> > +				unsigned int *num_planes,
> > +				unsigned int sizes[],
> > +				struct device *alloc_ctxs[])
> > +{
> > +	struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> > +	struct mtk_jpeg_q_data *q_data = NULL;
> > +	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > +	int i;
> > +
> > +	v4l2_dbg(1, debug, &jpeg->v4l2_dev, "(%d) buf_req count=%u\n",
> > +		 q->type, *num_buffers);
> > +
> > +	q_data = mtk_jpeg_get_q_data(ctx, q->type);
> > +	if (!q_data)
> > +		return -EINVAL;
> > +
> > +	*num_planes = q_data->fmt->colplanes;
> > +	for (i = 0; i < q_data->fmt->colplanes; i++) {
> > +		sizes[i] = q_data->sizeimage[i];
> > +		v4l2_dbg(1, debug, &jpeg->v4l2_dev, "sizeimage[%d]=%u\n",
> > +			 i, sizes[i]);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtk_jpeg_buf_prepare(struct vb2_buffer *vb)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> > +	struct mtk_jpeg_q_data *q_data = NULL;
> > +	int i;
> > +
> > +	q_data = mtk_jpeg_get_q_data(ctx, vb->vb2_queue->type);
> > +	if (!q_data)
> > +		return -EINVAL;
> > +
> > +	for (i = 0; i < q_data->fmt->colplanes; i++)
> > +		vb2_set_plane_payload(vb, i, q_data->sizeimage[i]);
> > +
> > +	return 0;
> > +}
> > +
> > +static bool mtk_jpeg_check_resolution_change(struct mtk_jpeg_ctx *ctx,
> > +					     struct mtk_jpeg_dec_param *param)
> > +{
> > +	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > +	struct mtk_jpeg_q_data *q_data;
> > +
> > +	q_data = &ctx->out_q;
> > +	if (q_data->w != param->pic_w || q_data->h != param->pic_h) {
> > +		v4l2_dbg(1, debug, &jpeg->v4l2_dev, "Picture size change\n");
> > +		return true;
> > +	}
> > +
> > +	q_data = &ctx->cap_q;
> > +	if (q_data->fmt != mtk_jpeg_find_format(ctx, param->dst_fourcc,
> > +						MTK_JPEG_FMT_TYPE_CAPTURE)) {
> > +		v4l2_dbg(1, debug, &jpeg->v4l2_dev, "format change\n");
> > +		return true;
> > +	}
> > +	return false;
> > +}
> > +
> > +static void mtk_jpeg_set_queue_data(struct mtk_jpeg_ctx *ctx,
> > +				    struct mtk_jpeg_dec_param *param)
> > +{
> > +	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > +	struct mtk_jpeg_q_data *q_data;
> > +	int i;
> > +
> > +	q_data = &ctx->out_q;
> > +	q_data->w = param->pic_w;
> > +	q_data->h = param->pic_h;
> > +
> > +	q_data = &ctx->cap_q;
> > +	q_data->w = param->dec_w;
> > +	q_data->h = param->dec_h;
> > +	q_data->fmt = mtk_jpeg_find_format(ctx,
> > +					   param->dst_fourcc,
> > +					   MTK_JPEG_FMT_TYPE_CAPTURE);
> > +
> > +	for (i = 0; i < q_data->fmt->colplanes; i++) {
> > +		q_data->bytesperline[i] = param->mem_stride[i];
> > +		q_data->sizeimage[i] = param->comp_size[i];
> > +	}
> > +
> > +	v4l2_dbg(1, debug, &jpeg->v4l2_dev,
> > +		 "set_parse cap:%s pic(%u, %u), buf(%u, %u)\n",
> > +		 q_data->fmt->name, param->pic_w, param->pic_h,
> > +		 param->dec_w, param->dec_h);
> > +}
> > +
> > +static void mtk_jpeg_buf_queue(struct vb2_buffer *vb)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> > +	struct mtk_jpeg_dec_param *param;
> > +	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > +	struct mtk_jpeg_src_buf *jpeg_src_buf;
> > +	bool header_valid;
> > +
> > +	v4l2_dbg(2, debug, &jpeg->v4l2_dev, "(%d) buf_q id=%d, vb=%p",
> > +		 vb->vb2_queue->type, vb->index, vb);
> > +
> > +	if (vb->vb2_queue->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > +		goto end;
> > +
> > +	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(vb);
> > +	param = &jpeg_src_buf->dec_param;
> > +	memset(param, 0, sizeof(*param));
> > +
> > +	if (jpeg_src_buf->flags & MTK_JPEG_BUF_FLAGS_LAST_FRAME) {
> > +		v4l2_dbg(1, debug, &jpeg->v4l2_dev, "Got eos");
> > +		goto end;
> > +	}
> > +	header_valid = mtk_jpeg_parse(param, (u8 *)vb2_plane_vaddr(vb, 0),
> > +				      vb2_get_plane_payload(vb, 0));
> > +	if (!header_valid) {
> > +		v4l2_err(&jpeg->v4l2_dev, "Header invalid.\n");
> > +		vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
> > +		return;
> > +	}
> > +
> > +	if (ctx->state == MTK_JPEG_INIT) {
> > +		mtk_jpeg_queue_src_chg_event(ctx);
> > +		mtk_jpeg_set_queue_data(ctx, param);
> > +		ctx->state = MTK_JPEG_RUNNING;
> > +	}
> > +end:
> > +	v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, to_vb2_v4l2_buffer(vb));
> > +}
> > +
> > +static void *mtk_jpeg_buf_remove(struct mtk_jpeg_ctx *ctx,
> > +				 enum v4l2_buf_type type)
> > +{
> > +	if (V4L2_TYPE_IS_OUTPUT(type))
> > +		return v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > +	else
> > +		return v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> > +}
> > +
> > +static int mtk_jpeg_start_streaming(struct vb2_queue *q, unsigned int count)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> > +	int ret = 0;
> > +
> > +	ret = pm_runtime_get_sync(ctx->jpeg->dev);
> > +
> 
> If start_streaming returns an error, then you must call
> v4l2_m2m_buf_done(to_vb2_v4l2_buffer(vb), VB2_BUF_STATE_QUEUED) for all
> buffers. Similar to what happens in stop_streaming, but with a different VB2_BUF_STATE.

Ok.

> > +	return ret > 0 ? 0 : ret;
> > +}
> > +
> > +static void mtk_jpeg_stop_streaming(struct vb2_queue *q)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> > +	struct vb2_buffer *vb;
> > +
> > +	/*
> > +	 * STREAMOFF is an acknowledgment for source change event.
> > +	 * Before STREAMOFF, we still have to return the old resolution and
> > +	 * subsampling. Update capture queue when the stream is off.
> > +	 */
> > +	if (ctx->state == MTK_JPEG_SOURCE_CHANGE &&
> > +	    !V4L2_TYPE_IS_OUTPUT(q->type)) {
> > +		struct mtk_jpeg_src_buf *src_buf;
> > +
> > +		vb = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > +		src_buf = mtk_jpeg_vb2_to_srcbuf(vb);
> > +		mtk_jpeg_set_queue_data(ctx, &src_buf->dec_param);
> > +		ctx->state = MTK_JPEG_RUNNING;
> > +	} else if (V4L2_TYPE_IS_OUTPUT(q->type)) {
> > +		ctx->state = MTK_JPEG_INIT;
> > +	}
> > +
> > +	vb = mtk_jpeg_buf_remove(ctx, q->type);
> > +	while (vb) {
> > +		v4l2_m2m_buf_done(to_vb2_v4l2_buffer(vb), VB2_BUF_STATE_ERROR);
> > +		vb = mtk_jpeg_buf_remove(ctx, q->type);
> > +	}
> > +
> > +	pm_runtime_put_sync(ctx->jpeg->dev);
> > +}
> > +
> > +static struct vb2_ops mtk_jpeg_qops = {
> > +	.queue_setup        = mtk_jpeg_queue_setup,
> > +	.buf_prepare        = mtk_jpeg_buf_prepare,
> > +	.buf_queue          = mtk_jpeg_buf_queue,
> > +	.wait_prepare       = vb2_ops_wait_prepare,
> > +	.wait_finish        = vb2_ops_wait_finish,
> > +	.start_streaming    = mtk_jpeg_start_streaming,
> > +	.stop_streaming     = mtk_jpeg_stop_streaming,
> > +};
> > +
> > +static void mtk_jpeg_set_dec_src(struct mtk_jpeg_ctx *ctx,
> > +				 struct vb2_buffer *src_buf,
> > +				 struct mtk_jpeg_bs *bs)
> > +{
> > +	bs->str_addr = vb2_dma_contig_plane_dma_addr(src_buf, 0);
> > +	bs->end_addr = bs->str_addr +
> > +			 mtk_jpeg_align(vb2_get_plane_payload(src_buf, 0), 16);
> > +	bs->size = mtk_jpeg_align(vb2_plane_size(src_buf, 0), 128);
> > +}
> > +
> > +static int mtk_jpeg_set_dec_dst(struct mtk_jpeg_ctx *ctx,
> > +				struct mtk_jpeg_dec_param *param,
> > +				struct vb2_buffer *dst_buf,
> > +				struct mtk_jpeg_fb *fb)
> > +{
> > +	int i;
> > +
> > +	if (param->comp_num != dst_buf->num_planes) {
> > +		dev_err(ctx->jpeg->dev, "plane number mismatch (%u != %u)\n",
> > +			param->comp_num, dst_buf->num_planes);
> > +		return -EINVAL;
> > +	}
> > +
> > +	for (i = 0; i < dst_buf->num_planes; i++) {
> > +		if (vb2_plane_size(dst_buf, i) < param->comp_size[i]) {
> > +			dev_err(ctx->jpeg->dev,
> > +				"buffer size is underflow (%lu < %u)\n",
> > +				vb2_plane_size(dst_buf, 0),
> > +				param->comp_size[i]);
> > +			return -EINVAL;
> > +		}
> > +		fb->plane_addr[i] = vb2_dma_contig_plane_dma_addr(dst_buf, i);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void mtk_jpeg_device_run(void *priv)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = priv;
> > +	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > +	struct vb2_buffer *src_buf, *dst_buf;
> > +	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> > +	unsigned long flags;
> > +	struct mtk_jpeg_src_buf *jpeg_src_buf;
> > +	struct mtk_jpeg_bs bs;
> > +	struct mtk_jpeg_fb fb;
> > +	int i;
> > +
> > +	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > +	dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> > +	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(src_buf);
> > +
> > +	if (jpeg_src_buf->flags & MTK_JPEG_BUF_FLAGS_LAST_FRAME) {
> > +		for (i = 0; i < dst_buf->num_planes; i++)
> > +			vb2_set_plane_payload(dst_buf, i, 0);
> > +		buf_state = VB2_BUF_STATE_DONE;
> > +		goto dec_end;
> > +	}
> > +
> > +	if (mtk_jpeg_check_resolution_change(ctx, &jpeg_src_buf->dec_param)) {
> > +		mtk_jpeg_queue_src_chg_event(ctx);
> > +		ctx->state = MTK_JPEG_SOURCE_CHANGE;
> > +		v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > +		return;
> > +	}
> > +
> > +	mtk_jpeg_set_dec_src(ctx, src_buf, &bs);
> > +	if (mtk_jpeg_set_dec_dst(ctx, &jpeg_src_buf->dec_param, dst_buf, &fb))
> > +		goto dec_end;
> > +
> > +	spin_lock_irqsave(&jpeg->hw_lock, flags);
> > +	mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> > +	mtk_jpeg_dec_set_config(jpeg->dec_reg_base,
> > +				&jpeg_src_buf->dec_param, &bs, &fb);
> > +
> > +	mtk_jpeg_dec_start(jpeg->dec_reg_base);
> > +	spin_unlock_irqrestore(&jpeg->hw_lock, flags);
> > +	return;
> > +
> > +dec_end:
> > +	v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > +	v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> > +	v4l2_m2m_buf_done(to_vb2_v4l2_buffer(src_buf), buf_state);
> > +	v4l2_m2m_buf_done(to_vb2_v4l2_buffer(dst_buf), buf_state);
> > +	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > +}
> > +
> > +static int mtk_jpeg_job_ready(void *priv)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = priv;
> > +
> > +	return (ctx->state == MTK_JPEG_RUNNING) ? 1 : 0;
> > +}
> > +
> > +static void mtk_jpeg_job_abort(void *priv)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = priv;
> > +	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > +	struct vb2_buffer *src_buf, *dst_buf;
> > +
> > +	src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > +	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> > +	v4l2_m2m_buf_done(to_vb2_v4l2_buffer(src_buf), VB2_BUF_STATE_ERROR);
> > +	v4l2_m2m_buf_done(to_vb2_v4l2_buffer(dst_buf), VB2_BUF_STATE_ERROR);
> > +	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > +}
> > +
> > +static struct v4l2_m2m_ops mtk_jpeg_m2m_ops = {
> > +	.device_run = mtk_jpeg_device_run,
> > +	.job_ready  = mtk_jpeg_job_ready,
> > +	.job_abort  = mtk_jpeg_job_abort,
> > +};
> > +
> > +static int mtk_jpeg_queue_init(void *priv, struct vb2_queue *src_vq,
> > +			       struct vb2_queue *dst_vq)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = priv;
> > +	int ret;
> > +
> > +	src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> > +	src_vq->io_modes = VB2_DMABUF | VB2_MMAP | VB2_USERPTR;
> 
> I would drop USERPTR, it really makes little sense for dma_contig.

Ok.

> > +	src_vq->drv_priv = ctx;
> > +	src_vq->buf_struct_size = sizeof(struct mtk_jpeg_src_buf);
> > +	src_vq->ops = &mtk_jpeg_qops;
> > +	src_vq->mem_ops = &vb2_dma_contig_memops;
> > +	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> > +	src_vq->lock = &ctx->jpeg->lock;
> > +	src_vq->dev = ctx->jpeg->dev;
> > +	ret = vb2_queue_init(src_vq);
> > +	if (ret)
> > +		return ret;
> > +
> > +	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> > +	dst_vq->io_modes = VB2_DMABUF | VB2_MMAP | VB2_USERPTR;
> 
> Ditto.

Ok.

> > +	dst_vq->drv_priv = ctx;
> > +	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> > +	dst_vq->ops = &mtk_jpeg_qops;
> > +	dst_vq->mem_ops = &vb2_dma_contig_memops;
> > +	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> > +	dst_vq->lock = &ctx->jpeg->lock;
> > +	dst_vq->dev = ctx->jpeg->dev;
> > +	ret = vb2_queue_init(dst_vq);
> > +
> > +	return ret;
> > +}
> > +
> > +static void mtk_jpeg_clk_on(struct mtk_jpeg_dev *jpeg)
> > +{
> > +	int ret;
> > +
> > +	ret = mtk_smi_larb_get(jpeg->larb);
> > +	if (ret)
> > +		dev_err(jpeg->dev, "mtk_smi_larb_get larbvdec fail %d\n", ret);
> > +	clk_prepare_enable(jpeg->clk_jdec_smi);
> > +	clk_prepare_enable(jpeg->clk_jdec);
> > +}
> > +
> > +static void mtk_jpeg_clk_off(struct mtk_jpeg_dev *jpeg)
> > +{
> > +	clk_disable_unprepare(jpeg->clk_jdec);
> > +	clk_disable_unprepare(jpeg->clk_jdec_smi);
> > +	mtk_smi_larb_put(jpeg->larb);
> > +}
> > +
> > +static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> > +{
> > +	struct mtk_jpeg_dev *jpeg = priv;
> > +	struct mtk_jpeg_ctx *ctx;
> > +	struct vb2_buffer *src_buf, *dst_buf;
> > +	struct mtk_jpeg_src_buf *jpeg_src_buf;
> > +	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> > +	u32	dec_irq_ret;
> > +	u32 dec_ret;
> > +	int i;
> > +
> > +	ctx = v4l2_m2m_get_curr_priv(jpeg->m2m_dev);
> > +	if (!ctx) {
> > +		v4l2_err(&jpeg->v4l2_dev, "Context is NULL\n");
> > +		return IRQ_HANDLED;
> > +	}
> > +
> > +	src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > +	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> > +	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(src_buf);
> > +
> > +	dec_ret = mtk_jpeg_dec_get_int_status(jpeg->dec_reg_base);
> > +	dec_irq_ret = mtk_jpeg_dec_enum_result(dec_ret);
> > +
> > +	if (dec_irq_ret >= MTK_JPEG_DEC_RESULT_UNDERFLOW)
> > +		mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> > +
> > +	if (dec_irq_ret != MTK_JPEG_DEC_RESULT_EOF_DONE) {
> > +		dev_err(jpeg->dev, "decode failed\n");
> > +		goto dec_end;
> > +	}
> > +
> > +	for (i = 0; i < dst_buf->num_planes; i++)
> > +		vb2_set_plane_payload(dst_buf, i,
> > +				      jpeg_src_buf->dec_param.comp_size[i]);
> > +
> > +	buf_state = VB2_BUF_STATE_DONE;
> > +
> > +dec_end:
> > +	v4l2_m2m_buf_done(to_vb2_v4l2_buffer(src_buf), buf_state);
> > +	v4l2_m2m_buf_done(to_vb2_v4l2_buffer(dst_buf), buf_state);
> > +	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static void mtk_jpeg_set_default_params(struct mtk_jpeg_ctx *ctx)
> > +{
> > +	struct mtk_jpeg_q_data *q = &ctx->out_q;
> > +	int i;
> > +
> > +	ctx->colorspace = V4L2_COLORSPACE_JPEG,
> > +	ctx->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > +	ctx->quantization = V4L2_QUANTIZATION_DEFAULT;
> > +	ctx->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> > +
> > +	q->fmt = mtk_jpeg_find_format(ctx, V4L2_PIX_FMT_JPEG,
> > +					      MTK_JPEG_FMT_TYPE_OUTPUT);
> > +	q->w = MTK_JPEG_MIN_WIDTH;
> > +	q->h = MTK_JPEG_MIN_HEIGHT;
> > +	q->bytesperline[0] = 0;
> > +	q->sizeimage[0] = MTK_JPEG_DEFAULT_SIZEIMAGE;
> > +
> > +	q = &ctx->cap_q;
> > +	q->fmt = mtk_jpeg_find_format(ctx, V4L2_PIX_FMT_YUV420M,
> > +					      MTK_JPEG_FMT_TYPE_CAPTURE);
> > +	q->w = MTK_JPEG_MIN_WIDTH;
> > +	q->h = MTK_JPEG_MIN_HEIGHT;
> > +
> > +	for (i = 0; i < q->fmt->colplanes; i++) {
> > +		u32 stride = q->w * q->fmt->h_sample[i] / 4;
> > +		u32 h = q->h * q->fmt->v_sample[i] / 4;
> > +
> > +		q->bytesperline[i] = stride;
> > +		q->sizeimage[i] = stride * h;
> > +	}
> > +}
> > +
> > +static int mtk_jpeg_open(struct file *file)
> > +{
> > +	struct mtk_jpeg_dev *jpeg = video_drvdata(file);
> > +	struct video_device *vfd = video_devdata(file);
> > +	struct mtk_jpeg_ctx *ctx;
> > +	int ret = 0;
> > +
> > +	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> > +	if (!ctx)
> > +		return -ENOMEM;
> > +
> > +	if (mutex_lock_interruptible(&jpeg->lock)) {
> > +		ret = -ERESTARTSYS;
> > +		goto free;
> > +	}
> > +
> > +	v4l2_fh_init(&ctx->fh, vfd);
> > +	file->private_data = &ctx->fh;
> > +	v4l2_fh_add(&ctx->fh);
> > +
> > +	ctx->jpeg = jpeg;
> > +	ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(jpeg->m2m_dev, ctx,
> > +					    mtk_jpeg_queue_init);
> > +	if (IS_ERR(ctx->fh.m2m_ctx)) {
> > +		ret = PTR_ERR(ctx->fh.m2m_ctx);
> > +		goto error;
> > +	}
> > +
> > +	mtk_jpeg_set_default_params(ctx);
> > +	mutex_unlock(&jpeg->lock);
> > +	return 0;
> > +
> > +error:
> > +	v4l2_fh_del(&ctx->fh);
> > +	v4l2_fh_exit(&ctx->fh);
> > +	mutex_unlock(&jpeg->lock);
> > +free:
> > +	kfree(ctx);
> > +	return ret;
> > +}
> > +
> > +static int mtk_jpeg_release(struct file *file)
> > +{
> > +	struct mtk_jpeg_dev *jpeg = video_drvdata(file);
> > +	struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(file->private_data);
> > +
> > +	mutex_lock(&jpeg->lock);
> > +	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
> > +	v4l2_fh_del(&ctx->fh);
> > +	v4l2_fh_exit(&ctx->fh);
> > +	kfree(ctx);
> > +	mutex_unlock(&jpeg->lock);
> > +	return 0;
> > +}
> > +
> > +static const struct v4l2_file_operations mtk_jpeg_fops = {
> > +	.owner          = THIS_MODULE,
> > +	.open           = mtk_jpeg_open,
> > +	.release        = mtk_jpeg_release,
> > +	.poll           = v4l2_m2m_fop_poll,
> > +	.unlocked_ioctl = video_ioctl2,
> > +	.mmap           = v4l2_m2m_fop_mmap,
> > +};
> > +
> > +static int mtk_jpeg_clk_init(struct mtk_jpeg_dev *jpeg)
> > +{
> > +	struct device_node *node;
> > +	struct platform_device *pdev;
> > +
> > +	node = of_parse_phandle(jpeg->dev->of_node, "mediatek,larb", 0);
> > +	if (!node)
> > +		return -EINVAL;
> > +	pdev = of_find_device_by_node(node);
> > +	if (WARN_ON(!pdev)) {
> > +		of_node_put(node);
> > +		return -EINVAL;
> > +	}
> > +	of_node_put(node);
> > +
> > +	jpeg->larb = &pdev->dev;
> > +
> > +	jpeg->clk_jdec = devm_clk_get(jpeg->dev, "jpgdec");
> > +	if (IS_ERR(jpeg->clk_jdec))
> > +		return -EINVAL;
> > +
> > +	jpeg->clk_jdec_smi = devm_clk_get(jpeg->dev, "jpgdec-smi");
> > +	if (IS_ERR(jpeg->clk_jdec_smi))
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtk_jpeg_probe(struct platform_device *pdev)
> > +{
> > +	struct mtk_jpeg_dev *jpeg;
> > +	struct resource *res;
> > +	int dec_irq;
> > +	int ret;
> > +
> > +	jpeg = devm_kzalloc(&pdev->dev, sizeof(*jpeg), GFP_KERNEL);
> > +	if (!jpeg)
> > +		return -ENOMEM;
> > +
> > +	mutex_init(&jpeg->lock);
> > +	spin_lock_init(&jpeg->hw_lock);
> > +	jpeg->dev = &pdev->dev;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	jpeg->dec_reg_base = devm_ioremap_resource(&pdev->dev, res);
> > +	if (IS_ERR(jpeg->dec_reg_base)) {
> > +		ret = PTR_ERR(jpeg->dec_reg_base);
> > +		return ret;
> > +	}
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > +	dec_irq = platform_get_irq(pdev, 0);
> > +	if (!res || dec_irq < 0) {
> > +		dev_err(&pdev->dev, "Failed to get dec_irq %d.\n", dec_irq);
> > +		ret = -EINVAL;
> > +		return ret;
> > +	}
> > +
> > +	ret = devm_request_irq(&pdev->dev, dec_irq, mtk_jpeg_dec_irq, 0,
> > +			       pdev->name, jpeg);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Failed to request dec_irq %d (%d)\n",
> > +			dec_irq, ret);
> > +		ret = -EINVAL;
> > +		goto err_req_irq;
> > +	}
> > +
> > +	ret = mtk_jpeg_clk_init(jpeg);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Failed to init clk, err %d\n", ret);
> > +		goto err_clk_init;
> > +	}
> > +
> > +	ret = v4l2_device_register(&pdev->dev, &jpeg->v4l2_dev);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Failed to register v4l2 device\n");
> > +		ret = -EINVAL;
> > +		goto err_dev_register;
> > +	}
> > +
> > +	jpeg->m2m_dev = v4l2_m2m_init(&mtk_jpeg_m2m_ops);
> > +	if (IS_ERR(jpeg->m2m_dev)) {
> > +		v4l2_err(&jpeg->v4l2_dev, "Failed to init mem2mem device\n");
> > +		ret = PTR_ERR(jpeg->m2m_dev);
> > +		goto err_m2m_init;
> > +	}
> > +
> > +	jpeg->dec_vdev = video_device_alloc();
> > +	if (!jpeg->dec_vdev) {
> > +		ret = -ENOMEM;
> > +		goto err_dec_vdev_alloc;
> > +	}
> > +	snprintf(jpeg->dec_vdev->name, sizeof(jpeg->dec_vdev->name),
> > +		 "%s-dec", MTK_JPEG_NAME);
> > +	jpeg->dec_vdev->fops = &mtk_jpeg_fops;
> > +	jpeg->dec_vdev->ioctl_ops = &mtk_jpeg_ioctl_ops;
> > +	jpeg->dec_vdev->minor = -1;
> > +	jpeg->dec_vdev->release = video_device_release;
> > +	jpeg->dec_vdev->lock = &jpeg->lock;
> > +	jpeg->dec_vdev->v4l2_dev = &jpeg->v4l2_dev;
> > +	jpeg->dec_vdev->vfl_dir = VFL_DIR_M2M;
> > +	jpeg->dec_vdev->device_caps = V4L2_CAP_STREAMING |
> > +				      V4L2_CAP_VIDEO_M2M_MPLANE;
> > +
> > +	ret = video_register_device(jpeg->dec_vdev, VFL_TYPE_GRABBER, 3);
> > +	if (ret) {
> > +		v4l2_err(&jpeg->v4l2_dev, "Failed to register video device\n");
> > +		goto err_dec_vdev_register;
> > +	}
> > +
> > +	video_set_drvdata(jpeg->dec_vdev, jpeg);
> > +	v4l2_info(&jpeg->v4l2_dev,
> > +		  "decoder device registered as /dev/video%d (%d,%d)\n",
> > +		  jpeg->dec_vdev->num, VIDEO_MAJOR, jpeg->dec_vdev->minor);
> > +
> > +	platform_set_drvdata(pdev, jpeg);
> > +
> > +	pm_runtime_enable(&pdev->dev);
> > +
> > +	return 0;
> > +
> > +err_dec_vdev_register:
> > +	video_device_release(jpeg->dec_vdev);
> > +
> > +err_dec_vdev_alloc:
> > +	v4l2_m2m_release(jpeg->m2m_dev);
> > +
> > +err_m2m_init:
> > +	v4l2_device_unregister(&jpeg->v4l2_dev);
> > +
> > +err_dev_register:
> > +
> > +err_clk_init:
> > +
> > +err_req_irq:
> > +
> > +	return ret;
> > +}
> > +
> > +static int mtk_jpeg_remove(struct platform_device *pdev)
> > +{
> > +	struct mtk_jpeg_dev *jpeg = platform_get_drvdata(pdev);
> > +
> > +	pm_runtime_disable(&pdev->dev);
> > +	video_unregister_device(jpeg->dec_vdev);
> > +	video_device_release(jpeg->dec_vdev);
> > +	v4l2_m2m_release(jpeg->m2m_dev);
> > +	v4l2_device_unregister(&jpeg->v4l2_dev);
> > +
> > +	return 0;
> > +}
> > +
> > +#ifdef CONFIG_PM
> > +static int mtk_jpeg_pm_suspend(struct device *dev)
> > +{
> > +	struct mtk_jpeg_dev *jpeg = dev_get_drvdata(dev);
> > +
> > +	mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> > +	mtk_jpeg_clk_off(jpeg);
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtk_jpeg_pm_resume(struct device *dev)
> > +{
> > +	struct mtk_jpeg_dev *jpeg = dev_get_drvdata(dev);
> > +
> > +	mtk_jpeg_clk_on(jpeg);
> > +	mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> > +
> > +	return 0;
> > +}
> > +#endif /* CONFIG_PM */
> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int mtk_jpeg_suspend(struct device *dev)
> > +{
> > +	int ret;
> > +
> > +	if (pm_runtime_suspended(dev))
> > +		return 0;
> > +
> > +	ret = mtk_jpeg_pm_suspend(dev);
> > +	return ret;
> > +}
> > +
> > +static int mtk_jpeg_resume(struct device *dev)
> > +{
> > +	int ret;
> > +
> > +	if (pm_runtime_suspended(dev))
> > +		return 0;
> > +
> > +	ret = mtk_jpeg_pm_resume(dev);
> > +
> > +	return ret;
> > +}
> > +#endif /* CONFIG_PM_SLEEP */
> > +
> > +static const struct dev_pm_ops mtk_jpeg_pm_ops = {
> > +	SET_SYSTEM_SLEEP_PM_OPS(mtk_jpeg_suspend, mtk_jpeg_resume)
> > +	SET_RUNTIME_PM_OPS(mtk_jpeg_pm_suspend, mtk_jpeg_pm_resume, NULL)
> > +};
> > +
> > +static const struct of_device_id mtk_jpeg_match[] = {
> > +	{
> > +		.compatible = "mediatek,mt8173-jpgdec",
> > +		.data       = NULL,
> > +	},
> > +	{
> > +		.compatible = "mediatek,mt2701-jpgdec",
> > +		.data       = NULL,
> > +	},
> > +	{},
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, mtk_jpeg_match);
> > +
> > +static struct platform_driver mtk_jpeg_driver = {
> > +	.probe = mtk_jpeg_probe,
> > +	.remove = mtk_jpeg_remove,
> > +	.driver = {
> > +		.owner          = THIS_MODULE,
> > +		.name           = MTK_JPEG_NAME,
> > +		.of_match_table = mtk_jpeg_match,
> > +		.pm             = &mtk_jpeg_pm_ops,
> > +	},
> > +};
> > +
> > +module_platform_driver(mtk_jpeg_driver);
> > +
> > +MODULE_DESCRIPTION("MediaTek JPEG codec driver");
> > +MODULE_LICENSE("GPL v2");
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > new file mode 100644
> > index 0000000..d862e3b
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > @@ -0,0 +1,141 @@
> > +/*
> > + * Copyright (c) 2016 MediaTek Inc.
> > + * Author: Ming Hsiu Tsai <minghsiu.tsai@mediatek.com>
> > + *         Rick Chang <rick.chang@mediatek.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#ifndef _MTK_JPEG_CORE_H
> > +#define _MTK_JPEG_CORE_H
> > +
> > +#include <linux/interrupt.h>
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/v4l2-fh.h>
> > +
> > +#define MTK_JPEG_NAME		"mtk-jpeg"
> > +
> > +#define MTK_JPEG_FMT_FLAG_DEC_OUTPUT	BIT(0)
> > +#define MTK_JPEG_FMT_FLAG_DEC_CAPTURE	BIT(1)
> > +
> > +#define MTK_JPEG_FMT_TYPE_OUTPUT	1
> > +#define MTK_JPEG_FMT_TYPE_CAPTURE	2
> > +
> > +#define MTK_JPEG_MIN_WIDTH	32
> > +#define MTK_JPEG_MIN_HEIGHT	32
> > +#define MTK_JPEG_MAX_WIDTH	8192
> > +#define MTK_JPEG_MAX_HEIGHT	8192
> > +
> > +#define MTK_JPEG_DEFAULT_SIZEIMAGE	(1 * 1024 * 1024)
> > +
> > +enum mtk_jpeg_ctx_state {
> > +	MTK_JPEG_INIT = 0,
> > +	MTK_JPEG_RUNNING,
> > +	MTK_JPEG_SOURCE_CHANGE,
> > +};
> > +
> > +/**
> > + * struct mt_jpeg - JPEG IP abstraction
> > + * @lock:		the mutex protecting this structure
> > + * @hw_lock:		spinlock protecting the hw device resource
> > + * @workqueue:		decode work queue
> > + * @dev:		JPEG device
> > + * @v4l2_dev:		v4l2 device for mem2mem mode
> > + * @m2m_dev:		v4l2 mem2mem device data
> > + * @alloc_ctx:		videobuf2 memory allocator's context
> > + * @dec_vdev:		video device node for decoder mem2mem mode
> > + * @dec_reg_base:	JPEG registers mapping
> > + * @clk_jdec:		JPEG hw working clock
> > + * @clk_jdec_smi:	JPEG SMI bus clock
> > + * @larb:		SMI device
> > + */
> > +struct mtk_jpeg_dev {
> > +	struct mutex		lock;
> > +	spinlock_t		hw_lock;
> > +	struct workqueue_struct	*workqueue;
> > +	struct device		*dev;
> > +	struct v4l2_device	v4l2_dev;
> > +	struct v4l2_m2m_dev	*m2m_dev;
> > +	void			*alloc_ctx;
> > +	struct video_device	*dec_vdev;
> > +	void __iomem		*dec_reg_base;
> > +	struct clk		*clk_jdec;
> > +	struct clk		*clk_jdec_smi;
> > +	struct device		*larb;
> > +};
> > +
> > +/**
> > + * struct jpeg_fmt - driver's internal color format data
> > + * @name:	format descritpion
> > + * @fourcc:	the fourcc code, 0 if not applicable
> > + * @h_sample:	horizontal sample count of plane in 4 * 4 pixel image
> > + * @v_sample:	vertical sample count of plane in 4 * 4 pixel image
> > + * @colplanes:	number of color planes (1 for packed formats)
> > + * @h_align:	horizontal alignment order (align to 2^h_align)
> > + * @v_align:	vertical alignment order (align to 2^v_align)
> > + * @flags:	flags describing format applicability
> > + */
> > +struct mtk_jpeg_fmt {
> > +	char	*name;
> > +	u32	fourcc;
> > +	int	h_sample[VIDEO_MAX_PLANES];
> > +	int	v_sample[VIDEO_MAX_PLANES];
> > +	int	colplanes;
> > +	int	h_align;
> > +	int	v_align;
> > +	u32	flags;
> > +};
> > +
> > +/**
> > + * mtk_jpeg_q_data - parameters of one queue
> > + * @fmt:	  driver-specific format of this queue
> > + * @w:		  image width
> > + * @h:		  image height
> > + * @bytesperline: distance in bytes between the leftmost pixels in two adjacent
> > + *                lines
> > + * @sizeimage:	  image buffer size in bytes
> > + */
> > +struct mtk_jpeg_q_data {
> > +	struct mtk_jpeg_fmt	*fmt;
> > +	u32			w;
> > +	u32			h;
> > +	u32			bytesperline[VIDEO_MAX_PLANES];
> > +	u32			sizeimage[VIDEO_MAX_PLANES];
> > +};
> > +
> > +/**
> > + * mtk_jpeg_ctx - the device context data
> > + * @jpeg:		JPEG IP device for this context
> > + * @out_q:		source (output) queue information
> > + * @cap_q:		destination (capture) queue queue information
> > + * @fh:			V4L2 file handle
> > + * @dec_param		parameters for HW decoding
> > + * @state:		state of the context
> > + * @header_valid:	set if header has been parsed and valid
> > + * @colorspace: enum v4l2_colorspace; supplemental to pixelformat
> > + * @ycbcr_enc: enum v4l2_ycbcr_encoding, Y'CbCr encoding
> > + * @quantization: enum v4l2_quantization, colorspace quantization
> > + * @xfer_func: enum v4l2_xfer_func, colorspace transfer function
> > + */
> > +struct mtk_jpeg_ctx {
> > +	struct mtk_jpeg_dev		*jpeg;
> > +	struct mtk_jpeg_q_data		out_q;
> > +	struct mtk_jpeg_q_data		cap_q;
> > +	struct v4l2_fh			fh;
> > +	enum mtk_jpeg_ctx_state		state;
> > +
> > +	enum v4l2_colorspace colorspace;
> > +	enum v4l2_ycbcr_encoding ycbcr_enc;
> > +	enum v4l2_quantization quantization;
> > +	enum v4l2_xfer_func xfer_func;
> > +};
> > +
> > +#endif /* _MTK_JPEG_CORE_H */
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.c
> > new file mode 100644
> > index 0000000..a6315f3
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.c
> > @@ -0,0 +1,417 @@
> > +/*
> > + * Copyright (c) 2016 MediaTek Inc.
> > + * Author: Ming Hsiu Tsai <minghsiu.tsai@mediatek.com>
> > + *         Rick Chang <rick.chang@mediatek.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <media/videobuf2-core.h>
> > +
> > +#include "mtk_jpeg_hw.h"
> > +
> > +#define MTK_JPEG_DUNUM_MASK(val)	(((val) - 1) & 0x3)
> > +
> > +enum mtk_jpeg_color {
> > +	MTK_JPEG_COLOR_420		= 0x00221111,
> > +	MTK_JPEG_COLOR_422		= 0x00211111,
> > +	MTK_JPEG_COLOR_444		= 0x00111111,
> > +	MTK_JPEG_COLOR_422V		= 0x00121111,
> > +	MTK_JPEG_COLOR_422X2		= 0x00412121,
> > +	MTK_JPEG_COLOR_422VX2		= 0x00222121,
> > +	MTK_JPEG_COLOR_400		= 0x00110000
> > +};
> > +
> > +static inline int mtk_jpeg_verify_align(u32 val, int align, u32 reg)
> > +{
> > +	if (val & (align - 1)) {
> > +		pr_err("mtk-jpeg: write reg %x without %d align\n", reg, align);
> > +		return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtk_jpeg_decide_format(struct mtk_jpeg_dec_param *param)
> > +{
> > +	param->src_color = (param->sampling_w[0] << 20) |
> > +			   (param->sampling_h[0] << 16) |
> > +			   (param->sampling_w[1] << 12) |
> > +			   (param->sampling_h[1] << 8) |
> > +			   (param->sampling_w[2] << 4) |
> > +			   (param->sampling_h[2]);
> > +
> > +	param->uv_brz_w = 0;
> > +	switch (param->src_color) {
> > +	case MTK_JPEG_COLOR_444:
> > +		param->uv_brz_w = 1;
> > +		param->dst_fourcc = V4L2_PIX_FMT_YUV422M;
> > +		break;
> > +	case MTK_JPEG_COLOR_422X2:
> > +	case MTK_JPEG_COLOR_422:
> > +		param->dst_fourcc = V4L2_PIX_FMT_YUV422M;
> > +		break;
> > +	case MTK_JPEG_COLOR_422V:
> > +	case MTK_JPEG_COLOR_422VX2:
> > +		param->uv_brz_w = 1;
> > +		param->dst_fourcc = V4L2_PIX_FMT_YUV420M;
> > +		break;
> > +	case MTK_JPEG_COLOR_420:
> > +		param->dst_fourcc = V4L2_PIX_FMT_YUV420M;
> > +		break;
> > +	case MTK_JPEG_COLOR_400:
> > +		param->dst_fourcc = V4L2_PIX_FMT_GREY;
> > +		break;
> > +	default:
> > +		param->dst_fourcc = 0;
> > +		return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void mtk_jpeg_calc_mcu(struct mtk_jpeg_dec_param *param)
> > +{
> > +	u32 factor_w, factor_h;
> > +	u32 i, comp, blk;
> > +
> > +	factor_w = 2 + param->sampling_w[0];
> > +	factor_h = 2 + param->sampling_h[0];
> > +	param->mcu_w = (param->pic_w + (1 << factor_w) - 1) >> factor_w;
> > +	param->mcu_h = (param->pic_h + (1 << factor_h) - 1) >> factor_h;
> > +	param->total_mcu = param->mcu_w * param->mcu_h;
> > +	param->unit_num = ((param->pic_w + 7) >> 3) * ((param->pic_h + 7) >> 3);
> > +	param->blk_num = 0;
> > +	for (i = 0; i < MTK_JPEG_COMP_MAX; i++) {
> > +		param->blk_comp[i] = 0;
> > +		if (i >= param->comp_num)
> > +			continue;
> > +		param->blk_comp[i] = param->sampling_w[i] *
> > +				     param->sampling_h[i];
> > +		param->blk_num += param->blk_comp[i];
> > +	}
> > +
> > +	param->membership = 0;
> > +	for (i = 0, blk = 0, comp = 0; i < MTK_JPEG_BLOCK_MAX; i++) {
> > +		if (i < param->blk_num && comp < param->comp_num) {
> > +			u32 tmp;
> > +
> > +			tmp = (0x04 + (comp & 0x3));
> > +			param->membership |= tmp << (i * 3);
> > +			if (++blk == param->blk_comp[comp]) {
> > +				comp++;
> > +				blk = 0;
> > +			}
> > +		} else {
> > +			param->membership |=  7 << (i * 3);
> > +		}
> > +	}
> > +}
> > +
> > +static void mtk_jpeg_calc_dma_group(struct mtk_jpeg_dec_param *param)
> > +{
> > +	u32 factor_mcu = 3;
> > +
> > +	if (param->src_color == MTK_JPEG_COLOR_444 &&
> > +	    param->dst_fourcc == V4L2_PIX_FMT_YUV422M)
> > +		factor_mcu = 4;
> > +	else if (param->src_color == MTK_JPEG_COLOR_422V &&
> > +		 param->dst_fourcc == V4L2_PIX_FMT_YUV420M)
> > +		factor_mcu = 4;
> > +	else if (param->src_color == MTK_JPEG_COLOR_422X2 &&
> > +		 param->dst_fourcc == V4L2_PIX_FMT_YUV422M)
> > +		factor_mcu = 2;
> > +	else if (param->src_color == MTK_JPEG_COLOR_400 ||
> > +		 (param->src_color & 0x0FFFF) == 0)
> > +		factor_mcu = 4;
> > +
> > +	param->dma_mcu = 1 << factor_mcu;
> > +	param->dma_group = param->mcu_w / param->dma_mcu;
> > +	param->dma_last_mcu = param->mcu_w % param->dma_mcu;
> > +	if (param->dma_last_mcu)
> > +		param->dma_group++;
> > +	else
> > +		param->dma_last_mcu = param->dma_mcu;
> > +}
> > +
> > +static int mtk_jpeg_calc_dst_size(struct mtk_jpeg_dec_param *param)
> > +{
> > +	u32 i, padding_w;
> > +	u32 ds_row_h[3];
> > +	u32 brz_w[3];
> > +
> > +	brz_w[0] = 0;
> > +	brz_w[1] = param->uv_brz_w;
> > +	brz_w[2] = brz_w[1];
> > +
> > +	for (i = 0; i < param->comp_num; i++) {
> > +		if (brz_w[i] > 3)
> > +			return -1;
> > +
> > +		padding_w = param->mcu_w * MTK_JPEG_DCTSIZE *
> > +				param->sampling_w[i];
> > +		/* output format is 420/422 */
> > +		param->comp_w[i] = padding_w >> brz_w[i];
> > +		param->comp_w[i] = mtk_jpeg_align(param->comp_w[i],
> > +						  MTK_JPEG_DCTSIZE);
> > +		param->img_stride[i] = i ? mtk_jpeg_align(param->comp_w[i], 16)
> > +					: mtk_jpeg_align(param->comp_w[i], 32);
> > +		ds_row_h[i] = (MTK_JPEG_DCTSIZE * param->sampling_h[i]);
> > +	}
> > +	param->dec_w = param->img_stride[0];
> > +	param->dec_h = ds_row_h[0] * param->mcu_h;
> > +
> > +	for (i = 0; i < MTK_JPEG_COMP_MAX; i++) {
> > +		/* They must be equal in frame mode. */
> > +		param->mem_stride[i] = param->img_stride[i];
> > +		param->comp_size[i] = param->mem_stride[i] * ds_row_h[i] *
> > +				      param->mcu_h;
> > +	}
> > +
> > +	param->y_size = param->comp_size[0];
> > +	param->uv_size = param->comp_size[1];
> > +	param->dec_size = param->y_size + (param->uv_size << 1);
> > +
> > +	return 0;
> > +}
> > +
> > +int mtk_jpeg_dec_fill_param(struct mtk_jpeg_dec_param *param)
> > +{
> > +	if (mtk_jpeg_decide_format(param))
> > +		return -1;
> > +
> > +	mtk_jpeg_calc_mcu(param);
> > +	mtk_jpeg_calc_dma_group(param);
> > +	if (mtk_jpeg_calc_dst_size(param))
> > +		return -2;
> > +
> > +	return 0;
> > +}
> > +
> > +u32 mtk_jpeg_dec_get_int_status(void __iomem *base)
> > +{
> > +	u32 ret;
> > +
> > +	ret = readl(base + JPGDEC_REG_INTERRUPT_STATUS) & BIT_INQST_MASK_ALLIRQ;
> > +	if (ret)
> > +		writel(ret, base + JPGDEC_REG_INTERRUPT_STATUS);
> > +
> > +	return ret;
> > +}
> > +
> > +u32 mtk_jpeg_dec_enum_result(u32 irq_result)
> > +{
> > +	if (irq_result & BIT_INQST_MASK_EOF)
> > +		return MTK_JPEG_DEC_RESULT_EOF_DONE;
> > +	else if (irq_result & BIT_INQST_MASK_PAUSE)
> > +		return MTK_JPEG_DEC_RESULT_PAUSE;
> > +	else if (irq_result & BIT_INQST_MASK_UNDERFLOW)
> > +		return MTK_JPEG_DEC_RESULT_UNDERFLOW;
> > +	else if (irq_result & BIT_INQST_MASK_OVERFLOW)
> > +		return MTK_JPEG_DEC_RESULT_OVERFLOW;
> > +	else if (irq_result & BIT_INQST_MASK_ERROR_BS)
> > +		return MTK_JPEG_DEC_RESULT_ERROR_BS;
> 
> No need for 'else' here since the previous 'if' always returns if true.

Ok.

> > +
> > +	return MTK_JPEG_DEC_RESULT_ERROR_UNKNOWN;
> > +}
> > +
> > +void mtk_jpeg_dec_start(void __iomem *base)
> > +{
> > +	writel(0, base + JPGDEC_REG_TRIG);
> > +}
> > +
> > +static void mtk_jpeg_dec_soft_reset(void __iomem *base)
> > +{
> > +	writel(0x0000FFFF, base + JPGDEC_REG_INTERRUPT_STATUS);
> > +	writel(0x00, base + JPGDEC_REG_RESET);
> > +	writel(0x01, base + JPGDEC_REG_RESET);
> > +}
> > +
> > +static void mtk_jpeg_dec_hard_reset(void __iomem *base)
> > +{
> > +	writel(0x00, base + JPGDEC_REG_RESET);
> > +	writel(0x10, base + JPGDEC_REG_RESET);
> > +}
> > +
> > +void mtk_jpeg_dec_reset(void __iomem *base)
> > +{
> > +	mtk_jpeg_dec_soft_reset(base);
> > +	mtk_jpeg_dec_hard_reset(base);
> > +}
> > +
> > +static void mtk_jpeg_dec_set_brz_factor(void __iomem *base, u8 yscale_w,
> > +					u8 yscale_h, u8 uvscale_w, u8 uvscale_h)
> > +{
> > +	u32 val;
> > +
> > +	val = (uvscale_h << 12) | (uvscale_w << 8) |
> > +	      (yscale_h << 4) | yscale_w;
> > +	writel(val, base + JPGDEC_REG_BRZ_FACTOR);
> > +}
> > +
> > +static void mtk_jpeg_dec_set_dst_bank0(void __iomem *base, u32 addr_y,
> > +				       u32 addr_u, u32 addr_v)
> > +{
> > +	mtk_jpeg_verify_align(addr_y, 16, JPGDEC_REG_DEST_ADDR0_Y);
> > +	writel(addr_y, base + JPGDEC_REG_DEST_ADDR0_Y);
> > +	mtk_jpeg_verify_align(addr_u, 16, JPGDEC_REG_DEST_ADDR0_U);
> > +	writel(addr_u, base + JPGDEC_REG_DEST_ADDR0_U);
> > +	mtk_jpeg_verify_align(addr_v, 16, JPGDEC_REG_DEST_ADDR0_V);
> > +	writel(addr_v, base + JPGDEC_REG_DEST_ADDR0_V);
> > +}
> > +
> > +static void mtk_jpeg_dec_set_dst_bank1(void __iomem *base, u32 addr_y,
> > +				       u32 addr_u, u32 addr_v)
> > +{
> > +	writel(addr_y, base + JPGDEC_REG_DEST_ADDR1_Y);
> > +	writel(addr_u, base + JPGDEC_REG_DEST_ADDR1_U);
> > +	writel(addr_v, base + JPGDEC_REG_DEST_ADDR1_V);
> > +}
> > +
> > +static void mtk_jpeg_dec_set_mem_stride(void __iomem *base, u32 stride_y,
> > +					u32 stride_uv)
> > +{
> > +	writel((stride_y & 0xFFFF), base + JPGDEC_REG_STRIDE_Y);
> > +	writel((stride_uv & 0xFFFF), base + JPGDEC_REG_STRIDE_UV);
> > +}
> > +
> > +static void mtk_jpeg_dec_set_img_stride(void __iomem *base, u32 stride_y,
> > +					u32 stride_uv)
> > +{
> > +	writel((stride_y & 0xFFFF), base + JPGDEC_REG_IMG_STRIDE_Y);
> > +	writel((stride_uv & 0xFFFF), base + JPGDEC_REG_IMG_STRIDE_UV);
> > +}
> > +
> > +static void mtk_jpeg_dec_set_pause_mcu_idx(void __iomem *base, u32 idx)
> > +{
> > +	writel(idx & 0x0003FFFFFF, base + JPGDEC_REG_PAUSE_MCU_NUM);
> > +}
> > +
> > +static void mtk_jpeg_dec_set_dec_mode(void __iomem *base, u32 mode)
> > +{
> > +	writel(mode & 0x03, base + JPGDEC_REG_OPERATION_MODE);
> > +}
> > +
> > +static void mtk_jpeg_dec_set_bs_write_ptr(void __iomem *base, u32 ptr)
> > +{
> > +	mtk_jpeg_verify_align(ptr, 16, JPGDEC_REG_FILE_BRP);
> > +	writel(ptr, base + JPGDEC_REG_FILE_BRP);
> > +}
> > +
> > +static void mtk_jpeg_dec_set_bs_info(void __iomem *base, u32 addr, u32 size)
> > +{
> > +	mtk_jpeg_verify_align(addr, 16, JPGDEC_REG_FILE_ADDR);
> > +	mtk_jpeg_verify_align(size, 128, JPGDEC_REG_FILE_TOTAL_SIZE);
> > +	writel(addr, base + JPGDEC_REG_FILE_ADDR);
> > +	writel(size, base + JPGDEC_REG_FILE_TOTAL_SIZE);
> > +}
> > +
> > +static void mtk_jpeg_dec_set_comp_id(void __iomem *base, u32 id_y, u32 id_u,
> > +				     u32 id_v)
> > +{
> > +	u32 val;
> > +
> > +	val = ((id_y & 0x00FF) << 24) | ((id_u & 0x00FF) << 16) |
> > +	      ((id_v & 0x00FF) << 8);
> > +	writel(val, base + JPGDEC_REG_COMP_ID);
> > +}
> > +
> > +static void mtk_jpeg_dec_set_total_mcu(void __iomem *base, u32 num)
> > +{
> > +	writel(num - 1, base + JPGDEC_REG_TOTAL_MCU_NUM);
> > +}
> > +
> > +static void mtk_jpeg_dec_set_comp0_du(void __iomem *base, u32 num)
> > +{
> > +	writel(num - 1, base + JPGDEC_REG_COMP0_DATA_UNIT_NUM);
> > +}
> > +
> > +static void mtk_jpeg_dec_set_du_membership(void __iomem *base, u32 member,
> > +					   u32 gmc, u32 isgray)
> > +{
> > +	if (isgray)
> > +		member = 0x3FFFFFFC;
> > +	member |= (isgray << 31) | (gmc << 30);
> > +	writel(member, base + JPGDEC_REG_DU_CTRL);
> > +}
> > +
> > +static void mtk_jpeg_dec_set_q_table(void __iomem *base, u32 id0, u32 id1,
> > +				     u32 id2)
> > +{
> > +	u32 val;
> > +
> > +	val = ((id0 & 0x0f) << 8) | ((id1 & 0x0f) << 4) | ((id2 & 0x0f) << 0);
> > +	writel(val, base + JPGDEC_REG_QT_ID);
> > +}
> > +
> > +static void mtk_jpeg_dec_set_dma_group(void __iomem *base, u32 mcu_group,
> > +				       u32 group_num, u32 last_mcu)
> > +{
> > +	u32 val;
> > +
> > +	val = (((mcu_group - 1) & 0x00FF) << 16) |
> > +	      (((group_num - 1) & 0x007F) << 8) |
> > +	      ((last_mcu - 1) & 0x00FF);
> > +	writel(val, base + JPGDEC_REG_WDMA_CTRL);
> > +}
> > +
> > +static void mtk_jpeg_dec_set_sampling_factor(void __iomem *base, u32 comp_num,
> > +					     u32 y_w, u32 y_h, u32 u_w,
> > +					     u32 u_h, u32 v_w, u32 v_h)
> > +{
> > +	u32 val;
> > +	u32 y_wh = (MTK_JPEG_DUNUM_MASK(y_w) << 2) | MTK_JPEG_DUNUM_MASK(y_h);
> > +	u32 u_wh = (MTK_JPEG_DUNUM_MASK(u_w) << 2) | MTK_JPEG_DUNUM_MASK(u_h);
> > +	u32 v_wh = (MTK_JPEG_DUNUM_MASK(v_w) << 2) | MTK_JPEG_DUNUM_MASK(v_h);
> > +
> > +	if (comp_num == 1)
> > +		val = 0;
> > +	else
> > +		val = (y_wh << 8) | (u_wh << 4) | v_wh;
> > +	writel(val, base + JPGDEC_REG_DU_NUM);
> > +}
> > +
> > +void mtk_jpeg_dec_set_config(void __iomem *base,
> > +			     struct mtk_jpeg_dec_param *config,
> > +			     struct mtk_jpeg_bs *bs,
> > +			     struct mtk_jpeg_fb *fb)
> > +{
> > +	mtk_jpeg_dec_set_brz_factor(base, 0, 0, config->uv_brz_w, 0);
> > +	mtk_jpeg_dec_set_dec_mode(base, 0);
> > +	mtk_jpeg_dec_set_comp0_du(base, config->unit_num);
> > +	mtk_jpeg_dec_set_total_mcu(base, config->total_mcu);
> > +	mtk_jpeg_dec_set_bs_info(base, bs->str_addr, bs->size);
> > +	mtk_jpeg_dec_set_bs_write_ptr(base, bs->end_addr);
> > +	mtk_jpeg_dec_set_du_membership(base, config->membership, 1,
> > +				       (config->comp_num == 1) ? 1 : 0);
> > +	mtk_jpeg_dec_set_comp_id(base, config->comp_id[0], config->comp_id[1],
> > +				 config->comp_id[2]);
> > +	mtk_jpeg_dec_set_q_table(base, config->qtbl_num[0],
> > +				 config->qtbl_num[1], config->qtbl_num[2]);
> > +	mtk_jpeg_dec_set_sampling_factor(base, config->comp_num,
> > +					 config->sampling_w[0],
> > +					 config->sampling_h[0],
> > +					 config->sampling_w[1],
> > +					 config->sampling_h[1],
> > +					 config->sampling_w[2],
> > +					 config->sampling_h[2]);
> > +	mtk_jpeg_dec_set_mem_stride(base, config->mem_stride[0],
> > +				    config->mem_stride[1]);
> > +	mtk_jpeg_dec_set_img_stride(base, config->img_stride[0],
> > +				    config->img_stride[1]);
> > +	mtk_jpeg_dec_set_dst_bank0(base, fb->plane_addr[0],
> > +				   fb->plane_addr[1], fb->plane_addr[2]);
> > +	mtk_jpeg_dec_set_dst_bank1(base, 0, 0, 0);
> > +	mtk_jpeg_dec_set_dma_group(base, config->dma_mcu, config->dma_group,
> > +				   config->dma_last_mcu);
> > +	mtk_jpeg_dec_set_pause_mcu_idx(base, config->total_mcu);
> > +}
> 
> Regards,
> 
> 	Hans

^ permalink raw reply

* [PATCH] ARM: dma-mapping: preallocate DMA-debug hash tables in pure_initcall
From: Arnd Bergmann @ 2016-11-15 10:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479204295-29773-1-git-send-email-m.szyprowski@samsung.com>

On Tuesday, November 15, 2016 11:04:55 AM CET Marek Szyprowski wrote:
> fs_initcall is definitely too late to initialize DMA-debug hash tables,
> because some drivers might get probed and use DMA mapping framework
> already in core_initcall. Late initialization of DMA-debug results in
> false warning about accessing memory, that was not allocated, like this
> one:

To ask the obvious question: what driver does DMA allocations in a
core_initcall, and have you tried to change that first?

	Arnd

^ permalink raw reply

* LEDs that change brightness "itself" -- that's a trigger. Re: PM regression with LED changes in next-20161109
From: Pavel Machek @ 2016-11-15 10:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <15cafbf5-d842-e184-2fd4-65f8272f505a@redhat.com>

Hi!

> >Hmm, v4 still calls led_notify_brightness_change(led_cdev)
> >from both __led_set_brightness() and __led_set_brightness_blocking().
> 
> Ugh, I see I accidentally send a v4 twice, instead of
> calling the version which dropped those called v5 as
> I should have, sorry.
> 
> The v4 which I would like to see merged, the one with
> those calls dropped, is here:
> 
> https://patchwork.kernel.org/patch/9423093/

Please, lets fix this properly.

The LED you are talking about _has_ a trigger, implemented in
hardware. That trigger can change LED brightness behind kernel's (and
userspace's) back. Don't pretend the trigger does not exist, it does.

And when you do that, you'll have nice place to report changes to
userspace -- trigger can now export that information, and offer poll()
interface.

Thanks,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161115/6fb9cbc8/attachment.sig>

^ permalink raw reply

* wdt, gpio: move arch_initcall into subsys_initcall ?
From: Heiko Schocher @ 2016-11-15 10:20 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

commit e188cbf7564f: "gpio: mxc: shift gpio_mxc_init() to subsys_initcall level"
moves the gpio initialization of the mxc gpio driver
from the arch_initcall level into subsys_initcall level.

This leads now on mxc boards, which use a gpio wdt driver
and the CONFIG_GPIO_WATCHDOG_ARCH_INITCALL option enabled,
to unwanted driver probe deferrals during kernel boot.

I see this currently on an imx6 based board (which has unfortunately
3 WDT: imx6 internal (disabled), gpio wdt and da9063 WDT ...).

Also a side effect from above commit is, that the da9063 WDT driver
is now probed before the gpio WDT driver ... so /dev/watchdog now
does not point to the gpio_wdt, instead it points to the da9063 WDT.

So there are 2 solutions possible:

- add a CONFIG_GPIO_MCX_ARCH_INITCALL option
   in drivers/gpio/gpio-mxc.c like for the gpio_wdt.c driver?

   But how can we guarantee, that first the gpio driver and then
   the gpio_wdt driver gets probed?

- move the arch_initcall in gpio_wdt.c into a subsys_initcall
   (Tested this, and the probe dereferral messages are gone ...)

   But this may results in problems on boards, which needs an early
   trigger on an gpio wdt ...

Other ideas?

Thanks for all suggestions!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

^ permalink raw reply

* [PATCHv2] ARM: ftrace: fix syscall name matching
From: Rabin Vincent @ 2016-11-15 10:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479128625-20948-1-git-send-email-rabin.vincent@axis.com>

From: Rabin Vincent <rabinv@axis.com>

ARM has a few system calls (most notably mmap) for which the names of
the functions which are referenced in the syscall table do not match the
names of the syscall tracepoints.  As a consequence of this, these
tracepoints are not made available.  Implement
arch_syscall_match_sym_name to fix this and allow tracing even these
system calls.

Signed-off-by: Rabin Vincent <rabinv@axis.com>
---
v2: get rid of unsafe-looking pointer adjustment

 arch/arm/include/asm/ftrace.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
index bfe2a2f..22b7311 100644
--- a/arch/arm/include/asm/ftrace.h
+++ b/arch/arm/include/asm/ftrace.h
@@ -54,6 +54,24 @@ static inline void *return_address(unsigned int level)
 
 #define ftrace_return_address(n) return_address(n)
 
+#define ARCH_HAS_SYSCALL_MATCH_SYM_NAME
+
+static inline bool arch_syscall_match_sym_name(const char *sym,
+					       const char *name)
+{
+	if (!strcmp(sym, "sys_mmap2"))
+		sym = "sys_mmap_pgoff";
+	else if (!strcmp(sym, "sys_statfs64_wrapper"))
+		sym = "sys_statfs64";
+	else if (!strcmp(sym, "sys_fstatfs64_wrapper"))
+		sym = "sys_fstatfs64";
+	else if (!strcmp(sym, "sys_arm_fadvise64_64"))
+		sym = "sys_fadvise64_64";
+
+	/* Ignore case since sym may start with "SyS" instead of "sys" */
+	return !strcasecmp(sym, name);
+}
+
 #endif /* ifndef __ASSEMBLY__ */
 
 #endif /* _ASM_ARM_FTRACE */
-- 
2.1.4

^ permalink raw reply related

* [RESEND PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver
From: Anurup M @ 2016-11-15 10:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161110175510.GB10137@leverpostej>



On Thursday 10 November 2016 11:25 PM, Mark Rutland wrote:
> On Thu, Nov 03, 2016 at 01:41:59AM -0400, Anurup M wrote:
>> From: Tan Xiaojun <tanxiaojun@huawei.com>
>>
>> 	The Hisilicon Djtag is an independent component which connects
>> 	with some other components in the SoC by Debug Bus. This driver
>> 	can be configured to access the registers of connecting components
>> 	(like L3 cache) during real time debugging.
>>
> Just to check, is this likely to be used in multi-socket hardware, and
> if so, are instances always-on?
Yes, It could be used in multi-socket hardware also.
The sockets are always enabled  after bootup. Sorry I didn't get the
>> Signed-off-by: Tan Xiaojun <tanxiaojun@huawei.com>
>> Signed-off-by: John Garry <john.garry@huawei.com>
>> Signed-off-by: Anurup M <anurup.m@huawei.com>
>> ---
>>   drivers/soc/Kconfig                 |   1 +
>>   drivers/soc/Makefile                |   1 +
>>   drivers/soc/hisilicon/Kconfig       |  12 +
>>   drivers/soc/hisilicon/Makefile      |   1 +
>>   drivers/soc/hisilicon/djtag.c       | 639 ++++++++++++++++++++++++++++++++++++
>>   include/linux/soc/hisilicon/djtag.h |  38 +++
>>   6 files changed, 692 insertions(+)
>>   create mode 100644 drivers/soc/hisilicon/Kconfig
>>   create mode 100644 drivers/soc/hisilicon/Makefile
>>   create mode 100644 drivers/soc/hisilicon/djtag.c
>>   create mode 100644 include/linux/soc/hisilicon/djtag.h
> Other than the PMU driver(s), what is going to use this?
>
> If you don't have something in particular, please also place this under
> drivers/perf/hisilicon, along with the PMU driver(s).
>
> We can always move it later if necessary.
>
> [...]
Arnd also suggested the same. Currently as there are no other users I 
shall move it to
drivers/perf/hisilicon.
>> +#define SC_DJTAG_TIMEOUT		100000	/* 100ms */
> This would be better as:
>
> #define SC_DJTAG_TIMEOUT_US	(100 * USEC_PER_MSEC)
>
> (you'll need to include <linux/time64.h>)
>
> [...]
OK.
>> +static void djtag_read32_relaxed(void __iomem *regs_base, u32 off, u32 *value)
>> +{
>> +	void __iomem *reg_addr = regs_base + off;
>> +
>> +	*value = readl_relaxed(reg_addr);
>> +}
>> +
>> +static void djtag_write32(void __iomem *regs_base, u32 off, u32 val)
>> +{
>> +	void __iomem *reg_addr = regs_base + off;
>> +
>> +	writel(val, reg_addr);
>> +}
> I think these make the driver harder to read, especially given the read
> function is void and takes an output pointer.
>
> In either case you can call readl/writel directly with base + off for
> the __iomem ptr. Please do that.
Ok.
>> +
>> +/*
>> + * djtag_readwrite_v1/v2: djtag read/write interface
>> + * @reg_base:	djtag register base address
>> + * @offset:	register's offset
>> + * @mod_sel:	module selection
>> + * @mod_mask:	mask to select specific modules for write
>> + * @is_w:	write -> true, read -> false
>> + * @wval:	value to register for write
>> + * @chain_id:	which sub module for read
>> + * @rval:	value in register for read
>> + *
>> + * Return non-zero if error, else return 0.
>> + */
>> +static int djtag_readwrite_v1(void __iomem *regs_base, u32 offset, u32 mod_sel,
>> +		u32 mod_mask, bool is_w, u32 wval, int chain_id, u32 *rval)
>> +{
>> +	u32 rd;
>> +	int timeout = SC_DJTAG_TIMEOUT;
>> +
>> +	if (!(mod_mask & CHAIN_UNIT_CFG_EN)) {
>> +		pr_warn("djtag: do nothing.\n");
>> +		return 0;
>> +	}
>> +
>> +	/* djtag mster enable & accelerate R,W */
>> +	djtag_write32(regs_base, SC_DJTAG_MSTR_EN,
>> +			DJTAG_NOR_CFG | DJTAG_MSTR_EN);
>> +
>> +	/* select module */
>> +	djtag_write32(regs_base, SC_DJTAG_DEBUG_MODULE_SEL, mod_sel);
>> +	djtag_write32(regs_base, SC_DJTAG_CHAIN_UNIT_CFG_EN,
>> +				mod_mask & CHAIN_UNIT_CFG_EN);
>> +
>> +	if (is_w) {
>> +		djtag_write32(regs_base, SC_DJTAG_MSTR_WR, DJTAG_MSTR_W);
>> +		djtag_write32(regs_base, SC_DJTAG_MSTR_DATA, wval);
>> +	} else
>> +		djtag_write32(regs_base, SC_DJTAG_MSTR_WR, DJTAG_MSTR_R);
>> +
>> +	/* address offset */
>> +	djtag_write32(regs_base, SC_DJTAG_MSTR_ADDR, offset);
>> +
>> +	/* start to write to djtag register */
>> +	djtag_write32(regs_base, SC_DJTAG_MSTR_START_EN, DJTAG_MSTR_START_EN);
>> +
>> +	/* ensure the djtag operation is done */
>> +	do {
>> +		djtag_read32_relaxed(regs_base, SC_DJTAG_MSTR_START_EN, &rd);
>> +		if (!(rd & DJTAG_MSTR_EN))
>> +			break;
>> +
>> +		udelay(1);
>> +	} while (timeout--);
>> +
>> +	if (timeout < 0) {
>> +		pr_err("djtag: %s timeout!\n", is_w ? "write" : "read");
>> +		return -EBUSY;
>> +	}
>> +
>> +	if (!is_w)
>> +		djtag_read32_relaxed(regs_base,
>> +			SC_DJTAG_RD_DATA_BASE + chain_id * 0x4, rval);
>> +
>> +	return 0;
>> +}
> Please factor out the common bits into helpers and have separate
> read/write functions. It's incredibly difficult to follow the code with
> read/write hidden behind a boolean parameter.
Ok. Shall change it.
>> +static const struct of_device_id djtag_of_match[] = {
>> +	/* for hip05(D02) cpu die */
>> +	{ .compatible = "hisilicon,hip05-cpu-djtag-v1",
>> +		.data = (void *)djtag_readwrite_v1 },
>> +	/* for hip05(D02) io die */
>> +	{ .compatible = "hisilicon,hip05-io-djtag-v1",
>> +		.data = (void *)djtag_readwrite_v1 },
>> +	/* for hip06(D03) cpu die */
>> +	{ .compatible = "hisilicon,hip06-cpu-djtag-v1",
>> +		.data = (void *)djtag_readwrite_v1 },
>> +	/* for hip06(D03) io die */
>> +	{ .compatible = "hisilicon,hip06-io-djtag-v2",
>> +		.data = (void *)djtag_readwrite_v2 },
>> +	/* for hip07(D05) cpu die */
>> +	{ .compatible = "hisilicon,hip07-cpu-djtag-v2",
>> +		.data = (void *)djtag_readwrite_v2 },
>> +	/* for hip07(D05) io die */
>> +	{ .compatible = "hisilicon,hip07-io-djtag-v2",
>> +		.data = (void *)djtag_readwrite_v2 },
>> +	{},
>> +};
> Binding documentation for all of these should be added *before* this
> patch, per Documentation/devicetree/bindings/submitting-patches.txt.
> Please move any relevant binding documentation earlier in the series.
The binding documentation added in "[RESEND PATCH v1 02/11] dt-bindings: 
hisi: Add Hisilicon HiP05/06/07 Sysctrl and Djtag dts bindings]".

>> +MODULE_DEVICE_TABLE(of, djtag_of_match);
>> +
>> +static ssize_t
>> +show_modalias(struct device *dev, struct device_attribute *attr, char *buf)
>> +{
>> +	struct hisi_djtag_client *client = to_hisi_djtag_client(dev);
>> +
>> +	return sprintf(buf, "%s%s\n", MODULE_PREFIX, client->name);
>> +}
>> +static DEVICE_ATTR(modalias, 0444, show_modalias, NULL);
>> +
>> +static struct attribute *hisi_djtag_dev_attrs[] = {
>> +	NULL,
>> +	/* modalias helps coldplug:  modprobe $(cat .../modalias) */
>> +	&dev_attr_modalias.attr,
>> +	NULL
>> +};
>> +ATTRIBUTE_GROUPS(hisi_djtag_dev);
> Why do we need to expose this under sysfs?
This is to know the djtag clients registered with the bus.
>> +struct bus_type hisi_djtag_bus = {
>> +	.name		= "hisi-djtag",
>> +	.match		= hisi_djtag_device_match,
>> +	.probe		= hisi_djtag_device_probe,
>> +	.remove		= hisi_djtag_device_remove,
>> +};
> I take it the core bus code handles reference-counting users of the bus?
The bus_register registers with kobject. Did It answer the question?
>> +struct hisi_djtag_client *hisi_djtag_new_device(struct hisi_djtag_host *host,
>> +						struct device_node *node)
>> +{
>> +	struct hisi_djtag_client *client;
>> +	int status;
>> +
>> +	client = kzalloc(sizeof(*client), GFP_KERNEL);
>> +	if (!client)
>> +		return NULL;
>> +
>> +	client->host = host;
>> +
>> +	client->dev.parent = &client->host->dev;
>> +	client->dev.bus = &hisi_djtag_bus;
>> +	client->dev.type = &hisi_djtag_client_type;
>> +	client->dev.of_node = node;
> I suspect that we should do:
>
> 	client->dev.of_node = of_node_get(node);
>
> ... so that it's guaranteed we retain a reference.
Ok.
>> +	snprintf(client->name, DJTAG_CLIENT_NAME_LEN, "%s%s",
>> +					DJTAG_PREFIX, node->name);
>> +	dev_set_name(&client->dev, "%s", client->name);
>> +
>> +	status = device_register(&client->dev);
>> +	if (status < 0) {
>> +		pr_err("error adding new device, status=%d\n", status);
>> +		kfree(client);
>> +		return NULL;
>> +	}
>> +
>> +	return client;
>> +}
>> +
>> +static struct hisi_djtag_client *hisi_djtag_of_register_device(
>> +						struct hisi_djtag_host *host,
>> +						struct device_node *node)
>> +{
>> +	struct hisi_djtag_client *client;
>> +
>> +	client = hisi_djtag_new_device(host, node);
>> +	if (client == NULL) {
>> +		dev_err(&host->dev, "error registering device %s\n",
>> +			node->full_name);
>> +		of_node_put(node);
> I don't think this should be here, given what djtag_register_devices()
> does.
Will move this to djtag_register_devices.
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> +
>> +	return client;
>> +}
>> +
>> +static void djtag_register_devices(struct hisi_djtag_host *host)
>> +{
>> +	struct device_node *node;
>> +	struct hisi_djtag_client *client;
>> +
>> +	if (!host->of_node)
>> +		return;
>> +
>> +	for_each_available_child_of_node(host->of_node, node) {
>> +		if (of_node_test_and_set_flag(node, OF_POPULATED))
>> +			continue;
>> +		client = hisi_djtag_of_register_device(host, node);
>> +		list_add(&client->next, &host->client_list);
>> +	}
>> +}
> Given hisi_djtag_of_register_device() can return ERR_PTR(-EINVAL), the
> list_add is not safe.
Shall add the check and handle accordingly.
>> +static int djtag_host_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct hisi_djtag_host *host;
>> +	const struct of_device_id *of_id;
>> +	struct resource *res;
>> +	int rc;
>> +
>> +	of_id = of_match_device(djtag_of_match, dev);
>> +	if (!of_id)
>> +		return -EINVAL;
>> +
>> +	host = kzalloc(sizeof(*host), GFP_KERNEL);
>> +	if (!host)
>> +		return -ENOMEM;
>> +
>> +	host->of_node = dev->of_node;
> 	host->of_node = of_node_get(dev->of_node);
>
>> +	host->djtag_readwrite = of_id->data;
>> +	spin_lock_init(&host->lock);
>> +
>> +	INIT_LIST_HEAD(&host->client_list);
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	if (!res) {
>> +		dev_err(&pdev->dev, "No reg resorces!\n");
>> +		kfree(host);
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (!resource_size(res)) {
>> +		dev_err(&pdev->dev, "Zero reg entry!\n");
>> +		kfree(host);
>> +		return -EINVAL;
>> +	}
>> +
>> +	host->sysctl_reg_map = devm_ioremap_resource(dev, res);
>> +	if (IS_ERR(host->sysctl_reg_map)) {
>> +		dev_warn(dev, "Unable to map sysctl registers.\n");
>> +		kfree(host);
>> +		return -EINVAL;
>> +	}
> Please have a common error path rather than duplicating this free.
>
> e.g. at the end of the function have:
>
> 	err_free:
> 		kfree(host);
> 		return err;
>
> ... and in cases like this, have:
>
> 	if (whatever()) {
> 		dev_warn(dev, "this failed xxx\n");
> 		err = -EINVAL;
> 		goto err_free;
> 	}
Ok. thanks. will change it.

Thanks,
Anurup
> Thanks,
> Mark.

^ permalink raw reply

* [PATCH v7 00/16] ACPI IORT ARM SMMU support
From: Lorenzo Pieralisi @ 2016-11-15 10:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAJZ5v0j09fsp4Y66v2UANOGnW4FNO7PWxpZY=Pgw=bUccEL_eA@mail.gmail.com>

Hi Rafael,

On Thu, Nov 10, 2016 at 12:36:12AM +0100, Rafael J. Wysocki wrote:
> Hi Lorenzo,
> 
> On Wed, Nov 9, 2016 at 3:19 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > This patch series is v7 of a previous posting:
> >
> > https://lkml.org/lkml/2016/10/18/506
> 
> I don't see anything objectionable in this series.
> 
> Please let me know which patches in particular to look at in detail.

Are patches 7 and consequently 16 (that builds on top of 7) ok with
you ? They should be equivalent to nop on anything other than ARM64
but they touch ACPI core so they require an ACK if they are ok please.

Thank you very much !
Lorenzo

^ permalink raw reply

* [PATCH 1/2] ARM: dts: sun5i: Add touchscreen node to reference-design-tablet.dtsi
From: Hans de Goede @ 2016-11-15 10:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161114200802.vdduhyxnkgmpsyd5@lukather>

Hi,

On 14-11-16 21:08, Maxime Ripard wrote:
> Hi,
>
> On Sun, Nov 13, 2016 at 08:22:02PM +0100, Hans de Goede wrote:
>> Just like on sun8i all sun5i tablets use the same interrupt and power
>> gpios for their touchscreens. I've checked all known a13 fex files and
>> only the UTOO P66 uses a different gpio for the interrupt.
>>
>> Add a touchscreen node to sun5i-reference-design-tablet.dtsi, which
>> fills in the necessary gpios to avoid duplication in the tablet dts files,
>> just like we do in sun8i-reference-design-tablet.dtsi.
>>
>> This will make future patches adding touchscreen nodes to a13 tablets
>> simpler.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  arch/arm/boot/dts/sun5i-a13-utoo-p66.dts           | 38 ++++++++--------------
>>  .../boot/dts/sun5i-reference-design-tablet.dtsi    | 25 ++++++++++++++
>>  2 files changed, 39 insertions(+), 24 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts b/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts
>> index a8b0bcc..3d7ff10 100644
>> --- a/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts
>> +++ b/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts
>> @@ -83,22 +83,6 @@
>>  	allwinner,pins = "PG3";
>>  };
>>
>> -&i2c1 {
>> -	icn8318: touchscreen at 40 {
>> -		compatible = "chipone,icn8318";
>> -		reg = <0x40>;
>> -		interrupt-parent = <&pio>;
>> -		interrupts = <6 9 IRQ_TYPE_EDGE_FALLING>; /* EINT9 (PG9) */
>> -		pinctrl-names = "default";
>> -		pinctrl-0 = <&ts_wake_pin_p66>;
>> -		wake-gpios = <&pio 1 3 GPIO_ACTIVE_HIGH>; /* PB3 */
>> -		touchscreen-size-x = <800>;
>> -		touchscreen-size-y = <480>;
>> -		touchscreen-inverted-x;
>> -		touchscreen-swapped-x-y;
>> -	};
>> -};
>> -
>>  &mmc2 {
>>  	pinctrl-names = "default";
>>  	pinctrl-0 = <&mmc2_pins_a>;
>> @@ -121,20 +105,26 @@
>>  		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
>>  		allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
>>  	};
>> -
>> -	ts_wake_pin_p66: ts_wake_pin at 0 {
>> -		allwinner,pins = "PB3";
>> -		allwinner,function = "gpio_out";
>> -		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
>> -		allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
>> -	};
>> -
>>  };
>>
>>  &reg_usb0_vbus {
>>  	gpio = <&pio 1 4 GPIO_ACTIVE_HIGH>; /* PB4 */
>>  };
>>
>> +&touchscreen {
>> +	compatible = "chipone,icn8318";
>> +	reg = <0x40>;
>> +	/* The P66 uses a different EINT then the reference design */
>> +	interrupts = <6 9 IRQ_TYPE_EDGE_FALLING>; /* EINT9 (PG9) */
>> +	/* The icn8318 binding expects wake-gpios instead of power-gpios */
>> +	wake-gpios = <&pio 1 3 GPIO_ACTIVE_HIGH>; /* PB3 */
>> +	touchscreen-size-x = <800>;
>> +	touchscreen-size-y = <480>;
>> +	touchscreen-inverted-x;
>> +	touchscreen-swapped-x-y;
>> +	status = "okay";
>> +};
>> +
>>  &uart1 {
>>  	/* The P66 uses the uart pins as gpios */
>>  	status = "disabled";
>> diff --git a/arch/arm/boot/dts/sun5i-reference-design-tablet.dtsi b/arch/arm/boot/dts/sun5i-reference-design-tablet.dtsi
>> index 20cc940..7af488a 100644
>> --- a/arch/arm/boot/dts/sun5i-reference-design-tablet.dtsi
>> +++ b/arch/arm/boot/dts/sun5i-reference-design-tablet.dtsi
>> @@ -41,6 +41,7 @@
>>   */
>>  #include "sunxi-reference-design-tablet.dtsi"
>>
>> +#include <dt-bindings/interrupt-controller/irq.h>
>>  #include <dt-bindings/pwm/pwm.h>
>>
>>  / {
>> @@ -84,6 +85,23 @@
>>  };
>>
>>  &i2c1 {
>> +	/*
>> +	 * The gsl1680 is rated at 400KHz and it will not work reliable at
>> +	 * 100KHz, this has been confirmed on multiple different q8 tablets.
>> +	 * All other devices on this bus are also rated for 400KHz.
>> +	 */
>> +	clock-frequency = <400000>;
>> +
>> +	touchscreen: touchscreen {
>> +		interrupt-parent = <&pio>;
>> +		interrupts = <6 11 IRQ_TYPE_EDGE_FALLING>; /* EINT11 (PG11) */
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&ts_power_pin>;
>> +		power-gpios = <&pio 1 3 GPIO_ACTIVE_HIGH>; /* PB3 */
>> +		/* Tablet dts must provide reg and compatible */
>> +		status = "disabled";
>> +	};
>> +
>>  	pcf8563: rtc at 51 {
>>  		compatible = "nxp,pcf8563";
>>  		reg = <0x51>;
>> @@ -125,6 +143,13 @@
>>  		allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
>>  	};
>>
>> +	ts_power_pin: ts_power_pin {
>> +		allwinner,pins = "PB3";
>> +		allwinner,function = "gpio_out";
>> +		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
>> +		allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
>> +	};
>> +
>
> For the next release, we'll switch to the generic pin mux properties
> ("pins" and "function"), and we actually implemented the fact that the
> drive and pull properties are optional, so you can drop them both.
>
> You'll need next + http://lists.infradead.org/pipermail/linux-arm-kernel/2016-November/467123.html

Ok, before I send a v2 first a question about this, for the touchscreen
case I actually need:

		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
		allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;

Because otherwise when the touchscreen controller is powered by a separate
regulator and that regulator is off, then it may draw just enough current
from its enable pin to be sort-of listening to the i2c bus and mess up
that bus.

So is this the default, or do we get the power-on default when not
specifying these? If it is the power-on default then we do need to
specify these, because AFAICT the power-on drive strength typically
is 20 mA.

Regards,

Hans

^ permalink raw reply

* [PATCH 2/2] ARM: davinci_all_defconfig: enable the mstpri and ddrctl drivers
From: Sekhar Nori @ 2016-11-15 10:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479144724-14231-3-git-send-email-bgolaszewski@baylibre.com>

On Monday 14 November 2016 11:02 PM, Bartosz Golaszewski wrote:
> With the da8xx memory controller and master peripheral priority
> drivers merged and corresponding device tree changes in place we can
> now enable appropriate options by default.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Applied to v4.10/defconfig

Thanks,
Sekhar

^ permalink raw reply

* PM regression with LED changes in next-20161109
From: Hans de Goede @ 2016-11-15 10:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <6d88b2a7-a506-89bf-fc76-af707ed7a00f@samsung.com>

Hi,

On 15-11-16 11:01, Jacek Anaszewski wrote:
> Hi,
>
> On 11/14/2016 01:51 PM, Hans de Goede wrote:

<snip>

>> Ugh, I see I accidentally send a v4 twice, instead of
>> calling the version which dropped those called v5 as
>> I should have, sorry.
>>
>> The v4 which I would like to see merged, the one with
>> those calls dropped, is here:
>>
>> https://patchwork.kernel.org/patch/9423093/
>
> Right I've had an impression that I've already seen something
> different than "first" v4.
>
> Regarding the patch - adding led_notify_brightness_change() to
> brightness_store() can have similar power consumption related
> implications if brightness is set frequently via sysfs.

That means that userspace is waking up frequently to write the
sysfs file, so in that case userspace is already draining
a lot of energy, so I don't think that is something we need to
worry about.

> I'm leaning
> towards adding a new brightness file similar to user_brightness
> discussed in this thread.
>
> It would cover shortcomings and read/write inconsistencies that
> brightness file currently has, but without breaking existing users.
>
> I'd not however go for "user_brightness" name due to the possible
> brightness adjustments made autonomously by firmware. I'm afraid
> that devising a meaningful name for the new file will be hard,
> so the simplest would be just brighntess2. Dedicated section
> in leds-class.txt should be devoted to it.

Ok, let me quote myself from another part of this thread:

We've 2 sorts of brightness really:

1) transient brightness, aka current brightness, when blinking or
triggers are used this will switch many times a second
between off and some on level.

2) non-transient brightness, for non blinking leds this is the
actual brightness, for blinking leds this is the brightness
level used when the led is on.

Now we want to have a sysfs attribute reflecting 2, so that
userspace can poll on that, both for my use-case as well as so
that userspace process a can detect changes made by writing to
the brightness file by process b.

So maybe we need to simply call the new attribute
non_transient_brightness instead of user_brightness?

non_transient_brightness certainly seems like a better name
then brightness2 ?

Regards,

Hans

^ permalink raw reply

* [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic
From: Lorenzo Pieralisi @ 2016-11-15 10:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <313844ca-d948-1297-84b2-669f3a7d57d2@arm.com>

On Mon, Nov 14, 2016 at 06:25:16PM +0000, Robin Murphy wrote:
> On 14/11/16 15:52, Joerg Roedel wrote:
> > On Mon, Nov 14, 2016 at 12:00:47PM +0000, Robin Murphy wrote:
> >> If we've already made the decision to move away from bus ops, I don't
> >> see that it makes sense to deliberately introduce new dependencies on
> >> them. Besides, as it stands, this patch literally implements "tell the
> >> iommu-core which hardware-iommus exist in the system and a seperate
> >> iommu_ops ptr for each of them" straight off.
> > 
> > Not sure which code you are looking at, but as I see it we have only
> > per-device iommu-ops now (with this patch). That is different from
> > having core-visible hardware-iommu instances where devices could link
> > to.
> 
> The per-device IOMMU ops are already there since 57f98d2f61e1. This
> patch generalises the other end, moving the "registering an IOMMU
> instance" (i.e. iommu_fwentry) bit into the IOMMU core, from being
> OF-specific. I'd be perfectly happy if we rename iommu_fwentry to
> iommu_instance, fwnode_iommu_set_ops() to iommu_register_instance(), and
> such if that makes the design intent clearer.

I second that and I need to know what to do with this patch sooner
rather than later so it is time we make a decision please.

Joerg, what's your opinion ?

Thanks,
Lorenzo

> If you'd also prefer to replace iommu_fwspec::ops with an opaque
> iommu_fwspec::iommu_instance pointer so that things are a bit more
> centralised (and users are forced to go through the API rather then call
> ops directly), I'd have no major objection either. My main point is that
> we've been deliberately putting the relevant building blocks in place -
> the of_iommu_{get,set}_ops stuff was designed from the start to
> accommodate per-instance ops, via the ops pointer *being* the instance
> token; the iommu_fwspec stuff is deliberately intended to provide
> per-device ops on top of that. The raw functionality is either there in
> iommu.c already, or moving there in patches already written, so if it
> doesn't look right all we need to focus on is making it look right.
> 
> > Also the rest of iommu-core code still makes use of the per-bus ops. The
> > per-device ops are only used for the of_xlate fn-ptr.
> 
> Hence my aforementioned patches intended for 4.10, directly following on
> from introducing iommu_fwspec in 4.9:
> 
> http://www.mail-archive.com/iommu at lists.linux-foundation.org/msg14576.html
> 
> ...the purpose being to provide a smooth transition from per-bus ops to
> per-device, per-instance ops. Apply those and we're 90% of the way there
> for OF-based IOMMU drivers (not that any of those actually need
> per-instance ops, admittedly; I did prototype it for the ARM SMMU ages
> ago, but it didn't seem worth the bother). Lorenzo's series broadens the
> scope to ACPI-based systems and moves the generically-useful parts into
> the core where we can easily build on them further if necessary. The
> major remaining work is to convert external callers of the current
> bus-dependent functions like iommu_domain_alloc(), iommu_present(), etc.
> to device-based alternatives.
> 
> Robin.

^ permalink raw reply

* [PATCH 1/2] ARM: dts: da850: add the mstpri and ddrctl nodes
From: Sekhar Nori @ 2016-11-15 10:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479144724-14231-2-git-send-email-bgolaszewski@baylibre.com>

On Monday 14 November 2016 11:02 PM, Bartosz Golaszewski wrote:
> Add the nodes for the MSTPRI configuration and DDR2/mDDR memory
> controller drivers to da850.dtsi.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  arch/arm/boot/dts/da850.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index 1bb1f6d..1635218 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -440,6 +440,11 @@
>  			interrupts = <52>;
>  			status = "disabled";
>  		};
> +
> +		mstpri: mstpri at 14110 {
> +			compatible = "ti,da850-mstpri";
> +			reg = <0x14110 0x0c>;
> +		};

Instead of adding the node to the end, can you place it above the
cfgchip node to keep it sorted by reg. We have not really followed that
in this file. May be we should have. But lets start with this.

>  	};
>  	aemif: aemif at 68000000 {
>  		compatible = "ti,da850-aemif";
> @@ -451,4 +456,8 @@
>  			  1 0 0x68000000 0x00008000>;
>  		status = "disabled";
>  	};
> +	ddrctl: ddrctl at b0000000 {
> +		compatible = "ti,da850-ddr-controller";
> +		reg = <0xb0000000 0xe8>;
> +	};

Can you name the node memory-controller at b0000000? Thats the generic name
suggested by ePAPR 1.1 for memory controllers.

I could not find any naming suggestions for the bus master priority
controller above, but based on the pattern used for other controllers,
may be it should be called priority-controller at 14110 instead of mstpri at 14110

Thanks,
Sekhar

^ permalink raw reply

* [PATCH] ARM: dma-mapping: preallocate DMA-debug hash tables in pure_initcall
From: Marek Szyprowski @ 2016-11-15 10:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CGME20161115100459eucas1p152b644d5d329192a0902c836b13f35ba@eucas1p1.samsung.com>

fs_initcall is definitely too late to initialize DMA-debug hash tables,
because some drivers might get probed and use DMA mapping framework
already in core_initcall. Late initialization of DMA-debug results in
false warning about accessing memory, that was not allocated, like this
one:
------------[ cut here ]------------
WARNING: CPU: 5 PID: 1 at lib/dma-debug.c:1104 check_unmap+0xa1c/0xe50
exynos-sysmmu 10a60000.sysmmu: DMA-API: device driver tries to free DMA memory it has not allocated [device
address=0x000000006ebd0000] [size=16384 bytes]
Modules linked in:
CPU: 5 PID: 1 Comm: swapper/0 Not tainted 4.9.0-rc5-00028-g39dde3d-dirty #44
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[<c0119dd4>] (unwind_backtrace) from [<c01122bc>] (show_stack+0x20/0x24)
[<c01122bc>] (show_stack) from [<c062714c>] (dump_stack+0x84/0xa0)
[<c062714c>] (dump_stack) from [<c0132560>] (__warn+0x14c/0x180)
[<c0132560>] (__warn) from [<c01325dc>] (warn_slowpath_fmt+0x48/0x50)
[<c01325dc>] (warn_slowpath_fmt) from [<c06814f8>] (check_unmap+0xa1c/0xe50)
[<c06814f8>] (check_unmap) from [<c06819c4>] (debug_dma_unmap_page+0x98/0xc8)
[<c06819c4>] (debug_dma_unmap_page) from [<c076c3e8>] (exynos_iommu_domain_free+0x158/0x380)
[<c076c3e8>] (exynos_iommu_domain_free) from [<c0764a30>] (iommu_domain_free+0x34/0x60)
[<c0764a30>] (iommu_domain_free) from [<c011f168>] (release_iommu_mapping+0x30/0xb8)
[<c011f168>] (release_iommu_mapping) from [<c011f23c>] (arm_iommu_release_mapping+0x4c/0x50)
[<c011f23c>] (arm_iommu_release_mapping) from [<c0b061ac>] (s5p_mfc_probe+0x640/0x80c)
[<c0b061ac>] (s5p_mfc_probe) from [<c07e6750>] (platform_drv_probe+0x70/0x148)
[<c07e6750>] (platform_drv_probe) from [<c07e25c0>] (driver_probe_device+0x12c/0x6b0)
[<c07e25c0>] (driver_probe_device) from [<c07e2c6c>] (__driver_attach+0x128/0x17c)
[<c07e2c6c>] (__driver_attach) from [<c07df74c>] (bus_for_each_dev+0x88/0xc8)
[<c07df74c>] (bus_for_each_dev) from [<c07e1b6c>] (driver_attach+0x34/0x58)
[<c07e1b6c>] (driver_attach) from [<c07e1350>] (bus_add_driver+0x18c/0x32c)
[<c07e1350>] (bus_add_driver) from [<c07e4198>] (driver_register+0x98/0x148)
[<c07e4198>] (driver_register) from [<c07e5cb0>] (__platform_driver_register+0x58/0x74)
[<c07e5cb0>] (__platform_driver_register) from [<c174cb30>] (s5p_mfc_driver_init+0x1c/0x20)
[<c174cb30>] (s5p_mfc_driver_init) from [<c0102690>] (do_one_initcall+0x64/0x258)
[<c0102690>] (do_one_initcall) from [<c17014c0>] (kernel_init_freeable+0x3d0/0x4d0)
[<c17014c0>] (kernel_init_freeable) from [<c116eeb4>] (kernel_init+0x18/0x134)
[<c116eeb4>] (kernel_init) from [<c010bbd8>] (ret_from_fork+0x14/0x3c)
---[ end trace dc54c54bd3581296 ]---

This patch moves initialization of DMA-debug to pure_initcall.

Reported-by: Seung-Woo Kim <sw0312.kim@samsung.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/mm/dma-mapping.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index ab4f745..d1abbcf 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1167,7 +1167,7 @@ static int __init dma_debug_do_init(void)
 	dma_debug_init(PREALLOC_DMA_DEBUG_ENTRIES);
 	return 0;
 }
-fs_initcall(dma_debug_do_init);
+pure_initcall(dma_debug_do_init);
 
 #ifdef CONFIG_ARM_DMA_USE_IOMMU
 
-- 
1.9.1

^ permalink raw reply related

* PM regression with LED changes in next-20161109
From: Jacek Anaszewski @ 2016-11-15 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <15cafbf5-d842-e184-2fd4-65f8272f505a@redhat.com>

Hi,

On 11/14/2016 01:51 PM, Hans de Goede wrote:
> Hi,
>
> On 14-11-16 10:12, Jacek Anaszewski wrote:
>> Hi,
>>
>> On 11/13/2016 02:52 PM, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 13-11-16 12:44, Jacek Anaszewski wrote:
>>>> Hi,
>>>>
>>>> On 11/12/2016 10:14 PM, Hans de Goede wrote:
>>>
>>> <snip>
>>>
>>>>>>> So I would like to propose creating a new read-write
>>>>>>> user_brightness file.
>>>>>>>
>>>>>>> The write behavior would be 100% identical to the brightness
>>>>>>> file (in code terms it will call the same store function).
>>>>>>>
>>>>>>> The the read behavior otoh will be different: it will shows
>>>>>>> the last brightness as set by the user, this would show the
>>>>>>> read behavior we really want of brightness: show the real
>>>>>>> brightness when not blinking / triggers are active and show
>>>>>>> the brightness used when on when blinking / triggers are active.
>>>>>>>
>>>>>>> We could then add poll support on this new user_brightness
>>>>>>> file, thus avoiding the problem with the extra cpu-load on
>>>>>>> notifications on blinking / triggers.
>>>>>>
>>>>>> I agree that user_brightness allows to solve the issues you raised
>>>>>> about inconsistent write and read brightness' semantics
>>>>>> (which is not that painful IMHO).
>>>>>>
>>>>>> Reporting non-user brightness changes on user_brightness file
>>>>>> doesn't sound reasonable though.
>>>>>
>>>>> The changes I'm interested in are user brightness changes they
>>>>> are just not done through sysfs, but through a hardwired hotkey,
>>>>> they are however very much done by the user.
>>>>
>>>> Ah, so this file name would be misleading especially taking into
>>>> account
>>>> the context in which "user" is used in kernel, which predominantly
>>>> means "userspace", e.g. copy_to_user(), copy_from_user().
>>>>
>>>>>> Also, how would we read the
>>>>>> brightness set by the firmware? We'd have to read brightness
>>>>>> file, so still two files would have to be opened which is
>>>>>> a second drawback of this approach.
>>>>>
>>>>> No, look carefully at the definition of the read behavior
>>>>> I plan to put in the ABI doc:
>>>>
>>>> OK, "user" was what confused me. So in this case changes made
>>>> by the firmware even if in a result of user activity
>>>> (pressing hardware key) obviously cannot be treated similarly
>>>> to the changes made from the userspace context.
>>>
>>> In the end both result on the brightness of the device
>>> changing, so any userspace process interested in monitoring
>>> the brightness will want to know about both type of changes.
>>>
>>>> Unless you're able to give references to the kernel code which
>>>> contradict my judgement.
>>>
>>> AFAIK the audio code will signal volume changes done by
>>> hardwired buttons the same way as audio changes done
>>> by userspace calling into the kernel. This also makes
>>> sense because in the end, what is interesting for a
>>> mixer app, is that the volume changed, and what the
>>> new volume is.
>>
>> OK, so it is indeed similar to your LED use case. Nonetheless
>> in case of LED controllers it is also possible that hardware
>> adjusts LED brightness in case of low battery voltage.
>>
>> If a device is able e.g. to generate an interrupt to notify this
>> kind of event, then we would like also to be able to notify the client
>> about that. It wouldn't be user generated brightness change though.
>>
>>>>> "Reading this file will return the actual led brightness
>>>>> when not blinking and no triggers are active; reading this
>>>>> file will return the brightness used when the led is on
>>>>> when blinking or triggers are active."
>>>>
>>>> This is unnecessarily entangled. Blinking means timer trigger
>>>> is active.
>>>
>>> Ok.
>>>
>>>>> So for e.g. the backlit keyboard case reading this single
>>>>> file will return the actual brightness of the backlight,
>>>>> since this does not involve blinking or triggers.
>>>>>
>>>>> Basically the idea is that the user_brightness file
>>>>> will have the semantics which IMHO the brightness file
>>>>> itself should have had from the beginning, but which
>>>>> we can't change now due to ABI reasons.
>>>>
>>>> And in fact introducing user_brightness file would indeed
>>>> fix that shortcoming. However without providing notifications
>>>> of hw brightness changes on it.
>>>
>>> See above, I believe such a file should report any
>>> changes in brightness, except those caused by triggers,
>>> so it would report hw brightness changes.
>>>
>>> Anyways if you're not interested in fixing the
>>> shortcomings of the current read behavior on the
>>> brightness file (I'm fine with that, I can live
>>> with the shortcomings) I suggest that we simply go
>>> with v2 of my poll() patch.
>>
>> v2 entails power consumption related issues.
>>
>> Generally I think that we could add the file you proposed,
>> however it would be good to devise a name which will cover
>> also the cases when brightness is changed by firmware without
>> user interaction.
>>
>>>>>> Having no difference in this area between the two approaches
>>>>>> I'm still in favour of the read-only file for notifying
>>>>>> brightness changes procured by hardware.
>>>>>
>>>>> That brings back the needing 2 fds problem; and does
>>>>> not solve userspace not being able to reliably read
>>>>> the led on brightness when blinking or using triggers.
>>>>>
>>>>> And this also has the issue that one is doing poll() on
>>>>> one fd to detect changes on another fd,
>>>>
>>>> It is not necessarily true. We can treat the polling on
>>>> hw_brightness_change file as a means to detect brightness
>>>> changes procured by hardware and we can read that brightness
>>>> by executing read on this same fd. It could return -ENODATA
>>>> if no such an event has occurred so far.
>>>
>>> That would still require 2 fds as userspace also wants to
>>> be able to set the keyboard backlight, but allowing read()
>>> on the hw_brightness_change file at least fixes the weirdness
>>> where userspace gets woken from poll() without being able to
>>> read. So if you insist on going the hw_brightness_change file
>>> route, then I can live with that (and upower will simply
>>> need to open 2 fds, that is doable).
>>>
>>> But, BUT, I would greatly prefer to just go for v4 of my
>>> patch, which fixes the only real problem we've seen with
>>> my patch as original merged without adding a new, somewhat
>>> convoluted sysfs attribute.
>>
>> Hmm, v4 still calls led_notify_brightness_change(led_cdev)
>> from both __led_set_brightness() and __led_set_brightness_blocking().
>
> Ugh, I see I accidentally send a v4 twice, instead of
> calling the version which dropped those called v5 as
> I should have, sorry.
>
> The v4 which I would like to see merged, the one with
> those calls dropped, is here:
>
> https://patchwork.kernel.org/patch/9423093/

Right I've had an impression that I've already seen something
different than "first" v4.

Regarding the patch - adding led_notify_brightness_change() to
brightness_store() can have similar power consumption related
implications if brightness is set frequently via sysfs. I'm leaning
towards adding a new brightness file similar to user_brightness
discussed in this thread.

It would cover shortcomings and read/write inconsistencies that
brightness file currently has, but without breaking existing users.

I'd not however go for "user_brightness" name due to the possible
brightness adjustments made autonomously by firmware. I'm afraid
that devising a meaningful name for the new file will be hard,
so the simplest would be just brighntess2. Dedicated section
in leds-class.txt should be devoted to it.

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply

* [PATCH v2] staging: vc04_services: rework ioctl code path
From: Greg KH @ 2016-11-15  9:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161111061531.23507-1-mzoran@crowfest.net>

On Thu, Nov 10, 2016 at 10:15:31PM -0800, Michael Zoran wrote:
> VCHIQ/vc04_services has a userland device interface
> that includes ioctls. The current ioctl implementation
> is a single monolithic function over 1,000+ lines
> that handles 17 different ioctls through a complex
> maze of switch and if statements.
> 
> The change reimplements that code path by breaking
> up the code into smaller, easier to maintain functions
> and uses a dispatch table to invoke the correct function.
> 
> Testing:
> 
> 1. vchiq_test -f 10 and vchiq_test -p 1 were run from a native
> 64-bit OS(debian sid).
> 
> 2. vchiq_test -f 10 and vchiq_test -p 1 where run from a 32-bit
> chroot install from the same OS.
> 
> Both test cases pass.
> 
> This is V2 of this patch.  Changes include:
> 
> 1. More code has been moved to the dispatch routine.
> The dispatch routine is now responsible for copying the top-level
> data into and out of kernel space by using the data encoded in
> the ioctl command number.
> 
> 2. The number of parameters have been reduced for the handling
> functions by giving a different prototype to ioctls that pass
> no arguments.
> 
> 3. Macros in linux/compat.h are now used for compatibility data
> structures.
> 
> Signed-off-by: Michael Zoran <mzoran@crowfest.net>
> ---
>  .../vc04_services/interface/vchiq_arm/vchiq_arm.c  | 1733 +++++++++++++-------
>  .../interface/vchiq_arm/vchiq_ioctl.h              |   96 ++
>  2 files changed, 1200 insertions(+), 629 deletions(-)

This is a rough patch to review, can you break this up into a patch
series that moves each ioctl to a separate function as needed?

thanks,

greg k-h

^ permalink raw reply

* [RESEND PATCH v1 05/11] dt-bindings: perf: hisi: Add Devicetree bindings for Hisilicon SoC PMU
From: Mark Rutland @ 2016-11-15  9:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <58290014.2020401@gmail.com>

On Mon, Nov 14, 2016 at 05:36:44AM +0530, Anurup M wrote:
> On Friday 11 November 2016 12:00 AM, Mark Rutland wrote:
> >On Thu, Nov 03, 2016 at 01:42:01AM -0400, Anurup M wrote:

> >>+	- scl-id : The Super Cluster ID. This can be the ID of the CPU die
> >>+		   or IO die in the chip.
> >What's this needed for?
> This is used as suffix to the PMU name. hisi_l3c<scl-id>. (hisi_l3c2
> - for scl-id = 2).
> This is to identify the pmu correspond to which CPU die in the socket.
> >>+	- num-events : No of events supported by this PMU device.
> >>+
> >>+	- num-counters : No of hardware counters available for counting.
> >This isn't probeable or well-known?
> My idea is to have the common properties of SoC PMU added here.
> The num-events, num-counters etc. So that handling can be made
> common in the driver.
> Is it not recommended? Please share your comments.

This feels like something that should be well-known for the programming
model of the device. If the number of events and/or counters shange, I'd
expect other things to also change such that the device is no longer
compatible with previous versions.

[...]

> The below two properties (module-id, cfgen-map) differs between
> chips hip05/06 and hip07.

The module-id property sounds like a HW description, but it's not
entirely clear to me what cfgen-map is; more comments on that below.

> Please suggest.
> >>+	- module-id : Module ID to input for djtag. This property is an array of
> >>+		      module_id for each L3 cache banks.
> >>+
> >>+	- num-banks : Number of banks or instances of the device.
> >What's a bank? Surely they have separate instances of the PMU?
> Yes each bank is a separate instance of PMU.
> If it is recommended to have each L3 cache bank registered as
> separate PMU with perf, then this property will be removed.

Generally, I think that separate instances are preferable. 

> >What order are these in?
> The bank number will start from "1" till "4" for L3 cache as there
> are four banks in hip05/06/07 chips.
> >>+	- cfgen-map : Config enable array to select the bank.
> >Huh?

As above, it's not clear to me what this property represents. Could you
please clarify?

Thanks,
Mark.

^ permalink raw reply

* [PATCH 2/2] ARM: davinci: PM: rework init: cleanup
From: Sekhar Nori @ 2016-11-15  9:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161114230441.356-3-khilman@baylibre.com>

On Tuesday 15 November 2016 04:34 AM, Kevin Hilman wrote:
> A follow-on cleanup renaming 'pdata' since it is no longer
> platform_data.
> 
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>

Looks good to me. Thanks for separating this out. Makes it much easier
to review.

In the subject line though, instead of just calling it cleanup, how
about a bit more descriptive statement like "remove references to pdata".

Thanks,
Sekhar

^ permalink raw reply

* [PATCH 1/2] ARM: davinci: PM: rework init, support DT platforms
From: Sekhar Nori @ 2016-11-15  9:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161114230441.356-2-khilman@baylibre.com>

Hi Kevin,

Looks good to me overall, I have some minor comments.

On Tuesday 15 November 2016 04:34 AM, Kevin Hilman wrote:
> Remove fake platform device used for PM init.  Move pdata values which
> are common across all current platforms into pm.c.
> 
> Also add PM support for DT platforms (vi da8xx-dt.c)

Can you please separate out PM enabling on DT platform to a separate
patch? Its a small change, but it will be nice to separate it from rest
of the cleanup.

> diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
> index f9f9713aacdd..3d7a13789661 100644
> --- a/arch/arm/mach-davinci/include/mach/da8xx.h
> +++ b/arch/arm/mach-davinci/include/mach/da8xx.h
> @@ -101,7 +101,6 @@ int da8xx_register_gpio(void *pdata);
>  int da850_register_cpufreq(char *async_clk);
>  int da8xx_register_cpuidle(void);
>  void __iomem *da8xx_get_mem_ctlr(void);
> -int da850_register_pm(struct platform_device *pdev);
>  int da850_register_sata(unsigned long refclkpn);
>  int da850_register_vpif(void);
>  int da850_register_vpif_display
> diff --git a/arch/arm/mach-davinci/pm.c b/arch/arm/mach-davinci/pm.c
> index 8929569b1f8a..fc6a5710b3fa 100644
> --- a/arch/arm/mach-davinci/pm.c
> +++ b/arch/arm/mach-davinci/pm.c
> @@ -23,13 +23,18 @@
>  #include <mach/da8xx.h>
>  #include "sram.h"
>  #include <mach/pm.h>
> +#include <mach/mux.h>

Can you please add the mux.h inclusion above pm.h? Looks like the sram.h
inclusion is out of place already, but since you are touching this part,
can you please move it below along with rest of the local includes.

I see that linux/ includes are not sorted as well, but lets keep that
aside until someone needs to touch them.

>  #include "clock.h"
> +#include "psc.h"
>  
> +#define DA850_PLL1_BASE		0x01e1a000
>  #define DEEPSLEEP_SLEEPCOUNT_MASK	0xFFFF
> +#define DEEPSLEEP_SLEEPCOUNT		128
>  
>  static void (*davinci_sram_suspend) (struct davinci_pm_config *);
> -static struct davinci_pm_config *pdata;
> +static struct davinci_pm_config pm_config;
> +static struct davinci_pm_config *pdata = &pm_config;
>  
>  static void davinci_sram_push(void *dest, void *src, unsigned int size)
>  {
> @@ -117,17 +122,38 @@ static const struct platform_suspend_ops davinci_pm_ops = {
>  	.valid		= suspend_valid_only_mem,
>  };
>  
> -static int __init davinci_pm_probe(struct platform_device *pdev)
> +int __init davinci_pm_init(void)
>  {
> -	pdata = pdev->dev.platform_data;
> -	if (!pdata) {
> -		dev_err(&pdev->dev, "cannot get platform data\n");
> -		return -ENOENT;
> +	int ret;
> +
> +	ret = davinci_cfg_reg(DA850_RTC_ALARM);
> +	if (ret)
> +		return ret;
> +
> +	pdata->sleepcount = DEEPSLEEP_SLEEPCOUNT;
> +	pdata->ddr2_ctlr_base = da8xx_get_mem_ctlr();
> +	pdata->deepsleep_reg = DA8XX_SYSCFG1_VIRT(DA8XX_DEEPSLEEP_REG);
> +	pdata->ddrpsc_num = DA8XX_LPSC1_EMIF3C;

Some of these could be statically initialized in pm_config. Can you
please move the constants to static initialization.

Thanks,
Sekhar

^ permalink raw reply

* [PATCH v2] arm64: SMMU-v2: Workaround for Cavium ThunderX erratum 28168
From: Marc Zyngier @ 2016-11-15  9:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479193220-6693-1-git-send-email-gakula@caviumnetworks.com>

On 15/11/16 07:00, Geetha sowjanya wrote:
> From: Tirumalesh Chalamarla <Tirumalesh.Chalamarla@cavium.com>
> 
>   This patch implements Cavium ThunderX erratum 28168.
> 
>   PCI requires stores complete in order. Due to erratum #28168
>   PCI-inbound MSI-X store to the interrupt controller are delivered
>   to the interrupt controller before older PCI-inbound memory stores
>   are committed.
>   Doing a sync on SMMU will make sure all prior data transfers are
>   completed before invoking ISR.
> 
> Signed-off-by: Tirumalesh Chalamarla <Tirumalesh.Chalamarla@cavium.com>
> Signed-off-by: Geetha sowjanya <gakula@caviumnetworks.com>
> ---
>  arch/arm64/Kconfig                  |   11 +++++++++++
>  arch/arm64/Kconfig.platforms        |    1 +
>  arch/arm64/include/asm/cpufeature.h |    3 ++-
>  arch/arm64/kernel/cpu_errata.c      |   16 ++++++++++++++++
>  drivers/iommu/arm-smmu.c            |   24 ++++++++++++++++++++++++
>  drivers/irqchip/irq-gic-common.h    |    1 +
>  drivers/irqchip/irq-gic-v3.c        |   19 +++++++++++++++++++
>  7 files changed, 74 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 30398db..751972c 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -474,6 +474,17 @@ config CAVIUM_ERRATUM_27456
>  
>  	  If unsure, say Y.
>  
> +config CAVIUM_ERRATUM_28168
> +       bool "Cavium erratum 28168: Make sure DMA data transfer is done before MSIX"
> +       depends on ARCH_THUNDER && ARM64
> +       default y
> +       help
> +        Due to erratum #28168 PCI-inbound MSI-X store to the interrupt
> +        controller are delivered to the interrupt controller before older
> +        PCI-inbound memory stores are committed. Doing a sync on SMMU
> +        will make sure all prior data transfers are done before invoking ISR.
> +
> +        If unsure, say Y.

Where is the entry in Documentation/arm64/silicon-errata.txt?

>  endmenu
>  
>  
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index cfbdf02..2ac4ac6 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -185,6 +185,7 @@ config ARCH_SPRD
>  
>  config ARCH_THUNDER
>  	bool "Cavium Inc. Thunder SoC Family"
> +	select IRQ_PREFLOW_FASTEOI
>  	help
>  	  This enables support for Cavium's Thunder Family of SoCs.
>  
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index 758d74f..821fc3c 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -40,8 +40,9 @@
>  #define ARM64_HAS_32BIT_EL0			13
>  #define ARM64_HYP_OFFSET_LOW			14
>  #define ARM64_MISMATCHED_CACHE_LINE_SIZE	15
> +#define ARM64_WORKAROUND_CAVIUM_28168		16
>  
> -#define ARM64_NCAPS				16
> +#define ARM64_NCAPS				17
>  
>  #ifndef __ASSEMBLY__
>  
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 0150394..0841a12 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -122,6 +122,22 @@ static void cpu_enable_trap_ctr_access(void *__unused)
>  		MIDR_RANGE(MIDR_THUNDERX_81XX, 0x00, 0x00),
>  	},
>  #endif
> +#ifdef CONFIG_CAVIUM_ERRATUM_28168
> +	{
> +	/* Cavium ThunderX, T88 pass 1.x - 2.1 */
> +		.desc = "Cavium erratum 28168",
> +		.capability = ARM64_WORKAROUND_CAVIUM_28168,
> +		MIDR_RANGE(MIDR_THUNDERX, 0x00,
> +			   (1 << MIDR_VARIANT_SHIFT) | 1),
> +	},
> +	{
> +	/* Cavium ThunderX, T81 pass 1.0 */
> +		.desc = "Cavium erratum 28168",
> +		.capability = ARM64_WORKAROUND_CAVIUM_28168,
> +		MIDR_RANGE(MIDR_THUNDERX_81XX, 0x00, 0x00),
> +	},
> +#endif

How is that a CPU bug? Shouldn't that be keyed on the SMMU version or
the ITs version?

> +
>  	{
>  		.desc = "Mismatched cache line size",
>  		.capability = ARM64_MISMATCHED_CACHE_LINE_SIZE,
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index c841eb7..1b4555c 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -570,6 +570,30 @@ static void __arm_smmu_tlb_sync(struct arm_smmu_device *smmu)
>  	}
>  }
>  
> +/*
> + * Cavium ThunderX erratum 28168
> + *
> + * Due to erratum #28168 PCI-inbound MSI-X store to the interrupt
> + * controller are delivered to the interrupt controller before older
> + * PCI-inbound memory stores are committed. Doing a sync on SMMU
> + * will make sure all prior data transfers are completed before
> + * invoking ISR.
> + *
> + */
> +void cavium_arm_smmu_tlb_sync(struct device *dev)
> +{
> +	struct iommu_fwspec *fwspec = dev->iommu_fwspec;
> +	struct arm_smmu_device *smmu;
> +
> +	smmu = fwspec_smmu(fwspec);
> +	if (!smmu)
> +		return;
> +	__arm_smmu_tlb_sync(smmu);
> +
> +}
> +EXPORT_SYMBOL(cavium_arm_smmu_tlb_sync);

Why does this need to be exported? The only user can only be built-in.

> +
> +
>  static void arm_smmu_tlb_sync(void *cookie)
>  {
>  	struct arm_smmu_domain *smmu_domain = cookie;
> diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
> index 205e5fd..4e88f55 100644
> --- a/drivers/irqchip/irq-gic-common.h
> +++ b/drivers/irqchip/irq-gic-common.h
> @@ -38,4 +38,5 @@ void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
>  
>  void gic_set_kvm_info(const struct gic_kvm_info *info);
>  
> +void cavium_arm_smmu_tlb_sync(struct device *dev);

Why should this be visible to GICv2 as well? I have the ugly feeling
this should stay private to the SMMU code and that a more standard
mechanism should be used... Robin, is there anything else we could
piggy-back on?

>  #endif /* _IRQ_GIC_COMMON_H */
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 19d642e..723cebe 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -28,6 +28,8 @@
>  #include <linux/of_irq.h>
>  #include <linux/percpu.h>
>  #include <linux/slab.h>
> +#include <linux/msi.h>
> +#include <linux/pci.h>
>  
>  #include <linux/irqchip.h>
>  #include <linux/irqchip/arm-gic-common.h>
> @@ -736,6 +738,20 @@ static inline void gic_cpu_pm_init(void) { }
>  
>  #define GIC_ID_NR		(1U << gic_data.rdists.id_bits)
>  
> +/*
> + * Due to #28168 erratum in ThunderX,
> + * we need to make sure DMA data transfer is done before MSIX.
> + */
> +static void cavium_irq_perflow_handler(struct irq_data *data)
> +{
> +	struct pci_dev *pdev;
> +
> +	pdev = msi_desc_to_pci_dev(irq_data_get_msi_desc(data));

What happens if this is not a PCI device?

> +	if ((pdev->vendor != 0x177d) &&
> +			((pdev->device & 0xA000) != 0xA000))
> +		cavium_arm_smmu_tlb_sync(&pdev->dev);

I've asked that before. What makes Cavium devices so special that they
are not sensitive to this bug?

> +}
> +
>  static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
>  			      irq_hw_number_t hw)
>  {
> @@ -773,6 +789,9 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
>  			return -EPERM;
>  		irq_domain_set_info(d, irq, hw, chip, d->host_data,
>  				    handle_fasteoi_irq, NULL, NULL);
> +		if (cpus_have_cap(ARM64_WORKAROUND_CAVIUM_28168))
> +			__irq_set_preflow_handler(irq,
> +						  cavium_irq_perflow_handler);

What happens if SMMUv2 is not compiled in? Also, since this only affects
LPI signaling, why is this in the core GICv3 code and not in the ITS.
And more specifically, in the PCI part of the ITS, since you seem to
exclusively consider PCI?

>  	}
>  
>  	return 0;
> 

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH v3 3/3] pinctrl: sunxi: Make sunxi_pconf_group_set use sunxi_pconf_reg helper
From: Linus Walleij @ 2016-11-15  9:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161111095036.11803-4-wens@csie.org>

On Fri, Nov 11, 2016 at 10:50 AM, Chen-Yu Tsai <wens@csie.org> wrote:

> The sunxi_pconf_reg helper introduced in the last patch gives us the
> chance to rework sunxi_pconf_group_set to have it match the structure
> of sunxi_pconf_(group_)get and make it easier to understand.
>
> For each config to set, it:
>
>     1. checks if the parameter is supported.
>     2. checks if the argument is within limits.
>     3. converts argument to the register value.
>     4. writes to the register with spinlock held.
>
> As a result the function now blocks unsupported config parameters,
> instead of silently ignoring them.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Patch applied.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH v3 2/3] pinctrl: sunxi: Add support for fetching pinconf settings from hardware
From: Linus Walleij @ 2016-11-15  9:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161111095036.11803-3-wens@csie.org>

On Fri, Nov 11, 2016 at 10:50 AM, Chen-Yu Tsai <wens@csie.org> wrote:

> The sunxi pinctrl driver only caches whatever pinconf setting was last
> set on a given pingroup. This is not particularly helpful, nor is it
> correct.
>
> Fix this by actually reading the hardware registers and returning
> the correct results or error codes. Also filter out unsupported
> pinconf settings. Since this driver has a peculiar setup of 1 pin
> per group, we can support both pin and pingroup pinconf setting
> read back with the same code. The sunxi_pconf_reg helper and code
> structure is inspired by pinctrl-msm.
>
> With this done we can also claim to support generic pinconf, by
> setting .is_generic = true in pinconf_ops.
>
> Also remove the cached config value. The behavior of this was never
> correct, as it only cached 1 setting instead of all of them. Since
> we can now read back settings directly from the hardware, it is no
> longer required.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Patch applied with Maxime's ACK.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH v3 1/3] pinctrl: sunxi: Fix PIN_CONFIG_BIAS_PULL_{DOWN, UP} argument
From: Linus Walleij @ 2016-11-15  9:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161111095036.11803-2-wens@csie.org>

On Fri, Nov 11, 2016 at 10:50 AM, Chen-Yu Tsai <wens@csie.org> wrote:

> According to pinconf-generic.h, the argument for
> PIN_CONFIG_BIAS_PULL_{DOWN,UP} is non-zero if the bias is enabled
> with a pull up/down resistor, zero if it is directly connected
> to VDD or ground.
>
> Since Allwinner hardware uses a weak pull resistor internally,
> the argument should be 1.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Applied this v3 rather than the v2.

Yours,
Linus Walleij

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox