| Message ID | 1536847191-17175-4-git-send-email-jacopo+renesas@jmondi.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | media: renesas-ceu: Introduce default mbus configuration | expand |
Hi Jacopo, On Thu, Sep 13, 2018 at 03:59:51PM +0200, Jacopo Mondi wrote: > As the v4l2-fwnode now allows drivers to set defaults, and eventually > override them by specifying properties in DTS, use defaults for the CEU > driver. > > Also remove endpoint properties from the gr-peach-audiocamerashield as > they match the defaults now specified in the driver code. > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > --- > arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi | 4 ---- > drivers/media/platform/renesas-ceu.c | 20 +++++++++++--------- > 2 files changed, 11 insertions(+), 13 deletions(-) > > diff --git a/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi b/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi > index e31a9e3..8d77579 100644 > --- a/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi > +++ b/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi > @@ -69,10 +69,6 @@ > > port { > ceu_in: endpoint { > - hsync-active = <1>; > - vsync-active = <1>; > - bus-width = <8>; > - pclk-sample = <1>; Do I understand correctly that pclk-sample was never relevant for the hardware, and is removed because of that? You could mention that in the commit message. That's perhaps a minor detail. The set seems good to me. > remote-endpoint = <&mt9v111_out>; > }; > }; > diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c > index 035f1d3..150196f 100644 > --- a/drivers/media/platform/renesas-ceu.c > +++ b/drivers/media/platform/renesas-ceu.c > @@ -1551,7 +1551,16 @@ static int ceu_parse_dt(struct ceu_device *ceudev) > return ret; > > for (i = 0; i < num_ep; i++) { > - struct v4l2_fwnode_endpoint fw_ep = { .bus_type = 0 }; > + struct v4l2_fwnode_endpoint fw_ep = { > + .bus_type = V4L2_MBUS_PARALLEL, > + .bus = { > + .parallel = { > + .flags = V4L2_MBUS_HSYNC_ACTIVE_HIGH | > + V4L2_MBUS_VSYNC_ACTIVE_HIGH, > + .bus_width = 8, > + }, > + }, > + }; > > ep = of_graph_get_endpoint_by_regs(of, 0, i); > if (!ep) { > @@ -1564,14 +1573,7 @@ static int ceu_parse_dt(struct ceu_device *ceudev) > ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &fw_ep); > if (ret) { > dev_err(ceudev->dev, > - "Unable to parse endpoint #%u.\n", i); > - goto error_cleanup; > - } > - > - if (fw_ep.bus_type != V4L2_MBUS_PARALLEL) { > - dev_err(ceudev->dev, > - "Only parallel input supported.\n"); > - ret = -EINVAL; > + "Unable to parse endpoint #%u: %d.\n", i, ret); > goto error_cleanup; > } >
Hi Sakari, On Thu, Sep 13, 2018 at 05:22:36PM +0300, Sakari Ailus wrote: > Hi Jacopo, > > On Thu, Sep 13, 2018 at 03:59:51PM +0200, Jacopo Mondi wrote: > > As the v4l2-fwnode now allows drivers to set defaults, and eventually > > override them by specifying properties in DTS, use defaults for the CEU > > driver. > > > > Also remove endpoint properties from the gr-peach-audiocamerashield as > > they match the defaults now specified in the driver code. > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > --- > > arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi | 4 ---- > > drivers/media/platform/renesas-ceu.c | 20 +++++++++++--------- > > 2 files changed, 11 insertions(+), 13 deletions(-) > > > > diff --git a/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi b/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi > > index e31a9e3..8d77579 100644 > > --- a/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi > > +++ b/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi > > @@ -69,10 +69,6 @@ > > > > port { > > ceu_in: endpoint { > > - hsync-active = <1>; > > - vsync-active = <1>; > > - bus-width = <8>; > > - pclk-sample = <1>; > > Do I understand correctly that pclk-sample was never relevant for the > hardware, and is removed because of that? You could mention that in the > commit message. That's perhaps a minor detail. Correct, it's not relevant for the CEU, as it cannot be configured. > > The set seems good to me. I can add that to commit message and re-send, or if it's easier for you, you can change the last paragraph when applying to: Also remove endpoint properties from the gr-peach-audiocamerashield as they match the defaults now specified in the driver code (h/vsync-active and bus-width) or are not relevant to the interface as they cannot be configured (pclk-sample). Thanks j > > > remote-endpoint = <&mt9v111_out>; > > }; > > }; > > diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c > > index 035f1d3..150196f 100644 > > --- a/drivers/media/platform/renesas-ceu.c > > +++ b/drivers/media/platform/renesas-ceu.c > > @@ -1551,7 +1551,16 @@ static int ceu_parse_dt(struct ceu_device *ceudev) > > return ret; > > > > for (i = 0; i < num_ep; i++) { > > - struct v4l2_fwnode_endpoint fw_ep = { .bus_type = 0 }; > > + struct v4l2_fwnode_endpoint fw_ep = { > > + .bus_type = V4L2_MBUS_PARALLEL, > > + .bus = { > > + .parallel = { > > + .flags = V4L2_MBUS_HSYNC_ACTIVE_HIGH | > > + V4L2_MBUS_VSYNC_ACTIVE_HIGH, > > + .bus_width = 8, > > + }, > > + }, > > + }; > > > > ep = of_graph_get_endpoint_by_regs(of, 0, i); > > if (!ep) { > > @@ -1564,14 +1573,7 @@ static int ceu_parse_dt(struct ceu_device *ceudev) > > ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &fw_ep); > > if (ret) { > > dev_err(ceudev->dev, > > - "Unable to parse endpoint #%u.\n", i); > > - goto error_cleanup; > > - } > > - > > - if (fw_ep.bus_type != V4L2_MBUS_PARALLEL) { > > - dev_err(ceudev->dev, > > - "Only parallel input supported.\n"); > > - ret = -EINVAL; > > + "Unable to parse endpoint #%u: %d.\n", i, ret); > > goto error_cleanup; > > } > > > > -- > Kind regards, > > Sakari Ailus > sakari.ailus@linux.intel.com
diff --git a/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi b/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi index e31a9e3..8d77579 100644 --- a/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi +++ b/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi @@ -69,10 +69,6 @@ port { ceu_in: endpoint { - hsync-active = <1>; - vsync-active = <1>; - bus-width = <8>; - pclk-sample = <1>; remote-endpoint = <&mt9v111_out>; }; }; diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c index 035f1d3..150196f 100644 --- a/drivers/media/platform/renesas-ceu.c +++ b/drivers/media/platform/renesas-ceu.c @@ -1551,7 +1551,16 @@ static int ceu_parse_dt(struct ceu_device *ceudev) return ret; for (i = 0; i < num_ep; i++) { - struct v4l2_fwnode_endpoint fw_ep = { .bus_type = 0 }; + struct v4l2_fwnode_endpoint fw_ep = { + .bus_type = V4L2_MBUS_PARALLEL, + .bus = { + .parallel = { + .flags = V4L2_MBUS_HSYNC_ACTIVE_HIGH | + V4L2_MBUS_VSYNC_ACTIVE_HIGH, + .bus_width = 8, + }, + }, + }; ep = of_graph_get_endpoint_by_regs(of, 0, i); if (!ep) { @@ -1564,14 +1573,7 @@ static int ceu_parse_dt(struct ceu_device *ceudev) ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &fw_ep); if (ret) { dev_err(ceudev->dev, - "Unable to parse endpoint #%u.\n", i); - goto error_cleanup; - } - - if (fw_ep.bus_type != V4L2_MBUS_PARALLEL) { - dev_err(ceudev->dev, - "Only parallel input supported.\n"); - ret = -EINVAL; + "Unable to parse endpoint #%u: %d.\n", i, ret); goto error_cleanup; }
As the v4l2-fwnode now allows drivers to set defaults, and eventually override them by specifying properties in DTS, use defaults for the CEU driver. Also remove endpoint properties from the gr-peach-audiocamerashield as they match the defaults now specified in the driver code. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> --- arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi | 4 ---- drivers/media/platform/renesas-ceu.c | 20 +++++++++++--------- 2 files changed, 11 insertions(+), 13 deletions(-) -- 2.7.4