[PATCH v5 01/14] PCI: rockchip-ep: Fix address translation unit programming

[Damien Le Moal]

On 11/16/24 07:41, Bjorn Helgaas wrote:
>> diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
>> index 136274533656..27a7febb74e0 100644
>> --- a/drivers/pci/controller/pcie-rockchip-ep.c
>> +++ b/drivers/pci/controller/pcie-rockchip-ep.c
>> @@ -63,16 +63,23 @@ static void rockchip_pcie_clear_ep_ob_atu(struct rockchip_pcie *rockchip,
>>  			    ROCKCHIP_PCIE_AT_OB_REGION_DESC1(region));
>>  }
>>  
>> +static int rockchip_pcie_ep_ob_atu_num_bits(struct rockchip_pcie *rockchip,
>> +					    u64 pci_addr, size_t size)
>> +{
>> +	int num_pass_bits = fls64(pci_addr ^ (pci_addr + size - 1));
>> +
>> +	return clamp(num_pass_bits, ROCKCHIP_PCIE_AT_MIN_NUM_BITS,
>> +		     ROCKCHIP_PCIE_AT_MAX_NUM_BITS);
>> +}
>> +
>>  static void rockchip_pcie_prog_ep_ob_atu(struct rockchip_pcie *rockchip, u8 fn,
>>  					 u32 r, u64 cpu_addr, u64 pci_addr,
>>  					 size_t size)
>>  {
>> -	int num_pass_bits = fls64(size - 1);
>> +	int num_pass_bits =
>> +		rockchip_pcie_ep_ob_atu_num_bits(rockchip, pci_addr, size);
>>  	u32 addr0, addr1, desc0;
>>  
>> -	if (num_pass_bits < 8)
>> -		num_pass_bits = 8;
>> -
>>  	addr0 = ((num_pass_bits - 1) & PCIE_CORE_OB_REGION_ADDR0_NUM_BITS) |
>>  		(lower_32_bits(pci_addr) & PCIE_CORE_OB_REGION_ADDR0_LO_ADDR);
> 
> PCIE_CORE_OB_REGION_ADDR0_NUM_BITS is 0x3f and
> rockchip_pcie_ep_ob_atu_num_bits() returns something between 8 and
> 0x14, inclusive?  So masking with PCIE_CORE_OB_REGION_ADDR0_NUM_BITS
> doesn't do anything, does it?

Indeed, we could remove that mask.

> Also, "..._NUM_BITS" is kind of a weird name for a mask.

Well, I did not change that. It was like this. Can clean that up too. Do you
want me to send a patch ?

> rockchip_pcie_prog_ob_atu() in pcie-rockchip-host.c is similar but
> different; it looks like all callers supply num_pass_bits=19.  I
> assume it doesn't need a similar change?

I did not check the TRM for host mode. But for my tests, I used 2 rockpro64, one
as RC and the other as EP, and the RC side was working just fine without any
change. So I assume it is OK as-is.

-- 
Damien Le Moal
Western Digital Research