diff mbox series

[net] net: phy: micrel: Make the GPIO to be non-exclusive

Message ID 20220824192827.437095-1-horatiu.vultur@microchip.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net] net: phy: micrel: Make the GPIO to be non-exclusive | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 85 this patch: 85
netdev/cc_maintainers success CCed 9 of 9 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 85 this patch: 85
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 9 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Horatiu Vultur Aug. 24, 2022, 7:28 p.m. UTC
The same GPIO line can be shared by multiple phys for the coma mode pin.
If that is the case then, all the other phys that share the same line
will failed to be probed because the access to the gpio line is not
non-exclusive.
Fix this by making access to the gpio line to be nonexclusive using flag
GPIOD_FLAGS_BIT_NONEXCLUSIVE. This allows all the other PHYs to be
probed.

Fixes: 738871b09250ee ("net: phy: micrel: add coma mode GPIO")
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 drivers/net/phy/micrel.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Andrew Lunn Aug. 24, 2022, 7:47 p.m. UTC | #1
On Wed, Aug 24, 2022 at 09:28:27PM +0200, Horatiu Vultur wrote:
> The same GPIO line can be shared by multiple phys for the coma mode pin.
> If that is the case then, all the other phys that share the same line
> will failed to be probed because the access to the gpio line is not
> non-exclusive.
> Fix this by making access to the gpio line to be nonexclusive using flag
> GPIOD_FLAGS_BIT_NONEXCLUSIVE. This allows all the other PHYs to be
> probed.
> 
> Fixes: 738871b09250ee ("net: phy: micrel: add coma mode GPIO")
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  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 e78d0bf69bc3..ea72ff64ad33 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -2878,7 +2878,8 @@ static int lan8814_release_coma_mode(struct phy_device *phydev)
>  	struct gpio_desc *gpiod;
>  
>  	gpiod = devm_gpiod_get_optional(&phydev->mdio.dev, "coma-mode",
> -					GPIOD_OUT_HIGH_OPEN_DRAIN);
> +					GPIOD_OUT_HIGH_OPEN_DRAIN |
> +					GPIOD_FLAGS_BIT_NONEXCLUSIVE);

I would suggest putting a comment here. You are assuming the driver
never gains a lan8814_take_coma_mode() when the PHY is put into
suspend, since it sounds like that will put all PHYs on the shared
GPIO into coma mode.

     Andrew
Horatiu Vultur Aug. 25, 2022, 6:54 a.m. UTC | #2
The 08/24/2022 21:47, Andrew Lunn wrote:

Hi Andrew,

> 
> On Wed, Aug 24, 2022 at 09:28:27PM +0200, Horatiu Vultur wrote:
> > The same GPIO line can be shared by multiple phys for the coma mode pin.
> > If that is the case then, all the other phys that share the same line
> > will failed to be probed because the access to the gpio line is not
> > non-exclusive.
> > Fix this by making access to the gpio line to be nonexclusive using flag
> > GPIOD_FLAGS_BIT_NONEXCLUSIVE. This allows all the other PHYs to be
> > probed.
> >
> > Fixes: 738871b09250ee ("net: phy: micrel: add coma mode GPIO")
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > ---
> >  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 e78d0bf69bc3..ea72ff64ad33 100644
> > --- a/drivers/net/phy/micrel.c
> > +++ b/drivers/net/phy/micrel.c
> > @@ -2878,7 +2878,8 @@ static int lan8814_release_coma_mode(struct phy_device *phydev)
> >       struct gpio_desc *gpiod;
> >
> >       gpiod = devm_gpiod_get_optional(&phydev->mdio.dev, "coma-mode",
> > -                                     GPIOD_OUT_HIGH_OPEN_DRAIN);
> > +                                     GPIOD_OUT_HIGH_OPEN_DRAIN |
> > +                                     GPIOD_FLAGS_BIT_NONEXCLUSIVE);
> 
> I would suggest putting a comment here. You are assuming the driver
> never gains a lan8814_take_coma_mode() when the PHY is put into
> suspend, since it sounds like that will put all PHYs on the shared
> GPIO into coma mode.

That is correct.
I will add a comment in the next version.

> 
>      Andrew
diff mbox series

Patch

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index e78d0bf69bc3..ea72ff64ad33 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -2878,7 +2878,8 @@  static int lan8814_release_coma_mode(struct phy_device *phydev)
 	struct gpio_desc *gpiod;
 
 	gpiod = devm_gpiod_get_optional(&phydev->mdio.dev, "coma-mode",
-					GPIOD_OUT_HIGH_OPEN_DRAIN);
+					GPIOD_OUT_HIGH_OPEN_DRAIN |
+					GPIOD_FLAGS_BIT_NONEXCLUSIVE);
 	if (IS_ERR(gpiod))
 		return PTR_ERR(gpiod);