* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.