Re: [PATCH v4 5/6] soc: qcom: Add LLCC support for multi channel DDR

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 6/28/2023 6:44 PM, Dmitry Baryshkov wrote:
On 28/06/2023 11:45, Komal Bajaj wrote:

No HTML emails on public mailing lists, please.



On 6/23/2023 7:56 PM, Dmitry Baryshkov wrote:
On Fri, 23 Jun 2023 at 17:19, Komal Bajaj<quic_kbajaj@xxxxxxxxxxx>  wrote:
Add LLCC support for multi channel DDR configuration
based on a feature register. Reading DDR channel
confiuration uses nvmem framework, so select the
dependency in Kconfig. Without this, there will be
errors while building the driver with COMPILE_TEST only.

Signed-off-by: Komal Bajaj<quic_kbajaj@xxxxxxxxxxx>
---
  drivers/soc/qcom/Kconfig     |  2 ++
  drivers/soc/qcom/llcc-qcom.c | 33 ++++++++++++++++++++++++++++++---
  2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index a491718f8064..cc9ad41c63aa 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -64,6 +64,8 @@ config QCOM_LLCC
         tristate "Qualcomm Technologies, Inc. LLCC driver"
         depends on ARCH_QCOM || COMPILE_TEST
         select REGMAP_MMIO
+       select NVMEM
No need to select NVMEM. The used functions are stubbed if NVMEM is disabled

With the previous patch, where this config was not selected, below error was flagged by kernel test robot -

    drivers/soc/qcom/llcc-qcom.c: In function 'qcom_llcc_get_cfg_index':
     >> drivers/soc/qcom/llcc-qcom.c:951:15: error: implicit declaration
    of function 'nvmem_cell_read_u8'; did you mean
    'nvmem_cell_read_u64'? [-Werror=implicit-function-declaration]
          951 |         ret = nvmem_cell_read_u8(&pdev->dev,
    "multi_chan_ddr", cfg_index);
              |               ^~~~~~~~~~~~~~~~~~
              |               nvmem_cell_read_u64
        cc1: some warnings being treated as errors

Judging from the rest of nvmem-consumer.h, it appears that not having stubs for this function is an omission. Please fix the header instead.

Okay, I will add the stub for this function in the header.



+       select QCOM_SCM
         help
           Qualcomm Technologies, Inc. platform specific
           Last Level Cache Controller(LLCC) driver for platforms such as, diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index 6cf373da5df9..3c29612da1c5 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>
@@ -943,6 +944,19 @@ 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 ret;
+
+       ret = nvmem_cell_read_u8(&pdev->dev, "multi-chan-ddr", cfg_index);
+       if (ret == -ENOENT) {
|| ret == -EOPNOTSUPP ?

Okay

+               *cfg_index = 0;
+               return 0;
+       }
+
+       return ret;
+}
+
  static int qcom_llcc_remove(struct platform_device *pdev)
  {
         /* Set the global pointer to a error code to avoid referencing it */ @@ -975,11 +989,13 @@ static int qcom_llcc_probe(struct platform_device *pdev)
         struct device *dev = &pdev->dev;
         int ret, i;
         struct platform_device *llcc_edac;
-       const struct qcom_llcc_config *cfg;
+       const struct qcom_llcc_config *cfg, *entry;
         const struct llcc_slice_config *llcc_cfg;
         u32 sz;
+       u8 cfg_index;
         u32 version;
         struct regmap *regmap;
+       u32 num_entries = 0;

         drv_data = devm_kzalloc(dev, sizeof(*drv_data), GFP_KERNEL);
         if (!drv_data) {
@@ -1040,8 +1056,19 @@ static int qcom_llcc_probe(struct platform_device *pdev)

         drv_data->version = version;

-       llcc_cfg = cfg[0]->sct_data;
-       sz = cfg[0]->size;
+       ret = qcom_llcc_get_cfg_index(pdev, &cfg_index);
+       if (ret)
+               goto err;
+
+       for (entry = cfg; entry->sct_data; entry++, num_entries++)
+               ;
Please add num_cfgs to the configuration data instead.

Shall I create a new wrapper struct having a field num_cfg and a pointer to those cfgs because configuration data is itself an instance of "struct qcom_llcc_config" and
we can have multiple instances of it.

A wrapper struct is a better approach in my opinion.

Okay, will follow this approach then.

Thanks
Komal



+       if (cfg_index >= num_entries || cfg_index < 0) {
cfg_index is unsigned, so it can not be less than 0.

Okay.

+               ret = -EINVAL;
+               goto err;
+       }
+
+       llcc_cfg = cfg[cfg_index].sct_data;
+       sz = cfg[cfg_index].size;

         for (i = 0; i < sz; i++)
                 if (llcc_cfg[i].slice_id > drv_data->max_slices)
--
2.40.1







[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux