Re: [PATCH 2/2] PCI: qcom: Move qcom_pcie_icc_opp_update() to notifier callback

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 7/16/25 7:28 AM, Manivannan Sadhasivam wrote:
> On Tue, Jul 15, 2025 at 12:45:36PM GMT, Konrad Dybcio wrote:
>> On 7/15/25 12:36 PM, Manivannan Sadhasivam wrote:
>>> On Tue, Jul 15, 2025 at 11:54:48AM GMT, Konrad Dybcio wrote:
>>>> On 7/14/25 8:01 PM, Manivannan Sadhasivam wrote:
>>>>> It allows us to group all the settings that need to be done when a PCI
>>>>> device is attached to the bus in a single place.
>>>>>
>>>>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>>>>> ---

[...]

>>> Let me think of other ways to call these two APIs during the device addition. If
>>> there are no sane ways, I'll drop *this* patch.
>>
>> Would it be too naive to assume BUS_NOTIFY_ADD_DEVICE is a good fit?
> 
> BUS_NOTIFY_ADD_DEVICE is not currently a good fit as ASPM link state
> initialization happen after all the devices are enumerated for the slot. This is
> something to be fixed in the PCI core and would allow us to use
> BUS_NOTIFY_ADD_DEVICE.
> 
> I talked to Bjorn H and we both agreed that this needs to be revisited. But I'm
> just worrried that until this happens, we cannot upstream the ASPM fix and not
> even backport it to 6.16/16.
> 
> So maybe we need to resort to this patch as an interim fix if everyone agrees.

I'm not opposed if there's going to be an improved solution next cycle.
Having ASPM 99.9% of the time is much better than not having it at all

> 
>> Do
>> ASPM setting need to be reapplied after the PCIe device is reset? (well
>> I would assume there are probably multiple levels of "reset" :/)
>>
> 
> I'm assuming that you are referring to link down reset here. PCI core takes care
> of saving both the endpoint as well as Root Port config space when that happens
> and restores them afterwards.

Nice, thanks for confirming

Konrad