* [PATCH 0/2] fixes for yt8511 phy driver @ 2021-05-25 12:26 Peter Geis 2021-05-25 12:26 ` [PATCH 1/2] net: phy: fix yt8511 clang uninitialized variable warning Peter Geis 2021-05-25 12:26 ` [PATCH 2/2] net: phy: abort loading yt8511 driver in unsupported modes Peter Geis 0 siblings, 2 replies; 7+ messages in thread From: Peter Geis @ 2021-05-25 12:26 UTC (permalink / raw) To: Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller, Jakub Kicinski, Nathan Chancellor Cc: netdev, linux-kernel, clang-built-linux, Peter Geis The Intel clang bot caught a few uninitialized variables in the new Motorcomm driver. While investigating the issue, it was found that the driver would have unintended effects when used in an unsupported mode. Fixed the uninitialized ret variable and abort loading the driver in unsupported modes. Thank you to the Intel clang bot for catching these. Peter Geis (2): net: phy: fix yt8511 clang uninitialized variable warning net: phy: abort loading yt8511 driver in unsupported modes drivers/net/phy/motorcomm.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] net: phy: fix yt8511 clang uninitialized variable warning 2021-05-25 12:26 [PATCH 0/2] fixes for yt8511 phy driver Peter Geis @ 2021-05-25 12:26 ` Peter Geis 2021-05-25 13:01 ` Andrew Lunn 2021-05-25 12:26 ` [PATCH 2/2] net: phy: abort loading yt8511 driver in unsupported modes Peter Geis 1 sibling, 1 reply; 7+ messages in thread From: Peter Geis @ 2021-05-25 12:26 UTC (permalink / raw) To: Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller, Jakub Kicinski, Nathan Chancellor Cc: netdev, linux-kernel, clang-built-linux, Peter Geis, kernel test robot clang doesn't preinitialize variables. If phy_select_page failed and returned an error, phy_restore_page would be called with `ret` being uninitialized. Even though phy_restore_page won't use `ret` in this scenario, initialize `ret` to silence the warning. Fixes: b1b41c047f73 ("net: phy: add driver for Motorcomm yt8511 phy") Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Peter Geis <pgwipeout@gmail.com> --- drivers/net/phy/motorcomm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c index 796b68f4b499..5795f446c528 100644 --- a/drivers/net/phy/motorcomm.c +++ b/drivers/net/phy/motorcomm.c @@ -51,7 +51,7 @@ static int yt8511_write_page(struct phy_device *phydev, int page) static int yt8511_config_init(struct phy_device *phydev) { unsigned int ge, fe; - int ret, oldpage; + int oldpage, ret = 0; /* set clock mode to 125mhz */ oldpage = phy_select_page(phydev, YT8511_EXT_CLK_GATE); -- 2.25.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] net: phy: fix yt8511 clang uninitialized variable warning 2021-05-25 12:26 ` [PATCH 1/2] net: phy: fix yt8511 clang uninitialized variable warning Peter Geis @ 2021-05-25 13:01 ` Andrew Lunn 2021-05-25 13:11 ` Peter Geis 0 siblings, 1 reply; 7+ messages in thread From: Andrew Lunn @ 2021-05-25 13:01 UTC (permalink / raw) To: Peter Geis Cc: Heiner Kallweit, Russell King, David S . Miller, Jakub Kicinski, Nathan Chancellor, netdev, linux-kernel, clang-built-linux, kernel test robot On Tue, May 25, 2021 at 08:26:14AM -0400, Peter Geis wrote: > clang doesn't preinitialize variables. If phy_select_page failed and > returned an error, phy_restore_page would be called with `ret` being > uninitialized. > Even though phy_restore_page won't use `ret` in this scenario, > initialize `ret` to silence the warning. > > Fixes: b1b41c047f73 ("net: phy: add driver for Motorcomm yt8511 phy") > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Peter Geis <pgwipeout@gmail.com> > --- > drivers/net/phy/motorcomm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c > index 796b68f4b499..5795f446c528 100644 > --- a/drivers/net/phy/motorcomm.c > +++ b/drivers/net/phy/motorcomm.c > @@ -51,7 +51,7 @@ static int yt8511_write_page(struct phy_device *phydev, int page) > static int yt8511_config_init(struct phy_device *phydev) > { > unsigned int ge, fe; > - int ret, oldpage; > + int oldpage, ret = 0; Please keep to reverse Christmas tree. With that fixed: Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] net: phy: fix yt8511 clang uninitialized variable warning 2021-05-25 13:01 ` Andrew Lunn @ 2021-05-25 13:11 ` Peter Geis 2021-05-25 13:59 ` Andrew Lunn 0 siblings, 1 reply; 7+ messages in thread From: Peter Geis @ 2021-05-25 13:11 UTC (permalink / raw) To: Andrew Lunn Cc: Heiner Kallweit, Russell King, David S . Miller, Jakub Kicinski, Nathan Chancellor, Linux Kernel Network Developers, Linux Kernel Mailing List, clang-built-linux, kernel test robot On Tue, May 25, 2021 at 9:02 AM Andrew Lunn <andrew@lunn.ch> wrote: > > On Tue, May 25, 2021 at 08:26:14AM -0400, Peter Geis wrote: > > clang doesn't preinitialize variables. If phy_select_page failed and > > returned an error, phy_restore_page would be called with `ret` being > > uninitialized. > > Even though phy_restore_page won't use `ret` in this scenario, > > initialize `ret` to silence the warning. > > > > Fixes: b1b41c047f73 ("net: phy: add driver for Motorcomm yt8511 phy") > > Reported-by: kernel test robot <lkp@intel.com> > > Signed-off-by: Peter Geis <pgwipeout@gmail.com> > > --- > > drivers/net/phy/motorcomm.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c > > index 796b68f4b499..5795f446c528 100644 > > --- a/drivers/net/phy/motorcomm.c > > +++ b/drivers/net/phy/motorcomm.c > > @@ -51,7 +51,7 @@ static int yt8511_write_page(struct phy_device *phydev, int page) > > static int yt8511_config_init(struct phy_device *phydev) > > { > > unsigned int ge, fe; > > - int ret, oldpage; > > + int oldpage, ret = 0; > > Please keep to reverse Christmas tree. Ah, I missed that. Do you want a v2 or will it be fixed on application? > > With that fixed: > > Reviewed-by: Andrew Lunn <andrew@lunn.ch> > > Andrew Thanks! Peter ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] net: phy: fix yt8511 clang uninitialized variable warning 2021-05-25 13:11 ` Peter Geis @ 2021-05-25 13:59 ` Andrew Lunn 0 siblings, 0 replies; 7+ messages in thread From: Andrew Lunn @ 2021-05-25 13:59 UTC (permalink / raw) To: Peter Geis Cc: Heiner Kallweit, Russell King, David S . Miller, Jakub Kicinski, Nathan Chancellor, Linux Kernel Network Developers, Linux Kernel Mailing List, clang-built-linux, kernel test robot > Do you want a v2 or will it be fixed on application? A v2. And please remember to add the Reviewed-by tags. Andrew ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] net: phy: abort loading yt8511 driver in unsupported modes 2021-05-25 12:26 [PATCH 0/2] fixes for yt8511 phy driver Peter Geis 2021-05-25 12:26 ` [PATCH 1/2] net: phy: fix yt8511 clang uninitialized variable warning Peter Geis @ 2021-05-25 12:26 ` Peter Geis 2021-05-25 13:03 ` Andrew Lunn 1 sibling, 1 reply; 7+ messages in thread From: Peter Geis @ 2021-05-25 12:26 UTC (permalink / raw) To: Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller, Jakub Kicinski, Nathan Chancellor Cc: netdev, linux-kernel, clang-built-linux, Peter Geis, kernel test robot While investigating the clang `ge` uninitialized variable report, it was discovered the default switch would have unintended consequences. Due to the switch to __phy_modify, the driver would modify the ID values in the default scenario. Fix this by promoting the interface mode switch and aborting when the mode is not a supported RGMII mode. This prevents the `ge` and `fe` variables from ever being used uninitialized. Fixes: b1b41c047f73 ("net: phy: add driver for Motorcomm yt8511 phy") Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Peter Geis <pgwipeout@gmail.com> --- drivers/net/phy/motorcomm.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c index 5795f446c528..53178e978da7 100644 --- a/drivers/net/phy/motorcomm.c +++ b/drivers/net/phy/motorcomm.c @@ -53,15 +53,10 @@ static int yt8511_config_init(struct phy_device *phydev) unsigned int ge, fe; int oldpage, ret = 0; - /* set clock mode to 125mhz */ oldpage = phy_select_page(phydev, YT8511_EXT_CLK_GATE); if (oldpage < 0) goto err_restore_page; - ret = __phy_modify(phydev, YT8511_PAGE, 0, YT8511_CLK_125M); - if (ret < 0) - goto err_restore_page; - /* set rgmii delay mode */ switch (phydev->interface) { case PHY_INTERFACE_MODE_RGMII: @@ -80,14 +75,20 @@ static int yt8511_config_init(struct phy_device *phydev) ge = YT8511_DELAY_RX | YT8511_DELAY_GE_TX_EN; fe = YT8511_DELAY_FE_TX_EN; break; - default: /* leave everything alone in other modes */ - break; + default: /* do not support other modes */ + ret = -EOPNOTSUPP; + goto err_restore_page; } ret = __phy_modify(phydev, YT8511_PAGE, (YT8511_DELAY_RX | YT8511_DELAY_GE_TX_EN), ge); if (ret < 0) goto err_restore_page; + /* set clock mode to 125mhz */ + ret = __phy_modify(phydev, YT8511_PAGE, 0, YT8511_CLK_125M); + if (ret < 0) + goto err_restore_page; + /* fast ethernet delay is in a separate page */ ret = __phy_write(phydev, YT8511_PAGE_SELECT, YT8511_EXT_DELAY_DRIVE); if (ret < 0) -- 2.25.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] net: phy: abort loading yt8511 driver in unsupported modes 2021-05-25 12:26 ` [PATCH 2/2] net: phy: abort loading yt8511 driver in unsupported modes Peter Geis @ 2021-05-25 13:03 ` Andrew Lunn 0 siblings, 0 replies; 7+ messages in thread From: Andrew Lunn @ 2021-05-25 13:03 UTC (permalink / raw) To: Peter Geis Cc: Heiner Kallweit, Russell King, David S . Miller, Jakub Kicinski, Nathan Chancellor, netdev, linux-kernel, clang-built-linux, kernel test robot On Tue, May 25, 2021 at 08:26:15AM -0400, Peter Geis wrote: > While investigating the clang `ge` uninitialized variable report, it was > discovered the default switch would have unintended consequences. Due to > the switch to __phy_modify, the driver would modify the ID values in the > default scenario. > > Fix this by promoting the interface mode switch and aborting when the > mode is not a supported RGMII mode. > > This prevents the `ge` and `fe` variables from ever being used > uninitialized. > > Fixes: b1b41c047f73 ("net: phy: add driver for Motorcomm yt8511 phy") > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Peter Geis <pgwipeout@gmail.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-05-25 13:59 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-25 12:26 [PATCH 0/2] fixes for yt8511 phy driver Peter Geis 2021-05-25 12:26 ` [PATCH 1/2] net: phy: fix yt8511 clang uninitialized variable warning Peter Geis 2021-05-25 13:01 ` Andrew Lunn 2021-05-25 13:11 ` Peter Geis 2021-05-25 13:59 ` Andrew Lunn 2021-05-25 12:26 ` [PATCH 2/2] net: phy: abort loading yt8511 driver in unsupported modes Peter Geis 2021-05-25 13:03 ` Andrew Lunn
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.