[4/4] dt-bindings: memory: Add jedec,lpddrX-channel binding

This patch adds a new device tree binding for an LPDDR channel to serve
as a top-level organizing node for LPDDR part nodes nested below it. An
LPDDR channel needs to have an "io-width" property to describe its width
(this is important because this width does not always match the io-width
of the part number, indicating that multiple parts are wired in parallel
on the same channel), as well as one or more nested "rank@X" nodes.
Those represent information about the individual ranks of each LPDDR
part connected on that channel and should match the existing
"jedec,lpddrX" bindings for individual LPDDR parts.

New platforms should be using this node -- the existing practice of
providing a raw, toplevel "jedec,lpddrX" node without indication of how
many identical parts are in the system should be considered deprecated.

Signed-off-by: Julius Werner <jwerner@chromium.org>
---
 .../ddr/jedec,lpddr-channel.yaml              | 116 ++++++++++++++++++
 .../ddr/jedec,lpddr-props.yaml                |  10 +-
 2 files changed, 125 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-channel.yaml

On 31/08/2022 04:33, Julius Werner wrote:
> This patch adds a new device tree binding for an LPDDR channel to serve
> as a top-level organizing node for LPDDR part nodes nested below it. An
> LPDDR channel needs to have an "io-width" property to describe its width
> (this is important because this width does not always match the io-width
> of the part number, indicating that multiple parts are wired in parallel
> on the same channel), as well as one or more nested "rank@X" nodes.
> Those represent information about the individual ranks of each LPDDR
> part connected on that channel and should match the existing
> "jedec,lpddrX" bindings for individual LPDDR parts.
> 
> New platforms should be using this node -- the existing practice of
> providing a raw, toplevel "jedec,lpddrX" node without indication of how
> many identical parts are in the system should be considered deprecated.
> 
> Signed-off-by: Julius Werner <jwerner@chromium.org>
> ---
>  .../ddr/jedec,lpddr-channel.yaml              | 116 ++++++++++++++++++
>  .../ddr/jedec,lpddr-props.yaml                |  10 +-
>  2 files changed, 125 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-channel.yaml
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-channel.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-channel.yaml
> new file mode 100644
> index 00000000000000..517e770d8e7133
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-channel.yaml
> @@ -0,0 +1,116 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/memory-controllers/ddr/jedec,lpddr-channel.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LPDDR channel with chip/rank topology description
> +
> +description:
> +  An LPDDR channel is a completely independent set of LPDDR pins (DQ, CA, CS,
> +  CK, etc.) that connect one or more LPDDR chips to a host system. The main
> +  purpose of this node is to overall LPDDR topology of the system, including the
> +  amount of individual LPDDR chips and the ranks per chip.

"channel" in this context confuses me a bit, because usually everyone is
talking about DDR controller channels, not memory channels. I think this
actually maps to a DDR controller channel?

> +
> +maintainers:
> +  - Julius Werner <jwerner@chromium.org>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - jedec,lpddr2-channel
> +      - jedec,lpddr3-channel
> +      - jedec,lpddr4-channel
> +      - jedec,lpddr5-channel
> +
> +  io-width:
> +    description:
> +      The number of DQ pins in the channel. If this number is different
> +      from (a multiple of) the io-width of the LPDDR chip, that means that
> +      multiple instances of that type of chip are wired in parallel on this
> +      channel (with the channel's DQ pins split up between the different
> +      chips, and the CA, CS, etc. pins of the different chips all shorted
> +      together).  This means that the total physical memory controlled by a
> +      channel is equal to the sum of the densities of each rank on the
> +      connected LPDDR chip, times the io-width of the channel divided by
> +      the io-width of the LPDDR chip.
> +    enum:
> +      - 8
> +      - 16
> +      - 32
> +      - 64
> +      - 128
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +patternProperties:
> +  "^rank@[0-9]+$":
> +    type: object
> +    description:
> +      Each physical LPDDR chip may have one or more ranks. Ranks are
> +      internal but fully independent sub-units of the chip. Each LPDDR bus
> +      transaction on the channel targets exactly one rank, based on the
> +      state of the CS pins. Different ranks may have different densities and
> +      timing requirements.
> +    oneOf:
> +      - $ref: /schemas/memory-controllers/ddr/jedec,lpddr2.yaml#
> +      - $ref: /schemas/memory-controllers/ddr/jedec,lpddr3.yaml#
> +      - $ref: /schemas/memory-controllers/ddr/jedec,lpddr4.yaml#
> +      - $ref: /schemas/memory-controllers/ddr/jedec,lpddr5.yaml#

This should be rather chosen depending on top-level compatible. IOW, add
allOf where ref is chosen. Something like:
https://lore.kernel.org/linux-devicetree/20220828084341.112146-15-krzysztof.kozlowski@linaro.org/

