Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Linaro-acpi] ACPI namespace details for ARM64
From: Hanjun Guo @ 2016-11-11 14:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161111093226.GA13333@red-moon>

On 11/11/2016 05:32 PM, Lorenzo Pieralisi wrote:
> On Thu, Nov 10, 2016 at 04:18:54PM -0700, Al Stone wrote:
>> On 11/09/2016 03:05 PM, Bjorn Helgaas wrote:
>>> Hi all,
>>>
>>> We've been working through the details of getting ACPI to work on
>>> arm64, and there have been lots of questions about what this means for
>>> PCI.  I've outlined this for several people individually, but I'm
>>> going to send this separately, apart from a specific patch series, to
>>> make sure we're all on the same page.  Please correct my errors and
>>> misunderstandings.
>>>
>>> Bjorn
>>>
>>> [snip....]
>>
>> A big +1 to all of this.  This also looks like something that should
>> be added to either PCI, ACPI or arm64 documentation (or even all three).
>
> And to arm64 platforms FW :)
>
>> What do you think?
>
> I do not think there is anything ARM64 specific in Bjorn's description,
> but I do think it is very useful to have it in documentation, these
> bits of information are scattered around ACPI specs and PCI FW specs,
> having a single source would help and would have prevented asking
> Bjorn the same questions 100 times.
>
>> Thank you for putting this together, Bjorn.
>
> +1, Thank you very much for this nice summary Bjorn.

+1, thanks a lot :)

Hanjun

^ permalink raw reply

* [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: liviu.dudau at arm.com @ 2016-11-11 14:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <EE11001F9E5DDD47B7634E2F8A612F2E1F8F900A@lhreml507-mbx>

On Fri, Nov 11, 2016 at 01:39:35PM +0000, Gabriele Paoloni wrote:
> Hi Arnd
> 
> > -----Original Message-----
> > From: Arnd Bergmann [mailto:arnd at arndb.de]
> > Sent: 10 November 2016 16:07
> > To: Gabriele Paoloni
> > Cc: linux-arm-kernel at lists.infradead.org; Yuanzhichang;
> > mark.rutland at arm.com; devicetree at vger.kernel.org;
> > lorenzo.pieralisi at arm.com; minyard at acm.org; linux-pci at vger.kernel.org;
> > benh at kernel.crashing.org; John Garry; will.deacon at arm.com; linux-
> > kernel at vger.kernel.org; xuwei (O); Linuxarm; zourongrong at gmail.com;
> > robh+dt at kernel.org; kantyzc at 163.com; linux-serial at vger.kernel.org;
> > catalin.marinas at arm.com; olof at lixom.net; liviu.dudau at arm.com;
> > bhelgaas at googl e.com; zhichang.yuan02 at gmail.com
> > Subject: Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on
> > Hip06
> > 
> > On Thursday, November 10, 2016 3:36:49 PM CET Gabriele Paoloni wrote:
> > >
> > > Where should we get the range from? For LPC we know that it is going
> > > Work on anything that is not used by PCI I/O space, and this is
> > > why we use [0, PCIBIOS_MIN_IO]
> > 
> > It should be allocated the same way we allocate PCI config space
> > segments. This is currently done with the io_range list in
> > drivers/pci/pci.c, which isn't perfect but could be extended
> > if necessary. Based on what others commented here, I'd rather
> > make the differences between ISA/LPC and PCI I/O ranges smaller
> > than larger.

Gabriele,

> 
> I am not sure this would make sense...
> 
> IMHO all the mechanism around io_range_list is needed to provide the
> "mapping" between I/O tokens and physical CPU addresses.
> 
> Currently the available tokens range from 0 to IO_SPACE_LIMIT.
> 
> As you know the I/O memory accessors operate on whatever
> __of_address_to_resource sets into the resource (start, end).
> 
> With this special device in place we cannot know if a resource is
> assigned with an I/O token or a physical address, unless we forbid
> the I/O tokens to be in a specific range.
> 
> So this is why we are changing the offsets of all the functions
> handling io_range_list (to make sure that a range is forbidden to
> the tokens and is available to the physical addresses).
> 
> We have chosen this forbidden range to be [0, PCIBIOS_MIN_IO)
> because this is the maximum physical I/O range that a non PCI device
> can operate on and because we believe this does not impose much
> restriction on the available I/O token range; that now is 
> [PCIBIOS_MIN_IO, IO_SPACE_LIMIT].
> So we believe that the chosen forbidden range can accommodate
> any special ISA bus device with no much constraint on the rest
> of I/O tokens...

Your idea is a good one, however you are abusing PCIBIOS_MIN_IO and you
actually need another variable for "reserving" an area in the I/O space
that can be used for physical addresses rather than I/O tokens.

The one good example for using PCIBIOS_MIN_IO is when your platform/architecture
does not support legacy ISA operations *at all*. In that case someone
sets the PCIBIOS_MIN_IO to a non-zero value to reserve that I/O range
so that it doesn't get used. With Zhichang's patch you now start forcing
those platforms to have a valid address below PCIBIOS_MIN_IO.

For the general case you also have to bear in mind that PCIBIOS_MIN_IO could
be zero. In that case, what is your "forbidden" range? [0, 0) ? So it makes
sense to add a new #define that should only be defined by those architectures/
platforms that want to reserve on top of PCIBIOS_MIN_IO another region
where I/O tokens can't be generated for.

Best regards,
Liviu

> 
> > 
> > > > Your current version has
> > > >
> > > >         if (arm64_extio_ops->pfout)                             \
> > > >                 arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
> > > >                        addr, value, sizeof(type));             \
> > > >
> > > > Instead, just subtract the start of the range from the logical
> > > > port number to transform it back into a bus-local port number:
> > >
> > > These accessors do not operate on IO tokens:
> > >
> > > If (arm64_extio_ops->start > addr || arm64_extio_ops->end < addr)
> > > addr is not going to be an I/O token; in fact patch 2/3 imposes that
> > > the I/O tokens will start at PCIBIOS_MIN_IO. So from 0 to
> > PCIBIOS_MIN_IO
> > > we have free physical addresses that the accessors can operate on.
> > 
> > Ah, I missed that part. I'd rather not use PCIBIOS_MIN_IO to refer to
> > the logical I/O tokens, the purpose of that macro is really meant
> > for allocating PCI I/O port numbers within the address space of
> > one bus.
> 
> As I mentioned above, special devices operate on CPU addresses directly,
> not I/O tokens. For them there is no way to distinguish....
> 
> > 
> > Note that it's equally likely that whichever next platform needs
> > non-mapped I/O access like this actually needs them for PCI I/O space,
> > and that will use it on addresses registered to a PCI host bridge.
> 
> Ok so here you are talking about a platform that has got an I/O range
> under the PCI host controller, right?
> And this I/O range cannot be directly memory mapped but needs special
> redirections for the I/O tokens, right?
> 
> In this scenario registering the I/O ranges with the forbidden range
> implemented by the current patch would still allow to redirect I/O
> tokens as long as arm64_extio_ops->start >= PCIBIOS_MIN_IO
> 
> So effectively the special PCI host controller
> 1) knows the physical range that needs special redirection
> 2) register such range
> 3) uses pci_pio_to_address() to retrieve the IO tokens for the
>    special accessors
> 4) sets arm64_extio_ops->start/end to the IO tokens retrieved in 3)
> 
> So to be honest I think this patch can fit well both with
> special PCI controllers that need I/O tokens redirection and with
> special non-PCI controllers that need non-PCI I/O physical
> address redirection...
> 
> Thanks (and sorry for the long reply but I didn't know how
> to make the explanation shorter :) )
> 
> Gab
> 
> > 
> > If we separate the two steps:
> > 
> > a) assign a range of logical I/O port numbers to a bus
> > b) register a set of helpers for redirecting logical I/O
> >    port to a helper function
> > 
> > then I think the code will get cleaner and more flexible.
> > It should actually then be able to replace the powerpc
> > specific implementation.
> > 
> > 	Arnd

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ?\_(?)_/?

^ permalink raw reply

* [RFC v2 8/8] iommu/arm-smmu: implement add_reserved_regions callback
From: Joerg Roedel @ 2016-11-11 15:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <479aeac0-71f9-a6b8-af6d-e2c25184a818@arm.com>

On Fri, Nov 11, 2016 at 02:34:39PM +0000, Robin Murphy wrote:
> On 10/11/16 16:16, Joerg Roedel wrote:
> > On Thu, Nov 10, 2016 at 04:07:08PM +0000, Robin Murphy wrote:
> >> On 10/11/16 15:46, Joerg Roedel wrote:
> >>> On Fri, Nov 04, 2016 at 11:24:06AM +0000, Eric Auger wrote:
> >>>> +	resource_list_for_each_entry(window, &bridge->windows) {
> >>>> +		if (resource_type(window->res) != IORESOURCE_MEM &&
> >>>> +		    resource_type(window->res) != IORESOURCE_IO)
> >>>> +			continue;
> >>>
> >>> Why do you care about IO resources?
> >>
> >> [since this is essentially code I wrote]
> >>
> >> Because they occupy some area of the PCI address space, therefore I
> >> assumed that, like memory windows, they would be treated as P2P. Is that
> >> not the case?
> > 
> > No, not at all. The IO-space is completly seperate from the MEM-space.
> > They are two different address-spaces, addressing different things. And
> > the IO-space is also not translated by any IOMMU I am aware of.
> 
> OK. On the particular root complex I have to hand, though, any DMA to
> IOVAs between 0x5f800000 and 0x5fffffff sends an error back to the
> endpoint, and that just so happens to be where the I/O window is placed
> (both on the PCI side and the AXI (i.e. CPU MMIO) side. Whether it's
> that the external MMIO view of the RC's I/O window is explicitly
> duplicated in its PCI memory space as some side-effect of the PCI/AXI
> bridge, or that the thing just doesn't actually respect the access type
> on the PCI side I don't know, but that's how it is (and I spent this
> morning recreating it to make sure I wasn't mistaken).

What you see is that on your platform the io-ports are accessed by an
mmio-window. On x86 you have dedicated instructions to access io-ports.
And the io-port ranges are what is what the io-resources describe. These
resources do no tell you where the mmio-region for that devices io-ports
are.


	Joerg

^ permalink raw reply

* [PATCH RFC 00/12] tda998x updates
From: Jyri Sarha @ 2016-11-11 15:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161108122420.GP1041@n2100.armlinux.org.uk>

On 11/08/16 14:24, Russell King - ARM Linux wrote:
> As no one responded to the previous round, I'm not spending soo much
> time writing up a description of these changes again.  It's also been
> quite a long time, so I've forgotten all the details of the changes,
> so I'll do my best.
> 
> Changes from the previous series include:
> - reorder the initial three patches
> - change the (now third patch)... I think to increase the size of the
>   locked region.
> - fix edid parsing for infoframe generation - as was pointed out for
>   dw-hdmi, parsing the EDID in get_modes() is incorrect, as that method
>   will not be called when an override-edid is in effect.  We need to
>   parse the override-edid.  Moreover, infoframe generation should not
>   be keyed to whether the monitor is HDMI or not, CEA-861B allows non-
>   HDMI to send infoframes.
> - only send audio if audio and infoframes are supported.
> 
> Otherwise, these are very much like the previous posting of the series,
> except rebased upon the mali/hdlcd/tda998x change to remove the
> drm_connector_register() call.
> 
> https://www.spinics.net/lists/dri-devel/msg121495.html
> 
> It'd be nice to have other tda998x users ack and test these patches,
> I've tried to test on Juno, but the Juno situation seems to be a huge
> fail.  (HBI0282B completely fails with latest firmware - (a) FPGA image
> incompatibilities io_b118 causes all FPGA AMBA devices to vanish, (b)
> seems no way to get SCPI support on it - adding the BL0 executable
> start address in the SCC registers seems to be incompatible with the
> devchip, causing the PLLs to fail.  In discussion with Sudeep over
> these issues, but no idea where things are with it at the moment, other
> than Sudeep needs to investigate.  All Linaro firmware releases are
> broken on HBI0282B.)
> 
>  drivers/gpu/drm/i2c/tda998x_drv.c | 826 ++++++++++++++++++++------------------
>  1 file changed, 429 insertions(+), 397 deletions(-)
> 

Reviewed-by: Jyri Sarha <jsarha@ti.com>

For the whole series. I am also happy to test these patches if I can
fetch them from some git repo.

Best regards,
Jyri

^ permalink raw reply

* [PATCH v5 2/3] vcodec: mediatek: Add Mediatek JPEG Decoder Driver
From: Hans Verkuil @ 2016-11-11 15:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478586880-3923-3-git-send-email-rick.chang@mediatek.com>

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).

> +	},
> +};
> +
> +#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'.

> +		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.

> +	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.

> +	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.

> +	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.

> +
> +	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 v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic
From: Joerg Roedel @ 2016-11-11 15:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161109141948.19244-5-lorenzo.pieralisi@arm.com>

On Wed, Nov 09, 2016 at 02:19:36PM +0000, Lorenzo Pieralisi wrote:
> +struct iommu_fwentry {
> +	struct list_head list;
> +	struct fwnode_handle *fwnode;
> +	const struct iommu_ops *ops;
> +};

Is there a reason the iommu_ops need to be stored there? Currently it
seems that the ops are only needed to get the of_xlate fn-ptr later. And
the place where it is called the iommu-ops should also be available
through pdev->dev->bus->iommu_ops.


	Joerg

^ permalink raw reply

* [PATCH RFC 00/12] tda998x updates
From: Russell King - ARM Linux @ 2016-11-11 15:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <964260d7-af7f-fefb-2adb-0e10ff750883@ti.com>

On Fri, Nov 11, 2016 at 05:10:09PM +0200, Jyri Sarha wrote:
> On 11/08/16 14:24, Russell King - ARM Linux wrote:
> > As no one responded to the previous round, I'm not spending soo much
> > time writing up a description of these changes again.  It's also been
> > quite a long time, so I've forgotten all the details of the changes,
> > so I'll do my best.
> > 
> > Changes from the previous series include:
> > - reorder the initial three patches
> > - change the (now third patch)... I think to increase the size of the
> >   locked region.
> > - fix edid parsing for infoframe generation - as was pointed out for
> >   dw-hdmi, parsing the EDID in get_modes() is incorrect, as that method
> >   will not be called when an override-edid is in effect.  We need to
> >   parse the override-edid.  Moreover, infoframe generation should not
> >   be keyed to whether the monitor is HDMI or not, CEA-861B allows non-
> >   HDMI to send infoframes.
> > - only send audio if audio and infoframes are supported.
> > 
> > Otherwise, these are very much like the previous posting of the series,
> > except rebased upon the mali/hdlcd/tda998x change to remove the
> > drm_connector_register() call.
> > 
> > https://www.spinics.net/lists/dri-devel/msg121495.html
> > 
> > It'd be nice to have other tda998x users ack and test these patches,
> > I've tried to test on Juno, but the Juno situation seems to be a huge
> > fail.  (HBI0282B completely fails with latest firmware - (a) FPGA image
> > incompatibilities io_b118 causes all FPGA AMBA devices to vanish, (b)
> > seems no way to get SCPI support on it - adding the BL0 executable
> > start address in the SCC registers seems to be incompatible with the
> > devchip, causing the PLLs to fail.  In discussion with Sudeep over
> > these issues, but no idea where things are with it at the moment, other
> > than Sudeep needs to investigate.  All Linaro firmware releases are
> > broken on HBI0282B.)
> > 
> >  drivers/gpu/drm/i2c/tda998x_drv.c | 826 ++++++++++++++++++++------------------
> >  1 file changed, 429 insertions(+), 397 deletions(-)
> > 
> 
> Reviewed-by: Jyri Sarha <jsarha@ti.com>
> 
> For the whole series. I am also happy to test these patches if I can
> fetch them from some git repo.

git://git.armlinux.org.uk/~rmk/linux-arm.git drm-tda998x-devel

The commit IDs are unstable, because I'll have to recommit them to add
your r-by and any other tags you later give me. :)

Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* [PATCH v14 4/9] acpi/arm64: Add GTDT table parse driver
From: Mark Rutland @ 2016-11-11 15:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5825CBB5.8090104@linaro.org>

