Re: [PATCH v5 5/5] platform/x86/intel/pmc: Add Arrow Lake U/H support to intel_pmc_core driver

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

On Tue, 4 Feb 2025, Xi Pardee wrote:

> Add Arrow Lake U and Arrow Lake H support in intel_pmc_core driver.
> 
> Signed-off-by: Rajvi Jingar <rajvi.jingar@linux.intel.com>
> Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
> ---
>  drivers/platform/x86/intel/pmc/arl.c  | 37 +++++++++++++++++++++++++++
>  drivers/platform/x86/intel/pmc/core.c |  2 ++
>  drivers/platform/x86/intel/pmc/core.h |  2 ++
>  3 files changed, 41 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel/pmc/arl.c b/drivers/platform/x86/intel/pmc/arl.c
> index 2e604f934f068..f62763865207b 100644
> --- a/drivers/platform/x86/intel/pmc/arl.c
> +++ b/drivers/platform/x86/intel/pmc/arl.c
> @@ -16,6 +16,7 @@
>  #define IOEP_LPM_REQ_GUID	0x5077612
>  #define SOCS_LPM_REQ_GUID	0x8478657
>  #define PCHS_LPM_REQ_GUID	0x9684572
> +#define SOCM_LPM_REQ_GUID	0x2625030
>  
>  static const u8 ARL_LPM_REG_INDEX[] = {0, 4, 5, 6, 8, 9, 10, 11, 12, 13, 14, 15, 16, 20};
>  
> @@ -650,6 +651,7 @@ const struct pmc_reg_map arl_pchs_reg_map = {
>  	.etr3_offset = ETR3_OFFSET,
>  };
>  
> +#define PMC_DEVID_SOCM 0x777f
>  #define PMC_DEVID_SOCS 0xae7f
>  #define PMC_DEVID_IOEP 0x7ecf
>  #define PMC_DEVID_PCHS 0x7f27
> @@ -669,11 +671,17 @@ static struct pmc_info arl_pmc_info_list[] = {
>  		.devid	= PMC_DEVID_PCHS,
>  		.map	= &arl_pchs_reg_map,
>  	},
> +	{
> +		.guid	= SOCM_LPM_REQ_GUID,
> +		.devid	= PMC_DEVID_SOCM,
> +		.map	= &mtl_socm_reg_map,
> +	},
>  	{}
>  };
>  
>  #define ARL_NPU_PCI_DEV			0xad1d
>  #define ARL_GNA_PCI_DEV			0xae4c
> +#define ARL_H_GNA_PCI_DEV		0x774c
>  /*
>   * Set power state of select devices that do not have drivers to D3
>   * so that they do not block Package C entry.
> @@ -684,6 +692,12 @@ static void arl_d3_fixup(void)
>  	pmc_core_set_device_d3(ARL_GNA_PCI_DEV);
>  }
>  
> +static void arl_h_d3_fixup(void)
> +{
> +	pmc_core_set_device_d3(ARL_NPU_PCI_DEV);
> +	pmc_core_set_device_d3(ARL_H_GNA_PCI_DEV);
> +}
> +
>  static int arl_resume(struct pmc_dev *pmcdev)
>  {
>  	arl_d3_fixup();
> @@ -691,6 +705,13 @@ static int arl_resume(struct pmc_dev *pmcdev)
>  	return cnl_resume(pmcdev);
>  }
>  
> +static int arl_h_resume(struct pmc_dev *pmcdev)
> +{
> +	arl_h_d3_fixup();
> +
> +	return cnl_resume(pmcdev);
> +}
> +
>  struct pmc_dev_info arl_pmc_dev = {
>  	.pci_func = 0,
>  	.dmu_guid = ARL_PMT_DMU_GUID,
> @@ -701,8 +722,24 @@ struct pmc_dev_info arl_pmc_dev = {
>  	.init = arl_core_init,
>  };
>  
> +struct pmc_dev_info arl_h_pmc_dev = {
> +	.pci_func = 2,
> +	.dmu_guid = ARL_PMT_DMU_GUID,
> +	.regmap_list = arl_pmc_info_list,
> +	.map = &mtl_socm_reg_map,
> +	.suspend = cnl_suspend,
> +	.resume = arl_h_resume,
> +	.init = arl_h_core_init,
> +};

This struct should come after the init function so you don't need to 
forward declare the init function and can make the init function a static 
one (as with the other cases in patch 4).

> +
>  int arl_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info)
>  {
>  	arl_d3_fixup();
>  	return generic_core_init(pmcdev, pmc_dev_info);
>  }
> +
> +int arl_h_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info)
> +{
> +	arl_h_d3_fixup();
> +	return generic_core_init(pmcdev, pmc_dev_info);
> +}
> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> index 628cb22221fbc..d819478fea29a 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -1410,6 +1410,8 @@ static const struct x86_cpu_id intel_pmc_core_ids[] = {
>  	X86_MATCH_VFM(INTEL_RAPTORLAKE_S,	&adl_pmc_dev),
>  	X86_MATCH_VFM(INTEL_METEORLAKE_L,	&mtl_pmc_dev),
>  	X86_MATCH_VFM(INTEL_ARROWLAKE,		&arl_pmc_dev),
> +	X86_MATCH_VFM(INTEL_ARROWLAKE_H,	&arl_h_pmc_dev),
> +	X86_MATCH_VFM(INTEL_ARROWLAKE_U,	&arl_h_pmc_dev),
>  	X86_MATCH_VFM(INTEL_LUNARLAKE_M,	&lnl_pmc_dev),
>  	{}
>  };
> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
> index 94039930422b3..0f0ee97ee00a8 100644
> --- a/drivers/platform/x86/intel/pmc/core.h
> +++ b/drivers/platform/x86/intel/pmc/core.h
> @@ -624,9 +624,11 @@ extern struct pmc_dev_info tgl_pmc_dev;
>  extern struct pmc_dev_info adl_pmc_dev;
>  extern struct pmc_dev_info mtl_pmc_dev;
>  extern struct pmc_dev_info arl_pmc_dev;
> +extern struct pmc_dev_info arl_h_pmc_dev;
>  extern struct pmc_dev_info lnl_pmc_dev;
>  
>  int arl_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info);
> +int arl_h_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info);

Not needed.

>  int mtl_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info);
>  int lnl_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info);
>  int tgl_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info);
> 

-- 
 i.