Re: [PATCH 2/5] dt-bindings: serial: uartlite: Add properties for synthesis-time parameters

[Sean Anderson <sean.anderson@seco.com>]

On 7/26/21 8:30 AM, Michal Simek wrote:
 >
 >
 > On 7/24/21 12:31 AM, Sean Anderson wrote:
 >> The uartlite device is a "soft" device. Many parameters, such as baud
 >> rate, data bits, and the presence of a parity bit are configured before
 >> synthesis and may not be changed (or discovered) at runtime. However, we
 >> must know what these settings are in order to properly calculate the
 >> uart timeout (and to inform the user about the actual baud of the uart).
 >>
 >> These properties are present for out-of-tree bindings generated by
 >> Xilinx's tools. However, they are also (mostly) present in in-tree
 >> bindings. I chose current-speed over xlnx,baudrate primarily because it
 >> seemed to be used by more existing bindings.
 >>
 >> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
 >> ---
 >>
 >>  .../bindings/serial/xlnx,opb-uartlite.yaml    | 39 +++++++++++++++++++
 >>  1 file changed, 39 insertions(+)
 >>
 >> diff --git a/Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.yaml b/Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.yaml
 >> index 4ef29784ae97..28859e70e60f 100644
 >> --- a/Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.yaml
 >> +++ b/Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.yaml
 >> @@ -32,13 +32,49 @@ properties:
 >>    clock-names:
 >>      const: s_axi_aclk
 >>
 >> +  current-speed:
 >> +    $ref: /schemas/types.yaml#/definitions/uint32
 >> +    description:
 >> +      The fixed baud rate that the device was configured for.
 >> +
 >> +  xlnx,data-bits:
 >> +    $ref: /schemas/types.yaml#/definitions/uint32
 >> +    enum: [5, 6, 7, 8]
 >> +    default: 8
 >> +    description:
 >> +      The fixed number of data bits that the device was configured for.
 >> +
 >> +  xlnx,use-parity:
 >> +    $ref: /schemas/types.yaml#/definitions/uint32
 >> +    enum: [0, 1]
 >> +    default: 0
 >> +    description:
 >> +      Whether parity checking was enabled when the device was configured.
 >> +
 >> +  xlnx,odd-parity:
 >> +    $ref: /schemas/types.yaml#/definitions/uint32
 >> +    enum: [0, 1]
 >> +    description:
 >> +      Whether odd parity was configured.
 >> +
 >>  required:
 >>    - compatible
 >>    - reg
 >>    - interrupts
 >> +  - current-speed
 >> +  - xlnx,data-bits
 >> +  - xlnx,use-parity
 >
 > I think all of these should be optional properties because were required
 > in past. Likely a lot of xilinx dt binding files have them but as it is
 > visible sh also has it without it.

As I understand it, the "required" here covers both properties which the
driver cannot function without and properties which should be set for all
new bindings. That is, the 'required" here is perscriptive. The worst
that happens is that the bindings author gets a warning when doing
dtbs_check, which is exactly what we want to happen.

In the driver itself, only "current-speed" is required. However, I could
make that optional as well (although perhaps with a warning). I feel
much less comfortable guessing the default baud rate than guessing 8n1.

--Sean

 >
 > M
 >
 >>
 >>  allOf:
 >>    - $ref: /schemas/serial.yaml#
 >> +  - if:
 >> +      properties:
 >> +        xlnx,use-parity:
 >> +          contains:
 >> +            const: 1
 >> +    then:
 >> +      required:
 >> +        - xlnx,odd-parity
 >>
 >>  additionalProperties: true
 >>
 >> @@ -49,5 +85,8 @@ examples:
 >>          reg = <0x800c0000 0x10000>;
 >>          interrupts = <0x0 0x6e 0x1>;
 >>          port-number = <0>;
 >> +        current-speed = <115200>;
 >> +        xlnx,data-bits = <8>;
 >> +        xlnx,use-parity = <0>;
 >>        };
 >>  ...
 >>
 >