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(&result->PART, &dataIn[1],sizeof(result->PART)); > > > + result->PBUILD = dataIn[3]; > > > + memcpy(&result->ID, &dataIn[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
[PATCH] Staging: ralink-gdma: ralink-gdma: Fix a blank line coding style issue
Fix a coding style issue as identified by checkpatch.pl Signed-off-by: Chris Bloomfield --- drivers/staging/ralink-gdma/ralink-gdma.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c b/drivers/staging/ralink-gdma/ralink-gdma.c index 655df317d0ee..a6181a167814 100644 --- a/drivers/staging/ralink-gdma/ralink-gdma.c +++ b/drivers/staging/ralink-gdma/ralink-gdma.c @@ -122,6 +122,7 @@ struct gdma_dma_dev { struct gdma_data *data; void __iomem *base; struct tasklet_struct task; + volatile unsigned long chan_issued; atomic_t cnt; -- 2.28.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: ralink-gdma: ralink-gdma: Fix a blank line coding style issue
On Thu, Dec 10, 2020 at 10:06:57AM +, Chris Bloomfield wrote: > Fix a coding style issue as identified by checkpatch.pl > > Signed-off-by: Chris Bloomfield > --- > drivers/staging/ralink-gdma/ralink-gdma.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c > b/drivers/staging/ralink-gdma/ralink-gdma.c > index 655df317d0ee..a6181a167814 100644 > --- a/drivers/staging/ralink-gdma/ralink-gdma.c > +++ b/drivers/staging/ralink-gdma/ralink-gdma.c > @@ -122,6 +122,7 @@ struct gdma_dma_dev { > struct gdma_data *data; > void __iomem *base; > struct tasklet_struct task; > + > volatile unsigned long chan_issued; > atomic_t cnt; > With your knowledge of C, do you think the above change looks correct? thanks, 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>; +}; + }; +}; + +&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"; +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
[PATCH v1 1/2] Staging: silabs si4455 serial driver: fix directory structure and coding style
fix: coding style fix: error checking remove: doc silabs,si4455.txt Signed-off-by: József Horváth --- .../bindings/staging/serial/silabs,si4455.txt | 39 - MAINTAINERS | 7 + drivers/staging/Kconfig | 2 - drivers/staging/Makefile | 1 - drivers/tty/serial/Kconfig| 8 + drivers/tty/serial/Makefile | 1 + .../{staging/si4455 => tty/serial}/si4455.c | 956 -- .../si4455 => tty/serial}/si4455_api.h| 0 8 files changed, 419 insertions(+), 595 deletions(-) delete mode 100644 Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt rename drivers/{staging/si4455 => tty/serial}/si4455.c (57%) rename drivers/{staging/si4455 => tty/serial}/si4455_api.h (100%) diff --git a/Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt b/Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt deleted file mode 100644 index abd659b7b952.. --- a/Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt +++ /dev/null @@ -1,39 +0,0 @@ -* 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>; -}; - }; -}; - -&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"; -spi-max-frequency = <300>; - }; -}; diff --git a/MAINTAINERS b/MAINTAINERS index 2ac5688db43a..a29bc17d446d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -15934,6 +15934,13 @@ S: Maintained F: drivers/input/touchscreen/silead.c F: drivers/platform/x86/touchscreen_dmi.c +SILICON LABS SI4455 SERIAL DRIVER +M: József Horváth +S: Maintained +F: Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt +F: drivers/tty/serial/si4455.c +F: drivers/tty/serial/si4455_api.h + SILICON LABS WIRELESS DRIVERS (for WFxxx series) M: Jérôme Pouiller S: Supported diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig index 2cf0c9bfe878..9b7cb7c5766a 100644 --- a/drivers/staging/Kconfig +++ b/drivers/staging/Kconfig @@ -118,6 +118,4 @@ 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 043c63287bc6..38226737c9f3 100644 --- a/drivers/staging/Makefile +++ b/drivers/staging/Makefile @@ -49,4 +49,3 @@ obj-$(CONFIG_QLGE)+= qlge/ obj-$(CONFIG_WIMAX)+= wimax/ obj-$(CONFIG_WFX) += wfx/ obj-y += hikey9xx/ -obj-$(CONFIG_SERIAL_SI4455)+= si4455/ diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig index 28f22e58639c..560aa311cd03 100644 --- a/drivers/tty/serial/Kconfig +++ b/drivers/tty/serial/Kconfig @@ -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 diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile index caf167f0c10a..94f8a727b2a1 100644 --- a/drivers/tty/serial/Makefile +++ b/drivers/tty/serial/Makefile @@ -96,3 +96,4 @@ obj-$(CONFIG_SERIAL_MCTRL_GPIO) += serial_mctrl_gpio.o obj-$(CONFIG_SERIAL_KGDB_NMI) += kgdb_nmi.o obj-$(CONFIG_KGDB_SERIAL_CONSOLE) += kgdboc.o +obj-$(CONFIG_SERIAL_SI4455) += si4455.o diff --git a/drivers/staging/si4455/si4455.c b/drivers/tty/serial/si4455.c similarity index 57% rename from drivers/staging/s
[PATCH v1 2/2] Staging: silabs si4455 serial driver: docs device tree binding
add: device tree binding schema Signed-off-by: József Horváth --- .../bindings/serial/silabs,si4455.yaml| 53 +++ MAINTAINERS | 2 +- 2 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/serial/silabs,si4455.yaml diff --git a/Documentation/devicetree/bindings/serial/silabs,si4455.yaml b/Documentation/devicetree/bindings/serial/silabs,si4455.yaml new file mode 100644 index ..80a73a61755b --- /dev/null +++ b/Documentation/devicetree/bindings/serial/silabs,si4455.yaml @@ -0,0 +1,53 @@ +# 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 + +allOf: + - $ref: "/schemas/serial.yaml#" + +properties: + compatible: +const: silabs,si4455 + + reg: +maxItems: 1 + + interrupts: +maxItems: 1 + + spi-max-frequency: +description: maximum clock frequency on SPI port +maximum: 50 + + shutdown-gpios: +description: gpio pin for SDN +maxItems: 1 + +required: + - reg + - interrupts + - spi-max-frequency + - shutdown-gpios + +additionalProperties: false + +examples: + - | +&spi0 { + serial0: si4455@0 { +compatible = "silabs,si4455"; +reg = <0>; +interrupt-parent = <&gpio>; +interrupts = <7 IRQ_TYPE_LEVEL_LOW>; +shutdown-gpios = <&gpio 26 1>; +spi-max-frequency = <30>; + }; +}; +... diff --git a/MAINTAINERS b/MAINTAINERS index a29bc17d446d..16cc96971ac2 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -15937,7 +15937,7 @@ F: drivers/platform/x86/touchscreen_dmi.c SILICON LABS SI4455 SERIAL DRIVER M: József Horváth S: Maintained -F: Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt +F: Documentation/devicetree/bindings/serial/silabs,si4455.yaml F: drivers/tty/serial/si4455.c F: drivers/tty/serial/si4455_api.h -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1 1/2] Staging: silabs si4455 serial driver: fix directory structure and coding style
On Thu, Dec 10, 2020 at 12:20:59PM +, József Horváth wrote: > fix: coding style > fix: error checking > remove: doc silabs,si4455.txt What does all of this mean? Please read the documentation for how to write an effective changelog text, and where to put the "changes from the first version" text at. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
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
Re: [PATCH v2 1/2] dt-bindings: pinctrl: rt2880: add binding document
On Tue, Dec 08, 2020 at 08:55:22AM +0100, Sergio Paracuellos wrote: > The commit adds rt2880 compatible node in binding document. > > Signed-off-by: Sergio Paracuellos > --- > .../pinctrl/ralink,rt2880-pinmux.yaml | 70 +++ > 1 file changed, 70 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml > > diff --git > a/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml > b/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml > new file mode 100644 > index ..7dea3e26d99e > --- /dev/null > +++ b/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml > @@ -0,0 +1,70 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pinctrl/ralink,rt2880-pinmux.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Ralink rt2880 pinmux controller > + > +maintainers: > + - Sergio Paracuellos > + > +description: > + The rt2880 pinmux can only set the muxing of pin groups. muxing indiviual > pins > + is not supported. There is no pinconf support. > + > +properties: > + compatible: > +enum: > + - ralink,rt2880-pinmux What's the control interface as you have no 'reg' property. > + > + pinctrl-0: > +description: > + A phandle to the node containing the subnodes containing default > + configurations. This is for pinctrl hogs. > + > + pinctrl-names: > +description: > + A pinctrl state named "default" can be defined. > +const: default These 2 properties go in consumer nodes. > + > +required: > + - compatible > + > +patternProperties: > + '[a-z0-9_-]+': > +if: > + type: object > + description: node for pinctrl. > + $ref: "pinmux-node.yaml" > +then: For new bindings, don't do this hack. Just name the nodes '-pins$' > + properties: > +groups: > + description: Name of the pin group to use for the functions. > + enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio, > + pcie, sdhci] > +function: > + description: The mux function to select > + enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, > + mdio, nand1, nand2, sdhci] additionalProperties: false > + > +additionalProperties: false > + > +examples: > + # Pinmux controller node > + - | > +pinctrl { > + compatible = "ralink,rt2880-pinmux"; > + pinctrl-names = "default"; > + pinctrl-0 = <&state_default>; > + > + state_default: pinctrl0 { > + }; > + > + i2c_pins: i2c0 { > +i2c0 { > + groups = "i2c"; > + function = "i2c"; > +}; > + }; > +}; > -- > 2.25.1 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Re: [PATCH] staging: rtl8712: check register_netdev() return value
Hi > > This function should not be calling register_netdev(). What does that > have to do with firmware? It should also not free_netdev() because > that will just lead to a use after free in the caller. > --> check code history author changed synchronous firmware loading to asynchronous firmware loading before this change, register_netdev() was not calling in firmware related function. For asynchronous loading, maybe register_netdev() be calling in rtl871x_load_fw_cb() is to ensure the netdev be registered after firmware loading completed --> for potential use after free issue Could I only call "free_irq(adapter->pnetdev->irq, adapter->pnetdev)" when register_netdev() failed ? If no need to change drivers/staging/rtl8712/hal_init.c file, I could give up my patch, thank you ! > -原始邮件- > 发件人: "Dan Carpenter" > 发送时间: 2020-12-10 01:46:15 (星期四) > 收件人: shaojie.d...@isrc.iscas.ac.cn > 抄送: larry.fin...@lwfinger.net, florian.c.schilha...@googlemail.com, gre...@linuxfoundation.org, de...@driverdev.osuosl.org, linux-ker...@vger.kernel.org > 主题: Re: [PATCH] staging: rtl8712: check register_netdev() return value > > On Wed, Dec 09, 2020 at 11:01:24PM +0800, shaojie.d...@isrc.iscas.ac.cn wrote: > > From: "shaojie.dong" > > > > Function register_netdev() can fail, so we should check it's return value > > > > Signed-off-by: shaojie.dong > > --- > > drivers/staging/rtl8712/hal_init.c | 5 - > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/staging/rtl8712/hal_init.c b/drivers/staging/rtl8712/hal_init.c > > index 715f1fe8b..38a3e3d44 100644 > > --- a/drivers/staging/rtl8712/hal_init.c > > +++ b/drivers/staging/rtl8712/hal_init.c > > @@ -45,7 +45,10 @@ static void rtl871x_load_fw_cb(const struct firmware *firmware, void *context) > > } > > adapter->fw = firmware; > > /* firmware available - start netdev */ > > - register_netdev(adapter->pnetdev); > > + if (register_netdev(adapter->pnetdev) != 0) { > > + netdev_err(adapter->pnetdev, "register_netdev() failed\n"); > > + free_netdev(adapter->pnetdev); > > + } > > This function should not be calling register_netdev(). What does that > have to do with firmware? It should also not free_netdev() because > that will just lead to a use after free in the caller. > > regards, > dan carpenter > > > complete(&adapter->rtl8712_fw_ready); > > } > > > > -- > > 2.17.1 > > > > ___ > > devel mailing list > > de...@linuxdriverproject.org > > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Re: [PATCH] staging: rtl8712: check register_netdev() return value
Hi Thanks ! I will modify sign name correctly later Sorry to say that I have no rtl8712 hardware, so that I could not test it. From Dan Carpenter's email reply, "free_netdev(adapter->pnetdev)" function may cause use after free issue So that I reply email to ensure if this return value should be check or how to handle this return value error > -原始邮件- > 发件人: "Greg KH" > 发送时间: 2020-12-09 23:13:40 (星期三) > 收件人: shaojie.d...@isrc.iscas.ac.cn > 抄送: larry.fin...@lwfinger.net, florian.c.schilha...@googlemail.com, de...@driverdev.osuosl.org, linux-ker...@vger.kernel.org > 主题: Re: [PATCH] staging: rtl8712: check register_netdev() return value > > On Wed, Dec 09, 2020 at 11:01:24PM +0800, shaojie.d...@isrc.iscas.ac.cn wrote: > > From: "shaojie.dong" > > > > Function register_netdev() can fail, so we should check it's return value > > > > Signed-off-by: shaojie.dong > > I doubt you sign your name with a '.' in it, right? > > Please resend with the correct name, and using Capital letters where > needed. > > > --- > > drivers/staging/rtl8712/hal_init.c | 5 - > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/staging/rtl8712/hal_init.c b/drivers/staging/rtl8712/hal_init.c > > index 715f1fe8b..38a3e3d44 100644 > > --- a/drivers/staging/rtl8712/hal_init.c > > +++ b/drivers/staging/rtl8712/hal_init.c > > @@ -45,7 +45,10 @@ static void rtl871x_load_fw_cb(const struct firmware *firmware, void *context) > > } > > adapter->fw = firmware; > > /* firmware available - start netdev */ > > - register_netdev(adapter->pnetdev); > > + if (register_netdev(adapter->pnetdev) != 0) { > > + netdev_err(adapter->pnetdev, "register_netdev() failed\n"); > > + free_netdev(adapter->pnetdev); > > + } > > Did you test this to see if this really properly cleans everything up? > > And your if statement can be simplified, please do so. > > thanks, > > greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Re: [PATCH] staging: rtl8712: check register_netdev() return value
On Thu, Dec 10, 2020 at 11:05:34PM +0800, shaojie.d...@isrc.iscas.ac.cn wrote: > Hi > > > > > This function should not be calling register_netdev(). What does that > > have to do with firmware? It should also not free_netdev() because > > that will just lead to a use after free in the caller. > > > > --> check code history author changed > synchronous firmware loading to asynchronous firmware loading > before this change, register_netdev() was not calling in firmware related > function. > For asynchronous loading, maybe register_netdev() be calling in > rtl871x_load_fw_cb() is to ensure the netdev be registered after firmware > loading completed > > --> for potential use after free issue > Could I only call "free_irq(adapter->pnetdev->irq, > adapter->pnetdev)" when register_netdev() failed ? > If no need to change drivers/staging/rtl8712/hal_init.c file, I could > give up my patch, thank you ! > Cleaning this up is a bit complicated and requires reworking the firmware loading and it requires testing. I don't think you have the hardware to actually test this driver? Probably, just leave this code for another day. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Re: Re: [PATCH] staging: rtl8712: check register_netdev() return value
Hi I do not have rtl8712 hardware So that I would remain this code and give up my patch Thank you ! > -原始邮件- > 发件人: "Dan Carpenter" > 发送时间: 2020-12-10 23:16:31 (星期四) > 收件人: shaojie.d...@isrc.iscas.ac.cn > 抄送: larry.fin...@lwfinger.net, florian.c.schilha...@googlemail.com, gre...@linuxfoundation.org, de...@driverdev.osuosl.org, linux-ker...@vger.kernel.org > 主题: Re: Re: [PATCH] staging: rtl8712: check register_netdev() return value > > On Thu, Dec 10, 2020 at 11:05:34PM +0800, shaojie.d...@isrc.iscas.ac.cn wrote: > > Hi > > > > > > > > This function should not be calling register_netdev(). What does that > > > have to do with firmware? It should also not free_netdev() because > > > that will just lead to a use after free in the caller. > > > > > > > --> check code history author changed synchronous firmware loading to asynchronous firmware loading > > before this change, register_netdev() was not calling in firmware related function. > > For asynchronous loading, maybe register_netdev() be calling in rtl871x_load_fw_cb() is to ensure the netdev be registered after firmware loading completed > > > > --> for potential use after free issue > > Could I only call "free_irq(adapter->pnetdev->irq, adapter->pnetdev)" when register_netdev() failed ? > > If no need to change drivers/staging/rtl8712/hal_init.c file, I could give up my patch, thank you ! > > > > Cleaning this up is a bit complicated and requires reworking the > firmware loading and it requires testing. I don't think you have the > hardware to actually test this driver? Probably, just leave this code > for another day. > > regards, > dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1 1/2] Staging: silabs si4455 serial driver: fix directory structure and coding style
On Thu, Dec 10, 2020 at 12:20:59PM +, József Horváth wrote: > fix: coding style > fix: error checking > remove: doc silabs,si4455.txt > > Signed-off-by: József Horváth Just fold (combine) this check in with the first. This is much better but there are still 54 checkpatch warnins. total: 0 errors, 2 warnings, 52 checks, 1311 lines checked This is much better. Do we have to print so many warnings? It's useful for debugging but it makes the code messy. The ioctl still has some security issues with regards to user pointers (see bottom). > struct si4455_part_info { > - u8 CHIPREV; > - u16 PART; > - u8 PBUILD; > - u16 ID; > - u8 CUSTOMER; > - u8 ROMID; > - u8 BOND; > + u8 CHIPREV; > + u16 PART; > + u8 PBUILD; > + u16 ID; > + u8 CUSTOMER; > + u8 ROMID; > + u8 BOND; Make these lower case letters. > static int si4455_get_response(struct uart_port *port, > int length, > u8 *data) > { > - int ret = -EIO; > - u8 dataOut[] = { SI4455_CMD_ID_READ_CMD_BUFF }; > - u8 *dataIn = devm_kzalloc(port->dev, 1 + length, GFP_KERNEL); > - struct spi_transfer xfer[] = { > - { > - .tx_buf = dataOut, > - .len = sizeof(dataOut), > - }, { > - .rx_buf = dataIn, > - .len = 1 + length, > - } > - }; > - int errCnt = 1; > + int ret; > + u8 data_out[] = { SI4455_CMD_ID_READ_CMD_BUFF }; > + u8 *data_in = NULL; > + struct spi_transfer xfer[2]; > + int err = 1; Use "cnt" instead of "err" otherwise people will get confused and start saying "err = some_function();" > > - while (errCnt > 0) { > - dataOut[0] = SI4455_CMD_ID_READ_CMD_BUFF; > + data_in = kzalloc(1 + length, GFP_KERNEL); > + if (!data_in) > + return -ENOMEM; > + > + memset(&xfer, 0x00, sizeof(xfer)); > + xfer[0].tx_buf = data_out; > + xfer[0].len = sizeof(data_out); > + xfer[1].rx_buf = data_in; > + xfer[1].len = 1 + length; > + > + while (--err > 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 == 0) { > - if (dataIn[0] == 0xFF) { > - if ((length > 0) && (data != NULL)) { > - memcpy(data, &dataIn[1], length); > - } else { > - if (length > 0) > - ret = -EINVAL; > - } > - break; > + if (ret) { > + dev_err(port->dev, "%s: spi_sync_transfer error(%i)", > __func__, ret); > + break; > + } > + > + if (data_in[0] == 0xFF) { > + if (length > 0 && data) { > + memcpy(data, &data_in[1], length); > + } else { > + if (length > 0) > + ret = -EINVAL; Should this break if we encounter an error? > } > - usleep_range(100, 200); > - } else { > break; > } > - errCnt--; > + usleep_range(100, 200); > } > - if (errCnt == 0) { > - dev_err(port->dev, "si4455_poll_cts:errCnt==%i", errCnt); > + if (err == 0) { > + dev_err(port->dev, "%s:err==%i", __func__, err); > ret = -EIO; > } > - if (dataIn != NULL) > - devm_kfree(port->dev, dataIn); > + kfree(data_in); > return ret; > } > > +static int si4455_poll_cts(struct uart_port *port) > +{ > + return si4455_get_response(port, 0, NULL); > +} > + > static int si4455_send_command(struct uart_port *port, > int length, > u8 *data) > @@ -214,134 +193,118 @@ static int si4455_send_command(struct uart_port *port, > int ret; > > ret = si4455_poll_cts(port); > - if (ret == 0) { > - ret = spi_write(to_spi_device(port->dev), data, length); > - if (ret) { > - dev_err(port->dev, > - "%s: spi_write error(%i)", > - __func__, > - ret); > - } > - } else { > - dev_err(port->dev, > - "%s: si4455_poll_cts error(%i)", > +
Re: [PATCH v1 2/2] Staging: silabs si4455 serial driver: docs device tree binding
On Thu, Dec 10, 2020 at 12:21:56PM +, József Horváth wrote: > add: device tree binding schema For the subject, follow conventions of the directory. Something like: dt-bindings: serial: Add SiLabs SI4455 schema > Signed-off-by: József Horváth > --- > .../bindings/serial/silabs,si4455.yaml| 53 +++ > MAINTAINERS | 2 +- > 2 files changed, 54 insertions(+), 1 deletion(-) > create mode 100644 > Documentation/devicetree/bindings/serial/silabs,si4455.yaml > > diff --git a/Documentation/devicetree/bindings/serial/silabs,si4455.yaml > b/Documentation/devicetree/bindings/serial/silabs,si4455.yaml > new file mode 100644 > index ..80a73a61755b > --- /dev/null > +++ b/Documentation/devicetree/bindings/serial/silabs,si4455.yaml > @@ -0,0 +1,53 @@ > +# 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 Add 'description' with some info on this h/w and possibly a link to datasheet if available. > + > +maintainers: > + - József Horváth > + > +allOf: > + - $ref: "/schemas/serial.yaml#" > + > +properties: > + compatible: > +const: silabs,si4455 > + > + reg: > +maxItems: 1 > + > + interrupts: > +maxItems: 1 > + > + spi-max-frequency: > +description: maximum clock frequency on SPI port > +maximum: 50 > + > + shutdown-gpios: > +description: gpio pin for SDN > +maxItems: 1 > + > +required: > + - reg > + - interrupts > + - spi-max-frequency > + - shutdown-gpios > + > +additionalProperties: false > + > +examples: > + - | > +&spi0 { > + serial0: si4455@0 { > +compatible = "silabs,si4455"; > +reg = <0>; > +interrupt-parent = <&gpio>; > +interrupts = <7 IRQ_TYPE_LEVEL_LOW>; > +shutdown-gpios = <&gpio 26 1>; > +spi-max-frequency = <30>; > + }; > +}; > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index a29bc17d446d..16cc96971ac2 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -15937,7 +15937,7 @@ F:drivers/platform/x86/touchscreen_dmi.c > SILICON LABS SI4455 SERIAL DRIVER > M: József Horváth > S: Maintained > -F: Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt This is a new file, right? > +F: Documentation/devicetree/bindings/serial/silabs,si4455.yaml > F: drivers/tty/serial/si4455.c > F: drivers/tty/serial/si4455_api.h > > -- > 2.17.1 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1 2/2] Staging: silabs si4455 serial driver: docs device tree binding
On Thu, 10 Dec 2020 12:21:56 +, József Horváth wrote: > add: device tree binding schema > > Signed-off-by: József Horváth > --- > .../bindings/serial/silabs,si4455.yaml| 53 +++ > MAINTAINERS | 2 +- > 2 files changed, 54 insertions(+), 1 deletion(-) > create mode 100644 > Documentation/devicetree/bindings/serial/silabs,si4455.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: Error: Documentation/devicetree/bindings/serial/silabs,si4455.example.dts:19.9-14 syntax error FATAL ERROR: Unable to parse input tree make[1]: *** [scripts/Makefile.lib:342: Documentation/devicetree/bindings/serial/silabs,si4455.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs make: *** [Makefile:1364: dt_binding_check] Error 2 See https://patchwork.ozlabs.org/patch/1414082 The base for the patch is generally the last rc1. Any dependencies should be noted. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 16/19] dt-bindings: media: Add A83T MIPI CSI-2 bindings documentation
On Sat, 28 Nov 2020 15:28:36 +0100, Paul Kocialkowski wrote: > This introduces YAML bindings documentation for the A83T MIPI CSI-2 > controller. > > Signed-off-by: Paul Kocialkowski > --- > .../media/allwinner,sun8i-a83t-mipi-csi2.yaml | 147 ++ > 1 file changed, 147 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/media/allwinner,sun8i-a83t-mipi-csi2.yaml > Reviewed-by: Rob Herring ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[driver-core:driver-core-testing] BUILD SUCCESS WITH WARNING 2a0387e8128ae0c3de9ff976bc25afaae3d4a916
allyesconfig mips rb532_defconfig sh urquell_defconfig sh sdk7786_defconfig powerpc ppc64e_defconfig arm palmz72_defconfig sh sh7785lcr_32bit_defconfig arm shannon_defconfig sh j2_defconfig m68k m5249evb_defconfig arm versatile_defconfig sh rts7751r2dplus_defconfig powerpc mpc834x_itx_defconfig sh alldefconfig powerpc cm5200_defconfig powerpc ppc40x_defconfig mips rm200_defconfig arm mxs_defconfig riscvalldefconfig powerpc arches_defconfig m68km5272c3_defconfig powerpc mpc837x_rdb_defconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig c6x allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig sparc defconfig i386 tinyconfig i386defconfig sparcallyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a004-20201209 i386 randconfig-a005-20201209 i386 randconfig-a001-20201209 i386 randconfig-a002-20201209 i386 randconfig-a006-20201209 i386 randconfig-a003-20201209 i386 randconfig-a001-20201210 i386 randconfig-a004-20201210 i386 randconfig-a003-20201210 i386 randconfig-a002-20201210 i386 randconfig-a005-20201210 i386 randconfig-a006-20201210 x86_64 randconfig-a016-20201209 x86_64 randconfig-a012-20201209 x86_64 randconfig-a013-20201209 x86_64 randconfig-a014-20201209 x86_64 randconfig-a015-20201209 x86_64 randconfig-a011-20201209 i386 randconfig-a013-20201209 i386 randconfig-a014-20201209 i386 randconfig-a011-20201209 i386 randconfig-a015-20201209 i386 randconfig-a012-20201209 i386 randconfig-a016-20201209 riscvnommu_k210_defconfig riscvallyesconfig riscvnommu_virt_defconfig riscv allnoconfig riscv defconfig riscv rv32_defconfig riscvallmodconfig x86_64 rhel x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64 kexec clang tested configs: x86_64 randconfig-a004-20201209 x86_64 randconfig-a006-20201209 x86_64 randconfig-a005-20201209 x86_64 randconfig-a001-20201209 x86_64 randconfig-a002-20201209 x86_64 randconfig-a003-20201209 x86_64 randconfig-a003-20201210 x86_64 randconfig-a006-20201210 x86_64 randconfig-a002-20201210 x86_64 randconfig-a005-20201210 x86_64 randconfig-a004-20201210 x86_64 randconfig-a001-20201210 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org ___ devel mailing list de...@linuxdriverproject.org http
Re: [PATCH v2 1/2] dt-bindings: pinctrl: rt2880: add binding document
Hi Rob, On Thu, Dec 10, 2020 at 2:47 PM Rob Herring wrote: > > On Tue, Dec 08, 2020 at 08:55:22AM +0100, Sergio Paracuellos wrote: > > The commit adds rt2880 compatible node in binding document. > > > > Signed-off-by: Sergio Paracuellos > > --- > > .../pinctrl/ralink,rt2880-pinmux.yaml | 70 +++ > > 1 file changed, 70 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml > > b/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml > > new file mode 100644 > > index ..7dea3e26d99e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml > > @@ -0,0 +1,70 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/pinctrl/ralink,rt2880-pinmux.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Ralink rt2880 pinmux controller > > + > > +maintainers: > > + - Sergio Paracuellos > > + > > +description: > > + The rt2880 pinmux can only set the muxing of pin groups. muxing > > indiviual pins > > + is not supported. There is no pinconf support. > > + > > +properties: > > + compatible: > > +enum: > > + - ralink,rt2880-pinmux > > What's the control interface as you have no 'reg' property. There is not used in pinctrl. Every pin has a gpio function and pinctrl and gpio are separate drivers. Here only pin functions and groups are defined. The glue code for this driver is done in arch/mips/ralink/mt7621.c using specific pinmux.h header defined for ralink and then all that settings are used in drivers through the pinctrl driver. > > > + > > + pinctrl-0: > > +description: > > + A phandle to the node containing the subnodes containing default > > + configurations. This is for pinctrl hogs. > > + > > + pinctrl-names: > > +description: > > + A pinctrl state named "default" can be defined. > > +const: default > > These 2 properties go in consumer nodes. Ok, So I have to remove them from here. I see. > > > + > > +required: > > + - compatible > > + > > +patternProperties: > > + '[a-z0-9_-]+': > > +if: > > + type: object > > + description: node for pinctrl. > > + $ref: "pinmux-node.yaml" > > +then: > > For new bindings, don't do this hack. Just name the nodes '-pins$' I see. I will update bindings for pinctrl in staging and avoid this if-then clause. > > > + properties: > > +groups: > > + description: Name of the pin group to use for the functions. > > + enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio, > > + pcie, sdhci] > > +function: > > + description: The mux function to select > > + enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, > > + mdio, nand1, nand2, sdhci] > > additionalProperties: false Ok, I will add this. > > > + > > +additionalProperties: false > > + > > +examples: > > + # Pinmux controller node > > + - | > > +pinctrl { > > + compatible = "ralink,rt2880-pinmux"; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&state_default>; > > + > > + state_default: pinctrl0 { > > + }; > > + > > + i2c_pins: i2c0 { > > +i2c0 { > > + groups = "i2c"; > > + function = "i2c"; > > +}; > > + }; > > +}; > > -- > > 2.25.1 > > Thanks for the review. Best regards, Sergio Paracuellos ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel