Re: 'bad remote port parent' warnings

[Fabio Estevam]

Hi Steve,

On Fri, Nov 23, 2018 at 8:37 PM Steve Longerbeam  wrote:

> Yes, this is a regression caused by the imx subdev notifier patches.
> I've already sent a patch to the list for this, see
>
> https://www.spinics.net/lists/linux-media/msg141809.html

Thanks, this fixes it.

Hopefully it will be applied as a fix for 4.20.

Thanks

Re: 'bad remote port parent' warnings

[Fabio Estevam]

Hi Sakari,

On Fri, Nov 23, 2018 at 10:35 AM Sakari Ailus
 wrote:

> Makes sense. This is not necessarily a fatal error. Could you send a patch?

Yes, I have just sent it.

Thanks

[PATCH] media: v4l2-fwnode: Demote warning to debug level

[Fabio Estevam]

On a imx6q-wandboard the following warnings are observed:

[4.327794] video-mux 20e.iomuxc-gpr:ipu1_csi0_mux: bad remote port 
parent
[4.336118] video-mux 20e.iomuxc-gpr:ipu2_csi1_mux: bad remote port 
parent

As explained by Philipp Zabel:

"There are empty endpoint nodes (without remote-endpoint property)
labeled ipu1_csi[01]_mux_from_parallel_sensor in the i.MX6 device trees
for board DT implementers' convenience. See commit 2539f517acbdc ("ARM:
dts: imx6qdl: Add video multiplexers, mipi_csi, and their connections")."

So demote the warning to debug level and make the wording a bit
less misleading.

Suggested-by: Philipp Zabel 
Signed-off-by: Fabio Estevam 
---
 drivers/media/v4l2-core/v4l2-fwnode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
b/drivers/media/v4l2-core/v4l2-fwnode.c
index 218f0da..7a3cc10 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -613,7 +613,7 @@ v4l2_async_notifier_fwnode_parse_endpoint(struct device 
*dev,
asd->match.fwnode =
fwnode_graph_get_remote_port_parent(endpoint);
if (!asd->match.fwnode) {
-   dev_warn(dev, "bad remote port parent\n");
+   dev_dbg(dev, "no remote endpoint found\n");
ret = -ENOTCONN;
goto out_err;
}
-- 
2.7.4

Re: 'bad remote port parent' warnings

[Fabio Estevam]

Hi Philipp,

On Thu, Nov 22, 2018 at 2:27 PM Philipp Zabel  wrote:

> There are empty endpoint nodes (without remote-endpoint property)
> labeled ipu1_csi[01]_mux_from_parallel_sensor in the i.MX6 device trees
> for board DT implementers' convenience. See commit 2539f517acbdc ("ARM:
> dts: imx6qdl: Add video multiplexers, mipi_csi, and their connections").
>
> We had a discussion about this issue in February when this caused a
> probing error: https://patchwork.kernel.org/patch/10234469/

Thanks for the clarification.

 We could demote the warning to a debug message, make the wording a bit
> less misleading (there is no bad remote port parent, there is just no
> remote endpoint at all), or we could just accept the error message for

Something like this?

--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -613,7 +613,7 @@ v4l2_async_notifier_fwnode_parse_endpoint(struct
device *dev,
asd->match.fwnode =
fwnode_graph_get_remote_port_parent(endpoint);
if (!asd->match.fwnode) {
-   dev_warn(dev, "bad remote port parent\n");
+   dev_dbg(dev, "no remote endpoint found\n");
ret = -ENOTCONN;
goto out_err;
}

And how should we treat these error probes?

[3.449564] imx-ipuv3 240.ipu: driver could not parse
port@1/endpoint@0 (-22)
[3.457342] imx-ipuv3-csi: probe of imx-ipuv3-csi.1 failed with error -22
[3.464498] imx-ipuv3 280.ipu: driver could not parse
port@0/endpoint@0 (-22)
[3.472120] imx-ipuv3-csi: probe of imx-ipuv3-csi.4 failed with error -22

Thanks

Re: 'bad remote port parent' warnings

[Fabio Estevam]

Hi Sakari,

On Tue, Nov 20, 2018 at 10:15 AM Sakari Ailus
 wrote:

> Where's the DT source for the board?

Board dts is arch/arm/boot/dts/imx6qdl-wandboard.dtsi

SoC dtsi is arch/arm/boot/dts/imx6q.dtsi

Also, since 4.20-rc the following errors are seen:

[3.449564] imx-ipuv3 240.ipu: driver could not parse
port@1/endpoint@0 (-22)
[3.457342] imx-ipuv3-csi: probe of imx-ipuv3-csi.1 failed with error -22
[3.464498] imx-ipuv3 280.ipu: driver could not parse
port@0/endpoint@0 (-22)
[3.472120] imx-ipuv3-csi: probe of imx-ipuv3-csi.4 failed with error -22

which were not present in 4.19.

Log from 4.19:
https://storage.kernelci.org/stable/linux-4.19.y/v4.19.2/arm/imx_v6_v7_defconfig/lab-baylibre-seattle/boot-imx6q-wandboard.html

Log from 4.20-rc3:
https://storage.kernelci.org/mainline/master/v4.20-rc3/arm/imx_v6_v7_defconfig/lab-baylibre-seattle/boot-imx6q-wandboard.html

Thanks

'bad remote port parent' warnings

[Fabio Estevam]

Hi,

On a imx6q-wandboard running linux-next 20181120 there the following warnings:

[4.327794] video-mux 20e.iomuxc-gpr:ipu1_csi0_mux: bad remote
port parent
[4.336118] video-mux 20e.iomuxc-gpr:ipu2_csi1_mux: bad remote
port parent

Is there anything we should do to prevent this from happening?

Thanks,

Fabio Estevam

[PATCH v3 2/3] media: imx-pxp: Check for pxp_soft_reset() error

[Fabio Estevam]

pxp_soft_reset() may fail with a timeout, so it is better to propagate
the error in this case.

Signed-off-by: Fabio Estevam 
Reviewed-by: Philipp Zabel 
---
Changes since v2:
- Jump to err_clck when pxp_soft_reset() fails. (Philipp)

 drivers/media/platform/imx-pxp.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/imx-pxp.c b/drivers/media/platform/imx-pxp.c
index 27780f1..986764d 100644
--- a/drivers/media/platform/imx-pxp.c
+++ b/drivers/media/platform/imx-pxp.c
@@ -1607,7 +1607,7 @@ static const struct v4l2_m2m_ops m2m_ops = {
.job_abort  = pxp_job_abort,
 };
 
-static void pxp_soft_reset(struct pxp_dev *dev)
+static int pxp_soft_reset(struct pxp_dev *dev)
 {
int ret;
u32 val;
@@ -1619,11 +1619,15 @@ static void pxp_soft_reset(struct pxp_dev *dev)
 
ret = readl_poll_timeout(dev->mmio + HW_PXP_CTRL, val,
 val & BM_PXP_CTRL_CLKGATE, 0, 100);
-   if (ret < 0)
+   if (ret < 0) {
pr_err("PXP reset timeout\n");
+   return ret;
+   }
 
writel(BM_PXP_CTRL_SFTRST, dev->mmio + HW_PXP_CTRL_CLR);
writel(BM_PXP_CTRL_CLKGATE, dev->mmio + HW_PXP_CTRL_CLR);
+
+   return 0;
 }
 
 static int pxp_probe(struct platform_device *pdev)
@@ -1670,7 +1674,9 @@ static int pxp_probe(struct platform_device *pdev)
if (ret < 0)
return ret;
 
-   pxp_soft_reset(dev);
+   ret = pxp_soft_reset(dev);
+   if (ret < 0)
+   goto err_clk;
 
spin_lock_init(>irqlock);
 
-- 
2.7.4

[PATCH v3 1/3] media: imx-pxp: Check the return value from clk_prepare_enable()

[Fabio Estevam]

clk_prepare_enable() may fail, so we should better check its return value
and propagate it in the case of error.

Signed-off-by: Fabio Estevam 
Reviewed-by: Philipp Zabel 
---
Changes since v2:
- None

 drivers/media/platform/imx-pxp.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/imx-pxp.c b/drivers/media/platform/imx-pxp.c
index b76cd0e..27780f1 100644
--- a/drivers/media/platform/imx-pxp.c
+++ b/drivers/media/platform/imx-pxp.c
@@ -1666,7 +1666,10 @@ static int pxp_probe(struct platform_device *pdev)
return ret;
}
 
-   clk_prepare_enable(dev->clk);
+   ret = clk_prepare_enable(dev->clk);
+   if (ret < 0)
+   return ret;
+
pxp_soft_reset(dev);
 
spin_lock_init(>irqlock);
-- 
2.7.4

[PATCH v3 3/3] media: imx-pxp: Improve pxp_soft_reset() error message

[Fabio Estevam]

Improve the pxp_soft_reset() error message by moving it to the
caller function, associating it with a proper device and also
by displaying the error code.

Signed-off-by: Fabio Estevam 
Reviewed-by: Philipp Zabel 
---
Changes since v2:
- None (only rebased against the change made in 2/3)

 drivers/media/platform/imx-pxp.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/imx-pxp.c b/drivers/media/platform/imx-pxp.c
index 986764d..b80d206 100644
--- a/drivers/media/platform/imx-pxp.c
+++ b/drivers/media/platform/imx-pxp.c
@@ -1619,10 +1619,8 @@ static int pxp_soft_reset(struct pxp_dev *dev)
 
ret = readl_poll_timeout(dev->mmio + HW_PXP_CTRL, val,
 val & BM_PXP_CTRL_CLKGATE, 0, 100);
-   if (ret < 0) {
-   pr_err("PXP reset timeout\n");
+   if (ret < 0)
return ret;
-   }
 
writel(BM_PXP_CTRL_SFTRST, dev->mmio + HW_PXP_CTRL_CLR);
writel(BM_PXP_CTRL_CLKGATE, dev->mmio + HW_PXP_CTRL_CLR);
@@ -1675,8 +1673,10 @@ static int pxp_probe(struct platform_device *pdev)
return ret;
 
ret = pxp_soft_reset(dev);
-   if (ret < 0)
+   if (ret < 0) {
+   dev_err(>dev, "PXP reset timeout: %d\n", ret);
goto err_clk;
+   }
 
spin_lock_init(>irqlock);
 
-- 
2.7.4

[PATCH v2 1/3] media: imx-pxp: Check the return value from clk_prepare_enable()

[Fabio Estevam]

clk_prepare_enable() may fail, so we should better check its return value
and propagate it in the case of error.

Signed-off-by: Fabio Estevam 
---
Changes since v1:
- Properly enumerate the series

 drivers/media/platform/imx-pxp.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/imx-pxp.c b/drivers/media/platform/imx-pxp.c
index b76cd0e..27780f1 100644
--- a/drivers/media/platform/imx-pxp.c
+++ b/drivers/media/platform/imx-pxp.c
@@ -1666,7 +1666,10 @@ static int pxp_probe(struct platform_device *pdev)
return ret;
}
 
-   clk_prepare_enable(dev->clk);
+   ret = clk_prepare_enable(dev->clk);
+   if (ret < 0)
+   return ret;
+
pxp_soft_reset(dev);
 
spin_lock_init(>irqlock);
-- 
2.7.4

[PATCH v2 2/3] media: imx-pxp: Check for pxp_soft_reset() error

[Fabio Estevam]

pxp_soft_reset() may fail with a timeout, so it is better to propagate
the error in this case.

Signed-off-by: Fabio Estevam 
---
Changes since v1:
- None

 drivers/media/platform/imx-pxp.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/imx-pxp.c b/drivers/media/platform/imx-pxp.c
index 27780f1..b3700b8 100644
--- a/drivers/media/platform/imx-pxp.c
+++ b/drivers/media/platform/imx-pxp.c
@@ -1607,7 +1607,7 @@ static const struct v4l2_m2m_ops m2m_ops = {
.job_abort  = pxp_job_abort,
 };
 
-static void pxp_soft_reset(struct pxp_dev *dev)
+static int pxp_soft_reset(struct pxp_dev *dev)
 {
int ret;
u32 val;
@@ -1619,11 +1619,15 @@ static void pxp_soft_reset(struct pxp_dev *dev)
 
ret = readl_poll_timeout(dev->mmio + HW_PXP_CTRL, val,
 val & BM_PXP_CTRL_CLKGATE, 0, 100);
-   if (ret < 0)
+   if (ret < 0) {
pr_err("PXP reset timeout\n");
+   return ret;
+   }
 
writel(BM_PXP_CTRL_SFTRST, dev->mmio + HW_PXP_CTRL_CLR);
writel(BM_PXP_CTRL_CLKGATE, dev->mmio + HW_PXP_CTRL_CLR);
+
+   return 0;
 }
 
 static int pxp_probe(struct platform_device *pdev)
@@ -1670,7 +1674,9 @@ static int pxp_probe(struct platform_device *pdev)
if (ret < 0)
return ret;
 
