* [PATCH] xen/arm: Remove unused tnode variables in DT IRQ map parsing
@ 2016-12-06 13:57 Artem Mygaiev
2016-12-06 14:19 ` Julien Grall
0 siblings, 1 reply; 5+ messages in thread
From: Artem Mygaiev @ 2016-12-06 13:57 UTC (permalink / raw)
To: xen-devel@lists.xenproject.org; +Cc: Julien Grall, Stefano Stabellini
Remove unused tnode variables in device tree IRQ map parsing
Coverity-ID 1381866, 1381867
Signed-off-by: Artem Mygaiev <artem_mygaiev@epam.com>
---
xen/common/device_tree.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 1be074b..edc48f6 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -1053,7 +1053,7 @@ int dt_for_each_irq_map(const struct dt_device_node *dev,
void *),
void *data)
{
- const struct dt_device_node *ipar, *tnode, *old = NULL;
+ const struct dt_device_node *ipar, *old = NULL;
const __be32 *tmp, *imap;
u32 intsize = 1, addrsize, pintsize = 0, paddrsize = 0;
u32 imaplen;
@@ -1078,7 +1078,6 @@ int dt_for_each_irq_map(const struct dt_device_node *dev,
intsize = be32_to_cpu(*tmp);
break;
}
- tnode = ipar;
ipar = dt_irq_find_parent(ipar);
} while ( ipar );
if ( ipar == NULL )
@@ -1101,8 +1100,7 @@ int dt_for_each_irq_map(const struct dt_device_node *dev,
old = ipar;
do {
tmp = dt_get_property(old, "#address-cells", NULL);
- tnode = dt_get_parent(old);
- old = tnode;
+ old = dt_get_parent(old);
} while ( old && tmp == NULL );
old = NULL;
@@ -1238,7 +1236,7 @@ static int dt_irq_map_raw(const struct dt_device_node *parent,
const __be32 *addr,
struct dt_raw_irq *oirq)
{
- const struct dt_device_node *ipar, *tnode, *old = NULL, *newpar = NULL;
+ const struct dt_device_node *ipar, *old = NULL, *newpar = NULL;
const __be32 *tmp, *imap, *imask;
u32 intsize = 1, addrsize, newintsize = 0, newaddrsize = 0;
u32 imaplen;
@@ -1261,7 +1259,6 @@ static int dt_irq_map_raw(const struct dt_device_node *parent,
intsize = be32_to_cpu(*tmp);
break;
}
- tnode = ipar;
ipar = dt_irq_find_parent(ipar);
} while ( ipar );
if ( ipar == NULL )
@@ -1281,8 +1278,7 @@ static int dt_irq_map_raw(const struct dt_device_node *parent,
old = ipar;
do {
tmp = dt_get_property(old, "#address-cells", NULL);
- tnode = dt_get_parent(old);
- old = tnode;
+ old = dt_get_parent(old);
} while ( old && tmp == NULL );
old = NULL;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] xen/arm: Remove unused tnode variables in DT IRQ map parsing
2016-12-06 13:57 [PATCH] xen/arm: Remove unused tnode variables in DT IRQ map parsing Artem Mygaiev
@ 2016-12-06 14:19 ` Julien Grall
2016-12-06 14:33 ` Artem Mygaiev
0 siblings, 1 reply; 5+ messages in thread
From: Julien Grall @ 2016-12-06 14:19 UTC (permalink / raw)
To: Artem Mygaiev, xen-devel@lists.xenproject.org; +Cc: Stefano Stabellini
Hi Artem,
On 06/12/16 13:57, Artem Mygaiev wrote:
> Remove unused tnode variables in device tree IRQ map parsing
This description is slightly wrong, the variable are used but not the value.
> Coverity-ID 1381866, 1381867
>
> Signed-off-by: Artem Mygaiev <artem_mygaiev@epam.com>
This code is from Linux (see drivers/of/irq.c) and I would prefer to
keep it very similar as long as it does not lead to a bug:
Nacked-by: Julien Grall <julien.grall@arm.com>
> ---
> xen/common/device_tree.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 1be074b..edc48f6 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -1053,7 +1053,7 @@ int dt_for_each_irq_map(const struct dt_device_node *dev,
> void *),
> void *data)
> {
> - const struct dt_device_node *ipar, *tnode, *old = NULL;
> + const struct dt_device_node *ipar, *old = NULL;
> const __be32 *tmp, *imap;
> u32 intsize = 1, addrsize, pintsize = 0, paddrsize = 0;
> u32 imaplen;
> @@ -1078,7 +1078,6 @@ int dt_for_each_irq_map(const struct dt_device_node *dev,
> intsize = be32_to_cpu(*tmp);
> break;
> }
> - tnode = ipar;
> ipar = dt_irq_find_parent(ipar);
> } while ( ipar );
> if ( ipar == NULL )
> @@ -1101,8 +1100,7 @@ int dt_for_each_irq_map(const struct dt_device_node *dev,
> old = ipar;
> do {
> tmp = dt_get_property(old, "#address-cells", NULL);
> - tnode = dt_get_parent(old);
> - old = tnode;
> + old = dt_get_parent(old);
> } while ( old && tmp == NULL );
>
> old = NULL;
> @@ -1238,7 +1236,7 @@ static int dt_irq_map_raw(const struct dt_device_node *parent,
> const __be32 *addr,
> struct dt_raw_irq *oirq)
> {
> - const struct dt_device_node *ipar, *tnode, *old = NULL, *newpar = NULL;
> + const struct dt_device_node *ipar, *old = NULL, *newpar = NULL;
> const __be32 *tmp, *imap, *imask;
> u32 intsize = 1, addrsize, newintsize = 0, newaddrsize = 0;
> u32 imaplen;
> @@ -1261,7 +1259,6 @@ static int dt_irq_map_raw(const struct dt_device_node *parent,
> intsize = be32_to_cpu(*tmp);
> break;
> }
> - tnode = ipar;
> ipar = dt_irq_find_parent(ipar);
> } while ( ipar );
> if ( ipar == NULL )
> @@ -1281,8 +1278,7 @@ static int dt_irq_map_raw(const struct dt_device_node *parent,
> old = ipar;
> do {
> tmp = dt_get_property(old, "#address-cells", NULL);
> - tnode = dt_get_parent(old);
> - old = tnode;
> + old = dt_get_parent(old);
> } while ( old && tmp == NULL );
>
> old = NULL;
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] xen/arm: Remove unused tnode variables in DT IRQ map parsing
2016-12-06 14:19 ` Julien Grall
@ 2016-12-06 14:33 ` Artem Mygaiev
2016-12-06 14:37 ` Julien Grall
0 siblings, 1 reply; 5+ messages in thread
From: Artem Mygaiev @ 2016-12-06 14:33 UTC (permalink / raw)
To: Julien Grall, xen-devel@lists.xenproject.org; +Cc: Stefano Stabellini
Hi Julien
On 06.12.16 16:19, Julien Grall wrote:
> Hi Artem,
>
> On 06/12/16 13:57, Artem Mygaiev wrote:
>> Remove unused tnode variables in device tree IRQ map parsing
>
> This description is slightly wrong, the variable are used but not the value.
True
>
>> Coverity-ID 1381866, 1381867
>>
>> Signed-off-by: Artem Mygaiev <artem_mygaiev@epam.com>
>
> This code is from Linux (see drivers/of/irq.c) and I would prefer to keep it very similar as long as it does not lead to a bug:
I understand, but in the kernel code tnode is used to safely decrement kobj refcount (e.g. in 4.8.12):
144 tnode = of_get_parent(old);
145 of_node_put(old);
146 old = tnode;
AFAIU in Xen refcounts are dropped, so probably tnode can be dropped as a part of the infrastructure... no?
> Nacked-by: Julien Grall <julien.grall@arm.com>
>
>> ---
>> xen/common/device_tree.c | 12 ++++--------
>> 1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
>> index 1be074b..edc48f6 100644
>> --- a/xen/common/device_tree.c
>> +++ b/xen/common/device_tree.c
>> @@ -1053,7 +1053,7 @@ int dt_for_each_irq_map(const struct dt_device_node *dev,
>> void *),
>> void *data)
>> {
>> - const struct dt_device_node *ipar, *tnode, *old = NULL;
>> + const struct dt_device_node *ipar, *old = NULL;
>> const __be32 *tmp, *imap;
>> u32 intsize = 1, addrsize, pintsize = 0, paddrsize = 0;
>> u32 imaplen;
>> @@ -1078,7 +1078,6 @@ int dt_for_each_irq_map(const struct dt_device_node *dev,
>> intsize = be32_to_cpu(*tmp);
>> break;
>> }
>> - tnode = ipar;
>> ipar = dt_irq_find_parent(ipar);
>> } while ( ipar );
>> if ( ipar == NULL )
>> @@ -1101,8 +1100,7 @@ int dt_for_each_irq_map(const struct dt_device_node *dev,
>> old = ipar;
>> do {
>> tmp = dt_get_property(old, "#address-cells", NULL);
>> - tnode = dt_get_parent(old);
>> - old = tnode;
>> + old = dt_get_parent(old);
>> } while ( old && tmp == NULL );
>>
>> old = NULL;
>> @@ -1238,7 +1236,7 @@ static int dt_irq_map_raw(const struct dt_device_node *parent,
>> const __be32 *addr,
>> struct dt_raw_irq *oirq)
>> {
>> - const struct dt_device_node *ipar, *tnode, *old = NULL, *newpar = NULL;
>> + const struct dt_device_node *ipar, *old = NULL, *newpar = NULL;
>> const __be32 *tmp, *imap, *imask;
>> u32 intsize = 1, addrsize, newintsize = 0, newaddrsize = 0;
>> u32 imaplen;
>> @@ -1261,7 +1259,6 @@ static int dt_irq_map_raw(const struct dt_device_node *parent,
>> intsize = be32_to_cpu(*tmp);
>> break;
>> }
>> - tnode = ipar;
>> ipar = dt_irq_find_parent(ipar);
>> } while ( ipar );
>> if ( ipar == NULL )
>> @@ -1281,8 +1278,7 @@ static int dt_irq_map_raw(const struct dt_device_node *parent,
>> old = ipar;
>> do {
>> tmp = dt_get_property(old, "#address-cells", NULL);
>> - tnode = dt_get_parent(old);
>> - old = tnode;
>> + old = dt_get_parent(old);
>> } while ( old && tmp == NULL );
>>
>> old = NULL;
>>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] xen/arm: Remove unused tnode variables in DT IRQ map parsing
2016-12-06 14:33 ` Artem Mygaiev
@ 2016-12-06 14:37 ` Julien Grall
2016-12-06 14:41 ` Artem Mygaiev
0 siblings, 1 reply; 5+ messages in thread
From: Julien Grall @ 2016-12-06 14:37 UTC (permalink / raw)
To: Artem Mygaiev, xen-devel@lists.xenproject.org; +Cc: Stefano Stabellini
On 06/12/16 14:33, Artem Mygaiev wrote:
> Hi Julien
>
>
> On 06.12.16 16:19, Julien Grall wrote:
>> Hi Artem,
>>
>> On 06/12/16 13:57, Artem Mygaiev wrote:
>>> Remove unused tnode variables in device tree IRQ map parsing
>>
>> This description is slightly wrong, the variable are used but not the value.
> True
>>
>>> Coverity-ID 1381866, 1381867
>>>
>>> Signed-off-by: Artem Mygaiev <artem_mygaiev@epam.com>
>>
>> This code is from Linux (see drivers/of/irq.c) and I would prefer to keep it very similar as long as it does not lead to a bug:
> I understand, but in the kernel code tnode is used to safely decrement kobj refcount (e.g. in 4.8.12):
>
> 144 tnode = of_get_parent(old);
> 145 of_node_put(old);
> 146 old = tnode;
>
> AFAIU in Xen refcounts are dropped, so probably tnode can be dropped as a part of the infrastructure... no?
I have an item in my todo list to re-sync the device-tree code with
Linux. So we have a proper copy rather than a modified one.
Meanwhile, I would strongly prefer to keep this code as close as
possible from Linux. This tnode variable is harmless, so I stand on what
I said.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xen/arm: Remove unused tnode variables in DT IRQ map parsing
2016-12-06 14:37 ` Julien Grall
@ 2016-12-06 14:41 ` Artem Mygaiev
0 siblings, 0 replies; 5+ messages in thread
From: Artem Mygaiev @ 2016-12-06 14:41 UTC (permalink / raw)
To: Julien Grall, xen-devel@lists.xenproject.org; +Cc: Stefano Stabellini
On 06.12.16 16:37, Julien Grall wrote:
>
>
> On 06/12/16 14:33, Artem Mygaiev wrote:
>> Hi Julien
>>
>>
>> On 06.12.16 16:19, Julien Grall wrote:
>>> Hi Artem,
>>>
>>> On 06/12/16 13:57, Artem Mygaiev wrote:
>>>> Remove unused tnode variables in device tree IRQ map parsing
>>>
>>> This description is slightly wrong, the variable are used but not the value.
>> True
>>>
>>>> Coverity-ID 1381866, 1381867
>>>>
>>>> Signed-off-by: Artem Mygaiev <artem_mygaiev@epam.com>
>>>
>>> This code is from Linux (see drivers/of/irq.c) and I would prefer to keep it very similar as long as it does not lead to a bug:
>> I understand, but in the kernel code tnode is used to safely decrement kobj refcount (e.g. in 4.8.12):
>>
>> 144 tnode = of_get_parent(old);
>> 145 of_node_put(old);
>> 146 old = tnode;
>>
>> AFAIU in Xen refcounts are dropped, so probably tnode can be dropped as a part of the infrastructure... no?
>
> I have an item in my todo list to re-sync the device-tree code with Linux. So we have a proper copy rather than a modified one.
Oh, I see, thanks.
>
> Meanwhile, I would strongly prefer to keep this code as close as possible from Linux. This tnode variable is harmless, so I stand on what I said.
>
> Regards,
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-12-06 14:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-06 13:57 [PATCH] xen/arm: Remove unused tnode variables in DT IRQ map parsing Artem Mygaiev
2016-12-06 14:19 ` Julien Grall
2016-12-06 14:33 ` Artem Mygaiev
2016-12-06 14:37 ` Julien Grall
2016-12-06 14:41 ` Artem Mygaiev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).