linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] Add support rockchip,grf property for RK3399 PMU/GRU
@ 2017-01-10  6:15 Xing Zheng
  2017-01-10  6:15 ` [PATCH v4 1/2] arm64: dts: rockchip: add "rockchip, grf" property for RK3399 PMUCRU/CRU Xing Zheng
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Xing Zheng @ 2017-01-10  6:15 UTC (permalink / raw)
  To: heiko
  Cc: dianders, linux-rockchip, Xing Zheng, Douglas Anderson, David Wu,
	Jianqun Xu, devicetree, Stephen Boyd, Elaine Zhang, Brian Norris,
	Michael Turquette, linux-kernel, linux-clk, Shawn Lin,
	Rob Herring, Will Deacon, Mark Rutland, Caesar Wang,
	Catalin Marinas, linux-arm-kernel


Hi,
  The structure rockchip_clk_provider needs to refer the GRF regmap
in somewhere, if the CRU node has not "rockchip,grf" property,
calling syscon_regmap_lookup_by_phandle will return an invalid GRF
regmap, and the MUXGRF type clock will be not supported.

Therefore, we need to add them.

Thanks.


Changes in v4:
- separte the binding patch
- update the decription for rockchip,grf property

Changes in v3:
- add optional roperty rockchip,grf in rockchip,rk3399-cru.txt

Changes in v2:
- referring pmugrf for PMUGRU
- fix the typo "invaild" in COMMIT message

Xing Zheng (2):
  arm64: dts: rockchip: add "rockchip, grf" property for RK3399
    PMUCRU/CRU
  dt-bindings: clk: add rockchip,grf property for RK3399

 Documentation/devicetree/bindings/clock/rockchip,rk3399-cru.txt | 6 ++++++
 arch/arm64/boot/dts/rockchip/rk3399.dtsi                        | 2 ++
 2 files changed, 8 insertions(+)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4 1/2] arm64: dts: rockchip: add "rockchip, grf" property for RK3399 PMUCRU/CRU
  2017-01-10  6:15 [PATCH v4 0/2] Add support rockchip,grf property for RK3399 PMU/GRU Xing Zheng
@ 2017-01-10  6:15 ` Xing Zheng
  2017-01-10 18:45   ` Doug Anderson
  2017-01-10  6:15 ` [PATCH v4 2/2] dt-bindings: clk: add rockchip,grf property for RK3399 Xing Zheng
  2017-01-13 19:10 ` [PATCH v4 0/2] Add support rockchip,grf property for RK3399 PMU/GRU Heiko Stuebner
  2 siblings, 1 reply; 14+ messages in thread
From: Xing Zheng @ 2017-01-10  6:15 UTC (permalink / raw)
  To: heiko
  Cc: dianders, linux-rockchip, Xing Zheng, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, Caesar Wang, Douglas Anderson,
	Shawn Lin, Brian Norris, Jianqun Xu, Elaine Zhang, David Wu,
	devicetree, linux-arm-kernel, linux-kernel

The structure rockchip_clk_provider needs to refer the GRF regmap
in somewhere, if the CRU node has not "rockchip,grf" property,
calling syscon_regmap_lookup_by_phandle will return an invalid GRF
regmap, and the MUXGRF type clock will be not supported.

Therefore, we need to add them.

Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
---

Changes in v4:
- separte the binding patch

Changes in v3:
- add optional roperty rockchip,grf in rockchip,rk3399-cru.txt

Changes in v2:
- referring pmugrf for PMUGRU
- fix the typo "invaild" in COMMIT message

 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index c928015..081621b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1077,6 +1077,7 @@
 	pmucru: pmu-clock-controller@ff750000 {
 		compatible = "rockchip,rk3399-pmucru";
 		reg = <0x0 0xff750000 0x0 0x1000>;
+		rockchip,grf = <&pmugrf>;
 		#clock-cells = <1>;
 		#reset-cells = <1>;
 		assigned-clocks = <&pmucru PLL_PPLL>;
@@ -1086,6 +1087,7 @@
 	cru: clock-controller@ff760000 {
 		compatible = "rockchip,rk3399-cru";
 		reg = <0x0 0xff760000 0x0 0x1000>;
+		rockchip,grf = <&grf>;
 		#clock-cells = <1>;
 		#reset-cells = <1>;
 		assigned-clocks =
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 2/2] dt-bindings: clk: add rockchip,grf property for RK3399
  2017-01-10  6:15 [PATCH v4 0/2] Add support rockchip,grf property for RK3399 PMU/GRU Xing Zheng
  2017-01-10  6:15 ` [PATCH v4 1/2] arm64: dts: rockchip: add "rockchip, grf" property for RK3399 PMUCRU/CRU Xing Zheng
