lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <2d12cdb3-e6ef-46d2-3bfb-58b5c54f6ab3@huawei.com> Date: Mon, 21 Aug 2023 09:15:50 +0800 From: Ruan Jinjie <ruanjinjie@...wei.com> To: Simon Horman <horms@...nel.org> CC: <rafal@...ecki.pl>, <bcm-kernel-feedback-list@...adcom.com>, <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>, <opendmb@...il.com>, <florian.fainelli@...adcom.com>, <bryan.whitehead@...rochip.com>, <UNGLinuxDriver@...rochip.com>, <netdev@...r.kernel.org> Subject: Re: [PATCH net-next v3 2/3] net: bcmgenet: Return PTR_ERR() for fixed_phy_register() On 2023/8/20 1:10, Simon Horman wrote: > On Sat, Aug 19, 2023 at 07:06:15PM +0200, Simon Horman wrote: >> On Fri, Aug 18, 2023 at 03:07:06PM +0800, Ruan Jinjie wrote: >>> fixed_phy_register() returns -EPROBE_DEFER, -EINVAL and -EBUSY, >>> etc, in addition to -ENODEV. The Best practice is to return these >>> error codes with PTR_ERR(). >>> >>> Signed-off-by: Ruan Jinjie <ruanjinjie@...wei.com> >>> --- >>> v3: >>> - Split the return value check into another patch set. >>> - Update the commit title and message. >>> --- >>> drivers/net/ethernet/broadcom/genet/bcmmii.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c >>> index 0092e46c46f8..4012a141a229 100644 >>> --- a/drivers/net/ethernet/broadcom/genet/bcmmii.c >>> +++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c >>> @@ -619,7 +619,7 @@ static int bcmgenet_mii_pd_init(struct bcmgenet_priv *priv) >>> phydev = fixed_phy_register(PHY_POLL, &fphy_status, NULL); >>> if (!phydev || IS_ERR(phydev)) { >>> dev_err(kdev, "failed to register fixed PHY device\n"); >>> - return -ENODEV; >>> + return PTR_ERR(phydev); >> >> Hi Ruan, >> >> thanks for your patch. >> >> Perhaps I am missing something, but this doesn't seem right to me. >> In the case where phydev is NULL will return 0. >> But bcmgenet_mii_pd_init() also returns 0 on success. >> >> Perhaps this is better? >> >> if (!phydev || IS_ERR(phydev)) { >> dev_err(kdev, "failed to register fixed PHY device\n"); >> return physdev ? PTR_ERR(phydev) : -ENODEV; >> } >> >> I have a similar concern for patch 1/3 of this series. >> Patch 3/3 seems fine in this regard. > > Sorry for the noise. > > I now see that fixed_phy_register() never returns NULL, > and that condition is being removed by another patchset [1]. > > I'm fine with this, other than that I suspect your two series > conflict with each other. Thank you! I'll resend this patch to be consistent. > > [1] https://lore.kernel.org/all/20230818051221.3634844-1-ruanjinjie@huawei.com/ >
Powered by blists - more mailing lists