Re: [PATCH 25/25] PCI: dwc: Add DW eDMA engine support

[Manivannan Sadhasivam]

On Sat, Apr 23, 2022 at 08:10:55PM +0530, Manivannan Sadhasivam wrote:
> On Tue, Apr 19, 2022 at 11:54:03PM +0300, Serge Semin wrote:
> > On Mon, Mar 28, 2022 at 07:45:21PM +0530, Manivannan Sadhasivam wrote:
> > > On Thu, Mar 24, 2022 at 04:48:36AM +0300, Serge Semin wrote:
> > > > Since the DW eDMA driver now supports eDMA controllers embedded into the
> > > > locally accessible DW PCIe Root Ports and End-points, we can use the
> > > > updated interface to register DW eDMA as DMA engine device if it's
> > > > available. In order to successfully do that the DW PCIe core driver need
> > > > to perform some preparations first. First of all it needs to find out the
> > > > eDMA controller CSRs base address, whether they are accessible over the
> > > > Port Logic or iATU unrolled space. Afterwards it can try to auto-detect
> > > > the eDMA controller availability and number of it's read/write channels.
> > > > If none was found the procedure will just silently halt with no error
> > > > returned. Secondly the platform is supposed to provide either combined or
> > > > per-channel IRQ signals. If no valid IRQs set is found the procedure will
> > > > also halt with no error returned so to be backward compatible with
> > > > platforms where DW PCIe controllers have eDMA embedded but lack of the
> > > > IRQs defined for them. Finally before actually probing the eDMA device we
> > > > need to allocate LLP items buffers. After that the DW eDMA can be
> > > > registered. If registration is successful the info-message regarding the
> > > > number of detected Read/Write eDMA channels will be printed to the system
> > > > log in the same way as it's done for iATU settings.
> > > > 
> > > > Signed-off-by: Serge Semin <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx>
> > > > ---
> > > >  .../pci/controller/dwc/pcie-designware-ep.c   |   4 +
> > > >  .../pci/controller/dwc/pcie-designware-host.c |  13 +-
> > > >  drivers/pci/controller/dwc/pcie-designware.c  | 188 ++++++++++++++++++
> > > >  drivers/pci/controller/dwc/pcie-designware.h  |  23 ++-
> > > >  4 files changed, 225 insertions(+), 3 deletions(-)
> > > > 

[...]

> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > > > index 4a95a7b112e9..dbe39a7ecb71 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c

[...]

> > > Should we introduce a new Kconfig option for enabling eDMA? My concern here is,
> > > if eDMA is really needed for an usecase and if the platform support is broken
> > > somehow (DT issues?), then we'll just simply go ahead without probe failure and
> > > it may break somewhere else.
> > > 
> > > And we are returning errors if something wrong happens during eDMA probe. This
> > > might annoy the existing users who don't care about eDMA but turning those
> > > errors to debug will affect the real users of eDMA.
> > > 
> > > For these reasons, I think it'd be better to probe eDMA only if the Kconfig
> > > option is enabled (which would be disabled by default). And properly return the
> > > failure.
> > 
> > I don't see a need in introducing of a new parametrization. Neither
> > there is a point in dropping the eDMA support on all the platforms for
> > the sake of some hypothetically malfunction hardware.
> 
> I'm not talking about "hypothetically malfunction hardware" but real customized
> ones like all Qcom platforms supporting PCIe.

Correction: It is not "all Qcom platforms" but some like SM8250, SM8450 etc...

Thanks,
Mani