From: Michael Ellerman <mpe@ellerman.id.au>
To: Gautam Menghani <Gautam.Menghani@linux.ibm.com>
Cc: Gautam Menghani <gautam@linux.ibm.com>,
linuxppc-dev@lists.ozlabs.org, npiggin@gmail.com,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS
Date: Sat, 29 Jul 2023 20:54:26 +1000 [thread overview]
Message-ID: <873517dm3h.fsf@mail.lhotse> (raw)
In-Reply-To: <jcxd7uvooawd3vadvvzmegv27owaqvrxsi66c2ds6hwraqs3zy@3ymyrpyp3q55>
Gautam Menghani <Gautam.Menghani@linux.ibm.com> writes:
> On Thu, Jul 06, 2023 at 05:50:57PM +1000, Jordan Niethe wrote:
>> On 30/6/23 3:56 pm, Gautam Menghani wrote:
>> > Remove an unnecessary piece of code that does an endianness conversion but
>> > does not use the result. The following warning was reported by Clang's
>> > static analyzer:
>> >
>> > arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
>> > 'server' is never read [deadcode.DeadStores]
>> > server = be16_to_cpu(oserver);
>> >
>> > As the result of endianness conversion is never used, delete the line
>> > and fix the warning.
>> >
>> > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
>>
>> 'server' was used as a parameter to opal_get_xive() in commit 5c7c1e9444d8
>> ("powerpc/powernv: Add OPAL ICS backend") when it was introduced. 'server'
>> was also used in an error message for the call to opal_get_xive().
>>
>> 'server' was always later set by a call to ics_opal_mangle_server() before
>> being used.
>>
>> Commit bf8e0f891a32 ("powerpc/powernv: Fix endian issues in OPAL ICS
>> backend") used a new variable 'oserver' as the parameter to opal_get_xive()
>> instead of 'server' for endian correctness. It also removed 'server' from
>> the error message for the call to opal_get_xive().
>>
>> It was commit bf8e0f891a32 that added the unnecessary conversion and never
>> used the result.
>>
>> Reviewed-by: Jordan Niethe <jniethe5@gmail.com>
>>
>
> Hello Michael,
>
> Do you have any more questions on this patch or is it good to go?
I was expecting you'd send a v2 with the changelog updated to include
all the extra detail Jordan added. I think it should also be tagged as
Fixes: bf8e0f891a32.
cheers
WARNING: multiple messages have this Message-ID (diff)
From: Michael Ellerman <mpe@ellerman.id.au>
To: Gautam Menghani <Gautam.Menghani@linux.ibm.com>
Cc: Gautam Menghani <gautam@linux.ibm.com>,
npiggin@gmail.com, christophe.leroy@csgroup.eu,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS
Date: Sat, 29 Jul 2023 20:54:26 +1000 [thread overview]
Message-ID: <873517dm3h.fsf@mail.lhotse> (raw)
In-Reply-To: <jcxd7uvooawd3vadvvzmegv27owaqvrxsi66c2ds6hwraqs3zy@3ymyrpyp3q55>
Gautam Menghani <Gautam.Menghani@linux.ibm.com> writes:
> On Thu, Jul 06, 2023 at 05:50:57PM +1000, Jordan Niethe wrote:
>> On 30/6/23 3:56 pm, Gautam Menghani wrote:
>> > Remove an unnecessary piece of code that does an endianness conversion but
>> > does not use the result. The following warning was reported by Clang's
>> > static analyzer:
>> >
>> > arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
>> > 'server' is never read [deadcode.DeadStores]
>> > server = be16_to_cpu(oserver);
>> >
>> > As the result of endianness conversion is never used, delete the line
>> > and fix the warning.
>> >
>> > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
>>
>> 'server' was used as a parameter to opal_get_xive() in commit 5c7c1e9444d8
>> ("powerpc/powernv: Add OPAL ICS backend") when it was introduced. 'server'
>> was also used in an error message for the call to opal_get_xive().
>>
>> 'server' was always later set by a call to ics_opal_mangle_server() before
>> being used.
>>
>> Commit bf8e0f891a32 ("powerpc/powernv: Fix endian issues in OPAL ICS
>> backend") used a new variable 'oserver' as the parameter to opal_get_xive()
>> instead of 'server' for endian correctness. It also removed 'server' from
>> the error message for the call to opal_get_xive().
>>
>> It was commit bf8e0f891a32 that added the unnecessary conversion and never
>> used the result.
>>
>> Reviewed-by: Jordan Niethe <jniethe5@gmail.com>
>>
>
> Hello Michael,
>
> Do you have any more questions on this patch or is it good to go?
I was expecting you'd send a v2 with the changelog updated to include
all the extra detail Jordan added. I think it should also be tagged as
Fixes: bf8e0f891a32.
cheers
next prev parent reply other threads:[~2023-07-29 10:55 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-30 5:56 [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS Gautam Menghani
2023-07-03 6:41 ` Michael Ellerman
2023-07-06 7:50 ` Jordan Niethe
2023-07-11 9:33 ` Gautam Menghani
2023-07-11 9:33 ` Gautam Menghani
2023-07-26 8:07 ` Gautam Menghani
2023-07-26 8:07 ` Gautam Menghani
2023-07-26 9:12 ` Gautam Menghani
2023-07-26 9:12 ` Gautam Menghani
2023-07-29 10:54 ` Michael Ellerman [this message]
2023-07-29 10:54 ` Michael Ellerman
2023-07-31 12:00 ` Gautam Menghani
2023-07-31 12:00 ` Gautam Menghani
-- strict thread matches above, loose matches on Subject: below --
2023-06-30 6:13 Gautam Menghani
2023-06-30 6:16 ` Gautam Menghani
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=873517dm3h.fsf@mail.lhotse \
--to=mpe@ellerman.id.au \
--cc=Gautam.Menghani@linux.ibm.com \
--cc=gautam@linux.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=npiggin@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.