Re: [PATCH v2] PM / devfreq: rk3399_dmc: Fix kernel oops when rockchip,pmu is absent

From: Marc Zyngier <maz@kernel.org>
To: Chanwoo Choi <cw00.choi@samsung.com>
Cc: "Heiko Stübner" <heiko@sntech.de>,
	"Enric Balletbo i Serra" <enric.balletbo@collabora.com>,
	"MyungJoo Ham" <myungjoo.ham@samsung.com>,
	"Kyungmin Park" <kyungmin.park@samsung.com>,
	linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
	kernel-team@android.com,
	"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: Re: [PATCH v2] PM / devfreq: rk3399_dmc: Fix kernel oops when rockchip,pmu is absent
Date: Mon, 29 Jun 2020 09:18:37 +0100	[thread overview]
Message-ID: <87tuyue142.wl-maz@kernel.org> (raw)
In-Reply-To: <784808d7-8943-44ab-f15a-34821e6d4d5f@samsung.com>

Hi Chanwoo,

On Mon, 29 Jun 2020 03:43:37 +0100,
Chanwoo Choi <cw00.choi@samsung.com> wrote:
> 
> Hi Marc,
> 
> On 6/23/20 12:28 AM, Marc Zyngier wrote:

[...]

> 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.

> 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.

> 
> 
>   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".

> 
>     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 <maz@kernel.org>
>     Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>

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.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.