[PATCH -next 1/2] net: bfin_mac: Fix error handling in probe function

[PATCH -next 1/2] net: bfin_mac: Fix error handling in probe function

[Guenter Roeck]

Commit e7f4dc3536a4 ("mdio: Move allocation of interrupts into core")
removed interrupt allocation code. While doing so, error handling got
messed up a bit, resulting in the following build warning and a missed
call to mdiobus_free() in the probe function error path.

drivers/net/ethernet/adi/bfin_mac.c: In function 'bfin_mii_bus_probe':
drivers/net/ethernet/adi/bfin_mac.c:1865:1: warning:
	label 'out_err_irq_alloc' defined but not used

Fixes: e7f4dc3536a4 ("mdio: Move allocation of interrupts into core")
Cc: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/net/ethernet/adi/bfin_mac.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/adi/bfin_mac.c b/drivers/net/ethernet/adi/bfin_mac.c
index 62862744c870..8c5132624510 100644
--- a/drivers/net/ethernet/adi/bfin_mac.c
+++ b/drivers/net/ethernet/adi/bfin_mac.c
@@ -1856,13 +1856,13 @@ static int bfin_mii_bus_probe(struct platform_device *pdev)
 	rc = mdiobus_register(miibus);
 	if (rc) {
 		dev_err(&pdev->dev, "Cannot register MDIO bus!\n");
-		goto out_err_alloc;
+		goto out_err_mdiobus_register;
 	}
 
 	platform_set_drvdata(pdev, miibus);
 	return 0;
 
-out_err_irq_alloc:
+out_err_mdiobus_register:
 	mdiobus_free(miibus);
 out_err_alloc:
 	peripheral_free_list(pin_req);
-- 
2.1.4

[PATCH -next 2/2] net: bfin_mac: Fix build error due to missed API change

[Guenter Roeck]