On Fri, Nov 11, 2016 at 09:46:29PM +0800, Hanjun Guo wrote:
> On 10/21/2016 12:37 AM, Mark Rutland wrote:
> >On Thu, Sep 29, 2016 at 02:17:12AM +0800, fu.wei at linaro.org wrote:
> >>+static int __init map_gt_gsi(u32 interrupt, u32 flags)
> >>+{
> >>+	int trigger, polarity;
> >>+
> >>+	if (!interrupt)
> >>+		return 0;
> >
> >Urgh.
> >
> >Only the secure interrupt (which we do not need) is optional in this
> >manner, and (hilariously), zero appears to also be a valid GSIV, per
> >figure 5-24 in the ACPI 6.1 spec.
> >
> >So, I think that:
> >
> >(a) we should not bother parsing the secure interrupt
> >(b) we should drop the check above
> >(c) we should report the spec issue to the ASWG
> 
> Sorry, I willing to do that, but I need to figure out the issue here.
> What kind of issue in detail? do you mean that zero should not be valid
> for arch timer interrupts?

As above, zero is a valid GSIV, and is valid for the non-secure timer
interrupts. The check is wrong for non-secure interrupts.

We can ignore the secure timer interrupt since it's irrelevant to us,
and remove the check.

Regardless, the spec is inconsistent w.r.t. the secure interrupt being
zero if not present, since zero is a valid GSIV. That should be reported
to the ASWG.

Thanks,
Mark.

^ permalink raw reply

* [RFC PATCH 6/6] [media] davinci: vpif_capture: get subdevs from DT
From: Hans Verkuil @ 2016-11-11 15:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161025235536.7342-7-khilman@baylibre.com>

On 10/26/2016 01:55 AM, Kevin Hilman wrote:
> First pass at getting subdevs from DT ports and endpoints.
> 
> The _get_pdata() function was larely inspired by (i.e. stolen from)
> am437x-vpfe.c
> 
> Questions:
> - Legacy board file passes subdev input & output routes via pdata
>   (e.g. tvp514x svideo or composite selection.)  How is this supposed
>   to be done via DT?

We have plans to model connectors as well in the device tree, but no
implementation exists yet. I think Laurent has some code in progress for this,
but I may be mistaken.

Anyway, hard-coding it like you do now is for now the only way.

	Hans

> 
> Not-Yet-Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> ---
>  drivers/media/platform/davinci/vpif_capture.c | 132 +++++++++++++++++++++++++-
>  include/media/davinci/vpif_types.h            |   9 +-
>  2 files changed, 134 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
> index becc3e63b472..df2af5cda37a 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c
> @@ -26,6 +26,8 @@
>  #include <linux/slab.h>
>  
>  #include <media/v4l2-ioctl.h>
> +#include <media/v4l2-of.h>
> +#include <media/i2c/tvp514x.h> /* FIXME: how to pass the INPUT_* OUTPUT* fields? */
>  
>  #include "vpif.h"
>  #include "vpif_capture.h"
> @@ -651,6 +653,10 @@ static int vpif_input_to_subdev(
>  
>  	vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>  
> +	if (!chan_cfg)
> +		return -1;
> +	if (input_index >= chan_cfg->input_count)
> +		return -1;
>  	subdev_name = chan_cfg->inputs[input_index].subdev_name;
>  	if (subdev_name == NULL)
>  		return -1;
> @@ -658,7 +664,7 @@ static int vpif_input_to_subdev(
>  	/* loop through the sub device list to get the sub device info */
>  	for (i = 0; i < vpif_cfg->subdev_count; i++) {
>  		subdev_info = &vpif_cfg->subdev_info[i];
> -		if (!strcmp(subdev_info->name, subdev_name))
> +		if (subdev_info && !strcmp(subdev_info->name, subdev_name))
>  			return i;
>  	}
>  	return -1;
> @@ -1328,13 +1334,25 @@ static int vpif_async_bound(struct v4l2_async_notifier *notifier,
>  {
>  	int i;
>  
> +	for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
> +		const struct device_node *node = vpif_obj.config->asd[i]->match.of.node;
> +
> +		if (node == subdev->of_node) {
> +			vpif_obj.sd[i] = subdev;
> +			vpif_obj.config->chan_config->inputs[i].subdev_name = subdev->of_node->full_name;
> +			vpif_dbg(2, debug, "%s: setting input %d subdev_name = %s\n", __func__,
> +				 i, subdev->of_node->full_name);
> +			return 0;
> +		}
> +	}
> +
>  	for (i = 0; i < vpif_obj.config->subdev_count; i++)
>  		if (!strcmp(vpif_obj.config->subdev_info[i].name,
>  			    subdev->name)) {
>  			vpif_obj.sd[i] = subdev;
>  			return 0;
>  		}
> -
> +	
>  	return -EINVAL;
>  }
>  
> @@ -1423,6 +1441,113 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)
>  	return vpif_probe_complete();
>  }
>  
> +static struct vpif_capture_config *
> +vpif_capture_get_pdata(struct platform_device *pdev)
> +{
> +	struct device_node *endpoint = NULL;
> +	struct v4l2_of_endpoint bus_cfg;
> +	struct vpif_capture_config *pdata;
> +	struct vpif_subdev_info *sdinfo;
> +	struct vpif_capture_chan_config *chan;
> +	unsigned int i;
> +
> +	dev_dbg(&pdev->dev, "vpif_get_pdata\n");
> +
> +	if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
> +		return pdev->dev.platform_data;
> +
> +	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return NULL;
> +	pdata->subdev_info = devm_kzalloc(&pdev->dev,
> +					  sizeof(*pdata->subdev_info) *
> +					  VPIF_CAPTURE_MAX_CHANNELS, GFP_KERNEL);
> +	if (!pdata->subdev_info)
> +		return NULL;
> +	dev_dbg(&pdev->dev, "%s\n", __func__);
> +
> +	for (i = 0; ; i++) {
> +		struct device_node *rem;
> +		unsigned int flags;
> +		int err;
> +		
> +		endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
> +						      endpoint);
> +		if (!endpoint)
> +			break;
> +
> +		dev_dbg(&pdev->dev, "found endpoint %s, %s\n",
> +			endpoint->name, endpoint->full_name);
> +
> +		sdinfo = &pdata->subdev_info[i];
> +		chan = &pdata->chan_config[i];
> +		chan->inputs = devm_kzalloc(&pdev->dev,
> +					    sizeof(*chan->inputs) *
> +					    VPIF_DISPLAY_MAX_CHANNELS,
> +					    GFP_KERNEL);
> +		
> +		/* sdinfo->name = devm_kzalloc(&pdev->dev, 16, GFP_KERNEL); */
> +		/* snprintf(sdinfo->name, 16, "VPIF input %d", i); */
> +		chan->input_count++;
> +		chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
> +		chan->inputs[i].input.std = V4L2_STD_ALL;
> +		chan->inputs[i].input.capabilities = V4L2_IN_CAP_STD;
> +		
> +		/* FIXME: need a new property? ch0:composite ch1: s-video */
> +		if (i == 0)
> +			chan->inputs[i].input_route = INPUT_CVBS_VI2B;
> +		else
> +			chan->inputs[i].input_route = INPUT_SVIDEO_VI2C_VI1C;
> +		chan->inputs[i].output_route = OUTPUT_10BIT_422_EMBEDDED_SYNC;
> +				
> +		err = v4l2_of_parse_endpoint(endpoint, &bus_cfg);
> +		if (err) {
> +			dev_err(&pdev->dev, "Could not parse the endpoint\n");
> +			goto done;
> +		}
> +		dev_dbg(&pdev->dev, "Endpoint %s, bus_width = %d\n",
> +			endpoint->full_name, bus_cfg.bus.parallel.bus_width);
> +		flags = bus_cfg.bus.parallel.flags;
> +
> +		if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> +			chan->vpif_if.hd_pol = 1;
> +
> +		if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
> +			chan->vpif_if.vd_pol = 1;
> +
> +		chan->vpif_if.if_type = VPIF_IF_BT656;
> +		rem = of_graph_get_remote_port_parent(endpoint);
> +		if (!rem) {
> +			dev_dbg(&pdev->dev, "Remote device at %s not found\n",
> +				endpoint->full_name);
> +			goto done;
> +		}
> +
> +		dev_dbg(&pdev->dev, "Remote device %s, %s found\n", rem->name, rem->full_name);
> +		sdinfo->name = rem->full_name;
> +
> +		pdata->asd[i] = devm_kzalloc(&pdev->dev,
> +					     sizeof(struct v4l2_async_subdev),
> +					     GFP_KERNEL);
> +		if (!pdata->asd[i]) {
> +			of_node_put(rem);
> +			pdata = NULL;
> +			goto done;
> +		}
> +
> +		pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_OF;
> +		pdata->asd[i]->match.of.node = rem;
> +		of_node_put(rem);
> +	}
> +
> +done:
> +	pdata->asd_sizes[0] = i;
> +	pdata->subdev_count = i;
> +	pdata->card_name = "DA850/OMAP-L138 Video Capture";
> +
> +	return pdata;
> +}
> +
>  /**
>   * vpif_probe : This function probes the vpif capture driver
>   * @pdev: platform device pointer
> @@ -1439,6 +1564,7 @@ static __init int vpif_probe(struct platform_device *pdev)
>  	int res_idx = 0;
>  	int i, err;
>  
> +	pdev->dev.platform_data = vpif_capture_get_pdata(pdev);;
>  	if (!pdev->dev.platform_data) {
>  		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
>  		return -EINVAL;
> @@ -1481,7 +1607,7 @@ static __init int vpif_probe(struct platform_device *pdev)
>  		goto vpif_unregister;
>  	}
>  
> -	if (!vpif_obj.config->asd_sizes) {
> +	if (!vpif_obj.config->asd_sizes[0]) {
>  		i2c_adap = i2c_get_adapter(1);
>  		for (i = 0; i < subdev_count; i++) {
>  			subdevdata = &vpif_obj.config->subdev_info[i];
> diff --git a/include/media/davinci/vpif_types.h b/include/media/davinci/vpif_types.h
> index 3cb1704a0650..4ee3b41975db 100644
> --- a/include/media/davinci/vpif_types.h
> +++ b/include/media/davinci/vpif_types.h
> @@ -65,14 +65,14 @@ struct vpif_display_config {
>  
>  struct vpif_input {
>  	struct v4l2_input input;
> -	const char *subdev_name;
> +	char *subdev_name;
>  	u32 input_route;
>  	u32 output_route;
>  };
>  
>  struct vpif_capture_chan_config {
>  	struct vpif_interface vpif_if;
> -	const struct vpif_input *inputs;
> +	struct vpif_input *inputs;
>  	int input_count;
>  };
>  
> @@ -83,7 +83,8 @@ struct vpif_capture_config {
>  	struct vpif_subdev_info *subdev_info;
>  	int subdev_count;
>  	const char *card_name;
> -	struct v4l2_async_subdev **asd;	/* Flat array, arranged in groups */
> -	int *asd_sizes;		/* 0-terminated array of asd group sizes */
> +
> +	struct v4l2_async_subdev *asd[VPIF_CAPTURE_MAX_CHANNELS];
> +	int asd_sizes[VPIF_CAPTURE_MAX_CHANNELS];
>  };
>  #endif /* _VPIF_TYPES_H */
> 

^ permalink raw reply

* [RFC PATCH 0/6] media: davinci: VPIF: add DT support
From: Hans Verkuil @ 2016-11-11 15:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161025235536.7342-1-khilman@baylibre.com>

Hi Kevin,

On 10/26/2016 01:55 AM, Kevin Hilman wrote:
> This series attempts to add DT support to the davinci VPIF capture
> driver.
> 
> I'm not sure I've completely grasped the proper use of the ports and
> endpoints stuff, so this RFC is primarily to get input on whether I'm
> on the right track.
> 
> The last patch is the one where all my questions are, the rest are
> just prep work to ge there.
> 
> Tested on da850-lcdk and was able to do basic frame capture from the
> composite input.
> 
> Series applies on v4.9-rc1
> 
> Kevin Hilman (6):
>   [media] davinci: add support for DT init
>   ARM: davinci: da8xx: VPIF: enable DT init
>   ARM: dts: davinci: da850: add VPIF
>   ARM: dts: davinci: da850-lcdk: enable VPIF capture
>   [media] davinci: vpif_capture: don't lock over s_stream
>   [media] davinci: vpif_capture: get subdevs from DT

Looks good, but wouldn't it be better to do the dts changes last when all the
supporting code is in?

Regards,

	Hans

> 
>  arch/arm/boot/dts/da850-lcdk.dts              |  30 ++++++
>  arch/arm/boot/dts/da850.dtsi                  |  28 +++++
>  arch/arm/mach-davinci/da8xx-dt.c              |  17 +++
>  drivers/media/platform/davinci/vpif.c         |   9 ++
>  drivers/media/platform/davinci/vpif_capture.c | 150 +++++++++++++++++++++++++-
>  include/media/davinci/vpif_types.h            |   9 +-
>  6 files changed, 236 insertions(+), 7 deletions(-)
> 

^ permalink raw reply

* [PATCH] Replacement for Arm initrd memblock reserve and free inconsistency.
From: william.helsby at stfc.ac.uk @ 2016-11-11 15:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161110174645.GB1041@n2100.armlinux.org.uk>

>From 45272bc4d3f27f2c316f5b607441d1337a5501ec Mon Sep 17 00:00:00 2001
From: William Helsby <william.helsby@stfc.ac.uk>
Date: Fri, 11 Nov 2016 15:04:04 +0000
Subject: [PATCH] arm: zynq::Fixing inconsistent reserve and free for initrd
 image
My first attempt at fixing this was very similar to your proposed patch,
though due me not understanding the need to switch outlook to plain text
it never got posted.
In the current case, with the initrd image starting page aligned, with space
above it, the patch worked.
However, on reflection it struck me that the placement of the ramdisk image
is done by the bootloader, so may not always be like this. 
This patch tries to round down the start and up the end, but checks that this 
does not cause an overlap. If it does overlap, it tries aligning just the end 
or start and if neither is possible falls back to not expanding the area 
reserved. 
The code then remembers the start and end of reserved area, with 
any successful expansions to page boundaries. 
If when ./init/initramfs.c calls free_initrd_mem(), the start or end match,
The respective extended values are used so complete pages are released.
Note that according to comments in ./init/initramfs.c, the crashkernel
region can overlap the initrd region, though I don't know anything about
this. 
The POISON_FREE_INITMEM value is used to cause free_reserved_area to
poison only the pages which are released.
Despite this code being careful not to overlap the kernel, there still may be 
an issue with it extending the RAMDISK image reserved are to overlap 
the device tree or something else I am unaware of.
Perhaps the call to early_init_fdt_reserve_self() should be moved 
to before the initrd code?
For the sake of freeing at most 2 partial 4k pages, I am not sure whether 
complexity and technical risk of this solution is justified - unless these 
pages are going to be in the middle of a DMA contiguous area.
Note that the arm64 code reverted to not extending the released area,
but enable clearing of the released areas at 
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/arch/arm64/mm/init.c?id=6b00f7efb5303418c231994c91fb8239f5ada260

Now
void free_initrd_mem(unsigned long start, unsigned long end)
{
	if (!keep_initrd)
		free_reserved_area((void *)start, (void *)end, 0, "initrd");
} 

Signed-off-by: William Helsby <william.helsby@stfc.ac.uk>
---
 arch/arm/mm/init.c | 38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 370581a..00a489d 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -49,6 +49,8 @@ unsigned long __init __clear_cr(unsigned long mask)

 static phys_addr_t phys_initrd_start __initdata = 0;
 static unsigned long phys_initrd_size __initdata = 0;
+static unsigned long initrd_reservation_start __initdata = 0;
+static unsigned long initrd_reservation_end __initdata = 0;

 static int __init early_initrd(char *p)
 {
@@ -255,11 +257,38 @@ void __init arm_memblock_init(const struct machine_desc *mdesc)
                phys_initrd_start = phys_initrd_size = 0;
        }
        if (phys_initrd_size) {
-               memblock_reserve(phys_initrd_start, phys_initrd_size);
+           /* try to round the initrd start and end down and up to page boundaries,
+              so when freed, whole pages can be released.
+              However the initrd image may be adjacent to something else, so check that this rounding is OK
+           */
+         /* First try rounding the start down and end up */
+           phys_addr_t phys_initrd_reservation_start = phys_initrd_start & PAGE_MASK;
+            unsigned long size_to_reserve = PAGE_ALIGN(phys_initrd_start+phys_initrd_size) - phys_initrd_reservation_start;
+           if (!memblock_is_region_memory(phys_initrd_reservation_start, size_to_reserve) ||
+                 memblock_is_region_reserved(phys_initrd_reservation_start, size_to_reserve)) {
+             /* This either does fit or overlaps something else, so try just rounding end up */
+             phys_initrd_reservation_start = phys_initrd_start;
+             size_to_reserve = PAGE_ALIGN(phys_initrd_start+phys_initrd_size) - phys_initrd_reservation_start;
+             if (!memblock_is_region_memory(phys_initrd_reservation_start, size_to_reserve) ||
+                 memblock_is_region_reserved(phys_initrd_reservation_start, size_to_reserve)) {
+               /* This either does not fit or overlaps something else, so try just rounding start down */
+               phys_initrd_reservation_start = phys_initrd_start & PAGE_MASK;
+               size_to_reserve = (phys_initrd_start+phys_initrd_size) - phys_initrd_reservation_start;
+               if (!memblock_is_region_memory(phys_initrd_reservation_start, size_to_reserve) ||
+                   memblock_is_region_reserved(phys_initrd_reservation_start, size_to_reserve)) {
+                 /* This either does not fit or overlaps something else, so do not round at all */
+                 phys_initrd_reservation_start = phys_initrd_start;
+                 size_to_reserve = (phys_initrd_start+phys_initrd_size) - phys_initrd_reservation_start;
+               }
+             }
+           }
+               memblock_reserve(phys_initrd_reservation_start, size_to_reserve);

                /* Now convert initrd to virtual addresses */
                initrd_start = __phys_to_virt(phys_initrd_start);
                initrd_end = initrd_start + phys_initrd_size;
+               initrd_reservation_start = __phys_to_virt(phys_initrd_reservation_start);
+               initrd_reservation_end = initrd_reservation_start + size_to_reserve;
        }
 #endif

@@ -771,12 +800,11 @@ void free_initrd_mem(unsigned long start, unsigned long end)
 {
        if (!keep_initrd) {
                if (start == initrd_start)
-                       start = round_down(start, PAGE_SIZE);
+                 start = initrd_reservation_start;
                if (end == initrd_end)
-                       end = round_up(end, PAGE_SIZE);
+                 end = initrd_reservation_end;

-               poison_init_mem((void *)start, PAGE_ALIGN(end) - start);
-               free_reserved_area((void *)start, (void *)end, -1, "initrd");
+               free_reserved_area((void *)start, (void *)end, POISON_FREE_INITMEM, "initrd");
        }
 }

--
1.8.3.1

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux at armlinux.org.uk] 
> Sent: 10 November 2016 17:47
>To: Helsby, William (STFC,DL,TECH)
> Cc: linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH] Replacement for Arm initrd memblock reserve and free inconsistency.

