| Message ID | 7f4abd1ba9f4b33fe6f66213f56aa4269db74317.1612271903.git.mchehab+huawei@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for Hikey 970 PCIe | expand |
On Tue, Feb 02, 2021 at 02:29:54PM +0100, Mauro Carvalho Chehab wrote: > On Hikey 970, there's a power supply controlled by Hi6421v600 > regulator that turns on the PCI devices on the board. Without > that, no PCI hardware would work. > > As this is device-dependent, such regulator line should be > optional. Supplies should only be optional if they may be physically absent from the system, if they are just sometimes not described well in firmware they should be requested via the normal regulator_get() interface, the core will supply a dummy regulator if there is nothing at all in the firmware description. Supplies should also generally be referred to with the naming used in the datasheet for the part.
Hi Mark, Em Tue, 2 Feb 2021 13:41:01 +0000 Mark Brown <broonie@kernel.org> escreveu: > On Tue, Feb 02, 2021 at 02:29:54PM +0100, Mauro Carvalho Chehab wrote: > > On Hikey 970, there's a power supply controlled by Hi6421v600 > > regulator that turns on the PCI devices on the board. Without > > that, no PCI hardware would work. > > > > As this is device-dependent, such regulator line should be > > optional. > > Supplies should only be optional if they may be physically absent from > the system, That's the case. On Hikey 960, the PCIe hardware supported by this driver doesn't require any regulator. On Hikey 970, the PCIe hardware is more complex. Some components are outside the SoC, and those require a regulator to be powered up. > if they are just sometimes not described well in firmware > they should be requested via the normal regulator_get() interface, the > core will supply a dummy regulator if there is nothing at all in the > firmware description. For Hikey 970, a normal regulator_get() would work. > Supplies should also generally be referred to with the naming used in > the datasheet for the part. Ok, I'll rename it to "pcie_vdd", which better reflects the schematics. Thanks, Mauro
On Tue, Feb 02, 2021 at 03:50:28PM +0100, Mauro Carvalho Chehab wrote: > Mark Brown <broonie@kernel.org> escreveu: > > On Tue, Feb 02, 2021 at 02:29:54PM +0100, Mauro Carvalho Chehab wrote: > > > As this is device-dependent, such regulator line should be > > > optional. > > Supplies should only be optional if they may be physically absent from > > the system, > That's the case. On Hikey 960, the PCIe hardware supported by this > driver doesn't require any regulator. > On Hikey 970, the PCIe hardware is more complex. Some components > are outside the SoC, and those require a regulator to be powered > up. That's not an optional supply, that's a required supply for a different device. The driver should select which supplies it is requesting based on the hardware it's controlling.
diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c index 2bce6e3750d4..42aea34dff4d 100644 --- a/drivers/pci/controller/dwc/pcie-kirin.c +++ b/drivers/pci/controller/dwc/pcie-kirin.c @@ -22,6 +22,7 @@ #include <linux/pci_regs.h> #include <linux/platform_device.h> #include <linux/regmap.h> +#include <linux/regulator/consumer.h> #include <linux/resource.h> #include <linux/types.h> #include "pcie-designware.h" @@ -295,6 +296,22 @@ static void kirin970_pcie_set_eyeparam(struct kirin_pcie *kirin_pcie) static long kirin_common_pcie_get_resource(struct kirin_pcie *kirin_pcie, struct platform_device *pdev) { + struct device *dev = &pdev->dev; + struct regulator *reg; + int ret; + + reg = devm_regulator_get_optional(dev, "pci"); + if (IS_ERR_OR_NULL(reg)) { + if (PTR_ERR(reg) == -EPROBE_DEFER) + return PTR_ERR(reg); + } else { + ret = regulator_enable(reg); + if (ret) { + dev_err(dev, "Failed to enable regulator\n"); + return ret; + } + } + kirin_pcie->apb_base = devm_platform_ioremap_resource_byname(pdev, "apb"); if (IS_ERR(kirin_pcie->apb_base))
On Hikey 970, there's a power supply controlled by Hi6421v600 regulator that turns on the PCI devices on the board. Without that, no PCI hardware would work. As this is device-dependent, such regulator line should be optional. Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> --- drivers/pci/controller/dwc/pcie-kirin.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)