Re: [PATCH] Staging: silabs si4455 serial driver
On Thu, Dec 10, 2020 at 12:20:10PM +, József Horváth wrote: > I send this again, because my original mail content was corrupted. > > This is a serial port driver for > Silicon Labs Si4455 Sub-GHz transciver. > > Signed-off-by: József Horváth > --- > .../bindings/staging/serial/silabs,si4455.txt | 39 + > 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 + I said I wasn't going to take this into drivers/staging/ so why is this still here? confused, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: silabs si4455 serial driver
I send this again, because my original mail content was corrupted.
This is a serial port driver for
Silicon Labs Si4455 Sub-GHz transciver.
Signed-off-by: József Horváth
---
.../bindings/staging/serial/silabs,si4455.txt | 39 +
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 ..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),
+ - "silabs,si4455b1a" for Silicon Labs Si4455-B1A,
+ - "silabs,si4455c2a" for Silicon Labs Si4455-C2A,
+- 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
+
+Example:
+
+/ {
+ clocks {
+si4455_1_2_osc: si4455_1_2_osc {
+compatible = "fixed-clock";
+#clock-cells = <0>;
+clock-frequency = <3000>;
+};
+ };
+};
+
+ {
+ si4455: si4455@0 {
+ compatible = "silabs,si4455";
+ reg = <0>;
+ clocks = <_1_2_osc>;
+ interrupt-parent = <>;
+ interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
+shdn-gpios = < 26 1>;
+status = "okay";
+spi-max-frequency = <300>;
+ };
+};
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 9b7cb7c5766a..2cf0c9bfe878 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -118,4 +118,6 @@ source "drivers/staging/wfx/Kconfig"
source "drivers/staging/hikey9xx/Kconfig"
+source "drivers/staging/si4455/Kconfig"
+
endif # STAGING
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 38226737c9f3..043c63287bc6 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -49,3 +49,4 @@ obj-$(CONFIG_QLGE)+= qlge/
obj-$(CONFIG_WIMAX)+= wimax/
obj-$(CONFIG_WFX) += wfx/
obj-y += hikey9xx/
+obj-$(CONFIG_SERIAL_SI4455)+= si4455/
diff --git a/drivers/staging/si4455/Kconfig b/drivers/staging/si4455/Kconfig
new file mode 100644
index ..666f726f2583
--- /dev/null
+++ b/drivers/staging/si4455/Kconfig
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+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.
diff --git a/drivers/staging/si4455/Makefile b/drivers/staging/si4455/Makefile
new file mode 100644
index ..1a6474673509
--- /dev/null
+++ b/drivers/staging/si4455/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_SERIAL_SI4455) := si4455.o
diff --git a/drivers/staging/si4455/TODO b/drivers/staging/si4455/TODO
new file mode 100644
index ..aee5c7613b31
--- /dev/null
+++ b/drivers/staging/si4455/TODO
@@ -0,0 +1,3 @@
+TODO:
+- add variable packet length support
+- add firmware patching support in case of Si4455-C2A
diff --git a/drivers/staging/si4455/si4455.c b/drivers/staging/si4455/si4455.c
new file mode 100644
index ..15f45f19ffdb
--- /dev/null
+++ b/drivers/staging/si4455/si4455.c
@@ -0,0 +1,1465 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 József Horváth
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
Re: [PATCH] Staging: silabs si4455 serial driver
On Wed, Dec 09, 2020 at 07:41:53PM +, József Horváth wrote:
> > > +static int si4455_get_part_info(struct uart_port *port,
> > > + struct si4455_part_info *result)
> > > +{
> > > + int ret;
> > > + u8 dataOut[] = { SI4455_CMD_ID_PART_INFO };
> > > + u8 dataIn[SI4455_CMD_REPLY_COUNT_PART_INFO];
> > > +
> > > + ret = si4455_send_command_get_response(port,
> > > + sizeof(dataOut),
> > > + dataOut,
> > > + sizeof(dataIn),
> > > + dataIn);
> >
> > Why not:
> >
>
> I changed all like this in my code already. I test it, and I'll send it again.
>
> Ps.: For my eyes is better to read line or list, reading table is harder :)
>
> line(arg1, arg2, arg3, arg4);
>
> list(arg1,
> arg2,
> arg3,
> arg4);
>
> table(arg1, arg2,
> arg3, arg4);
>
Use spaces to make arguments have to line up properly.
`checkpatch.pl --strict` will complain if it's not done.
table(arg1, arg2,
arg_whatver, foo);
[tab][space x 7]arg_whaver, foo);
But I think Jérôme's main point was to get rid of the dataOut buffer and
use "result" directly.
>
> >ret = si4455_send_command_get_response(port,
> > sizeof(*result), result,
> > sizeof(dataIn), dataIn);
> >
> > > + if (ret == 0) {
> > > + result->CHIPREV = dataIn[0];
> > > + memcpy(>PART, [1],sizeof(result->PART));
> > > + result->PBUILD = dataIn[3];
> > > + memcpy(>ID, [4], sizeof(result->ID));
> > > + result->CUSTOMER = dataIn[6];
> > > + result->ROMID = dataIn[7];
> > > + result->BOND = dataIn[8];
> >
> > ... it would avoid all these lines.
> >
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: silabs si4455 serial driver
On Wed, Dec 9, 2020 at 5:17 AM Info wrote:
>
> This is a serial port driver for
> Silicon Labs Si4455 Sub-GHz transciver.
>
> Signed-off-by: József Horváth
> ---
> .../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 ..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 = <3000>;
> +};
> + };
Don't need to show this for the example.
> +};
> +
> + {
> + si4455: si4455@0 {
> + compatible = "silabs,si4455";
> + reg = <0>;
> + clocks = <_1_2_osc>;
> + interrupt-parent = <>;
> + interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> +shdn-gpios = < 26 1>;
> +status = "okay";
Don't show status in examples.
> +spi-max-frequency = <300>;
> + };
> +};
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: silabs si4455 serial driver
Hello Jérôme,
On Wed, Dec 09, 2020 at 06:38:08PM +0100, Jérôme Pouiller wrote:
> On Wednesday 9 December 2020 12:09:58 CET Info wrote:
> >
> > This is a serial port driver for
> > Silicon Labs Si4455 Sub-GHz transciver.
>
> Hello József,
>
> Thank you for taking care of support of Silabs products :)
I think great products :) and great support :)
>
>
> > Signed-off-by: József Horváth
>
> I think you have to use your personal address to sign-off.
I'm a self-employed, currently this is my "personal" e-mail address.
>
> > ---
> > .../bindings/staging/serial/silabs,si4455.txt | 39 +
> > 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
>
> Since you add a new directory, you should also update MAINTAINERS file
> (checkpatch didn't warn you about that?).
>
>
> > 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 ..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
>
> AFAIK, Si4455 is a programmable product. So I think that this driver only
> work if the Si4455 use a specific firmware, isn't? In this case, you
> should mention it in the documentation.
Si4455 is programmed by silabs.
In case of C2A, it is possible to load patch.
My solution is to loading EZConfig(generated by WDS) and firmware patch with
ioctl calls.
You can see the definitions in si4455_api.h.
A short example for EZConfig loading(patch loading will be exacly the same if
Si4455 is rev C2A):
...
#include "si4455_api.h"
...
#include "radio.h" //< Generated by WDS3
#include "radio_config_Si4455.h"//< Generated by WDS3
...
struct si4455_iocbuff iocbuff = { 0 };
iocbuff.length = sizeof(Radio_Configuration_Data_Array);
memcpy(iocbuff.data, Radio_Configuration_Data_Array, iocbuff.length);
ret = ioctl(portFd, SI4455_IOC_SEZC, );
...
After SI4455_IOC_SEZC or SI4455_IOC_SEZP ioctl calls, the driver resets the
Si4455, and apply the configuration/patch.
>
>
> > +
> > +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),
> > + - "silabs,si4455b1a" for Silicon Labs Si4455-B1A,
> > + - "silabs,si4455c2a" for Silicon Labs Si4455-C2A,
> > +- 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
> > +
> > +Example:
> > +
> > +/ {
> > + clocks {
> > +si4455_1_2_osc: si4455_1_2_osc {
> > +compatible = "fixed-clock";
> > +#clock-cells = <0>;
> > +clock-frequency = <3000>;
> > +};
> > + };
> > +};
> > +
> > + {
> > + si4455: si4455@0 {
> > + compatible = "silabs,si4455";
> > + reg = <0>;
> > + clocks = <_1_2_osc>;
>
> It seems that the driver does not use this clock. So, is the clock
> attribute mandatory? What is the purpose of declaring a fixed-clock
> for this device?
>
Yes you are right, but the uart subsystem maybe. I'll check it again, and if
does not, I'll remove these definitions.
> > + interrupt-parent = <>;
> > + interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> > +shdn-gpios = < 26 1>;
> > +status = "okay";
> > +spi-max-frequency = <300>;
> > + };
> > +};
>
> [...]
>
>
> > diff --git a/drivers/staging/si4455/Kconfig b/drivers/staging/si4455/Kconfig
> > new file mode 100644
> > index
Re: [PATCH] Staging: silabs si4455 serial driver
On Wed, Dec 09, 2020 at 06:56:13PM +, József Horváth wrote: > Thank you for your suggestions. I made the canges suggested by you. > > I'll test the driver in my development environment, then I'll come > back with a new patch soon. Take your time. No rush. > > I set up a mail client on my linux dev environment, I hope my > mails/patches/codes will be in proper quality in the future. No problem at all. Part of getting used to the process. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: silabs si4455 serial driver
On Wed, Dec 09, 2020 at 03:42:06PM +0300, Dan Carpenter wrote:
> Change the From email header to say your name.
>
> The patch is corrupted and can't be applied. Please read the first
> paragraphs of Documentation/process/email-clients.rst
>
> This driver is pretty small and it might be easier to clean it up first
> before merging it into staging. Run checkpatch.pl --strict on the
> driver.
>
> > --- /dev/null
> > +++ b/drivers/staging/si4455/si4455.c
> > @@ -0,0 +1,1465 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2020 József Horváth
> > + *
> > + */
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include "si4455_api.h"
> > +
> > +#define PORT_SI44551096
> > +#define SI4455_NAME"si4455"
> > +#define SI4455_MAJOR 432
> > +#define SI4455_MINOR 567
> > +#define SI4455_UART_NRMAX 1
> > +#define SI4455_FIFO_SIZE 64
> > +
> > +#define SI4455_CMD_ID_EZCONFIG_CHECK 0x19
> > +#define SI4455_CMD_ID_PART_INFO0x01
> > +#define SI4455_CMD_REPLY_COUNT_PART_INFO 9
> > +#define SI4455_CMD_ID_GET_INT_STATUS 0x20
> > +#define SI4455_CMD_REPLY_COUNT_GET_INT_STATUS 8
> > +#define SI4455_CMD_ID_FIFO_INFO0x15
> > +#define SI4455_CMD_ARG_COUNT_FIFO_INFO 2
> > +#define SI4455_CMD_REPLY_COUNT_FIFO_INFO 2
> > +#define SI4455_CMD_FIFO_INFO_ARG_TX_BIT0x01
> > +#define SI4455_CMD_FIFO_INFO_ARG_RX_BIT0x02
> > +#define SI4455_CMD_ID_READ_CMD_BUFF0x44
> > +#define SI4455_CMD_ID_READ_RX_FIFO 0x77
> > +#define SI4455_CMD_ID_WRITE_TX_FIFO0x66
> > +#define SI4455_CMD_ID_START_RX 0x32
> > +#define SI4455_CMD_ARG_COUNT_START_RX 8
> > +#define SI4455_CMD_START_RX_ARG_RXTIMEOUT_STATE_ENUM_RX8
> > +#define SI4455_CMD_START_RX_ARG_RXVALID_STATE_ENUM_RX 8
> > +#define SI4455_CMD_START_RX_ARG_RXINVALID_STATE_ENUM_RX8
> > +#define SI4455_CMD_ID_START_TX 0x31
> > +#define SI4455_CMD_ARG_COUNT_START_TX 5
> > +#define SI4455_CMD_ID_CHANGE_STATE 0x34
> > +#define SI4455_CMD_ARG_COUNT_CHANGE_STATE 2
> > +#define SI4455_CMD_CHANGE_STATE_ARG_NEW_STATE_ENUM_READY 3
> > +#define SI4455_CMD_GET_CHIP_STATUS_REP_CMD_ERROR_PEND_MASK 0x08
> > +#define SI4455_CMD_GET_CHIP_STATUS_REP_CMD_ERROR_PEND_BIT 0x08
> > +#define SI4455_CMD_GET_INT_STATUS_REP_PACKET_SENT_PEND_BIT 0x20
> > +#define SI4455_CMD_GET_INT_STATUS_REP_PACKET_RX_PEND_BIT 0x10
> > +#define SI4455_CMD_GET_INT_STATUS_REP_CRC_ERROR_BIT0x08
>
> These names are unreadably long and just make the code messy.
>
> > +#define SI4455_CMD_ID_GET_MODEM_STATUS 0x22
> > +#define SI4455_CMD_ARG_COUNT_GET_MODEM_STATUS 2
> > +#define SI4455_CMD_REPLY_COUNT_GET_MODEM_STATUS8
> > +
> > +struct si4455_part_info {
> > + u8 CHIPREV;
> > + u16 PART;
> > + u8 PBUILD;
> > + u16 ID;
> > + u8 CUSTOMER;
> > + u8 ROMID;
> > + u8 BOND;
>
> Also the huge gap between the type and the struct member makes it
> hard to read.
>
> u8 chip_rev;
> u16 part;
> u8 pbuild;
>
> etc.
>
> > +};
> > +
> > +struct si4455_int_status {
> > + u8 INT_PEND;
> > + u8 INT_STATUS;
> > + u8 PH_PEND;
> > + u8 PH_STATUS;
> > + u8 MODEM_PEND;
> > + u8 MODEM_STATUS;
> > + u8 CHIP_PEND;
> > + u8 CHIP_STATUS;
> > +};
> > +
> > +struct si4455_modem_status {
> > + u8 MODEM_PEND;
> > + u8 MODEM_STATUS;
> > + u8 CURR_RSSI;
> > + u8 LATCH_RSSI;
> > + u8 ANT1_RSSI;
> > + u8
Re: [PATCH] Staging: silabs si4455 serial driver
On Wednesday 9 December 2020 12:09:58 CET Info wrote:
>
> This is a serial port driver for
> Silicon Labs Si4455 Sub-GHz transciver.
Hello József,
Thank you for taking care of support of Silabs products :)
> Signed-off-by: József Horváth
I think you have to use your personal address to sign-off.
> ---
> .../bindings/staging/serial/silabs,si4455.txt | 39 +
> 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
Since you add a new directory, you should also update MAINTAINERS file
(checkpatch didn't warn you about that?).
> 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 ..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
AFAIK, Si4455 is a programmable product. So I think that this driver only
work if the Si4455 use a specific firmware, isn't? In this case, you
should mention it in the documentation.
> +
> +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),
> + - "silabs,si4455b1a" for Silicon Labs Si4455-B1A,
> + - "silabs,si4455c2a" for Silicon Labs Si4455-C2A,
> +- 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
> +
> +Example:
> +
> +/ {
> + clocks {
> +si4455_1_2_osc: si4455_1_2_osc {
> +compatible = "fixed-clock";
> +#clock-cells = <0>;
> +clock-frequency = <3000>;
> +};
> + };
> +};
> +
> + {
> + si4455: si4455@0 {
> + compatible = "silabs,si4455";
> + reg = <0>;
> + clocks = <_1_2_osc>;
It seems that the driver does not use this clock. So, is the clock
attribute mandatory? What is the purpose of declaring a fixed-clock
for this device?
> + interrupt-parent = <>;
> + interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> +shdn-gpios = < 26 1>;
> +status = "okay";
> +spi-max-frequency = <300>;
> + };
> +};
[...]
> diff --git a/drivers/staging/si4455/Kconfig b/drivers/staging/si4455/Kconfig
> new file mode 100644
> index ..666f726f2583
> --- /dev/null
> +++ b/drivers/staging/si4455/Kconfig
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: GPL-2.0
> +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.
So, in fact, Si4455 is not a UART. I don't know how this kind of device
should be presented to the userspace. Have you check if similar devices
already exists in the kernel?
I suggest to add linux-w...@vger.kernel.org to the recipients of your
patch.
[...]
> +static int si4455_get_part_info(struct uart_port *port,
> + struct si4455_part_info *result)
> +{
> + int ret;
> + u8 dataOut[] = { SI4455_CMD_ID_PART_INFO };
> + u8 dataIn[SI4455_CMD_REPLY_COUNT_PART_INFO];
> +
> + ret = si4455_send_command_get_response(port,
> + sizeof(dataOut),
> + dataOut,
> + sizeof(dataIn),
> + dataIn);
Why not:
ret = si4455_send_command_get_response(port,
sizeof(*result), result,
sizeof(dataIn), dataIn);
>
RE: ***UNCHECKED*** Re: [PATCH] Staging: silabs si4455 serial driver
> On Wed, Dec 09, 2020 at 12:09:58PM +0100, Info wrote: > > This is a serial port driver for > > Silicon Labs Si4455 Sub-GHz transciver. > > > > Signed-off-by: József Horváth > > Note, your From: line does not match this line, so I can't take this. > > But: > > > --- > > .../bindings/staging/serial/silabs,si4455.txt | 39 + I'll fix this, sorry. > > staging drivers need to be self-contained, so this should be here. It needs > to be reviewed by the DT maintainers when moving out of staging. > > > index ..aee5c7613b31 > > --- /dev/null > > +++ b/drivers/staging/si4455/TODO > > @@ -0,0 +1,3 @@ > > +TODO: > > +- add variable packet length support > > +- add firmware patching support in case of Si4455-C2A > > Why are these a requirement to get it out of staging? Why go into staging at > all? Why not go into the 'real' part of the kernel directly? > What is keeping that from happening today? > > These look like new features that you can add later, and shouldn't be a > requirement for acceptance into the normal part of the kernel for this > driver. Why have you not tried doing that first? Yes, you are right. The TODO list is for me, and the driver can get out of staging without these. The curent state of the driver is completly covers my requirements, but I would complete these functions int he future. The main reason for not implementing the firmware patching feature is, currently I have no Si4455-C2A in my development system. My product is based on Si4455-B1A, but I'm waiting for the Si4455-C2A module. > thanks, > > greg k-h Üdvözlettel / Best regards: József Horváth ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: silabs si4455 serial driver
Change the From email header to say your name.
The patch is corrupted and can't be applied. Please read the first
paragraphs of Documentation/process/email-clients.rst
This driver is pretty small and it might be easier to clean it up first
before merging it into staging. Run checkpatch.pl --strict on the
driver.
> --- /dev/null
> +++ b/drivers/staging/si4455/si4455.c
> @@ -0,0 +1,1465 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 József Horváth
> + *
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "si4455_api.h"
> +
> +#define PORT_SI4455 1096
> +#define SI4455_NAME "si4455"
> +#define SI4455_MAJOR 432
> +#define SI4455_MINOR 567
> +#define SI4455_UART_NRMAX1
> +#define SI4455_FIFO_SIZE 64
> +
> +#define SI4455_CMD_ID_EZCONFIG_CHECK 0x19
> +#define SI4455_CMD_ID_PART_INFO 0x01
> +#define SI4455_CMD_REPLY_COUNT_PART_INFO 9
> +#define SI4455_CMD_ID_GET_INT_STATUS 0x20
> +#define SI4455_CMD_REPLY_COUNT_GET_INT_STATUS8
> +#define SI4455_CMD_ID_FIFO_INFO 0x15
> +#define SI4455_CMD_ARG_COUNT_FIFO_INFO 2
> +#define SI4455_CMD_REPLY_COUNT_FIFO_INFO 2
> +#define SI4455_CMD_FIFO_INFO_ARG_TX_BIT 0x01
> +#define SI4455_CMD_FIFO_INFO_ARG_RX_BIT 0x02
> +#define SI4455_CMD_ID_READ_CMD_BUFF 0x44
> +#define SI4455_CMD_ID_READ_RX_FIFO 0x77
> +#define SI4455_CMD_ID_WRITE_TX_FIFO 0x66
> +#define SI4455_CMD_ID_START_RX 0x32
> +#define SI4455_CMD_ARG_COUNT_START_RX8
> +#define SI4455_CMD_START_RX_ARG_RXTIMEOUT_STATE_ENUM_RX 8
> +#define SI4455_CMD_START_RX_ARG_RXVALID_STATE_ENUM_RX8
> +#define SI4455_CMD_START_RX_ARG_RXINVALID_STATE_ENUM_RX 8
> +#define SI4455_CMD_ID_START_TX 0x31
> +#define SI4455_CMD_ARG_COUNT_START_TX5
> +#define SI4455_CMD_ID_CHANGE_STATE 0x34
> +#define SI4455_CMD_ARG_COUNT_CHANGE_STATE2
> +#define SI4455_CMD_CHANGE_STATE_ARG_NEW_STATE_ENUM_READY 3
> +#define SI4455_CMD_GET_CHIP_STATUS_REP_CMD_ERROR_PEND_MASK 0x08
> +#define SI4455_CMD_GET_CHIP_STATUS_REP_CMD_ERROR_PEND_BIT0x08
> +#define SI4455_CMD_GET_INT_STATUS_REP_PACKET_SENT_PEND_BIT 0x20
> +#define SI4455_CMD_GET_INT_STATUS_REP_PACKET_RX_PEND_BIT 0x10
> +#define SI4455_CMD_GET_INT_STATUS_REP_CRC_ERROR_BIT 0x08
These names are unreadably long and just make the code messy.
> +#define SI4455_CMD_ID_GET_MODEM_STATUS 0x22
> +#define SI4455_CMD_ARG_COUNT_GET_MODEM_STATUS2
> +#define SI4455_CMD_REPLY_COUNT_GET_MODEM_STATUS 8
> +
> +struct si4455_part_info {
> + u8 CHIPREV;
> + u16 PART;
> + u8 PBUILD;
> + u16 ID;
> + u8 CUSTOMER;
> + u8 ROMID;
> + u8 BOND;
Also the huge gap between the type and the struct member makes it
hard to read.
u8 chip_rev;
u16 part;
u8 pbuild;
etc.
> +};
> +
> +struct si4455_int_status {
> + u8 INT_PEND;
> + u8 INT_STATUS;
> + u8 PH_PEND;
> + u8 PH_STATUS;
> + u8 MODEM_PEND;
> + u8 MODEM_STATUS;
> + u8 CHIP_PEND;
> + u8 CHIP_STATUS;
> +};
> +
> +struct si4455_modem_status {
> + u8 MODEM_PEND;
> + u8 MODEM_STATUS;
> + u8 CURR_RSSI;
> + u8 LATCH_RSSI;
> + u8 ANT1_RSSI;
> + u8 ANT2_RSSI;
> + u16 AFC_FREQ_OFFSET;
> +};
> +
> +struct si4455_fifo_info {
> + u8 RX_FIFO_COUNT;
> + u8 TX_FIFO_SPACE;
> +};
> +
> +struct si4455_one {
> +
Re: [PATCH] Staging: silabs si4455 serial driver
On Wed, Dec 09, 2020 at 12:09:58PM +0100, Info wrote: > This is a serial port driver for > Silicon Labs Si4455 Sub-GHz transciver. > > Signed-off-by: József Horváth Note, your From: line does not match this line, so I can't take this. But: > --- > .../bindings/staging/serial/silabs,si4455.txt | 39 + staging drivers need to be self-contained, so this should be here. It needs to be reviewed by the DT maintainers when moving out of staging. > index ..aee5c7613b31 > --- /dev/null > +++ b/drivers/staging/si4455/TODO > @@ -0,0 +1,3 @@ > +TODO: > +- add variable packet length support > +- add firmware patching support in case of Si4455-C2A Why are these a requirement to get it out of staging? Why go into staging at all? Why not go into the 'real' part of the kernel directly? What is keeping that from happening today? These look like new features that you can add later, and shouldn't be a requirement for acceptance into the normal part of the kernel for this driver. Why have you not tried doing that first? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: silabs si4455 serial driver
This is a serial port driver for
Silicon Labs Si4455 Sub-GHz transciver.
Signed-off-by: József Horváth
---
.../bindings/staging/serial/silabs,si4455.txt | 39 +
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 ..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),
+ - "silabs,si4455b1a" for Silicon Labs Si4455-B1A,
+ - "silabs,si4455c2a" for Silicon Labs Si4455-C2A,
+- 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
+
+Example:
+
+/ {
+ clocks {
+si4455_1_2_osc: si4455_1_2_osc {
+compatible = "fixed-clock";
+#clock-cells = <0>;
+clock-frequency = <3000>;
+};
+ };
+};
+
+ {
+ si4455: si4455@0 {
+ compatible = "silabs,si4455";
+ reg = <0>;
+ clocks = <_1_2_osc>;
+ interrupt-parent = <>;
+ interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
+shdn-gpios = < 26 1>;
+status = "okay";
+spi-max-frequency = <300>;
+ };
+};
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 9b7cb7c5766a..2cf0c9bfe878 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -118,4 +118,6 @@ source "drivers/staging/wfx/Kconfig"
source "drivers/staging/hikey9xx/Kconfig"
+source "drivers/staging/si4455/Kconfig"
+
endif # STAGING
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 38226737c9f3..043c63287bc6 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -49,3 +49,4 @@ obj-$(CONFIG_QLGE)+= qlge/
obj-$(CONFIG_WIMAX)+= wimax/
obj-$(CONFIG_WFX) += wfx/
obj-y += hikey9xx/
+obj-$(CONFIG_SERIAL_SI4455)+= si4455/
diff --git a/drivers/staging/si4455/Kconfig b/drivers/staging/si4455/Kconfig
new file mode 100644
index ..666f726f2583
--- /dev/null
+++ b/drivers/staging/si4455/Kconfig
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+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.
diff --git a/drivers/staging/si4455/Makefile
b/drivers/staging/si4455/Makefile
new file mode 100644
index ..1a6474673509
--- /dev/null
+++ b/drivers/staging/si4455/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_SERIAL_SI4455) := si4455.o
diff --git a/drivers/staging/si4455/TODO b/drivers/staging/si4455/TODO
new file mode 100644
index ..aee5c7613b31
--- /dev/null
+++ b/drivers/staging/si4455/TODO
@@ -0,0 +1,3 @@
+TODO:
+- add variable packet length support
+- add firmware patching support in case of Si4455-C2A
diff --git a/drivers/staging/si4455/si4455.c
b/drivers/staging/si4455/si4455.c
new file mode 100644
index ..15f45f19ffdb
--- /dev/null
+++ b/drivers/staging/si4455/si4455.c
@@ -0,0 +1,1465 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 József Horváth
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "si4455_api.h"
+
+#define PORT_SI4455
