[PATCH 0/2] ttyFDC: Fix build problems due to use of module_{init,exit}

[PATCH 0/2] ttyFDC: Fix build problems due to use of module_{init,exit}

[James Hogan]

Fix ttyFDC build breakage since v4.2-rc3, due to use of module_driver()
without including module.h (its currently a builtin only driver).

The first patch adds a macro used by second patch, so they both need to
go into stable v4.2 together.

James Hogan (2):
  MIPS: CDMM: Add builtin_mips_cdmm_driver() macro
  ttyFDC: Fix build problems due to use of module_{init,exit}

 arch/mips/include/asm/cdmm.h | 11 +++++++++++
 drivers/tty/mips_ejtag_fdc.c | 35 +----------------------------------
 2 files changed, 12 insertions(+), 34 deletions(-)

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: linux-mips@linux-mips.org
Cc: <stable@vger.kernel.org>
-- 
2.4.9

[PATCH 1/2] MIPS: CDMM: Add builtin_mips_cdmm_driver() macro

[James Hogan]

Add helper macro builtin_mips_cdmm_driver() for builtin CDMM drivers
that don't do anything special in init and have no exit. The
module_mips_cdmm_driver() helper isn't really appropriate for drivers
that can't be built as a module.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: <stable@vger.kernel.org> # 4.2.x-
---
 arch/mips/include/asm/cdmm.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/mips/include/asm/cdmm.h b/arch/mips/include/asm/cdmm.h
index bece2064cc8c..c06dbf8ba937 100644
--- a/arch/mips/include/asm/cdmm.h
+++ b/arch/mips/include/asm/cdmm.h
@@ -84,6 +84,17 @@ void mips_cdmm_driver_unregister(struct mips_cdmm_driver *);
 	module_driver(__mips_cdmm_driver, mips_cdmm_driver_register, \
 			mips_cdmm_driver_unregister)
 
+/*
+ * builtin_mips_cdmm_driver() - Helper macro for drivers that don't do anything
+ * special in init and have no exit. This eliminates some boilerplate. Each
+ * driver may only use this macro once, and calling it replaces device_initcall
+ * (or in some cases, the legacy __initcall). This is meant to be a direct
+ * parallel of module_mips_cdmm_driver() above but without the __exit stuff that
+ * is not used for builtin cases.
+ */
+#define builtin_mips_cdmm_driver(__mips_cdmm_driver) \
+	builtin_driver(__mips_cdmm_driver, mips_cdmm_driver_register)
+
 /* drivers/tty/mips_ejtag_fdc.c */
 
 #ifdef CONFIG_MIPS_EJTAG_FDC_EARLYCON
-- 
2.4.9

[PATCH 2/2] ttyFDC: Fix build problems due to use of module_{init,exit}

[James Hogan]

Commit 0fd972a7d91d (module: relocate module_init from init.h to
module.h) broke the build of ttyFDC driver due to that driver's (mis)use
of module_mips_cdmm_driver() without first including module.h, for
example:

In file included from ./arch/mips/include/asm/cdmm.h +11 :0,
                 from drivers/tty/mips_ejtag_fdc.c +34 :
include/linux/device.h +1295 :1: warning: data definition has no type or storage class
./arch/mips/include/asm/cdmm.h +84 :2: note: in expansion of macro ‘module_driver’
drivers/tty/mips_ejtag_fdc.c +1157 :1: note: in expansion of macro ‘module_mips_cdmm_driver’
include/linux/device.h +1295 :1: error: type defaults to ‘int’ in declaration of ‘module_init’ [-Werror=implicit-int]
./arch/mips/include/asm/cdmm.h +84 :2: note: in expansion of macro ‘module_driver’
drivers/tty/mips_ejtag_fdc.c +1157 :1: note: in expansion of macro ‘module_mips_cdmm_driver’
drivers/tty/mips_ejtag_fdc.c +1157 :1: warning: parameter names (without types) in function declaration

Instead of just adding the module.h include, switch to using the new
builtin_mips_cdmm_driver() helper macro and drop the remove callback,
since it isn't needed. If module support is added later, the code can
always be resurrected.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: linux-mips@linux-mips.org
Cc: <stable@vger.kernel.org> # 4.2.x-
---
 drivers/tty/mips_ejtag_fdc.c | 35 +----------------------------------
 1 file changed, 1 insertion(+), 34 deletions(-)

diff --git a/drivers/tty/mips_ejtag_fdc.c b/drivers/tty/mips_ejtag_fdc.c
index a8c8cfd52a23..57ff5d3157a6 100644
--- a/drivers/tty/mips_ejtag_fdc.c
+++ b/drivers/tty/mips_ejtag_fdc.c
@@ -1048,38 +1048,6 @@ err_destroy_ports:
 	return ret;
 }
 
