Hi Niklas,
On Wed, May 16, 2018 at 11:58:47PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2018-05-16 18:32:29 +0200, Jacopo Mondi wrote:
> > The data-active property has to be specified when running with embedded
> > synchronization. The VIN peripheral can use HSYNC in place of CLOCKENB
> > when the CLOCKENB pin is not connected, this requires explicit
> > synchronization to be in use.
>
> Is this really the intent of the data-active property? I read the
> video-interfaces.txt document as such as if no hsync-active,
> vsync-active and data-active we should use the embedded synchronization
> else set the polarity for the requested pins. What am I not
> understanding here?
Almost correct.
The presence of hsync-active, vsync-active and field-evev-active
properties determinate the bus type we're running on. If none of the
is specified, the bus is marked 'BT656' and we assume the system is
using embedded synchronization.
data-active does not take part in the bus identification, and my
reasoning was the other way around as explained in reply to your
comment on [2/6], and as explained there my reasoning is probably
wrong, and we should set CHS -only- when running with explicit
synchronization, instead of making it mandatory when running with
embedded syncs.
Thanks and sorry for my confusion.
j
>
> >
> > Now that the driver supports 'data-active' property, it makes not sense
> > to zero the mbus configuration flags when running with implicit synch
> > (V4L2_MBUS_BT656).
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
> > ---
> > drivers/media/platform/rcar-vin/rcar-core.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> > index d3072e1..075d08f 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > @@ -531,15 +531,21 @@ static int rvin_digital_parse_v4l2(struct device *dev,
> > return -ENOTCONN;
> >
> > vin->mbus_cfg.type = vep->bus_type;
> > + vin->mbus_cfg.flags = vep->bus.parallel.flags;
> >
> > switch (vin->mbus_cfg.type) {
> > case V4L2_MBUS_PARALLEL:
> > vin_dbg(vin, "Found PARALLEL media bus\n");
> > - vin->mbus_cfg.flags = vep->bus.parallel.flags;
> > break;
> > case V4L2_MBUS_BT656:
> > vin_dbg(vin, "Found BT656 media bus\n");
> > - vin->mbus_cfg.flags = 0;
> > +
> > + if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_HIGH) &&
> > + !(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW)) {
> > + vin_err(vin,
> > + "Missing data enable signal polarity property\n");
>
> I fear this can't be an error as that would break backward comp ability
> with existing dtb's.
>
> > + return -EINVAL;
> > + }
> > break;
> > default:
> > vin_err(vin, "Unknown media bus type\n");
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas Söderlund
Attachment:
signature.asc
Description: PGP signature
