staging: vt6655: mac.h: Fix possible precedence issue in macros

[PATCH] staging: vt6655: mac.h: Fix possible precedence issue in macros

Posted by Dominik Karol Piątkowski 2 months, 2 weeks ago

It is safer to put macro arguments in parentheses. This way, accidental
operator precedence issues can be avoided.

Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@protonmail.com>
---
 drivers/staging/vt6655/mac.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
index acf931c3f5fd..a33af2852227 100644
--- a/drivers/staging/vt6655/mac.h
+++ b/drivers/staging/vt6655/mac.h
@@ -537,9 +537,9 @@
 
 /*---------------------  Export Macros ------------------------------*/
 
-#define VT6655_MAC_SELECT_PAGE0(iobase) iowrite8(0, iobase + MAC_REG_PAGE1SEL)
+#define VT6655_MAC_SELECT_PAGE0(iobase) iowrite8(0, (iobase) + MAC_REG_PAGE1SEL)
 
-#define VT6655_MAC_SELECT_PAGE1(iobase) iowrite8(1, iobase + MAC_REG_PAGE1SEL)
+#define VT6655_MAC_SELECT_PAGE1(iobase) iowrite8(1, (iobase) + MAC_REG_PAGE1SEL)
 
 #define MAKEWORD(lb, hb) \
 	((unsigned short)(((unsigned char)(lb)) | (((unsigned short)((unsigned char)(hb))) << 8)))
-- 
2.34.1

Re: [PATCH] staging: vt6655: mac.h: Fix possible precedence issue in macros

Posted by Philipp Hortmann 2 months, 2 weeks ago

On 9/11/24 20:02, Dominik Karol Piątkowski wrote:
> It is safer to put macro arguments in parentheses. This way, accidental
> operator precedence issues can be avoided.
> 
> Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@protonmail.com>
> ---
>   drivers/staging/vt6655/mac.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
> index acf931c3f5fd..a33af2852227 100644
> --- a/drivers/staging/vt6655/mac.h
> +++ b/drivers/staging/vt6655/mac.h
> @@ -537,9 +537,9 @@
>   
>   /*---------------------  Export Macros ------------------------------*/
>   
> -#define VT6655_MAC_SELECT_PAGE0(iobase) iowrite8(0, iobase + MAC_REG_PAGE1SEL)
> +#define VT6655_MAC_SELECT_PAGE0(iobase) iowrite8(0, (iobase) + MAC_REG_PAGE1SEL)
>   
> -#define VT6655_MAC_SELECT_PAGE1(iobase) iowrite8(1, iobase + MAC_REG_PAGE1SEL)
> +#define VT6655_MAC_SELECT_PAGE1(iobase) iowrite8(1, (iobase) + MAC_REG_PAGE1SEL)
>   
>   #define MAKEWORD(lb, hb) \
>   	((unsigned short)(((unsigned char)(lb)) | (((unsigned short)((unsigned char)(hb))) << 8)))


Hi Dominik,

git shows your name with the following characters:

Author: Dominik Karol Pi^Etkowski <dominik.karol.piatkowski@protonmail.com>

I think it is better to change your name to only english letters.

If you send in a second version of this patch please use a change 
history. Description from Dan under:
https://staticthinking.wordpress.com/2022/07/27/how-to-send-a-v2-patch/

Thanks for your support.

Bye Philipp

Re: [PATCH] staging: vt6655: mac.h: Fix possible precedence issue in macros

Posted by Dominik Karol Piątkowski 2 months, 2 weeks ago

On Thursday, September 12th, 2024 at 21:29, Philipp Hortmann <philipp.g.hortmann@gmail.com> wrote:

> 
> 
> On 9/11/24 20:02, Dominik Karol Piątkowski wrote:
> 
> > It is safer to put macro arguments in parentheses. This way, accidental
> > operator precedence issues can be avoided.
> > 
> > Signed-off-by: Dominik Karol Piątkowski dominik.karol.piatkowski@protonmail.com
> > ---
> > drivers/staging/vt6655/mac.h | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
> > index acf931c3f5fd..a33af2852227 100644
> > --- a/drivers/staging/vt6655/mac.h
> > +++ b/drivers/staging/vt6655/mac.h
> > @@ -537,9 +537,9 @@
> > 
> > /--------------------- Export Macros ------------------------------/
> > 
> > -#define VT6655_MAC_SELECT_PAGE0(iobase) iowrite8(0, iobase + MAC_REG_PAGE1SEL)
> > +#define VT6655_MAC_SELECT_PAGE0(iobase) iowrite8(0, (iobase) + MAC_REG_PAGE1SEL)
> > 
> > -#define VT6655_MAC_SELECT_PAGE1(iobase) iowrite8(1, iobase + MAC_REG_PAGE1SEL)
> > +#define VT6655_MAC_SELECT_PAGE1(iobase) iowrite8(1, (iobase) + MAC_REG_PAGE1SEL)
> > 
> > #define MAKEWORD(lb, hb) \
> > ((unsigned short)(((unsigned char)(lb)) | (((unsigned short)((unsigned char)(hb))) << 8)))
> 
> 
> 
> Hi Dominik,
> 
> git shows your name with the following characters:
> 
> Author: Dominik Karol Pi^Etkowski dominik.karol.piatkowski@protonmail.com
> 
> 
> I think it is better to change your name to only english letters.
> 
> If you send in a second version of this patch please use a change
> history. Description from Dan under:
> https://staticthinking.wordpress.com/2022/07/27/how-to-send-a-v2-patch/
> 
> Thanks for your support.
> 
> Bye Philipp

Hi Philipp,

Thanks for testing my patch.

About the mangled author field - it was sent as
"From: =?UTF-8?q?Dominik=20Karol=20Pi=C4=85tkowski?= <dominik.karol.piatkowski@protonmail.com>"
and =C4=85 in UTF-8 is indeed 'ą' character. When looking at linux-next tree,
previously accepted patches also seem to have 'ą' as intended. I am not sure
why you are seeing "^E" instead.

Thanks,
Dominik Karol

Re: [PATCH] staging: vt6655: mac.h: Fix possible precedence issue in macros

Posted by Philipp Hortmann 2 months, 2 weeks ago

On 9/12/24 22:18, Dominik Karol Piątkowski wrote:
> On Thursday, September 12th, 2024 at 21:29, Philipp Hortmann <philipp.g.hortmann@gmail.com> wrote:
> 
>>
>>
>> On 9/11/24 20:02, Dominik Karol Piątkowski wrote:
>>
>>> It is safer to put macro arguments in parentheses. This way, accidental
>>> operator precedence issues can be avoided.
>>>
>>> Signed-off-by: Dominik Karol Piątkowski dominik.karol.piatkowski@protonmail.com
>>> ---
>>> drivers/staging/vt6655/mac.h | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
>>> index acf931c3f5fd..a33af2852227 100644
>>> --- a/drivers/staging/vt6655/mac.h
>>> +++ b/drivers/staging/vt6655/mac.h
>>> @@ -537,9 +537,9 @@
>>>
>>> /--------------------- Export Macros ------------------------------/
>>>
>>> -#define VT6655_MAC_SELECT_PAGE0(iobase) iowrite8(0, iobase + MAC_REG_PAGE1SEL)
>>> +#define VT6655_MAC_SELECT_PAGE0(iobase) iowrite8(0, (iobase) + MAC_REG_PAGE1SEL)
>>>
>>> -#define VT6655_MAC_SELECT_PAGE1(iobase) iowrite8(1, iobase + MAC_REG_PAGE1SEL)
>>> +#define VT6655_MAC_SELECT_PAGE1(iobase) iowrite8(1, (iobase) + MAC_REG_PAGE1SEL)
>>>
>>> #define MAKEWORD(lb, hb) \
>>> ((unsigned short)(((unsigned char)(lb)) | (((unsigned short)((unsigned char)(hb))) << 8)))
>>
>>
>>
>> Hi Dominik,
>>
>> git shows your name with the following characters:
>>
>> Author: Dominik Karol Pi^Etkowski dominik.karol.piatkowski@protonmail.com
>>
>>
>> I think it is better to change your name to only english letters.
>>
>> If you send in a second version of this patch please use a change
>> history. Description from Dan under:
>> https://staticthinking.wordpress.com/2022/07/27/how-to-send-a-v2-patch/
>>
>> Thanks for your support.
>>
>> Bye Philipp
> 
> Hi Philipp,
> 
> Thanks for testing my patch.
> 
> About the mangled author field - it was sent as
> "From: =?UTF-8?q?Dominik=20Karol=20Pi=C4=85tkowski?= <dominik.karol.piatkowski@protonmail.com>"
> and =C4=85 in UTF-8 is indeed 'ą' character. When looking at linux-next tree,
> previously accepted patches also seem to have 'ą' as intended. I am not sure
> why you are seeing "^E" instead.
> 
> Thanks,
> Dominik Karol


Hi Dominik Karol,

you are right. On the from Greg accepted patches the 'ą' is as intended.

Then everything is all right. Sorry for the noise.

Thanks for your support.

Bye Philipp

Re: [PATCH] staging: vt6655: mac.h: Fix possible precedence issue in macros

Posted by Philipp Hortmann 2 months, 2 weeks ago

On 9/11/24 20:02, Dominik Karol Piątkowski wrote:
> It is safer to put macro arguments in parentheses. This way, accidental
> operator precedence issues can be avoided.
> 
> Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@protonmail.com>
> ---
>   drivers/staging/vt6655/mac.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
> index acf931c3f5fd..a33af2852227 100644
> --- a/drivers/staging/vt6655/mac.h
> +++ b/drivers/staging/vt6655/mac.h
> @@ -537,9 +537,9 @@
>   
>   /*---------------------  Export Macros ------------------------------*/
>   
> -#define VT6655_MAC_SELECT_PAGE0(iobase) iowrite8(0, iobase + MAC_REG_PAGE1SEL)
> +#define VT6655_MAC_SELECT_PAGE0(iobase) iowrite8(0, (iobase) + MAC_REG_PAGE1SEL)
>   
> -#define VT6655_MAC_SELECT_PAGE1(iobase) iowrite8(1, iobase + MAC_REG_PAGE1SEL)
> +#define VT6655_MAC_SELECT_PAGE1(iobase) iowrite8(1, (iobase) + MAC_REG_PAGE1SEL)
>   
>   #define MAKEWORD(lb, hb) \
>   	((unsigned short)(((unsigned char)(lb)) | (((unsigned short)((unsigned char)(hb))) << 8)))


Tested-by: Philipp Hortmann <philipp.g.hortmann@gmail.com>