From: liujunliang_ljl <liujunliang_ljl@163•com>
To: "Joe Perches" <joe@perches•com>, horms <horms@verge•net.au>,
davem <davem@davemloft•net>
Cc: "Francois Romieu" <romieu@fr•zoreil.com>, gregkh <gregkh@suse•de>,
sunhecheng <sunhecheng@vip•126.com>,
linux-usb <linux-usb@vger•kernel.org>,
netdev <netdev@vger•kernel.org>,
linux-kernel <linux-kernel@vger•kernel.org>
Subject: Re: Re: Re: [PATCH-SR9700] Merge USB 1.1 Ethernet Adapter SR9700DeviceDriver into the Linux Kernel
Date: Thu, 22 Aug 2013 17:23:23 +0800 [thread overview]
Message-ID: <201308221723223905126@163.com> (raw)
In-Reply-To: 201308211807295009698@163.com
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.
next prev parent reply other threads:[~2013-08-22 9:23 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-20 10:41 [PATCH-SR9700] Merge USB 1.1 Ethernet Adapter SR9700 Device Driver into the Linux Kernel liujunliang_ljl
2013-08-20 20:46 ` Francois Romieu
2013-08-20 20:58 ` Joe Perches
2013-08-21 10:07 ` Re: [PATCH-SR9700] Merge USB 1.1 Ethernet Adapter SR9700 DeviceDriver " liujunliang_ljl
2013-08-21 11:34 ` Joe Perches
2013-08-22 9:23 ` liujunliang_ljl [this message]
2013-08-21 10:06 ` liujunliang_ljl
2013-08-21 16:20 ` Greg KH
2013-08-21 20:33 ` Francois Romieu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201308221723223905126@163.com \
--to=liujunliang_ljl@163$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=gregkh@suse$(echo .)de \
--cc=horms@verge$(echo .)net.au \
--cc=joe@perches$(echo .)com \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux-usb@vger$(echo .)kernel.org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=romieu@fr$(echo .)zoreil.com \
--cc=sunhecheng@vip$(echo .)126.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox