Re: [PATCH 2/2] thermal: intel: int340x: Allow temperature override

[srinivas pandruvada <srinivas.pandruvada@linux.intel.com>]

On Thu, 2025-06-05 at 02:18 +0000, Zhang, Rui wrote:
> On Wed, 2025-06-04 at 13:35 -0700, Srinivas Pandruvada wrote:
> > Add debugfs interface to override hardware provide temperature.
> > This
> > interface can be used primarily for debug. Alternatively this can
> > be also used to use hardware control loops to manage temperature
> > for
> > virtual sensors. Virtual sensors are soft sensors created by
> > kernel/
> > user space aggregating other sensors.
> > 
> > There are three attributes to override the maximum three instances
> > of
> > platform temperature control.
> > /sys/kernel/debug/plaftform_temperature_control/
> > ├── temperature_0
> > ├── temperature_1
> > └── temperature_2
> > 
> > These are write only attributes requires admin privilege. Any value
> > greater than 0, will override the temperature. A value of 0 will
> > stop overriding the temperature.
> > 
> > Signed-off-by: Srinivas Pandruvada
> > <srinivas.pandruvada@linux.intel.com>
> > ---
> >  .../platform_temperature_control.c            | 64
> > +++++++++++++++++++
> >  1 file changed, 64 insertions(+)
> > 
> > diff --git
> > a/drivers/thermal/intel/int340x_thermal/platform_temperature_contro
> > l.c
> > b/drivers/thermal/intel/int340x_thermal/platform_temperature_contro
> > l.c
> > index 6cd05783a52d..5dcfd2cc9082 100644
> > ---
> > a/drivers/thermal/intel/int340x_thermal/platform_temperature_contro
> > l.c
> > +++
> > b/drivers/thermal/intel/int340x_thermal/platform_temperature_contro
> > l.c
> > @@ -38,6 +38,7 @@
> >  
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> > +#include <linux/debugfs.h>
> >  #include <linux/pci.h>
> >  #include "processor_thermal_device.h"
> >  
> > @@ -53,6 +54,7 @@ struct mmio_reg {
> >  
> >  struct ptc_data {
> >  	u32 offset;
> > +	struct pci_dev *pdev;
> >  	struct attribute_group ptc_attr_group;
> >  	struct attribute *ptc_attrs[PTC_MAX_ATTRS];
> >  	struct device_attribute temperature_target_attr;
> > @@ -222,6 +224,63 @@ static int ptc_create_groups(struct pci_dev
> > *pdev,
> > int instance, struct ptc_data
> >  }
> >  
> >  static struct ptc_data ptc_instance[PTC_MAX_INSTANCES];
> > +static struct dentry *ptc_debugfs;
> > +
> > +#define PTC_TEMP_OVERRIDE_ENABLE_INDEX	4
> > +#define PTC_TEMP_OVERRIDE_INDEX		5
> > +
> > +static ssize_t ptc_temperature_write(struct file *file, const char
> > __user *data,
> > +				     size_t count, loff_t *ppos)
> > +{
> > +	struct ptc_data *ptc_instance = file->private_data;
> > +	struct pci_dev *pdev = ptc_instance->pdev;
> > +	char buf[32];
> > +	ssize_t len;
> > +	u32 value;
> > +
> > +	len = min(count, sizeof(buf) - 1);
> > +	if (copy_from_user(buf, data, len))
> > +		return -EFAULT;
> > +
> > +	buf[len] = '\0';
> > +	if (kstrtouint(buf, 0, &value))
> > +		return -EINVAL;
> > +
> > +	if (ptc_mmio_regs[PTC_TEMP_OVERRIDE_INDEX].units)
> > +		value /=
> > ptc_mmio_regs[PTC_TEMP_OVERRIDE_INDEX].units;
> > +
> > +	if (value > ptc_mmio_regs[PTC_TEMP_OVERRIDE_INDEX].mask)
> > +		return -EINVAL;
> > +
> > +	if (!value) {
> > +		ptc_mmio_write(pdev, ptc_instance->offset,
> > PTC_TEMP_OVERRIDE_ENABLE_INDEX, 0);
> > +	} else {
> > +		ptc_mmio_write(pdev, ptc_instance->offset,
> > PTC_TEMP_OVERRIDE_INDEX, value);
> > +		ptc_mmio_write(pdev, ptc_instance->offset,
> > PTC_TEMP_OVERRIDE_ENABLE_INDEX, 1);
> > +	}
> > +
> > +	return count;
> > +}
> > +
> > +static const struct file_operations ptc_fops = {
> > +	.open = simple_open,
> > +	.write = ptc_temperature_write,
> > +	.llseek = generic_file_llseek,
> > +};
> > +
> > +static void ptc_create_debugfs(void)
> > +{
> > +	ptc_debugfs =
> > debugfs_create_dir("plaftform_temperature_control", NULL);
> 
> s/platform/plaftform
> 
correct.

> And same in the changelog.
> 
> > +
> > +	debugfs_create_file("temperature_0",  0200, ptc_debugfs, 
> > &ptc_instance[0], &ptc_fops);
> > +	debugfs_create_file("temperature_1",  0200, ptc_debugfs, 
> > &ptc_instance[1], &ptc_fops);
> > +	debugfs_create_file("temperature_2",  0200, ptc_debugfs, 
> > &ptc_instance[2], &ptc_fops);
> > +}
> > +
> > +static void ptc_delete_debugfs(void)
> > +{
> > +	debugfs_remove_recursive(ptc_debugfs);
> > +}
> >  
> >  int proc_thermal_ptc_add(struct pci_dev *pdev, struct
> > proc_thermal_device *proc_priv)
> >  {
> > @@ -230,10 +289,13 @@ int proc_thermal_ptc_add(struct pci_dev
> > *pdev,
> > struct proc_thermal_device *proc_
> >  
> >  		for (i = 0; i < PTC_MAX_INSTANCES; i++) {
> >  			ptc_instance[i].offset = ptc_offsets[i];
> > +			ptc_instance[i].pdev = pdev;
> >  			ptc_create_groups(pdev, i,
> > &ptc_instance[i]);
> >  		}
> >  	}
> >  
> > +	ptc_create_debugfs();
> > +
> 
> should we create the debugfs only when PROC_THERMAL_FEATURE_PTC is
> set?

This function is only called when
 if (feature_mask & PROC_THERMAL_FEATURE_PTC) {
}


> 
> >  	return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(proc_thermal_ptc_add);
> > @@ -248,6 +310,8 @@ void proc_thermal_ptc_remove(struct pci_dev
> > *pdev)
> >  		for (i = 0; i < PTC_MAX_INSTANCES; i++)
> >  			sysfs_remove_group(&pdev->dev.kobj,
> > &ptc_instance[i].ptc_attr_group);
> >  	}
> > +
> > +	ptc_delete_debugfs();
> 
> ditto.
Same as above.

Thanks,
Srinivas

> 
> thanks,
> rui