Commit fe893027 authored by David S. Miller's avatar David S. Miller

Merge branch 'dsa-mv88e6xxx-Improve-indirect-addressing-performance'

Tobias Waldekranz says:

====================
net: dsa: mv88e6xxx: Improve indirect addressing performance

The individual patches have all the details. This work was triggered
by recent work on a platform that took 16s (sic) to load the mv88e6xxx
module.

The first patch gets rid of most of that time by replacing a very long
delay with a tighter poll loop to wait for the busy bit to clear.

The second patch shaves off some more time by avoiding redundant
busy-bit-checks, saving 1 out of 4 MDIO operations for every register
read/write in the optimal case.

v1 -> v2:
- Make sure that we always poll the busy bit at least twice, in the
  unlikely event that the first one is quick to query the hardware,
  but is then scheduled out for a long time before the timeout is
  checked.

v2 -> v3:
- Fallback to the longer sleeps after the initial two poll attempts.
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 0da8aa00 7bca16b2
...@@ -86,12 +86,16 @@ int mv88e6xxx_write(struct mv88e6xxx_chip *chip, int addr, int reg, u16 val) ...@@ -86,12 +86,16 @@ int mv88e6xxx_write(struct mv88e6xxx_chip *chip, int addr, int reg, u16 val)
int mv88e6xxx_wait_mask(struct mv88e6xxx_chip *chip, int addr, int reg, int mv88e6xxx_wait_mask(struct mv88e6xxx_chip *chip, int addr, int reg,
u16 mask, u16 val) u16 mask, u16 val)
{ {
const unsigned long timeout = jiffies + msecs_to_jiffies(50);
u16 data; u16 data;
int err; int err;
int i; int i;
/* There's no bus specific operation to wait for a mask */ /* There's no bus specific operation to wait for a mask. Even
for (i = 0; i < 16; i++) { * if the initial poll takes longer than 50ms, always do at
* least one more attempt.
*/
for (i = 0; time_before(jiffies, timeout) || (i < 2); i++) {
err = mv88e6xxx_read(chip, addr, reg, &data); err = mv88e6xxx_read(chip, addr, reg, &data);
if (err) if (err)
return err; return err;
...@@ -99,7 +103,10 @@ int mv88e6xxx_wait_mask(struct mv88e6xxx_chip *chip, int addr, int reg, ...@@ -99,7 +103,10 @@ int mv88e6xxx_wait_mask(struct mv88e6xxx_chip *chip, int addr, int reg,
if ((data & mask) == val) if ((data & mask) == val)
return 0; return 0;
usleep_range(1000, 2000); if (i < 2)
cpu_relax();
else
usleep_range(1000, 2000);
} }
dev_err(chip->dev, "Timeout while waiting for switch\n"); dev_err(chip->dev, "Timeout while waiting for switch\n");
......
...@@ -392,6 +392,7 @@ struct mv88e6xxx_chip { ...@@ -392,6 +392,7 @@ struct mv88e6xxx_chip {
struct mv88e6xxx_bus_ops { struct mv88e6xxx_bus_ops {
int (*read)(struct mv88e6xxx_chip *chip, int addr, int reg, u16 *val); int (*read)(struct mv88e6xxx_chip *chip, int addr, int reg, u16 *val);
int (*write)(struct mv88e6xxx_chip *chip, int addr, int reg, u16 val); int (*write)(struct mv88e6xxx_chip *chip, int addr, int reg, u16 val);
int (*init)(struct mv88e6xxx_chip *chip);
}; };
struct mv88e6xxx_mdio_bus { struct mv88e6xxx_mdio_bus {
......
...@@ -55,11 +55,15 @@ static int mv88e6xxx_smi_direct_write(struct mv88e6xxx_chip *chip, ...@@ -55,11 +55,15 @@ static int mv88e6xxx_smi_direct_write(struct mv88e6xxx_chip *chip,
static int mv88e6xxx_smi_direct_wait(struct mv88e6xxx_chip *chip, static int mv88e6xxx_smi_direct_wait(struct mv88e6xxx_chip *chip,
int dev, int reg, int bit, int val) int dev, int reg, int bit, int val)
{ {
const unsigned long timeout = jiffies + msecs_to_jiffies(50);
u16 data; u16 data;
int err; int err;
int i; int i;
for (i = 0; i < 16; i++) { /* Even if the initial poll takes longer than 50ms, always do
* at least one more attempt.
*/
for (i = 0; time_before(jiffies, timeout) || (i < 2); i++) {
err = mv88e6xxx_smi_direct_read(chip, dev, reg, &data); err = mv88e6xxx_smi_direct_read(chip, dev, reg, &data);
if (err) if (err)
return err; return err;
...@@ -67,7 +71,10 @@ static int mv88e6xxx_smi_direct_wait(struct mv88e6xxx_chip *chip, ...@@ -67,7 +71,10 @@ static int mv88e6xxx_smi_direct_wait(struct mv88e6xxx_chip *chip,
if (!!(data & BIT(bit)) == !!val) if (!!(data & BIT(bit)) == !!val)
return 0; return 0;
usleep_range(1000, 2000); if (i < 2)
cpu_relax();
else
usleep_range(1000, 2000);
} }
return -ETIMEDOUT; return -ETIMEDOUT;
...@@ -104,11 +111,6 @@ static int mv88e6xxx_smi_indirect_read(struct mv88e6xxx_chip *chip, ...@@ -104,11 +111,6 @@ static int mv88e6xxx_smi_indirect_read(struct mv88e6xxx_chip *chip,
{ {
int err; int err;
err = mv88e6xxx_smi_direct_wait(chip, chip->sw_addr,
MV88E6XXX_SMI_CMD, 15, 0);
if (err)
return err;
err = mv88e6xxx_smi_direct_write(chip, chip->sw_addr, err = mv88e6xxx_smi_direct_write(chip, chip->sw_addr,
MV88E6XXX_SMI_CMD, MV88E6XXX_SMI_CMD,
MV88E6XXX_SMI_CMD_BUSY | MV88E6XXX_SMI_CMD_BUSY |
...@@ -132,11 +134,6 @@ static int mv88e6xxx_smi_indirect_write(struct mv88e6xxx_chip *chip, ...@@ -132,11 +134,6 @@ static int mv88e6xxx_smi_indirect_write(struct mv88e6xxx_chip *chip,
{ {
int err; int err;
err = mv88e6xxx_smi_direct_wait(chip, chip->sw_addr,
MV88E6XXX_SMI_CMD, 15, 0);
if (err)
return err;
err = mv88e6xxx_smi_direct_write(chip, chip->sw_addr, err = mv88e6xxx_smi_direct_write(chip, chip->sw_addr,
MV88E6XXX_SMI_DATA, data); MV88E6XXX_SMI_DATA, data);
if (err) if (err)
...@@ -155,9 +152,20 @@ static int mv88e6xxx_smi_indirect_write(struct mv88e6xxx_chip *chip, ...@@ -155,9 +152,20 @@ static int mv88e6xxx_smi_indirect_write(struct mv88e6xxx_chip *chip,
MV88E6XXX_SMI_CMD, 15, 0); MV88E6XXX_SMI_CMD, 15, 0);
} }
static int mv88e6xxx_smi_indirect_init(struct mv88e6xxx_chip *chip)
{
/* Ensure that the chip starts out in the ready state. As both
* reads and writes always ensure this on return, they can
* safely depend on the chip not being busy on entry.
*/
return mv88e6xxx_smi_direct_wait(chip, chip->sw_addr,
MV88E6XXX_SMI_CMD, 15, 0);
}
static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_indirect_ops = { static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_indirect_ops = {
.read = mv88e6xxx_smi_indirect_read, .read = mv88e6xxx_smi_indirect_read,
.write = mv88e6xxx_smi_indirect_write, .write = mv88e6xxx_smi_indirect_write,
.init = mv88e6xxx_smi_indirect_init,
}; };
int mv88e6xxx_smi_init(struct mv88e6xxx_chip *chip, int mv88e6xxx_smi_init(struct mv88e6xxx_chip *chip,
...@@ -175,5 +183,8 @@ int mv88e6xxx_smi_init(struct mv88e6xxx_chip *chip, ...@@ -175,5 +183,8 @@ int mv88e6xxx_smi_init(struct mv88e6xxx_chip *chip,
chip->bus = bus; chip->bus = bus;
chip->sw_addr = sw_addr; chip->sw_addr = sw_addr;
if (chip->smi_ops->init)
return chip->smi_ops->init(chip);
return 0; return 0;
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment