| Message ID | 20220317174627.360815-2-miquel.raynal@bootlin.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | serial: 8250: dw: RZN1 DMA support | expand |
On Thu, Mar 17, 2022 at 9:56 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > From: Phil Edworthy <phil.edworthy@renesas.com> > > This structure needs to be reused from dwlib, so let's move it into a > shared header. There is no functional change. ... > #include <linux/types.h> > +#include <linux/clk.h> I have mentioned forward declarations. So, this can be simply replaced by struct clk; > +#include <linux/notifier.h> > +#include <linux/workqueue.h> > +#include <linux/reset.h> Ditto. struct reset_control; On top of that, please keep them ordered. Otherwise it looks good to me.
Hi Andy, andy.shevchenko@gmail.com wrote on Fri, 18 Mar 2022 12:51:29 +0200: > On Thu, Mar 17, 2022 at 9:56 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > From: Phil Edworthy <phil.edworthy@renesas.com> > > > > This structure needs to be reused from dwlib, so let's move it into a > > shared header. There is no functional change. > > ... > > > #include <linux/types.h> > > > +#include <linux/clk.h> > > I have mentioned forward declarations. Why do you want forward declarations more than includes? > So, this can be simply replaced by > > struct clk; > > > +#include <linux/notifier.h> > > +#include <linux/workqueue.h> And why these two should remain but reset and clk be replaced? > > > +#include <linux/reset.h> > > Ditto. > > struct reset_control; > > On top of that, please keep them ordered. > > Otherwise it looks good to me. > Thanks, Miquèl
Hi Andy, andy.shevchenko@gmail.com wrote on Tue, 29 Mar 2022 14:11:21 +0300: > On Tue, Mar 29, 2022 at 10:10:49AM +0200, Miquel Raynal wrote: > > andy.shevchenko@gmail.com wrote on Fri, 18 Mar 2022 12:51:29 +0200: > > > On Thu, Mar 17, 2022 at 9:56 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > ... > > > > > +#include <linux/clk.h> > > > > > > I have mentioned forward declarations. > > > > Why do you want forward declarations more than includes? > > Because they will speed up the kernel build and avoid dirtifying the namespace > (less possible collisions). > > > > So, this can be simply replaced by > > > > > > struct clk; > > > > > > > +#include <linux/notifier.h> > > > > +#include <linux/workqueue.h> > > > > And why these two should remain but reset and clk be replaced? > > Because these one are being used, clk and reset are not (the pointers > are opaque from the point of view of this header). Oh yeah, I forgot that point, thanks for the clarification. Thanks, Miquèl
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c index 96a62e95726b..d89731d6c94c 100644 --- a/drivers/tty/serial/8250/8250_dw.c +++ b/drivers/tty/serial/8250/8250_dw.c @@ -42,22 +42,6 @@ #define DW_UART_QUIRK_ARMADA_38X BIT(1) #define DW_UART_QUIRK_SKIP_SET_RATE BIT(2) -struct dw8250_data { - struct dw8250_port_data data; - - u8 usr_reg; - int msr_mask_on; - int msr_mask_off; - struct clk *clk; - struct clk *pclk; - struct notifier_block clk_notifier; - struct work_struct clk_work; - struct reset_control *rst; - - unsigned int skip_autocfg:1; - unsigned int uart_16550_compatible:1; -}; - static inline struct dw8250_data *to_dw8250_data(struct dw8250_port_data *data) { return container_of(data, struct dw8250_data, data); diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h index 83d528e5cc21..6ffbf502829e 100644 --- a/drivers/tty/serial/8250/8250_dwlib.h +++ b/drivers/tty/serial/8250/8250_dwlib.h @@ -2,6 +2,10 @@ /* Synopsys DesignWare 8250 library header file. */ #include <linux/types.h> +#include <linux/clk.h> +#include <linux/notifier.h> +#include <linux/workqueue.h> +#include <linux/reset.h> #include "8250.h" @@ -16,5 +20,21 @@ struct dw8250_port_data { u8 dlf_size; }; +struct dw8250_data { + struct dw8250_port_data data; + + u8 usr_reg; + int msr_mask_on; + int msr_mask_off; + struct clk *clk; + struct clk *pclk; + struct notifier_block clk_notifier; + struct work_struct clk_work; + struct reset_control *rst; + + unsigned int skip_autocfg:1; + unsigned int uart_16550_compatible:1; +}; + void dw8250_do_set_termios(struct uart_port *p, struct ktermios *termios, struct ktermios *old); void dw8250_setup_port(struct uart_port *p);