Re: [PATCH v5] Serial: silabs si4455 serial driver

[Rob Herring]

On Tue, Dec 15, 2020 at 07:20:07AM +0000, József Horváth wrote:
> This is a serial port driver for
>  Silicon Labs Si4455 Sub-GHz transciver.
> 
> The goal of this driver is to removing wires
>  between central(linux) device and remote serial devices/sensors,
>  but keeping the original user software.
>  It represents regular serial interface for the user space.
> 
> Datasheet: https://www.silabs.com/documents/public/data-sheets/Si4455.pdf
> 
> changes v1:
>  - fixed: drivers: serial: si4455: coding style
>  - fixed: drivers: serial: si4455: error checking and order
>  - fixed: drivers: serial: si4455: remove unnecessary compatibility
>    strings from si4455_dt_ids
>  - fixed: dt-bindings: rename sdn-gpios to shutdown-gpios
> 
> changes v2:
>  - fixed: drivers: serial: si4455: coding style
> 
> changes v3:
>  - fixed: drivers: serial: si4455: coding style
>  - fixed: drivers: serial: si4455: replace device configuration procedure
>    (SI4455_IOC_SEZC ioctl call) with request_firmware(...).
>    The firmware name comes from dt (silabs,ez-config)
>  - fixed: drivers: serial: si4455: replace transmit/receive channel
>    select (SI4455_IOC_STXC/SI4455_IOC_SRXC ioctl calls)
>    with sysfs entries (tx_channel, rx_channel).
>    Initial values comes from dt (silabs,tx-channel and silabs,rx-channel)
>  - fixed: drivers: serial: si4455: replace package size setting
>    (SI4455_IOC_SSIZ ioctl call) with sysfs entry (package_size).
>    Initial value comes from dt (silabs,package-size)
>  - fixed: drivers: serial: si4455: replace getting last rssi
>    (SI4455_IOC_GRSSI ioctl call) with sysfs entry (current_rssi)
>  - fixed: drivers: serial: si4455: remove si4455_api.h
>    and custom ioctl definitions
>  - fixed: dt-bindings: silabs,si4455: more detailed description
>  - added: dt-bindings: silabs,si4455: properties silabs,package-size,
>    silabs,tx-channel, silabs,rx-channel, silabs,ez-config
> 
> changes v4:
>  - fixed: dt-bindings: silabs,si4455: $id
>    from http://devicetree.org/schemas/serial/silabs,si4455.yaml
>    to http://devicetree.org/schemas/staging/serial/silabs,si4455.yaml
> 
> changes v5:
>  - fixed: drivers: serial: si4455: coding style
>  - fixed: drivers: serial: si4455: remove struct si4455_one,
>    members moved to struct si4455_port
>  - fixed: drivers: serial: si4455: fix line endings in dev_err and
>    dev_dbg messages
>  - fixed: drivers: serial: si4455: remove unnecessary else { ... }
>  - fixed: drivers: serial: si4455: refactor si4455_do_work(...),
>    xmit circular buffer handling and start tx moved to
>    si4455_start_tx_xmit(...)
>  - fixed: drivers: serial: si4455: refactor si4455_configure
>  - fixed: drivers: serial: si4455: refactor interrupt handling,
>    remove unnecessary wrapper
>  - fixed: drivers: serial: si4455: modem line(si4455_get_mctrl)
>    and tx buffer status(si4455_tx_empty) conditions and signaling
>  - fixed: drivers: serial: si4455: remove unsafe int to pointer conversion
>  - fixed: dt-bindings: silabs,si4455: $id
>    from http://devicetree.org/schemas/staging/serial/silabs,si4455.yaml
>    to http://devicetree.org/schemas/serial/silabs,si4455.yaml
>  - fixed: dt-bindings: silabs,si4455: serial.yaml reference added

The revision history should be after the '---' so it is not in the git 
history when applied.

> Signed-off-by: József Horváth <info@xxxxxxxxxxx>
> ---
>  .../bindings/serial/silabs,si4455.yaml        |   98 ++

Please make the binding a separate patch.

>  MAINTAINERS                                   |    6 +
>  drivers/tty/serial/Kconfig                    |    8 +
>  drivers/tty/serial/Makefile                   |    1 +
>  drivers/tty/serial/si4455.c                   | 1372 +++++++++++++++++
>  5 files changed, 1485 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/serial/silabs,si4455.yaml
>  create mode 100644 drivers/tty/serial/si4455.c
> 
> diff --git a/Documentation/devicetree/bindings/serial/silabs,si4455.yaml b/Documentation/devicetree/bindings/serial/silabs,si4455.yaml
> new file mode 100644
> index 000000000000..ddff67e6a667
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/silabs,si4455.yaml
> @@ -0,0 +1,98 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/serial/silabs,si4455.yaml#";
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#";
> +
> +title: Silicon Labs Si4455 device tree bindings
> +
> +maintainers:
> +  - József Horváth <info@xxxxxxxxxxx>
> +
> +description:
> +  This document is for describing the required device tree parameters for si4455 serial driver.
> +  The si4455 driver tries to represent the Silicon Labs Si4455 sub-GHz transceiver device
> +  like a serial port. The required parameters for proper operation are described below.
> +  https://www.silabs.com/documents/public/data-sheets/Si4455.pdf
> +
> +allOf:
> +  - $ref: "serial.yaml#"
> +
> +properties:
> +  compatible:
> +    const: silabs,si4455
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    description: maximum clock frequency on SPI port
> +    maximum: 500000
> +
> +  shutdown-gpios:
> +    description: gpio pin for SDN
> +    maxItems: 1
> +
> +  silabs,package-size:
> +    description:
> +      Radio payload length, variable packet length is not supported by driver.
> +      This value should equal with EZConfig payload length.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    maximum: 64
> +    minimum: 1
> +
> +  silabs,tx-channel:
> +    description:
> +      Radio transmit channel selection.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    maximum: 255
> +    minimum: 0
> +
> +  silabs,rx-channel:
> +    description:
> +      Radio receive channel selection.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    maximum: 255
> +    minimum: 0
> +
> +  silabs,ez-config:
> +    description:
> +      Radio configuration data file name.
> +    $ref: /schemas/types.yaml#/definitions/string
> +    items:
> +      pattern: ^[0-9a-z\._\-]{1,255}$

We use 'firmware-name' property for this purpose. 

Looks good otherwise.

> +
> +required:
> +  - reg
> +  - interrupts
> +  - spi-max-frequency
> +  - shutdown-gpios
> +  - silabs,package-size
> +  - silabs,tx-channel
> +  - silabs,rx-channel
> +  - silabs,ez-config
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    spi {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      si4455_0: serial@0 {
> +        compatible = "silabs,si4455";
> +        reg = <0>;
> +        interrupt-parent = <&gpio>;
> +        interrupts = <7 2>;
> +        shutdown-gpios = <&gpio 26 1>;
> +        spi-max-frequency = <300000>;
> +        silabs,package-size = <30>;
> +        silabs,tx-channel = <1>;
> +        silabs,rx-channel = <2>;
> +        silabs,ez-config = "si4455_spi0_0.ez.bin";
> +      };
> +    };
> +...