Skip to contentSkip to conversation

Serial: silabs si4455 serial driver

PATCH Superseded József Horváth 2 participants 7 messages Dec 12, 2020 – Dec 14, 2020

Subsystems (2)

v4 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

PATCH v4 1/1 Serial: silabs si4455 serial driver

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

---
 .../staging/serial/silabs,si4455.yaml         |   95 ++
 MAINTAINERS                                   |    6 +
 drivers/tty/serial/Kconfig                    |    8 +
 drivers/tty/serial/Makefile                   |    1 +
 drivers/tty/serial/si4455.c                   | 1328 +++++++++++++++++
 5 files changed, 1438 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/staging/serial/silabs,si4455.yaml
 create mode 100644 drivers/tty/serial/si4455.c
Documentation/devicetree/bindings/staging/serial/silabs,si4455.yaml new file +95 -0
new file mode 100644
index 000000000000..d3a8c07c6714
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/staging/serial/silabs,si4455.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Silicon Labs Si4455 device tree bindings
+
+maintainers:
+  - József Horváth <[email protected]>
+
+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
+
+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}$
+
+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";
+      };
+    };
+...
MAINTAINERS +6 -0
index a008b70f3c16..50db79c78005 100644
@@ -15952,6 +15952,12 @@ M:	Jérôme Pouiller <[email protected]>
 S:	Supported
 F:	drivers/staging/wfx/
 
+SILICON LABS SI4455 SERIAL DRIVER
+M:	József Horváth <[email protected]>
+S:	Maintained
+F:	Documentation/devicetree/bindings/staging/serial/silabs,si4455.yaml
+F:	drivers/tty/serial/si4455.c
+
 SILICON MOTION SM712 FRAME BUFFER DRIVER
 M:	Sudip Mukherjee <[email protected]>
 M:	Teddy Wang <[email protected]>
drivers/tty/serial/Kconfig +8 -0
index 28f22e58639c..560aa311cd03 100644
@@ -1583,6 +1583,14 @@ config SERIAL_MILBEAUT_USIO_CONSOLE
 	  receives all kernel messages and warnings and which allows logins in
 	  single user mode).
 
+config SERIAL_SI4455
+	tristate "Si4455 support"
+	depends on SPI
+	select SERIAL_CORE
+	help
+	  This driver is for Silicon Labs's Si4455 Sub-GHz transciver.
+	  Say 'Y' here if you wish to use it as serial port.
+
 endmenu
 
 config SERIAL_MCTRL_GPIO
drivers/tty/serial/Makefile +1 -0
index caf167f0c10a..2a3076b98c78 100644
@@ -90,6 +90,7 @@ obj-$(CONFIG_SERIAL_OWL)	+= owl-uart.o
 obj-$(CONFIG_SERIAL_RDA)	+= rda-uart.o
 obj-$(CONFIG_SERIAL_MILBEAUT_USIO) += milbeaut_usio.o
 obj-$(CONFIG_SERIAL_SIFIVE)	+= sifive.o
+obj-$(CONFIG_SERIAL_SI4455) += si4455.o
 
 # GPIOLIB helpers for modem control lines
 obj-$(CONFIG_SERIAL_MCTRL_GPIO)	+= serial_mctrl_gpio.o
drivers/tty/serial/si4455.c new file +1328 -0

1,328 lines not on this page · raw message

Signature · 1 line
2.17.1
#1 lore raw
Quoted 27 lines · József Horváth

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

---
 .../staging/serial/silabs,si4455.yaml         |   95 ++
 MAINTAINERS                                   |    6 +
 drivers/tty/serial/Kconfig                    |    8 +
 drivers/tty/serial/Makefile                   |    1 +
 drivers/tty/serial/si4455.c                   | 1328 +++++++++++++++++
 5 files changed, 1438 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/staging/serial/silabs,si4455.yaml
 create mode 100644 drivers/tty/serial/si4455.c
Documentation/devicetree/bindings/staging/serial/silabs,si4455.yaml new file +0 -0
new file mode 100644
index 000000000000..d3a8c07c6714

Documentation/devicetree/bindings/staging/serial/silabs,si4455.yaml line 4

Quoted 4 lines
Documentation/devicetree/bindings/staging/serial/silabs,si4455.yaml new file +4 -0
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/staging/serial/silabs,si4455.yaml#"

In this version I changed the $id to "http://.../staging/..." and removed $ref: "serial.yaml#". The make dt_binding_check completes without any error or warning, but this is not a good solution.

My question is, how can I use $ref: "serial.yaml" dependency, while my schema is under staging?
allOf:
- $ref: "serial.yaml#"

