Re: [PATCH v16 0/2] rcar-csi2: add Renesas R-Car MIPI CSI-2

[Laurent Pinchart]

Hi Niklas,

On Tuesday, 15 May 2018 03:56:33 EEST Niklas Söderlund wrote:
> Hi,
> 
> This is the latest incarnation of R-Car MIPI CSI-2 receiver driver. It's
> based on top of the media-tree and are tested on Renesas Salvator-X and
> Salvator-XS together with adv7482 and the now in tree rcar-vin driver :-)
> 
> I hope this is the last incarnation of this patch-set, I do think it is
> ready for upstream consumption :-)

So do I. Even though you dropped Hans' reviewed-by tag due to changes in the 
hardware initialization code, I think the part that Hans cares about the most 
is the V4L2 API implementation, so I believe his review still applies. In my 
opinion the series has received the necessary review.

Hans, would you like to take this through your tree, or should we send a pull 
request directly to Mauro ? I'd like the two patches to be merged in v4.18 if 
possible.

> * Changes since v15
> - Merge struct phtw_mbps and struct phypll_hsfreqrange into a new struct
>   which maps a mpbs value to a register value, struct rcsi2_mbps_reg.
> - Reduced number of loops and delay when waiting for LP-11 and
>   confirmation of PHTW write as suggested by Laurent.
> - Dropped dev_dbg() printouts of the requested link speed.
> - Fix small issues in comments.
> - Remove unneeded () in for-loop condition in rcsi2_phtw_write_array().
> - Remove __refdata from declaration of 'static struct platform_driver
>   rcar_csi2_pdrv'.
> - Update MODULE_DESCRIPTION to 'Renesas R-Car MIPI CSI-2 receiver
>   driver'.
> - Fixed two erroneous values in hsfreqrange_h3_v3h_m3n[]. Thanks Jacopo
>   for spotting this!
> - Max link speed for V3M and E3 are 1.125Gbps remove settings above that
>   limit in phtw_mbps_v3m_e3. This also changed in datasheet v1.0.
> - Add review tags from Laurent and Maxime.
> 
> * Changes since v14.
> - Data sheet update changed init sequence for PHY forcing a restructure
>   of the driver. The restructure was so big I felt compel to drop all
>   review tags :-(
> - The change was that the Renesas H3 procedure was aligned with other
>   SoC in the Gen3 family procedure. I had kept the rework as separate
>   patches and was planing to post once original driver with H3 and M3-W
>   support where merged. As review tags are dropped I chosen to squash
>   those patches into 2/2.
> - Add support for Gen3 M3-N.
> - Add support for Gen3 V3M.
> - Set PHTC_TESTCLR when stopping the PHY.
> - Revert back to the v12 and earlier phypll calculation as it turns out
>   it was correct after all.
> - Added compatible string for R8A77965 and R8A77970.
> - s/Port 0/port@0/
> - s/Port 1/port@1/
> - s/Endpoint 0/endpoint@0/
> 
> * Changes since v13
> - Change return rcar_csi2_formats + i to return _csi2_formats[i].
> - Add define for PHCLM_STOPSTATECKL.
> - Update spelling in comments.
> - Update calculation in rcar_csi2_calc_phypll() according to
>   https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one
>   before v14 did not take into account that 2 bits per sample is
>   transmitted.
> - Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch
>   statement to set correct number of lanes to enable.
> - Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match
>   style of rest of file.
> - Switch to %u instead of 0x%x when printing bus type.
> - Switch to %u instead of %d for priv->lanes which is unsigned.
> - Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in
>   rcar_csi2_formats[].
> - Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16.
> - Set INTSTATE after PL-11 is confirmed to match flow chart in
>   datasheet.
> - Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs.
> - Add Maxime's and laurent's tags.
> 
> * Changes since v12
> - Fixed spelling mistakes in commit messages and documentation patch,
>   thanks Laurent.
> - Mark endpoints in port 1 as optional as it is allowed to not connect a
>   VIN to the CSI-2 and instead have it only use its parallel input
>   source (for those VIN that have one).
> - Added Ack from Sakari, thanks!
> - Media bus codes are u32 not unsigned int.
> - Ignore error check for v4l2_subdev_call(sd, video, s_stream, 0)
> - Do not set subdev host data as it's not used,
>   v4l2_set_subdev_hostdata().
> - Fixed spelling errors in commit message.
> - Add SPDX header
> - Rename badly named variable tmp to vcdt_part.
> - Cache subdevice in bound callback instead of looking it up in the
>   media graph. By doing this rcar_csi2_sd_info() can be removed.
> - Print unsigned using %u not %d.
> - Call pm_runtime_enable() before calling v4l2_async_register_subdev().
> - Dropped of_match_ptr() as OF is required.
> 
> * Changes since v11
> - Added missing call to v4l2_async_notifier_unregister().
> - Fixed missing reg popery in bindings documentation.
> - Add Rob's ack to 01/02.
> - Dropped 'media:' prefix from patch subjects as it seems they are added
>   first when a patch is picked up by the maintainer.
> - Fixed typo in comment enpoint 

RE: [PATCH/RFC v3 1/4] base: devcon: add a new API to find the graph

[Yoshihiro Shimoda]

Hi Heikki,

> From: Heikki Krogerus, Sent: Monday, May 14, 2018 10:28 PM
> 
> On Mon, May 14, 2018 at 06:15:57PM +0900, Yoshihiro Shimoda wrote:

> > diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
> > index d427e80..5a0da33 100644
> > --- a/drivers/base/devcon.c
> > +++ b/drivers/base/devcon.c

> > +static int generic_graph_match(struct device *dev, void *fwnode)
> > +{
> > +   return dev->fwnode == fwnode;
> > +}
> > +
> > +/**
> > + * device_connection_find_by_graph - Find two devices connected together
> > + * @dev: Device to find connected device
> > + * @port: identifier of the @dev port node
> > + * @endpoint: identifier of the @dev endpoint node
> > + *
> > + * Find a connection with @port and @endpoint by using graph between @dev 
> > and
> > + * another device. On success returns handle to the device that is 
> > connected
> > + * to @dev, with the reference count for the found device incremented. 
> > Returns
> > + * NULL if no matching connection was found, or ERR_PTR(-EPROBE_DEFER) when
> > + * a connection was found but the other device has not been enumerated yet.
> > + */
> > +struct device *device_connection_find_by_graph(struct device *dev, u32 
> > port,
> > +  u32 endpoint)
> > +{
> > +   struct bus_type *bus;
> > +   struct fwnode_handle *remote;
> > +   struct device *conn;
> > +
> > +   remote = fwnode_graph_get_remote_node(dev_fwnode(dev), port, endpoint);
> > +   if (!remote)
> > +   return NULL;
> > +
> > +   for (bus = generic_match_buses[0]; bus; bus++) {
> > +   conn = bus_find_device(bus, NULL, remote, generic_graph_match);
> > +   if (conn)
> > +   return conn;
> > +   }
> > +
> > +   /*
> > +* We only get called if a connection was found, tell the caller to
> > +* wait for the other device to show up.
> > +*/
> > +   return ERR_PTR(-EPROBE_DEFER);
> > +}
> > +EXPORT_SYMBOL_GPL(device_connection_find_by_graph);
> 
> Why do we need more API for walking through the graph?

I thought there is difficult to find if a device has multiple ports or 
endpoints.
So, I'd like to use port and endpoint number for finding a device.

> I'm not sure exactly sure what is going on here, I'll try to study
> your patches more when I have time, but the approach looks wrong. That
> function looks like a helper, but just not that useful one.
> 
> We really should be able to use the existing functions. In practice
> device_connection_find_match() should eventually parse the graph, then
> fallback to build-in connections if no graph is found. Otherwise
> parsing graph here is not really useful at all.

I think using device_connection_find_match() for finding the graph becomes 
complicated.
The current arguments of the function is the below:

void *device_connection_find_match(struct device *dev, const char *con_id,
   void *data,
   void *(*match)(struct device_connection *con,
  int ep, void *data))

If finding the graph, the following arguments will be not used.
 - con_id
 - *con and ep of "match" arguments.

This is because these arguments are for the build-in connections.
So, should I modify the arguments of the function for finding both
the graph and built-in connections somehow?

Best regards,
Yoshihiro Shimoda

> 
> Thanks,
> 
> --
> heikki
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v16 1/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver documentation

[Niklas Söderlund]

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 than one VIN device which
simultaneously can receive video from the same CSI-2 device. Each VIN
device can also be connected to more than one CSI-2 device. The routing
of which links are used is 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.

Signed-off-by: Niklas Söderlund 
Acked-by: Rob Herring 
Acked-by: Sakari Ailus 
Reviewed-by: Laurent Pinchart 
Reviewed-by: Jacopo Mondi 

---

* Changes since v14.
- Added compatible string for R8A77965 and R8A77970.
- s/Port 0/port@0/
- s/Port 1/port@1/
- s/Endpoint 0/endpoint@0/

* Changes since v13
- Add Laurent's tag.
---
 .../bindings/media/renesas,rcar-csi2.txt  | 101 ++
 MAINTAINERS   |   1 +
 2 files changed, 102 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 ..2d385b65b275bc58
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
@@ -0,0 +1,101 @@
+Renesas R-Car MIPI CSI-2
+
+
+The R-Car CSI-2 receiver device provides MIPI CSI-2 capabilities for the
+Renesas R-Car family of devices. It is 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.
+   - "renesas,r8a77965-csi2" for the R8A77965 device.
+   - "renesas,r8a77970-csi2" for the R8A77970 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 to the CSI-2 source. port@1
+shall connect to all the R-Car VIN modules that have a hardware
+connection to the CSI-2 receiver.
+
+- port@0- Video source (mandatory)
+   - endpoint@0 - sub-node describing the endpoint that is the video source
+
+- port@1 - VIN instances (optional)
+   - One endpoint sub-node for every R-Car VIN instance which is connected
+ to the R-Car CSI-2 receiver.
+
+Example:
+
+   csi20: csi2@fea8 {
+   compatible = "renesas,r8a7796-csi2";
+   reg = <0 0xfea8 0 0x1>;
+   interrupts = <0 184 IRQ_TYPE_LEVEL_HIGH>;
+   clocks = < CPG_MOD 714>;
+   power-domains = < R8A7796_PD_ALWAYS_ON>;
+   resets = < 714>;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   reg = <0>;
+
+   csi20_in: endpoint@0 {
+   reg = <0>;
+   clock-lanes = <0>;
+   data-lanes = <1>;
+   remote-endpoint = <_txb>;
+   };
+   };
+
+   port@1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   reg = <1>;
+
+   csi20vin0: endpoint@0 {
+   reg = <0>;
+   remote-endpoint = <>;
+   };
+   csi20vin1: endpoint@1 {
+   reg = <1>;
+   remote-endpoint = <>;
+   };
+   csi20vin2: endpoint@2 {
+   reg = <2>;
+   remote-endpoint = <>;
+   };
+   csi20vin3: endpoint@3 {
+   reg = <3>;
+   remote-endpoint = <>;
+   };
+   csi20vin4: endpoint@4 {
+  

[PATCH v16 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver

[Niklas Söderlund]

A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver
supports the 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 
Reviewed-by: Laurent Pinchart 
Reviewed-by: Maxime Ripard 

---

* Changes since v15
- Merge struct phtw_mbps and struct phypll_hsfreqrange into a new struct
  which maps a mpbs value to a register value, struct rcsi2_mbps_reg.
- Reduced number of loops and delay when waiting for LP-11 and
  confirmation of PHTW write as suggested by Laurent.
- Dropped dev_dbg() printouts of the requested link speed.
- Fix small issues in comments.
- Remove unneeded () in for-loop condition in rcsi2_phtw_write_array().
- Remove __refdata from declaration of 'static struct platform_driver
  rcar_csi2_pdrv'.
- Update MODULE_DESCRIPTION to 'Renesas R-Car MIPI CSI-2 receiver
  driver'.
- Fixed two erroneous values in hsfreqrange_h3_v3h_m3n[]. Thanks Jacopo
  for spotting this!
- Max link speed for V3M and E3 are 1.125Gbps remove settings above that
  limit in phtw_mbps_v3m_e3. This also changed in datasheet v1.0.
- Add review tags from Laurent and Maxime.

* Changes since v14
- Data sheet update changed init sequence for PHY forcing a restructure
  of the driver. The restructure was so big I felt compel to drop all
  review tags :-(
- The change was that the Renesas H3 procedure was aligned with other
  SoC in the Gen3 family procedure. I had kept the rework as separate
  patches and was planing to post once original driver with H3 and M3-W
  support where merged. As review tags are dropped I chosen to squash
  those patches into 2/2.
- Add support for Gen3 V3M.
- Add support for Gen3 M3-N.
- Set PHTC_TESTCLR when stopping the PHY.
- Revert back to the v12 and earlier phypll calculation as it turns out
  it was correct after all.

* Changes since v13
- Change return rcar_csi2_formats + i to return _csi2_formats[i].
- Add define for PHCLM_STOPSTATECKL.
- Update spelling in comments.
- Update calculation in rcar_csi2_calc_phypll() according to
  https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one
  before v14 did not take into account that 2 bits per sample is
  transmitted.
- Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch
  statement to set correct number of lanes to enable.
- Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match
  style of rest of file.
- Switch to %u instead of 0x%x when printing bus type.
- Switch to %u instead of %d for priv->lanes which is unsigned.
- Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in
  rcar_csi2_formats[].
- Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16.
- Set INTSTATE after PL-11 is confirmed to match flow chart in
  datasheet.
- Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs.
- Add Maxime's and laurent's tags.
---
 drivers/media/platform/rcar-vin/Kconfig |   12 +
 drivers/media/platform/rcar-vin/Makefile|1 +
 drivers/media/platform/rcar-vin/rcar-csi2.c | 1084 +++
 3 files changed, 1097 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 8fa7ee468c63afb9..d5835da6d4100d87 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 ..e64f07fe184e7675
--- /dev/null
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -0,0 +1,1084 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Renesas R-Car MIPI CSI-2 Receiver
+ *
+ * Copyright (C) 2018 Renesas Electronics Corp.
+ 

[PATCH v16 0/2] rcar-csi2: add Renesas R-Car MIPI CSI-2

[Niklas Söderlund]

Hi,

This is the latest incarnation of R-Car MIPI CSI-2 receiver driver. It's
based on top of the media-tree and are tested on Renesas Salvator-X and
Salvator-XS together with adv7482 and the now in tree rcar-vin driver :-)

I hope this is the last incarnation of this patch-set, I do think it is
ready for upstream consumption :-)

* Changes since v15
- Merge struct phtw_mbps and struct phypll_hsfreqrange into a new struct 
  which maps a mpbs value to a register value, struct rcsi2_mbps_reg.
- Reduced number of loops and delay when waiting for LP-11 and 
  confirmation of PHTW write as suggested by Laurent.
- Dropped dev_dbg() printouts of the requested link speed.
- Fix small issues in comments.
- Remove unneeded () in for-loop condition in rcsi2_phtw_write_array().
- Remove __refdata from declaration of 'static struct platform_driver 
  rcar_csi2_pdrv'.
- Update MODULE_DESCRIPTION to 'Renesas R-Car MIPI CSI-2 receiver 
  driver'.
- Fixed two erroneous values in hsfreqrange_h3_v3h_m3n[]. Thanks Jacopo 
  for spotting this!
- Max link speed for V3M and E3 are 1.125Gbps remove settings above that 
  limit in phtw_mbps_v3m_e3. This also changed in datasheet v1.0.
- Add review tags from Laurent and Maxime.

* Changes since v14.
- Data sheet update changed init sequence for PHY forcing a restructure
  of the driver. The restructure was so big I felt compel to drop all
  review tags :-(
- The change was that the Renesas H3 procedure was aligned with other
  SoC in the Gen3 family procedure. I had kept the rework as separate
  patches and was planing to post once original driver with H3 and M3-W
  support where merged. As review tags are dropped I chosen to squash
  those patches into 2/2.
- Add support for Gen3 M3-N.
- Add support for Gen3 V3M.
- Set PHTC_TESTCLR when stopping the PHY.
- Revert back to the v12 and earlier phypll calculation as it turns out
  it was correct after all.
- Added compatible string for R8A77965 and R8A77970.
- s/Port 0/port@0/
- s/Port 1/port@1/
- s/Endpoint 0/endpoint@0/

* Changes since v13
- Change return rcar_csi2_formats + i to return _csi2_formats[i].
- Add define for PHCLM_STOPSTATECKL.
- Update spelling in comments.
- Update calculation in rcar_csi2_calc_phypll() according to
  https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one
  before v14 did not take into account that 2 bits per sample is
  transmitted.
- Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch
  statement to set correct number of lanes to enable.
- Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match
  style of rest of file.
- Switch to %u instead of 0x%x when printing bus type.
- Switch to %u instead of %d for priv->lanes which is unsigned.
- Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in
  rcar_csi2_formats[].
- Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16.
- Set INTSTATE after PL-11 is confirmed to match flow chart in
  datasheet.
- Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs.
- Add Maxime's and laurent's tags.

* Changes since v12
- Fixed spelling mistakes in commit messages and documentation patch,
  thanks Laurent.
- Mark endpoints in port 1 as optional as it is allowed to not connect a
  VIN to the CSI-2 and instead have it only use its parallel input
  source (for those VIN that have one).
- Added Ack from Sakari, thanks!
- Media bus codes are u32 not unsigned int.
- Ignore error check for v4l2_subdev_call(sd, video, s_stream, 0)
- Do not set subdev host data as it's not used,
  v4l2_set_subdev_hostdata().
- Fixed spelling errors in commit message.
- Add SPDX header
- Rename badly named variable tmp to vcdt_part.
- Cache subdevice in bound callback instead of looking it up in the
  media graph. By doing this rcar_csi2_sd_info() can be removed.
- Print unsigned using %u not %d.
- Call pm_runtime_enable() before calling v4l2_async_register_subdev().
- Dropped of_match_ptr() as OF is required.

* Changes since v11
- Added missing call to v4l2_async_notifier_unregister().
- Fixed missing reg popery in bindings documentation.
- Add Rob's ack to 01/02.
- Dropped 'media:' prefix from patch subjects as it seems they are added
  first when a patch is picked up by the maintainer.
- Fixed typo in comment enpoint -> endpoint, thanks Hans.
- Added Hans Reviewed-by to driver.

* Changes since v10
- Renamed Documentation/devicetree/bindings/media/rcar-csi2.txt to
  Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
- Add extra newline in rcar_csi2_code_to_fmt()
- Use locally stored format information instead of reading it from the
  remote subdevice, Sakari pointed out that the pipeline is validated
  before .s_stream() is called so this is safe.
- Do not check return from media_entity_to_v4l2_subdev() in
  rcar_csi2_start(), Sakari pointed out it can't fail. Also move logic
  to find the remote subdevice is moved to the only user of it,
  rcar_csi2_calc_phypll().
- Move pm_runtime_get_sync() and 

Re: [PATCH v2 26/26] drm/bridge: establish a link between the bridge supplier and consumer

[Peter Rosin]

On 2018-05-14 18:28, Daniel Vetter wrote:
> On Fri, May 11, 2018 at 09:37:47AM +0200, Peter Rosin wrote:
>> On 2018-05-10 10:10, Andrzej Hajda wrote:
>>> On 04.05.2018 15:52, Peter Rosin wrote:
 If the bridge supplier is unbound, this will bring the bridge consumer
 down along with the bridge. Thus, there will no longer linger any
 dangling pointers from the bridge consumer (the drm_device) to some
 non-existent bridge supplier.

 Signed-off-by: Peter Rosin 
 ---
  drivers/gpu/drm/drm_bridge.c | 18 ++
  include/drm/drm_bridge.h |  2 ++
  2 files changed, 20 insertions(+)

 diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
 index 78d186b6831b..0259f0a3ff27 100644
 --- a/drivers/gpu/drm/drm_bridge.c
 +++ b/drivers/gpu/drm/drm_bridge.c
 @@ -26,6 +26,7 @@
  #include 
  
  #include 
 +#include 
  #include 
  
  #include "drm_crtc_internal.h"
 @@ -127,12 +128,25 @@ int drm_bridge_attach(struct drm_encoder *encoder, 
 struct drm_bridge *bridge,
if (bridge->dev)
return -EBUSY;
  
 +  if (encoder->dev->dev != bridge->odev) {
>>>
>>> I wonder why device_link_add does not handle this case (self dependency)
>>> silently as noop, as it seems to be a correct behavior.
>>
>> It's kind-of a silly corner-case though, so perfectly understandable
>> that it isn't handled.
>>
 +  bridge->link = device_link_add(encoder->dev->dev,
 + bridge->odev, 0);
 +  if (!bridge->link) {
 +  dev_err(bridge->odev, "failed to link bridge to %s\n",
 +  dev_name(encoder->dev->dev));
 +  return -EINVAL;
 +  }
 +  }
 +
bridge->dev = encoder->dev;
bridge->encoder = encoder;
  
if (bridge->funcs->attach) {
ret = bridge->funcs->attach(bridge);
if (ret < 0) {
 +  if (bridge->link)
 +  device_link_del(bridge->link);
 +  bridge->link = NULL;
bridge->dev = NULL;
bridge->encoder = NULL;
return ret;
 @@ -159,6 +173,10 @@ void drm_bridge_detach(struct drm_bridge *bridge)
if (bridge->funcs->detach)
bridge->funcs->detach(bridge);
  
 +  if (bridge->link)
 +  device_link_del(bridge->link);
 +  bridge->link = NULL;
 +
bridge->dev = NULL;
  }
  
 diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
 index b656e505d11e..804189c63a4c 100644
 --- a/include/drm/drm_bridge.h
 +++ b/include/drm/drm_bridge.h
 @@ -261,6 +261,7 @@ struct drm_bridge_timings {
   * @list: to keep track of all added bridges
   * @timings: the timing specification for the bridge, if any (may
   * be NULL)
 + * @link: drm consumer <-> bridge supplier
>>>
>>> Nitpick: "<->" suggests symmetry, maybe "device link from drm consumer
>>> to the bridge" would be better.
>>
>> I meant "<->" to indicate that the link is bidirectional, not that the
>> relationship is in any way symmetric. I wasn't aware of any implication
>> of a symmetric relationship when using "<->", do you have a reference?
>> But I guess the different arrow notations in math are somewhat overloaded
>> and that someone at some point must have used "<->" to indicate a
>> symmetric relationship...
> 
> Yeah I agree with Andrzej here, for me <-> implies a symmetric
> relationship. Spelling it out like Andrzej suggested sounds like the
> better idea.
> -Daniel

Ok, I guess that means I have to do a v3 after all. Or can this
trivial documentation update be done by the committer? I hate to
spam everyone with another volley...

Or perhaps I should squash patches 2-23 that are all rather similar
and mechanic? I separated them to allow for easier review from
individual driver maintainers, but that didn't seem to happen
anyway...

Cheers,
Peter

> 
>>
>>> Anyway:
>>> Reviewed-by: Andrzej Hajda 
>>
>> Thanks!
>>
>> Cheers,
>> Peter
>>
>>>  --
>>> Regards
>>> Andrzej
>>>
   * @funcs: control functions
   * @driver_private: pointer to the bridge driver's internal context
   */
 @@ -271,6 +272,7 @@ struct drm_bridge {
struct drm_bridge *next;
struct list_head list;
const struct drm_bridge_timings *timings;
 +  struct device_link *link;
  
const struct drm_bridge_funcs *funcs;
void *driver_private;
>>>
>>>
>>
> 

Re: [PATCH 0/3] arm64: dts: Draak: Enable HDMI input and VIN4

[Geert Uytterhoeven]

Hi Jacopo,

On Fri, May 11, 2018 at 11:59 AM, Jacopo Mondi
 wrote:
>this series enables HDMI input and VIN4 on R-Car D3 Draak board.
>
> The Draak board has an HDMI input connected to an HDMI decoder that feeds
> the VIN capture interface through its parallel video interface.
>
> The series requires the just sent:
> [PATCH 0/5] rcar-vin: Add support for digital input on Gen3
>
> and enables image capture operations on D3 Draak board.
>
> The series has been developed on top of media-master tree but applies cleanly
> on top of latest renesas-driver.
>
> Geert: would you like a topic branch for this series to be included in
> renesas-drivers?

It seems patch 2 has been applied by Simon already, but there is some
discussion pending on patch 3?

> Patches for testing are available at:
> git://jmondi.org/linux d3/media-master/driver
> git://jmondi.org/linux d3/media-master/dts
> git://jmondi.org/linux d3/media-master/test
> git://jmondi.org/vin-tests d3
>
> Thanks
> j
>
> Jacopo Mondi (3):
>   dt-bindings: media: rcar-vin: Add R8A77995 support
>   arm64: dts: renesas: r8a77995: Add VIN4
>   arm64: dts: renesas: draak: Describe HDMI input
>
>  .../devicetree/bindings/media/rcar_vin.txt |  1 +
>  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 68 
> ++
>  arch/arm64/boot/dts/renesas/r8a77995.dtsi  | 11 
>  3 files changed, 80 insertions(+)

Gr{oetje,eeting}s,

Geert

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

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

Re: [PATCH 1/4] arm64: dts: renesas: r8a77990: Add PFC device node

[Geert Uytterhoeven]

Hi Simon,

On Sun, May 13, 2018 at 10:14 AM, Simon Horman  wrote:
> On Fri, May 11, 2018 at 01:31:18PM +0900, Yoshihiro Shimoda wrote:
>> This patch adds PFC device node for r8a77990 (R-Car E3).
>>
>> Signed-off-by: Yoshihiro Shimoda 
>> ---
>>  arch/arm64/boot/dts/renesas/r8a77990.dtsi | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi 
>> b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
>> index 4658029..efc3c0b 100644
>> --- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
>> @@ -60,6 +60,11 @@
>>   #size-cells = <2>;
>>   ranges;
>>
>> + pfc: pin-controller@e606 {
>> + compatible = "renesas,pfc-r8a77990";
>> + reg = <0 0xe606 0 0x508>;
>> + };
>
> Here the register size is 0x508 which matches r8a77995.dtsi in mainline.
> Other variants there are:
> * 0x50c: r8a7795.dtsi, r8a77965.dtsi, r8a7796.dtsi
> * 0x504: r8a77970.dtsi

The number of registers is SoC-specific.
R-Car H3, M3-W, and M3-N share PFC documentation.

> My reading of the documentation is that the size of the register range is
> 0x50c. So I suggest we either use that value consistently or move to a
> larger value after some rounding-up. Geert?

According to Section 6D ("Pin Function Controller (PFC)" for R-Car E3)
of the datasheet, the last register is at offset 0x504, so length 0x508 is
correct.

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

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

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

Re: [PATCH 3/6] pinctrl: sh-pfc: r8a77990: Add bias pinconf support

[Geert Uytterhoeven]

Hi Shimoda-san,

On Fri, May 11, 2018 at 5:22 AM, Yoshihiro Shimoda
 wrote:
> From: Takeshi Kihara 
>
> This patch implements control of pull-up and pull-down. On this SoC there
> is no simple mapping of GP pins to bias register bits, so we need a table.
>
> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Yoshihiro Shimoda 

Thanks for your patch!

> --- a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c

> @@ -1227,10 +1248,55 @@ enum {
>
> PINMUX_IPSR_GPSR(IP15_31_28,USB30_OVC),
> PINMUX_IPSR_MSEL(IP15_31_28,USB0_OVC_A, 
> SEL_USB_20_CH0_0),
> +
> +/*
> + * Static pins can not be muxed between different functions but
> + * still needs a mark entry in the pinmux list. Add each static

need mark entries

> + * pin to the list without an associated function. The sh-pfc
> + * core will do the right thing and skip trying to mux then pin

mux the pin

> + * while still applying configuration to it

period

I have just sent a patch to fix the other copies, in the hope these grammar
atrocities will stop spreading ;-)

> @@ -1708,8 +1774,263 @@ enum {
> { },
>  };
>
> +static const struct pinmux_bias_reg pinmux_bias_regs[] = {

The register definitions look OK to me.
I'll review the actual pin mappings later.

Gr{oetje,eeting}s,

Geert

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

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

Re: [PATCH] pinctrl: sh-pfc: rcar-gen3: Fix grammar in static pin comments

[Niklas Söderlund]

Hi Geert,

Thanks for your work.

On 2018-05-14 21:44:02 +0200, Geert Uytterhoeven wrote:
> The comment block explaining the rationale for static pins contains
> grammar errors.  It appeared first in the pin control driver for R-Car
> H3 ES1.x, and spread to R-Car M3-W, H3 ES2.0, and M3-N later.
> 
> Fix the grammar in all copies at once.
> 
> Signed-off-by: Geert Uytterhoeven 

Reviewed-by: Niklas Söderlund 

Not sure a review tag from the one who made the grammar errors in the 
first place is worth much... :-)

> ---
> To be queued in sh-pfc-for-v4.18.
> 
>  drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c | 6 +++---
>  drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 6 +++---
>  drivers/pinctrl/sh-pfc/pfc-r8a7796.c | 6 +++---
>  drivers/pinctrl/sh-pfc/pfc-r8a77965.c| 6 +++---
>  4 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c 
> b/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c
> index 82a1c411c952a63e..a6c5d50557e676de 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c
> @@ -1432,10 +1432,10 @@ static const u16 pinmux_data[] = {
>  
>  /*
>   * Static pins can not be muxed between different functions but
> - * still needs a mark entry in the pinmux list. Add each static
> + * still need mark entries in the pinmux list. Add each static
>   * pin to the list without an associated function. The sh-pfc
> - * core will do the right thing and skip trying to mux then pin
> - * while still applying configuration to it
> + * core will do the right thing and skip trying to mux the pin
> + * while still applying configuration to it.
>   */
>  #define FM(x)PINMUX_DATA(x##_MARK, 0),
>   PINMUX_STATIC
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c 
> b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> index 34626898f757a13a..4f55b1562ad4878a 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> @@ -1493,10 +1493,10 @@ static const u16 pinmux_data[] = {
>  
>  /*
>   * Static pins can not be muxed between different functions but
> - * still needs a mark entry in the pinmux list. Add each static
> + * still need mark entries in the pinmux list. Add each static
>   * pin to the list without an associated function. The sh-pfc
> - * core will do the right thing and skip trying to mux then pin
> - * while still applying configuration to it
> + * core will do the right thing and skip trying to mux the pin
> + * while still applying configuration to it.
>   */
>  #define FM(x)PINMUX_DATA(x##_MARK, 0),
>   PINMUX_STATIC
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7796.c 
> b/drivers/pinctrl/sh-pfc/pfc-r8a7796.c
> index 764afa13a8c63658..3ea133cfb241a3df 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7796.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7796.c
> @@ -1499,10 +1499,10 @@ static const u16 pinmux_data[] = {
>  
>  /*
>   * Static pins can not be muxed between different functions but
> - * still needs a mark entry in the pinmux list. Add each static
> + * still need mark entries in the pinmux list. Add each static
>   * pin to the list without an associated function. The sh-pfc
> - * core will do the right thing and skip trying to mux then pin
> - * while still applying configuration to it
> + * core will do the right thing and skip trying to mux the pin
> + * while still applying configuration to it.
>   */
>  #define FM(x)   PINMUX_DATA(x##_MARK, 0),
>   PINMUX_STATIC
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77965.c 
> b/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
> index 4d944e3c73e91b61..5caecb6a844180ee 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
> @@ -1501,10 +1501,10 @@ static const u16 pinmux_data[] = {
>  
>  /*
>   * Static pins can not be muxed between different functions but
> - * still needs a mark entry in the pinmux list. Add each static
> + * still need mark entries in the pinmux list. Add each static
>   * pin to the list without an associated function. The sh-pfc
> - * core will do the right thing and skip trying to mux then pin
> - * while still applying configuration to it
> + * core will do the right thing and skip trying to mux the pin
> + * while still applying configuration to it.
>   */
>  #define FM(x)   PINMUX_DATA(x##_MARK, 0),
>   PINMUX_STATIC
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

[PATCH] pinctrl: sh-pfc: rcar-gen3: Fix grammar in static pin comments

[Geert Uytterhoeven]

The comment block explaining the rationale for static pins contains
grammar errors.  It appeared first in the pin control driver for R-Car
H3 ES1.x, and spread to R-Car M3-W, H3 ES2.0, and M3-N later.

Fix the grammar in all copies at once.

Signed-off-by: Geert Uytterhoeven 
---
To be queued in sh-pfc-for-v4.18.

 drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c | 6 +++---
 drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 6 +++---
 drivers/pinctrl/sh-pfc/pfc-r8a7796.c | 6 +++---
 drivers/pinctrl/sh-pfc/pfc-r8a77965.c| 6 +++---
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c
index 82a1c411c952a63e..a6c5d50557e676de 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c
@@ -1432,10 +1432,10 @@ static const u16 pinmux_data[] = {
 
 /*
  * Static pins can not be muxed between different functions but
- * still needs a mark entry in the pinmux list. Add each static
+ * still need mark entries in the pinmux list. Add each static
  * pin to the list without an associated function. The sh-pfc
- * core will do the right thing and skip trying to mux then pin
- * while still applying configuration to it
+ * core will do the right thing and skip trying to mux the pin
+ * while still applying configuration to it.
  */
 #define FM(x)  PINMUX_DATA(x##_MARK, 0),
PINMUX_STATIC
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
index 34626898f757a13a..4f55b1562ad4878a 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
@@ -1493,10 +1493,10 @@ static const u16 pinmux_data[] = {
 
 /*
  * Static pins can not be muxed between different functions but
- * still needs a mark entry in the pinmux list. Add each static
+ * still need mark entries in the pinmux list. Add each static
  * pin to the list without an associated function. The sh-pfc
- * core will do the right thing and skip trying to mux then pin
- * while still applying configuration to it
+ * core will do the right thing and skip trying to mux the pin
+ * while still applying configuration to it.
  */
 #define FM(x)  PINMUX_DATA(x##_MARK, 0),
PINMUX_STATIC
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7796.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a7796.c
index 764afa13a8c63658..3ea133cfb241a3df 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7796.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7796.c
@@ -1499,10 +1499,10 @@ static const u16 pinmux_data[] = {
 
 /*
  * Static pins can not be muxed between different functions but
- * still needs a mark entry in the pinmux list. Add each static
+ * still need mark entries in the pinmux list. Add each static
  * pin to the list without an associated function. The sh-pfc
- * core will do the right thing and skip trying to mux then pin
- * while still applying configuration to it
+ * core will do the right thing and skip trying to mux the pin
+ * while still applying configuration to it.
  */
 #define FM(x)   PINMUX_DATA(x##_MARK, 0),
PINMUX_STATIC
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77965.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
index 4d944e3c73e91b61..5caecb6a844180ee 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
@@ -1501,10 +1501,10 @@ static const u16 pinmux_data[] = {
 
 /*
  * Static pins can not be muxed between different functions but
- * still needs a mark entry in the pinmux list. Add each static
+ * still need mark entries in the pinmux list. Add each static
  * pin to the list without an associated function. The sh-pfc
- * core will do the right thing and skip trying to mux then pin
- * while still applying configuration to it
+ * core will do the right thing and skip trying to mux the pin
+ * while still applying configuration to it.
  */
 #define FM(x)   PINMUX_DATA(x##_MARK, 0),
PINMUX_STATIC
-- 
2.7.4

Re: [PATCH v15 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver

[Niklas Söderlund]

Hi Jacopo,

Thanks for your feedback.

On 2018-05-14 15:14:48 +0200, Jacopo Mondi wrote:
> Hi Niklas,
>thanks for the patch
> 
> On Sun, May 13, 2018 at 09:19:17PM +0200, Niklas Söderlund wrote:
> > A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver
> > supports the 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 
> >
> > ---
> >
> > * Changes since v14
> > - Data sheet update changed init sequence for PHY forcing a restructure
> >   of the driver. The restructure was so big I felt compel to drop all
> >   review tags :-(
> > - The change was that the Renesas H3 procedure was aligned with other
> >   SoC in the Gen3 family procedure. I had kept the rework as separate
> >   patches and was planing to post once original driver with H3 and M3-W
> >   support where merged. As review tags are dropped I chosen to squash
> >   those patches into 2/2.
> > - Add support for Gen3 V3M.
> > - Add support for Gen3 M3-N.
> > - Set PHTC_TESTCLR when stopping the PHY.
> > - Revert back to the v12 and earlier phypll calculation as it turns out
> >   it was correct after all.
> >
> > * Changes since v13
> > - Change return rcar_csi2_formats + i to return _csi2_formats[i].
> > - Add define for PHCLM_STOPSTATECKL.
> > - Update spelling in comments.
> > - Update calculation in rcar_csi2_calc_phypll() according to
> >   https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one
> >   before v14 did not take into account that 2 bits per sample is
> >   transmitted.
> > - Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch
> >   statement to set correct number of lanes to enable.
> > - Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match
> >   style of rest of file.
> > - Switch to %u instead of 0x%x when printing bus type.
> > - Switch to %u instead of %d for priv->lanes which is unsigned.
> > - Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in
> >   rcar_csi2_formats[].
> > - Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16.
> > - Set INTSTATE after PL-11 is confirmed to match flow chart in
> >   datasheet.
> > - Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs.
> > - Add Maxime's and laurent's tags.
> > ---
> >  drivers/media/platform/rcar-vin/Kconfig |   12 +
> >  drivers/media/platform/rcar-vin/Makefile|1 +
> >  drivers/media/platform/rcar-vin/rcar-csi2.c | 1101 +++
> >  3 files changed, 1114 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 8fa7ee468c63afb9..d5835da6d4100d87 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 ..b19374f1516464dc
> > --- /dev/null
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > @@ -0,0 +1,1101 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Driver for Renesas R-Car MIPI CSI-2 Receiver
> > + *
> > + * Copyright (C) 2018 Renesas Electronics Corp.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +struct rcar_csi2;
> > +
> > +/* Register offsets and bits */
> > +
> > +/* Control Timing Select */
> > +#define TREF_REG   0x00
> > +#define TREF_TREF  BIT(0)
> > +
> > +/* Software 

Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input

[Laurent Pinchart]

Hi Niklas,

On Monday, 14 May 2018 13:23:26 EEST Niklas Söderlund wrote:
> On 2018-05-14 09:39:34 +0200, Jacopo Mondi wrote:
> > On Sun, May 13, 2018 at 02:57:55PM +0200, Niklas Söderlund wrote:
> >> On 2018-05-11 12:00:02 +0200, Jacopo Mondi wrote:
> >>> Describe HDMI input connected to VIN4 interface for R-Car D3 Draak
> >>> development board.
> >>> 
> >>> Signed-off-by: Jacopo Mondi 
> >>> ---
> >>> 
> >>>  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 68 +++
> >>>  1 file changed, 68 insertions(+)
> >>> 
> >>> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> >>> b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index
> >>> d03f194..e0ce462 100644
> >>> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> >>> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts

[snip]

> >>> + {
> >>> + pinctrl-0 = <_pins>;
> >>> + pinctrl-names = "default";
> >>> +
> >>> + status = "okay";
> >>> +
> >>> + ports {
> >>> + #address-cells = <1>;
> >>> + #size-cells = <0>;
> >>> +
> >>> + port@0 {
> >>> + reg = <0>;
> >>> +
> >>> + vin4_in: endpoint {
> >>> + hsync-active = <0>;
> >>> + vsync-active = <0>;
> >> 
> >> Comparing this to the Gen2 bindings some properties are missing,
> >> 
> >> bus-width = <24>;
> >> pclk-sample = <1>;
> >> data-active = <1>;
> > 
> > The VIN driver does not parse them, so there is no value in having
> > them there, if not confusing people as it happened to me reading the
> > Gen2 DT.
> 
> I have no objection removing them. Trying to understand why the
> description differed from Gen2.
> 
> >> This is not a big deal as the VIN driver don't use these properties so
> >> no functional change should come of this but still a difference.
> > 
> > Exactly.
> > 
> > On a side note. I have not seen a way to configure the pixel clock
> > sampling level in the interface datasheet. The register used to
> > configure synchronism signals polarities is VnDMR2, and there I read
> > we can configure HSYNC/VSYNC and CLOCKENB (which is data enable, not
> > pixel clock) polarities. Is it configured through some other
> > register?
> 
> I have not seen such a register no.
> 
> >> Over all I'm happy with this change but before I add my tag I would like
> >> to understand why it differs from the Gen2 configuration for the adv7612
> >> properties.
> >> 
> >> Also on a side not it is possible with hardware switches on the board
> >> switch the VIN4 source to a completely different pipeline CVBS connector
> >> -> adv7180 -> VIN4. But I think it's best we keep the HDMI as default as
> >> this seems to be how the boards are shipped. But maybe mentioning this
> >> in the commit message would not hurt if you end-up resending the patch.
> > 
> > Oh I see. SW-49 to SW-52 enables the HDMI input, SW53-SW54 CVBS one.
> > And actually, reading the 'initial setting of slide switches' in the
> > Draak board manual, it turns out that the board default configuration
> > is with CVBS input selected... What should we do here? reflect
> > defaults in the DT, or prioritize HDMI?
> 
> I feel this is a question for Laurent. My feeling for how we handled
> this in other cases is to go with the board default settings. I'm
> however sure there are exceptions to the rule. So maybe we should go
> with the most useful (what ever that is) configuration?

I think I'd go with CVBS as I don't think HDMI would be considered as the most 
useful configuration here. The Draak board is unlikely to be used by us as a 
reference platform to test HDMI capture, is it ?

This being said, you can instantiate the adv7612 and HDMI connector in DT, 
without connecting them to the VIN. That would make it easy to quickly change 
the configuration.

> >>> +
> >>> + remote-endpoint = <_out>;
> >>> + };
> >>> + };
> >>> + };
> >>> +};

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input

[Laurent Pinchart]

Hi Niklas,

On Monday, 14 May 2018 12:49:00 EEST Niklas Söderlund wrote:
> On 2018-05-14 05:49:41 +0300, Laurent Pinchart wrote:
> 
> [snip]
> 
> >>> + {
> >>> + pinctrl-0 = <_pins>;
> >>> + pinctrl-names = "default";
> >>> +
> >>> + status = "okay";
> >>> +
> >>> + ports {
> >>> + #address-cells = <1>;
> >>> + #size-cells = <0>;
> >>> +
> >>> + port@0 {
> >>> + reg = <0>;
> >>> + vin4_in: endpoint {
> >>> + hsync-active = <0>;
> >>> + vsync-active = <0>;
> >> 
> >> Comparing this to the Gen2 bindings some properties are missing,
> >> 
> >> bus-width = <24>;
> >> pclk-sample = <1>;
> >> data-active = <1>;
> >> 
> >> This is not a big deal as the VIN driver don't use these properties so
> >> no functional change should come of this but still a difference.
> > 
> > I think the VIN DT bindings should be updated to explicitly list the
> > endpoint properties that are mandatory, optional, or not allowed.
> 
> I think it's documented as it reference video-interfaces.txt which lists
> all these properties as optional. And in deed they are all optional.

I don't think that's good enough. They're all listed as optional in video-
interfaces.txt as the generic documentation can't know whether a particular 
device will require a particular property or not. It's the responsibility of 
device DT bindings to refine the bindings description. The VIN DT bindings 
should explicitly list the properties that apply to the VIN and tell whether 
they're optional or mandatory for the VIN. For optional properties, the 
default behaviour when the property is not specified should be documented too.

For instance, does VIN support selecting which pixel clock edge to sample data 
on ? If so the pclk-sample property should listed as either mandatory or 
optional with a documented default, even if not used by the driver today.

> If the VIN driver makes use of all the optional ones is another matter. How
> do we know that the remote subdevice is not looking at its remote
> endpoint for bus parameters not considered by the rcar-vin driver?

No driver should parse properties of remote nodes, as those properties are to 
be interpreted in the context of the remote node's DT bindings, which the 
driver doesn't know about. Parsing OF graph properties (ports and endpoints) 
is an exception, as by connecting a remote node to the local node with OF 
graph properties you imply that the remote node uses OF graph DT bindings, so 
those properties (and only those properties) can be parsed.

> The thing is that the rcar-vin driver only looks at the remote endpoint
> for these properties and ignores the on its local endpoint. Maybe some
> v4l2 framework change is needed here to make sure the bus properties are
> the same on both endpoints of a link. But I fear such a change would
> break a lot of stuff.

Properties are specified on both endpoints to account for components such as 
inverter gates between the two devices. They can thus be different on the two 
sides, that's perfectly valid. The VIN driver should parse its local 
properties, not the remote properties.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 26/26] drm/bridge: establish a link between the bridge supplier and consumer

[Daniel Vetter]

On Fri, May 11, 2018 at 09:37:47AM +0200, Peter Rosin wrote:
> On 2018-05-10 10:10, Andrzej Hajda wrote:
> > On 04.05.2018 15:52, Peter Rosin wrote:
> >> If the bridge supplier is unbound, this will bring the bridge consumer
> >> down along with the bridge. Thus, there will no longer linger any
> >> dangling pointers from the bridge consumer (the drm_device) to some
> >> non-existent bridge supplier.
> >>
> >> Signed-off-by: Peter Rosin 
> >> ---
> >>  drivers/gpu/drm/drm_bridge.c | 18 ++
> >>  include/drm/drm_bridge.h |  2 ++
> >>  2 files changed, 20 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> >> index 78d186b6831b..0259f0a3ff27 100644
> >> --- a/drivers/gpu/drm/drm_bridge.c
> >> +++ b/drivers/gpu/drm/drm_bridge.c
> >> @@ -26,6 +26,7 @@
> >>  #include 
> >>  
> >>  #include 
> >> +#include 
> >>  #include 
> >>  
> >>  #include "drm_crtc_internal.h"
> >> @@ -127,12 +128,25 @@ int drm_bridge_attach(struct drm_encoder *encoder, 
> >> struct drm_bridge *bridge,
> >>if (bridge->dev)
> >>return -EBUSY;
> >>  
> >> +  if (encoder->dev->dev != bridge->odev) {
> > 
> > I wonder why device_link_add does not handle this case (self dependency)
> > silently as noop, as it seems to be a correct behavior.
> 
> It's kind-of a silly corner-case though, so perfectly understandable
> that it isn't handled.
> 
> >> +  bridge->link = device_link_add(encoder->dev->dev,
> >> + bridge->odev, 0);
> >> +  if (!bridge->link) {
> >> +  dev_err(bridge->odev, "failed to link bridge to %s\n",
> >> +  dev_name(encoder->dev->dev));
> >> +  return -EINVAL;
> >> +  }
> >> +  }
> >> +
> >>bridge->dev = encoder->dev;
> >>bridge->encoder = encoder;
> >>  
> >>if (bridge->funcs->attach) {
> >>ret = bridge->funcs->attach(bridge);
> >>if (ret < 0) {
> >> +  if (bridge->link)
> >> +  device_link_del(bridge->link);
> >> +  bridge->link = NULL;
> >>bridge->dev = NULL;
> >>bridge->encoder = NULL;
> >>return ret;
> >> @@ -159,6 +173,10 @@ void drm_bridge_detach(struct drm_bridge *bridge)
> >>if (bridge->funcs->detach)
> >>bridge->funcs->detach(bridge);
> >>  
> >> +  if (bridge->link)
> >> +  device_link_del(bridge->link);
> >> +  bridge->link = NULL;
> >> +
> >>bridge->dev = NULL;
> >>  }
> >>  
> >> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> >> index b656e505d11e..804189c63a4c 100644
> >> --- a/include/drm/drm_bridge.h
> >> +++ b/include/drm/drm_bridge.h
> >> @@ -261,6 +261,7 @@ struct drm_bridge_timings {
> >>   * @list: to keep track of all added bridges
> >>   * @timings: the timing specification for the bridge, if any (may
> >>   * be NULL)
> >> + * @link: drm consumer <-> bridge supplier
> > 
> > Nitpick: "<->" suggests symmetry, maybe "device link from drm consumer
> > to the bridge" would be better.
> 
> I meant "<->" to indicate that the link is bidirectional, not that the
> relationship is in any way symmetric. I wasn't aware of any implication
> of a symmetric relationship when using "<->", do you have a reference?
> But I guess the different arrow notations in math are somewhat overloaded
> and that someone at some point must have used "<->" to indicate a
> symmetric relationship...

Yeah I agree with Andrzej here, for me <-> implies a symmetric
relationship. Spelling it out like Andrzej suggested sounds like the
better idea.
-Daniel

> 
> > Anyway:
> > Reviewed-by: Andrzej Hajda 
> 
> Thanks!
> 
> Cheers,
> Peter
> 
> >  --
> > Regards
> > Andrzej
> > 
> >>   * @funcs: control functions
> >>   * @driver_private: pointer to the bridge driver's internal context
> >>   */
> >> @@ -271,6 +272,7 @@ struct drm_bridge {
> >>struct drm_bridge *next;
> >>struct list_head list;
> >>const struct drm_bridge_timings *timings;
> >> +  struct device_link *link;
> >>  
> >>const struct drm_bridge_funcs *funcs;
> >>void *driver_private;
> > 
> > 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] arm64: dts: renesas: r8a77990: Add GPIO device nodes

[Sergei Shtylyov]

On 05/14/2018 05:30 PM, Simon Horman wrote:

>>> The compat string renesas,gpio-rcar has been deprecated since v4.14,
>>> the same release that r8a77990 SoC support was added. Thus
>>> renesas,gpio-rcar can safely be removed without any risk of behaviour
>>> changes between old and new mainline kernels and DTBs.
>>
>>This hardly matches the subject. :-)
> 
> Indeed, I will resubmit.

   I'm seeing this patch merged on Sunday...

>>> Signed-off-by: Simon Horman 
>> [...]

MBR, Sergei

Re: [PATCH V5] PCI: rcar: Use runtime PM to control controller clock

[Lorenzo Pieralisi]

On Mon, May 14, 2018 at 05:32:04PM +0200, Marek Vasut wrote:
> On 05/01/2018 12:55 PM, Lorenzo Pieralisi wrote:
> > On Fri, Apr 13, 2018 at 02:48:19PM +0200, Simon Horman wrote:
> >> On Tue, Apr 10, 2018 at 06:17:04PM +0200, Marek Vasut wrote:
> >>> On 04/10/2018 05:28 PM, Geert Uytterhoeven wrote:
> >>
> >> ...
> >>
> >> rcar_pcie_get_resources() is called while the device is
> >> runtime-enabled/resumed,
> >> pci_free_resource_list() is called while the device is 
> >> runtime-disabled.
> >>>
> >>> rcar_pcie_get_resources() is NOT a pair function for
> >>> pci_free_resource_list() . rcar_pcie_parse_request_of_pci_ranges() is a
> >>> pair function for pci_free_resource_list().
> >>>
> >>> rcar_pcie_parse_request_of_pci_ranges() calls
> >>> of_pci_get_host_bridge_resources() internally, so every single function
> >>> called after successful call of rcar_pcie_parse_request_of_pci_ranges()
> >>> must call pci_free_resource_list().
> >>>
> >>> Both of_pci_get_host_bridge_resources() and pci_free_resource_list() are
> >>> called with runtime PM disabled.
> >>>
> >>> The naming of the functions is confusing though.
> >>
> >> Hi,
> >>
> >> thanks everyone for their efforts in preparing/reviewing this patch.
> >>
> >> It seems there are some differences of opinion on how best to handle the
> >> error paths but unlike earlier versions this one seems correct to me. If
> >> that turns out to be false we can address it. But I don't think its likely
> >> things will be enhanced by continuing this review.
> >>
> >> Lorenzo, please consider taking this patch in its current form.
> >>
> >> Reviewed-by: Simon Horman 
> > 
> > Applied to pci/rcar for v4.18, thanks.
> 
> Is there any reason why this patch isnt in next yet ?

Bjorn will merge it into -next this week.

Thanks,
Lorenzo

Re: [PATCH 2/6] pinctrl: sh-pfc: Initial R8A77990 PFC support

[Geert Uytterhoeven]

Hi Shimoda-san,

On Fri, May 11, 2018 at 5:22 AM, Yoshihiro Shimoda
 wrote:
> From: Takeshi Kihara 
>
> This patch adds initial pinctrl driver to support for the R8A77990 SoC.
>
> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Yoshihiro Shimoda 

Reviewed-by: Geert Uytterhoeven 
i.e. will queue in sh-pfc-for-v4.18...

Note that I only looked at the CPU_ALL_PORT() macro, and the various
register addresses.

Gr{oetje,eeting}s,

Geert

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

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

Re: [PATCH V5] PCI: rcar: Use runtime PM to control controller clock

[Marek Vasut]

On 05/01/2018 12:55 PM, Lorenzo Pieralisi wrote:
> On Fri, Apr 13, 2018 at 02:48:19PM +0200, Simon Horman wrote:
>> On Tue, Apr 10, 2018 at 06:17:04PM +0200, Marek Vasut wrote:
>>> On 04/10/2018 05:28 PM, Geert Uytterhoeven wrote:
>>
>> ...
>>
>> rcar_pcie_get_resources() is called while the device is
>> runtime-enabled/resumed,
>> pci_free_resource_list() is called while the device is runtime-disabled.
>>>
>>> rcar_pcie_get_resources() is NOT a pair function for
>>> pci_free_resource_list() . rcar_pcie_parse_request_of_pci_ranges() is a
>>> pair function for pci_free_resource_list().
>>>
>>> rcar_pcie_parse_request_of_pci_ranges() calls
>>> of_pci_get_host_bridge_resources() internally, so every single function
>>> called after successful call of rcar_pcie_parse_request_of_pci_ranges()
>>> must call pci_free_resource_list().
>>>
>>> Both of_pci_get_host_bridge_resources() and pci_free_resource_list() are
>>> called with runtime PM disabled.
>>>
>>> The naming of the functions is confusing though.
>>
>> Hi,
>>
>> thanks everyone for their efforts in preparing/reviewing this patch.
>>
>> It seems there are some differences of opinion on how best to handle the
>> error paths but unlike earlier versions this one seems correct to me. If
>> that turns out to be false we can address it. But I don't think its likely
>> things will be enhanced by continuing this review.
>>
>> Lorenzo, please consider taking this patch in its current form.
>>
>> Reviewed-by: Simon Horman 
> 
> Applied to pci/rcar for v4.18, thanks.

Is there any reason why this patch isnt in next yet ?

-- 
Best regards,
Marek Vasut

Re: [PATCH 1/6] pinctrl: sh-pfc: Add PORT_GP_11 helper macro

[Geert Uytterhoeven]

Hi Shimoda-san,

On Fri, May 11, 2018 at 5:22 AM, Yoshihiro Shimoda
 wrote:
> From: Takeshi Kihara 
>
> This follows the style of existion PORT_GP_X macros and
> will be used by a follow-up patch for the r8a77990 SoC.

Thanks for your patch!

> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Yoshihiro Shimoda 

Reviewed-by: Geert Uytterhoeven 
i.e. will queue in sh-pfc-for-v4.18...

> --- a/drivers/pinctrl/sh-pfc/sh_pfc.h
> +++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
> @@ -415,9 +415,13 @@ struct sh_pfc_soc_info {
> PORT_GP_CFG_1(bank, 9,  fn, sfx, cfg)
>  #define PORT_GP_10(bank, fn, sfx)  PORT_GP_CFG_10(bank, fn, sfx, 0)
>
> -#define PORT_GP_CFG_12(bank, fn, sfx, cfg) \
> +#define PORT_GP_CFG_11(bank, fn, sfx, cfg) \
> PORT_GP_CFG_10(bank, fn, sfx, cfg), \
> -   PORT_GP_CFG_1(bank, 10, fn, sfx, cfg),  \
> +   PORT_GP_CFG_1(bank, 10,  fn, sfx, cfg)

... with one space in fron of "fn" removed.

Gr{oetje,eeting}s,

Geert

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

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

Re: [PATCH/RFC v3 1/4] base: devcon: add a new API to find the graph

[Randy Dunlap]

On 05/14/2018 02:15 AM, Yoshihiro Shimoda wrote:
> This patch adds a new API "device_connection_find_by_graph()" to
> find device connection by using graph.
> 
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  Documentation/driver-api/device_connection.rst |  4 +--
>  drivers/base/devcon.c  | 43 
> ++
>  include/linux/device.h |  2 ++
>  3 files changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/driver-api/device_connection.rst 
> b/Documentation/driver-api/device_connection.rst
> index affbc556..2e2d26f 100644
> --- a/Documentation/driver-api/device_connection.rst
> +++ b/Documentation/driver-api/device_connection.rst
> @@ -19,7 +19,7 @@ Device connections alone do not create a dependency between 
> the two devices.
>  They are only descriptions which are not tied to either of the devices 
> directly.
>  A dependency between the two devices exists only if one of the two endpoint
>  devices requests a reference to the other. The descriptions themselves can be
> -defined in firmware (not yet supported) or they can be built-in.
> +defined in firmware or they can be built-in.
>  
>  Usage
>  -
> @@ -40,4 +40,4 @@ API
>  ---
>  
>  .. kernel-doc:: drivers/base/devcon.c
> -   : functions: device_connection_find_match device_connection_find 
> device_connection_add device_connection_remove
> +   : functions: device_connection_find_match device_connection_find 
> device_connection_add device_connection_remove device_connection_find_by_graph

BTW, that line above should be like:
  :functions: ...
i.e., no space after the first ':'.

I have sent a patch for that and Jon Corbet has applied it to his docs tree.

-- 
~Randy

[PATCH] arm64: dts: renesas: r8a77995: don't use deprecated renesas,gpio-rcar

[Simon Horman]

The compat string renesas,gpio-rcar has been deprecated since v4.14,
the same release that r8a77990 SoC support was added. Thus
renesas,gpio-rcar can safely be removed without any risk of behaviour
changes between old and new mainline kernels and DTBs.

Signed-off-by: Simon Horman 
---
 arch/arm64/boot/dts/renesas/r8a77995.dtsi | 21 +++--
 1 file changed, 7 insertions(+), 14 deletions(-)

 Based on renesas-devel-20180511-v4.17-rc4

diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi 
b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
index 2506f46293e8..f3426266b5ea 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
@@ -88,8 +88,7 @@
 
gpio0: gpio@e605 {
compatible = "renesas,gpio-r8a77995",
-"renesas,rcar-gen3-gpio",
-"renesas,gpio-rcar";
+"renesas,rcar-gen3-gpio";
reg = <0 0xe605 0 0x50>;
interrupts = ;
#gpio-cells = <2>;
@@ -104,8 +103,7 @@
 
gpio1: gpio@e6051000 {
compatible = "renesas,gpio-r8a77995",
-"renesas,rcar-gen3-gpio",
-"renesas,gpio-rcar";
+"renesas,rcar-gen3-gpio";
reg = <0 0xe6051000 0 0x50>;
interrupts = ;
#gpio-cells = <2>;
@@ -120,8 +118,7 @@
 
gpio2: gpio@e6052000 {
compatible = "renesas,gpio-r8a77995",
-"renesas,rcar-gen3-gpio",
-"renesas,gpio-rcar";
+"renesas,rcar-gen3-gpio";
reg = <0 0xe6052000 0 0x50>;
interrupts = ;
#gpio-cells = <2>;
@@ -136,8 +133,7 @@
 
gpio3: gpio@e6053000 {
compatible = "renesas,gpio-r8a77995",
-"renesas,rcar-gen3-gpio",
-"renesas,gpio-rcar";
+"renesas,rcar-gen3-gpio";
reg = <0 0xe6053000 0 0x50>;
interrupts = ;
#gpio-cells = <2>;
@@ -152,8 +148,7 @@
 
gpio4: gpio@e6054000 {
compatible = "renesas,gpio-r8a77995",
-"renesas,rcar-gen3-gpio",
-"renesas,gpio-rcar";
+"renesas,rcar-gen3-gpio";
reg = <0 0xe6054000 0 0x50>;
interrupts = ;
#gpio-cells = <2>;
@@ -168,8 +163,7 @@
 
gpio5: gpio@e6055000 {
compatible = "renesas,gpio-r8a77995",
-"renesas,rcar-gen3-gpio",
-"renesas,gpio-rcar";
+"renesas,rcar-gen3-gpio";
reg = <0 0xe6055000 0 0x50>;
interrupts = ;
#gpio-cells = <2>;
@@ -184,8 +178,7 @@
 
gpio6: gpio@e6055400 {
compatible = "renesas,gpio-r8a77995",
-"renesas,rcar-gen3-gpio",
-"renesas,gpio-rcar";
+"renesas,rcar-gen3-gpio";
reg = <0 0xe6055400 0 0x50>;
interrupts = ;
#gpio-cells = <2>;
-- 
2.11.0

Re: [PATCH] arm64: dts: renesas: r8a77990: Add GPIO device nodes

[Simon Horman]

On Sun, May 13, 2018 at 12:12:27PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 5/13/2018 12:08 PM, Simon Horman wrote:
> 
> > The compat string renesas,gpio-rcar has been deprecated since v4.14,
> > the same release that r8a77990 SoC support was added. Thus
> > renesas,gpio-rcar can safely be removed without any risk of behaviour
> > changes between old and new mainline kernels and DTBs.
> 
>This hardly matches the subject. :-)

Indeed, I will resubmit.

> > Signed-off-by: Simon Horman 
> [...]
> 
> MBR, Sergei
> 

Re: Booting Salvator-X ES1 board with upstream kernel

[Geert Uytterhoeven]

Hi Gilad,

On Thu, May 10, 2018 at 10:12 AM, Gilad Ben-Yossef  wrote:
> I am trying to add support for the CryptoCell security IP in the
> R-Rcar boards to mainline but I've run into some trouble.

Ah, we have found a user ;-)

> I have an R-Car 3rd gen Salvator-X ES1 board. I've been able to boot
> Linux 4.9 on it from the rareness-bsp tree, using the defconfig and
> the r8a7795-es1-salvator-x DTB with no problems, so the HW is fine.
>
> However, I can't seem to boot an upstream kernel (Linus master or
> soc-for-v4.17 branch) on it. I just get total silence on the UART
> after u-boot.
>
> Any tips or ideas for me to try?

Please try appending the following to the kernel command line:

earlycon keep_bootcon

FWIW, I'm using:

tftpboot 0x4808 h3-salvator-x/Image
tftpboot 0x49f0 h3-salvator-x/r8a7795-es1-salvator-x.dtb
booti 0x4808 - 0x49f0

Which firmware revision do you have? It may be too old to boot moden
(large) kernel images. Check the first line of BL2 output.

Which config are you using? Please try renesas_defconfig from Simon's
repository.

Good luck!

Gr{oetje,eeting}s,

Geert

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

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

Re: [PATCH 0/3] fbdev/drm: sh_mobile: remove unused MERAM support

[Bartlomiej Zolnierkiewicz]

On Friday, April 27, 2018 02:09:31 PM Daniel Vetter wrote:
> On Fri, Apr 27, 2018 at 1:36 PM, Laurent Pinchart
>  wrote:
> > Hi Bartlomiej,
> >
> > On Friday, 27 April 2018 14:21:42 EEST Bartlomiej Zolnierkiewicz wrote:
> >> Hi,
> >>
> >> This patchset removes unused MERAM support (last user was removed
> >> 3 years ago) from shmobile fbdev & drm drivers and then removes
> >> MERAM driver itself.
> >>
> >> If it is okay to merge this patches I would like patch #1 to go
> >> through fbdev tree and patch #2 to go through drm tree. Once they
> >> are both upstream (v4.18) I will apply patch #3 to fbdev tree.
> >
> > Or you could merge everything through the fbdev tree in one go, the shmobile
> > driver hardly sees any activity these days.
> >
> > For the whole series,
> >
> > Acked-by: Laurent Pinchart 
> 
> Yeah merging all through fbdev is probably simplest, shmobile is not a
> driver that's actively changed a lot. Ack from me too.

Thanks, I've applied all patches to fbdev-for-next.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics

Re: [PATCH/RFC v3 1/4] base: devcon: add a new API to find the graph

[Heikki Krogerus]

On Mon, May 14, 2018 at 06:15:57PM +0900, Yoshihiro Shimoda wrote:
> This patch adds a new API "device_connection_find_by_graph()" to
> find device connection by using graph.
> 
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  Documentation/driver-api/device_connection.rst |  4 +--
>  drivers/base/devcon.c  | 43 
> ++
>  include/linux/device.h |  2 ++
>  3 files changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/driver-api/device_connection.rst 
> b/Documentation/driver-api/device_connection.rst
> index affbc556..2e2d26f 100644
> --- a/Documentation/driver-api/device_connection.rst
> +++ b/Documentation/driver-api/device_connection.rst
> @@ -19,7 +19,7 @@ Device connections alone do not create a dependency between 
> the two devices.
>  They are only descriptions which are not tied to either of the devices 
> directly.
>  A dependency between the two devices exists only if one of the two endpoint
>  devices requests a reference to the other. The descriptions themselves can be
> -defined in firmware (not yet supported) or they can be built-in.
> +defined in firmware or they can be built-in.
>  
>  Usage
>  -
> @@ -40,4 +40,4 @@ API
>  ---
>  
>  .. kernel-doc:: drivers/base/devcon.c
> -   : functions: device_connection_find_match device_connection_find 
> device_connection_add device_connection_remove
> +   : functions: device_connection_find_match device_connection_find 
> device_connection_add device_connection_remove device_connection_find_by_graph
> diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
> index d427e80..5a0da33 100644
> --- a/drivers/base/devcon.c
> +++ b/drivers/base/devcon.c
> @@ -7,6 +7,7 @@
>   */
>  
>  #include 
> +#include 
>  
>  static DEFINE_MUTEX(devcon_lock);
>  static LIST_HEAD(devcon_list);
> @@ -134,3 +135,45 @@ void device_connection_remove(struct device_connection 
> *con)
>   mutex_unlock(_lock);
>  }
>  EXPORT_SYMBOL_GPL(device_connection_remove);
> +
> +static int generic_graph_match(struct device *dev, void *fwnode)
> +{
> + return dev->fwnode == fwnode;
> +}
> +
> +/**
> + * device_connection_find_by_graph - Find two devices connected together
> + * @dev: Device to find connected device
> + * @port: identifier of the @dev port node
> + * @endpoint: identifier of the @dev endpoint node
> + *
> + * Find a connection with @port and @endpoint by using graph between @dev and
> + * another device. On success returns handle to the device that is connected
> + * to @dev, with the reference count for the found device incremented. 
> Returns
> + * NULL if no matching connection was found, or ERR_PTR(-EPROBE_DEFER) when
> + * a connection was found but the other device has not been enumerated yet.
> + */
> +struct device *device_connection_find_by_graph(struct device *dev, u32 port,
> +u32 endpoint)
> +{
> + struct bus_type *bus;
> + struct fwnode_handle *remote;
> + struct device *conn;
> +
> + remote = fwnode_graph_get_remote_node(dev_fwnode(dev), port, endpoint);
> + if (!remote)
> + return NULL;
> +
> + for (bus = generic_match_buses[0]; bus; bus++) {
> + conn = bus_find_device(bus, NULL, remote, generic_graph_match);
> + if (conn)
> + return conn;
> + }
> +
> + /*
> +  * We only get called if a connection was found, tell the caller to
> +  * wait for the other device to show up.
> +  */
> + return ERR_PTR(-EPROBE_DEFER);
> +}
> +EXPORT_SYMBOL_GPL(device_connection_find_by_graph);

Why do we need more API for walking through the graph?

I'm not sure exactly sure what is going on here, I'll try to study
your patches more when I have time, but the approach looks wrong. That
function looks like a helper, but just not that useful one.

We really should be able to use the existing functions. In practice
device_connection_find_match() should eventually parse the graph, then
fallback to build-in connections if no graph is found. Otherwise
parsing graph here is not really useful at all.


Thanks,

-- 
heikki

Re: [PATCH v15 1/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver documentation

[jacopo mondi]

Hi Niklas,
thanks for re-sending

On Sun, May 13, 2018 at 09:19:16PM +0200, 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 than one VIN device which
> simultaneously can receive video from the same CSI-2 device. Each VIN
> device can also be connected to more than one CSI-2 device. The routing
> of which links are used is 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.
>
> Signed-off-by: Niklas Söderlund 
> Acked-by: Rob Herring 
> Acked-by: Sakari Ailus 
> Reviewed-by: Laurent Pinchart 
>
> ---
>
> * Changes since v14.
> - Added compatible string for R8A77965 and R8A77970.
> - s/Port 0/port@0/
> - s/Port 1/port@1/
> - s/Endpoint 0/endpoint@0/
>
> * Changes since v13
> - Add Laurent's tag.
> ---
>  .../bindings/media/renesas,rcar-csi2.txt  | 101 ++
>  MAINTAINERS   |   1 +
>  2 files changed, 102 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 ..2d385b65b275bc58
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> @@ -0,0 +1,101 @@
> +Renesas R-Car MIPI CSI-2
> +
> +
> +The R-Car CSI-2 receiver device provides MIPI CSI-2 capabilities for the
> +Renesas R-Car family of devices. It is 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.
> +   - "renesas,r8a77965-csi2" for the R8A77965 device.
> +   - "renesas,r8a77970-csi2" for the R8A77970 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

s/child nodes according/child nodes modeled accordingly/

but don't fully trust my English language skills here.

> +bindings defined in Documentation/devicetree/bindings/media/
> +video-interfaces.txt. port@0 shall connect to the CSI-2 source. port@1
> +shall connect to all the R-Car VIN modules that have a hardware
> +connection to the CSI-2 receiver.
> +
> +- port@0- Video source (mandatory)
> + - endpoint@0 - sub-node describing the endpoint that is the video source
> +
> +- port@1 - VIN instances (optional)
> + - One endpoint sub-node for every R-Car VIN instance which is connected
> +   to the R-Car CSI-2 receiver.
> +
As commented on v14, I feel like there are some restrictions on the
accepted values for some endpoint properties in the hardware module,
and those should be described here.

Eg. the clock lane shall be fixed in position 0 (that's more an HW
design choice, I agree), and the number of supported data lanes is
either 1, 2 or 4 and 3 is not supported.

Apart from that, which I know you do not totally agree on and you're
free to ignore.

Reviewed-by Jacopo Mondi 

Thanks
   j

> +Example:
> +
> + csi20: csi2@fea8 {
> + compatible = "renesas,r8a7796-csi2";
> + reg = <0 0xfea8 0 0x1>;
> + interrupts = <0 184 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = < CPG_MOD 714>;
> + power-domains = < R8A7796_PD_ALWAYS_ON>;
> + resets = < 714>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + reg = <0>;
> +
> + csi20_in: endpoint@0 {
> + reg = <0>;
> + clock-lanes = <0>;
> + data-lanes = <1>;
> + remote-endpoint = <_txb>;
> + };
> + };
> +
> + port@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + reg = <1>;
> +
> + csi20vin0: endpoint@0 {
> +  

Re: [PATCH v15 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver

[jacopo mondi]

Hi Niklas,
   thanks for the patch

On Sun, May 13, 2018 at 09:19:17PM +0200, Niklas Söderlund wrote:
> A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver
> supports the 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 
>
> ---
>
> * Changes since v14
> - Data sheet update changed init sequence for PHY forcing a restructure
>   of the driver. The restructure was so big I felt compel to drop all
>   review tags :-(
> - The change was that the Renesas H3 procedure was aligned with other
>   SoC in the Gen3 family procedure. I had kept the rework as separate
>   patches and was planing to post once original driver with H3 and M3-W
>   support where merged. As review tags are dropped I chosen to squash
>   those patches into 2/2.
> - Add support for Gen3 V3M.
> - Add support for Gen3 M3-N.
> - Set PHTC_TESTCLR when stopping the PHY.
> - Revert back to the v12 and earlier phypll calculation as it turns out
>   it was correct after all.
>
> * Changes since v13
> - Change return rcar_csi2_formats + i to return _csi2_formats[i].
> - Add define for PHCLM_STOPSTATECKL.
> - Update spelling in comments.
> - Update calculation in rcar_csi2_calc_phypll() according to
>   https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one
>   before v14 did not take into account that 2 bits per sample is
>   transmitted.
> - Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch
>   statement to set correct number of lanes to enable.
> - Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match
>   style of rest of file.
> - Switch to %u instead of 0x%x when printing bus type.
> - Switch to %u instead of %d for priv->lanes which is unsigned.
> - Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in
>   rcar_csi2_formats[].
> - Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16.
> - Set INTSTATE after PL-11 is confirmed to match flow chart in
>   datasheet.
> - Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs.
> - Add Maxime's and laurent's tags.
> ---
>  drivers/media/platform/rcar-vin/Kconfig |   12 +
>  drivers/media/platform/rcar-vin/Makefile|1 +
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 1101 +++
>  3 files changed, 1114 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 8fa7ee468c63afb9..d5835da6d4100d87 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 ..b19374f1516464dc
> --- /dev/null
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -0,0 +1,1101 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Renesas R-Car MIPI CSI-2 Receiver
> + *
> + * Copyright (C) 2018 Renesas Electronics Corp.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct rcar_csi2;
> +
> +/* Register offsets and bits */
> +
> +/* Control Timing Select */
> +#define TREF_REG 0x00
> +#define TREF_TREFBIT(0)
> +
> +/* Software Reset */
> +#define SRST_REG 0x04
> +#define SRST_SRSTBIT(0)
> +
> +/* PHY Operation Control */
> +#define PHYCNT_REG   0x08
> +#define PHYCNT_SHUTDOWNZ BIT(17)
> +#define PHYCNT_RSTZ  BIT(16)
> +#define PHYCNT_ENABLECLK BIT(4)

Re: [GIT PULL FOR renesas-drivers] drm/du/m3n

[Kieran Bingham]

On 14/05/18 13:52, Geert Uytterhoeven wrote:
> Hi Kieran,
> 
> On Sat, May 12, 2018 at 10:09 AM, Kieran Bingham
>  wrote:
>> Please consider including this release in renesas-drivers.
>>
>> --
>> Regards
>>
>> Kieran
>>
>> The following changes since commit 6d08b06e67cd117f6992c46611dfb4ce267cd71e:
>>
>>   Linux 4.17-rc2 (2018-04-22 19:20:09 -0700)
>>
>> are available in the Git repository at:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git 
>> tags/drm/du/m3n
>>
>> for you to fetch changes up to f1e9a22ac3cff749077f40bf1a149aaaf587ae2d:
>>
>>   drm: rcar-du: Add R8A77965 support (2018-05-05 17:11:20 +0300)
> 
> Thanks, merged clealy into today's linux-next.

Great, thanks.

> 
>> 
>> drm: rcar-du: Provide support for the M3-N
>>
>> The M3-N has non linear addressing of it's group objects, which was not
> 
> non-linear?

Probably :)

> its

Definitely !



> 
>> supported by the rcar-du driver implementation due to asumptions on the

Hrm ... also s/asumptions/assumptions/

--
Thanks

Kieran

>> match between a CRTC index, and the hardware indexes.
>>
>> Implement support to remove this assumption and enable the M3-N
> 
> Gr{oetje,eeting}s,
> 
> Geert
> 

Re: [GIT PULL FOR renesas-drivers] drm/du/m3n

[Geert Uytterhoeven]

Hi Kieran,

On Sat, May 12, 2018 at 10:09 AM, Kieran Bingham
 wrote:
> Please consider including this release in renesas-drivers.
>
> --
> Regards
>
> Kieran
>
> The following changes since commit 6d08b06e67cd117f6992c46611dfb4ce267cd71e:
>
>   Linux 4.17-rc2 (2018-04-22 19:20:09 -0700)
>
> are available in the Git repository at:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git 
> tags/drm/du/m3n
>
> for you to fetch changes up to f1e9a22ac3cff749077f40bf1a149aaaf587ae2d:
>
>   drm: rcar-du: Add R8A77965 support (2018-05-05 17:11:20 +0300)

Thanks, merged clealy into today's linux-next.

> 
> drm: rcar-du: Provide support for the M3-N
>
> The M3-N has non linear addressing of it's group objects, which was not

non-linear?
its

> supported by the rcar-du driver implementation due to asumptions on the
> match between a CRTC index, and the hardware indexes.
>
> Implement support to remove this assumption and enable the M3-N

Gr{oetje,eeting}s,

Geert

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

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

Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input

[Niklas Söderlund]

Hi Jacopo,

On 2018-05-14 09:39:34 +0200, Jacopo Mondi wrote:
> Hi Niklas,
> 
> On Sun, May 13, 2018 at 02:57:55PM +0200, Niklas Söderlund wrote:
> > Hi Jacopo,
> >
> > Thanks for your patch.
> >
> > On 2018-05-11 12:00:02 +0200, Jacopo Mondi wrote:
> > > Describe HDMI input connected to VIN4 interface for R-Car D3 Draak
> > > development board.
> > >
> > > Signed-off-by: Jacopo Mondi 
> > > ---
> > >  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 68 
> > > ++
> > >  1 file changed, 68 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts 
> > > b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > > index d03f194..e0ce462 100644
> > > --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > > +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > > @@ -59,6 +59,17 @@
> > >   };
> > >   };
> > >
> > > + hdmi-in {
> > > + compatible = "hdmi-connector";
> > > + type = "a";
> > > +
> > > + port {
> > > + hdmi_con_in: endpoint {
> > > + remote-endpoint = <_in>;
> > > + };
> > > + };
> > > + };
> > > +
> > >   memory@4800 {
> > >   device_type = "memory";
> > >   /* first 128MB is reserved for secure area. */
> > > @@ -142,6 +153,11 @@
> > >   groups = "usb0";
> > >   function = "usb0";
> > >   };
> > > +
> > > + vin4_pins: vin4 {
> > > + groups = "vin4_data24", "vin4_sync", "vin4_clk", "vin4_clkenb";
> > > + function = "vin4";
> > > + };
> > >  };
> > >
> > >   {
> > > @@ -154,6 +170,35 @@
> > >   reg = <0x50>;
> > >   pagesize = <8>;
> > >   };
> > > +
> > > + hdmi-decoder@4c {
> > > + compatible = "adi,adv7612";
> > > + reg = <0x4c>;
> > > + default-input = <0>;
> > > +
> > > + ports {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + port@0 {
> > > + reg = <0>;
> > > + adv7612_in: endpoint {
> > > + remote-endpoint = <_con_in>;
> > > + };
> > > + };
> > > +
> > > + port@2 {
> > > + reg = <2>;
> > > + adv7612_out: endpoint {
> > > + pclk-sample = <0>;
> > > + hsync-active = <0>;
> > > + vsync-active = <0>;
> >
> > This differs from the Gen2 DT bindings which is a very similar hardware
> > setup using the same components. Defining these properties will make the
> > bus marked as V4L2_MBUS_PARALLEL instead of V4L2_MBUS_BT656.
> 
> And that's what we want
> 
> >
> > This will change how the hardware is configured for capture if the media
> > bus is in a UYVY format, see VNMC_INF register in rvin_setup(). Maybe
> > this it not an issue here but still I'm curious to why this differ
> > between Gen2 and Gen3 :-)
> 
> Actually this won't impact the VIN configuration as this is the
> 'remote endpoint' from VIN perspective and the properties used to
> configure the interface are the ones in the 'local endpoint'.

You are right, sorry for the confusion and thanks for educating me :-)

> 
> >
> > > +
> > > + remote-endpoint = <_in>;
> > > + };
> > > + };
> > > + };
> > > + };
> > >  };
> > >
> > >   {
> > > @@ -246,3 +291,26 @@
> > >   timeout-sec = <60>;
> > >   status = "okay";
> > >  };
> > > +
> > > + {
> > > + pinctrl-0 = <_pins>;
> > > + pinctrl-names = "default";
> > > +
> > > + status = "okay";
> > > +
> > > + ports {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + port@0 {
> > > + reg = <0>;
> > > +
> > > + vin4_in: endpoint {
> > > + hsync-active = <0>;
> > > + vsync-active = <0>;
> >
> > Comparing this to the Gen2 bindings some properties are missing,
> >
> > bus-width = <24>;
> > pclk-sample = <1>;
> > data-active = <1>;
> 
> The VIN driver does not parse them, so there is no value in having
> them there, if not confusing people as it happened to me reading the
> Gen2 DT.

I have no objection removing them. Trying to understand why the 
description differed from Gen2.

> 
> >
> > This is not a big deal as the VIN driver don't use these properties so
> > no functional change should come of this but still a difference.
> 
> Exactly.
> 
> On a side note. I have not seen a way to configure the pixel clock
> sampling level in the interface datasheet. The register used to
> configure synchronism signals polarities is VnDMR2, and there I read
> we can configure HSYNC/VSYNC and CLOCKENB (which is data enable, not
> pixel clock) polarities. Is it configured through some other
> 

Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input

[Niklas Söderlund]

Hi again,

On 2018-05-14 11:49:00 +0200, Niklas Söderlund wrote:
> Hi Laurent,
> 
> On 2018-05-14 05:49:41 +0300, Laurent Pinchart wrote:
> 
> [snip]
> 
> > > > + {
> > > > +   pinctrl-0 = <_pins>;
> > > > +   pinctrl-names = "default";
> > > > +
> > > > +   status = "okay";
> > > > +
> > > > +   ports {
> > > > +   #address-cells = <1>;
> > > > +   #size-cells = <0>;
> > > > +
> > > > +   port@0 {
> > > > +   reg = <0>;
> > > > +
> > > > +   vin4_in: endpoint {
> > > > +   hsync-active = <0>;
> > > > +   vsync-active = <0>;
> > > 
> > > Comparing this to the Gen2 bindings some properties are missing,
> > > 
> > > bus-width = <24>;
> > > pclk-sample = <1>;
> > > data-active = <1>;
> > > 
> > > This is not a big deal as the VIN driver don't use these properties so
> > > no functional change should come of this but still a difference.
> > 
> > I think the VIN DT bindings should be updated to explicitly list the 
> > endpoint 
> > properties that are mandatory, optional, or not allowed.
> 
> I think it's documented as it reference video-interfaces.txt which lists 
> all these properties as optional. And in deed they are all optional.  If 
> the VIN driver makes use of all the optional ones is another matter. How 
> do we know that the remote subdevice is not looking at its remote 
> endpoint for bus parameters not considered by the rcar-vin driver?
> 
> The thing is that the rcar-vin driver only looks at the remote endpoint 
> for these properties and ignores the on its local endpoint. Maybe some 
> v4l2 framework change is needed here to make sure the bus properties are 
> the same on both endpoints of a link. But I fear such a change would 
> break a lot of stuff.

Jacopo pointed out this statement is untrue. The rcar-vin only looks at 
it's local endpoint not the remote endpoint for it's bus parameters. The 
callback provided to v4l2_async_notifier_parse_fwnode_endpoints() 
confused me as the subdevice passed to it is the one describe the remote 
endpoint while the v4l2_fwnode_endpoint argument is that of the local 
endpoint. Sorry for the confusion and thanks Jacopo for correcting me.

-- 
Regards,
Niklas Söderlund

Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input

[Niklas Söderlund]

Hi Laurent,

On 2018-05-14 05:49:41 +0300, Laurent Pinchart wrote:

[snip]

> > > + {
> > > + pinctrl-0 = <_pins>;
> > > + pinctrl-names = "default";
> > > +
> > > + status = "okay";
> > > +
> > > + ports {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + port@0 {
> > > + reg = <0>;
> > > +
> > > + vin4_in: endpoint {
> > > + hsync-active = <0>;
> > > + vsync-active = <0>;
> > 
> > Comparing this to the Gen2 bindings some properties are missing,
> > 
> > bus-width = <24>;
> > pclk-sample = <1>;
> > data-active = <1>;
> > 
> > This is not a big deal as the VIN driver don't use these properties so
> > no functional change should come of this but still a difference.
> 
> I think the VIN DT bindings should be updated to explicitly list the endpoint 
> properties that are mandatory, optional, or not allowed.

I think it's documented as it reference video-interfaces.txt which lists 
all these properties as optional. And in deed they are all optional.  If 
the VIN driver makes use of all the optional ones is another matter. How 
do we know that the remote subdevice is not looking at its remote 
endpoint for bus parameters not considered by the rcar-vin driver?

The thing is that the rcar-vin driver only looks at the remote endpoint 
for these properties and ignores the on its local endpoint. Maybe some 
v4l2 framework change is needed here to make sure the bus properties are 
the same on both endpoints of a link. But I fear such a change would 
break a lot of stuff.

-- 
Regards,
Niklas Söderlund

[PATCH/RFC v3 2/4] usb: gadget: udc: renesas_usb3: Add register of usb role switch

[Yoshihiro Shimoda]

This patch adds role switch support for R-Car SoCs into the USB
3.0 peripheral driver. Some R-Car SoCs (e.g. R-Car H3) have USB
3.0 dual-role device controller which has the USB 3.0 xHCI host
and Renesas USB 3.0 peripheral.

Unfortunately, the mode change register contains the USB 3.0 peripheral
controller side only. So, the USB 3.0 peripheral driver (renesas_usb3)
manages this register now. However, in peripheral mode, the host
should stop. Also the host hardware needs to reinitialize its own
registers when the mode changes from peripheral to host mode.
Otherwise, the host cannot work correctly (e.g. detect a device as
high-speed).

To achieve this by a driver, this role switch driver manages
the mode change register and attach/release the xhci-plat driver.

Signed-off-by: Yoshihiro Shimoda 
---
 .../devicetree/bindings/usb/renesas_usb3.txt   | 15 +
 drivers/usb/gadget/udc/Kconfig |  1 +
 drivers/usb/gadget/udc/renesas_usb3.c  | 68 +-
 3 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt 
b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
index 2c071bb5..f6105aa 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
@@ -19,6 +19,9 @@ Required properties:
 Optional properties:
   - phys: phandle + phy specifier pair
   - phy-names: must be "usb"
+  - The connection to a usb3.0 host node needs by using OF graph bindings for
+usb role switch.
+   - port@0 = USB3.0 host port.
 
 Example of R-Car H3 ES1.x:
usb3_peri0: usb@ee02 {
@@ -27,6 +30,12 @@ Example of R-Car H3 ES1.x:
reg = <0 0xee02 0 0x400>;
interrupts = ;
clocks = < CPG_MOD 328>;
+
+   port {
+   usb3_peri0_ep: endpoint {
+   remote-endpoint = <_host0_ep>;
+   };
+   };
};
 
usb3_peri1: usb@ee06 {
@@ -35,4 +44,10 @@ Example of R-Car H3 ES1.x:
reg = <0 0xee06 0 0x400>;
interrupts = ;
clocks = < CPG_MOD 327>;
+
+   port {
+   usb3_peri1_ep: endpoint {
+   remote-endpoint = <_host1_ep>;
+   };
+   };
};
diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
index 0875d38..7e4a5dd 100644
--- a/drivers/usb/gadget/udc/Kconfig
+++ b/drivers/usb/gadget/udc/Kconfig
@@ -193,6 +193,7 @@ config USB_RENESAS_USB3
tristate 'Renesas USB3.0 Peripheral controller'
depends on ARCH_RENESAS || COMPILE_TEST
depends on EXTCON && HAS_DMA
+   select USB_ROLE_SWITCH
help
   Renesas USB3.0 Peripheral controller is a USB peripheral controller
   that supports super, high, and full speed USB 3.0 data transfers.
diff --git a/drivers/usb/gadget/udc/renesas_usb3.c 
b/drivers/usb/gadget/udc/renesas_usb3.c
index 409cde4..c878449 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* register definitions */
 #define USB3_AXI_INT_STA   0x008
@@ -334,6 +335,9 @@ struct renesas_usb3 {
struct work_struct extcon_work;
struct phy *phy;
 
+   struct usb_role_switch *role_sw;
+   struct device *host_dev;
+
struct renesas_usb3_ep *usb3_ep;
int num_usb3_eps;
 
@@ -643,7 +647,7 @@ static void usb3_check_vbus(struct renesas_usb3 *usb3)
}
 }
 
-static void usb3_set_mode(struct renesas_usb3 *usb3, bool host)
+static void _usb3_set_mode(struct renesas_usb3 *usb3, bool host)
 {
if (host)
usb3_clear_bit(usb3, DRD_CON_PERI_CON, USB3_DRD_CON);
@@ -651,6 +655,11 @@ static void usb3_set_mode(struct renesas_usb3 *usb3, bool 
host)
usb3_set_bit(usb3, DRD_CON_PERI_CON, USB3_DRD_CON);
 }
 
+static void usb3_set_mode(struct renesas_usb3 *usb3, bool host)
+{
+   _usb3_set_mode(usb3, host);
+}
+
 static void usb3_vbus_out(struct renesas_usb3 *usb3, bool enable)
 {
if (enable)
@@ -2294,6 +2303,41 @@ static int renesas_usb3_set_selfpowered(struct 
usb_gadget *gadget, int is_self)
.set_selfpowered= renesas_usb3_set_selfpowered,
 };
 
+static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
+{
+   struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+   enum usb_role cur_role;
+
+   pm_runtime_get_sync(dev);
+   cur_role = usb3_is_host(usb3) ? USB_ROLE_HOST : USB_ROLE_DEVICE;
+   pm_runtime_put(dev);
+
+   return cur_role;
+}
+
+static int renesas_usb3_role_switch_set(struct device *dev,
+   enum usb_role role)
+{
+   struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+   struct 

[PATCH/RFC v3 3/4] usb: gadget: udc: renesas_usb3: use usb role switch API

[Yoshihiro Shimoda]

This patch uses usb role switch API if the register suceeeded.

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/gadget/udc/renesas_usb3.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c 
b/drivers/usb/gadget/udc/renesas_usb3.c
index c878449..bfb5803 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -657,7 +657,11 @@ static void _usb3_set_mode(struct renesas_usb3 *usb3, bool 
host)
 
 static void usb3_set_mode(struct renesas_usb3 *usb3, bool host)
 {
-   _usb3_set_mode(usb3, host);
+   if (usb3->role_sw)
+   usb_role_switch_set_role(usb3->role_sw, host ?
+USB_ROLE_HOST : USB_ROLE_DEVICE);
+   else
+   _usb3_set_mode(usb3, host);
 }
 
 static void usb3_vbus_out(struct renesas_usb3 *usb3, bool enable)
@@ -672,8 +676,8 @@ static void usb3_mode_config(struct renesas_usb3 *usb3, 
bool host, bool a_dev)
 {
unsigned long flags;
 
-   spin_lock_irqsave(>lock, flags);
usb3_set_mode(usb3, host);
+   spin_lock_irqsave(>lock, flags);
usb3_vbus_out(usb3, a_dev);
/* for A-Peripheral or forced B-device mode */
if ((!host && a_dev) ||
-- 
1.9.1

[PATCH/RFC v3 0/4] usb: role: rcar-usb3-role-switch: add support for R-Car SoCs

[Yoshihiro Shimoda]

This patch set is based on Felipe's usb.git / testing/next branch
(commit id = 5d1332a8eabd8bd5b8c322d45542968ee6f113be).

I still marked this patch set as "RFC". I would like to know whether
this way is good or not.

Changes from RFC v2:
 - Add registering usb role switch into drivers/usb/gadget/udc/renesas_usb3
   because hardware resource (a register) is shared and remove individual
   usb role switch driver/dt-bindings for R-Car.
 - Remove "usb_role_switch_get_by_graph" API because the renesas_usb3 driver
   doesn't need such API now.

Changes from RFC:
 - Remove "device-connection-id" and "usb role switch driver" dt-bingings.
 - Remove drivers/of code.
 - Add a new API for find the connection by using graph on devcon.c and roles.c.
 - Use each new API on the rcar usb role switch and renesas_usb3 drivers.
 - Update the dtsi file for r8a7795.


Yoshihiro Shimoda (4):
  base: devcon: add a new API to find the graph
  usb: gadget: udc: renesas_usb3: Add register of usb role switch
  usb: gadget: udc: renesas_usb3: use usb role switch API
  arm64: dts: renesas: r8a7795: add OF graph for usb role switch

 .../devicetree/bindings/usb/renesas_usb3.txt   | 15 +
 Documentation/driver-api/device_connection.rst |  4 +-
 arch/arm64/boot/dts/renesas/r8a7795.dtsi   | 12 
 drivers/base/devcon.c  | 43 +
 drivers/usb/gadget/udc/Kconfig |  1 +
 drivers/usb/gadget/udc/renesas_usb3.c  | 74 +-
 include/linux/device.h |  2 +
 7 files changed, 147 insertions(+), 4 deletions(-)

-- 
1.9.1

[PATCH/RFC v3 1/4] base: devcon: add a new API to find the graph

[Yoshihiro Shimoda]

This patch adds a new API "device_connection_find_by_graph()" to
find device connection by using graph.

Signed-off-by: Yoshihiro Shimoda 
---
 Documentation/driver-api/device_connection.rst |  4 +--
 drivers/base/devcon.c  | 43 ++
 include/linux/device.h |  2 ++
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/Documentation/driver-api/device_connection.rst 
b/Documentation/driver-api/device_connection.rst
index affbc556..2e2d26f 100644
--- a/Documentation/driver-api/device_connection.rst
+++ b/Documentation/driver-api/device_connection.rst
@@ -19,7 +19,7 @@ Device connections alone do not create a dependency between 
the two devices.
 They are only descriptions which are not tied to either of the devices 
directly.
 A dependency between the two devices exists only if one of the two endpoint
 devices requests a reference to the other. The descriptions themselves can be
-defined in firmware (not yet supported) or they can be built-in.
+defined in firmware or they can be built-in.
 
 Usage
 -
@@ -40,4 +40,4 @@ API
 ---
 
 .. kernel-doc:: drivers/base/devcon.c
-   : functions: device_connection_find_match device_connection_find 
device_connection_add device_connection_remove
+   : functions: device_connection_find_match device_connection_find 
device_connection_add device_connection_remove device_connection_find_by_graph
diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
index d427e80..5a0da33 100644
--- a/drivers/base/devcon.c
+++ b/drivers/base/devcon.c
@@ -7,6 +7,7 @@
  */
 
 #include 
+#include 
 
 static DEFINE_MUTEX(devcon_lock);
 static LIST_HEAD(devcon_list);
@@ -134,3 +135,45 @@ void device_connection_remove(struct device_connection 
*con)
mutex_unlock(_lock);
 }
 EXPORT_SYMBOL_GPL(device_connection_remove);
+
+static int generic_graph_match(struct device *dev, void *fwnode)
+{
+   return dev->fwnode == fwnode;
+}
+
+/**
+ * device_connection_find_by_graph - Find two devices connected together
+ * @dev: Device to find connected device
+ * @port: identifier of the @dev port node
+ * @endpoint: identifier of the @dev endpoint node
+ *
+ * Find a connection with @port and @endpoint by using graph between @dev and
+ * another device. On success returns handle to the device that is connected
+ * to @dev, with the reference count for the found device incremented. Returns
+ * NULL if no matching connection was found, or ERR_PTR(-EPROBE_DEFER) when
+ * a connection was found but the other device has not been enumerated yet.
+ */
+struct device *device_connection_find_by_graph(struct device *dev, u32 port,
+  u32 endpoint)
+{
+   struct bus_type *bus;
+   struct fwnode_handle *remote;
+   struct device *conn;
+
+   remote = fwnode_graph_get_remote_node(dev_fwnode(dev), port, endpoint);
+   if (!remote)
+   return NULL;
+
+   for (bus = generic_match_buses[0]; bus; bus++) {
+   conn = bus_find_device(bus, NULL, remote, generic_graph_match);
+   if (conn)
+   return conn;
+   }
+
+   /*
+* We only get called if a connection was found, tell the caller to
+* wait for the other device to show up.
+*/
+   return ERR_PTR(-EPROBE_DEFER);
+}
+EXPORT_SYMBOL_GPL(device_connection_find_by_graph);
diff --git a/include/linux/device.h b/include/linux/device.h
index 0059b99..58f8544 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -751,6 +751,8 @@ void *device_connection_find_match(struct device *dev, 
const char *con_id,
 
 void device_connection_add(struct device_connection *con);
 void device_connection_remove(struct device_connection *con);
+struct device *device_connection_find_by_graph(struct device *dev, u32 port,
+  u32 endpoint);
 
 /**
  * enum device_link_state - Device link states.
-- 
1.9.1

Re: [PATCH 2/5] media: rcar-vin: Add digital input subdevice parsing

[jacopo mondi]

Hi Niklas,

On Fri, May 11, 2018 at 01:01:24PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work!
>
> The comments here apply to both 2/5 and 3/5.
>
> On 2018-05-11 11:59:38 +0200, Jacopo Mondi wrote:
> > Add support for digital input subdevices to Gen-3 rcar-vin.
> > The Gen-3, media-controller compliant, version has so far only accepted
> > CSI-2 input subdevices. Remove assumptions on the supported bus_type and
> > accepted number of subdevices, and allow digital input connections on 
> > port@0.
>
> I feel this much more complex then it has to be. You defer parsing the
> DT graph until all VIN's are probed just like it's done for the port@1
> parsing. For the CSI-2 endpoints in port@1 this is needed as they are
> shared for all VIN's in the group. While for the parallel input this is
> local for each VIN and could be parsed at probe time for each individual
> VIN. As a bonus for doing that I think you could reuse the parallel
> parsing functions from the Gen2 code whit small additions.
>
> Maybe something like this can be done:
>
> - At each VIN probe() rework the block
>
> if (vin->info->use_mc)
>   ret = rvin_mc_init(vin);
> else
>   ret = rvin_digital_graph_init(vin);
>
>   To:
> ret = rvin_digital_graph_init(vin);
> ...
> ret = rvin_mc_init(vin);
> ...
>
> - Then in rvin_digital_graph_init() don't call
>   v4l2_async_notifier_register() if vin->info->use_mc is set.
>
>   And instead as a last step in rvin_mc_init() call
>   v4l2_async_notifier_register() for each vin->notifier that contains a
>   subdevice.
>
> - As a last step add a check for vin->info->use_mc in
>   rvin_digital_notify_complete() and handle the case for Gen2 behavior
>   (what it does now) and Gen3 behavior (adding the MC link).
>
>
> I think my doing this you could greatly simplify the process of handling
> the parallel subdevice.
>
> Please see bellow for one additional comment.

Thanks for the suggestion.
My understanding was that the 'Gen2' was planned for removal in the
long term, and we should have paved the road for a
media-controller-only version of the driver. But I admit I didn't even
try to re-use that code part, I'll give your suggestions a spin, if
this won't prevent future removal of the non-mc part of the driver.
>
> >
> > Signed-off-by: Jacopo Mondi 
> > ---
> >  drivers/media/platform/rcar-vin/rcar-core.c | 166 
> > +++-
> >  drivers/media/platform/rcar-vin/rcar-vin.h  |  13 +++
> >  2 files changed, 153 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> > b/drivers/media/platform/rcar-vin/rcar-core.c
> > index e547ef7..105b6b6 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > @@ -697,29 +697,21 @@ static const struct v4l2_async_notifier_operations 
> > rvin_group_notify_ops = {
> > .complete = rvin_group_notify_complete,

[snip]

> > +
> > +   switch (vep->base.port) {
> > +   case RVIN_PORT_DIGITAL:
> > +   ret = rvin_mc_parse_of_digital(vin, vep, asd);
> > +   break;
> > +   case RVIN_PORT_CSI2:
> > +   default:
> > +   ret = rvin_mc_parse_of_csi2(vin, vep, asd);
> > +   break;
> > +   }
> > +   /*

[snip]

> > +
> > +   switch (ep.port) {
> > +   case RVIN_PORT_DIGITAL:
> > +   asd_struct_size = sizeof(struct rvin_graph_entity);
> > +   break;
> > +   case RVIN_PORT_CSI2:
> > +   asd_struct_size = sizeof(struct v4l2_async_subdev);
> > +   break;
> > +   default:
> >  };

[snip]

> >
> >  /**
> > + * enum rvin_port_id
> > + *
> > + * List the available VIN port functions.
> > + *
> > + * RVIN_PORT_DIGITAL   - Input port for digital video connection
> > + * RVIN_PORT_CSI2  - Input port for CSI-2 video connection
> > + */
> > +enum rvin_port_id {
> > +   RVIN_PORT_DIGITAL,
> > +   RVIN_PORT_CSI2
> > +};
>
> This enum is never used in the patch-set is it used later by a different
> patch-set or a left over from refactoring?

I see two occurencies each of this enumeration members in this patch.
I left them un-cut above.

Thanks
   j


signature.asc
Description: PGP signature

Re: [GIT PULL] Renesas ARM Based SoC Fixes for v4.17

[Olof Johansson]

On Wed, May 02, 2018 at 12:18:14PM +0200, Simon Horman wrote:
> Hi Olof, Hi Kevin, Hi Arnd,
> 
> Please consider these Renesas ARM based SoC fixes for v4.17.
> 
> 
> The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338:
> 
>   Linux 4.17-rc1 (2018-04-15 18:24:20 -0700)
> 
> are available in the git repository at:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git 
> tags/renesas-fixes-for-v4.17
> 
> for you to fetch changes up to edb0c3affe5214a21d71ffb82ca92ed068e828df:
> 
>   ARM: dts: r8a7793: Convert to new LVDS DT bindings (2018-04-23 12:31:42 
> +0200)

Merged, thanks.


-Olof

Re: [PATCH v15 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver

[Maxime Ripard]

On Sun, May 13, 2018 at 09:19:17PM +0200, Niklas Söderlund wrote:
> A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver
> supports the 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 

Reviewed-by: Maxime Ripard 

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature

Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input

[jacopo mondi]

Hi Niklas,

On Sun, May 13, 2018 at 02:57:55PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your patch.
>
> On 2018-05-11 12:00:02 +0200, Jacopo Mondi wrote:
> > Describe HDMI input connected to VIN4 interface for R-Car D3 Draak
> > development board.
> >
> > Signed-off-by: Jacopo Mondi 
> > ---
> >  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 68 
> > ++
> >  1 file changed, 68 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts 
> > b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > index d03f194..e0ce462 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > @@ -59,6 +59,17 @@
> > };
> > };
> >
> > +   hdmi-in {
> > +   compatible = "hdmi-connector";
> > +   type = "a";
> > +
> > +   port {
> > +   hdmi_con_in: endpoint {
> > +   remote-endpoint = <_in>;
> > +   };
> > +   };
> > +   };
> > +
> > memory@4800 {
> > device_type = "memory";
> > /* first 128MB is reserved for secure area. */
> > @@ -142,6 +153,11 @@
> > groups = "usb0";
> > function = "usb0";
> > };
> > +
> > +   vin4_pins: vin4 {
> > +   groups = "vin4_data24", "vin4_sync", "vin4_clk", "vin4_clkenb";
> > +   function = "vin4";
> > +   };
> >  };
> >
> >   {
> > @@ -154,6 +170,35 @@
> > reg = <0x50>;
> > pagesize = <8>;
> > };
> > +
> > +   hdmi-decoder@4c {
> > +   compatible = "adi,adv7612";
> > +   reg = <0x4c>;
> > +   default-input = <0>;
> > +
> > +   ports {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   port@0 {
> > +   reg = <0>;
> > +   adv7612_in: endpoint {
> > +   remote-endpoint = <_con_in>;
> > +   };
> > +   };
> > +
> > +   port@2 {
> > +   reg = <2>;
> > +   adv7612_out: endpoint {
> > +   pclk-sample = <0>;
> > +   hsync-active = <0>;
> > +   vsync-active = <0>;
>
> This differs from the Gen2 DT bindings which is a very similar hardware
> setup using the same components. Defining these properties will make the
> bus marked as V4L2_MBUS_PARALLEL instead of V4L2_MBUS_BT656.

And that's what we want

>
> This will change how the hardware is configured for capture if the media
> bus is in a UYVY format, see VNMC_INF register in rvin_setup(). Maybe
> this it not an issue here but still I'm curious to why this differ
> between Gen2 and Gen3 :-)

Actually this won't impact the VIN configuration as this is the
'remote endpoint' from VIN perspective and the properties used to
configure the interface are the ones in the 'local endpoint'.

>
> > +
> > +   remote-endpoint = <_in>;
> > +   };
> > +   };
> > +   };
> > +   };
> >  };
> >
> >   {
> > @@ -246,3 +291,26 @@
> > timeout-sec = <60>;
> > status = "okay";
> >  };
> > +
> > + {
> > +   pinctrl-0 = <_pins>;
> > +   pinctrl-names = "default";
> > +
> > +   status = "okay";
> > +
> > +   ports {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   port@0 {
> > +   reg = <0>;
> > +
> > +   vin4_in: endpoint {
> > +   hsync-active = <0>;
> > +   vsync-active = <0>;
>
> Comparing this to the Gen2 bindings some properties are missing,
>
> bus-width = <24>;
> pclk-sample = <1>;
> data-active = <1>;

The VIN driver does not parse them, so there is no value in having
them there, if not confusing people as it happened to me reading the
Gen2 DT.

>
> This is not a big deal as the VIN driver don't use these properties so
> no functional change should come of this but still a difference.

Exactly.

On a side note. I have not seen a way to configure the pixel clock
sampling level in the interface datasheet. The register used to
configure synchronism signals polarities is VnDMR2, and there I read
we can configure HSYNC/VSYNC and CLOCKENB (which is data enable, not
pixel clock) polarities. Is it configured through some other
register?
>
> Over all I'm happy with this change but before I add my tag I would like
> to understand why it differs from the Gen2 configuration for the adv7612
> properties.
>
> Also on a side not it is possible with hardware switches on the board
> switch the VIN4 source to a completely different pipeline CVBS connector
> -> adv7180 -> VIN4. But I think it's best we keep the HDMI as default as
> this seems to be