Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] iommu/arm-smmu: Fix build issues
From: Will Deacon @ 2019-08-20 11:41 UTC (permalink / raw)
  To: Robin Murphy; +Cc: joro, iommu, linux-arm-kernel
In-Reply-To: <909636ed9dfc702a7cb4806903e3e698ce9b29a6.1566301129.git.robin.murphy@arm.com>

On Tue, Aug 20, 2019 at 12:38:49PM +0100, Robin Murphy wrote:
> In a hurry to get things ready for merging, we missed that one more
> include needs moving to arm-smmu.h along with the register accessors
> to prevent 32-bit builds breaking, and some missing static specifiers
> made Sparse sad.

I already fixed the static stuff locally (and I think you missed
'calxeda_impl'), so I'll reword this and just take the missing include.

Thanks,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 03/14] arm64, hibernate: add trans_table public functions
From: Pavel Tatashin @ 2019-08-20 11:41 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Sasha Levin, Vladimir Murzin, Jonathan Corbet, Marc Zyngier,
	Catalin Marinas, Bhupesh Sharma, kexec mailing list, LKML,
	James Morris, linux-mm, James Morse, Eric W. Biederman,
	Matthias Brugger, will, Linux ARM
In-Reply-To: <20190820113000.GA49252@lakrids.cambridge.arm.com>

> > > While the architecture uses the term 'translation table', in the kernel
> > > we generally use 'pgdir' or 'pgd' to refer to the tables, so please keep
> > > to that naming scheme.
> >
> > The idea is to have a unique name space for new subsystem of page
> > tables that are used between kernels:
> > between stage 1 and stage 2 kexec kernel, and similarly between
> > kernels during hibernate boot process.
> >
> > I picked: "trans_table" that stands for transitional page table:
> > meaning they are used only during transition between worlds.
> >
> > All public functions in this subsystem will have trans_table_* prefix,
> > and page directory will be named: "trans_table". If this is confusing,
> > I can either use a different prefix, or describe what "trans_table"
> > stand for in trans_table.h/.c
>
> Ok.
>
> I think that "trans_table" is unfortunately confusing, as it clashes
> with the architecture terminology, and differs from what we have
> elsewhere.
>
> I think that "trans_pgd" would be better, as that better aligns with
> what we have elsewhere, and avoids the ambiguity.
>

Sounds good. I will rename trans_table* with trans_pgd*, and will also
add a note to the comments explaining what it stands for.

Thank you,
Pasha

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v7 1/5] dt-bindings: media: Add Allwinner A10 CSI binding
From: Sakari Ailus @ 2019-08-20 11:48 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, Rob Herring, Maxime Ripard,
	linux-kernel, Chen-Yu Tsai, Rob Herring, Hans Verkuil,
	Laurent Pinchart, Thomas Petazzoni, Mauro Carvalho Chehab,
	Frank Rowand, linux-arm-kernel, linux-media
In-Reply-To: <f490b35e62c5fd15174b5241ce1653e991c8fc9e.1566300265.git-series.maxime.ripard@bootlin.com>

Hi Maxime,

On Tue, Aug 20, 2019 at 01:24:32PM +0200, Maxime Ripard wrote:
> From: Maxime Ripard <maxime.ripard@bootlin.com>
> 
> The Allwinner A10 CMOS Sensor Interface is a camera capture interface also
> used in later (A10s, A13, A20, R8 and GR8) SoCs.
> 
> On some SoCs, like the A10, there's multiple instances of that controller,
> with one instance supporting more channels and having an ISP.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 107 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> new file mode 100644
> index 000000000000..9000bca344f9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> @@ -0,0 +1,107 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/allwinner,sun4i-a10-csi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Allwinner A10 CMOS Sensor Interface (CSI) Device Tree Bindings
> +
> +maintainers:
> +  - Chen-Yu Tsai <wens@csie.org>
> +  - Maxime Ripard <maxime.ripard@bootlin.com>
> +
> +description: |-
> +  The Allwinner A10 and later has a CMOS Sensor Interface to retrieve
> +  frames from a parallel or BT656 sensor.
> +
> +properties:
> +  compatible:
> +    const: allwinner,sun7i-a20-csi0
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: The CSI interface clock
> +      - description: The CSI module clock
> +      - description: The CSI ISP clock
> +      - description: The CSI DRAM clock
> +
> +  clock-names:
> +    items:
> +      - const: bus
> +      - const: mod
> +      - const: isp
> +      - const: ram
> +
> +  resets:
> +    maxItems: 1
> +
> +  port:
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      endpoint:
> +        properties:
> +          bus-width:
> +            const: 8
> +            description: Number of data lines actively used.

Are other values supported? If not, you could omit this.

> +
> +          data-active: true
> +          hsync-active: true
> +          pclk-sample: true
> +          remote-endpoint: true
> +          vsync-active: true
> +
> +        required:
> +          - bus-width
> +          - data-active
> +          - hsync-active
> +          - pclk-sample
> +          - remote-endpoint
> +          - vsync-active

Some of these are not allowed in the Bt.656 mode (vsync-active and
hsync-active) while they're required in Bt.601 mode. Is there a way to tell
that in YAML-based bindings?

Similarly, video-interfaces.txt should be referenced from here, shouldn't
it?

> +
> +    required:
> +      - endpoint
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/sun7i-a20-ccu.h>
> +    #include <dt-bindings/reset/sun4i-a10-ccu.h>
> +
> +    csi0: csi@1c09000 {
> +        compatible = "allwinner,sun7i-a20-csi0";
> +        reg = <0x01c09000 0x1000>;
> +        interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&ccu CLK_AHB_CSI0>, <&ccu CLK_CSI0>,
> +                 <&ccu CLK_CSI_SCLK>, <&ccu CLK_DRAM_CSI0>;
> +        clock-names = "bus", "mod", "isp", "ram";
> +        resets = <&ccu RST_CSI0>;
> +
> +        port {
> +            csi_from_ov5640: endpoint {
> +                remote-endpoint = <&ov5640_to_csi>;
> +                bus-width = <8>;
> +                hsync-active = <1>; /* Active high */
> +                vsync-active = <0>; /* Active low */
> +                data-active = <1>;  /* Active high */
> +                pclk-sample = <1>;  /* Rising */
> +            };
> +        };
> +    };
> +
> +...

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] efi/arm: fix allocation failure when reserving the kernel base
From: Russell King - ARM Linux admin @ 2019-08-20 11:54 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Juergen Gross, Joey Lee, linux-efi@vger.kernel.org,
	guillaume.gardet@arm.com, linux-kernel@vger.kernel.org,
	rppt@linux.ibm.com, Chester Lin, geert@linux-m68k.org,
	ren_guo@c-sky.com, Gary Lin, akpm@linux-foundation.org,
	mingo@kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAKv+Gu-yaNYsLQOOcr8srW91-nt-w0e+RBqxXGOagiGGT69n1Q@mail.gmail.com>

On Sun, Aug 04, 2019 at 10:57:00AM +0300, Ard Biesheuvel wrote:
> (The first TEXT_OFFSET bytes are no longer used in practice, which is
> why putting a reserved region of 4 KB bytes works at the moment, but
> this is fragile).

That is not correct for 32-bit ARM.  The swapper page table is still
located 16kiB below the kernel.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] efi/arm: fix allocation failure when reserving the kernel base
From: Russell King - ARM Linux admin @ 2019-08-20 11:56 UTC (permalink / raw)
  To: Chester Lin
  Cc: Juergen Gross, linux-efi@vger.kernel.org,
	ard.biesheuvel@linaro.org, guillaume.gardet@arm.com,
	linux-kernel@vger.kernel.org, rppt@linux.ibm.com, Joey Lee,
	geert@linux-m68k.org, ren_guo@c-sky.com, Gary Lin,
	akpm@linux-foundation.org, mingo@kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190802053744.5519-1-clin@suse.com>

On Fri, Aug 02, 2019 at 05:38:54AM +0000, Chester Lin wrote:
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index f3ce34113f89..909b11ba48d8 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1184,6 +1184,9 @@ void __init adjust_lowmem_bounds(void)
>  		phys_addr_t block_start = reg->base;
>  		phys_addr_t block_end = reg->base + reg->size;
>  
> +		if (memblock_is_nomap(reg))
> +			continue;
> +
>  		if (reg->base < vmalloc_limit) {
>  			if (block_end > lowmem_limit)
>  				/*

I think this hunk is sane - if the memory is marked nomap, then it isn't
available for the kernel's use, so as far as calculating where the
lowmem/highmem boundary is, it effectively doesn't exist and should be
skipped.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 1/4] iommu/arm-smmu: Introduce wrapper for writeq/readq
From: Will Deacon @ 2019-08-20 12:08 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: devicetree, Jason Cooper, Andrew Lunn, Antoine Tenart,
	Catalin Marinas, Joerg Roedel, Will Deacon, linux-kernel,
	Maxime Chevallier, Nadav Haklai, iommu, Rob Herring,
	Thomas Petazzoni, Miquèl Raynal, Robin Murphy, Hanna Hawa,
	linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <20190711150242.25290-2-gregory.clement@bootlin.com>

Hi Gregory, Hanna,

On Thu, Jul 11, 2019 at 05:02:39PM +0200, Gregory CLEMENT wrote:
> From: Hanna Hawa <hannah@marvell.com>
> 
> This patch introduces the smmu_writeq_relaxed/smmu_readq_relaxed
> helpers, as preparation to add specific Marvell work-around for
> accessing 64 bits width registers of ARM SMMU.
> 
> Signed-off-by: Hanna Hawa <hannah@marvell.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> ---
>  drivers/iommu/arm-smmu.c | 36 +++++++++++++++++++++++++++---------
>  1 file changed, 27 insertions(+), 9 deletions(-)

Sorry for the delay in replying to this -- Robin's been reworking the driver
so that implementation quirks can be specified more cleanly. Please can you
take a look at:

https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-joerg/arm-smmu/refactoring

and try to respin your patches on top of that?

Thanks,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] efi/arm: fix allocation failure when reserving the kernel base
From: Ard Biesheuvel @ 2019-08-20 12:27 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Juergen Gross, Joey Lee, linux-efi@vger.kernel.org,
	guillaume.gardet@arm.com, linux-kernel@vger.kernel.org,
	rppt@linux.ibm.com, Chester Lin, geert@linux-m68k.org,
	ren_guo@c-sky.com, Gary Lin, akpm@linux-foundation.org,
	mingo@kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190820115409.GO13294@shell.armlinux.org.uk>

On Tue, 20 Aug 2019 at 14:54, Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Sun, Aug 04, 2019 at 10:57:00AM +0300, Ard Biesheuvel wrote:
> > (The first TEXT_OFFSET bytes are no longer used in practice, which is
> > why putting a reserved region of 4 KB bytes works at the moment, but
> > this is fragile).
>
> That is not correct for 32-bit ARM.  The swapper page table is still
> located 16kiB below the kernel.
>

Right. So that means we can only permit reserved regions in the first
(TEXT_OFFSET - 16 kiB) bytes starting at the first 128 MiB aligned
address covered by system RAM, if we want to ensure that the
decompressor or the early kernel don't trample on it. (or TEXT_OFFSET
- 20 kiB for LPAE kernels)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] efi/arm: fix allocation failure when reserving the kernel base
From: Ard Biesheuvel @ 2019-08-20 12:28 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Juergen Gross, Joey Lee, linux-efi@vger.kernel.org,
	guillaume.gardet@arm.com, linux-kernel@vger.kernel.org,
	rppt@linux.ibm.com, Chester Lin, geert@linux-m68k.org,
	ren_guo@c-sky.com, Gary Lin, akpm@linux-foundation.org,
	mingo@kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190820115645.GP13294@shell.armlinux.org.uk>

On Tue, 20 Aug 2019 at 14:56, Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Fri, Aug 02, 2019 at 05:38:54AM +0000, Chester Lin wrote:
> > diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> > index f3ce34113f89..909b11ba48d8 100644
> > --- a/arch/arm/mm/mmu.c
> > +++ b/arch/arm/mm/mmu.c
> > @@ -1184,6 +1184,9 @@ void __init adjust_lowmem_bounds(void)
> >               phys_addr_t block_start = reg->base;
> >               phys_addr_t block_end = reg->base + reg->size;
> >
> > +             if (memblock_is_nomap(reg))
> > +                     continue;
> > +
> >               if (reg->base < vmalloc_limit) {
> >                       if (block_end > lowmem_limit)
> >                               /*
>
> I think this hunk is sane - if the memory is marked nomap, then it isn't
> available for the kernel's use, so as far as calculating where the
> lowmem/highmem boundary is, it effectively doesn't exist and should be
> skipped.
>

I agree.

Chester, could you explain what you need beyond this change (and my
EFI stub change involving TEXT_OFFSET) to make things work on the
RPi2?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v7 3/5] media: sunxi: Add A10 CSI driver
From: Sakari Ailus @ 2019-08-20 12:42 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, Maxime Ripard, linux-kernel,
	Chen-Yu Tsai, Rob Herring, Hans Verkuil, Laurent Pinchart,
	Thomas Petazzoni, Mauro Carvalho Chehab, Frank Rowand,
	linux-arm-kernel, linux-media
In-Reply-To: <6fdba6da624e2426937ca756d3a2c400dfd06993.1566300265.git-series.maxime.ripard@bootlin.com>

Hi Maxime,

Thanks for the update.

On Tue, Aug 20, 2019 at 01:24:34PM +0200, Maxime Ripard wrote:
> From: Maxime Ripard <maxime.ripard@bootlin.com>
> 
> The older CSI drivers have camera capture interface different from the one
> in the newer ones.
> 
> This IP is pretty simple. Some variants (one controller out of two
> instances on some SoCs) have an ISP embedded, but there's no code that make
> use of it, so we ignored that part for now.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  MAINTAINERS                                         |   8 +-
>  drivers/media/platform/sunxi/Kconfig                |   1 +-
>  drivers/media/platform/sunxi/Makefile               |   1 +-
>  drivers/media/platform/sunxi/sun4i-csi/Kconfig      |  11 +-
>  drivers/media/platform/sunxi/sun4i-csi/Makefile     |   5 +-
>  drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c  | 305 +++++++++-
>  drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h  | 159 +++++-
>  drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c  | 444 +++++++++++++-
>  drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c | 383 +++++++++++-
>  9 files changed, 1317 insertions(+)
>  create mode 100644 drivers/media/platform/sunxi/sun4i-csi/Kconfig
>  create mode 100644 drivers/media/platform/sunxi/sun4i-csi/Makefile
>  create mode 100644 drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
>  create mode 100644 drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
>  create mode 100644 drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
>  create mode 100644 drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 30bf852e6d6b..b15543b06a16 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1420,6 +1420,14 @@ F:	drivers/pinctrl/sunxi/
>  F:	drivers/soc/sunxi/
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git
>  
> +Allwinner A10 CSI driver
> +M:	Maxime Ripard <maxime.ripard@bootlin.com>
> +L:	linux-media@vger.kernel.org
> +T:	git git://linuxtv.org/media_tree.git
> +S:	Maintained
> +F:	drivers/media/platform/sunxi/sun4i-csi/
> +F:	Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml

The MAINTAINERS entry should be added in the first patch adding new files,
i.e. the DT binding patch.

> +
>  ARM/Amlogic Meson SoC CLOCK FRAMEWORK
>  M:	Neil Armstrong <narmstrong@baylibre.com>
>  M:	Jerome Brunet <jbrunet@baylibre.com>
> diff --git a/drivers/media/platform/sunxi/Kconfig b/drivers/media/platform/sunxi/Kconfig
> index 1b6e89cb78b2..71808e93ac2e 100644
> --- a/drivers/media/platform/sunxi/Kconfig
> +++ b/drivers/media/platform/sunxi/Kconfig
> @@ -1 +1,2 @@
> +source "drivers/media/platform/sunxi/sun4i-csi/Kconfig"
>  source "drivers/media/platform/sunxi/sun6i-csi/Kconfig"
> diff --git a/drivers/media/platform/sunxi/Makefile b/drivers/media/platform/sunxi/Makefile
> index 8d06f98500ee..a05127529006 100644
> --- a/drivers/media/platform/sunxi/Makefile
> +++ b/drivers/media/platform/sunxi/Makefile
> @@ -1 +1,2 @@
> +obj-y		+= sun4i-csi/
>  obj-y		+= sun6i-csi/
> diff --git a/drivers/media/platform/sunxi/sun4i-csi/Kconfig b/drivers/media/platform/sunxi/sun4i-csi/Kconfig
> new file mode 100644
> index 000000000000..e86e29b6a603
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun4i-csi/Kconfig
> @@ -0,0 +1,11 @@
> +config VIDEO_SUN4I_CSI
> +	tristate "Allwinner A10 CMOS Sensor Interface Support"
> +	depends on VIDEO_V4L2 && COMMON_CLK && VIDEO_V4L2_SUBDEV_API && HAS_DMA
> +	depends on ARCH_SUNXI || COMPILE_TEST
> +	select VIDEOBUF2_DMA_CONTIG
> +	select V4L2_FWNODE
> +	help
> +	  This is a V4L2 driver for the Allwinner A10 CSI
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called sun4i_csi.
> diff --git a/drivers/media/platform/sunxi/sun4i-csi/Makefile b/drivers/media/platform/sunxi/sun4i-csi/Makefile
> new file mode 100644
> index 000000000000..7c790a57f5ee
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun4i-csi/Makefile
> @@ -0,0 +1,5 @@
> +sun4i-csi-y += sun4i_csi.o
> +sun4i-csi-y += sun4i_dma.o
> +sun4i-csi-y += sun4i_v4l2.o
> +
> +obj-$(CONFIG_VIDEO_SUN4I_CSI)	+= sun4i-csi.o
> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> new file mode 100644
> index 000000000000..6f7980e28a98
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> @@ -0,0 +1,305 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2016 NextThing Co
> + * Copyright (C) 2016-2019 Bootlin
> + *
> + * Author: Maxime Ripard <maxime.ripard@bootlin.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_graph.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +#include <linux/videodev2.h>
> +
> +#include <media/v4l2-dev.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/v4l2-mediabus.h>
> +
> +#include <media/videobuf2-core.h>
> +#include <media/videobuf2-dma-contig.h>
> +
> +#include "sun4i_csi.h"
> +
> +static const struct media_entity_operations sun4i_csi_video_entity_ops = {
> +	.link_validate = v4l2_subdev_link_validate,
> +};
> +
> +static int sun4i_csi_notify_bound(struct v4l2_async_notifier *notifier,
> +				  struct v4l2_subdev *subdev,
> +				  struct v4l2_async_subdev *asd)
> +{
> +	struct sun4i_csi *csi = container_of(notifier, struct sun4i_csi,
> +					     notifier);
> +
> +	csi->src_subdev = subdev;
> +	csi->src_pad = media_entity_get_fwnode_pad(&subdev->entity,
> +						   subdev->fwnode,
> +						   MEDIA_PAD_FL_SOURCE);
> +	if (csi->src_pad < 0) {
> +		dev_err(csi->dev, "Couldn't find output pad for subdev %s\n",
> +			subdev->name);
> +		return csi->src_pad;
> +	}
> +
> +	dev_dbg(csi->dev, "Bound %s pad: %d\n", subdev->name, csi->src_pad);
> +	return 0;
> +}
> +
> +static int sun4i_csi_notify_complete(struct v4l2_async_notifier *notifier)
> +{
> +	struct sun4i_csi *csi = container_of(notifier, struct sun4i_csi,
> +					     notifier);
> +	struct v4l2_subdev *subdev = &csi->subdev;
> +	struct video_device *vdev = &csi->vdev;
> +	int ret;
> +
> +	ret = v4l2_device_register_subdev(&csi->v4l, subdev);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = sun4i_csi_v4l2_register(csi);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = media_device_register(&csi->mdev);
> +	if (ret)
> +		return ret;
> +
> +	/* Create link from subdev to main device */
> +	ret = media_create_pad_link(&subdev->entity, CSI_SUBDEV_SOURCE,
> +				    &vdev->entity, 0,
> +				    MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
> +	if (ret)
> +		goto err_clean_media;
> +
> +	ret = media_create_pad_link(&csi->src_subdev->entity, csi->src_pad,
> +				    &subdev->entity, CSI_SUBDEV_SINK,
> +				    MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
> +	if (ret)
> +		goto err_clean_media;
> +
> +	ret = v4l2_device_register_subdev_nodes(&csi->v4l);
> +	if (ret < 0)
> +		goto err_clean_media;
> +
> +	return 0;
> +
> +err_clean_media:
> +	media_device_unregister(&csi->mdev);
> +
> +	return ret;
> +}
> +
> +static const struct v4l2_async_notifier_operations sun4i_csi_notify_ops = {
> +	.bound		= sun4i_csi_notify_bound,
> +	.complete	= sun4i_csi_notify_complete,
> +};
> +
> +static int sun4i_csi_async_parse(struct device *dev,
> +				 struct v4l2_fwnode_endpoint *vep,
> +				 struct v4l2_async_subdev *asd)
> +{
> +	struct sun4i_csi *csi = dev_get_drvdata(dev);
> +
> +	if (vep->base.port || vep->base.id)
> +		return -EINVAL;
> +
> +	if (vep->bus_type != V4L2_MBUS_PARALLEL)
> +		return -EINVAL;
> +
> +	csi->bus = vep->bus.parallel;
> +
> +	return 0;
> +}
> +
> +static int sun4i_csi_probe(struct platform_device *pdev)
> +{
> +	struct v4l2_subdev *subdev;
> +	struct video_device *vdev;
> +	struct sun4i_csi *csi;
> +	struct resource *res;
> +	int ret;
> +	int irq;
> +
> +	csi = devm_kzalloc(&pdev->dev, sizeof(*csi), GFP_KERNEL);
> +	if (!csi)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, csi);
> +	csi->dev = &pdev->dev;
> +	subdev = &csi->subdev;
> +	vdev = &csi->vdev;
> +
> +	csi->mdev.dev = csi->dev;
> +	strscpy(csi->mdev.model, "Allwinner Video Capture Device",
> +		sizeof(csi->mdev.model));
> +	csi->mdev.hw_revision = 0;
> +	media_device_init(&csi->mdev);
> +	v4l2_async_notifier_init(&csi->notifier);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	csi->regs = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(csi->regs))
> +		return PTR_ERR(csi->regs);
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0)
> +		return irq;
> +
> +	csi->bus_clk = devm_clk_get(&pdev->dev, "bus");
> +	if (IS_ERR(csi->bus_clk)) {
> +		dev_err(&pdev->dev, "Couldn't get our bus clock\n");
> +		return PTR_ERR(csi->bus_clk);
> +	}
> +
> +	csi->isp_clk = devm_clk_get(&pdev->dev, "isp");
> +	if (IS_ERR(csi->isp_clk)) {
> +		dev_err(&pdev->dev, "Couldn't get our ISP clock\n");
> +		return PTR_ERR(csi->isp_clk);
> +	}
> +
> +	csi->ram_clk = devm_clk_get(&pdev->dev, "ram");
> +	if (IS_ERR(csi->ram_clk)) {
> +		dev_err(&pdev->dev, "Couldn't get our ram clock\n");
> +		return PTR_ERR(csi->ram_clk);
> +	}
> +
> +	csi->rst = devm_reset_control_get(&pdev->dev, NULL);
> +	if (IS_ERR(csi->rst)) {
> +		dev_err(&pdev->dev, "Couldn't get our reset line\n");
> +		return PTR_ERR(csi->rst);
> +	}
> +
> +	/* Initialize subdev */
> +	v4l2_subdev_init(subdev, &sun4i_csi_subdev_ops);
> +	subdev->flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
> +	subdev->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> +	subdev->owner = THIS_MODULE;
> +	snprintf(subdev->name, sizeof(subdev->name), "sun4i-csi-0");
> +	v4l2_set_subdevdata(subdev, csi);
> +
> +	csi->subdev_pads[CSI_SUBDEV_SINK].flags =
> +		MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
> +	csi->subdev_pads[CSI_SUBDEV_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
> +	ret = media_entity_pads_init(&subdev->entity, CSI_SUBDEV_PADS,
> +				     csi->subdev_pads);
> +	if (ret < 0)
> +		return ret;
> +
> +	csi->vdev_pad.flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
> +	vdev->entity.ops = &sun4i_csi_video_entity_ops;
> +	ret = media_entity_pads_init(&vdev->entity, 1, &csi->vdev_pad);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = sun4i_csi_dma_register(csi, irq);
> +	if (ret)
> +		goto err_clean_pad;
> +
> +	csi->v4l.mdev = &csi->mdev;
> +	ret = v4l2_async_notifier_parse_fwnode_endpoints(csi->dev,
> +							 &csi->notifier,
> +							 sizeof(struct v4l2_async_subdev),
> +							 sun4i_csi_async_parse);

Could you instead use fwnode_graph_get_endpoint_by_id() and then parse it
using v4l2_fwnode_endpoint_parse()? There's an example in
drivers/media/pci/intel/ipu3/ipu3-cio2.c .

> +	if (ret)
> +		goto err_unregister_media;
> +	csi->notifier.ops = &sun4i_csi_notify_ops;
> +
> +	ret = v4l2_async_notifier_register(&csi->v4l, &csi->notifier);
> +	if (ret) {
> +		dev_err(csi->dev,
> +			"Couldn't register our v4l2-async notifier\n");
> +		goto err_free_notifier;
> +	}
> +
> +	pm_runtime_enable(&pdev->dev);
> +
> +	return 0;
> +
> +err_free_notifier:
> +	v4l2_async_notifier_cleanup(&csi->notifier);
> +
> +err_unregister_media:
> +	media_device_unregister(&csi->mdev);
> +	sun4i_csi_dma_unregister(csi);
> +
> +err_clean_pad:
> +	media_device_cleanup(&csi->mdev);
> +
> +	return ret;
> +}
> +
> +static int sun4i_csi_remove(struct platform_device *pdev)
> +{
> +	struct sun4i_csi *csi = platform_get_drvdata(pdev);
> +
> +	v4l2_async_notifier_unregister(&csi->notifier);
> +	v4l2_async_notifier_cleanup(&csi->notifier);
> +	media_device_unregister(&csi->mdev);
> +	sun4i_csi_dma_unregister(csi);
> +	media_device_cleanup(&csi->mdev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id sun4i_csi_of_match[] = {
> +	{ .compatible = "allwinner,sun7i-a20-csi0" },
> +	{ /* Sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, sun4i_csi_of_match);
> +
> +static int __maybe_unused sun4i_csi_runtime_resume(struct device *dev)
> +{
> +	struct sun4i_csi *csi = dev_get_drvdata(dev);
> +
> +	reset_control_deassert(csi->rst);
> +	clk_prepare_enable(csi->bus_clk);
> +	clk_prepare_enable(csi->ram_clk);
> +	clk_set_rate(csi->isp_clk, 80000000);
> +	clk_prepare_enable(csi->isp_clk);
> +
> +	writel(1, csi->regs + CSI_EN_REG);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused sun4i_csi_runtime_suspend(struct device *dev)
> +{
> +	struct sun4i_csi *csi = dev_get_drvdata(dev);
> +
> +	clk_disable_unprepare(csi->isp_clk);
> +	clk_disable_unprepare(csi->ram_clk);
> +	clk_disable_unprepare(csi->bus_clk);
> +
> +	reset_control_assert(csi->rst);
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops sun4i_csi_pm_ops = {
> +	SET_RUNTIME_PM_OPS(sun4i_csi_runtime_suspend,
> +			   sun4i_csi_runtime_resume,
> +			   NULL)
> +};
> +
> +static struct platform_driver sun4i_csi_driver = {
> +	.probe	= sun4i_csi_probe,
> +	.remove	= sun4i_csi_remove,
> +	.driver	= {
> +		.name		= "sun4i-csi",
> +		.of_match_table	= sun4i_csi_of_match,
> +		.pm		= &sun4i_csi_pm_ops,
> +	},
> +};
> +module_platform_driver(sun4i_csi_driver);
> +
> +MODULE_DESCRIPTION("Allwinner A10 Camera Sensor Interface driver");
> +MODULE_AUTHOR("Maxime Ripard <mripard@kernel.org>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
> new file mode 100644
> index 000000000000..df23fda46c0e
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
> @@ -0,0 +1,159 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2016 NextThing Co
> + * Copyright (C) 2016-2019 Bootlin
> + *
> + * Author: Maxime Ripard <maxime.ripard@bootlin.com>
> + */
> +
> +#ifndef _SUN4I_CSI_H_
> +#define _SUN4I_CSI_H_
> +
> +#include <media/media-device.h>
> +#include <media/v4l2-async.h>
> +#include <media/v4l2-dev.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/videobuf2-core.h>
> +
> +#define CSI_EN_REG			0x00
> +
> +#define CSI_CFG_REG			0x04
> +#define CSI_CFG_INPUT_FMT(fmt)			((fmt) << 20)
> +#define CSI_CFG_OUTPUT_FMT(fmt)			((fmt) << 16)
> +#define CSI_CFG_YUV_DATA_SEQ(seq)		((seq) << 8)
> +#define CSI_CFG_VSYNC_POL(pol)			((pol) << 2)
> +#define CSI_CFG_HSYNC_POL(pol)			((pol) << 1)
> +#define CSI_CFG_PCLK_POL(pol)			((pol) << 0)
> +
> +#define CSI_CPT_CTRL_REG		0x08
> +#define CSI_CPT_CTRL_VIDEO_START		BIT(1)
> +#define CSI_CPT_CTRL_IMAGE_START		BIT(0)
> +
> +#define CSI_BUF_ADDR_REG(fifo, buf)	(0x10 + (0x8 * (fifo)) + (0x4 * (buf)))
> +
> +#define CSI_BUF_CTRL_REG		0x28
> +#define CSI_BUF_CTRL_DBN			BIT(2)
> +#define CSI_BUF_CTRL_DBS			BIT(1)
> +#define CSI_BUF_CTRL_DBE			BIT(0)
> +
> +#define CSI_INT_EN_REG			0x30
> +#define CSI_INT_FRM_DONE			BIT(1)
> +#define CSI_INT_CPT_DONE			BIT(0)
> +
> +#define CSI_INT_STA_REG			0x34
> +
> +#define CSI_WIN_CTRL_W_REG		0x40
> +#define CSI_WIN_CTRL_W_ACTIVE(w)		((w) << 16)
> +
> +#define CSI_WIN_CTRL_H_REG		0x44
> +#define CSI_WIN_CTRL_H_ACTIVE(h)		((h) << 16)
> +
> +#define CSI_BUF_LEN_REG			0x48
> +
> +#define CSI_MAX_BUFFER		2
> +#define CSI_MAX_HEIGHT		8192U
> +#define CSI_MAX_WIDTH		8192U
> +
> +enum csi_input {
> +	CSI_INPUT_RAW	= 0,
> +	CSI_INPUT_BT656	= 2,
> +	CSI_INPUT_YUV	= 3,
> +};
> +
> +enum csi_output_raw {
> +	CSI_OUTPUT_RAW_PASSTHROUGH = 0,
> +};
> +
> +enum csi_output_yuv {
> +	CSI_OUTPUT_YUV_422_PLANAR	= 0,
> +	CSI_OUTPUT_YUV_420_PLANAR	= 1,
> +	CSI_OUTPUT_YUV_422_UV		= 4,
> +	CSI_OUTPUT_YUV_420_UV		= 5,
> +	CSI_OUTPUT_YUV_422_MACRO	= 8,
> +	CSI_OUTPUT_YUV_420_MACRO	= 9,
> +};
> +
> +enum csi_yuv_data_seq {
> +	CSI_YUV_DATA_SEQ_YUYV	= 0,
> +	CSI_YUV_DATA_SEQ_YVYU	= 1,
> +	CSI_YUV_DATA_SEQ_UYVY	= 2,
> +	CSI_YUV_DATA_SEQ_VYUY	= 3,
> +};
> +
> +enum csi_subdev_pads {
> +	CSI_SUBDEV_SINK,
> +	CSI_SUBDEV_SOURCE,
> +
> +	CSI_SUBDEV_PADS,
> +};
> +
> +extern const struct v4l2_subdev_ops sun4i_csi_subdev_ops;
> +
> +struct sun4i_csi_format {
> +	u32			mbus;
> +	u32			fourcc;
> +	enum csi_input		input;
> +	u32			output;
> +	unsigned int		num_planes;
> +	u8			bpp[3];
> +	unsigned int		hsub;
> +	unsigned int		vsub;
> +};
> +
> +const struct sun4i_csi_format *sun4i_csi_find_format(const u32 *fourcc,
> +						     const u32 *mbus);
> +
> +struct sun4i_csi {
> +	/* Device resources */
> +	struct device			*dev;
> +
> +	void __iomem			*regs;
> +	struct clk			*bus_clk;
> +	struct clk			*isp_clk;
> +	struct clk			*ram_clk;
> +	struct reset_control		*rst;
> +
> +	struct vb2_v4l2_buffer		*current_buf[CSI_MAX_BUFFER];
> +
> +	struct {
> +		size_t			size;
> +		void			*vaddr;
> +		dma_addr_t		paddr;
> +	} scratch;
> +
> +	struct v4l2_fwnode_bus_parallel	bus;
> +
> +	/* Main Device */
> +	struct v4l2_device		v4l;
> +	struct media_device		mdev;
> +	struct video_device		vdev;
> +	struct media_pad		vdev_pad;
> +	struct v4l2_pix_format_mplane	fmt;
> +
> +	/* Local subdev */
> +	struct v4l2_subdev		subdev;
> +	struct media_pad		subdev_pads[CSI_SUBDEV_PADS];
> +	struct v4l2_mbus_framefmt	subdev_fmt;
> +
> +	/* V4L2 Async variables */
> +	struct v4l2_async_notifier	notifier;
> +	struct v4l2_subdev		*src_subdev;
> +	int				src_pad;
> +
> +	/* V4L2 variables */
> +	struct mutex			lock;
> +
> +	/* Videobuf2 */
> +	struct vb2_queue		queue;
> +	struct list_head		buf_list;
> +	spinlock_t			qlock;
> +	unsigned int			sequence;
> +};
> +
> +int sun4i_csi_dma_register(struct sun4i_csi *csi, int irq);
> +void sun4i_csi_dma_unregister(struct sun4i_csi *csi);
> +
> +int sun4i_csi_v4l2_register(struct sun4i_csi *csi);
> +
> +#endif /* _SUN4I_CSI_H_ */
> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
> new file mode 100644
> index 000000000000..06cea6d2ea87
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
> @@ -0,0 +1,444 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2016 NextThing Co
> + * Copyright (C) 2016-2019 Bootlin
> + *
> + * Author: Maxime Ripard <maxime.ripard@bootlin.com>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/interrupt.h>
> +#include <linux/list.h>
> +#include <linux/mutex.h>
> +#include <linux/spinlock.h>
> +#include <media/videobuf2-dma-contig.h>
> +#include <media/videobuf2-v4l2.h>
> +
> +#include "sun4i_csi.h"
> +
> +struct sun4i_csi_buffer {
> +	struct vb2_v4l2_buffer	vb;
> +	struct list_head	list;
> +};
> +
> +static inline struct sun4i_csi_buffer *
> +vb2_v4l2_to_csi_buffer(const struct vb2_v4l2_buffer *p)
> +{
> +	return container_of(p, struct sun4i_csi_buffer, vb);
> +}
> +
> +static inline struct sun4i_csi_buffer *
> +vb2_to_csi_buffer(const struct vb2_buffer *p)
> +{
> +	return vb2_v4l2_to_csi_buffer(to_vb2_v4l2_buffer(p));
> +}
> +
> +static void sun4i_csi_capture_start(struct sun4i_csi *csi)
> +{
> +	writel(CSI_CPT_CTRL_VIDEO_START, csi->regs + CSI_CPT_CTRL_REG);
> +}
> +
> +static void sun4i_csi_capture_stop(struct sun4i_csi *csi)
> +{
> +	writel(0, csi->regs + CSI_CPT_CTRL_REG);
> +}
> +
> +static int sun4i_csi_queue_setup(struct vb2_queue *vq,
> +				 unsigned int *nbuffers,
> +				 unsigned int *nplanes,
> +				 unsigned int sizes[],
> +				 struct device *alloc_devs[])
> +{
> +	struct sun4i_csi *csi = vb2_get_drv_priv(vq);
> +	unsigned int num_planes = csi->fmt.num_planes;
> +	unsigned int i;
> +
> +	if (*nplanes) {
> +                if (*nplanes != num_planes)
> +                        return -EINVAL;
> +
> +                for (i = 0; i < num_planes; i++)
> +                        if (sizes[i] < csi->fmt.plane_fmt[i].sizeimage)
> +                                return -EINVAL;
> +                return 0;
> +        }

Tabs, please.

> +
> +	*nplanes = num_planes;
> +	for (i = 0; i < num_planes; i++)
> +		sizes[i] = csi->fmt.plane_fmt[i].sizeimage;
> +
> +	return 0;
> +};
> +
> +static int sun4i_csi_buffer_prepare(struct vb2_buffer *vb)
> +{
> +	struct sun4i_csi *csi = vb2_get_drv_priv(vb->vb2_queue);
> +	unsigned int i;
> +
> +	for (i = 0; i < csi->fmt.num_planes; i++) {
> +		unsigned long size = csi->fmt.plane_fmt[i].sizeimage;
> +
> +		if (vb2_plane_size(vb, i) < size) {
> +			dev_err(csi->dev, "buffer too small (%lu < %lu)\n",
> +				vb2_plane_size(vb, i), size);
> +			return -EINVAL;
> +		}
> +
> +		vb2_set_plane_payload(vb, i, size);
> +	}
> +
> +	return 0;
> +}
> +
> +static int sun4i_csi_setup_scratch_buffer(struct sun4i_csi *csi, unsigned int slot)
> +{
> +	dma_addr_t addr = csi->scratch.paddr;
> +	unsigned int plane;
> +
> +	dev_dbg(csi->dev,
> +		"No more available buffer, using the scratch buffer\n");
> +
> +	for (plane = 0; plane < csi->fmt.num_planes; plane++) {
> +		writel(addr, csi->regs + CSI_BUF_ADDR_REG(plane, slot));
> +		addr += csi->fmt.plane_fmt[plane].sizeimage;
> +	}
> +
> +	csi->current_buf[slot] = NULL;
> +	return 0;
> +}
> +
> +static int sun4i_csi_buffer_fill_slot(struct sun4i_csi *csi, unsigned int slot)
> +{
> +	struct sun4i_csi_buffer *c_buf;
> +	struct vb2_v4l2_buffer *v_buf;
> +	unsigned int plane;
> +
> +	/*
> +	 * We should never end up in a situation where we overwrite an
> +	 * already filled slot.
> +	 */
> +	if (WARN_ON(csi->current_buf[slot]))
> +		return -EINVAL;
> +
> +	if (list_empty(&csi->buf_list))
> +		return sun4i_csi_setup_scratch_buffer(csi, slot);
> +
> +	c_buf = list_first_entry(&csi->buf_list, struct sun4i_csi_buffer, list);
> +	list_del_init(&c_buf->list);
> +
> +	v_buf = &c_buf->vb;
> +	csi->current_buf[slot] = v_buf;
> +
> +	for (plane = 0; plane < csi->fmt.num_planes; plane++) {
> +		dma_addr_t buf_addr;
> +
> +		buf_addr = vb2_dma_contig_plane_dma_addr(&v_buf->vb2_buf,
> +							 plane);
> +		writel(buf_addr, csi->regs + CSI_BUF_ADDR_REG(plane, slot));
> +	}
> +
> +	return 0;
> +}
> +
> +static int sun4i_csi_buffer_fill_all(struct sun4i_csi *csi)
> +{
> +	unsigned int slot;
> +	int ret;
> +
> +	for (slot = 0; slot < CSI_MAX_BUFFER; slot++) {
> +		ret = sun4i_csi_buffer_fill_slot(csi, slot);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void sun4i_csi_buffer_mark_done(struct sun4i_csi *csi,
> +				       unsigned int slot,
> +				       unsigned int sequence)
> +{
> +	struct vb2_v4l2_buffer *v_buf;
> +
> +	if (!csi->current_buf[slot]) {
> +		dev_dbg(csi->dev, "Scratch buffer was used, ignoring..\n");
> +		return;
> +	}
> +
> +	v_buf = csi->current_buf[slot];
> +	v_buf->field = csi->fmt.field;
> +	v_buf->sequence = sequence;
> +	v_buf->vb2_buf.timestamp = ktime_get_ns();
> +	vb2_buffer_done(&v_buf->vb2_buf, VB2_BUF_STATE_DONE);
> +
> +	csi->current_buf[slot] = NULL;
> +}
> +
> +static int sun4i_csi_buffer_flip(struct sun4i_csi *csi, unsigned int sequence)
> +{
> +	u32 reg = readl(csi->regs + CSI_BUF_CTRL_REG);
> +	unsigned int curr, next;
> +
> +	/* Our next buffer is not the current buffer */
> +	curr = !!(reg & CSI_BUF_CTRL_DBS);
> +	next = !curr;

Hmm. Isn't this the same as:

	next = !(reg & CSI_BUF_CTRL_DBS);

> +
> +	/* Report the previous buffer as done */
> +	sun4i_csi_buffer_mark_done(csi, next, sequence);
> +
> +	/* Put a new buffer in there */
> +	return sun4i_csi_buffer_fill_slot(csi, next);
> +}
> +
> +static void sun4i_csi_buffer_queue(struct vb2_buffer *vb)
> +{
> +	struct sun4i_csi *csi = vb2_get_drv_priv(vb->vb2_queue);
> +	struct sun4i_csi_buffer *buf = vb2_to_csi_buffer(vb);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&csi->qlock, flags);
> +	list_add_tail(&buf->list, &csi->buf_list);
> +	spin_unlock_irqrestore(&csi->qlock, flags);
> +}
> +
> +static void return_all_buffers(struct sun4i_csi *csi,
> +			       enum vb2_buffer_state state)
> +{
> +	struct sun4i_csi_buffer *buf, *node;
> +	unsigned int slot;
> +
> +	list_for_each_entry_safe(buf, node, &csi->buf_list, list) {
> +		vb2_buffer_done(&buf->vb.vb2_buf, state);
> +		list_del(&buf->list);
> +	}
> +
> +	for (slot = 0; slot < CSI_MAX_BUFFER; slot++) {
> +		struct vb2_v4l2_buffer *v_buf = csi->current_buf[slot];
> +
> +		if (!v_buf)
> +			continue;
> +
> +		vb2_buffer_done(&v_buf->vb2_buf, state);
> +		csi->current_buf[slot] = NULL;
> +	}
> +}
> +
> +static int sun4i_csi_start_streaming(struct vb2_queue *vq, unsigned int count)
> +{
> +	struct sun4i_csi *csi = vb2_get_drv_priv(vq);
> +	struct v4l2_fwnode_bus_parallel *bus = &csi->bus;
> +	const struct sun4i_csi_format *csi_fmt;
> +	unsigned long hsync_pol, pclk_pol, vsync_pol;
> +	unsigned long flags;
> +	unsigned int i;
> +	int ret;
> +
> +	csi_fmt = sun4i_csi_find_format(&csi->fmt.pixelformat, NULL);
> +	if (!csi_fmt)
> +		return -EINVAL;
> +
> +	dev_dbg(csi->dev, "Starting capture\n");
> +
> +	csi->sequence = 0;
> +
> +	/*
> +	 * We need a scratch buffer in case where we'll not have any
> +	 * more buffer queued so that we don't error out. One of those
> +	 * cases is when you end up at the last frame to capture, you
> +	 * don't havea any buffer queued any more, and yet it doesn't
> +	 * really matter since you'll never reach the next buffer.
> +	 *
> +	 * Since we support the multi-planar API, we need to have a
> +	 * buffer for each plane. Allocating a single one large enough
> +	 * to hold all the buffers is simpler, so let's go for that.
> +	 */
> +	csi->scratch.size = 0;
> +	for (i = 0; i < csi->fmt.num_planes; i++)
> +		csi->scratch.size += csi->fmt.plane_fmt[i].sizeimage;
> +
> +	csi->scratch.vaddr = dma_alloc_coherent(csi->dev,
> +						csi->scratch.size,
> +						&csi->scratch.paddr,
> +						GFP_KERNEL);
> +	if (!csi->scratch.vaddr) {
> +		dev_err(csi->dev, "Failed to allocate scratch buffer\n");
> +		ret = -ENOMEM;
> +		goto err_clear_dma_queue;
> +	}
> +
> +	ret = media_pipeline_start(&csi->vdev.entity, &csi->vdev.pipe);
> +	if (ret < 0)
> +		goto err_free_scratch_buffer;
> +
> +	spin_lock_irqsave(&csi->qlock, flags);
> +
> +	/* Setup timings */
> +	writel(CSI_WIN_CTRL_W_ACTIVE(csi->fmt.width * 2),
> +	       csi->regs + CSI_WIN_CTRL_W_REG);
> +	writel(CSI_WIN_CTRL_H_ACTIVE(csi->fmt.height),
> +	       csi->regs + CSI_WIN_CTRL_H_REG);
> +
> +	hsync_pol = !!(bus->flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH);
> +	pclk_pol = !!(bus->flags & V4L2_MBUS_DATA_ACTIVE_HIGH);
> +	vsync_pol = !!(bus->flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH);
> +	writel(CSI_CFG_INPUT_FMT(csi_fmt->input) |
> +	       CSI_CFG_OUTPUT_FMT(csi_fmt->output) |
> +	       CSI_CFG_VSYNC_POL(vsync_pol) |
> +	       CSI_CFG_HSYNC_POL(hsync_pol) |
> +	       CSI_CFG_PCLK_POL(pclk_pol),
> +	       csi->regs + CSI_CFG_REG);
> +
> +	/* Setup buffer length */
> +	writel(csi->fmt.plane_fmt[0].bytesperline,
> +	       csi->regs + CSI_BUF_LEN_REG);
> +
> +	/* Prepare our buffers in hardware */
> +	ret = sun4i_csi_buffer_fill_all(csi);
> +	if (ret) {
> +		spin_unlock_irqrestore(&csi->qlock, flags);
> +		goto err_disable_pipeline;
> +	}
> +
> +	/* Enable double buffering */
> +	writel(CSI_BUF_CTRL_DBE, csi->regs + CSI_BUF_CTRL_REG);
> +
> +	/* Clear the pending interrupts */
> +	writel(CSI_INT_FRM_DONE, csi->regs + 0x34);
> +
> +	/* Enable frame done interrupt */
> +	writel(CSI_INT_FRM_DONE, csi->regs + CSI_INT_EN_REG);
> +
> +	sun4i_csi_capture_start(csi);
> +
> +	spin_unlock_irqrestore(&csi->qlock, flags);
> +
> +	ret = v4l2_subdev_call(csi->src_subdev, video, s_stream, 1);
> +	if (ret < 0 && ret != -ENOIOCTLCMD)
> +		goto err_disable_device;
> +
> +	return 0;
> +
> +err_disable_device:
> +	sun4i_csi_capture_stop(csi);
> +
> +err_disable_pipeline:
> +	media_pipeline_stop(&csi->vdev.entity);
> +
> +err_free_scratch_buffer:
> +	dma_free_coherent(csi->dev, csi->scratch.size, csi->scratch.vaddr,
> +			  csi->scratch.paddr);
> +
> +err_clear_dma_queue:
> +	spin_lock_irqsave(&csi->qlock, flags);
> +	return_all_buffers(csi, VB2_BUF_STATE_QUEUED);
> +	spin_unlock_irqrestore(&csi->qlock, flags);
> +
> +	return ret;
> +}
> +
> +static void sun4i_csi_stop_streaming(struct vb2_queue *vq)
> +{
> +	struct sun4i_csi *csi = vb2_get_drv_priv(vq);
> +	unsigned long flags;
> +
> +	dev_dbg(csi->dev, "Stopping capture\n");
> +
> +	v4l2_subdev_call(csi->src_subdev, video, s_stream, 0);
> +	sun4i_csi_capture_stop(csi);
> +
> +	/* Release all active buffers */
> +	spin_lock_irqsave(&csi->qlock, flags);
> +	return_all_buffers(csi, VB2_BUF_STATE_ERROR);
> +	spin_unlock_irqrestore(&csi->qlock, flags);
> +
> +	media_pipeline_stop(&csi->vdev.entity);
> +
> +	dma_free_coherent(csi->dev, csi->scratch.size, csi->scratch.vaddr,
> +			  csi->scratch.paddr);
> +}
> +
> +static const struct vb2_ops sun4i_csi_qops = {
> +	.queue_setup		= sun4i_csi_queue_setup,
> +	.buf_prepare		= sun4i_csi_buffer_prepare,
> +	.buf_queue		= sun4i_csi_buffer_queue,
> +	.start_streaming	= sun4i_csi_start_streaming,
> +	.stop_streaming		= sun4i_csi_stop_streaming,
> +	.wait_prepare		= vb2_ops_wait_prepare,
> +	.wait_finish		= vb2_ops_wait_finish,
> +};
> +
> +static irqreturn_t sun4i_csi_irq(int irq, void *data)
> +{
> +	struct sun4i_csi *csi = data;
> +	u32 reg;
> +
> +	reg = readl(csi->regs + CSI_INT_STA_REG);
> +
> +	/* Acknowledge the interrupts */
> +	writel(reg, csi->regs + CSI_INT_STA_REG);
> +
> +	if (!(reg & CSI_INT_FRM_DONE))
> +		goto out;

I'd just return IRQ_HANDLED, no need for goto.

> +
> +	spin_lock(&csi->qlock);
> +	if (sun4i_csi_buffer_flip(csi, csi->sequence++)) {
> +		dev_warn(csi->dev, "%s: Flip failed\n", __func__);
> +		sun4i_csi_capture_stop(csi);
> +	}
> +	spin_unlock(&csi->qlock);
> +
> +out:
> +	return IRQ_HANDLED;
> +}
> +
> +int sun4i_csi_dma_register(struct sun4i_csi *csi, int irq)
> +{
> +	struct vb2_queue *q = &csi->queue;
> +	int ret;
> +	int i;
> +
> +	ret = v4l2_device_register(csi->dev, &csi->v4l);
> +	if (ret) {
> +		dev_err(csi->dev, "Couldn't register the v4l2 device\n");
> +		return ret;
> +	}
> +
> +	spin_lock_init(&csi->qlock);
> +	mutex_init(&csi->lock);
> +
> +	INIT_LIST_HEAD(&csi->buf_list);
> +	for (i = 0; i < CSI_MAX_BUFFER; i++)
> +		csi->current_buf[i] = NULL;
> +
> +	q->min_buffers_needed = 3;
> +	q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> +	q->io_modes = VB2_MMAP;
> +	q->lock = &csi->lock;
> +	q->drv_priv = csi;
> +	q->buf_struct_size = sizeof(struct sun4i_csi_buffer);
> +	q->ops = &sun4i_csi_qops;
> +	q->mem_ops = &vb2_dma_contig_memops;
> +	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> +	q->dev = csi->dev;
> +
> +	ret = vb2_queue_init(q);
> +	if (ret < 0) {
> +		dev_err(csi->dev, "failed to initialize VB2 queue\n");
> +		return ret;
> +	}
> +
> +	ret = devm_request_irq(csi->dev, irq, sun4i_csi_irq, 0,
> +			       dev_name(csi->dev), csi);
> +	if (ret) {
> +		dev_err(csi->dev, "Couldn't register our interrupt\n");
> +		vb2_queue_release(q);

mutex_destroy(); same above and in cun4i_csi_dma_unregister(). A few labels
for error handling would seem appropriate.

v4l2_device_unregister(), too. I'd actually register the V4L2 device as
last.

> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +void sun4i_csi_dma_unregister(struct sun4i_csi *csi)
> +{
> +	v4l2_device_unregister(&csi->v4l);
> +}
> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c
> new file mode 100644
> index 000000000000..7e5369fab8a2
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c
> @@ -0,0 +1,383 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2016 NextThing Co
> + * Copyright (C) 2016-2019 Bootlin
> + *
> + * Author: Maxime Ripard <maxime.ripard@bootlin.com>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/pm_runtime.h>
> +
> +#include <media/v4l2-ioctl.h>
> +#include <media/v4l2-mc.h>
> +#include <media/videobuf2-v4l2.h>
> +
> +#include "sun4i_csi.h"
> +
> +#define CSI_DEFAULT_WIDTH	640
> +#define CSI_DEFAULT_HEIGHT	480
> +
> +const struct sun4i_csi_format sun4i_csi_formats[] = {
> +	/* YUV422 inputs */
> +	{
> +		.mbus		= MEDIA_BUS_FMT_YUYV8_2X8,
> +		.fourcc		= V4L2_PIX_FMT_YUV420M,
> +		.input		= CSI_INPUT_YUV,
> +		.output		= CSI_OUTPUT_YUV_420_PLANAR,
> +		.num_planes	= 3,
> +		.bpp		= { 8, 8, 8 },
> +		.hsub		= 2,
> +		.vsub		= 2,
> +	},
> +};
> +
> +const struct sun4i_csi_format *sun4i_csi_find_format(const u32 *fourcc,
> +						     const u32 *mbus)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(sun4i_csi_formats); i++) {
> +		if (fourcc && *fourcc != sun4i_csi_formats[i].fourcc)
> +			continue;
> +
> +		if (mbus && *mbus != sun4i_csi_formats[i].mbus)
> +			continue;
> +
> +		return &sun4i_csi_formats[i];
> +	}
> +
> +	return NULL;
> +}
> +
> +static int sun4i_csi_querycap(struct file *file, void *priv,
> +			      struct v4l2_capability *cap)
> +{
> +	struct sun4i_csi *csi = video_drvdata(file);
> +
> +	strscpy(cap->driver, KBUILD_MODNAME, sizeof(cap->driver));
> +	strscpy(cap->card, "sun4i-csi", sizeof(cap->card));
> +	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> +		 dev_name(csi->dev));
> +
> +	return 0;
> +}
> +
> +static int sun4i_csi_enum_input(struct file *file, void *priv,
> +				struct v4l2_input *inp)
> +{
> +	if (inp->index != 0)
> +		return -EINVAL;
> +
> +	inp->type = V4L2_INPUT_TYPE_CAMERA;
> +	strscpy(inp->name, "Camera", sizeof(inp->name));
> +
> +	return 0;
> +}
> +
> +static int sun4i_csi_g_input(struct file *file, void *fh,
> +			     unsigned int *i)
> +{
> +	*i = 0;
> +
> +	return 0;
> +}
> +
> +static int sun4i_csi_s_input(struct file *file, void *fh,
> +			     unsigned int i)
> +{
> +	if (i != 0)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static void _sun4i_csi_try_fmt(struct sun4i_csi *csi,
> +			       struct v4l2_pix_format_mplane *pix)
> +{
> +	const struct sun4i_csi_format *_fmt;
> +	unsigned int height, width;
> +	unsigned int i;
> +
> +	_fmt = sun4i_csi_find_format(&pix->pixelformat, NULL);
> +	if (!_fmt)
> +		_fmt = &sun4i_csi_formats[0];
> +
> +	pix->field = V4L2_FIELD_NONE;
> +	pix->colorspace = V4L2_COLORSPACE_SRGB;
> +	pix->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(pix->colorspace);
> +	pix->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(pix->colorspace);
> +	pix->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true, pix->colorspace,
> +							  pix->ycbcr_enc);
> +
> +	pix->num_planes = _fmt->num_planes;
> +	pix->pixelformat = _fmt->fourcc;
> +
> +	memset(pix->reserved, 0, sizeof(pix->reserved));
> +
> +	/* Align the width and height on the subsampling */
> +	width = ALIGN(pix->width, _fmt->hsub);
> +	height = ALIGN(pix->height, _fmt->vsub);
> +
> +	/* Clamp the width and height to our capabilities */
> +	pix->width = clamp(width, _fmt->hsub, CSI_MAX_WIDTH);
> +	pix->height = clamp(height, _fmt->vsub, CSI_MAX_HEIGHT);
> +
> +	for (i = 0; i < _fmt->num_planes; i++) {
> +		unsigned int hsub = i > 0 ? _fmt->hsub : 1;
> +		unsigned int vsub = i > 0 ? _fmt->vsub : 1;
> +		unsigned int bpl;
> +
> +		bpl = pix->width / hsub * _fmt->bpp[i] / 8;
> +		pix->plane_fmt[i].bytesperline = bpl;
> +		pix->plane_fmt[i].sizeimage = bpl * pix->height / vsub;
> +		memset(pix->plane_fmt[i].reserved, 0,
> +		       sizeof(pix->plane_fmt[i].reserved));
> +	}
> +}
> +
> +static int sun4i_csi_try_fmt_vid_cap(struct file *file, void *priv,
> +				     struct v4l2_format *f)
> +{
> +	struct sun4i_csi *csi = video_drvdata(file);
> +
> +	_sun4i_csi_try_fmt(csi, &f->fmt.pix_mp);

Newline?

> +	return 0;
> +}
> +
> +static int sun4i_csi_s_fmt_vid_cap(struct file *file, void *priv,
> +				   struct v4l2_format *f)
> +{
> +	struct sun4i_csi *csi = video_drvdata(file);
> +
> +	_sun4i_csi_try_fmt(csi, &f->fmt.pix_mp);
> +	csi->fmt = f->fmt.pix_mp;
> +
> +	return 0;
> +}
> +
> +static int sun4i_csi_g_fmt_vid_cap(struct file *file, void *priv,
> +				   struct v4l2_format *f)
> +{
> +	struct sun4i_csi *csi = video_drvdata(file);
> +
> +	f->fmt.pix_mp = csi->fmt;
> +
> +	return 0;
> +}
> +
> +static int sun4i_csi_enum_fmt_vid_cap(struct file *file, void *priv,
> +				      struct v4l2_fmtdesc *f)
> +{
> +	if (f->index >= ARRAY_SIZE(sun4i_csi_formats))
> +		return -EINVAL;
> +
> +	f->pixelformat = sun4i_csi_formats[f->index].fourcc;
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_ioctl_ops sun4i_csi_ioctl_ops = {
> +	.vidioc_querycap		= sun4i_csi_querycap,
> +
> +	.vidioc_enum_fmt_vid_cap	= sun4i_csi_enum_fmt_vid_cap,
> +	.vidioc_g_fmt_vid_cap_mplane	= sun4i_csi_g_fmt_vid_cap,
> +	.vidioc_s_fmt_vid_cap_mplane	= sun4i_csi_s_fmt_vid_cap,
> +	.vidioc_try_fmt_vid_cap_mplane	= sun4i_csi_try_fmt_vid_cap,
> +
> +	.vidioc_enum_input		= sun4i_csi_enum_input,
> +	.vidioc_g_input			= sun4i_csi_g_input,
> +	.vidioc_s_input			= sun4i_csi_s_input,
> +
> +	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
> +	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
> +	.vidioc_querybuf		= vb2_ioctl_querybuf,
> +	.vidioc_qbuf			= vb2_ioctl_qbuf,
> +	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
> +	.vidioc_expbuf			= vb2_ioctl_expbuf,
> +	.vidioc_prepare_buf		= vb2_ioctl_prepare_buf,
> +	.vidioc_streamon		= vb2_ioctl_streamon,
> +	.vidioc_streamoff		= vb2_ioctl_streamoff,
> +};
> +
> +static int sun4i_csi_open(struct file *file)
> +{
> +	struct sun4i_csi *csi = video_drvdata(file);
> +	int ret;
> +
> +	ret = mutex_lock_interruptible(&csi->lock);
> +	if (ret)
> +		return ret;
> +
> +	ret = pm_runtime_get_sync(csi->dev);
> +	if (ret < 0)
> +		goto err_pm_put;
> +
> +	ret = v4l2_pipeline_pm_use(&csi->vdev.entity, 1);
> +	if (ret)
> +		goto err_pm_put;
> +
> +	ret = v4l2_fh_open(file);
> +	if (ret)
> +		goto err_pipeline_pm_put;
> +
> +	mutex_unlock(&csi->lock);
> +
> +	return 0;
> +
> +err_pipeline_pm_put:
> +	v4l2_pipeline_pm_use(&csi->vdev.entity, 0);
> +
> +err_pm_put:
> +	pm_runtime_put(csi->dev);
> +	mutex_unlock(&csi->lock);
> +
> +	return ret;
> +}
> +
> +static int sun4i_csi_release(struct file *file)
> +{
> +	struct sun4i_csi *csi = video_drvdata(file);
> +
> +	mutex_lock(&csi->lock);
> +
> +	v4l2_fh_release(file);
> +	v4l2_pipeline_pm_use(&csi->vdev.entity, 0);
> +	pm_runtime_put(csi->dev);
> +
> +	mutex_unlock(&csi->lock);
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_file_operations sun4i_csi_fops = {
> +	.owner		= THIS_MODULE,
> +	.open		= sun4i_csi_open,
> +	.release	= sun4i_csi_release,
> +	.unlocked_ioctl	= video_ioctl2,
> +	.read		= vb2_fop_read,
> +	.write		= vb2_fop_write,
> +	.poll		= vb2_fop_poll,
> +	.mmap		= vb2_fop_mmap,
> +};
> +
> +static const struct v4l2_mbus_framefmt sun4i_csi_pad_fmt_default = {
> +	.width = CSI_DEFAULT_WIDTH,
> +	.height = CSI_DEFAULT_HEIGHT,
> +	.code = sun4i_csi_formats[0].mbus,
> +	.field = V4L2_FIELD_NONE,
> +	.colorspace = V4L2_COLORSPACE_RAW,
> +	.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT,
> +	.quantization = V4L2_QUANTIZATION_DEFAULT,
> +	.xfer_func = V4L2_XFER_FUNC_DEFAULT,
> +};
> +
> +static int sun4i_csi_subdev_init_cfg(struct v4l2_subdev *subdev,
> +				     struct v4l2_subdev_pad_config *cfg)
> +{
> +	struct v4l2_mbus_framefmt *fmt;
> +
> +	fmt = v4l2_subdev_get_try_format(subdev, cfg, CSI_SUBDEV_SINK);
> +	*fmt = sun4i_csi_pad_fmt_default;
> +
> +	return 0;
> +}
> +
> +static int sun4i_csi_subdev_get_fmt(struct v4l2_subdev *subdev,
> +				    struct v4l2_subdev_pad_config *cfg,
> +				    struct v4l2_subdev_format *fmt)
> +{
> +	struct sun4i_csi *csi = container_of(subdev, struct sun4i_csi, subdev);
> +	struct v4l2_mbus_framefmt *subdev_fmt;
> +
> +	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
> +		subdev_fmt = v4l2_subdev_get_try_format(subdev, cfg, fmt->pad);
> +	else
> +		subdev_fmt = &csi->subdev_fmt;
> +
> +	fmt->format = *subdev_fmt;
> +
> +	return 0;
> +}
> +
> +static int sun4i_csi_subdev_set_fmt(struct v4l2_subdev *subdev,
> +				    struct v4l2_subdev_pad_config *cfg,
> +				    struct v4l2_subdev_format *fmt)
> +{
> +	struct sun4i_csi *csi = container_of(subdev, struct sun4i_csi, subdev);
> +	struct v4l2_mbus_framefmt *subdev_fmt;
> +
> +	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
> +		subdev_fmt = v4l2_subdev_get_try_format(subdev, cfg, fmt->pad);
> +	else
> +		subdev_fmt = &csi->subdev_fmt;
> +
> +	/* We can only set the format on the sink pad */
> +	if (fmt->pad == CSI_SUBDEV_SINK) {
> +		/* It's the sink, only allow changing the frame size */
> +		subdev_fmt->width = fmt->format.width;
> +		subdev_fmt->height = fmt->format.height;
> +		subdev_fmt->code = fmt->format.code;
> +	}
> +
> +	fmt->format = *subdev_fmt;
> +
> +	return 0;
> +}
> +
> +static int sun4i_csi_subdev_enum_mbus_code(struct v4l2_subdev *subdev,
> +					   struct v4l2_subdev_pad_config *cfg,
> +					   struct v4l2_subdev_mbus_code_enum *mbus)

Over 80 characters per line.

> +{
> +	if (mbus->index >= ARRAY_SIZE(sun4i_csi_formats))
> +		return -EINVAL;
> +
> +	mbus->code = sun4i_csi_formats[mbus->index].mbus;
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_subdev_pad_ops sun4i_csi_subdev_pad_ops = {
> +	.link_validate	= v4l2_subdev_link_validate_default,
> +	.init_cfg	= sun4i_csi_subdev_init_cfg,
> +	.get_fmt	= sun4i_csi_subdev_get_fmt,
> +	.set_fmt	= sun4i_csi_subdev_set_fmt,
> +	.enum_mbus_code	= sun4i_csi_subdev_enum_mbus_code,
> +};
> +
> +const struct v4l2_subdev_ops sun4i_csi_subdev_ops = {
> +	.pad = &sun4i_csi_subdev_pad_ops,
> +};
> +
> +int sun4i_csi_v4l2_register(struct sun4i_csi *csi)
> +{
> +	struct video_device *vdev = &csi->vdev;
> +	int ret;
> +
> +	vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE_MPLANE | V4L2_CAP_STREAMING;
> +	vdev->v4l2_dev = &csi->v4l;
> +	vdev->queue = &csi->queue;
> +	strscpy(vdev->name, KBUILD_MODNAME, sizeof(vdev->name));
> +	vdev->release = video_device_release_empty;
> +	vdev->lock = &csi->lock;
> +
> +	/* Set a default format */
> +	csi->fmt.pixelformat = sun4i_csi_formats[0].fourcc,
> +	csi->fmt.width = CSI_DEFAULT_WIDTH;
> +	csi->fmt.height = CSI_DEFAULT_HEIGHT;
> +	_sun4i_csi_try_fmt(csi, &csi->fmt);
> +	csi->subdev_fmt = sun4i_csi_pad_fmt_default;
> +
> +	vdev->fops = &sun4i_csi_fops;
> +	vdev->ioctl_ops = &sun4i_csi_ioctl_ops;
> +	video_set_drvdata(vdev, csi);
> +
> +	ret = video_register_device(&csi->vdev, VFL_TYPE_GRABBER, -1);
> +	if (ret)
> +		return ret;
> +
> +	dev_info(csi->dev, "Device registered as %s\n",
> +		 video_device_node_name(vdev));
> +
> +	return 0;
> +}

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v5 0/3] soc: ti: k3: Allow for exclusive and shared device requests
From: Lokesh Vutla @ 2019-08-20 12:48 UTC (permalink / raw)
  To: Nishanth Menon, Tero Kristo, Santosh Shilimkar, Rob Herring
  Cc: Device Tree Mailing List, Sekhar Nori, Linux ARM Mailing List
In-Reply-To: <20190729122453.32252-1-lokeshvutla@ti.com>



On 29/07/19 5:54 PM, Lokesh Vutla wrote:
> Sysfw provides an option for requesting exclusive access for a
> device using the flags MSG_FLAG_DEVICE_EXCLUSIVE. If this flag is
> not used, the device is meant to be shared across hosts. Once a device
> is requested from a host with this flag set, any request to this
> device from a different host will be nacked by sysfw.
> 
> Current tisci firmware and pm drivers always requests for device with
> exclusive permissions set. But this is not be true for certain devices
> that are expcted to be shared across different host contexts.
> So add support for getting the shared or exclusive permissions from DT
> and request firmware accordingly.

Gentle Ping on this series.

Thanks and regards,
Lokesh

> 
> Changes since v4: https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=148371
> - Split the driver and arch changes into a separate series.
> - Added Reviewed-by from Nishanth M
> - Rebased on top of v5.3-rc2
> 
> Lokesh Vutla (3):
>   firmware: ti_sci: Allow for device shared and exclusive requests
>   dt-bindings: ti_sci_pm_domains: Add support for exclusive and shared
>     access
>   soc: ti: ti_sci_pm_domains: Add support for exclusive and shared
>     access
> 
>  .../bindings/soc/ti/sci-pm-domain.txt         | 11 ++++-
>  MAINTAINERS                                   |  1 +
>  drivers/firmware/ti_sci.c                     | 45 ++++++++++++++++++-
>  drivers/soc/ti/ti_sci_pm_domains.c            | 23 +++++++++-
>  include/dt-bindings/soc/ti,sci_pm_domain.h    |  9 ++++
>  include/linux/soc/ti/ti_sci_protocol.h        |  3 ++
>  6 files changed, 86 insertions(+), 6 deletions(-)
>  create mode 100644 include/dt-bindings/soc/ti,sci_pm_domain.h
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v5 0/2] arm64: dts: ti: k3: Update the power-domain cells
From: Lokesh Vutla @ 2019-08-20 12:48 UTC (permalink / raw)
  To: Nishanth Menon, Tero Kristo, Santosh Shilimkar, Rob Herring
  Cc: Device Tree Mailing List, Sekhar Nori, Linux ARM Mailing List
In-Reply-To: <20190729123023.32702-1-lokeshvutla@ti.com>



On 29/07/19 6:00 PM, Lokesh Vutla wrote:
> Update the power-domains cells on all K3 based devices to reflect
> exclusive and shared permissions in each device.

Gentle Ping on this series.

Thanks and regards,
Lokesh


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 0/6] arm64: dts: ti: k3-j721e: Add gpio nodes
From: Lokesh Vutla @ 2019-08-20 12:49 UTC (permalink / raw)
  To: Tero Kristo, Nishanth Menon, linus.walleij
  Cc: Keerthy, Device Tree Mailing List, Rob Herring,
	Linux ARM Mailing List, linux-gpio
In-Reply-To: <20190809082947.30590-1-lokeshvutla@ti.com>

Tero,

On 09/08/19 1:59 PM, Lokesh Vutla wrote:
> This series adds gpio nodes for J721E SoC and enable gpio keys
> in J72E common process board.
> 
> Tested Boot log: https://pastebin.ubuntu.com/p/P6QqmZYtSC/
> 
> This series depends on Power-domain cells update series:
> https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=15210
Can you merge the patches 2-6?

Thanks and regards,
Lokesh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Applied "ASoC: uniphier: Fix double reset assersion when transitioning to suspend state" to the asoc tree
From: Mark Brown @ 2019-08-20 12:54 UTC (permalink / raw)
  To: Kunihiko Hayashi
  Cc: alsa-devel, Masami Hiramatsu, Liam Girdwood, linux-kernel,
	Jassi Brar, Mark Brown, linux-arm-kernel
In-Reply-To: <1566281764-14059-1-git-send-email-hayashi.kunihiko@socionext.com>

The patch

   ASoC: uniphier: Fix double reset assersion when transitioning to suspend state

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3

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 c372a35550c8d60f673b20210eea58a06d6d38cb Mon Sep 17 00:00:00 2001
From: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
Date: Tue, 20 Aug 2019 15:16:04 +0900
Subject: [PATCH] ASoC: uniphier: Fix double reset assersion when transitioning
 to suspend state

When transitioning to supend state, uniphier_aio_dai_suspend() is called
and asserts reset lines and disables clocks.

However, if there are two or more DAIs, uniphier_aio_dai_suspend() are
called multiple times, and double reset assersion will cause.

This patch defines the counter that has the number of DAIs at first, and
whenever uniphier_aio_dai_suspend() are called, it decrements the
counter. And only if the counter is zero, it asserts reset lines and
disables clocks.

In the same way, uniphier_aio_dai_resume() are called, it increments the
counter after deasserting reset lines and enabling clocks.

Fixes: 139a34200233 ("ASoC: uniphier: add support for UniPhier AIO CPU DAI driver")
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
Link: https://lore.kernel.org/r/1566281764-14059-1-git-send-email-hayashi.kunihiko@socionext.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/uniphier/aio-cpu.c | 31 +++++++++++++++++++++----------
 sound/soc/uniphier/aio.h     |  1 +
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/sound/soc/uniphier/aio-cpu.c b/sound/soc/uniphier/aio-cpu.c
index ee90e6c3937c..2ae582a99b63 100644
--- a/sound/soc/uniphier/aio-cpu.c
+++ b/sound/soc/uniphier/aio-cpu.c
@@ -424,8 +424,11 @@ int uniphier_aio_dai_suspend(struct snd_soc_dai *dai)
 {
 	struct uniphier_aio *aio = uniphier_priv(dai);
 
-	reset_control_assert(aio->chip->rst);
-	clk_disable_unprepare(aio->chip->clk);
+	aio->chip->num_wup_aios--;
+	if (!aio->chip->num_wup_aios) {
+		reset_control_assert(aio->chip->rst);
+		clk_disable_unprepare(aio->chip->clk);
+	}
 
 	return 0;
 }
@@ -439,13 +442,15 @@ int uniphier_aio_dai_resume(struct snd_soc_dai *dai)
 	if (!aio->chip->active)
 		return 0;
 
-	ret = clk_prepare_enable(aio->chip->clk);
-	if (ret)
-		return ret;
+	if (!aio->chip->num_wup_aios) {
+		ret = clk_prepare_enable(aio->chip->clk);
+		if (ret)
+			return ret;
 
-	ret = reset_control_deassert(aio->chip->rst);
-	if (ret)
-		goto err_out_clock;
+		ret = reset_control_deassert(aio->chip->rst);
+		if (ret)
+			goto err_out_clock;
+	}
 
 	aio_iecout_set_enable(aio->chip, true);
 	aio_chip_init(aio->chip);
@@ -458,7 +463,7 @@ int uniphier_aio_dai_resume(struct snd_soc_dai *dai)
 
 		ret = aio_init(sub);
 		if (ret)
-			goto err_out_clock;
+			goto err_out_reset;
 
 		if (!sub->setting)
 			continue;
@@ -466,11 +471,16 @@ int uniphier_aio_dai_resume(struct snd_soc_dai *dai)
 		aio_port_reset(sub);
 		aio_src_reset(sub);
 	}
+	aio->chip->num_wup_aios++;
 
 	return 0;
 
+err_out_reset:
+	if (!aio->chip->num_wup_aios)
+		reset_control_assert(aio->chip->rst);
 err_out_clock:
-	clk_disable_unprepare(aio->chip->clk);
+	if (!aio->chip->num_wup_aios)
+		clk_disable_unprepare(aio->chip->clk);
 
 	return ret;
 }
@@ -619,6 +629,7 @@ int uniphier_aio_probe(struct platform_device *pdev)
 		return PTR_ERR(chip->rst);
 
 	chip->num_aios = chip->chip_spec->num_dais;
+	chip->num_wup_aios = chip->num_aios;
 	chip->aios = devm_kcalloc(dev,
 				  chip->num_aios, sizeof(struct uniphier_aio),
 				  GFP_KERNEL);
diff --git a/sound/soc/uniphier/aio.h b/sound/soc/uniphier/aio.h
index ca6ccbae0ee8..a7ff7e556429 100644
--- a/sound/soc/uniphier/aio.h
+++ b/sound/soc/uniphier/aio.h
@@ -285,6 +285,7 @@ struct uniphier_aio_chip {
 
 	struct uniphier_aio *aios;
 	int num_aios;
+	int num_wup_aios;
 	struct uniphier_aio_pll *plls;
 	int num_plls;
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Applied "spi: zynq-qspi: Fix missing spi_unregister_controller when unload module" to the spi tree
From: Mark Brown @ 2019-08-20 12:54 UTC (permalink / raw)
  To: Axel Lin; +Cc: Mark Brown, Michal Simek, linux-arm-kernel, linux-spi
In-Reply-To: <20190818095113.2397-1-axel.lin@ingics.com>

The patch

   spi: zynq-qspi: Fix missing spi_unregister_controller when unload module

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-5.3

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 8eb2fd00f65a96143ed1535bdbf4ca4e129d30d1 Mon Sep 17 00:00:00 2001
From: Axel Lin <axel.lin@ingics.com>
Date: Sun, 18 Aug 2019 17:51:13 +0800
Subject: [PATCH] spi: zynq-qspi: Fix missing spi_unregister_controller when
 unload module

Use devm_spi_register_controller to fix missing spi_unregister_controller
when unload module.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
Acked-by: Michal Simek <michal.simek@xilinx.com>
Link: https://lore.kernel.org/r/20190818095113.2397-1-axel.lin@ingics.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-zynq-qspi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-zynq-qspi.c b/drivers/spi/spi-zynq-qspi.c
index c6bee67decb5..d812a215ae5c 100644
--- a/drivers/spi/spi-zynq-qspi.c
+++ b/drivers/spi/spi-zynq-qspi.c
@@ -695,7 +695,7 @@ static int zynq_qspi_probe(struct platform_device *pdev)
 	ctlr->setup = zynq_qspi_setup_op;
 	ctlr->max_speed_hz = clk_get_rate(xqspi->refclk) / 2;
 	ctlr->dev.of_node = np;
-	ret = spi_register_controller(ctlr);
+	ret = devm_spi_register_controller(&pdev->dev, ctlr);
 	if (ret) {
 		dev_err(&pdev->dev, "spi_register_master failed\n");
 		goto clk_dis_all;
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v2 17/17] iommu/arm-smmu: Add context init implementation hook
From: Robin Murphy @ 2019-08-20 13:00 UTC (permalink / raw)
  To: Vivek Gautam, will
  Cc: robdclark, gregory.clement, bjorn.andersson, iommu, jcrouse, joro,
	linux-arm-kernel
In-Reply-To: <8306f3f1-925c-0b02-8103-9d4a510005b2@codeaurora.org>

On 20/08/2019 11:15, Vivek Gautam wrote:
[...]
> Hi Robin,
> 
> Sorry for responding late to this series. I have couple of doubts here 
> that I wanted to discuss.
> 
> Are we standardizing these implementation specific ops? Each vendor 
> implementations will have something peculiar to take care. Things are 
> good right now with 'reset', 'cfg_probe', and 'init_context' hooks.
> But, on top of vendor implementation details, there can be SoC specific 
> errata changes that need to be added.

The idea behind the impl hooks is to try to have them in logical places 
which should be suitable for multiple different workarounds (e.g. 
init_context is arranged to allow replacing smmu_domain->tlb_ops) - I 
want to avoid proliferating dozens of them that end up each being 
specific to individual workarounds, but that's not to say that the 
design we're starting with here is by any means complete or final. We're 
almost certainly going to evolve more hooks (and possibly adjust the 
current ones) in future.

> Moreover, adding implementation data based on __model__ may not suffice 
> for long. Do you suggest adding any other data variable in the 
> ARM_SMMU_MATCH_DATA?

As commented in the code, the setup for the existing quirks works out to 
be deceptively simple, and it can and will change. Ultimately I'm fully 
expecting to end up with complex logic hanging off arm_smmu_impl_init() 
to detect the integration details and compose sets of impl hooks in 
various ways as appropriate - I doubt that it's going to be practical or 
even possible to have static data for *every* possibility. The 
fundamental shape of the code is based on "model" quirks being more 
general than "integration" quirks, such that the latter should be in a 
position to dynamically inherit (or statically replace, in simple cases) 
the hooks of the former.

> To show SoC specific needs, I have the change attached in this email to 
> take care of the SDM845 'wait-for-safe' sequence.
> Please take a look.

Other than that introducing QCOM_SMMU500 seems to be redundant, and 
whether it also needs ACPI-based detection, that certainly seems fairly 
reasonable for a simple isolated workaround. However, is the firmware 
call really a one-off probe-time thing? If the firmware does take care 
of preserving the wait-for-safe state across 
suspend/resume/hibernate/etc. then fair enough, but I would have assumed 
that the reset hook would be the more likely place to put it.

Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 02/10] clk: sunxi-ng: Mark AR100 clocks as critical
From: Samuel Holland @ 2019-08-20 13:02 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, linux-sunxi, Stephen Boyd,
	Michael Turquette, Jassi Brar, linux-kernel, Chen-Yu Tsai,
	Rob Herring, Corentin Labbe, linux-clk, linux-arm-kernel
In-Reply-To: <20190820071142.2bgfsnt75xfeyusp@flea>

On 8/20/19 2:11 AM, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Aug 19, 2019 at 10:23:03PM -0500, Samuel Holland wrote:
>> On sun8i, sun9i, and sun50i SoCs, system suspend/resume support requires
>> firmware running on the AR100 coprocessor (the "SCP"). Such firmware can
>> provide additional features, such as thermal monitoring and poweron/off
>> support for boards without a PMIC.
>>
>> Since the AR100 may be running critical firmware, even if Linux does not
>> know about it or directly interact with it (all requests may go through
>> an intermediary interface such as PSCI), Linux must not turn off its
>> clock.

This paragraph here is the key. The firmware won't necessarily have a device
tree node, and in the current design it will not, since Linux never communicates
with it directly. All communication goes through ATF via PSCI.

>> At this time, such power management firmware only exists for the A64 and
>> H5 SoCs.  However, it makes sense to take care of all CCU drivers now
>> for consistency, and to ease the transition in the future once firmware
>> is ported to the other SoCs.
>>
>> Leaving the clock running is safe even if no firmware is present, since
>> the AR100 stays in reset by default. In most cases, the AR100 clock is
>> kept enabled by Linux anyway, since it is the parent of all APB0 bus
>> peripherals. This change only prevents Linux from turning off the AR100
>> clock in the rare case that no peripherals are in use.
>>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
> 
> So I'm not really sure where you want to go with this.
> 
> That clock is only useful where you're having a firmware running on
> the AR100, and that firmware would have a device tree node of its own,
> where we could list the clocks needed for the firmware to keep
> running, if it ever runs. If the driver has not been compiled in /
> loaded, then we don't care either.

See above. I don't expect that the firmware would have a device tree node,
because the firmware doesn't need any Linux drivers.

> But more fundamentally, if we're going to use SCPI, then those clocks
> will not be handled by that driver anyway, but by the firmware, right?

In the future, we might use SCPI clocks/sensors/regulators/etc. from Linux, but
that's not the plan at the moment. Given that it's already been two years since
I started this project, I'm trying to limit its scope so I can get at least some
part merged. The first step is to integrate a firmware that provides
suspend/resume functionality only. That firmware does implement SCPI, and if the
top-level Linux SCPI driver worked with multiple mailbox channels, it could
query the firmware's version and fetures. But all of the SCPI commands used for
suspend/resume must go through ATF via PSCI.

> So I'm not really sure that we should do it statically this way, and
> that we should do it at all.

Do you have a better way to model "firmware uses this clock behind the scenes,
so Linux please don't touch it"? It's unfortunate that we have Linux and
firmware fighting over the R_CCU, but since we didn't have firmware (e.g. SCPI
clocks) in the beginning, it's where we are today.

The AR100 clock doesn't actually have a gate, and it generally has dependencies
like R_INTC in use. So as I mentioned in the commit message, the clock will
normally be on anyway. The goal was to model the fact that there are users of
this clock that Linux doesn't/can't know about.

> Maxime

Thanks,
Samuel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 03/10] dt-bindings: mailbox: Add a sunxi message box binding
From: Samuel Holland @ 2019-08-20 13:04 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, linux-sunxi, Rob Herring, Stephen Boyd,
	Michael Turquette, Jassi Brar, linux-kernel, Chen-Yu Tsai,
	Rob Herring, Corentin Labbe, linux-clk, linux-arm-kernel
In-Reply-To: <20190820071456.if5lyb4t3em77svl@flea>

On 8/20/19 2:14 AM, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Aug 19, 2019 at 10:23:04PM -0500, Samuel Holland wrote:
>> This mailbox hardware is present in Allwinner sun8i, sun9i, and sun50i
>> SoCs. Add a device tree binding for it.
>>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>> ---
>>  .../mailbox/allwinner,sunxi-msgbox.yaml       | 79 +++++++++++++++++++
>>  1 file changed, 79 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mailbox/allwinner,sunxi-msgbox.yaml
> 
> So we merged a bunch of schemas already, with the convention that the
> name was the first compatible to use that binding.
> 
> That would be allwinner,sun6i-a31-msgbox.yaml in that case

Okay, I'll rename the binding and driver (and Kconfig symbol?).

Thanks,
Samuel


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 04/10] mailbox: sunxi-msgbox: Add a new mailbox driver
From: Samuel Holland @ 2019-08-20 13:07 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
	Vasily Khoruzhick, devicetree, linux-kernel, linux-sunxi,
	linux-clk, linux-arm-kernel
In-Reply-To: <20190820111825.2w55fleehrnon27u@core.my.home>

On 8/20/19 6:18 AM, Ondřej Jirman wrote:
> Hi Samuel,
> 
> On Mon, Aug 19, 2019 at 10:23:05PM -0500, Samuel Holland wrote:
>> Allwinner sun8i, sun9i, and sun50i SoCs contain a hardware message box
>> used for communication between the ARM CPUs and the ARISC management
>> coprocessor. The hardware contains 8 unidirectional 4-message FIFOs.
>>
>> Add a driver for it, so it can be used for SCPI or other communication
>> protocols.
>>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>> ---
>>  drivers/mailbox/Kconfig        |  10 +
>>  drivers/mailbox/Makefile       |   2 +
>>  drivers/mailbox/sunxi-msgbox.c | 323 +++++++++++++++++++++++++++++++++
>>  3 files changed, 335 insertions(+)
>>  create mode 100644 drivers/mailbox/sunxi-msgbox.c
>>
>> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
>> index ab4eb750bbdd..57d12936175e 100644
>> --- a/drivers/mailbox/Kconfig
>> +++ b/drivers/mailbox/Kconfig
>> @@ -227,4 +227,14 @@ config ZYNQMP_IPI_MBOX
>>  	  message to the IPI buffer and will access the IPI control
>>  	  registers to kick the other processor or enquire status.
>>  
>> +config SUNXI_MSGBOX
>> +	tristate "Allwinner sunxi Message Box"
>> +	depends on ARCH_SUNXI || COMPILE_TEST
>> +	default ARCH_SUNXI
>> +	help
>> +	  Mailbox implementation for the hardware message box present in
>> +	  Allwinner sun8i, sun9i, and sun50i SoCs. The hardware message box is
>> +	  used for communication between the application CPUs and the power
>> +	  management coprocessor.
>> +
>>  endif
>> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
>> index c22fad6f696b..bec2d50b0976 100644
>> --- a/drivers/mailbox/Makefile
>> +++ b/drivers/mailbox/Makefile
>> @@ -48,3 +48,5 @@ obj-$(CONFIG_STM32_IPCC) 	+= stm32-ipcc.o
>>  obj-$(CONFIG_MTK_CMDQ_MBOX)	+= mtk-cmdq-mailbox.o
>>  
>>  obj-$(CONFIG_ZYNQMP_IPI_MBOX)	+= zynqmp-ipi-mailbox.o
>> +
>> +obj-$(CONFIG_SUNXI_MSGBOX)	+= sunxi-msgbox.o
>> diff --git a/drivers/mailbox/sunxi-msgbox.c b/drivers/mailbox/sunxi-msgbox.c
>> new file mode 100644
>> index 000000000000..29a5101a5390
>> --- /dev/null
>> +++ b/drivers/mailbox/sunxi-msgbox.c
>> @@ -0,0 +1,323 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +//
>> +// Copyright (c) 2017-2019 Samuel Holland <samuel@sholland.org>
>> +
>> +#include <linux/bitops.h>
>> +#include <linux/clk.h>
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/mailbox_controller.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/reset.h>
>> +#include <linux/spinlock.h>
>> +
>> +#define NUM_CHANS		8
>> +
>> +#define CTRL_REG(n)		(0x0000 + 0x4 * ((n) / 4))
>> +#define CTRL_RX(n)		BIT(0 + 8 * ((n) % 4))
>> +#define CTRL_TX(n)		BIT(4 + 8 * ((n) % 4))
>> +
>> +#define REMOTE_IRQ_EN_REG	0x0040
>> +#define REMOTE_IRQ_STAT_REG	0x0050
>> +#define LOCAL_IRQ_EN_REG	0x0060
>> +#define LOCAL_IRQ_STAT_REG	0x0070
>> +
>> +#define RX_IRQ(n)		BIT(0 + 2 * (n))
>> +#define RX_IRQ_MASK		0x5555
>> +#define TX_IRQ(n)		BIT(1 + 2 * (n))
>> +#define TX_IRQ_MASK		0xaaaa
>> +
>> +#define FIFO_STAT_REG(n)	(0x0100 + 0x4 * (n))
>> +#define FIFO_STAT_MASK		GENMASK(0, 0)
>> +
>> +#define MSG_STAT_REG(n)		(0x0140 + 0x4 * (n))
>> +#define MSG_STAT_MASK		GENMASK(2, 0)
>> +
>> +#define MSG_DATA_REG(n)		(0x0180 + 0x4 * (n))
>> +
>> +#define mbox_dbg(mbox, ...)	dev_dbg((mbox)->controller.dev, __VA_ARGS__)
>> +
>> +struct sunxi_msgbox {
>> +	struct mbox_controller controller;
>> +	struct clk *clk;
>> +	spinlock_t lock;
>> +	void __iomem *regs;
>> +};
>> +
>> +static bool sunxi_msgbox_last_tx_done(struct mbox_chan *chan);
>> +static bool sunxi_msgbox_peek_data(struct mbox_chan *chan);
>> +
>> +static inline int channel_number(struct mbox_chan *chan)
>> +{
>> +	return chan - chan->mbox->chans;
>> +}
>> +
>> +static inline struct sunxi_msgbox *channel_to_msgbox(struct mbox_chan *chan)
>> +{
>> +	return chan->con_priv;
>> +}
>> +
>> +static irqreturn_t sunxi_msgbox_irq(int irq, void *dev_id)
>> +{
>> +	struct sunxi_msgbox *mbox = dev_id;
>> +	uint32_t status;
>> +	int n;
>> +
>> +	/* Only examine channels that are currently enabled. */
>> +	status = readl(mbox->regs + LOCAL_IRQ_EN_REG) &
>> +		 readl(mbox->regs + LOCAL_IRQ_STAT_REG);
>> +
>> +	if (!(status & RX_IRQ_MASK))
>> +		return IRQ_NONE;
>> +
>> +	for (n = 0; n < NUM_CHANS; ++n) {
>> +		struct mbox_chan *chan = &mbox->controller.chans[n];
>> +
>> +		if (!(status & RX_IRQ(n)))
>> +			continue;
>> +
>> +		while (sunxi_msgbox_peek_data(chan)) {
>> +			uint32_t msg = readl(mbox->regs + MSG_DATA_REG(n));
>> +
>> +			mbox_dbg(mbox, "Channel %d received 0x%08x\n", n, msg);
>> +			mbox_chan_received_data(chan, &msg);
>> +		}
>> +
>> +		/* The IRQ can be cleared only once the FIFO is empty. */
>> +		writel(RX_IRQ(n), mbox->regs + LOCAL_IRQ_STAT_REG);
>> +	}
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int sunxi_msgbox_send_data(struct mbox_chan *chan, void *data)
>> +{
>> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
>> +	int n = channel_number(chan);
>> +	uint32_t msg = *(uint32_t *)data;
>> +
>> +	/* Using a channel backwards gets the hardware into a bad state. */
>> +	if (WARN_ON_ONCE(!(readl(mbox->regs + CTRL_REG(n)) & CTRL_TX(n))))
>> +		return 0;
>> +
>> +	/* We cannot post a new message if the FIFO is full. */
>> +	if (readl(mbox->regs + FIFO_STAT_REG(n)) & FIFO_STAT_MASK) {
>> +		mbox_dbg(mbox, "Channel %d busy sending 0x%08x\n", n, msg);
>> +		return -EBUSY;
>> +	}
>> +
>> +	writel(msg, mbox->regs + MSG_DATA_REG(n));
>> +	mbox_dbg(mbox, "Channel %d sent 0x%08x\n", n, msg);
>> +
>> +	return 0;
>> +}
>> +
>> +static int sunxi_msgbox_startup(struct mbox_chan *chan)
>> +{
>> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
>> +	int n = channel_number(chan);
>> +
>> +	/* The coprocessor is responsible for setting channel directions. */
>> +	if (readl(mbox->regs + CTRL_REG(n)) & CTRL_RX(n)) {
>> +		/* Flush the receive FIFO. */
>> +		while (sunxi_msgbox_peek_data(chan))
>> +			readl(mbox->regs + MSG_DATA_REG(n));
>> +		writel(RX_IRQ(n), mbox->regs + LOCAL_IRQ_STAT_REG);
>> +
>> +		/* Enable the receive IRQ. */
>> +		spin_lock(&mbox->lock);
>> +		writel(readl(mbox->regs + LOCAL_IRQ_EN_REG) | RX_IRQ(n),
>> +		       mbox->regs + LOCAL_IRQ_EN_REG);
>> +		spin_unlock(&mbox->lock);
>> +	}
>> +
>> +	mbox_dbg(mbox, "Channel %d startup complete\n", n);
>> +
>> +	return 0;
>> +}
>> +
>> +static void sunxi_msgbox_shutdown(struct mbox_chan *chan)
>> +{
>> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
>> +	int n = channel_number(chan);
>> +
>> +	if (readl(mbox->regs + CTRL_REG(n)) & CTRL_RX(n)) {
>> +		/* Disable the receive IRQ. */
>> +		spin_lock(&mbox->lock);
>> +		writel(readl(mbox->regs + LOCAL_IRQ_EN_REG) & ~RX_IRQ(n),
>> +		       mbox->regs + LOCAL_IRQ_EN_REG);
>> +		spin_unlock(&mbox->lock);
>> +
>> +		/* Attempt to flush the FIFO until the IRQ is cleared. */
>> +		do {
>> +			while (sunxi_msgbox_peek_data(chan))
>> +				readl(mbox->regs + MSG_DATA_REG(n));
>> +			writel(RX_IRQ(n), mbox->regs + LOCAL_IRQ_STAT_REG);
>> +		} while (readl(mbox->regs + LOCAL_IRQ_STAT_REG) & RX_IRQ(n));
>> +	}
>> +
>> +	mbox_dbg(mbox, "Channel %d shutdown complete\n", n);
>> +}
>> +
>> +static bool sunxi_msgbox_last_tx_done(struct mbox_chan *chan)
>> +{
>> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
>> +	int n = channel_number(chan);
>> +
>> +	/*
>> +	 * The hardware allows snooping on the remote user's IRQ statuses.
>> +	 * We consider a message to be acknowledged only once the receive IRQ
>> +	 * for that channel is cleared. Since the receive IRQ for a channel
>> +	 * cannot be cleared until the FIFO for that channel is empty, this
>> +	 * ensures that the message has actually been read. It also gives the
>> +	 * recipient an opportunity to perform minimal processing before
>> +	 * acknowledging the message.
>> +	 */
>> +	return !(readl(mbox->regs + REMOTE_IRQ_STAT_REG) & RX_IRQ(n));
>> +}
>> +
>> +static bool sunxi_msgbox_peek_data(struct mbox_chan *chan)
>> +{
>> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
>> +	int n = channel_number(chan);
>> +
>> +	return readl(mbox->regs + MSG_STAT_REG(n)) & MSG_STAT_MASK;
>> +}
>> +
>> +static const struct mbox_chan_ops sunxi_msgbox_chan_ops = {
>> +	.send_data    = sunxi_msgbox_send_data,
>> +	.startup      = sunxi_msgbox_startup,
>> +	.shutdown     = sunxi_msgbox_shutdown,
>> +	.last_tx_done = sunxi_msgbox_last_tx_done,
>> +	.peek_data    = sunxi_msgbox_peek_data,
>> +};
>> +
>> +static int sunxi_msgbox_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct mbox_chan *chans;
>> +	struct reset_control *reset;
>> +	struct resource *res;
>> +	struct sunxi_msgbox *mbox;
>> +	int i, ret;
>> +
>> +	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
>> +	if (!mbox)
>> +		return -ENOMEM;
>> +
>> +	chans = devm_kcalloc(dev, NUM_CHANS, sizeof(*chans), GFP_KERNEL);
>> +	if (!chans)
>> +		return -ENOMEM;
>> +
>> +	for (i = 0; i < NUM_CHANS; ++i)
>> +		chans[i].con_priv = mbox;
>> +
>> +	mbox->clk = devm_clk_get(dev, NULL);
>> +	if (IS_ERR(mbox->clk)) {
>> +		ret = PTR_ERR(mbox->clk);
>> +		dev_err(dev, "Failed to get clock: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = clk_prepare_enable(mbox->clk);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to enable clock: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	reset = devm_reset_control_get(dev, NULL);
>> +	if (IS_ERR(reset)) {
>> +		ret = PTR_ERR(reset);
>> +		dev_err(dev, "Failed to get reset control: %d\n", ret);
>> +		goto err_disable_unprepare;
>> +	}
>> +
>> +	ret = reset_control_deassert(reset);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to deassert reset: %d\n", ret);
>> +		goto err_disable_unprepare;
>> +	}
> 
> You need to assert the reset again from now on, in error paths. devm
> will not do that for you.

I know, and that's intentional. This same message box device is used for ATF to
communicate with SCP firmware (on a different channel). This could be happening
on a different core while Linux is running. So Linux is not allowed to deassert
the reset. clk_disable_unprepare() is only okay because the clock is marked as
critical.

>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	if (!res) {
>> +		ret = -ENODEV;
>> +		goto err_disable_unprepare;
>> +	}
>> +
>> +	mbox->regs = devm_ioremap_resource(&pdev->dev, res);
>> +	if (IS_ERR(mbox->regs)) {
>> +		ret = PTR_ERR(mbox->regs);
>> +		dev_err(dev, "Failed to map MMIO resource: %d\n", ret);
>> +		goto err_disable_unprepare;
>> +	}
>> +
>> +	/* Disable all IRQs for this end of the msgbox. */
>> +	writel(0, mbox->regs + LOCAL_IRQ_EN_REG);
>> +
>> +	ret = devm_request_irq(dev, irq_of_parse_and_map(dev->of_node, 0),
>> +			       sunxi_msgbox_irq, 0, dev_name(dev), mbox);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to register IRQ handler: %d\n", ret);
>> +		goto err_disable_unprepare;
>> +	}
>> +
>> +	mbox->controller.dev           = dev;
>> +	mbox->controller.ops           = &sunxi_msgbox_chan_ops;
>> +	mbox->controller.chans         = chans;
>> +	mbox->controller.num_chans     = NUM_CHANS;
>> +	mbox->controller.txdone_irq    = false;
>> +	mbox->controller.txdone_poll   = true;
>> +	mbox->controller.txpoll_period = 5;
>> +
>> +	spin_lock_init(&mbox->lock);
>> +	platform_set_drvdata(pdev, mbox);
>> +
>> +	ret = mbox_controller_register(&mbox->controller);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to register controller: %d\n", ret);
>> +		goto err_disable_unprepare;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_disable_unprepare:
>> +	clk_disable_unprepare(mbox->clk);
>> +
>> +	return ret;
>> +}
>> +
>> +static int sunxi_msgbox_remove(struct platform_device *pdev)
>> +{
>> +	struct sunxi_msgbox *mbox = platform_get_drvdata(pdev);
>> +
>> +	mbox_controller_unregister(&mbox->controller);
>> +	clk_disable_unprepare(mbox->clk);
> 
> Also, assert the reset here.

Same comment as above. This is intentional.

Thanks,
Samuel

>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id sunxi_msgbox_of_match[] = {
>> +	{ .compatible = "allwinner,sun6i-a31-msgbox", },
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, sunxi_msgbox_of_match);
>> +
>> +static struct platform_driver sunxi_msgbox_driver = {
>> +	.driver = {
>> +		.name = "sunxi-msgbox",
>> +		.of_match_table = sunxi_msgbox_of_match,
>> +	},
>> +	.probe  = sunxi_msgbox_probe,
>> +	.remove = sunxi_msgbox_remove,
>> +};
>> +module_platform_driver(sunxi_msgbox_driver);
>> +
>> +MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>");
>> +MODULE_DESCRIPTION("Allwinner sunxi Message Box");
>> +MODULE_LICENSE("GPL v2");
>> -- 
>> 2.21.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC 00/11] arm64: Add support for Amlogic SM1 SoC Family
From: Neil Armstrong @ 2019-08-20 13:16 UTC (permalink / raw)
  To: jbrunet, khilman; +Cc: linux-amlogic, linux-kernel, linux-arm-kernel
In-Reply-To: <20190701104705.18271-1-narmstrong@baylibre.com>

He Kevin, Martin,

On 01/07/2019 12:46, Neil Armstrong wrote:
> The new Amlogic SM1 SoC Family is a derivative of the Amlogic G12A
> SoC Family, with the following changes :
> - Cortex-A55 cores instead of A53
> - more power domains, including USB & PCIe
> - a neural network co-processor (NNA)
> - a CSI input and image processor
> - some changes in the audio complex, thus not yet enabled
> - new clocks, for NNA, CSI and a clock tree for each CPU Core
> 
> This serie does not add support for NNA, CSI or DVFS, it only
> aligns with the current G12A Support.
> 
> With thie serie, the SEI610 Board has supported :
> - Default-boot CPU frequency
> - 4k60 HDMI without audio
> - USB3 & USB-C OTG
> - Ethernet
> - LEDs
> - IR
> - GPIO Buttons
> - eMMC
> - SDCard
> - SDIO WiFi
> - UART Bluetooth
> 
> Audio (HDMI, Embedded HP, MIcs), IR Output, & RGB Led would be
> supported in following patchsets.

Following the comments in the power domain patches, I'll respin in 2 distinct
patches :
- initial support without USB, Display & power domain updated
- power domain support with USB & Display support

