Re: [PATCH] PCI: j721e: Fix the value of linkdown_irq_regfield for J784S4

[Siddharth Vadapalli]

On Tue, Mar 11, 2025 at 06:07:46AM +0900, Krzysztof Wilczyński wrote:
> Hello,
> 
> > Hence, set 'linkdown_irq_regfield' to the macro 'J7200_LINK_DOWN' which
> > expands to BIT(10) and was first defined for the J7200 SoC. Other SoCs
> > already reuse this macro since it accurately represents the link-state
> > field in their respective "PCIE_INTD_ENABLE_REG_SYS_2" register.
> 
> Can you confirm for me that the following use the correct macro?
> 
>   333-static const struct j721e_pcie_data j721e_pcie_rc_data = {
>   337:	.linkdown_irq_regfield = LINK_DOWN,
>   
>   341-static const struct j721e_pcie_data j721e_pcie_ep_data = {
>   343:	.linkdown_irq_regfield = LINK_DOWN,
>   
>   347-static const struct j721e_pcie_data j7200_pcie_rc_data = {
>   350:	.linkdown_irq_regfield = J7200_LINK_DOWN,
>   
>   362-static const struct j721e_pcie_data am64_pcie_rc_data = {
>   364:	.linkdown_irq_regfield = J7200_LINK_DOWN,
>   
>   369-static const struct j721e_pcie_data am64_pcie_ep_data = {
>   371:	.linkdown_irq_regfield = J7200_LINK_DOWN,
>   
>   375-static const struct j721e_pcie_data j784s4_pcie_rc_data = {
>   379:	.linkdown_irq_regfield = LINK_DOWN,
>   
>   383-static const struct j721e_pcie_data j784s4_pcie_ep_data = {
>   385:	.linkdown_irq_regfield = LINK_DOWN,
>   
>   389-static const struct j721e_pcie_data j722s_pcie_rc_data = {
>   391:	.linkdown_irq_regfield = J7200_LINK_DOWN,
> 
> I am asking as some use LINK_DOWN, so I wanted to make sure.

Yes, the above are accurate except for J784S4 which is fixed by this
patch. LINK_DOWN i.e. BIT(1) is applicable only to J721E which was the
first SoC after which the driver has been named. For all other SoCs, the
integration of the PCIe Controller into the SoC led to BIT(10) of the
register being used to indicate the link status.

> 
> Tht said, the following has no .linkdown_irq_regfield property set:
> 
>   355-static const struct j721e_pcie_data j7200_pcie_ep_data = {
>   356-	.mode = PCI_MODE_EP,
>   357-	.quirk_detect_quiet_flag = true,
>   358-	.quirk_disable_flr = true,
>   359-	.max_lanes = 2,
>   360-};
> 
> Would this be a problem?  Or is this as expected?

Thank you for pointing this out. This has to be fixed and the
"linkdown_irq_regfield" member has to be added to match
j7200_pcie_rc_data. I will post the fix for this.

Regards,
Siddharth.