Re: [PATCH] drivers:staging:tl8821ae Fixed the size of array to macro

2014-02-03 Thread Dan Carpenter
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 torva...@linux-foundation.org

regards,
dan carpenter

 Signed-off-by: Surendra Patil surendra@gmail.com
 ---
  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] drivers: staging: lustre: lustre: include: add __attribute__((packed)) for the related union

2014-02-03 Thread Chen Gang
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

2014-02-03 Thread David Laight
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

2014-02-03 Thread Chen Gang
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

2014-02-03 Thread David Laight
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

2014-02-03 Thread James Hogan
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

2014-02-03 Thread Chen Gang
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

2014-02-03 Thread David Laight
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.

2014-02-03 Thread Larry Finger

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 enough to do already. The driver 

[PATCH 1/1] Drivers: hv: vmbus: Support per-channel driver state

2014-02-03 Thread K. Y. Srinivasan
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 k...@microsoft.com
---
 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: [staging] unused rtl8192u/ieee80211/digest.c ?

2014-02-03 Thread Greg Kroah-Hartman
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.

2014-02-03 Thread Dan Carpenter
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.

2014-02-03 Thread Greg KH
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

2014-02-03 Thread Paul Zimmerman
 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()

2014-02-03 Thread Dan Carpenter
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 dan.carpen...@oracle.com

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()

2014-02-03 Thread Dan Carpenter
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 dan.carpen...@oracle.com

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

2014-02-03 Thread Stephen Warren
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 swar...@wwwdotorg.org

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

2014-02-03 Thread Surendra Patil
Reported-By: Linus Torvalds torva...@linux-foundation.org
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 surendra@gmail.com
---
 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

2014-02-03 Thread SeongJae Park
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 sj38.p...@gmail.com
---
 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