Re: [RFC PATCH v2 2/6] bus/cdx: add the cdx bus driver

From: Robin Murphy <robin.murphy@arm.com>
To: "Gupta, Nipun" <Nipun.Gupta@amd.com>, Jason Gunthorpe <jgg@nvidia.com>
Cc: Saravana Kannan <saravanak@google.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"krzysztof.kozlowski+dt@linaro.org" 
	<krzysztof.kozlowski+dt@linaro.org>,
	"rafael@kernel.org" <rafael@kernel.org>,
	"eric.auger@redhat.com" <eric.auger@redhat.com>,
	"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
	"cohuck@redhat.com" <cohuck@redhat.com>,
	"Gupta, Puneet (DCG-ENG)" <puneet.gupta@amd.com>,
	"song.bao.hua@hisilicon.com" <song.bao.hua@hisilicon.com>,
	"mchehab+huawei@kernel.org" <mchehab+huawei@kernel.org>,
	"maz@kernel.org" <maz@kernel.org>,
	"f.fainelli@gmail.com" <f.fainelli@gmail.com>,
	"jeffrey.l.hugo@gmail.com" <jeffrey.l.hugo@gmail.com>,
	"Michael.Srba@seznam.cz" <Michael.Srba@seznam.cz>,
	"mani@kernel.org" <mani@kernel.org>,
	"yishaih@nvidia.com" <yishaih@nvidia.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"okaya@kernel.org" <okaya@kernel.org>,
	"Anand, Harpreet" <harpreet.anand@amd.com>,
	"Agarwal, Nikhil" <nikhil.agarwal@amd.com>,
	"Simek, Michal" <michal.simek@amd.com>,
	"git (AMD-Xilinx)" <git@amd.com>
Subject: Re: [RFC PATCH v2 2/6] bus/cdx: add the cdx bus driver
Date: Tue, 30 Aug 2022 12:25:49 +0100	[thread overview]
Message-ID: <2db3b103-7034-6c0d-4ec8-caae7654b264@arm.com> (raw)
In-Reply-To: <MN2PR12MB30870CE2759A9ABE652FAFD8E8799@MN2PR12MB3087.namprd12.prod.outlook.com>

On 2022-08-30 08:06, Gupta, Nipun wrote:
> [AMD Official Use Only - General]
> 
> 
> 
>> -----Original Message-----
>> From: Jason Gunthorpe <jgg@nvidia.com>
>> Sent: Monday, August 29, 2022 9:02 PM
>> To: Gupta, Nipun <Nipun.Gupta@amd.com>
>> Cc: Robin Murphy <robin.murphy@arm.com>; Saravana Kannan
>> <saravanak@google.com>; Greg KH <gregkh@linuxfoundation.org>;
>> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; rafael@kernel.org;
>> eric.auger@redhat.com; alex.williamson@redhat.com; cohuck@redhat.com;
>> Gupta, Puneet (DCG-ENG) <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;
>> Michael.Srba@seznam.cz; mani@kernel.org; yishaih@nvidia.com; linux-
>> kernel@vger.kernel.org; devicetree@vger.kernel.org; kvm@vger.kernel.org;
>> okaya@kernel.org; Anand, Harpreet <harpreet.anand@amd.com>; Agarwal,
>> Nikhil <nikhil.agarwal@amd.com>; Simek, Michal <michal.simek@amd.com>;
>> git (AMD-Xilinx) <git@amd.com>
>> Subject: Re: [RFC PATCH v2 2/6] bus/cdx: add the cdx bus driver
>>
>> [CAUTION: External Email]
>>
>> On Mon, Aug 29, 2022 at 04:49:02AM +0000, Gupta, Nipun wrote:
>>
>>> Devices are created in FPFGA with a CDX wrapper, and CDX
>> controller(firmware)
>>> reads that CDX wrapper to find out new devices. Host driver then interacts
>> with
>>> firmware to find newly discovered devices. This bus aligns with PCI
>> infrastructure.
>>> It happens to be an embedded interface as opposed to off-chip
>> connection.
>>
>> Why do you need an FW in all of this?
>>
>> And why do you need DT at all?
> 
> We need DT to describe the CDX controller only, similar to
> how PCI controller is described in DT. PCI devices are
> never enumerated in DT. All children are to be dynamically
> discovered.
> 
> Children devices do not require DT as they will be discovered
> by the bus driver.
> 
> Like PCI controller talks to PCI device over PCI spec defined channel,
> we need CDX controller to talk to CDX device over a custom
> defined (FW managed) channel.

OK, thanks for clarifying - it actually sounds quite cool :)

I think it's clear now that this should be a a full-fledged bus 
implementation. Note that if the CDX interface provides a way to query 
arbitrary properties beyond standard resources then you may well also 
want your own fwnode type to hook into the device_property APIs too. 
Yes, it then means a bit more work adapting individual drivers too, but 
that should be far cleaner in the long run, and there's already plenty 
of precedent for IPs which exist with multiple standard interfaces for 
PCI/USB/SDIO/platform MMIO/etc.

Plus it means that if CDX ever makes its way into PCIe-attached FPGA 
cards which can be used on non-OF systems, you've not painted yourself 
into a corner.

Thanks,
Robin.