diff mbox

[3/3] arm64: include: asm: atomic.h: use 'unsigned int' and 'atomic_t' instead of 'unsigned long' for atomic_clear_mask()

Message ID 52561269.60900@asianux.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chen Gang Oct. 10, 2013, 2:35 a.m. UTC
In current kernel wide source, for arm64, only s390 scsi drivers use
atomic_clear_mask(), now, s390 itself need use 'unsigned int' and
'atomic_t', so need match s390's atomic_clear_mask().


Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
 arch/arm64/include/asm/atomic.h |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

Comments

Will Deacon Oct. 10, 2013, 10:07 a.m. UTC | #1
On Thu, Oct 10, 2013 at 03:35:21AM +0100, Chen Gang wrote:
> In current kernel wide source, for arm64, only s390 scsi drivers use
> atomic_clear_mask(), now, s390 itself need use 'unsigned int' and
> 'atomic_t', so need match s390's atomic_clear_mask().
> 
> 
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
>  arch/arm64/include/asm/atomic.h |   13 +++++++------
>  1 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/atomic.h b/arch/arm64/include/asm/atomic.h
> index 8363644..58808fc 100644
> --- a/arch/arm64/include/asm/atomic.h
> +++ b/arch/arm64/include/asm/atomic.h
> @@ -126,16 +126,17 @@ static inline int atomic_cmpxchg(atomic_t *ptr, int old, int new)
>  	return oldval;
>  }
>  
> -static inline void atomic_clear_mask(unsigned long mask, unsigned long *addr)
> +static inline void atomic_clear_mask(unsigned int mask, atomic_t *ptr)
>  {
> -	unsigned long tmp, tmp2;
> +	unsigned int tmp;

Same comment here as for ARM; I think you want a signed int.

Will
Chen Gang Oct. 10, 2013, 11:03 a.m. UTC | #2
On 10/10/2013 06:07 PM, Will Deacon wrote:
> On Thu, Oct 10, 2013 at 03:35:21AM +0100, Chen Gang wrote:
>> In current kernel wide source, for arm64, only s390 scsi drivers use
>> atomic_clear_mask(), now, s390 itself need use 'unsigned int' and
>> 'atomic_t', so need match s390's atomic_clear_mask().
>>
>>
>> Signed-off-by: Chen Gang <gang.chen@asianux.com>
>> ---
>>  arch/arm64/include/asm/atomic.h |   13 +++++++------
>>  1 files changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/atomic.h b/arch/arm64/include/asm/atomic.h
>> index 8363644..58808fc 100644
>> --- a/arch/arm64/include/asm/atomic.h
>> +++ b/arch/arm64/include/asm/atomic.h
>> @@ -126,16 +126,17 @@ static inline int atomic_cmpxchg(atomic_t *ptr, int old, int new)
>>  	return oldval;
>>  }
>>  
>> -static inline void atomic_clear_mask(unsigned long mask, unsigned long *addr)
>> +static inline void atomic_clear_mask(unsigned int mask, atomic_t *ptr)
>>  {
>> -	unsigned long tmp, tmp2;
>> +	unsigned int tmp;
> 
> Same comment here as for ARM; I think you want a signed int.
> 

OK, replied in patch 2/3 for ARM.

BTW: do arm64 need atomic_clear_mask()?


> Will
> 
> 

Thanks.
Will Deacon Oct. 10, 2013, 2:23 p.m. UTC | #3
On Thu, Oct 10, 2013 at 12:03:52PM +0100, Chen Gang wrote:
> On 10/10/2013 06:07 PM, Will Deacon wrote:
> > On Thu, Oct 10, 2013 at 03:35:21AM +0100, Chen Gang wrote:
> >> In current kernel wide source, for arm64, only s390 scsi drivers use
> >> atomic_clear_mask(), now, s390 itself need use 'unsigned int' and
> >> 'atomic_t', so need match s390's atomic_clear_mask().
> >>
> >>
> >> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> >> ---
> >>  arch/arm64/include/asm/atomic.h |   13 +++++++------
> >>  1 files changed, 7 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/arch/arm64/include/asm/atomic.h b/arch/arm64/include/asm/atomic.h
> >> index 8363644..58808fc 100644
> >> --- a/arch/arm64/include/asm/atomic.h
> >> +++ b/arch/arm64/include/asm/atomic.h
> >> @@ -126,16 +126,17 @@ static inline int atomic_cmpxchg(atomic_t *ptr, int old, int new)
> >>  	return oldval;
> >>  }
> >>  
> >> -static inline void atomic_clear_mask(unsigned long mask, unsigned long *addr)
> >> +static inline void atomic_clear_mask(unsigned int mask, atomic_t *ptr)
> >>  {
> >> -	unsigned long tmp, tmp2;
> >> +	unsigned int tmp;
> > 
> > Same comment here as for ARM; I think you want a signed int.
> > 
> 
> OK, replied in patch 2/3 for ARM.
> 
> BTW: do arm64 need atomic_clear_mask()?

No. Neither ARM nor arm64 need this function.

Will
Chen Gang Oct. 11, 2013, 1:18 a.m. UTC | #4
On 10/10/2013 10:23 PM, Will Deacon wrote:
> On Thu, Oct 10, 2013 at 12:03:52PM +0100, Chen Gang wrote:
>> On 10/10/2013 06:07 PM, Will Deacon wrote:
>>> On Thu, Oct 10, 2013 at 03:35:21AM +0100, Chen Gang wrote:
>>>> In current kernel wide source, for arm64, only s390 scsi drivers use
>>>> atomic_clear_mask(), now, s390 itself need use 'unsigned int' and
>>>> 'atomic_t', so need match s390's atomic_clear_mask().
>>>>
>>>>
>>>> Signed-off-by: Chen Gang <gang.chen@asianux.com>
>>>> ---
>>>>  arch/arm64/include/asm/atomic.h |   13 +++++++------
>>>>  1 files changed, 7 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/include/asm/atomic.h b/arch/arm64/include/asm/atomic.h
>>>> index 8363644..58808fc 100644
>>>> --- a/arch/arm64/include/asm/atomic.h
>>>> +++ b/arch/arm64/include/asm/atomic.h
>>>> @@ -126,16 +126,17 @@ static inline int atomic_cmpxchg(atomic_t *ptr, int old, int new)
>>>>  	return oldval;
>>>>  }
>>>>  
>>>> -static inline void atomic_clear_mask(unsigned long mask, unsigned long *addr)
>>>> +static inline void atomic_clear_mask(unsigned int mask, atomic_t *ptr)
>>>>  {
>>>> -	unsigned long tmp, tmp2;
>>>> +	unsigned int tmp;
>>>
>>> Same comment here as for ARM; I think you want a signed int.
>>>
>>
>> OK, replied in patch 2/3 for ARM.
>>
>> BTW: do arm64 need atomic_clear_mask()?
> 
> No. Neither ARM nor arm64 need this function.
> 

OK, thank you for your confirmation.

Hmm... can we remove atomic_clear_mask() from ARM and arm64? (in my
opinion, if not need, better to remove it).

> Will
> 
> 

Thanks.
Will Deacon Oct. 11, 2013, 10:44 a.m. UTC | #5
On Fri, Oct 11, 2013 at 02:18:26AM +0100, Chen Gang wrote:
> On 10/10/2013 10:23 PM, Will Deacon wrote:
> > On Thu, Oct 10, 2013 at 12:03:52PM +0100, Chen Gang wrote:
> >> BTW: do arm64 need atomic_clear_mask()?
> > 
> > No. Neither ARM nor arm64 need this function.
> 
> OK, thank you for your confirmation.
> 
> Hmm... can we remove atomic_clear_mask() from ARM and arm64? (in my
> opinion, if not need, better to remove it).

Yes!

I mentioned this a few times already...

Will
Chen Gang Oct. 11, 2013, 11:25 a.m. UTC | #6
On 10/11/2013 06:44 PM, Will Deacon wrote:
> On Fri, Oct 11, 2013 at 02:18:26AM +0100, Chen Gang wrote:
>> On 10/10/2013 10:23 PM, Will Deacon wrote:
>>> On Thu, Oct 10, 2013 at 12:03:52PM +0100, Chen Gang wrote:
>>>> BTW: do arm64 need atomic_clear_mask()?
>>>
>>> No. Neither ARM nor arm64 need this function.
>>
>> OK, thank you for your confirmation.
>>
>> Hmm... can we remove atomic_clear_mask() from ARM and arm64? (in my
>> opinion, if not need, better to remove it).
> 
> Yes!
> 

OK, thanks.

> I mentioned this a few times already...
> 

Hmm... firstly, you mentioned "except architectures, this is only used
under s390" (so I reply we need follow s390), and then you mentioned
"they are useless for arm/arm64" (so I ask whether it can be removed).

Excuse me, as a 'programmer' (at least, I am a 'programmer'), when
programming (assume our discussing belongs to programming), I have to
consider precisely (especially for confirmation words).  ;-)

