[PATCH 1/4] soc: qcom: socinfo: move SMEM item struct and defines to a header

[PATCH 1/4] soc: qcom: socinfo: move SMEM item struct and defines to a header

[Robert Marko]

Move SMEM item struct and related defines to a header in order to be able
to reuse them in the Qualcomm NVMEM CPUFreq driver instead of duplicating
them.

Signed-off-by: Robert Marko <robimarko@gmail.com>
---
 drivers/soc/qcom/socinfo.c       | 65 +-----------------------------
 include/linux/soc/qcom/socinfo.h | 68 ++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 64 deletions(-)
 create mode 100644 include/linux/soc/qcom/socinfo.h

diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
index 3b970a80f3aa..5919f863f369 100644
--- a/drivers/soc/qcom/socinfo.c
+++ b/drivers/soc/qcom/socinfo.c
@@ -11,6 +11,7 @@
 #include <linux/random.h>
 #include <linux/slab.h>
 #include <linux/soc/qcom/smem.h>
+#include <linux/soc/qcom/socinfo.h>
 #include <linux/string.h>
 #include <linux/stringify.h>
 #include <linux/sys_soc.h>
@@ -32,15 +33,6 @@
 #define qcom_board_id(id) QCOM_ID_ ## id, __stringify(id)
 #define qcom_board_id_named(id, name) QCOM_ID_ ## id, (name)
 
-#define SMEM_SOCINFO_BUILD_ID_LENGTH           32
-#define SMEM_SOCINFO_CHIP_ID_LENGTH            32
-
-/*
- * SMEM item id, used to acquire handles to respective
- * SMEM region.
- */
-#define SMEM_HW_SW_BUILD_ID            137
-
 #ifdef CONFIG_DEBUG_FS
 #define SMEM_IMAGE_VERSION_BLOCKS_COUNT        32
 #define SMEM_IMAGE_VERSION_SIZE                4096
@@ -121,62 +113,7 @@ static const char *const pmic_models[] = {
 	[58] = "PM8450",
 	[65] = "PM8010",
 };
-#endif /* CONFIG_DEBUG_FS */
-
-/* Socinfo SMEM item structure */
-struct socinfo {
-	__le32 fmt;
-	__le32 id;
-	__le32 ver;
-	char build_id[SMEM_SOCINFO_BUILD_ID_LENGTH];
-	/* Version 2 */
-	__le32 raw_id;
-	__le32 raw_ver;
-	/* Version 3 */
-	__le32 hw_plat;
-	/* Version 4 */
-	__le32 plat_ver;
-	/* Version 5 */
-	__le32 accessory_chip;
-	/* Version 6 */
-	__le32 hw_plat_subtype;
-	/* Version 7 */
-	__le32 pmic_model;
-	__le32 pmic_die_rev;
-	/* Version 8 */
-	__le32 pmic_model_1;
-	__le32 pmic_die_rev_1;
-	__le32 pmic_model_2;
-	__le32 pmic_die_rev_2;
-	/* Version 9 */
-	__le32 foundry_id;
-	/* Version 10 */
-	__le32 serial_num;
-	/* Version 11 */
-	__le32 num_pmics;
-	__le32 pmic_array_offset;
-	/* Version 12 */
-	__le32 chip_family;
-	__le32 raw_device_family;
-	__le32 raw_device_num;
-	/* Version 13 */
-	__le32 nproduct_id;
-	char chip_id[SMEM_SOCINFO_CHIP_ID_LENGTH];
-	/* Version 14 */
-	__le32 num_clusters;
-	__le32 ncluster_array_offset;
-	__le32 num_defective_parts;
-	__le32 ndefective_parts_array_offset;
-	/* Version 15 */
-	__le32 nmodem_supported;
-	/* Version 16 */
-	__le32  feature_code;
-	__le32  pcode;
-	__le32  npartnamemap_offset;
-	__le32  nnum_partname_mapping;
-};
 
-#ifdef CONFIG_DEBUG_FS
 struct socinfo_params {
 	u32 raw_device_family;
 	u32 hw_plat_subtype;
diff --git a/include/linux/soc/qcom/socinfo.h b/include/linux/soc/qcom/socinfo.h
new file mode 100644
index 000000000000..6a175d635617
--- /dev/null
+++ b/include/linux/soc/qcom/socinfo.h
@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __QCOM_SOCINFO_H__
+#define __QCOM_SOCINFO_H__
+
+/*
+ * SMEM item id, used to acquire handles to respective
+ * SMEM region.
+ */
+#define SMEM_HW_SW_BUILD_ID		137
+
+#define SMEM_SOCINFO_BUILD_ID_LENGTH	32
+#define SMEM_SOCINFO_CHIP_ID_LENGTH	32
+
+/* Socinfo SMEM item structure */
+struct socinfo {
+	__le32 fmt;
+	__le32 id;
+	__le32 ver;
+	char build_id[SMEM_SOCINFO_BUILD_ID_LENGTH];
+	/* Version 2 */
+	__le32 raw_id;
+	__le32 raw_ver;
+	/* Version 3 */
+	__le32 hw_plat;
+	/* Version 4 */
+	__le32 plat_ver;
+	/* Version 5 */
+	__le32 accessory_chip;
+	/* Version 6 */
+	__le32 hw_plat_subtype;
+	/* Version 7 */
+	__le32 pmic_model;
+	__le32 pmic_die_rev;
+	/* Version 8 */
+	__le32 pmic_model_1;
+	__le32 pmic_die_rev_1;
+	__le32 pmic_model_2;
+	__le32 pmic_die_rev_2;
+	/* Version 9 */
+	__le32 foundry_id;
+	/* Version 10 */
+	__le32 serial_num;
+	/* Version 11 */
+	__le32 num_pmics;
+	__le32 pmic_array_offset;
+	/* Version 12 */
+	__le32 chip_family;
+	__le32 raw_device_family;
+	__le32 raw_device_num;
+	/* Version 13 */
+	__le32 nproduct_id;
+	char chip_id[SMEM_SOCINFO_CHIP_ID_LENGTH];
+	/* Version 14 */
+	__le32 num_clusters;
+	__le32 ncluster_array_offset;
+	__le32 num_defective_parts;
+	__le32 ndefective_parts_array_offset;
+	/* Version 15 */
+	__le32 nmodem_supported;
+	/* Version 16 */
+	__le32  feature_code;
+	__le32  pcode;
+	__le32  npartnamemap_offset;
+	__le32  nnum_partname_mapping;
+};
+
+#endif
-- 
2.39.1

[PATCH 2/4] cpufreq: qcom-nvmem: reuse socinfo SMEM item struct

[Robert Marko]

Now that socinfo SMEM item struct and defines have been moved to a header
so we can utilize that instead.

Now the SMEM value can be accesed directly, there is no need for defining
the ID for the SMEM request as well.

Signed-off-by: Robert Marko <robimarko@gmail.com>
---
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
index a577586b23be..c0a7841a56c1 100644
--- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
+++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
@@ -28,8 +28,7 @@
 #include <linux/pm_opp.h>
 #include <linux/slab.h>
 #include <linux/soc/qcom/smem.h>
-
-#define MSM_ID_SMEM	137
+#include <linux/soc/qcom/socinfo.h>
 
 enum _msm_id {
 	MSM8996V3 = 0xF6ul,
@@ -143,17 +142,14 @@ static void get_krait_bin_format_b(struct device *cpu_dev,
 static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
 {
 	size_t len;
-	u32 *msm_id;
+	struct socinfo *info;
 	enum _msm8996_version version;
 
-	msm_id = qcom_smem_get(QCOM_SMEM_HOST_ANY, MSM_ID_SMEM, &len);
-	if (IS_ERR(msm_id))
+	info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, &len);
+	if (IS_ERR(info))
 		return NUM_OF_MSM8996_VERSIONS;
 
-	/* The first 4 bytes are format, next to them is the actual msm-id */
-	msm_id++;
-
-	switch ((enum _msm_id)*msm_id) {
+	switch (info->id) {
 	case MSM8996V3:
 	case APQ8096V3:
 		version = MSM8996_V3;
-- 
2.39.1

Re: [PATCH 2/4] cpufreq: qcom-nvmem: reuse socinfo SMEM item struct

[Bjorn Andersson]

On Sat, Jan 21, 2023 at 12:29:45PM +0100, Robert Marko wrote:
> Now that socinfo SMEM item struct and defines have been moved to a header
> so we can utilize that instead.
> 
> Now the SMEM value can be accesed directly, there is no need for defining
> the ID for the SMEM request as well.
> 
> Signed-off-by: Robert Marko <robimarko@gmail.com>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

@Rafael, @Viresh, would you mind providing an Ack for me to take these 3
patches, together with patch 1, through the Qualcomm tree? I have staged
changes for 6.3, so we'll otherwise end up with conflicts on the move of
the struct.

Thanks,
Bjorn

> ---
>  drivers/cpufreq/qcom-cpufreq-nvmem.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> index a577586b23be..c0a7841a56c1 100644
> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> @@ -28,8 +28,7 @@
>  #include <linux/pm_opp.h>
>  #include <linux/slab.h>
>  #include <linux/soc/qcom/smem.h>
> -
> -#define MSM_ID_SMEM	137
> +#include <linux/soc/qcom/socinfo.h>
>  
>  enum _msm_id {
>  	MSM8996V3 = 0xF6ul,
> @@ -143,17 +142,14 @@ static void get_krait_bin_format_b(struct device *cpu_dev,
>  static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
>  {
>  	size_t len;
> -	u32 *msm_id;
> +	struct socinfo *info;
>  	enum _msm8996_version version;
>  
> -	msm_id = qcom_smem_get(QCOM_SMEM_HOST_ANY, MSM_ID_SMEM, &len);
> -	if (IS_ERR(msm_id))
> +	info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, &len);
> +	if (IS_ERR(info))
>  		return NUM_OF_MSM8996_VERSIONS;
>  
> -	/* The first 4 bytes are format, next to them is the actual msm-id */
> -	msm_id++;
> -
> -	switch ((enum _msm_id)*msm_id) {
> +	switch (info->id) {
>  	case MSM8996V3:
>  	case APQ8096V3:
>  		version = MSM8996_V3;
> -- 
> 2.39.1
> 

Re: [PATCH 2/4] cpufreq: qcom-nvmem: reuse socinfo SMEM item struct

[Viresh Kumar]

On 06-02-23, 12:38, Bjorn Andersson wrote:
> On Sat, Jan 21, 2023 at 12:29:45PM +0100, Robert Marko wrote:
> > Now that socinfo SMEM item struct and defines have been moved to a header
> > so we can utilize that instead.
> > 
> > Now the SMEM value can be accesed directly, there is no need for defining
> > the ID for the SMEM request as well.
> > 
> > Signed-off-by: Robert Marko <robimarko@gmail.com>
> 
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> @Rafael, @Viresh, would you mind providing an Ack for me to take these 3
> patches, together with patch 1, through the Qualcomm tree? I have staged
> changes for 6.3, so we'll otherwise end up with conflicts on the move of
> the struct.

For all cpufreq patches.

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[PATCH 3/4] cpufreq: qcom-nvmem: use SoC ID-s from bindings

[Robert Marko]

SMEM SoC ID-s are now stored in DT bindings so lets use those instead of
defining them in the driver again.

Signed-off-by: Robert Marko <robimarko@gmail.com>
---
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
index c0a7841a56c1..da55d2e1925a 100644
--- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
+++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
@@ -30,12 +30,7 @@
 #include <linux/soc/qcom/smem.h>
 #include <linux/soc/qcom/socinfo.h>
 
-enum _msm_id {
-	MSM8996V3 = 0xF6ul,
-	APQ8096V3 = 0x123ul,
-	MSM8996SG = 0x131ul,
-	APQ8096SG = 0x138ul,
-};
+#include <dt-bindings/arm/qcom,ids.h>
 
 enum _msm8996_version {
 	MSM8996_V3,
@@ -150,12 +145,12 @@ static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
 		return NUM_OF_MSM8996_VERSIONS;
 
 	switch (info->id) {
-	case MSM8996V3:
-	case APQ8096V3:
+	case QCOM_ID_MSM8996:
+	case QCOM_ID_APQ8096:
 		version = MSM8996_V3;
 		break;
-	case MSM8996SG:
-	case APQ8096SG:
+	case QCOM_ID_MSM8996SG:
+	case QCOM_ID_APQ8096SG:
 		version = MSM8996_SG;
 		break;
 	default:
-- 
2.39.1

Re: [PATCH 3/4] cpufreq: qcom-nvmem: use SoC ID-s from bindings

[Konrad Dybcio]

On 21.01.2023 12:29, Robert Marko wrote:
> SMEM SoC ID-s are now stored in DT bindings so lets use those instead of
> defining them in the driver again.
> 
> Signed-off-by: Robert Marko <robimarko@gmail.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  drivers/cpufreq/qcom-cpufreq-nvmem.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> index c0a7841a56c1..da55d2e1925a 100644
> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> @@ -30,12 +30,7 @@
>  #include <linux/soc/qcom/smem.h>
>  #include <linux/soc/qcom/socinfo.h>
>  
> -enum _msm_id {
> -	MSM8996V3 = 0xF6ul,
> -	APQ8096V3 = 0x123ul,
> -	MSM8996SG = 0x131ul,
> -	APQ8096SG = 0x138ul,
> -};
> +#include <dt-bindings/arm/qcom,ids.h>
>  
>  enum _msm8996_version {
>  	MSM8996_V3,
> @@ -150,12 +145,12 @@ static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
>  		return NUM_OF_MSM8996_VERSIONS;
>  
>  	switch (info->id) {
> -	case MSM8996V3:
> -	case APQ8096V3:
> +	case QCOM_ID_MSM8996:
> +	case QCOM_ID_APQ8096:
>  		version = MSM8996_V3;
>  		break;
> -	case MSM8996SG:
> -	case APQ8096SG:
> +	case QCOM_ID_MSM8996SG:
> +	case QCOM_ID_APQ8096SG:
>  		version = MSM8996_SG;
>  		break;
>  	default:

Re: [PATCH 3/4] cpufreq: qcom-nvmem: use SoC ID-s from bindings

[Bjorn Andersson]

On Sat, Jan 21, 2023 at 12:29:46PM +0100, Robert Marko wrote:
> SMEM SoC ID-s are now stored in DT bindings so lets use those instead of
> defining them in the driver again.
> 
> Signed-off-by: Robert Marko <robimarko@gmail.com>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> ---
>  drivers/cpufreq/qcom-cpufreq-nvmem.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> index c0a7841a56c1..da55d2e1925a 100644
> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> @@ -30,12 +30,7 @@
>  #include <linux/soc/qcom/smem.h>
>  #include <linux/soc/qcom/socinfo.h>
>  
> -enum _msm_id {
> -	MSM8996V3 = 0xF6ul,
> -	APQ8096V3 = 0x123ul,
> -	MSM8996SG = 0x131ul,
> -	APQ8096SG = 0x138ul,
> -};
> +#include <dt-bindings/arm/qcom,ids.h>
>  
>  enum _msm8996_version {
>  	MSM8996_V3,
> @@ -150,12 +145,12 @@ static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
>  		return NUM_OF_MSM8996_VERSIONS;
>  
>  	switch (info->id) {
> -	case MSM8996V3:
> -	case APQ8096V3:
> +	case QCOM_ID_MSM8996:
> +	case QCOM_ID_APQ8096:
>  		version = MSM8996_V3;
>  		break;
> -	case MSM8996SG:
> -	case APQ8096SG:
> +	case QCOM_ID_MSM8996SG:
> +	case QCOM_ID_APQ8096SG:
>  		version = MSM8996_SG;
>  		break;
>  	default:
> -- 
> 2.39.1
> 

[PATCH 4/4] cpufreq: qcom-nvmem: make qcom_cpufreq_get_msm_id() return the SoC ID

[Robert Marko]

Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID
after getting it via SMEM call but instead uses an enum to encode the
matched SMEM ID to 2 variants of MSM8996 which are then used in
qcom_cpufreq_kryo_name_version() to set the supported version.

This prevents qcom_cpufreq_get_msm_id() from being universal and its doing
more than its name suggests, so lets make it just return the SoC ID
directly which allows matching directly on the SoC ID and removes the need
for msm8996_version enum which simplifies the driver.
It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s.

Signed-off-by: Robert Marko <robimarko@gmail.com>
---
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
 1 file changed, 12 insertions(+), 32 deletions(-)

diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
index da55d2e1925a..9deaf9521d6d 100644
--- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
+++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
@@ -32,12 +32,6 @@
 
 #include <dt-bindings/arm/qcom,ids.h>
 
-enum _msm8996_version {
-	MSM8996_V3,
-	MSM8996_SG,
-	NUM_OF_MSM8996_VERSIONS,
-};
-
 struct qcom_cpufreq_drv;
 
 struct qcom_cpufreq_match_data {
@@ -134,30 +128,16 @@ static void get_krait_bin_format_b(struct device *cpu_dev,
 	dev_dbg(cpu_dev, "PVS version: %d\n", *pvs_ver);
 }
 
-static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
+static int qcom_cpufreq_get_msm_id(void)
 {
 	size_t len;
 	struct socinfo *info;
-	enum _msm8996_version version;
 
 	info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, &len);
 	if (IS_ERR(info))
-		return NUM_OF_MSM8996_VERSIONS;
+		return PTR_ERR(info);
 
-	switch (info->id) {
-	case QCOM_ID_MSM8996:
-	case QCOM_ID_APQ8096:
-		version = MSM8996_V3;
-		break;
-	case QCOM_ID_MSM8996SG:
-	case QCOM_ID_APQ8096SG:
-		version = MSM8996_SG;
-		break;
-	default:
-		version = NUM_OF_MSM8996_VERSIONS;
-	}
-
-	return version;
+	return info->id;
 }
 
 static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
@@ -166,25 +146,25 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
 					  struct qcom_cpufreq_drv *drv)
 {
 	size_t len;
+	int msm_id;
 	u8 *speedbin;
-	enum _msm8996_version msm8996_version;
 	*pvs_name = NULL;
 
-	msm8996_version = qcom_cpufreq_get_msm_id();
-	if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
-		dev_err(cpu_dev, "Not Snapdragon 820/821!");
-		return -ENODEV;
-	}
+	msm_id = qcom_cpufreq_get_msm_id();
+	if (msm_id < 0)
+		return msm_id;
 
 	speedbin = nvmem_cell_read(speedbin_nvmem, &len);
 	if (IS_ERR(speedbin))
 		return PTR_ERR(speedbin);
 
-	switch (msm8996_version) {
-	case MSM8996_V3:
+	switch (msm_id) {
+	case QCOM_ID_MSM8996:
+	case QCOM_ID_APQ8096:
 		drv->versions = 1 << (unsigned int)(*speedbin);
 		break;
-	case MSM8996_SG:
+	case QCOM_ID_MSM8996SG:
+	case QCOM_ID_APQ8096SG:
 		drv->versions = 1 << ((unsigned int)(*speedbin) + 4);
 		break;
 	default:
-- 
2.39.1

Re: [PATCH 4/4] cpufreq: qcom-nvmem: make qcom_cpufreq_get_msm_id() return the SoC ID

[Bjorn Andersson]

On Sat, Jan 21, 2023 at 12:29:47PM +0100, Robert Marko wrote:
> Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID
> after getting it via SMEM call but instead uses an enum to encode the
> matched SMEM ID to 2 variants of MSM8996 which are then used in
> qcom_cpufreq_kryo_name_version() to set the supported version.
> 
> This prevents qcom_cpufreq_get_msm_id() from being universal and its doing
> more than its name suggests, so lets make it just return the SoC ID
> directly which allows matching directly on the SoC ID and removes the need
> for msm8996_version enum which simplifies the driver.
> It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s.
> 
> Signed-off-by: Robert Marko <robimarko@gmail.com>

Reviewed-by: Bjorn Andersson <andersson@kernel.org>

> ---
>  drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
>  1 file changed, 12 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> index da55d2e1925a..9deaf9521d6d 100644
> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> @@ -32,12 +32,6 @@
>  
>  #include <dt-bindings/arm/qcom,ids.h>
>  
> -enum _msm8996_version {
> -	MSM8996_V3,
> -	MSM8996_SG,
> -	NUM_OF_MSM8996_VERSIONS,
> -};
> -
>  struct qcom_cpufreq_drv;
>  
>  struct qcom_cpufreq_match_data {
> @@ -134,30 +128,16 @@ static void get_krait_bin_format_b(struct device *cpu_dev,
>  	dev_dbg(cpu_dev, "PVS version: %d\n", *pvs_ver);
>  }
>  
> -static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
> +static int qcom_cpufreq_get_msm_id(void)
>  {
>  	size_t len;
>  	struct socinfo *info;
> -	enum _msm8996_version version;
>  
>  	info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, &len);
>  	if (IS_ERR(info))
> -		return NUM_OF_MSM8996_VERSIONS;
> +		return PTR_ERR(info);
>  
> -	switch (info->id) {
> -	case QCOM_ID_MSM8996:
> -	case QCOM_ID_APQ8096:
> -		version = MSM8996_V3;
> -		break;
> -	case QCOM_ID_MSM8996SG:
> -	case QCOM_ID_APQ8096SG:
> -		version = MSM8996_SG;
> -		break;
> -	default:
> -		version = NUM_OF_MSM8996_VERSIONS;
> -	}
> -
> -	return version;
> +	return info->id;
>  }
>  
>  static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
> @@ -166,25 +146,25 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
>  					  struct qcom_cpufreq_drv *drv)
>  {
>  	size_t len;
> +	int msm_id;
>  	u8 *speedbin;
> -	enum _msm8996_version msm8996_version;
>  	*pvs_name = NULL;
>  
> -	msm8996_version = qcom_cpufreq_get_msm_id();
> -	if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
> -		dev_err(cpu_dev, "Not Snapdragon 820/821!");
> -		return -ENODEV;
> -	}
> +	msm_id = qcom_cpufreq_get_msm_id();
> +	if (msm_id < 0)
> +		return msm_id;
>  
>  	speedbin = nvmem_cell_read(speedbin_nvmem, &len);
>  	if (IS_ERR(speedbin))
>  		return PTR_ERR(speedbin);
>  
> -	switch (msm8996_version) {
> -	case MSM8996_V3:
> +	switch (msm_id) {
> +	case QCOM_ID_MSM8996:
> +	case QCOM_ID_APQ8096:
>  		drv->versions = 1 << (unsigned int)(*speedbin);
>  		break;
> -	case MSM8996_SG:
> +	case QCOM_ID_MSM8996SG:
> +	case QCOM_ID_APQ8096SG:
>  		drv->versions = 1 << ((unsigned int)(*speedbin) + 4);
>  		break;
>  	default:
> -- 
> 2.39.1
> 

Re: [PATCH 4/4] cpufreq: qcom-nvmem: make qcom_cpufreq_get_msm_id() return the SoC ID

[Konrad Dybcio]

On 21.01.2023 12:29, Robert Marko wrote:
> Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID
> after getting it via SMEM call but instead uses an enum to encode the
> matched SMEM ID to 2 variants of MSM8996 which are then used in
> qcom_cpufreq_kryo_name_version() to set the supported version.
> 
> This prevents qcom_cpufreq_get_msm_id() from being universal and its doing
> more than its name suggests, so lets make it just return the SoC ID
> directly which allows matching directly on the SoC ID and removes the need
> for msm8996_version enum which simplifies the driver.
> It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s.
> 
> Signed-off-by: Robert Marko <robimarko@gmail.com>
> ---
>  drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
>  1 file changed, 12 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> index da55d2e1925a..9deaf9521d6d 100644
> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> @@ -32,12 +32,6 @@
>  
>  #include <dt-bindings/arm/qcom,ids.h>
>  
> -enum _msm8996_version {
> -	MSM8996_V3,
> -	MSM8996_SG,
> -	NUM_OF_MSM8996_VERSIONS,
> -};
> -
>  struct qcom_cpufreq_drv;
>  
>  struct qcom_cpufreq_match_data {
> @@ -134,30 +128,16 @@ static void get_krait_bin_format_b(struct device *cpu_dev,
>  	dev_dbg(cpu_dev, "PVS version: %d\n", *pvs_ver);
>  }
>  
> -static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
> +static int qcom_cpufreq_get_msm_id(void)
This should be u32 as info->id is __le32

And please export this function from socinfo, it'll come in
useful for other drivers!

Konrad
>  {
>  	size_t len;
>  	struct socinfo *info;
> -	enum _msm8996_version version;
>  
>  	info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, &len);
>  	if (IS_ERR(info))
> -		return NUM_OF_MSM8996_VERSIONS;
> +		return PTR_ERR(info);
>  
> -	switch (info->id) {
> -	case QCOM_ID_MSM8996:
> -	case QCOM_ID_APQ8096:
> -		version = MSM8996_V3;
> -		break;
> -	case QCOM_ID_MSM8996SG:
> -	case QCOM_ID_APQ8096SG:
> -		version = MSM8996_SG;
> -		break;
> -	default:
> -		version = NUM_OF_MSM8996_VERSIONS;
> -	}
> -
> -	return version;
> +	return info->id;
>  }
>  
>  static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
> @@ -166,25 +146,25 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
>  					  struct qcom_cpufreq_drv *drv)
>  {
>  	size_t len;
> +	int msm_id;
>  	u8 *speedbin;
> -	enum _msm8996_version msm8996_version;
>  	*pvs_name = NULL;
>  
> -	msm8996_version = qcom_cpufreq_get_msm_id();
> -	if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
> -		dev_err(cpu_dev, "Not Snapdragon 820/821!");
> -		return -ENODEV;
> -	}
> +	msm_id = qcom_cpufreq_get_msm_id();
> +	if (msm_id < 0)
> +		return msm_id;
>  
>  	speedbin = nvmem_cell_read(speedbin_nvmem, &len);
>  	if (IS_ERR(speedbin))
>  		return PTR_ERR(speedbin);
>  
> -	switch (msm8996_version) {
> -	case MSM8996_V3:
> +	switch (msm_id) {
> +	case QCOM_ID_MSM8996:
> +	case QCOM_ID_APQ8096:
>  		drv->versions = 1 << (unsigned int)(*speedbin);
>  		break;
> -	case MSM8996_SG:
> +	case QCOM_ID_MSM8996SG:
> +	case QCOM_ID_APQ8096SG:
>  		drv->versions = 1 << ((unsigned int)(*speedbin) + 4);
>  		break;
>  	default:

Re: [PATCH 4/4] cpufreq: qcom-nvmem: make qcom_cpufreq_get_msm_id() return the SoC ID

[Dmitry Baryshkov]

On Sat, 18 Feb 2023 at 16:43, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>
> On 21.01.2023 12:29, Robert Marko wrote:
> > Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID
> > after getting it via SMEM call but instead uses an enum to encode the
> > matched SMEM ID to 2 variants of MSM8996 which are then used in
> > qcom_cpufreq_kryo_name_version() to set the supported version.
> >
> > This prevents qcom_cpufreq_get_msm_id() from being universal and its doing
> > more than its name suggests, so lets make it just return the SoC ID
> > directly which allows matching directly on the SoC ID and removes the need
> > for msm8996_version enum which simplifies the driver.
> > It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s.
> >
> > Signed-off-by: Robert Marko <robimarko@gmail.com>
> > ---
> >  drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
> >  1 file changed, 12 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> > index da55d2e1925a..9deaf9521d6d 100644
> > --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> > +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> > @@ -32,12 +32,6 @@
> >
> >  #include <dt-bindings/arm/qcom,ids.h>
> >
> > -enum _msm8996_version {
> > -     MSM8996_V3,
> > -     MSM8996_SG,
> > -     NUM_OF_MSM8996_VERSIONS,
> > -};
> > -
> >  struct qcom_cpufreq_drv;
> >
> >  struct qcom_cpufreq_match_data {
> > @@ -134,30 +128,16 @@ static void get_krait_bin_format_b(struct device *cpu_dev,
> >       dev_dbg(cpu_dev, "PVS version: %d\n", *pvs_ver);
> >  }
> >
> > -static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
> > +static int qcom_cpufreq_get_msm_id(void)
> This should be u32 as info->id is __le32
>
> And please export this function from socinfo, it'll come in
> useful for other drivers!

How? In my opinion newer drivers should use compat strings rather than
depending on the SoC ID. If we were not bound with the compatibility
for msm8996pro device trees already using higher bits, we could have
dropped this part too.

-- 
With best wishes
Dmitry

Re: [PATCH 4/4] cpufreq: qcom-nvmem: make qcom_cpufreq_get_msm_id() return the SoC ID

[Konrad Dybcio]

On 18.02.2023 21:36, Dmitry Baryshkov wrote:
> On Sat, 18 Feb 2023 at 16:43, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>>
>>
>> On 21.01.2023 12:29, Robert Marko wrote:
>>> Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID
>>> after getting it via SMEM call but instead uses an enum to encode the
>>> matched SMEM ID to 2 variants of MSM8996 which are then used in
>>> qcom_cpufreq_kryo_name_version() to set the supported version.
>>>
>>> This prevents qcom_cpufreq_get_msm_id() from being universal and its doing
>>> more than its name suggests, so lets make it just return the SoC ID
>>> directly which allows matching directly on the SoC ID and removes the need
>>> for msm8996_version enum which simplifies the driver.
>>> It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s.
>>>
>>> Signed-off-by: Robert Marko <robimarko@gmail.com>
>>> ---
>>>  drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
>>>  1 file changed, 12 insertions(+), 32 deletions(-)
>>>
>>> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
>>> index da55d2e1925a..9deaf9521d6d 100644
>>> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
>>> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
>>> @@ -32,12 +32,6 @@
>>>
>>>  #include <dt-bindings/arm/qcom,ids.h>
>>>
>>> -enum _msm8996_version {
>>> -     MSM8996_V3,
>>> -     MSM8996_SG,
>>> -     NUM_OF_MSM8996_VERSIONS,
>>> -};
>>> -
>>>  struct qcom_cpufreq_drv;
>>>
>>>  struct qcom_cpufreq_match_data {
>>> @@ -134,30 +128,16 @@ static void get_krait_bin_format_b(struct device *cpu_dev,
>>>       dev_dbg(cpu_dev, "PVS version: %d\n", *pvs_ver);
>>>  }
>>>
>>> -static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
>>> +static int qcom_cpufreq_get_msm_id(void)
>> This should be u32 as info->id is __le32
>>
>> And please export this function from socinfo, it'll come in
>> useful for other drivers!
> 
> How? In my opinion newer drivers should use compat strings rather than
> depending on the SoC ID. If we were not bound with the compatibility
> for msm8996pro device trees already using higher bits, we could have
> dropped this part too.
Adreno speedbin-to-fuse mapping could use SoC detection..

Konrad
> 

Re: [PATCH 4/4] cpufreq: qcom-nvmem: make qcom_cpufreq_get_msm_id() return the SoC ID

[Robert Marko]

On Sat, 18 Feb 2023 at 21:40, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>
> On 18.02.2023 21:36, Dmitry Baryshkov wrote:
> > On Sat, 18 Feb 2023 at 16:43, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> >>
> >>
> >>
> >> On 21.01.2023 12:29, Robert Marko wrote:
> >>> Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID
> >>> after getting it via SMEM call but instead uses an enum to encode the
> >>> matched SMEM ID to 2 variants of MSM8996 which are then used in
> >>> qcom_cpufreq_kryo_name_version() to set the supported version.
> >>>
> >>> This prevents qcom_cpufreq_get_msm_id() from being universal and its doing
> >>> more than its name suggests, so lets make it just return the SoC ID
> >>> directly which allows matching directly on the SoC ID and removes the need
> >>> for msm8996_version enum which simplifies the driver.
> >>> It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s.
> >>>
> >>> Signed-off-by: Robert Marko <robimarko@gmail.com>
> >>> ---
> >>>  drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
> >>>  1 file changed, 12 insertions(+), 32 deletions(-)
> >>>
> >>> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> >>> index da55d2e1925a..9deaf9521d6d 100644
> >>> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> >>> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> >>> @@ -32,12 +32,6 @@
> >>>
> >>>  #include <dt-bindings/arm/qcom,ids.h>
> >>>
> >>> -enum _msm8996_version {
> >>> -     MSM8996_V3,
> >>> -     MSM8996_SG,
> >>> -     NUM_OF_MSM8996_VERSIONS,
> >>> -};
> >>> -
> >>>  struct qcom_cpufreq_drv;
> >>>
> >>>  struct qcom_cpufreq_match_data {
> >>> @@ -134,30 +128,16 @@ static void get_krait_bin_format_b(struct device *cpu_dev,
> >>>       dev_dbg(cpu_dev, "PVS version: %d\n", *pvs_ver);
> >>>  }
> >>>
> >>> -static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
> >>> +static int qcom_cpufreq_get_msm_id(void)
> >> This should be u32 as info->id is __le32

Nice catch.


> >>
> >> And please export this function from socinfo, it'll come in
> >> useful for other drivers!

I intentionally did not do that as socinfo is currently fully optional
and I dont really like
the idea of making it required for anything using SMEM.

Regards,
Robert

>
> Konrad
> >

Re: [PATCH 4/4] cpufreq: qcom-nvmem: make qcom_cpufreq_get_msm_id() return the SoC ID

[Konrad Dybcio]

On 3.03.2023 19:38, Robert Marko wrote:
> On Sat, 18 Feb 2023 at 21:40, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>>
>>
>> On 18.02.2023 21:36, Dmitry Baryshkov wrote:
>>> On Sat, 18 Feb 2023 at 16:43, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>>>
>>>>
>>>>
>>>> On 21.01.2023 12:29, Robert Marko wrote:
>>>>> Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID
>>>>> after getting it via SMEM call but instead uses an enum to encode the
>>>>> matched SMEM ID to 2 variants of MSM8996 which are then used in
>>>>> qcom_cpufreq_kryo_name_version() to set the supported version.
>>>>>
>>>>> This prevents qcom_cpufreq_get_msm_id() from being universal and its doing
>>>>> more than its name suggests, so lets make it just return the SoC ID
>>>>> directly which allows matching directly on the SoC ID and removes the need
>>>>> for msm8996_version enum which simplifies the driver.
>>>>> It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s.
>>>>>
>>>>> Signed-off-by: Robert Marko <robimarko@gmail.com>
>>>>> ---
>>>>>  drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
>>>>>  1 file changed, 12 insertions(+), 32 deletions(-)
>>>>>
>>>>> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
>>>>> index da55d2e1925a..9deaf9521d6d 100644
>>>>> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
>>>>> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
>>>>> @@ -32,12 +32,6 @@
>>>>>
>>>>>  #include <dt-bindings/arm/qcom,ids.h>
>>>>>
>>>>> -enum _msm8996_version {
>>>>> -     MSM8996_V3,
>>>>> -     MSM8996_SG,
>>>>> -     NUM_OF_MSM8996_VERSIONS,
>>>>> -};
>>>>> -
>>>>>  struct qcom_cpufreq_drv;
>>>>>
>>>>>  struct qcom_cpufreq_match_data {
>>>>> @@ -134,30 +128,16 @@ static void get_krait_bin_format_b(struct device *cpu_dev,
>>>>>       dev_dbg(cpu_dev, "PVS version: %d\n", *pvs_ver);
>>>>>  }
>>>>>
>>>>> -static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
>>>>> +static int qcom_cpufreq_get_msm_id(void)
>>>> This should be u32 as info->id is __le32
> 
> Nice catch.
> 
> 
>>>>
>>>> And please export this function from socinfo, it'll come in
>>>> useful for other drivers!
> 
> I intentionally did not do that as socinfo is currently fully optional
> and I dont really like
> the idea of making it required for anything using SMEM.
"anything using SMEM"? As in the drivers, or SoCs?
If the former, I don't see how exporting a function from within
socid and using it here would make it required for other drivers.
If the latter, we're talking non-qcom SoCs. SMEM has been with
us forever.


I'm planning to reuse this for Adreno speedbin matching. It's one
of those blocks that don't have a revision and/or bin reigster
within themselves.

Konrad
> 
> Regards,
> Robert
> 
>>
>> Konrad
>>>

Re: [PATCH 4/4] cpufreq: qcom-nvmem: make qcom_cpufreq_get_msm_id() return the SoC ID

[Robert Marko]

On Fri, 3 Mar 2023 at 21:46, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>
> On 3.03.2023 19:38, Robert Marko wrote:
> > On Sat, 18 Feb 2023 at 21:40, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> >>
> >>
> >>
> >> On 18.02.2023 21:36, Dmitry Baryshkov wrote:
> >>> On Sat, 18 Feb 2023 at 16:43, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 21.01.2023 12:29, Robert Marko wrote:
> >>>>> Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID
> >>>>> after getting it via SMEM call but instead uses an enum to encode the
> >>>>> matched SMEM ID to 2 variants of MSM8996 which are then used in
> >>>>> qcom_cpufreq_kryo_name_version() to set the supported version.
> >>>>>
> >>>>> This prevents qcom_cpufreq_get_msm_id() from being universal and its doing
> >>>>> more than its name suggests, so lets make it just return the SoC ID
> >>>>> directly which allows matching directly on the SoC ID and removes the need
> >>>>> for msm8996_version enum which simplifies the driver.
> >>>>> It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s.
> >>>>>
> >>>>> Signed-off-by: Robert Marko <robimarko@gmail.com>
> >>>>> ---
> >>>>>  drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
> >>>>>  1 file changed, 12 insertions(+), 32 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> >>>>> index da55d2e1925a..9deaf9521d6d 100644
> >>>>> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> >>>>> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> >>>>> @@ -32,12 +32,6 @@
> >>>>>
> >>>>>  #include <dt-bindings/arm/qcom,ids.h>
> >>>>>
> >>>>> -enum _msm8996_version {
> >>>>> -     MSM8996_V3,
> >>>>> -     MSM8996_SG,
> >>>>> -     NUM_OF_MSM8996_VERSIONS,
> >>>>> -};
> >>>>> -
> >>>>>  struct qcom_cpufreq_drv;
> >>>>>
> >>>>>  struct qcom_cpufreq_match_data {
> >>>>> @@ -134,30 +128,16 @@ static void get_krait_bin_format_b(struct device *cpu_dev,
> >>>>>       dev_dbg(cpu_dev, "PVS version: %d\n", *pvs_ver);
> >>>>>  }
> >>>>>
> >>>>> -static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
> >>>>> +static int qcom_cpufreq_get_msm_id(void)
> >>>> This should be u32 as info->id is __le32
> >
> > Nice catch.
> >
> >
> >>>>
> >>>> And please export this function from socinfo, it'll come in
> >>>> useful for other drivers!
> >
> > I intentionally did not do that as socinfo is currently fully optional
> > and I dont really like
> > the idea of making it required for anything using SMEM.
> "anything using SMEM"? As in the drivers, or SoCs?
> If the former, I don't see how exporting a function from within
> socid and using it here would make it required for other drivers.
> If the latter, we're talking non-qcom SoCs. SMEM has been with
> us forever.

I feel we have a misunderstanding,
currently, cpufreq-nvmem does not depend on socinfo being built
so I don't want to require it as a dependency in order to get the SMEM ID.

Granted, socinfo is useful on any QCA SoC so if adding new dependecies to
cpufreq-nvmem is acceptable I am not against exporting it there.
>
>
> I'm planning to reuse this for Adreno speedbin matching. It's one
> of those blocks that don't have a revision and/or bin reigster
> within themselves.

I understand the use case, I am sure it will be required in some other places
sooner or later as well.

Regards,
Robert
>
> Konrad
> >
> > Regards,
> > Robert
> >
> >>
> >> Konrad
> >>>

Re: [PATCH 4/4] cpufreq: qcom-nvmem: make qcom_cpufreq_get_msm_id() return the SoC ID

[Konrad Dybcio]

On 3.03.2023 22:38, Robert Marko wrote:
> On Fri, 3 Mar 2023 at 21:46, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>>
>>
>> On 3.03.2023 19:38, Robert Marko wrote:
>>> On Sat, 18 Feb 2023 at 21:40, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>>>
>>>>
>>>>
>>>> On 18.02.2023 21:36, Dmitry Baryshkov wrote:
>>>>> On Sat, 18 Feb 2023 at 16:43, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 21.01.2023 12:29, Robert Marko wrote:
>>>>>>> Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID
>>>>>>> after getting it via SMEM call but instead uses an enum to encode the
>>>>>>> matched SMEM ID to 2 variants of MSM8996 which are then used in
>>>>>>> qcom_cpufreq_kryo_name_version() to set the supported version.
>>>>>>>
>>>>>>> This prevents qcom_cpufreq_get_msm_id() from being universal and its doing
>>>>>>> more than its name suggests, so lets make it just return the SoC ID
>>>>>>> directly which allows matching directly on the SoC ID and removes the need
>>>>>>> for msm8996_version enum which simplifies the driver.
>>>>>>> It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s.
>>>>>>>
>>>>>>> Signed-off-by: Robert Marko <robimarko@gmail.com>
>>>>>>> ---
>>>>>>>  drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
>>>>>>>  1 file changed, 12 insertions(+), 32 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
>>>>>>> index da55d2e1925a..9deaf9521d6d 100644
>>>>>>> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
>>>>>>> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
>>>>>>> @@ -32,12 +32,6 @@
>>>>>>>
>>>>>>>  #include <dt-bindings/arm/qcom,ids.h>
>>>>>>>
>>>>>>> -enum _msm8996_version {
>>>>>>> -     MSM8996_V3,
>>>>>>> -     MSM8996_SG,
>>>>>>> -     NUM_OF_MSM8996_VERSIONS,
>>>>>>> -};
>>>>>>> -
>>>>>>>  struct qcom_cpufreq_drv;
>>>>>>>
>>>>>>>  struct qcom_cpufreq_match_data {
>>>>>>> @@ -134,30 +128,16 @@ static void get_krait_bin_format_b(struct device *cpu_dev,
>>>>>>>       dev_dbg(cpu_dev, "PVS version: %d\n", *pvs_ver);
>>>>>>>  }
>>>>>>>
>>>>>>> -static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
>>>>>>> +static int qcom_cpufreq_get_msm_id(void)
>>>>>> This should be u32 as info->id is __le32
>>>
>>> Nice catch.
>>>
>>>
>>>>>>
>>>>>> And please export this function from socinfo, it'll come in
>>>>>> useful for other drivers!
>>>
>>> I intentionally did not do that as socinfo is currently fully optional
>>> and I dont really like
>>> the idea of making it required for anything using SMEM.
>> "anything using SMEM"? As in the drivers, or SoCs?
>> If the former, I don't see how exporting a function from within
>> socid and using it here would make it required for other drivers.
>> If the latter, we're talking non-qcom SoCs. SMEM has been with
>> us forever.
> 
> I feel we have a misunderstanding,
> currently, cpufreq-nvmem does not depend on socinfo being built
> so I don't want to require it as a dependency in order to get the SMEM ID.
Okay yeah we simply weren't on the same page.

> 
> Granted, socinfo is useful on any QCA SoC so if adding new dependecies to
> cpufreq-nvmem is acceptable I am not against exporting it there.
IMO, it would be acceptable. Let's hear if others are on board too.

Konrad
>>
>>
>> I'm planning to reuse this for Adreno speedbin matching. It's one
>> of those blocks that don't have a revision and/or bin reigster
>> within themselves.
> 
> I understand the use case, I am sure it will be required in some other places
> sooner or later as well.
> 
> Regards,
> Robert
>>
>> Konrad
>>>
>>> Regards,
>>> Robert
>>>
>>>>
>>>> Konrad
>>>>>

Re: [PATCH 4/4] cpufreq: qcom-nvmem: make qcom_cpufreq_get_msm_id() return the SoC ID

[Dmitry Baryshkov]

On 03/03/2023 22:46, Konrad Dybcio wrote:
> 
> 
> On 3.03.2023 19:38, Robert Marko wrote:
>> On Sat, 18 Feb 2023 at 21:40, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>>
>>>
>>>
>>> On 18.02.2023 21:36, Dmitry Baryshkov wrote:
>>>> On Sat, 18 Feb 2023 at 16:43, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 21.01.2023 12:29, Robert Marko wrote:
>>>>>> Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID
>>>>>> after getting it via SMEM call but instead uses an enum to encode the
>>>>>> matched SMEM ID to 2 variants of MSM8996 which are then used in
>>>>>> qcom_cpufreq_kryo_name_version() to set the supported version.
>>>>>>
>>>>>> This prevents qcom_cpufreq_get_msm_id() from being universal and its doing
>>>>>> more than its name suggests, so lets make it just return the SoC ID
>>>>>> directly which allows matching directly on the SoC ID and removes the need
>>>>>> for msm8996_version enum which simplifies the driver.
>>>>>> It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s.
>>>>>>
>>>>>> Signed-off-by: Robert Marko <robimarko@gmail.com>
>>>>>> ---
>>>>>>   drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
>>>>>>   1 file changed, 12 insertions(+), 32 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
>>>>>> index da55d2e1925a..9deaf9521d6d 100644
>>>>>> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
>>>>>> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
>>>>>> @@ -32,12 +32,6 @@
>>>>>>
>>>>>>   #include <dt-bindings/arm/qcom,ids.h>
>>>>>>
>>>>>> -enum _msm8996_version {
>>>>>> -     MSM8996_V3,
>>>>>> -     MSM8996_SG,
>>>>>> -     NUM_OF_MSM8996_VERSIONS,
>>>>>> -};
>>>>>> -
>>>>>>   struct qcom_cpufreq_drv;
>>>>>>
>>>>>>   struct qcom_cpufreq_match_data {
>>>>>> @@ -134,30 +128,16 @@ static void get_krait_bin_format_b(struct device *cpu_dev,
>>>>>>        dev_dbg(cpu_dev, "PVS version: %d\n", *pvs_ver);
>>>>>>   }
>>>>>>
>>>>>> -static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
>>>>>> +static int qcom_cpufreq_get_msm_id(void)
>>>>> This should be u32 as info->id is __le32
>>
>> Nice catch.
>>
>>
>>>>>
>>>>> And please export this function from socinfo, it'll come in
>>>>> useful for other drivers!
>>
>> I intentionally did not do that as socinfo is currently fully optional
>> and I dont really like
>> the idea of making it required for anything using SMEM.
> "anything using SMEM"? As in the drivers, or SoCs?
> If the former, I don't see how exporting a function from within
> socid and using it here would make it required for other drivers.
> If the latter, we're talking non-qcom SoCs. SMEM has been with
> us forever.
> 
> 
> I'm planning to reuse this for Adreno speedbin matching. It's one
> of those blocks that don't have a revision and/or bin reigster
> within themselves.

I have mixed feelings towards this. And anyway it might be better to add 
get_msm_id() function to SCM driver, rather than parsing the data here.


-- 
With best wishes
Dmitry

Re: [PATCH 1/4] soc: qcom: socinfo: move SMEM item struct and defines to a header

[Christophe JAILLET]

Le 21/01/2023 à 12:29, Robert Marko a écrit :
> Move SMEM item struct and related defines to a header in order to be able
> to reuse them in the Qualcomm NVMEM CPUFreq driver instead of duplicating
> them.
> 
> Signed-off-by: Robert Marko <robimarko@gmail.com>
> ---
>   drivers/soc/qcom/socinfo.c       | 65 +-----------------------------
>   include/linux/soc/qcom/socinfo.h | 68 ++++++++++++++++++++++++++++++++
>   2 files changed, 69 insertions(+), 64 deletions(-)
>   create mode 100644 include/linux/soc/qcom/socinfo.h
> 
> diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
> index 3b970a80f3aa..5919f863f369 100644
> --- a/drivers/soc/qcom/socinfo.c
> +++ b/drivers/soc/qcom/socinfo.c
> @@ -11,6 +11,7 @@
>   #include <linux/random.h>
>   #include <linux/slab.h>
>   #include <linux/soc/qcom/smem.h>
> +#include <linux/soc/qcom/socinfo.h>
>   #include <linux/string.h>
>   #include <linux/stringify.h>
>   #include <linux/sys_soc.h>
> @@ -32,15 +33,6 @@
>   #define qcom_board_id(id) QCOM_ID_ ## id, __stringify(id)
>   #define qcom_board_id_named(id, name) QCOM_ID_ ## id, (name)
>   
> -#define SMEM_SOCINFO_BUILD_ID_LENGTH           32
> -#define SMEM_SOCINFO_CHIP_ID_LENGTH            32
> -
> -/*
> - * SMEM item id, used to acquire handles to respective
> - * SMEM region.
> - */
> -#define SMEM_HW_SW_BUILD_ID            137
> -
>   #ifdef CONFIG_DEBUG_FS
>   #define SMEM_IMAGE_VERSION_BLOCKS_COUNT        32
>   #define SMEM_IMAGE_VERSION_SIZE                4096
> @@ -121,62 +113,7 @@ static const char *const pmic_models[] = {
>   	[58] = "PM8450",
>   	[65] = "PM8010",
>   };
> -#endif /* CONFIG_DEBUG_FS */

This removal is spurious.

CJ


> -
> -/* Socinfo SMEM item structure */
> -struct socinfo {
> -	__le32 fmt;
> -	__le32 id;
> -	__le32 ver;
> -	char build_id[SMEM_SOCINFO_BUILD_ID_LENGTH];
> -	/* Version 2 */
> -	__le32 raw_id;
> -	__le32 raw_ver;
> -	/* Version 3 */
> -	__le32 hw_plat;
> -	/* Version 4 */
> -	__le32 plat_ver;
> -	/* Version 5 */
> -	__le32 accessory_chip;
> -	/* Version 6 */
> -	__le32 hw_plat_subtype;
> -	/* Version 7 */
> -	__le32 pmic_model;
> -	__le32 pmic_die_rev;
> -	/* Version 8 */
> -	__le32 pmic_model_1;
> -	__le32 pmic_die_rev_1;
> -	__le32 pmic_model_2;
> -	__le32 pmic_die_rev_2;
> -	/* Version 9 */
> -	__le32 foundry_id;
> -	/* Version 10 */
> -	__le32 serial_num;
> -	/* Version 11 */
> -	__le32 num_pmics;
> -	__le32 pmic_array_offset;
> -	/* Version 12 */
> -	__le32 chip_family;
> -	__le32 raw_device_family;
> -	__le32 raw_device_num;
> -	/* Version 13 */
> -	__le32 nproduct_id;
> -	char chip_id[SMEM_SOCINFO_CHIP_ID_LENGTH];
> -	/* Version 14 */
> -	__le32 num_clusters;
> -	__le32 ncluster_array_offset;
> -	__le32 num_defective_parts;
> -	__le32 ndefective_parts_array_offset;
> -	/* Version 15 */
> -	__le32 nmodem_supported;
> -	/* Version 16 */
> -	__le32  feature_code;
> -	__le32  pcode;
> -	__le32  npartnamemap_offset;
> -	__le32  nnum_partname_mapping;
> -};
>   
> -#ifdef CONFIG_DEBUG_FS
>   struct socinfo_params {
>   	u32 raw_device_family;
>   	u32 hw_plat_subtype;
> diff --git a/include/linux/soc/qcom/socinfo.h b/include/linux/soc/qcom/socinfo.h
> new file mode 100644
> index 000000000000..6a175d635617
> --- /dev/null
> +++ b/include/linux/soc/qcom/socinfo.h
> @@ -0,0 +1,68 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __QCOM_SOCINFO_H__
> +#define __QCOM_SOCINFO_H__
> +
> +/*
> + * SMEM item id, used to acquire handles to respective
> + * SMEM region.
> + */
> +#define SMEM_HW_SW_BUILD_ID		137
> +
> +#define SMEM_SOCINFO_BUILD_ID_LENGTH	32
> +#define SMEM_SOCINFO_CHIP_ID_LENGTH	32
> +
> +/* Socinfo SMEM item structure */
> +struct socinfo {
> +	__le32 fmt;
> +	__le32 id;
> +	__le32 ver;
> +	char build_id[SMEM_SOCINFO_BUILD_ID_LENGTH];
> +	/* Version 2 */
> +	__le32 raw_id;
> +	__le32 raw_ver;
> +	/* Version 3 */
> +	__le32 hw_plat;
> +	/* Version 4 */
> +	__le32 plat_ver;
> +	/* Version 5 */
> +	__le32 accessory_chip;
> +	/* Version 6 */
> +	__le32 hw_plat_subtype;
> +	/* Version 7 */
> +	__le32 pmic_model;
> +	__le32 pmic_die_rev;
> +	/* Version 8 */
> +	__le32 pmic_model_1;
> +	__le32 pmic_die_rev_1;
> +	__le32 pmic_model_2;
> +	__le32 pmic_die_rev_2;
> +	/* Version 9 */
> +	__le32 foundry_id;
> +	/* Version 10 */
> +	__le32 serial_num;
> +	/* Version 11 */
> +	__le32 num_pmics;
> +	__le32 pmic_array_offset;
> +	/* Version 12 */
> +	__le32 chip_family;
> +	__le32 raw_device_family;
> +	__le32 raw_device_num;
> +	/* Version 13 */
> +	__le32 nproduct_id;
> +	char chip_id[SMEM_SOCINFO_CHIP_ID_LENGTH];
> +	/* Version 14 */
> +	__le32 num_clusters;
> +	__le32 ncluster_array_offset;
> +	__le32 num_defective_parts;
> +	__le32 ndefective_parts_array_offset;
> +	/* Version 15 */
> +	__le32 nmodem_supported;
> +	/* Version 16 */
> +	__le32  feature_code;
> +	__le32  pcode;
> +	__le32  npartnamemap_offset;
> +	__le32  nnum_partname_mapping;
> +};
> +
> +#endif

Re: [PATCH 1/4] soc: qcom: socinfo: move SMEM item struct and defines to a header

[Robert Marko]

On Sat, 21 Jan 2023 at 12:33, Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> Le 21/01/2023 à 12:29, Robert Marko a écrit :
> > Move SMEM item struct and related defines to a header in order to be able
> > to reuse them in the Qualcomm NVMEM CPUFreq driver instead of duplicating
> > them.
> >
> > Signed-off-by: Robert Marko <robimarko@gmail.com>
> > ---
> >   drivers/soc/qcom/socinfo.c       | 65 +-----------------------------
> >   include/linux/soc/qcom/socinfo.h | 68 ++++++++++++++++++++++++++++++++
> >   2 files changed, 69 insertions(+), 64 deletions(-)
> >   create mode 100644 include/linux/soc/qcom/socinfo.h
> >
> > diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
> > index 3b970a80f3aa..5919f863f369 100644
> > --- a/drivers/soc/qcom/socinfo.c
> > +++ b/drivers/soc/qcom/socinfo.c
> > @@ -11,6 +11,7 @@
> >   #include <linux/random.h>
> >   #include <linux/slab.h>
> >   #include <linux/soc/qcom/smem.h>
> > +#include <linux/soc/qcom/socinfo.h>
> >   #include <linux/string.h>
> >   #include <linux/stringify.h>
> >   #include <linux/sys_soc.h>
> > @@ -32,15 +33,6 @@
> >   #define qcom_board_id(id) QCOM_ID_ ## id, __stringify(id)
> >   #define qcom_board_id_named(id, name) QCOM_ID_ ## id, (name)
> >
> > -#define SMEM_SOCINFO_BUILD_ID_LENGTH           32
> > -#define SMEM_SOCINFO_CHIP_ID_LENGTH            32
> > -
> > -/*
> > - * SMEM item id, used to acquire handles to respective
> > - * SMEM region.
> > - */
> > -#define SMEM_HW_SW_BUILD_ID            137
> > -
> >   #ifdef CONFIG_DEBUG_FS
> >   #define SMEM_IMAGE_VERSION_BLOCKS_COUNT        32
> >   #define SMEM_IMAGE_VERSION_SIZE                4096
> > @@ -121,62 +113,7 @@ static const char *const pmic_models[] = {
> >       [58] = "PM8450",
> >       [65] = "PM8010",
> >   };
> > -#endif /* CONFIG_DEBUG_FS */
>
> This removal is spurious.

Hi,
Its intentional as only the socinfo struct was not guarded with an
debugfs ifdef,
so after moving it to a separate header you would have:

#endif /* CONFIG_DEBUG_FS */
#ifdef CONFIG_DEBUG_FS

This doesn't really make sense to me, so that is why endif is removed.

Regards,
Robert
>
> CJ
>
>
> > -
> > -/* Socinfo SMEM item structure */
> > -struct socinfo {
> > -     __le32 fmt;
> > -     __le32 id;
> > -     __le32 ver;
> > -     char build_id[SMEM_SOCINFO_BUILD_ID_LENGTH];
> > -     /* Version 2 */
> > -     __le32 raw_id;
> > -     __le32 raw_ver;
> > -     /* Version 3 */
> > -     __le32 hw_plat;
> > -     /* Version 4 */
> > -     __le32 plat_ver;
> > -     /* Version 5 */
> > -     __le32 accessory_chip;
> > -     /* Version 6 */
> > -     __le32 hw_plat_subtype;
> > -     /* Version 7 */
> > -     __le32 pmic_model;
> > -     __le32 pmic_die_rev;
> > -     /* Version 8 */
> > -     __le32 pmic_model_1;
> > -     __le32 pmic_die_rev_1;
> > -     __le32 pmic_model_2;
> > -     __le32 pmic_die_rev_2;
> > -     /* Version 9 */
> > -     __le32 foundry_id;
> > -     /* Version 10 */
> > -     __le32 serial_num;
> > -     /* Version 11 */
> > -     __le32 num_pmics;
> > -     __le32 pmic_array_offset;
> > -     /* Version 12 */
> > -     __le32 chip_family;
> > -     __le32 raw_device_family;
> > -     __le32 raw_device_num;
> > -     /* Version 13 */
> > -     __le32 nproduct_id;
> > -     char chip_id[SMEM_SOCINFO_CHIP_ID_LENGTH];
> > -     /* Version 14 */
> > -     __le32 num_clusters;
> > -     __le32 ncluster_array_offset;
> > -     __le32 num_defective_parts;
> > -     __le32 ndefective_parts_array_offset;
> > -     /* Version 15 */
> > -     __le32 nmodem_supported;
> > -     /* Version 16 */
> > -     __le32  feature_code;
> > -     __le32  pcode;
> > -     __le32  npartnamemap_offset;
> > -     __le32  nnum_partname_mapping;
> > -};
> >
> > -#ifdef CONFIG_DEBUG_FS
> >   struct socinfo_params {
> >       u32 raw_device_family;
> >       u32 hw_plat_subtype;
> > diff --git a/include/linux/soc/qcom/socinfo.h b/include/linux/soc/qcom/socinfo.h
> > new file mode 100644
> > index 000000000000..6a175d635617
> > --- /dev/null
> > +++ b/include/linux/soc/qcom/socinfo.h
> > @@ -0,0 +1,68 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#ifndef __QCOM_SOCINFO_H__
> > +#define __QCOM_SOCINFO_H__
> > +
> > +/*
> > + * SMEM item id, used to acquire handles to respective
> > + * SMEM region.
> > + */
> > +#define SMEM_HW_SW_BUILD_ID          137
> > +
> > +#define SMEM_SOCINFO_BUILD_ID_LENGTH 32
> > +#define SMEM_SOCINFO_CHIP_ID_LENGTH  32
> > +
> > +/* Socinfo SMEM item structure */
> > +struct socinfo {
> > +     __le32 fmt;
> > +     __le32 id;
> > +     __le32 ver;
> > +     char build_id[SMEM_SOCINFO_BUILD_ID_LENGTH];
> > +     /* Version 2 */
> > +     __le32 raw_id;
> > +     __le32 raw_ver;
> > +     /* Version 3 */
> > +     __le32 hw_plat;
> > +     /* Version 4 */
> > +     __le32 plat_ver;
> > +     /* Version 5 */
> > +     __le32 accessory_chip;
> > +     /* Version 6 */
> > +     __le32 hw_plat_subtype;
> > +     /* Version 7 */
> > +     __le32 pmic_model;
> > +     __le32 pmic_die_rev;
> > +     /* Version 8 */
> > +     __le32 pmic_model_1;
> > +     __le32 pmic_die_rev_1;
> > +     __le32 pmic_model_2;
> > +     __le32 pmic_die_rev_2;
> > +     /* Version 9 */
> > +     __le32 foundry_id;
> > +     /* Version 10 */
> > +     __le32 serial_num;
> > +     /* Version 11 */
> > +     __le32 num_pmics;
> > +     __le32 pmic_array_offset;
> > +     /* Version 12 */
> > +     __le32 chip_family;
> > +     __le32 raw_device_family;
> > +     __le32 raw_device_num;
> > +     /* Version 13 */
> > +     __le32 nproduct_id;
> > +     char chip_id[SMEM_SOCINFO_CHIP_ID_LENGTH];
> > +     /* Version 14 */
> > +     __le32 num_clusters;
> > +     __le32 ncluster_array_offset;
> > +     __le32 num_defective_parts;
> > +     __le32 ndefective_parts_array_offset;
> > +     /* Version 15 */
> > +     __le32 nmodem_supported;
> > +     /* Version 16 */
> > +     __le32  feature_code;
> > +     __le32  pcode;
> > +     __le32  npartnamemap_offset;
> > +     __le32  nnum_partname_mapping;
> > +};
> > +
> > +#endif
>

Re: (subset) [PATCH 1/4] soc: qcom: socinfo: move SMEM item struct and defines to a header

[Bjorn Andersson]

On Sat, 21 Jan 2023 12:29:44 +0100, Robert Marko wrote:
> Move SMEM item struct and related defines to a header in order to be able
> to reuse them in the Qualcomm NVMEM CPUFreq driver instead of duplicating
> them.
> 
> 

Applied, thanks!

[1/4] soc: qcom: socinfo: move SMEM item struct and defines to a header
      commit: ec001bb71e4476f7f5be9db693d5f43e65b9d8cb

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>