>On Wed, Nov 09, 2016 at 04:35:37PM +0000, william.helsby at stfc.ac.uk wrote:
>> A boot time system crash was noticed with a segmentation fault just after the initrd image had been used to initialise the ramdisk.
>> This occurred when the U-Boot loaded the ramdisk image from a FAT partition, but not when loaded by TFTPBOOT. This is not understood?
>> However the problem was caused by free_initrd_mem freeing and 
>> "poisoning" memory that had been allocted to init/main.c to store the saved_command_line.
>> This patch reverses "ARM: 8167/1: extend the reserved memory for initrd to be page aligned" 
>> because it is safer to leave a partial head or tail page reserved (wasted) than to free a page which is partially still in use.
>> If this is not acceptable (particularly if wanting large contiguous physical areas for DMA) then a better solution is required.
>> This would extend the region reserved to page boundaries, if possible without overlapping other regions. 
>>My previous attempt to fix this coded this scheme, to grow the are reserved. 
>> However, this? again is not safe if in growing the area it then overlaps a region that is in use.
>> Note this patch is against the 4.6 kernel, but as far as I can tell applies equally to 4.8.
>> diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c index 
> 370581a..ff3e9c3 100644
>> --- a/arch/arm/mm/init.c
>> +++ b/arch/arm/mm/init.c
>> @@ -770,12 +770,6 @@ static int keep_initrd; void 
>> free_initrd_mem(unsigned long start, unsigned long end) {
>> ??????? if (!keep_initrd) {
>> -?????????????? if (start == initrd_start)
>> -?????????????????????? start = round_down(start, PAGE_SIZE);
>> -?????????????? if (end == initrd_end)
>> -?????????????????????? end = round_up(end, PAGE_SIZE);
>> -
>> -???? ??????????poison_init_mem((void *)start, PAGE_ALIGN(end) - 
>> start);

> We're definitely not getting rid of the poisoning of the pages - the poisoning there is to detect accesses to this memory which should not be made.

> The point of rounding up and down is to ensure that the partly-used pages (which would have been previously reserved) are freed.

> Probably a better fix is to round the start up/end down of the initrd when reserving the memory region:

>  arch/arm/mm/init.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c index 370581aeb871..ee8509e4329d 100644
> --- a/arch/arm/mm/init.c
> +++ b/arch/arm/mm/init.c
> @@ -255,7 +255,11 @@ void __init arm_memblock_init(const struct machine_desc *mdesc)
>  		phys_initrd_start = phys_initrd_size = 0;
>  	}
>  	if (phys_initrd_size) {
> -		memblock_reserve(phys_initrd_start, phys_initrd_size);
> +		phys_addr_t start, size;
> +
> +		start = round_down(phys_initrd_start, PAGE_SIZE);
> +		end = round_up(phys_initrd_start + phys_initrd_size, PAGE_SIZE);
> +		memblock_reserve(start, end - start);
>  
>  		/* Now convert initrd to virtual addresses */
>  		initrd_start = __phys_to_virt(phys_initrd_start); 
>
> and this should ensure that memblock_alloc() doesn't try to allocate memory overlapping the pages containing the initrd.

> Intentionally using pages overlapping the initrd is a recipe for problems...

> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.

^ permalink raw reply related

* [RFC v2 8/8] iommu/arm-smmu: implement add_reserved_regions callback
From: Auger Eric @ 2016-11-11 15:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161111114223.GP2078@8bytes.org>

Hi Joerg,

On 11/11/2016 12:42, Joerg Roedel wrote:
> On Thu, Nov 10, 2016 at 07:00:52PM +0100, Auger Eric wrote:
>> GICv2m and GICV3 ITS use dma-mapping iommu_dma_map_msi_msg to allocate
>> an MSI IOVA on-demand.
> 
> Yes, and it the right thing to do there because as a DMA-API
> implementation the dma-iommu code cares about the address space
> allocation.
> 
> As I understand it this is different in your case, as someone else is
> defining the address space layout. So why do you need to allocate it
> yourself?
Effectively in passthrough use case, the userspace defines the address
space layout and maps guest RAM PA=IOVA to PAs (using
VFIO_IOMMU_MAP_DMA). But this address space does not comprise the MSI
IOVAs. Userspace does not care about MSI IOMMU mapping. So the MSI IOVA
region must be allocated by either the VFIO driver or the IOMMU driver I
think. Who else could initialize the IOVA allocator domain?

That's true that we have a mix of unmanaged addresses and "managed"
addresses which is not neat. But how to manage otherwise?

Thanks

Eric
> 
> 
> 	Joerg
> 

^ permalink raw reply

* [RFC PATCH 6/6] [media] davinci: vpif_capture: get subdevs from DT
From: Javier Martinez Canillas @ 2016-11-11 15:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <dad6e38b-093b-bd36-3e0d-a0c10bddea58@xs4all.nl>

Hello,

On Fri, Nov 11, 2016 at 12:36 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On 10/26/2016 01:55 AM, Kevin Hilman wrote:
>> First pass at getting subdevs from DT ports and endpoints.
>>
>> The _get_pdata() function was larely inspired by (i.e. stolen from)
>> am437x-vpfe.c
>>
>> Questions:
>> - Legacy board file passes subdev input & output routes via pdata
>>   (e.g. tvp514x svideo or composite selection.)  How is this supposed
>>   to be done via DT?
>
> We have plans to model connectors as well in the device tree, but no
> implementation exists yet. I think Laurent has some code in progress for this,
> but I may be mistaken.
>

I posted a RFC series [0] some time ago, that proposed a DT binding
for input connectors [1] using OF graphs.

I never re-spin the series because Laurent had some comments on the DT
bindings and I was waiting for a response on to my latest email [2].
So if you can comment on this and see if the DT bindings fits your,
would be very useful.

> Anyway, hard-coding it like you do now is for now the only way.
>
>         Hans
>
>>

[0]: https://www.mail-archive.com/linux-media at vger.kernel.org/msg96393.html
[1]: http://www.spinics.net/lists/linux-media/msg99421.html
[2]: http://www.spinics.net/lists/linux-media/msg99987.html

Best regards,
Javier

^ permalink raw reply

* Summary of LPC guest MSI discussion in Santa Fe
From: Alex Williamson @ 2016-11-11 15:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161111111944.GO2078@8bytes.org>

On Fri, 11 Nov 2016 12:19:44 +0100
Joerg Roedel <joro@8bytes.org> wrote:

> On Thu, Nov 10, 2016 at 10:46:01AM -0700, Alex Williamson wrote:
> > In the case of x86, we know that DMA mappings overlapping the MSI
> > doorbells won't be translated correctly, it's not a valid mapping for
> > that range, and therefore the iommu driver backing the IOMMU API
> > should describe that reserved range and reject mappings to it.  
> 
> The drivers actually allow mappings to the MSI region via the IOMMU-API,
> and I think it should stay this way also for other reserved ranges.
> Address space management is done by the IOMMU-API user already (and has
> to be done there nowadays), be it a DMA-API implementation which just
> reserves these regions in its address space allocator or be it VFIO with
> QEMU, which don't map RAM there anyway. So there is no point of checking
> this again in the IOMMU drivers and we can keep that out of the
> mapping/unmapping fast-path.

It's really just a happenstance that we don't map RAM over the x86 MSI
range though.  That property really can't be guaranteed once we mix
architectures, such as running an aarch64 VM on x86 host via TCG.
AIUI, the MSI range is actually handled differently than other DMA
ranges, so a iommu_map() overlapping a range that the iommu cannot map
should fail just like an attempt to map beyond the address width of the
iommu.
 
> > For PCI devices userspace can examine the topology of the iommu group
> > and exclude MMIO ranges of peer devices based on the BARs, which are
> > exposed in various places, pci-sysfs as well as /proc/iomem.  For
> > non-PCI or MSI controllers... ???  
> 
> Right, the hardware resources can be examined. But maybe this can be
> extended to also cover RMRR ranges? Then we would be able to assign
> devices with RMRR mappings to guests.

RMRRs are special in a different way, the VT-d spec requires that the
OS honor RMRRs, the user has no responsibility (and currently no
visibility) to make that same arrangement.  In order to potentially
protect the physical host platform, the iommu drivers should prevent a
user from remapping RMRRS.  Maybe there needs to be a different
interface used by untrusted users vs in-kernel drivers, but I think the
kernel really needs to be defensive in the case of user mappings, which
is where the IOMMU API is rooted.  Thanks,

Alex

^ permalink raw reply

* Applied "regulator: gpio: properly check return value of of_get_named_gpio" to the regulator tree
From: Mark Brown @ 2016-11-11 15:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161110092129.2203-1-jszhang@marvell.com>

The patch

   regulator: gpio: properly check return value of of_get_named_gpio

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 09f2ba0b0b7c44ecea49cf69a708203b76ba5535 Mon Sep 17 00:00:00 2001
From: Jisheng Zhang <jszhang@marvell.com>
Date: Thu, 10 Nov 2016 17:21:29 +0800
Subject: [PATCH] regulator: gpio: properly check return value of
 of_get_named_gpio

The function of_get_named_gpio() could return -ENOENT, -EPROBE_DEFER
-EINVAL and so on. Currently, for the optional property "enable-gpio",
we only check -EPROBE_DEFER, this is not enough since there may be
misconfigured "enable-gpio" in the DTB, of_get_named_gpio() will return
-EINVAL in this case, we should return immediately here. And for the
optional property "gpios", we didn't check the return value, the driver
will continue to the point where gpio_request_array() is called, it
doesn't make sense to continue if we got -EPROBE_DEFER or -EINVAL here.

This patch tries to address these two issues by properly checking the
return value of of_get_named_gpio.

Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/gpio-regulator.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/gpio-regulator.c b/drivers/regulator/gpio-regulator.c
index 83e89e5d4752..0fce06acfaec 100644
--- a/drivers/regulator/gpio-regulator.c
+++ b/drivers/regulator/gpio-regulator.c
@@ -162,8 +162,8 @@ of_get_gpio_regulator_config(struct device *dev, struct device_node *np,
 	of_property_read_u32(np, "startup-delay-us", &config->startup_delay);
 
 	config->enable_gpio = of_get_named_gpio(np, "enable-gpio", 0);
-	if (config->enable_gpio == -EPROBE_DEFER)
-		return ERR_PTR(-EPROBE_DEFER);
+	if (config->enable_gpio < 0 && config->enable_gpio != -ENOENT)
+		return ERR_PTR(config->enable_gpio);
 
 	/* Fetch GPIOs. - optional property*/
 	ret = of_gpio_count(np);
@@ -190,8 +190,11 @@ of_get_gpio_regulator_config(struct device *dev, struct device_node *np,
 
 		for (i = 0; i < config->nr_gpios; i++) {
 			gpio = of_get_named_gpio(np, "gpios", i);
-			if (gpio < 0)
+			if (gpio < 0) {
+				if (gpio != -ENOENT)
+					return ERR_PTR(gpio);
 				break;
+			}
 			config->gpios[i].gpio = gpio;
 			if (proplen > 0) {
 				of_property_read_u32_index(np, "gpios-states",
-- 
2.10.2

^ permalink raw reply related

* Applied "regulator: axp20x: Fix axp809 ldo_io registration error on cold boot" to the regulator tree
From: Mark Brown @ 2016-11-11 15:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161111031243.20236-1-wens@csie.org>

The patch

   regulator: axp20x: Fix axp809 ldo_io registration error on cold boot

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 618c808968852609d2d9f0e5cfc351a4807ef8d0 Mon Sep 17 00:00:00 2001
From: Chen-Yu Tsai <wens@csie.org>
Date: Fri, 11 Nov 2016 11:12:43 +0800
Subject: [PATCH] regulator: axp20x: Fix axp809 ldo_io registration error on
 cold boot

The maximum supported voltage for ldo_io# is 3.3V, but on cold boot
the selector comes up at 0x1f, which maps to 3.8V. This was previously
corrected by Allwinner's U-boot, which set all regulators on the PMICs
to some pre-configured voltage. With recent progress in U-boot SPL
support, this is no longer the case. In any case we should handle
this quirk in the kernel driver as well.

This invalid setting causes _regulator_get_voltage() to fail with -EINVAL
which causes regulator registration to fail when constrains are used:

[    1.054181] vcc-pg: failed to get the current voltage(-22)
[    1.059670] axp20x-regulator axp20x-regulator.0: Failed to register ldo_io0
[    1.069749] axp20x-regulator: probe of axp20x-regulator.0 failed with error -22

This commits makes the axp20x regulator driver accept the 0x1f register
value, fixing this.

The datasheet does not guarantee reliable operation above 3.3V, so on
boards where this regulator is used the regulator-max-microvolt setting
must be 3.3V or less.

This is essentially the same as the commit f40d4896bf32 ("regulator:
axp20x: Fix axp22x ldo_io registration error on cold boot") for AXP22x
PMICs.

Fixes: a51f9f4622a3 ("regulator: axp20x: support AXP809 variant")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/axp20x-regulator.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 54382ef902c6..e6a512ebeae2 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -337,10 +337,18 @@ static const struct regulator_desc axp809_regulators[] = {
 		 AXP22X_ELDO2_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2, BIT(1)),
 	AXP_DESC(AXP809, ELDO3, "eldo3", "eldoin", 700, 3300, 100,
 		 AXP22X_ELDO3_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2, BIT(2)),
-	AXP_DESC_IO(AXP809, LDO_IO0, "ldo_io0", "ips", 700, 3300, 100,
+	/*
+	 * Note the datasheet only guarantees reliable operation up to
+	 * 3.3V, this needs to be enforced via dts provided constraints
+	 */
+	AXP_DESC_IO(AXP809, LDO_IO0, "ldo_io0", "ips", 700, 3800, 100,
 		    AXP22X_LDO_IO0_V_OUT, 0x1f, AXP20X_GPIO0_CTRL, 0x07,
 		    AXP22X_IO_ENABLED, AXP22X_IO_DISABLED),
-	AXP_DESC_IO(AXP809, LDO_IO1, "ldo_io1", "ips", 700, 3300, 100,
+	/*
+	 * Note the datasheet only guarantees reliable operation up to
+	 * 3.3V, this needs to be enforced via dts provided constraints
+	 */
+	AXP_DESC_IO(AXP809, LDO_IO1, "ldo_io1", "ips", 700, 3800, 100,
 		    AXP22X_LDO_IO1_V_OUT, 0x1f, AXP20X_GPIO1_CTRL, 0x07,
 		    AXP22X_IO_ENABLED, AXP22X_IO_DISABLED),
 	AXP_DESC_FIXED(AXP809, RTC_LDO, "rtc_ldo", "ips", 1800),
-- 
2.10.2

^ permalink raw reply related

* [RFC PATCH 6/6] [media] davinci: vpif_capture: get subdevs from DT
From: Javier Martinez Canillas @ 2016-11-11 15:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CABxcv==ZR0=4GhXECiKtFJZ3WEY_5h-Egvs3hjBK56FiQeF6Jg@mail.gmail.com>

On Fri, Nov 11, 2016 at 12:50 PM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:

[snip]

> So if you can comment on this and see if the DT bindings fits your,
> would be very useful.
>

Sorry, I wanted to write "if the DT bindings fits your needs, it would
be very useful".

Best regards,
Javier

^ permalink raw reply

* [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Gabriele Paoloni @ 2016-11-11 15:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161111144539.GL10219@e106497-lin.cambridge.arm.com>

Hi Liviu

> -----Original Message-----
> From: liviu.dudau at arm.com [mailto:liviu.dudau at arm.com]
> Sent: 11 November 2016 14:46
> To: Gabriele Paoloni
> Cc: Arnd Bergmann; linux-arm-kernel at lists.infradead.org; Yuanzhichang;
> mark.rutland at arm.com; devicetree at vger.kernel.org;
> lorenzo.pieralisi at arm.com; minyard at acm.org; linux-pci at vger.kernel.org;
> benh at kernel.crashing.org; John Garry; will.deacon at arm.com; linux-
> kernel at vger.kernel.org; xuwei (O); Linuxarm; zourongrong at gmail.com;
> robh+dt at kernel.org; kantyzc at 163.com; linux-serial at vger.kernel.org;
> catalin.marinas at arm.com; olof at lixom.net; bhelgaas at googl e.com;
> zhichang.yuan02 at gmail.com
> Subject: Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on
> Hip06
> 
> On Fri, Nov 11, 2016 at 01:39:35PM +0000, Gabriele Paoloni wrote:
> > Hi Arnd
> >
> > > -----Original Message-----
> > > From: Arnd Bergmann [mailto:arnd at arndb.de]
> > > Sent: 10 November 2016 16:07
> > > To: Gabriele Paoloni
> > > Cc: linux-arm-kernel at lists.infradead.org; Yuanzhichang;
> > > mark.rutland at arm.com; devicetree at vger.kernel.org;
> > > lorenzo.pieralisi at arm.com; minyard at acm.org; linux-
> pci at vger.kernel.org;
> > > benh at kernel.crashing.org; John Garry; will.deacon at arm.com; linux-
> > > kernel at vger.kernel.org; xuwei (O); Linuxarm; zourongrong at gmail.com;
> > > robh+dt at kernel.org; kantyzc at 163.com; linux-serial at vger.kernel.org;
> > > catalin.marinas at arm.com; olof at lixom.net; liviu.dudau at arm.com;
> > > bhelgaas at googl e.com; zhichang.yuan02 at gmail.com
> > > Subject: Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on
> > > Hip06
> > >
> > > On Thursday, November 10, 2016 3:36:49 PM CET Gabriele Paoloni
> wrote:
> > > >
> > > > Where should we get the range from? For LPC we know that it is
> going
> > > > Work on anything that is not used by PCI I/O space, and this is
> > > > why we use [0, PCIBIOS_MIN_IO]
> > >
> > > It should be allocated the same way we allocate PCI config space
> > > segments. This is currently done with the io_range list in
> > > drivers/pci/pci.c, which isn't perfect but could be extended
> > > if necessary. Based on what others commented here, I'd rather
> > > make the differences between ISA/LPC and PCI I/O ranges smaller
> > > than larger.
> 
> Gabriele,
> 
> >
> > I am not sure this would make sense...
> >
> > IMHO all the mechanism around io_range_list is needed to provide the
> > "mapping" between I/O tokens and physical CPU addresses.
> >
> > Currently the available tokens range from 0 to IO_SPACE_LIMIT.
> >
> > As you know the I/O memory accessors operate on whatever
> > __of_address_to_resource sets into the resource (start, end).
> >
> > With this special device in place we cannot know if a resource is
> > assigned with an I/O token or a physical address, unless we forbid
> > the I/O tokens to be in a specific range.
> >
> > So this is why we are changing the offsets of all the functions
> > handling io_range_list (to make sure that a range is forbidden to
> > the tokens and is available to the physical addresses).
> >
> > We have chosen this forbidden range to be [0, PCIBIOS_MIN_IO)
> > because this is the maximum physical I/O range that a non PCI device
> > can operate on and because we believe this does not impose much
> > restriction on the available I/O token range; that now is
> > [PCIBIOS_MIN_IO, IO_SPACE_LIMIT].
> > So we believe that the chosen forbidden range can accommodate
> > any special ISA bus device with no much constraint on the rest
> > of I/O tokens...
> 
> Your idea is a good one, however you are abusing PCIBIOS_MIN_IO and you
> actually need another variable for "reserving" an area in the I/O space
> that can be used for physical addresses rather than I/O tokens.
> 
> The one good example for using PCIBIOS_MIN_IO is when your
> platform/architecture
> does not support legacy ISA operations *at all*. In that case someone
> sets the PCIBIOS_MIN_IO to a non-zero value to reserve that I/O range
> so that it doesn't get used. With Zhichang's patch you now start
> forcing
> those platforms to have a valid address below PCIBIOS_MIN_IO.

But if PCIBIOS_MIN_IO is 0 then it means that all I/O space is to be used
by PCI controllers only...so if you have a special bus device using
an I/O range in this case should be a PCI controller...i.e. I would
expect it to fall back into the case of I/O tokens redirection rather than
physical addresses redirection (as mentioned below from my previous reply).
What do you think?

Thanks

Gab


> 
> For the general case you also have to bear in mind that PCIBIOS_MIN_IO
> could
> be zero. In that case, what is your "forbidden" range? [0, 0) ? So it
> makes
> sense to add a new #define that should only be defined by those
> architectures/
> platforms that want to reserve on top of PCIBIOS_MIN_IO another region
> where I/O tokens can't be generated for.
> 
> Best regards,
> Liviu
> 
> >
> > >
> > > > > Your current version has
> > > > >
> > > > >         if (arm64_extio_ops->pfout)
> \
> > > > >                 arm64_extio_ops->pfout(arm64_extio_ops-
> >devpara,\
> > > > >                        addr, value, sizeof(type));
> \
> > > > >
> > > > > Instead, just subtract the start of the range from the logical
> > > > > port number to transform it back into a bus-local port number:
> > > >
> > > > These accessors do not operate on IO tokens:
> > > >
> > > > If (arm64_extio_ops->start > addr || arm64_extio_ops->end < addr)
> > > > addr is not going to be an I/O token; in fact patch 2/3 imposes
> that
> > > > the I/O tokens will start at PCIBIOS_MIN_IO. So from 0 to
> > > PCIBIOS_MIN_IO
> > > > we have free physical addresses that the accessors can operate
> on.
> > >
> > > Ah, I missed that part. I'd rather not use PCIBIOS_MIN_IO to refer
> to
> > > the logical I/O tokens, the purpose of that macro is really meant
> > > for allocating PCI I/O port numbers within the address space of
> > > one bus.
> >
> > As I mentioned above, special devices operate on CPU addresses
> directly,
> > not I/O tokens. For them there is no way to distinguish....
> >
> > >
> > > Note that it's equally likely that whichever next platform needs
> > > non-mapped I/O access like this actually needs them for PCI I/O
> space,
> > > and that will use it on addresses registered to a PCI host bridge.
> >
> > Ok so here you are talking about a platform that has got an I/O range
> > under the PCI host controller, right?
> > And this I/O range cannot be directly memory mapped but needs special
> > redirections for the I/O tokens, right?
> >
> > In this scenario registering the I/O ranges with the forbidden range
> > implemented by the current patch would still allow to redirect I/O
> > tokens as long as arm64_extio_ops->start >= PCIBIOS_MIN_IO
> >
> > So effectively the special PCI host controller
> > 1) knows the physical range that needs special redirection
> > 2) register such range
> > 3) uses pci_pio_to_address() to retrieve the IO tokens for the
> >    special accessors
> > 4) sets arm64_extio_ops->start/end to the IO tokens retrieved in 3)
> >
> > So to be honest I think this patch can fit well both with
> > special PCI controllers that need I/O tokens redirection and with
> > special non-PCI controllers that need non-PCI I/O physical
> > address redirection...
> >
> > Thanks (and sorry for the long reply but I didn't know how
> > to make the explanation shorter :) )
> >
> > Gab
> >
> > >
> > > If we separate the two steps:
> > >
> > > a) assign a range of logical I/O port numbers to a bus
> > > b) register a set of helpers for redirecting logical I/O
> > >    port to a helper function
> > >
> > > then I think the code will get cleaner and more flexible.
> > > It should actually then be able to replace the powerpc
> > > specific implementation.
> > >
> > > 	Arnd
> 
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ?\_(?)_/?

^ permalink raw reply

* [PATCH 1/2] ARM: dts: imx6q-cm-fx6: fix fec pinctrl
From: christopher.spinrath at rwth-aachen.de @ 2016-11-11 15:59 UTC (permalink / raw)
  To: linux-arm-kernel

From: Christopher Spinrath <christopher.spinrath@rwth-aachen.de>

According to the schematics of CompuLab's sbc-fx6 baseboard and the
vendor devicetree GPIO_16 is *not* muxed to ENET_REF_CLK but to SPDIF_IN.

Remove the wrong pinctrl setting.

Fixes: 682d055e6ac5 ("ARM: dts: Add initial support for cm-fx6.")
Signed-off-by: Christopher Spinrath <christopher.spinrath@rwth-aachen.de>
---
 arch/arm/boot/dts/imx6q-cm-fx6.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6q-cm-fx6.dts b/arch/arm/boot/dts/imx6q-cm-fx6.dts
index 59bc5a4..a150bca 100644
--- a/arch/arm/boot/dts/imx6q-cm-fx6.dts
+++ b/arch/arm/boot/dts/imx6q-cm-fx6.dts
@@ -183,7 +183,6 @@
 			MX6QDL_PAD_ENET_REF_CLK__ENET_TX_CLK	0x1b0b0
 			MX6QDL_PAD_ENET_MDIO__ENET_MDIO		0x1b0b0
 			MX6QDL_PAD_ENET_MDC__ENET_MDC		0x1b0b0
-			MX6QDL_PAD_GPIO_16__ENET_REF_CLK	0x4001b0a8
 		>;
 	};
 
-- 
2.10.2

^ permalink raw reply related

* [PATCH 2/2] ARM: dts: imx6q-utilite-pro: i2c1 is muxed
From: christopher.spinrath at rwth-aachen.de @ 2016-11-11 15:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161111155939.446-1-christopher.spinrath@rwth-aachen.de>

From: Christopher Spinrath <christopher.spinrath@rwth-aachen.de>

It turns out that the i2c1 adapter is connected to a multiplexer
controlled by a gpio line. The first (default) mux option connects
i2c1 to a bus connected to the already known peripherals. The other
one connects the adapter to the ddc pins of the DVI port.

Signed-off-by: Christopher Spinrath <christopher.spinrath@rwth-aachen.de>
---
 arch/arm/boot/dts/imx6q-utilite-pro.dts | 51 ++++++++++++++++++++++++++-------
 1 file changed, 40 insertions(+), 11 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q-utilite-pro.dts b/arch/arm/boot/dts/imx6q-utilite-pro.dts
index 6199063..246979a 100644
--- a/arch/arm/boot/dts/imx6q-utilite-pro.dts
+++ b/arch/arm/boot/dts/imx6q-utilite-pro.dts
@@ -71,6 +71,40 @@
 			gpio-key,wakeup;
 		};
 	};
