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:   Wed, 7 Sep 2022 14:32:56 +0200
From:   Greg KH <gregkh@linuxfoundation.org>
To:     Nipun Gupta <nipun.gupta@amd.com>
Cc:     robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
        rafael@kernel.org, eric.auger@redhat.com,
        alex.williamson@redhat.com, cohuck@redhat.com,
        puneet.gupta@amd.com, song.bao.hua@hisilicon.com,
        mchehab+huawei@kernel.org, maz@kernel.org, f.fainelli@gmail.com,
        jeffrey.l.hugo@gmail.com, saravanak@google.com,
        Michael.Srba@seznam.cz, mani@kernel.org, yishaih@nvidia.com,
        jgg@ziepe.ca, jgg@nvidia.com, robin.murphy@arm.com,
        will@kernel.org, joro@8bytes.org, masahiroy@kernel.org,
        ndesaulniers@google.com, linux-arm-kernel@lists.infradead.org,
        linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org, kvm@vger.kernel.org, okaya@kernel.org,
        harpreet.anand@amd.com, nikhil.agarwal@amd.com,
        michal.simek@amd.com, aleksandar.radovanovic@amd.com, git@amd.com
Subject: Re: [RFC PATCH v3 2/7] bus/cdx: add the cdx bus driver
Message-ID: <YxiPeOwo5tWrQuSE@kroah.com>
References: <20220803122655.100254-1-nipun.gupta@amd.com>
 <20220906134801.4079497-1-nipun.gupta@amd.com>
 <20220906134801.4079497-3-nipun.gupta@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220906134801.4079497-3-nipun.gupta@amd.com>
Precedence: bulk

On Tue, Sep 06, 2022 at 07:17:56PM +0530, Nipun Gupta wrote:
> CDX bus supports the scanning and probing of FPGA based
> devices. These devices are registers as CDX devices.
> 
> The bus driver sets up the basic infrastructure and fetches
> the device related information from the firmware.
> 
> CDX bus is capable of scanning devices dynamically,
> supporting rescanning of dynamically added, removed or
> updated devices.

Really?  Then why is the platform driver mess still in here?


> --- /dev/null
> +++ b/drivers/bus/cdx/cdx.c
> @@ -0,0 +1,437 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Platform driver for CDX bus.

Again, this should not be a platform driver.

Now you can have a CDX "bus" driver, that is a platform driver, but that
needs to be in a separate file and as a separate module and totally
independant of the CDX bus code entirely.

Otherwise this is a mess to try to sift through and determine what is,
and is not, going on.  Please split that up and get rid of all of the
platform driver stuff here and put it in a separate patch that happens
after the CDX bus logic is added.

thanks,

greg k-h