Quoting Geert Uytterhoeven (2021-09-30 01:01:24)
> Hi Stephen,
>
> On Thu, Sep 30, 2021 at 7:30 AM Stephen Boyd <sboyd@xxxxxxxxxx> wrote:
> > +linux-clk as I don't regularly read my inbox :/
> >
> > Quoting marek.vasut@xxxxxxxxx (2021-09-07 07:45:12)
> > > From: Marek Vasut <marek.vasut+renesas@xxxxxxxxx>
> > >
> > > Add COMMON_CLK dependency, otherwise the following build error occurs:
> > > arm-linux-gnueabi-ld: drivers/pci/controller/pcie-rcar-host.o: in function `rcar_pcie_aarch32_abort_handler':
> > > pcie-rcar-host.c:(.text+0xdd0): undefined reference to `__clk_is_enabled'
> > > This should be OK, since all platforms shipping this controller also
> > > need COMMON_CLK enabled for their clock driver.
> > >
> > > Fixes: a115b1bd3af0 ("PCI: rcar: Add L1 link state fix into data abort hook")
> > > Signed-off-by: Marek Vasut <marek.vasut+renesas@xxxxxxxxx>
> > > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> > > Cc: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
> > > Cc: Stephen Boyd <sboyd@xxxxxxxxxx>
> > > Cc: Wolfram Sang <wsa@xxxxxxxxxxxxx>
> > > Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
> > > Cc: linux-renesas-soc@xxxxxxxxxxxxxxx
> > > ---
> > > +CC Stephen, please double-check whether this is the right approach or
> > > whether there is some better option
> >
> > Stop using __clk_is_enabled()? I don't quite understand what's going on in
> > the code but __clk_is_enabled() should really go away. I thought we were
> > close to doing that but now I see a handful of calls have come up. The
> > API should be replaced by clk_hw_is_enabled() and then removed. We move
> > it to clk_hw API so that only clk providers can look at it.
>
> But this is not a clk provider...
>
> > Sigh!
>
> ;-)
Exactly!
>
> > Anyway, fixing the dependency is "ok" but really the long term fix would
> > be to not use a "is this clk enabled" sort of API. If I'm reading the
> > code correctly, this is some sort of fault handler that's trying to
> > avoid hanging the bus while handling the fault so it tries to make sure
> > the clk is enabled first? Is it a problem if the clk is not actually
> > enabled here? Would runtime PM enable state of the device work just as
> > well?
>
> Thanks, checking Runtime PM state is a good suggestion. Doing so
> would require caching a pointer to the PCIe struct device instead of
> the struct clk.
> However, pcie_bus_clk is not the module clock, which is managed by
> Runtime PM, but the PCIe bus clock, which is managed explicitly by
> the driver.
> However, I believe that we are checking the wrong clock, as register
> access needs the module clock to be enabled, not the PCIe bus clock?
> As the driver just calls pm_runtime_get_sync() and clk_prepare_enable()
> in .probe(), and never touches Runtime PM status or the PCIe bus clock
> during the further lifetime of the driver (it cannot be unloaded), both
> the module clock and the PCIe bus clock should always[*] be enabled
> when the static copy of the remapped PCIe controller address is valid.
> [*] Modulo system-wide power transitions like s2ram. Does
> pm_runtime_suspended() reflect that state, too?
>
Great! If that's all correct then simply removing the call to
__clk_is_enabled() should work. Can we do that?
