Messages in this thread | | | Subject | Re: [PATCH v9 1/2] dt-bindings: dma: Add bindings for Intel LGM SoC | From | "Reddy, MallikarjunaX" <> | Date | Wed, 25 Nov 2020 18:39:44 +0800 |
| |
Hi Vinod,
Thanks for your review comments, My comments inline.
On 11/25/2020 1:23 AM, Vinod Koul wrote: > On 24-11-20, 00:30, Reddy, MallikarjunaX wrote: >> Hi Vinod, >> >> Thanks for your valuable review. My comments inline. >> >> On 11/21/2020 8:19 PM, Vinod Koul wrote: >>> On 20-11-20, 19:30, Reddy, MallikarjunaX wrote: >>>> Hi Vinod, >>>> Thanks for the review. My comments inline. >>>> >>>> On 11/18/2020 11:55 PM, Vinod Koul wrote: >>>>> On 12-11-20, 13:38, Amireddy Mallikarjuna reddy wrote: >>>>>> Add DT bindings YAML schema for DMA controller driver >>>>>> of Lightning Mountain (LGM) SoC. >>>>>> >>>>>> Signed-off-by: Amireddy Mallikarjuna reddy <mallikarjunax.reddy@linux.intel.com> >>>>>> --- >>>>>> v1: >>>>>> - Initial version. >>>>>> >>>>>> v2: >>>>>> - Fix bot errors. >>>>>> >>>>>> v3: >>>>>> - No change. >>>>>> >>>>>> v4: >>>>>> - Address Thomas langer comments >>>>>> - use node name pattern as dma-controller as in common binding. >>>>>> - Remove "_" (underscore) in instance name. >>>>>> - Remove "port-" and "chan-" in attribute name for both 'dma-ports' & 'dma-channels' child nodes. >>>>>> >>>>>> v5: >>>>>> - Moved some of the attributes in 'dma-ports' & 'dma-channels' child nodes to dma client/consumer side as cells in 'dmas' properties. >>>>>> >>>>>> v6: >>>>>> - Add additionalProperties: false >>>>>> - completely removed 'dma-ports' and 'dma-channels' child nodes. >>>>>> - Moved channel dt properties to client side dmas. >>>>>> - Use standard dma-channels and dma-channel-mask properties. >>>>>> - Documented reset-names >>>>>> - Add description for dma-cells >>>>>> >>>>>> v7: >>>>>> - modified compatible to oneof >>>>>> - Reduced number of dma-cells to 3 >>>>>> - Fine tune the description of some properties. >>>>>> >>>>>> v7-resend: >>>>>> - rebase to 5.10-rc1 >>>>>> >>>>>> v8: >>>>>> - rebased to 5.10-rc3 >>>>>> - Fixing the bot issues (wrong indentation) >>>>>> >>>>>> v9: >>>>>> - rebased to 5.10-rc3 >>>>>> - Use 'enum' instead of oneOf+const >>>>>> - Drop '#dma-cells' in required:, already covered in dma-common.yaml >>>>>> - Drop nodename Already covered by dma-controller.yaml >>>>>> --- >>>>>> .../devicetree/bindings/dma/intel,ldma.yaml | 130 +++++++++++++++++++++ >>>>>> 1 file changed, 130 insertions(+) >>>>>> create mode 100644 Documentation/devicetree/bindings/dma/intel,ldma.yaml >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/dma/intel,ldma.yaml b/Documentation/devicetree/bindings/dma/intel,ldma.yaml >>>>>> new file mode 100644 >>>>>> index 000000000000..c06281a10178 >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/dma/intel,ldma.yaml >>>>>> @@ -0,0 +1,130 @@ >>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>>>> +%YAML 1.2 >>>>>> +--- >>>>>> +$id: http://devicetree.org/schemas/dma/intel,ldma.yaml# >>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>>>> + >>>>>> +title: Lightning Mountain centralized low speed DMA and high speed DMA controllers. >>>>>> + >>>>>> +maintainers: >>>>>> + - chuanhua.lei@intel.com >>>>>> + - mallikarjunax.reddy@intel.com >>>>>> + >>>>>> +allOf: >>>>>> + - $ref: "dma-controller.yaml#" >>>>>> + >>>>>> +properties: >>>>>> + compatible: >>>>>> + enum: >>>>>> + - intel,lgm-cdma >>>>>> + - intel,lgm-dma2tx >>>>>> + - intel,lgm-dma1rx >>>>>> + - intel,lgm-dma1tx >>>>>> + - intel,lgm-dma0tx >>>>>> + - intel,lgm-dma3 >>>>>> + - intel,lgm-toe-dma30 >>>>>> + - intel,lgm-toe-dma31 >>>>>> + >>>>>> + reg: >>>>>> + maxItems: 1 >>>>>> + >>>>>> + "#dma-cells": >>>>>> + const: 3 >>>>>> + description: >>>>>> + The first cell is the peripheral's DMA request line. >>>>>> + The second cell is the peripheral's (port) number corresponding to the channel. >>>>>> + The third cell is the burst length of the channel. >>>>>> + >>>>>> + dma-channels: >>>>>> + minimum: 1 >>>>>> + maximum: 16 >>>>>> + >>>>>> + dma-channel-mask: >>>>>> + maxItems: 1 >>>>>> + >>>>>> + clocks: >>>>>> + maxItems: 1 >>>>>> + >>>>>> + resets: >>>>>> + maxItems: 1 >>>>>> + >>>>>> + reset-names: >>>>>> + items: >>>>>> + - const: ctrl >>>>>> + >>>>>> + interrupts: >>>>>> + maxItems: 1 >>>>>> + >>>>>> + intel,dma-poll-cnt: >>>>>> + $ref: /schemas/types.yaml#definitions/uint32 >>>>>> + description: >>>>>> + DMA descriptor polling counter is used to control the poling mechanism >>>>> s/poling/polling >>>> Ok, Thanks. >>>>>> + for the descriptor fetching for all channels. >>>>>> + >>>>>> + intel,dma-byte-en: >>>>>> + type: boolean >>>>>> + description: >>>>>> + DMA byte enable is only valid for DMA write(RX). >>>>>> + Byte enable(1) means DMA write will be based on the number of dwords >>>>>> + instead of the whole burst. >>>>> Can you explain this, also sounds you could use _maxburst values..? >>>> when dma-byte-en = 0 (disabled) DMA write will be in terms of burst length, >>>> dma-byte-en = 1 (enabled) write will be in terms of Dwords. >>>> >>>> Byte enable = 0 (Disabled) means that DMA write will be based on the burst >>>> length, even if it only transmits one byte. >>>> Byte enable = 1(enabled) means that DMA write will be based on the number of >>>> Dwords, instead of the whole burst. >>> Sounds like a hw property or is this configurable to engine..? >> Yes its hw property. Not configurable to engine. >>>>>> + >>>>>> + intel,dma-drb: >>>>>> + type: boolean >>>>>> + description: >>>>>> + DMA descriptor read back to make sure data and desc synchronization. >>>>>> + >>>>>> + intel,dma-desc-in-sram: >>>>>> + type: boolean >>>>>> + description: >>>>>> + DMA descritpors in SRAM or not. Some old controllers descriptors >>>>>> + can be in DRAM or SRAM. The new ones are all in SRAM. >>>>> should that not be decided by driver..? Or is this a hw property? >>>> This is DMA controller capability. It can be decided from driver also. i >>>> will change accordingly. >>>>>> + >>>>>> + intel,dma-orrc: >>>>>> + $ref: /schemas/types.yaml#definitions/uint32 >>>>>> + description: >>>>>> + DMA outstanding read counter value determine the number of >>>>>> + ORR-Outstanding Read Request. The maximum value is 16. >>>>> How would this be used by folks..? >>>> A register bit will be used to enable/disable the ORR feature. >>>> >>>> Outstanding Read Capability introduce CMD FIFO to support up to 16 >>>> outstanding reads for different packet in same channel. >>>> >>>> For large packets up to 16 OR can be issued, the number of OR is >>>> configurable. >>> How will configure this and when..? >> This is DMA (ver > DMA_VER22) hw capability and is configured from device >> tree. >> >> If this property is not present or count is zero means orrc capability is >> disabled. >> If orrc count is 4 <= orr_cnt < 16 then write the enable bit and value to >> corresponding register. >> >> Ex: >> if (d->orrc > 0 && d->orrc <= DMA_ORRC_MAX_CNT) >> val = DMA_ORRC_EN | FIELD_PREP(DMA_ORRC_ORRCNT, d->orrc); >> >> ldma_update_bits(d, mask, val, DMA_ORRC); >> >> This hw capability supports dma instances ver > DMA_VER22. > Sounds like this can be coded in driver and used based on compatible? Yes, we can do that. I will update in the next patch. >
| |