[PATCH] [PATCH] prism54: set carrier flags correctly

[PATCH] [PATCH] prism54: set carrier flags correctly

[John W. Linville]

From: Roy Marples <uberlord@gentoo.org>

prism54 should set the carrier flags correctly when it thinks the
link can be used.

Signed-off-by: Roy Marples <uberlord@gentoo.org>
---
I found this in my pile from a long time back.  Anyone have any
comments on it?

 drivers/net/wireless/prism54/isl_ioctl.c  |    5 ++++-
 drivers/net/wireless/prism54/islpci_dev.c |    4 +++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/prism54/isl_ioctl.c b/drivers/net/wireless/prism54/isl_ioctl.c
index 838d510..25d42ac 100644
--- a/drivers/net/wireless/prism54/isl_ioctl.c
+++ b/drivers/net/wireless/prism54/isl_ioctl.c
@@ -2093,8 +2093,11 @@ link_changed(struct net_device *ndev, u32 bitrate)
 		} else
 			send_simple_event(netdev_priv(ndev),
 					  "Link established");
-	} else
+		netif_carrier_on(ndev);
+	} else {
 		send_simple_event(netdev_priv(ndev), "Link lost");
+		netif_carrier_off(ndev);
+	}
 }
 
 /* Beacon/ProbeResp payload header */
diff --git a/drivers/net/wireless/prism54/islpci_dev.c b/drivers/net/wireless/prism54/islpci_dev.c
index f057fd9..9c8c6f2 100644
--- a/drivers/net/wireless/prism54/islpci_dev.c
+++ b/drivers/net/wireless/prism54/islpci_dev.c
@@ -386,7 +386,9 @@ islpci_open(struct net_device *ndev)
 	}
 
 	netif_start_queue(ndev);
-/*      netif_mark_up( ndev ); */
+
+	/* Turn off carrier unless we know we have associated */
+	netif_carrier_off(ndev);
 
 	return 0;
 }
-- 
1.4.4.2

Re: [PATCH] [PATCH] prism54: set carrier flags correctly

[Dan Williams]

On Mon, 2008-03-31 at 14:28 -0400, John W. Linville wrote:
> From: Roy Marples <uberlord@gentoo.org>
> 
> prism54 should set the carrier flags correctly when it thinks the
> link can be used.
> 
> Signed-off-by: Roy Marples <uberlord@gentoo.org>
> ---
> I found this in my pile from a long time back.  Anyone have any
> comments on it?

Other than the fact that the carrier flag on wireless devices is kind of
useless, it's fine.  I guess it would be somewhat useful if we could
ensure that the carrier would be on _only_ when the card was
successfully associated and authenticated to the requested AP, but until
every driver was converted properly it would be useless because you
couldn't depend on it.

Dan

>  drivers/net/wireless/prism54/isl_ioctl.c  |    5 ++++-
>  drivers/net/wireless/prism54/islpci_dev.c |    4 +++-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/prism54/isl_ioctl.c b/drivers/net/wireless/prism54/isl_ioctl.c
> index 838d510..25d42ac 100644
> --- a/drivers/net/wireless/prism54/isl_ioctl.c
> +++ b/drivers/net/wireless/prism54/isl_ioctl.c
> @@ -2093,8 +2093,11 @@ link_changed(struct net_device *ndev, u32 bitrate)
>  		} else
>  			send_simple_event(netdev_priv(ndev),
>  					  "Link established");
> -	} else
> +		netif_carrier_on(ndev);
> +	} else {
>  		send_simple_event(netdev_priv(ndev), "Link lost");
> +		netif_carrier_off(ndev);
> +	}
>  }
>  
>  /* Beacon/ProbeResp payload header */
> diff --git a/drivers/net/wireless/prism54/islpci_dev.c b/drivers/net/wireless/prism54/islpci_dev.c
> index f057fd9..9c8c6f2 100644
> --- a/drivers/net/wireless/prism54/islpci_dev.c
> +++ b/drivers/net/wireless/prism54/islpci_dev.c
> @@ -386,7 +386,9 @@ islpci_open(struct net_device *ndev)
>  	}
>  
>  	netif_start_queue(ndev);
> -/*      netif_mark_up( ndev ); */
> +
> +	/* Turn off carrier unless we know we have associated */
> +	netif_carrier_off(ndev);
>  
>  	return 0;
>  }

Re: [PATCH] [PATCH] prism54: set carrier flags correctly

[Luis R. Rodriguez]

On Mon, Mar 31, 2008 at 02:59:50PM -0400, Dan Williams wrote:
> On Mon, 2008-03-31 at 14:28 -0400, John W. Linville wrote:
> > From: Roy Marples <uberlord@gentoo.org>
> > 
> > prism54 should set the carrier flags correctly when it thinks the
> > link can be used.
> > 
> > Signed-off-by: Roy Marples <uberlord@gentoo.org>
> > ---
> > I found this in my pile from a long time back.  Anyone have any
> > comments on it?
> 
> Other than the fact that the carrier flag on wireless devices is kind of
> useless, it's fine. 

Agreed, so sure, this is OK but I rather we turn the carrier on
or off *before* sending an event, like this.

Signed-off-by: Roy Marples <uberlord@gentoo.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@winlab.rutgers.edu>

diff --git a/drivers/net/wireless/prism54/isl_ioctl.c b/drivers/net/wireless/prism54/isl_ioctl.c
index 2e25e19..e5b3c28 100644
--- a/drivers/net/wireless/prism54/isl_ioctl.c
+++ b/drivers/net/wireless/prism54/isl_ioctl.c
@@ -2081,6 +2081,7 @@ link_changed(struct net_device *ndev, u32 bitrate)
 	islpci_private *priv = netdev_priv(ndev);
 
 	if (bitrate) {
+		netif_carrier_on(ndev);
 		if (priv->iw_mode == IW_MODE_INFRA) {
 			union iwreq_data uwrq;
 			prism54_get_wap(ndev, NULL, (struct sockaddr *) &uwrq,
@@ -2089,8 +2090,10 @@ link_changed(struct net_device *ndev, u32 bitrate)
 		} else
 			send_simple_event(netdev_priv(ndev),
 					  "Link established");
-	} else
+	} else {
+		netif_carrier_off(ndev);
 		send_simple_event(netdev_priv(ndev), "Link lost");
+	}
 }
 
 /* Beacon/ProbeResp payload header */
diff --git a/drivers/net/wireless/prism54/islpci_dev.c b/drivers/net/wireless/prism54/islpci_dev.c
index eb7c1c6..04c2638 100644
--- a/drivers/net/wireless/prism54/islpci_dev.c
+++ b/drivers/net/wireless/prism54/islpci_dev.c
@@ -387,7 +387,9 @@ islpci_open(struct net_device *ndev)
 	}
 
 	netif_start_queue(ndev);
-/*      netif_mark_up( ndev ); */
+
+	/* Turn off carrier unless we know we have associated */
+	netif_carrier_off(ndev);
 
 	return 0;
 }

Re: [PATCH] [PATCH] prism54: set carrier flags correctly

[Dan Williams]

On Tue, 2008-04-01 at 15:17 -0400, Luis R. Rodriguez wrote:
> On Mon, Mar 31, 2008 at 02:59:50PM -0400, Dan Williams wrote:
> > On Mon, 2008-03-31 at 14:28 -0400, John W. Linville wrote:
> > > From: Roy Marples <uberlord@gentoo.org>
> > > 
> > > prism54 should set the carrier flags correctly when it thinks the
> > > link can be used.
> > > 
> > > Signed-off-by: Roy Marples <uberlord@gentoo.org>
> > > ---
> > > I found this in my pile from a long time back.  Anyone have any
> > > comments on it?
> > 
> > Other than the fact that the carrier flag on wireless devices is kind of
> > useless, it's fine. 
> 
> Agreed, so sure, this is OK but I rather we turn the carrier on
> or off *before* sending an event, like this.

+1 from me, that's the right sequence.

Dan

> Signed-off-by: Roy Marples <uberlord@gentoo.org>
> Signed-off-by: Luis R. Rodriguez <mcgrof@winlab.rutgers.edu>
> 
> diff --git a/drivers/net/wireless/prism54/isl_ioctl.c b/drivers/net/wireless/prism54/isl_ioctl.c
> index 2e25e19..e5b3c28 100644
> --- a/drivers/net/wireless/prism54/isl_ioctl.c
> +++ b/drivers/net/wireless/prism54/isl_ioctl.c
> @@ -2081,6 +2081,7 @@ link_changed(struct net_device *ndev, u32 bitrate)
>  	islpci_private *priv = netdev_priv(ndev);
>  
>  	if (bitrate) {
> +		netif_carrier_on(ndev);
>  		if (priv->iw_mode == IW_MODE_INFRA) {
>  			union iwreq_data uwrq;
>  			prism54_get_wap(ndev, NULL, (struct sockaddr *) &uwrq,
> @@ -2089,8 +2090,10 @@ link_changed(struct net_device *ndev, u32 bitrate)
>  		} else
>  			send_simple_event(netdev_priv(ndev),
>  					  "Link established");
> -	} else
> +	} else {
> +		netif_carrier_off(ndev);
>  		send_simple_event(netdev_priv(ndev), "Link lost");
> +	}
>  }
>  
>  /* Beacon/ProbeResp payload header */
> diff --git a/drivers/net/wireless/prism54/islpci_dev.c b/drivers/net/wireless/prism54/islpci_dev.c
> index eb7c1c6..04c2638 100644
> --- a/drivers/net/wireless/prism54/islpci_dev.c
> +++ b/drivers/net/wireless/prism54/islpci_dev.c
> @@ -387,7 +387,9 @@ islpci_open(struct net_device *ndev)
>  	}
>  
>  	netif_start_queue(ndev);
> -/*      netif_mark_up( ndev ); */
> +
> +	/* Turn off carrier unless we know we have associated */
> +	netif_carrier_off(ndev);
>  
>  	return 0;
>  }