Re: [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces

[Sui Jingfeng <15330273260@xxxxxx>]

Hi

On 2023/5/11 15:55, Thomas Zimmermann wrote:
> Hi
>
> Am 10.05.23 um 20:20 schrieb Sui Jingfeng:
> > Hi, Thomas
> >
> >
> > I love your patch, yet something to improve:
> >
> >
> > On 2023/5/10 19:05, Thomas Zimmermann wrote:
> > > Fix coding style. No functional changes.
> > >
> > > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> > > Reviewed-by: Arnd Bergmann <arnd@xxxxxxxx>
> > > Reviewed-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
> > > Reviewed-by: Sui Jingfeng <suijingfeng@xxxxxxxxxxx>
> > > Tested-by: Sui Jingfeng <suijingfeng@xxxxxxxxxxx>
> > > ---
> > >   drivers/video/fbdev/matrox/matroxfb_accel.c | 6 +++---
> > >   drivers/video/fbdev/matrox/matroxfb_base.h  | 4 ++--
> > >   2 files changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/video/fbdev/matrox/matroxfb_accel.c 
> > > b/drivers/video/fbdev/matrox/matroxfb_accel.c
> > > index 9cb0685feddd..ce51227798a1 100644
> > > --- a/drivers/video/fbdev/matrox/matroxfb_accel.c
> > > +++ b/drivers/video/fbdev/matrox/matroxfb_accel.c
> > > @@ -88,7 +88,7 @@
> > >   static inline void matrox_cfb4_pal(u_int32_t* pal) {
> > >       unsigned int i;
> > > -
> > > +
> > >       for (i = 0; i < 16; i++) {
> > >           pal[i] = i * 0x11111111U;
> > >       }
> > > @@ -96,7 +96,7 @@ static inline void matrox_cfb4_pal(u_int32_t* pal) {
> > >   static inline void matrox_cfb8_pal(u_int32_t* pal) {
> > >       unsigned int i;
> > > -
> > > +
> > >       for (i = 0; i < 16; i++) {
> > >           pal[i] = i * 0x01010101U;
> > >       }
> > > @@ -482,7 +482,7 @@ static void matroxfb_1bpp_imageblit(struct 
> > > matrox_fb_info *minfo, u_int32_t fgx,
> > >               /* Tell... well, why bother... */
> > >               while (height--) {
> > >                   size_t i;
> > > -
> > > +
> > >                   for (i = 0; i < step; i += 4) {
> > >                       /* Hope that there are at least three readable 
> > > bytes beyond the end of bitmap */
> > > fb_writel(get_unaligned((u_int32_t*)(chardata + i)),mmio.vaddr);
> > > diff --git a/drivers/video/fbdev/matrox/matroxfb_base.h 
> > > b/drivers/video/fbdev/matrox/matroxfb_base.h
> > > index 958be6805f87..c93c69bbcd57 100644
> > > --- a/drivers/video/fbdev/matrox/matroxfb_base.h
> > > +++ b/drivers/video/fbdev/matrox/matroxfb_base.h
> > > @@ -301,9 +301,9 @@ struct matrox_altout {
> > >       int        (*verifymode)(void* altout_dev, u_int32_t mode);
> > >       int        (*getqueryctrl)(void* altout_dev,
> > >                       struct v4l2_queryctrl* ctrl);
> >
> > Noticed that there are plenty of coding style problems in 
> > matroxfb_base.h,
> >
> > why you only fix a few of them?   Take this two line as an example, 
> > shouldn't
> >
> > they be fixed also as following?
>
> I configured my text editor to remove trailing whitespaces 
> automatically. That keeps my own patches free of them.  But the editor 
> removes all trailing whitespaces, including those that have been there 
> before. If I encounter such a case, I split out the whitespace fix and 
> submit it separately.
>
> But the work I do within fbdev is mostly for improving DRM. For the 
> other issues in this file, I don't think that matroxfb should even be 
> around any longer. Fbdev has been deprecated for a long time. But a 
> small number of drivers are still in use and we still need its 
> framebuffer console. So someone should either put significant effort 
> into maintaining fbdev, or it should be phased out. But neither is 
> happening.
Ok, no problem, that sound fine and reasonable then.

The lines being modified has trailing whitespaces.

And I tested your patch again last night on loongarch and mips platform.

It still works in my testing case.

> Best regards
> Thomas
>
> >       int        (*verifymode)(void *altout_dev, u_int32_t mode);
> >       int        (*getqueryctrl)(void *altout_dev,
> >                       struct v4l2_queryctrl *ctrl);
> >
> >
> > > -    int        (*getctrl)(void* altout_dev,
> > > +    int        (*getctrl)(void *altout_dev,
> > >                      struct v4l2_control* ctrl);
> > > -    int        (*setctrl)(void* altout_dev,
> > > +    int        (*setctrl)(void *altout_dev,
> > >                      struct v4l2_control* ctrl);
> > >   };