Message ID | c46694357c903d8791f07e2ec287c684832056a5.1445018913.git.hns@goldelico.com |
---|---|
State | Under Review, archived |
Headers | show |
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 devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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 devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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 */
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