Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v4 0/4] fix the clock setting for SAR ADC
From: Jonathan Cameron @ 2017-11-11  0:37 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: Mark Rutland, devicetree, linux-arm-kernel, Neil Armstrong,
	linux-iio, Stephen Boyd, Michael Turquette, Yixun Lan,
	linux-kernel, Rob Herring, Xingyu Chen, Kevin Hilman,
	Carlo Caione, linux-amlogic, linux-clk, Jonathan Cameron,
	Jerome Brunet
In-Reply-To: <CAFBinCB=2pb9dZkpSqdVAiumkSCzgaW0oejftjZNP7aYbiQJuw@mail.gmail.com>

On Tue, 7 Nov 2017 22:36:00 +0100
Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Hi Yixun,
> 
> On Tue, Nov 7, 2017 at 3:09 PM, Yixun Lan <yixun.lan@amlogic.com> wrote:
> > patch [1/4]:
> >   Fix wrong SARADC/SANA clock gate bit in Meson-GXBB/GXL,
> > the published datasheets[4] also has wrong description about this.
> >   This patch should be explicitly merged *before* other patches.
> >
> > patch [2-4/4]:
> >   Drop the "sana" clock from SAR ADC module,  
> I agree with Jerome that patch 2/4 should be applied last.

Let me know when I should take this.

Thanks,

Jonathan

> when I wrote the driver I couldn't get it to work on my GXBB board
> (which unfortunately has died since then) because the clocks were
> disabled (they weren't enabled by the bootloader). people who are
> using an old .dtb would get the same problem again until the clock
> driver is merged
> 
> >   From the hardware perspective, the SAR ADC module doesn't
> > require "sana" clock to wrok. This should apply to all SoC,
> > including meson6,8, GXBB, GXL..  
> thank you for clarifying this!
> 
> > Note: the whole patchset series has been tested at GXL-P212 board,
> > we haven't got any meson6,8 board to test, so I would appreciate
> > if someone (Martin?) could help to confirm it works there.  
> I can test this on a Meson8b and a Meson8m2 board on the weekend -
> I'll let you know about the results
> 
> 
> Regards
> Martin
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v11 1/2] media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver documentation
From: Niklas Söderlund @ 2017-11-11  0:25 UTC (permalink / raw)
  To: Laurent Pinchart, Hans Verkuil, linux-media
  Cc: linux-renesas-soc, tomoharu.fukawa.eb, Kieran Bingham,
	Sakari Ailus, Geert Uytterhoeven, Niklas Söderlund,
	Rob Herring, devicetree
In-Reply-To: <20171111002526.2646-1-niklas.soderlund+renesas@ragnatech.se>

Documentation for Renesas R-Car MIPI CSI-2 receiver. The CSI-2 receivers
are located between the video sources (CSI-2 transmitters) and the video
grabbers (VIN) on Gen3 of Renesas R-Car SoC.

Each CSI-2 device is connected to more then one VIN device which
simultaneously can receive video from the same CSI-2 device. Each VIN
device can also be connected to more then one CSI-2 device. The routing
of which link are used are controlled by the VIN devices. There are only
a few possible routes which are set by hardware limitations, which are
different for each SoC in the Gen3 family.

To work with the limitations of routing possibilities it is necessary
for the DT bindings to describe which VIN device is connected to which
CSI-2 device. This is why port 1 needs to to assign reg numbers for each
VIN device that be connected to it. To setup and to know which links are
valid for each SoC is the responsibility of the VIN driver since the
register to configure it belongs to the VIN hardware.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 .../bindings/media/renesas,rcar-csi2.txt           | 104 +++++++++++++++++++++
 MAINTAINERS                                        |   1 +
 2 files changed, 105 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt

diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
new file mode 100644
index 0000000000000000..24705d8997b14a10
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
@@ -0,0 +1,104 @@
+Renesas R-Car MIPI CSI-2
+------------------------
+
+The rcar-csi2 device provides MIPI CSI-2 capabilities for the Renesas R-Car
+family of devices. It is to be used in conjunction with the R-Car VIN module,
+which provides the video capture capabilities.
+
+Mandatory properties
+--------------------
+ - compatible: Must be one or more of the following
+   - "renesas,r8a7795-csi2" for the R8A7795 device.
+   - "renesas,r8a7796-csi2" for the R8A7796 device.
+
+ - reg: the register base and size for the device registers
+ - interrupts: the interrupt for the device
+ - clocks: Reference to the parent clock
+
+The device node shall contain two 'port' child nodes according to the
+bindings defined in Documentation/devicetree/bindings/media/
+video-interfaces.txt. Port 0 shall connect the node that is the video
+source for to the CSI-2. Port 1 shall connect all the R-Car VIN
+modules, which can make use of the CSI-2 module.
+
+- Port 0 - Video source (Mandatory)
+	- Endpoint 0 - sub-node describing the endpoint that is the video source
+
+- Port 1 - VIN instances (Mandatory for all VIN present in the SoC)
+	- Endpoint 0 - sub-node describing the endpoint that is VIN0
+	- Endpoint 1 - sub-node describing the endpoint that is VIN1
+	- Endpoint 2 - sub-node describing the endpoint that is VIN2
+	- Endpoint 3 - sub-node describing the endpoint that is VIN3
+	- Endpoint 4 - sub-node describing the endpoint that is VIN4
+	- Endpoint 5 - sub-node describing the endpoint that is VIN5
+	- Endpoint 6 - sub-node describing the endpoint that is VIN6
+	- Endpoint 7 - sub-node describing the endpoint that is VIN7
+
+Example:
+
+	csi20: csi2@fea80000 {
+		compatible = "renesas,r8a7796-csi2", "renesas,rcar-gen3-csi2";
+		reg = <0 0xfea80000 0 0x10000>;
+		interrupts = <0 184 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cpg CPG_MOD 714>;
+		power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+		resets = <&cpg 714>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				reg = <0>;
+
+				csi20_in: endpoint@0 {
+					clock-lanes = <0>;
+					data-lanes = <1>;
+					remote-endpoint = <&adv7482_txb>;
+				};
+			};
+
+			port@1 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				reg = <1>;
+
+				csi20vin0: endpoint@0 {
+					reg = <0>;
+					remote-endpoint = <&vin0csi20>;
+				};
+				csi20vin1: endpoint@1 {
+					reg = <1>;
+					remote-endpoint = <&vin1csi20>;
+				};
+				csi20vin2: endpoint@2 {
+					reg = <2>;
+					remote-endpoint = <&vin2csi20>;
+				};
+				csi20vin3: endpoint@3 {
+					reg = <3>;
+					remote-endpoint = <&vin3csi20>;
+				};
+				csi20vin4: endpoint@4 {
+					reg = <4>;
+					remote-endpoint = <&vin4csi20>;
+				};
+				csi20vin5: endpoint@5 {
+					reg = <5>;
+					remote-endpoint = <&vin5csi20>;
+				};
+				csi20vin6: endpoint@6 {
+					reg = <6>;
+					remote-endpoint = <&vin6csi20>;
+				};
+				csi20vin7: endpoint@7 {
+					reg = <7>;
+					remote-endpoint = <&vin7csi20>;
+				};
+			};
+		};
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index adbf69306e9ee3d2..fe999e9de76e3cb3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8565,6 +8565,7 @@ L:	linux-media@vger.kernel.org
 L:	linux-renesas-soc@vger.kernel.org
 T:	git git://linuxtv.org/media_tree.git
 S:	Supported
+F:	Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
 F:	Documentation/devicetree/bindings/media/rcar_vin.txt
 F:	drivers/media/platform/rcar-vin/
 
-- 
2.15.0

^ permalink raw reply related

* Re: [PATCH v10 2/2] media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
From: Niklas Söderlund @ 2017-11-11  0:11 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Laurent Pinchart, Hans Verkuil, linux-media, Rob Herring,
	devicetree, linux-renesas-soc, tomoharu.fukawa.eb, Kieran Bingham,
	Sakari Ailus, Geert Uytterhoeven
In-Reply-To: <20171110223227.pug7d4qi7rdi4b4b@valkosipuli.retiisi.org.uk>

Hej Sakari,

On 2017-11-11 00:32:27 +0200, Sakari Ailus wrote:
> Hej Niklas,
> 
> Tack för uppdaterade lappar! Jag har några kommentar nedan... det ser bra
> ut överallt.

Tack för din feedback!

> 
> On Fri, Nov 10, 2017 at 02:31:37PM +0100, Niklas Söderlund wrote:
> > A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver
> > supports the rcar-vin driver on R-Car Gen3 SoCs where separate CSI-2
> > hardware blocks are connected between the video sources and the video
> > grabbers (VIN).
> > 
> > Driver is based on a prototype by Koji Matsuoka in the Renesas BSP.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> >  drivers/media/platform/rcar-vin/Kconfig     |  12 +
> >  drivers/media/platform/rcar-vin/Makefile    |   1 +
> >  drivers/media/platform/rcar-vin/rcar-csi2.c | 934 ++++++++++++++++++++++++++++
> >  3 files changed, 947 insertions(+)
> >  create mode 100644 drivers/media/platform/rcar-vin/rcar-csi2.c
> > 
> > diff --git a/drivers/media/platform/rcar-vin/Kconfig b/drivers/media/platform/rcar-vin/Kconfig
> > index af4c98b44d2e22cb..6875f30c1ae42631 100644
> > --- a/drivers/media/platform/rcar-vin/Kconfig
> > +++ b/drivers/media/platform/rcar-vin/Kconfig
> > @@ -1,3 +1,15 @@
> > +config VIDEO_RCAR_CSI2
> > +	tristate "R-Car MIPI CSI-2 Receiver"
> > +	depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF
> > +	depends on ARCH_RENESAS || COMPILE_TEST
> > +	select V4L2_FWNODE
> > +	---help---
> > +	  Support for Renesas R-Car MIPI CSI-2 receiver.
> > +	  Supports R-Car Gen3 SoCs.
> > +
> > +	  To compile this driver as a module, choose M here: the
> > +	  module will be called rcar-csi2.
> > +
> >  config VIDEO_RCAR_VIN
> >  	tristate "R-Car Video Input (VIN) Driver"
> >  	depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF && HAS_DMA && MEDIA_CONTROLLER
> > diff --git a/drivers/media/platform/rcar-vin/Makefile b/drivers/media/platform/rcar-vin/Makefile
> > index 48c5632c21dc060b..5ab803d3e7c1aa57 100644
> > --- a/drivers/media/platform/rcar-vin/Makefile
> > +++ b/drivers/media/platform/rcar-vin/Makefile
> > @@ -1,3 +1,4 @@
> >  rcar-vin-objs = rcar-core.o rcar-dma.o rcar-v4l2.o
> >  
> > +obj-$(CONFIG_VIDEO_RCAR_CSI2) += rcar-csi2.o
> >  obj-$(CONFIG_VIDEO_RCAR_VIN) += rcar-vin.o
> > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > new file mode 100644
> > index 0000000000000000..27d09da191a09b39
> > --- /dev/null
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > @@ -0,0 +1,934 @@
> > +/*
> > + * Driver for Renesas R-Car MIPI CSI-2 Receiver
> > + *
> > + * Copyright (C) 2017 Renesas Electronics Corp.
> > + *
> > + * This program is free software; you can redistribute  it and/or modify it
> > + * under  the terms of  the GNU General  Public License as published by the
> > + * Free Software Foundation;  either version 2 of the  License, or (at your
> > + * option) any later version.
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_graph.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/sys_soc.h>
> > +
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/v4l2-fwnode.h>
> > +#include <media/v4l2-mc.h>
> > +#include <media/v4l2-subdev.h>
> > +
> > +/* Register offsets and bits */
> > +
> > +/* Control Timing Select */
> > +#define TREF_REG			0x00
> > +#define TREF_TREF			BIT(0)
> > +
> > +/* Software Reset */
> > +#define SRST_REG			0x04
> > +#define SRST_SRST			BIT(0)
> > +
> > +/* PHY Operation Control */
> > +#define PHYCNT_REG			0x08
> > +#define PHYCNT_SHUTDOWNZ		BIT(17)
> > +#define PHYCNT_RSTZ			BIT(16)
> > +#define PHYCNT_ENABLECLK		BIT(4)
> > +#define PHYCNT_ENABLE_3			BIT(3)
> > +#define PHYCNT_ENABLE_2			BIT(2)
> > +#define PHYCNT_ENABLE_1			BIT(1)
> > +#define PHYCNT_ENABLE_0			BIT(0)
> > +
> > +/* Checksum Control */
> > +#define CHKSUM_REG			0x0c
> > +#define CHKSUM_ECC_EN			BIT(1)
> > +#define CHKSUM_CRC_EN			BIT(0)
> > +
> > +/*
> > + * Channel Data Type Select
> > + * VCDT[0-15]:  Channel 1 VCDT[16-31]:  Channel 2
> > + * VCDT2[0-15]: Channel 3 VCDT2[16-31]: Channel 4
> > + */
> > +#define VCDT_REG			0x10
> > +#define VCDT2_REG			0x14
> > +#define VCDT_VCDTN_EN			BIT(15)
> > +#define VCDT_SEL_VC(n)			(((n) & 0x3) << 8)
> > +#define VCDT_SEL_DTN_ON			BIT(6)
> > +#define VCDT_SEL_DT(n)			(((n) & 0x3f) << 0)
> > +
> > +/* Frame Data Type Select */
> > +#define FRDT_REG			0x18
> > +
> > +/* Field Detection Control */
> > +#define FLD_REG				0x1c
> > +#define FLD_FLD_NUM(n)			(((n) & 0xff) << 16)
> > +#define FLD_FLD_EN4			BIT(3)
> > +#define FLD_FLD_EN3			BIT(2)
> > +#define FLD_FLD_EN2			BIT(1)
> > +#define FLD_FLD_EN			BIT(0)
> > +
> > +/* Automatic Standby Control */
> > +#define ASTBY_REG			0x20
> > +
> > +/* Long Data Type Setting 0 */
> > +#define LNGDT0_REG			0x28
> > +
> > +/* Long Data Type Setting 1 */
> > +#define LNGDT1_REG			0x2c
> > +
> > +/* Interrupt Enable */
> > +#define INTEN_REG			0x30
> > +
> > +/* Interrupt Source Mask */
> > +#define INTCLOSE_REG			0x34
> > +
> > +/* Interrupt Status Monitor */
> > +#define INTSTATE_REG			0x38
> > +#define INTSTATE_INT_ULPS_START		BIT(7)
> > +#define INTSTATE_INT_ULPS_END		BIT(6)
> > +
> > +/* Interrupt Error Status Monitor */
> > +#define INTERRSTATE_REG			0x3c
> > +
> > +/* Short Packet Data */
> > +#define SHPDAT_REG			0x40
> > +
> > +/* Short Packet Count */
> > +#define SHPCNT_REG			0x44
> > +
> > +/* LINK Operation Control */
> > +#define LINKCNT_REG			0x48
> > +#define LINKCNT_MONITOR_EN		BIT(31)
> > +#define LINKCNT_REG_MONI_PACT_EN	BIT(25)
> > +#define LINKCNT_ICLK_NONSTOP		BIT(24)
> > +
> > +/* Lane Swap */
> > +#define LSWAP_REG			0x4c
> > +#define LSWAP_L3SEL(n)			(((n) & 0x3) << 6)
> > +#define LSWAP_L2SEL(n)			(((n) & 0x3) << 4)
> > +#define LSWAP_L1SEL(n)			(((n) & 0x3) << 2)
> > +#define LSWAP_L0SEL(n)			(((n) & 0x3) << 0)
> > +
> > +/* PHY Test Interface Write Register */
> > +#define PHTW_REG			0x50
> > +
> > +/* PHY Test Interface Clear */
> > +#define PHTC_REG			0x58
> > +#define PHTC_TESTCLR			BIT(0)
> > +
> > +/* PHY Frequency Control */
> > +#define PHYPLL_REG			0x68
> > +#define PHYPLL_HSFREQRANGE(n)		((n) << 16)
> > +
> > +struct phypll_hsfreqrange {
> > +	u16 mbps;
> > +	u16 reg;
> > +};
> > +
> > +static const struct phypll_hsfreqrange hsfreqrange_h3_v3h_m3n[] = {
> > +	{ .mbps =   80, .reg = 0x00 },
> > +	{ .mbps =   90, .reg = 0x10 },
> > +	{ .mbps =  100, .reg = 0x20 },
> > +	{ .mbps =  110, .reg = 0x30 },
> > +	{ .mbps =  120, .reg = 0x01 },
> > +	{ .mbps =  130, .reg = 0x11 },
> > +	{ .mbps =  140, .reg = 0x21 },
> > +	{ .mbps =  150, .reg = 0x31 },
> > +	{ .mbps =  160, .reg = 0x02 },
> > +	{ .mbps =  170, .reg = 0x12 },
> > +	{ .mbps =  180, .reg = 0x22 },
> > +	{ .mbps =  190, .reg = 0x32 },
> > +	{ .mbps =  205, .reg = 0x03 },
> > +	{ .mbps =  220, .reg = 0x13 },
> > +	{ .mbps =  235, .reg = 0x23 },
> > +	{ .mbps =  250, .reg = 0x33 },
> > +	{ .mbps =  275, .reg = 0x04 },
> > +	{ .mbps =  300, .reg = 0x14 },
> > +	{ .mbps =  325, .reg = 0x25 },
> > +	{ .mbps =  350, .reg = 0x35 },
> > +	{ .mbps =  400, .reg = 0x05 },
> > +	{ .mbps =  450, .reg = 0x26 },
> > +	{ .mbps =  500, .reg = 0x36 },
> > +	{ .mbps =  550, .reg = 0x37 },
> > +	{ .mbps =  600, .reg = 0x07 },
> > +	{ .mbps =  650, .reg = 0x18 },
> > +	{ .mbps =  700, .reg = 0x28 },
> > +	{ .mbps =  750, .reg = 0x39 },
> > +	{ .mbps =  800, .reg = 0x09 },
> > +	{ .mbps =  850, .reg = 0x19 },
> > +	{ .mbps =  900, .reg = 0x29 },
> > +	{ .mbps =  950, .reg = 0x3a },
> > +	{ .mbps = 1000, .reg = 0x0a },
> > +	{ .mbps = 1050, .reg = 0x1a },
> > +	{ .mbps = 1100, .reg = 0x2a },
> > +	{ .mbps = 1150, .reg = 0x3b },
> > +	{ .mbps = 1200, .reg = 0x0b },
> > +	{ .mbps = 1250, .reg = 0x1b },
> > +	{ .mbps = 1300, .reg = 0x2b },
> > +	{ .mbps = 1350, .reg = 0x3c },
> > +	{ .mbps = 1400, .reg = 0x0c },
> > +	{ .mbps = 1450, .reg = 0x1c },
> > +	{ .mbps = 1500, .reg = 0x2c },
> > +	/* guard */
> > +	{ .mbps =   0,	.reg = 0x00 },
> > +};
> > +
> > +static const struct phypll_hsfreqrange hsfreqrange_m3w_h3es1[] = {
> > +	{ .mbps =   80,	.reg = 0x00 },
> > +	{ .mbps =   90,	.reg = 0x10 },
> > +	{ .mbps =  100,	.reg = 0x20 },
> > +	{ .mbps =  110,	.reg = 0x30 },
> > +	{ .mbps =  120,	.reg = 0x01 },
> > +	{ .mbps =  130,	.reg = 0x11 },
> > +	{ .mbps =  140,	.reg = 0x21 },
> > +	{ .mbps =  150,	.reg = 0x31 },
> > +	{ .mbps =  160,	.reg = 0x02 },
> > +	{ .mbps =  170,	.reg = 0x12 },
> > +	{ .mbps =  180,	.reg = 0x22 },
> > +	{ .mbps =  190,	.reg = 0x32 },
> > +	{ .mbps =  205,	.reg = 0x03 },
> > +	{ .mbps =  220,	.reg = 0x13 },
> > +	{ .mbps =  235,	.reg = 0x23 },
> > +	{ .mbps =  250,	.reg = 0x33 },
> > +	{ .mbps =  275,	.reg = 0x04 },
> > +	{ .mbps =  300,	.reg = 0x14 },
> > +	{ .mbps =  325,	.reg = 0x05 },
> > +	{ .mbps =  350,	.reg = 0x15 },
> > +	{ .mbps =  400,	.reg = 0x25 },
> > +	{ .mbps =  450,	.reg = 0x06 },
> > +	{ .mbps =  500,	.reg = 0x16 },
> > +	{ .mbps =  550,	.reg = 0x07 },
> > +	{ .mbps =  600,	.reg = 0x17 },
> > +	{ .mbps =  650,	.reg = 0x08 },
> > +	{ .mbps =  700,	.reg = 0x18 },
> > +	{ .mbps =  750,	.reg = 0x09 },
> > +	{ .mbps =  800,	.reg = 0x19 },
> > +	{ .mbps =  850,	.reg = 0x29 },
> > +	{ .mbps =  900,	.reg = 0x39 },
> > +	{ .mbps =  950,	.reg = 0x0A },
> > +	{ .mbps = 1000,	.reg = 0x1A },
> > +	{ .mbps = 1050,	.reg = 0x2A },
> > +	{ .mbps = 1100,	.reg = 0x3A },
> > +	{ .mbps = 1150,	.reg = 0x0B },
> > +	{ .mbps = 1200,	.reg = 0x1B },
> > +	{ .mbps = 1250,	.reg = 0x2B },
> > +	{ .mbps = 1300,	.reg = 0x3B },
> > +	{ .mbps = 1350,	.reg = 0x0C },
> > +	{ .mbps = 1400,	.reg = 0x1C },
> > +	{ .mbps = 1450,	.reg = 0x2C },
> > +	{ .mbps = 1500,	.reg = 0x3C },
> > +	/* guard */
> > +	{ .mbps =   0,	.reg = 0x00 },
> > +};
> > +
> > +/* PHY ESC Error Monitor */
> > +#define PHEERM_REG			0x74
> > +
> > +/* PHY Clock Lane Monitor */
> > +#define PHCLM_REG			0x78
> > +
> > +/* PHY Data Lane Monitor */
> > +#define PHDLM_REG			0x7c
> > +
> > +/* CSI0CLK Frequency Configuration Preset Register */
> > +#define CSI0CLKFCPR_REG			0x260
> > +#define CSI0CLKFREQRANGE(n)		((n & 0x3f) << 16)
> > +
> > +struct rcar_csi2_format {
> > +	unsigned int code;
> > +	unsigned int datatype;
> > +	unsigned int bpp;
> > +};
> > +
> > +static const struct rcar_csi2_format rcar_csi2_formats[] = {
> > +	{ .code = MEDIA_BUS_FMT_RGB888_1X24,	.datatype = 0x24, .bpp = 24 },
> > +	{ .code = MEDIA_BUS_FMT_UYVY8_1X16,	.datatype = 0x1e, .bpp = 16 },
> > +	{ .code = MEDIA_BUS_FMT_UYVY8_2X8,	.datatype = 0x1e, .bpp = 16 },
> > +	{ .code = MEDIA_BUS_FMT_YUYV10_2X10,	.datatype = 0x1e, .bpp = 16 },
> > +};
> > +
> > +static const struct rcar_csi2_format *rcar_csi2_code_to_fmt(unsigned int code)
> > +{
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(rcar_csi2_formats); i++)
> > +		if (rcar_csi2_formats[i].code == code)
> > +			return rcar_csi2_formats + i;
> 
> Extra newline would be nice.

