| Message ID | 20220414074011.500533-2-herve.codina@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RZN1 USB Host support | expand |
Hi Hervé, Thanks for your patch! On Thu, Apr 14, 2022 at 9:40 AM Herve Codina <herve.codina@bootlin.com> wrote: > The PCI rcar-gen2 does not call any clk_prepare_enable(). Correct, this driver manages the clocks indirectly through Runtime PM. > This lead to an access failure when the driver tries to access > the IP (at least on a RZ/N1D platform). I expect adding power-domans = <&sysctrl>; to the pci_usb node makes this patch redundant. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, On Thu, 14 Apr 2022 10:45:54 +0200 Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Hervé, > > Thanks for your patch! > > On Thu, Apr 14, 2022 at 9:40 AM Herve Codina <herve.codina@bootlin.com> wrote: > > The PCI rcar-gen2 does not call any clk_prepare_enable(). > > Correct, this driver manages the clocks indirectly through Runtime PM. > > > This lead to an access failure when the driver tries to access > > the IP (at least on a RZ/N1D platform). > > I expect adding > > power-domans = <&sysctrl>; > > to the pci_usb node makes this patch redundant. Seems not enough. I tried what you suggest : - Added 'power-domains = <&systrl>;' to the pci_usb node - Added missing '#power-domain-cells = <0>;' to sysctrl node - Reverted my patch. The system crashed at boot: --- 8< --- ... [ 0.705309] loop: module loaded [ 0.709597] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver [ 0.716804] ehci-pci: EHCI PCI platform driver [ 0.721716] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver [ 0.728522] ohci-pci: OHCI PCI platform driver [ 0.733581] usbcore: registered new interface driver usb-storage [ 0.740458] ThumbEE CPU extension supported. [ 0.745093] Registering SWP/SWPB emulation handler [ 0.797518] rzn1-pinctrl 40067000.pinctrl: probed [ 0.803311] pci-rcar-gen2 40030000.pci: host bridge /soc/pci@40030000 ranges: [ 0.811255] pci-rcar-gen2 40030000.pci: MEM 0x0040020000..0x004002ffff -> 0x0040020000 [ 0.820373] pci-rcar-gen2 40030000.pci: IB MEM 0x0080000000..0x00bfffffff -> 0x0080000000 [ 0.829609] 8<--- cut here --- [ 0.832958] Unhandled fault: external abort on non-linefetch (0x1008) at 0x90b5f848 [ 0.841259] [90b5f848] *pgd=82149811, *pte=40030653, *ppte=40030453 [ 0.848093] Internal error: : 1008 [#1] SMP ARM [ 0.853024] Modules linked in: [ 0.856398] CPU: 0 PID: 31 Comm: kworker/u4:1 Not tainted 5.18.0-rc2-00009-g803ee9fd9fa5-dirty #5 [ 0.865998] Hardware name: Generic DT based system [ 0.871176] Workqueue: events_unbound deferred_probe_work_func [ 0.877539] PC is at rcar_pci_probe+0x15c/0x2f8 [ 0.882454] LR is at _raw_spin_unlock_irqrestore+0x24/0x2c [ 0.888434] pc : [<803ea428>] lr : [<804dc9b0>] psr: 60000013 [ 0.895193] sp : 90aa5e40 ip : 8217c4e0 fp : 00000000 [ 0.900857] r10: 80e7bd30 r9 : 80000000 r8 : 40000000 [ 0.906532] r7 : 80000000 r6 : 8217c410 r5 : 821d3400 r4 : 90b5f000 [ 0.913580] r3 : 00000009 r2 : 5c120fb6 r1 : 60000013 r0 : 00000000 [ 0.920646] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 0.928365] Control: 10c5387d Table: 8000406a DAC: 00000051 ... --- 8< --- I also added a trace printk in r9a06g032-clocks.c and r9a06g032_attach_dev() was never called. Did I miss to set something ? Regards, Hervé
Hi Hervé, On Thu, Apr 14, 2022 at 1:29 PM Herve Codina <herve.codina@bootlin.com> wrote: > On Thu, 14 Apr 2022 10:45:54 +0200 > Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Thu, Apr 14, 2022 at 9:40 AM Herve Codina <herve.codina@bootlin.com> wrote: > > > The PCI rcar-gen2 does not call any clk_prepare_enable(). > > > > Correct, this driver manages the clocks indirectly through Runtime PM. > > > > > This lead to an access failure when the driver tries to access > > > the IP (at least on a RZ/N1D platform). > > > > I expect adding > > > > power-domans = <&sysctrl>; > > > > to the pci_usb node makes this patch redundant. > > Seems not enough. > I tried what you suggest : > - Added 'power-domains = <&systrl>;' to the pci_usb node > - Added missing '#power-domain-cells = <0>;' to sysctrl node > - Reverted my patch. > > The system crashed at boot: > [ 0.832958] Unhandled fault: external abort on non-linefetch (0x1008) at 0x90b5f848 That's indeed a typical symptom of accessing a module's registers while the module's clock is disabled. > I also added a trace printk in r9a06g032-clocks.c and > r9a06g032_attach_dev() was never called. > > Did I miss to set something ? Do you have CONFIG_PM and CONFIG_PM_GENERIC_DOMAINS enabled? Apparently ARCH_RZN1 does not select these options yet. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, On Thu, 14 Apr 2022 13:48:22 +0200 Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Hervé, > > On Thu, Apr 14, 2022 at 1:29 PM Herve Codina <herve.codina@bootlin.com> wrote: > > On Thu, 14 Apr 2022 10:45:54 +0200 > > Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > On Thu, Apr 14, 2022 at 9:40 AM Herve Codina <herve.codina@bootlin.com> wrote: > > > > The PCI rcar-gen2 does not call any clk_prepare_enable(). > > > > > > Correct, this driver manages the clocks indirectly through Runtime PM. > > > > > > > This lead to an access failure when the driver tries to access > > > > the IP (at least on a RZ/N1D platform). > > > > > > I expect adding > > > > > > power-domans = <&sysctrl>; > > > > > > to the pci_usb node makes this patch redundant. > > > > Seems not enough. > > I tried what you suggest : > > - Added 'power-domains = <&systrl>;' to the pci_usb node > > - Added missing '#power-domain-cells = <0>;' to sysctrl node > > - Reverted my patch. > > > > The system crashed at boot: > > > [ 0.832958] Unhandled fault: external abort on non-linefetch (0x1008) at 0x90b5f848 > > That's indeed a typical symptom of accessing a module's registers > while the module's clock is disabled. > > > I also added a trace printk in r9a06g032-clocks.c and > > r9a06g032_attach_dev() was never called. > > > > Did I miss to set something ? > > Do you have CONFIG_PM and CONFIG_PM_GENERIC_DOMAINS > enabled? > Apparently ARCH_RZN1 does not select these options yet. > Thanks a lot for pointing this. I added select CONFIG_PM and CONFIG_PM_GENERIC_DOMAINS in ARCH_RZN1 and it works. I will remove my patch calling clk_bulk_prepare_enable() and add some new patches to enable power domains in the v3 series. Regards, Hervé
diff --git a/drivers/pci/controller/pci-rcar-gen2.c b/drivers/pci/controller/pci-rcar-gen2.c index 35804ea394fd..528bc3780e01 100644 --- a/drivers/pci/controller/pci-rcar-gen2.c +++ b/drivers/pci/controller/pci-rcar-gen2.c @@ -8,6 +8,7 @@ * Author: Valentine Barshak <valentine.barshak@cogentembedded.com> */ +#include <linux/clk.h> #include <linux/delay.h> #include <linux/init.h> #include <linux/interrupt.h> @@ -99,6 +100,8 @@ struct rcar_pci { struct resource mem_res; struct resource *cfg_res; int irq; + struct clk_bulk_data *clocks; + int nclocks; }; /* PCI configuration space operations */ @@ -282,6 +285,7 @@ static int rcar_pci_probe(struct platform_device *pdev) struct rcar_pci *priv; struct pci_host_bridge *bridge; void __iomem *reg; + int ret; bridge = devm_pci_alloc_host_bridge(dev, sizeof(*priv)); if (!bridge) @@ -305,13 +309,25 @@ static int rcar_pci_probe(struct platform_device *pdev) priv->mem_res = *mem_res; priv->cfg_res = cfg_res; + ret = devm_clk_bulk_get_all(dev, &priv->clocks); + if (ret < 0) { + dev_err(dev, "failed to get clocks %d\n", ret); + return ret; + } + priv->nclocks = ret; + + ret = clk_bulk_prepare_enable(priv->nclocks, priv->clocks); + if (ret) + return ret; + priv->irq = platform_get_irq(pdev, 0); priv->reg = reg; priv->dev = dev; if (priv->irq < 0) { dev_err(dev, "no valid irq found\n"); - return priv->irq; + ret = priv->irq; + goto disable_clocks; } bridge->ops = &rcar_pci_ops; @@ -320,7 +336,15 @@ static int rcar_pci_probe(struct platform_device *pdev) rcar_pci_setup(priv); - return pci_host_probe(bridge); + ret = pci_host_probe(bridge); + if (ret < 0) + goto disable_clocks; + + return 0; + +disable_clocks: + clk_bulk_disable_unprepare(priv->nclocks, priv->clocks); + return ret; } static const struct of_device_id rcar_pci_of_match[] = {
The PCI rcar-gen2 does not call any clk_prepare_enable(). This lead to an access failure when the driver tries to access the IP (at least on a RZ/N1D platform). Prepare and enable clocks using the bulk version of clk_prepare_enable() in order to prepare and enable all clocks attached to this device. Signed-off-by: Herve Codina <herve.codina@bootlin.com> --- drivers/pci/controller/pci-rcar-gen2.c | 28 ++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-)