[RFC PATCH 0/3] Enable connecting DSA-based switch to the USB RMII interface.

[RFC PATCH 0/3] Enable connecting DSA-based switch to the USB RMII interface.

[Jan Kaisrlik]

From: Jan Kaisrlik <ja.kaisrlik@gmail.com>

This RFC patch enables connecting a DSA-based switch to the USB RMII interface.

I use switch Marvell 88E6065 as the DSA-based switch
connected to USB RMII interface provides by Asix AX88772b.

I temporarily modified the mv88e6060 driver to work with 88E6065.

Jan Kaisrlik (3):
  net/dsa: Refactor dsa_probe()
  net/dsa: Allow probing dsa from usbnet
  driver/net/usb: Add support for DSA to ax88772b

 drivers/net/usb/asix.h         |   7 ++
 drivers/net/usb/asix_devices.c | 258 ++++++++++++++++++++++++++++++++++++++++-
 include/net/dsa.h              |   3 +
 net/dsa/dsa.c                  | 197 ++++++++++++++++++++++++++-----
 4 files changed, 429 insertions(+), 36 deletions(-)

-- 
2.1.3

Re: [RFC PATCH 0/3] Enable connecting DSA-based switch to the USB RMII interface.

[Andrew Lunn]

Hi Jan

Interesting work, but i think the architecture is wrong.

DSA needs an Ethernet device, an MDIO bus, and information about ports
on the switch. The MDIO bus and the Ethernet need no knowledge of
DSA. So putting your DSA configuration code in the MDIO driver is
wrong.

The problem you have is where the put the configuration data. There
are the currently two choices, using a platform driver, which you can
find some examples of in arch/arm/mach-orion5x, or via device tree. Or
you need a new method.

Part of your problem is hotplug, since you have a USB device, and no
stable names for the ethernet device nor the MDIO device. Your
hardware is not fixed, you could hang any switch off the USB
device. So it does sound like you need a user space API.

I would however say that sysfs is the wrong API. The linux network
stack uses netlink for most configuration activities. So i would
suggest adding a netlink binding to DSA, and place the code in
net/dsa/, not within an MDIO driver.

Device tree overlays might be a solution, if you can dynamically load
a blob as part of a USB hotplug event. What makes it easier is that
both the Ethernet device and MDIO bus are on the same USB device, so
all your phandles are within the blob.

What is your long term goal? Is this just a development tool? Are you
thinking of making a product which integrates both the switch and the
USB ethernet onto a USB dongle? This could also change the
architecture, since it makes the configuration more fixed.

    Andrew

Re: [RFC PATCH 0/3] Enable connecting DSA-based switch to the USB RMII interface.

[Florian Fainelli]

On 21/04/15 05:47, Andrew Lunn wrote:
> Hi Jan
> 
> Interesting work, but i think the architecture is wrong.
> 
> DSA needs an Ethernet device, an MDIO bus, and information about ports
> on the switch. 

That requirement is completely artificial as it is today, and just comes
from arbitrary limitations imposed in the initial DSA design, something
that I am still trying to get away from.

> The MDIO bus and the Ethernet need no knowledge of
> DSA. So putting your DSA configuration code in the MDIO driver is
> wrong.

I agree with that.

> 
> The problem you have is where the put the configuration data. There
> are the currently two choices, using a platform driver, which you can
> find some examples of in arch/arm/mach-orion5x, or via device tree. Or
> you need a new method.
> 
> Part of your problem is hotplug, since you have a USB device, and no
> stable names for the ethernet device nor the MDIO device. Your
> hardware is not fixed, you could hang any switch off the USB
> device. So it does sound like you need a user space API.
> 
> I would however say that sysfs is the wrong API. The linux network
> stack uses netlink for most configuration activities. So i would
> suggest adding a netlink binding to DSA, and place the code in
> net/dsa/, not within an MDIO driver.

I suppose we could do that, but that sounds like a pretty radical change
in how DSA is currently configured (that is statically at boot time),
part in order to allow booting from DSA-enabled network devices (e.g:
nfsroot).

> 
> Device tree overlays might be a solution, if you can dynamically load
> a blob as part of a USB hotplug event. What makes it easier is that
> both the Ethernet device and MDIO bus are on the same USB device, so
> all your phandles are within the blob.
> 
> What is your long term goal? Is this just a development tool? Are you
> thinking of making a product which integrates both the switch and the
> USB ethernet onto a USB dongle? This could also change the
> architecture, since it makes the configuration more fixed.

My goal in reworking this weird DSA device/driver model is that you
could just register your switch devices as an enhanced
phy_driver/spi_driver/pci_driver etc..., such that libphy-ready drivers
could just take advantage of that when they scan/detect their MDIO buses
and find a switch. We are not quite there yet, but some help could be
welcome, here are the WIP patches (tested with platform_driver only so far):

https://github.com/ffainelli/linux/tree/dsa-model-b53
-- 
Florian

Re: [RFC PATCH 0/3] Enable connecting DSA-based switch to the USB RMII interface.

[Andrew Lunn]

> My goal in reworking this weird DSA device/driver model is that you
> could just register your switch devices as an enhanced
> phy_driver/spi_driver/pci_driver etc..., such that libphy-ready drivers
> could just take advantage of that when they scan/detect their MDIO buses
> and find a switch. We are not quite there yet, but some help could be
> welcome, here are the WIP patches (tested with platform_driver only so far):

We are hijacking another thread, but...

I don't understand you here. Who calls dsa_switch_register()?

I know of a board coming soon which has three switch chips on
it. There is one MDIO device in the Soc, but there is an external MDIO
multiplexor controlled via gpio lines, such that each switch has its
own MDIO bus. The DT binding does not support this currently, but the
underlying data structures do.

How do you envisage dsa_switch_register() to work in such a setup?

Thanks
    Andrew

Re: [RFC PATCH 0/3] Enable connecting DSA-based switch to the USB RMII interface.

[Florian Fainelli]

