lkml.org 
[lkml]   [2020]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v9 1/2] dt-bindings: dma: Add bindings for Intel LGM SoC
    From
    Date
    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.
    >
    >>>> +
    >>>> + intel,dma-dburst-wr:
    >>>> + type: boolean
    >>>> + description:
    >>>> + Enable RX dynamic burst write. When it is enabled, the DMA does RX dynamic burst;
    >>>> + if it is disabled, the DMA RX will still support programmable fixed burst size of 2,4,8,16.
    >>>> + It only applies to RX DMA and memcopy DMA.
    >>>> +
    >>>> +required:
    >>>> + - compatible
    >>>> + - reg
    >>> So only two are mandatory, what about the bunch of intel properties you
    >>> added above..?
    >> Some of the properties are DMA capabilities, Enabling from device tree.
    >> other properties will use default values from driver if we dont pass it from
    >> device tree.
    >

    \
     
     \ /
      Last update: 2020-11-23 17:32    [W:5.577 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site