Re: [PATCH v2 1/9] PCI: rockchip: Remove writes to unused registers

[Damien Le Moal]

On 2/14/23 23:08, Rick Wertenbroek wrote:
> Remove write accesses to registers that are marked "unused" (and
> therefore read-only) in the technical reference manual (TRM)
> (see RK3399 TRM 17.6.8.1)
> 
> Signed-off-by: Rick Wertenbroek <rick.wertenbroek@xxxxxxxxx>

I checked the TRM and indeed these registers are listed as unused.
However, with this patch, nothing work for me using a Pine rockpro64
board. Keeping this patch, your series (modulo some other fixes, more
emails coming) is making things work !

So I think the bug is with the TRM, not the code. THinking logically about
htis, it makes sense: this is programming the address translation unit to
translate mmio & dma between host PCI address and local CPU space address.
If we never set the PU address, how can that unit possibly ever translate
anything ?

> ---
>  drivers/pci/controller/pcie-rockchip-ep.c | 10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
> index d1a200b93..d5c477020 100644
> --- a/drivers/pci/controller/pcie-rockchip-ep.c
> +++ b/drivers/pci/controller/pcie-rockchip-ep.c
> @@ -61,10 +61,6 @@ static void rockchip_pcie_clear_ep_ob_atu(struct rockchip_pcie *rockchip,
>  			    ROCKCHIP_PCIE_AT_OB_REGION_DESC0(region));
>  	rockchip_pcie_write(rockchip, 0,
>  			    ROCKCHIP_PCIE_AT_OB_REGION_DESC1(region));
> -	rockchip_pcie_write(rockchip, 0,
> -			    ROCKCHIP_PCIE_AT_OB_REGION_CPU_ADDR0(region));
> -	rockchip_pcie_write(rockchip, 0,
> -			    ROCKCHIP_PCIE_AT_OB_REGION_CPU_ADDR1(region));
>  }
>  
>  static void rockchip_pcie_prog_ep_ob_atu(struct rockchip_pcie *rockchip, u8 fn,
> @@ -114,12 +110,6 @@ static void rockchip_pcie_prog_ep_ob_atu(struct rockchip_pcie *rockchip, u8 fn,
>  		     PCIE_CORE_OB_REGION_ADDR0_LO_ADDR);
>  		addr1 = upper_32_bits(cpu_addr);
>  	}
> -
> -	/* CPU bus address region */
> -	rockchip_pcie_write(rockchip, addr0,
> -			    ROCKCHIP_PCIE_AT_OB_REGION_CPU_ADDR0(r));
> -	rockchip_pcie_write(rockchip, addr1,
> -			    ROCKCHIP_PCIE_AT_OB_REGION_CPU_ADDR1(r));
>  }
>  
>  static int rockchip_pcie_ep_write_header(struct pci_epc *epc, u8 fn, u8 vfn,

-- 
Damien Le Moal
Western Digital Research