On Thu, Sep 06, 2012 at 10:46:00PM +0200, Fabio Baltieri wrote:

On Thu, Sep 06, 2012 at 09:31:07PM +0200, Oliver Hartkopp wrote:

quoted

quoted

quoted

+
+/*
+ * NETDEV rename notifier to rename the associated led triggers too
+ */
+static int can_led_notifier(struct notifier_block *nb, unsigned long msg,
+			void *data)
+{
+	struct net_device *netdev = (struct net_device *)data;
+	struct can_priv *priv = netdev_priv(netdev);

That's the main problem, which I also got stuck into when I did my first
can-led implementation.  As LED structures are in netdev's private data,
you can only use it if your driver is based on the can-dev API, and
there are no way to be sure of that if you get outside driver's code
itself.

This would give problems with vcan, slcan, and probabily other
non-mainlined drivers.

Do you think, this is really a problem?

If a driver decides not to use the can-dev framework it has to implement own
solutions or just adopt can-dev.

Agreed, but this still means that we can't assume that
netdev_priv(netdev) to a netdev where netdev->type == ARPHRD_CAN points
to a struct can_priv, right?

It remains a problem for vcan & slcan.
I tend to start looking at how netlink deals with it.
I see 2 options:
* Invent a new flag for netdev->features, like NETIF_F_CANDEV, to be assigned
  at http://lxr.free-electrons.com/source/drivers/net/can/dev.c#L464

* test for the rtnl_link_ops:
  We put a function can_priv_safe(netdev) in driver/net/can/dev.c that tests
  netdev->rtnl_link_ops == &can_link_ops, like in
  http://lxr.free-electrons.com/source/drivers/net/can/dev.c#L751
  and for candev based drivers, it returns a can_priv*, otherwise NULL.
  I'll prepare something early next week ...

Kind regards,
Kurt