Neil

> 
> Dependencies:
> - g12-common.dtsi from the DVFS patchset at [1]
> 
> [1] https://patchwork.kernel.org/cover/11025309/
> 
> Neil Armstrong (11):
>   soc: amlogic: meson-gx-socinfo: Add SM1 and S905X3 IDs
>   dt-bindings: power: amlogic, meson-gx-pwrc: Add SM1 bindings
>   soc: amlogic: gx-pwrc-vpu: add SM1 support
>   soc: amlogic: Add support for SM1 power controller
>   dt-bindings: soc: amlogic: clk-measure: Add SM1 compatible
>   soc: amlogic: clk-measure: Add support for SM1
>   dt-bindings: media: meson-ao-cec: add SM1 compatible
>   media: platform: meson-ao-cec-g12a: add support for SM1
>   dt-bindings: arm: amlogic: add SM1 bindings
>   dt-bindings: arm: amlogic: add SEI Robotics SEI610 bindings
>   arm64: dts: add support for SM1 based SEI Robotics SEI610
> 
>  .../devicetree/bindings/arm/amlogic.yaml      |   5 +
>  .../bindings/media/meson-ao-cec.txt           |   8 +-
>  .../bindings/power/amlogic,meson-gx-pwrc.txt  |  35 ++
>  .../bindings/soc/amlogic/clk-measure.txt      |   1 +
>  arch/arm64/boot/dts/amlogic/Makefile          |   1 +
>  .../boot/dts/amlogic/meson-sm1-sei610.dts     | 329 ++++++++++++++++++
>  arch/arm64/boot/dts/amlogic/meson-sm1.dtsi    |  77 ++++
>  drivers/media/platform/meson/ao-cec-g12a.c    |  37 +-
>  drivers/soc/amlogic/Kconfig                   |  11 +
>  drivers/soc/amlogic/Makefile                  |   1 +
>  drivers/soc/amlogic/meson-clk-measure.c       | 134 +++++++
>  drivers/soc/amlogic/meson-gx-pwrc-vpu.c       | 120 +++++++
>  drivers/soc/amlogic/meson-gx-socinfo.c        |   2 +
>  drivers/soc/amlogic/meson-sm1-pwrc.c          | 245 +++++++++++++
>  include/dt-bindings/power/meson-sm1-power.h   |  15 +
>  15 files changed, 1017 insertions(+), 4 deletions(-)
>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-sm1.dtsi
>  create mode 100644 drivers/soc/amlogic/meson-sm1-pwrc.c
>  create mode 100644 include/dt-bindings/power/meson-sm1-power.h
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 05/10] ARM: dts: sunxi: a80: Add msgbox node
From: Samuel Holland @ 2019-08-20 13:17 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, linux-sunxi, Stephen Boyd,
	Michael Turquette, Jassi Brar, linux-kernel, Chen-Yu Tsai,
	Rob Herring, Corentin Labbe, linux-clk, linux-arm-kernel
In-Reply-To: <20190820081528.7g2lo4njkut5lanu@flea>

Hi,

On 8/20/19 3:15 AM, Maxime Ripard wrote:
> On Mon, Aug 19, 2019 at 10:23:06PM -0500, Samuel Holland wrote:
>> The A80 SoC contains a message box that can be used to send messages and
>> interrupts back and forth between the ARM application CPUs and the ARISC
>> coprocessor. Add a device tree node for it.
>>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
> 
> I think you mentionned that crust has been tested only on the A64 and
> the H3/H5, did you test the mailbox on those other SoCs as well?

No, I only have A64/H3/H5, and recently H6, hardware to test. I've looked
through the manuals to verify that the registers are all the same, but I haven't
run the driver on earlier SoCs.

On 32-bit SoCs, where there's no other user of SRAM A2, it should be easy to get
the toy firmware running. All you should need to do is:
  1) Update the MMIO base/clock addresses in drivers/msgbox/sunxi-msgbox.c
  2) Update the load address in platform/sun50i/include/platform/memory.h
  3) Load the firmware to SRAM A2 (can be done from a U-Boot shell)
  4) Initialize the reset vector (algorithm is in tools/test.c:109)
  5) Deassert AR100 reset (again, these last two steps can be done from U-Boot)

Thanks,
Samuel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 1/3] kprobes/x86: use instruction_pointer and instruction_pointer_set
From: Peter Zijlstra @ 2019-08-20 13:21 UTC (permalink / raw)
  To: Jisheng Zhang
  Cc: Jonathan Corbet, Catalin Marinas, x86@kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Anil S Keshavamurthy, Ingo Molnar, Borislav Petkov,
	Masami Hiramatsu, H. Peter Anvin, Naveen N. Rao, Thomas Gleixner,
	Will Deacon, David S. Miller,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190820165152.20275268@xhacker.debian>

On Tue, Aug 20, 2019 at 09:02:59AM +0000, Jisheng Zhang wrote:
> In v2, actually, the arm64 version's kprobe_ftrace_handler() is the same
> as x86's, the only difference is comment, e.g
> 
> /* Kprobe handler expects regs->ip = ip + 1 as breakpoint hit */
> 
> while in arm64
> 
> /* Kprobe handler expects regs->pc = ip + 1 as breakpoint hit */

What's weird; I thought ARM has fixed sized instructions and they are
all 4 bytes? So how does a single byte offset make sense for ARM?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Status of Subsystems - MICROCHIP SAMA5D2-COMPATIBLE PIOBU GPIO
From: Sebastian Duda @ 2019-08-20 13:27 UTC (permalink / raw)
  To: Andrei Stefanescu
  Cc: linux-gpio, linux-kernel, linux-arm-kernel, lukas.bulwahn

Hello Andrei,

in my master thesis, I'm using the association of subsystems to 
maintainers/reviewers and its status given in the MAINTAINERS file.
During the research I noticed that there are several subsystems without 
a status in the maintainers file. One of them is the subsystem 
`MICROCHIP SAMA5D2-COMPATIBLE PIOBU GPIO` where you're mentioned as 
maintainer.

Is it intended not to mention a status for your subsystems?
What is the current status of your subsystem?

Kind regards
Sebastian Duda

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] MAINTAINERS: Extend patterns for Samsung SoC, Security Subsystem and clock drivers
From: Sylwester Nawrocki @ 2019-08-20 13:34 UTC (permalink / raw)
  To: Krzysztof Kozlowski, linux-samsung-soc, linux-clk
  Cc: Herbert Xu, Stephen Boyd, linux-kernel, Michael Turquette,
	Tomasz Figa, Vladimir Zapolskiy, Chanwoo Choi, Kukjin Kim,
	linux-crypto, Kamil Konieczny, David S. Miller, linux-arm-kernel
In-Reply-To: <20190818172750.20921-1-krzk@kernel.org>

On 8/18/19 19:27, Krzysztof Kozlowski wrote:
> Extend the patterns to cover all related files in respective
> categories:
> 1. Samsung Exynos ARM architecture: add soc drivers headers and make
>    directory matches consistent,
> 2. Samsung Security SubSystem driver (crypto): add bindings,
> 3. Samsung SoC clock drivers: add S3C24xx, S3C64xx and S5Pv210 bindings.

Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 04/10] mailbox: sunxi-msgbox: Add a new mailbox driver
From: Ondřej Jirman @ 2019-08-20 13:34 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Mark Rutland, devicetree, linux-sunxi, Maxime Ripard,
	Michael Turquette, Jassi Brar, linux-kernel, Stephen Boyd,
	Chen-Yu Tsai, Rob Herring, Corentin Labbe, linux-clk,
	linux-arm-kernel
In-Reply-To: <bc09e14c-1cf5-8124-fc34-c651b78577ce@sholland.org>

Hi,

