RE: [PATCH] PCI: rcar-gen2: Ensure the mandatory clock is enabled

[Gareth Williams]

Hi Geert,

I will take a look into how I can do this. Thanks for taking the time to give
some background and pointers.

Kind Regards,

Gareth

On Fri, Mar 22, 2019 at 19:14 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: 
> Hi Gareth, Phil,
> 
> Thanks for your patch!
> 
> On Fri, Mar 22, 2019 at 5:11 PM Gareth Williams
> <gareth.williams.jx@xxxxxxxxxxx> wrote:
> > From: Phil Edworthy <phil.edworthy@xxxxxxxxxxx>
> >
> > All SoC devices that use this driver have a module stop clock
> > associated with it that we must ensure is enabled. All SoCs enabled
> > this clock by default, so I guess no one noticed that the driver didn't enable
> it.
> 
> This is not really correct: the module clock has always been controlled
> through the clock domain the device is part of, either through the "power-
> domains" property in DT, or through the legacy clock domain before that,
> combined with Runtime PM (see also commit fb178d8b2fab3f2a
> ("PCI: rcar: Add runtime PM support")).
> 
> Furthermore adding explicit clock management (with a mandatory clock)
> makes it harder to use the device with virtualization and VFIO, and to reuse
> the driver on other SoCs where no module clock may be present.
> Using Runtime PM allows to abstract the presence of the module clock, or
> whatever power saving feature that may be present (e.g. a power area),
> which is an integration feature, and not a feature of the actual device.
> 
> So I don't think this change is needed or wanted.
> 
> NAKed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> 
> > Note: This change was tested on RZN1 and R-Car M2 hardware. NULL is
> > passed to devm_clk_get for backwards compatibility reasons with R-Car
> > gen2 devices which do not name the clock.
> >
> > DTS files for Renesas devices that use this driver have been checked
> > to ensure they specify a clock and do not name that clock.
> 
> I understand RZ/N1 has the same PCI controller, and you're trying to add
> support for it?  I think the solution there is to add clock domain support to
> drivers/clk/renesas/r9a06g032-clocks.c.
> 
> 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