-----Original Message-----
From: Mark Brown <broonie@kernel.org>
Sent: Tuesday, October 26, 2021 6:58 PM
To: Richard Zhu <hongxing.zhu@nxp.com>
Cc: Francesco Dolcini <redacted>;
l.stach@pengutronix.de; bhelgaas@google.com;
lorenzo.pieralisi@arm.com; jingoohan1@gmail.com;
linux-pci@vger.kernel.org; dl-linux-imx [off-list ref];
linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
kernel@pengutronix.de
Subject: Re: [PATCH v3 3/7] PCI: imx6: Fix the regulator dump when link
never came up

On Tue, Oct 26, 2021 at 02:18:18AM +0000, Richard Zhu wrote:

quoted

quoted

I should probably also say that the check for the regulator looks
buggy as well, regulators should only be optional if they can be
physically absent which does not seem likely for PCI devices.  If
the driver is not doing something to reconfigure the hardware to
account for a missing supply this is generally a big warning sign.

I really don't understand why regulator support is so frequently
problematic for PCI controllers.  :(

quoted

[Richard Zhu] Hi Mark:
The _enabled check is used because that this regulator is optional in the

HW design.

quoted

To make the codes clean and aligned on different HW boards, the

_enabled check is added.

I would be really surprised to see PCI hardware that was able to support a
supply being physically absent, and this use of _is_enabled() is quite
simply not how any of this is supposed to work in the regulator API even
for regulators that can be optional.

[Richard Zhu] Actually, this regulator is one GPIO fixed regulator.
Controlled by SW to turn on (GPIO high) or turn off (GPIO low) the supply.
In some boards designs, this supply might be always on(GPIO high).
So, in point of SW driver view, this regulator is optional.

quoted

The root cause is that the error return is not handled properly by the

controller driver.

quoted

I.MX PCIe controller doesn't support the Hot-Plug, and it would return
-110 error when PCIe link never came up. Thus, the _probe would be

failed in the end.

quoted

The clocks/regulator usage balance should be considered by i.MX PCIe

controller, that's all.

quoted

It's not a general case, and the problem is not caused by the regulator

support.

Perhaps it's not causing problems in this design but if the supply is ever
shared with anything else then the software will run into trouble.
There will also be problems with the error handling on a system where
the regulator needs to be controlled.

[Richard Zhu] This GPIO fixed regulator is only used by controller driver.
It makes sense to disable the enabled regulator when driver probe is failed.

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages
much easier to read and reply to.

[Richard] Sorry about that.
BR
Richard

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel