[v3,1/4] media: pisp_be: Drop reference to non-existing function

Message ID 20240827074018.534354-2-jacopo.mondi@ideasonboard.com (mailing list archive)
State New
Headers
Series [v3,1/4] media: pisp_be: Drop reference to non-existing function |

Commit Message

Jacopo Mondi Aug. 27, 2024, 7:40 a.m. UTC
  A comment in the pisp_be driver references to the
 pispbe_schedule_internal() which doesn't exist.

Drop it.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 drivers/media/platform/raspberrypi/pisp_be/pisp_be.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
  

Comments

Laurent Pinchart Aug. 31, 2024, 1:03 a.m. UTC | #1
Hi Jacopo,

Thank you for the patch.

On Tue, Aug 27, 2024 at 09:40:15AM +0200, Jacopo Mondi wrote:
> A comment in the pisp_be driver references to the

s/to the/the/

>  pispbe_schedule_internal() which doesn't exist.

s/which/function which/

> 
> Drop it.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  drivers/media/platform/raspberrypi/pisp_be/pisp_be.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
> index 65ff2382cffe..8ba1b9f43ba1 100644
> --- a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
> +++ b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
> @@ -368,10 +368,7 @@ static void pispbe_xlate_addrs(struct pispbe_dev *pispbe,
>  	ret = pispbe_get_planes_addr(addrs, buf[MAIN_INPUT_NODE],
>  				     &pispbe->node[MAIN_INPUT_NODE]);
>  	if (ret <= 0) {
> -		/*
> -		 * This shouldn't happen; pispbe_schedule_internal should insist
> -		 * on an input.
> -		 */
> +		/* Shouldn't happen, we have validated an input is available. */

Actually, where is that validated ?

>  		dev_warn(pispbe->dev, "ISP-BE missing input\n");
>  		hw_en->bayer_enables = 0;
>  		hw_en->rgb_enables = 0;
  
Jacopo Mondi Aug. 31, 2024, 2:44 p.m. UTC | #2
Hi Laurent

On Sat, Aug 31, 2024 at 04:03:30AM GMT, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Tue, Aug 27, 2024 at 09:40:15AM +0200, Jacopo Mondi wrote:
> > A comment in the pisp_be driver references to the
>
> s/to the/the/
>
> >  pispbe_schedule_internal() which doesn't exist.
>
> s/which/function which/
>
> >
> > Drop it.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> >  drivers/media/platform/raspberrypi/pisp_be/pisp_be.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
> > index 65ff2382cffe..8ba1b9f43ba1 100644
> > --- a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
> > +++ b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
> > @@ -368,10 +368,7 @@ static void pispbe_xlate_addrs(struct pispbe_dev *pispbe,
> >  	ret = pispbe_get_planes_addr(addrs, buf[MAIN_INPUT_NODE],
> >  				     &pispbe->node[MAIN_INPUT_NODE]);
> >  	if (ret <= 0) {
> > -		/*
> > -		 * This shouldn't happen; pispbe_schedule_internal should insist
> > -		 * on an input.
> > -		 */
> > +		/* Shouldn't happen, we have validated an input is available. */
>
> Actually, where is that validated ?
>

When preparing a job, the MAIN_INPUT_NODE buffer is mandatory if I
read the usage of the 'ignore_buffers' flag right ?




> >  		dev_warn(pispbe->dev, "ISP-BE missing input\n");
> >  		hw_en->bayer_enables = 0;
> >  		hw_en->rgb_enables = 0;
>
> --
> Regards,
>
> Laurent Pinchart
  
Laurent Pinchart Aug. 31, 2024, 3 p.m. UTC | #3
On Sat, Aug 31, 2024 at 04:44:06PM +0200, Jacopo Mondi wrote:
> On Sat, Aug 31, 2024 at 04:03:30AM GMT, Laurent Pinchart wrote:
> > On Tue, Aug 27, 2024 at 09:40:15AM +0200, Jacopo Mondi wrote:
> > > A comment in the pisp_be driver references to the
> >
> > s/to the/the/
> >
> > >  pispbe_schedule_internal() which doesn't exist.
> >
> > s/which/function which/
> >
> > >
> > > Drop it.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > > ---
> > >  drivers/media/platform/raspberrypi/pisp_be/pisp_be.c | 5 +----
> > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
> > > index 65ff2382cffe..8ba1b9f43ba1 100644
> > > --- a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
> > > +++ b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
> > > @@ -368,10 +368,7 @@ static void pispbe_xlate_addrs(struct pispbe_dev *pispbe,
> > >  	ret = pispbe_get_planes_addr(addrs, buf[MAIN_INPUT_NODE],
> > >  				     &pispbe->node[MAIN_INPUT_NODE]);
> > >  	if (ret <= 0) {
> > > -		/*
> > > -		 * This shouldn't happen; pispbe_schedule_internal should insist
> > > -		 * on an input.
> > > -		 */
> > > +		/* Shouldn't happen, we have validated an input is available. */
> >
> > Actually, where is that validated ?
> 
> When preparing a job, the MAIN_INPUT_NODE buffer is mandatory if I
> read the usage of the 'ignore_buffers' flag right ?

I think you're right. The logic is convoluted, it relies on the fact
that streaming_map is first checked for MAIN_INPUT_NODE. Hopefully it
will be clearer after all the cleanups.

On a side note, is the pisp_format check in pispbe_get_planes_addr()
needed ?

> > >  		dev_warn(pispbe->dev, "ISP-BE missing input\n");
> > >  		hw_en->bayer_enables = 0;
> > >  		hw_en->rgb_enables = 0;
  

Patch

diff --git a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
index 65ff2382cffe..8ba1b9f43ba1 100644
--- a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
+++ b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
@@ -368,10 +368,7 @@  static void pispbe_xlate_addrs(struct pispbe_dev *pispbe,
 	ret = pispbe_get_planes_addr(addrs, buf[MAIN_INPUT_NODE],
 				     &pispbe->node[MAIN_INPUT_NODE]);
 	if (ret <= 0) {
-		/*
-		 * This shouldn't happen; pispbe_schedule_internal should insist
-		 * on an input.
-		 */
+		/* Shouldn't happen, we have validated an input is available. */
 		dev_warn(pispbe->dev, "ISP-BE missing input\n");
 		hw_en->bayer_enables = 0;
 		hw_en->rgb_enables = 0;