* Re: [RESEND v5 09/21] mtd: rawnand: Create a new enumeration to describe properly ECC types
From: Boris Brezillon @ 2020-05-26 22:59 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree
In-Reply-To: <20200526195633.11543-10-miquel.raynal@bootlin.com>
On Tue, 26 May 2020 21:56:21 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Now that the misleading mix between ECC engine type and OOB placement
> has been addressed, add a new enumeration to properly define ECC types
> (also called provider or mode).
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/nand_base.c | 7 +++++++
> include/linux/mtd/rawnand.h | 16 ++++++++++++++++
> 2 files changed, 23 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 515cd4681660..5c6ab5b93270 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -5018,6 +5018,13 @@ static const char * const nand_ecc_modes[] = {
> [NAND_ECC_ON_DIE] = "on-die",
> };
>
> +static const char * const nand_ecc_engine_providers[] = {
This table is not used here, are you sure it should be introduced now?
> + [NAND_ECC_ENGINE_NONE] = "none",
> + [NAND_ECC_ENGINE_SOFT] = "soft",
> + [NAND_ECC_ENGINE_CONTROLLER] = "hw",
^ "on-controller" ?
> + [NAND_ECC_ENGINE_ON_DIE] = "on-die",
> +};
> +
> static const char * const nand_ecc_placement[] = {
> [NAND_ECC_PLACEMENT_INTERLEAVED] = "interleaved",
> };
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index dc909fb977c7..a2078c5f3d21 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -92,6 +92,22 @@ enum nand_ecc_mode {
> NAND_ECC_ON_DIE,
> };
>
> +/**
> + * enum nand_ecc_engine_type - NAND ECC engine type/provider
> + * @NAND_ECC_ENGINE_INVALID: Invalid value
> + * @NAND_ECC_ENGINE_NONE: No ECC correction
> + * @NAND_ECC_ENGINE_SOFT: Software ECC correction
> + * @NAND_ECC_ENGINE_CONTROLLER: Hardware controller ECC correction
> + * @NAND_ECC_ENGINE_ON_DIE: On chip hardware ECC correction
> + */
> +enum nand_ecc_engine_type {
> + NAND_ECC_ENGINE_INVALID,
> + NAND_ECC_ENGINE_NONE,
> + NAND_ECC_ENGINE_SOFT,
> + NAND_ECC_ENGINE_CONTROLLER,
> + NAND_ECC_ENGINE_ON_DIE,
> +};
> +
> /**
> * enum nand_ecc_placement - NAND ECC placement
> * @NAND_ECC_PLACEMENT_FREE: The driver can decide where to put ECC bytes.
^ permalink raw reply
* Re: [RESEND v5 09/21] mtd: rawnand: Create a new enumeration to describe properly ECC types
From: Boris Brezillon @ 2020-05-26 22:55 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree
In-Reply-To: <20200526195633.11543-10-miquel.raynal@bootlin.com>
On Tue, 26 May 2020 21:56:21 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Now that the misleading mix between ECC engine type and OOB placement
> has been addressed, add a new enumeration to properly define ECC types
> (also called provider or mode).
Let's pick a name and stick to it. I think "ECC provider type" or
"ECC engine type" are good names.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/nand_base.c | 7 +++++++
> include/linux/mtd/rawnand.h | 16 ++++++++++++++++
> 2 files changed, 23 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 515cd4681660..5c6ab5b93270 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -5018,6 +5018,13 @@ static const char * const nand_ecc_modes[] = {
> [NAND_ECC_ON_DIE] = "on-die",
> };
>
> +static const char * const nand_ecc_engine_providers[] = {
I'd rename that one nand_ecc_engine_types or nand_ecc_provider_types.
> + [NAND_ECC_ENGINE_NONE] = "none",
> + [NAND_ECC_ENGINE_SOFT] = "soft",
> + [NAND_ECC_ENGINE_CONTROLLER] = "hw",
> + [NAND_ECC_ENGINE_ON_DIE] = "on-die",
> +};
> +
> static const char * const nand_ecc_placement[] = {
> [NAND_ECC_PLACEMENT_INTERLEAVED] = "interleaved",
> };
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index dc909fb977c7..a2078c5f3d21 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -92,6 +92,22 @@ enum nand_ecc_mode {
> NAND_ECC_ON_DIE,
> };
>
> +/**
> + * enum nand_ecc_engine_type - NAND ECC engine type/provider
> + * @NAND_ECC_ENGINE_INVALID: Invalid value
> + * @NAND_ECC_ENGINE_NONE: No ECC correction
> + * @NAND_ECC_ENGINE_SOFT: Software ECC correction
> + * @NAND_ECC_ENGINE_CONTROLLER: Hardware controller ECC correction
> + * @NAND_ECC_ENGINE_ON_DIE: On chip hardware ECC correction
> + */
> +enum nand_ecc_engine_type {
Looks like you went for ecc_engine_type here, so let's stick to that.
> + NAND_ECC_ENGINE_INVALID,
NAND_ECC_ENGINE_TYPE_xxx
> + NAND_ECC_ENGINE_NONE,
Do we really need a value for NONE? I'd expect the engine type to be
applicable to NAND that have some sort of ECC engine connected to them.
> + NAND_ECC_ENGINE_SOFT,
> + NAND_ECC_ENGINE_CONTROLLER,
> + NAND_ECC_ENGINE_ON_DIE,
> +};
> +
> /**
> * enum nand_ecc_placement - NAND ECC placement
> * @NAND_ECC_PLACEMENT_FREE: The driver can decide where to put ECC bytes.
^ permalink raw reply
* Re: [PATCH v5 06/14] dt-bindings: PCI: cadence: Remove "mem" from reg binding
From: Rob Herring @ 2020-05-26 22:53 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Tom Joseph, linux-pci, linux-arm-kernel, Lorenzo Pieralisi,
Bjorn Helgaas, linux-kernel, Greg Kroah-Hartman, Arnd Bergmann,
devicetree, linux-omap
In-Reply-To: <20200522033631.32574-7-kishon@ti.com>
On Fri, 22 May 2020 09:06:23 +0530, Kishon Vijay Abraham I wrote:
> "mem" is not a memory resource and it overlaps with PCIe config space
> and memory region. Removve "mem" from reg binding.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> .../devicetree/bindings/pci/cdns,cdns-pcie-host.yaml | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH v3 04/10] dmaengine: Introduce max SG list entries capability
From: Serge Semin @ 2020-05-26 22:50 UTC (permalink / raw)
To: Vinod Koul, Viresh Kumar, Dan Williams
Cc: Serge Semin, Serge Semin, Andy Shevchenko, Alexey Malahov,
Thomas Bogendoerfer, Arnd Bergmann, Rob Herring, linux-mips,
devicetree, dmaengine, linux-kernel
In-Reply-To: <20200526225022.20405-1-Sergey.Semin@baikalelectronics.ru>
Some devices may lack the support of the hardware accelerated SG list
entries automatic walking through and execution. In this case a burden of
the SG list traversal and DMA engine re-initialization lies on the
DMA engine driver (normally implemented by using a DMA transfer completion
IRQ to recharge the DMA device with a next SG list entry). But such
solution may not be suitable for some DMA consumers. In particular SPI
devices need both Tx and Rx DMA channels work synchronously in order
to avoid the Rx FIFO overflow. In case if Rx DMA channel is paused for
some time while the Tx DMA channel works implicitly pulling data into the
Rx FIFO, the later will be eventually overflown, which will cause the data
loss. So if SG list entries aren't automatically fetched by the DMA
engine, but are one-by-one manually selected for execution in the
ISRs/deferred work/etc., such problem will eventually happen due to the
non-deterministic latencies of the service execution.
In order to let the DMA consumer know about the DMA device capabilities
regarding the hardware accelerated SG list traversal we introduce the
max_sg_list capability. It is supposed to be initialized by the DMA engine
driver with 0 if there is no limitation for the number of SG entries
atomically executed and with non-zero value if there is such constraints,
so the upper limit is determined by the number set to the property.
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v3:
- This is a new patch created as a result of the discussion with Vinud and
Andy in the framework of DW DMA burst and LLP capabilities.
---
drivers/dma/dmaengine.c | 1 +
include/linux/dmaengine.h | 8 ++++++++
2 files changed, 9 insertions(+)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index b332ffe52780..ad56ad58932c 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -592,6 +592,7 @@ int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps)
caps->directions = device->directions;
caps->min_burst = device->min_burst;
caps->max_burst = device->max_burst;
+ caps->max_sg_nents = device->max_sg_nents;
caps->residue_granularity = device->residue_granularity;
caps->descriptor_reuse = device->descriptor_reuse;
caps->cmd_pause = !!device->device_pause;
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 0c7403b27133..6801200c76b6 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -467,6 +467,9 @@ enum dma_residue_granularity {
* should be checked by controller as well
* @min_burst: min burst capability per-transfer
* @max_burst: max burst capability per-transfer
+ * @max_sg_nents: max number of SG list entries executed in a single atomic
+ * DMA tansaction with no intermediate IRQ for reinitialization. Zero
+ * value means unlimited number if entries.
* @cmd_pause: true, if pause is supported (i.e. for reading residue or
* for resume later)
* @cmd_resume: true, if resume is supported
@@ -481,6 +484,7 @@ struct dma_slave_caps {
u32 directions;
u32 min_burst;
u32 max_burst;
+ u32 max_sg_nents;
bool cmd_pause;
bool cmd_resume;
bool cmd_terminate;
@@ -773,6 +777,9 @@ struct dma_filter {
* should be checked by controller as well
* @min_burst: min burst capability per-transfer
* @max_burst: max burst capability per-transfer
+ * @max_sg_nents: max number of SG list entries executed in a single atomic
+ * DMA tansaction with no intermediate IRQ for reinitialization. Zero
+ * value means unlimited number if entries.
* @residue_granularity: granularity of the transfer residue reported
* by tx_status
* @device_alloc_chan_resources: allocate resources and return the
@@ -844,6 +851,7 @@ struct dma_device {
u32 directions;
u32 min_burst;
u32 max_burst;
+ u32 max_sg_nents;
bool descriptor_reuse;
enum dma_residue_granularity residue_granularity;
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v5 4/5] dt-bindings: counter: microchip-tcb-capture counter
From: Rob Herring @ 2020-05-26 22:51 UTC (permalink / raw)
To: Kamel Bouhara
Cc: linux-input, devicetree, Rob Herring, linux-arm-kernel,
Thomas Petazzoni, Alexandre Belloni, William Breathitt Gray,
Nicolas Ferre, Mark Rutland, Ludovic Desroches, linux-iio
In-Reply-To: <20200519083716.938384-5-kamel.bouhara@bootlin.com>
On Tue, 19 May 2020 10:37:15 +0200, Kamel Bouhara wrote:
> Describe the devicetree binding for the Microchip TCB module.
> Each counter blocks exposes three independent counters.
>
> However, when configured in quadrature decoder, both channel <0> and <1>
> are required for speed/position and rotation capture (yet only the
> position is captured).
>
> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> ---
> .../soc/microchip/atmel,at91rm9200-tcb.yaml | 32 +++++++++++++++++--
> 1 file changed, 29 insertions(+), 3 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH v3 03/10] dmaengine: Introduce min burst length capability
From: Serge Semin @ 2020-05-26 22:50 UTC (permalink / raw)
To: Vinod Koul, Viresh Kumar, Dan Williams
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Arnd Bergmann, Andy Shevchenko, Rob Herring, linux-mips,
devicetree, dmaengine, linux-kernel
In-Reply-To: <20200526225022.20405-1-Sergey.Semin@baikalelectronics.ru>
Some hardware aside from default 0/1 may have greater minimum burst
transactions length constraints. Here we introduce the DMA device
and slave capability, which if required can be initialized by the DMA
engine driver with the device-specific value.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v3:
- This is a new patch created as a result of the discussion with Vinud and
Andy in the framework of DW DMA burst and LLP capabilities.
---
drivers/dma/dmaengine.c | 1 +
include/linux/dmaengine.h | 4 ++++
2 files changed, 5 insertions(+)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index d31076d9ef25..b332ffe52780 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -590,6 +590,7 @@ int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps)
caps->src_addr_widths = device->src_addr_widths;
caps->dst_addr_widths = device->dst_addr_widths;
caps->directions = device->directions;
+ caps->min_burst = device->min_burst;
caps->max_burst = device->max_burst;
caps->residue_granularity = device->residue_granularity;
caps->descriptor_reuse = device->descriptor_reuse;
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index e1c03339918f..0c7403b27133 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -465,6 +465,7 @@ enum dma_residue_granularity {
* Since the enum dma_transfer_direction is not defined as bit flag for
* each type, the dma controller should set BIT(<TYPE>) and same
* should be checked by controller as well
+ * @min_burst: min burst capability per-transfer
* @max_burst: max burst capability per-transfer
* @cmd_pause: true, if pause is supported (i.e. for reading residue or
* for resume later)
@@ -478,6 +479,7 @@ struct dma_slave_caps {
u32 src_addr_widths;
u32 dst_addr_widths;
u32 directions;
+ u32 min_burst;
u32 max_burst;
bool cmd_pause;
bool cmd_resume;
@@ -769,6 +771,7 @@ struct dma_filter {
* Since the enum dma_transfer_direction is not defined as bit flag for
* each type, the dma controller should set BIT(<TYPE>) and same
* should be checked by controller as well
+ * @min_burst: min burst capability per-transfer
* @max_burst: max burst capability per-transfer
* @residue_granularity: granularity of the transfer residue reported
* by tx_status
@@ -839,6 +842,7 @@ struct dma_device {
u32 src_addr_widths;
u32 dst_addr_widths;
u32 directions;
+ u32 min_burst;
u32 max_burst;
bool descriptor_reuse;
enum dma_residue_granularity residue_granularity;
--
2.26.2
^ permalink raw reply related
* [PATCH v3 01/10] dt-bindings: dma: dw: Convert DW DMAC to DT binding
From: Serge Semin @ 2020-05-26 22:50 UTC (permalink / raw)
To: Vinod Koul, Viresh Kumar, Rob Herring, Andy Shevchenko
Cc: Serge Semin, Serge Semin, Rob Herring, Alexey Malahov,
Thomas Bogendoerfer, Arnd Bergmann, linux-mips, dmaengine,
devicetree, linux-kernel
In-Reply-To: <20200526225022.20405-1-Sergey.Semin@baikalelectronics.ru>
Modern device tree bindings are supposed to be created as YAML-files
in accordance with dt-schema. This commit replaces the Synopsis
Designware DMA controller legacy bare text bindings with YAML file.
The only required prorties are "compatible", "reg", "#dma-cells" and
"interrupts", which will be used by the driver to correctly find the
controller memory region and handle its events. The rest of the properties
are optional, since in case if either "dma-channels" or "dma-masters" isn't
specified, the driver will attempt to auto-detect the IP core
configuration.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: linux-mips@vger.kernel.org
---
Changelog v2:
- Rearrange SoBs.
- Move $ref to the root level of the properties. So do do with the
constraints.
- Discard default settings defined out of the property enum constraint.
- Replace "additionalProperties: false" with "unevaluatedProperties: false"
property.
- Remove a label definition from the binding example.
---
.../bindings/dma/snps,dma-spear1340.yaml | 161 ++++++++++++++++++
.../devicetree/bindings/dma/snps-dma.txt | 69 --------
2 files changed, 161 insertions(+), 69 deletions(-)
create mode 100644 Documentation/devicetree/bindings/dma/snps,dma-spear1340.yaml
delete mode 100644 Documentation/devicetree/bindings/dma/snps-dma.txt
diff --git a/Documentation/devicetree/bindings/dma/snps,dma-spear1340.yaml b/Documentation/devicetree/bindings/dma/snps,dma-spear1340.yaml
new file mode 100644
index 000000000000..e7611840a7cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/snps,dma-spear1340.yaml
@@ -0,0 +1,161 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/snps,dma-spear1340.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys Designware DMA Controller
+
+maintainers:
+ - Viresh Kumar <vireshk@kernel.org>
+ - Andy Shevchenko <andriy.shevchenko@linux.intel.com>
+
+allOf:
+ - $ref: "dma-controller.yaml#"
+
+properties:
+ compatible:
+ const: snps,dma-spear1340
+
+ "#dma-cells":
+ const: 3
+ description: |
+ First cell is a phandle pointing to the DMA controller. Second one is
+ the DMA request line number. Third cell is the memory master identifier
+ for transfers on dynamically allocated channel. Fourth cell is the
+ peripheral master identifier for transfers on an allocated channel.
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ description: AHB interface reference clock.
+ const: hclk
+
+ dma-channels:
+ description: |
+ Number of DMA channels supported by the controller. In case if
+ not specified the driver will try to auto-detect this and
+ the rest of the optional parameters.
+ minimum: 1
+ maximum: 8
+
+ dma-requests:
+ minimum: 1
+ maximum: 16
+
+ dma-masters:
+ $ref: /schemas/types.yaml#definitions/uint32
+ description: |
+ Number of DMA masters supported by the controller. In case if
+ not specified the driver will try to auto-detect this and
+ the rest of the optional parameters.
+ minimum: 1
+ maximum: 4
+
+ chan_allocation_order:
+ $ref: /schemas/types.yaml#definitions/uint32
+ description: |
+ DMA channels allocation order specifier. Zero means ascending order
+ (first free allocated), while one - descending (last free allocated).
+ default: 0
+ enum: [0, 1]
+
+ chan_priority:
+ $ref: /schemas/types.yaml#definitions/uint32
+ description: |
+ DMA channels priority order. Zero means ascending channels priority
+ so the very first channel has the highest priority. While 1 means
+ descending priority (the last channel has the highest priority).
+ default: 0
+ enum: [0, 1]
+
+ block_size:
+ $ref: /schemas/types.yaml#definitions/uint32
+ description: Maximum block size supported by the DMA controller.
+ enum: [3, 7, 15, 31, 63, 127, 255, 511, 1023, 2047, 4095]
+
+ data-width:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: Data bus width per each DMA master in bytes.
+ items:
+ maxItems: 4
+ items:
+ enum: [4, 8, 16, 32]
+
+ data_width:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ deprecated: true
+ description: |
+ Data bus width per each DMA master in (2^n * 8) bits. This property is
+ deprecated. It' usage is discouraged in favor of data-width one. Moreover
+ the property incorrectly permits to define data-bus width of 8 and 16
+ bits, which is impossible in accordance with DW DMAC IP-core data book.
+ items:
+ maxItems: 4
+ items:
+ enum:
+ - 0 # 8 bits
+ - 1 # 16 bits
+ - 2 # 32 bits
+ - 3 # 64 bits
+ - 4 # 128 bits
+ - 5 # 256 bits
+ default: 0
+
+ multi-block:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ LLP-based multi-block transfer supported by hardware per
+ each DMA channel.
+ items:
+ maxItems: 8
+ items:
+ enum: [0, 1]
+ default: 1
+
+ snps,dma-protection-control:
+ $ref: /schemas/types.yaml#definitions/uint32
+ description: |
+ Bits one-to-one passed to the AHB HPROT[3:1] bus. Each bit setting
+ indicates the following features: bit 0 - privileged mode,
+ bit 1 - DMA is bufferable, bit 2 - DMA is cacheable.
+ default: 0
+ minimum: 0
+ maximum: 7
+
+unevaluatedProperties: false
+
+required:
+ - compatible
+ - "#dma-cells"
+ - reg
+ - interrupts
+
+examples:
+ - |
+ dma-controller@fc000000 {
+ compatible = "snps,dma-spear1340";
+ reg = <0xfc000000 0x1000>;
+ interrupt-parent = <&vic1>;
+ interrupts = <12>;
+
+ dma-channels = <8>;
+ dma-requests = <16>;
+ dma-masters = <4>;
+ #dma-cells = <3>;
+
+ chan_allocation_order = <1>;
+ chan_priority = <1>;
+ block_size = <0xfff>;
+ data-width = <8 8>;
+ multi-block = <0 0 0 0 0 0 0 0>;
+ snps,max-burst-len = <16 16 4 4 4 4 4 4>;
+ };
+...
diff --git a/Documentation/devicetree/bindings/dma/snps-dma.txt b/Documentation/devicetree/bindings/dma/snps-dma.txt
deleted file mode 100644
index 0bedceed1963..000000000000
--- a/Documentation/devicetree/bindings/dma/snps-dma.txt
+++ /dev/null
@@ -1,69 +0,0 @@
-* Synopsys Designware DMA Controller
-
-Required properties:
-- compatible: "snps,dma-spear1340"
-- reg: Address range of the DMAC registers
-- interrupt: Should contain the DMAC interrupt number
-- dma-channels: Number of channels supported by hardware
-- dma-requests: Number of DMA request lines supported, up to 16
-- dma-masters: Number of AHB masters supported by the controller
-- #dma-cells: must be <3>
-- chan_allocation_order: order of allocation of channel, 0 (default): ascending,
- 1: descending
-- chan_priority: priority of channels. 0 (default): increase from chan 0->n, 1:
- increase from chan n->0
-- block_size: Maximum block size supported by the controller
-- data-width: Maximum data width supported by hardware per AHB master
- (in bytes, power of 2)
-
-
-Deprecated properties:
-- data_width: Maximum data width supported by hardware per AHB master
- (0 - 8bits, 1 - 16bits, ..., 5 - 256bits)
-
-
-Optional properties:
-- multi-block: Multi block transfers supported by hardware. Array property with
- one cell per channel. 0: not supported, 1 (default): supported.
-- snps,dma-protection-control: AHB HPROT[3:1] protection setting.
- The default value is 0 (for non-cacheable, non-buffered,
- unprivileged data access).
- Refer to include/dt-bindings/dma/dw-dmac.h for possible values.
-
-Example:
-
- dmahost: dma@fc000000 {
- compatible = "snps,dma-spear1340";
- reg = <0xfc000000 0x1000>;
- interrupt-parent = <&vic1>;
- interrupts = <12>;
-
- dma-channels = <8>;
- dma-requests = <16>;
- dma-masters = <2>;
- #dma-cells = <3>;
- chan_allocation_order = <1>;
- chan_priority = <1>;
- block_size = <0xfff>;
- data-width = <8 8>;
- };
-
-DMA clients connected to the Designware DMA controller must use the format
-described in the dma.txt file, using a four-cell specifier for each channel.
-The four cells in order are:
-
-1. A phandle pointing to the DMA controller
-2. The DMA request line number
-3. Memory master for transfers on allocated channel
-4. Peripheral master for transfers on allocated channel
-
-Example:
-
- serial@e0000000 {
- compatible = "arm,pl011", "arm,primecell";
- reg = <0xe0000000 0x1000>;
- interrupts = <0 35 0x4>;
- dmas = <&dmahost 12 0 1>,
- <&dmahost 13 1 0>;
- dma-names = "rx", "rx";
- };
--
2.26.2
^ permalink raw reply related
* [PATCH v3 06/10] dmaengine: dw: Take HC_LLP flag into account for noLLP auto-config
From: Serge Semin @ 2020-05-26 22:50 UTC (permalink / raw)
To: Vinod Koul, Viresh Kumar, Andy Shevchenko, Dan Williams
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Arnd Bergmann, Rob Herring, linux-mips, devicetree, dmaengine,
linux-kernel
In-Reply-To: <20200526225022.20405-1-Sergey.Semin@baikalelectronics.ru>
Full multi-block transfers functionality is enabled in DW DMA
controller only if CHx_MULTI_BLK_EN is set. But LLP-based transfers
can be executed only if hardcode channel x LLP register feature isn't
enabled, which can be switched on at the IP core synthesis for
optimization. If it's enabled then the LLP register is hardcoded to
zero, so the blocks chaining based on the LLPs is unsupported.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v2:
- Rearrange SoBs.
- Add comment about why hardware accelerated LLP list support depends
on both MBLK_EN and HC_LLP configs setting.
- Use explicit bits state comparison operator.
Changelog v3:
- Move the patch to the head of the series.
---
drivers/dma/dw/core.c | 11 ++++++++++-
drivers/dma/dw/regs.h | 1 +
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 21cb2a58dbd2..33e99d95b3d3 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -1178,8 +1178,17 @@ int do_dma_probe(struct dw_dma_chip *chip)
*/
dwc->block_size =
(4 << ((pdata->block_size >> 4 * i) & 0xf)) - 1;
+
+ /*
+ * According to the DW DMA databook the true scatter-
+ * gether LLPs aren't available if either multi-block
+ * config is disabled (CHx_MULTI_BLK_EN == 0) or the
+ * LLP register is hard-coded to zeros
+ * (CHx_HC_LLP == 1).
+ */
dwc->nollp =
- (dwc_params >> DWC_PARAMS_MBLK_EN & 0x1) == 0;
+ (dwc_params >> DWC_PARAMS_MBLK_EN & 0x1) == 0 ||
+ (dwc_params >> DWC_PARAMS_HC_LLP & 0x1) == 1;
} else {
dwc->block_size = pdata->block_size;
dwc->nollp = !pdata->multi_block[i];
diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h
index 3fce66ecee7a..1ab840b06e79 100644
--- a/drivers/dma/dw/regs.h
+++ b/drivers/dma/dw/regs.h
@@ -125,6 +125,7 @@ struct dw_dma_regs {
/* Bitfields in DWC_PARAMS */
#define DWC_PARAMS_MBLK_EN 11 /* multi block transfer */
+#define DWC_PARAMS_HC_LLP 13 /* set LLP register to zero */
/* bursts size */
enum dw_dma_msize {
--
2.26.2
^ permalink raw reply related
* [PATCH v3 05/10] dmaengine: Introduce DMA-device device_caps callback
From: Serge Semin @ 2020-05-26 22:50 UTC (permalink / raw)
To: Vinod Koul, Viresh Kumar, Dan Williams
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Arnd Bergmann, Andy Shevchenko, Rob Herring, linux-mips,
devicetree, dmaengine, linux-kernel
In-Reply-To: <20200526225022.20405-1-Sergey.Semin@baikalelectronics.ru>
There are DMA devices (like ours version of Synopsys DW DMAC) which have
DMA capabilities non-uniformly redistributed amongst the device channels.
In order to provide a way of exposing the channel-specific parameters to
the DMA engine consumers, we introduce a new DMA-device callback. In case
if provided it gets called from the dma_get_slave_caps() method and is
able to override the generic DMA-device capabilities.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v3:
- This is a new patch created as a result of the discussion with Vinod and
Andy in the framework of DW DMA burst and LLP capabilities.
---
drivers/dma/dmaengine.c | 3 +++
include/linux/dmaengine.h | 2 ++
2 files changed, 5 insertions(+)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index ad56ad58932c..edbb11d56cde 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -599,6 +599,9 @@ int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps)
caps->cmd_resume = !!device->device_resume;
caps->cmd_terminate = !!device->device_terminate_all;
+ if (device->device_caps)
+ device->device_caps(chan, caps);
+
return 0;
}
EXPORT_SYMBOL_GPL(dma_get_slave_caps);
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 6801200c76b6..429eef3a702b 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -899,6 +899,8 @@ struct dma_device {
struct dma_chan *chan, dma_addr_t dst, u64 data,
unsigned long flags);
+ void (*device_caps)(struct dma_chan *chan,
+ struct dma_slave_caps *caps);
int (*device_config)(struct dma_chan *chan,
struct dma_slave_config *config);
int (*device_pause)(struct dma_chan *chan);
--
2.26.2
^ permalink raw reply related
* [PATCH v3 10/10] dmaengine: dw: Initialize max_sg_nents with nollp flag
From: Serge Semin @ 2020-05-26 22:50 UTC (permalink / raw)
To: Vinod Koul, Viresh Kumar, Andy Shevchenko, Dan Williams
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Arnd Bergmann, Rob Herring, linux-mips, devicetree, dmaengine,
linux-kernel
In-Reply-To: <20200526225022.20405-1-Sergey.Semin@baikalelectronics.ru>
Multi-block support provides a way to map the kernel-specific SG-table so
the DW DMA device would handle it as a whole instead of handling the
SG-list items or so called LLP block items one by one. So if true LLP
list isn't supported by the DW DMA engine, then soft-LLP mode will be
utilized to load and execute each LLP-block one by one. The soft-LLP mode
of the DMA transactions execution might not work well for some DMA
consumers like SPI due to its Tx and Rx buffers inter-dependency. Let's
expose the nollp flag indicating the soft-LLP mode by means of the
max_sg_nents capability, so the DMA consumer would be ready to somehow
workaround errors caused by such mode being utilized.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v3:
- This is a new patch created as a result of the discussion with Vinud and
Andy in the framework of DW DMA burst and LLP capabilities.
---
drivers/dma/dw/core.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 29c4ef08311d..b850eb7fd084 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -1054,6 +1054,15 @@ static void dwc_caps(struct dma_chan *chan, struct dma_slave_caps *caps)
struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
caps->max_burst = dwc->max_burst;
+
+ /*
+ * It might be crucial for some devices to have the hardware
+ * accelerated multi-block transfers supported, aka LLPs in DW DMAC
+ * notation. So if LLPs are supported then max_sg_nents is set to
+ * zero which means unlimited number of SG entries can be handled in a
+ * single DMA transaction, otherwise it's just one SG entry.
+ */
+ caps->max_sg_nents = dwc->nollp;
}
int do_dma_probe(struct dw_dma_chip *chip)
--
2.26.2
^ permalink raw reply related
* [PATCH v3 08/10] dmaengine: dw: Add dummy device_caps callback
From: Serge Semin @ 2020-05-26 22:50 UTC (permalink / raw)
To: Vinod Koul, Viresh Kumar, Andy Shevchenko, Dan Williams
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Arnd Bergmann, Rob Herring, linux-mips, devicetree, dmaengine,
linux-kernel
In-Reply-To: <20200526225022.20405-1-Sergey.Semin@baikalelectronics.ru>
Since some DW DMA controllers (like one installed on Baikal-T1 SoC) may
have non-uniform DMA capabilities per device channels, let's add
the DW DMA specific device_caps callback to expose that specifics up to
the DMA consumer. It's a dummy function for now. We'll fill it in with
capabilities overrides in the next commits.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v3:
- This is a new patch created as a result of the discussion with Vinud and
Andy in the framework of DW DMA burst and LLP capabilities.
---
drivers/dma/dw/core.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index fb95920c429e..ceded21537e2 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -1049,6 +1049,11 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
dev_vdbg(chan2dev(chan), "%s: done\n", __func__);
}
+static void dwc_caps(struct dma_chan *chan, struct dma_slave_caps *caps)
+{
+
+}
+
int do_dma_probe(struct dw_dma_chip *chip)
{
struct dw_dma *dw = chip->dw;
@@ -1214,6 +1219,7 @@ int do_dma_probe(struct dw_dma_chip *chip)
dw->dma.device_prep_dma_memcpy = dwc_prep_dma_memcpy;
dw->dma.device_prep_slave_sg = dwc_prep_slave_sg;
+ dw->dma.device_caps = dwc_caps;
dw->dma.device_config = dwc_config;
dw->dma.device_pause = dwc_pause;
dw->dma.device_resume = dwc_resume;
--
2.26.2
^ permalink raw reply related
* [PATCH v3 09/10] dmaengine: dw: Introduce max burst length hw config
From: Serge Semin @ 2020-05-26 22:50 UTC (permalink / raw)
To: Vinod Koul, Viresh Kumar, Andy Shevchenko, Dan Williams
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Arnd Bergmann, Rob Herring, linux-mips, devicetree, dmaengine,
linux-kernel
In-Reply-To: <20200526225022.20405-1-Sergey.Semin@baikalelectronics.ru>
IP core of the DW DMA controller may be synthesized with different
max burst length of the transfers per each channel. According to Synopsis
having the fixed maximum burst transactions length may provide some
performance gain. At the same time setting up the source and destination
multi size exceeding the max burst length limitation may cause a serious
problems. In our case the DMA transaction just hangs up. In order to fix
this lets introduce the max burst length platform config of the DW DMA
controller device and don't let the DMA channels configuration code
exceed the burst length hardware limitation.
Note the maximum burst length parameter can be detected either in runtime
from the DWC parameter registers or from the dedicated DT property.
Depending on the IP core configuration the maximum value can vary from
channel to channel so by overriding the channel slave max_burst capability
we make sure a DMA consumer will get the channel-specific max burst
length.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v2:
- Rearrange SoBs.
- Discard dwc_get_maxburst() accessor. It's enough to have a clamping
guard against exceeding the hardware max burst limitation.
Changelog v3:
- Override the slave channel max_burst capability instead of calculating
the minimum value of max burst lengths and setting the DMA-device
generic capability.
---
drivers/dma/dw/core.c | 5 +++++
drivers/dma/dw/dw.c | 1 +
drivers/dma/dw/of.c | 9 +++++++++
drivers/dma/dw/regs.h | 2 ++
include/linux/platform_data/dma-dw.h | 4 ++++
5 files changed, 21 insertions(+)
diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index ceded21537e2..29c4ef08311d 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -1051,7 +1051,9 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
static void dwc_caps(struct dma_chan *chan, struct dma_slave_caps *caps)
{
+ struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
+ caps->max_burst = dwc->max_burst;
}
int do_dma_probe(struct dw_dma_chip *chip)
@@ -1194,9 +1196,12 @@ int do_dma_probe(struct dw_dma_chip *chip)
dwc->nollp =
(dwc_params >> DWC_PARAMS_MBLK_EN & 0x1) == 0 ||
(dwc_params >> DWC_PARAMS_HC_LLP & 0x1) == 1;
+ dwc->max_burst =
+ (0x4 << (dwc_params >> DWC_PARAMS_MSIZE & 0x7));
} else {
dwc->block_size = pdata->block_size;
dwc->nollp = !pdata->multi_block[i];
+ dwc->max_burst = pdata->max_burst[i] ?: DW_DMA_MAX_BURST;
}
}
diff --git a/drivers/dma/dw/dw.c b/drivers/dma/dw/dw.c
index 7a085b3c1854..4d6b1ecabda4 100644
--- a/drivers/dma/dw/dw.c
+++ b/drivers/dma/dw/dw.c
@@ -86,6 +86,7 @@ static void dw_dma_encode_maxburst(struct dw_dma_chan *dwc, u32 *maxburst)
* Fix burst size according to dw_dmac. We need to convert them as:
* 1 -> 0, 4 -> 1, 8 -> 2, 16 -> 3.
*/
+ *maxburst = clamp(*maxburst, 0U, dwc->max_burst);
*maxburst = *maxburst > 1 ? fls(*maxburst) - 2 : 0;
}
diff --git a/drivers/dma/dw/of.c b/drivers/dma/dw/of.c
index 9e27831dee32..d7323aad7cb5 100644
--- a/drivers/dma/dw/of.c
+++ b/drivers/dma/dw/of.c
@@ -98,6 +98,15 @@ struct dw_dma_platform_data *dw_dma_parse_dt(struct platform_device *pdev)
pdata->multi_block[tmp] = 1;
}
+ if (!of_property_read_u32_array(np, "snps,max-burst-len", mb,
+ nr_channels)) {
+ for (tmp = 0; tmp < nr_channels; tmp++)
+ pdata->max_burst[tmp] = mb[tmp];
+ } else {
+ for (tmp = 0; tmp < nr_channels; tmp++)
+ pdata->max_burst[tmp] = DW_DMA_MAX_BURST;
+ }
+
if (!of_property_read_u32(np, "snps,dma-protection-control", &tmp)) {
if (tmp > CHAN_PROTCTL_MASK)
return NULL;
diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h
index 1ab840b06e79..76654bd13c1a 100644
--- a/drivers/dma/dw/regs.h
+++ b/drivers/dma/dw/regs.h
@@ -126,6 +126,7 @@ struct dw_dma_regs {
/* Bitfields in DWC_PARAMS */
#define DWC_PARAMS_MBLK_EN 11 /* multi block transfer */
#define DWC_PARAMS_HC_LLP 13 /* set LLP register to zero */
+#define DWC_PARAMS_MSIZE 16 /* max group transaction size */
/* bursts size */
enum dw_dma_msize {
@@ -284,6 +285,7 @@ struct dw_dma_chan {
/* hardware configuration */
unsigned int block_size;
bool nollp;
+ u32 max_burst;
/* custom slave configuration */
struct dw_dma_slave dws;
diff --git a/include/linux/platform_data/dma-dw.h b/include/linux/platform_data/dma-dw.h
index f3eaf9ec00a1..13e679afc0e0 100644
--- a/include/linux/platform_data/dma-dw.h
+++ b/include/linux/platform_data/dma-dw.h
@@ -12,6 +12,7 @@
#define DW_DMA_MAX_NR_MASTERS 4
#define DW_DMA_MAX_NR_CHANNELS 8
+#define DW_DMA_MAX_BURST 256
/**
* struct dw_dma_slave - Controller-specific information about a slave
@@ -42,6 +43,8 @@ struct dw_dma_slave {
* @data_width: Maximum data width supported by hardware per AHB master
* (in bytes, power of 2)
* @multi_block: Multi block transfers supported by hardware per channel.
+ * @max_burst: Maximum value of burst transaction size supported by hardware
+ * per channel (in units of CTL.SRC_TR_WIDTH/CTL.DST_TR_WIDTH).
* @protctl: Protection control signals setting per channel.
*/
struct dw_dma_platform_data {
@@ -56,6 +59,7 @@ struct dw_dma_platform_data {
unsigned char nr_masters;
unsigned char data_width[DW_DMA_MAX_NR_MASTERS];
unsigned char multi_block[DW_DMA_MAX_NR_CHANNELS];
+ unsigned int max_burst[DW_DMA_MAX_NR_CHANNELS];
#define CHAN_PROTCTL_PRIVILEGED BIT(0)
#define CHAN_PROTCTL_BUFFERABLE BIT(1)
#define CHAN_PROTCTL_CACHEABLE BIT(2)
--
2.26.2
^ permalink raw reply related
* [PATCH v3 07/10] dmaengine: dw: Set DMA device max segment size parameter
From: Serge Semin @ 2020-05-26 22:50 UTC (permalink / raw)
To: Vinod Koul, Viresh Kumar, Andy Shevchenko, Dan Williams
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Arnd Bergmann, Rob Herring, linux-mips, devicetree, dmaengine,
linux-kernel
In-Reply-To: <20200526225022.20405-1-Sergey.Semin@baikalelectronics.ru>
Maximum block size DW DMAC configuration corresponds to the max segment
size DMA parameter in the DMA core subsystem notation. Lets set it with a
value specific to the probed DW DMA controller. It shall help the DMA
clients to create size-optimized SG-list items for the controller. This in
turn will cause less dw_desc allocations, less LLP reinitializations,
better DMA device performance.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v2:
- This is a new patch created in place of the dropped one:
"dmaengine: dw: Add LLP and block size config accessors".
Changelog v3:
- Use the block_size found for the very first channel instead of looking for
the maximum of maximum block sizes.
- Don't define device-specific device_dma_parameters object, since it has
already been defined by the platform device core.
---
drivers/dma/dw/core.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 33e99d95b3d3..fb95920c429e 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -1229,6 +1229,13 @@ int do_dma_probe(struct dw_dma_chip *chip)
BIT(DMA_MEM_TO_MEM);
dw->dma.residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
+ /*
+ * For now there is no hardware with non uniform maximum block size
+ * across all of the device channels, so we set the maximum segment
+ * size as the block size found for the very first channel.
+ */
+ dma_set_max_seg_size(dw->dma.dev, dw->chan[0].block_size);
+
err = dma_async_device_register(&dw->dma);
if (err)
goto err_dma_register;
--
2.26.2
^ permalink raw reply related
* [PATCH v3 00/10] dmaengine: dw: Take Baikal-T1 SoC DW DMAC peculiarities into account
From: Serge Semin @ 2020-05-26 22:50 UTC (permalink / raw)
To: Vinod Koul, Viresh Kumar
Cc: Serge Semin, Serge Semin, Alexey Malahov, Maxim Kaurkin,
Pavel Parkhomenko, Ramil Zaripov, Ekaterina Skachko, Vadim Vlasov,
Alexey Kolotnikov, Thomas Bogendoerfer, Arnd Bergmann,
Andy Shevchenko, Rob Herring, linux-mips, dmaengine, devicetree,
linux-kernel
Baikal-T1 SoC has an DW DMAC on-board to provide a Mem-to-Mem, low-speed
peripherals Dev-to-Mem and Mem-to-Dev functionality. Mostly it's compatible
with currently implemented in the kernel DW DMAC driver, but there are some
peculiarities which must be taken into account in order to have the device
fully supported.
First of all traditionally we replaced the legacy plain text-based dt-binding
file with yaml-based one. Secondly Baikal-T1 DW DMA Controller provides eight
channels, which alas have different max burst length configuration.
In particular first two channels may burst up to 128 bits (16 bytes) at a time
while the rest of them just up to 32 bits. We must make sure that the DMA
subsystem doesn't set values exceeding these limitations otherwise the
controller will hang up. In third currently we discovered the problem in using
the DW APB SPI driver together with DW DMAC. The problem happens if there is no
natively implemented multi-block LLP transfers support and the SPI-transfer
length exceeds the max lock size. In this case due to asynchronous handling of
Tx- and Rx- SPI transfers interrupt we might end up with Dw APB SSI Rx FIFO
overflow. So if DW APB SSI (or any other DMAC service consumer) intends to use
the DMAC to asynchronously execute the transfers we'd have to at least warn
the user of the possible errors. In forth it's worth to set the DMA device max
segment size with max block size config specific to the DW DMA controller. It
shall help the DMA clients to create size-optimized SG-list items for the
controller. This in turn will cause less dw_desc allocations, less LLP
reinitializations, better DMA device performance.
Finally there is a bug in the algorithm of the nollp flag detection.
In particular even if DW DMAC parameters state the multi-block transfers
support there is still HC_LLP (hardcode LLP) flag, which if set makes expected
by the driver true multi-block LLP functionality unusable. This happens cause'
if HC_LLP flag is set the LLP registers will be hardcoded to zero so the
contiguous multi-block transfers will be only supported. We must take the
flag into account when detecting the LLP support otherwise the driver just
won't work correctly.
This patchset is rebased and tested on the mainline Linux kernel 5.7-rc4:
0e698dfa2822 ("Linux 5.7-rc4")
tag: v5.7-rc4
Changelog v2:
- Rearrange SoBs.
- Move $ref to the root level of the properties. So do do with the
constraints in the DT binding.
- Replace "additionalProperties: false" with "unevaluatedProperties: false"
property in the DT binding file.
- Discard default settings defined out of property enum constraint.
- Set default max-burst-len to 256 TR-WIDTH words in the DT binding.
- Discard noLLP and block_size accessors.
- Set max segment size of the DMA device structure with the DW DMA block size
config.
- Print warning if noLLP flag is set.
- Discard max burst length accessor.
- Add comment about why hardware accelerated LLP list support depends
on both MBLK_EN and HC_LLP configs setting.
- Use explicit bits state comparison operator in noLLP flag setting.
Changelog v3:
- Use the block_size found for the very first channel instead of looking for
the maximum of maximum block sizes.
- Don't define device-specific device_dma_parameters object, since it has
already been defined by the platform device core.
- Add more details into the property description about what limitations
snps,max-burst-len defines.
- Move commit fb7e3bbfc830 ("dmaengine: dw: Take HC_LLP flag into account for
noLLP auto-config") to the head of the series.
- Add a new patch "dmaengine: Introduce min burst length capability" as a
result of the discussion with Vinod and Andy regarding the burst length
capability.
- Add a new patch "dmaengine: Introduce max SG list entries capability"
suggested by Andy.
- Add a new patch "dmaengine: Introduce DMA-device device_caps callback" as
a result of the discussion with Vinud and Andy in the framework of DW DMA
burst and LLP capabilities.
- Add a new patch "dmaengine: dw: Add dummy device_caps callback" as a
preparation commit before setting the max_burst and max_sg_nents
DW DMA capabilities.
- Override the slave channel max_burst capability instead of calculating
the minimum value of max burst lengths and setting the DMA-device
generic capability.
- Add a new patch "dmaengine: dw: Initialize max_sg_nents with nollp flag".
This is required to fix the DW APB SSI issue of the Tx and Rx DMA
channels de-synchronization.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru>
Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru>
Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: dmaengine@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Serge Semin (10):
dt-bindings: dma: dw: Convert DW DMAC to DT binding
dt-bindings: dma: dw: Add max burst transaction length property
dmaengine: Introduce min burst length capability
dmaengine: Introduce max SG list entries capability
dmaengine: Introduce DMA-device device_caps callback
dmaengine: dw: Take HC_LLP flag into account for noLLP auto-config
dmaengine: dw: Set DMA device max segment size parameter
dmaengine: dw: Add dummy device_caps callback
dmaengine: dw: Introduce max burst length hw config
dmaengine: dw: Initialize max_sg_nents with nollp flag
.../bindings/dma/snps,dma-spear1340.yaml | 176 ++++++++++++++++++
.../devicetree/bindings/dma/snps-dma.txt | 69 -------
drivers/dma/dmaengine.c | 5 +
drivers/dma/dw/core.c | 38 +++-
drivers/dma/dw/dw.c | 1 +
drivers/dma/dw/of.c | 9 +
drivers/dma/dw/regs.h | 3 +
include/linux/dmaengine.h | 14 ++
include/linux/platform_data/dma-dw.h | 4 +
9 files changed, 249 insertions(+), 70 deletions(-)
create mode 100644 Documentation/devicetree/bindings/dma/snps,dma-spear1340.yaml
delete mode 100644 Documentation/devicetree/bindings/dma/snps-dma.txt
--
2.26.2
^ permalink raw reply
* [PATCH v3 02/10] dt-bindings: dma: dw: Add max burst transaction length property
From: Serge Semin @ 2020-05-26 22:50 UTC (permalink / raw)
To: Vinod Koul, Viresh Kumar, Rob Herring
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Arnd Bergmann, Andy Shevchenko, linux-mips, dmaengine, devicetree,
linux-kernel
In-Reply-To: <20200526225022.20405-1-Sergey.Semin@baikalelectronics.ru>
This array property is used to indicate the maximum burst transaction
length supported by each DMA channel.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: linux-mips@vger.kernel.org
---
Changelog v2:
- Rearrange SoBs.
- Move $ref to the root level of the properties. So do with the
constraints.
- Set default max-burst-len to 256 TR-WIDTH words.
Changelog v3:
- Add more details into the property description about what limitations
snps,max-burst-len defines.
---
.../bindings/dma/snps,dma-spear1340.yaml | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/Documentation/devicetree/bindings/dma/snps,dma-spear1340.yaml b/Documentation/devicetree/bindings/dma/snps,dma-spear1340.yaml
index e7611840a7cf..20870f5c14dd 100644
--- a/Documentation/devicetree/bindings/dma/snps,dma-spear1340.yaml
+++ b/Documentation/devicetree/bindings/dma/snps,dma-spear1340.yaml
@@ -120,6 +120,21 @@ properties:
enum: [0, 1]
default: 1
+ snps,max-burst-len:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ Maximum length of the burst transactions supported by the controller.
+ This property defines the upper limit of the run-time burst setting
+ (CTLx.SRC_MSIZE/CTLx.DST_MSIZE fields) so the allowed burst length
+ will be from 1 to max-burst-len words. It's an array property with one
+ cell per channel in the units determined by the value set in the
+ CTLx.SRC_TR_WIDTH/CTLx.DST_TR_WIDTH fields (data width).
+ items:
+ maxItems: 8
+ items:
+ enum: [4, 8, 16, 32, 64, 128, 256]
+ default: 256
+
snps,dma-protection-control:
$ref: /schemas/types.yaml#definitions/uint32
description: |
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v5 3/5] dt-bindings: microchip: atmel, at91rm9200-tcb: add sama5d2 compatible
From: Rob Herring @ 2020-05-26 22:50 UTC (permalink / raw)
To: Kamel Bouhara
Cc: devicetree, Alexandre Belloni, Rob Herring,
William Breathitt Gray, linux-input, Nicolas Ferre,
Thomas Petazzoni, Ludovic Desroches, Mark Rutland, linux-iio,
linux-arm-kernel
In-Reply-To: <20200519083716.938384-4-kamel.bouhara@bootlin.com>
On Tue, 19 May 2020 10:37:14 +0200, Kamel Bouhara wrote:
> From: Alexandre Belloni <alexandre.belloni@bootlin.com>
>
> The sama5d2 TC block TIMER_CLOCK1 is different from the at91sam9x5 one.
> Instead of being MCK / 2, it is the TCB GCLK.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> .../soc/microchip/atmel,at91rm9200-tcb.yaml | 36 +++++++++++++++----
> 1 file changed, 30 insertions(+), 6 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v5 2/5] dt-bindings: atmel-tcb: convert bindings to json-schema
From: Rob Herring @ 2020-05-26 22:49 UTC (permalink / raw)
To: Kamel Bouhara
Cc: William Breathitt Gray, Mark Rutland, Nicolas Ferre,
Alexandre Belloni, Ludovic Desroches, linux-arm-kernel,
Thomas Petazzoni, linux-input, devicetree, linux-iio
In-Reply-To: <20200519083716.938384-3-kamel.bouhara@bootlin.com>
On Tue, May 19, 2020 at 10:37:13AM +0200, Kamel Bouhara wrote:
> From: Alexandre Belloni <alexandre.belloni@bootlin.com>
>
> Convert Atmel Timer Counter Blocks bindings to DT schema format using
> json-schema.
>
> Also move it out of mfd as it is not and has never been related to mfd.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
This should have your Sob too.
> ---
> .../devicetree/bindings/mfd/atmel-tcb.txt | 56 --------
> .../soc/microchip/atmel,at91rm9200-tcb.yaml | 126 ++++++++++++++++++
> 2 files changed, 126 insertions(+), 56 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-tcb.txt
> create mode 100644 Documentation/devicetree/bindings/soc/microchip/atmel,at91rm9200-tcb.yaml
>
> diff --git a/Documentation/devicetree/bindings/mfd/atmel-tcb.txt b/Documentation/devicetree/bindings/mfd/atmel-tcb.txt
> deleted file mode 100644
> index c4a83e364cb6..000000000000
> --- a/Documentation/devicetree/bindings/mfd/atmel-tcb.txt
> +++ /dev/null
> @@ -1,56 +0,0 @@
> -* Device tree bindings for Atmel Timer Counter Blocks
> -- compatible: Should be "atmel,<chip>-tcb", "simple-mfd", "syscon".
> - <chip> can be "at91rm9200" or "at91sam9x5"
> -- reg: Should contain registers location and length
> -- #address-cells: has to be 1
> -- #size-cells: has to be 0
> -- interrupts: Should contain all interrupts for the TC block
> - Note that you can specify several interrupt cells if the TC
> - block has one interrupt per channel.
> -- clock-names: tuple listing input clock names.
> - Required elements: "t0_clk", "slow_clk"
> - Optional elements: "t1_clk", "t2_clk"
> -- clocks: phandles to input clocks.
> -
> -The TCB can expose multiple subdevices:
> - * a timer
> - - compatible: Should be "atmel,tcb-timer"
> - - reg: Should contain the TCB channels to be used. If the
> - counter width is 16 bits (at91rm9200-tcb), two consecutive
> - channels are needed. Else, only one channel will be used.
> -
> -Examples:
> -
> -One interrupt per TC block:
> - tcb0: timer@fff7c000 {
> - compatible = "atmel,at91rm9200-tcb", "simple-mfd", "syscon";
> - #address-cells = <1>;
> - #size-cells = <0>;
> - reg = <0xfff7c000 0x100>;
> - interrupts = <18 4>;
> - clocks = <&tcb0_clk>, <&clk32k>;
> - clock-names = "t0_clk", "slow_clk";
> -
> - timer@0 {
> - compatible = "atmel,tcb-timer";
> - reg = <0>, <1>;
> - };
> -
> - timer@2 {
> - compatible = "atmel,tcb-timer";
> - reg = <2>;
> - };
> - };
> -
> -One interrupt per TC channel in a TC block:
> - tcb1: timer@fffdc000 {
> - compatible = "atmel,at91rm9200-tcb", "simple-mfd", "syscon";
> - #address-cells = <1>;
> - #size-cells = <0>;
> - reg = <0xfffdc000 0x100>;
> - interrupts = <26 4>, <27 4>, <28 4>;
> - clocks = <&tcb1_clk>, <&clk32k>;
> - clock-names = "t0_clk", "slow_clk";
> - };
> -
> -
> diff --git a/Documentation/devicetree/bindings/soc/microchip/atmel,at91rm9200-tcb.yaml b/Documentation/devicetree/bindings/soc/microchip/atmel,at91rm9200-tcb.yaml
> new file mode 100644
> index 000000000000..4b683151265e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/microchip/atmel,at91rm9200-tcb.yaml
> @@ -0,0 +1,126 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/soc/microchip/atmel,at91rm9200-tcb.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Atmel Timer Counter Block
> +
> +maintainers:
> + - Alexandre Belloni <alexandre.belloni@bootlin.com>
> +
> +description: |
> + The Atmel (now Microchip) SoCs have timers named Timer Counter Block. Each
> + timer has three channels with two counters each.
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - atmel,at91rm9200-tcb
> + - atmel,at91sam9x5-tcb
> + - const: simple-mfd
> + - const: syscon
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + description:
> + List of interrupts. One interrupt per TCB channel if available or one
> + interrupt for the TC block
> + minItems: 1
> + maxItems: 3
> +
> + clock-names:
> + description:
> + List of clock names. Always includes t0_clk and slow clk. Also includes
> + t1_clk and t2_clk if a clock per channel is available.
This can express the 2nd sentence:
allOf:
- contains:
const: t0_clk
- contains:
const: slow_clk
> + minItems: 2
> + maxItems: 4
> + items:
> + enum:
> + - t0_clk
> + - t1_clk
> + - t2_clk
> + - slow_clk
> +
> + clocks:
> + minItems: 2
> + maxItems: 4
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> +patternProperties:
> + "^timer@[0-2]$":
> + description: The timer block channels that are used as timers.
> + type: object
> + properties:
> + compatible:
> + const: atmel,tcb-timer
> + reg:
> + description:
> + List of channels to use for this particular timer.
> + minItems: 1
> + maxItems: 3
> +
> + required:
> + - compatible
> + - reg
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - '#address-cells'
> + - '#size-cells'
> +
> +examples:
> + - |
> + /* One interrupt per TC block: */
> + tcb0: timer@fff7c000 {
> + compatible = "atmel,at91rm9200-tcb", "simple-mfd", "syscon";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0xfff7c000 0x100>;
> + interrupts = <18 4>;
> + clocks = <&tcb0_clk>, <&clk32k>;
> + clock-names = "t0_clk", "slow_clk";
> +
> + timer@0 {
> + compatible = "atmel,tcb-timer";
> + reg = <0>, <1>;
> + };
> +
> + timer@2 {
> + compatible = "atmel,tcb-timer";
> + reg = <2>;
> + };
> + };
> +
> + /* One interrupt per TC channel in a TC block: */
> + tcb1: timer@fffdc000 {
> + compatible = "atmel,at91rm9200-tcb", "simple-mfd", "syscon";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0xfffdc000 0x100>;
> + interrupts = <26 4>, <27 4>, <28 4>;
> + clocks = <&tcb1_clk>, <&clk32k>;
> + clock-names = "t0_clk", "slow_clk";
> +
> + timer@0 {
> + compatible = "atmel,tcb-timer";
> + reg = <0>;
> + };
> +
> + timer@1 {
> + compatible = "atmel,tcb-timer";
> + reg = <1>;
> + };
> + };
> --
> 2.25.0
>
^ permalink raw reply
* Re: [RESEND v5 03/21] mtd: rawnand: Drop OOB_FIRST placement scheme
From: Boris Brezillon @ 2020-05-26 22:46 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree
In-Reply-To: <20200526195633.11543-4-miquel.raynal@bootlin.com>
On Tue, 26 May 2020 21:56:15 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> This scheme has been introduced for the Davinci controller and means
> that the OOB area must be read *before* the rest of the data. This has
> nothing to do with the ECC in OOB placement as it could be understood
> and most importantly, there is no point in having this function out of
> the Davinci NAND controller driver. A DT property for this scheme has
> been added but never used, even by the Davinci driver which only uses
> this scheme to change the default nand_read_page().
>
> Move the main read_page() helper into the Davinci driver and remove
> the remaining boilerplate.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/davinci_nand.c | 126 +++++++++++++++++++++-------
> drivers/mtd/nand/raw/nand_base.c | 81 ------------------
> include/linux/mtd/rawnand.h | 1 -
> 3 files changed, 98 insertions(+), 110 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/davinci_nand.c b/drivers/mtd/nand/raw/davinci_nand.c
> index 52b87304954b..d975a62caaa5 100644
> --- a/drivers/mtd/nand/raw/davinci_nand.c
> +++ b/drivers/mtd/nand/raw/davinci_nand.c
> @@ -371,6 +371,77 @@ static int nand_davinci_correct_4bit(struct nand_chip *chip, u_char *data,
> return corrected;
> }
>
> +/**
> + * nand_read_page_hwecc_oob_first - hw ecc, read oob first
> + * @chip: nand chip info structure
> + * @buf: buffer to store read data
> + * @oob_required: caller requires OOB data read to chip->oob_poi
> + * @page: page number to read
> + *
> + * Hardware ECC for large page chips, require OOB to be read first. For this
> + * ECC mode, the write_page method is re-used from ECC_HW. These methods
> + * read/write ECC from the OOB area, unlike the ECC_HW_SYNDROME support with
> + * multiple ECC steps, follows the "infix ECC" scheme and reads/writes ECC from
> + * the data area, by overwriting the NAND manufacturer bad block markings.
> + */
> +static int nand_davinci_read_page_hwecc_oob_first(struct nand_chip *chip,
> + uint8_t *buf,
> + int oob_required, int page)
> +{
> + struct mtd_info *mtd = nand_to_mtd(chip);
> + int i, eccsize = chip->ecc.size, ret;
> + int eccbytes = chip->ecc.bytes;
> + int eccsteps = chip->ecc.steps;
> + uint8_t *p = buf;
> + uint8_t *ecc_code = chip->ecc.code_buf;
> + uint8_t *ecc_calc = chip->ecc.calc_buf;
> + unsigned int max_bitflips = 0;
> +
> + /* Read the OOB area first */
> + ret = nand_read_oob_op(chip, page, 0, chip->oob_poi, mtd->oobsize);
> + if (ret)
> + return ret;
> +
> + ret = nand_read_page_op(chip, page, 0, NULL, 0);
> + if (ret)
> + return ret;
> +
> + ret = mtd_ooblayout_get_eccbytes(mtd, ecc_code, chip->oob_poi, 0,
> + chip->ecc.total);
> + if (ret)
> + return ret;
> +
> + for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
> + int stat;
> +
> + chip->ecc.hwctl(chip, NAND_ECC_READ);
> +
> + ret = nand_read_data_op(chip, p, eccsize, false, false);
> + if (ret)
> + return ret;
> +
> + chip->ecc.calculate(chip, p, &ecc_calc[i]);
> +
> + stat = chip->ecc.correct(chip, p, &ecc_code[i], NULL);
> + if (stat == -EBADMSG &&
> + (chip->ecc.options & NAND_ECC_GENERIC_ERASED_CHECK)) {
> + /* check for empty pages with bitflips */
> + stat = nand_check_erased_ecc_chunk(p, eccsize,
> + &ecc_code[i],
> + eccbytes, NULL, 0,
> + chip->ecc.strength);
> + }
> +
> + if (stat < 0) {
> + mtd->ecc_stats.failed++;
> + } else {
> + mtd->ecc_stats.corrected += stat;
> + max_bitflips = max_t(unsigned int, max_bitflips, stat);
> + }
> + }
> + return max_bitflips;
> +}
> +
> /*----------------------------------------------------------------------*/
>
> /* An ECC layout for using 4-bit ECC with small-page flash, storing
> @@ -530,6 +601,13 @@ static int davinci_nand_attach_chip(struct nand_chip *chip)
> break;
> case NAND_ECC_HW:
> if (pdata->ecc_bits == 4) {
> + int chunks = mtd->writesize / 512;
> +
> + if (!chunks || mtd->oobsize < 16) {
> + dev_dbg(&info->pdev->dev, "too small\n");
> + return -EINVAL;
> + }
> +
> /*
> * No sanity checks: CPUs must support this,
> * and the chips may not use NAND_BUSWIDTH_16.
> @@ -552,6 +630,26 @@ static int davinci_nand_attach_chip(struct nand_chip *chip)
> info->chip.ecc.bytes = 10;
> info->chip.ecc.options = NAND_ECC_GENERIC_ERASED_CHECK;
> info->chip.ecc.algo = NAND_ECC_BCH;
> +
> + /*
> + * Update ECC layout if needed ... for 1-bit HW ECC, the
> + * default is OK, but it allocates 6 bytes when only 3
> + * are needed (for each 512 bytes). For 4-bit HW ECC,
> + * the default is not usable: 10 bytes needed, not 6.
> + *
> + * For small page chips, preserve the manufacturer's
> + * badblock marking data ... and make sure a flash BBT
> + * table marker fits in the free bytes.
> + */
> + if (chunks == 1) {
> + mtd_set_ooblayout(mtd,
> + &hwecc4_small_ooblayout_ops);
> + } else if (chunks == 4 || chunks == 8) {
> + mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
> + info->chip.ecc.read_page = nand_davinci_read_page_hwecc_oob_first;
> + } else {
> + return -EIO;
> + }
> } else {
> /* 1bit ecc hamming */
> info->chip.ecc.calculate = nand_davinci_calculate_1bit;
> @@ -567,34 +665,6 @@ static int davinci_nand_attach_chip(struct nand_chip *chip)
> return -EINVAL;
> }
>
> - /*
> - * Update ECC layout if needed ... for 1-bit HW ECC, the default
> - * is OK, but it allocates 6 bytes when only 3 are needed (for
> - * each 512 bytes). For the 4-bit HW ECC, that default is not
> - * usable: 10 bytes are needed, not 6.
> - */
> - if (pdata->ecc_bits == 4) {
> - int chunks = mtd->writesize / 512;
> -
> - if (!chunks || mtd->oobsize < 16) {
> - dev_dbg(&info->pdev->dev, "too small\n");
> - return -EINVAL;
> - }
> -
> - /* For small page chips, preserve the manufacturer's
> - * badblock marking data ... and make sure a flash BBT
> - * table marker fits in the free bytes.
> - */
> - if (chunks == 1) {
> - mtd_set_ooblayout(mtd, &hwecc4_small_ooblayout_ops);
> - } else if (chunks == 4 || chunks == 8) {
> - mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
> - info->chip.ecc.mode = NAND_ECC_HW_OOB_FIRST;
> - } else {
> - return -EIO;
> - }
> - }
> -
> return ret;
> }
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index e2cdfbb45923..7176e513a0bb 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -3075,76 +3075,6 @@ static int nand_read_page_hwecc(struct nand_chip *chip, uint8_t *buf,
> return max_bitflips;
> }
>
> -/**
> - * nand_read_page_hwecc_oob_first - [REPLACEABLE] hw ecc, read oob first
> - * @chip: nand chip info structure
> - * @buf: buffer to store read data
> - * @oob_required: caller requires OOB data read to chip->oob_poi
> - * @page: page number to read
> - *
> - * Hardware ECC for large page chips, require OOB to be read first. For this
> - * ECC mode, the write_page method is re-used from ECC_HW. These methods
> - * read/write ECC from the OOB area, unlike the ECC_HW_SYNDROME support with
> - * multiple ECC steps, follows the "infix ECC" scheme and reads/writes ECC from
> - * the data area, by overwriting the NAND manufacturer bad block markings.
> - */
> -static int nand_read_page_hwecc_oob_first(struct nand_chip *chip, uint8_t *buf,
> - int oob_required, int page)
> -{
> - struct mtd_info *mtd = nand_to_mtd(chip);
> - int i, eccsize = chip->ecc.size, ret;
> - int eccbytes = chip->ecc.bytes;
> - int eccsteps = chip->ecc.steps;
> - uint8_t *p = buf;
> - uint8_t *ecc_code = chip->ecc.code_buf;
> - uint8_t *ecc_calc = chip->ecc.calc_buf;
> - unsigned int max_bitflips = 0;
> -
> - /* Read the OOB area first */
> - ret = nand_read_oob_op(chip, page, 0, chip->oob_poi, mtd->oobsize);
> - if (ret)
> - return ret;
> -
> - ret = nand_read_page_op(chip, page, 0, NULL, 0);
> - if (ret)
> - return ret;
> -
> - ret = mtd_ooblayout_get_eccbytes(mtd, ecc_code, chip->oob_poi, 0,
> - chip->ecc.total);
> - if (ret)
> - return ret;
> -
> - for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
> - int stat;
> -
> - chip->ecc.hwctl(chip, NAND_ECC_READ);
> -
> - ret = nand_read_data_op(chip, p, eccsize, false, false);
> - if (ret)
> - return ret;
> -
> - chip->ecc.calculate(chip, p, &ecc_calc[i]);
> -
> - stat = chip->ecc.correct(chip, p, &ecc_code[i], NULL);
> - if (stat == -EBADMSG &&
> - (chip->ecc.options & NAND_ECC_GENERIC_ERASED_CHECK)) {
> - /* check for empty pages with bitflips */
> - stat = nand_check_erased_ecc_chunk(p, eccsize,
> - &ecc_code[i], eccbytes,
> - NULL, 0,
> - chip->ecc.strength);
> - }
> -
> - if (stat < 0) {
> - mtd->ecc_stats.failed++;
> - } else {
> - mtd->ecc_stats.corrected += stat;
> - max_bitflips = max_t(unsigned int, max_bitflips, stat);
> - }
> - }
> - return max_bitflips;
> -}
> -
> /**
> * nand_read_page_syndrome - [REPLACEABLE] hardware ECC syndrome based page read
> * @chip: nand chip info structure
> @@ -5085,7 +5015,6 @@ static const char * const nand_ecc_modes[] = {
> [NAND_ECC_SOFT] = "soft",
> [NAND_ECC_HW] = "hw",
> [NAND_ECC_HW_SYNDROME] = "hw_syndrome",
> - [NAND_ECC_HW_OOB_FIRST] = "hw_oob_first",
> [NAND_ECC_ON_DIE] = "on-die",
> };
>
> @@ -5834,16 +5763,6 @@ static int nand_scan_tail(struct nand_chip *chip)
> */
>
> switch (ecc->mode) {
> - case NAND_ECC_HW_OOB_FIRST:
> - /* Similar to NAND_ECC_HW, but a separate read_page handle */
> - if (!ecc->calculate || !ecc->correct || !ecc->hwctl) {
> - WARN(1, "No ECC functions supplied; hardware ECC not possible\n");
> - ret = -EINVAL;
> - goto err_nand_manuf_cleanup;
> - }
> - if (!ecc->read_page)
> - ecc->read_page = nand_read_page_hwecc_oob_first;
> - fallthrough;
> case NAND_ECC_HW:
> /* Use standard hwecc read page function? */
> if (!ecc->read_page)
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 0911e845cced..7b87c5dc89bd 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -88,7 +88,6 @@ enum nand_ecc_mode {
> NAND_ECC_SOFT,
> NAND_ECC_HW,
> NAND_ECC_HW_SYNDROME,
> - NAND_ECC_HW_OOB_FIRST,
> NAND_ECC_ON_DIE,
> };
>
^ permalink raw reply
* Re: [RESEND v5 04/21] dt-bindings: mtd: Deprecate OOB_FIRST mode
From: Boris Brezillon @ 2020-05-26 22:42 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree
In-Reply-To: <20200526195633.11543-5-miquel.raynal@bootlin.com>
On Tue, 26 May 2020 21:56:16 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> This mode has never actually been used, it was introduced for a single
> driver and even this driver did not use it in the DT but only in the
> code. Now that this mode has been removed, let's trim the bindings
> definition to avoid carrying useless properties.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> Documentation/devicetree/bindings/mtd/atmel-nand.txt | 3 +--
> Documentation/devicetree/bindings/mtd/nand-controller.yaml | 2 +-
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> index 3aa297c97ab6..ead1826d0d51 100644
> --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> @@ -143,8 +143,7 @@ Required properties:
> Optional properties:
> - atmel,nand-has-dma : boolean to support dma transfer for nand read/write.
> - nand-ecc-mode : String, operation mode of the NAND ecc mode, soft by default.
> - Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
> - "soft_bch".
> + Supported values are: "none", "soft", "hw", "hw_syndrome", "soft_bch".
> - atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware,
> capable of BCH encoding and decoding, on devices where it is present.
> - atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
> diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> index d261b7096c69..d529f8587ba6 100644
> --- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> +++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> @@ -49,7 +49,7 @@ patternProperties:
> nand-ecc-mode:
> allOf:
> - $ref: /schemas/types.yaml#/definitions/string
> - - enum: [ none, soft, hw, hw_syndrome, hw_oob_first, on-die ]
> + - enum: [ none, soft, hw, hw_syndrome, on-die ]
> description:
> Desired ECC engine, either hardware (most of the time
> embedded in the NAND controller) or software correction
^ permalink raw reply
* Re: [RESEND v5 05/21] mtd: rawnand: Return an enum from of_get_nand_ecc_algo()
From: Boris Brezillon @ 2020-05-26 22:41 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree
In-Reply-To: <20200526195633.11543-6-miquel.raynal@bootlin.com>
On Tue, 26 May 2020 21:56:17 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> There is an enumeration to list ECC algorithm, let's use it instead of
> returning an int.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/nand_base.c | 35 +++++++++++++++++---------------
> 1 file changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 7176e513a0bb..a756f3193558 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -5048,17 +5048,20 @@ static const char * const nand_ecc_algos[] = {
> [NAND_ECC_RS] = "rs",
> };
>
> -static int of_get_nand_ecc_algo(struct device_node *np)
> +static enum nand_ecc_algo of_get_nand_ecc_algo(struct device_node *np)
> {
> + enum nand_ecc_algo ecc_algo;
> const char *pm;
> - int err, i;
> + int err;
>
> err = of_property_read_string(np, "nand-ecc-algo", &pm);
> if (!err) {
> - for (i = NAND_ECC_HAMMING; i < ARRAY_SIZE(nand_ecc_algos); i++)
> - if (!strcasecmp(pm, nand_ecc_algos[i]))
> - return i;
> - return -ENODEV;
> + for (ecc_algo = NAND_ECC_HAMMING;
> + ecc_algo < ARRAY_SIZE(nand_ecc_algos);
> + ecc_algo++) {
> + if (!strcasecmp(pm, nand_ecc_algos[ecc_algo]))
> + return ecc_algo;
> + }
> }
>
> /*
> @@ -5066,15 +5069,14 @@ static int of_get_nand_ecc_algo(struct device_node *np)
> * for some obsoleted values that were specifying ECC algorithm.
> */
> err = of_property_read_string(np, "nand-ecc-mode", &pm);
> - if (err < 0)
> - return err;
> + if (!err) {
> + if (!strcasecmp(pm, "soft"))
> + return NAND_ECC_HAMMING;
> + else if (!strcasecmp(pm, "soft_bch"))
> + return NAND_ECC_BCH;
> + }
>
> - if (!strcasecmp(pm, "soft"))
> - return NAND_ECC_HAMMING;
> - else if (!strcasecmp(pm, "soft_bch"))
> - return NAND_ECC_BCH;
> -
> - return -ENODEV;
> + return NAND_ECC_UNKNOWN;
> }
>
> static int of_get_nand_ecc_step_size(struct device_node *np)
> @@ -5119,7 +5121,8 @@ static bool of_get_nand_on_flash_bbt(struct device_node *np)
> static int nand_dt_init(struct nand_chip *chip)
> {
> struct device_node *dn = nand_get_flash_node(chip);
> - int ecc_mode, ecc_algo, ecc_strength, ecc_step;
> + enum nand_ecc_algo ecc_algo;
> + int ecc_mode, ecc_strength, ecc_step;
>
> if (!dn)
> return 0;
> @@ -5141,7 +5144,7 @@ static int nand_dt_init(struct nand_chip *chip)
> if (ecc_mode >= 0)
> chip->ecc.mode = ecc_mode;
>
> - if (ecc_algo >= 0)
> + if (ecc_algo != NAND_ECC_UNKNOWN)
> chip->ecc.algo = ecc_algo;
>
> if (ecc_strength >= 0)
^ permalink raw reply
* Re: [RESEND v5 07/21] mtd: rawnand: Create a new enumeration to describe OOB placement
From: Boris Brezillon @ 2020-05-26 22:39 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree
In-Reply-To: <20200526195633.11543-8-miquel.raynal@bootlin.com>
On Tue, 26 May 2020 21:56:19 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> There is currently a confusion between the ECC type/mode/provider
> (eg. hardware, software, on-die or none) and the ECC bytes placement.
>
> Create a new enumeration to describe this placement.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/nand_base.c | 4 ++++
> include/linux/mtd/rawnand.h | 12 ++++++++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index ef70ca0828c3..a4470a19c805 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -5018,6 +5018,10 @@ static const char * const nand_ecc_modes[] = {
> [NAND_ECC_ON_DIE] = "on-die",
> };
>
> +static const char * const nand_ecc_placement[] = {
> + [NAND_ECC_PLACEMENT_INTERLEAVED] = "interleaved",
> +};
> +
> static int of_get_nand_ecc_mode(struct device_node *np)
> {
> const char *pm;
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 8187056dd3a0..6eb4d91b07eb 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -92,6 +92,18 @@ enum nand_ecc_mode {
> NAND_ECC_ON_DIE,
> };
>
> +/**
> + * enum nand_ecc_placement - NAND ECC placement
> + * @NAND_ECC_PLACEMENT_FREE: The driver can decide where to put ECC bytes.
Can we name that one UNDEFINED instead of FREE, and it's not really the
driver that decides (unless you have a choice or use SW ECC), more the ECC
engine itself.
> + * Default behavior is to put them at the end of the
> + * OOB area.
I wouldn't even define a default behavior here, but instead add a value for
OOB/TAIL placement.
> + * @NAND_ECC_PLACEMENT_INTERLEAVED: Syndrome layout: interleave data and OOB.
^ECC bytes
> + */
> +enum nand_ecc_placement {
> + NAND_ECC_PLACEMENT_FREE,
> + NAND_ECC_PLACEMENT_INTERLEAVED,
> +};
> +
> enum nand_ecc_algo {
> NAND_ECC_UNKNOWN,
> NAND_ECC_HAMMING,
^ permalink raw reply
* Re: [RESEND][V5 PATCH 1/2] dt-bindings: Added device tree binding for max98390
From: Rob Herring @ 2020-05-26 22:36 UTC (permalink / raw)
To: Steve Lee
Cc: lgirdwood, broonie, alsa-devel, devicetree, linux-kernel,
ryan.lee.maxim, ryans.lee, steves.lee.maxim
In-Reply-To: <20200518004930.20973-1-steves.lee@maximintegrated.com>
On Mon, May 18, 2020 at 09:49:30AM +0900, Steve Lee wrote:
> Add documentation for DT binding of max98390 amplifier driver.
>
> Signed-off-by: Steve Lee <steves.lee@maximintegrated.com>
> ---
>
>
> Changed since V4:
> * No changes.
> Changed since V3:
> * No changes.
> Changed since V2:
> * No changes.
> Changed since V1:
> * Modified sample text in example
You are obviously sending patches too quickly. Give folks a chance to
review.
>
> .../devicetree/bindings/sound/max98390.txt | 26 +++++++++++++++++++
Bindings are now in DT schema format. Please convert this. See
Documentation/devicetree/writing-schema.rst
> 1 file changed, 26 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/max98390.txt
>
> diff --git a/Documentation/devicetree/bindings/sound/max98390.txt b/Documentation/devicetree/bindings/sound/max98390.txt
> new file mode 100644
> index 000000000000..0ddd4c6ae55e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/max98390.txt
> @@ -0,0 +1,26 @@
> +Maxim Integrated MAX98390 Speaker Amplifier
> +
> +This device supports I2C.
> +
> +Required properties:
> +
> + - compatible : "maxim,max98390"
> +
> + - reg : the I2C address of the device.
> +
> +Optional properties:
> +
> +- maxim,temperature_calib
> + u32. The calculated temperature data was measured while doing the calibration. Data : Temp / 100 * 2^12
> +
> +- maxim,r0_calib
> + u32. This is r0 calibration data which was measured in factory mode.
Unless these are shared already with other Maxim chips, s/_/-/.
> +
> +Example:
> +
> +codec: max98390@38 {
amplifier@38
> + compatible = "maxim,max98390";
> + reg = <0x38>;
> + maxim,temperature_calib = <1024>;
> + maxim,r0_calib = <100232>;
> +};
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v5 1/5] dt-bindings: remoteproc: Add Qualcomm PIL info binding
From: Rob Herring @ 2020-05-26 22:32 UTC (permalink / raw)
To: Bjorn Andersson
Cc: linux-kernel, linux-arm-msm, devicetree, Ohad Ben-Cohen,
linux-remoteproc, Stephen Boyd, Andy Gross, Rob Herring
In-Reply-To: <20200513055641.1413100-2-bjorn.andersson@linaro.org>
On Tue, 12 May 2020 22:56:37 -0700, Bjorn Andersson wrote:
> Add a devicetree binding for the Qualcomm peripheral image loader
> relocation information region found in the IMEM.
>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> Changes since v4:
> - Fixed reg in example to make it compile
>
> .../bindings/remoteproc/qcom,pil-info.yaml | 44 +++++++++++++++++++
> 1 file changed, 44 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,pil-info.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [RFC v1 2/3] drivers: nvmem: Add driver for QTI qfprom-efuse support
From: Doug Anderson @ 2020-05-26 22:31 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Ravi Kumar Bokka (Temp), Rob Herring, LKML,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Rajendra Nayak, Sai Prakash Ranjan, dhavalp, mturney, sparate,
c_rbokka, mkurumel
In-Reply-To: <871dd2c1-4b16-f883-b8c5-806a0df1edf8@linaro.org>
Hi,
On Fri, May 22, 2020 at 4:18 AM Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
>
> On 21/05/2020 22:28, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, May 21, 2020 at 8:56 AM Srinivas Kandagatla
> > <srinivas.kandagatla@linaro.org> wrote:
> >>
> >> On 21/05/2020 16:10, Doug Anderson wrote:
> >>>> On 20/05/2020 23:48, Doug Anderson wrote:
> >>>>>> Is this only applicable for corrected address space?
> >>>>> I guess I was proposing a two dts-node / two drive approach here.
> >>>>>
> >>>>> dts node #1:just covers the memory range for accessing the FEC-corrected data
> >>>>> driver #1: read-only and reads the FEC-corrected data
> >>>>>
> >>>>> dts node #2: covers the memory range that's_not_ the FEC-corrected
> >>>>> memory range.
> >>>>> driver #2: read-write. reading reads uncorrected data
> >>>>>
> >>>>> Does that seem sane?
> >>>> I see your point but it does not make sense to have two node for same thing.
> >>> OK, so that sounds as if we want to go with the proposal where we
> >>> "deprecate the old driver and/or bindings and say that there really
> >>> should just be one node and one driver".
> >>>
> >>> Would this be acceptable to you?
> >>>
> >>> 1. Officially mark the old bindings as deprecated.
> >>
> >> Possibly Yes for some reasons below!
> >>
> >>>
> >>> 2. Leave the old driver there to support the old deprecated bindings,
> >>> at least until everyone can be transferred over. There seem to be
> >>> quite a few existing users of "qcom,qfprom" and we're supposed to make
> >>> an attempt at keeping the old device trees working, at least for a
> >>> little while. Once everyone is transferred over we could decide to
> >>> delete the old driver.
> >> we could consider "qcom,qfrom" to be only passing corrected address
> >> space. Till we transition users to new bindings!
> >>
> >>>
> >> Yes.
> >>
> >>> 3. We will have a totally new driver here.
> >> No, we should still be using the same driver. But the exiting driver
> >> seems to incorrect and is need of fixing.
> >>
> >> Having a look at the memory map for old SoCs like msm8996 and msm8916
> >> shows that memory map that was passed to qfprom driver is corrected
> >> address space. Writes will not obviously work!
> >>
> >> This should also be true with sdm845 or sc7180
> >>
> >> That needs to be fixed first!
> >
> > OK, so to summarize:
> >
>
> > 1. We will have one driver: "drivers/nvmem/qfprom.c"
>
> Yes, we should one driver for this because we are dealing with exactly
> same IP.
>
> >
> > 2. If the driver detects that its reg is pointing to the corrected
> > address space then it should operate in read-only mode. Maybe it can
> > do this based on the compatible string being just "qcom,qfprom" or
> > maybe it can do this based on the size of the "reg".
>
> I found out that there is a version register at offset of 0x6000 which
> can give MAJOR, MINOR and STEP numbers.
>
> So we could still potentially continue using "qcom,qfprom"
OK, sounds good. I think it's still good practice to include both the
SoC specific and the generic. Even if the driver never does anything
with the SoC-specific compatible string it doesn't hurt to have it
there. Thus, we'd want:
compatible = "qcom,msm8996-qfprom", "qcom,qfprom"
The driver itself would never need to refer to the SoC-specific name
but that does give us more flexibility.
> The address space can be split into 3 resources, which is inline with
> Specs as well
>
> 1. Raw address space ("raw")
> 2. Configuration address space ("conf" or "core")
> 3. Corrected address space ("corrected")
Sure, this is OK with me then. Originally Ravi had 3 ranges but then
he was (in the driver) treating it as one range. As long as the
driver truly treats it as 3 ranges I have no problem doing it like
this.
In general, over the years, there has been a push to keep
implementation details out of the dts and rely more on the "of_match"
table to add SoC-specific details. This becomes really important when
1 year down the road you realize that you need one more random
property or address range to fix some bug and then you need to figure
out how to try to keep old "dts" files compatible. It's not a
hard-and-fast rule, though.
> Exiting qfprom entries or read-only qfprom will have "corrected"
> address space which can be the only resource provided by device tree
> entries.
> Other two entries("raw" and "conf") are optional.
>
> qfprom: qfprom@780000 {
> compatible = "qcom,qfprom";
> reg = <0 0x00780000 0 0x8ff>,
> <0 0x00782000 0 0x100>,
> <0 0x00784000 0 0x8ff>;
> reg-names = "raw", "conf", "corrected";
>
> vcc-supply = <&vreg_xyz>;
>
> clocks = <&gcc GCC_SEC_CTRL_CLK_SRC>;
> clock-names = "secclk";
>
> assigned-clocks = <&gcc GCC_SEC_CTRL_CLK_SRC>;
> assigned-clock-rates = <19200000>;
>
> qcom,fuse-blow-frequency = <4800000>
>
> #address-cells = <1>;
> #size-cells = <1>;
>
> qusb2p_hstx_trim: hstx-trim-primary@25b {
> reg = <0x25b 0x1>;
> bits = <1 3>;
> };
> };
>
> Regarding clk rate setting, the default rate can be set using
> assigned-clock-rates property, however the blow frequency can go as new
> binding.
> regarding voltage range for regulator, it should come as part of board
> specific voltage regulator node. In worst case we can discuss on adding
> new bindings for allowing specific range.
I'd up to you (and Rob H, who probably will wait for the next rev of
the binding before chiming in) but the "qcom,fuse-blow-frequency" is
the type of property that feels better in the driver and achieved from
the of_match table. Then you don't need to worry about adding it to
the bindings. Voltage (if needed) would be similar, but I would hope
we don't need it.
> for Older SoCs: we still continue to use old style with just one
> resource corresponding to corrected by default.
>
> qfprom: qfprom@784000 {
> compatible = "qcom,qfprom";
> reg = <0 0x00784000 0 0x8ff>;
> #address-cells = <1>;
> #size-cells = <1>;
>
> qusb2p_hstx_trim: hstx-trim-primary@1eb {
> reg = <0x1eb 0x1>;
> bits = <1 4>;
> };
>
> qusb2s_hstx_trim: hstx-trim-secondary@1eb {
> reg = <0x1eb 0x2>;
> bits = <6 4>;
> };
> };
>
>
> I see the patch as adding write support to qfprom, rather than adding
> new driver or new SoC support.
>
> This in summary should give us good direction for this patch!
>
> Correct me if I miss understood something here!
Sounds sane to me.
-Doug
^ permalink raw reply
* Re: [PATCH V5 5/5] dt-bindings: clock: Convert i.MX6UL clock to json-schema
From: Rob Herring @ 2020-05-26 22:31 UTC (permalink / raw)
To: Anson Huang
Cc: linux-kernel, Linux-imx, shawnguo, robh+dt, sboyd,
linux-arm-kernel, devicetree, linux-clk, s.hauer, mturquette,
festevam, kernel
In-Reply-To: <1589328684-1397-6-git-send-email-Anson.Huang@nxp.com>
On Wed, 13 May 2020 08:11:24 +0800, Anson Huang wrote:
> Convert the i.MX6UL clock binding to DT schema format using json-schema.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> Acked-by: Stephen Boyd <sboyd@kernel.org>
> ---
> Changes since V4:
> - add descriptions for interrupts and each item of it.
> ---
> .../devicetree/bindings/clock/imx6ul-clock.txt | 13 -----
> .../devicetree/bindings/clock/imx6ul-clock.yaml | 66 ++++++++++++++++++++++
> 2 files changed, 66 insertions(+), 13 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/clock/imx6ul-clock.txt
> create mode 100644 Documentation/devicetree/bindings/clock/imx6ul-clock.yaml
>
Applied, thanks!
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox