[v5,1/2] dt-bindings: dma: Add bindings for intel LGM SOC

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.
---
 .../devicetree/bindings/dma/intel,ldma.yaml        | 319 +++++++++++++++++++++
 1 file changed, 319 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/intel,ldma.yaml

On Fri, Aug 14, 2020 at 01:26:09PM +0800, 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.
> ---
>  .../devicetree/bindings/dma/intel,ldma.yaml        | 319 +++++++++++++++++++++
>  1 file changed, 319 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..9beaf191a6de
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/intel,ldma.yaml
> @@ -0,0 +1,319 @@
> +# 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:
> + $nodename:
> +   pattern: "^dma-controller(@.*)?$"
> +
> + "#dma-cells":
> +   const: 1

Example says 3.

> +
> + compatible:
> +  anyOf:
> +   - const: intel,lgm-cdma
> +   - const: intel,lgm-dma2tx
> +   - const: intel,lgm-dma1rx
> +   - const: intel,lgm-dma1tx
> +   - const: intel,lgm-dma0tx
> +   - const: intel,lgm-dma3
> +   - const: intel,lgm-toe-dma30
> +   - const: intel,lgm-toe-dma31

'anyOf' doesn't make sense here. This should be a single 'enum'.

> +
> + reg:
> +  maxItems: 1
> +
> + clocks:
> +  maxItems: 1
> +
> + resets:
> +  maxItems: 1
> +
> + interrupts:
> +  maxItems: 1
> +
> + intel,dma-poll-cnt:
> +   $ref: /schemas/types.yaml#definitions/uint32
> +   description:
> +     DMA descriptor polling counter. It may need fine tune according
> +     to the system application scenario.
> +
> + 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.
> +
> + intel,dma-drb:
> +    type: boolean
> +    description:
> +      DMA descriptor read back to make sure data and desc synchronization.
> +
> + intel,dma-burst:
> +    $ref: /schemas/types.yaml#definitions/uint32
> +    description:
> +       Specifiy the DMA burst size(in dwords), the valid value will be 8, 16, 32.
> +       Default is 16 for data path dma, 32 is for memcopy DMA.
> +
> + intel,dma-polling-cnt:
> +    $ref: /schemas/types.yaml#definitions/uint32
> +    description:
> +       DMA descriptor polling counter. It may need fine tune according to
> +       the system application scenario.
> +
> + 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.
> +
> + intel,dma-orrc:
> +    $ref: /schemas/types.yaml#definitions/uint32
> +    description:
> +       DMA outstanding read counter. The maximum value is 16, and it may
> +       need fine tune according to the system application scenarios.
> +
> + intel,dma-dburst-wr:
> +    type: boolean
> +    description:
> +       Enable RX dynamic burst write. It only applies to RX DMA and memcopy DMA.
> +
> +
> + dma-ports:
> +    type: object
> +    description:
> +       This sub-node must contain a sub-node for each DMA port.
> +    properties:
> +      '#address-cells':
> +        const: 1
> +      '#size-cells':
> +        const: 0
> +
> +    patternProperties:
> +      "^dma-ports@[0-9]+$":
> +          type: object
> +
> +          properties:
> +            reg:
> +              items:
> +                - enum: [0, 1, 2, 3, 4, 5]
> +              description:
> +                 Which port this node refers to.
> +
> +            intel,name:
> +              $ref: /schemas/types.yaml#definitions/string-array
> +              description:
> +                 Port name of each DMA port.

No other DMA controller needs this, why do you?

> +
> +            intel,chans:
> +              $ref: /schemas/types.yaml#/definitions/uint32-array
> +              description:
> +                 The channels included on this port. Format is channel start
> +                 number and how many channels on this port.

Why does this need to be in DT? This all seems like it can be in the dma 
cells for each client.

> +
> +          required:
> +            - reg
> +            - intel,name
> +            - intel,chans
> +
> +
> + ldma-channels:
> +    type: object
> +    description:
> +       This sub-node must contain a sub-node for each DMA channel.
> +    properties:
> +      '#address-cells':
> +        const: 1
> +      '#size-cells':
> +        const: 0
> +
> +    patternProperties:
> +      "^ldma-channels@[0-15]+$":
> +          type: object
> +
> +          properties:
> +            reg:
> +              items:
> +                - enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
> +              description:
> +                 Which channel this node refers to.
> +
> +            intel,desc_num:
> +              $ref: /schemas/types.yaml#/definitions/uint32
> +              description:
> +                 Per channel maximum descriptor number. The max value is 255.
> +
> +            intel,hdr-mode:
> +              $ref: /schemas/types.yaml#/definitions/uint32-array
> +              description:
> +                 The first parameter is header mode size, the second
> +                 parameter is checksum enable or disable. If enabled,
> +                 header mode size is ignored. If disabled, header mode
> +                 size must be provided.
> +
> +            intel,hw-desc:
> +              $ref: /schemas/types.yaml#/definitions/uint32-array
> +              description:
> +                 Per channel dma hardware descriptor configuration.
> +                 The first parameter is descriptor physical address and the
> +                 second parameter hardware descriptor number.

Again, this all seems like per client information for dma cells.

> +
> +          required:
> +            - reg
> +
> +required:
> + - compatible
> + - reg
> + - '#dma-cells'

Add:

additionalProperties: false

> +
> +examples:
> + - |
> +   dma0: dma-controller@e0e00000 {
> +     compatible = "intel,lgm-cdma";
> +     reg = <0xe0e00000 0x1000>;
> +     #dma-cells = <3>;
> +     interrupt-parent = <&ioapic1>;
> +     interrupts = <82 1>;
> +     resets = <&rcu0 0x30 0>;
> +     reset-names = "ctrl";

Not documented.

> +     clocks = <&cgu0 80>;
> +     intel,dma-poll-cnt = <4>;
> +     intel,dma-byte-en;
> +     intel,dma-drb;
> +     dma-ports {
> +       #address-cells = <1>;
> +       #size-cells = <0>;
> +
> +       dma-ports@0 {
> +           reg = <0>;
> +           intel,name = "SPI0";
> +           intel,chans = <0 2>;
> +       };
> +       dma-ports@1 {
> +           reg = <1>;
> +           intel,name = "SPI1";
> +           intel,chans = <2 2>;
> +       };
> +       dma-ports@2 {
> +           reg = <2>;
> +           intel,name = "SPI2";
> +           intel,chans = <4 2>;
> +       };
> +       dma-ports@3 {
> +           reg = <3>;
> +           intel,name = "SPI3";
> +           intel,chans = <6 2>;
> +       };
> +       dma-ports@4 {
> +           reg = <4>;
> +           intel,name = "HSNAND";
> +           intel,chans = <8 2>;
> +       };
> +       dma-ports@5 {
> +           reg = <5>;
> +           intel,name = "PCM";
> +           intel,chans = <10 6>;
> +       };
> +     };
> +     ldma-channels {
> +       #address-cells = <1>;
> +       #size-cells = <0>;
> +
> +       ldma-channels@0 {
> +           reg = <0>;
> +           intel,desc_num = <1>;
> +       };
> +       ldma-channels@1 {
> +           reg = <1>;
> +           intel,desc_num = <1>;
> +       };
> +       ldma-channels@2 {
> +           reg = <2>;
> +           intel,desc_num = <1>;
> +       };
> +       ldma-channels@3 {
> +           reg = <3>;
> +           intel,desc_num = <1>;
> +       };
> +       ldma-channels@4 {
> +           reg = <4>;
> +           intel,desc_num = <1>;
> +       };
> +       ldma-channels@5 {
> +           reg = <5>;
> +           intel,desc_num = <1>;
> +       };
> +       ldma-channels@6 {
> +           reg = <6>;
> +           intel,desc_num = <1>;
> +       };
> +       ldma-channels@7 {
> +           reg = <7>;
> +           intel,desc_num = <1>;
> +       };
> +       ldma-channels@8 {
> +           reg = <8>;
> +       };
> +       ldma-channels@9 {
> +           reg = <9>;
> +       };
> +       ldma-channels@10 {
> +           reg = <10>;
> +       };
> +       ldma-channels@11 {
> +           reg = <11>;
> +       };
> +       ldma-channels@12 {
> +           reg = <12>;
> +       };
> +       ldma-channels@13 {
> +           reg = <13>;
> +       };
> +       ldma-channels@14 {
> +           reg = <14>;
> +       };
> +       ldma-channels@15 {
> +           reg = <15>;
> +       };
> +     };
> +   };
> + - |
> +   dma3: dma-controller@ec800000 {
> +     compatible = "intel,lgm-dma3";
> +     reg = <0xec800000 0x1000>;
> +     clocks = <&cgu0 71>;
> +     resets = <&rcu0 0x10 9>;
> +     #dma-cells = <7>;
> +     intel,dma-burst = <32>;
> +     intel,dma-polling-cnt = <16>;
> +     intel,dma-desc-in-sram;
> +     intel,dma-orrc = <16>;
> +     intel,dma-byte-en;
> +     intel,dma-dburst-wr;
> +     ldma-channels {
> +         #address-cells = <1>;
> +         #size-cells = <0>;
> +
> +         ldma-channels@12 {
> +             reg = <12>;
> +             intel,hdr-mode = <128 0>;
> +             intel,hw-desc = <0x20000000 8>;
> +         };
> +         ldma-channels@13 {
> +             reg = <13>;
> +             intel,hdr-mode = <128 0>;
> +         };
> +     };
> +   };
> -- 
> 2.11.0
>

Hi Rob,
Thanks for your valuable comments. Please see my comments inline..

On 8/15/2020 4:32 AM, Rob Herring wrote:
> On Fri, Aug 14, 2020 at 01:26:09PM +0800, 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.
>> ---
>>   .../devicetree/bindings/dma/intel,ldma.yaml        | 319 +++++++++++++++++++++
>>   1 file changed, 319 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..9beaf191a6de
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/dma/intel,ldma.yaml
>> @@ -0,0 +1,319 @@
>> +# 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:
>> + $nodename:
>> +   pattern: "^dma-controller(@.*)?$"
>> +
>> + "#dma-cells":
>> +   const: 1
> Example says 3.
OK, i will fix it.
>
>> +
>> + compatible:
>> +  anyOf:
>> +   - const: intel,lgm-cdma
>> +   - const: intel,lgm-dma2tx
>> +   - const: intel,lgm-dma1rx
>> +   - const: intel,lgm-dma1tx
>> +   - const: intel,lgm-dma0tx
>> +   - const: intel,lgm-dma3
>> +   - const: intel,lgm-toe-dma30
>> +   - const: intel,lgm-toe-dma31
> 'anyOf' doesn't make sense here. This should be a single 'enum'.
ok. I will update it.
>
>> +
>> + reg:
>> +  maxItems: 1
>> +
>> + clocks:
>> +  maxItems: 1
>> +
>> + resets:
>> +  maxItems: 1
>> +
>> + interrupts:
>> +  maxItems: 1
>> +
>> + intel,dma-poll-cnt:
>> +   $ref: /schemas/types.yaml#definitions/uint32
>> +   description:
>> +     DMA descriptor polling counter. It may need fine tune according
>> +     to the system application scenario.
>> +
>> + 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.
>> +
>> + intel,dma-drb:
>> +    type: boolean
>> +    description:
>> +      DMA descriptor read back to make sure data and desc synchronization.
>> +
>> + intel,dma-burst:
>> +    $ref: /schemas/types.yaml#definitions/uint32
>> +    description:
>> +       Specifiy the DMA burst size(in dwords), the valid value will be 8, 16, 32.
>> +       Default is 16 for data path dma, 32 is for memcopy DMA.
>> +
>> + intel,dma-polling-cnt:
>> +    $ref: /schemas/types.yaml#definitions/uint32
>> +    description:
>> +       DMA descriptor polling counter. It may need fine tune according to
>> +       the system application scenario.
>> +
>> + 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.
>> +
>> + intel,dma-orrc:
>> +    $ref: /schemas/types.yaml#definitions/uint32
>> +    description:
>> +       DMA outstanding read counter. The maximum value is 16, and it may
>> +       need fine tune according to the system application scenarios.
>> +
>> + intel,dma-dburst-wr:
>> +    type: boolean
>> +    description:
>> +       Enable RX dynamic burst write. It only applies to RX DMA and memcopy DMA.
>> +
>> +
>> + dma-ports:
>> +    type: object
>> +    description:
>> +       This sub-node must contain a sub-node for each DMA port.
>> +    properties:
>> +      '#address-cells':
>> +        const: 1
>> +      '#size-cells':
>> +        const: 0
>> +
>> +    patternProperties:
>> +      "^dma-ports@[0-9]+$":
>> +          type: object
>> +
>> +          properties:
>> +            reg:
>> +              items:
>> +                - enum: [0, 1, 2, 3, 4, 5]
>> +              description:
>> +                 Which port this node refers to.
>> +
>> +            intel,name:
>> +              $ref: /schemas/types.yaml#definitions/string-array
>> +              description:
>> +                 Port name of each DMA port.
> No other DMA controller needs this, why do you?
  Answered below. (*ABC)
>
>> +
>> +            intel,chans:
>> +              $ref: /schemas/types.yaml#/definitions/uint32-array
>> +              description:
>> +                 The channels included on this port. Format is channel start
>> +                 number and how many channels on this port.
> Why does this need to be in DT? This all seems like it can be in the dma
> cells for each client.
(*ABC)
Yes. We need this.
for dma0(lgm-cdma) old SOC supports 16 channels and the new SOC supports 
22 channels. and the logical channel mapping for the peripherals also 
differ b/w old and new SOCs.

Because of this hardware limitation we are trying to configure the total 
channels and port-channel mapping dynamically from device tree.

based on port name we are trying to configure the default values for 
different peripherals(ports).
Example: burst length is not same for all ports, so using port name to 
do default configurations.
>
>> +
>> +          required:
>> +            - reg
>> +            - intel,name
>> +            - intel,chans
>> +
>> +
>> + ldma-channels:
>> +    type: object
>> +    description:
>> +       This sub-node must contain a sub-node for each DMA channel.
>> +    properties:
>> +      '#address-cells':
>> +        const: 1
>> +      '#size-cells':
>> +        const: 0
>> +
>> +    patternProperties:
>> +      "^ldma-channels@[0-15]+$":
>> +          type: object
>> +
>> +          properties:
>> +            reg:
>> +              items:
>> +                - enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
>> +              description:
>> +                 Which channel this node refers to.
>> +
>> +            intel,desc_num:
>> +              $ref: /schemas/types.yaml#/definitions/uint32
>> +              description:
>> +                 Per channel maximum descriptor number. The max value is 255.
>> +
>> +            intel,hdr-mode:
>> +              $ref: /schemas/types.yaml#/definitions/uint32-array
>> +              description:
>> +                 The first parameter is header mode size, the second
>> +                 parameter is checksum enable or disable. If enabled,
>> +                 header mode size is ignored. If disabled, header mode
>> +                 size must be provided.
>> +
>> +            intel,hw-desc:
>> +              $ref: /schemas/types.yaml#/definitions/uint32-array
>> +              description:
>> +                 Per channel dma hardware descriptor configuration.
>> +                 The first parameter is descriptor physical address and the
>> +                 second parameter hardware descriptor number.
> Again, this all seems like per client information for dma cells.
  Ok, if we move all these attributes to 'dmas' then 'dma-channels' 
child node is not needed in dtsi.
#dma-cells number i am already using 7. If we move all these attributes 
to 'dmas' then integer cells will increase.

Is there any limitation in using a number of integer cells & as 
determined by the #dma-cells property?

>
>> +
>> +          required:
>> +            - reg
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - '#dma-cells'
> Add:
>
> additionalProperties: false
Sure i will update it in the next patch..

>
>> +
>> +examples:
>> + - |
>> +   dma0: dma-controller@e0e00000 {
>> +     compatible = "intel,lgm-cdma";
>> +     reg = <0xe0e00000 0x1000>;
>> +     #dma-cells = <3>;
>> +     interrupt-parent = <&ioapic1>;
>> +     interrupts = <82 1>;
>> +     resets = <&rcu0 0x30 0>;
>> +     reset-names = "ctrl";
> Not documented.
ok. will Document in the next patch.
>
>> +     clocks = <&cgu0 80>;
>> +     intel,dma-poll-cnt = <4>;
>> +     intel,dma-byte-en;
>> +     intel,dma-drb;
>> +     dma-ports {
>> +       #address-cells = <1>;
>> +       #size-cells = <0>;
>> +
>> +       dma-ports@0 {
>> +           reg = <0>;
>> +           intel,name = "SPI0";
>> +           intel,chans = <0 2>;
>> +       };
>> +       dma-ports@1 {
>> +           reg = <1>;
>> +           intel,name = "SPI1";
>> +           intel,chans = <2 2>;
>> +       };
>> +       dma-ports@2 {
>> +           reg = <2>;
>> +           intel,name = "SPI2";
>> +           intel,chans = <4 2>;
>> +       };
>> +       dma-ports@3 {
>> +           reg = <3>;
>> +           intel,name = "SPI3";
>> +           intel,chans = <6 2>;
>> +       };
>> +       dma-ports@4 {
>> +           reg = <4>;
>> +           intel,name = "HSNAND";
>> +           intel,chans = <8 2>;
>> +       };
>> +       dma-ports@5 {
>> +           reg = <5>;
>> +           intel,name = "PCM";
>> +           intel,chans = <10 6>;
>> +       };
>> +     };
>> +     ldma-channels {
>> +       #address-cells = <1>;
>> +       #size-cells = <0>;
>> +
>> +       ldma-channels@0 {
>> +           reg = <0>;
>> +           intel,desc_num = <1>;
>> +       };
>> +       ldma-channels@1 {
>> +           reg = <1>;
>> +           intel,desc_num = <1>;
>> +       };
>> +       ldma-channels@2 {
>> +           reg = <2>;
>> +           intel,desc_num = <1>;
>> +       };
>> +       ldma-channels@3 {
>> +           reg = <3>;
>> +           intel,desc_num = <1>;
>> +       };
>> +       ldma-channels@4 {
>> +           reg = <4>;
>> +           intel,desc_num = <1>;
>> +       };
>> +       ldma-channels@5 {
>> +           reg = <5>;
>> +           intel,desc_num = <1>;
>> +       };
>> +       ldma-channels@6 {
>> +           reg = <6>;
>> +           intel,desc_num = <1>;
>> +       };
>> +       ldma-channels@7 {
>> +           reg = <7>;
>> +           intel,desc_num = <1>;
>> +       };
>> +       ldma-channels@8 {
>> +           reg = <8>;
>> +       };
>> +       ldma-channels@9 {
>> +           reg = <9>;
>> +       };
>> +       ldma-channels@10 {
>> +           reg = <10>;
>> +       };
>> +       ldma-channels@11 {
>> +           reg = <11>;
>> +       };
>> +       ldma-channels@12 {
>> +           reg = <12>;
>> +       };
>> +       ldma-channels@13 {
>> +           reg = <13>;
>> +       };
>> +       ldma-channels@14 {
>> +           reg = <14>;
>> +       };
>> +       ldma-channels@15 {
>> +           reg = <15>;
>> +       };
>> +     };
>> +   };
>> + - |
>> +   dma3: dma-controller@ec800000 {
>> +     compatible = "intel,lgm-dma3";
>> +     reg = <0xec800000 0x1000>;
>> +     clocks = <&cgu0 71>;
>> +     resets = <&rcu0 0x10 9>;
>> +     #dma-cells = <7>;
>> +     intel,dma-burst = <32>;
>> +     intel,dma-polling-cnt = <16>;
>> +     intel,dma-desc-in-sram;
>> +     intel,dma-orrc = <16>;
>> +     intel,dma-byte-en;
>> +     intel,dma-dburst-wr;
>> +     ldma-channels {
>> +         #address-cells = <1>;
>> +         #size-cells = <0>;
>> +
>> +         ldma-channels@12 {
>> +             reg = <12>;
>> +             intel,hdr-mode = <128 0>;
>> +             intel,hw-desc = <0x20000000 8>;
>> +         };
>> +         ldma-channels@13 {
>> +             reg = <13>;
>> +             intel,hdr-mode = <128 0>;
>> +         };
>> +     };
>> +   };
>> -- 
>> 2.11.0
>>

On 18-08-20, 15:00, Reddy, MallikarjunaX wrote:

> > > +
> > > +            intel,chans:
> > > +              $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +              description:
> > > +                 The channels included on this port. Format is channel start
> > > +                 number and how many channels on this port.
> > Why does this need to be in DT? This all seems like it can be in the dma
> > cells for each client.
> (*ABC)
> Yes. We need this.
> for dma0(lgm-cdma) old SOC supports 16 channels and the new SOC supports 22
> channels. and the logical channel mapping for the peripherals also differ
> b/w old and new SOCs.
> 
> Because of this hardware limitation we are trying to configure the total
> channels and port-channel mapping dynamically from device tree.
> 
> based on port name we are trying to configure the default values for
> different peripherals(ports).
> Example: burst length is not same for all ports, so using port name to do
> default configurations.

Sorry that does not make sense to me, why not specify the values to be
used here instead of defining your own name scheme!

Only older soc it should create 16 channels and new 22 (hint this is hw
description so perfectly okay to specify in DT or in using driver_data
and compatible for each version

> > 
> > > +
> > > +          required:
> > > +            - reg
> > > +            - intel,name
> > > +            - intel,chans
> > > +
> > > +
> > > + ldma-channels:
> > > +    type: object
> > > +    description:
> > > +       This sub-node must contain a sub-node for each DMA channel.
> > > +    properties:
> > > +      '#address-cells':
> > > +        const: 1
> > > +      '#size-cells':
> > > +        const: 0
> > > +
> > > +    patternProperties:
> > > +      "^ldma-channels@[0-15]+$":
> > > +          type: object
> > > +
> > > +          properties:
> > > +            reg:
> > > +              items:
> > > +                - enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
> > > +              description:
> > > +                 Which channel this node refers to.
> > > +
> > > +            intel,desc_num:
> > > +              $ref: /schemas/types.yaml#/definitions/uint32
> > > +              description:
> > > +                 Per channel maximum descriptor number. The max value is 255.
> > > +
> > > +            intel,hdr-mode:
> > > +              $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +              description:
> > > +                 The first parameter is header mode size, the second
> > > +                 parameter is checksum enable or disable. If enabled,
> > > +                 header mode size is ignored. If disabled, header mode
> > > +                 size must be provided.
> > > +
> > > +            intel,hw-desc:
> > > +              $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +              description:
> > > +                 Per channel dma hardware descriptor configuration.
> > > +                 The first parameter is descriptor physical address and the
> > > +                 second parameter hardware descriptor number.
> > Again, this all seems like per client information for dma cells.
>  Ok, if we move all these attributes to 'dmas' then 'dma-channels' child
> node is not needed in dtsi.
> #dma-cells number i am already using 7. If we move all these attributes to
> 'dmas' then integer cells will increase.
> 
> Is there any limitation in using a number of integer cells & as determined
> by the #dma-cells property?

No I dont think there is but it needs to make sense :-)

Hi Vinod,
Thanks for the review comments.