Commit 7f854420fbfe ("phy: Add API for {un}registering an mdio device to
a bus") introduces an API to access mii_bus structures, but missed to
update the bfin_mac driver. This results in the following error message

drivers/net/ethernet/adi/bfin_mac.c: In function 'mii_probe':
drivers/net/ethernet/adi/bfin_mac.c:401:52: error:
	'struct mii_bus' has no member named 'phy_map'

Fixes: 7f854420fbfe ("phy: Add API for {un}registering an mdio device to a bus")
Cc: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/net/ethernet/adi/bfin_mac.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/adi/bfin_mac.c b/drivers/net/ethernet/adi/bfin_mac.c
index 8c5132624510..1281cae5b70f 100644
--- a/drivers/net/ethernet/adi/bfin_mac.c
+++ b/drivers/net/ethernet/adi/bfin_mac.c
@@ -398,7 +398,8 @@ static int mii_probe(struct net_device *dev, int phy_mode)
 
 	/* search for connected PHY device */
 	for (i = 0; i < PHY_MAX_ADDR; ++i) {
-		struct phy_device *const tmp_phydev = lp->mii_bus->phy_map[i];
+		struct phy_device *const tmp_phydev =
+			mdiobus_get_phy(lp->mii_bus, i);
 
 		if (!tmp_phydev)
 			continue; /* no PHY here... */
-- 
2.1.4

Re: [PATCH -next 1/2] net: bfin_mac: Fix error handling in probe function

[Andrew Lunn]

Hi Guenter

Same fix was submitted yesterday by Sudip Mukherjee.

Thanks
	Andrew


On Sat, Jan 09, 2016 at 08:37:06AM -0800, Guenter Roeck wrote:
> Commit e7f4dc3536a4 ("mdio: Move allocation of interrupts into core")
> removed interrupt allocation code. While doing so, error handling got
> messed up a bit, resulting in the following build warning and a missed
> call to mdiobus_free() in the probe function error path.
> 
> drivers/net/ethernet/adi/bfin_mac.c: In function 'bfin_mii_bus_probe':
> drivers/net/ethernet/adi/bfin_mac.c:1865:1: warning:
> 	label 'out_err_irq_alloc' defined but not used
> 
> Fixes: e7f4dc3536a4 ("mdio: Move allocation of interrupts into core")
> Cc: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/net/ethernet/adi/bfin_mac.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/adi/bfin_mac.c b/drivers/net/ethernet/adi/bfin_mac.c
> index 62862744c870..8c5132624510 100644
> --- a/drivers/net/ethernet/adi/bfin_mac.c
> +++ b/drivers/net/ethernet/adi/bfin_mac.c
> @@ -1856,13 +1856,13 @@ static int bfin_mii_bus_probe(struct platform_device *pdev)
>  	rc = mdiobus_register(miibus);
>  	if (rc) {
>  		dev_err(&pdev->dev, "Cannot register MDIO bus!\n");
> -		goto out_err_alloc;
> +		goto out_err_mdiobus_register;
>  	}
>  
>  	platform_set_drvdata(pdev, miibus);
>  	return 0;
>  
> -out_err_irq_alloc:
> +out_err_mdiobus_register:
>  	mdiobus_free(miibus);
>  out_err_alloc:
>  	peripheral_free_list(pin_req);
> -- 
> 2.1.4
> 

Re: [PATCH -next 1/2] net: bfin_mac: Fix error handling in probe function

[Guenter Roeck]

Hi Andrew,

On 01/09/2016 08:58 AM, Andrew Lunn wrote:
> Hi Guenter
>
> Same fix was submitted yesterday by Sudip Mukherjee.
>

I missed that one, sorry.

How about drivers/net/ethernet/toshiba/tc35815.c ? It now has an unused
variable due to e7f4dc3536a4, and it fails to build because of 7f854420fbfe.

Which leads to the question which of the other build failures due to
7f854420fbfe have been addressed already. In addition to the toshiba driver,
I see build failures in drivers/net/ethernet/ti/cpmac.c and possibly
drivers/staging/octeon/ethernet-rgmii.c (the error message is different,
so it may be something else).

I don't see patches for those, but I guess I must be blind, so I figure I
better ask before I send out mine.

Thanks,
Guenter

> Thanks
> 	Andrew
>
>
> On Sat, Jan 09, 2016 at 08:37:06AM -0800, Guenter Roeck wrote:
>> Commit e7f4dc3536a4 ("mdio: Move allocation of interrupts into core")
>> removed interrupt allocation code. While doing so, error handling got
>> messed up a bit, resulting in the following build warning and a missed
>> call to mdiobus_free() in the probe function error path.
>>
>> drivers/net/ethernet/adi/bfin_mac.c: In function 'bfin_mii_bus_probe':
>> drivers/net/ethernet/adi/bfin_mac.c:1865:1: warning:
>> 	label 'out_err_irq_alloc' defined but not used
>>
>> Fixes: e7f4dc3536a4 ("mdio: Move allocation of interrupts into core")
>> Cc: Andrew Lunn <andrew@lunn.ch>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>>   drivers/net/ethernet/adi/bfin_mac.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/adi/bfin_mac.c b/drivers/net/ethernet/adi/bfin_mac.c
>> index 62862744c870..8c5132624510 100644
>> --- a/drivers/net/ethernet/adi/bfin_mac.c
>> +++ b/drivers/net/ethernet/adi/bfin_mac.c
>> @@ -1856,13 +1856,13 @@ static int bfin_mii_bus_probe(struct platform_device *pdev)
>>   	rc = mdiobus_register(miibus);
>>   	if (rc) {
>>   		dev_err(&pdev->dev, "Cannot register MDIO bus!\n");
>> -		goto out_err_alloc;
>> +		goto out_err_mdiobus_register;
>>   	}
>>
>>   	platform_set_drvdata(pdev, miibus);
>>   	return 0;
>>
>> -out_err_irq_alloc:
>> +out_err_mdiobus_register:
>>   	mdiobus_free(miibus);
>>   out_err_alloc:
>>   	peripheral_free_list(pin_req);
>> --
>> 2.1.4
>>
>

Re: [PATCH -next 1/2] net: bfin_mac: Fix error handling in probe function

[Andrew Lunn]

On Sat, Jan 09, 2016 at 10:10:41AM -0800, Guenter Roeck wrote:
> Hi Andrew,
> 
> On 01/09/2016 08:58 AM, Andrew Lunn wrote:
> >Hi Guenter
> >
> >Same fix was submitted yesterday by Sudip Mukherjee.
> >
> 
> I missed that one, sorry.
> 
> How about drivers/net/ethernet/toshiba/tc35815.c ? It now has an unused
> variable due to e7f4dc3536a4, and it fails to build because of 7f854420fbfe.
> 
> Which leads to the question which of the other build failures due to
> 7f854420fbfe have been addressed already. In addition to the toshiba driver,
> I see build failures in drivers/net/ethernet/ti/cpmac.c and possibly
> drivers/staging/octeon/ethernet-rgmii.c (the error message is different,
> so it may be something else).
> 
> I don't see patches for those, but I guess I must be blind, so I figure I
> better ask before I send out mine.

That i have seen so far:

Wantiq_etp.c: Me
octeon/ethernet-rgmii.c: Me
adi/bfin_mac.c: Sudip Mukerjee
nxp/lpc_eth.c: Fabio Estevam

Plus there is an interesting one for micrel.c which is going to have
to wait a while.

I think you should post your fix for ti/cpmac.c and toshiba/tc35815.c

Looks like i did a poor jobs of build testing :-(

      Andrew

Re: [PATCH -next 1/2] net: bfin_mac: Fix error handling in probe function

[Guenter Roeck]

On 01/09/2016 12:52 PM, Andrew Lunn wrote:
> On Sat, Jan 09, 2016 at 10:10:41AM -0800, Guenter Roeck wrote:
>> Hi Andrew,
>>
>> On 01/09/2016 08:58 AM, Andrew Lunn wrote:
>>> Hi Guenter
>>>
>>> Same fix was submitted yesterday by Sudip Mukherjee.
>>>
>>
>> I missed that one, sorry.
>>
>> How about drivers/net/ethernet/toshiba/tc35815.c ? It now has an unused
>> variable due to e7f4dc3536a4, and it fails to build because of 7f854420fbfe.
>>
>> Which leads to the question which of the other build failures due to
>> 7f854420fbfe have been addressed already. In addition to the toshiba driver,
>> I see build failures in drivers/net/ethernet/ti/cpmac.c and possibly
>> drivers/staging/octeon/ethernet-rgmii.c (the error message is different,
>> so it may be something else).
>>
>> I don't see patches for those, but I guess I must be blind, so I figure I
>> better ask before I send out mine.
>
> That i have seen so far:
>
> Wantiq_etp.c: Me
> octeon/ethernet-rgmii.c: Me
> adi/bfin_mac.c: Sudip Mukerjee

I re-posted this one anyway, using phy_find_first.

> nxp/lpc_eth.c: Fabio Estevam
>
> Plus there is an interesting one for micrel.c which is going to have
> to wait a while.
>
How about those ?

drivers/net/ethernet/freescale/ucc_geth.c: In function 'uec_configure_serdes':
drivers/net/ethernet/freescale/ucc_geth.c:1719:20: error: 'struct phy_device' has no member named 'dev'

drivers/net/ethernet/freescale/fs_enet/mac-fec.c: In function 'stop':
drivers/net/ethernet/freescale/fs_enet/mac-fec.c:366:36: error: 'struct phy_device' has no member named 'bus'

Guenter

Re: [PATCH -next 1/2] net: bfin_mac: Fix error handling in probe function

[Andrew Lunn]

> How about those ?
> 
> drivers/net/ethernet/freescale/ucc_geth.c: In function 'uec_configure_serdes':
> drivers/net/ethernet/freescale/ucc_geth.c:1719:20: error: 'struct phy_device' has no member named 'dev'
> 
> drivers/net/ethernet/freescale/fs_enet/mac-fec.c: In function 'stop':
> drivers/net/ethernet/freescale/fs_enet/mac-fec.c:366:36: error: 'struct phy_device' has no member named 'bus'

Not seen fixes for those, nor error reports either.

I think the next time i do something like this, i'm going to ask you
to run some built tests for me! You seem to have better coverage than
0-day.

     Andrew

Re: [PATCH -next 1/2] net: bfin_mac: Fix error handling in probe function

[Guenter Roeck]

On 01/10/2016 07:49 AM, Andrew Lunn wrote:
>> How about those ?
>>
>> drivers/net/ethernet/freescale/ucc_geth.c: In function 'uec_configure_serdes':
>> drivers/net/ethernet/freescale/ucc_geth.c:1719:20: error: 'struct phy_device' has no member named 'dev'
>>
>> drivers/net/ethernet/freescale/fs_enet/mac-fec.c: In function 'stop':
>> drivers/net/ethernet/freescale/fs_enet/mac-fec.c:366:36: error: 'struct phy_device' has no member named 'bus'
>
> Not seen fixes for those, nor error reports either.
>
Ok, I'll see if I can fix those.

Also:

drivers/net/ethernet/amd/au1000_eth.c:  /* WARNING: bus->phy_map[phy_addr].attached_dev == dev does
	[ no idea what to do with that comment ]
drivers/net/ethernet/broadcom/sb1250-mac.c:             phy_dev = sc->mii_bus->phy_map[i];
	[ can use phy_find_first ]
drivers/staging/netlogic/xlr_net.c:     struct phy_device *phydev = priv->mii_bus->phy_map[priv->phy_addr];
	[ several ]	

> I think the next time i do something like this, i'm going to ask you
> to run some built tests for me! You seem to have better coverage than
> 0-day.
>
Sure, no problem.

Guenter

Re: [PATCH -next 1/2] net: bfin_mac: Fix error handling in probe function

[Andrew Lunn]

> drivers/net/ethernet/amd/au1000_eth.c:  /* WARNING: bus->phy_map[phy_addr].attached_dev == dev does
> 	[ no idea what to do with that comment ]

That comment is very generic, it applies to all MDIO devices. So i'm
tempted to remove it. I will send a patch, probably tomorrow.

> drivers/net/ethernet/broadcom/sb1250-mac.c:             phy_dev = sc->mii_bus->phy_map[i];
> 	[ can use phy_find_first ]

Yes, phy_find_first.

> drivers/staging/netlogic/xlr_net.c:     struct phy_device *phydev = priv->mii_bus->phy_map[priv->phy_addr];
> 	[ several ]	

Odd that i missed that file for phy_map, but did patch it for
phydev_name().

I will send a patch for that, but probably tomorrow.

  Andrew

Re: [PATCH -next 1/2] net: bfin_mac: Fix error handling in probe function

[Guenter Roeck]

On 01/10/2016 09:21 AM, Andrew Lunn wrote:
>> drivers/net/ethernet/amd/au1000_eth.c:  /* WARNING: bus->phy_map[phy_addr].attached_dev == dev does
>> 	[ no idea what to do with that comment ]
>
> That comment is very generic, it applies to all MDIO devices. So i'm
> tempted to remove it. I will send a patch, probably tomorrow.
>
>> drivers/net/ethernet/broadcom/sb1250-mac.c:             phy_dev = sc->mii_bus->phy_map[i];
>> 	[ can use phy_find_first ]
>
> Yes, phy_find_first.
>
>> drivers/staging/netlogic/xlr_net.c:     struct phy_device *phydev = priv->mii_bus->phy_map[priv->phy_addr];
>> 	[ several ]	
>
> Odd that i missed that file for phy_map, but did patch it for
> phydev_name().
>

xlr_net.c also fails to build with

drivers/staging/netlogic/xlr_net.c: In function 'xlr_setup_mdio':
drivers/staging/netlogic/xlr_net.c:879:21: error: assignment to expression with array type
   priv->mii_bus->irq = kmalloc(sizeof(int)*PHY_MAX_ADDR, GFP_KERNEL);

This might be related to "mdio: Move allocation of interrupts into core",
though I didn't track it down.

Guenter

Re: [PATCH -next 1/2] net: bfin_mac: Fix error handling in probe function

[Andrew Lunn]

> xlr_net.c also fails to build with
> 
> drivers/staging/netlogic/xlr_net.c: In function 'xlr_setup_mdio':
> drivers/staging/netlogic/xlr_net.c:879:21: error: assignment to expression with array type
>   priv->mii_bus->irq = kmalloc(sizeof(int)*PHY_MAX_ADDR, GFP_KERNEL);
> 
> This might be related to "mdio: Move allocation of interrupts into core",
> though I didn't track it down.

Yes, that was the interrupt change.

I sent a patch which fixes both.

  Andrew

Re: [PATCH -next 1/2] net: bfin_mac: Fix error handling in probe function

[Sudip Mukherjee]

On Sun, Jan 10, 2016 at 07:16:16AM -0800, Guenter Roeck wrote:
> On 01/09/2016 12:52 PM, Andrew Lunn wrote:
> >On Sat, Jan 09, 2016 at 10:10:41AM -0800, Guenter Roeck wrote:
> >>Hi Andrew,
> >>
> >>On 01/09/2016 08:58 AM, Andrew Lunn wrote:
> >>>Hi Guenter
> >>>
> >>>Same fix was submitted yesterday by Sudip Mukherjee.
> >>>
> >>
> >>I missed that one, sorry.
> >>
> >>How about drivers/net/ethernet/toshiba/tc35815.c ? It now has an unused
> >>variable due to e7f4dc3536a4, and it fails to build because of 7f854420fbfe.
> >>
> >>Which leads to the question which of the other build failures due to
> >>7f854420fbfe have been addressed already. In addition to the toshiba driver,
> >>I see build failures in drivers/net/ethernet/ti/cpmac.c and possibly
> >>drivers/staging/octeon/ethernet-rgmii.c (the error message is different,
> >>so it may be something else).
> >>
> >>I don't see patches for those, but I guess I must be blind, so I figure I
> >>better ask before I send out mine.
> >
> >That i have seen so far:
> >
> >Wantiq_etp.c: Me
> >octeon/ethernet-rgmii.c: Me
> >adi/bfin_mac.c: Sudip Mukerjee
> 
> I re-posted this one anyway, using phy_find_first.
> 
> >nxp/lpc_eth.c: Fabio Estevam
> >
> >Plus there is an interesting one for micrel.c which is going to have
> >to wait a while.
> >
> How about those ?
> 
> drivers/net/ethernet/freescale/ucc_geth.c: In function 'uec_configure_serdes':
> drivers/net/ethernet/freescale/ucc_geth.c:1719:20: error: 'struct phy_device' has no member named 'dev'
> 
> drivers/net/ethernet/freescale/fs_enet/mac-fec.c: In function 'stop':
> drivers/net/ethernet/freescale/fs_enet/mac-fec.c:366:36: error: 'struct phy_device' has no member named 'bus'

My builds didn't show them not even the other ones mentioned above. :(
Which config you used for these?

regards
sudip

Re: [PATCH -next 1/2] net: bfin_mac: Fix error handling in probe function

[Guenter Roeck]

On 01/11/2016 12:43 AM, Sudip Mukherjee wrote:
> On Sun, Jan 10, 2016 at 07:16:16AM -0800, Guenter Roeck wrote:
>> On 01/09/2016 12:52 PM, Andrew Lunn wrote:
>>> On Sat, Jan 09, 2016 at 10:10:41AM -0800, Guenter Roeck wrote:
>>>> Hi Andrew,
>>>>
>>>> On 01/09/2016 08:58 AM, Andrew Lunn wrote:
>>>>> Hi Guenter
>>>>>
>>>>> Same fix was submitted yesterday by Sudip Mukherjee.
>>>>>
>>>>
>>>> I missed that one, sorry.
>>>>
>>>> How about drivers/net/ethernet/toshiba/tc35815.c ? It now has an unused
>>>> variable due to e7f4dc3536a4, and it fails to build because of 7f854420fbfe.
>>>>
>>>> Which leads to the question which of the other build failures due to
>>>> 7f854420fbfe have been addressed already. In addition to the toshiba driver,
>>>> I see build failures in drivers/net/ethernet/ti/cpmac.c and possibly
>>>> drivers/staging/octeon/ethernet-rgmii.c (the error message is different,
>>>> so it may be something else).
>>>>
>>>> I don't see patches for those, but I guess I must be blind, so I figure I
>>>> better ask before I send out mine.
>>>
>>> That i have seen so far:
>>>
>>> Wantiq_etp.c: Me
>>> octeon/ethernet-rgmii.c: Me
>>> adi/bfin_mac.c: Sudip Mukerjee
>>
>> I re-posted this one anyway, using phy_find_first.
>>
>>> nxp/lpc_eth.c: Fabio Estevam
>>>
>>> Plus there is an interesting one for micrel.c which is going to have
>>> to wait a while.
>>>
>> How about those ?
>>
>> drivers/net/ethernet/freescale/ucc_geth.c: In function 'uec_configure_serdes':
>> drivers/net/ethernet/freescale/ucc_geth.c:1719:20: error: 'struct phy_device' has no member named 'dev'
>>
>> drivers/net/ethernet/freescale/fs_enet/mac-fec.c: In function 'stop':
>> drivers/net/ethernet/freescale/fs_enet/mac-fec.c:366:36: error: 'struct phy_device' has no member named 'bus'
>
> My builds didn't show them not even the other ones mentioned above. :(
> Which config you used for these?
>

This is with powerpc:mpc85xx_defconfig and powerpc:mpc85xx_smp_defconfig.

Sometimes one has to build _all_ configurations for an architecture. I'd love to do that
if I had more powerful servers.

Guenter