| Message ID | 20220317174627.360815-2-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| 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
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). > > > +#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 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);