[PATCH 0/3] arm64: dts: renesas: r8a77970: connect VIN to CSI40

2017-11-25 Thread Niklas Söderlund
Hi,

This series describes how to connect VIN0-3 to CSI40 and the HDMI input 
of the adv7482 on the expansion board. It is tested on V3M together with 
V3M enablement for VIN and CSI-2 drivers on-top of Kieran's 
renesas-drivers-next-v3m branch [1].

It works and it's possible to program the EDID and capture frame frames.  
Remember to switch SW18 to the off position to route the CSI-2 bus to 
the expansion board instead of the MAX9286 for GMSL input.

1. git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git

Niklas Söderlund (3):
  arm64: dts: renesas: r8a77970: add VIN and CSI-2
  arm64: dts: renesas: eagle: expansion: enable and connect CSI40
  arm64: dts: renesas: eagle: enable VIN

 .../boot/dts/renesas/r8a77970-eagle-expansion.dtsi |  18 ++-
 arch/arm64/boot/dts/renesas/r8a77970-eagle.dts |  16 +++
 arch/arm64/boot/dts/renesas/r8a77970.dtsi  | 151 +
 3 files changed, 184 insertions(+), 1 deletion(-)

-- 
2.15.0



[PATCH 2/3] arm64: dts: renesas: eagle: expansion: enable and connect CSI40

2017-11-25 Thread Niklas Söderlund
On the extension board CSI40 is connected to the adv7482.

Signed-off-by: Niklas Söderlund 
---
 .../boot/dts/renesas/r8a77970-eagle-expansion.dtsi | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle-expansion.dtsi 
b/arch/arm64/boot/dts/renesas/r8a77970-eagle-expansion.dtsi
index e3dc5c32433014c9..565008595a6ce81b 100644
--- a/arch/arm64/boot/dts/renesas/r8a77970-eagle-expansion.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle-expansion.dtsi
@@ -44,6 +44,22 @@
};
 };
 