The make dt_binding_check output, while the schema is in staging, and $ref: "serial.yaml" dependency in the schema:

Unknown file referenced: [Errno 2] No such file or directory: '/home/administrator/.local/lib/python3.6/site-packages/dtschema/schemas/staging/serial/serial.yaml'
xargs: dt-doc-validate: exited with status 255; aborting
Documentation/devicetree/bindings/Makefile:59: recipe for target 'Documentation/devicetree/bindings/processed-schema-examples.json' failed make[1]: *** [Documentation/devicetree/bindings/processed-schema-examples.json] Error 124
Makefile:1364: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2

Documentation/devicetree/bindings/staging/serial/silabs,si4455.yaml line 8

Quoted 6 lines
Documentation/devicetree/bindings/staging/serial/silabs,si4455.yaml new file +4 -0
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Silicon Labs Si4455 device tree bindings
+

Üdvözlettel / Best regards:
József Horváth

maintainer
#2 lore raw
In reply to József Horváth
Quoted 9 lines · József Horváth

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

A description of changes between v1..v4 here, please.

Quoted 1 line

...

Quoted 3 lines
drivers/tty/serial/si4455.c new file +0 -0
@@ -0,0 +1,1328 @@

...

Quoted 1 line

+struct si4455_one {

What is this si4455_one good for? I mean, why not just inline these two in si4455_port?

drivers/tty/serial/si4455.c line 108

Quoted 8 lines
drivers/tty/serial/si4455.c new file +8 -0
+	struct uart_port port;
+	struct work_struct tx_work;
+};
+
+struct si4455_port {
+	struct mutex mutex; /* For syncing access to device */
+	int power_count;
+	bool connected;

If you put this int and bool after u32s below, the structure won't contain holes AFAICS.

Quoted 12 lines
drivers/tty/serial/si4455.c new file +12 -0
+	struct gpio_desc *shdn_gpio;
+	struct si4455_part_info part_info;
+	struct si4455_modem_status modem_status;
+	u32 tx_channel;
+	u32 rx_channel;
+	u32 package_size;
+	bool configured;
+	bool tx_pending;
+	bool rx_pending;
+	u32 current_rssi;
+	struct si4455_one one;
+};

...

drivers/tty/serial/si4455.c line 139

