linux-kernel - Re: [PATCH 10/16] PCI: dwc: Drop iATU regions enumeration

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]

Date:   Sun, 17 Apr 2022 20:52:17 +0300
From:   Serge Semin <fancer.lancer@...il.com>
To:     Rob Herring <robh@...nel.org>
Cc:     Serge Semin <Sergey.Semin@...kalelectronics.ru>,
        Jingoo Han <jingoohan1@...il.com>,
        Gustavo Pimentel <gustavo.pimentel@...opsys.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Krzysztof Wilczyński <kw@...ux.com>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Pavel Parkhomenko <Pavel.Parkhomenko@...kalelectronics.ru>,
        Frank Li <Frank.Li@....com>,
        Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
        linux-pci@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 10/16] PCI: dwc: Drop iATU regions enumeration -
 dw_pcie_region_type

On Tue, Mar 29, 2022 at 10:31:18AM -0500, Rob Herring wrote:
> On Thu, Mar 24, 2022 at 04:37:28AM +0300, Serge Semin wrote:
> > There is no point in having the dw_pcie_region_type enumeration for almost
> > the same reasons as it was stated for dw_pcie_as_type. First of all it's
> > redundant since the driver already has a set of macro declared which
> > describe the possible inbound and outbound iATU regions. Having an
> > addition abstraction just needlessly complicates the code. Secondly
> > checking the region index passed to the dw_pcie_disable_atu() method for
> > validity is pointless since the erroneous situation will be just
> > ignored in the current code implementation. So to speak let's drop the
> > redundant dw_pcie_region_type enumeration replacing it with the direct
> > iATU direction macro usage.
> > 
> > While at it we suggest to convert the dw_pcie_disable_atu() method to
> > being more consistent with the dw_pcie_readl_atu{_ib}() and
> > dw_pcie_readl_atu{_ob}() functions by having the direction parameter
> > specified ahead of the region index. Thus the code will be a little bit
> > more pleasant to read.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@...kalelectronics.ru>
> > ---
> >  drivers/pci/controller/dwc/pcie-designware-ep.c  |  4 ++--
> >  .../pci/controller/dwc/pcie-designware-host.c    |  2 +-
> >  drivers/pci/controller/dwc/pcie-designware.c     | 16 +---------------
> >  drivers/pci/controller/dwc/pcie-designware.h     |  9 +--------
> >  4 files changed, 5 insertions(+), 26 deletions(-)
> 

> This answers my question. I would have expected this to come before the 
> previous patch, but if it's easier to do it this way it's fine.
> 
> Reviewed-by: Rob Herring <robh@...nel.org>

I thought about placing this patch before the previous one, but it
turned a bit easier for me to split the changes in the reverse order.
It has made this patch a bit smaller and more coherent. But seeing you
weren't happy with too many changes in the previous patch I'll do as
you suggest and change the patches order. Since the patch content will
be changed I won't add your reviewed-by tag there on v2. So please
consider re-reviewing it one more time.

-Sergey