On 8/25/2020 7:21 PM, Vinod Koul wrote:
> On 18-08-20, 15:00, Reddy, MallikarjunaX wrote:
>
>>>> +
>>>> +            intel,chans:
>>>> +              $ref: /schemas/types.yaml#/definitions/uint32-array
>>>> +              description:
>>>> +                 The channels included on this port. Format is channel start
>>>> +                 number and how many channels on this port.
>>> Why does this need to be in DT? This all seems like it can be in the dma
>>> cells for each client.
>> (*ABC)
>> Yes. We need this.
>> for dma0(lgm-cdma) old SOC supports 16 channels and the new SOC supports 22
>> channels. and the logical channel mapping for the peripherals also differ
>> b/w old and new SOCs.
>>
>> Because of this hardware limitation we are trying to configure the total
>> channels and port-channel mapping dynamically from device tree.
>>
>> based on port name we are trying to configure the default values for
>> different peripherals(ports).
>> Example: burst length is not same for all ports, so using port name to do
>> default configurations.
> Sorry that does not make sense to me, why not specify the values to be
> used here instead of defining your own name scheme!
OK. Agreed. I will remove port name from DT and only use intel,chans
>
> Only older soc it should create 16 channels and new 22 (hint this is hw
> description so perfectly okay to specify in DT or in using driver_data
> and compatible for each version
>
>>>> +
>>>> +          required:
>>>> +            - reg
>>>> +            - intel,name
>>>> +            - intel,chans
>>>> +
>>>> +
>>>> + ldma-channels:
>>>> +    type: object
>>>> +    description:
>>>> +       This sub-node must contain a sub-node for each DMA channel.
>>>> +    properties:
>>>> +      '#address-cells':
>>>> +        const: 1
>>>> +      '#size-cells':
>>>> +        const: 0
>>>> +
>>>> +    patternProperties:
>>>> +      "^ldma-channels@[0-15]+$":
>>>> +          type: object
>>>> +
>>>> +          properties:
>>>> +            reg:
>>>> +              items:
>>>> +                - enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
>>>> +              description:
>>>> +                 Which channel this node refers to.
>>>> +
>>>> +            intel,desc_num:
>>>> +              $ref: /schemas/types.yaml#/definitions/uint32
>>>> +              description:
>>>> +                 Per channel maximum descriptor number. The max value is 255.
>>>> +
>>>> +            intel,hdr-mode:
>>>> +              $ref: /schemas/types.yaml#/definitions/uint32-array
>>>> +              description:
>>>> +                 The first parameter is header mode size, the second
>>>> +                 parameter is checksum enable or disable. If enabled,
>>>> +                 header mode size is ignored. If disabled, header mode
>>>> +                 size must be provided.
>>>> +
>>>> +            intel,hw-desc:
>>>> +              $ref: /schemas/types.yaml#/definitions/uint32-array
>>>> +              description:
>>>> +                 Per channel dma hardware descriptor configuration.
>>>> +                 The first parameter is descriptor physical address and the
>>>> +                 second parameter hardware descriptor number.
>>> Again, this all seems like per client information for dma cells.
>>   Ok, if we move all these attributes to 'dmas' then 'dma-channels' child
>> node is not needed in dtsi.
>> #dma-cells number i am already using 7. If we move all these attributes to
>> 'dmas' then integer cells will increase.
>>
>> Is there any limitation in using a number of integer cells & as determined
>> by the #dma-cells property?
> No I dont think there is but it needs to make sense :-)
OK.
>

On 27-08-20, 17:54, Reddy, MallikarjunaX wrote:
> Hi Vinod,
> Thanks for the review comments.
> 
> On 8/25/2020 7:21 PM, Vinod Koul wrote:
> > On 18-08-20, 15:00, Reddy, MallikarjunaX wrote:
> > 
> > > > > +
> > > > > +            intel,chans:
> > > > > +              $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > > +              description:
> > > > > +                 The channels included on this port. Format is channel start
> > > > > +                 number and how many channels on this port.
> > > > Why does this need to be in DT? This all seems like it can be in the dma
> > > > cells for each client.
> > > (*ABC)
> > > Yes. We need this.
> > > for dma0(lgm-cdma) old SOC supports 16 channels and the new SOC supports 22
> > > channels. and the logical channel mapping for the peripherals also differ
> > > b/w old and new SOCs.
> > > 
> > > Because of this hardware limitation we are trying to configure the total
> > > channels and port-channel mapping dynamically from device tree.
> > > 
> > > based on port name we are trying to configure the default values for
> > > different peripherals(ports).
> > > Example: burst length is not same for all ports, so using port name to do
> > > default configurations.
> > Sorry that does not make sense to me, why not specify the values to be
> > used here instead of defining your own name scheme!
> OK. Agreed. I will remove port name from DT and only use intel,chans

what is intel,chans, why not use dma-channels?

Hi Vinod,

Thanks for the review. Please see my comment inline.

On 8/28/2020 6:45 PM, Vinod Koul wrote:
> On 27-08-20, 17:54, Reddy, MallikarjunaX wrote:
>> Hi Vinod,
>> Thanks for the review comments.
>>
>> On 8/25/2020 7:21 PM, Vinod Koul wrote:
>>> On 18-08-20, 15:00, Reddy, MallikarjunaX wrote:
>>>
>>>>>> +
>>>>>> +            intel,chans:
>>>>>> +              $ref: /schemas/types.yaml#/definitions/uint32-array
>>>>>> +              description:
>>>>>> +                 The channels included on this port. Format is channel start
>>>>>> +                 number and how many channels on this port.
>>>>> Why does this need to be in DT? This all seems like it can be in the dma
>>>>> cells for each client.
>>>> (*ABC)
>>>> Yes. We need this.
>>>> for dma0(lgm-cdma) old SOC supports 16 channels and the new SOC supports 22
>>>> channels. and the logical channel mapping for the peripherals also differ
>>>> b/w old and new SOCs.
>>>>
>>>> Because of this hardware limitation we are trying to configure the total
>>>> channels and port-channel mapping dynamically from device tree.
>>>>
>>>> based on port name we are trying to configure the default values for
>>>> different peripherals(ports).
>>>> Example: burst length is not same for all ports, so using port name to do
>>>> default configurations.
>>> Sorry that does not make sense to me, why not specify the values to be
>>> used here instead of defining your own name scheme!
>> OK. Agreed. I will remove port name from DT and only use intel,chans
> what is intel,chans, why not use dma-channels?
  The intel,chans says about the channels included on the correspondng 
port. Format is channel start number and how many channels on this port.
  The reasong behind using this attribute instead of standrad 
dma-channels is...


DMA_VER22 HW supports 22 channels. But there is a hole in HW, total it 
can use only 16.

Old soc supports 4ports and 16 channels.
New soc supports 6ports and 22 channels.
(old and new soc carry the same version VER22)

port channel mapping for the old and new soc also not the same.
old soc: logical channels:(Rx, Tx)
0, 1 - SPI0
2, 3 - SPI1
4, 5 - HSNAND
12, 14, 13, 15 - Memcopy

New soc: Logical channels(Rx, Tx)
0, 1 - SPI0
2, 3 - SPI1
4, 5 - SPI2
6, 7 - SPI3
8, 9 - HSNAND
10 to 21 - Mcopy

Because of these reasons we are trying to use "intel,chans" attribute, 
and reading then number of channels from the dt.
Advantaage:
1. we can map the channels correspondign to port
2. Dynamically configure the channels (due to hw limitation)

If this is not ok, please suggest us the better way to handle this.
>

On 31-08-20, 16:06, Reddy, MallikarjunaX wrote:
> Hi Vinod,
> 
> Thanks for the review. Please see my comment inline.
> 
> On 8/28/2020 6:45 PM, Vinod Koul wrote:
> > On 27-08-20, 17:54, Reddy, MallikarjunaX wrote:
> > > Hi Vinod,
> > > Thanks for the review comments.
> > > 
> > > On 8/25/2020 7:21 PM, Vinod Koul wrote:
> > > > On 18-08-20, 15:00, Reddy, MallikarjunaX wrote:
> > > > 
> > > > > > > +
> > > > > > > +            intel,chans:
> > > > > > > +              $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > > > > +              description:
> > > > > > > +                 The channels included on this port. Format is channel start
> > > > > > > +                 number and how many channels on this port.
> > > > > > Why does this need to be in DT? This all seems like it can be in the dma
> > > > > > cells for each client.
> > > > > (*ABC)
> > > > > Yes. We need this.
> > > > > for dma0(lgm-cdma) old SOC supports 16 channels and the new SOC supports 22
> > > > > channels. and the logical channel mapping for the peripherals also differ
> > > > > b/w old and new SOCs.
> > > > > 
> > > > > Because of this hardware limitation we are trying to configure the total
> > > > > channels and port-channel mapping dynamically from device tree.
> > > > > 
> > > > > based on port name we are trying to configure the default values for
> > > > > different peripherals(ports).
> > > > > Example: burst length is not same for all ports, so using port name to do
> > > > > default configurations.
> > > > Sorry that does not make sense to me, why not specify the values to be
> > > > used here instead of defining your own name scheme!
> > > OK. Agreed. I will remove port name from DT and only use intel,chans
> > what is intel,chans, why not use dma-channels?
>  The intel,chans says about the channels included on the correspondng port.

What do you mean by a port here?

> Format is channel start number and how many channels on this port.

It is perfectly reasonable to have 16 channels but linux not use use all, lets
say from 5th channel channel onwards

So you need to use standard dma-channels also with dma-channel-mask to
specify which channels linux can use

>  The reasong behind using this attribute instead of standrad dma-channels
> is...
> 
> 
> DMA_VER22 HW supports 22 channels. But there is a hole in HW, total it can
> use only 16.
> 
> Old soc supports 4ports and 16 channels.
> New soc supports 6ports and 22 channels.
> (old and new soc carry the same version VER22)
> 
> port channel mapping for the old and new soc also not the same.
> old soc: logical channels:(Rx, Tx)
> 0, 1 - SPI0
> 2, 3 - SPI1
> 4, 5 - HSNAND
> 12, 14, 13, 15 - Memcopy
> 
> New soc: Logical channels(Rx, Tx)
> 0, 1 - SPI0
> 2, 3 - SPI1
> 4, 5 - SPI2
> 6, 7 - SPI3
> 8, 9 - HSNAND
> 10 to 21 - Mcopy

Mapping is different, client can set that channel required in dmas
property and use a specific required channel.

> Because of these reasons we are trying to use "intel,chans" attribute, and
> reading then number of channels from the dt.
> Advantaage:
> 1. we can map the channels correspondign to port
> 2. Dynamically configure the channels (due to hw limitation)
> 
> If this is not ok, please suggest us the better way to handle this.
> >

Hi Vinod,
Thanks for the review, please see my comments inline.

