[PATCH] staging: sm750fb: Remove volatile

[PATCH] staging: sm750fb: Remove volatile

[Aliya Rahmani]

Remove volatile to fix checkpatch.pl warning.

Signed-off-by: Aliya Rahmani <aliyarahmani786@gmail.com>
---
 drivers/staging/sm750fb/sm750.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
index aff69661c8e6..7a11f8593596 100644
--- a/drivers/staging/sm750fb/sm750.h
+++ b/drivers/staging/sm750fb/sm750.h
@@ -52,7 +52,7 @@ struct lynx_accel {
 	/* base virtual address of DPR registers */
 	volatile unsigned char __iomem *dprBase;
 	/* base virtual address of de data port */
-	volatile unsigned char __iomem *dpPortBase;
+	unsigned char __iomem *dpPortBase;
 
 	/* function pointers */
 	void (*de_init)(struct lynx_accel *accel);
-- 
2.25.1

Re: [PATCH] staging: sm750fb: Remove volatile

[Alison Schofield]

On Thu, Apr 14, 2022 at 02:09:15AM +0530, Aliya Rahmani wrote:

Hi Aliya,

Wondering why the recipient list different than this:

$ perl scripts/get_maintainer.pl --nogit-fallback --no-rolestats -f drivers/staging/sm750fb/sm750.h
Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Teddy Wang <teddy.wang@siliconmotion.com>
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
linux-fbdev@vger.kernel.org
linux-staging@lists.linux.dev
linux-kernel@vger.kernel.org


> Remove volatile to fix checkpatch.pl warning.
> 
> Signed-off-by: Aliya Rahmani <aliyarahmani786@gmail.com>
> ---
>  drivers/staging/sm750fb/sm750.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
> index aff69661c8e6..7a11f8593596 100644
> --- a/drivers/staging/sm750fb/sm750.h
> +++ b/drivers/staging/sm750fb/sm750.h
> @@ -52,7 +52,7 @@ struct lynx_accel {
>  	/* base virtual address of DPR registers */
>  	volatile unsigned char __iomem *dprBase;
>  	/* base virtual address of de data port */
> -	volatile unsigned char __iomem *dpPortBase;
> +	unsigned char __iomem *dpPortBase;
>  
>  	/* function pointers */
>  	void (*de_init)(struct lynx_accel *accel);
> -- 
> 2.25.1
> 
> 

Re: [PATCH] staging: sm750fb: Remove volatile

[Aliya]

On Thu, Apr 14, 2022 at 6:40 AM Alison Schofield
<alison.schofield@intel.com> wrote:
>
> On Thu, Apr 14, 2022 at 02:09:15AM +0530, Aliya Rahmani wrote:
>
> Hi Aliya,
>
> Wondering why the recipient list different than this:
>
> $ perl scripts/get_maintainer.pl --nogit-fallback --no-rolestats -f drivers/staging/sm750fb/sm750.h
> Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> Teddy Wang <teddy.wang@siliconmotion.com>
> Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> linux-fbdev@vger.kernel.org
> linux-staging@lists.linux.dev
> linux-kernel@vger.kernel.org
>
Do I need to include all the maintainers and mailing lists?
>
> > Remove volatile to fix checkpatch.pl warning.
> >
> > Signed-off-by: Aliya Rahmani <aliyarahmani786@gmail.com>
> > ---
> >  drivers/staging/sm750fb/sm750.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
> > index aff69661c8e6..7a11f8593596 100644
> > --- a/drivers/staging/sm750fb/sm750.h
> > +++ b/drivers/staging/sm750fb/sm750.h
> > @@ -52,7 +52,7 @@ struct lynx_accel {
> >       /* base virtual address of DPR registers */
> >       volatile unsigned char __iomem *dprBase;
> >       /* base virtual address of de data port */
> > -     volatile unsigned char __iomem *dpPortBase;
> > +     unsigned char __iomem *dpPortBase;
> >
> >       /* function pointers */
> >       void (*de_init)(struct lynx_accel *accel);
> > --
> > 2.25.1
> >
> >

Thanks,
Aliya

Re: [PATCH] staging: sm750fb: Remove volatile

[Greg KH]

On Thu, Apr 14, 2022 at 02:09:15AM +0530, Aliya Rahmani wrote:
> Remove volatile to fix checkpatch.pl warning.
> 
> Signed-off-by: Aliya Rahmani <aliyarahmani786@gmail.com>
> ---
>  drivers/staging/sm750fb/sm750.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
> index aff69661c8e6..7a11f8593596 100644
> --- a/drivers/staging/sm750fb/sm750.h
> +++ b/drivers/staging/sm750fb/sm750.h
> @@ -52,7 +52,7 @@ struct lynx_accel {
>  	/* base virtual address of DPR registers */
>  	volatile unsigned char __iomem *dprBase;
>  	/* base virtual address of de data port */
> -	volatile unsigned char __iomem *dpPortBase;
> +	unsigned char __iomem *dpPortBase;

So you have properly determined that this code is now fixed and does not
require this volatile marking?  :)

Please don't blindly follow checkpatch suggestions without understanding
what they do.  While the use of volatile is probably wrong here, you
need to ensure that you can successfully remove this so you need to
prove it in the changelog text that all is still ok.

Also, why did you only remove one?

thanks,

greg k-h

Re: [PATCH] staging: sm750fb: Remove volatile

[Dan Carpenter]

On Thu, Apr 14, 2022 at 02:09:15AM +0530, Aliya Rahmani wrote:
> Remove volatile to fix checkpatch.pl warning.
> 

You want to sound more confident like you know what you're talking
about and not just doing things because checkpatch says.  "The volatile
is meaningless here and checkpatch.pl complains about it.  Delete it."

> Signed-off-by: Aliya Rahmani <aliyarahmani786@gmail.com>
> ---
>  drivers/staging/sm750fb/sm750.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
> index aff69661c8e6..7a11f8593596 100644
> --- a/drivers/staging/sm750fb/sm750.h
> +++ b/drivers/staging/sm750fb/sm750.h
> @@ -52,7 +52,7 @@ struct lynx_accel {
>  	/* base virtual address of DPR registers */
>  	volatile unsigned char __iomem *dprBase;
        ^^^^^^^^
Missed one here.

>  	/* base virtual address of de data port */
> -	volatile unsigned char __iomem *dpPortBase;
> +	unsigned char __iomem *dpPortBase;

regards,
dan carpenter

Re: [PATCH] staging: sm750fb: Remove volatile

[Aliya]

On Thu, Apr 14, 2022 at 12:26 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Apr 14, 2022 at 02:09:15AM +0530, Aliya Rahmani wrote:
> > Remove volatile to fix checkpatch.pl warning.
> >
> > Signed-off-by: Aliya Rahmani <aliyarahmani786@gmail.com>
> > ---
> >  drivers/staging/sm750fb/sm750.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
> > index aff69661c8e6..7a11f8593596 100644
> > --- a/drivers/staging/sm750fb/sm750.h
> > +++ b/drivers/staging/sm750fb/sm750.h
> > @@ -52,7 +52,7 @@ struct lynx_accel {
> >       /* base virtual address of DPR registers */
> >       volatile unsigned char __iomem *dprBase;
> >       /* base virtual address of de data port */
> > -     volatile unsigned char __iomem *dpPortBase;
> > +     unsigned char __iomem *dpPortBase;
>
> So you have properly determined that this code is now fixed and does not
> require this volatile marking?  :)
>
> Please don't blindly follow checkpatch suggestions without understanding
> what they do.  While the use of volatile is probably wrong here, you
> need to ensure that you can successfully remove this so you need to
> prove it in the changelog text that all is still ok.
>
> Also, why did you only remove one?
>
> thanks,
>
> greg k-h

My apologies, I'll verify and understand the code before following
checkpatch suggestions.

Thanks,
Aliya

Re: [PATCH] staging: sm750fb: Remove volatile

[Alison Schofield]

On Wed, Apr 13, 2022 at 11:06:53PM -0700, Aliya wrote:
> On Thu, Apr 14, 2022 at 6:40 AM Alison Schofield
> <alison.schofield@intel.com> wrote:
> >
> > On Thu, Apr 14, 2022 at 02:09:15AM +0530, Aliya Rahmani wrote:
> >
> > Hi Aliya,
> >
> > Wondering why the recipient list different than this:
> >
> > $ perl scripts/get_maintainer.pl --nogit-fallback --no-rolestats -f drivers/staging/sm750fb/sm750.h
> > Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> > Teddy Wang <teddy.wang@siliconmotion.com>
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > linux-fbdev@vger.kernel.org
> > linux-staging@lists.linux.dev
> > linux-kernel@vger.kernel.org
> >
> Do I need to include all the maintainers and mailing lists?

Usually - yes. It's possible that a maintainer has instructed
otherwise, and in that case the maintainers file can be updated.
That's why I posed it as a 'wondering why' question :)

Instructions for using get_maintainers.pl are here:
https://kernelnewbies.org/Outreachyfirstpatch

And, here's a prior round email discussion of same:
https://lore.kernel.org/outreachy/20211015171331.GA431883@alison-desk/

Alison

> >





> > > Remove volatile to fix checkpatch.pl warning.
> > >
> > > Signed-off-by: Aliya Rahmani <aliyarahmani786@gmail.com>
> > > ---
> > >  drivers/staging/sm750fb/sm750.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
> > > index aff69661c8e6..7a11f8593596 100644
> > > --- a/drivers/staging/sm750fb/sm750.h
> > > +++ b/drivers/staging/sm750fb/sm750.h
> > > @@ -52,7 +52,7 @@ struct lynx_accel {
> > >       /* base virtual address of DPR registers */
> > >       volatile unsigned char __iomem *dprBase;
> > >       /* base virtual address of de data port */
> > > -     volatile unsigned char __iomem *dpPortBase;
> > > +     unsigned char __iomem *dpPortBase;
> > >
> > >       /* function pointers */
> > >       void (*de_init)(struct lynx_accel *accel);
> > > --
> > > 2.25.1
> > >
> > >
> 
> Thanks,
> Aliya