lkml.org 
[lkml]   [2021]   [Oct]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [EXTERNAL] Re: [PATCH v2 1/2] PCI: hv: Make the code arch neutral
Date
On Friday, October 8, 2021 12:17 PM
Bjorn Helgaas <helgaas@kernel.org> 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.

> > 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&amp;data=04%7C01%7Csunilmut%40microsoft.com%7Cb40962bf1
> 5614957781608d98a9030c8%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C
> 0%7C637693174187863024%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sda
> ta=XlrM%2Bzg92v%2BXxXObWQbznaB%2F6ZW6u79NtDoG4FfyhXw%3D&amp;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.

> > +++ 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.

> > +/*
> > + * Hyper-V vPCI irqchip.
>
> > +++ 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

\
 
 \ /
  Last update: 2021-10-08 21:57    [W:0.064 / U:0.504 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site