On 8/31/2020 7:00 PM, Vinod Koul wrote:
> On 31-08-20, 16:06, Reddy, MallikarjunaX wrote:
>> Hi Vinod,
>>
>> Thanks for the review. Please see my comment inline.
>>
>> On 8/28/2020 6:45 PM, Vinod Koul wrote:
>>> On 27-08-20, 17:54, Reddy, MallikarjunaX wrote:
>>>> Hi Vinod,
>>>> Thanks for the review comments.
>>>>
>>>> On 8/25/2020 7:21 PM, Vinod Koul wrote:
>>>>> On 18-08-20, 15:00, Reddy, MallikarjunaX wrote:
>>>>>
>>>>>>>> +
>>>>>>>> +            intel,chans:
>>>>>>>> +              $ref: /schemas/types.yaml#/definitions/uint32-array
>>>>>>>> +              description:
>>>>>>>> +                 The channels included on this port. Format is channel start
>>>>>>>> +                 number and how many channels on this port.
>>>>>>> Why does this need to be in DT? This all seems like it can be in the dma
>>>>>>> cells for each client.
>>>>>> (*ABC)
>>>>>> Yes. We need this.
>>>>>> for dma0(lgm-cdma) old SOC supports 16 channels and the new SOC supports 22
>>>>>> channels. and the logical channel mapping for the peripherals also differ
>>>>>> b/w old and new SOCs.
>>>>>>
>>>>>> Because of this hardware limitation we are trying to configure the total
>>>>>> channels and port-channel mapping dynamically from device tree.
>>>>>>
>>>>>> based on port name we are trying to configure the default values for
>>>>>> different peripherals(ports).
>>>>>> Example: burst length is not same for all ports, so using port name to do
>>>>>> default configurations.
>>>>> Sorry that does not make sense to me, why not specify the values to be
>>>>> used here instead of defining your own name scheme!
>>>> OK. Agreed. I will remove port name from DT and only use intel,chans
>>> what is intel,chans, why not use dma-channels?
>>   The intel,chans says about the channels included on the correspondng port.
> What do you mean by a port here?
Here Port is nothing but Peripheral(SPI, HSNAND...)
>
>> Format is channel start number and how many channels on this port.
> It is perfectly reasonable to have 16 channels but linux not use use all, lets
> say from 5th channel channel onwards
>
> So you need to use standard dma-channels also with dma-channel-mask to
> specify which channels linux can use
Ok, let me verify and use the standard dma-channels also dma-channel-mask.
>
>>   The reasong behind using this attribute instead of standrad dma-channels
>> is...
>>
>>
>> DMA_VER22 HW supports 22 channels. But there is a hole in HW, total it can
>> use only 16.
>>
>> Old soc supports 4ports and 16 channels.
>> New soc supports 6ports and 22 channels.
>> (old and new soc carry the same version VER22)
>>
>> port channel mapping for the old and new soc also not the same.
>> old soc: logical channels:(Rx, Tx)
>> 0, 1 - SPI0
>> 2, 3 - SPI1
>> 4, 5 - HSNAND
>> 12, 14, 13, 15 - Memcopy
>>
>> New soc: Logical channels(Rx, Tx)
>> 0, 1 - SPI0
>> 2, 3 - SPI1
>> 4, 5 - SPI2
>> 6, 7 - SPI3
>> 8, 9 - HSNAND
>> 10 to 21 - Mcopy
> Mapping is different, client can set that channel required in dmas
> property and use a specific required channel.
OK.
>
>> Because of these reasons we are trying to use "intel,chans" attribute, and
>> reading then number of channels from the dt.
>> Advantaage:
>> 1. we can map the channels correspondign to port
>> 2. Dynamically configure the channels (due to hw limitation)
>>
>> If this is not ok, please suggest us the better way to handle this.

On 18/08/2020 10.00, Reddy, MallikarjunaX wrote:
> Hi Rob,
> Thanks for your valuable comments. Please see my comments inline..
> 
> On 8/15/2020 4:32 AM, Rob Herring wrote:
>> On Fri, Aug 14, 2020 at 01:26:09PM +0800, 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.
>>> ---
>>>   .../devicetree/bindings/dma/intel,ldma.yaml        | 319
>>> +++++++++++++++++++++
>>>   1 file changed, 319 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..9beaf191a6de
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/dma/intel,ldma.yaml
>>> @@ -0,0 +1,319 @@
>>> +# 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:
>>> + $nodename:
>>> +   pattern: "^dma-controller(@.*)?$"
>>> +
>>> + "#dma-cells":
>>> +   const: 1
>> Example says 3.
> OK, i will fix it.

It would help if you would add description of what is the meaning of the
individual cell.

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

On 9/4/2020 2:31 PM, Peter Ujfalusi wrote:
>
> On 18/08/2020 10.00, Reddy, MallikarjunaX wrote:
>> Hi Rob,
>> Thanks for your valuable comments. Please see my comments inline..
>>
>> On 8/15/2020 4:32 AM, Rob Herring wrote:
>>> On Fri, Aug 14, 2020 at 01:26:09PM +0800, 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.
>>>> ---
>>>>    .../devicetree/bindings/dma/intel,ldma.yaml        | 319
>>>> +++++++++++++++++++++
>>>>    1 file changed, 319 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..9beaf191a6de
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/dma/intel,ldma.yaml
>>>> @@ -0,0 +1,319 @@
>>>> +# 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:
>>>> + $nodename:
>>>> +   pattern: "^dma-controller(@.*)?$"
>>>> +
>>>> + "#dma-cells":
>>>> +   const: 1
>>> Example says 3.
>> OK, i will fix it.
> It would help if you would add description of what is the meaning of the
> individual cell.
I am already prepared the patch by addressing previous comments and just 
before sending i received your review comment. :-)

Let me Edit , include the description and prepare the patch again.
>
> - Péter
>
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>