Re: [RFC] uart: add ISR callback mechanism for UART drivers


Daniel Leung <daniel.leung@...>
 

On Thu, Mar 03, 2016 at 05:07:31PM +0100, Tomasz Bursztyka wrote:
Hi Daniel,

I am a bit late on that one. I fully agree on such addition.
1 comment below:

+/**
+ * @brief Define the application callback function signature for UART.
+ *
+ * @param port Device struct for the UART device.
+ */
+typedef void (*uart_irq_callback_t)(struct device *port);
+
Could be wise to add a void *user_data private pointer through the callback.
It's something we are missing not only here (gpio etc...)
Do you have a particular use case in mind, w.r.t. UART, that requires
user data? At this point, the patch is to simply move the IRQ setup
code into the UART drivers. This is not a callback in generic terms,
but just a an ISR calling another ISR. Since the first ISR does not have
any additional user data, the second ISR should not be dependent on
having additional information.

+/* For configuring IRQ on each individual UART device. Internal use only. */
+typedef void (*uart_irq_config_func_t)(struct device *port);
+
/** @brief UART device configuration.*/
struct uart_device_config {
/**
@@ -90,6 +100,10 @@ struct uart_device_config {
#ifdef CONFIG_PCI
struct pci_dev_info pci_dev;
#endif /* CONFIG_PCI */
+
+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+ uart_irq_config_func_t irq_config_func;
+#endif
};

/** @brief Driver API structure. */
@@ -145,6 +159,9 @@ struct uart_driver_api {
/** Interrupt driven input hook function */
int (*irq_input_hook)(struct device *dev, uint8_t byte);

+ /** Set the callback function */
+ void (*irq_callback_set)(struct device *dev, uart_irq_callback_t cb);
+
#endif

#ifdef CONFIG_UART_LINE_CTRL
@@ -516,6 +533,30 @@ static inline void uart_irq_input_hook_set(struct device *dev,
}
}

+
+/**
+ * @brief Set the IRQ callback function pointer.
+ *
+ * This sets up the callback for IRQ. When an IRQ is triggered,
+ * the specified function will be called.
+ *
+ * @param dev UART device structure.
+ * @param cb Pointer to the callback function.
+ *
+ * @return N/A
+ */
+static inline void uart_irq_callback_set(struct device *dev,
+ uart_irq_callback_t cb)
so it would have a void *user_data parameter as well

+{
+ struct uart_driver_api *api;
+
+ api = (struct uart_driver_api *)dev->driver_api;
+
+ if ((api != NULL) && (api->irq_callback_set != NULL)) {
I guess we do this api != NULL check because of the
UART_INTERRUPT_DRIVEN option.
I guess at some point we will be only interrupt driven anyway.
The UART drivers are done in a way where driver_api is NULL
if the driver init fails, hence the (api != NULL) check.


-----
Daniel Leung

Join devel@lists.zephyrproject.org to automatically receive all group messages.