Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 18 Apr 2023 14:40:20 -0600
From:   Alex Williamson <alex.williamson@redhat.com>
To:     "Gupta, Nipun" <Nipun.Gupta@amd.com>
Cc:     Jason Gunthorpe <jgg@ziepe.ca>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "masahiroy@kernel.org" <masahiroy@kernel.org>,
        "nathan@kernel.org" <nathan@kernel.org>,
        "ndesaulniers@google.com" <ndesaulniers@google.com>,
        "nicolas@fjasle.eu" <nicolas@fjasle.eu>,
        "git (AMD-Xilinx)" <git@amd.com>,
        "Anand, Harpreet" <harpreet.anand@amd.com>,
        "Jansen Van Vuuren, Pieter" <pieter.jansen-van-vuuren@amd.com>,
        "Agarwal, Nikhil" <nikhil.agarwal@amd.com>,
        "Simek, Michal" <michal.simek@amd.com>
Subject: Re: [PATCH v4] vfio/cdx: add support for CDX bus
Message-ID: <20230418144020.6b8368db.alex.williamson@redhat.com>
In-Reply-To: <CH3PR12MB8308DE1607789063D04B3529E89D9@CH3PR12MB8308.namprd12.prod.outlook.com>
References: <20230418113655.25207-1-nipun.gupta@amd.com>
        <ZD6IiHjWQOv47ZMg@ziepe.ca>
        <CH3PR12MB8308DE1607789063D04B3529E89D9@CH3PR12MB8308.namprd12.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Tue, 18 Apr 2023 12:50:13 +0000
"Gupta, Nipun" <Nipun.Gupta@amd.com> wrote:

> > -----Original Message-----
> > From: Jason Gunthorpe <jgg@ziepe.ca>
> > Sent: Tuesday, April 18, 2023 5:40 PM
> > To: Gupta, Nipun <Nipun.Gupta@amd.com>
> > Cc: alex.williamson@redhat.com; linux-kernel@vger.kernel.org;
> > kvm@vger.kernel.org; masahiroy@kernel.org; nathan@kernel.org;
> > ndesaulniers@google.com; nicolas@fjasle.eu; git (AMD-Xilinx) <git@amd.com>;
> > Anand, Harpreet <harpreet.anand@amd.com>; Jansen Van Vuuren, Pieter
> > <pieter.jansen-van-vuuren@amd.com>; Agarwal, Nikhil
> > <nikhil.agarwal@amd.com>; Simek, Michal <michal.simek@amd.com>
> > Subject: Re: [PATCH v4] vfio/cdx: add support for CDX bus
> > 
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> > 
> > 
> > On Tue, Apr 18, 2023 at 05:06:55PM +0530, Nipun Gupta wrote:
> >   
> > > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> > > index 89e06c981e43..aba36f5be4ec 100644
> > > --- a/drivers/vfio/Kconfig
> > > +++ b/drivers/vfio/Kconfig
> > > @@ -57,6 +57,7 @@ source "drivers/vfio/pci/Kconfig"
> > >  source "drivers/vfio/platform/Kconfig"
> > >  source "drivers/vfio/mdev/Kconfig"
> > >  source "drivers/vfio/fsl-mc/Kconfig"
> > > +source "drivers/vfio/cdx/Kconfig"  
> > 
> > keep sorted  
> 
> Since it is not sorted as of now, should a separate patch to be created for
> sorting, before adding vfio-cdx?

These are essentially in chronological order rather than alphabetical,
so I don't really understand this request from Jason.  Perhaps if it
was already alphabetical the request would be justified, but I don't
see any obligation here.

> > >  endif
> > >
> > >  source "virt/lib/Kconfig"
> > > diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
> > > index 70e7dcb302ef..1a27b2516612 100644
> > > --- a/drivers/vfio/Makefile
> > > +++ b/drivers/vfio/Makefile
> > > @@ -14,3 +14,4 @@ obj-$(CONFIG_VFIO_PCI) += pci/
> > >  obj-$(CONFIG_VFIO_PLATFORM) += platform/
> > >  obj-$(CONFIG_VFIO_MDEV) += mdev/
> > >  obj-$(CONFIG_VFIO_FSL_MC) += fsl-mc/
> > > +obj-$(CONFIG_VFIO_CDX) += cdx/  
> > 
> > keep sorted  
> 
> Is there Linux guideline here on how objects and folders in Makefile are sorted,
> as there are mix and match of files and folders in "drivers/vfio/Makefile".
> I could not find any reference for sorting in other Makefiles as well.

Same here, and I also don't know of a best practices reference that
suggests an alphabetical ordering policy.

> > > diff --git a/drivers/vfio/cdx/Makefile b/drivers/vfio/cdx/Makefile
> > > new file mode 100644
> > > index 000000000000..82e4ef412c0f
> > > --- /dev/null
> > > +++ b/drivers/vfio/cdx/Makefile
> > > @@ -0,0 +1,8 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +#
> > > +# Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
> > > +#
> > > +
> > > +obj-$(CONFIG_VFIO_CDX) += vfio-cdx.o
> > > +
> > > +vfio-cdx-objs := vfio_cdx.o  
> > 
> > Linus has asked about "suttering" filenames before, suggest to call
> > this
> > 
> > "vfio/cdx/main.c"  
> 
> Okay, I do not any strong affiliation towards the name.
> Alex, your thoughts on this please?

I think Jason means "stuttering" file paths, or essentially self
redundancy, ex. vfio/cdx/vfio_cdx.c.  Yes, Linus has scolded us for
vfio/pci/vfio_pci_* naming in the past, so in the spirit of not being
further scolded I'd agree with the suggestion here.  Thanks,

Alex