Re: [PATCH] Staging: silabs si4455 serial driver

[Rob Herring <robh+dt@kernel.org>]

On Wed, Dec 9, 2020 at 5:17 AM Info <info@ministro.hu> wrote:
>
> This is a serial port driver for
> Silicon Labs Si4455 Sub-GHz transciver.
>
> Signed-off-by: József Horváth <info@ministro.hu>
> ---
>  .../bindings/staging/serial/silabs,si4455.txt |   39 +

Looks straightforward enough to not be in staging. Plus
bindings/staging/serial is not a directory I want.

DT bindings are in DT schema format now. See
Documentation/devicetree/writing-schema.rst.

>  drivers/staging/Kconfig                       |    2 +
>  drivers/staging/Makefile                      |    1 +
>  drivers/staging/si4455/Kconfig                |    8 +
>  drivers/staging/si4455/Makefile               |    2 +
>  drivers/staging/si4455/TODO                   |    3 +
>  drivers/staging/si4455/si4455.c               | 1465 +++++++++++++++++
>  drivers/staging/si4455/si4455_api.h           |   56 +
>  8 files changed, 1576 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt
>  create mode 100644 drivers/staging/si4455/Kconfig
>  create mode 100644 drivers/staging/si4455/Makefile
>  create mode 100644 drivers/staging/si4455/TODO
>  create mode 100644 drivers/staging/si4455/si4455.c
>  create mode 100644 drivers/staging/si4455/si4455_api.h
>
> diff --git
> a/Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt
> b/Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt
> new file mode 100644
> index 000000000000..abd659b7b952
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt
> @@ -0,0 +1,39 @@
> +* Silicon Labs Si4455 EASY-TO-USE, LOW-CURRENT OOK/(G)FSK SUB-GHZ
> TRANSCEIVER
> +
> +Required properties:
> +- compatible: Should be one of the following:
> +  - "silabs,si4455" for Silicon Labs Si4455-B1A or Si4455-C2A (driver
> automatically detects the part info),

Either do this or...

> +  - "silabs,si4455b1a" for Silicon Labs Si4455-B1A,
> +  - "silabs,si4455c2a" for Silicon Labs Si4455-C2A,

this. Not both. I assume there's an id register or something you can
read to determine which one. That's fine, but consider if there's any
power sequencing differences that you'd need to handle before you can
read that register.

> +- reg: SPI chip select number.
> +- interrupts: Specifies the interrupt source of the parent interrupt
> +  controller. The format of the interrupt specifier depends on the
> +  parent interrupt controller.
> +- clocks: phandle to the IC source clock (only external clock source
> supported).
> +- spi-max-frequency: maximum clock frequency on SPI port
> +- shdn-gpios: gpio pin for SDN

shutdown-gpios is the semi-standard name.

> +
> +Example:
> +
> +/ {
> +       clocks {
> +                si4455_1_2_osc: si4455_1_2_osc {
> +                        compatible = "fixed-clock";
> +                        #clock-cells = <0>;
> +                        clock-frequency  = <30000000>;
> +                };
> +       };

Don't need to show this for the example.

> +};
> +
> +&spi0 {
> +       si4455: si4455@0 {
> +               compatible = "silabs,si4455";
> +               reg = <0>;
> +               clocks = <&si4455_1_2_osc>;
> +               interrupt-parent = <&gpio>;
> +               interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> +                shdn-gpios = <&gpio 26 1>;
> +                status = "okay";

Don't show status in examples.

> +                spi-max-frequency = <3000000>;
> +       };
> +};