| Message ID | 20220719112335.9528-3-i.bornyakov@metrotek.ru (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Lattice ECP5 FPGA manager | expand |
Hi, On Tue, Jul 19, 2022 at 02:23:35PM +0300, Ivan Bornyakov wrote: > +properties: > + compatible: > + enum: > + - lattice,ecp5-fpga-mgr Since this driver uses the same interface as the existing drivers/fpga/machxo2-spi.c driver, wouldn't it be advisable to use a similar compatible id, i.e. lattice,ecp5-slave-spi? > +required: > + - compatible > + - reg > + - program-gpios > + - init-gpios > + - done-gpios I think some of the GPIOs can be made optional by reading the status register or using the refresh command, assuming the slave spi interface stayed enabled after previous programming and we are not dealing with several chained FPGAs. But that can of course be left as an exercise for other developers. Best regards, Daniel
On Fri, Jul 29, 2022 at 11:01:24AM +0200, Daniel Glöckner wrote: > Hi, > > On Tue, Jul 19, 2022 at 02:23:35PM +0300, Ivan Bornyakov wrote: > > +properties: > > + compatible: > > + enum: > > + - lattice,ecp5-fpga-mgr > > Since this driver uses the same interface as the existing > drivers/fpga/machxo2-spi.c driver, wouldn't it be advisable to use a > similar compatible id, i.e. lattice,ecp5-slave-spi? That's a good clue for me. I searched the machxo2 & ecp5 Programming Usage Guide and seems they share the similar slave SPI sysCONFIG interface, at least the command word tables are the same. So could we have a generic driver for the lattice slave SPI sysCONFIG interface, rather than create similar drivers for each board? Thanks, Yilun > > > +required: > > + - compatible > > + - reg > > + - program-gpios > > + - init-gpios > > + - done-gpios > > I think some of the GPIOs can be made optional by reading the status > register or using the refresh command, assuming the slave spi interface > stayed enabled after previous programming and we are not dealing with > several chained FPGAs. But that can of course be left as an exercise for > other developers. > > Best regards, > > Daniel
On Fri, Jul 29, 2022 at 11:01:24AM +0200, Daniel Glöckner wrote: > Hi, > > On Tue, Jul 19, 2022 at 02:23:35PM +0300, Ivan Bornyakov wrote: > > +properties: > > + compatible: > > + enum: > > + - lattice,ecp5-fpga-mgr > > Since this driver uses the same interface as the existing > drivers/fpga/machxo2-spi.c driver, wouldn't it be advisable to use a > similar compatible id, i.e. lattice,ecp5-slave-spi? > To quote Krzysztof Kozlowski from v1 review: > Do not encode interface name in compatible so no "spi" See https://lore.kernel.org/linux-fpga/044a9736-a4ec-c250-7755-c80a5bcbe38b@linaro.org/ > > +required: > > + - compatible > > + - reg > > + - program-gpios > > + - init-gpios > > + - done-gpios > > I think some of the GPIOs can be made optional by reading the status > register or using the refresh command, assuming the slave spi interface > stayed enabled after previous programming and we are not dealing with > several chained FPGAs. But that can of course be left as an exercise for > other developers. I would prefer the latter.
On Fri, Jul 29, 2022 at 07:33:47PM +0300, Ivan Bornyakov wrote: > On Fri, Jul 29, 2022 at 11:01:24AM +0200, Daniel Glöckner wrote: > > On Tue, Jul 19, 2022 at 02:23:35PM +0300, Ivan Bornyakov wrote: > > > +properties: > > > + compatible: > > > + enum: > > > + - lattice,ecp5-fpga-mgr > > > > Since this driver uses the same interface as the existing > > drivers/fpga/machxo2-spi.c driver, wouldn't it be advisable to use a > > similar compatible id, i.e. lattice,ecp5-slave-spi? > > > > To quote Krzysztof Kozlowski from v1 review: > > Do not encode interface name in compatible so no "spi" > > See https://lore.kernel.org/linux-fpga/044a9736-a4ec-c250-7755-c80a5bcbe38b@linaro.org/ true... MachXO2 to MachXO5 and those Nexus chips speak that protocol both over spi and i2c and the position in the device tree decides which one it should be. Maybe the compatible string in the existing driver should be changed to lattice,machxo2. After all there is no other MachXO2-specific interface/protocol except maybe jtag. Best regards, Daniel
diff --git a/Documentation/devicetree/bindings/fpga/lattice,ecp5-fpga-mgr.yaml b/Documentation/devicetree/bindings/fpga/lattice,ecp5-fpga-mgr.yaml new file mode 100644 index 000000000000..34693a3c2f1e --- /dev/null +++ b/Documentation/devicetree/bindings/fpga/lattice,ecp5-fpga-mgr.yaml @@ -0,0 +1,74 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/fpga/lattice,ecp5-fpga-mgr.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Lattice ECP5 Slave SPI FPGA manager + +maintainers: + - Ivan Bornyakov <i.bornyakov@metrotek.ru> + +description: + Lattice ECP5 sysCONFIG port, which is used for device configuration, among + others, have Slave Serial Peripheral Interface. Only full reconfiguration + with uncompressed bitstream image in .bit format is supported. + +allOf: + - $ref: /schemas/spi/spi-peripheral-props.yaml + +properties: + compatible: + enum: + - lattice,ecp5-fpga-mgr + + reg: + maxItems: 1 + + spi-max-frequency: + maximum: 60000000 + + program-gpios: + description: + A GPIO line connected to PROGRAMN (active low) pin of the device. + Initiates configuration sequence. + maxItems: 1 + + init-gpios: + description: + A GPIO line connected to INITN (active low) pin of the device. + Indicates that the FPGA is ready to be configured. + maxItems: 1 + + done-gpios: + description: + A GPIO line connected to DONE (active high) pin of the device. + Indicates that the configuration sequence is complete. + maxItems: 1 + +required: + - compatible + - reg + - program-gpios + - init-gpios + - done-gpios + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + spi { + #address-cells = <1>; + #size-cells = <0>; + + fpga-mgr@0 { + compatible = "lattice,ecp5-fpga-mgr"; + reg = <0>; + spi-max-frequency = <20000000>; + program-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>; + init-gpios = <&gpio3 3 GPIO_ACTIVE_LOW>; + done-gpios = <&gpio3 2 GPIO_ACTIVE_HIGH>; + }; + };