ASoC: fsl_asrc: protect macro argument
diff mbox

Message ID 20170807063657.12581-1-stefan@agner.ch
State Accepted
Commit d1b726a9018e3f684ce190a1cbe012cb64f363d8
Headers show

Commit Message

Stefan Agner Aug. 7, 2017, 6:36 a.m. UTC
Protect macro argument with parentheses to avoid ambiguity.
This fixes a warning seen with clang:
  warning: logical not is only applied to the left hand side of this comparison

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 sound/soc/fsl/fsl_asrc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Nicolin Chen Aug. 7, 2017, 7:08 a.m. UTC | #1
On Sun, Aug 06, 2017 at 11:36:57PM -0700, Stefan Agner wrote:
> Protect macro argument with parentheses to avoid ambiguity.
> This fixes a warning seen with clang:
>   warning: logical not is only applied to the left hand side of this comparison
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>

Thanks

> ---
>  sound/soc/fsl/fsl_asrc.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/fsl/fsl_asrc.h b/sound/soc/fsl/fsl_asrc.h
> index 0f163abe4ba3..ec33dab4b909 100644
> --- a/sound/soc/fsl/fsl_asrc.h
> +++ b/sound/soc/fsl/fsl_asrc.h
> @@ -57,7 +57,7 @@
>  #define REG_ASRDOC			0x74
>  #define REG_ASRDI(i)			(REG_ASRDIA + (i << 3))
>  #define REG_ASRDO(i)			(REG_ASRDOA + (i << 3))
> -#define REG_ASRDx(x, i)			(x == IN ? REG_ASRDI(i) : REG_ASRDO(i))
> +#define REG_ASRDx(x, i)			((x) == IN ? REG_ASRDI(i) : REG_ASRDO(i))
>  
>  #define REG_ASRIDRHA			0x80
>  #define REG_ASRIDRLA			0x84
> -- 
> 2.13.3
>
Stefan Agner Dec. 6, 2017, 1:53 p.m. UTC | #2
On 2017-08-07 09:08, Nicolin Chen wrote:
> On Sun, Aug 06, 2017 at 11:36:57PM -0700, Stefan Agner wrote:
>> Protect macro argument with parentheses to avoid ambiguity.
>> This fixes a warning seen with clang:
>>   warning: logical not is only applied to the left hand side of this comparison
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
> 
> Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>

Mark, you seem to have merged other patches for this driver, can you
merge this one too?

--
Stefan

> 
> Thanks
> 
>> ---
>>  sound/soc/fsl/fsl_asrc.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/fsl/fsl_asrc.h b/sound/soc/fsl/fsl_asrc.h
>> index 0f163abe4ba3..ec33dab4b909 100644
>> --- a/sound/soc/fsl/fsl_asrc.h
>> +++ b/sound/soc/fsl/fsl_asrc.h
>> @@ -57,7 +57,7 @@
>>  #define REG_ASRDOC			0x74
>>  #define REG_ASRDI(i)			(REG_ASRDIA + (i << 3))
>>  #define REG_ASRDO(i)			(REG_ASRDOA + (i << 3))
>> -#define REG_ASRDx(x, i)			(x == IN ? REG_ASRDI(i) : REG_ASRDO(i))
>> +#define REG_ASRDx(x, i)			((x) == IN ? REG_ASRDI(i) : REG_ASRDO(i))
>>
>>  #define REG_ASRIDRHA			0x80
>>  #define REG_ASRIDRLA			0x84
>> --
>> 2.13.3
>>
Mark Brown Dec. 6, 2017, 2:11 p.m. UTC | #3
On Wed, Dec 06, 2017 at 02:53:39PM +0100, Stefan Agner wrote:
> On 2017-08-07 09:08, Nicolin Chen wrote:
> > On Sun, Aug 06, 2017 at 11:36:57PM -0700, Stefan Agner wrote:

> Mark, you seem to have merged other patches for this driver, can you
> merge this one too?

Please don't send content free pings and please allow a reasonable time
for review.  People get busy, go on holiday, attend conferences and so 
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review.  If there have been
review comments then people may be waiting for those to be addressed.

Sending content free pings adds to the mail volume (if they are seen at
all) which is often the problem and since they can't be reviewed
directly if something has gone wrong you'll have to resend the patches
anyway, though there are some other maintainers who like them - if in
doubt look at how patches for the subsystem are normally handled.
Stefan Agner Dec. 6, 2017, 2:25 p.m. UTC | #4
On 2017-12-06 15:11, Mark Brown wrote:
> On Wed, Dec 06, 2017 at 02:53:39PM +0100, Stefan Agner wrote:
>> On 2017-08-07 09:08, Nicolin Chen wrote:
>> > On Sun, Aug 06, 2017 at 11:36:57PM -0700, Stefan Agner wrote:
> 
>> Mark, you seem to have merged other patches for this driver, can you
>> merge this one too?
> 
> Please don't send content free pings and please allow a reasonable time
> for review.  People get busy, go on holiday, attend conferences and so 
> on so unless there is some reason for urgency (like critical bug fixes)
> please allow at least a couple of weeks for review.  If there have been
> review comments then people may be waiting for those to be addressed.

Well, it was not entirely content free since I added you/Liam to the
list of recipient which I missed in the initial patch. I probably should
have mentioned that.

I did not meant to bug people, just wanted to make sure the patch does
not falls through the cracks... After all, patch as well as the Ack
already more than 3 months old....

--
Stefan
Mark Brown Dec. 6, 2017, 2:39 p.m. UTC | #5
On Wed, Dec 06, 2017 at 03:25:37PM +0100, Stefan Agner wrote:
> On 2017-12-06 15:11, Mark Brown wrote:

> > Please don't send content free pings and please allow a reasonable time
> > for review.  People get busy, go on holiday, attend conferences and so 
> > on so unless there is some reason for urgency (like critical bug fixes)
> > please allow at least a couple of weeks for review.  If there have been
> > review comments then people may be waiting for those to be addressed.

> Well, it was not entirely content free since I added you/Liam to the
> list of recipient which I missed in the initial patch. I probably should
> have mentioned that.

Then it's definitely just a content free ping since I obviously don't
have the patch if you didn't send it to me and as the mail you're
replying to said you'll need to send me a copy.

> I did not meant to bug people, just wanted to make sure the patch does
> not falls through the cracks... After all, patch as well as the Ack
> already more than 3 months old....

You need to send patches to the relevant maintainers as covered in
SubmittingPatches, if you don't send us patches we won't have them and
can't apply them.
Stefan Agner Dec. 6, 2017, 3:03 p.m. UTC | #6
On 2017-12-06 15:39, Mark Brown wrote:
> On Wed, Dec 06, 2017 at 03:25:37PM +0100, Stefan Agner wrote:
>> On 2017-12-06 15:11, Mark Brown wrote:
> 
[...]
>> I did not meant to bug people, just wanted to make sure the patch does
>> not falls through the cracks... After all, patch as well as the Ack
>> already more than 3 months old....
> 
> You need to send patches to the relevant maintainers as covered in
> SubmittingPatches, if you don't send us patches we won't have them and
> can't apply them.

Yeah this is where it gets a bit fuzzy around the affected file, there
are maintainers mentioned in the MAINTAINER file but I just realized
today that it was typically you which merged patches for the file.

Will resend the patch and add you as recipient.

--
Stefan

Patch
diff mbox

diff --git a/sound/soc/fsl/fsl_asrc.h b/sound/soc/fsl/fsl_asrc.h
index 0f163abe4ba3..ec33dab4b909 100644
--- a/sound/soc/fsl/fsl_asrc.h
+++ b/sound/soc/fsl/fsl_asrc.h
@@ -57,7 +57,7 @@ 
 #define REG_ASRDOC			0x74
 #define REG_ASRDI(i)			(REG_ASRDIA + (i << 3))
 #define REG_ASRDO(i)			(REG_ASRDOA + (i << 3))
-#define REG_ASRDx(x, i)			(x == IN ? REG_ASRDI(i) : REG_ASRDO(i))
+#define REG_ASRDx(x, i)			((x) == IN ? REG_ASRDI(i) : REG_ASRDO(i))
 
 #define REG_ASRIDRHA			0x80
 #define REG_ASRIDRLA			0x84