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 <agarc...@amd.com>

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.andr...@amd.com>

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

Reply via email to