soc: qcom: socinfo: move SMEM item struct and defines to a header

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 <[email protected]>

---
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

drivers/cpufreq/qcom-cpufreq-nvmem.c +5 -9

index a577586b23be..c0a7841a56c1 100644
@@ -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;

Signature · 1 line

2.39.1

Quoted 6 lines · 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.

Quoted 1 line

Signed-off-by Robert Marko <[email protected]>

Reviewed-by Bjorn Andersson <[email protected]>

@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.

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 <[email protected]>

---

 drivers/cpufreq/qcom-cpufreq-nvmem.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

drivers/cpufreq/qcom-cpufreq-nvmem.c +5 -10

index c0a7841a56c1..da55d2e1925a 100644
@@ -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:

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 <[email protected]>

---
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

drivers/cpufreq/qcom-cpufreq-nvmem.c +5 -10

index c0a7841a56c1..da55d2e1925a 100644
@@ -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:

Signature · 1 line

2.39.1

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 <[email protected]>

---
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
 1 file changed, 12 insertions(+), 32 deletions(-)

drivers/cpufreq/qcom-cpufreq-nvmem.c +12 -32

index da55d2e1925a..9deaf9521d6d 100644
@@ -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:

Signature · 1 line

2.39.1

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 <[email protected]>

---
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
 1 file changed, 12 insertions(+), 32 deletions(-)

drivers/cpufreq/qcom-cpufreq-nvmem.c +0 -6

index da55d2e1925a..9deaf9521d6d 100644
@@ -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 {

drivers/cpufreq/qcom-cpufreq-nvmem.c +11 -25

 {
 	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:

Quoted 37 lines · 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 <[email protected]>

---
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
 1 file changed, 12 insertions(+), 32 deletions(-)

drivers/cpufreq/qcom-cpufreq-nvmem.c +0 -6

index da55d2e1925a..9deaf9521d6d 100644
@@ -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);
 }
 

