* Re: [RFC PATCH v1 08/25] printk: add ring buffer and kthread
From: Petr Mladek @ 2019-02-19 13:54 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20190212143003.48446-9-john.ogness@linutronix.de>
On Tue 2019-02-12 15:29:46, John Ogness wrote:
> The printk ring buffer provides an NMI-safe interface for writing
> messages to a ring buffer. Using such a buffer for alleviates printk
> callers from the current burdens of disabled preemption while calling
> the console drivers (and possibly printing out many messages that
> another task put into the log buffer).
>
> Create a ring buffer to be used for storing messages to be
> printed to the consoles.
>
> Create a dedicated printk kthread to block on the ring buffer
> and call the console drivers for the read messages.
It was already mentioned in the reply to the cover letter.
An offloading console handling to a kthread is very problematic:
+ It reduces the chance that the messages will ever reach
the console when the system becomes unstable or is dying.
Note that panic() is the better alternative. The system
could die without calling panic().
+ There are situations when the kthread will not be usable
by definition, for example, early boot, suspend, kexec,
shutdown.
+ It is hard to do a single thread efficient enough. It could
not have too high priority to avoid staling normal processes.
It won't be scheduled in time when processed with a higher
priority produce too many messages.
That said, I think that some offloading would be useful and
even necessary, especially on the real time systems. But we
need to be more conservative, for example:
+ offload only when it takes too long
+ do not offload in emergency situations
+ keep the console owner logic
The above ideas come from the old discussions about introducing
printk kthread. This patchset brings one more idea to push
emergency messages directly to "lockless" consoles.
I could imagine that the default setting will be more
conservative while real time kernel would do a more
aggressive offloading.
Anyway, I would split this entire patchset into two.
The first one would replace the existing main buffer
and per-cpu safe buffers with the lockless one.
The other patchset would try to reduce delays caused
by console handling by introducing lockless consoles,
offloading, ...
> NOTE: The printk_delay is relocated to _after_ the message is
> printed, where it makes more sense.
>
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
> ---
> kernel/printk/printk.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 105 insertions(+)
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index d3d170374ceb..08e079b95652 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -44,6 +44,8 @@
> #include <linux/irq_work.h>
> #include <linux/ctype.h>
> #include <linux/uio.h>
> +#include <linux/kthread.h>
> +#include <linux/printk_ringbuffer.h>
> #include <linux/sched/clock.h>
> #include <linux/sched/debug.h>
> #include <linux/sched/task_stack.h>
> @@ -397,7 +399,12 @@ DEFINE_RAW_SPINLOCK(logbuf_lock);
> printk_safe_exit_irqrestore(flags); \
> } while (0)
>
> +DECLARE_STATIC_PRINTKRB_CPULOCK(printk_cpulock);
> +
> #ifdef CONFIG_PRINTK
> +/* record buffer */
> +DECLARE_STATIC_PRINTKRB(printk_rb, CONFIG_LOG_BUF_SHIFT, &printk_cpulock);
> +
> DECLARE_WAIT_QUEUE_HEAD(log_wait);
> /* the next printk record to read by syslog(READ) or /proc/kmsg */
> static u64 syslog_seq;
> @@ -744,6 +751,10 @@ static ssize_t msg_print_ext_body(char *buf, size_t size,
> return p - buf;
> }
>
> +#define PRINTK_SPRINT_MAX (LOG_LINE_MAX + PREFIX_MAX)
> +#define PRINTK_RECORD_MAX (sizeof(struct printk_log) + \
> + CONSOLE_EXT_LOG_MAX + PRINTK_SPRINT_MAX)
> +
> /* /dev/kmsg - userspace message inject/listen interface */
> struct devkmsg_user {
> u64 seq;
> @@ -1566,6 +1577,34 @@ SYSCALL_DEFINE3(syslog, int, type, char __user *, buf, int, len)
> return do_syslog(type, buf, len, SYSLOG_FROM_READER);
> }
>
> +static void format_text(struct printk_log *msg, u64 seq,
> + char *ext_text, size_t *ext_len,
> + char *text, size_t *len, bool time)
> +{
> + if (suppress_message_printing(msg->level)) {
> + /*
> + * Skip record that has level above the console
> + * loglevel and update each console's local seq.
> + */
> + *len = 0;
> + *ext_len = 0;
> + return;
> + }
> +
> + *len = msg_print_text(msg, console_msg_format & MSG_FORMAT_SYSLOG,
> + time, text, PRINTK_SPRINT_MAX);
> + if (nr_ext_console_drivers) {
> + *ext_len = msg_print_ext_header(ext_text, CONSOLE_EXT_LOG_MAX,
> + msg, seq);
> + *ext_len += msg_print_ext_body(ext_text + *ext_len,
> + CONSOLE_EXT_LOG_MAX - *ext_len,
> + log_dict(msg), msg->dict_len,
> + log_text(msg), msg->text_len);
> + } else {
> + *ext_len = 0;
> + }
> +}
Please, refactor the existing code instead of cut&pasting. It will
help to check eventual regressions. Also it will help to understand
the evolution when digging in the git history. [*]
> +
> /*
> * Special console_lock variants that help to reduce the risk of soft-lockups.
> * They allow to pass console_lock to another printk() call using a busy wait.
> @@ -2899,6 +2938,72 @@ void wake_up_klogd(void)
> preempt_enable();
> }
>
> +static int printk_kthread_func(void *data)
> +{
> + struct prb_iterator iter;
> + struct printk_log *msg;
> + size_t ext_len;
> + char *ext_text;
> + u64 master_seq;
> + size_t len;
> + char *text;
> + char *buf;
> + int ret;
> +
> + ext_text = kmalloc(CONSOLE_EXT_LOG_MAX, GFP_KERNEL);
> + text = kmalloc(PRINTK_SPRINT_MAX, GFP_KERNEL);
> + buf = kmalloc(PRINTK_RECORD_MAX, GFP_KERNEL);
> + if (!ext_text || !text || !buf)
> + return -1;
We need to have some fallback solution when the kthread can't
get started properly.
> +
> + prb_iter_init(&iter, &printk_rb, NULL);
> +
> + /* the printk kthread never exits */
> + for (;;) {
> + ret = prb_iter_wait_next(&iter, buf,
> + PRINTK_RECORD_MAX, &master_seq);
> + if (ret == -ERESTARTSYS) {
> + continue;
> + } else if (ret < 0) {
> + /* iterator invalid, start over */
> + prb_iter_init(&iter, &printk_rb, NULL);
> + continue;
> + }
> +
> + msg = (struct printk_log *)buf;
> + format_text(msg, master_seq, ext_text, &ext_len, text,
> + &len, printk_time);
> +
> + console_lock();
> + if (len > 0 || ext_len > 0) {
> + call_console_drivers(ext_text, ext_len, text, len);
> + boot_delay_msec(msg->level);
> + printk_delay();
> + }
> + console_unlock();
This calls console_unlock() that still calls console drivers at this
stage. Note the each patch should keep the kernel buildable and
avoid regressions. Otherwise, it would break bisection. [*]
> + }
> +
> + kfree(ext_text);
> + kfree(text);
> + kfree(buf);
This will never get called. Well, I think that the kthread
will look different when it is used only as a callback.
We will need some buffers also when the direct console
is handled by the printk() caller directly.
> + return 0;
> +}
> +
[*] I am not sure whether I should comment this "details" at this
stage [RFC]. I guess that you worked on this patchset many
weeks or even months. You tried various approaches,
refactored the code several times. It is hard to keep
such a big patchset clean. It might even be wasting of time.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH] tty: not call tty close in fallback
From: Greg KH @ 2019-02-19 12:41 UTC (permalink / raw)
To: yes; +Cc: robh, jslaby, linux-serial, linux-kernel
In-Reply-To: <5c3c3289.1c69fb81.c1953.0900SMTPIN_ADDED_BROKEN@mx.google.com>
On Mon, Jan 14, 2019 at 02:56:01PM +0800, yes wrote:
> From: Li RongQing <lirongqing@baidu.com>
>
> when fail to open tty, tty is not in open status and not need
> to call close
>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
> drivers/tty/serdev/serdev-ttyport.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
> index fa1672993b4c..bcc1e27d00de 100644
> --- a/drivers/tty/serdev/serdev-ttyport.c
> +++ b/drivers/tty/serdev/serdev-ttyport.c
> @@ -121,7 +121,7 @@ static int ttyport_open(struct serdev_controller *ctrl)
>
> ret = tty->ops->open(serport->tty, NULL);
> if (ret)
> - goto err_close;
> + goto err_unlock;
>
> tty_unlock(serport->tty);
>
> @@ -142,8 +142,6 @@ static int ttyport_open(struct serdev_controller *ctrl)
>
> return 0;
>
> -err_close:
> - tty->ops->close(tty, NULL);
No, close is always called after open, even if open fails.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v10 1/3] dmaengine: 8250_mtk_dma: add MediaTek uart DMA support
From: Vinod Koul @ 2019-02-19 12:14 UTC (permalink / raw)
To: Long Cheng
Cc: Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee, Sean Wang,
Nicolas Boichat, Matthias Brugger, Dan Williams,
Greg Kroah-Hartman, Jiri Slaby, Sean Wang, dmaengine, devicetree,
linux-arm-kernel, linux-mediatek, linux-kernel, linux-serial,
srv_heupstream, Yingjoe Chen, YT Shen, Zhenbao Liu
In-Reply-To: <1550112296.7678.5.camel@mhfsdcap03>
On 14-02-19, 10:44, Long Cheng wrote:
> On Mon, 2019-02-04 at 12:51 +0530, Vinod Koul wrote:
> > > +static int mtk_uart_apdma_device_pause(struct dma_chan *chan)
> > > +{
> > > + /* just for check caps pass */
> > > + return 0;
> > > +}
> >
> > please remove, this is not a mandatory fn
>
> Can't remove it. Before the mail, i had explained it. in 8250 uart
> framework, will check the function..
I think you should fix the 8250 uart to handle driver not having a
pause. Keeping a dummy pause is just wrong and sets wrong expectations
for uart..
--
~Vinod
^ permalink raw reply
* Re: [PATCH v2 09/15] dt-bindings: serial: Add Milbeaut serial driver description
From: Sugaya, Taichi @ 2019-02-19 8:11 UTC (permalink / raw)
To: Rob Herring
Cc: linux-serial, devicetree, linux-arm-kernel, linux-kernel,
Greg Kroah-Hartman, Mark Rutland, Takao Orito, Kazuhiro Kasai,
Shinji Kanematsu, Jassi Brar, Masami Hiramatsu
In-Reply-To: <20190218195713.GA18625@bogus>
Hi,
On 2019/02/19 4:57, Rob Herring wrote:
> On Fri, Feb 08, 2019 at 09:27:29PM +0900, Sugaya Taichi wrote:
>> Add DT bindings document for Milbeaut serial driver.
>>
>> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
>> ---
>> .../devicetree/bindings/serial/milbeaut-uart.txt | 21 +++++++++++++++++++++
>> 1 file changed, 21 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/serial/milbeaut-uart.txt
>>
>> diff --git a/Documentation/devicetree/bindings/serial/milbeaut-uart.txt b/Documentation/devicetree/bindings/serial/milbeaut-uart.txt
>> new file mode 100644
>> index 0000000..8f61c38
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/serial/milbeaut-uart.txt
>> @@ -0,0 +1,21 @@
>> +Socionext Milbeaut UART controller
>> +
>> +Required properties:
>> +- compatible: should be "socionext,milbeaut-usio-uart".
>> +- reg: offset and length of the register set for the device.
>> +- interrupts: two interrupts specifier.
>> +- clocks: phandle to the input clock.
>> +- interrupt-names: should be "rx", "tx".
>
> Put this next to 'interrupts'
>
OK, change the order.
>> +
>> +Optional properties:
>> +- uart-flow-enable: flow control enable.
>
> We already have a standard property for this: auto-flow-control
>
Okay, I change the property name to "auto-flow-control"
>> +
>> +Example:
>> + usio1: usio_uart@1e700010 {
>
> serial@...
>
I got it.
Thanks,
Sugaya Taichi
>> + compatible = "socionext,milbeaut-usio-uart";
>> + reg = <0x1e700010 0x10>;
>> + interrupts = <0 141 0x4>, <0 149 0x4>;
>> + interrupt-names = "rx", "tx";
>> + clocks = <&clk 2>;
>> + uart-flow-enable;
>> + };
>> --
>> 1.9.1
>>
^ permalink raw reply
* Re: [PATCH v7 5/6] arm64: dts: mt8183: add pintcrl file
From: Nicolas Boichat @ 2019-02-19 7:45 UTC (permalink / raw)
To: Erin Lo
Cc: Linus Walleij, Matthias Brugger, Rob Herring, Mark Rutland,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Greg Kroah-Hartman,
Stephen Boyd, devicetree, srv_heupstream, lkml, linux-serial,
moderated list:ARM/Mediatek SoC support, linux-arm Mailing List,
Yingjoe Chen, Cheng Mars, Eddie Huang
In-Reply-To: <1550210558-30516-6-git-send-email-erin.lo@mediatek.com>
On Fri, Feb 15, 2019 at 2:07 PM Erin Lo <erin.lo@mediatek.com> wrote:
> Subject: [PATCH v7 5/6] arm64: dts: mt8183: add pintcrl file
minor spelling mistake in the commit title: pinctrl
>
> This patch adds pinctrl file for mt8183.
>
> Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com>
> Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> ---
^ permalink raw reply
* Re: [PATCH v2 00/15] Add basic support for Socionext Milbeaut M10V SoC
From: Sugaya, Taichi @ 2019-02-19 7:38 UTC (permalink / raw)
To: Arnd Bergmann
Cc: DTML, Linux Kernel Mailing List, Linux ARM, linux-clk,
open list:GPIO SUBSYSTEM, linux-serial, Rob Herring, Mark Rutland,
Michael Turquette, Stephen Boyd, Linus Walleij,
Greg Kroah-Hartman, Daniel Lezcano, Thomas Gleixner, Russell King,
Jiri Slaby, Takao Orito, Kazuhiro Kasai, Shinji
In-Reply-To: <CAK8P3a3_j2W+KXSynW_SkVZk_+PJ-DqyxjLkgJL-OichS0fwWw@mail.gmail.com>
Hi,
Thank you for your comments.
On 2019/02/18 21:20, Arnd Bergmann wrote:
> On Fri, Feb 8, 2019 at 1:24 PM Sugaya Taichi
> <sugaya.taichi@socionext.com> wrote:
>>
>> Hi,
>>
>> Here is the series of patches the initial support for SC2000(M10V) of
>> Milbeaut SoCs. "M10V" is the internal name of SC2000, so commonly used in
>> source code.
>>
>> SC2000 is a SoC of the Milbeaut series. equipped with a DSP optimized for
>> computer vision. It also features advanced functionalities such as 360-degree,
>> real-time spherical stitching with multi cameras, image stabilization for
>> without mechanical gimbals, and rolling shutter correction. More detail is
>> below:
>> https://www.socionext.com/en/products/assp/milbeaut/SC2000.html
>>
>> Specifications for developers are below:
>> - Quad-core 32bit Cortex-A7 on ARMv7-A architecture
>> - NEON support
>> - DSP
>> - GPU
>> - MAX 3GB DDR3
>> - Cortex-M0 for power control
>> - NAND Flash Interface
>> - SD UHS-I
>> - SD UHS-II
>> - SDIO
>> - USB2.0 HOST / Device
>> - USB3.0 HOST / Device
>> - PCI express Gen2
>> - Ethernet Engine
>> - I2C
>> - UART
>> - SPI
>> - PWM
>>
>> Support is quite minimal for now, since it only includes timer, clock,
>> pictrl and serial controller drivers, so we can only boot to userspace
>> through initramfs. Support for the other peripherals will come eventually.
>
> I've looked over the platform once more. Overall, it looks very good, and
> I'd still like to merge this for linux-5.1, but we are running out of time
> there. If you send the patches to soc@kernel.org quickly, we can try to
> still merge them in, but if anything goes wrong, it will have to wait until
> we start merging patches for 5.2, directly after 5.1 is out.
>
I see.
I do my best to make it.
> I did not look at the device driver patches (clk, clocksource, pinctrl, serial)
> in much detail. If you have an Ack from the maintainers, feel free to
> include them in the series, otherwise let's merge the rest now and then
> you can send the updated patches for inclusion through the subsystem
> trees in 5.2.
>
OK, I'll add the patches with "Acked" or "Reviewed" tag.
> I have sent a few comments. The only one that is really important
> here is the missing platform check in the suspend options. Please
> try to address most of the other commentsm, either by changing the
> code, or by explaining why your version is correct.
>
Okay, try to address them.
I send the next version as soon as possible.
Thanks.
Sugaya Taichi
>
>
> Arnd
>
^ permalink raw reply
* [PATCH 5/5] serial: sprd: Add DMA mode support
From: Baolin Wang @ 2019-02-19 7:31 UTC (permalink / raw)
To: gregkh, jslaby, robh+dt, mark.rutland, orsonzhai, zhang.lyra
Cc: baolin.wang, broonie, lanqing.liu, linux-serial, linux-kernel,
devicetree
In-Reply-To: <cover.1550560916.git.baolin.wang@linaro.org>
From: Lanqing Liu <lanqing.liu@unisoc.com>
Add DMA mode support for the Spreadtrum serial controller.
Signed-off-by: Lanqing Liu <lanqing.liu@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/tty/serial/sprd_serial.c | 440 ++++++++++++++++++++++++++++++++++++--
1 file changed, 426 insertions(+), 14 deletions(-)
diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
index 8f45b66..6aebd77 100644
--- a/drivers/tty/serial/sprd_serial.c
+++ b/drivers/tty/serial/sprd_serial.c
@@ -10,6 +10,9 @@
#include <linux/clk.h>
#include <linux/console.h>
#include <linux/delay.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/dma/sprd-dma.h>
#include <linux/io.h>
#include <linux/ioport.h>
#include <linux/kernel.h>
@@ -75,6 +78,7 @@
/* control register 1 */
#define SPRD_CTL1 0x001C
+#define SPRD_DMA_EN BIT(15)
#define RX_HW_FLOW_CTL_THLD BIT(6)
#define RX_HW_FLOW_CTL_EN BIT(7)
#define TX_HW_FLOW_CTL_EN BIT(8)
@@ -86,6 +90,7 @@
#define THLD_TX_EMPTY 0x40
#define THLD_TX_EMPTY_SHIFT 8
#define THLD_RX_FULL 0x40
+#define THLD_RX_FULL_MASK GENMASK(6, 0)
/* config baud rate register */
#define SPRD_CLKD0 0x0024
@@ -102,15 +107,36 @@
#define SPRD_IMSR_TIMEOUT BIT(13)
#define SPRD_DEFAULT_SOURCE_CLK 26000000
+#define SPRD_RX_DMA_STEP 1
+#define SPRD_RX_FIFO_FULL 1
+#define SPRD_TX_FIFO_FULL 0x20
+#define SPRD_UART_RX_SIZE (UART_XMIT_SIZE / 4)
+
+struct sprd_uart_dma {
+ struct dma_chan *chn;
+ unsigned char *virt;
+ dma_addr_t phys_addr;
+ dma_cookie_t cookie;
+ u32 trans_len;
+ bool enable;
+};
+
struct sprd_uart_port {
struct uart_port port;
char name[16];
struct clk *clk;
+ struct sprd_uart_dma tx_dma;
+ struct sprd_uart_dma rx_dma;
+ dma_addr_t pos;
+ unsigned char *rx_buf_tail;
};
static struct sprd_uart_port *sprd_port[UART_NR_MAX];
static int sprd_ports_num;
+static int sprd_start_dma_rx(struct uart_port *port);
+static int sprd_tx_dma_config(struct uart_port *port);
+
static inline unsigned int serial_in(struct uart_port *port,
unsigned int offset)
{
@@ -141,45 +167,389 @@ static void sprd_set_mctrl(struct uart_port *port, unsigned int mctrl)
/* nothing to do */
}
-static void sprd_stop_tx(struct uart_port *port)
+static void sprd_stop_rx(struct uart_port *port)
{
+ struct sprd_uart_port *sp =
+ container_of(port, struct sprd_uart_port, port);
unsigned int ien, iclr;
+ if (sp->rx_dma.enable)
+ dmaengine_terminate_all(sp->rx_dma.chn);
+
iclr = serial_in(port, SPRD_ICLR);
ien = serial_in(port, SPRD_IEN);
- iclr |= SPRD_IEN_TX_EMPTY;
- ien &= ~SPRD_IEN_TX_EMPTY;
+ ien &= ~(SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT);
+ iclr |= SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT;
- serial_out(port, SPRD_ICLR, iclr);
serial_out(port, SPRD_IEN, ien);
+ serial_out(port, SPRD_ICLR, iclr);
}
-static void sprd_start_tx(struct uart_port *port)
+static void sprd_uart_dma_enable(struct uart_port *port, bool enable)
{
- unsigned int ien;
+ u32 val = serial_in(port, SPRD_CTL1);
- ien = serial_in(port, SPRD_IEN);
- if (!(ien & SPRD_IEN_TX_EMPTY)) {
- ien |= SPRD_IEN_TX_EMPTY;
- serial_out(port, SPRD_IEN, ien);
+ if (enable)
+ val |= SPRD_DMA_EN;
+ else
+ val &= ~SPRD_DMA_EN;
+
+ serial_out(port, SPRD_CTL1, val);
+}
+
+static void sprd_stop_tx_dma(struct uart_port *port)
+{
+ struct sprd_uart_port *sp =
+ container_of(port, struct sprd_uart_port, port);
+ struct circ_buf *xmit = &port->state->xmit;
+ struct dma_tx_state state;
+ u32 trans_len;
+
+ dmaengine_pause(sp->tx_dma.chn);
+
+ dmaengine_tx_status(sp->tx_dma.chn, sp->tx_dma.cookie, &state);
+ if (state.residue) {
+ trans_len = state.residue - sp->tx_dma.phys_addr;
+ xmit->tail = (xmit->tail + trans_len) & (UART_XMIT_SIZE - 1);
+ port->icount.tx += trans_len;
+ dma_unmap_single(port->dev, sp->tx_dma.phys_addr,
+ sp->tx_dma.trans_len, DMA_TO_DEVICE);
}
+
+ dmaengine_terminate_all(sp->tx_dma.chn);
+ sp->tx_dma.trans_len = 0;
}
-static void sprd_stop_rx(struct uart_port *port)
+static int sprd_tx_buf_remap(struct uart_port *port)
+{
+ struct sprd_uart_port *sp =
+ container_of(port, struct sprd_uart_port, port);
+ struct circ_buf *xmit = &port->state->xmit;
+
+ sp->tx_dma.trans_len =
+ CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
+
+ sp->tx_dma.phys_addr = dma_map_single(port->dev,
+ (void *)&(xmit->buf[xmit->tail]),
+ sp->tx_dma.trans_len,
+ DMA_TO_DEVICE);
+ return dma_mapping_error(port->dev, sp->tx_dma.phys_addr);
+}
+
+static void sprd_complete_tx_dma(void *data)
+{
+ struct uart_port *port = (struct uart_port *)data;
+ struct sprd_uart_port *sp =
+ container_of(port, struct sprd_uart_port, port);
+ struct circ_buf *xmit = &port->state->xmit;
+ unsigned long flags;
+
+ spin_lock_irqsave(&port->lock, flags);
+ dma_unmap_single(port->dev, sp->tx_dma.phys_addr,
+ sp->tx_dma.trans_len, DMA_TO_DEVICE);
+
+ xmit->tail = (xmit->tail + sp->tx_dma.trans_len) & (UART_XMIT_SIZE - 1);
+ port->icount.tx += sp->tx_dma.trans_len;
+
+ if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+ uart_write_wakeup(port);
+
+ if (uart_circ_empty(xmit) || sprd_tx_buf_remap(port) ||
+ sprd_tx_dma_config(port))
+ sp->tx_dma.trans_len = 0;
+
+ spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static int sprd_uart_dma_submit(struct uart_port *port,
+ struct sprd_uart_dma *ud, u32 trans_len,
+ enum dma_transfer_direction direction,
+ dma_async_tx_callback callback)
{
+ struct dma_async_tx_descriptor *dma_des;
+ unsigned long flags;
+
+ flags = SPRD_DMA_FLAGS(SPRD_DMA_CHN_MODE_NONE,
+ SPRD_DMA_NO_TRG,
+ SPRD_DMA_FRAG_REQ,
+ SPRD_DMA_TRANS_INT);
+
+ dma_des = dmaengine_prep_slave_single(ud->chn, ud->phys_addr, trans_len,
+ direction, flags);
+ if (!dma_des)
+ return -ENODEV;
+
+ dma_des->callback = callback;
+ dma_des->callback_param = port;
+
+ ud->cookie = dmaengine_submit(dma_des);
+ if (dma_submit_error(ud->cookie))
+ return dma_submit_error(ud->cookie);
+
+ dma_async_issue_pending(ud->chn);
+
+ return 0;
+}
+
+static int sprd_tx_dma_config(struct uart_port *port)
+{
+ struct sprd_uart_port *sp =
+ container_of(port, struct sprd_uart_port, port);
+ u32 burst = sp->tx_dma.trans_len > SPRD_TX_FIFO_FULL ?
+ SPRD_TX_FIFO_FULL : sp->tx_dma.trans_len;
+ int ret;
+ struct dma_slave_config cfg = {
+ .dst_addr = port->mapbase + SPRD_TXD,
+ .src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE,
+ .dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE,
+ .src_maxburst = burst,
+ };
+
+ ret = dmaengine_slave_config(sp->tx_dma.chn, &cfg);
+ if (ret < 0)
+ return ret;
+
+ return sprd_uart_dma_submit(port, &sp->tx_dma, sp->tx_dma.trans_len,
+ DMA_MEM_TO_DEV, sprd_complete_tx_dma);
+}
+
+static void sprd_start_tx_dma(struct uart_port *port)
+{
+ struct sprd_uart_port *sp =
+ container_of(port, struct sprd_uart_port, port);
+ struct circ_buf *xmit = &port->state->xmit;
+
+ if (port->x_char) {
+ serial_out(port, SPRD_TXD, port->x_char);
+ port->icount.tx++;
+ port->x_char = 0;
+ return;
+ }
+
+ if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
+ sprd_stop_tx_dma(port);
+ return;
+ }
+
+ if (sp->tx_dma.trans_len)
+ return;
+
+ if (sprd_tx_buf_remap(port) || sprd_tx_dma_config(port))
+ sp->tx_dma.trans_len = 0;
+}
+
+static void sprd_rx_full_thld(struct uart_port *port, u32 thld)
+{
+ u32 val = serial_in(port, SPRD_CTL2);
+
+ val &= ~THLD_RX_FULL_MASK;
+ val |= thld & THLD_RX_FULL_MASK;
+ serial_out(port, SPRD_CTL2, val);
+}
+
+static int sprd_rx_alloc_buf(struct sprd_uart_port *sp)
+{
+ sp->rx_dma.virt = dma_alloc_coherent(sp->port.dev, SPRD_UART_RX_SIZE,
+ &sp->rx_dma.phys_addr, GFP_KERNEL);
+ if (!sp->rx_dma.virt)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static void sprd_rx_free_buf(struct sprd_uart_port *sp)
+{
+ if (sp->rx_dma.virt)
+ dma_free_coherent(sp->port.dev, SPRD_UART_RX_SIZE,
+ sp->rx_dma.virt, sp->rx_dma.phys_addr);
+
+}
+
+static int sprd_rx_dma_config(struct uart_port *port, u32 burst)
+{
+ struct sprd_uart_port *sp =
+ container_of(port, struct sprd_uart_port, port);
+ struct dma_slave_config cfg = {
+ .src_addr = port->mapbase + SPRD_RXD,
+ .src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE,
+ .dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE,
+ .src_maxburst = burst,
+ };
+
+ return dmaengine_slave_config(sp->rx_dma.chn, &cfg);
+}
+
+static void sprd_uart_dma_rx(struct uart_port *port)
+{
+ struct sprd_uart_port *sp =
+ container_of(port, struct sprd_uart_port, port);
+ struct tty_port *tty = &port->state->port;
+
+ port->icount.rx += sp->rx_dma.trans_len;
+ tty_insert_flip_string(tty, sp->rx_buf_tail, sp->rx_dma.trans_len);
+ tty_flip_buffer_push(tty);
+}
+
+static void sprd_uart_dma_irq(struct uart_port *port)
+{
+ struct sprd_uart_port *sp =
+ container_of(port, struct sprd_uart_port, port);
+ struct dma_tx_state state;
+ enum dma_status status;
+
+ status = dmaengine_tx_status(sp->rx_dma.chn,
+ sp->rx_dma.cookie, &state);
+ if (status == DMA_ERROR)
+ sprd_stop_rx(port);
+
+ if (!state.residue && sp->pos == sp->rx_dma.phys_addr)
+ return;
+
+ if (!state.residue) {
+ sp->rx_dma.trans_len = SPRD_UART_RX_SIZE +
+ sp->rx_dma.phys_addr - sp->pos;
+ sp->pos = sp->rx_dma.phys_addr;
+ } else {
+ sp->rx_dma.trans_len = state.residue - sp->pos;
+ sp->pos = state.residue;
+ }
+
+ sprd_uart_dma_rx(port);
+ sp->rx_buf_tail += sp->rx_dma.trans_len;
+}
+
+static void sprd_complete_rx_dma(void *data)
+{
+ struct uart_port *port = (struct uart_port *)data;
+ struct sprd_uart_port *sp =
+ container_of(port, struct sprd_uart_port, port);
+ struct dma_tx_state state;
+ enum dma_status status;
+ unsigned long flags;
+
+ spin_lock_irqsave(&port->lock, flags);
+
+ status = dmaengine_tx_status(sp->rx_dma.chn,
+ sp->rx_dma.cookie, &state);
+ if (status != DMA_COMPLETE) {
+ sprd_stop_rx(port);
+ spin_unlock_irqrestore(&port->lock, flags);
+ return;
+ }
+
+ if (sp->pos != sp->rx_dma.phys_addr) {
+ sp->rx_dma.trans_len = SPRD_UART_RX_SIZE +
+ sp->rx_dma.phys_addr - sp->pos;
+ sprd_uart_dma_rx(port);
+ sp->rx_buf_tail += sp->rx_dma.trans_len;
+ }
+
+ if (sprd_start_dma_rx(port))
+ sprd_stop_rx(port);
+
+ spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static int sprd_start_dma_rx(struct uart_port *port)
+{
+ struct sprd_uart_port *sp =
+ container_of(port, struct sprd_uart_port, port);
+ int ret;
+
+ if (!sp->rx_dma.enable)
+ return 0;
+
+ sp->pos = sp->rx_dma.phys_addr;
+ sp->rx_buf_tail = sp->rx_dma.virt;
+ sprd_rx_full_thld(port, SPRD_RX_FIFO_FULL);
+ ret = sprd_rx_dma_config(port, SPRD_RX_DMA_STEP);
+ if (ret)
+ return ret;
+
+ return sprd_uart_dma_submit(port, &sp->rx_dma, SPRD_UART_RX_SIZE,
+ DMA_DEV_TO_MEM, sprd_complete_rx_dma);
+}
+
+static void sprd_release_dma(struct uart_port *port)
+{
+ struct sprd_uart_port *sp =
+ container_of(port, struct sprd_uart_port, port);
+
+ sprd_uart_dma_enable(port, false);
+
+ if (sp->rx_dma.enable)
+ dma_release_channel(sp->rx_dma.chn);
+
+ if (sp->tx_dma.enable)
+ dma_release_channel(sp->tx_dma.chn);
+
+ sp->tx_dma.enable = false;
+ sp->rx_dma.enable = false;
+}
+
+static void sprd_request_dma(struct uart_port *port)
+{
+ struct sprd_uart_port *sp =
+ container_of(port, struct sprd_uart_port, port);
+
+ sp->tx_dma.enable = true;
+ sp->rx_dma.enable = true;
+
+ sp->tx_dma.chn = dma_request_chan(port->dev, "tx");
+ if (IS_ERR(sp->tx_dma.chn)) {
+ dev_err(port->dev, "request TX DMA channel failed, ret = %ld\n",
+ PTR_ERR(sp->tx_dma.chn));
+ sp->tx_dma.enable = false;
+ }
+
+ sp->rx_dma.chn = dma_request_chan(port->dev, "rx");
+ if (IS_ERR(sp->rx_dma.chn)) {
+ dev_err(port->dev, "request RX DMA channel failed, ret = %ld\n",
+ PTR_ERR(sp->tx_dma.chn));
+ sp->rx_dma.enable = false;
+ }
+}
+
+static void sprd_stop_tx(struct uart_port *port)
+{
+ struct sprd_uart_port *sp = container_of(port, struct sprd_uart_port,
+ port);
unsigned int ien, iclr;
+ if (sp->tx_dma.enable) {
+ sprd_stop_tx_dma(port);
+ return;
+ }
+
iclr = serial_in(port, SPRD_ICLR);
ien = serial_in(port, SPRD_IEN);
- ien &= ~(SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT);
- iclr |= SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT;
+ iclr |= SPRD_IEN_TX_EMPTY;
+ ien &= ~SPRD_IEN_TX_EMPTY;
serial_out(port, SPRD_IEN, ien);
serial_out(port, SPRD_ICLR, iclr);
}
+static void sprd_start_tx(struct uart_port *port)
+{
+ struct sprd_uart_port *sp = container_of(port, struct sprd_uart_port,
+ port);
+ unsigned int ien;
+
+ if (sp->tx_dma.enable) {
+ sprd_start_tx_dma(port);
+ return;
+ }
+
+ ien = serial_in(port, SPRD_IEN);
+ if (!(ien & SPRD_IEN_TX_EMPTY)) {
+ ien |= SPRD_IEN_TX_EMPTY;
+ serial_out(port, SPRD_IEN, ien);
+ }
+}
+
/* The Sprd serial does not support this function. */
static void sprd_break_ctl(struct uart_port *port, int break_state)
{
@@ -220,9 +590,16 @@ static int handle_lsr_errors(struct uart_port *port,
static inline void sprd_rx(struct uart_port *port)
{
+ struct sprd_uart_port *sp = container_of(port, struct sprd_uart_port,
+ port);
struct tty_port *tty = &port->state->port;
unsigned int ch, flag, lsr, max_count = SPRD_TIMEOUT;
+ if (sp->rx_dma.enable) {
+ sprd_uart_dma_irq(port);
+ return;
+ }
+
while ((serial_in(port, SPRD_STS1) & SPRD_RX_FIFO_CNT_MASK) &&
max_count--) {
lsr = serial_in(port, SPRD_LSR);
@@ -306,6 +683,25 @@ static irqreturn_t sprd_handle_irq(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static void sprd_uart_dma_startup(struct uart_port *port,
+ struct sprd_uart_port *sp)
+{
+ int ret;
+
+ sprd_request_dma(port);
+ if (!(sp->rx_dma.enable || sp->tx_dma.enable))
+ return;
+
+ ret = sprd_start_dma_rx(port);
+ if (ret) {
+ sp->rx_dma.enable = false;
+ dma_release_channel(sp->rx_dma.chn);
+ dev_warn(port->dev, "fail to start RX dma mode\n");
+ }
+
+ sprd_uart_dma_enable(port, true);
+}
+
static int sprd_startup(struct uart_port *port)
{
int ret = 0;
@@ -334,6 +730,9 @@ static int sprd_startup(struct uart_port *port)
/* allocate irq */
sp = container_of(port, struct sprd_uart_port, port);
snprintf(sp->name, sizeof(sp->name), "sprd_serial%d", port->line);
+
+ sprd_uart_dma_startup(port, sp);
+
ret = devm_request_irq(port->dev, port->irq, sprd_handle_irq,
IRQF_SHARED, sp->name, port);
if (ret) {
@@ -348,7 +747,9 @@ static int sprd_startup(struct uart_port *port)
/* enable interrupt */
spin_lock_irqsave(&port->lock, flags);
ien = serial_in(port, SPRD_IEN);
- ien |= SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT | SPRD_IEN_TIMEOUT;
+ ien |= SPRD_IEN_BREAK_DETECT | SPRD_IEN_TIMEOUT;
+ if (!sp->rx_dma.enable)
+ ien |= SPRD_IEN_RX_FULL;
serial_out(port, SPRD_IEN, ien);
spin_unlock_irqrestore(&port->lock, flags);
@@ -357,6 +758,7 @@ static int sprd_startup(struct uart_port *port)
static void sprd_shutdown(struct uart_port *port)
{
+ sprd_release_dma(port);
serial_out(port, SPRD_IEN, 0);
serial_out(port, SPRD_ICLR, ~0);
devm_free_irq(port->dev, port->irq, port);
@@ -687,6 +1089,8 @@ static int sprd_remove(struct platform_device *dev)
if (!sprd_ports_num)
uart_unregister_driver(&sprd_uart_driver);
+ sprd_rx_free_buf(sup);
+
return 0;
}
@@ -775,6 +1179,14 @@ static int sprd_probe(struct platform_device *pdev)
}
up->irq = irq;
+ /*
+ * Allocate one dma buffer to prepare for receive transfer, in case
+ * memory allocation failure at runtime.
+ */
+ ret = sprd_rx_alloc_buf(sprd_port[index]);
+ if (ret)
+ return ret;
+
if (!sprd_ports_num) {
ret = uart_register_driver(&sprd_uart_driver);
if (ret < 0) {
--
1.7.9.5
^ permalink raw reply related
* [PATCH 4/5] dt-bindings: serial: sprd: Add dma properties to support DMA mode
From: Baolin Wang @ 2019-02-19 7:31 UTC (permalink / raw)
To: gregkh, jslaby, robh+dt, mark.rutland, orsonzhai, zhang.lyra
Cc: baolin.wang, broonie, lanqing.liu, linux-serial, linux-kernel,
devicetree
In-Reply-To: <cover.1550560916.git.baolin.wang@linaro.org>
From: Lanqing Liu <lanqing.liu@unisoc.com>
This patch adds dmas and dma-names properties for the UART DMA mode.
Signed-off-by: Lanqing Liu <lanqing.liu@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
.../devicetree/bindings/serial/sprd-uart.txt | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/serial/sprd-uart.txt b/Documentation/devicetree/bindings/serial/sprd-uart.txt
index 6eb5863..9ac28f6 100644
--- a/Documentation/devicetree/bindings/serial/sprd-uart.txt
+++ b/Documentation/devicetree/bindings/serial/sprd-uart.txt
@@ -15,12 +15,18 @@ Required properties:
UART clock and source clock are optional properties, but enable clock
is required.
+Optional properties:
+- dma-names: Should contain "tx" for transmit and "rx" for receive channels.
+- dmas: A list of dma specifiers, one for each entry in dma-names.
+
Example:
uart0: serial@0 {
compatible = "sprd,sc9860-uart",
"sprd,sc9836-uart";
reg = <0x0 0x100>;
interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
+ dma-names = "rx", "tx";
+ dmas = <&ap_dma 19 19>, <&ap_dma 20 20>;
clock-names = "enable", "uart", "source";
clocks = <&clk_ap_apb_gates 9>, <&clk_uart0>, <&ext_26m>;
};
--
1.7.9.5
^ permalink raw reply related
* [PATCH 3/5] serial: sprd: Add power management for the Spreadtrum serial controller
From: Baolin Wang @ 2019-02-19 7:31 UTC (permalink / raw)
To: gregkh, jslaby, robh+dt, mark.rutland, orsonzhai, zhang.lyra
Cc: baolin.wang, broonie, lanqing.liu, linux-serial, linux-kernel,
devicetree
In-Reply-To: <cover.1550560916.git.baolin.wang@linaro.org>
From: Lanqing Liu <lanqing.liu@unisoc.com>
This patch adds power management for the Spreadtrum serial controller.
Signed-off-by: Lanqing Liu <lanqing.liu@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/tty/serial/sprd_serial.c | 61 +++++++++++++++++++++++++++++++++++---
1 file changed, 57 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
index 1891a45..8f45b66 100644
--- a/drivers/tty/serial/sprd_serial.c
+++ b/drivers/tty/serial/sprd_serial.c
@@ -100,10 +100,12 @@
#define SPRD_IMSR_TX_FIFO_EMPTY BIT(1)
#define SPRD_IMSR_BREAK_DETECT BIT(7)
#define SPRD_IMSR_TIMEOUT BIT(13)
+#define SPRD_DEFAULT_SOURCE_CLK 26000000
struct sprd_uart_port {
struct uart_port port;
char name[16];
+ struct clk *clk;
};
static struct sprd_uart_port *sprd_port[UART_NR_MAX];
@@ -491,6 +493,22 @@ static int sprd_verify_port(struct uart_port *port, struct serial_struct *ser)
return 0;
}
+static void sprd_pm(struct uart_port *port, unsigned int state,
+ unsigned int oldstate)
+{
+ struct sprd_uart_port *sup =
+ container_of(port, struct sprd_uart_port, port);
+
+ switch (state) {
+ case UART_PM_STATE_ON:
+ clk_prepare_enable(sup->clk);
+ break;
+ case UART_PM_STATE_OFF:
+ clk_disable_unprepare(sup->clk);
+ break;
+ }
+}
+
static const struct uart_ops serial_sprd_ops = {
.tx_empty = sprd_tx_empty,
.get_mctrl = sprd_get_mctrl,
@@ -507,6 +525,7 @@ static int sprd_verify_port(struct uart_port *port, struct serial_struct *ser)
.request_port = sprd_request_port,
.config_port = sprd_config_port,
.verify_port = sprd_verify_port,
+ .pm = sprd_pm,
};
#ifdef CONFIG_SERIAL_SPRD_CONSOLE
@@ -671,11 +690,45 @@ static int sprd_remove(struct platform_device *dev)
return 0;
}
+static int sprd_clk_init(struct uart_port *uport)
+{
+ struct clk *clk_uart, *clk_parent;
+ struct sprd_uart_port *u = sprd_port[uport->line];
+
+ clk_uart = devm_clk_get(uport->dev, "uart");
+ if (IS_ERR(clk_uart)) {
+ dev_warn(uport->dev, "uart%d can't get uart clock\n",
+ uport->line);
+ clk_uart = NULL;
+ }
+
+ clk_parent = devm_clk_get(uport->dev, "source");
+ if (IS_ERR(clk_parent)) {
+ dev_warn(uport->dev, "uart%d can't get source clock\n",
+ uport->line);
+ clk_parent = NULL;
+ }
+
+ if (!clk_uart || clk_set_parent(clk_uart, clk_parent))
+ uport->uartclk = SPRD_DEFAULT_SOURCE_CLK;
+ else
+ uport->uartclk = clk_get_rate(clk_uart);
+
+ u->clk = devm_clk_get(uport->dev, "enable");
+ if (IS_ERR(u->clk)) {
+ if (PTR_ERR(u->clk) != -EPROBE_DEFER)
+ dev_err(uport->dev, "uart%d can't get enable clock\n",
+ uport->line);
+ return PTR_ERR(u->clk);
+ }
+
+ return 0;
+}
+
static int sprd_probe(struct platform_device *pdev)
{
struct resource *res;
struct uart_port *up;
- struct clk *clk;
int irq;
int index;
int ret;
@@ -704,9 +757,9 @@ static int sprd_probe(struct platform_device *pdev)
up->ops = &serial_sprd_ops;
up->flags = UPF_BOOT_AUTOCONF;
- clk = devm_clk_get(&pdev->dev, NULL);
- if (!IS_ERR_OR_NULL(clk))
- up->uartclk = clk_get_rate(clk);
+ ret = sprd_clk_init(up);
+ if (ret)
+ return ret;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
up->membase = devm_ioremap_resource(&pdev->dev, res);
--
1.7.9.5
^ permalink raw reply related
* [PATCH 2/5] dt-bindings: serial: sprd: Add clocks and clocks-names properties
From: Baolin Wang @ 2019-02-19 7:31 UTC (permalink / raw)
To: gregkh, jslaby, robh+dt, mark.rutland, orsonzhai, zhang.lyra
Cc: baolin.wang, broonie, lanqing.liu, linux-serial, linux-kernel,
devicetree
In-Reply-To: <cover.1550560916.git.baolin.wang@linaro.org>
From: Lanqing Liu <lanqing.liu@unisoc.com>
This patch adds clocks and clocks-names properties, which are used to do
power management for our UART driver.
Signed-off-by: Lanqing Liu <lanqing.liu@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
.../devicetree/bindings/serial/sprd-uart.txt | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/serial/sprd-uart.txt b/Documentation/devicetree/bindings/serial/sprd-uart.txt
index cab40f0..6eb5863 100644
--- a/Documentation/devicetree/bindings/serial/sprd-uart.txt
+++ b/Documentation/devicetree/bindings/serial/sprd-uart.txt
@@ -7,7 +7,13 @@ Required properties:
- reg: offset and length of the register set for the device
- interrupts: exactly one interrupt specifier
-- clocks: phandles to input clocks.
+- clock-names: Should contain following entries:
+ "enable" for UART module enable clock,
+ "uart" for UART clock,
+ "source" for UART source (parent) clock.
+- clocks: Should contain a clock specifier for each entry in clock-names.
+ UART clock and source clock are optional properties, but enable clock
+ is required.
Example:
uart0: serial@0 {
@@ -15,5 +21,6 @@ Example:
"sprd,sc9836-uart";
reg = <0x0 0x100>;
interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&ext_26m>;
+ clock-names = "enable", "uart", "source";
+ clocks = <&clk_ap_apb_gates 9>, <&clk_uart0>, <&ext_26m>;
};
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/5] serial: sprd: Modify the baud rate calculation formula
From: Baolin Wang @ 2019-02-19 7:31 UTC (permalink / raw)
To: gregkh, jslaby, robh+dt, mark.rutland, orsonzhai, zhang.lyra
Cc: baolin.wang, broonie, lanqing.liu, linux-serial, linux-kernel,
devicetree
In-Reply-To: <cover.1550560916.git.baolin.wang@linaro.org>
From: Lanqing Liu <lanqing.liu@unisoc.com>
When the source clock is not divisible by the expected baud rate and
the remainder is not less than half of the expected baud rate, the old
formular will round up the frequency division coefficient. This will
make the actual baud rate less than the expected value and can not meet
the external transmission requirements.
Thus this patch modifies the baud rate calculation formula to support
the serial controller output the maximum baud rate.
Signed-off-by: Lanqing Liu <lanqing.liu@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/tty/serial/sprd_serial.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
index 4287ca3..1891a45 100644
--- a/drivers/tty/serial/sprd_serial.c
+++ b/drivers/tty/serial/sprd_serial.c
@@ -371,7 +371,7 @@ static void sprd_set_termios(struct uart_port *port,
/* ask the core to calculate the divisor for us */
baud = uart_get_baud_rate(port, termios, old, 0, SPRD_BAUD_IO_LIMIT);
- quot = (unsigned int)((port->uartclk + baud / 2) / baud);
+ quot = port->uartclk / baud;
/* set data length */
switch (termios->c_cflag & CSIZE) {
--
1.7.9.5
^ permalink raw reply related
* [PATCH 0/5] Add new features for the Spreadtrum serial controller
From: Baolin Wang @ 2019-02-19 7:31 UTC (permalink / raw)
To: gregkh, jslaby, robh+dt, mark.rutland, orsonzhai, zhang.lyra
Cc: baolin.wang, broonie, lanqing.liu, linux-serial, linux-kernel,
devicetree
This patch set fixes the baud rate calculation formula issue, as well as
adding power management support and DMA mode support for the Spreadtrum
serial controller.
Lanqing Liu (5):
serial: sprd: Modify the baud rate calculation formula
dt-bindings: serial: sprd: Add clocks and clocks-names properties
serial: sprd: Add power management for the Spreadtrum serial
controller
dt-bindings: serial: sprd: Add dma properties to support DMA mode
serial: sprd: Add DMA mode support
.../devicetree/bindings/serial/sprd-uart.txt | 17 +-
drivers/tty/serial/sprd_serial.c | 503 +++++++++++++++++++-
2 files changed, 499 insertions(+), 21 deletions(-)
--
1.7.9.5
^ permalink raw reply
* Re: [PATCH v7 3/6] dt-bindings: pinctrl: mt8183: add binding document
From: Zhiyong Tao @ 2019-02-19 2:45 UTC (permalink / raw)
To: Rob Herring
Cc: Erin Lo, Linus Walleij, Matthias Brugger, Mark Rutland,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Greg Kroah-Hartman,
Stephen Boyd, devicetree, srv_heupstream, linux-kernel,
linux-serial, linux-mediatek, linux-arm-kernel, yingjoe.chen,
mars.cheng, eddie.huang, linux-clk
In-Reply-To: <20190218163211.GB2714@bogus>
On Mon, 2019-02-18 at 10:32 -0600, Rob Herring wrote:
> On Fri, Feb 15, 2019 at 02:02:35PM +0800, Erin Lo wrote:
> > From: Zhiyong Tao <zhiyong.tao@mediatek.com>
> >
> > The commit adds mt8183 compatible node in binding document.
> >
> > Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com>
> > Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> > ---
> > .../devicetree/bindings/pinctrl/pinctrl-mt8183.txt | 115 +++++++++++++++++++++
> > 1 file changed, 115 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt
> > new file mode 100644
> > index 0000000..364e673
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt
> > @@ -0,0 +1,115 @@
> > +* Mediatek MT8183 Pin Controller
> > +
> > +The Mediatek's Pin controller is used to control SoC pins.
> > +
> > +Required properties:
> > +- compatible: value should be one of the following.
> > + "mediatek,mt8183-pinctrl", compatible with mt8183 pinctrl.
> > +- gpio-controller : Marks the device node as a gpio controller.
> > +- #gpio-cells: number of cells in GPIO specifier. Since the generic GPIO
> > + binding is used, the amount of cells must be specified as 2. See the below
> > + mentioned gpio binding representation for description of particular cells.
> > +- gpio-ranges : gpio valid number range.
> > +- reg: physicall address base for gpio base registers. There are nine
> > + physicall address base in mt8183. They are 0x10005000, 0x11F20000,
> > + 0x11E80000, 0x11E70000, 0x11E90000, 0x11D30000, 0x11D20000, 0x11C50000,
> > + 0x11F30000.
>
> You don't need to list out each address, just what each address is. (Or
> just '9 GPIO base addresses'.)
==>ok, we will change it.
>
> > +
> > + Eg: <&pio 6 0>
>
> How is this an example of reg? Seems something is missing.
>
> > + <[phandle of the gpio controller node]
> > + [line number within the gpio controller]
> > + [flags]>
> > +
> > + Values for gpio specifier:
> > + - Line number: is a value between 0 to 202.
> > + - Flags: bit field of flags, as defined in <dt-bindings/gpio/gpio.h>.
> > + Only the following flags are supported:
> > + 0 - GPIO_ACTIVE_HIGH
> > + 1 - GPIO_ACTIVE_LOW
> > +
> > +Optional properties:
> > +- reg-names: gpio base register names. There are nine gpio base register
> > + names in mt8183. They are "iocfg0", "iocfg1", "iocfg2", "iocfg3", "iocfg4",
> > + "iocfg5", "iocfg6", "iocfg7", "iocfg8".
> > +- interrupt-controller: Marks the device node as an interrupt controller
> > +- #interrupt-cells: Should be two.
> > +- interrupts : The interrupt outputs from the controller.
>
> outputs? More than 1? If so, need to say what they are and the order.
>
==> there is only use one interrupt in mt8183. we will change
"interrupts" to "interrupt" in v8.
> > +
> > +Please refer to pinctrl-bindings.txt in this directory for details of the
> > +common pinctrl bindings used by client devices.
> > +
> > +Subnode format
> > +A pinctrl node should contain at least one subnodes representing the
> > +pinctrl groups available on the machine. Each subnode will list the
> > +pins it needs, and how they should be configured, with regard to muxer
> > +configuration, pullups, drive strength, input enable/disable and input schmitt.
> > +
> > + node {
> > + pinmux = <PIN_NUMBER_PINMUX>;
> > + GENERIC_PINCONFIG;
> > + };
> > +
> > +Required properties:
> > +- pinmux: integer array, represents gpio pin number and mux setting.
> > + Supported pin number and mux varies for different SoCs, and are defined
> > + as macros in boot/dts/<soc>-pinfunc.h directly.
> > +
> > +Optional properties:
> > +- GENERIC_PINCONFIG: is the generic pinconfig options to use, bias-disable,
> > + bias-pull-down, bias-pull-up, input-enable, input-disable, output-low, output-high,
> > + input-schmitt-enable, input-schmitt-disable and drive-strength are valid.
> > +
> > + Some special pins have extra pull up strength, there are R0 and R1 pull-up
> > + resistors available, but for user, it's only need to set R1R0 as 00, 01, 10 or 11.
> > + So when config mediatek,pull-up-adv or mediatek,pull-down-adv,
> > + it support arguments for those special pins.
> > +
> > + When config drive-strength, it can support some arguments, such as
> > + MTK_DRIVE_4mA, MTK_DRIVE_6mA, etc. See dt-bindings/pinctrl/mt65xx.h.
> > +
> > +Examples:
> > +
> > +#include "mt8183-pinfunc.h"
> > +
> > +...
> > +{
> > + pio: pinctrl@10005000 {
> > + compatible = "mediatek,mt8183-pinctrl";
> > + reg = <0 0x10005000 0 0x1000>,
> > + <0 0x11F20000 0 0x1000>,
> > + <0 0x11E80000 0 0x1000>,
> > + <0 0x11E70000 0 0x1000>,
> > + <0 0x11E90000 0 0x1000>,
> > + <0 0x11D30000 0 0x1000>,
> > + <0 0x11D20000 0 0x1000>,
> > + <0 0x11C50000 0 0x1000>,
> > + <0 0x11F30000 0 0x1000>;
> > + reg-names = "iocfg0", "iocfg1", "iocfg2",
> > + "iocfg3", "iocfg4", "iocfg5",
> > + "iocfg6", "iocfg7", "iocfg8";
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + gpio-ranges = <&pio 0 0 192>;
> > + interrupt-controller;
> > + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
> > + interrupt-parent = <&gic>;
> > + #interrupt-cells = <2>;
> > +
> > + i2c0_pins_a: i2c0 {
> > + pins1 {
> > + pinmux = <PINMUX_GPIO48__FUNC_SCL5>,
> > + <PINMUX_GPIO49__FUNC_SDA5>;
> > + mediatek,pull-up-adv = <11>;
> > + };
> > + };
> > +
> > + i2c1_pins_a: i2c1 {
> > + pins {
> > + pinmux = <PINMUX_GPIO50__FUNC_SCL3>,
> > + <PINMUX_GPIO51__FUNC_SDA3>;
> > + mediatek,pull-down-adv = <10>;
> > + };
> > + };
> > + ...
> > + };
> > +};
> > --
> > 1.9.1
> >
^ permalink raw reply
* Re: [PATCH v2 09/15] dt-bindings: serial: Add Milbeaut serial driver description
From: Rob Herring @ 2019-02-18 19:57 UTC (permalink / raw)
To: Sugaya Taichi
Cc: linux-serial, devicetree, linux-arm-kernel, linux-kernel,
Greg Kroah-Hartman, Mark Rutland, Takao Orito, Kazuhiro Kasai,
Shinji Kanematsu, Jassi Brar, Masami Hiramatsu
In-Reply-To: <1549628849-31726-1-git-send-email-sugaya.taichi@socionext.com>
On Fri, Feb 08, 2019 at 09:27:29PM +0900, Sugaya Taichi wrote:
> Add DT bindings document for Milbeaut serial driver.
>
> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> ---
> .../devicetree/bindings/serial/milbeaut-uart.txt | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/serial/milbeaut-uart.txt
>
> diff --git a/Documentation/devicetree/bindings/serial/milbeaut-uart.txt b/Documentation/devicetree/bindings/serial/milbeaut-uart.txt
> new file mode 100644
> index 0000000..8f61c38
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/milbeaut-uart.txt
> @@ -0,0 +1,21 @@
> +Socionext Milbeaut UART controller
> +
> +Required properties:
> +- compatible: should be "socionext,milbeaut-usio-uart".
> +- reg: offset and length of the register set for the device.
> +- interrupts: two interrupts specifier.
> +- clocks: phandle to the input clock.
> +- interrupt-names: should be "rx", "tx".
Put this next to 'interrupts'
> +
> +Optional properties:
> +- uart-flow-enable: flow control enable.
We already have a standard property for this: auto-flow-control
> +
> +Example:
> + usio1: usio_uart@1e700010 {
serial@...
> + compatible = "socionext,milbeaut-usio-uart";
> + reg = <0x1e700010 0x10>;
> + interrupts = <0 141 0x4>, <0 149 0x4>;
> + interrupt-names = "rx", "tx";
> + clocks = <&clk 2>;
> + uart-flow-enable;
> + };
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH v7 5/6] arm64: dts: mt8183: add pintcrl file
From: Rob Herring @ 2019-02-18 16:33 UTC (permalink / raw)
To: Erin Lo
Cc: Linus Walleij, Matthias Brugger, Mark Rutland, Thomas Gleixner,
Jason Cooper, Marc Zyngier, Greg Kroah-Hartman, Stephen Boyd,
devicetree, srv_heupstream, linux-kernel, linux-serial,
linux-mediatek, linux-arm-kernel, yingjoe.chen, mars.cheng,
eddie.huang, linux-clk, Zhiyong Tao
In-Reply-To: <1550210558-30516-6-git-send-email-erin.lo@mediatek.com>
On Fri, Feb 15, 2019 at 02:02:37PM +0800, Erin Lo wrote:
> This patch adds pinctrl file for mt8183.
>
> Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com>
> Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> ---
> arch/arm64/boot/dts/mediatek/mt8183-pinfunc.h | 1120 +++++++++++++++++++++++++
> 1 file changed, 1120 insertions(+)
> create mode 100644 arch/arm64/boot/dts/mediatek/mt8183-pinfunc.h
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-pinfunc.h b/arch/arm64/boot/dts/mediatek/mt8183-pinfunc.h
> new file mode 100644
> index 0000000..768e41e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-pinfunc.h
> @@ -0,0 +1,1120 @@
> +// SPDX-License-Identifier: GPL-2.0
Run checkpatch. It will tell you that this should be /* */ comments.
Otherwise,
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v7 3/6] dt-bindings: pinctrl: mt8183: add binding document
From: Rob Herring @ 2019-02-18 16:32 UTC (permalink / raw)
To: Erin Lo
Cc: Linus Walleij, Matthias Brugger, Mark Rutland, Thomas Gleixner,
Jason Cooper, Marc Zyngier, Greg Kroah-Hartman, Stephen Boyd,
devicetree, srv_heupstream, linux-kernel, linux-serial,
linux-mediatek, linux-arm-kernel, yingjoe.chen, mars.cheng,
eddie.huang, linux-clk, Zhiyong Tao
In-Reply-To: <1550210558-30516-4-git-send-email-erin.lo@mediatek.com>
On Fri, Feb 15, 2019 at 02:02:35PM +0800, Erin Lo wrote:
> From: Zhiyong Tao <zhiyong.tao@mediatek.com>
>
> The commit adds mt8183 compatible node in binding document.
>
> Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com>
> Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> ---
> .../devicetree/bindings/pinctrl/pinctrl-mt8183.txt | 115 +++++++++++++++++++++
> 1 file changed, 115 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt
> new file mode 100644
> index 0000000..364e673
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt
> @@ -0,0 +1,115 @@
> +* Mediatek MT8183 Pin Controller
> +
> +The Mediatek's Pin controller is used to control SoC pins.
> +
> +Required properties:
> +- compatible: value should be one of the following.
> + "mediatek,mt8183-pinctrl", compatible with mt8183 pinctrl.
> +- gpio-controller : Marks the device node as a gpio controller.
> +- #gpio-cells: number of cells in GPIO specifier. Since the generic GPIO
> + binding is used, the amount of cells must be specified as 2. See the below
> + mentioned gpio binding representation for description of particular cells.
> +- gpio-ranges : gpio valid number range.
> +- reg: physicall address base for gpio base registers. There are nine
> + physicall address base in mt8183. They are 0x10005000, 0x11F20000,
> + 0x11E80000, 0x11E70000, 0x11E90000, 0x11D30000, 0x11D20000, 0x11C50000,
> + 0x11F30000.
You don't need to list out each address, just what each address is. (Or
just '9 GPIO base addresses'.)
> +
> + Eg: <&pio 6 0>
How is this an example of reg? Seems something is missing.
> + <[phandle of the gpio controller node]
> + [line number within the gpio controller]
> + [flags]>
> +
> + Values for gpio specifier:
> + - Line number: is a value between 0 to 202.
> + - Flags: bit field of flags, as defined in <dt-bindings/gpio/gpio.h>.
> + Only the following flags are supported:
> + 0 - GPIO_ACTIVE_HIGH
> + 1 - GPIO_ACTIVE_LOW
> +
> +Optional properties:
> +- reg-names: gpio base register names. There are nine gpio base register
> + names in mt8183. They are "iocfg0", "iocfg1", "iocfg2", "iocfg3", "iocfg4",
> + "iocfg5", "iocfg6", "iocfg7", "iocfg8".
> +- interrupt-controller: Marks the device node as an interrupt controller
> +- #interrupt-cells: Should be two.
> +- interrupts : The interrupt outputs from the controller.
outputs? More than 1? If so, need to say what they are and the order.
> +
> +Please refer to pinctrl-bindings.txt in this directory for details of the
> +common pinctrl bindings used by client devices.
> +
> +Subnode format
> +A pinctrl node should contain at least one subnodes representing the
> +pinctrl groups available on the machine. Each subnode will list the
> +pins it needs, and how they should be configured, with regard to muxer
> +configuration, pullups, drive strength, input enable/disable and input schmitt.
> +
> + node {
> + pinmux = <PIN_NUMBER_PINMUX>;
> + GENERIC_PINCONFIG;
> + };
> +
> +Required properties:
> +- pinmux: integer array, represents gpio pin number and mux setting.
> + Supported pin number and mux varies for different SoCs, and are defined
> + as macros in boot/dts/<soc>-pinfunc.h directly.
> +
> +Optional properties:
> +- GENERIC_PINCONFIG: is the generic pinconfig options to use, bias-disable,
> + bias-pull-down, bias-pull-up, input-enable, input-disable, output-low, output-high,
> + input-schmitt-enable, input-schmitt-disable and drive-strength are valid.
> +
> + Some special pins have extra pull up strength, there are R0 and R1 pull-up
> + resistors available, but for user, it's only need to set R1R0 as 00, 01, 10 or 11.
> + So when config mediatek,pull-up-adv or mediatek,pull-down-adv,
> + it support arguments for those special pins.
> +
> + When config drive-strength, it can support some arguments, such as
> + MTK_DRIVE_4mA, MTK_DRIVE_6mA, etc. See dt-bindings/pinctrl/mt65xx.h.
> +
> +Examples:
> +
> +#include "mt8183-pinfunc.h"
> +
> +...
> +{
> + pio: pinctrl@10005000 {
> + compatible = "mediatek,mt8183-pinctrl";
> + reg = <0 0x10005000 0 0x1000>,
> + <0 0x11F20000 0 0x1000>,
> + <0 0x11E80000 0 0x1000>,
> + <0 0x11E70000 0 0x1000>,
> + <0 0x11E90000 0 0x1000>,
> + <0 0x11D30000 0 0x1000>,
> + <0 0x11D20000 0 0x1000>,
> + <0 0x11C50000 0 0x1000>,
> + <0 0x11F30000 0 0x1000>;
> + reg-names = "iocfg0", "iocfg1", "iocfg2",
> + "iocfg3", "iocfg4", "iocfg5",
> + "iocfg6", "iocfg7", "iocfg8";
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&pio 0 0 192>;
> + interrupt-controller;
> + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-parent = <&gic>;
> + #interrupt-cells = <2>;
> +
> + i2c0_pins_a: i2c0 {
> + pins1 {
> + pinmux = <PINMUX_GPIO48__FUNC_SCL5>,
> + <PINMUX_GPIO49__FUNC_SDA5>;
> + mediatek,pull-up-adv = <11>;
> + };
> + };
> +
> + i2c1_pins_a: i2c1 {
> + pins {
> + pinmux = <PINMUX_GPIO50__FUNC_SCL3>,
> + <PINMUX_GPIO51__FUNC_SDA3>;
> + mediatek,pull-down-adv = <10>;
> + };
> + };
> + ...
> + };
> +};
> --
> 1.9.1
>
^ permalink raw reply
* Re: [RFC PATCH v1 07/25] printk-rb: add functionality required by printk
From: Petr Mladek @ 2019-02-18 15:59 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20190212143003.48446-8-john.ogness@linutronix.de>
On Tue 2019-02-12 15:29:45, John Ogness wrote:
> The printk subsystem needs to be able to query the size of the ring
> buffer, seek to specific entries within the ring buffer, and track
> if records could not be stored in the ring buffer.
>
> diff --git a/lib/printk_ringbuffer.c b/lib/printk_ringbuffer.c
> index c2ddf4cb9f92..ce33b5add5a1 100644
> --- a/lib/printk_ringbuffer.c
> +++ b/lib/printk_ringbuffer.c
> @@ -175,11 +175,16 @@ void prb_commit(struct prb_handle *h)
> head = PRB_WRAP_LPOS(rb, head, 1);
> continue;
> }
> + while (atomic_long_read(&rb->lost)) {
> + atomic_long_dec(&rb->lost);
> + rb->seq++;
The lost-messages handling should be in a separate patch.
At least I do not see any close relation with prb_iter_seek().
I would personally move prb_iter_seek() to the 5th patch that
implements the other get/iterator functions.
On the contrary, the patch adding support for lost messages
should implement a way how to inform the user about lost messages.
E.g. to add a warning when some space becomes available again.
> + }
> e->seq = ++rb->seq;
> head += e->size;
> changed = true;
> }
> atomic_long_set_release(&rb->head, res);
> +
> atomic_dec(&rb->ctx);
>
> if (atomic_long_read(&rb->reserve) == res)
> @@ -486,3 +491,93 @@ int prb_iter_wait_next(struct prb_iterator *iter, char *buf, int size, u64 *seq)
>
> return ret;
> }
> +
> +/*
> + * prb_iter_seek: Seek forward to a specific record.
> + * @iter: Iterator to advance.
> + * @seq: Record number to advance to.
> + *
> + * Advance @iter such that a following call to prb_iter_data() will provide
> + * the contents of the specified record. If a record is specified that does
> + * not yet exist, advance @iter to the end of the record list.
> + *
> + * Note that iterators cannot be rewound. So if a record is requested that
> + * exists but is previous to @iter in position, @iter is considered invalid.
> + *
> + * It is safe to call this function from any context and state.
> + *
> + * Returns 1 on succces, 0 if specified record does not yet exist (@iter is
> + * now at the end of the list), or -EINVAL if @iter is now invalid.
> + */
Do we really need to distinguish when the iterator is invalid and when
we are at the end of the buffer?
It seems that the reaction in both situation always is to call
prb_iter_init(&iter, &printk_rb, &some_seq). I am still a bit
confused what your prb_iter_init() does. Therefore I am not
sure what it is supposed to do.
Anyway, it seems to be typically used when you need to start
from tail. I would personally do something like (based on my code
in reply to 5th patch:
int prb_iter_seek_to_seq(struct prb_iterator *iter, u64 seq)
{
int ret;
ret = prb_iter_tail_entry(iter);
while (!ret && iter->entry->seq != seq)
ret = prb_iter_next_valid_entry(iter);
return ret;
}
When I see it, I would make the functionality more obvious
by renaming:
prb_iter_tail_entry() -> prb_iter_set_tail_entry()
> +int prb_iter_seek(struct prb_iterator *iter, u64 seq)
> +{
> + u64 cur_seq;
> + int ret;
> +
> + /* first check if the iterator is already at the wanted seq */
> + if (seq == 0) {
> + if (iter->lpos == PRB_INIT)
> + return 1;
> + else
> + return -EINVAL;
> + }
> + if (iter->lpos != PRB_INIT) {
> + if (prb_iter_data(iter, NULL, 0, &cur_seq) >= 0) {
> + if (cur_seq == seq)
> + return 1;
> + if (cur_seq > seq)
> + return -EINVAL;
> + }
> + }
> +
> + /* iterate to find the wanted seq */
> + for (;;) {
> + ret = prb_iter_next(iter, NULL, 0, &cur_seq);
> + if (ret <= 0)
> + break;
> +
> + if (cur_seq == seq)
> + break;
> +
> + if (cur_seq > seq) {
> + ret = -EINVAL;
> + break;
> + }
This is a good example why prb_iter_data() and prb_iter_next() is
a weird interface. You need to read the documentation very carefully
to understand the difference (functionality, error codes). At least
for me, it is far from obvious why they are used this way and
if it is correct.
Best Regards,
Petr
> + }
> +
> + return ret;
> +}
^ permalink raw reply
* Re: [PATCH v7 3/6] dt-bindings: pinctrl: mt8183: add binding document
From: Rob Herring @ 2019-02-18 15:48 UTC (permalink / raw)
To: Matthias Brugger
Cc: Erin Lo, Linus Walleij, Mark Rutland, Thomas Gleixner,
Jason Cooper, Marc Zyngier, Greg Kroah-Hartman, Stephen Boyd,
devicetree, srv_heupstream, linux-kernel, linux-serial,
linux-mediatek, linux-arm-kernel, yingjoe.chen, mars.cheng,
eddie.huang, linux-clk, Zhiyong Tao
In-Reply-To: <2b221339-3933-2a25-ab60-d4d772d2873a@gmail.com>
On Fri, Feb 15, 2019 at 10:35:39AM +0100, Matthias Brugger wrote:
>
>
> On 15/02/2019 07:02, Erin Lo wrote:
> > From: Zhiyong Tao <zhiyong.tao@mediatek.com>
> >
> > The commit adds mt8183 compatible node in binding document.
> >
> > Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com>
> > Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> > ---
> > .../devicetree/bindings/pinctrl/pinctrl-mt8183.txt | 115 +++++++++++++++++++++
> > 1 file changed, 115 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt
> > new file mode 100644
> > index 0000000..364e673
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt
> > @@ -0,0 +1,115 @@
> > +* Mediatek MT8183 Pin Controller
> > +
> > +The Mediatek's Pin controller is used to control SoC pins.
> > +
> > +Required properties:
> > +- compatible: value should be one of the following.
> > + "mediatek,mt8183-pinctrl", compatible with mt8183 pinctrl.
> > +- gpio-controller : Marks the device node as a gpio controller.
> > +- #gpio-cells: number of cells in GPIO specifier. Since the generic GPIO
> > + binding is used, the amount of cells must be specified as 2. See the below
> > + mentioned gpio binding representation for description of particular cells.
> > +- gpio-ranges : gpio valid number range.
> > +- reg: physicall address base for gpio base registers. There are nine
>
> s/physicall/physical
>
> > + physicall address base in mt8183. They are 0x10005000, 0x11F20000,
> > + 0x11E80000, 0x11E70000, 0x11E90000, 0x11D30000, 0x11D20000, 0x11C50000,
> > + 0x11F30000.
> > +
> > + Eg: <&pio 6 0>
> > + <[phandle of the gpio controller node]
> > + [line number within the gpio controller]
> > + [flags]>
> > +
> > + Values for gpio specifier:
> > + - Line number: is a value between 0 to 202.
> > + - Flags: bit field of flags, as defined in <dt-bindings/gpio/gpio.h>.
> > + Only the following flags are supported:
> > + 0 - GPIO_ACTIVE_HIGH
> > + 1 - GPIO_ACTIVE_LOW
> > +
> > +Optional properties:
> > +- reg-names: gpio base register names. There are nine gpio base register
> > + names in mt8183. They are "iocfg0", "iocfg1", "iocfg2", "iocfg3", "iocfg4",
> > + "iocfg5", "iocfg6", "iocfg7", "iocfg8".
> > +- interrupt-controller: Marks the device node as an interrupt controller
> > +- #interrupt-cells: Should be two.
> > +- interrupts : The interrupt outputs from the controller.
>
> we are missing interrupt-parent here.
No, because interrupt-parent is implied and may be in a parent node.
What's missing is how many interrupts and what are they?
Rob
^ permalink raw reply
* Re: [RFC PATCH v1 06/25] printk-rb: add blocking reader support
From: Petr Mladek @ 2019-02-18 14:05 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20190212143003.48446-7-john.ogness@linutronix.de>
On Tue 2019-02-12 15:29:44, John Ogness wrote:
> Add a blocking read function for readers. An irq_work function is
> used to signal the wait queue so that write notification can
> be triggered from any context.
I would be more precise what exacly is problematic in which context.
Something like:
An irq_work function is used because wake_up() cannot be called safely
from NMI and scheduler context.
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
> ---
> include/linux/printk_ringbuffer.h | 20 ++++++++++++++++
> lib/printk_ringbuffer.c | 49 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 69 insertions(+)
>
> diff --git a/include/linux/printk_ringbuffer.h b/include/linux/printk_ringbuffer.h
> index 5fdaf632c111..106f20ef8b4d 100644
> --- a/include/linux/printk_ringbuffer.h
> +++ b/include/linux/printk_ringbuffer.h
> @@ -2,8 +2,10 @@
> #ifndef _LINUX_PRINTK_RINGBUFFER_H
> #define _LINUX_PRINTK_RINGBUFFER_H
>
> +#include <linux/irq_work.h>
> #include <linux/atomic.h>
> #include <linux/percpu.h>
> +#include <linux/wait.h>
>
> struct prb_cpulock {
> atomic_t owner;
> @@ -22,6 +24,10 @@ struct printk_ringbuffer {
>
> struct prb_cpulock *cpulock;
> atomic_t ctx;
> +
> + struct wait_queue_head *wq;
> + atomic_long_t wq_counter;
> + struct irq_work *wq_work;
> };
>
> struct prb_entry {
> @@ -59,6 +65,15 @@ struct prb_iterator {
> #define DECLARE_STATIC_PRINTKRB(name, szbits, cpulockptr) \
> static char _##name##_buffer[1 << (szbits)] \
> __aligned(__alignof__(long)); \
> +static DECLARE_WAIT_QUEUE_HEAD(_##name##_wait); \
> +static void _##name##_wake_work_func(struct irq_work *irq_work) \
> +{ \
> + wake_up_interruptible_all(&_##name##_wait); \
> +} \
All ring buffers might share the same generic function, something like:
void prb_wake_readers_work_func(struct irq_work *irq_work)
{
struct printk_ringbuffer *rb;
rb = container_of(irq_work, struct printk_ring_buffer, wq_work);
wake_up_interruptible_all(rb->wq); \
}
> +static struct irq_work _##name##_wake_work = { \
> + .func = _##name##_wake_work_func, \
> + .flags = IRQ_WORK_LAZY, \
> +}; \
> static struct printk_ringbuffer name = { \
> .buffer = &_##name##_buffer[0], \
> .size_bits = szbits, \
> diff --git a/lib/printk_ringbuffer.c b/lib/printk_ringbuffer.c
> index 1d1e886a0966..c2ddf4cb9f92 100644
> --- a/lib/printk_ringbuffer.c
> +++ b/lib/printk_ringbuffer.c
> @@ -185,6 +188,12 @@ void prb_commit(struct prb_handle *h)
> }
>
> prb_unlock(rb->cpulock, h->cpu);
> +
> + if (changed) {
> + atomic_long_inc(&rb->wq_counter);
> + if (wq_has_sleeper(rb->wq))
> + irq_work_queue(rb->wq_work);
> + }
> }
>
> /*
> @@ -437,3 +446,43 @@ int prb_iter_next(struct prb_iterator *iter, char *buf, int size, u64 *seq)
>
> return 1;
> }
> +
> +/*
> + * prb_iter_wait_next: Advance to the next record, blocking if none available.
> + * @iter: Iterator tracking the current position.
> + * @buf: A buffer to store the data of the next record. May be NULL.
> + * @size: The size of @buf. (Ignored if @buf is NULL.)
> + * @seq: The sequence number of the next record. May be NULL.
> + *
> + * If a next record is already available, this function works like
> + * prb_iter_next(). Otherwise block interruptible until a next record is
> + * available.
> + *
> + * When a next record is available, @iter is advanced and (if specified)
> + * the data and/or sequence number of that record are provided.
> + *
> + * This function might sleep.
> + *
> + * Returns 1 if @iter was advanced, -EINVAL if @iter is now invalid, or
> + * -ERESTARTSYS if interrupted by a signal.
> + */
> +int prb_iter_wait_next(struct prb_iterator *iter, char *buf, int size, u64 *seq)
> +{
> + unsigned long last_seen;
> + int ret;
> +
> + for (;;) {
> + last_seen = atomic_long_read(&iter->rb->wq_counter);
> +
> + ret = prb_iter_next(iter, buf, size, seq);
> + if (ret != 0)
> + break;
> +
> + ret = wait_event_interruptible(*iter->rb->wq,
> + last_seen != atomic_long_read(&iter->rb->wq_counter));
Do we really need yet another counter here?
I think that rb->seq might do the same job. Or if there is problem
with atomicity then rb->head might work as well. Or do I miss
anything?
Best Regards,
Petr
> + if (ret < 0)
> + break;
> + }
> +
> + return ret;
> +}
> --
> 2.11.0
>
^ permalink raw reply
* Re: [RFC PATCH v1 05/25] printk-rb: add basic non-blocking reading interface
From: Petr Mladek @ 2019-02-18 12:54 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20190212143003.48446-6-john.ogness@linutronix.de>
On Tue 2019-02-12 15:29:43, John Ogness wrote:
> Add reader iterator static declaration/initializer, dynamic
> initializer, and functions to iterate and retrieve ring buffer data.
>
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
> ---
> include/linux/printk_ringbuffer.h | 20 ++++
> lib/printk_ringbuffer.c | 190 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 210 insertions(+)
>
> diff --git a/include/linux/printk_ringbuffer.h b/include/linux/printk_ringbuffer.h
> index 1aec9d5666b1..5fdaf632c111 100644
> --- a/include/linux/printk_ringbuffer.h
> +++ b/include/linux/printk_ringbuffer.h
> @@ -43,6 +43,19 @@ static struct prb_cpulock name = { \
> .irqflags = &_##name##_percpu_irqflags, \
> }
>
> +#define PRB_INIT ((unsigned long)-1)
> +
> +#define DECLARE_STATIC_PRINTKRB_ITER(name, rbaddr) \
> +static struct prb_iterator name = { \
Please, define the macro without static. The static variable
should get declared as:
static DECLARE_PRINTKRB_ITER();
> + .rb = rbaddr, \
> + .lpos = PRB_INIT, \
> +}
> +
> +struct prb_iterator {
> + struct printk_ringbuffer *rb;
> + unsigned long lpos;
> +};
Please, define the structure before it is used (even in macros).
It is strange to initialize something that is not yet defined.
> #define DECLARE_STATIC_PRINTKRB(name, szbits, cpulockptr) \
> static char _##name##_buffer[1 << (szbits)] \
> __aligned(__alignof__(long)); \
> @@ -62,6 +75,13 @@ char *prb_reserve(struct prb_handle *h, struct printk_ringbuffer *rb,
> unsigned int size);
> void prb_commit(struct prb_handle *h);
>
> +/* reader interface */
> +void prb_iter_init(struct prb_iterator *iter, struct printk_ringbuffer *rb,
> + u64 *seq);
> +void prb_iter_copy(struct prb_iterator *dest, struct prb_iterator *src);
> +int prb_iter_next(struct prb_iterator *iter, char *buf, int size, u64 *seq);
> +int prb_iter_data(struct prb_iterator *iter, char *buf, int size, u64 *seq);
> +
> /* utility functions */
> void prb_lock(struct prb_cpulock *cpu_lock, unsigned int *cpu_store);
> void prb_unlock(struct prb_cpulock *cpu_lock, unsigned int cpu_store);
> diff --git a/lib/printk_ringbuffer.c b/lib/printk_ringbuffer.c
> index 90c7f9a9f861..1d1e886a0966 100644
> --- a/lib/printk_ringbuffer.c
> +++ b/lib/printk_ringbuffer.c
> @@ -1,5 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
> #include <linux/smp.h>
> +#include <linux/string.h>
> +#include <linux/errno.h>
> #include <linux/printk_ringbuffer.h>
>
> #define PRB_SIZE(rb) (1 << rb->size_bits)
> @@ -8,6 +10,7 @@
> #define PRB_WRAPS(rb, lpos) (lpos >> rb->size_bits)
> #define PRB_WRAP_LPOS(rb, lpos, xtra) \
> ((PRB_WRAPS(rb, lpos) + xtra) << rb->size_bits)
> +#define PRB_DATA_SIZE(e) (e->size - sizeof(struct prb_entry))
> #define PRB_DATA_ALIGN sizeof(long)
>
> static bool __prb_trylock(struct prb_cpulock *cpu_lock,
> @@ -247,3 +250,190 @@ char *prb_reserve(struct prb_handle *h, struct printk_ringbuffer *rb,
>
> return &h->entry->data[0];
> }
> +
> +/*
> + * prb_iter_copy: Copy an iterator.
> + * @dest: The iterator to copy to.
> + * @src: The iterator to copy from.
> + *
> + * Make a deep copy of an iterator. This is particularly useful for making
> + * backup copies of an iterator in case a form of rewinding it needed.
> + *
> + * It is safe to call this function from any context and state. But
> + * note that this function is not atomic. Callers should not make copies
> + * to/from iterators that can be accessed by other tasks/contexts.
> + */
> +void prb_iter_copy(struct prb_iterator *dest, struct prb_iterator *src)
> +{
> + memcpy(dest, src, sizeof(*dest));
> +}
> +
> +/*
> + * prb_iter_init: Initialize an iterator for a ring buffer.
> + * @iter: The iterator to initialize.
> + * @rb: A ring buffer to that @iter should iterate.
> + * @seq: The sequence number of the position preceding the first record.
> + * May be NULL.
> + *
> + * Initialize an iterator to be used with a specified ring buffer. If @seq
> + * is non-NULL, it will be set such that prb_iter_next() will provide a
> + * sequence value of "@seq + 1" if no records were missed.
> + *
> + * It is safe to call this function from any context and state.
> + */
> +void prb_iter_init(struct prb_iterator *iter, struct printk_ringbuffer *rb,
> + u64 *seq)
> +{
> + memset(iter, 0, sizeof(*iter));
> + iter->rb = rb;
> + iter->lpos = PRB_INIT;
> +
> + if (!seq)
> + return;
> +
> + for (;;) {
> + struct prb_iterator tmp_iter;
> + int ret;
> +
> + prb_iter_copy(&tmp_iter, iter);
It looks strange to copy something that has been hardly initialized.
I hope that we could do this without a copy, see below.
> +
> + ret = prb_iter_next(&tmp_iter, NULL, 0, seq);
prb_iter_next() and prb_iter_data() are too complex spaghetti
code. They do basically the same but they do not share
any helper function. The error handling is different
which is really confusing. See below.
> + if (ret < 0)
> + continue;
> +
> + if (ret == 0)
> + *seq = 0;
> + else
> + (*seq)--;
The decrement is another suspicious things here.
> + break;
Finally, I am confused by the semantic of this function.
What is supposed to be the initialized lpos, seq number,
please?
I would expect a function that initializes the iterator
either to the first valid entry (tail-one) or
to the one defined by the given seq number.
Well, I think that we need to start with a more low-level functions.
IMHO. we need something to read one entry a safe way. Then it will
be much easier to live with races in the rest of the code:
/*
* Return valid entry on the given lpos. Data are read
* only when the buffer is is not zero.
*/
int prb_get_entry(struct struct printk_ringbuffer *rb,
unsigned long lpos,
struct prb_entry *entry,
unsigned int data_buf_size)
{
/*
* Pointer to the ring buffer. The data might get lost
* at any time.
*/
struct prb_entry *weak_entry;
if (!is_valid(lpos))
return -EINVAL;
/* Make sure that data are valid for the given valid lpos. */
smp_rmb();
weak_entry = to_entry(lpos);
entry->seq = weak_entry->seq;
if (data_buf_size) {
unsigned int size;
size = min(data_buf_size, weak_entry->size);
memcpy(entry->data, weak_entry->data, size);
entry->size = size;
} else {
entry->size = weak_data->size;
}
/* Make sure that the copy is done before we check validity. */
smp_mb();
return is_valid(lpos);
}
Then I would do the support for iterating the following way.
First, I would extend the structure:
struct prb_iterator {
struct printk_ringbuffer *rb;
struct prb_entry *entry;
unsigned int data_buffer_size;
unsigned long lpos;
};
And do something like:
void prg_iter_init(struct struct printk_ringbuffer *rb,
struct prb_entry *entry,
unsigned int data_buffer_size,
struct prb_iterator *iter)
{
iter->rb = rb;
iter->entry = entry;
iter->data_buffer_size = data_buffer_size;
lpos = 0UL;
}
Then we could do iterator support the following way:
/* Start iteration with reading the tail entry. */
int prb_iter_tail_entry(struct prb_iterator *iter);
{
unsigned long tail;
int ret;
for (;;) {
tail = atomic_long_read(&rb->tail);
/* Ring buffer is empty? */
if (unlikely(!is_valid(tail)))
return -EINVAL;
ret = prb_get_entry(iter->rb, tail,
iter->entry, iter->data_buf_size);
if (!ret) {
iter->lpos = tail;
break;
}
}
return 0;
}
unsigned long next_lpos(unsineg long lpos, struct prb_entry *entry)
{
return lpos + sizeof(struct entry) + entry->size;
}
/* Try to get next entry using a valid iterator */
int prb_iter_next_entry(struct prb_iterator *iter)
{
iter->lpos = next_lpos(iter->lpos, iter->etnry);
return prb_get_entry(rb, lpos, entry, data_buf_size;
}
/* Try to get the next entry. Allow to skip lost messages. */
int prb_iter_next_valid_entry(struct prb_iterator *iter)
{
int ret;
ret = prb_iter_next_entry(iter);
if (!ret)
return 0;
/* Next entry has been lost. Skip to the current tail. */
return prb_iter_tail_entry(rb, *lpos, entry, data_buf_size);
}
> +static bool is_valid(struct printk_ringbuffer *rb, unsigned long lpos)
> +{
> + unsigned long head, tail;
> +
> + tail = atomic_long_read(&rb->tail);
> + head = atomic_long_read(&rb->head);
> + head -= tail;
> + lpos -= tail;
> +
> + if (lpos >= head)
> + return false;
> + return true;
> +}
> +
IMPORTANT:
Please, do not start rewriting the entire patchset after reading this
mail. I suggest to take a rest from coding. Just read the feadback,
ask if anything is unclear, and let your brain processing it
on background.
The motivation:
1. This is a really complex patchset and it will be a long run. For
example, I worked on atomic livepatch replace patchset more
than 1 year. There were 15 iterations, see
https://lkml.kernel.org/r/20190109123739.21841-1-pmladek@suse.com
And I was really familiar with this subsystem, was reviewer from
the beginning.
Another example is kthread worker API. It took more than
1 year from RFC until v10 was accepted, see
1470754545-17632-1-git-send-email-pmladek@suse.com
In both cases, the final revision looked completely
different from the initial RFC.
Printk word is even more complicated. I worked several months
on race-free NMI buffer (it was my first big kernel project).
It was put into a garbage dump within 1 day, see
https://lkml.kernel.org/r/CA+55aFzseOqF-EpKMvwKpfhBJZQSLqKpJ3shzVee9s0+mvyCuA@mail.gmail.com
The patches offloading printk console to a kthread was
floating around few years and was never accepted.
That said, I do not want to discourage you. Your patchset has
interesting ideas that are worth finishing. I just think
that it is better to take a rest when you need to wait
for feedback. It will help you to see it from another
perspective and start working on v2 with a fresh mind.
2. There are 25 patches already. It might be hard to follow
the discussion. And it will be even harder if there
are more variants of the patches discussed in the same
thread.
I suggest to just proceed the feedback. Ask if anything
is unclear. Eventually discuss code alternatives but
do not send entire patches. Send full v2. Then we could
start with a clean table again.
3. I have seen only 5 patches out of 25 so far. My feedback
is based past experience and common sense. I might
view some things differently once I get to the other
patches.
You might feel frustrated when you rework something
based on my feedback and I later change my mind
and suggest something different.
I do not want to make you frustrated. Therefore I feel
stressed and afraid to send more feedback before I get
the full picture. But then it might take weeks until
I send something. Many ideas might be lost in the meantime.
The result might be hard to understand because I would
describe some final statements without explaining
all the reasons.
4. There might be feedback from other people and they
might have another opinion.
Thanks for working on it.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH v2 00/15] Add basic support for Socionext Milbeaut M10V SoC
From: Daniel Lezcano @ 2019-02-18 12:29 UTC (permalink / raw)
To: Arnd Bergmann, Sugaya Taichi
Cc: DTML, Linux Kernel Mailing List, Linux ARM, linux-clk,
open list:GPIO SUBSYSTEM, linux-serial, Rob Herring, Mark Rutland,
Michael Turquette, Stephen Boyd, Linus Walleij,
Greg Kroah-Hartman, Thomas Gleixner, Russell King, Jiri Slaby,
Takao Orito, Kazuhiro Kasai, Shinji Kanematsu, Jassi
In-Reply-To: <CAK8P3a3_j2W+KXSynW_SkVZk_+PJ-DqyxjLkgJL-OichS0fwWw@mail.gmail.com>
On 18/02/2019 13:20, Arnd Bergmann wrote:
> On Fri, Feb 8, 2019 at 1:24 PM Sugaya Taichi
> <sugaya.taichi@socionext.com> wrote:
>>
>> Hi,
>>
>> Here is the series of patches the initial support for SC2000(M10V) of
>> Milbeaut SoCs. "M10V" is the internal name of SC2000, so commonly used in
>> source code.
>>
>> SC2000 is a SoC of the Milbeaut series. equipped with a DSP optimized for
>> computer vision. It also features advanced functionalities such as 360-degree,
>> real-time spherical stitching with multi cameras, image stabilization for
>> without mechanical gimbals, and rolling shutter correction. More detail is
>> below:
>> https://www.socionext.com/en/products/assp/milbeaut/SC2000.html
>>
>> Specifications for developers are below:
>> - Quad-core 32bit Cortex-A7 on ARMv7-A architecture
>> - NEON support
>> - DSP
>> - GPU
>> - MAX 3GB DDR3
>> - Cortex-M0 for power control
>> - NAND Flash Interface
>> - SD UHS-I
>> - SD UHS-II
>> - SDIO
>> - USB2.0 HOST / Device
>> - USB3.0 HOST / Device
>> - PCI express Gen2
>> - Ethernet Engine
>> - I2C
>> - UART
>> - SPI
>> - PWM
>>
>> Support is quite minimal for now, since it only includes timer, clock,
>> pictrl and serial controller drivers, so we can only boot to userspace
>> through initramfs. Support for the other peripherals will come eventually.
>
> I've looked over the platform once more. Overall, it looks very good, and
> I'd still like to merge this for linux-5.1, but we are running out of time
> there. If you send the patches to soc@kernel.org quickly, we can try to
> still merge them in, but if anything goes wrong, it will have to wait until
> we start merging patches for 5.2, directly after 5.1 is out.
>
> I did not look at the device driver patches (clk, clocksource, pinctrl, serial)
> in much detail. If you have an Ack from the maintainers, feel free to
> include them in the series, otherwise let's merge the rest now and then
> you can send the updated patches for inclusion through the subsystem
> trees in 5.2.
I'm not in copy of the clocksource patch but I acked it for the initial
post.
> I have sent a few comments. The only one that is really important
> here is the missing platform check in the suspend options. Please
> try to address most of the other commentsm, either by changing the
> code, or by explaining why your version is correct.
>
>
>
> Arnd
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH v2 00/15] Add basic support for Socionext Milbeaut M10V SoC
From: Arnd Bergmann @ 2019-02-18 12:20 UTC (permalink / raw)
To: Sugaya Taichi
Cc: DTML, Linux Kernel Mailing List, Linux ARM, linux-clk,
open list:GPIO SUBSYSTEM, linux-serial, Rob Herring, Mark Rutland,
Michael Turquette, Stephen Boyd, Linus Walleij,
Greg Kroah-Hartman, Daniel Lezcano, Thomas Gleixner, Russell King,
Jiri Slaby, Takao Orito, Kazuhiro Kasai, Shinji
In-Reply-To: <1549628687-29628-1-git-send-email-sugaya.taichi@socionext.com>
On Fri, Feb 8, 2019 at 1:24 PM Sugaya Taichi
<sugaya.taichi@socionext.com> wrote:
>
> Hi,
>
> Here is the series of patches the initial support for SC2000(M10V) of
> Milbeaut SoCs. "M10V" is the internal name of SC2000, so commonly used in
> source code.
>
> SC2000 is a SoC of the Milbeaut series. equipped with a DSP optimized for
> computer vision. It also features advanced functionalities such as 360-degree,
> real-time spherical stitching with multi cameras, image stabilization for
> without mechanical gimbals, and rolling shutter correction. More detail is
> below:
> https://www.socionext.com/en/products/assp/milbeaut/SC2000.html
>
> Specifications for developers are below:
> - Quad-core 32bit Cortex-A7 on ARMv7-A architecture
> - NEON support
> - DSP
> - GPU
> - MAX 3GB DDR3
> - Cortex-M0 for power control
> - NAND Flash Interface
> - SD UHS-I
> - SD UHS-II
> - SDIO
> - USB2.0 HOST / Device
> - USB3.0 HOST / Device
> - PCI express Gen2
> - Ethernet Engine
> - I2C
> - UART
> - SPI
> - PWM
>
> Support is quite minimal for now, since it only includes timer, clock,
> pictrl and serial controller drivers, so we can only boot to userspace
> through initramfs. Support for the other peripherals will come eventually.
I've looked over the platform once more. Overall, it looks very good, and
I'd still like to merge this for linux-5.1, but we are running out of time
there. If you send the patches to soc@kernel.org quickly, we can try to
still merge them in, but if anything goes wrong, it will have to wait until
we start merging patches for 5.2, directly after 5.1 is out.
I did not look at the device driver patches (clk, clocksource, pinctrl, serial)
in much detail. If you have an Ack from the maintainers, feel free to
include them in the series, otherwise let's merge the rest now and then
you can send the updated patches for inclusion through the subsystem
trees in 5.2.
I have sent a few comments. The only one that is really important
here is the missing platform check in the suspend options. Please
try to address most of the other commentsm, either by changing the
code, or by explaining why your version is correct.
Arnd
^ permalink raw reply
* Re: [PATCH v7 3/6] dt-bindings: pinctrl: mt8183: add binding document
From: Zhiyong Tao @ 2019-02-18 9:44 UTC (permalink / raw)
To: Matthias Brugger
Cc: Rob Herring, Mark Rutland, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Greg Kroah-Hartman, Stephen Boyd, devicetree,
srv_heupstream, linux-kernel, linux-serial, linux-mediatek,
linux-arm-kernel, yingjoe.chen, mars.cheng, eddie.huang,
linux-clk, Linus Walleij, Erin Lo
In-Reply-To: <2b221339-3933-2a25-ab60-d4d772d2873a@gmail.com>
On Fri, 2019-02-15 at 10:35 +0100, Matthias Brugger wrote:
>
> On 15/02/2019 07:02, Erin Lo wrote:
> > From: Zhiyong Tao <zhiyong.tao@mediatek.com>
> >
> > The commit adds mt8183 compatible node in binding document.
> >
> > Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com>
> > Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> > ---
> > .../devicetree/bindings/pinctrl/pinctrl-mt8183.txt | 115 +++++++++++++++++++++
> > 1 file changed, 115 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt
> > new file mode 100644
> > index 0000000..364e673
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt
> > @@ -0,0 +1,115 @@
> > +* Mediatek MT8183 Pin Controller
> > +
> > +The Mediatek's Pin controller is used to control SoC pins.
> > +
> > +Required properties:
> > +- compatible: value should be one of the following.
> > + "mediatek,mt8183-pinctrl", compatible with mt8183 pinctrl.
> > +- gpio-controller : Marks the device node as a gpio controller.
> > +- #gpio-cells: number of cells in GPIO specifier. Since the generic GPIO
> > + binding is used, the amount of cells must be specified as 2. See the below
> > + mentioned gpio binding representation for description of particular cells.
> > +- gpio-ranges : gpio valid number range.
> > +- reg: physicall address base for gpio base registers. There are nine
>
> s/physicall/physical
==>we will modify it.
>
> > + physicall address base in mt8183. They are 0x10005000, 0x11F20000,
> > + 0x11E80000, 0x11E70000, 0x11E90000, 0x11D30000, 0x11D20000, 0x11C50000,
> > + 0x11F30000.
> > +
> > + Eg: <&pio 6 0>
> > + <[phandle of the gpio controller node]
> > + [line number within the gpio controller]
> > + [flags]>
> > +
> > + Values for gpio specifier:
> > + - Line number: is a value between 0 to 202.
> > + - Flags: bit field of flags, as defined in <dt-bindings/gpio/gpio.h>.
> > + Only the following flags are supported:
> > + 0 - GPIO_ACTIVE_HIGH
> > + 1 - GPIO_ACTIVE_LOW
> > +
> > +Optional properties:
> > +- reg-names: gpio base register names. There are nine gpio base register
> > + names in mt8183. They are "iocfg0", "iocfg1", "iocfg2", "iocfg3", "iocfg4",
> > + "iocfg5", "iocfg6", "iocfg7", "iocfg8".
> > +- interrupt-controller: Marks the device node as an interrupt controller
> > +- #interrupt-cells: Should be two.
> > +- interrupts : The interrupt outputs from the controller.
>
> we are missing interrupt-parent here.
==> we will add it in next version. we will add like this:
- interrupts-parents : The interrupt connector. In mt8183, it is sysirq.
>
> > +
> > +Please refer to pinctrl-bindings.txt in this directory for details of the
> > +common pinctrl bindings used by client devices.
> > +
> > +Subnode format
> > +A pinctrl node should contain at least one subnodes representing the
> > +pinctrl groups available on the machine. Each subnode will list the
> > +pins it needs, and how they should be configured, with regard to muxer
> > +configuration, pullups, drive strength, input enable/disable and input schmitt.
> > +
> > + node {
> > + pinmux = <PIN_NUMBER_PINMUX>;
> > + GENERIC_PINCONFIG;
> > + };
> > +
> > +Required properties:
> > +- pinmux: integer array, represents gpio pin number and mux setting.
> > + Supported pin number and mux varies for different SoCs, and are defined
> > + as macros in boot/dts/<soc>-pinfunc.h directly.
> > +
> > +Optional properties:
> > +- GENERIC_PINCONFIG: is the generic pinconfig options to use, bias-disable,
> > + bias-pull-down, bias-pull-up, input-enable, input-disable, output-low, output-high,
> > + input-schmitt-enable, input-schmitt-disable and drive-strength are valid.
> > +
> > + Some special pins have extra pull up strength, there are R0 and R1 pull-up
> > + resistors available, but for user, it's only need to set R1R0 as 00, 01, 10 or 11.
> > + So when config mediatek,pull-up-adv or mediatek,pull-down-adv,
> > + it support arguments for those special pins.
>
> I wonder if we should mention which this special pins are. A look at the driver
> told me that it is not possible to know that by reading the source code neither.
> Linus, what do you think?
>
> What about mediatek,tdsel and mediatek,rdsel? They are not supported?
>
> Wouldn't it make sense to have one binding description for paris and then just
> create SoC specifics bindings?
==> It can support mediatek,tdsel and mediatek,rdsel in mt8183 design.
I will try to create one new binding description and separate it from
soc specifics bindings.
>
> > +
> > + When config drive-strength, it can support some arguments, such as
> > + MTK_DRIVE_4mA, MTK_DRIVE_6mA, etc. See dt-bindings/pinctrl/mt65xx.h.
>
> It can support all drive strength defined in that header file, 2, 4, 6, 8, 10,
> 12, 14, 16, 20, 24, 28 and 32 mA?
==>It can only support drive strength 2,4,6,8,10,12,14,16mA in mt8183.
>
> > +
> > +Examples:
> > +
> > +#include "mt8183-pinfunc.h"
> > +
> > +...
> > +{
> > + pio: pinctrl@10005000 {
> > + compatible = "mediatek,mt8183-pinctrl";
> > + reg = <0 0x10005000 0 0x1000>,
> > + <0 0x11F20000 0 0x1000>,
> > + <0 0x11E80000 0 0x1000>,
> > + <0 0x11E70000 0 0x1000>,
> > + <0 0x11E90000 0 0x1000>,
> > + <0 0x11D30000 0 0x1000>,
> > + <0 0x11D20000 0 0x1000>,
> > + <0 0x11C50000 0 0x1000>,
> > + <0 0x11F30000 0 0x1000>;
> > + reg-names = "iocfg0", "iocfg1", "iocfg2",
> > + "iocfg3", "iocfg4", "iocfg5",
> > + "iocfg6", "iocfg7", "iocfg8";
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + gpio-ranges = <&pio 0 0 192>;
> > + interrupt-controller;
> > + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
> > + interrupt-parent = <&gic>;
> > + #interrupt-cells = <2>;
> > +
> > + i2c0_pins_a: i2c0 {
> > + pins1 {
> > + pinmux = <PINMUX_GPIO48__FUNC_SCL5>,
> > + <PINMUX_GPIO49__FUNC_SDA5>;
> > + mediatek,pull-up-adv = <11>;
> > + };
> > + };
> > +
> > + i2c1_pins_a: i2c1 {
> > + pins {
> > + pinmux = <PINMUX_GPIO50__FUNC_SCL3>,
> > + <PINMUX_GPIO51__FUNC_SDA3>;
> > + mediatek,pull-down-adv = <10>;
> > + };
> > + };
> > + ...
> > + };
> > +};
> >
^ permalink raw reply
* Re: [PATCH v7 4/6] arm64: dts: Add Mediatek SoC MT8183 and evaluation board dts and Makefile
From: Erin Lo @ 2019-02-18 6:10 UTC (permalink / raw)
To: Linus Walleij
Cc: Matthias Brugger, Rob Herring, Mark Rutland, Thomas Gleixner,
Jason Cooper, Marc Zyngier, Greg Kroah-Hartman, Stephen Boyd,
devicetree, srv_heupstream, linux-kernel, linux-serial,
linux-mediatek, linux-arm-kernel, yingjoe.chen, mars.cheng,
eddie.huang, linux-clk, Ben Ho, Seiya Wang, Weiyi Lu, Zhiyong
In-Reply-To: <1550210558-30516-5-git-send-email-erin.lo@mediatek.com>
On Fri, 2019-02-15 at 14:02 +0800, Erin Lo wrote:
> From: Ben Ho <Ben.Ho@mediatek.com>
>
> Add basic chip support for Mediatek 8183, include
> uart node with correct uart clocks, pwrap device
>
> Add clock controller nodes, include topckgen, infracfg,
> apmixedsys and subsystem.
>
> Signed-off-by: Ben Ho <Ben.Ho@mediatek.com>
> Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> Signed-off-by: Seiya Wang <seiya.wang@mediatek.com>
> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
> Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com>
> Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>
> Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
> ---
> arch/arm64/boot/dts/mediatek/Makefile | 1 +
> arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 31 +++
> arch/arm64/boot/dts/mediatek/mt8183.dtsi | 336 ++++++++++++++++++++++++++++
> 3 files changed, 368 insertions(+)
> create mode 100644 arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> create mode 100644 arch/arm64/boot/dts/mediatek/mt8183.dtsi
>
> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> index e8f952f..458bbc4 100644
> --- a/arch/arm64/boot/dts/mediatek/Makefile
> +++ b/arch/arm64/boot/dts/mediatek/Makefile
> @@ -7,3 +7,4 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-evb.dtb
> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8183-evb.dtb
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> new file mode 100644
> index 0000000..9b52559
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> @@ -0,0 +1,31 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: Ben Ho <ben.ho@mediatek.com>
> + * Erin Lo <erin.lo@mediatek.com>
> + */
> +
> +/dts-v1/;
> +#include "mt8183.dtsi"
> +
> +/ {
> + model = "MediaTek MT8183 evaluation board";
> + compatible = "mediatek,mt8183-evb", "mediatek,mt8183";
> +
> + aliases {
> + serial0 = &uart0;
> + };
> +
> + memory@40000000 {
> + device_type = "memory";
> + reg = <0 0x40000000 0 0x80000000>;
> + };
> +
> + chosen {
> + stdout-path = "serial0:921600n8";
> + };
> +};
> +
> +&uart0 {
> + status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> new file mode 100644
> index 0000000..1bd1399
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -0,0 +1,336 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: Ben Ho <ben.ho@mediatek.com>
> + * Erin Lo <erin.lo@mediatek.com>
> + */
> +
> +#include <dt-bindings/clock/mt8183-clk.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +/ {
> + compatible = "mediatek,mt8183";
> + interrupt-parent = <&sysirq>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu-map {
> + cluster0 {
> + core0 {
> + cpu = <&cpu0>;
> + };
> + core1 {
> + cpu = <&cpu1>;
> + };
> + core2 {
> + cpu = <&cpu2>;
> + };
> + core3 {
> + cpu = <&cpu3>;
> + };
> + };
> +
> + cluster1 {
> + core0 {
> + cpu = <&cpu4>;
> + };
> + core1 {
> + cpu = <&cpu5>;
> + };
> + core2 {
> + cpu = <&cpu6>;
> + };
> + core3 {
> + cpu = <&cpu7>;
> + };
> + };
> + };
> +
> + cpu0: cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x000>;
> + enable-method = "psci";
> + };
> +
> + cpu1: cpu@1 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x001>;
> + enable-method = "psci";
> + };
> +
> + cpu2: cpu@2 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x002>;
> + enable-method = "psci";
> + };
> +
> + cpu3: cpu@3 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x003>;
> + enable-method = "psci";
> + };
> +
> + cpu4: cpu@100 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a73";
> + reg = <0x100>;
> + enable-method = "psci";
> + };
> +
> + cpu5: cpu@101 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a73";
> + reg = <0x101>;
> + enable-method = "psci";
> + };
> +
> + cpu6: cpu@102 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a73";
> + reg = <0x102>;
> + enable-method = "psci";
> + };
> +
> + cpu7: cpu@103 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a73";
> + reg = <0x103>;
> + enable-method = "psci";
> + };
> + };
> +
> + pmu-a53 {
> + compatible = "arm,cortex-a53-pmu";
> + interrupt-parent = <&gic>;
> + interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW &ppi_cluster0>;
> + };
> +
> + pmu-a73 {
> + compatible = "arm,cortex-a73-pmu";
> + interrupt-parent = <&gic>;
> + interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW &ppi_cluster1>;
> + };
> +
> + psci {
> + compatible = "arm,psci-1.0";
> + method = "smc";
> + };
> +
> + clk26m: oscillator {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <26000000>;
> + clock-output-names = "clk26m";
> + };
> +
> + timer {
> + compatible = "arm,armv8-timer";
> + interrupt-parent = <&gic>;
> + interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW 0>,
> + <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW 0>,
> + <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW 0>,
> + <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW 0>;
> + };
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + compatible = "simple-bus";
> + ranges;
> +
> + gic: interrupt-controller@c000000 {
> + compatible = "arm,gic-v3";
> + #interrupt-cells = <4>;
> + interrupt-parent = <&gic>;
> + interrupt-controller;
> + reg = <0 0x0c000000 0 0x40000>, /* GICD */
> + <0 0x0c100000 0 0x200000>, /* GICR */
> + <0 0x0c400000 0 0x2000>, /* GICC */
> + <0 0x0c410000 0 0x1000>, /* GICH */
> + <0 0x0c420000 0 0x2000>; /* GICV */
> +
> + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH 0>;
> + ppi-partitions {
> + ppi_cluster0: interrupt-partition-0 {
> + affinity = <&cpu0 &cpu1 &cpu2 &cpu3>;
> + };
> + ppi_cluster1: interrupt-partition-1 {
> + affinity = <&cpu4 &cpu5 &cpu6 &cpu7>;
> + };
> + };
> + };
> +
> + mcucfg: syscon@c530000 {
> + compatible = "mediatek,mt8183-mcucfg", "syscon";
> + reg = <0 0x0c530000 0 0x1000>;
> + #clock-cells = <1>;
> + };
> +
> + sysirq: interrupt-controller@c530a80 {
> + compatible = "mediatek,mt8183-sysirq",
> + "mediatek,mt6577-sysirq";
> + interrupt-controller;
> + #interrupt-cells = <3>;
> + interrupt-parent = <&gic>;
> + reg = <0 0x0c530a80 0 0x50>;
> + };
> +
> + topckgen: syscon@10000000 {
> + compatible = "mediatek,mt8183-topckgen", "syscon";
> + reg = <0 0x10000000 0 0x1000>;
> + #clock-cells = <1>;
> + };
> +
> + infracfg: syscon@10001000 {
> + compatible = "mediatek,mt8183-infracfg", "syscon";
> + reg = <0 0x10001000 0 0x1000>;
> + #clock-cells = <1>;
> + };
> +
> + pio: pinctrl@10005000 {
> + compatible = "mediatek,mt8183-pinctrl";
> + reg = <0 0x10005000 0 0x1000>,
> + <0 0x11f20000 0 0x1000>,
> + <0 0x11e80000 0 0x1000>,
> + <0 0x11e70000 0 0x1000>,
> + <0 0x11e90000 0 0x1000>,
> + <0 0x11d30000 0 0x1000>,
> + <0 0x11d20000 0 0x1000>,
> + <0 0x11c50000 0 0x1000>,
> + <0 0x11f30000 0 0x1000>,
> + <0 0x1000b000 0 0x1000>;
> + reg-names = "iocfg0", "iocfg1", "iocfg2",
> + "iocfg3", "iocfg4", "iocfg5",
> + "iocfg6", "iocfg7", "iocfg8",
> + "eint";
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&pio 0 0 192>;
> + interrupt-controller;
> + interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH 0>;
> + interrupt-parent = <&gic>;
interrupt-parent should be sysirq not gic
We will remove it in next version.
Best Regards,
Erin
> + #interrupt-cells = <4>;
> + };
> +
> + apmixedsys: syscon@1000c000 {
> + compatible = "mediatek,mt8183-apmixedsys", "syscon";
> + reg = <0 0x1000c000 0 0x1000>;
> + #clock-cells = <1>;
> + };
> +
> + pwrap: pwrap@1000d000 {
> + compatible = "mediatek,mt8183-pwrap";
> + reg = <0 0x1000d000 0 0x1000>;
> + reg-names = "pwrap";
> + interrupts = <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&topckgen CLK_TOP_MUX_PMICSPI>,
> + <&infracfg CLK_INFRA_PMIC_AP>;
> + clock-names = "spi", "wrap";
> + };
> +
> + uart0: serial@11002000 {
> + compatible = "mediatek,mt8183-uart",
> + "mediatek,mt6577-uart";
> + reg = <0 0x11002000 0 0x1000>;
> + interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&clk26m>, <&infracfg CLK_INFRA_UART0>;
> + clock-names = "baud", "bus";
> + status = "disabled";
> + };
> +
> + uart1: serial@11003000 {
> + compatible = "mediatek,mt8183-uart",
> + "mediatek,mt6577-uart";
> + reg = <0 0x11003000 0 0x1000>;
> + interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&clk26m>, <&infracfg CLK_INFRA_UART1>;
> + clock-names = "baud", "bus";
> + status = "disabled";
> + };
> +
> + uart2: serial@11004000 {
> + compatible = "mediatek,mt8183-uart",
> + "mediatek,mt6577-uart";
> + reg = <0 0x11004000 0 0x1000>;
> + interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&clk26m>, <&infracfg CLK_INFRA_UART2>;
> + clock-names = "baud", "bus";
> + status = "disabled";
> + };
> +
> + audiosys: syscon@11220000 {
> + compatible = "mediatek,mt8183-audiosys", "syscon";
> + reg = <0 0x11220000 0 0x1000>;
> + #clock-cells = <1>;
> + };
> +
> + mfgcfg: syscon@13000000 {
> + compatible = "mediatek,mt8183-mfgcfg", "syscon";
> + reg = <0 0x13000000 0 0x1000>;
> + #clock-cells = <1>;
> + };
> +
> + mmsys: syscon@14000000 {
> + compatible = "mediatek,mt8183-mmsys", "syscon";
> + reg = <0 0x14000000 0 0x1000>;
> + #clock-cells = <1>;
> + };
> +
> + imgsys: syscon@15020000 {
> + compatible = "mediatek,mt8183-imgsys", "syscon";
> + reg = <0 0x15020000 0 0x1000>;
> + #clock-cells = <1>;
> + };
> +
> + vdecsys: syscon@16000000 {
> + compatible = "mediatek,mt8183-vdecsys", "syscon";
> + reg = <0 0x16000000 0 0x1000>;
> + #clock-cells = <1>;
> + };
> +
> + vencsys: syscon@17000000 {
> + compatible = "mediatek,mt8183-vencsys", "syscon";
> + reg = <0 0x17000000 0 0x1000>;
> + #clock-cells = <1>;
> + };
> +
> + ipu_conn: syscon@19000000 {
> + compatible = "mediatek,mt8183-ipu_conn", "syscon";
> + reg = <0 0x19000000 0 0x1000>;
> + #clock-cells = <1>;
> + };
> +
> + ipu_adl: syscon@19010000 {
> + compatible = "mediatek,mt8183-ipu_adl", "syscon";
> + reg = <0 0x19010000 0 0x1000>;
> + #clock-cells = <1>;
> + };
> +
> + ipu_core0: syscon@19180000 {
> + compatible = "mediatek,mt8183-ipu_core0", "syscon";
> + reg = <0 0x19180000 0 0x1000>;
> + #clock-cells = <1>;
> + };
> +
> + ipu_core1: syscon@19280000 {
> + compatible = "mediatek,mt8183-ipu_core1", "syscon";
> + reg = <0 0x19280000 0 0x1000>;
> + #clock-cells = <1>;
> + };
> +
> + camsys: syscon@1a000000 {
> + compatible = "mediatek,mt8183-camsys", "syscon";
> + reg = <0 0x1a000000 0 0x1000>;
> + #clock-cells = <1>;
> + };
> + };
> +};
^ permalink raw reply
* Re: [PATCH] tty: serial: msm_serial: Remove __init from msm_console_setup()
From: Bjorn Andersson @ 2019-02-17 7:14 UTC (permalink / raw)
To: Jeffrey Hugo
Cc: andy.gross, david.brown, gregkh, jslaby, linux-arm-msm,
linux-serial, linux-kernel
In-Reply-To: <1550379952-19457-1-git-send-email-jhugo@codeaurora.org>
On Sat 16 Feb 21:05 PST 2019, Jeffrey Hugo wrote:
> Due to the complexities of modern Qualcomm SoCs, about a half dozen drivers
> must successfully probe before the clocks for the console are present, and
> the console can successfully probe. Depending on several random factors
> such as probe order and modules vs builtin, msm_serial may not be able to
> successfully probe for some, at which point, __init annotated functions
> may become unmapped. If this occurs, msm_console_setup() will be called
> from the probe path, but will no longer exist, resulting in a kernel
> panic.
>
> Resolve this issue by removing the __init annotation from
> msm_console_setup().
I'm pretty sure I've stumbled upon this several times without knowing
what hit me.
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Regards,
Bjorn
>
> Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org>
> ---
> drivers/tty/serial/msm_serial.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
> index 736b74f..1090960 100644
> --- a/drivers/tty/serial/msm_serial.c
> +++ b/drivers/tty/serial/msm_serial.c
> @@ -1634,7 +1634,7 @@ static void msm_console_write(struct console *co, const char *s,
> __msm_console_write(port, s, count, msm_port->is_uartdm);
> }
>
> -static int __init msm_console_setup(struct console *co, char *options)
> +static int msm_console_setup(struct console *co, char *options)
> {
> struct uart_port *port;
> int baud = 115200;
> --
> Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
> Qualcomm Technologies, Inc. is a member of the
> Code Aurora Forum, a Linux Foundation Collaborative Project.
>
^ 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