Re: [PATCH] thermal: rockchip: shut up GRF warning

[Sebastian Reichel]

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

Attachment: signature.asc
Description: PGP signature