I will send one patch for removing it from arm and arm64.

Thanks.

> Will
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


Thanks.
diff mbox

Patch

diff --git a/arch/arm64/include/asm/atomic.h b/arch/arm64/include/asm/atomic.h
index 8363644..58808fc 100644
--- a/arch/arm64/include/asm/atomic.h
+++ b/arch/arm64/include/asm/atomic.h
@@ -126,16 +126,17 @@  static inline int atomic_cmpxchg(atomic_t *ptr, int old, int new)
 	return oldval;
 }
 
-static inline void atomic_clear_mask(unsigned long mask, unsigned long *addr)
+static inline void atomic_clear_mask(unsigned int mask, atomic_t *ptr)
 {
-	unsigned long tmp, tmp2;
+	unsigned int tmp;
+	unsigned long tmp2;
 
 	asm volatile("// atomic_clear_mask\n"
-"1:	ldxr	%0, %2\n"
-"	bic	%0, %0, %3\n"
-"	stxr	%w1, %0, %2\n"
+"1:	ldxr	%w0, %2\n"
+"	bic	%w0, %w0, %w3\n"
+"	stxr	%w1, %w0, %2\n"
 "	cbnz	%w1, 1b"
-	: "=&r" (tmp), "=&r" (tmp2), "+Q" (*addr)
+	: "=&r" (tmp), "=&r" (tmp2), "+Q" (ptr->counter)
 	: "Ir" (mask)
 	: "cc");
 }