Patchwork [11/12] Remove defines of DMA_XXBIT_MASK micro

login
register
mail settings
Submitter Yang Hongyang
Date Feb. 19, 2009, 6:54 a.m.
Message ID <499D0221.9090407@cn.fujitsu.com>
Download mbox | patch
Permalink /patch/23412/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Yang Hongyang - Feb. 19, 2009, 6:54 a.m.
Remove defines of DMA_XXBIT_MASK micro

Signed-off-by: Yang Hongyang<yanghy@cn.fujitsu.com>

---
 include/linux/dma-mapping.h |   20 +-------------------
 1 files changed, 1 insertions(+), 19 deletions(-)
Ingo Molnar - Feb. 19, 2009, 8:14 a.m.
* Yang Hongyang <yanghy@cn.fujitsu.com> wrote:

> Remove defines of DMA_XXBIT_MASK micro
> 
> Signed-off-by: Yang Hongyang<yanghy@cn.fujitsu.com>
> 
> ---
>  include/linux/dma-mapping.h |   20 +-------------------
>  1 files changed, 1 insertions(+), 19 deletions(-)
> 
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index f238c7e..70f9c63 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -13,27 +13,9 @@ enum dma_data_direction {
>  	DMA_NONE = 3,
>  };
>  
> +/*use DMA_BIT_MASK(n) within your driver instead of DMA_xxBIT_MASK*/
>  #define DMA_BIT_MASK(n)	(((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
>  
> -/*
> - * NOTE: do not use the below macros in new code and do not add new definitions
> - * here.
> - *
> - * Instead, just open-code DMA_BIT_MASK(n) within your driver
> - */
> -#define DMA_64BIT_MASK	DMA_BIT_MASK(64)
> -#define DMA_48BIT_MASK	DMA_BIT_MASK(48)
> -#define DMA_47BIT_MASK	DMA_BIT_MASK(47)
> -#define DMA_40BIT_MASK	DMA_BIT_MASK(40)
> -#define DMA_39BIT_MASK	DMA_BIT_MASK(39)
> -#define DMA_35BIT_MASK	DMA_BIT_MASK(35)
> -#define DMA_32BIT_MASK	DMA_BIT_MASK(32)
> -#define DMA_31BIT_MASK	DMA_BIT_MASK(31)
> -#define DMA_30BIT_MASK	DMA_BIT_MASK(30)
> -#define DMA_29BIT_MASK	DMA_BIT_MASK(29)
> -#define DMA_28BIT_MASK	DMA_BIT_MASK(28)
> -#define DMA_24BIT_MASK	DMA_BIT_MASK(24)

Looks good beyond the s/micro/macro typo fix, but i'd suggest to 
keep these old defines for one more kernel cycle, then do a 
final removal of all remaining uses, in a single patch.

That way we'll save ourselves from quite a bit of unnecessary 
build breakages (as your patchset shows there's still a lot of 
users of the old macros), as these definitions get moved around, 
reintroduced, etc.

Conflict resolution becomes easier as well - if such a patch 
conflicts with some ongoing work then we can by avoid the 
conflict by just dropping that hunk and delaying that particular 
conversion to the 'final' stage.

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yang Hongyang - Feb. 19, 2009, 8:56 a.m.
Ingo Molnar wrote:
> * Yang Hongyang <yanghy@cn.fujitsu.com> wrote:
> 
>> Remove defines of DMA_XXBIT_MASK micro
>>
>> Signed-off-by: Yang Hongyang<yanghy@cn.fujitsu.com>
>>
>> ---
>>  include/linux/dma-mapping.h |   20 +-------------------
>>  1 files changed, 1 insertions(+), 19 deletions(-)
>>
>> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
>> index f238c7e..70f9c63 100644
>> --- a/include/linux/dma-mapping.h
>> +++ b/include/linux/dma-mapping.h
>> @@ -13,27 +13,9 @@ enum dma_data_direction {
>>  	DMA_NONE = 3,
>>  };
>>  
>> +/*use DMA_BIT_MASK(n) within your driver instead of DMA_xxBIT_MASK*/
>>  #define DMA_BIT_MASK(n)	(((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
>>  
>> -/*
>> - * NOTE: do not use the below macros in new code and do not add new definitions
>> - * here.
>> - *
>> - * Instead, just open-code DMA_BIT_MASK(n) within your driver
>> - */
>> -#define DMA_64BIT_MASK	DMA_BIT_MASK(64)
>> -#define DMA_48BIT_MASK	DMA_BIT_MASK(48)
>> -#define DMA_47BIT_MASK	DMA_BIT_MASK(47)
>> -#define DMA_40BIT_MASK	DMA_BIT_MASK(40)
>> -#define DMA_39BIT_MASK	DMA_BIT_MASK(39)
>> -#define DMA_35BIT_MASK	DMA_BIT_MASK(35)
>> -#define DMA_32BIT_MASK	DMA_BIT_MASK(32)
>> -#define DMA_31BIT_MASK	DMA_BIT_MASK(31)
>> -#define DMA_30BIT_MASK	DMA_BIT_MASK(30)
>> -#define DMA_29BIT_MASK	DMA_BIT_MASK(29)
>> -#define DMA_28BIT_MASK	DMA_BIT_MASK(28)
>> -#define DMA_24BIT_MASK	DMA_BIT_MASK(24)
> 
> Looks good beyond the s/micro/macro typo fix, but i'd suggest to 
> keep these old defines for one more kernel cycle, then do a 
> final removal of all remaining uses, in a single patch.
> 
> That way we'll save ourselves from quite a bit of unnecessary 
> build breakages (as your patchset shows there's still a lot of 
> users of the old macros), as these definitions get moved around, 
> reintroduced, etc.
> 
> Conflict resolution becomes easier as well - if such a patch 
> conflicts with some ongoing work then we can by avoid the 
> conflict by just dropping that hunk and delaying that particular 
> conversion to the 'final' stage.
> 
> 	Ingo

That's a good idea,Seems it's nessary to keep these old
defines.
I will resend the patch with s/micro/macro typo fix and keep
these old defines,Thanks Ingo!~
Yang Hongyang - Feb. 19, 2009, 9:32 a.m.
v1->v2:fix s/micro/macro typo and keep the old defines
            of DMA_nBIT_MASK
----------------------
Replace all DMA_nBIT_MASK macro with the new DMA_BIT_MASK(n) macro

01:Replace all DMA_64BIT_MASK macro with DMA_BIT_MASK(64)
02:Replace all DMA_48BIT_MASK macro with DMA_BIT_MASK(48)
03:Replace all DMA_40BIT_MASK macro with DMA_BIT_MASK(40)
04:Replace all DMA_39BIT_MASK macro with DMA_BIT_MASK(39)
05:Replace all DMA_35BIT_MASK macro with DMA_BIT_MASK(35)
06:Replace all DMA_32BIT_MASK macro with DMA_BIT_MASK(32)
07:Replace all DMA_31BIT_MASK macro with DMA_BIT_MASK(31)
08:Replace all DMA_30BIT_MASK macro with DMA_BIT_MASK(30)
09:Replace all DMA_28BIT_MASK macro with DMA_BIT_MASK(28)
10:Replace all DMA_24BIT_MASK macro with DMA_BIT_MASK(24)
11:Update the old macro DMA_nBIT_MASK related documentations
Stefan Richter - Feb. 19, 2009, 12:47 p.m.
Yang Hongyang wrote:
> v1->v2:fix s/micro/macro typo and keep the old defines
>             of DMA_nBIT_MASK
> ----------------------
> Replace all DMA_nBIT_MASK macro with the new DMA_BIT_MASK(n) macro
> 
> 01:Replace all DMA_64BIT_MASK macro with DMA_BIT_MASK(64)
> 02:Replace all DMA_48BIT_MASK macro with DMA_BIT_MASK(48)
> 03:Replace all DMA_40BIT_MASK macro with DMA_BIT_MASK(40)
> 04:Replace all DMA_39BIT_MASK macro with DMA_BIT_MASK(39)
> 05:Replace all DMA_35BIT_MASK macro with DMA_BIT_MASK(35)
> 06:Replace all DMA_32BIT_MASK macro with DMA_BIT_MASK(32)
> 07:Replace all DMA_31BIT_MASK macro with DMA_BIT_MASK(31)
> 08:Replace all DMA_30BIT_MASK macro with DMA_BIT_MASK(30)
> 09:Replace all DMA_28BIT_MASK macro with DMA_BIT_MASK(28)
> 10:Replace all DMA_24BIT_MASK macro with DMA_BIT_MASK(24)
> 11:Update the old macro DMA_nBIT_MASK related documentations
> 

Shouldn't you organize the patch series per subsystem, not per old
macro?  And then Cc the respective maintainers?

