2023-08-10 06:55:33

by Komal Bajaj

[permalink] [raw]
Subject: [PATCH 4/6] soc: qcom: Add LLCC support for multi channel DDR

Add LLCC support for multi channel DDR configuration
based on a feature register.

Signed-off-by: Komal Bajaj <[email protected]>
---
drivers/soc/qcom/llcc-qcom.c | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index 76e8083d053d..4fad2cff5e1e 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -12,6 +12,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/nvmem-consumer.h>
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/regmap.h>
@@ -998,6 +999,24 @@ static int qcom_llcc_cfg_program(struct platform_device *pdev,
return ret;
}

+static int qcom_llcc_get_cfg_index(struct platform_device *pdev, u8 *cfg_index, int num_config)
+{
+ int ret;
+
+ ret = nvmem_cell_read_u8(&pdev->dev, "multi-chan-ddr", cfg_index);
+ if (ret == -ENOENT || ret == -EOPNOTSUPP) {
+ if (num_config != DEF_NUM_CFG)
+ return -EINVAL;
+ *cfg_index = DEF_NUM_CFG - 1;
+ return 0;
+ }
+
+ if (!ret && *cfg_index >= num_config)
+ ret = -EINVAL;
+
+ return ret;
+}
+
static int qcom_llcc_remove(struct platform_device *pdev)
{
/* Set the global pointer to a error code to avoid referencing it */
@@ -1034,6 +1053,7 @@ static int qcom_llcc_probe(struct platform_device *pdev)
const struct qcom_llcc_config *cfg;
const struct llcc_slice_config *llcc_cfg;
u32 sz;
+ u8 cfg_index;
u32 version;
struct regmap *regmap;

@@ -1051,11 +1071,14 @@ static int qcom_llcc_probe(struct platform_device *pdev)
}

cfgs = of_device_get_match_data(&pdev->dev);
- if (!cfgs || cfgs->num_config != DEF_NUM_CFG) {
+ if (!cfgs) {
ret = -EINVAL;
goto err;
}
- cfg = &cfgs->llcc_config[DEF_NUM_CFG - 1];
+ ret = qcom_llcc_get_cfg_index(pdev, &cfg_index, cfgs->num_config);
+ if (ret)
+ goto err;
+ cfg = &cfgs->llcc_config[cfg_index];

ret = regmap_read(regmap, cfg->reg_offset[LLCC_COMMON_STATUS0], &num_banks);
if (ret)
--
2.41.0



2023-08-10 12:21:19

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH 4/6] soc: qcom: Add LLCC support for multi channel DDR

On 10/08/2023 07:11, Komal Bajaj wrote:
> Add LLCC support for multi channel DDR configuration
> based on a feature register.
>
> Signed-off-by: Komal Bajaj <[email protected]>
> ---
> drivers/soc/qcom/llcc-qcom.c | 27 +++++++++++++++++++++++++--
> 1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
> index 76e8083d053d..4fad2cff5e1e 100644
> --- a/drivers/soc/qcom/llcc-qcom.c
> +++ b/drivers/soc/qcom/llcc-qcom.c
> @@ -12,6 +12,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> +#include <linux/nvmem-consumer.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/regmap.h>
> @@ -998,6 +999,24 @@ static int qcom_llcc_cfg_program(struct platform_device *pdev,
> return ret;
> }
>
> +static int qcom_llcc_get_cfg_index(struct platform_device *pdev, u8 *cfg_index, int num_config)
> +{
> + int ret;
> +
> + ret = nvmem_cell_read_u8(&pdev->dev, "multi-chan-ddr", cfg_index);
> + if (ret == -ENOENT || ret == -EOPNOTSUPP) {
> + if (num_config != DEF_NUM_CFG)
> + return -EINVAL;
> + *cfg_index = DEF_NUM_CFG - 1;
> + return 0;
> + }
> +
> + if (!ret && *cfg_index >= num_config)
> + ret = -EINVAL;
> +
> + return ret;
> +}
> +
> static int qcom_llcc_remove(struct platform_device *pdev)
> {
> /* Set the global pointer to a error code to avoid referencing it */
> @@ -1034,6 +1053,7 @@ static int qcom_llcc_probe(struct platform_device *pdev)
> const struct qcom_llcc_config *cfg;
> const struct llcc_slice_config *llcc_cfg;
> u32 sz;
> + u8 cfg_index;
> u32 version;
> struct regmap *regmap;
>
> @@ -1051,11 +1071,14 @@ static int qcom_llcc_probe(struct platform_device *pdev)
> }
>
> cfgs = of_device_get_match_data(&pdev->dev);
> - if (!cfgs || cfgs->num_config != DEF_NUM_CFG) {
> + if (!cfgs) {
> ret = -EINVAL;
> goto err;
> }
> - cfg = &cfgs->llcc_config[DEF_NUM_CFG - 1];
> + ret = qcom_llcc_get_cfg_index(pdev, &cfg_index, cfgs->num_config);
> + if (ret)
> + goto err;
> + cfg = &cfgs->llcc_config[cfg_index];
>
> ret = regmap_read(regmap, cfg->reg_offset[LLCC_COMMON_STATUS0], &num_banks);
> if (ret)
> --
> 2.41.0
>

This patch doesn't apply to -next

deckard@sagittarius-a:~/Development/qualcomm/qlt-kernel$ git checkout -b
llc-review-v0 linux-next/master
branch 'llc-review-v0' set up to track 'linux-next/master'.
Switched to a new branch 'llc-review-v0'
deckard@sagittarius-a:~/Development/qualcomm/qlt-kernel$ git am <
../patches/linux/lllc-review/v0/\[PATCH\ 1_6\]\ dt-bindings\:\ cache\:\
qcom\,llcc\:\ Add\ LLCC\ compatible\ for\ QDU1000_QRU1000\ -\ Komal\
Bajaj\ \<[email protected]\>\ -\ 2023-08-10\ 0711.eml
Applying: dt-bindings: cache: qcom,llcc: Add LLCC compatible for
QDU1000/QRU1000
deckard@sagittarius-a:~/Development/qualcomm/qlt-kernel$ git am <
../patches/linux/lllc-review/v0/\[PATCH\ 2_6\]\ soc\:\ qcom\:\ llcc\:\
Refactor\ llcc\ driver\ to\ support\ multiple\ configuration\ -\ Komal\
Bajaj\ \<[email protected]\>\ -\ 2023-08-10\ 0711.eml
Applying: soc: qcom: llcc: Refactor llcc driver to support multiple
configuration
deckard@sagittarius-a:~/Development/qualcomm/qlt-kernel$ git am <
../patches/linux/lllc-review/v0/\[PATCH\ 3_6\]\ nvmem\:\ core\:\ Add\
stub\ for\ nvmem_cell_read_u8\ -\ Komal\ Bajaj\
\<[email protected]\>\ -\ 2023-08-10\ 0711.eml
Applying: nvmem: core: Add stub for nvmem_cell_read_u8
deckard@sagittarius-a:~/Development/qualcomm/qlt-kernel$ git am <
../patches/linux/lllc-review/v0/\[PATCH\ 4_6\]\ soc\:\ qcom\:\ Add\
LLCC\ support\ for\ multi\ channel\ DDR\ -\ Komal\ Bajaj\
\<[email protected]\>\ -\ 2023-08-10\ 0711.eml
Applying: soc: qcom: Add LLCC support for multi channel DDR
error: patch failed: drivers/soc/qcom/llcc-qcom.c:12
error: drivers/soc/qcom/llcc-qcom.c: patch does not apply
Patch failed at 0001 soc: qcom: Add LLCC support for multi channel DDR
hint: Use 'git am --show-current-patch=diff' to see the failed patch

6f48f84d73ab5 (HEAD -> llc-review-v0) HEAD@{0}: am --abort
6f48f84d73ab5 (HEAD -> llc-review-v0) HEAD@{1}: am: nvmem: core: Add
stub for nvmem_cell_read_u8
7e5adcab05af4 HEAD@{2}: am: soc: qcom: llcc: Refactor llcc driver to
support multiple configuration
0990c31774948 HEAD@{3}: am: dt-bindings: cache: qcom,llcc: Add LLCC
compatible for QDU1000/QRU1000
21ef7b1e17d03 (tag: next-20230809, linux-next/master) HEAD@{4}:
checkout: moving from linux-next-23-08-07-db410c-rb3-camss-dts-v2 to
llc-review-v0

---
bod

2023-08-10 14:08:44

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH 4/6] soc: qcom: Add LLCC support for multi channel DDR

On 10/08/2023 07:11, Komal Bajaj wrote:
> + ret = nvmem_cell_read_u8(&pdev->dev, "multi-chan-ddr", cfg_index);
> + if (ret == -ENOENT || ret == -EOPNOTSUPP) {
> + if (num_config != DEF_NUM_CFG)
> + return -EINVAL;

In other words if multi-chan-ddr is not present in the dts and the
num_config != 1 return -EINVAL

You can just as easily say if (num_config > 1) and drop the define from
this code.

> + *cfg_index = DEF_NUM_CFG - 1;
> + return 0;

*cfg_index = 0;

For example if #define DEF_NUM_CFG 0x20 then taking the last index of it
would be 100% wrong.

Please kill that define.

---
bod