+ {
+   status = "okay";
+
+   ports {
+   port@0 {
+   reg = <0>;
+
+   csi40_in: endpoint {
+   clock-lanes = <0>;
+   data-lanes = <1 2 3 4>;
+   remote-endpoint = <_txa>;
+   };
+   };
+   };
+};
+
  {
/* GPIO Expander@27:
 *   IO.0: ROUTE_I2C_ENn
@@ -94,7 +110,7 @@
adv7482_txa: endpoint {
clock-lanes = <0>;
data-lanes = <1 2 3 4>;
-   // remote-endpoint = <_in>;
+   remote-endpoint = <_in>;
};
};
};
-- 
2.15.0



[PATCH 3/3] arm64: dts: renesas: eagle: enable VIN

2017-11-25 Thread Niklas Söderlund
Signed-off-by: Niklas Söderlund 
---
 arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 16 
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts 
b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
index 41e3b4521a5aa39e..d3c6bcb82a5176c6 100644
--- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
@@ -226,5 +226,21 @@
status = "okay";
 };
 
+ {
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
 /* FAKRA Overlay */
 #include "eagle-fakra.dtsi"
\ No newline at end of file
-- 
2.15.0



[PATCH 1/3] arm64: dts: renesas: r8a77970: add VIN and CSI-2

2017-11-25 Thread Niklas Söderlund
Define the nodes for CSI40 and VIN0-3.

Signed-off-by: Niklas Söderlund 
---
 arch/arm64/boot/dts/renesas/r8a77970.dtsi | 151 ++
 1 file changed, 151 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77970.dtsi 
b/arch/arm64/boot/dts/renesas/r8a77970.dtsi
index b70649b7acd05c98..505a9a9c0c4f5a68 100644
--- a/arch/arm64/boot/dts/renesas/r8a77970.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77970.dtsi
@@ -624,5 +624,156 @@
#size-cells = <0>;
status = "disabled";
};
+
+   csi40: csi2@feaa {
+   compatible = "renesas,r8a7795-csi2", 
"renesas,rcar-gen3-csi2";
+   reg = <0 0xfeaa 0 0x1>;
+   interrupts = ;
+   clocks = < CPG_MOD 716>;
+   power-domains = < R8A77970_PD_ALWAYS_ON>;
+   resets = < 716>;
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   reg = <1>;
+
+   csi40vin0: endpoint@0 {
+   reg = <0>;
+   remote-endpoint = <>;
+   };
+   csi40vin1: endpoint@1 {
+   reg = <1>;
+   remote-endpoint = <>;
+   };
+   csi40vin2: endpoint@2 {
+   reg = <2>;
+   remote-endpoint = <>;
+   };
+   csi40vin3: endpoint@3 {
+   reg = <3>;
+   remote-endpoint = <>;
+   };
+   };
+   };
+   };
+
+   vin0: video@e6ef {
+   compatible = "renesas,vin-r8a7795";
+   reg = <0 0xe6ef 0 0x1000>;
+   interrupts = ;
+   clocks = < CPG_MOD 811>;
+   power-domains = < R8A77970_PD_ALWAYS_ON>;
+   resets = < 811>;
+   renesas,id = <0>;
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   reg = <1>;
+
+   vin0csi40: endpoint@2 {
+   reg = <2>;
+   remote-endpoint= <>;
+   };
+   };
+   };
+   };
+
+   vin1: video@e6ef1000 {
+   compatible = "renesas,vin-r8a7795";
+   reg = <0 0xe6ef1000 0 0x1000>;
+   interrupts = ;
+   clocks = < CPG_MOD 810>;
+   power-domains = < R8A77970_PD_ALWAYS_ON>;
+   resets = < 810>;
+   renesas,id = <1>;
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   reg = <1>;
+
+   vin1csi40: endpoint@2 {
+   reg = <2>;
+   remote-endpoint= <>;
+   };
+   };
+   };
+   };
+
+   vin2: video@e6ef2000 {
+   compatible = "renesas,vin-r8a7795";
+   reg = <0 0xe6ef2000 0 0x1000>;
+   interrupts = ;
+   clocks = < CPG_MOD 809>;
+   power-domains = < R8A77970_PD_ALWAYS_ON>;
+   resets = < 809>;
+  

[PATCH 0/2] rcar-csi2: enable support for r8a77970

2017-11-25 Thread Niklas Söderlund
Hi,

This series adds Renesas r8a77970 (V3M) support to rcar-csi2. It is 
tested on V3M using the HDMI input connected to the adv748x on the V3M 
extension board. It depends on the series 'rcar-csi2: add Renesas R-Car 
MIPI CSI-2'.  

There are many different types of video sources and pipelines on the V3M 
and this is only tested with one of them as there are other none 
rcar-csi2 related issues in testing all of them. Therefor I do not wish 
to squash these changes into the series it depends on which adds the 
rcar-csi2 driver.

It's my firm belief that this is all that is needed to add CSI-2 support 
on V3M but it would be great if more pipelines where tested before this 
series is picked up. Therefor I post it here so other developers working 
on video capture on V3M can use this for CSI-2 support.

Niklas Söderlund (2):
  rcar-csi2: split Mbps calculation to separate function
  rcar-csi2: enable support for r8a77970

 .../bindings/media/renesas,rcar-csi2.txt   |   1 +
 drivers/media/platform/rcar-vin/rcar-csi2.c| 126 ++---
 2 files changed, 114 insertions(+), 13 deletions(-)

-- 
2.15.0



[PATCH 1/2] rcar-csi2: split Mbps calculation to separate function

2017-11-25 Thread Niklas Söderlund
The calculation of the Mbps to configured the link with needs to be
split out if other configuration procedures on new SoCs are to be able
to reuse the calculation procedure.

Signed-off-by: Niklas Söderlund 
---
 drivers/media/platform/rcar-vin/rcar-csi2.c | 32 +
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c 
b/drivers/media/platform/rcar-vin/rcar-csi2.c
index f1363cc58eb74977..9afbd03d2dedf4cb 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -348,11 +348,8 @@ static int rcar_csi2_wait_phy_start(struct rcar_csi2 *priv)
return -ETIMEDOUT;
 }
 
-static int rcar_csi2_calc_phypll(struct rcar_csi2 *priv, unsigned int bpp,
-u32 *phypll)
+static int rcar_csi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp)
 {
-
-   const struct phypll_hsfreqrange *hsfreq;
struct media_pad *pad, *source_pad;
struct v4l2_subdev *source = NULL;
struct v4l2_ctrl *ctrl;
@@ -382,19 +379,26 @@ static int rcar_csi2_calc_phypll(struct rcar_csi2 *priv, 
unsigned int bpp,
mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * bpp;
do_div(mbps, priv->lanes * 100);
 
+   return mbps;
+}
+
+static int rcar_csi2_set_phypll(struct rcar_csi2 *priv, unsigned int mbps)
+{
+   const struct phypll_hsfreqrange *hsfreq;
+
for (hsfreq = priv->info->hsfreqrange; hsfreq->mbps != 0; hsfreq++)
if (hsfreq->mbps >= mbps)
break;
 
if (!hsfreq->mbps) {
-   dev_err(priv->dev, "Unsupported PHY speed (%llu Mbps)", mbps);
+   dev_err(priv->dev, "Unsupported PHY speed (%u Mbps)", mbps);
return -ERANGE;
}
 
-   dev_dbg(priv->dev, "PHY HSFREQRANGE requested %llu got %u Mbps\n", mbps,
+   dev_dbg(priv->dev, "PHY HSFREQRANGE requested %u got %u Mbps\n", mbps,
hsfreq->mbps);
 
-   *phypll = PHYPLL_HSFREQRANGE(hsfreq->reg);
+   rcar_csi2_write(priv, PHYPLL_REG, PHYPLL_HSFREQRANGE(hsfreq->reg));
 
return 0;
 }
@@ -402,10 +406,10 @@ static int rcar_csi2_calc_phypll(struct rcar_csi2 *priv, 
unsigned int bpp,
 static int rcar_csi2_start(struct rcar_csi2 *priv)
 {
const struct rcar_csi2_format *format;
-   u32 phycnt, phypll, tmp;
+   u32 phycnt, tmp;
u32 vcdt = 0, vcdt2 = 0;
unsigned int i;
-   int ret;
+   int mbps, ret;
 
dev_dbg(priv->dev, "Input size (%ux%u%c)\n",
priv->mf.width, priv->mf.height,
@@ -447,9 +451,9 @@ static int rcar_csi2_start(struct rcar_csi2 *priv)
return -EINVAL;
}
 
-   ret = rcar_csi2_calc_phypll(priv, format->bpp, );
-   if (ret)
-   return ret;
+   mbps = rcar_csi2_calc_mbps(priv, format->bpp);
+   if (mbps < 0)
+   return mbps;
 
/* Clear Ultra Low Power interrupt */
if (priv->info->clear_ulps)
@@ -490,7 +494,9 @@ static int rcar_csi2_start(struct rcar_csi2 *priv)
}
 
/* Start */
-   rcar_csi2_write(priv, PHYPLL_REG, phypll);
+   ret = rcar_csi2_set_phypll(priv, mbps);
+   if (ret)
+   return ret;
 
/* Set frequency range if we have it */
if (priv->info->csi0clkfreqrange)
-- 
2.15.0



[PATCH 2/2] rcar-csi2: enable support for r8a77970

2017-11-25 Thread Niklas Söderlund
Enable support for Renesas r8a77970 (V3M) by adding the new SoC specific
setup procedure and device tree binding.

Signed-off-by: Niklas Söderlund 
---
 .../bindings/media/renesas,rcar-csi2.txt   |   1 +
 drivers/media/platform/rcar-vin/rcar-csi2.c| 100 -
 2 files changed, 98 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt 
b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
index 688afd83bf66f8cf..2dd0a20e59d4c294 100644
--- a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
+++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
@@ -10,6 +10,7 @@ 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,r8a77970-csi2" for the R8A77970 device.
 
  - reg: the register base and size for the device registers
  - interrupts: the interrupt for the device
diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c 
b/drivers/media/platform/rcar-vin/rcar-csi2.c
index 9afbd03d2dedf4cb..5345f4b1e30226e9 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -118,6 +118,11 @@
 
 /* PHY Test Interface Write Register */
 #define PHTW_REG   0x50
+#define PHTW_DWEN  BIT(24)
+#define PHTW_TESTDIN_DATA(n)   (((n & 0xff)) << 16)
+#define PHTW_CWEN  BIT(8)
+#define PHTW_TESTDIN_CODE(n)   ((n & 0xff))
+
 
 /* PHY Test Interface Clear */
 #define PHTC_REG   0x58
@@ -228,6 +233,55 @@ static const struct phypll_hsfreqrange 
hsfreqrange_m3w_h3es1[] = {
{ .mbps =   0,  .reg = 0x00 },
 };
 
+struct phtw_testdin_data {
+   u16 mbps;
+   u16 reg;
+};
+
+static const struct phtw_testdin_data testdin_data_v3m_e3[] = {
+   { .mbps =   80, .reg = 0x00 },
+   { .mbps =   90, .reg = 0x20 },
+   { .mbps =  100, .reg = 0x40 },
+   { .mbps =  110, .reg = 0x02 },
+   { .mbps =  130, .reg = 0x22 },
+   { .mbps =  140, .reg = 0x42 },
+   { .mbps =  150, .reg = 0x04 },
+   { .mbps =  170, .reg = 0x24 },
+   { .mbps =  180, .reg = 0x44 },
+   { .mbps =  200, .reg = 0x06 },
+   { .mbps =  220, .reg = 0x26 },
+   { .mbps =  240, .reg = 0x46 },
+   { .mbps =  250, .reg = 0x08 },
+   { .mbps =  270, .reg = 0x28 },
+   { .mbps =  300, .reg = 0x0a },
+   { .mbps =  330, .reg = 0x2a },
+   { .mbps =  360, .reg = 0x4a },
+   { .mbps =  400, .reg = 0x0c },
+   { .mbps =  450, .reg = 0x2c },
+   { .mbps =  500, .reg = 0x0e },
+   { .mbps =  550, .reg = 0x2e },
+   { .mbps =  600, .reg = 0x10 },
+   { .mbps =  650, .reg = 0x30 },
+   { .mbps =  700, .reg = 0x12 },
+   { .mbps =  750, .reg = 0x32 },
+   { .mbps =  800, .reg = 0x52 },
+   { .mbps =  850, .reg = 0x72 },
+   { .mbps =  900, .reg = 0x14 },
+   { .mbps =  950, .reg = 0x34 },
+   { .mbps = 1000, .reg = 0x54 },
+   { .mbps = 1050, .reg = 0x74 },
+   { .mbps = 1100, .reg = 0x16 },
+   { .mbps = 1150, .reg = 0x36 },
+   { .mbps = 1200, .reg = 0x56 },
+   { .mbps = 1250, .reg = 0x76 },
+   { .mbps = 1300, .reg = 0x18 },
+   { .mbps = 1350, .reg = 0x38 },
+   { .mbps = 1400, .reg = 0x58 },
+   { .mbps = 1500, .reg = 0x78 },
+   /* guard */
+   { .mbps =   0,  .reg = 0x00 },
+};
+
 /* PHY ESC Error Monitor */
 #define PHEERM_REG 0x74
 
@@ -276,6 +330,7 @@ enum rcar_csi2_pads {
 
 struct rcar_csi2_info {
const struct phypll_hsfreqrange *hsfreqrange;
+   const struct phtw_testdin_data *testdin_data;
unsigned int csi0clkfreqrange;
bool clear_ulps;
bool init_phtw;
@@ -403,6 +458,29 @@ static int rcar_csi2_set_phypll(struct rcar_csi2 *priv, 
unsigned int mbps)
return 0;
 }
 
+static int rcar_csi2_set_phtw(struct rcar_csi2 *priv, unsigned int mbps)
+{
+   const struct phtw_testdin_data *testdin;
+
+   for (testdin = priv->info->testdin_data; testdin->mbps != 0; testdin++)
+   if (testdin->mbps >= mbps)
+   break;
+
+   if (!testdin->mbps) {
+   dev_err(priv->dev, "Unsupported PHY speed (%u Mbps)", mbps);
+   return -ERANGE;
+   }
+
+   dev_dbg(priv->dev, "PHY TESTDIN_DATA requested %u got %u Mbps\n", mbps,
+   testdin->mbps);
+
+   rcar_csi2_write(priv, PHTW_REG,
+   PHTW_DWEN | PHTW_TESTDIN_DATA(testdin->reg) |
+   PHTW_CWEN | PHTW_TESTDIN_CODE(0x44));
+
+   return 0;
+}
+
 static int rcar_csi2_start(struct rcar_csi2 *priv)
 {
const struct rcar_csi2_format *format;
@@ -494,9 +572,17 @@ static int rcar_csi2_start(struct rcar_csi2 *priv)
}
 
/* Start */
-   ret 

[PATCH] rcar-vin: enable support for r8a77970

2017-11-25 Thread Niklas Söderlund
Add the SoC specific information for Renesas r8a77970.

Signed-off-by: Niklas Söderlund 
---
 .../devicetree/bindings/media/rcar_vin.txt |  1 +
 drivers/media/platform/rcar-vin/rcar-core.c| 40 ++
 2 files changed, 41 insertions(+)

Hi,

I plan to include this patch in the next version of the series for Gen3 
support. But to enable who wish to start working with VIN on V3M I
send it out now as a separate patch as the big Gen3 series is waiting 
for -rc1 to be available before being re-posted.

Regards,
Niklas

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
b/Documentation/devicetree/bindings/media/rcar_vin.txt
index 17e0d642320fa90d..276af4b4b47703f6 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -19,6 +19,7 @@ on Gen3 to a CSI-2 receiver.
- "renesas,vin-r8a7794" for the R8A7794 device
- "renesas,vin-r8a7795" for the R8A7795 device
- "renesas,vin-r8a7796" for the R8A7796 device
+   - "renesas,vin-r8a77970" for the R8A77970 device
- "renesas,rcar-gen2-vin" for a generic R-Car Gen2 compatible device.
 
When compatible with the generic version nodes must list the
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
b/drivers/media/platform/rcar-vin/rcar-core.c
index e042cb12229d23d6..a13bc8325a1d1309 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -1149,6 +1149,42 @@ static const struct rvin_info rcar_info_r8a7796 = {
},
 };
 
+static const struct rvin_info rcar_info_r8a77970 = {
+   .chip = RCAR_GEN3,
+   .use_mc = true,
+   .max_width = 4096,
+   .max_height = 4096,
+
+   .num_chsels = 5,
+   .chsels = {
+   {
+   { .csi = RVIN_CSI40, .chan = 0 },
+   { .csi = RVIN_NC, .chan = 0 },
+   { .csi = RVIN_NC, .chan = 0 },
+   { .csi = RVIN_CSI40, .chan = 0 },
+   { .csi = RVIN_NC, .chan = 0 },
+   }, {
+   { .csi = RVIN_NC, .chan = 0 },
+   { .csi = RVIN_NC, .chan = 0 },
+   { .csi = RVIN_CSI40, .chan = 0 },
+   { .csi = RVIN_CSI40, .chan = 1 },
+   { .csi = RVIN_NC, .chan = 0 },
+   }, {
+   { .csi = RVIN_NC, .chan = 0 },
+   { .csi = RVIN_CSI40, .chan = 0 },
+   { .csi = RVIN_NC, .chan = 0 },
+   { .csi = RVIN_CSI40, .chan = 2 },
+   { .csi = RVIN_NC, .chan = 0 },
+   }, {
+   { .csi = RVIN_CSI40, .chan = 1 },
+   { .csi = RVIN_NC, .chan = 0 },
+   { .csi = RVIN_NC, .chan = 0 },
+   { .csi = RVIN_CSI40, .chan = 3 },
+   { .csi = RVIN_NC, .chan = 0 },
+   },
+   },
+};
+
 static const struct of_device_id rvin_of_id_table[] = {
{
.compatible = "renesas,vin-r8a7778",
@@ -1186,6 +1222,10 @@ static const struct of_device_id rvin_of_id_table[] = {
.compatible = "renesas,vin-r8a7796",
.data = _info_r8a7796,
},
+   {
+   .compatible = "renesas,vin-r8a77970",
+   .data = _info_r8a77970,
+   },
{ },
 };
 MODULE_DEVICE_TABLE(of, rvin_of_id_table);
-- 
2.15.0



Re: [PATCH v1 03/10] v4l: platform: Add Renesas CEU driver

2017-11-25 Thread jacopo mondi
Hi Sakari!

On Sat, Nov 25, 2017 at 05:56:14PM +0200, Sakari Ailus wrote:
> On Fri, Nov 17, 2017 at 10:33:55AM +0100, jacopo mondi wrote:
> > Hi Sakari!
> >

[snip]

> > I would like to make sure we're all on the same page with this. My
> > preference would be:
> >
> > 1) Have renesas-ceu.c driver merged with Migo-R ported to use this new
> > driver as an 'example'.
> > 2) Do not remove any of the existing soc_camera code at this point
> > 3) Port all other 4 SH users of sh_mobile_ceu_camera to use the now
> > merged renesas-ceu driver
> > 4) Remove sh_mobile_ceu_camera and soc_camera sensor drivers whose
> > only users were those 4 SH boards
> > 5) Remove soc_camera completely. For my understanding there are some
> > PXA platforms still using soc_camera provided utilities somewhere.
> > Hans knows better, but we can discuss this once we'll get there.
>
> The first point here is basically done by this patchset and your intent
> would be to proceed with the rest, right?

Yep, you're right!

>
> The above seems good; what I wanted to say was that I'd like to avoid
> ending up in a permanent situation where some hardware works with the new
> driver and some will continue to use the old one.

I hope that being the last users of soc_camera, there will be enough
motivations to complete all the above points :)

Thanks
   j

>
> --
> Kind regards,
>
> Sakari Ailus
> e-mail: sakari.ai...@iki.fi


Re: [PATCH v1 08/10] media: i2c: ov772x: Remove soc_camera dependencies

2017-11-25 Thread Sakari Ailus
On Fri, Nov 17, 2017 at 10:14:51AM +0100, jacopo mondi wrote:
> Hi Sakari!
> 
> On Fri, Nov 17, 2017 at 02:43:15AM +0200, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > On Wed, Nov 15, 2017 at 11:56:01AM +0100, Jacopo Mondi wrote:
> > >
> 
> [snip]
> 
> > > +#include 
> > >  #include 
> > >  #include 
> > >  #include 
> > > @@ -25,8 +26,8 @@
> > >  #include 
> > >
> > >  #include 
> > > -#include 
> > > -#include 
> > > +
> > > +#include 
> >
> > Alphabetical order would be nice.
> 
> ups!
> 
> >
> > >  #include 
> > >  #include 
> > >  #include 
> > > @@ -393,7 +394,7 @@ struct ov772x_win_size {
> > >  struct ov772x_priv {
> > >   struct v4l2_subdevsubdev;
> > >   struct v4l2_ctrl_handler  hdl;
> > > - struct v4l2_clk  *clk;
> > > + struct clk   *clk;
> > >   struct ov772x_camera_info*info;
> > >   const struct ov772x_color_format *cfmt;
> > >   const struct ov772x_win_size *win;
> > > @@ -550,7 +551,7 @@ static int ov772x_reset(struct i2c_client *client)
> > >  }
> > >
> > >  /*
> > > - * soc_camera_ops function
> > > + * subdev ops
> > >   */
> > >
> > >  static int ov772x_s_stream(struct v4l2_subdev *sd, int enable)
> > > @@ -650,13 +651,36 @@ static int ov772x_s_register(struct v4l2_subdev *sd,
> > >  }
> > >  #endif
> > >
> > > +static int ov772x_power_on(struct ov772x_priv *priv)
> > > +{
> > > + int ret;
> > > +
> > > + if (priv->info->platform_enable) {
> > > + ret = priv->info->platform_enable();
> > > + if (ret)
> > > + return ret;
> >
> > What does this do, enable the regulator?
> 
> Well, it depends on what function the platform code stores in
> 'platform_enable' pointer, doesn't it?
> 
> As you can see in [05/10] of this series, for Migo-R it's not about
> a regulator, but switching between the two available video inputs
> (OV7725 and TW9910) toggling their 'enable' pins.

Ok. That's not a very nice design.

Fair enough. I guess it's good to proceed one thing at a time.

If someone has this sensor on a board with DT support, we can use the
regulator framework and just ignore the platform callbacks.

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


Re: [PATCH v1 03/10] v4l: platform: Add Renesas CEU driver

2017-11-25 Thread Sakari Ailus
On Fri, Nov 17, 2017 at 10:33:55AM +0100, jacopo mondi wrote:
> Hi Sakari!
> 
> On Fri, Nov 17, 2017 at 02:36:51AM +0200, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > On Wed, Nov 15, 2017 at 03:25:11PM +0100, jacopo mondi wrote:
> > > Hi Sakari,
> > >thanks for review!
> >
> > You're welcome!
> >
> > > On Wed, Nov 15, 2017 at 02:45:51PM +0200, Sakari Ailus wrote:
> > > > Hi Jacopo,
> > > >
> > > > Could you remove the original driver and send the patch using git
> > > > send-email -C ? That way a single patch would address converting it to a
> > > > proper V4L2 driver as well as move it to the correct location. The 
> > > > changes
> > > > would be easier to review that way since then, well, it'd be easier to 
> > > > see
> > > > the changes. :-)
> > >
> > > Actually I prefer not to remove the existing driver at the moment. See
> > > the cover letter for reasons why not to do so right now...
> >
> > So it's about testing mostly? Does someone (possibly you) have those boards
> > to test? I'd like to see this patchset to remove that last remaining SoC
> > camera bridge driver. :-)
> 
> Well, we agreed that for most of those platforms, compile testing it
> would be enough (let's believe in "if it compiles, it works"). I
> personally don't have access to those boards, and frankly I'm not even
> sure there are many of them around these days (I guess most of them
> are not even produced anymore).
> 
> >
> > >
> > > Also, there's not that much code from the old driver in here, surely
> > > less than the default 50% -C and -M options of 'git format-patch' use
> > > as a threshold for detecting copies iirc..
> >
> > Oh, if that's so, then makes sense to review it as a new driver.
> 
> thanks :)
> 
> >
> > >
> > > > The same goes for the two V4L2 SoC camera sensor / video decoder 
> > > > drivers at
> > > > the end of the set.
> > > >
> > >
> > > Also in this case I prefer not to remove existing code, as long as
> > > there are platforms using it..
> >
> > Couldn't they use this driver instead?
> 
> Oh, they will eventually, I hope :)
> 
> I would like to make sure we're all on the same page with this. My
> preference would be:
> 
> 1) Have renesas-ceu.c driver merged with Migo-R ported to use this new
> driver as an 'example'.
> 2) Do not remove any of the existing soc_camera code at this point
> 3) Port all other 4 SH users of sh_mobile_ceu_camera to use the now
> merged renesas-ceu driver
> 4) Remove sh_mobile_ceu_camera and soc_camera sensor drivers whose
> only users were those 4 SH boards
> 5) Remove soc_camera completely. For my understanding there are some
> PXA platforms still using soc_camera provided utilities somewhere.
> Hans knows better, but we can discuss this once we'll get there.

The first point here is basically done by this patchset and your intent
would be to proceed with the rest, right?

The above seems good; what I wanted to say was that I'd like to avoid
ending up in a permanent situation where some hardware works with the new
driver and some will continue to use the old one.

-- 
Kind regards,

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