| Message ID | 20240430-loongson1-nand-v7-1-60787c314fa4@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Add support for Loongson-1 NAND | expand |
Hello, devnull+keguang.zhang.gmail.com@kernel.org wrote on Tue, 30 Apr 2024 19:11:10 +0800: > From: Keguang Zhang <keguang.zhang@gmail.com> > > Add devicetree binding document for Loongson-1 NAND Controller. > > Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com> > --- > Changes in v7: > - rename the file to loongson,ls1b-nfc.yaml > > Changes in v6: > - A newly added patch > --- > .../devicetree/bindings/mtd/loongson,ls1b-nfc.yaml | 66 ++++++++++++++++++++++ > 1 file changed, 66 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mtd/loongson,ls1b-nfc.yaml b/Documentation/devicetree/bindings/mtd/loongson,ls1b-nfc.yaml > new file mode 100644 > index 000000000000..a69f22b9fd9e > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/loongson,ls1b-nfc.yaml > @@ -0,0 +1,66 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mtd/loongson,ls1b-nfc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Loongson-1 NAND Controller > + > +maintainers: > + - Keguang Zhang <keguang.zhang@gmail.com> > + > +allOf: > + - $ref: nand-controller.yaml > + > +properties: > + compatible: > + oneOf: > + - const: loongson,ls1b-nfc What is the rationale behind this choice? Seems like the b variant has two possible implementations and should always be preceded by a more specific compatible. As there is currently no description of this controller upstream, I would not care too much about any out-of-tree description and directly go for a clean description. > + - items: > + - enum: > + - loongson,ls1a-nfc > + - loongson,ls1c-nfc > + - const: loongson,ls1b-nfc > + > + reg: > + maxItems: 1 > + > + dmas: > + maxItems: 1 > + > + dma-names: > + const: rxtx > + > +patternProperties: > + "^nand@[0-3]$": > + type: object > + $ref: raw-nand-chip.yaml > + > + unevaluatedProperties: false > + > +required: > + - compatible > + - reg > + - dmas > + - dma-names Should DMA props be required? > + > +unevaluatedProperties: false > + > +examples: > + - | > + nand-controller@1fe78000 { > + compatible = "loongson,ls1b-nfc"; > + reg = <0x1fe78000 0x40>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + dmas = <&dma 0>; > + dma-names = "rxtx"; There is a preferred spacing for DT nodes, see: https://docs.kernel.org/devicetree/bindings/dts-coding-style.html > + > + nand@0 { > + reg = <0>; > + nand-use-soft-ecc-engine; > + nand-ecc-algo = "hamming"; These two properties are not needed. Unless there is no hardware ECC capability on this controller and in this case you need to ensure the properties are present in the schema. > + }; > + }; > Thanks, Miquèl
On Mon, May 6, 2024 at 3:14 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hello, > > devnull+keguang.zhang.gmail.com@kernel.org wrote on Tue, 30 Apr 2024 > 19:11:10 +0800: > > > From: Keguang Zhang <keguang.zhang@gmail.com> > > > > Add devicetree binding document for Loongson-1 NAND Controller. > > > > Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com> > > --- > > Changes in v7: > > - rename the file to loongson,ls1b-nfc.yaml > > > > Changes in v6: > > - A newly added patch > > --- > > .../devicetree/bindings/mtd/loongson,ls1b-nfc.yaml | 66 ++++++++++++++++++++++ > > 1 file changed, 66 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/mtd/loongson,ls1b-nfc.yaml b/Documentation/devicetree/bindings/mtd/loongson,ls1b-nfc.yaml > > new file mode 100644 > > index 000000000000..a69f22b9fd9e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mtd/loongson,ls1b-nfc.yaml > > @@ -0,0 +1,66 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/mtd/loongson,ls1b-nfc.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Loongson-1 NAND Controller > > + > > +maintainers: > > + - Keguang Zhang <keguang.zhang@gmail.com> > > + > > +allOf: > > + - $ref: nand-controller.yaml > > + > > +properties: > > + compatible: > > + oneOf: > > + - const: loongson,ls1b-nfc > > What is the rationale behind this choice? Seems like the b variant has > two possible implementations and should always be preceded by a more > specific compatible. > > As there is currently no description of this controller upstream, I > would not care too much about any out-of-tree description and directly > go for a clean description. > Excuse me, should I add a description for this property? > > + - items: > > + - enum: > > + - loongson,ls1a-nfc > > + - loongson,ls1c-nfc > > + - const: loongson,ls1b-nfc > > + > > + reg: > > + maxItems: 1 > > + > > + dmas: > > + maxItems: 1 > > + > > + dma-names: > > + const: rxtx > > + > > +patternProperties: > > + "^nand@[0-3]$": > > + type: object > > + $ref: raw-nand-chip.yaml > > + > > + unevaluatedProperties: false > > + > > +required: > > + - compatible > > + - reg > > + - dmas > > + - dma-names > > Should DMA props be required? > Yes. This NAND controller only works with DMA, which means the DMA is necessary. > > + > > +unevaluatedProperties: false > > + > > +examples: > > + - | > > + nand-controller@1fe78000 { > > + compatible = "loongson,ls1b-nfc"; > > + reg = <0x1fe78000 0x40>; > > + > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + dmas = <&dma 0>; > > + dma-names = "rxtx"; > > There is a preferred spacing for DT nodes, see: > https://docs.kernel.org/devicetree/bindings/dts-coding-style.html > Sorry. I don't get the meaning of preferred spacing. https://docs.kernel.org/devicetree/bindings/writing-schema.html says "For DTS examples in the schema, preferred is four-space indentation." Then I used four-space indentation. > > + > > + nand@0 { > > + reg = <0>; > > + nand-use-soft-ecc-engine; > > + nand-ecc-algo = "hamming"; > > These two properties are not needed. Unless there is no hardware ECC > capability on this controller and in this case you need to ensure the > properties are present in the schema. Exactly. This NAND controller doesn't support hardware ECC. 'nand-use-soft-ecc-engine' and 'nand-ecc-algo' are present in nand-chip.yaml. Is there anything else I should do? Thanks for your view! > > > + }; > > + }; > > > > > Thanks, > Miquèl
Hi, keguang.zhang@gmail.com wrote on Sat, 18 May 2024 16:01:01 +0800: > On Mon, May 6, 2024 at 3:14 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > Hello, > > > > devnull+keguang.zhang.gmail.com@kernel.org wrote on Tue, 30 Apr 2024 > > 19:11:10 +0800: > > > > > From: Keguang Zhang <keguang.zhang@gmail.com> > > > > > > Add devicetree binding document for Loongson-1 NAND Controller. > > > > > > Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com> > > > --- > > > Changes in v7: > > > - rename the file to loongson,ls1b-nfc.yaml > > > > > > Changes in v6: > > > - A newly added patch > > > --- > > > .../devicetree/bindings/mtd/loongson,ls1b-nfc.yaml | 66 ++++++++++++++++++++++ > > > 1 file changed, 66 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/mtd/loongson,ls1b-nfc.yaml b/Documentation/devicetree/bindings/mtd/loongson,ls1b-nfc.yaml > > > new file mode 100644 > > > index 000000000000..a69f22b9fd9e > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/mtd/loongson,ls1b-nfc.yaml > > > @@ -0,0 +1,66 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/mtd/loongson,ls1b-nfc.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Loongson-1 NAND Controller > > > + > > > +maintainers: > > > + - Keguang Zhang <keguang.zhang@gmail.com> > > > + > > > +allOf: > > > + - $ref: nand-controller.yaml > > > + > > > +properties: > > > + compatible: > > > + oneOf: > > > + - const: loongson,ls1b-nfc > > > > What is the rationale behind this choice? Seems like the b variant has > > two possible implementations and should always be preceded by a more > > specific compatible. > > > > As there is currently no description of this controller upstream, I > > would not care too much about any out-of-tree description and directly > > go for a clean description. > > > Excuse me, should I add a description for this property? No, description is not needed. But you are allowing the "loongson,ls1b-nfc" compatible alone, which I think is not relevant, unless you convince me it is :-) > > > + - items: > > > + - enum: > > > + - loongson,ls1a-nfc > > > + - loongson,ls1c-nfc > > > + - const: loongson,ls1b-nfc > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + dmas: > > > + maxItems: 1 > > > + > > > + dma-names: > > > + const: rxtx > > > + > > > +patternProperties: > > > + "^nand@[0-3]$": > > > + type: object > > > + $ref: raw-nand-chip.yaml > > > + > > > + unevaluatedProperties: false > > > + > > > +required: > > > + - compatible > > > + - reg > > > + - dmas > > > + - dma-names > > > > Should DMA props be required? > > > Yes. This NAND controller only works with DMA, which means the DMA is necessary. Ok > > > > + > > > +unevaluatedProperties: false > > > + > > > +examples: > > > + - | > > > + nand-controller@1fe78000 { > > > + compatible = "loongson,ls1b-nfc"; > > > + reg = <0x1fe78000 0x40>; > > > + > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + dmas = <&dma 0>; > > > + dma-names = "rxtx"; > > > > There is a preferred spacing for DT nodes, see: > > https://docs.kernel.org/devicetree/bindings/dts-coding-style.html > > > Sorry. I don't get the meaning of preferred spacing. > https://docs.kernel.org/devicetree/bindings/writing-schema.html says > "For DTS examples in the schema, preferred is four-space indentation." > Then I used four-space indentation. I'm talking about the new lines (\n) between the properties. > > > > + > > > + nand@0 { > > > + reg = <0>; > > > + nand-use-soft-ecc-engine; > > > + nand-ecc-algo = "hamming"; > > > > These two properties are not needed. Unless there is no hardware ECC > > capability on this controller and in this case you need to ensure the > > properties are present in the schema. > > Exactly. This NAND controller doesn't support hardware ECC. > 'nand-use-soft-ecc-engine' and 'nand-ecc-algo' are present in nand-chip.yaml. > Is there anything else I should do? Thry are in nand-chip.yaml, which means they are allowed, but I want them mandatory: required: - foo - bar Thanks, Miquèl
Hi Miquel, Sorry for the late reply. On Sat, May 18, 2024 at 6:47 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi, > > keguang.zhang@gmail.com wrote on Sat, 18 May 2024 16:01:01 +0800: > > > On Mon, May 6, 2024 at 3:14 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > Hello, > > > > > > devnull+keguang.zhang.gmail.com@kernel.org wrote on Tue, 30 Apr 2024 > > > 19:11:10 +0800: > > > > > > > From: Keguang Zhang <keguang.zhang@gmail.com> > > > > > > > > Add devicetree binding document for Loongson-1 NAND Controller. > > > > > > > > Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com> > > > > --- > > > > Changes in v7: > > > > - rename the file to loongson,ls1b-nfc.yaml > > > > > > > > Changes in v6: > > > > - A newly added patch > > > > --- > > > > .../devicetree/bindings/mtd/loongson,ls1b-nfc.yaml | 66 ++++++++++++++++++++++ > > > > 1 file changed, 66 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/mtd/loongson,ls1b-nfc.yaml b/Documentation/devicetree/bindings/mtd/loongson,ls1b-nfc.yaml > > > > new file mode 100644 > > > > index 000000000000..a69f22b9fd9e > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/mtd/loongson,ls1b-nfc.yaml > > > > @@ -0,0 +1,66 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/mtd/loongson,ls1b-nfc.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Loongson-1 NAND Controller > > > > + > > > > +maintainers: > > > > + - Keguang Zhang <keguang.zhang@gmail.com> > > > > + > > > > +allOf: > > > > + - $ref: nand-controller.yaml > > > > + > > > > +properties: > > > > + compatible: > > > > + oneOf: > > > > + - const: loongson,ls1b-nfc > > > > > > What is the rationale behind this choice? Seems like the b variant has > > > two possible implementations and should always be preceded by a more > > > specific compatible. > > > > > > As there is currently no description of this controller upstream, I > > > would not care too much about any out-of-tree description and directly > > > go for a clean description. > > > > > Excuse me, should I add a description for this property? > > No, description is not needed. But you are allowing the > "loongson,ls1b-nfc" compatible alone, which I think is not relevant, > unless you convince me it is :-) > "loongson,ls1b-nfc" itself is a specific implementation. I was suggested to set up a fallback before. https://lore.kernel.org/all/20231007-untapped-masses-01f80b7c13c7@spud/ Then "loongson,ls1b-nfc" became the fallback. > > > > + - items: > > > > + - enum: > > > > + - loongson,ls1a-nfc > > > > + - loongson,ls1c-nfc > > > > + - const: loongson,ls1b-nfc > > > > + > > > > + reg: > > > > + maxItems: 1 > > > > + > > > > + dmas: > > > > + maxItems: 1 > > > > + > > > > + dma-names: > > > > + const: rxtx > > > > + > > > > +patternProperties: > > > > + "^nand@[0-3]$": > > > > + type: object > > > > + $ref: raw-nand-chip.yaml > > > > + > > > > + unevaluatedProperties: false > > > > + > > > > +required: > > > > + - compatible > > > > + - reg > > > > + - dmas > > > > + - dma-names > > > > > > Should DMA props be required? > > > > > Yes. This NAND controller only works with DMA, which means the DMA is necessary. > > Ok > > > > > > > + > > > > +unevaluatedProperties: false > > > > + > > > > +examples: > > > > + - | > > > > + nand-controller@1fe78000 { > > > > + compatible = "loongson,ls1b-nfc"; > > > > + reg = <0x1fe78000 0x40>; > > > > + > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + > > > > + dmas = <&dma 0>; > > > > + dma-names = "rxtx"; > > > > > > There is a preferred spacing for DT nodes, see: > > > https://docs.kernel.org/devicetree/bindings/dts-coding-style.html > > > > > Sorry. I don't get the meaning of preferred spacing. > > https://docs.kernel.org/devicetree/bindings/writing-schema.html says > > "For DTS examples in the schema, preferred is four-space indentation." > > Then I used four-space indentation. > > I'm talking about the new lines (\n) between the properties. > Will remove the superfluous new lines. > > > > > > + > > > > + nand@0 { > > > > + reg = <0>; > > > > + nand-use-soft-ecc-engine; > > > > + nand-ecc-algo = "hamming"; > > > > > > These two properties are not needed. Unless there is no hardware ECC > > > capability on this controller and in this case you need to ensure the > > > properties are present in the schema. > > > > Exactly. This NAND controller doesn't support hardware ECC. > > 'nand-use-soft-ecc-engine' and 'nand-ecc-algo' are present in nand-chip.yaml. > > Is there anything else I should do? > > Thry are in nand-chip.yaml, which means they are allowed, but I want > them mandatory: > Will do. > required: > - foo > - bar > > Thanks, > Miquèl
Hi, > > > > > +properties: > > > > > + compatible: > > > > > + oneOf: > > > > > + - const: loongson,ls1b-nfc > > > > > > > > What is the rationale behind this choice? Seems like the b variant has > > > > two possible implementations and should always be preceded by a more > > > > specific compatible. > > > > > > > > As there is currently no description of this controller upstream, I > > > > would not care too much about any out-of-tree description and directly > > > > go for a clean description. > > > > > > > Excuse me, should I add a description for this property? > > > > No, description is not needed. But you are allowing the > > "loongson,ls1b-nfc" compatible alone, which I think is not relevant, > > unless you convince me it is :-) > > > "loongson,ls1b-nfc" itself is a specific implementation. > I was suggested to set up a fallback before. > https://lore.kernel.org/all/20231007-untapped-masses-01f80b7c13c7@spud/ > Then "loongson,ls1b-nfc" became the fallback. You cannot allow 'the fallback', alone. But this is what you do above. Below description is fine. Just don't allow the ls1b-nfc compatible alone. Thanks, Miquèl
On Fri, Jun 7, 2024 at 5:33 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi, > > > > > > > +properties: > > > > > > + compatible: > > > > > > + oneOf: > > > > > > + - const: loongson,ls1b-nfc > > > > > > > > > > What is the rationale behind this choice? Seems like the b variant has > > > > > two possible implementations and should always be preceded by a more > > > > > specific compatible. > > > > > > > > > > As there is currently no description of this controller upstream, I > > > > > would not care too much about any out-of-tree description and directly > > > > > go for a clean description. > > > > > > > > > Excuse me, should I add a description for this property? > > > > > > No, description is not needed. But you are allowing the > > > "loongson,ls1b-nfc" compatible alone, which I think is not relevant, > > > unless you convince me it is :-) > > > > > "loongson,ls1b-nfc" itself is a specific implementation. > > I was suggested to set up a fallback before. > > https://lore.kernel.org/all/20231007-untapped-masses-01f80b7c13c7@spud/ > > Then "loongson,ls1b-nfc" became the fallback. > > You cannot allow 'the fallback', alone. But this is what you do above. > Below description is fine. Just don't allow the ls1b-nfc compatible > alone. > Sorry. I still don't get this. According to https://lore.kernel.org/all/20221212163532.142533-2-krzysztof.kozlowski@linaro.org/, the generic fallback should not be used alone. In contrast, the specific fallback has to be used alone, as I understand it. If not, could you please show me the right way? Thanks very much! > Thanks, > Miquèl
diff --git a/Documentation/devicetree/bindings/mtd/loongson,ls1b-nfc.yaml b/Documentation/devicetree/bindings/mtd/loongson,ls1b-nfc.yaml new file mode 100644 index 000000000000..a69f22b9fd9e --- /dev/null +++ b/Documentation/devicetree/bindings/mtd/loongson,ls1b-nfc.yaml @@ -0,0 +1,66 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mtd/loongson,ls1b-nfc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Loongson-1 NAND Controller + +maintainers: + - Keguang Zhang <keguang.zhang@gmail.com> + +allOf: + - $ref: nand-controller.yaml + +properties: + compatible: + oneOf: + - const: loongson,ls1b-nfc + - items: + - enum: + - loongson,ls1a-nfc + - loongson,ls1c-nfc + - const: loongson,ls1b-nfc + + reg: + maxItems: 1 + + dmas: + maxItems: 1 + + dma-names: + const: rxtx + +patternProperties: + "^nand@[0-3]$": + type: object + $ref: raw-nand-chip.yaml + + unevaluatedProperties: false + +required: + - compatible + - reg + - dmas + - dma-names + +unevaluatedProperties: false + +examples: + - | + nand-controller@1fe78000 { + compatible = "loongson,ls1b-nfc"; + reg = <0x1fe78000 0x40>; + + #address-cells = <1>; + #size-cells = <0>; + + dmas = <&dma 0>; + dma-names = "rxtx"; + + nand@0 { + reg = <0>; + nand-use-soft-ecc-engine; + nand-ecc-algo = "hamming"; + }; + };