| Message ID | 20240910175357.229075-14-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | media: platform: rzg2l-cru: CSI-2 and CRU enhancements | expand |
Hi Prabhakar, Thank you for the patch. On Tue, Sep 10, 2024 at 06:53:54PM +0100, Prabhakar wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Utilize `rzg2l_cru_ip_code_to_fmt()` in `rzg2l_cru_mc_validate_format()` > to validate whether the format is supported. This change removes the need > to manually add new entries when a new format is added to the CRU driver, > simplifying the validation process. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > index 014c0ff2721b..c32608c557a3 100644 > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > @@ -201,12 +201,8 @@ static int rzg2l_cru_mc_validate_format(struct rzg2l_cru_dev *cru, > if (v4l2_subdev_call_state_active(sd, pad, get_fmt, &fmt)) > return -EPIPE; > > - switch (fmt.format.code) { > - case MEDIA_BUS_FMT_UYVY8_1X16: > - break; > - default: > + if (!rzg2l_cru_ip_code_to_fmt(fmt.format.code)) > return -EPIPE; > - } This looks fine, but I think you should take it one step further and perform format validation in .link_validate(). See https://lore.kernel.org/all/20240826124106.3823-8-laurent.pinchart+renesas@ideasonboard.com/ > > switch (fmt.format.field) { > case V4L2_FIELD_TOP:
Hi Laurent, Thank you for the review. On Sat, Sep 28, 2024 at 12:09 AM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Prabhakar, > > Thank you for the patch. > > On Tue, Sep 10, 2024 at 06:53:54PM +0100, Prabhakar wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Utilize `rzg2l_cru_ip_code_to_fmt()` in `rzg2l_cru_mc_validate_format()` > > to validate whether the format is supported. This change removes the need > > to manually add new entries when a new format is added to the CRU driver, > > simplifying the validation process. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 6 +----- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > > index 014c0ff2721b..c32608c557a3 100644 > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > > @@ -201,12 +201,8 @@ static int rzg2l_cru_mc_validate_format(struct rzg2l_cru_dev *cru, > > if (v4l2_subdev_call_state_active(sd, pad, get_fmt, &fmt)) > > return -EPIPE; > > > > - switch (fmt.format.code) { > > - case MEDIA_BUS_FMT_UYVY8_1X16: > > - break; > > - default: > > + if (!rzg2l_cru_ip_code_to_fmt(fmt.format.code)) > > return -EPIPE; > > - } > > This looks fine, but I think you should take it one step further and > perform format validation in .link_validate(). See > https://lore.kernel.org/all/20240826124106.3823-8-laurent.pinchart+renesas@ideasonboard.com/ > OK, I'll implement link_validate() and do format checking in there and get rid of rzg2l_cru_mc_validate_format(). Thanks for the pointer. Cheers, Prabhakar
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c index 014c0ff2721b..c32608c557a3 100644 --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c @@ -201,12 +201,8 @@ static int rzg2l_cru_mc_validate_format(struct rzg2l_cru_dev *cru, if (v4l2_subdev_call_state_active(sd, pad, get_fmt, &fmt)) return -EPIPE; - switch (fmt.format.code) { - case MEDIA_BUS_FMT_UYVY8_1X16: - break; - default: + if (!rzg2l_cru_ip_code_to_fmt(fmt.format.code)) return -EPIPE; - } switch (fmt.format.field) { case V4L2_FIELD_TOP: