[PATCH v4 net-next 0/2] Fixed warnings

[PATCH v4 net-next 0/2] Fixed warnings

[Divya Koppera]

Fixed warnings related to PTR_ERR and initialization.

Divya Koppera (2):
  net: phy: micrel: Fixed error related to uninitialized symbol ret
  net: phy: micrel: Fix warn: passing zero to PTR_ERR

 drivers/net/phy/micrel.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

-- 
2.17.1

[PATCH v4 net-next 1/2] net: phy: micrel: Fixed error related to uninitialized symbol ret

[Divya Koppera]

Initialized return variable

Fixes Old smatch warnings:
drivers/net/phy/micrel.c:1750 ksz886x_cable_test_get_status() error:
uninitialized symbol 'ret'.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Fixes: 21b688dabecb ("net: phy: micrel: Cable Diag feature for lan8814 phy")
Signed-off-by: Divya Koppera <Divya.Koppera@microchip.com>
---
v3 -> v4:
- Split the patch for different warnings.

v1 -> v3:
- No changes
---
 drivers/net/phy/micrel.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 26ce0c5defcd..1bcdb828db56 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -2088,7 +2088,8 @@ static int ksz886x_cable_test_get_status(struct phy_device *phydev,
 	const struct kszphy_type *type = phydev->drv->driver_data;
 	unsigned long pair_mask = type->pair_mask;
 	int retries = 20;
-	int pair, ret;
+	int ret = 0;
+	int pair;
 
 	*finished = false;
 
-- 
2.17.1

[PATCH v4 net-next 2/2] net: phy: micrel: Fix warn: passing zero to PTR_ERR

[Divya Koppera]

Handle the NULL pointer case

Fixes New smatch warnings:
drivers/net/phy/micrel.c:2613 lan8814_ptp_probe_once() warn: passing zero to 'PTR_ERR'

vim +/PTR_ERR +2613 drivers/net/phy/micrel.c
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Fixes: ece19502834d ("net: phy: micrel: 1588 support for LAN8814 phy")
Signed-off-by: Divya Koppera <Divya.Koppera@microchip.com>
---
v3 -> v4:
- Split the patch for different warnings
- Renamed variable from shared_priv to shared.

v2 -> v3:
- Changed subject line from net to net-next
- Removed config check for ptp and clock configuration
  instead added null check for ptp_clock
- Fixed one more warning related to initialisaton.

v1 -> v2:
- Handled NULL pointer case
- Changed subject line with net-next to net
---
 drivers/net/phy/micrel.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 1bcdb828db56..0399f3830700 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -2971,12 +2971,13 @@ static int lan8814_config_intr(struct phy_device *phydev)
 
 static void lan8814_ptp_init(struct phy_device *phydev)
 {
+	struct lan8814_shared_priv *shared = phydev->shared->priv;
 	struct kszphy_priv *priv = phydev->priv;
 	struct kszphy_ptp_priv *ptp_priv = &priv->ptp_priv;
 	u32 temp;
 
-	if (!IS_ENABLED(CONFIG_PTP_1588_CLOCK) ||
-	    !IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING))
+	/* Check if PHC support is missing at the configuration level */
+	if (!shared->ptp_clock)
 		return;
 
 	lanphy_write_page_reg(phydev, 5, TSU_HARD_RESET, TSU_HARD_RESET_);
@@ -3017,10 +3018,6 @@ static int lan8814_ptp_probe_once(struct phy_device *phydev)
 {
 	struct lan8814_shared_priv *shared = phydev->shared->priv;
 
-	if (!IS_ENABLED(CONFIG_PTP_1588_CLOCK) ||
-	    !IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING))
-		return 0;
-
 	/* Initialise shared lock for clock*/
 	mutex_init(&shared->shared_lock);
 
@@ -3040,12 +3037,16 @@ static int lan8814_ptp_probe_once(struct phy_device *phydev)
 
 	shared->ptp_clock = ptp_clock_register(&shared->ptp_clock_info,
 					       &phydev->mdio.dev);
-	if (IS_ERR_OR_NULL(shared->ptp_clock)) {
+	if (IS_ERR(shared->ptp_clock)) {
 		phydev_err(phydev, "ptp_clock_register failed %lu\n",
 			   PTR_ERR(shared->ptp_clock));
 		return -EINVAL;
 	}
 
+	/* Check if PHC support is missing at the configuration level */
+	if (!shared->ptp_clock)
+		return 0;
+
 	phydev_dbg(phydev, "successfully registered ptp clock\n");
 
 	shared->phydev = phydev;
-- 
2.17.1

Re: [PATCH v4 net-next 1/2] net: phy: micrel: Fixed error related to uninitialized symbol ret

[Andrew Lunn]

On Mon, Dec 05, 2022 at 04:05:49PM +0530, Divya Koppera wrote:
> Initialized return variable
> 
> Fixes Old smatch warnings:
> drivers/net/phy/micrel.c:1750 ksz886x_cable_test_get_status() error:
> uninitialized symbol 'ret'.

I guess this patch has been rebased without the smatch warning being
changed because line 1750 in net-next/main is a blank line between two
functions.

> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Fixes: 21b688dabecb ("net: phy: micrel: Cable Diag feature for lan8814 phy")
> Signed-off-by: Divya Koppera <Divya.Koppera@microchip.com>

So once i looked in the correct place:

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH v4 net-next 2/2] net: phy: micrel: Fix warn: passing zero to PTR_ERR

[Andrew Lunn]

On Mon, Dec 05, 2022 at 04:05:50PM +0530, Divya Koppera wrote:
> Handle the NULL pointer case
> 
> Fixes New smatch warnings:
> drivers/net/phy/micrel.c:2613 lan8814_ptp_probe_once() warn: passing zero to 'PTR_ERR'
> 
> vim +/PTR_ERR +2613 drivers/net/phy/micrel.c
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Fixes: ece19502834d ("net: phy: micrel: 1588 support for LAN8814 phy")
> Signed-off-by: Divya Koppera <Divya.Koppera@microchip.com>
> ---
> v3 -> v4:
> - Split the patch for different warnings
> - Renamed variable from shared_priv to shared.
> 
> v2 -> v3:
> - Changed subject line from net to net-next
> - Removed config check for ptp and clock configuration
>   instead added null check for ptp_clock
> - Fixed one more warning related to initialisaton.
> 
> v1 -> v2:
> - Handled NULL pointer case
> - Changed subject line with net-next to net
> ---
>  drivers/net/phy/micrel.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 1bcdb828db56..0399f3830700 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -2971,12 +2971,13 @@ static int lan8814_config_intr(struct phy_device *phydev)
>  
>  static void lan8814_ptp_init(struct phy_device *phydev)
>  {
> +	struct lan8814_shared_priv *shared = phydev->shared->priv;
>  	struct kszphy_priv *priv = phydev->priv;
>  	struct kszphy_ptp_priv *ptp_priv = &priv->ptp_priv;
>  	u32 temp;
>  
> -	if (!IS_ENABLED(CONFIG_PTP_1588_CLOCK) ||
> -	    !IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING))
> +	/* Check if PHC support is missing at the configuration level */
> +	if (!shared->ptp_clock)
>  		return;

Can you somehow keep the IS_ENABLED() ? It gets evaluated at compile
time. The optimizer can see the function will always return here, and
all the code that follows is pointless, and so remove it. By turning
this into a runtime test, you have made the image bigger.

     Andrew

RE: [PATCH v4 net-next 2/2] net: phy: micrel: Fix warn: passing zero to PTR_ERR

[Divya.Koppera]

Hi Andrew,

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Monday, December 5, 2022 7:12 PM
> To: Divya Koppera - I30481 <Divya.Koppera@microchip.com>
> Cc: hkallweit1@gmail.com; linux@armlinux.org.uk; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> richardcochran@gmail.com; UNGLinuxDriver
> <UNGLinuxDriver@microchip.com>; Madhuri Sripada - I34878
> <Madhuri.Sripada@microchip.com>
> Subject: Re: [PATCH v4 net-next 2/2] net: phy: micrel: Fix warn: passing zero
> to PTR_ERR
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> On Mon, Dec 05, 2022 at 04:05:50PM +0530, Divya Koppera wrote:
> > Handle the NULL pointer case
> >
> > Fixes New smatch warnings:
> > drivers/net/phy/micrel.c:2613 lan8814_ptp_probe_once() warn: passing
> zero to 'PTR_ERR'
> >
> > vim +/PTR_ERR +2613 drivers/net/phy/micrel.c
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Fixes: ece19502834d ("net: phy: micrel: 1588 support for LAN8814 phy")
> > Signed-off-by: Divya Koppera <Divya.Koppera@microchip.com>
> > ---
> > v3 -> v4:
> > - Split the patch for different warnings
> > - Renamed variable from shared_priv to shared.
> >
> > v2 -> v3:
> > - Changed subject line from net to net-next
> > - Removed config check for ptp and clock configuration
> >   instead added null check for ptp_clock
> > - Fixed one more warning related to initialisaton.
> >
> > v1 -> v2:
> > - Handled NULL pointer case
> > - Changed subject line with net-next to net
> > ---
> >  drivers/net/phy/micrel.c | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c index
> > 1bcdb828db56..0399f3830700 100644
> > --- a/drivers/net/phy/micrel.c
> > +++ b/drivers/net/phy/micrel.c
> > @@ -2971,12 +2971,13 @@ static int lan8814_config_intr(struct
> > phy_device *phydev)
> >
> >  static void lan8814_ptp_init(struct phy_device *phydev)  {
> > +     struct lan8814_shared_priv *shared = phydev->shared->priv;
> >       struct kszphy_priv *priv = phydev->priv;
> >       struct kszphy_ptp_priv *ptp_priv = &priv->ptp_priv;
> >       u32 temp;
> >
> > -     if (!IS_ENABLED(CONFIG_PTP_1588_CLOCK) ||
> > -         !IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING))
> > +     /* Check if PHC support is missing at the configuration level */
> > +     if (!shared->ptp_clock)
> >               return;
> 
> Can you somehow keep the IS_ENABLED() ? It gets evaluated at compile time.
> The optimizer can see the function will always return here, and all the code
> that follows is pointless, and so remove it. By turning this into a runtime test,
> you have made the image bigger.
> 

Thanks, will change this.

>      Andrew
/Divya

RE: [PATCH v4 net-next 1/2] net: phy: micrel: Fixed error related to uninitialized symbol ret

[Divya.Koppera]

Hi Andrew,

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Monday, December 5, 2022 7:07 PM
> To: Divya Koppera - I30481 <Divya.Koppera@microchip.com>
> Cc: hkallweit1@gmail.com; linux@armlinux.org.uk; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> richardcochran@gmail.com; UNGLinuxDriver
> <UNGLinuxDriver@microchip.com>; Madhuri Sripada - I34878
> <Madhuri.Sripada@microchip.com>
> Subject: Re: [PATCH v4 net-next 1/2] net: phy: micrel: Fixed error related to
> uninitialized symbol ret
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> On Mon, Dec 05, 2022 at 04:05:49PM +0530, Divya Koppera wrote:
> > Initialized return variable
> >
> > Fixes Old smatch warnings:
> > drivers/net/phy/micrel.c:1750 ksz886x_cable_test_get_status() error:
> > uninitialized symbol 'ret'.
> 
> I guess this patch has been rebased without the smatch warning being
> changed because line 1750 in net-next/main is a blank line between two
> functions.
> 

Yes, rebased without fix.

> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Fixes: 21b688dabecb ("net: phy: micrel: Cable Diag feature for lan8814
> > phy")
> > Signed-off-by: Divya Koppera <Divya.Koppera@microchip.com>
> 
> So once i looked in the correct place:
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
>     Andrew