Re: [PATCH v3 2/7] PCI: cadence: Add post-link delay for LGA and j721e glue driver

On 5/12/26 08:24, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Compilation error in `j721e_pcie_set_link_speed` due to incorrect structure member access on a pointer.
> --
> 
> commit b3897a7e50194e1f9cc94b232c615c7f8f651006
> Author: Hans Zhang <[email protected]>
> 
> PCI: cadence: Add post-link delay for LGA and j721e glue driver
> 
> The Cadence LGA PCIe host controller lacks the mandatory 100 ms delay after
> link training completes for speeds > 5.0 GT/s. This commit adds a
> max_link_speed field to struct cdns_pcie and inserts the required delay
> after the link has been successfully established.
> 
>> diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
>> index bfdfe98d5aba7..ee85b8e04f5b8 100644
>> --- a/drivers/pci/controller/cadence/pci-j721e.c
>> +++ b/drivers/pci/controller/cadence/pci-j721e.c
>> @@ -206,6 +206,7 @@ static int j721e_pcie_set_link_speed(struct j721e_pcie *pcie,
>>   	    (pcie_get_link_speed(link_speed) == PCI_SPEED_UNKNOWN))
>>   		link_speed = 2;
>>   
>> +	pcie->cdns_pcie.max_link_speed = link_speed;
> 
> Will this cause a compilation error?

Sorry. Will change.

Best regards,
Hans

> 
> Looking at struct j721e_pcie, cdns_pcie is defined as a pointer:
> 
> struct j721e_pcie {
> 	struct cdns_pcie	*cdns_pcie;
> 	struct clk		*refclk;
> 	...
> };
> 
> Does this need to use the arrow operator instead of a dot
> (pcie->cdns_pcie->max_link_speed) so it correctly accesses the
> max_link_speed member?
> 
>>   	val = link_speed - 1;
>>   	ret = regmap_update_bits(syscon, offset, GENERATION_SEL_MASK, val);
>>   	if (ret)
>