* Re: [PATCH v2] tty: serial: msm_serial: Fix XON/XOFF
From: Jorge Ramirez @ 2019-05-20 18:57 UTC (permalink / raw)
To: Bjorn Andersson
Cc: agross, david.brown, gregkh, sboyd, jslaby, keescook, anton,
ccross, tony.luck, linux-arm-msm, linux-serial, linux-kernel
In-Reply-To: <20190520185008.GX2085@tuxbook-pro>
On 5/20/19 20:50, Bjorn Andersson wrote:
> On Mon 20 May 11:38 PDT 2019, Jorge Ramirez-Ortiz wrote:
>
>> When the tty layer requests the uart to throttle, the current code
>> executing in msm_serial will trigger "Bad mode in Error Handler" and
>> generate an invalid stack frame in pstore before rebooting (that is if
>> pstore is indeed configured: otherwise the user shall just notice a
>> reboot with no further information dumped to the console).
>>
>> This patch replaces the PIO byte accessor with the word accessor
>> already used in PIO mode.
>>
>> Fixes: 68252424a7c7 ("tty: serial: msm: Support big-endian CPUs")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
>> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>
> You missed Stephen's
>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
argh sorry Stephen. can the maintainer add it when it gets merged or
shall I post V3?
>
> Regards,
> Bjorn
>
>> ---
>> drivers/tty/serial/msm_serial.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
>> index 109096033bb1..23833ad952ba 100644
>> --- a/drivers/tty/serial/msm_serial.c
>> +++ b/drivers/tty/serial/msm_serial.c
>> @@ -860,6 +860,7 @@ static void msm_handle_tx(struct uart_port *port)
>> struct circ_buf *xmit = &msm_port->uart.state->xmit;
>> struct msm_dma *dma = &msm_port->tx_dma;
>> unsigned int pio_count, dma_count, dma_min;
>> + char buf[4] = { 0 };
>> void __iomem *tf;
>> int err = 0;
>>
>> @@ -869,10 +870,12 @@ static void msm_handle_tx(struct uart_port *port)
>> else
>> tf = port->membase + UART_TF;
>>
>> + buf[0] = port->x_char;
>> +
>> if (msm_port->is_uartdm)
>> msm_reset_dm_count(port, 1);
>>
>> - iowrite8_rep(tf, &port->x_char, 1);
>> + iowrite32_rep(tf, buf, 1);
>> port->icount.tx++;
>> port->x_char = 0;
>> return;
>> --
>> 2.21.0
>>
>
^ permalink raw reply
* Re: [RFC v2 00/11] DVFS in the OPP core
From: Viresh Kumar @ 2019-05-21 6:22 UTC (permalink / raw)
To: Rajendra Nayak
Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
dri-devel, linux-scsi, swboyd, ulf.hansson, dianders, rafael,
vincent.guittot
In-Reply-To: <20190320094918.20234-1-rnayak@codeaurora.org>
On 20-03-19, 15:19, Rajendra Nayak wrote:
> This is a v2 of the RFC posted earlier by Stephen Boyd [1]
>
> As part of v2 I still follow the same approach of dev_pm_opp_set_rate()
> API using clk framework to round the frequency passed and making it
> accept 0 as a valid frequency indicating the frequency isn't required
> anymore. It just has a few more drivers converted to use this approach
> like dsi/dpu and ufs.
> ufs demonstrates the case of having to handle multiple power domains, one
> of which is scalable.
>
> The patches are based on 5.1-rc1 and depend on some ufs fixes I posted
> earlier [2] and a DT patch to include the rpmpd header [3]
>
> [1] https://lkml.org/lkml/2019/1/28/2086
> [2] https://lkml.org/lkml/2019/3/8/70
> [3] https://lkml.org/lkml/2019/3/20/120
Hi Rajendra,
I am inclined to apply/push this series for 5.3-rc1, will it be
possible for you to spend some time on this at priority ?
--
viresh
^ permalink raw reply
* [PATCH 0/2] support 8250-mtk uart in-band wake up
From: Claire Chang @ 2019-05-21 8:46 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
Cc: Claire Chang, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
changqi.hu-NuS5LvNUpcJWk0Htik3J/w
enable irq wake on an edge sensitive interrupt of Rx pin before suspend to
support uart in-band wake up
Claire Chang (2):
dt-bindings: serial: add documentation for Rx in-band wakeup support
uart: mediatek: support Rx in-band wakeup
.../devicetree/bindings/serial/mtk-uart.txt | 13 ++++++--
drivers/tty/serial/8250/8250_mtk.c | 33 +++++++++++++++++++
2 files changed, 44 insertions(+), 2 deletions(-)
--
2.21.0.1020.gf2820cf01a-goog
^ permalink raw reply
* [PATCH 1/2] dt-bindings: serial: add documentation for Rx in-band wakeup support
From: Claire Chang @ 2019-05-21 8:47 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
Cc: Claire Chang, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
changqi.hu-NuS5LvNUpcJWk0Htik3J/w
In-Reply-To: <20190521084701.100179-1-tientzu-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Claire Chang <tientzu-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
.../devicetree/bindings/serial/mtk-uart.txt | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
index c6b5262eb352..11966de2a4b3 100644
--- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
+++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
@@ -23,7 +23,12 @@ Required properties:
- reg: The base address of the UART register bank.
-- interrupts: A single interrupt specifier.
+- interrupts or interrupts-extended:
+ index 0: an interrupt specifier for the UART controller itself
+ index 1: optional, an interrupt specifier with edge sensitivity on Rx pin to
+ support Rx in-band wake up. If one would like to use this feature,
+ one must create an addtional pinctrl to reconfigure Rx pin to normal
+ GPIO before suspend.
- clocks : Must contain an entry for each entry in clock-names.
See ../clocks/clock-bindings.txt for details.
@@ -39,7 +44,11 @@ Example:
uart0: serial@11006000 {
compatible = "mediatek,mt6589-uart", "mediatek,mt6577-uart";
reg = <0x11006000 0x400>;
- interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
+ interrupts-extended = <&sysirq GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>,
+ <&gpio 121 IRQ_TYPE_EDGE_FALLING>;
clocks = <&uart_clk>, <&bus_clk>;
clock-names = "baud", "bus";
+ pinctrl-names = "default", "sleep";
+ pinctrl-0 = <&uart_pin>;
+ pinctrl-1 = <&uart_pin_sleep>;
};
--
2.21.0.1020.gf2820cf01a-goog
^ permalink raw reply related
* [PATCH 2/2] uart: mediatek: support Rx in-band wakeup
From: Claire Chang @ 2019-05-21 8:47 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
Cc: Claire Chang, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
changqi.hu-NuS5LvNUpcJWk0Htik3J/w
In-Reply-To: <20190521084701.100179-1-tientzu-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
In order to support Rx in-band wakeup, we need to enable irq wake on an
edge sensitive interrupt of Rx pin before suspend and disable it when
resuming.
This interrupt is used only as wake source to resume the system when
suspended. Note that the sent character will be lost as the controller is
actually suspended.
We use this to support wakeup on bluetooth. Bluetooth will repeatedly send
0xFD to wakeup host. Once host detects Rx falling, an interrupt is
triggered, and the system leaves sleep state. Then, the bluetooth driver
will send 0xFC to bluetooth and bluetooth can start to send normal HCI
packets.
Signed-off-by: Claire Chang <tientzu-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
drivers/tty/serial/8250/8250_mtk.c | 33 ++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index 417c7c810df9..61892abf707d 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -10,6 +10,7 @@
#include <linux/module.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
+#include <linux/pinctrl/consumer.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/serial_8250.h>
@@ -70,6 +71,7 @@ struct mtk8250_data {
#ifdef CONFIG_SERIAL_8250_DMA
enum dma_rx_status rx_status;
#endif
+ int rx_wakeup_irq;
};
/* flow control mode */
@@ -551,6 +553,8 @@ static int mtk8250_probe(struct platform_device *pdev)
pm_runtime_set_active(&pdev->dev);
pm_runtime_enable(&pdev->dev);
+ data->rx_wakeup_irq = platform_get_irq(pdev, 1);
+
return 0;
}
@@ -572,15 +576,44 @@ static int mtk8250_remove(struct platform_device *pdev)
static int __maybe_unused mtk8250_suspend(struct device *dev)
{
struct mtk8250_data *data = dev_get_drvdata(dev);
+ struct uart_8250_port *up = serial8250_get_port(data->line);
+ int irq = data->rx_wakeup_irq;
+ int err;
serial8250_suspend_port(data->line);
+ pinctrl_pm_select_sleep_state(dev);
+ if (irq >= 0) {
+ err = enable_irq_wake(irq);
+ if (err) {
+ dev_err(dev,
+ "failed to enable irq wake on IRQ %d: %d\n",
+ irq, err);
+ pinctrl_pm_select_default_state(dev);
+ serial8250_resume_port(data->line);
+ return err;
+ }
+ }
+
return 0;
}
static int __maybe_unused mtk8250_resume(struct device *dev)
{
struct mtk8250_data *data = dev_get_drvdata(dev);
+ int irq = data->rx_wakeup_irq;
+ int err;
+
+ if (irq >= 0) {
+ err = disable_irq_wake(irq);
+ if (err) {
+ dev_err(dev,
+ "failed to disable irq wake on IRQ %d: %d\n",
+ irq, err);
+ return err;
+ }
+ }
+ pinctrl_pm_select_default_state(dev);
serial8250_resume_port(data->line);
--
2.21.0.1020.gf2820cf01a-goog
^ permalink raw reply related
* Re: [PATCH 2/2] uart: mediatek: support Rx in-band wakeup
From: Nicolas Boichat @ 2019-05-21 9:44 UTC (permalink / raw)
To: Claire Chang
Cc: Greg Kroah-Hartman, moderated list:ARM/Mediatek SoC support,
changqi.hu-NuS5LvNUpcJWk0Htik3J/w,
linux-serial-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20190521084701.100179-3-tientzu-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
On Tue, May 21, 2019 at 4:47 PM Claire Chang <tientzu-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>
> In order to support Rx in-band wakeup, we need to enable irq wake on an
> edge sensitive interrupt of Rx pin before suspend and disable it when
> resuming.
>
> This interrupt is used only as wake source to resume the system when
> suspended. Note that the sent character will be lost as the controller is
> actually suspended.
>
> We use this to support wakeup on bluetooth. Bluetooth will repeatedly send
> 0xFD to wakeup host. Once host detects Rx falling, an interrupt is
> triggered, and the system leaves sleep state. Then, the bluetooth driver
> will send 0xFC to bluetooth and bluetooth can start to send normal HCI
> packets.
>
> Signed-off-by: Claire Chang <tientzu-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
> drivers/tty/serial/8250/8250_mtk.c | 33 ++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> index 417c7c810df9..61892abf707d 100644
> --- a/drivers/tty/serial/8250/8250_mtk.c
> +++ b/drivers/tty/serial/8250/8250_mtk.c
> @@ -10,6 +10,7 @@
> #include <linux/module.h>
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> +#include <linux/pinctrl/consumer.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> #include <linux/serial_8250.h>
> @@ -70,6 +71,7 @@ struct mtk8250_data {
> #ifdef CONFIG_SERIAL_8250_DMA
> enum dma_rx_status rx_status;
> #endif
> + int rx_wakeup_irq;
> };
>
> /* flow control mode */
> @@ -551,6 +553,8 @@ static int mtk8250_probe(struct platform_device *pdev)
> pm_runtime_set_active(&pdev->dev);
> pm_runtime_enable(&pdev->dev);
>
> + data->rx_wakeup_irq = platform_get_irq(pdev, 1);
> +
> return 0;
> }
>
> @@ -572,15 +576,44 @@ static int mtk8250_remove(struct platform_device *pdev)
> static int __maybe_unused mtk8250_suspend(struct device *dev)
> {
> struct mtk8250_data *data = dev_get_drvdata(dev);
> + struct uart_8250_port *up = serial8250_get_port(data->line);
> + int irq = data->rx_wakeup_irq;
> + int err;
>
> serial8250_suspend_port(data->line);
>
> + pinctrl_pm_select_sleep_state(dev);
> + if (irq >= 0) {
> + err = enable_irq_wake(irq);
> + if (err) {
> + dev_err(dev,
> + "failed to enable irq wake on IRQ %d: %d\n",
> + irq, err);
> + pinctrl_pm_select_default_state(dev);
> + serial8250_resume_port(data->line);
> + return err;
> + }
> + }
> +
> return 0;
> }
>
> static int __maybe_unused mtk8250_resume(struct device *dev)
> {
> struct mtk8250_data *data = dev_get_drvdata(dev);
> + int irq = data->rx_wakeup_irq;
> + int err;
> +
> + if (irq >= 0) {
> + err = disable_irq_wake(irq);
> + if (err) {
> + dev_err(dev,
> + "failed to disable irq wake on IRQ %d: %d\n",
> + irq, err);
> + return err;
I'd drop this return err in the resume path, still better to restore
the pinctrl and resume the port, even if we can't disable the irq_wake
(which, really, shouldn't happen if we could enable it earlier...).
Also, many other callers of disable_irq_wake just ignore the return
value, so maybe it's ok to do the same, but I'll let other people
comment.
> + }
> + }
> + pinctrl_pm_select_default_state(dev);
>
> serial8250_resume_port(data->line);
>
> --
> 2.21.0.1020.gf2820cf01a-goog
>
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH 2/2] serial: 8250: Add support for 8250/16550 as MFD function
From: Greg Kroah-Hartman @ 2019-05-21 10:09 UTC (permalink / raw)
To: Esben Haabendal
Cc: Lee Jones, linux-serial, Jiri Slaby, Nishanth Menon, Vignesh R,
Tony Lindgren, Lokesh Vutla, Florian Fainelli, linux-kernel
In-Reply-To: <87imudky2o.fsf@haabendal.dk>
On Tue, May 14, 2019 at 02:41:35PM +0200, Esben Haabendal wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>
> > On Tue, May 14, 2019 at 11:47:41AM +0100, Lee Jones wrote:
> >> On Tue, 14 May 2019, Esben Haabendal wrote:
> >>
> >> > Lee Jones <lee.jones@linaro.org> writes:
> >> >
> >> > > On Tue, 07 May 2019, Esben Haabendal wrote:
> >> > >
> >> > >> Lee Jones <lee.jones@linaro.org> writes:
> >> > >>
> >> > >> > On Fri, 26 Apr 2019, Esben Haabendal wrote:
> >> > >> >
> >> > >> >> The serial8250-mfd driver is for adding 8250/16550 UART ports as functions
> >> > >> >> to an MFD driver.
> >> > >> >>
> >> > >> >> When calling mfd_add_device(), platform_data should be a pointer to a
> >> > >> >> struct plat_serial8250_port, with proper settings like .flags, .type,
> >> > >> >> .iotype, .regshift and .uartclk. Memory (or ioport) and IRQ should be
> >> > >> >> passed as cell resources.
> >> > >> >
> >> > >> > What? No, please!
> >> > >> >
> >> > >> > If you *must* create a whole driver just to be able to use
> >> > >> > platform_*() helpers (which I don't think you should), then please
> >> > >> > call it something else. This doesn't have anything to do with MFD.
> >> > >>
> >> > >> True.
> >> > >>
> >> > >> I really don't think it is a good idea to create a whole driver just to
> >> > >> be able to use platform_get_*() helpers. And if I am forced to do this,
> >> > >> because I am unable to convince Andy to improve the standard serial8250
> >> > >> driver to support that, it should be called MFD. The driver would be
> >> > >
> >> > > I assume you mean "shouldn't"?
> >> >
> >> > Of-course.
> >> >
> >> > >> generally usable for all usecases where platform_get_*() works.
> >> > >>
> >> > >> I don't have any idea what to call such a driver. It really would just
> >> > >> be a fork of the current serial8250 driver, just allowing use of
> >> > >> platform_get_*(), supporting exactly the same hardware.
> >> > >>
> >> > >> I am still hoping that we can find a way to improve serial8250 to be
> >> > >> usable in these cases.
> >> > >
> >> > > Me too.
> >> >
> >> > Unfortunately, I don't seem to be able to convince Andy to accept
> >> > something like that.
> >>
> >> Andy is not he Maintainer.
> >>
> >> What are Greg and Jiri's opinions?
> >
> > I've been ignoring all of this at the moment because of the 5.2-rc merge
> > window. I'll look at it after -rc1 is out.
> >
> > thanks,
> > greg k-h
>
> Great, thanks!
>
> I will try ad hold back with this thread until you get back to it.
Ok, I have no idea what is going on here, sorry. This is a really long
and meandering thread, and I can't even find the original patches in my
queue.
So can you resend things and we can start over? :)
But note, using a mfd for a uart seems VERY odd to me...
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] serial-uartlite: Fix null-ptr-deref in ulite_exit
From: Greg KH @ 2019-05-21 10:10 UTC (permalink / raw)
To: Johan Hovold
Cc: YueHaibing, jacmet, jslaby, shubhrajyoti.datta, linux-kernel,
linux-serial
In-Reply-To: <20190517075502.GE28564@localhost>
On Fri, May 17, 2019 at 09:55:02AM +0200, Johan Hovold wrote:
> On Thu, May 16, 2019 at 12:09:31PM +0800, YueHaibing wrote:
> > If ulite_probe is not called or failed to registed
> > uart_register_driver, unload the module will call
> > uart_unregister_driver, which will tigger NULL
> > pointer dereference like this:
> >
> > BUG: KASAN: null-ptr-deref in tty_unregister_driver+0x19/0x100
> > Read of size 4 at addr 0000000000000034 by task syz-executor.0/4246
>
> > This patch fix this by moving uart_unregister_driver
> > to ulite_remove.
> >
> > Reported-by: Hulk Robot <hulkci@huawei.com>
> > Fixes: 415b43bdb008 ("tty: serial: uartlite: Move uart register to probe")
> > Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> > ---
> > drivers/tty/serial/uartlite.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
> > index b8b912b..2e49fb6 100644
> > --- a/drivers/tty/serial/uartlite.c
> > +++ b/drivers/tty/serial/uartlite.c
> > @@ -867,6 +867,7 @@ static int ulite_remove(struct platform_device *pdev)
> > pm_runtime_disable(&pdev->dev);
> > pm_runtime_set_suspended(&pdev->dev);
> > pm_runtime_dont_use_autosuspend(&pdev->dev);
> > + uart_unregister_driver(&ulite_uart_driver);
>
> This broken. Consider what happens if you have tho ports registered and
> you unbind the first.
>
> Someone else sent a fix for this here
>
> https://lkml.kernel.org/r/20190514033219.169947-1-wangkefeng.wang@huawei.com
>
> That fix also has some issues, but is still better given the current
> state this driver is in.
I'm not taking any of these patches until people agree on what needs to
be done here :)
Why is this driver so "special" it is having these types of problems?
Why can't it do what all other drivers do in this case?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2] tty: serial: msm_serial: Fix XON/XOFF
From: Greg KH @ 2019-05-21 10:11 UTC (permalink / raw)
To: Jorge Ramirez
Cc: Bjorn Andersson, agross, david.brown, sboyd, jslaby, keescook,
anton, ccross, tony.luck, linux-arm-msm, linux-serial,
linux-kernel
In-Reply-To: <ef705e54-78bb-27e2-5060-31056234dad3@linaro.org>
On Mon, May 20, 2019 at 08:57:39PM +0200, Jorge Ramirez wrote:
> On 5/20/19 20:50, Bjorn Andersson wrote:
> > On Mon 20 May 11:38 PDT 2019, Jorge Ramirez-Ortiz wrote:
> >
> >> When the tty layer requests the uart to throttle, the current code
> >> executing in msm_serial will trigger "Bad mode in Error Handler" and
> >> generate an invalid stack frame in pstore before rebooting (that is if
> >> pstore is indeed configured: otherwise the user shall just notice a
> >> reboot with no further information dumped to the console).
> >>
> >> This patch replaces the PIO byte accessor with the word accessor
> >> already used in PIO mode.
> >>
> >> Fixes: 68252424a7c7 ("tty: serial: msm: Support big-endian CPUs")
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> >> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> >
> > You missed Stephen's
> >
> > Reviewed-by: Stephen Boyd <swboyd@chromium.org>
>
> argh sorry Stephen. can the maintainer add it when it gets merged or
> shall I post V3?
I'll fix it up...
^ permalink raw reply
* Re: [PATCH 0/7] tty: max310x: Simplify the code and fix a few bugs
From: Greg Kroah-Hartman @ 2019-05-21 10:17 UTC (permalink / raw)
To: Serge Semin; +Cc: Jiri Slaby, Serge Semin, linux-serial, linux-kernel
In-Reply-To: <20190514101415.26754-1-fancer.lancer@gmail.com>
On Tue, May 14, 2019 at 01:14:08PM +0300, Serge Semin wrote:
> I started using this driver two years ago in kernek 4.4 and then in kernel
> 4.9. It didn't go well from the very beginning due to my platform
> peculiarities: DW SPI core with hardware CS and relatively slow MIPS-based
> SoC. This patchset is intended to fix some of the problems I found out
> during the max310x driver utilization with max14830 device.
>
> First of all it was discovered, that workqueue API isn't optimally used.
> Work context isn't re-entrant by design, so the mutex used to guard the
> TX-method is redundant. schedule_work() method is also created in a way
> the work item is scheduled only if it isn't pending. Patch 1 concerns all
> these fixes. Seeing the similar container_of(uart_port) is used three
> times in the driver, the patch 2 introduces a macro to_max310x_port() to
> get a pointer to corresponding struct max310x_one. This is the code
> simplification and is going to be used in the following patches.
>
> It was found out, that batch read and write methods used buffers allocated
> on the kernel stack. Since they might be utilized by SPI controllers for
> DMA it might be unsafe on some platforms. Patch 3 provides a dedicated
> kmalloced buffers for this.
>
> The baud-rate calculator function didn't work correct for all the possible
> baud-rates requested within a pre-defined input reference frequency.
> Instead an algo fully compliant with datasheet divisor formulae is
> implemented in patch 4.
>
> Patches 5 and 6 are created to fix some rs485 issues. Particularly the
> rs485 mode is configured on the port startup if it's enabled. And seeing
> the mode2 register provides a way to enable/disable the echo-suppression
> in RS485 mode, it is used to implement the SER_RS485_RX_DURING_TX flag
> support.
>
> Finally it was discovered that in case if inbound hardware FIFO
> experienced overflow, a lot of '\0' characters inserted into the
> flip-buffer as a character of the RX-FIFO overrun. It isn't quite correct
> since the overflow happened only after the last character had been
> received. Patch 7 is dedicated to push only a single RX-FIFO overrun
> character in this case.
>
> Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
Nice cleanups, all now applied, thanks!
greg k-h
^ permalink raw reply
* Re: [PATCH v2] tty: serial: msm_serial: Fix XON/XOFF
From: Vinod Koul @ 2019-05-21 10:31 UTC (permalink / raw)
To: Jorge Ramirez-Ortiz
Cc: agross, david.brown, gregkh, sboyd, bjorn.andersson, jslaby,
keescook, anton, ccross, tony.luck, linux-arm-msm, linux-serial,
linux-kernel
In-Reply-To: <20190520183848.27719-1-jorge.ramirez-ortiz@linaro.org>
On 20-05-19, 20:38, Jorge Ramirez-Ortiz wrote:
> When the tty layer requests the uart to throttle, the current code
> executing in msm_serial will trigger "Bad mode in Error Handler" and
> generate an invalid stack frame in pstore before rebooting (that is if
> pstore is indeed configured: otherwise the user shall just notice a
> reboot with no further information dumped to the console).
>
> This patch replaces the PIO byte accessor with the word accessor
> already used in PIO mode.
>
> Fixes: 68252424a7c7 ("tty: serial: msm: Support big-endian CPUs")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-by: Vinod Koul <vkoul@kernel.org>
--
~Vinod
^ permalink raw reply
* Re: [PATCH 2/2] serial: 8250: Add support for 8250/16550 as MFD function
From: Esben Haabendal @ 2019-05-21 11:11 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Lee Jones, linux-serial, Jiri Slaby, Nishanth Menon, Vignesh R,
Tony Lindgren, Lokesh Vutla, Florian Fainelli, linux-kernel
In-Reply-To: <20190521100904.GA13612@kroah.com>
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> I will try ad hold back with this thread until you get back to it.
>
> Ok, I have no idea what is going on here, sorry. This is a really long
> and meandering thread, and I can't even find the original patches in my
> queue.
>
> So can you resend things and we can start over? :)
Will do.
> But note, using a mfd for a uart seems VERY odd to me...
Ok. In my case, I have a pcie card with an fpga which includes 5 uart
ports, 3 ethernet interfaces and a number of custom IP blocks.
I believe that an mfd driver for that pcie card in that case.
/Esben
^ permalink raw reply
* Re: [PATCH 2/2] serial: 8250: Add support for 8250/16550 as MFD function
From: Greg Kroah-Hartman @ 2019-05-21 11:18 UTC (permalink / raw)
To: Esben Haabendal
Cc: Lee Jones, linux-serial, Jiri Slaby, Nishanth Menon, Vignesh R,
Tony Lindgren, Lokesh Vutla, Florian Fainelli, linux-kernel
In-Reply-To: <87pnocm59v.fsf@haabendal.dk>
On Tue, May 21, 2019 at 01:11:08PM +0200, Esben Haabendal wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>
> >> I will try ad hold back with this thread until you get back to it.
> >
> > Ok, I have no idea what is going on here, sorry. This is a really long
> > and meandering thread, and I can't even find the original patches in my
> > queue.
> >
> > So can you resend things and we can start over? :)
>
> Will do.
>
> > But note, using a mfd for a uart seems VERY odd to me...
>
> Ok. In my case, I have a pcie card with an fpga which includes 5 uart
> ports, 3 ethernet interfaces and a number of custom IP blocks.
> I believe that an mfd driver for that pcie card in that case.
I believe you need to fix that fpga to expose individual pci devices
such that you can properly bind the individual devices to the expected
drivers :)
Seriously, who makes such a broken fpga device that goes against the PCI
spec that way? Well, not so much as "goes against it", as "ignores all
of the proper ideas of the past 20 years for working with PCI devices".
thanks,
greg k-h
^ permalink raw reply
* [PATCH resend] serial: 8250: Add support for using platform_device resources
From: Esben Haabendal @ 2019-05-21 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, linux-serial
Cc: Lee Jones, Enrico Weigelt, Jiri Slaby, Andy Shevchenko,
Darwin Dingel, Jisheng Zhang, Sebastian Andrzej Siewior, He Zhe,
Marek Vasut, Douglas Anderson, Paul Burton, linux-kernel
In-Reply-To: <20190430140416.4707-1-esben@geanix.com>
Allow getting memory resource (mapbase or iobase) as well as irq from
platform_device resources.
The UPF_DEV_RESOURCES flag must be set for devices where platform_device
resources are to be used. When not set, driver behaves as before.
This allows use of the serial8250 driver together with devices with
resources added by platform_device_add_resources(), such as mfd child
devices added with mfd_add_devices().
When UPF_DEV_RESOURCES flag is set, the following platform_data fields should
not be used: mapbase, iobase, mapsize, and irq. They are superseded by the
resources attached to the device.
Signed-off-by: Esben Haabendal <esben@geanix.com>
---
drivers/tty/serial/8250/8250_core.c | 56 +++++++++++++++++++++++++++++++++----
drivers/tty/serial/8250/8250_port.c | 15 ++++++----
include/linux/serial_core.h | 1 +
3 files changed, 62 insertions(+), 10 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index e441221..9df6a98 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -788,6 +788,48 @@ void serial8250_resume_port(int line)
}
EXPORT_SYMBOL(serial8250_resume_port);
+static int serial8250_probe_resources(struct platform_device *pdev,
+ unsigned int num,
+ struct plat_serial8250_port *p,
+ struct uart_8250_port *uart)
+{
+ struct resource *r;
+ int irq;
+
+ switch (p->iotype) {
+ case UPIO_AU:
+ case UPIO_TSI:
+ case UPIO_MEM32:
+ case UPIO_MEM32BE:
+ case UPIO_MEM16:
+ case UPIO_MEM:
+ r = platform_get_resource(pdev, IORESOURCE_MEM, num);
+ if (!r)
+ return -ENODEV;
+ uart->port.mapbase = r->start;
+ uart->port.mapsize = resource_size(r);
+ uart->port.flags |= UPF_IOREMAP;
+ break;
+ case UPIO_HUB6:
+ case UPIO_PORT:
+ r = platform_get_resource(pdev, IORESOURCE_IO, num);
+ if (!r)
+ return -ENODEV;
+ uart->port.iobase = r->start;
+ uart->port.mapsize = resource_size(r);
+ break;
+ }
+
+ irq = platform_get_irq(pdev, num);
+ if (irq == -ENXIO)
+ uart->port.irq = 0; /* no interrupt -> use polling */
+ else if (irq < 0)
+ return irq;
+ uart->port.irq = irq;
+
+ return 0;
+}
+
/*
* Register a set of serial devices attached to a platform device. The
* list is terminated with a zero flags entry, which means we expect
@@ -805,15 +847,19 @@ static int serial8250_probe(struct platform_device *dev)
irqflag = IRQF_SHARED;
for (i = 0; p && p->flags != 0; p++, i++) {
- uart.port.iobase = p->iobase;
- uart.port.membase = p->membase;
- uart.port.irq = p->irq;
+ uart.port.flags = p->flags;
+ if (p->flags & UPF_DEV_RESOURCES) {
+ serial8250_probe_resources(dev, i, p, &uart);
+ } else {
+ uart.port.iobase = p->iobase;
+ uart.port.mapbase = p->mapbase;
+ uart.port.membase = p->membase;
+ uart.port.irq = p->irq;
+ }
uart.port.irqflags = p->irqflags;
uart.port.uartclk = p->uartclk;
uart.port.regshift = p->regshift;
uart.port.iotype = p->iotype;
- uart.port.flags = p->flags;
- uart.port.mapbase = p->mapbase;
uart.port.hub6 = p->hub6;
uart.port.private_data = p->private_data;
uart.port.type = p->type;
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index d2f3310..7fa1e49 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2863,7 +2863,8 @@ static int serial8250_request_std_resource(struct uart_8250_port *up)
if (!port->mapbase)
break;
- if (!request_mem_region(port->mapbase, size, "serial")) {
+ if (!(port->flags & UPF_DEV_RESOURCES) &&
+ !request_mem_region(port->mapbase, size, "serial")) {
ret = -EBUSY;
break;
}
@@ -2871,7 +2872,8 @@ static int serial8250_request_std_resource(struct uart_8250_port *up)
if (port->flags & UPF_IOREMAP) {
port->membase = ioremap_nocache(port->mapbase, size);
if (!port->membase) {
- release_mem_region(port->mapbase, size);
+ if (!(port->flags & UPF_DEV_RESOURCES))
+ release_mem_region(port->mapbase, size);
ret = -ENOMEM;
}
}
@@ -2879,7 +2881,8 @@ static int serial8250_request_std_resource(struct uart_8250_port *up)
case UPIO_HUB6:
case UPIO_PORT:
- if (!request_region(port->iobase, size, "serial"))
+ if (!(port->flags & UPF_DEV_RESOURCES) &&
+ !request_region(port->iobase, size, "serial"))
ret = -EBUSY;
break;
}
@@ -2906,12 +2909,14 @@ static void serial8250_release_std_resource(struct uart_8250_port *up)
port->membase = NULL;
}
- release_mem_region(port->mapbase, size);
+ if (!(port->flags & UPF_DEV_RESOURCES))
+ release_mem_region(port->mapbase, size);
break;
case UPIO_HUB6:
case UPIO_PORT:
- release_region(port->iobase, size);
+ if (!(port->flags & UPF_DEV_RESOURCES))
+ release_region(port->iobase, size);
break;
}
}
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 5fe2b03..87b4ed3 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -207,6 +207,7 @@ struct uart_port {
#define UPF_BUGGY_UART ((__force upf_t) ASYNC_BUGGY_UART /* 14 */ )
#define UPF_MAGIC_MULTIPLIER ((__force upf_t) ASYNC_MAGIC_MULTIPLIER /* 16 */ )
+#define UPF_DEV_RESOURCES ((__force upf_t) (1 << 18))
#define UPF_NO_THRE_TEST ((__force upf_t) (1 << 19))
/* Port has hardware-assisted h/w flow control */
#define UPF_AUTO_CTS ((__force upf_t) (1 << 20))
--
2.4.11
^ permalink raw reply related
* Re: [PATCH 2/2] serial: 8250: Add support for 8250/16550 as MFD function
From: Esben Haabendal @ 2019-05-21 11:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Lee Jones, linux-serial, Jiri Slaby, Nishanth Menon, Vignesh R,
Tony Lindgren, Lokesh Vutla, Florian Fainelli, linux-kernel
In-Reply-To: <20190521111817.GA24911@kroah.com>
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> On Tue, May 21, 2019 at 01:11:08PM +0200, Esben Haabendal wrote:
>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>>
>> >> I will try ad hold back with this thread until you get back to it.
>> >
>> > Ok, I have no idea what is going on here, sorry. This is a really long
>> > and meandering thread, and I can't even find the original patches in my
>> > queue.
>> >
>> > So can you resend things and we can start over? :)
>>
>> Will do.
>>
>> > But note, using a mfd for a uart seems VERY odd to me...
>>
>> Ok. In my case, I have a pcie card with an fpga which includes 5 uart
>> ports, 3 ethernet interfaces and a number of custom IP blocks.
>> I believe that an mfd driver for that pcie card in that case.
>
> I believe you need to fix that fpga to expose individual pci devices
> such that you can properly bind the individual devices to the expected
> drivers :)
Well, that is really out-of-scope of what I am doing here.
> Seriously, who makes such a broken fpga device that goes against the PCI
> spec that way? Well, not so much as "goes against it", as "ignores all
> of the proper ideas of the past 20 years for working with PCI devices".
Might be. But that is the firmware I have to work with here, and I
still hope we can find a good solution for implementing a driver without
having to maintain out-of-tree patches.
/Esben
^ permalink raw reply
* Re: [PATCH resend] serial: 8250: Add support for using platform_device resources
From: Andy Shevchenko @ 2019-05-21 12:42 UTC (permalink / raw)
To: Esben Haabendal
Cc: Greg Kroah-Hartman, linux-serial, Lee Jones, Enrico Weigelt,
Jiri Slaby, Darwin Dingel, Jisheng Zhang,
Sebastian Andrzej Siewior, He Zhe, Marek Vasut, Douglas Anderson,
Paul Burton, linux-kernel
In-Reply-To: <20190521113426.16790-1-esben@geanix.com>
On Tue, May 21, 2019 at 01:34:26PM +0200, Esben Haabendal wrote:
> Allow getting memory resource (mapbase or iobase) as well as irq from
> platform_device resources.
>
> The UPF_DEV_RESOURCES flag must be set for devices where platform_device
> resources are to be used. When not set, driver behaves as before.
>
> This allows use of the serial8250 driver together with devices with
> resources added by platform_device_add_resources(), such as mfd child
> devices added with mfd_add_devices().
>
> When UPF_DEV_RESOURCES flag is set, the following platform_data fields should
> not be used: mapbase, iobase, mapsize, and irq. They are superseded by the
> resources attached to the device.
>
Same comment here: Requesting resource is orthogonal to the retrieving or
slicing them.
> + if (p->flags & UPF_DEV_RESOURCES) {
> + serial8250_probe_resources(dev, i, p, &uart);
This can be easily detected by checking for the resources directly, like
res = platform_get_resource(...);
if (res)
new_scheme();
else
old_scheme();
Otherwise looks good.
> - if (!request_mem_region(port->mapbase, size, "serial")) {
> + if (!(port->flags & UPF_DEV_RESOURCES) &&
> + !request_mem_region(port->mapbase, size, "serial")) {
> - release_mem_region(port->mapbase, size);
> + if (!(port->flags & UPF_DEV_RESOURCES))
> + release_mem_region(port->mapbase, size);
> - if (!request_region(port->iobase, size, "serial"))
> + if (!(port->flags & UPF_DEV_RESOURCES) &&
> + !request_region(port->iobase, size, "serial"))
> - release_mem_region(port->mapbase, size);
> + if (!(port->flags & UPF_DEV_RESOURCES))
> + release_mem_region(port->mapbase, size);
> - release_region(port->iobase, size);
> + if (!(port->flags & UPF_DEV_RESOURCES))
> + release_region(port->iobase, size);
All these changes are not related to what you describe in the commit message.
is a workaround for the bug in the parent MFD driver of the 8250.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 2/2] serial: 8250: Add support for 8250/16550 as MFD function
From: Greg Kroah-Hartman @ 2019-05-21 12:56 UTC (permalink / raw)
To: Esben Haabendal
Cc: Lee Jones, linux-serial, Jiri Slaby, Nishanth Menon, Vignesh R,
Tony Lindgren, Lokesh Vutla, Florian Fainelli, linux-kernel
In-Reply-To: <87lfz0m3ge.fsf@haabendal.dk>
On Tue, May 21, 2019 at 01:50:25PM +0200, Esben Haabendal wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>
> > On Tue, May 21, 2019 at 01:11:08PM +0200, Esben Haabendal wrote:
> >> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >>
> >> >> I will try ad hold back with this thread until you get back to it.
> >> >
> >> > Ok, I have no idea what is going on here, sorry. This is a really long
> >> > and meandering thread, and I can't even find the original patches in my
> >> > queue.
> >> >
> >> > So can you resend things and we can start over? :)
> >>
> >> Will do.
> >>
> >> > But note, using a mfd for a uart seems VERY odd to me...
> >>
> >> Ok. In my case, I have a pcie card with an fpga which includes 5 uart
> >> ports, 3 ethernet interfaces and a number of custom IP blocks.
> >> I believe that an mfd driver for that pcie card in that case.
> >
> > I believe you need to fix that fpga to expose individual pci devices
> > such that you can properly bind the individual devices to the expected
> > drivers :)
>
> Well, that is really out-of-scope of what I am doing here.
Not really, if you have control over the fpga firmware (and odds are you
do), just fix that and instantly your device works with all kernels, no
need to change anything.
Why not do this?
> > Seriously, who makes such a broken fpga device that goes against the PCI
> > spec that way? Well, not so much as "goes against it", as "ignores all
> > of the proper ideas of the past 20 years for working with PCI devices".
>
> Might be. But that is the firmware I have to work with here, and I
> still hope we can find a good solution for implementing a driver without
> having to maintain out-of-tree patches.
As this hardware will not work on any operating system as-is, why not
fix the firmware to keep from having to support a one-off device that no
one else would be crazy enough to create? :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH resend] serial: 8250: Add support for using platform_device resources
From: Greg Kroah-Hartman @ 2019-05-21 13:11 UTC (permalink / raw)
To: Esben Haabendal
Cc: linux-serial, Lee Jones, Enrico Weigelt, Jiri Slaby,
Andy Shevchenko, Darwin Dingel, Jisheng Zhang,
Sebastian Andrzej Siewior, He Zhe, Marek Vasut, Douglas Anderson,
Paul Burton, linux-kernel
In-Reply-To: <20190521113426.16790-1-esben@geanix.com>
On Tue, May 21, 2019 at 01:34:26PM +0200, Esben Haabendal wrote:
> Allow getting memory resource (mapbase or iobase) as well as irq from
> platform_device resources.
>
> The UPF_DEV_RESOURCES flag must be set for devices where platform_device
> resources are to be used. When not set, driver behaves as before.
Nothing actually sets this flag in this patch, so I can't take this as
you are adding new features that no one uses :(
Where is the driver that sets this?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 4/4] serial: 8250-mtk: modify uart DMA rx
From: Vinod Koul @ 2019-05-21 13:57 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: <1556336193-15198-5-git-send-email-long.cheng@mediatek.com>
On 27-04-19, 11:36, Long Cheng wrote:
> Modify uart rx and complete for DMA.
>
> Signed-off-by: Long Cheng <long.cheng@mediatek.com>
> ---
> drivers/tty/serial/8250/8250_mtk.c | 53 ++++++++++++++++--------------------
> 1 file changed, 23 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> index c1fdbc0..04081a6 100644
> --- a/drivers/tty/serial/8250/8250_mtk.c
> +++ b/drivers/tty/serial/8250/8250_mtk.c
> @@ -30,7 +30,6 @@
> #define MTK_UART_DMA_EN_TX 0x2
> #define MTK_UART_DMA_EN_RX 0x5
>
> -#define MTK_UART_TX_SIZE UART_XMIT_SIZE
> #define MTK_UART_RX_SIZE 0x8000
> #define MTK_UART_TX_TRIGGER 1
> #define MTK_UART_RX_TRIGGER MTK_UART_RX_SIZE
> @@ -64,28 +63,30 @@ static void mtk8250_dma_rx_complete(void *param)
> struct mtk8250_data *data = up->port.private_data;
> struct tty_port *tty_port = &up->port.state->port;
> struct dma_tx_state state;
> + int copied, cnt, tmp;
> unsigned char *ptr;
> - int copied;
>
> - dma_sync_single_for_cpu(dma->rxchan->device->dev, dma->rx_addr,
> - dma->rx_size, DMA_FROM_DEVICE);
> + if (data->rx_status == DMA_RX_SHUTDOWN)
> + return;
>
> dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state);
> + cnt = dma->rx_size - state.residue;
> + tmp = cnt;
>
> - if (data->rx_status == DMA_RX_SHUTDOWN)
> - return;
> + if ((data->rx_pos + cnt) > dma->rx_size)
> + tmp = dma->rx_size - data->rx_pos;
>
> - if ((data->rx_pos + state.residue) <= dma->rx_size) {
> - ptr = (unsigned char *)(data->rx_pos + dma->rx_buf);
> - copied = tty_insert_flip_string(tty_port, ptr, state.residue);
> - } else {
> - ptr = (unsigned char *)(data->rx_pos + dma->rx_buf);
> - copied = tty_insert_flip_string(tty_port, ptr,
> - dma->rx_size - data->rx_pos);
> + ptr = (unsigned char *)(data->rx_pos + dma->rx_buf);
> + copied = tty_insert_flip_string(tty_port, ptr, tmp);
> + data->rx_pos += tmp;
> +
> + if (cnt > tmp) {
> ptr = (unsigned char *)(dma->rx_buf);
> - copied += tty_insert_flip_string(tty_port, ptr,
> - data->rx_pos + state.residue - dma->rx_size);
> + tmp = cnt - tmp;
> + copied += tty_insert_flip_string(tty_port, ptr, tmp);
> + data->rx_pos = tmp;
> }
> +
> up->port.icount.rx += copied;
>
> tty_flip_buffer_push(tty_port);
> @@ -96,9 +97,7 @@ static void mtk8250_dma_rx_complete(void *param)
> static void mtk8250_rx_dma(struct uart_8250_port *up)
> {
> struct uart_8250_dma *dma = up->dma;
> - struct mtk8250_data *data = up->port.private_data;
> struct dma_async_tx_descriptor *desc;
> - struct dma_tx_state state;
>
> desc = dmaengine_prep_slave_single(dma->rxchan, dma->rx_addr,
> dma->rx_size, DMA_DEV_TO_MEM,
> @@ -113,12 +112,6 @@ static void mtk8250_rx_dma(struct uart_8250_port *up)
>
> dma->rx_cookie = dmaengine_submit(desc);
>
> - dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state);
> - data->rx_pos = state.residue;
> -
> - dma_sync_single_for_device(dma->rxchan->device->dev, dma->rx_addr,
> - dma->rx_size, DMA_FROM_DEVICE);
> -
> dma_async_issue_pending(dma->rxchan);
> }
>
> @@ -131,13 +124,13 @@ static void mtk8250_dma_enable(struct uart_8250_port *up)
> if (data->rx_status != DMA_RX_START)
> return;
>
> - dma->rxconf.direction = DMA_DEV_TO_MEM;
> - dma->rxconf.src_addr_width = dma->rx_size / 1024;
> - dma->rxconf.src_addr = dma->rx_addr;
> + dma->rxconf.direction = DMA_DEV_TO_MEM;
Direction field is deprecated, please do not use this anymore...
> + dma->rxconf.src_port_window_size = dma->rx_size;
> + dma->rxconf.src_addr = dma->rx_addr;
>
> - dma->txconf.direction = DMA_MEM_TO_DEV;
> - dma->txconf.dst_addr_width = MTK_UART_TX_SIZE / 1024;
> - dma->txconf.dst_addr = dma->tx_addr;
> + dma->txconf.direction = DMA_MEM_TO_DEV;
> + dma->txconf.dst_port_window_size = UART_XMIT_SIZE;
> + dma->txconf.dst_addr = dma->tx_addr;
>
> serial_out(up, UART_FCR, UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR |
> UART_FCR_CLEAR_XMIT);
> @@ -217,7 +210,7 @@ static void mtk8250_shutdown(struct uart_port *port)
> * Mediatek UARTs use an extra highspeed register (UART_MTK_HIGHS)
> *
> * We need to recalcualte the quot register, as the claculation depends
> - * on the vaule in the highspeed register.
> + * on the value in the highspeed register.
> *
> * Some baudrates are not supported by the chip, so we use the next
> * lower rate supported and update termios c_flag.
> --
> 1.7.9.5
--
~Vinod
^ permalink raw reply
* Re: [PATCH 1/4] dmaengine: mediatek: Add MediaTek UART APDMA support
From: Vinod Koul @ 2019-05-21 14:00 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: <1556336193-15198-2-git-send-email-long.cheng@mediatek.com>
On 27-04-19, 11:36, Long Cheng wrote:
> Add 8250 UART APDMA to support MediaTek UART. If MediaTek UART is
> enabled by SERIAL_8250_MT6577, and we can enable this driver to offload
> the UART device moving bytes.
Applied, thanks
--
~Vinod
^ permalink raw reply
* Re: [PATCH 3/4] dt-bindings: dma: uart: rename binding
From: Vinod Koul @ 2019-05-21 14:01 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: <1556336193-15198-4-git-send-email-long.cheng@mediatek.com>
On 27-04-19, 11:36, Long Cheng wrote:
> The filename matches mtk-uart-apdma.c.
> So using "mtk-uart-apdma.txt" should be better.
> And add some property.
Applied with Robs r-b tag in last version, thanks
Also fixed a trailing line in patch :(
--
~Vinod
^ permalink raw reply
* Re: [PATCH 2/2] serial: 8250: Add support for 8250/16550 as MFD function
From: Esben Haabendal @ 2019-05-21 14:31 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Lee Jones, linux-serial, Jiri Slaby, Nishanth Menon, Vignesh R,
Tony Lindgren, Lokesh Vutla, Florian Fainelli, linux-kernel
In-Reply-To: <20190521125651.GA6264@kroah.com>
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> On Tue, May 21, 2019 at 01:50:25PM +0200, Esben Haabendal wrote:
>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>>
>> > On Tue, May 21, 2019 at 01:11:08PM +0200, Esben Haabendal wrote:
>> >> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> >>
>> >> >> I will try ad hold back with this thread until you get back to it.
>> >> >
>> >> > Ok, I have no idea what is going on here, sorry. This is a really long
>> >> > and meandering thread, and I can't even find the original patches in my
>> >> > queue.
>> >> >
>> >> > So can you resend things and we can start over? :)
>> >>
>> >> Will do.
>> >>
>> >> > But note, using a mfd for a uart seems VERY odd to me...
>> >>
>> >> Ok. In my case, I have a pcie card with an fpga which includes 5 uart
>> >> ports, 3 ethernet interfaces and a number of custom IP blocks.
>> >> I believe that an mfd driver for that pcie card in that case.
>> >
>> > I believe you need to fix that fpga to expose individual pci devices
>> > such that you can properly bind the individual devices to the expected
>> > drivers :)
>>
>> Well, that is really out-of-scope of what I am doing here.
>
> Not really, if you have control over the fpga firmware (and odds are you
> do), just fix that and instantly your device works with all kernels, no
> need to change anything.
>
> Why not do this?
Because I do not have control over fpga firmware.
>> > Seriously, who makes such a broken fpga device that goes against the PCI
>> > spec that way? Well, not so much as "goes against it", as "ignores all
>> > of the proper ideas of the past 20 years for working with PCI devices".
>>
>> Might be. But that is the firmware I have to work with here, and I
>> still hope we can find a good solution for implementing a driver without
>> having to maintain out-of-tree patches.
>
> As this hardware will not work on any operating system as-is, why not
> fix the firmware to keep from having to support a one-off device that no
> one else would be crazy enough to create? :)
Clearly, someone has been crazy enough. Hopefully, we can be smart
enough to make Linux fit to it.
/Esben
^ permalink raw reply
* Re: [PATCH resend] serial: 8250: Add support for using platform_device resources
From: Esben Haabendal @ 2019-05-21 14:43 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, linux-serial, Lee Jones, Enrico Weigelt,
Jiri Slaby, Darwin Dingel, Jisheng Zhang,
Sebastian Andrzej Siewior, He Zhe, Marek Vasut, Douglas Anderson,
Paul Burton, linux-kernel
In-Reply-To: <20190521124202.GE9224@smile.fi.intel.com>
Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:
> On Tue, May 21, 2019 at 01:34:26PM +0200, Esben Haabendal wrote:
>> Allow getting memory resource (mapbase or iobase) as well as irq from
>> platform_device resources.
>>
>> The UPF_DEV_RESOURCES flag must be set for devices where platform_device
>> resources are to be used. When not set, driver behaves as before.
>>
>> This allows use of the serial8250 driver together with devices with
>> resources added by platform_device_add_resources(), such as mfd child
>> devices added with mfd_add_devices().
>>
>> When UPF_DEV_RESOURCES flag is set, the following platform_data fields should
>> not be used: mapbase, iobase, mapsize, and irq. They are superseded by the
>> resources attached to the device.
>>
>
> Same comment here: Requesting resource is orthogonal to the retrieving or
> slicing them.
Yes. But for MFD devices, I do think it makes sense for the MFD parent
device to request the entire memory resource, and then split it.
And for drivers that actually are aware of the struct resource given,
both approaches work. Throwing away the resource.parent information
and calling out request_mem_region() manually breaks the idea of
managing IORESOURCE_MEM as a tree structure.
Are we not supposed to be using the parent/child part of struct
resource?
>> + if (p->flags & UPF_DEV_RESOURCES) {
>> + serial8250_probe_resources(dev, i, p, &uart);
>
> This can be easily detected by checking for the resources directly, like
>
> res = platform_get_resource(...);
> if (res)
> new_scheme();
> else
> old_scheme();
>
> Otherwise looks good.
Sounds fine with me. I was afraid that it could cause problems with
existing drivers, where platform_get_resource() would work, but return
something else than desired. That would probably have gone unnoticed by
now. But can ofcourse be fixed if it occurs.
>> - if (!request_mem_region(port->mapbase, size, "serial")) {
>> + if (!(port->flags & UPF_DEV_RESOURCES) &&
>> + !request_mem_region(port->mapbase, size, "serial")) {
>
>> - release_mem_region(port->mapbase, size);
>> + if (!(port->flags & UPF_DEV_RESOURCES))
>> + release_mem_region(port->mapbase, size);
>
>> - if (!request_region(port->iobase, size, "serial"))
>> + if (!(port->flags & UPF_DEV_RESOURCES) &&
>> + !request_region(port->iobase, size, "serial"))
>
>> - release_mem_region(port->mapbase, size);
>> + if (!(port->flags & UPF_DEV_RESOURCES))
>> + release_mem_region(port->mapbase, size);
>
>> - release_region(port->iobase, size);
>> + if (!(port->flags & UPF_DEV_RESOURCES))
>> + release_region(port->iobase, size);
>
> All these changes are not related to what you describe in the commit message.
> is a workaround for the bug in the parent MFD driver of the 8250.
You are right, this is not adequately described in commit message.
But unless we are not supposed to allow parent/child memory resource
management, I don't think it is a workaround, but a fix.
But I can split it out in a separate patch. Would be nice if I at least
can get the other part of the change merged.
/Esben
^ permalink raw reply
* Re: [PATCH 2/2] serial: 8250: Add support for 8250/16550 as MFD function
From: Greg Kroah-Hartman @ 2019-05-21 14:43 UTC (permalink / raw)
To: Esben Haabendal
Cc: Lee Jones, linux-serial, Jiri Slaby, Nishanth Menon, Vignesh R,
Tony Lindgren, Lokesh Vutla, Florian Fainelli, linux-kernel
In-Reply-To: <87h89nnajr.fsf@haabendal.dk>
On Tue, May 21, 2019 at 04:31:52PM +0200, Esben Haabendal wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>
> > On Tue, May 21, 2019 at 01:50:25PM +0200, Esben Haabendal wrote:
> >> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >>
> >> > On Tue, May 21, 2019 at 01:11:08PM +0200, Esben Haabendal wrote:
> >> >> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >> >>
> >> >> >> I will try ad hold back with this thread until you get back to it.
> >> >> >
> >> >> > Ok, I have no idea what is going on here, sorry. This is a really long
> >> >> > and meandering thread, and I can't even find the original patches in my
> >> >> > queue.
> >> >> >
> >> >> > So can you resend things and we can start over? :)
> >> >>
> >> >> Will do.
> >> >>
> >> >> > But note, using a mfd for a uart seems VERY odd to me...
> >> >>
> >> >> Ok. In my case, I have a pcie card with an fpga which includes 5 uart
> >> >> ports, 3 ethernet interfaces and a number of custom IP blocks.
> >> >> I believe that an mfd driver for that pcie card in that case.
> >> >
> >> > I believe you need to fix that fpga to expose individual pci devices
> >> > such that you can properly bind the individual devices to the expected
> >> > drivers :)
> >>
> >> Well, that is really out-of-scope of what I am doing here.
> >
> > Not really, if you have control over the fpga firmware (and odds are you
> > do), just fix that and instantly your device works with all kernels, no
> > need to change anything.
> >
> > Why not do this?
>
> Because I do not have control over fpga firmware.
Who does? Why did they create it this way if it can not be accessed by
an operating system as-is? Has it passed the PCI tests? Do you have a
link to where you can get this crazy device?
> >> > Seriously, who makes such a broken fpga device that goes against the PCI
> >> > spec that way? Well, not so much as "goes against it", as "ignores all
> >> > of the proper ideas of the past 20 years for working with PCI devices".
> >>
> >> Might be. But that is the firmware I have to work with here, and I
> >> still hope we can find a good solution for implementing a driver without
> >> having to maintain out-of-tree patches.
> >
> > As this hardware will not work on any operating system as-is, why not
> > fix the firmware to keep from having to support a one-off device that no
> > one else would be crazy enough to create? :)
>
> Clearly, someone has been crazy enough. Hopefully, we can be smart
> enough to make Linux fit to it.
Sometimes you need to go tell the hardware/firmware people not to do
foolish things. You can not always fix their problems in software.
Please push back on this.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH resend] serial: 8250: Add support for using platform_device resources
From: Esben Haabendal @ 2019-05-21 14:45 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-serial, Lee Jones, Enrico Weigelt, Jiri Slaby,
Andy Shevchenko, Darwin Dingel, Jisheng Zhang,
Sebastian Andrzej Siewior, He Zhe, Marek Vasut, Douglas Anderson,
Paul Burton, linux-kernel
In-Reply-To: <20190521131131.GA19685@kroah.com>
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> On Tue, May 21, 2019 at 01:34:26PM +0200, Esben Haabendal wrote:
>> Allow getting memory resource (mapbase or iobase) as well as irq from
>> platform_device resources.
>>
>> The UPF_DEV_RESOURCES flag must be set for devices where platform_device
>> resources are to be used. When not set, driver behaves as before.
>
> Nothing actually sets this flag in this patch, so I can't take this as
> you are adding new features that no one uses :(
>
> Where is the driver that sets this?
It sits here. It is a rather big and clunky mfd driver, not ready for
upstreaming in its current form. I hope to get around to clean it up.
But it is for a very specific hardware that is really available or
usable for anybody else. Does it make sense to spend effort on
submitting such a driver?
/Esben
^ 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