Re: [PATCH v4 06/18] platform/x86/intel/pmt: Do not remap when using callbacks

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Tue, 1 Sep 2026, Michael J. Ruhl wrote:

> A callback indicates that the parent driver is responsible
> for accessing the data area.  Creating a PMT memory remap
> is redundant.
> 
> If a read_telem callback has been provided, do not create a
> remap.
> 
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---
>  drivers/platform/x86/intel/pmt/class.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
> index af7c951b569a..350129121ff1 100644
> --- a/drivers/platform/x86/intel/pmt/class.c
> +++ b/drivers/platform/x86/intel/pmt/class.c
> @@ -373,7 +373,6 @@ static int intel_pmt_dev_register(struct intel_pmt_entry *entry,
>  				  struct device *parent)
>  {
>  	struct intel_vsec_device *ivdev = dev_to_ivdev(parent);
> -	struct resource res = {0};
>  	struct device *dev;
>  	int ret;
>  
> @@ -403,14 +402,18 @@ static int intel_pmt_dev_register(struct intel_pmt_entry *entry,
>  	if (!entry->size)
>  		return 0;
>  
> -	res.start = entry->base_addr;
> -	res.end = res.start + entry->size - 1;
> -	res.flags = IORESOURCE_MEM;
> +	if (!is_cb_read_telem(entry->cb)) {
> +		struct resource res = {0};

I see you're copying this from the existing one, but this is wrong on 
multiple counts:

- For general initialization to default values, = {} should be used
  (the extra zero is snakeoil).
- HOWEVER, with struct resource, that won't produce a valid resource!

In this case, DEFINE_RES_MEM() should be used as you've everything 
available to setup it right away.

> -	entry->base = devm_ioremap_resource(dev, &res);
> -	if (IS_ERR(entry->base)) {
> -		ret = PTR_ERR(entry->base);
> -		goto fail_ioremap;
> +		res.start = entry->base_addr;
> +		res.end = res.start + entry->size - 1;

For the record, there would also be resource_set_range() but since you're 
going to use DEFINE_RES_MEM(), that isn't necessary here.

> +		res.flags = IORESOURCE_MEM;
> +
> +		entry->base = devm_ioremap_resource(dev, &res);
> +		if (IS_ERR(entry->base)) {
> +			ret = PTR_ERR(entry->base);
> +			goto fail_ioremap;
> +		}
>  	}
>  
>  	sysfs_bin_attr_init(&entry->pmt_bin_attr);
> 

-- 
 i.