Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1249032 > unrolled thread

[PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle

Started by"H. Nikolaus Schaller" <hns@goldelico.com>
First post2015-10-16 20:10 +0200
Last post2015-10-16 21:40 +0200
Articles 6 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle "H. Nikolaus Schaller" <hns@goldelico.com> - 2015-10-16 20:10 +0200
    Re: [PATCH v3 1/3] tty: serial core: provide a method to search uart  by phandle Mark Rutland <mark.rutland@arm.com> - 2015-10-16 20:40 +0200
      Re: [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle "H. Nikolaus Schaller" <hns@goldelico.com> - 2015-10-16 21:30 +0200
    Re: [PATCH v3 1/3] tty: serial core: provide a method to search uart  by phandle kbuild test robot <lkp@intel.com> - 2015-10-16 20:50 +0200
    Re: [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle Arnd Bergmann <arnd@arndb.de> - 2015-10-16 21:10 +0200
    Re: [PATCH v3 1/3] tty: serial core: provide a method to search uart  by phandle kbuild test robot <lkp@intel.com> - 2015-10-16 21:40 +0200

#1249032 — [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle

From"H. Nikolaus Schaller" <hns@goldelico.com>
Date2015-10-16 20:10 +0200
Subject[PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle
Message-ID<qkhC2-5DI-7@gated-at.bofh.it>
1. add uart_ports to a search list as soon as they are registered
2. provide a function to search an uart_port by phandle. This copies the
   mechanism how devm_usb_get_phy_by_phandle() works
3. add a bindings document how serial slaves should use this feature
4. add Documentation how serla slaves work in general

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 .../devicetree/bindings/serial/slaves.txt          |  16 +++
 Documentation/serial/slaves.txt                    |  36 +++++++
 drivers/tty/serial/serial_core.c                   | 107 +++++++++++++++++++++
 include/linux/serial_core.h                        |  10 ++
 4 files changed, 169 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/slaves.txt
 create mode 100644 Documentation/serial/slaves.txt

diff --git a/Documentation/devicetree/bindings/serial/slaves.txt b/Documentation/devicetree/bindings/serial/slaves.txt
new file mode 100644
index 0000000..353b87f
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/slaves.txt
@@ -0,0 +1,16 @@
+Device-Tree bindings for UART slave devices
+
+A node describing a slave device defines a phandle to reference the UART
+the device is connected to. In the (unexpected) case of two or more UARTs
+a list of phandles can be specified.
+
+properties:
+	- uart: (list of) phandle(s) of UART(s) the device is connected to
+
+
+example:
+
+	gps {
+		compatible = "wi2wi,w2sg0004";
+		uart = <&uart1>;
+	};
diff --git a/Documentation/serial/slaves.txt b/Documentation/serial/slaves.txt
new file mode 100644
index 0000000..6f8d44d
--- /dev/null
+++ b/Documentation/serial/slaves.txt
@@ -0,0 +1,36 @@
+UART slave device support
+
+A remote device connected to a RS232 interface is usually power controlled by the DTR line.
+The DTR line is managed automatically by the UART driver for open() and close() syscalls
+and on demand by tcsetattr().
+
+With embedded devices, the serial peripheral might be directly and always connected to the UART
+and there might be no physical DTR line involved. Power control (on/off) has to be done by some
+chip specific device driver (which we call "UART slave") through some mechanisms (I2C, GPIOs etc.)
+not related to the serial interface. Some devices do not explicitly tell their power state except
+by sending or not sending data to the UART. In such a case the device driver must be able to monitor
+data activity. The role of the device driver is to encapsulate such power control in a single place.
+
+This patch series allows to support such drivers by providing:
+* a mechanism that a slave driver can identify the UART instance it is connected to
+* a mechanism that UART slave drivers can register to be notified
+* notfications for DTR (and other modem control) state changes
+* notifications that the UART has received some data from the UART
+
+A slave device simply adds a phandle reference to the UART it is connected to, e.g.
+
+	gps {
+		compatible = "wi2wi,w2sg0004";
+		uart = <&uart1>;
+	};
+
+The slave driver calls devm_serial_get_uart_by_phandle() to identify the uart driver.
+This API follows the concept of devm_usb_get_phy_by_phandle().
+
+A slave device driver registers itself with serial_register_slave() to receive notifications.
+Notification handler callbacks can be registered by serial_register_mctrl_notification() and
+serial_register_rx_notification(). If an UART has registered a NULL slave or a NULL handler,
+no notifications are sent.
+
+RX notification handlers can define a ktermios during setup and the handler function can modify
+or decide to throw away each character that is passed upwards.
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 603d2cc..9caa33e 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -38,6 +38,36 @@
 #include <asm/irq.h>
 #include <asm/uaccess.h>
 
+static LIST_HEAD(uart_list);
+static DEFINE_SPINLOCK(uart_lock);
+
+/* same concept as __of_usb_find_phy */
+static struct uart_port *__of_serial_find_uart(struct device_node *node)
+{
+	struct uart_port  *uart;
+
+	if (!of_device_is_available(node))
+		return ERR_PTR(-ENODEV);
+
+	list_for_each_entry(uart, &uart_list, head) {
+		if (node != uart->dev->of_node)
+			continue;
+
+		return uart;
+	}
+
+	return ERR_PTR(-EPROBE_DEFER);
+}
+
+static void devm_serial_uart_release(struct device *dev, void *res)
+{
+	struct uart_port *uart = *(struct uart_port **)res;
+
+	/* FIXME: I don't understand the serial subsystem well enough
+	 * to know if we should call serial_put_uart(uart); here
+	 */
+}
+
 /*
  * This is used to lock changes in serial line configuration.
  */
@@ -64,6 +94,79 @@ static int uart_dcd_enabled(struct uart_port *uport)
 	return !!(uport->status & UPSTAT_DCD_ENABLE);
 }
 
+/**
+ * devm_serial_get_uart_by_phandle - find the uart by phandle
+ * @dev - device that requests this uart
+ * @phandle - name of the property holding the uart phandle value
+ * @index - the index of the uart
+ *
+ * Returns the uart_port associated with the given phandle value,
+ * after getting a refcount to it, -ENODEV if there is no such uart or
+ * -EPROBE_DEFER if there is a phandle to the uart, but the device is
+ * not yet loaded. While at that, it also associates the device with
+ * the uart using devres. On driver detach, release function is invoked
+ * on the devres data, then, devres data is freed.
+ *
+ * For use by tty host and peripheral drivers.
+ */
+
+/* same concept as devm_usb_get_phy_by_phandle() */
+
+struct uart_port *devm_serial_get_uart_by_phandle(struct device *dev,
+		const char *phandle, u8 index)
+{
+	struct uart_port  *uart = ERR_PTR(-ENOMEM), **ptr;
+	unsigned long   flags;
+	struct device_node *node;
+
+	if (!dev->of_node) {
+		dev_err(dev, "device does not have a device node entry\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	node = of_parse_phandle(dev->of_node, phandle, index);
+	if (!node) {
+		dev_err(dev, "failed to get %s phandle in %s node\n", phandle,
+			dev->of_node->full_name);
+		return ERR_PTR(-ENODEV);
+	}
+
+	ptr = devres_alloc(devm_serial_uart_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr) {
+		dev_err(dev, "failed to allocate memory for devres\n");
+		goto err0;
+	}
+
+	spin_lock_irqsave(&uart_lock, flags);
+
+	uart = __of_serial_find_uart(node);
+	if (IS_ERR(uart)) {
+		devres_free(ptr);
+		goto err1;
+	}
+
+	if (!try_module_get(uart->dev->driver->owner)) {
+		uart = ERR_PTR(-ENODEV);
+		devres_free(ptr);
+		goto err1;
+	}
+
+	*ptr = uart;
+	devres_add(dev, ptr);
+
+	get_device(uart->dev);
+
+err1:
+	spin_unlock_irqrestore(&uart_lock, flags);
+
+err0:
+	of_node_put(node);
+
+	return uart;
+}
+EXPORT_SYMBOL_GPL(devm_serial_get_uart_by_phandle);
+
+
 /*
  * This routine is used by the interrupt handler to schedule processing in
  * the software interrupt portion of the driver.
@@ -2733,6 +2836,8 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
 	 */
 	uport->flags &= ~UPF_DEAD;
 
+	list_add_tail(&uport->head, &uart_list);
+
  out:
 	mutex_unlock(&port->mutex);
 	mutex_unlock(&port_mutex);
@@ -2764,6 +2869,8 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport)
 
 	mutex_lock(&port_mutex);
 
+	list_del(&uport->head);
+
 	/*
 	 * Mark the port "dead" - this prevents any opens from
 	 * succeeding while we shut down the port.
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 297d4fa..d7a2e15 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -247,6 +247,7 @@ struct uart_port {
 	const struct attribute_group **tty_groups;	/* all attributes (serial core use only) */
 	struct serial_rs485     rs485;
 	void			*private_data;		/* generic platform data pointer */
+	struct list_head	head;			/* uarts list (lookup by phandle) */
 };
 
 static inline int serial_port_in(struct uart_port *up, int offset)
@@ -475,4 +476,13 @@ static inline int uart_handle_break(struct uart_port *port)
 					 (cflag) & CRTSCTS || \
 					 !((cflag) & CLOCAL))
 
+/*
+ * Helper functions for UART slave drivers
+ */
+
+/* find UART by phandle (e.g. with 'uart = <&uart2>;' then call as
+ * devm_serial_get_uart_by_phandle(dev, "uart", 0);
+ */
+extern struct uart_port *devm_serial_get_uart_by_phandle(struct device *dev,
+		const char *phandle, u8 index);
 #endif /* LINUX_SERIAL_CORE_H */
-- 
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1249050 — Re: [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle

FromMark Rutland <mark.rutland@arm.com>
Date2015-10-16 20:40 +0200
SubjectRe: [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle
Message-ID<qki54-6bZ-1@gated-at.bofh.it>
In reply to#1249032
On Fri, Oct 16, 2015 at 08:08:33PM +0200, H. Nikolaus Schaller wrote:
> 1. add uart_ports to a search list as soon as they are registered
> 2. provide a function to search an uart_port by phandle. This copies the
>    mechanism how devm_usb_get_phy_by_phandle() works
> 3. add a bindings document how serial slaves should use this feature
> 4. add Documentation how serla slaves work in general

I thought maintainers preferred the child node approach to the phandle
approach, and this series comes with no rationale (nor change log,
despite being 'v3').

I don't understand. What is going on here?

Mark.

> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  .../devicetree/bindings/serial/slaves.txt          |  16 +++
>  Documentation/serial/slaves.txt                    |  36 +++++++
>  drivers/tty/serial/serial_core.c                   | 107 +++++++++++++++++++++
>  include/linux/serial_core.h                        |  10 ++
>  4 files changed, 169 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/serial/slaves.txt
>  create mode 100644 Documentation/serial/slaves.txt
> 
> diff --git a/Documentation/devicetree/bindings/serial/slaves.txt b/Documentation/devicetree/bindings/serial/slaves.txt
> new file mode 100644
> index 0000000..353b87f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/slaves.txt
> @@ -0,0 +1,16 @@
> +Device-Tree bindings for UART slave devices
> +
> +A node describing a slave device defines a phandle to reference the UART
> +the device is connected to. In the (unexpected) case of two or more UARTs
> +a list of phandles can be specified.
> +
> +properties:
> +	- uart: (list of) phandle(s) of UART(s) the device is connected to
> +
> +
> +example:
> +
> +	gps {
> +		compatible = "wi2wi,w2sg0004";
> +		uart = <&uart1>;
> +	};
> diff --git a/Documentation/serial/slaves.txt b/Documentation/serial/slaves.txt
> new file mode 100644
> index 0000000..6f8d44d
> --- /dev/null
> +++ b/Documentation/serial/slaves.txt
> @@ -0,0 +1,36 @@
> +UART slave device support
> +
> +A remote device connected to a RS232 interface is usually power controlled by the DTR line.
> +The DTR line is managed automatically by the UART driver for open() and close() syscalls
> +and on demand by tcsetattr().
> +
> +With embedded devices, the serial peripheral might be directly and always connected to the UART
> +and there might be no physical DTR line involved. Power control (on/off) has to be done by some
> +chip specific device driver (which we call "UART slave") through some mechanisms (I2C, GPIOs etc.)
> +not related to the serial interface. Some devices do not explicitly tell their power state except
> +by sending or not sending data to the UART. In such a case the device driver must be able to monitor
> +data activity. The role of the device driver is to encapsulate such power control in a single place.
> +
> +This patch series allows to support such drivers by providing:
> +* a mechanism that a slave driver can identify the UART instance it is connected to
> +* a mechanism that UART slave drivers can register to be notified
> +* notfications for DTR (and other modem control) state changes
> +* notifications that the UART has received some data from the UART
> +
> +A slave device simply adds a phandle reference to the UART it is connected to, e.g.
> +
> +	gps {
> +		compatible = "wi2wi,w2sg0004";
> +		uart = <&uart1>;
> +	};
> +
> +The slave driver calls devm_serial_get_uart_by_phandle() to identify the uart driver.
> +This API follows the concept of devm_usb_get_phy_by_phandle().
> +
> +A slave device driver registers itself with serial_register_slave() to receive notifications.
> +Notification handler callbacks can be registered by serial_register_mctrl_notification() and
> +serial_register_rx_notification(). If an UART has registered a NULL slave or a NULL handler,
> +no notifications are sent.
> +
> +RX notification handlers can define a ktermios during setup and the handler function can modify
> +or decide to throw away each character that is passed upwards.
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 603d2cc..9caa33e 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -38,6 +38,36 @@
>  #include <asm/irq.h>
>  #include <asm/uaccess.h>
>  
> +static LIST_HEAD(uart_list);
> +static DEFINE_SPINLOCK(uart_lock);
> +
> +/* same concept as __of_usb_find_phy */
> +static struct uart_port *__of_serial_find_uart(struct device_node *node)
> +{
> +	struct uart_port  *uart;
> +
> +	if (!of_device_is_available(node))
> +		return ERR_PTR(-ENODEV);
> +
> +	list_for_each_entry(uart, &uart_list, head) {
> +		if (node != uart->dev->of_node)
> +			continue;
> +
> +		return uart;
> +	}
> +
> +	return ERR_PTR(-EPROBE_DEFER);
> +}
> +
> +static void devm_serial_uart_release(struct device *dev, void *res)
> +{
> +	struct uart_port *uart = *(struct uart_port **)res;
> +
> +	/* FIXME: I don't understand the serial subsystem well enough
> +	 * to know if we should call serial_put_uart(uart); here
> +	 */
> +}
> +
>  /*
>   * This is used to lock changes in serial line configuration.
>   */
> @@ -64,6 +94,79 @@ static int uart_dcd_enabled(struct uart_port *uport)
>  	return !!(uport->status & UPSTAT_DCD_ENABLE);
>  }
>  
> +/**
> + * devm_serial_get_uart_by_phandle - find the uart by phandle
> + * @dev - device that requests this uart
> + * @phandle - name of the property holding the uart phandle value
> + * @index - the index of the uart
> + *
> + * Returns the uart_port associated with the given phandle value,
> + * after getting a refcount to it, -ENODEV if there is no such uart or
> + * -EPROBE_DEFER if there is a phandle to the uart, but the device is
> + * not yet loaded. While at that, it also associates the device with
> + * the uart using devres. On driver detach, release function is invoked
> + * on the devres data, then, devres data is freed.
> + *
> + * For use by tty host and peripheral drivers.
> + */
> +
> +/* same concept as devm_usb_get_phy_by_phandle() */
> +
> +struct uart_port *devm_serial_get_uart_by_phandle(struct device *dev,
> +		const char *phandle, u8 index)
> +{
> +	struct uart_port  *uart = ERR_PTR(-ENOMEM), **ptr;
> +	unsigned long   flags;
> +	struct device_node *node;
> +
> +	if (!dev->of_node) {
> +		dev_err(dev, "device does not have a device node entry\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	node = of_parse_phandle(dev->of_node, phandle, index);
> +	if (!node) {
> +		dev_err(dev, "failed to get %s phandle in %s node\n", phandle,
> +			dev->of_node->full_name);
> +		return ERR_PTR(-ENODEV);
> +	}
> +
> +	ptr = devres_alloc(devm_serial_uart_release, sizeof(*ptr), GFP_KERNEL);
> +	if (!ptr) {
> +		dev_err(dev, "failed to allocate memory for devres\n");
> +		goto err0;
> +	}
> +
> +	spin_lock_irqsave(&uart_lock, flags);
> +
> +	uart = __of_serial_find_uart(node);
> +	if (IS_ERR(uart)) {
> +		devres_free(ptr);
> +		goto err1;
> +	}
> +
> +	if (!try_module_get(uart->dev->driver->owner)) {
> +		uart = ERR_PTR(-ENODEV);
> +		devres_free(ptr);
> +		goto err1;
> +	}
> +
> +	*ptr = uart;
> +	devres_add(dev, ptr);
> +
> +	get_device(uart->dev);
> +
> +err1:
> +	spin_unlock_irqrestore(&uart_lock, flags);
> +
> +err0:
> +	of_node_put(node);
> +
> +	return uart;
> +}
> +EXPORT_SYMBOL_GPL(devm_serial_get_uart_by_phandle);
> +
> +
>  /*
>   * This routine is used by the interrupt handler to schedule processing in
>   * the software interrupt portion of the driver.
> @@ -2733,6 +2836,8 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
>  	 */
>  	uport->flags &= ~UPF_DEAD;
>  
> +	list_add_tail(&uport->head, &uart_list);
> +
>   out:
>  	mutex_unlock(&port->mutex);
>  	mutex_unlock(&port_mutex);
> @@ -2764,6 +2869,8 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport)
>  
>  	mutex_lock(&port_mutex);
>  
> +	list_del(&uport->head);
> +
>  	/*
>  	 * Mark the port "dead" - this prevents any opens from
>  	 * succeeding while we shut down the port.
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 297d4fa..d7a2e15 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -247,6 +247,7 @@ struct uart_port {
>  	const struct attribute_group **tty_groups;	/* all attributes (serial core use only) */
>  	struct serial_rs485     rs485;
>  	void			*private_data;		/* generic platform data pointer */
> +	struct list_head	head;			/* uarts list (lookup by phandle) */
>  };
>  
>  static inline int serial_port_in(struct uart_port *up, int offset)
> @@ -475,4 +476,13 @@ static inline int uart_handle_break(struct uart_port *port)
>  					 (cflag) & CRTSCTS || \
>  					 !((cflag) & CLOCAL))
>  
> +/*
> + * Helper functions for UART slave drivers
> + */
> +
> +/* find UART by phandle (e.g. with 'uart = <&uart2>;' then call as
> + * devm_serial_get_uart_by_phandle(dev, "uart", 0);
> + */
> +extern struct uart_port *devm_serial_get_uart_by_phandle(struct device *dev,
> +		const char *phandle, u8 index);
>  #endif /* LINUX_SERIAL_CORE_H */
> -- 
> 2.5.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249098

From"H. Nikolaus Schaller" <hns@goldelico.com>
Date2015-10-16 21:30 +0200
Message-ID<qkiRt-7pv-33@gated-at.bofh.it>
In reply to#1249050
Am 16.10.2015 um 20:39 schrieb Mark Rutland <mark.rutland@arm.com>:

> On Fri, Oct 16, 2015 at 08:08:33PM +0200, H. Nikolaus Schaller wrote:
>> 1. add uart_ports to a search list as soon as they are registered
>> 2. provide a function to search an uart_port by phandle. This copies the
>>   mechanism how devm_usb_get_phy_by_phandle() works
>> 3. add a bindings document how serial slaves should use this feature
>> 4. add Documentation how serla slaves work in general
> 
> I thought maintainers preferred the child node approach to the phandle
> approach,

> and this series comes with no rationale (nor change log,
> despite being 'v3').

For unknown reasons it was not part of the outgoing git send-email.

Today is not my day of operating command line git...

I have added it as a reply.

> 
> I don't understand. What is going on here?

There was a discussion about child vs. phandle and I came up with thousands
of technical arguments and examples from other subsystems where phandle
is common. Because I still don't believe that child node approach is the right one.

At some time of this discussion, I was asked to provide code because people
wanted to compare both ideas on code basis.

Therefore I wrote an implementation that works and shows all aspects.
I published V1 and V2 and got some comments, but not really much.

What I never got was a really convincing argumentation or principle or DT writer's
guideline why uart slaves must be subnodes (except that "uart" is a degenerate
variant of a "bus" and therefore must be prepared to have multiple child nodes).

The latest counter-example I have found is how iio adcs are accessed:
Documentation/devicetree/bindings/iio/iio-bindings.txt

One could as well argue that adcs are a "bus" (especially if they have multiple
inputs) and therefore all consumers of adc data must be their children... But they
are not.

Nothing has happened since I submittted RFC V2. I.e. there is no child node based
approach accepted. There was no significant comparison or discussion.

Therefore I took the freedom to resubmit my code and prevent it from bitrotting
in my local git repo.

Hope this explains.

BR and thanks,
Nikolaus


> Mark.
> 
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> .../devicetree/bindings/serial/slaves.txt          |  16 +++
>> Documentation/serial/slaves.txt                    |  36 +++++++
>> drivers/tty/serial/serial_core.c                   | 107 +++++++++++++++++++++
>> include/linux/serial_core.h                        |  10 ++
>> 4 files changed, 169 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/serial/slaves.txt
>> create mode 100644 Documentation/serial/slaves.txt
>> 
>> diff --git a/Documentation/devicetree/bindings/serial/slaves.txt b/Documentation/devicetree/bindings/serial/slaves.txt
>> new file mode 100644
>> index 0000000..353b87f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/serial/slaves.txt
>> @@ -0,0 +1,16 @@
>> +Device-Tree bindings for UART slave devices
>> +
>> +A node describing a slave device defines a phandle to reference the UART
>> +the device is connected to. In the (unexpected) case of two or more UARTs
>> +a list of phandles can be specified.
>> +
>> +properties:
>> +	- uart: (list of) phandle(s) of UART(s) the device is connected to
>> +
>> +
>> +example:
>> +
>> +	gps {
>> +		compatible = "wi2wi,w2sg0004";
>> +		uart = <&uart1>;
>> +	};
>> diff --git a/Documentation/serial/slaves.txt b/Documentation/serial/slaves.txt
>> new file mode 100644
>> index 0000000..6f8d44d
>> --- /dev/null
>> +++ b/Documentation/serial/slaves.txt
>> @@ -0,0 +1,36 @@
>> +UART slave device support
>> +
>> +A remote device connected to a RS232 interface is usually power controlled by the DTR line.
>> +The DTR line is managed automatically by the UART driver for open() and close() syscalls
>> +and on demand by tcsetattr().
>> +
>> +With embedded devices, the serial peripheral might be directly and always connected to the UART
>> +and there might be no physical DTR line involved. Power control (on/off) has to be done by some
>> +chip specific device driver (which we call "UART slave") through some mechanisms (I2C, GPIOs etc.)
>> +not related to the serial interface. Some devices do not explicitly tell their power state except
>> +by sending or not sending data to the UART. In such a case the device driver must be able to monitor
>> +data activity. The role of the device driver is to encapsulate such power control in a single place.
>> +
>> +This patch series allows to support such drivers by providing:
>> +* a mechanism that a slave driver can identify the UART instance it is connected to
>> +* a mechanism that UART slave drivers can register to be notified
>> +* notfications for DTR (and other modem control) state changes
>> +* notifications that the UART has received some data from the UART
>> +
>> +A slave device simply adds a phandle reference to the UART it is connected to, e.g.
>> +
>> +	gps {
>> +		compatible = "wi2wi,w2sg0004";
>> +		uart = <&uart1>;
>> +	};
>> +
>> +The slave driver calls devm_serial_get_uart_by_phandle() to identify the uart driver.
>> +This API follows the concept of devm_usb_get_phy_by_phandle().
>> +
>> +A slave device driver registers itself with serial_register_slave() to receive notifications.
>> +Notification handler callbacks can be registered by serial_register_mctrl_notification() and
>> +serial_register_rx_notification(). If an UART has registered a NULL slave or a NULL handler,
>> +no notifications are sent.
>> +
>> +RX notification handlers can define a ktermios during setup and the handler function can modify
>> +or decide to throw away each character that is passed upwards.
>> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
>> index 603d2cc..9caa33e 100644
>> --- a/drivers/tty/serial/serial_core.c
>> +++ b/drivers/tty/serial/serial_core.c
>> @@ -38,6 +38,36 @@
>> #include <asm/irq.h>
>> #include <asm/uaccess.h>
>> 
>> +static LIST_HEAD(uart_list);
>> +static DEFINE_SPINLOCK(uart_lock);
>> +
>> +/* same concept as __of_usb_find_phy */
>> +static struct uart_port *__of_serial_find_uart(struct device_node *node)
>> +{
>> +	struct uart_port  *uart;
>> +
>> +	if (!of_device_is_available(node))
>> +		return ERR_PTR(-ENODEV);
>> +
>> +	list_for_each_entry(uart, &uart_list, head) {
>> +		if (node != uart->dev->of_node)
>> +			continue;
>> +
>> +		return uart;
>> +	}
>> +
>> +	return ERR_PTR(-EPROBE_DEFER);
>> +}
>> +
>> +static void devm_serial_uart_release(struct device *dev, void *res)
>> +{
>> +	struct uart_port *uart = *(struct uart_port **)res;
>> +
>> +	/* FIXME: I don't understand the serial subsystem well enough
>> +	 * to know if we should call serial_put_uart(uart); here
>> +	 */
>> +}
>> +
>> /*
>>  * This is used to lock changes in serial line configuration.
>>  */
>> @@ -64,6 +94,79 @@ static int uart_dcd_enabled(struct uart_port *uport)
>> 	return !!(uport->status & UPSTAT_DCD_ENABLE);
>> }
>> 
>> +/**
>> + * devm_serial_get_uart_by_phandle - find the uart by phandle
>> + * @dev - device that requests this uart
>> + * @phandle - name of the property holding the uart phandle value
>> + * @index - the index of the uart
>> + *
>> + * Returns the uart_port associated with the given phandle value,
>> + * after getting a refcount to it, -ENODEV if there is no such uart or
>> + * -EPROBE_DEFER if there is a phandle to the uart, but the device is
>> + * not yet loaded. While at that, it also associates the device with
>> + * the uart using devres. On driver detach, release function is invoked
>> + * on the devres data, then, devres data is freed.
>> + *
>> + * For use by tty host and peripheral drivers.
>> + */
>> +
>> +/* same concept as devm_usb_get_phy_by_phandle() */
>> +
>> +struct uart_port *devm_serial_get_uart_by_phandle(struct device *dev,
>> +		const char *phandle, u8 index)
>> +{
>> +	struct uart_port  *uart = ERR_PTR(-ENOMEM), **ptr;
>> +	unsigned long   flags;
>> +	struct device_node *node;
>> +
>> +	if (!dev->of_node) {
>> +		dev_err(dev, "device does not have a device node entry\n");
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> +
>> +	node = of_parse_phandle(dev->of_node, phandle, index);
>> +	if (!node) {
>> +		dev_err(dev, "failed to get %s phandle in %s node\n", phandle,
>> +			dev->of_node->full_name);
>> +		return ERR_PTR(-ENODEV);
>> +	}
>> +
>> +	ptr = devres_alloc(devm_serial_uart_release, sizeof(*ptr), GFP_KERNEL);
>> +	if (!ptr) {
>> +		dev_err(dev, "failed to allocate memory for devres\n");
>> +		goto err0;
>> +	}
>> +
>> +	spin_lock_irqsave(&uart_lock, flags);
>> +
>> +	uart = __of_serial_find_uart(node);
>> +	if (IS_ERR(uart)) {
>> +		devres_free(ptr);
>> +		goto err1;
>> +	}
>> +
>> +	if (!try_module_get(uart->dev->driver->owner)) {
>> +		uart = ERR_PTR(-ENODEV);
>> +		devres_free(ptr);
>> +		goto err1;
>> +	}
>> +
>> +	*ptr = uart;
>> +	devres_add(dev, ptr);
>> +
>> +	get_device(uart->dev);
>> +
>> +err1:
>> +	spin_unlock_irqrestore(&uart_lock, flags);
>> +
>> +err0:
>> +	of_node_put(node);
>> +
>> +	return uart;
>> +}
>> +EXPORT_SYMBOL_GPL(devm_serial_get_uart_by_phandle);
>> +
>> +
>> /*
>>  * This routine is used by the interrupt handler to schedule processing in
>>  * the software interrupt portion of the driver.
>> @@ -2733,6 +2836,8 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
>> 	 */
>> 	uport->flags &= ~UPF_DEAD;
>> 
>> +	list_add_tail(&uport->head, &uart_list);
>> +
>>  out:
>> 	mutex_unlock(&port->mutex);
>> 	mutex_unlock(&port_mutex);
>> @@ -2764,6 +2869,8 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport)
>> 
>> 	mutex_lock(&port_mutex);
>> 
>> +	list_del(&uport->head);
>> +
>> 	/*
>> 	 * Mark the port "dead" - this prevents any opens from
>> 	 * succeeding while we shut down the port.
>> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
>> index 297d4fa..d7a2e15 100644
>> --- a/include/linux/serial_core.h
>> +++ b/include/linux/serial_core.h
>> @@ -247,6 +247,7 @@ struct uart_port {
>> 	const struct attribute_group **tty_groups;	/* all attributes (serial core use only) */
>> 	struct serial_rs485     rs485;
>> 	void			*private_data;		/* generic platform data pointer */
>> +	struct list_head	head;			/* uarts list (lookup by phandle) */
>> };
>> 
>> static inline int serial_port_in(struct uart_port *up, int offset)
>> @@ -475,4 +476,13 @@ static inline int uart_handle_break(struct uart_port *port)
>> 					 (cflag) & CRTSCTS || \
>> 					 !((cflag) & CLOCAL))
>> 
>> +/*
>> + * Helper functions for UART slave drivers
>> + */
>> +
>> +/* find UART by phandle (e.g. with 'uart = <&uart2>;' then call as
>> + * devm_serial_get_uart_by_phandle(dev, "uart", 0);
>> + */
>> +extern struct uart_port *devm_serial_get_uart_by_phandle(struct device *dev,
>> +		const char *phandle, u8 index);
>> #endif /* LINUX_SERIAL_CORE_H */
>> -- 
>> 2.5.1
>> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249070 — Re: [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle

Fromkbuild test robot <lkp@intel.com>
Date2015-10-16 20:50 +0200
SubjectRe: [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle
Message-ID<qkieK-6nN-19@gated-at.bofh.it>
In reply to#1249032

[Multipart message — attachments visible in raw view] — view raw

Hi Nikolaus,

[auto build test WARNING on tty/tty-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/H-Nikolaus-Schaller/UART-slave-device-support-goldelico-version/20151017-021238
config: i386-randconfig-s1-201541 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/tty/serial/serial_core.c: In function 'devm_serial_uart_release':
>> drivers/tty/serial/serial_core.c:64:20: warning: unused variable 'uart' [-Wunused-variable]
     struct uart_port *uart = *(struct uart_port **)res;
                       ^

vim +/uart +64 drivers/tty/serial/serial_core.c

    48	
    49		if (!of_device_is_available(node))
    50			return ERR_PTR(-ENODEV);
    51	
    52		list_for_each_entry(uart, &uart_list, head) {
    53			if (node != uart->dev->of_node)
    54				continue;
    55	
    56			return uart;
    57		}
    58	
    59		return ERR_PTR(-EPROBE_DEFER);
    60	}
    61	
    62	static void devm_serial_uart_release(struct device *dev, void *res)
    63	{
  > 64		struct uart_port *uart = *(struct uart_port **)res;
    65	
    66		/* FIXME: I don't understand the serial subsystem well enough
    67		 * to know if we should call serial_put_uart(uart); here
    68		 */
    69	}
    70	
    71	/*
    72	 * This is used to lock changes in serial line configuration.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1249079

FromArnd Bergmann <arnd@arndb.de>
Date2015-10-16 21:10 +0200
Message-ID<qkiy6-70o-13@gated-at.bofh.it>
In reply to#1249032
On Friday 16 October 2015 20:08:33 H. Nikolaus Schaller wrote:
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/slaves.txt
> @@ -0,0 +1,16 @@
> +Device-Tree bindings for UART slave devices
> +
> +A node describing a slave device defines a phandle to reference the UART
> +the device is connected to. In the (unexpected) case of two or more UARTs
> +a list of phandles can be specified.
> +
> +properties:
> +       - uart: (list of) phandle(s) of UART(s) the device is connected to
> +
> +
> +example:
> +
> +       gps {
> +               compatible = "wi2wi,w2sg0004";
> +               uart = <&uart1>;
> +       };
> 


I would have expected the gps device here to be a child node of the
uart in DT. Can you explain why you chose differently?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249104 — Re: [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle

Fromkbuild test robot <lkp@intel.com>
Date2015-10-16 21:40 +0200
SubjectRe: [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle
Message-ID<qkj17-7AO-1@gated-at.bofh.it>
In reply to#1249032

[Multipart message — attachments visible in raw view] — view raw

Hi Nikolaus,

[auto build test WARNING on tty/tty-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/H-Nikolaus-Schaller/UART-slave-device-support-goldelico-version/20151017-021238
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   include/linux/init.h:1: warning: no structured comments found
   kernel/sys.c:1: warning: no structured comments found
   drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found
   drivers/dma-buf/reservation.c:1: warning: no structured comments found
   include/linux/reservation.h:1: warning: no structured comments found
   include/media/v4l2-dv-timings.h:29: warning: cannot understand function prototype: 'const struct v4l2_dv_timings v4l2_dv_timings_presets[]; '
   include/media/v4l2-dv-timings.h:147: warning: No description found for parameter 'frame_height'
   include/media/v4l2-dv-timings.h:147: warning: No description found for parameter 'hfreq'
   include/media/v4l2-dv-timings.h:147: warning: No description found for parameter 'vsync'
   include/media/v4l2-dv-timings.h:147: warning: No description found for parameter 'active_width'
   include/media/v4l2-dv-timings.h:147: warning: No description found for parameter 'polarities'
   include/media/v4l2-dv-timings.h:147: warning: No description found for parameter 'interlaced'
   include/media/v4l2-dv-timings.h:147: warning: No description found for parameter 'fmt'
   include/media/v4l2-dv-timings.h:171: warning: No description found for parameter 'frame_height'
   include/media/v4l2-dv-timings.h:171: warning: No description found for parameter 'hfreq'
   include/media/v4l2-dv-timings.h:171: warning: No description found for parameter 'vsync'
   include/media/v4l2-dv-timings.h:171: warning: No description found for parameter 'polarities'
   include/media/v4l2-dv-timings.h:171: warning: No description found for parameter 'interlaced'
   include/media/v4l2-dv-timings.h:171: warning: No description found for parameter 'aspect'
   include/media/v4l2-dv-timings.h:171: warning: No description found for parameter 'fmt'
   include/media/v4l2-dv-timings.h:184: warning: No description found for parameter 'hor_landscape'
   include/media/v4l2-dv-timings.h:184: warning: No description found for parameter 'vert_portrait'
   include/media/videobuf2-core.h:112: warning: No description found for parameter 'get_dmabuf'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_alloc'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_put'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_get_dmabuf'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_get_userptr'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_put_userptr'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_prepare'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_finish'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_attach_dmabuf'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_detach_dmabuf'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_map_dmabuf'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_unmap_dmabuf'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_vaddr'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_cookie'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_num_users'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_mmap'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_buf_init'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_buf_prepare'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_buf_finish'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_buf_cleanup'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_buf_queue'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_buf_done'
   drivers/media/dvb-core/dvbdev.h:199: warning: Excess function parameter 'device' description in 'dvb_register_device'
   drivers/media/dvb-core/dvbdev.h:199: warning: Excess function parameter 'adapter_nums' description in 'dvb_register_device'
>> drivers/tty/serial/serial_core.c:117: warning: No description found for parameter 'dev'
>> drivers/tty/serial/serial_core.c:117: warning: No description found for parameter 'phandle'
>> drivers/tty/serial/serial_core.c:117: warning: No description found for parameter 'index'
   include/linux/hsi/hsi.h:150: warning: Excess struct/union/enum/typedef member 'e_handler' description in 'hsi_client'
   include/linux/hsi/hsi.h:150: warning: Excess struct/union/enum/typedef member 'pclaimed' description in 'hsi_client'
   include/linux/hsi/hsi.h:150: warning: Excess struct/union/enum/typedef member 'nb' description in 'hsi_client'

vim +/dev +117 drivers/tty/serial/serial_core.c

   101	 * @index - the index of the uart
   102	 *
   103	 * Returns the uart_port associated with the given phandle value,
   104	 * after getting a refcount to it, -ENODEV if there is no such uart or
   105	 * -EPROBE_DEFER if there is a phandle to the uart, but the device is
   106	 * not yet loaded. While at that, it also associates the device with
   107	 * the uart using devres. On driver detach, release function is invoked
   108	 * on the devres data, then, devres data is freed.
   109	 *
   110	 * For use by tty host and peripheral drivers.
   111	 */
   112	
   113	/* same concept as devm_usb_get_phy_by_phandle() */
   114	
   115	struct uart_port *devm_serial_get_uart_by_phandle(struct device *dev,
   116			const char *phandle, u8 index)
 > 117	{
   118		struct uart_port  *uart = ERR_PTR(-ENOMEM), **ptr;
   119		unsigned long   flags;
   120		struct device_node *node;
   121	
   122		if (!dev->of_node) {
   123			dev_err(dev, "device does not have a device node entry\n");
   124			return ERR_PTR(-EINVAL);
   125		}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web