2015-08-19 05:31:29

by Vaishali Thakkar

[permalink] [raw]
Subject: [PATCH] usb: gadget: f_uac1: Convert use of __constant_cpu_to_le16 to cpu_to_le16

In big endian cases, macro cpu_to_le16 unfolds to __swab16 which
provides special case for constants. In little endian cases,
__constant_cpu_to_le16 and cpu_to_le16 expand directly to the
same expression. So, replace __constant_cpu_to_le16 with
cpu_to_le16 with the goal of getting rid of the definition of
__constant_cpu_to_le16 completely.

The semantic patch that performs this transformation is as follows:

@@expression x;@@

- __constant_cpu_to_le16(x)
+ cpu_to_le16(x)

Signed-off-by: Vaishali Thakkar <[email protected]>
---
drivers/usb/gadget/function/f_uac1.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/f_uac1.c b/drivers/usb/gadget/function/f_uac1.c
index 7856b33..5aa8d8a 100644
--- a/drivers/usb/gadget/function/f_uac1.c
+++ b/drivers/usb/gadget/function/f_uac1.c
@@ -57,8 +57,8 @@ static struct uac1_ac_header_descriptor_1 ac_header_desc = {
.bLength = UAC_DT_AC_HEADER_LENGTH,
.bDescriptorType = USB_DT_CS_INTERFACE,
.bDescriptorSubtype = UAC_HEADER,
- .bcdADC = __constant_cpu_to_le16(0x0100),
- .wTotalLength = __constant_cpu_to_le16(UAC_DT_TOTAL_LENGTH),
+ .bcdADC = cpu_to_le16(0x0100),
+ .wTotalLength = cpu_to_le16(UAC_DT_TOTAL_LENGTH),
.bInCollection = F_AUDIO_NUM_INTERFACES,
.baInterfaceNr = {
/* Interface number of the first AudioStream interface */
@@ -186,7 +186,7 @@ static struct uac_iso_endpoint_descriptor as_iso_out_desc = {
.bDescriptorSubtype = UAC_EP_GENERAL,
.bmAttributes = 1,
.bLockDelayUnits = 1,
- .wLockDelay = __constant_cpu_to_le16(1),
+ .wLockDelay = cpu_to_le16(1),
};

static struct usb_descriptor_header *f_audio_desc[] = {
--
1.9.1


2015-08-20 10:53:45

by David Laight

[permalink] [raw]
Subject: RE: [PATCH] usb: gadget: f_uac1: Convert use of __constant_cpu_to_le16 to cpu_to_le16

From: Vaishali Thakkar
> Sent: 19 August 2015 06:31
> To: Felipe Balbi
> Cc: Greg Kroah-Hartman; [email protected]; [email protected]
> Subject: [PATCH] usb: gadget: f_uac1: Convert use of __constant_cpu_to_le16 to cpu_to_le16
>
> In big endian cases, macro cpu_to_le16 unfolds to __swab16 which
> provides special case for constants. In little endian cases,
> __constant_cpu_to_le16 and cpu_to_le16 expand directly to the
> same expression. So, replace __constant_cpu_to_le16 with
> cpu_to_le16 with the goal of getting rid of the definition of
> __constant_cpu_to_le16 completely.
>
> The semantic patch that performs this transformation is as follows:
>
> @@expression x;@@
>
> - __constant_cpu_to_le16(x)
> + cpu_to_le16(x)
>
> Signed-off-by: Vaishali Thakkar <[email protected]>
> ---
> drivers/usb/gadget/function/f_uac1.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_uac1.c b/drivers/usb/gadget/function/f_uac1.c
> index 7856b33..5aa8d8a 100644
> --- a/drivers/usb/gadget/function/f_uac1.c
> +++ b/drivers/usb/gadget/function/f_uac1.c
> @@ -57,8 +57,8 @@ static struct uac1_ac_header_descriptor_1 ac_header_desc = {
> .bLength = UAC_DT_AC_HEADER_LENGTH,
> .bDescriptorType = USB_DT_CS_INTERFACE,
> .bDescriptorSubtype = UAC_HEADER,
> - .bcdADC = __constant_cpu_to_le16(0x0100),
> - .wTotalLength = __constant_cpu_to_le16(UAC_DT_TOTAL_LENGTH),
> + .bcdADC = cpu_to_le16(0x0100),
> + .wTotalLength = cpu_to_le16(UAC_DT_TOTAL_LENGTH),

Have you test compiled this on a big-endian system?
My gut feeling is that is fails.

David

2015-08-22 01:57:30

by Vaishali Thakkar

[permalink] [raw]
Subject: Re: [PATCH] usb: gadget: f_uac1: Convert use of __constant_cpu_to_le16 to cpu_to_le16

On Thu, Aug 20, 2015 at 4:20 PM, David Laight <[email protected]> wrote:
> From: Vaishali Thakkar
>> Sent: 19 August 2015 06:31
>> To: Felipe Balbi
>> Cc: Greg Kroah-Hartman; [email protected]; [email protected]
>> Subject: [PATCH] usb: gadget: f_uac1: Convert use of __constant_cpu_to_le16 to cpu_to_le16
>>
>> In big endian cases, macro cpu_to_le16 unfolds to __swab16 which
>> provides special case for constants. In little endian cases,
>> __constant_cpu_to_le16 and cpu_to_le16 expand directly to the
>> same expression. So, replace __constant_cpu_to_le16 with
>> cpu_to_le16 with the goal of getting rid of the definition of
>> __constant_cpu_to_le16 completely.
>>
>> The semantic patch that performs this transformation is as follows:
>>
>> @@expression x;@@
>>
>> - __constant_cpu_to_le16(x)
>> + cpu_to_le16(x)
>>
>> Signed-off-by: Vaishali Thakkar <[email protected]>
>> ---
>> drivers/usb/gadget/function/f_uac1.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_uac1.c b/drivers/usb/gadget/function/f_uac1.c
>> index 7856b33..5aa8d8a 100644
>> --- a/drivers/usb/gadget/function/f_uac1.c
>> +++ b/drivers/usb/gadget/function/f_uac1.c
>> @@ -57,8 +57,8 @@ static struct uac1_ac_header_descriptor_1 ac_header_desc = {
>> .bLength = UAC_DT_AC_HEADER_LENGTH,
>> .bDescriptorType = USB_DT_CS_INTERFACE,
>> .bDescriptorSubtype = UAC_HEADER,
>> - .bcdADC = __constant_cpu_to_le16(0x0100),
>> - .wTotalLength = __constant_cpu_to_le16(UAC_DT_TOTAL_LENGTH),
>> + .bcdADC = cpu_to_le16(0x0100),
>> + .wTotalLength = cpu_to_le16(UAC_DT_TOTAL_LENGTH),
>
> Have you test compiled this on a big-endian system?
> My gut feeling is that is fails.

No. I have tested it on little-endian system only. But I'll
be really surprised if this will fail. Can you please tell me
if I am missing something in this particular case or same
applies for other cases because most of the cases like
__constant_<foo> are already converted to <foo>?

As far as I know, if the argument is a constant the
conversion happens at compile time. And unfolding both
definitions returns to same expression. Still I am trying if
someone can test it for me on big endian system.

> David
>



--
Vaishali

2015-08-24 09:01:19

by David Laight

[permalink] [raw]
Subject: RE: [PATCH] usb: gadget: f_uac1: Convert use of __constant_cpu_to_le16 to cpu_to_le16

From: Vaishali Thakkar [mailto:[email protected]]
> Sent: 22 August 2015 02:57
...
> >> - .bcdADC = __constant_cpu_to_le16(0x0100),
> >> - .wTotalLength = __constant_cpu_to_le16(UAC_DT_TOTAL_LENGTH),
> >> + .bcdADC = cpu_to_le16(0x0100),
> >> + .wTotalLength = cpu_to_le16(UAC_DT_TOTAL_LENGTH),
> >
> > Have you test compiled this on a big-endian system?
> > My gut feeling is that is fails.
>
> No. I have tested it on little-endian system only. But I'll
> be really surprised if this will fail. Can you please tell me
> if I am missing something in this particular case or same
> applies for other cases because most of the cases like
> __constant_<foo> are already converted to <foo>?
>
> As far as I know, if the argument is a constant the
> conversion happens at compile time. And unfolding both
> definitions returns to same expression. Still I am trying if
> someone can test it for me on big endian system.

Flip one to cpu_to_be16() and see if it still compiles.

Static initialisers and case labels can be expressions, but the
expression itself must only contain constants.
So it needs to be constant regardless of the value of any constants.
If it contains 'a ? t : f' then both 't' and 'f' must be constant.

In code, if 'a' is constant the optimiser discards one of 't' or 'f'.
I'm not sure what happens for non-static initialisers (they generate
odd code at the best of times).

David

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2015-08-24 10:42:31

by Vaishali Thakkar

[permalink] [raw]
Subject: Re: [PATCH] usb: gadget: f_uac1: Convert use of __constant_cpu_to_le16 to cpu_to_le16

On Mon, Aug 24, 2015 at 2:29 PM, David Laight <[email protected]> wrote:
> From: Vaishali Thakkar [mailto:[email protected]]
>> Sent: 22 August 2015 02:57
> ...
>> >> - .bcdADC = __constant_cpu_to_le16(0x0100),
>> >> - .wTotalLength = __constant_cpu_to_le16(UAC_DT_TOTAL_LENGTH),
>> >> + .bcdADC = cpu_to_le16(0x0100),
>> >> + .wTotalLength = cpu_to_le16(UAC_DT_TOTAL_LENGTH),
>> >
>> > Have you test compiled this on a big-endian system?
>> > My gut feeling is that is fails.
>>
>> No. I have tested it on little-endian system only. But I'll
>> be really surprised if this will fail. Can you please tell me
>> if I am missing something in this particular case or same
>> applies for other cases because most of the cases like
>> __constant_<foo> are already converted to <foo>?
>>
>> As far as I know, if the argument is a constant the
>> conversion happens at compile time. And unfolding both
>> definitions returns to same expression. Still I am trying if
>> someone can test it for me on big endian system.
>
> Flip one to cpu_to_be16() and see if it still compiles.

Yes. It still compiles.

> Static initialisers and case labels can be expressions, but the
> expression itself must only contain constants.
> So it needs to be constant regardless of the value of any constants.
> If it contains 'a ? t : f' then both 't' and 'f' must be constant.
>
> In code, if 'a' is constant the optimiser discards one of 't' or 'f'.
> I'm not sure what happens for non-static initialisers (they generate
> odd code at the best of times).
>
> David
>



--
Vaishali