1. Patchew
  2. linux
  3. [PATCH v2 00/16] media: platform: rzg2l-cru: CSI-2 and CRU enhancements
  4. View patch

[PATCH v2 14/16] media: platform: rzg2l-cru: rzg2l-csi2: Make use of rzg2l_csi2_formats array in rzg2l_csi2_enum_frame_size()

Prabhakar posted 16 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH v2 14/16] media: platform: rzg2l-cru: rzg2l-csi2: Make use of rzg2l_csi2_formats array in rzg2l_csi2_enum_frame_size()
Posted by Prabhakar 2 months, 2 weeks ago 
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Make use `rzg2l_csi2_formats` array in rzg2l_csi2_enum_frame_size().

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
index 79d99d865c1f..e630283dd1f1 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
@@ -570,7 +570,10 @@ static int rzg2l_csi2_enum_frame_size(struct v4l2_subdev *sd,
 				      struct v4l2_subdev_state *sd_state,
 				      struct v4l2_subdev_frame_size_enum *fse)
 {
-	if (fse->index != 0)
+	if (fse->index >= ARRAY_SIZE(rzg2l_csi2_formats))
+		return -EINVAL;
+
+	if (!rzg2l_csi2_code_to_fmt(fse->code))
 		return -EINVAL;
 
 	fse->min_width = RZG2L_CSI2_MIN_WIDTH;
-- 
2.34.1
Re: [PATCH v2 14/16] media: platform: rzg2l-cru: rzg2l-csi2: Make use of rzg2l_csi2_formats array in rzg2l_csi2_enum_frame_size()
Posted by Laurent Pinchart 2 months ago 
Hi Prabhakar,

Thank you for the patch.

I've just noticed that the subject line of most of your patches is much
longer than the 72 characters limit. Please try to shorten them. You can
replace the prefixes with "media: rzg2l-cru:", and reword the subject
lines that mention long function names.

On Tue, Sep 10, 2024 at 06:53:55PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Make use `rzg2l_csi2_formats` array in rzg2l_csi2_enum_frame_size().
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> index 79d99d865c1f..e630283dd1f1 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> @@ -570,7 +570,10 @@ static int rzg2l_csi2_enum_frame_size(struct v4l2_subdev *sd,
>  				      struct v4l2_subdev_state *sd_state,
>  				      struct v4l2_subdev_frame_size_enum *fse)
>  {
> -	if (fse->index != 0)
> +	if (fse->index >= ARRAY_SIZE(rzg2l_csi2_formats))
> +		return -EINVAL;

Same comment as in 11/16. With this fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> +
> +	if (!rzg2l_csi2_code_to_fmt(fse->code))
>  		return -EINVAL;
>  
>  	fse->min_width = RZG2L_CSI2_MIN_WIDTH;

-- 
Regards,

Laurent Pinchart
Re: [PATCH v2 14/16] media: platform: rzg2l-cru: rzg2l-csi2: Make use of rzg2l_csi2_formats array in rzg2l_csi2_enum_frame_size()
Posted by Lad, Prabhakar 2 months ago 
Hi Laurent,

Thank you for the review.

On Sat, Sep 28, 2024 at 12:11 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Prabhakar,
>
> Thank you for the patch.
>
> I've just noticed that the subject line of most of your patches is much
> longer than the 72 characters limit. Please try to shorten them. You can
> replace the prefixes with "media: rzg2l-cru:", and reword the subject
> lines that mention long function names.
>
Ok, I'll rework the subject line so that it fits within 72 characters.

> On Tue, Sep 10, 2024 at 06:53:55PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Make use `rzg2l_csi2_formats` array in rzg2l_csi2_enum_frame_size().
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > index 79d99d865c1f..e630283dd1f1 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > @@ -570,7 +570,10 @@ static int rzg2l_csi2_enum_frame_size(struct v4l2_subdev *sd,
> >                                     struct v4l2_subdev_state *sd_state,
> >                                     struct v4l2_subdev_frame_size_enum *fse)
> >  {
> > -     if (fse->index != 0)
> > +     if (fse->index >= ARRAY_SIZE(rzg2l_csi2_formats))
> > +             return -EINVAL;
>
> Same comment as in 11/16. With this fixed,
>
Ok, I'll drop this check.

> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>

Cheers,
Prabhakar
Re: [PATCH v2 14/16] media: platform: rzg2l-cru: rzg2l-csi2: Make use of rzg2l_csi2_formats array in rzg2l_csi2_enum_frame_size()
Posted by Laurent Pinchart 2 months ago 
On Mon, Sep 30, 2024 at 01:19:25PM +0100, Lad, Prabhakar wrote:
> Hi Laurent,
> 
> Thank you for the review.
> 
> On Sat, Sep 28, 2024 at 12:11 AM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> >
> > Hi Prabhakar,
> >
> > Thank you for the patch.
> >
> > I've just noticed that the subject line of most of your patches is much
> > longer than the 72 characters limit. Please try to shorten them. You can
> > replace the prefixes with "media: rzg2l-cru:", and reword the subject
> > lines that mention long function names.
> >
> Ok, I'll rework the subject line so that it fits within 72 characters.
> 
> > On Tue, Sep 10, 2024 at 06:53:55PM +0100, Prabhakar wrote:
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > Make use `rzg2l_csi2_formats` array in rzg2l_csi2_enum_frame_size().
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > > index 79d99d865c1f..e630283dd1f1 100644
> > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > > @@ -570,7 +570,10 @@ static int rzg2l_csi2_enum_frame_size(struct v4l2_subdev *sd,
> > >                                     struct v4l2_subdev_state *sd_state,
> > >                                     struct v4l2_subdev_frame_size_enum *fse)
> > >  {
> > > -     if (fse->index != 0)
> > > +     if (fse->index >= ARRAY_SIZE(rzg2l_csi2_formats))
> > > +             return -EINVAL;
> >
> > Same comment as in 11/16. With this fixed,
> >
> Ok, I'll drop this check.

Don't drop the check, drop the change. if (fse->index != 0) is the
right check (testing > 0 works too).

> > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

-- 
Regards,

Laurent Pinchart
Re: [PATCH v2 14/16] media: platform: rzg2l-cru: rzg2l-csi2: Make use of rzg2l_csi2_formats array in rzg2l_csi2_enum_frame_size()
Posted by Lad, Prabhakar 2 months ago 
Hi Laurent,

On Mon, Sep 30, 2024 at 1:52 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Mon, Sep 30, 2024 at 01:19:25PM +0100, Lad, Prabhakar wrote:
> > Hi Laurent,
> >
> > Thank you for the review.
> >
> > On Sat, Sep 28, 2024 at 12:11 AM Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com> wrote:
> > >
> > > Hi Prabhakar,
> > >
> > > Thank you for the patch.
> > >
> > > I've just noticed that the subject line of most of your patches is much
> > > longer than the 72 characters limit. Please try to shorten them. You can
> > > replace the prefixes with "media: rzg2l-cru:", and reword the subject
> > > lines that mention long function names.
> > >
> > Ok, I'll rework the subject line so that it fits within 72 characters.
> >
> > > On Tue, Sep 10, 2024 at 06:53:55PM +0100, Prabhakar wrote:
> > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > >
> > > > Make use `rzg2l_csi2_formats` array in rzg2l_csi2_enum_frame_size().
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > ---
> > > >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c | 5 ++++-
> > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > > > index 79d99d865c1f..e630283dd1f1 100644
> > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > > > @@ -570,7 +570,10 @@ static int rzg2l_csi2_enum_frame_size(struct v4l2_subdev *sd,
> > > >                                     struct v4l2_subdev_state *sd_state,
> > > >                                     struct v4l2_subdev_frame_size_enum *fse)
> > > >  {
> > > > -     if (fse->index != 0)
> > > > +     if (fse->index >= ARRAY_SIZE(rzg2l_csi2_formats))
> > > > +             return -EINVAL;
> > >
> > > Same comment as in 11/16. With this fixed,
> > >
> > Ok, I'll drop this check.
>
> Don't drop the check, drop the change. if (fse->index != 0) is the
> right check (testing > 0 works too).
>
Ahh sorry for not being clear, I meant I will drop the updated check.

Cheers,
Prabhakar

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.