@ 2017-01-10  6:15 ` Xing Zheng
  2017-01-10 18:34   ` Doug Anderson
  2017-01-13 16:41   ` Rob Herring
  2017-01-13 19:10 ` [PATCH v4 0/2] Add support rockchip,grf property for RK3399 PMU/GRU Heiko Stuebner
  2 siblings, 2 replies; 14+ messages in thread
From: Xing Zheng @ 2017-01-10  6:15 UTC (permalink / raw)
  To: heiko
  Cc: dianders, linux-rockchip, Xing Zheng, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, linux-clk, devicetree,
	linux-arm-kernel, linux-kernel

Add support for rockchip,grf property which is used for GRF muxes
on RK3399.

Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
---

Changes in v4:
- update the decription for rockchip,grf property

Changes in v3: None
Changes in v2: None

 Documentation/devicetree/bindings/clock/rockchip,rk3399-cru.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/rockchip,rk3399-cru.txt b/Documentation/devicetree/bindings/clock/rockchip,rk3399-cru.txt
index 3888dd3..3bc56fa 100644
--- a/Documentation/devicetree/bindings/clock/rockchip,rk3399-cru.txt
+++ b/Documentation/devicetree/bindings/clock/rockchip,rk3399-cru.txt
@@ -13,6 +13,12 @@ Required Properties:
 - #clock-cells: should be 1.
 - #reset-cells: should be 1.
 
+Optional Properties:
+
+- rockchip,grf: phandle to the syscon managing the "general register files".
+  It is used for GRF muxes, if missing any muxes present in the GRF will not
+  be available.
+
 Each clock is assigned an identifier and client nodes can use this identifier
 to specify the clock which they consume. All available clocks are defined as
 preprocessor macros in the dt-bindings/clock/rk3399-cru.h headers and can be
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 2/2] dt-bindings: clk: add rockchip,grf property for RK3399
  2017-01-10  6:15 ` [PATCH v4 2/2] dt-bindings: clk: add rockchip,grf property for RK3399 Xing Zheng
@ 2017-01-10 18:34   ` Doug Anderson
  2017-01-13 16:41   ` Rob Herring
  1 sibling, 0 replies; 14+ messages in thread
From: Doug Anderson @ 2017-01-10 18:34 UTC (permalink / raw)
  To: Xing Zheng
  Cc: Heiko Stübner, open list:ARM/Rockchip SoC...,
	Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
	linux-clk, devicetree, linux-arm-kernel, linux-kernel

Hi,

On Mon, Jan 9, 2017 at 10:15 PM, Xing Zheng <zhengxing@rock-chips.com> wrote:
> Add support for rockchip,grf property which is used for GRF muxes
> on RK3399.
>
> Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
> ---
>
> Changes in v4:
> - update the decription for rockchip,grf property
>
> Changes in v3: None
> Changes in v2: None
>
>  Documentation/devicetree/bindings/clock/rockchip,rk3399-cru.txt | 6 ++++++
>  1 file changed, 6 insertions(+)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 1/2] arm64: dts: rockchip: add "rockchip, grf" property for RK3399 PMUCRU/CRU
  2017-01-10  6:15 ` [PATCH v4 1/2] arm64: dts: rockchip: add "rockchip, grf" property for RK3399 PMUCRU/CRU Xing Zheng
@ 2017-01-10 18:45   ` Doug Anderson
  2017-01-10 19:46     ` Heiko Stübner
  2017-01-11  1:04     ` Xing Zheng
  0 siblings, 2 replies; 14+ messages in thread
From: Doug Anderson @ 2017-01-10 18:45 UTC (permalink / raw)
  To: Xing Zheng
  Cc: Heiko Stübner, open list:ARM/Rockchip SoC...,
	Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Caesar Wang, Shawn Lin, Brian Norris, Jianqun Xu, Elaine Zhang,
	David Wu, devicetree, linux-arm-kernel, linux-kernel

Hi,

On Mon, Jan 9, 2017 at 10:15 PM, Xing Zheng <zhengxing@rock-chips.com> wrote:
> The structure rockchip_clk_provider needs to refer the GRF regmap
> in somewhere, if the CRU node has not "rockchip,grf" property,
> calling syscon_regmap_lookup_by_phandle will return an invalid GRF
> regmap, and the MUXGRF type clock will be not supported.
>
> Therefore, we need to add them.
>
> Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
> ---
>
> Changes in v4:
> - separte the binding patch
>
> Changes in v3:
> - add optional roperty rockchip,grf in rockchip,rk3399-cru.txt
>
> Changes in v2:
> - referring pmugrf for PMUGRU
> - fix the typo "invaild" in COMMIT message
>
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 2 ++
>  1 file changed, 2 insertions(+)

This seems fine to me, so:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

...but I will say that before you actually add any real "MUXGRF"
clocks on rk3399 you _might_ need to rework the code to make things
truly "optional".  If it turns out that any existing clocks that
already exist today already go through one of these muxes in the GRF
and we've always been assuming one setting of the mux, we'll need to
make sure we keep assuming that setting of the mux even if the "grf"
isn't specified.

As I understand it, your motivation for this patch is to eventually be
able to model the EDP reference clock which can either be xin24 or
"edp osc".  Presumably the eDP "reference clock" isn't related to any
of the pre-existing eDP clocks so that one should be safe.

-Doug

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 1/2] arm64: dts: rockchip: add "rockchip, grf" property for RK3399 PMUCRU/CRU
  2017-01-10 18:45   ` Doug Anderson
@ 2017-01-10 19:46     ` Heiko Stübner
  2017-01-10 19:58       ` Heiko Stübner
  2017-01-11  1:04     ` Xing Zheng
  1 sibling, 1 reply; 14+ messages in thread
From: Heiko Stübner @ 2017-01-10 19:46 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Xing Zheng, open list:ARM/Rockchip SoC...,
	Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Caesar Wang, Shawn Lin, Brian Norris, Jianqun Xu, Elaine Zhang,
	David Wu, devicetree, linux-arm-kernel, linux-kernel

Am Dienstag, 10. Januar 2017, 10:45:48 schrieb Doug Anderson:
> Hi,
> 
> On Mon, Jan 9, 2017 at 10:15 PM, Xing Zheng <zhengxing@rock-chips.com> 
wrote:
> > The structure rockchip_clk_provider needs to refer the GRF regmap
> > in somewhere, if the CRU node has not "rockchip,grf" property,
> > calling syscon_regmap_lookup_by_phandle will return an invalid GRF
> > regmap, and the MUXGRF type clock will be not supported.
> > 
> > Therefore, we need to add them.
> > 
> > Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
> > ---
> > 
> > Changes in v4:
> > - separte the binding patch
> > 
> > Changes in v3:
> > - add optional roperty rockchip,grf in rockchip,rk3399-cru.txt
> > 
> > Changes in v2:
> > - referring pmugrf for PMUGRU
> > - fix the typo "invaild" in COMMIT message
> > 
> >  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 2 ++
> >  1 file changed, 2 insertions(+)
> 
> This seems fine to me, so:
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> 
> ...but I will say that before you actually add any real "MUXGRF"
> clocks on rk3399 you _might_ need to rework the code to make things
> truly "optional".  If it turns out that any existing clocks that
> already exist today already go through one of these muxes in the GRF
> and we've always been assuming one setting of the mux, we'll need to
> make sure we keep assuming that setting of the mux even if the "grf"
> isn't specified.

I guess I see that a bit more relaxed :-) .

I.e. the GRF being optional is a remnant of syscons not being available when 
the clocks get set up- so were coming in later or not at all. For the rk3288 I 
converted, there we never really had the case of the GRF missing.

And the GRF mux for the vcodec now present is not being used by anything yet 
(neither driver nor binding), so no old devicetree can break.


> As I understand it, your motivation for this patch is to eventually be
> able to model the EDP reference clock which can either be xin24 or
> "edp osc".  Presumably the eDP "reference clock" isn't related to any
> of the pre-existing eDP clocks so that one should be safe.

Same here, so far we don't even have edp or even any other graphical output on 
the rk3399, so again there is no old devicetree that could break when the grf 
is not specified.


So, for me that looks good.


Heiko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 1/2] arm64: dts: rockchip: add "rockchip, grf" property for RK3399 PMUCRU/CRU
  2017-01-10 19:46     ` Heiko Stübner
@ 2017-01-10 19:58       ` Heiko Stübner
  2017-01-12  0:50         ` Doug Anderson
  0 siblings, 1 reply; 14+ messages in thread
From: Heiko Stübner @ 2017-01-10 19:58 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Xing Zheng, open list:ARM/Rockchip SoC...,
	Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Caesar Wang, Shawn Lin, Brian Norris, Jianqun Xu, Elaine Zhang,
	David Wu, devicetree, linux-arm-kernel, linux-kernel

Hi Doug,