On 21/04/15 10:30, Andrew Lunn wrote:
>> My goal in reworking this weird DSA device/driver model is that you
>> could just register your switch devices as an enhanced
>> phy_driver/spi_driver/pci_driver etc..., such that libphy-ready drivers
>> could just take advantage of that when they scan/detect their MDIO buses
>> and find a switch. We are not quite there yet, but some help could be
>> welcome, here are the WIP patches (tested with platform_driver only so far):
> 
> We are hijacking another thread, but...
> 
> I don't understand you here. Who calls dsa_switch_register()?

Any driver which is backing the underlying device, if this is a PCI(e)
switch, a pci_driver's probe function gets called, and then registers
with DSA a switch device, very much like this:

https://github.com/ffainelli/linux/commit/f94efc3d7b489955351c01efeafcc89939df388e

> 
> I know of a board coming soon which has three switch chips on
> it. There is one MDIO device in the Soc, but there is an external MDIO
> multiplexor controlled via gpio lines, such that each switch has its
> own MDIO bus. The DT binding does not support this currently, but the
> underlying data structures do.
> 
> How do you envisage dsa_switch_register() to work in such a setup?

I would envision something where we can scan all of these switches
individually using their respective device drivers, with the help of
Device Tree or platform_data, figure out which position in a
dsa_switch_tree they should have, and make sure that we create a
dsa_switch_tree which reflects that, taking probe ordering into account.
All of these switches would be phy_driver instances, like this:
https://github.com/ffainelli/linux/commit/4a5c6b17de36377f6a71423b91f80bc1c7fee7be

We can keep discussing the details in a separate thread, I think that
would be useful.
-- 
Florian

Re: [RFC PATCH 0/3] Enable connecting DSA-based switch to the USB RMII interface.

[Andrew Lunn]

> > I would however say that sysfs is the wrong API. The linux network
> > stack uses netlink for most configuration activities. So i would
> > suggest adding a netlink binding to DSA, and place the code in
> > net/dsa/, not within an MDIO driver.
> 
> I suppose we could do that, but that sounds like a pretty radical change
> in how DSA is currently configured (that is statically at boot time),
> part in order to allow booting from DSA-enabled network devices (e.g:
> nfsroot).

We would keep both DT and platform device. But statically at boot does
not work for a USB hotpluggable switch!

    Andrew

Re: [RFC PATCH 0/3] Enable connecting DSA-based switch to the USB RMII interface.

[Florian Fainelli]

On 21/04/15 10:39, Andrew Lunn wrote:
>>> I would however say that sysfs is the wrong API. The linux network
>>> stack uses netlink for most configuration activities. So i would
>>> suggest adding a netlink binding to DSA, and place the code in
>>> net/dsa/, not within an MDIO driver.
>>
>> I suppose we could do that, but that sounds like a pretty radical change
>> in how DSA is currently configured (that is statically at boot time),
>> part in order to allow booting from DSA-enabled network devices (e.g:
>> nfsroot).
> 
> We would keep both DT and platform device. But statically at boot does
> not work for a USB hotpluggable switch!

Is the switch really hotpluggable, or it is the USB-Ethernet adapter
connecting to it? If the former, then I agree, if not, I would imagine
that there is nothing that prevents creating the switch device first,
and wait for its "master_netdev" to show up later before it starts doing
anything useful?
-- 
Florian

Re: [RFC PATCH 0/3] Enable connecting DSA-based switch to the USB RMII interface.

[Jan Kaisrlik]

2015-04-21 17:51 GMT+00:00 Florian Fainelli <f.fainelli@gmail.com>:
> On 21/04/15 10:39, Andrew Lunn wrote:
>>>> I would however say that sysfs is the wrong API. The linux network
>>>> stack uses netlink for most configuration activities. So i would
>>>> suggest adding a netlink binding to DSA, and place the code in
>>>> net/dsa/, not within an MDIO driver.
>>>
>>> I suppose we could do that, but that sounds like a pretty radical change
>>> in how DSA is currently configured (that is statically at boot time),
>>> part in order to allow booting from DSA-enabled network devices (e.g:
>>> nfsroot).
>>
>> We would keep both DT and platform device. But statically at boot does
>> not work for a USB hotpluggable switch!
>
> Is the switch really hotpluggable, or it is the USB-Ethernet adapter
> connecting to it? If the former, then I agree, if not, I would imagine
> that there is nothing that prevents creating the switch device first,
> and wait for its "master_netdev" to show up later before it starts doing
> anything useful?
> --
> Florian

Thank you for your quick and helpful answers.

The goal of this project is to extend embeded modules without integreted
MII to add possibility to connect ethernet switch.

Current version of switch is hotplugable but this feature is not required.
In my humble opinion, hotplugable switch seems to be pretty interesting idea.

Jan

Re: [RFC PATCH 0/3] Enable connecting DSA-based switch to the USB RMII interface.

[Andrew Lunn]

On Wed, Apr 22, 2015 at 04:14:33PM +0000, Jan Kaisrlik wrote:
> 2015-04-21 17:51 GMT+00:00 Florian Fainelli <f.fainelli@gmail.com>:
> > On 21/04/15 10:39, Andrew Lunn wrote:
> >>>> I would however say that sysfs is the wrong API. The linux network
> >>>> stack uses netlink for most configuration activities. So i would
> >>>> suggest adding a netlink binding to DSA, and place the code in
> >>>> net/dsa/, not within an MDIO driver.
> >>>
> >>> I suppose we could do that, but that sounds like a pretty radical change
> >>> in how DSA is currently configured (that is statically at boot time),
> >>> part in order to allow booting from DSA-enabled network devices (e.g:
> >>> nfsroot).
> >>
> >> We would keep both DT and platform device. But statically at boot does
> >> not work for a USB hotpluggable switch!
> >
> > Is the switch really hotpluggable, or it is the USB-Ethernet adapter
> > connecting to it? If the former, then I agree, if not, I would imagine
> > that there is nothing that prevents creating the switch device first,
> > and wait for its "master_netdev" to show up later before it starts doing
> > anything useful?
> > --
> > Florian
> 
> Thank you for your quick and helpful answers.
> 
> The goal of this project is to extend embeded modules without integreted
> MII to add possibility to connect ethernet switch.
> 
> Current version of switch is hotplugable but this feature is not required.
> In my humble opinion, hotplugable switch seems to be pretty interesting idea.

Hi Jan

Thanks for the extra information.

I don't know this USB device. Can you change the product:vendor ID?
Could you imply from the USB product:vendor ID what the DSA
configuration is? So have a wrapper driver around the asix driver
which installs a dsa platform device and then instantiates the asix
driver? That eliminates all your DSA changes, no need for a user space
API, etc.

     Andrew

[RFC PATCH 1/3] net/dsa: Refactor dsa_probe()

[Jan Kaisrlik]

From: Jan Kaisrlik <ja.kaisrlik@gmail.com>

This patch refactors dsa_probe in order to simplify code in the patch 2/3.

---
 net/dsa/dsa.c | 82 ++++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 47 insertions(+), 35 deletions(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 3731714..e2c0703 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -699,12 +699,56 @@ static inline void dsa_of_remove(struct platform_device *pdev)
 }
 #endif
 
-static int dsa_probe(struct platform_device *pdev)
+static int dsa_probe_common(struct dsa_switch_tree *dst, struct device *parent)
 {
+	struct dsa_platform_data *pd = dst->pd;
+	struct net_device *dev = dst->master_netdev;
+	int i;
+
+	dst->cpu_switch = -1;
+	dst->cpu_port = -1;
+
+	for (i = 0; i < pd->nr_chips; i++) {
+		struct dsa_switch *ds;
+
+		ds = dsa_switch_setup(dst, i, parent, pd->chip[i].host_dev);
+		if (IS_ERR(ds)) {
+			netdev_err(dev, "[%d]: couldn't create dsa switch instance (error %ld)\n",
+				   i, PTR_ERR(ds));
+			continue;
+		}
+
+		dst->ds[i] = ds;
+		if (ds->drv->poll_link != NULL)
+			dst->link_poll_needed = 1;
+	}
+
+	/*
+	 * If we use a tagging format that doesn't have an ethertype
+	 * field, make sure that all packets from this point on get
+	 * sent to the tag format's receive function.
+	 */
+	wmb();
+	dev->dsa_ptr = (void *)dst;
+
+	if (dst->link_poll_needed) {
+		INIT_WORK(&dst->link_poll_work, dsa_link_poll_work);
+		init_timer(&dst->link_poll_timer);
+		dst->link_poll_timer.data = (unsigned long)dst;
+		dst->link_poll_timer.function = dsa_link_poll_timer;
+		dst->link_poll_timer.expires = round_jiffies(jiffies + HZ);
+		add_timer(&dst->link_poll_timer);
+	}
+
+	return 0;
+
+}
+
+static int dsa_probe(struct platform_device *pdev){
 	struct dsa_platform_data *pd = pdev->dev.platform_data;
 	struct net_device *dev;
 	struct dsa_switch_tree *dst;
-	int i, ret;
+	int ret;
 
 	pr_notice_once("Distributed Switch Architecture driver version %s\n",
 		       dsa_driver_version);
@@ -743,40 +787,8 @@ static int dsa_probe(struct platform_device *pdev)
 
 	dst->pd = pd;
 	dst->master_netdev = dev;
-	dst->cpu_switch = -1;
-	dst->cpu_port = -1;
-
-	for (i = 0; i < pd->nr_chips; i++) {
-		struct dsa_switch *ds;
-
-		ds = dsa_switch_setup(dst, i, &pdev->dev, pd->chip[i].host_dev);
-		if (IS_ERR(ds)) {
-			netdev_err(dev, "[%d]: couldn't create dsa switch instance (error %ld)\n",
-				   i, PTR_ERR(ds));
-			continue;
-		}
-
-		dst->ds[i] = ds;
-		if (ds->drv->poll_link != NULL)
-			dst->link_poll_needed = 1;
-	}
-
-	/*
-	 * If we use a tagging format that doesn't have an ethertype
-	 * field, make sure that all packets from this point on get
-	 * sent to the tag format's receive function.
-	 */
-	wmb();
-	dev->dsa_ptr = (void *)dst;
 
-	if (dst->link_poll_needed) {
-		INIT_WORK(&dst->link_poll_work, dsa_link_poll_work);
-		init_timer(&dst->link_poll_timer);
-		dst->link_poll_timer.data = (unsigned long)dst;
-		dst->link_poll_timer.function = dsa_link_poll_timer;
-		dst->link_poll_timer.expires = round_jiffies(jiffies + HZ);
-		add_timer(&dst->link_poll_timer);
-	}
+	dsa_probe_common(dst, &pdev->dev);
 
 	return 0;
 
-- 
2.1.3

Re: [RFC PATCH 1/3] net/dsa: Refactor dsa_probe()

[Florian Fainelli]

On 21/04/15 06:26, Jan Kaisrlik wrote:
> From: Jan Kaisrlik <ja.kaisrlik@gmail.com>
> 
> This patch refactors dsa_probe in order to simplify code in the patch 2/3.

It does not look like you are working on the latest net-next tree, that
part of the code has already been refactored to have separate helper
functions such as dsa_setup_dst(), dsa_switch_setup() and
dsa_switch_setup_one().
-- 
Florian

[RFC PATCH 2/3] net/dsa: Allow probing dsa from usbnet

[Jan Kaisrlik]

From: Jan Kaisrlik <ja.kaisrlik@gmail.com>

This patch adds a function which helps to connect net device
to DSA switch based on mii_bus and netdev.

The switch parameters of the switch are configured in fill_platform_data().
Currently, the configuration data is hardcoded in the code.
I don't know how to pass the configuration data from
user space.

It is not possible to determine the configuration data in plug-and-play
manner in mv88e6060 driver.

I have thought about two possibilities how to do that. First one is to
load data from the device tree, because loading from device tree is
already implemented in dsa_of_probe().

Second possibility is to send configuration of switch via sysfs.

In my opinion, the second one is better because I have already used
sysfs to bind USB to DSA in patch 3/3.

---
 include/net/dsa.h |   3 ++
 net/dsa/dsa.c     | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index ed3c34b..df7b748 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -290,4 +290,7 @@ static inline bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
 {
 	return dst->rcv != NULL;
 }
+
+int dsa_probe_mii(struct mii_bus *bus, struct net_device * dev);
+
 #endif
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index e2c0703..cb5d9c2 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -798,6 +798,127 @@ out:
 	return ret;
 }
 
