2015-06-20 13:29:29

by Guennadi Liakhovetski

[permalink] [raw]
Subject: Re: [PATCH 3/4] media: pxa_camera: trivial move of dma irq functions

On Sun, 22 Mar 2015, Robert Jarzmik wrote:

> From: Robert Jarzmik <[email protected]>
>
> This moves the dma irq handling functions up in the source file, so that
> they are available before DMA preparation functions. It prepares the
> conversion to DMA engine, where the descriptors are populated with these
> functions as callbacks.
>
> Signed-off-by: Robert Jarzmik <[email protected]>
> ---
> drivers/media/platform/soc_camera/pxa_camera.c | 40 ++++++++++++++------------
> 1 file changed, 22 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/media/platform/soc_camera/pxa_camera.c b/drivers/media/platform/soc_camera/pxa_camera.c
> index c0c0f0f..8b39f44 100644
> --- a/drivers/media/platform/soc_camera/pxa_camera.c
> +++ b/drivers/media/platform/soc_camera/pxa_camera.c
> @@ -311,6 +311,28 @@ static int calculate_dma_sglen(struct scatterlist *sglist, int sglen,
>
> BUG_ON(size != 0);
> return i + 1;
> +static void pxa_camera_dma_irq(struct pxa_camera_dev *pcdev,
> + enum pxa_camera_active_dma act_dma);
> +
> +static void pxa_camera_dma_irq_y(void *data)

Wait, how is this patch trivial? You change pxa_camera_dma_irq_?()
prototypes, which are used as PXA DMA callbacks. Does this mean, that
either before or after this patch compilation is broken?

Thanks
Guennadi

> +{
> + struct pxa_camera_dev *pcdev = data;
> +
> + pxa_camera_dma_irq(pcdev, DMA_Y);
> +}
> +
> +static void pxa_camera_dma_irq_u(void *data)
> +{
> + struct pxa_camera_dev *pcdev = data;
> +
> + pxa_camera_dma_irq(pcdev, DMA_U);
> +}
> +
> +static void pxa_camera_dma_irq_v(void *data)
> +{
> + struct pxa_camera_dev *pcdev = data;
> +
> + pxa_camera_dma_irq(pcdev, DMA_V);
> }
>
> /**
> @@ -810,24 +832,6 @@ out:
> spin_unlock_irqrestore(&pcdev->lock, flags);
> }
>
> -static void pxa_camera_dma_irq_y(int channel, void *data)
> -{
> - struct pxa_camera_dev *pcdev = data;
> - pxa_camera_dma_irq(channel, pcdev, DMA_Y);
> -}
> -
> -static void pxa_camera_dma_irq_u(int channel, void *data)
> -{
> - struct pxa_camera_dev *pcdev = data;
> - pxa_camera_dma_irq(channel, pcdev, DMA_U);
> -}
> -
> -static void pxa_camera_dma_irq_v(int channel, void *data)
> -{
> - struct pxa_camera_dev *pcdev = data;
> - pxa_camera_dma_irq(channel, pcdev, DMA_V);
> -}
> -
> static struct videobuf_queue_ops pxa_videobuf_ops = {
> .buf_setup = pxa_videobuf_setup,
> .buf_prepare = pxa_videobuf_prepare,
> --
> 2.1.4
>


2015-06-20 21:50:33

by Robert Jarzmik

[permalink] [raw]
Subject: Re: [PATCH 3/4] media: pxa_camera: trivial move of dma irq functions

Guennadi Liakhovetski <[email protected]> writes:

>> +static void pxa_camera_dma_irq(struct pxa_camera_dev *pcdev,
>> + enum pxa_camera_active_dma act_dma);
>> +
>> +static void pxa_camera_dma_irq_y(void *data)
>
> Wait, how is this patch trivial? You change pxa_camera_dma_irq_?()
> prototypes, which are used as PXA DMA callbacks. Does this mean, that
> either before or after this patch compilation is broken?

Jeez you're right.
So I can either fold that with patch 4, or try to rework it somehow ...

Cheers.

--
Robert

2015-06-21 16:11:52

by Guennadi Liakhovetski

[permalink] [raw]
Subject: Re: [PATCH 3/4] media: pxa_camera: trivial move of dma irq functions

On Sat, 20 Jun 2015, Robert Jarzmik wrote:

> Guennadi Liakhovetski <[email protected]> writes:
>
> >> +static void pxa_camera_dma_irq(struct pxa_camera_dev *pcdev,
> >> + enum pxa_camera_active_dma act_dma);
> >> +
> >> +static void pxa_camera_dma_irq_y(void *data)
> >
> > Wait, how is this patch trivial? You change pxa_camera_dma_irq_?()
> > prototypes, which are used as PXA DMA callbacks. Does this mean, that
> > either before or after this patch compilation is broken?
>
> Jeez you're right.
> So I can either fold that with patch 4, or try to rework it somehow ...

How about letting that patch do exactly what it says it does? Just move
functions up in the file if you need them there, without changing them,
and only change them when it's needed?

Thanks
Guennadi

2015-06-21 18:07:21

by Robert Jarzmik

[permalink] [raw]
Subject: Re: [PATCH 3/4] media: pxa_camera: trivial move of dma irq functions

Guennadi Liakhovetski <[email protected]> writes:

> On Sat, 20 Jun 2015, Robert Jarzmik wrote:
>
>> Guennadi Liakhovetski <[email protected]> writes:
>>
>> >> +static void pxa_camera_dma_irq(struct pxa_camera_dev *pcdev,
>> >> + enum pxa_camera_active_dma act_dma);
>> >> +
>> >> +static void pxa_camera_dma_irq_y(void *data)
>> >
>> > Wait, how is this patch trivial? You change pxa_camera_dma_irq_?()
>> > prototypes, which are used as PXA DMA callbacks. Does this mean, that
>> > either before or after this patch compilation is broken?
>>
>> Jeez you're right.
>> So I can either fold that with patch 4, or try to rework it somehow ...
>
> How about letting that patch do exactly what it says it does? Just move
> functions up in the file if you need them there, without changing them,
> and only change them when it's needed?
Deal, for next iteration.

Cheers.

--
Robert