Macro dt_irq() is broken because the macro parameter has the same name with
the struct dt_irq member "irq".
Macro dt_irq_flags() is broken as well because struct dt_irq has no member
named "flags".

Since no one seems to have ever tried to use them and eventually stumble upon
the issues above, remove them instead of fixing them.

Fixes: 886f34045bf0 ("xen/arm: Add helpers to retrieve an interrupt description from the device tree")
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---

Changes in v3:
  - new patch

 xen/include/xen/device_tree.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 7839a199e311..19a74909cece 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -159,9 +159,6 @@ struct dt_raw_irq {
     u32 specifier[DT_MAX_IRQ_SPEC];
 };
 
-#define dt_irq(irq) ((irq)->irq)
-#define dt_irq_flags(irq) ((irq)->flags)
-
 typedef int (*device_tree_node_func)(const void *fdt,
                                      int node, const char *name, int depth,
                                      u32 address_cells, u32 size_cells,
-- 
2.37.2

> On 3 Feb 2023, at 19:09, Xenia Ragiadakou <burzalodowa@gmail.com> wrote:
> 
> Macro dt_irq() is broken because the macro parameter has the same name with
> the struct dt_irq member "irq".

I’m not sure about the wording “broken”, it should work anyway or am I wrong?

> Macro dt_irq_flags() is broken as well because struct dt_irq has no member
> named "flags".

Yes this depends if the macro was meant to access the structure dt_irq, I’ve had a look
on the commit introducing it but I could not figure out the usage.

> 
> Since no one seems to have ever tried to use them and eventually stumble upon
> the issues above, remove them instead of fixing them.
> 
> Fixes: 886f34045bf0 ("xen/arm: Add helpers to retrieve an interrupt description from the device tree")
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
> ---
> 
> Changes in v3:
>  - new patch
> 
> xen/include/xen/device_tree.h | 3 ---
> 1 file changed, 3 deletions(-)
> 
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index 7839a199e311..19a74909cece 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -159,9 +159,6 @@ struct dt_raw_irq {
>     u32 specifier[DT_MAX_IRQ_SPEC];
> };
> 
> -#define dt_irq(irq) ((irq)->irq)
> -#define dt_irq_flags(irq) ((irq)->flags)
> -
> typedef int (*device_tree_node_func)(const void *fdt,
>                                      int node, const char *name, int depth,
>                                      u32 address_cells, u32 size_cells,
> -- 
> 2.37.2
> 
> 

They seems indeed unused, so for me the code looks good:

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

Hi Luca

On 2/6/23 16:42, Luca Fancellu wrote:
> 
> 
>> On 3 Feb 2023, at 19:09, Xenia Ragiadakou <burzalodowa@gmail.com> wrote:
>>
>> Macro dt_irq() is broken because the macro parameter has the same name with
>> the struct dt_irq member "irq".
> 
> I’m not sure about the wording “broken”, it should work anyway or am I wrong?

No, it won't work because the structure member will be substituted as 
well by the macro argument (for instance dt_irq(blah) will be replaced 
by (blah)->blah).

> 
>> Macro dt_irq_flags() is broken as well because struct dt_irq has no member
>> named "flags".
> 
> Yes this depends if the macro was meant to access the structure dt_irq, I’ve had a look
> on the commit introducing it but I could not figure out the usage.

Given the macro name, I assumed that it was meant to be used for 
accessing a dt_irq field.

> 
>>
>> Since no one seems to have ever tried to use them and eventually stumble upon
>> the issues above, remove them instead of fixing them.
>>
>> Fixes: 886f34045bf0 ("xen/arm: Add helpers to retrieve an interrupt description from the device tree")
>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
>> ---
>>
>> Changes in v3:
>>   - new patch
>>
>> xen/include/xen/device_tree.h | 3 ---
>> 1 file changed, 3 deletions(-)
>>
>> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
>> index 7839a199e311..19a74909cece 100644
>> --- a/xen/include/xen/device_tree.h
>> +++ b/xen/include/xen/device_tree.h
>> @@ -159,9 +159,6 @@ struct dt_raw_irq {
>>      u32 specifier[DT_MAX_IRQ_SPEC];
>> };
>>
>> -#define dt_irq(irq) ((irq)->irq)
>> -#define dt_irq_flags(irq) ((irq)->flags)
>> -
>> typedef int (*device_tree_node_func)(const void *fdt,
>>                                       int node, const char *name, int depth,
>>                                       u32 address_cells, u32 size_cells,
>> -- 
>> 2.37.2
>>
>>
> 
> They seems indeed unused, so for me the code looks good:
> 
> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
> 
> 

-- 
Xenia

> On 6 Feb 2023, at 14:51, Xenia Ragiadakou <burzalodowa@gmail.com> wrote:
> 
> Hi Luca
> 
> On 2/6/23 16:42, Luca Fancellu wrote:
>>> On 3 Feb 2023, at 19:09, Xenia Ragiadakou <burzalodowa@gmail.com> wrote:
>>> 
>>> Macro dt_irq() is broken because the macro parameter has the same name with
>>> the struct dt_irq member "irq".
>> I’m not sure about the wording “broken”, it should work anyway or am I wrong?
> 
> No, it won't work because the structure member will be substituted as well by the macro argument (for instance dt_irq(blah) will be replaced by (blah)->blah).

Yes you are right, I was reading it in the wrong way! 

> 
>>> Macro dt_irq_flags() is broken as well because struct dt_irq has no member
>>> named "flags".
>> Yes this depends if the macro was meant to access the structure dt_irq, I’ve had a look
>> on the commit introducing it but I could not figure out the usage.
> 
> Given the macro name, I assumed that it was meant to be used for accessing a dt_irq field.

Yes I would come to the same conclusion