[PATCH] staging: sm750fb: Avoid multiple assignments

[PATCH] staging: sm750fb: Avoid multiple assignments

[Nishka Dasgupta]

Avoid multiple assignments in the same line. Issue found with
Checkpatch.

Signed-off-by: Nishka Dasgupta <nishka.dasgupta@yahoo.com>
---
 drivers/staging/sm750fb/sm750.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index e9f10c2669ea..739841c3b5d6 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -782,7 +782,8 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
 		0x800f0 + (int)crtc->channel * 0x140;
 
 	pr_info("crtc->cursor.mmio = %p\n", crtc->cursor.mmio);
-	crtc->cursor.maxH = crtc->cursor.maxW = 64;
+	crtc->cursor.maxW = 64;
+	crtc->cursor.maxH = crtc->cursor.maxW;
 	crtc->cursor.size = crtc->cursor.maxH * crtc->cursor.maxW * 2 / 8;
 	crtc->cursor.vstart = sm750_dev->pvMem + crtc->cursor.offset;
 
-- 
2.19.1

Re: [Outreachy kernel] [PATCH] staging: sm750fb: Avoid multiple assignments

[Julia Lawall]

On Tue, 5 Mar 2019, 'Nishka Dasgupta' via outreachy-kernel wrote:

Something is going wrong in how you are sending your mail.  This message
was put in the group's spam folder.  Maybe you are subscribed with a
different address?

julia



> Avoid multiple assignments in the same line. Issue found with
> Checkpatch.
>
> Signed-off-by: Nishka Dasgupta <nishka.dasgupta@yahoo.com>
> ---
>  drivers/staging/sm750fb/sm750.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index e9f10c2669ea..739841c3b5d6 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -782,7 +782,8 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
>  		0x800f0 + (int)crtc->channel * 0x140;
>
>  	pr_info("crtc->cursor.mmio = %p\n", crtc->cursor.mmio);
> -	crtc->cursor.maxH = crtc->cursor.maxW = 64;
> +	crtc->cursor.maxW = 64;
> +	crtc->cursor.maxH = crtc->cursor.maxW;

Maybe it would be simpler to say = 64 both times?

julia

>  	crtc->cursor.size = crtc->cursor.maxH * crtc->cursor.maxW * 2 / 8;
>  	crtc->cursor.vstart = sm750_dev->pvMem + crtc->cursor.offset;
>
> --
> 2.19.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20190304183110.3702-1-nishka.dasgupta%40yahoo.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH] staging: sm750fb: Avoid multiple assignments

[Nishka Dasgupta]

On Tue, Mar 5, 2019 at 2:24 AM Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
>
> On Tue, 5 Mar 2019, 'Nishka Dasgupta' via outreachy-kernel wrote:
>
> Something is going wrong in how you are sending your mail.  This message
> was put in the group's spam folder.  Maybe you are subscribed with a
> different address?
>
Yes, I am subscribed with nishka.dasgupta_ug18@ashoka.edu.in but I am
sending patches from nishka.dasgupta@yahoo.com. I tried to join the
kernel google group with my Yahoo address but it wouldn't let me; what
should I do?
> julia
>
>
>
> > Avoid multiple assignments in the same line. Issue found with
> > Checkpatch.
> >
> > Signed-off-by: Nishka Dasgupta <nishka.dasgupta@yahoo.com>
> > ---
> >  drivers/staging/sm750fb/sm750.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> > index e9f10c2669ea..739841c3b5d6 100644
> > --- a/drivers/staging/sm750fb/sm750.c
> > +++ b/drivers/staging/sm750fb/sm750.c
> > @@ -782,7 +782,8 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
> >               0x800f0 + (int)crtc->channel * 0x140;
> >
> >       pr_info("crtc->cursor.mmio = %p\n", crtc->cursor.mmio);
> > -     crtc->cursor.maxH = crtc->cursor.maxW = 64;
> > +     crtc->cursor.maxW = 64;
> > +     crtc->cursor.maxH = crtc->cursor.maxW;
>
> Maybe it would be simpler to say = 64 both times?
Yes, it would, but I wasn't sure if it was important for maxH and maxW
to always have the same value; if so, then setting one to 64 and the
other to always have the same value as the first seemed safer. If they
don't have to be the same, I can change accordingly.

> julia
>
> >       crtc->cursor.size = crtc->cursor.maxH * crtc->cursor.maxW * 2 / 8;
> >       crtc->cursor.vstart = sm750_dev->pvMem + crtc->cursor.offset;
> >
> > --
> > 2.19.1
> >
> > --
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20190304183110.3702-1-nishka.dasgupta%40yahoo.com.
> > For more options, visit https://groups.google.com/d/optout.
> >
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/alpine.DEB.2.21.1903042153230.2591%40hadrien.
> For more options, visit https://groups.google.com/d/optout.

- Nishka

Re: [PATCH] staging: sm750fb: Avoid multiple assignments

[Greg KH]

On Tue, Mar 05, 2019 at 12:01:10AM +0530, Nishka Dasgupta wrote:
> Avoid multiple assignments in the same line. Issue found with
> Checkpatch.
> 
> Signed-off-by: Nishka Dasgupta <nishka.dasgupta@yahoo.com>
> ---
>  drivers/staging/sm750fb/sm750.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index e9f10c2669ea..739841c3b5d6 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -782,7 +782,8 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
>  		0x800f0 + (int)crtc->channel * 0x140;
>  
>  	pr_info("crtc->cursor.mmio = %p\n", crtc->cursor.mmio);
> -	crtc->cursor.maxH = crtc->cursor.maxW = 64;
> +	crtc->cursor.maxW = 64;
> +	crtc->cursor.maxH = crtc->cursor.maxW;

The original code is fine here.  I would just ignore this checkpatch.pl
message for this line.

thanks,

greg k-h

Re: [PATCH] staging: sm750fb: Avoid multiple assignments

[Nishka Dasgupta]

On 05/03/19 1:21 PM, Greg KH wrote:
> On Tue, Mar 05, 2019 at 12:01:10AM +0530, Nishka Dasgupta wrote:
>> Avoid multiple assignments in the same line. Issue found with
>> Checkpatch.
>>
>> Signed-off-by: Nishka Dasgupta <nishka.dasgupta@yahoo.com>
>> ---
>>   drivers/staging/sm750fb/sm750.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
>> index e9f10c2669ea..739841c3b5d6 100644
>> --- a/drivers/staging/sm750fb/sm750.c
>> +++ b/drivers/staging/sm750fb/sm750.c
>> @@ -782,7 +782,8 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
>>   		0x800f0 + (int)crtc->channel * 0x140;
>>   
>>   	pr_info("crtc->cursor.mmio = %p\n", crtc->cursor.mmio);
>> -	crtc->cursor.maxH = crtc->cursor.maxW = 64;
>> +	crtc->cursor.maxW = 64;
>> +	crtc->cursor.maxH = crtc->cursor.maxW;
> 
> The original code is fine here.  I would just ignore this checkpatch.pl
> message for this line.
Okay, will do. What should I do for similar Checkpatch warnings in 
future? Do I just ignore them?

> thanks,
> 
> greg k-h
> 

Re: [PATCH] staging: sm750fb: Avoid multiple assignments

[Greg KH]

On Tue, Mar 05, 2019 at 10:42:08PM +0530, Nishka Dasgupta wrote:
> 
> 
> On 05/03/19 1:21 PM, Greg KH wrote:
> > On Tue, Mar 05, 2019 at 12:01:10AM +0530, Nishka Dasgupta wrote:
> > > Avoid multiple assignments in the same line. Issue found with
> > > Checkpatch.
> > > 
> > > Signed-off-by: Nishka Dasgupta <nishka.dasgupta@yahoo.com>
> > > ---
> > >   drivers/staging/sm750fb/sm750.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> > > index e9f10c2669ea..739841c3b5d6 100644
> > > --- a/drivers/staging/sm750fb/sm750.c
> > > +++ b/drivers/staging/sm750fb/sm750.c
> > > @@ -782,7 +782,8 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
> > >   		0x800f0 + (int)crtc->channel * 0x140;
> > >   	pr_info("crtc->cursor.mmio = %p\n", crtc->cursor.mmio);
> > > -	crtc->cursor.maxH = crtc->cursor.maxW = 64;
> > > +	crtc->cursor.maxW = 64;
> > > +	crtc->cursor.maxH = crtc->cursor.maxW;
> > 
> > The original code is fine here.  I would just ignore this checkpatch.pl
> > message for this line.
> Okay, will do. What should I do for similar Checkpatch warnings in future?
> Do I just ignore them?

It all depends on what they look like.  This one is setting a width and
height to the exact same value, making it pretty obvious what is going
on here.  If other code is doing other things (like just zeroing out
stuff, or using temp variables), maybe it should be fixed up.

Again, checkpatch is just a hint, you still need to use your brain to
see if what it says makes sense :)

thanks,

greg k-h

Re: [PATCH] staging: sm750fb: Avoid multiple assignments

[Nishka Dasgupta]

On 05/03/19 10:47 PM, Greg KH wrote:
> On Tue, Mar 05, 2019 at 10:42:08PM +0530, Nishka Dasgupta wrote:
>>
>>
>> On 05/03/19 1:21 PM, Greg KH wrote:
>>> On Tue, Mar 05, 2019 at 12:01:10AM +0530, Nishka Dasgupta wrote:
>>>> Avoid multiple assignments in the same line. Issue found with
>>>> Checkpatch.
>>>>
>>>> Signed-off-by: Nishka Dasgupta <nishka.dasgupta@yahoo.com>
>>>> ---
>>>>    drivers/staging/sm750fb/sm750.c | 3 ++-
>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
>>>> index e9f10c2669ea..739841c3b5d6 100644
>>>> --- a/drivers/staging/sm750fb/sm750.c
>>>> +++ b/drivers/staging/sm750fb/sm750.c
>>>> @@ -782,7 +782,8 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
>>>>    		0x800f0 + (int)crtc->channel * 0x140;
>>>>    	pr_info("crtc->cursor.mmio = %p\n", crtc->cursor.mmio);
>>>> -	crtc->cursor.maxH = crtc->cursor.maxW = 64;
>>>> +	crtc->cursor.maxW = 64;
>>>> +	crtc->cursor.maxH = crtc->cursor.maxW;
>>>
>>> The original code is fine here.  I would just ignore this checkpatch.pl
>>> message for this line.
>> Okay, will do. What should I do for similar Checkpatch warnings in future?
>> Do I just ignore them?
> 
> It all depends on what they look like.  This one is setting a width and
> height to the exact same value, making it pretty obvious what is going
> on here.  If other code is doing other things (like just zeroing out
> stuff, or using temp variables), maybe it should be fixed up.

> Again, checkpatch is just a hint, you still need to use your brain to
> see if what it says makes sense :)
Yes, of course. I'll take it on a case-by-case basis then, as you've 
suggested. Thank you!
  -Nishka

> thanks,
> 
> greg k-h
>