-static int mips_ejtag_fdc_tty_remove(struct mips_cdmm_device *dev)
-{
-	struct mips_ejtag_fdc_tty *priv = mips_cdmm_get_drvdata(dev);
-	struct mips_ejtag_fdc_tty_port *dport;
-	int nport;
-	unsigned int cfg;
-
-	if (priv->irq >= 0) {
-		raw_spin_lock_irq(&priv->lock);
-		cfg = mips_ejtag_fdc_read(priv, REG_FDCFG);
-		/* Disable interrupts */
-		cfg &= ~(REG_FDCFG_TXINTTHRES | REG_FDCFG_RXINTTHRES);
-		cfg |= REG_FDCFG_TXINTTHRES_DISABLED;
-		cfg |= REG_FDCFG_RXINTTHRES_DISABLED;
-		mips_ejtag_fdc_write(priv, REG_FDCFG, cfg);
-		raw_spin_unlock_irq(&priv->lock);
-	} else {
-		priv->removing = true;
-		del_timer_sync(&priv->poll_timer);
-	}
-	kthread_stop(priv->thread);
-	if (dev->cpu == 0)
-		mips_ejtag_fdc_con.tty_drv = NULL;
-	tty_unregister_driver(priv->driver);
-	for (nport = 0; nport < NUM_TTY_CHANNELS; nport++) {
-		dport = &priv->ports[nport];
-		tty_port_destroy(&dport->port);
-	}
-	put_tty_driver(priv->driver);
-	return 0;
-}
-
 static int mips_ejtag_fdc_tty_cpu_down(struct mips_cdmm_device *dev)
 {
 	struct mips_ejtag_fdc_tty *priv = mips_cdmm_get_drvdata(dev);
@@ -1152,12 +1120,11 @@ static struct mips_cdmm_driver mips_ejtag_fdc_tty_driver = {
 		.name	= "mips_ejtag_fdc",
 	},
 	.probe		= mips_ejtag_fdc_tty_probe,
-	.remove		= mips_ejtag_fdc_tty_remove,
 	.cpu_down	= mips_ejtag_fdc_tty_cpu_down,
 	.cpu_up		= mips_ejtag_fdc_tty_cpu_up,
 	.id_table	= mips_ejtag_fdc_tty_ids,
 };
-module_mips_cdmm_driver(mips_ejtag_fdc_tty_driver);
+builtin_mips_cdmm_driver(mips_ejtag_fdc_tty_driver);
 
 static int __init mips_ejtag_fdc_init_console(void)
 {
-- 
2.4.9

Re: [PATCH 2/2] ttyFDC: Fix build problems due to use of module_{init,exit}

[Paul Gortmaker]

[[PATCH 2/2] ttyFDC: Fix build problems due to use of module_{init,exit}] On 06/10/2015 (Tue 15:12) James Hogan wrote:

> Commit 0fd972a7d91d (module: relocate module_init from init.h to
> module.h) broke the build of ttyFDC driver due to that driver's (mis)use
> of module_mips_cdmm_driver() without first including module.h, for
> example:

The driver must not be hooked into any mips defconfigs or my build
coverage should have caught it (or it was merged after my change went on
in)  -- in any case this is only the 2nd that slipped through out of
the big shuffle that came from that, so I think that is not bad.

> 
> In file included from ./arch/mips/include/asm/cdmm.h +11 :0,
>                  from drivers/tty/mips_ejtag_fdc.c +34 :
> include/linux/device.h +1295 :1: warning: data definition has no type or storage class
> ./arch/mips/include/asm/cdmm.h +84 :2: note: in expansion of macro ‘module_driver’
> drivers/tty/mips_ejtag_fdc.c +1157 :1: note: in expansion of macro ‘module_mips_cdmm_driver’
> include/linux/device.h +1295 :1: error: type defaults to ‘int’ in declaration of ‘module_init’ [-Werror=implicit-int]
> ./arch/mips/include/asm/cdmm.h +84 :2: note: in expansion of macro ‘module_driver’
> drivers/tty/mips_ejtag_fdc.c +1157 :1: note: in expansion of macro ‘module_mips_cdmm_driver’
> drivers/tty/mips_ejtag_fdc.c +1157 :1: warning: parameter names (without types) in function declaration
> 
> Instead of just adding the module.h include, switch to using the new
> builtin_mips_cdmm_driver() helper macro and drop the remove callback,
> since it isn't needed. If module support is added later, the code can
> always be resurrected.

Thanks for taking the cleanup approach here and not the one line
approach.  Saves me doing the cleanup work myself later.

Acked-by: Paul Gortmaker <paul.gortmaker@windriver.com>

P.
--

> 
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jslaby@suse.com>
> Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> Cc: linux-mips@linux-mips.org
> Cc: <stable@vger.kernel.org> # 4.2.x-
> ---
>  drivers/tty/mips_ejtag_fdc.c | 35 +----------------------------------
>  1 file changed, 1 insertion(+), 34 deletions(-)
> 
> diff --git a/drivers/tty/mips_ejtag_fdc.c b/drivers/tty/mips_ejtag_fdc.c
> index a8c8cfd52a23..57ff5d3157a6 100644
> --- a/drivers/tty/mips_ejtag_fdc.c
> +++ b/drivers/tty/mips_ejtag_fdc.c
> @@ -1048,38 +1048,6 @@ err_destroy_ports:
>  	return ret;
>  }
>  
> -static int mips_ejtag_fdc_tty_remove(struct mips_cdmm_device *dev)
> -{
> -	struct mips_ejtag_fdc_tty *priv = mips_cdmm_get_drvdata(dev);
> -	struct mips_ejtag_fdc_tty_port *dport;
> -	int nport;
> -	unsigned int cfg;
> -
> -	if (priv->irq >= 0) {
> -		raw_spin_lock_irq(&priv->lock);
> -		cfg = mips_ejtag_fdc_read(priv, REG_FDCFG);
> -		/* Disable interrupts */
> -		cfg &= ~(REG_FDCFG_TXINTTHRES | REG_FDCFG_RXINTTHRES);
> -		cfg |= REG_FDCFG_TXINTTHRES_DISABLED;
> -		cfg |= REG_FDCFG_RXINTTHRES_DISABLED;
> -		mips_ejtag_fdc_write(priv, REG_FDCFG, cfg);
> -		raw_spin_unlock_irq(&priv->lock);
> -	} else {
> -		priv->removing = true;
> -		del_timer_sync(&priv->poll_timer);
> -	}
> -	kthread_stop(priv->thread);
> -	if (dev->cpu == 0)
> -		mips_ejtag_fdc_con.tty_drv = NULL;
> -	tty_unregister_driver(priv->driver);
> -	for (nport = 0; nport < NUM_TTY_CHANNELS; nport++) {
> -		dport = &priv->ports[nport];
> -		tty_port_destroy(&dport->port);
> -	}
> -	put_tty_driver(priv->driver);
> -	return 0;
> -}
> -
>  static int mips_ejtag_fdc_tty_cpu_down(struct mips_cdmm_device *dev)
>  {
>  	struct mips_ejtag_fdc_tty *priv = mips_cdmm_get_drvdata(dev);
> @@ -1152,12 +1120,11 @@ static struct mips_cdmm_driver mips_ejtag_fdc_tty_driver = {
>  		.name	= "mips_ejtag_fdc",
>  	},
>  	.probe		= mips_ejtag_fdc_tty_probe,
> -	.remove		= mips_ejtag_fdc_tty_remove,
>  	.cpu_down	= mips_ejtag_fdc_tty_cpu_down,
>  	.cpu_up		= mips_ejtag_fdc_tty_cpu_up,
>  	.id_table	= mips_ejtag_fdc_tty_ids,
>  };
> -module_mips_cdmm_driver(mips_ejtag_fdc_tty_driver);
> +builtin_mips_cdmm_driver(mips_ejtag_fdc_tty_driver);
>  
>  static int __init mips_ejtag_fdc_init_console(void)
>  {
> -- 
> 2.4.9
> 

Re: [PATCH 2/2] ttyFDC: Fix build problems due to use of module_{init,exit}

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 2286 bytes --]

