* [GIT PULL] TTY/Serial patches for 4.19-rc1
From: Greg KH @ 2018-08-18 15:56 UTC (permalink / raw)
To: Linus Torvalds, Jiri Slaby
Cc: Stephen Rothwell, Andrew Morton, linux-kernel, linux-serial
The following changes since commit 021c91791a5e7e85c567452f1be3e4c2c6cb6063:
Linux 4.18-rc3 (2018-07-01 16:04:53 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tags/tty-4.19-rc1
for you to fetch changes up to 47ac76662ca9c5852fd353093f19de3ae85f2e66:
tty: serial: 8250: Revert NXP SC16C2552 workaround (2018-08-12 18:53:30 +0200)
----------------------------------------------------------------
TTY/Serial driver patches for 4.19-rc1
Here is the big tty and serial driver pull request for 4.19-rc1.
It's not all that big, just a number of small serial driver updates and
fixes, along with some better vt handling for unicode characters for
those using braille terminals.
Full details are in the shortlog.
All of these patches have been in linux-next for a long time with no
reported issues.
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
----------------------------------------------------------------
Aaron Sierra (1):
serial: 8250_exar: Read INT0 from slave device, too
Adam Borowski (4):
vt: don't reinvent min()
vt: selection: handle storing of characters above U+FFFF
vt: selection: take screen contents from uniscr if available
vt: drop unused struct vt_struct
Alexander Sverdlin (2):
serial: 8250: of: Correct of_platform_serial_setup() error handling
serial: 8250: of: Defer probe on missing IRQ
Andy Shevchenko (5):
serial: 8250_dw: Introduce IO accessors to extended registers
serial: 8250_exar: Remove useless has_slave member
serial: 8250_exar: Describe all members in struct exar8250_board
serial: 8250_exar: Drop unused variable in pci_xr17v35x_setup()
serial: 8250: Use cached port name directly in messages
Anton Vasilyev (1):
tty: rocket: Fix possible buffer overwrite on register_PCI
Arnd Bergmann (1):
tty: use 64-bit timstamp
Baruch Siach (1):
dt-bindings: serial: imx: clarify rs485 support usage
Chen Hu (1):
serial: 8250_dw: always set baud rate in dw8250_set_termios
Chris Brandt (6):
serial: sh-sci: Add support for R7S9210
serial: sh-sci: Document r7s9210 bindings
serial: sh-sci: Improve interrupts description
serial: sh-sci: Allow for compressed SCIF address
serial: sh-sci: Remove SCIx_RZ_SCIFA_REGTYPE
serial: sh-sci: Improve support for separate TEI and DRI interrupts
Christophe JAILLET (1):
serial: pxa: Fix an error handling path in 'serial_pxa_probe()'
Colin Ian King (2):
parport: parport_serial: Add WCH CH382L PCIe single parallel port support
tty: serial: jsm: remove redundant pointer ch
Daniel Golle (1):
tty: serial: exar: generalize RS485 setup
Geert Uytterhoeven (7):
serial: sh-sci: Stop RX FIFO timer during port shutdown
serial: sh-sci: Postpone DMA release when falling back to PIO
serial: sh-sci: Stop TX DMA workqueue during port shutdown
serial: sh-sci: Stop using deprecated dmaengine_terminate_all()
serial: sh-sci: Fix use-after-free on subsequent port startup
serial: sh-sci: Drop superfluous assignment in sci_request_dma()
serial: sh-sci: Document that serial aliases became optional
Girish Mahadevan (1):
tty: serial: qcom_geni_serial: Add support for flow control
Greg Kroah-Hartman (1):
Merge 4.18-rc3 into tty-next
Helmut Grohne (3):
serial: xuartps: fix typo in cdns_uart_startup
serial: xuartps: reduce hardware TX race condition
serial: xuartps: remove unnecessary register write
Jan Kundrát (1):
serial: max310x: Check the clock readiness
Jisheng Zhang (4):
serial: 8250: let serial8250_get_divisor() get uart_port * as param
serial: 8250: introduce get_divisor() and set_divisor() hook
serial: 8250: export serial8250_do_set_divisor()
serial: 8250_dw: add fractional divisor support
Johan Hovold (4):
serdev: add controller runtime PM support
tty: fix termios input-speed encoding
tty: fix termios input-speed encoding when using BOTHER
tty: support CIBAUD without BOTHER
Mark (1):
tty: serial: 8250: Revert NXP SC16C2552 workaround
Mars Cheng (3):
dt-bindings: mediatek: add support for mt6765 reference board
dt-bindings: mtk-uart: add mt6765 uart bindings
dt-bindings: interrupt-controller: add binding for mt6765
Matthijs van Duin (1):
pty: fix O_CLOEXEC for TIOCGPTPEER
Nicolas Pitre (6):
vt: preserve unicode values corresponding to screen characters
vt: introduce unicode mode for /dev/vcs
vt: unicode fallback for scrollback
vt: coherence validation code for the unicode screen buffer
vt: add /dev/vcsu* to devices.txt
vt: avoid a VLA in the unicode screen scroll function
Nishanth Menon (2):
dt-bindings: serial: 8250_omap: Add compatible for AM654 UART controller
serial: 8250_omap: Add support for AM654 UART controller
Peter Zijlstra (1):
atomic/tty: Fix up atomic abuse in ldsem
Phil Edworthy (2):
dt: serial: Add Renesas RZ/N1 binding documentation
serial: 8250_dw: Add compatible string for Renesas RZ/N1 UART
Robin Gong (1):
tty: serial: imx: correct dma cookie status
Samuel Thibault (1):
kbd: complete dead keys definitions
Sean Wang (1):
serdev: add dev_pm_domain_attach|detach()
Shubhrajyoti Datta (4):
tty: serial: uartlite: Add structure for private data
tty: serial: uartlite: Add clock adaptation
tty: serial: uartlite: Add support for suspend and resume
dt-bindings: serial: Add binding for uartlite
Srinath Mannam (1):
serial: 8250_dw: Add ACPI support for uart on Broadcom SoC
Tycho Andersen (1):
uart: fix race between uart_put_char() and uart_shutdown()
Uwe Kleine-König (1):
serial: imx: fix comment about UCR2_SRST and its handling for shadowing
Zhouyang Jia (1):
drivers/tty: add error handling for pcmcia_loop_config
Documentation/admin-guide/devices.txt | 16 +-
Documentation/devicetree/bindings/arm/mediatek.txt | 4 +
.../interrupt-controller/mediatek,sysirq.txt | 1 +
.../devicetree/bindings/serial/fsl-imx-uart.txt | 6 +-
.../devicetree/bindings/serial/mtk-uart.txt | 1 +
.../devicetree/bindings/serial/omap_serial.txt | 1 +
.../bindings/serial/renesas,rzn1-uart.txt | 10 +
.../bindings/serial/renesas,sci-serial.txt | 19 +-
.../bindings/serial/xlnx,opb-uartlite.txt | 23 ++
.../devicetree/bindings/soc/qcom/qcom,geni-se.txt | 2 +-
drivers/parport/parport_serial.c | 9 +
drivers/s390/char/keyboard.c | 30 +-
drivers/tty/pty.c | 2 +-
drivers/tty/rocket.c | 2 +-
drivers/tty/serdev/core.c | 48 ++-
drivers/tty/serial/8250/8250_core.c | 6 +-
drivers/tty/serial/8250/8250_dw.c | 74 ++++-
drivers/tty/serial/8250/8250_exar.c | 54 +--
drivers/tty/serial/8250/8250_of.c | 6 +-
drivers/tty/serial/8250/8250_omap.c | 1 +
drivers/tty/serial/8250/8250_port.c | 63 ++--
drivers/tty/serial/8250/serial_cs.c | 6 +-
drivers/tty/serial/imx.c | 5 +-
drivers/tty/serial/jsm/jsm_tty.c | 3 +-
drivers/tty/serial/max310x.c | 14 +-
drivers/tty/serial/pxa.c | 3 +-
drivers/tty/serial/qcom_geni_serial.c | 261 +++++++++++++--
drivers/tty/serial/serial_core.c | 17 +-
drivers/tty/serial/sh-sci.c | 191 ++++++-----
drivers/tty/serial/uartlite.c | 112 ++++++-
drivers/tty/serial/xilinx_uartps.c | 23 +-
drivers/tty/tty_baudrate.c | 22 +-
drivers/tty/tty_io.c | 19 +-
drivers/tty/tty_ldsem.c | 82 ++---
drivers/tty/vt/keyboard.c | 30 +-
drivers/tty/vt/selection.c | 48 +--
drivers/tty/vt/vc_screen.c | 90 ++++-
drivers/tty/vt/vt.c | 367 ++++++++++++++++++++-
include/linux/console_struct.h | 5 +-
include/linux/selection.h | 6 +
include/linux/serial_8250.h | 3 +
include/linux/serial_core.h | 7 +
include/linux/tty_ldisc.h | 4 +-
include/uapi/linux/keyboard.h | 23 +-
44 files changed, 1381 insertions(+), 338 deletions(-)
create mode 100644 Documentation/devicetree/bindings/serial/renesas,rzn1-uart.txt
create mode 100644 Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.txt
^ permalink raw reply
* [PATCH v4 2/2] kgdboc: Change printk to the right fashion
From: zhe.he @ 2018-08-17 14:42 UTC (permalink / raw)
To: jason.wessel, gregkh, jslaby, kgdb-bugreport, linux-serial,
linux-kernel
In-Reply-To: <1534516949-420150-1-git-send-email-zhe.he@windriver.com>
From: He Zhe <zhe.he@windriver.com>
pr_* is preferred according to scripts/checkpatch.pl.
Cc: jason.wessel@windriver.com
Cc: gregkh@linuxfoundation.org
Cc: jslaby@suse.com
Signed-off-by: He Zhe <zhe.he@windriver.com>
Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
---
v2:
- Split printk cleanups into a single patch
- Add cc to stable@vger.kernel.org
v3:
- Add pr_fmt for mod name
- Remove cc to stable@vger.kernel.org
v4:
- Add cc to maintainers and supporters
drivers/tty/serial/kgdboc.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index f4d0ef6..371357d 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -8,6 +8,9 @@
*
* 2007-2008 (c) Jason Wessel - Wind River Systems, Inc.
*/
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/kernel.h>
#include <linux/ctype.h>
#include <linux/kgdb.h>
@@ -131,12 +134,12 @@ static void kgdboc_unregister_kbd(void)
static int kgdboc_option_setup(char *opt)
{
if (!opt) {
- pr_err("kgdboc: config string not provided\n");
+ pr_err("config string not provided\n");
return -EINVAL;
}
if (strlen(opt) >= MAX_CONFIG_LEN) {
- printk(KERN_ERR "kgdboc: config string too long\n");
+ pr_err("config string too long\n");
return -ENOSPC;
}
strcpy(config, opt);
@@ -253,7 +256,7 @@ static int param_set_kgdboc_var(const char *kmessage,
int len = strlen(kmessage);
if (len >= MAX_CONFIG_LEN) {
- printk(KERN_ERR "kgdboc: config string too long\n");
+ pr_err("config string too long\n");
return -ENOSPC;
}
@@ -264,8 +267,7 @@ static int param_set_kgdboc_var(const char *kmessage,
}
if (kgdb_connected) {
- printk(KERN_ERR
- "kgdboc: Cannot reconfigure while KGDB is connected.\n");
+ pr_err("Cannot reconfigure while KGDB is connected.\n");
return -EBUSY;
}
--
2.7.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply related
* [PATCH v4 1/2] kgdboc: Passing ekgdboc to command line causes panic
From: zhe.he @ 2018-08-17 14:42 UTC (permalink / raw)
To: jason.wessel, gregkh, jslaby, kgdb-bugreport, linux-serial,
linux-kernel
From: He Zhe <zhe.he@windriver.com>
kgdboc_option_setup does not check input argument before passing it
to strlen. The argument would be a NULL pointer if "ekgdboc", without
its value, is set in command line and thus cause the following panic.
PANIC: early exception 0xe3 IP 10:ffffffff8fbbb620 error 0 cr2 0x0
[ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.18-rc8+ #1
[ 0.000000] RIP: 0010:strlen+0x0/0x20
...
[ 0.000000] Call Trace
[ 0.000000] ? kgdboc_option_setup+0x9/0xa0
[ 0.000000] ? kgdboc_early_init+0x6/0x1b
[ 0.000000] ? do_early_param+0x4d/0x82
[ 0.000000] ? parse_args+0x212/0x330
[ 0.000000] ? rdinit_setup+0x26/0x26
[ 0.000000] ? parse_early_options+0x20/0x23
[ 0.000000] ? rdinit_setup+0x26/0x26
[ 0.000000] ? parse_early_param+0x2d/0x39
[ 0.000000] ? setup_arch+0x2f7/0xbf4
[ 0.000000] ? start_kernel+0x5e/0x4c2
[ 0.000000] ? load_ucode_bsp+0x113/0x12f
[ 0.000000] ? secondary_startup_64+0xa5/0xb0
This patch adds a check to prevent the panic.
Cc: stable@vger.kernel.org
Cc: jason.wessel@windriver.com
Cc: gregkh@linuxfoundation.org
Cc: jslaby@suse.com
Signed-off-by: He Zhe <zhe.he@windriver.com>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
---
v2:
- Split out printk cleanups
- Add cc to stable@vger.kernel.org
v3:
- Use more meaningful error message
v4:
- Add cc to maintainers and supporters
drivers/tty/serial/kgdboc.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index b4ba2b1..f4d0ef6 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -130,6 +130,11 @@ static void kgdboc_unregister_kbd(void)
static int kgdboc_option_setup(char *opt)
{
+ if (!opt) {
+ pr_err("kgdboc: config string not provided\n");
+ return -EINVAL;
+ }
+
if (strlen(opt) >= MAX_CONFIG_LEN) {
printk(KERN_ERR "kgdboc: config string too long\n");
return -ENOSPC;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v3 2/2] kgdboc: Change printk to the right fashion
From: Daniel Thompson @ 2018-08-17 11:23 UTC (permalink / raw)
To: zhe.he
Cc: jason.wessel, gregkh, jslaby, kgdb-bugreport, linux-serial,
linux-kernel
In-Reply-To: <1534258660-80070-2-git-send-email-zhe.he@windriver.com>
On Tue, Aug 14, 2018 at 10:57:40PM +0800, zhe.he@windriver.com wrote:
> From: He Zhe <zhe.he@windriver.com>
>
> pr_* is preferred according to scripts/checkpatch.pl.
>
> Signed-off-by: He Zhe <zhe.he@windriver.com>
I thought I'd already done this but it looks like maybe I didn't get
round to it...
Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
> v2:
> - Split printk cleanups into a single patch
> - Add cc to stable@vger.kernel.org
> v3:
> - Add pr_fmt for mod name
>
> drivers/tty/serial/kgdboc.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
> index f4d0ef6..371357d 100644
> --- a/drivers/tty/serial/kgdboc.c
> +++ b/drivers/tty/serial/kgdboc.c
> @@ -8,6 +8,9 @@
> *
> * 2007-2008 (c) Jason Wessel - Wind River Systems, Inc.
> */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> #include <linux/kernel.h>
> #include <linux/ctype.h>
> #include <linux/kgdb.h>
> @@ -131,12 +134,12 @@ static void kgdboc_unregister_kbd(void)
> static int kgdboc_option_setup(char *opt)
> {
> if (!opt) {
> - pr_err("kgdboc: config string not provided\n");
> + pr_err("config string not provided\n");
> return -EINVAL;
> }
>
> if (strlen(opt) >= MAX_CONFIG_LEN) {
> - printk(KERN_ERR "kgdboc: config string too long\n");
> + pr_err("config string too long\n");
> return -ENOSPC;
> }
> strcpy(config, opt);
> @@ -253,7 +256,7 @@ static int param_set_kgdboc_var(const char *kmessage,
> int len = strlen(kmessage);
>
> if (len >= MAX_CONFIG_LEN) {
> - printk(KERN_ERR "kgdboc: config string too long\n");
> + pr_err("config string too long\n");
> return -ENOSPC;
> }
>
> @@ -264,8 +267,7 @@ static int param_set_kgdboc_var(const char *kmessage,
> }
>
> if (kgdb_connected) {
> - printk(KERN_ERR
> - "kgdboc: Cannot reconfigure while KGDB is connected.\n");
> + pr_err("Cannot reconfigure while KGDB is connected.\n");
>
> return -EBUSY;
> }
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH] serial: sh-sci: Document r8a774a1 bindings
From: Simon Horman @ 2018-08-17 9:59 UTC (permalink / raw)
To: Fabrizio Castro
Cc: Greg Kroah-Hartman, Rob Herring, Mark Rutland, linux-serial,
devicetree, linux-kernel, Geert Uytterhoeven, Chris Paterson,
Biju Das, linux-renesas-soc
In-Reply-To: <1534250042-15815-1-git-send-email-fabrizio.castro@bp.renesas.com>
On Tue, Aug 14, 2018 at 01:34:02PM +0100, Fabrizio Castro wrote:
> RZ/G2M (R8A774A1) SoC also has the R-Car Gen3 compatible SCIF and
> HSCIF ports, so document the SoC specific bindings. While at it,
> update the RZ/G1 and RZ/G2 family specific strings description as
> outdated.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das@bp.renesas.com>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
^ permalink raw reply
* Re: [PATCH] serial: sh-sci: Document r8a774a1 bindings
From: Rob Herring @ 2018-08-15 20:22 UTC (permalink / raw)
Cc: Greg Kroah-Hartman, Mark Rutland, Fabrizio Castro, linux-serial,
devicetree, linux-kernel, Simon Horman, Geert Uytterhoeven,
Chris Paterson, Biju Das, linux-renesas-soc
In-Reply-To: <1534250042-15815-1-git-send-email-fabrizio.castro@bp.renesas.com>
On Tue, 14 Aug 2018 13:34:02 +0100, Fabrizio Castro wrote:
> RZ/G2M (R8A774A1) SoC also has the R-Car Gen3 compatible SCIF and
> HSCIF ports, so document the SoC specific bindings. While at it,
> update the RZ/G1 and RZ/G2 family specific strings description as
> outdated.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das@bp.renesas.com>
> ---
> .../devicetree/bindings/serial/renesas,sci-serial.txt | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: serdev: How to attach serdev devices to USB based tty devices?
From: Rob Herring @ 2018-08-15 19:53 UTC (permalink / raw)
To: Andreas Färber
Cc: open list:SERIAL DRIVERS, Linux USB List,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Linux-MIPS, rehm, Xue Liu, LoRa_Community_Support, oneukum,
Alexander Graf, Ben Whitten, devicetree, starnight, netdev,
Johan Hovold
In-Reply-To: <3639955d-5990-1c82-7158-ac07b33c41f2@suse.de>
On Mon, Aug 13, 2018 at 8:28 PM Andreas Färber <afaerber@suse.de> wrote:
>
> Hi Rob et al.,
>
> For my LoRa network driver project [1] I have found your serdev
> framework to be a valuable help for dealing with hardware modules
> exposing some textual or binary UART interface.
>
> In particular on arm(64) and mips this allows to define an unlimited
> number of serdev drivers [2] that are associated via their Device Tree
> compatible string and can optionally be configured via DT properties.
>
> And in theory it seems serdev has also grown support for ACPI.
>
> Now, a growing number of vendors are placing such modules on a USB stick
> for easy evaluation on x86_64 PC hardware, or are designing mPCIe or M.2
> cards using their USB pins. While I do not yet have access to such a
> device myself, it is my understanding that devices with USB-UART bridge
> chipsets (e.g., FTDI) will show up as /dev/ttyUSBx and devices with an
> MCU implementing the CDC USB protocol (e.g., Pico-cell gateway = picoGW)
> will show up as /dev/ttyACMx.
> On the Raspberry Pi I've seen that Device Tree nodes can be used to pass
> information to on-board devices such as MAC address to Ethernet chipset,
> but that does not seem all that useful for passing a serdev child node
> to hot-plugged devices at unpredictable hub/port location (where it
> should not interfere with regular USB-UART cables for debugging), nor
> would it help ACPI based platforms such as x86_64.
>
> My idea then was that if we had some unique criteria like vendor and
> product IDs (or whatever is supported in usb_device_id), we could write
> a usb_driver with suitable USB_DEVICE*() macro. In its probe function we
> could call into the existing tty driver's probe function and afterwards
> try creating and attaching the appropriate serdev device, i.e. a fixed
> USB-to-serdev driver mapping. Problem is that most devices don't seem to
> implement any unique identifier I could make this depend on - either by
> using a standard FT232/FT2232/CH340G chip or by using STMicroelectronics
> virtual com port identifiers in CDC firmware and only differing in the
> textual description [3] the usb_device_id does not seem to match on.
>
> The obvious solution would of course be if hardware vendors could revise
> their designs to configure FTDI/etc. chips uniquely. I hear that that
> may involve exchanging the chipset, increasing costs, and may impact
> existing drivers. Wouldn't help for devices out there today either.
>
> For the picoGW CDC firmware, Semtech does appear to own a USB vendor ID,
> so it would seem possible to allocate their own product IDs for SX1301
> and SX1308 respectively to replace the generic STMicroelectronics IDs,
> which the various vendors could offer as firmware updates.
>
> All outside my control though.
>
> Oliver therefore suggested to not mess with USB drivers and instead use
> a line discipline (ldisc). It seems that for example the userspace tool
> slattach takes a tty device and performs an ioctl to switch the generic
> tty device into a special N_SLIP protocol mode, implemented in [4].
>
> However, the existing number of such ldisc modes appears to be below 30,
> with hardly any vendor-specific implementation, so polluting its number
> space seems undesirable? And in some cases I would like to use the same
> protocol implementation over direct UART and over USB, so would like to
> avoid duplicate serdev_device_driver and tty_ldisc_ops implementations.
>
> Long story short, has there been any thinking about a userspace
> interface to attach a given serdev driver to a tty device?
There was this[1] posted.
The main problem is the only way we know to instantiate a serdev ctrlr
is if there's a slave device described. I did make a series[2] that
makes serdev and tty device co-exist. Then you can more easily
manually attach a device. The problems are you get mismatches in
opens/closes in the tty layer and what should the behavior be if
userspace is trying to access the same port via both the tty and
serdev. After breaking things last time I touched tty open and close,
I'm hesitant to do that again. :)
> Or is there, on OF_DYNAMIC platforms, a way from userspace to associate
> a DT fragment (!= DT Overlay) with a given USB device dynamically, to
> attach a serdev node with sub-nodes?
There's been some discussions but no real progress. I think we need to
be able to support multiple DT roots and then assign/apply DTs to
arbitrary devices. That's first going to require that of_root is not
exposed outside of drivers/of/ and then there could be some issues
with assuming root==NULL is the base of the single DT. Beyond that, I
haven't given it too much thought.
An alternative is we create DT nodes for all devices which don't have
them (or only certain buses) and then we can apply overlays. This is
kind of headed down the path of doing an OpenFirmware implementation
which would enumerate all the devices and pass that DT to the OS.
Rob
[1] https://www.spinics.net/lists/linux-serial/msg30732.html
[2] https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/log/?h=serdev-ldisc-v2
^ permalink raw reply
* Re: serdev: How to attach serdev devices to USB based tty devices?
From: Sebastian Reichel @ 2018-08-15 18:21 UTC (permalink / raw)
To: Andreas Färber, Johan Hovold
Cc: Rob Herring, linux-serial@vger.kernel.org, linux-usb, Linux-MIPS,
Xue Liu, Ben Whitten, devicetree, netdev@vger.kernel.org,
Oliver Neukum, Alexander Graf, LoRa_Community_Support@semtech.com,
Jian-Hong Pan, Stefan Rehm, linux-arm-kernel@lists.infradead.org
In-Reply-To: <3639955d-5990-1c82-7158-ac07b33c41f2@suse.de>
[-- Attachment #1: Type: text/plain, Size: 5346 bytes --]
Hi,
+cc Johan Hovold <johan@kernel.org>
Johan told me, that he is working on this at ELCE 2017. Also he is
the subsystem maintainer of the USB serial subsystem.
-- Sebastian
On Tue, Aug 14, 2018 at 04:28:20AM +0200, Andreas Färber wrote:
> Hi Rob et al.,
>
> For my LoRa network driver project [1] I have found your serdev
> framework to be a valuable help for dealing with hardware modules
> exposing some textual or binary UART interface.
>
> In particular on arm(64) and mips this allows to define an unlimited
> number of serdev drivers [2] that are associated via their Device Tree
> compatible string and can optionally be configured via DT properties.
>
> And in theory it seems serdev has also grown support for ACPI.
>
> Now, a growing number of vendors are placing such modules on a USB stick
> for easy evaluation on x86_64 PC hardware, or are designing mPCIe or M.2
> cards using their USB pins. While I do not yet have access to such a
> device myself, it is my understanding that devices with USB-UART bridge
> chipsets (e.g., FTDI) will show up as /dev/ttyUSBx and devices with an
> MCU implementing the CDC USB protocol (e.g., Pico-cell gateway = picoGW)
> will show up as /dev/ttyACMx.
> On the Raspberry Pi I've seen that Device Tree nodes can be used to pass
> information to on-board devices such as MAC address to Ethernet chipset,
> but that does not seem all that useful for passing a serdev child node
> to hot-plugged devices at unpredictable hub/port location (where it
> should not interfere with regular USB-UART cables for debugging), nor
> would it help ACPI based platforms such as x86_64.
>
> My idea then was that if we had some unique criteria like vendor and
> product IDs (or whatever is supported in usb_device_id), we could write
> a usb_driver with suitable USB_DEVICE*() macro. In its probe function we
> could call into the existing tty driver's probe function and afterwards
> try creating and attaching the appropriate serdev device, i.e. a fixed
> USB-to-serdev driver mapping. Problem is that most devices don't seem to
> implement any unique identifier I could make this depend on - either by
> using a standard FT232/FT2232/CH340G chip or by using STMicroelectronics
> virtual com port identifiers in CDC firmware and only differing in the
> textual description [3] the usb_device_id does not seem to match on.
>
> The obvious solution would of course be if hardware vendors could revise
> their designs to configure FTDI/etc. chips uniquely. I hear that that
> may involve exchanging the chipset, increasing costs, and may impact
> existing drivers. Wouldn't help for devices out there today either.
>
> For the picoGW CDC firmware, Semtech does appear to own a USB vendor ID,
> so it would seem possible to allocate their own product IDs for SX1301
> and SX1308 respectively to replace the generic STMicroelectronics IDs,
> which the various vendors could offer as firmware updates.
>
> All outside my control though.
>
> Oliver therefore suggested to not mess with USB drivers and instead use
> a line discipline (ldisc). It seems that for example the userspace tool
> slattach takes a tty device and performs an ioctl to switch the generic
> tty device into a special N_SLIP protocol mode, implemented in [4].
>
> However, the existing number of such ldisc modes appears to be below 30,
> with hardly any vendor-specific implementation, so polluting its number
> space seems undesirable? And in some cases I would like to use the same
> protocol implementation over direct UART and over USB, so would like to
> avoid duplicate serdev_device_driver and tty_ldisc_ops implementations.
>
> Long story short, has there been any thinking about a userspace
> interface to attach a given serdev driver to a tty device?
>
> Or is there, on OF_DYNAMIC platforms, a way from userspace to associate
> a DT fragment (!= DT Overlay) with a given USB device dynamically, to
> attach a serdev node with sub-nodes?
>
> Any other ideas how to cleanly solve this?
>
> In some cases we're talking about a "simple" AT-like command interface;
> the picoGW implements a semi-generic USB-SPI bridge that may host a
> choice of 2+ chipsets, which in turn has two further sub-devices with 3+
> chipset choices (theoretically clk output and rx/tx options etc.) each.
> (For the latter I'm thinking we'll need a serdev driver exposing a
> regmap_bus and then implement regmap_bus based versions of the SPI
> drivers like Ben and I refactored SX1257 in [2] last weekend.)
>
> Thanks,
> Andreas
>
> [1] https://patchwork.ozlabs.org/cover/937545/
> [2]
> https://git.kernel.org/pub/scm/linux/kernel/git/afaerber/linux-lora.git/tree/drivers/net/lora?h=lora-next
> [3]
> https://github.com/Lora-net/picoGW_mcu/blob/master/src/usb_cdc/Src/usbd_desc.cpp#L59
> [4]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/slip/slip.c#n1281
>
> --
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: serdev: How to attach serdev devices to USB based tty devices?
From: Andrew Lunn @ 2018-08-15 0:46 UTC (permalink / raw)
To: Andreas Färber
Cc: Rob Herring, linux-serial@vger.kernel.org, linux-usb, Linux-MIPS,
Xue Liu, Ben Whitten, devicetree, netdev@vger.kernel.org,
Oliver Neukum, Alexander Graf, LoRa_Community_Support@semtech.com,
Jian-Hong Pan, Stefan Rehm, linux-arm-kernel@lists.infradead.org
In-Reply-To: <3639955d-5990-1c82-7158-ac07b33c41f2@suse.de>
Hi Andreas
This not my area of expertise. But i wanted to point out that there
exists a mechanism to make the FDTI driver release a port so that it
can be used from user space, e.g. for JTAG. It might be possible to
extend this mechanism to make the FTDI driver perform the registration
to serdev.
It is however not a generic solution.
Andrew
^ permalink raw reply
* Re: [PATCH v2 2/2] kgdboc: Change printk to the right fashion
From: He Zhe @ 2018-08-14 15:39 UTC (permalink / raw)
To: Daniel Thompson, Joe Perches
Cc: kgdb-bugreport, jslaby, linux-kernel, jason.wessel, linux-serial,
gregkh
In-Reply-To: <20180814144132.mpqzz3sxdbfzn7fe@holly.lan>
On 2018年08月14日 22:41, Daniel Thompson wrote:
> On Tue, Aug 14, 2018 at 07:04:11AM -0700, Joe Perches wrote:
>> On Tue, 2018-08-14 at 14:35 +0100, Daniel Thompson wrote:
>>> On Tue, Aug 14, 2018 at 08:46:01PM +0800, zhe.he@windriver.com wrote:
>>>> From: He Zhe <zhe.he@windriver.com>
>>>>
>>>> pr_* is preferred according to scripts/checkpatch.pl.
>> []
>>>> diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
>> []
>>>> @@ -136,7 +136,7 @@ static int kgdboc_option_setup(char *opt)
>>>> }
>>>>
>>>> if (strlen(opt) >= MAX_CONFIG_LEN) {
>>>> - printk(KERN_ERR "kgdboc: config string too long\n");
>>>> + pr_err("kgdboc: config string too long\n");
>>> Looks like you should remove the tags from pr_err and use pr_fmt(fmt) to
>>> put tags on the messages:
>>>
>>> #define pr_fmt(fmt) "kgdboc: " fmt
>> True and it's probably better to use:
>>
>> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> Good point! Thanks.
Daniel, Joe, thanks for your good suggestions. v3 is sent.
Zhe
>
>
> Daniel.
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Kgdb-bugreport mailing list
Kgdb-bugreport@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport
^ permalink raw reply
* [PATCH v3 2/2] kgdboc: Change printk to the right fashion
From: zhe.he @ 2018-08-14 14:57 UTC (permalink / raw)
To: jason.wessel, daniel.thompson, gregkh, jslaby, kgdb-bugreport,
linux-serial, linux-kernel
In-Reply-To: <1534258660-80070-1-git-send-email-zhe.he@windriver.com>
From: He Zhe <zhe.he@windriver.com>
pr_* is preferred according to scripts/checkpatch.pl.
Signed-off-by: He Zhe <zhe.he@windriver.com>
---
v2:
- Split printk cleanups into a single patch
- Add cc to stable@vger.kernel.org
v3:
- Add pr_fmt for mod name
drivers/tty/serial/kgdboc.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index f4d0ef6..371357d 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -8,6 +8,9 @@
*
* 2007-2008 (c) Jason Wessel - Wind River Systems, Inc.
*/
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/kernel.h>
#include <linux/ctype.h>
#include <linux/kgdb.h>
@@ -131,12 +134,12 @@ static void kgdboc_unregister_kbd(void)
static int kgdboc_option_setup(char *opt)
{
if (!opt) {
- pr_err("kgdboc: config string not provided\n");
+ pr_err("config string not provided\n");
return -EINVAL;
}
if (strlen(opt) >= MAX_CONFIG_LEN) {
- printk(KERN_ERR "kgdboc: config string too long\n");
+ pr_err("config string too long\n");
return -ENOSPC;
}
strcpy(config, opt);
@@ -253,7 +256,7 @@ static int param_set_kgdboc_var(const char *kmessage,
int len = strlen(kmessage);
if (len >= MAX_CONFIG_LEN) {
- printk(KERN_ERR "kgdboc: config string too long\n");
+ pr_err("config string too long\n");
return -ENOSPC;
}
@@ -264,8 +267,7 @@ static int param_set_kgdboc_var(const char *kmessage,
}
if (kgdb_connected) {
- printk(KERN_ERR
- "kgdboc: Cannot reconfigure while KGDB is connected.\n");
+ pr_err("Cannot reconfigure while KGDB is connected.\n");
return -EBUSY;
}
--
2.7.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply related
* [PATCH v3 1/2] kgdboc: Passing ekgdboc to command line causes panic
From: zhe.he @ 2018-08-14 14:57 UTC (permalink / raw)
To: jason.wessel, daniel.thompson, gregkh, jslaby, kgdb-bugreport,
linux-serial, linux-kernel
From: He Zhe <zhe.he@windriver.com>
kgdboc_option_setup does not check input argument before passing it
to strlen. The argument would be a NULL pointer if "ekgdboc", without
its value, is set in command line and thus cause the following panic.
PANIC: early exception 0xe3 IP 10:ffffffff8fbbb620 error 0 cr2 0x0
[ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.18-rc8+ #1
[ 0.000000] RIP: 0010:strlen+0x0/0x20
...
[ 0.000000] Call Trace
[ 0.000000] ? kgdboc_option_setup+0x9/0xa0
[ 0.000000] ? kgdboc_early_init+0x6/0x1b
[ 0.000000] ? do_early_param+0x4d/0x82
[ 0.000000] ? parse_args+0x212/0x330
[ 0.000000] ? rdinit_setup+0x26/0x26
[ 0.000000] ? parse_early_options+0x20/0x23
[ 0.000000] ? rdinit_setup+0x26/0x26
[ 0.000000] ? parse_early_param+0x2d/0x39
[ 0.000000] ? setup_arch+0x2f7/0xbf4
[ 0.000000] ? start_kernel+0x5e/0x4c2
[ 0.000000] ? load_ucode_bsp+0x113/0x12f
[ 0.000000] ? secondary_startup_64+0xa5/0xb0
This patch adds a check to prevent the panic.
Cc: stable@vger.kernel.org
Signed-off-by: He Zhe <zhe.he@windriver.com>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
---
v2:
- Split out printk cleanups
- Add cc to stable@vger.kernel.org
v3:
- Use more meaningful error message
drivers/tty/serial/kgdboc.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index b4ba2b1..f4d0ef6 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -130,6 +130,11 @@ static void kgdboc_unregister_kbd(void)
static int kgdboc_option_setup(char *opt)
{
+ if (!opt) {
+ pr_err("kgdboc: config string not provided\n");
+ return -EINVAL;
+ }
+
if (strlen(opt) >= MAX_CONFIG_LEN) {
printk(KERN_ERR "kgdboc: config string too long\n");
return -ENOSPC;
--
2.7.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply related
* Re: [PATCH v2 2/2] kgdboc: Change printk to the right fashion
From: Daniel Thompson @ 2018-08-14 14:41 UTC (permalink / raw)
To: Joe Perches
Cc: zhe.he, jason.wessel, gregkh, jslaby, kgdb-bugreport,
linux-serial, linux-kernel
In-Reply-To: <1678613360b7aebbf1ab42dd89139cb666ee8cd9.camel@perches.com>
On Tue, Aug 14, 2018 at 07:04:11AM -0700, Joe Perches wrote:
> On Tue, 2018-08-14 at 14:35 +0100, Daniel Thompson wrote:
> > On Tue, Aug 14, 2018 at 08:46:01PM +0800, zhe.he@windriver.com wrote:
> > > From: He Zhe <zhe.he@windriver.com>
> > >
> > > pr_* is preferred according to scripts/checkpatch.pl.
> []
> > > diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
> []
> > > @@ -136,7 +136,7 @@ static int kgdboc_option_setup(char *opt)
> > > }
> > >
> > > if (strlen(opt) >= MAX_CONFIG_LEN) {
> > > - printk(KERN_ERR "kgdboc: config string too long\n");
> > > + pr_err("kgdboc: config string too long\n");
> >
> > Looks like you should remove the tags from pr_err and use pr_fmt(fmt) to
> > put tags on the messages:
> >
> > #define pr_fmt(fmt) "kgdboc: " fmt
>
> True and it's probably better to use:
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
Good point! Thanks.
Daniel.
^ permalink raw reply
* Re: [PATCH v2 2/2] kgdboc: Change printk to the right fashion
From: Joe Perches @ 2018-08-14 14:04 UTC (permalink / raw)
To: Daniel Thompson, zhe.he
Cc: jason.wessel, gregkh, jslaby, kgdb-bugreport, linux-serial,
linux-kernel
In-Reply-To: <20180814133536.bfpygzwnmlqfsee5@holly.lan>
On Tue, 2018-08-14 at 14:35 +0100, Daniel Thompson wrote:
> On Tue, Aug 14, 2018 at 08:46:01PM +0800, zhe.he@windriver.com wrote:
> > From: He Zhe <zhe.he@windriver.com>
> >
> > pr_* is preferred according to scripts/checkpatch.pl.
[]
> > diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
[]
> > @@ -136,7 +136,7 @@ static int kgdboc_option_setup(char *opt)
> > }
> >
> > if (strlen(opt) >= MAX_CONFIG_LEN) {
> > - printk(KERN_ERR "kgdboc: config string too long\n");
> > + pr_err("kgdboc: config string too long\n");
>
> Looks like you should remove the tags from pr_err and use pr_fmt(fmt) to
> put tags on the messages:
>
> #define pr_fmt(fmt) "kgdboc: " fmt
True and it's probably better to use:
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
^ permalink raw reply
* Re: [PATCH v2 2/2] kgdboc: Change printk to the right fashion
From: Daniel Thompson @ 2018-08-14 13:35 UTC (permalink / raw)
To: zhe.he
Cc: jason.wessel, gregkh, jslaby, kgdb-bugreport, linux-serial,
linux-kernel
In-Reply-To: <1534250761-824-2-git-send-email-zhe.he@windriver.com>
On Tue, Aug 14, 2018 at 08:46:01PM +0800, zhe.he@windriver.com wrote:
> From: He Zhe <zhe.he@windriver.com>
>
> pr_* is preferred according to scripts/checkpatch.pl.
>
> Cc: stable@vger.kernel.org
This change does not "fix a problem that causes a build error
(but not for things marked CONFIG_BROKEN), an oops, a hang, data
corruption, a real security issue, or some “oh, that’s not good” issue.
In short, something critical.". Only the first patch meets this criteria
and only that patch should be Cc:ed to stable@ .
Please remove from this patch.
> Signed-off-by: He Zhe <zhe.he@windriver.com>
> ---
> v2:
> - Split printk cleanups into a single patch
> - Add cc to stable@vger.kernel.org
>
> drivers/tty/serial/kgdboc.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
> index 206f8c2..0003d6c 100644
> --- a/drivers/tty/serial/kgdboc.c
> +++ b/drivers/tty/serial/kgdboc.c
> @@ -136,7 +136,7 @@ static int kgdboc_option_setup(char *opt)
> }
>
> if (strlen(opt) >= MAX_CONFIG_LEN) {
> - printk(KERN_ERR "kgdboc: config string too long\n");
> + pr_err("kgdboc: config string too long\n");
Looks like you should remove the tags from pr_err and use pr_fmt(fmt) to
put tags on the messages:
#define pr_fmt(fmt) "kgdboc: " fmt
Daniel.
> return -ENOSPC;
> }
> strcpy(config, opt);
> @@ -253,7 +253,7 @@ static int param_set_kgdboc_var(const char *kmessage,
> int len = strlen(kmessage);
>
> if (len >= MAX_CONFIG_LEN) {
> - printk(KERN_ERR "kgdboc: config string too long\n");
> + pr_err("kgdboc: config string too long\n");
> return -ENOSPC;
> }
>
> @@ -264,8 +264,7 @@ static int param_set_kgdboc_var(const char *kmessage,
> }
>
> if (kgdb_connected) {
> - printk(KERN_ERR
> - "kgdboc: Cannot reconfigure while KGDB is connected.\n");
> + pr_err("kgdboc: Cannot reconfigure while KGDB is connected.\n");
>
> return -EBUSY;
> }
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v2 1/2] kgdboc: Passing ekgdboc to command line causes panic
From: Daniel Thompson @ 2018-08-14 13:26 UTC (permalink / raw)
To: zhe.he
Cc: jason.wessel, gregkh, jslaby, kgdb-bugreport, linux-serial,
linux-kernel
In-Reply-To: <1534250761-824-1-git-send-email-zhe.he@windriver.com>
On Tue, Aug 14, 2018 at 08:46:00PM +0800, zhe.he@windriver.com wrote:
> From: He Zhe <zhe.he@windriver.com>
>
> kgdboc_option_setup does not check input argument before passing it
> to strlen. The argument would be a NULL pointer if "ekgdboc", without
> its value, is set in command line and thus cause the following panic.
>
> PANIC: early exception 0xe3 IP 10:ffffffff8fbbb620 error 0 cr2 0x0
> [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.18-rc8+ #1
> [ 0.000000] RIP: 0010:strlen+0x0/0x20
> ...
> [ 0.000000] Call Trace
> [ 0.000000] ? kgdboc_option_setup+0x9/0xa0
> [ 0.000000] ? kgdboc_early_init+0x6/0x1b
> [ 0.000000] ? do_early_param+0x4d/0x82
> [ 0.000000] ? parse_args+0x212/0x330
> [ 0.000000] ? rdinit_setup+0x26/0x26
> [ 0.000000] ? parse_early_options+0x20/0x23
> [ 0.000000] ? rdinit_setup+0x26/0x26
> [ 0.000000] ? parse_early_param+0x2d/0x39
> [ 0.000000] ? setup_arch+0x2f7/0xbf4
> [ 0.000000] ? start_kernel+0x5e/0x4c2
> [ 0.000000] ? load_ucode_bsp+0x113/0x12f
> [ 0.000000] ? secondary_startup_64+0xa5/0xb0
>
> This patch adds a check to prevent the panic.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: He Zhe <zhe.he@windriver.com>
> ---
> v2:
> - Split out printk cleanups
> - Add cc to stable@vger.kernel.org
>
> drivers/tty/serial/kgdboc.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
> index b4ba2b1..206f8c2 100644
> --- a/drivers/tty/serial/kgdboc.c
> +++ b/drivers/tty/serial/kgdboc.c
> @@ -130,6 +130,11 @@ static void kgdboc_unregister_kbd(void)
>
> static int kgdboc_option_setup(char *opt)
> {
> + if (!opt) {
> + pr_err("kgdboc: null option\n");
Apologies... I should have picked this up when I replied earlier but
this error message describes what the function gets when I think it
should report what the user actually did. So should be something like:
pr_err("kgdboc: config string not provided\n");
With that (or a very similar) change:
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
> + return -EINVAL;
> + }
> +
> if (strlen(opt) >= MAX_CONFIG_LEN) {
> printk(KERN_ERR "kgdboc: config string too long\n");
> return -ENOSPC;
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v2 2/2] kgdboc: Change printk to the right fashion
From: zhe.he @ 2018-08-14 12:46 UTC (permalink / raw)
To: jason.wessel, daniel.thompson, gregkh, jslaby, kgdb-bugreport,
linux-serial, linux-kernel
Cc: zhe.he
In-Reply-To: <1534250761-824-1-git-send-email-zhe.he@windriver.com>
From: He Zhe <zhe.he@windriver.com>
pr_* is preferred according to scripts/checkpatch.pl.
Cc: stable@vger.kernel.org
Signed-off-by: He Zhe <zhe.he@windriver.com>
---
v2:
- Split printk cleanups into a single patch
- Add cc to stable@vger.kernel.org
drivers/tty/serial/kgdboc.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index 206f8c2..0003d6c 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -136,7 +136,7 @@ static int kgdboc_option_setup(char *opt)
}
if (strlen(opt) >= MAX_CONFIG_LEN) {
- printk(KERN_ERR "kgdboc: config string too long\n");
+ pr_err("kgdboc: config string too long\n");
return -ENOSPC;
}
strcpy(config, opt);
@@ -253,7 +253,7 @@ static int param_set_kgdboc_var(const char *kmessage,
int len = strlen(kmessage);
if (len >= MAX_CONFIG_LEN) {
- printk(KERN_ERR "kgdboc: config string too long\n");
+ pr_err("kgdboc: config string too long\n");
return -ENOSPC;
}
@@ -264,8 +264,7 @@ static int param_set_kgdboc_var(const char *kmessage,
}
if (kgdb_connected) {
- printk(KERN_ERR
- "kgdboc: Cannot reconfigure while KGDB is connected.\n");
+ pr_err("kgdboc: Cannot reconfigure while KGDB is connected.\n");
return -EBUSY;
}
--
2.7.4
^ permalink raw reply related
* [PATCH v2 1/2] kgdboc: Passing ekgdboc to command line causes panic
From: zhe.he @ 2018-08-14 12:46 UTC (permalink / raw)
To: jason.wessel, daniel.thompson, gregkh, jslaby, kgdb-bugreport,
linux-serial, linux-kernel
Cc: zhe.he
From: He Zhe <zhe.he@windriver.com>
kgdboc_option_setup does not check input argument before passing it
to strlen. The argument would be a NULL pointer if "ekgdboc", without
its value, is set in command line and thus cause the following panic.
PANIC: early exception 0xe3 IP 10:ffffffff8fbbb620 error 0 cr2 0x0
[ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.18-rc8+ #1
[ 0.000000] RIP: 0010:strlen+0x0/0x20
...
[ 0.000000] Call Trace
[ 0.000000] ? kgdboc_option_setup+0x9/0xa0
[ 0.000000] ? kgdboc_early_init+0x6/0x1b
[ 0.000000] ? do_early_param+0x4d/0x82
[ 0.000000] ? parse_args+0x212/0x330
[ 0.000000] ? rdinit_setup+0x26/0x26
[ 0.000000] ? parse_early_options+0x20/0x23
[ 0.000000] ? rdinit_setup+0x26/0x26
[ 0.000000] ? parse_early_param+0x2d/0x39
[ 0.000000] ? setup_arch+0x2f7/0xbf4
[ 0.000000] ? start_kernel+0x5e/0x4c2
[ 0.000000] ? load_ucode_bsp+0x113/0x12f
[ 0.000000] ? secondary_startup_64+0xa5/0xb0
This patch adds a check to prevent the panic.
Cc: stable@vger.kernel.org
Signed-off-by: He Zhe <zhe.he@windriver.com>
---
v2:
- Split out printk cleanups
- Add cc to stable@vger.kernel.org
drivers/tty/serial/kgdboc.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index b4ba2b1..206f8c2 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -130,6 +130,11 @@ static void kgdboc_unregister_kbd(void)
static int kgdboc_option_setup(char *opt)
{
+ if (!opt) {
+ pr_err("kgdboc: null option\n");
+ return -EINVAL;
+ }
+
if (strlen(opt) >= MAX_CONFIG_LEN) {
printk(KERN_ERR "kgdboc: config string too long\n");
return -ENOSPC;
--
2.7.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply related
* [PATCH] serial: sh-sci: Document r8a774a1 bindings
From: Fabrizio Castro @ 2018-08-14 12:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rob Herring, Mark Rutland
Cc: Fabrizio Castro, linux-serial, devicetree, linux-kernel,
Simon Horman, Geert Uytterhoeven, Chris Paterson, Biju Das,
linux-renesas-soc
RZ/G2M (R8A774A1) SoC also has the R-Car Gen3 compatible SCIF and
HSCIF ports, so document the SoC specific bindings. While at it,
update the RZ/G1 and RZ/G2 family specific strings description as
outdated.
Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
Reviewed-by: Biju Das <biju.das@bp.renesas.com>
---
.../devicetree/bindings/serial/renesas,sci-serial.txt | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
index eaca9da..1994ab8 100644
--- a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
+++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
@@ -20,6 +20,8 @@ Required properties:
- "renesas,hscif-r8a7745" for R8A7745 (RZ/G1E) HSCIF compatible UART.
- "renesas,scif-r8a77470" for R8A77470 (RZ/G1C) SCIF compatible UART.
- "renesas,hscif-r8a77470" for R8A77470 (RZ/G1C) HSCIF compatible UART.
+ - "renesas,scif-r8a774a1" for R8A774A1 (RZ/G2M) SCIF compatible UART.
+ - "renesas,hscif-r8a774a1" for R8A774A1 (RZ/G2M) HSCIF compatible UART.
- "renesas,scif-r8a7778" for R8A7778 (R-Car M1) SCIF compatible UART.
- "renesas,scif-r8a7779" for R8A7779 (R-Car H1) SCIF compatible UART.
- "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible UART.
@@ -55,13 +57,13 @@ Required properties:
- "renesas,scifa-sh73a0" for SH73A0 (SH-Mobile AG5) SCIFA compatible UART.
- "renesas,scifb-sh73a0" for SH73A0 (SH-Mobile AG5) SCIFB compatible UART.
- "renesas,rcar-gen1-scif" for R-Car Gen1 SCIF compatible UART,
- - "renesas,rcar-gen2-scif" for R-Car Gen2 SCIF compatible UART,
- - "renesas,rcar-gen3-scif" for R-Car Gen3 SCIF compatible UART,
- - "renesas,rcar-gen2-scifa" for R-Car Gen2 SCIFA compatible UART,
- - "renesas,rcar-gen2-scifb" for R-Car Gen2 SCIFB compatible UART,
+ - "renesas,rcar-gen2-scif" for R-Car Gen2 and RZ/G1 SCIF compatible UART,
+ - "renesas,rcar-gen3-scif" for R-Car Gen3 and RZ/G2 SCIF compatible UART,
+ - "renesas,rcar-gen2-scifa" for R-Car Gen2 and RZ/G1 SCIFA compatible UART,
+ - "renesas,rcar-gen2-scifb" for R-Car Gen2 and RZ/G1 SCIFB compatible UART,
- "renesas,rcar-gen1-hscif" for R-Car Gen1 HSCIF compatible UART,
- - "renesas,rcar-gen2-hscif" for R-Car Gen2 HSCIF compatible UART,
- - "renesas,rcar-gen3-hscif" for R-Car Gen3 HSCIF compatible UART,
+ - "renesas,rcar-gen2-hscif" for R-Car Gen2 and RZ/G1 HSCIF compatible UART,
+ - "renesas,rcar-gen3-hscif" for R-Car Gen3 and RZ/G2 HSCIF compatible UART,
- "renesas,scif" for generic SCIF compatible UART.
- "renesas,scifa" for generic SCIFA compatible UART.
- "renesas,scifb" for generic SCIFB compatible UART.
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] kgdboc: Passing ekgdboc to command line causes panic
From: Daniel Thompson @ 2018-08-14 9:16 UTC (permalink / raw)
To: zhe.he
Cc: jason.wessel, gregkh, jslaby, kgdb-bugreport, linux-serial,
linux-kernel
In-Reply-To: <1533743984-426660-1-git-send-email-zhe.he@windriver.com>
On Wed, Aug 08, 2018 at 11:59:44PM +0800, zhe.he@windriver.com wrote:
> From: He Zhe <zhe.he@windriver.com>
>
> kgdboc_option_setup does not check input argument before passing it
> to strlen. The argument would be a NULL pointer if "ekgdboc", without
> its value, is set in command line and thus cause the following panic.
>
> PANIC: early exception 0xe3 IP 10:ffffffff8fbbb620 error 0 cr2 0x0
> [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.18-rc8+ #1
> [ 0.000000] RIP: 0010:strlen+0x0/0x20
> ...
> [ 0.000000] Call Trace
> [ 0.000000] ? kgdboc_option_setup+0x9/0xa0
> [ 0.000000] ? kgdboc_early_init+0x6/0x1b
> [ 0.000000] ? do_early_param+0x4d/0x82
> [ 0.000000] ? parse_args+0x212/0x330
> [ 0.000000] ? rdinit_setup+0x26/0x26
> [ 0.000000] ? parse_early_options+0x20/0x23
> [ 0.000000] ? rdinit_setup+0x26/0x26
> [ 0.000000] ? parse_early_param+0x2d/0x39
> [ 0.000000] ? setup_arch+0x2f7/0xbf4
> [ 0.000000] ? start_kernel+0x5e/0x4c2
> [ 0.000000] ? load_ucode_bsp+0x113/0x12f
> [ 0.000000] ? secondary_startup_64+0xa5/0xb0
>
> This patch adds a check to prevent the panic and changes some printk
> to right fashion.
>
> Signed-off-by: He Zhe <zhe.he@windriver.com>
For the bug fix portions of this patch you should add the stable kernel
ML to the Cc: of the sign off area: See:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-1
> ---
> drivers/tty/serial/kgdboc.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
> index b4ba2b1..0003d6c 100644
> --- a/drivers/tty/serial/kgdboc.c
> +++ b/drivers/tty/serial/kgdboc.c
> @@ -130,8 +130,13 @@ static void kgdboc_unregister_kbd(void)
>
> static int kgdboc_option_setup(char *opt)
> {
> + if (!opt) {
> + pr_err("kgdboc: null option\n");
> + return -EINVAL;
> + }
> +
> if (strlen(opt) >= MAX_CONFIG_LEN) {
> - printk(KERN_ERR "kgdboc: config string too long\n");
> + pr_err("kgdboc: config string too long\n");
These changes are not bug fixes and are not normally candidates for
backporting. It would therefore be better to put the s/printk KERN_ERR /pr_err(/
changes into a separate patch.
> return -ENOSPC;
> }
> strcpy(config, opt);
> @@ -248,7 +253,7 @@ static int param_set_kgdboc_var(const char *kmessage,
> int len = strlen(kmessage);
>
> if (len >= MAX_CONFIG_LEN) {
> - printk(KERN_ERR "kgdboc: config string too long\n");
> + pr_err("kgdboc: config string too long\n");
> return -ENOSPC;
> }
>
> @@ -259,8 +264,7 @@ static int param_set_kgdboc_var(const char *kmessage,
> }
>
> if (kgdb_connected) {
> - printk(KERN_ERR
> - "kgdboc: Cannot reconfigure while KGDB is connected.\n");
> + pr_err("kgdboc: Cannot reconfigure while KGDB is connected.\n");
>
> return -EBUSY;
> }
> --
> 2.7.4
>
^ permalink raw reply
* Re: serdev: How to attach serdev devices to USB based tty devices?
From: Oliver Neukum @ 2018-08-14 7:23 UTC (permalink / raw)
To: Andreas Färber, Rob Herring, linux-serial@vger.kernel.org,
linux-usb
Cc: Jian-Hong Pan, Xue Liu, Ben Whitten, Linux-MIPS,
linux-arm-kernel@lists.infradead.org, Stefan Rehm,
LoRa_Community_Support@semtech.com, Alexander Graf, devicetree,
netdev@vger.kernel.org
In-Reply-To: <3639955d-5990-1c82-7158-ac07b33c41f2@suse.de>
On Di, 2018-08-14 at 04:28 +0200, Andreas Färber wrote:
> My idea then was that if we had some unique criteria like vendor and
> product IDs (or whatever is supported in usb_device_id), we could write
> a usb_driver with suitable USB_DEVICE*() macro. In its probe function we
> could call into the existing tty driver's probe function and afterwards
> try creating and attaching the appropriate serdev device, i.e. a fixed
> USB-to-serdev driver mapping. Problem is that most devices don't seem to
> implement any unique identifier I could make this depend on - either by
> using a standard FT232/FT2232/CH340G chip or by using STMicroelectronics
> virtual com port identifiers in CDC firmware and only differing in the
> textual description [3] the usb_device_id does not seem to match on.
If you really must do this you can benignly fail probe(). Thus you
can compare strings within your probe() method.
This sucks because you need to make sure your drivers are always
loaded in a certain order and you really rely on undocumented
properties, but it can be done.
Regards
Oliver
^ permalink raw reply
* serdev: How to attach serdev devices to USB based tty devices?
From: Andreas Färber @ 2018-08-14 2:28 UTC (permalink / raw)
To: Rob Herring, linux-serial@vger.kernel.org, linux-usb
Cc: linux-arm-kernel@lists.infradead.org, Linux-MIPS, Stefan Rehm,
Xue Liu, LoRa_Community_Support@semtech.com, Oliver Neukum,
Alexander Graf, Ben Whitten, devicetree, Jian-Hong Pan,
netdev@vger.kernel.org
Hi Rob et al.,
For my LoRa network driver project [1] I have found your serdev
framework to be a valuable help for dealing with hardware modules
exposing some textual or binary UART interface.
In particular on arm(64) and mips this allows to define an unlimited
number of serdev drivers [2] that are associated via their Device Tree
compatible string and can optionally be configured via DT properties.
And in theory it seems serdev has also grown support for ACPI.
Now, a growing number of vendors are placing such modules on a USB stick
for easy evaluation on x86_64 PC hardware, or are designing mPCIe or M.2
cards using their USB pins. While I do not yet have access to such a
device myself, it is my understanding that devices with USB-UART bridge
chipsets (e.g., FTDI) will show up as /dev/ttyUSBx and devices with an
MCU implementing the CDC USB protocol (e.g., Pico-cell gateway = picoGW)
will show up as /dev/ttyACMx.
On the Raspberry Pi I've seen that Device Tree nodes can be used to pass
information to on-board devices such as MAC address to Ethernet chipset,
but that does not seem all that useful for passing a serdev child node
to hot-plugged devices at unpredictable hub/port location (where it
should not interfere with regular USB-UART cables for debugging), nor
would it help ACPI based platforms such as x86_64.
My idea then was that if we had some unique criteria like vendor and
product IDs (or whatever is supported in usb_device_id), we could write
a usb_driver with suitable USB_DEVICE*() macro. In its probe function we
could call into the existing tty driver's probe function and afterwards
try creating and attaching the appropriate serdev device, i.e. a fixed
USB-to-serdev driver mapping. Problem is that most devices don't seem to
implement any unique identifier I could make this depend on - either by
using a standard FT232/FT2232/CH340G chip or by using STMicroelectronics
virtual com port identifiers in CDC firmware and only differing in the
textual description [3] the usb_device_id does not seem to match on.
The obvious solution would of course be if hardware vendors could revise
their designs to configure FTDI/etc. chips uniquely. I hear that that
may involve exchanging the chipset, increasing costs, and may impact
existing drivers. Wouldn't help for devices out there today either.
For the picoGW CDC firmware, Semtech does appear to own a USB vendor ID,
so it would seem possible to allocate their own product IDs for SX1301
and SX1308 respectively to replace the generic STMicroelectronics IDs,
which the various vendors could offer as firmware updates.
All outside my control though.
Oliver therefore suggested to not mess with USB drivers and instead use
a line discipline (ldisc). It seems that for example the userspace tool
slattach takes a tty device and performs an ioctl to switch the generic
tty device into a special N_SLIP protocol mode, implemented in [4].
However, the existing number of such ldisc modes appears to be below 30,
with hardly any vendor-specific implementation, so polluting its number
space seems undesirable? And in some cases I would like to use the same
protocol implementation over direct UART and over USB, so would like to
avoid duplicate serdev_device_driver and tty_ldisc_ops implementations.
Long story short, has there been any thinking about a userspace
interface to attach a given serdev driver to a tty device?
Or is there, on OF_DYNAMIC platforms, a way from userspace to associate
a DT fragment (!= DT Overlay) with a given USB device dynamically, to
attach a serdev node with sub-nodes?
Any other ideas how to cleanly solve this?
In some cases we're talking about a "simple" AT-like command interface;
the picoGW implements a semi-generic USB-SPI bridge that may host a
choice of 2+ chipsets, which in turn has two further sub-devices with 3+
chipset choices (theoretically clk output and rx/tx options etc.) each.
(For the latter I'm thinking we'll need a serdev driver exposing a
regmap_bus and then implement regmap_bus based versions of the SPI
drivers like Ben and I refactored SX1257 in [2] last weekend.)
Thanks,
Andreas
[1] https://patchwork.ozlabs.org/cover/937545/
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/afaerber/linux-lora.git/tree/drivers/net/lora?h=lora-next
[3]
https://github.com/Lora-net/picoGW_mcu/blob/master/src/usb_cdc/Src/usbd_desc.cpp#L59
[4]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/slip/slip.c#n1281
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply
* Re: [PATCH v2 18/18] dt-bindings: serial: lantiq: Add optional properties for CCF
From: Rob Herring @ 2018-08-13 17:53 UTC (permalink / raw)
To: Songjun Wu
Cc: hua.ma, yixin.zhu, chuanhua.lei, qi-ming.wu, linux-mips,
linux-clk, linux-serial, devicetree, linux-kernel,
Greg Kroah-Hartman, Mark Rutland
In-Reply-To: <20180803030237.3366-19-songjun.wu@linux.intel.com>
On Fri, Aug 03, 2018 at 11:02:37AM +0800, Songjun Wu wrote:
> Clocks and clock-names are updated in device tree binding.
>
> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
> ---
>
> Changes in v2: None
>
> Documentation/devicetree/bindings/serial/lantiq_asc.txt | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v5 01/11] dt-bindings: clock: mediatek: document clk bindings for Mediatek MT6765 SoC
From: Owen Chen @ 2018-08-13 9:12 UTC (permalink / raw)
To: Rob Herring
Cc: Mars Cheng, Matthias Brugger, Marc Zyngier, Ryder Lee,
Stephen Boyd, Sean Wang, CC Hwang, Loda Chou, linux-kernel,
linux-mediatek, devicetree, wsd_upstream, linux-serial,
linux-arm-kernel, linux-clk
In-Reply-To: <20180720174331.GA26945@rob-hp-laptop>
On Fri, 2018-07-20 at 11:43 -0600, Rob Herring wrote:
> On Tue, Jul 17, 2018 at 04:52:22PM +0800, Mars Cheng wrote:
> > This patch adds the binding documentation for apmixedsys, audsys, camsys,
> > imgsys, infracfg, mipi0a, topckgen, vcodecsys
> >
> > Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> > Signed-off-by: Owen Chen <owen.chen@mediatek.com>
> > ---
> > .../bindings/arm/mediatek/mediatek,apmixedsys.txt | 1 +
> > .../bindings/arm/mediatek/mediatek,audsys.txt | 1 +
> > .../bindings/arm/mediatek/mediatek,camsys.txt | 27 ++++++++++++++++++++
> > .../bindings/arm/mediatek/mediatek,imgsys.txt | 1 +
> > .../bindings/arm/mediatek/mediatek,infracfg.txt | 1 +
> > .../bindings/arm/mediatek/mediatek,mipi0a.txt | 23 +++++++++++++++++
> > .../bindings/arm/mediatek/mediatek,mmsys.txt | 1 +
> > .../bindings/arm/mediatek/mediatek,pericfg.txt | 1 +
> > .../bindings/arm/mediatek/mediatek,topckgen.txt | 1 +
> > .../bindings/arm/mediatek/mediatek,vcodecsys.txt | 22 ++++++++++++++++
> > 10 files changed, 79 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,camsys.txt
> > create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mipi0a.txt
> > create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,vcodecsys.txt
> >
> > diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,apmixedsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,apmixedsys.txt
> > index b404d59..44eaeac 100644
> > --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,apmixedsys.txt
> > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,apmixedsys.txt
> > @@ -8,6 +8,7 @@ Required Properties:
> > - compatible: Should be one of:
> > - "mediatek,mt2701-apmixedsys"
> > - "mediatek,mt2712-apmixedsys", "syscon"
> > + - "mediatek,mt6765-apmixedsys", "syscon"
> > - "mediatek,mt6797-apmixedsys"
> > - "mediatek,mt7622-apmixedsys"
> > - "mediatek,mt8135-apmixedsys"
> > diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt
> > index 34a69ba..9a8672a 100644
> > --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt
> > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt
> > @@ -7,6 +7,7 @@ Required Properties:
> >
> > - compatible: Should be one of:
> > - "mediatek,mt2701-audsys", "syscon"
> > + - "mediatek,mt6765-audsys", "syscon"
> > - "mediatek,mt7622-audsys", "syscon"
> > - #clock-cells: Must be 1
> >
> > diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,camsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,camsys.txt
> > new file mode 100644
> > index 0000000..dc75783
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,camsys.txt
> > @@ -0,0 +1,27 @@
> > +MediaTek CAMSYS controller
> > +============================
> > +
> > +The MediaTek CAMSYS controller provides various clocks to the system.
>
> Only clocks? If so, then this should be moved to bindings/clocks/.
>
camsys conclude not only clks but also mtcmos, so it's better to put
this node in this path.
> > +
> > +Required Properties:
> > +
> > +- compatible: Should be one of:
> > + - "mediatek,mt6765-camsys", "syscon"
> > +- #clock-cells: Must be 1
> > +
> > +The AUDSYS controller uses the common clk binding from
> > +Documentation/devicetree/bindings/clock/clock-bindings.txt
> > +The available clocks are defined in dt-bindings/clock/mt*-clk.h.
> > +
> > +Required sub-nodes:
> > +-------
> > +For common binding part and usage, refer to
> > +../sonud/mt2701-afe-pcm.txt.
> > +
> > +Example:
> > +
> > +camsys: camsys@1a000000 {
>
> clock-controller@...
>
> if the above answer is yes.
>
> Same comments on the other docs.
^ permalink raw reply
* Re: [PATCH v5 09/11] clk: mediatek: add new clkmux register API
From: Owen Chen @ 2018-08-13 9:09 UTC (permalink / raw)
To: Sean Wang
Cc: Mars Cheng, Matthias Brugger, Rob Herring, Marc Zyngier,
Ryder Lee, Stephen Boyd, CC Hwang, Loda Chou, linux-kernel,
linux-mediatek, devicetree, wsd_upstream, linux-serial,
linux-arm-kernel, linux-clk
In-Reply-To: <1531983463.8953.235.camel@mtkswgap22>
On Thu, 2018-07-19 at 14:57 +0800, Sean Wang wrote:
> On Tue, 2018-07-17 at 16:52 +0800, Mars Cheng wrote:
> > From: Owen Chen <owen.chen@mediatek.com>
> >
> > MT6765 add "set/clr" register for each clkmux setting, and
> > one update register to trigger value change. It is designed
> > to prevent read-modify-write racing issue. The sw design
> > need to add a new API to handle this hw change with a new
> > mtk_clk_mux/mtk_clk_upd struct in new file "clk-mux"and
> > clk-upd".
> >
>
> I don't see any word mtk_clk_upd or clk-upd in the patch
>
> and the patch needs to be split into more patches
>
clk-upd is old description, no more clk-upd for handling update bit.
I will remove it next version.
> > Signed-off-by: Owen Chen <owen.chen@mediatek.com>
> > ---
> > drivers/clk/mediatek/Makefile | 2 +-
> > drivers/clk/mediatek/clk-mtk.c | 41 +++++++
> > drivers/clk/mediatek/clk-mtk.h | 85 ++++++++++++---
> > drivers/clk/mediatek/clk-mux.c | 236 ++++++++++++++++++++++++++++++++++++++++
> > drivers/clk/mediatek/clk-mux.h | 38 +++++++
> > 5 files changed, 388 insertions(+), 14 deletions(-)
> > create mode 100644 drivers/clk/mediatek/clk-mux.c
> > create mode 100644 drivers/clk/mediatek/clk-mux.h
> >
> > diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
> > index 844b55d..b97980d 100644
> > --- a/drivers/clk/mediatek/Makefile
> > +++ b/drivers/clk/mediatek/Makefile
> > @@ -1,5 +1,5 @@
> > # SPDX-License-Identifier: GPL-2.0
> > -obj-$(CONFIG_COMMON_CLK_MEDIATEK) += clk-mtk.o clk-pll.o clk-gate.o clk-apmixed.o clk-cpumux.o reset.o
> > +obj-$(CONFIG_COMMON_CLK_MEDIATEK) += clk-mtk.o clk-pll.o clk-gate.o clk-apmixed.o clk-cpumux.o reset.o clk-mux.o
> > obj-$(CONFIG_COMMON_CLK_MT6797) += clk-mt6797.o
> > obj-$(CONFIG_COMMON_CLK_MT6797_IMGSYS) += clk-mt6797-img.o
> > obj-$(CONFIG_COMMON_CLK_MT6797_MMSYS) += clk-mt6797-mm.o
> > diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
> > index 9c0ae42..50becd0 100644
> > --- a/drivers/clk/mediatek/clk-mtk.c
> > +++ b/drivers/clk/mediatek/clk-mtk.c
> > @@ -22,6 +22,7 @@
> > #include <linux/mfd/syscon.h>
> >
> > #include "clk-mtk.h"
> > +#include "clk-mux.h"
> > #include "clk-gate.h"
> >
> > struct clk_onecell_data *mtk_alloc_clk_data(unsigned int clk_num)
> > @@ -144,6 +145,46 @@ int mtk_clk_register_gates(struct device_node *node,
> > return 0;
> > }
> >
> > +int mtk_clk_register_muxes(const struct mtk_mux *muxes,
> > + int num, struct device_node *node,
> > + spinlock_t *lock,
> > + struct clk_onecell_data *clk_data)
> > +{
> > + struct regmap *regmap;
> > + struct clk *clk;
> > + int i;
> > +
> > + if (!clk_data)
> > + return -ENOMEM;
> > +
>
> general register function is able to handle that there is no clk_data.
> It looks like a optional, not a mandatory
>
clk_data is reused in topcksys clk registration, because of the
registration include muxes/gates/dividers/fixed_clks, so we need to make
sure clk_data is not NULL pointer so we can use to store the clk
structure we allocated.
> > + regmap = syscon_node_to_regmap(node);
> > + if (IS_ERR(regmap)) {
> > + pr_err("Cannot find regmap for %pOF: %ld\n", node,
> > + PTR_ERR(regmap));
> > + return PTR_ERR(regmap);
> > + }
> > +
> > + for (i = 0; i < num; i++) {
> > + const struct mtk_mux *mux = &muxes[i];
> > +
> > + if (clk_data && !IS_ERR_OR_NULL(clk_data->clks[mux->id]))
> > + continue;
> > +
>
> it seems not necessary to check clk data every time
>
> and always use positive check is good to read
>
Yes, we will remove clk_data check at this point since not necessary.
the if condition check would alter next version.
> > + clk = mtk_clk_register_mux(mux, regmap, lock);
> > +
> > + if (IS_ERR(clk)) {
> > + pr_err("Failed to register clk %s: %ld\n",
> > + mux->name, PTR_ERR(clk));
> > + continue;
> > + }
> > +
> > + if (clk_data)
> > + clk_data->clks[mux->id] = clk;
>
> don't alter any data from input, that is a surprise for users
>
As I mentioned on previous question, clk_data need to be alterd because
of after registration done, we need to offer clk_data as input of
of_clk_add_provider(...,clk_data)
> > + }
> > +
> > + return 0;
> > +}
> > +
> > struct clk *mtk_clk_register_composite(const struct mtk_composite *mc,
> > void __iomem *base, spinlock_t *lock)
> > {
> > diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
> > index 1882221..61693f6 100644
> > --- a/drivers/clk/mediatek/clk-mtk.h
> > +++ b/drivers/clk/mediatek/clk-mtk.h
> > @@ -24,7 +24,9 @@
> >
> > #define MAX_MUX_GATE_BIT 31
> > #define INVALID_MUX_GATE_BIT (MAX_MUX_GATE_BIT + 1)
> > -
> > +#define INVALID_OFS -1
> > +#define INVALID_SHFT -1
> > +#define INVALID_WIDTH -1
> > #define MHZ (1000 * 1000)
> >
> > struct mtk_fixed_clk {
> > @@ -84,10 +86,72 @@ struct mtk_composite {
> > signed char num_parents;
> > };
> >
> > +struct mtk_mux {
> > + int id;
> > + const char *name;
> > + const char * const *parent_names;
> > + unsigned int flags;
> > +
> > + u32 mux_ofs;
> > + u32 set_ofs;
> > + u32 clr_ofs;
> > + u32 upd_ofs;
> > +
> > + signed char mux_shift;
> > + signed char mux_width;
> > + signed char gate_shift;
> > + signed char upd_shift;
> > +
> > + const struct clk_ops *ops;
> > +
> > + signed char num_parents;
> > +};
> > +
>
> you have created a mtk-mux.h, why is you don't move the newly create
> struct in?
>
Okay, I would remove mtk_mux structure and only preserve mtk_clk_mux in
clk_mux.h.
> > /*
> > * In case the rate change propagation to parent clocks is undesirable,
> > * this macro allows to specify the clock flags manually.
> > */
> > +#define CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, _mux_set_ofs,\
> > + _mux_clr_ofs, _shift, _width, _gate, \
> > + _upd_ofs, _upd, _flags, _ops) { \
> > + .id = _id, \
> > + .name = _name, \
> > + .mux_ofs = _mux_ofs, \
> > + .set_ofs = _mux_set_ofs, \
> > + .clr_ofs = _mux_clr_ofs, \
> > + .upd_ofs = _upd_ofs, \
> > + .mux_shift = _shift, \
> > + .mux_width = _width, \
> > + .gate_shift = _gate, \
> > + .upd_shift = _upd, \
> > + .parent_names = _parents, \
> > + .num_parents = ARRAY_SIZE(_parents), \
> > + .flags = _flags, \
> > + .ops = &_ops, \
> > + }
> > +
> > +#define MUX_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, _mux_set_ofs,\
> > + _mux_clr_ofs, _shift, _width, _gate, \
> > + _upd_ofs, _upd, _flags) \
> > + CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \
> > + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> > + _gate, _upd_ofs, _upd, _flags, \
> > + mtk_mux_clr_set_upd_ops)
> > +
> > +#define MUX_CLR_SET_UPD(_id, _name, _parents, _mux_ofs, _mux_set_ofs, \
> > + _mux_clr_ofs, _shift, _width, _gate, \
> > + _upd_ofs, _upd) \
> > + MUX_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \
> > + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> > + _gate, _upd_ofs, _upd, CLK_SET_RATE_PARENT)
> > +
> > +#define MUX_UPD(_id, _name, _parents, _mux_ofs, _shift, _width, _gate, \
> > + _upd_ofs, _upd) \
> > + CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \
> > + INVALID_OFS, INVALID_OFS, _shift, _width, \
> > + _gate, _upd_ofs, _upd, CLK_SET_RATE_PARENT, \
> > + mtk_mux_upd_ops)
> > +
> > #define MUX_GATE_FLAGS(_id, _name, _parents, _reg, _shift, _width, \
> > _gate, _flags) { \
> > .id = _id, \
> > @@ -111,18 +175,8 @@ struct mtk_composite {
> > MUX_GATE_FLAGS(_id, _name, _parents, _reg, _shift, _width, \
> > _gate, CLK_SET_RATE_PARENT)
> >
> > -#define MUX(_id, _name, _parents, _reg, _shift, _width) { \
> > - .id = _id, \
> > - .name = _name, \
> > - .mux_reg = _reg, \
> > - .mux_shift = _shift, \
> > - .mux_width = _width, \
> > - .gate_shift = -1, \
> > - .divider_shift = -1, \
> > - .parent_names = _parents, \
> > - .num_parents = ARRAY_SIZE(_parents), \
> > - .flags = CLK_SET_RATE_PARENT, \
> > - }
>
> As Matthias always said that, you alter the common thing that is already
> used by a lot of SoC.
>
> You should have a dedicate patch to state why you need it, provide the
> way you propose. and use another patches for migrating old SoC, finally
> then add the patch for your own SoC.
>
> Don't mix everything in a single patch. The patch can't become reusable
> hard to read it, even hard to backport to the other SoC picking up
> patches they are really wanting.
>
Okay, I will restore it to original version.
> > +#define MUX(_id, _name, _parents, _reg, _shift, _width) \
> > + MUX_GATE(_id, _name, _parents, _reg, _shift, _width, INVALID_SHFT)
> >
> > #define DIV_GATE(_id, _name, _parent, _gate_reg, _gate_shift, _div_reg, \
> > _div_width, _div_shift) { \
> > @@ -138,6 +192,11 @@ struct mtk_composite {
> > .flags = 0, \
> > }
> >
> > +int mtk_clk_register_muxes(const struct mtk_mux *muxes,
> > + int num, struct device_node *node,
> > + spinlock_t *lock,
> > + struct clk_onecell_data *clk_data);
> > +
>
> move to mtk-mux.h
>
Again, we can observe that other(gate/divider/fixed_clk) registration
functions all gather together in clk_mtk.h, if we want to move mtk_mux
to clk_mux.h, I think we may start a new patch to move all other three
registration functions and distribute it to their own driver code such
as clk_gate.h/clk_divider.h...etc.
> > struct clk *mtk_clk_register_composite(const struct mtk_composite *mc,
> > void __iomem *base, spinlock_t *lock);
> >
> > diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
> > new file mode 100644
> > index 0000000..219181b
> > --- /dev/null
> > +++ b/drivers/clk/mediatek/clk-mux.c
> > @@ -0,0 +1,236 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2018 MediaTek Inc.
> > + * Author: Owen Chen <owen.chen@mediatek.com>
> > + */
> > +
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/slab.h>
> > +
> > +#include "clk-mtk.h"
> > +#include "clk-mux.h"
> > +
> > +static inline struct mtk_clk_mux
> > + *to_mtk_clk_mux(struct clk_hw *hw)
> > +{
> > + return container_of(hw, struct mtk_clk_mux, hw);
> > +}
> > +
> > +static int mtk_mux_enable(struct clk_hw *hw)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 mask = BIT(mux->gate_shift);
> > + unsigned long flags = 0;
> > +
> > + if (mux->lock)
> > + spin_lock_irqsave(mux->lock, flags);
> > +
>
> we can see many functions in kernel, similar to your need, they always
> be defined as two versions. for example.
>
> mtk_mux_enable refer to lock version
>
> mtk_mux_enable_nolock for lock-free version
>
> that makes less condition, more readable, and users don't care much
> about what stuff is put inside
>
clk.c already provides spin_lock at entrance of clk_enable/clk_disalbe
API, so the lock version seems redundant here, I would remove the lock
version and keep this API lock-free as my next modification.
> > + regmap_update_bits(mux->regmap, mux->mux_ofs, mask, 0);
> > +
> > + if (mux->lock)
> > + spin_unlock_irqrestore(mux->lock, flags);
> > + return 0;
> > +}
> > +
> > +static void mtk_mux_disable(struct clk_hw *hw)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 mask = BIT(mux->gate_shift);
> > + unsigned long flags = 0;
> > +
> > + if (mux->lock)
> > + spin_lock_irqsave(mux->lock, flags);
> > +
>
> ditto
>
I would remove lock next modification.
> > + regmap_update_bits(mux->regmap, mux->mux_ofs, mask, mask);
> > +
> > + if (mux->lock)
> > + spin_unlock_irqrestore(mux->lock, flags);
> > +}
> > +
> > +static int mtk_mux_enable_setclr(struct clk_hw *hw)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 val;
> > + unsigned long flags = 0;
> > +
> > + if (mux->lock)
> > + spin_lock_irqsave(mux->lock, flags);
> > +
>
> ditto
>
I would remove lock next modification.
> > + val = BIT(mux->gate_shift);
> > + regmap_write(mux->regmap, mux->mux_clr_ofs, val);
> > +
> > + if (mux->lock)
> > + spin_unlock_irqrestore(mux->lock, flags);
> > + return 0;
> > +}
> > +
> > +static void mtk_mux_disable_setclr(struct clk_hw *hw)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 val;
> > + unsigned long flags = 0;
> > +
> > + if (mux->lock)
> > + spin_lock_irqsave(mux->lock, flags);
> > +
>
> ditto
>
I would remove lock next modification.
> > + val = BIT(mux->gate_shift);
> > + regmap_write(mux->regmap, mux->mux_set_ofs, val);
> > +
> > + if (mux->lock)
> > + spin_unlock_irqrestore(mux->lock, flags);
> > +}
> > +
> > +static int mtk_mux_is_enabled(struct clk_hw *hw)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 val = 0;
> > +
> > + if (mux->gate_shift < 0)
> > + return true;
> > +
>
> return value should be bool
>
due to the proto type of is_enabled function is defined to return
integer. I will alter the return value to 1 or 0.
> > + regmap_read(mux->regmap, mux->mux_ofs, &val);
> > +
> > + return (val & BIT(mux->gate_shift)) == 0;
> > +}
> > +
> > +static u8 mtk_mux_get_parent(struct clk_hw *hw)
> > +{
>
>
> return value should be int
Okay, I would alter it next version.
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + int num_parents = clk_hw_get_num_parents(hw);
> > + u32 mask = GENMASK(mux->mux_width - 1, 0);
> > + u32 val;
> > +
> > + regmap_read(mux->regmap, mux->mux_ofs, &val);
> > + val = (val >> mux->mux_shift) & mask;
> > +
> > + if (val >= num_parents)
> > + return -EINVAL;
> > +
> > + return val;
> > +}
> > +
> > +static int mtk_mux_set_parent(struct clk_hw *hw, u8 index)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 mask = GENMASK(mux->mux_width - 1, 0);
> > + u32 val, orig;
> > + unsigned long flags = 0;
> > +
> > + if (mux->lock)
> > + spin_lock_irqsave(mux->lock, flags);
> > +
>
> use lock-free-or-not funciton
>
I would remove lock next modification.
> > + regmap_read(mux->regmap, mux->mux_ofs, &val);
> > + orig = val;
> > + val &= ~(mask << mux->mux_shift);
> > + val |= index << mux->mux_shift;
> > +
> > + if (val != orig) {
> > + regmap_write(mux->regmap, mux->mux_ofs, val);
> > +
> > + if (mux->upd_shift >= 0)
> > + regmap_write(mux->regmap, mux->upd_ofs,
> > + BIT(mux->upd_shift));
>
>
> why not use regmap_update_bits like function ?
>
Okay, I would alter it next version..
> > + }
> > +
> > + if (mux->lock)
> > + spin_unlock_irqrestore(mux->lock, flags);
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_mux_set_parent_setclr(struct clk_hw *hw, u8 index)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 mask = GENMASK(mux->mux_width - 1, 0);
> > + u32 val, orig;
> > + unsigned long flags = 0;
> > +
> > + if (mux->lock)
> > + spin_lock_irqsave(mux->lock, flags);
> > +
>
> use lock-free-or-not funciton
>
I would remove lock next modification.
> > + regmap_read(mux->regmap, mux->mux_ofs, &val);
> > + orig = val;
> > + val &= ~(mask << mux->mux_shift);
> > + val |= index << mux->mux_shift;
> > +
> > + if (val != orig) {
> > + val = (mask << mux->mux_shift);
> > + regmap_write(mux->regmap, mux->mux_clr_ofs, val);
> > + val = (index << mux->mux_shift);
> > + regmap_write(mux->regmap, mux->mux_set_ofs, val);
> > +
>
> why not use regmap_update_bits like function ?
This function specified that mux use set/clr register to update, so it
would be better to use regmap_write to write the whole register instead
of regmap_update_bit.
> > + if (mux->upd_shift >= 0)
> > + regmap_write(mux->regmap, mux->upd_ofs,
> > + BIT(mux->upd_shift));
> > + }
> > +
> > + if (mux->lock)
> > + spin_unlock_irqrestore(mux->lock, flags);
> > +
> > + return 0;
> > +}
> > +
> > +const struct clk_ops mtk_mux_upd_ops = {
> > + .enable = mtk_mux_enable,
> > + .disable = mtk_mux_disable,
> > + .is_enabled = mtk_mux_is_enabled,
> > + .get_parent = mtk_mux_get_parent,
> > + .set_parent = mtk_mux_set_parent,
> > + .determine_rate = NULL,
>
> explicitly set as NULL can be removed
Okay, I would remove it next version.
> > +};
> > +
> > +const struct clk_ops mtk_mux_clr_set_upd_ops = {
> > + .enable = mtk_mux_enable_setclr,
> > + .disable = mtk_mux_disable_setclr,
> > + .is_enabled = mtk_mux_is_enabled,
> > + .get_parent = mtk_mux_get_parent,
> > + .set_parent = mtk_mux_set_parent_setclr,
> > + .determine_rate = NULL,
>
> explicitly set as NULL can be removed
Okay, I would remove it next version.
> > +};
> > +
> > +struct clk *mtk_clk_register_mux(const struct mtk_mux *mux,
> > + struct regmap *regmap,
> > + spinlock_t *lock)
> > +{
> > + struct clk *clk;
> > + struct clk_init_data init;
> > + struct mtk_clk_mux *mtk_mux = NULL;
> > + int ret;
> > +
> make declaration as reverse xmas tree
>
Okay, I would alter it next version.
> > + mtk_mux = kzalloc(sizeof(*mtk_mux), GFP_KERNEL);
> > + if (!mtk_mux)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + init.name = mux->name;
> > + init.flags = (mux->flags) | CLK_SET_RATE_PARENT;
> > + init.parent_names = mux->parent_names;
> > + init.num_parents = mux->num_parents;
> > + init.ops = mux->ops;
> > +
> > + mtk_mux->regmap = regmap;
> > + mtk_mux->name = mux->name;
> > + mtk_mux->mux_ofs = mux->mux_ofs;
> > + mtk_mux->mux_set_ofs = mux->set_ofs;
> > + mtk_mux->mux_clr_ofs = mux->clr_ofs;
> > + mtk_mux->upd_ofs = mux->upd_ofs;
> > + mtk_mux->mux_shift = mux->mux_shift;
> > + mtk_mux->mux_width = mux->mux_width;
> > + mtk_mux->gate_shift = mux->gate_shift;
> > + mtk_mux->upd_shift = mux->upd_shift;
> > +
> > + mtk_mux->lock = lock;
> > + mtk_mux->hw.init = &init;
> > +
> > + clk = clk_register(NULL, &mtk_mux->hw);
> > + if (IS_ERR(clk)) {
> > + ret = PTR_ERR(clk);
>
>
> ret is superfluous
>
Okay, I would remove it next version.
> > + goto err_out;
> > + }
> > +
> > + return clk;
> > +err_out:
> > + kfree(mtk_mux);
> > +
>
> I felt err path can be optimized
>
Okay, I would remove it next version.
> > + return ERR_PTR(ret);
> > +}
> > diff --git a/drivers/clk/mediatek/clk-mux.h b/drivers/clk/mediatek/clk-mux.h
> > new file mode 100644
> > index 0000000..64f8e7c
> > --- /dev/null
> > +++ b/drivers/clk/mediatek/clk-mux.h
> > @@ -0,0 +1,38 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (c) 2018 MediaTek Inc.
> > + * Author: Owen Chen <owen.chen@mediatek.com>
> > + */
> > +
> > +#ifndef __DRV_CLK_MUX_H
> > +#define __DRV_CLK_MUX_H
> > +
> > +#include <linux/clk-provider.h>
> > +
> > +struct mtk_clk_mux {
> > + struct clk_hw hw;
> > + struct regmap *regmap;
> > +
> > + const char *name;
> > +
> > + int mux_set_ofs;
> > + int mux_clr_ofs;
> > + int mux_ofs;
> > + int upd_ofs;
> > +
> > + s8 mux_shift;
> > + s8 mux_width;
> > + s8 gate_shift;
> > + s8 upd_shift;
> > +
> > + spinlock_t *lock;
> > +};
> > +
> > +extern const struct clk_ops mtk_mux_upd_ops;
> > +extern const struct clk_ops mtk_mux_clr_set_upd_ops;
> > +
>
>
> extern is superfluous
>
Okay, I would remove it next version.
> > +struct clk *mtk_clk_register_mux(const struct mtk_mux *mux,
> > + struct regmap *regmap,
> > + spinlock_t *lock);
> > +
> > +#endif /* __DRV_CLK_MUX_H */
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox