RE: [EXTERNAL] Re: [PATCH v2 1/2] PCI: hv: Make the code arch neutral

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Friday, October 8, 2021 12:17 PM
Bjorn Helgaas <helgaas@xxxxxxxxxx> 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




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux