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, 25 Jul 2023 15:59:55 -0500
From:   Bjorn Helgaas <helgaas@kernel.org>
To:     Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc:     Shuai Xue <xueshuai@linux.alibaba.com>, chengyou@linux.alibaba.com,
        kaishen@linux.alibaba.com, yangyicong@huawei.com, will@kernel.org,
        baolin.wang@linux.alibaba.com, robin.murphy@arm.com,
        linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-pci@vger.kernel.org, rdunlap@infradead.org,
        mark.rutland@arm.com, zhuo.song@linux.alibaba.com
Subject: Re: [PATCH v6 0/4] drivers/perf: add Synopsys DesignWare PCIe PMU
 driver support
Message-ID: <20230725205955.GA665326@bhelgaas>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230724101807.000012bf@Huawei.com>
Precedence: bulk

On Mon, Jul 24, 2023 at 10:18:07AM +0100, Jonathan Cameron wrote:
> On Mon, 24 Jul 2023 10:34:08 +0800
> Shuai Xue <xueshuai@linux.alibaba.com> wrote:
> > On 2023/7/10 20:04, Shuai Xue wrote:
> > > On 2023/6/16 16:39, Shuai Xue wrote:  
> > >> On 2023/6/6 15:49, Shuai Xue wrote:  

> > >>> This patchset adds the PCIe Performance Monitoring Unit (PMU) driver support
> > >>> for T-Head Yitian 710 SoC chip. Yitian 710 is based on the Synopsys PCI Express
> > >>> Core controller IP which provides statistics feature.

> ...
> Really a question for Bjorn I think, but here is my 2 cents...
> 
> The problem here is that we need to do that fundamental redesign of the
> way the PCI ports drivers work.  I'm not sure there is a path to merging
> this until that is done.  The bigger problem is that I'm not sure anyone
> is actively looking at that yet.  I'd like to look at this (as I have
> the same problem for some other drivers), but it is behind various
> other things on my todo list.
> 
> Bjorn might be persuaded on a temporary solution, but that would come
> with some maintenance problems, particularly when we try to do it
> 'right' in the future.  Maybe adding another service driver would be
> a stop gap as long as we know we won't keep doing so for ever. Not sure.

I think the question here is around the for_each_pci_dev() in
__dwc_pcie_pmu_probe()?  I don't *like* that because of the
assumptions it breaks (autoload doesn't work, hotplug doesn't work),
but:

  - There are several other drivers that also do this,
  - I don't have a better suggest for any of them,
  - It's not a drivers/pci thing, so not really up to me anyway,

so I don't have any problem with this being merged as-is, as long as
you can live with the limitations.

I don't think this series does anything to work around those
limitations, i.e., it doesn't make up fake device IDs for module
loading or fake events for hotplug, so it seems like we could improve
the implementation later if we ever have a way to do it.

Bjorn