+static int fill_platform_data(struct dsa_platform_data *pd,
+		struct mii_bus *bus, struct device * parent){
+	struct dsa_chip_data * cd;
+	int i;
+
+	static struct device_node dn = {
+		.name = "name",
+		.type = "type",
+		.phandle = 0,
+		.full_name = "fullname",
+		.fwnode =  {1},
+		.properties = NULL,
+		.deadprops = NULL,
+		.parent = NULL,
+		.child = NULL,
+		.sibling = NULL,
+		.kobj = {NULL},
+		._flags = 0,
+		.data = NULL
+	};
+	static struct device_node dnc = {
+		.name = "name",
+		.type = "type",
+		.phandle = 0,
+		.full_name = "fullname",
+		.fwnode =  {1},
+		.properties = NULL,
+		.deadprops = NULL,
+		.parent = &dn,
+		.child = NULL,
+		.sibling = NULL,
+		.kobj = {NULL},
+		._flags = 0,
+		.data = NULL
+	};
+	static char *port_names[12] = {"0", "1", "2", "3", "4",
+		"5", "6", "7", "8", "9", "10", "11"};
+
+	pd->nr_chips = 1;
+	pd->netdev = parent;
+
+	cd = kzalloc(sizeof(*cd), GFP_KERNEL);
+	if (cd == NULL)
+		return -ENOMEM;
+
+	pd->chip = cd;
+
+	cd->host_dev = parent;
+	cd->sw_addr = 0x10;
+	cd->eeprom_len = 256;
+	cd->of_node = &dn;
+	cd->rtable = 0;
+
+//	cd->of_node = kzalloc(sizeof(*cd->of_node), GFP_KERNEL);
+//	if(cd->of_node == NULL)
+//		goto free;
+
+	for (i = 0; i < DSA_MAX_PORTS; i++) {
+		cd->port_names[i] = port_names[i];
+		cd->port_dn[i] = &dnc;
+	}
+
+	return 0;
+
+//free:
+//	kfree(cd);
+//	return -ENOMEM;
+}
+
+int dsa_probe_mii(struct mii_bus *bus, struct net_device * dev)
+{
+	struct dsa_platform_data *pd;
+	struct dsa_switch_tree *dst;
+	int ret;
+
+	pr_notice_once("Distributed Switch Architecture driver version %s\n",
+		       dsa_driver_version);
+
+	if (dev == NULL || bus == NULL)
+		return -EINVAL;
+
+	pd = kzalloc(sizeof(*pd), GFP_KERNEL);
+	if (pd == NULL) {
+		ret = -ENOMEM;
+		goto freep;
+	}
+
+	ret = fill_platform_data(pd, bus, bus->parent); //TODO fix it!
+	if (ret)
+		goto freep;
+
+	if (dev->dsa_ptr != NULL) {
+		dev_put(dev);
+		ret = -EEXIST;
+		goto freep;
+	}
+
+	dst = kzalloc(sizeof(*dst), GFP_KERNEL);
+	if (dst == NULL) {
+		dev_put(dev);
+		ret = -ENOMEM;
+		goto freed;
+	}
+
+	dev_set_drvdata(bus->parent, dst);
+
+	dst->pd = pd;
+	dst->master_netdev = dev;
+
+	dsa_probe_common(dst, bus->parent);
+
+	return 0;
+
+freed:
+	kfree(dst);
+freep:
+	kfree(pd);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dsa_probe_mii);
+
 static int dsa_remove(struct platform_device *pdev)
 {
 	struct dsa_switch_tree *dst = platform_get_drvdata(pdev);
-- 
2.1.3

Re: [RFC PATCH 2/3] net/dsa: Allow probing dsa from usbnet

[rajeev kumar]

On Tue, Apr 21, 2015 at 6:56 PM, Jan Kaisrlik <kaisrja1@fel.cvut.cz> wrote:
> From: Jan Kaisrlik <ja.kaisrlik@gmail.com>
>
> This patch adds a function which helps to connect net device
> to DSA switch based on mii_bus and netdev.
>
> The switch parameters of the switch are configured in fill_platform_data().
> Currently, the configuration data is hardcoded in the code.
> I don't know how to pass the configuration data from
> user space.
>
> It is not possible to determine the configuration data in plug-and-play
> manner in mv88e6060 driver.
>
> I have thought about two possibilities how to do that. First one is to
> load data from the device tree, because loading from device tree is
> already implemented in dsa_of_probe().
>
> Second possibility is to send configuration of switch via sysfs.
>
> In my opinion, the second one is better because I have already used
> sysfs to bind USB to DSA in patch 3/3.
>
> ---
>  include/net/dsa.h |   3 ++
>  net/dsa/dsa.c     | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 124 insertions(+)
>
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index ed3c34b..df7b748 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -290,4 +290,7 @@ static inline bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
>  {
>         return dst->rcv != NULL;
>  }
> +
> +int dsa_probe_mii(struct mii_bus *bus, struct net_device * dev);
> +
>  #endif
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index e2c0703..cb5d9c2 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -798,6 +798,127 @@ out:
>         return ret;
>  }
>
> +static int fill_platform_data(struct dsa_platform_data *pd,
> +               struct mii_bus *bus, struct device * parent){
> +       struct dsa_chip_data * cd;
> +       int i;
> +
> +       static struct device_node dn = {
> +               .name = "name",
> +               .type = "type",
> +               .phandle = 0,
> +               .full_name = "fullname",
> +               .fwnode =  {1},
> +               .properties = NULL,
> +               .deadprops = NULL,
> +               .parent = NULL,
> +               .child = NULL,
> +               .sibling = NULL,
> +               .kobj = {NULL},
> +               ._flags = 0,
> +               .data = NULL
> +       };
> +       static struct device_node dnc = {
> +               .name = "name",
> +               .type = "type",
> +               .phandle = 0,
> +               .full_name = "fullname",
> +               .fwnode =  {1},
> +               .properties = NULL,
> +               .deadprops = NULL,
> +               .parent = &dn,
> +               .child = NULL,
> +               .sibling = NULL,
> +               .kobj = {NULL},
> +               ._flags = 0,
> +               .data = NULL
> +       };
> +       static char *port_names[12] = {"0", "1", "2", "3", "4",
> +               "5", "6", "7", "8", "9", "10", "11"};
> +
> +       pd->nr_chips = 1;
> +       pd->netdev = parent;
> +
> +       cd = kzalloc(sizeof(*cd), GFP_KERNEL);
> +       if (cd == NULL)
> +               return -ENOMEM;
> +
> +       pd->chip = cd;
> +
> +       cd->host_dev = parent;
> +       cd->sw_addr = 0x10;
> +       cd->eeprom_len = 256;
> +       cd->of_node = &dn;
> +       cd->rtable = 0;
> +
> +//     cd->of_node = kzalloc(sizeof(*cd->of_node), GFP_KERNEL);
> +//     if(cd->of_node == NULL)
> +//             goto free;
> +
> +       for (i = 0; i < DSA_MAX_PORTS; i++) {
> +               cd->port_names[i] = port_names[i];
> +               cd->port_dn[i] = &dnc;
> +       }
> +
> +       return 0;
> +
> +//free:
> +//     kfree(cd);
> +//     return -ENOMEM;
> +}

Use proper commenting style.
why are you keeping these commented code ?

> +
> +int dsa_probe_mii(struct mii_bus *bus, struct net_device * dev)
> +{
> +       struct dsa_platform_data *pd;
> +       struct dsa_switch_tree *dst;
> +       int ret;
> +
> +       pr_notice_once("Distributed Switch Architecture driver version %s\n",
> +                      dsa_driver_version);
> +
> +       if (dev == NULL || bus == NULL)
> +               return -EINVAL;
> +
> +       pd = kzalloc(sizeof(*pd), GFP_KERNEL);
> +       if (pd == NULL) {
> +               ret = -ENOMEM;
> +               goto freep;
> +       }
> +
> +       ret = fill_platform_data(pd, bus, bus->parent); //TODO fix it!
> +       if (ret)
> +               goto freep;
> +
> +       if (dev->dsa_ptr != NULL) {
> +               dev_put(dev);
> +               ret = -EEXIST;
> +               goto freep;
> +       }
> +
> +       dst = kzalloc(sizeof(*dst), GFP_KERNEL);

Use devm_* variant

~Rajeev
> +       if (dst == NULL) {
> +               dev_put(dev);
> +               ret = -ENOMEM;
> +               goto freed;
> +       }
> +
> +       dev_set_drvdata(bus->parent, dst);
> +
> +       dst->pd = pd;
> +       dst->master_netdev = dev;
> +
> +       dsa_probe_common(dst, bus->parent);
> +
> +       return 0;
> +
> +freed:
> +       kfree(dst);
> +freep:
> +       kfree(pd);
> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(dsa_probe_mii);
> +
>  static int dsa_remove(struct platform_device *pdev)
>  {
>         struct dsa_switch_tree *dst = platform_get_drvdata(pdev);
> --
> 2.1.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH 3/3] driver/net/usb: Add support for DSA to ax88772b

[Jan Kaisrlik]

From: Jan Kaisrlik <ja.kaisrlik@gmail.com>

This patch adds a possibility to use the RMII interface of the ax88772b
instead of the Ethernet PHY which is used now.

Binding DSA to a USB device is done via sysfs.

---
 drivers/net/usb/asix.h         |   7 ++
 drivers/net/usb/asix_devices.c | 258 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 261 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
index 5d049d0..6b1a5f5 100644
--- a/drivers/net/usb/asix.h
+++ b/drivers/net/usb/asix.h
@@ -174,6 +174,13 @@ struct asix_rx_fixup_info {
 
 struct asix_common_private {
 	struct asix_rx_fixup_info rx_fixup_info;
+#ifdef CONFIG_NET_DSA
+	struct kobject kobj;
+	struct mii_bus *mdio;
+	int use_embphy;
+	bool dsa_up;
+	struct usbnet *dev;
+#endif
 };
 
 extern const struct driver_info ax88172a_info;
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index bf49792..57b3a34 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -35,6 +35,88 @@
 
 #define	PHY_MODE_RTL8211CL	0x000C
 
+#ifdef CONFIG_NET_DSA
+
+#define AX88772_RMII 0x02
+#define AX88772_MAX_PORTS 0x20
+#define MV88e6065_ID  0x0c89
+
+#include <linux/phy.h>
+#include <net/dsa.h>
+
+#define to_asix_obj(x) container_of(x, struct asix_common_private, kobj)
+#define to_asix_attr(x) container_of(x, struct asix_attribute, attr)
+
+static int mii_asix_read(struct mii_bus *bus, int phy_id, int regnum)
+{
+	return asix_mdio_read(((struct usbnet *)bus->priv)->net, phy_id, regnum);
+}
+
+static int mii_asix_write(struct mii_bus *bus, int phy_id, int regnum, u16 val)
+{
+	asix_mdio_write(((struct usbnet *)bus->priv)->net, phy_id, regnum, val);
+	return 0;
+}
+
+static int ax88772_init_mdio(struct usbnet *dev){
+	struct asix_common_private *priv = dev->driver_priv;
+	int ret, i;
+
+	priv->mdio = mdiobus_alloc();
+	if (!priv->mdio) {
+		netdev_err(dev->net, "Could not allocate mdio bus\n");
+		return -ENOMEM;
+	}
+
+	priv->mdio->priv = (void *)dev;
+	priv->mdio->read = &mii_asix_read;
+	priv->mdio->write = &mii_asix_write;
+	priv->mdio->name = "ax88772 mdio bus";
+	priv->mdio->parent = &dev->udev->dev;
+
+	/* mii bus name is usb-<usb bus number>-<usb device number> */
+	snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "usb-%03d:%03d",dev->udev->bus->busnum, dev->udev->devnum);
+
+	priv->mdio->irq = kzalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
+	if (!priv->mdio->irq) {
+		ret = -ENOMEM;
+		goto free;
+	}
+
+	for (i = 0; i < PHY_MAX_ADDR; i++)
+		priv->mdio->irq[i] = PHY_POLL;
+
+	ret = mdiobus_register(priv->mdio);
+	if (ret) {
+		netdev_err(dev->net, "Could not register MDIO bus\n");
+		goto free_irq;
+	}
+
+	netdev_info(dev->net, "registered mdio bus %s\n", priv->mdio->id);
+	return 0;
+
+free_irq:
+	kfree(priv->mdio->irq);
+free:
+	mdiobus_free(priv->mdio);
+	return ret;
+}
+
+static void ax88772_remove_mdio(struct usbnet *dev)
+{
+	struct asix_common_private *priv = dev->driver_priv;
+
+//	dsa_free(); TODO
+
+	netdev_info(dev->net, "deregistering mdio bus %s\n", priv->mdio->id);
+	mdiobus_unregister(priv->mdio);
+	kfree(priv->mdio->irq);
+	mdiobus_free(priv->mdio);
+	kfree(priv);
+}
+
+#endif
+
 struct ax88172_int_data {
 	__le16 res1;
 	u8 link;
@@ -301,6 +383,27 @@ static int ax88772_reset(struct usbnet *dev)
 	int ret, embd_phy;
 	u16 rx_ctl;
 
+#ifdef CONFIG_NET_DSA
+	int temp = AX88772_RMII;
+	struct asix_common_private *priv = dev->driver_priv;
+
+	if (priv->use_embphy == 1) {
+		data->phymode = PHY_MODE_MARVELL;
+		data->ledmode = 0;
+
+		/* Set AX88772 to enable RMII interface for external PHY */
+		asix_write_cmd(dev, AX_CMD_SW_PHY_SELECT, 0, 0, 0, NULL);
+		asix_write_cmd(dev, AX_CMD_SW_PHY_SELECT, 0, 0, 1, &temp);
+
+		asix_sw_reset(dev, 0);
+		msleep(150);
+
+		asix_write_rx_ctl(dev, 0);
+		msleep(60);
+   }
+
+#endif
+
 	ret = asix_write_gpio(dev,
 			AX_GPIO_RSE | AX_GPIO_GPO_2 | AX_GPIO_GPO2EN, 5);
 	if (ret < 0)
@@ -415,11 +518,131 @@ static const struct net_device_ops ax88772_netdev_ops = {
 	.ndo_set_rx_mode        = asix_set_multicast,
 };
 
+
+#ifdef CONFIG_NET_DSA
+struct asix_attribute {
+	struct attribute attr;
+	ssize_t (*show)(struct asix_common_private *priv, struct asix_attribute *attr, char *buf);
+	ssize_t (*store)(struct asix_common_private *priv, struct asix_attribute *attr, const char *buf, size_t count);
+};
+
+static int ax88772_set_bind_dsa(struct asix_common_private *priv)
+{
+	struct usbnet *dev = priv->dev;
+	int i, ret, embd_phy;
+	u32 phyid;
+	int temp = AX88772_RMII;
+
+	if (priv->dsa_up == 1)
+		return -EINVAL;
+	priv->dsa_up = 1;
+
+	/* Enable RMII interface for external PHY */
+	asix_write_cmd(dev, AX_CMD_SW_PHY_SELECT, 0, 0, 1, &temp);
+	for (i = 0; i < AX88772_MAX_PORTS; i++){
+		phyid = asix_mdio_read(dev->net, i, 0x3);
+		if (phyid == MV88e6065_ID)
+			break;
+	}
+
+	if (phyid == MV88e6065_ID) {
+		ret = ax88772_init_mdio(dev);
+		if (ret)
+			return ret;
+
+		ret = asix_read_cmd(dev, AX_CMD_SW_PHY_STATUS, 0, 0, 1, &priv->use_embphy);
+		priv->use_embphy = 1;
+		ret = dsa_probe_mii(priv->mdio, dev->net);
+		if (ret)
+			return ret;
+
+		dev->mii.phy_id = 0x11; //TODO load addr of mii reg
+	}else{
+		/* Revert to previous settings */
+		embd_phy = ((dev->mii.phy_id & 0x1f) == 0x10 ? 1 : 0);
+		ret = asix_write_cmd(dev, AX_CMD_SW_PHY_SELECT, embd_phy, 0, 0, NULL);
+		if (ret)
+			return ret;
+
+		priv->use_embphy = 0;
+	}
+
+	return 0;
+}
+
+static ssize_t usb_dsa_store(struct asix_common_private *priv,
+		struct asix_attribute *attr, const char *buf, size_t count)
+{
+	ax88772_set_bind_dsa(priv);
+	return count;
+}
+
+static ssize_t usb_dsa_show(struct asix_common_private *priv,
+		struct asix_attribute *attr, char *buf)
+{
+	return scnprintf(buf, PAGE_SIZE, "usb_dsa_binding.\n");
+}
+
+static void driver_release(struct kobject *kobj)
+{
+	pr_debug("driver: '%s': %s\n", kobject_name(kobj), __func__);
+//   kfree(drv_priv); TODO free
+}
+
+static ssize_t usb_dsa_attr_show(struct kobject *kobj,
+	struct attribute *attr,
+	char *buf)
+{
+	struct asix_attribute *attribute;
+	struct asix_common_private *priv;
+
+	attribute = to_asix_attr(attr);
+	priv = to_asix_obj(kobj);
+
+	if (!attribute->show)
+		return -EINVAL;
+
+	return attribute->show(priv, attribute, buf);
+}
+static ssize_t usb_dsa_attr_store(struct kobject *kobj,
+		struct attribute *attr,
+		const char *buf, size_t len)
+{
+	struct asix_attribute *attribute;
+	struct asix_common_private *priv;
+
+	attribute = to_asix_attr(attr);
+	priv = to_asix_obj(kobj);
+
+	if (!attribute->store)
+		return -EINVAL;
+	return attribute->store(priv, attribute, buf, len);
+}
+
+static struct asix_attribute asix_attribute =  __ATTR(usb_dsa_bind, 0664, usb_dsa_show, usb_dsa_store);
+static struct attribute *asix_default_attrs[] = {
+	&asix_attribute.attr,
+	NULL,
+};
+static const struct sysfs_ops dsa_bind_sysfs_ops = {
+	.show   = usb_dsa_attr_show,
+	.store  = usb_dsa_attr_store,
+};
+static struct kobj_type dsa_bind_ktype = {
+	.sysfs_ops      = &dsa_bind_sysfs_ops,
+	.release        = driver_release,
+	.default_attrs  = asix_default_attrs,
+};
+#endif
+
 static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 {
 	int ret, embd_phy, i;
 	u8 buf[ETH_ALEN];
 	u32 phyid;
+#ifdef CONFIG_NET_DSA
+	struct asix_common_private *priv;
+#endif
 
 	usbnet_get_endpoints(dev,intf);
 
@@ -465,6 +688,25 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 		return ret;
 	}
 
+	dev->driver_priv = kzalloc(sizeof(struct asix_common_private), GFP_KERNEL);
+	if (!dev->driver_priv)
+		return -ENOMEM;
+
+#ifdef CONFIG_NET_DSA
+	priv = dev->driver_priv;
+	priv->dev = dev;
+	priv->dsa_up = 0;
+	priv->kobj.kset = kset_create_and_add("DSA_BIND", NULL, kobject_get(&dev->udev->dev.kobj));
+	if (!priv->kobj.kset){
+		ret = -ENOMEM;
+		goto free;
+	}
+
+	ret = kobject_init_and_add(&priv->kobj, &dsa_bind_ktype, NULL, "dsa_bind");
+	if (ret)
+		goto free_kobj;
+#endif
+
 	ax88772_reset(dev);
 
 	/* Read PHYID register *AFTER* the PHY was reset properly */
@@ -478,15 +720,23 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 		dev->rx_urb_size = 2048;
 	}
 
-	dev->driver_priv = kzalloc(sizeof(struct asix_common_private), GFP_KERNEL);
-	if (!dev->driver_priv)
-		return -ENOMEM;
-
 	return 0;
+
+#ifdef CONFIG_NET_DSA
+free_kobj:
+	kobject_put(&priv->kobj);
+free:
+	kfree(priv);
+	return ret;
+#endif
 }
 
 static void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
+#ifdef CONFIG_NET_DSA
+	if (((struct asix_common_private *)dev->driver_priv)->dsa_up == 1)
+	   ax88772_remove_mdio(dev);
+#endif
 	kfree(dev->driver_priv);
 }
 
-- 
2.1.3

Re: [RFC PATCH 3/3] driver/net/usb: Add support for DSA to ax88772b

[Bjørn Mork]

Jan Kaisrlik <kaisrja1@fel.cvut.cz> writes:

> From: Jan Kaisrlik <ja.kaisrlik@gmail.com>
>
> This patch adds a possibility to use the RMII interface of the ax88772b
> instead of the Ethernet PHY which is used now.
>
> Binding DSA to a USB device is done via sysfs.
>
> ---
>  drivers/net/usb/asix.h         |   7 ++
>  drivers/net/usb/asix_devices.c | 258 ++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 261 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
> index 5d049d0..6b1a5f5 100644
> --- a/drivers/net/usb/asix.h
> +++ b/drivers/net/usb/asix.h
> @@ -174,6 +174,13 @@ struct asix_rx_fixup_info {
>  
>  struct asix_common_private {
>  	struct asix_rx_fixup_info rx_fixup_info;
> +#ifdef CONFIG_NET_DSA
> +	struct kobject kobj;
> +	struct mii_bus *mdio;
> +	int use_embphy;
> +	bool dsa_up;
> +	struct usbnet *dev;
> +#endif
>  };
>  
>  extern const struct driver_info ax88172a_info;
> diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
> index bf49792..57b3a34 100644
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -35,6 +35,88 @@
>  
>  #define	PHY_MODE_RTL8211CL	0x000C
>  
> +#ifdef CONFIG_NET_DSA
> +
> +#define AX88772_RMII 0x02
> +#define AX88772_MAX_PORTS 0x20
> +#define MV88e6065_ID  0x0c89
> +
> +#include <linux/phy.h>
> +#include <net/dsa.h>
> +
> +#define to_asix_obj(x) container_of(x, struct asix_common_private, kobj)
> +#define to_asix_attr(x) container_of(x, struct asix_attribute, attr)
> +
> +static int mii_asix_read(struct mii_bus *bus, int phy_id, int regnum)
> +{
> +	return asix_mdio_read(((struct usbnet *)bus->priv)->net, phy_id, regnum);
> +}
> +
> +static int mii_asix_write(struct mii_bus *bus, int phy_id, int regnum, u16 val)
> +{
> +	asix_mdio_write(((struct usbnet *)bus->priv)->net, phy_id, regnum, val);
> +	return 0;
> +}
> +
> +static int ax88772_init_mdio(struct usbnet *dev){
> +	struct asix_common_private *priv = dev->driver_priv;
> +	int ret, i;
> +
> +	priv->mdio = mdiobus_alloc();
> +	if (!priv->mdio) {
> +		netdev_err(dev->net, "Could not allocate mdio bus\n");
> +		return -ENOMEM;
> +	}
> +
> +	priv->mdio->priv = (void *)dev;
> +	priv->mdio->read = &mii_asix_read;
> +	priv->mdio->write = &mii_asix_write;
> +	priv->mdio->name = "ax88772 mdio bus";
> +	priv->mdio->parent = &dev->udev->dev;
> +
> +	/* mii bus name is usb-<usb bus number>-<usb device number> */
> +	snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "usb-%03d:%03d",dev->udev->bus->busnum, dev->udev->devnum);
> +
> +	priv->mdio->irq = kzalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
> +	if (!priv->mdio->irq) {
> +		ret = -ENOMEM;
> +		goto free;
> +	}
> +
> +	for (i = 0; i < PHY_MAX_ADDR; i++)
> +		priv->mdio->irq[i] = PHY_POLL;
> +
> +	ret = mdiobus_register(priv->mdio);
> +	if (ret) {
> +		netdev_err(dev->net, "Could not register MDIO bus\n");
> +		goto free_irq;
> +	}
> +
> +	netdev_info(dev->net, "registered mdio bus %s\n", priv->mdio->id);
> +	return 0;
> +
> +free_irq:
> +	kfree(priv->mdio->irq);
> +free:
> +	mdiobus_free(priv->mdio);
> +	return ret;
> +}

There is already identical code in drivers/net/usb/ax88172a.c.  Any
chance these ASIX devices can share some code, or does it all have to be
duplicated for each new chip?


> +//	dsa_free(); TODO

Probably not like that...


> +static ssize_t usb_dsa_store(struct asix_common_private *priv,
> +		struct asix_attribute *attr, const char *buf, size_t count)
> +{
> +	ax88772_set_bind_dsa(priv);
> +	return count;
> +}
> +
> +static ssize_t usb_dsa_show(struct asix_common_private *priv,
> +		struct asix_attribute *attr, char *buf)
> +{
> +	return scnprintf(buf, PAGE_SIZE, "usb_dsa_binding.\n");
> +}

I'm not sure I understand this at all.  What kind of userspace API are
you trying to provide here? Maybe you could document these attributes
Documentation/ABI/testing/ to make it more clear?

> +static void driver_release(struct kobject *kobj)
> +{
> +	pr_debug("driver: '%s': %s\n", kobject_name(kobj), __func__);
> +//   kfree(drv_priv); TODO free
> +}

Ah, I guess you might have missed this section of
Documentation/kobject.txt ?:

  One important point cannot be overstated: every kobject must have a
  release() method, and the kobject must persist (in a consistent state)
  until that method is called. If these constraints are not met, the
  code is flawed.  Note that the kernel will warn you if you forget to
  provide a release() method.  Do not try to get rid of this warning by
  providing an "empty" release function; you will be mocked mercilessly
  by the kobject maintainer if you attempt this.

