Re: [PATCH v7 17/20] PCI: dwc: Introduce generic resources getter

[Serge Semin]

Hi Bjorn,

On Wed, Nov 23, 2022 at 01:44:36PM -0600, Bjorn Helgaas wrote:
> Hi Serge,
> 
> On Sun, Nov 13, 2022 at 10:12:58PM +0300, Serge Semin wrote:
> > Currently the DW PCIe Root Port and Endpoint CSR spaces are retrieved in
> > the separate parts of the DW PCIe core driver. It doesn't really make
> > sense since the both controller types have identical set of the core CSR
> > regions: DBI, DBI CS2 and iATU/eDMA. Thus we can simplify the DW PCIe Host
> > and EP initialization methods by moving the platform-specific registers
> > space getting and mapping into a common method. It gets to be even more
> > justified seeing the CSRs base address pointers are preserved in the
> > common DW PCIe descriptor. Note all the OF-based common DW PCIe settings
> > initialization will be moved to the new method too in order to have a
> > single function for all the generic platform properties handling in single
> > place.
> > 
> > A nice side-effect of this change is that the pcie-designware-host.c and
> > pcie-designware-ep.c drivers are cleaned up from all the direct dw_pcie
> > storage modification, which makes the DW PCIe core, Root Port and Endpoint
> > modules more coherent.
> 

> Thanks for these new generic interfaces in the DWC core!  And thanks
> for the changes in this patch to take advantage of them in the
> pcie-designware drivers.
> 
> Do you plan similar changes to other drivers to take advantage of
> these DWC-generic data and interfaces?  If you add generic things to
> the DWC core but only take advantage of them in your driver, I don't
> think they are really usefully generic.

Could you be more specific what generic things you are referring to? I
am asking because the only part of the changes which is used in my
low-level driver only is introduced in another patch of this series.
It's
< [PATCH v7 19/20] PCI: dwc: Introduce generic platform clocks and resets
The new clock/reset request interface has been implemented the way it
is due to reasons I in details described to Rob here:
Link: https://lore.kernel.org/linux-pci/20220520160246.guczq52v2ycfgc6c@mobilestation
To cut it short it can't be used by the most of the already available
low-level drivers since they already have their own versions of the
names for the clock and reset resources (or don't have any name
defined at all). The only driver for which the interface could be
utilized is Toshiba Visconti PCIe host controller driver. The device
DT-bindings defines the clock names matching the generic names
introduced in the patches of this series. If you find it appropriate
enough I can provide a patch for that driver.

Note the main goal of the patch
[PATCH v7 19/20] PCI: dwc: Introduce generic platform clocks and resets
was to create some interface to stop the developers of the new drivers
from creating the platform-specific DT-bindings to the same clock and
reset resources. Since the already defined DT-bindings can't be
changed anyway I don't think it would worth risking to catch
regressions on an attempt to provide a more complicated interface
utilized in the old drivers too.

-Serge(y)

> 
> Bjorn