* [PATCH -next] net: broadcom: Use helper function IS_ERR_OR_NULL()
@ 2023-08-16 9:53 Ruan Jinjie
2023-08-17 7:19 ` Leon Romanovsky
0 siblings, 1 reply; 5+ messages in thread
From: Ruan Jinjie @ 2023-08-16 9:53 UTC (permalink / raw
To: netdev, Rafał Miłecki,
Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Doug Berger,
Florian Fainelli
Cc: ruanjinjie
Use IS_ERR_OR_NULL() instead of open-coding it
to simplify the code.
Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com>
---
drivers/net/ethernet/broadcom/bgmac.c | 2 +-
drivers/net/ethernet/broadcom/genet/bcmmii.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 10c7c232cc4e..4cd7c6abb548 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1448,7 +1448,7 @@ int bgmac_phy_connect_direct(struct bgmac *bgmac)
int err;
phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, NULL);
- if (!phy_dev || IS_ERR(phy_dev)) {
+ if (IS_ERR_OR_NULL(phy_dev)) {
dev_err(bgmac->dev, "Failed to register fixed PHY device\n");
return -ENODEV;
}
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 0092e46c46f8..aa9a436fb3ce 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -617,7 +617,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)) {
+ if (IS_ERR_OR_NULL(phydev)) {
dev_err(kdev, "failed to register fixed PHY device\n");
return -ENODEV;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH -next] net: broadcom: Use helper function IS_ERR_OR_NULL()
2023-08-16 9:53 [PATCH -next] net: broadcom: Use helper function IS_ERR_OR_NULL() Ruan Jinjie
@ 2023-08-17 7:19 ` Leon Romanovsky
2023-08-17 7:25 ` Ruan Jinjie
2023-08-17 9:01 ` Ruan Jinjie
0 siblings, 2 replies; 5+ messages in thread
From: Leon Romanovsky @ 2023-08-17 7:19 UTC (permalink / raw
To: Ruan Jinjie
Cc: netdev, Rafał Miłecki,
Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Doug Berger,
Florian Fainelli
On Wed, Aug 16, 2023 at 05:53:56PM +0800, Ruan Jinjie wrote:
> Use IS_ERR_OR_NULL() instead of open-coding it
> to simplify the code.
>
> Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com>
> ---
> drivers/net/ethernet/broadcom/bgmac.c | 2 +-
> drivers/net/ethernet/broadcom/genet/bcmmii.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> index 10c7c232cc4e..4cd7c6abb548 100644
> --- a/drivers/net/ethernet/broadcom/bgmac.c
> +++ b/drivers/net/ethernet/broadcom/bgmac.c
> @@ -1448,7 +1448,7 @@ int bgmac_phy_connect_direct(struct bgmac *bgmac)
> int err;
>
> phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, NULL);
When can fixed_phy_register() return NULL?
It looks like it returns or valid phy_dev or ERR_PTR().
Thanks
> - if (!phy_dev || IS_ERR(phy_dev)) {
> + if (IS_ERR_OR_NULL(phy_dev)) {
> dev_err(bgmac->dev, "Failed to register fixed PHY device\n");
> return -ENODEV;
> }
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
> index 0092e46c46f8..aa9a436fb3ce 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
> @@ -617,7 +617,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)) {
> + if (IS_ERR_OR_NULL(phydev)) {
> dev_err(kdev, "failed to register fixed PHY device\n");
> return -ENODEV;
> }
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH -next] net: broadcom: Use helper function IS_ERR_OR_NULL()
2023-08-17 7:19 ` Leon Romanovsky
@ 2023-08-17 7:25 ` Ruan Jinjie
2023-08-17 9:01 ` Ruan Jinjie
1 sibling, 0 replies; 5+ messages in thread
From: Ruan Jinjie @ 2023-08-17 7:25 UTC (permalink / raw
To: Leon Romanovsky
Cc: netdev, Rafał Miłecki,
Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Doug Berger,
Florian Fainelli
On 2023/8/17 15:19, Leon Romanovsky wrote:
> On Wed, Aug 16, 2023 at 05:53:56PM +0800, Ruan Jinjie wrote:
>> Use IS_ERR_OR_NULL() instead of open-coding it
>> to simplify the code.
>>
>> Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com>
>> ---
>> drivers/net/ethernet/broadcom/bgmac.c | 2 +-
>> drivers/net/ethernet/broadcom/genet/bcmmii.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
>> index 10c7c232cc4e..4cd7c6abb548 100644
>> --- a/drivers/net/ethernet/broadcom/bgmac.c
>> +++ b/drivers/net/ethernet/broadcom/bgmac.c
>> @@ -1448,7 +1448,7 @@ int bgmac_phy_connect_direct(struct bgmac *bgmac)
>> int err;
>>
>> phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, NULL);
>
> When can fixed_phy_register() return NULL?
> It looks like it returns or valid phy_dev or ERR_PTR().
Right! All the returns of fixed_phy_register() are either ERR_PTR() or
the valid phy_device. I'll fix the issue sooner. Thank you very much.
>
> Thanks
>
>
>> - if (!phy_dev || IS_ERR(phy_dev)) {
>> + if (IS_ERR_OR_NULL(phy_dev)) {
>> dev_err(bgmac->dev, "Failed to register fixed PHY device\n");
>> return -ENODEV;
>> }
>> diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
>> index 0092e46c46f8..aa9a436fb3ce 100644
>> --- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
>> +++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
>> @@ -617,7 +617,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)) {
>> + if (IS_ERR_OR_NULL(phydev)) {
>> dev_err(kdev, "failed to register fixed PHY device\n");
>> return -ENODEV;
>> }
>> --
>> 2.34.1
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH -next] net: broadcom: Use helper function IS_ERR_OR_NULL()
2023-08-17 7:19 ` Leon Romanovsky
2023-08-17 7:25 ` Ruan Jinjie
@ 2023-08-17 9:01 ` Ruan Jinjie
2023-08-18 18:41 ` Leon Romanovsky
1 sibling, 1 reply; 5+ messages in thread
From: Ruan Jinjie @ 2023-08-17 9:01 UTC (permalink / raw
To: Leon Romanovsky
Cc: netdev, Rafał Miłecki,
Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Doug Berger,
Florian Fainelli
On 2023/8/17 15:19, Leon Romanovsky wrote:
> On Wed, Aug 16, 2023 at 05:53:56PM +0800, Ruan Jinjie wrote:
>> Use IS_ERR_OR_NULL() instead of open-coding it
>> to simplify the code.
>>
>> Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com>
>> ---
>> drivers/net/ethernet/broadcom/bgmac.c | 2 +-
>> drivers/net/ethernet/broadcom/genet/bcmmii.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
>> index 10c7c232cc4e..4cd7c6abb548 100644
>> --- a/drivers/net/ethernet/broadcom/bgmac.c
>> +++ b/drivers/net/ethernet/broadcom/bgmac.c
>> @@ -1448,7 +1448,7 @@ int bgmac_phy_connect_direct(struct bgmac *bgmac)
>> int err;
>>
>> phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, NULL);
>
> When can fixed_phy_register() return NULL?
> It looks like it returns or valid phy_dev or ERR_PTR().
It seems the following code has a problem:
226 static struct phy_device *__fixed_phy_register(unsigned int irq,
227 struct fixed_phy_status *status,
228 struct device_node *np,
229 struct gpio_desc *gpiod)
230 {
231 struct fixed_mdio_bus *fmb = &platform_fmb;
232 struct phy_device *phy;
233 int phy_addr;
234 int ret;
235
236 if (!fmb->mii_bus || fmb->mii_bus->state != MDIOBUS_REGISTERED)
237 return ERR_PTR(-EPROBE_DEFER);
238
239 /* Check if we have a GPIO associated with this fixed phy */
240 if (!gpiod) {
241 gpiod = fixed_phy_get_gpiod(np);
242 if (IS_ERR(gpiod))
243 return ERR_CAST(gpiod);
244 }
fixed_phy_get_gpiod() return valid gpio_desc or NULL.If
fixed_phy_get_gpiod(np) failed, the error can not be returned with
IS_ERR(gpiod) is true, so the 243 line code is a dead code.
>
> Thanks
>
>
>> - if (!phy_dev || IS_ERR(phy_dev)) {
>> + if (IS_ERR_OR_NULL(phy_dev)) {
>> dev_err(bgmac->dev, "Failed to register fixed PHY device\n");
>> return -ENODEV;
>> }
>> diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
>> index 0092e46c46f8..aa9a436fb3ce 100644
>> --- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
>> +++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
>> @@ -617,7 +617,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)) {
>> + if (IS_ERR_OR_NULL(phydev)) {
>> dev_err(kdev, "failed to register fixed PHY device\n");
>> return -ENODEV;
>> }
>> --
>> 2.34.1
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH -next] net: broadcom: Use helper function IS_ERR_OR_NULL()
2023-08-17 9:01 ` Ruan Jinjie
@ 2023-08-18 18:41 ` Leon Romanovsky
0 siblings, 0 replies; 5+ messages in thread
From: Leon Romanovsky @ 2023-08-18 18:41 UTC (permalink / raw
To: Ruan Jinjie
Cc: netdev, Rafał Miłecki,
Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Doug Berger,
Florian Fainelli
On Thu, Aug 17, 2023 at 05:01:51PM +0800, Ruan Jinjie wrote:
>
>
> On 2023/8/17 15:19, Leon Romanovsky wrote:
> > On Wed, Aug 16, 2023 at 05:53:56PM +0800, Ruan Jinjie wrote:
> >> Use IS_ERR_OR_NULL() instead of open-coding it
> >> to simplify the code.
> >>
> >> Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com>
> >> ---
> >> drivers/net/ethernet/broadcom/bgmac.c | 2 +-
> >> drivers/net/ethernet/broadcom/genet/bcmmii.c | 2 +-
> >> 2 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> >> index 10c7c232cc4e..4cd7c6abb548 100644
> >> --- a/drivers/net/ethernet/broadcom/bgmac.c
> >> +++ b/drivers/net/ethernet/broadcom/bgmac.c
> >> @@ -1448,7 +1448,7 @@ int bgmac_phy_connect_direct(struct bgmac *bgmac)
> >> int err;
> >>
> >> phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, NULL);
> >
> > When can fixed_phy_register() return NULL?
> > It looks like it returns or valid phy_dev or ERR_PTR().
>
> It seems the following code has a problem:
>
> 226 static struct phy_device *__fixed_phy_register(unsigned int irq,
> 227 struct fixed_phy_status *status,
> 228 struct device_node *np,
> 229 struct gpio_desc *gpiod)
> 230 {
> 231 struct fixed_mdio_bus *fmb = &platform_fmb;
> 232 struct phy_device *phy;
> 233 int phy_addr;
> 234 int ret;
> 235
> 236 if (!fmb->mii_bus || fmb->mii_bus->state != MDIOBUS_REGISTERED)
> 237 return ERR_PTR(-EPROBE_DEFER);
> 238
> 239 /* Check if we have a GPIO associated with this fixed phy */
> 240 if (!gpiod) {
> 241 gpiod = fixed_phy_get_gpiod(np);
> 242 if (IS_ERR(gpiod))
> 243 return ERR_CAST(gpiod);
> 244 }
>
> fixed_phy_get_gpiod() return valid gpio_desc or NULL.If
> fixed_phy_get_gpiod(np) failed, the error can not be returned with
> IS_ERR(gpiod) is true, so the 243 line code is a dead code.
Not really, gpiod can be ERR_PTR(-EPROBE_DEFER) too.
Thanks
>
> >
> > Thanks
> >
> >
> >> - if (!phy_dev || IS_ERR(phy_dev)) {
> >> + if (IS_ERR_OR_NULL(phy_dev)) {
> >> dev_err(bgmac->dev, "Failed to register fixed PHY device\n");
> >> return -ENODEV;
> >> }
> >> diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
> >> index 0092e46c46f8..aa9a436fb3ce 100644
> >> --- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
> >> +++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
> >> @@ -617,7 +617,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)) {
> >> + if (IS_ERR_OR_NULL(phydev)) {
> >> dev_err(kdev, "failed to register fixed PHY device\n");
> >> return -ENODEV;
> >> }
> >> --
> >> 2.34.1
> >>
> >>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-08-18 18:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-16 9:53 [PATCH -next] net: broadcom: Use helper function IS_ERR_OR_NULL() Ruan Jinjie
2023-08-17 7:19 ` Leon Romanovsky
2023-08-17 7:25 ` Ruan Jinjie
2023-08-17 9:01 ` Ruan Jinjie
2023-08-18 18:41 ` Leon Romanovsky
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.