All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Jiri Pirko <jiri@resnulli.us>
Cc: netdev@vger.kernel.org, davem@davemloft.net,
	jakub.kicinski@netronome.com, mlxsw@mellanox.com
Subject: Re: [patch net-next] devlink: add format requirement for devlink param names
Date: Fri, 18 Oct 2019 22:27:48 +0200	[thread overview]
Message-ID: <20191018202748.GL4780@lunn.ch> (raw)
In-Reply-To: <20191018200822.GI2185@nanopsycho>

On Fri, Oct 18, 2019 at 10:08:22PM +0200, Jiri Pirko wrote:
> Fri, Oct 18, 2019 at 07:43:04PM CEST, andrew@lunn.ch wrote:
> >On Fri, Oct 18, 2019 at 06:07:26PM +0200, Jiri Pirko wrote:
> >> From: Jiri Pirko <jiri@mellanox.com>
> >> 
> >> Currently, the name format is not required by the code, however it is
> >> required during patch review. All params added until now are in-lined
> >> with the following format:
> >> 1) lowercase characters, digits and underscored are allowed
> >> 2) underscore is neither at the beginning nor at the end and
> >>    there is no more than one in a row.
> >> 
> >> Add checker to the code to require this format from drivers and warn if
> >> they don't follow.
> >
> >Hi Jiri
> >
> >Could you add a reference to where these requirements are derived
> >from. What can go wrong if they are ignored? I assume it is something
> 
> Well, no reference. All existing params, both generic and driver
> specific are following this format. I just wanted to make that required
> so all params are looking similar.
> 
> 
> >to do with sysfs?
> 
> No, why would you think so?

I was not expecting it to be totally arbitrary. I thought you would
have a real technical reason. Spaces often cause problems, as well as
/ etc.

I've had problems with hwmon device names breaking assumptions in the
user space code, etc. I was expecting something like this.

I don't really like the all lower case restriction. It makes it hard
to be consistent. All Marvell Docs refer to the Address Translation
Unit as ATU. I don't think there is any reference to atu. I would
prefer to be consistent with the documentation and use ATU. But that
is against your arbitrary rules.

       Andrew

  reply	other threads:[~2019-10-18 20:28 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-18 16:07 [patch net-next] devlink: add format requirement for devlink param names Jiri Pirko
2019-10-18 16:25 ` Jakub Kicinski
2019-10-18 16:58   ` Jiri Pirko
2019-10-18 16:33 ` Stephen Hemminger
2019-10-18 16:58   ` Jiri Pirko
2019-10-18 16:35 ` Stephen Hemminger
2019-10-18 16:58   ` Jiri Pirko
2019-10-18 17:43 ` Andrew Lunn
2019-10-18 20:08   ` Jiri Pirko
2019-10-18 20:27     ` Andrew Lunn [this message]
2019-10-18 21:34       ` Jakub Kicinski
2019-10-19  5:39       ` Jiri Pirko

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=20191018202748.GL4780@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=jakub.kicinski@netronome.com \
    --cc=jiri@resnulli.us \
    --cc=mlxsw@mellanox.com \
    --cc=netdev@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.