Unless LPDDR3 memory can be put in LPDDR5 channel? But I think it cannot...

> +    required:
> +      - reg
> +
> +required:
> +  - compatible
> +  - io-width
> +  - "#address-cells"
> +  - "#size-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    lpddr-channel0 {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      compatible = "jedec,lpddr4-channel";
> +      io-width = <32>;
> +
> +      rank@0 {
> +        compatible = "lpddr4-ff,0100", "jedec,lpddr4";
> +        reg = <0>;
> +        density = <8192>;
> +        io-width = <16>;
> +        manufacturer-id = <255>;
> +        revision-id = <1 0>;
> +      };
> +    };
> +
> +    lpddr-channel1 {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      compatible = "jedec,lpddr4-channel";
> +      io-width = <32>;

I wonder now, how does it exactly work - channel is 32 bits, two ranks
each with 32 bit IO bus. Your description said that:

total_ram = (rank0 + rank1) * (channel_width / chip_width)
so for this case: (4+2)*(32/32) = 6 Mbit

If channel io-width = <64>, then memories are stacked in parallel and
according to your description total RAM would be: (4+2)*(64/32) = 12 Mbit
I wonder why stacking memories in parallel increases their size?

> +
> +      rank@0 {
> +        compatible = "lpddr4-05,0301", "jedec,lpddr4";
> +        reg = <0>;
> +        density = <4096>;
> +        io-width = <32>;
> +        manufacturer-id = <5>;
> +        revision-id = <3 1>;
> +      };
> +
> +      rank@1 {
> +        compatible = "lpddr4-05,0301", "jedec,lpddr4";
> +        reg = <1>;
> +        density = <2048>;
> +        io-width = <32>;
> +        manufacturer-id = <5>;
> +        revision-id = <3 1>;
> +      };
> +    };
> diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml
> index e1182e75ca1a3f..53a4836028cd25 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml
> @@ -9,7 +9,8 @@ title: Common properties for LPDDR types
>  description:
>    Different LPDDR types generally use the same properties and only differ in the
>    range of legal values for each. This file defines the common parts that can be
> -  reused for each type.
> +  reused for each type. Nodes using this schema should generally be nested under
> +  an LPDDR channel node.
>  
>  maintainers:
>    - Krzysztof Kozlowski <krzk@kernel.org>
> @@ -71,4 +72,11 @@ properties:
>        - 16
>        - 8
>  
> +  reg:
> +    description:
> +      The rank number of this LPDDR rank when used as a subnode to an LPDDR
> +      channel.
> +    minimum: 0
> +    maximum: 3

Put reg just after compatible.

> +
>  additionalProperties: true


Best regards,
Krzysztof

> > +description:
> > +  An LPDDR channel is a completely independent set of LPDDR pins (DQ, CA, CS,
> > +  CK, etc.) that connect one or more LPDDR chips to a host system. The main
> > +  purpose of this node is to overall LPDDR topology of the system, including the
> > +  amount of individual LPDDR chips and the ranks per chip.
>
> "channel" in this context confuses me a bit, because usually everyone is
> talking about DDR controller channels, not memory channels. I think this
> actually maps to a DDR controller channel?

I'm not really sure what you mean by "memory channel" here (that would
be different from the DDR controller channel)? According to my
understanding there's only one kind of "channel" in the context of
main memory, that's the DDR controller channel (i.e. each separate
complete set of DDR pins coming out of the controller, as I tried to
explain in the description).