Hi Paul,

On Tue, Oct 06, 2015 at 06:23:00PM -0400, Paul Gortmaker wrote:
> [[PATCH 2/2] ttyFDC: Fix build problems due to use of module_{init,exit}] On 06/10/2015 (Tue 15:12) James Hogan wrote:
> 
> > Commit 0fd972a7d91d (module: relocate module_init from init.h to
> > module.h) broke the build of ttyFDC driver due to that driver's (mis)use
> > of module_mips_cdmm_driver() without first including module.h, for
> > example:
> 
> The driver must not be hooked into any mips defconfigs or my build
> coverage should have caught it (or it was merged after my change went on

Indeed, something I intend to fix (or at least get enabled in our
buildbot builds).

> in)  -- in any case this is only the 2nd that slipped through out of
> the big shuffle that came from that, so I think that is not bad.

embarrasingly I think the other was the metag_da driver which this one
was based on. I should have guessed it'd affect ttyFDC too :-).

> 
> > 
> > In file included from ./arch/mips/include/asm/cdmm.h +11 :0,
> >                  from drivers/tty/mips_ejtag_fdc.c +34 :
> > include/linux/device.h +1295 :1: warning: data definition has no type or storage class
> > ./arch/mips/include/asm/cdmm.h +84 :2: note: in expansion of macro ‘module_driver’
> > drivers/tty/mips_ejtag_fdc.c +1157 :1: note: in expansion of macro ‘module_mips_cdmm_driver’
> > include/linux/device.h +1295 :1: error: type defaults to ‘int’ in declaration of ‘module_init’ [-Werror=implicit-int]
> > ./arch/mips/include/asm/cdmm.h +84 :2: note: in expansion of macro ‘module_driver’
> > drivers/tty/mips_ejtag_fdc.c +1157 :1: note: in expansion of macro ‘module_mips_cdmm_driver’
> > drivers/tty/mips_ejtag_fdc.c +1157 :1: warning: parameter names (without types) in function declaration
> > 
> > Instead of just adding the module.h include, switch to using the new
> > builtin_mips_cdmm_driver() helper macro and drop the remove callback,
> > since it isn't needed. If module support is added later, the code can
> > always be resurrected.
> 
> Thanks for taking the cleanup approach here and not the one line
> approach.  Saves me doing the cleanup work myself later.
> 
> Acked-by: Paul Gortmaker <paul.gortmaker@windriver.com>

Thanks
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]