Hi Enric,

Could you check this issue? Your patch[1] causes this issue.
As Marc mentioned, although rk3399-dmc.c handled 'rockchip,pmu'
as the mandatory property, your patch[1] didn't add the 'rockchip,pmu'
property to the documentation. 

[1] 9173c5ceb035 ("PM / devfreq: rk3399_dmc: Pass ODT
and auto power down parameters to TF-A.")


On 6/29/20 5:18 PM, Marc Zyngier wrote:

Hi Chanwoo,

On Mon, 29 Jun 2020 03:43:37 +0100,
Chanwoo Choi [off-list ref] wrote:

quoted

Hi Marc,

On 6/23/20 12:28 AM, Marc Zyngier wrote:

[...]

quoted

It looks good to me. But, I think that it is not necessary
fully kernel panic log about NULL pointer. It is enoughspsp
just mentioning the NULL pointer issue without full kernel panic log.

I personally find the backtrace useful as it allows people with the
same issue to trawl the kernel log and find whether it has already be
fixed upstream. But it's only me, and I'm not attached to it.

quoted

So, how about editing the patch description as following or others simply?
and we need to add 'stable@vger.kernel.org' to Cc list for applying it
to stable branch.

Looks good to me.

quoted

  PM / devfreq: rk3399_dmc: Fix kernel oops when rockchip,pmu is absent

    Booting a recent kernel on a rk3399-based system (nanopc-t4),
    equipped with a recent u-boot and ATF results in the kernel panic
    about NULL pointer issue.

nit: "results in a kernel panic on dereferencing a NULL pointer".

quoted

    This turns out to be due to the rk3399-dmc driver looking for
    an *undocumented* property (rockchip,pmu), and happily using
    a NULL pointer when the property isn't there.

    Instead, make most of what was brought in with 9173c5ceb035
    ("PM / devfreq: rk3399_dmc: Pass ODT and auto power down parameters
    to TF-A.") conditioned on finding this property in the device-tree,
    preventing the driver from exploding.

    Fixes: 9173c5ceb035 ("PM / devfreq: rk3399_dmc: Pass ODT and auto power down parameters to TF-A.")
    Signed-off-by: Marc Zyngier [off-list ref]
    Signed-off-by: Chanwoo Choi [off-list ref]

Note that the biggest issue is still there: the driver is using an
undocumented property, and this patch is just papering over it.
Since I expect this property to be useful for something, it would be
good for whoever knows what it does to document it.

Hi Marc,

You are right. We have to do two step:
1. Add missing explanation of 'rockchip,pmu' property to dt-binding document
2. If possible, add 'rockchip,pmu' property node to rk3399_dmc dt node.

When I tried to find usage example of 'rockchip,pmu' property,
I found them as following: The 'rockchip,pmu' property[2] indicates
'PMU (Power Management Unit)'. 

$ grep -rn "rockchip,pmu" arch/arm64/boot/dts/
arch/arm64/boot/dts/rockchip/px30.dtsi:1211:		rockchip,pmu = <&pmugrf>;
arch/arm64/boot/dts/rockchip/rk3399.dtsi:1909:		rockchip,pmu = <&pmugrf>;
arch/arm64/boot/dts/rockchip/rk3368.dtsi:807:		rockchip,pmu = <&pmugrf>;

[2] the description of 'rockchip,pmu' property
- https://elixir.bootlin.com/linux/v5.7.2/source/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.txt#L40


If don't receive the any reply, I'll add as following:

cwchoi00@chan-linux-pc:~/kernel/git.kernel/linux.chanwoo$ d

diff --git a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
index 0ec68141f85a..161e60ea874b 100644
--- a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
+++ b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt

@@ -18,6 +18,8 @@ Optional properties:
                         format depends on the interrupt controller.
                         It should be a DCF interrupt. When DDR DVFS finishes
                         a DCF interrupt is triggered.
+- rockchip,pmu:                 Phandle to the syscon managing the "pmu general
+                        register files".
 
 Following properties relate to DDR timing:
 

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics