IronPort-SDR: 1r9ADWDX6XydGOkGmmcPEvfxjpNUM1SADtww1unqf19kTz1ws/2RM1tqQ2zQaCFU5fZVWWKBIB
 j6qzTFxsB/3fO6luZO5dOF/z3UWBJuMgU4Lh+WC3JvJ7+Evdt7dmu42AbT5GE9MQAeZZDH7/1D
 bCNnT0Qyozc2jbLQm0ujw/+WEcy1bNdFWiDW9Vp3l51eshCRcjl9f5kAOJQg2AnDVHiywfEIVV
 1qaWBlrZ8WgWejkcUkZ4MfXTX4lWSZJHqiUWTYke6Q83fcKeLpIS+50FgHP1fHHrWcA1B9uPm6
 v/8=
IronPort-SDR: 4r3NCBWzodo9EJwR3SO/TjRad1c5h/7UHm8LsYtBMU08d/23B2wfe0GYEoK/tIPPtJdN8EqQKe
 B8snGoAy3GCSYEYWquhHz7HQfKjl/TbAcnMoYTVMmBaHC9jmr0g+3JhRMc1xCUqbkIa0xV3LVC
 ALezbZbKKfIM8Qhb174HGjsQVr6oV9nViWaBg4RlWh7ASuwkBT7zUg8p13gGVYlVimTwbZp3kU
 hH6RuiKWFSS2gd831bs8PrHsxVIUKVk24ic4olsnuz7cdP1PGGWZzgxwDHTIKvYp6u9gD/RQ7s
 t8c=
WDCIronportException: Internal
Message-ID: <2ebd33e2-46ef-356d-ff4c-81b74950d02f@opensource.wdc.com>
Date:   Wed, 15 Feb 2023 08:56:01 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH v2 1/9] PCI: rockchip: Remove writes to unused registers
Content-Language: en-US
To:     Rick Wertenbroek <rick.wertenbroek@gmail.com>,
        alberto.dassatti@heig-vd.ch
Cc:     xxm@rock-chips.com, rick.wertenbroek@heig-vd.ch,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Heiko Stuebner <heiko@sntech.de>,
        Shawn Lin <shawn.lin@rock-chips.com>,
        Lorenzo Pieralisi <lpieralisi@kernel.org>,
        =?UTF-8?Q?Krzysztof_Wilczy=c5=84ski?= <kw@linux.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Jani Nikula <jani.nikula@intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        Mikko Kovanen <mikko.kovanen@aavamobile.com>,
        devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-pci@vger.kernel.org
References: <20230214140858.1133292-1-rick.wertenbroek@gmail.com>
 <20230214140858.1133292-2-rick.wertenbroek@gmail.com>
From:   Damien Le Moal <damien.lemoal@opensource.wdc.com>
Organization: Western Digital Research
In-Reply-To: <20230214140858.1133292-2-rick.wertenbroek@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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@gmail.com>

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