Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1477873 > unrolled thread

[PATCH 0/2] lan78xx: Remove trailing underscores from macros

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] lan78xx: Remove trailing underscores from macros Joe Perches <joe@perches.com> - 2016-09-07 01:10 +0200
    RE: [PATCH 0/2] lan78xx: Remove trailing underscores from macros <Woojung.Huh@microchip.com> - 2016-09-07 01:20 +0200
      Re: [PATCH 0/2] lan78xx: Remove trailing underscores from macros Joe Perches <joe@perches.com> - 2016-09-07 03:30 +0200
        RE: [PATCH 0/2] lan78xx: Remove trailing underscores from macros <Ronnie.Kunin@microchip.com> - 2016-09-07 04:10 +0200
          Re: [PATCH 0/2] lan78xx: Remove trailing underscores from macros Joe Perches <joe@perches.com> - 2016-09-07 04:20 +0200

#1477873 — [PATCH 0/2] lan78xx: Remove trailing underscores from macros

Joe Perches (2):
  lan78xx: Remove locally defined trailing underscores from defines and uses
  microchipphy.h and uses: Remove trailing underscores from defines and uses

 drivers/net/phy/microchip.c  |    4 +-
 drivers/net/usb/lan78xx.c    |  368 +++++++--------
 drivers/net/usb/lan78xx.h    | 1068 +++++++++++++++++++++---------------------
 include/linux/microchipphy.h |   72 +--
 4 files changed, 756 insertions(+), 756 deletions(-)

-- 
2.10.0.rc2.1.g053435c

[toc] | [next] | [standalone]

#1477878

> Joe Perches (2):
>   lan78xx: Remove locally defined trailing underscores from defines and uses
>   microchipphy.h and uses: Remove trailing underscores from defines and
> uses
> 
>  drivers/net/phy/microchip.c  |    4 +-
>  drivers/net/usb/lan78xx.c    |  368 +++++++--------
>  drivers/net/usb/lan78xx.h    | 1068 +++++++++++++++++++++-----------------
> ----
>  include/linux/microchipphy.h |   72 +--
>  4 files changed, 756 insertions(+), 756 deletions(-)

Because there is no specific rule how to name defines, I'm not sure it is worth to change 1000+ lines.
It may be better to set guideline for new submissions.

Welcome any comments.

[toc] | [prev] | [next] | [standalone]

#1477914

On Tue, 2016-09-06 at 23:19 +0000, Woojung.Huh@microchip.com wrote:
> > Joe Perches (2):
> >   lan78xx: Remove locally defined trailing underscores from defines and uses
> >   microchipphy.h and uses: Remove trailing underscores from defines and
> > uses
> > 
> >  drivers/net/phy/microchip.c  |    4 +-
> >  drivers/net/usb/lan78xx.c    |  368 +++++++--------
> >  drivers/net/usb/lan78xx.h    | 1068 +++++++++++++++++++++-----------------
> > ----
> >  include/linux/microchipphy.h |   72 +--
> >  4 files changed, 756 insertions(+), 756 deletions(-)
> 
> 
> Because there is no specific rule how to name defines, I'm not sure it is worth to change 1000+ lines.
> It may be better to set guideline for new submissions.
> 
> Welcome any comments.

Generally, more conforming to norms is better.
These FOO_ uses are non-conforming.

Is there anything other than a one-time cost
to apply these?  Is the same code used for
other platforms?

[toc] | [prev] | [next] | [standalone]

#1477929

Microchip's internal convention is for register (offset) definitions to be capitalized (i.e.: MY_REGISTER). Our convention for bits (position) definitions within a register is to carry as a prefix the name of the register and suffix it with the bit name and adding a trailing underscore (i.e. MY_REGISTER_MY_BIT_). The trailing underscore is what easily lets us distinguish a bit from a register definition when reading code. We have been using this convention for many years and has worked very well for us across all projects (by now hundreds).

>Is there anything other than a one-time cost
>to apply these?  Is the same code used for
>other platforms?
Yes, a single header file with the definition of registers and bits is shared (either as a standalone file or with its contents pasted into a native environment "carrier" header file) across all drivers (and other non driver software projects as well) for the same device. So a change like this indeed has a high cost for Microchip and we'd rather not do this unless it is an absolutely mandated requirement. 

Thanks,
Ronnie
________________________________________
From: Joe Perches [joe@perches.com]
Sent: Tuesday, September 06, 2016 9:18 PM
To: Woojung Huh - C21699; netdev@vger.kernel.org; linux-usb@vger.kernel.org
Cc: f.fainelli@gmail.com; UNGLinuxDriver; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/2] lan78xx: Remove trailing underscores from macros

On Tue, 2016-09-06 at 23:19 +0000, Woojung.Huh@microchip.com wrote:
> > Joe Perches (2):
> >   lan78xx: Remove locally defined trailing underscores from defines and uses
> >   microchipphy.h and uses: Remove trailing underscores from defines and
> > uses
> >
> >  drivers/net/phy/microchip.c  |    4 +-
> >  drivers/net/usb/lan78xx.c    |  368 +++++++--------
> >  drivers/net/usb/lan78xx.h    | 1068 +++++++++++++++++++++-----------------
> > ----
> >  include/linux/microchipphy.h |   72 +--
> >  4 files changed, 756 insertions(+), 756 deletions(-)
>
>
> Because there is no specific rule how to name defines, I'm not sure it is worth to change 1000+ lines.
> It may be better to set guideline for new submissions.
>
> Welcome any comments.

Generally, more conforming to norms is better.
These FOO_ uses are non-conforming.

Is there anything other than a one-time cost
to apply these?  Is the same code used for
other platforms?

[toc] | [prev] | [next] | [standalone]

#1477933

On Wed, 2016-09-07 at 02:00 +0000, Ronnie.Kunin@microchip.com wrote:
> Microchip's internal convention is for register (offset) definitions
> to be capitalized (i.e.: MY_REGISTER). Our convention for bits
> (position) definitions within a register is to carry as a prefix the
> name of the register and suffix it with the bit name and adding a
> trailing underscore (i.e. MY_REGISTER_MY_BIT_). The trailing
> underscore is what easily lets us distinguish a bit from a register
> definition when reading code. We have been using this convention for
> many years and has worked very well for us across all projects (by now
> hundreds).

I think it's kind of an ugly convention, but <shrug> no
skin off my nose really.

> > Is there anything other than a one-time cost
> > to apply these?  Is the same code used for
> > other platforms?
> 
> Yes, a single header file with the definition of registers and bits is
> shared (either as a standalone file or with its contents pasted into a
> native environment "carrier" header file) across all drivers (and
> other non driver software projects as well) for the same device. So a
> change like this indeed has a high cost for Microchip and we'd rather
> not do this unless it is an absolutely mandated requirement. 

No worries, if you don't like it, don't apply it.

Send a NAK too so David Miller doesn't apply it either.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web