Re: [PATCH v11 17/32] rcar-vin: move media bus configuration to struct rvin_info

2018-03-03 Thread Niklas Söderlund
Hi Laurent,

Thanks for your feedback.

On 2018-03-02 13:26:58 +0200, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Friday, 2 March 2018 03:57:36 EET Niklas Söderlund wrote:
> > Bus configuration will once the driver is extended to support Gen3
> > contain information not specific to only the directly connected parallel
> > subdevice. Move it to struct rvin_dev to show it's not always coupled
> > to the parallel subdevice.
> 
> The subject line still mentions rvin_info. Are you so emotionally attached to 
> it that you have trouble fixing that ? ;-)

I had fixed this, but when I reviewed the patches before I sent them out 
I miss read the diff and it looked like I had forgotten to fix this and 
I broke it again. So in short it seems my subconscious are still 
emotionally attached to rvin_info :-)

> 
> > Signed-off-by: Niklas Söderlund 
> > Reviewed-by: Hans Verkuil 
> > ---
> >  drivers/media/platform/rcar-vin/rcar-core.c | 18 +-
> >  drivers/media/platform/rcar-vin/rcar-dma.c  | 11 ++-
> >  drivers/media/platform/rcar-vin/rcar-v4l2.c |  2 +-
> >  drivers/media/platform/rcar-vin/rcar-vin.h  |  9 -
> >  4 files changed, 20 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> > b/drivers/media/platform/rcar-vin/rcar-core.c index
> > cc863e4ec9a4d4b3..449175c3133e42c6 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > @@ -65,10 +65,10 @@ static int rvin_digital_subdevice_attach(struct rvin_dev
> > *vin, vin->digital->sink_pad = ret < 0 ? 0 : ret;
> > 
> > /* Find compatible subdevices mbus format */
> > -   vin->digital->code = 0;
> > +   vin->mbus_code = 0;
> > code.index = 0;
> > code.pad = vin->digital->source_pad;
> > -   while (!vin->digital->code &&
> > +   while (!vin->mbus_code &&
> >!v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL, )) {
> > code.index++;
> > switch (code.code) {
> > @@ -76,16 +76,16 @@ static int rvin_digital_subdevice_attach(struct rvin_dev
> > *vin, case MEDIA_BUS_FMT_UYVY8_2X8:
> > case MEDIA_BUS_FMT_UYVY10_2X10:
> > case MEDIA_BUS_FMT_RGB888_1X24:
> > -   vin->digital->code = code.code;
> > +   vin->mbus_code = code.code;
> > vin_dbg(vin, "Found media bus format for %s: %d\n",
> > -   subdev->name, vin->digital->code);
> > +   subdev->name, vin->mbus_code);
> > break;
> > default:
> > break;
> > }
> > }
> > 
> > -   if (!vin->digital->code) {
> > +   if (!vin->mbus_code) {
> > vin_err(vin, "Unsupported media bus format for %s\n",
> > subdev->name);
> > return -EINVAL;
> > @@ -190,16 +190,16 @@ static int rvin_digital_parse_v4l2(struct device *dev,
> > if (vep->base.port || vep->base.id)
> > return -ENOTCONN;
> > 
> > -   rvge->mbus_cfg.type = vep->bus_type;
> > +   vin->mbus_cfg.type = vep->bus_type;
> > 
> > -   switch (rvge->mbus_cfg.type) {
> > +   switch (vin->mbus_cfg.type) {
> > case V4L2_MBUS_PARALLEL:
> > vin_dbg(vin, "Found PARALLEL media bus\n");
> > -   rvge->mbus_cfg.flags = vep->bus.parallel.flags;
> > +   vin->mbus_cfg.flags = vep->bus.parallel.flags;
> > break;
> > case V4L2_MBUS_BT656:
> > vin_dbg(vin, "Found BT656 media bus\n");
> > -   rvge->mbus_cfg.flags = 0;
> > +   vin->mbus_cfg.flags = 0;
> > break;
> > default:
> > vin_err(vin, "Unknown media bus type\n");
> > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> > b/drivers/media/platform/rcar-vin/rcar-dma.c index
> > c8831e189d362c8b..4ebf76c30a3e9117 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > @@ -633,7 +633,7 @@ static int rvin_setup(struct rvin_dev *vin)
> > /*
> >  * Input interface
> >  */
> > -   switch (vin->digital->code) {
> > +   switch (vin->mbus_code) {
> > case MEDIA_BUS_FMT_YUYV8_1X16:
> > /* BT.601/BT.1358 16bit YCbCr422 */
> > vnmc |= VNMC_INF_YUV16;
> > @@ -641,7 +641,7 @@ static int rvin_setup(struct rvin_dev *vin)
> > break;
> > case MEDIA_BUS_FMT_UYVY8_2X8:
> > /* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */
> > -   vnmc |= vin->digital->mbus_cfg.type == V4L2_MBUS_BT656 ?
> > +   vnmc |= vin->mbus_cfg.type == V4L2_MBUS_BT656 ?
> > VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601;
> > input_is_yuv = true;
> > break;
> > @@ -650,7 +650,7 @@ static int rvin_setup(struct rvin_dev *vin)
> > break;
> > case MEDIA_BUS_FMT_UYVY10_2X10:
> >   

Re: [PATCH v11 17/32] rcar-vin: move media bus configuration to struct rvin_info

2018-03-02 Thread Laurent Pinchart
Hi Niklas,

Thank you for the patch.

On Friday, 2 March 2018 03:57:36 EET Niklas Söderlund wrote:
> Bus configuration will once the driver is extended to support Gen3
> contain information not specific to only the directly connected parallel
> subdevice. Move it to struct rvin_dev to show it's not always coupled
> to the parallel subdevice.

The subject line still mentions rvin_info. Are you so emotionally attached to 
it that you have trouble fixing that ? ;-)

> Signed-off-by: Niklas Söderlund 
> Reviewed-by: Hans Verkuil 
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 18 +-
>  drivers/media/platform/rcar-vin/rcar-dma.c  | 11 ++-
>  drivers/media/platform/rcar-vin/rcar-v4l2.c |  2 +-
>  drivers/media/platform/rcar-vin/rcar-vin.h  |  9 -
>  4 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> b/drivers/media/platform/rcar-vin/rcar-core.c index
> cc863e4ec9a4d4b3..449175c3133e42c6 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -65,10 +65,10 @@ static int rvin_digital_subdevice_attach(struct rvin_dev
> *vin, vin->digital->sink_pad = ret < 0 ? 0 : ret;
> 
>   /* Find compatible subdevices mbus format */
> - vin->digital->code = 0;
> + vin->mbus_code = 0;
>   code.index = 0;
>   code.pad = vin->digital->source_pad;
> - while (!vin->digital->code &&
> + while (!vin->mbus_code &&
>  !v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL, )) {
>   code.index++;
>   switch (code.code) {
> @@ -76,16 +76,16 @@ static int rvin_digital_subdevice_attach(struct rvin_dev
> *vin, case MEDIA_BUS_FMT_UYVY8_2X8:
>   case MEDIA_BUS_FMT_UYVY10_2X10:
>   case MEDIA_BUS_FMT_RGB888_1X24:
> - vin->digital->code = code.code;
> + vin->mbus_code = code.code;
>   vin_dbg(vin, "Found media bus format for %s: %d\n",
> - subdev->name, vin->digital->code);
> + subdev->name, vin->mbus_code);
>   break;
>   default:
>   break;
>   }
>   }
> 
> - if (!vin->digital->code) {
> + if (!vin->mbus_code) {
>   vin_err(vin, "Unsupported media bus format for %s\n",
>   subdev->name);
>   return -EINVAL;
> @@ -190,16 +190,16 @@ static int rvin_digital_parse_v4l2(struct device *dev,
> if (vep->base.port || vep->base.id)
>   return -ENOTCONN;
> 
> - rvge->mbus_cfg.type = vep->bus_type;
> + vin->mbus_cfg.type = vep->bus_type;
> 
> - switch (rvge->mbus_cfg.type) {
> + switch (vin->mbus_cfg.type) {
>   case V4L2_MBUS_PARALLEL:
>   vin_dbg(vin, "Found PARALLEL media bus\n");
> - rvge->mbus_cfg.flags = vep->bus.parallel.flags;
> + vin->mbus_cfg.flags = vep->bus.parallel.flags;
>   break;
>   case V4L2_MBUS_BT656:
>   vin_dbg(vin, "Found BT656 media bus\n");
> - rvge->mbus_cfg.flags = 0;
> + vin->mbus_cfg.flags = 0;
>   break;
>   default:
>   vin_err(vin, "Unknown media bus type\n");
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> b/drivers/media/platform/rcar-vin/rcar-dma.c index
> c8831e189d362c8b..4ebf76c30a3e9117 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -633,7 +633,7 @@ static int rvin_setup(struct rvin_dev *vin)
>   /*
>* Input interface
>*/
> - switch (vin->digital->code) {
> + switch (vin->mbus_code) {
>   case MEDIA_BUS_FMT_YUYV8_1X16:
>   /* BT.601/BT.1358 16bit YCbCr422 */
>   vnmc |= VNMC_INF_YUV16;
> @@ -641,7 +641,7 @@ static int rvin_setup(struct rvin_dev *vin)
>   break;
>   case MEDIA_BUS_FMT_UYVY8_2X8:
>   /* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */
> - vnmc |= vin->digital->mbus_cfg.type == V4L2_MBUS_BT656 ?
> + vnmc |= vin->mbus_cfg.type == V4L2_MBUS_BT656 ?
>   VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601;
>   input_is_yuv = true;
>   break;
> @@ -650,7 +650,7 @@ static int rvin_setup(struct rvin_dev *vin)
>   break;
>   case MEDIA_BUS_FMT_UYVY10_2X10:
>   /* BT.656 10bit YCbCr422 or BT.601 10bit YCbCr422 */
> - vnmc |= vin->digital->mbus_cfg.type == V4L2_MBUS_BT656 ?
> + vnmc |= vin->mbus_cfg.type == V4L2_MBUS_BT656 ?
>   VNMC_INF_YUV10_BT656 : VNMC_INF_YUV10_BT601;
>   input_is_yuv = true;
>   break;
> @@ -662,11 +662,11 @@ static int rvin_setup(struct rvin_dev *vin)
>   dmr2 = VNDMR2_FTEV | VNDMR2_VLV(1);
>