Re: [PATCH v1 1/5] platform/x86: intel_pmc_ipc: Fix register names

[Andy Shevchenko]

On Thu, Nov 16, 2017 at 12:18 PM, Souvik Kumar Chakravarty
<souvik.k.chakravarty@xxxxxxxxx> wrote:
> GCR TELEM register names have been fixed as per the External Design
> Sepcification (EDS) of Apollolake and GeminiLake. This makes it
> possible to fetch the complete 64-bit S0ix counter using exported APIs.
>
> This patch also fixes some alignment issues in the macro definition
> section.

>  /* Commands */
>  #define PMC_IPC_PMIC_ACCESS            0xFF
> -#define                PMC_IPC_PMIC_ACCESS_READ        0x0
> -#define                PMC_IPC_PMIC_ACCESS_WRITE       0x1
> +#define        PMC_IPC_PMIC_ACCESS_READ        0x0
> +#define        PMC_IPC_PMIC_ACCESS_WRITE       0x1

I don't see any usefulness of this hunk, honestly.


>  /* GCR reg offsets from gcr base*/
>  #define PMC_GCR_PMC_CFG_REG            0x08
> -#define PMC_GCR_TELEM_DEEP_S0IX_REG    0x78
> -#define PMC_GCR_TELEM_SHLW_S0IX_REG    0x80
> +#define PMC_GCR_TELEM_DEEP_S0IX_LO_REG 0x78
> +#define PMC_GCR_TELEM_DEEP_S0IX_HI_REG 0x7C
> +#define PMC_GCR_TELEM_SHLW_S0IX_LO_REG 0x80
> +#define PMC_GCR_TELEM_SHLW_S0IX_HI_REG 0x84

> -       deep = gcr_data_readq(PMC_GCR_TELEM_DEEP_S0IX_REG);
> -       shlw = gcr_data_readq(PMC_GCR_TELEM_SHLW_S0IX_REG);
> +       deep = gcr_data_readq(PMC_GCR_TELEM_DEEP_S0IX_LO_REG);
> +       shlw = gcr_data_readq(PMC_GCR_TELEM_SHLW_S0IX_LO_REG);

I didn't get how this helps to what commit message states.
Moreover, it's a common practice to name 64-bit registers without
LO/HI split (in case we access them using readq() / writeq() or alike
functions).

-- 
With Best Regards,
Andy Shevchenko