Am Dienstag, 10. Januar 2017, 20:46:12 schrieb Heiko Stübner:
> Am Dienstag, 10. Januar 2017, 10:45:48 schrieb Doug Anderson:
> > Hi,
> > 
> > On Mon, Jan 9, 2017 at 10:15 PM, Xing Zheng <zhengxing@rock-chips.com>
> 
> wrote:
> > > The structure rockchip_clk_provider needs to refer the GRF regmap
> > > in somewhere, if the CRU node has not "rockchip,grf" property,
> > > calling syscon_regmap_lookup_by_phandle will return an invalid GRF
> > > regmap, and the MUXGRF type clock will be not supported.
> > > 
> > > Therefore, we need to add them.
> > > 
> > > Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
> > > ---
> > > 
> > > Changes in v4:
> > > - separte the binding patch
> > > 
> > > Changes in v3:
> > > - add optional roperty rockchip,grf in rockchip,rk3399-cru.txt
> > > 
> > > Changes in v2:
> > > - referring pmugrf for PMUGRU
> > > - fix the typo "invaild" in COMMIT message
> > > 
> > >  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 2 ++
> > >  1 file changed, 2 insertions(+)
> > 
> > This seems fine to me, so:
> > 
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > 
> > ...but I will say that before you actually add any real "MUXGRF"
> > clocks on rk3399 you _might_ need to rework the code to make things
> > truly "optional".  If it turns out that any existing clocks that
> > already exist today already go through one of these muxes in the GRF
> > and we've always been assuming one setting of the mux, we'll need to
> > make sure we keep assuming that setting of the mux even if the "grf"
> > isn't specified.
> 
> I guess I see that a bit more relaxed :-) .
> 
> I.e. the GRF being optional is a remnant of syscons not being available when
> the clocks get set up- so were coming in later or not at all. For the
> rk3288 I converted, there we never really had the case of the GRF missing.
> 
> And the GRF mux for the vcodec now present is not being used by anything yet
> (neither driver nor binding), so no old devicetree can break.
> 
> > As I understand it, your motivation for this patch is to eventually be
> > able to model the EDP reference clock which can either be xin24 or
> > "edp osc".  Presumably the eDP "reference clock" isn't related to any
> > of the pre-existing eDP clocks so that one should be safe.
> 
> Same here, so far we don't even have edp or even any other graphical output
> on the rk3399, so again there is no old devicetree that could break when
> the grf is not specified.

reading all of the above again, it feels like you essentially also said 
similar things already in your original reply and I misread some of it.

But again, I don't see the need for any more code right now, as hopefully the 
simple stuff we currently only support does not have any grf-based muxes in 
it. Xing + Rockchip people, please correct me if I'm wrong here :-)


Heiko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 1/2] arm64: dts: rockchip: add "rockchip, grf" property for RK3399 PMUCRU/CRU
  2017-01-10 18:45   ` Doug Anderson
  2017-01-10 19:46     ` Heiko Stübner
@ 2017-01-11  1:04     ` Xing Zheng
  2017-01-11  1:12       ` Heiko Stuebner
  1 sibling, 1 reply; 14+ messages in thread
From: Xing Zheng @ 2017-01-11  1:04 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Heiko Stübner, open list:ARM/Rockchip SoC...,
	Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Caesar Wang, Shawn Lin, Brian Norris, Jianqun Xu, Elaine Zhang,
	David Wu, devicetree, linux-arm-kernel, linux-kernel

Hi Doug,

On 2017年01月11日 02:45, Doug Anderson wrote:
> Hi,
>
> On Mon, Jan 9, 2017 at 10:15 PM, Xing Zheng <zhengxing@rock-chips.com> wrote:
>> The structure rockchip_clk_provider needs to refer the GRF regmap
>> in somewhere, if the CRU node has not "rockchip,grf" property,
>> calling syscon_regmap_lookup_by_phandle will return an invalid GRF
>> regmap, and the MUXGRF type clock will be not supported.
>>
>> Therefore, we need to add them.
>>
>> Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
>> ---
>>
>> Changes in v4:
>> - separte the binding patch
>>
>> Changes in v3:
>> - add optional roperty rockchip,grf in rockchip,rk3399-cru.txt
>>
>> Changes in v2:
>> - referring pmugrf for PMUGRU
>> - fix the typo "invaild" in COMMIT message
>>
>>   arch/arm64/boot/dts/rockchip/rk3399.dtsi | 2 ++
>>   1 file changed, 2 insertions(+)
> This seems fine to me, so:
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>
> ...but I will say that before you actually add any real "MUXGRF"
> clocks on rk3399 you _might_ need to rework the code to make things
> truly "optional".  If it turns out that any existing clocks that
> already exist today already go through one of these muxes in the GRF
> and we've always been assuming one setting of the mux, we'll need to
> make sure we keep assuming that setting of the mux even if the "grf"
> isn't specified.
>
> As I understand it, your motivation for this patch is to eventually be
> able to model the EDP reference clock which can either be xin24 or
> "edp osc".  Presumably the eDP "reference clock" isn't related to any
> of the pre-existing eDP clocks so that one should be safe.

Hmm... I had intended to use this patch for EDP reference clock, but we 
don't need to change the parent
clock (see the BUG: 61664).

I just woud like to add this patch to avoid getting some unavailable 
MUXGRF clock and need to debug it again,
if we support it one day in future.

Thanks.

-- 
- Xing Zheng

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 1/2] arm64: dts: rockchip: add "rockchip, grf" property for RK3399 PMUCRU/CRU
  2017-01-11  1:04     ` Xing Zheng
@ 2017-01-11  1:12       ` Heiko Stuebner
  2017-01-11  2:02         ` Xing Zheng
  0 siblings, 1 reply; 14+ messages in thread
From: Heiko Stuebner @ 2017-01-11  1:12 UTC (permalink / raw)
  To: Xing Zheng
  Cc: Doug Anderson, open list:ARM/Rockchip SoC...,
	Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Caesar Wang, Shawn Lin, Brian Norris, Jianqun Xu, Elaine Zhang,
	David Wu, devicetree, linux-arm-kernel, linux-kernel

Am Mittwoch, 11. Januar 2017, 09:04:24 CET schrieb Xing Zheng:
> Hi Doug,
> 
> On 2017年01月11日 02:45, Doug Anderson wrote:
> > Hi,
> > 
> > On Mon, Jan 9, 2017 at 10:15 PM, Xing Zheng <zhengxing@rock-chips.com> 
wrote:
> >> The structure rockchip_clk_provider needs to refer the GRF regmap
> >> in somewhere, if the CRU node has not "rockchip,grf" property,
> >> calling syscon_regmap_lookup_by_phandle will return an invalid GRF
> >> regmap, and the MUXGRF type clock will be not supported.
> >> 
> >> Therefore, we need to add them.
> >> 
> >> Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
> >> ---
> >> 
> >> Changes in v4:
> >> - separte the binding patch
> >> 
> >> Changes in v3:
> >> - add optional roperty rockchip,grf in rockchip,rk3399-cru.txt
> >> 
> >> Changes in v2:
> >> - referring pmugrf for PMUGRU
> >> - fix the typo "invaild" in COMMIT message
> >> 
> >>   arch/arm64/boot/dts/rockchip/rk3399.dtsi | 2 ++
> >>   1 file changed, 2 insertions(+)
> > 
> > This seems fine to me, so:
> > 
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > 
> > ...but I will say that before you actually add any real "MUXGRF"
> > clocks on rk3399 you _might_ need to rework the code to make things
> > truly "optional".  If it turns out that any existing clocks that
> > already exist today already go through one of these muxes in the GRF
> > and we've always been assuming one setting of the mux, we'll need to
> > make sure we keep assuming that setting of the mux even if the "grf"
> > isn't specified.
> > 
> > As I understand it, your motivation for this patch is to eventually be
> > able to model the EDP reference clock which can either be xin24 or
> > "edp osc".  Presumably the eDP "reference clock" isn't related to any
> > of the pre-existing eDP clocks so that one should be safe.
> 
> Hmm... I had intended to use this patch for EDP reference clock, but we
> don't need to change the parent
> clock (see the BUG: 61664).

Yep that sounds ok. As I said in my replies, we don't support the edp in the 
mainline kernel yet, so nothing old can break here :-)


> I just woud like to add this patch to avoid getting some unavailable
> MUXGRF clock and need to debug it again,
> if we support it one day in future.

could you just check, if we have any other grf-based muxes that we may need in 
the future. Right now I only see pclkin_isp1_wrapper describing such a mux, 
but it would be cool if you could check again.


Thanks
Heiko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 1/2] arm64: dts: rockchip: add "rockchip, grf" property for RK3399 PMUCRU/CRU
  2017-01-11  1:12       ` Heiko Stuebner
