Re: [PATCH v3 2/2] xen/device_tree: remove incorrect and unused dt_irq() and dt_irq_flags() macros

[Xenia Ragiadakou <burzalodowa@xxxxxxxxx>]

Hi Luca

On 2/6/23 16:42, Luca Fancellu wrote:
> > On 3 Feb 2023, at 19:09, Xenia Ragiadakou <burzalodowa@xxxxxxxxx> 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@xxxxxxxxx>
> > ---
> >
> > 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@xxxxxxx>
--
Xenia