Re: [PATCH v2] soc/tegra: fuse: use platform info with soc revision

[Jon Hunter <jonathanh@nvidia.com>]

On 09/11/2022 06:26, Kartik wrote:
> Tegra pre-silicon platforms do not have chip revisions. This makes the
> revision soc attribute meaningless on these platforms.
> 
> Instead, populate the revision soc attribute with
> "platform name + chip revision" for Silicon. For pre-silicon platforms
> populate it with "platform name" instead.
> 
> Signed-off-by: Kartik <kkartik@nvidia.com>
> ---
> v1->v2:
>   * Updated commit message.
> 
>   drivers/soc/tegra/fuse/fuse-tegra.c    | 23 +++++++++++++++++++++--
>   drivers/soc/tegra/fuse/tegra-apbmisc.c |  1 +
>   include/soc/tegra/fuse.h               | 15 +++++++++++++++
>   3 files changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/tegra/fuse/fuse-tegra.c b/drivers/soc/tegra/fuse/fuse-tegra.c
> index ea25a1dcafc2..a20c9e0105dc 100644
> --- a/drivers/soc/tegra/fuse/fuse-tegra.c
> +++ b/drivers/soc/tegra/fuse/fuse-tegra.c
> @@ -35,6 +35,19 @@ static const char *tegra_revision_name[TEGRA_REVISION_MAX] = {
>   	[TEGRA_REVISION_A04]     = "A04",
>   };
>   
> +static const char *tegra_platform_name[TEGRA_PLATFORM_MAX] = {
> +	[TEGRA_PLATFORM_SILICON]			= "Silicon",
> +	[TEGRA_PLATFORM_QT]				= "QT",
> +	[TEGRA_PLATFORM_SYSTEM_FPGA]			= "System FPGA",
> +	[TEGRA_PLATFORM_UNIT_FPGA]			= "Unit FPGA",
> +	[TEGRA_PLATFORM_ASIM_QT]			= "Asim QT",
> +	[TEGRA_PLATFORM_ASIM_LINSIM]			= "Asim Linsim",
> +	[TEGRA_PLATFORM_DSIM_ASIM_LINSIM]		= "Dsim Asim Linsim",
> +	[TEGRA_PLATFORM_VERIFICATION_SIMULATION]	= "Verification Simulation",
> +	[TEGRA_PLATFORM_VDK]				= "VDK",
> +	[TEGRA_PLATFORM_VSP]				= "VSP",
> +};
> +
>   static const struct of_device_id car_match[] __initconst = {
>   	{ .compatible = "nvidia,tegra20-car", },
>   	{ .compatible = "nvidia,tegra30-car", },
> @@ -370,8 +383,14 @@ struct device * __init tegra_soc_device_register(void)
>   		return NULL;
>   
>   	attr->family = kasprintf(GFP_KERNEL, "Tegra");
> -	attr->revision = kasprintf(GFP_KERNEL, "%s",
> -		tegra_revision_name[tegra_sku_info.revision]);
> +	if (tegra_is_silicon()) {

curly braces are not needed.

> +		attr->revision = kasprintf(GFP_KERNEL, "%s %s",
> +					   tegra_platform_name[tegra_sku_info.platform],

Do we need to include platform here? Can't we just print the revision?

> +					   tegra_revision_name[tegra_sku_info.revision]);
> +	} else {
> +		attr->revision = kasprintf(GFP_KERNEL, "%s",
> +					   tegra_platform_name[tegra_sku_info.platform]);
> +	}
>   	attr->soc_id = kasprintf(GFP_KERNEL, "%u", tegra_get_chip_id());
>   	attr->custom_attr_group = fuse->soc->soc_attr_group;
>   
> diff --git a/drivers/soc/tegra/fuse/tegra-apbmisc.c b/drivers/soc/tegra/fuse/tegra-apbmisc.c
> index 3351bd872ab2..4591c5bcb690 100644
> --- a/drivers/soc/tegra/fuse/tegra-apbmisc.c
> +++ b/drivers/soc/tegra/fuse/tegra-apbmisc.c
> @@ -156,6 +156,7 @@ void __init tegra_init_revision(void)
>   	}
>   
>   	tegra_sku_info.sku_id = tegra_fuse_read_early(FUSE_SKU_INFO);
> +	tegra_sku_info.platform = tegra_get_platform();
>   }
>   
>   void __init tegra_init_apbmisc(void)
> diff --git a/include/soc/tegra/fuse.h b/include/soc/tegra/fuse.h
> index 977c334136e9..a63de5da8124 100644
> --- a/include/soc/tegra/fuse.h
> +++ b/include/soc/tegra/fuse.h
> @@ -34,6 +34,20 @@ enum tegra_revision {
>   	TEGRA_REVISION_MAX,
>   };
>   
> +enum tegra_platform {
> +	TEGRA_PLATFORM_SILICON = 0,
> +	TEGRA_PLATFORM_QT,
> +	TEGRA_PLATFORM_SYSTEM_FPGA,
> +	TEGRA_PLATFORM_UNIT_FPGA,
> +	TEGRA_PLATFORM_ASIM_QT,
> +	TEGRA_PLATFORM_ASIM_LINSIM,
> +	TEGRA_PLATFORM_DSIM_ASIM_LINSIM,
> +	TEGRA_PLATFORM_VERIFICATION_SIMULATION,
> +	TEGRA_PLATFORM_VDK,
> +	TEGRA_PLATFORM_VSP,
> +	TEGRA_PLATFORM_MAX,
> +};
> +
>   struct tegra_sku_info {
>   	int sku_id;
>   	int cpu_process_id;
> @@ -47,6 +61,7 @@ struct tegra_sku_info {
>   	int gpu_speedo_id;
>   	int gpu_speedo_value;
>   	enum tegra_revision revision;
> +	enum tegra_platform platform;
>   };
>   
>   #ifdef CONFIG_ARCH_TEGRA

-- 
nvpublic