* RE: Linux USB Serial
From: Steven J. Ackerman @ 2012-09-17 22:27 UTC (permalink / raw)
To: 'Greg KH'; +Cc: 'Bjørn Mork', linux-usb, linux-serial
In-Reply-To: <20120917220813.GA7780@kroah.com>
> That's fine, the cdc-acm interface will work for that.
>
> But is your device really a CDC ACM device? Or is it something else?
The device is a 3.5" color LCD display with touchscreen that can act as a
serial
terminal. It just needs to be able to send and receive characters over an
USB
serial port.
> You wouldn't, if you use the ftdi serial chip in your device. Which
> begs me to ask, what type of usb to serial chip is in your device? What
> protocol does it use to talk to the host?
The device is based upon a Renesas RX62N processor which has a USB interface
in it. The protocol is supposedly based upon the CDC - a control endpoint
and
two bulk endpoints. The software was derived from a demo supplied by the
manufacturer - and it does work under Windows. The device enumerates as
a COM port and you can interact with the display using a program or a
terminal
emulator. Unfortunately the chip vendor doesn't have any support for Linux.
> They shouldn't have to do that, once we get it working, they get the
> update automatically from their distro. But if you can't test any
> changes we make to try to get this to work, there's not much we can do
> here, right?
>
I guess that I'm operating under the assumption that there is something
wrong on my end - an incorrectly configured descriptor for example. It
appears
the other USB serial devices work OK under Linux - even without custom
drivers. I'm hoping that I can find an error message somewhere that will
tell me what I'm doing wrong - or that somebody who has been down that
path before can't point to something obvious - like Bjrn did.
> Oh, and fix that descriptor up in your firmware, that might solve
> everything :)
Going through the USB CDC documentation between e-mails. Other than
the incorrect bInterfaceSubClass value - which I have corrected - nothing
else seems wrong.
>
> Hope this helps,
>
> greg k-h
Always appreciate your assistance - thank you!
Steven J. Ackerman, Consultant
ACS, Sarasota, FL
http://www.acscontrol.com
mailto:steve@acscontrol.com
^ permalink raw reply
* Re: Linux USB Serial
From: Oliver Neukum @ 2012-09-18 6:46 UTC (permalink / raw)
To: Steven J. Ackerman; +Cc: 'Bjørn Mork', linux-usb, linux-serial
In-Reply-To: <00b801cd9518$715dd160$54197420$@acscontrol.com>
On Monday 17 September 2012 17:07:26 Steven J. Ackerman wrote:
> Bjrn-
>
> Thank you for your response.
>
> This change gets me closer. I can now successfully execute the modprobe
> without error, but the device still doesn't show up in /dev/ttyUSB? .
It shouldn't. Your device follows the CDC ACM specification, aside from
the incorrect subclass. Such devices don't generate /dev/ttyUSB devices
nodes. They generate /dev/ttyACM nodes.
> sja@UBUNTU-10:~$ sudo modprobe usbserial vendor=0x0c6a product=0x0005
That is the wrong driver. usbserial is for vendor specific serial devices.
Your device follows a class specification. You need cdc_acm. As soon
as the subclass is fixed, it should autoload.
If cdc_acm doesn't bind, please post "dmesg".
Regards
Oliver
^ permalink raw reply
* [PATCH 1/2] serial: imx: set sport as drvdata, like it's used elsewhere
From: Richard Zhao @ 2012-09-18 8:14 UTC (permalink / raw)
To: linux-serial; +Cc: alan, gregkh, shawn.guo, kernel, Richard Zhao
Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
drivers/tty/serial/imx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 5952b25..49f664f 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1540,7 +1540,7 @@ static int serial_imx_probe(struct platform_device *pdev)
ret = uart_add_one_port(&imx_reg, &sport->port);
if (ret)
goto deinit;
- platform_set_drvdata(pdev, &sport->port);
+ platform_set_drvdata(pdev, sport);
return 0;
deinit:
--
1.7.9.5
^ permalink raw reply related
* [PATCH 2/2] serial: imx: remove null check of sport in suspend/resume function
From: Richard Zhao @ 2012-09-18 8:14 UTC (permalink / raw)
To: linux-serial; +Cc: alan, gregkh, shawn.guo, kernel, Richard Zhao
In-Reply-To: <1347956099-15257-1-git-send-email-richard.zhao@freescale.com>
platform_get_drvdata always retrun a valid value after probe succeed.
It also fixed smatch warnings:
drivers/tty/serial/imx.c:1376 serial_imx_suspend() warn: variable dereferenced before check 'sport' (see line 1372)
drivers/tty/serial/imx.c:1392 serial_imx_resume() warn: variable dereferenced before check 'sport' (see line 1388)
Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
drivers/tty/serial/imx.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 49f664f..efeb8be 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1373,8 +1373,7 @@ static int serial_imx_suspend(struct platform_device *dev, pm_message_t state)
val |= UCR3_AWAKEN;
writel(val, sport->port.membase + UCR3);
- if (sport)
- uart_suspend_port(&imx_reg, &sport->port);
+ uart_suspend_port(&imx_reg, &sport->port);
return 0;
}
@@ -1389,8 +1388,7 @@ static int serial_imx_resume(struct platform_device *dev)
val &= ~UCR3_AWAKEN;
writel(val, sport->port.membase + UCR3);
- if (sport)
- uart_resume_port(&imx_reg, &sport->port);
+ uart_resume_port(&imx_reg, &sport->port);
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH] serial: omap: Remove unnecessary checks from suspend/resume
From: Sourav Poddar @ 2012-09-18 11:35 UTC (permalink / raw)
To: gregkh
Cc: alan, tony, khilman, linux-omap, linux-serial, linux-arm-kernel,
balbi, santosh.shilimkar, Sourav Poddar
Drop the check for "up" being valid on suspend/resume callbacks.
It should be valid always. Get rid of the "pdata" check also as
serial_omap_get_context_loss_count() checks for it.
Tested on omap4 panda and 3630 based Beagle board.
Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
---
drivers/tty/serial/omap-serial.c | 23 +++++++++--------------
1 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index f175385..3c05c5e 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1222,10 +1222,8 @@ static int serial_omap_suspend(struct device *dev)
{
struct uart_omap_port *up = dev_get_drvdata(dev);
- if (up) {
- uart_suspend_port(&serial_omap_reg, &up->port);
- flush_work_sync(&up->qos_work);
- }
+ uart_suspend_port(&serial_omap_reg, &up->port);
+ flush_work_sync(&up->qos_work);
return 0;
}
@@ -1234,8 +1232,8 @@ static int serial_omap_resume(struct device *dev)
{
struct uart_omap_port *up = dev_get_drvdata(dev);
- if (up)
- uart_resume_port(&serial_omap_reg, &up->port);
+ uart_resume_port(&serial_omap_reg, &up->port);
+
return 0;
}
#endif
@@ -1553,17 +1551,14 @@ static int serial_omap_runtime_suspend(struct device *dev)
static int serial_omap_runtime_resume(struct device *dev)
{
struct uart_omap_port *up = dev_get_drvdata(dev);
- struct omap_uart_port_info *pdata = dev->platform_data;
- if (up && pdata) {
- u32 loss_cnt = serial_omap_get_context_loss_count(up);
+ u32 loss_cnt = serial_omap_get_context_loss_count(up);
- if (up->context_loss_cnt != loss_cnt)
- serial_omap_restore_context(up);
+ if (up->context_loss_cnt != loss_cnt)
+ serial_omap_restore_context(up);
- up->latency = up->calc_latency;
- schedule_work(&up->qos_work);
- }
+ up->latency = up->calc_latency;
+ schedule_work(&up->qos_work);
return 0;
}
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] serial: omap: Remove unnecessary checks from suspend/resume
From: Felipe Balbi @ 2012-09-18 11:34 UTC (permalink / raw)
To: Sourav Poddar
Cc: gregkh, alan, tony, khilman, linux-omap, linux-serial,
linux-arm-kernel, balbi, santosh.shilimkar
In-Reply-To: <1347968154-27997-1-git-send-email-sourav.poddar@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2321 bytes --]
On Tue, Sep 18, 2012 at 05:05:54PM +0530, Sourav Poddar wrote:
> Drop the check for "up" being valid on suspend/resume callbacks.
> It should be valid always. Get rid of the "pdata" check also as
> serial_omap_get_context_loss_count() checks for it.
>
> Tested on omap4 panda and 3630 based Beagle board.
you need a blank line here. Other than that:
Reviewed-by: Felipe Balbi <balbi@ti.com>
ps: what kind of tests did you run, btw ?
> Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
> ---
> drivers/tty/serial/omap-serial.c | 23 +++++++++--------------
> 1 files changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index f175385..3c05c5e 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -1222,10 +1222,8 @@ static int serial_omap_suspend(struct device *dev)
> {
> struct uart_omap_port *up = dev_get_drvdata(dev);
>
> - if (up) {
> - uart_suspend_port(&serial_omap_reg, &up->port);
> - flush_work_sync(&up->qos_work);
> - }
> + uart_suspend_port(&serial_omap_reg, &up->port);
> + flush_work_sync(&up->qos_work);
>
> return 0;
> }
> @@ -1234,8 +1232,8 @@ static int serial_omap_resume(struct device *dev)
> {
> struct uart_omap_port *up = dev_get_drvdata(dev);
>
> - if (up)
> - uart_resume_port(&serial_omap_reg, &up->port);
> + uart_resume_port(&serial_omap_reg, &up->port);
> +
> return 0;
> }
> #endif
> @@ -1553,17 +1551,14 @@ static int serial_omap_runtime_suspend(struct device *dev)
> static int serial_omap_runtime_resume(struct device *dev)
> {
> struct uart_omap_port *up = dev_get_drvdata(dev);
> - struct omap_uart_port_info *pdata = dev->platform_data;
>
> - if (up && pdata) {
> - u32 loss_cnt = serial_omap_get_context_loss_count(up);
> + u32 loss_cnt = serial_omap_get_context_loss_count(up);
>
> - if (up->context_loss_cnt != loss_cnt)
> - serial_omap_restore_context(up);
> + if (up->context_loss_cnt != loss_cnt)
> + serial_omap_restore_context(up);
>
> - up->latency = up->calc_latency;
> - schedule_work(&up->qos_work);
> - }
> + up->latency = up->calc_latency;
> + schedule_work(&up->qos_work);
>
> return 0;
> }
> --
> 1.7.1
>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] serial: omap: Remove unnecessary checks from suspend/resume
From: Poddar, Sourav @ 2012-09-18 11:44 UTC (permalink / raw)
To: balbi
Cc: gregkh, alan, tony, khilman, linux-omap, linux-serial,
linux-arm-kernel, santosh.shilimkar
In-Reply-To: <20120918113444.GG24047@arwen.pp.htv.fi>
Hi Felipe,
On Tue, Sep 18, 2012 at 5:04 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Tue, Sep 18, 2012 at 05:05:54PM +0530, Sourav Poddar wrote:
>> Drop the check for "up" being valid on suspend/resume callbacks.
>> It should be valid always. Get rid of the "pdata" check also as
>> serial_omap_get_context_loss_count() checks for it.
>>
>> Tested on omap4 panda and 3630 based Beagle board.
>
> you need a blank line here. Other than that:
>
> Reviewed-by: Felipe Balbi <balbi@ti.com>
>
> ps: what kind of tests did you run, btw ?
>
Boot tested on Panda.
Boot and PM tested(hitiing Off) on Beagle.
>> Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
>> ---
>> drivers/tty/serial/omap-serial.c | 23 +++++++++--------------
>> 1 files changed, 9 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
>> index f175385..3c05c5e 100644
>> --- a/drivers/tty/serial/omap-serial.c
>> +++ b/drivers/tty/serial/omap-serial.c
>> @@ -1222,10 +1222,8 @@ static int serial_omap_suspend(struct device *dev)
>> {
>> struct uart_omap_port *up = dev_get_drvdata(dev);
>>
>> - if (up) {
>> - uart_suspend_port(&serial_omap_reg, &up->port);
>> - flush_work_sync(&up->qos_work);
>> - }
>> + uart_suspend_port(&serial_omap_reg, &up->port);
>> + flush_work_sync(&up->qos_work);
>>
>> return 0;
>> }
>> @@ -1234,8 +1232,8 @@ static int serial_omap_resume(struct device *dev)
>> {
>> struct uart_omap_port *up = dev_get_drvdata(dev);
>>
>> - if (up)
>> - uart_resume_port(&serial_omap_reg, &up->port);
>> + uart_resume_port(&serial_omap_reg, &up->port);
>> +
>> return 0;
>> }
>> #endif
>> @@ -1553,17 +1551,14 @@ static int serial_omap_runtime_suspend(struct device *dev)
>> static int serial_omap_runtime_resume(struct device *dev)
>> {
>> struct uart_omap_port *up = dev_get_drvdata(dev);
>> - struct omap_uart_port_info *pdata = dev->platform_data;
>>
>> - if (up && pdata) {
>> - u32 loss_cnt = serial_omap_get_context_loss_count(up);
>> + u32 loss_cnt = serial_omap_get_context_loss_count(up);
>>
>> - if (up->context_loss_cnt != loss_cnt)
>> - serial_omap_restore_context(up);
>> + if (up->context_loss_cnt != loss_cnt)
>> + serial_omap_restore_context(up);
>>
>> - up->latency = up->calc_latency;
>> - schedule_work(&up->qos_work);
>> - }
>> + up->latency = up->calc_latency;
>> + schedule_work(&up->qos_work);
>>
>> return 0;
>> }
>> --
>> 1.7.1
>>
>
> --
> balbi
^ permalink raw reply
* [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
From: Sourav Poddar @ 2012-09-18 12:40 UTC (permalink / raw)
To: gregkh
Cc: alan, tony, khilman, linux-omap, linux-arm-kernel, linux-serial,
linux-kernel, santosh.shilimkar, balbi, paul, Sourav Poddar
Greg's tty-next is not booting on 2420 based N800. The failure is
observed at serial init itself. The reason might be that n800 tries to
resume even though it is not suspended before.
Reported-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
---
This patch is developed on top of greg's tty-next branch
CommitId: e740d8f tty: serial: Samsung: Fix return value +
the following patch which I have already posted to the mailing list.
http://comments.gmane.org/gmane.linux.ports.arm.omap/84729
drivers/tty/serial/omap-serial.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 3c05c5e..bc355f2 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -110,6 +110,7 @@ struct uart_omap_port {
u32 calc_latency;
struct work_struct qos_work;
struct pinctrl *pins;
+ unsigned int suspended:1;
};
#define to_uart_omap_port(p) ((container_of((p), struct uart_omap_port, port)))
@@ -1545,14 +1546,20 @@ static int serial_omap_runtime_suspend(struct device *dev)
up->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
schedule_work(&up->qos_work);
+ up->suspended = true;
+
return 0;
}
static int serial_omap_runtime_resume(struct device *dev)
{
struct uart_omap_port *up = dev_get_drvdata(dev);
+ u32 loss_cnt;
+
+ if (!up->suspended)
+ return 0;
- u32 loss_cnt = serial_omap_get_context_loss_count(up);
+ loss_cnt = serial_omap_get_context_loss_count(up);
if (up->context_loss_cnt != loss_cnt)
serial_omap_restore_context(up);
@@ -1560,6 +1567,8 @@ static int serial_omap_runtime_resume(struct device *dev)
up->latency = up->calc_latency;
schedule_work(&up->qos_work);
+ up->suspended = false;
+
return 0;
}
#endif
--
1.7.1
^ permalink raw reply related
* RE: Linux USB Serial
From: Steven J. Ackerman @ 2012-09-18 14:00 UTC (permalink / raw)
To: 'Oliver Neukum'
Cc: 'Bjørn Mork', linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5789366.GGq3fAeCyr-ugxBuEnWX9yG/4A2pS7c2Q@public.gmane.org>
> It shouldn't. Your device follows the CDC ACM specification, aside from
> the incorrect subclass. Such devices don't generate /dev/ttyUSB devices
> nodes. They generate /dev/ttyACM nodes.
>
Thank you. I did some more research on this last night and found out that
I was being mis-lead by observing how other USB Serial devices were
working and used under Linux - without having to install a driver.
Apparently
these were all using the FTDI chip - and there is a driver that is already
installed for them.
> That is the wrong driver. usbserial is for vendor specific serial devices.
> Your device follows a class specification. You need cdc_acm. As soon
> as the subclass is fixed, it should autoload.
>
Yes - the driver now autoloads and I can communicate with the device
using the /dev/ttyACM0 node. I was able to install picocom and talk to and
from our display.
Thanks to everyone who replied to this question for your assistance.
Steven J. Ackerman, Consultant
ACS, Sarasota, FL
http://www.acscontrol.com
mailto:steve-I01wfHm1r9xGyKhy+UYuEA@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
From: Felipe Balbi @ 2012-09-18 14:02 UTC (permalink / raw)
To: Sourav Poddar
Cc: gregkh, alan, tony, khilman, linux-omap, linux-arm-kernel,
linux-serial, linux-kernel, santosh.shilimkar, balbi, paul
In-Reply-To: <1347972050-3509-1-git-send-email-sourav.poddar@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2666 bytes --]
On Tue, Sep 18, 2012 at 06:10:50PM +0530, Sourav Poddar wrote:
> Greg's tty-next is not booting on 2420 based N800. The failure is
> observed at serial init itself. The reason might be that n800 tries to
> resume even though it is not suspended before.
>
> Reported-by: Paul Walmsley <paul@pwsan.com>
> Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
FWIW:
Reviewed-by: Felipe Balbi <balbi@ti.com>
Paul does this fix the issue for you ? Note that it depends on a
previous patch Sourav sent [1]
[1] http://marc.info/?l=linux-omap&m=134796819607889&w=2
There's one thing that gets my attention, though, why only n800 would
fail here ?
I wonder if we should be using:
pm_runtime_set_active(dev);
pm_runtime_get_enable(dev);
to prevent our runtime_resume() to be called from probe, but Sourav
tested and it doesn't work on BeagleBoard, but it works on PandaBoard.
Does it ring any bell ??
> ---
> This patch is developed on top of greg's tty-next branch
> CommitId: e740d8f tty: serial: Samsung: Fix return value +
> the following patch which I have already posted to the mailing list.
> http://comments.gmane.org/gmane.linux.ports.arm.omap/84729
>
> drivers/tty/serial/omap-serial.c | 11 ++++++++++-
> 1 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index 3c05c5e..bc355f2 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -110,6 +110,7 @@ struct uart_omap_port {
> u32 calc_latency;
> struct work_struct qos_work;
> struct pinctrl *pins;
> + unsigned int suspended:1;
> };
>
> #define to_uart_omap_port(p) ((container_of((p), struct uart_omap_port, port)))
> @@ -1545,14 +1546,20 @@ static int serial_omap_runtime_suspend(struct device *dev)
> up->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
> schedule_work(&up->qos_work);
>
> + up->suspended = true;
> +
> return 0;
> }
>
> static int serial_omap_runtime_resume(struct device *dev)
> {
> struct uart_omap_port *up = dev_get_drvdata(dev);
> + u32 loss_cnt;
> +
> + if (!up->suspended)
> + return 0;
>
> - u32 loss_cnt = serial_omap_get_context_loss_count(up);
> + loss_cnt = serial_omap_get_context_loss_count(up);
>
> if (up->context_loss_cnt != loss_cnt)
> serial_omap_restore_context(up);
> @@ -1560,6 +1567,8 @@ static int serial_omap_runtime_resume(struct device *dev)
> up->latency = up->calc_latency;
> schedule_work(&up->qos_work);
>
> + up->suspended = false;
> +
> return 0;
> }
> #endif
> --
> 1.7.1
>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH] 8250: fix autoconfig to work with serial console
From: Flavio Leitner @ 2012-09-18 19:21 UTC (permalink / raw)
To: linux-serial, linux-kernel; +Cc: Flavio Leitner
The autoconfig prints messages while holding the
port's spinlock and that causes a deadlock when
using serial console.
Signed-off-by: Flavio Leitner <fbl@redhat.com>
---
drivers/tty/serial/8250/8250.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index 8123f78..17e0c3f 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -1037,6 +1037,7 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
unsigned char save_lcr, save_mcr;
struct uart_port *port = &up->port;
unsigned long flags;
+ unsigned int old_capabilities;
if (!port->iobase && !port->mapbase && !port->membase)
return;
@@ -1087,6 +1088,7 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
/*
* We failed; there's nothing here
*/
+ spin_unlock_irqrestore(&port->lock, flags);
DEBUG_AUTOCONF("IER test failed (%02x, %02x) ",
scratch2, scratch3);
goto out;
@@ -1110,6 +1112,7 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
status1 = serial_in(up, UART_MSR) & 0xF0;
serial_out(up, UART_MCR, save_mcr);
if (status1 != 0x90) {
+ spin_unlock_irqrestore(&port->lock, flags);
DEBUG_AUTOCONF("LOOP test failed (%02x) ",
status1);
goto out;
@@ -1132,8 +1135,6 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
serial_out(up, UART_FCR, UART_FCR_ENABLE_FIFO);
scratch = serial_in(up, UART_IIR) >> 6;
- DEBUG_AUTOCONF("iir=%d ", scratch);
-
switch (scratch) {
case 0:
autoconfig_8250(up);
@@ -1167,19 +1168,13 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
serial_out(up, UART_LCR, save_lcr);
- if (up->capabilities != uart_config[port->type].flags) {
- printk(KERN_WARNING
- "ttyS%d: detected caps %08x should be %08x\n",
- serial_index(port), up->capabilities,
- uart_config[port->type].flags);
- }
-
port->fifosize = uart_config[up->port.type].fifo_size;
+ old_capabilities = up->capabilities;
up->capabilities = uart_config[port->type].flags;
up->tx_loadsz = uart_config[port->type].tx_loadsz;
if (port->type == PORT_UNKNOWN)
- goto out;
+ goto out_lock;
/*
* Reset the UART.
@@ -1196,8 +1191,16 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
else
serial_out(up, UART_IER, 0);
- out:
+out_lock:
spin_unlock_irqrestore(&port->lock, flags);
+ if (up->capabilities != old_capabilities) {
+ printk(KERN_WARNING
+ "ttyS%d: detected caps %08x should be %08x\n",
+ serial_index(port), old_capabilities,
+ up->capabilities);
+ }
+out:
+ DEBUG_AUTOCONF("iir=%d ", scratch);
DEBUG_AUTOCONF("type=%s\n", uart_config[port->type].name);
}
--
1.7.11.4
^ permalink raw reply related
* Top secret, please treat as urgent
From: Mohan Das @ 2012-09-18 22:11 UTC (permalink / raw)
Dear friend,
I am Mr Mohan Das a staff of Banque Internationale du Burkina (B.I.B)
here in Burkina Faso / Ouagadougou . In my department I discover an
abandoned sum of (US$18.5 Million US Dollars) in an account that
belongs to one of our foreign customer who died along with his family
in plane crash.
It is therefore upon this discovery that i now decided to make this
business proposal to you and release the money to you as the next of
kin or relation to the deceased for the safety and subsequent
disbursement since nobody is coming for it.
I agree that 40% of this money will be for you, and 10% will be set
aside for expenses incurred during the business and 50% would be for
me. Then after the money is been transferred into your account, i will
visit your country for an investment under your kind control.
You have to contact my Bank directly as the real next of kin of this
deceased account with next of kin application form which i will send
to you immediately i hear from you.
I am waiting for your urgent respond to enable us proceed further for
the transfer.
Yours faithfully
Mr Mohan Das
^ permalink raw reply
* Top secret, please treat as urgent
From: Mohan Das @ 2012-09-18 22:20 UTC (permalink / raw)
Dear friend,
I am Mr Mohan Das a staff of Banque Internationale du Burkina (B.I.B)
here in Burkina Faso / Ouagadougou . In my department I discover an
abandoned sum of (US$18.5 Million US Dollars) in an account that
belongs to one of our foreign customer who died along with his family
in plane crash.
It is therefore upon this discovery that i now decided to make this
business proposal to you and release the money to you as the next of
kin or relation to the deceased for the safety and subsequent
disbursement since nobody is coming for it.
I agree that 40% of this money will be for you, and 10% will be set
aside for expenses incurred during the business and 50% would be for
me. Then after the money is been transferred into your account, i will
visit your country for an investment under your kind control.
You have to contact my Bank directly as the real next of kin of this
deceased account with next of kin application form which i will send
to you immediately i hear from you.
I am waiting for your urgent respond to enable us proceed further for
the transfer.
Yours faithfully
Mr Mohan Das
^ permalink raw reply
* Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
From: Paul Walmsley @ 2012-09-18 22:57 UTC (permalink / raw)
To: Felipe Balbi, khilman
Cc: Sourav Poddar, gregkh, alan, tony, linux-omap, linux-arm-kernel,
linux-serial, linux-kernel, santosh.shilimkar
In-Reply-To: <20120918140213.GA27463@arwen.pp.htv.fi>
On Tue, 18 Sep 2012, Felipe Balbi wrote:
> On Tue, Sep 18, 2012 at 06:10:50PM +0530, Sourav Poddar wrote:
> > Greg's tty-next is not booting on 2420 based N800. The failure is
> > observed at serial init itself. The reason might be that n800 tries to
> > resume even though it is not suspended before.
> >
> > Reported-by: Paul Walmsley <paul@pwsan.com>
> > Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
>
> FWIW:
>
> Reviewed-by: Felipe Balbi <balbi@ti.com>
>
> Paul does this fix the issue for you ? Note that it depends on a
> previous patch Sourav sent [1]
>
> [1] http://marc.info/?l=linux-omap&m=134796819607889&w=2
Yes, thanks - that fixes it,
Tested-by: Paul Walmsley <paul@pwsan.com>
> There's one thing that gets my attention, though, why only n800 would
> fail here ?
>
> I wonder if we should be using:
>
> pm_runtime_set_active(dev);
> pm_runtime_get_enable(dev);
>
> to prevent our runtime_resume() to be called from probe, but Sourav
> tested and it doesn't work on BeagleBoard, but it works on PandaBoard.
>
> Does it ring any bell ??
Nothing off the top of my head but haven't looked into it -- maybe this
rings a bell with Kevin?
- Paul
^ permalink raw reply
* Re: [PATCH 1/2] serial: imx: set sport as drvdata, like it's used elsewhere
From: Shawn Guo @ 2012-09-19 3:36 UTC (permalink / raw)
To: Richard Zhao; +Cc: linux-serial, alan, gregkh, kernel
In-Reply-To: <1347956099-15257-1-git-send-email-richard.zhao@freescale.com>
On Tue, Sep 18, 2012 at 04:14:58PM +0800, Richard Zhao wrote:
> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
I think it's worth to have a sensible commit log explaining why it
has not trigger any issue.
It happens to work without problem because "struct uart_port port" is
the first member of "struct imx_port".
Otherwise,
Acked-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> drivers/tty/serial/imx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 5952b25..49f664f 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -1540,7 +1540,7 @@ static int serial_imx_probe(struct platform_device *pdev)
> ret = uart_add_one_port(&imx_reg, &sport->port);
> if (ret)
> goto deinit;
> - platform_set_drvdata(pdev, &sport->port);
> + platform_set_drvdata(pdev, sport);
>
> return 0;
> deinit:
> --
> 1.7.9.5
>
>
^ permalink raw reply
* Re: [PATCH 2/2] serial: imx: remove null check of sport in suspend/resume function
From: Shawn Guo @ 2012-09-19 3:37 UTC (permalink / raw)
To: Richard Zhao; +Cc: linux-serial, alan, gregkh, kernel
In-Reply-To: <1347956099-15257-2-git-send-email-richard.zhao@freescale.com>
On Tue, Sep 18, 2012 at 04:14:59PM +0800, Richard Zhao wrote:
> platform_get_drvdata always retrun a valid value after probe succeed.
>
> It also fixed smatch warnings:
> drivers/tty/serial/imx.c:1376 serial_imx_suspend() warn: variable dereferenced before check 'sport' (see line 1372)
> drivers/tty/serial/imx.c:1392 serial_imx_resume() warn: variable dereferenced before check 'sport' (see line 1388)
>
> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
Acked-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> drivers/tty/serial/imx.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 49f664f..efeb8be 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -1373,8 +1373,7 @@ static int serial_imx_suspend(struct platform_device *dev, pm_message_t state)
> val |= UCR3_AWAKEN;
> writel(val, sport->port.membase + UCR3);
>
> - if (sport)
> - uart_suspend_port(&imx_reg, &sport->port);
> + uart_suspend_port(&imx_reg, &sport->port);
>
> return 0;
> }
> @@ -1389,8 +1388,7 @@ static int serial_imx_resume(struct platform_device *dev)
> val &= ~UCR3_AWAKEN;
> writel(val, sport->port.membase + UCR3);
>
> - if (sport)
> - uart_resume_port(&imx_reg, &sport->port);
> + uart_resume_port(&imx_reg, &sport->port);
>
> return 0;
> }
> --
> 1.7.9.5
>
>
^ permalink raw reply
* Re: [PATCH 02/11] kdb: Implement disable_nmi command
From: Jason Wessel @ 2012-09-19 11:52 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Andrew Morton, Russell King, Greg Kroah-Hartman, Alan Cox,
Arve Hjønnevåg, Colin Cross, Brian Swetland,
John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
linaro-kernel, patches, kernel-team, kgdb-bugreport, linux-serial
In-Reply-To: <1347548615-18227-2-git-send-email-anton.vorontsov@linaro.org>
On 09/13/2012 10:03 AM, Anton Vorontsov wrote:
> This command disables NMI-entry. If NMI source has been previously shared
> with a serial console ("debug port"), this effectively releases the port
> from KDB exclusive use, and makes the console available for normal use.
>
> Of course, NMI can be reenabled, enable_nmi modparam is used for that:
>
> echo 1 > /sys/module/kdb/parameters/enable_nmi
>
> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
> ---
> kernel/debug/kdb/kdb_main.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index 31df170..9fadff1 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -21,6 +21,7 @@
> #include <linux/smp.h>
> #include <linux/utsname.h>
> #include <linux/vmalloc.h>
> +#include <linux/atomic.h>
> #include <linux/module.h>
> #include <linux/mm.h>
> #include <linux/init.h>
> @@ -2107,6 +2108,32 @@ static int kdb_dmesg(int argc, const char **argv)
> return 0;
> }
> #endif /* CONFIG_PRINTK */
> +
> +/* Make sure we balance enable/disable calls, must disable first. */
> +static atomic_t kdb_nmi_disabled;
> +
> +static int kdb_disable_nmi(int argc, const char *argv[])
> +{
> + if (atomic_read(&kdb_nmi_disabled))
> + return 0;
> + atomic_set(&kdb_nmi_disabled, 1);
> + kgdb_enable_nmi(0);
> + return 0;
> +}
> +
> +static int kdb_param_enable_nmi(const char *val, const struct kernel_param *kp)
> +{
> + if (!atomic_add_unless(&kdb_nmi_disabled, -1, 0))
> + return -EINVAL;
> + kgdb_enable_nmi(1);
> + return 0;
> +}
> +
> +static const struct kernel_param_ops kdb_param_ops_enable_nmi = {
> + .set = kdb_param_enable_nmi,
> +};
> +module_param_cb(enable_nmi, &kdb_param_ops_enable_nmi, NULL, 0600);
> +
> /*
> * kdb_cpu - This function implements the 'cpu' command.
> * cpu [<cpunum>]
> @@ -2851,6 +2878,8 @@ static void __init kdb_inittab(void)
> kdb_register_repeat("dmesg", kdb_dmesg, "[lines]",
> "Display syslog buffer", 0, KDB_REPEAT_NONE);
> #endif
Based on what I commented in the 01/11 patch, we don't want to register a command if the architecture doesn't actually have it, because it just leads to confusion.
It needs to have an "if"
if (arch_kgdb_ops->enable_nmi) {
> + kdb_register_repeat("disable_nmi", kdb_disable_nmi, "",
> + "Disable NMI entry to KDB", 0, KDB_REPEAT_NONE);
}
> kdb_register_repeat("defcmd", kdb_defcmd, "name \"usage\" \"help\"",
> "Define a set of commands, down to endefcmd", 0, KDB_REPEAT_NONE);
> kdb_register_repeat("kill", kdb_kill, "<-signal> <pid>",
>
^ permalink raw reply
* Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
From: Grazvydas Ignotas @ 2012-09-19 11:52 UTC (permalink / raw)
To: balbi
Cc: Sourav Poddar, gregkh, alan, tony, khilman, linux-omap,
linux-arm-kernel, linux-serial, linux-kernel, santosh.shilimkar,
paul
In-Reply-To: <20120918140213.GA27463@arwen.pp.htv.fi>
On Tue, Sep 18, 2012 at 5:02 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Tue, Sep 18, 2012 at 06:10:50PM +0530, Sourav Poddar wrote:
>> Greg's tty-next is not booting on 2420 based N800. The failure is
>> observed at serial init itself. The reason might be that n800 tries to
>> resume even though it is not suspended before.
>>
>> Reported-by: Paul Walmsley <paul@pwsan.com>
>> Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
>
> FWIW:
>
> Reviewed-by: Felipe Balbi <balbi@ti.com>
>
> Paul does this fix the issue for you ? Note that it depends on a
> previous patch Sourav sent [1]
>
> [1] http://marc.info/?l=linux-omap&m=134796819607889&w=2
>
> There's one thing that gets my attention, though, why only n800 would
> fail here ?
>
> I wonder if we should be using:
>
> pm_runtime_set_active(dev);
> pm_runtime_get_enable(dev);
>
> to prevent our runtime_resume() to be called from probe, but Sourav
> tested and it doesn't work on BeagleBoard, but it works on PandaBoard.
>
> Does it ring any bell ??
Well I guess it's normal resume callback is called during probe as
pm_runtime_get_sync() is called there, and according to documentation
[1], devices are assumed to be suspended before probe is called.
According to [1], pm_runtime_set_active() should be called before
pm_runtime_enable() to indicate to the core that device is active
during probe if that's the case, I guess this means
pm_runtime_get_sync() is not needed in that case, but I'm not sure
what's the actual serial state during probe nowadays.
[1] chapter 5 of Documentation/power/runtime_pm.txt:
The initial runtime PM status of all devices is 'suspended', but it
need not reflect the actual physical state of the device. Thus, if the
device is initially active (i.e. it is able to process I/O), its
runtime PM status must be changed to 'active', with the help of
pm_runtime_set_active(), before pm_runtime_enable() is called for the
device.
--
Gražvydas
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 01/11] kernel/debug: Mask KGDB NMI upon entry
From: Jason Wessel @ 2012-09-19 11:52 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Andrew Morton, Russell King, Greg Kroah-Hartman, Alan Cox,
Arve Hjønnevåg, Colin Cross, Brian Swetland,
John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
linaro-kernel, patches, kernel-team, kgdb-bugreport, linux-serial
In-Reply-To: <1347548615-18227-1-git-send-email-anton.vorontsov@linaro.org>
On 09/13/2012 10:03 AM, Anton Vorontsov wrote:
> The new arch callback should manage NMIs that usually cause KGDB to
> enter. That is, not all NMIs should be enabled/disabled, but only
> those that issue kgdb_handle_exception().
>
> We must mask it as serial-line interrupt can be used as an NMI, so
> if the original KGDB-entry cause was say a breakpoint, then every
> input to KDB console will cause KGDB to reenter, which we don't want.
>
> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
> ---
> include/linux/kgdb.h | 23 +++++++++++++++++++++++
> kernel/debug/debug_core.c | 36 +++++++++++++++++++++++++++++++++---
> 2 files changed, 56 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
> index c4d2fc1..3b111a6 100644
> --- a/include/linux/kgdb.h
> +++ b/include/linux/kgdb.h
> @@ -221,6 +221,29 @@ extern int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt);
> */
> extern void kgdb_arch_late(void);
>
> +/**
> + * kgdb_arch_enable_nmi - Enable or disable KGDB-entry NMI
> + * @on: Flag to either enable or disable an NMI
> + *
> + * This is an architecture-specific "back-end" for kgdb_enable_nmi(). The
> + * call does not count disable/enable requests, do not use it directly.
> + */
> +extern void kgdb_arch_enable_nmi(bool on);
I realize this is more of a clean up, and this doesn't necessarily
have to gate the acceptance of these patches to the mainline, but I
would like to not see us using the kgdb_arch_enable_nmi() as a weak
function. This belongs in:
struct kgdb_arch {
...
void enable_nmi(bool on);
}
> +
> +/**
> + * kgdb_enable_nmi - Enable or disable KGDB-entry NMI
> + * @on: Flag to either enable or disable an NMI
> + *
> + * This function manages NMIs that usually cause KGDB to enter. That is,
> + * not all NMIs should be enabled or disabled, but only those that issue
> + * kgdb_handle_exception().
> + *
> + * The call counts disable requests, and thus allows to nest disables.
> + * But trying to enable already enabled NMI is an error. The call returns
> + * 1 if NMI has been actually enabled after the call, and a value <= 0 if
> + * it is still disabled.
> + */
> +extern int kgdb_enable_nmi(bool on);
Clearly you need something arch specific here. With respect to the
atomic math, it is not clear that would be the case for this function
unilaterally accross other architectures. This comment block and the
"implementation" code belong in the arch stub: arch/kernel/arm/kgdb.c
The whole kgdb_enable_nmi can go away if using the struct kgdb_arch
callbacks
> --- a/kernel/debug/debug_core.c
> +++ b/kernel/debug/debug_core.c
> @@ -214,6 +214,30 @@ int __weak kgdb_skipexception(int exception, struct pt_regs *regs)
> return 0;
> }
>
> +void __weak kgdb_arch_enable_nmi(bool on)
> +{
> +}
> +
> +int kgdb_enable_nmi(bool on)
> +{
> + static atomic_t cnt;
> + int ret;
> +
> + ret = atomic_add_return(on ? 1 : -1, &cnt);
> + if (ret > 1 && on) {
> + /*
> + * There should be only one instance that calls this function
> + * in "enable, disable" order. All other users must call
> + * disable first, then enable. If not, something is wrong.
> + */
> + WARN_ON(1);
> + return 1;
> + }
> +
> + kgdb_arch_enable_nmi(ret > 0);
> + return ret;
> +}
> +
> /*
> * Some architectures need cache flushes when we set/clear a
> * breakpoint:
> @@ -672,6 +696,9 @@ kgdb_handle_exception(int evector, int signo, int ecode, struct pt_regs *regs)
> {
> struct kgdb_state kgdb_var;
> struct kgdb_state *ks = &kgdb_var;
> + int ret = 0;
> +
> + kgdb_enable_nmi(0);
With what I mentioned before this becomes:
if (arch_kgdb_ops->enable_nmi)
arch_kgdb_ops->enable_nmi(0)
>
> ks->cpu = raw_smp_processor_id();
> ks->ex_vector = evector;
> @@ -681,11 +708,14 @@ kgdb_handle_exception(int evector, int signo, int ecode, struct pt_regs *regs)
> ks->linux_regs = regs;
>
> if (kgdb_reenter_check(ks))
> - return 0; /* Ouch, double exception ! */
> + goto out; /* Ouch, double exception ! */
> if (kgdb_info[ks->cpu].enter_kgdb != 0)
> - return 0;
> + goto out;
>
> - return kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
> + ret = kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
> +out:
> + kgdb_enable_nmi(1);
Becomes:
if (arch_kgdb_ops->enable_nmi)
arch_kgdb_ops->enable_nmi(1)
> + return ret;
> }
>
> int kgdb_nmicallback(int cpu, void *regs)
>
Cheers,
Jason.
^ permalink raw reply
* Re: [PATCH 07/11] tty/serial: Add kgdb_nmi driver
From: Jason Wessel @ 2012-09-19 11:52 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Andrew Morton, Russell King, Greg Kroah-Hartman, Alan Cox,
Arve Hjønnevåg, Colin Cross, Brian Swetland,
John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
linaro-kernel, patches, kernel-team, kgdb-bugreport, linux-serial
In-Reply-To: <1347548615-18227-7-git-send-email-anton.vorontsov@linaro.org>
On 09/13/2012 10:03 AM, Anton Vorontsov wrote:
> This special driver makes it possible to temporary use NMI debugger port
> as a normal console by issuing 'nmi_console' command (assuming that the
> port is attached to KGDB).
>
> Unlike KDB's disable_nmi command, with this driver you are always able
> to go back to the debugger using KGDB escape sequence ($3#33). This is
> because this console driver processes the input in NMI context, and thus
> is able to intercept the magic sequence.
>
> Note that since the console interprets input and uses polling
> communication methods, for things like PPP it is still better to fully
> detach debugger port from the KGDB NMI (i.e. disable_nmi), and use raw
> console.
>
> Usually, to enter the debugger one have to type the magic sequence, so
> initially the kernel will print the following prompt on the NMI debugger
> console:
>
> Type $3#33 to enter the debugger>
>
> For convenience, there is a kgdb_fiq.knock kernel command line option,
> when set to 0, this turns the special command to just a return key
> press, so the kernel will be printing this:
>
> Hit <return> to enter the debugger>
>
> This is more convenient for long debugging sessions, although it makes
> nmi_console feature somewhat useless.
>
> And for the cases when NMI connected to a dedicated button, the knocking
> can be disabled altogether by setting kgdb_fiq.knock to -1.
>
> Suggested-by: Colin Cross <ccross@android.com>
> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
> ---
> drivers/tty/serial/Kconfig | 19 ++
> drivers/tty/serial/Makefile | 1 +
> drivers/tty/serial/kgdb_nmi.c | 396 ++++++++++++++++++++++++++++++++++++++++++
> drivers/tty/serial/kgdboc.c | 6 +
> include/linux/kgdb.h | 10 ++
> 5 files changed, 432 insertions(+)
> create mode 100644 drivers/tty/serial/kgdb_nmi.c
>
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 26907cf..b22e45b 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -141,6 +141,25 @@ config SERIAL_ATMEL_TTYAT
>
> Say Y if you have an external 8250/16C550 UART. If unsure, say N.
>
> +config SERIAL_KGDB_NMI
> + bool "Serial console over KGDB NMI debugger port"
> + depends on KGDB_SERIAL_CONSOLE
> + help
> + This special driver allows you to temporary use NMI debugger port
> + as a normal console (assuming that the port is attached to KGDB).
> +
> + Unlike KDB's disable_nmi command, with this driver you are always
> + able to go back to the debugger using KGDB escape sequence ($3#33).
> + This is because this console driver processes the input in NMI
> + context, and thus is able to intercept the magic sequence.
> +
> + Note that since the console interprets input and uses polling
> + communication methods, for things like PPP you still must fully
> + detach debugger port from the KGDB NMI (i.e. disable_nmi), and
> + use raw console.
> +
> + If unsure, say N.
> +
> config SERIAL_KS8695
> bool "Micrel KS8695 (Centaur) serial port support"
> depends on ARCH_KS8695
> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
> index ce88667..4f694da 100644
> --- a/drivers/tty/serial/Makefile
> +++ b/drivers/tty/serial/Makefile
> @@ -61,6 +61,7 @@ obj-$(CONFIG_SERIAL_MSM_HS) += msm_serial_hs.o
> obj-$(CONFIG_SERIAL_NETX) += netx-serial.o
> obj-$(CONFIG_SERIAL_OF_PLATFORM) += of_serial.o
> obj-$(CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL) += nwpserial.o
> +obj-$(CONFIG_SERIAL_KGDB_NMI) += kgdb_nmi.o
> obj-$(CONFIG_SERIAL_KS8695) += serial_ks8695.o
> obj-$(CONFIG_SERIAL_OMAP) += omap-serial.o
> obj-$(CONFIG_SERIAL_ALTERA_UART) += altera_uart.o
> diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c
> new file mode 100644
> index 0000000..57bf744
> --- /dev/null
> +++ b/drivers/tty/serial/kgdb_nmi.c
> @@ -0,0 +1,396 @@
> +/*
> + * KGDB NMI serial console
> + *
> + * Copyright 2010 Google, Inc.
> + * Arve Hjønnevåg <arve@android.com>
> + * Colin Cross <ccross@android.com>
> + * Copyright 2012 Linaro Ltd.
> + * Anton Vorontsov <anton.vorontsov@linaro.org>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> + * by the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/compiler.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/errno.h>
> +#include <linux/atomic.h>
> +#include <linux/console.h>
> +#include <linux/tty.h>
> +#include <linux/tty_driver.h>
> +#include <linux/tty_flip.h>
> +#include <linux/interrupt.h>
> +#include <linux/hrtimer.h>
> +#include <linux/tick.h>
> +#include <linux/kfifo.h>
> +#include <linux/kgdb.h>
> +#include <linux/kdb.h>
> +
> +static int kgdb_nmi_knock = 1;
> +module_param_named(knock, kgdb_nmi_knock, int, 0600);
> +MODULE_PARM_DESC(knock, "if set to 1 (default), the special '$3#33' command "
> + "must be used to enter the debugger; when set to 0, "
> + "hitting return key is enough to enter the debugger; "
> + "when set to -1, the debugger is entered immediately "
> + "upon NMI");
> +
> +static char *kgdb_nmi_magic = "$3#33";
> +module_param_named(magic, kgdb_nmi_magic, charp, 0600);
> +MODULE_PARM_DESC(magic, "magic sequence to enter NMI debugger (default $3#33)");
> +
> +static bool kgdb_nmi_tty_enabled;
> +
> +static void kgdb_nmi_console_write(struct console *co, const char *s, uint c)
> +{
> + int i;
> +
> + if (!kgdb_nmi_tty_enabled || atomic_read(&kgdb_active) >= 0)
> + return;
> +
> + for (i = 0; i < c; i++)
> + dbg_io_ops->write_char(s[i]);
> +}
> +
> +static struct tty_driver *kgdb_nmi_tty_driver;
> +
> +static struct tty_driver *kgdb_nmi_console_device(struct console *co, int *idx)
> +{
> + *idx = co->index;
> + return kgdb_nmi_tty_driver;
> +}
> +
> +static struct console kgdb_nmi_console = {
> + .name = "ttyNMI",
> + .write = kgdb_nmi_console_write,
> + .device = kgdb_nmi_console_device,
> + .flags = CON_PRINTBUFFER | CON_ANYTIME | CON_ENABLED,
> + .index = -1,
> +};
> +
> +/*
> + * This is usually the maximum rate on debug ports. We make fifo large enough
> + * to make copy-pasting to the terminal usable.
> + */
> +#define KGDB_NMI_BAUD 115200
> +#define KGDB_NMI_FIFO_SIZE roundup_pow_of_two(KGDB_NMI_BAUD / 8 / HZ)
> +
> +struct kgdb_nmi_tty_priv {
> + struct tty_port port;
> + struct tasklet_struct tlet;
> + STRUCT_KFIFO(char, KGDB_NMI_FIFO_SIZE) fifo;
> +};
> +
> +static struct kgdb_nmi_tty_priv *kgdb_nmi_port_to_priv(struct tty_port *port)
> +{
> + return container_of(port, struct kgdb_nmi_tty_priv, port);
> +}
> +
> +/*
> + * Our debugging console is polled in a tasklet, so we'll check for input
> + * every tick. In HZ-less mode, we should program the next tick. We have
> + * to use the lowlevel stuff as no locks should be grabbed.
> + */
> +#ifdef CONFIG_HIGH_RES_TIMERS
> +static void kgdb_tty_poke(void)
> +{
> + tick_program_event(ktime_get(), 0);
> +}
> +#else
> +static inline void kgdb_tty_poke(void) {}
> +#endif
> +
> +static struct tty_port *kgdb_nmi_port;
> +
> +static void kgdb_tty_recv(int ch)
> +{
> + struct kgdb_nmi_tty_priv *priv;
> + char c = ch;
> +
> + if (!kgdb_nmi_port || ch < 0)
> + return;
> + /*
> + * Can't use port->tty->driver_data as tty might be not there. Tasklet
> + * will check for tty and will get the ref, but here we don't have to
> + * do that, and actually, we can't: we're in NMI context, no locks are
> + * possible.
> + */
> + priv = kgdb_nmi_port_to_priv(kgdb_nmi_port);
> + kfifo_in(&priv->fifo, &c, 1);
> + kgdb_tty_poke();
> +}
> +
> +static int kgdb_nmi_poll_one_knock(void)
> +{
> + static int n;
> + int c = -1;
> + const char *magic = kgdb_nmi_magic;
> + size_t m = strlen(magic);
> + bool printch = 0;
> +
> + c = dbg_io_ops->read_char();
> + if (c == NO_POLL_CHAR)
> + return c;
> +
> + if (!kgdb_nmi_knock && (c == '\r' || c == '\n')) {
> + return 1;
> + } else if (c == magic[n]) {
> + n = (n + 1) % m;
> + if (!n)
> + return 1;
> + printch = 1;
> + } else {
> + n = 0;
> + }
> +
> + if (kgdb_nmi_tty_enabled) {
> + kgdb_tty_recv(c);
> + return 0;
> + }
> +
> + if (printch) {
> + kdb_printf("%c", c);
> + return 0;
> + }
> +
> + kdb_printf("\r%s %s to enter the debugger> %*s",
> + kgdb_nmi_knock ? "Type" : "Hit",
> + kgdb_nmi_knock ? magic : "<return>", m, "");
> + while (m--)
> + kdb_printf("\b");
> + return 0;
> +}
> +
> +/**
> + * kgdb_nmi_poll_knock - Check if it is time to enter the debugger
> + *
> + * "Serial ports are often noisy, especially when muxed over another port (we
> + * often use serial over the headset connector). Noise on the async command
> + * line just causes characters that are ignored, on a command line that blocked
> + * execution noise would be catastrophic." -- Colin Cross
> + *
> + * So, this function implements KGDB/KDB knocking on the serial line: we won't
> + * enter the debugger until we receive a known magic phrase (which is actually
> + * "$3#33", known as "escape to KDB" command. There is also a relaxed variant
> + * of knocking, i.e. just pressing the return key is enough to enter the
> + * debugger. And if knocking is disabled, the function always returns 1.
> + */
> +bool kgdb_nmi_poll_knock(void)
> +{
> + if (kgdb_nmi_knock < 0)
> + return 1;
> +
> + while (1) {
> + int ret;
> +
> + ret = kgdb_nmi_poll_one_knock();
> + if (ret == NO_POLL_CHAR)
> + return 0;
> + else if (ret == 1)
> + break;
> + }
> + return 1;
> +}
> +
> +/*
> + * The tasklet is cheap, it does not cause wakeups when reschedules itself,
> + * instead it waits for the next tick.
> + */
> +static void kgdb_nmi_tty_receiver(unsigned long data)
> +{
> + struct kgdb_nmi_tty_priv *priv = (void *)data;
> + struct tty_struct *tty;
> + char ch;
> +
> + tasklet_schedule(&priv->tlet);
> +
> + if (likely(!kgdb_nmi_tty_enabled || !kfifo_len(&priv->fifo)))
> + return;
> +
> + /* Port is there, but tty might be hung up, check. */
> + tty = tty_port_tty_get(kgdb_nmi_port);
> + if (!tty)
> + return;
> +
> + while (kfifo_out(&priv->fifo, &ch, 1))
> + tty_insert_flip_char(priv->port.tty, ch, TTY_NORMAL);
> + tty_flip_buffer_push(priv->port.tty);
> +
> + tty_kref_put(tty);
> +}
> +
> +static int kgdb_nmi_tty_activate(struct tty_port *port, struct tty_struct *tty)
> +{
> + struct kgdb_nmi_tty_priv *priv = tty->driver_data;
> +
> + kgdb_nmi_port = port;
> + tasklet_schedule(&priv->tlet);
> + return 0;
> +}
> +
> +static void kgdb_nmi_tty_shutdown(struct tty_port *port)
> +{
> + struct kgdb_nmi_tty_priv *priv = port->tty->driver_data;
> +
> + tasklet_kill(&priv->tlet);
> + kgdb_nmi_port = NULL;
> +}
> +
> +static const struct tty_port_operations kgdb_nmi_tty_port_ops = {
> + .activate = kgdb_nmi_tty_activate,
> + .shutdown = kgdb_nmi_tty_shutdown,
> +};
> +
> +static int kgdb_nmi_tty_install(struct tty_driver *drv, struct tty_struct *tty)
> +{
> + struct kgdb_nmi_tty_priv *priv;
> + int ret;
> +
> + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + INIT_KFIFO(priv->fifo);
> + tasklet_init(&priv->tlet, kgdb_nmi_tty_receiver, (unsigned long)priv);
> + tty_port_init(&priv->port);
> + priv->port.ops = &kgdb_nmi_tty_port_ops;
> + tty->driver_data = priv;
> +
> + ret = tty_port_install(&priv->port, drv, tty);
> + if (ret) {
> + pr_err("%s: can't nstall tty port: %d\n", __func__, ret);
> + goto err;
> + }
> + return 0;
> +err:
> + kfree(priv);
> + return ret;
> +}
> +
> +static void kgdb_nmi_tty_cleanup(struct tty_struct *tty)
> +{
> + struct kgdb_nmi_tty_priv *priv = tty->driver_data;
> +
> + tty->driver_data = NULL;
> + kfree(priv);
> +}
> +
> +static int kgdb_nmi_tty_open(struct tty_struct *tty, struct file *file)
> +{
> + struct kgdb_nmi_tty_priv *priv = tty->driver_data;
> +
> + return tty_port_open(&priv->port, tty, file);
> +}
> +
> +static void kgdb_nmi_tty_close(struct tty_struct *tty, struct file *file)
> +{
> + struct kgdb_nmi_tty_priv *priv = tty->driver_data;
> +
> + tty_port_close(&priv->port, tty, file);
> +}
> +
> +static void kgdb_nmi_tty_hangup(struct tty_struct *tty)
> +{
> + struct kgdb_nmi_tty_priv *priv = tty->driver_data;
> +
> + tty_port_hangup(&priv->port);
> +}
> +
> +static int kgdb_nmi_tty_write_room(struct tty_struct *tty)
> +{
> + /* Actually, we can handle any amount as we use polled writes. */
> + return 2048;
> +}
> +
> +static int kgdb_nmi_tty_write(struct tty_struct *tty, const unchar *buf, int c)
> +{
> + int i;
> +
> + for (i = 0; i < c; i++)
> + dbg_io_ops->write_char(buf[i]);
> + return c;
> +}
> +
> +static const struct tty_operations kgdb_nmi_tty_ops = {
> + .open = kgdb_nmi_tty_open,
> + .close = kgdb_nmi_tty_close,
> + .install = kgdb_nmi_tty_install,
> + .cleanup = kgdb_nmi_tty_cleanup,
> + .hangup = kgdb_nmi_tty_hangup,
> + .write_room = kgdb_nmi_tty_write_room,
> + .write = kgdb_nmi_tty_write,
> +};
> +
> +static int kgdb_nmi_enable_console(int argc, const char *argv[])
> +{
> + kgdb_nmi_tty_enabled = !(argc == 1 && !strcmp(argv[1], "off"));
> + return 0;
> +}
> +
> +int kgdb_register_nmi_console(void)
> +{
> + int ret;
> +
> + kgdb_nmi_tty_driver = alloc_tty_driver(1);
> + if (!kgdb_nmi_tty_driver) {
> + pr_err("%s: cannot allocate tty\n", __func__);
> + return -ENOMEM;
> + }
> + kgdb_nmi_tty_driver->driver_name = "ttyNMI";
> + kgdb_nmi_tty_driver->name = "ttyNMI";
> + kgdb_nmi_tty_driver->num = 1;
> + kgdb_nmi_tty_driver->type = TTY_DRIVER_TYPE_SERIAL;
> + kgdb_nmi_tty_driver->subtype = SERIAL_TYPE_NORMAL;
> + kgdb_nmi_tty_driver->flags = TTY_DRIVER_REAL_RAW;
> + kgdb_nmi_tty_driver->init_termios = tty_std_termios;
> + tty_termios_encode_baud_rate(&kgdb_nmi_tty_driver->init_termios,
> + KGDB_NMI_BAUD, KGDB_NMI_BAUD);
> + tty_set_operations(kgdb_nmi_tty_driver, &kgdb_nmi_tty_ops);
> +
> + ret = tty_register_driver(kgdb_nmi_tty_driver);
> + if (ret) {
> + pr_err("%s: can't register tty driver: %d\n", __func__, ret);
> + goto err_drv_reg;
> + }
> +
> + ret = kdb_register("nmi_console", kgdb_nmi_enable_console, "[off]",
> + "switch to Linux NMI console", 0);
> + if (ret) {
> + pr_err("%s: can't register kdb command: %d\n", __func__, ret);
> + goto err_kdb_reg;
> + }
> +
> + register_console(&kgdb_nmi_console);
> + kgdb_enable_nmi(1);
> +
> + return 0;
> +err_kdb_reg:
> + tty_unregister_driver(kgdb_nmi_tty_driver);
> +err_drv_reg:
> + put_tty_driver(kgdb_nmi_tty_driver);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(kgdb_register_nmi_console);
> +
> +int kgdb_unregister_nmi_console(void)
> +{
> + int ret;
> +
> + kgdb_enable_nmi(0);
> + kdb_unregister("nmi_console");
> +
> + ret = unregister_console(&kgdb_nmi_console);
> + if (ret)
> + return ret;
> +
> + ret = tty_unregister_driver(kgdb_nmi_tty_driver);
> + if (ret)
> + return ret;
> + put_tty_driver(kgdb_nmi_tty_driver);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(kgdb_unregister_nmi_console);
> diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
> index 2b42a01..ed97cfd 100644
> --- a/drivers/tty/serial/kgdboc.c
> +++ b/drivers/tty/serial/kgdboc.c
> @@ -145,6 +145,8 @@ __setup("kgdboc=", kgdboc_option_setup);
>
> static void cleanup_kgdboc(void)
> {
> + if (kgdb_unregister_nmi_console())
> + return;
> kgdboc_unregister_kbd();
> if (configured == 1)
> kgdb_unregister_io_module(&kgdboc_io_ops);
> @@ -198,6 +200,10 @@ do_register:
> if (err)
> goto noconfig;
>
> + err = kgdb_register_nmi_console();
> + if (err)
> + goto noconfig;
> +
Just one question on this one, what do you expect to happen, or how does this work when you use a different serial port for example on a board that has a real serial port and an FIQ port? It was not obvious that there was a path to check if the port you are registering is an FIQ enabled port and then call the kgdb_register_nmi_console at that point.
Jason.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger
From: Jason Wessel @ 2012-09-19 11:57 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Anton Vorontsov, Andrew Morton, Russell King, Alan Cox,
Arve Hjønnevåg, Colin Cross, Brian Swetland,
John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
linaro-kernel, patches, kernel-team, kgdb-bugreport, linux-serial
In-Reply-To: <20120917155408.GA302@kroah.com>
On 09/17/2012 10:54 AM, Greg Kroah-Hartman wrote:
> On Mon, Sep 17, 2012 at 08:39:43AM -0700, Anton Vorontsov wrote:
>> On Mon, Sep 17, 2012 at 05:54:27AM -0700, Greg Kroah-Hartman wrote:
>> [...]
>>>>> To make it easier, can I just take the tty driver patches now? Will
>>>>> that break anything?
>>>> I have not made my way through the entire series yet, so I am not sure
>>>> if you need the kdb header changes or not, but so far it looks like
>>>> the tty pieces are separate. If you add your ack Greg, I'll take the
>>>> whole series and merge it into kgdb-next, or after I finish the review
>>>> we could do it the other way around.
>>> Your tree is fine:
>>>
>>> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>
>>> Let me know if you have problems merging this due to any tty patches,
>>> and if so, I'll be glad to take them through my tree.
>> Guys, thanks for taking a look into this!
>>
>> The patch that adds nmi console driver (i.e. tty/serial/kgdb_nmi.c)
>> depends on the first KDB patches in these series, and it depends on
>> amba-pl1011 and tty_port work, which is in tty-next.
>>
>> So, the series have to go via tty-next; I believe Jason won't able
>> to take it via his tree.
> Ok, I'll wait for Jason to review them. Jason, let me know how it
> goes...
>
Other than 1, 2 and 7. They all get:
Acked-by: Jason Wessel <jason.wessel@windriver.com>
I ran out of time to review things yesterday, but it is all reviewed now. After we get some more comments from Anton, we can move forward.
Cheers,
Jason.
^ permalink raw reply
* Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
From: Felipe Balbi @ 2012-09-19 11:59 UTC (permalink / raw)
To: Grazvydas Ignotas
Cc: balbi, Sourav Poddar, gregkh, alan, tony, khilman, linux-omap,
linux-arm-kernel, linux-serial, linux-kernel, santosh.shilimkar,
paul
In-Reply-To: <CANOLnONC1oDN3yu=ipao2A6MfuSkZfROVPewWLLEM5fNTKipxQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2331 bytes --]
On Wed, Sep 19, 2012 at 02:52:18PM +0300, Grazvydas Ignotas wrote:
> On Tue, Sep 18, 2012 at 5:02 PM, Felipe Balbi <balbi@ti.com> wrote:
> > On Tue, Sep 18, 2012 at 06:10:50PM +0530, Sourav Poddar wrote:
> >> Greg's tty-next is not booting on 2420 based N800. The failure is
> >> observed at serial init itself. The reason might be that n800 tries to
> >> resume even though it is not suspended before.
> >>
> >> Reported-by: Paul Walmsley <paul@pwsan.com>
> >> Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
> >
> > FWIW:
> >
> > Reviewed-by: Felipe Balbi <balbi@ti.com>
> >
> > Paul does this fix the issue for you ? Note that it depends on a
> > previous patch Sourav sent [1]
> >
> > [1] http://marc.info/?l=linux-omap&m=134796819607889&w=2
> >
> > There's one thing that gets my attention, though, why only n800 would
> > fail here ?
> >
> > I wonder if we should be using:
> >
> > pm_runtime_set_active(dev);
> > pm_runtime_get_enable(dev);
> >
> > to prevent our runtime_resume() to be called from probe, but Sourav
> > tested and it doesn't work on BeagleBoard, but it works on PandaBoard.
> >
> > Does it ring any bell ??
>
> Well I guess it's normal resume callback is called during probe as
> pm_runtime_get_sync() is called there, and according to documentation
> [1], devices are assumed to be suspended before probe is called.
>
> According to [1], pm_runtime_set_active() should be called before
> pm_runtime_enable() to indicate to the core that device is active
> during probe if that's the case, I guess this means
> pm_runtime_get_sync() is not needed in that case, but I'm not sure
> what's the actual serial state during probe nowadays.
>
> [1] chapter 5 of Documentation/power/runtime_pm.txt:
> The initial runtime PM status of all devices is 'suspended', but it
> need not reflect the actual physical state of the device. Thus, if the
> device is initially active (i.e. it is able to process I/O), its
> runtime PM status must be changed to 'active', with the help of
> pm_runtime_set_active(), before pm_runtime_enable() is called for the
> device.
this is what I mean, actually. If we remove pm_runtime_get_sync() in
exchange for pm_runtime_set_active() before pm_runtime_enable(), it
works on PandaBoard, but breaks BeagleBoard.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] 8250: fix autoconfig to work with serial console
From: Alan Cox @ 2012-09-19 14:02 UTC (permalink / raw)
To: Flavio Leitner; +Cc: linux-serial, linux-kernel
In-Reply-To: <1347996092-30957-1-git-send-email-fbl@redhat.com>
On Tue, 18 Sep 2012 16:21:32 -0300
Flavio Leitner <fbl@redhat.com> wrote:
> The autoconfig prints messages while holding the
> port's spinlock and that causes a deadlock when
> using serial console.
Doing autoconfig on your serial console while in use is insane, but it
shouldn't crash the system either.
Acked-by: Alan Cox <alan@linux.intel.com>
^ permalink raw reply
* Vizzini
From: Alan Cox @ 2012-09-19 15:23 UTC (permalink / raw)
To: greg, linux-serial
/* This version of the Linux driver source contains a number of
abominable conditional compilation sections to manage the API
changes between kernel versions 2.6.18, 2.6.25, and the latest
(currently 2.6.27). At some point we'll hand a version of this
driver off to the mainline Linux source tree, and we'll strip
all these sections out. For now it makes it much easier to
keep it all in sync while the driver is being developed. */
It doesn't seem to
#include <linux/usb/cdc.h>
#ifndef CDC_DATA_INTERFACE_TYPE
#define CDC_DATA_INTERFACE_TYPE 0x0a
#endif
I was wondering why this wasn't using CDC-ACM ?
struct vizzini_serial_private {
struct usb_interface *data_interface;
};
Perhaps that can get simplified ?
static struct vizzini_baud_rate vizzini_baud_rates[] = {
Some explanation might be useful ?
static int vizzini_set_baud_rate(struct usb_serial_port *port,
And the actual rate hould get passed back in the termios
(tty_termios_encode*)
static void vizzini_set_termios(struct tty_struct *tty_param,
struct usb_serial_port *port,
struct ktermios *old_termios)
{
struct tty_struct *tty = port->port.tty;
This mistake is all over the driver. port->port.tty is unsafe as it may
change under you. That's *why* we pass a safe tty reference that the
driver then ignores.
If the driver disable/enable can lose bytes it also needs to figure out
if a real hardware change has occurred or some apps will get upset.
} else if ((cflag & CSIZE) == CS5) {
/* Enabling 5-bit mode is really 9-bit mode! */
format_size = UART_FORMAT_SIZE_9;
If this is magic hackery for 9bit char then it needs doing properly,
if it's a clever way to get 5bit then fine.
} else {
format_size = UART_FORMAT_SIZE_8;
}
And for unsupported formats we need to report the format we actually
picked in the tty->termios data.
static int vizzini_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
{
struct usb_serial_port *port = tty->driver_data;
struct vizzini_port_private *portdata = usb_get_serial_port_data(port);
struct serial_struct ss;
dev_dbg(&port->dev, "%s %08x\n", __func__, cmd);
switch (cmd) {
case TIOCGSERIAL:
if (!arg)
return -EFAULT;
memset(&ss, 0, sizeof(ss));
ss.baud_base = portdata->baud_base;
This leaves lots of fields blank rather than correctly filled in as the
apps will expect. If we support it then it needs to support it all for
read even if only writing odd bits.
if (copy_to_user((void __user *)arg, &ss, sizeof(ss)))
return -EFAULT;
break;
case TIOCSSERIAL:
if (!arg)
return -EFAULT;
if (copy_from_user(&ss, (void __user *)arg, sizeof(ss)))
return -EFAULT;
portdata->baud_base = ss.baud_base;
Because setting stuff as non superuser without any validation is good.
Should also support low latency config if we are doing this.
#ifdef VIZZINI_IWA
static const int vizzini_parity[] = {
0, 1, 1, 0, 1, 0, 0, 1, 1, 0, 0, 1, 0, 1, 1, 0,
1, 0, 0, 1, 0, 1, 1, 0, 0, 1, 1, 0, 1, 0, 0, 1,
1, 0, 0, 1, 0, 1, 1, 0, 0, 1, 1, 0, 1, 0, 0, 1,
0, 1, 1, 0, 1, 0, 0, 1, 1, 0, 0, 1, 0, 1, 1, 0,
1, 0, 0, 1, 0, 1, 1, 0, 0, 1, 1, 0, 1, 0, 0, 1,
0, 1, 1, 0, 1, 0, 0, 1, 1, 0, 0, 1, 0, 1, 1, 0,
0, 1, 1, 0, 1, 0, 0, 1, 1, 0, 0, 1, 0, 1, 1, 0,
1, 0, 0, 1, 0, 1, 1, 0, 0, 1, 1, 0, 1, 0, 0, 1,
1, 0, 0, 1, 0, 1, 1, 0, 0, 1, 1, 0, 1, 0, 0, 1,
0, 1, 1, 0, 1, 0, 0, 1, 1, 0, 0, 1, 0, 1, 1, 0,
0, 1, 1, 0, 1, 0, 0, 1, 1, 0, 0, 1, 0, 1, 1, 0,
1, 0, 0, 1, 0, 1, 1, 0, 0, 1, 1, 0, 1, 0, 0, 1,
0, 1, 1, 0, 1, 0, 0, 1, 1, 0, 0, 1, 0, 1, 1, 0,
1, 0, 0, 1, 0, 1, 1, 0, 0, 1, 1, 0, 1, 0, 0, 1,
1, 0, 0, 1, 0, 1, 1, 0, 0, 1, 1, 0, 1, 0, 0, 1,
0, 1, 1, 0, 1, 0, 0, 1, 1, 0, 0, 1, 0, 1, 1, 0
};
#endif
Doing parity is something I never bothered with but if we do it then we
ought to have a helper in the tty core instead. Also it's stupid to use
ints for this as it just blows more cache. In fact you might as well use
bits and just test_bit() for parity.
static int vizzini_write_room(struct tty_struct *tty)
{
This seems dodgy - it must never report more than it can guarantee will
fit. If it ever reports 2048 it can't un-report them as free until those
2048 bytes are written. Probably it should be using the kfifo code ?
buffer = kmalloc(bufsize, GFP_ATOMIC);
A fifo would also avoid most of these problems as you can just retry
tidily. This whole path is really ugly to be honest as it gets called
with irqs off by code expecting the queueing to be fast.
static void vizzini_in_callback(struct urb *urb)
{
int endpoint = usb_pipeendpoint(urb->pipe);
struct usb_serial_port *port = urb->context;
struct vizzini_port_private *portdata = usb_get_serial_port_data(port);
struct tty_struct *tty = port->port.tty;
No kref taken so unsafe, could also be NULL, so you execute
NULL->functiontptr. Not good on platforms that don't have low memory
mapping blocked !
room = tty_buffer_request_room(tty, length);
if (room != length)
dev_dbg(&port->dev, "Not enough room in TTY buf, dropped %d chars.\n", length - room);
if (room) {
Not worth checking, just shovel it in - the buffer code will do the work
and only fail when its really congested and the box is stuffed up. Plus
your dev_dbg here will deadlock if this is a USB serial console I think ?
static void vizzini_int_callback(struct urb *urb)
{
struct usb_serial_port *port = urb->context;
struct vizzini_port_private *portdata =
usb_get_serial_port_data(port); struct tty_struct *tty =
port->port.tty;
Again kref...
:
dev_dbg(&port->dev, "Resubmitting interrupt IN urb %p\n", urb);
status = usb_submit_urb(urb, GFP_ATOMIC);
if (status)
dev_err(&port->dev, "usb_submit_urb failed with result %d", status);
Whats the error recovery path for this - a single fail kills the port ?
}
static int vizzini_open(struct tty_struct *tty_param, struct
usb_serial_port *port) {
struct vizzini_port_private *portdata;
struct usb_serial *serial = port->serial;
struct tty_struct *tty = port->port.tty;
Again we pass the tty for a reason !
static void vizzini_close(struct usb_serial_port *port)
{
int i;
struct usb_serial *serial = port->serial;
struct vizzini_port_private *portdata;
struct tty_struct *tty = port->port.tty;
You can't safelty reference tty from here, but it's not used anyway so
just kill it off.
portdata = usb_get_serial_port_data(port);
acm_set_control(port, portdata->ctrlout = 0);
if (serial->dev) {
/* Stop reading/writing urbs */
for (i = 0; i < N_IN_URB; i++)
usb_kill_urb(portdata->in_urbs[i]);
}
usb_kill_urb(port->interrupt_in_urb);
tty = NULL; /* FIXME */
Alan
^ permalink raw reply
* Re: [PATCH 07/11] tty/serial: Add kgdb_nmi driver
From: Anton Vorontsov @ 2012-09-19 16:54 UTC (permalink / raw)
To: Jason Wessel
Cc: Andrew Morton, Russell King, Greg Kroah-Hartman, Alan Cox,
Arve Hjønnevåg, Colin Cross, Brian Swetland,
John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
linaro-kernel, patches, kernel-team, kgdb-bugreport, linux-serial
In-Reply-To: <5059B200.9040706@windriver.com>
On Wed, Sep 19, 2012 at 06:52:32AM -0500, Jason Wessel wrote:
[...]
> > --- a/drivers/tty/serial/kgdboc.c
> > +++ b/drivers/tty/serial/kgdboc.c
> > @@ -145,6 +145,8 @@ __setup("kgdboc=", kgdboc_option_setup);
> >
> > static void cleanup_kgdboc(void)
> > {
> > + if (kgdb_unregister_nmi_console())
> > + return;
> > kgdboc_unregister_kbd();
> > if (configured == 1)
> > kgdb_unregister_io_module(&kgdboc_io_ops);
> > @@ -198,6 +200,10 @@ do_register:
> > if (err)
> > goto noconfig;
> >
> > + err = kgdb_register_nmi_console();
> > + if (err)
> > + goto noconfig;
> > +
>
> Just one question on this one, what do you expect to happen, or how
> does this work when you use a different serial port for example on a
> board that has a real serial port and an FIQ port? It was not
> obvious that there was a path to check if the port you are
> registering is an FIQ enabled port and then call the
> kgdb_register_nmi_console at that point.
Yes, it works perfectly fine. If the port is not FIQ-enabled (which,
technically, can be enabled later -- it's just not implemented), then
ttyNMI just won't able to receive data.
The reason why I register nmi console in KGDB code (as opposite to arch
code), is the dependcy: the port should be initialized before it can be
used as ttyNMI. We could place the ttyNMI registration code into
late_initcall in arch/, and check for dbg_io_ops, but that is ugly by
itself, plus dbg_io_ops can change.
But with your kgdb_arch->enable_nmi suggestion (which I'll surely
implement), we can skip the registration at least for the cases when
the arch doesn't have this feature.
Thanks!
Anton.
^ 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