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@xxxxxxxxxxx> >>>>> --- >>>>> .../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@xxxxxxxxxxx> >>>>> + >>>>> +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@xxxxxxxxxxxxx/ > 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@xxxxxxxxxxxxxxxxxx/ > > 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
