Re: [PATCH V1] PCI: dwc: Use dev_info for PCIe link down event logging

[Manivannan Sadhasivam]

On Tue, Sep 13, 2022 at 06:00:30PM +0100, Jon Hunter wrote:
> 
> On 13/09/2022 17:51, Manivannan Sadhasivam wrote:
> > On Tue, Sep 13, 2022 at 03:42:37PM +0530, Vidya Sagar wrote:
> > > Some of the platforms (like Tegra194 and Tegra234) have open slots and
> > > not having an endpoint connected to the slot is not an error.
> > > So, changing the macro from dev_err to dev_info to log the event.
> > > 
> > 
> > But the link up not happening is an actual error and -ETIMEDOUT is being
> > returned. So I don't think the log severity should be changed.
> 
> Yes it is an error in the sense it is a timeout, but reporting an error
> because nothing is attached to a PCI slot seems a bit noisy. Please note
> that a similar change was made by the following commit and it also seems
> appropriate here ...
> 

I got the concern. But the problem here is, we cannot distinguish if the
error is due to slot empty or an actual issues with link/phy. Also it
should be noted that if the slot is empty, the dwc core anyway waits for
the link to be up. IMO this is a boot time overhead that could be
avoided if the specific instance could be disabled in platform data like
devicetree.

> commit 4b16a8227907118e011fb396022da671a52b2272
> Author: Manikanta Maddireddy <mmaddireddy@xxxxxxxxxx>
> Date:   Tue Jun 18 23:32:06 2019 +0530
> 
>     PCI: tegra: Change link retry log level to debug
> 
> 
> BTW, we check for error messages in the dmesg output and this is a new error
> seen as of Linux v6.0 and so this was flagged in a test. We can ignore the
> error, but in this case it seem more appropriate to make this a info or
> debug level print.
> 

On some of the Qcom based platforms, we avoid this situation by
disabling the specific PCIe node in devicetree for which we know that
there would be no devices connected.

This not only avoids the error message in log but also removes the time
spent waiting for link to be up.

Thanks,
Mani

> Jon
> 
> -- 
> nvpublic