| Message ID | 20191101124210.14510-32-linux@rasmusvillemoes.dk (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [v3,01/36] soc: fsl: qe: remove space-before-tab | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (904ea5d546fe35c670396e4813e15c8b075b69f1) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 35 lines checked |
Le 01/11/2019 à 13:42, Rasmus Villemoes a écrit : > The Soft UART hack is only needed for some PPC-based SOCs. To allow > building this driver for non-PPC, guard soft_uart_init() and its > helpers by CONFIG_PPC32, and use a no-op soft_uart_init() otherwise. I don't like too much ifdefs in C files, especially arch specific ifdefs in generic drivers. How do we get the QE version on ARM ? Wouldn't it be cleaner to create a helper for getting the QE version, which would be defined in an arch specific header file, calling mfspr(SPRN_SVR) on powerpc and whatever needed on arm ? Christophe > > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> > --- > drivers/tty/serial/ucc_uart.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c > index f286e91714cb..313697842e24 100644 > --- a/drivers/tty/serial/ucc_uart.c > +++ b/drivers/tty/serial/ucc_uart.c > @@ -33,7 +33,10 @@ > > #include <linux/firmware.h> > #include <soc/fsl/cpm.h> > -#include <asm/reg.h> > + > +#ifdef CONFIG_PPC32 > +#include <asm/reg.h> /* mfspr, SPRN_SVR */ > +#endif > > /* > * The GUMR flag for Soft UART. This would normally be defined in qe.h, > @@ -1096,6 +1099,8 @@ static const struct uart_ops qe_uart_pops = { > .verify_port = qe_uart_verify_port, > }; > > + > +#ifdef CONFIG_PPC32 > /* > * Obtain the SOC model number and revision level > * > @@ -1238,6 +1243,16 @@ static int soft_uart_init(struct platform_device *ofdev) > return 0; > } > > +#else /* !CONFIG_PPC32 */ > + > +static int soft_uart_init(struct platform_device *ofdev) > +{ > + return 0; > +} > + > +#endif > + > + > static int ucc_uart_probe(struct platform_device *ofdev) > { > struct device_node *np = ofdev->dev.of_node; >
On 01/11/2019 17.27, Christophe Leroy wrote: > > > Le 01/11/2019 à 13:42, Rasmus Villemoes a écrit : >> The Soft UART hack is only needed for some PPC-based SOCs. To allow >> building this driver for non-PPC, guard soft_uart_init() and its >> helpers by CONFIG_PPC32, and use a no-op soft_uart_init() otherwise. > > I don't like too much ifdefs in C files, especially arch specific ifdefs > in generic drivers. Me neither. However, > How do we get the QE version on ARM ? > > Wouldn't it be cleaner to create a helper for getting the QE version, > which would be defined in an arch specific header file, calling > mfspr(SPRN_SVR) on powerpc and whatever needed on arm ? First, that would mean we'd have to introduce "depends on PPC32 || ARM" rather than drop "depends on PPC32". Second, the way the SOC version is being used to construct the name of the microcode blob seems very ppc-specific, so I don't see how one can reasonably define an interface that would work for all architectures. But it's also kind of moot since the ARM SOCs don't need this workaround. IMO, having to add qe-specific code to arch/ is a cure worse than the disease. I'd much rather add arch-specific code to qe. Rasmus
diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c index f286e91714cb..313697842e24 100644 --- a/drivers/tty/serial/ucc_uart.c +++ b/drivers/tty/serial/ucc_uart.c @@ -33,7 +33,10 @@ #include <linux/firmware.h> #include <soc/fsl/cpm.h> -#include <asm/reg.h> + +#ifdef CONFIG_PPC32 +#include <asm/reg.h> /* mfspr, SPRN_SVR */ +#endif /* * The GUMR flag for Soft UART. This would normally be defined in qe.h, @@ -1096,6 +1099,8 @@ static const struct uart_ops qe_uart_pops = { .verify_port = qe_uart_verify_port, }; + +#ifdef CONFIG_PPC32 /* * Obtain the SOC model number and revision level * @@ -1238,6 +1243,16 @@ static int soft_uart_init(struct platform_device *ofdev) return 0; } +#else /* !CONFIG_PPC32 */ + +static int soft_uart_init(struct platform_device *ofdev) +{ + return 0; +} + +#endif + + static int ucc_uart_probe(struct platform_device *ofdev) { struct device_node *np = ofdev->dev.of_node;
The Soft UART hack is only needed for some PPC-based SOCs. To allow building this driver for non-PPC, guard soft_uart_init() and its helpers by CONFIG_PPC32, and use a no-op soft_uart_init() otherwise. Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> --- drivers/tty/serial/ucc_uart.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)