+
+	i2cmux {
+		compatible = "i2c-mux-gpio";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_i2c1mux>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		mux-gpios = <&gpio1 2 GPIO_ACTIVE_HIGH>;
+		i2c-parent = <&i2c1>;
+
+		i2c at 0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			eeprom at 50 {
+				compatible = "at24,24c02";
+				reg = <0x50>;
+				pagesize = <16>;
+			};
+
+			em3027: rtc at 56 {
+				compatible = "emmicro,em3027";
+				reg = <0x56>;
+			};
+		};
+
+		i2c_dvi_ddc: i2c at 1 {
+			reg = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+	};
 };
 
 &hdmi {
@@ -82,17 +116,6 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_i2c1>;
 	status = "okay";
-
-	eeprom at 50 {
-		compatible = "at24,24c02";
-		reg = <0x50>;
-		pagesize = <16>;
-	};
-
-	em3027: rtc at 56 {
-		compatible = "emmicro,em3027";
-		reg = <0x56>;
-	};
 };
 
 &i2c2 {
@@ -115,6 +138,12 @@
 		>;
 	};
 
+	pinctrl_i2c1mux: i2c1muxgrp {
+		fsl,pins = <
+			MX6QDL_PAD_GPIO_2__GPIO1_IO02 0x1b0b0
+		>;
+	};
+
 	pinctrl_i2c2: i2c2grp {
 		fsl,pins = <
 			MX6QDL_PAD_KEY_COL3__I2C2_SCL 0x4001b8b1
-- 
2.10.2

^ permalink raw reply related

* Summary of LPC guest MSI discussion in Santa Fe
From: Don Dutile @ 2016-11-11 16:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161111111944.GO2078@8bytes.org>

On 11/11/2016 06:19 AM, Joerg Roedel wrote:
> On Thu, Nov 10, 2016 at 10:46:01AM -0700, Alex Williamson wrote:
>> In the case of x86, we know that DMA mappings overlapping the MSI
>> doorbells won't be translated correctly, it's not a valid mapping for
>> that range, and therefore the iommu driver backing the IOMMU API
>> should describe that reserved range and reject mappings to it.
>
> The drivers actually allow mappings to the MSI region via the IOMMU-API,
> and I think it should stay this way also for other reserved ranges.
> Address space management is done by the IOMMU-API user already (and has
> to be done there nowadays), be it a DMA-API implementation which just
> reserves these regions in its address space allocator or be it VFIO with
> QEMU, which don't map RAM there anyway. So there is no point of checking
> this again in the IOMMU drivers and we can keep that out of the
> mapping/unmapping fast-path.
>
>> For PCI devices userspace can examine the topology of the iommu group
>> and exclude MMIO ranges of peer devices based on the BARs, which are
>> exposed in various places, pci-sysfs as well as /proc/iomem.  For
>> non-PCI or MSI controllers... ???
>
> Right, the hardware resources can be examined. But maybe this can be
> extended to also cover RMRR ranges? Then we would be able to assign
> devices with RMRR mappings to guests.
>
eh gads no!

Assigning devices w/RMRR's is a security issue waiting to happen, if
it doesn't crash the system before the guest even gets the device --
reset the device before assignment; part of device is gathering system
environmental data; if BIOS/SMM support doesn't get env. data update,
it NMI's the system..... in fear that it may overheat ...


>
>
> 	Joerg
>

^ permalink raw reply

* ??
From: Steven Rostedt @ 2016-11-11 16:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAG2=9p_3nLD6MuLXPsbfC8LqK76LbwD3BDRUjX3uEUrijZnYCg@mail.gmail.com>

On Fri, 11 Nov 2016 11:38:45 +0800
Chunyan Zhang <zhang.chunyan@linaro.org> wrote:


What happened to the subject?

> >>> +static void
> >>> +trace_process_export(struct trace_export *export,
> >>> +            struct ring_buffer_event *event)
> >>> +{
> >>> +     struct trace_entry *entry;
> >>> +     unsigned int size = 0;
> >>> +
> >>> +     entry = ring_buffer_event_data(event);
> >>> +
> >>> +     size = ring_buffer_event_length(event);
> >>> +
> >>> +     if (export->write)
> >>> +             export->write((char *)entry, size);  
> >>
> >> Is there ever going to be a time where export->write wont be set?  
> >
> > There hasn't been since only one trace_export (i.e. stm_ftrace) was
> > added in this patch-set , I just wanted to make sure the write() has
> > been set before registering trace_export like what I added in 2/3 of
> > this series.
> >  
> >>
> >> And if there is, this can be racy. As in
> >>
> >>
> >>         CPU 0:                  CPU 1:
> >>         ------                  ------
> >>         if (export->write)
> >>
> >>                                 export->write = NULL;  
> >
> > Is there going to be this kind of use case? Why some one needs to
> > change export->write() rather than register a new trace_export?
> >
> > I probably haven't understood your point thoroughly, please correct me
> > if my guess was wrong.
> >  
> 
> Any further comments? :)

I don't remember which patch series this goes to, so right now, no.

-- Steve

^ permalink raw reply

* Summary of LPC guest MSI discussion in Santa Fe
From: Alex Williamson @ 2016-11-11 16:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161111085056.4cf8989d@t450s.home>

On Fri, 11 Nov 2016 08:50:56 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:

> On Fri, 11 Nov 2016 12:19:44 +0100
> Joerg Roedel <joro@8bytes.org> wrote:
> 
> > On Thu, Nov 10, 2016 at 10:46:01AM -0700, Alex Williamson wrote:  
> > > In the case of x86, we know that DMA mappings overlapping the MSI
> > > doorbells won't be translated correctly, it's not a valid mapping for
> > > that range, and therefore the iommu driver backing the IOMMU API
> > > should describe that reserved range and reject mappings to it.    
> > 
> > The drivers actually allow mappings to the MSI region via the IOMMU-API,
> > and I think it should stay this way also for other reserved ranges.
> > Address space management is done by the IOMMU-API user already (and has
> > to be done there nowadays), be it a DMA-API implementation which just
> > reserves these regions in its address space allocator or be it VFIO with
> > QEMU, which don't map RAM there anyway. So there is no point of checking
> > this again in the IOMMU drivers and we can keep that out of the
> > mapping/unmapping fast-path.  
> 
> It's really just a happenstance that we don't map RAM over the x86 MSI
> range though.  That property really can't be guaranteed once we mix
> architectures, such as running an aarch64 VM on x86 host via TCG.
> AIUI, the MSI range is actually handled differently than other DMA
> ranges, so a iommu_map() overlapping a range that the iommu cannot map
> should fail just like an attempt to map beyond the address width of the
> iommu.

(clarification, this is x86 specific, the MSI controller - interrupt
remapper - is embedded in the iommu AIUI, so the iommu is actually not
able to provide DMA translation for this range.  In architectures where
the MSI controller is separate from the iommu, I agree that the iommu
has no responsibility to fault mapping of iova ranges in the shadow of
an external MSI controller)
 
> > > For PCI devices userspace can examine the topology of the iommu group
> > > and exclude MMIO ranges of peer devices based on the BARs, which are
> > > exposed in various places, pci-sysfs as well as /proc/iomem.  For
> > > non-PCI or MSI controllers... ???    
> > 
> > Right, the hardware resources can be examined. But maybe this can be
> > extended to also cover RMRR ranges? Then we would be able to assign
> > devices with RMRR mappings to guests.  
> 
> RMRRs are special in a different way, the VT-d spec requires that the
> OS honor RMRRs, the user has no responsibility (and currently no
> visibility) to make that same arrangement.  In order to potentially
> protect the physical host platform, the iommu drivers should prevent a
> user from remapping RMRRS.  Maybe there needs to be a different
> interface used by untrusted users vs in-kernel drivers, but I think the
> kernel really needs to be defensive in the case of user mappings, which
> is where the IOMMU API is rooted.  Thanks,
> 
> Alex
> _______________________________________________
> iommu mailing list
> iommu at lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply

* [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic
From: Robin Murphy @ 2016-11-11 16:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161111152248.GS2078@8bytes.org>

On 11/11/16 15:22, Joerg Roedel wrote:
> On Wed, Nov 09, 2016 at 02:19:36PM +0000, Lorenzo Pieralisi wrote:
>> +struct iommu_fwentry {
>> +	struct list_head list;
>> +	struct fwnode_handle *fwnode;
>> +	const struct iommu_ops *ops;
>> +};
> 
> Is there a reason the iommu_ops need to be stored there? Currently it
> seems that the ops are only needed to get the of_xlate fn-ptr later. And
> the place where it is called the iommu-ops should also be available
> through pdev->dev->bus->iommu_ops.

In the original of_iommu_configure design, the thought was that an ops
structure could be IOMMU-instance-specific (hence the later-removed
"priv" member), so I suppose right now it is mostly a hangover from
that. However, it's also what we initialise a device's fwspec with, so
becomes important again if we're ever going to get past the limitations
of buses-which-are-not-actually-buses[1].

Robin.

[1]:http://www.mail-archive.com/iommu at lists.linux-foundation.org/msg14576.html

^ permalink raw reply

* [RFC v2 8/8] iommu/arm-smmu: implement add_reserved_regions callback
From: Joerg Roedel @ 2016-11-11 16:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <79da09b8-ac1e-cebf-5393-7d67f002b3e3@redhat.com>

Hi Eric,

On Fri, Nov 11, 2016 at 04:47:01PM +0100, Auger Eric wrote:
> Effectively in passthrough use case, the userspace defines the address
> space layout and maps guest RAM PA=IOVA to PAs (using
> VFIO_IOMMU_MAP_DMA). But this address space does not comprise the MSI
> IOVAs. Userspace does not care about MSI IOMMU mapping. So the MSI IOVA
> region must be allocated by either the VFIO driver or the IOMMU driver I
> think. Who else could initialize the IOVA allocator domain?

So I think we need a way to tell userspace about the reserved regions
(per iommu-group) so that userspace knows where it can not map anything,
and VFIO can enforce that. But the right struct here is not an
iova-allocator rb-tree, a ordered linked list should be sufficient.


	Joerg

^ 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