Re: [PATCH v2 2/4] thunderbolt: Separate out common NHI bits

[Konrad Dybcio]

On 5/4/26 8:54 AM, Mika Westerberg wrote:
> Hi,

[...]

>> +void nhi_pci_shutdown(struct tb_nhi *nhi)
> 
> Why these are not static?

[...]

>> +static const struct tb_nhi_ops pci_nhi_default_ops = {
>> +	.pre_nvm_auth = nhi_pci_start_dma_port,
>> +	.post_nvm_auth = nhi_pci_complete_dma_port,
>> +	.request_ring_irq = nhi_pci_ring_request_msix,
>> +	.release_ring_irq = nhi_pci_ring_release_msix,
>> +	.shutdown = nhi_pci_shutdown,
>> +	.is_present = nhi_pci_is_present,
>> +	.init_interrupts = nhi_pci_init_msi,
> 
> You populate them here so there is no need to expose outside of pci.c.

nhi_ops.c needs them too, as they were previously called
unconditionally for all NHI flavors

[...]


>> +/*
>> + * During suspend the Thunderbolt controller is reset and all PCIe
>> + * tunnels are lost. The NHI driver will try to reestablish all tunnels
>> + * during resume. This adds device links between the tunneled PCIe
>> + * downstream ports and the NHI so that the device core will make sure
>> + * NHI is resumed first before the rest.
>> + */
>> +bool tb_apple_add_links(struct tb_nhi *nhi)
> 
> Okay you moved it here good. I think we can call it in nhi_pci_probe()
> directly so no need to expose outside.

Yeah that seems like a good idea. It's already there, behind N calls
in the software CM case.

Do we have to check the CM type though, or do you think it'd be fine
to just call it unconditionally? (either because there are presumably
no Apple machines with ICM or because these devlinks would be harmless?)

(ack for all the remaining comments)

Konrad