Re: [PATCH v3 2/5] thermal/drivers: airoha: Generalize probe function

[Christian Marangi <ansuelsmth@gmail.com>]

On Tue, Nov 25, 2025 at 06:16:29PM +0100, Daniel Lezcano wrote:
> On 11/6/25 23:59, Christian Marangi wrote:
> > In preparation for support of Airoha AN7583, generalize the probe
> > function to address for the 2 SoC differece.
> > 
> > Implement a match_data struct where it's possible to define a more
> > specific probe and post_probe function and specific thermal ops and
> > pllrg protect value.
> > 
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> >   drivers/thermal/airoha_thermal.c | 102 +++++++++++++++++++++++--------
> >   1 file changed, 75 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/thermal/airoha_thermal.c b/drivers/thermal/airoha_thermal.c
> > index 01ed49a4887e..864a01fd8fd8 100644
> > --- a/drivers/thermal/airoha_thermal.c
> > +++ b/drivers/thermal/airoha_thermal.c
> > @@ -198,12 +198,23 @@ struct airoha_thermal_priv {
> >   	struct regmap *chip_scu;
> >   	struct resource scu_adc_res;
> > +	u32 pllrg_protect;
> > +
> >   	struct thermal_zone_device *tz;
> >   	int init_temp;
> >   	int default_slope;
> >   	int default_offset;
> >   };
> > +struct airoha_thermal_soc_data {
> > +	u32 pllrg_protect;
> > +
> > +	const struct thermal_zone_device_ops *thdev_ops;
> > +	int (*probe)(struct platform_device *pdev,
> > +		     struct airoha_thermal_priv *priv);
> > +	int (*post_probe)(struct platform_device *pdev);
> 
> What the post-probe provides compared to calling the corresponding code in
> the probe function ?
>

Hi Daniel,

the usage would be to do those operation that should be done AFTER the
thermal zone are registered. In this specific case, to enable the
interrupt.

(they can't be enabled before as they will fire up immediately due to
wrong configuration of trip point)

I can also limit enabling the interrupt to AN7581 by checking the
compatible. I probably went too much with the modular approach.

> > +};
> > +
> >   static int airoha_get_thermal_ADC(struct airoha_thermal_priv *priv)
> >   {
> >   	u32 val;
> > @@ -220,7 +231,8 @@ static void airoha_init_thermal_ADC_mode(struct airoha_thermal_priv *priv)
> >   	regmap_read(priv->chip_scu, EN7581_PLLRG_PROTECT, &pllrg);
> >   	/* Give access to thermal regs */
> > -	regmap_write(priv->chip_scu, EN7581_PLLRG_PROTECT, EN7581_SCU_THERMAL_PROTECT_KEY);
> > +	regmap_write(priv->chip_scu, EN7581_PLLRG_PROTECT,
> > +		     priv->pllrg_protect);
> >   	adc_mux = FIELD_PREP(EN7581_MUX_TADC, EN7581_SCU_THERMAL_MUX_DIODE1);
> >   	regmap_write(priv->chip_scu, EN7581_PWD_TADC, adc_mux);
> > @@ -228,7 +240,7 @@ static void airoha_init_thermal_ADC_mode(struct airoha_thermal_priv *priv)
> >   	regmap_write(priv->chip_scu, EN7581_PLLRG_PROTECT, pllrg);
> >   }
> > -static int airoha_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
> > +static int en7581_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
> 
> Please provide a separate patch before where s/airoha/en7581/
> 

Ok no problem.

> [ ... ]
> > +static int en7581_thermal_post_probe(struct platform_device *pdev)
> > +{
> > +	struct airoha_thermal_priv *priv = platform_get_drvdata(pdev);
> > +
> > +	/* Enable LOW and HIGH interrupt (if supported) */
> 
> Why "(if supported)" ?
> 

Airoha AN7583 won't support this. But I understand this is confusing
since this function is specific to AN7581.

> > +	regmap_write(priv->map, EN7581_TEMPMONINT,
> > +		     EN7581_HOFSINTEN0 | EN7581_LOFSINTEN0);
> > +
> > +	return 0;
> > +}
> > +
> > +static int airoha_thermal_probe(struct platform_device *pdev)
> > +{
> > +	const struct airoha_thermal_soc_data *soc_data;
> > +	struct airoha_thermal_priv *priv;
> > +	struct device *dev = &pdev->dev;
> > +	int ret;
> > +
> > +	soc_data = device_get_match_data(dev);
> > +
> > +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	priv->pllrg_protect = soc_data->pllrg_protect;
> > +
> > +	if (!soc_data->probe)
> > +		return -EINVAL;
> 
> Shall the driver check its own code ?
> > +	ret = soc_data->probe(pdev, priv);
> > +	if (ret)
> 

Well it's expected to be always present so I guess I can drop this.

> [ ... ]
> > +static const struct airoha_thermal_soc_data en7581_data = {
> > +	.pllrg_protect = EN7581_SCU_THERMAL_PROTECT_KEY,
> > +	.thdev_ops = &en7581_thdev_ops,
> > +	.probe = &en7581_thermal_probe,
> > +	.post_probe = &en7581_thermal_post_probe,
> 	.probe = en7581_thermal_probe,
> 	.post_probe = en7581_thermal_post_probe,
> 
> > +};
> > +
> >   static const struct of_device_id airoha_thermal_match[] = {
> > -	{ .compatible = "airoha,en7581-thermal" },
> > +	{ .compatible = "airoha,en7581-thermal", .data = &en7581_data },
> >   	{},
> >   };
> >   MODULE_DEVICE_TABLE(of, airoha_thermal_match);
> 
> 
> -- 
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog

-- 
	Ansuel