* [PATCH 4/4] serial: enable serdev support
From: Johan Hovold @ 2017-04-11 17:07 UTC (permalink / raw)
To: Rob Herring, Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, Johan Hovold
In-Reply-To: <20170411170731.4085-1-johan@kernel.org>
Enable serdev support by using the new device-registration helpers.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/tty/serial/serial_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 3fe56894974a..9f5eb0ced359 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2785,7 +2785,7 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
* Register the port whether it's detected or not. This allows
* setserial to be used to alter this port's parameters.
*/
- tty_dev = tty_port_register_device_attr(port, drv->tty_driver,
+ tty_dev = tty_port_register_device_attr_serdev(port, drv->tty_driver,
uport->line, uport->dev, port, uport->tty_groups);
if (likely(!IS_ERR(tty_dev))) {
device_set_wakeup_capable(tty_dev, 1);
@@ -2848,7 +2848,7 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport)
/*
* Remove the devices from the tty layer
*/
- tty_unregister_device(drv->tty_driver, uport->line);
+ tty_port_unregister_device(port, drv->tty_driver, uport->line);
tty = tty_port_tty_get(port);
if (tty) {
--
2.12.2
^ permalink raw reply related
* [PATCH 3/4] tty/serdev: add serdev registration interface
From: Johan Hovold @ 2017-04-11 17:07 UTC (permalink / raw)
To: Rob Herring, Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, Johan Hovold
In-Reply-To: <20170411170731.4085-1-johan@kernel.org>
Add a new interface for registering a serdev controller and clients, and
a helper function to deregister serdev devices (or a tty device) that
were previously registered using the new interface.
Once every driver currently using the tty_port_register_device() helpers
have been vetted and converted to use the new serdev registration
interface (at least for deregistration), we can move serdev registration
to the current helpers and get rid of the serdev-specific functions.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/tty/serdev/serdev-ttyport.c | 6 ++-
drivers/tty/tty_port.c | 76 +++++++++++++++++++++++++++++++++++++
include/linux/serdev.h | 7 +++-
include/linux/tty.h | 9 +++++
4 files changed, 94 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index 06e17faa6dfb..8f9d7a0c27d1 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -216,16 +216,18 @@ struct device *serdev_tty_port_register(struct tty_port *port,
return ERR_PTR(ret);
}
-void serdev_tty_port_unregister(struct tty_port *port)
+int serdev_tty_port_unregister(struct tty_port *port)
{
struct serdev_controller *ctrl = port->client_data;
struct serport *serport = serdev_controller_get_drvdata(ctrl);
if (!serport)
- return;
+ return -ENODEV;
serdev_controller_remove(ctrl);
port->client_ops = NULL;
port->client_data = NULL;
serdev_controller_put(ctrl);
+
+ return 0;
}
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index 0c880f17d27e..2b4c62189346 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -16,6 +16,7 @@
#include <linux/bitops.h>
#include <linux/delay.h>
#include <linux/module.h>
+#include <linux/serdev.h>
static int tty_port_default_receive_buf(struct tty_port *port,
const unsigned char *p,
@@ -134,6 +135,81 @@ struct device *tty_port_register_device_attr(struct tty_port *port,
}
EXPORT_SYMBOL_GPL(tty_port_register_device_attr);
+/**
+ * tty_port_register_device_attr_serdev - register tty or serdev device
+ * @port: tty_port of the device
+ * @driver: tty_driver for this device
+ * @index: index of the tty
+ * @device: parent if exists, otherwise NULL
+ * @drvdata: driver data for the device
+ * @attr_grp: attribute group for the device
+ *
+ * Register a serdev or tty device depending on if the parent device has any
+ * defined serdev clients or not.
+ */
+struct device *tty_port_register_device_attr_serdev(struct tty_port *port,
+ struct tty_driver *driver, unsigned index,
+ struct device *device, void *drvdata,
+ const struct attribute_group **attr_grp)
+{
+ struct device *dev;
+
+ tty_port_link_device(port, driver, index);
+
+ dev = serdev_tty_port_register(port, device, driver, index);
+ if (PTR_ERR(dev) != -ENODEV) {
+ /* Skip creating cdev if we registered a serdev device */
+ return dev;
+ }
+
+ return tty_register_device_attr(driver, index, device, drvdata,
+ attr_grp);
+}
+EXPORT_SYMBOL_GPL(tty_port_register_device_attr_serdev);
+
+/**
+ * tty_port_register_device_serdev - register tty or serdev device
+ * @port: tty_port of the device
+ * @driver: tty_driver for this device
+ * @index: index of the tty
+ * @device: parent if exists, otherwise NULL
+ *
+ * Register a serdev or tty device depending on if the parent device has any
+ * defined serdev clients or not.
+ */
+struct device *tty_port_register_device_serdev(struct tty_port *port,
+ struct tty_driver *driver, unsigned index,
+ struct device *device)
+{
+ return tty_port_register_device_attr_serdev(port, driver, index,
+ device, NULL, NULL);
+}
+EXPORT_SYMBOL_GPL(tty_port_register_device_serdev);
+
+
+/**
+ * tty_port_unregister_device - deregister a tty or serdev device
+ * @port: tty_port of the device
+ * @driver: tty_driver for this device
+ * @index: index of the tty
+ *
+ * If a tty or serdev device is registered with a call to
+ * tty_port_register_device_serdev() then this function must be called when
+ * the device is gone.
+ */
+void tty_port_unregister_device(struct tty_port *port,
+ struct tty_driver *driver, unsigned index)
+{
+ int ret;
+
+ ret = serdev_tty_port_unregister(port);
+ if (ret == 0)
+ return;
+
+ tty_unregister_device(driver, index);
+}
+EXPORT_SYMBOL_GPL(tty_port_unregister_device);
+
int tty_port_alloc_xmit_buf(struct tty_port *port)
{
/* We may sleep in get_zeroed_page() */
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index 9519da6253a8..d612953c63c2 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -248,7 +248,7 @@ struct tty_driver;
struct device *serdev_tty_port_register(struct tty_port *port,
struct device *parent,
struct tty_driver *drv, int idx);
-void serdev_tty_port_unregister(struct tty_port *port);
+int serdev_tty_port_unregister(struct tty_port *port);
#else
static inline struct device *serdev_tty_port_register(struct tty_port *port,
struct device *parent,
@@ -256,7 +256,10 @@ static inline struct device *serdev_tty_port_register(struct tty_port *port,
{
return ERR_PTR(-ENODEV);
}
-static inline void serdev_tty_port_unregister(struct tty_port *port) {}
+static inline int serdev_tty_port_unregister(struct tty_port *port)
+{
+ return -ENODEV;
+}
#endif /* CONFIG_SERIAL_DEV_CTRL_TTYPORT */
#endif /*_LINUX_SERDEV_H */
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 1017e904c0a3..0a3eeca4761a 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -555,6 +555,15 @@ extern struct device *tty_port_register_device_attr(struct tty_port *port,
struct tty_driver *driver, unsigned index,
struct device *device, void *drvdata,
const struct attribute_group **attr_grp);
+extern struct device *tty_port_register_device_serdev(struct tty_port *port,
+ struct tty_driver *driver, unsigned index,
+ struct device *device);
+extern struct device *tty_port_register_device_attr_serdev(struct tty_port *port,
+ struct tty_driver *driver, unsigned index,
+ struct device *device, void *drvdata,
+ const struct attribute_group **attr_grp);
+extern void tty_port_unregister_device(struct tty_port *port,
+ struct tty_driver *driver, unsigned index);
extern int tty_port_alloc_xmit_buf(struct tty_port *port);
extern void tty_port_free_xmit_buf(struct tty_port *port);
extern void tty_port_destroy(struct tty_port *port);
--
2.12.2
^ permalink raw reply related
* [PATCH 2/4] serdev: fix tty-port client deregistration
From: Johan Hovold @ 2017-04-11 17:07 UTC (permalink / raw)
To: Rob Herring, Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, Johan Hovold
In-Reply-To: <20170411170731.4085-1-johan@kernel.org>
The port client data must be set when registering the serdev controller
or client deregistration will fail (and the serdev devices are left
registered and allocated) if the port was never opened in between.
Make sure to clear the port client data on any probe errors to avoid a
use-after-free when the client is later deregistered unconditionally
(e.g. in a tty-port deregistration helper).
Also move port client operation initialisation to registration. Note
that the client ops must be restored on failed probe.
Fixes: bed35c6dfa6a ("serdev: add a tty port controller driver")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/tty/serdev/serdev-ttyport.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index d05393594f15..06e17faa6dfb 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -101,9 +101,6 @@ static int ttyport_open(struct serdev_controller *ctrl)
return PTR_ERR(tty);
serport->tty = tty;
- serport->port->client_ops = &client_ops;
- serport->port->client_data = ctrl;
-
if (tty->ops->open)
tty->ops->open(serport->tty, NULL);
else
@@ -181,6 +178,7 @@ struct device *serdev_tty_port_register(struct tty_port *port,
struct device *parent,
struct tty_driver *drv, int idx)
{
+ const struct tty_port_client_operations *old_ops;
struct serdev_controller *ctrl;
struct serport *serport;
int ret;
@@ -199,15 +197,22 @@ struct device *serdev_tty_port_register(struct tty_port *port,
ctrl->ops = &ctrl_ops;
+ old_ops = port->client_ops;
+ port->client_ops = &client_ops;
+ port->client_data = ctrl;
+
ret = serdev_controller_add(ctrl);
if (ret)
- goto err_controller_put;
+ goto err_reset_data;
dev_info(&ctrl->dev, "tty port %s%d registered\n", drv->name, idx);
return &ctrl->dev;
-err_controller_put:
+err_reset_data:
+ port->client_data = NULL;
+ port->client_ops = old_ops;
serdev_controller_put(ctrl);
+
return ERR_PTR(ret);
}
--
2.12.2
^ permalink raw reply related
* [PATCH 1/4] Revert "tty_port: register tty ports with serdev bus"
From: Johan Hovold @ 2017-04-11 17:07 UTC (permalink / raw)
To: Rob Herring, Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, Johan Hovold
In-Reply-To: <20170411170731.4085-1-johan@kernel.org>
This reverts commit 8ee3fde047589dc9c201251f07d0ca1dc776feca.
The new serdev bus hooked into the tty layer in
tty_port_register_device() by registering a serdev controller instead of
a tty device whenever a serdev client is present, and by deregistering
the controller in the tty-port destructor. This is broken in several
ways:
Firstly, it leads to a NULL-pointer dereference whenever a tty driver
later deregisters its devices as no corresponding character device will
exist.
Secondly, far from every tty driver uses tty-port refcounting (e.g.
serial core) so the serdev devices might never be deregistered or
deallocated.
Thirdly, deregistering at tty-port destruction is too late as the
underlying device and structures may be long gone by then. A port is not
released before an open tty device is closed, something which a
registered serdev client can prevent from ever happening. A driver
callback while the device is gone typically also leads to crashes.
Many tty drivers even keep their ports around until the driver is
unloaded (e.g. serial core), something which even if a late callback
never happens, leads to leaks if a device is unbound from its driver and
is later rebound.
The right solution here is to add a new tty_port_unregister_device()
helper and to never call tty_device_unregister() whenever the port has
been claimed by serdev, but since this requires modifying just about
every tty driver (and multiple subsystems) it will need to be done
incrementally.
Reverting the offending patch is the first step in fixing the broken
lifetime assumptions. A follow-up patch will add a new pair of
tty-device registration helpers, which a vetted tty driver can use to
support serdev (initially serial core). When every tty driver uses the
serdev helpers (at least for deregistration), we can add serdev
registration to tty_port_register_device() again.
Note that this also fixes another issue with serdev, which currently
allocates and registers a serdev controller for every tty device
registered using tty_port_device_register() only to immediately
deregister and deallocate it when the corresponding OF node or serdev
child node is missing. This should be addressed before enabling serdev
for hot-pluggable buses.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/tty/tty_port.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index 1d21a9c1d33e..0c880f17d27e 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -16,7 +16,6 @@
#include <linux/bitops.h>
#include <linux/delay.h>
#include <linux/module.h>
-#include <linux/serdev.h>
static int tty_port_default_receive_buf(struct tty_port *port,
const unsigned char *p,
@@ -129,15 +128,7 @@ struct device *tty_port_register_device_attr(struct tty_port *port,
struct device *device, void *drvdata,
const struct attribute_group **attr_grp)
{
- struct device *dev;
-
tty_port_link_device(port, driver, index);
-
- dev = serdev_tty_port_register(port, device, driver, index);
- if (PTR_ERR(dev) != -ENODEV)
- /* Skip creating cdev if we registered a serdev device */
- return dev;
-
return tty_register_device_attr(driver, index, device, drvdata,
attr_grp);
}
@@ -189,9 +180,6 @@ static void tty_port_destructor(struct kref *kref)
/* check if last port ref was dropped before tty release */
if (WARN_ON(port->itty))
return;
-
- serdev_tty_port_unregister(port);
-
if (port->xmit_buf)
free_page((unsigned long)port->xmit_buf);
tty_port_destroy(port);
--
2.12.2
^ permalink raw reply related
* [PATCH 0/4] serdev: fix broken lifetime assumptions
From: Johan Hovold @ 2017-04-11 17:07 UTC (permalink / raw)
To: Rob Herring, Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, Johan Hovold
This series fixes a number of issues with the new serdev code, which was
based on incorrect tty-port lifetime assumptions.
The first patch disables serdev support by reverting the patch which
hooked into the tty layer in a broken way that leads to crashes and
leaks when deregistering devices. This one should probably go into 4.11.
The second patch fixes a specific bug in the tty-port client
registration code, while the third patch adds a new interface for
registering serdev devices. The final patch ultimately enables serdev
again for the serial drivers.
More details can be found in the individual commit messages.
Johan
Johan Hovold (4):
Revert "tty_port: register tty ports with serdev bus"
serdev: fix tty-port client deregistration
tty/serdev: add serdev registration interface
serial: enable serdev support
drivers/tty/serdev/serdev-ttyport.c | 21 +++++++----
drivers/tty/serial/serial_core.c | 4 +-
drivers/tty/tty_port.c | 74 ++++++++++++++++++++++++++++++++++---
include/linux/serdev.h | 7 +++-
include/linux/tty.h | 9 +++++
5 files changed, 99 insertions(+), 16 deletions(-)
--
2.12.2
^ permalink raw reply
* Re: [PATCHv3 00/10] Nokia H4+ support
From: Greg Kroah-Hartman @ 2017-04-11 14:06 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Sebastian Reichel, Gustavo F. Padovan, Johan Hedberg,
Samuel Thibault, Pavel Machek, Tony Lindgren, Jiri Slaby,
Mark Rutland, open list:BLUETOOTH DRIVERS,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David S. Miller, Rob Herring
In-Reply-To: <4FEFA1D8-BE23-40FB-BAA3-3EC910FE9F96-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
On Tue, Apr 11, 2017 at 01:36:44PM +0200, Marcel Holtmann wrote:
> Hi Sebastian,
>
> >>>>>> Here is PATCHv3 for the Nokia bluetooth patchset. I addressed all comments from
> >>>>>> Rob and Pavel regarding the serdev patches and dropped the *.dts patches, since
> >>>>>> they were queued by Tony. I also changed the patch order, so that the serdev
> >>>>>> patches come first. All of them have Acked-by from Rob, so I think it makes
> >>>>>> sense to merge them to serdev subsystem (now) and provide an immutable branch
> >>>>>> for the bluetooth subsystem.
> >>>>>
> >>>>> Greg doesn't read cover letters generally and since the serdev patches
> >>>>> are Cc rather than To him, he's probably not planning to pick them up.
> >>>>
> >>>> I wonder actually if we should merge all of these via bluetooth-next
> >>>> tree with proper Ack from Greg. However it would be good to also get
> >>>> buy in from Dave for merging this ultimately through net-next.
> >>>
> >>> I don't really care where it goes. I can take the whole thing in my
> >>> tty/serial tree now if no one objects and I get an ack from the relevant
> >>> maintainers {hint...}
> >>
> >> I think it is better if it goes thru BT tree. I have another driver
> >> converted that is dependent on this series. There's a couple other
> >> serdev changes on the list too, but this shouldn't depend on them.
> >
> > Is this waiting for something, or could it be queued to
> > bluetooth-next then? It would be nice to finally have
> > this in 4.12 :)
>
> I would prefer if we can get an ACK from Greg. Then I merge it through the bluetooth-next tree.
Sorry thought this was coming through mine:
Acked-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
Merge away!
greg k-h
^ permalink raw reply
* Re: [PATCH v3 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Andy Shevchenko @ 2017-04-11 12:15 UTC (permalink / raw)
To: Joel Stanley
Cc: Greg Kroah-Hartman, Jiri Slaby, Mark Rutland, Rob Herring,
Jeremy Kerr, linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree,
Benjamin Herrenschmidt, openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <20170410040400.5509-3-joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
On Mon, Apr 10, 2017 at 7:04 AM, Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org> wrote:
> From: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
>
> This change adds a driver for the 16550-based Aspeed virtual UART
> device. We use a similar process to the of_serial driver for device
> probe, but expose some VUART-specific functions through sysfs too.
>
> The VUART is two UART 'front ends' connected by their FIFO (no actual
> serial line in between). One is on the BMC side (management controller)
> and one is on the host CPU side.
>
> This driver is for the BMC side. The sysfs files allow the BMC
> userspace, which owns the system configuration policy, to specify at
> what IO port and interrupt number the host side will appear to the host
> on the Host <-> BMC LPC bus. It could be different on a different system
> (though most of them use 3f8/4).
>
> OpenPOWER host firmware doesn't like it when the host-side of the
> VUART's FIFO is not drained. This driver only disables host TX discard
> mode when the port is in use. We set the VUART enabled bit when we bind
> to the device, and clear it on unbind.
>
> We don't want to do this on open/release, as the host may be using this
> bit to configure serial output modes, which is independent of whether
> the devices has been opened by BMC userspace.
> +static void aspeed_vuart_set_enabled(struct aspeed_vuart *vuart, bool enabled)
> +{
> + u8 reg;
> +
> + reg = readb(vuart->regs + ASPEED_VUART_GCRA);
> + reg &= ~ASPEED_VUART_GCRA_VUART_EN;
> + if (enabled)
> + reg |= ASPEED_VUART_GCRA_VUART_EN;
Usually the pattern is
if (something)
set x bit;
else
clear x bit;
It would make it one operation in any case and a bit more understandable.
> + writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
> +}
> +
> +static void aspeed_vuart_set_host_tx_discard(struct aspeed_vuart *vuart,
> + bool discard)
> +{
> + u8 reg;
> +
> + reg = readb(vuart->regs + ASPEED_VUART_GCRA);
> +
> + /* If the DISABLE_HOST_TX_DISCARD bit is set, discard is disabled */
> + reg &= ~ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
> + if (!discard)
> + reg |= ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
Ditto.
> +
> + writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
> +}
> + /* The 8510 core creates the mapping, which we grab a reference to
> + * for VUART-specific registers */
Hmm... What about multi-line style?
> + port.port.irq = irq_of_parse_and_map(np, 0);
The benefit of use platform_get_irq() is to get rid of some OF specific headers.
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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: [PATCHv3 00/10] Nokia H4+ support
From: Marcel Holtmann @ 2017-04-11 11:36 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Greg Kroah-Hartman, Gustavo F. Padovan, Johan Hedberg,
Samuel Thibault, Pavel Machek, Tony Lindgren, Jiri Slaby,
Mark Rutland, open list:BLUETOOTH DRIVERS,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David S. Miller, Rob Herring
In-Reply-To: <20170410231041.vz35anezpzhscu4t@earth>
Hi Sebastian,
>>>>>> Here is PATCHv3 for the Nokia bluetooth patchset. I addressed all comments from
>>>>>> Rob and Pavel regarding the serdev patches and dropped the *.dts patches, since
>>>>>> they were queued by Tony. I also changed the patch order, so that the serdev
>>>>>> patches come first. All of them have Acked-by from Rob, so I think it makes
>>>>>> sense to merge them to serdev subsystem (now) and provide an immutable branch
>>>>>> for the bluetooth subsystem.
>>>>>
>>>>> Greg doesn't read cover letters generally and since the serdev patches
>>>>> are Cc rather than To him, he's probably not planning to pick them up.
>>>>
>>>> I wonder actually if we should merge all of these via bluetooth-next
>>>> tree with proper Ack from Greg. However it would be good to also get
>>>> buy in from Dave for merging this ultimately through net-next.
>>>
>>> I don't really care where it goes. I can take the whole thing in my
>>> tty/serial tree now if no one objects and I get an ack from the relevant
>>> maintainers {hint...}
>>
>> I think it is better if it goes thru BT tree. I have another driver
>> converted that is dependent on this series. There's a couple other
>> serdev changes on the list too, but this shouldn't depend on them.
>
> Is this waiting for something, or could it be queued to
> bluetooth-next then? It would be nice to finally have
> this in 4.12 :)
I would prefer if we can get an ACK from Greg. Then I merge it through the bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v9 3/3] printk: fix double printing with earlycon
From: Petr Mladek @ 2017-04-11 7:43 UTC (permalink / raw)
To: Aleksey Makarov
Cc: linux-serial, linux-kernel, Sudeep Holla, Greg Kroah-Hartman,
Peter Hurley, Jiri Slaby, Robin Murphy, Steven Rostedt,
Nair, Jayachandran, Sergey Senozhatsky
In-Reply-To: <0514db53-4f8b-fa08-f461-902de85777ee@linaro.org>
On Mon 2017-04-10 21:00:35, Aleksey Makarov wrote:
>
>
> On 04/10/2017 05:22 PM, Petr Mladek wrote:
> >On Wed 2017-04-05 23:20:00, Aleksey Makarov wrote:
> >>If a console was specified by ACPI SPCR table _and_ command line
> >>parameters like "console=ttyAMA0" _and_ "earlycon" were specified,
> >>then log messages appear twice.
> >>
> >>The root cause is that the code traverses the list of specified
> >>consoles (the `console_cmdline` array) and stops at the first match.
> >>But it may happen that the same console is referred by the elements
> >>of this array twice:
> >>
> >> pl011,mmio,0x87e024000000,115200 -- from SPCR
> >> ttyAMA0 -- from command line
> >>
> >>but in this case `preferred_console` points to the second entry and
> >>the flag CON_CONSDEV is not set, so bootconsole is not deregistered.
> >>
> >>To fix that, introduce an invariant "The last non-braille console
> >>is always the preferred one" on the entries of the console_cmdline
> >>array. Then traverse it in reverse order to be sure that if
> >>the console is preferred then it will be the first matching entry.
> >>Introduce variable console_cmdline_cnt that keeps the number
> >>of elements of the console_cmdline array (Petr Mladek). It helps
> >>to get rid of the loop that searches for the end of this array.
> >>
> >>Reported-by: Sudeep Holla <sudeep.holla@arm.com>
> >>Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> >
> >This version looks fine to me. Just a small nitpick below.
> >Anyway:
> >
> >Reviewed-by: Petr Mladek <pmladek@suse.com>
>
> Thank you for review. Can you (or anybody else) ACK it?
> I am going to resend the whole series without those empty lines.
> May I add your Acked-by:?
Sure. Feel free to use:
Acked-by: Petr Mladek <pmladek@suse.com>
The meaning of the tags is a bit unclear. Acked-by means that
the maintainer agrees with the idea. But it does not necessarily
means that she reviewed the code in details. I agree with the idea
and did the review, so I used the Reviewed-by tag.
Also you do not need to resend the patchset just because the two
empty lines. Sergey agrees. I will wait day or two and push
all three patches into the printk.git if nobody complains
in the meantime. I could remove the two empty lines when doing so.
Thanks a lot for the fix and patience.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH v9 3/3] printk: fix double printing with earlycon
From: Sergey Senozhatsky @ 2017-04-11 1:54 UTC (permalink / raw)
To: Aleksey Makarov
Cc: Petr Mladek, linux-serial, linux-kernel, Sudeep Holla,
Greg Kroah-Hartman, Peter Hurley, Jiri Slaby, Robin Murphy,
Steven Rostedt, Nair, Jayachandran, Sergey Senozhatsky
In-Reply-To: <0514db53-4f8b-fa08-f461-902de85777ee@linaro.org>
On (04/10/17 21:00), Aleksey Makarov wrote:
> On 04/10/2017 05:22 PM, Petr Mladek wrote:
> > On Wed 2017-04-05 23:20:00, Aleksey Makarov wrote:
[..]
> > > To fix that, introduce an invariant "The last non-braille console
> > > is always the preferred one" on the entries of the console_cmdline
> > > array. Then traverse it in reverse order to be sure that if
> > > the console is preferred then it will be the first matching entry.
> > > Introduce variable console_cmdline_cnt that keeps the number
> > > of elements of the console_cmdline array (Petr Mladek). It helps
> > > to get rid of the loop that searches for the end of this array.
> > >
> > > Reported-by: Sudeep Holla <sudeep.holla@arm.com>
> > > Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> >
> > This version looks fine to me. Just a small nitpick below.
> > Anyway:
> >
> > Reviewed-by: Petr Mladek <pmladek@suse.com>
>
> Thank you for review. Can you (or anybody else) ACK it?
> I am going to resend the whole series without those empty lines.
> May I add your Acked-by:?
Acked-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
feel free to resend. I know some setups that have repeating (double)
lines on the serial console, I'll ask people to test the patches.
-ss
^ permalink raw reply
* Re: [PATCHv3 00/10] Nokia H4+ support
From: Sebastian Reichel @ 2017-04-10 23:10 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Greg Kroah-Hartman, Gustavo F. Padovan, Johan Hedberg,
Samuel Thibault, Pavel Machek, Tony Lindgren, Jiri Slaby,
Mark Rutland, open list:BLUETOOTH DRIVERS,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David S. Miller, Rob Herring
In-Reply-To: <CAL_Jsq+NU3M5yuBpK1UGgzCVvq0eABMApCqEe3_d5+tDaABsgQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1694 bytes --]
Hi,
On Wed, Apr 05, 2017 at 01:16:58PM -0500, Rob Herring wrote:
> On Fri, Mar 31, 2017 at 8:33 AM, Greg Kroah-Hartman
> <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
> > On Wed, Mar 29, 2017 at 11:33:26PM +0200, Marcel Holtmann wrote:
> >> Hi Rob,
> >>
> >> >> Here is PATCHv3 for the Nokia bluetooth patchset. I addressed all comments from
> >> >> Rob and Pavel regarding the serdev patches and dropped the *.dts patches, since
> >> >> they were queued by Tony. I also changed the patch order, so that the serdev
> >> >> patches come first. All of them have Acked-by from Rob, so I think it makes
> >> >> sense to merge them to serdev subsystem (now) and provide an immutable branch
> >> >> for the bluetooth subsystem.
> >> >
> >> > Greg doesn't read cover letters generally and since the serdev patches
> >> > are Cc rather than To him, he's probably not planning to pick them up.
> >>
> >> I wonder actually if we should merge all of these via bluetooth-next
> >> tree with proper Ack from Greg. However it would be good to also get
> >> buy in from Dave for merging this ultimately through net-next.
> >
> > I don't really care where it goes. I can take the whole thing in my
> > tty/serial tree now if no one objects and I get an ack from the relevant
> > maintainers {hint...}
>
> I think it is better if it goes thru BT tree. I have another driver
> converted that is dependent on this series. There's a couple other
> serdev changes on the list too, but this shouldn't depend on them.
Is this waiting for something, or could it be queued to
bluetooth-next then? It would be nice to finally have
this in 4.12 :)
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v9 3/3] printk: fix double printing with earlycon
From: Aleksey Makarov @ 2017-04-10 18:00 UTC (permalink / raw)
To: Petr Mladek
Cc: linux-serial, linux-kernel, Sudeep Holla, Greg Kroah-Hartman,
Peter Hurley, Jiri Slaby, Robin Murphy, Steven Rostedt,
Nair, Jayachandran, Sergey Senozhatsky
In-Reply-To: <20170410142214.GA3452@pathway.suse.cz>
On 04/10/2017 05:22 PM, Petr Mladek wrote:
> On Wed 2017-04-05 23:20:00, Aleksey Makarov wrote:
>> If a console was specified by ACPI SPCR table _and_ command line
>> parameters like "console=ttyAMA0" _and_ "earlycon" were specified,
>> then log messages appear twice.
>>
>> The root cause is that the code traverses the list of specified
>> consoles (the `console_cmdline` array) and stops at the first match.
>> But it may happen that the same console is referred by the elements
>> of this array twice:
>>
>> pl011,mmio,0x87e024000000,115200 -- from SPCR
>> ttyAMA0 -- from command line
>>
>> but in this case `preferred_console` points to the second entry and
>> the flag CON_CONSDEV is not set, so bootconsole is not deregistered.
>>
>> To fix that, introduce an invariant "The last non-braille console
>> is always the preferred one" on the entries of the console_cmdline
>> array. Then traverse it in reverse order to be sure that if
>> the console is preferred then it will be the first matching entry.
>> Introduce variable console_cmdline_cnt that keeps the number
>> of elements of the console_cmdline array (Petr Mladek). It helps
>> to get rid of the loop that searches for the end of this array.
>>
>> Reported-by: Sudeep Holla <sudeep.holla@arm.com>
>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>
> This version looks fine to me. Just a small nitpick below.
> Anyway:
>
> Reviewed-by: Petr Mladek <pmladek@suse.com>
Thank you for review. Can you (or anybody else) ACK it?
I am going to resend the whole series without those empty lines.
May I add your Acked-by:?
Thank you
Aleksey Makarov
>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> index fd752f0c8ef1..be657af45758 100644
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -269,6 +269,7 @@ static struct console *exclusive_console;
>> #define MAX_CMDLINECONSOLES 8
>>
>> static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES];
>> +static int console_cmdline_cnt;
>>
>> static int preferred_console = -1;
>> int console_set_on_cmdline;
>> @@ -1905,12 +1906,26 @@ static int __add_preferred_console(char *name, int idx, char *options,
>> * See if this tty is not yet registered, and
>> * if we have a slot free.
>> */
>> - for (i = 0, c = console_cmdline;
>> - i < MAX_CMDLINECONSOLES && c->name[0];
>> - i++, c++) {
>> + for (i = 0, c = console_cmdline; i < console_cmdline_cnt; i++, c++) {
>> if (strcmp(c->name, name) == 0 && c->index == idx) {
>> - if (!brl_options)
>> - preferred_console = i;
>> +
>
> This extra new line is non-standard and looks slightly weird to me.
> I just point it out. I personally do not mind ;-)
>
>
>> + if (brl_options)
>> + return 0;
>> +
>> + /*
>> + * Maintain an invariant that will help to find if
>> + * the matching console is preferred, see
>> + * register_console():
>> + *
>> + * The last non-braille console is always
>> + * the preferred one.
>> + */
>> + if (i != console_cmdline_cnt - 1)
>> + swap(console_cmdline[i],
>> + console_cmdline[console_cmdline_cnt - 1]);
>> +
>> + preferred_console = console_cmdline_cnt - 1;
>> +
>> return 0;
>> }
>> }
>> @@ -2457,12 +2473,24 @@ void register_console(struct console *newcon)
>> }
>>
>> /*
>> - * See if this console matches one we selected on
>> - * the command line.
>> + * See if this console matches one we selected on the command line.
>> + *
>> + * There may be several entries in the console_cmdline array matching
>> + * with the same console, one with newcon->match(), another by
>> + * name/index:
>> + *
>> + * pl011,mmio,0x87e024000000,115200 -- added from SPCR
>> + * ttyAMA0 -- added from command line
>> + *
>> + * Traverse the console_cmdline array in reverse order to be
>> + * sure that if this console is preferred then it will be the first
>> + * matching entry. We use the invariant that is maintained in
>> + * __add_preferred_console().
>> */
>> - for (i = 0, c = console_cmdline;
>> - i < MAX_CMDLINECONSOLES && c->name[0];
>> - i++, c++) {
>> + for (i = console_cmdline_cnt - 1; i >= 0; i--) {
>> +
>
> Same here.
>
>> + c = console_cmdline + i;
>> +
>> if (!newcon->match ||
>> newcon->match(newcon, c->name, c->index, c->options) != 0) {
>> /* default matching */
>
> Best Regards,
> Petr
>
^ permalink raw reply
* Re: [PATCHv3 02/10] serdev: add serdev_device_wait_until_sent
From: Sebastian Reichel @ 2017-04-10 17:10 UTC (permalink / raw)
To: Rob Herring
Cc: Greg Kroah-Hartman, Marcel Holtmann, Gustavo Padovan,
Johan Hedberg, Samuel Thibault, Pavel Machek, Tony Lindgren,
Jiri Slaby, Mark Rutland, open list:BLUETOOTH DRIVERS,
linux-serial@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, Andrey Smirnov
In-Reply-To: <CAL_JsqJCjob6NDfHe0r7X3dXPEniKh5vChXhBxfyHZRu7Fov7A@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3810 bytes --]
Hi,
On Mon, Apr 10, 2017 at 11:12:39AM -0500, Rob Herring wrote:
> On Mon, Apr 10, 2017 at 9:03 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Mon, Apr 10, 2017 at 08:46:57AM -0500, Rob Herring wrote:
> >> On Sat, Apr 8, 2017 at 11:57 AM, Greg Kroah-Hartman
> >> <gregkh@linuxfoundation.org> wrote:
> >> > On Tue, Mar 28, 2017 at 05:59:31PM +0200, Sebastian Reichel wrote:
> >> >> Add method, which waits until the transmission buffer has been sent.
> >> >> Note, that the change in ttyport_write_wakeup is related, since
> >> >> tty_wait_until_sent will hang without that change.
> >> >>
> >> >> Acked-by: Rob Herring <robh@kernel.org>
> >> >> Acked-by: Pavel Machek <pavel@ucw.cz>
> >> >> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> >> >> ---
> >> >> Changes since PATCHv2:
> >> >> * Avoid goto in ttyport_write_wakeup
> >> >> ---
> >> >> drivers/tty/serdev/core.c | 11 +++++++++++
> >> >> drivers/tty/serdev/serdev-ttyport.c | 18 ++++++++++++++----
> >> >> include/linux/serdev.h | 3 +++
> >> >> 3 files changed, 28 insertions(+), 4 deletions(-)
> >> >>
> >> >> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> >> >> index f4c6c90add78..a63b74031e22 100644
> >> >> --- a/drivers/tty/serdev/core.c
> >> >> +++ b/drivers/tty/serdev/core.c
> >> >> @@ -173,6 +173,17 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
> >> >> }
> >> >> EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
> >> >>
> >> >> +void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
> >> >> +{
> >> >> + struct serdev_controller *ctrl = serdev->ctrl;
> >> >> +
> >> >> + if (!ctrl || !ctrl->ops->wait_until_sent)
> >> >> + return;
> >> >> +
> >> >> + ctrl->ops->wait_until_sent(ctrl, timeout);
> >> >> +}
> >> >> +EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
> >> >
> >> > Is this still needed now that we have serdev_device_write() with an
> >> > unlimited timeout available?
> >>
> >> Yes, because only this waits until the data is on the wire.
> >
> > What "wire" is that? The serial wire? How do you know this? Many usb
> > to serial devices have no way to determine this, given that there is
> > another uart hanging off of the end of a USB connection.
>
> Okay, maybe it's just out of linux s/w buffers for h/w which you don't
> know. It is the same semantics as tty_wait_until_sent which is
> documented as: "Wait for characters pending in a tty driver to hit the
> wire, or for a timeout to occur (eg due to flow control)"
For embedded h/w it usually means the serial wire. tty_wait_until_sent()
first waits for the tty buffer to be empty and then calls wait_until_sent()
in the driver providing the tty. In case of serial-core that is
implemented by uart_wait_until_sent(), which waits for the serial
driver's tx_empty() operation becoming true (grepping for
".tx_empty" returned 81 hits for me). Also at least some of the usb to
serial adapters seem to support this using usb_serial_generic_wait_until_sent()
and ".tx_empty":
$ git grep "\.tx_empty"
cp210x.c: .tx_empty = cp210x_tx_empty,
f81534.c: .tx_empty = f81534_tx_empty,
ftdi_sio.c: .tx_empty = ftdi_tx_empty,
io_ti.c: .tx_empty = edge_tx_empty,
io_ti.c: .tx_empty = edge_tx_empty,
mxuport.c: .tx_empty = mxuport_tx_empty,
ti_usb_3410_5052.c: .tx_empty = ti_tx_empty,
ti_usb_3410_5052.c: .tx_empty = ti_tx_empty,
The other ones will only wait for empty s/w buffer, but that's
already the case for tty_wait_until_sent(), so IMHO a different
problem.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] serial: omap: suspend device on probe errors
From: Tony Lindgren @ 2017-04-10 16:15 UTC (permalink / raw)
To: Johan Hovold
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-omap,
linux-kernel, Shubhrajyoti D
In-Reply-To: <20170410092139.11441-2-johan@kernel.org>
* Johan Hovold <johan@kernel.org> [170410 02:27]:
> Make sure to actually suspend the device before returning after a failed
> (or deferred) probe.
>
> Note that autosuspend must be disabled before runtime pm is disabled in
> order to balance the usage count due to a negative autosuspend delay as
> well as to make the final put suspend the device synchronously.
>
> Fixes: 388bc2622680 ("omap-serial: Fix the error handling in the omap_serial probe")
> Cc: Shubhrajyoti D <shubhrajyoti@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>
Acked-by: Tony Lindgren <tony@atomide.com>
^ permalink raw reply
* Re: [PATCH 1/2] serial: omap: fix runtime-pm handling on unbind
From: Tony Lindgren @ 2017-04-10 16:15 UTC (permalink / raw)
To: Johan Hovold
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-omap,
linux-kernel, Felipe Balbi, Santosh Shilimkar
In-Reply-To: <20170410092139.11441-1-johan@kernel.org>
* Johan Hovold <johan@kernel.org> [170410 02:27]:
> An unbalanced and misplaced synchronous put was used to suspend the
> device on driver unbind, something which with a likewise misplaced
> pm_runtime_disable leads to external aborts when an open port is being
> removed.
>
> Unhandled fault: external abort on non-linefetch (0x1028) at 0xfa024010
> ...
> [<c046e760>] (serial_omap_set_mctrl) from [<c046a064>] (uart_update_mctrl+0x50/0x60)
> [<c046a064>] (uart_update_mctrl) from [<c046a400>] (uart_shutdown+0xbc/0x138)
> [<c046a400>] (uart_shutdown) from [<c046bd2c>] (uart_hangup+0x94/0x190)
> [<c046bd2c>] (uart_hangup) from [<c045b760>] (__tty_hangup+0x404/0x41c)
> [<c045b760>] (__tty_hangup) from [<c045b794>] (tty_vhangup+0x1c/0x20)
> [<c045b794>] (tty_vhangup) from [<c046ccc8>] (uart_remove_one_port+0xec/0x260)
> [<c046ccc8>] (uart_remove_one_port) from [<c046ef4c>] (serial_omap_remove+0x40/0x60)
> [<c046ef4c>] (serial_omap_remove) from [<c04845e8>] (platform_drv_remove+0x34/0x4c)
>
> Fix this up by resuming the device before deregistering the port and by
> suspending and disabling runtime pm only after the port has been
> removed.
>
> Also make sure to disable autosuspend before disabling runtime pm so
> that the usage count is balanced and device actually suspended before
> returning.
>
> Note that due to a negative autosuspend delay being set in probe, the
> unbalanced put would actually suspend the device on first driver unbind,
> while rebinding and again unbinding would result in a negative
> power.usage_count.
>
> Fixes: 7e9c8e7dbf3b ("serial: omap: make sure to suspend device before remove")
> Cc: Felipe Balbi <balbi@kernel.org>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>
Acked-by: Tony Lindgren <tony@atomide.com>
^ permalink raw reply
* Re: [PATCHv3 02/10] serdev: add serdev_device_wait_until_sent
From: Rob Herring @ 2017-04-10 16:12 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Sebastian Reichel, Marcel Holtmann, Gustavo Padovan,
Johan Hedberg, Samuel Thibault, Pavel Machek, Tony Lindgren,
Jiri Slaby, Mark Rutland, open list:BLUETOOTH DRIVERS,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Andrey Smirnov
In-Reply-To: <20170410140313.GA31894-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
+ Andrey
On Mon, Apr 10, 2017 at 9:03 AM, Greg Kroah-Hartman
<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
> On Mon, Apr 10, 2017 at 08:46:57AM -0500, Rob Herring wrote:
>> On Sat, Apr 8, 2017 at 11:57 AM, Greg Kroah-Hartman
>> <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
>> > On Tue, Mar 28, 2017 at 05:59:31PM +0200, Sebastian Reichel wrote:
>> >> Add method, which waits until the transmission buffer has been sent.
>> >> Note, that the change in ttyport_write_wakeup is related, since
>> >> tty_wait_until_sent will hang without that change.
>> >>
>> >> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> >> Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
>> >> Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> >> ---
>> >> Changes since PATCHv2:
>> >> * Avoid goto in ttyport_write_wakeup
>> >> ---
>> >> drivers/tty/serdev/core.c | 11 +++++++++++
>> >> drivers/tty/serdev/serdev-ttyport.c | 18 ++++++++++++++----
>> >> include/linux/serdev.h | 3 +++
>> >> 3 files changed, 28 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
>> >> index f4c6c90add78..a63b74031e22 100644
>> >> --- a/drivers/tty/serdev/core.c
>> >> +++ b/drivers/tty/serdev/core.c
>> >> @@ -173,6 +173,17 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
>> >> }
>> >> EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
>> >>
>> >> +void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
>> >> +{
>> >> + struct serdev_controller *ctrl = serdev->ctrl;
>> >> +
>> >> + if (!ctrl || !ctrl->ops->wait_until_sent)
>> >> + return;
>> >> +
>> >> + ctrl->ops->wait_until_sent(ctrl, timeout);
>> >> +}
>> >> +EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
>> >
>> > Is this still needed now that we have serdev_device_write() with an
>> > unlimited timeout available?
>>
>> Yes, because only this waits until the data is on the wire.
>
> What "wire" is that? The serial wire? How do you know this? Many usb
> to serial devices have no way to determine this, given that there is
> another uart hanging off of the end of a USB connection.
Okay, maybe it's just out of linux s/w buffers for h/w which you don't
know. It is the same semantics as tty_wait_until_sent which is
documented as: "Wait for characters pending in a tty driver to hit the
wire, or for a timeout to occur (eg due to flow control)"
> Doesn't serdev_device_write() return when the write is finished?
No, it returns when data is accepted by the tty layer (as serdev has
no buffering of its own).
> I
> think we need some good documentation here for all of the different
> variants of how to send data, as I'm sure confused...
Fair enough. However, I think that's somewhat orthogonal to this function.
There's 2 modes of sending data which are basically sync and async operation.
- serdev_device_write_buf - Only sends what can be immediately
accepted by lower layers. Caller should provide write_wakeup callback
to be notified.
- serdev_device_write - Sleeps until all data is accepted by lower
layers or a timeout occurs.
It's valid to call serdev_device_wait_until_sent in either case.
Yes, the naming is not the best, but we kept serdev_device_write_buf
to avoid any cross tree merges. We can remove or rename later as it is
just a wrapper now for serdev_device_write.
Rob
^ permalink raw reply
* Re: [PATCH v9 3/3] printk: fix double printing with earlycon
From: Petr Mladek @ 2017-04-10 14:22 UTC (permalink / raw)
To: Aleksey Makarov
Cc: linux-serial, linux-kernel, Sudeep Holla, Greg Kroah-Hartman,
Peter Hurley, Jiri Slaby, Robin Murphy, Steven Rostedt,
Nair, Jayachandran, Sergey Senozhatsky
In-Reply-To: <20170405202006.18234-1-aleksey.makarov@linaro.org>
On Wed 2017-04-05 23:20:00, Aleksey Makarov wrote:
> If a console was specified by ACPI SPCR table _and_ command line
> parameters like "console=ttyAMA0" _and_ "earlycon" were specified,
> then log messages appear twice.
>
> The root cause is that the code traverses the list of specified
> consoles (the `console_cmdline` array) and stops at the first match.
> But it may happen that the same console is referred by the elements
> of this array twice:
>
> pl011,mmio,0x87e024000000,115200 -- from SPCR
> ttyAMA0 -- from command line
>
> but in this case `preferred_console` points to the second entry and
> the flag CON_CONSDEV is not set, so bootconsole is not deregistered.
>
> To fix that, introduce an invariant "The last non-braille console
> is always the preferred one" on the entries of the console_cmdline
> array. Then traverse it in reverse order to be sure that if
> the console is preferred then it will be the first matching entry.
> Introduce variable console_cmdline_cnt that keeps the number
> of elements of the console_cmdline array (Petr Mladek). It helps
> to get rid of the loop that searches for the end of this array.
>
> Reported-by: Sudeep Holla <sudeep.holla@arm.com>
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
This version looks fine to me. Just a small nitpick below.
Anyway:
Reviewed-by: Petr Mladek <pmladek@suse.com>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index fd752f0c8ef1..be657af45758 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -269,6 +269,7 @@ static struct console *exclusive_console;
> #define MAX_CMDLINECONSOLES 8
>
> static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES];
> +static int console_cmdline_cnt;
>
> static int preferred_console = -1;
> int console_set_on_cmdline;
> @@ -1905,12 +1906,26 @@ static int __add_preferred_console(char *name, int idx, char *options,
> * See if this tty is not yet registered, and
> * if we have a slot free.
> */
> - for (i = 0, c = console_cmdline;
> - i < MAX_CMDLINECONSOLES && c->name[0];
> - i++, c++) {
> + for (i = 0, c = console_cmdline; i < console_cmdline_cnt; i++, c++) {
> if (strcmp(c->name, name) == 0 && c->index == idx) {
> - if (!brl_options)
> - preferred_console = i;
> +
This extra new line is non-standard and looks slightly weird to me.
I just point it out. I personally do not mind ;-)
> + if (brl_options)
> + return 0;
> +
> + /*
> + * Maintain an invariant that will help to find if
> + * the matching console is preferred, see
> + * register_console():
> + *
> + * The last non-braille console is always
> + * the preferred one.
> + */
> + if (i != console_cmdline_cnt - 1)
> + swap(console_cmdline[i],
> + console_cmdline[console_cmdline_cnt - 1]);
> +
> + preferred_console = console_cmdline_cnt - 1;
> +
> return 0;
> }
> }
> @@ -2457,12 +2473,24 @@ void register_console(struct console *newcon)
> }
>
> /*
> - * See if this console matches one we selected on
> - * the command line.
> + * See if this console matches one we selected on the command line.
> + *
> + * There may be several entries in the console_cmdline array matching
> + * with the same console, one with newcon->match(), another by
> + * name/index:
> + *
> + * pl011,mmio,0x87e024000000,115200 -- added from SPCR
> + * ttyAMA0 -- added from command line
> + *
> + * Traverse the console_cmdline array in reverse order to be
> + * sure that if this console is preferred then it will be the first
> + * matching entry. We use the invariant that is maintained in
> + * __add_preferred_console().
> */
> - for (i = 0, c = console_cmdline;
> - i < MAX_CMDLINECONSOLES && c->name[0];
> - i++, c++) {
> + for (i = console_cmdline_cnt - 1; i >= 0; i--) {
> +
Same here.
> + c = console_cmdline + i;
> +
> if (!newcon->match ||
> newcon->match(newcon, c->name, c->index, c->options) != 0) {
> /* default matching */
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCHv3 02/10] serdev: add serdev_device_wait_until_sent
From: Greg Kroah-Hartman @ 2017-04-10 14:03 UTC (permalink / raw)
To: Rob Herring
Cc: Sebastian Reichel, Marcel Holtmann, Gustavo Padovan,
Johan Hedberg, Samuel Thibault, Pavel Machek, Tony Lindgren,
Jiri Slaby, Mark Rutland, open list:BLUETOOTH DRIVERS,
linux-serial@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <CAL_JsqLK=pbF7Nx-2w98t62N53zbL1oGZpGEbZUK-xg5ms4AZw@mail.gmail.com>
On Mon, Apr 10, 2017 at 08:46:57AM -0500, Rob Herring wrote:
> On Sat, Apr 8, 2017 at 11:57 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Tue, Mar 28, 2017 at 05:59:31PM +0200, Sebastian Reichel wrote:
> >> Add method, which waits until the transmission buffer has been sent.
> >> Note, that the change in ttyport_write_wakeup is related, since
> >> tty_wait_until_sent will hang without that change.
> >>
> >> Acked-by: Rob Herring <robh@kernel.org>
> >> Acked-by: Pavel Machek <pavel@ucw.cz>
> >> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> >> ---
> >> Changes since PATCHv2:
> >> * Avoid goto in ttyport_write_wakeup
> >> ---
> >> drivers/tty/serdev/core.c | 11 +++++++++++
> >> drivers/tty/serdev/serdev-ttyport.c | 18 ++++++++++++++----
> >> include/linux/serdev.h | 3 +++
> >> 3 files changed, 28 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> >> index f4c6c90add78..a63b74031e22 100644
> >> --- a/drivers/tty/serdev/core.c
> >> +++ b/drivers/tty/serdev/core.c
> >> @@ -173,6 +173,17 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
> >> }
> >> EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
> >>
> >> +void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
> >> +{
> >> + struct serdev_controller *ctrl = serdev->ctrl;
> >> +
> >> + if (!ctrl || !ctrl->ops->wait_until_sent)
> >> + return;
> >> +
> >> + ctrl->ops->wait_until_sent(ctrl, timeout);
> >> +}
> >> +EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
> >
> > Is this still needed now that we have serdev_device_write() with an
> > unlimited timeout available?
>
> Yes, because only this waits until the data is on the wire.
What "wire" is that? The serial wire? How do you know this? Many usb
to serial devices have no way to determine this, given that there is
another uart hanging off of the end of a USB connection.
Doesn't serdev_device_write() return when the write is finished? I
think we need some good documentation here for all of the different
variants of how to send data, as I'm sure confused...
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCHv3 02/10] serdev: add serdev_device_wait_until_sent
From: Rob Herring @ 2017-04-10 13:46 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Sebastian Reichel, Marcel Holtmann, Gustavo Padovan,
Johan Hedberg, Samuel Thibault, Pavel Machek, Tony Lindgren,
Jiri Slaby, Mark Rutland, open list:BLUETOOTH DRIVERS,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20170408165740.GA20058-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
On Sat, Apr 8, 2017 at 11:57 AM, Greg Kroah-Hartman
<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
> On Tue, Mar 28, 2017 at 05:59:31PM +0200, Sebastian Reichel wrote:
>> Add method, which waits until the transmission buffer has been sent.
>> Note, that the change in ttyport_write_wakeup is related, since
>> tty_wait_until_sent will hang without that change.
>>
>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
>> Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> ---
>> Changes since PATCHv2:
>> * Avoid goto in ttyport_write_wakeup
>> ---
>> drivers/tty/serdev/core.c | 11 +++++++++++
>> drivers/tty/serdev/serdev-ttyport.c | 18 ++++++++++++++----
>> include/linux/serdev.h | 3 +++
>> 3 files changed, 28 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
>> index f4c6c90add78..a63b74031e22 100644
>> --- a/drivers/tty/serdev/core.c
>> +++ b/drivers/tty/serdev/core.c
>> @@ -173,6 +173,17 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
>> }
>> EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
>>
>> +void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
>> +{
>> + struct serdev_controller *ctrl = serdev->ctrl;
>> +
>> + if (!ctrl || !ctrl->ops->wait_until_sent)
>> + return;
>> +
>> + ctrl->ops->wait_until_sent(ctrl, timeout);
>> +}
>> +EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
>
> Is this still needed now that we have serdev_device_write() with an
> unlimited timeout available?
Yes, because only this waits until the data is on the wire.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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: [PATCH v2] serial: 8250_early: Add earlycon support for Palmchip UART
From: Marc Gonzalez @ 2017-04-10 11:39 UTC (permalink / raw)
To: linux-serial, Peter Hurley, Greg Kroah-Hartman
Cc: Russell King, Andreas Farber, Jean Delvare, Rob Herring,
Mans Rullgard, Jiri Slaby, Masahiro Yamada, Vineet Gupta,
Scott Wood, Robin Murphy, Thibaud Cornic, Mason, Linux ARM, LKML
In-Reply-To: <7a016ff6-08fc-2811-92e0-7c4603fa8586@sigmadesigns.com>
On 10/04/2017 11:47, Marc Gonzalez wrote:
> Define an OF early console for Palmchip UART, which can be enabled
> by passing "earlycon" on the boot command line.
>
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---
> drivers/tty/serial/8250/8250_early.c | 24 ++++++++++++++++++++++++
> drivers/tty/serial/8250/8250_port.c | 4 ++--
> 2 files changed, 26 insertions(+), 2 deletions(-)
As pointed out by Andreas, Russell, and Robin, specifying
the console on the command line is still required. In other
words, the stdout-path method is not 100% functional.
Testing different boot command-lines, with a kernel that
artificially panics in the early stages:
"mem=256M"
"mem=256M console=ttyS0,115200"
prints nothing, as expected.
"mem=256M earlycon"
does not show the panic message.
Hangs after printing:
[ 0.014146] Console: colour dummy device 80x30
[ 0.018615] console [tty0] enabled
[ 0.022038] bootconsole [palmchip0] disabled
"mem=256M console=ttyS0,115200 earlycon"
"mem=256M console=ttyS0 earlycon"
the panic message does appear, as expected.
When I remove the panic, "mem=256M console=ttyS0 earlycon"
stops printing kernel messages when the kernel switches
the early console off.
[ 0.750562] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
[ 0.758976] console [ttyS0] disabled
[ 0.762664] 10700.serial: ttyS0 at MMIO 0x10700 (irq = 20, base_baud = 460800) is a Palmchip BK-3103
Therefore, the only 100% functional combination for me is
"mem=256M console=ttyS0,115200 earlycon"
For completeness, this method shows some "stuttering" in
the console handling code:
[ 0.000000] earlycon: palmchip0 at MMIO 0x00010700 (options '115200n8')
[ 0.000000] bootconsole [palmchip0] enabled
[ 0.000000] Kernel command line: mem=256M console=ttyS0,115200 earlycon
[ 0.014149] Console: colour dummy device 80x30
[ 0.754670] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
[ 0.763045] console [ttyS0] disabled
[ 0.766733] 10700.serial: ttyS0 at MMIO 0x10700 (irq = 20, base_baud = 460800) is a Palmchip BK-3103
[ 0.775969] console [ttyS0] enabled
[ 0.775969] console [ttyS0] enabled
[ 0.782979] bootconsole [palmchip0] disabled
[ 0.782979] bootconsole [palmchip0] disabled
This stuttering is likely related to this comment:
/*
* By unregistering the bootconsoles after we enable the real console
* we get the "console xxx enabled" message on all the consoles -
* boot consoles, real consoles, etc - this is to ensure that end
* users know there might be something in the kernel's log buffer that
* went to the bootconsole (that they do not see on the real console)
*/
In fact, if I add dump_stack() before
pr_info("%sconsole [%s%d] enabled\n",
pr_info("%sconsole [%s%d] disabled\n",
then I get the following output:
[ 0.000000] earlycon: palmchip0 at MMIO 0x00010700 (options '115200n8')
[ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.9.20-1-rc3 #18
[ 0.000000] Hardware name: Sigma Tango DT
[ 0.000000] [<c010ed6c>] (unwind_backtrace) from [<c010adc4>] (show_stack+0x10/0x14)
[ 0.000000] [<c010adc4>] (show_stack) from [<c02cca50>] (dump_stack+0x84/0x98)
[ 0.000000] [<c02cca50>] (dump_stack) from [<c0157b3c>] (register_console+0x224/0x3c8)
[ 0.000000] [<c0157b3c>] (register_console) from [<c06189f0>] (of_setup_earlycon+0x1cc/0x1e0)
[ 0.000000] [<c06189f0>] (of_setup_earlycon) from [<c061c0a0>] (early_init_dt_scan_chosen_stdout+0x144/0x158)
[ 0.000000] [<c061c0a0>] (early_init_dt_scan_chosen_stdout) from [<c0600488>] (do_early_param+0x78/0xbc)
[ 0.000000] [<c0600488>] (do_early_param) from [<c0132aec>] (parse_args+0x284/0x3d8)
[ 0.000000] [<c0132aec>] (parse_args) from [<c06008a0>] (parse_early_options+0x3c/0x44)
[ 0.000000] [<c06008a0>] (parse_early_options) from [<c06008d8>] (parse_early_param+0x30/0x44)
[ 0.000000] [<c06008d8>] (parse_early_param) from [<c0603274>] (setup_arch+0x5b8/0xa48)
[ 0.000000] [<c0603274>] (setup_arch) from [<c060094c>] (start_kernel+0x5c/0x380)
[ 0.000000] [<c060094c>] (start_kernel) from [<8000807c>] (0x8000807c)
[ 0.000000] bootconsole [palmchip0] enabled
(Lines 2 and 3 are a strange relic of the very first lines output.)
[ 0.750471] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
[ 0.758744] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.9.20-1-rc3 #18
[ 0.765317] Hardware name: Sigma Tango DT
[ 0.769376] [<c010ed6c>] (unwind_backtrace) from [<c010adc4>] (show_stack+0x10/0x14)
[ 0.777162] [<c010adc4>] (show_stack) from [<c02cca50>] (dump_stack+0x84/0x98)
[ 0.784430] [<c02cca50>] (dump_stack) from [<c01577e0>] (unregister_console+0xc/0x144)
[ 0.792395] [<c01577e0>] (unregister_console) from [<c033cf20>] (uart_remove_one_port+0x1d0/0x1fc)
[ 0.801404] [<c033cf20>] (uart_remove_one_port) from [<c033e5c0>] (serial8250_register_8250_port+0x94/0x380)
[ 0.811291] [<c033e5c0>] (serial8250_register_8250_port) from [<c0345da8>] (of_platform_serial_probe+0x3f0/0x4ac)
[ 0.821614] [<c0345da8>] (of_platform_serial_probe) from [<c0350d88>] (platform_drv_probe+0x34/0x7c)
[ 0.830796] [<c0350d88>] (platform_drv_probe) from [<c034f824>] (really_probe+0x1c4/0x254)
[ 0.839103] [<c034f824>] (really_probe) from [<c034f978>] (__driver_attach+0xc4/0xc8)
[ 0.846972] [<c034f978>] (__driver_attach) from [<c034dd4c>] (bus_for_each_dev+0x68/0x9c)
[ 0.855192] [<c034dd4c>] (bus_for_each_dev) from [<c034ee90>] (bus_add_driver+0x1a0/0x218)
[ 0.863498] [<c034ee90>] (bus_add_driver) from [<c034fed4>] (driver_register+0x78/0xf8)
[ 0.871542] [<c034fed4>] (driver_register) from [<c0101754>] (do_one_initcall+0x44/0x174)
[ 0.879770] [<c0101754>] (do_one_initcall) from [<c0600dc4>] (kernel_init_freeable+0x154/0x1e4)
[ 0.888516] [<c0600dc4>] (kernel_init_freeable) from [<c04b2fc8>] (kernel_init+0x8/0x10c)
[ 0.896736] [<c04b2fc8>] (kernel_init) from [<c0107738>] (ret_from_fork+0x14/0x3c)
[ 0.904409] console [ttyS0] disabled
[ 0.908072] 10700.serial: ttyS0 at MMIO 0x10700 (irq = 20, base_baud = 460800) is a Palmchip BK-3103
[ 0.917316] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.9.20-1-rc3 #18
[ 0.917316] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.9.20-1-rc3 #18
[ 0.930425] Hardware name: Sigma Tango DT
[ 0.930425] Hardware name: Sigma Tango DT
[ 0.938477] [<c010ed6c>] (unwind_backtrace) from [<c010adc4>] (show_stack+0x10/0x14)
[ 0.938477] [<c010ed6c>] (unwind_backtrace) from [<c010adc4>] (show_stack+0x10/0x14)
[ 0.954036] [<c010adc4>] (show_stack) from [<c02cca50>] (dump_stack+0x84/0x98)
[ 0.954036] [<c010adc4>] (show_stack) from [<c02cca50>] (dump_stack+0x84/0x98)
[ 0.968548] [<c02cca50>] (dump_stack) from [<c0157b3c>] (register_console+0x224/0x3c8)
[ 0.968548] [<c02cca50>] (dump_stack) from [<c0157b3c>] (register_console+0x224/0x3c8)
[ 0.984458] [<c0157b3c>] (register_console) from [<c033cc98>] (uart_add_one_port+0x424/0x4dc)
[ 0.984458] [<c0157b3c>] (register_console) from [<c033cc98>] (uart_add_one_port+0x424/0x4dc)
[ 1.001590] [<c033cc98>] (uart_add_one_port) from [<c033e794>] (serial8250_register_8250_port+0x268/0x380)
[ 1.001590] [<c033cc98>] (uart_add_one_port) from [<c033e794>] (serial8250_register_8250_port+0x268/0x380)
[ 1.020995] [<c033e794>] (serial8250_register_8250_port) from [<c0345da8>] (of_platform_serial_probe+0x3f0/0x4ac)
[ 1.020995] [<c033e794>] (serial8250_register_8250_port) from [<c0345da8>] (of_platform_serial_probe+0x3f0/0x4ac)
[ 1.041621] [<c0345da8>] (of_platform_serial_probe) from [<c0350d88>] (platform_drv_probe+0x34/0x7c)
[ 1.041621] [<c0345da8>] (of_platform_serial_probe) from [<c0350d88>] (platform_drv_probe+0x34/0x7c)
[ 1.059975] [<c0350d88>] (platform_drv_probe) from [<c034f824>] (really_probe+0x1c4/0x254)
[ 1.059975] [<c0350d88>] (platform_drv_probe) from [<c034f824>] (really_probe+0x1c4/0x254)
[ 1.076580] [<c034f824>] (really_probe) from [<c034f978>] (__driver_attach+0xc4/0xc8)
[ 1.076580] [<c034f824>] (really_probe) from [<c034f978>] (__driver_attach+0xc4/0xc8)
[ 1.092311] [<c034f978>] (__driver_attach) from [<c034dd4c>] (bus_for_each_dev+0x68/0x9c)
[ 1.092311] [<c034f978>] (__driver_attach) from [<c034dd4c>] (bus_for_each_dev+0x68/0x9c)
[ 1.108742] [<c034dd4c>] (bus_for_each_dev) from [<c034ee90>] (bus_add_driver+0x1a0/0x218)
[ 1.108742] [<c034dd4c>] (bus_for_each_dev) from [<c034ee90>] (bus_add_driver+0x1a0/0x218)
[ 1.125347] [<c034ee90>] (bus_add_driver) from [<c034fed4>] (driver_register+0x78/0xf8)
[ 1.125347] [<c034ee90>] (bus_add_driver) from [<c034fed4>] (driver_register+0x78/0xf8)
[ 1.141429] [<c034fed4>] (driver_register) from [<c0101754>] (do_one_initcall+0x44/0x174)
[ 1.141429] [<c034fed4>] (driver_register) from [<c0101754>] (do_one_initcall+0x44/0x174)
[ 1.157862] [<c0101754>] (do_one_initcall) from [<c0600dc4>] (kernel_init_freeable+0x154/0x1e4)
[ 1.157862] [<c0101754>] (do_one_initcall) from [<c0600dc4>] (kernel_init_freeable+0x154/0x1e4)
[ 1.175342] [<c0600dc4>] (kernel_init_freeable) from [<c04b2fc8>] (kernel_init+0x8/0x10c)
[ 1.175342] [<c0600dc4>] (kernel_init_freeable) from [<c04b2fc8>] (kernel_init+0x8/0x10c)
[ 1.191773] [<c04b2fc8>] (kernel_init) from [<c0107738>] (ret_from_fork+0x14/0x3c)
[ 1.191773] [<c04b2fc8>] (kernel_init) from [<c0107738>] (ret_from_fork+0x14/0x3c)
[ 1.207013] console [ttyS0] enabled
[ 1.207013] console [ttyS0] enabled
[ 1.214019] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.9.20-1-rc3 #18
[ 1.214019] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.9.20-1-rc3 #18
[ 1.227126] Hardware name: Sigma Tango DT
[ 1.227126] Hardware name: Sigma Tango DT
[ 1.235175] [<c010ed6c>] (unwind_backtrace) from [<c010adc4>] (show_stack+0x10/0x14)
[ 1.235175] [<c010ed6c>] (unwind_backtrace) from [<c010adc4>] (show_stack+0x10/0x14)
[ 1.250732] [<c010adc4>] (show_stack) from [<c02cca50>] (dump_stack+0x84/0x98)
[ 1.250732] [<c010adc4>] (show_stack) from [<c02cca50>] (dump_stack+0x84/0x98)
[ 1.265242] [<c02cca50>] (dump_stack) from [<c01577e0>] (unregister_console+0xc/0x144)
[ 1.265242] [<c02cca50>] (dump_stack) from [<c01577e0>] (unregister_console+0xc/0x144)
[ 1.281149] [<c01577e0>] (unregister_console) from [<c0157bc8>] (register_console+0x2b0/0x3c8)
[ 1.281149] [<c01577e0>] (unregister_console) from [<c0157bc8>] (register_console+0x2b0/0x3c8)
[ 1.298455] [<c0157bc8>] (register_console) from [<c033cc98>] (uart_add_one_port+0x424/0x4dc)
[ 1.298455] [<c0157bc8>] (register_console) from [<c033cc98>] (uart_add_one_port+0x424/0x4dc)
[ 1.315586] [<c033cc98>] (uart_add_one_port) from [<c033e794>] (serial8250_register_8250_port+0x268/0x380)
[ 1.315586] [<c033cc98>] (uart_add_one_port) from [<c033e794>] (serial8250_register_8250_port+0x268/0x380)
[ 1.334989] [<c033e794>] (serial8250_register_8250_port) from [<c0345da8>] (of_platform_serial_probe+0x3f0/0x4ac)
[ 1.334989] [<c033e794>] (serial8250_register_8250_port) from [<c0345da8>] (of_platform_serial_probe+0x3f0/0x4ac)
[ 1.355614] [<c0345da8>] (of_platform_serial_probe) from [<c0350d88>] (platform_drv_probe+0x34/0x7c)
[ 1.355614] [<c0345da8>] (of_platform_serial_probe) from [<c0350d88>] (platform_drv_probe+0x34/0x7c)
[ 1.373968] [<c0350d88>] (platform_drv_probe) from [<c034f824>] (really_probe+0x1c4/0x254)
[ 1.373968] [<c0350d88>] (platform_drv_probe) from [<c034f824>] (really_probe+0x1c4/0x254)
[ 1.390574] [<c034f824>] (really_probe) from [<c034f978>] (__driver_attach+0xc4/0xc8)
[ 1.390574] [<c034f824>] (really_probe) from [<c034f978>] (__driver_attach+0xc4/0xc8)
[ 1.406305] [<c034f978>] (__driver_attach) from [<c034dd4c>] (bus_for_each_dev+0x68/0x9c)
[ 1.406305] [<c034f978>] (__driver_attach) from [<c034dd4c>] (bus_for_each_dev+0x68/0x9c)
[ 1.422735] [<c034dd4c>] (bus_for_each_dev) from [<c034ee90>] (bus_add_driver+0x1a0/0x218)
[ 1.422735] [<c034dd4c>] (bus_for_each_dev) from [<c034ee90>] (bus_add_driver+0x1a0/0x218)
[ 1.439339] [<c034ee90>] (bus_add_driver) from [<c034fed4>] (driver_register+0x78/0xf8)
[ 1.439339] [<c034ee90>] (bus_add_driver) from [<c034fed4>] (driver_register+0x78/0xf8)
[ 1.455421] [<c034fed4>] (driver_register) from [<c0101754>] (do_one_initcall+0x44/0x174)
[ 1.455421] [<c034fed4>] (driver_register) from [<c0101754>] (do_one_initcall+0x44/0x174)
[ 1.471854] [<c0101754>] (do_one_initcall) from [<c0600dc4>] (kernel_init_freeable+0x154/0x1e4)
[ 1.471854] [<c0101754>] (do_one_initcall) from [<c0600dc4>] (kernel_init_freeable+0x154/0x1e4)
[ 1.489332] [<c0600dc4>] (kernel_init_freeable) from [<c04b2fc8>] (kernel_init+0x8/0x10c)
[ 1.489332] [<c0600dc4>] (kernel_init_freeable) from [<c04b2fc8>] (kernel_init+0x8/0x10c)
[ 1.505763] [<c04b2fc8>] (kernel_init) from [<c0107738>] (ret_from_fork+0x14/0x3c)
[ 1.505763] [<c04b2fc8>] (kernel_init) from [<c0107738>] (ret_from_fork+0x14/0x3c)
[ 1.521038] bootconsole [palmchip0] disabled
[ 1.521038] bootconsole [palmchip0] disabled
Maybe it's possible to detect when console and earlycon are
the same, and avoid a pair of unregister/register calls.
Regards.
^ permalink raw reply
* [PATCH v2] serial: 8250_early: Add earlycon support for Palmchip UART
From: Marc Gonzalez @ 2017-04-10 9:47 UTC (permalink / raw)
To: linux-serial, Peter Hurley, Greg Kroah-Hartman
Cc: Russell King, Andreas Farber, Jean Delvare, Rob Herring,
Mans Rullgard, Jiri Slaby, Masahiro Yamada, Vineet Gupta,
Scott Wood, Robin Murphy, Thibaud Cornic, Mason, Linux ARM, LKML
Define an OF early console for Palmchip UART, which can be enabled
by passing "earlycon" on the boot command line.
Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
drivers/tty/serial/8250/8250_early.c | 24 ++++++++++++++++++++++++
drivers/tty/serial/8250/8250_port.c | 4 ++--
2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index 85a12f032402..82fc48eca1df 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -39,6 +39,7 @@
static unsigned int __init serial8250_early_in(struct uart_port *port, int offset)
{
+ int reg_offset = offset;
offset <<= port->regshift;
switch (port->iotype) {
@@ -52,6 +53,8 @@ static unsigned int __init serial8250_early_in(struct uart_port *port, int offse
return ioread32be(port->membase + offset);
case UPIO_PORT:
return inb(port->iobase + offset);
+ case UPIO_AU:
+ return port->serial_in(port, reg_offset);
default:
return 0;
}
@@ -59,6 +62,7 @@ static unsigned int __init serial8250_early_in(struct uart_port *port, int offse
static void __init serial8250_early_out(struct uart_port *port, int offset, int value)
{
+ int reg_offset = offset;
offset <<= port->regshift;
switch (port->iotype) {
@@ -77,6 +81,9 @@ static void __init serial8250_early_out(struct uart_port *port, int offset, int
case UPIO_PORT:
outb(value, port->iobase + offset);
break;
+ case UPIO_AU:
+ port->serial_out(port, reg_offset, value);
+ break;
}
}
@@ -172,3 +179,20 @@ OF_EARLYCON_DECLARE(omap8250, "ti,omap3-uart", early_omap8250_setup);
OF_EARLYCON_DECLARE(omap8250, "ti,omap4-uart", early_omap8250_setup);
#endif
+
+#ifdef CONFIG_SERIAL_8250_RT288X
+
+unsigned int au_serial_in(struct uart_port *p, int offset);
+void au_serial_out(struct uart_port *p, int offset, int value);
+
+static int __init early_au_setup(struct earlycon_device *dev, const char *opt)
+{
+ dev->port.serial_in = au_serial_in;
+ dev->port.serial_out = au_serial_out;
+ dev->port.iotype = UPIO_AU;
+ dev->con->write = early_serial8250_write;
+ return 0;
+}
+OF_EARLYCON_DECLARE(palmchip, "ralink,rt2880-uart", early_au_setup);
+
+#endif
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 080d5a59d0a7..1f08d22d1a80 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -313,7 +313,7 @@ static const s8 au_io_out_map[8] = {
-1, /* UART_SCR (unmapped) */
};
-static unsigned int au_serial_in(struct uart_port *p, int offset)
+unsigned int au_serial_in(struct uart_port *p, int offset)
{
if (offset >= ARRAY_SIZE(au_io_in_map))
return UINT_MAX;
@@ -323,7 +323,7 @@ static unsigned int au_serial_in(struct uart_port *p, int offset)
return __raw_readl(p->membase + (offset << p->regshift));
}
-static void au_serial_out(struct uart_port *p, int offset, int value)
+void au_serial_out(struct uart_port *p, int offset, int value)
{
if (offset >= ARRAY_SIZE(au_io_out_map))
return;
--
2.11.0
^ permalink raw reply related
* [PATCH 2/2] serial: omap: suspend device on probe errors
From: Johan Hovold @ 2017-04-10 9:21 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-omap, linux-kernel, Johan Hovold,
Shubhrajyoti D, Tony Lindgren
In-Reply-To: <20170410092139.11441-1-johan@kernel.org>
Make sure to actually suspend the device before returning after a failed
(or deferred) probe.
Note that autosuspend must be disabled before runtime pm is disabled in
order to balance the usage count due to a negative autosuspend delay as
well as to make the final put suspend the device synchronously.
Fixes: 388bc2622680 ("omap-serial: Fix the error handling in the omap_serial probe")
Cc: Shubhrajyoti D <shubhrajyoti@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/tty/serial/omap-serial.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 8c2086e14b51..e4210b9ad0d3 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1767,7 +1767,8 @@ static int serial_omap_probe(struct platform_device *pdev)
return 0;
err_add_port:
- pm_runtime_put(&pdev->dev);
+ pm_runtime_dont_use_autosuspend(&pdev->dev);
+ pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
pm_qos_remove_request(&up->pm_qos_request);
device_init_wakeup(up->dev, false);
--
2.12.2
^ permalink raw reply related
* [PATCH 1/2] serial: omap: fix runtime-pm handling on unbind
From: Johan Hovold @ 2017-04-10 9:21 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-omap, linux-kernel, Johan Hovold,
Felipe Balbi, Santosh Shilimkar, Tony Lindgren
An unbalanced and misplaced synchronous put was used to suspend the
device on driver unbind, something which with a likewise misplaced
pm_runtime_disable leads to external aborts when an open port is being
removed.
Unhandled fault: external abort on non-linefetch (0x1028) at 0xfa024010
...
[<c046e760>] (serial_omap_set_mctrl) from [<c046a064>] (uart_update_mctrl+0x50/0x60)
[<c046a064>] (uart_update_mctrl) from [<c046a400>] (uart_shutdown+0xbc/0x138)
[<c046a400>] (uart_shutdown) from [<c046bd2c>] (uart_hangup+0x94/0x190)
[<c046bd2c>] (uart_hangup) from [<c045b760>] (__tty_hangup+0x404/0x41c)
[<c045b760>] (__tty_hangup) from [<c045b794>] (tty_vhangup+0x1c/0x20)
[<c045b794>] (tty_vhangup) from [<c046ccc8>] (uart_remove_one_port+0xec/0x260)
[<c046ccc8>] (uart_remove_one_port) from [<c046ef4c>] (serial_omap_remove+0x40/0x60)
[<c046ef4c>] (serial_omap_remove) from [<c04845e8>] (platform_drv_remove+0x34/0x4c)
Fix this up by resuming the device before deregistering the port and by
suspending and disabling runtime pm only after the port has been
removed.
Also make sure to disable autosuspend before disabling runtime pm so
that the usage count is balanced and device actually suspended before
returning.
Note that due to a negative autosuspend delay being set in probe, the
unbalanced put would actually suspend the device on first driver unbind,
while rebinding and again unbinding would result in a negative
power.usage_count.
Fixes: 7e9c8e7dbf3b ("serial: omap: make sure to suspend device before remove")
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/tty/serial/omap-serial.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 6c6f82ad8d5c..8c2086e14b51 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1780,9 +1780,13 @@ static int serial_omap_remove(struct platform_device *dev)
{
struct uart_omap_port *up = platform_get_drvdata(dev);
+ pm_runtime_get_sync(up->dev);
+
+ uart_remove_one_port(&serial_omap_reg, &up->port);
+
+ pm_runtime_dont_use_autosuspend(up->dev);
pm_runtime_put_sync(up->dev);
pm_runtime_disable(up->dev);
- uart_remove_one_port(&serial_omap_reg, &up->port);
pm_qos_remove_request(&up->pm_qos_request);
device_init_wakeup(&dev->dev, false);
--
2.12.2
^ permalink raw reply related
* Re: 8250: Possible race between console message vs DMA?
From: Vignesh R @ 2017-04-10 8:16 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-serial@vger.kernel.org, Peter Hurley, Greg Kroah-Hartman,
linux-omap@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAHp75Vfc6FznNByidzjRcgsj14=DLtwqb=wtut4Cb4gj5UO42w@mail.gmail.com>
On Sunday 09 April 2017 04:37 PM, Andy Shevchenko wrote:
> On Fri, Apr 7, 2017 at 2:08 PM, Vignesh R <vigneshr@ti.com> wrote:
>> Hi All,
>>
>> I seem to be hitting a race condition using 8250_dma (and 8250_omap
>> specific dma) support:
>>
>> Kernel writes log messages to console via
>> serial8250_console_write()->serial8250_console_putchar() which directly
>> accesses UART_TX register with port->lock acquired.
>>
>> Now, if the same UART instance is being used by systemd/userspace,
>> characters are written to UART_TX register by serial8250_tx_chars(). The
>> concurrent access by serial8250_console_write() and
>> serial8250_tx_chars() is serialized by the use of port->lock spinlock
>> and hence there is no issue with` non DMA case.
>>
>> But when using DMA with 8250 UART, I see that port->lock is held before
>> scheduling of DMA TX transfer and released as soon as the transfer is
>> submitted. The lock is not held until the transfer actually completes
>> See,
>> uart_start()
>> ->serial8250_start_tx()->
>> __start_tx()
>> ->up->dma->tx_dma(up)
>> Or
>> __dma_tx_complete() in 8250_dma.c that acquires and releases port->lock
>> once TX DMA transfer is submitted in serial8250_tx_dma()
>>
>> So, when the port->lock is released, it is quite possible that DMA is
>> still transferring data to UART TX FIFO and UART FIFO might be almost full.
>> I see that when DMA is writing to UART TX FIFO,
>> serial8250_console_write() may also write kernel log messages to UART TX
>> FIFO(as port->lock is now free to be acquired), which is leading to
>> overflow and lose of data. serial8250_console_write() checks for
>> UART_LSR_THRE to check if Transmit hold register is empty but that may
>> not be enough as DMA might put data before CPU write.
>>
>> It seems that both DMA and CPU might simultaneously put data to UART
>> FIFO and lead to potential loss of data.
>> Is the expectation that UART instance used to print kernel log messages
>> is not intended to use DMA? Or am I missing something?
>>
>>
>> Any help appreciated!
>
> I have one patch in my tree for a long time already:
> https://bitbucket.org/andy-shev/linux/commits/9f86c648e53bd25b8ec374933764577b2a340468?at=topic/uart/rpm
I had similar patch in mind. Do you plan to submit above patch to the
mailing list? You may also consider to add the issue I mentioned above
to the commit description. Thanks!
>
> Besides that I have patch to disable power management on kernel
> console (and non-hackish implementation of runtime PM for UART is
> there in case you are wondering what that repository for).
>
Nice!
--
Regards
Vignesh
^ permalink raw reply
* [PATCH v3 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Joel Stanley @ 2017-04-10 4:04 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Mark Rutland, Rob Herring
Cc: Jeremy Kerr, linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Andy Shevchenko,
Benjamin Herrenschmidt, openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <20170410040400.5509-1-joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
From: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
This change adds a driver for the 16550-based Aspeed virtual UART
device. We use a similar process to the of_serial driver for device
probe, but expose some VUART-specific functions through sysfs too.
The VUART is two UART 'front ends' connected by their FIFO (no actual
serial line in between). One is on the BMC side (management controller)
and one is on the host CPU side.
This driver is for the BMC side. The sysfs files allow the BMC
userspace, which owns the system configuration policy, to specify at
what IO port and interrupt number the host side will appear to the host
on the Host <-> BMC LPC bus. It could be different on a different system
(though most of them use 3f8/4).
OpenPOWER host firmware doesn't like it when the host-side of the
VUART's FIFO is not drained. This driver only disables host TX discard
mode when the port is in use. We set the VUART enabled bit when we bind
to the device, and clear it on unbind.
We don't want to do this on open/release, as the host may be using this
bit to configure serial output modes, which is independent of whether
the devices has been opened by BMC userspace.
Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
v3:
- remove whitespace in header
- reformat comment
- don't check for reg-io-width property; we don't need any special
accessors for reading/writing bytes
- move file to 8250_aspeed_vuart.c
v2:
- Use attribute groups and DEVICE_ATTR_RW
- Use platform_get_resource/devm_ioremap_resource
- of_find_property -> of_property_read_bool
- Drop unncessary 0xff mask
- Fix comment style
- Use BIT and GENMASK where pssible
- Move to 8250 directory
- Rename ast -> aspeed to match other Aspeed drivers
- Add documentation of sysfs file
- Add detail to the commit message
Documentation/ABI/stable/sysfs-driver-aspeed-vuart | 15 +
Documentation/devicetree/bindings/serial/8250.txt | 2 +
drivers/tty/serial/8250/8250_aspeed_vuart.c | 323 +++++++++++++++++++++
drivers/tty/serial/8250/Kconfig | 10 +
drivers/tty/serial/8250/Makefile | 1 +
5 files changed, 351 insertions(+)
create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-vuart
create mode 100644 drivers/tty/serial/8250/8250_aspeed_vuart.c
diff --git a/Documentation/ABI/stable/sysfs-driver-aspeed-vuart b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
new file mode 100644
index 000000000000..8062953ce77b
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
@@ -0,0 +1,15 @@
+What: /sys/bus/platform/drivers/aspeed-vuart/*/lpc_address
+Date: April 2017
+Contact: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
+Description: Configures which IO port the host side of the UART
+ will appear on the host <-> BMC LPC bus.
+Users: OpenBMC. Proposed changes should be mailed to
+ openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
+
+What: /sys/bus/platform/drivers/aspeed-vuart*/sirq
+Date: April 2017
+Contact: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
+Description: Configures which interrupt number the host side of
+ the UART will appear on the host <-> BMC LPC bus.
+Users: OpenBMC. Proposed changes should be mailed to
+ openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
index 10276a46ecef..656733949309 100644
--- a/Documentation/devicetree/bindings/serial/8250.txt
+++ b/Documentation/devicetree/bindings/serial/8250.txt
@@ -20,6 +20,8 @@ Required properties:
- "fsl,16550-FIFO64"
- "fsl,ns16550"
- "ti,da830-uart"
+ - "aspeed,ast2400-vuart"
+ - "aspeed,ast2500-vuart"
- "serial" if the port type is unknown.
- reg : offset and length of the register set for the device.
- interrupts : should contain uart interrupt.
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
new file mode 100644
index 000000000000..5db0023e0225
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -0,0 +1,323 @@
+/*
+ * Serial Port driver for Aspeed VUART device
+ *
+ * Copyright (C) 2016 Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>, IBM Corp.
+ * Copyright (C) 2006 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>, IBM Corp.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/clk.h>
+
+#include "8250.h"
+
+#define ASPEED_VUART_GCRA 0x20
+#define ASPEED_VUART_GCRA_VUART_EN BIT(0)
+#define ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD BIT(5)
+#define ASPEED_VUART_GCRB 0x24
+#define ASPEED_VUART_GCRB_HOST_SIRQ_MASK GENMASK(7, 4)
+#define ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT 4
+#define ASPEED_VUART_ADDRL 0x28
+#define ASPEED_VUART_ADDRH 0x2c
+
+struct aspeed_vuart {
+ struct device *dev;
+ void __iomem *regs;
+ struct clk *clk;
+ int line;
+};
+
+/*
+ * The VUART is basically two UART 'front ends' connected by their FIFO
+ * (no actual serial line in between). One is on the BMC side (management
+ * controller) and one is on the host CPU side.
+ *
+ * It allows the BMC to provide to the host a "UART" that pipes into
+ * the BMC itself and can then be turned by the BMC into a network console
+ * of some sort for example.
+ *
+ * This driver is for the BMC side. The sysfs files allow the BMC
+ * userspace which owns the system configuration policy, to specify
+ * at what IO port and interrupt number the host side will appear
+ * to the host on the Host <-> BMC LPC bus. It could be different on a
+ * different system (though most of them use 3f8/4).
+ */
+
+static ssize_t lpc_address_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+ u16 addr;
+
+ addr = (readb(vuart->regs + ASPEED_VUART_ADDRH) << 8) |
+ (readb(vuart->regs + ASPEED_VUART_ADDRL));
+
+ return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
+}
+
+static ssize_t lpc_address_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+ unsigned long val;
+ int err;
+
+ err = kstrtoul(buf, 0, &val);
+ if (err)
+ return err;
+
+ writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
+ writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(lpc_address);
+
+static ssize_t sirq_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+ u8 reg;
+
+ reg = readb(vuart->regs + ASPEED_VUART_GCRB);
+ reg &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+ reg >>= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
+
+ return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
+}
+
+static ssize_t sirq_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+ unsigned long val;
+ int err;
+ u8 reg;
+
+ err = kstrtoul(buf, 0, &val);
+ if (err)
+ return err;
+
+ val <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
+ val &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+
+ reg = readb(vuart->regs + ASPEED_VUART_GCRB);
+ reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+ reg |= val;
+ writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(sirq);
+
+static struct attribute *aspeed_vuart_attrs[] = {
+ &dev_attr_sirq.attr,
+ &dev_attr_lpc_address.attr,
+ NULL,
+};
+
+static const struct attribute_group aspeed_vuart_attr_group = {
+ .attrs = aspeed_vuart_attrs,
+};
+
+static void aspeed_vuart_set_enabled(struct aspeed_vuart *vuart, bool enabled)
+{
+ u8 reg;
+
+ reg = readb(vuart->regs + ASPEED_VUART_GCRA);
+ reg &= ~ASPEED_VUART_GCRA_VUART_EN;
+ if (enabled)
+ reg |= ASPEED_VUART_GCRA_VUART_EN;
+ writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
+}
+
+static void aspeed_vuart_set_host_tx_discard(struct aspeed_vuart *vuart,
+ bool discard)
+{
+ u8 reg;
+
+ reg = readb(vuart->regs + ASPEED_VUART_GCRA);
+
+ /* If the DISABLE_HOST_TX_DISCARD bit is set, discard is disabled */
+ reg &= ~ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
+ if (!discard)
+ reg |= ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
+
+ writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
+}
+
+static int aspeed_vuart_startup(struct uart_port *uart_port)
+{
+ struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
+ struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
+ int rc;
+
+ rc = serial8250_do_startup(uart_port);
+ if (rc)
+ return rc;
+
+ aspeed_vuart_set_host_tx_discard(vuart, false);
+
+ return 0;
+}
+
+static void aspeed_vuart_shutdown(struct uart_port *uart_port)
+{
+ struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
+ struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
+
+ aspeed_vuart_set_host_tx_discard(vuart, true);
+
+ serial8250_do_shutdown(uart_port);
+}
+
+static int aspeed_vuart_probe(struct platform_device *pdev)
+{
+ struct uart_8250_port port;
+ struct aspeed_vuart *vuart;
+ struct device_node *np;
+ struct resource *res;
+ u32 clk, prop;
+ int rc;
+
+ np = pdev->dev.of_node;
+
+ vuart = devm_kzalloc(&pdev->dev, sizeof(*vuart), GFP_KERNEL);
+ if (!vuart)
+ return -ENOMEM;
+
+ vuart->dev = &pdev->dev;
+
+ /* The 8510 core creates the mapping, which we grab a reference to
+ * for VUART-specific registers */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ vuart->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(vuart->regs))
+ return PTR_ERR(vuart->regs);
+
+ memset(&port, 0, sizeof(port));
+ port.port.private_data = vuart;
+ port.port.membase = vuart->regs;
+ port.port.mapbase = res->start;
+ port.port.mapsize = resource_size(res);
+ port.port.startup = aspeed_vuart_startup;
+ port.port.shutdown = aspeed_vuart_shutdown;
+ port.port.dev = &pdev->dev;
+
+ rc = sysfs_create_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
+ if (rc < 0)
+ return rc;
+
+ if (of_property_read_u32(np, "clock-frequency", &clk)) {
+ vuart->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(vuart->clk)) {
+ dev_warn(&pdev->dev,
+ "clk or clock-frequency not defined\n");
+ return PTR_ERR(vuart->clk);
+ }
+
+ rc = clk_prepare_enable(vuart->clk);
+ if (rc < 0)
+ return rc;
+
+ clk = clk_get_rate(vuart->clk);
+ }
+
+ /* If current-speed was set, then try not to change it. */
+ if (of_property_read_u32(np, "current-speed", &prop) == 0)
+ port.port.custom_divisor = clk / (16 * prop);
+
+ /* Check for shifted address mapping */
+ if (of_property_read_u32(np, "reg-offset", &prop) == 0)
+ port.port.mapbase += prop;
+
+ /* Check for registers offset within the devices address range */
+ if (of_property_read_u32(np, "reg-shift", &prop) == 0)
+ port.port.regshift = prop;
+
+ /* Check for fifo size */
+ if (of_property_read_u32(np, "fifo-size", &prop) == 0)
+ port.port.fifosize = prop;
+
+ /* Check for a fixed line number */
+ rc = of_alias_get_id(np, "serial");
+ if (rc >= 0)
+ port.port.line = rc;
+
+ port.port.irq = irq_of_parse_and_map(np, 0);
+ port.port.irqflags = IRQF_SHARED;
+ port.port.iotype = UPIO_MEM;
+ port.port.type = PORT_16550A;
+ port.port.uartclk = clk;
+ port.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF
+ | UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_NO_THRE_TEST;
+
+ if (of_property_read_bool(np, "no-loopback-test"))
+ port.port.flags |= UPF_SKIP_TEST;
+
+ if (port.port.fifosize)
+ port.capabilities = UART_CAP_FIFO;
+
+ if (of_property_read_bool(np, "auto-flow-control"))
+ port.capabilities |= UART_CAP_AFE;
+
+ rc = serial8250_register_8250_port(&port);
+ if (rc < 0)
+ goto err_clk_disable;
+
+ vuart->line = rc;
+
+ aspeed_vuart_set_enabled(vuart, true);
+ aspeed_vuart_set_host_tx_discard(vuart, true);
+ platform_set_drvdata(pdev, vuart);
+
+ return 0;
+
+err_clk_disable:
+ clk_disable_unprepare(vuart->clk);
+ irq_dispose_mapping(port.port.irq);
+ return rc;
+}
+
+static int aspeed_vuart_remove(struct platform_device *pdev)
+{
+ struct aspeed_vuart *vuart = platform_get_drvdata(pdev);
+
+ aspeed_vuart_set_enabled(vuart, false);
+ serial8250_unregister_port(vuart->line);
+ sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
+ clk_disable_unprepare(vuart->clk);
+
+ return 0;
+}
+
+static const struct of_device_id aspeed_vuart_table[] = {
+ { .compatible = "aspeed,ast2400-vuart" },
+ { .compatible = "aspeed,ast2500-vuart" },
+ { },
+};
+
+static struct platform_driver aspeed_vuart_driver = {
+ .driver = {
+ .name = "aspeed-vuart",
+ .of_match_table = aspeed_vuart_table,
+ },
+ .probe = aspeed_vuart_probe,
+ .remove = aspeed_vuart_remove,
+};
+
+module_platform_driver(aspeed_vuart_driver);
+
+MODULE_AUTHOR("Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Driver for Aspeed VUART device");
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index a65fb8197aec..fb217f02ec94 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -220,6 +220,16 @@ config SERIAL_8250_ACCENT
To compile this driver as a module, choose M here: the module
will be called 8250_accent.
+config SERIAL_8250_ASPEED_VUART
+ tristate "Aspeed Virtual UART"
+ depends on SERIAL_8250
+ depends on OF
+ help
+ If you want to use the virtual UART (VUART) device on Aspeed
+ BMC platforms, enable this option. This enables the 16550A-
+ compatible device on the local LPC bus, giving a UART device
+ with no physical RS232 connections.
+
config SERIAL_8250_BOCA
tristate "Support Boca cards"
depends on SERIAL_8250 != n && ISA && SERIAL_8250_MANY_PORTS
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index 2f30f9ecdb1b..a44a99a3e623 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_SERIAL_8250_EXAR) += 8250_exar.o
obj-$(CONFIG_SERIAL_8250_HP300) += 8250_hp300.o
obj-$(CONFIG_SERIAL_8250_CS) += serial_cs.o
obj-$(CONFIG_SERIAL_8250_ACORN) += 8250_acorn.o
+obj-$(CONFIG_SERIAL_8250_ASPEED_VUART) += 8250_aspeed_vuart.o
obj-$(CONFIG_SERIAL_8250_BCM2835AUX) += 8250_bcm2835aux.o
obj-$(CONFIG_SERIAL_8250_CONSOLE) += 8250_early.o
obj-$(CONFIG_SERIAL_8250_FOURPORT) += 8250_fourport.o
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 related
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