On Friday, October 8, 2021 12:17 PM
Bjorn Helgaas [off-list ref] wrote:

Can you put some specifics in the subject line, please?  If this patch
does several things, that might be an indication that it should be
split into several patches.

Will do in v3. But, this patch only makes the current Hyper-V vPCI code
architectural neutral by adding those interfaces that you mention below.
I will update the wording to make this clear in v3. thanks for the suggestion.

quoted

This patch makes the Hyper-V vPCI code architectural neutral by
introducing an irqchip that takes care of architectural
dependencies. This allows for the implementation of Hyper-V vPCI
for other architecture such as ARM64.

No need to include "This patch"; we already know we're talking about
this patch.

Write in "imperative mood", e.g., "Encapsulate arch dependencies in
X ..." instead of "This patch makes the code ...".  See
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fchris.be
ams.io%2Fposts%2Fgit-
commit%2F&data=04%7C01%7Csunilmut%40microsoft.com%7Cb40962bf1
5614957781608d98a9030c8%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C
0%7C637693174187863024%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sda
ta=XlrM%2Bzg92v%2BXxXObWQbznaB%2F6ZW6u79NtDoG4FfyhXw%3D&r
eserved=0

Wrap the text to fill 75 columns.

All of the above points noted; thanks. Will address in v3.

You said this "introduces an irqchip", but I don't see a new
struct irq_chip or similar.

The important part about making this arch-neutral seems to be adding
these interfaces that will encapsulate arch dependencies:

  hv_pci_irqchip_init()
  hv_pci_irqchip_free()
  hv_msi_get_int_vector()
  hv_set_msi_entry_from_desc()
  hv_msi_prepare()

Yes, the wording of 'irqchip' is a bit misleading for this patch as it gets
added in 2/2. Thanks for the suggestion; I will reword in v3.

I'm not sure wrapping them in "#ifdef CONFIG_X86_64" is the best
approach, but the IRQ folks will know better.

Hyper-V vPCI is not firmware enumerated and so we cannot take a
dependency on the firmware to dictate whether or not to instantiate
an irqchip or not. I am open to suggestions though.

quoted

+++ b/drivers/pci/controller/pci-hyperv-irqchip.c

@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0
+

Spurious blank line (follow style of nearby files).

Noted, thanks. Will fix in v3.

quoted

+/*
+ * Hyper-V vPCI irqchip.

quoted

+++ b/drivers/pci/controller/pci-hyperv-irqchip.h

@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+

Spurious blank line (follow style of nearby files).

Noted, thanks. Will fix in v3.

- Sunil