[PATCH 1/2] net can softing: drop 'channel' sysfs attribute

[PATCH 1/2] net can softing: drop 'channel' sysfs attribute

[Kurt Van Dijck]

netdev->dev_id obsoletes this property.
None of the remaining properties contribute to udev detection methods.
The regular calls for the sysfs group can thus safely be restored.

Signed-off-by: Kurt Van Dijck <kurt.van.dijck@eia.be>
---
 drivers/net/can/softing/softing_main.c |   16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/net/can/softing/softing_main.c b/drivers/net/can/softing/softing_main.c
index 65eef1e..6cfbccd 100644
--- a/drivers/net/can/softing/softing_main.c
+++ b/drivers/net/can/softing/softing_main.c
@@ -558,15 +558,6 @@ failed:
 /*
  * netdev sysfs
  */
-static ssize_t show_channel(struct device *dev, struct device_attribute *attr,
-		char *buf)
-{
-	struct net_device *ndev = to_net_dev(dev);
-	struct softing_priv *priv = netdev2softing(ndev);
-
-	return sprintf(buf, "%i\n", priv->index);
-}
-
 static ssize_t show_chip(struct device *dev, struct device_attribute *attr,
 		char *buf)
 {
@@ -611,12 +602,10 @@ static ssize_t store_output(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
-static const DEVICE_ATTR(channel, S_IRUGO, show_channel, NULL);
 static const DEVICE_ATTR(chip, S_IRUGO, show_chip, NULL);
 static const DEVICE_ATTR(output, S_IRUGO | S_IWUSR, show_output, store_output);
 
 static const struct attribute *const netdev_sysfs_attrs[] = {
-	&dev_attr_channel.attr,
 	&dev_attr_chip.attr,
 	&dev_attr_output.attr,
 	NULL,
@@ -680,17 +669,20 @@ static int softing_netdev_register(struct net_device *netdev)
 {
 	int ret;
 
-	netdev->sysfs_groups[0] = &netdev_sysfs_group;
 	ret = register_candev(netdev);
 	if (ret) {
 		dev_alert(&netdev->dev, "register failed\n");
 		return ret;
 	}
+	if (sysfs_create_group(&netdev->dev.kobj, &netdev_sysfs_group) < 0)
+		netdev_alert(netdev, "sysfs group failed\n");
+
 	return 0;
 }
 
 static void softing_netdev_cleanup(struct net_device *netdev)
 {
+	sysfs_remove_group(&netdev->dev.kobj, &netdev_sysfs_group);
 	unregister_candev(netdev);
 	free_candev(netdev);
 }
-- 
1.7.10.4

[PATCH 2/2] net can softing: remove unused sysfs attributes

[Kurt Van Dijck]

'frequency' indicates the embedded cpu's frequency, but that
should not be necessary for any purpose.
'txpending' is an attribute for debugging.

Signed-off-by: Kurt Van Dijck <kurt.van.dijck@eia.be>
---
 drivers/net/can/softing/softing_main.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/can/softing/softing_main.c b/drivers/net/can/softing/softing_main.c
index 6cfbccd..fb5312c 100644
--- a/drivers/net/can/softing/softing_main.c
+++ b/drivers/net/can/softing/softing_main.c
@@ -714,8 +714,6 @@ DEV_ATTR_RO(firmware_version, id.fw_version);
 DEV_ATTR_RO_STR(hardware, pdat->name);
 DEV_ATTR_RO(hardware_version, id.hw_version);
 DEV_ATTR_RO(license, id.license);
-DEV_ATTR_RO(frequency, id.freq);
-DEV_ATTR_RO(txpending, tx.pending);
 
 static struct attribute *softing_pdev_attrs[] = {
 	&dev_attr_serial.attr,
@@ -724,8 +722,6 @@ static struct attribute *softing_pdev_attrs[] = {
 	&dev_attr_hardware.attr,
 	&dev_attr_hardware_version.attr,
 	&dev_attr_license.attr,
-	&dev_attr_frequency.attr,
-	&dev_attr_txpending.attr,
 	NULL,
 };
 
-- 
1.7.10.4

Re: [PATCH 1/2] net can softing: drop 'channel' sysfs attribute

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 2638 bytes --]

On 04/02/2014 09:17 PM, Kurt Van Dijck wrote:
> netdev->dev_id obsoletes this property.
> None of the remaining properties contribute to udev detection methods.
> The regular calls for the sysfs group can thus safely be restored.
> 
> Signed-off-by: Kurt Van Dijck <kurt.van.dijck@eia.be>
> ---
>  drivers/net/can/softing/softing_main.c |   16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/can/softing/softing_main.c b/drivers/net/can/softing/softing_main.c
> index 65eef1e..6cfbccd 100644
> --- a/drivers/net/can/softing/softing_main.c
> +++ b/drivers/net/can/softing/softing_main.c
> @@ -558,15 +558,6 @@ failed:
>  /*
>   * netdev sysfs
>   */
> -static ssize_t show_channel(struct device *dev, struct device_attribute *attr,
> -		char *buf)
> -{
> -	struct net_device *ndev = to_net_dev(dev);
> -	struct softing_priv *priv = netdev2softing(ndev);
> -
> -	return sprintf(buf, "%i\n", priv->index);
> -}
> -
>  static ssize_t show_chip(struct device *dev, struct device_attribute *attr,
>  		char *buf)
>  {
> @@ -611,12 +602,10 @@ static ssize_t store_output(struct device *dev, struct device_attribute *attr,
>  	return count;
>  }
>  
> -static const DEVICE_ATTR(channel, S_IRUGO, show_channel, NULL);
>  static const DEVICE_ATTR(chip, S_IRUGO, show_chip, NULL);
>  static const DEVICE_ATTR(output, S_IRUGO | S_IWUSR, show_output, store_output);
>  
>  static const struct attribute *const netdev_sysfs_attrs[] = {
> -	&dev_attr_channel.attr,
>  	&dev_attr_chip.attr,
>  	&dev_attr_output.attr,
>  	NULL,
> @@ -680,17 +669,20 @@ static int softing_netdev_register(struct net_device *netdev)
>  {
>  	int ret;
>  
> -	netdev->sysfs_groups[0] = &netdev_sysfs_group;
>  	ret = register_candev(netdev);
>  	if (ret) {
>  		dev_alert(&netdev->dev, "register failed\n");
>  		return ret;
>  	}
> +	if (sysfs_create_group(&netdev->dev.kobj, &netdev_sysfs_group) < 0)
> +		netdev_alert(netdev, "sysfs group failed\n");
> +

What is the difference between using sysfs_create_group() and
netdev->sysfs_groups[0]?

>  	return 0;
>  }
>  
>  static void softing_netdev_cleanup(struct net_device *netdev)
>  {
> +	sysfs_remove_group(&netdev->dev.kobj, &netdev_sysfs_group);
>  	unregister_candev(netdev);
>  	free_candev(netdev);
>  }
> 

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]

Re: [PATCH 1/2] net can softing: drop 'channel' sysfs attribute

[Kurt Van Dijck]

On Thu, Apr 17, 2014 at 10:29:28PM +0200, Marc Kleine-Budde wrote:
> On 04/02/2014 09:17 PM, Kurt Van Dijck wrote:
> > netdev->dev_id obsoletes this property.
> > None of the remaining properties contribute to udev detection methods.
> > The regular calls for the sysfs group can thus safely be restored.
> > 
> > Signed-off-by: Kurt Van Dijck <kurt.van.dijck@eia.be>
> > ---
> >  drivers/net/can/softing/softing_main.c |   16 ++++------------
> >  1 file changed, 4 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/net/can/softing/softing_main.c b/drivers/net/can/softing/softing_main.c
> > index 65eef1e..6cfbccd 100644
> > --- a/drivers/net/can/softing/softing_main.c
> > +++ b/drivers/net/can/softing/softing_main.c
[...]
> > @@ -680,17 +669,20 @@ static int softing_netdev_register(struct net_device *netdev)
> >  {
> >  	int ret;
> >  
> > -	netdev->sysfs_groups[0] = &netdev_sysfs_group;
> >  	ret = register_candev(netdev);
> >  	if (ret) {
> >  		dev_alert(&netdev->dev, "register failed\n");
> >  		return ret;
> >  	}
> > +	if (sysfs_create_group(&netdev->dev.kobj, &netdev_sysfs_group) < 0)
> > +		netdev_alert(netdev, "sysfs group failed\n");
> > +
> 
> What is the difference between using sysfs_create_group() and
> netdev->sysfs_groups[0]?
> 

register_candev() will (indirectly) add a sysfs node
and trigger a uevent.

calling sysfs_create_group only after that implies that the uevent
is fired potentially before the attr_group is added, and your uevent
handling code will not see it.

If you introduce sysfs attributes that way that are used to
discriminate between net_devices, then a race condition is introduced.

calling sysfs_create_group before register_candev doesn't work because
the sysfs node is not yet there.

netdev->sysfs_groups was introduced (at the same time as I needed this)
to avoid the race condition: the attr_groups listed there are added
before the uevent is fired, and after the sysfs device is created.

I needed to add to sysfs_groups because I added a sysfs attribute
that indicated the channel, potentially used to identify the can net_device.

So, the answer to your question is:
The point in time relative to the uevent at which the attr_group is added.

I dropped that sysfs attribute, and so I can use the sysfs_create_group,
which is more common and therefore a better choice.

Kurt

Re: [PATCH 1/2] net can softing: drop 'channel' sysfs attribute

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 621 bytes --]

On 04/02/2014 09:17 PM, Kurt Van Dijck wrote:
> netdev->dev_id obsoletes this property.
> None of the remaining properties contribute to udev detection methods.
> The regular calls for the sysfs group can thus safely be restored.
> 
> Signed-off-by: Kurt Van Dijck <kurt.van.dijck@eia.be>

Both applied to can-next/master

Tnx,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]