linux-kernel - Re: [PATCH v9 24/27] dmaengine: dw-edma: Relax driver config settings

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: <20230125232357.GA1176625@bhelgaas>
Date:   Wed, 25 Jan 2023 17:23:57 -0600
From:   Bjorn Helgaas <helgaas@...nel.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>, Rob Herring <robh@...nel.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Cai Huoqing <cai.huoqing@...ux.dev>,
        Robin Murphy <robin.murphy@....com>,
        Jingoo Han <jingoohan1@...il.com>, Frank Li <Frank.Li@....com>,
        Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Pavel Parkhomenko <Pavel.Parkhomenko@...kalelectronics.ru>,
        Krzysztof Wilczyński <kw@...ux.com>,
        caihuoqing <caihuoqing@...du.com>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
        linux-pci@...r.kernel.org, dmaengine@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v9 24/27] dmaengine: dw-edma: Relax driver config settings

On Wed, Jan 25, 2023 at 05:40:19PM +0300, Serge Semin wrote:
> On Tue, Jan 24, 2023 at 05:47:44PM -0600, Bjorn Helgaas wrote:

> > In the commit log, I think "forcibly selecting the DW eDMA driver from
> > the DW PCIe RP/EP kconfig" actually refers to just the "DW eDMA PCIe"
> > driver" not the "DW PCIe RP/EP driver," right?
> 
> Right.

Good.  I think it's worth updating the commit log to clear this up
because there are several things with very similar names, so it's
confusing enough already ;)

> > The undefined reference to dw_edma_probe() doesn't actually happen
> > unless we merge 27/27 without *this* patch, right? 
> 
> Right.

Thanks, I got unreasonably focused on the "fix 'undefined reference'
error" comment, wondering if we needed to identify a Fixes: commit, so
this clears that up, too.

> > I would use "depends on
> >      DW_EDMA" instead of adding if/endif around DW_EDMA_PCIE.
> 
> Could you explain why is the "depends on" operator more preferable
> than if/endif? In this case since we have a single core kconfig from
> which all the eDMA LLDD config(s) (except PCIE_DW for the reason
> previously described) will surely depend on, using if/endif would
> cause the possible new eDMA-capable LLDD(s) adding their kconfig
> entries within the if-endif clause without need to copy the same
> "depends on DW_EDMA" pattern over and over. That seems to look a bit
> more maintainable than the alternative you suggest. Do you think
> otherwise?

Only that "depends on" is much more common and I always try to avoid
unusual constructs.  But I wasn't looking into the future and
imagining several LLDDs with similar uses of "depends on DW_EDMA".
Thanks for that perspective; with it, I think it's OK either way.

> > What do you think? 
> 
> What you described was the second option I had in mind for the update
> to look like, but after all I decided to take a shorter path and
> combine the modifications into a single patch. If you think that
> splitting it up would make the update looking simpler then I'll do as
> you suggest. But in that case Lorenzo will need to re-merge the
> updated patchset v10.

It's a pretty trivial update, so I just did it myself.  The result is
at https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/log/?h=pci/ctrl/dwc&id=ecadcaed4ef7

I split this patch and tweaked some commit messages for consistency
(including the "DW eDMA PCIe driver" change above).  "git diff -b"
with Lorenzo's current branch (95624672bb3e ("PCI: dwc: Add DW eDMA
engine support")) is empty except for a minor comment change.  

Bjorn