* [PATCH 1/2] miiphyutil: Fix inconsistent miiphy_write() error return value @ 2022-02-09 15:32 Daniel Klauer 2022-02-09 15:32 ` [PATCH 2/2] net: phy: mv88e6352: Fix miiphy_read/miiphy_write return value checks Daniel Klauer 0 siblings, 1 reply; 6+ messages in thread From: Daniel Klauer @ 2022-02-09 15:32 UTC (permalink / raw) To: u-boot miiphy_write() should not directly return the error return value from bus->write(), because that is typically a -errno value, while generally miiphy_write() and other miiphy_*() functions return 1 on error. Some miiphy_write() callers only check for > 0 to detect errors. Fix it to match miiphy_read(), which also converts bus->read() failure to "return 1". Signed-off-by: Daniel Klauer <daniel.klauer@gin.de> --- common/miiphyutil.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/common/miiphyutil.c b/common/miiphyutil.c index 7d4d15ed91..86a27665e3 100644 --- a/common/miiphyutil.c +++ b/common/miiphyutil.c @@ -236,7 +236,7 @@ static struct mii_dev *miiphy_get_active_dev(const char *devname) * This API is deprecated. Use phy_read on a phy_device found via phy_connect * * Returns: - * 0 on success + * 0 on success, 1 on error */ int miiphy_read(const char *devname, unsigned char addr, unsigned char reg, unsigned short *value) @@ -264,18 +264,23 @@ int miiphy_read(const char *devname, unsigned char addr, unsigned char reg, * This API is deprecated. Use phy_write on a phy_device found by phy_connect * * Returns: - * 0 on success + * 0 on success, 1 on error */ int miiphy_write(const char *devname, unsigned char addr, unsigned char reg, unsigned short value) { struct mii_dev *bus; + int ret; bus = miiphy_get_active_dev(devname); - if (bus) - return bus->write(bus, addr, MDIO_DEVAD_NONE, reg, value); + if (!bus) + return 1; - return 1; + ret = bus->write(bus, addr, MDIO_DEVAD_NONE, reg, value); + if (ret < 0) + return 1; + + return 0; } /***************************************************************************** -- 2.32.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] net: phy: mv88e6352: Fix miiphy_read/miiphy_write return value checks 2022-02-09 15:32 [PATCH 1/2] miiphyutil: Fix inconsistent miiphy_write() error return value Daniel Klauer @ 2022-02-09 15:32 ` Daniel Klauer 2022-02-12 11:50 ` Ramon Fried 0 siblings, 1 reply; 6+ messages in thread From: Daniel Klauer @ 2022-02-09 15:32 UTC (permalink / raw) To: u-boot The miiphy_read/miiphy_write functions return 1 on error, not -errno. Fix up the checks accordingly and insert -EIO as fallback error code. Signed-off-by: Daniel Klauer <daniel.klauer@gin.de> --- drivers/net/phy/mv88e6352.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/drivers/net/phy/mv88e6352.c b/drivers/net/phy/mv88e6352.c index 56060762d8..a87af7ed24 100644 --- a/drivers/net/phy/mv88e6352.c +++ b/drivers/net/phy/mv88e6352.c @@ -36,16 +36,14 @@ static int sw_wait_rdy(const char *devname, u8 phy_addr) { u16 command; u32 timeout = 100; - int ret; /* wait till the SMI is not busy */ do { /* read command register */ - ret = miiphy_read(devname, phy_addr, COMMAND_REG, &command); - if (ret < 0) { + if (miiphy_read(devname, phy_addr, COMMAND_REG, &command)) { printf("%s: Error reading command register\n", __func__); - return ret; + return -EIO; } if (timeout-- == 0) { printf("Err..(%s) SMI busy timeout\n", __func__); @@ -69,17 +67,17 @@ static int sw_reg_read(const char *devname, u8 phy_addr, u8 port, command = SMI_HDR | SMIRD_OP | ((port&SMI_MASK) << PORT_SHIFT) | (reg & SMI_MASK); debug("%s: write to command: %#x\n", __func__, command); - ret = miiphy_write(devname, phy_addr, COMMAND_REG, command); - if (ret) - return ret; + if (miiphy_write(devname, phy_addr, COMMAND_REG, command)) + return -EIO; ret = sw_wait_rdy(devname, phy_addr); if (ret) return ret; - ret = miiphy_read(devname, phy_addr, DATA_REG, data); + if (miiphy_read(devname, phy_addr, DATA_REG, data)) + return -EIO; - return ret; + return 0; } static int sw_reg_write(const char *devname, u8 phy_addr, u8 port, @@ -93,16 +91,14 @@ static int sw_reg_write(const char *devname, u8 phy_addr, u8 port, return ret; debug("%s: write to data: %#x\n", __func__, data); - ret = miiphy_write(devname, phy_addr, DATA_REG, data); - if (ret) - return ret; + if (miiphy_write(devname, phy_addr, DATA_REG, data)) + return -EIO; value = SMI_HDR | SMIWR_OP | ((port & SMI_MASK) << PORT_SHIFT) | (reg & SMI_MASK); debug("%s: write to command: %#x\n", __func__, value); - ret = miiphy_write(devname, phy_addr, COMMAND_REG, value); - if (ret) - return ret; + if (miiphy_write(devname, phy_addr, COMMAND_REG, value)) + return -EIO; ret = sw_wait_rdy(devname, phy_addr); if (ret) -- 2.32.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] net: phy: mv88e6352: Fix miiphy_read/miiphy_write return value checks 2022-02-09 15:32 ` [PATCH 2/2] net: phy: mv88e6352: Fix miiphy_read/miiphy_write return value checks Daniel Klauer @ 2022-02-12 11:50 ` Ramon Fried 2022-02-14 10:33 ` Daniel Klauer 0 siblings, 1 reply; 6+ messages in thread From: Ramon Fried @ 2022-02-12 11:50 UTC (permalink / raw) To: Daniel Klauer; +Cc: U-Boot Mailing List On Wed, Feb 9, 2022 at 5:41 PM Daniel Klauer <daniel.klauer@gin.de> wrote: > > The miiphy_read/miiphy_write functions return 1 on error, not -errno. Why don't you just fix the miiphy_read/miiphy_write functions ? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] net: phy: mv88e6352: Fix miiphy_read/miiphy_write return value checks 2022-02-12 11:50 ` Ramon Fried @ 2022-02-14 10:33 ` Daniel Klauer 2022-02-15 7:08 ` Ramon Fried 0 siblings, 1 reply; 6+ messages in thread From: Daniel Klauer @ 2022-02-14 10:33 UTC (permalink / raw) To: Ramon Fried; +Cc: U-Boot Mailing List On 12.02.22 12:50, Ramon Fried wrote: > On Wed, Feb 9, 2022 at 5:41 PM Daniel Klauer <daniel.klauer@gin.de> wrote: >> >> The miiphy_read/miiphy_write functions return 1 on error, not -errno. > Why don't you just fix the miiphy_read/miiphy_write functions ? Other functions from that module do the same, so I assumed it's intentional. It could be fixed too though, with a corresponding fix up of the few callers that expect the positive return value on error. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] net: phy: mv88e6352: Fix miiphy_read/miiphy_write return value checks 2022-02-14 10:33 ` Daniel Klauer @ 2022-02-15 7:08 ` Ramon Fried 2022-02-16 10:52 ` Daniel Klauer 0 siblings, 1 reply; 6+ messages in thread From: Ramon Fried @ 2022-02-15 7:08 UTC (permalink / raw) To: Daniel Klauer; +Cc: U-Boot Mailing List On Mon, Feb 14, 2022 at 12:33 PM Daniel Klauer <daniel.klauer@gin.de> wrote: > > On 12.02.22 12:50, Ramon Fried wrote: > > On Wed, Feb 9, 2022 at 5:41 PM Daniel Klauer <daniel.klauer@gin.de> wrote: > >> > >> The miiphy_read/miiphy_write functions return 1 on error, not -errno. > > Why don't you just fix the miiphy_read/miiphy_write functions ? > > Other functions from that module do the same, so I assumed it's intentional. > It could be fixed too though, with a corresponding fix up of the few callers > that expect the positive return value on error. Sometimes a real fix needs more work. What caused you to change it on your side to -EIO, is there someone checking for EIO in the framework code ? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] net: phy: mv88e6352: Fix miiphy_read/miiphy_write return value checks 2022-02-15 7:08 ` Ramon Fried @ 2022-02-16 10:52 ` Daniel Klauer 0 siblings, 0 replies; 6+ messages in thread From: Daniel Klauer @ 2022-02-16 10:52 UTC (permalink / raw) To: Ramon Fried; +Cc: U-Boot Mailing List On 15.02.22 08:08, Ramon Fried wrote: > On Mon, Feb 14, 2022 at 12:33 PM Daniel Klauer <daniel.klauer@gin.de> wrote: >> >> On 12.02.22 12:50, Ramon Fried wrote: >> > On Wed, Feb 9, 2022 at 5:41 PM Daniel Klauer <daniel.klauer@gin.de> wrote: >> >> >> >> The miiphy_read/miiphy_write functions return 1 on error, not -errno. >> > Why don't you just fix the miiphy_read/miiphy_write functions ? >> >> Other functions from that module do the same, so I assumed it's intentional. >> It could be fixed too though, with a corresponding fix up of the few callers >> that expect the positive return value on error. > Sometimes a real fix needs more work. > What caused you to change it on your side to -EIO, is there someone > checking for EIO in the framework code ? It's not that anything specifically expects -EIO, I just picked it because I had to pick something and it seemed like a good solution. Of course that only makes sense with the current miiphy_read/write functions which don't propagate the real errno. If that were to be changed, this patch for mv88e6352.c would not be useful anymore. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-16 10:53 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-02-09 15:32 [PATCH 1/2] miiphyutil: Fix inconsistent miiphy_write() error return value Daniel Klauer 2022-02-09 15:32 ` [PATCH 2/2] net: phy: mv88e6352: Fix miiphy_read/miiphy_write return value checks Daniel Klauer 2022-02-12 11:50 ` Ramon Fried 2022-02-14 10:33 ` Daniel Klauer 2022-02-15 7:08 ` Ramon Fried 2022-02-16 10:52 ` Daniel Klauer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox