linux-kernel - Re: [PATCH] thermal: rockchip: shut up GRF warning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ciuhfx4u6xs2g43hn7nzjfmhaakfip6ndcpftuvg3kgzf52zkt@3vyvcjflmtqm>
Date: Tue, 19 Aug 2025 15:56:42 +0200
From: Sebastian Reichel <sebastian.reichel@...labora.com>
To: Robin Murphy <robin.murphy@....com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, 
	Daniel Lezcano <daniel.lezcano@...aro.org>, Zhang Rui <rui.zhang@...el.com>, 
	Lukasz Luba <lukasz.luba@....com>, Heiko Stuebner <heiko@...ech.de>, linux-pm@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	kernel@...labora.com
Subject: Re: [PATCH] thermal: rockchip: shut up GRF warning

Hello Robin,

On Tue, Aug 19, 2025 at 01:42:42PM +0100, Robin Murphy wrote:
> > +	if (thermal->chip->grf_mode != GRF_NONE) {
> > +		thermal->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
> > +		if (IS_ERR(thermal->grf)) {
> > +			ret = PTR_ERR(thermal->grf);
> > +			if (thermal->chip->grf_mode == GRF_OPTIONAL)
> > +				dev_warn(dev, "Missing rockchip,grf property\n");
> > +			else
> > +				return dev_err_probe(dev, ret, "Missing rockchip,grf property\n");
> > +		}
> > +	}
> 
> Nit: Does the lookup itself need to be made conditional? I think I'd
> also agree that the "optional" mode seems suspect, so potentially it
> could be a whole lot simpler, e.g.:
> 
> 	thermal->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
> 	if (IS_ERR(thermal->grf) && thermal->chip->grf_required)
> 		return dev_err_probe(dev, PTR_ERR(thermal->grf),
> 				     "Missing rockchip,grf property\n");

I came up with the enum, because I think most platforms having
"optional" GRF support actually require it, so I want to keep the
warning. At the same time I don't want to mark them as GRF required
at this point, since that is potentially a DT ABI break. It really
needs to be checked per platform in the TRM and/or by testing on
real HW. With this patch we can easily handle this platform by
platform by moving them from GRF_OPTIONAL to GRF_MANDATORY without
affecting the unchecked platforms. Also switching a platform from
optional to required needs to be reflected in the DT binding. So
this involves a lot of work. I think it makes sense to carry the
slightly complex check in the driver's probe routine for now.

Greetings,

-- Sebastian

> 
> Thanks,
> Robin.
> 
> >   	rockchip_get_trim_configuration(dev, np, thermal);
> > 
> > ---
> > base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
> > change-id: 20250818-thermal-rockchip-grf-warning-05f7f56286a2
> > 
> > Best regards,
> 
> 

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)