Yes :-)

> 
> > +	return NULL;
> > +}
> > +
> > +enum rcar_csi2_pads {
> > +	RCAR_CSI2_SINK,
> > +	RCAR_CSI2_SOURCE_VC0,
> > +	RCAR_CSI2_SOURCE_VC1,
> > +	RCAR_CSI2_SOURCE_VC2,
> > +	RCAR_CSI2_SOURCE_VC3,
> > +	NR_OF_RCAR_CSI2_PAD,
> > +};
> > +
> > +struct rcar_csi2_info {
> > +	const struct phypll_hsfreqrange *hsfreqrange;
> > +	unsigned int csi0clkfreqrange;
> > +	bool clear_ulps;
> > +	bool have_phtw;
> > +};
> > +
> > +struct rcar_csi2 {
> > +	struct device *dev;
> > +	void __iomem *base;
> > +	const struct rcar_csi2_info *info;
> > +
> > +	struct v4l2_subdev subdev;
> > +	struct media_pad pads[NR_OF_RCAR_CSI2_PAD];
> > +
> > +	struct v4l2_async_notifier notifier;
> > +	struct v4l2_async_subdev remote;
> > +
> > +	struct v4l2_mbus_framefmt mf;
> > +
> > +	struct mutex lock;
> > +	int stream_count;
> > +
> > +	unsigned short lanes;
> > +	unsigned char lane_swap[4];
> > +};
> > +
> > +static inline struct rcar_csi2 *sd_to_csi2(struct v4l2_subdev *sd)
> > +{
> > +	return container_of(sd, struct rcar_csi2, subdev);
> > +}
> > +
> > +static inline struct rcar_csi2 *notifier_to_csi2(struct v4l2_async_notifier *n)
> > +{
> > +	return container_of(n, struct rcar_csi2, notifier);
> > +}
> > +
> > +static u32 rcar_csi2_read(struct rcar_csi2 *priv, unsigned int reg)
> > +{
> > +	return ioread32(priv->base + reg);
> > +}
> > +
> > +static void rcar_csi2_write(struct rcar_csi2 *priv, unsigned int reg, u32 data)
> > +{
> > +	iowrite32(data, priv->base + reg);
> > +}
> > +
> > +static void rcar_csi2_reset(struct rcar_csi2 *priv)
> > +{
> > +	rcar_csi2_write(priv, SRST_REG, SRST_SRST);
> > +	usleep_range(100, 150);
> > +	rcar_csi2_write(priv, SRST_REG, 0);
> > +}
> > +
> > +static int rcar_csi2_wait_phy_start(struct rcar_csi2 *priv)
> > +{
> > +	int timeout;
> > +
> > +	/* Wait for the clock and data lanes to enter LP-11 state. */
> > +	for (timeout = 100; timeout > 0; timeout--) {
> > +		const u32 lane_mask = (1 << priv->lanes) - 1;
> > +
> > +		if ((rcar_csi2_read(priv, PHCLM_REG) & 1) == 1 &&
> > +		    (rcar_csi2_read(priv, PHDLM_REG) & lane_mask) == lane_mask)
> > +			return 0;
> > +
> > +		msleep(20);
> > +	}
> > +
> > +	dev_err(priv->dev, "Timeout waiting for LP-11 state\n");
> > +
> > +	return -ETIMEDOUT;
> > +}
> > +
> > +static int rcar_csi2_calc_phypll(struct rcar_csi2 *priv,
> > +				 struct v4l2_subdev *source,
> > +				 struct v4l2_mbus_framefmt *mf,
> > +				 u32 *phypll)
> > +{
> > +	const struct phypll_hsfreqrange *hsfreq;
> > +	const struct rcar_csi2_format *format;
> > +	struct v4l2_ctrl *ctrl;
> > +	u64 mbps;
> > +
> > +	ctrl = v4l2_ctrl_find(source->ctrl_handler, V4L2_CID_PIXEL_RATE);
> 
> How about LINK_FREQ instead? It'd be more straightforward to calculate
> this. Up to you.

I need to use PIXEL_RATE as my test setup uses the adv748x driver which 
only implement that control. In the short term I would like to support 
both, but I need a setup to test that before I can add support for it.  
In the long term, maybe we should deprecate one of the controls?

> 
> > +	if (!ctrl) {
> > +		dev_err(priv->dev, "no link freq control in subdev %s\n",
> > +			source->name);
> > +		return -EINVAL;
> > +	}
> > +
> > +	format = rcar_csi2_code_to_fmt(mf->code);
> > +	if (!format) {
> > +		dev_err(priv->dev, "Unknown format: %d\n", mf->code);
> > +		return -EINVAL;
> > +	}
> > +
> > +	mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * format->bpp;
> > +	do_div(mbps, priv->lanes * 1000000);
> > +
> > +	for (hsfreq = priv->info->hsfreqrange; hsfreq->mbps != 0; hsfreq++)
> > +		if (hsfreq->mbps >= mbps)
> > +			break;
> > +
> > +	if (!hsfreq->mbps) {
> > +		dev_err(priv->dev, "Unsupported PHY speed (%llu Mbps)", mbps);
> > +		return -ERANGE;
> > +	}
> > +
> > +	dev_dbg(priv->dev, "PHY HSFREQRANGE requested %llu got %u Mbps\n", mbps,
> > +		hsfreq->mbps);
> > +
> > +	*phypll = PHYPLL_HSFREQRANGE(hsfreq->reg);
> > +
> > +	return 0;
> > +}
> > +
> > +static int rcar_csi2_start(struct rcar_csi2 *priv)
> > +{
> > +	const struct rcar_csi2_format *format;
> > +	struct v4l2_subdev_format fmt;
> > +	struct media_pad *pad, *source_pad;
> > +	struct v4l2_subdev *source = NULL;
> > +	struct v4l2_mbus_framefmt *mf = &fmt.format;
> > +	u32 phycnt, phypll, tmp;
> > +	u32 vcdt = 0, vcdt2 = 0;
> > +	unsigned int i;
> > +	int ret;
> > +
> > +	pad = &priv->subdev.entity.pads[RCAR_CSI2_SINK];
> > +	source_pad = media_entity_remote_pad(pad);
> > +	if (!source_pad) {
> > +		dev_err(priv->dev, "Could not find remote source pad\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	source = media_entity_to_v4l2_subdev(source_pad->entity);
> > +	if (!source) {
> > +		dev_err(priv->dev, "Could not find remote subdevice\n");
> > +		return -ENODEV;
> > +	}
> 
> Something must be really, really wrong if the above would happen. A pad
> without an entity?

Good catch, I will remove the unneeded check.

> 
> > +
> > +	dev_dbg(priv->dev, "Using source %s pad: %u\n", source->name,
> > +		source_pad->index);
> > +
> > +	fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> > +	fmt.pad = source_pad->index;
> > +	ret = v4l2_subdev_call(source, pad, get_fmt, NULL, &fmt);
> > +	if (ret)
> > +		return ret;
> 
> The link format has been validated already by this point this isn't
> supposed to fail or produce different results than in link validation.
> 
> You could get the same format from the local pad, too.

Another good catch, I will use the format stored locally to make the 
code simpler. As you state the formats are already validated so it's 
safe to do so. I still need to get the remote subdevice to be able to 
read the PIXEL_RATE control, but I can move that to 
rcar_csi2_calc_phypll() where it's actually used.

> 
> > +
> > +	dev_dbg(priv->dev, "Input size (%ux%u%c)\n", mf->width,
> > +		mf->height, mf->field == V4L2_FIELD_NONE ? 'p' : 'i');
> > +
> > +	/*
> > +	 * Enable all Virtual Channels
> > +	 *
> > +	 * NOTE: It's not possible to get individual format for each
> > +	 *       source virtual channel. Once this is possible in V4L2
> > +	 *       it should be used here.
> > +	 */
> > +	for (i = 0; i < 4; i++) {
> > +		format = rcar_csi2_code_to_fmt(mf->code);
> > +		if (!format) {
> > +			dev_err(priv->dev, "Unsupported media bus format: %d\n",
> > +				mf->code);
> > +			return -EINVAL;
> > +		}
> > +
> > +		tmp = VCDT_SEL_VC(i) | VCDT_VCDTN_EN | VCDT_SEL_DTN_ON |
> > +			VCDT_SEL_DT(format->datatype);
> > +
> > +		/* Store in correct reg and offset */
> > +		if (i < 2)
> > +			vcdt |= tmp << ((i % 2) * 16);
> > +		else
> > +			vcdt2 |= tmp << ((i % 2) * 16);
> > +	}
> > +
> > +	switch (priv->lanes) {
> > +	case 1:
> > +		phycnt = PHYCNT_ENABLECLK | PHYCNT_ENABLE_0;
> > +		break;
> > +	case 2:
> > +		phycnt = PHYCNT_ENABLECLK | PHYCNT_ENABLE_1 | PHYCNT_ENABLE_0;
> > +		break;
> > +	case 4:
> > +		phycnt = PHYCNT_ENABLECLK | PHYCNT_ENABLE_3 | PHYCNT_ENABLE_2 |
> > +			PHYCNT_ENABLE_1 | PHYCNT_ENABLE_0;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = rcar_csi2_calc_phypll(priv, source, mf, &phypll);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Clear Ultra Low Power interrupt */
> > +	if (priv->info->clear_ulps)
> > +		rcar_csi2_write(priv, INTSTATE_REG,
> > +				INTSTATE_INT_ULPS_START |
> > +				INTSTATE_INT_ULPS_END);
> > +
> > +	/* Init */
> > +	rcar_csi2_write(priv, TREF_REG, TREF_TREF);
> > +	rcar_csi2_reset(priv);
> > +	rcar_csi2_write(priv, PHTC_REG, 0);
> > +
> > +	/* Configure */
> > +	rcar_csi2_write(priv, FLD_REG, FLD_FLD_NUM(2) | FLD_FLD_EN4 |
> > +			FLD_FLD_EN3 | FLD_FLD_EN2 | FLD_FLD_EN);
> > +	rcar_csi2_write(priv, VCDT_REG, vcdt);
> > +	rcar_csi2_write(priv, VCDT2_REG, vcdt2);
> > +	/* Lanes are zero indexed */
> > +	rcar_csi2_write(priv, LSWAP_REG,
> > +			LSWAP_L0SEL(priv->lane_swap[0] - 1) |
> > +			LSWAP_L1SEL(priv->lane_swap[1] - 1) |
> > +			LSWAP_L2SEL(priv->lane_swap[2] - 1) |
> > +			LSWAP_L3SEL(priv->lane_swap[3] - 1));
> > +
> > +	if (priv->info->have_phtw) {
> > +		/*
> > +		 * This is for H3 ES2.0
> > +		 *
> > +		 * NOTE: Additional logic is needed here when
> > +		 * support for V3H and/or M3-N is added
> > +		 */
> > +		rcar_csi2_write(priv, PHTW_REG, 0x01cc01e2);
> > +		rcar_csi2_write(priv, PHTW_REG, 0x010101e3);
> > +		rcar_csi2_write(priv, PHTW_REG, 0x010101e4);
> > +		rcar_csi2_write(priv, PHTW_REG, 0x01100104);
> > +		rcar_csi2_write(priv, PHTW_REG, 0x01030100);
> > +		rcar_csi2_write(priv, PHTW_REG, 0x01800100);
> > +	}
> > +
> > +	/* Start */
> > +	rcar_csi2_write(priv, PHYPLL_REG, phypll);
> > +
> > +	/* Set frequency range if we have it */
> > +	if (priv->info->csi0clkfreqrange)
> > +		rcar_csi2_write(priv, CSI0CLKFCPR_REG,
> > +				CSI0CLKFREQRANGE(priv->info->csi0clkfreqrange));
> > +
> > +	rcar_csi2_write(priv, PHYCNT_REG, phycnt);
> > +	rcar_csi2_write(priv, LINKCNT_REG, LINKCNT_MONITOR_EN |
> > +			LINKCNT_REG_MONI_PACT_EN | LINKCNT_ICLK_NONSTOP);
> > +	rcar_csi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ);
> > +	rcar_csi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ |
> > +			PHYCNT_RSTZ);
> > +
> > +	return rcar_csi2_wait_phy_start(priv);
> > +}
> > +
> > +static void rcar_csi2_stop(struct rcar_csi2 *priv)
> > +{
> > +	rcar_csi2_write(priv, PHYCNT_REG, 0);
> > +
> > +	rcar_csi2_reset(priv);
> > +}
> > +
> > +static int rcar_csi2_sd_info(struct rcar_csi2 *priv, struct v4l2_subdev **sd)
> > +{
> > +	struct media_pad *pad;
> > +
> > +	pad = media_entity_remote_pad(&priv->pads[RCAR_CSI2_SINK]);
> > +	if (!pad) {
> > +		dev_err(priv->dev, "Could not find remote pad\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	*sd = media_entity_to_v4l2_subdev(pad->entity);
> > +	if (!*sd) {
> > +		dev_err(priv->dev, "Could not find remote subdevice\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int rcar_csi2_s_stream(struct v4l2_subdev *sd, int enable)
> > +{
> > +	struct rcar_csi2 *priv = sd_to_csi2(sd);
> > +	struct v4l2_subdev *nextsd;
> > +	int ret;
> > +
> > +	mutex_lock(&priv->lock);
> > +
> > +	ret = rcar_csi2_sd_info(priv, &nextsd);
> > +	if (ret)
> > +		goto out;
> > +
> > +	if (enable && priv->stream_count == 0) {
> > +		ret =  rcar_csi2_start(priv);
> > +		if (ret)
> > +			goto out;
> > +
> > +		ret = v4l2_subdev_call(nextsd, video, s_stream, 1);
> > +		if (ret) {
> > +			rcar_csi2_stop(priv);
> > +			goto out;
> > +		}
> > +	} else if (!enable && priv->stream_count == 1) {
> > +		rcar_csi2_stop(priv);
> > +		ret = v4l2_subdev_call(nextsd, video, s_stream, 0);
> > +	}
> > +
> > +	priv->stream_count += enable ? 1 : -1;
> > +out:
> > +	mutex_unlock(&priv->lock);
> > +
> > +	return ret;
> > +}
> > +
> > +static int rcar_csi2_s_power(struct v4l2_subdev *sd, int on)
> > +{
> > +	struct rcar_csi2 *priv = sd_to_csi2(sd);
> > +
> > +	if (on)
> > +		pm_runtime_get_sync(priv->dev);
> > +	else
> > +		pm_runtime_put(priv->dev);
> 
> The pipeline you have is already rather complex. Would it be an option to
> power the hardware on when streaming is started? The smiapp driver does
> this, without even implementing the s_power callback. (You'd still need to
> call it on the image source, as long as we have drivers that need it.)

I tested this early in early development for this driver and back then I 
could not. But now 10 versions later it is possible so will do so for 
version 11 :-) Really good you pointed this out, makes the code much 
better thanks!

> 
> > +
> > +	return 0;
> > +}
> > +
> > +static int rcar_csi2_set_pad_format(struct v4l2_subdev *sd,
> > +				    struct v4l2_subdev_pad_config *cfg,
> > +				    struct v4l2_subdev_format *format)
> > +{
> > +	struct rcar_csi2 *priv = sd_to_csi2(sd);
> > +	struct v4l2_mbus_framefmt *framefmt;
> > +
> 
> Is everything possible? For instance, are there limits regarding width or
> height? Or the pixel code? They should be checked here.

Yes it do care about pixel code which was only validated at s_stream() 
time, will move the check here thanks!

> 
> Also the format on the source pad is, presumably, dependent on the format
> on the sink pad.

Yes, but since this driver can't change the format in any way is there 
any harm in mirror whatever is set on one pad on the other(s)? This will 
change once multiple stream support is added in a later series.

> 
> > +	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> > +		priv->mf = format->format;
> > +	} else {
> > +		framefmt = v4l2_subdev_get_try_format(sd, cfg, 0);
> > +		*framefmt = format->format;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int rcar_csi2_get_pad_format(struct v4l2_subdev *sd,
> > +				    struct v4l2_subdev_pad_config *cfg,
> > +				    struct v4l2_subdev_format *format)
> > +{
> > +	struct rcar_csi2 *priv = sd_to_csi2(sd);
> > +
> > +	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> > +		format->format = priv->mf;
> > +	else
> > +		format->format = *v4l2_subdev_get_try_format(sd, cfg, 0);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct v4l2_subdev_video_ops rcar_csi2_video_ops = {
> > +	.s_stream = rcar_csi2_s_stream,
> > +};
> > +
> > +static const struct v4l2_subdev_core_ops rcar_csi2_subdev_core_ops = {
> > +	.s_power = rcar_csi2_s_power,
> > +};
> > +
> > +static const struct v4l2_subdev_pad_ops rcar_csi2_pad_ops = {
> > +	.set_fmt = rcar_csi2_set_pad_format,
> > +	.get_fmt = rcar_csi2_get_pad_format,
> > +};
> > +
> > +static const struct v4l2_subdev_ops rcar_csi2_subdev_ops = {
> > +	.video	= &rcar_csi2_video_ops,
> > +	.core	= &rcar_csi2_subdev_core_ops,
> > +	.pad	= &rcar_csi2_pad_ops,
> > +};
> > +
> > +/* -----------------------------------------------------------------------------
> > + * Async and registered of subdevices and links
> > + */
> > +
> > +static int rcar_csi2_notify_bound(struct v4l2_async_notifier *notifier,
> > +				   struct v4l2_subdev *subdev,
> > +				   struct v4l2_async_subdev *asd)
> > +{
> > +	struct rcar_csi2 *priv = notifier_to_csi2(notifier);
> > +	int pad;
> > +
> > +	v4l2_set_subdev_hostdata(subdev, priv);
> > +
> > +	pad = media_entity_get_fwnode_pad(&subdev->entity,
> > +					  asd->match.fwnode.fwnode,
> > +					  MEDIA_PAD_FL_SOURCE);
> > +	if (pad < 0) {
> > +		dev_err(priv->dev, "Failed to find pad for %s\n", subdev->name);
> > +		return pad;
> > +	}
> > +
> > +	dev_dbg(priv->dev, "Bound %s pad: %d\n", subdev->name, pad);
> > +
> > +	return media_create_pad_link(&subdev->entity, pad,
> > +				     &priv->subdev.entity, 0,
> > +				     MEDIA_LNK_FL_ENABLED |
> > +				     MEDIA_LNK_FL_IMMUTABLE);
> > +}
> > +
> > +static void rcar_csi2_notify_unbind(struct v4l2_async_notifier *notifier,
> > +				     struct v4l2_subdev *subdev,
> > +				     struct v4l2_async_subdev *asd)
> > +{
> > +	struct rcar_csi2 *priv = notifier_to_csi2(notifier);
> > +
> > +	dev_dbg(priv->dev, "Unbind %s\n", subdev->name);
> 
> This doesn't really do anything. I'd probably just remove this.

I liked the debug message when running bind/unbind tests, but you are 
right it adds no real value and I can just as easily move this to a 
local test patch. Will remove for next version.

> 
> > +}
> > +
> > +static const struct v4l2_async_notifier_operations rcar_csi2_notify_ops = {
> > +	.bound = rcar_csi2_notify_bound,
> > +	.unbind = rcar_csi2_notify_unbind,
> > +};
> > +
> > +static int rcar_csi2_parse_v4l2(struct rcar_csi2 *priv,
> > +				struct v4l2_fwnode_endpoint *vep)
> > +{
> > +	unsigned int i;
> > +
> > +	/* Only port 0 enpoint 0 is valid */
> > +	if (vep->base.port || vep->base.id)
> > +		return -ENOTCONN;
> > +
> > +	if (vep->bus_type != V4L2_MBUS_CSI2) {
> > +		dev_err(priv->dev, "Unsupported bus: 0x%x\n", vep->bus_type);
> > +		return -EINVAL;
> > +	}
> > +
> > +	priv->lanes = vep->bus.mipi_csi2.num_data_lanes;
> > +	if (priv->lanes != 1 && priv->lanes != 2 && priv->lanes != 4) {
> > +		dev_err(priv->dev, "Unsupported number of data-lanes: %d\n",
> > +			priv->lanes);
> > +		return -EINVAL;
> > +	}
> > +
> > +	for (i = 0; i < ARRAY_SIZE(priv->lane_swap); i++) {
> > +		priv->lane_swap[i] = i < priv->lanes ?
> > +			vep->bus.mipi_csi2.data_lanes[i] : i;
> > +
> > +		/* Check for valid lane number */
> > +		if (priv->lane_swap[i] < 1 || priv->lane_swap[i] > 4) {
> > +			dev_err(priv->dev, "data-lanes must be in 1-4 range\n");
> > +			return -EINVAL;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int rcar_csi2_parse_dt(struct rcar_csi2 *priv)
> > +{
> > +	struct device_node *ep;
> > +	struct v4l2_fwnode_endpoint v4l2_ep;
> > +	int ret;
> > +
> > +	ep = of_graph_get_endpoint_by_regs(priv->dev->of_node, 0, 0);
> > +	if (!ep) {
> > +		dev_err(priv->dev, "Not connected to subdevice\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &v4l2_ep);
> > +	if (ret) {
> > +		dev_err(priv->dev, "Could not parse v4l2 endpoint\n");
> > +		of_node_put(ep);
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = rcar_csi2_parse_v4l2(priv, &v4l2_ep);
> > +	if (ret)
> > +		return ret;
> > +
> > +	priv->remote.match.fwnode.fwnode =
> > +		fwnode_graph_get_remote_endpoint(of_fwnode_handle(ep));
> > +	priv->remote.match_type = V4L2_ASYNC_MATCH_FWNODE;
> > +
> > +	of_node_put(ep);
> > +
> > +	priv->notifier.subdevs = devm_kzalloc(priv->dev,
> > +					      sizeof(*priv->notifier.subdevs),
> > +					      GFP_KERNEL);
> > +	if (priv->notifier.subdevs == NULL)
> > +		return -ENOMEM;
> > +
> > +	priv->notifier.num_subdevs = 1;
> > +	priv->notifier.subdevs[0] = &priv->remote;
> > +	priv->notifier.ops = &rcar_csi2_notify_ops;
> > +
> > +	dev_dbg(priv->dev, "Found '%pOF'\n",
> > +		to_of_node(priv->remote.match.fwnode.fwnode));
> > +
> > +	return v4l2_async_subdev_notifier_register(&priv->subdev,
> > +						   &priv->notifier);
> 
> Could you use v4l2_async_notifier_parse_fwnode_endpoints_by_port() or does
> something bad happen if you do? The intent with that was to avoid functions
> like the above in drivers.

Unfortunately not :-(

I have a local patch which do this, but it won't work since this driver 
needs to register remote endpoints (fwnode_graph_get_remote_endpoint() 
above) and not the remote parent in the async framework. The reason for 
this is that that is what the adv748x driver registers with the async 
framework given that it only have one node in DT but registers multiple 
subdevices. I would love to in the future resolve this in the helper 
functions and switch to such a helper here.

> 
> > +}
> > +
> > +/* -----------------------------------------------------------------------------
> > + * Platform Device Driver
> > + */
> > +
> > +static const struct media_entity_operations rcar_csi2_entity_ops = {
> > +	.link_validate = v4l2_subdev_link_validate,
> > +};
> > +
> > +static int rcar_csi2_probe_resources(struct rcar_csi2 *priv,
> > +				     struct platform_device *pdev)
> > +{
> > +	struct resource *res;
> > +	int irq;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	priv->base = devm_ioremap_resource(&pdev->dev, res);
> > +	if (IS_ERR(priv->base))
> > +		return PTR_ERR(priv->base);
> > +
> > +	irq = platform_get_irq(pdev, 0);
> > +	if (irq < 0)
> > +		return irq;
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct rcar_csi2_info rcar_csi2_info_r8a7795 = {
> > +	.hsfreqrange = hsfreqrange_h3_v3h_m3n,
> > +	.clear_ulps = true,
> > +	.have_phtw = true,
> > +	.csi0clkfreqrange = 0x20,
> > +};
> > +
> > +static const struct rcar_csi2_info rcar_csi2_info_r8a7795es1 = {
> > +	.hsfreqrange = hsfreqrange_m3w_h3es1,
> > +};
> > +
> > +static const struct rcar_csi2_info rcar_csi2_info_r8a7796 = {
> > +	.hsfreqrange = hsfreqrange_m3w_h3es1,
> > +};
> > +
> > +static const struct of_device_id rcar_csi2_of_table[] = {
> > +	{
> > +		.compatible = "renesas,r8a7795-csi2",
> > +		.data = &rcar_csi2_info_r8a7795,
> > +	},
> > +	{
> > +		.compatible = "renesas,r8a7796-csi2",
> > +		.data = &rcar_csi2_info_r8a7796,
> > +	},
> > +	{ /* sentinel */ },
> > +};
> > +MODULE_DEVICE_TABLE(of, rcar_csi2_of_table);
> > +
> > +static const struct soc_device_attribute r8a7795es1[] = {
> > +	{
> > +		.soc_id = "r8a7795", .revision = "ES1.*",
> > +		.data = &rcar_csi2_info_r8a7795es1,
> > +	},
> > +	{ /* sentinel */}
> > +};
> > +
> > +static int rcar_csi2_probe(struct platform_device *pdev)
> > +{
> > +	const struct soc_device_attribute *attr;
> > +	struct rcar_csi2 *priv;
> > +	unsigned int i;
> > +	int ret;
> > +
> > +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	priv->info = of_device_get_match_data(&pdev->dev);
> > +
> > +	/* r8a7795 ES1.x behaves different then ES2.0+ but no own compat */
> > +	attr = soc_device_match(r8a7795es1);
> > +	if (attr)
> > +		priv->info = attr->data;
> > +
> > +	priv->dev = &pdev->dev;
> > +
> > +	mutex_init(&priv->lock);
> > +	priv->stream_count = 0;
> > +
> > +	ret = rcar_csi2_probe_resources(priv, pdev);
> > +	if (ret) {
> > +		dev_err(priv->dev, "Failed to get resources\n");
> > +		return ret;
> > +	}
> > +
> > +	platform_set_drvdata(pdev, priv);
> > +
> > +	ret = rcar_csi2_parse_dt(priv);
> > +	if (ret)
> > +		return ret;
> > +
> > +	priv->subdev.owner = THIS_MODULE;
> > +	priv->subdev.dev = &pdev->dev;
> > +	v4l2_subdev_init(&priv->subdev, &rcar_csi2_subdev_ops);
> > +	v4l2_set_subdevdata(&priv->subdev, &pdev->dev);
> > +	snprintf(priv->subdev.name, V4L2_SUBDEV_NAME_SIZE, "%s %s",
> > +		 KBUILD_MODNAME, dev_name(&pdev->dev));
> > +	priv->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> > +
> > +	priv->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
> > +	priv->subdev.entity.ops = &rcar_csi2_entity_ops;
> > +
> > +	priv->pads[RCAR_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
> > +	for (i = RCAR_CSI2_SOURCE_VC0; i < NR_OF_RCAR_CSI2_PAD; i++)
> > +		priv->pads[i].flags = MEDIA_PAD_FL_SOURCE;
> > +
> > +	ret = media_entity_pads_init(&priv->subdev.entity, NR_OF_RCAR_CSI2_PAD,
> > +				     priv->pads);
> > +	if (ret)
> > +		goto error;
> > +
> > +	ret = v4l2_async_register_subdev(&priv->subdev);
> > +	if (ret < 0)
> > +		goto error;
> > +
> > +	pm_runtime_enable(&pdev->dev);
> > +
> > +	dev_info(priv->dev, "%d lanes found\n", priv->lanes);
> > +
> > +	return 0;
> > +
> > +error:
> > +	v4l2_async_notifier_cleanup(&priv->notifier);
> > +
> > +	return ret;
> > +}
> > +
> > +static int rcar_csi2_remove(struct platform_device *pdev)
> > +{
> > +	struct rcar_csi2 *priv = platform_get_drvdata(pdev);
> > +
> > +	v4l2_async_notifier_cleanup(&priv->notifier);
> > +	v4l2_async_unregister_subdev(&priv->subdev);
> > +
> > +	pm_runtime_disable(&pdev->dev);
> > +
> > +	return 0;
> > +}
> > +
> > +static struct platform_driver __refdata rcar_csi2_pdrv = {
> > +	.remove	= rcar_csi2_remove,
> > +	.probe	= rcar_csi2_probe,
> > +	.driver	= {
> > +		.name	= "rcar-csi2",
> > +		.of_match_table	= of_match_ptr(rcar_csi2_of_table),
> > +	},
> > +};
> > +
> > +module_platform_driver(rcar_csi2_pdrv);
> > +
> > +MODULE_AUTHOR("Niklas Söderlund <niklas.soderlund@ragnatech.se>");
> > +MODULE_DESCRIPTION("Renesas R-Car MIPI CSI-2 receiver");
> > +MODULE_LICENSE("GPL v2");
> 
> -- 
> Kind regards,
> 
> Sakari Ailus
> e-mail: sakari.ailus@iki.fi

-- 
Regards,
Niklas Söderlund

^ permalink raw reply

* Hello Friend
From: Wang Jianlin @ 2017-11-10 22:54 UTC (permalink / raw)


I intend to give you a portion of my wealth as a free-will financial
donation to you, Respond to partake.
Wang Jianlin
Wanda Group
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v10 1/2] media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver documentation
From: Niklas Söderlund @ 2017-11-10 22:41 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Laurent Pinchart, Hans Verkuil, linux-media, Rob Herring,
	devicetree, linux-renesas-soc, tomoharu.fukawa.eb, Kieran Bingham,
	Sakari Ailus, Geert Uytterhoeven
In-Reply-To: <20171110221113.asnpanymn3toytqf@valkosipuli.retiisi.org.uk>

Hej Sakari,

Tack för din feedback.

On 2017-11-11 00:11:13 +0200, Sakari Ailus wrote:
> Hejssan,
> 
> On Fri, Nov 10, 2017 at 02:31:36PM +0100, Niklas Söderlund wrote:
> > Documentation for Renesas R-Car MIPI CSI-2 receiver. The CSI-2 receivers
> > are located between the video sources (CSI-2 transmitters) and the video
> > grabbers (VIN) on Gen3 of Renesas R-Car SoC.
> > 
> > Each CSI-2 device is connected to more then one VIN device which
> > simultaneously can receive video from the same CSI-2 device. Each VIN
> > device can also be connected to more then one CSI-2 device. The routing
> > of which link are used are controlled by the VIN devices. There are only
> > a few possible routes which are set by hardware limitations, which are
> > different for each SoC in the Gen3 family.
> > 
> > To work with the limitations of routing possibilities it is necessary
> > for the DT bindings to describe which VIN device is connected to which
> > CSI-2 device. This is why port 1 needs to to assign reg numbers for each
> > VIN device that be connected to it. To setup and to know which links are
> > valid for each SoC is the responsibility of the VIN driver since the
> > register to configure it belongs to the VIN hardware.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> >  .../devicetree/bindings/media/rcar-csi2.txt        | 104 +++++++++++++++++++++
> >  MAINTAINERS                                        |   1 +
> >  2 files changed, 105 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/rcar-csi2.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/media/rcar-csi2.txt b/Documentation/devicetree/bindings/media/rcar-csi2.txt
> > new file mode 100644
> > index 0000000000000000..24705d8997b14a10
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/rcar-csi2.txt
> 
> How about using vendor prefix and comma for the file name? Most others in
> the directory do so, including other Renesas bindings.

To be clear you are suggesting a rename to 'renesas,rcar-csi2.txt'?  I'm 
not oppose to that but chose this name as this driver is closely tied to 
the rcar-vin driver which is documented in:

Documentation/devicetree/bindings/media/rcar_vin.txt

You also had comments on patch 2/2 so I will resend this series once 
more addressing them so will rename this as you suggests.

> 
> > @@ -0,0 +1,104 @@
> > +Renesas R-Car MIPI CSI-2
> > +------------------------
> > +
> > +The rcar-csi2 device provides MIPI CSI-2 capabilities for the Renesas R-Car
> > +family of devices. It is to be used in conjunction with the R-Car VIN module,
> > +which provides the video capture capabilities.
> > +
> > +Mandatory properties
> > +--------------------
> > + - compatible: Must be one or more of the following
> > +   - "renesas,r8a7795-csi2" for the R8A7795 device.
> > +   - "renesas,r8a7796-csi2" for the R8A7796 device.
> > +
> > + - reg: the register base and size for the device registers
> > + - interrupts: the interrupt for the device
> > + - clocks: Reference to the parent clock
> > +
> > +The device node shall contain two 'port' child nodes according to the
> > +bindings defined in Documentation/devicetree/bindings/media/
> > +video-interfaces.txt. Port 0 shall connect the node that is the video
> > +source for to the CSI-2. Port 1 shall connect all the R-Car VIN
> > +modules, which can make use of the CSI-2 module.
> > +
> > +- Port 0 - Video source (Mandatory)
> > +	- Endpoint 0 - sub-node describing the endpoint that is the video source
> > +
> > +- Port 1 - VIN instances (Mandatory for all VIN present in the SoC)
> > +	- Endpoint 0 - sub-node describing the endpoint that is VIN0
> > +	- Endpoint 1 - sub-node describing the endpoint that is VIN1
> > +	- Endpoint 2 - sub-node describing the endpoint that is VIN2
> > +	- Endpoint 3 - sub-node describing the endpoint that is VIN3
> > +	- Endpoint 4 - sub-node describing the endpoint that is VIN4
> > +	- Endpoint 5 - sub-node describing the endpoint that is VIN5
> > +	- Endpoint 6 - sub-node describing the endpoint that is VIN6
> > +	- Endpoint 7 - sub-node describing the endpoint that is VIN7
> > +
> > +Example:
> > +
> > +	csi20: csi2@fea80000 {
> > +		compatible = "renesas,r8a7796-csi2", "renesas,rcar-gen3-csi2";
> > +		reg = <0 0xfea80000 0 0x10000>;
> > +		interrupts = <0 184 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&cpg CPG_MOD 714>;
> > +		power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
> > +		resets = <&cpg 714>;
> > +
> > +		ports {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			port@0 {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +
> > +				reg = <0>;
> > +
> > +				csi20_in: endpoint@0 {
> > +					clock-lanes = <0>;
> > +					data-lanes = <1>;
> > +					remote-endpoint = <&adv7482_txb>;
> > +				};
> > +			};
> > +
> > +			port@1 {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +
> > +				reg = <1>;
> > +
> > +				csi20vin0: endpoint@0 {
> > +					reg = <0>;
> > +					remote-endpoint = <&vin0csi20>;
> > +				};
> > +				csi20vin1: endpoint@1 {
> > +					reg = <1>;
> > +					remote-endpoint = <&vin1csi20>;
> > +				};
> > +				csi20vin2: endpoint@2 {
> > +					reg = <2>;
> > +					remote-endpoint = <&vin2csi20>;
> > +				};
> > +				csi20vin3: endpoint@3 {
> > +					reg = <3>;
> > +					remote-endpoint = <&vin3csi20>;
> > +				};
> > +				csi20vin4: endpoint@4 {
> > +					reg = <4>;
> > +					remote-endpoint = <&vin4csi20>;
> > +				};
> > +				csi20vin5: endpoint@5 {
> > +					reg = <5>;
> > +					remote-endpoint = <&vin5csi20>;
> > +				};
> > +				csi20vin6: endpoint@6 {
> > +					reg = <6>;
> > +					remote-endpoint = <&vin6csi20>;
> > +				};
> > +				csi20vin7: endpoint@7 {
> > +					reg = <7>;
> > +					remote-endpoint = <&vin7csi20>;
> > +				};
> > +			};
> > +		};
> > +	};
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index adbf69306e9ee3d2..fa81ee6e80274646 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -8565,6 +8565,7 @@ L:	linux-media@vger.kernel.org
> >  L:	linux-renesas-soc@vger.kernel.org
> >  T:	git git://linuxtv.org/media_tree.git
> >  S:	Supported
> > +F:	Documentation/devicetree/bindings/media/rcar-csi2.txt
> >  F:	Documentation/devicetree/bindings/media/rcar_vin.txt
> >  F:	drivers/media/platform/rcar-vin/
> >  
> 
> -- 
> Sakari Ailus
> e-mail: sakari.ailus@iki.fi

-- 
Regards,
Niklas Söderlund

^ permalink raw reply

* Re: [PATCH v10 2/2] media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
From: Sakari Ailus @ 2017-11-10 22:32 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Laurent Pinchart, Hans Verkuil,
	linux-media-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
	tomoharu.fukawa.eb-zM6kxYcvzFBBDgjK7y7TUQ, Kieran Bingham,
	Sakari Ailus, Geert Uytterhoeven
In-Reply-To: <20171110133137.9137-3-niklas.soderlund+renesas-1zkq55x86MTxsAP9Fp7wbw@public.gmane.org>

Hej Niklas,

Tack för uppdaterade lappar! Jag har några kommentar nedan... det ser bra
ut överallt.

On Fri, Nov 10, 2017 at 02:31:37PM +0100, Niklas Söderlund wrote:
> A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver
> supports the rcar-vin driver on R-Car Gen3 SoCs where separate CSI-2
> hardware blocks are connected between the video sources and the video
> grabbers (VIN).
> 
> Driver is based on a prototype by Koji Matsuoka in the Renesas BSP.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas-1zkq55x86MTxsAP9Fp7wbw@public.gmane.org>
> ---
>  drivers/media/platform/rcar-vin/Kconfig     |  12 +
>  drivers/media/platform/rcar-vin/Makefile    |   1 +
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 934 ++++++++++++++++++++++++++++
>  3 files changed, 947 insertions(+)
>  create mode 100644 drivers/media/platform/rcar-vin/rcar-csi2.c
> 
> diff --git a/drivers/media/platform/rcar-vin/Kconfig b/drivers/media/platform/rcar-vin/Kconfig
> index af4c98b44d2e22cb..6875f30c1ae42631 100644
> --- a/drivers/media/platform/rcar-vin/Kconfig
> +++ b/drivers/media/platform/rcar-vin/Kconfig
> @@ -1,3 +1,15 @@
> +config VIDEO_RCAR_CSI2
> +	tristate "R-Car MIPI CSI-2 Receiver"
> +	depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF
> +	depends on ARCH_RENESAS || COMPILE_TEST
> +	select V4L2_FWNODE
> +	---help---
> +	  Support for Renesas R-Car MIPI CSI-2 receiver.
> +	  Supports R-Car Gen3 SoCs.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called rcar-csi2.
> +
>  config VIDEO_RCAR_VIN
>  	tristate "R-Car Video Input (VIN) Driver"
>  	depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF && HAS_DMA && MEDIA_CONTROLLER
> diff --git a/drivers/media/platform/rcar-vin/Makefile b/drivers/media/platform/rcar-vin/Makefile
> index 48c5632c21dc060b..5ab803d3e7c1aa57 100644
> --- a/drivers/media/platform/rcar-vin/Makefile
> +++ b/drivers/media/platform/rcar-vin/Makefile
> @@ -1,3 +1,4 @@
>  rcar-vin-objs = rcar-core.o rcar-dma.o rcar-v4l2.o
>  
> +obj-$(CONFIG_VIDEO_RCAR_CSI2) += rcar-csi2.o
>  obj-$(CONFIG_VIDEO_RCAR_VIN) += rcar-vin.o
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> new file mode 100644
> index 0000000000000000..27d09da191a09b39
> --- /dev/null
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -0,0 +1,934 @@
> +/*
> + * Driver for Renesas R-Car MIPI CSI-2 Receiver
> + *
> + * Copyright (C) 2017 Renesas Electronics Corp.
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_graph.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/sys_soc.h>
> +
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-mc.h>
> +#include <media/v4l2-subdev.h>
> +
> +/* Register offsets and bits */
> +
> +/* Control Timing Select */
> +#define TREF_REG			0x00
> +#define TREF_TREF			BIT(0)
> +
> +/* Software Reset */
> +#define SRST_REG			0x04
> +#define SRST_SRST			BIT(0)
> +
> +/* PHY Operation Control */
> +#define PHYCNT_REG			0x08
> +#define PHYCNT_SHUTDOWNZ		BIT(17)
> +#define PHYCNT_RSTZ			BIT(16)
> +#define PHYCNT_ENABLECLK		BIT(4)
> +#define PHYCNT_ENABLE_3			BIT(3)
> +#define PHYCNT_ENABLE_2			BIT(2)
> +#define PHYCNT_ENABLE_1			BIT(1)
> +#define PHYCNT_ENABLE_0			BIT(0)
> +
> +/* Checksum Control */
> +#define CHKSUM_REG			0x0c
> +#define CHKSUM_ECC_EN			BIT(1)
> +#define CHKSUM_CRC_EN			BIT(0)
> +
> +/*
> + * Channel Data Type Select
> + * VCDT[0-15]:  Channel 1 VCDT[16-31]:  Channel 2
> + * VCDT2[0-15]: Channel 3 VCDT2[16-31]: Channel 4
> + */
> +#define VCDT_REG			0x10
> +#define VCDT2_REG			0x14
> +#define VCDT_VCDTN_EN			BIT(15)
> +#define VCDT_SEL_VC(n)			(((n) & 0x3) << 8)
> +#define VCDT_SEL_DTN_ON			BIT(6)
> +#define VCDT_SEL_DT(n)			(((n) & 0x3f) << 0)
> +
> +/* Frame Data Type Select */
> +#define FRDT_REG			0x18
> +
> +/* Field Detection Control */
> +#define FLD_REG				0x1c
> +#define FLD_FLD_NUM(n)			(((n) & 0xff) << 16)
> +#define FLD_FLD_EN4			BIT(3)
> +#define FLD_FLD_EN3			BIT(2)
> +#define FLD_FLD_EN2			BIT(1)
> +#define FLD_FLD_EN			BIT(0)
> +
> +/* Automatic Standby Control */
> +#define ASTBY_REG			0x20
> +
> +/* Long Data Type Setting 0 */
> +#define LNGDT0_REG			0x28
> +
> +/* Long Data Type Setting 1 */
> +#define LNGDT1_REG			0x2c
> +
> +/* Interrupt Enable */
> +#define INTEN_REG			0x30
> +
> +/* Interrupt Source Mask */
> +#define INTCLOSE_REG			0x34
> +
> +/* Interrupt Status Monitor */
> +#define INTSTATE_REG			0x38
> +#define INTSTATE_INT_ULPS_START		BIT(7)
> +#define INTSTATE_INT_ULPS_END		BIT(6)
> +
> +/* Interrupt Error Status Monitor */
> +#define INTERRSTATE_REG			0x3c
> +
> +/* Short Packet Data */
> +#define SHPDAT_REG			0x40
> +
> +/* Short Packet Count */
> +#define SHPCNT_REG			0x44
> +
> +/* LINK Operation Control */
> +#define LINKCNT_REG			0x48
> +#define LINKCNT_MONITOR_EN		BIT(31)
> +#define LINKCNT_REG_MONI_PACT_EN	BIT(25)
> +#define LINKCNT_ICLK_NONSTOP		BIT(24)
> +
> +/* Lane Swap */
> +#define LSWAP_REG			0x4c
> +#define LSWAP_L3SEL(n)			(((n) & 0x3) << 6)
> +#define LSWAP_L2SEL(n)			(((n) & 0x3) << 4)
> +#define LSWAP_L1SEL(n)			(((n) & 0x3) << 2)
> +#define LSWAP_L0SEL(n)			(((n) & 0x3) << 0)
> +
> +/* PHY Test Interface Write Register */
> +#define PHTW_REG			0x50
> +
> +/* PHY Test Interface Clear */
> +#define PHTC_REG			0x58
> +#define PHTC_TESTCLR			BIT(0)
> +
> +/* PHY Frequency Control */
> +#define PHYPLL_REG			0x68
> +#define PHYPLL_HSFREQRANGE(n)		((n) << 16)
> +
> +struct phypll_hsfreqrange {
> +	u16 mbps;
> +	u16 reg;
> +};
> +
> +static const struct phypll_hsfreqrange hsfreqrange_h3_v3h_m3n[] = {
> +	{ .mbps =   80, .reg = 0x00 },
> +	{ .mbps =   90, .reg = 0x10 },
> +	{ .mbps =  100, .reg = 0x20 },
> +	{ .mbps =  110, .reg = 0x30 },
> +	{ .mbps =  120, .reg = 0x01 },
> +	{ .mbps =  130, .reg = 0x11 },
> +	{ .mbps =  140, .reg = 0x21 },
> +	{ .mbps =  150, .reg = 0x31 },
> +	{ .mbps =  160, .reg = 0x02 },
> +	{ .mbps =  170, .reg = 0x12 },
> +	{ .mbps =  180, .reg = 0x22 },
> +	{ .mbps =  190, .reg = 0x32 },
> +	{ .mbps =  205, .reg = 0x03 },
> +	{ .mbps =  220, .reg = 0x13 },
> +	{ .mbps =  235, .reg = 0x23 },
> +	{ .mbps =  250, .reg = 0x33 },
> +	{ .mbps =  275, .reg = 0x04 },
> +	{ .mbps =  300, .reg = 0x14 },
> +	{ .mbps =  325, .reg = 0x25 },
> +	{ .mbps =  350, .reg = 0x35 },
> +	{ .mbps =  400, .reg = 0x05 },
> +	{ .mbps =  450, .reg = 0x26 },
> +	{ .mbps =  500, .reg = 0x36 },
> +	{ .mbps =  550, .reg = 0x37 },
> +	{ .mbps =  600, .reg = 0x07 },
> +	{ .mbps =  650, .reg = 0x18 },
> +	{ .mbps =  700, .reg = 0x28 },
> +	{ .mbps =  750, .reg = 0x39 },
> +	{ .mbps =  800, .reg = 0x09 },
> +	{ .mbps =  850, .reg = 0x19 },
> +	{ .mbps =  900, .reg = 0x29 },
> +	{ .mbps =  950, .reg = 0x3a },
> +	{ .mbps = 1000, .reg = 0x0a },
> +	{ .mbps = 1050, .reg = 0x1a },
> +	{ .mbps = 1100, .reg = 0x2a },
> +	{ .mbps = 1150, .reg = 0x3b },
> +	{ .mbps = 1200, .reg = 0x0b },
> +	{ .mbps = 1250, .reg = 0x1b },
> +	{ .mbps = 1300, .reg = 0x2b },
> +	{ .mbps = 1350, .reg = 0x3c },
> +	{ .mbps = 1400, .reg = 0x0c },
> +	{ .mbps = 1450, .reg = 0x1c },
> +	{ .mbps = 1500, .reg = 0x2c },
> +	/* guard */
> +	{ .mbps =   0,	.reg = 0x00 },
> +};
> +
> +static const struct phypll_hsfreqrange hsfreqrange_m3w_h3es1[] = {
> +	{ .mbps =   80,	.reg = 0x00 },
> +	{ .mbps =   90,	.reg = 0x10 },
> +	{ .mbps =  100,	.reg = 0x20 },
> +	{ .mbps =  110,	.reg = 0x30 },
> +	{ .mbps =  120,	.reg = 0x01 },
> +	{ .mbps =  130,	.reg = 0x11 },
> +	{ .mbps =  140,	.reg = 0x21 },
> +	{ .mbps =  150,	.reg = 0x31 },
> +	{ .mbps =  160,	.reg = 0x02 },
> +	{ .mbps =  170,	.reg = 0x12 },
> +	{ .mbps =  180,	.reg = 0x22 },
> +	{ .mbps =  190,	.reg = 0x32 },
> +	{ .mbps =  205,	.reg = 0x03 },
> +	{ .mbps =  220,	.reg = 0x13 },
> +	{ .mbps =  235,	.reg = 0x23 },
> +	{ .mbps =  250,	.reg = 0x33 },
> +	{ .mbps =  275,	.reg = 0x04 },
> +	{ .mbps =  300,	.reg = 0x14 },
> +	{ .mbps =  325,	.reg = 0x05 },
> +	{ .mbps =  350,	.reg = 0x15 },
> +	{ .mbps =  400,	.reg = 0x25 },
> +	{ .mbps =  450,	.reg = 0x06 },
> +	{ .mbps =  500,	.reg = 0x16 },
> +	{ .mbps =  550,	.reg = 0x07 },
> +	{ .mbps =  600,	.reg = 0x17 },
> +	{ .mbps =  650,	.reg = 0x08 },
> +	{ .mbps =  700,	.reg = 0x18 },
> +	{ .mbps =  750,	.reg = 0x09 },
> +	{ .mbps =  800,	.reg = 0x19 },
> +	{ .mbps =  850,	.reg = 0x29 },
> +	{ .mbps =  900,	.reg = 0x39 },
> +	{ .mbps =  950,	.reg = 0x0A },
> +	{ .mbps = 1000,	.reg = 0x1A },
> +	{ .mbps = 1050,	.reg = 0x2A },
> +	{ .mbps = 1100,	.reg = 0x3A },
> +	{ .mbps = 1150,	.reg = 0x0B },
> +	{ .mbps = 1200,	.reg = 0x1B },
> +	{ .mbps = 1250,	.reg = 0x2B },
> +	{ .mbps = 1300,	.reg = 0x3B },
> +	{ .mbps = 1350,	.reg = 0x0C },
> +	{ .mbps = 1400,	.reg = 0x1C },
> +	{ .mbps = 1450,	.reg = 0x2C },
> +	{ .mbps = 1500,	.reg = 0x3C },
> +	/* guard */
> +	{ .mbps =   0,	.reg = 0x00 },
> +};
> +
> +/* PHY ESC Error Monitor */
> +#define PHEERM_REG			0x74
> +
> +/* PHY Clock Lane Monitor */
> +#define PHCLM_REG			0x78
> +
> +/* PHY Data Lane Monitor */
> +#define PHDLM_REG			0x7c
> +
> +/* CSI0CLK Frequency Configuration Preset Register */
> +#define CSI0CLKFCPR_REG			0x260
> +#define CSI0CLKFREQRANGE(n)		((n & 0x3f) << 16)
> +
> +struct rcar_csi2_format {
> +	unsigned int code;
> +	unsigned int datatype;
> +	unsigned int bpp;
> +};
> +
> +static const struct rcar_csi2_format rcar_csi2_formats[] = {
> +	{ .code = MEDIA_BUS_FMT_RGB888_1X24,	.datatype = 0x24, .bpp = 24 },
> +	{ .code = MEDIA_BUS_FMT_UYVY8_1X16,	.datatype = 0x1e, .bpp = 16 },
> +	{ .code = MEDIA_BUS_FMT_UYVY8_2X8,	.datatype = 0x1e, .bpp = 16 },
> +	{ .code = MEDIA_BUS_FMT_YUYV10_2X10,	.datatype = 0x1e, .bpp = 16 },
> +};
> +
> +static const struct rcar_csi2_format *rcar_csi2_code_to_fmt(unsigned int code)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(rcar_csi2_formats); i++)
> +		if (rcar_csi2_formats[i].code == code)
> +			return rcar_csi2_formats + i;

Extra newline would be nice.

> +	return NULL;
> +}
> +
> +enum rcar_csi2_pads {
> +	RCAR_CSI2_SINK,
> +	RCAR_CSI2_SOURCE_VC0,
> +	RCAR_CSI2_SOURCE_VC1,
> +	RCAR_CSI2_SOURCE_VC2,
> +	RCAR_CSI2_SOURCE_VC3,
> +	NR_OF_RCAR_CSI2_PAD,
> +};
> +
> +struct rcar_csi2_info {
> +	const struct phypll_hsfreqrange *hsfreqrange;
> +	unsigned int csi0clkfreqrange;
> +	bool clear_ulps;
> +	bool have_phtw;
> +};
> +
> +struct rcar_csi2 {
> +	struct device *dev;
> +	void __iomem *base;
> +	const struct rcar_csi2_info *info;
> +
> +	struct v4l2_subdev subdev;
> +	struct media_pad pads[NR_OF_RCAR_CSI2_PAD];
> +
> +	struct v4l2_async_notifier notifier;
> +	struct v4l2_async_subdev remote;
> +
> +	struct v4l2_mbus_framefmt mf;
> +
> +	struct mutex lock;
> +	int stream_count;
> +
> +	unsigned short lanes;
> +	unsigned char lane_swap[4];
> +};
> +
> +static inline struct rcar_csi2 *sd_to_csi2(struct v4l2_subdev *sd)
> +{
> +	return container_of(sd, struct rcar_csi2, subdev);
> +}
> +
> +static inline struct rcar_csi2 *notifier_to_csi2(struct v4l2_async_notifier *n)
> +{
> +	return container_of(n, struct rcar_csi2, notifier);
> +}
> +
> +static u32 rcar_csi2_read(struct rcar_csi2 *priv, unsigned int reg)
> +{
> +	return ioread32(priv->base + reg);
> +}
> +
> +static void rcar_csi2_write(struct rcar_csi2 *priv, unsigned int reg, u32 data)
> +{
> +	iowrite32(data, priv->base + reg);
> +}
> +
> +static void rcar_csi2_reset(struct rcar_csi2 *priv)
> +{
> +	rcar_csi2_write(priv, SRST_REG, SRST_SRST);
> +	usleep_range(100, 150);
> +	rcar_csi2_write(priv, SRST_REG, 0);
> +}
> +
> +static int rcar_csi2_wait_phy_start(struct rcar_csi2 *priv)
> +{
> +	int timeout;
> +
> +	/* Wait for the clock and data lanes to enter LP-11 state. */
> +	for (timeout = 100; timeout > 0; timeout--) {
> +		const u32 lane_mask = (1 << priv->lanes) - 1;
> +
> +		if ((rcar_csi2_read(priv, PHCLM_REG) & 1) == 1 &&
> +		    (rcar_csi2_read(priv, PHDLM_REG) & lane_mask) == lane_mask)
> +			return 0;
> +
> +		msleep(20);
> +	}
> +
> +	dev_err(priv->dev, "Timeout waiting for LP-11 state\n");
> +
> +	return -ETIMEDOUT;
> +}
> +
> +static int rcar_csi2_calc_phypll(struct rcar_csi2 *priv,
> +				 struct v4l2_subdev *source,
> +				 struct v4l2_mbus_framefmt *mf,
> +				 u32 *phypll)
> +{
> +	const struct phypll_hsfreqrange *hsfreq;
> +	const struct rcar_csi2_format *format;
> +	struct v4l2_ctrl *ctrl;
> +	u64 mbps;
> +
> +	ctrl = v4l2_ctrl_find(source->ctrl_handler, V4L2_CID_PIXEL_RATE);

How about LINK_FREQ instead? It'd be more straightforward to calculate
this. Up to you.

> +	if (!ctrl) {
> +		dev_err(priv->dev, "no link freq control in subdev %s\n",
> +			source->name);
> +		return -EINVAL;
> +	}
> +
> +	format = rcar_csi2_code_to_fmt(mf->code);
> +	if (!format) {
> +		dev_err(priv->dev, "Unknown format: %d\n", mf->code);
> +		return -EINVAL;
> +	}
> +
> +	mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * format->bpp;
> +	do_div(mbps, priv->lanes * 1000000);
> +
> +	for (hsfreq = priv->info->hsfreqrange; hsfreq->mbps != 0; hsfreq++)
> +		if (hsfreq->mbps >= mbps)
> +			break;
> +
> +	if (!hsfreq->mbps) {
> +		dev_err(priv->dev, "Unsupported PHY speed (%llu Mbps)", mbps);
> +		return -ERANGE;
> +	}
> +
> +	dev_dbg(priv->dev, "PHY HSFREQRANGE requested %llu got %u Mbps\n", mbps,
> +		hsfreq->mbps);
> +
> +	*phypll = PHYPLL_HSFREQRANGE(hsfreq->reg);
> +
> +	return 0;
> +}
> +
> +static int rcar_csi2_start(struct rcar_csi2 *priv)
> +{
> +	const struct rcar_csi2_format *format;
> +	struct v4l2_subdev_format fmt;
> +	struct media_pad *pad, *source_pad;
> +	struct v4l2_subdev *source = NULL;
> +	struct v4l2_mbus_framefmt *mf = &fmt.format;
> +	u32 phycnt, phypll, tmp;
> +	u32 vcdt = 0, vcdt2 = 0;
> +	unsigned int i;
> +	int ret;
> +
> +	pad = &priv->subdev.entity.pads[RCAR_CSI2_SINK];
> +	source_pad = media_entity_remote_pad(pad);
> +	if (!source_pad) {
> +		dev_err(priv->dev, "Could not find remote source pad\n");
> +		return -ENODEV;
> +	}
> +
> +	source = media_entity_to_v4l2_subdev(source_pad->entity);
> +	if (!source) {
> +		dev_err(priv->dev, "Could not find remote subdevice\n");
> +		return -ENODEV;
> +	}

Something must be really, really wrong if the above would happen. A pad
without an entity?

> +
> +	dev_dbg(priv->dev, "Using source %s pad: %u\n", source->name,
> +		source_pad->index);
> +
> +	fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> +	fmt.pad = source_pad->index;
> +	ret = v4l2_subdev_call(source, pad, get_fmt, NULL, &fmt);
> +	if (ret)
> +		return ret;

The link format has been validated already by this point this isn't
supposed to fail or produce different results than in link validation.

You could get the same format from the local pad, too.

> +
> +	dev_dbg(priv->dev, "Input size (%ux%u%c)\n", mf->width,
> +		mf->height, mf->field == V4L2_FIELD_NONE ? 'p' : 'i');
> +
> +	/*
> +	 * Enable all Virtual Channels
> +	 *
> +	 * NOTE: It's not possible to get individual format for each
> +	 *       source virtual channel. Once this is possible in V4L2
> +	 *       it should be used here.
> +	 */
> +	for (i = 0; i < 4; i++) {
> +		format = rcar_csi2_code_to_fmt(mf->code);
> +		if (!format) {
> +			dev_err(priv->dev, "Unsupported media bus format: %d\n",
> +				mf->code);
> +			return -EINVAL;
> +		}
> +
> +		tmp = VCDT_SEL_VC(i) | VCDT_VCDTN_EN | VCDT_SEL_DTN_ON |
> +			VCDT_SEL_DT(format->datatype);
> +
> +		/* Store in correct reg and offset */
> +		if (i < 2)
> +			vcdt |= tmp << ((i % 2) * 16);
> +		else
> +			vcdt2 |= tmp << ((i % 2) * 16);
> +	}
> +
> +	switch (priv->lanes) {
> +	case 1:
> +		phycnt = PHYCNT_ENABLECLK | PHYCNT_ENABLE_0;
> +		break;
> +	case 2:
> +		phycnt = PHYCNT_ENABLECLK | PHYCNT_ENABLE_1 | PHYCNT_ENABLE_0;
> +		break;
> +	case 4:
> +		phycnt = PHYCNT_ENABLECLK | PHYCNT_ENABLE_3 | PHYCNT_ENABLE_2 |
> +			PHYCNT_ENABLE_1 | PHYCNT_ENABLE_0;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret = rcar_csi2_calc_phypll(priv, source, mf, &phypll);
> +	if (ret)
> +		return ret;
> +
> +	/* Clear Ultra Low Power interrupt */
> +	if (priv->info->clear_ulps)
> +		rcar_csi2_write(priv, INTSTATE_REG,
> +				INTSTATE_INT_ULPS_START |
> +				INTSTATE_INT_ULPS_END);
> +
> +	/* Init */
> +	rcar_csi2_write(priv, TREF_REG, TREF_TREF);
> +	rcar_csi2_reset(priv);
> +	rcar_csi2_write(priv, PHTC_REG, 0);
> +
> +	/* Configure */
> +	rcar_csi2_write(priv, FLD_REG, FLD_FLD_NUM(2) | FLD_FLD_EN4 |
> +			FLD_FLD_EN3 | FLD_FLD_EN2 | FLD_FLD_EN);
> +	rcar_csi2_write(priv, VCDT_REG, vcdt);
> +	rcar_csi2_write(priv, VCDT2_REG, vcdt2);
> +	/* Lanes are zero indexed */
> +	rcar_csi2_write(priv, LSWAP_REG,
> +			LSWAP_L0SEL(priv->lane_swap[0] - 1) |
> +			LSWAP_L1SEL(priv->lane_swap[1] - 1) |
> +			LSWAP_L2SEL(priv->lane_swap[2] - 1) |
> +			LSWAP_L3SEL(priv->lane_swap[3] - 1));
> +
> +	if (priv->info->have_phtw) {
> +		/*
> +		 * This is for H3 ES2.0
> +		 *
> +		 * NOTE: Additional logic is needed here when
> +		 * support for V3H and/or M3-N is added
> +		 */
> +		rcar_csi2_write(priv, PHTW_REG, 0x01cc01e2);
> +		rcar_csi2_write(priv, PHTW_REG, 0x010101e3);
> +		rcar_csi2_write(priv, PHTW_REG, 0x010101e4);
> +		rcar_csi2_write(priv, PHTW_REG, 0x01100104);
> +		rcar_csi2_write(priv, PHTW_REG, 0x01030100);
> +		rcar_csi2_write(priv, PHTW_REG, 0x01800100);
> +	}
> +
> +	/* Start */
> +	rcar_csi2_write(priv, PHYPLL_REG, phypll);
> +
> +	/* Set frequency range if we have it */
> +	if (priv->info->csi0clkfreqrange)
> +		rcar_csi2_write(priv, CSI0CLKFCPR_REG,
> +				CSI0CLKFREQRANGE(priv->info->csi0clkfreqrange));
> +
> +	rcar_csi2_write(priv, PHYCNT_REG, phycnt);
> +	rcar_csi2_write(priv, LINKCNT_REG, LINKCNT_MONITOR_EN |
> +			LINKCNT_REG_MONI_PACT_EN | LINKCNT_ICLK_NONSTOP);
> +	rcar_csi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ);
> +	rcar_csi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ |
> +			PHYCNT_RSTZ);
> +
> +	return rcar_csi2_wait_phy_start(priv);
> +}
> +
> +static void rcar_csi2_stop(struct rcar_csi2 *priv)
> +{
> +	rcar_csi2_write(priv, PHYCNT_REG, 0);
> +
> +	rcar_csi2_reset(priv);
> +}
> +
> +static int rcar_csi2_sd_info(struct rcar_csi2 *priv, struct v4l2_subdev **sd)
> +{
> +	struct media_pad *pad;
> +
> +	pad = media_entity_remote_pad(&priv->pads[RCAR_CSI2_SINK]);
> +	if (!pad) {
> +		dev_err(priv->dev, "Could not find remote pad\n");
> +		return -ENODEV;
> +	}
> +
> +	*sd = media_entity_to_v4l2_subdev(pad->entity);
> +	if (!*sd) {
> +		dev_err(priv->dev, "Could not find remote subdevice\n");
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +
> +static int rcar_csi2_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> +	struct rcar_csi2 *priv = sd_to_csi2(sd);
> +	struct v4l2_subdev *nextsd;
> +	int ret;
> +
> +	mutex_lock(&priv->lock);
> +
> +	ret = rcar_csi2_sd_info(priv, &nextsd);
> +	if (ret)
> +		goto out;
> +
> +	if (enable && priv->stream_count == 0) {
> +		ret =  rcar_csi2_start(priv);
> +		if (ret)
> +			goto out;
> +
> +		ret = v4l2_subdev_call(nextsd, video, s_stream, 1);
> +		if (ret) {
> +			rcar_csi2_stop(priv);
> +			goto out;
> +		}
> +	} else if (!enable && priv->stream_count == 1) {
> +		rcar_csi2_stop(priv);
> +		ret = v4l2_subdev_call(nextsd, video, s_stream, 0);
> +	}
> +
> +	priv->stream_count += enable ? 1 : -1;
> +out:
> +	mutex_unlock(&priv->lock);
> +
> +	return ret;
> +}
> +
> +static int rcar_csi2_s_power(struct v4l2_subdev *sd, int on)
> +{
> +	struct rcar_csi2 *priv = sd_to_csi2(sd);
> +
> +	if (on)
> +		pm_runtime_get_sync(priv->dev);
> +	else
> +		pm_runtime_put(priv->dev);

The pipeline you have is already rather complex. Would it be an option to
power the hardware on when streaming is started? The smiapp driver does
this, without even implementing the s_power callback. (You'd still need to
call it on the image source, as long as we have drivers that need it.)

> +
> +	return 0;
> +}
> +
> +static int rcar_csi2_set_pad_format(struct v4l2_subdev *sd,
> +				    struct v4l2_subdev_pad_config *cfg,
> +				    struct v4l2_subdev_format *format)
> +{
> +	struct rcar_csi2 *priv = sd_to_csi2(sd);
> +	struct v4l2_mbus_framefmt *framefmt;
> +

Is everything possible? For instance, are there limits regarding width or
height? Or the pixel code? They should be checked here.

Also the format on the source pad is, presumably, dependent on the format
on the sink pad.

> +	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> +		priv->mf = format->format;
> +	} else {
> +		framefmt = v4l2_subdev_get_try_format(sd, cfg, 0);
> +		*framefmt = format->format;
> +	}
> +
> +	return 0;
> +}
> +
> +static int rcar_csi2_get_pad_format(struct v4l2_subdev *sd,
> +				    struct v4l2_subdev_pad_config *cfg,
> +				    struct v4l2_subdev_format *format)
> +{
> +	struct rcar_csi2 *priv = sd_to_csi2(sd);
> +
> +	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> +		format->format = priv->mf;
> +	else
> +		format->format = *v4l2_subdev_get_try_format(sd, cfg, 0);
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_subdev_video_ops rcar_csi2_video_ops = {
> +	.s_stream = rcar_csi2_s_stream,
> +};
> +
> +static const struct v4l2_subdev_core_ops rcar_csi2_subdev_core_ops = {
> +	.s_power = rcar_csi2_s_power,
> +};
> +
> +static const struct v4l2_subdev_pad_ops rcar_csi2_pad_ops = {
> +	.set_fmt = rcar_csi2_set_pad_format,
> +	.get_fmt = rcar_csi2_get_pad_format,
> +};
> +
> +static const struct v4l2_subdev_ops rcar_csi2_subdev_ops = {
> +	.video	= &rcar_csi2_video_ops,
> +	.core	= &rcar_csi2_subdev_core_ops,
> +	.pad	= &rcar_csi2_pad_ops,
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * Async and registered of subdevices and links
> + */
> +
> +static int rcar_csi2_notify_bound(struct v4l2_async_notifier *notifier,
> +				   struct v4l2_subdev *subdev,
> +				   struct v4l2_async_subdev *asd)
> +{
> +	struct rcar_csi2 *priv = notifier_to_csi2(notifier);
> +	int pad;
> +
> +	v4l2_set_subdev_hostdata(subdev, priv);
> +
> +	pad = media_entity_get_fwnode_pad(&subdev->entity,
> +					  asd->match.fwnode.fwnode,
> +					  MEDIA_PAD_FL_SOURCE);
> +	if (pad < 0) {
> +		dev_err(priv->dev, "Failed to find pad for %s\n", subdev->name);
> +		return pad;
> +	}
> +
> +	dev_dbg(priv->dev, "Bound %s pad: %d\n", subdev->name, pad);
> +
> +	return media_create_pad_link(&subdev->entity, pad,
> +				     &priv->subdev.entity, 0,
> +				     MEDIA_LNK_FL_ENABLED |
> +				     MEDIA_LNK_FL_IMMUTABLE);
> +}
> +
> +static void rcar_csi2_notify_unbind(struct v4l2_async_notifier *notifier,
> +				     struct v4l2_subdev *subdev,
> +				     struct v4l2_async_subdev *asd)
> +{
> +	struct rcar_csi2 *priv = notifier_to_csi2(notifier);
> +
> +	dev_dbg(priv->dev, "Unbind %s\n", subdev->name);

This doesn't really do anything. I'd probably just remove this.

> +}
> +
> +static const struct v4l2_async_notifier_operations rcar_csi2_notify_ops = {
> +	.bound = rcar_csi2_notify_bound,
> +	.unbind = rcar_csi2_notify_unbind,
> +};
> +
> +static int rcar_csi2_parse_v4l2(struct rcar_csi2 *priv,
> +				struct v4l2_fwnode_endpoint *vep)
> +{
> +	unsigned int i;
> +
> +	/* Only port 0 enpoint 0 is valid */
> +	if (vep->base.port || vep->base.id)
> +		return -ENOTCONN;
> +
> +	if (vep->bus_type != V4L2_MBUS_CSI2) {
> +		dev_err(priv->dev, "Unsupported bus: 0x%x\n", vep->bus_type);
> +		return -EINVAL;
> +	}
> +
> +	priv->lanes = vep->bus.mipi_csi2.num_data_lanes;
> +	if (priv->lanes != 1 && priv->lanes != 2 && priv->lanes != 4) {
> +		dev_err(priv->dev, "Unsupported number of data-lanes: %d\n",
> +			priv->lanes);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(priv->lane_swap); i++) {
> +		priv->lane_swap[i] = i < priv->lanes ?
> +			vep->bus.mipi_csi2.data_lanes[i] : i;
> +
> +		/* Check for valid lane number */
> +		if (priv->lane_swap[i] < 1 || priv->lane_swap[i] > 4) {
> +			dev_err(priv->dev, "data-lanes must be in 1-4 range\n");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int rcar_csi2_parse_dt(struct rcar_csi2 *priv)
> +{
> +	struct device_node *ep;
> +	struct v4l2_fwnode_endpoint v4l2_ep;
> +	int ret;
> +
> +	ep = of_graph_get_endpoint_by_regs(priv->dev->of_node, 0, 0);
> +	if (!ep) {
> +		dev_err(priv->dev, "Not connected to subdevice\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &v4l2_ep);
> +	if (ret) {
> +		dev_err(priv->dev, "Could not parse v4l2 endpoint\n");
> +		of_node_put(ep);
> +		return -EINVAL;
> +	}
> +
> +	ret = rcar_csi2_parse_v4l2(priv, &v4l2_ep);
> +	if (ret)
> +		return ret;
> +
> +	priv->remote.match.fwnode.fwnode =
> +		fwnode_graph_get_remote_endpoint(of_fwnode_handle(ep));
> +	priv->remote.match_type = V4L2_ASYNC_MATCH_FWNODE;
> +
> +	of_node_put(ep);
> +
> +	priv->notifier.subdevs = devm_kzalloc(priv->dev,
> +					      sizeof(*priv->notifier.subdevs),
> +					      GFP_KERNEL);
> +	if (priv->notifier.subdevs == NULL)
> +		return -ENOMEM;
> +
> +	priv->notifier.num_subdevs = 1;
> +	priv->notifier.subdevs[0] = &priv->remote;
> +	priv->notifier.ops = &rcar_csi2_notify_ops;
> +
> +	dev_dbg(priv->dev, "Found '%pOF'\n",
> +		to_of_node(priv->remote.match.fwnode.fwnode));
> +
> +	return v4l2_async_subdev_notifier_register(&priv->subdev,
> +						   &priv->notifier);

Could you use v4l2_async_notifier_parse_fwnode_endpoints_by_port() or does
something bad happen if you do? The intent with that was to avoid functions
like the above in drivers.

> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Platform Device Driver
> + */
> +
> +static const struct media_entity_operations rcar_csi2_entity_ops = {
> +	.link_validate = v4l2_subdev_link_validate,
> +};
> +
> +static int rcar_csi2_probe_resources(struct rcar_csi2 *priv,
> +				     struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	int irq;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	priv->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(priv->base))
> +		return PTR_ERR(priv->base);
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0)
> +		return irq;
> +
> +	return 0;
> +}
> +
> +static const struct rcar_csi2_info rcar_csi2_info_r8a7795 = {
> +	.hsfreqrange = hsfreqrange_h3_v3h_m3n,
> +	.clear_ulps = true,
> +	.have_phtw = true,
> +	.csi0clkfreqrange = 0x20,
> +};
> +
> +static const struct rcar_csi2_info rcar_csi2_info_r8a7795es1 = {
> +	.hsfreqrange = hsfreqrange_m3w_h3es1,
> +};
> +
> +static const struct rcar_csi2_info rcar_csi2_info_r8a7796 = {
> +	.hsfreqrange = hsfreqrange_m3w_h3es1,
> +};
> +
> +static const struct of_device_id rcar_csi2_of_table[] = {
> +	{
> +		.compatible = "renesas,r8a7795-csi2",
> +		.data = &rcar_csi2_info_r8a7795,
> +	},
> +	{
> +		.compatible = "renesas,r8a7796-csi2",
> +		.data = &rcar_csi2_info_r8a7796,
> +	},
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, rcar_csi2_of_table);
> +
> +static const struct soc_device_attribute r8a7795es1[] = {
> +	{
> +		.soc_id = "r8a7795", .revision = "ES1.*",
> +		.data = &rcar_csi2_info_r8a7795es1,
> +	},
> +	{ /* sentinel */}
> +};
> +
> +static int rcar_csi2_probe(struct platform_device *pdev)
> +{
> +	const struct soc_device_attribute *attr;
> +	struct rcar_csi2 *priv;
> +	unsigned int i;
> +	int ret;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->info = of_device_get_match_data(&pdev->dev);
> +
> +	/* r8a7795 ES1.x behaves different then ES2.0+ but no own compat */
> +	attr = soc_device_match(r8a7795es1);
> +	if (attr)
> +		priv->info = attr->data;
> +
> +	priv->dev = &pdev->dev;
> +
> +	mutex_init(&priv->lock);
> +	priv->stream_count = 0;
> +
> +	ret = rcar_csi2_probe_resources(priv, pdev);
> +	if (ret) {
> +		dev_err(priv->dev, "Failed to get resources\n");
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	ret = rcar_csi2_parse_dt(priv);
> +	if (ret)
> +		return ret;
> +
> +	priv->subdev.owner = THIS_MODULE;
> +	priv->subdev.dev = &pdev->dev;
> +	v4l2_subdev_init(&priv->subdev, &rcar_csi2_subdev_ops);
> +	v4l2_set_subdevdata(&priv->subdev, &pdev->dev);
> +	snprintf(priv->subdev.name, V4L2_SUBDEV_NAME_SIZE, "%s %s",
> +		 KBUILD_MODNAME, dev_name(&pdev->dev));
> +	priv->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +
> +	priv->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
> +	priv->subdev.entity.ops = &rcar_csi2_entity_ops;
> +
> +	priv->pads[RCAR_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
> +	for (i = RCAR_CSI2_SOURCE_VC0; i < NR_OF_RCAR_CSI2_PAD; i++)
> +		priv->pads[i].flags = MEDIA_PAD_FL_SOURCE;
> +
> +	ret = media_entity_pads_init(&priv->subdev.entity, NR_OF_RCAR_CSI2_PAD,
> +				     priv->pads);
> +	if (ret)
> +		goto error;
> +
> +	ret = v4l2_async_register_subdev(&priv->subdev);
> +	if (ret < 0)
> +		goto error;
> +
> +	pm_runtime_enable(&pdev->dev);
> +
> +	dev_info(priv->dev, "%d lanes found\n", priv->lanes);
> +
> +	return 0;
> +
> +error:
> +	v4l2_async_notifier_cleanup(&priv->notifier);
> +
> +	return ret;
> +}
> +
> +static int rcar_csi2_remove(struct platform_device *pdev)
> +{
> +	struct rcar_csi2 *priv = platform_get_drvdata(pdev);
> +
> +	v4l2_async_notifier_cleanup(&priv->notifier);
> +	v4l2_async_unregister_subdev(&priv->subdev);
> +
> +	pm_runtime_disable(&pdev->dev);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver __refdata rcar_csi2_pdrv = {
> +	.remove	= rcar_csi2_remove,
> +	.probe	= rcar_csi2_probe,
> +	.driver	= {
> +		.name	= "rcar-csi2",
> +		.of_match_table	= of_match_ptr(rcar_csi2_of_table),
> +	},
> +};
> +
> +module_platform_driver(rcar_csi2_pdrv);
> +
> +MODULE_AUTHOR("Niklas Söderlund <niklas.soderlund-1zkq55x86MTxsAP9Fp7wbw@public.gmane.org>");
> +MODULE_DESCRIPTION("Renesas R-Car MIPI CSI-2 receiver");
> +MODULE_LICENSE("GPL v2");

-- 
Kind regards,

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

^ permalink raw reply

* Re: [PATCH v10 1/2] media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver documentation
From: Sakari Ailus @ 2017-11-10 22:11 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Laurent Pinchart, Hans Verkuil, linux-media, Rob Herring,
	devicetree, linux-renesas-soc, tomoharu.fukawa.eb, Kieran Bingham,
	Sakari Ailus, Geert Uytterhoeven
In-Reply-To: <20171110133137.9137-2-niklas.soderlund+renesas@ragnatech.se>

Hejssan,

On Fri, Nov 10, 2017 at 02:31:36PM +0100, Niklas Söderlund wrote:
> Documentation for Renesas R-Car MIPI CSI-2 receiver. The CSI-2 receivers
> are located between the video sources (CSI-2 transmitters) and the video
> grabbers (VIN) on Gen3 of Renesas R-Car SoC.
> 
> Each CSI-2 device is connected to more then one VIN device which
> simultaneously can receive video from the same CSI-2 device. Each VIN
> device can also be connected to more then one CSI-2 device. The routing
> of which link are used are controlled by the VIN devices. There are only
> a few possible routes which are set by hardware limitations, which are
> different for each SoC in the Gen3 family.
> 
> To work with the limitations of routing possibilities it is necessary
> for the DT bindings to describe which VIN device is connected to which
> CSI-2 device. This is why port 1 needs to to assign reg numbers for each
> VIN device that be connected to it. To setup and to know which links are
> valid for each SoC is the responsibility of the VIN driver since the
> register to configure it belongs to the VIN hardware.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  .../devicetree/bindings/media/rcar-csi2.txt        | 104 +++++++++++++++++++++
>  MAINTAINERS                                        |   1 +
>  2 files changed, 105 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/rcar-csi2.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar-csi2.txt b/Documentation/devicetree/bindings/media/rcar-csi2.txt
> new file mode 100644
> index 0000000000000000..24705d8997b14a10
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/rcar-csi2.txt

How about using vendor prefix and comma for the file name? Most others in
the directory do so, including other Renesas bindings.

> @@ -0,0 +1,104 @@
> +Renesas R-Car MIPI CSI-2
> +------------------------
> +
> +The rcar-csi2 device provides MIPI CSI-2 capabilities for the Renesas R-Car
> +family of devices. It is to be used in conjunction with the R-Car VIN module,
> +which provides the video capture capabilities.
> +
> +Mandatory properties
> +--------------------
> + - compatible: Must be one or more of the following
> +   - "renesas,r8a7795-csi2" for the R8A7795 device.
> +   - "renesas,r8a7796-csi2" for the R8A7796 device.
> +
> + - reg: the register base and size for the device registers
> + - interrupts: the interrupt for the device
> + - clocks: Reference to the parent clock
> +
> +The device node shall contain two 'port' child nodes according to the
> +bindings defined in Documentation/devicetree/bindings/media/
> +video-interfaces.txt. Port 0 shall connect the node that is the video
> +source for to the CSI-2. Port 1 shall connect all the R-Car VIN
> +modules, which can make use of the CSI-2 module.
> +
> +- Port 0 - Video source (Mandatory)
> +	- Endpoint 0 - sub-node describing the endpoint that is the video source
> +
> +- Port 1 - VIN instances (Mandatory for all VIN present in the SoC)
> +	- Endpoint 0 - sub-node describing the endpoint that is VIN0
> +	- Endpoint 1 - sub-node describing the endpoint that is VIN1
> +	- Endpoint 2 - sub-node describing the endpoint that is VIN2
> +	- Endpoint 3 - sub-node describing the endpoint that is VIN3
> +	- Endpoint 4 - sub-node describing the endpoint that is VIN4
> +	- Endpoint 5 - sub-node describing the endpoint that is VIN5
> +	- Endpoint 6 - sub-node describing the endpoint that is VIN6
> +	- Endpoint 7 - sub-node describing the endpoint that is VIN7
> +
> +Example:
> +
> +	csi20: csi2@fea80000 {
> +		compatible = "renesas,r8a7796-csi2", "renesas,rcar-gen3-csi2";
> +		reg = <0 0xfea80000 0 0x10000>;
> +		interrupts = <0 184 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&cpg CPG_MOD 714>;
> +		power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
> +		resets = <&cpg 714>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				reg = <0>;
> +
> +				csi20_in: endpoint@0 {
> +					clock-lanes = <0>;
> +					data-lanes = <1>;
> +					remote-endpoint = <&adv7482_txb>;
> +				};
> +			};
> +
> +			port@1 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				reg = <1>;
> +
> +				csi20vin0: endpoint@0 {
> +					reg = <0>;
> +					remote-endpoint = <&vin0csi20>;
> +				};
> +				csi20vin1: endpoint@1 {
> +					reg = <1>;
> +					remote-endpoint = <&vin1csi20>;
> +				};
> +				csi20vin2: endpoint@2 {
> +					reg = <2>;
> +					remote-endpoint = <&vin2csi20>;
> +				};
> +				csi20vin3: endpoint@3 {
> +					reg = <3>;
> +					remote-endpoint = <&vin3csi20>;
> +				};
> +				csi20vin4: endpoint@4 {
> +					reg = <4>;
> +					remote-endpoint = <&vin4csi20>;
> +				};
> +				csi20vin5: endpoint@5 {
> +					reg = <5>;
> +					remote-endpoint = <&vin5csi20>;
> +				};
> +				csi20vin6: endpoint@6 {
> +					reg = <6>;
> +					remote-endpoint = <&vin6csi20>;
> +				};
> +				csi20vin7: endpoint@7 {
> +					reg = <7>;
> +					remote-endpoint = <&vin7csi20>;
> +				};
> +			};
> +		};
> +	};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index adbf69306e9ee3d2..fa81ee6e80274646 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8565,6 +8565,7 @@ L:	linux-media@vger.kernel.org
>  L:	linux-renesas-soc@vger.kernel.org
>  T:	git git://linuxtv.org/media_tree.git
>  S:	Supported
> +F:	Documentation/devicetree/bindings/media/rcar-csi2.txt
>  F:	Documentation/devicetree/bindings/media/rcar_vin.txt
>  F:	drivers/media/platform/rcar-vin/
>  

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

^ permalink raw reply

* Re: [PATCH] dt-bindings: serial: fsl-imx-uart: Remove 'fsl,irda-mode' property
From: Rob Herring @ 2017-11-10 21:59 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Fabio Estevam
In-Reply-To: <1510157005-5110-1-git-send-email-festevam-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Wed, Nov 08, 2017 at 02:03:25PM -0200, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
> 
> 'fsl,irda-mode' property has been removed since commit afe9cbb1a6ad
> ("serial: imx: drop support for IRDA"), so remove it from the binding
> document.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/serial/fsl-imx-uart.txt | 1 -
>  1 file changed, 1 deletion(-)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/5] dt-bindings: serial: Add common rs485 binding for RTS polarity
From: Rob Herring @ 2017-11-10 21:58 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Greg Kroah-Hartman, Jiri Slaby, Sascha Hauer, Uwe Kleine-Koenig,
	Mark Jackson, Michal Oleszczyk,
	linux-serial-u79uwXL29TY76Z2rM5mHXA, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <60d6ef2cf6c599d2d8717250fe52a0ccb1272be8.1510134353.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>

On Wed, Nov 08, 2017 at 11:32:34AM +0100, Lukas Wunner wrote:
> rs485 allows for robust half-duplex serial communication.  It is often
> implemented by attaching an rs485 transceiver to a UART.  The UART's
> RTS line is wired to the transceiver's Transmit Enable pin and
> determines whether the transceiver is sending or receiving.
> 
> Examples for such transceivers are Maxim MAX13451E and TI SN65HVD1781A:
> https://datasheets.maximintegrated.com/en/ds/MAX13450E-MAX13451E.pdf
> http://www.ti.com/lit/ds/symlink/sn65hvd1781a-q1.pdf
> 
> In the devicetree, the transceiver itself is not represented, only the
> UART is.  A few rs485-specific dt-bindings already exist and these go
> into the UART's device node.

That often ends up being a mistake. We don't describe things, then add a 
few properties and then finally realize we need a full representation. 
Connectors are a good example.

> This commit adds a binding to set the RTS polarity.  Most (if not all)
> transceivers require the Transmit Enable pin be driven high for sending,
> but in some cases boards may negate the pin and RTS must then be driven
> low.  Consequently the polarity defaults to active high but can be
> inverted with the newly added "rs485-rts-active-low" binding.
> 
> Document this binding in rs485.txt and in the two drivers fsl-imx-uart
> and fsl-lpuart that are about to be amended with support for it.
> 
> Curiously, the omap_serial driver defaults to active low and already
> supports an "rs485-rts-active-high" binding to invert the polarity.
> This is left unchanged to retain compatibility, but the binding is
> herewith documented.
> 
> Cc: Mark Jackson <mpfj-2FZW7xY0fHgqdlJmJB21zg@public.gmane.org>
> Cc: Michał Oleszczyk <oleszczyk.m-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/serial/fsl-imx-uart.txt | 3 ++-
>  Documentation/devicetree/bindings/serial/fsl-lpuart.txt   | 3 ++-
>  Documentation/devicetree/bindings/serial/omap_serial.txt  | 1 +
>  Documentation/devicetree/bindings/serial/rs485.txt        | 1 +
>  4 files changed, 6 insertions(+), 2 deletions(-)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] clk: qoriq: add more divider clocks support
From: Rob Herring @ 2017-11-10 21:45 UTC (permalink / raw)
  To: andy.tang
  Cc: mturquette, sboyd, mark.rutland, linux-clk, devicetree,
	linux-kernel
In-Reply-To: <20171108082615.46769-1-andy.tang@nxp.com>

On Wed, Nov 08, 2017 at 04:26:15PM +0800, andy.tang@nxp.com wrote:
> From: Yuantian Tang <andy.tang@nxp.com>
> 
> More divider clocks are needed by IP. So enlarge the PLL divider
> array to accommodate more divider clocks.
> 
> Signed-off-by: Tang Yuantian <andy.tang@nxp.com>
> ---
>  Documentation/devicetree/bindings/clock/qoriq-clock.txt | 1 +
>  drivers/clk/clk-qoriq.c                                 | 9 ++++++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v2 1/3] dt-bindings: phy: Add Cygnus usb phy binding
From: Rob Herring @ 2017-11-10 21:44 UTC (permalink / raw)
  To: Raveendra Padasalagi
  Cc: Mark Rutland, Kishon Vijay Abraham I, Russell King, Scott Branden,
	Ray Jui, Srinath Mannam, Vikram Prakash, Jon Mason,
	Florian Fainelli, Yoshihiro Shimoda, Raviteja Garimella,
	Rafal Milecki, Arnd Bergmann, Viresh Kumar, Jaehoon Chung,
	devicetree, linux-arm-kernel, linux-kernel,
	bcm-kernel-feedback-list
In-Reply-To: <1510127203-5815-2-git-send-email-raveendra.padasalagi@broadcom.com>

On Wed, Nov 08, 2017 at 01:16:41PM +0530, Raveendra Padasalagi wrote:
> Add devicetree binding document for broadcom's
> Cygnus SoC specific usb phy controller driver.
> 
> Signed-off-by: Raveendra Padasalagi <raveendra.padasalagi@broadcom.com>
> ---
>  .../bindings/phy/brcm,cygnus-usb-phy.txt           | 106 +++++++++++++++++++++
>  1 file changed, 106 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt b/Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt
> new file mode 100644
> index 0000000..bbc4b94
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt
> @@ -0,0 +1,106 @@
> +BROADCOM CYGNUS USB PHY
> +
> +Required Properties:
> +- compatible:  brcm,cygnus-usb-phy
> +- reg : the register start address and length for
> +        crmu_usbphy_aon_ctrl,
> +        cdru usb phy control,
> +        usb host idm registers,
> +        usb device idm registers.
> +- reg-names: a list of the names corresponding to the previous register ranges
> +  Should contain
> +        "crmu-usbphy-aon-ctrl",
> +        "cdru-usbphy",
> +        "usb2h-idm",
> +        "usb2d-idm".
> +- address-cells: should be 1
> +- size-cells: should be 0
> +
> +Sub-nodes:
> +  Each port's PHY should be represented as a sub-node.
> +
> +Sub-nodes required properties:
> +- reg: the PHY number
> +- #phy-cells must be 1
> +  The node that uses the phy must provide 1 integer argument specifying
> +  port number.
> +
> +Optional Properties:
> +- vbus-p#-supply : The regulator for vbus out control for the host

Is this a literal # or something else?

> +  functionality enabled ports.
> +- vbus-gpios: vbus gpio binding
> +  This is mandatory for port 2, as port 2 is used as dual role phy.
> +  Based on the vbus and id values device or host role is determined
> +  for phy 2.

These optional properties don't match with the example.

> +
> +- extcon: extcon phandle
> +  This is mandatory for port 2,  as port 2 is used as dual role phy.
> +  extcon should be phandle to external usb gpio module which provide
> +  device or host role notifications based on the ID and VBUS gpio's state.
> +
> +
> +Refer to phy/phy-bindings.txt for the generic PHY binding properties
> +
> +NOTE: port 0 and port 1 are host only and port 2 is dual role port.
> +
> +Example of phy :
> +	usbphy: usb-phy@0301c028 {
> +		compatible = "brcm,cygnus-usb-phy";
> +		reg = <0x0301c028 0x4>,
> +		      <0x0301d1b4 0x5c>,
> +		      <0x18115000 0xa00>,
> +		      <0x18111000 0xa00>;
> +		reg-names = "crmu-usbphy-aon-ctrl", "cdru-usbphy",
> +			    "usb2h-idm", "usb2d-idm";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		usbphy0: usb-phy@0 {
> +			reg = <0>;
> +			#phy-cells = <1>;
> +		};
> +
> +		usbphy1: usb-phy@1 {
> +			reg = <1>;
> +			#phy-cells = <1>;
> +		};
> +
> +		usbphy2: usb-phy@2 {
> +			reg = <2>;
> +			#phy-cells = <1>;
> +			extcon = <&extcon_usb>;
> +		};
> +	};
> +
> +	extcon_usb: extcon_usb {
> +		compatible = "linux,extcon-usb-gpio";
> +		vbus-gpio = <&gpio_asiu 121 0>;
> +		id-gpio = <&gpio_asiu 122 0>;
> +		status = "okay";
> +	};
> +
> +
> +Example of node using the phy:
> +
> +	/* This nodes declares all three ports, port 0
> +	and port 1 are host and port 2 is device or host */
> +
> +	ehci0: usb@18048000 {
> +		compatible = "generic-ehci";
> +		reg = <0x18048000 0x100>;
> +		interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
> +		phys = <&usbphy0 0 &usbphy1 1 &usbphy2 2>;
> +		phy-names = "usbp0","usbp1","usbp2";
> +		status = "okay";

Don't show status in examples.

> +	};
> +
> +	/* This node declares port 2 phy
> +	and configures it for device */
> +
> +	usbd_udc_dwc1: usb@1804c000 {
> +		compatible = "iproc-udc";
> +		reg = <0x1804c000 0x2000>;
> +		interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
> +		phys = <&usbphy2 2>;
> +		phy-names = "usbdrd";
> +	};
> -- 
> 1.9.1
> 

^ permalink raw reply

* Re: [PATCH v1 1/2] dt-bindings: Add binding for Ilitek ILI9225 display panels
From: Rob Herring @ 2017-11-10 21:31 UTC (permalink / raw)
  To: David Lechner
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Mark Rutland,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1510113136-6788-2-git-send-email-david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>

On Tue, Nov 07, 2017 at 09:52:15PM -0600, David Lechner wrote:
> This adds a new binding for display panels that use an Ilitek ILI9225
> controller.
> 
> The "generic,2.2in-176x220-ili9225-tft" device listed has no identification
> markings whatsoever and an hour of googling turned up nothing, hence the use
> of "generic".

Just use ilitek then. I would do "ilitek,ili9225-2.2in-176x220" and 
hopefully there aren't others that size. I'm guessing you would have 
found them if so.

Otherwise, looks fine.

> An example of this nameless device can be found at:
> https://github.com/Nkawu/TFT_22_ILI9225
> 
> Signed-off-by: David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
> ---
>  .../devicetree/bindings/display/ilitek,ili9225.txt | 25 ++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9225.txt
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Applied "ASoC: add mclk-fs to audio graph card binding" to the asoc tree
From: Mark Brown @ 2017-11-10 21:29 UTC (permalink / raw)
  Cc: Kuninori Morimoto, Mark Brown, lgirdwood
In-Reply-To: <1510236478-2351-2-git-send-email-olivier.moysan@st.com>

The patch

   ASoC: add mclk-fs to audio graph card binding

has been applied to the asoc tree at

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

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

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

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

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

Thanks,
Mark

>From b8b0d10c3a9b6ee94d56d0c3b162d05c177fb101 Mon Sep 17 00:00:00 2001
From: Olivier Moysan <olivier.moysan@st.com>
Date: Thu, 9 Nov 2017 15:07:57 +0100
Subject: [PATCH] ASoC: add mclk-fs to audio graph card binding

Add mclk-fs support to audio graph card
as initially supported in simple card.

Signed-off-by: Olivier Moysan <olivier.moysan@st.com>
Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 Documentation/devicetree/bindings/sound/audio-graph-card.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/sound/audio-graph-card.txt b/Documentation/devicetree/bindings/sound/audio-graph-card.txt
index 6e6720aa33f1..d04ea3b1a1dd 100644
--- a/Documentation/devicetree/bindings/sound/audio-graph-card.txt
+++ b/Documentation/devicetree/bindings/sound/audio-graph-card.txt
@@ -17,6 +17,7 @@ Below are same as Simple-Card.
 - bitclock-master
 - bitclock-inversion
 - frame-inversion
+- mclk-fs
 - dai-tdm-slot-num
 - dai-tdm-slot-width
 - clocks / system-clock-frequency
-- 
2.15.0

^ permalink raw reply related

* Applied "ASoC: add mclk-fs support to audio graph card" to the asoc tree
From: Mark Brown @ 2017-11-10 21:29 UTC (permalink / raw)
  Cc: Kuninori Morimoto, Mark Brown, lgirdwood
In-Reply-To: <1510236478-2351-3-git-send-email-olivier.moysan@st.com>

The patch

   ASoC: add mclk-fs support to audio graph card

has been applied to the asoc tree at

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

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

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

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

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

Thanks,
Mark

>From 757652dd597139adb09e196a93db93feb6fb8a64 Mon Sep 17 00:00:00 2001
From: Olivier Moysan <olivier.moysan@st.com>
Date: Thu, 9 Nov 2017 15:07:58 +0100
Subject: [PATCH] ASoC: add mclk-fs support to audio graph card

Add mclk-fs support to audio graph card
as it was previously implemented in simple card.

Signed-off-by: Olivier Moysan <olivier.moysan@st.com>
Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/generic/audio-graph-card.c | 47 ++++++++++++++++++++++++++++++------
 1 file changed, 39 insertions(+), 8 deletions(-)

diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
index 488c52f9405f..1b6164249341 100644
--- a/sound/soc/generic/audio-graph-card.c
+++ b/sound/soc/generic/audio-graph-card.c
@@ -29,7 +29,9 @@ struct graph_card_data {
 	struct graph_dai_props {
 		struct asoc_simple_dai cpu_dai;
 		struct asoc_simple_dai codec_dai;
+		unsigned int mclk_fs;
 	} *dai_props;
+	unsigned int mclk_fs;
 	struct snd_soc_dai_link *dai_link;
 	struct gpio_desc *pa_gpio;
 };
@@ -95,9 +97,43 @@ static void asoc_graph_card_shutdown(struct snd_pcm_substream *substream)
 	asoc_simple_card_clk_disable(&dai_props->codec_dai);
 }
 
+static int asoc_graph_card_hw_params(struct snd_pcm_substream *substream,
+				     struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	struct graph_card_data *priv = snd_soc_card_get_drvdata(rtd->card);
+	struct graph_dai_props *dai_props = graph_priv_to_props(priv, rtd->num);
+	unsigned int mclk, mclk_fs = 0;
+	int ret = 0;
+
+	if (priv->mclk_fs)
+		mclk_fs = priv->mclk_fs;
+	else if (dai_props->mclk_fs)
+		mclk_fs = dai_props->mclk_fs;
+
+	if (mclk_fs) {
+		mclk = params_rate(params) * mclk_fs;
+		ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk,
+					     SND_SOC_CLOCK_IN);
+		if (ret && ret != -ENOTSUPP)
+			goto err;
+
+		ret = snd_soc_dai_set_sysclk(cpu_dai, 0, mclk,
+					     SND_SOC_CLOCK_OUT);
+		if (ret && ret != -ENOTSUPP)
+			goto err;
+	}
+	return 0;
+err:
+	return ret;
+}
+
 static const struct snd_soc_ops asoc_graph_card_ops = {
 	.startup = asoc_graph_card_startup,
 	.shutdown = asoc_graph_card_shutdown,
+	.hw_params = asoc_graph_card_hw_params,
 };
 
 static int asoc_graph_card_dai_init(struct snd_soc_pcm_runtime *rtd)
@@ -146,10 +182,7 @@ static int asoc_graph_card_dai_link_of(struct device_node *cpu_port,
 	if (ret < 0)
 		goto dai_link_of_err;
 
-	/*
-	 * we need to consider "mclk-fs" around here
-	 * see simple-card
-	 */
+	of_property_read_u32(rcpu_ep, "mclk-fs", &dai_props->mclk_fs);
 
 	ret = asoc_simple_card_parse_graph_cpu(cpu_ep, dai_link);
 	if (ret < 0)
@@ -217,10 +250,8 @@ static int asoc_graph_card_parse_of(struct graph_card_data *priv)
 	if (ret < 0)
 		return ret;
 
-	/*
-	 * we need to consider "mclk-fs" around here
-	 * see simple-card
-	 */
+	/* Factor to mclk, used in hw_params() */
+	of_property_read_u32(node, "mclk-fs", &priv->mclk_fs);
 
 	of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
 		ret = asoc_graph_card_dai_link_of(it.node, priv, idx++);
-- 
2.15.0

^ permalink raw reply related

* Re: [PATCH 2/3] dt-bindings: iio: add Intersil isl76683 light sensor bindings
From: Rob Herring @ 2017-11-10 21:26 UTC (permalink / raw)
  To: Christoph Fritz
  Cc: Jonathan Cameron, Peter Meerwald-Stadler,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1510068983-25769-3-git-send-email-chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

On Tue, Nov 07, 2017 at 04:36:22PM +0100, Christoph Fritz wrote:
> This patch adds documentation of device tree bindings for Intersil
> isl76683 light sensor.
> 
> Signed-off-by: Christoph Fritz <chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  .../devicetree/bindings/iio/light/isl76683.txt     | 26 ++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/light/isl76683.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/isl76683.txt b/Documentation/devicetree/bindings/iio/light/isl76683.txt
> new file mode 100644
> index 0000000..657f17d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/isl76683.txt
> @@ -0,0 +1,26 @@
> +* ISL76683 ambient light sensor
> +
> +http://www.intersil.com/content/dam/Intersil/documents/isl7/isl76683.pdf
> +
> +Required properties:
> +
> +  - compatible: must be "isil,isl76683"
> +  - reg: the I2C address of the sensor
> +  - interrupt-parent: should be the phandle for the interrupt controller
> +  - interrupts: interrupt mapping for GPIO IRQ
> +
> +Optional properties:
> +
> +  - isil,external-resistor: integer in kOhm of external resistor R_ext.
> +                            Valid values are from 1 to 1000.
> +                            If not supplied, 100 kOhm will be assumed.

Add an unit suffix as defined in property-units.txt.

> +
> +Example:
> +
> +isl76683@44 {
> +	compatible = "isil,isl76683";
> +	reg = <0x44>;
> +	interrupt-parent = <&gpio1>;
> +	interrupts = <20 IRQ_TYPE_LEVEL_LOW>;
> +	isil,external-resistor = <50>;
> +};
> -- 
> 2.1.4
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2] display: panel: Add Tianma tm070rvhg71 display support (800x480)
From: Rob Herring @ 2017-11-10 21:24 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Mark Rutland, Thierry Reding, David Airlie,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171107153058.5554-1-lukma-ynQEQJNshbs@public.gmane.org>

On Tue, Nov 07, 2017 at 04:30:58PM +0100, Lukasz Majewski wrote:
> Signed-off-by: Lukasz Majewski <lukma-ynQEQJNshbs@public.gmane.org>
> 
> ---
> Changes for v2:
> - Provide more detailed ./Documentation/devicetree/bindings/display/panel
>   entry to describe this panel device.
> ---
>  .../bindings/display/panel/tianma,tm070rvhg71.txt  | 29 ++++++++++++++++++++++
>  drivers/gpu/drm/panel/panel-simple.c               | 27 ++++++++++++++++++++
>  2 files changed, 56 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/tianma,tm070rvhg71.txt

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/8] dt-bindings: can: rcar_can: document r8a774[35] can support
From: Rob Herring @ 2017-11-10 21:23 UTC (permalink / raw)
  To: Fabrizio Castro
  Cc: Wolfgang Grandegger, Marc Kleine-Budde, Mark Rutland,
	Simon Horman, Geert Uytterhoeven, linux-can, netdev, devicetree,
	linux-renesas-soc, Chris Paterson, Biju Das
In-Reply-To: <1510067449-17017-2-git-send-email-fabrizio.castro@bp.renesas.com>

On Tue, Nov 07, 2017 at 03:10:42PM +0000, Fabrizio Castro wrote:
> Document "renesas,can-r8a7743" and "renesas,can-r8a7745" compatible
> strings. Since the fallback compatible string ("renesas,rcar-gen2-can")
> activates the right code in the driver, no driver change is needed.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das@bp.renesas.com>
> ---
>  Documentation/devicetree/bindings/net/can/rcar_can.txt | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH 2/2] ASoC: codecs: Add initial PCM1862/63/64/65 universal ADC driver
From: Mark Brown @ 2017-11-10 21:21 UTC (permalink / raw)
  To: Andrew F. Davis
  Cc: Liam Girdwood, Rob Herring, Mark Rutland,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171110195711.24210-2-afd-l0cyMroinI0@public.gmane.org>

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

On Fri, Nov 10, 2017 at 01:57:11PM -0600, Andrew F. Davis wrote:

This looks mostly good, a few small issues though:

> +	dev_info(&i2c->dev, "%s() i2c->addr=%d\n", __func__, i2c->addr);

No need for this as standard, we already have I2C level logging
facilities if they're really needed.  It's OK to do things like log chip
revisions read back from hardware but this is pure software.

> +static const struct snd_kcontrol_new pcm1863_snd_controls[] = {
> +	SOC_DOUBLE_R_S_TLV("Analog Gain", PCM186X_PGA_VAL_CH1_L,

Volume controls should end in Volume to tell userspace how to handle
them, see ControlNames.txt.

> +	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(priv->supplies),
> +				      priv->supplies);
> +	if (ret != 0) {
> +		dev_err(dev, "failed to request supplies: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Reset device registers for a consistent power-on like state */
> +	ret = regmap_write(regmap, PCM186X_PAGE, PCM186X_RESET);
> +	if (ret != 0) {
> +		dev_err(dev, "failed to write device: %d\n", ret);
> +		return ret;
> +	}

We didn't enable the supplies for the device before we reset it, if the
device was powered off then the write will hopefully not work and we'll
get a spurious error here.  The driver should bounce the supplies on at
least long enough to do the reset, it's not ideal but it's robust.

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

^ permalink raw reply

* Re: [PATCH v4 3/4] dt-bindings: iio: adc: update the doc for SAR ADC
From: Rob Herring @ 2017-11-10 21:21 UTC (permalink / raw)
  To: Yixun Lan
  Cc: Jonathan Cameron, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, Kevin Hilman, Neil Armstrong,
	Jerome Brunet, Martin Blumenstingl, Carlo Caione, Xingyu Chen,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171107141020.30419-1-yixun.lan-LpR1jeaWuhtBDgjK7y7TUQ@public.gmane.org>

On Tue, Nov 07, 2017 at 10:10:20PM +0800, Yixun Lan wrote:
> From: Xingyu Chen <xingyu.chen-LpR1jeaWuhtBDgjK7y7TUQ@public.gmane.org>
> 
> Update the doc as the SAR ADC modules doesn't require "sana" clock.
> 
> Singed-off-by: Xingyu Chen <xingyu.chen-LpR1jeaWuhtBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Yixun Lan <yixun.lan-LpR1jeaWuhtBDgjK7y7TUQ@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt | 1 -
>  1 file changed, 1 deletion(-)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

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

^ permalink raw reply

* RE: [PATCHv4 1/3] ARMv8: dts: ls1046a: add the property of IB and OB
From: Leo Li @ 2017-11-10 21:20 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Xiaowei Bao, robh+dt@kernel.org,
	mark.rutland@arm.com, catalin.marinas@arm.com,
	will.deacon@arm.com, bhelgaas@google.com, shawnguo@kernel.org,
	Madalin-cristian Bucur, Sumit Garg, Y.b. Lu, Andy Tang,
	jingoohan1@gmail.com, pbrobinson@gmail.com,
	songxiaowei@hisilicon.com, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20b8f1d2-f817-469f-afbf-66f25d6f8c46@ti.com>



> -----Original Message-----
> From: Kishon Vijay Abraham I [mailto:kishon@ti.com]
> Sent: Friday, November 10, 2017 12:22 AM
> To: Xiaowei Bao <xiaowei.bao@nxp.com>; robh+dt@kernel.org;
> mark.rutland@arm.com; catalin.marinas@arm.com; will.deacon@arm.com;
> bhelgaas@google.com; shawnguo@kernel.org; Madalin-cristian Bucur
> <madalin.bucur@nxp.com>; Sumit Garg <sumit.garg@nxp.com>; Y.b. Lu
> <yangbo.lu@nxp.com>; hongtao.jia@nxp.com; Andy Tang
> <andy.tang@nxp.com>; Leo Li <leoyang.li@nxp.com>; jingoohan1@gmail.com;
> pbrobinson@gmail.com; songxiaowei@hisilicon.com;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; linux-pci@vger.kernel.org; linuxppc-
> dev@lists.ozlabs.org; Z.q. Hou <zhiqiang.hou@nxp.com>; Mingkai Hu
> <mingkai.hu@nxp.com>; M.h. Lian <minghuan.lian@nxp.com>
> Subject: Re: [PATCHv4 1/3] ARMv8: dts: ls1046a: add the property of IB and OB
> 
> Hi Bao,
> 
> On Friday 10 November 2017 09:18 AM, Bao Xiaowei wrote:
> > Add the property of inbound and outbound windows number for ep driver.
> >
> > Signed-off-by: Bao Xiaowei <xiaowei.bao@nxp.com>
> > Acked-by: Minghuan Lian <minghuan.Lian@nxp.com>
> > ---
> >  v2:
> >  - no change
> >  v3:
> >  - modify the commit message
> >  v4:
> >  - no change
> >
> >  arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 6 ++++++
> >  1 file changed, 6 insertions(+)
> 
> $subject should start with something like
> arm64: dts: ls1046a: **
> >
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> > b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> > index 06b5e12d04d8..f8332669663c 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> > @@ -674,6 +674,8 @@
> >  			device_type = "pci";
> >  			dma-coherent;
> >  			num-lanes = <4>;
> > +			num-ib-windows = <6>;
> > +			num-ob-windows = <6>;
> 
> EP specific properties shouldn't be added in RC dt node. Ideally you should have
> a separate dt node for RC and EP.

It is a single PCIe controller which can be configured to either RC mode or EP mode.  Wouldn't it conflict with the device tree principles to have two device tree nodes for the same PCIe controller?  And obviously the two modes cannot be used at the same time so we cannot have two drivers both probe on the same hardware.

Regards,
Leo

^ permalink raw reply

* Re: [PATCH v2 1/3] drivers: usb: phy: Add qoriq usb 3.0 phy driver support
From: Rob Herring @ 2017-11-10 21:19 UTC (permalink / raw)
  To: Ran Wang
  Cc: linux-kernel, linux-usb, devicetree, linux-arm-kernel,
	Felipe Balbi, mark.rutland, kishon, catalin.marinas, will.deacon,
	stern, pku.leo, mathias.nyman, gregkh, suresh.gupta, felipe.balbi,
	Shawn Guo, Sriram Dash, Li Yang, Hongtao Jia
In-Reply-To: <20171107072055.26945-1-ran.wang_1@nxp.com>

On Tue, Nov 07, 2017 at 03:20:53PM +0800, Ran Wang wrote:
> Adds qoriq usb 3.0 phy driver to implement erratum related workaround
> for qoriq SoC.
> 
> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
> Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> ---
> Change in v2:
> 	- Replace funciont __raw_writel() by iowrite32be()
> 	- Remove qoriq_usb3_phy_read() and qoriq_usb3_phy_write()
> 	- Remove USB3PRM1CR_VAL define, use numbers directly
> 	- Use C-style numeric constants on 32'h27672B2A in annotation
> 	- Add prefix 'static' to struct qoriq_usb3_phy_erratum
> 
>  .../devicetree/bindings/phy/phy-qoriq-usb3.txt     |  36 ++++

Please split bindings to separate patch.

>  drivers/phy/Kconfig                                |   9 +
>  drivers/phy/Makefile                               |   1 +
>  drivers/phy/phy-qoriq-usb3.c                       | 191 +++++++++++++++++++++
>  4 files changed, 237 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt
>  create mode 100644 drivers/phy/phy-qoriq-usb3.c
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt b/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt
> new file mode 100644
> index 000000000000..d956f9c89fbf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt
> @@ -0,0 +1,36 @@
> +Driver for Freescale USB 3.0 PHY
> +
> +Required properties:
> +
> +- compatible :	fsl,qoriq-usb3-phy
> +- reg :		register mappings for Parameter Configuration Register
> +		and Phy base offset.
> +- reg-names :	"param_ctrl" and "phy_base"
> +- phy_type :	For multi port host USB controllers, should be one of
> +		"ulpi", or "serial". For dual role USB controllers,
> +		should be one of "ulpi", "utmi", "utmi_wide", or "serial".
> +
> +Example:
> +		usbphy0: usb-phy@084F0000 {

Drop leading 0s and use lower case hex. Building dts with W=2 will tell 
you this.

> +				compatible = "fsl,qoriq-usb3-phy";
> +                reg = <0x0 0x01570070 0x0 0xC>, <0x0 0x084F0000 0x0 0x5000>;
> +                reg-names = "param_ctrl", "phy_base";
> +                #phy-cells = <0>;
> +                phy_type = "utmi";
> +		};
> +
> +        usbphy1: usb-phy@08500000 {
> +                compatible = "fsl,qoriq-usb3-phy";
> +                reg = <0x0 0x0157007C 0x0 0xC>, <0x0 0x08500000 0x0 0x5000>;
> +                reg-names = "param_ctrl", "phy_base";
> +                #phy-cells = <0>;
> +                phy_type = "utmi";
> +        };
> +
> +        usbphy2: usb-phy@08510000 {
> +                compatible = "fsl,qoriq-usb3-phy";
> +                reg = <0x0 0x01570088 0x0 0xC>, <0x0 0x08510000 0x0 0x5000>;
> +                reg-names = "param_ctrl", "phy_base";
> +                #phy-cells = <0>;
> +                phy_type = "utmi";
> +        };

^ permalink raw reply

* [PATCH] ARM: dts: imx6: Remove unneeded address-cells/size-cells
From: Fabio Estevam @ 2017-11-10 21:12 UTC (permalink / raw)
  To: shawnguo-DgEjT+Ai2ygdnm+yROfE0A
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Fabio Estevam

From: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>

According to Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt:

"- #address-cells, #size-cells : Must be present if the device has
sub-nodes representing partitions."

Remove the occurrences of #address-cells/size-cells where no sub-nodes
representing partitions are present.

Signed-off-by: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
---
 arch/arm/boot/dts/imx6q-cm-fx6.dts           | 2 --
 arch/arm/boot/dts/imx6q-mccmon6.dts          | 2 --
 arch/arm/boot/dts/imx6qdl-aristainetos.dtsi  | 2 --
 arch/arm/boot/dts/imx6qdl-aristainetos2.dtsi | 2 --
 arch/arm/boot/dts/imx6qdl-sabreauto.dtsi     | 2 --
 arch/arm/boot/dts/imx6qdl-sabresd.dtsi       | 2 --
 arch/arm/boot/dts/imx6sl-evk.dts             | 2 --
 arch/arm/boot/dts/imx6sx-sdb-reva.dts        | 4 ----
 arch/arm/boot/dts/imx6sx-sdb.dts             | 4 ----
 9 files changed, 22 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q-cm-fx6.dts b/arch/arm/boot/dts/imx6q-cm-fx6.dts
index bc7587c..5df84e2 100644
--- a/arch/arm/boot/dts/imx6q-cm-fx6.dts
+++ b/arch/arm/boot/dts/imx6q-cm-fx6.dts
@@ -194,8 +194,6 @@
 	status = "okay";
 
 	m25p80@0 {
-		#address-cells = <1>;
-		#size-cells = <1>;
 		compatible = "st,m25p", "jedec,spi-nor";
 		spi-max-frequency = <20000000>;
 		reg = <0>;
diff --git a/arch/arm/boot/dts/imx6q-mccmon6.dts b/arch/arm/boot/dts/imx6q-mccmon6.dts
index cab36f4..9636e15 100644
--- a/arch/arm/boot/dts/imx6q-mccmon6.dts
+++ b/arch/arm/boot/dts/imx6q-mccmon6.dts
@@ -90,8 +90,6 @@
 	status = "okay";
 
 	s25sl032p: flash@0 {
-		#address-cells = <1>;
-		#size-cells = <1>;
 		compatible = "jedec,spi-nor";
 		spi-max-frequency = <40000000>;
 		reg = <0>;
diff --git a/arch/arm/boot/dts/imx6qdl-aristainetos.dtsi b/arch/arm/boot/dts/imx6qdl-aristainetos.dtsi
index 7d64075..d8f578c 100644
--- a/arch/arm/boot/dts/imx6qdl-aristainetos.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-aristainetos.dtsi
@@ -106,8 +106,6 @@
 	status = "okay";
 
 	flash: m25p80@0 {
-		#address-cells = <1>;
-		#size-cells = <1>;
 		compatible = "micron,n25q128a11", "jedec,spi-nor";
 		spi-max-frequency = <20000000>;
 		reg = <0>;
diff --git a/arch/arm/boot/dts/imx6qdl-aristainetos2.dtsi b/arch/arm/boot/dts/imx6qdl-aristainetos2.dtsi
index 1b18728..5875252 100644
--- a/arch/arm/boot/dts/imx6qdl-aristainetos2.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-aristainetos2.dtsi
@@ -136,8 +136,6 @@
 	status = "okay";
 
 	flash: m25p80@1 {
-		#address-cells = <1>;
-		#size-cells = <1>;
 		compatible = "micron,n25q128a11", "jedec,spi-nor";
 		spi-max-frequency = <20000000>;
 		reg = <1>;
diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index 4fa2fac..828a9a5 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
@@ -200,8 +200,6 @@
 	status = "disabled"; /* pin conflict with WEIM NOR */
 
 	flash: m25p80@0 {
-		#address-cells = <1>;
-		#size-cells = <1>;
 		compatible = "st,m25p32", "jedec,spi-nor";
 		spi-max-frequency = <20000000>;
 		reg = <0>;
diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index 0a50705..acb0292 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -196,8 +196,6 @@
 	status = "okay";
 
 	flash: m25p80@0 {
-		#address-cells = <1>;
-		#size-cells = <1>;
 		compatible = "st,m25p32", "jedec,spi-nor";
 		spi-max-frequency = <20000000>;
 		reg = <0>;
diff --git a/arch/arm/boot/dts/imx6sl-evk.dts b/arch/arm/boot/dts/imx6sl-evk.dts
index 60600b4..03a2240 100644
--- a/arch/arm/boot/dts/imx6sl-evk.dts
+++ b/arch/arm/boot/dts/imx6sl-evk.dts
@@ -123,8 +123,6 @@
 	status = "okay";
 
 	flash: m25p80@0 {
-		#address-cells = <1>;
-		#size-cells = <1>;
 		compatible = "st,m25p32", "jedec,spi-nor";
 		spi-max-frequency = <20000000>;
 		reg = <0>;
diff --git a/arch/arm/boot/dts/imx6sx-sdb-reva.dts b/arch/arm/boot/dts/imx6sx-sdb-reva.dts
index e3533e7..ebbed57 100644
--- a/arch/arm/boot/dts/imx6sx-sdb-reva.dts
+++ b/arch/arm/boot/dts/imx6sx-sdb-reva.dts
@@ -127,16 +127,12 @@
 
 	flash0: s25fl128s@0 {
 		reg = <0>;
-		#address-cells = <1>;
-		#size-cells = <1>;
 		compatible = "spansion,s25fl128s", "jedec,spi-nor";
 		spi-max-frequency = <66000000>;
 	};
 
 	flash1: s25fl128s@1 {
 		reg = <1>;
-		#address-cells = <1>;
-		#size-cells = <1>;
 		compatible = "spansion,s25fl128s", "jedec,spi-nor";
 		spi-max-frequency = <66000000>;
 	};
diff --git a/arch/arm/boot/dts/imx6sx-sdb.dts b/arch/arm/boot/dts/imx6sx-sdb.dts
index 6dd9beb..4b7481a 100644
--- a/arch/arm/boot/dts/imx6sx-sdb.dts
+++ b/arch/arm/boot/dts/imx6sx-sdb.dts
@@ -113,16 +113,12 @@
 	status = "okay";
 
 	flash0: n25q256a@0 {
-		#address-cells = <1>;
-		#size-cells = <1>;
 		compatible = "micron,n25q256a", "jedec,spi-nor";
 		spi-max-frequency = <29000000>;
 		reg = <0>;
 	};
 
 	flash1: n25q256a@1 {
-		#address-cells = <1>;
-		#size-cells = <1>;
 		compatible = "micron,n25q256a", "jedec,spi-nor";
 		spi-max-frequency = <29000000>;
 		reg = <1>;
-- 
2.7.4

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

^ permalink raw reply related

* Re: [PATCH V4 09/12] clk: sprd: Add dt-bindings include file for SC9860
From: Rob Herring @ 2017-11-10 21:11 UTC (permalink / raw)
  To: Chunyan Zhang
  Cc: Stephen Boyd, Michael Turquette, Mark Rutland, Catalin Marinas,
	Will Deacon, linux-clk, linux-kernel, devicetree,
	linux-arm-kernel, Arnd Bergmann, Mark Brown, Xiaolong Zhang,
	Ben Li, Orson Zhai, Chunyan Zhang
In-Reply-To: <20171110063607.3250-10-chunyan.zhang@spreadtrum.com>

On Fri, Nov 10, 2017 at 02:36:04PM +0800, Chunyan Zhang wrote:
> This file defines all SC9860 clock indexes, it should be included in the
> device tree in which there's device using the clocks.
> 
> Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
> ---
>  include/dt-bindings/clock/sprd,sc9860-clk.h | 408 ++++++++++++++++++++++++++++
>  1 file changed, 408 insertions(+)
>  create mode 100644 include/dt-bindings/clock/sprd,sc9860-clk.h

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH V4 02/12] dt-bindings: Add Spreadtrum clock binding documentation
From: Rob Herring @ 2017-11-10 21:11 UTC (permalink / raw)
  To: Chunyan Zhang
  Cc: Stephen Boyd, Michael Turquette, Mark Rutland, Catalin Marinas,
	Will Deacon, linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Arnd Bergmann,
	Mark Brown, Xiaolong Zhang, Ben Li, Orson Zhai, Chunyan Zhang
In-Reply-To: <20171110063607.3250-3-chunyan.zhang-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>

On Fri, Nov 10, 2017 at 02:35:57PM +0800, Chunyan Zhang wrote:
> Introduce a new binding with its documentation for Spreadtrum clock
> sub-framework.
> 
> Signed-off-by: Chunyan Zhang <chunyan.zhang-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/clock/sprd.txt | 63 ++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/sprd.txt

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH V3 09/11] clk: sprd: add clocks support for SC9860
From: Rob Herring @ 2017-11-10 21:10 UTC (permalink / raw)
  To: Chunyan Zhang
  Cc: Chunyan Zhang, Stephen Boyd, Michael Turquette, Mark Rutland,
	Catalin Marinas, Will Deacon, linux-clk,
	devicetree@vger.kernel.org, Arnd Bergmann, Mark Brown,
	Xiaolong Zhang, Ben Li, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, Orson Zhai
In-Reply-To: <CAAfSe-tadvpgJ0OC00Ccc+y+84t90dArfAu9ASOG77Hnw9Z=RA@mail.gmail.com>

On Thu, Nov 9, 2017 at 2:18 AM, Chunyan Zhang <zhang.lyra@gmail.com> wrote:
> Hi Rob and Stephen,
>
> On 7 November 2017 at 05:43, Rob Herring <robh@kernel.org> wrote:
>> On Thu, Nov 02, 2017 at 02:56:24PM +0800, Chunyan Zhang wrote:
>>> This patch added the list of clocks for Spreadtrum's SC9860 SoC,
>>> together with clock initialization code.
>>>
>>> Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
>>> ---
>>>  drivers/clk/sprd/Kconfig                    |   10 +
>>>  drivers/clk/sprd/Makefile                   |    3 +
>>>  drivers/clk/sprd/sc9860-clk.c               | 1987 +++++++++++++++++++++++++++
>>>  include/dt-bindings/clock/sprd,sc9860-clk.h |  408 ++++++
>>
>> This belongs in the binding doc patch.
>
> I noticed that this kind of files were submitted in driver patch
> before when adding a new driver, how about I just put it into a single
> patch in the next version?

It's been done all ways, but the correct way is as I told you. Binding
doc and header in one, driver in another patch.

Rob

^ 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