> > +    lpddr-channel1 {
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +      compatible = "jedec,lpddr4-channel";
> > +      io-width = <32>;
>
> I wonder now, how does it exactly work - channel is 32 bits, two ranks
> each with 32 bit IO bus. Your description said that:
>
> total_ram = (rank0 + rank1) * (channel_width / chip_width)
> so for this case: (4+2)*(32/32) = 6 Mbit
>
> If channel io-width = <64>, then memories are stacked in parallel and
> according to your description total RAM would be: (4+2)*(64/32) = 12 Mbit
> I wonder why stacking memories in parallel increases their size?

Well, stacking in parallel just means you have more of them? In the
original example, you have a single LPDDR chip with two ranks, one
4Gbit rank and one 2Gbit rank. That chip is directly hooked up to the
LPDDR controller and that's the only chip you have, so you have 4+2 =
6Gbit total memory in the system.

In your next example, the LPDDR controller has a 64 bit wide channel,
but you're still using that same 6Gbit LPDDR chip that only has 32 DQ
pins. The only way to fill out that 64 bit channel with this kind of
chip is to have two of them in parallel (one connected to DQ[0:31] and
one connected to DQ[32:63]). So we infer from the mismatch in io-width
that we have two chips. Each chip still has 6Gbit of memory, so the
total system would have 12Gbit.

On 01/09/2022 03:11, Julius Werner wrote:
>>> +description:
>>> +  An LPDDR channel is a completely independent set of LPDDR pins (DQ, CA, CS,
>>> +  CK, etc.) that connect one or more LPDDR chips to a host system. The main
>>> +  purpose of this node is to overall LPDDR topology of the system, including the
>>> +  amount of individual LPDDR chips and the ranks per chip.
>>
>> "channel" in this context confuses me a bit, because usually everyone is
>> talking about DDR controller channels, not memory channels. I think this
>> actually maps to a DDR controller channel?
> 
> I'm not really sure what you mean by "memory channel" here (that would
> be different from the DDR controller channel)? According to my
> understanding there's only one kind of "channel" in the context of
> main memory, that's the DDR controller channel (i.e. each separate
> complete set of DDR pins coming out of the controller, as I tried to
> explain in the description).
> 
>>> +    lpddr-channel1 {
>>> +      #address-cells = <1>;
>>> +      #size-cells = <0>;
>>> +      compatible = "jedec,lpddr4-channel";
>>> +      io-width = <32>;
>>
>> I wonder now, how does it exactly work - channel is 32 bits, two ranks
>> each with 32 bit IO bus. Your description said that:
>>
>> total_ram = (rank0 + rank1) * (channel_width / chip_width)
>> so for this case: (4+2)*(32/32) = 6 Mbit
>>
>> If channel io-width = <64>, then memories are stacked in parallel and
>> according to your description total RAM would be: (4+2)*(64/32) = 12 Mbit
>> I wonder why stacking memories in parallel increases their size?
> 
> Well, stacking in parallel just means you have more of them? In the
> original example, you have a single LPDDR chip with two ranks, one
> 4Gbit rank and one 2Gbit rank. That chip is directly hooked up to the
> LPDDR controller and that's the only chip you have, so you have 4+2 =
> 6Gbit total memory in the system.
> 
> In your next example, the LPDDR controller has a 64 bit wide channel,
> but you're still using that same 6Gbit LPDDR chip that only has 32 DQ
> pins. The only way to fill out that 64 bit channel with this kind of
> chip is to have two of them in parallel (one connected to DQ[0:31] and
> one connected to DQ[32:63]). So we infer from the mismatch in io-width
> that we have two chips. Each chip still has 6Gbit of memory, so the
> total system would have 12Gbit.

Two chips so more device nodes? Since there are no DTSes with it, please
provide an additional example in the bindings.

Best regards,
Krzysztof

> > Well, stacking in parallel just means you have more of them? In the
> > original example, you have a single LPDDR chip with two ranks, one
> > 4Gbit rank and one 2Gbit rank. That chip is directly hooked up to the
> > LPDDR controller and that's the only chip you have, so you have 4+2 =
> > 6Gbit total memory in the system.
> >
> > In your next example, the LPDDR controller has a 64 bit wide channel,
> > but you're still using that same 6Gbit LPDDR chip that only has 32 DQ
> > pins. The only way to fill out that 64 bit channel with this kind of
> > chip is to have two of them in parallel (one connected to DQ[0:31] and
> > one connected to DQ[32:63]). So we infer from the mismatch in io-width
> > that we have two chips. Each chip still has 6Gbit of memory, so the
> > total system would have 12Gbit.
>
> Two chips so more device nodes? Since there are no DTSes with it, please
> provide an additional example in the bindings.

No, there isn't a separate node for each chip in this case. There's
still only one node (per rank), but if the io-width of the rank node
is smaller than the io-width of the channel node, that implicitly
indicates that there are in fact multiple chips of the same type wired
in parallel on that channel. I tried to explain this in the
description for the channel's io-width property.

I chose to model it this way because having separate nodes for each
chip would be redundant since all their properties have to be equal
anyway, and because it more closely resembles the way this looks to
the firmware and the DDR controller. The DDR controller doesn't
actually "see" that there are multiple separate chips and cannot
enumerate them as individual entities, because only the DQ pins are
split among the different chips -- all other pins like chip select and
column address are shorted together between all the parallel chips,
and mode register values are only returned through the lowest DQ pins
(DQ[7:0]). So it's impossible for the DDR controller to read mode
register values from the other chips, it can only read them from the
first chip and it must trust that all the other chips are the exact
same part number, because that's the only valid way to wire this (and
when the controller writes timing configuration to the mode registers,
the same value is written out to all chips at once via the shorted
column address pins).

My example does contain this case already, in lpddr-channel0, rank@0:
there's only one rank node with density 8Gbits, but since that node
has io-width 16 and the channel has io-width 32, it is implied that
there are actually two single-rank chips wired in parallel on this
channel, and since each of them have 8Gbits of memory the channel has
16Gbits in total.