drivers/cpufreq/qcom-cpufreq-nvmem.c static void get_krait_bin_format_b(struct device *cpu_dev, · line 131

Quoted 2 lines

drivers/cpufreq/qcom-cpufreq-nvmem.c +1 -1

-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!

Quoted 45 lines · Konrad Dybcio

Quoted 34 lines · 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 <[email protected]>

---
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
 1 file changed, 12 insertions(+), 32 deletions(-)

drivers/cpufreq/qcom-cpufreq-nvmem.c +0 -6

index da55d2e1925a..9deaf9521d6d 100644
@@ -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 {

drivers/cpufreq/qcom-cpufreq-nvmem.c static void get_krait_bin_format_b(struct device *cpu_dev, · line 131

Quoted 5 lines

drivers/cpufreq/qcom-cpufreq-nvmem.c +1 -1

@@ -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

Quoted 2 lines

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.

Quoted 37 lines · Dmitry Baryshkov

Quoted 36 lines · Konrad Dybcio

Quoted 32 lines · 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 <[email protected]>

---
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
 1 file changed, 12 insertions(+), 32 deletions(-)

drivers/cpufreq/qcom-cpufreq-nvmem.c +0 -6

index da55d2e1925a..9deaf9521d6d 100644
@@ -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;
 

Quoted 8 lines

Quoted 8 lines

drivers/cpufreq/qcom-cpufreq-nvmem.c static void get_krait_bin_format_b(struct device *cpu_dev, · line 131

Quoted 7 lines

drivers/cpufreq/qcom-cpufreq-nvmem.c +1 -1

 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

Quoted 3 lines

Quoted 3 lines

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

Konrad

Quoted 1 line

Quoted 41 lines · Konrad Dybcio

Quoted 37 lines · Dmitry Baryshkov

Quoted 36 lines · Konrad Dybcio

Quoted 32 lines · 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 <[email protected]>

---
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
 1 file changed, 12 insertions(+), 32 deletions(-)

drivers/cpufreq/qcom-cpufreq-nvmem.c +0 -6

index da55d2e1925a..9deaf9521d6d 100644
@@ -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;
 

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

drivers/cpufreq/qcom-cpufreq-nvmem.c static void get_krait_bin_format_b(struct device *cpu_dev, · line 131

Quoted 7 lines

drivers/cpufreq/qcom-cpufreq-nvmem.c +1 -1

 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.

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

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

Quoted 3 lines

Konrad

Quoted 1 line

Quoted 58 lines · Robert Marko

Quoted 41 lines · Konrad Dybcio

Quoted 37 lines · Dmitry Baryshkov

Quoted 36 lines · Konrad Dybcio

Quoted 32 lines · 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 <[email protected]>

---
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
 1 file changed, 12 insertions(+), 32 deletions(-)

drivers/cpufreq/qcom-cpufreq-nvmem.c +0 -6

index da55d2e1925a..9deaf9521d6d 100644
@@ -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;
 

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

drivers/cpufreq/qcom-cpufreq-nvmem.c static void get_krait_bin_format_b(struct device *cpu_dev, · line 131

Quoted 7 lines

drivers/cpufreq/qcom-cpufreq-nvmem.c +1 -1

 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.

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

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

Quoted 3 lines

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

Quoted 7 lines

Regards,
Robert

Quoted 3 lines

Konrad

Quoted 1 line

Quoted 62 lines · Konrad Dybcio

Quoted 58 lines · Robert Marko

Quoted 41 lines · Konrad Dybcio

Quoted 37 lines · Dmitry Baryshkov

Quoted 36 lines · Konrad Dybcio

Quoted 32 lines · 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 <[email protected]>

---
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
 1 file changed, 12 insertions(+), 32 deletions(-)

drivers/cpufreq/qcom-cpufreq-nvmem.c +0 -6

index da55d2e1925a..9deaf9521d6d 100644
@@ -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;
 

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

drivers/cpufreq/qcom-cpufreq-nvmem.c static void get_krait_bin_format_b(struct device *cpu_dev, · line 131

Quoted 7 lines

drivers/cpufreq/qcom-cpufreq-nvmem.c +1 -1

 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.

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

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

Quoted 8 lines

Quoted 3 lines

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.

Quoted 5 lines

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

Quoted 9 lines

Konrad

Quoted 7 lines

Regards,
Robert

Quoted 3 lines

Konrad

Quoted 1 line

Quoted 55 lines · Robert Marko

Quoted 41 lines · Konrad Dybcio

Quoted 37 lines · Dmitry Baryshkov

Quoted 36 lines · Konrad Dybcio

Quoted 32 lines · 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 <[email protected]>

---
  drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
  1 file changed, 12 insertions(+), 32 deletions(-)

drivers/cpufreq/qcom-cpufreq-nvmem.c +0 -6

index da55d2e1925a..9deaf9521d6d 100644
@@ -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;
 

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

drivers/cpufreq/qcom-cpufreq-nvmem.c static void get_krait_bin_format_b(struct device *cpu_dev, · line 131

Quoted 7 lines

drivers/cpufreq/qcom-cpufreq-nvmem.c +1 -1

  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.

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 6 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

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.

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 <[email protected]>

---
  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

drivers/soc/qcom/socinfo.c +1 -9

index 3b970a80f3aa..5919f863f369 100644
@@ -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

drivers/soc/qcom/socinfo.c +0 -54

-
-/* 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;

include/linux/soc/qcom/socinfo.h new file +68 -0

new file mode 100644
index 000000000000..6a175d635617
@@ -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

Quoted 38 lines · 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 <[email protected]>

---
  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

drivers/soc/qcom/socinfo.c +1 -9

index 3b970a80f3aa..5919f863f369 100644
@@ -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

drivers/soc/qcom/socinfo.c static const char *const pmic_models[] = { · line 124

Quoted 6 lines

drivers/soc/qcom/socinfo.c +0 -1

  #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

Quoted 132 lines

drivers/soc/qcom/socinfo.c +0 -54

-
-/* 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;

include/linux/soc/qcom/socinfo.h new file +68 -0

new file mode 100644
index 000000000000..6a175d635617
@@ -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

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.