As it stands, the patches cannot be routed through the normal channels;
yet there is no fundamental reason to handle these patches differently
from normal patches.
Ingo Molnar - Feb. 19, 2009, 3:08 p.m.
* Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:

> Yang Hongyang wrote:
> > v1->v2:fix s/micro/macro typo and keep the old defines
> >             of DMA_nBIT_MASK
> > ----------------------
> > Replace all DMA_nBIT_MASK macro with the new DMA_BIT_MASK(n) macro
> > 
> > 01:Replace all DMA_64BIT_MASK macro with DMA_BIT_MASK(64)
> > 02:Replace all DMA_48BIT_MASK macro with DMA_BIT_MASK(48)
> > 03:Replace all DMA_40BIT_MASK macro with DMA_BIT_MASK(40)
> > 04:Replace all DMA_39BIT_MASK macro with DMA_BIT_MASK(39)
> > 05:Replace all DMA_35BIT_MASK macro with DMA_BIT_MASK(35)
> > 06:Replace all DMA_32BIT_MASK macro with DMA_BIT_MASK(32)
> > 07:Replace all DMA_31BIT_MASK macro with DMA_BIT_MASK(31)
> > 08:Replace all DMA_30BIT_MASK macro with DMA_BIT_MASK(30)
> > 09:Replace all DMA_28BIT_MASK macro with DMA_BIT_MASK(28)
> > 10:Replace all DMA_24BIT_MASK macro with DMA_BIT_MASK(24)
> > 11:Update the old macro DMA_nBIT_MASK related documentations
> > 
> 
> Shouldn't you organize the patch series per subsystem, not per old
> macro?  And then Cc the respective maintainers?
> 
> As it stands, the patches cannot be routed through the normal channels;
> yet there is no fundamental reason to handle these patches differently
> from normal patches.

Traditionally such trivially correct convert-it-all patches 
lived in -mm and were merged upstream in one go, near the end of 
the merge window.

Sprinkling it into dozens of subsystem channels (some of which 
are very unreliable) is neither good nor an economic use of our 
resources.

Patches that can potentially cause trouble should go via the 
usual channels.

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrew Morton - Feb. 19, 2009, 11:14 p.m.
On Thu, 19 Feb 2009 16:08:51 +0100
Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> 
> > Yang Hongyang wrote:
> > > v1->v2:fix s/micro/macro typo and keep the old defines
> > >             of DMA_nBIT_MASK
> > > ----------------------
> > > Replace all DMA_nBIT_MASK macro with the new DMA_BIT_MASK(n) macro
> > > 
> > > 01:Replace all DMA_64BIT_MASK macro with DMA_BIT_MASK(64)
> > > 02:Replace all DMA_48BIT_MASK macro with DMA_BIT_MASK(48)
> > > 03:Replace all DMA_40BIT_MASK macro with DMA_BIT_MASK(40)
> > > 04:Replace all DMA_39BIT_MASK macro with DMA_BIT_MASK(39)
> > > 05:Replace all DMA_35BIT_MASK macro with DMA_BIT_MASK(35)
> > > 06:Replace all DMA_32BIT_MASK macro with DMA_BIT_MASK(32)
> > > 07:Replace all DMA_31BIT_MASK macro with DMA_BIT_MASK(31)
> > > 08:Replace all DMA_30BIT_MASK macro with DMA_BIT_MASK(30)
> > > 09:Replace all DMA_28BIT_MASK macro with DMA_BIT_MASK(28)
> > > 10:Replace all DMA_24BIT_MASK macro with DMA_BIT_MASK(24)
> > > 11:Update the old macro DMA_nBIT_MASK related documentations
> > > 
> > 
> > Shouldn't you organize the patch series per subsystem, not per old
> > macro?  And then Cc the respective maintainers?
> > 
> > As it stands, the patches cannot be routed through the normal channels;
> > yet there is no fundamental reason to handle these patches differently
> > from normal patches.
> 
> Traditionally such trivially correct convert-it-all patches 
> lived in -mm and were merged upstream in one go, near the end of 
> the merge window.
> 
> Sprinkling it into dozens of subsystem channels (some of which 
> are very unreliable) is neither good nor an economic use of our 
> resources.
> 
> Patches that can potentially cause trouble should go via the 
> usual channels.
> 

yes, fun.

I hit several rejects merging these, easily fixed.  After this lot is
merged there will probably be a few unconverted sites which will need a
second pass.  After that we can think about removing the old #defines.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ingo Molnar - Feb. 20, 2009, 10:29 a.m.
* Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu, 19 Feb 2009 16:08:51 +0100
> Ingo Molnar <mingo@elte.hu> wrote:
> 
> > 
> > * Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> > 
> > > Yang Hongyang wrote:
> > > > v1->v2:fix s/micro/macro typo and keep the old defines
> > > >             of DMA_nBIT_MASK
> > > > ----------------------
> > > > Replace all DMA_nBIT_MASK macro with the new DMA_BIT_MASK(n) macro
> > > > 
> > > > 01:Replace all DMA_64BIT_MASK macro with DMA_BIT_MASK(64)
> > > > 02:Replace all DMA_48BIT_MASK macro with DMA_BIT_MASK(48)
> > > > 03:Replace all DMA_40BIT_MASK macro with DMA_BIT_MASK(40)
> > > > 04:Replace all DMA_39BIT_MASK macro with DMA_BIT_MASK(39)
> > > > 05:Replace all DMA_35BIT_MASK macro with DMA_BIT_MASK(35)
> > > > 06:Replace all DMA_32BIT_MASK macro with DMA_BIT_MASK(32)
> > > > 07:Replace all DMA_31BIT_MASK macro with DMA_BIT_MASK(31)
> > > > 08:Replace all DMA_30BIT_MASK macro with DMA_BIT_MASK(30)
> > > > 09:Replace all DMA_28BIT_MASK macro with DMA_BIT_MASK(28)
> > > > 10:Replace all DMA_24BIT_MASK macro with DMA_BIT_MASK(24)
> > > > 11:Update the old macro DMA_nBIT_MASK related documentations
> > > > 
> > > 
> > > Shouldn't you organize the patch series per subsystem, not per old
> > > macro?  And then Cc the respective maintainers?
> > > 
> > > As it stands, the patches cannot be routed through the normal channels;
> > > yet there is no fundamental reason to handle these patches differently
> > > from normal patches.
> > 
> > Traditionally such trivially correct convert-it-all patches 
> > lived in -mm and were merged upstream in one go, near the end of 
> > the merge window.
> > 
> > Sprinkling it into dozens of subsystem channels (some of which 
> > are very unreliable) is neither good nor an economic use of our 
> > resources.
> > 
> > Patches that can potentially cause trouble should go via the 
> > usual channels.
> > 
> 
> yes, fun.
> 
> I hit several rejects merging these, easily fixed.  After this 
> lot is merged there will probably be a few unconverted sites 
> which will need a second pass.  After that we can think about 
> removing the old #defines.

Yeah. I wanted to suggest for you to _drop_ all conflicts - 
because the old defines still live.

That makes it the easiest for you to keep it all merged up in 
the future too with minimum fuss, and we need a second pass 
anyway so dropping a few hunks is no big issue.

[ But since you merged it up that's fine too - it's just that 
  code that got changed recently and created conflicts has a 
  higher chance of being changed in the near future too - and 
  causing you ongoing conflicts in that area, in the next ~1.5 
  months until the next merge window closes. ]

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index f238c7e..70f9c63 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -13,27 +13,9 @@  enum dma_data_direction {
 	DMA_NONE = 3,
 };
 
+/*use DMA_BIT_MASK(n) within your driver instead of DMA_xxBIT_MASK*/
 #define DMA_BIT_MASK(n)	(((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
 
-/*
- * NOTE: do not use the below macros in new code and do not add new definitions
- * here.
- *
- * Instead, just open-code DMA_BIT_MASK(n) within your driver
- */
-#define DMA_64BIT_MASK	DMA_BIT_MASK(64)
-#define DMA_48BIT_MASK	DMA_BIT_MASK(48)
-#define DMA_47BIT_MASK	DMA_BIT_MASK(47)
-#define DMA_40BIT_MASK	DMA_BIT_MASK(40)
-#define DMA_39BIT_MASK	DMA_BIT_MASK(39)
-#define DMA_35BIT_MASK	DMA_BIT_MASK(35)
-#define DMA_32BIT_MASK	DMA_BIT_MASK(32)
-#define DMA_31BIT_MASK	DMA_BIT_MASK(31)
-#define DMA_30BIT_MASK	DMA_BIT_MASK(30)
-#define DMA_29BIT_MASK	DMA_BIT_MASK(29)
-#define DMA_28BIT_MASK	DMA_BIT_MASK(28)
-#define DMA_24BIT_MASK	DMA_BIT_MASK(24)
-
 #define DMA_MASK_NONE	0x0ULL
 
 static inline int valid_dma_direction(int dma_direction)