Hello Miquel! Thank you for looking into it. On Tue, 2023-05-02 at 11:48 +0200, Miquel Raynal wrote: > Hi Nikita, > > nikita.shubin@xxxxxxxxxxx wrote on Mon, 24 Apr 2023 15:34:38 +0300: > > > Add YAML bindings for ts7250 NAND. > > > > Signed-off-by: Nikita Shubin <nikita.shubin@xxxxxxxxxxx> > > --- > > .../bindings/mtd/technologic,nand.yaml | 56 > > +++++++++++++++++++ > > 1 file changed, 56 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/mtd/technologic,nand.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/mtd/technologic,nand.yaml > > b/Documentation/devicetree/bindings/mtd/technologic,nand.yaml > > new file mode 100644 > > index 000000000000..3234d93a1c21 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mtd/technologic,nand.yaml > > @@ -0,0 +1,56 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/mtd/technologic,nand.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Technologic Systems NAND controller > > + > > +maintainers: > > + - Lukasz Majewski <lukma@xxxxxxx> > > + > > +properties: > > + compatible: > > + items: > > + - const: technologic,ts7200-nand > > would -nand-controller instead of -nand work as a suffix here? > > You mention ts7250 in the title, should we have a more specific > compatible than ts7200 as well? > > I see by looking at the mtd patch that you actually try to match > both, > so they should both be defined in the bindings. > > > + - const: gen_nand > > This is a old hack for very simple controllers (converted to DT > probing > 12 years ago). The logic used by this driver has been deprecated for > like 10 years and does not really apply to modern APIs. I would > really > like to keep this driver contained with platform data coming from > arch/ > data only. > > I suggest you create a real NAND controller driver based on the > generic one (should not be very complex, just duplicate the code so > the > migration to the up-to-date API is eased) and you flag it as "must be > updated to ->exec_op() somehow. This way if someone starts the > conversion, it does not need to cope with the 5 other users of the > generic driver which anyway share nothing in common besides the > deprecated ->cmd_ctrl() backbone. > > I read the comments on the cover letter, people are kind of pushing > on > having this merged quickly. I am fine accepting a legacy controller > driver and migrating it to ->exec_op() later, but the current driver > conversion does not fit the approach taken years ago towards a > cleaner > mtd tree. Did you mean that i should at least implement legacy nand controller, like, for example, Xway (xway_nand.c) ?: data->chip.legacy.cmd_ctrl = xway_cmd_ctrl; data->chip.legacy.dev_ready = xway_dev_ready; data->chip.legacy.select_chip = xway_select_chip; data->chip.legacy.write_buf = xway_write_buf; data->chip.legacy.read_buf = xway_read_buf; data->chip.legacy.read_byte = xway_read_byte; data->chip.legacy.chip_delay = 30; And the best solution would be switching to exec_op completely ? > > > + > > + reg: > > + maxItems: 1 > > + > > + '#address-cells': true > > + '#size-cells': true > > + > > +required: > > + - compatible > > + - reg > > + > > +unevaluatedProperties: true > > + > > +examples: > > + - | > > + nand-parts@0 { > > + compatible = "technologic,ts7200-nand", "gen_nand"; > > + reg = <0x60000000 0x8000000>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + partition@0 { > > + label = "TS-BOOTROM"; > > + reg = <0x00000000 0x00020000>; > > + read-only; > > + }; > > Partitions are not useful here, but if you want them, use the > partitions container instead, please. > > > + > > + partition@20000 { > > + label = "Linux"; > > + reg = <0x00020000 0x07d00000>; > > + }; > > + > > + partition@7d20000 { > > + label = "RedBoot"; > > + reg = <0x07d20000 0x002e0000>; > > + read-only; > > + }; > > + }; > > + > > +... > > > Thanks, > Miquèl
