All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Gortmaker <paul.gortmaker@windriver.com>
To: James Hogan <james.hogan@imgtec.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jslaby@suse.com>, Ralf Baechle <ralf@linux-mips.org>,
	<linux-mips@linux-mips.org>, <stable@vger.kernel.org>
Subject: Re: [PATCH 2/2] ttyFDC: Fix build problems due to use of module_{init,exit}
Date: Tue, 6 Oct 2015 18:23:00 -0400	[thread overview]
Message-ID: <20151006222300.GI14103@windriver.com> (raw)
In-Reply-To: <1444140726-5740-3-git-send-email-james.hogan@imgtec.com>

[[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
> 

WARNING: multiple messages have this Message-ID (diff)
From: Paul Gortmaker <paul.gortmaker@windriver.com>
To: James Hogan <james.hogan@imgtec.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jslaby@suse.com>, Ralf Baechle <ralf@linux-mips.org>,
	linux-mips@linux-mips.org, stable@vger.kernel.org
Subject: Re: [PATCH 2/2] ttyFDC: Fix build problems due to use of module_{init,exit}
Date: Tue, 6 Oct 2015 18:23:00 -0400	[thread overview]
Message-ID: <20151006222300.GI14103@windriver.com> (raw)
Message-ID: <20151006222300.jaUcjSNiZ9x1vag8SkP4MxgJK9HpkHq_yR6EXIgeP3Q@z> (raw)
In-Reply-To: <1444140726-5740-3-git-send-email-james.hogan@imgtec.com>

[[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
> 

  reply	other threads:[~2015-10-06 22:23 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-06 14:12 [PATCH 0/2] ttyFDC: Fix build problems due to use of module_{init,exit} James Hogan
2015-10-06 14:12 ` James Hogan
2015-10-06 14:12 ` [PATCH 1/2] MIPS: CDMM: Add builtin_mips_cdmm_driver() macro James Hogan
2015-10-06 14:12   ` James Hogan
2015-10-06 14:12 ` [PATCH 2/2] ttyFDC: Fix build problems due to use of module_{init,exit} James Hogan
2015-10-06 14:12   ` James Hogan
2015-10-06 22:23   ` Paul Gortmaker [this message]
2015-10-06 22:23     ` Paul Gortmaker
2015-10-06 22:26     ` James Hogan
2015-10-06 22:26       ` James Hogan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151006222300.GI14103@windriver.com \
    --to=paul.gortmaker@windriver.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=james.hogan@imgtec.com \
    --cc=jslaby@suse.com \
    --cc=linux-mips@linux-mips.org \
    --cc=ralf@linux-mips.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.