Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1700072
| From | Vivien Didelot <vivien.didelot@savoirfairelinux.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH net-next 2/2] net: dsa: lan9303: Simplify lan9303_xxx_packet_processing() usage |
| Date | 2017-07-31 16:10 +0200 |
| Message-ID | <u9jyq-7fE-5@gated-at.bofh.it> (permalink) |
| References | <u9hdg-5GQ-7@gated-at.bofh.it> <u9hdh-5GQ-35@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
Hi Egil,
A few nitpicks below for lan9303_disable_processing.
Egil Hjelmeland <privat@egil-hjelmeland.no> writes:
> static int lan9303_disable_processing(struct lan9303 *chip)
> {
> - int ret;
> + int p;
>
> - ret = lan9303_disable_packet_processing(chip, 0);
> - if (ret)
> - return ret;
> - ret = lan9303_disable_packet_processing(chip, 1);
> - if (ret)
> - return ret;
> - return lan9303_disable_packet_processing(chip, 2);
> + for (p = 0; p <= 2; p++) {
Exclusive limits are often prefer, i.e. 'p < 3'.
> + int ret;
> +
> + ret = lan9303_disable_packet_processing(chip, p);
> + if (ret)
> + return ret;
When any non-zero return code means an error, we usually see 'err'
instead of 'ret'.
> + }
blank line before return statments are appreciated.
> + return 0;
> }
>
> static int lan9303_check_device(struct lan9303 *chip)
> @@ -760,7 +761,6 @@ static int lan9303_port_enable(struct dsa_switch *ds, int port,
> /* enable internal packet processing */
> switch (port) {
> case 1:
> - return lan9303_enable_packet_processing(chip, port);
Is this deletion intentional? The commit message does not explain this.
When possible, it is appreciated to separate functional from
non-functional changes. For example one commit adding the loop in
lan9303_disable_processing and another one to not enable/disable packet
processing on port 1.
> case 2:
> return lan9303_enable_packet_processing(chip, port);
> default:
> @@ -779,13 +779,9 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port,
> /* disable internal packet processing */
> switch (port) {
> case 1:
> - lan9303_disable_packet_processing(chip, port);
> - lan9303_phy_write(ds, chip->phy_addr_sel_strap + 1,
> - MII_BMCR, BMCR_PDOWN);
> - break;
> case 2:
> lan9303_disable_packet_processing(chip, port);
> - lan9303_phy_write(ds, chip->phy_addr_sel_strap + 2,
> + lan9303_phy_write(ds, chip->phy_addr_sel_strap + port,
> MII_BMCR, BMCR_PDOWN);
> break;
Thanks,
Vivien
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH net-next 0/2] Refactor lan9303_xxx_packet_processing Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-31 13:40 +0200
[PATCH net-next 2/2] net: dsa: lan9303: Simplify lan9303_xxx_packet_processing() usage Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-31 13:40 +0200
Re: [PATCH net-next 2/2] net: dsa: lan9303: Simplify lan9303_xxx_packet_processing() usage Andrew Lunn <andrew@lunn.ch> - 2017-07-31 15:50 +0200
Re: [PATCH net-next 2/2] net: dsa: lan9303: Simplify lan9303_xxx_packet_processing() usage Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-31 16:20 +0200
Re: [PATCH net-next 2/2] net: dsa: lan9303: Simplify lan9303_xxx_packet_processing() usage Andrew Lunn <andrew@lunn.ch> - 2017-07-31 16:50 +0200
Re: [PATCH net-next 2/2] net: dsa: lan9303: Simplify lan9303_xxx_packet_processing() usage Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-07-31 16:10 +0200
Re: [PATCH net-next 2/2] net: dsa: lan9303: Simplify lan9303_xxx_packet_processing() usage Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-31 16:40 +0200
Re: [PATCH net-next 2/2] net: dsa: lan9303: Simplify lan9303_xxx_packet_processing() usage Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-07-31 16:50 +0200
Re: [PATCH net-next 2/2] net: dsa: lan9303: Simplify lan9303_xxx_packet_processing() usage Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-31 17:00 +0200
Re: [PATCH net-next 2/2] net: dsa: lan9303: Simplify lan9303_xxx_packet_processing() usage Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-31 17:10 +0200
Re: [PATCH net-next 2/2] net: dsa: lan9303: Simplify lan9303_xxx_packet_processing() usage Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-07-31 17:20 +0200
Re: [PATCH net-next 2/2] net: dsa: lan9303: Simplify lan9303_xxx_packet_processing() usage Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-07-31 17:10 +0200
Re: [PATCH net-next 0/2] Refactor lan9303_xxx_packet_processing Andrew Lunn <andrew@lunn.ch> - 2017-07-31 15:30 +0200
Re: [PATCH net-next 0/2] Refactor lan9303_xxx_packet_processing Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-31 15:40 +0200
csiph-web