Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: dts: imx8mn: Add gpio-ranges property
From: Anson.Huang @ 2019-08-15  2:57 UTC (permalink / raw)
  To: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam,
	leonard.crestez, abel.vesa, daniel.baluta, jun.li, devicetree,
	linux-arm-kernel, linux-kernel
  Cc: Linux-imx

From: Anson Huang <Anson.Huang@nxp.com>

Add "gpio-ranges" property to establish connections between GPIOs
and PINs on i.MX8MN pinctrl driver.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mn.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index f5eff35..1d8899b 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -173,6 +173,7 @@
 				#gpio-cells = <2>;
 				interrupt-controller;
 				#interrupt-cells = <2>;
+				gpio-ranges = <&iomuxc 0 10 30>;
 			};
 
 			gpio2: gpio@30210000 {
@@ -185,6 +186,7 @@
 				#gpio-cells = <2>;
 				interrupt-controller;
 				#interrupt-cells = <2>;
+				gpio-ranges = <&iomuxc 0 40 21>;
 			};
 
 			gpio3: gpio@30220000 {
@@ -197,6 +199,7 @@
 				#gpio-cells = <2>;
 				interrupt-controller;
 				#interrupt-cells = <2>;
+				gpio-ranges = <&iomuxc 0 61 26>;
 			};
 
 			gpio4: gpio@30230000 {
@@ -209,6 +212,7 @@
 				#gpio-cells = <2>;
 				interrupt-controller;
 				#interrupt-cells = <2>;
+				gpio-ranges = <&iomuxc 21 108 11>;
 			};
 
 			gpio5: gpio@30240000 {
@@ -221,6 +225,7 @@
 				#gpio-cells = <2>;
 				interrupt-controller;
 				#interrupt-cells = <2>;
+				gpio-ranges = <&iomuxc 0 119 30>;
 			};
 
 			wdog1: watchdog@30280000 {
-- 
2.7.4


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

^ permalink raw reply related

* [PATCH] arm: dts: rockchip: fix vcc_host_5v regulator for usb3 host
From: Kever Yang @ 2019-08-15  2:59 UTC (permalink / raw)
  To: heiko
  Cc: Mark Rutland, devicetree, Jonas Karlman, Katsuhiro Suzuki,
	Kever Yang, linux-kernel, linux-rockchip, Chen-Yu Tsai,
	Rob Herring, Tomohiro Mayama, linux-arm-kernel

According to rock64 schemetic V2 and V3, the VCC_HOST_5V output is
controlled by USB_20_HOST_DRV, which is the same as VCC_HOST1_5V.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

 arch/arm64/boot/dts/rockchip/rk3328-rock64.dts | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
index 7cfd5ca6cc85..bd4ad1635e0b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
@@ -35,9 +35,9 @@
 	vcc_host_5v: vcc-host-5v-regulator {
 		compatible = "regulator-fixed";
 		enable-active-high;
-		gpio = <&gpio0 RK_PA0 GPIO_ACTIVE_HIGH>;
+		gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>;
 		pinctrl-names = "default";
-		pinctrl-0 = <&usb30_host_drv>;
+		pinctrl-0 = <&usb20_host_drv>;
 		regulator-name = "vcc_host_5v";
 		regulator-always-on;
 		regulator-boot-on;
@@ -320,12 +320,6 @@
 			rockchip,pins = <0 RK_PA2 RK_FUNC_GPIO &pcfg_pull_none>;
 		};
 	};
-
-	usb3 {
-		usb30_host_drv: usb30-host-drv {
-			rockchip,pins = <0 RK_PA0 RK_FUNC_GPIO &pcfg_pull_none>;
-		};
-	};
 };
 
 &sdmmc {
-- 
2.17.1


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

^ permalink raw reply related

* Re: [PATCH v2 0/2] drm/mediatek: make imported PRIME buffers contiguous
From: CK Hu @ 2019-08-15  1:40 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: Philipp Zabel, David Airlie, linux-kernel, dri-devel, Tomasz Figa,
	linux-mediatek, Daniel Vetter, Matthias Brugger, linux-arm-kernel
In-Reply-To: <20190729053335.251379-1-acourbot@chromium.org>

Hi, Alexandre:

On Mon, 2019-07-29 at 14:33 +0900, Alexandre Courbot wrote:
> The default DMA segment size was used when importing PRIME buffers,
> which resulted in a chance of them not being contiguous in the virtual
> IO space of the device and mtk_gem_prime_import_sg_table() complaining
> that the SG table was not contiguous as it expects.
> 
> This series fixes this issue by
> 
> 1) Using the correct DMA device when importing PRIME buffers,
> 2) Setting a more suitable DMA segment size on the DMA device than the
> default 64KB.

For the series, applied to mediatek-drm-fixes-5.3 [1], thanks.

[1]
https://github.com/ckhu-mediatek/linux.git-tags/commits/mediatek-drm-fixes-5.3

> 
> Changes since v1:
> - Split into two patches,
> - Fixed an error path that would have returned 0.
> 
> Alexandre Courbot (2):
>   drm/mediatek: use correct device to import PRIME buffers
>   drm/mediatek: set DMA max segment size
> 
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 49 ++++++++++++++++++++++++--
>  drivers/gpu/drm/mediatek/mtk_drm_drv.h |  2 ++
>  2 files changed, 48 insertions(+), 3 deletions(-)
> 



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

^ permalink raw reply

* Re: kexec on rk3399
From: Vicente Bergas @ 2019-08-15  1:15 UTC (permalink / raw)
  To: Robin Murphy, Felipe Balbi
  Cc: Matthias Brugger, Heiko Stuebner, Marc Zyngier, Catalin Marinas,
	linux-usb, Will Deacon, linux-kernel, linux-rockchip,
	Greg Kroah-Hartman, linux-arm-kernel
In-Reply-To: <0408cb6c-1b16-eacb-d47e-17f4ff89e2b8@arm.com>

On Wednesday, August 14, 2019 3:12:26 PM CEST, Robin Murphy wrote:
> On 14/08/2019 13:53, Vicente Bergas wrote:
>> On Monday, July 22, 2019 4:31:27 PM CEST, Vicente Bergas wrote: ...
>
> This particular change looks like it's implicitly specific to 
> RK3399, which wouldn't be ideal. Presumably if the core dwc3 
> driver implemented shutdown correctly (echoing parts of 
> dwc3_remove(), I guess) then the glue layers shouldn't need 
> anything special anyway.
>
> Robin.

I just checked simple->resets from dwc3-of-simple.c and it is an array
with multiple resets whereas dwc->reset from core.c is NULL.
So the reset seems specific to the glue layers.
Is there another way than resetting the thing that is
generic enough to go to core.c and allows kexec?

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

^ permalink raw reply

* Re: [PATCH v8 05/14] media: rkisp1: add Rockchip ISP1 subdev driver
From: Helen Koike @ 2019-08-15  0:58 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: devicetree, eddie.cai.linux, kernel, heiko, jacob2.chen,
	jeffy.chen, zyc, linux-kernel, tfiga, linux-rockchip, Allon Huang,
	Jacob Chen, hans.verkuil, laurent.pinchart, zhengsq, mchehab,
	ezequiel, linux-arm-kernel, linux-media
In-Reply-To: <20190808091406.GQ21370@paasikivi.fi.intel.com>

Hi Sakari,

Thanks for your review. I just have some comments/questions below.

On 8/8/19 6:14 AM, Sakari Ailus wrote:
> Hi Helen,
> 
> On Tue, Jul 30, 2019 at 03:42:47PM -0300, Helen Koike wrote:
>> From: Jacob Chen <jacob2.chen@rock-chips.com>
>>
>> Add the subdev driver for rockchip isp1.
>>
>> Signed-off-by: Jacob Chen <jacob2.chen@rock-chips.com>
>> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
>> Signed-off-by: Yichong Zhong <zyc@rock-chips.com>
>> Signed-off-by: Jacob Chen <cc@rock-chips.com>
>> Signed-off-by: Eddie Cai <eddie.cai.linux@gmail.com>
>> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
>> Signed-off-by: Allon Huang <allon.huang@rock-chips.com>
>> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
>> [fixed unknown entity type / switched to PIXEL_RATE]
>> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
>> [update for upstream]
>> Signed-off-by: Helen Koike <helen.koike@collabora.com>
>>
>> ---
>>
>> Changes in v8: None
>> Changes in v7:
>> - fixed warning because of unknown entity type
>> - fixed v4l2-compliance errors regarding rkisp1 formats, try formats
>> and default values
>> - fix typo riksp1/rkisp1
>> - redesign: remove mipi/csi subdevice, sensors connect directly to the
>> isp subdevice in the media topology now. As a consequence, remove the
>> hack in mipidphy_g_mbus_config() where information from the sensor was
>> being propagated through the topology.
>> - From the old dphy:
>>         * cache get_remote_sensor() in s_stream
>>         * use V4L2_CID_PIXEL_RATE instead of V4L2_CID_LINK_FREQ
>> - Replace stream state with a boolean
>> - code styling and checkpatch fixes
>> - fix stop_stream (return after calling stop, do not reenable the stream)
>> - fix rkisp1_isp_sd_get_selection when V4L2_SUBDEV_FORMAT_TRY is set
>> - fix get format in output (isp_sd->out_fmt.mbus_code was being ignored)
>> - s/intput/input
>> - remove #define sd_to_isp_sd(_sd), add a static inline as it will be
>> reused by the capture
>>
>>  drivers/media/platform/rockchip/isp1/rkisp1.c | 1286 +++++++++++++++++
>>  drivers/media/platform/rockchip/isp1/rkisp1.h |  111 ++
>>  2 files changed, 1397 insertions(+)
>>  create mode 100644 drivers/media/platform/rockchip/isp1/rkisp1.c
>>  create mode 100644 drivers/media/platform/rockchip/isp1/rkisp1.h
>>
>> diff --git a/drivers/media/platform/rockchip/isp1/rkisp1.c b/drivers/media/platform/rockchip/isp1/rkisp1.c
>> new file mode 100644
>> index 000000000000..6d0c0ffb5e03
>> --- /dev/null
>> +++ b/drivers/media/platform/rockchip/isp1/rkisp1.c
>> @@ -0,0 +1,1286 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Rockchip isp1 driver
>> + *
>> + * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
>> + */
>> +
>> +#include <linux/iopoll.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/phy/phy-mipi-dphy.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/videodev2.h>
>> +#include <linux/vmalloc.h>
>> +#include <media/v4l2-event.h>
>> +
>> +#include "common.h"
>> +#include "regs.h"
>> +
>> +#define CIF_ISP_INPUT_W_MAX		4032
>> +#define CIF_ISP_INPUT_H_MAX		3024
>> +#define CIF_ISP_INPUT_W_MIN		32
>> +#define CIF_ISP_INPUT_H_MIN		32
>> +#define CIF_ISP_OUTPUT_W_MAX		CIF_ISP_INPUT_W_MAX
>> +#define CIF_ISP_OUTPUT_H_MAX		CIF_ISP_INPUT_H_MAX
>> +#define CIF_ISP_OUTPUT_W_MIN		CIF_ISP_INPUT_W_MIN
>> +#define CIF_ISP_OUTPUT_H_MIN		CIF_ISP_INPUT_H_MIN
>> +
>> +/*
>> + * NOTE: MIPI controller and input MUX are also configured in this file,
>> + * because ISP Subdev is not only describe ISP submodule(input size,format,
>> + * output size, format), but also a virtual route device.
>> + */
>> +
>> +/*
>> + * There are many variables named with format/frame in below code,
>> + * please see here for their meaning.
>> + *
>> + * Cropping regions of ISP
>> + *
>> + * +---------------------------------------------------------+
>> + * | Sensor image                                            |
>> + * | +---------------------------------------------------+   |
>> + * | | ISP_ACQ (for black level)                         |   |
>> + * | | in_frm                                            |   |
>> + * | | +--------------------------------------------+    |   |
>> + * | | |    ISP_OUT                                 |    |   |
>> + * | | |    in_crop                                 |    |   |
>> + * | | |    +---------------------------------+     |    |   |
>> + * | | |    |   ISP_IS                        |     |    |   |
>> + * | | |    |   rkisp1_isp_subdev: out_crop   |     |    |   |
>> + * | | |    +---------------------------------+     |    |   |
>> + * | | +--------------------------------------------+    |   |
>> + * | +---------------------------------------------------+   |
>> + * +---------------------------------------------------------+
>> + */
>> +
>> +static inline struct rkisp1_device *sd_to_isp_dev(struct v4l2_subdev *sd)
>> +{
>> +	return container_of(sd->v4l2_dev, struct rkisp1_device, v4l2_dev);
>> +}
>> +
>> +/* Get sensor by enabled media link */
>> +static struct v4l2_subdev *get_remote_sensor(struct v4l2_subdev *sd)
>> +{
>> +	struct media_pad *local, *remote;
>> +	struct media_entity *sensor_me;
>> +
>> +	local = &sd->entity.pads[RKISP1_ISP_PAD_SINK];
>> +	remote = media_entity_remote_pad(local);
>> +	if (!remote) {
>> +		v4l2_warn(sd, "No link between isp and sensor\n");
>> +		return NULL;
>> +	}
>> +
>> +	sensor_me = media_entity_remote_pad(local)->entity;
>> +	return media_entity_to_v4l2_subdev(sensor_me);
>> +}
>> +
>> +static struct rkisp1_sensor *sd_to_sensor(struct rkisp1_device *dev,
>> +					       struct v4l2_subdev *sd)
> 
> Indentation.
> 
>> +{
>> +	struct rkisp1_sensor *sensor;
>> +
>> +	list_for_each_entry(sensor, &dev->sensors, list)
>> +		if (sensor->sd == sd)
>> +			return sensor;
>> +
>> +	return NULL;
>> +}
>> +
>> +/****************  register operations ****************/
>> +
>> +/*
>> + * Image Stabilization.
>> + * This should only be called when configuring CIF
>> + * or at the frame end interrupt
>> + */
>> +static void rkisp1_config_ism(struct rkisp1_device *dev)
>> +{
>> +	void __iomem *base = dev->base_addr;
>> +	struct v4l2_rect *out_crop = &dev->isp_sdev.out_crop;
>> +	u32 val;
>> +
>> +	writel(0, base + CIF_ISP_IS_RECENTER);
>> +	writel(0, base + CIF_ISP_IS_MAX_DX);
>> +	writel(0, base + CIF_ISP_IS_MAX_DY);
>> +	writel(0, base + CIF_ISP_IS_DISPLACE);
>> +	writel(out_crop->left, base + CIF_ISP_IS_H_OFFS);
>> +	writel(out_crop->top, base + CIF_ISP_IS_V_OFFS);
>> +	writel(out_crop->width, base + CIF_ISP_IS_H_SIZE);
>> +	writel(out_crop->height, base + CIF_ISP_IS_V_SIZE);
>> +
>> +	/* IS(Image Stabilization) is always on, working as output crop */
>> +	writel(1, base + CIF_ISP_IS_CTRL);
>> +	val = readl(base + CIF_ISP_CTRL);
>> +	val |= CIF_ISP_CTRL_ISP_CFG_UPD;
>> +	writel(val, base + CIF_ISP_CTRL);
>> +}
>> +
>> +/*
>> + * configure isp blocks with input format, size......
>> + */
>> +static int rkisp1_config_isp(struct rkisp1_device *dev)
>> +{
>> +	u32 isp_ctrl = 0, irq_mask = 0, acq_mult = 0, signal = 0;
>> +	struct v4l2_rect *out_crop, *in_crop;
>> +	void __iomem *base = dev->base_addr;
>> +	struct v4l2_mbus_framefmt *in_frm;
>> +	struct ispsd_out_fmt *out_fmt;
>> +	struct rkisp1_sensor *sensor;
>> +	struct ispsd_in_fmt *in_fmt;
>> +
>> +	sensor = dev->active_sensor;
>> +	in_frm = &dev->isp_sdev.in_frm;
>> +	in_fmt = &dev->isp_sdev.in_fmt;
>> +	out_fmt = &dev->isp_sdev.out_fmt;
>> +	out_crop = &dev->isp_sdev.out_crop;
>> +	in_crop = &dev->isp_sdev.in_crop;
>> +
>> +	if (in_fmt->fmt_type == FMT_BAYER) {
>> +		acq_mult = 1;
>> +		if (out_fmt->fmt_type == FMT_BAYER) {
>> +			if (sensor->mbus.type == V4L2_MBUS_BT656)
>> +				isp_ctrl =
>> +					CIF_ISP_CTRL_ISP_MODE_RAW_PICT_ITU656;
>> +			else
>> +				isp_ctrl =
>> +					CIF_ISP_CTRL_ISP_MODE_RAW_PICT;
>> +		} else {
>> +			writel(CIF_ISP_DEMOSAIC_TH(0xc),
>> +			       base + CIF_ISP_DEMOSAIC);
>> +
>> +			if (sensor->mbus.type == V4L2_MBUS_BT656)
>> +				isp_ctrl = CIF_ISP_CTRL_ISP_MODE_BAYER_ITU656;
>> +			else
>> +				isp_ctrl = CIF_ISP_CTRL_ISP_MODE_BAYER_ITU601;
>> +		}
>> +	} else if (in_fmt->fmt_type == FMT_YUV) {
>> +		acq_mult = 2;
>> +		if (sensor->mbus.type == V4L2_MBUS_CSI2_DPHY) {
>> +			isp_ctrl = CIF_ISP_CTRL_ISP_MODE_ITU601;
>> +		} else {
>> +			if (sensor->mbus.type == V4L2_MBUS_BT656)
>> +				isp_ctrl = CIF_ISP_CTRL_ISP_MODE_ITU656;
>> +			else
>> +				isp_ctrl = CIF_ISP_CTRL_ISP_MODE_ITU601;
>> +
>> +		}
>> +
>> +		irq_mask |= CIF_ISP_DATA_LOSS;
>> +	}
>> +
>> +	/* Set up input acquisition properties */
>> +	if (sensor->mbus.type == V4L2_MBUS_BT656 ||
>> +	    sensor->mbus.type == V4L2_MBUS_PARALLEL) {
>> +		if (sensor->mbus.flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
>> +			signal = CIF_ISP_ACQ_PROP_POS_EDGE;
>> +	}
>> +
>> +	if (sensor->mbus.type == V4L2_MBUS_PARALLEL) {
>> +		if (sensor->mbus.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)
>> +			signal |= CIF_ISP_ACQ_PROP_VSYNC_LOW;
>> +
>> +		if (sensor->mbus.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW)
>> +			signal |= CIF_ISP_ACQ_PROP_HSYNC_LOW;
>> +	}
>> +
>> +	writel(isp_ctrl, base + CIF_ISP_CTRL);
>> +	writel(signal | in_fmt->yuv_seq |
>> +	       CIF_ISP_ACQ_PROP_BAYER_PAT(in_fmt->bayer_pat) |
>> +	       CIF_ISP_ACQ_PROP_FIELD_SEL_ALL, base + CIF_ISP_ACQ_PROP);
>> +	writel(0, base + CIF_ISP_ACQ_NR_FRAMES);
>> +
>> +	/* Acquisition Size */
>> +	writel(0, base + CIF_ISP_ACQ_H_OFFS);
>> +	writel(0, base + CIF_ISP_ACQ_V_OFFS);
>> +	writel(acq_mult * in_frm->width, base + CIF_ISP_ACQ_H_SIZE);
>> +	writel(in_frm->height, base + CIF_ISP_ACQ_V_SIZE);
>> +
>> +	/* ISP Out Area */
>> +	writel(in_crop->left, base + CIF_ISP_OUT_H_OFFS);
>> +	writel(in_crop->top, base + CIF_ISP_OUT_V_OFFS);
>> +	writel(in_crop->width, base + CIF_ISP_OUT_H_SIZE);
>> +	writel(in_crop->height, base + CIF_ISP_OUT_V_SIZE);
>> +
>> +	/* interrupt mask */
>> +	irq_mask |= CIF_ISP_FRAME | CIF_ISP_V_START | CIF_ISP_PIC_SIZE_ERROR |
>> +		    CIF_ISP_FRAME_IN;
>> +	writel(irq_mask, base + CIF_ISP_IMSC);
>> +
>> +	if (out_fmt->fmt_type == FMT_BAYER)
>> +		rkisp1_params_disable_isp(&dev->params_vdev);
>> +	else
>> +		rkisp1_params_configure_isp(&dev->params_vdev, in_fmt,
>> +					    dev->isp_sdev.quantization);
>> +
>> +	return 0;
>> +}
>> +
>> +static int rkisp1_config_dvp(struct rkisp1_device *dev)
>> +{
>> +	struct ispsd_in_fmt *in_fmt = &dev->isp_sdev.in_fmt;
>> +	void __iomem *base = dev->base_addr;
>> +	u32 val, input_sel;
>> +
>> +	switch (in_fmt->bus_width) {
>> +	case 8:
>> +		input_sel = CIF_ISP_ACQ_PROP_IN_SEL_8B_ZERO;
>> +		break;
>> +	case 10:
>> +		input_sel = CIF_ISP_ACQ_PROP_IN_SEL_10B_ZERO;
>> +		break;
>> +	case 12:
>> +		input_sel = CIF_ISP_ACQ_PROP_IN_SEL_12B;
>> +		break;
>> +	default:
>> +		v4l2_err(&dev->v4l2_dev, "Invalid bus width\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	val = readl(base + CIF_ISP_ACQ_PROP);
>> +	writel(val | input_sel, base + CIF_ISP_ACQ_PROP);
>> +
>> +	return 0;
>> +}
>> +
>> +static int rkisp1_config_mipi(struct rkisp1_device *dev)
>> +{
>> +	struct ispsd_in_fmt *in_fmt = &dev->isp_sdev.in_fmt;
>> +	struct rkisp1_sensor *sensor = dev->active_sensor;
>> +	void __iomem *base = dev->base_addr;
>> +	unsigned int lanes;
>> +	u32 mipi_ctrl;
>> +
>> +	/*
>> +	 * sensor->mbus is set in isp or d-phy notifier_bound function
>> +	 */
>> +	switch (sensor->mbus.flags & V4L2_MBUS_CSI2_LANES) {
>> +	case V4L2_MBUS_CSI2_4_LANE:
>> +		lanes = 4;
>> +		break;
>> +	case V4L2_MBUS_CSI2_3_LANE:
>> +		lanes = 3;
>> +		break;
>> +	case V4L2_MBUS_CSI2_2_LANE:
>> +		lanes = 2;
>> +		break;
>> +	case V4L2_MBUS_CSI2_1_LANE:
>> +		lanes = 1;
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	mipi_ctrl = CIF_MIPI_CTRL_NUM_LANES(lanes - 1) |
>> +		    CIF_MIPI_CTRL_SHUTDOWNLANES(0xf) |
>> +		    CIF_MIPI_CTRL_ERR_SOT_SYNC_HS_SKIP |
>> +		    CIF_MIPI_CTRL_CLOCKLANE_ENA;
>> +
>> +	writel(mipi_ctrl, base + CIF_MIPI_CTRL);
>> +
>> +	/* Configure Data Type and Virtual Channel */
>> +	writel(CIF_MIPI_DATA_SEL_DT(in_fmt->mipi_dt) | CIF_MIPI_DATA_SEL_VC(0),
>> +	       base + CIF_MIPI_IMG_DATA_SEL);
>> +
>> +	/* Clear MIPI interrupts */
>> +	writel(~0, base + CIF_MIPI_ICR);
>> +	/*
>> +	 * Disable CIF_MIPI_ERR_DPHY interrupt here temporary for
>> +	 * isp bus may be dead when switch isp.
>> +	 */
>> +	writel(CIF_MIPI_FRAME_END | CIF_MIPI_ERR_CSI | CIF_MIPI_ERR_DPHY |
>> +	       CIF_MIPI_SYNC_FIFO_OVFLW(0x03) | CIF_MIPI_ADD_DATA_OVFLW,
>> +	       base + CIF_MIPI_IMSC);
>> +
>> +	v4l2_dbg(1, rkisp1_debug, &dev->v4l2_dev, "\n  MIPI_CTRL 0x%08x\n"
>> +		 "  MIPI_IMG_DATA_SEL 0x%08x\n"
>> +		 "  MIPI_STATUS 0x%08x\n"
>> +		 "  MIPI_IMSC 0x%08x\n",
>> +		 readl(base + CIF_MIPI_CTRL),
>> +		 readl(base + CIF_MIPI_IMG_DATA_SEL),
>> +		 readl(base + CIF_MIPI_STATUS),
>> +		 readl(base + CIF_MIPI_IMSC));
>> +
>> +	return 0;
>> +}
>> +
>> +/* Configure MUX */
>> +static int rkisp1_config_path(struct rkisp1_device *dev)
>> +{
>> +	struct rkisp1_sensor *sensor = dev->active_sensor;
>> +	u32 dpcl = readl(dev->base_addr + CIF_VI_DPCL);
>> +	int ret = 0;
>> +
>> +	if (sensor->mbus.type == V4L2_MBUS_BT656 ||
>> +	    sensor->mbus.type == V4L2_MBUS_PARALLEL) {
>> +		ret = rkisp1_config_dvp(dev);
>> +		dpcl |= CIF_VI_DPCL_IF_SEL_PARALLEL;
>> +	} else if (sensor->mbus.type == V4L2_MBUS_CSI2_DPHY) {
>> +		ret = rkisp1_config_mipi(dev);
>> +		dpcl |= CIF_VI_DPCL_IF_SEL_MIPI;
>> +	}
>> +
>> +	writel(dpcl, dev->base_addr + CIF_VI_DPCL);
>> +
>> +	return ret;
>> +}
>> +
>> +/* Hareware configure Entry */
>> +static int rkisp1_config_cif(struct rkisp1_device *dev)
>> +{
>> +	u32 cif_id;
>> +	int ret;
>> +
>> +	v4l2_dbg(1, rkisp1_debug, &dev->v4l2_dev,
>> +		 "SP streaming = %d, MP streaming = %d\n",
>> +		 dev->stream[RKISP1_STREAM_SP].streaming,
>> +		 dev->stream[RKISP1_STREAM_MP].streaming);
>> +
>> +	cif_id = readl(dev->base_addr + CIF_VI_ID);
>> +	v4l2_dbg(1, rkisp1_debug, &dev->v4l2_dev, "CIF_ID 0x%08x\n", cif_id);
>> +
>> +	ret = rkisp1_config_isp(dev);
>> +	if (ret < 0)
>> +		return ret;
>> +	ret = rkisp1_config_path(dev);
>> +	if (ret < 0)
>> +		return ret;
>> +	rkisp1_config_ism(dev);
>> +
>> +	return 0;
>> +}
>> +
>> +/* Mess register operations to stop isp */
>> +static int rkisp1_isp_stop(struct rkisp1_device *dev)
>> +{
>> +	void __iomem *base = dev->base_addr;
>> +	u32 val;
>> +
>> +	v4l2_dbg(1, rkisp1_debug, &dev->v4l2_dev,
>> +		 "SP streaming = %d, MP streaming = %d\n",
>> +		 dev->stream[RKISP1_STREAM_SP].streaming,
>> +		 dev->stream[RKISP1_STREAM_MP].streaming);
>> +
>> +	/*
>> +	 * ISP(mi) stop in mi frame end -> Stop ISP(mipi) ->
>> +	 * Stop ISP(isp) ->wait for ISP isp off
>> +	 */
>> +	/* stop and clear MI, MIPI, and ISP interrupts */
>> +	writel(0, base + CIF_MIPI_IMSC);
>> +	writel(~0, base + CIF_MIPI_ICR);
>> +
>> +	writel(0, base + CIF_ISP_IMSC);
>> +	writel(~0, base + CIF_ISP_ICR);
>> +
>> +	writel(0, base + CIF_MI_IMSC);
>> +	writel(~0, base + CIF_MI_ICR);
>> +	val = readl(base + CIF_MIPI_CTRL);
>> +	writel(val & (~CIF_MIPI_CTRL_OUTPUT_ENA), base + CIF_MIPI_CTRL);
>> +	/* stop ISP */
>> +	val = readl(base + CIF_ISP_CTRL);
>> +	val &= ~(CIF_ISP_CTRL_ISP_INFORM_ENABLE | CIF_ISP_CTRL_ISP_ENABLE);
>> +	writel(val, base + CIF_ISP_CTRL);
>> +
>> +	val = readl(base + CIF_ISP_CTRL);
>> +	writel(val | CIF_ISP_CTRL_ISP_CFG_UPD, base + CIF_ISP_CTRL);
>> +
>> +	readx_poll_timeout(readl, base + CIF_ISP_RIS,
>> +			   val, val & CIF_ISP_OFF, 20, 100);
>> +	v4l2_dbg(1, rkisp1_debug, &dev->v4l2_dev,
>> +		"streaming(MP:%d, SP:%d), MI_CTRL:%x, ISP_CTRL:%x, MIPI_CTRL:%x\n",
>> +		 dev->stream[RKISP1_STREAM_SP].streaming,
>> +		 dev->stream[RKISP1_STREAM_MP].streaming,
>> +		 readl(base + CIF_MI_CTRL),
>> +		 readl(base + CIF_ISP_CTRL),
>> +		 readl(base + CIF_MIPI_CTRL));
>> +
>> +	writel(CIF_IRCL_MIPI_SW_RST | CIF_IRCL_ISP_SW_RST, base + CIF_IRCL);
>> +	writel(0x0, base + CIF_IRCL);
>> +
>> +	return 0;
>> +}
>> +
>> +/* Mess register operations to start isp */
>> +static int rkisp1_isp_start(struct rkisp1_device *dev)
>> +{
>> +	struct rkisp1_sensor *sensor = dev->active_sensor;
>> +	void __iomem *base = dev->base_addr;
>> +	u32 val;
>> +
>> +	v4l2_dbg(1, rkisp1_debug, &dev->v4l2_dev,
>> +		 "SP streaming = %d, MP streaming = %d\n",
>> +		 dev->stream[RKISP1_STREAM_SP].streaming,
>> +		 dev->stream[RKISP1_STREAM_MP].streaming);
>> +
>> +	/* Activate MIPI */
>> +	if (sensor->mbus.type == V4L2_MBUS_CSI2_DPHY) {
>> +		val = readl(base + CIF_MIPI_CTRL);
>> +		writel(val | CIF_MIPI_CTRL_OUTPUT_ENA, base + CIF_MIPI_CTRL);
>> +	}
>> +	/* Activate ISP */
>> +	val = readl(base + CIF_ISP_CTRL);
>> +	val |= CIF_ISP_CTRL_ISP_CFG_UPD | CIF_ISP_CTRL_ISP_ENABLE |
>> +	       CIF_ISP_CTRL_ISP_INFORM_ENABLE;
>> +	writel(val, base + CIF_ISP_CTRL);
>> +
>> +	/* XXX: Is the 1000us too long?
>> +	 * CIF spec says to wait for sufficient time after enabling
>> +	 * the MIPI interface and before starting the sensor output.
>> +	 */
>> +	usleep_range(1000, 1200);
>> +
>> +	v4l2_dbg(1, rkisp1_debug, &dev->v4l2_dev,
>> +		 "SP streaming = %d, MP streaming = %d MI_CTRL 0x%08x\n"
>> +		 "  ISP_CTRL 0x%08x MIPI_CTRL 0x%08x\n",
>> +		 dev->stream[RKISP1_STREAM_SP].streaming,
>> +		 dev->stream[RKISP1_STREAM_MP].streaming,
>> +		 readl(base + CIF_MI_CTRL),
>> +		 readl(base + CIF_ISP_CTRL),
>> +		 readl(base + CIF_MIPI_CTRL));
>> +
>> +	return 0;
>> +}
>> +
>> +static void rkisp1_config_clk(struct rkisp1_device *dev)
>> +{
>> +	u32 val = CIF_ICCL_ISP_CLK | CIF_ICCL_CP_CLK | CIF_ICCL_MRSZ_CLK |
>> +		  CIF_ICCL_SRSZ_CLK | CIF_ICCL_JPEG_CLK | CIF_ICCL_MI_CLK |
>> +		  CIF_ICCL_IE_CLK | CIF_ICCL_MIPI_CLK | CIF_ICCL_DCROP_CLK;
>> +
>> +	writel(val, dev->base_addr + CIF_ICCL);
>> +}
>> +
>> +/***************************** isp sub-devs *******************************/
>> +
>> +static const struct ispsd_in_fmt rkisp1_isp_input_formats[] = {
>> +	{
>> +		.mbus_code	= MEDIA_BUS_FMT_SBGGR10_1X10,
>> +		.fmt_type	= FMT_BAYER,
>> +		.mipi_dt	= CIF_CSI2_DT_RAW10,
>> +		.bayer_pat	= RAW_BGGR,
>> +		.bus_width	= 10,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SRGGB10_1X10,
>> +		.fmt_type	= FMT_BAYER,
>> +		.mipi_dt	= CIF_CSI2_DT_RAW10,
>> +		.bayer_pat	= RAW_RGGB,
>> +		.bus_width	= 10,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SGBRG10_1X10,
>> +		.fmt_type	= FMT_BAYER,
>> +		.mipi_dt	= CIF_CSI2_DT_RAW10,
>> +		.bayer_pat	= RAW_GBRG,
>> +		.bus_width	= 10,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SGRBG10_1X10,
>> +		.fmt_type	= FMT_BAYER,
>> +		.mipi_dt	= CIF_CSI2_DT_RAW10,
>> +		.bayer_pat	= RAW_GRBG,
>> +		.bus_width	= 10,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SRGGB12_1X12,
>> +		.fmt_type	= FMT_BAYER,
>> +		.mipi_dt	= CIF_CSI2_DT_RAW12,
>> +		.bayer_pat	= RAW_RGGB,
>> +		.bus_width	= 12,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SBGGR12_1X12,
>> +		.fmt_type	= FMT_BAYER,
>> +		.mipi_dt	= CIF_CSI2_DT_RAW12,
>> +		.bayer_pat	= RAW_BGGR,
>> +		.bus_width	= 12,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SGBRG12_1X12,
>> +		.fmt_type	= FMT_BAYER,
>> +		.mipi_dt	= CIF_CSI2_DT_RAW12,
>> +		.bayer_pat	= RAW_GBRG,
>> +		.bus_width	= 12,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SGRBG12_1X12,
>> +		.fmt_type	= FMT_BAYER,
>> +		.mipi_dt	= CIF_CSI2_DT_RAW12,
>> +		.bayer_pat	= RAW_GRBG,
>> +		.bus_width	= 12,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SRGGB8_1X8,
>> +		.fmt_type	= FMT_BAYER,
>> +		.mipi_dt	= CIF_CSI2_DT_RAW8,
>> +		.bayer_pat	= RAW_RGGB,
>> +		.bus_width	= 8,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SBGGR8_1X8,
>> +		.fmt_type	= FMT_BAYER,
>> +		.mipi_dt	= CIF_CSI2_DT_RAW8,
>> +		.bayer_pat	= RAW_BGGR,
>> +		.bus_width	= 8,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SGBRG8_1X8,
>> +		.fmt_type	= FMT_BAYER,
>> +		.mipi_dt	= CIF_CSI2_DT_RAW8,
>> +		.bayer_pat	= RAW_GBRG,
>> +		.bus_width	= 8,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SGRBG8_1X8,
>> +		.fmt_type	= FMT_BAYER,
>> +		.mipi_dt	= CIF_CSI2_DT_RAW8,
>> +		.bayer_pat	= RAW_GRBG,
>> +		.bus_width	= 8,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_YUYV8_1X16,
>> +		.fmt_type	= FMT_YUV,
>> +		.mipi_dt	= CIF_CSI2_DT_YUV422_8b,
>> +		.yuv_seq	= CIF_ISP_ACQ_PROP_YCBYCR,
>> +		.bus_width	= 16,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_YVYU8_1X16,
>> +		.fmt_type	= FMT_YUV,
>> +		.mipi_dt	= CIF_CSI2_DT_YUV422_8b,
>> +		.yuv_seq	= CIF_ISP_ACQ_PROP_YCRYCB,
>> +		.bus_width	= 16,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_UYVY8_1X16,
>> +		.fmt_type	= FMT_YUV,
>> +		.mipi_dt	= CIF_CSI2_DT_YUV422_8b,
>> +		.yuv_seq	= CIF_ISP_ACQ_PROP_CBYCRY,
>> +		.bus_width	= 16,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_VYUY8_1X16,
>> +		.fmt_type	= FMT_YUV,
>> +		.mipi_dt	= CIF_CSI2_DT_YUV422_8b,
>> +		.yuv_seq	= CIF_ISP_ACQ_PROP_CRYCBY,
>> +		.bus_width	= 16,
>> +	},
>> +};
>> +
>> +static const struct ispsd_out_fmt rkisp1_isp_output_formats[] = {
>> +	{
>> +		.mbus_code	= MEDIA_BUS_FMT_YUYV8_2X8,
>> +		.fmt_type	= FMT_YUV,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SRGGB12_1X12,
>> +		.fmt_type	= FMT_BAYER,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SBGGR12_1X12,
>> +		.fmt_type	= FMT_BAYER,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SGBRG12_1X12,
>> +		.fmt_type	= FMT_BAYER,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SGRBG12_1X12,
>> +		.fmt_type	= FMT_BAYER,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SRGGB10_1X10,
>> +		.fmt_type	= FMT_BAYER,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SBGGR10_1X10,
>> +		.fmt_type	= FMT_BAYER,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SGBRG10_1X10,
>> +		.fmt_type	= FMT_BAYER,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SGRBG10_1X10,
>> +		.fmt_type	= FMT_BAYER,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SRGGB8_1X8,
>> +		.fmt_type	= FMT_BAYER,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SBGGR8_1X8,
>> +		.fmt_type	= FMT_BAYER,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SGBRG8_1X8,
>> +		.fmt_type	= FMT_BAYER,
>> +	}, {
>> +		.mbus_code	= MEDIA_BUS_FMT_SGRBG8_1X8,
>> +		.fmt_type	= FMT_BAYER,
>> +	},
>> +};
>> +
>> +static const struct ispsd_in_fmt *find_in_fmt(u32 mbus_code)
>> +{
>> +	unsigned int i, array_size = ARRAY_SIZE(rkisp1_isp_input_formats);
> 
> I think it'd be nicer to just use ARRAY_SIZE(...) in the condition. Same
> below.
> 
>> +	const struct ispsd_in_fmt *fmt;
>> +
>> +	for (i = 0; i < array_size; i++) {
>> +		fmt = &rkisp1_isp_input_formats[i];
>> +		if (fmt->mbus_code == mbus_code)
>> +			return fmt;
>> +	}
>> +
>> +	return NULL;
>> +}
>> +
>> +static const struct ispsd_out_fmt *find_out_fmt(u32 mbus_code)
>> +{
>> +	unsigned int i, array_size = ARRAY_SIZE(rkisp1_isp_output_formats);
>> +	const struct ispsd_out_fmt *fmt;
>> +
>> +	for (i = 0; i < array_size; i++) {
>> +		fmt = &rkisp1_isp_output_formats[i];
>> +		if (fmt->mbus_code == mbus_code)
>> +			return fmt;
>> +	}
>> +
>> +	return NULL;
>> +}
>> +
>> +static int rkisp1_isp_sd_enum_mbus_code(struct v4l2_subdev *sd,
>> +					struct v4l2_subdev_pad_config *cfg,
>> +					struct v4l2_subdev_mbus_code_enum *code)
>> +{
>> +	unsigned int i = code->index;
>> +
>> +	if ((code->pad != RKISP1_ISP_PAD_SINK) &&
>> +	    (code->pad != RKISP1_ISP_PAD_SOURCE_PATH)) {
>> +		if (i > 0)
>> +			return -EINVAL;
>> +		code->code = MEDIA_BUS_FMT_FIXED;
>> +		return 0;
>> +	}
>> +
>> +	if (code->pad == RKISP1_ISP_PAD_SINK) {
>> +		if (i >= ARRAY_SIZE(rkisp1_isp_input_formats))
>> +			return -EINVAL;
>> +		code->code = rkisp1_isp_input_formats[i].mbus_code;
>> +	} else {
>> +		if (i >= ARRAY_SIZE(rkisp1_isp_output_formats))
>> +			return -EINVAL;
>> +		code->code = rkisp1_isp_output_formats[i].mbus_code;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int rkisp1_isp_sd_init_config(struct v4l2_subdev *sd,
>> +				     struct v4l2_subdev_pad_config *cfg)
>> +{
>> +	struct v4l2_rect *mf_in_crop, *mf_out_crop;
>> +	struct v4l2_mbus_framefmt *mf_in, *mf_out;
>> +
>> +	mf_in = v4l2_subdev_get_try_format(sd, cfg, RKISP1_ISP_PAD_SINK);
>> +	mf_in->width = RKISP1_DEFAULT_WIDTH;
>> +	mf_in->height = RKISP1_DEFAULT_HEIGHT;
>> +	mf_in->field = V4L2_FIELD_NONE;
>> +	mf_in->code = rkisp1_isp_input_formats[0].mbus_code;
>> +
>> +	mf_in_crop = v4l2_subdev_get_try_crop(sd, cfg, RKISP1_ISP_PAD_SINK);
>> +	mf_in_crop->width = RKISP1_DEFAULT_WIDTH;
>> +	mf_in_crop->height = RKISP1_DEFAULT_HEIGHT;
>> +	mf_in_crop->left = 0;
>> +	mf_in_crop->top = 0;
>> +
>> +	mf_out = v4l2_subdev_get_try_format(sd, cfg,
>> +					    RKISP1_ISP_PAD_SOURCE_PATH);
>> +	*mf_out = *mf_in;
>> +	mf_out->code = rkisp1_isp_output_formats[0].mbus_code;
>> +	mf_out->quantization = V4L2_QUANTIZATION_FULL_RANGE;
>> +
>> +	mf_out_crop = v4l2_subdev_get_try_crop(sd, cfg,
>> +					       RKISP1_ISP_PAD_SOURCE_PATH);
>> +	*mf_out_crop = *mf_in_crop;
>> +
>> +	return 0;
>> +}
>> +
>> +static int rkisp1_isp_sd_get_fmt(struct v4l2_subdev *sd,
>> +				 struct v4l2_subdev_pad_config *cfg,
>> +				 struct v4l2_subdev_format *fmt)
>> +{
>> +	struct rkisp1_isp_subdev *isp_sd = sd_to_isp_sd(sd);
>> +	struct v4l2_mbus_framefmt *mf = &fmt->format;
>> +
>> +	if ((fmt->pad != RKISP1_ISP_PAD_SINK) &&
>> +	    (fmt->pad != RKISP1_ISP_PAD_SOURCE_PATH)) {
>> +		fmt->format.code = MEDIA_BUS_FMT_FIXED;
>> +		/*
>> +		 * NOTE: setting a format here doesn't make much sense
>> +		 * but v4l2-compliance complains
>> +		 */
>> +		fmt->format.width = RKISP1_DEFAULT_WIDTH;
>> +		fmt->format.height = RKISP1_DEFAULT_HEIGHT;
>> +		fmt->format.field = V4L2_FIELD_NONE;
>> +		return 0;
>> +	}
>> +
>> +	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
>> +		mf = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
>> +		fmt->format = *mf;
>> +		return 0;
>> +	}
>> +
>> +	if (fmt->pad == RKISP1_ISP_PAD_SINK) {
>> +		*mf = isp_sd->in_frm;
>> +	} else if (fmt->pad == RKISP1_ISP_PAD_SOURCE_PATH) {
>> +		/* format of source pad */
>> +		*mf = isp_sd->in_frm;
>> +		mf->code = isp_sd->out_fmt.mbus_code;
>> +		/* window size of source pad */
>> +		mf->width = isp_sd->out_crop.width;
>> +		mf->height = isp_sd->out_crop.height;
>> +		mf->quantization = isp_sd->quantization;
>> +	}
>> +	mf->field = V4L2_FIELD_NONE;
>> +
>> +	return 0;
>> +}
>> +
>> +static void rkisp1_isp_sd_try_fmt(struct v4l2_subdev *sd,
>> +				  unsigned int pad,
>> +				  struct v4l2_mbus_framefmt *fmt)
>> +{
>> +	struct rkisp1_device *isp_dev = sd_to_isp_dev(sd);
>> +	struct rkisp1_isp_subdev *isp_sd = &isp_dev->isp_sdev;
>> +	const struct ispsd_out_fmt *out_fmt;
>> +	const struct ispsd_in_fmt *in_fmt;
>> +
>> +	switch (pad) {
>> +	case RKISP1_ISP_PAD_SINK:
>> +		in_fmt = find_in_fmt(fmt->code);
>> +		if (in_fmt)
>> +			fmt->code = in_fmt->mbus_code;
>> +		else
>> +			fmt->code = MEDIA_BUS_FMT_SRGGB10_1X10;
>> +		fmt->width  = clamp_t(u32, fmt->width, CIF_ISP_INPUT_W_MIN,
>> +				      CIF_ISP_INPUT_W_MAX);
>> +		fmt->height = clamp_t(u32, fmt->height, CIF_ISP_INPUT_H_MIN,
>> +				      CIF_ISP_INPUT_H_MAX);
>> +		break;
>> +	case RKISP1_ISP_PAD_SOURCE_PATH:
>> +		out_fmt = find_out_fmt(fmt->code);
>> +		if (out_fmt)
>> +			fmt->code = out_fmt->mbus_code;
>> +		else
>> +			fmt->code = rkisp1_isp_output_formats[0].mbus_code;
>> +		/* window size is set in s_selection */
>> +		fmt->width  = isp_sd->out_crop.width;
>> +		fmt->height = isp_sd->out_crop.height;
>> +		/* full range by default */
>> +		if (!fmt->quantization)
>> +			fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
>> +		break;
>> +	}
>> +
>> +	fmt->field = V4L2_FIELD_NONE;
>> +}
>> +
>> +static int rkisp1_isp_sd_set_fmt(struct v4l2_subdev *sd,
>> +				 struct v4l2_subdev_pad_config *cfg,
>> +				 struct v4l2_subdev_format *fmt)
>> +{
>> +	struct rkisp1_device *isp_dev = sd_to_isp_dev(sd);
>> +	struct rkisp1_isp_subdev *isp_sd = &isp_dev->isp_sdev;
>> +	struct v4l2_mbus_framefmt *mf = &fmt->format;
>> +
> 
> Note that for sub-device nodes, the driver is itself responsible for
> serialising the access to its data structures.

But looking at subdev_do_ioctl_lock(), it seems that it serializes the
ioctl calls for subdevs, no? Or I'm misunderstanding something (which is
most probably) ?

> 
>> +	if ((fmt->pad != RKISP1_ISP_PAD_SINK) &&
>> +	    (fmt->pad != RKISP1_ISP_PAD_SOURCE_PATH))
>> +		return rkisp1_isp_sd_get_fmt(sd, cfg, fmt);
>> +
>> +	rkisp1_isp_sd_try_fmt(sd, fmt->pad, mf);
>> +
>> +	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
>> +		struct v4l2_mbus_framefmt *try_mf;
>> +
>> +		try_mf = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
>> +		*try_mf = *mf;
>> +		return 0;
>> +	}
>> +
>> +	if (fmt->pad == RKISP1_ISP_PAD_SINK) {
>> +		const struct ispsd_in_fmt *in_fmt;
>> +
>> +		in_fmt = find_in_fmt(mf->code);
>> +		isp_sd->in_fmt = *in_fmt;
>> +		isp_sd->in_frm = *mf;
>> +	} else if (fmt->pad == RKISP1_ISP_PAD_SOURCE_PATH) {
>> +		const struct ispsd_out_fmt *out_fmt;
>> +
>> +		/* Ignore width/height */
>> +		out_fmt = find_out_fmt(mf->code);
>> +		isp_sd->out_fmt = *out_fmt;
>> +		/*
>> +		 * It is quantization for output,
>> +		 * isp use bt601 limit-range in internal
>> +		 */
>> +		isp_sd->quantization = mf->quantization;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void rkisp1_isp_sd_try_crop(struct v4l2_subdev *sd,
>> +				  struct v4l2_subdev_pad_config *cfg,
>> +				  struct v4l2_subdev_selection *sel)
>> +{
>> +	struct rkisp1_isp_subdev *isp_sd = sd_to_isp_sd(sd);
>> +	struct v4l2_mbus_framefmt in_frm = isp_sd->in_frm;
>> +	struct v4l2_rect in_crop = isp_sd->in_crop;
>> +	struct v4l2_rect *input = &sel->r;
>> +
>> +	if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
>> +		in_frm = *v4l2_subdev_get_try_format(sd, cfg,
>> +						     RKISP1_ISP_PAD_SINK);
>> +		in_crop = *v4l2_subdev_get_try_crop(sd, cfg,
>> +						    RKISP1_ISP_PAD_SINK);
>> +	}
>> +
>> +	input->left = ALIGN(input->left, 2);
>> +	input->width = ALIGN(input->width, 2);
>> +
>> +	if (sel->pad == RKISP1_ISP_PAD_SINK) {
>> +		input->left = clamp_t(u32, input->left, 0, in_frm.width);
>> +		input->top = clamp_t(u32, input->top, 0, in_frm.height);
>> +		input->width = clamp_t(u32, input->width, CIF_ISP_INPUT_W_MIN,
>> +				       in_frm.width - input->left);
>> +		input->height = clamp_t(u32, input->height,
>> +					CIF_ISP_INPUT_H_MIN,
>> +					in_frm.height - input->top);
>> +	} else if (sel->pad == RKISP1_ISP_PAD_SOURCE_PATH) {
>> +		input->left = clamp_t(u32, input->left, 0, in_crop.width);
>> +		input->top = clamp_t(u32, input->top, 0, in_crop.height);
>> +		input->width = clamp_t(u32, input->width, CIF_ISP_OUTPUT_W_MIN,
>> +				       in_crop.width - input->left);
>> +		input->height = clamp_t(u32, input->height,
>> +					CIF_ISP_OUTPUT_H_MIN,
>> +					in_crop.height - input->top);
>> +	}
>> +}
>> +
>> +static int rkisp1_isp_sd_get_selection(struct v4l2_subdev *sd,
>> +				       struct v4l2_subdev_pad_config *cfg,
>> +				       struct v4l2_subdev_selection *sel)
>> +{
>> +	struct rkisp1_isp_subdev *isp_sd = sd_to_isp_sd(sd);
>> +	struct v4l2_mbus_framefmt *frm;
>> +	struct v4l2_rect *rect;
>> +
>> +	if (sel->pad != RKISP1_ISP_PAD_SOURCE_PATH &&
>> +	    sel->pad != RKISP1_ISP_PAD_SINK)
>> +		return -EINVAL;
>> +
>> +	switch (sel->target) {
>> +	case V4L2_SEL_TGT_CROP_BOUNDS:
>> +		if (sel->pad == RKISP1_ISP_PAD_SINK) {
>> +			if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
>> +				frm = v4l2_subdev_get_try_format(sd, cfg,
>> +								 sel->pad);
>> +			else
>> +				frm = &isp_sd->in_frm;
>> +
>> +			sel->r.height = frm->height;
>> +			sel->r.width = frm->width;
>> +			sel->r.left = 0;
>> +			sel->r.top = 0;
>> +		} else {
>> +			if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
>> +				rect = v4l2_subdev_get_try_crop(sd, cfg,
>> +							RKISP1_ISP_PAD_SINK);
>> +			else
>> +				rect = &isp_sd->in_crop;
>> +			sel->r = *rect;
>> +		}
>> +		break;
>> +	case V4L2_SEL_TGT_CROP:
>> +		if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
>> +			rect = v4l2_subdev_get_try_crop(sd, cfg, sel->pad);
>> +		else if (sel->pad == RKISP1_ISP_PAD_SINK)
>> +			rect = &isp_sd->in_crop;
>> +		else
>> +			rect = &isp_sd->out_crop;
>> +		sel->r = *rect;
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int rkisp1_isp_sd_set_selection(struct v4l2_subdev *sd,
>> +				       struct v4l2_subdev_pad_config *cfg,
>> +				       struct v4l2_subdev_selection *sel)
>> +{
>> +	struct rkisp1_isp_subdev *isp_sd = sd_to_isp_sd(sd);
>> +	struct rkisp1_device *dev = sd_to_isp_dev(sd);
>> +
>> +	if (sel->pad != RKISP1_ISP_PAD_SOURCE_PATH &&
>> +	    sel->pad != RKISP1_ISP_PAD_SINK)
>> +		return -EINVAL;
>> +	if (sel->target != V4L2_SEL_TGT_CROP)
>> +		return -EINVAL;
>> +
>> +	v4l2_dbg(1, rkisp1_debug, &dev->v4l2_dev,
>> +		 "%s: pad: %d sel(%d,%d)/%dx%d\n", __func__, sel->pad,
>> +		 sel->r.left, sel->r.top, sel->r.width, sel->r.height);
>> +	rkisp1_isp_sd_try_crop(sd, cfg, sel);
>> +
>> +	if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
>> +		struct v4l2_rect *try_sel =
>> +			v4l2_subdev_get_try_crop(sd, cfg, sel->pad);
>> +
>> +		*try_sel = sel->r;
>> +		return 0;
>> +	}
>> +
>> +	if (sel->pad == RKISP1_ISP_PAD_SINK)
>> +		isp_sd->in_crop = sel->r;
>> +	else
>> +		isp_sd->out_crop = sel->r;
>> +
>> +	return 0;
>> +}
>> +
>> +static int mipi_csi2_s_stream_start(struct rkisp1_isp_subdev *isp_sd,
>> +				    struct rkisp1_sensor *sensor)
>> +{
>> +	union phy_configure_opts opts = { 0 };
>> +	struct phy_configure_opts_mipi_dphy *cfg = &opts.mipi_dphy;
>> +	struct v4l2_ctrl *pixel_rate;
>> +	s64 pixel_clock;
>> +
>> +	pixel_rate = v4l2_ctrl_find(sensor->sd->ctrl_handler,
>> +				    V4L2_CID_PIXEL_RATE);
>> +	if (!pixel_rate) {
>> +		v4l2_warn(sensor->sd, "No pixel rate control in subdev\n");
>> +		return -EPIPE;
>> +	}
>> +
>> +	pixel_clock = v4l2_ctrl_g_ctrl_int64(pixel_rate);
>> +	if (!pixel_clock) {
>> +		v4l2_err(sensor->sd, "Invalid pixel rate value\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	phy_mipi_dphy_get_default_config(pixel_clock, isp_sd->in_fmt.bus_width,
>> +					 sensor->lanes, cfg);
>> +	phy_set_mode(sensor->dphy, PHY_MODE_MIPI_DPHY);
>> +	phy_configure(sensor->dphy, &opts);
>> +	phy_power_on(sensor->dphy);
>> +
>> +	return 0;
>> +}
>> +
>> +static void mipi_csi2_s_stream_stop(struct rkisp1_sensor *sensor)
>> +{
>> +	phy_power_off(sensor->dphy);
>> +}
>> +
>> +static int rkisp1_isp_sd_s_stream(struct v4l2_subdev *sd, int on)
>> +{
>> +	struct rkisp1_device *isp_dev = sd_to_isp_dev(sd);
>> +	struct v4l2_subdev *sensor_sd;
>> +	int ret = 0;
>> +
>> +	if (!on) {
>> +		ret = rkisp1_isp_stop(isp_dev);
>> +		if (ret < 0)
>> +			return ret;
>> +		mipi_csi2_s_stream_stop(isp_dev->active_sensor);
>> +		return 0;
>> +	}
>> +
>> +	sensor_sd = get_remote_sensor(sd);
>> +	if (!sensor_sd)
>> +		return -ENODEV;
>> +
>> +	isp_dev->active_sensor = sd_to_sensor(isp_dev, sensor_sd);
>> +	if (!isp_dev->active_sensor)
>> +		return -ENODEV;
>> +
>> +	atomic_set(&isp_dev->isp_sdev.frm_sync_seq, 0);
>> +	ret = rkisp1_config_cif(isp_dev);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	/* TODO: support other interfaces */
>> +	if (isp_dev->active_sensor->mbus.type != V4L2_MBUS_CSI2_DPHY)
>> +		return -EINVAL;
>> +
>> +	ret = mipi_csi2_s_stream_start(&isp_dev->isp_sdev,
>> +				       isp_dev->active_sensor);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = rkisp1_isp_start(isp_dev);
>> +	if (ret)
>> +		mipi_csi2_s_stream_stop(isp_dev->active_sensor);
>> +
>> +	return ret;
>> +}
>> +
>> +static int rkisp1_isp_sd_s_power(struct v4l2_subdev *sd, int on)
> 
> If you support runtime PM, you shouldn't implement the s_power op.

Is is ok to completly remove the usage of runtime PM then?
Like this http://ix.io/1RJb ?

tbh I'm not that familar with runtime PM and I'm not sure what is the
difference of it and using s_power op (and Documentation/power/runtime_pm.rst
is not being that helpful tbh).

> 
> You'll still need to call s_power on external subdevs though.
> 
>> +{
>> +	struct rkisp1_device *isp_dev = sd_to_isp_dev(sd);
>> +	int ret;
>> +
>> +	v4l2_dbg(1, rkisp1_debug, &isp_dev->v4l2_dev, "s_power: %d\n", on);
>> +
>> +	if (on) {
>> +		ret = pm_runtime_get_sync(isp_dev->dev);

If this is not ok to remove suport for runtime PM, then where should I put
the call to pm_runtime_get_sync() if not in this s_power op ?

>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		rkisp1_config_clk(isp_dev);
>> +	} else {
>> +		ret = pm_runtime_put(isp_dev->dev);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int rkisp1_subdev_link_validate(struct media_link *link)
>> +{
>> +	if (link->source->index == RKISP1_ISP_PAD_SINK_PARAMS)
> 
> Is this test correct? The source is the source end of the link, i.e. the
> video node.

Ah yes, it should be link->sink->index (and not source), thanks for spotting this.

> 
> How about the links that end in a video node?

I thought that the only possibilities were sensor->isp1 and params->isp1 (where params
is an output video node that should be catched by the corrected version of the if
statement above.

Or do you mean another thing?

> 
>> +		return 0;
>> +
>> +	return v4l2_subdev_link_validate(link);
>> +}
>> +
>> +static int rkisp1_subdev_fmt_link_validate(struct v4l2_subdev *sd,
>> +					struct media_link *link,
>> +					struct v4l2_subdev_format *source_fmt,
>> +					struct v4l2_subdev_format *sink_fmt)
>> +{
>> +	if (source_fmt->format.code != sink_fmt->format.code)
>> +		return -EINVAL;

ops, should be -EPIPE

>> +
>> +	/* Crop is available */
>> +	if (source_fmt->format.width < sink_fmt->format.width ||
>> +	    source_fmt->format.height < sink_fmt->format.height)
>> +		return -EINVAL;

-EPIPE

>> +
> 
> Could you use v4l2_subdev_link_validate_default()?

v4l2_subdev_link_validate_default() only allows for an exact width/height match,
but here we allow the sink to be smaller then the source for cropping, no?

Thanks again for your review!
Helen

> 
>> +	return 0;
>> +}
>> +
>> +static void rkisp1_isp_queue_event_sof(struct rkisp1_isp_subdev *isp)
>> +{
>> +	struct v4l2_event event = {
>> +		.type = V4L2_EVENT_FRAME_SYNC,
>> +		.u.frame_sync.frame_sequence =
>> +			atomic_inc_return(&isp->frm_sync_seq) - 1,
>> +	};
>> +	v4l2_event_queue(isp->sd.devnode, &event);
>> +}
>> +
>> +static int rkisp1_isp_sd_subs_evt(struct v4l2_subdev *sd, struct v4l2_fh *fh,
>> +				  struct v4l2_event_subscription *sub)
>> +{
>> +	if (sub->type != V4L2_EVENT_FRAME_SYNC)
>> +		return -EINVAL;
>> +
>> +	/* Line number. For now only zero accepted. */
>> +	if (sub->id != 0)
>> +		return -EINVAL;
>> +
>> +	return v4l2_event_subscribe(fh, sub, 0, NULL);
>> +}
>> +
>> +static const struct v4l2_subdev_pad_ops rkisp1_isp_sd_pad_ops = {
>> +	.enum_mbus_code = rkisp1_isp_sd_enum_mbus_code,
>> +	.get_selection = rkisp1_isp_sd_get_selection,
>> +	.set_selection = rkisp1_isp_sd_set_selection,
>> +	.init_cfg = rkisp1_isp_sd_init_config,
>> +	.get_fmt = rkisp1_isp_sd_get_fmt,
>> +	.set_fmt = rkisp1_isp_sd_set_fmt,
>> +	.link_validate = rkisp1_subdev_fmt_link_validate,
>> +};
>> +
>> +static const struct media_entity_operations rkisp1_isp_sd_media_ops = {
>> +	.link_validate = rkisp1_subdev_link_validate,
>> +};
>> +
>> +static const struct v4l2_subdev_video_ops rkisp1_isp_sd_video_ops = {
>> +	.s_stream = rkisp1_isp_sd_s_stream,
>> +};
>> +
>> +static const struct v4l2_subdev_core_ops rkisp1_isp_core_ops = {
>> +	.subscribe_event = rkisp1_isp_sd_subs_evt,
>> +	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
>> +	.s_power = rkisp1_isp_sd_s_power,
>> +};
>> +
>> +static struct v4l2_subdev_ops rkisp1_isp_sd_ops = {
> 
> const
> 
>> +	.core = &rkisp1_isp_core_ops,
>> +	.video = &rkisp1_isp_sd_video_ops,
>> +	.pad = &rkisp1_isp_sd_pad_ops,
>> +};
>> +
>> +static void rkisp1_isp_sd_init_default_fmt(struct rkisp1_isp_subdev *isp_sd)
>> +{
>> +	struct v4l2_mbus_framefmt *in_frm = &isp_sd->in_frm;
>> +	struct v4l2_rect *in_crop = &isp_sd->in_crop;
>> +	struct v4l2_rect *out_crop = &isp_sd->out_crop;
>> +	struct ispsd_in_fmt *in_fmt = &isp_sd->in_fmt;
>> +	struct ispsd_out_fmt *out_fmt = &isp_sd->out_fmt;
>> +
>> +	*in_fmt = rkisp1_isp_input_formats[0];
>> +	in_frm->width = RKISP1_DEFAULT_WIDTH;
>> +	in_frm->height = RKISP1_DEFAULT_HEIGHT;
>> +	in_frm->code = in_fmt->mbus_code;
>> +
>> +	in_crop->width = in_frm->width;
>> +	in_crop->height = in_frm->height;
>> +	in_crop->left = 0;
>> +	in_crop->top = 0;
>> +
>> +	/* propagate to source */
>> +	*out_crop = *in_crop;
>> +	*out_fmt = rkisp1_isp_output_formats[0];
>> +	isp_sd->quantization = V4L2_QUANTIZATION_FULL_RANGE;
>> +}
>> +
>> +int rkisp1_register_isp_subdev(struct rkisp1_device *isp_dev,
>> +			       struct v4l2_device *v4l2_dev)
>> +{
>> +	struct rkisp1_isp_subdev *isp_sdev = &isp_dev->isp_sdev;
>> +	struct v4l2_subdev *sd = &isp_sdev->sd;
>> +	int ret;
>> +
>> +	v4l2_subdev_init(sd, &rkisp1_isp_sd_ops);
>> +	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
>> +	sd->entity.ops = &rkisp1_isp_sd_media_ops;
>> +	snprintf(sd->name, sizeof(sd->name), "rkisp1-isp-subdev");
> 
> strscpy()
> 
>> +
>> +	isp_sdev->pads[RKISP1_ISP_PAD_SINK].flags =
>> +		MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
>> +	isp_sdev->pads[RKISP1_ISP_PAD_SINK_PARAMS].flags = MEDIA_PAD_FL_SINK;
>> +	isp_sdev->pads[RKISP1_ISP_PAD_SOURCE_PATH].flags = MEDIA_PAD_FL_SOURCE;
>> +	isp_sdev->pads[RKISP1_ISP_PAD_SOURCE_STATS].flags = MEDIA_PAD_FL_SOURCE;
>> +	sd->entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
>> +	ret = media_entity_pads_init(&sd->entity, RKISP1_ISP_PAD_MAX,
>> +				     isp_sdev->pads);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	sd->owner = THIS_MODULE;
>> +	v4l2_set_subdevdata(sd, isp_dev);
>> +
>> +	sd->grp_id = GRP_ID_ISP;
>> +	ret = v4l2_device_register_subdev(v4l2_dev, sd);
>> +	if (ret < 0) {
>> +		v4l2_err(sd, "Failed to register isp subdev\n");
>> +		goto err_cleanup_media_entity;
>> +	}
>> +
>> +	rkisp1_isp_sd_init_default_fmt(isp_sdev);
>> +
>> +	return 0;
> 
> A newline would be nice here.
> 
>> +err_cleanup_media_entity:
>> +	media_entity_cleanup(&sd->entity);
> 
> And here.
> 
>> +	return ret;
>> +}
>> +
>> +void rkisp1_unregister_isp_subdev(struct rkisp1_device *isp_dev)
>> +{
>> +	struct v4l2_subdev *sd = &isp_dev->isp_sdev.sd;
>> +
>> +	v4l2_device_unregister_subdev(sd);
>> +	media_entity_cleanup(&sd->entity);
>> +}
>> +
>> +/****************  Interrupter Handler ****************/
>> +
>> +void rkisp1_mipi_isr(unsigned int mis, struct rkisp1_device *dev)
>> +{
>> +	struct v4l2_device *v4l2_dev = &dev->v4l2_dev;
>> +	void __iomem *base = dev->base_addr;
>> +	u32 val;
>> +
>> +	writel(~0, base + CIF_MIPI_ICR);
>> +
>> +	/*
>> +	 * Disable DPHY errctrl interrupt, because this dphy
>> +	 * erctrl signal is asserted until the next changes
>> +	 * of line state. This time is may be too long and cpu
>> +	 * is hold in this interrupt.
>> +	 */
>> +	if (mis & CIF_MIPI_ERR_CTRL(0x0f)) {
>> +		val = readl(base + CIF_MIPI_IMSC);
>> +		writel(val & ~CIF_MIPI_ERR_CTRL(0x0f), base + CIF_MIPI_IMSC);
>> +		dev->isp_sdev.dphy_errctrl_disabled = true;
>> +	}
>> +
>> +	/*
>> +	 * Enable DPHY errctrl interrupt again, if mipi have receive
>> +	 * the whole frame without any error.
>> +	 */
>> +	if (mis == CIF_MIPI_FRAME_END) {
>> +		/*
>> +		 * Enable DPHY errctrl interrupt again, if mipi have receive
>> +		 * the whole frame without any error.
>> +		 */
>> +		if (dev->isp_sdev.dphy_errctrl_disabled) {
>> +			val = readl(base + CIF_MIPI_IMSC);
>> +			val |= CIF_MIPI_ERR_CTRL(0x0f);
>> +			writel(val, base + CIF_MIPI_IMSC);
>> +			dev->isp_sdev.dphy_errctrl_disabled = false;
>> +		}
>> +	} else {
>> +		v4l2_warn(v4l2_dev, "MIPI mis error: 0x%08x\n", mis);
>> +	}
>> +}
>> +
>> +void rkisp1_isp_isr(unsigned int isp_mis, struct rkisp1_device *dev)
>> +{
>> +	void __iomem *base = dev->base_addr;
>> +	unsigned int isp_mis_tmp = 0;
>> +	unsigned int isp_err = 0;
>> +
>> +	/* start edge of v_sync */
>> +	if (isp_mis & CIF_ISP_V_START) {
>> +		rkisp1_isp_queue_event_sof(&dev->isp_sdev);
>> +
>> +		writel(CIF_ISP_V_START, base + CIF_ISP_ICR);
>> +		isp_mis_tmp = readl(base + CIF_ISP_MIS);
>> +		if (isp_mis_tmp & CIF_ISP_V_START)
>> +			v4l2_err(&dev->v4l2_dev, "isp icr v_statr err: 0x%x\n",
>> +				 isp_mis_tmp);
>> +	}
>> +
>> +	if ((isp_mis & CIF_ISP_PIC_SIZE_ERROR)) {
> 
> Extra parentheses.
> 
>> +		/* Clear pic_size_error */
>> +		writel(CIF_ISP_PIC_SIZE_ERROR, base + CIF_ISP_ICR);
>> +		isp_err = readl(base + CIF_ISP_ERR);
>> +		v4l2_err(&dev->v4l2_dev,
>> +			 "CIF_ISP_PIC_SIZE_ERROR (0x%08x)", isp_err);
>> +		writel(isp_err, base + CIF_ISP_ERR_CLR);
>> +	} else if ((isp_mis & CIF_ISP_DATA_LOSS)) {
>> +		/* Clear data_loss */
>> +		writel(CIF_ISP_DATA_LOSS, base + CIF_ISP_ICR);
>> +		v4l2_err(&dev->v4l2_dev, "CIF_ISP_DATA_LOSS\n");
>> +		writel(CIF_ISP_DATA_LOSS, base + CIF_ISP_ICR);
>> +	}
>> +
>> +	/* sampled input frame is complete */
>> +	if (isp_mis & CIF_ISP_FRAME_IN) {
>> +		writel(CIF_ISP_FRAME_IN, base + CIF_ISP_ICR);
>> +		isp_mis_tmp = readl(base + CIF_ISP_MIS);
>> +		if (isp_mis_tmp & CIF_ISP_FRAME_IN)
>> +			v4l2_err(&dev->v4l2_dev, "isp icr frame_in err: 0x%x\n",
>> +				 isp_mis_tmp);
>> +	}
>> +
>> +	/* frame was completely put out */
>> +	if (isp_mis & CIF_ISP_FRAME) {
>> +		u32 isp_ris = 0;
>> +		/* Clear Frame In (ISP) */
>> +		writel(CIF_ISP_FRAME, base + CIF_ISP_ICR);
>> +		isp_mis_tmp = readl(base + CIF_ISP_MIS);
>> +		if (isp_mis_tmp & CIF_ISP_FRAME)
>> +			v4l2_err(&dev->v4l2_dev,
>> +				 "isp icr frame end err: 0x%x\n", isp_mis_tmp);
>> +
>> +		isp_ris = readl(base + CIF_ISP_RIS);
>> +		if (isp_ris & (CIF_ISP_AWB_DONE | CIF_ISP_AFM_FIN |
>> +			       CIF_ISP_EXP_END | CIF_ISP_HIST_MEASURE_RDY))
>> +			rkisp1_stats_isr(&dev->stats_vdev, isp_ris);
>> +	}
>> +
>> +	/*
>> +	 * Then update changed configs. Some of them involve
>> +	 * lot of register writes. Do those only one per frame.
>> +	 * Do the updates in the order of the processing flow.
>> +	 */
>> +	rkisp1_params_isr(&dev->params_vdev, isp_mis);
>> +}
>> diff --git a/drivers/media/platform/rockchip/isp1/rkisp1.h b/drivers/media/platform/rockchip/isp1/rkisp1.h
>> new file mode 100644
>> index 000000000000..b0366e354ec2
>> --- /dev/null
>> +++ b/drivers/media/platform/rockchip/isp1/rkisp1.h
>> @@ -0,0 +1,111 @@
>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>> +/*
>> + * Rockchip isp1 driver
>> + *
>> + * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
>> + */
>> +
>> +#ifndef _RKISP1_H
>> +#define _RKISP1_H
>> +
>> +#include <linux/platform_device.h>
>> +#include <media/v4l2-fwnode.h>
>> +
>> +#include "common.h"
>> +
>> +struct rkisp1_stream;
>> +
>> +/*
>> + * struct ispsd_in_fmt - ISP intput-pad format
>> + *
>> + * Translate mbus_code to hardware format values
>> + *
>> + * @bus_width: used for parallel
>> + */
>> +struct ispsd_in_fmt {
>> +	u32 mbus_code;
>> +	u8 fmt_type;
>> +	u32 mipi_dt;
>> +	u32 yuv_seq;
>> +	enum rkisp1_fmt_raw_pat_type bayer_pat;
>> +	u8 bus_width;
>> +};
>> +
>> +struct ispsd_out_fmt {
>> +	u32 mbus_code;
>> +	u8 fmt_type;
>> +};
>> +
>> +struct rkisp1_ie_config {
>> +	unsigned int effect;
>> +};
>> +
>> +enum rkisp1_isp_pad {
>> +	RKISP1_ISP_PAD_SINK,
>> +	RKISP1_ISP_PAD_SINK_PARAMS,
>> +	RKISP1_ISP_PAD_SOURCE_PATH,
>> +	RKISP1_ISP_PAD_SOURCE_STATS,
>> +	RKISP1_ISP_PAD_MAX
>> +};
>> +
>> +/*
>> + * struct rkisp1_isp_subdev - ISP sub-device
>> + *
>> + * See Cropping regions of ISP in rkisp1.c for details
>> + * @in_frm: input size, don't have to be equal to sensor size
>> + * @in_fmt: input format
>> + * @in_crop: crop for sink pad
>> + * @out_fmt: output format
>> + * @out_crop: output size
>> + *
>> + * @dphy_errctrl_disabled: if dphy errctrl is disabled(avoid endless interrupt)
>> + * @frm_sync_seq: frame sequence, to sync frame_id between video devices.
>> + * @quantization: output quantization
>> + */
>> +struct rkisp1_isp_subdev {
>> +	struct v4l2_subdev sd;
>> +	struct media_pad pads[RKISP1_ISP_PAD_MAX];
>> +	struct v4l2_ctrl_handler ctrl_handler;
>> +	struct v4l2_mbus_framefmt in_frm;
>> +	struct ispsd_in_fmt in_fmt;
>> +	struct v4l2_rect in_crop;
>> +	struct ispsd_out_fmt out_fmt;
>> +	struct v4l2_rect out_crop;
>> +	bool dphy_errctrl_disabled;
>> +	atomic_t frm_sync_seq;
>> +	enum v4l2_quantization quantization;
>> +};
>> +
>> +int rkisp1_register_isp_subdev(struct rkisp1_device *isp_dev,
>> +			       struct v4l2_device *v4l2_dev);
>> +
>> +void rkisp1_unregister_isp_subdev(struct rkisp1_device *isp_dev);
>> +
>> +void rkisp1_mipi_isr(unsigned int mipi_mis, struct rkisp1_device *dev);
>> +
>> +void rkisp1_isp_isr(unsigned int isp_mis, struct rkisp1_device *dev);
>> +
>> +static inline
>> +struct ispsd_out_fmt *rkisp1_get_ispsd_out_fmt(struct rkisp1_isp_subdev *isp_sdev)
>> +{
>> +	return &isp_sdev->out_fmt;
>> +}
>> +
>> +static inline
>> +struct ispsd_in_fmt *rkisp1_get_ispsd_in_fmt(struct rkisp1_isp_subdev *isp_sdev)
>> +{
>> +	return &isp_sdev->in_fmt;
>> +}
>> +
>> +static inline
>> +struct v4l2_rect *rkisp1_get_isp_sd_win(struct rkisp1_isp_subdev *isp_sdev)
>> +{
>> +	return &isp_sdev->out_crop;
>> +}
> 
> I'd just use the struct fields directly in the code as it's easier to
> figure out which field in the struct is being accessed.
> 
>> +
>> +static inline struct rkisp1_isp_subdev *sd_to_isp_sd(struct v4l2_subdev *sd)
>> +{
>> +	return container_of(sd, struct rkisp1_isp_subdev, sd);
>> +}
>> +
>> +#endif /* _RKISP1_H */
> 

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

^ permalink raw reply

* [PATCH RFC 11/11] ARM: dts: qcom: msm8974-hammerhead: add support for external display
From: Brian Masney @ 2019-08-15  0:48 UTC (permalink / raw)
  To: bjorn.andersson, robh+dt, agross, a.hajda, narmstrong, robdclark,
	sean
  Cc: mark.rutland, devicetree, jernej.skrabec, jonas, airlied,
	linux-arm-msm, linus.walleij, linux-kernel, dri-devel,
	Laurent.pinchart, daniel, enric.balletbo, freedreno,
	linux-arm-kernel
In-Reply-To: <20190815004854.19860-1-masneyb@onstation.org>

Add HDMI nodes and other supporting infrastructure in order to support
the external display. This is based on work from Jonathan Marek.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
The hdmi-tx node in the downstream MSM sources:
https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/boot/dts/msm8974-mdss.dtsi#L101

 .../qcom-msm8974-lge-nexus5-hammerhead.dts    | 140 ++++++++++++++++++
 1 file changed, 140 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
index 3487daf98e81..83416b6d6634 100644
--- a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
+++ b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
@@ -234,6 +234,34 @@
 		pinctrl-names = "default";
 		pinctrl-0 = <&wlan_regulator_pin>;
 	};
+
+	anx_avdd33: avdd33 {
+		compatible = "regulator-fixed";
+
+		regulator-name = "avdd-3p3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&pm8941_gpios 26 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&anx_avdd33_pin>;
+	};
+
+	anx_vdd10: vdd10 {
+		compatible = "regulator-fixed";
+
+		regulator-name = "vdd-1p0";
+		regulator-min-microvolt = <1000000>;
+		regulator-max-microvolt = <1000000>;
+
+		gpio = <&pm8941_gpios 8 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&anx_vdd10_pin>;
+	};
 };
 
 &soc {
@@ -355,6 +383,40 @@
 				bias-disable;
 			};
 		};
+
+		hdmi_pin: hdmi {
+			cec {
+				pins = "gpio31";
+				function = "hdmi_cec";
+			};
+
+			ddc {
+				pins = "gpio32", "gpio33";
+				function = "hdmi_ddc";
+			};
+
+			hpd {
+				pins = "gpio34";
+				function = "hdmi_hpd";
+			};
+		};
+
+		anx_msm_pin: anx {
+			irq {
+				pins = "gpio28";
+				function = "gpio";
+				drive-strength = <8>;
+				bias-pull-up;
+				input-enable;
+			};
+
+			reset {
+				pins = "gpio68";
+				function = "gpio";
+				drive-strength = <8>;
+				bias-pull-up;
+			};
+		};
 	};
 
 	sdhci@f9824900 {
@@ -440,6 +502,28 @@
 				default-brightness = <200>;
 			};
 		};
+
+		anx7808@72 {
+			compatible = "analogix,anx7808";
+			reg = <0x72>;
+			interrupts-extended = <&msmgpio 28 IRQ_TYPE_EDGE_RISING>;
+
+			hpd-gpios = <&pm8941_gpios 13 GPIO_ACTIVE_HIGH>;
+			pd-gpios = <&pm8941_gpios 14 GPIO_ACTIVE_HIGH>;
+			reset-gpios = <&msmgpio 68 GPIO_ACTIVE_LOW>;
+
+			pinctrl-names = "default";
+			pinctrl-0 = <&anx_msm_pin>, <&anx_pin>;
+
+			dvdd10-supply = <&anx_vdd10>;
+			avdd33-supply = <&anx_avdd33>;
+
+			port {
+				anx7808_in: endpoint {
+					remote-endpoint = <&hdmi_out>;
+				};
+			};
+		};
 	};
 
 	i2c@f9968000 {
@@ -621,6 +705,29 @@
 
 			vddio-supply = <&pm8941_l12>;
 		};
+
+		hdmi-tx@fd922100 {
+			status = "ok";
+
+			pinctrl-names = "default";
+			pinctrl-0 = <&hdmi_pin>;
+
+			qcom,hdmi-tx-ddc-clk = <&msmgpio 32 GPIO_ACTIVE_HIGH>;
+			qcom,hdmi-tx-ddc-data = <&msmgpio 33 GPIO_ACTIVE_HIGH>;
+			qcom,hdmi-tx-hpd = <&msmgpio 34 GPIO_ACTIVE_HIGH>;
+
+			ports {
+				port@1 {
+					hdmi_out: endpoint {
+						remote-endpoint = <&anx7808_in>;
+					};
+				};
+			};
+		};
+
+		hdmi-phy@fd922500 {
+			status = "ok";
+		};
 	};
 };
 
@@ -657,6 +764,39 @@
 				output-high;
 				line-name = "otg-gpio";
 			};
+
+			anx_pin: anx {
+				cbldet {
+					pins = "gpio13";
+					function = "normal";
+					input-enable;
+					bias-pull-down;
+					power-source = <PM8941_GPIO_S3>;
+				};
+
+				pd {
+					pins = "gpio14";
+					function = "normal";
+					bias-disable;
+					power-source = <PM8941_GPIO_S3>;
+				};
+			};
+
+			anx_avdd33_pin: anxvdd3  {
+				pins = "gpio26";
+				function = "normal";
+
+				bias-disable;
+				power-source = <PM8941_GPIO_S3>;
+			};
+
+			anx_vdd10_pin: anxvdd1 {
+				pins = "gpio8";
+				function = "normal";
+
+				bias-disable;
+				power-source = <PM8941_GPIO_S3>;
+			};
 		};
 	};
 };
-- 
2.21.0


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

^ permalink raw reply related

* [PATCH RFC 10/11] ARM: dts: qcom: msm8974: add HDMI nodes
From: Brian Masney @ 2019-08-15  0:48 UTC (permalink / raw)
  To: bjorn.andersson, robh+dt, agross, a.hajda, narmstrong, robdclark,
	sean
  Cc: mark.rutland, devicetree, jernej.skrabec, jonas, airlied,
	linux-arm-msm, linus.walleij, linux-kernel, dri-devel,
	Laurent.pinchart, daniel, enric.balletbo, freedreno,
	linux-arm-kernel
In-Reply-To: <20190815004854.19860-1-masneyb@onstation.org>

Add HDMI tx and phy nodes to support an external display that can be
connected over the SlimPort. This is based on work from Jonathan Marek.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
The hdmi-tx node in the downstream MSM sources:
https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/boot/dts/msm8974-mdss.dtsi#L101

 arch/arm/boot/dts/qcom-msm8974.dtsi | 80 +++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index 369e58f64145..35c51336a9d4 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -1139,6 +1139,13 @@
 
 					port@0 {
 						reg = <0>;
+						mdp5_intf3_out: endpoint {
+							remote-endpoint = <&hdmi_in>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
 						mdp5_intf1_out: endpoint {
 							remote-endpoint = <&dsi0_in>;
 						};
@@ -1216,6 +1223,79 @@
 				clocks = <&mmcc MDSS_AHB_CLK>;
 				clock-names = "iface";
 			};
+
+			hdmi: hdmi-tx@fd922100 {
+				status = "disabled";
+
+				compatible = "qcom,hdmi-tx-8974";
+				reg = <0xfd922100 0x35c>,
+				      <0xfc4b8000 0x60f0>;
+				reg-names = "core_physical",
+				            "qfprom_physical";
+
+				interrupt-parent = <&mdss>;
+				interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
+
+				power-domains = <&mmcc MDSS_GDSC>;
+
+				clocks = <&mmcc MDSS_MDP_CLK>,
+				         <&mmcc MDSS_AHB_CLK>,
+				         <&mmcc MDSS_HDMI_CLK>,
+				         <&mmcc MDSS_HDMI_AHB_CLK>,
+				         <&mmcc MDSS_EXTPCLK_CLK>;
+				clock-names = "mdp_core",
+				              "iface",
+				              "core",
+				              "alt_iface",
+				              "extp";
+
+				hpd-5v-supply = <&pm8941_5vs2>;
+				core-vdda-supply = <&pm8941_l12>;
+				core-vcc-supply = <&pm8941_s3>;
+
+				/*
+				 * FIXME - drivers/gpu/drm/msm/hdmi/hdmi.c via hpd_reg_names_8x74
+				 * looks for hpd-gdsc-supply. What should be used here? Shouldn't
+				 * this functionality be provided by the power-domains above?
+				 */
+
+				phys = <&hdmi_phy>;
+				phy-names = "hdmi_phy";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+						hdmi_in: endpoint {
+							remote-endpoint = <&mdp5_intf3_out>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+					};
+				};
+			};
+
+			hdmi_phy: hdmi-phy@fd922500 {
+				status = "disabled";
+
+				compatible = "qcom,hdmi-phy-8974";
+				reg = <0xfd922500 0x7c>;
+				reg-names = "hdmi_phy";
+
+				clocks = <&mmcc MDSS_AHB_CLK>,
+				         <&mmcc MDSS_HDMI_AHB_CLK>;
+				clock-names = "iface",
+				              "alt_iface";
+
+				core-vdda-supply = <&pm8941_l12>;
+				vddio-supply = <&pm8941_s3>;
+
+				#phy-cells = <0>;
+			};
 		};
 	};
 
