Re: [patch 08/10] bcm43xx: sysfs code cleanup

[Michael Buesch <mb@bu3sch.de>]

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

On Wednesday 19 April 2006 23:50, you wrote:
> akpm@osdl.org <akpm@osdl.org> :
> > 
> > From: Michael Buesch <mb@bu3sch.de>
> > 
> > This cleans up the bcm43xx sysfs code and makes it compliant with the
> > unwritten sysfs rules (at least I hope so).
> > 
> > Signed-off-by: Michael Buesch <mb@bu3sch.de>
> > Cc: Jeff Garzik <jeff@garzik.org>
> > Cc: Greg KH <greg@kroah.com>
> > Signed-off-by: Andrew Morton <akpm@osdl.org>
> [...]
> > diff -puN drivers/net/wireless/bcm43xx/bcm43xx.h~bcm43xx-sysfs-code-cleanup drivers/net/wireless/bcm43xx/bcm43xx.h
> > --- devel/drivers/net/wireless/bcm43xx/bcm43xx.h~bcm43xx-sysfs-code-cleanup	2006-04-12 18:11:12.000000000 -0700
> > +++ devel-akpm/drivers/net/wireless/bcm43xx/bcm43xx.h	2006-04-12 18:11:12.000000000 -0700
> [...]
> > @@ -772,6 +769,20 @@ struct bcm43xx_private * bcm43xx_priv(st
> >  	return ieee80211softmac_priv(dev);
> >  }
> >  
> > +struct device;
> > +
> > +static inline
> > +struct bcm43xx_private * dev_to_bcm(struct device *dev)
> > +{
> > +	struct net_device *net_dev;
> > +	struct bcm43xx_private *bcm;
> > +
> > +	net_dev = dev_get_drvdata(dev);
> > +	bcm = bcm43xx_priv(net_dev);
> > +
> > +	return bcm;
> > +}
> > +
> 
> It's used only in bcm43xx_sysfs.c. It could go there and avoid
> an extra forward declaration.

But it is a very generic helper, that does not have anything to do
with sysfs. It converts a struct device to struct bcm43xx_private.
bcm43xx_sysfs.h does only contain sysfs specific stuff for bcm.

> [...]
> > diff -puN drivers/net/wireless/bcm43xx/bcm43xx_sysfs.c~bcm43xx-sysfs-code-cleanup drivers/net/wireless/bcm43xx/bcm43xx_sysfs.c
> > --- devel/drivers/net/wireless/bcm43xx/bcm43xx_sysfs.c~bcm43xx-sysfs-code-cleanup	2006-04-12 18:11:12.000000000 -0700
> > +++ devel-akpm/drivers/net/wireless/bcm43xx/bcm43xx_sysfs.c	2006-04-12 18:11:12.000000000 -0700
> > @@ -71,14 +71,46 @@ static int get_boolean(const char *buf, 
> >  	return -EINVAL;
> >  }
> >  
> > +static int sprom2hex(const u16 *sprom, char *buf, size_t buf_len)
> > +{
> > +	int i, pos = 0;
> > +
> > +	for (i = 0; i < BCM43xx_SPROM_SIZE; i++) {
> > +		pos += snprintf(buf + pos, buf_len - pos - 1,
> > +				"%04X", swab16(sprom[i]) & 0xFFFF);
> > +	}
> > +	pos += snprintf(buf + pos, buf_len - pos - 1, "\n");
> > +
> > +	return pos + 1;
> > +}
> > +
> > +static int hex2sprom(u16 *sprom, const char *dump, size_t len)
> > +{
> > +	char tmp[5] = { 0 };
> > +	int cnt = 0;
> > +	unsigned long parsed;
> > +
> > +	if (len < BCM43xx_SPROM_SIZE * sizeof(u16) * 2)
> > +		return -EINVAL;
> > +
> > +	while (cnt < BCM43xx_SPROM_SIZE) {
> > +		memcpy(tmp, dump, 4);
> > +		dump += 4;
> > +		parsed = simple_strtoul(tmp, NULL, 16);
> > +		sprom[cnt++] = swab16((u16)parsed);
> > +	}
> > +
> > +	return 0;
> > +}
> 
> "4" suggests that the code walks dump -> dump + 4*BCM43xx_SPROM_SIZE
> 
> >  static ssize_t bcm43xx_attr_sprom_store(struct device *dev,
> >  					const char *buf, size_t count)
> [...]
> > -	for (i = 0; i < BCM43xx_SPROM_SIZE; i++) {
> > -		sprom[i] = buf[i * 2] & 0xFF;
> > -		sprom[i] |= ((u16)(buf[i * 2 + 1] & 0xFF)) << 8;
> > -	}
> > +	err = hex2sprom(sprom, buf, count);
> 
> ... whereas here it only parses buf -> buf + 2*BCM43xx_SPROM_SIZE

the deleted code parses binary input. The
new code parses human readable hex input.

-- 
Greetings Michael.

[-- Attachment #2: Type: application/pgp-signature, Size: 191 bytes --]