Re: [PATCH] drivers: staging: lustre: lustre: include: add "__attribute__((packed))" for the related union
On Sat, Feb 01, 2014 at 09:57:39PM +0800, Chen Gang wrote:
> It seems, our kernel still stick to treate 'pack' region have effect
> with both 'align' and 'sizeof'.
>
It's not about packed regions. It's about unions. It's saying the
sizeof() a union is a multiple of 4 unless it's packed.
union foo {
short x;
short y;
};
The author intended the sizeof(union foo) to be 2 but on metag arch then
it is 4.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers:staging:tl8821ae Fixed the size of array to macro
On Sat, Feb 01, 2014 at 03:43:27PM -0800, Surendra Patil wrote:
> Changed array size from "bb_swing_idx_ofdm[2]" to
> "bb_swing_idx_ofdm[MAX_RF_PATH]"
> as discussed on thread - https://lkml.org/lkml/2014/2/1/75
>
Changelog sucks. We don't want to have to follow the link. Add a
Reported-by: Linus Torvalds
regards,
dan carpenter
> Signed-off-by: Surendra Patil
> ---
> drivers/staging/rtl8821ae/wifi.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8821ae/wifi.h
> b/drivers/staging/rtl8821ae/wifi.h
> index cfe88a1..76bef93 100644
> --- a/drivers/staging/rtl8821ae/wifi.h
> +++ b/drivers/staging/rtl8821ae/wifi.h
> @@ -1414,7 +1414,7 @@ struct rtl_dm {
>
>
> /*88e tx power tracking*/
> - u8 bb_swing_idx_ofdm[2];
> + u8 bb_swing_idx_ofdm[MAX_RF_PATH];
> u8 bb_swing_idx_ofdm_current;
> u8 bb_swing_idx_ofdm_base[MAX_RF_PATH];
> bool bb_swing_flag_Ofdm;
> --
> 1.8.3.2
>
> ___
> devel mailing list
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: r8188eu: Fix typo in USB_DEVICE list
On Sun, Feb 02, 2014 at 02:07:06PM -0600, Larry Finger wrote:
> There is a typo in the device list that interchanges the vendor and
> product codes for one of the entries.
>
> Signed-off-by: Larry Finger
> ---
> drivers/staging/rtl8188eu/os_dep/usb_intf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
> b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
> index 0a341d6..e9e3c76 100644
> --- a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
> +++ b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
> @@ -53,7 +53,7 @@ static struct usb_device_id rtw_usb_id_tbl[] = {
> {USB_DEVICE(USB_VENDER_ID_REALTEK, 0x0179)}, /* 8188ETV */
> /*=== Customer ID ===*/
> /** 8188EUS /
> - {USB_DEVICE(0x8179, 0x07B8)}, /* Abocom - Abocom */
> + {USB_DEVICE(0x07bb, 0x8179)}, /* Abocom - Abocom */
^^
Should this be 0x07b8?
regards,
dan carpenter
> {USB_DEVICE(0x2001, 0x330F)}, /* DLink DWA-125 REV D1 */
> {} /* Terminating entry */
> };
> --
> 1.8.4
>
> ___
> devel mailing list
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: lustre: include: add "__attribute__((packed))" for the related union
On 02/03/2014 04:58 PM, Dan Carpenter wrote:
> On Sat, Feb 01, 2014 at 09:57:39PM +0800, Chen Gang wrote:
>> It seems, our kernel still stick to treate 'pack' region have effect
>> with both 'align' and 'sizeof'.
>>
>
> It's not about packed regions. It's about unions. It's saying the
> sizeof() a union is a multiple of 4 unless it's packed.
>
> union foo {
> short x;
> short y;
> };
>
> The author intended the sizeof(union foo) to be 2 but on metag arch then
> it is 4.
>
Yeah, just like your original discussion. :-)
Hmm... can we say: "for metag compiler, in a pack region, it considers
variables alignment, but does not consider about struct/union alignment
(except append packed to each related struct/union)".
For compatible (consider about its ABI), it has to keep this features,
but for kernel, it needs be changed.
So, I suggest to add one parameter to compiler to switch this feature,
and append this parameter to KBUILD_CFLAGS in "arch/metag/Makefile"
which can satisfy both ABI and kernel.
Thanks.
--
Chen Gang
Open, share and attitude like air, water and life which God blessed
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] drivers: staging: lustre: lustre: include: add "__attribute__((packed))" for the related union
From: Dan Carpenter
> On Sat, Feb 01, 2014 at 09:57:39PM +0800, Chen Gang wrote:
> > It seems, our kernel still stick to treate 'pack' region have effect
> > with both 'align' and 'sizeof'.
>
> It's not about packed regions. It's about unions. It's saying the
> sizeof() a union is a multiple of 4 unless it's packed.
>
> union foo {
> short x;
> short y;
> };
>
> The author intended the sizeof(union foo) to be 2 but on metag arch then
> it is 4.
The same is probably be true of: struct foo { _u16 bar; };
Architectures that define such alignment rules are a right PITA.
You either need to get the size to 2 without using 'packed', or
just not define such structures.
It is worth seeing if adding aligned(2) will change the size - I'm
not sure.
David
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: lustre: include: add "__attribute__((packed))" for the related union
On 03/02/14 10:05, David Laight wrote:
> From: Dan Carpenter
>> On Sat, Feb 01, 2014 at 09:57:39PM +0800, Chen Gang wrote:
>>> It seems, our kernel still stick to treate 'pack' region have effect
>>> with both 'align' and 'sizeof'.
>>
>> It's not about packed regions. It's about unions. It's saying the
>> sizeof() a union is a multiple of 4 unless it's packed.
>>
>> union foo {
>> short x;
>> short y;
>> };
>>
>> The author intended the sizeof(union foo) to be 2 but on metag arch then
>> it is 4.
>
> The same is probably be true of: struct foo { _u16 bar; };
Yes indeed.
> Architectures that define such alignment rules are a right PITA.
> You either need to get the size to 2 without using 'packed', or
> just not define such structures.
> It is worth seeing if adding aligned(2) will change the size - I'm
> not sure.
__aligned(2) alone doesn't seem to have any effect on sizeof() or
__alignof__() unless it is accompanied by __packed. x86_64 is similar in
that respect (it just packs sanely in the first place).
Combining __packed with __aligned(2) does the trick though (__packed
alone sets __aligned(1) which is obviously going to be suboptimal).
Cheers
James
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: lustre: include: add "__attribute__((packed))" for the related union
On 02/03/2014 06:05 PM, David Laight wrote:
> From: Dan Carpenter
>> On Sat, Feb 01, 2014 at 09:57:39PM +0800, Chen Gang wrote:
>>> It seems, our kernel still stick to treate 'pack' region have effect
>>> with both 'align' and 'sizeof'.
>>
>> It's not about packed regions. It's about unions. It's saying the
>> sizeof() a union is a multiple of 4 unless it's packed.
>>
>> union foo {
>> short x;
>> short y;
>> };
>>
>> The author intended the sizeof(union foo) to be 2 but on metag arch then
>> it is 4.
>
> The same is probably be true of: struct foo { _u16 bar; };
>
I guess so.
> Architectures that define such alignment rules are a right PITA.
Sorry, I do not know about PITA (after google or wiki, I can not get
more related information).
Could you provide more information about PITA, thanks?
> You either need to get the size to 2 without using 'packed', or
> just not define such structures.
Excuse me, I don't quite understand your meaning. I guess your meaning
is:
"normally, we should not use a struct/union like that, no matter what it is
(2 or 4)".
Is it correct.
> It is worth seeing if adding aligned(2) will change the size - I'm
> not sure.
>
Yes, it will/should make sure that it must be 2.
Thanks.
--
Chen Gang
Open, share and attitude like air, water and life which God blessed
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: lustre: include: add "__attribute__((packed))" for the related union
On 02/03/2014 06:22 PM, James Hogan wrote:
> On 03/02/14 10:05, David Laight wrote:
>> From: Dan Carpenter
>>> On Sat, Feb 01, 2014 at 09:57:39PM +0800, Chen Gang wrote:
It seems, our kernel still stick to treate 'pack' region have effect
with both 'align' and 'sizeof'.
>>>
>>> It's not about packed regions. It's about unions. It's saying the
>>> sizeof() a union is a multiple of 4 unless it's packed.
>>>
>>> union foo {
>>> short x;
>>> short y;
>>> };
>>>
>>> The author intended the sizeof(union foo) to be 2 but on metag arch then
>>> it is 4.
>>
>> The same is probably be true of: struct foo { _u16 bar; };
>
> Yes indeed.
>
>> Architectures that define such alignment rules are a right PITA.
>> You either need to get the size to 2 without using 'packed', or
>> just not define such structures.
>> It is worth seeing if adding aligned(2) will change the size - I'm
>> not sure.
>
> __aligned(2) alone doesn't seem to have any effect on sizeof() or
> __alignof__() unless it is accompanied by __packed. x86_64 is similar in
> that respect (it just packs sanely in the first place).
>
> Combining __packed with __aligned(2) does the trick though (__packed
> alone sets __aligned(1) which is obviously going to be suboptimal).
>
Oh, thank you for your explanation.
And hope this feature issue can be fixed, and satisfy both kernel and
ABI. :-)
Thanks.
--
Chen Gang
Open, share and attitude like air, water and life which God blessed
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] drivers: staging: lustre: lustre: include: add "__attribute__((packed))" for the related union
From: James Hogan
> On 03/02/14 10:05, David Laight wrote:
> > From: Dan Carpenter
> >> On Sat, Feb 01, 2014 at 09:57:39PM +0800, Chen Gang wrote:
> >>> It seems, our kernel still stick to treate 'pack' region have effect
> >>> with both 'align' and 'sizeof'.
> >>
> >> It's not about packed regions. It's about unions. It's saying the
> >> sizeof() a union is a multiple of 4 unless it's packed.
> >>
> >> union foo {
> >>short x;
> >>short y;
> >> };
> >>
> >> The author intended the sizeof(union foo) to be 2 but on metag arch then
> >> it is 4.
> >
> > The same is probably be true of: struct foo { _u16 bar; };
>
> Yes indeed.
>
> > Architectures that define such alignment rules are a right PITA.
> > You either need to get the size to 2 without using 'packed', or
> > just not define such structures.
> > It is worth seeing if adding aligned(2) will change the size - I'm
> > not sure.
>
> __aligned(2) alone doesn't seem to have any effect on sizeof() or
> __alignof__() unless it is accompanied by __packed. x86_64 is similar in
> that respect (it just packs sanely in the first place).
>
> Combining __packed with __aligned(2) does the trick though (__packed
> alone sets __aligned(1) which is obviously going to be suboptimal).
Compile some code for a cpu that doesn't support misaligned transfers
(probably one of sparc, arm, ppc) and see if the compiler generates a
single 16bit request or two 8 bits ones.
You don't want the compiler generating multiple byte-sized memory transfers.
David
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: lustre: include: add "__attribute__((packed))" for the related union
On 03/02/14 10:35, David Laight wrote:
> From: James Hogan
>> On 03/02/14 10:05, David Laight wrote:
>>> Architectures that define such alignment rules are a right PITA.
>>> You either need to get the size to 2 without using 'packed', or
>>> just not define such structures.
>>> It is worth seeing if adding aligned(2) will change the size - I'm
>>> not sure.
>>
>> __aligned(2) alone doesn't seem to have any effect on sizeof() or
>> __alignof__() unless it is accompanied by __packed. x86_64 is similar in
>> that respect (it just packs sanely in the first place).
>>
>> Combining __packed with __aligned(2) does the trick though (__packed
>> alone sets __aligned(1) which is obviously going to be suboptimal).
>
> Compile some code for a cpu that doesn't support misaligned transfers
> (probably one of sparc, arm, ppc) and see if the compiler generates a
> single 16bit request or two 8 bits ones.
> You don't want the compiler generating multiple byte-sized memory transfers.
Meta is also one of those arches, and according to my quick tests,
__packed alone does correctly make it fall back to byte loads/stores,
but with __packed __aligned(2) it uses 16bit loads/stores. I've also
confirmed that with an ARM toolchain (see below for example).
Cheers
James
input:
#define __aligned(x)__attribute__((aligned(x)))
#define __packed__attribute__((packed))
union a {
short x, y;
} __aligned(2) __packed;
struct b {
short x;
} __aligned(2) __packed;
unsigned int soa = sizeof(union a);
unsigned int aoa = __alignof__(union a);
unsigned int sob = sizeof(struct b);
unsigned int aob = __alignof__(struct b);
void t(struct b *x, union a *y)
{
++x->x;
++y->x;
}
ARM output (-O2):
.cpu arm10tdmi
.fpu softvfp
.eabi_attribute 20, 1
.eabi_attribute 21, 1
.eabi_attribute 23, 3
.eabi_attribute 24, 1
.eabi_attribute 25, 1
.eabi_attribute 26, 2
.eabi_attribute 30, 2
.eabi_attribute 34, 0
.eabi_attribute 18, 4
.file "alignment4.c"
.text
.align 2
.global t
.type t, %function
t:
@ args = 0, pretend = 0, frame = 0
@ frame_needed = 0, uses_anonymous_args = 0
@ link register save eliminated.
ldrhr3, [r0, #0]
add r3, r3, #1
strhr3, [r0, #0]@ movhi
ldrhr3, [r1, #0]
add r3, r3, #1
strhr3, [r1, #0]@ movhi
bx lr
.size t, .-t
.global aob
.global sob
.global aoa
.global soa
.data
.align 2
.type aob, %object
.size aob, 4
aob:
.word 2
.type sob, %object
.size sob, 4
sob:
.word 2
.type aoa, %object
.size aoa, 4
aoa:
.word 2
.type soa, %object
.size soa, 4
soa:
.word 2
.ident "GCC: (GNU) 4.7.1 20120606 (Red Hat 4.7.1-0.1.20120606)"
.section.note.GNU-stack,"",%progbits
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: lustre: include: add "__attribute__((packed))" for the related union
On 02/03/2014 06:03 PM, Chen Gang wrote:
> On 02/03/2014 04:58 PM, Dan Carpenter wrote:
>> On Sat, Feb 01, 2014 at 09:57:39PM +0800, Chen Gang wrote:
>>> It seems, our kernel still stick to treate 'pack' region have effect
>>> with both 'align' and 'sizeof'.
>>>
>>
>> It's not about packed regions. It's about unions. It's saying the
>> sizeof() a union is a multiple of 4 unless it's packed.
>>
>> union foo {
>> short x;
>> short y;
>> };
>>
>> The author intended the sizeof(union foo) to be 2 but on metag arch then
>> it is 4.
>>
>
> Yeah, just like your original discussion. :-)
>
>
> Hmm... can we say: "for metag compiler, in a pack region, it considers
> variables alignment, but does not consider about struct/union alignment
> (except append packed to each related struct/union)".
>
> For compatible (consider about its ABI), it has to keep this features,
> but for kernel, it needs be changed.
>
> So, I suggest to add one parameter to compiler to switch this feature,
> and append this parameter to KBUILD_CFLAGS in "arch/metag/Makefile"
> which can satisfy both ABI and kernel.
>
After append the parameter to KBUILD_CFLAGS in "arch/metag/Makefile",
- I guess/assume "include/uapi/*" should/will not need be modified.
- but need check all files in "arch/metag/include/uapi/*".
(add padding data for packed struct/union when __KERNEL__ defined)
- maybe we have to process metag related ABI which not in "*/uapi/*"
(add padding data for packed struct/union when __KERNEL__ defined)
and when we find them, recommend to move all of them to "*/uapi/*".
Sorry, I don't know whether this way is the best way or not, but for me
it is an executable way to solve this feature issue and satisfy both
kernel and ABI.
Thanks.
--
Chen Gang
Open, share and attitude like air, water and life which God blessed
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] drivers: staging: lustre: lustre: include: add "__attribute__((packed))" for the related union
From: James Hogan > On 03/02/14 10:35, David Laight wrote: > > From: James Hogan > >> Combining __packed with __aligned(2) does the trick though (__packed > >> alone sets __aligned(1) which is obviously going to be suboptimal). ... > > Meta is also one of those arches, and according to my quick tests, > __packed alone does correctly make it fall back to byte loads/stores, > but with __packed __aligned(2) it uses 16bit loads/stores. I've also > confirmed that with an ARM toolchain (see below for example). I would either: 1a) Add explicit padding to the relevant structures so that they are multiple of 4 bytes. or: 1b) #define some token to "__packed __aligned(2)" before all the structures that require changing, and use that in there definitions. This lets you comment on WHY you are doing it. and: 2) Add a compile-time assert that the structures are the correct size. Clearly you don't want to mark anything that contains a 32bit value with __packed __aligned(2). I'm not at all clear whether you are sometimes using a different compiler. David ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: rtl8821ae.
On 02/02/2014 12:07 PM, Stefan Lippers-Hollmann wrote: Hi [ CC'ing the relevant parties ] On Sunday 02 February 2014, Dave Jones wrote: On Sun, Feb 02, 2014 at 03:41:27AM -0800, scan-ad...@coverity.com wrote: > > Please find the latest report on new defect(s) introduced to Linux found with Coverity Scan. > > Defect(s) Reported-by: Coverity Scan > Showing 20 of 83 defect(s) Ugh, this is even worse than the usual realtek drivers. (With the exception of rtl8188eu) All 83 of those new defects came from this new driver, and while there's a bunch of "who cares" type things in there, there's a load of stuff that needs fixing a lot more urgently than CodingStyle issues or anything else in the TODO for that driver. A bigger problem though, is what is the plan for these realtek drivers ? They've been in staging forever. rtl8187se has been there for _five_ years with no indication it's ever getting promoted to first class status. Actually rtl8187se (aka rtl8185b) seems to have gotten some attention recently: http://lkml.kernel.org/r/can8yu5pgkx9s9dewpfto_ztdr-+wdd5cx2jrv1zd1m1q0bp...@mail.gmail.com The git logs are littered mostly with CodingStyle cleanups, sparse cleanups and such, meanwhile for five years they've had out of bounds reads, overflows, and such for this whole time. Even worse, when one of the drivers gets fixes for actual problems like this, they never make it back to Realtek, who clone the same old shitty driver they shipped last time, and reintroduce new variants of the same damn bugs, and then we import the new turd into staging and start all over again. I get the whole "a shit driver is better than no driver", but there's no discernable effort to ever improve this pile, just to keep adding to it. Dave I think there are mostly two major problems with these drivers, besides RealTek still working on a non-mac80211 codebase for USB based devices. The sheer number of slightly different RealTek drivers for similar chipsets, for which RealTek as forked off a dedicated driver each, rather than extending the existing ones. With the other, probably even larger, problem being that it isn't possible to port wireless drivers from non-mac80111 to mac80211 in a gradual fashion, it's always a parallel re-implementation. Just look at the recent history of staging wireless drivers: the successful ones: - csr --> /dev/null - otus --> ar9170 --> carl9170 - ( rt2870 && rt3070 ) --> rt2800usb - rt2870 --> rt2800pci - [ at76c503a --> ] at76_usb --> at76c50x-usb [*] not in staging the pending ones - rtl8187se [ --> rtl8180 ] [*] hopefully soon - rtl8188eu --> ? - [ rtl8192du --> ? ] [*] not in staging, [1] - rtl8192e --> ? - rtl8192u --> ? - rtl8192su --> rtl8712 --> ? [ r92su[2] would add cfg80211 support, but it being a fullmac like re-implementation doesn't get it anywhere ] - rtl8821ae [ --> mac80211 port planned for 3.15[3]? ] these devices are, besides rtl8187se (802.11g) and rtl8821ae (802.11ac), all 802.11n compatible, but were quickly EOLed by the vendor, probably making it hard to get enough traction for a proper mac80211 port. Coincidentally these chipsets are also very popular, rtl8187se being the chipset of the early netbook craze, rtl8188eu pretty ubiquitous on embedded platforms, the others making the bulk of aftermarket USB devices. ancient hardware, probably not going anywhere: - vt6655 --> ? - vt6656 --> ? - wlags49_h2 --> ? - wlags49_h25 --> ? - wlan-ng --> ? This likely leaves staging wireless drivers to small corrections and bugfixes. In the hope that the devices will get enough traction that someone takes up the effort of doing a parallel re-implementation of a proper mac80211 based driver, using the staging source only as reference. Regards Stefan Lippers-Hollmann [1] https://github.com/lwfinger/rtl8192du [2] https://github.com/chunkeey/rtl8192su/tree/master/r92su [3] http://lkml.kernel.org/r/52e066a1.9010...@lwfinger.net http://lkml.kernel.org/r/52e7d9f6.5070...@lwfinger.net There are many issues here. I will try to address them without writing something so long that no one reads it. Just to be clear, my work with Realtek drivers is as a volunteer and is done to provide a service to Linux users. The only thing I get from Realtek is advance copies of drivers (sometimes), and sample cards for PCI devices. With one exception, the USB devices in my possession have have been purchased with my own funds. 1. Coverity errors in rtl8821ae: This driver was added to staging with essentially no cleanup for several reasons. The first was as a favor to GregKH, Linus, and roughly 1000 other users who recently got this hardware and want a driver in 3.14. The second reason is that I do not yet have this hardware and did not want to make any changes as all testing would have to be conducted by Greg, and I think he has enou
RE: [PATCH v4] Move DWC2 driver out of staging
> From: Stephen Warren [mailto:swar...@wwwdotorg.org] > Sent: Saturday, February 01, 2014 7:44 PM > > On 02/01/2014 03:00 AM, Andre Heider wrote: > > On Fri, Jan 31, 2014 at 11:48:37PM -0700, Stephen Warren wrote: > >> On 01/31/2014 11:12 AM, Andre Heider wrote: > >>> On Mon, Jan 13, 2014 at 01:50:09PM -0800, Paul Zimmerman wrote: > The DWC2 driver should now be in good enough shape to move out of > staging. I have stress tested it overnight on RPI running mass > storage and Ethernet transfers in parallel, and for several days > on our proprietary PCI-based platform. > >> ... > >>> this looks just fine, but for whatever reason it breaks sdhci on my rpi. > >>> With today's Linus' master the dwc2 controller seems to initialize fine, > >>> but I get this upon boot: > >>> > >>> [1.783316] sdhci-bcm2835 2030.sdhci: sdhci_pltfm_init failed -12 > >>> [1.794820] sdhci-bcm2835: probe of 2030.sdhci failed with error > >>> -12 > ... > >> This is due to the following code: > ... > >> What ends up happening, simply due to memory allocation order, is that > >> the memory writes inside usb_settoggle() end up setting the SDHCI struct > >> platform_device's num_resources to 0, so that it's call to > >> platform_get_resource() fails. > >> > >> With the DWC2 move patch reverted, some other random piece of memory is > >> being corrupted, which just happens not to cause any visible problem. > >> Likely it's some other struct platform_device that's already had its > >> resources read by the time DWC2 probes and corrupts them. > >> > >> (Yes, this was hard to find!) > > > > Nice work, but how did you pinpoint this? Am I missing some option/tool > > or did I just not stare for long enough? > > Well, there was a clear place where an issue was present; the resource > lookup in sdhci_pltfm_init() was failing, so I put a bunch of printfs > into that function to dump out the data platform_get_resource() used. > This clearly pointed at num_resources==0 being the problem. Next, I > dumped the same data from the code in drivers/of that sets it up, and it > was OK there, so I knew it was getting over-written somewhere. I then > basically added hundreds of calls to the same data dumping function > throughout kernel functions like really_probe() to track down the > location of the problem. Luckily, the behaviour was stable, so I wasn't > chasing a race/timing condition. Eventually I narrowed the window to the > few lines of code I mentioned in _dwc2_hcd_endpoint_reset(). It would > have been much harder if it was e.g. the USB HW DMAing to memory that > caused the corruption, so I was lucky:-) Nice work Stephen, thanks. I will try to come up with a patch to fix this ASAP, along the lines of what Alan suggested. -- Paul ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] drivers:staging:tl8821ae Fixed the size of array to macro
Changed array size from "bb_swing_idx_ofdm[2]" to
"bb_swing_idx_ofdm[MAX_RF_PATH]"
Reported-By: Linus Torvalds
Signed-off-by: Surendra Patil
---
drivers/staging/rtl8821ae/wifi.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8821ae/wifi.h b/drivers/staging/rtl8821ae/wifi.h
index cfe88a1..76bef93 100644
--- a/drivers/staging/rtl8821ae/wifi.h
+++ b/drivers/staging/rtl8821ae/wifi.h
@@ -1414,7 +1414,7 @@ struct rtl_dm {
/*88e tx power tracking*/
- u8 bb_swing_idx_ofdm[2];
+ u8 bb_swing_idx_ofdm[MAX_RF_PATH];
u8 bb_swing_idx_ofdm_current;
u8 bb_swing_idx_ofdm_base[MAX_RF_PATH];
bool bb_swing_flag_Ofdm;
--
1.8.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers:staging:tl8821ae Fixed the size of array to macro
On Mon, Feb 03, 2014 at 10:13:36AM -0800, Surendra Patil wrote: > Dan, > > Thanks for the suggestion,modified the change log and sent new patch. > Gar, no. Put the information from the link directly into the changelog so that we know what the problem was and what the fix is without following the link. You just removed the link without adding the information. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/2] MIPS/staging: Probe octeon-usb driver via device-tree
Hi,
On Wed, Dec 04, 2013 at 03:29:53PM -0800, Greg Kroah-Hartman wrote:
> On Tue, Dec 03, 2013 at 11:46:50AM -0800, David Daney wrote:
> > From: David Daney
> >
> > Tested against both EdgeRouter LITE (no bootloader supplied device
> > tree), and ebb5610 (device tree supplied by bootloader).
> >
> > The patch set is spread across both the MIPS and staging trees, so it
> > would be great if Ralf could merge at least the MIPS parts, if not
> > both parts.
>
> That's fine with me:
>
> Acked-by: Greg Kroah-Hartman
It seems only the MIPS part was merged, and as a result the OCTEON
USB build is now broken in 3.14-rc1.
Would it be possible to merge the missing part through the staging
tree? The original is here: http://patchwork.linux-mips.org/patch/6186/
The below one is rebased for 3.14-rc1 and tested on EdgeRouter Lite.
A.
---8<---
>From 82be878d85647d4f60f4ae185e0e63f9644af28d Mon Sep 17 00:00:00 2001
From: David Daney
Date: Mon, 3 Feb 2014 19:39:01 +0200
Subject: [PATCH] staging: octeon-usb: Probe via device tree populated platform
device.
Extract clocking parameters from the device tree, and remove now dead
code and types.
Signed-off-by: David Daney
Tested-by: Aaro Koskinen
Signed-off-by: Aaro Koskinen
---
drivers/staging/octeon-usb/octeon-hcd.c | 273 ++--
1 file changed, 116 insertions(+), 157 deletions(-)
diff --git a/drivers/staging/octeon-usb/octeon-hcd.c
b/drivers/staging/octeon-usb/octeon-hcd.c
index 47e0a91238a1..5a001d9b4252 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -275,13 +275,6 @@ enum cvmx_usb_pipe_flags {
*/
#define MAX_TRANSFER_PACKETS ((1<<10)-1)
-enum {
- USB_CLOCK_TYPE_REF_12,
- USB_CLOCK_TYPE_REF_24,
- USB_CLOCK_TYPE_REF_48,
- USB_CLOCK_TYPE_CRYSTAL_12,
-};
-
/**
* Logical transactions may take numerous low level
* transactions, especially when splits are concerned. This
@@ -471,19 +464,6 @@ struct octeon_hcd {
/* Returns the IO address to push/pop stuff data from the FIFOs */
#define USB_FIFO_ADDRESS(channel, usb_index) (CVMX_USBCX_GOTGCTL(usb_index) +
((channel)+1)*0x1000)
-static int octeon_usb_get_clock_type(void)
-{
- switch (cvmx_sysinfo_get()->board_type) {
- case CVMX_BOARD_TYPE_BBGW_REF:
- case CVMX_BOARD_TYPE_LANAI2_A:
- case CVMX_BOARD_TYPE_LANAI2_U:
- case CVMX_BOARD_TYPE_LANAI2_G:
- case CVMX_BOARD_TYPE_UBNT_E100:
- return USB_CLOCK_TYPE_CRYSTAL_12;
- }
- return USB_CLOCK_TYPE_REF_48;
-}
-
/**
* Read a USB 32bit CSR. It performs the necessary address swizzle
* for 32bit CSRs and logs the value in a readable format if
@@ -582,37 +562,6 @@ static inline int __cvmx_usb_get_data_pid(struct
cvmx_usb_pipe *pipe)
return 0; /* Data0 */
}
-
-/**
- * Return the number of USB ports supported by this Octeon
- * chip. If the chip doesn't support USB, or is not supported
- * by this API, a zero will be returned. Most Octeon chips
- * support one usb port, but some support two ports.
- * cvmx_usb_initialize() must be called on independent
- * struct cvmx_usb_state.
- *
- * Returns: Number of port, zero if usb isn't supported
- */
-static int cvmx_usb_get_num_ports(void)
-{
- int arch_ports = 0;
-
- if (OCTEON_IS_MODEL(OCTEON_CN56XX))
- arch_ports = 1;
- else if (OCTEON_IS_MODEL(OCTEON_CN52XX))
- arch_ports = 2;
- else if (OCTEON_IS_MODEL(OCTEON_CN50XX))
- arch_ports = 1;
- else if (OCTEON_IS_MODEL(OCTEON_CN31XX))
- arch_ports = 1;
- else if (OCTEON_IS_MODEL(OCTEON_CN30XX))
- arch_ports = 1;
- else
- arch_ports = 0;
-
- return arch_ports;
-}
-
/**
* Initialize a USB port for use. This must be called before any
* other access to the Octeon USB port is made. The port starts
@@ -628,41 +577,16 @@ static int cvmx_usb_get_num_ports(void)
* Returns: 0 or a negative error code.
*/
static int cvmx_usb_initialize(struct cvmx_usb_state *usb,
- int usb_port_number)
+ int usb_port_number,
+ enum cvmx_usb_initialize_flags flags)
{
union cvmx_usbnx_clk_ctl usbn_clk_ctl;
union cvmx_usbnx_usbp_ctl_status usbn_usbp_ctl_status;
- enum cvmx_usb_initialize_flags flags = 0;
int i;
/* At first allow 0-1 for the usb port number */
if ((usb_port_number < 0) || (usb_port_number > 1))
return -EINVAL;
- /* For all chips except 52XX there is only one port */
- if (!OCTEON_IS_MODEL(OCTEON_CN52XX) && (usb_port_number > 0))
- return -EINVAL;
- /* Try to determine clock type automatically */
- if (octeon_usb_get_clock_type() == USB_CLOCK_TYPE_CRYSTAL_12) {
- /* Only 12 MHZ crystals are supported */
- flags |= CVMX_USB_INITIALIZE_FLAGS_C
[staging] unused rtl8192u/ieee80211/digest.c ?
I was doing some code audits looking at scattergather uses, and noticed that update() in drivers/staging/rtl8192u/ieee80211/digest.c uses sg.page which doesn't exist any longer, so this can't possibly compile. Turns out that digest.c is actually unused. It doesn't get referenced in a Makefile or get compiled and doesn't get used as far as I can see. This driver looks pretty dead. The original author hasn't touched it in the 4 years since it got in to staging/. Shouldn't we be kicking stuff like this out of staging? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] Drivers: hv: vmbus: Support per-channel driver state
As we implement Virtual Receive Side Scaling on the networking side
(the VRSS patches are currently under review), it will be useful to have
per-channel state that vmbus drivers can manage. Add support for
managing per-channel state.
Signed-off-by: K. Y. Srinivasan
---
include/linux/hyperv.h | 14 ++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 15da677..96fb70e 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1043,6 +1043,10 @@ struct vmbus_channel {
* This will be NULL for the primary channel.
*/
struct vmbus_channel *primary_channel;
+ /*
+* Support per-channel state for use by vmbus drivers.
+*/
+ void *per_channel_state;
};
static inline void set_channel_read_state(struct vmbus_channel *c, bool state)
@@ -1050,6 +1054,16 @@ static inline void set_channel_read_state(struct
vmbus_channel *c, bool state)
c->batched_reading = state;
}
+static inline void set_per_channel_state(struct vmbus_channel *c, void *s)
+{
+ c->per_channel_state = s;
+}
+
+static inline void *get_per_channel_state(struct vmbus_channel *c)
+{
+ return c->per_channel_state;
+}
+
void vmbus_onmessage(void *context);
int vmbus_request_offers(void);
--
1.7.4.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: rtl8821ae.
> >On Sunday 02 February 2014, Dave Jones wrote: > >>The git logs are littered mostly with CodingStyle cleanups, sparse cleanups > >>and such, > >>meanwhile for five years they've had out of bounds reads, overflows, and > >>such > >>for this whole time. Really, any sort of cleanup is good, though... There are bugs I would fix if the code didn't look like puke. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/2] MIPS/staging: Probe octeon-usb driver via device-tree
On Mon, Feb 03, 2014 at 08:35:06PM +0200, Aaro Koskinen wrote: > Hi, > > On Wed, Dec 04, 2013 at 03:29:53PM -0800, Greg Kroah-Hartman wrote: > > On Tue, Dec 03, 2013 at 11:46:50AM -0800, David Daney wrote: > > > From: David Daney > > > > > > Tested against both EdgeRouter LITE (no bootloader supplied device > > > tree), and ebb5610 (device tree supplied by bootloader). > > > > > > The patch set is spread across both the MIPS and staging trees, so it > > > would be great if Ralf could merge at least the MIPS parts, if not > > > both parts. > > > > That's fine with me: > > > > Acked-by: Greg Kroah-Hartman > > It seems only the MIPS part was merged, and as a result the OCTEON > USB build is now broken in 3.14-rc1. > > Would it be possible to merge the missing part through the staging > tree? The original is here: http://patchwork.linux-mips.org/patch/6186/ > The below one is rebased for 3.14-rc1 and tested on EdgeRouter Lite. Sure, I can queue it up for 3.14, and will do so in a few days. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [staging] unused rtl8192u/ieee80211/digest.c ?
On Mon, Feb 03, 2014 at 11:33:41AM -0800, Dave Hansen wrote: > I was doing some code audits looking at scattergather uses, and noticed > that update() in drivers/staging/rtl8192u/ieee80211/digest.c uses > sg.page which doesn't exist any longer, so this can't possibly compile. > > Turns out that digest.c is actually unused. It doesn't get referenced > in a Makefile or get compiled and doesn't get used as far as I can see. Great, please send a patch to delete it :) > This driver looks pretty dead. The original author hasn't touched it in > the 4 years since it got in to staging/. Shouldn't we be kicking stuff > like this out of staging? Ther eare users of this hardware, so I can't delete it. See the relevant lkml thread the past few days about rtl wireless drivers for all of the details as to the mess we have here. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: rtl8821ae.
On Mon, Feb 03, 2014 at 11:05:42AM -0600, Larry Finger wrote: > A combined driver would require very many branches based on chip > number and would certainly execute much more slowly. I seriously doubt there are performance issues with merging the drivers. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: rtl8821ae.
On Mon, Feb 3, 2014 at 12:12 PM, Dan Carpenter wrote: > On Mon, Feb 03, 2014 at 11:05:42AM -0600, Larry Finger wrote: >> A combined driver would require very many branches based on chip >> number and would certainly execute much more slowly. > > I seriously doubt there are performance issues with merging the drivers. Quite frankly, merging drivers can be a f*cking pain. I would seriously suggest avoiding it *unless* the hardware is literally almost identical, or the drivers have been written with multi-chip support from the ground up by people who actually understood the hardware. But the reason isn't performance - it's subtle breakage. Merged drivers have a tendency to break support for chip A when you fix something for chip B. And the end result is a driver that nobody understands, and nobody can sanely test. Sometimes it's simply better to leave old drivers alone. Linus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: rtl8821ae.
On Mon, Feb 03, 2014 at 11:05:42AM -0600, Larry Finger wrote: > I am trying to improve the quality of this pile of , but I am only one > person who was a life-long Fortran-coding scientist until I retired 15 years > ago. Since then, I have been learning C and kernel-style coding practices. > Certainly there is both a lot to do and a lot to learn. I greatly appreciate all of the work you have done here, with this almost impossible task (no specs, random code drops from external people, limited feedback loop from the company, etc.) I blame the company for the situation here, not you at all, and its great that the PCI division is doing much better now. Hopefully the USB side will come around over time. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v4] Move DWC2 driver out of staging
> From: Paul Zimmerman > Sent: Monday, February 03, 2014 9:36 AM > >> From: Stephen Warren [mailto:swar...@wwwdotorg.org] >> Sent: Saturday, February 01, 2014 7:44 PM >> >> On 02/01/2014 03:00 AM, Andre Heider wrote: >>> On Fri, Jan 31, 2014 at 11:48:37PM -0700, Stephen Warren wrote: On 01/31/2014 11:12 AM, Andre Heider wrote: > On Mon, Jan 13, 2014 at 01:50:09PM -0800, Paul Zimmerman wrote: >> The DWC2 driver should now be in good enough shape to move out of >> staging. I have stress tested it overnight on RPI running mass >> storage and Ethernet transfers in parallel, and for several days >> on our proprietary PCI-based platform. ... > this looks just fine, but for whatever reason it breaks sdhci on my rpi. > With today's Linus' master the dwc2 controller seems to initialize fine, > but I get this upon boot: > > [1.783316] sdhci-bcm2835 2030.sdhci: sdhci_pltfm_init failed -12 > [1.794820] sdhci-bcm2835: probe of 2030.sdhci failed with error > -12 >> ... This is due to the following code: >> ... What ends up happening, simply due to memory allocation order, is that the memory writes inside usb_settoggle() end up setting the SDHCI struct platform_device's num_resources to 0, so that it's call to platform_get_resource() fails. With the DWC2 move patch reverted, some other random piece of memory is being corrupted, which just happens not to cause any visible problem. Likely it's some other struct platform_device that's already had its resources read by the time DWC2 probes and corrupts them. (Yes, this was hard to find!) >>> >>> Nice work, but how did you pinpoint this? Am I missing some option/tool >>> or did I just not stare for long enough? >> >> Well, there was a clear place where an issue was present; the resource >> lookup in sdhci_pltfm_init() was failing, so I put a bunch of printfs >> into that function to dump out the data platform_get_resource() used. >> This clearly pointed at num_resources==0 being the problem. Next, I >> dumped the same data from the code in drivers/of that sets it up, and it >> was OK there, so I knew it was getting over-written somewhere. I then >> basically added hundreds of calls to the same data dumping function >> throughout kernel functions like really_probe() to track down the >> location of the problem. Luckily, the behaviour was stable, so I wasn't >> chasing a race/timing condition. Eventually I narrowed the window to the >> few lines of code I mentioned in _dwc2_hcd_endpoint_reset(). It would >> have been much harder if it was e.g. the USB HW DMAing to memory that >> caused the corruption, so I was lucky:-) > > Nice work Stephen, thanks. I will try to come up with a patch to fix this > ASAP, along the lines of what Alan suggested. Stephen, Andre, Can you test the attached patch, please? It works for my on the Synopsys PCIe-based FPGA board. Unfortunately my RPI board is currently broken, so I am unable to test it there to verify it actually fixes the problem you are seeing. The dwc2 driver doesn't use the usb_device toggle bits anywhere else, so the quickest fix is to just remove the problematic code from _dwc2_hcd_endpoint_reset(). If you give me your tested-bys, I will submit this as a proper patch to Greg. -- Paul dwc2-toggle.patch Description: dwc2-toggle.patch ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch 1/2] staging: r8188eu: array overflow in rtw_mp_ioctl_hdl()
MAX_MP_IOCTL_SUBCODE (35) and mp_ioctl_hdl (32 elements) are no longer
in sync. It leads to a bogus pointer dereference.
Signed-off-by: Dan Carpenter
diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
index dec992569476..684bc8107a48 100644
--- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
@@ -2500,7 +2500,7 @@ static int rtw_mp_ioctl_hdl(struct net_device *dev,
struct iw_request_info *info
("rtw_mp_ioctl_hdl: subcode [%d], len[%d], buffer_len[%d]\r\n",
poidparam->subcode, poidparam->len, len));
- if (poidparam->subcode >= MAX_MP_IOCTL_SUBCODE) {
+ if (poidparam->subcode >= ARRAY_SIZE(mp_ioctl_hdl)) {
RT_TRACE(_module_rtl871x_ioctl_os_c, _drv_err_, ("no matching
drvext subcodes\r\n"));
ret = -EINVAL;
goto _rtw_mp_ioctl_hdl_exit;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch 2/2] staging: r8188eu: overflow in rtw_p2p_get_go_device_address()
The go_devadd_str[] array is two characters too small to hold the
address so we corrupt memory.
I've changed the user space API slightly and I don't have a way to test
if this breaks anything. In the original code we truncated away the
last digit of the address and the NUL terminator so it was already a bit
broken.
Signed-off-by: Dan Carpenter
diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
index dec992569476..4ad80ae1067f 100644
--- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
@@ -3164,9 +3164,7 @@ static int rtw_p2p_get_go_device_address(struct
net_device *dev,
u8 *p2pie;
uint p2pielen = 0, attr_contentlen = 0;
u8 attr_content[100] = {0x00};
-
- u8 go_devadd_str[17 + 10] = {0x00};
- /* +10 is for the str "go_devadd =", we have to clear it at
wrqu->data.pointer */
+ u8 go_devadd_str[17 + 12] = {};
/* Commented by Albert 20121209 */
/* The input data is the GO's interface address which the
application wants to know its device address. */
@@ -3223,12 +3221,12 @@ static int rtw_p2p_get_go_device_address(struct
net_device *dev,
spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
if (!blnMatch)
- sprintf(go_devadd_str, "\n\ndev_add = NULL");
+ snprintf(go_devadd_str, sizeof(go_devadd_str), "\n\ndev_add =
NULL");
else
- sprintf(go_devadd_str, "\n\ndev_add
=%.2X:%.2X:%.2X:%.2X:%.2X:%.2X",
+ snprintf(go_devadd_str, sizeof(go_devadd_str), "\n\ndev_add
=%.2X:%.2X:%.2X:%.2X:%.2X:%.2X",
attr_content[0], attr_content[1], attr_content[2],
attr_content[3], attr_content[4], attr_content[5]);
- if (copy_to_user(wrqu->data.pointer, go_devadd_str, 10 + 17))
+ if (copy_to_user(wrqu->data.pointer, go_devadd_str,
sizeof(go_devadd_str)))
return -EFAULT;
return ret;
}
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] Move DWC2 driver out of staging
On 02/03/2014 01:51 PM, Paul Zimmerman wrote: ... > Stephen, Andre, > > Can you test the attached patch, please? It works for my on the Synopsys > PCIe-based FPGA board. Unfortunately my RPI board is currently broken, > so I am unable to test it there to verify it actually fixes the problem > you are seeing. > > The dwc2 driver doesn't use the usb_device toggle bits anywhere else, > so the quickest fix is to just remove the problematic code from > _dwc2_hcd_endpoint_reset(). > > If you give me your tested-bys, I will submit this as a proper patch > to Greg. I've tested a basically equivalent patch (link below), so I feel comfortable saying: Tested-by: Stephen Warren https://github.com/swarren/linux-rpi/commit/f7b9c896153cc0501acecb58053db978ec00a5bf > @@ -2579,9 +2579,11 @@ static void _dwc2_hcd_endpoint_reset(struct usb_hcd > *hcd, > > spin_lock_irqsave(&hsotg->lock, flags); > > +#if 0 > usb_settoggle(udev, epnum, is_out, 0); > if (is_control) > usb_settoggle(udev, epnum, !is_out, 0); > +#endif > dwc2_hcd_endpoint_reset(hsotg, ep); > > spin_unlock_irqrestore(&hsotg->lock, flags); ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] drivers:staging:tl8821ae Fixed the size of array to macro as discussed by Linus
Reported-By: Linus Torvalds
It causes an interesting warning for me:
drivers/staging/rtl8821ae/rtl8821ae/dm.c: In function
???rtl8821ae_dm_clear_txpower_tracking_state???:
drivers/staging/rtl8821ae/rtl8821ae/dm.c:487:31: warning: iteration 2u
invokes undefined behavior [-Waggressive-loop-optimizations]
rtldm->bb_swing_idx_ofdm[p] = rtldm->default_ofdm_index;
^
drivers/staging/rtl8821ae/rtl8821ae/dm.c:485:2: note: containing loop
for (p = RF90_PATH_A; p < MAX_RF_PATH; ++p) {
^
and gcc is entirely correct: that loop iterates from 0 to 3, and does this:
rtldm->bb_swing_idx_ofdm[p] = rtldm->default_ofdm_index;
but the bb_swing_idx_ofdm[] array only has two members. So the last
two iterations will overwrite bb_swing_idx_ofdm_current and the first
entry in bb_swing_idx_ofdm_base[].
Now, the bug does seem to be benign: bb_swing_idx_ofdm_current isn't
actually ever *used* as far as I can tell, and the first entry of
bb_swing_idx_ofdm_base[] will have been written with that same
"rtldm->default_ofdm_index" value.
But gcc is absolutely correct, and that driver needs fixing.
I've pulled it and will let it be because it doesn't seem to be an
issue in practice, but please fix it. The obvious fix would seem to
change the size of "2" to be "MAX_RF_PATH", but I'll abstain from
doing those kinds of changes in the merge when it doesn't seem to
affect the build or functionality).
Signed-off-by: Surendra Patil
---
drivers/staging/rtl8821ae/wifi.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8821ae/wifi.h b/drivers/staging/rtl8821ae/wifi.h
index cfe88a1..76bef93 100644
--- a/drivers/staging/rtl8821ae/wifi.h
+++ b/drivers/staging/rtl8821ae/wifi.h
@@ -1414,7 +1414,7 @@ struct rtl_dm {
/*88e tx power tracking*/
- u8 bb_swing_idx_ofdm[2];
+ u8 bb_swing_idx_ofdm[MAX_RF_PATH];
u8 bb_swing_idx_ofdm_current;
u8 bb_swing_idx_ofdm_base[MAX_RF_PATH];
bool bb_swing_flag_Ofdm;
--
1.8.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: bcm: fix pointer-integer size mismatch warnings
Fix the pointer-integer size mismatch warnings below:
drivers/staging/bcm/CmHost.c: In function
‘StoreCmControlResponseMessage’:
drivers/staging/bcm/CmHost.c:1387:39: warning: cast to pointer from
integer of different size [-Wint-to-pointer-cast]
pstAddIndication->psfAuthorizedSet = (struct bcm_connect_mgr_params
*)ntohl((ULONG)pstAddIndication->psfAuthorizedSet);
^
drivers/staging/bcm/CmHost.c:1426:37: warning: cast to pointer from
integer of different size [-Wint-to-pointer-cast]
pstAddIndication->psfAdmittedSet = (struct bcm_connect_mgr_params
*)ntohl((ULONG)pstAddIndication->psfAdmittedSet);
^
drivers/staging/bcm/CmHost.c:1440:35: warning: cast to pointer from
integer of different size [-Wint-to-pointer-cast]
pstAddIndication->psfActiveSet = (struct bcm_connect_mgr_params
*)ntohl((ULONG)pstAddIndication->psfActiveSet);
^
Signed-off-by: SeongJae Park
---
drivers/staging/bcm/CmHost.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/bcm/CmHost.c b/drivers/staging/bcm/CmHost.c
index cc91b5e..dd8f8f7 100644
--- a/drivers/staging/bcm/CmHost.c
+++ b/drivers/staging/bcm/CmHost.c
@@ -1384,7 +1384,8 @@ ULONG StoreCmControlResponseMessage(struct
bcm_mini_adapter *Adapter, PVOID pvBu
}
/* this can't possibly be right */
- pstAddIndication->psfAuthorizedSet = (struct bcm_connect_mgr_params
*)ntohl((ULONG)pstAddIndication->psfAuthorizedSet);
+ pstAddIndication->psfAuthorizedSet = (struct bcm_connect_mgr_params *)
+ (uintptr_t)ntohl((ULONG)pstAddIndication->psfAuthorizedSet);
if (pstAddIndicationAlt->u8Type == DSA_REQ) {
struct bcm_add_request AddRequest;
@@ -1423,7 +1424,8 @@ ULONG StoreCmControlResponseMessage(struct
bcm_mini_adapter *Adapter, PVOID pvBu
return 0;
}
- pstAddIndication->psfAdmittedSet = (struct bcm_connect_mgr_params
*)ntohl((ULONG)pstAddIndication->psfAdmittedSet);
+ pstAddIndication->psfAdmittedSet = (struct bcm_connect_mgr_params *)
+ (uintptr_t)ntohl((ULONG)pstAddIndication->psfAdmittedSet);
/* ACTIVE SET */
pstAddIndication->psfActiveSet = (struct bcm_connect_mgr_params *)
@@ -1437,7 +1439,8 @@ ULONG StoreCmControlResponseMessage(struct
bcm_mini_adapter *Adapter, PVOID pvBu
return 0;
}
- pstAddIndication->psfActiveSet = (struct bcm_connect_mgr_params
*)ntohl((ULONG)pstAddIndication->psfActiveSet);
+ pstAddIndication->psfActiveSet = (struct bcm_connect_mgr_params *)
+ (uintptr_t)ntohl((ULONG)pstAddIndication->psfActiveSet);
(*puBufferLength) = sizeof(struct bcm_add_indication);
*(struct bcm_add_indication *)pvBuffer = *pstAddIndication;
--
1.8.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: cxt1e1: fix pointer-integer size mismatch warning
Fix the pointer-integer size mismatch warning below:
drivers/staging/cxt1e1/functions.c: In function ‘VMETRO_TRACE’:
drivers/staging/cxt1e1/functions.c:268:21: warning: cast from pointer
to integer of different size [-Wpointer-to-int-cast]
u_int32_t y = (u_int32_t) x;
^
Signed-off-by: SeongJae Park
---
drivers/staging/cxt1e1/functions.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/cxt1e1/functions.c
b/drivers/staging/cxt1e1/functions.c
index 95218e2..8f19a39 100644
--- a/drivers/staging/cxt1e1/functions.c
+++ b/drivers/staging/cxt1e1/functions.c
@@ -265,7 +265,7 @@ extern ci_t *CI;/* dummy pointer to board
ZERO's data */
void
VMETRO_TRACE (void *x)
{
-u_int32_t y = (u_int32_t) x;
+u_int32_t y = (u_int32_t)(uintptr_t) x;
pci_write_32 ((u_int32_t *) &CI->cpldbase->leds, y);
}
--
1.8.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
