Re: [PATCH 02/19] x86: Add missing pci_dev forward declaration in asm/pci.h

[Jason Andryuk <jason.andryuk@xxxxxxx>]

On 2025-06-02 09:45, Alejandro Vallejo wrote:
> On Fri May 30, 2025 at 11:04 PM CEST, Jason Andryuk wrote:
> > On 2025-05-30 08:02, Alejandro Vallejo wrote:
> > > Not a functional change.
> > >
> > > Signed-off-by: Alejandro Vallejo <agarciav@xxxxxxx>
> > Some sort of reason would be good in the commit message.
> >
> > "struct pci_dev is used in function prototypes within the header.  This
> > is in preparation for including (transitively) in device tree"?
> > ... I'm guessing that is why.  Stating  it would be better.
> Yes, but I'm not sure the second part of that explanation is relevant. Unless
> specifically noted in the header, they are meant to stand by themselves when
> possible and not require preinclusion of something else (in this case, 
> xen/pci.h).
>
> This patch would still be relevant (imo) even if I wasn't using the header for
> anything.
Yes, this could be made as a totally independent clean up, and that 
would be fine.  If this is a prerequisite for some later change, I think 
it is useful to highlight it.  It would not otherwise be evident when 
looking at the history.  Because I haven't gotten farther into the 
series, I don't know if this patch is an independent cleanup or a 
prerequisite.
> > With a suitable reason:
> >
> > Reviewed-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> Thanks,
>
> Does this sound reasonable?
>
>    struct pci_dev is used in function prototypes within the header, so it must
>    be forward declared for asm/pci.h not to depend on xen/pci.h being included
>    first.
Sure.  Or just:
struct pci_dev is used in function prototypes within the header, so this 
forward declaration makes it standalone.
My point is: we should explicitly state what the patch is trying to 
achieve.  It helps reviewers and future git history readers.
Regards,
Jason