Re: [PATCH V2] ssb: Implement virtual SPROM on disk

2010-03-24 Thread Michael Buesch
On Wednesday 24 March 2010 15:16:03 Larry Finger wrote:
 I have modified ssb to supply a MAC address of 80:80:80:80:80:80, rather than

What about also setting the local-assignment bit for this temporary address?

 The one remaining problem is that the interface has already been renamed 
 before
 60-persistent-b43-mac.rules is processed. In my case, the interface is wlan13,
 not wlan0. After I manually modified 60-..., then the new address is applied.
 I'm still working on this problem.

Well, udev scripts are processed in alphabetical order. Can't you simply run
the persistent mac rules before the persistent ifname rules?

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH V2] ssb: Implement virtual SPROM on disk

2010-03-23 Thread Michael Buesch
On Tuesday 23 March 2010 00:45:28 Larry Finger wrote:
 (2) In drivers/ssb/pci.c, the firmware loading process reads the file, then
 modifies it using the bus address.

No. You don't need firmware loading at all.
udev can just set the mac address through the standard ioctl calls.
Just like how ifconfig and ip can do.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH V2] ssb: Implement virtual SPROM on disk

2010-03-23 Thread Michael Buesch
On Tuesday 23 March 2010 09:55:39 Florian Fainelli wrote:
 Why not use random_ether_addr and only use the digits that you are interested 
 in?

Because it's not constant across reboots.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH V2] ssb: Implement virtual SPROM on disk

2010-03-23 Thread Michael Buesch
On Tuesday 23 March 2010 21:58:22 Calvin Walton wrote:
 On Tue, 2010-03-23 at 09:25 -0500, Larry Finger wrote:
  Will someone please write me udev rule(s) that do the following:
  
  1. Detect a MAC address of FF:FF:FF:FF:FF:FF
  2. If this is the first time for this bus address, then generate a random 
  MAC
  address with the bus address encoded in it.
  3. Preserve the address for future reloads
  4. Load the saved address into the device.
  5. Do the above with only standard external commands - no new programs
  
  My skills with udev are not up to the task.
 
 I will warn you that the following is rather untested, as I don't have
 any of the affected hardware (or any b43 devices at all, actually), but
 something along these lines should work. There's no syntax errors, at
 least :)
 
 --- /lib/udev/rules.d/65-persistent-b43-mac-generator.rules
 
 ACTION!=add GOTO=persistent_b43_mac_generator_end
 
 SUBSYSTEM==net, DRIVERS==b43, ATTR{address}==ff:ff:ff:ff:ff:ff, 
 IMPORT{program}=write_persistent_b43_mac
 
 SUBSYSTEM==net, ENV{MACADDRESS_NEW}==?*, RUN+=ifconfig $env{INTERFACE} 
 hw ether $env{MACADDRESS_NEW}
 
 LABEL=persistent_b43_mac_generator_end
 
 --- /lib/udev/write_persistent_b43_mac (chmod +x)
 
 #!/bin/bash
 
 # This mac address generation function could be replaced with something better
 MACADDRESS=$(dd if=/dev/urandom bs=1 count=6 2/dev/null | od -tx1 | head -1 
 | cut -d' ' -f2- | awk '{ print $1:$2:$3:$4:$5:$6 }')
 
 RULES_FILE='/etc/udev/rules.d/60-persistent-b43-mac.rules'
 
 . /lib64/udev/rule_generator.functions
 
 lock_rules_file
 
 choose_rules_file
 
 echo DEVPATH==\$DEVPATH\, DRIVERS==\b43\, 
 ATTR{address}==\ff:ff:ff:ff:ff:ff\, RUN+=\ifconfig $INTERFACE hw ether 
 $MACADDRESS\  $RULES_FILE
 
 echo MACADDRESS_NEW=$MACADDRESS
 
 unlock_rules_file
 
 ---
 
 A new file /etc/udev/rules.d/60-persistent-b43-mac.rules will be
 created, which will contain the the saved mac address and bypass the
 generating script on future boots.
 
 This should probably be run by the udev maintainers, but is a start,
 anyways.
 
 

Thanks a lot for this proof of concept. This looks very nice.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH V2] ssb: Implement virtual SPROM on disk

2010-03-22 Thread Michael Buesch
On Monday 22 March 2010 07:28:23 Calvin Walton wrote:
 Hi,
 
 On Sat, 2010-03-20 at 19:14 -0500, Larry Finger wrote:
 Some recent BCM43XX devices lack an on-board SPROM. The pertinent data
  from the SPROM could be included in the kernel; however, this presents
  a problem in the generation of a unique, reproducible MAC address. The
  solution has been to create a utility that generates a virtual SPROM
  image with a random MAC address. This image is stored in the firmware
  area, and loaded using the asyncronous firmware load facility.
 
 I'm curious, how would this firmware-loading scheme deal with having
 multiple cards of this type installed? This seems like an unusual
 situation, but it looks like this patch will cause all of the cards to
 start up with the same MAC address due to the fixed filename.
 
 Instead of using a firmware file to load in the MAC address, might it be
 possible to move the persistent MAC setting to a simple udev rule which
 generates a persistent MAC address, saves it, then sets it each boot
 using a command like ip link set wlan0 address XX:XX:XX:XX:XX:XX ?
 
 This would remove the need to have this fake firmware file available
 at boot, provided that the driver can handle leaving the address
 unconfigured until userspace gets around to setting it. As well, it
 could be written to work with multiple cards easily, saving a different
 MAC for each.
 
 Some thoughts for your consideration,
 

I think this actually is a very good idea.
This way we could live without a user-supplied sprom image, which I would 
_really_
prefer. 

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH V2] ssb: Implement virtual SPROM on disk

2010-03-22 Thread Michael Buesch
On Monday 22 March 2010 22:56:44 Larry Finger wrote:
 Does anyone have any suggestions on what characteristic could be used to
 generate a unique MAC address for a box in a udev rule?

/dev/urandom

Yeah, there's the chance of clashes. In practice there won't be any clashes,
however. If you think there's a real risk, you should start playing
the lottery tomorrow. You'll immediately win a million dollars so you don't have
to worry about those questions anymore. ;)

In fact, I think the risk for mac clashes is not really reduced by generating 
the mac
address from serial numbers, whatever, etc...

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH V2] ssb: Implement virtual SPROM on disk

2010-03-22 Thread Michael Buesch
On Monday 22 March 2010 23:19:54 Larry Finger wrote:
 On 03/22/2010 04:55 PM, Michael Buesch wrote:
  
  I don't see a problem for udev to distinguish the cards. It can do it 
  merely on
  the bus-ID. That's unique. Yeah, it might change if you change the hardware.
  But do we care? I say no, because you cannot actually change the hardware 
  in real life
  for any of these devices. And even if you could reorder the devices on the 
  bus or whatever.
  What would happen? The card would get a new MAC address. That's all. That's 
  acceptable.
  
  The kernel would (for example) just set the mac address to all-ones. Udev 
  would
  notice this (invalid) mac address and reassign a new persistent one to the 
  device. It then
  stores the address on the harddisk.
 
 What ensures that this persistent name would be unique?

The same mechanism that ensures that an UUID is unique: A good random number 
generator.

  In fact, if we implement a mechanism in the kernel, we have _exactly_ the 
  same problem.
  However, currently Larry's patches just ignore that problem and assume that 
  there's only
  one card in the system anyway.
 
 As I said in a posting a few minutes ago, that problem is solved.

Well. You distinguish by bus-ID, right? That's exactly the thing that udev does.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH] ssb: Implement virtual SPROM on disk

2010-03-21 Thread Michael Buesch
On Sunday 21 March 2010 01:14:51 Larry Finger wrote:
 Some recent BCM43XX devices lack an on-board SPROM. The pertinent data
 from the SPROM could be included in the kernel; however, this presents
 a problem in the generation of a unique, reproducible MAC address. The
 solution has been to create a utility that generates a virtual SPROM
 image with a random MAC address. This image is stored in the firmware
 area, and loaded using the asyncronous firmware load facility.
 
 Signed-off-by: Larry Finger larry.fin...@lwfinger.net
 ---
 
 Michael,
 
 I think this patch eliminates the need for the fallback sprom code; however,
 I have not touched that facility yet.
 
 Larry
 ---
 
 Index: wireless-testing/drivers/ssb/pci.c
 ===
 --- wireless-testing.orig/drivers/ssb/pci.c
 +++ wireless-testing/drivers/ssb/pci.c
 @@ -19,6 +19,7 @@
  #include linux/ssb/ssb_regs.h
  #include linux/pci.h
  #include linux/delay.h
 +#include linux/firmware.h
  
  #include ssb_private.h
  
 @@ -613,6 +614,39 @@ static int sprom_extract(struct ssb_bus
   return 0;
  }
  
 +static int ssb_get_vsprom(struct device *dev, u16 *buf, size_t size)
 +{
 + /* Use the firmware load facility to get the disk image of an SPROM */
 + const struct firmware *blob;
 + int err;
 + int i;
 +
 + err = request_firmware(blob, ssb/vsprom_image, dev);

Well, this has the problem every synchronous firmware fetching mechanism has:
bootup with builtin module.
I think it should be done asynchronously.

 + if (err) {
 + printk(KERN_ERR ssb: The BCM43XX device does not have an 
 +SPROM built in, and the virtual SPROM file is not
 + available.\n);
 + printk(KERN_ERR ssb: Please go to 
 +http://wireless.kernel.org/en/users/Drivers/b43 
 +and download the utility to create the file.\n);
 + return err;
 + }
 + if (blob-size != size * 2) {
 + printk(KERN_ERR ssb: The virtual SPROM file has the wrong
 + size\n);
 + return -ENOENT;
 + }
 + for (i = 0; i  size; i++)
 + buf[i] = blob-data[2 * i + 1]  8 | blob-data[2 * i];
 + err = sprom_check_crc(buf, size);
 + if (err) {
 + printk(KERN_ERR ssb: Invalid CRC for virtual SPROM\n);
 + return err;
 + }
 +
 + return 0;
 +}
 +
  static int ssb_pci_sprom_get(struct ssb_bus *bus,
struct ssb_sprom *sprom)
  {
 @@ -620,8 +654,18 @@ static int ssb_pci_sprom_get(struct ssb_
   int err = -ENOMEM;
   u16 *buf;
  
 - if (!ssb_is_sprom_available(bus))
 - return -ENODEV;
 + if (!ssb_is_sprom_available(bus)) {
 + buf = kcalloc(SSB_SPROMSIZE_WORDS_R4, sizeof(u16),
 +   GFP_KERNEL);
 + if (!buf)
 + return -ENOMEM;
 + bus-sprom_size = SSB_SPROMSIZE_WORDS_R4;
 + if (ssb_get_vsprom(bus-host_pci-dev, buf, bus-sprom_size)) {
 + err = -ENODEV;
 + goto out_free;
 + }
 + goto extract;
 + }
  
   buf = kcalloc(SSB_SPROMSIZE_WORDS_R123, sizeof(u16), GFP_KERNEL);
   if (!buf)
 @@ -653,6 +697,7 @@ static int ssb_pci_sprom_get(struct ssb_
   SPROM CRC (corrupt SPROM)\n);
   }
   }
 +extract:
   err = sprom_extract(bus, sprom, buf, bus-sprom_size);
  
  out_free:
 
 

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: RFC: A workaround for BCM43XX devices with no on-board SPROM

2010-03-19 Thread Michael Buesch
On Thursday 18 March 2010 22:38:17 Larry Finger wrote:
 On 03/18/2010 02:31 PM, Michael Buesch wrote:
  On Thursday 18 March 2010 18:46:35 Larry Finger wrote:
  (1) Modify b43-fwcutter to take data from an existing SPROM,
  
  Why not extend the ssb-sprom tool? I don't think this has anything to do 
  with
  firmware, except that we (ab)use the firmware loading mechanism of the 
  kernel
  for loading the blob into the kernel.
 
 It has nothing to do with firmware, but the existing fwcutter has all the 
 parts
 to generate files in the firmware directory,

Everything needed to generate a file in the firmware directory are the open()
write() and close() syscalls.

  
  I have chosen to implement this in
  fwcutter rather than ssb_sprom because the ordinary user will not have 
  access to
  ssb_sprom;
  
  Huh? ssb-sprom is GPL software. I have no problem relicensing it under BSD 
  or
  even something more liberal. I don't see a problem for ordinary users 
  here.
 
 It has nothing to do with the license. My distro, openSUSE, packages fwcutter
 along with a script that uses wget to download the Broadcom drivers and 
 extract
 firmware for both b43 and b43legacy. The average user only has to execute that
 script. Of course, the package could include both fwcutter and ssb_sprom
 programs, but that would make a bigger change to the openSUSE package than 
 just
 a patch to fwcutter. I suspect that other distros use similar packages.
 
  Well, but that version won't do anything on the SPROM, too.
 
 Yes, but if fwcutter were modified, it could write the virtual SPROM file.

I think it really is abuse of fwcutter.
What if you don't want any proprietary firmware at all, but still want an SPROM 
image?
What about distros that do _not_ automatically use fwcutter to put proprietary 
fw in place
for legal reasons? (Which most likely is the majority of distributions).

Why create yet another dependency on fwcutter. I thought the long term plan was 
to get rid
of proprietary firmware and fwcutter?

Is it really such a big deal for a distribution to include yet another tiny 
opensource
package? If that really is a problem for a distribution, they should just 
completely
stop doing their distro.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: RFC: A workaround for BCM43XX devices with no on-board SPROM

2010-03-18 Thread Michael Buesch
On Thursday 18 March 2010 18:46:35 Larry Finger wrote:
 (1) Modify b43-fwcutter to take data from an existing SPROM,

Why not extend the ssb-sprom tool? I don't think this has anything to do with
firmware, except that we (ab)use the firmware loading mechanism of the kernel
for loading the blob into the kernel.

 I have chosen to implement this in
 fwcutter rather than ssb_sprom because the ordinary user will not have access 
 to
 ssb_sprom;

Huh? ssb-sprom is GPL software. I have no problem relicensing it under BSD or
even something more liberal. I don't see a problem for ordinary users here.

 however, they do have a version of fwcutter supplied by the distro.

Well, but that version won't do anything on the SPROM, too.

 It it reasonable to keep the vendor portion of the MAC and only replace the
 serial number, or would it be better to randomize all 6 octants?

I think it doesn't really matter.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: BCM 43

2010-03-13 Thread Michael Buesch
On Saturday 13 March 2010 19:38:01 Gábor Stefanik wrote:
  I know this one it is compatible with aircrack supporting injection
  Thank you for your invaluable help ..
  thank
 
 
 As a general rule, do not bug Michael with injection-related
 questions; the only one in the team who cares about injection is me.
 Exception: when an injection-related problem also affects hostapd,
 feel free to contact Michael.

WTF?
Injection is not my business at all. It doesn't have _anything_
to do with the driver. It's purely a mac80211 stack feature.
(So _any_ mac80211 based driver is supported by aircrack-ng, btw)

So feel free to contact anybody but me, because I can't help you at all.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: LP-PHY Fatal DMA error 0x00000800 on non-ULV Core 2 Duo?!?!!??!

2010-03-02 Thread Michael Buesch
On Tuesday 02 March 2010 23:25:48 William Bourque wrote:
 So if I get this right, this code is responsible of handling the b43
 devices, as well as several other PCI-E devices, correct?

Nah, this is a broadcom specific thing of the on-chip SSB bus.

 
 Because now that you mention this, the wired network card (Marvel Yukon,
 with sky2 drivers) on this netbook also have a tons of issue (doesn't
 show in lspci on a clean boot, oops the kernel if network cable is
 unplugged while in use, fails to load if the module is ever unloaded, ... )
 I thought it was unrelated but from your comment, I feel like this could
 be linked to the same PCI-E bugs as well.

Uh, well. Are you sure your hardware is OK then?

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: LP-PHY Fatal DMA error 0x00000800 on non-ULV Core 2 Duo?!?!!??!

