Re: [PATCH] soc: qcom: smem: support Glymur DRAM info

[Abel Vesa]

On 26-08-20 18:30:08, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
> 
> Glymur firmware provides a version 7 DDR-info SMEM entry whose 872-byte
> layout is different from the one the driver supports today.
> 
> The data contains 16 channel descriptors, 14 DDR frequency slots, four
> 4 DDR region slots, a 10-entry SHUB frequency plan, and v6 misc information.
> It accounts for 616 bytes; the captured SMEM entry has a further 256
> zero bytes.
> 
> Add a Glymur-specific version 7 layout and recognize that complete entry.
> Parse the DDR frequencies and highest-bank bit from the common fields.
> 
> Assisted-by: LLM
> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
> ---
> Validated on the ASUS Zenbook A16 and Glymur CRD
> ---
>  drivers/soc/qcom/smem_dramc.c | 66 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
> 
> diff --git a/drivers/soc/qcom/smem_dramc.c b/drivers/soc/qcom/smem_dramc.c
> index 63ebe7f6db59..9e3234a67406 100644
> --- a/drivers/soc/qcom/smem_dramc.c
> +++ b/drivers/soc/qcom/smem_dramc.c
> @@ -20,12 +20,22 @@
>  #define MAX_DDR_FREQ_NUM_V5		14
>  
>  #define MAX_CHAN_NUM			8
> +#define MAX_CHAN_NUM_V7_GLYMUR		16
>  #define MAX_RANK_NUM			2
>  
>  #define DDR_HBB_MIN			13
>  #define DDR_HBB_MAX			19
>  
>  #define MAX_SHUB_ENTRIES		8
> +#define MAX_SHUB_ENTRIES_V7_GLYMUR	10
> +#define MAX_DDR_REGIONS_V7_GLYMUR	4
> +
> +/*
> + * Glymur's DRAM information entry has 256 bytes of trailing zeroes
> + * after its DDR details, four region slots, 10-entry SHUB frequency
> + * plan, and v6 misc information.
> + */
> +#define DDR_V7_GLYMUR_RESERVED_SIZE	256
>  
>  static struct smem_dram *__dram;
>  
> @@ -39,6 +49,7 @@ enum ddr_info_version {
>  	INFO_V6, /* INFO_V6 seems to only have shipped with 6 DDR regions, unlike V7 */
>  	INFO_V7,
>  	INFO_V7_WITH_6_REGIONS,
> +	INFO_V7_GLYMUR,

Just curious, isn't there like a minor version number that could be
used instead of the GLYMUR suffix here?

Anyway, looks OK to me, so:

Reviewed-by: Abel Vesa <abel.vesa@xxxxxxxxxxxxxxxx>