On Mon, Feb 14, 2022 at 10:05 AM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > > [+cc Rob for possible DT/kernel match issue, > Lorenzo (native host bridge driver maintainer)] > > s/fix finding/Fix finding/ (in subject) > Or even better, say something specific about the DT properties in > question, e.g., look for "reset" instead of "reset-gpios". This is purely a kernel issue. The gpio API appends '-gpios' (and a fallback to '-gpio') to the name passed. So passing 'reset-gpios' looks for 'reset-gpios-gpios' property. This happens enough, there should probably be a check for this mistake. > > On Mon, Feb 14, 2022 at 08:21:43AM +0000, Ben Dooks wrote: > > The calls to devm_gpiod_get_optional() have the -gpios at the end of > > the name. This means the pcie driver is not finding the necessary > > reset or power GPOOs to allow the PCIe devices on the SiFive Unmatched > > boards. > > s/pcie/PCIe/ > s/GPOOs/GPIOs/ > "to allow the PCIe devices ...?" Something is missing from this > sentence. "To allow the devices <to do what>"? Or maybe the driver > needs these GPIOs to power up the PCIe devices? > > I guess the implication is that the code looks for "reset-gpios" and > "pwren-gpios", but the DT contains "reset" and "pwren"? > > But both Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml > and arch/riscv/boot/dts/sifive/fu740-c000.dtsi actually do contain > "reset-gpios" and "pwren-gpios". > > If we *do* want to change the code, please change the error messages > to match. > > > This has not been a noted bug as the PCIe probe from u-boot has been > > required to get the PCIe working due to other issues with the system > > setup. It could have been broken since the driver inclusion, and not > > been noticed as it is not necessary for the driver to funciton. > > s/u-boot/U-Boot/ > s/funciton/function/ > > Please add a line about what the connection between U-Boot and this > issue is, e.g., maybe U-Boot powers up the devices, so we wouldn't > notice the kernel's inability to do so? > > > Signed-off-by: Ben Dooks <ben.dooks@xxxxxxxxxxxxxxx> > > --- > > drivers/pci/controller/dwc/pcie-fu740.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-fu740.c b/drivers/pci/controller/dwc/pcie-fu740.c > > index 00cde9a248b5..842b7202b96e 100644 > > --- a/drivers/pci/controller/dwc/pcie-fu740.c > > +++ b/drivers/pci/controller/dwc/pcie-fu740.c > > @@ -259,11 +259,11 @@ static int fu740_pcie_probe(struct platform_device *pdev) > > return PTR_ERR(afp->mgmt_base); > > > > /* Fetch GPIOs */ > > - afp->reset = devm_gpiod_get_optional(dev, "reset-gpios", GPIOD_OUT_LOW); > > + afp->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); > > if (IS_ERR(afp->reset)) > > return dev_err_probe(dev, PTR_ERR(afp->reset), "unable to get reset-gpios\n"); > > > > - afp->pwren = devm_gpiod_get_optional(dev, "pwren-gpios", GPIOD_OUT_LOW); > > + afp->pwren = devm_gpiod_get_optional(dev, "pwren", GPIOD_OUT_LOW); > > if (IS_ERR(afp->pwren)) > > return dev_err_probe(dev, PTR_ERR(afp->pwren), "unable to get pwren-gpios\n"); > > > > -- > > 2.34.1 > >
