Re: [ 07/91] net/wireless: ipw2200: Fix WARN_ON occurring in wiphy_register called by ipw_pci_probe

[Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>]

On Sun, May 27, 2012 at 09:25:13AM +0900, Greg KH wrote:
> 3.4-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------

As I see, this is not needed on 3.4.x or 3.3.x, since the commit
"ipw2200: Fix order of device registration", wasn't included in 3.4 or
any earlier kernels.

This is fixing an issue introduced on that commit ("ipw2200: Fix order of
device registration"), that moved register_netdev after ipw_wdev_init,
but ipw_wdev_init needs initialization made on ndo_init callback.

Unless there is any plan to include any of the "... Fix order of device
registration" commits on 3.4 stable, I don't see any reason to include
the fix for them.

Besides that, I noted that ipw2100 seems to need a similar fix as below
after the change "ipw2100: Fix order of device registration". ipw2100
will have the same issue I expect: because ipw2100_wdev_init was moved
before register_netdev, the ndo_init callback that runs ipw2100_up will
only execute later with required initialization, after wiphy_register
inside ipw2100_wdev_init runs.

> 
> From: Stanislav Yakovlev <stas.yakovlev@gmail.com>
> 
> commit 7ed85b65ab611ba841d508ae4ff8c45fa0c48c80 upstream.
> 
> The problem was found by Stefan Lippers-Hollmann
> http://marc.info/?l=linux-wireless&m=132720334512946&w=2
> 
> WARNING: at /tmp/buildd/linux-aptosid-3.2/debian/build/source_i386_none/net/wireless/core.c:562 wiphy_register+0x45/0x38d [cfg80211]()
> Hardware name: TravelMate 290 \xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff
> Modules linked in: ipw2200(+) iTCO_wdt libipw joydev drm snd_seq snd_timer snd_seq_device iTCO_vendor_support yenta_socket snd intel_agp i2c_i801 pcmcia_rsrc cfg80211 soundcore parport_pc psmouse parport rng_core snd_page_alloc serio_raw pcspkr i2c_algo_bit intel_gtt pcmcia_core evdev irda crc_ccitt rfkill lib80211 processor container ac battery shpchp pci_hotplug button ext4 mbcache jbd2 crc16 dm_mod sd_mod sr_mod crc_t10dif cdrom ata_generic pata_acpi ata_piix libata scsi_mod firewire_ohci firewire_core crc_itu_t 8139too 8139cp mii uhci_hcd ehci_hcd usbcore usb_common [last unloaded: scsi_wait_scan]
> Pid: 328, comm: modprobe Not tainted 3.2-1.slh.4-aptosid-686 #1
> Call Trace:
> [<c012eaf4>] ? warn_slowpath_common+0x7c/0x8f
> [<e0ff0b3e>] ? wiphy_register+0x45/0x38d [cfg80211]
> [<e0ff0b3e>] ? wiphy_register+0x45/0x38d [cfg80211]
> [<c012eb22>] ? warn_slowpath_null+0x1b/0x1f
> [<e0ff0b3e>] ? wiphy_register+0x45/0x38d [cfg80211]
> [<c01f89d7>] ? internal_create_group+0xf5/0xff
> [<e0a2de1c>] ? ipw_pci_probe+0xa9a/0xbd0 [ipw2200]
> [<c01519f4>] ? arch_local_irq_save+0xf/0x14
> [<c0252986>] ? pci_device_probe+0x53/0x9a
> [<c02c2820>] ? driver_probe_device+0x94/0x124
> [<c0252871>] ? pci_match_id+0x15/0x34
> [<c02c28f0>] ? __driver_attach+0x40/0x5b
> [<c02c1d81>] ? bus_for_each_dev+0x37/0x60
> [<c02c25aa>] ? driver_attach+0x17/0x1a
> [<c02c28b0>] ? driver_probe_device+0x124/0x124
> [<c02c22c4>] ? bus_add_driver+0x92/0x1d1
> [<e099d000>] ? 0xe099cfff
> [<c02c2cb8>] ? driver_register+0x7d/0xd4
> [<c017cd50>] ? jump_label_module_notify+0xec/0x167
> [<e099d000>] ? 0xe099cfff
> [<c0253017>] ? __pci_register_driver+0x32/0x87
> [<e099d000>] ? 0xe099cfff
> [<e099d02e>] ? ipw_init+0x2e/0x72 [ipw2200]
> [<c0101173>] ? do_one_initcall+0x7d/0x132
> [<c0145016>] ? __blocking_notifier_call_chain+0x47/0x4f
> [<c0154a73>] ? sys_init_module+0x13a4/0x159c
> [<c03a639f>] ? sysenter_do_call+0x12/0x28
> 
> This warning appears only if we apply Ben Hutchings' fix
> http://marc.info/?l=linux-wireless&m=132720195012653&w=2
> for the bug reported by Cesare Leonardi
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=656813
> with cfg80211 warning during device registration
> ("cfg80211: failed to add phy80211 symlink to netdev!").
> 
> We separate device bring up and registration with network stack
> to avoid the problem.
> 
> After that Ben Hutchings' fix can be applied to fix the bug.
> 
> Signed-off-by: Stanislav Yakovlev <stas.yakovlev@gmail.com>
> Tested-by: Stefan Lippers-Hollmann <s.l-h@gmx.de>
> Signed-off-by: John W. Linville <linville@tuxdriver.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> ---
>  drivers/net/wireless/ipw2x00/ipw2200.c |   21 ++++++---------------
>  1 file changed, 6 insertions(+), 15 deletions(-)
> 
> --- a/drivers/net/wireless/ipw2x00/ipw2200.c
> +++ b/drivers/net/wireless/ipw2x00/ipw2200.c
> @@ -11443,20 +11443,6 @@ static void ipw_bg_down(struct work_stru
>  	mutex_unlock(&priv->mutex);
>  }
>  
> -/* Called by register_netdev() */
> -static int ipw_net_init(struct net_device *dev)
> -{
> -	int rc = 0;
> -	struct ipw_priv *priv = libipw_priv(dev);
> -
> -	mutex_lock(&priv->mutex);
> -	if (ipw_up(priv))
> -		rc = -EIO;
> -	mutex_unlock(&priv->mutex);
> -
> -	return rc;
> -}
> -
>  static int ipw_wdev_init(struct net_device *dev)
>  {
>  	int i, rc = 0;
> @@ -11722,7 +11708,6 @@ static void ipw_prom_free(struct ipw_pri
>  #endif
>  
>  static const struct net_device_ops ipw_netdev_ops = {
> -	.ndo_init		= ipw_net_init,
>  	.ndo_open		= ipw_net_open,
>  	.ndo_stop		= ipw_net_stop,
>  	.ndo_set_rx_mode	= ipw_net_set_multicast_list,
> @@ -11849,6 +11834,12 @@ static int __devinit ipw_pci_probe(struc
>  		goto out_release_irq;
>  	}
>  
> +	if (ipw_up(priv)) {
> +		mutex_unlock(&priv->mutex);
> +		err = -EIO;
> +		goto out_remove_sysfs;
> +	}
> +
>  	mutex_unlock(&priv->mutex);
>  	err = register_netdev(net_dev);
>  	if (err) {
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
[]'s
Herton