linux-kernel - Re: Re: Re: [PATCH-SR9700] Merge USB 1.1 Ethernet Adapter SR9700DeviceDriver into the Linux Kernel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <201308221723223905126@163.com>
Date:	Thu, 22 Aug 2013 17:23:23 +0800
From:	"liujunliang_ljl" <liujunliang_ljl@....com>
To:	"Joe Perches" <joe@...ches.com>,
	"horms" <horms@...ge.net.au>,
	"davem" <davem@...emloft.net>
Cc:	"Francois Romieu" <romieu@...zoreil.com>,
	"gregkh" <gregkh@...e.de>,
	"sunhecheng" <sunhecheng@....126.com>,
	"linux-usb" <linux-usb@...r.kernel.org>,
	"netdev" <netdev@...r.kernel.org>,
	"linux-kernel" <linux-kernel@...r.kernel.org>
Subject: Re: Re: Re: [PATCH-SR9700] Merge USB 1.1 Ethernet Adapter SR9700DeviceDriver into the Linux Kernel

Dear all :

		Thanks all of you about your attentions and suggestions, and I think firstly I should read the documentations carefully.

		and then commiting the patch. 

		Thanks all of you for your help and welcome all of you to come to china.

		Thanks again.



2013-08-22 



liujunliang_ljl 



发件人： Joe Perches 
发送时间： 2013-08-21  19:34:10 
收件人： liujunliang_ljl 
抄送： Francois Romieu; gregkh; sunhecheng; linux-usb; netdev; linux-kernel 
主题： Re: Re: [PATCH-SR9700] Merge USB 1.1 Ethernet Adapter SR9700DeviceDriver into the Linux Kernel 
 
On Wed, 2013-08-21 at 18:07 +0800, liujunliang_ljl wrote:
>  Thanks a lot and I have been fixed all the problems mentioned above. please check the following patch and thanks again. 
Just trivial comments below:
> diff --git a/drivers/net/usb/sr9700.c b/drivers/net/usb/sr9700.c
[]
> +static void sr_write_reg_async(struct usbnet *dev, u8 reg, u8 value)
> +{
> + usbnet_write_cmd_async(dev, SR_WR_REGS, SR_REQ_WR_REG,
> + value, reg, NULL, 0);
> +}
> +
> +static int sr_share_read_word(struct usbnet *dev, int phy, u8 reg, __le16 *value)
> +{
[]
> + netdev_dbg(dev->net, "read shared %d 0x%02x returned 0x%04x, %d\n",
> +        phy, reg, *value, ret);
You have a lot of code that uses inconsistent
indentation.  Code in drivers/net and drivers/usb/net
generally prefers to use alignment to parenthesis for
multi-line statements
The first could use
usbnet_write_cmd_async(dev, SR_WR_REGS, SR_REQ_WR_REG,
       value, reg, NULL, 0);
and the second
netdev_dbg(dev->net, "read shared %d 0x%02x returned 0x%04x, %d\n",
   phy, reg, *value, ret);
Maximal use of 8 space indentation tabs followed by
minimal spaces.
There are many of these above.
> +static int sr9700_get_eeprom(struct net_device *net, struct ethtool_eeprom *eeprom, u8 *data)
[]
> + for (i = 0; i < eeprom->len / 2; i++)
> + ret = sr_read_eeprom_word(dev, eeprom->offset / 2 + i, &ebuf[i]);
One too many tabs for the second line, a few of these...
[]
> +static int sr_mdio_read(struct net_device *netdev, int phy_id, int loc)
[]
> + if (rc == 1)
> + return le16_to_cpu(res) | BMSR_LSTATUS;
> + else
> + return le16_to_cpu(res) & ~BMSR_LSTATUS;
The code below the returns here is unreachable.
> +
> + netdev_dbg(dev->net,
> +        "sr_mdio_read() phy_id=0x%02x, loc=0x%02x, returns=0x%04x\n",
> +        phy_id, loc, le16_to_cpu(res));
> +
> + return le16_to_cpu(res);
> +}
You might try to use scripts/checkpatch.pl --strict if you
care about these.  It should flag most of these coding
style inconsistencies.