Better CC Greg KH on your next attempt to make sure you get the mocking
you deserve :-)


> +static ssize_t usb_dsa_attr_show(struct kobject *kobj,
> +	struct attribute *attr,
> +	char *buf)
> +{
> +	struct asix_attribute *attribute;
> +	struct asix_common_private *priv;
> +
> +	attribute = to_asix_attr(attr);
> +	priv = to_asix_obj(kobj);
> +
> +	if (!attribute->show)
> +		return -EINVAL;
> +
> +	return attribute->show(priv, attribute, buf);
> +}
> +static ssize_t usb_dsa_attr_store(struct kobject *kobj,
> +		struct attribute *attr,
> +		const char *buf, size_t len)
> +{
> +	struct asix_attribute *attribute;
> +	struct asix_common_private *priv;
> +
> +	attribute = to_asix_attr(attr);
> +	priv = to_asix_obj(kobj);
> +
> +	if (!attribute->store)
> +		return -EINVAL;
> +	return attribute->store(priv, attribute, buf, len);
> +}
> +
> +static struct asix_attribute asix_attribute =  __ATTR(usb_dsa_bind, 0664, usb_dsa_show, usb_dsa_store);
> +static struct attribute *asix_default_attrs[] = {
> +	&asix_attribute.attr,
> +	NULL,
> +};
> +static const struct sysfs_ops dsa_bind_sysfs_ops = {
> +	.show   = usb_dsa_attr_show,
> +	.store  = usb_dsa_attr_store,
> +};
> +static struct kobj_type dsa_bind_ktype = {
> +	.sysfs_ops      = &dsa_bind_sysfs_ops,
> +	.release        = driver_release,
> +	.default_attrs  = asix_default_attrs,
> +};
> +#endif
> +
>  static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
>  {
>  	int ret, embd_phy, i;
>  	u8 buf[ETH_ALEN];
>  	u32 phyid;
> +#ifdef CONFIG_NET_DSA
> +	struct asix_common_private *priv;
> +#endif
>  
>  	usbnet_get_endpoints(dev,intf);
>  
> @@ -465,6 +688,25 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
>  		return ret;
>  	}
>  
> +	dev->driver_priv = kzalloc(sizeof(struct asix_common_private), GFP_KERNEL);

Unconditional allocation.

> +	if (!dev->driver_priv)
> +		return -ENOMEM;
> +
> +#ifdef CONFIG_NET_DSA
> +	priv = dev->driver_priv;
> +	priv->dev = dev;
> +	priv->dsa_up = 0;
> +	priv->kobj.kset = kset_create_and_add("DSA_BIND", NULL, kobject_get(&dev->udev->dev.kobj));
> +	if (!priv->kobj.kset){
> +		ret = -ENOMEM;
> +		goto free;
> +	}
> +
> +	ret = kobject_init_and_add(&priv->kobj, &dsa_bind_ktype, NULL, "dsa_bind");
> +	if (ret)
> +		goto free_kobj;
> +#endif

I see that you reference the usb device here, but I don't see why.  This
is related to et network device, isn't it?  What's wrong about simply
using dev->net->sysfs_groups[0] instead?


> +#ifdef CONFIG_NET_DSA
> +free_kobj:
> +	kobject_put(&priv->kobj);
> +free:
> +	kfree(priv);
> +	return ret;
> +#endif

Conditional kfree.  Obfuscated by the fact that you have a conditionally
defined *priv pointing to dev->driver_priv, but it doesn't change the
fact that you leak on errors.




Bjørn