-   pxp_soft_reset(dev);
+   ret = pxp_soft_reset(dev);
+   if (ret < 0)
+   return ret;
 
spin_lock_init(>irqlock);
 
-- 
2.7.4

[PATCH v2 3/3] media: imx-pxp: Improve pxp_soft_reset() error message

[Fabio Estevam]

Improve the pxp_soft_reset() error message by moving it to the
caller function, associating it with a proper device and also
by displaying the error code.

Signed-off-by: Fabio Estevam 
---
Changes since v1:
- Newly introduced in this version

 drivers/media/platform/imx-pxp.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/imx-pxp.c b/drivers/media/platform/imx-pxp.c
index b3700b8..1b765c9 100644
--- a/drivers/media/platform/imx-pxp.c
+++ b/drivers/media/platform/imx-pxp.c
@@ -1619,10 +1619,8 @@ static int pxp_soft_reset(struct pxp_dev *dev)
 
ret = readl_poll_timeout(dev->mmio + HW_PXP_CTRL, val,
 val & BM_PXP_CTRL_CLKGATE, 0, 100);
-   if (ret < 0) {
-   pr_err("PXP reset timeout\n");
+   if (ret < 0)
return ret;
-   }
 
writel(BM_PXP_CTRL_SFTRST, dev->mmio + HW_PXP_CTRL_CLR);
writel(BM_PXP_CTRL_CLKGATE, dev->mmio + HW_PXP_CTRL_CLR);
@@ -1675,8 +1673,10 @@ static int pxp_probe(struct platform_device *pdev)
return ret;
 
ret = pxp_soft_reset(dev);
-   if (ret < 0)
+   if (ret < 0) {
+   dev_err(>dev, "PXP reset timeout: %d\n", ret);
return ret;
+   }
 
spin_lock_init(>irqlock);
 
-- 
2.7.4

[PATCH 2/2] media: imx-pxp: Check for pxp_soft_reset() error

[Fabio Estevam]

pxp_soft_reset() may fail with a timeout, so it is better to propagate
the error in this case.

Signed-off-by: Fabio Estevam 
---
 drivers/media/platform/imx-pxp.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/imx-pxp.c b/drivers/media/platform/imx-pxp.c
index 27780f1..b3700b8 100644
--- a/drivers/media/platform/imx-pxp.c
+++ b/drivers/media/platform/imx-pxp.c
@@ -1607,7 +1607,7 @@ static const struct v4l2_m2m_ops m2m_ops = {
.job_abort  = pxp_job_abort,
 };
 
-static void pxp_soft_reset(struct pxp_dev *dev)
+static int pxp_soft_reset(struct pxp_dev *dev)
 {
int ret;
u32 val;
@@ -1619,11 +1619,15 @@ static void pxp_soft_reset(struct pxp_dev *dev)
 
ret = readl_poll_timeout(dev->mmio + HW_PXP_CTRL, val,
 val & BM_PXP_CTRL_CLKGATE, 0, 100);
-   if (ret < 0)
+   if (ret < 0) {
pr_err("PXP reset timeout\n");
+   return ret;
+   }
 
writel(BM_PXP_CTRL_SFTRST, dev->mmio + HW_PXP_CTRL_CLR);
writel(BM_PXP_CTRL_CLKGATE, dev->mmio + HW_PXP_CTRL_CLR);
+
+   return 0;
 }
 
 static int pxp_probe(struct platform_device *pdev)
@@ -1670,7 +1674,9 @@ static int pxp_probe(struct platform_device *pdev)
if (ret < 0)
return ret;
 
-   pxp_soft_reset(dev);
+   ret = pxp_soft_reset(dev);
+   if (ret < 0)
+   return ret;
 
spin_lock_init(>irqlock);
 
-- 
2.7.4

[PATCH] media: imx-pxp: Check the return value from clk_prepare_enable()

[Fabio Estevam]

clk_prepare_enable() may fail, so we should better check its return value
and propagate it in the case of error.

Signed-off-by: Fabio Estevam 
---
 drivers/media/platform/imx-pxp.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/imx-pxp.c b/drivers/media/platform/imx-pxp.c
index b76cd0e..27780f1 100644
--- a/drivers/media/platform/imx-pxp.c
+++ b/drivers/media/platform/imx-pxp.c
@@ -1666,7 +1666,10 @@ static int pxp_probe(struct platform_device *pdev)
return ret;
}
 
-   clk_prepare_enable(dev->clk);
+   ret = clk_prepare_enable(dev->clk);
+   if (ret < 0)
+   return ret;
+
pxp_soft_reset(dev);
 
spin_lock_init(>irqlock);
-- 
2.7.4

Re: [PATCH v2 3/4] media: imx-pxp: add i.MX Pixel Pipeline driver

[Fabio Estevam]

Hi Philipp,

On Wed, Sep 5, 2018 at 7:00 AM, Philipp Zabel  wrote:

> index ..2f90c692f3fe
> --- /dev/null
> +++ b/drivers/media/platform/imx-pxp.c
> @@ -0,0 +1,1774 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later

The recommended SPDX format in this case is:

// SPDX-License-Identifier: GPL-2.0+

as per Documentation/process/license-rules.rst

Thanks

Re: [PATCH 0/4] media: ov2680: follow up from initial version

[Fabio Estevam]

Hi Rui,

On Tue, May 8, 2018 at 10:52 AM, Rui Miguel Silva  wrote:

> So, my understand is that the patches will be applied or are already
> applied to someone tree (strange patchwork does not send who or which
> tree), but since I do not want to break someone workflow I will wait for
> some maintainer word on this... If it is better to send the original series
> or the follow up patches.

Ah, didn't know it has been applied. It seems it will soon appear in
linux-next then.

In this case, the follow up series makes sense.

Thanks

Re: [PATCH 0/4] media: ov2680: follow up from initial version

[Fabio Estevam]

Hi Rui,

On Mon, May 7, 2018 at 12:56 PM, Rui Miguel Silva  wrote:
> Sorry I have Out-of-Office some part of last week, I had v6 of the original
> series ready but since I have received the notification from patchwork that 
> the
> v5 was accepted, I am sending this as a follow up tha address Fabio comments.
>
> - this adds the power supplies to this sensor
> - fix gpio polarity and naming.
>
> Cheers,
>Rui
>
>
> Rui Miguel Silva (4):
>   media: ov2680: dt: add voltage supplies as required
>   media: ov2680: dt: rename gpio to reset and fix polarity
>   media: ov2680: rename powerdown gpio and fix polarity
>   media: ov2680: add regulators to supply control

As the initial ov2680 series has not been applied, I think it would be
better if you send a new version with all these fixes.

Re: [PATCH v3 12/14] ARM: dts: imx7s-warp: add ov2680 sensor node

[Fabio Estevam]

Hi Rui,

On Mon, May 7, 2018 at 1:21 PM, Rui Miguel Silva  wrote:

> +   reg_peri_3p15v: regulator-peri-3p15v {
> +   compatible = "regulator-fixed";
> +   regulator-name = "peri_3p15v_reg";
> +   regulator-min-microvolt = <315>;
> +   regulator-max-microvolt = <315>;
> +   regulator-always-on;

You can remove the 'regulator-always-on' property as this regulator
will be controlled by AVDD-supply.

Re: [PATCH v5 1/2] media: ov2680: dt: Add bindings for OV2680

[Fabio Estevam]

Hi Rui,

On Thu, Apr 19, 2018 at 8:00 AM, Rui Miguel Silva  wrote:
> Add device tree binding documentation for the OV2680 camera sensor.
>
> Reviewed-by: Rob Herring 
> CC: devicet...@vger.kernel.org
> Signed-off-by: Rui Miguel Silva 
> ---
>  .../devicetree/bindings/media/i2c/ov2680.txt  | 40 +++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
> b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> new file mode 100644
> index ..0e29f1a113c0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> @@ -0,0 +1,40 @@
> +* Omnivision OV2680 MIPI CSI-2 sensor
> +
> +Required Properties:
> +- compatible: should be "ovti,ov2680".
> +- clocks: reference to the xvclk input clock.
> +- clock-names: should be "xvclk".

You missed to pass the camera power supplies as required properties:

DOVDD-supply
AVDD-supply
DVDD-supply

Re: [PATCH v5 1/2] media: ov2680: dt: Add bindings for OV2680

[Fabio Estevam]

On Mon, Apr 23, 2018 at 12:09 PM, Rui Miguel Silva  wrote:

> Yes, you are correct, I will fix this, and the dts entry.

As this pin has both reset and powerdown functionalities, maybe you
can name the property as 'reset-gpios'

Thanks

Re: [PATCH v5 1/2] media: ov2680: dt: Add bindings for OV2680

[Fabio Estevam]

Hi Rui,

On Thu, Apr 19, 2018 at 8:00 AM, Rui Miguel Silva  wrote:

> +Optional Properties:
> +- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
> +if any. This is an active high signal to the OV2680.

I looked at the OV2680 datasheet and I see a pin called XSHUTDN, which has
the following description:

XSHUTDN: reset and power down (active low with internal pull down resistor)

So it should be active low, not active high.

Re: [PATCH v2] media: imx-media-csi: Fix inconsistent IS_ERR and PTR_ERR

[Fabio Estevam]

Hi Hans,

Unfortunately this one missed to be applied into 4.17-rc and now the
imx-media-csi driver does not probe.

Please consider applying it for 4.17-rc3 to avoid the regression.

Thanks

On Mon, Apr 16, 2018 at 2:28 PM, Fabio Estevam <feste...@gmail.com> wrote:
> From: From: Gustavo A. R. Silva <gust...@embeddedor.com>
>
> Fix inconsistent IS_ERR and PTR_ERR in imx_csi_probe.
> The proper pointer to be passed as argument is pinctrl
> instead of priv->vdev.
>
> This issue was detected with the help of Coccinelle.
>
> Fixes: 52e17089d185 ("media: imx: Don't initialize vars that won't be used")
> Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
> Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
> Acked-by: Philipp Zabel <p.za...@pengutronix.de>
> ---
> Changes since v1:
> - Rebased against 4.17-rc1
>
>  drivers/staging/media/imx/imx-media-csi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/imx/imx-media-csi.c 
> b/drivers/staging/media/imx/imx-media-csi.c
> index 16cab40..aeab05f 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -1799,7 +1799,7 @@ static int imx_csi_probe(struct platform_device *pdev)
> priv->dev->of_node = pdata->of_node;
> pinctrl = devm_pinctrl_get_select_default(priv->dev);
> if (IS_ERR(pinctrl)) {
> -   ret = PTR_ERR(priv->vdev);
> +   ret = PTR_ERR(pinctrl);
> dev_dbg(priv->dev,
> "devm_pinctrl_get_select_default() failed: %d\n", 
> ret);
> if (ret != -ENODEV)
> --
> 2.7.4
>

[PATCH RESEND 2/3] media: ov5695: Remove owner assignment from i2c_driver

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

Structure i2c_driver does not need to set the owner field, as this will
be populated by the driver core.

Generated by scripts/coccinelle/api/platform_no_drv_owner.cocci.

Cc: Shunqian Zheng <zhen...@rock-chips.com>
Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/media/i2c/ov5695.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/i2c/ov5695.c b/drivers/media/i2c/ov5695.c
index 9be38a0..9a80dec 100644
--- a/drivers/media/i2c/ov5695.c
+++ b/drivers/media/i2c/ov5695.c
@@ -1385,7 +1385,6 @@ MODULE_DEVICE_TABLE(of, ov5695_of_match);
 static struct i2c_driver ov5695_i2c_driver = {
.driver = {
.name = "ov5695",
-   .owner = THIS_MODULE,
.pm = _pm_ops,
.of_match_table = of_match_ptr(ov5695_of_match),
},
-- 
2.7.4

[PATCH RESEND 3/3] media: ov13858: Remove owner assignment from i2c_driver

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

Structure i2c_driver does not need to set the owner field, as this will
be populated by the driver core.

Generated by scripts/coccinelle/api/platform_no_drv_owner.cocci.

Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/media/i2c/ov13858.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
index 30ee9f7..5b617a7 100644
--- a/drivers/media/i2c/ov13858.c
+++ b/drivers/media/i2c/ov13858.c
@@ -1805,7 +1805,6 @@ MODULE_DEVICE_TABLE(acpi, ov13858_acpi_ids);
 static struct i2c_driver ov13858_i2c_driver = {
.driver = {
.name = "ov13858",
-   .owner = THIS_MODULE,
.pm = _pm_ops,
.acpi_match_table = ACPI_PTR(ov13858_acpi_ids),
},
-- 
2.7.4

Re: imx-media: MT9P031 Capture issues on IMX6

[Fabio Estevam]

On Thu, Apr 19, 2018 at 1:55 PM, Ibtsam Ul-Haq
 wrote:

> I can see by using a logic analyzer that the PIXCLK does not look
> nice. It looks similar to the issue mentioned here:
> https://community.nxp.com/thread/454467
>
> except that in my case it looks pulled up instead of down.
> However I do not yet have a clue what causes this.
> VSYNC and HSYNC waveforms look ok, until the whole capture is stopped
> due to the error, after 14 frames.
> The relevant pinctrl settings in the dts are:
>
> MX6QDL_PAD_CSI0_PIXCLK__IPU1_CSI0_PIXCLK0x4001b0b0
> MX6QDL_PAD_CSI0_MCLK__IPU1_CSI0_HSYNC0x4001b0b0
> MX6QDL_PAD_CSI0_VSYNC__IPU1_CSI0_VSYNC0x4001b0b0

Not sure why you are setting the SION bit (bit 30) on the CSI pads.

Does it work better if you do not set it?

For your reference: this is what we do on imx6qdl-sabresd.dtsi:

MX6QDL_PAD_CSI0_DAT12__IPU1_CSI0_DATA120x1b0b0
MX6QDL_PAD_CSI0_DAT13__IPU1_CSI0_DATA130x1b0b0
MX6QDL_PAD_CSI0_DAT14__IPU1_CSI0_DATA140x1b0b0
MX6QDL_PAD_CSI0_DAT15__IPU1_CSI0_DATA150x1b0b0
MX6QDL_PAD_CSI0_DAT16__IPU1_CSI0_DATA160x1b0b0
MX6QDL_PAD_CSI0_DAT17__IPU1_CSI0_DATA170x1b0b0
MX6QDL_PAD_CSI0_DAT18__IPU1_CSI0_DATA180x1b0b0
MX6QDL_PAD_CSI0_DAT19__IPU1_CSI0_DATA190x1b0b0
MX6QDL_PAD_CSI0_PIXCLK__IPU1_CSI0_PIXCLK   0x1b0b0
MX6QDL_PAD_CSI0_MCLK__IPU1_CSI0_HSYNC  0x1b0b0
MX6QDL_PAD_CSI0_VSYNC__IPU1_CSI0_VSYNC 0x1b0b0

Re: [PATCH v3 1/2] media: imx-media-csi: Fix inconsistent IS_ERR and PTR_ERR

[Fabio Estevam]

On Mon, Apr 16, 2018 at 10:16 AM, Philipp Zabel  wrote:

> The second patch is applied now, but this part is still missing in
> v4.17-rc1, causing the CSI subdev probe to fail:
>
>   imx-ipuv3-csi: probe of imx-ipuv3-csi.0 failed with error -1369528304
>   imx-ipuv3-csi: probe of imx-ipuv3-csi.1 failed with error -1369528304
>   imx-ipuv3-csi: probe of imx-ipuv3-csi.5 failed with error -1369528304
>   imx-ipuv3-csi: probe of imx-ipuv3-csi.6 failed with error -1369528304

Yes, this original patch does not apply against 4.17-rc1 anymore, so I
rebased and resend it.

Thanks

[PATCH v2] media: imx-media-csi: Fix inconsistent IS_ERR and PTR_ERR

[Fabio Estevam]

From: From: Gustavo A. R. Silva <gust...@embeddedor.com>

Fix inconsistent IS_ERR and PTR_ERR in imx_csi_probe.
The proper pointer to be passed as argument is pinctrl
instead of priv->vdev.

This issue was detected with the help of Coccinelle.

Fixes: 52e17089d185 ("media: imx: Don't initialize vars that won't be used")
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
Acked-by: Philipp Zabel <p.za...@pengutronix.de>
---
Changes since v1:
- Rebased against 4.17-rc1

 drivers/staging/media/imx/imx-media-csi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 16cab40..aeab05f 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1799,7 +1799,7 @@ static int imx_csi_probe(struct platform_device *pdev)
priv->dev->of_node = pdata->of_node;
pinctrl = devm_pinctrl_get_select_default(priv->dev);
if (IS_ERR(pinctrl)) {
-   ret = PTR_ERR(priv->vdev);
+   ret = PTR_ERR(pinctrl);
dev_dbg(priv->dev,
"devm_pinctrl_get_select_default() failed: %d\n", ret);
if (ret != -ENODEV)
-- 
2.7.4

Re: IMX6 Media dev node not created

[Fabio Estevam]

Hi Ibtsam,

On Thu, Apr 5, 2018 at 11:52 AM, Ibtsam Ul-Haq
 wrote:

> That worked like a charm! Thanks a lot guys, I would have never thought of 
> that!

Excellent!

If you have a chance, please submit the the dts patch that enables the
camera to the linux-arm-kernel mailing list for review and inclusion
to the mainline kernel.

Re: IMX6 Media dev node not created

[Fabio Estevam]

Hi Ibtsam,

[Adding Steve and Philipp in case they can provide some suggestions]

On Thu, Apr 5, 2018 at 9:30 AM, Ibtsam Ul-Haq  wrote:
> Greetings everyone,
>
> I'm running Linux 4.14.31 on an IMX6 QuadPlus based Phytec board
> (PCM-058). I have connected an mt9p031 sensor to ipu1_csi0. The
> problem is that I am not seeing the /dev/media0 node.

Can you share your dts?

> I have already read the fix mentioned in a previous discussion:
>
> https://www.spinics.net/lists/linux-media/msg121965.html
>
> and that does not seem to be the problem in my case as I do get the
> "ipu1_csi0_mux" registered. Running a grep on dmesg I get:
>
> [3.235383] imx-media: Registered subdev ipu1_vdic
> [3.241134] imx-media: Registered subdev ipu2_vdic
> [3.246830] imx-media: Registered subdev ipu1_ic_prp
> [3.252115] imx-media: Registered subdev ipu1_ic_prpenc
> [3.266991] imx-media: Registered subdev ipu1_ic_prpvf
> [3.280228] imx-media: Registered subdev ipu2_ic_prp
> [3.285580] imx-media: Registered subdev ipu2_ic_prpenc
> [3.299335] imx-media: Registered subdev ipu2_ic_prpvf
> [3.350034] imx-media: Registered subdev ipu1_csi0
> [3.363017] imx-media: Registered subdev ipu1_csi1
> [3.375523] imx-media: Registered subdev ipu2_csi0
> [3.388615] imx-media: Registered subdev ipu2_csi1
> [3.560351] imx-media: Registered subdev ipu1_csi0_mux
> [3.566151] imx-media: Registered subdev ipu2_csi1_mux
> [   10.525497] imx-media: Registered subdev mt9p031 0-0048
> [   10.530816] imx-media capture-subsystem: Entity type for entity
> mt9p031 0-0048 was not initialized!
> [   10.569201] mt9p031 0-0048: MT9P031 detected at address 0x48
> [   10.582895] imx-media: Registered subdev mt9p031 0-005d
> [   10.588335] imx-media capture-subsystem: Entity type for entity
> mt9p031 0-005d was not initialized!
> [   10.618795] mt9p031 0-005d: MT9P031 not detected, wrong version 0xfffa

Why do you have the camera in two I2C addresses: 0x48 and 0x5d?

> Also my config does appear to have the required options activated;
> running "zcat /proc/config.gz | egrep 'VIDEO_MUX|MUX_MMIO|VIDEO_IMX'"
> I get:
>
> # CONFIG_MDIO_BUS_MUX_MMIOREG is not set
> CONFIG_VIDEO_MUX=y
> CONFIG_VIDEO_IMX_VDOA=m
> CONFIG_VIDEO_IMX_MEDIA=y
> CONFIG_VIDEO_IMX_CSI=y
> CONFIG_MUX_MMIO=y
>
> I would really appreciate if anyone could help me trying to find out
> what went wrong and why the /dev/media0 node is not showing up.
>
> Many thanks and best regards,
> Ibtsam Haq

[PATCH 3/3] media: ov13858: Remove owner assignment from i2c_driver

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

Structure i2c_driver does not need to set the owner field, as this will
be populated by the driver core.

Generated by scripts/coccinelle/api/platform_no_drv_owner.cocci.

Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/media/i2c/ov13858.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
index 30ee9f7..5b617a7 100644
--- a/drivers/media/i2c/ov13858.c
+++ b/drivers/media/i2c/ov13858.c
@@ -1805,7 +1805,6 @@ MODULE_DEVICE_TABLE(acpi, ov13858_acpi_ids);
 static struct i2c_driver ov13858_i2c_driver = {
.driver = {
.name = "ov13858",
-   .owner = THIS_MODULE,
.pm = _pm_ops,
.acpi_match_table = ACPI_PTR(ov13858_acpi_ids),
},
-- 
2.7.4

[PATCH 1/3] media: si2165: Remove owner assignment from i2c_driver

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

Structure i2c_driver does not need to set the owner field, as this will
be populated by the driver core.

Generated by scripts/coccinelle/api/platform_no_drv_owner.cocci.

Cc: Matthias Schwarzott <z...@gentoo.org>
Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/media/dvb-frontends/si2165.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/si2165.c 
b/drivers/media/dvb-frontends/si2165.c
index 2dd336f..d132e3c 100644
--- a/drivers/media/dvb-frontends/si2165.c
+++ b/drivers/media/dvb-frontends/si2165.c
@@ -1299,7 +1299,6 @@ MODULE_DEVICE_TABLE(i2c, si2165_id_table);
 
 static struct i2c_driver si2165_driver = {
.driver = {
-   .owner  = THIS_MODULE,
.name   = "si2165",
},
.probe  = si2165_probe,
-- 
2.7.4

[PATCH 2/3] media: ov5695: Remove owner assignment from i2c_driver

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

Structure i2c_driver does not need to set the owner field, as this will
be populated by the driver core.

Generated by scripts/coccinelle/api/platform_no_drv_owner.cocci.

Cc: Shunqian Zheng <zhen...@rock-chips.com>
Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/media/i2c/ov5695.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/i2c/ov5695.c b/drivers/media/i2c/ov5695.c
index 9be38a0..9a80dec 100644
--- a/drivers/media/i2c/ov5695.c
+++ b/drivers/media/i2c/ov5695.c
@@ -1385,7 +1385,6 @@ MODULE_DEVICE_TABLE(of, ov5695_of_match);
 static struct i2c_driver ov5695_i2c_driver = {
.driver = {
.name = "ov5695",
-   .owner = THIS_MODULE,
.pm = _pm_ops,
.of_match_table = of_match_ptr(ov5695_of_match),
},
-- 
2.7.4

Re: [PATCH v3 2/2] media: imx-media-csi: Do not propagate the error when pinctrl is not found

[Fabio Estevam]

Hi Mauro and Hans,

On Sat, Mar 10, 2018 at 12:53 PM, Fabio Estevam <feste...@gmail.com> wrote:
> Hi,
>
> On Sat, Mar 3, 2018 at 9:56 AM, Fabio Estevam <feste...@gmail.com> wrote:
>> From: Fabio Estevam <fabio.este...@nxp.com>
>>
>> Since commit 52e17089d185 ("media: imx: Don't initialize vars that
>> won't be used") imx_csi_probe() fails to probe after propagating the
>> devm_pinctrl_get_select_default() error.
>>
>> devm_pinctrl_get_select_default() may return -ENODEV when the CSI pinctrl
>> entry is not found, so better not to propagate the error in the -ENODEV
>> case to avoid a regression.
>>
>> Suggested-by: Philipp Zabel <p.za...@pengutronix.de>
>> Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
>> Reviewed-by: Steve Longerbeam <steve_longerb...@mentor.com>
>
> A gentle ping.
>
> This series fixes a regression on the imx-media-csi driver.

Could you please consider applying this series that fixes the probe of
the imx-media-csi driver?

Thanks

Re: [PATCH 1/3] dt-bindings: display: dw_hdmi.txt

[Fabio Estevam]

Hi Hans,

On Mon, Mar 19, 2018 at 8:43 AM, Hans Verkuil  wrote:
> From: Hans Verkuil 

It seems the Subject line is missing some content?

[PATCH v2] media: ov2685: Remove owner assignment from i2c_driver

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

Structure i2c_driver does not need to set the owner field, as this will
be populated by the driver core.

Generated by scripts/coccinelle/api/platform_no_drv_owner.cocci.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
Changes since v1:
- s/platform_driver/i2c_driver in the commit log

 drivers/media/i2c/ov2685.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/i2c/ov2685.c b/drivers/media/i2c/ov2685.c
index 83c55e8..385c188 100644
--- a/drivers/media/i2c/ov2685.c
+++ b/drivers/media/i2c/ov2685.c
@@ -832,7 +832,6 @@ MODULE_DEVICE_TABLE(of, ov2685_of_match);
 static struct i2c_driver ov2685_i2c_driver = {
.driver = {
.name = "ov2685",
-   .owner = THIS_MODULE,
.pm = _pm_ops,
.of_match_table = of_match_ptr(ov2685_of_match),
},
-- 
2.7.4

[PATCH] media: ov2685: Remove owner assignment from platform_driver

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

platform_driver does not need to set the owner field, as this will
be populated by the driver core.

Generated by scripts/coccinelle/api/platform_no_drv_owner.cocci.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/media/i2c/ov2685.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/i2c/ov2685.c b/drivers/media/i2c/ov2685.c
index 83c55e8..385c188 100644
--- a/drivers/media/i2c/ov2685.c
+++ b/drivers/media/i2c/ov2685.c
@@ -832,7 +832,6 @@ MODULE_DEVICE_TABLE(of, ov2685_of_match);
 static struct i2c_driver ov2685_i2c_driver = {
.driver = {
.name = "ov2685",
-   .owner = THIS_MODULE,
.pm = _pm_ops,
.of_match_table = of_match_ptr(ov2685_of_match),
},
-- 
2.7.4

Re: [PATCH v2] media: staging/imx: fill vb2_v4l2_buffer sequence entry

[Fabio Estevam]

Hi Peter,

On Tue, Mar 13, 2018 at 7:34 PM, Peter Seiderer  wrote:
> Signed-off-by: Peter Seiderer 

Could you please resend with a commit log explaining the reason for this patch?

Tahnks

Re: coda: i.MX6 decoding performance issues for multi-streaming

[Fabio Estevam]

Hi Javier,

On Mon, Mar 12, 2018 at 1:54 PM, Javier Martin  wrote:
> Hi,
> we have an i.MX6 Solo based board running the latest mainline kernel
> (4.15.3).
>
> As part of our development we were measuring the decoding performance of the
> i.MX6 coda chip.
>
> For that purpose we are feeding the decoder with 640x368 @ 30fps H.264
> streams that have been generated by another i.MX6 coda encoder configured
> with fixed qp = 25 and gopsize = 16.
>
> For 1-2 streams it works smoothly. However, when adding the 3rd stream the
> first decoder instance starts to output these kind of errors:
>
> DEC_PIC_SUCCESS = 2097153  -> 0x21
> DEC_PIC_SUCCESS = 2621441  -> 0x280001
>
> Every time one of these errors appears we can observe a weird artifact in
> the decoded video (pixelated macroblocks and/or jumps back in time).
>
> I tried looking at the original VPU lib implementation by Freescale [1] but
> they don't seem to handle these errors either. As I don't have access to any
> kind of Coda IP documentation it's quite hard to me to perform any
> additional debugging.
>
> Has anyone experienced these kind of performance issues too? I'm open to any
> suggestions and willing to perform extra tests to get to the bottom of this.

Are you passing 'capture-io-mode=dmabuf' in your Gstreamer pipeline?

This really improves the performance of video decoding.

Re: [PATCH v3 2/2] media: imx-media-csi: Do not propagate the error when pinctrl is not found

[Fabio Estevam]

Hi,

On Sat, Mar 3, 2018 at 9:56 AM, Fabio Estevam <feste...@gmail.com> wrote:
> From: Fabio Estevam <fabio.este...@nxp.com>
>
> Since commit 52e17089d185 ("media: imx: Don't initialize vars that
> won't be used") imx_csi_probe() fails to probe after propagating the
> devm_pinctrl_get_select_default() error.
>
> devm_pinctrl_get_select_default() may return -ENODEV when the CSI pinctrl
> entry is not found, so better not to propagate the error in the -ENODEV
> case to avoid a regression.
>
> Suggested-by: Philipp Zabel <p.za...@pengutronix.de>
> Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
> Reviewed-by: Steve Longerbeam <steve_longerb...@mentor.com>

A gentle ping.

This series fixes a regression on the imx-media-csi driver.

Thanks

Re: [PATCH] media: ov5640: fix get_/set_fmt colorspace related fields

[Fabio Estevam]

Hi Sakari,

On Wed, Mar 7, 2018 at 5:13 AM, Sakari Ailus  wrote:

>> @@ -2497,16 +2504,22 @@ static int ov5640_probe(struct i2c_client *client,
>>   struct fwnode_handle *endpoint;
>>   struct ov5640_dev *sensor;
>>   int ret;
>> + struct v4l2_mbus_framefmt *fmt;
>
> This one I'd arrange before ret. The local variable declarations should
> generally look like a Christmas tree but upside down.

It seems Mauro is not happy with reverse Christmas tree ordering:
https://www.mail-archive.com/linux-media@vger.kernel.org/msg127221.html

Re: [PATCH] media: dvbdev: fix building on ia64

[Fabio Estevam]

Hi Mauro,

On Wed, Mar 7, 2018 at 6:14 AM, Mauro Carvalho Chehab
<mche...@s-opensource.com> wrote:
> Not sure why, but, on ia64, with Linaro's gcc 7.3 compiler,
> using #ifdef (CONFIG_I2C) is not OK.

Looking at the kbuild report the failure happens when CONFIG_I2C=m.

IS_ENABLED() macro takes care of both built-in and module as it will expand to:

#if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)

and that's the reason why IS_ENABLE() fixes the build problem.

Regards,

Fabio Estevam

[PATCH v3 2/2] media: imx-media-csi: Do not propagate the error when pinctrl is not found

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

Since commit 52e17089d185 ("media: imx: Don't initialize vars that
won't be used") imx_csi_probe() fails to probe after propagating the
devm_pinctrl_get_select_default() error.

devm_pinctrl_get_select_default() may return -ENODEV when the CSI pinctrl
entry is not found, so better not to propagate the error in the -ENODEV
case to avoid a regression.

Suggested-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
Reviewed-by: Steve Longerbeam <steve_longerb...@mentor.com>
---
Changes since v2:
- Remove extraneous 'drivers/staging/media/imx/imx-media-csi.c' in commit log
Changes since v1:
- Add \n to the dbg message

 drivers/staging/media/imx/imx-media-csi.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 4f290a0..5af66f6 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1799,7 +1799,10 @@ static int imx_csi_probe(struct platform_device *pdev)
pinctrl = devm_pinctrl_get_select_default(priv->dev);
if (IS_ERR(pinctrl)) {
ret = PTR_ERR(pinctrl);
-   goto free;
+   dev_dbg(priv->dev,
+   "devm_pinctrl_get_select_default() failed: %d\n", ret);
+   if (ret != -ENODEV)
+   goto free;
}
 
ret = v4l2_async_register_subdev(>sd);
-- 
2.7.4

[PATCH v3 1/2] media: imx-media-csi: Fix inconsistent IS_ERR and PTR_ERR

[Fabio Estevam]

From: Gustavo A. R. Silva <gust...@embeddedor.com>

Fix inconsistent IS_ERR and PTR_ERR in imx_csi_probe.
The proper pointer to be passed as argument is pinctrl
instead of priv->vdev.

This issue was detected with the help of Coccinelle.

Fixes: 52e17089d185 ("media: imx: Don't initialize vars that won't be used")
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
Acked-by: Philipp Zabel <p.za...@pengutronix.de>
---
Changes since v2:
- None
Changes since v1:
- None

 drivers/staging/media/imx/imx-media-csi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 5a195f8..4f290a0 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1798,7 +1798,7 @@ static int imx_csi_probe(struct platform_device *pdev)
priv->dev->of_node = pdata->of_node;
pinctrl = devm_pinctrl_get_select_default(priv->dev);
if (IS_ERR(pinctrl)) {
-   ret = PTR_ERR(priv->vdev);
+   ret = PTR_ERR(pinctrl);
goto free;
}
 
-- 
2.7.4

[PATCH v2 1/2] media: imx-media-csi: Fix inconsistent IS_ERR and PTR_ERR

[Fabio Estevam]

From: Gustavo A. R. Silva <gust...@embeddedor.com>

Fix inconsistent IS_ERR and PTR_ERR in imx_csi_probe.
The proper pointer to be passed as argument is pinctrl
instead of priv->vdev.

This issue was detected with the help of Coccinelle.

Fixes: 52e17089d185 ("media: imx: Don't initialize vars that won't be used")
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
Acked-by: Philipp Zabel <p.za...@pengutronix.de>
---
Changes since v1:
- None

 drivers/staging/media/imx/imx-media-csi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 5a195f8..4f290a0 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1798,7 +1798,7 @@ static int imx_csi_probe(struct platform_device *pdev)
priv->dev->of_node = pdata->of_node;
pinctrl = devm_pinctrl_get_select_default(priv->dev);
if (IS_ERR(pinctrl)) {
-   ret = PTR_ERR(priv->vdev);
+   ret = PTR_ERR(pinctrl);
goto free;
}
 
-- 
2.7.4

[PATCH v2 2/2] media: imx-media-csi: Do not propagate the error when pinctrl is not found

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

Since commit 52e17089d185 ("media: imx: Don't initialize vars that
won't be used") imx_csi_probe() fails to probe after propagating the
devm_pinctrl_get_select_default() error.

devm_pinctrl_get_select_default() may return -ENODEV when the CSI pinctrl
entry is not found, so better not to propagate the error in the -ENODEV
case to avoid a regression.
drivers/staging/media/imx/imx-media-csi.c
Suggested-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
Reviewed-by: Steve Longerbeam <steve_longerb...@mentor.com>
---
Changes since v1:
- Add \n to the dbg message

 drivers/staging/media/imx/imx-media-csi.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 4f290a0..5af66f6 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1799,7 +1799,10 @@ static int imx_csi_probe(struct platform_device *pdev)
pinctrl = devm_pinctrl_get_select_default(priv->dev);
if (IS_ERR(pinctrl)) {
ret = PTR_ERR(pinctrl);
-   goto free;
+   dev_dbg(priv->dev,
+   "devm_pinctrl_get_select_default() failed: %d\n", ret);
+   if (ret != -ENODEV)
+   goto free;
}
 
ret = v4l2_async_register_subdev(>sd);
-- 
2.7.4

[PATCH 2/2] media: imx-media-csi: Do not propagate the error when pinctrl is not found

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

Since commit 52e17089d185 ("media: imx: Don't initialize vars that
won't be used") imx_csi_probe() fails to probe after propagating the
devm_pinctrl_get_select_default() error.

devm_pinctrl_get_select_default() may return -ENODEV when the CSI pinctrl
entry is not found, so better not to propagate the error in the -ENODEV
case to avoid a regression.

Suggested-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/staging/media/imx/imx-media-csi.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 4f290a0..5af66f6 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1799,7 +1799,10 @@ static int imx_csi_probe(struct platform_device *pdev)
pinctrl = devm_pinctrl_get_select_default(priv->dev);
if (IS_ERR(pinctrl)) {
ret = PTR_ERR(pinctrl);
-   goto free;
+   dev_dbg(priv->dev,
+   "devm_pinctrl_get_select_default() failed: %d", ret);
+   if (ret != -ENODEV)
+   goto free;
}
 
ret = v4l2_async_register_subdev(>sd);
-- 
2.7.4

[PATCH 1/2] media: imx-media-csi: Fix inconsistent IS_ERR and PTR_ERR

[Fabio Estevam]

From: Gustavo A. R. Silva <gust...@embeddedor.com>

Fix inconsistent IS_ERR and PTR_ERR in imx_csi_probe.
The proper pointer to be passed as argument is pinctrl
instead of priv->vdev.

This issue was detected with the help of Coccinelle.

Fixes: 52e17089d185 ("media: imx: Don't initialize vars that won't be used")
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
Acked-by: Philipp Zabel <p.za...@pengutronix.de>
---
 drivers/staging/media/imx/imx-media-csi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 5a195f8..4f290a0 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1798,7 +1798,7 @@ static int imx_csi_probe(struct platform_device *pdev)
priv->dev->of_node = pdata->of_node;
pinctrl = devm_pinctrl_get_select_default(priv->dev);
if (IS_ERR(pinctrl)) {
-   ret = PTR_ERR(priv->vdev);
+   ret = PTR_ERR(pinctrl);
goto free;
}
 
-- 
2.7.4

Re: [PATCH] staging/imx: Fix inconsistent IS_ERR and PTR_ERR

[Fabio Estevam]

On Thu, Mar 1, 2018 at 1:27 PM, Philipp Zabel  wrote:

> Oh, this only works for csi ports that have pinctrl in their csi port
> node, like:
>
> _csi0 {
> pinctrl-names = "default";
> pinctrl-0 = <_ipu1_csi0>;
> };

This is the case for imx6qdl-sabresd.dtsi and even in this case
devm_pinctrl_get_select_default() fails

> pinctrl would have to be moved out of the csi port nodes, for example
> into their parent ipu nodes, or maybe more correctly, into the video mux
> nodes in each device tree.

Tried it like this:

--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -154,12 +154,9 @@
 };

 _csi0_mux_from_parallel_sensor {
-   remote-endpoint = <_to_ipu1_csi0_mux>;
-};
-
-_csi0 {
pinctrl-names = "default";
pinctrl-0 = <_ipu1_csi0>;
+   remote-endpoint = <_to_ipu1_csi0_mux>;
 };

 _csi {


but still get the devm_pinctrl_get_select_default() failure.

I was not able to change the dts so that
devm_pinctrl_get_select_default() succeeds.

If you agree I can send the following change:

diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index 5a195f8..c40f786 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1797,11 +1797,8 @@ static int imx_csi_probe(struct platform_device *pdev)
 */
priv->dev->of_node = pdata->of_node;
pinctrl = devm_pinctrl_get_select_default(priv->dev);
-   if (IS_ERR(pinctrl)) {
-   ret = PTR_ERR(priv->vdev);
-   goto free;
-   }
-
+   if (IS_ERR(pinctrl))
+   dev_dbg(priv->dev, "pintrl_get_select_default() failed\n");
ret = v4l2_async_register_subdev(>sd);
if (ret)
goto free;

So that the error is ignored and we still can change the pinctrl values via dts.

What do you think?

Re: [PATCH] staging/imx: Fix inconsistent IS_ERR and PTR_ERR

[Fabio Estevam]

Steve, Phiipp,

On Thu, Mar 1, 2018 at 1:02 PM, Fabio Estevam <feste...@gmail.com> wrote:

> So imx_csi_probe() does not succeed anymore since
> devm_pinctrl_get_select_default() always fails.
>
> Not sure I understand the comments that explain the need for pinctrl
> handling inside the driver.
>
> Can't we just get rid of it like this?

Just tested and if devm_pinctrl_get_select_default() is removed, I am
not able to change the ipu csi pinctrl settings anymore.

I had to ignore devm_pinctrl_get_select_default() error value so that
the driver can probe again:

diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index 5a195f8..c40f786 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1797,11 +1797,8 @@ static int imx_csi_probe(struct platform_device *pdev)
 */
priv->dev->of_node = pdata->of_node;
pinctrl = devm_pinctrl_get_select_default(priv->dev);
-   if (IS_ERR(pinctrl)) {
-   ret = PTR_ERR(priv->vdev);
-   goto free;
-   }
-
+   if (IS_ERR(pinctrl))
+   dev_dbg(priv->dev, "pintrl_get_select_default() failed\n");
ret = v4l2_async_register_subdev(>sd);
if (ret)
goto free;

Is there a better solution for this issue?

Re: [PATCH] staging/imx: Fix inconsistent IS_ERR and PTR_ERR

[Fabio Estevam]

On Thu, Mar 1, 2018 at 1:09 AM, Gustavo A. R. Silva
 wrote:
> Fix inconsistent IS_ERR and PTR_ERR in imx_csi_probe.
> The proper pointer to be passed as argument is pinctrl
> instead of priv->vdev.
>
> This issue was detected with the help of Coccinelle.
>
> Fixes: 52e17089d185 ("media: imx: Don't initialize vars that won't be used")
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/staging/media/imx/imx-media-csi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/imx/imx-media-csi.c 
> b/drivers/staging/media/imx/imx-media-csi.c
> index 5a195f8..4f290a0 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -1798,7 +1798,7 @@ static int imx_csi_probe(struct platform_device *pdev)
> priv->dev->of_node = pdata->of_node;
> pinctrl = devm_pinctrl_get_select_default(priv->dev);
> if (IS_ERR(pinctrl)) {
> -   ret = PTR_ERR(priv->vdev);
> +   ret = PTR_ERR(pinctrl);
> goto free;

This patch is correct, but now I am seeing that
devm_pinctrl_get_select_default() always fails.

I added this debug line:

--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1799,6 +1799,7 @@ static int imx_csi_probe(struct platform_device *pdev)
pinctrl = devm_pinctrl_get_select_default(priv->dev);
if (IS_ERR(pinctrl)) {
ret = PTR_ERR(pinctrl);
+   pr_err(" pinctrl failed\n");
goto free;
}

and this is what I get in imx6q-sabresd:

[3.453905] imx-media: subdev ipu1_vdic bound
[3.458601] imx-media: subdev ipu2_vdic bound
[3.463341] imx-media: subdev ipu1_ic_prp bound
[3.468924] ipu1_ic_prpenc: Registered ipu1_ic_prpenc capture as /dev/video0
[3.476237] imx-media: subdev ipu1_ic_prpenc bound
[3.481621] ipu1_ic_prpvf: Registered ipu1_ic_prpvf capture as /dev/video1
[3.488805] imx-media: subdev ipu1_ic_prpvf bound
[3.493659] imx-media: subdev ipu2_ic_prp bound
[3.498839] ipu2_ic_prpenc: Registered ipu2_ic_prpenc capture as /dev/video2
[3.505958] imx-media: subdev ipu2_ic_prpenc bound
[3.511318] ipu2_ic_prpvf: Registered ipu2_ic_prpvf capture as /dev/video3
[3.518335] imx-media: subdev ipu2_ic_prpvf bound
[3.524622] ipu1_csi0: Registered ipu1_csi0 capture as /dev/video4
[3.530902] imx-media: subdev ipu1_csi0 bound
[3.535453]  pinctrl failed
[3.539684]  pinctrl failed
[3.543677]  pinctrl failed
[3.548278] imx-media: subdev imx6-mipi-csi2 bound

So imx_csi_probe() does not succeed anymore since
devm_pinctrl_get_select_default() always fails.

Not sure I understand the comments that explain the need for pinctrl
handling inside the driver.

Can't we just get rid of it like this?

--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -14,7 +14,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -1739,7 +1738,6 @@ static const struct v4l2_subdev_internal_ops
csi_internal_ops = {
 static int imx_csi_probe(struct platform_device *pdev)
 {
struct ipu_client_platformdata *pdata;
-   struct pinctrl *pinctrl;
struct csi_priv *priv;
int ret;

@@ -1789,19 +1787,7 @@ static int imx_csi_probe(struct platform_device *pdev)
v4l2_ctrl_handler_init(>ctrl_hdlr, 0);
priv->sd.ctrl_handler = >ctrl_hdlr;

-   /*
-* The IPUv3 driver did not assign an of_node to this
-* device. As a result, pinctrl does not automatically
-* configure our pin groups, so we need to do that manually
-* here, after setting this device's of_node.
-*/
priv->dev->of_node = pdata->of_node;
-   pinctrl = devm_pinctrl_get_select_default(priv->dev);
-   if (IS_ERR(pinctrl)) {
-   ret = PTR_ERR(pinctrl);
-   goto free;
-   }
-
ret = v4l2_async_register_subdev(>sd);
if (ret)
goto free;

Re: [PATCH 1/2] media: ov2680: dt: Add bindings for OV2680

[Fabio Estevam]

On Thu, Feb 22, 2018 at 7:23 AM, Rui Miguel Silva  wrote:
> Add device tree binding documentation for the OV5640 camera sensor.

s/OV5640/OV2680

[PATCH v3 1/2] media: imx-media-internal-sd: Use empty initializer

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

{0} explicitly assigns 0 to the first member of the structure.

The first element of the platform_device_info struct is a pointer and
when writing 0 to a pointer the following sparse error is seen:

drivers/staging/media/imx/imx-media-internal-sd.c:274:49: warning: Using plain 
integer as NULL pointer

Fix this problem by using an empty initializer, which also clears the
struct members and avoids the sparse warning.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
Acked-by: Steve Longerbeam <steve_longerb...@mentor.com>
---
Changes sice v2:
- Improve commit log

Changes since v1:
- Use empty initializer insted of memset() - Ian

 drivers/staging/media/imx/imx-media-internal-sd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-internal-sd.c 
b/drivers/staging/media/imx/imx-media-internal-sd.c
index 70833fe..daf66c2 100644
--- a/drivers/staging/media/imx/imx-media-internal-sd.c
+++ b/drivers/staging/media/imx/imx-media-internal-sd.c
@@ -271,7 +271,7 @@ static int add_internal_subdev(struct imx_media_dev *imxmd,
   int ipu_id)
 {
struct imx_media_internal_sd_platformdata pdata;
-   struct platform_device_info pdevinfo = {0};
+   struct platform_device_info pdevinfo = {};
struct platform_device *pdev;
 
pdata.grp_id = isd->id->grp_id;
-- 
2.7.4

[PATCH v3 2/2] media: imx-ic-prpencvf: Use empty initializer

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

{0} explicitly assigns 0 to the first member of the structure.

Even though the first member of the v4l2_subdev_format structure is of
_u32 type, make the initialization more robust by using the empty
initializer.

This way in case someday the struct is changed so that the first
member becomes a pointer, we will not have sparse warnings.

Reported-by: Steve Longerbeam <slongerb...@gmail.com>
Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
Changes since v1:
- Improve commit log

 drivers/staging/media/imx/imx-ic-prpencvf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c 
b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 143038c..60f392a 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -923,7 +923,7 @@ static int prp_enum_frame_size(struct v4l2_subdev *sd,
   struct v4l2_subdev_frame_size_enum *fse)
 {
struct prp_priv *priv = sd_to_priv(sd);
-   struct v4l2_subdev_format format = {0};
+   struct v4l2_subdev_format format = {};
const struct imx_media_pixfmt *cc;
int ret = 0;
 
-- 
2.7.4

Re: i.MX53 using imx-media to capture analog video through ADV7180

[Fabio Estevam]

[Adding Steve and Philipp in case they could provide some suggestions]

On Wed, Feb 14, 2018 at 1:21 PM, Matthew Starr  wrote:
> I have successfully modified device tree files in the mainline 4.15.1 kernel 
> to get a display product using the i.MX53 processor to initialize the 
> imx-media drivers.  I think up to this point they have only been tested on 
> i.MX6 processors.  I am using two ADV7180 analog capture chips, one per CSI 
> port, on this display product.
>
> I have everything initialize successfully at boot, but I am unable to get the 
> media-ctl command to link the ADV7180 devices to the CSI ports.  I used the 
> following website as guidance of how to setup the links between media devices:
> https://linuxtv.org/downloads/v4l-dvb-apis/v4l-drivers/imx.html
>
> When trying to link the ADV7180 chip to a CSI port, I use the following 
> command and get the result below:
>
> media-ctl -v -l "'adv7180 1-0021':0->'ipu1_csi0':0[1]"
>
> No link between "adv7180 1-0021":0 and "ipu1_csi0":0
> media_parse_setup_link: Unable to parse link
> Unable to parse link: Invalid argument (22)
>
> How do I get the ADV7180 and CSI port on the i.MX53 processor to link?
>
> The difference for the i.MX53 compared to the i.MX6 processor is that there 
> is only one IPU and no mipi support, so my device tree does not use any 
> video-mux or mux devices.  Could this have something to do with why I can't 
> link the ADV7180 to the CSI port?
>
> Here is the output of the "media-ctl -p -v" command:
>
> Opening media device /dev/media0
> Enumerating entities
> looking up device: 81:10
> looking up device: 81:11
> looking up device: 81:12
> looking up device: 81:4
> looking up device: 81:13
> looking up device: 81:5
> looking up device: 81:14
> looking up device: 81:15
> looking up device: 81:16
> looking up device: 81:17
> looking up device: 81:6
> looking up device: 81:18
> looking up device: 81:7
> looking up device: 81:19
> looking up device: 81:20
> looking up device: 81:8
> looking up device: 81:21
> looking up device: 81:9
> Found 18 entities
> Enumerating pads and links
> Media controller API version 4.15.1
>
> Media device information
> 
> driver  imx-media
> model   imx-media
> serial
> bus info
> hw revision 0x0
> driver version  4.15.1
>
> Device topology
> - entity 1: adv7180 1-0021 (1 pad, 0 link)
> type V4L2 subdev subtype Unknown flags 20004
> device node name /dev/v4l-subdev0
> pad0: Source
> [fmt:UYVY8_2X8/720x480 field:interlaced]
>
> - entity 3: adv7180 1-0020 (1 pad, 0 link)
> type V4L2 subdev subtype Unknown flags 20004
> device node name /dev/v4l-subdev1
> pad0: Source
> [fmt:UYVY8_2X8/720x480 field:interlaced]
>
> - entity 5: ipu1_csi1 (3 pads, 3 links)
> type V4L2 subdev subtype Unknown flags 0
> device node name /dev/v4l-subdev2
> pad0: Sink
> [fmt:UYVY8_2X8/640x480 field:none
>  crop.bounds:(0,0)/640x480
>  crop:(0,0)/640x480
>  compose.bounds:(0,0)/640x480
>  compose:(0,0)/640x480]
> pad1: Source
> [fmt:AYUV8_1X32/640x480 field:none]
> -> "ipu1_ic_prp":0 []
> -> "ipu1_vdic":0 []
> pad2: Source
> [fmt:AYUV8_1X32/640x480 field:none]
> -> "ipu1_csi1 capture":0 []
>
> - entity 9: ipu1_csi1 capture (1 pad, 1 link)
> type Node subtype V4L flags 0
> device node name /dev/video4
> pad0: Sink
> <- "ipu1_csi1":2 []
>
> - entity 15: ipu1_csi0 (3 pads, 3 links)
>  type V4L2 subdev subtype Unknown flags 0
>  device node name /dev/v4l-subdev3
> pad0: Sink
> [fmt:UYVY8_2X8/640x480 field:none
>  crop.bounds:(0,0)/640x480
>  crop:(0,0)/640x480
>  compose.bounds:(0,0)/640x480
>  compose:(0,0)/640x480]
> pad1: Source
> [fmt:AYUV8_1X32/640x480 field:none]
> -> "ipu1_ic_prp":0 []
> -> "ipu1_vdic":0 [ENABLED]
> pad2: Source
> [fmt:AYUV8_1X32/640x480 field:none]
> -> "ipu1_csi0 capture":0 []
>
> - entity 19: ipu1_csi0 capture (1 pad, 1 link)
>  type Node subtype V4L flags 0
>  device node name /dev/video5
> pad0: Sink
> <- "ipu1_csi0":2 []
>
> - entity 25: ipu1_ic_prp (3 pads, 5 links)
>  type V4L2 subdev subtype Unknown flags 0
>  device node name /dev/v4l-subdev4
> pad0: Sink
> [fmt:AYUV8_1X32/640x480 field:none]
> <- "ipu1_csi1":1 []
> <- "ipu1_csi0":1 []
> <- "ipu1_vdic":2 [ENABLED]
> pad1: Source
> 

Re: [PATCH v2] media: imx-media-internal-sd: Use empty initializer

[Fabio Estevam]

Hi Steve,

On Fri, Feb 9, 2018 at 9:42 PM, Steve Longerbeam  wrote:

> There is one other case of the use of "{0}" to initialize a stack/local
> struct, in prp_enum_frame_size(). It should be fixed there as well, to
> be consistent.

Yes, just sent a patch as you suggested.

Thanks

[PATCH] media: imx-ic-prpencvf: Use empty initializer to clear all struct members

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

{0} only cleans the first member of the structure.

Use {} instead, which cleans all the members of the structure.

Reported-by: Steve Longerbeam <slongerb...@gmail.com>
Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/staging/media/imx/imx-ic-prpencvf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c 
b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 143038c..60f392a 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -923,7 +923,7 @@ static int prp_enum_frame_size(struct v4l2_subdev *sd,
   struct v4l2_subdev_frame_size_enum *fse)
 {
struct prp_priv *priv = sd_to_priv(sd);
-   struct v4l2_subdev_format format = {0};
+   struct v4l2_subdev_format format = {};
const struct imx_media_pixfmt *cc;
int ret = 0;
 
-- 
2.7.4

[PATCH v2] media: imx-media-internal-sd: Use empty initializer

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

When building with W=1 the following warning shows up:

drivers/staging/media/imx/imx-media-internal-sd.c:274:49: warning: Using plain 
integer as NULL pointer

Fix this problem by using an empty initializer, which guarantees that all
struct members are zero-cleared.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
Changes since v1:
- Use empty initializer insted of memset() - Ian

 drivers/staging/media/imx/imx-media-internal-sd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-internal-sd.c 
b/drivers/staging/media/imx/imx-media-internal-sd.c
index 70833fe..daf66c2 100644
--- a/drivers/staging/media/imx/imx-media-internal-sd.c
+++ b/drivers/staging/media/imx/imx-media-internal-sd.c
@@ -271,7 +271,7 @@ static int add_internal_subdev(struct imx_media_dev *imxmd,
   int ipu_id)
 {
struct imx_media_internal_sd_platformdata pdata;
-   struct platform_device_info pdevinfo = {0};
+   struct platform_device_info pdevinfo = {};
struct platform_device *pdev;
 
pdata.grp_id = isd->id->grp_id;
-- 
2.7.4

[PATCH] media: imx-media-internal-sd: Use memset to clear pdevinfo struct

[Fabio Estevam]

When building with W=1 the following warning shows up:

drivers/staging/media/imx/imx-media-internal-sd.c:274:49: warning: Using plain 
integer as NULL pointer

Fix this problem by using memset() to clear the pdevinfo structure.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/staging/media/imx/imx-media-internal-sd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-internal-sd.c 
b/drivers/staging/media/imx/imx-media-internal-sd.c
index 70833fe503b5..377c20863b76 100644
--- a/drivers/staging/media/imx/imx-media-internal-sd.c
+++ b/drivers/staging/media/imx/imx-media-internal-sd.c
@@ -271,7 +271,7 @@ static int add_internal_subdev(struct imx_media_dev *imxmd,
   int ipu_id)
 {
struct imx_media_internal_sd_platformdata pdata;
-   struct platform_device_info pdevinfo = {0};
+   struct platform_device_info pdevinfo;
struct platform_device *pdev;
 
pdata.grp_id = isd->id->grp_id;
@@ -283,6 +283,7 @@ static int add_internal_subdev(struct imx_media_dev *imxmd,
imx_media_grp_id_to_sd_name(pdata.sd_name, sizeof(pdata.sd_name),
pdata.grp_id, ipu_id);
 
+   memset(, 0, sizeof(pdevinfo));
pdevinfo.name = isd->id->name;
pdevinfo.id = ipu_id * num_isd + isd->id->index;
pdevinfo.parent = imxmd->md.dev;
-- 
2.14.1

Re: [PATCH v2 9/9] media: i2c: tw9910: Remove soc_camera dependencies

[Fabio Estevam]

On Wed, Jan 3, 2018 at 3:37 PM, jacopo mondi  wrote:

>> Initially the rest GPIO was doing:
>>
>> -   gpio_set_value(GPIO_PTT3, 0);
>> -   mdelay(10);
>> -   gpio_set_value(GPIO_PTT3, 1);
>> -   mdelay(10); /* wait to let chip come out of reset */
>
> And that's what my driver code does :)

No, on 5/9 you converted the original code to:

GPIO_LOOKUP("sh7722_pfc", GPIO_PTT3, "rstb", GPIO_ACTIVE_HIGH)

It should be GPIO_ACTIVE_LOW instead.

> My point is that if I read the manual and I see an active low gpio (0
> is reset state) then the driver code uses it as and active high one (1
> is the reset state), that would be weird to me.

Then on this patch you should do:

gpiod_set_value(priv->rstb_gpio, 1);  ---> This tells the GPIO to go
to its active state (In this case active == logic level 0)
usleep_range(500, 1000);
gpiod_set_value(priv->rstb_gpio, 0); ---> This tells the GPIO to go to
its inactive state (In this case inactive == logic level 1)

You can also look at Documentation/gpio/consumer.txt where the usage
of the gpiod_xxx API is described.

It seems you are confusing it with the legacy gpio_set_value() API
(Documentation/gpio/gpio-legacy.txt)

Hope this helps.

Re: [PATCH v2 9/9] media: i2c: tw9910: Remove soc_camera dependencies

[Fabio Estevam]

Hi Jacopo,

On Wed, Jan 3, 2018 at 3:13 PM, jacopo mondi  wrote:

> That would be true if I would have declared the GPIO to be ACTIVE_LOW.
> See patch [5/9] in this series and search for "rstb". The GPIO (which
> is shared between two devices) is said to be active high...

Just looked at your patch 5/9 and it seems it only works because you
made two inversions :-)

Initially the rest GPIO was doing:

-   gpio_set_value(GPIO_PTT3, 0);
-   mdelay(10);
-   gpio_set_value(GPIO_PTT3, 1);
-   mdelay(10); /* wait to let chip come out of reset */

So this is an active low reset.

So you should have converted it to:

GPIO_LOOKUP("sh7722_pfc", GPIO_PTT3, "rstb", GPIO_ACTIVE_LOW),

and then in this patch you should do as I said earlier:

gpiod_set_value(priv->rstb_gpio, 1);
usleep_range(500, 1000);
gpiod_set_value(priv->rstb_gpio, 0);

[PATCH] media: ov5640: Check the return value from clk_prepare_enable()

[Fabio Estevam]

clk_prepare_enable() may fail, so we should better check its return value
and propagate it in the case of error.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/media/i2c/ov5640.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index c89ed66..fd93b14 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1552,7 +1552,9 @@ static int ov5640_set_power(struct ov5640_dev *sensor, 
bool on)
int ret = 0;
 
if (on) {
-   clk_prepare_enable(sensor->xclk);
+   ret = clk_prepare_enable(sensor->xclk);
+   if (ret)
+   return ret;
 
ret = regulator_bulk_enable(OV5640_NUM_SUPPLIES,
sensor->supplies);
-- 
2.7.4

Re: [PATCH v2 9/9] media: i2c: tw9910: Remove soc_camera dependencies

[Fabio Estevam]

Hi Jacopo,

On Thu, Dec 28, 2017 at 12:01 PM, Jacopo Mondi
 wrote:

> +   if (priv->rstb_gpio) {
> +   gpiod_set_value(priv->rstb_gpio, 0);
> +   usleep_range(500, 1000);
> +   gpiod_set_value(priv->rstb_gpio, 1);
> +   usleep_range(500, 1000);

This seems to be inverted.

Consider you have an active low GPIO reset.

In order to reset it:

Put the GPIO to logic level 0
Wait some time
Put the GPIO to logic level 1

gpiod_set_value(priv->rstb_gpio, 1), means the GPIO in the active
state (0 in this example).

, so this should be:

gpiod_set_value(priv->rstb_gpio, 1);
usleep_range(500, 1000);
gpiod_set_value(priv->rstb_gpio, 0);

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Fabio Estevam]

On Tue, Dec 19, 2017 at 11:43 AM, Sakari Ailus  wrote:

> Both seem to exist. See e.g. c3a3d1d6b8b363a02234e5564692db3647f183e6 .

This patch fixes .h files to use /* SPDX style comment, which is the
recommendation.

.c files should use // SPDX style.

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Fabio Estevam]

Hi Sakari,

On Tue, Dec 19, 2017 at 11:05 AM, Sakari Ailus  wrote:

> I guess it depends on who do you ask and when. Looking at what has been
> recently merged to media tree master, the latter is preferred.

Just did 'git grep SPDX drivers/media'

and it consistently shows // SPDX style for C files.

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Fabio Estevam]

Hi Sakari,

On Tue, Dec 19, 2017 at 7:22 AM, Sakari Ailus  wrote:
> On Mon, Dec 11, 2017 at 09:31:46AM +0800, Wenyou Yang wrote:
>> The ov7740 (color) image sensor is a high performance VGA CMOS
>> image snesor, which supports for output formats: RAW RGB and YUV
>> and image sizes: VGA, and QVGA, CIF and any size smaller.
>>
>> Signed-off-by: Songjun Wu 
>> Signed-off-by: Wenyou Yang 
>
> Applied with this diff:
>
> diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
> index 0308ba437bbb..041a77039d70 100644
> --- a/drivers/media/i2c/ov7740.c
> +++ b/drivers/media/i2c/ov7740.c
> @@ -1,5 +1,7 @@
> -// SPDX-License-Identifier: GPL-2.0
> -// Copyright (c) 2017 Microchip Corporation.
> +/*
> + * SPDX-License-Identifier: GPL-2.0
> + * Copyright (c) 2017 Microchip Corporation.
> + */

The original version is the recommended format for the SPDX identifier.

[PATCH] [media] coda/imx-vdoa: Remove irq member from vdoa_data struct

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

The 'irq' member of the vdoa_data struct is only used inside probe,
so there is no need for it. Use a local variable 'ret' instead.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/media/platform/coda/imx-vdoa.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/coda/imx-vdoa.c 
b/drivers/media/platform/coda/imx-vdoa.c
index 8eb3e0c..eec27d3 100644
--- a/drivers/media/platform/coda/imx-vdoa.c
+++ b/drivers/media/platform/coda/imx-vdoa.c
@@ -86,7 +86,6 @@ struct vdoa_data {
struct device   *dev;
struct clk  *vdoa_clk;
void __iomem*regs;
-   int irq;
 };
 
 struct vdoa_q_data {
@@ -293,6 +292,7 @@ static int vdoa_probe(struct platform_device *pdev)
 {
struct vdoa_data *vdoa;
struct resource *res;
+   int ret;
 
dma_set_coherent_mask(>dev, DMA_BIT_MASK(32));
 
@@ -316,12 +316,12 @@ static int vdoa_probe(struct platform_device *pdev)
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (!res)
return -EINVAL;
-   vdoa->irq = devm_request_threaded_irq(>dev, res->start, NULL,
+   ret = devm_request_threaded_irq(>dev, res->start, NULL,
vdoa_irq_handler, IRQF_ONESHOT,
"vdoa", vdoa);
-   if (vdoa->irq < 0) {
+   if (ret < 0) {
dev_err(vdoa->dev, "Failed to get irq\n");
-   return vdoa->irq;
+   return ret;
}
 
platform_set_drvdata(pdev, vdoa);
-- 
2.7.4

Re: [PATCH v2 2/2] media: coda: Add i.MX51 (CodaHx4) support

[Fabio Estevam]

On Mon, Dec 18, 2017 at 8:16 AM, Philipp Zabel <p.za...@pengutronix.de> wrote:
> Add support for the CodaHx4 VPU used on i.MX51.
>
> Decoding h.264, MPEG-4, and MPEG-2 video works, as well as encoding
> h.264. MPEG-4 encoding is not enabled, it currently produces visual
> artifacts.
>
> Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>

Reviewed-by: Fabio Estevam <fabio.este...@nxp.com>

Re: [alsa-devel] [PATCH 3/5] media: i2c: Add TDA1997x HDMI receiver driver

[Fabio Estevam]

Hi Tim,

On Thu, Nov 9, 2017 at 4:45 PM, Tim Harvey  wrote:

> +static int tda1997x_set_power(struct tda1997x_state *state, bool on)
> +{
> +   int ret = 0;
> +
> +   if (on) {
> +   ret = regulator_bulk_enable(TDA1997X_NUM_SUPPLIES,
> +state->supplies);
> +   msleep(300);

Didn't you miss a 'return ret' here?

Otherwise regulator_bulk_disable() will always be called below.

> +   }
> +
> +   ret = regulator_bulk_disable(TDA1997X_NUM_SUPPLIES,
> +  state->supplies);
> +   return ret;
> +}

Re: [PATCH v2 3/4] media: ov5640: add support of DVP parallel interface

[Fabio Estevam]

Hi Hugues,

On Wed, Nov 29, 2017 at 3:11 PM, Hugues Fruchet  wrote:
> Add support of DVP parallel mode in addition of
> existing MIPI CSI mode. The choice between two modes
> and configuration is made through device tree.

What about explaining how to select between the two modes in
Documentation/devicetree/bindings/media/i2c/ov5640.txt ?

Thanks

Re: [PATCH v2 2/4] media: ov5640: check chip id

[Fabio Estevam]

Hi Hugues,

On Wed, Nov 29, 2017 at 3:11 PM, Hugues Fruchet  wrote:

>  /* read exposure, in number of line periods */
>  static int ov5640_get_exposure(struct ov5640_dev *sensor)
>  {
> @@ -1562,6 +1586,10 @@ static int ov5640_set_power(struct ov5640_dev *sensor, 
> bool on)
> ov5640_reset(sensor);
> ov5640_power(sensor, true);
>
> +   ret = ov5640_check_chip_id(sensor);
> +   if (ret)
> +   goto power_off;

Wouldn't it make more sense to add this check in ov5640_probe()
function instead?

Re: [PATCH][MEDIA]i.MX6 CSI: Fix MIPI camera operation in RAW/Bayer mode

[Fabio Estevam]

On Tue, Oct 17, 2017 at 11:53 AM, Russell King - ARM Linux
 wrote:

> I do wish the patch was merged (which fixes a real problem) rather than
> hanging around for optimisation questions.  We can always increase it
> in the future if it's deemed that a larger burst size is beneficial.

Agreed.

Re: [PATCH][MEDIA]i.MX6 CSI: Fix MIPI camera operation in RAW/Bayer mode

[Fabio Estevam]

Hi Krzysztof,

On Tue, Oct 17, 2017 at 4:27 AM, Krzysztof Hałasa  wrote:
> Without this fix, in 8-bit Y/Bayer mode, every other 8-byte group
> of pixels is lost.
> Not sure about possible side effects, though.
>
> Signed-off-by: Krzysztof Hałasa 
>
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -340,7 +340,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
> case V4L2_PIX_FMT_SGBRG8:
> case V4L2_PIX_FMT_SGRBG8:
> case V4L2_PIX_FMT_SRGGB8:
> -   burst_size = 8;
> +   burst_size = 16;
> passthrough = true;
> passthrough_bits = 8;
> break;

Russell has sent the same fix and Philipp made a comment about the
possibility of using 32-byte or 64-byte bursts:
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2017-October/111960.html

Re: [PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock

[Fabio Estevam]

Hi Pierre,

On Sat, Oct 14, 2017 at 10:21 AM, Pierre-Hugues Husson <p...@phh.me> wrote:

> As far as I understand, dw_hdmi_cec_enable only gates
> the CEC clock inside the Synopsis IP,
> but the SOC still has to provide a specific CEC clock to it.
> To enable such an external CEC clock, the binding documentation [1]
> mentions a "cec" optional clock, and I'm not seeing any code in dw-hdmi
> to enable it.

Ok, understood.

In the case of i.MX6 we control the CEC clock via
HDMI_MC_CLKDIS_CECCLK_DISABLE bit inside the Synopsis IP.

Looks like Rockchip needs the external CEC clock then.

Regards,

Fabio Estevam

Re: [PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock

[Fabio Estevam]

Hi Pierre-Hugues,

On Fri, Oct 13, 2017 at 7:53 PM, Pierre-Hugues Husson  wrote:
> The documentation already mentions "cec" optional clock, but
> currently the driver doesn't enable it.

The cec clock is enabled at dw_hdmi_cec_enable().

Re: [PATCH 1/1] v4l: async: Fix notifier complete callback error handling

[Fabio Estevam]

Hi Russell,

On Mon, Oct 2, 2017 at 7:12 PM, Russell King - ARM Linux
 wrote:

>> 
>
> Any chance of sending me that patch so I can test this patch?  I'd
> rather not manually de-html-ise the above patch.

You can get it from https://patchwork.linuxtv.org/patch/44512/

(Click on the patch link and it will download a pure patch file)

Re: IMX6 ADV7180 no /dev/media

[Fabio Estevam]

On Fri, Sep 15, 2017 at 8:46 PM, Fabio Estevam <feste...@gmail.com> wrote:

> Tim,
>
> Care to send the patch enabling CONFIG_VIDEO_MUX?

Nevermind. I have just sent it.

Re: IMX6 ADV7180 no /dev/media

[Fabio Estevam]

Hi Steve,

On Fri, Sep 15, 2017 at 8:39 PM, Steve Longerbeam  wrote:

> Agreed, but I notice now that CONFIG_MEDIA_CONTROLLER and
> CONFIG_VIDEO_V4L2_SUBDEV_API are not enabled there anymore.

I do see them enabled in mainline with imx_v6_v7_defconfig.

Tim,

Care to send the patch enabling CONFIG_VIDEO_MUX?

Thanks

[PATCH 1/4] [media] max2175: Propagate the real error on devm_clk_get() failure

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

When devm_clk_get() fails we should return the real error code
instead of always returning -ENODEV.

This allows defer probe to happen in the case the clock provider has
not been enabled by the time max2175 driver gets probed.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/media/i2c/max2175.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/max2175.c b/drivers/media/i2c/max2175.c
index a4736a8..bf0e821 100644
--- a/drivers/media/i2c/max2175.c
+++ b/drivers/media/i2c/max2175.c
@@ -1319,7 +1319,7 @@ static int max2175_probe(struct i2c_client *client,
if (IS_ERR(clk)) {
ret = PTR_ERR(clk);
dev_err(>dev, "cannot get clock %d\n", ret);
-   return -ENODEV;
+   return ret;
}
 
regmap = devm_regmap_init_i2c(client, _regmap_config);
-- 
2.7.4

[PATCH 3/4] [media] ov2640: Propagate the real error on devm_clk_get() failure

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

devm_clk_get() may return different error codes other than -EPROBE_DEFER,
so it is better to return the real error code instead.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/media/i2c/ov2640.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
index e6d0c1f..e6cbe01 100644
--- a/drivers/media/i2c/ov2640.c
+++ b/drivers/media/i2c/ov2640.c
@@ -1107,7 +1107,7 @@ static int ov2640_probe(struct i2c_client *client,
if (client->dev.of_node) {
priv->clk = devm_clk_get(>dev, "xvclk");
if (IS_ERR(priv->clk))
-   return -EPROBE_DEFER;
+   return PTR_ERR(priv->clk);
clk_prepare_enable(priv->clk);
}
 
-- 
2.7.4

[PATCH 2/4] [media] mt9m111: Propagate the real error on v4l2_clk_get() failure

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

v4l2_clk_get() may return different error codes other than -EPROBE_DEFER,
so it is better to return the real error code instead.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/media/i2c/mt9m111.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
index 99b992e..b1665d9 100644
--- a/drivers/media/i2c/mt9m111.c
+++ b/drivers/media/i2c/mt9m111.c
@@ -945,7 +945,7 @@ static int mt9m111_probe(struct i2c_client *client,
 
mt9m111->clk = v4l2_clk_get(>dev, "mclk");
if (IS_ERR(mt9m111->clk))
-   return -EPROBE_DEFER;
+   return PTR_ERR(mt9m111->clk);
 
/* Default HIGHPOWER context */
mt9m111->ctx = _b;
-- 
2.7.4

[PATCH 4/4] [media] ov2640: Check the return value from clk_prepare_enable()

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

clk_prepare_enable() may fail, so we should better check its return value
and propagate it in the case of error.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/media/i2c/ov2640.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
index e6cbe01..5f013c8 100644
--- a/drivers/media/i2c/ov2640.c
+++ b/drivers/media/i2c/ov2640.c
@@ -1108,7 +1108,9 @@ static int ov2640_probe(struct i2c_client *client,
priv->clk = devm_clk_get(>dev, "xvclk");
if (IS_ERR(priv->clk))
return PTR_ERR(priv->clk);
-   clk_prepare_enable(priv->clk);
+   ret = clk_prepare_enable(priv->clk);
+   if (ret)
+   return ret;
}
 
ret = ov2640_probe_dt(client, priv);
-- 
2.7.4

[PATCH] [media] mx2_emmaprp: Check for platform_get_irq() error

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

platform_get_irq() may fail, so we should better check its return
value and propagate it in the case of error.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/media/platform/mx2_emmaprp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/mx2_emmaprp.c 
b/drivers/media/platform/mx2_emmaprp.c
index 03e47e0..f90eaa0 100644
--- a/drivers/media/platform/mx2_emmaprp.c
+++ b/drivers/media/platform/mx2_emmaprp.c
@@ -942,6 +942,8 @@ static int emmaprp_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, pcdev);
 
irq = platform_get_irq(pdev, 0);
+   if (irq < 0)
+   return irq;
ret = devm_request_irq(>dev, irq, emmaprp_irq, 0,
   dev_name(>dev), pcdev);
if (ret)
-- 
2.7.4

[PATCH] [media] coda/imx-vdoa: Check for platform_get_resource() error

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

platform_get_resource() may fail and in this case a NULL dereference
will occur.

Prevent this from happening by returning an error on
platform_get_resource() failure. 

Fixes: b0444f18e0b18abce ("[media] coda: add i.MX6 VDOA driver")
Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/media/platform/coda/imx-vdoa.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/coda/imx-vdoa.c 
b/drivers/media/platform/coda/imx-vdoa.c
index df9b716..8eb3e0c 100644
--- a/drivers/media/platform/coda/imx-vdoa.c
+++ b/drivers/media/platform/coda/imx-vdoa.c
@@ -314,6 +314,8 @@ static int vdoa_probe(struct platform_device *pdev)
return PTR_ERR(vdoa->regs);
 
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+   if (!res)
+   return -EINVAL;
vdoa->irq = devm_request_threaded_irq(>dev, res->start, NULL,
vdoa_irq_handler, IRQF_ONESHOT,
"vdoa", vdoa);
-- 
2.7.4

Re: [PATCH 1/2] [media] ov7670: Return the real error code

[Fabio Estevam]

Hi Sakari,

On Tue, Aug 8, 2017 at 8:24 AM, Sakari Ailus  wrote:

> I don't think -EPROBE_DEFER is returned by clk_get() if the clock can't be
> found. The clock providers often are e.g. ISP drivers that may well be

Yes, it is.

Please check:

commit a34cd4666f3da84228a82f70c94b8d9b692034ea
Author: Jean-Francois Moine 
Date:   Mon Nov 25 19:47:04 2013 +0100

clk: return probe defer when DT clock not yet ready

At probe time, a clock device may not be ready when some other device
wants to use it.

This patch lets the functions clk_get/devm_clk_get return a probe defer
when the clock is defined in the DT but not yet available.

Signed-off-by: Jean-Francois Moine 
Reviewed-by: Sylwester Nawrocki 
Tested-by: Sylwester Nawrocki 
Signed-off-by: Mike Turquette 

[PATCH] [media] ov5640: Remove unneeded gpiod NULL check

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

The gpiod API checks for NULL descriptors, so there is no need to
duplicate the check in the driver.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/media/i2c/ov5640.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 1f5b483..39a2269 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1524,8 +1524,7 @@ static int ov5640_restore_mode(struct ov5640_dev *sensor)
 
 static void ov5640_power(struct ov5640_dev *sensor, bool enable)
 {
-   if (sensor->pwdn_gpio)
-   gpiod_set_value(sensor->pwdn_gpio, enable ? 0 : 1);
+   gpiod_set_value(sensor->pwdn_gpio, enable ? 0 : 1);
 }
 
 static void ov5640_reset(struct ov5640_dev *sensor)
-- 
2.7.4

[PATCH 2/2] [media] ov7670: Check the return value from clk_prepare_enable()

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

clk_prepare_enable() may fail, so we should better check its return value
and propagate it in the case of error.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/media/i2c/ov7670.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 552a881..e88549f 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1615,7 +1615,9 @@ static int ov7670_probe(struct i2c_client *client,
info->clk = devm_clk_get(>dev, "xclk");
if (IS_ERR(info->clk))
return PTR_ERR(info->clk);
-   clk_prepare_enable(info->clk);
+   ret = clk_prepare_enable(info->clk);
+   if (ret)
+   return ret;
 
ret = ov7670_init_gpio(client, info);
if (ret)
-- 
2.7.4

[PATCH 1/2] [media] ov7670: Return the real error code

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

When devm_clk_get() fails the real error code should be propagated,
instead of always returning -EPROBE_DEFER.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/media/i2c/ov7670.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 7270c68..552a881 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1614,7 +1614,7 @@ static int ov7670_probe(struct i2c_client *client,
 
info->clk = devm_clk_get(>dev, "xclk");
if (IS_ERR(info->clk))
-   return -EPROBE_DEFER;
+   return PTR_ERR(info->clk);
clk_prepare_enable(info->clk);
 
ret = ov7670_init_gpio(client, info);
-- 
2.7.4

Re: coda 2040000.vpu: firmware request failed

[Fabio Estevam]

Hi Jagan,

On Fri, Jul 7, 2017 at 9:45 AM, Jagan Teki  wrote:
> Hi,
>
> I'm observing firmware request failure with i.MX6Q board, This is with
> latest linux-next (4.12) with firmware from, [1] and converted
> v4l-coda960-imx6q.bin using [2].

There is no need to do the conversion with current code.

Re: Shawn Guo: your attetion is needed here Re: [PATCH v8 00/34] i.MX Media Driver

[Fabio Estevam]

On Tue, Jun 20, 2017 at 5:29 AM, Pavel Machek  wrote:

> Hmm. I changed the subject to grab Shawn's attetion.
>
> But his acks should not be needed for forward progress. Yes, it would
> be good, but he does not react -- so just reorder the series so that
> dts changes come last, then apply the parts you can apply: driver can
> go in.
>
> And actually... if maintainer does not respond at all, there are ways
> to deal with that, too...

Shawn has already applied the dts part of the series and they show up
in linux-next.

Re: how to link up audio bus from media controller driver to soc dai bus?

[Fabio Estevam]

Hi Tim,

On Mon, Jun 12, 2017 at 4:15 PM, Tim Harvey  wrote:
> Greetings,
>
> I'm working on a media controller driver for the tda1997x HDMI
> receiver which provides an audio bus supporting I2S/SPDIF/OBA/HBR/DST.
> I'm unclear how to bind the audio bus to a SoC's audio bus, for
> example the IMX6 SSI (I2S) bus. I thought perhaps it was via a
> simple-audio-card device-tree binding but that appears to require an
> ALSA codec to bind to?
>
> Can anyone point me to an example of a media controller device driver
> that supports audio and video and how the audio is bound to a I2S bus?

Does the tda998x.txt example help?

Documentation/devicetree/bindings/display/bridge/tda998x.txt

and the dts example:

arch/arm/boot/dts/am335x-boneblack-common.dtsi

Re: [PATCH v5 07/39] ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround

[Fabio Estevam]

On Fri, Mar 10, 2017 at 6:57 PM, Pavel Machek  wrote:

> And it should not depend on configuration. Hardware vendor should be
> able to ship board with working device tree...

We are talking about pin conflict here. Please read the commit log of
this patch for details.

Re: [PATCH v5 07/39] ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround

[Fabio Estevam]

On Fri, Mar 10, 2017 at 3:59 PM, Troy Kisky
 wrote:
> On 3/9/2017 8:52 PM, Steve Longerbeam wrote:
>> There is a pin conflict with GPIO_6. This pin functions as a power
>> input pin to the OV5642 camera sensor, but ENET uses it as the h/w
>> workaround for erratum ERR006687, to wake-up the ARM cores on normal
>> RX and TX packet done events. So we need to remove the h/w workaround
>> to support the OV5642. The result is that the CPUidle driver will no
>> longer allow entering the deep idle states on the sabrelite.
>>
>> This is a partial revert of
>>
>> commit 6261c4c8f13e ("ARM: dts: imx6qdl-sabrelite: use GPIO_6 for FEC
>>   interrupt.")
>> commit a28eeb43ee57 ("ARM: dts: imx6: tag boards that have the HW workaround
>>   for ERR006687")
>>
>> Signed-off-by: Steve Longerbeam 
>> ---
>>  arch/arm/boot/dts/imx6qdl-sabrelite.dtsi | 4 
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi 
>> b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
>> index 8413179..89dce27 100644
>> --- a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
>> +++ b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
>> @@ -270,9 +270,6 @@
>>   txd1-skew-ps = <0>;
>>   txd2-skew-ps = <0>;
>>   txd3-skew-ps = <0>;
>
> How about
>
> +#if !IS_ENABLED(CONFIG_VIDEO_OV5642)

Or maybe just create a new device tree for using the camera, like
imx6q-sabrelite-camera.dts.

This way we can keep the FEC erratum for the existing sabrelite dtb's.

Re: [PATCH] [media] coda: restore original firmware locations

[Fabio Estevam]

On Wed, Mar 1, 2017 at 12:36 PM, Philipp Zabel <p.za...@pengutronix.de> wrote:
> Recently, an unfinished patch was merged that added a third entry to the
> beginning of the array of firmware locations without changing the code
> to also look at the third element, thus pushing an old firmware location
> off the list.
>
> Fixes: 8af7779f3cbc ("[media] coda: add Freescale firmware compatibility 
> location")
> Cc: Baruch Siach <bar...@tkos.co.il>
> Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>

Reviewed-by: Fabio Estevam <fabio.este...@nxp.com>

Re: [PATCH v4 07/36] ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors

[Fabio Estevam]

Hi Steve,

On Thu, Feb 16, 2017 at 12:19 AM, Steve Longerbeam
 wrote:
> Enables the OV5642 parallel-bus sensor, and the OV5640 MIPI CSI-2 sensor.
>
> The OV5642 connects to the parallel-bus mux input port on ipu1_csi0_mux.
>
> The OV5640 connects to the input port on the MIPI CSI-2 receiver on
> mipi_csi.
>
> Until the OV5652 sensor module compatible with the SabreSD becomes
> available for testing, the ov5642 node is currently disabled.

You missed your Signed-off-by.

Re: [git:media_tree/master] [media] coda: add Freescale firmware compatibility location

[Fabio Estevam]

On Fri, Feb 3, 2017 at 2:23 PM, Mauro Carvalho Chehab
<mche...@s-opensource.com> wrote:
> This is an automatic generated email to let you know that the following patch 
> were queued:
>
> Subject: [media] coda: add Freescale firmware compatibility location
> Author:  Baruch Siach <bar...@tkos.co.il>
> Date:Sun Jan 15 08:33:53 2017 -0200
>
> The Freescale provided imx-vpu looks for firmware files under 
> /lib/firmware/vpu
> by default. Make coda look there for firmware files to ease the update path.
>
> Cc: Fabio Estevam <feste...@gmail.com>
> Signed-off-by: Baruch Siach <bar...@tkos.co.il>
> Reviewed-by: Fabio Estevam <fabio.este...@nxp.com>
> Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
>
>  drivers/media/platform/coda/coda-common.c | 4 
>  drivers/media/platform/coda/coda.h| 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> ---
>
> diff --git a/drivers/media/platform/coda/coda-common.c 
> b/drivers/media/platform/coda/coda-common.c
> index a918b294adef..eb6548f46cba 100644
> --- a/drivers/media/platform/coda/coda-common.c
> +++ b/drivers/media/platform/coda/coda-common.c
> @@ -2221,6 +2221,7 @@ static const struct coda_devtype coda_devdata[] = {
> [CODA_IMX27] = {
> .firmware = {
> "vpu_fw_imx27_TO2.bin",
> +   "vpu/vpu_fw_imx27_TO2.bin",
> "v4l-codadx6-imx27.bin"
> },
> .product  = CODA_DX6,
> @@ -2234,6 +2235,7 @@ static const struct coda_devtype coda_devdata[] = {
> [CODA_IMX53] = {
> .firmware = {
> "vpu_fw_imx53.bin",
> +   "vpu/vpu_fw_imx53.bin",
> "v4l-coda7541-imx53.bin"
> },
> .product  = CODA_7541,
> @@ -2248,6 +2250,7 @@ static const struct coda_devtype coda_devdata[] = {
> [CODA_IMX6Q] = {
> .firmware = {
> "vpu_fw_imx6q.bin",
> +   "vpu/vpu_fw_imx6q.bin",
> "v4l-coda960-imx6q.bin"
> },
> .product  = CODA_960,
> @@ -2262,6 +2265,7 @@ static const struct coda_devtype coda_devdata[] = {
> [CODA_IMX6DL] = {
> .firmware = {
> "vpu_fw_imx6d.bin",
> +   "vpu/vpu_fw_imx6d.bin",
> "v4l-coda960-imx6dl.bin"
> },
> .product  = CODA_960,
> diff --git a/drivers/media/platform/coda/coda.h 
> b/drivers/media/platform/coda/coda.h
> index 7ed79eb774e7..4b831c91ae4a 100644
> --- a/drivers/media/platform/coda/coda.h
> +++ b/drivers/media/platform/coda/coda.h
> @@ -50,7 +50,7 @@ enum coda_product {
>  struct coda_video_device;
>
>  struct coda_devtype {
> -   char*firmware[2];
> +   char*firmware[3];
> enum coda_product   product;
> const struct coda_codec *codecs;
> unsigned intnum_codecs;

I think there was an issue pointed out by Philipp on this patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] [media] coda: add Freescale firmware compatibility location

[Fabio Estevam]

On Sun, Jan 15, 2017 at 8:33 AM, Baruch Siach <bar...@tkos.co.il> wrote:
> The Freescale provided imx-vpu looks for firmware files under 
> /lib/firmware/vpu
> by default. Make coda look there for firmware files to ease the update path.
>
> Cc: Fabio Estevam <feste...@gmail.com>
> Signed-off-by: Baruch Siach <bar...@tkos.co.il>

Reviewed-by: Fabio Estevam <fabio.este...@nxp.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 05/19] ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors

[Fabio Estevam]

On Tue, Jan 3, 2017 at 6:57 PM, Steve Longerbeam  wrote:

> +   camera: ov5642@3c {
> +   compatible = "ovti,ov5642";
> +   pinctrl-names = "default";
> +   pinctrl-0 = <_ov5642>;
> +   clocks = < IMX6QDL_CLK_CKO>;
> +   clock-names = "xclk";
> +   reg = <0x3c>;
> +   xclk = <2400>;
> +   DOVDD-supply = <_reg>; /* 1.8v */
> +   AVDD-supply = <_reg>;  /* 2.8v, rev C board is VGEN3
> +   rev B board is VGEN5 */

Please use vgen3 so that by default we have the valid AVDD-supply for
revC boards which is more recent and more the users have access to.

> +   mipi_camera: ov5640@3c {
> +   compatible = "ovti,ov5640_mipi";
> +   pinctrl-names = "default";
> +   pinctrl-0 = <_ov5640>;
> +   reg = <0x3c>;
> +   clocks = < IMX6QDL_CLK_CKO>;
> +   clock-names = "xclk";
> +   xclk = <2400>;
> +   DOVDD-supply = <_reg>; /* 1.8v */
> +   AVDD-supply = <_reg>;  /* 2.8v, rev C board is VGEN3
> +   rev B board is VGEN5 */

Same here.

> +   pinctrl_ov5640: ov5640grp {
> +   fsl,pins = <
> +   MX6QDL_PAD_SD1_DAT2__GPIO1_IO19 0x8000
> +   MX6QDL_PAD_SD1_CLK__GPIO1_IO20  0x8000

Please avoid all the 0x8000 IOMUX settings and replace them by
their real values.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 00/19] i.MX Media Driver

[Fabio Estevam]

On Tue, Jan 3, 2017 at 6:57 PM, Steve Longerbeam <slongerb...@gmail.com> wrote:
> In version 2 (no functional changes):
>
> - removed patch "gpio: pca953x: Add optional reset gpio control", it
>   has been submitted separately.
> - fixed some whitespace errors.
> - added a few missing Signed-off-by's.

Tested the series on a mx6qsabresd, so:

Tested-by: Fabio Estevam <fabio.este...@nxp.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/20] i.MX Media Driver

[Fabio Estevam]

Hi Steve,

On Thu, Dec 29, 2016 at 8:27 PM, Steve Longerbeam <slongerb...@gmail.com> wrote:
> This is a media driver for video capture on i.MX.
>
> Refer to Documentation/media/v4l-drivers/imx.rst for example capture
> pipelines on SabreSD, SabreAuto, and SabreLite reference platforms.
>
> This patchset includes the OF graph layout as proposed by Philipp Zabel,
> with only minor changes which are enumerated in the patch header.

Patches 13, 14 and 19 miss your Signed-off-by tag.

Tested the whole series on a mx6qsabresd:

Tested-by: Fabio Estevam <fabio.este...@nxp.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 00/21] Basic i.MX IPUv3 capture support

[Fabio Estevam]

Hi Jean-Michel,

On Mon, Jan 2, 2017 at 12:59 PM, Jean-Michel Hautbois
 wrote:

> Steve: which branch is the correct one on your github ?

I have tested imx-media-staging-md-v2 (based on 4.9-rc) and also
imx-media-staging-md-v4 branch, which is based on 4.10-rc1.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html