[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150225001534.GB15633@lunn.ch>
Date: Wed, 25 Feb 2015 01:15:34 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Florian Fainelli <f.fainelli@...il.com>
Cc: Rafa?? Mi??ecki <zajec5@...il.com>,
Andy Gospodarek <gospo@...ulusnetworks.com>,
"David S. Miller" <davem@...emloft.net>,
Network Development <netdev@...r.kernel.org>,
Jonas Gorski <jogo@...nwrt.org>,
Hauke Mehrtens <hauke@...ke-m.de>,
Felix Fietkau <nbd@...nwrt.org>, Jiri Pirko <jiri@...nulli.us>
Subject: Re: [PATCH] net: phy: b53: switchdev driver for Broadcom BCM53xx
switches
On Tue, Feb 24, 2015 at 02:55:58PM -0800, Florian Fainelli wrote:
> On 24/02/15 14:50, Rafa?? Mi??ecki wrote:
> > On 24 February 2015 at 23:30, Andy Gospodarek <gospo@...ulusnetworks.com> wrote:
> >> On Tue, Feb 24, 2015 at 06:42:07PM +0100, Rafa?? Mi??ecki wrote:
> >>> BCM53xx is series of Broadcom Ethernet switches that can be found in
> >>> various (mostly home) routers.
> >>> They are quite simple switches with mainly just support for:
> >>> 1) Tagging incoming packets (PVID)
> >>> 2) Untagging outgoing packets
> >>> 3) Forwarding all packets across a single VLAN
> >>>
> >>> This driver is split into common code (module) and bus specific code.
> >>> Right now only PHY (MDIO) support is included, other could follow after
> >>> accepting this driver. It was successfully tested on BCM4706 SoC with
> >>> BCM53125.
> >>>
> >>> You could notice it's yet another try of submitting b53 driver. This
> >>> time it was modified to use recently introduced switchdev API which
> >>> hopefully make it possible to accept it mainline.
> >>
> >> I must confess I do not know the entire history of this driver, but I
> >> have a few comments....
> >
> > I guess previous discussions were mostly focusing on API (like
> > swconfig), so a good review of b53 is still highly wanted!
> >
> >
> >>> diff --git a/drivers/net/phy/b53/b53_common.c b/drivers/net/phy/b53/b53_common.c
> >>> new file mode 100644
> >>> index 0000000..fce6b71
> >>> --- /dev/null
> >>> +++ b/drivers/net/phy/b53/b53_common.c
> >> [...]
> >>> +
> >>> +/*****************
> >>> + * Net device ops
> >>> + *****************/
> >>> +
> >>> +static netdev_tx_t b53_port_xmit(struct sk_buff *skb, struct net_device *dev)
> >>> +{
> >>> + dev_kfree_skb(skb);
> >>> + dev->stats.tx_dropped++;
> >>> +
> >>> + return NETDEV_TX_OK;
> >>> +}
> >>> +
> >>> (...)
> >>> +
> >>> +static const struct net_device_ops b53_port_netdev_ops = {
> >>> + .ndo_start_xmit = b53_port_xmit,
> >>> + .ndo_vlan_rx_add_vid = b53_port_vlan_rx_add_vid,
> >>> + .ndo_vlan_rx_kill_vid = b53_port_vlan_rx_kill_vid,
> >>> + .ndo_switch_parent_id_get = b53_port_switch_parent_id_get,
> >>> + .ndo_bridge_setlink = b53_port_bridge_setlink,
> >>> +};
> >>
> >> I see there is an xmit function, but no napi registration at all.
> >>
> >> Is this simply because the SoC ethernet dev is doing all the frame rx?
> >> Is is able to demux the frames correctly, so it is clear which swXpY
> >> received the frame? That will be necessary to ensure that standard
> >> applications that want to open sockets directly on the needed interfaces
> >> can still function properly.
> >
> > If you take a look at b53_port_xmit you'll start understanding why
> > there is no NAPI ;)
> >
> > As said in the commit message, these switches are really simple
> > devices. We can't actually send packets to the particular ports
> > (unless something has changed in the more recent hardware).
>
> These switches all support Broadcom tags, so you could use your host CPU
> Ethernet MAC to send/receive packets to/from specific ports of the
> switch, and then this is just like DSA, but everything that you say
> below is true.
If this hardware does support the concept of tags compatible to the
existing Broadcom Starfighter 2, should we not do that? Is there a
good reason these chips should use a difference abstraction than
Starfighter 2 and the Marvell devices?
Andrew
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists