Re: [PATCH 6/6] thermal: versal-thermal: Support thermal management in AI Engine

[Krzysztof Kozlowski <krzk@kernel.org>]

On 05/09/2025 10:41, Michal Simek wrote:
> +	vti->channel = iio_channel_get(NULL, SYSMON_TEMP_CH_NAME);
> +	if (IS_ERR(vti->channel)) {
> +		vti->num_aie_channels = 0;
> +		versal_thermal_iio_chan_release(vti);
> +		return dev_err_probe(&pdev->dev, PTR_ERR(vti->channel),
> +							"IIO channel %s not found\n",
> +							SYSMON_TEMP_CH_NAME);
> +	}
> +
>  	vti->tzd = devm_thermal_of_zone_register(&pdev->dev, 0, vti, &thermal_zone_ops);
>  	if (IS_ERR(vti->tzd))
>  		return dev_err_probe(&pdev->dev, PTR_ERR(vti->tzd),
>  				     "Thermal zone sensor register failed\n");
>  
> -	return devm_thermal_add_hwmon_sysfs(&pdev->dev, vti->tzd);
> +	ret = devm_thermal_add_hwmon_sysfs(&pdev->dev, vti->tzd);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "Failed to add hwmon sysfs for sysmon temp\n");
> +
> +	sysmon_node = of_find_node_by_name(NULL, "sysmon");

Undocumented ABI. Please don't do that. sysmon is not an approved name
and for sure can be changed anytime to anything.

Phandles express relationships usually.


> +	if (sysmon_node) {
> +		ret = of_property_read_u32(sysmon_node, "xlnx,numaiechannels",
> +					   &num_aie_channels);
> +		if (ret < 0)
> +			num_aie_channels = 0;
> +	}
> +
> +	if (num_aie_channels > 0) {
> +		aie_temp_chan_names = devm_kcalloc(&pdev->dev, num_aie_channels,
> +						   sizeof(*aie_temp_chan_names),
> +						   GFP_KERNEL);
> +		if (!aie_temp_chan_names)
> +			return -ENOMEM;
> +
> +		for_each_child_of_node(sysmon_node, child_node) {
> +			if (of_property_present(child_node, "xlnx,aie-temp")) {
> +				ret = of_property_read_string(child_node, "xlnx,name",
> +							      &aie_temp_chan_name);
> +				if (ret < 0) {
> +					of_node_put(child_node);
> +					return ret;
> +				}
> +				aie_temp_chan_names[aie_ch_index] = aie_temp_chan_name;
> +				aie_ch_index++;
> +			}
> +		}
> +
> +		/* Allocate memory for the dynamic aie temperature channels */
> +		vti->channel_aie = devm_kcalloc(&pdev->dev, num_aie_channels,
> +						sizeof(*vti->channel_aie), GFP_KERNEL);
> +		if (!vti->channel_aie)
> +			return -ENOMEM;
> +
> +		for (aie_ch_index = 0; aie_ch_index < num_aie_channels; aie_ch_index++) {
> +			vti->channel_aie[aie_ch_index] =
> +			    iio_channel_get(NULL, aie_temp_chan_names[aie_ch_index]);
> +			if (IS_ERR(vti->channel_aie[aie_ch_index])) {
> +				vti->num_aie_channels = aie_ch_index + 1;
> +				versal_thermal_iio_chan_release(vti);
> +				return dev_err_probe(&pdev->dev,
> +						     PTR_ERR(vti->channel_aie[aie_ch_index]),
> +						     "IIO AIE TEMP channel %s not found\n",
> +						     aie_temp_chan_names[aie_ch_index]);
> +			}
> +		}
> +
> +		vti->tzd_aie = devm_thermal_of_zone_register(&pdev->dev, 1, vti,
> +							     &thermal_zone_ops_aie);
> +		if (IS_ERR(vti->tzd_aie))
> +			return dev_err_probe(&pdev->dev, PTR_ERR(vti->tzd_aie),
> +					      "Failed to register thermal zone aie temp\n");
> +
> +		ret = devm_thermal_add_hwmon_sysfs(&pdev->dev, vti->tzd_aie);
> +		if (ret)
> +			return dev_err_probe(&pdev->dev, ret,
> +					     "Failed to add hwmon sysfs for aie temp\n");
> +	}
> +	vti->num_aie_channels = num_aie_channels;
> +	platform_set_drvdata(pdev, vti);
> +	return 0;
> +}
> +
> +static void versal_thermal_remove(struct platform_device *pdev)
> +{
> +	struct versal_thermal_info *vti = platform_get_drvdata(pdev);
> +
> +	versal_thermal_iio_chan_release(vti);

Don't mix non-devm with devm. This should be a proper devm cleanup action.



Best regards,
Krzysztof