2010-03-02 Thread Michael Buesch
On Monday 01 March 2010 01:22:50 Michael Buesch wrote:
 Well, you are confusing address spaces here.

 On a PCI based SSB device all host-side MMIO transfers go into
 the PCI device's address space first. The core-switching moves the window of
 the SSB address space that is mapped into 0-0xFFF of the PCI address space.
 So if you write to anything above 0xFFF on the PCI device, the write will
 not (directly) map to the SSB bus or any device on it.
 On the PCI device there is more stuff mapped on top of the SSB sliding
 window. For example the SPROM is mapped right on top of it.
 
 So it might be the case that on a PCI-E device the PCI-E-core's registers
 are permanently mapped into 0x2000 of the PCI address apace. This is to
 avoid sliding the SSB address space window when accessing the PCI-E core.
 This can have several reasons: For one speed (unlikely) and for another
 to avoid concurrency and ugly races when we need to access the PCI-E core
 while the wireless core is already running and generating interrupts.
 Note that this is a GUESS, but it would make sense to me.
 It would be cool if somebody could compare more registers of the PCI-E
 core using the sliding window and the 0x2000 + reg method to check my theory.
 

So what's the status on this? I think the fact that the testing patch showed 
some
improvement is a clear indicator that something in the PCI-E core init is wrong.
It's also not surprising that something is going wrong there. The whole PCI-E 
core
code basically is undebugged. I wrote most of it long time ago, but I still
don't have a device that tests it (and probably won't get one anytime soon).
So I'm really not surprised that there are bugs. There also are missing parts.

A bug in the PCI-E core code is able to show such behavior, because all memory
transfers (MMIO and DMA) from the PCI device to the wireless core are translated
by the PCI-E core.
I think the whole PCI-E core code has to be audited (also the specs, probably).

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 01/11] b43: N-PHY: add some registers and structs definitions

2010-02-28 Thread Michael Buesch
On Sunday 28 February 2010 02:37:21 Rafał Miłecki wrote:
 W dniu 27 lutego 2010 15:51 użytkownik Michael Buesch m...@bu3sch.de 
 napisał:
  On Saturday 27 February 2010 12:12:23 Rafał Miłecki wrote:
  2010/2/10 Michael Buesch m...@bu3sch.de:
   On Tuesday 09 February 2010 21:04:33 Rafał Miłecki wrote:
   +#define B43_MMIO_PSM_PHY_HDR         0x492   /* programmable state 
   machine */
  
   The comment doesn't make a lot of sense.
   In case you don't know, the PSM is the part of the hardware
   that executes the firmware.
 
  Well, guess you're right. It was me hearing for the first time PSM
  so decided to write it. Guess it's pretty obvious for every device
  driver developer :)
 
 
  Well, what I wanted to say is that it's more important to explain
  what PHY_HDR means. Something like /* PSM PHY header */ ?
 
 Sure, explaining HDR sounds great, but do you actually know it's
 header? Personally I have no idea, so I am not sure if you know that
 with nice probability of being right, or are you just guessing? :)

Well, I assumed you could deduce the meaning from the code. If not, it's
better to leave it without a comment. I don't know for sure what this means.
But declaring the definition as Programmable State Machine is also
wrong. So let's simply leave it without a comment.


-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: LP-PHY Fatal DMA error 0x00000800 on non-ULV Core 2 Duo?!?!!??!

2010-02-28 Thread Michael Buesch
On Sunday 28 February 2010 19:52:53 Gábor Stefanik wrote:
 2010/2/28 Rafał Miłecki zaj...@gmail.com:
  2010/2/28 Gábor Stefanik netrolller...@gmail.com:
  On Sun, Feb 28, 2010 at 7:00 PM, William Bourque
  william.bour...@polymtl.ca wrote:
  I confirm, it still crashes on my notebook as well. However the new
  fallback to PIO behavior introduced earlier do a fine job getting it 
  back
  on track.
 
  Btw, you are often refering to some documentation that document the 
  register
  for this device, where could I find it? I probably won't be able to do 
  much,
  but I'm curious to see...
 
  New test patch attached.
 
  Patch adds this /incorrect/ ssb_write32 to 0x280a, right? By incorrect
  I mean over range.
 
  Would be nice to see if dumping tool generates same log about 0x280a
  now (for wl and b43 patched).
 
 I added both the incorrect 0x280a and the correct 0x80a here - it
 is possible that the 0x280a one takes advantage of an undocumented
 feature in PhoenixBIOS.

Hell, it is pure luck that this does not blow up the whole machine.

Please check the memory region of your wireless card with lspci -vvnn:

0001:10:12.0 Network controller [0280]: Broadcom Corporation BCM4306 802.11b/g 
Wireless LAN Controller [14e4:4320] (rev 03)
Subsystem: Apple Computer Inc. AirPort Extreme [106b:004e]
Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast TAbort- TAbort- 
MAbort- SERR- PERR- INTx-
Latency: 16
Interrupt: pin A routed to IRQ 52
Region 0: Memory at a0006000 (32-bit, non-prefetchable) [size=8K]
Capabilities: access denied
Kernel driver in use: b43-pci-bridge
Kernel modules: ssb

It says 8k for all of my devices there. So an MMIO write to 0x2000 and above
writes to completely random memory.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: LP-PHY Fatal DMA error 0x00000800 on non-ULV Core 2 Duo?!?!!??!

2010-02-28 Thread Michael Buesch
On Sunday 28 February 2010 21:30:38 Chris Vine wrote:
 On Sun, 28 Feb 2010 19:58:11 +0100
 Michael Buesch m...@bu3sch.de wrote:
  It says 8k for all of my devices there. So an MMIO write to 0x2000
  and above writes to completely random memory.
  
 My BCM4312 device is 16K:
 
   Region 0: Memory at f800 (64-bit, non-prefetchable) [size=16K]

Ok, then this write is _clearly_ not to be done unconditionally.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: LP-PHY Fatal DMA error 0x00000800 on non-ULV Core 2 Duo?!?!!??!

2010-02-28 Thread Michael Buesch
On Monday 01 March 2010 00:38:16 Nathan Schulte wrote:
 2010/2/28 Gábor Stefanik netrolller...@gmail.com:
  The latest patch, which is a partial success according to some
  testers, writes to core 1 (PCI-E) instead of core 0 (ChipCommon).
 Then either I am misinterpreting the logs, or the last patch in this
 thread is not the patch you are referring to.
 
 A successful write/read to PCI config register 0x80 indicates that any
 following MMIO read/writes will be done on that core, correct?
 
 With the lpphy-test.patch you posted earlier, I see the following
 output from b43:
 Wrote 0x18003000 to pos 0x80
 Read 0x18003000 from pos 0x80
 MAP 1 0xf400 0xc900225b8000 0x4000 0x0 0
 [snip some mmio read/writes and some PCI config read/writes]
 Wrote 0x1800 to pos 0x80
 Read 0x1800 from pos 0x80
 R 4 1 0xf400280a 0x6dbe 0x0 0
 W 4 1 0xf400280a 0xedbe 0x0 0
 
 This first maps core 3, does some read/writes with it, then maps core
 0, and sets bit 0x8000, correct?
 
 Also, is the address space limited to the 4k range?  wl maps core 1,
 but sets bit 0x8000 at address 0x280a, which when added to 0x18001000
 is 0x1800380a, right in the PCIE cores address space (for address
 0x100a).

Well, you are confusing address spaces here.

On a PCI based SSB device all host-side MMIO transfers go into
the PCI device's address space first. The core-switching moves the window of
the SSB address space that is mapped into 0-0xFFF of the PCI address space.
So if you write to anything above 0xFFF on the PCI device, the write will
not (directly) map to the SSB bus or any device on it.
On the PCI device there is more stuff mapped on top of the SSB sliding
window. For example the SPROM is mapped right on top of it.

So it might be the case that on a PCI-E device the PCI-E-core's registers
are permanently mapped into 0x2000 of the PCI address apace. This is to
avoid sliding the SSB address space window when accessing the PCI-E core.
This can have several reasons: For one speed (unlikely) and for another
to avoid concurrency and ugly races when we need to access the PCI-E core
while the wireless core is already running and generating interrupts.
Note that this is a GUESS, but it would make sense to me.
It would be cool if somebody could compare more registers of the PCI-E
core using the sliding window and the 0x2000 + reg method to check my theory.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: LP-PHY Fatal DMA error 0x00000800 on non-ULV Core 2 Duo?!?!!??!

2010-02-27 Thread Michael Buesch
On Friday 26 February 2010 23:03:28 Gábor Stefanik wrote:
 BTW there is an interesting difference in the early init between wl
 and b43: b43 sets bit 0x200 in core register 0x600, while wl sets
 0x8000 in register 0x280a - an undocumented register.

Well, it is not only undocumented, it's also far beyond the address space.
SSB core address space goes from 0-0xFFF. And (more important!) the PCI address
space for MMIO is only 8k wide (I think there also are 4k devices).
Are you really sure your dumping is correct? I suspect a bug in your mmio
dumping tools.
I'd say the undocumented register is a completely unrelated hardware access
in a completely different device that just happens to be mapped right after ssb.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: LP-PHY Fatal DMA error 0x00000800 on non-ULV Core 2 Duo?!?!!??!

2010-02-27 Thread Michael Buesch
On Saturday 27 February 2010 02:41:48 Gábor Stefanik wrote:
 Someone should test the following:
 -Open drivers/ssb/driver_chipcommon_pmu.c
 -In ssb_pmu_init, replace 0x4325 with 0x4312.

Uh, wait a second. Do _all_ cards that show the behavior have a PMU on the SSB?
If so, you should really make sure the PMU setup is correct. If the PMU
cuts power to the device at inappropriate times, it sure does result in DMA 
errors (and
silent MMIO errors).

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 01/11] b43: N-PHY: add some registers and structs definitions

2010-02-27 Thread Michael Buesch
On Saturday 27 February 2010 12:12:23 Rafał Miłecki wrote:
 2010/2/10 Michael Buesch m...@bu3sch.de:
  On Tuesday 09 February 2010 21:04:33 Rafał Miłecki wrote:
  +#define B43_MMIO_PSM_PHY_HDR         0x492   /* programmable state 
  machine */
 
  The comment doesn't make a lot of sense.
  In case you don't know, the PSM is the part of the hardware
  that executes the firmware.
 
 Well, guess you're right. It was me hearing for the first time PSM
 so decided to write it. Guess it's pretty obvious for every device
 driver developer :)
 

Well, what I wanted to say is that it's more important to explain
what PHY_HDR means. Something like /* PSM PHY header */ ?

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 08/11] b43: implement writing to MMIO shared memory

2010-02-27 Thread Michael Buesch
On Saturday 27 February 2010 12:09:30 Rafał Miłecki wrote:
 2010/2/9 Michael Buesch m...@bu3sch.de:
  On Tuesday 09 February 2010 21:04:40 Rafał Miłecki wrote:
  Signed-off-by: Rafał Miłecki zaj...@gmail.com
  ---
   drivers/net/wireless/b43/phy_common.c |   11 +++
   drivers/net/wireless/b43/phy_common.h |    2 ++
   2 files changed, 13 insertions(+), 0 deletions(-)
 
  diff --git a/drivers/net/wireless/b43/phy_common.c 
  b/drivers/net/wireless/b43/phy_common.c
  index 8f7d7ef..0b0f9df 100644
  --- a/drivers/net/wireless/b43/phy_common.c
  +++ b/drivers/net/wireless/b43/phy_common.c
  @@ -466,3 +466,14 @@ struct b43_c32 b43_cordic(int theta)
 
        return ret;
   }
  +
  +/* http://bcm-v4.sipsolutions.net/802.11/PHY/BmacWriteShm */
  +void b43_bmac_write_shm(struct b43_wldev *dev, u32 offset, u16 value)
  +{
  +     b43_write32(dev, B43_MMIO_SHM_CONTROL, 0x0001 | (offset  2));
  +     b43_read32(dev, B43_MMIO_SHM_CONTROL);
  +     if (offset  2)
  +             b43_write16(dev, 0x165, value);
  +     else
  +             b43_write16(dev, B43_MMIO_SHM_DATA, value);
  +}
 
  I'd like to put a bg questionmark on this.
  We already have SHM access. Your function does exactly the same, except 
  that it
  accesses the bullshit register h165.
 
 Yeah, that 0x165 is really crappy. Let's hope it really should be
 0x166 (B43_MMIO_SHM_DATA_UNALIGNED).
 
 Do you think we should add dummy-read to b43_shm_control_word as
 BmacWriteShm does? Currently we do not perform that.

That doesn't hurt.
It probably won't help much either, as the whole driver depends on implicit 
write posting.
But I don't care whether you add it or not.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: LP-PHY Fatal DMA error 0x00000800 on non-ULV Core 2 Duo?!?!!??!

2010-02-27 Thread Michael Buesch
On Saturday 27 February 2010 16:55:09 Larry Finger wrote:
 On 02/27/2010 09:16 AM, Michael Buesch wrote:
  On Friday 26 February 2010 23:03:28 Gábor Stefanik wrote:
  BTW there is an interesting difference in the early init between wl
  and b43: b43 sets bit 0x200 in core register 0x600, while wl sets
  0x8000 in register 0x280a - an undocumented register.
  
  Well, it is not only undocumented, it's also far beyond the address space.
  SSB core address space goes from 0-0xFFF. And (more important!) the PCI 
  address
  space for MMIO is only 8k wide (I think there also are 4k devices).
  Are you really sure your dumping is correct? I suspect a bug in your mmio
  dumping tools.
  I'd say the undocumented register is a completely unrelated hardware 
  access
  in a completely different device that just happens to be mapped right after 
  ssb.
 
 There are definite instances where the vendor driver writes beyond 0xFFF. I'm
 pretty sure that some models shadow EEPROM above 0x1000. I don't know about 
 0x280A.

Well, writing beyond 0xFFF is one thing. But writing beyond 0x2000 is another.
The latter one writes beyond the mapped PCI space. Which basically writes to 
random
memory or whatever happens to be mapped there (arch dependent).

I would also not rule out typos in the vendor driver.
So, you might try if that particular write fixes the problem. But I doubt it 
can do so.
And if it doesn't, we should not apply it, because it's obviously incorrect.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: LP-PHY Fatal DMA error 0x00000800 on non-ULV Core 2 Duo?!?!!??!

2010-02-27 Thread Michael Buesch
On Saturday 27 February 2010 17:05:41 Larry Finger wrote:
 On 02/27/2010 09:20 AM, Michael Buesch wrote:
  On Saturday 27 February 2010 02:41:48 Gábor Stefanik wrote:
  Someone should test the following:
  -Open drivers/ssb/driver_chipcommon_pmu.c
  -In ssb_pmu_init, replace 0x4325 with 0x4312.
  
  Uh, wait a second. Do _all_ cards that show the behavior have a PMU on the 
  SSB?
  If so, you should really make sure the PMU setup is correct. If the PMU
  cuts power to the device at inappropriate times, it sure does result in DMA 
  errors (and
  silent MMIO errors).
 
 Yes, all of the LP PHY 14e4:4315 devices have a PMU. Mine shows Found rev 1 
 PMU
 (capabilities 0x02A62F01). As I recall, at least one of the problem devices
 shows the same capabilities.

*HINT HINT HINT* ;)
Please make sure the PMU code really is correct.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: LP-PHY Fatal DMA error 0x00000800 on non-ULV Core 2 Duo?!?!!??!

2010-02-27 Thread Michael Buesch
On Saturday 27 February 2010 20:45:50 Gábor Stefanik wrote:
 2. Just before the SPROM readout, we are missing a Set 0x8000 in
 MMIO offset 0x280a. This looks curiously like PCI-E Miscellaneous
 Configuration, from http://bcm-v4.sipsolutions.net/PCI-E - and
 indeed, the value read out from 0x280a is identical to offset 0xa in
 my SPROM. So, the right thing to do here is probably Switch to the
 PCIE core, set 0x8000 in MIMO offset 0x80a (or maybe 0x1000a?), switch
 back to ChipCommon. I don't know what core is active during this

Well, 0x280a and 0x80a basically is not the same in my part of the world.
To ask it again: Is it possible that the 0x2000 bit of the address comes from
some random burp/bug in the tracing code? It _really_ doesn't make any sense,
because it's beyond the mapped space of the device.

 write, as mmiotrace doesn't catch pci_read/write_config_dword calls.
 Larry, do you know a way to monitor PCI config space access in a way
 that can be matched (e.g. using timestamps) to the MMIO trace?
 3. Right after the SPROM is read, wl writes zeros to MMIO offsets
 0x58 and 0x5c (32-bit), then does the PMU setup. These are not valid
 registers for ChipCommon and PCIE, but for 802.11, they fall directly
 into the DMA area!

The writes (_if_ they happen on the 802.11 core) fall into the
interrupt status and masks of the 8th DMA engine (they do not fall into the
MMIO of the DMA engines).
We don't use the 8th engine. It probably is a good idea to write zeros
to the mask register anyway, so I'll spin up a quick test patch.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: Unexpected behavior observed in handling IEEE80211_FCTL_PM bit with b43 driver

2010-02-21 Thread Michael Buesch
PS is currently not supported on b43.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: skb_over_panic from b43 after a few hours

2010-02-19 Thread Michael Buesch
On Friday 19 February 2010 16:40:40 Larry Finger wrote:
  [8018c2d4] skb_put+0x74/0x90
  [80c9a8e4] b43_dma_rx+0x338/0x444 [b43]
  [80c87420] b43_controller_restart+0x7a0/0x974 [b43]
 
 
 The traceback indicates a controller restart. Does your log show any reason 
 for
 that event? That may help me understand why skb-tail is past skb-end.

Note that on bcm47xx backtraces are _extremely_ unreliable.
As b43_controller_restart does not call b43_dma_rx, yet it shows it
this way in the backtrace, I think that line is just invalid.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: skb_over_panic from b43 after a few hours

2010-02-19 Thread Michael Buesch
On Friday 19 February 2010 21:28:01 Stefan Wahren wrote:
 @Michael: Okay, is there any method to get more reliable information
 about the kernel panic (strace, ...) or a idea to faster reproduce the
 kernel panic than waiting?

I think the backtrace is pretty good as-is, if you ignore the controller_restart
line. It's pretty obvious that this is an skb overflow panic caused by a
received packet. There's nothing in the trace that falsifies this, as far as I
can see.
So what if you set up another device in monitor mode and simply capture all 
packets
until the error happens again? If the panic happened by receiving a malformed
packet, it should be obvious to see then.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 08/11] b43: implement writing to MMIO shared memory

2010-02-09 Thread Michael Buesch
On Tuesday 09 February 2010 21:04:40 Rafał Miłecki wrote:
 Signed-off-by: Rafał Miłecki zaj...@gmail.com
 ---
  drivers/net/wireless/b43/phy_common.c |   11 +++
  drivers/net/wireless/b43/phy_common.h |2 ++
  2 files changed, 13 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/net/wireless/b43/phy_common.c 
 b/drivers/net/wireless/b43/phy_common.c
 index 8f7d7ef..0b0f9df 100644
 --- a/drivers/net/wireless/b43/phy_common.c
 +++ b/drivers/net/wireless/b43/phy_common.c
 @@ -466,3 +466,14 @@ struct b43_c32 b43_cordic(int theta)
  
   return ret;
  }
 +
 +/* http://bcm-v4.sipsolutions.net/802.11/PHY/BmacWriteShm */
 +void b43_bmac_write_shm(struct b43_wldev *dev, u32 offset, u16 value)
 +{
 + b43_write32(dev, B43_MMIO_SHM_CONTROL, 0x0001 | (offset  2));
 + b43_read32(dev, B43_MMIO_SHM_CONTROL);
 + if (offset  2)
 + b43_write16(dev, 0x165, value);
 + else
 + b43_write16(dev, B43_MMIO_SHM_DATA, value);
 +}

I'd like to put a bg questionmark on this.
We already have SHM access. Your function does exactly the same, except that it
accesses the bullshit register h165.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 09/11] b43: N-PHY: isloate 2055 radio setup

2010-02-09 Thread Michael Buesch
On Tuesday 09 February 2010 21:04:41 Rafał Miłecki wrote:
 Signed-off-by: Rafał Miłecki zaj...@gmail.com
 ---
  drivers/net/wireless/b43/phy_n.c |   24 ++--
  1 files changed, 18 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/net/wireless/b43/phy_n.c 
 b/drivers/net/wireless/b43/phy_n.c
 index 835d0da..631e01f 100644
 --- a/drivers/net/wireless/b43/phy_n.c
 +++ b/drivers/net/wireless/b43/phy_n.c
 @@ -155,6 +155,23 @@ static void b43_nphy_tx_power_fix(struct b43_wldev *dev)
   //TODO
  }
  
 +
 +/* http://bcm-v4.sipsolutions.net/802.11/PHY/Radio/2055Setup */
 +static void b43_radio_2055_setup(struct b43_wldev *dev,
 + const struct b43_nphy_channeltab_entry *e)
 +{
 + B43_WARN_ON(dev-phy.rev = 3);
 +
 + b43_chantab_radio_upload(dev, e);
 + udelay(50);
 + b43_radio_write(dev, B2055_VCO_CAL10, 5);
 + b43_radio_write(dev, B2055_VCO_CAL10, 45);
 + if (dev-dev-bus-bustype == SSB_BUSTYPE_PCI)
 + b43_read32(dev, B43_MMIO_MACCTL);

Just read MACCTL unconditionally. It obviously is just used for bus posting.
We usually also add a comment.

b43_read32(dev, B43_MMIO_MACCTL); /* flush writes */

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 01/11] b43: N-PHY: add some registers and structs definitions

2010-02-09 Thread Michael Buesch
On Tuesday 09 February 2010 21:04:33 Rafał Miłecki wrote:
 +#define B43_MMIO_PSM_PHY_HDR 0x492   /* programmable state machine */

The comment doesn't make a lot of sense.
In case you don't know, the PSM is the part of the hardware
that executes the firmware.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: b43legacy-phy3: Radio hardware status changed to XXX

2010-02-07 Thread Michael Buesch
On Sunday 07 February 2010 13:48:59 Rafał Miłecki wrote:
 2010/2/7 strk s...@keybit.net:
  On Sat, Feb 06, 2010 at 06:54:44PM -0600, Larry Finger wrote:
 
  You can get the wireless-compat sources for kernel 2.6.26 and build that.
  Othersise, go to kernel.org and get the full sources.
 
  I opted for an upgrade first (debian lenny to debian squeeze).
  So I'm now at kernel version 2.6.32 and the problem is somehow
  different (still related to kill switch).
 
  The messages in this case are more clear (on ifconfig wlan0 ):
 
  b43legacy ssb0:0: firmware: requesting b3legacy/ucode2.fw
  b43legacy ssb0:0: firmware: requesting b3legacy/pcm4.fw
  b43legacy ssb0:0: firmware: requesting b3legacy/b0g0initvals2.fw
  b43legacy-phy0: Loading firmware version 0x127, patch level 14 (2005-04-18 
  02:36:27)
  b43legacy-phy0: Radio hardware status changed to DISABLED
  b43legacy-phy0: Radio turned on by software
  b43legacy-phy0: The hardware RF-kill button still turns the radio 
  physically off. Press th ebutton to turn it on.
 
 I'm not sure... if radio is turned off physically (by hardware) can
 compiling without RFKILL help at all? AFAIU rfkill is just for
 disabling radio by software...

rfkill is a hardware lock in the case of broadcom. Software can only read the 
state.

(There may be laptops with broadcom cards where rfkill can be changed by 
software.
But that's done by other means (BIOS) and the broadcom hardware doesn't know 
about it.)

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: b43legacy-phy3: Radio hardware status changed to XXX

2010-02-07 Thread Michael Buesch
On Sunday 07 February 2010 14:18:43 Rafał Miłecki wrote:
 So pressing something like Fn+FX can eventually be handled by BIOS and
 can cause hardware (physical) enabling radio?

Yeah, could be the case. It could also be handled by another kernel driver 
(wmi).
In no case will the wireless driver be able to modify the state.

 In such a case Broadcom hardware doesn't know about this (wow, that's
 weird), and so b43 driver doesn't know as well? As the result b43
 doesn't try to run wireless.

Well, the only thing the driver can do is _observing_ that the radio is killed.

 If we compile without rfkill we won't check for radio status, and we
 will just try to use radio, that may be enabled at some point by BIOS.

Yeah.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: b43legacy-phy3: Radio hardware status changed to XXX

2010-02-07 Thread Michael Buesch
On Sunday 07 February 2010 15:09:05 Rafał Miłecki wrote:
 W dniu 7 lutego 2010 14:22 użytkownik Michael Buesch m...@bu3sch.de napisał:
  On Sunday 07 February 2010 14:18:43 Rafał Miłecki wrote:
  So pressing something like Fn+FX can eventually be handled by BIOS and
  can cause hardware (physical) enabling radio?
 
  Yeah, could be the case. It could also be handled by another kernel driver 
  (wmi).
  In no case will the wireless driver be able to modify the state.
 
  In such a case Broadcom hardware doesn't know about this (wow, that's
  weird), and so b43 driver doesn't know as well? As the result b43
  doesn't try to run wireless.
 
  Well, the only thing the driver can do is _observing_ that the radio is 
  killed.
 
 Hm, AFAIR my old Acer Aspire 5024 was using acer-wmi module to manage
 (turn off) rfkill. I was using this with b43 driver so somehow in my
 case b43 (and hardware?) did know state of radio state... But you
 wrote:
 
  But that's done by other means (BIOS) and the broadcom hardware doesn't 
  know about it
 
 so finally... when we are able to know rfkill status and when we are
 not? Is this like following?
 1) If BIOS handles rfkill we don't know status of radio and radio
 doesn't know as well
 2) If something like WMI handles rfkill, we know status of radio
 ?

We always know about the status of the radio (rfkill). We just can't
modify it from within the broadcom device.
The rfkill signal is a physical signal that goes into the broadcom radio.
If that signal is turned to kill by the BIOS, there's nothing the broadcom
chip can do about it. It will just show the state of that physical line to
the driver via a bit in a register or via IRQ.

So if rfkill can be switched by software, that is totally unrelated to b43.
It's done by completely unrelated hardware registers that are not on the 
broadcom device
or by other means (BIOS calls, etc...).

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: b43legacy-phy3: Radio hardware status changed to XXX

2010-02-07 Thread Michael Buesch
On Sunday 07 February 2010 15:30:18 strk wrote:
 On Sun, Feb 07, 2010 at 02:05:09PM +0100, Michael Buesch wrote:
 
  rfkill is a hardware lock in the case of broadcom. Software can only read 
  the state.
 
  (There may be laptops with broadcom cards where rfkill can be changed by 
  software.
  But that's done by other means (BIOS) and the broadcom hardware doesn't 
  know about it.)
 
 Uhm, but still I'm pretty sure there's no physical switch on this laptop 
 (L5800C).
 Also, if there was one, surely I wasn't turning it on and off continuosly to
 justify those notices coming down to b43legacy.
 
 Note that bcm43xx used to work fine. 

I'm not talking about possible bugs in the software. I'm just explaining
how the hardware works, because I think there's some confusion here.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: b43legacy-phy3: Radio hardware status changed to XXX

2010-02-07 Thread Michael Buesch
On Sunday 07 February 2010 15:44:01 strk wrote:
 On Sun, Feb 07, 2010 at 03:37:36PM +0100, Michael Buesch wrote:
  On Sunday 07 February 2010 15:30:18 strk wrote:
   On Sun, Feb 07, 2010 at 02:05:09PM +0100, Michael Buesch wrote:
   
rfkill is a hardware lock in the case of broadcom. Software can only 
read the state.
   
(There may be laptops with broadcom cards where rfkill can be changed 
by software.
But that's done by other means (BIOS) and the broadcom hardware doesn't 
know about it.)
   
   Uhm, but still I'm pretty sure there's no physical switch on this laptop 
   (L5800C).
   Also, if there was one, surely I wasn't turning it on and off continuosly 
   to
   justify those notices coming down to b43legacy.
   
   Note that bcm43xx used to work fine. 
  
  I'm not talking about possible bugs in the software. I'm just explaining
  how the hardware works, because I think there's some confusion here.
 
 Ok, so supposedly here the situation can be on of these:
 
  1) b43legacy driver is wrong while reading kill switch state
  2) something (software) is toggling kill switch 
 
 Would case 1 be fixed by having b43legacy NOT check the state ?

Well, it would not be a fix, but it would possibly be a workaround for it.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: b43legacy-phy3: Radio hardware status changed to XXX

2010-02-07 Thread Michael Buesch
On Sunday 07 February 2010 15:59:53 strk wrote:
 On Sun, Feb 07, 2010 at 03:48:07PM +0100, Michael Buesch wrote:
  On Sunday 07 February 2010 15:44:01 strk wrote:
 
   Ok, so supposedly here the situation can be on of these:
   
1) b43legacy driver is wrong while reading kill switch state
2) something (software) is toggling kill switch 
   
   Would case 1 be fixed by having b43legacy NOT check the state ?
  
  Well, it would not be a fix, but it would possibly be a workaround for it.
 
 If it *does* work as a workaround it'd be a good reason to add 
 a load-time option to the driver (allow workaround to bogus behaviours).
 I'll let you know if it does work.

We are not going to add module options to workaround bugs. We fix the bug 
instead.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH] b43/b43legacy: Wake queues in wireless_core_start

2010-02-03 Thread Michael Buesch
On Wednesday 03 February 2010 20:33:44 Larry Finger wrote:
 If b43 or b43legacy are deauthenticated or disconnected, there is a
 possibility that a reconnection is tried with the queues stopped in
 mac80211. To prevent this, start the queues before setting
 STAT_INITIALIZED.
 
 In b43, a similar change has been in place (twice) in the
 wireless_core_init() routine. Remove the duplicate and add similar
 code to b43legacy.
 
 Signed-off-by: Larry Finger larry.fin...@lwfinger.net
 Cc: Stable sta...@kernel.org   [2.6.32]
 ---
 
 John,
 
 The b43 patch to wireless_core_start() seems to help a regression
 between 2.6.31 and 2.6.32. Accordingly, these changes should be
 applied to 2.6.33 with the automatic backport to 2.6.32.
 
 Larry
 ---
 
 Index: wireless-testing/drivers/net/wireless/b43/main.c
 ===
 --- wireless-testing.orig/drivers/net/wireless/b43/main.c
 +++ wireless-testing/drivers/net/wireless/b43/main.c
 @@ -3980,6 +3980,7 @@ static int b43_wireless_core_start(struc
   }
  
   /* We are ready to run. */
 + ieee80211_wake_queues(dev-wl-hw);
   b43_set_status(dev, B43_STAT_STARTED);
  
   /* Start data flow (TX/RX). */
 @@ -4389,8 +4390,6 @@ static int b43_wireless_core_init(struct
  
   ieee80211_wake_queues(dev-wl-hw);
  
 - ieee80211_wake_queues(dev-wl-hw);
 -
   b43_set_status(dev, B43_STAT_INITIALIZED);

Well, I wonder why it makes a difference.
I think we only call core_start() right after core_init() calls.

Anyway, I think wake_queues should both be removed from core_init()
and queues should only be woken in core_start.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 2/4] b43: make cordic common (LP-PHY and N-PHY need it)

2010-01-25 Thread Michael Buesch
On Monday 25 January 2010 18:59:59 Rafał Miłecki wrote:
 +/* Complex number using 2 32-bit signed integers */
 +typedef struct { s32 i, q; } b43_c32;

No typedef. ever.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 2/4] b43: make cordic common (LP-PHY and N-PHY need it)

2010-01-25 Thread Michael Buesch
On Monday 25 January 2010 19:36:27 Rafał Miłecki wrote:
 W dniu 25 stycznia 2010 19:35 użytkownik Rafał Miłecki
 zaj...@gmail.com napisał:
  2010/1/25 Michael Buesch m...@bu3sch.de:
  On Monday 25 January 2010 18:59:59 Rafał Miłecki wrote:
  +/* Complex number using 2 32-bit signed integers */
  +typedef struct { s32 i, q; } b43_c32;
 
  No typedef. ever.
 
  Well, I just copied (Gabor's?) code here. But of course I can fix this
  by the way, no problem :)

Yeah, I saw that. We can fix it while we're at it. ;)

  Just read about typedef in Linux Kernel Coding Style, didn't know
  about this earlier. Thanks for pointing.
 
 Is this OK to fix this in separated patch? Or should I modify this set
 of patches?

Well, as you touch any reference to the typedef anyway (you renamed it),
you can just put the keyword struct in front of the references and no 
separate patch is needed.
It won't even grow your current patch in the number of changed lines.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


[PATCH] b43: Workaround circular locking in hw-tkip key update callback

2010-01-24 Thread Michael Buesch
The TKIP key update callback is called from the RX path, where the driver
mutex is already locked. This results in a circular locking bug.
Avoid this by removing the lock.

Johannes noted that there is a separate bug: The callback still breaks on SDIO
hardware, because SDIO hardware access needs to sleep, but we are not allowed
to sleep in the callback due to mac80211's RCU locking.

Signed-off-by: Michael Buesch m...@bu3sch.de
Tested-by: Larry Finger larry.fin...@lwfinger.net
Reported-by: ke...@kutfo.hit.bme.hu
Cc: Johannes Berg johan...@sipsolutions.net
Cc: stable sta...@kernel.org

---
 drivers/net/wireless/b43/main.c |   13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

--- wireless-testing.orig/drivers/net/wireless/b43/main.c
+++ wireless-testing/drivers/net/wireless/b43/main.c
@@ -856,22 +856,19 @@ static void b43_op_update_tkip_key(struc
if (B43_WARN_ON(!modparam_hwtkip))
return;
 
-   mutex_lock(wl-mutex);
-
+   /* This is only called from the RX path through mac80211, where
+* our mutex is already locked. */
+   B43_WARN_ON(!mutex_is_locked(wl-mutex));
dev = wl-current_dev;
-   if (!dev || b43_status(dev)  B43_STAT_INITIALIZED)
-   goto out_unlock;
+   B43_WARN_ON(!dev || b43_status(dev)  B43_STAT_INITIALIZED);
 
keymac_write(dev, index, NULL); /* First zero out mac to avoid race */
 
rx_tkip_phase1_write(dev, index, iv32, phase1key);
/* only pairwise TKIP keys are supported right now */
if (WARN_ON(!sta))
-   goto out_unlock;
+   return;
keymac_write(dev, index, sta-addr);
-
-out_unlock:
-   mutex_unlock(wl-mutex);
 }
 
 static void do_key_write(struct b43_wldev *dev,

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 0/5] b43: more N-PHY stuff

2010-01-23 Thread Michael Buesch
On Saturday 23 January 2010 04:36:50 David Woodhouse wrote:
 On Fri, 2010-01-22 at 20:16 -0500, John W. Linville wrote:
  On Fri, Jan 22, 2010 at 11:33:40PM +0100, Michael Buesch wrote:
  
   So while we are at it, I'd really like to migrate away from the berlios 
   list.
   It's really just annoying. Does somebody have a good reliable mailinglist 
   service
   we could migrate to? Does vger offer lists to driver projects?
  
  Probably -- I think davem is the person to ask?  Infradead is probably
  another option (dwmw2).
 
 Yeah, pick one and either of us can set up a new list at the drop of a
 hat. Strictly speaking I suspect it should be postmas...@vger rather
 than just davem, but the effect is much the same most of the time.
 
 Can you provide a list of existing subscribers?

I think Stefano has access to our berlios mailman.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 0/5] b43: more N-PHY stuff

2010-01-22 Thread Michael Buesch
On Friday 22 January 2010 21:47:03 Rafał Miłecki wrote:
 As you can see I've used git send-email for submission this time! I've
 no idea what I did wrong that patch 1/5 was sent incorrectly. I just
 generated patches with
 git format-patch --cover-letter -o b43
 , then modified -...patch (ONLY this one) and finally sent all
 patches from b43 directory.

The mails may be re-encoded on intermediate mailservers.
My server, for example, re-encodes all of your mails to base64.
(or it may be the list-mailserver re-encoding the mails, because by server
doesn't advertise the correct flags, I dunno...)
I don't know why it does this, however. I'm still pretty sure that it's related
to the characters in your name. Does anybody has an idea how to fix this? I'm 
using exim.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 0/5] b43: more N-PHY stuff

2010-01-22 Thread Michael Buesch
On Friday 22 January 2010 22:59:26 Michael Buesch wrote:
 On Friday 22 January 2010 21:47:03 Rafał Miłecki wrote:
  As you can see I've used git send-email for submission this time! I've
  no idea what I did wrong that patch 1/5 was sent incorrectly. I just
  generated patches with
  git format-patch --cover-letter -o b43
  , then modified -...patch (ONLY this one) and finally sent all
  patches from b43 directory.
 
 The mails may be re-encoded on intermediate mailservers.
 My server, for example, re-encodes all of your mails to base64.
 (or it may be the list-mailserver re-encoding the mails, because by server
 doesn't advertise the correct flags, I dunno...)
 I don't know why it does this, however. I'm still pretty sure that it's 
 related
 to the characters in your name. Does anybody has an idea how to fix this? I'm 
 using exim.
 

Oh, I just realize that these patches received via vger.kernel.org list are 
just fine.
It's just berlios' mailman which mangles the mails for me. Which is not 
surprising,
because berlios is a really really broken platform and its mailservers are even 
worse than that.

So I guess everything is fine now.

So while we are at it, I'd really like to migrate away from the berlios list.
It's really just annoying. Does somebody have a good reliable mailinglist 
service
we could migrate to? Does vger offer lists to driver projects?

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 5/5 V2] b43: N-PHY: silence warnings, add missing call

2010-01-20 Thread Michael Buesch
On Tuesday 19 January 2010 23:59:32 Rafał Miłecki wrote:
 Out of curiosity, could someone tell what Opera does incorrectly when
 sending patches?

They are base64 encoded.
We use a 7 or 8bit encoding for our mails.
I think that opera automatically switches to base64 encoding due to
the special characters in your first and last name.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 2/7] b43: N-PHY: implement TX PHY cleanup and setup

2010-01-17 Thread Michael Buesch
On Sunday 17 January 2010 17:37:29 Gábor Stefanik wrote:
 While you are at it, why aren't you programming these table writes?
 The table handling functions are AFAIK already in place...

Because we like small patches.
Please do _not_ increase the patchsize.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 1/4] b43: N-PHY: add writing one element tables

2010-01-17 Thread Michael Buesch
On Sunday 17 January 2010 23:37:50 Rafał Miłecki wrote:
 @@ -1895,14 +1880,12 @@ static int b43_nphy_cal_tx_iq_lo(struct b43_wldev 
 *dev,
   b43_phy_write(dev, B43_NPHY_IQLOCAL_CMDNNUM, tmp);
 
   if (type == 1 || type == 3 || type == 4) {
 - /* TODO: Read an N PHY Table with ID 15,
 - length 1, offset 69 + core,
 - width 16, and data pointer buffer */
 + b43_ntab_write(dev, B43_NTAB16(15, 69 + core),
 + buffer[0]);
   diq_start = buffer[0];
   buffer[0] = 0;
 - /* TODO: Write an N PHY Table with ID 15,
 - length 1, offset 69 + core, width 16,
 - and data of 0 */
 + b43_ntab_write(dev, B43_NTAB16(15, 69 + core),
 + 0);
   }
 
   b43_phy_write(dev, B43_NPHY_IQLOCAL_CMD, cmd);

What is this supposed to do? It writes a value to a table entry and immediately
clobbers the entry with 0. This doesn't make any sense.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 1/2] b43: make finding the most significant bit common function

2010-01-14 Thread Michael Buesch
On Thursday 14 January 2010 13:21:27 Rafał Miłecki wrote:
 
 Signed-off-by: Rafał Miłecki zaj...@gmail.com
 ---
   drivers/net/wireless/b43/phy_common.c |   14 ++
   drivers/net/wireless/b43/phy_common.h |1 +
   drivers/net/wireless/b43/phy_lp.c |   17 ++---
   3 files changed, 17 insertions(+), 15 deletions(-)
 
 diff --git a/drivers/net/wireless/b43/phy_common.c 
 b/drivers/net/wireless/b43/phy_common.c
 index 75b26e1..1389ab2 100644
 --- a/drivers/net/wireless/b43/phy_common.c
 +++ b/drivers/net/wireless/b43/phy_common.c
 @@ -421,3 +421,17 @@ void b43_phyop_switch_analog_generic(struct b43_wldev 
 *dev, bool on)
   {
   b43_write16(dev, B43_MMIO_PHY0, on ? 0 : 0xF4);
   }
 +
 +/* Find the position of the most significant bit */
 +u8 phy_nbits(s32 val)
 +{
 + u32 tmp = abs(val);
 + u8 nbits = 0;
 +
 + while (tmp != 0) {
 + nbits++;
 + tmp = 1;
 + }
 +
 + return nbits;
 +}

We already have fls() in bitops.h
I think the following call would be equivalent:

x = fls(abs(val));

So we don't need our own implementation in phy_common.c.
Just use the standard funcs.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 1/3] b43: N-PHY: add b43_nphy_rx_iq_est and b43_nphy_tx_iq_workaround

2010-01-12 Thread Michael Buesch
On Tuesday 12 January 2010 20:38:07 Rafał Miłecki wrote:
   struct nphy_txgains { u16 txgm[2]; u16 pga[2]; u16 pad[2]; u16 ipa[2]; };
 +struct nphy_iq_est { s32 iq0_prod; u32 i0_pwr; u32 q0_pwr; s32 iq1_prod;
 + u32 i1_pwr; u32 q1_pwr; };

So it seems I didn't notice this earlier, but this violates kernel coding style.
Please do a separate patch that converts all structs from
struct foo { a; b; c; };
to
struct foo {
a;
b;
c;
};

 + if (wait)
 + b43_phy_set(dev, B43_NPHY_IQEST_CMD, B43_NPHY_IQEST_CMD_MODE);
 + else
 + b43_phy_mask(dev, B43_NPHY_IQEST_CMD, B43_NPHY_IQEST_CMD_MODE);

Looks wrong to me.
Do you want this?

+   if (wait)
+   b43_phy_set(dev, B43_NPHY_IQEST_CMD, B43_NPHY_IQEST_CMD_MODE);
+   else
+   b43_phy_mask(dev, B43_NPHY_IQEST_CMD, ~B43_NPHY_IQEST_CMD_MODE);

 +/* http://bcm-v4.sipsolutions.net/802.11/PHY/N/TxIqWar */
 +static void b43_nphy_tx_iq_workaround(struct b43_wldev *dev)
 +{
 + u16 array[4];
 + int i;
 +
 + b43_phy_write(dev, B43_NPHY_TABLE_ADDR, 0x3C50);
 + for (i = 0; i  4; i++)
 + array[i] = b43_phy_read(dev, B43_NPHY_TABLE_DATALO);
 +
 + b43_shm_write16(dev, B43_SHM_SHARED, 0x0700, array[0]);
 + b43_shm_write16(dev, B43_SHM_SHARED, 0x0702, array[1]);
 + b43_shm_write16(dev, B43_SHM_SHARED, 0x0704, array[2]);
 + b43_shm_write16(dev, B43_SHM_SHARED, 0x0706, array[3]);

Do we have some names for the SHM locations?
If not, just do some defines.
#define B43_SHM_SH_NPHY_TXIQW0  0x0700
#define B43_SHM_SH_NPHY_TXIQW1  0x0702
...
in b43.h (next to the other SHM_SH defines)

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 3/3] b43: N-PHY: add RX IQ calculation for rev 3

2010-01-12 Thread Michael Buesch
On Tuesday 12 January 2010 20:38:56 Rafał Miłecki wrote:
 
 Signed-off-by: Rafał Miłecki zaj...@gmail.com
 ---
 
 Uh, bigger one. This patch causes false warning:
 drivers/net/wireless/b43/phy_n.c: In function ‘b43_nphy_rev2_cal_rx_iq’:
 drivers/net/wireless/b43/phy_n.c:627: warning: large integer implicitly 
 truncated to unsigned type
 
 That's for:

I think the warning is caused by

b43_phy_maskset(dev, B43_NPHY_RFSEQCA,
THIS   ~B43_NPHY_RFSEQCA_RXDIS,
((1 - i)  B43_NPHY_RFSEQCA_RXDIS_SHIFT));

Well, I think the warning is pretty much bogus, but I think it can be worked 
around
by doing
(u16)~B43_NPHY_RFSEQCA_RXDIS


-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 2/6] b43: N-PHY: add RSSI functions: poll and set 2055 vcm

2010-01-11 Thread Michael Buesch
On Monday 11 January 2010 03:38:53 Larry Finger wrote:
 Yes, my fault. The specs are now corrected so that these statements are
 
 ((s8)((s[1]  8)  0x3F)  2)  2
 
 I think that is right.

No it is not.

You need to do this:
(s8)(((s[1]  8)  0x3F)  2)  2

Alternatively add another pair of parenthesis to make it clearer:

((s8)(((s[1]  8)  0x3F)  2))  2

This basically shifts left unsigned and then shifts right _arithmetically_.
In your example, the compiler will optimize both shifts away (it may actually
also optimize the shifts in my case, but the sign extension will persist.

Just try it and you'll see:

m...@homer:~$ cat t.c
#include stdio.h
#include stdint.h

int main()
{
int8_t s0;
int8_t s1;
uint8_t u;

u = 0x3D;

s0 = ((int8_t)(u  0x3F)  2)  2;
s1 = ((int8_t)((u  0x3F)  2))  2;

printf(%d %d\n, s0, s1);
}
m...@homer:~$ gcc -o t t.c
m...@homer:~$ ./t
61 -3


-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 3/6] b43: N-PHY: add RSSI calculation for PHY rev 3

2010-01-11 Thread Michael Buesch
On Monday 11 January 2010 22:13:31 Rafał Miłecki wrote:
 2010/1/10 Michael Buesch m...@bu3sch.de:
  On Sunday 10 January 2010 23:13:34 Rafał Miłecki wrote:
  +     s32 results_min[4];
  +     u8 vcm_final[4];
  +     s32 results[4][4];
  +     s32 miniq[4][2];
  +     memset(results_min, 0, sizeof(s32) * 4);
  +     memset(vcm_final, 0, sizeof(u8) * 4);
  +     memset(results, 0, sizeof(s32) * 4 * 4);
  +     memset(miniq, 0, sizeof(s32) * 4 * 2);
 
  Just initialize the variables to zero instead of doing a memset:
 
  +       s32 results_min[4] = { 0, };
  +       u8 vcm_final[4] = { 0, };
  +       s32 results[4][4] = { 0, };
  +       s32 miniq[4][2] = { 0, };
 
 Nice trick, thanks :) Just for two-dimensional arrays I'll have to hack it to:
 s32 results[4][4] = { { 0, }, { 0, }, { 0, }, { 0, } };
 I believe.

No I don't think so.
It's C standard that uninitialized elements on automatic variables are 
initialized
to zero, _if_ at least one element is initialized to something.
So if you init one element to 0, all others will be 0, too.
I think that should also work for multidimensional arrays. So my example
s32 results[4][4] = { 0, };
should do the right thing. Am I wrong?

Here's a test-program:


m...@maggie:~$ cat t.c
#include stdio.h
#include stdint.h

int main(void)
{
int i, j;
int32_t results[4][4]
#ifdef INIT_IT
= { 0, };
#else
;
#endif

for (i = 0; i  4; i++)
for (j = 0; j  4; j++)
printf(%d\n, results[i][j]);

}
m...@maggie:~$ gcc -o t t.c
m...@maggie:~$ ./t
0
0
0
1
-1077483840
0
0
0
-1077483824
268436224
268536212
0
-1077483776
268436888
268353524
0
m...@maggie:~$ gcc -D INIT_IT -o t t.c
m...@maggie:~$ ./t
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 3/6] b43: N-PHY: add RSSI calculation for PHY rev 3

2010-01-11 Thread Michael Buesch
On Monday 11 January 2010 23:27:18 Rafał Miłecki wrote:
 Whoops, I should have explained what I mean. I am not sure what CFLAGS
 make picks for compiled but I get:
 
   CC [M]  drivers/net/wireless/b43/phy_n.o
 drivers/net/wireless/b43/phy_n.c: In function ‘b43_nphy_rev2_rssi_cal’:
 drivers/net/wireless/b43/phy_n.c:887: warning: missing braces around 
 initializer
 drivers/net/wireless/b43/phy_n.c:887: warning: (near initialization
 for ‘results[0]’)
 
 for s32 results[4][4] = { 0, };
 

But does this work?
s32 results[4][4] = { };

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 2/6] b43: N-PHY: add RSSI functions: poll and set 2055 vcm

2010-01-10 Thread Michael Buesch
On Sunday 10 January 2010 23:13:20 Rafał Miłecki wrote:
 + buf[0] += (s8)(((s[0]  0x3F)  2)  2);
 + buf[1] += (s8)s[0]  8)  0x3F)  2)  2);
 + buf[2] += (s8)(((s[1]  0x3F)  2)  2);
 + buf[3] += (s8)s[1]  8)  0x3F)  2)  2);

I suggest buf[3] += (s8)s[1]  8)  0x3F)  2)  2)  2)  2)  2)  
2)  2)  2)  2)  2)  2)  2)  2)  2)
;)
No, seriously, why shift left and then shift right? Is this a translation error?
I _guess_ it's some mistranslation of the sign extension going on.
Or alternatively a compiler going insane on sign extension.

The question is: Do we want these integers to be signextended or not?

What we currently do is this:
buf[3] += (s8)((s[1]  8)  0x3F);

which will always result in a positive 8bit integer, as far as I can see.
Which smells fishy.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 3/6] b43: N-PHY: add RSSI calculation for PHY rev 3

2010-01-10 Thread Michael Buesch
On Sunday 10 January 2010 23:13:34 Rafał Miłecki wrote:
 + s32 results_min[4];
 + u8 vcm_final[4];
 + s32 results[4][4];
 + s32 miniq[4][2];
 + memset(results_min, 0, sizeof(s32) * 4);
 + memset(vcm_final, 0, sizeof(u8) * 4);
 + memset(results, 0, sizeof(s32) * 4 * 4);
 + memset(miniq, 0, sizeof(s32) * 4 * 2);

Just initialize the variables to zero instead of doing a memset:

+   s32 results_min[4] = { 0, };
+   u8 vcm_final[4] = { 0, };
+   s32 results[4][4] = { 0, };
+   s32 miniq[4][2] = { 0, };

 + b43_radio_maskset(dev, B2055_C1_PD_RSSIMISC, 0xF8, 0);
 + b43_radio_maskset(dev, B2055_C2_PD_RSSIMISC, 0xF8, 0);

If you don't set anything, you can use
b43_radio_mask();

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: PHY: where to put forcing gater clocks function

2010-01-08 Thread Michael Buesch
On Friday 08 January 2010 09:30:27 Rafał Miłecki wrote:
 I need to implement
 http://bcm-v4.sipsolutions.net/802.11/PHY/ClkFgc
 
 Where I can put my
 void b43_phy_clock_fgc(struct b43_wldev *dev, bool clock) { ... }
 ? Is end of phy_common.c file OK for this?

yes. Note that the 0x2 bit has defines in ssb_regs.h. So you should use 
them.
Also use ssb_read/write to access that register. It's essentially the same
as b43_read/write, but this way it's obvious to humans that we are accessing a 
backplane register.
Also maybe don't call the parameter clock but force. Would make more sense 
to me...

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: LP-PHY: Some fixes, notices

2010-01-07 Thread Michael Buesch
On Thursday 07 January 2010 00:22:11 Rafał Miłecki wrote:
 W dniu 6 stycznia 2010 22:30 użytkownik Gábor Stefanik
 netrolller...@gmail.com napisał:
  On Wed, Jan 6, 2010 at 10:09 PM, Rafał Miłecki zaj...@gmail.com wrote:
  diff --git a/drivers/net/wireless/b43/phy_lp.c
  b/drivers/net/wireless/b43/phy_lp.c
  index 1e318d8..43a9fcc 100644
  --- a/drivers/net/wireless/b43/phy_lp.c
  +++ b/drivers/net/wireless/b43/phy_lp.c
  @@ -198,6 +198,17 @@ static void lpphy_table_init(struct b43_wldev *dev)
 
         lpphy_init_tx_gain_table(dev);
 
  +       if (dev-phy.rev =2) {
  +               int i;
  +               for (i = 0; i  128; ++i) {
 
  Two style comments:
  1. Declare i at the beginning of the function.
  2. The preferred style in b43 is i++.
 
 1. May I ask, why? I don't know much about kernel programming, so
 that's not obvious for me. I declared this at beginning of rev2+
 block, because it's used only here. I didn't want to waste memory of
 (rev  2) users (yeah, it's just 1 int but let's imagine 1 int in 100
 drivers...)

It does not waste memory, because the compiler will either:
a) sink it into the branch
or
b) allocate the stack space in any case on the beginning of
the func, because it's only one operation for allocating all stack 
space.
So the compiler simply won't care and do the right thing anyway.
Declaring variables inside of branches is hardly used in the kernel.
And besides that, variables on the stack don't really waste anything. The stack 
is
permanently allocated and the compiler is smart enough to manage it properly.
The only thing we need to take care of w.r.t. the stack is to now overflow it. 
But
that's only an issue if we allocate large arrays on the stack. It's hardly 
possible
to overflow it with ordinary variables.

 2. I read in many places ++i is faster than i++, but it wasn't about
 kernel programming actually. Thanks for tip.

There are some places in C++ (iterators) where ++i _can_ be faster. In C, 
however,
there is no speed difference. So we prefer i++, unless the semantics of ++i are 
required.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 1/5] b43: N-PHY: implement b43_nphy_stay_carrier_search and it's calls

2010-01-06 Thread Michael Buesch
On Wednesday 06 January 2010 16:40:32 Rafał Miłecki wrote:
 b43: N-PHY: implement b43_nphy_stay_carrier_search and it's calls

Hm, The phrase stay carrier earch doesn't make a lot of sense to me.
Is stray carrier search or something like that meant?
Not that I care much, but I'm just wondering if this is just a typo.

 +static void b43_nphy_write_clip_detection(struct b43_wldev *dev, u16 *vals)

We know that these values are the clip thresholds, so use a better variable 
name, please.

 +{
 + b43_phy_write(dev, B43_NPHY_C1_CLIP1THRES, vals[0]);
 + b43_phy_write(dev, B43_NPHY_C2_CLIP1THRES, vals[1]);
 +}
 +
 +static void b43_nphy_read_clip_detection(struct b43_wldev *dev, u16 *vals)
 +{
 + vals[0] = b43_phy_read(dev, B43_NPHY_C1_CLIP1THRES);
 + vals[1] = b43_phy_read(dev, B43_NPHY_C2_CLIP1THRES);
 +}
 +
 +static u16 b43_nphy_classifier(struct b43_wldev *dev, u16 mask, u16 val)
 +{
 + u16 tmp;
 + bool suspended = false;
 +
 + if (dev-dev-id.revision == 16  dev-mac_suspended == 0) {

Do not check for mac_suspended==0 here. b43_mac_suspended does this internally.

 + b43_mac_suspend(dev);
 + suspended = true;
 + }
 +
 + tmp = b43_phy_read(dev, B43_NPHY_CLASSCTL);
 + tmp = (B43_NPHY_CLASSCTL_CCKEN | B43_NPHY_CLASSCTL_OFDMEN |
 + B43_NPHY_CLASSCTL_WAITEDEN);
 + tmp = ~mask;
 + tmp |= (val  mask);
 + b43_phy_maskset(dev, B43_NPHY_CLASSCTL, 0xFFF8, tmp);
 +
 + if (suspended)
 + b43_mac_enable(dev);
 +
 + return tmp;
 +}
 +
 +static void b43_nphy_stay_carrier_search(struct b43_wldev *dev, bool enable)
 +{
 + struct b43_phy *phy = dev-phy;
 + struct b43_phy_n *nphy = phy-n;
 +
 + if (enable) {
 + u16 clip[] = { 0x, 0x };
 + if (nphy-deaf_count++ == 0) {
 + nphy-classifier_state = b43_nphy_classifier(dev, 0, 0);
 + b43_nphy_classifier(dev, 0x7, 0);
 + b43_nphy_read_clip_detection(dev, nphy-clip_state);
 + b43_nphy_write_clip_detection(dev, clip);
 + }
 + b43_nphy_reset_cca(dev);
 + } else {
 + if (--nphy-deaf_count != 0) {

If this test logic correct? The following would make more sense to me:

if (--nphy-deaf_count == 0) {

 + b43_nphy_classifier(dev, 0x7, nphy-classifier_state);
 + b43_nphy_write_clip_detection(dev, nphy-clip_state);
 + }
 + }
 +}
 +
   enum b43_nphy_rf_sequence {
   B43_RFSEQ_RX2TX,
   B43_RFSEQ_TX2RX,
 diff --git a/drivers/net/wireless/b43/phy_n.h 
 b/drivers/net/wireless/b43/phy_n.h
 index e5e402a..6ab07fc 100644
 --- a/drivers/net/wireless/b43/phy_n.h
 +++ b/drivers/net/wireless/b43/phy_n.h
 @@ -932,6 +932,9 @@ struct b43_phy_n {
   u32 deaf_count;
   bool mute;
 
 + u16 classifier_state;
 + u16 clip_state[2];
 +
   u8 iqcal_chanspec_2G;
   u8 rssical_chanspec_2G;
 



-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH 2/5] b43: N-PHY: b43_nphy_get_tx_gains

2010-01-06 Thread Michael Buesch
On Wednesday 06 January 2010 19:18:39 Rafał Miłecki wrote:
  Dude what is up with this e-mail data on your patches in the commit log?
 
 That is way of encoding non-ASCII chars in mail header. You can find
 info about this in RFC. That is =?UTF-8?B? prefix and base 64
 encoded text.
 
 That was generated with git (git format-patch -5) and it understood by
 git (git am ...).

Well, however, there is no reason to include this in the mail body (and thus
the GIT commit message).
As you said, these are mail _headers_.
Additionally, attaching the patch as mail attachment is _not_ required. We just
put the patch into the body.

You may read Documentation/SubmittingPatches and format your future
patches according to that.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: b43 kills my kernel

2010-01-05 Thread Michael Buesch
On Monday 04 January 2010 22:51:40 Oncaphillis wrote:
 Did any of the patches for a device without a sprom make it
 into the 2.6.33-rc2 ?

No we decided that the patches were not acceptable and need a rewrite towards
firmware loading mechanism.
Nobody's currently doing that, though.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: N-PHY init: current code vs. specs

2010-01-05 Thread Michael Buesch
On Monday 04 January 2010 23:34:19 Rafał Miłecki wrote:
 W dniu 4 stycznia 2010 21:36 użytkownik Rafał Miłecki
 zaj...@gmail.com napisał:
  Next there is a lot of code after b43_nphy_workarounds(dev); call
  that I can not recognize. Let's just take some lines for example:
  b43_nphy_reset_cca(dev);
 
 Actually specs still tell about resetting CCA, but that is done (in
 specs) without call to separated function (just part of init code):
 29. Read PHY Register 0x01 and save in val
 30. Write val | 0x4000 to PHY Register 0x1
 31. Write val  0xBFFF to PHY Register 0x1
 
 Should I strictly follow specs (put CCA reset directly in init code)
 or should I keep b43_nphy_reset_cca function and just modify if to
 match current specs?
 

Well, just do the thing that makes most sense.
In general we all agree that we should _not_ implement crap, just because
broadcom does so, if we can do better. So, in this case, if we can do a
subfunction call and that function does make sense, we should do so for the
sake of readability (I didn't look into detail, though.).
Same goes for algorithms and stuff. If we realize that we can do better, do 
_not_
implement Broadcrap and instead implement a better version.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [RFC][RFH] Small fixes to N-PHY init

2010-01-04 Thread Michael Buesch
On Monday 04 January 2010 01:10:32 Rafał Miłecki wrote:
 Please, see table on:
 http://bcm-v4.sipsolutions.net/ChipCommon

No I mean where does the specs say to _write_ to that register?

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [RFC][RFH] Small fixes to N-PHY init

2010-01-04 Thread Michael Buesch
On Monday 04 January 2010 11:23:36 Rafał Miłecki wrote:
 W dniu 4 stycznia 2010 11:03 użytkownik Michael Buesch m...@bu3sch.de 
 napisał:
  On Monday 04 January 2010 01:10:32 Rafał Miłecki wrote:
  Please, see table on:
  http://bcm-v4.sipsolutions.net/ChipCommon
 
  No I mean where does the specs say to _write_ to that register?
 
 Whoops, sorry. You can find this in:
 http://bcm-v4.sipsolutions.net/802.11/PHY/Init/N
 3. If PHY revision = 3, bit 0x1000 is set in the board flags, and
 this is a 2 GHz band
 a. Set bit 0x40 in the Chip Control register (0x28)


Ok, it's not entirely clear to me whether this is the CHIPCTL register in
the chipcommon core. But if it is, just do something like this:

chipco_set32(dev-dev-bus-chipco, SSB_CHIPCO_CHIPCTL, 0x40);

I think it's probably not necessary to introduce a function in 
driver_chipcommon.c
for this oneliner.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: b43: acking patches

2010-01-03 Thread Michael Buesch
On Sunday 03 January 2010 22:30:27 Rafał Miłecki wrote:
 Michael, you gave me some reviews of patches I was very glad to see. I
 posted some (hopefully) corrected versions but didn't get your
 response.
 
 Can we expect you ACKing patches as well?
 
 I post this question publicly as I may be not the only one person who
 would like to know this.

Well, no response means I'm OK with it (or that I don't care).
In the past I used to ack all patches, but we changed that and I only
reply if I see problems with patches.
I usually respond within less than a day, if there's something I disagree with.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [RFC][RFH] Small fixes to N-PHY init

2010-01-03 Thread Michael Buesch
On Sunday 03 January 2010 22:34:38 Rafał Miłecki wrote:
  Also I need some help with:
  //TODO: Set bit 0x40 in the Chip Control register (0x28)
  could someone let me know how can I write to chip control register? Is
  following correct?
  tmp = b43_read16(dev, 0x28);
  b43_write16(dev, 0x28, tmp | 0x40);
 
  I see there are read/write for PHY, radio and just b43 (like above). Did I
  use correct one?
 
 Chip Common is generally used/programmed in driver_chipcommon.c 
 friends. Probably I'll need to hack that part to perform Set bit 0x40
 in the Chip Control register (0x28).

Well, what does the specification say about this Chip Control register?
Is it on the PHY, MAC core or another SSB core like the ChipCommon?

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH] b43: N-PHY: clean table init, check PHY rev (V2)

2010-01-02 Thread Michael Buesch
On Saturday 02 January 2010 17:33:53 Rafał Miłecki wrote:
 --- a/drivers/net/wireless/b43/tables_nphy.h
 +++ b/drivers/net/wireless/b43/tables_nphy.h

 -extern const u8 b43_ntab_adjustpower0[];
 +static const u8 b43_ntab_adjustpower0[];

Are you serious? o.O
Come on... You can do better ;)

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: Fatal DMA error problem with netbook and BCM4312 - Test 3

2009-12-28 Thread Michael Buesch
On Monday 28 December 2009 05:49:14 Larry Finger wrote:
+  tmp = ~(SSB_IMCFGLO_SERTO | SSB_IMCFGLO_REQTO_SHIFT);

This does not make any sense.
Did you mean:
+   tmp = ~(SSB_IMCFGLO_SERTO | SSB_IMCFGLO_REQTO);


+  tmp |= 3;

So you set SER-timeout to 3 and REQ-timeout to 0. Is that what we want?
REQ=zero smells fishy to me, but if the broadcom code also does this, I'm OK 
with it.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: Fatal DMA error problem with netbook and BCM4312 - Test 3

2009-12-28 Thread Michael Buesch
On Monday 28 December 2009 20:09:05 Larry Finger wrote:
 I did get that wrong. The Broadcom code does the equivalent of
 
 tmp = tmp  ~0x77 | 3

Ok, so you need my version of the masking.

 which is what my code ended up doing by accident, but REQ is set to zero.

Yeah, OK to me. It's a workaround after all...

 Are these values discussed anywhere in the open? I have not found anything
 regarding the SSB configuration.

Well, there's no real docs on these, but I'm pretty sure they are related
to the posting of messages on the SSB bus. These values are timeouts for
the communication on the bus. So yeah, I think in _theory_ it can cause
DMA (and other) trouble.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: Problem with b43_get_and_map_ringmem()

2009-12-28 Thread Michael Buesch
As I have said several times. I am not interested in fixing this. Just revert 
the patch.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH] b43: N-PHY: clean table init, check PHY rev

2009-12-23 Thread Michael Buesch
On Wednesday 23 December 2009 11:34:39 Rafał Miłecki wrote:
 W dniu 22 grudnia 2009 02:40 użytkownik Rafał Miłecki
 zaj...@gmail.com napisał:
  It's just compilation-tested as I don't own N-PHY device yet (should receive
  one for Christmas). Not sure if we even support SSB used for N-PHY cards.
 
 Is posting on this ML enough to get patch in testing and mainline
 later? If it just needs some time, *I am completely fine with it*.
 
 I ask because maybe I'm waiting for something that won't happen due to
 my (some) mistake. Should I do anything else to get this patch
 reviewed  up?

Send it to John Linville and the linux wireless list. Also CCing this list is a 
good idea.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH][resend with linux-wireless] b43: N-PHY: clean table init, check PHY rev

2009-12-23 Thread Michael Buesch
On Wednesday 23 December 2009 19:19:19 Larry Finger wrote:
 ACK for the relocation of the tables.

Well, the _tables_ aren't actually relocated with that patch. The tables
are already in the tables_nphy.c file. You just relocate the functions that 
upload
the tables.
So if you want to relocate the upload-functions, the question arises whether
the tables should be made static.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: Fatal DMA error problem with netbook and BCM4312 - Test 2

2009-12-21 Thread Michael Buesch
On Monday 21 December 2009 22:31:10 Larry Finger wrote:
 Hi,
 
 I placed a number of test prints in my system trying to find where a
 DMA data error might occur. In doing so, I found that 3 slots in the
 DMA header cache cross a page boundary. Such a situation is allowed on
 my system, but it might be forbidden on Atom processors. Please try
 this really ugly hack to see if it makes any difference.

First thing is that the DMA buffers are allowed to cross any boundary (and
the header is just a buffer). The boundary requirements only apply to the
memory holding the descriptors.

Second thing is: How does the patch prevent a boundary crossing?

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: Fatal DMA error problem with netbook and BCM4312 - Test 2

2009-12-21 Thread Michael Buesch
On Monday 21 December 2009 23:02:39 Larry Finger wrote:
 On 12/21/2009 03:47 PM, Michael Buesch wrote:
  On Monday 21 December 2009 22:31:10 Larry Finger wrote:
  Hi,
 
  I placed a number of test prints in my system trying to find where a
  DMA data error might occur. In doing so, I found that 3 slots in the
  DMA header cache cross a page boundary. Such a situation is allowed on
  my system, but it might be forbidden on Atom processors. Please try
  this really ugly hack to see if it makes any difference.
  
  First thing is that the DMA buffers are allowed to cross any boundary (and
  the header is just a buffer). The boundary requirements only apply to the
  memory holding the descriptors.
 
 That is what we will test for the Atom. I know it doesn't matter for
 my CPU, but ...

I'm pretty sure the broadcom DMA engine is not _that_ braindead.
(It is pretty weird, but not _that_ weird).
It would effectively mean that we'd have to bouncebuffer _every_ TX packet.

  Second thing is: How does the patch prevent a boundary crossing?
 
 The number of slots is reduced to the point that the header cache fits
 in one page, just as the RX header cache does. As I said, this is
 really ugly. If this fixes the problem, then a more elegant fix will
 be needed.

But the allocation is not required to be aligned to the page start.
The first header might start at offset 4000 of a page, so you cross
the page border after the first few headers.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: Fatal DMA error problem with netbook and BCM4312 - Test 2

2009-12-21 Thread Michael Buesch
On Monday 21 December 2009 23:20:06 Larry Finger wrote:
 Here, it was slot 74 that crossed the page boundary. At 110 bytes per
 every 2 slots, that works out to 4070 bytes for 0 - 73. From that, I
 infer that the cache starts on a page boundary.

Yeah well. For you.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: PCI config register 0x40

2009-12-19 Thread Michael Buesch
On Saturday 19 December 2009 07:26:10 Larry Finger wrote:
 Michael,
 
 As you may recall, I dumped writes to the PCI config space for both
 b43 and the wl driver. I found that wl wrote to address 0x40. In
 looking around other drivers, I found the following fragment in ipw2100:
 
 /* We disable the RETRY_TIMEOUT register (0x41) to keep
  * PCI Tx retries from interfering with C3 CPU state */
 pci_read_config_dword(pci_dev, 0x40, val);
 if ((val  0xff00) != 0)
 pci_write_config_dword(pci_dev, 0x40, val 
  0x00ff);

Well, if 0x40 is used as RETRY_TIMEOUT in ipw, that doesn't
mean it's the same in b43. Is the 0x40 register standardized in any way?

And if you do a patch, don't put it into ssb. Put it into b43.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: PCI config register 0x40

2009-12-19 Thread Michael Buesch
On Saturday 19 December 2009 17:11:30 Larry Finger wrote:
 On 12/19/2009 04:11 AM, Michael Buesch wrote:
  
  And if you do a patch, don't put it into ssb. Put it into b43.
 
 One further question about putting the patch into b43. Apparently,
 register 0x40 is not preserved across a suspend/resume to disk. In all
 the drivers that use this code, 0x40 is modified in the .probe and
 .resume routines. As b43 does the resume indirectly through mac80211,
 an alternate location is needed. It looks to me that placing the code
 in b43_wireless_core_init() would satisfy both needs. Is that correct?

Yes. Also don't forget to test bus-bustype==SSB_BUSTYPE_PCI before
accessing bus-host_pci.


-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH] b43: add config option to force QoS off

2009-12-13 Thread Michael Buesch
On Sunday 13 December 2009 17:45:31 Albert Herranz wrote:
 The b43 driver includes a capability mechanism that open source firmwares
 (like OpenFWWF) can use to inform the driver about supported
 characteristics.
 The OpenFWWF firmware doesn't support yet QoS and reflects that via
 its capabilities word.
 QoS is enabled by default when b43 starts (unless modparam_qos is set to 0).
 If the capabilities word of the firmware loaded informs that QoS is not
 supported then QoS is disabled.
 
 Unfortunately, due to an unidentified bug, this automatic disabling of
 QoS doesn't work properly. It works, though, if QoS is disabled from the very
 beginning.

Well, something in mac80211 was changed that breaks this.
mac80211 currently seems to assume that the number of queues does not change
after ieee80211_register, which was not the case previously. This breaks
the QoS disable, because b43 modifies the nr of queues variable in the
ieee80211_hw structure.

 This patch adds a config option to allow disabling QoS from the beginning.
 This is the only way to workaround the described problem when building the
 b43 driver in-kernel, as in that case modparam_qos can't be changed.
 
 Signed-off-by: Albert Herranz albert_herr...@yahoo.es

Why are we currently adding all kind of workaround patches instead of fixing 
the bugs?
This bug's been there for months, so I don't see a reason to push out a 
workaround now.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [RFC/RFT] b43: Allow PIO mode to be selected at module load

2009-12-10 Thread Michael Buesch
On Thursday 10 December 2009 17:27:57 Larry Finger wrote:
 If a user encounters the Fatal DMA Problem with a BCM43XX device, and
 still wishes to use b43 as the driver, their only option is to rebuild
 the kernel with CONFIG_B43_FORCE_PIO. This patch removes this option and
 allows PIO mode to be selected with a load-time option for the module.
 
 Once the DMA problem with the BCM4312 devices is solved, this patch will
 likely be reverted.
 
 Signed-off-by: Larry Finger larry.fin...@lwfinger.net
 ---
 
 
 Index: wireless-testing/drivers/net/wireless/b43/Kconfig
 ===
 --- wireless-testing.orig/drivers/net/wireless/b43/Kconfig
 +++ wireless-testing/drivers/net/wireless/b43/Kconfig
 @@ -79,10 +79,11 @@ config B43_SDIO
 If unsure, say N.
  
  # Data transfers to the device via PIO
 -# This is only needed on PCMCIA and SDIO devices. All others can do DMA 
 properly.
 +# Technically, this is only needed on PCMCIA and SDIO devices; but some LP 
 PHYs
 +# cannot do DMA properly.
  config B43_PIO
   bool
 - depends on B43  (B43_SDIO || B43_PCMCIA || B43_FORCE_PIO)
 + depends on B43
   select SSB_BLOCKIO
   default y

Just remove the kconfig entry, too. ;)
Add the select SSB_BLOCKIO to CONFIG_B43

 @@ -137,12 +138,4 @@ config B43_DEBUG
 for production use.
 Only say Y, if you are debugging a problem in the b43 driver 
 sourcecode.
  
 -config B43_FORCE_PIO
 - bool Force usage of PIO instead of DMA
 - depends on B43  B43_DEBUG
 - ---help---
 -   This will disable DMA and always enable PIO instead.
  
 -   Say N!
 -   This is only for debugging the PIO engine code. You do
 -   _NOT_ want to enable this.
 Index: wireless-testing/drivers/net/wireless/b43/Makefile
 ===
 --- wireless-testing.orig/drivers/net/wireless/b43/Makefile
 +++ wireless-testing/drivers/net/wireless/b43/Makefile
 @@ -12,7 +12,7 @@ b43-y   += xmit.o
  b43-y+= lo.o
  b43-y+= wa.o
  b43-y+= dma.o
 -b43-$(CONFIG_B43_PIO)+= pio.o
 +b43-y+= pio.o
  b43-y+= rfkill.o
  b43-$(CONFIG_B43_LEDS)   += leds.o
  b43-$(CONFIG_B43_PCMCIA) += pcmcia.o
 Index: wireless-testing/drivers/net/wireless/b43/b43.h
 ===
 --- wireless-testing.orig/drivers/net/wireless/b43/b43.h
 +++ wireless-testing/drivers/net/wireless/b43/b43.h
 @@ -821,11 +821,9 @@ struct b43_wl {
   /* The device LEDs. */
   struct b43_leds leds;
  
 -#ifdef CONFIG_B43_PIO
   /* Kmalloc'ed scratch space for PIO TX/RX. Protected by wl-mutex. */
   u8 pio_scratchspace[110] __attribute__((__aligned__(8)));
   u8 pio_tailspace[4] __attribute__((__aligned__(8)));
 -#endif /* CONFIG_B43_PIO */
  };
  
  static inline struct b43_wl *hw_to_b43_wl(struct ieee80211_hw *hw)
 @@ -876,20 +874,9 @@ static inline void b43_write32(struct b4
  
  static inline bool b43_using_pio_transfers(struct b43_wldev *dev)
  {
 -#ifdef CONFIG_B43_PIO
   return dev-__using_pio_transfers;
 -#else
 - return 0;
 -#endif
  }
  
 -#ifdef CONFIG_B43_FORCE_PIO
 -# define B43_FORCE_PIO   1
 -#else
 -# define B43_FORCE_PIO   0
 -#endif
 -
 -
  /* Message printing */
  void b43info(struct b43_wl *wl, const char *fmt, ...)
  __attribute__ ((format(printf, 2, 3)));
 Index: wireless-testing/drivers/net/wireless/b43/main.c
 ===
 --- wireless-testing.orig/drivers/net/wireless/b43/main.c
 +++ wireless-testing/drivers/net/wireless/b43/main.c
 @@ -102,6 +102,9 @@ int b43_modparam_verbose = B43_VERBOSITY
  module_param_named(verbose, b43_modparam_verbose, int, 0644);
  MODULE_PARM_DESC(verbose, Log message verbosity: 0=error, 1=warn, 
 2=info(default), 3=debug);
  
 +static int modparam_pio;
 +module_param_named(pio, modparam_pio, int, 0444);
 +MODULE_PARM_DESC(pio, enable(1) / disable(0) PIO mode);
  
  static const struct ssb_device_id b43_ssb_tbl[] = {
   SSB_DEVICE(SSB_VENDOR_BROADCOM, SSB_DEV_80211, 5),
 @@ -1786,8 +1789,8 @@ static void b43_do_interrupt_thread(stru
  dma_reason[4], dma_reason[5]);
   b43err(dev-wl, This device does not support DMA 
  on your system. Please use PIO instead.\n);
 - b43err(dev-wl, CONFIG_B43_FORCE_PIO must be set in 
 -your kernel configuration.\n);
 + b43err(dev-wl, Unload the b43 module and reload 
 +with 'pio=1'\n);
   return;
   }
   if (merged_dma_reason  B43_DMAIRQ_NONFATALMASK) {
 @@ -4353,11 +4356,9 @@ static int 

Re: II. Does this help b43 DMA errors?

2009-11-28 Thread Michael Buesch
On Saturday 28 November 2009 18:12:20 Larry Finger wrote:
 Thanks for testing the other patch. To try to test the differences,
 I also dumped the PCI configuration writes and found one that wl does.
 
 Please see if this one helps.
 
 Larry
 
 
 
 
 Index: wireless-testing/drivers/ssb/driver_pcicore.c
 ===
 --- wireless-testing.orig/drivers/ssb/driver_pcicore.c
 +++ wireless-testing/drivers/ssb/driver_pcicore.c
 @@ -325,6 +325,8 @@ static void ssb_pcicore_fixup_pcibridge(
   ssb_printk(KERN_INFO PCI: Fixing latency timer of device %s to %u\n,
  pci_name(dev), lat);
   pci_write_config_byte(dev, PCI_LATENCY_TIMER, lat);
 +
 + pci_write_config_dword(dev, 0x40, 0x6030001);
  }
  DECLARE_PCI_FIXUP_EARLY(PCI_ANY_ID, PCI_ANY_ID, ssb_pcicore_fixup_pcibridge);

Uhm, this code is for embedded MIPS, only.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: II. Does this help b43 DMA errors?

2009-11-28 Thread Michael Buesch
On Saturday 28 November 2009 19:03:30 Larry Finger wrote:
 On my x86_64 system, I get the Fixing latency ... message printed out
 just before the code writes 0x40 to PCI-E configuration register 0x0d
 (PCI_LATENCY_TIMER). The wl driver does that same write on my machine.
 
 Do you have any knowledge as to why this number is increased from the
 default of 32?

The code should not even be compiled on that system. If the code is really
executed, that's a serious breakage.

The code is ifdefed unter CONFIG_SSB_PCICORE_HOSTMODE.
This config option must not be enabled on anything but MIPS.

I think, however, enabling CONFIG_SSB_PCICORE_HOSTMODE should lead to compile
failures on anything but MIPS.

(And the code writes 0xA8, not 0x40.)

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: III. Does this help b43 DMA errors?

2009-11-28 Thread Michael Buesch
On Sunday 29 November 2009 00:43:29 Larry Finger wrote:
 Sorry about the problem with the previous try. This one does write the
 PCI configuration the same as wl does.
 
 Please see if this one helps.

 Index: wireless-testing/drivers/ssb/pci.c
 ===
 --- wireless-testing.orig/drivers/ssb/pci.c
 +++ wireless-testing/drivers/ssb/pci.c
 @@ -567,6 +567,8 @@ static int sprom_extract(struct ssb_bus
const u16 *in, u16 size)
  {
   pci_write_config_dword(bus-host_pci, 0x40, 0x6030001);
 + printk(KERN_DEBUG ssb: Set PCI Configuration word 0x40 to 
 +   0x6030001\n);
 
   memset(out, 0, sizeof(*out));

/me scratches head
quilt refresh? ;)

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH RFC] ssb: Generic SPROM override for devices without SPROM

2009-11-24 Thread Michael Buesch
On Tuesday 24 November 2009 09:51:37 Oncaphillis wrote:
 On 11/20/2009 12:12 PM, Michael Buesch wrote:
  This patch adds a generic mechanism for overriding the SPROM mechanism
  on devices without SPROM hardware.
 
  There currently is a major problem with this:
  It tries to deduce a MAC address from various hardware parameters. But
  currently it will result in the same MAC address for machines of the same
  type. Does somebody have an idea of some device-instance specific serial
  number or something similar that could be hashed into the MAC?
 
 What version is this patch against ? I tries rc7,rc8 and 2.6.31.6.
 But it doesn't work for me.

wireless testing

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: DMA errors with BCM4312 - an update

2009-11-24 Thread Michael Buesch
On Tuesday 24 November 2009 22:15:52 Chris Vine wrote:
 On Tue, 24 Nov 2009 13:06:43 -0600
 Larry Finger larry.fin...@lwfinger.net wrote:
  This E-mail is to summarize what I have learned to date.
  
  The pm_qos change does nothing useful. It may have helped a little,
  but the side effects are far worse than the benefits.
  
  Most systems work better with b43 when warm booted after Broadcom's
  wl driver was loaded. The conclusion is that wl is making some change
  in the setup that b43 is not.
  
  (3) Based on the above, I have done MMIO and PCI-E configuration
  tracing for the two drivers and found some real differences. After
  seeing these, I did more RE work, and found some setup for the PCI-E
  core that was missed earlier. I am still working on the changes. What
  I have completed is found at
  
  http://bcm-v4.sipsolutions.net/PCI-E#PCI-E_Setup
  
  I doubt that most of these new routines will affect the problem
  interfaces as they apply only to PCI-E core revisions 7 and 8. My
  BCM4312 has rev 9. I do not know what versions are giving the
  trouble. With SSB_DEBUG enabled, it will be in a log line as follows:
  
  ssb: Core 3 found: PCI-E (cc 0x820, rev 0x09, vendor 0x4243)
  
  If you are seeing the DMA error, please supply the above info.
  
  The PCI-E Miscellaneous Configuration routine that is not yet
  finished does run on my system and is the source of the tracing
  differences. If the problem cards has a revision newer than 9, I will
  probably need an MMIO trace for your device.
 
 This is mine, the same revision as yours, but which demonstrates the
 DMA errors:
 
 ssb: Core 0 found: ChipCommon (cc 0x800, rev 0x16, vendor 0x4243)
 ssb: Core 1 found: IEEE 802.11 (cc 0x812, rev 0x0F, vendor 0x4243)
 ssb: Core 2 found: PCMCIA (cc 0x80D, rev 0x0A, vendor 0x4243)
 ssb: Core 3 found: PCI-E (cc 0x820, rev 0x09, vendor 0x4243)
 ssb: Found rev 1 PMU (capabilities 0x02A62F01)

Larry, do you also have the same PMU?

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH] b43: Rewrite DMA Tx status handling sanity checks

2009-11-23 Thread Michael Buesch
On Monday 23 November 2009 02:34:09 Larry Finger wrote:
 On 11/19/2009 03:24 PM, Michael Buesch wrote:
  This rewrites the error handling policies in the TX status handler.
  It tries to be error-tolerant as in try hard to not crash the machine.
  It won't recover from errors (that are bugs in the firmware or driver),
  because that's impossible. However, it will return a more or less useful
  error message and bail out. It also tries hard to use rate-limited messages
  to not flood the syslog in case of a failure.
  
  Signed-off-by: Michael Buesch m...@bu3sch.de
  
  ---
 
 Tested and ACKed by Larry Finger. Not only does this improve the error 
 handling
 for b43, but it also appears to fix the skb == NULL error that I experienced
 with the open-source firmware.

I don't think there's any way it can fix this. The patch doesn't change the
code behavior. It just changes the sanity checks, that under normal 
circumstances
should never trigger.

 John - please push this into wireless-testing. It should also go to 2.6.32, 
 but
 it is likely too large for the current stage. At least Cc it to stable.

Don't put it into stable. This is not a fix.
I don't think it's suitable for 2.6.32 at this stage, too.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH] b43: Rewrite DMA Tx status handling sanity checks

2009-11-23 Thread Michael Buesch
On Monday 23 November 2009 05:45:47 Larry Finger wrote:
 On 11/19/2009 03:24 PM, Michael Buesch wrote:
  This rewrites the error handling policies in the TX status handler.
  It tries to be error-tolerant as in try hard to not crash the machine.
  It won't recover from errors (that are bugs in the firmware or driver),
  because that's impossible. However, it will return a more or less useful
  error message and bail out. It also tries hard to use rate-limited messages
  to not flood the syslog in case of a failure.
 
 This patch definitely helped open-source firmware, but it is not a complete 
 fix.

It is no fix _at_ _all_.
The patch does not change a single line of code that wasn't either an assertion
or a machine crash before.
So it just transforms assertions into more verbose assertions and crashes into
assertions without a crash.

 debug: Out of order TX status report on DMA ring 1. Expected 114, but got 146

Ok, this is what I expected.

Let's see what's going on. Here's the ring. o is unused, * is used.

ooo***ooo
   ^   ^ ^
   114 146   newest
   oldest

So as you can see, the firmware reported a TX status for a frame right in the 
middle of
the ringbuffer. The new code detects this now before getting a double free 
and/or silent
memory corruption (freeing of used memory).

It really is illegal to report a TX status for a frame that's not the oldest 
one in the ring.
The firmware is required to process all frames in-order on one ring.

So how can this failure happen? I think there basically are three ways this can 
happen.

- First is that the ordering within one ring really gets messed up and it loses 
track
  of its ring pointers. I'm not sure if this is likely. Probably not.
- It messes up the ring membership. So it reports a TX status on the wrong ring.
  Note that the ring kernel pointer in the TX status report handler is derived
  from the cookie (and so also the number in the message Out of order TX status
  report on DMA ring 1 is derived from the cookie). So it's untrustworthy in 
case of
  broken firmware. The firmware has QoS-alike mechanisms, even if QoS is 
disabled. Maybe
  these mechanisms are broken.
- Third is the possibility of a driver bug. I rule that out as long as nobody is
  able to reproduce it with proprietary firmware.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH] b43: Rewrite DMA Tx status handling sanity checks

2009-11-23 Thread Michael Buesch
On Monday 23 November 2009 12:00:15 Francesco Gringoli wrote:
 Hi Michael,
 
 so you can observe this behavior at your site. Do you mind describing  
 the exact configuration? Maybe this time I can reproduce this  
 behavior, as I tried everything to make it happen. I also asked Larry  
 one of his boards and put it into several PCs but had no chance to  
 reproduce the crash. Could you please also report the neighboring  
 stations, the AP you are connected and so on.

I don't reproduce this, because I never really tried running the opensource 
firmware.
Larry reproduced it, this time.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


[PATCH] ssb: Fix SPROM writing

2009-11-23 Thread Michael Buesch
The SPROM writing routines were broken since we rewrote the suspend
handling on wireless devices, because SPROM writing depended on suspend.

This patch changes it and freezes devices with the driver remove(), probe()
callbacks instead. This also simplifies the whole logics a lot.

Signed-off-by: Michael Buesch m...@bu3sch.de

---

Tested on BCM4306


Index: wireless-testing/drivers/ssb/main.c
===
--- wireless-testing.orig/drivers/ssb/main.c2009-11-06 21:45:29.0 
+0100
+++ wireless-testing/drivers/ssb/main.c 2009-11-23 19:59:24.0 +0100
@@ -140,6 +140,19 @@ static void ssb_device_put(struct ssb_de
put_device(dev-dev);
 }
 
