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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220425052236.GA5587@thinkpad>
Date:   Mon, 25 Apr 2022 10:52:36 +0530
From:   Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To:     Serge Semin <fancer.lancer@...il.com>
Cc:     Serge Semin <Sergey.Semin@...kalelectronics.ru>,
        Gustavo Pimentel <gustavo.pimentel@...opsys.com>,
        Vinod Koul <vkoul@...nel.org>,
        Jingoo Han <jingoohan1@...il.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Frank Li <Frank.Li@....com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Wilczyński <kw@...ux.com>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Pavel Parkhomenko <Pavel.Parkhomenko@...kalelectronics.ru>,
        linux-pci@...r.kernel.org, dmaengine@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 25/25] PCI: dwc: Add DW eDMA engine support

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@...kalelectronics.ru>
> > > > ---
> > > >  .../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