On Tue, Aug 20, 2019 at 08:07:53AM -0500, Samuel Holland wrote:
> On 8/20/19 6:18 AM, Ondřej Jirman wrote:
> > Hi Samuel,
> > 
> > On Mon, Aug 19, 2019 at 10:23:05PM -0500, Samuel Holland wrote:
> >> Allwinner sun8i, sun9i, and sun50i SoCs contain a hardware message box
> >> used for communication between the ARM CPUs and the ARISC management
> >> coprocessor. The hardware contains 8 unidirectional 4-message FIFOs.
> >>
> >> Add a driver for it, so it can be used for SCPI or other communication
> >> protocols.
> >>
> >> Signed-off-by: Samuel Holland <samuel@sholland.org>
> >> ---
> >>  drivers/mailbox/Kconfig        |  10 +
> >>  drivers/mailbox/Makefile       |   2 +
> >>  drivers/mailbox/sunxi-msgbox.c | 323 +++++++++++++++++++++++++++++++++
> >>  3 files changed, 335 insertions(+)
> >>  create mode 100644 drivers/mailbox/sunxi-msgbox.c
> >>
> >> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> >> index ab4eb750bbdd..57d12936175e 100644
> >> --- a/drivers/mailbox/Kconfig
> >> +++ b/drivers/mailbox/Kconfig
> >> @@ -227,4 +227,14 @@ config ZYNQMP_IPI_MBOX
> >>  	  message to the IPI buffer and will access the IPI control
> >>  	  registers to kick the other processor or enquire status.
> >>  
> >> +config SUNXI_MSGBOX
> >> +	tristate "Allwinner sunxi Message Box"
> >> +	depends on ARCH_SUNXI || COMPILE_TEST
> >> +	default ARCH_SUNXI
> >> +	help
> >> +	  Mailbox implementation for the hardware message box present in
> >> +	  Allwinner sun8i, sun9i, and sun50i SoCs. The hardware message box is
> >> +	  used for communication between the application CPUs and the power
> >> +	  management coprocessor.
> >> +
> >>  endif
> >> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> >> index c22fad6f696b..bec2d50b0976 100644
> >> --- a/drivers/mailbox/Makefile
> >> +++ b/drivers/mailbox/Makefile
> >> @@ -48,3 +48,5 @@ obj-$(CONFIG_STM32_IPCC) 	+= stm32-ipcc.o
> >>  obj-$(CONFIG_MTK_CMDQ_MBOX)	+= mtk-cmdq-mailbox.o
> >>  
> >>  obj-$(CONFIG_ZYNQMP_IPI_MBOX)	+= zynqmp-ipi-mailbox.o
> >> +
> >> +obj-$(CONFIG_SUNXI_MSGBOX)	+= sunxi-msgbox.o
> >> diff --git a/drivers/mailbox/sunxi-msgbox.c b/drivers/mailbox/sunxi-msgbox.c
> >> new file mode 100644
> >> index 000000000000..29a5101a5390
> >> --- /dev/null
> >> +++ b/drivers/mailbox/sunxi-msgbox.c
> >> @@ -0,0 +1,323 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +//
> >> +// Copyright (c) 2017-2019 Samuel Holland <samuel@sholland.org>
> >> +
> >> +#include <linux/bitops.h>
> >> +#include <linux/clk.h>
> >> +#include <linux/device.h>
> >> +#include <linux/err.h>
> >> +#include <linux/interrupt.h>
> >> +#include <linux/io.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/mailbox_controller.h>
> >> +#include <linux/module.h>
> >> +#include <linux/of.h>
> >> +#include <linux/of_irq.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/reset.h>
> >> +#include <linux/spinlock.h>
> >> +
> >> +#define NUM_CHANS		8
> >> +
> >> +#define CTRL_REG(n)		(0x0000 + 0x4 * ((n) / 4))
> >> +#define CTRL_RX(n)		BIT(0 + 8 * ((n) % 4))
> >> +#define CTRL_TX(n)		BIT(4 + 8 * ((n) % 4))
> >> +
> >> +#define REMOTE_IRQ_EN_REG	0x0040
> >> +#define REMOTE_IRQ_STAT_REG	0x0050
> >> +#define LOCAL_IRQ_EN_REG	0x0060
> >> +#define LOCAL_IRQ_STAT_REG	0x0070
> >> +
> >> +#define RX_IRQ(n)		BIT(0 + 2 * (n))
> >> +#define RX_IRQ_MASK		0x5555
> >> +#define TX_IRQ(n)		BIT(1 + 2 * (n))
> >> +#define TX_IRQ_MASK		0xaaaa
> >> +
> >> +#define FIFO_STAT_REG(n)	(0x0100 + 0x4 * (n))
> >> +#define FIFO_STAT_MASK		GENMASK(0, 0)
> >> +
> >> +#define MSG_STAT_REG(n)		(0x0140 + 0x4 * (n))
> >> +#define MSG_STAT_MASK		GENMASK(2, 0)
> >> +
> >> +#define MSG_DATA_REG(n)		(0x0180 + 0x4 * (n))
> >> +
> >> +#define mbox_dbg(mbox, ...)	dev_dbg((mbox)->controller.dev, __VA_ARGS__)
> >> +
> >> +struct sunxi_msgbox {
> >> +	struct mbox_controller controller;
> >> +	struct clk *clk;
> >> +	spinlock_t lock;
> >> +	void __iomem *regs;
> >> +};
> >> +
> >> +static bool sunxi_msgbox_last_tx_done(struct mbox_chan *chan);
> >> +static bool sunxi_msgbox_peek_data(struct mbox_chan *chan);
> >> +
> >> +static inline int channel_number(struct mbox_chan *chan)
> >> +{
> >> +	return chan - chan->mbox->chans;
> >> +}
> >> +
> >> +static inline struct sunxi_msgbox *channel_to_msgbox(struct mbox_chan *chan)
> >> +{
> >> +	return chan->con_priv;
> >> +}
> >> +
> >> +static irqreturn_t sunxi_msgbox_irq(int irq, void *dev_id)
> >> +{
> >> +	struct sunxi_msgbox *mbox = dev_id;
> >> +	uint32_t status;
> >> +	int n;
> >> +
> >> +	/* Only examine channels that are currently enabled. */
> >> +	status = readl(mbox->regs + LOCAL_IRQ_EN_REG) &
> >> +		 readl(mbox->regs + LOCAL_IRQ_STAT_REG);
> >> +
> >> +	if (!(status & RX_IRQ_MASK))
> >> +		return IRQ_NONE;
> >> +
> >> +	for (n = 0; n < NUM_CHANS; ++n) {
> >> +		struct mbox_chan *chan = &mbox->controller.chans[n];
> >> +
> >> +		if (!(status & RX_IRQ(n)))
> >> +			continue;
> >> +
> >> +		while (sunxi_msgbox_peek_data(chan)) {
> >> +			uint32_t msg = readl(mbox->regs + MSG_DATA_REG(n));
> >> +
> >> +			mbox_dbg(mbox, "Channel %d received 0x%08x\n", n, msg);
> >> +			mbox_chan_received_data(chan, &msg);
> >> +		}
> >> +
> >> +		/* The IRQ can be cleared only once the FIFO is empty. */
> >> +		writel(RX_IRQ(n), mbox->regs + LOCAL_IRQ_STAT_REG);
> >> +	}
> >> +
> >> +	return IRQ_HANDLED;
> >> +}
> >> +
> >> +static int sunxi_msgbox_send_data(struct mbox_chan *chan, void *data)
> >> +{
> >> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
> >> +	int n = channel_number(chan);
> >> +	uint32_t msg = *(uint32_t *)data;
> >> +
> >> +	/* Using a channel backwards gets the hardware into a bad state. */
> >> +	if (WARN_ON_ONCE(!(readl(mbox->regs + CTRL_REG(n)) & CTRL_TX(n))))
> >> +		return 0;
> >> +
> >> +	/* We cannot post a new message if the FIFO is full. */
> >> +	if (readl(mbox->regs + FIFO_STAT_REG(n)) & FIFO_STAT_MASK) {
> >> +		mbox_dbg(mbox, "Channel %d busy sending 0x%08x\n", n, msg);
> >> +		return -EBUSY;
> >> +	}
> >> +
> >> +	writel(msg, mbox->regs + MSG_DATA_REG(n));
> >> +	mbox_dbg(mbox, "Channel %d sent 0x%08x\n", n, msg);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int sunxi_msgbox_startup(struct mbox_chan *chan)
> >> +{
> >> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
> >> +	int n = channel_number(chan);
> >> +
> >> +	/* The coprocessor is responsible for setting channel directions. */
> >> +	if (readl(mbox->regs + CTRL_REG(n)) & CTRL_RX(n)) {
> >> +		/* Flush the receive FIFO. */
> >> +		while (sunxi_msgbox_peek_data(chan))
> >> +			readl(mbox->regs + MSG_DATA_REG(n));
> >> +		writel(RX_IRQ(n), mbox->regs + LOCAL_IRQ_STAT_REG);
> >> +
> >> +		/* Enable the receive IRQ. */
> >> +		spin_lock(&mbox->lock);
> >> +		writel(readl(mbox->regs + LOCAL_IRQ_EN_REG) | RX_IRQ(n),
> >> +		       mbox->regs + LOCAL_IRQ_EN_REG);
> >> +		spin_unlock(&mbox->lock);
> >> +	}
> >> +
> >> +	mbox_dbg(mbox, "Channel %d startup complete\n", n);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static void sunxi_msgbox_shutdown(struct mbox_chan *chan)
> >> +{
> >> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
> >> +	int n = channel_number(chan);
> >> +
> >> +	if (readl(mbox->regs + CTRL_REG(n)) & CTRL_RX(n)) {
> >> +		/* Disable the receive IRQ. */
> >> +		spin_lock(&mbox->lock);
> >> +		writel(readl(mbox->regs + LOCAL_IRQ_EN_REG) & ~RX_IRQ(n),
> >> +		       mbox->regs + LOCAL_IRQ_EN_REG);
> >> +		spin_unlock(&mbox->lock);
> >> +
> >> +		/* Attempt to flush the FIFO until the IRQ is cleared. */
> >> +		do {
> >> +			while (sunxi_msgbox_peek_data(chan))
> >> +				readl(mbox->regs + MSG_DATA_REG(n));
> >> +			writel(RX_IRQ(n), mbox->regs + LOCAL_IRQ_STAT_REG);
> >> +		} while (readl(mbox->regs + LOCAL_IRQ_STAT_REG) & RX_IRQ(n));
> >> +	}
> >> +
> >> +	mbox_dbg(mbox, "Channel %d shutdown complete\n", n);
> >> +}
> >> +
> >> +static bool sunxi_msgbox_last_tx_done(struct mbox_chan *chan)
> >> +{
> >> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
> >> +	int n = channel_number(chan);
> >> +
> >> +	/*
> >> +	 * The hardware allows snooping on the remote user's IRQ statuses.
> >> +	 * We consider a message to be acknowledged only once the receive IRQ
> >> +	 * for that channel is cleared. Since the receive IRQ for a channel
> >> +	 * cannot be cleared until the FIFO for that channel is empty, this
> >> +	 * ensures that the message has actually been read. It also gives the
> >> +	 * recipient an opportunity to perform minimal processing before
> >> +	 * acknowledging the message.
> >> +	 */
> >> +	return !(readl(mbox->regs + REMOTE_IRQ_STAT_REG) & RX_IRQ(n));
> >> +}
> >> +
> >> +static bool sunxi_msgbox_peek_data(struct mbox_chan *chan)
> >> +{
> >> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
> >> +	int n = channel_number(chan);
> >> +
> >> +	return readl(mbox->regs + MSG_STAT_REG(n)) & MSG_STAT_MASK;
> >> +}
> >> +
> >> +static const struct mbox_chan_ops sunxi_msgbox_chan_ops = {
> >> +	.send_data    = sunxi_msgbox_send_data,
> >> +	.startup      = sunxi_msgbox_startup,
> >> +	.shutdown     = sunxi_msgbox_shutdown,
> >> +	.last_tx_done = sunxi_msgbox_last_tx_done,
> >> +	.peek_data    = sunxi_msgbox_peek_data,
> >> +};
> >> +
> >> +static int sunxi_msgbox_probe(struct platform_device *pdev)
> >> +{
> >> +	struct device *dev = &pdev->dev;
> >> +	struct mbox_chan *chans;
> >> +	struct reset_control *reset;
> >> +	struct resource *res;
> >> +	struct sunxi_msgbox *mbox;
> >> +	int i, ret;
> >> +
> >> +	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> >> +	if (!mbox)
> >> +		return -ENOMEM;
> >> +
> >> +	chans = devm_kcalloc(dev, NUM_CHANS, sizeof(*chans), GFP_KERNEL);
> >> +	if (!chans)
> >> +		return -ENOMEM;
> >> +
> >> +	for (i = 0; i < NUM_CHANS; ++i)
> >> +		chans[i].con_priv = mbox;
> >> +
> >> +	mbox->clk = devm_clk_get(dev, NULL);
> >> +	if (IS_ERR(mbox->clk)) {
> >> +		ret = PTR_ERR(mbox->clk);
> >> +		dev_err(dev, "Failed to get clock: %d\n", ret);
> >> +		return ret;
> >> +	}
> >> +
> >> +	ret = clk_prepare_enable(mbox->clk);
> >> +	if (ret) {
> >> +		dev_err(dev, "Failed to enable clock: %d\n", ret);
> >> +		return ret;
> >> +	}
> >> +
> >> +	reset = devm_reset_control_get(dev, NULL);
> >> +	if (IS_ERR(reset)) {
> >> +		ret = PTR_ERR(reset);
> >> +		dev_err(dev, "Failed to get reset control: %d\n", ret);
> >> +		goto err_disable_unprepare;
> >> +	}
> >> +
> >> +	ret = reset_control_deassert(reset);
> >> +	if (ret) {
> >> +		dev_err(dev, "Failed to deassert reset: %d\n", ret);
> >> +		goto err_disable_unprepare;
> >> +	}
> > 
> > You need to assert the reset again from now on, in error paths. devm
> > will not do that for you.
> 
> I know, and that's intentional. This same message box device is used for ATF to
> communicate with SCP firmware (on a different channel). This could be happening
> on a different core while Linux is running. So Linux is not allowed to deassert
> the reset. clk_disable_unprepare() is only okay because the clock is marked as
> critical.

Okay. It needs to be docummented here though, so that someone will
not "fix" it in the future, after finding this with coccinelle or
something.

regards,
	o.

> >> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> +	if (!res) {
> >> +		ret = -ENODEV;
> >> +		goto err_disable_unprepare;
> >> +	}
> >> +
> >> +	mbox->regs = devm_ioremap_resource(&pdev->dev, res);
> >> +	if (IS_ERR(mbox->regs)) {
> >> +		ret = PTR_ERR(mbox->regs);
> >> +		dev_err(dev, "Failed to map MMIO resource: %d\n", ret);
> >> +		goto err_disable_unprepare;
> >> +	}
> >> +
> >> +	/* Disable all IRQs for this end of the msgbox. */
> >> +	writel(0, mbox->regs + LOCAL_IRQ_EN_REG);
> >> +
> >> +	ret = devm_request_irq(dev, irq_of_parse_and_map(dev->of_node, 0),
> >> +			       sunxi_msgbox_irq, 0, dev_name(dev), mbox);
> >> +	if (ret) {
> >> +		dev_err(dev, "Failed to register IRQ handler: %d\n", ret);
> >> +		goto err_disable_unprepare;
> >> +	}
> >> +
> >> +	mbox->controller.dev           = dev;
> >> +	mbox->controller.ops           = &sunxi_msgbox_chan_ops;
> >> +	mbox->controller.chans         = chans;
> >> +	mbox->controller.num_chans     = NUM_CHANS;
> >> +	mbox->controller.txdone_irq    = false;
> >> +	mbox->controller.txdone_poll   = true;
> >> +	mbox->controller.txpoll_period = 5;
> >> +
> >> +	spin_lock_init(&mbox->lock);
> >> +	platform_set_drvdata(pdev, mbox);
> >> +
> >> +	ret = mbox_controller_register(&mbox->controller);
> >> +	if (ret) {
> >> +		dev_err(dev, "Failed to register controller: %d\n", ret);
> >> +		goto err_disable_unprepare;
> >> +	}
> >> +
> >> +	return 0;
> >> +
> >> +err_disable_unprepare:
> >> +	clk_disable_unprepare(mbox->clk);
> >> +
> >> +	return ret;
> >> +}
> >> +
> >> +static int sunxi_msgbox_remove(struct platform_device *pdev)
> >> +{
> >> +	struct sunxi_msgbox *mbox = platform_get_drvdata(pdev);
> >> +
> >> +	mbox_controller_unregister(&mbox->controller);
> >> +	clk_disable_unprepare(mbox->clk);
> > 
> > Also, assert the reset here.
> 
> Same comment as above. This is intentional.
> 
> Thanks,
> Samuel
> 
> >> +	return 0;
> >> +}
> >> +
> >> +static const struct of_device_id sunxi_msgbox_of_match[] = {
> >> +	{ .compatible = "allwinner,sun6i-a31-msgbox", },
> >> +	{},
> >> +};
> >> +MODULE_DEVICE_TABLE(of, sunxi_msgbox_of_match);
> >> +
> >> +static struct platform_driver sunxi_msgbox_driver = {
> >> +	.driver = {
> >> +		.name = "sunxi-msgbox",
> >> +		.of_match_table = sunxi_msgbox_of_match,
> >> +	},
> >> +	.probe  = sunxi_msgbox_probe,
> >> +	.remove = sunxi_msgbox_remove,
> >> +};
> >> +module_platform_driver(sunxi_msgbox_driver);
> >> +
> >> +MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>");
> >> +MODULE_DESCRIPTION("Allwinner sunxi Message Box");
> >> +MODULE_LICENSE("GPL v2");
> >> -- 
> >> 2.21.0
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 4/4] iommu/io-pgtable-arm: Prepare for TTBR1 usage
From: Robin Murphy @ 2019-08-20 13:51 UTC (permalink / raw)
  To: will, joro, iommu, linux-arm-kernel, robdclark
In-Reply-To: <20190819223439.GG28465@jcrouse1-lnx.qualcomm.com>

On 19/08/2019 23:34, Jordan Crouse wrote:
> On Mon, Aug 19, 2019 at 07:19:31PM +0100, Robin Murphy wrote:
>> Now that callers are free to use a given table for TTBR1 if they wish
>> (all they need do is shift the provided attributes when constructing
>> their final TCR value), the only remaining impediment is the address
>> validation on map/unmap. The fact that the LPAE address space split is
>> symmetric makes this easy to accommodate - by simplifying the current
>> range checks into explicit tests that address bits above IAS are all
>> zero, it then follows straightforwardly to add the inverse test to
>> allow the all-ones case as well.
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>>   drivers/iommu/io-pgtable-arm.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
>> index 09cb20671fbb..f39c50356351 100644
>> --- a/drivers/iommu/io-pgtable-arm.c
>> +++ b/drivers/iommu/io-pgtable-arm.c
>> @@ -475,13 +475,13 @@ static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
>>   	arm_lpae_iopte *ptep = data->pgd;
>>   	int ret, lvl = ARM_LPAE_START_LVL(data);
>>   	arm_lpae_iopte prot;
>> +	long iaext = (long)iova >> data->iop.cfg.ias;
>>   
>>   	/* If no access, then nothing to do */
>>   	if (!(iommu_prot & (IOMMU_READ | IOMMU_WRITE)))
>>   		return 0;
>>   
>> -	if (WARN_ON(iova >= (1ULL << data->iop.cfg.ias) ||
>> -		    paddr >= (1ULL << data->iop.cfg.oas)))
>> +	if (WARN_ON((iaext && ~iaext) || paddr >> data->iop.cfg.oas))
>>   		return -ERANGE;
>>   
>>   	prot = arm_lpae_prot_to_pte(data, iommu_prot);
> 
> We'll want to cast away the sign extended bits before mapping the iova, this
> might be a good patch for that too as long as we are calculating the iaext.

Ah good point, I'd forgotten that ARM_LPAE_LVL_IDX() doesn't actually 
cap to IAS if the top level is smaller than bits_per_level (I suppose we 
*could* make it do so for purity, but that's bound to hurt efficiency 
far more than just zeroing out the offending bits here).

Thanks,
Robin.

> 
>> @@ -647,8 +647,9 @@ static size_t arm_lpae_unmap(struct io_pgtable_ops *ops, unsigned long iova,
>>   	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
>>   	arm_lpae_iopte *ptep = data->pgd;
>>   	int lvl = ARM_LPAE_START_LVL(data);
>> +	long iaext = (long)iova >> data->iop.cfg.ias;
>>   
>> -	if (WARN_ON(iova >= (1ULL << data->iop.cfg.ias)))
>> +	if (WARN_ON(iaext && ~iaext))
>>   		return 0;
>>   
>>   	return __arm_lpae_unmap(data, iova, size, lvl, ptep);
> 
> And here too.
> 
> Jordan
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ 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