@ 2017-01-11  2:02         ` Xing Zheng
  0 siblings, 0 replies; 14+ messages in thread
From: Xing Zheng @ 2017-01-11  2:02 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Doug Anderson, open list:ARM/Rockchip SoC...,
	Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Caesar Wang, Shawn Lin, Brian Norris, Jianqun Xu, Elaine Zhang,
	David Wu, devicetree, linux-arm-kernel, linux-kernel

Hi Heiko,

On 2017年01月11日 09:12, Heiko Stuebner wrote:
>> I just woud like to add this patch to avoid getting some unavailable
>> MUXGRF clock and need to debug it again,
>> if we support it one day in future.
> could you just check, if we have any other grf-based muxes that we may need in
> the future. Right now I only see pclkin_isp1_wrapper describing such a mux,
> but it would be cool if you could check again.
>
>
OK, we will check these.  :)

Thanks.

-- 
- Xing Zheng

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 1/2] arm64: dts: rockchip: add "rockchip, grf" property for RK3399 PMUCRU/CRU
  2017-01-10 19:58       ` Heiko Stübner
@ 2017-01-12  0:50         ` Doug Anderson
  2017-01-12  1:28           ` Heiko Stuebner
  0 siblings, 1 reply; 14+ messages in thread
From: Doug Anderson @ 2017-01-12  0:50 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: Xing Zheng, open list:ARM/Rockchip SoC...,
	Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Caesar Wang, Shawn Lin, Brian Norris, Jianqun Xu, Elaine Zhang,
	David Wu, devicetree, linux-arm-kernel, linux-kernel

Hi,

On Tue, Jan 10, 2017 at 11:58 AM, Heiko Stübner <heiko@sntech.de> wrote:
> Hi Doug,
>
> Am Dienstag, 10. Januar 2017, 20:46:12 schrieb Heiko Stübner:
>> Am Dienstag, 10. Januar 2017, 10:45:48 schrieb Doug Anderson:
>> > Hi,
>> >
>> > On Mon, Jan 9, 2017 at 10:15 PM, Xing Zheng <zhengxing@rock-chips.com>
>>
>> wrote:
>> > > The structure rockchip_clk_provider needs to refer the GRF regmap
>> > > in somewhere, if the CRU node has not "rockchip,grf" property,
>> > > calling syscon_regmap_lookup_by_phandle will return an invalid GRF
>> > > regmap, and the MUXGRF type clock will be not supported.
>> > >
>> > > Therefore, we need to add them.
>> > >
>> > > Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
>> > > ---
>> > >
>> > > Changes in v4:
>> > > - separte the binding patch
>> > >
>> > > Changes in v3:
>> > > - add optional roperty rockchip,grf in rockchip,rk3399-cru.txt
>> > >
>> > > Changes in v2:
>> > > - referring pmugrf for PMUGRU
>> > > - fix the typo "invaild" in COMMIT message
>> > >
>> > >  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 2 ++
>> > >  1 file changed, 2 insertions(+)
>> >
>> > This seems fine to me, so:
>> >
>> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
>> >
>> > ...but I will say that before you actually add any real "MUXGRF"
>> > clocks on rk3399 you _might_ need to rework the code to make things
>> > truly "optional".  If it turns out that any existing clocks that
>> > already exist today already go through one of these muxes in the GRF
>> > and we've always been assuming one setting of the mux, we'll need to
>> > make sure we keep assuming that setting of the mux even if the "grf"
>> > isn't specified.
>>
>> I guess I see that a bit more relaxed :-) .
>>
>> I.e. the GRF being optional is a remnant of syscons not being available when
>> the clocks get set up- so were coming in later or not at all. For the
>> rk3288 I converted, there we never really had the case of the GRF missing.
>>
>> And the GRF mux for the vcodec now present is not being used by anything yet
>> (neither driver nor binding), so no old devicetree can break.
>>
>> > As I understand it, your motivation for this patch is to eventually be
>> > able to model the EDP reference clock which can either be xin24 or
>> > "edp osc".  Presumably the eDP "reference clock" isn't related to any
>> > of the pre-existing eDP clocks so that one should be safe.
>>
>> Same here, so far we don't even have edp or even any other graphical output
>> on the rk3399, so again there is no old devicetree that could break when
>> the grf is not specified.
>
> reading all of the above again, it feels like you essentially also said
> similar things already in your original reply and I misread some of it.
>
> But again, I don't see the need for any more code right now, as hopefully the
> simple stuff we currently only support does not have any grf-based muxes in
> it. Xing + Rockchip people, please correct me if I'm wrong here :-)

Right.  I have no objection to Xing's patch.  I just want to make sure
that if it's listed as "Optional" that it's really optional.

I was worried that we would introduce some GRF-based mux in the
_middle_ of some existing clock tree because we simply didn't model
the mux before and assumed one particular setting.  If nothing like
that ever happens then we're fine.

Sorry to be so confusing.

-Doug

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 1/2] arm64: dts: rockchip: add "rockchip, grf" property for RK3399 PMUCRU/CRU
  2017-01-12  0:50         ` Doug Anderson
@ 2017-01-12  1:28           ` Heiko Stuebner
  0 siblings, 0 replies; 14+ messages in thread
From: Heiko Stuebner @ 2017-01-12  1:28 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Xing Zheng, open list:ARM/Rockchip SoC...,
	Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Caesar Wang, Shawn Lin, Brian Norris, Jianqun Xu, Elaine Zhang,
	David Wu, devicetree, linux-arm-kernel, linux-kernel

Am Mittwoch, 11. Januar 2017, 16:50:10 CET schrieb Doug Anderson:
> Hi,
> 
> On Tue, Jan 10, 2017 at 11:58 AM, Heiko Stübner <heiko@sntech.de> wrote:
> > Hi Doug,
> > 
> > Am Dienstag, 10. Januar 2017, 20:46:12 schrieb Heiko Stübner:
> >> Am Dienstag, 10. Januar 2017, 10:45:48 schrieb Doug Anderson:
> >> > Hi,
> >> > 
> >> > On Mon, Jan 9, 2017 at 10:15 PM, Xing Zheng <zhengxing@rock-chips.com>
> >> 
> >> wrote:
> >> > > The structure rockchip_clk_provider needs to refer the GRF regmap
> >> > > in somewhere, if the CRU node has not "rockchip,grf" property,
> >> > > calling syscon_regmap_lookup_by_phandle will return an invalid GRF
> >> > > regmap, and the MUXGRF type clock will be not supported.
> >> > > 
> >> > > Therefore, we need to add them.
> >> > > 
> >> > > Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
> >> > > ---
> >> > > 
> >> > > Changes in v4:
> >> > > - separte the binding patch
> >> > > 
> >> > > Changes in v3:
> >> > > - add optional roperty rockchip,grf in rockchip,rk3399-cru.txt
> >> > > 
> >> > > Changes in v2:
> >> > > - referring pmugrf for PMUGRU
> >> > > - fix the typo "invaild" in COMMIT message
> >> > > 
> >> > >  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 2 ++
> >> > >  1 file changed, 2 insertions(+)
> >> > 
> >> > This seems fine to me, so:
> >> > 
> >> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> >> > 
> >> > ...but I will say that before you actually add any real "MUXGRF"
> >> > clocks on rk3399 you _might_ need to rework the code to make things
> >> > truly "optional".  If it turns out that any existing clocks that
> >> > already exist today already go through one of these muxes in the GRF
> >> > and we've always been assuming one setting of the mux, we'll need to
> >> > make sure we keep assuming that setting of the mux even if the "grf"
> >> > isn't specified.
> >> 
> >> I guess I see that a bit more relaxed :-) .
> >> 
> >> I.e. the GRF being optional is a remnant of syscons not being available
> >> when the clocks get set up- so were coming in later or not at all. For
> >> the rk3288 I converted, there we never really had the case of the GRF
> >> missing.
> >> 
> >> And the GRF mux for the vcodec now present is not being used by anything
> >> yet (neither driver nor binding), so no old devicetree can break.
> >> 
> >> > As I understand it, your motivation for this patch is to eventually be
> >> > able to model the EDP reference clock which can either be xin24 or
> >> > "edp osc".  Presumably the eDP "reference clock" isn't related to any
> >> > of the pre-existing eDP clocks so that one should be safe.
> >> 
> >> Same here, so far we don't even have edp or even any other graphical
> >> output
> >> on the rk3399, so again there is no old devicetree that could break when
> >> the grf is not specified.
> > 
> > reading all of the above again, it feels like you essentially also said
> > similar things already in your original reply and I misread some of it.
> > 
> > But again, I don't see the need for any more code right now, as hopefully
> > the simple stuff we currently only support does not have any grf-based
> > muxes in it. Xing + Rockchip people, please correct me if I'm wrong here
> > :-)
> Right.  I have no objection to Xing's patch.  I just want to make sure
> that if it's listed as "Optional" that it's really optional.
> 
> I was worried that we would introduce some GRF-based mux in the
> _middle_ of some existing clock tree because we simply didn't model
> the mux before and assumed one particular setting.  If nothing like
> that ever happens then we're fine.

Thankfully the clock diagrams on the old socs were pretty verbose, listing all
grf-based clocks as well. For example the rk3288 seems to have two and it
seems I've been carrying dummy definitions for those from the time I did the
initial clock tree [0].

And thankfully grf-based clocks somehow always only get used for more
"esotheric" components like vcodec and iep on the rk3288 :-) .


Heiko

[0] https://github.com/mmind/linux-rockchip/blob/devel/workbench/drivers/clk/rockchip/clk-rk3288.c
lines 371 and 799 .. looks like I'll need to add the iep clock as well
at some point.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 2/2] dt-bindings: clk: add rockchip,grf property for RK3399
  2017-01-10  6:15 ` [PATCH v4 2/2] dt-bindings: clk: add rockchip,grf property for RK3399 Xing Zheng
  2017-01-10 18:34   ` Doug Anderson
@ 2017-01-13 16:41   ` Rob Herring
  1 sibling, 0 replies; 14+ messages in thread
From: Rob Herring @ 2017-01-13 16:41 UTC (permalink / raw)
  To: Xing Zheng
  Cc: heiko, dianders, linux-rockchip, Michael Turquette, Stephen Boyd,
	Mark Rutland, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel

On Tue, Jan 10, 2017 at 02:15:30PM +0800, Xing Zheng wrote:
> Add support for rockchip,grf property which is used for GRF muxes
> on RK3399.
> 
> Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
> ---
> 
> Changes in v4:
> - update the decription for rockchip,grf property
> 
> Changes in v3: None
> Changes in v2: None
> 
>  Documentation/devicetree/bindings/clock/rockchip,rk3399-cru.txt | 6 ++++++
>  1 file changed, 6 insertions(+)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 0/2] Add support rockchip,grf property for RK3399 PMU/GRU
  2017-01-10  6:15 [PATCH v4 0/2] Add support rockchip,grf property for RK3399 PMU/GRU Xing Zheng
  2017-01-10  6:15 ` [PATCH v4 1/2] arm64: dts: rockchip: add "rockchip, grf" property for RK3399 PMUCRU/CRU Xing Zheng
  2017-01-10  6:15 ` [PATCH v4 2/2] dt-bindings: clk: add rockchip,grf property for RK3399 Xing Zheng
@ 2017-01-13 19:10 ` Heiko Stuebner
  2 siblings, 0 replies; 14+ messages in thread
From: Heiko Stuebner @ 2017-01-13 19:10 UTC (permalink / raw)
  To: Xing Zheng
  Cc: dianders, linux-rockchip, Douglas Anderson, David Wu, Jianqun Xu,
	devicetree, Stephen Boyd, Elaine Zhang, Brian Norris,
	Michael Turquette, linux-kernel, linux-clk, Shawn Lin,
	Rob Herring, Will Deacon, Mark Rutland, Caesar Wang,
	Catalin Marinas, linux-arm-kernel

Am Dienstag, 10. Januar 2017, 14:15:28 CET schrieb Xing Zheng:
> Hi,
>   The structure rockchip_clk_provider needs to refer the GRF regmap
> in somewhere, if the CRU node has not "rockchip,grf" property,
> calling syscon_regmap_lookup_by_phandle will return an invalid GRF
> regmap, and the MUXGRF type clock will be not supported.
> 
> Therefore, we need to add them.
> 
> Thanks.

applied both for 4.11

binding-change is in my clock-branch (so that dt-binding doc changes stay 
together) and the dts change is of course in the dts64 branch.


Thanks
Heiko

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2017-01-13 19:11 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-10  6:15 [PATCH v4 0/2] Add support rockchip,grf property for RK3399 PMU/GRU Xing Zheng
2017-01-10  6:15 ` [PATCH v4 1/2] arm64: dts: rockchip: add "rockchip, grf" property for RK3399 PMUCRU/CRU Xing Zheng
2017-01-10 18:45   ` Doug Anderson
2017-01-10 19:46     ` Heiko Stübner
2017-01-10 19:58       ` Heiko Stübner
2017-01-12  0:50         ` Doug Anderson
2017-01-12  1:28           ` Heiko Stuebner
2017-01-11  1:04     ` Xing Zheng
2017-01-11  1:12       ` Heiko Stuebner
2017-01-11  2:02         ` Xing Zheng
2017-01-10  6:15 ` [PATCH v4 2/2] dt-bindings: clk: add rockchip,grf property for RK3399 Xing Zheng
2017-01-10 18:34   ` Doug Anderson
2017-01-13 16:41   ` Rob Herring
2017-01-13 19:10 ` [PATCH v4 0/2] Add support rockchip,grf property for RK3399 PMU/GRU Heiko Stuebner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).