* [PATCH 0/3] powerpc/kernel: Simplify rtas_initialize()
@ 2017-01-22 23:25 Gavin Shan
2017-01-22 23:25 ` [PATCH 1/3] powerpc/kernel: Remove nested if statements in rtas_initialize() Gavin Shan
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Gavin Shan @ 2017-01-22 23:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mpe, Gavin Shan
This simplifies rtas_initialize(), no functional changes introduced:
* PATCH[1/3] removes the unnecessary nested if statements.
* PATCH[2/3] uses of_property_read_u32() instead of converting the
property's data to target in CPU endian.
* PATCH[3/3] decreases RTAS device-tree node's refcount in error path.
Gavin Shan (3):
powerpc/kernel: Remove nested if statements in rtas_initialize()
powerpc/kernel: Use of_property_read_u32() in rtas_initialize()
powerpc/kernel: Fix unbalanced refcount on RTAS device node
arch/powerpc/kernel/rtas.c | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] powerpc/kernel: Remove nested if statements in rtas_initialize()
2017-01-22 23:25 [PATCH 0/3] powerpc/kernel: Simplify rtas_initialize() Gavin Shan
@ 2017-01-22 23:25 ` Gavin Shan
2017-01-23 9:08 ` Michael Ellerman
2017-01-22 23:25 ` [PATCH 2/3] powerpc/kernel: Use of_property_read_u32() " Gavin Shan
2017-01-22 23:25 ` [PATCH 3/3] powerpc/kernel: Fix unbalanced refcount on RTAS device node Gavin Shan
2 siblings, 1 reply; 6+ messages in thread
From: Gavin Shan @ 2017-01-22 23:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mpe, Gavin Shan
This removes the unnecessary nested if statements in function
rtas_initialize(), to simplify the code. No functional changes
introduced.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
arch/powerpc/kernel/rtas.c | 33 ++++++++++++++++-----------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 112cc3b..9ba0f67 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -1145,31 +1145,30 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs)
void __init rtas_initialize(void)
{
unsigned long rtas_region = RTAS_INSTANTIATE_MAX;
+ const __be32 *basep, *entryp, *sizep;
/* Get RTAS dev node and fill up our "rtas" structure with infos
* about it.
*/
rtas.dev = of_find_node_by_name(NULL, "rtas");
- if (rtas.dev) {
- const __be32 *basep, *entryp, *sizep;
-
- basep = of_get_property(rtas.dev, "linux,rtas-base", NULL);
- sizep = of_get_property(rtas.dev, "rtas-size", NULL);
- if (basep != NULL && sizep != NULL) {
- rtas.base = __be32_to_cpu(*basep);
- rtas.size = __be32_to_cpu(*sizep);
- entryp = of_get_property(rtas.dev,
- "linux,rtas-entry", NULL);
- if (entryp == NULL) /* Ugh */
- rtas.entry = rtas.base;
- else
- rtas.entry = __be32_to_cpu(*entryp);
- } else
- rtas.dev = NULL;
- }
if (!rtas.dev)
return;
+ basep = of_get_property(rtas.dev, "linux,rtas-base", NULL);
+ sizep = of_get_property(rtas.dev, "rtas-size", NULL);
+ if (basep == NULL && sizep == NULL) {
+ rtas.dev = NULL;
+ return;
+ }
+
+ rtas.base = __be32_to_cpu(*basep);
+ rtas.size = __be32_to_cpu(*sizep);
+ entryp = of_get_property(rtas.dev, "linux,rtas-entry", NULL);
+ if (entryp == NULL) /* Ugh */
+ rtas.entry = rtas.base;
+ else
+ rtas.entry = __be32_to_cpu(*entryp);
+
/* If RTAS was found, allocate the RMO buffer for it and look for
* the stop-self token if any
*/
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] powerpc/kernel: Use of_property_read_u32() in rtas_initialize()
2017-01-22 23:25 [PATCH 0/3] powerpc/kernel: Simplify rtas_initialize() Gavin Shan
2017-01-22 23:25 ` [PATCH 1/3] powerpc/kernel: Remove nested if statements in rtas_initialize() Gavin Shan
@ 2017-01-22 23:25 ` Gavin Shan
2017-01-22 23:25 ` [PATCH 3/3] powerpc/kernel: Fix unbalanced refcount on RTAS device node Gavin Shan
2 siblings, 0 replies; 6+ messages in thread
From: Gavin Shan @ 2017-01-22 23:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mpe, Gavin Shan
This uses of_property_read_u32() in rtas_initialize() so that we
needn't explicitly care the CPU's endian.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
arch/powerpc/kernel/rtas.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 9ba0f67..d13b92d 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -1145,7 +1145,8 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs)
void __init rtas_initialize(void)
{
unsigned long rtas_region = RTAS_INSTANTIATE_MAX;
- const __be32 *basep, *entryp, *sizep;
+ u32 base, size, entry;
+ int no_base, no_size, no_entry;
/* Get RTAS dev node and fill up our "rtas" structure with infos
* about it.
@@ -1154,20 +1155,17 @@ void __init rtas_initialize(void)
if (!rtas.dev)
return;
- basep = of_get_property(rtas.dev, "linux,rtas-base", NULL);
- sizep = of_get_property(rtas.dev, "rtas-size", NULL);
- if (basep == NULL && sizep == NULL) {
+ no_base = of_property_read_u32(rtas.dev, "linux,rtas-base", &base);
+ no_size = of_property_read_u32(rtas.dev, "rtas-size", &size);
+ if (no_base && no_size) {
rtas.dev = NULL;
return;
}
- rtas.base = __be32_to_cpu(*basep);
- rtas.size = __be32_to_cpu(*sizep);
- entryp = of_get_property(rtas.dev, "linux,rtas-entry", NULL);
- if (entryp == NULL) /* Ugh */
- rtas.entry = rtas.base;
- else
- rtas.entry = __be32_to_cpu(*entryp);
+ rtas.base = base;
+ rtas.size = size;
+ no_entry = of_property_read_u32(rtas.dev, "linux,rtas-entry", &entry);
+ rtas.entry = no_entry ? rtas.base : entry;
/* If RTAS was found, allocate the RMO buffer for it and look for
* the stop-self token if any
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] powerpc/kernel: Fix unbalanced refcount on RTAS device node
2017-01-22 23:25 [PATCH 0/3] powerpc/kernel: Simplify rtas_initialize() Gavin Shan
2017-01-22 23:25 ` [PATCH 1/3] powerpc/kernel: Remove nested if statements in rtas_initialize() Gavin Shan
2017-01-22 23:25 ` [PATCH 2/3] powerpc/kernel: Use of_property_read_u32() " Gavin Shan
@ 2017-01-22 23:25 ` Gavin Shan
2 siblings, 0 replies; 6+ messages in thread
From: Gavin Shan @ 2017-01-22 23:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mpe, Gavin Shan
The RTAS device-tree node's refcount has been increased by one in
the function call of_find_node_by_name(), but it's missed to be
decreased by one in the error path. It leads to unbalanced refcount
on RTAS device-tree node.
This fixes above issue by decreasing RTAS device-tree node's refcount
in error path.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
arch/powerpc/kernel/rtas.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index d13b92d..0823fe3 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -1158,6 +1158,7 @@ void __init rtas_initialize(void)
no_base = of_property_read_u32(rtas.dev, "linux,rtas-base", &base);
no_size = of_property_read_u32(rtas.dev, "rtas-size", &size);
if (no_base && no_size) {
+ of_node_put(rtas.dev);
rtas.dev = NULL;
return;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] powerpc/kernel: Remove nested if statements in rtas_initialize()
2017-01-22 23:25 ` [PATCH 1/3] powerpc/kernel: Remove nested if statements in rtas_initialize() Gavin Shan
@ 2017-01-23 9:08 ` Michael Ellerman
2017-01-23 22:26 ` Gavin Shan
0 siblings, 1 reply; 6+ messages in thread
From: Michael Ellerman @ 2017-01-23 9:08 UTC (permalink / raw)
To: Gavin Shan, linuxppc-dev; +Cc: Gavin Shan
Gavin Shan <gwshan@linux.vnet.ibm.com> writes:
> This removes the unnecessary nested if statements in function
> rtas_initialize(), to simplify the code. No functional changes
> introduced.
>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
> arch/powerpc/kernel/rtas.c | 33 ++++++++++++++++-----------------
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 112cc3b..9ba0f67 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -1145,31 +1145,30 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs)
> void __init rtas_initialize(void)
> {
> unsigned long rtas_region = RTAS_INSTANTIATE_MAX;
> + const __be32 *basep, *entryp, *sizep;
>
> /* Get RTAS dev node and fill up our "rtas" structure with infos
> * about it.
> */
> rtas.dev = of_find_node_by_name(NULL, "rtas");
> - if (rtas.dev) {
> - const __be32 *basep, *entryp, *sizep;
> -
> - basep = of_get_property(rtas.dev, "linux,rtas-base", NULL);
> - sizep = of_get_property(rtas.dev, "rtas-size", NULL);
> - if (basep != NULL && sizep != NULL) {
...
> - } else
Previously we set rtas.dev to NULL if either basep or sizep was NULL.
> - rtas.dev = NULL;
> - }
> if (!rtas.dev)
> return;
>
> + basep = of_get_property(rtas.dev, "linux,rtas-base", NULL);
> + sizep = of_get_property(rtas.dev, "rtas-size", NULL);
> + if (basep == NULL && sizep == NULL) {
But now you set it to NULL only if BOTH basep and sizep are NULL.
Was that intentional? If so you need to mention it in the change log.
> + rtas.dev = NULL;
> + return;
> + }
The proper negation of:
if (basep != NULL && sizep != NULL) {
is:
if (basep == NULL || sizep == NULL) {
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] powerpc/kernel: Remove nested if statements in rtas_initialize()
2017-01-23 9:08 ` Michael Ellerman
@ 2017-01-23 22:26 ` Gavin Shan
0 siblings, 0 replies; 6+ messages in thread
From: Gavin Shan @ 2017-01-23 22:26 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Gavin Shan, linuxppc-dev
On Mon, Jan 23, 2017 at 08:08:20PM +1100, Michael Ellerman wrote:
>Gavin Shan <gwshan@linux.vnet.ibm.com> writes:
>
>> This removes the unnecessary nested if statements in function
>> rtas_initialize(), to simplify the code. No functional changes
>> introduced.
>>
>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/kernel/rtas.c | 33 ++++++++++++++++-----------------
>> 1 file changed, 16 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
>> index 112cc3b..9ba0f67 100644
>> --- a/arch/powerpc/kernel/rtas.c
>> +++ b/arch/powerpc/kernel/rtas.c
>> @@ -1145,31 +1145,30 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs)
>> void __init rtas_initialize(void)
>> {
>> unsigned long rtas_region = RTAS_INSTANTIATE_MAX;
>> + const __be32 *basep, *entryp, *sizep;
>>
>> /* Get RTAS dev node and fill up our "rtas" structure with infos
>> * about it.
>> */
>> rtas.dev = of_find_node_by_name(NULL, "rtas");
>> - if (rtas.dev) {
>> - const __be32 *basep, *entryp, *sizep;
>> -
>> - basep = of_get_property(rtas.dev, "linux,rtas-base", NULL);
>> - sizep = of_get_property(rtas.dev, "rtas-size", NULL);
>> - if (basep != NULL && sizep != NULL) {
> ...
>> - } else
>
>Previously we set rtas.dev to NULL if either basep or sizep was NULL.
>
>> - rtas.dev = NULL;
>> - }
>> if (!rtas.dev)
>> return;
>>
>> + basep = of_get_property(rtas.dev, "linux,rtas-base", NULL);
>> + sizep = of_get_property(rtas.dev, "rtas-size", NULL);
>> + if (basep == NULL && sizep == NULL) {
>
>But now you set it to NULL only if BOTH basep and sizep are NULL.
>
>Was that intentional? If so you need to mention it in the change log.
>
>> + rtas.dev = NULL;
>> + return;
>> + }
>
>The proper negation of:
>
> if (basep != NULL && sizep != NULL) {
>is:
> if (basep == NULL || sizep == NULL) {
>
Thanks, Michael. It's not intentional. I'll update in v2.
Thanks,
Gavin
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-01-23 22:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-22 23:25 [PATCH 0/3] powerpc/kernel: Simplify rtas_initialize() Gavin Shan
2017-01-22 23:25 ` [PATCH 1/3] powerpc/kernel: Remove nested if statements in rtas_initialize() Gavin Shan
2017-01-23 9:08 ` Michael Ellerman
2017-01-23 22:26 ` Gavin Shan
2017-01-22 23:25 ` [PATCH 2/3] powerpc/kernel: Use of_property_read_u32() " Gavin Shan
2017-01-22 23:25 ` [PATCH 3/3] powerpc/kernel: Fix unbalanced refcount on RTAS device node Gavin Shan
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.