Re: [PATCH net-next v2 04/10] net: ethernet: oa_tc6: Export the C45 access functions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] Missing forward declaration for `struct mii_bus` in `include/linux/oa_tc6.h`.

Pre-existing issues:
- [High] `oa_tc6_mdiobus_read` silently masks negative error codes into `1` due to using a `bool` type for the return code variable.
--

commit 4bde583a95e265a961f05530eea079dbd8883081
Author: Ciprian Regus <ciprian.regus@analog.com>

net: ethernet: oa_tc6: Export the C45 access functions

The C45 access functions can still be used by some Ethernet drivers
which set the OA_TC6_BROKEN_PHY flag. Export them.

> diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
> index 7ae3639beaddd..4a1fd9fd75abe 100644
> --- a/drivers/net/ethernet/oa_tc6.c
> +++ b/drivers/net/ethernet/oa_tc6.c
> @@ -499,8 +499,8 @@ static int oa_tc6_get_phy_c45_mms(int devnum)
>  	}
>  }
>  
> -static int oa_tc6_mdiobus_read_c45(struct mii_bus *bus, int addr, int devnum,
> -				   int regnum)
> +int oa_tc6_mdiobus_read_c45(struct mii_bus *bus, int addr, int devnum,
> +			    int regnum)

[Severity: High]
This is a pre-existing issue, but looking closely at the related
oa_tc6_mdiobus_read() function in this file, it declares the return value
variable as a bool:

static int oa_tc6_mdiobus_read(struct mii_bus *bus, int addr, int regnum)
{
	...
	bool ret;

	ret = oa_tc6_read_register(...);
	if (ret)
		return ret;

Could this cause negative error codes like -EPROTO to be silently masked into
1, which the MDIO subsystem might treat as a successful read of 0x0001?

Should the type of ret be changed to int here?

[ ... ]

> diff --git a/include/linux/oa_tc6.h b/include/linux/oa_tc6.h
> index 6c38bf49e2a7e..95e041d7d77b6 100644
> --- a/include/linux/oa_tc6.h
> +++ b/include/linux/oa_tc6.h
> @@ -32,3 +32,7 @@ int oa_tc6_read_registers(struct oa_tc6 *tc6, u32 address, u32 value[],
>  			  u8 length);
>  netdev_tx_t oa_tc6_start_xmit(struct oa_tc6 *tc6, struct sk_buff *skb);
>  int oa_tc6_zero_align_receive_frame_enable(struct oa_tc6 *tc6);
> +int oa_tc6_mdiobus_read_c45(struct mii_bus *bus, int addr, int devnum,
> +			    int regnum);

[Severity: Low]
Does this header file need a forward declaration for struct mii_bus?

If another file includes linux/oa_tc6.h without first including mdio.h or
phy.h, this might cause compiler warnings or errors since struct mii_bus
has not been declared yet.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260527-adin1140-driver-v2-0-37e5c8d4e0a0@analog.com?part=4