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  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:   Wed, 2 Jan 2019 10:21:25 +0000
From:   Gustavo Pimentel <gustavo.pimentel@...opsys.com>
To:     Andrey Smirnov <andrew.smirnov@...il.com>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>
CC:     Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Fabio Estevam <fabio.estevam@....com>,
        Chris Healy <cphealy@...il.com>,
        Lucas Stach <l.stach@...gutronix.de>,
        Leonard Crestez <leonard.crestez@....com>,
        "A.s. Dong" <aisheng.dong@....com>,
        Richard Zhu <hongxing.zhu@....com>,
        "linux-imx@....com" <linux-imx@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 08/21] PCI: designware: Share code for
 dw_pcie_rd/wr_other_conf()

Hi,

On 21/12/2018 07:27, Andrey Smirnov wrote:
> Default implementation of pcie_rd_other_conf() and
> dw_pcie_wd_other_conf() share more than 80% of their code. Move shared
> code into a dedicated subroutine and convert pcie_rd_other_conf() and
> dw_pcie_wd_other_conf() to use it. No functional change intended.
> 
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@....com>
> Cc: Bjorn Helgaas <bhelgaas@...gle.com>
> Cc: Fabio Estevam <fabio.estevam@....com>
> Cc: Chris Healy <cphealy@...il.com>
> Cc: Lucas Stach <l.stach@...gutronix.de>
> Cc: Leonard Crestez <leonard.crestez@....com>
> Cc: "A.s. Dong" <aisheng.dong@....com>
> Cc: Richard Zhu <hongxing.zhu@....com>
> Cc: linux-imx@....com
> Cc: linux-arm-kernel@...ts.infradead.org
> Cc: linux-kernel@...r.kernel.org
> Cc: linux-pci@...r.kernel.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov@...il.com>
> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 61 +++++++------------
>  1 file changed, 23 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 721d60a5d9e4..8f957cd6901b 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -512,8 +512,9 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  	return ret;
>  }
>  
> -static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> -				 u32 devfn, int where, int size, u32 *val)
> +static int dw_pcie_access_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> +				     u32 devfn, int where, int size, u32 *val,
> +				     bool write)
>  {
>  	int ret, type;
>  	u32 busdev, cfg_size;
> @@ -521,9 +522,6 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
>  	void __iomem *va_cfg_base;
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>  
> -	if (pp->ops->rd_other_conf)
> -		return pp->ops->rd_other_conf(pp, bus, devfn, where, size, val);
> -
>  	busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) |
>  		 PCIE_ATU_FUNC(PCI_FUNC(devfn));
>  
> @@ -542,7 +540,11 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
>  	dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1,
>  				  type, cpu_addr,
>  				  busdev, cfg_size);
> -	ret = dw_pcie_read(va_cfg_base + where, size, val);
> +	if (write)
> +		ret = dw_pcie_write(va_cfg_base + where, size, *val);
> +	else
> +		ret = dw_pcie_read(va_cfg_base + where, size, val);
> +
>  	if (pci->num_viewport <= 2)
>  		dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1,
>  					  PCIE_ATU_TYPE_IO, pp->io_base,
> @@ -551,43 +553,26 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
>  	return ret;
>  }
>  
> +static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> +				 u32 devfn, int where, int size, u32 *val)
> +{
> +	if (pp->ops->rd_other_conf)
> +		return pp->ops->rd_other_conf(pp, bus, devfn, where,
> +					      size, val);
> +
> +	return dw_pcie_access_other_conf(pp, bus, devfn, where, size, val,
> +					 false);
> +}
> +
>  static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
>  				 u32 devfn, int where, int size, u32 val)
>  {
> -	int ret, type;
> -	u32 busdev, cfg_size;
> -	u64 cpu_addr;
> -	void __iomem *va_cfg_base;
> -	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> -
>  	if (pp->ops->wr_other_conf)
> -		return pp->ops->wr_other_conf(pp, bus, devfn, where, size, val);
> -
> -	busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) |
> -		 PCIE_ATU_FUNC(PCI_FUNC(devfn));
> +		return pp->ops->wr_other_conf(pp, bus, devfn, where,
> +					      size, val);
>  
> -	if (bus->parent->number == pp->root_bus_nr) {
> -		type = PCIE_ATU_TYPE_CFG0;
> -		cpu_addr = pp->cfg0_base;
> -		cfg_size = pp->cfg0_size;
> -		va_cfg_base = pp->va_cfg0_base;
> -	} else {
> -		type = PCIE_ATU_TYPE_CFG1;
> -		cpu_addr = pp->cfg1_base;
> -		cfg_size = pp->cfg1_size;
> -		va_cfg_base = pp->va_cfg1_base;
> -	}
> -
> -	dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1,
> -				  type, cpu_addr,
> -				  busdev, cfg_size);
> -	ret = dw_pcie_write(va_cfg_base + where, size, val);
> -	if (pci->num_viewport <= 2)
> -		dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1,
> -					  PCIE_ATU_TYPE_IO, pp->io_base,
> -					  pp->io_bus_addr, pp->io_size);
> -
> -	return ret;
> +	return dw_pcie_access_other_conf(pp, bus, devfn, where, size, &val,
> +					 true);
>  }
>  
>  static int dw_pcie_valid_device(struct pcie_port *pp, struct pci_bus *bus,
> 

Nice!

Acked-by: Gustavo Pimentel <gustavo.pimentel@...opsys.com>

Regards,
Gustavo

Powered by blists - more mailing lists