* [PATCHv2 1/1] [RFC] uartclk from serial_core exposed to sysfs
From: Tomas Hlavacek @ 2012-08-15 17:12 UTC (permalink / raw)
To: gregkh, alan, linux-serial, linux-kernel; +Cc: marek.vasut, Tomas Hlavacek
In-Reply-To: <1344929718-22736-1-git-send-email-tmshlvck@gmail.com>
Added file /sys/devices/.../tty/ttySX/uartclk to allow read/modify
uartclk value in struct uart_port in serial_core via sysfs.
It simplifies initialization of no-name cards that have non-standard
oscillator speed while having no distinguishing PCI IDs to allow
autodetection.
Signed-off-by: Tomas Hlavacek <tmshlvck@gmail.com>
---
drivers/tty/serial/serial_core.c | 55 ++++++++++++++++++++++++++++++++++++++
drivers/tty/tty_io.c | 17 ++++++++++++
include/linux/tty.h | 2 ++
3 files changed, 74 insertions(+)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index a21dc8e..0929fe3 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2293,6 +2293,47 @@ struct tty_driver *uart_console_device(struct console *co, int *index)
return p->tty_driver;
}
+static ssize_t uart_get_attr_uartclk(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int ret;
+
+ struct uart_state *state = dev_get_drvdata(dev);
+ mutex_lock(&state->port.mutex);
+ ret = snprintf(buf, PAGE_SIZE, "%d\n", state->uart_port->uartclk);
+ mutex_unlock(&state->port.mutex);
+ return ret;
+}
+
+static ssize_t uart_set_attr_uartclk(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct uart_state *state = dev_get_drvdata(dev);
+ unsigned int val;
+ int ret;
+
+ ret = kstrtouint(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ mutex_lock(&state->port.mutex);
+
+ /*
+ * Check value: baud_base does not make sense to be set below 9600
+ * and since uartclock = (baud_base * 16) it has to be equal or greater
+ * than 9600 * 16 = 153600.
+ */
+ if (val >= 153600)
+ state->uart_port->uartclk = val;
+
+ mutex_unlock(&state->port.mutex);
+
+ return count;
+}
+
+static DEVICE_ATTR(uartclk, S_IRUGO | S_IWUSR, uart_get_attr_uartclk,
+ uart_set_attr_uartclk);
+
/**
* uart_add_one_port - attach a driver-defined port structure
* @drv: pointer to the uart low level driver structure for this port
@@ -2355,6 +2396,14 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
}
/*
+ * Expose uartclk in sysfs. Use driverdata of the tty device for
+ * referencing the UART port.
+ */
+ dev_set_drvdata(tty_dev, state);
+ if (device_create_file(tty_dev, &dev_attr_uartclk) < 0)
+ dev_err(tty_dev, "Failed to add uartclk attr\n");
+
+ /*
* Ensure UPF_DEAD is not set.
*/
uport->flags &= ~UPF_DEAD;
@@ -2397,6 +2446,12 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport)
mutex_unlock(&port->mutex);
/*
+ * Remove uartclk file from sysfs.
+ */
+ device_remove_file(tty_lookup_device(drv->tty_driver, uport->line),
+ &dev_attr_uartclk);
+
+ /*
* Remove the devices from the tty layer
*/
tty_unregister_device(drv->tty_driver, uport->line);
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index b425c79..8ea8622 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3049,6 +3049,23 @@ void tty_unregister_device(struct tty_driver *driver, unsigned index)
}
EXPORT_SYMBOL(tty_unregister_device);
+/*
+ * tty_lookup_device - lookup a tty device
+ * @driver: the tty driver that describes the tty device
+ * @index: the index in the tty driver for this tty device
+ *
+ * This function returns a struct device pointer when device has
+ * been found and NULL otherwise.
+ *
+ * Locking: ??
+ */
+struct device *tty_lookup_device(struct tty_driver *driver, unsigned index)
+{
+ dev_t devt = MKDEV(driver->major, driver->minor_start) + index;
+ return class_find_device(tty_class, NULL, &devt, dev_match_devt);
+}
+EXPORT_SYMBOL(tty_lookup_device);
+
struct tty_driver *__alloc_tty_driver(int lines, struct module *owner)
{
struct tty_driver *driver;
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 9f47ab5..5d408a1 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -410,6 +410,8 @@ extern int tty_register_driver(struct tty_driver *driver);
extern int tty_unregister_driver(struct tty_driver *driver);
extern struct device *tty_register_device(struct tty_driver *driver,
unsigned index, struct device *dev);
+extern struct device *tty_lookup_device(struct tty_driver *driver,
+ unsigned index);
extern void tty_unregister_device(struct tty_driver *driver, unsigned index);
extern int tty_read_raw_data(struct tty_struct *tty, unsigned char *bufp,
int buflen);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH 5/8] video: vt8500: Add devicetree support for vt8500-fb and wm8505-fb
From: Stephen Warren @ 2012-08-15 19:21 UTC (permalink / raw)
To: Tony Prisk
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
vt8500-wm8505-linux-kernel-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
Russell King, Florian Tobias Schandinat, Alan Stern,
Eric Andersson,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
rtc-linux-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org, Hauke Mehrtens,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
Rob Herring, Stephen Warren, Neil Zhang,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <76F764B079F92A4E843589C893D0A022D1DCA8D0-A1+cU8XkcJSYgi1/3OOQJ8krCUz0bFs7@public.gmane.org>
On 08/08/2012 01:37 PM, Tony Prisk wrote:
>> On 08/07/2012 07:39 PM, Tony Prisk wrote:
>>> Update vt8500-fb, wm8505-fb and wmt-ge-rops to support device
>>> tree bindings.
>>> diff --git a/drivers/video/vt8500lcdfb.c b/drivers/video/vt8500lcdfb.c
>
>>> + np = of_find_node_by_name(NULL, "display");
>>> + if (!np) {
>>> + pr_err("%s: No display description in Device Tree\n", __func__);
>>> + ret = -EINVAL;
>>> + goto failed_free_res;
>>> + }
>
>> I believe that using hard-coded node names is frowned upon. Better would
>> be to put a phandle into the display controller's node that points at
>> the node representing the display, e.g.:
>
> Will do.
I don't think this change made it into v2?
^ permalink raw reply
* Re: [PATCHv2 0/8] *** ARM: Update arch-vt8500 to Devicetree ***
From: Stephen Warren @ 2012-08-15 19:23 UTC (permalink / raw)
To: Tony Prisk
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Mark Brown,
vt8500-wm8505-linux-kernel-/JYPxA39Uh5TLH3MbocFFw, Russell King,
rtc-linux-/JYPxA39Uh5TLH3MbocFFw, Florian Tobias Schandinat,
Alan Stern, Eric Andersson, linux-serial-u79uwXL29TY76Z2rM5mHXA,
Hauke Mehrtens, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
Rob Herring, Stephen Warren, Neil Zhang,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Alan Cox,
Alessandro Zummo, Linus Walleij, Greg Kroah-Hartman,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Felipe Balbi
In-Reply-To: <1344477300-25251-1-git-send-email-linux-ci5G2KO2hbZ+pU9mqzGVBQ@public.gmane.org>
On 08/08/2012 07:54 PM, Tony Prisk wrote:
> This patchset updates arch-vt8500 to devicetree and removes all the old-style
> code. Support for WM8650 has also been added.
Sorry for taking such a long time to re-review this.
I scanned the series looking for just the changes to the issues I raised
on v1, and I do see almost everything has been fixed up OK, except for
the one issue I just commented on. So, aside from that, this series is
fine by me. I'll defer to others about whether it's necessary to fix the
display node-name-vs.-phandle issue I pointed out, or defer that until
later.
^ permalink raw reply
* Re: [PATCH V2] Remove BUG_ON from n_tty_read()
From: Stanislav Kozina @ 2012-08-16 7:52 UTC (permalink / raw)
To: Alan Cox; +Cc: Greg Kroah-Hartman, linux-serial, Stanislaw Gruszka
In-Reply-To: <50251CF5.5050709@redhat.com>
Alan,
What about integration of this patch?
I will try to do some testing to understand how it can happen that we
read from tty already closed.
Thanks and regards,
-Stanislav
> Change the BUG_ON to WARN_ON and return in case of tty->read_buf==NULL
>
> Signed-off-by: Stanislav Kozina <skozina@redhat.com>
> ---
> drivers/tty/n_tty.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index df21f39..6b9b5e0 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -1728,7 +1728,8 @@ static ssize_t n_tty_read(struct tty_struct
> *tty, struct file *file,
>
> do_it_again:
>
> - BUG_ON(!tty->read_buf);
> + if (WARN_ON(!tty->read_buf))
> + return -EAGAIN;
>
> c = job_control(tty, file);
> if (c < 0)
^ permalink raw reply
* RE: [PATCHv2 0/8] *** ARM: Update arch-vt8500 to Devicetree ***
From: Tony Prisk @ 2012-08-16 10:18 UTC (permalink / raw)
To: Stephen Warren
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mark Brown,
vt8500-wm8505-linux-kernel-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
Russell King, rtc-linux-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
Florian Tobias Schandinat, Alan Stern, Eric Andersson,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Hauke Mehrtens,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
Rob Herring, Stephen Warren, Neil Zhang,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Alan Cox, Alessandro Zummo
In-Reply-To: <502BF734.9000608-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
>>On 08/08/2012 07:54 PM, Tony Prisk wrote:
>> This patchset updates arch-vt8500 to devicetree and removes all the old-style
>> code. Support for WM8650 has also been added.
>Sorry for taking such a long time to re-review this.
>I scanned the series looking for just the changes to the issues I raised
>on v1, and I do see almost everything has been fixed up OK, except for
>the one issue I just commented on. So, aside from that, this series is
>fine by me. I'll defer to others about whether it's necessary to fix the
>display node-name-vs.-phandle issue I pointed out, or defer that until
>later.
Thanks for the review Stephen.
There will be a v3 shortly - I noticed a few other issues that needed to
be corrected as well. I agree with the display-node/phandle comment
and will make sure its included for v3. To be honest, it just slipped my
memory at the time.
Tony Prisk
^ permalink raw reply
* Re: [PATCH 1/1] [RFC] uartclk from serial_core exposed to sysfs
From: Alan Cox @ 2012-08-16 10:31 UTC (permalink / raw)
To: Tomas Hlavacek; +Cc: gregkh, alan, linux-serial, linux-kernel, marek.vasut
In-Reply-To: <1344929718-22736-1-git-send-email-tmshlvck@gmail.com>
> + /*
> + * Check value: baud_base has to be more than 9600
> + * and uartclock = baud_base * 16 .
> + */
> + if (val >= 153600)
> + state->uart_port->uartclk = val;
> +
> + mutex_unlock(&state->port.mutex);
> +
> + return count;
This breaks if for example the port is in use. Fixing that looks pretty
horrible as you need a valid tty pointer to stop and restart the pot.
It's also not calling the verfy method of the port as is expected.
At minimum I think you need to be able to do the same work
uart_get_info/uart_set_info perform and with the same checks on ->verify
etc.
I'm not 100% sure the drvdata on the tty_dev is clear to use. It does
seem to be in all the drivers I looked at. I'd rather however it pointed
to the tty_port that each tty device has (or very soon will be required
to have). You can still find the uart_foo structs from that but it means
we can do the dev_set_drvdata() in a consistent manner for all tty
devices in the kernel. That in turn means we can make some of the sysfs
valid the same way.
I want to have think about the setting side of it. Can you submit a
revised version that just allows the user to read the value this way but
does the drvdata setting etc and sysfs node create/delete.
I'll merge that and we can throw it over the parapet and see if anything
explodes.
To make the setting part work properly I think I need to sort out
uart_get_info/set_info so the core part of it can be called with kernel
space structures and the caller handling locks.
Alan
^ permalink raw reply
* [PATCH 1/2] 8250: add AgeStar AS-PRS2-009
From: Alan Cox @ 2012-08-16 11:01 UTC (permalink / raw)
To: greg, linux-serial
From: Alan Cox <alan@linux.intel.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>
Resolves-bug: https://bugzilla.kernel.org/show_bug.cgi?id=22502
---
drivers/tty/serial/8250/8250_pci.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 62e10fe..ad4bb8d 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1194,6 +1194,8 @@ pci_xr17c154_setup(struct serial_private *priv,
#define PCIE_DEVICE_ID_NEO_2_OX_IBM 0x00F6
#define PCI_DEVICE_ID_PLX_CRONYX_OMEGA 0xc001
#define PCI_DEVICE_ID_INTEL_PATSBURG_KT 0x1d3d
+#define PCI_VENDOR_ID_AGESTAR 0x5372
+#define PCI_DEVICE_ID_AGESTAR_9375 0x6872
#define PCI_VENDOR_ID_ASIX 0x9710
/* Unknown vendors/cards - this should not be in linux/pci_ids.h */
@@ -4189,6 +4191,12 @@ static struct pci_device_id serial_pci_tbl[] = {
pbn_omegapci },
/*
+ * AgeStar as-prs2-009
+ */
+ { PCI_VENDOR_ID_AGESTAR, PCI_DEVICE_ID_AGESTAR_9375,
+ PCI_ANY_ID, PCI_ANY_ID,
+ 0, 0, pbn_b0_bt_2_115200 },
+ /*
* These entries match devices with class COMMUNICATION_SERIAL,
* COMMUNICATION_MODEM or COMMUNICATION_MULTISERIAL
*/
^ permalink raw reply related
* [PATCH 2/2] Remove BUG_ON from n_tty_read()
From: Alan Cox @ 2012-08-16 11:01 UTC (permalink / raw)
To: greg, linux-serial
In-Reply-To: <20120816110130.774.87642.stgit@localhost.localdomain>
From: Stanislav Kozina <skozina@redhat.com>
Change the BUG_ON to WARN_ON and return in case of tty->read_buf==NULL. We want to track a
couple of long standing reports of this but at the same time we can avoid killing the box.
Signed-off-by: Stanislav Kozina <skozina@redhat.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>
Cc: Horses <stable@kernel.org>
---
drivers/tty/n_tty.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 4776aa2..ece22c0 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1734,7 +1734,8 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
do_it_again:
- BUG_ON(!tty->read_buf);
+ if (WARN_ON(!tty->read_buf))
+ return -EAGAIN;
c = job_control(tty, file);
if (c < 0)
^ permalink raw reply related
* Re: [PATCH] Powerpc 8xx CPM_UART delay in receive
From: leroy christophe @ 2012-08-16 13:16 UTC (permalink / raw)
To: Alan Cox
Cc: Alan Cox, Vitaly Bordug, Marcelo Tosatti, linux-kernel,
linux-serial, linuxppc-dev
In-Reply-To: <20120814155227.018988da@pyramind.ukuu.org.uk>
Le 14/08/2012 16:52, Alan Cox a écrit :
> On Tue, 14 Aug 2012 16:26:28 +0200
> Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>
>> Hello,
>>
>> I'm not sure who to address this Patch to either
>>
>> It fixes a delay issue with CPM UART driver on Powerpc MPC8xx.
>> The problem is that with the actual code, the driver waits 32 IDLE patterns before returning the received data to the upper level. It means for instance about 1 second at 300 bauds.
>> This fix limits to one byte the waiting period.
> Take a look how the 8250 does it - I think you want to set the value
> based upon the data rate. Your patch will break it for everyone doing
> high seed I/O.
>
> Alan
>
I'm not sure I understand what you mean. As far as I can see 8250/16550
is working a bit different, as it is based on a fifo and triggers an
interrupt as soon as a given number of bytes is received. I also see
that in case this amount is not reached, there is a receive-timeout
which goes on after no byte is received for a duration of more than 4 bytes.
The PowerPC CPM is working differently. It doesn't use a fifo but
buffers. Buffers are handed to the microprocessor only when they are
full or after a timeout period which is adjustable. In the driver, the
buffers are configured with a size of 32 bytes. And the timeout is set
to the size of the buffer. That is this timeout that I'm reducing to 1
byte in my proposed patch. I can't see what it would break for high
speed I/O.
Christophe
^ permalink raw reply
* Re: [PATCH] Powerpc 8xx CPM_UART delay in receive
From: Alan Cox @ 2012-08-16 14:29 UTC (permalink / raw)
To: leroy christophe
Cc: Marcelo Tosatti, linux-kernel, linux-serial, linuxppc-dev,
Alan Cox
In-Reply-To: <502CF2A0.8080109@c-s.fr>
> The PowerPC CPM is working differently. It doesn't use a fifo but
> buffers. Buffers are handed to the microprocessor only when they are
> full or after a timeout period which is adjustable. In the driver, the
Which is different how - remembering we empty the FIFO on an IRQ
> buffers are configured with a size of 32 bytes. And the timeout is set
> to the size of the buffer. That is this timeout that I'm reducing to 1
> byte in my proposed patch. I can't see what it would break for high
> speed I/O.
How can a timeout be measured in "bytes". Can we have a bit more clarity
on how the hardware works and take it from there ?
Alan
^ permalink raw reply
* Re: [PATCH] Powerpc 8xx CPM_UART delay in receive
From: leroy christophe @ 2012-08-16 14:35 UTC (permalink / raw)
To: Alan Cox
Cc: Alan Cox, Vitaly Bordug, Marcelo Tosatti, linux-kernel,
linux-serial, linuxppc-dev
In-Reply-To: <20120816152918.5ed2649f@pyramind.ukuu.org.uk>
Le 16/08/2012 16:29, Alan Cox a écrit :
>> The PowerPC CPM is working differently. It doesn't use a fifo but
>> buffers. Buffers are handed to the microprocessor only when they are
>> full or after a timeout period which is adjustable. In the driver, the
> Which is different how - remembering we empty the FIFO on an IRQ
>
>> buffers are configured with a size of 32 bytes. And the timeout is set
>> to the size of the buffer. That is this timeout that I'm reducing to 1
>> byte in my proposed patch. I can't see what it would break for high
>> speed I/O.
> How can a timeout be measured in "bytes". Can we have a bit more clarity
> on how the hardware works and take it from there ?
>
> Alan
>
The reference manual of MPC885 says the following about the MAX_IDL
parameter:
MAX_IDL: Maximum idle characters. When a character is received, the
receiver begins counting idle characters. If MAX_IDL idle characters are
received before the next data character, an idle timeout occurs and the
buffer is closed,
generating a maskable interrupt request to the core to receive the data
from the buffer. Thus, MAX_IDL offers a way to demarcate frames. To
disable the feature, clear MAX_IDL. The bit length of an idle character
is calculated as follows: 1 + data length (5–9) + 1 (if parity is used)
+ number of stop bits (1–2). For 8 data bits, no parity, and 1 stop bit,
the character length is 10 bits
If the UART is receiving data and gets an idle character (all ones), the
channel begins counting consecutive idle characters received. If MAX_IDL
is reached, the buffer is closed and an RX interrupt is generated if not
masked. If no buffer is open, this event does not generate an interrupt
or any status information. The internal idle counter (IDLC) is reset
every time a character is received. To disable the idle sequence
function, clear MAX_IDL
The datasheet of the 16550 UART says:
Besides, for FIFO mode operation a time out mechanism is implemented.
Independently of the trigger level of the FIFO, an interrupt will be
generated if there is at least one word in the FIFO and for a time
equivalent to the transmission of four characters
- no new character has been received and
- the microprocessor has not read the RHR
To compute the time out, the current total number of bits (start, data,
parity and stop(s)) is used, together with the current baud rate (i.e.,
it depends on the contents of the LCR, DLL, DLM and PSD registers).
Christophe
--
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] Powerpc 8xx CPM_UART delay in receive
From: Alan Cox @ 2012-08-16 15:21 UTC (permalink / raw)
To: leroy christophe
Cc: Alan Cox, Vitaly Bordug, Marcelo Tosatti, linux-kernel,
linux-serial, linuxppc-dev
In-Reply-To: <502D054B.3010606@c-s.fr>
> MAX_IDL: Maximum idle characters. When a character is received, the
> receiver begins counting idle characters. If MAX_IDL idle characters
> are received before the next data character, an idle timeout occurs
> and the buffer is closed,
> generating a maskable interrupt request to the core to receive the
> data from the buffer. Thus, MAX_IDL offers a way to demarcate frames.
> To disable the feature, clear MAX_IDL. The bit length of an idle
> character is calculated as follows: 1 + data length (5–9) + 1 (if
> parity is used)
> + number of stop bits (1–2). For 8 data bits, no parity, and 1 stop
> bit, the character length is 10 bits
So if you have slightly bursty high speed data as its quite typical
before your change you would get one interrupt per buffer of 32 bytes,
with it you'll get a lot more interrupts.
You have two available hints about the way to set this - one of them is
the baud rate (low baud rates mean the fifo isn't a big win and the
latency is high), the other is the low_latency flag if the driver
supports the low latency feature (and arguably you can still use a
request for it as a hint even if you refuse the actual feature).
So I think a reasonable approach would be set the idle timeout down for
low baud rates or if low_latency is requested.
> generated if there is at least one word in the FIFO and for a time
> equivalent to the transmission of four characters
Which is a bit more reasonable than one, although problematic at low
speed (hence the fifo on/off).
--
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] [media] winbond-cir: Fix initialization
From: Greg KH @ 2012-08-16 16:19 UTC (permalink / raw)
To: Sean Young
Cc: Mauro Carvalho Chehab, Jarod Wilson, David Härdeman,
Alan Cox, linux-media, linux-serial, lirc-list
In-Reply-To: <1343731023-9822-1-git-send-email-sean@mess.org>
On Tue, Jul 31, 2012 at 11:37:03AM +0100, Sean Young wrote:
> The serial driver will detect the winbond cir device as a serial port,
> since it looks exactly like a serial port unless you know what it is
> from the PNP ID.
>
> Winbond CIR 00:04: Region 0x2f8-0x2ff already in use!
> Winbond CIR 00:04: disabled
> Winbond CIR: probe of 00:04 failed with error -16
>
> Signed-off-by: Sean Young <sean@mess.org>
> ---
> drivers/media/rc/winbond-cir.c | 21 ++++++++++++++++++++-
> drivers/tty/serial/8250/8250.c | 1 +
> 2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
> index 54ee348..20a0bbb 100644
> --- a/drivers/media/rc/winbond-cir.c
> +++ b/drivers/media/rc/winbond-cir.c
> @@ -55,6 +55,7 @@
> #include <linux/slab.h>
> #include <linux/wait.h>
> #include <linux/sched.h>
> +#include <linux/serial_8250.h>
> #include <media/rc-core.h>
>
> #define DRVNAME "winbond-cir"
> @@ -957,6 +958,7 @@ wbcir_probe(struct pnp_dev *device, const struct pnp_device_id *dev_id)
> struct device *dev = &device->dev;
> struct wbcir_data *data;
> int err;
> + struct resource *io;
>
> if (!(pnp_port_len(device, 0) == EHFUNC_IOMEM_LEN &&
> pnp_port_len(device, 1) == WAKEUP_IOMEM_LEN &&
> @@ -1049,7 +1051,24 @@ wbcir_probe(struct pnp_dev *device, const struct pnp_device_id *dev_id)
> goto exit_release_wbase;
> }
>
> - if (!request_region(data->sbase, SP_IOMEM_LEN, DRVNAME)) {
> + io = request_region(data->sbase, SP_IOMEM_LEN, DRVNAME);
> +
> + /*
> + * The winbond cir device looks exactly like an NS16550A serial port
> + * unless you know what it is. We've got here via the PNP ID.
> + */
> +#ifdef CONFIG_SERIAL_8250
> + if (!io) {
> + struct uart_port port = { .iobase = data->sbase };
> + int line = serial8250_find_port(&port);
> + if (line >= 0) {
> + serial8250_unregister_port(line);
> +
> + io = request_region(data->sbase, SP_IOMEM_LEN, DRVNAME);
> + }
> + }
> +#endif
> + if (!io) {
> dev_err(dev, "Region 0x%lx-0x%lx already in use!\n",
> data->sbase, data->sbase + SP_IOMEM_LEN - 1);
> err = -EBUSY;
> diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
> index 5c27f7e..d38615f 100644
> --- a/drivers/tty/serial/8250/8250.c
> +++ b/drivers/tty/serial/8250/8250.c
> @@ -2914,6 +2914,7 @@ int serial8250_find_port(struct uart_port *p)
> }
> return -ENODEV;
> }
> +EXPORT_SYMBOL(serial8250_find_port);
EXPORT_SYMBOL_GPL please.
But can't this be done as a quirk to the 8250 driver so that it just
does not bind to this device in the first place? Wouldn't that make
more sense?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 2/2] Remove BUG_ON from n_tty_read()
From: Greg KH @ 2012-08-16 18:46 UTC (permalink / raw)
To: Alan Cox; +Cc: linux-serial
In-Reply-To: <20120816110146.774.73249.stgit@localhost.localdomain>
On Thu, Aug 16, 2012 at 12:01:47PM +0100, Alan Cox wrote:
> From: Stanislav Kozina <skozina@redhat.com>
>
> Change the BUG_ON to WARN_ON and return in case of tty->read_buf==NULL. We want to track a
> couple of long standing reports of this but at the same time we can avoid killing the box.
>
> Signed-off-by: Stanislav Kozina <skozina@redhat.com>
> Signed-off-by: Alan Cox <alan@linux.intel.com>
> Cc: Horses <stable@kernel.org>
Heh, "Horses"? That's a first...
^ permalink raw reply
* Re: [PATCH 1/8] n_gsm.c: Implement 3GPP27.010 DLC start-up procedure in MUX
From: Greg KH @ 2012-08-16 18:57 UTC (permalink / raw)
To: Alan Cox; +Cc: linux-serial
In-Reply-To: <20120813124254.6125.70371.stgit@localhost.localdomain>
On Mon, Aug 13, 2012 at 01:43:15PM +0100, Alan Cox wrote:
> From: xiaojin <jin.xiao@intel.com>
>
> In 3GPP27.010 5.8.1, it defined:
> The TE multiplexer initiates the establishment of the multiplexer control channel by sending a SABM frame on DLCI 0 using the procedures of clause 5.4.1.
> Once the multiplexer channel is established other DLCs may be established using the procedures of clause 5.4.1.
> This patch implement 5.8.1 in MUX level, it make sure DLC0 is the first channel to be setup.
>
> [or for those not familiar with the specification: it was possible to try
> and open a data connection while the control channel was not yet fully
> open, which is a spec violation and confuses some modems]
>
> Signed-off-by: xiaojin <jin.xiao@intel.com>
> Tested-by: Yin, Fengwei <fengwei.yin@intel.com>
> [tweaked the order we check things and error code]
> Signed-off-by: Alan Cox <alan@linux.intel.com>
> Cc: The Horsebox <stable@kernel.org>
In the future, it's <stable@vger.kernel.org>, I can catch these
addresses, but it's not as automated.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 1/8] n_gsm.c: Implement 3GPP27.010 DLC start-up procedure in MUX
From: Greg KH @ 2012-08-16 19:01 UTC (permalink / raw)
To: Alan Cox; +Cc: linux-serial
In-Reply-To: <20120813124254.6125.70371.stgit@localhost.localdomain>
On Mon, Aug 13, 2012 at 01:43:15PM +0100, Alan Cox wrote:
> From: xiaojin <jin.xiao@intel.com>
>
> In 3GPP27.010 5.8.1, it defined:
> The TE multiplexer initiates the establishment of the multiplexer control channel by sending a SABM frame on DLCI 0 using the procedures of clause 5.4.1.
> Once the multiplexer channel is established other DLCs may be established using the procedures of clause 5.4.1.
> This patch implement 5.8.1 in MUX level, it make sure DLC0 is the first channel to be setup.
>
> [or for those not familiar with the specification: it was possible to try
> and open a data connection while the control channel was not yet fully
> open, which is a spec violation and confuses some modems]
>
> Signed-off-by: xiaojin <jin.xiao@intel.com>
> Tested-by: Yin, Fengwei <fengwei.yin@intel.com>
> [tweaked the order we check things and error code]
> Signed-off-by: Alan Cox <alan@linux.intel.com>
> Cc: The Horsebox <stable@vger.kernel.org>
> ---
>
> drivers/tty/n_gsm.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index 7a4bf30..5c6c2e2 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -2889,6 +2889,10 @@ static int gsmtty_open(struct tty_struct *tty, struct file *filp)
> gsm = gsm_mux[mux];
> if (gsm->dead)
> return -EL2HLT;
> + /* If DLCI 0 is not yet fully open return an error. This is ok from a locking
> + perspective as we don't have to worry about this if DLCI0 is lost */
> + if (gsm->dlci[0] && gsm->dlci[0]->state != DLCI_OPEN)
> + return -EL2NSYNC;
Odd, what tree did you make this against?
This applies in the gsmtty_init() function, not gsmtty_open(), is that
correct? It also has a bunch of fuzz.
I'll apply it, but can you verify I didn't do anything wrong?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 1/8] n_gsm.c: Implement 3GPP27.010 DLC start-up procedure in MUX
From: Alan Cox @ 2012-08-16 19:12 UTC (permalink / raw)
To: Greg KH; +Cc: linux-serial
In-Reply-To: <20120816190104.GA3160@kroah.com>
On Thu, 16 Aug 2012 12:01:04 -0700
Greg KH <greg@kroah.com> wrote:
> On Mon, Aug 13, 2012 at 01:43:15PM +0100, Alan Cox wrote:
> > From: xiaojin <jin.xiao@intel.com>
> >
> > In 3GPP27.010 5.8.1, it defined:
> > The TE multiplexer initiates the establishment of the multiplexer control channel by sending a SABM frame on DLCI 0 using the procedures of clause 5.4.1.
> > Once the multiplexer channel is established other DLCs may be established using the procedures of clause 5.4.1.
> > This patch implement 5.8.1 in MUX level, it make sure DLC0 is the first channel to be setup.
> >
> > [or for those not familiar with the specification: it was possible to try
> > and open a data connection while the control channel was not yet fully
> > open, which is a spec violation and confuses some modems]
> >
> > Signed-off-by: xiaojin <jin.xiao@intel.com>
> > Tested-by: Yin, Fengwei <fengwei.yin@intel.com>
> > [tweaked the order we check things and error code]
> > Signed-off-by: Alan Cox <alan@linux.intel.com>
> > Cc: The Horsebox <stable@vger.kernel.org>
> > ---
> >
> > drivers/tty/n_gsm.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> > index 7a4bf30..5c6c2e2 100644
> > --- a/drivers/tty/n_gsm.c
> > +++ b/drivers/tty/n_gsm.c
> > @@ -2889,6 +2889,10 @@ static int gsmtty_open(struct tty_struct *tty, struct file *filp)
> > gsm = gsm_mux[mux];
> > if (gsm->dead)
> > return -EL2HLT;
> > + /* If DLCI 0 is not yet fully open return an error. This is ok from a locking
> > + perspective as we don't have to worry about this if DLCI0 is lost */
> > + if (gsm->dlci[0] && gsm->dlci[0]->state != DLCI_OPEN)
> > + return -EL2NSYNC;
>
> Odd, what tree did you make this against?
>
> This applies in the gsmtty_init() function, not gsmtty_open(), is that
> correct? It also has a bunch of fuzz.
No it's not correct..
Ah its colliding with the tty_port changes - my bad. I'll rebase my tree.
Alan
^ permalink raw reply
* Re: [PATCH 1/8] n_gsm.c: Implement 3GPP27.010 DLC start-up procedure in MUX
From: Greg KH @ 2012-08-16 19:17 UTC (permalink / raw)
To: Alan Cox; +Cc: linux-serial
In-Reply-To: <20120816201255.7303f38f@pyramind.ukuu.org.uk>
On Thu, Aug 16, 2012 at 08:12:55PM +0100, Alan Cox wrote:
> On Thu, 16 Aug 2012 12:01:04 -0700
> Greg KH <greg@kroah.com> wrote:
>
> > On Mon, Aug 13, 2012 at 01:43:15PM +0100, Alan Cox wrote:
> > > From: xiaojin <jin.xiao@intel.com>
> > >
> > > In 3GPP27.010 5.8.1, it defined:
> > > The TE multiplexer initiates the establishment of the multiplexer control channel by sending a SABM frame on DLCI 0 using the procedures of clause 5.4.1.
> > > Once the multiplexer channel is established other DLCs may be established using the procedures of clause 5.4.1.
> > > This patch implement 5.8.1 in MUX level, it make sure DLC0 is the first channel to be setup.
> > >
> > > [or for those not familiar with the specification: it was possible to try
> > > and open a data connection while the control channel was not yet fully
> > > open, which is a spec violation and confuses some modems]
> > >
> > > Signed-off-by: xiaojin <jin.xiao@intel.com>
> > > Tested-by: Yin, Fengwei <fengwei.yin@intel.com>
> > > [tweaked the order we check things and error code]
> > > Signed-off-by: Alan Cox <alan@linux.intel.com>
> > > Cc: The Horsebox <stable@vger.kernel.org>
> > > ---
> > >
> > > drivers/tty/n_gsm.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> > > index 7a4bf30..5c6c2e2 100644
> > > --- a/drivers/tty/n_gsm.c
> > > +++ b/drivers/tty/n_gsm.c
> > > @@ -2889,6 +2889,10 @@ static int gsmtty_open(struct tty_struct *tty, struct file *filp)
> > > gsm = gsm_mux[mux];
> > > if (gsm->dead)
> > > return -EL2HLT;
> > > + /* If DLCI 0 is not yet fully open return an error. This is ok from a locking
> > > + perspective as we don't have to worry about this if DLCI0 is lost */
> > > + if (gsm->dlci[0] && gsm->dlci[0]->state != DLCI_OPEN)
> > > + return -EL2NSYNC;
> >
> > Odd, what tree did you make this against?
> >
> > This applies in the gsmtty_init() function, not gsmtty_open(), is that
> > correct? It also has a bunch of fuzz.
>
> No it's not correct..
Odd, git's 3-way merge thinks it figured it out as being ok.
> Ah its colliding with the tty_port changes - my bad. I'll rebase my tree.
Ok, I've applied it now, if there's a fixup needed, please send it
instead.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] [media] winbond-cir: Fix initialization
From: Sean Young @ 2012-08-16 21:16 UTC (permalink / raw)
To: Greg KH
Cc: Mauro Carvalho Chehab, Jarod Wilson, David Härdeman,
Alan Cox, linux-media, linux-serial, lirc-list
In-Reply-To: <20120816161909.GB29199@kroah.com>
On Thu, Aug 16, 2012 at 09:19:09AM -0700, Greg KH wrote:
> On Tue, Jul 31, 2012 at 11:37:03AM +0100, Sean Young wrote:
> > The serial driver will detect the winbond cir device as a serial port,
> > since it looks exactly like a serial port unless you know what it is
> > from the PNP ID.
> >
> > Winbond CIR 00:04: Region 0x2f8-0x2ff already in use!
> > Winbond CIR 00:04: disabled
> > Winbond CIR: probe of 00:04 failed with error -16
> >
> > Signed-off-by: Sean Young <sean@mess.org>
> > ---
> > drivers/media/rc/winbond-cir.c | 21 ++++++++++++++++++++-
> > drivers/tty/serial/8250/8250.c | 1 +
> > 2 files changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
> > index 54ee348..20a0bbb 100644
> > --- a/drivers/media/rc/winbond-cir.c
> > +++ b/drivers/media/rc/winbond-cir.c
> > @@ -55,6 +55,7 @@
> > #include <linux/slab.h>
> > #include <linux/wait.h>
> > #include <linux/sched.h>
> > +#include <linux/serial_8250.h>
> > #include <media/rc-core.h>
> >
> > #define DRVNAME "winbond-cir"
> > @@ -957,6 +958,7 @@ wbcir_probe(struct pnp_dev *device, const struct pnp_device_id *dev_id)
> > struct device *dev = &device->dev;
> > struct wbcir_data *data;
> > int err;
> > + struct resource *io;
> >
> > if (!(pnp_port_len(device, 0) == EHFUNC_IOMEM_LEN &&
> > pnp_port_len(device, 1) == WAKEUP_IOMEM_LEN &&
> > @@ -1049,7 +1051,24 @@ wbcir_probe(struct pnp_dev *device, const struct pnp_device_id *dev_id)
> > goto exit_release_wbase;
> > }
> >
> > - if (!request_region(data->sbase, SP_IOMEM_LEN, DRVNAME)) {
> > + io = request_region(data->sbase, SP_IOMEM_LEN, DRVNAME);
> > +
> > + /*
> > + * The winbond cir device looks exactly like an NS16550A serial port
> > + * unless you know what it is. We've got here via the PNP ID.
> > + */
> > +#ifdef CONFIG_SERIAL_8250
> > + if (!io) {
> > + struct uart_port port = { .iobase = data->sbase };
> > + int line = serial8250_find_port(&port);
> > + if (line >= 0) {
> > + serial8250_unregister_port(line);
> > +
> > + io = request_region(data->sbase, SP_IOMEM_LEN, DRVNAME);
> > + }
> > + }
> > +#endif
> > + if (!io) {
> > dev_err(dev, "Region 0x%lx-0x%lx already in use!\n",
> > data->sbase, data->sbase + SP_IOMEM_LEN - 1);
> > err = -EBUSY;
> > diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
> > index 5c27f7e..d38615f 100644
> > --- a/drivers/tty/serial/8250/8250.c
> > +++ b/drivers/tty/serial/8250/8250.c
> > @@ -2914,6 +2914,7 @@ int serial8250_find_port(struct uart_port *p)
> > }
> > return -ENODEV;
> > }
> > +EXPORT_SYMBOL(serial8250_find_port);
>
> EXPORT_SYMBOL_GPL please.
Ah yes.
> But can't this be done as a quirk to the 8250 driver so that it just
> does not bind to this device in the first place? Wouldn't that make
> more sense?
Absolutely. However for such a quirk to work, it'll need to detect the
IR port. The problem is, based just the I/O ports for the serial port,
it seems impossible. I've been banging my head against a brick wall
trying to find a way to do this. As soon as you start looking at
the other I/O ports for the IR then it becomes clear, but you'll need
to glean from PNP what the port numbers are. Either that or some sort
of Super I/O detection.
Sean
^ permalink raw reply
* [PATCHv3 1/1] [RFC] uartclk from serial_core exposed to sysfs
From: Tomas Hlavacek @ 2012-08-17 14:43 UTC (permalink / raw)
To: gregkh, alan, linux-serial, linux-kernel; +Cc: marek.vasut, Tomas Hlavacek
In-Reply-To: <1344929718-22736-1-git-send-email-tmshlvck@gmail.com>
Added file /sys/devices/.../tty/ttySX/uartclk to allow reading
uartclk value in struct uart_port in serial_core via sysfs.
It simplifies initialization verification of no-name cards that
have non-standard oscillator speed while having no distinguishing
PCI IDs to allow autodetection.
Signed-off-by: Tomas Hlavacek <tmshlvck@gmail.com>
---
drivers/tty/serial/serial_core.c | 32 ++++++++++++++++++++++++++++++++
drivers/tty/tty_io.c | 17 +++++++++++++++++
include/linux/tty.h | 2 ++
3 files changed, 51 insertions(+)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index a21dc8e..454e9d3 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2293,6 +2293,24 @@ struct tty_driver *uart_console_device(struct console *co, int *index)
return p->tty_driver;
}
+static ssize_t uart_get_attr_uartclk(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int ret;
+
+ struct tty_port *port = dev_get_drvdata(dev);
+ struct uart_state *state = container_of(port, struct uart_state, port);
+
+ mutex_lock(&state->port.mutex);
+ ret = snprintf(buf, PAGE_SIZE, "%d\n", state->uart_port->uartclk);
+ mutex_unlock(&state->port.mutex);
+
+ return ret;
+}
+
+static DEVICE_ATTR(uartclk, S_IRUSR | S_IRGRP, uart_get_attr_uartclk,
+ NULL);
+
/**
* uart_add_one_port - attach a driver-defined port structure
* @drv: pointer to the uart low level driver structure for this port
@@ -2355,6 +2373,14 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
}
/*
+ * Expose uartclk in sysfs. Use driverdata of the tty device for
+ * referencing the UART port.
+ */
+ dev_set_drvdata(tty_dev, port);
+ if (device_create_file(tty_dev, &dev_attr_uartclk) < 0)
+ dev_err(tty_dev, "Failed to add uartclk attr\n");
+
+ /*
* Ensure UPF_DEAD is not set.
*/
uport->flags &= ~UPF_DEAD;
@@ -2397,6 +2423,12 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport)
mutex_unlock(&port->mutex);
/*
+ * Remove uartclk file from sysfs.
+ */
+ device_remove_file(tty_lookup_device(drv->tty_driver, uport->line),
+ &dev_attr_uartclk);
+
+ /*
* Remove the devices from the tty layer
*/
tty_unregister_device(drv->tty_driver, uport->line);
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index b425c79..8ea8622 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3049,6 +3049,23 @@ void tty_unregister_device(struct tty_driver *driver, unsigned index)
}
EXPORT_SYMBOL(tty_unregister_device);
+/*
+ * tty_lookup_device - lookup a tty device
+ * @driver: the tty driver that describes the tty device
+ * @index: the index in the tty driver for this tty device
+ *
+ * This function returns a struct device pointer when device has
+ * been found and NULL otherwise.
+ *
+ * Locking: ??
+ */
+struct device *tty_lookup_device(struct tty_driver *driver, unsigned index)
+{
+ dev_t devt = MKDEV(driver->major, driver->minor_start) + index;
+ return class_find_device(tty_class, NULL, &devt, dev_match_devt);
+}
+EXPORT_SYMBOL(tty_lookup_device);
+
struct tty_driver *__alloc_tty_driver(int lines, struct module *owner)
{
struct tty_driver *driver;
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 9f47ab5..5d408a1 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -410,6 +410,8 @@ extern int tty_register_driver(struct tty_driver *driver);
extern int tty_unregister_driver(struct tty_driver *driver);
extern struct device *tty_register_device(struct tty_driver *driver,
unsigned index, struct device *dev);
+extern struct device *tty_lookup_device(struct tty_driver *driver,
+ unsigned index);
extern void tty_unregister_device(struct tty_driver *driver, unsigned index);
extern int tty_read_raw_data(struct tty_struct *tty, unsigned char *bufp,
int buflen);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCHv3 1/1] [RFC] uartclk from serial_core exposed to sysfs
From: Alan Cox @ 2012-08-17 15:07 UTC (permalink / raw)
To: Tomas Hlavacek; +Cc: gregkh, linux-serial, marek.vasut
In-Reply-To: <1345214637-4189-1-git-send-email-tmshlvck@gmail.com>
On Fri, 17 Aug 2012 16:43:57 +0200
Tomas Hlavacek <tmshlvck@gmail.com> wrote:
> Added file /sys/devices/.../tty/ttySX/uartclk to allow reading
> uartclk value in struct uart_port in serial_core via sysfs.
>
> It simplifies initialization verification of no-name cards that
> have non-standard oscillator speed while having no distinguishing
> PCI IDs to allow autodetection.
>
> Signed-off-by: Tomas Hlavacek <tmshlvck@gmail.com>
Acked-by: Alan Cox <alan@linux.intel.com>
^ permalink raw reply
* Re: [PATCHv3 1/1] [RFC] uartclk from serial_core exposed to sysfs
From: Greg KH @ 2012-08-17 15:06 UTC (permalink / raw)
To: Tomas Hlavacek; +Cc: alan, linux-serial, linux-kernel, marek.vasut
In-Reply-To: <1345214637-4189-1-git-send-email-tmshlvck@gmail.com>
On Fri, Aug 17, 2012 at 04:43:57PM +0200, Tomas Hlavacek wrote:
> Added file /sys/devices/.../tty/ttySX/uartclk to allow reading
> uartclk value in struct uart_port in serial_core via sysfs.
Whenever you add/remove/modify a sysfs file, you also need to add an
update to Documentation/ABI/ as well.
Please add this to the patch and resend.
> It simplifies initialization verification of no-name cards that
> have non-standard oscillator speed while having no distinguishing
> PCI IDs to allow autodetection.
>
> Signed-off-by: Tomas Hlavacek <tmshlvck@gmail.com>
> ---
> drivers/tty/serial/serial_core.c | 32 ++++++++++++++++++++++++++++++++
> drivers/tty/tty_io.c | 17 +++++++++++++++++
> include/linux/tty.h | 2 ++
> 3 files changed, 51 insertions(+)
>
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index a21dc8e..454e9d3 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -2293,6 +2293,24 @@ struct tty_driver *uart_console_device(struct console *co, int *index)
> return p->tty_driver;
> }
>
> +static ssize_t uart_get_attr_uartclk(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + int ret;
> +
> + struct tty_port *port = dev_get_drvdata(dev);
> + struct uart_state *state = container_of(port, struct uart_state, port);
> +
> + mutex_lock(&state->port.mutex);
> + ret = snprintf(buf, PAGE_SIZE, "%d\n", state->uart_port->uartclk);
> + mutex_unlock(&state->port.mutex);
> +
> + return ret;
> +}
> +
> +static DEVICE_ATTR(uartclk, S_IRUSR | S_IRGRP, uart_get_attr_uartclk,
> + NULL);
> +
> /**
> * uart_add_one_port - attach a driver-defined port structure
> * @drv: pointer to the uart low level driver structure for this port
> @@ -2355,6 +2373,14 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
> }
>
> /*
> + * Expose uartclk in sysfs. Use driverdata of the tty device for
> + * referencing the UART port.
> + */
> + dev_set_drvdata(tty_dev, port);
> + if (device_create_file(tty_dev, &dev_attr_uartclk) < 0)
> + dev_err(tty_dev, "Failed to add uartclk attr\n");
I think you just raced with userspace in creating the file after the
device was announced to userspace. Are you sure it's ok?
If not (hint, I don't think so), please make it a default attribute of
the device, which will then cause the file to be created before it is
announced to userspace. It will also be less code as you don't have to
clean it up by hand :)
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -3049,6 +3049,23 @@ void tty_unregister_device(struct tty_driver *driver, unsigned index)
> }
> EXPORT_SYMBOL(tty_unregister_device);
>
> +/*
> + * tty_lookup_device - lookup a tty device
> + * @driver: the tty driver that describes the tty device
> + * @index: the index in the tty driver for this tty device
> + *
> + * This function returns a struct device pointer when device has
> + * been found and NULL otherwise.
> + *
> + * Locking: ??
> + */
> +struct device *tty_lookup_device(struct tty_driver *driver, unsigned index)
> +{
> + dev_t devt = MKDEV(driver->major, driver->minor_start) + index;
> + return class_find_device(tty_class, NULL, &devt, dev_match_devt);
> +}
> +EXPORT_SYMBOL(tty_lookup_device);
EXPORT_SYMBOL_GPL as you are just wrapping a class_find_device() call?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCHv3 1/1] [RFC] uartclk from serial_core exposed to sysfs
From: Tomas Hlavacek @ 2012-08-17 16:30 UTC (permalink / raw)
To: Greg KH; +Cc: alan, linux-serial, linux-kernel, marek.vasut
In-Reply-To: <20120817150603.GA13684@kroah.com>
Hello Greg!
On Fri, Aug 17, 2012 at 5:06 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> @@ -2355,6 +2373,14 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
>> }
>>
>> /*
>> + * Expose uartclk in sysfs. Use driverdata of the tty device for
>> + * referencing the UART port.
>> + */
>> + dev_set_drvdata(tty_dev, port);
>> + if (device_create_file(tty_dev, &dev_attr_uartclk) < 0)
>> + dev_err(tty_dev, "Failed to add uartclk attr\n");
>
> I think you just raced with userspace in creating the file after the
> device was announced to userspace. Are you sure it's ok?
>
> If not (hint, I don't think so), please make it a default attribute of
> the device, which will then cause the file to be created before it is
> announced to userspace. It will also be less code as you don't have to
> clean it up by hand :)
Do you mean I should modify the tty_register_device() function not to
use device_create() but it should rather do the device initialization
on it's own. And I should add add the attribute (via struct
attribute_group) to struct device in between device_initialize() and
device_add() calls. Did I get it right?
Thanks,
Tomas
^ permalink raw reply
* Re: [PATCHv3 1/1] [RFC] uartclk from serial_core exposed to sysfs
From: Greg KH @ 2012-08-17 16:54 UTC (permalink / raw)
To: Tomas Hlavacek; +Cc: alan, linux-serial, linux-kernel, marek.vasut
In-Reply-To: <CAEB7QLBEdNArtKz9zP7R4Za54049jJqCEqhPvE6rS59j1tD0PQ@mail.gmail.com>
On Fri, Aug 17, 2012 at 06:30:36PM +0200, Tomas Hlavacek wrote:
> Hello Greg!
>
> On Fri, Aug 17, 2012 at 5:06 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> @@ -2355,6 +2373,14 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
> >> }
> >>
> >> /*
> >> + * Expose uartclk in sysfs. Use driverdata of the tty device for
> >> + * referencing the UART port.
> >> + */
> >> + dev_set_drvdata(tty_dev, port);
> >> + if (device_create_file(tty_dev, &dev_attr_uartclk) < 0)
> >> + dev_err(tty_dev, "Failed to add uartclk attr\n");
> >
> > I think you just raced with userspace in creating the file after the
> > device was announced to userspace. Are you sure it's ok?
> >
> > If not (hint, I don't think so), please make it a default attribute of
> > the device, which will then cause the file to be created before it is
> > announced to userspace. It will also be less code as you don't have to
> > clean it up by hand :)
>
> Do you mean I should modify the tty_register_device() function not to
> use device_create() but it should rather do the device initialization
> on it's own.
No, not at all.
> And I should add add the attribute (via struct attribute_group) to
> struct device in between device_initialize() and device_add() calls.
> Did I get it right?
No, make this a driver attribute, that way when the device is
registered, it adds the attribute automagically to the device that is
bound to it.
Does that make sense?
greg k-h
^ permalink raw reply
* [GIT PATCH] Serial fixes for 3.6-rc3
From: Greg KH @ 2012-08-17 18:10 UTC (permalink / raw)
To: Linus Torvalds, Alan Cox, Jiri Slaby
Cc: Andrew Morton, linux-kernel, linux-serial
The following changes since commit 0d7614f09c1ebdbaa1599a5aba7593f147bf96ee:
Linux 3.6-rc1 (2012-08-02 16:38:10 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/ tags/tty-3.6-rc3
for you to fetch changes up to 38f8eefccf3a23c4058a570fa2938a4f553cf8e0:
pmac_zilog,kdb: Fix console poll hook to return instead of loop (2012-08-16 12:20:20 -0700)
----------------------------------------------------------------
TTY fixes for 3.6-rc3
Here are 4 tiny patches, each fixing a serial driver problem that people
have reported.
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
----------------------------------------------------------------
Alexander Shiyan (1):
serial: Change Kconfig entry for CLPS711X-target
Fengguang Wu (1):
serial: ifx6x60: fix paging fault on spi_register_driver
Huang Shijie (1):
serial: mxs-auart: fix the wrong RTS hardware flow control
Jason Wessel (1):
pmac_zilog,kdb: Fix console poll hook to return instead of loop
drivers/tty/serial/Kconfig | 10 +++++-----
drivers/tty/serial/ifx6x60.c | 2 +-
drivers/tty/serial/mxs-auart.c | 14 +++++++++-----
drivers/tty/serial/pmac_zilog.c | 12 +++++++++---
4 files changed, 24 insertions(+), 14 deletions(-)
^ 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