Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH] MIPS: implement a "bootargs-append" DT property
From: James Hogan @ 2017-11-13 11:23 UTC (permalink / raw)
  To: Daniel Gimpelevich
  Cc: linux-mips-6z/3iImG2C8G8FEW9MqTrA, Rob Herring, Frank Rowand,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1510420788-25184-1-git-send-email-daniel-R/FLGEdV95bo9U+Z1CfBt0SU0eOFXohjCypLqA8HKkk@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2248 bytes --]

On Sat, Nov 11, 2017 at 09:19:48AM -0800, Daniel Gimpelevich wrote:
> There are two uses for this:
> 
> 1) It may be useful to split a device-specific kernel command line between
> a .dts file and a .dtsi file, with "bootargs" in one and "bootargs-append"
> in the other, such as for variations of a reference board.
> 
> 2) There are kernel configuration options for prepending "bootargs" to the
> kernel command line that the bootloader has passed, but not for appending.
> A new option for this would be a less future-proof solution, since things
> like this should be in the dtb.
> 
> This is tested on MIPS, but it can be useful on other architectures also.
> 
> Signed-off-by: Daniel Gimpelevich <daniel-R/FLGEdV95bo9U+Z1CfBt0SU0eOFXohjCypLqA8HKkk@public.gmane.org>

The device tree maintainers should be on Cc. Adding them now.

Cheers
James

> ---
>  arch/mips/kernel/setup.c | 3 +++
>  drivers/of/fdt.c         | 4 ++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> index fe39397..95e9bf2 100644
> --- a/arch/mips/kernel/setup.c
> +++ b/arch/mips/kernel/setup.c
> @@ -826,7 +826,10 @@ static void __init arch_mem_init(char **cmdline_p)
>  	extern void plat_mem_setup(void);
>  
>  	/* call board setup routine */
> +	strlcpy(boot_command_line, arcs_cmdline, COMMAND_LINE_SIZE);
>  	plat_mem_setup();
> +	if (strncmp(boot_command_line, arcs_cmdline, COMMAND_LINE_SIZE) == 0)
> +		boot_command_line[0] = '\0';
>  
>  	/*
>  	 * Make sure all kernel memory is in the maps.  The "UP" and
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index ce30c9a..65dbda6 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -1127,6 +1127,10 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>  	p = of_get_flat_dt_prop(node, "bootargs", &l);
>  	if (p != NULL && l > 0)
>  		strlcpy(data, p, min((int)l, COMMAND_LINE_SIZE));
> +	p = of_get_flat_dt_prop(node, "bootargs-append", &l);
> +	if (p != NULL && l > 0)
> +		strlcat(data, p, min_t(int, strlen(data) + l,
> +					COMMAND_LINE_SIZE));
>  
>  	/*
>  	 * CONFIG_CMDLINE is meant to be a default in case nothing else
> -- 
> 1.9.1
> 
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v2 1/2] ARM: dts: exynos: Switch to dedicated Odroid-XU3 sound card binding
From: Sylwester Nawrocki @ 2017-11-13 11:30 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: kgene-DgEjT+Ai2ygdnm+yROfE0A, mihailescu2m-Re5JQEeQqe8AvxtiuMwx3w,
	m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ,
	b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171110171417.qbnn7gsihor2fwng@kozik-lap>

On 11/10/2017 06:14 PM, Krzysztof Kozlowski wrote:
> On Fri, Nov 03, 2017 at 05:54:45PM +0100, Sylwester Nawrocki wrote:
>> The new sound card DT binding is used for Odroid XU3 in order
>> to properly support the HDMI audio path.
>> Clocks configuration is changed so the I2S controller is now the bit
>> and the frame clock master with EPLL as the root clock source.
>
> Patch looks good but I do not understand this last sentence. What did
> you want to say?

Which part is hard to understand exactly? Perhaps this is clearer?

"Clocks configuration is changed so the I2S controller is now the bit
clock and the frame clock master.  The EPLL output clock is now the 
audio root clock adjusted to each audio sample rate."

Previously the CODEC's internal PLL was used for generating the I2S
bit and frame clocks accorrding to each audio sample rate.

-- 
Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 1/2] ARM: dts: exynos: Switch to dedicated Odroid-XU3 sound card binding
From: Krzysztof Kozlowski @ 2017-11-13 11:33 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: kgene, mihailescu2m, Marek Szyprowski,
	Bartłomiej Żołnierkiewicz, linux-samsung-soc,
	linux-arm-kernel, linux-kernel, devicetree
In-Reply-To: <6ffab0bd-abc7-be8a-38f0-695269271427@samsung.com>

On Mon, Nov 13, 2017 at 12:30 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> On 11/10/2017 06:14 PM, Krzysztof Kozlowski wrote:
>> On Fri, Nov 03, 2017 at 05:54:45PM +0100, Sylwester Nawrocki wrote:
>>> The new sound card DT binding is used for Odroid XU3 in order
>>> to properly support the HDMI audio path.
>>> Clocks configuration is changed so the I2S controller is now the bit
>>> and the frame clock master with EPLL as the root clock source.
>>
>> Patch looks good but I do not understand this last sentence. What did
>> you want to say?
>
> Which part is hard to understand exactly? Perhaps this is clearer?
>
> "Clocks configuration is changed so the I2S controller is now the bit
> clock and the frame clock master.  The EPLL output clock is now the
> audio root clock adjusted to each audio sample rate."
>
> Previously the CODEC's internal PLL was used for generating the I2S
> bit and frame clocks accorrding to each audio sample rate.

Ahhh, makes sense, the previous "is not the bit and..." mislead me. I
thought it's a typo, you wanted to use "but" and something is missing.
The extended description is I think better, so I'll use it in commit
msg when applying.

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH v3 1/3] media: V3s: Add support for Allwinner CSI.
From: 'Ondřej Jirman' via linux-sunxi @ 2017-11-13 11:44 UTC (permalink / raw)
  To: yong.deng-+3dxTMOEIRNWk0Htik3J/w,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
  Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai,
	David S. Miller, Greg Kroah-Hartman, Hans Verkuil, Randy Dunlap,
	Benoit Parrot, Stanimir Varbanov, Arnd Bergmann, Hugues Fruchet,
	Philipp Zabel, Benjamin Gaignard, Ramesh Shanmugasundaram,
	Yannick Fertre, Sakari Ailus, Rick Chang,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-su
In-Reply-To: <1510558216-43800-1-git-send-email-yong.deng-+3dxTMOEIRNWk0Htik3J/w@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 70908 bytes --]

Hello,

Thanks for your patches. I've been working with CSI on A83T, so I have
a few notes. My review is below. :)

Yong Deng píše v Po 13. 11. 2017 v 15:30 +0800:
> Allwinner V3s SoC have two CSI module. CSI0 is used for MIPI interface
> and CSI1 is used for parallel interface. This is not documented in
> datasheet but by testing and guess.
> 
> This patch implement a v4l2 framework driver for it.
> 
> Currently, the driver only support the parallel interface. MIPI-CSI2,
> ISP's support are not included in this patch.
> 
> Signed-off-by: Yong Deng <yong.deng-+3dxTMOEIRNWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/media/platform/Kconfig                     |   1 +
>  drivers/media/platform/Makefile                    |   2 +
>  drivers/media/platform/sunxi/sun6i-csi/Kconfig     |   9 +
>  drivers/media/platform/sunxi/sun6i-csi/Makefile    |   3 +
>  drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 918 +++++++++++++++++++++
>  drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h | 146 ++++
>  .../media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h | 203 +++++
>  .../media/platform/sunxi/sun6i-csi/sun6i_video.c   | 722 ++++++++++++++++
>  .../media/platform/sunxi/sun6i-csi/sun6i_video.h   |  61 ++
>  9 files changed, 2065 insertions(+)
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/Kconfig
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/Makefile
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.h
> 
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index fd0c998..41017e3 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -150,6 +150,7 @@ source "drivers/media/platform/am437x/Kconfig"
>  source "drivers/media/platform/xilinx/Kconfig"
>  source "drivers/media/platform/rcar-vin/Kconfig"
>  source "drivers/media/platform/atmel/Kconfig"
> +source "drivers/media/platform/sunxi/sun6i-csi/Kconfig"
>  
>  config VIDEO_TI_CAL
>  	tristate "TI CAL (Camera Adaptation Layer) driver"
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 012eb47..1cc806a 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -96,3 +96,5 @@ obj-$(CONFIG_VIDEO_QCOM_CAMSS)		+= qcom/camss-8x16/
>  obj-$(CONFIG_VIDEO_QCOM_VENUS)		+= qcom/venus/
>  
>  obj-y					+= meson/
> +
> +obj-$(CONFIG_VIDEO_SUN6I_CSI)		+= sunxi/sun6i-csi/
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/Kconfig b/drivers/media/platform/sunxi/sun6i-csi/Kconfig
> new file mode 100644
> index 0000000..314188a
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun6i-csi/Kconfig
> @@ -0,0 +1,9 @@
> +config VIDEO_SUN6I_CSI
> +	tristate "Allwinner V3s Camera Sensor Interface driver"
> +	depends on VIDEO_V4L2 && COMMON_CLK && VIDEO_V4L2_SUBDEV_API && HAS_DMA
> +	depends on ARCH_SUNXI || COMPILE_TEST
> +	select VIDEOBUF2_DMA_CONTIG
> +	select REGMAP_MMIO
> +	select V4L2_FWNODE
> +	---help---
> +	   Support for the Allwinner Camera Sensor Interface Controller on V3s.
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/Makefile b/drivers/media/platform/sunxi/sun6i-csi/Makefile
> new file mode 100644
> index 0000000..213cb6b
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun6i-csi/Makefile
> @@ -0,0 +1,3 @@
> +sun6i-csi-y += sun6i_video.o sun6i_csi.o
> +
> +obj-$(CONFIG_VIDEO_SUN6I_CSI) += sun6i-csi.o
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> new file mode 100644
> index 0000000..3f4de09
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> @@ -0,0 +1,918 @@
> +/*
> + * Copyright (c) 2017 Magewell Electronics Co., Ltd. (Nanjing).
> + * All rights reserved.
> + * Author: Yong Deng <yong.deng-+3dxTMOEIRNWk0Htik3J/w@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/err.h>
> +#include <linux/fs.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/ioctl.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/sched.h>
> +#include <linux/sizes.h>
> +#include <linux/slab.h>
> +
> +#include "sun6i_csi.h"
> +#include "sun6i_csi_reg.h"
> +
> +#define MODULE_NAME	"sun6i-csi"
> +
> +struct sun6i_csi_dev {
> +	struct sun6i_csi		csi;
> +	struct device			*dev;
> +
> +	struct regmap			*regmap;
> +	struct clk			*clk_mod;
> +	struct clk			*clk_ram;
> +	struct reset_control		*rstc_bus;
> +
> +	int				planar_offset[3];
> +};
> +
> +static const u32 supported_pixformats[] = {
> +	V4L2_PIX_FMT_SBGGR8,
> +	V4L2_PIX_FMT_SGBRG8,
> +	V4L2_PIX_FMT_SGRBG8,
> +	V4L2_PIX_FMT_SRGGB8,
> +	V4L2_PIX_FMT_SBGGR10,
> +	V4L2_PIX_FMT_SGBRG10,
> +	V4L2_PIX_FMT_SGRBG10,
> +	V4L2_PIX_FMT_SRGGB10,
> +	V4L2_PIX_FMT_SBGGR12,
> +	V4L2_PIX_FMT_SGBRG12,
> +	V4L2_PIX_FMT_SGRBG12,
> +	V4L2_PIX_FMT_SRGGB12,
> +	V4L2_PIX_FMT_YUYV,
> +	V4L2_PIX_FMT_YVYU,
> +	V4L2_PIX_FMT_UYVY,
> +	V4L2_PIX_FMT_VYUY,
> +	V4L2_PIX_FMT_HM12,
> +	V4L2_PIX_FMT_NV12,
> +	V4L2_PIX_FMT_NV21,
> +	V4L2_PIX_FMT_YUV420,
> +	V4L2_PIX_FMT_YVU420,
> +	V4L2_PIX_FMT_NV16,
> +	V4L2_PIX_FMT_NV61,
> +	V4L2_PIX_FMT_YUV422P,
> +};
> +
> +static inline struct sun6i_csi_dev *sun6i_csi_to_dev(struct sun6i_csi *csi)
> +{
> +	return container_of(csi, struct sun6i_csi_dev, csi);
> +}
> +
> +/* TODO add 10&12 bit YUV, RGB support */
> +static bool __is_format_support(struct sun6i_csi_dev *sdev,
> +			      u32 fourcc, u32 mbus_code)

The function name should be prefixed with sun6i_csi_* for easier to
read stack traces.

> +{
> +	/*
> +	 * Some video receiver have capability both 8bit and 16bit.
> +	 * Identify the media bus format from device tree.
> +	 */
> +	if ((sdev->csi.v4l2_ep.bus_type == V4L2_MBUS_PARALLEL
> +	      || sdev->csi.v4l2_ep.bus_type == V4L2_MBUS_BT656)
> +	     && sdev->csi.v4l2_ep.bus.parallel.bus_width == 16) {
> +		switch (fourcc) {
> +		case V4L2_PIX_FMT_HM12:
> +		case V4L2_PIX_FMT_NV12:
> +		case V4L2_PIX_FMT_NV21:
> +		case V4L2_PIX_FMT_NV16:
> +		case V4L2_PIX_FMT_NV61:
> +		case V4L2_PIX_FMT_YUV420:
> +		case V4L2_PIX_FMT_YVU420:
> +		case V4L2_PIX_FMT_YUV422P:
> +			switch (mbus_code) {
> +			case MEDIA_BUS_FMT_UYVY8_1X16:
> +			case MEDIA_BUS_FMT_VYUY8_1X16:
> +			case MEDIA_BUS_FMT_YUYV8_1X16:
> +			case MEDIA_BUS_FMT_YVYU8_1X16:
> +				return true;
> +			}
> +			break;
> +		}
> +		return false;
> +	}
> +
> +	switch (fourcc) {
> +	case V4L2_PIX_FMT_SBGGR8:
> +		if (mbus_code == MEDIA_BUS_FMT_SBGGR8_1X8)
> +			return true;
> +		break;

Can be return mbus_code == MEDIA_BUS_FMT_SBGGR8_1X8; to save three
linse. Ditto for similar cases below.

> +	case V4L2_PIX_FMT_SGBRG8:
> +		if (mbus_code == MEDIA_BUS_FMT_SGBRG8_1X8)
> +			return true;
> +		break;
> +	case V4L2_PIX_FMT_SGRBG8:
> +		if (mbus_code == MEDIA_BUS_FMT_SGRBG8_1X8)
> +			return true;
> +		break;
> +	case V4L2_PIX_FMT_SRGGB8:
> +		if (mbus_code == MEDIA_BUS_FMT_SRGGB8_1X8)
> +			return true;
> +		break;
> +	case V4L2_PIX_FMT_SBGGR10:
> +		if (mbus_code == MEDIA_BUS_FMT_SBGGR10_1X10)
> +			return true;
> +		break;
> +	case V4L2_PIX_FMT_SGBRG10:
> +		if (mbus_code == MEDIA_BUS_FMT_SGBRG10_1X10)
> +			return true;
> +		break;
> +	case V4L2_PIX_FMT_SGRBG10:
> +		if (mbus_code == MEDIA_BUS_FMT_SGRBG10_1X10)
> +			return true;
> +		break;
> +	case V4L2_PIX_FMT_SRGGB10:
> +		if (mbus_code == MEDIA_BUS_FMT_SRGGB10_1X10)
> +			return true;
> +		break;
> +	case V4L2_PIX_FMT_SBGGR12:
> +		if (mbus_code == MEDIA_BUS_FMT_SBGGR12_1X12)
> +			return true;
> +		break;
> +	case V4L2_PIX_FMT_SGBRG12:
> +		if (mbus_code == MEDIA_BUS_FMT_SGBRG12_1X12)
> +			return true;
> +		break;
> +	case V4L2_PIX_FMT_SGRBG12:
> +		if (mbus_code == MEDIA_BUS_FMT_SGRBG12_1X12)
> +			return true;
> +		break;
> +	case V4L2_PIX_FMT_SRGGB12:
> +		if (mbus_code == MEDIA_BUS_FMT_SRGGB12_1X12)
> +			return true;
> +		break;
> +
> +	case V4L2_PIX_FMT_YUYV:
> +		if (mbus_code == MEDIA_BUS_FMT_YUYV8_2X8)
> +			return true;
> +		break;
> +	case V4L2_PIX_FMT_YVYU:
> +		if (mbus_code == MEDIA_BUS_FMT_YVYU8_2X8)
> +			return true;
> +		break;
> +	case V4L2_PIX_FMT_UYVY:
> +		if (mbus_code == MEDIA_BUS_FMT_UYVY8_2X8)
> +			return true;
> +		break;
> +	case V4L2_PIX_FMT_VYUY:
> +		if (mbus_code == MEDIA_BUS_FMT_VYUY8_2X8)
> +			return true;
> +		break;
> +
> +	case V4L2_PIX_FMT_HM12:
> +	case V4L2_PIX_FMT_NV12:
> +	case V4L2_PIX_FMT_NV21:
> +	case V4L2_PIX_FMT_NV16:
> +	case V4L2_PIX_FMT_NV61:
> +	case V4L2_PIX_FMT_YUV420:
> +	case V4L2_PIX_FMT_YVU420:
> +	case V4L2_PIX_FMT_YUV422P:
> +		switch (mbus_code) {
> +		case MEDIA_BUS_FMT_UYVY8_2X8:
> +		case MEDIA_BUS_FMT_VYUY8_2X8:
> +		case MEDIA_BUS_FMT_YUYV8_2X8:
> +		case MEDIA_BUS_FMT_YVYU8_2X8:
> +			return true;
> +		}
> +		break;
> +	}
> +
> +	return false;
> +}
> +
> +static enum csi_input_fmt get_csi_input_format(u32 mbus_code, u32 pixformat)
> +{
> +	/* bayer */
> +	if ((mbus_code & 0xF000) == 0x3000)
> +		return CSI_INPUT_FORMAT_RAW;
> +
> +	switch (pixformat) {
> +	case V4L2_PIX_FMT_YUYV:
> +	case V4L2_PIX_FMT_YVYU:
> +	case V4L2_PIX_FMT_UYVY:
> +	case V4L2_PIX_FMT_VYUY:
> +		return CSI_INPUT_FORMAT_RAW;
> +	}
> +
> +	/* not support YUV420 input format yet */
> +	return CSI_INPUT_FORMAT_YUV422;
> +}
> +
> +static enum csi_output_fmt get_csi_output_format(u32 pixformat, u32 field)
> +{
> +	bool buf_interlaced = false;
> +
> +	if (field == V4L2_FIELD_INTERLACED
> +	    || field == V4L2_FIELD_INTERLACED_TB
> +	    || field == V4L2_FIELD_INTERLACED_BT)
> +		buf_interlaced = true;
> +
> +	switch (pixformat) {
> +	case V4L2_PIX_FMT_SBGGR8:
> +	case V4L2_PIX_FMT_SGBRG8:
> +	case V4L2_PIX_FMT_SGRBG8:
> +	case V4L2_PIX_FMT_SRGGB8:
> +		return buf_interlaced ? CSI_FRAME_RAW_8 : CSI_FIELD_RAW_8;
> +	case V4L2_PIX_FMT_SBGGR10:
> +	case V4L2_PIX_FMT_SGBRG10:
> +	case V4L2_PIX_FMT_SGRBG10:
> +	case V4L2_PIX_FMT_SRGGB10:
> +		return buf_interlaced ? CSI_FRAME_RAW_10 : CSI_FIELD_RAW_10;
> +	case V4L2_PIX_FMT_SBGGR12:
> +	case V4L2_PIX_FMT_SGBRG12:
> +	case V4L2_PIX_FMT_SGRBG12:
> +	case V4L2_PIX_FMT_SRGGB12:
> +		return buf_interlaced ? CSI_FRAME_RAW_12 : CSI_FIELD_RAW_12;
> +
> +	case V4L2_PIX_FMT_YUYV:
> +	case V4L2_PIX_FMT_YVYU:
> +	case V4L2_PIX_FMT_UYVY:
> +	case V4L2_PIX_FMT_VYUY:
> +		return buf_interlaced ? CSI_FRAME_RAW_8 : CSI_FIELD_RAW_8;
> +
> +	case V4L2_PIX_FMT_HM12:
> +		return buf_interlaced ? CSI_FRAME_MB_YUV420 :
> +					CSI_FIELD_MB_YUV420;
> +	case V4L2_PIX_FMT_NV12:
> +	case V4L2_PIX_FMT_NV21:
> +		return buf_interlaced ? CSI_FRAME_UV_CB_YUV420 :
> +					CSI_FIELD_UV_CB_YUV420;
> +	case V4L2_PIX_FMT_YUV420:
> +	case V4L2_PIX_FMT_YVU420:
> +		return buf_interlaced ? CSI_FRAME_PLANAR_YUV420 :
> +					CSI_FIELD_PLANAR_YUV420;
> +	case V4L2_PIX_FMT_NV16:
> +	case V4L2_PIX_FMT_NV61:
> +		return buf_interlaced ? CSI_FRAME_UV_CB_YUV422 :
> +					CSI_FIELD_UV_CB_YUV422;
> +	case V4L2_PIX_FMT_YUV422P:
> +		return buf_interlaced ? CSI_FRAME_PLANAR_YUV422 :
> +					CSI_FIELD_PLANAR_YUV422;
> +	}
> +
> +	return 0;
> +}
> +
> +static enum csi_input_seq get_csi_input_seq(u32 mbus_code, u32 pixformat)
> +{
> +
> +	switch (pixformat) {
> +	case V4L2_PIX_FMT_HM12:
> +	case V4L2_PIX_FMT_NV12:
> +	case V4L2_PIX_FMT_NV16:
> +	case V4L2_PIX_FMT_YUV420:
> +	case V4L2_PIX_FMT_YUV422P:
> +		switch (mbus_code) {
> +		case MEDIA_BUS_FMT_UYVY8_2X8:
> +		case MEDIA_BUS_FMT_UYVY8_1X16:
> +			return CSI_INPUT_SEQ_UYVY;
> +		case MEDIA_BUS_FMT_VYUY8_2X8:
> +		case MEDIA_BUS_FMT_VYUY8_1X16:
> +			return CSI_INPUT_SEQ_VYUY;
> +		case MEDIA_BUS_FMT_YUYV8_2X8:
> +		case MEDIA_BUS_FMT_YUYV8_1X16:
> +			return CSI_INPUT_SEQ_YUYV;
> +		case MEDIA_BUS_FMT_YVYU8_1X16:
> +		case MEDIA_BUS_FMT_YVYU8_2X8:
> +			return CSI_INPUT_SEQ_YVYU;
> +		}
> +		break;
> +	case V4L2_PIX_FMT_NV21:
> +	case V4L2_PIX_FMT_NV61:
> +	case V4L2_PIX_FMT_YVU420:
> +		switch (mbus_code) {
> +		case MEDIA_BUS_FMT_UYVY8_2X8:
> +		case MEDIA_BUS_FMT_UYVY8_1X16:
> +			return CSI_INPUT_SEQ_VYUY;
> +		case MEDIA_BUS_FMT_VYUY8_2X8:
> +		case MEDIA_BUS_FMT_VYUY8_1X16:
> +			return CSI_INPUT_SEQ_UYVY;
> +		case MEDIA_BUS_FMT_YUYV8_2X8:
> +		case MEDIA_BUS_FMT_YUYV8_1X16:
> +			return CSI_INPUT_SEQ_YVYU;
> +		case MEDIA_BUS_FMT_YVYU8_1X16:
> +		case MEDIA_BUS_FMT_YVYU8_2X8:
> +			return CSI_INPUT_SEQ_YUYV;
> +		}
> +		break;
> +	}
> +
> +	return CSI_INPUT_SEQ_YUYV;
> +}
> +
> +static void sun6i_csi_setup_bus(struct sun6i_csi_dev *sdev)
> +{
> +	struct v4l2_fwnode_endpoint *endpoint = &sdev->csi.v4l2_ep;
> +	unsigned char bus_width;
> +	u32 flags;
> +	u32 cfg;
> +
> +	bus_width = endpoint->bus.parallel.bus_width;
> +
> +	regmap_read(sdev->regmap, CSI_IF_CFG_REG, &cfg);
> +
> +	cfg &= ~(CSI_IF_CFG_CSI_IF_MASK | CSI_IF_CFG_MIPI_IF_MASK |
> +		 CSI_IF_CFG_IF_DATA_WIDTH_MASK |
> +		 CSI_IF_CFG_CLK_POL_MASK | CSI_IF_CFG_VREF_POL_MASK |
> +		 CSI_IF_CFG_HREF_POL_MASK | CSI_IF_CFG_FIELD_MASK);
> +
> +	switch (endpoint->bus_type) {
> +	case V4L2_MBUS_CSI2:
> +		cfg |= CSI_IF_CFG_MIPI_IF_MIPI;
> +		break;

You're not supporting CSI2, you can drop this code.

> +	case V4L2_MBUS_PARALLEL:
> +		cfg |= CSI_IF_CFG_MIPI_IF_CSI;
> +
> +		flags = endpoint->bus.parallel.flags;
> +
> +		cfg |= (bus_width == 16) ? CSI_IF_CFG_CSI_IF_YUV422_16BIT :
> +					   CSI_IF_CFG_CSI_IF_YUV422_INTLV;
> +
> +		if (flags & V4L2_MBUS_FIELD_EVEN_LOW)
> +			cfg |= CSI_IF_CFG_FIELD_POSITIVE;
> +
> +		if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
> +			cfg |= CSI_IF_CFG_VREF_POL_POSITIVE;
> +		if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> +			cfg |= CSI_IF_CFG_HREF_POL_POSITIVE;
> +
> +		if (flags & V4L2_MBUS_PCLK_SAMPLE_FALLING)
> +			cfg |= CSI_IF_CFG_CLK_POL_FALLING_EDGE;
> +		break;
> +	case V4L2_MBUS_BT656:
> +		cfg |= CSI_IF_CFG_MIPI_IF_CSI;
> +
> +		flags = endpoint->bus.parallel.flags;
> +
> +		cfg |= (bus_width == 16) ? CSI_IF_CFG_CSI_IF_BT1120 :
> +					   CSI_IF_CFG_CSI_IF_BT656;
> +
> +		if (flags & V4L2_MBUS_FIELD_EVEN_LOW)
> +			cfg |= CSI_IF_CFG_FIELD_POSITIVE;
> +
> +		if (flags & V4L2_MBUS_PCLK_SAMPLE_FALLING)
> +			cfg |= CSI_IF_CFG_CLK_POL_FALLING_EDGE;
> +		break;
> +	default:
> +		WARN_ON(1);
> +		break;
> +	}
> +
> +	switch (bus_width) {
> +	case 8:
> +		cfg |= CSI_IF_CFG_IF_DATA_WIDTH_8BIT;
> +		break;
> +	case 10:
> +		cfg |= CSI_IF_CFG_IF_DATA_WIDTH_10BIT;
> +		break;
> +	case 12:
> +		cfg |= CSI_IF_CFG_IF_DATA_WIDTH_12BIT;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	regmap_write(sdev->regmap, CSI_IF_CFG_REG, cfg);
> +}
> +
> +static void sun6i_csi_set_format(struct sun6i_csi_dev *sdev)
> +{
> +	struct sun6i_csi *csi = &sdev->csi;
> +	u32 cfg;
> +	u32 val;
> +
> +	regmap_read(sdev->regmap, CSI_CH_CFG_REG, &cfg);
> +
> +	cfg &= ~(CSI_CH_CFG_INPUT_FMT_MASK |
> +		 CSI_CH_CFG_OUTPUT_FMT_MASK | CSI_CH_CFG_VFLIP_EN |
> +		 CSI_CH_CFG_HFLIP_EN | CSI_CH_CFG_FIELD_SEL_MASK |
> +		 CSI_CH_CFG_INPUT_SEQ_MASK);
> +
> +	val = get_csi_input_format(csi->config.code, csi->config.pixelformat);
> +	cfg |= CSI_CH_CFG_INPUT_FMT(val);
> +
> +	val = get_csi_output_format(csi->config.pixelformat, csi->config.field);
> +	cfg |= CSI_CH_CFG_OUTPUT_FMT(val);
> +
> +	val = get_csi_input_seq(csi->config.code, csi->config.pixelformat);
> +	cfg |= CSI_CH_CFG_INPUT_SEQ(val);
> +
> +	if (csi->config.field == V4L2_FIELD_TOP)
> +		cfg |= CSI_CH_CFG_FIELD_SEL_FIELD0;
> +	else if (csi->config.field == V4L2_FIELD_BOTTOM)
> +		cfg |= CSI_CH_CFG_FIELD_SEL_FIELD1;
> +	else
> +		cfg |= CSI_CH_CFG_FIELD_SEL_BOTH;
> +
> +	regmap_write(sdev->regmap, CSI_CH_CFG_REG, cfg);
> +}
> +
> +static void sun6i_csi_set_window(struct sun6i_csi_dev *sdev)
> +{
> +	struct sun6i_csi_config *config = &sdev->csi.config;
> +	u32 bytesperline_y;
> +	u32 bytesperline_c;
> +	int *planar_offset = sdev->planar_offset;
> +	u32 width = config->width;
> +	u32 height = config->height;
> +	u32 hor_len = width;
> +
> +	switch (config->pixelformat) {
> +	case V4L2_PIX_FMT_YUYV:
> +	case V4L2_PIX_FMT_YVYU:
> +	case V4L2_PIX_FMT_UYVY:
> +	case V4L2_PIX_FMT_VYUY:
> +		hor_len = width * 2;
> +		break;
> +	}
> +
> +	regmap_write(sdev->regmap, CSI_CH_HSIZE_REG,
> +		     CSI_CH_HSIZE_HOR_LEN(hor_len) |
> +		     CSI_CH_HSIZE_HOR_START(0));
> +	regmap_write(sdev->regmap, CSI_CH_VSIZE_REG,
> +		     CSI_CH_VSIZE_VER_LEN(height) |
> +		     CSI_CH_VSIZE_VER_START(0));
> +
> +	planar_offset[0] = 0;
> +	switch (config->pixelformat) {
> +	case V4L2_PIX_FMT_HM12:
> +	case V4L2_PIX_FMT_NV12:
> +	case V4L2_PIX_FMT_NV21:
> +	case V4L2_PIX_FMT_NV16:
> +	case V4L2_PIX_FMT_NV61:
> +		bytesperline_y = width;
> +		bytesperline_c = width;
> +		planar_offset[1] = bytesperline_y * height;
> +		planar_offset[2] = -1;
> +		break;
> +	case V4L2_PIX_FMT_YUV420:
> +	case V4L2_PIX_FMT_YVU420:
> +		bytesperline_y = width;
> +		bytesperline_c = width / 2;
> +		planar_offset[1] = bytesperline_y * height;
> +		planar_offset[2] = planar_offset[1] +
> +				bytesperline_c * height / 2;
> +		break;
> +	case V4L2_PIX_FMT_YUV422P:
> +		bytesperline_y = width;
> +		bytesperline_c = width / 2;
> +		planar_offset[1] = bytesperline_y * height;
> +		planar_offset[2] = planar_offset[1] +
> +				bytesperline_c * height;
> +		break;
> +	default: /* raw */
> +		bytesperline_y = (v4l2_pixformat_get_bpp(config->pixelformat) *
> +				  config->width) / 8;
> +		bytesperline_c = 0;
> +		planar_offset[1] = -1;
> +		planar_offset[2] = -1;
> +		break;
> +	}
> +
> +	regmap_write(sdev->regmap, CSI_CH_BUF_LEN_REG,
> +		     CSI_CH_BUF_LEN_BUF_LEN_C(bytesperline_c) |
> +		     CSI_CH_BUF_LEN_BUF_LEN_Y(bytesperline_y));
> +}
> +
> +int sun6i_csi_get_supported_pixformats(struct sun6i_csi *csi,
> +				    const u32 **pixformats)
> +{
> +	if (pixformats != NULL)
> +		*pixformats = supported_pixformats;
> +
> +	return ARRAY_SIZE(supported_pixformats);
> +}
> +
> +bool sun6i_csi_is_format_support(struct sun6i_csi *csi, u32 pixformat,
> +			      u32 mbus_code)
> +{
> +	struct sun6i_csi_dev *sdev = sun6i_csi_to_dev(csi);
> +
> +	return __is_format_support(sdev, pixformat, mbus_code);
> +}
> +
> +int sun6i_csi_set_power(struct sun6i_csi *csi, bool enable)
> +{
> +	struct sun6i_csi_dev *sdev = sun6i_csi_to_dev(csi);
> +	struct regmap *regmap = sdev->regmap;
> +	int ret;
> +
> +	if (!enable) {
> +		regmap_update_bits(regmap, CSI_EN_REG, CSI_EN_CSI_EN, 0);
> +
> +		clk_disable_unprepare(sdev->clk_ram);
> +		clk_disable_unprepare(sdev->clk_mod);
> +		reset_control_assert(sdev->rstc_bus);
> +		return 0;
> +	}
> +
> +	ret = clk_prepare_enable(sdev->clk_mod);
> +	if (ret) {
> +		dev_err(sdev->dev, "Enable csi clk err %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(sdev->clk_ram);
> +	if (ret) {
> +		dev_err(sdev->dev, "Enable clk_dram_csi clk err %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = reset_control_deassert(sdev->rstc_bus);
> +	if (ret) {
> +		dev_err(sdev->dev, "reset err %d\n", ret);
> +		return ret;
> +	}
> +
> +	regmap_update_bits(regmap, CSI_EN_REG, CSI_EN_CSI_EN, CSI_EN_CSI_EN);
> +
> +	return 0;
> +}
> +
> +int sun6i_csi_update_config(struct sun6i_csi *csi,
> +			 struct sun6i_csi_config *config)
> +{
> +	struct sun6i_csi_dev *sdev = sun6i_csi_to_dev(csi);
> +
> +	if (config == NULL)
> +		return -EINVAL;
> +
> +	memcpy(&csi->config, config, sizeof(csi->config));
> +
> +	sun6i_csi_setup_bus(sdev);
> +	sun6i_csi_set_format(sdev);
> +	sun6i_csi_set_window(sdev);
> +
> +	return 0;
> +}
> +
> +int sun6i_csi_update_buf_addr(struct sun6i_csi *csi, dma_addr_t addr)
> +{
> +	struct sun6i_csi_dev *sdev = sun6i_csi_to_dev(csi);
> +	/* transform physical address to bus address */
> +	dma_addr_t bus_addr = addr - PHYS_OFFSET;
> +
> +	regmap_write(sdev->regmap, CSI_CH_F0_BUFA_REG,
> +		     (bus_addr + sdev->planar_offset[0]) >> 2);
> +	if (sdev->planar_offset[1] != -1)
> +		regmap_write(sdev->regmap, CSI_CH_F1_BUFA_REG,
> +			     (bus_addr + sdev->planar_offset[1]) >> 2);
> +	if (sdev->planar_offset[2] != -1)
> +		regmap_write(sdev->regmap, CSI_CH_F2_BUFA_REG,
> +			     (bus_addr + sdev->planar_offset[2]) >> 2);
> +
> +	return 0;
> +}
> +
> +int sun6i_csi_set_stream(struct sun6i_csi *csi, bool enable)
> +{
> +	struct sun6i_csi_dev *sdev = sun6i_csi_to_dev(csi);
> +	struct regmap *regmap = sdev->regmap;
> +
> +	if (!enable) {
> +		regmap_update_bits(regmap, CSI_CAP_REG, CSI_CAP_CH0_VCAP_ON, 0);
> +		regmap_write(regmap, CSI_CH_INT_EN_REG, 0);
> +		return 0;
> +	}
> +
> +	regmap_write(regmap, CSI_CH_INT_STA_REG, 0xFF);
> +	regmap_write(regmap, CSI_CH_INT_EN_REG,
> +		     CSI_CH_INT_EN_HB_OF_INT_EN |
> +		     CSI_CH_INT_EN_FIFO2_OF_INT_EN |
> +		     CSI_CH_INT_EN_FIFO1_OF_INT_EN |
> +		     CSI_CH_INT_EN_FIFO0_OF_INT_EN |
> +		     CSI_CH_INT_EN_FD_INT_EN |
> +		     CSI_CH_INT_EN_CD_INT_EN);
> +
> +	regmap_update_bits(regmap, CSI_CAP_REG, CSI_CAP_CH0_VCAP_ON,
> +			   CSI_CAP_CH0_VCAP_ON);
> +
> +	return 0;
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Media Controller and V4L2
> + */
> +static int sun6i_csi_link_entity(struct sun6i_csi *csi,
> +				 struct media_entity *entity)
> +{
> +	struct media_entity *sink;
> +	struct media_pad *sink_pad;
> +	int ret;
> +	int i;
> +
> +	if (!entity->num_pads) {
> +		dev_err(csi->dev, "%s: invalid entity\n", entity->name);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i < entity->num_pads; i++) {
> +		if (entity->pads[i].flags & MEDIA_PAD_FL_SOURCE)
> +			break;
> +	}
> +
> +	if (i == entity->num_pads) {
> +		dev_err(csi->dev, "%s: no source pad in external entity %s\n",
> +			__func__, entity->name);
> +		return -EINVAL;
> +	}
> +
> +	sink = &csi->video.vdev.entity;
> +	sink_pad = &csi->video.pad;
> +
> +	dev_dbg(csi->dev, "creating %s:%u -> %s:%u link\n",
> +		entity->name, i, sink->name, sink_pad->index);
> +	ret = media_create_pad_link(entity, i, sink, sink_pad->index,
> +				    MEDIA_LNK_FL_ENABLED);
> +	if (ret < 0) {
> +		dev_err(csi->dev, "failed to create %s:%u -> %s:%u link\n",
> +			entity->name, i, sink->name, sink_pad->index);
> +		return ret;
> +	}
> +
> +	return media_entity_call(sink, link_setup, sink_pad, &entity->pads[i],
> +				 MEDIA_LNK_FL_ENABLED);
> +}
> +
> +static int sun6i_subdev_notify_complete(struct v4l2_async_notifier *notifier)
> +{
> +	struct sun6i_csi *csi = container_of(notifier, struct sun6i_csi,
> +					     notifier);
> +	struct v4l2_device *v4l2_dev = &csi->v4l2_dev;
> +	struct v4l2_subdev *sd;
> +	int ret;
> +
> +	dev_dbg(csi->dev, "notify complete, all subdevs registered\n");
> +
> +	if (notifier->num_subdevs != 1)
> +		return -EINVAL;
> +
> +	sd = list_first_entry(&v4l2_dev->subdevs, struct v4l2_subdev, list);
> +	if (sd == NULL)
> +		return -EINVAL;
> +
> +	ret = sun6i_csi_link_entity(csi, &sd->entity);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = v4l2_device_register_subdev_nodes(&csi->v4l2_dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	return media_device_register(&csi->media_dev);
> +}
> +
> +static const struct v4l2_async_notifier_operations sun6i_csi_async_ops = {
> +	.complete = sun6i_subdev_notify_complete,
> +};
> +
> +static int sun6i_csi_fwnode_parse(struct device *dev,
> +				   struct v4l2_fwnode_endpoint *vep,
> +				   struct v4l2_async_subdev *asd)
> +{
> +	struct sun6i_csi *csi = dev_get_drvdata(dev);
> +
> +	if (vep->base.port || vep->base.id) {
> +		dev_warn(dev, "Only support a single port with one endpoint\n");
> +		return -ENOTCONN;
> +	}
> +
> +	switch (vep->bus_type) {
> +	case V4L2_MBUS_PARALLEL:
> +	case V4L2_MBUS_BT656:
> +		csi->v4l2_ep = *vep;
> +		return 0;
> +	default:
> +		dev_err(dev, "Unsupported media bus type\n");
> +		return -ENOTCONN;
> +	}
> +}
> +
> +static void sun6i_csi_v4l2_cleanup(struct sun6i_csi *csi)
> +{
> +	v4l2_async_notifier_cleanup(&csi->notifier);
> +	v4l2_async_notifier_unregister(&csi->notifier);
> +	sun6i_video_cleanup(&csi->video);
> +	v4l2_device_unregister(&csi->v4l2_dev);
> +	media_device_unregister(&csi->media_dev);
> +	media_device_cleanup(&csi->media_dev);
> +}
> +
> +static int sun6i_csi_v4l2_init(struct sun6i_csi *csi)
> +{
> +	int ret;
> +
> +	csi->media_dev.dev = csi->dev;
> +	strlcpy(csi->media_dev.model, "Allwinner Video Capture Device",
> +		sizeof(csi->media_dev.model));
> +	csi->media_dev.hw_revision = 0;
> +
> +	media_device_init(&csi->media_dev);
> +
> +	csi->v4l2_dev.mdev = &csi->media_dev;
> +	ret = v4l2_device_register(csi->dev, &csi->v4l2_dev);
> +	if (ret) {
> +		dev_err(csi->dev, "V4L2 device registration failed (%d)\n",
> +			ret);
> +		goto v4l2_reg_err;
> +	}
> +
> +	ret = sun6i_video_init(&csi->video, csi, "sun6i-csi");
> +	if (ret)
> +		goto video_init_err;
> +
> +	ret = v4l2_async_notifier_parse_fwnode_endpoints(
> +		csi->dev, &csi->notifier, sizeof(struct v4l2_async_subdev),
> +		sun6i_csi_fwnode_parse);
> +	if (ret)
> +		goto fwnode_parse_err;
> +
> +	csi->notifier.ops = &sun6i_csi_async_ops;
> +
> +	ret = v4l2_async_notifier_register(&csi->v4l2_dev, &csi->notifier);
> +	if (ret) {
> +		dev_err(csi->dev, "notifier registration failed\n");
> +		goto notifier_reg_err;
> +	}
> +
> +	return 0;
> +
> +notifier_reg_err:
> +	v4l2_async_notifier_cleanup(&csi->notifier);
> +fwnode_parse_err:
> +	sun6i_video_cleanup(&csi->video);
> +video_init_err:
> +	v4l2_device_unregister(&csi->v4l2_dev);
> +v4l2_reg_err:
> +	media_device_cleanup(&csi->media_dev);
> +
> +	return ret;
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Resources and IRQ
> + */
> +static irqreturn_t sun6i_csi_isr(int irq, void *dev_id)
> +{
> +	struct sun6i_csi_dev *sdev = (struct sun6i_csi_dev *)dev_id;
> +	struct regmap *regmap = sdev->regmap;
> +	u32 status;
> +
> +	regmap_read(regmap, CSI_CH_INT_STA_REG, &status);
> +
> +	if (!(status & 0xFF))
> +		return IRQ_NONE;
> +
> +	if ((status & CSI_CH_INT_STA_FIFO0_OF_PD) ||
> +	    (status & CSI_CH_INT_STA_FIFO1_OF_PD) ||
> +	    (status & CSI_CH_INT_STA_FIFO2_OF_PD) ||
> +	    (status & CSI_CH_INT_STA_HB_OF_PD)) {
> +		regmap_write(regmap, CSI_CH_INT_STA_REG, status);
> +		regmap_update_bits(regmap, CSI_EN_REG, CSI_EN_CSI_EN, 0);
> +		regmap_update_bits(regmap, CSI_EN_REG, CSI_EN_CSI_EN,
> +				   CSI_EN_CSI_EN);
> +		return IRQ_HANDLED;
> +	}
> +
> +	if (status & CSI_CH_INT_STA_FD_PD)
> +		sun6i_video_frame_done(&sdev->csi.video);
> +
> +	regmap_write(regmap, CSI_CH_INT_STA_REG, status);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static const struct regmap_config sun6i_csi_regmap_config = {
> +	.reg_bits       = 32,
> +	.reg_stride     = 4,
> +	.val_bits       = 32,
> +	.max_register	= 0x1000,
> +};
> +
> +static int sun6i_csi_resource_request(struct sun6i_csi_dev *sdev,
> +				      struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	void __iomem *io_base;
> +	int ret;
> +	int irq;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	io_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(io_base))
> +		return PTR_ERR(io_base);
> +
> +	sdev->regmap = devm_regmap_init_mmio_clk(&pdev->dev, "bus", io_base,
> +					    &sun6i_csi_regmap_config);
> +	if (IS_ERR(sdev->regmap)) {
> +		dev_err(&pdev->dev, "Failed to init register map\n");
> +		return PTR_ERR(sdev->regmap);
> +	}
> +
> +	sdev->clk_mod = devm_clk_get(&pdev->dev, "mod");
> +	if (IS_ERR(sdev->clk_mod)) {
> +		dev_err(&pdev->dev, "Unable to acquire csi clock\n");
> +		return PTR_ERR(sdev->clk_mod);
> +	}
> +
> +	sdev->clk_ram = devm_clk_get(&pdev->dev, "ram");
> +	if (IS_ERR(sdev->clk_ram)) {
> +		dev_err(&pdev->dev, "Unable to acquire dram-csi clock\n");
> +		return PTR_ERR(sdev->clk_ram);
> +	}
> +
> +	sdev->rstc_bus = devm_reset_control_get_shared(&pdev->dev, NULL);
> +	if (IS_ERR(sdev->rstc_bus)) {
> +		dev_err(&pdev->dev, "Cannot get reset controller\n");
> +		return PTR_ERR(sdev->rstc_bus);
> +	}
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0) {
> +		dev_err(&pdev->dev, "No csi IRQ specified\n");
> +		ret = -ENXIO;
> +		return ret;
> +	}
> +
> +	ret = devm_request_irq(&pdev->dev, irq, sun6i_csi_isr, 0, MODULE_NAME,
> +			       sdev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Cannot request csi IRQ\n");
> +		return ret;
> +	}
> +	return 0;
> +}
> +
> +static int sun6i_csi_probe(struct platform_device *pdev)
> +{
> +	struct sun6i_csi_dev *sdev;
> +	int ret;
> +
> +	sdev = devm_kzalloc(&pdev->dev, sizeof(*sdev), GFP_KERNEL);
> +	if (!sdev)
> +		return -ENOMEM;
> +
> +	sdev->dev = &pdev->dev;
> +
> +	ret = sun6i_csi_resource_request(sdev, pdev);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, sdev);
> +
> +	sdev->csi.dev = &pdev->dev;
> +	ret = sun6i_csi_v4l2_init(&sdev->csi);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int sun6i_csi_remove(struct platform_device *pdev)
> +{
> +	struct sun6i_csi_dev *sdev = platform_get_drvdata(pdev);
> +
> +	sun6i_csi_v4l2_cleanup(&sdev->csi);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id sun6i_csi_of_match[] = {
> +	{ .compatible = "allwinner,sun8i-v3s-csi", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, sun6i_csi_of_match);
> +
> +static struct platform_driver sun6i_csi_platform_driver = {
> +	.probe = sun6i_csi_probe,
> +	.remove = sun6i_csi_remove,
> +	.driver = {
> +		.name = MODULE_NAME,
> +		.of_match_table = of_match_ptr(sun6i_csi_of_match),
> +	},
> +};
> +module_platform_driver(sun6i_csi_platform_driver);
> +
> +MODULE_DESCRIPTION("Allwinner V3s Camera Sensor Interface driver");
> +MODULE_AUTHOR("Yong Deng <yong.deng-+3dxTMOEIRNWk0Htik3J/w@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
> new file mode 100644
> index 0000000..12508ff
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
> @@ -0,0 +1,146 @@
> +/*
> + * Copyright (c) 2017 Yong Deng <yong.deng-+3dxTMOEIRNWk0Htik3J/w@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __SUN6I_CSI_H__
> +#define __SUN6I_CSI_H__
> +
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> +
> +#include "sun6i_video.h"
> +
> +struct sun6i_csi;
> +
> +/**
> + * struct sun6i_csi_config - configs for sun6i csi
> + * @pixelformat: v4l2 pixel format (V4L2_PIX_FMT_*)
> + * @code:	media bus format code (MEDIA_BUS_FMT_*)
> + * @field:	used interlacing type (enum v4l2_field)
> + * @width:	frame width
> + * @height:	frame height
> + */
> +struct sun6i_csi_config {
> +	u32		pixelformat;
> +	u32		code;
> +	u32		field;
> +	u32		width;
> +	u32		height;
> +};
> +
> +struct sun6i_csi {
> +	struct device			*dev;
> +	struct v4l2_device		v4l2_dev;
> +	struct media_device		media_dev;
> +
> +	struct v4l2_async_notifier	notifier;
> +
> +	/* video port settings */
> +	struct v4l2_fwnode_endpoint	v4l2_ep;
> +
> +	struct sun6i_csi_config		config;
> +
> +	struct sun6i_video		video;
> +};
> +
> +/**
> + * sun6i_csi_get_supported_pixformats() - get csi supported pixformats
> + * @csi:	pointer to the csi
> + * @pixformats:	supported pixformats return from csi
> + *
> + * @return the count of pixformats or error(< 0)
> + */
> +int sun6i_csi_get_supported_pixformats(struct sun6i_csi *csi,
> +				       const u32 **pixformats);
> +
> +/**
> + * sun6i_csi_is_format_support() - check if the format supported by csi
> + * @csi:	pointer to the csi
> + * @pixformat:	v4l2 pixel format (V4L2_PIX_FMT_*)
> + * @mbus_code:	media bus format code (MEDIA_BUS_FMT_*)
> + */
> +bool sun6i_csi_is_format_support(struct sun6i_csi *csi, u32 pixformat,
> +				 u32 mbus_code);
> +
> +/**
> + * sun6i_csi_set_power() - power on/off the csi
> + * @csi:	pointer to the csi
> + * @enable:	on/off
> + */
> +int sun6i_csi_set_power(struct sun6i_csi *csi, bool enable);
> +
> +/**
> + * sun6i_csi_update_config() - update the csi register setttings
> + * @csi:	pointer to the csi
> + * @config:	see struct sun6i_csi_config
> + */
> +int sun6i_csi_update_config(struct sun6i_csi *csi,
> +			    struct sun6i_csi_config *config);
> +
> +/**
> + * sun6i_csi_update_buf_addr() - update the csi frame buffer address
> + * @csi:	pointer to the csi
> + * @addr:	frame buffer's physical address
> + */
> +int sun6i_csi_update_buf_addr(struct sun6i_csi *csi, dma_addr_t addr);
> +
> +/**
> + * sun6i_csi_set_stream() - start/stop csi streaming
> + * @csi:	pointer to the csi
> + * @enable:	start/stop
> + */
> +int sun6i_csi_set_stream(struct sun6i_csi *csi, bool enable);
> +
> +static inline int v4l2_pixformat_get_bpp(unsigned int pixformat)

Probably not a good name. v4l2_ prefix is for V4L2 core API functions.

Rename it or you can move this into core, if you think that getting
bits per pixel for pixformats can help other drivers too.

> +{
> +	switch (pixformat) {
> +	case V4L2_PIX_FMT_SBGGR8:
> +	case V4L2_PIX_FMT_SGBRG8:
> +	case V4L2_PIX_FMT_SGRBG8:
> +	case V4L2_PIX_FMT_SRGGB8:
> +		return 8;
> +	case V4L2_PIX_FMT_SBGGR10:
> +	case V4L2_PIX_FMT_SGBRG10:
> +	case V4L2_PIX_FMT_SGRBG10:
> +	case V4L2_PIX_FMT_SRGGB10:
> +		return 10;
> +	case V4L2_PIX_FMT_SBGGR12:
> +	case V4L2_PIX_FMT_SGBRG12:
> +	case V4L2_PIX_FMT_SGRBG12:
> +	case V4L2_PIX_FMT_SRGGB12:
> +	case V4L2_PIX_FMT_HM12:
> +	case V4L2_PIX_FMT_NV12:
> +	case V4L2_PIX_FMT_NV21:
> +	case V4L2_PIX_FMT_YUV420:
> +	case V4L2_PIX_FMT_YVU420:
> +		return 12;
> +	case V4L2_PIX_FMT_YUYV:
> +	case V4L2_PIX_FMT_YVYU:
> +	case V4L2_PIX_FMT_UYVY:
> +	case V4L2_PIX_FMT_VYUY:
> +	case V4L2_PIX_FMT_NV16:
> +	case V4L2_PIX_FMT_NV61:
> +	case V4L2_PIX_FMT_YUV422P:
> +		return 16;
> +	case V4L2_PIX_FMT_RGB24:
> +	case V4L2_PIX_FMT_BGR24:
> +		return 24;
> +	case V4L2_PIX_FMT_RGB32:
> +	case V4L2_PIX_FMT_BGR32:
> +		return 32;
> +	}
> +
> +	return 0;
> +}
> +
> +#endif /* __SUN6I_CSI_H__ */
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h
> new file mode 100644
> index 0000000..8e80467
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h
> @@ -0,0 +1,203 @@
> +/*
> + * Copyright (c) 2017 Yong Deng <yong.deng-+3dxTMOEIRNWk0Htik3J/w@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __SUN6I_CSI_V3S_H__
> +#define __SUN6I_CSI_V3S_H__

No longer V3S. __SUN6I_CSI_REG_H__

> +#include <linux/kernel.h>
> +
> +#define CSI_EN_REG			0x0
> +#define CSI_EN_VER_EN				BIT(30)
> +#define CSI_EN_CSI_EN				BIT(0)
> +
> +#define CSI_IF_CFG_REG			0x4
> +#define CSI_IF_CFG_SRC_TYPE_MASK		BIT(21)
> +#define CSI_IF_CFG_SRC_TYPE_PROGRESSED		((0 << 21) & CSI_IF_CFG_SRC_TYPE_MASK)
> +#define CSI_IF_CFG_SRC_TYPE_INTERLACED		((1 << 21) & CSI_IF_CFG_SRC_TYPE_MASK)
> +#define CSI_IF_CFG_FPS_DS_EN			BIT(20)
> +#define CSI_IF_CFG_FIELD_MASK			BIT(19)
> +#define CSI_IF_CFG_FIELD_NEGATIVE		((0 << 19) & CSI_IF_CFG_FIELD_MASK)
> +#define CSI_IF_CFG_FIELD_POSITIVE		((1 << 19) & CSI_IF_CFG_FIELD_MASK)
> +#define CSI_IF_CFG_VREF_POL_MASK		BIT(18)
> +#define CSI_IF_CFG_VREF_POL_NEGATIVE		((0 << 18) & CSI_IF_CFG_VREF_POL_MASK)
> +#define CSI_IF_CFG_VREF_POL_POSITIVE		((1 << 18) & CSI_IF_CFG_VREF_POL_MASK)
> +#define CSI_IF_CFG_HREF_POL_MASK		BIT(17)
> +#define CSI_IF_CFG_HREF_POL_NEGATIVE		((0 << 17) & CSI_IF_CFG_HREF_POL_MASK)
> +#define CSI_IF_CFG_HREF_POL_POSITIVE		((1 << 17) & CSI_IF_CFG_HREF_POL_MASK)
> +#define CSI_IF_CFG_CLK_POL_MASK			BIT(16)
> +#define CSI_IF_CFG_CLK_POL_RISING_EDGE		((0 << 16) & CSI_IF_CFG_CLK_POL_MASK)
> +#define CSI_IF_CFG_CLK_POL_FALLING_EDGE		((1 << 16) & CSI_IF_CFG_CLK_POL_MASK)
> +#define CSI_IF_CFG_IF_DATA_WIDTH_MASK		GENMASK(10, 8)
> +#define CSI_IF_CFG_IF_DATA_WIDTH_8BIT		((0 << 8) & CSI_IF_CFG_IF_DATA_WIDTH_MASK)
> +#define CSI_IF_CFG_IF_DATA_WIDTH_10BIT		((1 << 8) & CSI_IF_CFG_IF_DATA_WIDTH_MASK)
> +#define CSI_IF_CFG_IF_DATA_WIDTH_12BIT		((2 << 8) & CSI_IF_CFG_IF_DATA_WIDTH_MASK)
> +#define CSI_IF_CFG_MIPI_IF_MASK			BIT(7)
> +#define CSI_IF_CFG_MIPI_IF_CSI			(0 << 7)
> +#define CSI_IF_CFG_MIPI_IF_MIPI			(1 << 7)
> +#define CSI_IF_CFG_CSI_IF_MASK			GENMASK(4, 0)
> +#define CSI_IF_CFG_CSI_IF_YUV422_INTLV		((0 << 0) & CSI_IF_CFG_CSI_IF_MASK)
> +#define CSI_IF_CFG_CSI_IF_YUV422_16BIT		((1 << 0) & CSI_IF_CFG_CSI_IF_MASK)
> +#define CSI_IF_CFG_CSI_IF_BT656			((4 << 0) & CSI_IF_CFG_CSI_IF_MASK)
> +#define CSI_IF_CFG_CSI_IF_BT1120		((5 << 0) & CSI_IF_CFG_CSI_IF_MASK)
> +
> +#define CSI_CAP_REG			0x8
> +#define CSI_CAP_CH0_CAP_MASK_MASK		GENMASK(5, 2)
> +#define CSI_CAP_CH0_CAP_MASK(count)		((count << 2) & CSI_CAP_CH0_CAP_MASK_MASK)
> +#define CSI_CAP_CH0_VCAP_ON			BIT(1)
> +#define CSI_CAP_CH0_SCAP_ON			BIT(0)
> +
> +#define CSI_SYNC_CNT_REG		0xc
> +#define CSI_FIFO_THRS_REG		0x10
> +#define CSI_BT656_HEAD_CFG_REG		0x14
> +#define CSI_PTN_LEN_REG			0x30
> +#define CSI_PTN_ADDR_REG		0x34
> +#define CSI_VER_REG			0x3c
> +
> +#define CSI_CH_CFG_REG			0x44
> +#define CSI_CH_CFG_INPUT_FMT_MASK		GENMASK(23, 20)
> +#define CSI_CH_CFG_INPUT_FMT(fmt)		((fmt << 20) & CSI_CH_CFG_INPUT_FMT_MASK)
> +#define CSI_CH_CFG_OUTPUT_FMT_MASK		GENMASK(19, 16)
> +#define CSI_CH_CFG_OUTPUT_FMT(fmt)		((fmt << 16) & CSI_CH_CFG_OUTPUT_FMT_MASK)
> +#define CSI_CH_CFG_VFLIP_EN			BIT(13)
> +#define CSI_CH_CFG_HFLIP_EN			BIT(12)
> +#define CSI_CH_CFG_FIELD_SEL_MASK		GENMASK(11, 10)
> +#define CSI_CH_CFG_FIELD_SEL_FIELD0		((0 << 10) & CSI_CH_CFG_FIELD_SEL_MASK)
> +#define CSI_CH_CFG_FIELD_SEL_FIELD1		((1 << 10) & CSI_CH_CFG_FIELD_SEL_MASK)
> +#define CSI_CH_CFG_FIELD_SEL_BOTH		((2 << 10) & CSI_CH_CFG_FIELD_SEL_MASK)
> +#define CSI_CH_CFG_INPUT_SEQ_MASK		GENMASK(9, 8)
> +#define CSI_CH_CFG_INPUT_SEQ(seq)		((seq << 8) & CSI_CH_CFG_INPUT_SEQ_MASK)
> +
> +#define CSI_CH_SCALE_REG		0x4c
> +#define CSI_CH_SCALE_QUART_EN			BIT(0)
> +
> +#define CSI_CH_F0_BUFA_REG		0x50
> +
> +#define CSI_CH_F1_BUFA_REG		0x58
> +
> +#define CSI_CH_F2_BUFA_REG		0x60
> +
> +#define CSI_CH_STA_REG			0x6c
> +#define CSI_CH_STA_FIELD_STA_MASK		BIT(2)
> +#define CSI_CH_STA_FIELD_STA_FIELD0		((0 << 2) & CSI_CH_STA_FIELD_STA_MASK)
> +#define CSI_CH_STA_FIELD_STA_FIELD1		((1 << 2) & CSI_CH_STA_FIELD_STA_MASK)
> +#define CSI_CH_STA_VCAP_STA			BIT(1)
> +#define CSI_CH_STA_SCAP_STA			BIT(0)
> +
> +#define CSI_CH_INT_EN_REG		0x70
> +#define CSI_CH_INT_EN_VS_INT_EN			BIT(7)
> +#define CSI_CH_INT_EN_HB_OF_INT_EN		BIT(6)
> +#define CSI_CH_INT_EN_MUL_ERR_INT_EN		BIT(5)
> +#define CSI_CH_INT_EN_FIFO2_OF_INT_EN		BIT(4)
> +#define CSI_CH_INT_EN_FIFO1_OF_INT_EN		BIT(3)
> +#define CSI_CH_INT_EN_FIFO0_OF_INT_EN		BIT(2)
> +#define CSI_CH_INT_EN_FD_INT_EN			BIT(1)
> +#define CSI_CH_INT_EN_CD_INT_EN			BIT(0)
> +
> +#define CSI_CH_INT_STA_REG		0x74
> +#define CSI_CH_INT_STA_VS_PD			BIT(7)
> +#define CSI_CH_INT_STA_HB_OF_PD			BIT(6)
> +#define CSI_CH_INT_STA_MUL_ERR_PD		BIT(5)
> +#define CSI_CH_INT_STA_FIFO2_OF_PD		BIT(4)
> +#define CSI_CH_INT_STA_FIFO1_OF_PD		BIT(3)
> +#define CSI_CH_INT_STA_FIFO0_OF_PD		BIT(2)
> +#define CSI_CH_INT_STA_FD_PD			BIT(1)
> +#define CSI_CH_INT_STA_CD_PD			BIT(0)
> +
> +#define CSI_CH_FLD1_VSIZE_REG		0x78
> +
> +#define CSI_CH_HSIZE_REG		0x80
> +#define CSI_CH_HSIZE_HOR_LEN_MASK		GENMASK(28, 16)
> +#define CSI_CH_HSIZE_HOR_LEN(len)		((len << 16) & CSI_CH_HSIZE_HOR_LEN_MASK)
> +#define CSI_CH_HSIZE_HOR_START_MASK		GENMASK(12, 0)
> +#define CSI_CH_HSIZE_HOR_START(start)		((start << 0) & CSI_CH_HSIZE_HOR_START_MASK)
> +
> +#define CSI_CH_VSIZE_REG		0x84
> +#define CSI_CH_VSIZE_VER_LEN_MASK		GENMASK(28, 16)
> +#define CSI_CH_VSIZE_VER_LEN(len)		((len << 16) & CSI_CH_VSIZE_VER_LEN_MASK)
> +#define CSI_CH_VSIZE_VER_START_MASK		GENMASK(12, 0)
> +#define CSI_CH_VSIZE_VER_START(start)		((start << 0) & CSI_CH_VSIZE_VER_START_MASK)
> +
> +#define CSI_CH_BUF_LEN_REG		0x88
> +#define CSI_CH_BUF_LEN_BUF_LEN_C_MASK		GENMASK(29, 16)
> +#define CSI_CH_BUF_LEN_BUF_LEN_C(len)		((len << 16) & CSI_CH_BUF_LEN_BUF_LEN_C_MASK)
> +#define CSI_CH_BUF_LEN_BUF_LEN_Y_MASK		GENMASK(13, 0)
> +#define CSI_CH_BUF_LEN_BUF_LEN_Y(len)		((len << 0) & CSI_CH_BUF_LEN_BUF_LEN_Y_MASK)
> +
> +#define CSI_CH_FLIP_SIZE_REG		0x8c
> +#define CSI_CH_FLIP_SIZE_VER_LEN_MASK		GENMASK(28, 16)
> +#define CSI_CH_FLIP_SIZE_VER_LEN(len)		((len << 16) & CSI_CH_FLIP_SIZE_VER_LEN_MASK)
> +#define CSI_CH_FLIP_SIZE_VALID_LEN_MASK		GENMASK(12, 0)
> +#define CSI_CH_FLIP_SIZE_VALID_LEN(len)		((len << 0) & CSI_CH_FLIP_SIZE_VALID_LEN_MASK)
> +
> +#define CSI_CH_FRM_CLK_CNT_REG		0x90
> +#define CSI_CH_ACC_ITNL_CLK_CNT_REG	0x94
> +#define CSI_CH_FIFO_STAT_REG		0x98
> +#define CSI_CH_PCLK_STAT_REG		0x9c
> +
> +/*
> + * csi input data format
> + */
> +enum csi_input_fmt {
> +	CSI_INPUT_FORMAT_RAW		= 0,
> +	CSI_INPUT_FORMAT_YUV422		= 3,
> +	CSI_INPUT_FORMAT_YUV420		= 4,
> +};
> +
> +/*
> + * csi output data format
> + */
> +enum csi_output_fmt {
> +	/* only when input format is RAW */
> +	CSI_FIELD_RAW_8			= 0,
> +	CSI_FIELD_RAW_10		= 1,
> +	CSI_FIELD_RAW_12		= 2,
> +	CSI_FIELD_RGB565		= 4,
> +	CSI_FIELD_RGB888		= 5,
> +	CSI_FIELD_PRGB888		= 6,
> +	CSI_FRAME_RAW_8			= 8,
> +	CSI_FRAME_RAW_10		= 9,
> +	CSI_FRAME_RAW_12		= 10,
> +	CSI_FRAME_RGB565		= 12,
> +	CSI_FRAME_RGB888		= 13,
> +	CSI_FRAME_PRGB888		= 14,
> +
> +	/* only when input format is YUV422 */
> +	CSI_FIELD_PLANAR_YUV422		= 0,
> +	CSI_FIELD_PLANAR_YUV420		= 1,
> +	CSI_FRAME_PLANAR_YUV420		= 2,
> +	CSI_FRAME_PLANAR_YUV422		= 3,
> +	CSI_FIELD_UV_CB_YUV422		= 4,
> +	CSI_FIELD_UV_CB_YUV420		= 5,
> +	CSI_FRAME_UV_CB_YUV420		= 6,
> +	CSI_FRAME_UV_CB_YUV422		= 7,
> +	CSI_FIELD_MB_YUV422		= 8,
> +	CSI_FIELD_MB_YUV420		= 9,
> +	CSI_FRAME_MB_YUV420		= 10,
> +	CSI_FRAME_MB_YUV422		= 11,
> +	CSI_FIELD_UV_CB_YUV422_10	= 12,
> +	CSI_FIELD_UV_CB_YUV420_10	= 13,
> +};
> +
> +/*
> + * csi YUV input data sequence
> + */
> +enum csi_input_seq {
> +	/* only when input format is YUV422 */
> +	CSI_INPUT_SEQ_YUYV = 0,
> +	CSI_INPUT_SEQ_YVYU,
> +	CSI_INPUT_SEQ_UYVY,
> +	CSI_INPUT_SEQ_VYUY,
> +};
> +
> +#endif /* __SUN6I_CSI_V3S_H__ */

Name. See above.

> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
> new file mode 100644
> index 0000000..0cebcbd
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
> @@ -0,0 +1,722 @@
> +/*
> + * Copyright (c) 2017 Magewell Electronics Co., Ltd. (Nanjing).
> + * All rights reserved.
> + * Author: Yong Deng <yong.deng-+3dxTMOEIRNWk0Htik3J/w@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/of.h>
> +
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/v4l2-mc.h>
> +#include <media/videobuf2-dma-contig.h>
> +#include <media/videobuf2-v4l2.h>
> +
> +#include "sun6i_csi.h"
> +#include "sun6i_video.h"
> +
> +struct sun6i_csi_buffer {
> +	struct vb2_v4l2_buffer		vb;
> +	struct list_head		list;
> +
> +	dma_addr_t			dma_addr;
> +};
> +
> +static struct sun6i_csi_format *
> +find_format_by_fourcc(struct sun6i_video *video, unsigned int fourcc)
> +{
> +	unsigned int num_formats = video->num_formats;
> +	struct sun6i_csi_format *fmt;
> +	unsigned int i;
> +
> +	for (i = 0; i < num_formats; i++) {
> +		fmt = &video->formats[i];
> +		if (fmt->fourcc == fourcc)
> +			return fmt;
> +	}
> +
> +	return NULL;
> +}

Perhaps keep the naming consistent and use pixformat everywhere,
instead of fourcc once and pixformat elsewhere?

> +static struct v4l2_subdev *
> +sun6i_video_remote_subdev(struct sun6i_video *video, u32 *pad)
> +{
> +	struct media_pad *remote;
> +
> +	remote = media_entity_remote_pad(&video->pad);
> +
> +	if (!remote || !is_media_entity_v4l2_subdev(remote->entity))
> +		return NULL;
> +
> +	if (pad)
> +		*pad = remote->index;
> +
> +	return media_entity_to_v4l2_subdev(remote->entity);
> +}
> +
> +static int sun6i_video_queue_setup(struct vb2_queue *vq,
> +				 unsigned int *nbuffers, unsigned int *nplanes,
> +				 unsigned int sizes[],
> +				 struct device *alloc_devs[])
> +{
> +	struct sun6i_video *video = vb2_get_drv_priv(vq);
> +	unsigned int size = video->fmt.fmt.pix.sizeimage;
> +
> +	if (*nplanes)
> +		return sizes[0] < size ? -EINVAL : 0;
> +
> +	*nplanes = 1;
> +	sizes[0] = size;
> +
> +	return 0;
> +}
> +
> +static int sun6i_video_buffer_prepare(struct vb2_buffer *vb)
> +{
> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +	struct sun6i_csi_buffer *buf =
> +			container_of(vbuf, struct sun6i_csi_buffer, vb);
> +	struct sun6i_video *video = vb2_get_drv_priv(vb->vb2_queue);
> +	unsigned long size = video->fmt.fmt.pix.sizeimage;
> +
> +	if (vb2_plane_size(vb, 0) < size) {
> +		v4l2_err(video->vdev.v4l2_dev, "buffer too small (%lu < %lu)\n",
> +			 vb2_plane_size(vb, 0), size);
> +		return -EINVAL;
> +	}
> +
> +	vb2_set_plane_payload(vb, 0, size);
> +
> +	buf->dma_addr = vb2_dma_contig_plane_dma_addr(vb, 0);
> +
> +	vbuf->field = video->fmt.fmt.pix.field;
> +
> +	return 0;
> +}
> +
> +static int sun6i_pipeline_set_stream(struct sun6i_video *video, bool enable)
> +{
> +	struct media_entity *entity;
> +	struct media_pad *pad;
> +	struct v4l2_subdev *subdev;
> +	int ret;
> +
> +	entity = &video->vdev.entity;
> +	while (1) {
> +		pad = &entity->pads[0];
> +		if (!(pad->flags & MEDIA_PAD_FL_SINK))
> +			break;
> +
> +		pad = media_entity_remote_pad(pad);
> +		if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
> +			break;
> +
> +		entity = pad->entity;
> +		subdev = media_entity_to_v4l2_subdev(entity);
> +
> +		ret = v4l2_subdev_call(subdev, video, s_stream, enable);
> +		if (enable && ret < 0 && ret != -ENOIOCTLCMD)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int sun6i_video_start_streaming(struct vb2_queue *vq, unsigned int count)
> +{
> +	struct sun6i_video *video = vb2_get_drv_priv(vq);
> +	struct sun6i_csi_buffer *buf;
> +	struct sun6i_csi_config config;
> +	unsigned long flags;
> +	int ret;
> +
> +	video->sequence = 0;
> +
> +	ret = media_pipeline_start(&video->vdev.entity, &video->vdev.pipe);
> +	if (ret < 0)
> +		goto err_start_pipeline;
> +
> +	ret = sun6i_pipeline_set_stream(video, true);
> +	if (ret < 0)
> +		goto err_start_stream;

You're starting the stream from camera before enabling the capture on
the CSI, you may lose initial frame(s), which can add significant
latency if the sensor is set to 1fps for example.

> +	config.pixelformat = video->fmt.fmt.pix.pixelformat;
> +	config.code = video->current_fmt->mbus_code;
> +	config.field = video->fmt.fmt.pix.field;
> +	config.width = video->fmt.fmt.pix.width;
> +	config.height = video->fmt.fmt.pix.height;
> +
> +	ret = sun6i_csi_update_config(video->csi, &config);
> +	if (ret < 0)
> +		goto err_update_config;
> +
> +	spin_lock_irqsave(&video->dma_queue_lock, flags);
> +	video->cur_frm = list_first_entry(&video->dma_queue,
> +					  struct sun6i_csi_buffer, list);
> +	list_del(&video->cur_frm->list);
> +	spin_unlock_irqrestore(&video->dma_queue_lock, flags);
> +
> +	ret = sun6i_csi_update_buf_addr(video->csi, video->cur_frm->dma_addr);
> +	if (ret < 0)
> +		goto err_update_addr;

Note, that CSI has an internal queue for the registers you're changing
in sun6i_csi_update_buf_addr. This means that for updates like this:

A1 A2 A3 A4 A5....

you'll actually get writes like this in your ISR:

A1 A1 A2 A3 A4 A5 ...

A# being buffer addresses. You need to take this into account.

> +	ret = sun6i_csi_set_stream(video->csi, true);
> +	if (ret < 0)
> +		goto err_csi_stream;
> +
> +	return 0;
> +
> +err_csi_stream:
> +err_update_addr:
> +err_update_config:

One label can do it.

> +	sun6i_pipeline_set_stream(video, false);
> +err_start_stream:
> +	media_pipeline_stop(&video->vdev.entity);
> +err_start_pipeline:

^^^ Please name the labels by what the release code does, not by what
part of code it was jumped from. So it would be:

err_stop_stream:
  ...
err_stop_pipeline:
  ...
etc.

Also do this elsewhere in the code where there are multiple error path
labels.

> +	spin_lock_irqsave(&video->dma_queue_lock, flags);
> +	list_for_each_entry(buf, &video->dma_queue, list)
> +		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED);

You're leaking ->cur_frm. Perhaps don't save ->cur_frm separately and
peek into the queue when necessary. It will simplify other functions
too.

> +	INIT_LIST_HEAD(&video->dma_queue);
> +	spin_unlock_irqrestore(&video->dma_queue_lock, flags);
> +
> +	return ret;
> +}
> +
> +static void sun6i_video_stop_streaming(struct vb2_queue *vq)
> +{
> +	struct sun6i_video *video = vb2_get_drv_priv(vq);
> +	unsigned long flags;
> +	struct sun6i_csi_buffer *buf;
> +
> +	sun6i_pipeline_set_stream(video, false);
> +
> +	sun6i_csi_set_stream(video->csi, false);
> +
> +	media_pipeline_stop(&video->vdev.entity);
> +
> +	/* Release all active buffers */
> +	spin_lock_irqsave(&video->dma_queue_lock, flags);
> +	if (unlikely(video->cur_frm)) {
> +		vb2_buffer_done(&video->cur_frm->vb.vb2_buf,
> +				VB2_BUF_STATE_ERROR);
> +		video->cur_frm = NULL;
> +	}
> +	list_for_each_entry(buf, &video->dma_queue, list)
> +		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
> +	INIT_LIST_HEAD(&video->dma_queue);
> +	spin_unlock_irqrestore(&video->dma_queue_lock, flags);
> +}
> +
> +static void sun6i_video_buffer_queue(struct vb2_buffer *vb)
> +{
> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +	struct sun6i_csi_buffer *buf =
> +			container_of(vbuf, struct sun6i_csi_buffer, vb);
> +	struct sun6i_video *video = vb2_get_drv_priv(vb->vb2_queue);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&video->dma_queue_lock, flags);
> +	if (!video->cur_frm && list_empty(&video->dma_queue) &&
> +		vb2_is_streaming(vb->vb2_queue)) {
> +		video->cur_frm = buf;
> +		sun6i_csi_update_buf_addr(video->csi, video->cur_frm->dma_addr);
> +		sun6i_csi_set_stream(video->csi, 1);
> +	} else
> +		list_add_tail(&buf->list, &video->dma_queue);

You can simplify this by not keeping ->cur_frm off the queue.

> +	spin_unlock_irqrestore(&video->dma_queue_lock, flags);
> +}
> +
> +void sun6i_video_frame_done(struct sun6i_video *video)
> +{
> +	spin_lock(&video->dma_queue_lock);
> +
> +	if (video->cur_frm) {
> +		struct vb2_v4l2_buffer *vbuf = &video->cur_frm->vb;
> +		struct vb2_buffer *vb = &vbuf->vb2_buf;
> +
> +		vb->timestamp = ktime_get_ns();
> +		vbuf->sequence = video->sequence++;
> +		vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
> +		video->cur_frm = NULL;
> +	}

This will not work for the reasons above.

> +	if (!list_empty(&video->dma_queue)
> +	    && vb2_is_streaming(&video->vb2_vidq)) {
> +		video->cur_frm = list_first_entry(&video->dma_queue,
> +				struct sun6i_csi_buffer, list);
> +		list_del(&video->cur_frm->list);
> +		sun6i_csi_update_buf_addr(video->csi, video->cur_frm->dma_addr);
> +	} else
> +		sun6i_csi_set_stream(video->csi, 0);

Starting and stopping streaming like this on this CSI controller, will
need to account for the behavior of the hardware's internal queue of
buffer addresses. It will also add latency.

I have a working version of buffer handling for A83T CSI here:

https://github.com/megous/linux/blob/tbs-csi-hm5065/drivers/media/platf
orm/sun6i-csi/sun6i_csi.c#L230

It works around this by not stopping the stream when there are not
enough buffers and by making sure that HW and kernel aggree on where
the data is actually written to, which is not really obvious.

If you're testing with unchanging frames you may not see that the HW is
actually writing data to a stale address. It's painfully obvious with
real camera.

> +	spin_unlock(&video->dma_queue_lock);
> +}
> +
> +static struct vb2_ops sun6i_csi_vb2_ops = {
> +	.queue_setup		= sun6i_video_queue_setup,
> +	.wait_prepare		= vb2_ops_wait_prepare,
> +	.wait_finish		= vb2_ops_wait_finish,
> +	.buf_prepare		= sun6i_video_buffer_prepare,
> +	.start_streaming	= sun6i_video_start_streaming,
> +	.stop_streaming		= sun6i_video_stop_streaming,
> +	.buf_queue		= sun6i_video_buffer_queue,
> +};
> +
> +static int vidioc_querycap(struct file *file, void *priv,
> +				struct v4l2_capability *cap)
> +{

sun6i_csi_*

> +	struct sun6i_video *video = video_drvdata(file);
> +
> +	strlcpy(cap->driver, "sun6i-video", sizeof(cap->driver));
> +	strlcpy(cap->card, video->vdev.name, sizeof(cap->card));
> +	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> +		 video->csi->dev->of_node->name);
> +
> +	return 0;
> +}
> +
> +static int vidioc_enum_fmt_vid_cap(struct file *file, void *priv,
> +				   struct v4l2_fmtdesc *f)

sun6i_csi_*

> +{
> +	struct sun6i_video *video = video_drvdata(file);
> +	u32 index = f->index;
> +
> +	if (index >= video->num_formats)
> +		return -EINVAL;
> +
> +	f->pixelformat = video->formats[index].fourcc;
> +
> +	return 0;
> +}
> +
> +static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
> +				struct v4l2_format *fmt)
> +{

sun6i_csi_*

It's the same with many functions below.

> +	struct sun6i_video *video = video_drvdata(file);
> +
> +	*fmt = video->fmt;
> +
> +	return 0;
> +}
> +
> +static int sun6i_video_try_fmt(struct sun6i_video *video, struct v4l2_format *f,
> +			       struct sun6i_csi_format **current_fmt)
> +{
> +	struct sun6i_csi_format *csi_fmt;
> +	struct v4l2_pix_format *pixfmt = &f->fmt.pix;
> +	struct v4l2_subdev_format format;
> +	struct v4l2_subdev *subdev;
> +	u32 pad;
> +	int ret;
> +
> +	subdev = sun6i_video_remote_subdev(video, &pad);
> +	if (subdev == NULL)
> +		return -ENXIO;
> +
> +	csi_fmt = find_format_by_fourcc(video, pixfmt->pixelformat);
> +	if (csi_fmt == NULL) {
> +		if (video->num_formats > 0) {
> +			csi_fmt = &video->formats[0];
> +			pixfmt->pixelformat = csi_fmt->fourcc;
> +		} else
> +			return -EINVAL;
> +	}
> +
> +	format.pad = pad;
> +	format.which = V4L2_SUBDEV_FORMAT_TRY;
> +	v4l2_fill_mbus_format(&format.format, pixfmt, csi_fmt->mbus_code);
> +	ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &format);
> +	if (ret)
> +		return ret;

> +	v4l2_fill_pix_format(pixfmt, &format.format);
> +
> +	pixfmt->bytesperline = (pixfmt->width * csi_fmt->bpp) >> 3;
> +	pixfmt->sizeimage = (pixfmt->width * csi_fmt->bpp * pixfmt->height) / 8;
> +
> +	if (current_fmt)
> +		*current_fmt = csi_fmt;
> +
> +	return 0;
> +}
> +
> +static int sun6i_video_set_fmt(struct sun6i_video *video, struct v4l2_format *f)
> +{
> +	struct v4l2_subdev_format format;
> +	struct sun6i_csi_format *current_fmt;
> +	struct v4l2_subdev *subdev;
> +	u32 pad;
> +	int ret;
> +
> +	subdev = sun6i_video_remote_subdev(video, &pad);
> +	if (subdev == NULL)
> +		return -ENXIO;
> +
> +	ret = sun6i_video_try_fmt(video, f, &current_fmt);
> +	if (ret)
> +		return ret;
> +
> +	format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> +	v4l2_fill_mbus_format(&format.format, &f->fmt.pix,
> +			      current_fmt->mbus_code);
> +	ret = v4l2_subdev_call(subdev, pad, set_fmt, NULL, &format);
> +	if (ret < 0)
> +		return ret;

Subdev may alter anything in the format to what it supports. You
probably need to check that it still matches your driver expectatations
about width/height/mbus_code, etc.

thanks and regards,
  Ondrej Jirman

> +	video->fmt = *f;
> +	video->current_fmt = current_fmt;
> +
> +	return 0;
> +}
> +
> +static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
> +				struct v4l2_format *f)
> +{
> +	struct sun6i_video *video = video_drvdata(file);
> +
> +	if (vb2_is_busy(&video->vb2_vidq))
> +		return -EBUSY;
> +
> +	return sun6i_video_set_fmt(video, f);
> +}
> +
> +static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
> +				  struct v4l2_format *f)
> +{
> +	struct sun6i_video *video = video_drvdata(file);
> +
> +	return sun6i_video_try_fmt(video, f, NULL);
> +}
> +
> +static int vidioc_enum_input(struct file *file, void *fh,
> +			 struct v4l2_input *inp)
> +{
> +	struct sun6i_video *video = video_drvdata(file);
> +	struct v4l2_subdev *subdev;
> +	u32 pad;
> +	int ret;
> +
> +	if (inp->index != 0)
> +		return -EINVAL;
> +
> +	subdev = sun6i_video_remote_subdev(video, &pad);
> +	if (subdev == NULL)
> +		return -ENXIO;
> +
> +	ret = v4l2_subdev_call(subdev, video, g_input_status, &inp->status);
> +	if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
> +		return ret;
> +
> +	inp->type = V4L2_INPUT_TYPE_CAMERA;
> +
> +	if (v4l2_subdev_has_op(subdev, pad, dv_timings_cap)) {
> +		inp->capabilities = V4L2_IN_CAP_DV_TIMINGS;
> +		inp->std = 0;
> +	} else {
> +		inp->capabilities = 0;
> +		inp->std = 0;
> +	}
> +
> +	strlcpy(inp->name, subdev->name, sizeof(inp->name));
> +
> +	return 0;
> +}
> +
> +static int vidioc_g_input(struct file *file, void *fh, unsigned int *i)
> +{
> +	*i = 0;
> +
> +	return 0;
> +}
> +
> +static int vidioc_s_input(struct file *file, void *fh, unsigned int i)
> +{
> +	if (i != 0)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_ioctl_ops sun6i_video_ioctl_ops = {
> +	.vidioc_querycap		= vidioc_querycap,
> +	.vidioc_enum_fmt_vid_cap	= vidioc_enum_fmt_vid_cap,
> +	.vidioc_g_fmt_vid_cap		= vidioc_g_fmt_vid_cap,
> +	.vidioc_s_fmt_vid_cap		= vidioc_s_fmt_vid_cap,
> +	.vidioc_try_fmt_vid_cap		= vidioc_try_fmt_vid_cap,
> +
> +	.vidioc_enum_input		= vidioc_enum_input,
> +	.vidioc_s_input			= vidioc_s_input,
> +	.vidioc_g_input			= vidioc_g_input,
> +
> +	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
> +	.vidioc_querybuf		= vb2_ioctl_querybuf,
> +	.vidioc_qbuf			= vb2_ioctl_qbuf,
> +	.vidioc_expbuf			= vb2_ioctl_expbuf,
> +	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
> +	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
> +	.vidioc_prepare_buf		= vb2_ioctl_prepare_buf,
> +	.vidioc_streamon		= vb2_ioctl_streamon,
> +	.vidioc_streamoff		= vb2_ioctl_streamoff,
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * V4L2 file operations
> + */
> +static int sun6i_video_open(struct file *file)
> +{
> +	struct sun6i_video *video = video_drvdata(file);
> +	int ret;
> +
> +	if (mutex_lock_interruptible(&video->lock))
> +		return -ERESTARTSYS;
> +
> +	ret = v4l2_fh_open(file);
> +	if (ret < 0)
> +		goto unlock;
> +
> +	ret = v4l2_pipeline_pm_use(&video->vdev.entity, 1);
> +	if (ret < 0)
> +		goto fh_release;
> +
> +	if (!v4l2_fh_is_singular_file(file))
> +		goto unlock;
> +
> +	ret = sun6i_csi_set_power(video->csi, true);
> +	if (ret < 0)
> +		goto fh_release;
> +
> +	mutex_unlock(&video->lock);
> +	return 0;
> +
> +fh_release:
> +	v4l2_fh_release(file);
> +unlock:
> +	mutex_unlock(&video->lock);
> +	return ret;
> +}
> +
> +static int sun6i_video_close(struct file *file)
> +{
> +	struct sun6i_video *video = video_drvdata(file);
> +	bool last_fh;
> +
> +	mutex_lock(&video->lock);
> +
> +	last_fh = v4l2_fh_is_singular_file(file);
> +
> +	_vb2_fop_release(file, NULL);
> +
> +	v4l2_pipeline_pm_use(&video->vdev.entity, 0);
> +
> +	if (last_fh)
> +		sun6i_csi_set_power(video->csi, false);
> +
> +	mutex_unlock(&video->lock);
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_file_operations sun6i_video_fops = {
> +	.owner		= THIS_MODULE,
> +	.open		= sun6i_video_open,
> +	.release	= sun6i_video_close,
> +	.unlocked_ioctl	= video_ioctl2,
> +	.mmap		= vb2_fop_mmap,
> +	.poll		= vb2_fop_poll
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * Media Operations
> + */
> +static int sun6i_video_formats_init(struct sun6i_video *video)
> +{
> +	struct v4l2_subdev_mbus_code_enum mbus_code = { 0 };
> +	struct sun6i_csi *csi = video->csi;
> +	struct v4l2_format format;
> +	struct v4l2_subdev *subdev;
> +	u32 pad;
> +	const u32 *pixformats;
> +	int pixformat_count = 0;
> +	u32 subdev_codes[32]; /* subdev format codes, 32 should be enough */
> +	int codes_count = 0;
> +	int num_fmts = 0;
> +	int i, j;
> +
> +	subdev = sun6i_video_remote_subdev(video, &pad);
> +	if (subdev == NULL)
> +		return -ENXIO;
> +
> +	/* Get supported pixformats of CSI */
> +	pixformat_count = sun6i_csi_get_supported_pixformats(csi, &pixformats);
> +	if (pixformat_count <= 0)
> +		return -ENXIO;
> +
> +	/* Get subdev formats codes */
> +	mbus_code.pad = pad;
> +	mbus_code.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> +	while (!v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL,
> +				 &mbus_code)) {
> +		if (codes_count >= ARRAY_SIZE(subdev_codes)) {
> +			dev_warn(video->csi->dev,
> +				 "subdev_codes array is full!\n");
> +			break;
> +		}
> +		subdev_codes[codes_count] = mbus_code.code;
> +		codes_count++;
> +		mbus_code.index++;
> +	}
> +
> +	if (!codes_count)
> +		return -ENXIO;
> +
> +	/* Get supported formats count */
> +	for (i = 0; i < codes_count; i++) {
> +		for (j = 0; j < pixformat_count; j++) {
> +			if (!sun6i_csi_is_format_support(csi, pixformats[j],
> +					mbus_code.code)) {
> +				continue;
> +			}
> +			num_fmts++;
> +		}
> +	}
> +
> +	if (!num_fmts)
> +		return -ENXIO;
> +
> +	video->num_formats = num_fmts;
> +	video->formats = devm_kcalloc(video->csi->dev, num_fmts,
> +			sizeof(struct sun6i_csi_format), GFP_KERNEL);
> +	if (!video->formats)
> +		return -ENOMEM;
> +
> +	/* Get supported formats */
> +	num_fmts = 0;
> +	for (i = 0; i < codes_count; i++) {
> +		for (j = 0; j < pixformat_count; j++) {
> +			if (!sun6i_csi_is_format_support(csi, pixformats[j],
> +					mbus_code.code)) {
> +				continue;
> +			}
> +
> +			video->formats[num_fmts].fourcc = pixformats[j];
> +			video->formats[num_fmts].mbus_code =
> +					mbus_code.code;
> +			video->formats[num_fmts].bpp =
> +					v4l2_pixformat_get_bpp(pixformats[j]);
> +			num_fmts++;
> +		}
> +	}
> +
> +	/* setup default format */
> +	format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> +	format.fmt.pix.width = 1280;
> +	format.fmt.pix.height = 720;
> +	format.fmt.pix.pixelformat = video->formats[0].fourcc;
> +	sun6i_video_set_fmt(video, &format);
> +
> +	return 0;
> +}
> +
> +static int sun6i_video_link_setup(struct media_entity *entity,
> +				  const struct media_pad *local,
> +				  const struct media_pad *remote, u32 flags)
> +{
> +	struct video_device *vdev = media_entity_to_video_device(entity);
> +	struct sun6i_video *video = video_get_drvdata(vdev);
> +
> +	if (WARN_ON(video == NULL))
> +		return 0;
> +
> +	return sun6i_video_formats_init(video);
> +}
> +
> +static const struct media_entity_operations sun6i_video_media_ops = {
> +	.link_setup = sun6i_video_link_setup,
> +};
> +
> +int sun6i_video_init(struct sun6i_video *video, struct sun6i_csi *csi,
> +		     const char *name)
> +{
> +	struct video_device *vdev = &video->vdev;
> +	struct vb2_queue *vidq = &video->vb2_vidq;
> +	int ret;
> +
> +	video->csi = csi;
> +
> +	/* Initialize the media entity... */
> +	video->pad.flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
> +	vdev->entity.ops = &sun6i_video_media_ops;
> +	ret = media_entity_pads_init(&vdev->entity, 1, &video->pad);
> +	if (ret < 0)
> +		return ret;
> +
> +	mutex_init(&video->lock);
> +
> +	INIT_LIST_HEAD(&video->dma_queue);
> +	spin_lock_init(&video->dma_queue_lock);
> +
> +	video->cur_frm = NULL;
> +	video->sequence = 0;
> +	video->num_formats = 0;
> +
> +	/* Initialize videobuf2 queue */
> +	vidq->type			= V4L2_BUF_TYPE_VIDEO_CAPTURE;
> +	vidq->io_modes			= VB2_MMAP | VB2_DMABUF;
> +	vidq->drv_priv			= video;
> +	vidq->buf_struct_size		= sizeof(struct sun6i_csi_buffer);
> +	vidq->ops			= &sun6i_csi_vb2_ops;
> +	vidq->mem_ops			= &vb2_dma_contig_memops;
> +	vidq->timestamp_flags		= V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> +	vidq->lock			= &video->lock;
> +	vidq->min_buffers_needed	= 1;
> +	vidq->dev			= csi->dev;
> +
> +	ret = vb2_queue_init(vidq);
> +	if (ret) {
> +		v4l2_err(&csi->v4l2_dev, "vb2_queue_init failed: %d\n", ret);
> +		goto error;
> +	}
> +
> +	/* Register video device */
> +	strlcpy(vdev->name, name, sizeof(vdev->name));
> +	vdev->release		= video_device_release_empty;
> +	vdev->fops		= &sun6i_video_fops;
> +	vdev->ioctl_ops		= &sun6i_video_ioctl_ops;
> +	vdev->vfl_type		= VFL_TYPE_GRABBER;
> +	vdev->vfl_dir		= VFL_DIR_RX;
> +	vdev->v4l2_dev		= &csi->v4l2_dev;
> +	vdev->queue		= vidq;
> +	vdev->lock		= &video->lock;
> +	vdev->device_caps	= V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_CAPTURE;
> +	video_set_drvdata(vdev, video);
> +
> +	ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
> +	if (ret < 0) {
> +		v4l2_err(&csi->v4l2_dev,
> +			 "video_register_device failed: %d\n", ret);
> +		goto error;
> +	}
> +
> +	return 0;
> +
> +error:
> +	sun6i_video_cleanup(video);
> +	return ret;
> +}
> +
> +void sun6i_video_cleanup(struct sun6i_video *video)
> +{
> +	if (video_is_registered(&video->vdev))
> +		video_unregister_device(&video->vdev);
> +
> +	media_entity_cleanup(&video->vdev.entity);
> +}
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.h b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.h
> new file mode 100644
> index 0000000..14eac6e
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.h
> @@ -0,0 +1,61 @@
> +/*
> + * Copyright (c) 2017 Yong Deng <yong.deng-+3dxTMOEIRNWk0Htik3J/w@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __SUN6I_VIDEO_H__
> +#define __SUN6I_VIDEO_H__
> +
> +#include <media/v4l2-dev.h>
> +#include <media/videobuf2-core.h>
> +
> +/*
> + * struct sun6i_csi_format - CSI media bus format information
> + * @fourcc: Fourcc code for this format
> + * @mbus_code: V4L2 media bus format code.
> + * @bpp: Bytes per pixel (when stored in memory)
> + */
> +struct sun6i_csi_format {
> +	u32				fourcc;
> +	u32				mbus_code;
> +	u8				bpp;
> +};
> +
> +struct sun6i_csi;
> +
> +struct sun6i_video {
> +	struct video_device		vdev;
> +	struct media_pad		pad;
> +	struct sun6i_csi		*csi;
> +
> +	struct mutex			lock;
> +
> +	struct vb2_queue		vb2_vidq;
> +	spinlock_t			dma_queue_lock;
> +	struct list_head		dma_queue;
> +
> +	struct sun6i_csi_buffer		*cur_frm;
> +	unsigned int			sequence;
> +
> +	struct sun6i_csi_format		*formats;
> +	unsigned int			num_formats;
> +	struct sun6i_csi_format		*current_fmt;
> +	struct v4l2_format		fmt;
> +};
> +
> +int sun6i_video_init(struct sun6i_video *video, struct sun6i_csi *csi,
> +		     const char *name);
> +void sun6i_video_cleanup(struct sun6i_video *video);
> +
> +void sun6i_video_frame_done(struct sun6i_video *video);
> +
> +#endif /* __SUN6I_VIDEO_H__ */
> -- 
> 1.8.3.1
> 

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 1/3] Input: twl4030-vibra: fix sibling-node lookup
From: Johan Hovold @ 2017-11-13 11:51 UTC (permalink / raw)
  To: Lee Jones
  Cc: Johan Hovold, Dmitry Torokhov, linux-input, linux-kernel, stable,
	Peter Ujfalusi, Marek Belisko, Rob Herring, devicetree
In-Reply-To: <20171113102028.c53rlcnwrjaey2tv@dell>

On Mon, Nov 13, 2017 at 10:20:28AM +0000, Lee Jones wrote:
> On Mon, 13 Nov 2017, Johan Hovold wrote:
> 
> > On Mon, Nov 13, 2017 at 09:11:44AM +0000, Lee Jones wrote:
> > > On Sun, 12 Nov 2017, Johan Hovold wrote:
> > > 
> > > > [ +CC: Lee, Rob and device-tree list ]
> > > > 
> > > > On Sat, Nov 11, 2017 at 09:50:59AM -0800, Dmitry Torokhov wrote:
> > > > > On Sat, Nov 11, 2017 at 04:43:37PM +0100, Johan Hovold wrote:
> > > > > > A helper purported to look up a child node based on its name was using
> > > > > > the wrong of-helper and ended up prematurely freeing the parent of-node
> > > > > > while searching the whole device tree depth-first starting at the parent
> > > > > > node.
> > > > > 
> > > > > Ugh, this all is pretty ugly business. Can we teach MFD to allow
> > > > > specifying firmware node to be attached to the platform devices it
> > > > > creates in mfd_add_device() so that the leaf drivers simply call
> > > > > device_property_read_XXX() on their own device and not be bothered with
> > > > > weird OF refcount issues or what node they need to locate and parse?
> > > 
> > > If a child compatible is provided, we already set the child's
> > > of_node.  It's then up to the driver (set) author(s) to use it in the
> > > correct manner. 
> > > 
> > > > Yeah, that may have helped. You can actually specify a compatible string
> > > > in struct mfd_cell today which does make mfd_add_device() associate a
> > > > matching child node.
> > > > 
> > > > Some best practice regarding how to deal with MFD and device tree would
> > > > be good to determine and document too. For example, when should
> > > > of_platform_populate() be used in favour of mfd_add_device()?
> > > 
> > > When the device supports DT and its entire hierarchical layout, along
> > > with all of its attributes can be expressed in DT.
> > 
> > Ok, a follow up: When there are different variants of an MFD and that
> > affects the child drivers, then that should be expressed in in the child
> > node compatibles rather than having the child match on the parent node?
> > 
> > I'm asking because this came up recently during review and their seems
> > to be no precedent for matching on the parent compatible in child
> > drivers:
> > 
> > 	https://lkml.kernel.org/r/20171105154725.GA11226@localhost
> 
> Accessing the parent's of_device_id .data directly doesn't sit well
> with me.  The parent driver should pass this type of configuration
> though pdata IMHO.

The child driver is only matching on the parent-node compatible string
IIRC, and therefore keeps its own table of all parent compatibles with
its own set of (child) private match data (i.e. the parent compatible
property is matched first by the parent driver, and then again by the
child).

Passing through pdata here is not possible since mfd_add_device() isn't
used, right? It could of course be described using properties of the
child node (e.g. by using different child compatible strings).

> > > > And how best to deal with sibling nodes, which is part of the problem
> > > > here (I think the mfd should have provided a flag rather than having
> > > > subdrivers deal with sibling nodes, for example).
> > > 
> > > I disagree.  The only properties the MFD (parent) driver is interested
> > > in is ones which are shared across multiple child devices.
> > > *Everything* which pertains to only a single child device should be
> > > handled by its accompanying driver. 
> > 
> > Even if that means leaking details of one child driver into a sibling?
> 
> Not sure what you mean here.  Could you please elaborate or provide an
> example?

I mean that the sibling node needs to be aware of the name, compatible
string, or other node properties of its sibling node to be able to parse
sibling nodes itself (rather than the sibling or parent doing so on its
behalf). But it seems you answer this below.

> > Isn't it then cleaner to use the parent MFD to coordinate between the
> > cells, just as we do for IO?
> > 
> > In this case a child driver looked up a sibling node based on name, but
> 
> This should not be allowed.  If >1 sibling requires access to a
> particular property, this is normally evidence enough that this
> property should be shared and handled by the parent.
> 
> > that doesn't mean the node is active, that there's a driver bound, or
> > that the sibling node has some other random property for example. The
> > parent could be used for such coordination, if only to pass information
> > from one sibling to another.
> 
> Right.

Ok, it seems we're in agreement here.

Given that MFD has evolved over time and device-tree support has been
added retroactively to some drivers, we've ended up with a multitude of
different ways of dealing with such issues. I think it may still be a
good idea to jot down some best practices for future driver developers.

Thanks,
Johan

^ permalink raw reply

* RE: Problem with commit to add overlay symbols to live device tree
From: Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w @ 2017-11-13 12:11 UTC (permalink / raw)
  To: frowand.list-Re5JQEeQqe8AvxtiuMwx3w, frank.rowand-7U/KSKJipcs,
	robh-DgEjT+Ai2ygdnm+yROfE0A
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <a444bccd-1d42-44b9-398c-095fb4fb9055-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

> On 11/09/17 09:09, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org wrote:
> >> On 10/20/17 05:11, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org wrote:
> >>>> Hi Alexander,
> >>>>
> >>>> On 10/19/17 10:06, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org wrote:
> >>>>> Hi Frank, Rob,
> >>>>>
> >>>>> I ran some tests with kernel v4.14-rc2, where I came across an issue
> >>>>> with your commit "of: overlay: add overlay symbols to live device
> >>>>> tree" (d1651b03c2df75db8eda3fbcd3a07adb337ee8b0,
> >>>>>
> >>>>
> >>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/d
> >>>> rivers/of?h=v4.14-
> >> rc5&id=d1651b03c2df75db8eda3fbcd3a07adb337ee8b0).
> >>>>> I am not sure whether this is a problem with your change or whether I
> >>>>> made a mistake somewhere.
> >>>>>
> >>>>> Without this commit, the following device tree overlay works fine on
> >>>>> a Raspberry Pi:
> >>>>>
> >>>>> /dts-v1/;
> >>>>> /plugin/;
> >>>>>
> >>>>> / {
> >>>>> 	fragment@0 {
> >>>>> 		target-path = "/soc/spi@7e204000";
> >>>>> 		__overlay__ {
> >>>>> 			#address-cells = <1>;
> >>>>> 			#size-cells = <0>;
> >>>>> 			status = "okay";
> >>>>>
> >>>>> 		        spidev1: spi@1 {
> >>>>> 				compatible = "spidev";
> >>>>> 				reg = <1>;
> >>>>> 				spi-max-frequency = <5000000>;
> >>>>> 			};
> >>>>> 		};
> >>>>> 	};
> >>>>> };
> >>>>>
> >>>>> But with the commit, it is rejected:
> >>>>>
> >>>>> OF: overlay: no symbols in root of device tree.
> >>>>> OF: overlay: of_build_overlay_info() failed for tree@/
> >>>>> create_overlay: Failed to create overlay (err=-22)
> >>>>>
> >>>>> Only if I remove the spidev1 label, it continues to work with the
> >>>>> commit (but this is not always possible in more complex overlays).
> >>>>>
> >>>>> I hope this does not make a difference, but I use the configfs
> >>>>> interface to load the overlay, that is not part of mainline, but
> >>>>> shipped by several distributions nonetheless (for example in the
> >>>>> Raspberry Pi kernel,
> >>>>>
> >>>>
> >>
> https://github.com/raspberrypi/linux/commit/cccc24635da69799335566eb46
> >>>> 4a4c9e1fb4a8ad).
> >>>>>
> >>>>>
> >>>>> Could you tell me whether this change in behavior is intentional and
> >>>>> whether I have to fix something in my usage?
> >>>>
> >>>> Yes, the change is intentional.
> >>>>
> >>>> If you are using overlays, the expectation is that the device tree
> >>>> that was used to boot contains symbols (the paths of nodes that
> >>>> have phandles) so that phandle references in the overlay can be
> >>>> fixed up to match the values in the base devicetree.  Symbols
> >>>> will be added to the device tree if if is compiled with the
> >>>> "@" option.  This is the first method to avoid the overlay
> >>>> load error.
> >>>
> >>> Ah, thanks, this does solve the problem. I use nothing special to
> >>> generate the base devicetree, just "make dtbs". Would it make sense
> >>> to add the "@" option to the kernel Makefile, so that symbols are
> >>> added by default? Or does this have any downside? I'd suspect that
> >>
> >> Yes, there is a large downside.  Compiling with the "@" option will
> >> increase the size of the DTB.  Most boards will not use overlays,
> >> and in those cases this overhead has no value.  So "@" should be
> >> opt-in, not default.
> >
> > I see. Is there a way (a config option maybe?) that I can use to get this
> behavior without patching the Makefile?
> 
> To compile a DTB with -@, you can specify the value of DTC_FLAGS
> on the make command line.  For example:
> 
>    DTC_FLAGS="-@" make qcom-apq8074-dragonboard.dtb
> 
> This will work for arm.

I tried that for my Raspberry Pi 2, with the mainline kernel (so no Raspberry Pi specific patches, still v4.14-rc2). Unfortunately, with that DTB the machine does not boot anymore, there is no output at all from the kernel:

---
switch to partitions #0, OK
mmc0 is current device
Scanning mmc 0:1...
Found U-Boot script /boot.scr
reading /boot.scr
696 bytes read in 11 ms (61.5 KiB/s)
## Executing script at 02000000
reading /zImage
6017520 bytes read in 791 ms (7.3 MiB/s)
reading /dtbs/bcm2836-rpi-2-b.dtb
18795 bytes read in 188 ms (96.7 KiB/s)
reading /initramfs-linux.img
4173703 bytes read in 546 ms (7.3 MiB/s)
Kernel image @ 0x1000000 [ 0x000000 - 0x5bd1f0 ]
## Flattened Device Tree blob at 00000100
   Booting using the fdt blob at 0x000100
   reserving fdt memory region: addr=0 size=1000
   Using Device Tree in place at 00000100, end 00007a6a

Starting kernel ...
---

Using an older DTB for the same kernel build works though.

I've used the dtc to get back the source for both DTBs and compared those. They are identical, except for several (linux,)phandle properties that appeared in various nodes and the large __symbols__ section. Also, the dtc generates two more warnings when parsing the new DTB, but I'm not sure whether those are significant:

Warning (clocks_property): property 'clocks' size (21) is invalid, expected multiple of 4 in node /__symbols__
Warning (gpios_property): property 'gpio' size (19) is invalid, expected multiple of 4 in node /__symbols__

Can something else be broken in the DTB, that is not visible within the DTS? Or could this be a size issue? The new DTB is ~6KB larger than the old one (18KB vs. 12KB). Any idea how to debug this further?

> For some other architectures, you may want to
> add additional flags to DTC_FLAGS, and for microblaze this will not work
> with the current makefile (the arch makefiles should probably be changed
> to be: DTC_FLAGS += ... or to remove the "-p 1024" if no longer needed):
> 
>   $ git grep DTC_FLAGS | grep Makefile: | grep arch
>   arch/c6x/boot/dts/Makefile:DTC_FLAGS ?= -p 1024
>   arch/microblaze/boot/dts/Makefile:DTC_FLAGS := -p 1024
>   arch/openrisc/boot/dts/Makefile:#DTC_FLAGS ?= -p 1024
>   arch/powerpc/boot/Makefile:DTC_FLAGS	?= -p 1024
> 
> 
> >>> many more people will be hit by this change, since most tutorials for
> >>> overlays that I have seen include the "@" option for the dtc calls.>
> >>>> If you compile the overlay with the "@" option then symbols
> >>>> will be added to the overlay DTB.  Previous to commit
> >>>> d1651b03c2df, these symbols would be ignored when an overlay
> >>>> was loaded.  If you remove the "@" from the compile of
> >>>> overlays, then everything should work the same way it did
> >>>> before the commit.  This is the second way to avoid the
> >>>> load overlay error.
> >
> > Do I understand that correctly, that when I remove the "@" option
> > from the dtc calls, the same call should work both with an old
> > (pre-d1651b03c2df) and a new kernel? If the old kernel just ignored
> > the symbols, they were not used for anything, so it should not matter
> > whether they were contained in the overlay DTB or not, right?
> 
> I just mentioned that workaround as another temporary way to work
> around your specific case.  You should probably just forget I even
> mentioned it.
> 
> 
> >>>>
> >>>>
> >>>>> Thanks
> >>>>> Alexander
> >>>>> --
> >>>>> To unsubscribe from this list: send the line "unsubscribe devicetree"
> in
> >>>>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >>>>> More majordomo info at  http://vger.kernel.org/majordomo-
> info.html
> >>>
> >>
> >
> >
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/2] pinctrl: sh-pfc: add PORT_GP_CFG_{6|22}() helper macros
From: Geert Uytterhoeven @ 2017-11-13 12:23 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Linus Walleij, Laurent Pinchart, Geert Uytterhoeven,
	Linux-Renesas, linux-gpio@vger.kernel.org, Rob Herring,
	devicetree@vger.kernel.org, Mark Rutland, Vladimir Barinov
In-Reply-To: <20171110181547.933934369@cogentembedded.com>

Hi Sergei,

On Fri, Nov 10, 2017 at 6:59 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> They follow the style of the existing PORT_GP_CFG_<n>() macros and
> will be used by a follow-up  patch for the R8A77970 SoC.
>
> Based on the original (and large) patch by Daisuke Matsushita
> <daisuke.matsushita.ns@hitachi.com>.
>
> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Thanks for your patch!

Soon we will have all 32 sets of macros ;-)

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
I.e. will queue in sh-pfc-for-v4.16.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH] MIPS: implement a "bootargs-append" DT property
From: Geert Uytterhoeven @ 2017-11-13 12:31 UTC (permalink / raw)
  To: James Hogan, Daniel Gimpelevich
  Cc: Linux MIPS Mailing List, Rob Herring, Frank Rowand,
	devicetree@vger.kernel.org, Linux-Renesas
In-Reply-To: <20171113112312.GZ15260@jhogan-linux>

Hi James, Daniel,

On Mon, Nov 13, 2017 at 12:23 PM, James Hogan <james.hogan@mips.com> wrote:
> On Sat, Nov 11, 2017 at 09:19:48AM -0800, Daniel Gimpelevich wrote:
>> There are two uses for this:
>>
>> 1) It may be useful to split a device-specific kernel command line between
>> a .dts file and a .dtsi file, with "bootargs" in one and "bootargs-append"
>> in the other, such as for variations of a reference board.

I've seen other use cases, e.g. the extension of the du node's "clocks" and
"clock-names" properties from arch/arm64/boot/dts/renesas/r8a7795.dtsi to
arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts.

To avoid the proliferation of "-append" versions of existing properties, what
about handling this in dtc, by adding support for an "/append-property/"
keyword?

     bootargs = "first part"
     ...
     /append-property/ bootargs = " second part".

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v7 1/1] pwm: mediatek: add MT2712/MT7622 support
From: Zhi Mao @ 2017-11-13 12:39 UTC (permalink / raw)
  To: john
  Cc: Thierry Reding, claudiu.beznea, Rob Herring, Mark Rutland,
	Matthias Brugger, linux-pwm, srv_heupstream, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, yingjoe.chen,
	yt.shen, sean.wang, zhi.mao, zhenbao.liu
In-Reply-To: <1508926261-25015-1-git-send-email-zhi.mao@mediatek.com>

Hi Thierry,

Just have a ping for this patch.

Regards
Zhi


On Wed, 2017-10-25 at 18:11 +0800, Zhi Mao wrote:
> Add support to MT2712 and MT7622.
> Due to register offset address of pwm7 for MT2712 is not fixed 0x40,
> add mtk_pwm_reg_offset array for pwm register offset.
> 
> Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
> Signed-off-by: Zhi Mao <zhi.mao@mediatek.com>
> ---
> changee in v7:
> - adjust the commit message
> 
> Changes in v6:
> - remove "struct mtk_pwm_platform_data" member from "mtk_pwm_chip". 
> 
> Changes in v5:
> - Add NULL pointer checking for "data"
> 
>  drivers/pwm/pwm-mediatek.c |   53 ++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 44 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
> index 1d78ab1..cdd4d31 100644
> --- a/drivers/pwm/pwm-mediatek.c
> +++ b/drivers/pwm/pwm-mediatek.c
> @@ -16,6 +16,7 @@
>  #include <linux/module.h>
>  #include <linux/clk.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/pwm.h>
>  #include <linux/slab.h>
> @@ -40,11 +41,19 @@ enum {
>  	MTK_CLK_PWM3,
>  	MTK_CLK_PWM4,
>  	MTK_CLK_PWM5,
> +	MTK_CLK_PWM6,
> +	MTK_CLK_PWM7,
> +	MTK_CLK_PWM8,
>  	MTK_CLK_MAX,
>  };
>  
> -static const char * const mtk_pwm_clk_name[] = {
> -	"main", "top", "pwm1", "pwm2", "pwm3", "pwm4", "pwm5"
> +static const char * const mtk_pwm_clk_name[MTK_CLK_MAX] = {
> +	"main", "top", "pwm1", "pwm2", "pwm3", "pwm4", "pwm5", "pwm6", "pwm7",
> +	"pwm8"
> +};
> +
> +struct mtk_pwm_platform_data {
> +	unsigned int num_pwms;
>  };
>  
>  /**
> @@ -59,6 +68,10 @@ struct mtk_pwm_chip {
>  	struct clk *clks[MTK_CLK_MAX];
>  };
>  
> +static const unsigned int mtk_pwm_reg_offset[] = {
> +	0x0010, 0x0050, 0x0090, 0x00d0, 0x0110, 0x0150, 0x0190, 0x0220
> +};
> +
>  static inline struct mtk_pwm_chip *to_mtk_pwm_chip(struct pwm_chip *chip)
>  {
>  	return container_of(chip, struct mtk_pwm_chip, chip);
> @@ -103,14 +116,14 @@ static void mtk_pwm_clk_disable(struct pwm_chip *chip, struct pwm_device *pwm)
>  static inline u32 mtk_pwm_readl(struct mtk_pwm_chip *chip, unsigned int num,
>  				unsigned int offset)
>  {
> -	return readl(chip->regs + 0x10 + (num * 0x40) + offset);
> +	return readl(chip->regs + mtk_pwm_reg_offset[num] + offset);
>  }
>  
>  static inline void mtk_pwm_writel(struct mtk_pwm_chip *chip,
>  				  unsigned int num, unsigned int offset,
>  				  u32 value)
>  {
> -	writel(value, chip->regs + 0x10 + (num * 0x40) + offset);
> +	writel(value, chip->regs + mtk_pwm_reg_offset[num] + offset);
>  }
>  
>  static int mtk_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> @@ -185,6 +198,7 @@ static void mtk_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
>  
>  static int mtk_pwm_probe(struct platform_device *pdev)
>  {
> +	const struct mtk_pwm_platform_data *data;
>  	struct mtk_pwm_chip *pc;
>  	struct resource *res;
>  	unsigned int i;
> @@ -194,15 +208,22 @@ static int mtk_pwm_probe(struct platform_device *pdev)
>  	if (!pc)
>  		return -ENOMEM;
>  
> +	data = of_device_get_match_data(&pdev->dev);
> +	if (data == NULL)
> +		return -EINVAL;
> +
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	pc->regs = devm_ioremap_resource(&pdev->dev, res);
>  	if (IS_ERR(pc->regs))
>  		return PTR_ERR(pc->regs);
>  
> -	for (i = 0; i < MTK_CLK_MAX; i++) {
> +	for (i = 0; i < data->num_pwms + 2; i++) {
>  		pc->clks[i] = devm_clk_get(&pdev->dev, mtk_pwm_clk_name[i]);
> -		if (IS_ERR(pc->clks[i]))
> +		if (IS_ERR(pc->clks[i])) {
> +			dev_err(&pdev->dev, "clock: %s fail: %ld\n",
> +				mtk_pwm_clk_name[i], PTR_ERR(pc->clks[i]));
>  			return PTR_ERR(pc->clks[i]);
> +		}
>  	}
>  
>  	platform_set_drvdata(pdev, pc);
> @@ -210,7 +231,7 @@ static int mtk_pwm_probe(struct platform_device *pdev)
>  	pc->chip.dev = &pdev->dev;
>  	pc->chip.ops = &mtk_pwm_ops;
>  	pc->chip.base = -1;
> -	pc->chip.npwm = 5;
> +	pc->chip.npwm = data->num_pwms;
>  
>  	ret = pwmchip_add(&pc->chip);
>  	if (ret < 0) {
> @@ -228,9 +249,23 @@ static int mtk_pwm_remove(struct platform_device *pdev)
>  	return pwmchip_remove(&pc->chip);
>  }
>  
> +static const struct mtk_pwm_platform_data mt2712_pwm_data = {
> +	.num_pwms = 8,
> +};
> +
> +static const struct mtk_pwm_platform_data mt7622_pwm_data = {
> +	.num_pwms = 6,
> +};
> +
> +static const struct mtk_pwm_platform_data mt7623_pwm_data = {
> +	.num_pwms = 5,
> +};
> +
>  static const struct of_device_id mtk_pwm_of_match[] = {
> -	{ .compatible = "mediatek,mt7623-pwm" },
> -	{ }
> +	{ .compatible = "mediatek,mt2712-pwm", .data = &mt2712_pwm_data },
> +	{ .compatible = "mediatek,mt7622-pwm", .data = &mt7622_pwm_data },
> +	{ .compatible = "mediatek,mt7623-pwm", .data = &mt7623_pwm_data },
> +	{ },
>  };
>  MODULE_DEVICE_TABLE(of, mtk_pwm_of_match);
>  

^ permalink raw reply

* [PATCH] drm/tilcdc: Remove obsolete "ti, tilcdc, slave" dts binding support
From: Jyri Sarha @ 2017-11-13 12:42 UTC (permalink / raw)
  To: dri-devel, devicetree, bcousson, tony
  Cc: airlied, broonie, stephen.boyd, Jyri Sarha, tomi.valkeinen,
	laurent.pinchart, frank.rowand

This patch removes DRM_TILCDC_SLAVE_COMPAT option for supporting the
obsolete "ti,tilcdc,slave" device tree binding. The new of_graph based
binding - that is widely used in other drm driver too - has been
supported since Linux v4.2. Maintaining the the backwards dts
conversion code in 0the DRM_TILCDC_SLAVE_COMPAT has become a nuisance
for the device/of development so the we decided to drop it after Linux
v4.14, the 2017 LTS.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/Kconfig                 |  11 -
 drivers/gpu/drm/tilcdc/Makefile                |   3 -
 drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c   | 269 -------------------------
 drivers/gpu/drm/tilcdc/tilcdc_slave_compat.dts |  72 -------
 drivers/gpu/drm/tilcdc/tilcdc_slave_compat.h   |  25 ---
 5 files changed, 380 deletions(-)
 delete mode 100644 drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c
 delete mode 100644 drivers/gpu/drm/tilcdc/tilcdc_slave_compat.dts
 delete mode 100644 drivers/gpu/drm/tilcdc/tilcdc_slave_compat.h

diff --git a/drivers/gpu/drm/tilcdc/Kconfig b/drivers/gpu/drm/tilcdc/Kconfig
index 28fed7e..81ac824 100644
--- a/drivers/gpu/drm/tilcdc/Kconfig
+++ b/drivers/gpu/drm/tilcdc/Kconfig
@@ -12,14 +12,3 @@ config DRM_TILCDC
 	  controller, for example AM33xx in beagle-bone, DA8xx, or
 	  OMAP-L1xx.  This driver replaces the FB_DA8XX fbdev driver.
 
-config DRM_TILCDC_SLAVE_COMPAT
-	bool "Support device tree blobs using TI LCDC Slave binding"
-	depends on DRM_TILCDC
-	default y
-	select OF_RESOLVE
-	select OF_OVERLAY
-	help
-	  Choose this option if you need a kernel that is compatible
-	  with device tree blobs using the obsolete "ti,tilcdc,slave"
-	  binding. If you find "ti,tilcdc,slave"-string from your DTB,
-	  you probably need this. Otherwise you do not.
diff --git a/drivers/gpu/drm/tilcdc/Makefile b/drivers/gpu/drm/tilcdc/Makefile
index 55ebd51..efc2c4f 100644
--- a/drivers/gpu/drm/tilcdc/Makefile
+++ b/drivers/gpu/drm/tilcdc/Makefile
@@ -2,9 +2,6 @@ ifeq (, $(findstring -W,$(EXTRA_CFLAGS)))
 	ccflags-y += -Werror
 endif
 
-obj-$(CONFIG_DRM_TILCDC_SLAVE_COMPAT) += tilcdc_slave_compat.o \
-					 tilcdc_slave_compat.dtb.o
-
 tilcdc-y := \
 	tilcdc_plane.o \
 	tilcdc_crtc.o \
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c b/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c
deleted file mode 100644
index 482299a..0000000
--- a/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c
+++ /dev/null
@@ -1,269 +0,0 @@
-/*
- * Copyright (C) 2015 Texas Instruments
- * Author: Jyri Sarha <jsarha@ti.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published by
- * the Free Software Foundation.
- *
- */
-
-/*
- * To support the old "ti,tilcdc,slave" binding the binding has to be
- * transformed to the new external encoder binding.
- */
-
-#include <linux/kernel.h>
-#include <linux/of.h>
-#include <linux/of_graph.h>
-#include <linux/of_fdt.h>
-#include <linux/slab.h>
-#include <linux/list.h>
-
-#include "tilcdc_slave_compat.h"
-
-struct kfree_table {
-	int total;
-	int num;
-	void **table;
-};
-
-static int __init kfree_table_init(struct kfree_table *kft)
-{
-	kft->total = 32;
-	kft->num = 0;
-	kft->table = kmalloc(kft->total * sizeof(*kft->table),
-			     GFP_KERNEL);
-	if (!kft->table)
-		return -ENOMEM;
-
-	return 0;
-}
-
-static int __init kfree_table_add(struct kfree_table *kft, void *p)
-{
-	if (kft->num == kft->total) {
-		void **old = kft->table;
-
-		kft->total *= 2;
-		kft->table = krealloc(old, kft->total * sizeof(*kft->table),
-				      GFP_KERNEL);
-		if (!kft->table) {
-			kft->table = old;
-			kfree(p);
-			return -ENOMEM;
-		}
-	}
-	kft->table[kft->num++] = p;
-	return 0;
-}
-
-static void __init kfree_table_free(struct kfree_table *kft)
-{
-	int i;
-
-	for (i = 0; i < kft->num; i++)
-		kfree(kft->table[i]);
-
-	kfree(kft->table);
-}
-
-static
-struct property * __init tilcdc_prop_dup(const struct property *prop,
-					 struct kfree_table *kft)
-{
-	struct property *nprop;
-
-	nprop = kzalloc(sizeof(*nprop), GFP_KERNEL);
-	if (!nprop || kfree_table_add(kft, nprop))
-		return NULL;
-
-	nprop->name = kstrdup(prop->name, GFP_KERNEL);
-	if (!nprop->name || kfree_table_add(kft, nprop->name))
-		return NULL;
-
-	nprop->value = kmemdup(prop->value, prop->length, GFP_KERNEL);
-	if (!nprop->value || kfree_table_add(kft, nprop->value))
-		return NULL;
-
-	nprop->length = prop->length;
-
-	return nprop;
-}
-
-static void __init tilcdc_copy_props(struct device_node *from,
-				     struct device_node *to,
-				     const char * const props[],
-				     struct kfree_table *kft)
-{
-	struct property *prop;
-	int i;
-
-	for (i = 0; props[i]; i++) {
-		prop = of_find_property(from, props[i], NULL);
-		if (!prop)
-			continue;
-
-		prop = tilcdc_prop_dup(prop, kft);
-		if (!prop)
-			continue;
-
-		prop->next = to->properties;
-		to->properties = prop;
-	}
-}
-
-static int __init tilcdc_prop_str_update(struct property *prop,
-					  const char *str,
-					  struct kfree_table *kft)
-{
-	prop->value = kstrdup(str, GFP_KERNEL);
-	if (kfree_table_add(kft, prop->value) || !prop->value)
-		return -ENOMEM;
-	prop->length = strlen(str)+1;
-	return 0;
-}
-
-static void __init tilcdc_node_disable(struct device_node *node)
-{
-	struct property *prop;
-
-	prop = kzalloc(sizeof(*prop), GFP_KERNEL);
-	if (!prop)
-		return;
-
-	prop->name = "status";
-	prop->value = "disabled";
-	prop->length = strlen((char *)prop->value)+1;
-
-	of_update_property(node, prop);
-}
-
-static struct device_node * __init tilcdc_get_overlay(struct kfree_table *kft)
-{
-	const int size = __dtb_tilcdc_slave_compat_end -
-		__dtb_tilcdc_slave_compat_begin;
-	static void *overlay_data;
-	struct device_node *overlay;
-	int ret;
-
-	if (!size) {
-		pr_warn("%s: No overlay data\n", __func__);
-		return NULL;
-	}
-
-	overlay_data = kmemdup(__dtb_tilcdc_slave_compat_begin,
-			       size, GFP_KERNEL);
-	if (!overlay_data || kfree_table_add(kft, overlay_data))
-		return NULL;
-
-	of_fdt_unflatten_tree(overlay_data, NULL, &overlay);
-	if (!overlay) {
-		pr_warn("%s: Unfattening overlay tree failed\n", __func__);
-		return NULL;
-	}
-
-	ret = of_resolve_phandles(overlay);
-	if (ret) {
-		pr_err("%s: Failed to resolve phandles: %d\n", __func__, ret);
-		return NULL;
-	}
-
-	return overlay;
-}
-
-static const struct of_device_id tilcdc_slave_of_match[] __initconst = {
-	{ .compatible = "ti,tilcdc,slave", },
-	{},
-};
-
-static const struct of_device_id tilcdc_of_match[] __initconst = {
-	{ .compatible = "ti,am33xx-tilcdc", },
-	{},
-};
-
-static const struct of_device_id tilcdc_tda998x_of_match[] __initconst = {
-	{ .compatible = "nxp,tda998x", },
-	{},
-};
-
-static const char * const tilcdc_slave_props[] __initconst = {
-	"pinctrl-names",
-	"pinctrl-0",
-	"pinctrl-1",
-	NULL
-};
-
-static void __init tilcdc_convert_slave_node(void)
-{
-	struct device_node *slave = NULL, *lcdc = NULL;
-	struct device_node *i2c = NULL, *fragment = NULL;
-	struct device_node *overlay, *encoder;
-	struct property *prop;
-	/* For all memory needed for the overlay tree. This memory can
-	   be freed after the overlay has been applied. */
-	struct kfree_table kft;
-	int ret;
-
-	if (kfree_table_init(&kft))
-		return;
-
-	lcdc = of_find_matching_node(NULL, tilcdc_of_match);
-	slave = of_find_matching_node(NULL, tilcdc_slave_of_match);
-
-	if (!slave || !of_device_is_available(lcdc))
-		goto out;
-
-	i2c = of_parse_phandle(slave, "i2c", 0);
-	if (!i2c) {
-		pr_err("%s: Can't find i2c node trough phandle\n", __func__);
-		goto out;
-	}
-
-	overlay = tilcdc_get_overlay(&kft);
-	if (!overlay)
-		goto out;
-
-	encoder = of_find_matching_node(overlay, tilcdc_tda998x_of_match);
-	if (!encoder) {
-		pr_err("%s: Failed to find tda998x node\n", __func__);
-		goto out;
-	}
-
-	tilcdc_copy_props(slave, encoder, tilcdc_slave_props, &kft);
-
-	for_each_child_of_node(overlay, fragment) {
-		prop = of_find_property(fragment, "target-path", NULL);
-		if (!prop)
-			continue;
-		if (!strncmp("i2c", (char *)prop->value, prop->length))
-			if (tilcdc_prop_str_update(prop, i2c->full_name, &kft))
-				goto out;
-		if (!strncmp("lcdc", (char *)prop->value, prop->length))
-			if (tilcdc_prop_str_update(prop, lcdc->full_name, &kft))
-				goto out;
-	}
-
-	tilcdc_node_disable(slave);
-
-	ret = of_overlay_create(overlay);
-	if (ret)
-		pr_err("%s: Creating overlay failed: %d\n", __func__, ret);
-	else
-		pr_info("%s: ti,tilcdc,slave node successfully converted\n",
-			__func__);
-out:
-	kfree_table_free(&kft);
-	of_node_put(i2c);
-	of_node_put(slave);
-	of_node_put(lcdc);
-	of_node_put(fragment);
-}
-
-static int __init tilcdc_slave_compat_init(void)
-{
-	tilcdc_convert_slave_node();
-	return 0;
-}
-
-subsys_initcall(tilcdc_slave_compat_init);
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.dts b/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.dts
deleted file mode 100644
index 693f8b0..0000000
--- a/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.dts
+++ /dev/null
@@ -1,72 +0,0 @@
-/*
- * DTS overlay for converting ti,tilcdc,slave binding to new binding.
- *
- * Copyright (C) 2015 Texas Instruments Inc.
- * Author: Jyri Sarha <jsarha@ti.com>
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * version 2 as published by the Free Software Foundation.
- */
-
-/*
- * target-path property values are simple tags that are replaced with
- * correct values in tildcdc_slave_compat.c. Some properties are also
- * copied over from the ti,tilcdc,slave node.
- */
-
-/dts-v1/;
-/ {
-	fragment@0 {
-		target-path = "i2c";
-		__overlay__ {
-			#address-cells = <1>;
-			#size-cells = <0>;
-			tda19988 {
-				compatible = "nxp,tda998x";
-				reg = <0x70>;
-				status = "okay";
-
-				port {
-					hdmi_0: endpoint@0 {
-						remote-endpoint = <&lcd_0>;
-					};
-				};
-			};
-		};
-	};
-
-	fragment@1 {
-		target-path = "lcdc";
-		__overlay__ {
-			port {
-				lcd_0: endpoint@0 {
-					remote-endpoint = <&hdmi_0>;
-				};
-			};
-		};
-	};
-
-	__local_fixups__ {
-		fragment@0 {
-			__overlay__ {
-				tda19988 {
-					port {
-						endpoint@0 {
-							remote-endpoint	= <0>;
-						};
-					};
-				};
-			};
-		};
-		fragment@1 {
-			__overlay__ {
-				port {
-					endpoint@0 {
-						remote-endpoint	= <0>;
-					};
-				};
-			};
-		};
-	};
-};
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.h b/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.h
deleted file mode 100644
index 403d35d..0000000
--- a/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.h
+++ /dev/null
@@ -1,25 +0,0 @@
-/*
- * Copyright (C) 2015 Texas Instruments
- * Author: Jyri Sarha <jsarha@ti.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published by
- * the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-/* This header declares the symbols defined in tilcdc_slave_compat.dts */
-
-#ifndef __TILCDC_SLAVE_COMPAT_H__
-#define __TILCDC_SLAVE_COMPAT_H__
-
-extern uint8_t __dtb_tilcdc_slave_compat_begin[];
-extern uint8_t __dtb_tilcdc_slave_compat_end[];
-
-#endif /* __TILCDC_SLAVE_COMPAT_H__ */
-- 
1.9.1

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related

* Re: [PATCH] MIPS: implement a "bootargs-append" DT property
From: Daniel Gimpelevich @ 2017-11-13 12:42 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: James Hogan, Linux MIPS Mailing List, Rob Herring, Frank Rowand,
	devicetree@vger.kernel.org, Linux-Renesas
In-Reply-To: <CAMuHMdUtHhSLbrgmOW7gkEUg8pif+Ddc-zZgWzCZ4WL3JTeOKg@mail.gmail.com>

On Mon, 2017-11-13 at 13:31 +0100, Geert Uytterhoeven wrote:
> I've seen other use cases, e.g. the extension of the du node's
> "clocks" and
> "clock-names" properties from arch/arm64/boot/dts/renesas/r8a7795.dtsi
> to
> arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts.
> 
> To avoid the proliferation of "-append" versions of existing
> properties, what
> about handling this in dtc, by adding support for an
> "/append-property/"
> keyword?

That would not address use case #2 that I mentioned, where the dtb would
have a "bootargs-append" property but no "bootargs" property.

^ permalink raw reply

* Re: [PATCH] MIPS: implement a "bootargs-append" DT property
From: Geert Uytterhoeven @ 2017-11-13 13:18 UTC (permalink / raw)
  To: Daniel Gimpelevich
  Cc: James Hogan, Linux MIPS Mailing List, Rob Herring, Frank Rowand,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux-Renesas
In-Reply-To: <1510576953.9806.1.camel@chimera>

Hi Daniel,

On Mon, Nov 13, 2017 at 1:42 PM, Daniel Gimpelevich
<daniel-R/FLGEdV95bo9U+Z1CfBt0SU0eOFXohjCypLqA8HKkk@public.gmane.org> wrote:
> On Mon, 2017-11-13 at 13:31 +0100, Geert Uytterhoeven wrote:
>> I've seen other use cases, e.g. the extension of the du node's
>> "clocks" and
>> "clock-names" properties from arch/arm64/boot/dts/renesas/r8a7795.dtsi
>> to
>> arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts.
>>
>> To avoid the proliferation of "-append" versions of existing
>> properties, what
>> about handling this in dtc, by adding support for an
>> "/append-property/"
>> keyword?
>
> That would not address use case #2 that I mentioned, where the dtb would
> have a "bootargs-append" property but no "bootargs" property.

For use case #2, you were talking about kernel configuration?
Isn't that something completely different?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] arm: dts: ls1021a: add "fsl, ls1021a-esdhc" compatible string to esdhc node
From: Rasmus Villemoes @ 2017-11-13 14:39 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Russell King
  Cc: devicetree, linux-kernel, linux-arm-kernel, Rasmus Villemoes

Commit a22950c888e3 (mmc: sdhci-of-esdhc: add quirk
SDHCI_QUIRK_BROKEN_TIMEOUT_VAL for ls1021a) added logic to the driver to
enable the broken timeout val quirk for ls1021a, but did not add the
corresponding compatible string to the device tree, so it didn't really
have any effect. Fix that.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 arch/arm/boot/dts/ls1021a.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index 3ff2b8a9f01a..9cdec545decc 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -162,7 +162,7 @@
 		};
 
 		esdhc: esdhc@1560000 {
-			compatible = "fsl,esdhc";
+			compatible = "fsl,ls1021a-esdhc", "fsl,esdhc";
 			reg = <0x0 0x1560000 0x0 0x10000>;
 			interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
 			clock-frequency = <0>;
-- 
2.7.4

^ permalink raw reply related

* Re:
From: Amos Kalonzo @ 2017-11-13 14:42 UTC (permalink / raw)


Attn:

I am wondering why You haven't respond to my email for some days now.
reference to my client's contract balance payment of (11.7M,USD)
Kindly get back to me for more details.

Best Regards

Amos Kalonzo
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] arm: dts: ls1021a: add "fsl, ls1021a-esdhc" compatible string to esdhc node
From: Fabio Estevam @ 2017-11-13 15:16 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Rob Herring, Mark Rutland, Russell King,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <1510583976-6661-1-git-send-email-rasmus.villemoes-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>

On Mon, Nov 13, 2017 at 12:39 PM, Rasmus Villemoes
<rasmus.villemoes-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org> wrote:
> Commit a22950c888e3 (mmc: sdhci-of-esdhc: add quirk
> SDHCI_QUIRK_BROKEN_TIMEOUT_VAL for ls1021a) added logic to the driver to
> enable the broken timeout val quirk for ls1021a, but did not add the
> corresponding compatible string to the device tree, so it didn't really
> have any effect. Fix that.
>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>

Reviewed-by: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH RESEND 1/4] crypto: caam: add caam-dma node to SEC4.0 device tree binding
From: Kim Phillips @ 2017-11-13 15:21 UTC (permalink / raw)
  To: Horia Geantă
  Cc: Radu Andrei Alexe, Vinod Koul, Herbert Xu, David S. Miller,
	Dan Douglass, Shawn Guo, dmaengine@vger.kernel.org,
	linux-crypto@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <VI1PR0402MB3342C1736F492A323096541C982B0@VI1PR0402MB3342.eurprd04.prod.outlook.com>

On Mon, 13 Nov 2017 08:32:24 +0000
Horia Geantă <horia.geanta@nxp.com> wrote:

> On 11/10/2017 6:44 PM, Kim Phillips wrote:
> > On Fri, 10 Nov 2017 08:02:01 +0000
> > Radu Andrei Alexe <radu.alexe@nxp.com> wrote:
> [snip]>> 2. I wanted this driver to be tracked by the dma engine team.
> They have
> >> the right expertise to provide adequate feedback. If all the code was in 
> >> the crypto directory they wouldn't know about this driver or any 
> >> subsequent changes to it.
> > 
> > dma subsystem bits could still be put in the dma area if deemed
> > necessary but I don't think it is: I see
> > drivers/crypto/ccp/ccp-dmaengine.c calls dma_async_device_register for
> > example.
> > 
> Please see previous discussion with Vinod:
> https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg21468.html

Vinod says: "If the dma controller is internal to crypto, then it might
be okay to be inside the crypto driver."

Is that the case for the CCP driver?  Isn't it the case here?

In any case, I don't care that much about that, this all begat from new
*devices* coming out of nowhere.

> > What is the rationale for using the crypto h/w as a dma engine anyway?
> SoCs that don't have a system DMA, for e.g. LS1012A.

OK.

Kim

^ permalink raw reply

* Re: [PATCH RESEND 1/4] crypto: caam: add caam-dma node to SEC4.0 device tree binding
From: Kim Phillips @ 2017-11-13 15:22 UTC (permalink / raw)
  To: Radu Andrei Alexe
  Cc: Horia Geantă, Vinod Koul, Herbert Xu, David S. Miller,
	Dan Douglass, Shawn Guo, dmaengine@vger.kernel.org,
	linux-crypto@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <VI1PR04MB132502335EEB864169926F4E8A2B0@VI1PR04MB1325.eurprd04.prod.outlook.com>

On Mon, 13 Nov 2017 09:44:06 +0000
Radu Andrei Alexe <radu.alexe@nxp.com> wrote:

> On 11/10/2017 6:44 PM, Kim Phillips wrote:
> > On Fri, 10 Nov 2017 08:02:01 +0000
> > Radu Andrei Alexe <radu.alexe@nxp.com> wrote:
> > 
> >> On 11/9/2017 6:34 PM, Kim Phillips wrote:
> >>> On Thu, 9 Nov 2017 11:54:13 +0000
> >>> Radu Andrei Alexe <radu.alexe@nxp.com> wrote:
> >>>> The next patch version will create the platform device dynamically at
> >>>> run time.
> >>>
> >>> Why create a new device when that h/w already has one?
> >>>
> >>> Why doesn't the existing crypto driver register dma capabilities with
> >>> the dma driver subsystem?
> >>>
> >> I can think of two reasons:
> >>
> >> 1. The code that this driver introduces has nothing to do with crypto
> >> and everything to do with dma.
> > 
> > I would think that at least a crypto "null" algorithm implementation
> > would share code.
> >
> >> Placing the code in the same directory as
> >> the caam subsystem would only create confusion for the reader of an
> >> already complex driver.
> > 
> > this different directory argument seems to be identical to your 2 below:
> > 
> >> 2. I wanted this driver to be tracked by the dma engine team. They have
> >> the right expertise to provide adequate feedback. If all the code was in
> >> the crypto directory they wouldn't know about this driver or any
> >> subsequent changes to it.
> > 
> > dma subsystem bits could still be put in the dma area if deemed
> > necessary but I don't think it is: I see
> > drivers/crypto/ccp/ccp-dmaengine.c calls dma_async_device_register for
> > example.
> > 
> > I also don't see how that complicates things much further.
> > 
> 
> So who made their review? The guys from crypto?

Don't see how that's relevant here, but people applying patches should
solicit acks from the appropriate sources, esp. if a patch is across
multiple subsystems.

> If someone wants to enable only the DMA functionality of the CCP and not 
> the crypto part how do they do it? Look for it in the crypto submenu?

Why would they want to do that?

In any case, I suspect you're thinking about cross-subsystem Kconfig
entries, which is common, but something like that can be a module
parameter, too.

I would say that maybe CRYPTO_DEV_FSL_CAAM should be made to not depend
on CRYPTO_HW, but I think that's overkill for the addition of this
minor feature.

> > What is the rationale for using the crypto h/w as a dma engine anyway?
> > Are there supporting performance figures?
> 
> We have a platform that doesn't have a dedicated DMA controller but has 
> the CAAM hardware block that can perform dma transfers.  We have a

OK, please mention that next time.

> use-case where we need to issue large transfers (hundred of MBs) 
> asynchronously, without using the core.

Curious: what subsystem does that?

Thanks,

Kim

^ permalink raw reply

* Re: [PATCH 3/5] dt-bindings: arm: Document Socionext MB86S71 and Fujitsu F-Cue
From: Andreas Färber @ 2017-11-13 15:40 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Leif Lindholm, Grant Likely, Masahiro Yamada, Satoru OKAMOTO,
	Arnd Bergmann, Olof Johansson, Mark Rutland, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux Kernel Mailing List, Amit Kucheria, linux-arm-kernel,
	Yang Zhang
In-Reply-To: <CAKv+Gu_Wkdd3EdOjKJYZN5Ypjodpu5kFU=Ubg9H5e6ER3zq8Mg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Am 06.11.2017 um 12:28 schrieb Ard Biesheuvel:
> On 6 November 2017 at 06:58, Andreas Färber <afaerber-l3A5Bk7waGM@public.gmane.org> wrote:
>> Am 05.11.2017 um 04:39 schrieb Ard Biesheuvel:
> [...]
>>>
>>> Again, I am not the one who is ranting here. You hit a nerve by
>>> accusing me of 'rebelling against linux.git' while this is quite the
>>> opposite of what I am doing.
>>
>> Actually you did confirm that point by starting an argument about not
>> needing a central repository and you not liking Linux as the location.
>> That was exactly what I meant with my original comment.
>>
>> Adding Actions Semi was somewhat easy as a new vendor and now - roughly
>> a year after the board went to market - there's Linaro contributions
>> from Mani that I'm thankful for.
>>
>> Whereas patches keep falling into a dark hole when there's already other
>> work for a certain vendor, such as Marvell and now Socionext, with no
>> one feeling responsible for either taking them or saying, "hey, we're
>> not going to submit any conflicting DT bindings for SynQuacer because we
>> use ACPI, so please go ahead with proposal X, thanks for your efforts".
>>
>> Don't complain about me ranting if you belittle my volunteer work that I
>> believe Linaro and its partners should've done in the first place: If I
>> can get an initial mainline PoC done as an individual on a few
>> evenings/weekends, then the same should be super-easy for an
>> organization with lots of engineers and paying member companies.
> 
> The only person doing the ranting, rebelling and belittling in this
> thread is you. I have never commented on the nature of your work, let
> alone belittle it.

You have stated your opinion that Device Trees don't belong in a central
repository and that Linux was the wrong place for them. My contributions
to Linux have been mainly such Device Trees and bindings, such as this
patch series here. Quod erat demonstrandum.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/2] can: flexcan: Remodel FlexCAN register r/w APIs for big endian FlexCAN controllers.
From: Marc Kleine-Budde @ 2017-11-13 15:50 UTC (permalink / raw)
  To: Pankaj Bansal, wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org,
	linux-can-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
  Cc: Varun Sethi, Poonam Aggrwal,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <AM0PR0402MB3940DE05B2BA456D0FF54498F1540-mYCQpYF9suc3mfjNbz3WnI3W/0Ik+aLCnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 2323 bytes --]

On 11/10/2017 05:32 PM, Pankaj Bansal wrote:
> Thanks for sharing the video and slides. It was really helpful. But I
> would still like to solve this problem using device tree property. My
> rationale behind it is that, if a platform designer uses same IP
> block whose support is present is linux kernel, but with different
> endianness. Then he would not need to add his platform data in each
> driver to support his platform in linux. He can just add endianness
> property in device tree and can use latest linux kernel right off the
> bat. This is also mentioned in
> "Documentation/devicetree/bindings/regmap/regmap.txt".
> 
> Regmap defaults to little-endian register access on MMIO based
> devices, this is by far the most common setting. On CPU
> architectures that typically run big-endian operating systems
> (e.g. PowerPC), registers can be defined as big-endian and must
> be marked that way in the devicetree.
> 
> This rule was apparently not followed in P1010RDB flexcan node.
> 
> To solve this problem, I suggest that we define 2 optional device
> tree properties for flexcan. little-endian : for powerpc
> architecture, if this property is defined then controller is little
> endian otherwise big endian (default) big-endian : for other
> architectures, if this property is defined then controller is big
> endian otherwise little endian (default)
> 
> Although the controller drivers should be architecture independent,
> but apparently there is no way around it in flexcan.

Please keep the endianess as default es stated in the comment in the driver:

>> /* Abstract off the read/write for arm versus ppc. This
>>  * assumes that PPC uses big-endian registers and everything
>>  * else uses little-endian registers, independent of CPU
>>  * endianness.
>>  */

See description of commit 0e4b949e6620 ("can: flexcan: fix flexcan
driver build for big endian on ARM and little endian on PowerPc") for
more information.

I'll not ACK a change in the driver, that's breaking PPC.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH 3/5] dt-bindings: arm: Document Socionext MB86S71 and Fujitsu F-Cue
From: Ard Biesheuvel @ 2017-11-13 15:55 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Leif Lindholm, Grant Likely, Masahiro Yamada, Satoru OKAMOTO,
	Arnd Bergmann, Olof Johansson, Mark Rutland, Rob Herring,
	devicetree@vger.kernel.org, Linux Kernel Mailing List,
	Amit Kucheria, linux-arm-kernel, Yang Zhang
In-Reply-To: <4c5fdcbd-057f-12a2-55f4-ff5f85b80f64@suse.de>

On 13 November 2017 at 15:40, Andreas Färber <afaerber@suse.de> wrote:
> Am 06.11.2017 um 12:28 schrieb Ard Biesheuvel:
>> On 6 November 2017 at 06:58, Andreas Färber <afaerber@suse.de> wrote:
>>> Am 05.11.2017 um 04:39 schrieb Ard Biesheuvel:
>> [...]
>>>>
>>>> Again, I am not the one who is ranting here. You hit a nerve by
>>>> accusing me of 'rebelling against linux.git' while this is quite the
>>>> opposite of what I am doing.
>>>
>>> Actually you did confirm that point by starting an argument about not
>>> needing a central repository and you not liking Linux as the location.
>>> That was exactly what I meant with my original comment.
>>>
>>> Adding Actions Semi was somewhat easy as a new vendor and now - roughly
>>> a year after the board went to market - there's Linaro contributions
>>> from Mani that I'm thankful for.
>>>
>>> Whereas patches keep falling into a dark hole when there's already other
>>> work for a certain vendor, such as Marvell and now Socionext, with no
>>> one feeling responsible for either taking them or saying, "hey, we're
>>> not going to submit any conflicting DT bindings for SynQuacer because we
>>> use ACPI, so please go ahead with proposal X, thanks for your efforts".
>>>
>>> Don't complain about me ranting if you belittle my volunteer work that I
>>> believe Linaro and its partners should've done in the first place: If I
>>> can get an initial mainline PoC done as an individual on a few
>>> evenings/weekends, then the same should be super-easy for an
>>> organization with lots of engineers and paying member companies.
>>
>> The only person doing the ranting, rebelling and belittling in this
>> thread is you. I have never commented on the nature of your work, let
>> alone belittle it.
>
> You have stated your opinion that Device Trees don't belong in a central
> repository and that Linux was the wrong place for them.

Not as strongly as that, but ok.

> My contributions
> to Linux have been mainly such Device Trees and bindings, such as this
> patch series here.

Again, I don't have a clue what it is you work on, although I just
found out (from the other thread you started) that it involves a
Fujitsu not-quite-96board that shares IP with the SynQuacer SoC? It
was my understanding (from information I received from Socionext) that
any upstreaming efforts involving that SoC had been discarded. I guess
the Socionext and Fujitsu engineers are not talking to each other
either.

> Quod erat demonstrandum.

Mathematical proof usually involves inferring new facts from existing
facts. You have done nothing of the kind, and I am not sure what you
are so angry about, but I think it would be better to leave the
emotions out of it, and try to remain factual. Especially when it
comes to representing other people's statements.

-- 
Ard.

^ permalink raw reply

* Re: [PATCH] MIPS: implement a "bootargs-append" DT property
From: Rob Herring @ 2017-11-13 16:34 UTC (permalink / raw)
  To: James Hogan
  Cc: Daniel Gimpelevich, Linux-MIPS, Frank Rowand,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Geert Uytterhoeven
In-Reply-To: <20171113112312.GZ15260@jhogan-linux>

On Mon, Nov 13, 2017 at 5:23 AM, James Hogan <james.hogan-8NJIiSa5LzA@public.gmane.org> wrote:
> On Sat, Nov 11, 2017 at 09:19:48AM -0800, Daniel Gimpelevich wrote:
>> There are two uses for this:
>>
>> 1) It may be useful to split a device-specific kernel command line between
>> a .dts file and a .dtsi file, with "bootargs" in one and "bootargs-append"
>> in the other, such as for variations of a reference board.

What Geert said.

>> 2) There are kernel configuration options for prepending "bootargs" to the
>> kernel command line that the bootloader has passed, but not for appending.
>> A new option for this would be a less future-proof solution, since things
>> like this should be in the dtb.

This is a kernel problem. What's the use case where you want the DT to
override the kernel?

One way you could handle this is make bootargs be multiple strings.
Well 2 specifically, the first string is prepended and the 2nd is
appended. That complicates how you'd implement /append-property/
though as you'd probably want to support both string cat and 2
strings. Though the latter works more generically without knowing the
data type.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: et8ek8: Document support for flash and lens devices
From: Sakari Ailus @ 2017-11-13 17:05 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Pavel Machek, Sakari Ailus, linux-media, niklas.soderlund,
	maxime.ripard, hverkuil, laurent.pinchart, linux-acpi, devicetree
In-Reply-To: <20171112142547.km6haalndqiohoq2@earth>

On Sun, Nov 12, 2017 at 03:25:47PM +0100, Sebastian Reichel wrote:
> Hi,
> 
> On Sun, Nov 12, 2017 at 12:27:29PM +0100, Pavel Machek wrote:
> > 
> > Document dts support of LED/focus.
> > 
> > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

Thanks, guys!

Applied, with "dt-bindings: " prefix added to the subject.

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi

^ permalink raw reply

* Re: [PATCH v5 2/2] memory: ti-emif-sram: introduce relocatable suspend/resume handlers
From: Santosh Shilimkar @ 2017-11-13 17:07 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: devicetree, Dave Gerlach, Tony Lindgren, Keerthy J, linux-kernel,
	Johan Hovold, Rob Herring, Santosh Shilimkar, linux-omap,
	linux-arm-kernel
In-Reply-To: <20171110163922.GK28231@n2100.armlinux.org.uk>

On 11/10/2017 8:39 AM, Russell King - ARM Linux wrote:
> On Mon, Oct 16, 2017 at 08:31:59AM -0700, Santosh Shilimkar wrote:
>> Hi Russell,
>>
>> On 10/13/2017 1:59 PM, Dave Gerlach wrote:
>>> Certain SoCs like Texas Instruments AM335x and AM437x require parts
>>> of the EMIF PM code to run late in the suspend sequence from SRAM,
>>> such as saving and restoring the EMIF context and placing the memory
>>> into self-refresh.
>>>
>>> One requirement for these SoCs to suspend and enter its lowest power
>>> mode, called DeepSleep0, is that the PER power domain must be shut off.
>>> Because the EMIF (DDR Controller) resides within this power domain, it
>>> will lose context during a suspend operation, so we must save it so we
>>> can restore once we resume. However, we cannot execute this code from
>>> external memory, as it is not available at this point, so the code must
>>> be executed late in the suspend path from SRAM.
>>>
>>> This patch introduces a ti-emif-sram driver that includes several
>>> functions written in ARM ASM that are relocatable so the PM SRAM
>>> code can use them. It also allocates a region of writable SRAM to
>>> be used by the code running in the executable region of SRAM to save
>>> and restore the EMIF context. It can export a table containing the
>>> absolute addresses of the available PM functions so that other SRAM
>>> code can branch to them. This code is required for suspend/resume on
>>> AM335x and AM437x to work.
>>>
>>> In addition to this, to be able to share data structures between C and
>>> the ti-emif-sram-pm assembly code, we can automatically generate all of
>>> the C struct member offsets and sizes as macros by processing
>>> emif-asm-offsets.c into assembly code and then extracting the relevant
>>> data as is done for the generated platform asm-offsets.h files.
>>>
>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>> ---
>>> v4->v5:
>>> * Simplify Makefile to generate include/generated/ti-emif-asm-offsets.h
>>>
>> If you are happy with this version from Dave, can I get your ack please?
>> I can add this to the driver-soc then.
> 
> Does it need my ack anymore?  It doesn't touch anything in arch/arm.
> If you still want it, then... notwithstanding the loss of type checking
> that fncpy() gave and the new sram stuff messes up...
> 
> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
> 
Thanks Russell !!

^ permalink raw reply

* Re: [PATCH] ARM: dts: keystone-k2l: Add the second gpio bank node
From: Santosh Shilimkar @ 2017-11-13 17:12 UTC (permalink / raw)
  To: Keerthy, ssantosh
  Cc: devicetree, linux-kernel, linux, t-kristo, robh+dt, russ.dill,
	linux-arm-kernel
In-Reply-To: <1510227778-20491-1-git-send-email-j-keerthy@ti.com>

On 11/9/2017 3:42 AM, Keerthy wrote:
> In case of k2l there are 2 more banks with 16 pins each.
> Adding the node as the da-vinci driver now supports multiple
> banks.
> 
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
> 
> This is based on linux-next branch. Boot tested on keystone-k2l-evm.
> 
Will pick this up Keerthy !!

Regards,
Santosh

^ permalink raw reply

* [PATCH  0/3] ARM: dts: Add PCIEC support
From: Biju Das @ 2017-11-13 17:43 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland
  Cc: Simon Horman, Magnus Damm, Chris Paterson, Fabrizio Castro,
	devicetree, linux-renesas-soc, Biju Das

Hello,

This series aims to add PCIEC support for iWave RZ/G1M (R8A7743) board.

This patch has documentation dependency on below patch

* dt-bindings: PCI: rcar: Add device tree support for r8a7743
https://patchwork.kernel.org/patch/10056407/

This patch series has dependency on below patches

* ARM: dts: r8a7743: Add CAN[01] SoC support
https://patchwork.kernel.org/patch/10046875/

* ARM: dts: iwg20d-q7-common: Add can0 support to carrier board
https://patchwork.kernel.org/patch/10046879/

Biju Das (3):
  ARM: dts: r8a7743: Add default PCIe bus clock
  ARM: dts: r8a7743: Add PCIe Controller device node
  ARM: dts: iwg20d-q7: Enable PCIe Controller

 arch/arm/boot/dts/iwg20d-q7-common.dtsi |  8 ++++++++
 arch/arm/boot/dts/r8a7743.dtsi          | 35 +++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

-- 
1.9.1

^ 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