| Message ID | 1450336895-4279-3-git-send-email-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | Deferred |
| Headers | show |
Hi Laurent, On Thu, Dec 17, 2015 at 8:21 AM, Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> wrote: > diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi > index 9ce6a5ea6629..47f97ffc6c8b 100644 > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi > @@ -775,5 +775,119 @@ > clocks = <&cpg CPG_MOD 915>; > status = "disabled"; > }; > + > + vspbc: vsp@fe920000 { > + compatible = "renesas,vsp2"; > + reg = <0 0xfe920000 0 0x8000>; > + interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 624>; No "power-domains = <&cpg_clocks>;"? Each vsp device node corresponds nicely to one vsp module, and the driver doesn't care at all about the clock, except for (explicit) PM of the vsp modules. That would allow to simplify vsp1_device_get(), vsp1_pm_suspend(), and vsp1_pm_resume(), and get rid of vsp1_device_put() in favor of pm_runtime_put(). 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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Geert, On Tuesday 22 December 2015 14:43:34 Geert Uytterhoeven wrote: > On Thu, Dec 17, 2015 at 8:21 AM, Laurent Pinchart wrote: > > diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi > > b/arch/arm64/boot/dts/renesas/r8a7795.dtsi index > > 9ce6a5ea6629..47f97ffc6c8b 100644 > > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi > > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi > > @@ -775,5 +775,119 @@ > > clocks = <&cpg CPG_MOD 915>; > > status = "disabled"; > > }; > > + > > + vspbc: vsp@fe920000 { > > + compatible = "renesas,vsp2"; > > + reg = <0 0xfe920000 0 0x8000>; > > + interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&cpg CPG_MOD 624>; > > No "power-domains = <&cpg_clocks>;"? > Each vsp device node corresponds nicely to one vsp module, and the driver > doesn't care at all about the clock, except for (explicit) PM of the vsp > modules. > > That would allow to simplify vsp1_device_get(), vsp1_pm_suspend(), and > vsp1_pm_resume(), and get rid of vsp1_device_put() in favor of > pm_runtime_put(). I'm all for simplification :-) I'll try to find time to move the driver to runtime PM. What will happen if the power domain is specified in DT but the driver doesn't use runtime PM yet ?
Hi Laurent, On Sun, Dec 27, 2015 at 9:20 AM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > On Tuesday 22 December 2015 14:43:34 Geert Uytterhoeven wrote: >> On Thu, Dec 17, 2015 at 8:21 AM, Laurent Pinchart wrote: >> > diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi >> > b/arch/arm64/boot/dts/renesas/r8a7795.dtsi index >> > 9ce6a5ea6629..47f97ffc6c8b 100644 >> > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi >> > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi >> > @@ -775,5 +775,119 @@ >> > clocks = <&cpg CPG_MOD 915>; >> > status = "disabled"; >> > }; >> > + >> > + vspbc: vsp@fe920000 { >> > + compatible = "renesas,vsp2"; >> > + reg = <0 0xfe920000 0 0x8000>; >> > + interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>; >> > + clocks = <&cpg CPG_MOD 624>; >> >> No "power-domains = <&cpg_clocks>;"? >> Each vsp device node corresponds nicely to one vsp module, and the driver >> doesn't care at all about the clock, except for (explicit) PM of the vsp >> modules. >> >> That would allow to simplify vsp1_device_get(), vsp1_pm_suspend(), and >> vsp1_pm_resume(), and get rid of vsp1_device_put() in favor of >> pm_runtime_put(). > > I'm all for simplification :-) I'll try to find time to move the driver to > runtime PM. What will happen if the power domain is specified in DT but the Thanks! > driver doesn't use runtime PM yet ? The device will be added to the PM Domain, but the PM Domain will not be turned on, and the gpd_dev_ops.start() callback will never be called for the device. You need to call pm_runtime_enable() and pm_runtime_get_sync() to do that. The CPG/MSSR PM Domain is a Clock Domain, so the driver can choose to manage the device's modulo clock itself instead of relying on the gpd_dev_ops.start() callback. But if a VSP IP core ever ends up in an SoC with a hardware Power Domain[*], the hardware Power Domain may not be managed correctly. Unlike for clocks, you cannot escape proper Runtime PM when dealing with hardware Power Domains ;-) Note that the vsp nodes in r8a7790.dtsi and r8a7791.dtsi already have power-domains properties. [*] Is VSP1 in R-Mobile APE6, which has hardware Power Domains) compatible with VSP1 in R-Car Gen2? 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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi index 9ce6a5ea6629..47f97ffc6c8b 100644 --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi @@ -775,5 +775,119 @@ clocks = <&cpg CPG_MOD 915>; status = "disabled"; }; + + vspbc: vsp@fe920000 { + compatible = "renesas,vsp2"; + reg = <0 0xfe920000 0 0x8000>; + interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 624>; + + renesas,has-lut; + renesas,has-sru; + renesas,#rpf = <5>; + renesas,#wpf = <1>; + status = "disabled"; + }; + + vspbd: vsp@fe960000 { + compatible = "renesas,vsp2"; + reg = <0 0xfe960000 0 0x8000>; + interrupts = <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 626>; + + renesas,#rpf = <5>; + renesas,#wpf = <1>; + status = "disabled"; + }; + + vspi0: vsp@fe9a0000 { + compatible = "renesas,vsp2"; + reg = <0 0xfe9a0000 0 0x8000>; + interrupts = <GIC_SPI 444 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 631>; + + renesas,has-lut; + renesas,has-sru; + renesas,#rpf = <1>; + renesas,#uds = <3>; + renesas,#wpf = <1>; + status = "disabled"; + }; + + vspi1: vsp@fe9b0000 { + compatible = "renesas,vsp2"; + reg = <0 0xfe9b0000 0 0x8000>; + interrupts = <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 630>; + + renesas,has-lut; + renesas,has-sru; + renesas,#rpf = <1>; + renesas,#uds = <1>; + renesas,#wpf = <1>; + status = "disabled"; + }; + + vspi2: vsp@fe9c0000 { + compatible = "renesas,vsp2"; + reg = <0 0xfe9c0000 0 0x8000>; + interrupts = <GIC_SPI 446 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 629>; + + renesas,has-lut; + renesas,has-sru; + renesas,#rpf = <1>; + renesas,#uds = <1>; + renesas,#wpf = <1>; + status = "disabled"; + }; + + vspd0: vsp@fea20000 { + compatible = "renesas,vsp2"; + reg = <0 0xfea20000 0 0x8000>; + interrupts = <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 623>; + + renesas,has-lif; + renesas,has-lut; + renesas,#rpf = <5>; + renesas,#wpf = <2>; + }; + + vspd1: vsp@fea28000 { + compatible = "renesas,vsp2"; + reg = <0 0xfea28000 0 0x8000>; + interrupts = <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 622>; + + renesas,has-lif; + renesas,has-lut; + renesas,#rpf = <5>; + renesas,#wpf = <2>; + }; + + vspd2: vsp@fea30000 { + compatible = "renesas,vsp2"; + reg = <0 0xfea30000 0 0x8000>; + interrupts = <GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 621>; + + renesas,has-lif; + renesas,has-lut; + renesas,#rpf = <5>; + renesas,#wpf = <2>; + }; + + vspd3: vsp@fea38000 { + compatible = "renesas,vsp2"; + reg = <0 0xfea38000 0 0x8000>; + interrupts = <GIC_SPI 469 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 620>; + + renesas,has-lif; + renesas,has-lut; + renesas,#rpf = <5>; + renesas,#wpf = <2>; + }; }; };
The r8a7795 has 9 VSP instances with various capabilities. Only the VSPD instances are currently enabled as the other 5 instances cause the following crash when reading the version register. [ 5.284206] Bad mode in Error handler detected, code 0xbf000002 -- SError [ 5.287590] CPU: 0 PID: 518 Comm: mdev Not tainted 4.4.0-rc3+ #162 [ 5.290666] Hardware name: Renesas Salvator-X board based on r8a7795 (DT) [ 5.294044] task: ffffffc039a8b700 ti: ffffffc038db4000 task.ti: ffffffc038db4000 [ 5.297770] PC is at 0x9dd0 [ 5.299160] LR is at 0x0 [ 5.300420] pc : [<0000000000009dd0>] lr : [<0000000000000000>] pstate: 00000010 [ 5.304101] sp : 00000000ffd94300 [ 5.305750] x12: 0000000000000000 [ 5.307446] x11: 0000000000000000 x10: 0000000000000000 [ 5.310096] x9 : 0000000000000000 x8 : 0000000000000000 [ 5.312745] x7 : 0000000000000000 x6 : 0000000000000000 [ 5.315395] x5 : 0000000000000000 x4 : 0000000000000000 [ 5.318044] x3 : 0000000000000000 x2 : 0000000000000000 [ 5.320694] x1 : 0000000000000000 x0 : 0000000000000000 Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- arch/arm64/boot/dts/renesas/r8a7795.dtsi | 114 +++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+)