From: Andrew Lunn <andrew@lunn.ch>
To: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Cc: netdev <netdev@vger.kernel.org>,
Florian Fainelli <f.fainelli@gmail.com>,
John Crispin <john@phrozen.org>,
Bryan.Whitehead@microchip.com
Subject: Re: [RFC PATCH 11/16] net: dsa: Refactor selection of tag ops into a function
Date: Fri, 27 May 2016 22:11:43 +0200 [thread overview]
Message-ID: <20160527201143.GB23212@lunn.ch> (raw)
In-Reply-To: <878tyvz42q.fsf@ketchup.mtl.sfl>
On Fri, May 27, 2016 at 03:35:57PM -0400, Vivien Didelot wrote:
> Hi Andrew,
>
> Andrew Lunn <andrew@lunn.ch> writes:
>
> > @@ -26,6 +26,7 @@ enum dsa_tag_protocol {
> > DSA_TAG_PROTO_TRAILER,
> > DSA_TAG_PROTO_EDSA,
> > DSA_TAG_PROTO_BRCM,
> > + _DSA_TAG_LAST,
> > };
>
> I would avoid _ prefixed functions or symbols, we've seen in mv88e6xxx
> that it doesn't make the code really readable.
The point is to make is special, so that people look at it, wonder why
it is special, and don't make the stupid mistakes of adding a new
protocol after it.
However, if you don't like the _, we can just add a comment
/* MUST BE LAST */
> I don't see the need to add a dsa_device_ops array. I'd keep the switch
> case on tag_protocol within this new dsa_resolve_tag_protocol function,
> to make it more readable (no value checking necessary).
I don't see one way being better than the other. I see it as a
personal preference. If you don't like it, i won't NACK a patch
submitted after it has been accepted which changes it to your personal
preference.
Andrew
next prev parent reply other threads:[~2016-05-27 20:11 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-27 1:20 [RFC PATCH 00/16] New DSA bind, switches as devices Andrew Lunn
2016-05-27 1:20 ` [RFC PATCH 01/16] dsa: slave: chip data is optional, don't dereference NULL Andrew Lunn
2016-05-27 18:45 ` Vivien Didelot
2016-05-27 21:24 ` Florian Fainelli
2016-05-27 1:20 ` [RFC PATCH 02/16] net: dsa: mv88e6xxx: fix circular lock in PPU work Andrew Lunn
2016-05-27 1:20 ` [RFC PATCH 03/16] dsa: slave: Remove MDIO address from switch MDIO bus name Andrew Lunn
2016-05-27 1:20 ` [RFC PATCH 04/16] dsa: tag_{e}dsa.c: Remove dependency on platform data Andrew Lunn
2016-05-27 1:20 ` [RFC PATCH 05/16] dsa: Add a ports structure and use it in the switch structure Andrew Lunn
2016-05-27 1:20 ` [RFC PATCH 06/16] dsa: Move port device node into port structure Andrew Lunn
2016-05-27 1:20 ` [RFC PATCH 07/16] dsa: Remove dynamic allocate of routing table Andrew Lunn
2016-05-27 18:54 ` Vivien Didelot
2016-05-27 1:20 ` [RFC PATCH 08/16] dsa: Copy the routing table into the switch structure Andrew Lunn
2016-05-27 1:20 ` [RFC PATCH 09/16] dsa: dsa: Split up creating/destroying of DSA and CPU ports Andrew Lunn
2016-05-27 14:33 ` Vivien Didelot
2016-05-27 15:07 ` Andrew Lunn
2016-05-27 16:36 ` Vivien Didelot
2016-05-27 19:25 ` Vivien Didelot
2016-05-27 20:01 ` Andrew Lunn
2016-05-27 20:29 ` Andrew Lunn
2016-05-27 1:20 ` [RFC PATCH 10/16] net: dsa: mv88e6xxx: Only support EDSA tagging Andrew Lunn
2016-05-27 1:20 ` [RFC PATCH 11/16] net: dsa: Refactor selection of tag ops into a function Andrew Lunn
2016-05-27 19:35 ` Vivien Didelot
2016-05-27 20:11 ` Andrew Lunn [this message]
2016-05-27 1:20 ` [RFC PATCH 12/16] dsa: Make mdio bus optional Andrew Lunn
2016-05-27 14:55 ` Vivien Didelot
2016-05-27 15:18 ` Andrew Lunn
2016-05-27 16:38 ` Vivien Didelot
2016-05-27 1:20 ` [RFC PATCH 13/16] net: dsa: mv88e6xxx: Refactor MDIO so driver registers mdio bus Andrew Lunn
2016-05-27 19:45 ` Vivien Didelot
2016-05-27 1:20 ` [RFC PATCH 14/16] net: dsa: Add new binding implementation Andrew Lunn
2016-05-27 20:39 ` Vivien Didelot
2016-05-27 20:57 ` Andrew Lunn
2016-05-27 21:29 ` Florian Fainelli
2016-05-28 8:23 ` Richard Cochran
2016-05-27 1:20 ` [RFC PATCH 15/16] arm: dt: vf610-zii-devel-b: Make use of new DSA binding Andrew Lunn
2016-05-27 1:20 ` [RFC PATCH 16/16] dsa: Document new binding Andrew Lunn
2016-05-27 18:30 ` [RFC PATCH 00/16] New DSA bind, switches as devices Vivien Didelot
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=20160527201143.GB23212@lunn.ch \
--to=andrew@lunn.ch \
--cc=Bryan.Whitehead@microchip.com \
--cc=f.fainelli@gmail.com \
--cc=john@phrozen.org \
--cc=netdev@vger.kernel.org \
--cc=vivien.didelot@savoirfairelinux.com \
/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.