| Message ID | 20200819135707.15486-2-pali@kernel.org |
|---|---|
| State | Accepted |
| Commit | 828d32621686aec593076d16445d39b9b8d49c05 |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | [1/2] arm64: a37xx: pci: Make PCIe Reset GPIO DT compatible with Linux kernel DT | expand |
On 19.08.20 15:57, Pali Rohár wrote: > This change ensures that PCIe card is put into reset state when U-Boot > stops using it. > > DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback > prior booting Linux kernel. > > Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal > prior initializing it. If it does not issue reset then some PCIe cards > (specially Compex WiFi cards) are not detected at all. > > Putting PCIe card into reset state prior booting Linux kernel would ensure > that card would be properly reset at time when Linux kernel starts > initializing pci-aardvark driver. > > Signed-off-by: Pali Rohár <pali@kernel.org> > --- > drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------ > 1 file changed, 21 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c > index 5b3f23c184..8996be5309 100644 > --- a/drivers/pci/pci-aardvark.c > +++ b/drivers/pci/pci-aardvark.c > @@ -148,6 +148,9 @@ struct pcie_advk { > void *base; > int first_busno; > struct udevice *dev; > +#if CONFIG_IS_ENABLED(DM_GPIO) > + struct gpio_desc reset_gpio; > +#endif > }; Adding more #ifdef's is not recommended. Can't you "depend" this driver on DM_GPIO in Kconfig instead? Are there any used that don't support DM_GPIO right now? > static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg) > @@ -614,9 +617,7 @@ static int pcie_advk_probe(struct udevice *dev) > struct pcie_advk *pcie = dev_get_priv(dev); > > #if CONFIG_IS_ENABLED(DM_GPIO) If you change the driver to rely on DM_GPIO, then please also remove all other #ifdef's with DM_GPIO. Thanks, Stefan > - struct gpio_desc reset_gpio; > - > - gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio, > + gpio_request_by_name(dev, "reset-gpios", 0, &pcie->reset_gpio, > GPIOD_IS_OUT); > /* > * Issue reset to add-in card through the dedicated GPIO. > @@ -631,11 +632,11 @@ static int pcie_advk_probe(struct udevice *dev) > * possible before PCIe PHY initialization. Moreover, the PCIe > * clock should be gated as well. > */ > - if (dm_gpio_is_valid(&reset_gpio)) { > + if (dm_gpio_is_valid(&pcie->reset_gpio)) { > dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n"); > - dm_gpio_set_value(&reset_gpio, 1); > + dm_gpio_set_value(&pcie->reset_gpio, 1); > mdelay(200); > - dm_gpio_set_value(&reset_gpio, 0); > + dm_gpio_set_value(&pcie->reset_gpio, 0); > } > #else > dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n"); > @@ -647,6 +648,18 @@ static int pcie_advk_probe(struct udevice *dev) > return pcie_advk_setup_hw(pcie); > } > > +static int pcie_advk_remove(struct udevice *dev) > +{ > +#if CONFIG_IS_ENABLED(DM_GPIO) > + struct pcie_advk *pcie = dev_get_priv(dev); > + > + if (dm_gpio_is_valid(&pcie->reset_gpio)) > + dm_gpio_set_value(&pcie->reset_gpio, 1); > +#endif /* DM_GPIO */ > + > + return 0; > +} > + > /** > * pcie_advk_ofdata_to_platdata() - Translate from DT to device state > * > @@ -687,5 +700,7 @@ U_BOOT_DRIVER(pcie_advk) = { > .ops = &pcie_advk_ops, > .ofdata_to_platdata = pcie_advk_ofdata_to_platdata, > .probe = pcie_advk_probe, > + .remove = pcie_advk_remove, > + .flags = DM_FLAG_OS_PREPARE, > .priv_auto_alloc_size = sizeof(struct pcie_advk), > }; > Viele Grüße, Stefan
On Thursday 20 August 2020 07:05:58 Stefan Roese wrote: > On 19.08.20 15:57, Pali Rohár wrote: > > This change ensures that PCIe card is put into reset state when U-Boot > > stops using it. > > > > DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback > > prior booting Linux kernel. > > > > Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal > > prior initializing it. If it does not issue reset then some PCIe cards > > (specially Compex WiFi cards) are not detected at all. > > > > Putting PCIe card into reset state prior booting Linux kernel would ensure > > that card would be properly reset at time when Linux kernel starts > > initializing pci-aardvark driver. > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > --- > > drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------ > > 1 file changed, 21 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c > > index 5b3f23c184..8996be5309 100644 > > --- a/drivers/pci/pci-aardvark.c > > +++ b/drivers/pci/pci-aardvark.c > > @@ -148,6 +148,9 @@ struct pcie_advk { > > void *base; > > int first_busno; > > struct udevice *dev; > > +#if CONFIG_IS_ENABLED(DM_GPIO) > > + struct gpio_desc reset_gpio; > > +#endif > > }; > > Adding more #ifdef's is not recommended. Can't you "depend" this driver > on DM_GPIO in Kconfig instead? Are there any used that don't support > DM_GPIO right now? I'm not sure if this dependency is what people want. CCing Simon. In past Simon created commit bcee8d6764f9215f16b393a35581000178633254 where described that want to build SPL without GPIO uclass support. > > static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg) > > @@ -614,9 +617,7 @@ static int pcie_advk_probe(struct udevice *dev) > > struct pcie_advk *pcie = dev_get_priv(dev); > > #if CONFIG_IS_ENABLED(DM_GPIO) > > If you change the driver to rely on DM_GPIO, then please also remove all > other #ifdef's with DM_GPIO. > > Thanks, > Stefan > > > - struct gpio_desc reset_gpio; > > - > > - gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio, > > + gpio_request_by_name(dev, "reset-gpios", 0, &pcie->reset_gpio, > > GPIOD_IS_OUT); > > /* > > * Issue reset to add-in card through the dedicated GPIO. > > @@ -631,11 +632,11 @@ static int pcie_advk_probe(struct udevice *dev) > > * possible before PCIe PHY initialization. Moreover, the PCIe > > * clock should be gated as well. > > */ > > - if (dm_gpio_is_valid(&reset_gpio)) { > > + if (dm_gpio_is_valid(&pcie->reset_gpio)) { > > dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n"); > > - dm_gpio_set_value(&reset_gpio, 1); > > + dm_gpio_set_value(&pcie->reset_gpio, 1); > > mdelay(200); > > - dm_gpio_set_value(&reset_gpio, 0); > > + dm_gpio_set_value(&pcie->reset_gpio, 0); > > } > > #else > > dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n"); > > @@ -647,6 +648,18 @@ static int pcie_advk_probe(struct udevice *dev) > > return pcie_advk_setup_hw(pcie); > > } > > +static int pcie_advk_remove(struct udevice *dev) > > +{ > > +#if CONFIG_IS_ENABLED(DM_GPIO) > > + struct pcie_advk *pcie = dev_get_priv(dev); > > + > > + if (dm_gpio_is_valid(&pcie->reset_gpio)) > > + dm_gpio_set_value(&pcie->reset_gpio, 1); > > +#endif /* DM_GPIO */ > > + > > + return 0; > > +} > > + > > /** > > * pcie_advk_ofdata_to_platdata() - Translate from DT to device state > > * > > @@ -687,5 +700,7 @@ U_BOOT_DRIVER(pcie_advk) = { > > .ops = &pcie_advk_ops, > > .ofdata_to_platdata = pcie_advk_ofdata_to_platdata, > > .probe = pcie_advk_probe, > > + .remove = pcie_advk_remove, > > + .flags = DM_FLAG_OS_PREPARE, > > .priv_auto_alloc_size = sizeof(struct pcie_advk), > > }; > > > > > Viele Grüße, > Stefan > > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
On 20.08.20 09:43, Pali Rohár wrote: > On Thursday 20 August 2020 07:05:58 Stefan Roese wrote: >> On 19.08.20 15:57, Pali Rohár wrote: >>> This change ensures that PCIe card is put into reset state when U-Boot >>> stops using it. >>> >>> DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback >>> prior booting Linux kernel. >>> >>> Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal >>> prior initializing it. If it does not issue reset then some PCIe cards >>> (specially Compex WiFi cards) are not detected at all. >>> >>> Putting PCIe card into reset state prior booting Linux kernel would ensure >>> that card would be properly reset at time when Linux kernel starts >>> initializing pci-aardvark driver. >>> >>> Signed-off-by: Pali Rohár <pali@kernel.org> >>> --- >>> drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------ >>> 1 file changed, 21 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c >>> index 5b3f23c184..8996be5309 100644 >>> --- a/drivers/pci/pci-aardvark.c >>> +++ b/drivers/pci/pci-aardvark.c >>> @@ -148,6 +148,9 @@ struct pcie_advk { >>> void *base; >>> int first_busno; >>> struct udevice *dev; >>> +#if CONFIG_IS_ENABLED(DM_GPIO) >>> + struct gpio_desc reset_gpio; >>> +#endif >>> }; >> >> Adding more #ifdef's is not recommended. Can't you "depend" this driver >> on DM_GPIO in Kconfig instead? Are there any used that don't support >> DM_GPIO right now? > > I'm not sure if this dependency is what people want. CCing Simon. > > In past Simon created commit bcee8d6764f9215f16b393a35581000178633254 > where described that want to build SPL without GPIO uclass support. Is this PCI driver ever built into SPL? Thanks, Stefan
On Thursday 20 August 2020 09:48:17 Stefan Roese wrote: > On 20.08.20 09:43, Pali Rohár wrote: > > On Thursday 20 August 2020 07:05:58 Stefan Roese wrote: > > > On 19.08.20 15:57, Pali Rohár wrote: > > > > This change ensures that PCIe card is put into reset state when U-Boot > > > > stops using it. > > > > > > > > DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback > > > > prior booting Linux kernel. > > > > > > > > Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal > > > > prior initializing it. If it does not issue reset then some PCIe cards > > > > (specially Compex WiFi cards) are not detected at all. > > > > > > > > Putting PCIe card into reset state prior booting Linux kernel would ensure > > > > that card would be properly reset at time when Linux kernel starts > > > > initializing pci-aardvark driver. > > > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > > --- > > > > drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------ > > > > 1 file changed, 21 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c > > > > index 5b3f23c184..8996be5309 100644 > > > > --- a/drivers/pci/pci-aardvark.c > > > > +++ b/drivers/pci/pci-aardvark.c > > > > @@ -148,6 +148,9 @@ struct pcie_advk { > > > > void *base; > > > > int first_busno; > > > > struct udevice *dev; > > > > +#if CONFIG_IS_ENABLED(DM_GPIO) > > > > + struct gpio_desc reset_gpio; > > > > +#endif > > > > }; > > > > > > Adding more #ifdef's is not recommended. Can't you "depend" this driver > > > on DM_GPIO in Kconfig instead? Are there any used that don't support > > > DM_GPIO right now? > > > > I'm not sure if this dependency is what people want. CCing Simon. > > > > In past Simon created commit bcee8d6764f9215f16b393a35581000178633254 > > where described that want to build SPL without GPIO uclass support. > > Is this PCI driver ever built into SPL? SPL is not used for Espressobin and Turris MOX. But I'm not sure for other boards. Maybe Simon could provide more information about it. Basically I added GPIO handling in aardvark driver as it was used on other place.
Hi Kosta, this one as well please. ;) Thanks, Stefan On 19.08.20 19:31, Kostya Porotchkin wrote: > > > ________________________________________ > From: Pali Rohár <pali@kernel.org> > Sent: Wednesday, August 19, 2020 16:57 > To: Stefan Roese; Kostya Porotchkin > Cc: u-boot@lists.denx.de > Subject: [EXT] [PATCH 2/2] arm64: a37xx: pci: Assert PERST# signal when unloading driver > > External Email > > ---------------------------------------------------------------------- > This change ensures that PCIe card is put into reset state when U-Boot > stops using it. > > DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback > prior booting Linux kernel. > > Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal > prior initializing it. If it does not issue reset then some PCIe cards > (specially Compex WiFi cards) are not detected at all. > > Putting PCIe card into reset state prior booting Linux kernel would ensure > that card would be properly reset at time when Linux kernel starts > initializing pci-aardvark driver. > > Signed-off-by: Pali Rohár <pali@kernel.org> > Reviewed-by: Konstantin Porotchkin <kostap@marvell.com> > --- > drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------ > 1 file changed, 21 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c > index 5b3f23c184..8996be5309 100644 > --- a/drivers/pci/pci-aardvark.c > +++ b/drivers/pci/pci-aardvark.c > @@ -148,6 +148,9 @@ struct pcie_advk { > void *base; > int first_busno; > struct udevice *dev; > +#if CONFIG_IS_ENABLED(DM_GPIO) > + struct gpio_desc reset_gpio; > +#endif > }; > > static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg) > @@ -614,9 +617,7 @@ static int pcie_advk_probe(struct udevice *dev) > struct pcie_advk *pcie = dev_get_priv(dev); > > #if CONFIG_IS_ENABLED(DM_GPIO) > - struct gpio_desc reset_gpio; > - > - gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio, > + gpio_request_by_name(dev, "reset-gpios", 0, &pcie->reset_gpio, > GPIOD_IS_OUT); > /* > * Issue reset to add-in card through the dedicated GPIO. > @@ -631,11 +632,11 @@ static int pcie_advk_probe(struct udevice *dev) > * possible before PCIe PHY initialization. Moreover, the PCIe > * clock should be gated as well. > */ > - if (dm_gpio_is_valid(&reset_gpio)) { > + if (dm_gpio_is_valid(&pcie->reset_gpio)) { > dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n"); > - dm_gpio_set_value(&reset_gpio, 1); > + dm_gpio_set_value(&pcie->reset_gpio, 1); > mdelay(200); > - dm_gpio_set_value(&reset_gpio, 0); > + dm_gpio_set_value(&pcie->reset_gpio, 0); > } > #else > dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n"); > @@ -647,6 +648,18 @@ static int pcie_advk_probe(struct udevice *dev) > return pcie_advk_setup_hw(pcie); > } > > +static int pcie_advk_remove(struct udevice *dev) > +{ > +#if CONFIG_IS_ENABLED(DM_GPIO) > + struct pcie_advk *pcie = dev_get_priv(dev); > + > + if (dm_gpio_is_valid(&pcie->reset_gpio)) > + dm_gpio_set_value(&pcie->reset_gpio, 1); > +#endif /* DM_GPIO */ > + > + return 0; > +} > + > /** > * pcie_advk_ofdata_to_platdata() - Translate from DT to device state > * > @@ -687,5 +700,7 @@ U_BOOT_DRIVER(pcie_advk) = { > .ops = &pcie_advk_ops, > .ofdata_to_platdata = pcie_advk_ofdata_to_platdata, > .probe = pcie_advk_probe, > + .remove = pcie_advk_remove, > + .flags = DM_FLAG_OS_PREPARE, > .priv_auto_alloc_size = sizeof(struct pcie_advk), > }; > -- > 2.20.1 > Viele Grüße, Stefan
On 20.08.20 09:52, Pali Rohár wrote: > On Thursday 20 August 2020 09:48:17 Stefan Roese wrote: >> On 20.08.20 09:43, Pali Rohár wrote: >>> On Thursday 20 August 2020 07:05:58 Stefan Roese wrote: >>>> On 19.08.20 15:57, Pali Rohár wrote: >>>>> This change ensures that PCIe card is put into reset state when U-Boot >>>>> stops using it. >>>>> >>>>> DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback >>>>> prior booting Linux kernel. >>>>> >>>>> Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal >>>>> prior initializing it. If it does not issue reset then some PCIe cards >>>>> (specially Compex WiFi cards) are not detected at all. >>>>> >>>>> Putting PCIe card into reset state prior booting Linux kernel would ensure >>>>> that card would be properly reset at time when Linux kernel starts >>>>> initializing pci-aardvark driver. >>>>> >>>>> Signed-off-by: Pali Rohár <pali@kernel.org> >>>>> --- >>>>> drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------ >>>>> 1 file changed, 21 insertions(+), 6 deletions(-) >>>>> >>>>> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c >>>>> index 5b3f23c184..8996be5309 100644 >>>>> --- a/drivers/pci/pci-aardvark.c >>>>> +++ b/drivers/pci/pci-aardvark.c >>>>> @@ -148,6 +148,9 @@ struct pcie_advk { >>>>> void *base; >>>>> int first_busno; >>>>> struct udevice *dev; >>>>> +#if CONFIG_IS_ENABLED(DM_GPIO) >>>>> + struct gpio_desc reset_gpio; >>>>> +#endif >>>>> }; >>>> >>>> Adding more #ifdef's is not recommended. Can't you "depend" this driver >>>> on DM_GPIO in Kconfig instead? Are there any used that don't support >>>> DM_GPIO right now? >>> >>> I'm not sure if this dependency is what people want. CCing Simon. >>> >>> In past Simon created commit bcee8d6764f9215f16b393a35581000178633254 >>> where described that want to build SPL without GPIO uclass support. >> >> Is this PCI driver ever built into SPL? > > SPL is not used for Espressobin and Turris MOX. But I'm not sure for > other boards. Maybe Simon could provide more information about it. AFAIK (I did not work on Armada for quite some time), only 32bit Armada uses SPL. All 64bit Aramda 37xx / 7xxx / 8xxx do not use SPL. > Basically I added GPIO handling in aardvark driver as it was used on > other place. I understand. I suggest to "depend" this driver on DM_GPIO and include the GPIO code without any #ifdef's. And please run a world-build (buildman, Travis...) to see, if nothing breaks. Thanks, Stefan
> -----Original Message----- > From: Pali Rohár <pali@kernel.org> > Sent: Wednesday, August 19, 2020 16:57 > To: Stefan Roese <sr@denx.de>; Kostya Porotchkin <kostap@marvell.com> > Cc: u-boot@lists.denx.de > Subject: [EXT] [PATCH 2/2] arm64: a37xx: pci: Assert PERST# signal when > unloading driver > > External Email > > ---------------------------------------------------------------------- > This change ensures that PCIe card is put into reset state when U-Boot stops > using it. > > DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove > callback prior booting Linux kernel. > > Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal > prior initializing it. If it does not issue reset then some PCIe cards (specially > Compex WiFi cards) are not detected at all. > > Putting PCIe card into reset state prior booting Linux kernel would ensure > that card would be properly reset at time when Linux kernel starts initializing > pci-aardvark driver. > > Signed-off-by: Pali Rohár <pali@kernel.org> Reviewed-by: Konstantin Porotchkin <kostap@marvell.com> > --- > drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------ > 1 file changed, 21 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c index > 5b3f23c184..8996be5309 100644 > --- a/drivers/pci/pci-aardvark.c > +++ b/drivers/pci/pci-aardvark.c > @@ -148,6 +148,9 @@ struct pcie_advk { > void *base; > int first_busno; > struct udevice *dev; > +#if CONFIG_IS_ENABLED(DM_GPIO) > + struct gpio_desc reset_gpio; > +#endif > }; > > static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg) @@ - > 614,9 +617,7 @@ static int pcie_advk_probe(struct udevice *dev) > struct pcie_advk *pcie = dev_get_priv(dev); > > #if CONFIG_IS_ENABLED(DM_GPIO) > - struct gpio_desc reset_gpio; > - > - gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio, > + gpio_request_by_name(dev, "reset-gpios", 0, &pcie->reset_gpio, > GPIOD_IS_OUT); > /* > * Issue reset to add-in card through the dedicated GPIO. > @@ -631,11 +632,11 @@ static int pcie_advk_probe(struct udevice *dev) > * possible before PCIe PHY initialization. Moreover, the PCIe > * clock should be gated as well. > */ > - if (dm_gpio_is_valid(&reset_gpio)) { > + if (dm_gpio_is_valid(&pcie->reset_gpio)) { > dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n"); > - dm_gpio_set_value(&reset_gpio, 1); > + dm_gpio_set_value(&pcie->reset_gpio, 1); > mdelay(200); > - dm_gpio_set_value(&reset_gpio, 0); > + dm_gpio_set_value(&pcie->reset_gpio, 0); > } > #else > dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n"); @@ > -647,6 +648,18 @@ static int pcie_advk_probe(struct udevice *dev) > return pcie_advk_setup_hw(pcie); > } > > +static int pcie_advk_remove(struct udevice *dev) { #if > +CONFIG_IS_ENABLED(DM_GPIO) > + struct pcie_advk *pcie = dev_get_priv(dev); > + > + if (dm_gpio_is_valid(&pcie->reset_gpio)) > + dm_gpio_set_value(&pcie->reset_gpio, 1); #endif /* > DM_GPIO */ > + > + return 0; > +} > + > /** > * pcie_advk_ofdata_to_platdata() - Translate from DT to device state > * > @@ -687,5 +700,7 @@ U_BOOT_DRIVER(pcie_advk) = { > .ops = &pcie_advk_ops, > .ofdata_to_platdata = pcie_advk_ofdata_to_platdata, > .probe = pcie_advk_probe, > + .remove = pcie_advk_remove, > + .flags = DM_FLAG_OS_PREPARE, > .priv_auto_alloc_size = sizeof(struct pcie_advk), > }; > -- > 2.20.1
On Thursday 20 August 2020 10:00:52 Stefan Roese wrote: > I suggest to "depend" this driver on DM_GPIO and include the GPIO code > without any #ifdef's. And please run a world-build (buildman, Travis...) > to see, if nothing breaks. I sent third patch which removes those #ifdef's and run CI testing on Github: https://github.com/u-boot/u-boot/pull/37 Everything passed.
On 25.08.20 10:46, Pali Rohár wrote: > On Thursday 20 August 2020 10:00:52 Stefan Roese wrote: >> I suggest to "depend" this driver on DM_GPIO and include the GPIO code >> without any #ifdef's. And please run a world-build (buildman, Travis...) >> to see, if nothing breaks. > > I sent third patch which removes those #ifdef's and run CI testing on > Github: https://github.com/u-boot/u-boot/pull/37 > > Everything passed. Perfect. ;) Thanks, Stefan
On 19/08/2020 15:57, Pali Rohár wrote: > This change ensures that PCIe card is put into reset state when U-Boot > stops using it. > > DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback > prior booting Linux kernel. > > Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal > prior initializing it. If it does not issue reset then some PCIe cards > (specially Compex WiFi cards) are not detected at all. > > Putting PCIe card into reset state prior booting Linux kernel would ensure > that card would be properly reset at time when Linux kernel starts > initializing pci-aardvark driver. > > Signed-off-by: Pali Rohár <pali@kernel.org> Tested-by: Andre Heider <a.heider@gmail.com>
On 19.08.20 15:57, Pali Rohár wrote: > This change ensures that PCIe card is put into reset state when U-Boot > stops using it. > > DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback > prior booting Linux kernel. > > Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal > prior initializing it. If it does not issue reset then some PCIe cards > (specially Compex WiFi cards) are not detected at all. > > Putting PCIe card into reset state prior booting Linux kernel would ensure > that card would be properly reset at time when Linux kernel starts > initializing pci-aardvark driver. > > Signed-off-by: Pali Rohár <pali@kernel.org> Applied to u-boot-marvell/master Thanks, Stefan > --- > drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------ > 1 file changed, 21 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c > index 5b3f23c184..8996be5309 100644 > --- a/drivers/pci/pci-aardvark.c > +++ b/drivers/pci/pci-aardvark.c > @@ -148,6 +148,9 @@ struct pcie_advk { > void *base; > int first_busno; > struct udevice *dev; > +#if CONFIG_IS_ENABLED(DM_GPIO) > + struct gpio_desc reset_gpio; > +#endif > }; > > static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg) > @@ -614,9 +617,7 @@ static int pcie_advk_probe(struct udevice *dev) > struct pcie_advk *pcie = dev_get_priv(dev); > > #if CONFIG_IS_ENABLED(DM_GPIO) > - struct gpio_desc reset_gpio; > - > - gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio, > + gpio_request_by_name(dev, "reset-gpios", 0, &pcie->reset_gpio, > GPIOD_IS_OUT); > /* > * Issue reset to add-in card through the dedicated GPIO. > @@ -631,11 +632,11 @@ static int pcie_advk_probe(struct udevice *dev) > * possible before PCIe PHY initialization. Moreover, the PCIe > * clock should be gated as well. > */ > - if (dm_gpio_is_valid(&reset_gpio)) { > + if (dm_gpio_is_valid(&pcie->reset_gpio)) { > dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n"); > - dm_gpio_set_value(&reset_gpio, 1); > + dm_gpio_set_value(&pcie->reset_gpio, 1); > mdelay(200); > - dm_gpio_set_value(&reset_gpio, 0); > + dm_gpio_set_value(&pcie->reset_gpio, 0); > } > #else > dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n"); > @@ -647,6 +648,18 @@ static int pcie_advk_probe(struct udevice *dev) > return pcie_advk_setup_hw(pcie); > } > > +static int pcie_advk_remove(struct udevice *dev) > +{ > +#if CONFIG_IS_ENABLED(DM_GPIO) > + struct pcie_advk *pcie = dev_get_priv(dev); > + > + if (dm_gpio_is_valid(&pcie->reset_gpio)) > + dm_gpio_set_value(&pcie->reset_gpio, 1); > +#endif /* DM_GPIO */ > + > + return 0; > +} > + > /** > * pcie_advk_ofdata_to_platdata() - Translate from DT to device state > * > @@ -687,5 +700,7 @@ U_BOOT_DRIVER(pcie_advk) = { > .ops = &pcie_advk_ops, > .ofdata_to_platdata = pcie_advk_ofdata_to_platdata, > .probe = pcie_advk_probe, > + .remove = pcie_advk_remove, > + .flags = DM_FLAG_OS_PREPARE, > .priv_auto_alloc_size = sizeof(struct pcie_advk), > }; > Viele Grüße, Stefan
diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c index 5b3f23c184..8996be5309 100644 --- a/drivers/pci/pci-aardvark.c +++ b/drivers/pci/pci-aardvark.c @@ -148,6 +148,9 @@ struct pcie_advk { void *base; int first_busno; struct udevice *dev; +#if CONFIG_IS_ENABLED(DM_GPIO) + struct gpio_desc reset_gpio; +#endif }; static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg) @@ -614,9 +617,7 @@ static int pcie_advk_probe(struct udevice *dev) struct pcie_advk *pcie = dev_get_priv(dev); #if CONFIG_IS_ENABLED(DM_GPIO) - struct gpio_desc reset_gpio; - - gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio, + gpio_request_by_name(dev, "reset-gpios", 0, &pcie->reset_gpio, GPIOD_IS_OUT); /* * Issue reset to add-in card through the dedicated GPIO. @@ -631,11 +632,11 @@ static int pcie_advk_probe(struct udevice *dev) * possible before PCIe PHY initialization. Moreover, the PCIe * clock should be gated as well. */ - if (dm_gpio_is_valid(&reset_gpio)) { + if (dm_gpio_is_valid(&pcie->reset_gpio)) { dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n"); - dm_gpio_set_value(&reset_gpio, 1); + dm_gpio_set_value(&pcie->reset_gpio, 1); mdelay(200); - dm_gpio_set_value(&reset_gpio, 0); + dm_gpio_set_value(&pcie->reset_gpio, 0); } #else dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n"); @@ -647,6 +648,18 @@ static int pcie_advk_probe(struct udevice *dev) return pcie_advk_setup_hw(pcie); } +static int pcie_advk_remove(struct udevice *dev) +{ +#if CONFIG_IS_ENABLED(DM_GPIO) + struct pcie_advk *pcie = dev_get_priv(dev); + + if (dm_gpio_is_valid(&pcie->reset_gpio)) + dm_gpio_set_value(&pcie->reset_gpio, 1); +#endif /* DM_GPIO */ + + return 0; +} + /** * pcie_advk_ofdata_to_platdata() - Translate from DT to device state * @@ -687,5 +700,7 @@ U_BOOT_DRIVER(pcie_advk) = { .ops = &pcie_advk_ops, .ofdata_to_platdata = pcie_advk_ofdata_to_platdata, .probe = pcie_advk_probe, + .remove = pcie_advk_remove, + .flags = DM_FLAG_OS_PREPARE, .priv_auto_alloc_size = sizeof(struct pcie_advk), };
This change ensures that PCIe card is put into reset state when U-Boot stops using it. DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback prior booting Linux kernel. Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal prior initializing it. If it does not issue reset then some PCIe cards (specially Compex WiFi cards) are not detected at all. Putting PCIe card into reset state prior booting Linux kernel would ensure that card would be properly reset at time when Linux kernel starts initializing pci-aardvark driver. Signed-off-by: Pali Rohár <pali@kernel.org> --- drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-)