| Message ID | CAGh4K3Lym9yETNbMutfjNuANrgwtvfrGes6emr3Mvhx47D3B2g@mail.gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Mon, 2012-08-06 at 11:49 +0000, Christopher Sacchi wrote: > The source file needed a change that was told in the description to > know that the #include needed to be fixed, and the function changed > another value to 0 (yes) in the description (that said FIXME.) The > patch fixes a typo-like error and has been checked with checkpatch.pl > in the scripts directory. > Here's the patch: It's great that you're sending patches, but it also helps the developers when the patches are correctly formatted. See: http://linux.yyz.us/patch-format.html Here's a good example: http://lkml.org/lkml/2012/8/2/286 1) the subject line starts with [PATCH] *and* the module being patches, then quickly describes the patch 2) There's a Signed-off-by comes *before* the patch data 3) The full path to the files being patched is in the patch header (ie, not just "main.c" but foo/bar/baz/main.c) Cheers, Dan > --- main.c 2012-07-21 20:58:29.000000000 +0000 > +++ mainnew.c 2012-08-05 20:00:37.000000000 +0000 > @@ -32,7 +32,7 @@ > #include "led.h" > #include "cfg.h" > #include "debugfs.h" > - > +#include "net/rfkill/rfkill.h" > static struct lock_class_key ieee80211_rx_skb_queue_class; > > void ieee80211_configure_filter(struct ieee80211_local *local) > @@ -183,6 +183,7 @@ int ieee80211_hw_config(struct ieee80211 > * > */ > /* WARN_ON(ret); */ > + WARN_ON(1) > } > > return ret; > Signed-off-by: Christopher P. Sacchi <chris.sacchi@gmail.com> > -- > -- 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
Very generic subject line. I would expect something like: [PATCH] mac80211: include rfkill header implicitly On 08/06/2012 01:49 PM, Christopher Sacchi wrote: > The source file needed a change that was told in the description to > know that the #include needed to be fixed, and the function changed > another value to 0 (yes) in the description (that said FIXME.) The > patch fixes a typo-like error and has been checked with checkpatch.pl > in the scripts directory. > Here's the patch: Do need a heads-up stating the obvious. > -- > --- main.c 2012-07-21 20:58:29.000000000 +0000 > +++ mainnew.c 2012-08-05 20:00:37.000000000 +0000 How did you create this patch? diff -U? When I try to apply your patch: $ git am patches/mac80211-crappy-patch-test.eml Applying: Mac80211 driver and I checked the patch error: mainnew.c: does not exist in index Patch failed at 0001 Mac80211 driver and I checked the patch When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort". You really should do your changes in a git repository. I changed it to: --- a/net/mac80211/main.c 2012-07-21 20:58:29.000000000 +0000 +++ b/net/mac80211/main.c 2012-08-05 20:00:37.000000000 +0000 and applied it. That applied, but it does not build. > @@ -32,7 +32,7 @@ > #include "led.h" > #include "cfg.h" > #include "debugfs.h" > - > +#include "net/rfkill/rfkill.h" add empty line between #include and function prototype. > static struct lock_class_key ieee80211_rx_skb_queue_class; > > void ieee80211_configure_filter(struct ieee80211_local *local) > @@ -183,6 +183,7 @@ int ieee80211_hw_config(struct ieee80211 > * > */ > /* WARN_ON(ret); */ > + WARN_ON(1) Still no ';' here. > } > > return ret; > Signed-off-by: Christopher P. Sacchi <chris.sacchi@gmail.com> Now this line will be added in the code. You really do not want that! (I hope). > -- > More general: What are you trying to change? I did take a look in net/mac80211/main.c and found the FIXME you refer to: int ieee80211_hw_config(struct ieee80211_local *local, u32 changed) { : : if (changed && local->open_count) { ret = drv_config(local, changed); /* * Goal: * HW reconfiguration should never fail, the driver has told * us what it can support so it should live up to that promise. * * Current status: * rfkill is not integrated with mac80211 and a * configuration command can thus fail if hardware rfkill * is enabled * * FIXME: integrate rfkill with mac80211 and then add this * WARN_ON() back * */ /* WARN_ON(ret); */ } return ret; } Back to your change: 1) You are a WARN_ON(1). So on every drv_config() call you get a kernel warning. Unlikely we want that. 2) For the warning to be put back, the comment says "integrate rfkill with mac80211". Adding an include of the rfkill header file does not really cover that statement. All in all you seem to forget a number of steps in Documentation/SubmittingPatches. Apart from passing checkpatch.pl it should first of all at least compile :-( Gr. AvS -- 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 08/06/2012 07:59 PM, Arend van Spriel wrote: > On 08/06/2012 01:49 PM, Christopher Sacchi wrote: >> > The source file needed a change that was told in the description to >> > know that the #include needed to be fixed, and the function changed >> > another value to 0 (yes) in the description (that said FIXME.) The >> > patch fixes a typo-like error and has been checked with checkpatch.pl >> > in the scripts directory. > >> > Here's the patch: > Do need a heads-up stating the obvious. I meant "Do not need" :-p Gr. AvS -- 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
--- main.c 2012-07-21 20:58:29.000000000 +0000 +++ mainnew.c 2012-08-05 20:00:37.000000000 +0000 @@ -32,7 +32,7 @@ #include "led.h" #include "cfg.h" #include "debugfs.h" - +#include "net/rfkill/rfkill.h" static struct lock_class_key ieee80211_rx_skb_queue_class; void ieee80211_configure_filter(struct ieee80211_local *local) @@ -183,6 +183,7 @@ int ieee80211_hw_config(struct ieee80211 * */ /* WARN_ON(ret); */ + WARN_ON(1) } return ret;
The source file needed a change that was told in the description to know that the #include needed to be fixed, and the function changed another value to 0 (yes) in the description (that said FIXME.) The patch fixes a typo-like error and has been checked with checkpatch.pl in the scripts directory. Here's the patch: -- Signed-off-by: Christopher P. Sacchi <chris.sacchi@gmail.com> --