| Message ID | 20170803094507.3439-4-privat@egil-hjelmeland.no |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Thu, Aug 03, 2017 at 11:45:05AM +0200, Egil Hjelmeland wrote: > Simplify usage of lan9303_enable_packet_processing, > lan9303_disable_packet_processing() > > Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On 08/03/2017 02:45 AM, Egil Hjelmeland wrote: > Simplify usage of lan9303_enable_packet_processing, > lan9303_disable_packet_processing() > > Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> took a little while to figure out that we are utilizing fall through of the switch/case statement and that's why it's okay. > --- > drivers/net/dsa/lan9303-core.c | 24 ++++++++++-------------- > 1 file changed, 10 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c > index 126e8b84bdf0..31fe66fbe39a 100644 > --- a/drivers/net/dsa/lan9303-core.c > +++ b/drivers/net/dsa/lan9303-core.c > @@ -564,15 +564,16 @@ static int lan9303_handle_reset(struct lan9303 *chip) > /* stop processing packets for all ports */ > 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 < LAN9303_NUM_PORTS; p++) { > + int ret = lan9303_disable_packet_processing(chip, p); > + > + if (ret) > + return ret; > + } > + > + return 0; > } > > static int lan9303_check_device(struct lan9303 *chip) > @@ -765,7 +766,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); > case 2: > return lan9303_enable_packet_processing(chip, port); > default: > @@ -784,13 +784,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; > default: >
Den 03. aug. 2017 20:06, skrev Florian Fainelli: > On 08/03/2017 02:45 AM, Egil Hjelmeland wrote: >> Simplify usage of lan9303_enable_packet_processing, >> lan9303_disable_packet_processing() >> >> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no> > > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> > > took a little while to figure out that we are utilizing fall through of > the switch/case statement and that's why it's okay. > >> >> static int lan9303_check_device(struct lan9303 *chip) >> @@ -765,7 +766,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); >> case 2: >> return lan9303_enable_packet_processing(chip, port); >> default: I suppose if we later change to dsa_switch_alloc(...,3), then it could be further simplified to if (port != 0) return lan9303_enable_packet_processing(chip, port); Or perhaps no test is needed at all. The driver assumes port 0 is cpu port, which is the sensible way to use the chip. (Because port 0 has no phy, the others have phy). Declaring a different port as cpu port in DTS will not work, but it will not crash the kernel. Egil
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c index 126e8b84bdf0..31fe66fbe39a 100644 --- a/drivers/net/dsa/lan9303-core.c +++ b/drivers/net/dsa/lan9303-core.c @@ -564,15 +564,16 @@ static int lan9303_handle_reset(struct lan9303 *chip) /* stop processing packets for all ports */ 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 < LAN9303_NUM_PORTS; p++) { + int ret = lan9303_disable_packet_processing(chip, p); + + if (ret) + return ret; + } + + return 0; } static int lan9303_check_device(struct lan9303 *chip) @@ -765,7 +766,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); case 2: return lan9303_enable_packet_processing(chip, port); default: @@ -784,13 +784,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; default:
Simplify usage of lan9303_enable_packet_processing, lan9303_disable_packet_processing() Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no> --- drivers/net/dsa/lan9303-core.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-)