Quoted 5 lines
drivers/tty/serial/si4455.c new file +5 -0
+static int si4455_get_response(struct uart_port *port, int length, u8 *data)
+{
+	int ret;
+	u8 data_out[] = { SI4455_CMD_ID_READ_CMD_BUFF };
+	u8 *data_in = NULL;

A useless initialization.

Quoted 12 lines
drivers/tty/serial/si4455.c new file +12 -0
+	struct spi_transfer xfer[2];
+	int timeout = 10000;
+
+	if (length > 0 && !data)
+		return -EINVAL;
+
+	data_in = kzalloc(1 + length, GFP_KERNEL);
+	if (!data_in)
+		return -ENOMEM;
+
+	memset(&xfer, 0x00, sizeof(xfer));
+	xfer[0].tx_buf = data_out;

drivers/tty/serial/si4455.c line 161

Quoted 10 lines
drivers/tty/serial/si4455.c new file +10 -0
+	xfer[0].len = sizeof(data_out);
+	xfer[1].rx_buf = data_in;
+	xfer[1].len = 1 + length;
+
+	while (--timeout > 0) {
+		data_out[0] = SI4455_CMD_ID_READ_CMD_BUFF;
+		ret = spi_sync_transfer(to_spi_device(port->dev), xfer,
+					ARRAY_SIZE(xfer));
+		if (ret) {
+			dev_err(port->dev, "%s: spi_sync_transfer error(%i)", __func__, ret);

Missing \n. And a space before (.

Quoted 18 lines
drivers/tty/serial/si4455.c new file +18 -0
+			break;
+		}
+
+		if (data_in[0] == 0xFF) {
+			if (length > 0 && data)
+				memcpy(data, &data_in[1], length);
+
+			break;
+		}
+		usleep_range(100, 200);
+	}
+	if (timeout == 0) {
+		dev_err(port->dev, "%s:timeout==%i", __func__, timeout);
+		ret = -EIO;
+	}
+	kfree(data_in);
+	return ret;
+}

...

drivers/tty/serial/si4455.c line 195

Quoted 10 lines
drivers/tty/serial/si4455.c new file +10 -0
+static int si4455_send_command(struct uart_port *port, int length, u8 *data)
+{
+	int ret;
+
+	ret = si4455_poll_cts(port);
+	if (ret) {
+		dev_err(port->dev,
+			"%s: si4455_poll_cts error(%i)", __func__, ret);
+		return ret;
+	}

Put a newline here.

drivers/tty/serial/si4455.c line 200

Quoted 5 lines
drivers/tty/serial/si4455.c new file +5 -0
+	ret = spi_write(to_spi_device(port->dev), data, length);
+	if (ret) {
+		dev_err(port->dev,
+			"%s: spi_write error(%i)", __func__, ret);
+	}

And here.

Quoted 9 lines
drivers/tty/serial/si4455.c new file +9 -0
+	return ret;
+}
+
+static int si4455_send_command_get_response(struct uart_port *port,
+					    int out_length, u8 *data_out,
+					    int in_length, u8 *data_in)
+{
+	int ret;
+

drivers/tty/serial/si4455.c line 219

Quoted 10 lines
drivers/tty/serial/si4455.c new file +10 -0
+	ret = si4455_send_command(port, out_length, data_out);
+	if (ret) {
+		dev_err(port->dev,
+			"%s: si4455_send_command error(%i)", __func__, ret);
+		return ret;
+	}
+
+	ret = si4455_get_response(port, in_length, data_in);
+
+	return ret;

Simplify by return si4455_get_response() directly.

Quoted 19 lines
drivers/tty/serial/si4455.c new file +19 -0
+}
+
+static int si4455_read_data(struct uart_port *port, u8 command, int poll,
+			    int length, u8 *data)
+{
+	int ret = 0;
+	u8 data_out[] = { command };
+	struct spi_transfer xfer[] = {
+		{
+			.tx_buf = data_out,
+			.len = sizeof(data_out),
+		}, {
+			.rx_buf = data,
+			.len = length,
+		}
+	};
+
+	if (poll) {
+		ret = si4455_poll_cts(port);

drivers/tty/serial/si4455.c line 248

Quoted 10 lines
drivers/tty/serial/si4455.c new file +10 -0
+		if (ret)
+			return ret;
+	}
+
+	ret = spi_sync_transfer(to_spi_device(port->dev),
+				xfer,
+				ARRAY_SIZE(xfer));
+	if (ret) {
+		dev_err(port->dev,
+			"%s: spi_sync_transfer error(%i)", __func__, ret);

\n here. And in all the other dev_errs all around.

drivers/tty/serial/si4455.c

Quoted 1 line
drivers/tty/serial/si4455.c new file +1 -0
+	}

A newline here.

Quoted 2 lines
drivers/tty/serial/si4455.c new file +2 -0
+	return ret;
+}

...

Quoted 9 lines
drivers/tty/serial/si4455.c new file +9 -0
+static int si4455_get_int_status(struct uart_port *port, u8 ph_clear,
+				 u8 modem_clear, u8 chip_clear,
+				 struct si4455_int_status *result)
+{
+	int ret;
+	u8 data_out[] = {
+		SI4455_CMD_ID_GET_INT_STATUS,
+		ph_clear,
+		modem_clear,

drivers/tty/serial/si4455.c line 353

Quoted 10 lines
drivers/tty/serial/si4455.c new file +10 -0
+		chip_clear
+	};
+	u8 data_in[SI4455_CMD_REPLY_COUNT_GET_INT_STATUS];
+
+	ret = si4455_send_command_get_response(port, sizeof(data_out), data_out,
+					       sizeof(data_in), data_in);
+	if (ret) {
+		dev_err(port->dev,
+			"%s: si4455_send_command_get_response error(%i)",
+			__func__, ret);

return ret; here. And no else there:

Quoted 19 lines
drivers/tty/serial/si4455.c new file +19 -0
+	} else {
+		result->int_pend       = data_in[0];
+		result->int_status     = data_in[1];
+		result->ph_pend        = data_in[2];
+		result->ph_status      = data_in[3];
+		result->modem_pend     = data_in[4];
+		result->modem_status   = data_in[5];
+		result->chip_pend      = data_in[6];
+		result->chip_status    = data_in[7];
+	}
+	return ret;
+}
+
+static int si4455_get_modem_status(struct uart_port *port, u8 modem_clear,
+				   struct si4455_modem_status *result)
+{
+	int ret;
+	u8 data_out[] = {
+		SI4455_CMD_ID_GET_MODEM_STATUS,

drivers/tty/serial/si4455.c line 382

Quoted 10 lines
drivers/tty/serial/si4455.c new file +10 -0
+		modem_clear,
+	};
+	u8 data_in[SI4455_CMD_REPLY_COUNT_GET_MODEM_STATUS];
+
+	ret = si4455_send_command_get_response(port, sizeof(data_out), data_out,
+					       sizeof(data_in), data_in);
+	if (ret) {
+		dev_err(port->dev,
+			"%s: si4455_send_command_get_response error(%i)",
+			__func__, ret);

The same here and the other functions below.

Quoted 12 lines
drivers/tty/serial/si4455.c new file +12 -0
+	} else {
+		result->modem_pend      = data_in[0];
+		result->modem_status    = data_in[1];
+		result->curr_rssi       = data_in[2];
+		result->latch_rssi      = data_in[3];
+		result->ant1_rssi       = data_in[4];
+		result->ant2_rssi       = data_in[5];
+		memcpy(&result->afc_freq_offset,
+		       &data_in[6], sizeof(result->afc_freq_offset));
+	}
+	return ret;
+}

...

drivers/tty/serial/si4455.c line 588

Quoted 8 lines
drivers/tty/serial/si4455.c new file +8 -0
+static int si4455_configure(struct uart_port *port, const u8 *configuration_data)
+{
+	int ret = 0;
+	u8 col;
+	u8 response;
+	u8 count;
+	struct si4455_int_status int_status = { 0 };
+	u8 radio_cmd[16u];

Any reason for 16 specifically _unsigned_?

drivers/tty/serial/si4455.c line 594

Quoted 6 lines
drivers/tty/serial/si4455.c new file +6 -0
+
+	/* While cycle as far as the pointer points to a command */
+	while (*configuration_data != 0x00) {
+		/* Commands structure in the array:
+		 * --------------------------------
+		 * LEN | <LEN length of data>

Multiline comments should look like this:
/*
* comment 1
* comment 2
*/

drivers/tty/serial/si4455.c line 599

Quoted 5 lines
drivers/tty/serial/si4455.c new file +5 -0
+		 */
+		count = *configuration_data++;
+		dev_dbg(port->dev, "%s: count(%u)",
+			__func__, count);
+		if (count > 16u) {

And here?

drivers/tty/serial/si4455.c line 603

Quoted 4 lines
drivers/tty/serial/si4455.c new file +4 -0
+			/* Initial configuration of Si4x55 */
+			if (SI4455_CMD_ID_WRITE_TX_FIFO
+				 == *configuration_data) {
+				if (count > 128u) {

And here?

BTW too many nestings here. Could you switch the SI4455_CMD_ID_WRITE_TX_FIFO if condition and break here. No need for else branch then and the code would go one indentation level left.

Quoted 77 lines
drivers/tty/serial/si4455.c new file +77 -0
+					/* Number of command bytes exceeds
+					 * maximal allowable length
+					 */
+					dev_err(port->dev, "%s: command length error(%i)",
+						__func__, count);
+					ret = -EINVAL;
+					break;
+				}
+
+				/* Load array to the device */
+				configuration_data++;
+				ret = si4455_write_data(port,
+							SI4455_CMD_ID_WRITE_TX_FIFO,
+							1,
+							count - 1,
+							configuration_data);
+				if (ret) {
+					dev_err(port->dev, "%s: si4455_write_data error(%i)",
+						__func__, ret);
+					break;
+				}
+
+				/* Point to the next command */
+				configuration_data += count - 1;
+
+				/* Continue command interpreter */
+				continue;
+			} else {
+				/* Number of command bytes exceeds
+				 * maximal allowable length
+				 */
+				ret = -EINVAL;
+				break;
+			}
+		}
+
+		for (col = 0u; col < count; col++) {
+			radio_cmd[col] = *configuration_data;
+			configuration_data++;
+		}
+
+		dev_dbg(port->dev, "%s: radio_cmd[0](%u)", __func__, radio_cmd[0]);
+		ret = si4455_send_command_get_response(port, count, radio_cmd,
+						       1, &response);
+		if (ret) {
+			dev_err(port->dev,
+				"%s: si4455_send_command_get_response error(%i)",
+				__func__, ret);
+			break;
+		}
+
+		/* Check response byte of EZCONFIG_CHECK command */
+		if (radio_cmd[0] == SI4455_CMD_ID_EZCONFIG_CHECK) {
+			if (response) {
+				/* Number of command bytes exceeds
+				 * maximal allowable length
+				 */
+				ret = -EIO;
+				dev_err(port->dev, "%s: EZConfig check error(%i)",
+					__func__, radio_cmd[0]);
+				break;
+			}
+		}
+
+		/* Get and clear all interrupts.  An error has occurred... */
+		si4455_get_int_status(port, 0, 0, 0, &int_status);
+		if (int_status.chip_pend
+		    & SI4455_CMD_GET_CHIP_STATUS_ERROR_PEND_MASK) {
+			ret = -EIO;
+			dev_err(port->dev, "%s: chip error(%i)",
+				__func__, int_status.chip_pend);
+			break;
+		}
+	}
+
+	return ret;
+}

...

Quoted 13 lines
drivers/tty/serial/si4455.c new file +13 -0
+static int si4455_do_work(struct uart_port *port)
+{
+	int ret = 0;
+	struct si4455_port *s = dev_get_drvdata(port->dev);
+	struct circ_buf *xmit = &port->state->xmit;
+	unsigned int tx_pending = 0;
+	unsigned int tx_to_end = 0;
+	u8 *data = NULL;
+
+	mutex_lock(&s->mutex);
+	dev_dbg(port->dev, "%s(connected=%i, configured=%i, power_count=%i)",
+		__func__, s->connected, s->configured, s->power_count);
+	if (s->connected && s->configured && s->power_count > 0) {

drivers/tty/serial/si4455.c line 722

Quoted 10 lines
drivers/tty/serial/si4455.c new file +10 -0
+		if (!(uart_circ_empty(xmit) || uart_tx_stopped(port) || s->tx_pending)) {
+			tx_pending = uart_circ_chars_pending(xmit);
+			if (s->package_size > 0) {
+				if (tx_pending >= s->package_size) {
+					tx_pending = s->package_size;
+					data = kzalloc(s->package_size, GFP_KERNEL);
+					tx_to_end = CIRC_CNT_TO_END(xmit->head,
+								    xmit->tail,
+								    UART_XMIT_SIZE);
+					if (tx_to_end < tx_pending) {

Again, too much spaghetti here.

Quoted 33 lines
drivers/tty/serial/si4455.c new file +33 -0
+						memcpy(data,
+						       xmit->buf + xmit->tail,
+						       tx_to_end);
+						memcpy(data,
+						       xmit->buf,
+						       tx_pending - tx_to_end);
+					} else {
+						memcpy(data,
+						       xmit->buf + xmit->tail,
+						       tx_pending);
+					}
+					if (si4455_begin_tx(port, s->tx_channel,
+							    tx_pending, data) == 0) {
+						s->tx_pending = true;
+					}
+					kfree(data);
+				}
+			} else {
+				//TODO: variable packet length
+			}
+		}
+		if (!s->tx_pending) {
+			if (s->package_size > 0) {
+				ret = si4455_begin_rx(port, s->rx_channel,
+						      s->package_size);
+			} else {
+				//TODO: variable packet length
+			}
+		}
+	}
+	mutex_unlock(&s->mutex);
+	return ret;
+}

...

drivers/tty/serial/si4455.c line 806

Quoted 4 lines
drivers/tty/serial/si4455.c new file +4 -0
+static irqreturn_t si4455_port_irq(struct si4455_port *s)
+{
+	struct uart_port *port = &s->one.port;
+	irqreturn_t ret = IRQ_NONE;

You never return IRQ_NONE, right?

Quoted 26 lines
drivers/tty/serial/si4455.c new file +26 -0
+	struct si4455_int_status int_status = { 0 };
+	struct si4455_fifo_info fifo_info = { 0 };
+
+	mutex_lock(&s->mutex);
+	if (s->connected && s->configured && s->power_count > 0) {
+		if (!si4455_get_int_status(port, 0, 0, 0, &int_status)) {
+			si4455_get_modem_status(port, 0, &s->modem_status);
+			if (int_status.chip_pend
+			    & SI4455_CMD_GET_CHIP_STATUS_ERROR_PEND_BIT) {
+				dev_err(port->dev,
+					"%s: chip_status:CMD_ERROR_PEND",
+					__func__);
+			} else if (int_status.ph_pend
+				   & SI4455_CMD_GET_INT_STATUS_PACKET_SENT_PEND_BIT) {
+				dev_dbg(port->dev,
+					"%s: ph_status:PACKET_SENT_PEND",
+					__func__);
+				si4455_handle_tx_pend(s);
+			} else if (int_status.ph_pend
+				   & SI4455_CMD_GET_INT_STATUS_PACKET_RX_PEND_BIT) {
+				dev_dbg(port->dev,
+					"%s: ph_status:PACKET_RX_PEND",
+					__func__);
+				s->current_rssi = s->modem_status.curr_rssi;
+				si4455_fifo_info(port, 0, &fifo_info);
+				si4455_handle_rx_pend(s);

drivers/tty/serial/si4455.c line 842

Quoted 10 lines
drivers/tty/serial/si4455.c new file +10 -0
+			} else if (int_status.ph_pend
+				   & SI4455_CMD_GET_INT_STATUS_CRC_ERROR_BIT) {
+				dev_dbg(port->dev,
+					"%s: ph_status:CRC_ERROR_PEND",
+					__func__);
+			}
+			ret = IRQ_HANDLED;
+		}
+	} else {
+		ret = IRQ_HANDLED;

How can this be IRQ_HANDLED when the device is off?

drivers/tty/serial/si4455.c line 849

Quoted 7 lines
drivers/tty/serial/si4455.c new file +7 -0
+	}
+	mutex_unlock(&s->mutex);
+	si4455_do_work(port);
+	return ret;
+}
+
+static irqreturn_t si4455_ist(int irq, void *dev_id)

Why is this wrapper needed?

drivers/tty/serial/si4455.c line 857

Quoted 8 lines
drivers/tty/serial/si4455.c new file +8 -0
+{
+	struct si4455_port *s = (struct si4455_port *)dev_id;
+	bool handled = false;
+
+	if (si4455_port_irq(s) == IRQ_HANDLED)
+		handled = true;
+
+	return IRQ_RETVAL(handled);

A weird way of just doing:
return si4455_port_irq(s);

Quoted 1 line

+}

...

drivers/tty/serial/si4455.c line 869

Quoted 3 lines
drivers/tty/serial/si4455.c new file +3 -0
+static unsigned int si4455_tx_empty(struct uart_port *port)
+{
+	return TIOCSER_TEMT;

Always free to send?

drivers/tty/serial/si4455.c line 875

Quoted 6 lines
drivers/tty/serial/si4455.c new file +6 -0
+}
+
+static unsigned int si4455_get_mctrl(struct uart_port *port)
+{
+	dev_dbg(port->dev, "%s", __func__);
+	return TIOCM_DSR | TIOCM_CAR;

And always carrier?

Quoted 1 line

+}

...

drivers/tty/serial/si4455.c line 952

Quoted 9 lines
drivers/tty/serial/si4455.c new file +9 -0
+static void si4455_start_tx(struct uart_port *port)
+{
+	struct si4455_one *one = container_of(port,
+					      struct si4455_one,
+					      port);
+
+	dev_dbg(port->dev, "%s", __func__);
+
+	if (!work_pending(&one->tx_work))

This if is pointless.

Quoted 2 lines
drivers/tty/serial/si4455.c new file +2 -0
+		schedule_work(&one->tx_work);
+}

...

drivers/tty/serial/si4455.c line 981

Quoted 5 lines
drivers/tty/serial/si4455.c new file +5 -0
+static int __maybe_unused si4455_suspend(struct device *dev)
+{
+	struct si4455_port *s = dev_get_drvdata(dev);
+
+	uart_suspend_port(&si4455_uart, &s->one.port);

Newline here.

Quoted 2 lines
drivers/tty/serial/si4455.c new file +2 -0
+	return 0;
+}

...

Quoted 64 lines
drivers/tty/serial/si4455.c new file +64 -0
+static int si4455_probe(struct device *dev,
+			int irq)
+{
+	int ret;
+	struct si4455_port *s;
+	const void *of_ptr;
+	char ez_fw_name[255] = { 0 };
+	const struct firmware *ez_fw = NULL;
+
+	/* Alloc port structure */
+	dev_dbg(dev, "%s\n", __func__);
+	s = devm_kzalloc(dev, sizeof(*s), GFP_KERNEL);
+	if (!s) {
+		dev_err(dev, "Error allocating port structure\n");
+		return -ENOMEM;
+	}
+
+	dev_set_drvdata(dev, s);
+	mutex_init(&s->mutex);
+
+	s->shdn_gpio = devm_gpiod_get(dev, "shutdown", GPIOD_OUT_HIGH);
+	if (IS_ERR(s->shdn_gpio)) {
+		dev_err(dev, "Unable to reguest shdn gpio\n");
+		ret = -EINVAL;
+		goto out_generic;
+	}
+
+	of_ptr = of_get_property(dev->of_node, "silabs,package-size", NULL);
+	if (IS_ERR_OR_NULL(of_ptr)) {
+		dev_err(dev, "dt silabs,package-size property not present\n");
+		ret = -EINVAL;
+		goto out_generic;
+	}
+	s->package_size = be32_to_cpup(of_ptr);
+	if (s->package_size > SI4455_FIFO_SIZE) {
+		dev_err(dev, "dt silabs,package-size property maximum is %i\n", SI4455_FIFO_SIZE);
+		ret = -EINVAL;
+		goto out_generic;
+	}
+
+	of_ptr = of_get_property(dev->of_node, "silabs,tx-channel", NULL);
+	if (IS_ERR_OR_NULL(of_ptr)) {
+		dev_err(dev, "dt silabs,tx-channel property not present\n");
+		ret = -EINVAL;
+		goto out_generic;
+	}
+	s->tx_channel = be32_to_cpup(of_ptr);
+
+	of_ptr = of_get_property(dev->of_node, "silabs,rx-channel", NULL);
+	if (IS_ERR_OR_NULL(of_ptr)) {
+		dev_err(dev, "dt silabs,rx-channel property not present\n");
+		ret = -EINVAL;
+		goto out_generic;
+	}
+	s->rx_channel = be32_to_cpup(of_ptr);
+
+	of_ptr = of_get_property(dev->of_node, "silabs,ez-config", NULL);
+	if (IS_ERR_OR_NULL(of_ptr)) {
+		dev_err(dev, "dt silabs,ez-config property not present\n");
+		ret = -EINVAL;
+		goto out_generic;
+	}
+	strncpy(ez_fw_name, of_ptr, sizeof(ez_fw_name) - 1);
+

drivers/tty/serial/si4455.c line 1175

Quoted 10 lines
drivers/tty/serial/si4455.c new file +10 -0
+	/* Initialize port data */
+	s->one.port.dev	= dev;
+	s->one.port.line = 0;
+	s->one.port.irq	= irq;
+	s->one.port.type	= PORT_SI4455;
+	s->one.port.fifosize	= SI4455_FIFO_SIZE;
+	s->one.port.flags	= UPF_FIXED_TYPE | UPF_LOW_LATENCY;
+	s->one.port.iotype	= UPIO_PORT;
+	s->one.port.iobase	= 0x00;
+	s->one.port.membase	= (void __iomem *)~0;

This is a cast of int to a pointer, isn't it?

Quoted 32 lines
drivers/tty/serial/si4455.c new file +32 -0
+	s->one.port.rs485_config = NULL;
+	s->one.port.ops	= &si4455_ops;
+
+	si4455_s_power(dev, 1);
+
+	//detect
+	ret = si4455_get_part_info(&s->one.port, &s->part_info);
+	dev_dbg(dev, "si4455_get_part_info()==%i", ret);
+	if (ret == 0) {
+		dev_dbg(dev, "partInfo.chip_rev= %u", s->part_info.chip_rev);
+		dev_dbg(dev, "partInfo.part= %u", s->part_info.part);
+		dev_dbg(dev, "partInfo.pbuild= %u", s->part_info.pbuild);
+		dev_dbg(dev, "partInfo.id= %u", s->part_info.id);
+		dev_dbg(dev, "partInfo.customer= %u", s->part_info.customer);
+		dev_dbg(dev, "partInfo.rom_id= %u", s->part_info.rom_id);
+		dev_dbg(dev, "partInfo.bond= %u", s->part_info.bond);
+		if (s->part_info.part != 0x5544) {
+			dev_err(dev, "part(%u) error", s->part_info.part);
+			ret = -ENODEV;
+		}
+	}
+
+	if (ret)
+		goto out_generic;
+
+	ret = request_firmware(&ez_fw, ez_fw_name, dev);
+	if (ret) {
+		dev_err(dev, "firmware(%s) request error(%i)\n", ez_fw_name, ret);
+		ret = -EINVAL;
+		goto out_generic;
+	}
+

drivers/tty/serial/si4455.c line 1217

Quoted 10 lines
drivers/tty/serial/si4455.c new file +10 -0
+	ret = si4455_re_configure(&s->one.port, ez_fw);
+	release_firmware(ez_fw);
+	if (ret) {
+		dev_err(dev, "device configuration error(%i)\n", ret);
+		ret = -EINVAL;
+		goto out_generic;
+	}
+
+	/* Initialize queue for start TX */
+	INIT_WORK(&s->one.tx_work, si4455_tx_proc);

Isn't this too late? It might not be right now. But I would do it right after allocation, along with others above.

Quoted 32 lines
drivers/tty/serial/si4455.c new file +32 -0
+
+	/* Register port */
+	ret = uart_add_one_port(&si4455_uart, &s->one.port);
+	if (ret) {
+		s->one.port.dev = NULL;
+		goto out_uart;
+	}
+
+	ret = sysfs_create_group(&dev->kobj, &si4455_attr_group);
+	if (ret) {
+		dev_err(dev, "sysfs_create_group error(%i)\n", ret);
+		goto out_uart;
+	}
+	/* Setup interrupt */
+	ret = devm_request_threaded_irq(dev, irq, NULL, si4455_ist,
+					IRQF_ONESHOT | IRQF_SHARED,
+					dev_name(dev), s);
+
+	if (!ret)
+		return 0;
+
+	dev_err(dev, "Unable to reguest IRQ %i\n", irq);
+	sysfs_remove_group(&dev->kobj, &si4455_attr_group);
+
+out_uart:
+	uart_remove_one_port(&si4455_uart, &s->one.port);
+out_generic:
+	mutex_destroy(&s->mutex);
+	si4455_s_power(dev, 0);
+
+	return ret;
+}

...

drivers/tty/serial/si4455.c line 1313

Quoted 8 lines
drivers/tty/serial/si4455.c new file +8 -0
+static int __init si4455_uart_init(void)
+{
+	int ret;
+
+	ret = uart_register_driver(&si4455_uart);
+	if (ret)
+		return ret;
+	spi_register_driver(&si4455_spi_driver);

spi_register_driver can fail too.

drivers/tty/serial/si4455.c line 306

Quoted 3 lines
drivers/tty/serial/si4455.c new file +3 -0
+
+	return 0;
+}

regards,

Signature · 1 line
js
#3 lore raw
Quoted 16 lines · Jiri Slaby
Quoted 9 lines · József Horváth

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

A description of changes between v1..v4 here, please.

Quoted 1 line

...

Quoted 9 lines

spi_register_driver can fail too.

drivers/tty/serial/si4455.c line 306

Quoted 3 lines
drivers/tty/serial/si4455.c new file +3 -0
+
+	return 0;
+}

