| Message ID | 1421731543-13290-1-git-send-email-der.herr@hofr.at (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Kalle Valo |
| Headers | show |
On Tuesday, January 20, 2015 06:25:43 AM Nicholas Mc Guire wrote: > if(!wait_for_completion_interruptible_timeout(...)) > only handles the timeout case - this patch adds handling the > signal case the same as timeout. > > Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at> Acked-by: Christian Lamparter <chunkeey@googlemail.com> > diff --git a/drivers/net/wireless/p54/fwio.c b/drivers/net/wireless/p54/fwio.c > index bc065e8..5367d51 100644 > --- a/drivers/net/wireless/p54/fwio.c > +++ b/drivers/net/wireless/p54/fwio.c > @@ -249,9 +250,11 @@ int p54_download_eeprom(struct p54_common *priv, void *buf, > > p54_tx(priv, skb); > > - if (!wait_for_completion_interruptible_timeout( > - &priv->eeprom_comp, HZ)) { > - wiphy_err(priv->hw->wiphy, "device does not respond!\n"); > + timeout = wait_for_completion_interruptible_timeout( > + &priv->eeprom_comp, HZ); > + if (timeout <= 0) { > + wiphy_err(priv->hw->wiphy, > + "device does not respond or signal received!\n"); > ret = -EBUSY; > } > priv->eeprom = NULL; > CHECK: Alignment should match open parenthesis #98: FILE: drivers/net/wireless/p54/fwio.c:257: + wiphy_err(priv->hw->wiphy, + "device does not respond or signal received!\n"); total: 0 errors, 0 warnings, 1 checks, 21 lines checked -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 22 Jan 2015, Christian Lamparter wrote: > On Tuesday, January 20, 2015 06:25:43 AM Nicholas Mc Guire wrote: > > if(!wait_for_completion_interruptible_timeout(...)) > > only handles the timeout case - this patch adds handling the > > signal case the same as timeout. > > > > Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at> > Acked-by: Christian Lamparter <chunkeey@googlemail.com> > > > diff --git a/drivers/net/wireless/p54/fwio.c b/drivers/net/wireless/p54/fwio.c > > index bc065e8..5367d51 100644 > > --- a/drivers/net/wireless/p54/fwio.c > > +++ b/drivers/net/wireless/p54/fwio.c > > @@ -249,9 +250,11 @@ int p54_download_eeprom(struct p54_common *priv, void *buf, > > > > p54_tx(priv, skb); > > > > - if (!wait_for_completion_interruptible_timeout( > > - &priv->eeprom_comp, HZ)) { > > - wiphy_err(priv->hw->wiphy, "device does not respond!\n"); > > + timeout = wait_for_completion_interruptible_timeout( > > + &priv->eeprom_comp, HZ); > > + if (timeout <= 0) { > > + wiphy_err(priv->hw->wiphy, > > + "device does not respond or signal received!\n"); > > ret = -EBUSY; > > } > > priv->eeprom = NULL; > > > > CHECK: Alignment should match open parenthesis > #98: FILE: drivers/net/wireless/p54/fwio.c:257: > + wiphy_err(priv->hw->wiphy, > + "device does not respond or signal received!\n"); > > total: 0 errors, 0 warnings, 1 checks, 21 lines checked > Tried fixing this up - but simply no clue what coding style rule that might have violated - and my attempts to fix this did not succeed - allignment seems righta - the complete siequence is: timeout = wait_for_completion_interruptible_timeout( &priv->eeprom_comp, HZ); if (timeout <= 0) { wiphy_err(priv->hw->wiphy, "device does not respond or signal received!\n"); ret = -EBUSY; } what am I overlooking ? thx! hofrat -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> if(!wait_for_completion_interruptible_timeout(...)) > only handles the timeout case - this patch adds handling the > signal case the same as timeout. > > Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at> > Acked-by: Christian Lamparter <chunkeey@googlemail.com> Thanks, applied to wireless-drivers-next.git but I removed "wireless:" from the title. Kalle Valo -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday, January 23, 2015 01:27:05 PM Nicholas Mc Guire wrote: > On Thu, 22 Jan 2015, Christian Lamparter wrote: > > On Tuesday, January 20, 2015 06:25:43 AM Nicholas Mc Guire wrote: > > > @@ -249,9 +250,11 @@ int p54_download_eeprom(struct p54_common *priv, void *buf, > > > + wiphy_err(priv->hw->wiphy, > > > + "device does not respond or signal received!\n"); > > Tried fixing this up - but simply no clue what coding style > rule that might have violated - and my attempts to fix this > did not succeed - allignment seems righta - the complete siequence is: > > timeout = wait_for_completion_interruptible_timeout( > &priv->eeprom_comp, HZ); > if (timeout <= 0) { > wiphy_err(priv->hw->wiphy, > "device does not respond or signal received!\n"); > ret = -EBUSY; > } > > what am I overlooking ? Not much, you might be confused simply because your editor and email program doesn't use a fixed space font (or it is not enabled (yet)). Fixing this is just a matter of adding a few extra spaces so the text lines up with the open parenthesis. original: + wiphy_err(priv->hw->wiphy, + "device does not respond or signal received!\n"); aligned: + wiphy_err(priv->hw->wiphy, + "device does not respond or signal received!\n"); Regards, Christian -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/wireless/p54/fwio.c b/drivers/net/wireless/p54/fwio.c index bc065e8..5367d51 100644 --- a/drivers/net/wireless/p54/fwio.c +++ b/drivers/net/wireless/p54/fwio.c @@ -220,6 +220,7 @@ int p54_download_eeprom(struct p54_common *priv, void *buf, struct sk_buff *skb; size_t eeprom_hdr_size; int ret = 0; + long timeout; if (priv->fw_var >= 0x509) eeprom_hdr_size = sizeof(*eeprom_hdr); @@ -249,9 +250,11 @@ int p54_download_eeprom(struct p54_common *priv, void *buf, p54_tx(priv, skb); - if (!wait_for_completion_interruptible_timeout( - &priv->eeprom_comp, HZ)) { - wiphy_err(priv->hw->wiphy, "device does not respond!\n"); + timeout = wait_for_completion_interruptible_timeout( + &priv->eeprom_comp, HZ); + if (timeout <= 0) { + wiphy_err(priv->hw->wiphy, + "device does not respond or signal received!\n"); ret = -EBUSY; } priv->eeprom = NULL;
if(!wait_for_completion_interruptible_timeout(...)) only handles the timeout case - this patch adds handling the signal case the same as timeout. Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at> --- Only the timeout case was being handled, the signal case (-ERESTARTSYS) was treated just like the case of successful completion, which is most likely not reasonable. p54_download_eeprom() is called in p54_read_eeprom() and will terminate if p54_download_eeprom() returned != 0 so the logic should be correct - but this needs a check from someone who knows the driver. Translating -ETIMEOUT to -EBUSY might be ok not sure if -ERESTARTSYS also should be returned as -EBUSY ? Patch was only compild tested with x86_64_defcofnig + CONFIG_P54_COMMON=m, CONFIG_P54_PCI=m Patch is against 3.19.0-rc5 -next-20150119 drivers/net/wireless/p54/fwio.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)