Re: [PATCH resend] Make the dev_*() family of macros in device.h complete

Re: [PATCH resend] Make the dev_*() family of macros in device.h complete

[Tilman Schmidt]

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

Am 29.10.2007 23:43 schrieb Emil Medve:
> Removed duplicates defined elsewhere
> 
> Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>

Acked-by: Tilman Schmidt <tilman@imap.cc>

for:

> diff --git a/drivers/isdn/gigaset/gigaset.h b/drivers/isdn/gigaset/gigaset.h
> index a0317ab..02bdaf2 100644
> --- a/drivers/isdn/gigaset/gigaset.h
> +++ b/drivers/isdn/gigaset/gigaset.h
> @@ -106,12 +106,6 @@ enum debuglevel {
>  					 activated */
>  };
>  
> -/* missing from linux/device.h ... */
> -#ifndef dev_notice
> -#define dev_notice(dev, format, arg...)		\
> -	dev_printk(KERN_NOTICE , dev , format , ## arg)
> -#endif
> -
>  /* Kernel message macros for situations where dev_printk and friends cannot be
>   * used for lack of reliable access to a device structure.
>   * linux/usb.h already contains these but in an obsolete form which clutters

-- 
Tilman Schmidt                          E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 253 bytes --]

Re: [PATCH resend] Make the dev_*() family of macros in device.h complete

[Greg KH]

On Mon, Oct 29, 2007 at 05:43:15PM -0500, Emil Medve wrote:
> Removed duplicates defined elsewhere
> 
> Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
> ---
> 
> Reseding the patch to a larger audience
> 
> The macros are defined in the relative order KERN_* are defined in kernel.h
> 
> linux-2.6> scripts/checkpatch.pl 0001-Make-the-dev_-family-of-macros-in-device.h-comple.patch 
> Your patch has no obvious style problems and is ready for submission.
> 
>  drivers/i2c/chips/isp1301_omap.c |    6 ------
>  drivers/isdn/gigaset/gigaset.h   |    6 ------
>  include/linux/device.h           |   26 ++++++++++++++++----------
>  3 files changed, 16 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/i2c/chips/isp1301_omap.c b/drivers/i2c/chips/isp1301_omap.c
> index fe04e46..35b9909 100644
> --- a/drivers/i2c/chips/isp1301_omap.c
> +++ b/drivers/i2c/chips/isp1301_omap.c
> @@ -259,12 +259,6 @@ static inline const char *state_name(struct isp1301 *isp)
>  	return state_string(isp->otg.state);
>  }
>  
> -#ifdef	VERBOSE
> -#define	dev_vdbg			dev_dbg
> -#else
> -#define	dev_vdbg(dev, fmt, arg...)	do{}while(0)
> -#endif
> -
>  /*-------------------------------------------------------------------------*/
>  
>  /* NOTE:  some of this ISP1301 setup is specific to H2 boards;
> diff --git a/drivers/isdn/gigaset/gigaset.h b/drivers/isdn/gigaset/gigaset.h
> index a0317ab..02bdaf2 100644
> --- a/drivers/isdn/gigaset/gigaset.h
> +++ b/drivers/isdn/gigaset/gigaset.h
> @@ -106,12 +106,6 @@ enum debuglevel {
>  					 activated */
>  };
>  
> -/* missing from linux/device.h ... */
> -#ifndef dev_notice
> -#define dev_notice(dev, format, arg...)		\
> -	dev_printk(KERN_NOTICE , dev , format , ## arg)
> -#endif
> -
>  /* Kernel message macros for situations where dev_printk and friends cannot be
>   * used for lack of reliable access to a device structure.
>   * linux/usb.h already contains these but in an obsolete form which clutters
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 2e15822..e4f8a1c 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -557,9 +557,24 @@ extern const char *dev_driver_string(struct device *dev);
>  #define dev_printk(level, dev, format, arg...)	\
>  	printk(level "%s %s: " format , dev_driver_string(dev) , (dev)->bus_id , ## arg)
>  
> +#define dev_emerg(dev, format, arg...)		\
> +	dev_printk(KERN_EMERG, dev, format, ## arg)
> +#define dev_alert(dev, format, arg...)		\
> +	dev_printk(KERN_ALERT, dev, format, ## arg)
> +#define dev_crit(dev, format, arg...)		\
> +	dev_printk(KERN_CRIT, dev, format, ## arg)
> +#define dev_err(dev, format, arg...)		\
> +	dev_printk(KERN_ERR, dev, format, ## arg)
> +#define dev_warn(dev, format, arg...)		\
> +	dev_printk(KERN_WARNING, dev, format, ## arg)
> +#define dev_notice(dev, format, arg...)		\
> +	dev_printk(KERN_NOTICE, dev, format, ## arg)
> +#define dev_info(dev, format, arg...)		\
> +	dev_printk(KERN_INFO, dev, format, ## arg)
> +
>  #ifdef DEBUG
>  #define dev_dbg(dev, format, arg...)		\
> -	dev_printk(KERN_DEBUG , dev , format , ## arg)
> +	dev_printk(KERN_DEBUG, dev, format, ## arg)

Those extra spaces are there for a good reason, older versions of gcc
are broken without it.  So please, put them all back...

thanks,

greg k-h

RE: [PATCH resend] Make the dev_*() family of macros in device.hcomplete

[Medve Emilian-EMMEDVE1]

Hi Greg K-H,


> > +#define dev_info(dev, format, arg...)		\
> > +	dev_printk(KERN_INFO, dev, format, ## arg)
> > +
> >  #ifdef DEBUG
> >  #define dev_dbg(dev, format, arg...)		\
> > -	dev_printk(KERN_DEBUG , dev , format , ## arg)
> > +	dev_printk(KERN_DEBUG, dev, format, ## arg)
> 
> Those extra spaces are there for a good reason, older versions of gcc
> are broken without it.  So please, put them all back...

You mean I should add spaces before commas only where they were
initially or to all new code and/or macros? I've observed other kernel
code and more often there are no spaces before commas. I'm asking
because the CodingStyle document is not very explicit about this rule.


Thank you,
Emil.

Re: [PATCH resend] Make the dev_*() family of macros in device.hcomplete

[Randy Dunlap]

On Tue, 30 Oct 2007 05:11:24 -0700 Medve Emilian-EMMEDVE1 wrote:

> Hi Greg K-H,
> 
> 
> > > +#define dev_info(dev, format, arg...)		\
> > > +	dev_printk(KERN_INFO, dev, format, ## arg)
> > > +
> > >  #ifdef DEBUG
> > >  #define dev_dbg(dev, format, arg...)		\
> > > -	dev_printk(KERN_DEBUG , dev , format , ## arg)
> > > +	dev_printk(KERN_DEBUG, dev, format, ## arg)
> > 
> > Those extra spaces are there for a good reason, older versions of gcc
> > are broken without it.  So please, put them all back...
> 
> You mean I should add spaces before commas only where they were
> initially or to all new code and/or macros? I've observed other kernel
> code and more often there are no spaces before commas. I'm asking
> because the CodingStyle document is not very explicit about this rule.

My (hazy?) recollection of the gcc issue is that it is only the
comma before the ## that needs spaces around it.

I'm not even sure that we are still supporting that gcc version,
but you should just put the spaces back to be safe IMO.

---
~Randy

Re: [PATCH resend] Make the dev_*() family of macros in device.hcomplete

[Greg KH]

On Tue, Oct 30, 2007 at 05:11:24AM -0700, Medve Emilian-EMMEDVE1 wrote:
> Hi Greg K-H,
> 
> 
> > > +#define dev_info(dev, format, arg...)		\
> > > +	dev_printk(KERN_INFO, dev, format, ## arg)
> > > +
> > >  #ifdef DEBUG
> > >  #define dev_dbg(dev, format, arg...)		\
> > > -	dev_printk(KERN_DEBUG , dev , format , ## arg)
> > > +	dev_printk(KERN_DEBUG, dev, format, ## arg)
> > 
> > Those extra spaces are there for a good reason, older versions of gcc
> > are broken without it.  So please, put them all back...
> 
> You mean I should add spaces before commas only where they were
> initially or to all new code and/or macros?

Put it back where it was, and do the same for all other macros.

> I've observed other kernel code and more often there are no spaces
> before commas. I'm asking because the CodingStyle document is not very
> explicit about this rule.

This is a gcc rule, for variable length macros, not a CodingStyle
guideline.  It just will not work without it :)

thanks,

greg k-h

Re: [PATCH resend] Make the dev_*() family of macros in device.hcomplete

[Andrew Morton]

On Tue, 30 Oct 2007 08:40:08 -0700
Greg KH <gregkh@suse.de> wrote:

> On Tue, Oct 30, 2007 at 05:11:24AM -0700, Medve Emilian-EMMEDVE1 wrote:
> > Hi Greg K-H,
> > 
> > 
> > > > +#define dev_info(dev, format, arg...)		\
> > > > +	dev_printk(KERN_INFO, dev, format, ## arg)
> > > > +
> > > >  #ifdef DEBUG
> > > >  #define dev_dbg(dev, format, arg...)		\
> > > > -	dev_printk(KERN_DEBUG , dev , format , ## arg)
> > > > +	dev_printk(KERN_DEBUG, dev, format, ## arg)
> > > 
> > > Those extra spaces are there for a good reason, older versions of gcc
> > > are broken without it.  So please, put them all back...
> > 
> > You mean I should add spaces before commas only where they were
> > initially or to all new code and/or macros?
> 
> Put it back where it was, and do the same for all other macros.
> 
> > I've observed other kernel code and more often there are no spaces
> > before commas. I'm asking because the CodingStyle document is not very
> > explicit about this rule.
> 
> This is a gcc rule, for variable length macros, not a CodingStyle
> guideline.  It just will not work without it :)
> 

The space-before-a-comma requirement was for gcc-2.95, iirc.

It got to the stage where I was the only person testing with gcc-2.95 so I
spent inordinate amounts of time adding spaces before people's newly-added
commas.  Fortunately we abamdoned that gcc version so the space-before-a-comma
requirement no longer exists.

RE: [PATCH resend] Make the dev_*() family of macros in device.hcomplete

[Medve Emilian-EMMEDVE1]

Hello Andrew,


Thanks for the detailed background.

By now I also submitted a patch with extra spaces. Which one would be a
better candidate for applying in a maintainer's tree (which tree?)?
Could you please pick up one of the versions? Should I resubmit the
first version without extra spaces?


Cheers,
Emil.


> -----Original Message-----
> From: Andrew Morton [mailto:akpm@linux-foundation.org] 
> Sent: Thursday, November 01, 2007 6:57 PM
> To: Greg KH
> Cc: Medve Emilian-EMMEDVE1; linux-kernel@vger.kernel.org; 
> hjlipp@web.de; tilman@imap.cc; 
> gigaset307x-common@lists.sourceforge.net; kkeil@suse.de; 
> isdn4linux@listserv.isdn4linux.de; mac@melware.de; 
> linux-omap-open-source@linux.omap.com
> Subject: Re: [PATCH resend] Make the dev_*() family of macros 
> in device.hcomplete
> 
> On Tue, 30 Oct 2007 08:40:08 -0700
> Greg KH <gregkh@suse.de> wrote:
> 
> > On Tue, Oct 30, 2007 at 05:11:24AM -0700, Medve 
> Emilian-EMMEDVE1 wrote:
> > > Hi Greg K-H,
> > > 
> > > 
> > > > > +#define dev_info(dev, format, arg...)		\
> > > > > +	dev_printk(KERN_INFO, dev, format, ## arg)
> > > > > +
> > > > >  #ifdef DEBUG
> > > > >  #define dev_dbg(dev, format, arg...)		\
> > > > > -	dev_printk(KERN_DEBUG , dev , format , ## arg)
> > > > > +	dev_printk(KERN_DEBUG, dev, format, ## arg)
> > > > 
> > > > Those extra spaces are there for a good reason, older 
> versions of gcc
> > > > are broken without it.  So please, put them all back...
> > > 
> > > You mean I should add spaces before commas only where they were
> > > initially or to all new code and/or macros?
> > 
> > Put it back where it was, and do the same for all other macros.
> > 
> > > I've observed other kernel code and more often there are no spaces
> > > before commas. I'm asking because the CodingStyle 
> document is not very
> > > explicit about this rule.
> > 
> > This is a gcc rule, for variable length macros, not a CodingStyle
> > guideline.  It just will not work without it :)
> > 
> 
> The space-before-a-comma requirement was for gcc-2.95, iirc.
> 
> It got to the stage where I was the only person testing with 
> gcc-2.95 so I
> spent inordinate amounts of time adding spaces before 
> people's newly-added
> commas.  Fortunately we abamdoned that gcc version so the 
> space-before-a-comma
> requirement no longer exists.

Re: [PATCH resend] Make the dev_*() family of macros in device.hcomplete

[Greg KH]

On Fri, Nov 02, 2007 at 04:59:48AM -0700, Medve Emilian-EMMEDVE1 wrote:
> Hello Andrew,
> 
> 
> Thanks for the detailed background.
> 
> By now I also submitted a patch with extra spaces. Which one would be a
> better candidate for applying in a maintainer's tree (which tree?)?
> Could you please pick up one of the versions? Should I resubmit the
> first version without extra spaces?

The second patch looks good to me, it's "safer" and I'll add it to my
queue.

thanks,

greg k-h

Re: [PATCH resend] Make the dev_*() family of macros in device.hcomplete

[Greg KH]

On Thu, Nov 01, 2007 at 04:57:00PM -0700, Andrew Morton wrote:
> On Tue, 30 Oct 2007 08:40:08 -0700
> Greg KH <gregkh@suse.de> wrote:
> 
> > On Tue, Oct 30, 2007 at 05:11:24AM -0700, Medve Emilian-EMMEDVE1 wrote:
> > > Hi Greg K-H,
> > > 
> > > 
> > > > > +#define dev_info(dev, format, arg...)		\
> > > > > +	dev_printk(KERN_INFO, dev, format, ## arg)
> > > > > +
> > > > >  #ifdef DEBUG
> > > > >  #define dev_dbg(dev, format, arg...)		\
> > > > > -	dev_printk(KERN_DEBUG , dev , format , ## arg)
> > > > > +	dev_printk(KERN_DEBUG, dev, format, ## arg)
> > > > 
> > > > Those extra spaces are there for a good reason, older versions of gcc
> > > > are broken without it.  So please, put them all back...
> > > 
> > > You mean I should add spaces before commas only where they were
> > > initially or to all new code and/or macros?
> > 
> > Put it back where it was, and do the same for all other macros.
> > 
> > > I've observed other kernel code and more often there are no spaces
> > > before commas. I'm asking because the CodingStyle document is not very
> > > explicit about this rule.
> > 
> > This is a gcc rule, for variable length macros, not a CodingStyle
> > guideline.  It just will not work without it :)
> > 
> 
> The space-before-a-comma requirement was for gcc-2.95, iirc.
> 
> It got to the stage where I was the only person testing with gcc-2.95 so I
> spent inordinate amounts of time adding spaces before people's newly-added
> commas.  Fortunately we abamdoned that gcc version so the space-before-a-comma
> requirement no longer exists.

Ah, ok, that's good to remember in the future, thanks.

greg k-h