From: Yang Guang <[email protected]>
Use the macro 'swap()' defined in 'include/linux/minmax.h' to avoid
opencoding it.
Signed-off-by: Yang Guang <[email protected]>
---
drivers/media/pci/saa7134/saa7134-video.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
index 374c8e1087de..6f4132058c35 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -823,7 +823,7 @@ static int buffer_activate(struct saa7134_dev *dev,
{
struct saa7134_dmaqueue *dmaq = buf->vb2.vb2_buf.vb2_queue->drv_priv;
unsigned long base,control,bpl;
- unsigned long bpl_uv,lines_uv,base2,base3,tmp; /* planar */
+ unsigned long bpl_uv, lines_uv, base2, base3; /* planar */
video_dbg("buffer_activate buf=%p\n", buf);
buf->top_seen = 0;
@@ -869,9 +869,7 @@ static int buffer_activate(struct saa7134_dev *dev,
base2 = base + bpl * dev->height;
base3 = base2 + bpl_uv * lines_uv;
if (dev->fmt->uvswap) {
- tmp = base2;
- base2 = base3;
- base3 = tmp;
+ swap(base2, base3);
}
video_dbg("uv: bpl=%ld lines=%ld base2/3=%ld/%ld\n",
bpl_uv,lines_uv,base2,base3);
--
2.30.2
Hi David,
Is this patch something you discovered somewhere and have posted on
behalf of Yang Guang?
Have you made any modifications to it that would require your sign off?
or is it simply a repost of a patch you came across?
Quoting [email protected] (2021-11-03 08:33:37)
> From: Yang Guang <[email protected]>
>
> Use the macro 'swap()' defined in 'include/linux/minmax.h' to avoid
> opencoding it.
>
> Signed-off-by: Yang Guang <[email protected]>
The change itself looks fine to me though.
Reviewed-by: Kieran Bingham <[email protected]>
> ---
> drivers/media/pci/saa7134/saa7134-video.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
> index 374c8e1087de..6f4132058c35 100644
> --- a/drivers/media/pci/saa7134/saa7134-video.c
> +++ b/drivers/media/pci/saa7134/saa7134-video.c
> @@ -823,7 +823,7 @@ static int buffer_activate(struct saa7134_dev *dev,
> {
> struct saa7134_dmaqueue *dmaq = buf->vb2.vb2_buf.vb2_queue->drv_priv;
> unsigned long base,control,bpl;
> - unsigned long bpl_uv,lines_uv,base2,base3,tmp; /* planar */
> + unsigned long bpl_uv, lines_uv, base2, base3; /* planar */
>
> video_dbg("buffer_activate buf=%p\n", buf);
> buf->top_seen = 0;
> @@ -869,9 +869,7 @@ static int buffer_activate(struct saa7134_dev *dev,
> base2 = base + bpl * dev->height;
> base3 = base2 + bpl_uv * lines_uv;
> if (dev->fmt->uvswap) {
> - tmp = base2;
> - base2 = base3;
> - base3 = tmp;
> + swap(base2, base3);
> }
> video_dbg("uv: bpl=%ld lines=%ld base2/3=%ld/%ld\n",
> bpl_uv,lines_uv,base2,base3);
> --
> 2.30.2
>
Hi Kieran,
Both of [email protected] and [email protected] are
mine.
Because the [email protected] can't send email out. So I use the
gmail to send.
Thanks.
On Wed, Nov 03, 2021 at 03:14:14PM +0000, Kieran Bingham wrote:
> Hi David,
>
> Is this patch something you discovered somewhere and have posted on
> behalf of Yang Guang?
>
> Have you made any modifications to it that would require your sign off?
> or is it simply a repost of a patch you came across?
>
> Quoting [email protected] (2021-11-03 08:33:37)
> > From: Yang Guang <[email protected]>
> >
> > Use the macro 'swap()' defined in 'include/linux/minmax.h' to avoid
> > opencoding it.
> >
> > Signed-off-by: Yang Guang <[email protected]>
>
> The change itself looks fine to me though.
>
> Reviewed-by: Kieran Bingham <[email protected]>
>
> > ---
> > drivers/media/pci/saa7134/saa7134-video.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
> > index 374c8e1087de..6f4132058c35 100644
> > --- a/drivers/media/pci/saa7134/saa7134-video.c
> > +++ b/drivers/media/pci/saa7134/saa7134-video.c
> > @@ -823,7 +823,7 @@ static int buffer_activate(struct saa7134_dev *dev,
> > {
> > struct saa7134_dmaqueue *dmaq = buf->vb2.vb2_buf.vb2_queue->drv_priv;
> > unsigned long base,control,bpl;
> > - unsigned long bpl_uv,lines_uv,base2,base3,tmp; /* planar */
> > + unsigned long bpl_uv, lines_uv, base2, base3; /* planar */
> >
> > video_dbg("buffer_activate buf=%p\n", buf);
> > buf->top_seen = 0;
> > @@ -869,9 +869,7 @@ static int buffer_activate(struct saa7134_dev *dev,
> > base2 = base + bpl * dev->height;
> > base3 = base2 + bpl_uv * lines_uv;
> > if (dev->fmt->uvswap) {
> > - tmp = base2;
> > - base2 = base3;
> > - base3 = tmp;
> > + swap(base2, base3);
> > }
> > video_dbg("uv: bpl=%ld lines=%ld base2/3=%ld/%ld\n",
> > bpl_uv,lines_uv,base2,base3);
> > --
> > 2.30.2
> >
On Wed, Nov 3, 2021 at 10:34 AM <[email protected]> wrote:
> Use the macro 'swap()' defined in 'include/linux/minmax.h' to avoid
> opencoding it.
Same comments as per all your valuable contributions: just think more
about the code that you are dealing with!
> if (dev->fmt->uvswap) {
> - tmp = base2;
> - base2 = base3;
> - base3 = tmp;
> + swap(base2, base3);
> }
Have you run checkpatch? What did it say?
--
With Best Regards,
Andy Shevchenko
On 04/11/2021 11:43, Andy Shevchenko wrote:
> On Wed, Nov 3, 2021 at 10:34 AM <[email protected]> wrote:
>> Use the macro 'swap()' defined in 'include/linux/minmax.h' to avoid
>> opencoding it.
>
> Same comments as per all your valuable contributions: just think more
> about the code that you are dealing with!
>
>> if (dev->fmt->uvswap) {
>> - tmp = base2;
>> - base2 = base3;
>> - base3 = tmp;
>> + swap(base2, base3);
>> }
>
> Have you run checkpatch? What did it say?
>
checkpatch says all is fine :-)
But yes, the {} can now be dropped. If I apply the patch, then run checkpatch,
it will indeed complain about the {}.
Regards,
Hans