Message ID | 20220729130346.2961889-5-o.rempel@pengutronix.de (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | net: dsa: microchip: add error handling and register access validation | expand |
Context | Check | Description |
---|---|---|
netdev/tree_selection | success | Clearly marked for net-next |
netdev/fixes_present | success | Fixes tag not required for -next series |
netdev/subject_prefix | success | Link |
netdev/cover_letter | success | Series has a cover letter |
netdev/patch_count | success | Link |
netdev/header_inline | success | No static functions without inline keyword in header files |
netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
netdev/cc_maintainers | success | CCed 11 of 11 maintainers |
netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
netdev/module_param | success | Was 0 now: 0 |
netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
netdev/check_selftest | success | No net selftest shell script |
netdev/verify_fixes | success | No Fixes tag |
netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
netdev/checkpatch | warning | WARNING: ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP |
netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
netdev/source_inline | success | Was 0 now: 0 |
On Fri, Jul 29, 2022 at 03:03:40PM +0200, Oleksij Rempel wrote: > } else { > - ksz_pread16(dev, addr, 0x100 + (reg << 1), &val); > + ret = ksz_pread16(dev, addr, 0x100 + (reg << 1), &val); > + if (ret) > + return ret; > + > ksz9477_r_phy_quirks(dev, addr, reg, &val); > } > > @@ -340,11 +344,9 @@ int ksz9477_w_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 val) > > /* No gigabit support. Do not write to this register. */ > if (!(dev->features & GBIT_SUPPORT) && reg == MII_CTRL1000) > - return 0; > + return -ENOTSUPP; I wonder if ENOTSUPP is the most appropriate error code, given that I see it defined under a comment "Defined for the NFSv3 protocol". How about -ENXIO? > > - ksz_pwrite16(dev, addr, 0x100 + (reg << 1), val); > - > - return 0; > + return ksz_pwrite16(dev, addr, 0x100 + (reg << 1), val); > } > > void ksz9477_cfg_port_member(struct ksz_device *dev, int port, u8 member) > -- > 2.30.2 >
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c index 9d749537cf01..074d4c16d427 100644 --- a/drivers/net/dsa/microchip/ksz9477.c +++ b/drivers/net/dsa/microchip/ksz9477.c @@ -283,6 +283,7 @@ static void ksz9477_r_phy_quirks(struct ksz_device *dev, u16 addr, u16 reg, int ksz9477_r_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 *data) { u16 val = 0xffff; + int ret; /* No real PHY after this. Simulate the PHY. * A fixed PHY can be setup in the device tree, but this function is @@ -323,7 +324,10 @@ int ksz9477_r_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 *data) break; } } else { - ksz_pread16(dev, addr, 0x100 + (reg << 1), &val); + ret = ksz_pread16(dev, addr, 0x100 + (reg << 1), &val); + if (ret) + return ret; + ksz9477_r_phy_quirks(dev, addr, reg, &val); } @@ -340,11 +344,9 @@ int ksz9477_w_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 val) /* No gigabit support. Do not write to this register. */ if (!(dev->features & GBIT_SUPPORT) && reg == MII_CTRL1000) - return 0; + return -ENOTSUPP; - ksz_pwrite16(dev, addr, 0x100 + (reg << 1), val); - - return 0; + return ksz_pwrite16(dev, addr, 0x100 + (reg << 1), val); } void ksz9477_cfg_port_member(struct ksz_device *dev, int port, u8 member)
Now ksz_pread/ksz_pwrite can return error value. So, make use of it. Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- drivers/net/dsa/microchip/ksz9477.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)