regards,

Signature · 1 line
js

Thank you for your suggestions.

I'm in trouble with the device tree binding schema of this driver too.

When I run "make dt_binding_check" with $id: "http://devicetree.org/schemas/serial/silabs,si4455.yaml#" in schema,
and silabs,si4455.yaml is under Documentation/devicetree/bindings/serial/,
it completes successfully.

When I run "make dt_binding_check" with $id: "http://devicetree.org/schemas/staging/serial/silabs,si4455.yaml#" in schema,
and silabs,si4455.yaml is under Documentation/devicetree/bindings/staging/serial/,
the make dt_binding_check output:

Unknown file referenced: [Errno 2] No such file or directory: '/home/administrator/.local/lib/python3.6/site-packages/dtschema/schemas/staging/serial/serial.yaml'
xargs: dt-doc-validate: exited with status 255; aborting
Documentation/devicetree/bindings/Makefile:59: recipe for target 'Documentation/devicetree/bindings/processed-schema-examples.json' failed make[1]: *** [Documentation/devicetree/bindings/processed-schema-examples.json] Error 124
Makefile:1364: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2

When I run "make dt_binding_check" with $id: "http://devicetree.org/schemas/staging/serial/silabs,si4455.yaml#" in schema,
and silabs,si4455.yaml is under Documentation/devicetree/bindings/staging/serial/,
and removed $ref: "serial.yaml#".
The make dt_binding_check completes successfully, but this is not a good solution.

My question is, how can I use $ref: "serial.yaml" dependency in silabs,si4455.yaml, while my schema is under staging?
allOf:
- $ref: "serial.yaml#"

If you have any suggestion that brings me to the right direction it would be great.

Üdvözlettel / Best regards:
József Horváth

maintainer
#4 lore raw
Quoted 1 line · József Horváth

I'm in trouble with the device tree binding schema of this driver too.

Sorry, someone else has to help you who actually masters DT details.

Signature · 1 line
js
#5 lore raw
Quoted 7 lines · Jiri Slaby
Quoted 1 line · József Horváth

I'm in trouble with the device tree binding schema of this driver too.

Sorry, someone else has to help you who actually masters DT details.

Signature · 1 line
js

Thank you anyway.

Üdvözlettel / Best regards:
József Horváth

#6 lore raw
In reply to Jiri Slaby
Quoted 7 lines · Jiri Slaby
Quoted 1 line · József Horváth

I'm in trouble with the device tree binding schema of this driver too.

Sorry, someone else has to help you who actually masters DT details.

Signature · 1 line
js

I have answer. I forgot read before write...
The silabs,si4455.yaml was in wrong place, the good place is
Documentation/devicetree/bindings/serial/silabs,si4455.yaml as Rob wrote earlier.
Everything is fine now with it.

Sorry for this.

Üdvözlettel / Best regards:
József Horváth