* Re: [PATCH V0 3/6] soc: qcom: dcc:Add driver support for Data Capture and Compare unit(DCC)
[not found] ` <f182b10f318db7cb09216c0176a5b26656d9ef49.1613541226.git.schowdhu@codeaurora.org>
@ 2021-02-17 8:08 ` kernel test robot
2021-02-18 6:59 ` Vinod Koul
1 sibling, 0 replies; 5+ messages in thread
From: kernel test robot @ 2021-02-17 8:08 UTC (permalink / raw)
To: Souradeep Chowdhury, Andy Gross, Bjorn Andersson, Rob Herring
Cc: devicetree, Sai Prakash Ranjan, Rajendra Nayak, linux-arm-msm,
linux-kernel, Sibi Sankar, kbuild-all, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 5717 bytes --]
Hi Souradeep,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v5.11 next-20210216]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Souradeep-Chowdhury/Add-driver-support-for-Data-Capture-and-Compare-Engine-DCC-for-SM8150/20210217-145428
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/98f7664a4e41764c0d2111d6b17905d974aad65d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Souradeep-Chowdhury/Add-driver-support-for-Data-Capture-and-Compare-Engine-DCC-for-SM8150/20210217-145428
git checkout 98f7664a4e41764c0d2111d6b17905d974aad65d
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/soc/qcom/dcc.c: In function '_dcc_ll_cfg_default':
>> drivers/soc/qcom/dcc.c:360:4: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
360 | if (ret)
| ^~
drivers/soc/qcom/dcc.c:362:5: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
362 | cfg->sram_offset += 4;
| ^~~
At top level:
drivers/soc/qcom/dcc.c:787:12: warning: 'dcc_add_write' defined but not used [-Wunused-function]
787 | static int dcc_add_write(struct dcc_drvdata *drvdata, unsigned int addr,
| ^~~~~~~~~~~~~
drivers/soc/qcom/dcc.c:750:12: warning: 'dcc_rd_mod_wr_add' defined but not used [-Wunused-function]
750 | static int dcc_rd_mod_wr_add(struct dcc_drvdata *drvdata, unsigned int mask,
| ^~~~~~~~~~~~~~~~~
drivers/soc/qcom/dcc.c:733:12: warning: 'dcc_add_loop' defined but not used [-Wunused-function]
733 | static int dcc_add_loop(struct dcc_drvdata *drvdata, unsigned long loop_cnt)
| ^~~~~~~~~~~~
drivers/soc/qcom/dcc.c:631:12: warning: 'dcc_config_add' defined but not used [-Wunused-function]
631 | static int dcc_config_add(struct dcc_drvdata *drvdata, unsigned int addr,
| ^~~~~~~~~~~~~~
drivers/soc/qcom/dcc.c:574:12: warning: 'dcc_enable' defined but not used [-Wunused-function]
574 | static int dcc_enable(struct dcc_drvdata *drvdata)
| ^~~~~~~~~~
vim +/if +360 drivers/soc/qcom/dcc.c
338
339 static int _dcc_ll_cfg_default(struct dcc_drvdata *drvdata,
340 struct dcc_config_entry *entry, struct dcc_cfg_attr *cfg, u32 *pos, u32 *total_len)
341 {
342 int ret = 0;
343 u32 off;
344
345 cfg->addr = (entry->base >> 4) & BM(0, 27);
346
347 if (entry->apb_bus)
348 cfg->addr |= DCC_ADDR_DESCRIPTOR | DCC_READ_IND | DCC_APB_IND;
349 else
350 cfg->addr |= DCC_ADDR_DESCRIPTOR | DCC_READ_IND | DCC_AHB_IND;
351
352 off = entry->offset/4;
353
354 *total_len += entry->len * 4;
355
356 if (!cfg->prev_addr || cfg->prev_addr != cfg->addr || cfg->prev_off > off) {
357 /* Check if we need to write prev link entry */
358 if (cfg->link) {
359 ret = dcc_sram_writel(drvdata, cfg->link, cfg->sram_offset);
> 360 if (ret)
361 return ret;
362 cfg->sram_offset += 4;
363 }
364 dev_dbg(drvdata->dev, "DCC: sram address 0x%x\n", cfg->sram_offset);
365
366 /* Write address */
367 ret = dcc_sram_writel(drvdata, cfg->addr, cfg->sram_offset);
368
369 if (ret)
370 return ret;
371
372 cfg->sram_offset += 4;
373
374 /* Reset link and prev_off */
375 cfg->link = 0;
376 cfg->prev_off = 0;
377 }
378
379 if ((off - cfg->prev_off) > 0xFF || entry->len > MAX_DCC_LEN) {
380 dev_err(drvdata->dev, "DCC: Programming error Base: 0x%x, offset 0x%x\n",
381 entry->base, entry->offset);
382 ret = -EINVAL;
383 return ret;
384 }
385
386 if (cfg->link) {
387 /*
388 * link already has one offset-length so new
389 * offset-length needs to be placed at
390 * bits [29:15]
391 */
392 *pos = 15;
393
394 /* Clear bits [31:16] */
395 cfg->link &= BM(0, 14);
396 } else {
397 /*
398 * link is empty, so new offset-length needs
399 * to be placed at bits [15:0]
400 */
401 *pos = 0;
402 cfg->link = 1 << 15;
403 }
404
405 /* write new offset-length pair to correct position */
406 cfg->link |= (((off-cfg->prev_off) & BM(0, 7)) | ((entry->len << 8) & BM(8, 14))) << *pos;
407
408 cfg->link |= DCC_LINK_DESCRIPTOR;
409
410 if (*pos) {
411 ret = dcc_sram_writel(drvdata, cfg->link, cfg->sram_offset);
412 if (ret)
413 return ret;
414 cfg->sram_offset += 4;
415 cfg->link = 0;
416 }
417
418 cfg->prev_off = off + entry->len - 1;
419 cfg->prev_addr = cfg->addr;
420 return ret;
421 }
422
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 67240 bytes --]
[-- Attachment #3: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V0 1/6] dt-bindings: Added the yaml bindings for DCC
[not found] ` <5da43657817066e0ffe1e24cfb17104138515452.1613541226.git.schowdhu@codeaurora.org>
@ 2021-02-17 11:02 ` Vinod Koul
2021-02-17 20:24 ` Rob Herring
1 sibling, 0 replies; 5+ messages in thread
From: Vinod Koul @ 2021-02-17 11:02 UTC (permalink / raw)
To: Souradeep Chowdhury
Cc: devicetree, Sai Prakash Ranjan, Rajendra Nayak, linux-arm-msm,
linux-kernel, Rob Herring, Bjorn Andersson, Andy Gross,
Sibi Sankar, linux-arm-kernel
On 17-02-21, 12:18, Souradeep Chowdhury wrote:
> Documentation for Data Capture and Compare(DCC) device tree bindings
> in yaml format.
>
> Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
> ---
> .../devicetree/bindings/arm/msm/qcom,dcc.yaml | 49 ++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,dcc.yaml
>
> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,dcc.yaml b/Documentation/devicetree/bindings/arm/msm/qcom,dcc.yaml
> new file mode 100644
> index 0000000..8f09578
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/msm/qcom,dcc.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/msm/qcom,dcc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Data Capture and Compare
> +
> +maintainers:
> + - Souradeep Chowdhury <schowdhu@codeaurora.org>
> +
> +description: |
> + DCC (Data Capture and Compare) is a DMA engine which is used to save
> + configuration data or system memory contents during catastrophic failure
> + or SW trigger.DCC is used to capture and store data for debugging purpose
space after .
> +
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - qcom,sm8150-dcc
> + - const: qcom,dcc
> +
> + reg:
> + items:
> + - description: DCC base register region
> + - description: DCC RAM base register region
> +
> + reg-names:
> + items:
> + - const: dcc-base
> + - const: dcc-ram-base
drop dcc from names
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + dcc@010a2000{
> + compatible = "qcom,sm8150-dcc";
should this not be:
compatible = "qcom,sm8150-dcc", "qcom,dcc";
> + reg = <0 0x010a2000 0 0x1000>,
> + <0 0x010ae000 0 0x2000>;
> + reg-names = "dcc-base", "dcc-ram-base";
> + };
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
--
~Vinod
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V0 2/6] arm64: dts: qcom: sm8150: Add Data Capture and Compare(DCC) support node
[not found] ` <893022aecd4ba354adb57bd463206dd93fc19886.1613541226.git.schowdhu@codeaurora.org>
@ 2021-02-17 11:03 ` Vinod Koul
0 siblings, 0 replies; 5+ messages in thread
From: Vinod Koul @ 2021-02-17 11:03 UTC (permalink / raw)
To: Souradeep Chowdhury
Cc: devicetree, Sai Prakash Ranjan, Rajendra Nayak, linux-arm-msm,
linux-kernel, Rob Herring, Bjorn Andersson, Andy Gross,
Sibi Sankar, linux-arm-kernel
On 17-02-21, 12:18, Souradeep Chowdhury wrote:
> Add the DCC(Data Capture and Compare) device tree node entry along with
> the addresses for register regions.
This should be last patch..
>
> Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
> ---
> arch/arm64/boot/dts/qcom/sm8150.dtsi | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> index e5bb17b..3198bd3 100644
> --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> @@ -654,6 +654,13 @@
> interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
> };
>
> + dcc@010a2000{
no leading zero here and space before {
> + compatible = "qcom,sm8150-dcc", "qcom,dcc";
> + reg = <0x0 0x010a2000 0x0 0x1000>,
> + <0x0 0x010ad000 0x0 0x3000>;
pls align this to preceding line
> + reg-names = "dcc-base", "dcc-ram-base";
> + };
> +
> ufs_mem_hc: ufshc@1d84000 {
> compatible = "qcom,sm8150-ufshc", "qcom,ufshc",
> "jedec,ufs-2.0";
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
--
~Vinod
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V0 1/6] dt-bindings: Added the yaml bindings for DCC
[not found] ` <5da43657817066e0ffe1e24cfb17104138515452.1613541226.git.schowdhu@codeaurora.org>
2021-02-17 11:02 ` [PATCH V0 1/6] dt-bindings: Added the yaml bindings for DCC Vinod Koul
@ 2021-02-17 20:24 ` Rob Herring
1 sibling, 0 replies; 5+ messages in thread
From: Rob Herring @ 2021-02-17 20:24 UTC (permalink / raw)
To: Souradeep Chowdhury
Cc: devicetree, Sai Prakash Ranjan, Rajendra Nayak, linux-kernel,
Andy Gross, Bjorn Andersson, Rob Herring, Sibi Sankar,
linux-arm-msm, linux-arm-kernel
On Wed, 17 Feb 2021 12:18:22 +0530, Souradeep Chowdhury wrote:
> Documentation for Data Capture and Compare(DCC) device tree bindings
> in yaml format.
>
> Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
> ---
> .../devicetree/bindings/arm/msm/qcom,dcc.yaml | 49 ++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,dcc.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
./Documentation/devicetree/bindings/arm/msm/qcom,dcc.yaml:21:5: [warning] wrong indentation: expected 6 but found 4 (indentation)
./Documentation/devicetree/bindings/arm/msm/qcom,dcc.yaml:22:7: [warning] wrong indentation: expected 8 but found 6 (indentation)
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/arm/msm/qcom,dcc.example.dts:19.21-24.11: Warning (unit_address_format): /example-0/dcc@010a2000: unit name should not have leading 0s
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/msm/qcom,dcc.example.dt.yaml: example-0: dcc@010a2000:reg:0: [0, 17440768, 0, 4096] is too long
From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/msm/qcom,dcc.example.dt.yaml: example-0: dcc@010a2000:reg:1: [0, 17489920, 0, 8192] is too long
From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/msm/qcom,dcc.example.dt.yaml: dcc@010a2000: compatible: ['qcom,sm8150-dcc'] is too short
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/msm/qcom,dcc.yaml
See https://patchwork.ozlabs.org/patch/1441182
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V0 3/6] soc: qcom: dcc:Add driver support for Data Capture and Compare unit(DCC)
[not found] ` <f182b10f318db7cb09216c0176a5b26656d9ef49.1613541226.git.schowdhu@codeaurora.org>
2021-02-17 8:08 ` [PATCH V0 3/6] soc: qcom: dcc:Add driver support for Data Capture and Compare unit(DCC) kernel test robot
@ 2021-02-18 6:59 ` Vinod Koul
1 sibling, 0 replies; 5+ messages in thread
From: Vinod Koul @ 2021-02-18 6:59 UTC (permalink / raw)
To: Souradeep Chowdhury
Cc: devicetree, Sai Prakash Ranjan, Rajendra Nayak, linux-arm-msm,
linux-kernel, Rob Herring, Bjorn Andersson, Andy Gross,
Sibi Sankar, linux-arm-kernel
On 17-02-21, 12:18, Souradeep Chowdhury wrote:
> The DCC is a DMA Engine designed to capture and store data
> during system crash or software triggers.The DCC operates
^^^
Space after . (quite a few here, pls fix them)
> based on link list entries which provides it with data and
> addresses and the function it needs to perform.These functions
> are read,write and loop.Added the basic driver in this patch
> which contains a probe method which instantiates all the link
> list data specific to a SoC.Methods have also been added to
> handle all the functionalities specific to a linked list.Each
> DCC has it's own SRAM which needs to be instantiated at probe
> time as well.
So help me understand, in case of system crash how will this be used..?
>
> Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
> ---
> drivers/soc/qcom/Kconfig | 8 +
> drivers/soc/qcom/Makefile | 1 +
> drivers/soc/qcom/dcc.c | 1055 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1064 insertions(+)
> create mode 100644 drivers/soc/qcom/dcc.c
>
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index 79b568f..8819e0b 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -69,6 +69,14 @@ config QCOM_LLCC
> SDM845. This provides interfaces to clients that use the LLCC.
> Say yes here to enable LLCC slice driver.
>
> +config QCOM_DCC
> + tristate "Qualcomm Technologies, Inc. Data Capture and Compare engine driver"
> + depends on ARCH_QCOM || COMPILE_TEST
> + help
> + This option enables driver for Data Capture and Compare engine. DCC
> + driver provides interface to configure DCC block and read back
> + captured data from DCC's internal SRAM.
> +
> config QCOM_KRYO_L2_ACCESSORS
> bool
> depends on ARCH_QCOM && ARM64 || COMPILE_TEST
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index ad675a6..1b00870 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -26,3 +26,4 @@ obj-$(CONFIG_QCOM_LLCC) += llcc-qcom.o
> obj-$(CONFIG_QCOM_RPMHPD) += rpmhpd.o
> obj-$(CONFIG_QCOM_RPMPD) += rpmpd.o
> obj-$(CONFIG_QCOM_KRYO_L2_ACCESSORS) += kryo-l2-accessors.o
> +obj-$(CONFIG_QCOM_DCC) += dcc.o
> diff --git a/drivers/soc/qcom/dcc.c b/drivers/soc/qcom/dcc.c
> new file mode 100644
> index 0000000..d67452b
> --- /dev/null
> +++ b/drivers/soc/qcom/dcc.c
> @@ -0,0 +1,1055 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/cdev.h>
> +#include <linux/delay.h>
> +#include <linux/fs.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
> +
> +#define TIMEOUT_US 100
> +
> +#define BM(lsb, msb) ((BIT(msb) - BIT(lsb)) + BIT(msb))
> +#define BMVAL(val, lsb, msb) ((val & BM(lsb, msb)) >> lsb)
> +#define BVAL(val, n) ((val & BIT(n)) >> n)
Pls use macros available in bitfield.h rather than inventing your own..
> +
> +#define dcc_writel(drvdata, val, off) \
> + writel((val), drvdata->base + dcc_offset_conv(drvdata, off))
> +#define dcc_readl(drvdata, off) \
> + readl(drvdata->base + dcc_offset_conv(drvdata, off))
> +
> +#define dcc_sram_readl(drvdata, off) \
> + readl(drvdata->ram_base + off)
> +
> +/* DCC registers */
> +#define DCC_HW_INFO 0x04
> +#define DCC_LL_NUM_INFO 0x10
> +#define DCC_STATUS 0x1C
> +#define DCC_LL_LOCK(m) (0x34 + 0x80 * m)
> +#define DCC_LL_CFG(m) (0x38 + 0x80 * m)
> +#define DCC_LL_BASE(m) (0x3c + 0x80 * m)
> +#define DCC_FD_BASE(m) (0x40 + 0x80 * m)
> +#define DCC_LL_TIMEOUT(m) (0x44 + 0x80 * m)
> +#define DCC_LL_INT_ENABLE(m) (0x4C + 0x80 * m)
> +#define DCC_LL_INT_STATUS(m) (0x50 + 0x80 * m)
> +#define DCC_LL_SW_TRIGGER(m) (0x60 + 0x80 * m)
> +#define DCC_LL_BUS_ACCESS_STATUS(m) (0x64 + 0x80 * m)
> +
> +#define DCC_MAP_LEVEL1 0x18
> +#define DCC_MAP_LEVEL2 0x34
> +#define DCC_MAP_LEVEL3 0x4C
> +
> +#define DCC_MAP_OFFSET1 0x10
> +#define DCC_MAP_OFFSET2 0x18
> +#define DCC_MAP_OFFSET3 0x1C
> +#define DCC_MAP_OFFSET4 0x8
> +
> +#define DCC_FIX_LOOP_OFFSET 16
> +#define DCC_VER_INFO_BIT 9
> +
> +#define DCC_READ 0
> +#define DCC_WRITE 1
> +#define DCC_LOOP 2
> +#define DCC_READ_WRITE 3
> +
> +#define MAX_DCC_OFFSET (0xFF * 4)
> +#define MAX_DCC_LEN 0x7F
> +#define MAX_LOOP_CNT 0xFF
> +
> +#define DCC_ADDR_DESCRIPTOR 0x00
> +#define DCC_LOOP_DESCRIPTOR (BIT(30))
> +#define DCC_RD_MOD_WR_DESCRIPTOR (BIT(31))
> +#define DCC_LINK_DESCRIPTOR (BIT(31) | BIT(30))
we have GENMASK() for this
> +
> +#define DCC_READ_IND 0x00
> +#define DCC_WRITE_IND (BIT(28))
> +
> +#define DCC_AHB_IND 0x00
> +#define DCC_APB_IND BIT(29)
> +
> +#define DCC_MAX_LINK_LIST 8
> +#define DCC_INVALID_LINK_LIST 0xFF
> +
> +#define DCC_VER_MASK1 0x7F
> +#define DCC_VER_MASK2 0x3F
Genmask for these too...
> +
> +#define DCC_RD_MOD_WR_ADDR 0xC105E
> +
> +struct qcom_dcc_config {
> + const int dcc_ram_offset;
> +};
> +
> +static const struct qcom_dcc_config sm8150_cfg = {
> + .dcc_ram_offset = 0x5000,
> +};
maybe move it down near compatible table?
> +
> +enum dcc_descriptor_type {
> + DCC_ADDR_TYPE,
> + DCC_LOOP_TYPE,
> + DCC_READ_WRITE_TYPE,
> + DCC_WRITE_TYPE
> +};
> +
> +enum dcc_mem_map_ver {
> + DCC_MEM_MAP_VER1,
> + DCC_MEM_MAP_VER2,
> + DCC_MEM_MAP_VER3
> +};
> +
> +struct dcc_config_entry {
> + u32 base;
> + u32 offset;
> + u32 len;
> + u32 index;
> + u32 loop_cnt;
> + u32 write_val;
> + u32 mask;
> + bool apb_bus;
> + enum dcc_descriptor_type desc_type;
> + struct list_head list;
> +};
> +
> +struct dcc_drvdata {
> + void __iomem *base;
> + u32 reg_size;
> + struct device *dev;
> + struct mutex mutex;
> + void __iomem *ram_base;
> + u32 ram_size;
> + u32 ram_offset;
> + enum dcc_mem_map_ver mem_map_ver;
> + u32 ram_cfg;
> + u32 ram_start;
> + bool *enable;
> + bool *configured;
> + bool interrupt_disable;
> + char *sram_node;
> + struct cdev sram_dev;
> + struct class *sram_class;
> + struct list_head *cfg_head;
> + u32 *nr_config;
> + u32 nr_link_list;
> + u8 curr_list;
> + u8 loopoff;
> +};
> +
> +struct dcc_cfg_attr {
> + u32 addr;
> + u32 prev_addr;
> + u32 prev_off;
> + u32 link;
> + u32 sram_offset;
> +};
> +
> +struct dcc_cfg_loop_attr {
> + u32 loop;
> + bool loop_start;
> + u32 loop_cnt;
> + u32 loop_len;
> + u32 loop_off;
> +};
> +
> +static u32 dcc_offset_conv(struct dcc_drvdata *drvdata, u32 off)
> +{
> + if (drvdata->mem_map_ver == DCC_MEM_MAP_VER1) {
> + if ((off & DCC_VER_MASK1) >= DCC_MAP_LEVEL3)
> + return (off - DCC_MAP_OFFSET3);
> + if ((off & DCC_VER_MASK1) >= DCC_MAP_LEVEL2)
> + return (off - DCC_MAP_OFFSET2);
> + else if ((off & DCC_VER_MASK1) >= DCC_MAP_LEVEL1)
> + return (off - DCC_MAP_OFFSET1);
> + } else if (drvdata->mem_map_ver == DCC_MEM_MAP_VER2) {
> + if ((off & DCC_VER_MASK1) >= DCC_MAP_LEVEL2)
> + return (off - DCC_MAP_OFFSET4);
> + }
> + return off;
> +}
> +
> +static int dcc_sram_writel(struct dcc_drvdata *drvdata,
> + u32 val, u32 off)
> +{
> + if (unlikely(off > (drvdata->ram_size - 4)))
> + return -EINVAL;
> +
> + writel((val), drvdata->ram_base + off);
> +
> + return 0;
> +}
> +
> +static int _dcc_ll_cfg_read_write(struct dcc_drvdata *drvdata,
> +struct dcc_config_entry *entry, struct dcc_cfg_attr *cfg)
this looks a bit hard to read, can you make it better (also you can go
upto 100 chars now), do you checkpatch with --strict option to get
better alignment of code
> +{
> + int ret = 0;
Superfluous init?
> +
> + if (cfg->link) {
> + /* write new offset = 1 to continue
> + * processing the list
kernel uses:
/*
* this is a
* multi line comment style
*/
> + */
> +
> + ret = dcc_sram_writel(drvdata, cfg->link, cfg->sram_offset);
> + if (ret)
> + return ret;
> + cfg->sram_offset += 4;
> + /* Reset link and prev_off */
> + cfg->addr = 0x00;
> + cfg->link = 0;
> + cfg->prev_off = 0;
memset cfg first?
> + cfg->prev_addr = cfg->addr;
> + }
> +
> + cfg->addr = DCC_RD_MOD_WR_DESCRIPTOR;
> +
> + ret = dcc_sram_writel(drvdata, cfg->addr, cfg->sram_offset);
> +
drop this empty line
> + if (ret)
> + return ret;
> +
> + cfg->sram_offset += 4;
> +
> + ret = dcc_sram_writel(drvdata, entry->mask, cfg->sram_offset);
> +
> + if (ret)
> + return ret;
> +
> + cfg->sram_offset += 4;
> +
> + ret = dcc_sram_writel(drvdata, entry->write_val, cfg->sram_offset);
> +
> + if (ret)
> + return ret;
> +
> + cfg->sram_offset += 4;
> +
> + cfg->addr = 0;
> +
> + return ret;
> +}
> +
> +static int _dcc_ll_cfg_loop(struct dcc_drvdata *drvdata, struct dcc_config_entry *entry,
> +struct dcc_cfg_attr *cfg, struct dcc_cfg_loop_attr *cfg_loop, u32 *total_len)
here as well
> +{
> +
> + int ret = 0;
> +
> + /* Check if we need to write link of prev entry */
> + if (cfg->link) {
> + ret = dcc_sram_writel(drvdata, cfg->link, cfg->sram_offset);
> + if (ret)
> + return ret;
> + cfg->sram_offset += 4;
> + }
> +
> + if (cfg_loop->loop_start) {
> + cfg_loop->loop = (cfg->sram_offset - cfg_loop->loop_off) / 4;
> + cfg_loop->loop |= (cfg_loop->loop_cnt << drvdata->loopoff) &
> + BM(drvdata->loopoff, 27);
> + cfg_loop->loop |= DCC_LOOP_DESCRIPTOR;
> + *total_len += (*total_len - cfg_loop->loop_len) * cfg_loop->loop_cnt;
> +
> + ret = dcc_sram_writel(drvdata, cfg_loop->loop, cfg->sram_offset);
> +
> + if (ret)
> + return ret;
> + cfg->sram_offset += 4;
> +
> + cfg_loop->loop_start = false;
> + cfg_loop->loop_len = 0;
> + cfg_loop->loop_off = 0;
seems quite similar to last one..? Maybe a helper for common code
> + } else {
> + cfg_loop->loop_start = true;
> + cfg_loop->loop_cnt = entry->loop_cnt - 1;
> + cfg_loop->loop_len = *total_len;
> + cfg_loop->loop_off = cfg->sram_offset;
> + }
> +
> + /* Reset link and prev_off */
> +
> + cfg->addr = 0x00;
> + cfg->link = 0;
> + cfg->prev_off = 0;
> + cfg->prev_addr = cfg->addr;
> +
> + return ret;
> +}
> +
> +static int _dcc_ll_cfg_write(struct dcc_drvdata *drvdata,
> +struct dcc_config_entry *entry, struct dcc_cfg_attr *cfg, u32 *total_len)
> +{
> + u32 off;
> + int ret = 0;
> +
> + if (cfg->link) {
> + /* write new offset = 1 to continue
> + * processing the list
> + */
> + ret = dcc_sram_writel(drvdata, cfg->link, cfg->sram_offset);
> +
> + if (ret)
> + return ret;
> +
> + cfg->sram_offset += 4;
> + /* Reset link and prev_off */
> + cfg->addr = 0x00;
> + cfg->prev_off = 0;
> + cfg->prev_addr = cfg->addr;
> + }
> +
> + off = entry->offset/4;
> + /* write new offset-length pair to correct position */
> + cfg->link |= ((off & BM(0, 7)) | BIT(15) | ((entry->len << 8) & BM(8, 14)));
> + cfg->link |= DCC_LINK_DESCRIPTOR;
> +
> + /* Address type */
> + cfg->addr = (entry->base >> 4) & BM(0, 27);
> + if (entry->apb_bus)
> + cfg->addr |= DCC_ADDR_DESCRIPTOR | DCC_WRITE_IND | DCC_APB_IND;
> + else
> + cfg->addr |= DCC_ADDR_DESCRIPTOR | DCC_WRITE_IND | DCC_AHB_IND;
> + ret = dcc_sram_writel(drvdata, cfg->addr, cfg->sram_offset);
> +
> + if (ret)
> + return ret;
> + cfg->sram_offset += 4;
> +
> + ret = dcc_sram_writel(drvdata, cfg->link, cfg->sram_offset);
> + if (ret)
> + return ret;
> + cfg->sram_offset += 4;
> +
> + ret = dcc_sram_writel(drvdata, entry->write_val, cfg->sram_offset);
> +
> + if (ret)
> + return ret;
> +
> + cfg->sram_offset += 4;
> + cfg->addr = 0x00;
> + cfg->link = 0;
> + return ret;
> +}
> +
> +static int _dcc_ll_cfg_default(struct dcc_drvdata *drvdata,
> +struct dcc_config_entry *entry, struct dcc_cfg_attr *cfg, u32 *pos, u32 *total_len)
> +{
> + int ret = 0;
> + u32 off;
> +
> + cfg->addr = (entry->base >> 4) & BM(0, 27);
> +
> + if (entry->apb_bus)
> + cfg->addr |= DCC_ADDR_DESCRIPTOR | DCC_READ_IND | DCC_APB_IND;
> + else
> + cfg->addr |= DCC_ADDR_DESCRIPTOR | DCC_READ_IND | DCC_AHB_IND;
> +
> + off = entry->offset/4;
> +
> + *total_len += entry->len * 4;
> +
> + if (!cfg->prev_addr || cfg->prev_addr != cfg->addr || cfg->prev_off > off) {
> + /* Check if we need to write prev link entry */
> + if (cfg->link) {
> + ret = dcc_sram_writel(drvdata, cfg->link, cfg->sram_offset);
> + if (ret)
> + return ret;
> + cfg->sram_offset += 4;
> + }
> + dev_dbg(drvdata->dev, "DCC: sram address 0x%x\n", cfg->sram_offset);
> +
> + /* Write address */
> + ret = dcc_sram_writel(drvdata, cfg->addr, cfg->sram_offset);
> +
> + if (ret)
> + return ret;
> +
> + cfg->sram_offset += 4;
> +
> + /* Reset link and prev_off */
> + cfg->link = 0;
> + cfg->prev_off = 0;
> + }
> +
> + if ((off - cfg->prev_off) > 0xFF || entry->len > MAX_DCC_LEN) {
> + dev_err(drvdata->dev, "DCC: Programming error Base: 0x%x, offset 0x%x\n",
> + entry->base, entry->offset);
> + ret = -EINVAL;
> + return ret;
> + }
> +
> + if (cfg->link) {
> + /*
> + * link already has one offset-length so new
> + * offset-length needs to be placed at
> + * bits [29:15]
> + */
> + *pos = 15;
> +
> + /* Clear bits [31:16] */
> + cfg->link &= BM(0, 14);
> + } else {
> + /*
> + * link is empty, so new offset-length needs
> + * to be placed at bits [15:0]
> + */
> + *pos = 0;
> + cfg->link = 1 << 15;
> + }
> +
> + /* write new offset-length pair to correct position */
> + cfg->link |= (((off-cfg->prev_off) & BM(0, 7)) | ((entry->len << 8) & BM(8, 14))) << *pos;
> +
> + cfg->link |= DCC_LINK_DESCRIPTOR;
> +
> + if (*pos) {
> + ret = dcc_sram_writel(drvdata, cfg->link, cfg->sram_offset);
> + if (ret)
> + return ret;
> + cfg->sram_offset += 4;
> + cfg->link = 0;
> + }
> +
> + cfg->prev_off = off + entry->len - 1;
> + cfg->prev_addr = cfg->addr;
> + return ret;
> +}
> +
> +static int __dcc_ll_cfg(struct dcc_drvdata *drvdata, int curr_list)
> +{
> + int ret = 0;
> + u32 total_len, pos;
> + struct dcc_config_entry *entry;
> + struct dcc_cfg_attr cfg;
> + struct dcc_cfg_loop_attr cfg_loop;
> +
> + cfg.sram_offset = drvdata->ram_cfg * 4;
> + cfg.prev_off = 0;
> + cfg_loop.loop_off = 0;
> + total_len = 0;
> + cfg_loop.loop_len = 0;
> + cfg_loop.loop_cnt = 0;
> + cfg_loop.loop_start = false;
> + cfg.prev_addr = 0;
> + cfg.addr = 0;
> + cfg.link = 0;
again use memset for these
> +
> + list_for_each_entry(entry, &drvdata->cfg_head[curr_list], list) {
> + switch (entry->desc_type) {
> + case DCC_READ_WRITE_TYPE:
> + {
checkpatch should have told you this is not typical kernel style, pls
fix this and many other things before we process further
--
~Vinod
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-02-18 7:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <cover.1613541226.git.schowdhu@codeaurora.org>
[not found] ` <5da43657817066e0ffe1e24cfb17104138515452.1613541226.git.schowdhu@codeaurora.org>
2021-02-17 11:02 ` [PATCH V0 1/6] dt-bindings: Added the yaml bindings for DCC Vinod Koul
2021-02-17 20:24 ` Rob Herring
[not found] ` <893022aecd4ba354adb57bd463206dd93fc19886.1613541226.git.schowdhu@codeaurora.org>
2021-02-17 11:03 ` [PATCH V0 2/6] arm64: dts: qcom: sm8150: Add Data Capture and Compare(DCC) support node Vinod Koul
[not found] ` <f182b10f318db7cb09216c0176a5b26656d9ef49.1613541226.git.schowdhu@codeaurora.org>
2021-02-17 8:08 ` [PATCH V0 3/6] soc: qcom: dcc:Add driver support for Data Capture and Compare unit(DCC) kernel test robot
2021-02-18 6:59 ` Vinod Koul
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).