| Message ID | 20220714122657.17972-3-i.bornyakov@metrotek.ru (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Lattice ECP5 FPGA manager | expand |
On 14/07/2022 14:26, Ivan Bornyakov wrote: > Add Device Tree Binding doc for Lattice ECP5 FPGA manager using slave > SPI to load .bit formatted uncompressed bitstream image. > > Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru> > --- > .../fpga/lattice,ecp5-spi-fpga-mgr.yaml | 71 +++++++++++++++++++ > 1 file changed, 71 insertions(+) > create mode 100644 Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml > > diff --git a/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml b/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml > new file mode 100644 > index 000000000000..79868f9c84e2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml > @@ -0,0 +1,71 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/fpga/lattice,ecp5-spi-fpga-mgr.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Lattice ECP5 FPGA manager. > + > +maintainers: > + - Ivan Bornyakov <i.bornyakov@metrotek.ru> > + > +description: > + Device Tree Bindings for Lattice ECP5 FPGA Manager using slave SPI to > + load the uncompressed bitstream in .bit format. s/Device Tree Bindings for// Instead describe the hardware you are adding bindings for. What is a "Manager"? It is so broad and unspecific... It is some dedicated hardware to communicate with FPGA or you just called this regular FPGA interface exposed to the CPU/SoC? > + > +properties: > + compatible: > + enum: > + - lattice,ecp5-spi-fpga-mgr Do not encode interface name in compatible so no "spi". > + > + reg: > + description: SPI chip select > + maxItems: 1 > + > + spi-max-frequency: > + maximum: 60000000 Reference spi/spi-peripheral-props.yaml in allOf > + > + 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 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>; Wrong indentation. 4-spaces for DTS example. > + #size-cells = <0>; > + > + fpga_mgr@0 { No underscores in node names. > + compatible = "lattice,ecp5-spi-fpga-mgr"; > + spi-max-frequency = <20000000>; > + reg = <0>; > + program-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>; > + init-gpios = <&gpio3 3 GPIO_ACTIVE_LOW>; > + done-gpios = <&gpio3 2 GPIO_ACTIVE_HIGH>; > + }; > + }; Best regards, Krzysztof
On Fri, Jul 15, 2022 at 11:33:54AM +0200, Krzysztof Kozlowski wrote: > On 14/07/2022 14:26, Ivan Bornyakov wrote: > > Add Device Tree Binding doc for Lattice ECP5 FPGA manager using slave > > SPI to load .bit formatted uncompressed bitstream image. > > > > Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru> > > --- > > .../fpga/lattice,ecp5-spi-fpga-mgr.yaml | 71 +++++++++++++++++++ > > 1 file changed, 71 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml > > > > diff --git a/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml b/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml > > new file mode 100644 > > index 000000000000..79868f9c84e2 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml > > @@ -0,0 +1,71 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/fpga/lattice,ecp5-spi-fpga-mgr.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Lattice ECP5 FPGA manager. > > + > > +maintainers: > > + - Ivan Bornyakov <i.bornyakov@metrotek.ru> > > + > > +description: > > + Device Tree Bindings for Lattice ECP5 FPGA Manager using slave SPI to > > + load the uncompressed bitstream in .bit format. > > s/Device Tree Bindings for// > > Instead describe the hardware you are adding bindings for. What is a > "Manager"? It is so broad and unspecific... It is some dedicated > hardware to communicate with FPGA or you just called this regular FPGA > interface exposed to the CPU/SoC? > "FPGA Manager" is a kernel subsystem that exports a set of functions for programming an FPGA with a bitstream image. See Documentation/driver-api/fpga/fpga-mgr.rst > > + > > +properties: > > + compatible: > > + enum: > > + - lattice,ecp5-spi-fpga-mgr > > Do not encode interface name in compatible so no "spi". > Recently when I submitted FPGA manager for Microchip PolarFire, I was asked the opposite, to add "spi" in compatible. The reason was that FPGA can be programmed through other interfaces as well. > > + > > + reg: > > + description: SPI chip select > > + maxItems: 1 > > + > > + spi-max-frequency: > > + maximum: 60000000 > > Reference spi/spi-peripheral-props.yaml in allOf > > > + > > + 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 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>; > > Wrong indentation. 4-spaces for DTS example. > > > + #size-cells = <0>; > > + > > + fpga_mgr@0 { > > No underscores in node names. > > > + compatible = "lattice,ecp5-spi-fpga-mgr"; > > + spi-max-frequency = <20000000>; > > + reg = <0>; > > + program-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>; > > + init-gpios = <&gpio3 3 GPIO_ACTIVE_LOW>; > > + done-gpios = <&gpio3 2 GPIO_ACTIVE_HIGH>; > > + }; > > + }; > > > Best regards, > Krzysztof
On 15/07/2022 12:03, Ivan Bornyakov wrote: > On Fri, Jul 15, 2022 at 11:33:54AM +0200, Krzysztof Kozlowski wrote: >> On 14/07/2022 14:26, Ivan Bornyakov wrote: >>> Add Device Tree Binding doc for Lattice ECP5 FPGA manager using slave >>> SPI to load .bit formatted uncompressed bitstream image. >>> >>> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru> >>> --- >>> .../fpga/lattice,ecp5-spi-fpga-mgr.yaml | 71 +++++++++++++++++++ >>> 1 file changed, 71 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml b/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml >>> new file mode 100644 >>> index 000000000000..79868f9c84e2 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml >>> @@ -0,0 +1,71 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/fpga/lattice,ecp5-spi-fpga-mgr.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Lattice ECP5 FPGA manager. >>> + >>> +maintainers: >>> + - Ivan Bornyakov <i.bornyakov@metrotek.ru> >>> + >>> +description: >>> + Device Tree Bindings for Lattice ECP5 FPGA Manager using slave SPI to >>> + load the uncompressed bitstream in .bit format. >> >> s/Device Tree Bindings for// >> >> Instead describe the hardware you are adding bindings for. What is a >> "Manager"? It is so broad and unspecific... It is some dedicated >> hardware to communicate with FPGA or you just called this regular FPGA >> interface exposed to the CPU/SoC? >> > > "FPGA Manager" is a kernel subsystem that exports a set of functions for > programming an FPGA with a bitstream image. > See Documentation/driver-api/fpga/fpga-mgr.rst This is what you want to include in the bindings document? How is it related to bindings? We do not talk about driver API but we talk about hardware. Bindings are for the hardware. > >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - lattice,ecp5-spi-fpga-mgr >> >> Do not encode interface name in compatible so no "spi". >> > > Recently when I submitted FPGA manager for Microchip PolarFire, I was > asked the opposite, to add "spi" in compatible. The reason was that FPGA > can be programmed through other interfaces as well. I don't see such comment from Rob (DT maintainer): https://lore.kernel.org/all/?q=%22dt-bindings%3A+fpga%3A+add+binding+doc+for+microchip-spi+fpga+mgr%22 Can you point me to it? Best regards, Krzysztof
On Mon, Jul 18, 2022 at 03:06:18PM +0200, Krzysztof Kozlowski wrote: > On 15/07/2022 12:03, Ivan Bornyakov wrote: > > On Fri, Jul 15, 2022 at 11:33:54AM +0200, Krzysztof Kozlowski wrote: > >> On 14/07/2022 14:26, Ivan Bornyakov wrote: > >>> Add Device Tree Binding doc for Lattice ECP5 FPGA manager using slave > >>> SPI to load .bit formatted uncompressed bitstream image. > >>> > >>> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru> > >>> --- > >>> .../fpga/lattice,ecp5-spi-fpga-mgr.yaml | 71 +++++++++++++++++++ > >>> 1 file changed, 71 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml > >>> > >>> diff --git a/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml b/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml > >>> new file mode 100644 > >>> index 000000000000..79868f9c84e2 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml > >>> @@ -0,0 +1,71 @@ > >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>> +%YAML 1.2 > >>> +--- > >>> +$id: http://devicetree.org/schemas/fpga/lattice,ecp5-spi-fpga-mgr.yaml# > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>> + > >>> +title: Lattice ECP5 FPGA manager. > >>> + > >>> +maintainers: > >>> + - Ivan Bornyakov <i.bornyakov@metrotek.ru> > >>> + > >>> +description: > >>> + Device Tree Bindings for Lattice ECP5 FPGA Manager using slave SPI to > >>> + load the uncompressed bitstream in .bit format. > >> > >> s/Device Tree Bindings for// > >> > >> Instead describe the hardware you are adding bindings for. What is a > >> "Manager"? It is so broad and unspecific... It is some dedicated > >> hardware to communicate with FPGA or you just called this regular FPGA > >> interface exposed to the CPU/SoC? > >> > > > > "FPGA Manager" is a kernel subsystem that exports a set of functions for > > programming an FPGA with a bitstream image. > > See Documentation/driver-api/fpga/fpga-mgr.rst > > This is what you want to include in the bindings document? How is it > related to bindings? We do not talk about driver API but we talk about > hardware. Bindings are for the hardware. > I've send out v3 not too long ago. If you found the wording there not good enough, could you look through Documentation/devicetree/bindings/fpga/ and point me to a proper example? > > > >>> + > >>> +properties: > >>> + compatible: > >>> + enum: > >>> + - lattice,ecp5-spi-fpga-mgr > >> > >> Do not encode interface name in compatible so no "spi". > >> > > > > Recently when I submitted FPGA manager for Microchip PolarFire, I was > > asked the opposite, to add "spi" in compatible. The reason was that FPGA > > can be programmed through other interfaces as well. > > I don't see such comment from Rob (DT maintainer): > https://lore.kernel.org/all/?q=%22dt-bindings%3A+fpga%3A+add+binding+doc+for+microchip-spi+fpga+mgr%22 > > Can you point me to it? > Yeah, it was not Rob but other developer: https://lore.kernel.org/all/79328410-e56f-7c8a-9d17-de9bfdb98f51@microchip.com/ And at that point I had not even written the bindings doc, so neither you nor Rob weren't in the Cc. But eventually Rob reviewed DT bindings doc for PolarFire with compatible string to be "microchip,mpf-spi-fpga-mgr" https://lore.kernel.org/all/YkORrgC1FdzaKCMW@robh.at.kernel.org/ So I thought it was OK.
On 18/07/2022 15:46, Ivan Bornyakov wrote: > On Mon, Jul 18, 2022 at 03:06:18PM +0200, Krzysztof Kozlowski wrote: >> On 15/07/2022 12:03, Ivan Bornyakov wrote: >>> On Fri, Jul 15, 2022 at 11:33:54AM +0200, Krzysztof Kozlowski wrote: >>>> On 14/07/2022 14:26, Ivan Bornyakov wrote: >>>>> Add Device Tree Binding doc for Lattice ECP5 FPGA manager using slave >>>>> SPI to load .bit formatted uncompressed bitstream image. >>>>> >>>>> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru> >>>>> --- >>>>> .../fpga/lattice,ecp5-spi-fpga-mgr.yaml | 71 +++++++++++++++++++ >>>>> 1 file changed, 71 insertions(+) >>>>> create mode 100644 Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml b/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml >>>>> new file mode 100644 >>>>> index 000000000000..79868f9c84e2 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml >>>>> @@ -0,0 +1,71 @@ >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>>> +%YAML 1.2 >>>>> +--- >>>>> +$id: http://devicetree.org/schemas/fpga/lattice,ecp5-spi-fpga-mgr.yaml# >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>>> + >>>>> +title: Lattice ECP5 FPGA manager. >>>>> + >>>>> +maintainers: >>>>> + - Ivan Bornyakov <i.bornyakov@metrotek.ru> >>>>> + >>>>> +description: >>>>> + Device Tree Bindings for Lattice ECP5 FPGA Manager using slave SPI to >>>>> + load the uncompressed bitstream in .bit format. >>>> >>>> s/Device Tree Bindings for// >>>> >>>> Instead describe the hardware you are adding bindings for. What is a >>>> "Manager"? It is so broad and unspecific... It is some dedicated >>>> hardware to communicate with FPGA or you just called this regular FPGA >>>> interface exposed to the CPU/SoC? >>>> >>> >>> "FPGA Manager" is a kernel subsystem that exports a set of functions for >>> programming an FPGA with a bitstream image. >>> See Documentation/driver-api/fpga/fpga-mgr.rst >> >> This is what you want to include in the bindings document? How is it >> related to bindings? We do not talk about driver API but we talk about >> hardware. Bindings are for the hardware. >> > > I've send out v3 not too long ago. If you found the wording there not > good enough, could you look through > Documentation/devicetree/bindings/fpga/ and point me to a proper example? > >>> >>>>> + >>>>> +properties: >>>>> + compatible: >>>>> + enum: >>>>> + - lattice,ecp5-spi-fpga-mgr >>>> >>>> Do not encode interface name in compatible so no "spi". >>>> >>> >>> Recently when I submitted FPGA manager for Microchip PolarFire, I was >>> asked the opposite, to add "spi" in compatible. The reason was that FPGA >>> can be programmed through other interfaces as well. >> >> I don't see such comment from Rob (DT maintainer): >> https://lore.kernel.org/all/?q=%22dt-bindings%3A+fpga%3A+add+binding+doc+for+microchip-spi+fpga+mgr%22 >> >> Can you point me to it? >> > > Yeah, it was not Rob but other developer: > https://lore.kernel.org/all/79328410-e56f-7c8a-9d17-de9bfdb98f51@microchip.com/ > The type of bus should not be included in the compatible. It's obvious by looking at the parent, so Conor's comment was not helpful, IMO. > And at that point I had not even written the bindings doc, so neither > you nor Rob weren't in the Cc. > > But eventually Rob reviewed DT bindings doc for PolarFire with > compatible string to be "microchip,mpf-spi-fpga-mgr" > https://lore.kernel.org/all/YkORrgC1FdzaKCMW@robh.at.kernel.org/ > > So I thought it was OK. If spi was at the end, probably would be easier to spot thus would trigger a comment. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml b/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml new file mode 100644 index 000000000000..79868f9c84e2 --- /dev/null +++ b/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml @@ -0,0 +1,71 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/fpga/lattice,ecp5-spi-fpga-mgr.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Lattice ECP5 FPGA manager. + +maintainers: + - Ivan Bornyakov <i.bornyakov@metrotek.ru> + +description: + Device Tree Bindings for Lattice ECP5 FPGA Manager using slave SPI to + load the uncompressed bitstream in .bit format. + +properties: + compatible: + enum: + - lattice,ecp5-spi-fpga-mgr + + reg: + description: SPI chip select + 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 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-spi-fpga-mgr"; + spi-max-frequency = <20000000>; + reg = <0>; + program-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>; + init-gpios = <&gpio3 3 GPIO_ACTIVE_LOW>; + done-gpios = <&gpio3 2 GPIO_ACTIVE_HIGH>; + }; + };
Add Device Tree Binding doc for Lattice ECP5 FPGA manager using slave SPI to load .bit formatted uncompressed bitstream image. Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru> --- .../fpga/lattice,ecp5-spi-fpga-mgr.yaml | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml