On 03/03/2024 18:21, Charles Perry wrote: > On Feb 27, 2024, at 3:10 AM, Krzysztof Kozlowski krzysztof.kozlowski@xxxxxxxxxx wrote: > >> On 21/02/2024 20:50, Charles Perry wrote: >>> Document the SelectMAP interface of Xilinx 7 series FPGA. >>> >>> Signed-off-by: Charles Perry <charles.perry@xxxxxxxxxxxxxxxxxxxx> >>> --- >>> .../bindings/fpga/xlnx,fpga-selectmap.yaml | 86 +++++++++++++++++++ >>> 1 file changed, 86 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/fpga/xlnx,fpga-selectmap.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/fpga/xlnx,fpga-selectmap.yaml >>> b/Documentation/devicetree/bindings/fpga/xlnx,fpga-selectmap.yaml >>> new file mode 100644 >>> index 0000000000000..08a5e92781657 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/fpga/xlnx,fpga-selectmap.yaml >>> @@ -0,0 +1,86 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/fpga/xlnx,fpga-selectmap.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Xilinx SelectMAP FPGA interface >>> + >>> +maintainers: >>> + - Charles Perry <charles.perry@xxxxxxxxxxxxxxxxxxxx> >>> + >>> +description: | >>> + Xilinx 7 Series FPGAs support a method of loading the bitstream over a >>> + parallel port named the SelectMAP interface in the documentation. Only >>> + the x8 mode is supported where data is loaded at one byte per rising edge of >>> + the clock, with the MSB of each byte presented to the D0 pin. >>> + >>> + Datasheets: >>> + >>> https://www.xilinx.com/support/documentation/user_guides/ug470_7Series_Config.pdf >>> + >>> +allOf: >>> + - $ref: /schemas/memory-controllers/mc-peripheral-props.yaml# >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - xlnx,fpga-xc7s-selectmap >>> + - xlnx,fpga-xc7a-selectmap >>> + - xlnx,fpga-xc7k-selectmap >>> + - xlnx,fpga-xc7v-selectmap >>> + >>> + reg: >>> + description: >>> + At least 1 byte of memory mapped IO >>> + maxItems: 1 >>> + >>> + prog_b-gpios: >> >> I commented on this and still see underscore. Nothing in commit msg >> explains why this should have underscore. Changelog is also vague - >> describes that you brought back underscores, instead of explaining why >> you did it. >> >> So the same comments as usual: >> >> No underscores in names. >> >> Best regards, >> Krzysztof > > Hello Krzysztof, > > Yes, I've gone full circle on that issue. Here's what I tried so far: And what part of the commit description allows me to understand this? > > 1) Reuse the same gpio names: Duplicates errors of the past, Krzysztof > doesn't like it. > 2) Different gpio names for new driver only: Makes the driver code > overly complicated, Yilun doesn't like it. That's a new driver, right? So what is complicated here? You have new code and you take prog-b or prog_b? > 3) Change gpio names for both drivers, deprecate the old names: Makes > the DT binding and the driver code overly complicated, Rob doesn't > like it. I don't think I proposed changing existing bindings. > > I think that while the driver code shouldn't be the driving force for > the DT spec, it can be a good indication that the spec is unpractical to > implement. What is impractical in implementing this? You just pass either A or B to function requesting GPIO. Just choose proper name. > > In this case, there are two interfaces on a chip that uses the same GPIO > protocol, it would only make sense that they use the same names, this > discards solution #2. I don't understand this. You have devm_gpiod_get() in your new code. Why is it difficult to use different name? Best regards, Krzysztof