+static inline struct ssb_driver *ssb_driver_get(struct ssb_driver *drv)
+{
+   if (drv)
+   get_driver(drv-drv);
+   return drv;
+}
+
+static inline void ssb_driver_put(struct ssb_driver *drv)
+{
+   if (drv)
+   put_driver(drv-drv);
+}
+
 static int ssb_device_resume(struct device *dev)
 {
struct ssb_device *ssb_dev = dev_to_ssb_dev(dev);
@@ -210,90 +223,81 @@ int ssb_bus_suspend(struct ssb_bus *bus)
 EXPORT_SYMBOL(ssb_bus_suspend);
 
 #ifdef CONFIG_SSB_SPROM
-int ssb_devices_freeze(struct ssb_bus *bus)
+/** ssb_devices_freeze - Freeze all devices on the bus.
+ *
+ * After freezing no device driver will be handling a device
+ * on this bus anymore. ssb_devices_thaw() must be called after
+ * a successful freeze to reactivate the devices.
+ *
+ * @bus: The bus.
+ * @ctx: Context structure. Pass this to ssb_devices_thaw().
+ */
+int ssb_devices_freeze(struct ssb_bus *bus, struct ssb_freeze_context *ctx)
 {
-   struct ssb_device *dev;
-   struct ssb_driver *drv;
-   int err = 0;
-   int i;
-   pm_message_t state = PMSG_FREEZE;
+   struct ssb_device *sdev;
+   struct ssb_driver *sdrv;
+   unsigned int i;
+
+   memset(ctx, 0, sizeof(*ctx));
+   ctx-bus = bus;
+   SSB_WARN_ON(bus-nr_devices  ARRAY_SIZE(ctx-device_frozen));
 
-   /* First check that we are capable to freeze all devices. */
for (i = 0; i  bus-nr_devices; i++) {
-   dev = (bus-devices[i]);
-   if (!dev-dev ||
-   !dev-dev-driver ||
-   !device_is_registered(dev-dev))
-   continue;
-   drv = drv_to_ssb_drv(dev-dev-driver);
-   if (!drv)
+   sdev = ssb_device_get(bus-devices[i]);
+
+   if (!sdev-dev || !sdev-dev-driver ||
+   !device_is_registered(sdev-dev)) {
+   ssb_device_put(sdev);
continue;
-   if (!drv-suspend) {
-   /* Nope, can't suspend this one. */
-   return -EOPNOTSUPP;
}
-   }
-   /* Now suspend all devices */
-   for (i = 0; i  bus-nr_devices; i++) {
-   dev = (bus-devices[i]);
-   if (!dev-dev ||
-   !dev-dev-driver ||
-   !device_is_registered(dev-dev))
-   continue;
-   drv = drv_to_ssb_drv(dev-dev-driver);
-   if (!drv)
+   sdrv = ssb_driver_get(drv_to_ssb_drv(sdev-dev-driver));
+   if (!sdrv || SSB_WARN_ON(!sdrv-remove)) {
+   ssb_device_put(sdev);
continue;
-   err = drv-suspend(dev, state);
-   if (err) {
-   ssb_printk(KERN_ERR PFX Failed to freeze device %s\n,
-  dev_name(dev-dev));
-   goto err_unwind;
}
+   sdrv-remove(sdev);
+   ctx-device_frozen[i] = 1;
}
 
return 0;
-err_unwind:
-   for (i--; i = 0; i--) {
-   dev = (bus-devices[i]);
-   if (!dev-dev ||
-   !dev-dev-driver ||
-   !device_is_registered(dev-dev))
-   continue;
-   drv = drv_to_ssb_drv(dev-dev-driver);
-   if (!drv)
-   continue;
-   if (drv-resume)
-   drv-resume(dev);
-   }
-   return err;
 }
 
-int ssb_devices_thaw(struct ssb_bus *bus)
+/** ssb_devices_thaw - Unfreeze all devices on the bus.
+ *
+ * This will re-attach the device drivers and re-init the devices.
+ *
+ * @ctx: The context structure from ssb_devices_freeze()
+ */
+int ssb_devices_thaw(struct ssb_freeze_context *ctx)
 {
-   struct ssb_device *dev;
-   struct ssb_driver *drv;
-   int err;
-   int i;
+   struct ssb_bus *bus = ctx-bus;
+   struct ssb_device *sdev;
+   struct ssb_driver *sdrv;
+   unsigned int i;
+   int err, result = 0;
 
for (i = 0; i  bus-nr_devices; i++) {
-   dev = (bus-devices[i]);
-   if (!dev-dev ||
-   !dev-dev-driver

[PATCH] ssb: Fix range check in sprom write

2009-11-23 Thread Michael Buesch
The range check in the sprom image parser hex2sprom() is broken.
One sprom word is 4 hex characters.
This fixes the check and also adds much better sanity checks to the code.
We better make sure the image is OK by doing some sanity checks to avoid
bricking the device by accident.

Signed-off-by: Michael Buesch m...@bu3sch.de
Cc: sta...@kernel.org

---

Index: wireless-testing/drivers/ssb/sprom.c
===
--- wireless-testing.orig/drivers/ssb/sprom.c   2009-11-23 14:24:57.0 
+0100
+++ wireless-testing/drivers/ssb/sprom.c2009-11-23 20:43:04.0 
+0100
@@ -13,6 +13,8 @@
 
 #include ssb_private.h
 
+#include linux/ctype.h
+
 
 static const struct ssb_sprom *fallback_sprom;
 
@@ -33,17 +35,27 @@ static int sprom2hex(const u16 *sprom, c
 static int hex2sprom(u16 *sprom, const char *dump, size_t len,
 size_t sprom_size_words)
 {
-   char tmp[5] = { 0 };
-   int cnt = 0;
+   char c, tmp[5] = { 0 };
+   int err, cnt = 0;
unsigned long parsed;
 
-   if (len  sprom_size_words * 2)
+   /* Strip whitespace at the end. */
+   while (len) {
+   c = dump[len - 1];
+   if (!isspace(c)  c != '\0')
+   break;
+   len--;
+   }
+   /* Length must match exactly. */
+   if (len != sprom_size_words * 4)
return -EINVAL;
 
while (cnt  sprom_size_words) {
memcpy(tmp, dump, 4);
dump += 4;
-   parsed = simple_strtoul(tmp, NULL, 16);
+   err = strict_strtoul(tmp, 16, parsed);
+   if (err)
+   return err;
sprom[cnt++] = swab16((u16)parsed);
}
 

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [RFC] b43: Fix regression from Bug #14538

2009-11-23 Thread Michael Buesch
On Monday 23 November 2009 20:55:06 Larry Finger wrote:
 The routine b43_is_hw_radio_enabled() has long been a problem.
 For PPC architecture with PHY Revision  3, a read of the register
 B43_MMIO_HWENABLED_LO will cause a CPU fault unless b43_status()
 returns a value of 2 (B43_STAT_STARTED) (BUG 14181). Fixing that
 results in Bug 14538 in which the driver is unable to reassociate
 after resuming from hibernation because b43_status() returns 0.
 
 The correct fix would be to determine why the status is 0; however,
 I have not yet found why that happens. The correct value is found for
 my device, which has PHY revision = 3.
 
 Returning TRUE when the PHY revision  3 and b43_status() returns 0 fixes
 the regression for 2.6.32.
 
 Signed-off-by: Larry Finger larry.fin...@lwfinger.net
 Tested-by: Christian Casteyde casteyde.christ...@free.fr
 ---
 
 Index: wireless-testing/drivers/net/wireless/b43/rfkill.c
 ===
 --- wireless-testing.orig/drivers/net/wireless/b43/rfkill.c
 +++ wireless-testing/drivers/net/wireless/b43/rfkill.c
 @@ -33,9 +33,16 @@ bool b43_is_hw_radio_enabled(struct b43_
  B43_MMIO_RADIO_HWENABLED_HI_MASK))
   return 1;
   } else {
 - if (b43_status(dev) = B43_STAT_STARTED 
 - b43_read16(dev, B43_MMIO_RADIO_HWENABLED_LO)
 -  B43_MMIO_RADIO_HWENABLED_LO_MASK)
 + /* To prevent CPU fault on PPC, do not read a register
 +  * unless the interface is started; however, on resume
 +  * for hibernation, this routine is entered early. When
 +  * that happens, unconditionally return TRUE.
 +  */
 + if (b43_status(dev) = B43_STAT_STARTED) {
 + if (b43_read16(dev, B43_MMIO_RADIO_HWENABLED_LO)
 +  B43_MMIO_RADIO_HWENABLED_LO_MASK)
 + return 1;
 + } else
   return 1;
   }
   return 0;

looks OK as quick workaround.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH] b43: Rewrite DMA Tx status handling sanity checks

2009-11-22 Thread Michael Buesch
On Sunday 22 November 2009 19:11:52 Larry Finger wrote:
 On 11/22/2009 11:52 AM, Michael Buesch wrote:
  On Thursday 19 November 2009 22:24:29 Michael Buesch wrote:
  This rewrites the error handling policies in the TX status handler.
  It tries to be error-tolerant as in try hard to not crash the machine.
  It won't recover from errors (that are bugs in the firmware or driver),
  because that's impossible. However, it will return a more or less useful
  error message and bail out. It also tries hard to use rate-limited messages
  to not flood the syslog in case of a failure.
 
  Signed-off-by: Michael Buesch m...@bu3sch.de
  
  So did somebody try this with opensource firmware, yet?
 
 I'm testing now. So far, it has survived about 18 hours running tcpperf in one
 console, and a flood ping in another.

Cool. Thanks for testing. I'd have expected it to blow up, though. It's a 
little bit
strange, because there still are reports of blowing up opensource firmware. 
This patch
should produce better error messages in that case (it will not fix the blown 
firmware).

 It looks really good, but I want at least 
 24 hours before committing.

Well, no. Just commit it, please. If this breaks, the _firmware_ has to be 
fixed.
Not the patch.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: b43 kills my kernel

2009-11-20 Thread Michael Buesch
On Friday 20 November 2009 02:41:58 Oncaphillis wrote:
 On 11/20/2009 12:46 AM, Oncaphillis wrote:
  On 11/19/2009 06:44 PM, Michael Buesch wrote:
  On Thursday 19 November 2009 16:41:12 Michael Buesch wrote:
  Wait, that still can't work. I'll fix it soon...
 
  Ok, here's the updated version. Please test this:
  http://bu3sch.de/patches/wireless-testing/20091119-1842/patches/002-ssb-rewrite-sprom-fallback-mechanism.patch
 
 
  Heureka -- seems like I'm the first linux user on the planet with a
  WLAN connection on that device. The MAC address is random which
  of course is a pain for DHCP but it seems to work.

Thanks. I'll see what I can do about this.


 [ 6415.479127] b43-phy0 debug: Removing Interface type 2
 [ 6415.479601] b43-phy0 debug: Wireless interface stopped
 [ 6415.479615] b43-phy0 debug: DMA-64 rx_ring: Used slots 8/64, Failed 
 frames 0/0 = 0.0%, Average tries 0.00
 [ 6415.479710] b43-phy0 debug: DMA-64 tx_ring_AC_BK: Used slots 0/256, 
 Failed frames 0/0 = 0.0%, Average tries 0.0
 [ 6415.481511] b43-phy0 debug: DMA-64 tx_ring_AC_BE: Used slots 152/256, 
 Failed frames 2795/23615 = 11.8%, Averag tries 3.38
 [ 6415.483077] b43-phy0 debug: DMA-64 tx_ring_AC_VI: Used slots 0/256, 
 Failed frames 0/0 = 0.0%, Average tries 0.0
 [ 6415.485064] b43-phy0 debug: DMA-64 tx_ring_AC_VO: Used slots 12/256, 
 Failed frames 35/2939 = 1.1%, Average tris 1.05
 [ 6415.487069] b43-phy0 debug: DMA-64 tx_ring_mcast: Used slots 0/256, 
 Failed frames 0/0 = 0.0%, Average tries 0.0
 
 and loose the interface

Well, somebody shuts down the interfsce. There's nothing wrong with these logs.
I would point at network manager or something like that.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH] b43: Enforce DMA descriptor memory constraints

2009-11-20 Thread Michael Buesch
On Friday 20 November 2009 00:55:07 Larry Finger wrote:
 On 11/18/2009 11:21 PM, William Bourque wrote:
  Also, just saying, but it seems Larry's pm_qos_update_requirement 
  patch had some good effects; I can hardly get any connectivity without 
  it. With the patch, the wireless seems to be stable for a few minutes 
  before generating DMA errors... without the patch the error start piling 
  up as soon the interface get up.
 
 If the pm_qos patch has some positive benefits, I'll push it; however, I think
 this is just a band aid, not a real fix. It also has the bad side effect of
 keeping the CPU from going into deep sleep, which increases the power usage 
 with
 reduced battery life.

Yes, that's why it should at least only be set if DMA is used.
We could also make it depend on specific PCI IDs, but I'm not sure how big the 
list would grow.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: b43 kills my kernel

2009-11-20 Thread Michael Buesch
On Friday 20 November 2009 11:29:20 Oncaphillis wrote:
 Ok -- Some more details about my experience that it appears to be slow.


Note that there are several issues. First one being the sprom calibration
values being _wrong_ for your card. Second one is LP-PHY performance being 
crappy in
general for the current implementation.

Can somebody give me a genuine SPROM image for an LP-PHY card, please?
Just do this:
sudo cat $(find /sys/devices -name ssb_sprom)  ssb_sprom_copy

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


[PATCH RFC] ssb: Generic SPROM override for devices without SPROM

2009-11-20 Thread Michael Buesch
);
+   spin_unlock(override_list_lock);
 }
+EXPORT_SYMBOL(ssb_unregister_sprom_override);
 
-const struct ssb_sprom *ssb_get_fallback_sprom(void)
+int ssb_find_sprom_override(struct ssb_bus *bus, struct ssb_sprom *buf)
 {
-   return fallback_sprom;
+   struct ssb_sprom_override *ovr;
+   int err = -ENODEV;
+
+   spin_lock(override_list_lock);
+   list_for_each_entry(ovr, override_list, list) {
+   err = ovr-probe(bus, buf);
+   if (!err)
+   break;
+   }
+   spin_unlock(override_list_lock);
+
+   return err;
 }
Index: wireless-testing/drivers/ssb/ssb_private.h
===
--- wireless-testing.orig/drivers/ssb/ssb_private.h 2009-11-19 
18:34:42.0 +0100
+++ wireless-testing/drivers/ssb/ssb_private.h  2009-11-19 18:37:27.0 
+0100
@@ -171,7 +171,7 @@ ssize_t ssb_attr_sprom_store(struct ssb_
 const char *buf, size_t count,
 int (*sprom_check_crc)(const u16 *sprom, size_t 
size),
 int (*sprom_write)(struct ssb_bus *bus, const u16 
*sprom));
-extern const struct ssb_sprom *ssb_get_fallback_sprom(void);
+extern int ssb_find_sprom_override(struct ssb_bus *bus, struct ssb_sprom *buf);
 
 
 /* core.c */
Index: wireless-testing/include/linux/ssb/ssb.h
===
--- wireless-testing.orig/include/linux/ssb/ssb.h   2009-11-19 
18:34:42.0 +0100
+++ wireless-testing/include/linux/ssb/ssb.h2009-11-19 18:37:27.0 
+0100
@@ -394,9 +394,20 @@ extern int ssb_bus_sdiobus_register(stru
 
 extern void ssb_bus_unregister(struct ssb_bus *bus);
 
-/* Set a fallback SPROM.
- * See kdoc at the function definition for complete documentation. */
-extern int ssb_arch_set_fallback_sprom(const struct ssb_sprom *sprom);
+/** struct ssb_sprom_override - SPROM override handler
+ * @probe: Callback function used to probe for a SPROM override.
+ * Puts the override image into buf and returns 0.
+ * If there's no need to override the image, nonzero is returned.
+ * This callback runs in atomic context.
+ * @list: Used internally in ssb. Do not use in the device driver.
+ */
+struct ssb_sprom_override {
+   int (*probe)(struct ssb_bus *bus, struct ssb_sprom *buf);
+   struct list_head list;
+};
+
+extern void ssb_register_sprom_override(struct ssb_sprom_override *ovr);
+extern void ssb_unregister_sprom_override(struct ssb_sprom_override *ovr);
 
 /* Suspend a SSB bus.
  * Call this from the parent bus suspend routine. */
Index: wireless-testing/drivers/ssb/b43_pci_bridge.c
===
--- wireless-testing.orig/drivers/ssb/b43_pci_bridge.c  2009-11-19 
18:34:42.0 +0100
+++ wireless-testing/drivers/ssb/b43_pci_bridge.c   2009-11-20 
12:04:09.0 +0100
@@ -5,13 +5,15 @@
  * because of its small size we include it in the SSB core
  * instead of creating a standalone module.
  *
- * Copyright 2007  Michael Buesch m...@bu3sch.de
+ * Copyright 2007-2009  Michael Buesch m...@bu3sch.de
  *
  * Licensed under the GNU/GPL. See COPYING for details.
  */
 
 #include linux/pci.h
 #include linux/ssb/ssb.h
+#include linux/etherdevice.h
+#include linux/jhash.h
 
 #include ssb_private.h
 
@@ -36,6 +38,76 @@ static const struct pci_device_id b43_pc
 };
 MODULE_DEVICE_TABLE(pci, b43_pci_bridge_tbl);
 
+
+static void pcidev_deduce_mac_address(struct pci_dev *pdev,
+ struct ssb_sprom *sprom,
+ const char *oui)
+{
+   u32 hash = 0x63E72B6D;
+
+   hash = jhash(pdev-device, sizeof(pdev-device), hash);
+   hash = jhash(pdev-subsystem_device, sizeof(pdev-subsystem_device), 
hash);
+   hash = jhash(pdev-devfn, sizeof(pdev-devfn), hash);
+   //TODO: Need machine specific seed
+
+   sprom-il0mac[3] = hash;
+   sprom-il0mac[4] = hash  8;
+   sprom-il0mac[5] = hash  16;
+   memcpy(sprom-il0mac, oui, 3);
+   memcpy(sprom-et0mac, sprom-il0mac, ETH_ALEN);
+   memcpy(sprom-et1mac, sprom-il0mac, ETH_ALEN);
+}
+
+#define IS_PDEV(pdev, _vendor, _device, _subvendor, _subdevice)
( \
+   (pdev-vendor == PCI_VENDOR_ID_##_vendor) \
+   (pdev-device == _device) \
+   (pdev-subsystem_vendor == PCI_VENDOR_ID_##_subvendor)\
+   (pdev-subsystem_device == _subdevice)  )
+
+static int b43_sprom_override_probe(struct ssb_bus *bus,
+   struct ssb_sprom *sprom)
+{
+   struct pci_dev *pdev;
+
+   if (bus-bustype != SSB_BUSTYPE_PCI)
+   return -ENODEV;
+   pdev = bus-host_pci;
+
+   if (IS_PDEV(pdev, BROADCOM, 0x4315, FOXCONN, 0xE01B)) {
+   static const struct ssb_sprom image

Re: [PATCH RFC] ssb: Generic SPROM override for devices without SPROM

2009-11-20 Thread Michael Buesch
On Friday 20 November 2009 12:38:07 Florian Fainelli wrote:
 Why not call once random_ether_addr instead of using some kind of hash? Is it 
 because you
 want the same MAC from a reboot to another?

yes.

 Ok, so for BCM63xx I would no longer have to declare my SPROM, fine.

No you still need to do that. The sprom is device specific.
You need to call ssb_register_sprom_override() from the arch code with
that callback that sets up your sprom image.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: [PATCH] b43: Enforce DMA descriptor memory constraints

2009-11-19 Thread Michael Buesch
On Thursday 19 November 2009 06:21:45 William Bourque wrote:
 
 Michael Buesch wrote:
  Enforce all device constraints on the descriptor memory region.
  
  There are several constraints on the descriptor memory, as documented
  in the specification. The current code does not enforce them and/or
  incorrectly enforces them.
  
  Those constraints are:
  - The address limitations on 30/32bit engines, that also apply to
the skbs.
  - The 4k alignment requirement on 30/32bit engines.
  - The 8k alignment requirement on 64bit engines.
  
 
 No gain on my bcm4312 [14e4:4315] on HP Mini 1116R (Atom architecture). 
 The DMA errors are still present.

Well, this patch doesn't fix known breakages. It just _ensures_ that the device
constraints are met, so we can rule that out and can go on looking for other 
bugs.
Another thing is that it removes the extremely ugly always-do-GFP_DMA thing.

 Also, just saying, but it seems Larry's pm_qos_update_requirement 
 patch had some good effects; I can hardly get any connectivity without 
 it. With the patch, the wireless seems to be stable for a few minutes 
 before generating DMA errors... without the patch the error start piling 
 up as soon the interface get up.

I think we do need the PM_QOS stuff. It clearly fixes part of the problem.
I think we have multiple problems here.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: b43 kills my kernel

2009-11-19 Thread Michael Buesch
On Thursday 19 November 2009 12:07:30 Oncaphillis wrote:
 So I'm at a loss here, but if someone comes up with a bright
 idea to test or needs more informations I'm willing to test
 the resulting code on my machine.

Erm, no. Can you please answer the questions that you didn't answer, yet?
Especially the request for the original vendor driver.

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: b43 kills my kernel

2009-11-19 Thread Michael Buesch
Can you please try the following patch?
http://bu3sch.de/patches/wireless-testing/20091119-1349/patches/002-ssb-rewrite-sprom-fallback-mechanism.patch

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: b43 kills my kernel

2009-11-19 Thread Michael Buesch
On Thursday 19 November 2009 14:26:42 Oncaphillis wrote:
 On 11/19/2009 01:49 PM, Michael Buesch wrote:
  Can you please try the following patch?
  http://bu3sch.de/patches/wireless-testing/20091119-1349/patches/002-ssb-rewrite-sprom-fallback-mechanism.patch
 
 
 That seems to freeze my kernel. I tell you more in a couple of hours.

There was a small thinko.
Please test this one:
http://bu3sch.de/patches/wireless-testing/20091119-1626/patches/002-ssb-rewrite-sprom-fallback-mechanism.patch

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: b43 kills my kernel

2009-11-19 Thread Michael Buesch
On Thursday 19 November 2009 16:27:01 Michael Buesch wrote:
 On Thursday 19 November 2009 14:26:42 Oncaphillis wrote:
  On 11/19/2009 01:49 PM, Michael Buesch wrote:
   Can you please try the following patch?
   http://bu3sch.de/patches/wireless-testing/20091119-1349/patches/002-ssb-rewrite-sprom-fallback-mechanism.patch
  
  
  That seems to freeze my kernel. I tell you more in a couple of hours.
 
 There was a small thinko.
 Please test this one:
 http://bu3sch.de/patches/wireless-testing/20091119-1626/patches/002-ssb-rewrite-sprom-fallback-mechanism.patch
 

Wait, that still can't work. I'll fix it soon...

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


Re: b43 kills my kernel

2009-11-19 Thread Michael Buesch
On Thursday 19 November 2009 16:41:12 Michael Buesch wrote:
 Wait, that still can't work. I'll fix it soon...

Ok, here's the updated version. Please test this:
http://bu3sch.de/patches/wireless-testing/20091119-1842/patches/002-ssb-rewrite-sprom-fallback-mechanism.patch

-- 
Greetings, Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev


  1   2   3   4   5   6   7   8   9   10   >