-- 
2.21.0


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

^ permalink raw reply related

* [PATCH 09/11] ARM: dts: qcom: pm8941: add 5vs2 regulator node
From: Brian Masney @ 2019-08-15  0:48 UTC (permalink / raw)
  To: bjorn.andersson, robh+dt, agross, a.hajda, narmstrong, robdclark,
	sean
  Cc: mark.rutland, devicetree, jernej.skrabec, jonas, airlied,
	linux-arm-msm, linus.walleij, linux-kernel, dri-devel,
	Laurent.pinchart, daniel, enric.balletbo, freedreno,
	linux-arm-kernel
In-Reply-To: <20190815004854.19860-1-masneyb@onstation.org>

pm8941 is missing the 5vs2 regulator node so let's add it since its
needed to get the external display working. This regulator was already
configured in the interrupts property on the parent node.

Note that this regulator is referred to as mvs2 in the downstream MSM
kernel sources.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 arch/arm/boot/dts/qcom-pm8941.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-pm8941.dtsi b/arch/arm/boot/dts/qcom-pm8941.dtsi
index f198480c8ef4..c1f2012d1c8b 100644
--- a/arch/arm/boot/dts/qcom-pm8941.dtsi
+++ b/arch/arm/boot/dts/qcom-pm8941.dtsi
@@ -178,6 +178,16 @@
 				qcom,vs-soft-start-strength = <0>;
 				regulator-initial-mode = <1>;
 			};
+
+			pm8941_5vs2: 5vs2 {
+				regulator-enable-ramp-delay = <1000>;
+				regulator-pull-down;
+				regulator-over-current-protection;
+				qcom,ocp-max-retries = <10>;
+				qcom,ocp-retry-delay = <30>;
+				qcom,vs-soft-start-strength = <0>;
+				regulator-initial-mode = <1>;
+			};
 		};
 	};
 };
-- 
2.21.0


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

^ permalink raw reply related

* [PATCH 08/11] drm/msm/hdmi: silence -EPROBE_DEFER warning
From: Brian Masney @ 2019-08-15  0:48 UTC (permalink / raw)
  To: bjorn.andersson, robh+dt, agross, a.hajda, narmstrong, robdclark,
	sean
  Cc: mark.rutland, devicetree, jernej.skrabec, jonas, airlied,
	linux-arm-msm, linus.walleij, linux-kernel, dri-devel,
	Laurent.pinchart, daniel, enric.balletbo, freedreno,
	linux-arm-kernel
In-Reply-To: <20190815004854.19860-1-masneyb@onstation.org>

Silence a warning message due to an -EPROBE_DEFER error to help cleanup
the system boot log.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/gpu/drm/msm/hdmi/hdmi_phy.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_phy.c b/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
index 1697e61f9c2f..8a38d4b95102 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
@@ -29,8 +29,12 @@ static int msm_hdmi_phy_resource_init(struct hdmi_phy *phy)
 		reg = devm_regulator_get(dev, cfg->reg_names[i]);
 		if (IS_ERR(reg)) {
 			ret = PTR_ERR(reg);
-			DRM_DEV_ERROR(dev, "failed to get phy regulator: %s (%d)\n",
-				cfg->reg_names[i], ret);
+			if (ret != -EPROBE_DEFER) {
+				DRM_DEV_ERROR(dev,
+					      "failed to get phy regulator: %s (%d)\n",
+					      cfg->reg_names[i], ret);
+			}
+
 			return ret;
 		}
 
-- 
2.21.0


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

^ permalink raw reply related

* [PATCH 07/11] ARM: qcom_defconfig: add CONFIG_DRM_ANALOGIX_ANX78XX
From: Brian Masney @ 2019-08-15  0:48 UTC (permalink / raw)
  To: bjorn.andersson, robh+dt, agross, a.hajda, narmstrong, robdclark,
	sean
  Cc: mark.rutland, devicetree, jernej.skrabec, jonas, airlied,
	linux-arm-msm, linus.walleij, linux-kernel, dri-devel,
	Laurent.pinchart, daniel, enric.balletbo, freedreno,
	linux-arm-kernel
In-Reply-To: <20190815004854.19860-1-masneyb@onstation.org>

Add CONFIG_DRM_ANALOGIX_ANX78XX as a module so that the external display
can be used on the Nexus 5 phones.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 arch/arm/configs/qcom_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig
index 34433bf5885d..139e6610f034 100644
--- a/arch/arm/configs/qcom_defconfig
+++ b/arch/arm/configs/qcom_defconfig
@@ -148,6 +148,7 @@ CONFIG_REGULATOR_QCOM_SPMI=y
 CONFIG_MEDIA_SUPPORT=y
 CONFIG_DRM=y
 CONFIG_DRM_PANEL_SIMPLE=y
+CONFIG_DRM_ANALOGIX_ANX78XX=m
 CONFIG_FB=y
 CONFIG_FRAMEBUFFER_CONSOLE=y
 # CONFIG_LCD_CLASS_DEVICE is not set
-- 
2.21.0


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

^ permalink raw reply related

* [PATCH RFC 06/11] drm/bridge: analogix-anx78xx: add support for avdd33 regulator
From: Brian Masney @ 2019-08-15  0:48 UTC (permalink / raw)
  To: bjorn.andersson, robh+dt, agross, a.hajda, narmstrong, robdclark,
	sean
  Cc: mark.rutland, devicetree, jernej.skrabec, jonas, airlied,
	linux-arm-msm, linus.walleij, linux-kernel, dri-devel,
	Laurent.pinchart, daniel, enric.balletbo, freedreno,
	linux-arm-kernel
In-Reply-To: <20190815004854.19860-1-masneyb@onstation.org>

Add support for the avdd33 regulator to the analogix-anx78xx driver.
Note that the regulator is currently enabled during driver probe and
disabled when the driver is removed. This is currently how the
downstream MSM kernel sources do this.

Let's not merge this upstream for the mean time until I get the external
display fully working on the Nexus 5 and then I can submit proper
support then that powers down this regulator in the power off function.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/gpu/drm/bridge/analogix-anx78xx.c | 33 +++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
index 8daee6b1fa88..48adf010816c 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
@@ -48,6 +48,7 @@ static const u8 anx78xx_i2c_addresses[] = {
 
 struct anx78xx_platform_data {
 	struct regulator *dvdd10;
+	struct regulator *avdd33;
 	struct gpio_desc *gpiod_hpd;
 	struct gpio_desc *gpiod_pd;
 	struct gpio_desc *gpiod_reset;
@@ -707,10 +708,42 @@ static int anx78xx_start(struct anx78xx *anx78xx)
 	return err;
 }
 
+static void anx78xx_disable_regulator_action(void *_data)
+{
+	struct anx78xx_platform_data *pdata = _data;
+
+	regulator_disable(pdata->avdd33);
+}
+
 static int anx78xx_init_pdata(struct anx78xx *anx78xx)
 {
 	struct anx78xx_platform_data *pdata = &anx78xx->pdata;
 	struct device *dev = &anx78xx->client->dev;
+	int err;
+
+	/* 3.3V digital core power regulator  */
+	pdata->avdd33 = devm_regulator_get(dev, "avdd33");
+	if (IS_ERR(pdata->avdd33)) {
+		err = PTR_ERR(pdata->avdd33);
+		if (err != -EPROBE_DEFER)
+			DRM_ERROR("avdd33 regulator not found\n");
+
+		return err;
+	}
+
+	err = regulator_enable(pdata->avdd33);
+	if (err) {
+		DRM_ERROR("Failed to enable avdd33 regulator: %d\n", err);
+		return err;
+	}
+
+	err = devm_add_action(dev, anx78xx_disable_regulator_action,
+			      pdata);
+	if (err < 0) {
+		dev_err(dev, "Failed to setup regulator cleanup action %d\n",
+			err);
+		return err;
+	}
 
 	/* 1.0V digital core power regulator  */
 	pdata->dvdd10 = devm_regulator_get(dev, "dvdd10");
-- 
2.21.0


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

^ permalink raw reply related

* [PATCH 05/11] drm/bridge: analogix-anx78xx: correct value of TX_P0
From: Brian Masney @ 2019-08-15  0:48 UTC (permalink / raw)
  To: bjorn.andersson, robh+dt, agross, a.hajda, narmstrong, robdclark,
	sean
  Cc: mark.rutland, devicetree, jernej.skrabec, jonas, airlied,
	linux-arm-msm, linus.walleij, linux-kernel, dri-devel,
	Laurent.pinchart, daniel, enric.balletbo, freedreno,
	linux-arm-kernel
In-Reply-To: <20190815004854.19860-1-masneyb@onstation.org>

When attempting to configure this driver on a Nexus 5 phone (msm8974),
setting up the dummy i2c bus for TX_P0 would fail due to an -EBUSY
error. The downstream MSM kernel sources [1] shows that the proper value
for TX_P0 is 0x78, not 0x70, so correct the value to allow device
probing to succeed.

[1] https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/drivers/video/slimport/slimport_tx_reg.h

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/gpu/drm/bridge/analogix-anx78xx.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.h b/drivers/gpu/drm/bridge/analogix-anx78xx.h
index 25e063bcecbc..bc511fc605c9 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.h
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.h
@@ -6,7 +6,7 @@
 #ifndef __ANX78xx_H
 #define __ANX78xx_H
 
-#define TX_P0				0x70
+#define TX_P0				0x78
 #define TX_P1				0x7a
 #define TX_P2				0x72
 
-- 
2.21.0


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

^ permalink raw reply related

* [PATCH 03/11] drm/bridge: analogix-anx78xx: silence -EPROBE_DEFER warnings
From: Brian Masney @ 2019-08-15  0:48 UTC (permalink / raw)
  To: bjorn.andersson, robh+dt, agross, a.hajda, narmstrong, robdclark,
	sean
  Cc: mark.rutland, devicetree, jernej.skrabec, jonas, airlied,
	linux-arm-msm, linus.walleij, linux-kernel, dri-devel,
	Laurent.pinchart, daniel, enric.balletbo, freedreno,
	linux-arm-kernel
In-Reply-To: <20190815004854.19860-1-masneyb@onstation.org>

Silence two warning messages that occur due to -EPROBE_DEFER errors to
help cleanup the system boot log.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/gpu/drm/bridge/analogix-anx78xx.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
index 9acdbedf1245..62dfced91384 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
@@ -715,7 +715,9 @@ static int anx78xx_init_pdata(struct anx78xx *anx78xx)
 	/* 1.0V digital core power regulator  */
 	pdata->dvdd10 = devm_regulator_get(dev, "dvdd10");
 	if (IS_ERR(pdata->dvdd10)) {
-		DRM_ERROR("DVDD10 regulator not found\n");
+		if (PTR_ERR(pdata->dvdd10) != -EPROBE_DEFER)
+			DRM_ERROR("DVDD10 regulator not found\n");
+
 		return PTR_ERR(pdata->dvdd10);
 	}
 
@@ -1333,7 +1335,9 @@ static int anx78xx_i2c_probe(struct i2c_client *client,
 
 	err = anx78xx_init_pdata(anx78xx);
 	if (err) {
-		DRM_ERROR("Failed to initialize pdata: %d\n", err);
+		if (err != -EPROBE_DEFER)
+			DRM_ERROR("Failed to initialize pdata: %d\n", err);
+
 		return err;
 	}
 
-- 
2.21.0


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

^ permalink raw reply related

* [PATCH 00/11] ARM: dts: qcom: msm8974: add support for external display
From: Brian Masney @ 2019-08-15  0:48 UTC (permalink / raw)
  To: bjorn.andersson, robh+dt, agross, a.hajda, narmstrong, robdclark,
	sean
  Cc: mark.rutland, devicetree, jernej.skrabec, jonas, airlied,
	linux-arm-msm, linus.walleij, linux-kernel, dri-devel,
	Laurent.pinchart, daniel, enric.balletbo, freedreno,
	linux-arm-kernel

This patch series begins to add support for the external display over
HDMI that is supported on msm8974 SoCs. I'm testing this series on the
Nexus 5, and I'm able to communicate with the HDMI bridge via the
analogix-anx78xx driver, however the external display is not working
yet.

When I plug in the HDMI cable, the monitor detects that a device is
hooked up, but nothing is shown on the external monitor. The hot plug
detect GPIO (hpd-gpios) on the analogix-anx78xx bridge and MSM HDMI
drivers do not change state when the slimport adapter or HDMI cable is
plugged in or removed. I wonder if a regulator is not enabled somewhere?
I have a comment in patch 10 regarding 'hpd-gdsc-supply' that may
potentially be an issue.

I'm still digging in on this, however I'd appreciate any feedback if
anyone has time. Most of these patches are ready now, so I marked the
ones that aren't ready with 'PATCH RFC'.

I'm using an Analogix Semiconductor SP6001 SlimPort Micro-USB to 4K HDMI
Adapter to connect my phone to an external display via a standard HDMI
cable. This works just fine with the downstream MSM kernel using
Android.

Brian Masney (11):
  dt-bindings: drm/bridge: analogix-anx78xx: add new variants
  drm/bridge: analogix-anx78xx: add new variants
  drm/bridge: analogix-anx78xx: silence -EPROBE_DEFER warnings
  drm/bridge: analogix-anx78xx: convert to i2c_new_dummy_device
  drm/bridge: analogix-anx78xx: correct value of TX_P0
  drm/bridge: analogix-anx78xx: add support for avdd33 regulator
  ARM: qcom_defconfig: add CONFIG_DRM_ANALOGIX_ANX78XX
  drm/msm/hdmi: silence -EPROBE_DEFER warning
  ARM: dts: qcom: pm8941: add 5vs2 regulator node
  ARM: dts: qcom: msm8974: add HDMI nodes
  ARM: dts: qcom: msm8974-hammerhead: add support for external display

 .../bindings/display/bridge/anx7814.txt       |   6 +-
 .../qcom-msm8974-lge-nexus5-hammerhead.dts    | 140 ++++++++++++++++++
 arch/arm/boot/dts/qcom-msm8974.dtsi           |  80 ++++++++++
 arch/arm/boot/dts/qcom-pm8941.dtsi            |  10 ++
 arch/arm/configs/qcom_defconfig               |   1 +
 drivers/gpu/drm/bridge/analogix-anx78xx.c     |  60 +++++++-
 drivers/gpu/drm/bridge/analogix-anx78xx.h     |   2 +-
 drivers/gpu/drm/msm/hdmi/hdmi_phy.c           |   8 +-
 8 files changed, 295 insertions(+), 12 deletions(-)

-- 
2.21.0


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

^ permalink raw reply

* [PATCH 04/11] drm/bridge: analogix-anx78xx: convert to i2c_new_dummy_device
From: Brian Masney @ 2019-08-15  0:48 UTC (permalink / raw)
  To: bjorn.andersson, robh+dt, agross, a.hajda, narmstrong, robdclark,
	sean
  Cc: mark.rutland, devicetree, jernej.skrabec, jonas, airlied,
	linux-arm-msm, linus.walleij, linux-kernel, dri-devel,
	Laurent.pinchart, daniel, enric.balletbo, freedreno,
	linux-arm-kernel
In-Reply-To: <20190815004854.19860-1-masneyb@onstation.org>

The i2c_new_dummy() function is deprecated since it returns NULL on
error. Change this to use the recommended replacement
i2c_new_dummy_device() that returns an error code that can be read with
PTR_ERR() and friends.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/gpu/drm/bridge/analogix-anx78xx.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
index 62dfced91384..8daee6b1fa88 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
@@ -1355,15 +1355,18 @@ static int anx78xx_i2c_probe(struct i2c_client *client,
 
 	/* Map slave addresses of ANX7814 */
 	for (i = 0; i < I2C_NUM_ADDRESSES; i++) {
-		anx78xx->i2c_dummy[i] = i2c_new_dummy(client->adapter,
-						anx78xx_i2c_addresses[i] >> 1);
-		if (!anx78xx->i2c_dummy[i]) {
-			err = -ENOMEM;
-			DRM_ERROR("Failed to reserve I2C bus %02x\n",
-				  anx78xx_i2c_addresses[i]);
+		struct i2c_client *i2c_dummy;
+
+		i2c_dummy = i2c_new_dummy_device(client->adapter,
+						 anx78xx_i2c_addresses[i] >> 1);
+		if (IS_ERR(i2c_dummy)) {
+			err = PTR_ERR(i2c_dummy);
+			DRM_ERROR("Failed to reserve I2C bus %02x: %d\n",
+				  anx78xx_i2c_addresses[i], err);
 			goto err_unregister_i2c;
 		}
 
+		anx78xx->i2c_dummy[i] = i2c_dummy;
 		anx78xx->map[i] = devm_regmap_init_i2c(anx78xx->i2c_dummy[i],
 						       &anx78xx_regmap_config);
 		if (IS_ERR(anx78xx->map[i])) {
-- 
2.21.0


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

^ permalink raw reply related

* [PATCH 01/11] dt-bindings: drm/bridge: analogix-anx78xx: add new variants
From: Brian Masney @ 2019-08-15  0:48 UTC (permalink / raw)
  To: bjorn.andersson, robh+dt, agross, a.hajda, narmstrong, robdclark,
	sean
  Cc: mark.rutland, devicetree, jernej.skrabec, jonas, airlied,
	linux-arm-msm, linus.walleij, linux-kernel, dri-devel,
	Laurent.pinchart, daniel, enric.balletbo, freedreno,
	linux-arm-kernel
In-Reply-To: <20190815004854.19860-1-masneyb@onstation.org>

Add support for the analogix,anx7808, analogix,anx7812, and
analogix,anx7818 variants.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 .../devicetree/bindings/display/bridge/anx7814.txt          | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/anx7814.txt b/Documentation/devicetree/bindings/display/bridge/anx7814.txt
index dbd7c84ee584..17258747fff6 100644
--- a/Documentation/devicetree/bindings/display/bridge/anx7814.txt
+++ b/Documentation/devicetree/bindings/display/bridge/anx7814.txt
@@ -6,7 +6,11 @@ designed for portable devices.
 
 Required properties:
 
- - compatible		: "analogix,anx7814"
+ - compatible		: Must be one of:
+			  "analogix,anx7808"
+			  "analogix,anx7812"
+			  "analogix,anx7814"
+			  "analogix,anx7818"
  - reg			: I2C address of the device
  - interrupts		: Should contain the INTP interrupt
  - hpd-gpios		: Which GPIO to use for hpd
-- 
2.21.0


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

^ permalink raw reply related

* [PATCH 02/11] drm/bridge: analogix-anx78xx: add new variants
From: Brian Masney @ 2019-08-15  0:48 UTC (permalink / raw)
  To: bjorn.andersson, robh+dt, agross, a.hajda, narmstrong, robdclark,
	sean
  Cc: mark.rutland, devicetree, jernej.skrabec, jonas, airlied,
	linux-arm-msm, linus.walleij, linux-kernel, dri-devel,
	Laurent.pinchart, daniel, enric.balletbo, freedreno,
	linux-arm-kernel
In-Reply-To: <20190815004854.19860-1-masneyb@onstation.org>

Add support for the 7808 variant. While we're here, the of match table
was missing support for the 7812 and 7818 variants, so add them as well.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/gpu/drm/bridge/analogix-anx78xx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
index 3c7cc5af735c..9acdbedf1245 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
@@ -1301,6 +1301,7 @@ static const struct regmap_config anx78xx_regmap_config = {
 };
 
 static const u16 anx78xx_chipid_list[] = {
+	0x7808,
 	0x7812,
 	0x7814,
 	0x7818,
@@ -1463,7 +1464,10 @@ MODULE_DEVICE_TABLE(i2c, anx78xx_id);
 
 #if IS_ENABLED(CONFIG_OF)
 static const struct of_device_id anx78xx_match_table[] = {
+	{ .compatible = "analogix,anx7808", },
+	{ .compatible = "analogix,anx7812", },
 	{ .compatible = "analogix,anx7814", },
+	{ .compatible = "analogix,anx7818", },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, anx78xx_match_table);
-- 
2.21.0


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

^ permalink raw reply related

* Re: [PATCH v6 5/8] clk: mediatek: Add MT6765 clock support
From: Stephen Boyd @ 2019-08-15  0:27 UTC (permalink / raw)
  To: Macpaul Lin, Marc Zyngier, Mars Cheng, Matthias Brugger,
	Owen Chen, Rob Herring, Ryder Lee, Sean Wang, linux-arm-kernel,
	linux-kernel, linux-mediatek
  Cc: devicetree, CC Hwang, wsd_upstream, Loda Chou, Macpaul Lin,
	linux-serial, linux-clk
In-Reply-To: <1562924653-10056-6-git-send-email-macpaul.lin@mediatek.com>

Quoting Macpaul Lin (2019-07-12 02:43:41)
> diff --git a/drivers/clk/mediatek/clk-mt6765-audio.c b/drivers/clk/mediatek/clk-mt6765-audio.c
> new file mode 100644
> index 000000000000..41f19343dfb9
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mt6765-audio.c
> @@ -0,0 +1,109 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: Owen Chen <owen.chen@mediatek.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.

Please use SPDX tags.

> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/platform_device.h>
> +
> +#include "clk-mtk.h"
> +#include "clk-gate.h"
> +
> diff --git a/drivers/clk/mediatek/clk-mt6765-vcodec.c b/drivers/clk/mediatek/clk-mt6765-vcodec.c
> new file mode 100644
> index 000000000000..eb9ae1c2c99c
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mt6765-vcodec.c
> @@ -0,0 +1,79 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: Owen Chen <owen.chen@mediatek.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */

SPDX tags.

> diff --git a/drivers/clk/mediatek/clk-mt6765.c b/drivers/clk/mediatek/clk-mt6765.c
> new file mode 100644
> index 000000000000..f716a48a926d
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mt6765.c
> @@ -0,0 +1,961 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: Owen Chen <owen.chen@mediatek.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.

SPDX tags.

> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +#include <linux/mfd/syscon.h>

Is this used? Maybe I deleted it.

> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
[...]
> +
> +static const char * const axi_parents[] = {
> +       "clk26m",
> +       "syspll_d7",
> +       "syspll1_d4",
> +       "syspll3_d2"
> +};
> +
> +static const char * const mem_parents[] = {
> +       "clk26m",
> +       "dmpll_ck",
> +       "apll1_ck"
> +};
> +
> +static const char * const mm_parents[] = {
> +       "clk26m",
> +       "mmpll_ck",
> +       "syspll1_d2",
> +       "syspll_d5",
> +       "syspll1_d4",
> +       "univpll_d5",
> +       "univpll1_d2",
> +       "mmpll_d2"
> +};
> +
> +static const char * const scp_parents[] = {
> +       "clk26m",
> +       "syspll4_d2",
> +       "univpll2_d2",
> +       "syspll1_d2",
> +       "univpll1_d2",
> +       "syspll_d3",
> +       "univpll_d3"
> +};
> +
> +static const char * const mfg_parents[] = {
> +       "clk26m",
> +       "mfgpll_ck",
> +       "syspll_d3",
> +       "univpll_d3"
> +};
> +
> +static const char * const atb_parents[] = {
> +       "clk26m",
> +       "syspll1_d4",
> +       "syspll1_d2"
> +};
> +
> +static const char * const camtg_parents[] = {
> +       "clk26m",
> +       "usb20_192m_d8",
> +       "univpll2_d8",
> +       "usb20_192m_d4",
> +       "univpll2_d32",
> +       "usb20_192m_d16",
> +       "usb20_192m_d32"
> +};
> +
> +static const char * const uart_parents[] = {
> +       "clk26m",
> +       "univpll2_d8"
> +};
> +
> +static const char * const spi_parents[] = {
> +       "clk26m",
> +       "syspll3_d2",
> +       "syspll4_d2",
> +       "syspll2_d4"
> +};
> +
> +static const char * const msdc5hclk_parents[] = {
> +       "clk26m",
> +       "syspll1_d2",
> +       "univpll1_d4",
> +       "syspll2_d2"
> +};
> +
> +static const char * const msdc50_0_parents[] = {
> +       "clk26m",
> +       "msdcpll_ck",
> +       "syspll2_d2",
> +       "syspll4_d2",
> +       "univpll1_d2",
> +       "syspll1_d2",
> +       "univpll_d5",
> +       "univpll1_d4"
> +};
> +
> +static const char * const msdc30_1_parents[] = {
> +       "clk26m",
> +       "msdcpll_d2",
> +       "univpll2_d2",
> +       "syspll2_d2",
> +       "syspll1_d4",
> +       "univpll1_d4",
> +       "usb20_192m_d4",
> +       "syspll2_d4"
> +};
> +
> +static const char * const audio_parents[] = {
> +       "clk26m",
> +       "syspll3_d4",
> +       "syspll4_d4",
> +       "syspll1_d16"
> +};
> +
> +static const char * const aud_intbus_parents[] = {
> +       "clk26m",
> +       "syspll1_d4",
> +       "syspll4_d2"
> +};
> +
> +static const char * const aud_1_parents[] = {
> +       "clk26m",
> +       "apll1_ck"
> +};
> +
> +static const char * const aud_engen1_parents[] = {
> +       "clk26m",
> +       "apll1_d2",
> +       "apll1_d4",
> +       "apll1_d8"
> +};
> +
> +static const char * const disp_pwm_parents[] = {
> +       "clk26m",
> +       "univpll2_d4",
> +       "ulposc1_d2",
> +       "ulposc1_d8"
> +};
> +
> +static const char * const sspm_parents[] = {
> +       "clk26m",
> +       "syspll1_d2",
> +       "syspll_d3"
> +};
> +
> +static const char * const dxcc_parents[] = {
> +       "clk26m",
> +       "syspll1_d2",
> +       "syspll1_d4",
> +       "syspll1_d8"
> +};
> +
> +static const char * const usb_top_parents[] = {
> +       "clk26m",
> +       "univpll3_d4"
> +};
> +
> +static const char * const spm_parents[] = {
> +       "clk26m",
> +       "syspll1_d8"
> +};
> +
> +static const char * const i2c_parents[] = {
> +       "clk26m",
> +       "univpll3_d4",
> +       "univpll3_d2",
> +       "syspll1_d8",
> +       "syspll2_d8"
> +};
> +
> +static const char * const pwm_parents[] = {
> +       "clk26m",
> +       "univpll3_d4",
> +       "syspll1_d8"
> +};
> +
> +static const char * const seninf_parents[] = {
> +       "clk26m",
> +       "univpll1_d4",
> +       "univpll1_d2",
> +       "univpll2_d2"
> +};
> +
> +static const char * const aes_fde_parents[] = {
> +       "clk26m",
> +       "msdcpll_ck",
> +       "univpll_d3",
> +       "univpll2_d2",
> +       "univpll1_d2",
> +       "syspll1_d2"
> +};
> +
> +static const char * const ulposc_parents[] = {
> +       "clk26m",
> +       "ulposc1_d4",
> +       "ulposc1_d8",
> +       "ulposc1_d16",
> +       "ulposc1_d32"
> +};
> +
> +static const char * const camtm_parents[] = {
> +       "clk26m",
> +       "univpll1_d4",
> +       "univpll1_d2",
> +       "univpll2_d2"
> +};
> +

Can you migrate this driver to the new way of specifying clk parents?
That way we don't just have lists of strings.

> +#define INVALID_UPDATE_REG 0xFFFFFFFF
> +#define INVALID_UPDATE_SHIFT -1
> +#define INVALID_MUX_GATE -1
> +
> +static const struct mtk_mux top_muxes[] = {
> +       /* CLK_CFG_0 */
> +       MUX_GATE_CLR_SET_UPD_FLAGS(CLK_TOP_AXI_SEL, "axi_sel", axi_parents,
> +                             CLK_CFG_0, CLK_CFG_0_SET, CLK_CFG_0_CLR,
> +                             0, 2, 7, CLK_CFG_UPDATE, 0, CLK_IS_CRITICAL),

Please add a comment why CLK_IS_CRITICAL flag is used in each place.

> +       MUX_GATE_CLR_SET_UPD_FLAGS(CLK_TOP_MEM_SEL, "mem_sel", mem_parents,
> +                             CLK_CFG_0, CLK_CFG_0_SET, CLK_CFG_0_CLR,
> +                             8, 2, 15, CLK_CFG_UPDATE, 1, CLK_IS_CRITICAL),
> +       MUX_GATE_CLR_SET_UPD(CLK_TOP_MM_SEL, "mm_sel", mm_parents, CLK_CFG_0,
> +                       CLK_CFG_0_SET, CLK_CFG_0_CLR, 16, 3, 23,
> +                       CLK_CFG_UPDATE, 2),
> +       MUX_GATE_CLR_SET_UPD(CLK_TOP_SCP_SEL, "scp_sel", scp_parents, CLK_CFG_0,
> +                       CLK_CFG_0_SET, CLK_CFG_0_CLR, 24, 3, 31,
> +                       CLK_CFG_UPDATE, 3),
[...]
> +       }, {
> +               .compatible = "mediatek,mt6765-topckgen",
> +               .data = clk_mt6765_top_probe,
> +       }, {
> +               .compatible = "mediatek,mt6765-infracfg",
> +               .data = clk_mt6765_ifr_probe,
> +       }, {
> +               /* sentinel */
> +       }
> +};
> +
> +static int clk_mt6765_probe(struct platform_device *pdev)
> +{
> +       int (*clk_probe)(struct platform_device *d);
> +       int r;
> +
> +       clk_probe = of_device_get_match_data(&pdev->dev);
> +       if (!clk_probe)
> +               return -EINVAL;
> +
> +       r = clk_probe(pdev);
> +       if (r)
> +               dev_err(&pdev->dev,
> +                       "could not register clock provider: %s: %d\n",
> +                       pdev->name, r);
> +
> +       return r;
> +}
> +
> +static struct platform_driver clk_mt6765_drv = {
> +       .probe = clk_mt6765_probe,
> +       .driver = {
> +               .name = "clk-mt6765",
> +               .owner = THIS_MODULE,

Remove this line, platform_driver_register() should take care of it.

> +               .of_match_table = of_match_clk_mt6765,
> +       },
> +};
> +

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

^ permalink raw reply

* Re: [PATCH v5 1/4] clk: core: link consumer with clock driver
From: Stephen Boyd @ 2019-08-15  0:03 UTC (permalink / raw)
  To: Michael Turquette, Miquel Raynal, Russell King
  Cc: Antoine Tenart, Gregory Clement, linux-kernel, Maxime Chevallier,
	Nadav Haklai, Thomas Petazzoni, Miquel Raynal, linux-clk,
	linux-arm-kernel
In-Reply-To: <20190521125114.20357-2-miquel.raynal@bootlin.com>

Quoting Miquel Raynal (2019-05-21 05:51:10)
> One major concern when, for instance, suspending/resuming a platform
> is to never access registers before the underlying clock has been
> resumed, otherwise most of the time the kernel will just crash. One
> solution is to use syscore operations when registering clock drivers
> suspend/resume callbacks. One problem of using syscore_ops is that the
> suspend/resume scheduling will depend on the order of the
> registrations, which brings (unacceptable) randomness in the process.
> 
> A feature called device links has been introduced to handle such
> situation. It creates dependencies between consumers and providers,
> enforcing e.g. the suspend/resume order when needed. Such feature is
> already in use for regulators.
> 
> Add device links support in the clock subsystem by creating/deleting
> the links at get/put time.
> 
> Example of a boot (ESPRESSObin, A3700 SoC) with devices linked to clocks:
> 
> marvell-armada-3700-tbg-clock d0013200.tbg: Linked as a consumer to d0013800.pinctrl:xtal-clk
> marvell-armada-3700-tbg-clock d0013200.tbg: Dropping the link to d0013800.pinctrl:xtal-clk
> marvell-armada-3700-tbg-clock d0013200.tbg: Linked as a consumer to d0013800.pinctrl:xtal-clk
> marvell-armada-3700-periph-clock d0013000.nb-periph-clk: Linked as a consumer to d0013200.tbg
> marvell-armada-3700-periph-clock d0013000.nb-periph-clk: Linked as a consumer to d0013800.pinctrl:xtal-clk
> marvell-armada-3700-periph-clock d0018000.sb-periph-clk: Linked as a consumer to d0013200.tbg
> mvneta d0030000.ethernet: Linked as a consumer to d0018000.sb-periph-clk
> xhci-hcd d0058000.usb: Linked as a consumer to d0018000.sb-periph-clk
> xenon-sdhci d00d0000.sdhci: Linked as a consumer to d0013000.nb-periph-clk
> xenon-sdhci d00d0000.sdhci: Dropping the link to d0013000.nb-periph-clk
> mvebu-uart d0012000.serial: Linked as a consumer to d0013800.pinctrl:xtal-clk
> advk-pcie d0070000.pcie: Linked as a consumer to d0018000.sb-periph-clk
> xenon-sdhci d00d0000.sdhci: Linked as a consumer to d0013000.nb-periph-clk
> xenon-sdhci d00d0000.sdhci: Linked as a consumer to regulator.1
> cpu cpu0: Linked as a consumer to d0013000.nb-periph-clk
> cpu cpu0: Dropping the link to d0013000.nb-periph-clk
> cpu cpu0: Linked as a consumer to d0013000.nb-periph-clk
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---

This patch doesn't apply. Things have changed upstream.

> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index ec6f04dcf5e6..e6b84ab43f9f 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1676,6 +1710,8 @@ static void clk_reparent(struct clk_core *core, struct clk_core *new_parent)
>  
>                 if (was_orphan != becomes_orphan)
>                         clk_core_update_orphan_status(core, becomes_orphan);
> +
> +               clk_link_hierarchy(core, new_parent);

This isn't going to work.

 BUG: sleeping function called from invalid context at kernel/locking/mutex.c:909
 in_atomic(): 1, irqs_disabled(): 128, pid: 1, name: swapper/0
 3 locks held by swapper/0/1:
  #0: (____ptrval____) (&dev->mutex){....}, at: __device_driver_lock+0x40/0x4c
  #1: (____ptrval____) (prepare_lock){+.+.}, at: clk_prepare_lock+0x18/0x94
  #2: (____ptrval____) (enable_lock){....}, at: clk_enable_lock+0x34/0xdc
 irq event stamp: 311516
 hardirqs last  enabled at (311515): [<ffffff901fce5c90>] _raw_spin_unlock_irqrestore+0x54/0x90
 hardirqs last disabled at (311516): [<ffffff901f73d468>] clk_enable_lock+0x28/0xdc
 softirqs last  enabled at (311348): [<ffffff901f28188c>] __do_softirq+0x4cc/0x514
 softirqs last disabled at (311341): [<ffffff901f2f89ac>] irq_exit+0xd8/0xf8
 CPU: 4 PID: 1 Comm: swapper/0 Tainted: G        W         5.3.0-rc4-00005-g6be06bbec80ef #10
 Hardware name: Google Cheza (rev3+) (DT)
 Call trace:
  dump_backtrace+0x0/0x13c
  show_stack+0x20/0x2c
  dump_stack+0xc4/0x12c
  ___might_sleep+0x1b4/0x1c4
  __might_sleep+0x50/0x88
  __mutex_lock_common+0x5c/0xbfc
  mutex_lock_nested+0x40/0x50
  device_link_add+0x88/0x3ac
  clk_reparent+0xc4/0x114
  __clk_set_parent_before+0x74/0x90
  clk_change_rate+0x98/0x854
  clk_core_set_rate_nolock+0x1b0/0x21c
  clk_set_rate+0x3c/0x6c
  of_clk_set_defaults+0x29c/0x364
  platform_drv_probe+0x28/0xb0
  really_probe+0x130/0x2b4
  driver_probe_device+0x64/0xfc
  device_driver_attach+0x4c/0x6c
  __driver_attach+0xb0/0xc4
  bus_for_each_dev+0x84/0xcc
  driver_attach+0x2c/0x38
  bus_add_driver+0xfc/0x1d0
  driver_register+0x64/0xf0
  __platform_driver_register+0x4c/0x58
  msm_drm_register+0x5c/0x60
  do_one_initcall+0x1e0/0x478
  do_initcall_level+0x21c/0x25c
  do_basic_setup+0x60/0x78
  kernel_init_freeable+0x128/0x1b0
  kernel_init+0x14/0x100
  ret_from_fork+0x10/0x18

>         } else {
>                 hlist_add_head(&core->child_node, &clk_orphan_list);
>                 if (!was_orphan)
> @@ -2402,6 +2438,8 @@ __clk_init_parent(struct clk_core *core, bool update_orphan)
>         if (!parent_hw)
>                 return NULL;
>  
> +       clk_link_hierarchy(core, parent_hw->core);
> +

This is the hunk that doesn't apply anymore.

>         return parent_hw->core;
>  }
>  

The general thought is that it would be good to _not_ call the device
link APIs from deep within the clk parent changing code or even parent
initialization code. It would be better to make device links based on
the possible parents of a clk controller when the clk is registered and
after the clk prepare lock (i.e. the registration lock) is dropped. Is
this possible? The problem is that we're deeply nested in locks that are
already hard to reason about and get out from underneath. I don't want
to get into some sort of ABBA deadlock scenario with the PM core. The
usage of runtime PM in the clk framework is probably busted right now
because it is used under the prepare lock. Ugh.

Is it necessary to add the device links between different clk
controllers either? I mean, is it necessary to create links between clks
and their parents right now?  Maybe we can take the easy way out and
just make links between devices that call clk_get() and the devices that
provide those clks (the consumer side). I suppose you may want to order
suspend/resume of a device with the parent clks of some clk that is
acquired from clk_get(). I hope it isn't required though, because this
is a problem to do with ordering suspend/resume of the clk tree itself,
which isn't really solved at all.

We probably need to solve that by doing something clk provider specific
in the clk framework to figure out a way for device drivers that provide
clks to get callbacks to suspend/resume clks in the clk tree in some
sort of topo-sorted order. That way we can traverse the clk tree and
call down into provider drivers for each clk it registered to do things
like restore the clk frequency or clk enable/prepare state, etc. It
needs to be done in a certain order and it's not possible to flatten
that order into a sequential list of providers (that correspond 1:1 with
devices) given that there are loops between providers.

But from the perspective of a consumer driver like PCI, I don't see why
it needs to care about the clk tree suspend/resume ordering details. It
really only cares that the clk it's consuming, at the edge of the tree,
is resumed before the consumer itself, PCI, is resumed. However the
dependencies of that clk it's consuming is managed, be it with device
links or something clk framework specific, doesn't matter to the PCI
driver. And other clks that are parents or grandparents of the clk
consumed by PCI could have device link dependencies themselves, on
something like an i2c controller or such. Even then, we don't need to
use device links in the clk tree to describe ordering between clks. We
can do it without device links and break the device link chain when it
crosses the clk tree.

  PCI -[device link]-> PCI leaf clk provider -[clk framework ordering black box]-> parent of leaf clk -[device link]-> i2c controller 


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

^ permalink raw reply

* Re: [PATCH v2] coresight: tmc-etr: Fix perf_data check.
From: Yabin Cui @ 2019-08-14 23:50 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Alexander Shishkin
  Cc: Yabin Cui, linux-kernel, linux-arm-kernel
In-Reply-To: <CANLsYkz3_bzRCQEVb00Tbf3Rdww13mePN-woncctOu7OanF00A@mail.gmail.com>

> Did you actually see the check fail or is this a theoretical thing?
> I'm really perplex here has I have tested this scenario many times
> without issues.
>
I have seen this warning in dmesg output, that's how I find the problem.

> In CPU wide scenarios each perf event (one per CPU) is associated with
> an event_data during the setup process.  The event_data is the
> etr_perf holding a reference to the perf ring buffer for that specific
> event along with the etr_buf, regardless of who created the latter.

Agree.

> From there, when the event is installed on a CPU, the csdev for that
> CPU is given a reference to the event_data of that event[1].  Before
> going further notice how there is a per CPU csdev and event handle to
> keep track of event specifics[2]. As such both (per CPU) csdev and
> event handle carry the exact same reference to the etr_perf.
>
On my test device (Pixel 3), there is an ETM device on each cpu, but only
one ETR device for the whole device. So there is only one instance of etr
csdev in the kernel. If multiple cpus are scheduling on etm perf events at
the same time, all of them are trying to set their event_data to the same
etr csdev. And different perf events have different event_data. A warning
situation is as below:

   cpu 0
   schedule on event A (set etr csdev->perf_data to event_a.etr_perf)

   cpu 1
   schedule on event B (set etr csdev->perf_data to event_b.etr_perf)

   cpu 1
   schedule off event B (update buffer, does nothing since csdev->refcnt != 1)

   cpu 0
   schedule off event A (update buffer, but etr csdev->perf_data check fail)

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

^ permalink raw reply

* Re: [PATCH 04/22] media: Move v4l2_fwnode_parse_link from v4l2 to driver base
From: Russell King - ARM Linux admin @ 2019-08-14 23:15 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
	Laurent Pinchart, Shawn Guo, Fabio Estevam,
	open list:STAGING SUBSYSTEM, Mauro Carvalho Chehab, Michal Simek,
	open list:ACPI, Andy Shevchenko, NXP Linux Team, Len Brown,
	Philipp Zabel, Sascha Hauer, Thomas Gleixner, Andy Shevchenko,
	moderated list:ARM/ZYNQ ARCHITECTURE, Hyun Kwon,
	Greg Kroah-Hartman, open list, Jacopo Mondi,
	Pengutronix Kernel Team, Hans Verkuil, Linux Media Mailing List,
	Enrico Weigelt
In-Reply-To: <1842bf8f-4f97-6294-41db-74f9f8e2befd@gmail.com>

On Wed, Aug 14, 2019 at 04:00:30PM -0700, Steve Longerbeam wrote:
> 
> 
> On 8/14/19 3:04 PM, Russell King - ARM Linux admin wrote:
> > On Wed, Aug 14, 2019 at 12:04:41PM -0700, Steve Longerbeam wrote:
> > > 
> > > On 8/14/19 3:30 AM, Russell King - ARM Linux admin wrote:
> > > > On Tue, Aug 06, 2019 at 09:53:41AM -0700, Steve Longerbeam wrote:
> > > > > The full patchset doesn't seem to be up yet, but see [1] for the cover
> > > > > letter.
> > > > Was the entire series copied to the mailing lists, or just selected
> > > > patches?  I only saw 4, 9, 11 and 13-22 via lakml.
> > > The whole series was posted to the linux-media ML, see [1]. At the time,
> > > none of the linux-media ML archives had the whole series.
> > > 
> > > > In the absence of the other patches, will this solve imx-media binding
> > > > the internal subdevs of sensor devices to the CSI2 interface?
> > > "internal subdevs of sensor devices" ?? That doesn't make any sense.
> > Sorry, but it makes complete sense when you consider that sensor
> > devices may have more than one subdev, but there should be only one
> > that is the "output" to whatever the camera is attached to.  The
> > other subdevs are internal to the sensor.
> 
> Ah, thanks for the clarification. Yes, by "internal subdevs" I understand
> what you mean now. The adv748x and smiapp are examples.
> 
> > 
> > subdevs are not purely the remit of SoC drivers.
> 
> So there is no binding of internal subdevs to the receiver CSI-2. The
> receiver CSI-2 subdev will create media links to the subdev that has an
> externally exposed fwnode endpoint that connects with the CSI-2 sink pad.

Maybe - with 5.2, I get:

- entity 15: imx6-mipi-csi2 (5 pads, 6 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev2
        pad0: Sink
...
                <- "imx219 0-0010":0 []
                <- "imx219 pixel 0-0010":0 []

Adding some debug in gives:

[   11.963362] imx-media: imx_media_create_of_links() for imx6-mipi-csi2
[   11.963396] imx-media: create_of_link(): /soc/aips-bus@2000000/iomuxc-gpr@20e0000/ipu1_csi0_mux
[   11.963422] imx-media: create_of_link(): /soc/ipu@2400000
[   11.963450] imx-media: create_of_link(): /soc/ipu@2800000
[   11.963478] imx-media: create_of_link(): /soc/aips-bus@2000000/iomuxc-gpr@20e0000/ipu2_csi1_mux
[   11.963489] imx-media: imx6-mipi-csi2:4 -> ipu2_csi1_mux:0
[   11.963522] imx-media: create_of_link(): /soc/aips-bus@2100000/i2c@21a0000/camera@10
[   11.963533] imx-media: imx219 0-0010:0 -> imx6-mipi-csi2:0
[   11.963549] imx-media: imx_media_create_of_links() for imx219 pixel 0-0010
[   11.963577] imx-media: create_of_link(): /soc/aips-bus@2100000/mipi@21dc000
[   11.963587] imx-media: imx219 pixel 0-0010:0 -> imx6-mipi-csi2:0
[   11.963602] imx-media: imx_media_create_of_links() for imx219 0-0010

Note that it's not created by imx6-mipi-csi2, but by imx-media delving
around in the imx219 subdevs.

From what I can see, smiapp does the same thing that I do in imx219 -
sets the subdev->dev member to point at the struct device, which then
means that v4l2_device_register_subdev() will associate the same fwnode
with both "imx219 pixel 0-0010" and "imx219 0-0010".

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

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

^ permalink raw reply

* Re: [PATCH V2 09/13] dt-bindings: arm: Convert BCM2835 board/soc bindings to json-schema
From: Rob Herring @ 2019-08-14 23:06 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Mark Rutland, devicetree, Florian Fainelli, Scott Branden,
	Wolfram Sang, Stephen Boyd, Ray Jui, Michael Turquette,
	Eric Anholt, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
	Linux I2C, linux-clk,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2835 ARM ARCHITECTURE
In-Reply-To: <f9acf678-854d-720d-3c84-d9a05766c02e@gmx.net>

On Wed, Aug 14, 2019 at 1:21 PM Stefan Wahren <wahrenst@gmx.net> wrote:
>
> Hi Rob,
>
> Am 13.08.19 um 19:22 schrieb Rob Herring:
> > On Tue, Aug 13, 2019 at 10:21 AM Stefan Wahren <wahrenst@gmx.net> wrote:
> >> Convert the BCM2835/6/7 SoC bindings to DT schema format using json-schema.
> >> All the other Broadcom boards are maintained by Florian Fainelli.
> >>
> >> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> >> Acked-by: Eric Anholt <eric@anholt.net>
> >> ---
> >>  .../devicetree/bindings/arm/bcm/bcm2835.yaml       | 46 +++++++++++++++
> >>  .../devicetree/bindings/arm/bcm/brcm,bcm2835.txt   | 67 ----------------------
> >>  2 files changed, 46 insertions(+), 67 deletions(-)
> >>  create mode 100644 Documentation/devicetree/bindings/arm/bcm/bcm2835.yaml
> >>  delete mode 100644 Documentation/devicetree/bindings/arm/bcm/brcm,bcm2835.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/bcm/bcm2835.yaml b/Documentation/devicetree/bindings/arm/bcm/bcm2835.yaml
> >> new file mode 100644
> >> index 0000000..1a4be26
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/arm/bcm/bcm2835.yaml
> >> @@ -0,0 +1,46 @@
> >> +# SPDX-License-Identifier: GPL-2.0
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/arm/bcm/bcm2835.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Broadcom BCM2711/BCM2835 Platforms Device Tree Bindings
> >> +
> >> +maintainers:
> >> +  - Eric Anholt <eric@anholt.net>
> >> +  - Stefan Wahren <wahrenst@gmx.net>
> >> +
> >> +properties:
> >> +  $nodename:
> >> +    const: '/'
> >> +  compatible:
> >> +    oneOf:
> >> +      - description: BCM2835 based Boards
> >> +        items:
> >> +          - enum:
> >> +              - raspberrypi,model-a
> >> +              - raspberrypi,model-a-plus
> >> +              - raspberrypi,model-b
> >> +              - raspberrypi,model-b-i2c0  # Raspberry Pi Model B (no P5)
> >> +              - raspberrypi,model-b-rev2
> >> +              - raspberrypi,model-b-plus
> >> +              - raspberrypi,compute-module
> >> +              - raspberrypi,model-zero
> >> +              - raspberrypi,model-zero-w
> >> +          - const: brcm,bcm2835
> >> +
> >> +      - description: BCM2836 based Boards
> >> +        items:
> >> +          - enum:
> >> +              - raspberrypi,2-model-b
> > Don't you need brcm,bcm2836 here?
> >
> >> +
> >> +      - description: BCM2837 based Boards
> >> +        items:
> >> +          - enum:
> >> +              - raspberrypi,3-model-a-plus
> >> +              - raspberrypi,3-model-b
> >> +              - raspberrypi,3-model-b-plus
> >> +              - raspberrypi,3-compute-module
> >> +              - raspberrypi,3-compute-module-lite
> > Don't you need brcm,bcm2837 here?
> >
> > Please run 'dtbs_check' and make sure there aren't warnings (in the root node).
>
> thanks, after addressing your comments the root node doesn't have
> warnings anymore.
>
> Beside that there a lot of other warnings:
>
>   DTC     arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml
>   CHECK   arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml:
> arm-pmu: compatible: ['arm,cortex-a72-pmu', 'arm,armv8-pmuv3'] is too long
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml:
> arm-pmu: compatible: Additional items are not allowed ('arm,armv8-pmuv3'
> was unexpected)
>
> I think the schema is a little bit too strict by prohibit a fallback
> compatible.

IIRC, Will Deacon said there wasn't any point to fallbacks as every
cpu has its own list of events. Or if we want a fallback, then add it
to the schema.

>
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml:
> serial@7e201800: Additional properties are not allowed
> ('arm,primecell-periphid' was unexpected)
>
> In the old txt version this was an allowed property.

You really need an override? Haven't seen that in a while. We can drop
'additionalProperties: false' in the pl011 schema.

>
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml:
> firmware: $nodename:0: 'firmware' does not match
> '^(bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$'
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml:
> firmware: '#address-cells' is a required property
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml:
> firmware: '#size-cells' is a required property
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml:
> firmware: 'ranges' is a required property
>
> I suggest to fix this by removing the "simple-bus".

Probably. 'firmware' doesn't sound like a bus.

>
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml:
> serial@7e201a00: Additional properties are not allowed
> ('arm,primecell-periphid' was unexpected)
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml:
> gpio@7e200000: 'pinctrl-0' is a dependency of 'pinctrl-names'
>
> This could be fixed by removing pinctrl-names.
>
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml:
> serial@7e201600: Additional properties are not allowed
> ('arm,primecell-periphid' was unexpected)
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml:
> gic400@40041000: $nodename:0: 'gic400@40041000' does not match
> '^interrupt-controller(@[0-9a-f,]+)*$'
>
> I will rename gic400 to interrupt-controller.
>
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml:
> serial@7e201400: Additional properties are not allowed
> ('arm,primecell-periphid' was unexpected)
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml:
> serial@7e201000: compatible: ['brcm,bcm2835-pl011', 'arm,pl011',
> 'arm,primecell'] is not valid under any of the given schemas
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml:
> serial@7e201000: Additional properties are not allowed ('bluetooth',
> 'arm,primecell-periphid' were unexpected)
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml:
> sd_io_1v8_reg: states:0: [1800000, 1, 3300000, 0] is too long
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml:
> sd_io_1v8_reg: states:0: Additional items are not allowed (3300000, 0
> were unexpected)
>
> No idea what is wrong here

The schema is stricter about <> groupings is my guess. Looks like this
should be 2 entries with 2 cells each.

> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml: clocks:
> #size-cells:0:0: 0 is not one of [1, 2]
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml: clocks:
> $nodename:0: 'clocks' does not match '^(bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$'
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml: clocks:
> clock@3:reg:0: [3] is too short
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml: clocks:
> clock@4:reg:0: [4] is too short
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml: clocks:
> 'ranges' is a required property
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml:
> clock@3: 'reg' does not match any of the regexes: 'pinctrl-[0-9]+'
> /home/stefan/torvalds/arch/arm/boot/dts/bcm2711-rpi-4-b.dt.yaml:
> clock@4: 'reg' does not match any of the regexes: 'pinctrl-[0-9]+'
>
> This could be fixed by avoiding a simple-bus for the fixed clocks.

Right, they are not a bus.

Rob

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

^ permalink raw reply

* Re: [PATCH v2] coresight: tmc-etr: Fix perf_data check.
From: Mathieu Poirier @ 2019-08-14 23:00 UTC (permalink / raw)
  To: Yabin Cui
  Cc: Alexander Shishkin, Linux Kernel Mailing List, linux-arm-kernel,
	Suzuki K Poulose
In-Reply-To: <20190812190320.209988-1-yabinc@google.com>

Hi Yabin,

On Mon, 12 Aug 2019 at 13:03, Yabin Cui <yabinc@google.com> wrote:
>
> When tracing etm data of multiple threads on multiple cpus through
> perf interface, each cpu has a unique etr_perf_buffer while sharing
> the same etr device. There is no guarantee that the last cpu starts
> etm tracing also stops last. This makes perf_data check fail.

Did you actually see the check fail or is this a theoretical thing?
I'm really perplex here has I have tested this scenario many times
without issues.

>
> Fix it by checking etr_buf instead of etr_perf_buffer.
>
> Fixes: 3147da92a8a8 ("coresight: tmc-etr: Allocate and free ETR memory buffers for CPU-wide scenarios")
> Signed-off-by: Yabin Cui <yabinc@google.com>
> ---
>
> v1 -> v2: rename perf_data to perf_buf. Add fixes tag.
>
> ---
>  drivers/hwtracing/coresight/coresight-tmc-etr.c | 6 +++---
>  drivers/hwtracing/coresight/coresight-tmc.h     | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 17006705287a..90d1548ad268 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -1484,7 +1484,7 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
>                 goto out;
>         }
>
> -       if (WARN_ON(drvdata->perf_data != etr_perf)) {
> +       if (WARN_ON(drvdata->perf_buf != etr_buf)) {

In CPU wide scenarios each perf event (one per CPU) is associated with
an event_data during the setup process.  The event_data is the
etr_perf holding a reference to the perf ring buffer for that specific
event along with the etr_buf, regardless of who created the latter.
From there, when the event is installed on a CPU, the csdev for that
CPU is given a reference to the event_data of that event[1].  Before
going further notice how there is a per CPU csdev and event handle to
keep track of event specifics[2]. As such both (per CPU) csdev and
event handle carry the exact same reference to the etr_perf.

When an event is stopped the exact same per CPU references are
taken[3] and later fed to sink->update_buffer().  If
sink->update_buffer() is called on an event (again one per CPU)
function etm_event_start() must have been called and
(drvdata->perf_data == etf_perf) needs to hold.

If that is not the case, an event from a completely different trace
session has been installed on that CPU and that can't happen thanks to
drvdata->id.

As such if you get a condition where (drvdata->perf_buf != etr_buf),
one of two things has happened:

1) Something went seriously wrong and the WARN_ON() did its job.
2) A corner case is manifesting in your test environment that I
haven't provisioned for.  If that is the case we need to figure out
exactly what is happening before considering a fix.

Thanks,
Mathieu

[1]. https://elixir.bootlin.com/linux/v5.3-rc4/source/drivers/hwtracing/coresight/coresight-tmc-etr.c#L1559
[2]. https://elixir.bootlin.com/linux/v5.3-rc4/source/drivers/hwtracing/coresight/coresight-etm-perf.c#L298
[3]. https://elixir.bootlin.com/linux/v5.3-rc4/source/drivers/hwtracing/coresight/coresight-etm-perf.c#L348

>                 lost = true;
>                 spin_unlock_irqrestore(&drvdata->spinlock, flags);
>                 goto out;
> @@ -1497,7 +1497,7 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
>
>         CS_LOCK(drvdata->base);
>         /* Reset perf specific data */
> -       drvdata->perf_data = NULL;
> +       drvdata->perf_buf = NULL;
>         spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
>         size = etr_buf->len;
> @@ -1556,7 +1556,7 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data)
>         }
>
>         etr_perf->head = PERF_IDX2OFF(handle->head, etr_perf);
> -       drvdata->perf_data = etr_perf;
> +       drvdata->perf_buf = etr_perf->etr_buf;
>
>         /*
>          * No HW configuration is needed if the sink is already in
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index 1ed50411cc3c..f9a0c95e9ba2 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -178,8 +178,8 @@ struct etr_buf {
>   *             device configuration register (DEVID)
>   * @idr:       Holds etr_bufs allocated for this ETR.
>   * @idr_mutex: Access serialisation for idr.
> - * @perf_data: PERF buffer for ETR.
> - * @sysfs_data:        SYSFS buffer for ETR.
> + * @sysfs_buf: SYSFS buffer for ETR.
> + * @perf_buf:  PERF buffer for ETR.
>   */
>  struct tmc_drvdata {
>         void __iomem            *base;
> @@ -202,7 +202,7 @@ struct tmc_drvdata {
>         struct idr              idr;
>         struct mutex            idr_mutex;
>         struct etr_buf          *sysfs_buf;
> -       void                    *perf_data;
> +       struct etr_buf          *perf_buf;
>  };
>
>  struct etr_buf_operations {
> --
> 2.23.0.rc1.153.gdeed80330f-goog
>

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

^ permalink raw reply

* Re: [PATCH 04/22] media: Move v4l2_fwnode_parse_link from v4l2 to driver base
From: Steve Longerbeam @ 2019-08-14 23:00 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
	Laurent Pinchart, Shawn Guo, Fabio Estevam,
	open list:STAGING SUBSYSTEM, Mauro Carvalho Chehab, Michal Simek,
	open list:ACPI, Andy Shevchenko, NXP Linux Team, Len Brown,
	Philipp Zabel, Sascha Hauer, Thomas Gleixner, Andy Shevchenko,
	moderated list:ARM/ZYNQ ARCHITECTURE, Hyun Kwon,
	Greg Kroah-Hartman, open list, Jacopo Mondi,
	Pengutronix Kernel Team, Hans Verkuil, Linux Media Mailing List,
	Enrico Weigelt
In-Reply-To: <20190814220437.GJ13294@shell.armlinux.org.uk>



On 8/14/19 3:04 PM, Russell King - ARM Linux admin wrote:
> On Wed, Aug 14, 2019 at 12:04:41PM -0700, Steve Longerbeam wrote:
>>
>> On 8/14/19 3:30 AM, Russell King - ARM Linux admin wrote:
>>> On Tue, Aug 06, 2019 at 09:53:41AM -0700, Steve Longerbeam wrote:
>>>> The full patchset doesn't seem to be up yet, but see [1] for the cover
>>>> letter.
>>> Was the entire series copied to the mailing lists, or just selected
>>> patches?  I only saw 4, 9, 11 and 13-22 via lakml.
>> The whole series was posted to the linux-media ML, see [1]. At the time,
>> none of the linux-media ML archives had the whole series.
>>
>>> In the absence of the other patches, will this solve imx-media binding
>>> the internal subdevs of sensor devices to the CSI2 interface?
>> "internal subdevs of sensor devices" ?? That doesn't make any sense.
> Sorry, but it makes complete sense when you consider that sensor
> devices may have more than one subdev, but there should be only one
> that is the "output" to whatever the camera is attached to.  The
> other subdevs are internal to the sensor.

Ah, thanks for the clarification. Yes, by "internal subdevs" I 
understand what you mean now. The adv748x and smiapp are examples.

>
> subdevs are not purely the remit of SoC drivers.

So there is no binding of internal subdevs to the receiver CSI-2. The 
receiver CSI-2 subdev will create media links to the subdev that has an 
externally exposed fwnode endpoint that connects with the CSI-2 sink pad.

Steve


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

^ permalink raw reply

* Re: [PATCH 12/16] arm64: prefer __section from compiler_attributes.h
From: Nick Desaulniers @ 2019-08-14 22:20 UTC (permalink / raw)
  To: Will Deacon, Miguel Ojeda
  Cc: Song Liu, Catalin Marinas, Alexei Starovoitov, Daniel Borkmann,
	clang-built-linux, Allison Randal, Yonghong Song,
	Masayoshi Mizuma, Suzuki K Poulose, Andrey Konovalov,
	Shaokun Zhang, Alexios Zavras, Josh Poimboeuf, Sedat Dilek,
	Thomas Gleixner, bpf, Linux ARM, Greg Kroah-Hartman, linux-kernel,
	Network Development, Andrew Morton, Enrico Weigelt,
	Martin KaFai Lau
In-Reply-To: <20190813170829.c3lryb6va3eopxd7@willie-the-truck>

On Tue, Aug 13, 2019 at 10:08 AM Will Deacon <will@kernel.org> wrote:
>
> On Tue, Aug 13, 2019 at 02:36:06PM +0200, Miguel Ojeda wrote:
> > On Tue, Aug 13, 2019 at 10:27 AM Will Deacon <will@kernel.org> wrote:
> > > On Mon, Aug 12, 2019 at 02:50:45PM -0700, Nick Desaulniers wrote:
> > > > GCC unescapes escaped string section names while Clang does not. Because
> > > > __section uses the `#` stringification operator for the section name, it
> > > > doesn't need to be escaped.
> > > >
> > > > This antipattern was found with:
> > > > $ grep -e __section\(\" -e __section__\(\" -r
> > > >
> > > > Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
> > > > Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
> > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > > > ---
> > > >  arch/arm64/include/asm/cache.h     | 2 +-
> > > >  arch/arm64/kernel/smp_spin_table.c | 2 +-
> > > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > Does this fix a build issue, or is it just cosmetic or do we end up with
> > > duplicate sections or something else?
> >
> > This should be cosmetic -- basically we are trying to move all users
> > of current available __attribute__s in compiler_attributes.h to the
> > __attr forms. I am also adding (slowly) new attributes that are
> > already used but we don't have them yet in __attr form.

This lone patch of the series is just cosmetic, but patch 14/16 fixes
a real boot issue:
https://github.com/ClangBuiltLinux/linux/issues/619
Miguel, I'd like to get that one landed ASAP; the rest are just for consistency.

> >
> > > Happy to route it via arm64, just having trouble working out whether it's
> > > 5.3 material!

Thanks!
https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?h=for-next/core&id=80d838122643a09a9f99824adea4b4261e4451e6

> >
> > As you prefer! Those that are not taken by a maintainer I will pick up
> > and send via compiler-attributes.

Miguel, how do you want to take the rest of these patches? Will picked
up the arm64 one, I think the SuperH one got picked up.  There was
feedback to add more info to individual commits' commit messages.

I kept these tree wide changes separate to improve the likelihood that
they'd backport to stable cleanly, but could always squash if you'd
prefer to have 1 patch instead of a series.  Just let me know.
-- 
Thanks,
~Nick Desaulniers

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

^ permalink raw reply


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