Re: b44: regression in 2.6.22 (resend)

2007-05-30 Thread Michael Buesch
On Tuesday 29 May 2007 23:36:51 Gary Zambrano wrote:
> On Tue, 2007-05-29 at 18:39 -0400, Jeff Garzik wrote:
> 
> > We check for 0x because that is often how a fault is indicated, 
> > when the memory location is read during or immediately after hotplug (or 
> > if the PCI bus is truly faulty).  So for most hardware, you see
> > 
> > tmp = read(irq status)
> > if (!tmp)
> > return irq-none /* no irq events raised */
> > if (tmp == 0x)
> > return irq-none /* hot unplug or h/w fault */
> > 
> > and the method that determines no interrupt handling is needed.
> > 
> 
> I guess you are right, but then shouldn't the driver be checking for
> faults in other parts of the code too? What if a fault/hotplug occurs
> immediately after an interrupt, but before a tx?
> Thanks,

Well, in general it's not desired (or even possible) to check every
single MMIO access. General practice is to check on entering the IRQ handler
and on values from registers or DMA which could crash the driver.
For example on DMA we get some cookie back from the device in the TX
status report that says which packet this was associated to.
That value is used to look up a table.
In bcm43xx I initialize that to 0 in the driver, which is not a valid value.
Later I check for that. So if the device is unplugged before DMA was
on that value was complete it will recognize it and it won't crash.

In general we can only do our very best to prevent bad sideeffects from
a pull-in-full-operation. We can't do much here anyway. Best we can do
is to _try_ to prevent a crash. It might not always be 100% possible
to do so, though.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: bond_compute_features() does not handle devices with different CSUM

2007-05-30 Thread Michael Buesch
On Wednesday 30 May 2007 04:47:09 Laurent Chavey wrote:
> proposed change.
> 
> --- 2.6.20/drivers/net/bonding/bond_main.c  2007-05-29
> 19:43:39.010565000 -0700
> +++ 2.6.20.fix/drivers/net/bonding/bond_main.c  2007-05-29
> 19:46:12.37698 -0700
> @@ -1227,7 +1227,14 @@
> int i;
> 
> bond_for_each_slave(bond, slave, i) {
> -   features &= (slave->dev->features & BOND_INTERSECT_FEATURES);
> +   /* NETIF_F_HW_CSUM includes support for NET_IF_IP_CSUM
> +* as such when looking for the intersection we need to
> +* add it to the device supported features
> +*/
> +   unsigned long dev_features = slave->dev->features;
> +   if (slave->dev->features & NETIF_F_HW_CSUM)
> +   dev_features |= NETIF_F_IP_CSUM;


> +   features &= (features & BOND_INTERSECT_FEATURES);

Is this statement correct?
It's the same as
features = features & (features & BOND_INTERSECT_FEATURES);
which looks strange to me.
Did you mean something like
features |= (dev_features & BOND_INTERSECT_FEATURES);
?

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/5] NetXen: Multicast filter for NetXen driver

2007-06-01 Thread Michael Buesch
On Friday 01 June 2007 13:14:35 Mithlesh Thukral wrote:
> NetXen: Add multicast filter code
> This patch will add manage the multicast filter from driver.
> It will add capability to write multicast addresses to hardware.
> 
> Signed-by: Mithlesh Thukral <[EMAIL PROTECTED]>
> ---
> 
>  drivers/net/netxen/netxen_nic.h |   24 +
>  drivers/net/netxen/netxen_nic_hdr.h |3 
>  drivers/net/netxen/netxen_nic_hw.c  |  115 +-
>  3 files changed, 139 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/netxen/netxen_nic.h b/drivers/net/netxen/netxen_nic.h
> index ad6688e..c74402f 100644
> --- a/drivers/net/netxen/netxen_nic.h
> +++ b/drivers/net/netxen/netxen_nic.h
> @@ -261,6 +261,27 @@ #define netxen_set_msg_ctxid(config_word
>  #define netxen_set_msg_opcode(config_word, val)  \
>   ((config_word) &= ~(0xf<<28), (config_word) |= (val & 0xf) << 28)
>  
> +#define netxen_set_addr_ctl_id_pool0(config_word, val)   \
> + ((config_word) &= ~3, (config_word) |= val & 0x3)
> +#define netxen_set_addr_ctl_enable_xtnd_0(config_word)   \
> + ((config_word) |= 1 << 2)
> +#define netxen_set_addr_ctl_id_pool1(config_word, val)   \
> + ((config_word) &= ~(0x3<<4), (config_word) |= (val & 0x3) << 4)
> +#define netxen_set_addr_ctl_enable_xtnd_1(config_word)   \
> + ((config_word) |= 1 << 6)
> +#define netxen_set_addr_ctl_id_pool2(config_word, val)   \
> + ((config_word) &= ~(0x3<<8), (config_word) |= (val & 0x3) << 8)
> +#define netxen_set_addr_ctl_enable_xtnd_2(config_word)   \
> + ((config_word) |= 1 << 10)
> +#define netxen_set_addr_ctl_id_pool3(config_word, val)   \
> + ((config_word) &= ~(0x3<<12), (config_word) |= (val & 0x3) << 12)
> +#define netxen_set_addr_ctl_enable_xtnd_3(config_word)   \
> + ((config_word) |= 1 << 14)
> +#define netxen_set_addr_ctl_mode(config_word, val)   \
> + ((config_word) &= ~(0x3<<26), (config_word) |= (val & 0x3) << 26)
> +#define netxen_set_addr_ctl_enable_poll(config_word, val)\
> + ((config_word) &= ~(0xf<<30), (config_word) |= (val & 0xf) << 30)
> +
>  struct netxen_rcv_context {
>   __le64 rcv_ring_addr;
>   __le32 rcv_ring_size;
> @@ -883,6 +904,9 @@ struct netxen_adapter {
>   unsigned char mac_addr[ETH_ALEN];
>   int mtu;
>   int portnum;
> + u8 promisc;
> + u8 mc_enabled;
> + u8 max_mc_count;
>  
>   spinlock_t tx_lock;
>   spinlock_t lock;
> diff --git a/drivers/net/netxen/netxen_nic_hdr.h 
> b/drivers/net/netxen/netxen_nic_hdr.h
> index 608e37b..2bfecbc 100644
> --- a/drivers/net/netxen/netxen_nic_hdr.h
> +++ b/drivers/net/netxen/netxen_nic_hdr.h
> @@ -545,6 +545,9 @@ #define NETXEN_MULTICAST_ADDR_HI_1(NETX
>  #define NETXEN_MULTICAST_ADDR_HI_2   (NETXEN_CRB_NIU + 0x1018)
>  #define NETXEN_MULTICAST_ADDR_HI_3   (NETXEN_CRB_NIU + 0x101c)
>  
> +#define NETXEN_UNICAST_ADDR_BASE (NETXEN_CRB_NIU + 0x1080)
> +#define NETXEN_MULTICAST_ADDR_BASE   (NETXEN_CRB_NIU + 0x1100)
> +
>  #define  NETXEN_NIU_GB_MAC_CONFIG_0(I)   \
>   (NETXEN_CRB_NIU + 0x3 + (I)*0x1)
>  #define  NETXEN_NIU_GB_MAC_CONFIG_1(I)   \
> diff --git a/drivers/net/netxen/netxen_nic_hw.c 
> b/drivers/net/netxen/netxen_nic_hw.c
> index baff17a..fff3844 100644
> --- a/drivers/net/netxen/netxen_nic_hw.c
> +++ b/drivers/net/netxen/netxen_nic_hw.c
> @@ -303,6 +303,93 @@ int netxen_nic_set_mac(struct net_device
>   return 0;
>  }
>  
> +#define NETXEN_UNICAST_ADDR(port, index) \
> + (NETXEN_UNICAST_ADDR_BASE+(port*32)+(index*8))
> +
> +int netxen_nic_enable_mcast_filter(struct netxen_adapter *adapter)
> +{
> + u32 val = 0;
> + u16 port = physical_port[adapter->portnum];
> +
> + if (adapter->mc_enabled)
> + return 0;
> + 
> + netxen_set_addr_ctl_enable_poll(val, 0xf);
> +
> + if (adapter->ahw.board_type == NETXEN_NIC_XGBE)
> + netxen_set_addr_ctl_mode(val, 0x3);
> + else
> + netxen_set_addr_ctl_mode(val, 0x0);
> +
> + netxen_set_addr_ctl_id_pool0(val, 0x0);
> + netxen_set_addr_ctl_id_pool1(val, 0x1);
> + netxen_set_addr_ctl_id_pool2(val, 0x2);
> + netxen_set_addr_ctl_id_pool3(val, 0x3);
> +
> + netxen_set_addr_ctl_enable_xtnd_0(val);
> + netxen_set_addr_ctl_enable_xtnd_1(val);
> + netxen_set_addr_ctl_enable_xtnd_2(val);
> + netxen_set_addr_ctl_enable_xtnd_3(val);
> + 
> + netxen_crb_writelit_adapter(adapter, NETXEN_MAC_ADDR_CNTL_REG, val);
> + 
> + val = 0xff;
> +
> + netxen_crb_writelit_adapter(adapter, NETXEN_UNICAST_ADDR(port,0), val);
> + netxen_crb_writelit_adapter(adapter, NETXEN_UNICAST_ADDR(port,0)+4, 
> + val);
> + 
> + memcpy(&val, adapter->mac_addr, 3);
> + netxen_crb_writelit_adapter(adapter, NETXEN_UNICAST_ADDR(port,1), val);
> +
> + memcpy(&val, adapter->mac_addr+3, 3);
> + netxen_crb_writelit_adapter(adapter, NETXEN_UNICA

Re: [PATCH] V2: O_CLOEXEC for SCM_RIGHTS

2007-06-02 Thread Michael Buesch
On Saturday 02 June 2007 23:48:31 Ulrich Drepper wrote:
> Take two: I forgot to change the compat code.  This has now happened.  Only 
> one
> additional line changed.
> 
> Everything else from the first patch remains the same.  I try to avoid 
> clogging
> the list unnecessarily by not resending the test program.
> 
> 
> Signed-off-by: Ulrich Drepper <[EMAIL PROTECTED]>
> 
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -855,7 +855,7 @@
>  /*
>   * Find an empty file descriptor entry, and mark it busy.
>   */
> -static int get_unused_fd_flags(int flags)
> +int get_unused_fd_flags(int flags)
>  {
>   struct files_struct * files = current->files;
>   int fd, error;
> --- a/include/linux/file.h
> +++ b/include/linux/file.h
> @@ -73,6 +73,7 @@ extern struct file * FASTCALL(fget_light(unsigned int fd, 
> int *fput_needed));
>  extern void FASTCALL(set_close_on_exec(unsigned int fd, int flag));
>  extern void put_filp(struct file *);
>  extern int get_unused_fd(void);
> +extern int FASTCALL(get_unused_fd_flags(int flags));

If you use FASTCALL macro here, you must use fastcall macro at
the definition above, too.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] b44: timer power saving

2007-06-06 Thread Michael Buesch
On Monday 04 June 2007 22:25:37 Stephen Hemminger wrote:
> Make the PHY and statistic timer run on one second boundary
> for powersaving.
> 
> On resume, the driver should check for link up immediately, to
> get online faster (rather than waiting for the next second).
> 
> Signed-off-by: Stephen Hemminger <[EMAIL PROTECTED]>
> 
> 
> ---
>  drivers/net/b44.c |9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> --- a/drivers/net/b44.c   2007-06-04 12:31:27.0 -0700
> +++ b/drivers/net/b44.c   2007-06-04 12:31:34.0 -0700
> @@ -599,8 +599,7 @@ static void b44_timer(unsigned long __op
>  
>   spin_unlock_irq(&bp->lock);
>  
> - bp->timer.expires = jiffies + HZ;
> - add_timer(&bp->timer);
> + mod_timer(&bp->timer, round_jiffies(jiffies + HZ));
>  }
>  
>  static void b44_tx(struct b44 *bp)
> @@ -2348,11 +2347,11 @@ static int b44_resume(struct pci_dev *pd
>   netif_device_attach(bp->dev);
>   spin_unlock_irq(&bp->lock);
>  
> - bp->timer.expires = jiffies + HZ;
> - add_timer(&bp->timer);
> -
>   b44_enable_ints(bp);
>   netif_wake_queue(dev);
> +
> + mod_timer(&bp->timer, jiffies + 1);

I don't think we need +1, if you need to fire immediately
(on the next tick). The timer core will always fire
timers that are in the past immediately.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] b44: timer power saving

2007-06-07 Thread Michael Buesch
On Wednesday 06 June 2007 23:04:07 Stephen Hemminger wrote:
> 
> > I don't think we need +1, if you need to fire immediately
> > (on the next tick). The timer core will always fire
> > timers that are in the past immediately.
> 
> Thanks, but is it worth bothering to change it again?

No, I don't think so. I just wanted to point that out.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/4] NetXen: Add correct routines to setup multicast address

2007-06-10 Thread Michael Buesch
On Thursday 07 June 2007 13:34:37 Mithlesh Thukral wrote:
> NetXen: Add multi cast filter code
> This patch adds multi cast filter code to NetXen NIC driver.
> It also adds capabilities to setup the multicast address in hardware
> from the host side.

> +int netxen_nic_enable_mcast_filter(struct netxen_adapter *adapter)
> +{
> + u32 val = 0;

> + memcpy(&val, adapter->mac_addr, 3);

Broken on BigEndian architectures.

> + netxen_crb_writelit_adapter(adapter, NETXEN_UNICAST_ADDR(port,1), val);
> +
> + memcpy(&val, adapter->mac_addr+3, 3);

Broken on BigEndian architectures.

> + netxen_crb_writelit_adapter(adapter, NETXEN_UNICAST_ADDR(port,1) + 4,
> + val);
> +
> + adapter->mc_enabled = 1;
> + return 0;
> +}
> +
> +int netxen_nic_disable_mcast_filter(struct netxen_adapter *adapter)
> +{
> + u32 val =  0;
> + u16 port = physical_port[adapter->portnum];
> +
> + if(!adapter->mc_enabled)
> + return 0;
> +
> + netxen_crb_writelit_adapter(adapter, NETXEN_MAC_ADDR_CNTL_REG, val);
> +
> + memcpy(&val, adapter->mac_addr, 3);

Broken on BigEndian architectures.

> + netxen_crb_writelit_adapter(adapter, NETXEN_UNICAST_ADDR(port,0), val);
> +
> + memcpy(&val, adapter->mac_addr+3, 3);

Broken on BigEndian architectures.

> + netxen_crb_writelit_adapter(adapter, NETXEN_UNICAST_ADDR(port,0) + 4,
> + val);
> +
> + adapter->mc_enabled = 0;
> + return 0;
> +}
> +
> +#define NETXEN_MCAST_ADDR(port, index)   \
> + (NETXEN_MULTICAST_ADDR_BASE+(port*0x80)+(index*8))
> +
> +int netxen_nic_set_mcast_addr(struct netxen_adapter *adapter, int index, 
> + u8 *addr)
> +{
> + u32 hi = 0;
> + u32 lo = 0;
> + u16 port = physical_port[adapter->portnum];
> +
> + hi = (u32) addr[0] | 
> + ((u32) addr[1] << 8) |
> + ((u32) addr[2] << 16);
> + lo = (u32) addr[3] |
> + ((u32) addr[4] << 8) |
> + ((u32) addr[5] << 16);

That is the correct solution. Do that above, too.

> + netxen_crb_writelit_adapter(adapter, NETXEN_MCAST_ADDR(port,index), hi);
> + netxen_crb_writelit_adapter(adapter, NETXEN_MCAST_ADDR(port,index) + 4,
> + hi);
> + return 0;
> +}

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/4] NetXen: Add correct routines to setup multicast address

2007-06-10 Thread Michael Buesch
On Sunday 10 June 2007 11:54:11 Michael Buesch wrote:
> > +int netxen_nic_set_mcast_addr(struct netxen_adapter *adapter, int index, 
> > +   u8 *addr)
> > +{
> > +   u32 hi = 0;
> > +   u32 lo = 0;
> > +   u16 port = physical_port[adapter->portnum];
> > +
> > +   hi = (u32) addr[0] | 
> > +   ((u32) addr[1] << 8) |
> > +   ((u32) addr[2] << 16);
> > +   lo = (u32) addr[3] |
> > +   ((u32) addr[4] << 8) |
> > +   ((u32) addr[5] << 16);
> 
> That is the correct solution. Do that above, too.
> 
> > +   netxen_crb_writelit_adapter(adapter, NETXEN_MCAST_ADDR(port,index), hi);
> > +   netxen_crb_writelit_adapter(adapter, NETXEN_MCAST_ADDR(port,index) + 4,
> > +   hi);
> > +   return 0;
> > +}

Well, actually no. It is broken, too. You write hi twice. And I would
swap the variable names, as "low" is addr[0] in my understanding.



-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Xen-devel] [PATCH 0/4] [Net] Support accelerated network plugin modules

2007-06-15 Thread Michael Buesch
On Friday 15 June 2007 13:26:03 Keir Fraser wrote:
> On 15/6/07 11:46, "Kieran Mansley" <[EMAIL PROTECTED]> wrote:
> 
> > This is a repost of some earlier patches to the xen-devel mailing list,
> > with a number of changes thanks to some useful suggestions from others.
> 
> The coding style needs fixing in various ways.
> 
> Hard tabs need to be used, no spaces inside brackets, but should include
> spaces between if/while/for and bracket, and bracket and brace:
> if (foo) {
> Not
> if( foo ){
> if(foo ) {
> Or various other possibilities.
> 
> No use of the following please:
> If (foo) return 1; else return 0;
> Is clearer as:
> Return foo;

But it's not the same.
return !!foo;
would be the same. And yes, it does matter:

int x(void)
{
unsigned long long v = 0xFF00ULL;
/*foo*/
return v;
}

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Xen-devel] [PATCH 0/4] [Net] Support accelerated network plugin modules

2007-06-15 Thread Michael Buesch
On Friday 15 June 2007 14:20:34 Keir Fraser wrote:
> 
> On 15/6/07 13:11, "Michael Buesch" <[EMAIL PROTECTED]> wrote:
> 
> >> No use of the following please:
> >> If (foo) return 1; else return 0;
> >> Is clearer as:
> >> Return foo;
> > 
> > But it's not the same.
> > return !!foo;
> > would be the same. And yes, it does matter:
> 
> True in general, but not the cases I've seen in this patchset, where 'foo'
> is a predicate.

Ok, if foo is a variable containing an error code, it's
better to return that error code. I assumed that foo is a
variable containing some value (counter or something).

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: b44: high ping times with wireless-dev

2007-06-16 Thread Michael Buesch
On Sunday 17 June 2007 02:42:18 Maximilian Engelhardt wrote:
> I did build a kernel without the three mentioned above but the problem is 
> still the same. I also did remove everything but eth0 on interrupt 10 so the 
> only device using that interrupt is eth0 and then the card completely stopped 
> working.

_That_ is interesting...
Maybe the IRQ isn't correctly wired up on the backplane and
it doesn't generate IRQs at all (so it only worked, because other devices
on the same IRQ line triggered the line).
I'll take a look at that code.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: b44: high ping times with wireless-dev

2007-06-17 Thread Michael Buesch
On Saturday 16 June 2007 23:27:43 Maximilian Engelhardt wrote:
> [...]
> ACPI: PCI Interrupt Link [LNKD] enabled at IRQ 10
> ACPI: PCI Interrupt :02:02.0[A] -> Link [LNKD] -> GSI 10 (level, low) -> 
> IRQ 10
> ssb: Sonics Silicon Backplane found on PCI device :02:02.0
> b44.c:v2.0
> eth0: Broadcom 44xx/47xx 10/100BaseT Ethernet 00:c0:9f:29:99:a7
> [...]

Ok, I prepared two debugging patches.

Please enable SonicsSiliconBackplane Debugging in the kernel kconfig,
so I can get more detail information about your card.
Device Drivers/Sonics Silicon Backplane/SSB debugging
(Must disable "No SSB kernel messages")

Please apply and test the attached debugging patches in a row.
So apply patch 1 and test if it works again. If not, apply
patch 2 and test if it works.
Always save complete dmesg log on each test run and send it to me.

Thanks for testing.
(This time it seems we are actually getting somewhere, when
dealing with sane people. :D )

-- 
Greetings Michael.
Index: bu3sch-wireless-dev/drivers/ssb/driver_pcicore.c
===
--- bu3sch-wireless-dev.orig/drivers/ssb/driver_pcicore.c	2007-04-07 17:19:03.0 +0200
+++ bu3sch-wireless-dev/drivers/ssb/driver_pcicore.c	2007-06-17 12:51:37.0 +0200
@@ -477,6 +477,7 @@ int ssb_pcicore_dev_irqvecs_enable(struc
 		goto out;
 	bus = pdev->bus;
 
+printk("Enabling IRQ vectors\n");
 	/* Enable interrupts for this device. */
 	if (bus->host_pci &&
 	((pdev->id.revision >= 6) || (pdev->id.coreid == SSB_DEV_PCIE))) {
@@ -497,8 +498,10 @@ int ssb_pcicore_dev_irqvecs_enable(struc
 
 		intvec = ssb_read32(pdev, SSB_INTVEC);
 		tmp = ssb_read32(dev, SSB_TPSFLAG);
+printk("Writing INTVEC. TPSFLAG is 0x%08X\n", tmp);
 		tmp &= SSB_TPSFLAG_BPFLAG;
-		intvec |= tmp;
+//		intvec |= tmp;
+intvec |= 0x0002;
 		ssb_write32(pdev, SSB_INTVEC, intvec);
 	}
 
Index: bu3sch-wireless-dev/drivers/ssb/driver_pcicore.c
===
--- bu3sch-wireless-dev.orig/drivers/ssb/driver_pcicore.c	2007-06-17 12:50:32.0 +0200
+++ bu3sch-wireless-dev/drivers/ssb/driver_pcicore.c	2007-06-17 12:51:32.0 +0200
@@ -514,6 +514,8 @@ intvec |= 0x0002;
 		tmp |= SSB_PCICORE_SBTOPCI_BURST;
 		pcicore_write32(pc, SSB_PCICORE_SBTOPCI2, tmp);
 
+printk("Wrote translation\n");
+#if 0
 		if (pdev->id.revision < 5) {
 			tmp = ssb_read32(pdev, SSB_IMCFGLO);
 			tmp &= ~SSB_IMCFGLO_SERTO;
@@ -527,6 +529,7 @@ intvec |= 0x0002;
 			tmp |= SSB_PCICORE_SBTOPCI_MRM;
 			pcicore_write32(pc, SSB_PCICORE_SBTOPCI2, tmp);
 		}
+#endif
 	} else {
 		assert(pdev->id.coreid == SSB_DEV_PCIE);
 		//TODO: Better make defines for all these magic PCIE values.


Re: b44: high ping times with wireless-dev

2007-06-17 Thread Michael Buesch
On Sunday 17 June 2007 12:55:39 Michael Buesch wrote:
> On Saturday 16 June 2007 23:27:43 Maximilian Engelhardt wrote:
> > [...]
> > ACPI: PCI Interrupt Link [LNKD] enabled at IRQ 10
> > ACPI: PCI Interrupt :02:02.0[A] -> Link [LNKD] -> GSI 10 (level, low) 
> > -> 
> > IRQ 10
> > ssb: Sonics Silicon Backplane found on PCI device :02:02.0
> > b44.c:v2.0
> > eth0: Broadcom 44xx/47xx 10/100BaseT Ethernet 00:c0:9f:29:99:a7
> > [...]
> 
> Ok, I prepared two debugging patches.
> 
> Please enable SonicsSiliconBackplane Debugging in the kernel kconfig,
> so I can get more detail information about your card.
> Device Drivers/Sonics Silicon Backplane/SSB debugging
> (Must disable "No SSB kernel messages")
> 
> Please apply and test the attached debugging patches in a row.
> So apply patch 1 and test if it works again. If not, apply
> patch 2 and test if it works.
> Always save complete dmesg log on each test run and send it to me.
> 
> Thanks for testing.
> (This time it seems we are actually getting somewhere, when
> dealing with sane people. :D )
> 

Ah, forgot to say. Apply patch 2 on top of patch 1.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Fix stupid wol variable overflow bug

2007-06-17 Thread Michael Buesch
This bug was introduced by me.
val is 16bit, but the read value is 32bit.
Seems like I was smoking crack when porting that part of the driver.
I guess that's the last stupid bug in these 4 lines of code.

Signed-off-by: Michael Buesch <[EMAIL PROTECTED]>

Index: bu3sch-wireless-dev/drivers/net/b44.c
===
--- bu3sch-wireless-dev.orig/drivers/net/b44.c  2007-06-16 19:56:41.0 
+0200
+++ bu3sch-wireless-dev/drivers/net/b44.c   2007-06-17 13:35:23.0 
+0200
@@ -1574,8 +1574,7 @@ static void b44_setup_wol_pci(struct b44
u16 val;
 
if (bp->sdev->bus->bustype != SSB_BUSTYPE_SSB) {
-   val = br32(bp, SSB_TMSLOW);
-   bw32(bp, SSB_TMSLOW, val | SSB_TMSLOW_PE);
+   bw32(bp, SSB_TMSLOW, br32(bp, SSB_TMSLOW) | SSB_TMSLOW_PE);
pci_read_config_word(bp->sdev->bus->host_pci, SSB_PMCSR, &val);
pci_write_config_word(bp->sdev->bus->host_pci, SSB_PMCSR, val | 
SSB_PE);
}

Please apply this to wireless-dev.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: b44: high ping times with wireless-dev

2007-06-17 Thread Michael Buesch
On Sunday 17 June 2007 14:03:30 Maximilian Engelhardt wrote:
> On Sunday 17 June 2007, Michael Buesch wrote:
> > On Saturday 16 June 2007 23:27:43 Maximilian Engelhardt wrote:
> > > [...]
> > > ACPI: PCI Interrupt Link [LNKD] enabled at IRQ 10
> > > ACPI: PCI Interrupt :02:02.0[A] -> Link [LNKD] -> GSI 10 (level, low)
> > > -> IRQ 10
> > > ssb: Sonics Silicon Backplane found on PCI device :02:02.0
> > > b44.c:v2.0
> > > eth0: Broadcom 44xx/47xx 10/100BaseT Ethernet 00:c0:9f:29:99:a7
> > > [...]
> >
> > Ok, I prepared two debugging patches.
> >
> > Please enable SonicsSiliconBackplane Debugging in the kernel kconfig,
> > so I can get more detail information about your card.
> > Device Drivers/Sonics Silicon Backplane/SSB debugging
> > (Must disable "No SSB kernel messages")
> >
> > Please apply and test the attached debugging patches in a row.
> > So apply patch 1 and test if it works again. If not, apply
> > patch 2 and test if it works.
> > Always save complete dmesg log on each test run and send it to me.
> >
> > Thanks for testing.
> > (This time it seems we are actually getting somewhere, when
> > dealing with sane people. :D )
> 
> I did the tests with my kernel where only the card is on interrupt 10. dmesg 
> is attached.
> With the first patch applied networking does work again. I also additionally 
> tried patch2 and it also does work.

Great!
To me this seems to be a silicon bug. The IRQ routing value, which is read
from the chip here, is hardcoded in the old b44 driver.
I'll implement a workaround for this and submit a patch.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] b44-ssb: Fix IRQ routing bits on the backplane

2007-06-17 Thread Michael Buesch
The backplane on the b44 chip seems to have a silicon bug
in the IRQ routing. The ethernet core IRQ routing does not work
with the routing bit returned from the backplaneflag register.
This patch adds a workaround for the b44 chip to use a hardcoded
constant. This constant was also used in the old b44 driver.

Signed-off-by: Michael Buesch <[EMAIL PROTECTED]>
Cc: Maximilian Engelhardt <[EMAIL PROTECTED]>
Cc: Gary Zambrano <[EMAIL PROTECTED]>

Index: bu3sch-wireless-dev/drivers/ssb/driver_pcicore.c
===
--- bu3sch-wireless-dev.orig/drivers/ssb/driver_pcicore.c   2007-06-17 
13:35:45.0 +0200
+++ bu3sch-wireless-dev/drivers/ssb/driver_pcicore.c2007-06-17 
14:19:05.0 +0200
@@ -496,9 +496,15 @@ int ssb_pcicore_dev_irqvecs_enable(struc
u32 intvec;
 
intvec = ssb_read32(pdev, SSB_INTVEC);
-   tmp = ssb_read32(dev, SSB_TPSFLAG);
-   tmp &= SSB_TPSFLAG_BPFLAG;
-   intvec |= tmp;
+   if ((bus->chip_id & 0xFF00) == 0x4400) {
+   /* Workaround: On the BCM44XX the BPFLAG routing
+* bit is wrong. Use a hardcoded constant. */
+   intvec |= 0x0002;
+   } else {
+   tmp = ssb_read32(dev, SSB_TPSFLAG);
+   tmp &= SSB_TPSFLAG_BPFLAG;
+   intvec |= tmp;
+   }
ssb_write32(pdev, SSB_INTVEC, intvec);
}

 
Please apply this to wireless-dev.
This is also most likely the bug Uwe Bugla was
bullshitting about^W^Wreporting.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] NetXen: Fix the rmmod error on PBlades due incorrect cleanup

2007-06-22 Thread Michael Buesch
On Friday 22 June 2007 10:42:38 Mithlesh Thukral wrote:
> diff --git a/drivers/net/netxen/netxen_nic_hw.c 
> b/drivers/net/netxen/netxen_nic_hw.c
> index 4e958c9..f0df6fb 100644
> --- a/drivers/net/netxen/netxen_nic_hw.c
> +++ b/drivers/net/netxen/netxen_nic_hw.c
> @@ -378,6 +378,7 @@ int netxen_nic_hw_resources(struct netxe
>  crb_rcvpeg_state));
>   while (state != PHAN_PEG_RCV_INITIALIZED && loops < 20) {
>   udelay(100);
> + schedule();
>   /* Window 1 call */
>   state = readl(NETXEN_CRB_NORMALIZE(adapter,
>  recv_crb_registers

Better do msleep(1); instead of udelay+schedule.

> @@ -700,7 +701,7 @@ void netxen_nic_pci_change_crbwindow(str
>   adapter->curr_window = 0;
>  }
>  
> -void netxen_load_firmware(struct netxen_adapter *adapter)
> +int netxen_load_firmware(struct netxen_adapter *adapter)
>  {
>   int i;
>   u32 data, size = 0;
> @@ -712,15 +713,25 @@ void netxen_load_firmware(struct netxen_
>   writel(1, NETXEN_CRB_NORMALIZE(adapter, NETXEN_ROMUSB_GLB_CAS_RST));
>  
>   for (i = 0; i < size; i++) {
> - if (netxen_rom_fast_read(adapter, flashaddr, (int *)&data) != 
> 0) {
> - DPRINTK(ERR,
> - "Error in netxen_rom_fast_read(). Will skip"
> - "loading flash image\n");
> - return;
> + while (netxen_rom_fast_read(adapter, flashaddr, (int *)&data) 
> != 0) {
> + long timeout = 2 * HZ;
> + while (timeout) {
> + if (signal_pending(current)) {
> + printk( "%s: Got a signal, exiting\n", 
> __FUNCTION__ );
> + return -1;
> + }
> + set_current_state(TASK_INTERRUPTIBLE);
> + timeout = schedule_timeout(timeout);
> + }

You're opencoding msleep_interruptible() here?
And this sleeps two seconds between each rom-read attempt. Is that
really your intention?
I'd say better attempt to read more often and sleep less each time.

>   off = netxen_nic_pci_set_window(adapter, memaddr);
>   addr = pci_base_offset(adapter, off);
>   writel(data, addr);
> + while (readl(addr) != data) {
> + mdelay(100);
> + writel(data, addr);
> + }

Add a timeout. Else this will result in a system hang, if the hardware is 
faulty.

> diff --git a/drivers/net/netxen/netxen_nic_init.c 
> b/drivers/net/netxen/netxen_nic_init.c
> index 15f6dc5..8f5f4f8 100644
> --- a/drivers/net/netxen/netxen_nic_init.c
> +++ b/drivers/net/netxen/netxen_nic_init.c
> @@ -408,8 +408,12 @@ static inline int
>  do_rom_fast_read(struct netxen_adapter *adapter, int addr, int *valp)
>  {
>   if (jiffies > (last_schedule_time + (8 * HZ))) {
> - last_schedule_time = jiffies;
> - schedule();
> + if (last_schedule_time) {
> + last_schedule_time = jiffies;
> + schedule();
> + } else {
> + last_schedule_time = jiffies;
> + }

Why this strange thing?
I'd simply call cond_resched() instead of all this custom
schedule timekeeping. That's best for system latency.

> -void netxen_phantom_init(struct netxen_adapter *adapter, int pegtune_val)
> +int netxen_phantom_init(struct netxen_adapter *adapter, int pegtune_val)
>  {
>   u32 val = 0;
> - int loops = 0;
>  
>   if (!pegtune_val) {
> - val = readl(NETXEN_CRB_NORMALIZE(adapter, CRB_CMDPEG_STATE));
> - while (val != PHAN_INITIALIZE_COMPLETE && 
> - val != PHAN_INITIALIZE_ACK && loops < 20) {
> - udelay(100);
> - schedule();
> - val =
> - readl(NETXEN_CRB_NORMALIZE
> + do {
> + long timeout = 10 * HZ;
> + while (timeout) {
> + if (signal_pending(current)) {
> + printk(KERN_INFO"%s: Got a signal, 
> exiting\n", __FUNCTION__ );
> + printk(KERN_INFO"%s: val=0x%x, 
> pegtune_val=0x%x\n", __FUNCTION__,
> + val, pegtune_val );
> + return -1;
> + }
> + set_current_state(TASK_INTERRUPTIBLE);
> + timeout = schedule_timeout(timeout);
> + }
> + val = readl(NETXEN_CRB_NORMALIZE
> (adapter, CRB_CMDPEG_STATE));

msleep_interruptible()?

> @@ -1278,11 +1

Re: [PATCH 1/3] NetXen: Fix MSI issues by using PCI function 0

2007-06-22 Thread Michael Buesch
On Friday 22 June 2007 10:40:41 Mithlesh Thukral wrote:
> NetXen: Fix issue of MSI not working correctly
> NetXen driver uses PCI function 0 to provide the functionality of MSI.
> The patch makes driver check the bus master bit for function 0 and
> enable it after the card initialization.
> 
> Signed-off-by: Milan Bag <[EMAIL PROTECTED]>
> Signed-off-by: Wen Xiong <[EMAIL PROTECTED]>
> Signed-off-by: Mithlesh Thukral <[EMAIL PROTECTED]>
> 
> ---
> 
>  drivers/net/netxen/netxen_nic_main.c |   13 ++---
>  1 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/netxen/netxen_nic_main.c 
> b/drivers/net/netxen/netxen_nic_main.c
> index 6167b58..e68356b 100644
> --- a/drivers/net/netxen/netxen_nic_main.c
> +++ b/drivers/net/netxen/netxen_nic_main.c
> @@ -355,13 +355,6 @@ #endif
>   /* initialize the adapter */
>   netxen_initialize_adapter_hw(adapter);
>  
> -#ifdef CONFIG_PPC
> - if ((adapter->ahw.boardcfg.board_type ==
> - NETXEN_BRDTYPE_P2_SB31_10G_IMEZ) &&
> - (pci_func_id == 2))
> - goto err_out_free_adapter;
> -#endif /* CONFIG_PPC */
> -
>   /*
>*  Adapter in our case is quad port so initialize it before
>*  initializing the ports
> @@ -509,6 +502,12 @@ #endif
>   NETXEN_CAM_RAM(0x1fc)));
>   if (val == 0x) {
>   /* This is the first boot after power up */
> + netxen_nic_read_w0(adapter, NETXEN_PCIE_REG(0x4), &val);
> + if (!(val & 0x4)) {
> + val |= 0x4;
> + netxen_nic_write_w0(adapter, NETXEN_PCIE_REG(0x4), val);
> + mdelay(100);
> + }

msleep()?
Or wait, what is this delay trying to do? Commit the register access?
The better way to commit a register write is to read-back the value, usually.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFD] First draft of RDNSS-in-RA support for IPv6 DNS autoconfiguration

2007-06-22 Thread Michael Buesch
On Saturday 23 June 2007 01:26:19 C. Scott Ananian wrote:
> Attached is my first draft of a patch to implement RDNSS-in-Router
> Advertisements support for IPv6 (
> http://tools.ietf.org/html/draft-jeong-dnsop-ipv6-dns-discovery-12 )
> as implemented in radvd ( http://www.litech.org/radvd/ ).  It
> currently exports the autoconfigured DNS list as /proc/net/ipv6_dns --
> ultimately it ought to (a) implement inotify on this file, so that
> glibc could use it like /etc/resolv.conf and get notifications when
> the DNS list changes, and (b) export the DNS list via netlink as well.
> 
> Comments & discussion, please!
>  --scott

> diff -ruHpN -X dontdiff linux-2.6.22-rc5-orig/include/net/ip6_rdnss.h
> linux-2.6.22-rc5/include/net/ip6_rdnss.h
> --- linux-2.6.22-rc5-orig/include/net/ip6_rdnss.h   1969-12-31
> 19:00:00.0 -0500
> +++ linux-2.6.22-rc5/include/net/ip6_rdnss.h2007-06-21
> 18:16:33.0 -0400 @@ -0,0 +1,58 @@
> +#ifndef _NET_IP6_RDNSS_H
> +#define _NET_IP6_RDNSS_H
> +
> +#ifdef __KERNEL__
> +
> +#include 
> +
> +struct nd_opt_rdnss {
> +   __u8type;
> +   __u8length;
> +#if defined(__BIG_ENDIAN_BITFIELD)
> +   __u8priority:4,
> +   open:1,
> +   reserved1:3;
> +#elif defined(__LITTLE_ENDIAN_BITFIELD)
> +   __u8reserved1:3,
> +   open:1,
> +   priority:4;
> +#else
> +# error not little or big endian
> +#endif

That is not endianess-safe. Don't use foo:x at all
for stuff where a specific endianess is needed. The
compiler doesn't make any guarantee about it.

Please do
__u8 flags;
#define FOOBAR_RESERVED 0x07
#define FOOBAR_OPEN 0x08
#define FOOBAR_PRIORITY 0xF0

and use them in the code.

In general I try to avoid the foo:x stuff, as it
has little or no gain. It just generates worse code.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFD] First draft of RDNSS-in-RA support for IPv6 DNS autoconfiguration

2007-06-22 Thread Michael Buesch
On Saturday 23 June 2007 01:26:19 C. Scott Ananian wrote:
> +struct rdns6_info {
> +   rwlock_tlock;
> +   struct timer_list   expiry_timer;
> +   struct rdns6_entry *rdnss_list;
> +   struct inet6_dev *  in6_dev; /* back pointer for netlink notify */
> +   int expire_all : 1, /* remove entries on ifdown */
> +   free_me : 1; /* safely free this struct */
> +};

Sparse will complain about that.
I suggest you do:

+struct rdns6_info {
+   rwlock_tlock;
+   struct timer_list   expiry_timer;
+   struct rdns6_entry *rdnss_list;
+   struct inet6_dev *  in6_dev; /* back pointer for netlink notify */
+   u8 expire_all; /* remove entries on ifdown */
+   u8 free_me; /* safely free this struct */
+};

Will generate better code and
struct size shouldn't increase. So it's a net win.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFD] First draft of RDNSS-in-RA support for IPv6 DNS autoconfiguration

2007-06-23 Thread Michael Buesch
On Saturday 23 June 2007 02:09:18 C. Scott Ananian wrote:
> > > diff -ruHpN -X dontdiff linux-2.6.22-rc5-orig/include/net/ip6_rdnss.h
> > > linux-2.6.22-rc5/include/net/ip6_rdnss.h
> > > --- linux-2.6.22-rc5-orig/include/net/ip6_rdnss.h1969-12-31
> > > 19:00:00.0 -0500
> > > +++ linux-2.6.22-rc5/include/net/ip6_rdnss.h2007-06-21
> > > 18:16:33.0 -0400 @@ -0,0 +1,58 @@
> > > +#ifndef _NET_IP6_RDNSS_H
> > > +#define _NET_IP6_RDNSS_H
> > > +
> > > +#ifdef __KERNEL__
> > > +
> > > +#include 
> > > +
> > > +struct nd_opt_rdnss {
> > > +__u8type;
> > > +__u8length;
> > > +#if defined(__BIG_ENDIAN_BITFIELD)
> > > +__u8priority:4,
> > > +open:1,
> > > +reserved1:3;
> > > +#elif defined(__LITTLE_ENDIAN_BITFIELD)
> > > +__u8reserved1:3,
> > > +open:1,
> > > +priority:4;
> > > +#else
> > > +# error not little or big endian
> > > +#endif
> >
> > That is not endianess-safe. Don't use foo:x at all
> > for stuff where a specific endianess is needed. The
> > compiler doesn't make any guarantee about it.
> 
> This was copied directly from include/net/ip6_route.h.  I believe that
> it does in fact work, and I (for one) find this much more readable
> than the alternative.  If it is in fact broken, then
> include/net/ip6_route.h (and the 35 other files which use this #ifdef
> in this manner) should be fixed.

Yeah, it might work. But I think the compiler doesn't guarantee
you anything about it.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] b44: power down PHY when interface down

2007-06-30 Thread Michael Buesch
On Saturday 30 June 2007 13:47:35 Török Edvin wrote:
> When the interface is down (or driver removed), the BroadCom 44xx card remains
> powered on, and both its MAC and PHY is using up power.
> This patch makes the driver issue a MAC_CTRL_PHY_PDOWN when the interface
> is halted, and does a partial chip reset turns off the activity LEDs too.
> 
> Applies to 2.6.22-rc6, or current git head.
> 
> Tested on a Broadcom BCM4401-B0 card, it saves ~0.5W (measured using 
> powertop).

Hm, I was going to measure the real power advantage with a
PCI-extender card. But my B44B0 card doesn't seem to work in
that extender card. It works perfectly fine sticked directly into
the motherboard, though, and other cards like a BCM4318 work in
the extender, too.
Not sure what this is.
The extender has an application note about nonworking cards in the
extender and a too big resistor on the board IDSEL pin being the
cause of this.
Maybe I can try with another machine tomorrow.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] NetXen: Support per PCI-function interrupt mask registers

2007-06-30 Thread Michael Buesch
On Saturday 30 June 2007 22:38:46 [EMAIL PROTECTED] wrote:
> This patch updates the various access routines to access different
> control and status settings present in different register locations.
> This will fix problems related to working of different ports in
> multi Port card.
> 
> Signed-off by: Dhananjay Phadke <[EMAIL PROTECTED]>
> Signed-off by: Milan Bag <[EMAIL PROTECTED]>

> + /* Window = 0 or 1 */
> + if (!(adapter->flags & NETXEN_NIC_MSI_ENABLED)) {
> + do {
> + writel(0x, (void *)
> + (PCI_OFFSET_SECOND_RANGE(adapter, 
> ISR_INT_TARGET_STATUS)));
> + mask = readl((void *)
> + (pci_base_offset(adapter, 
> ISR_INT_VECTOR)));

I think you should add a small delay into this loop. Otherwise
it depends on the speed of the CPU (and chipset) how big the total
timeout (32) of the loop is. Could be too small on fast systems.
Something like:
if (!(mask & 0x80))
break;
udelay(10);
} while (++count < 32);
and drop the next line.

> + } while (((mask & 0x80) != 0) && (++count < 32));
> +
> + if ((mask & 0x80) != 0) {
> + printk(KERN_NOTICE "Could not disable interrupt 
> completely\n");
> + }
> + }
> +
> + DPRINTK(1, INFO, "Done with Disable Int\n");
> +
> + return;
>  }

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] NetXen: Graceful teardown of interface and hardware upon module unload

2007-06-30 Thread Michael Buesch
On Saturday 30 June 2007 22:38:47 [EMAIL PROTECTED] wrote:
> These changes allow driver close routine to be called during module unload,
> to clean-up buffers and other software resources, flush queues etc. Also,
> hardware is reset to pristine state. 
> 
> Signed-off-by: Dhananjay Phadke <[EMAIL PROTECTED]>
> Signed-off-by: Milan Bag <[EMAIL PROTECTED]>
> Signed-off-by: Wen Xiong <[EMAIL PROTECTED]>

>   off = netxen_nic_pci_set_window(adapter, memaddr);
>   addr = pci_base_offset(adapter, off);
>   writel(data, addr);
> + do {
> + if (readl(addr) == data)
> + break;
> + msleep_interruptible(100);

If you use msleep_interruptible(), I'd say you should check for
the return value of that call and probably abort firmware
processing here if a signal interrupted us.

>   netxen_nic_reg_write(adapter, NETXEN_ROMUSB_ROM_ADDRESS, addr);
>   netxen_nic_reg_write(adapter, NETXEN_ROMUSB_ROM_ABYTE_CNT, 3);
> @@ -856,10 +853,10 @@ int netxen_pinit_from_rom(struct netxen_
>   netxen_nic_pci_change_crbwindow(adapter, 1);
>   }
>   if (init_delay == 1) {
> - ssleep(1);
> + ssleep(2);

Is it possible/desired to do some interruptible sleep here?
Two seconds uninterruptible sleep is probably a long time.
Even at init/boot.

> Index: netdev-2.6/drivers/net/netxen/netxen_nic_main.c
> ===
> --- netdev-2.6.orig/drivers/net/netxen/netxen_nic_main.c
> +++ netdev-2.6/drivers/net/netxen/netxen_nic_main.c
> @@ -511,6 +511,8 @@ netxen_nic_probe(struct pci_dev *pdev, c
>   val |= 0x4;
>   netxen_nic_write_w0(adapter, NETXEN_PCIE_REG(0x4), val);
>   netxen_nic_read_w0(adapter, NETXEN_PCIE_REG(0x4), &val);
> + if (!(val & 0x4))
> + printk(KERN_ERR "NetXen: bit set failed\n");

That's the award for the most useless error message. :P
Can you give the user a hint what this bit is about?

> + if (adapter->portnum == 0) {
> + if (init_firmware_done) {
> + dma_watchdog_shutdown_request(adapter);
> + msleep(100);
> + i = 100;
> + while ((dma_watchdog_shutdown_poll_result(adapter) != 
> 1) && i) {
> + printk(KERN_INFO "dma_watchdog_shutdown_poll 
> still in progress\n");
> + msleep(100);
> + i--;
> + }
> +
> + if (i == 0) {
> + printk(KERN_ERR "dma_watchdog_shutdown_request 
> failed\n");
> + return;
> + }
> +
> + /* clear the register for future unloads/loads */
> + writel(0, NETXEN_CRB_NORMALIZE(adapter, 
> NETXEN_CAM_RAM(0x1fc)));
> + printk(KERN_INFO "State: 0x%0x\n",
> + readl(NETXEN_CRB_NORMALIZE(adapter, 
> CRB_CMDPEG_STATE)));
> +
> + /* leave the hw in the same state as reboot */
> + writel(0, NETXEN_CRB_NORMALIZE(adapter, 
> CRB_CMDPEG_STATE));
> + if (netxen_pinit_from_rom(adapter, 0))
> + return;
> + udelay(500);

I guess we can do msleep(1) here, too, instead of the huge delay,
although it's twice as long.
Though, I could live with a 500us delay, too, if that's desired.


The rest looks pretty good to me.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] b44: power down PHY when interface down

2007-06-30 Thread Michael Buesch
On Sunday 01 July 2007 00:03:01 Lennert Buytenhek wrote:
> On Sat, Jun 30, 2007 at 11:53:25PM +0200, Michael Buesch wrote:
> 
> > > When the interface is down (or driver removed), the BroadCom 44xx card 
> > > remains
> > > powered on, and both its MAC and PHY is using up power.
> > > This patch makes the driver issue a MAC_CTRL_PHY_PDOWN when the interface
> > > is halted, and does a partial chip reset turns off the activity LEDs too.
> > > 
> > > Applies to 2.6.22-rc6, or current git head.
> > > 
> > > Tested on a Broadcom BCM4401-B0 card, it saves ~0.5W (measured using 
> > > powertop).
> > 
> > Hm, I was going to measure the real power advantage with a
> > PCI-extender card. But my B44B0 card doesn't seem to work in
> > that extender card. It works perfectly fine sticked directly into
> > the motherboard, though, and other cards like a BCM4318 work in
> > the extender, too.
> > Not sure what this is.
> > The extender has an application note about nonworking cards in the
> > extender and a too big resistor on the board IDSEL pin being the
> > cause of this.
> 
> Does the card show up in lspci at all?

No it doesn't.

> IDSEL drive strength 
> issues should only affect config space accesses.

Yeah.

> Does the extender board have a PCI-PCI bridge on it?  (If not,
> there's not really any reason to resistively couple the IDSEL
> line to the host, since the host should take care of that.)

There's no bridge. It just decouples all voltage lines, so you can
drive it from external supply and/or measure voltages and current.
On the PCB it looks like the the IDSEL line is rather directly
routed to the host IDSEL. It just goes through one of the bus
isolation chips. So I guess (just my guess) that this chip has some
resistance and if the total resistance of the chip + the IDSEL
resistor on the mainboard goes above some threshold it doesn't work
anymore for some cards. In the application note they write
about trouble for IDSEL resistors >51ohms.

> > Maybe I can try with another machine tomorrow.
> 
> That would only make a difference if there is no PCI-PCI bridge on
> the extender board.

Well, they suggest it in the application note as a possible fix. ;)


-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RESEND [PATCH 2/3] NetXen: Support per PCI-function interrupt mask registers

2007-07-01 Thread Michael Buesch
On Sunday 01 July 2007 04:43:27 [EMAIL PROTECTED] wrote:
> This patch updates the various access routines to access different
> control and status settings present in different register locations.
> This will fix problems related to working of different ports in
> multi Port card.
> 
> Signed-off by: Dhananjay Phadke <[EMAIL PROTECTED]>
> Signed-off by: Milan Bag <[EMAIL PROTECTED]>
> 
> Index: netdev-2.6/drivers/net/netxen/netxen_nic.h
> ===
> --- netdev-2.6.orig/drivers/net/netxen/netxen_nic.h
> +++ netdev-2.6/drivers/net/netxen/netxen_nic.h
> @@ -937,6 +937,7 @@ struct netxen_adapter {
>   struct netxen_ring_ctx *ctx_desc;
>   struct pci_dev *ctx_desc_pdev;
>   dma_addr_t ctx_desc_phys_addr;
> + int intr_scheme;
>   int (*enable_phy_interrupts) (struct netxen_adapter *);
>   int (*disable_phy_interrupts) (struct netxen_adapter *);
>   void (*handle_phy_intr) (struct netxen_adapter *);
> @@ -1080,37 +1081,103 @@ struct net_device_stats *netxen_nic_get_
>  
>  static inline void netxen_nic_disable_int(struct netxen_adapter *adapter)
>  {
> - /*
> -  * ISR_INT_MASK: Can be read from window 0 or 1.
> -  */
> - writel(0x7ff, PCI_OFFSET_SECOND_RANGE(adapter, ISR_INT_MASK));
> + uint32_tmask = 0x7ff;
> + int count = 0;
>  
> + DPRINTK(1, INFO, "Entered ISR Disable \n");
> +
> + switch (adapter->portnum) {
> + case 0:
> + writel(0x0, NETXEN_CRB_NORMALIZE(adapter, CRB_SW_INT_MASK_0));
> + break;
> + case 1:
> + writel(0x0, NETXEN_CRB_NORMALIZE(adapter, CRB_SW_INT_MASK_1));
> + break;
> + case 2:
> + writel(0x0, NETXEN_CRB_NORMALIZE(adapter, CRB_SW_INT_MASK_2));
> + break;
> + case 3:
> + writel(0x0, NETXEN_CRB_NORMALIZE(adapter, CRB_SW_INT_MASK_3));
> + break;
> + }
> +
> + if (adapter->intr_scheme != -1 &&
> + adapter->intr_scheme != INTR_SCHEME_PERPORT) {
> + writel(mask,
> + (void *)(PCI_OFFSET_SECOND_RANGE(adapter, 
> ISR_INT_MASK)));
> + }
> +
> + /* Window = 0 or 1 */
> + if (!(adapter->flags & NETXEN_NIC_MSI_ENABLED)) {
> + do {
> + writel(0x, (void *)
> + (PCI_OFFSET_SECOND_RANGE(adapter, 
> ISR_INT_TARGET_STATUS)));
> + mask = readl((void *)
> + (pci_base_offset(adapter, 
> ISR_INT_VECTOR)));
> + udelay(10);

This needlessly always delays at least 10 uS, even if it succeed on the
first attempt.
Better do it like I suggested to avoid that.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] b44: power down PHY when interface down

2007-07-01 Thread Michael Buesch
On Sunday 01 July 2007 01:17:34 Lennert Buytenhek wrote:
> More or less.  You can't add the resistances like that, since the
> bus isolation chip buffers the IDSEL signal, but it is correct that
> if the host's IDSEL resistor is larger than a certain value, the
> combination of the resistive coupling of IDSEL plus the extra buffer
> in the isolator might be causing the IDSEL input on the 'guest' PCI
> board to assert too late (or not assert at all), causing config
> accesses to fail.
> 
> (This also depends on the specific 'guest' PCI board used, as you
> noted, due to differing IDSEL trace lengths/capacitances and input
> pin capacitances on different PCI boards.  Also, it might work at
> 33 MHz but not work at 66 MHz, etc.)

It doesn't work on any of my boards :(

> If you feel adventurous, you could try to hack around this by
> figuring out which AD[31:16] line this PCI slot's IDSEL line is
> resistively coupled to (depends on the slot), and then adding
> another parallel resistor on the board itself to make the bus
> isolator's input buffer charge faster.  Note that this does
> increase the load on that specific AD[] line, which might cause
> other funny effects.

Well, but how to find out to which address line it's connected to?
Pretty hard to follow the PCB traces, especially since it's
multilayered.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] b44: power down PHY when interface down

2007-07-01 Thread Michael Buesch
On Sunday 01 July 2007 14:49:23 Török Edvin wrote:
> On 6/30/07, Stephen Hemminger <[EMAIL PROTECTED]> wrote:
> >
> > This is a non-standard formatting for comments, please follow
> > Coding style:
> 
> Thanks, I have updated the patch.
> 
> When the interface is down (or driver removed), the BroadCom 44xx card
> remains powered on,
> and both its MAC and PHY is using up power.
> This patch makes the driver issue a MAC_CTRL_PHY_PDOWN when the
> interface is halted.
> Also doing a partial chip reset turns off the activity LEDs too.
> 
> Signed-off-by: Torok Edwin <[EMAIL PROTECTED]>
> ---
>  b44.c |   28 +++-
>   1 file changed, 23 insertions(+), 5 deletions(-)
> diff --git a/drivers/net/b44.c b/drivers/net/b44.c
> index 879a2ff..43926fd 100644
> --- a/drivers/net/b44.c
> +++ b/drivers/net/b44.c
> @@ -113,6 +113,8 @@ static void b44_init_rings(struct b44 *);
>  #define B44_FULL_RESET   1
>  #define B44_FULL_RESET_SKIP_PHY  2
>  #define B44_PARTIAL_RESET3
> +#define B44_CHIP_RESET_FULL 4
> +#define B44_CHIP_RESET_PARTIAL  5
> 
>  static void b44_init_hw(struct b44 *, int);
> 
> @@ -1283,7 +1285,7 @@ static void b44_clear_stats(struct b44 *bp)
>  }
> 
>  /* bp->lock is held. */
> -static void b44_chip_reset(struct b44 *bp)
> +static void b44_chip_reset(struct b44 *bp, int reset_kind)
>  {
>   if (ssb_is_core_up(bp)) {
>   bw32(bp, B44_RCV_LAZY, 0);
> @@ -1307,6 +1309,13 @@ static void b44_chip_reset(struct b44 *bp)
> 
>   b44_clear_stats(bp);
> 
> + /*
> +  * Don't enable PHY if we are doing a partial reset
> +  * we are probably going to power down
> +  */
> + if (reset_kind == B44_CHIP_RESET_PARTIAL)
> + return;
> +
>   /* Make PHY accessible. */
>   bw32(bp, B44_MDIO_CTRL, (MDIO_CTRL_PREAMBLE |
>(0x0d & MDIO_CTRL_MAXF_MASK)));
> @@ -1332,7 +1341,14 @@ static void b44_chip_reset(struct b44 *bp)
>  static void b44_halt(struct b44 *bp)
>  {
>   b44_disable_ints(bp);
> - b44_chip_reset(bp);
> + /* reset PHY */
> + b44_phy_reset(bp);
> + /* power down PHY */
> + printk(KERN_INFO PFX "%s: powering down PHY\n", bp->dev->name);
> + bw32(bp, B44_MAC_CTRL, MAC_CTRL_PHY_PDOWN);
> + /* now reset the chip, but without enabling the MAC&PHY
> +  * part of it. This has to be done _after_ we shut down the PHY */
> + b44_chip_reset(bp, B44_CHIP_RESET_PARTIAL);
>  }
> 
>  /* bp->lock is held. */
> @@ -1376,7 +1392,7 @@ static void b44_init_hw(struct b44 *bp, int reset_kind)
>  {
>   u32 val;
> 
> - b44_chip_reset(bp);
> + b44_chip_reset(bp, B44_CHIP_RESET_FULL);
>   if (reset_kind == B44_FULL_RESET) {
>   b44_phy_reset(bp);
>   b44_setup_phy(bp);
> @@ -1430,7 +1446,7 @@ static int b44_open(struct net_device *dev)
> 
>   err = request_irq(dev->irq, b44_interrupt, IRQF_SHARED, dev->name, dev);
>   if (unlikely(err < 0)) {
> - b44_chip_reset(bp);
> + b44_chip_reset(bp, B44_CHIP_RESET_PARTIAL);
>   b44_free_rings(bp);
>   b44_free_consistent(bp);
>   goto out;
> @@ -2250,7 +2266,7 @@ static int __devinit b44_init_one(struct pci_dev *pdev,
>   /* Chip reset provides power to the b44 MAC & PCI cores, which
>* is necessary for MAC register access.
>*/
> - b44_chip_reset(bp);
> + b44_chip_reset(bp, B44_CHIP_RESET_FULL);
> 
>   printk(KERN_INFO "%s: Broadcom 4400 10/100BaseT Ethernet ", dev->name);
>   for (i = 0; i < 6; i++)
> @@ -2284,6 +2300,7 @@ static void __devexit b44_remove_one(struct pci_dev 
> *pdev)
>   free_netdev(dev);
>   pci_release_regions(pdev);
>   pci_disable_device(pdev);
> + pci_set_power_state(pdev, PCI_D3hot);
>   pci_set_drvdata(pdev, NULL);
>  }
> 
> @@ -2312,6 +2329,7 @@ static int b44_suspend(struct pci_dev *pdev,
> pm_message_t state)

You patch is also damaged here.
Check your MUA settings.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] b44: power down PHY when interface down

2007-07-01 Thread Michael Buesch
On Sunday 01 July 2007 17:00:06 Lennert Buytenhek wrote:
> On Sun, Jul 01, 2007 at 12:23:16PM +0200, Michael Buesch wrote:
> 
> > > More or less.  You can't add the resistances like that, since the
> > > bus isolation chip buffers the IDSEL signal, but it is correct that
> > > if the host's IDSEL resistor is larger than a certain value, the
> > > combination of the resistive coupling of IDSEL plus the extra buffer
> > > in the isolator might be causing the IDSEL input on the 'guest' PCI
> > > board to assert too late (or not assert at all), causing config
> > > accesses to fail.
> > > 
> > > (This also depends on the specific 'guest' PCI board used, as you
> > > noted, due to differing IDSEL trace lengths/capacitances and input
> > > pin capacitances on different PCI boards.  Also, it might work at
> > > 33 MHz but not work at 66 MHz, etc.)
> > 
> > It doesn't work on any of my boards :(
> 
> What extender board is this?  Do you have docs/schematics?

catalyst pcibx32
http://bu3sch.de/pcibx.php
Docs yes, schematics no.

> And what motherboard brand/type?

ABit AI7
The other was some MSI and some very old random board. dunno.
It works perfectly fine with other cards, like a linksys
wlan card with a broadcom 4318 chip. It's just the b44
that doesn't work in the extender.

> Actually, the IDSEL resistor would be on the computer's
> motherboard, not on the PCI board.  And to which address line

Yeah, I know.

> the IDSEL line is connected depends on which PCI slot on the
> motherboard you're looking at.

Sure.

> A multimeter should do the trick, but I would advise against this
> if you're not totally comfortable with hacking hardware.

Well, you mean to measure the idsel against each possible AD line?
It's difficult, because the motherboard is inside of a standard
computer case and a watercooling system is mounted. So I would
have to disassemble all that stuff. :/
Probably I can measure it with very thin probes on the slots
without unmounting the board, hm...

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] b44: power down PHY when interface down

2007-07-01 Thread Michael Buesch
On Sunday 01 July 2007 17:00:06 Lennert Buytenhek wrote:
> A multimeter should do the trick, but I would advise against this
> if you're not totally comfortable with hacking hardware.

Ok, the resistor on the board is 100ohm, which is too big
according to the docs of the extender.
So what I tried is to connect a 10ohm resistor between
IDSEL and the AD pin, on the extender itself. But it didn't
do the trick. It still doesn't work.
I also connected a 10ohm resistor between IDSEL and the AD
pin directly on the mainboard. Also doesn't work. So I guess
it's some other issue.
But I have no idea what could be wrong besides that.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RESEND [PATCH 3/3] NetXen: Graceful teardown of interface and hardware upon module unload

2007-07-01 Thread Michael Buesch
On Sunday 01 July 2007 20:56:01 [EMAIL PROTECTED] wrote:
> These changes allow driver close routine to be called during module unload,
> to clean-up buffers and other software resources, flush queues etc. Also,
> hardware is reset to pristine state. 
> 
> Signed-off-by: Dhananjay Phadke <[EMAIL PROTECTED]>
> Signed-off-by: Milan Bag <[EMAIL PROTECTED]>
> Signed-off-by: Wen Xiong <[EMAIL PROTECTED]>
> 


> + msleep(1);
> + if (netxen_load_firmware(adapter))
> + return;
> + netxen_phantom_init(adapter, NETXEN_NIC_PEG_TUNE);
> + }
> +
> + /* clear the register for future unloads/loads */
> + writel(0, NETXEN_CRB_NORMALIZE(adapter, NETXEN_CAM_RAM(0x1fc)));
> + printk(KERN_INFO "State: 0x%0x\n",
> + readl(NETXEN_CRB_NORMALIZE(adapter, CRB_CMDPEG_STATE)));
> +
> + dma_watchdog_shutdown_request(adapter);
> + mdelay(100);
> + i = 100;
> + while ((dma_watchdog_shutdown_poll_result(adapter) != 1) && i) {
> + printk(KERN_INFO "dma_watchdog_shutdown_poll still in 
> progress\n");
> + mdelay(100);
> + i--;
> + }

msleep, please.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ixgbe: Introduce new 10GbE driver for Intel 82598 based PCI Express adapters...

2007-07-02 Thread Michael Buesch
On Tuesday 03 July 2007 00:02:57 Kok, Auke wrote:
> well, FWIW when I started looking at adding these flags I looked in various 
> subsystems in the kernel and picked an implementation that suited. Guess what 
> pci.h has? ...:
> 
>   unsigned int msi_enabled:1;
>   unsigned int msix_enabled:1;
> 
> this is literally where I copied the example from
> 
> I suppose I can fix those, but I really don't understand what all the fuzz is 
> about here. We're only conserving memory and staying far away from the real 

I'm not sure if these bitfields actually _do_ conserve memory.
Generated code gets bigger (need bitwise masks and stuff).
Code also needs memory. It probably only conserves memory, if the
structure is instanciated a lot.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2.6] SoftMAC : fix ESSID problem

2007-07-09 Thread Michael Buesch
On Tuesday 10 July 2007 00:19:22 Jean Tourrilhes wrote:
>   Hi,
> 
>   Victor Porton reported that the SoftMAC layer had random
> problem when setting the ESSID :
>   http://bugzilla.kernel.org/show_bug.cgi?id=8686
>   After investigation, it turned out to be worse, the SoftMAC
> layer is left in an inconsistent state. The fix is pretty trivial.
> John : would you mind pushing that to 2.6.23 ?
> Thanks...
> 
> Jean
> 
> Signed-off-by: Jean Tourrilhes <[EMAIL PROTECTED]>
> 
> ---
> 
> --- linux/net/ieee80211/softmac/ieee80211softmac_assoc.j1.c   2007-07-09 
> 13:56:13.0 -0700
> +++ linux/net/ieee80211/softmac/ieee80211softmac_assoc.c  2007-07-09 
> 13:56:41.0 -0700
> @@ -271,8 +271,11 @@ ieee80211softmac_assoc_work(struct work_
>*/
>   dprintk(KERN_INFO PFX "Associate: Scanning for networks 
> first.\n");
>   ieee80211softmac_notify(mac->dev, 
> IEEE80211SOFTMAC_EVENT_SCAN_FINISHED, ieee80211softmac_assoc_notify_scan, 
> NULL);
> - if (ieee80211softmac_start_scan(mac))
> + if (ieee80211softmac_start_scan(mac)) {
>   dprintk(KERN_INFO PFX "Associate: failed to 
> initiate scan. Is device up?\n");
> + mac->associnfo.associating = 0;
> + mac->associnfo.associated = 0;
> +     }
>   goto out;
>   } else {
>   mac->associnfo.associating = 0;
> 

Acked-by: Michael Buesch <[EMAIL PROTECTED]>


-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] b44: Port to SSB

2007-07-12 Thread Michael Buesch
As SSB is submitted for inclusion in mainline,
the b44 port should also be submitted.

All b44-ssb bugs known to me are fixed in this patch.

Please review this. If you want to test this, please pull my
tree from http://bu3sch.de/git/wireless-dev.git/

Index: linux-2.6/drivers/net/b44.c
===
--- linux-2.6.orig/drivers/net/b44.c2007-07-12 01:05:15.0 +0200
+++ linux-2.6/drivers/net/b44.c 2007-07-12 02:03:48.0 +0200
@@ -1,8 +1,11 @@
-/* b44.c: Broadcom 4400 device driver.
+/* b44.c: Broadcom 44xx/47xx Fast Ethernet device driver.
  *
  * Copyright (C) 2002 David S. Miller ([EMAIL PROTECTED])
- * Fixed by Pekka Pietikainen ([EMAIL PROTECTED])
+ * Copyright (C) 2004 Pekka Pietikainen ([EMAIL PROTECTED])
+ * Copyright (C) 2004 Florian Schirmer ([EMAIL PROTECTED])
+ * Copyright (C) 2006 Felix Fietkau ([EMAIL PROTECTED])
  * Copyright (C) 2006 Broadcom Corporation.
+ * Copyright (C) 2007 Michael Buesch <[EMAIL PROTECTED]>
  *
  * Distribute under GPL.
  */
@@ -21,17 +24,18 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
 #include 
 
+
 #include "b44.h"
 
 #define DRV_MODULE_NAME"b44"
 #define PFX DRV_MODULE_NAME": "
-#define DRV_MODULE_VERSION "1.01"
-#define DRV_MODULE_RELDATE "Jun 16, 2006"
+#define DRV_MODULE_VERSION "2.0"
 
 #define B44_DEF_MSG_ENABLE   \
(NETIF_MSG_DRV  | \
@@ -85,10 +89,10 @@
 #define B44_ETHIPV4UDP_HLEN42
 
 static char version[] __devinitdata =
-   DRV_MODULE_NAME ".c:v" DRV_MODULE_VERSION " (" DRV_MODULE_RELDATE ")\n";
+   DRV_MODULE_NAME ".c:v" DRV_MODULE_VERSION "\n";
 
-MODULE_AUTHOR("Florian Schirmer, Pekka Pietikainen, David S. Miller");
-MODULE_DESCRIPTION("Broadcom 4400 10/100 PCI ethernet driver");
+MODULE_AUTHOR("Felix Fietkau, Florian Schirmer, Pekka Pietikainen, David S. 
Miller");
+MODULE_DESCRIPTION("Broadcom 44xx/47xx 10/100 PCI ethernet driver");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_MODULE_VERSION);
 
@@ -96,18 +100,28 @@ static int b44_debug = -1; /* -1 == use 
 module_param(b44_debug, int, 0);
 MODULE_PARM_DESC(b44_debug, "B44 bitmapped debugging message enable value");
 
-static struct pci_device_id b44_pci_tbl[] = {
-   { PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_BCM4401,
- PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL },
-   { PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_BCM4401B0,
- PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL },
-   { PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_BCM4401B1,
- PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL },
-   { } /* terminate list with empty entry */
-};
 
+#ifdef CONFIG_B44_PCI
+static const struct pci_device_id b44_pci_tbl[] = {
+   { PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_BCM4401) },
+   { PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_BCM4401B0) },
+   { PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_BCM4401B1) },
+   { 0 } /* terminate list with empty entry */
+};
 MODULE_DEVICE_TABLE(pci, b44_pci_tbl);
 
+static struct pci_driver b44_pci_driver = {
+   .name   = DRV_MODULE_NAME,
+   .id_table   = b44_pci_tbl,
+};
+#endif /* CONFIG_B44_PCI */
+
+static const struct ssb_device_id b44_ssb_tbl[] = {
+   SSB_DEVICE(SSB_VENDOR_BROADCOM, SSB_DEV_ETHERNET, SSB_ANY_REV),
+   SSB_DEVTABLE_END
+};
+MODULE_DEVICE_TABLE(ssb, b44_ssb_tbl);
+
 static void b44_halt(struct b44 *);
 static void b44_init_rings(struct b44 *);
 
@@ -119,6 +133,7 @@ static void b44_init_hw(struct b44 *, in
 
 static int dma_desc_align_mask;
 static int dma_desc_sync_size;
+static int instance;
 
 static const char b44_gstrings[][ETH_GSTRING_LEN] = {
 #define _B44(x...) # x,
@@ -126,35 +141,35 @@ B44_STAT_REG_DECLARE
 #undef _B44
 };
 
-static inline void b44_sync_dma_desc_for_device(struct pci_dev *pdev,
-dma_addr_t dma_base,
-unsigned long offset,
-enum dma_data_direction dir)
-{
-   dma_sync_single_range_for_device(&pdev->dev, dma_base,
-offset & dma_desc_align_mask,
-dma_desc_sync_size, dir);
-}
-
-static inline void b44_sync_dma_desc_for_cpu(struct pci_dev *pdev,
- dma_addr_t dma_base,
- unsigned long offset,
- enum dma_data_direction dir)
-{
-   dma_sync_single_range_for_cpu(&pdev->dev, dma_base,
- offset & dma_desc_align_mask,
- dma_desc_sync_size, dir);
+static inlin

Re: [PATCH #2] Merge the Sonics Silicon Backplane subsystem

2007-07-14 Thread Michael Buesch
On Saturday 14 July 2007 19:26:05 Christoph Hellwig wrote:
> On Sat, Jul 14, 2007 at 07:18:20PM +0200, Michael Buesch wrote:
> > --- /dev/null
> > +++ b/drivers/ssb/Makefile
> > @@ -0,0 +1,11 @@
> > +ssb-builtin-drivers-y  += 
> > driver_chipcommon.o
> > +ssb-builtin-drivers-$(CONFIG_SSB_DRIVER_MIPS)  += 
> > driver_mipscore.o
> > +ssb-builtin-drivers-$(CONFIG_SSB_DRIVER_PCICORE)   += driver_pcicore.o
> > +
> > +ssb-hostsupport-$(CONFIG_SSB_PCIHOST)  += pci.o 
> > pcihost_wrapper.o
> > +ssb-hostsupport-$(CONFIG_SSB_PCMCIAHOST)   += pcmcia.o
> > +
> > +obj-$(CONFIG_SSB) += ssb.o
> > +
> > +ssb-objs   := main.o scan.o \
> > +  $(ssb-hostsupport-y) $(ssb-builtin-drivers-y)
> 
> Whoa, this makefile is more than ugly :)
> 
> Please try something like:
> 
> # core
> ssb-y += main.o scan.o
> 
> # host support
> ssb-$(CONFIG_SSB_PCIHOST) += pci.o pcihost_wrapper.o
> ssb-$(CONFIG_SSB_PCMCIAHOST)  += pcmcia.o
> 
> # drivers
> ssb-y += driver_chipcommon.o
> ssb-$(CONFIG_SSB_DRIVER_MIPS) += driver_mipscore.o
> ssb-$(CONFIG_SSB_DRIVER_PCICORE)  += driver_pcicore.o
> 
> obj-$(CONFIG_SSB) += ssb.o
> 
> instead
> 
> 
> > +   default y
> 
> please don't add defauly y statements for random drivers.
> 

Thanks for your comments. I fixed that and will submit it
with the next round.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] Blackfin ethernet driver: on chip ethernet MAC controller driver

2007-07-14 Thread Michael Buesch
On Saturday 14 July 2007 20:49:53 Bryan Wu wrote:
> +#if defined(CONFIG_BFIN_MAC_USE_L1)
> +# define bfin_mac_alloc(dma_handle, size)  l1_data_sram_zalloc(size)
> +# define bfin_mac_free(dma_handle, ptr)l1_data_sram_free(ptr)
> +#else
> +# define bfin_mac_alloc(dma_handle, size) \
> +  dma_alloc_coherent(NULL, size, dma_handle, GFP_DMA)

You shouldn't be using GFP_DMA here (that's only for legacy ISA-DMA
memory), but one of GFP_KERNEL or GFP_ATOMIC.

> +# define bfin_mac_free(dma_handle, ptr) \
> + dma_free_coherent(NULL, sizeof(*ptr), ptr, dma_handle)
> +#endif
> +

> +/* pointers to maintain transmit list */
> +static struct net_dma_desc_tx *tx_list_head;
> +static struct net_dma_desc_tx *tx_list_tail;
> +static struct net_dma_desc_rx *rx_list_head;
> +static struct net_dma_desc_rx *rx_list_tail;
> +static struct net_dma_desc_rx *current_rx_ptr;
> +static struct net_dma_desc_tx *current_tx_ptr;
> +static struct net_dma_desc_tx *tx_desc;
> +static struct net_dma_desc_rx *rx_desc;

Hm, from this I guess only one of these devices is possible
at a time in the system?

> +static int desc_list_init(void)
> +{
> + struct net_dma_desc_tx *tmp_desc_tx;
> + struct net_dma_desc_rx *tmp_desc_rx;
> + int i;
> + struct sk_buff *new_skb;
> +#if !defined(CONFIG_BFIN_MAC_USE_L1)
> + dma_addr_t dma_handle;
> +#endif
> +
> + tx_desc =
> + bfin_mac_alloc(&dma_handle,
> +sizeof(struct net_dma_desc_tx) *
> +CONFIG_BFIN_TX_DESC_NUM);
> + if (tx_desc == NULL)
> + goto init_error;
> +
> + rx_desc =
> + bfin_mac_alloc(&dma_handle,
> +sizeof(struct net_dma_desc_rx) *
> +CONFIG_BFIN_RX_DESC_NUM);

You are overriding and throwing dma_handle away, here.
You might need it later for freeing. (and maybe for other
purposes, too, like writing the address to a device register?)

> + if (rx_desc == NULL)
> + goto init_error;
> +
> + /* init tx_list */
> + for (i = 0; i < CONFIG_BFIN_TX_DESC_NUM; i++) {
> +
> + tmp_desc_tx = tx_desc + i;
> +
> + if (i == 0) {
> + tx_list_head = tmp_desc_tx;
> + tx_list_tail = tmp_desc_tx;
> + }
> +
> + tmp_desc_tx->desc_a.start_addr =
> + (unsigned long)tmp_desc_tx->packet;
> + tmp_desc_tx->desc_a.x_count = 0;
> + /* disabled */
> + tmp_desc_tx->desc_a.config.b_DMA_EN = 0;
> + /* read from memory */
> + tmp_desc_tx->desc_a.config.b_WNR = 0;
> + /* wordsize is 32 bits */
> + tmp_desc_tx->desc_a.config.b_WDSIZE = 2;
> + /* 6 half words is desc size. */
> + tmp_desc_tx->desc_a.config.b_NDSIZE = 6;
> + /* large desc flow */
> + tmp_desc_tx->desc_a.config.b_FLOW = 7;
> + tmp_desc_tx->desc_a.next_dma_desc = &(tmp_desc_tx->desc_b);
> +
> + tmp_desc_tx->desc_b.start_addr =
> + (unsigned long)(&(tmp_desc_tx->status));
> + tmp_desc_tx->desc_b.x_count = 0;
> + /* enabled */
> + tmp_desc_tx->desc_b.config.b_DMA_EN = 1;
> + /* write to memory */
> + tmp_desc_tx->desc_b.config.b_WNR = 1;
> + /* wordsize is 32 bits */
> + tmp_desc_tx->desc_b.config.b_WDSIZE = 2;
> + /* disable interrupt */
> + tmp_desc_tx->desc_b.config.b_DI_EN = 0;
> + tmp_desc_tx->desc_b.config.b_NDSIZE = 6;
> + /* stop mode */
> + tmp_desc_tx->desc_b.config.b_FLOW = 7;
> + tmp_desc_tx->skb = NULL;
> + tx_list_tail->desc_b.next_dma_desc = &(tmp_desc_tx->desc_a);
> + tx_list_tail->next = tmp_desc_tx;
> +
> + tx_list_tail = tmp_desc_tx;
> + }
> + tx_list_tail->next = tx_list_head;  /* tx_list is a circle */
> + tx_list_tail->desc_b.next_dma_desc = &(tx_list_head->desc_a);
> + current_tx_ptr = tx_list_head;
> +
> + /* init rx_list */
> + for (i = 0; i < CONFIG_BFIN_RX_DESC_NUM; i++) {
> +
> + tmp_desc_rx = rx_desc + i;
> +
> + if (i == 0) {
> + rx_list_head = tmp_desc_rx;
> + rx_list_tail = tmp_desc_rx;
> + }
> +
> + /* allocat a new skb for next time receive */
> + new_skb = dev_alloc_skb(PKT_BUF_SZ + 2);
> + if (!new_skb) {
> + printk(KERN_NOTICE CARDNAME
> +": init: low on mem - packet dropped\n");
> + goto init_error;
> + }
> + skb_reserve(new_skb, 2);
> + tmp_desc_rx->skb = new_skb;
> + /* since RXDWA is enabled */
> + tmp_desc_rx->desc_a.start_addr =
> + (unsigned long)new_skb->data - 2;
> + tmp_desc_rx->desc_a.x_

Re: [PATCH 1/3] Blackfin ethernet driver: on chip ethernet MAC controller driver

2007-07-14 Thread Michael Buesch
On Saturday 14 July 2007 21:49:21 Mike Frysinger wrote:
> On 7/14/07, Michael Buesch <[EMAIL PROTECTED]> wrote:
> > On Saturday 14 July 2007 20:49:53 Bryan Wu wrote:
> > > +static int __init bf537mac_probe(struct net_device *dev)
> > > +{
> > > + struct bf537mac_local *lp = netdev_priv(dev);
> > > + int retval;
> > > +
> > > + /* Grab the MAC address in the MAC */
> > > + *(u32 *) (&(dev->dev_addr[0])) = bfin_read_EMAC_ADDRLO();
> > > + *(u16 *) (&(dev->dev_addr[4])) = (u16) bfin_read_EMAC_ADDRHI();
> >
> > Endianess broken.
> >
> > > +struct dma_config_reg {
> > > + unsigned short b_DMA_EN:1;  /* Bit 0 : DMA Enable */
> > > + unsigned short b_WNR:1; /* Bit 1 : DMA Direction */
> > > + unsigned short b_WDSIZE:2;  /* Bit 2 & 3 : DMA Tranfer Word size */
> > > + unsigned short b_DMA2D:1;   /* Bit 4 : DMA Mode 2D or 1D */
> > > + unsigned short b_RESTART:1; /* Bit 5 : Retain the FIFO */
> > > + unsigned short b_DI_SEL:1;  /* Bit 6 : Data Interrupt Timing Select 
> > > */
> > > + unsigned short b_DI_EN:1;   /* Bit 7 : Data Interrupt Enable */
> > > + unsigned short b_NDSIZE:4;  /* Bit 8 to 11 : Flex descriptor Size */
> > > + unsigned short b_FLOW:3;/* Bit 12 to 14 : FLOW */
> > > +};
> >
> > This is most likely not endianess safe.
> 
> do we really need to care about this ?  this is a driver for a MAC
> which can only be found on Blackfin processors and Blackfin itself is
> only little endian.

Well, this bitfield _might_ be OK (although I don't like bitfields
at all), but the above pointer casting stuff should really use
leXX_to_cpu. It's so easy to use and it is easier to read and
maintain the code afterwards.
If the bitfield stays, a comment must be added.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] Blackfin ethernet driver: on chip ethernet MAC controller driver

2007-07-15 Thread Michael Buesch
On Sunday 15 July 2007 11:16:56 Bryan Wu wrote:
> In current Blackfin DMA allocation/free, the return value from 
> bfin_mac_alloc() is used as the dma_handle, here it is the
> tx_desc/rx_desc. 
> The "dma_handle" is useless in the following code.

I think a comment has to be added, at least, as it's very confusing.

> This is some magic code for PHY ID. In the future, we will rewrite some
> code based on kernel phy abstraction layer to support more phy device.

Ok, nice idea.

> > Unwind the allocations above, if registering fails.
> > 
> 
> In fact, it is safe. Because if registering fails, bf537mac_probe will
> return none zero to bfin_mac_probe which will do free_netdev.

Hm, weren't there some DMA allocations, too? Are they free'd properly?

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH try#2] Blackfin ethernet driver: on chip ethernet MAC controller driver

2007-07-15 Thread Michael Buesch
On Sunday 15 July 2007 11:27:09 Bryan Wu wrote:
> +#if defined(CONFIG_BFIN_MAC_USE_L1)
> +# define bfin_mac_alloc(dma_handle, size)  l1_data_sram_zalloc(size)
> +# define bfin_mac_free(dma_handle, ptr)l1_data_sram_free(ptr)
> +#else
> +# define bfin_mac_alloc(dma_handle, size) \
> +  dma_alloc_coherent(NULL, size, dma_handle, GFP_NORMAL)

What is GFP_NORMAL? It's not defined in latest linus' tree.
I think you should use GFP_KERNEL, if you can sleep, or GFP_ATOMIC,
if you can't.

The rest looks OK. Except the endianess issues. It might be no
issue on the hardware this runs on, but in favour of "clean code"
you might use leXX_to_cpu and friends anyway. :)
This kind of bugs is done so often, even in places where it _does_
matter. So, at least for the human reader, the leXX_to_cpu stuff
says that you really understood what you were doing when writing
the code. The current code says (to me), that it works by
accident, somehow, although it seems you knew what you were doing. :)

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH try#2] Blackfin ethernet driver: on chip ethernet MAC controller driver

2007-07-15 Thread Michael Buesch
On Sunday 15 July 2007 14:07:44 Bryan Wu wrote:
> @@ -483,9 +487,12 @@
>  
>  void setup_mac_addr(u8 * mac_addr)
>  {
> + u32 addr_low = le32_to_cpu(*(u32 *) & mac_addr[0]);
> + u16 addr_hi = le16_to_cpu(*(u16 *) & mac_addr[4]);
> +
>   /* this depends on a little-endian machine */
> - bfin_write_EMAC_ADDRLO(*(u32 *) & mac_addr[0]);
> - bfin_write_EMAC_ADDRHI(*(u16 *) & mac_addr[4]);
> + bfin_write_EMAC_ADDRLO(addr_low);
> + bfin_write_EMAC_ADDRHI(addr_hi);
>  }
>  
>  static void adjust_tx_list(void)
> @@ -866,10 +873,10 @@
>   int retval;
>  
>   /* Grab the MAC address in the MAC */
> - *(u32 *) (&(dev->dev_addr[0])) = bfin_read_EMAC_ADDRLO();
> - *(u16 *) (&(dev->dev_addr[4])) = (u16) bfin_read_EMAC_ADDRHI();
> + *(u32 *) (&(dev->dev_addr[0])) = cpu_to_le32(bfin_read_EMAC_ADDRLO());
> + *(u16 *) (&(dev->dev_addr[4])) = cpu_to_le16((u16) 
> bfin_read_EMAC_ADDRHI());

Try something like this:

@@ -483,9 +487,12 @@
 
 void setup_mac_addr(u8 * mac_addr)
 {
+   u32 addr_low = le32_to_cpu(*(__le32 *) & mac_addr[0]);
+   u16 addr_hi = le16_to_cpu(*(__le16 *) & mac_addr[4]);
+
-   /* this depends on a little-endian machine */
-   bfin_write_EMAC_ADDRLO(*(u32 *) & mac_addr[0]);
-   bfin_write_EMAC_ADDRHI(*(u16 *) & mac_addr[4]);
+   bfin_write_EMAC_ADDRLO(addr_low);
+   bfin_write_EMAC_ADDRHI(addr_hi);
 }
 
 static void adjust_tx_list(void)
@@ -866,10 +873,10 @@
int retval;
 
/* Grab the MAC address in the MAC */
-   *(u32 *) (&(dev->dev_addr[0])) = bfin_read_EMAC_ADDRLO();
-   *(u16 *) (&(dev->dev_addr[4])) = (u16) bfin_read_EMAC_ADDRHI();
+   *(__le32 *) (&(dev->dev_addr[0])) = 
cpu_to_le32(bfin_read_EMAC_ADDRLO());
+   *(__le16 *) (&(dev->dev_addr[4])) = cpu_to_le16((u16) 
bfin_read_EMAC_ADDRHI());

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH try#2] Blackfin ethernet driver: on chip ethernet MAC controller driver

2007-07-15 Thread Michael Buesch
On Sunday 15 July 2007 16:01, Bryan Wu wrote:

> drivers/net/e100.c: ns->tx_window_errors += 
> le32_to_cpu(s->tx_late_collisions);
> drivers/net/e100.c: ns->tx_carrier_errors += 
> le32_to_cpu(s->tx_lost_crs);
> drivers/net/e100.c: ns->tx_fifo_errors += 
> le32_to_cpu(s->tx_underruns);
> drivers/net/e100.c: ns->tx_errors += 
> le32_to_cpu(s->tx_max_collisions) +
> drivers/net/e100.c: le32_to_cpu(s->tx_lost_crs);
> drivers/net/e100.c: ns->rx_length_errors += 
> le32_to_cpu(s->rx_short_frame_errors) +
> drivers/net/e100.c: ns->rx_crc_errors += 
> le32_to_cpu(s->rx_crc_errors);
> drivers/net/e100.c: ns->rx_frame_errors += 
> le32_to_cpu(s->rx_alignment_errors);
> drivers/net/e100.c: ns->rx_over_errors += 
> le32_to_cpu(s->rx_overrun_errors);
> drivers/net/e100.c: ns->rx_fifo_errors += 
> le32_to_cpu(s->rx_overrun_errors);
> drivers/net/e100.c: ns->rx_missed_errors += 
> le32_to_cpu(s->rx_resource_errors);
> drivers/net/e100.c: ns->rx_errors += 
> le32_to_cpu(s->rx_crc_errors) +
> drivers/net/e100.c: le32_to_cpu(s->rx_alignment_errors) +
> drivers/net/e100.c: le32_to_cpu(s->rx_short_frame_errors) 
> +
> drivers/net/e100.c: le32_to_cpu(s->rx_cdt_errors);
> drivers/net/e100.c: nic->tx_deferred += 
> le32_to_cpu(s->tx_deferred);
> drivers/net/e100.c: le32_to_cpu(s->tx_single_collisions);
> drivers/net/e100.c: 
> le32_to_cpu(s->tx_multiple_collisions);
> drivers/net/e100.c: nic->tx_fc_pause += 
> le32_to_cpu(s->fc_xmt_pause);
> drivers/net/e100.c: nic->rx_fc_pause += 
> le32_to_cpu(s->fc_rcv_pause);
> drivers/net/e100.c: 
> le32_to_cpu(s->fc_rcv_unsupported);
> drivers/net/e100.c: 
> le32_to_cpu(cb->u.tcb.tbd.buf_addr),
> drivers/net/e100.c: 
> le32_to_cpu(cb->u.tcb.tbd.buf_addr),
> ---
> 
> Normally, it is used to protect some rx/tx status flags or dma buf addr.
> 
> Any guide line for this leXX_to_cpu usage?

Easy.

leXX_to_cpu converts a little endian value to CPU-endianess.
So if your CPU is LE, this is a no-op.
The cpu_to_leXX func converts from CPU-endianess to LE.
Same goes for the bigendian variants of the funcs.

In drivers you often have values with specific endianess
(mostly LE, sometimes BE). For example when values are put
by the device to DMA memory. So if the device works in LE,
you must convert the value from LE to CPU endianess after
reading it from DMA.

I your case, however, the issue is different. You had a byte
array and did pointer casting tricks on it. Pointer casting
_is_ a common source for endianess issues. In general, if
you try to cast a pointer, think twice about it. It's likely
a bug. So you had a pointer to a byte array, which is u8*.
You casted that to u16* and u32*. That's broken, because
an u8 array is "little endian". LE means the least significant
part of the entity comes first in memory. Which is obviously
always true for a byte array. So you must use
leXX_to_cpu when doing this kind of pointer tricks.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] remove unused routines from d80211 version of bcm43xx

2006-07-14 Thread Michael Buesch
On Friday 14 July 2006 23:04, you wrote:
> This patch removes some unused static inline routines from bcm43xx-d80211 
> version of bcm43xx_main.h.
> 
> Signed-Off-By: Larry Finger <[EMAIL PROTECTED]>

Signed-off-by: Michael Buesch <[EMAIL PROTECTED]>

> ===
> 
> index e228833..04d8b91 100644
> --- a/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.h
> +++ b/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.h
> @@ -112,30 +112,6 @@ int bcm43xx_channel_to_freq(struct bcm43
>  return bcm43xx_channel_to_freq_bg(channel);
>   }
> 
> -/* Lightweight function to check if a channel number is valid.
> - * Note that this does _NOT_ check for geographical restrictions!
> - */
> -static inline
> -int bcm43xx_is_valid_channel_a(u8 channel)
> -{
> -return (channel >= IEEE80211_52GHZ_MIN_CHANNEL
> -   && channel <= IEEE80211_52GHZ_MAX_CHANNEL);
> -}
> -static inline
> -int bcm43xx_is_valid_channel_bg(u8 channel)
> -{
> -return (channel >= IEEE80211_24GHZ_MIN_CHANNEL
> -   && channel <= IEEE80211_24GHZ_MAX_CHANNEL);
> -}
> -static inline
> -int bcm43xx_is_valid_channel(struct bcm43xx_private *bcm,
> -  u8 channel)
> -{
> -if (bcm43xx_current_phy(bcm)->type == BCM43xx_PHYTYPE_A)
> -return bcm43xx_is_valid_channel_a(channel);
> -return bcm43xx_is_valid_channel_bg(channel);
> -}
> -
>   void bcm43xx_tsf_read(struct bcm43xx_private *bcm, u64 *tsf);
>   void bcm43xx_tsf_write(struct bcm43xx_private *bcm, u64 tsf);

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC, RFT] Re: [PATCH] d80211: make sleeping in hw->config possible #2

2006-07-18 Thread Michael Buesch
On Tuesday 18 July 2006 18:57, Jiri Benc wrote:
> On Tue, 11 Jul 2006 00:54:33 +0200, Michael Buesch wrote:
> > This patch makes sleeping in the hw->config callback possible
> > by removing the only atomic caller. The atomic caller was
> > a timer and is replaced by a workqueue.
> 
> This is a modified version of the patch that doesn't use
> cancel_rearming_delayed_work.
> 
> Signed-off-by: Jiri Benc <[EMAIL PROTECTED]>
> 
> ---
>  net/d80211/ieee80211.c   |   23 +++
>  net/d80211/ieee80211_i.h |3 ++-
>  net/d80211/ieee80211_iface.c |   10 --
>  net/d80211/ieee80211_sta.c   |   37 +
>  4 files changed, 42 insertions(+), 31 deletions(-)
> 
> --- dscape.orig/net/d80211/ieee80211.c
> +++ dscape/net/d80211/ieee80211.c
> @@ -4552,14 +4552,6 @@ void ieee80211_unregister_hw(struct net_
>  tasklet_disable(&local->tasklet);
>  /* TODO: skb_queue should be empty here, no need to do anything? */
>  
> - if (local->rate_limit)
> - del_timer_sync(&local->rate_limit_timer);
> - if (local->stat_time)
> - del_timer_sync(&local->stat_timer);
> - if (local->scan_timer.data)
> - del_timer_sync(&local->scan_timer);
> - ieee80211_rx_bss_list_deinit(dev);
> -
>   rtnl_lock();
>   local->reg_state = IEEE80211_DEV_UNREGISTERED;
>   if (local->apdev)
> @@ -4572,6 +4564,21 @@ void ieee80211_unregister_hw(struct net_
>   }
>   rtnl_unlock();
>  
> + if (local->rate_limit)
> + del_timer_sync(&local->rate_limit_timer);
> + if (local->stat_time)
> + del_timer_sync(&local->stat_timer);
> + if (local->scan_work.data) {
> + local->sta_scanning = 0;
> + cancel_delayed_work(&local->scan_work);
> + flush_scheduled_work();
> + /* The scan_work is guaranteed not to be called at this
> +  * point. It is not scheduled and not running now. It can be

If it is guaranteed to be not to be called, the
cancel_delayed_work(&local->scan_work);
is unnecessary.
If it is possible to be scheduled and delayed here,
it's a bug.

So, if the first is true, we should remove it and if the second
is true, well :)

The flush_scheduled_work() call is necessary, though.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC, RFT] Re: [PATCH] d80211: make sleeping in hw->config possible #2

2006-07-18 Thread Michael Buesch
On Tuesday 18 July 2006 19:36, Michael Buesch wrote:
> On Tuesday 18 July 2006 18:57, Jiri Benc wrote:
> > On Tue, 11 Jul 2006 00:54:33 +0200, Michael Buesch wrote:
> > > This patch makes sleeping in the hw->config callback possible
> > > by removing the only atomic caller. The atomic caller was
> > > a timer and is replaced by a workqueue.
> > 
> > This is a modified version of the patch that doesn't use
> > cancel_rearming_delayed_work.
> > 
> > Signed-off-by: Jiri Benc <[EMAIL PROTECTED]>
> > 
> > ---
> >  net/d80211/ieee80211.c   |   23 +++
> >  net/d80211/ieee80211_i.h |3 ++-
> >  net/d80211/ieee80211_iface.c |   10 --
> >  net/d80211/ieee80211_sta.c   |   37 +
> >  4 files changed, 42 insertions(+), 31 deletions(-)
> > 
> > --- dscape.orig/net/d80211/ieee80211.c
> > +++ dscape/net/d80211/ieee80211.c
> > @@ -4552,14 +4552,6 @@ void ieee80211_unregister_hw(struct net_
> >  tasklet_disable(&local->tasklet);
> >  /* TODO: skb_queue should be empty here, no need to do anything? */
> >  
> > -   if (local->rate_limit)
> > -   del_timer_sync(&local->rate_limit_timer);
> > -   if (local->stat_time)
> > -   del_timer_sync(&local->stat_timer);
> > -   if (local->scan_timer.data)
> > -   del_timer_sync(&local->scan_timer);
> > -   ieee80211_rx_bss_list_deinit(dev);
> > -
> > rtnl_lock();
> > local->reg_state = IEEE80211_DEV_UNREGISTERED;
> > if (local->apdev)
> > @@ -4572,6 +4564,21 @@ void ieee80211_unregister_hw(struct net_
> > }
> > rtnl_unlock();
> >  
> > +   if (local->rate_limit)
> > +   del_timer_sync(&local->rate_limit_timer);
> > +   if (local->stat_time)
> > +   del_timer_sync(&local->stat_timer);
> > +   if (local->scan_work.data) {
> > +   local->sta_scanning = 0;
> > +   cancel_delayed_work(&local->scan_work);
> > +   flush_scheduled_work();
> > +   /* The scan_work is guaranteed not to be called at this
> > +* point. It is not scheduled and not running now. It can be
> 
> If it is guaranteed to be not to be called, the
> cancel_delayed_work(&local->scan_work);
> is unnecessary.
> If it is possible to be scheduled and delayed here,
> it's a bug.
> 
> So, if the first is true, we should remove it and if the second
> is true, well :)
> 
> The flush_scheduled_work() call is necessary, though.

Oh, no. I misread the code.
I think it's ok. Sorry for the noise :(

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Wireless statistics for bcm43xx-d80211

2006-07-19 Thread Michael Buesch
On Wednesday 19 July 2006 06:24, Larry Finger wrote:
> I have gotten most things working to produce wireless statistics through 
> /proc/net/wireless for
> bcm43xx-d80211; however, I have one problem that I have not yet been able to 
> solve. When I do a 'cat
> /proc/net/wireless', the following is printed:
> 
> Inter-| sta-|   Quality|   Discarded packets   | Missed | 
> WE
>   face | tus | link level noise |  nwid  crypt   frag  retry   misc | beacon 
> | 20
> wmaster0:   100.0.0.   0  0  0  0  00
>   wlan1:   100.  -26.  -67.   0  0  0  0  00
> 
> Based on the numbers obtained using bcm43xx-softmac for my interface, the 
> numbers for level and 
> noise for wlan1 are what I expected (in dBm). The link value has not yet been 
> finished. The main 
> problem is that the wireless kicker applet for KDE, which I use for a 
> display, is only looking at 
> the first line, 

So it is broken. Fullstop.
Bug KDE for this.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: incorrect usage of UTS_RELEASE in bcm43xx_get_drvinfo

2006-07-21 Thread Michael Buesch
On Friday 21 July 2006 20:59, Olaf Hering wrote:
> 
> bcm43xx_get_drvinfo in 2.6.18-rc2 unfortunately uses UTS_RELEASE as
> driver version. I think this specific info can be obtained by other
> ways.
> Can you give bcm43xx a real version number and provide it via the
> ethtool ioctl?

I am not going to maintain a bogus version number.
What about simply returning 1.0 and be done with it.
I don't think it matters. 1.0 is as bogus as any other value.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: incorrect usage of UTS_RELEASE in bcm43xx_get_drvinfo

2006-07-21 Thread Michael Buesch
On Friday 21 July 2006 21:13, Jason Lunz wrote:
> On Fri, Jul 21, 2006 at 09:03:23PM +0200, Michael Buesch wrote:
> > I am not going to maintain a bogus version number.
> > What about simply returning 1.0 and be done with it.
> > I don't think it matters. 1.0 is as bogus as any other value.
> 
> Put it to use! Increment it whenever there's a major change in the

What is a major change? Why only on major change? Why not on
simple changes, too? Simple changes sometimes introduce bugs, too.

> driver, like the locking rewrite or the init routine rewrite. At the
> very least, it makes it easier to rule certain things out if a user
> reports lockup or something.

What I wanted to say: I _never_ had debugging-problems, because
I did not know the source state it was compiled from.

If there is a lockup, and I don't see if a specific change
that might cause it is in or not, I simply ask him. It's _much_
easier than maintaining a bogus counter.

The version counter will cause patch hunks to fail and whatever.
It's only useless trouble.

If someone is not able to answer questions like:
"Which kernel version do you run?"
"Please look at foo.c line 234 and tell me what's there."
He does not want to debug the problem anyway. Problem solved.
Someone else will appear to help debugging it, if it's a real bug.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 2/2] myri10ge - Write the firmware in 256-bytes chunks

2006-07-21 Thread Michael Buesch
On Friday 21 July 2006 21:49, Brice Goglin wrote:
> When writing the firmware to the NIC, the FIFO is 256-bytes long,
> so we use 256-bytes chunks and a read to wait until the previous
> write is done.
> 
> Signed-off-by: Brice Goglin <[EMAIL PROTECTED]>
> ---
>  drivers/net/myri10ge/myri10ge.c |   20 
>  1 file changed, 8 insertions(+), 12 deletions(-)
> 
> Index: linux-mm/drivers/net/myri10ge/myri10ge.c
> ===
> --- linux-mm.orig/drivers/net/myri10ge/myri10ge.c 2006-07-18 
> 15:17:55.0 -0400
> +++ linux-mm/drivers/net/myri10ge/myri10ge.c  2006-07-18 15:19:44.0 
> -0400
> @@ -448,6 +448,7 @@
>   struct mcp_gen_header *hdr;
>   size_t hdr_offset;
>   int status;
> + unsigned i;
>  
>   if ((status = request_firmware(&fw, mgp->fw_name, dev)) < 0) {
>   dev_err(dev, "Unable to load %s firmware image via hotplug\n",
> @@ -479,18 +480,13 @@
>   goto abort_with_fw;
>  
>   crc = crc32(~0, fw->data, fw->size);
> - if (mgp->tx.boundary == 2048) {
> - /* Avoid PCI burst on chipset with unaligned completions. */
> - int i;
> - __iomem u32 *ptr = (__iomem u32 *) (mgp->sram +
> - MYRI10GE_FW_OFFSET);
> - for (i = 0; i < fw->size / 4; i++) {
> - __raw_writel(((u32 *) fw->data)[i], ptr + i);
> - wmb();
> - }
> - } else {
> - myri10ge_pio_copy(mgp->sram + MYRI10GE_FW_OFFSET, fw->data,
> -   fw->size);
> + for (i = 0; i < fw->size; i += 256) {
> + myri10ge_pio_copy(mgp->sram + MYRI10GE_FW_OFFSET + i,
> +   fw->data + i,
> +   min(256U, (unsigned)(fw->size - i)));
> + mb();
> + readb(mgp->sram);
> + mb();

Why two mb() here?
I would say actually none is needed.
The readb fully synchronizes the previous writes on bus level
(and so on CPU level, too).

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH wireless-dev] d80211: Make MACSTR/MAC2STR macro available to drivers

2006-07-23 Thread Michael Buesch
On Sunday 23 July 2006 10:43, Michael Wu wrote:
> d80211: Make MACSTR/MAC2STR macro available to drivers
> 
> This patch moves the MACSTR/MAC2STR macros to d80211.h
> so that they are available to drivers. It also converts the adm8211
> and bcm43xx drivers to use this macro.

I'd say d80211.h is still the wrong place for this.
What about etherdevice.h?

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


wireless-2.6 git repos broken

2006-07-23 Thread Michael Buesch
Hi John,

I would say something is broken in the wireless-2.6 git repository.
When I try to clone the tree, I get this error:

[EMAIL PROTECTED]:/tmp$ git --version
git version 1.2.3
[EMAIL PROTECTED]:/tmp$ git clone  
git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git
fatal: packfile '/tmp/wireless-2.6/.git/objects/pack/tmp-ABKsAK' SHA1 mismatch
error: git-clone-pack: unable to read from git-index-pack
error: git-index-pack died with error code 128
clone-pack from 
'git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git' 
failed.


[EMAIL PROTECTED]:/tmp$ git --version
git version 1.4.1
[EMAIL PROTECTED]:/tmp$ git clone  
git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git
Generating pack...
Done counting 290305 objects.
Deltifying 290305 objects.
 100% (290305/290305) done
fatal: unexpected EOF)  
fatal: packfile '/tmp/wireless-2.6/.git/objects/pack/tmp-DuBFrs' SHA1 mismatch
error: git-fetch-pack: unable to read from git-index-pack
error: git-index-pack died with error code 128
fetch-pack from 
'git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git' 
failed.

I tried several times on two different machines and also (as you can see)
with different git versions.
Could it be a git bug?

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: wireless-2.6 git repos broken

2006-07-23 Thread Michael Buesch
On Sunday 23 July 2006 17:59, Larry Finger wrote:
> > Do you have the same problem on other git trees?
> > I saw some people running into this error when cloning Linus' 
> > linux-2.6.git. I couldn't reproduce it, using exactly the same git version.
> 
> I had the same error when pulling from Linus's tree. It was fixed with a 'git 
> fsck-objects --full' 
> command.

Uhm, that tells me the git tree on kernel.org is broken
and John has to run this command, right?

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] forcedeth: coding style cleanups (rev2)

2006-07-27 Thread Michael Buesch
On Thursday 27 July 2006 19:48, Stephen Hemminger wrote:
> --- sky2.orig/drivers/net/forcedeth.c 2006-07-13 12:53:48.0 -0700
> +++ sky2/drivers/net/forcedeth.c  2006-07-27 10:45:49.0 -0700
> @@ -381,21 +381,21 @@
>  
>  /* Big endian: should work, but is untested */
>  struct ring_desc {
> - u32 PacketBuffer;
> - u32 FlagLen;
> + u32 buf;
> + u32 flaglen;
>  };
>  
>  struct ring_desc_ex {
> - u32 PacketBufferHigh;
> - u32 PacketBufferLow;
> - u32 TxVlan;
> - u32 FlagLen;
> + u32 bufhigh;
> + u32 buflow;
> + u32 txvlan;
> + u32 flaglen;
>  };

Shouldn't these two structs be __attribute__((packed)) ?

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] forcedeth: coding style cleanups (rev2)

2006-07-27 Thread Michael Buesch
On Thursday 27 July 2006 21:24, Stephen Hemminger wrote:
> On Thu, 27 Jul 2006 21:21:14 +0200
> Michael Buesch <[EMAIL PROTECTED]> wrote:
> 
> > On Thursday 27 July 2006 19:48, Stephen Hemminger wrote:
> > > --- sky2.orig/drivers/net/forcedeth.c 2006-07-13 12:53:48.0 
> > > -0700
> > > +++ sky2/drivers/net/forcedeth.c  2006-07-27 10:45:49.0 -0700
> > > @@ -381,21 +381,21 @@
> > >  
> > >  /* Big endian: should work, but is untested */
> > >  struct ring_desc {
> > > - u32 PacketBuffer;
> > > - u32 FlagLen;
> > > + u32 buf;
> > > + u32 flaglen;
> > >  };
> > >  
> > >  struct ring_desc_ex {
> > > - u32 PacketBufferHigh;
> > > - u32 PacketBufferLow;
> > > - u32 TxVlan;
> > > - u32 FlagLen;
> > > + u32 bufhigh;
> > > + u32 buflow;
> > > + u32 txvlan;
> > > + u32 flaglen;
> > >  };
> > 
> > Shouldn't these two structs be __attribute__((packed)) ?
> > 
> 
> Not really necessary since the elements are all the same size.

Yes, sure. But I always ask myself if we really want to depend on this.
(There are other drivers also depending on this...)

I would say, if the device depends on the fields being
there without padding, we should tell the compiler.

> Plus, this was a style cleanup patch...

Sure. Was more a generic comment from me. ;)

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH wireless-dev] Use IRQF_SHARED instead of SA_SHIRQ

2006-08-05 Thread Michael Buesch
On Saturday 05 August 2006 09:57, Michael Wu wrote:
> Use IRQF_SHARED instead of SA_SHIRQ
> 
> This patch switches all instances of SA_SHIRQ to IRQF_SHARED for drivers in 
> the drivers/net/wireless/d80211 directory. According to 
> feature-removal-schedule.txt, the interrupt related SA_* flags are obsolete 
> and should be replaced by the appropriate IRQF_* version before January 2007.
> 
> Signed-off-by: Michael Wu <[EMAIL PROTECTED]>

Signed-off-by: Michael Buesch <[EMAIL PROTECTED]>

Are you also planning to do such a patch against softmac-bcm43xx? (Mainline)

> ---
> 
>  drivers/net/wireless/d80211/adm8211/adm8211.c  |2 +-
>  drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c |2 +-
>  drivers/net/wireless/d80211/rt2x00/rt2400pci.c |2 +-
>  drivers/net/wireless/d80211/rt2x00/rt2500pci.c |2 +-
>  drivers/net/wireless/d80211/rt2x00/rt61pci.c   |2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/wireless/d80211/adm8211/adm8211.c 
> b/drivers/net/wireless/d80211/adm8211/adm8211.c
> index 339b69c..dcabeab 100644
> --- a/drivers/net/wireless/d80211/adm8211/adm8211.c
> +++ b/drivers/net/wireless/d80211/adm8211/adm8211.c
> @@ -1627,7 +1627,7 @@ static int adm8211_open(struct net_devic
>   adm8211_rf_set_channel(dev, priv->channel);
>  
>   retval = request_irq(dev->irq, &adm8211_interrupt,
> -  SA_SHIRQ, dev->name, dev);
> +  IRQF_SHARED, dev->name, dev);
>   if (retval) {
>   printk(KERN_ERR "%s: failed to register IRQ handler\n",
>  dev->name);
> diff --git a/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c 
> b/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c
> index 59d137b..6366020 100644
> --- a/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c
> +++ b/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c
> @@ -2295,7 +2295,7 @@ #ifdef CONFIG_BCM947XX
>   }
>  #endif
>   err = request_irq(bcm->irq, bcm43xx_interrupt_handler,
> -   SA_SHIRQ, KBUILD_MODNAME, bcm);
> +   IRQF_SHARED, KBUILD_MODNAME, bcm);
>   if (err)
>   printk(KERN_ERR PFX "Cannot register IRQ%d\n", bcm->irq);
>  
> diff --git a/drivers/net/wireless/d80211/rt2x00/rt2400pci.c 
> b/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
> index 709cdbf..16e78b9 100644
> --- a/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
> +++ b/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
> @@ -1356,7 +1356,7 @@ static int rt2400pci_initialize(struct r
>* Register interrupt handler.
>*/
>   if (request_irq(rt2x00dev_pci(rt2x00dev)->irq, rt2400pci_interrupt,
> - SA_SHIRQ, net_dev->name, rt2x00dev)) {
> + IRQF_SHARED, net_dev->name, rt2x00dev)) {
>   ERROR("IRQ %d allocation failed.\n",
>   rt2x00dev_pci(rt2x00dev)->irq);
>   goto exit_fail;
> diff --git a/drivers/net/wireless/d80211/rt2x00/rt2500pci.c 
> b/drivers/net/wireless/d80211/rt2x00/rt2500pci.c
> index 8a22968..f79d386 100644
> --- a/drivers/net/wireless/d80211/rt2x00/rt2500pci.c
> +++ b/drivers/net/wireless/d80211/rt2x00/rt2500pci.c
> @@ -1476,7 +1476,7 @@ static int rt2500pci_initialize(struct r
>* Register interrupt handler.
>*/
>   if (request_irq(rt2x00dev_pci(rt2x00dev)->irq, rt2500pci_interrupt,
> - SA_SHIRQ, net_dev->name, rt2x00dev)) {
> + IRQF_SHARED, net_dev->name, rt2x00dev)) {
>   ERROR("IRQ %d allocation failed.\n",
>   rt2x00dev_pci(rt2x00dev)->irq);
>   goto exit_fail;
> diff --git a/drivers/net/wireless/d80211/rt2x00/rt61pci.c 
> b/drivers/net/wireless/d80211/rt2x00/rt61pci.c
> index 8bb9de9..b28e113 100644
> --- a/drivers/net/wireless/d80211/rt2x00/rt61pci.c
> +++ b/drivers/net/wireless/d80211/rt2x00/rt61pci.c
> @@ -1902,7 +1902,7 @@ static int rt61pci_initialize(struct rt2
>* Register interrupt handler.
>*/
>   if (request_irq(rt2x00dev_pci(rt2x00dev)->irq, rt61pci_interrupt,
> - SA_SHIRQ, net_dev->name, rt2x00dev)) {
> + IRQF_SHARED, net_dev->name, rt2x00dev)) {
>   ERROR("IRQ %d allocation failed.\n",
>   rt2x00dev_pci(rt2x00dev)->irq);
>   goto exit_fail;
> 

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH dscape] d80211: Switch d80211.h to IEEE80211_ style names

2006-08-05 Thread Michael Buesch
On Saturday 05 August 2006 13:46, Christoph Hellwig wrote:
> On Tue, Aug 01, 2006 at 08:21:49AM -0400, John W. Linville wrote:
> > I don't anticipate the d80211 naming conventions to ever make it out
> > of wireless-dev.  By the time we are ready to push that stuff upstream,
> > we will have cleaned-up our messes.
> 
> Yes, absolutely.
> 
> > This does raise the question: Should we start taking patches to
> > wireless-dev that migrate the current (i.e. ieee80211/softmac) stack
> > out of the kernel?  This would include (re-)moving the current stack
> > code, pointing non-migrated drivers (ipw2[12]00, zd1211rw) at the old
> > code, moving drivers out of drivers/net/wireless/d80211 up a level,
> > removing the softmac-based version of the bcm43xx driver, etc.

Yes, I personally would like to see that happen now.

> I think you first need to come up with a way to deal with ipw2[12]00,
> dealing with that kind of devices is priority number one before the
> devicescape stack can go anywhere.  Once that is handled all software
> MAC drivers in wireless-dev should be migrated to the device-scape based
> stack.

Well, I think we all agreed on what to do with ipw at the wireless summit.
As far as I remember all agreed to move the softmac-only drivers to their
own directory. Simply the reverse of what is currently in wireless-dev.
Currently we have d80211 drivers in their own dir.
After that move was done, we port softmac drivers over to d80211
and delete the softmac version once it's stabilized. If no softmac
driver is left, we delete the old ieee80211 subsystem.

> After that we're in the right position to deal with last big step of
> the merge:
> 
>  a) deal with the userspace interface issue.  Either rever to the single
> network device and eth%d name interface (at least for the non-AP case)
> so existing userspace won't know about the name underlying stack at
> all or alternatively go through the painfull design process for a new
> userspace interface.  I think the first variant will be a lot easier..

Not in the long term, IMHO.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH dscape] d80211: Switch d80211.h to IEEE80211_ style names

2006-08-05 Thread Michael Buesch
On Saturday 05 August 2006 19:30, Michael Buesch wrote:
> On Saturday 05 August 2006 13:46, Christoph Hellwig wrote:
> > On Tue, Aug 01, 2006 at 08:21:49AM -0400, John W. Linville wrote:
> > > I don't anticipate the d80211 naming conventions to ever make it out
> > > of wireless-dev.  By the time we are ready to push that stuff upstream,
> > > we will have cleaned-up our messes.
> > 
> > Yes, absolutely.
> > 
> > > This does raise the question: Should we start taking patches to
> > > wireless-dev that migrate the current (i.e. ieee80211/softmac) stack
> > > out of the kernel?  This would include (re-)moving the current stack
> > > code, pointing non-migrated drivers (ipw2[12]00, zd1211rw) at the old
> > > code, moving drivers out of drivers/net/wireless/d80211 up a level,
> > > removing the softmac-based version of the bcm43xx driver, etc.
> 
> Yes, I personally would like to see that happen now.

I forgot to say why ;)

It's getting a major pain to support two up-to-date bcm43xx drivers.
It's getting nontrivial to port drivers over from bcm43xx-softmac
to bcm43xx-d80211; even with tools like quilt and wiggle.

What I'd like to see is: After 2.6.18 got out, we freeze bcm43xx-softmac
and only allow small bugfixes to it. All development work would go
into the d80211 branch.
That saves me a _lot_ of time and pain.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC: -mm patch] bcm43xx_main.c: remove 3 functions

2006-08-08 Thread Michael Buesch
On Monday 07 August 2006 23:04, Adrian Bunk wrote:
> This patch removes three no longer used functions (that are even 
> generating gcc warnings).
> 
> This patch doesn't look right, but it is the result of 
> 58e5528ee464d38040b9489e10033c9387a10d56 in git-netdev...

Hm, can't find that commit in a tree.
I looked at linus', netdev-2.6.

But one thing is for sure. This patch is _wrong_. ;)

> Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]>

NACK.

>  drivers/net/wireless/bcm43xx/bcm43xx_main.c |   33 
>  1 file changed, 33 deletions(-)
> 
> --- linux-2.6.18-rc3-mm2-full/drivers/net/wireless/bcm43xx/bcm43xx_main.c.old 
> 2006-08-07 18:21:31.0 +0200
> +++ linux-2.6.18-rc3-mm2-full/drivers/net/wireless/bcm43xx/bcm43xx_main.c 
> 2006-08-07 18:23:36.0 +0200
> @@ -3194,39 +3194,6 @@
>   bcm43xx_clear_keys(bcm);
>  }
>  
> -static int bcm43xx_rng_read(struct hwrng *rng, u32 *data)
> -{
> - struct bcm43xx_private *bcm = (struct bcm43xx_private *)rng->priv;
> - unsigned long flags;
> -
> - spin_lock_irqsave(&(bcm)->irq_lock, flags);
> - *data = bcm43xx_read16(bcm, BCM43xx_MMIO_RNG);
> - spin_unlock_irqrestore(&(bcm)->irq_lock, flags);
> -
> - return (sizeof(u16));
> -}
> -
> -static void bcm43xx_rng_exit(struct bcm43xx_private *bcm)
> -{
> - hwrng_unregister(&bcm->rng);
> -}
> -
> -static int bcm43xx_rng_init(struct bcm43xx_private *bcm)
> -{
> - int err;
> -
> - snprintf(bcm->rng_name, ARRAY_SIZE(bcm->rng_name),
> -  "%s_%s", KBUILD_MODNAME, bcm->net_dev->name);
> - bcm->rng.name = bcm->rng_name;
> - bcm->rng.data_read = bcm43xx_rng_read;
> - bcm->rng.priv = (unsigned long)bcm;
> - err = hwrng_register(&bcm->rng);
> - if (err)
> - printk(KERN_ERR PFX "RNG init failed (%d)\n", err);
> -
> - return err;
> -}
> -
>  static int bcm43xx_shutdown_all_wireless_cores(struct bcm43xx_private *bcm)
>  {

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC: -mm patch] bcm43xx_main.c: remove 3 functions

2006-08-08 Thread Michael Buesch
On Tuesday 08 August 2006 21:42, you wrote:
> And it seems to be your fault.  ;-)

Uh, oh. I'm trapped.

> commit 58e5528ee464d38040b9489e10033c9387a10d56
> Author: Michael Buesch <[EMAIL PROTECTED]>
> Date:   Sat Jul 8 22:02:18 2006 +0200
> 
> [PATCH] bcm43xx: init routine rewrite

Ah, I guessed it.
This was caused by some merge-race ;)
Will send a fix for this, soon.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


d80211 merge plans

2006-08-08 Thread Michael Buesch
Hi,

I am wondering about d80211 mainline merge plans.

Short:
I would like to see a mainline merge of d80211 happening
as soon as possible (next feature merge window).

Long:
I, as one of the main maintainers of the bcm43xx driver, am
maintaining two different branches of the driver at the moment.
It's getting harder and harder, but it's still ok.
But: We are so --><-- close before another branch of the project
must happen. That would mean we would have to maintain four
different branches. I am certainly not going to do this.

One might wonder why we require yet another branch of the driver.
It's simple. Broadcom effectively branched their driver, too. The
new 4.x binary driver version has _many_ differences and does not
even support older chipsets. That makes it necessary for our open
source driver to branch, too. At least for several months. We don't
know, if we are going to branch forever, or if we are going to merge
them after some months when it's stabilized, yet.

To heavily reduce maintainance burden I would like to see d80211
going mainline as soon as possible, so that we can feature-freeze
the softmac port of bcm43xx and reduce maintainance to next to zero
there. So if people want freaky features, we simply point them
to the d80211 port, which would coexist in mainline.

d80211 in mainline would clearly marked as experimental, for sure.
Notes must be added for possibly breaking usermode interfaces.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] zd1211rw: cleanups

2006-08-13 Thread Michael Buesch
On Saturday 12 August 2006 19:00, Daniel Drake wrote:
> --- linux-2.6.orig/drivers/net/wireless/zd1211rw/zd_mac.c
> +++ linux-2.6/drivers/net/wireless/zd1211rw/zd_mac.c
> @@ -127,11 +127,9 @@ out:
>  
>  void zd_mac_clear(struct zd_mac *mac)
>  {
> - /* Aquire the lock. */
> - spin_lock(&mac->lock);
> - spin_unlock(&mac->lock);
>   zd_chip_clear(&mac->chip);
> - memset(mac, 0, sizeof(*mac));
> + ZD_ASSERT(!spin_is_locked(&mac->lock));

I think this assertion will always trigger on UP kernels
with spinlock debugging disabled.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] zd1211rw: Convert installer CDROM device into WLAN device

2006-08-13 Thread Michael Buesch
On Saturday 12 August 2006 18:59, Daniel Drake wrote:
> +static int eject_installer(struct usb_interface *intf)
> +{
> + struct usb_device *udev = interface_to_usbdev(intf);
> + struct usb_host_interface *iface_desc = &intf->altsetting[0];
> + struct usb_endpoint_descriptor *endpoint;
> + unsigned char *cmd;
> + u8 bulk_out_ep;
> + int r;
> +
> + /* Find bulk out endpoint */
> + endpoint = &iface_desc->endpoint[1].desc;
> + if ((endpoint->bEndpointAddress & USB_TYPE_MASK) == USB_DIR_OUT &&
> + (endpoint->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) ==
> + USB_ENDPOINT_XFER_BULK) {
> + bulk_out_ep = endpoint->bEndpointAddress;
> + } else {
> + dev_err(&udev->dev,
> + "zd1211rw: Could not find bulk out endpoint\n");
> + return -ENODEV;
> + }
> +
> + cmd = kzalloc(31, GFP_KERNEL);
> + if (cmd == NULL)
> + return -ENODEV;
> +
> + /* USB bulk command block */
> + cmd[0] = 0x55;  /* bulk command signature */
> + cmd[1] = 0x53;  /* bulk command signature */
> + cmd[2] = 0x42;  /* bulk command signature */
> + cmd[3] = 0x43;  /* bulk command signature */
> + cmd[14] = 6;/* command length */
> +
> + cmd[15] = 0x1b; /* SCSI command: START STOP UNIT */
> + cmd[19] = 0x2;  /* eject disc */
> +
> + dev_info(&udev->dev, "Ejecting virtual installer media...\n");
> + r = usb_bulk_msg(udev, usb_sndbulkpipe(udev, bulk_out_ep),
> + cmd, 31, NULL, 2000);

Does usb_bulk_msg kfree cmd, or are we leaking it here?

> + if (r)
> + return r;
> +
> + /* At this point, the device disconnects and reconnects with the real
> +  * ID numbers. */
> +
> + usb_set_intfdata(intf, NULL);
> + return 0;
> +}

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] zd1211rw: Convert installer CDROM device into WLAN device

2006-08-13 Thread Michael Buesch
On Sunday 13 August 2006 13:15, Daniel Drake wrote:
> Some devices identify themselves as a virtual USB CDROM drive. The virtual CD
> includes the windows driver. We aren't interested in this, so we eject the
> virtual CDROM and then the real wireless device appears.
> 
> Patch fixed over the earlier version to not leak cmd, thanks to Michael Buesch
> for spotting that.
> 
> Signed-off-by: Daniel Drake <[EMAIL PROTECTED]>

This one should be ok.

Acked-by: Michael Buesch <[EMAIL PROTECTED]>

> Index: linux-2.6/drivers/net/wireless/zd1211rw/zd_usb.c
> ===
> --- linux-2.6.orig/drivers/net/wireless/zd1211rw/zd_usb.c
> +++ linux-2.6/drivers/net/wireless/zd1211rw/zd_usb.c
> @@ -46,6 +46,8 @@ static struct usb_device_id usb_ids[] = 
>   { USB_DEVICE(0x0ace, 0x1215), .driver_info = DEVICE_ZD1211B },
>   { USB_DEVICE(0x157e, 0x300d), .driver_info = DEVICE_ZD1211B },
>   { USB_DEVICE(0x079b, 0x0062), .driver_info = DEVICE_ZD1211B },
> + /* "Driverless" devices that need ejecting */
> + { USB_DEVICE(0x0ace, 0x2011), .driver_info = DEVICE_INSTALLER },
>   {}
>  };
>  
> @@ -914,6 +916,55 @@ static void print_id(struct usb_device *
>  #define print_id(udev) do { } while (0)
>  #endif
>  
> +static int eject_installer(struct usb_interface *intf)
> +{
> + struct usb_device *udev = interface_to_usbdev(intf);
> + struct usb_host_interface *iface_desc = &intf->altsetting[0];
> + struct usb_endpoint_descriptor *endpoint;
> + unsigned char *cmd;
> + u8 bulk_out_ep;
> + int r;
> +
> + /* Find bulk out endpoint */
> + endpoint = &iface_desc->endpoint[1].desc;
> + if ((endpoint->bEndpointAddress & USB_TYPE_MASK) == USB_DIR_OUT &&
> + (endpoint->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) ==
> + USB_ENDPOINT_XFER_BULK) {
> + bulk_out_ep = endpoint->bEndpointAddress;
> + } else {
> + dev_err(&udev->dev,
> + "zd1211rw: Could not find bulk out endpoint\n");
> + return -ENODEV;
> + }
> +
> + cmd = kzalloc(31, GFP_KERNEL);
> + if (cmd == NULL)
> + return -ENODEV;
> +
> + /* USB bulk command block */
> + cmd[0] = 0x55;  /* bulk command signature */
> + cmd[1] = 0x53;  /* bulk command signature */
> + cmd[2] = 0x42;  /* bulk command signature */
> + cmd[3] = 0x43;  /* bulk command signature */
> + cmd[14] = 6;/* command length */
> +
> + cmd[15] = 0x1b; /* SCSI command: START STOP UNIT */
> + cmd[19] = 0x2;  /* eject disc */
> +
> + dev_info(&udev->dev, "Ejecting virtual installer media...\n");
> + r = usb_bulk_msg(udev, usb_sndbulkpipe(udev, bulk_out_ep),
> + cmd, 31, NULL, 2000);
> + kfree(cmd);
> + if (r)
> + return r;
> +
> + /* At this point, the device disconnects and reconnects with the real
> +  * ID numbers. */
> +
> + usb_set_intfdata(intf, NULL);
> + return 0;
> +}
> +
>  static int probe(struct usb_interface *intf, const struct usb_device_id *id)
>  {
>   int r;
> @@ -922,6 +973,9 @@ static int probe(struct usb_interface *i
>  
>   print_id(udev);
>  
> + if (id->driver_info & DEVICE_INSTALLER)
> + return eject_installer(intf);
> +
>   switch (udev->speed) {
>   case USB_SPEED_LOW:
>   case USB_SPEED_FULL:
> @@ -987,6 +1041,11 @@ static void disconnect(struct usb_interf
>   struct zd_mac *mac = zd_netdev_mac(netdev);
>   struct zd_usb *usb = &mac->chip.usb;
>  
> + /* Either something really bad happened, or we're just dealing with
> +  * a DEVICE_INSTALLER. */
> + if (netdev == NULL)
> + return;
> +
>   dev_dbg_f(zd_usb_dev(usb), "\n");
>  
>   zd_netdev_disconnect(netdev);
> Index: linux-2.6/drivers/net/wireless/zd1211rw/zd_usb.h
> ===
> --- linux-2.6.orig/drivers/net/wireless/zd1211rw/zd_usb.h
> +++ linux-2.6/drivers/net/wireless/zd1211rw/zd_usb.h
> @@ -30,6 +30,7 @@
>  enum devicetype {
>   DEVICE_ZD1211  = 0,
>   DEVICE_ZD1211B = 1,
> + DEVICE_INSTALLER = 2,
>  };
>  
>  enum endpoints {
> 

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Remove ugly TODO output from log files in bcm43xx-softmac

2006-08-14 Thread Michael Buesch
On Monday 14 August 2006 20:55, John W. Linville wrote:
> On Wed, Aug 09, 2006 at 04:13:25PM -0500, Larry Finger wrote:
> > This patch removes the ugly TODO output from the logs for bcm43xx-softmac. 
> > The
> > patch is for the latest version of Linville's wireless-2.6 tree.
> 
> I'm not sure if this is the right approach.  In fact I know it isn't --
> the right approach would be to implement the missing code! :-)
> 
> But barring that, wouldn't it be better to change the TODO() macro
> to key off a build-time definition like CONFIG_BCM43XX_DEBUG?
> 
> Michael, what do you think?

Heh, well.
Actually the TODO define should have died since months.
BUT: There are still 11 uses of it and they are _all_ valid uses.
_None_ of these should be removed without implementing the code
at the same time.

So, well. What about compiling it to a no-op if BCM43XX_DEBUG is
not defined. I think this is a bad idea.
These TODOs are there for a _very_ good reason. They say:
"If you see this in dmesg, feature foobar does not work at all".
They are a very good hint to the user (and escpecially to the
developers when dealing with "bugreports").

So: If someone would like that a TODO disappears in the logs,
go forward and implement the code. (Actually, most TODOs are
there because of incomplete specifications. But that might have
changed inbetween).

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: bcm43xx for d80211 softirq loop

2006-08-14 Thread Michael Buesch
On Monday 14 August 2006 10:10, Johannes Berg wrote:
> [23750.726463] NETDEV WATCHDOG: wmaster0: transmit timed out
> [23750.726482] wmaster0: resetting interface.
> [23750.726490] bcm43xx_d80211: Controller RESET (IEEE reset) ...
> [23750.753458] bcm43xx_d80211: select_wireless_core: cleanup
> [23750.753477] bcm43xx_d80211: Radio turned off
> [23750.753538] bcm43xx_d80211: DMA 0x0200 (RX) max used slots: 1/64
> [23750.755208] bcm43xx_d80211: DMA 0x0260 (TX) max used slots: 0/512
> [23750.755601] bcm43xx_d80211: DMA 0x0240 (TX) max used slots: 0/512
> [23750.755992] bcm43xx_d80211: DMA 0x0220 (TX) max used slots: 0/512
> [23750.756383] bcm43xx_d80211: DMA 0x0200 (TX) max used slots: 0/512
> [23750.957250] bcm43xx_d80211: Radio turned on
> [23751.129344] bcm43xx_d80211: Chip initialized
> [23751.137239] bcm43xx_d80211: DMA initialized
> [23751.137503] bcm43xx_d80211: Keys cleared
> [23751.153242] bcm43xx_d80211: Selected 802.11 core (phytype 2)
> [23751.153263] bcm43xx_d80211: Controller restarted
> 
> 
> after this, ksoftirq started running wild...
> 
> Since oprofile doesn't show any function ever called for a tasklet, I 
> started to investigate but couldn't find what was up. Then I removed the 
> bcm43xx_dscape module and the kernel blew up right away.
> 
> Then I patched the kernel with a runaway tasklet patch (see mail on 
> lkml) and got this (yeah, previous iteration of the patch where I forgot 
> a "\n"):
> 
> [15214.574151] NETDEV WATCHDOG: wmaster0: transmit timed out
> [15214.574166] wmaster0: resetting interface.
> [15214.574174] bcm43xx_d80211: Controller RESET (IEEE reset) ...
> [15214.591194] bcm43xx_d80211: select_wireless_core: cleanup
> [15214.591209] bcm43xx_d80211: Radio turned off
> [15214.591270] bcm43xx_d80211: DMA 0x0200 (RX) max used slots: 1/64
> [15214.592901] bcm43xx_d80211: DMA 0x0260 (TX) max used slots: 0/512
> [15214.593294] bcm43xx_d80211: DMA 0x0240 (TX) max used slots: 0/512
> [15214.593685] bcm43xx_d80211: DMA 0x0220 (TX) max used slots: 0/512
> [15214.594075] bcm43xx_d80211: DMA 0x0200 (TX) max used slots: 0/512
> [15214.773961] bcm43xx_d80211: Radio turned on
> [15214.945901] bcm43xx_d80211: Chip initialized
> [15214.953815] bcm43xx_d80211: DMA initialized
> [15214.954078] bcm43xx_d80211: Keys cleared
> [15214.969864] bcm43xx_d80211: Selected 802.11 core (phytype 2)
> [15214.969889] bcm43xx_d80211: Controller restarted
> [15215.019746] tasklet 
> drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c:4376 is scheduled but 
> hasn't been enabled for too long!
> <6>NETDEV WATCHDOG: wmaster0: transmit timed out
> [15769.964164] wmaster0: resetting interface.
> [15769.964175] bcm43xx_d80211: Controller RESET (IEEE reset) ...
> [15769.995145] bcm43xx_d80211: select_wireless_core: cleanup
> [15769.995166] bcm43xx_d80211: Radio turned off
> [15769.995249] bcm43xx_d80211: DMA 0x0200 (RX) max used slots: 0/64
> [15769.997222] bcm43xx_d80211: DMA 0x0260 (TX) max used slots: 0/512
> [15769.997615] bcm43xx_d80211: DMA 0x0240 (TX) max used slots: 0/512
> [15769.998006] bcm43xx_d80211: DMA 0x0220 (TX) max used slots: 0/512
> [15769.998397] bcm43xx_d80211: DMA 0x0200 (TX) max used slots: 0/512
> [15770.162013] bcm43xx_d80211: IRQ_READY timeout
> [15770.162059] bcm43xx_d80211: core_up for active 802.11 core failed (-19)
> [15770.162076] bcm43xx_d80211: Controller restart failed
> 
> 
> Hence, I'm able to point to bcm43xx now ;) Sorry I can't give a better 
> indication of what's up, I can try a different patch if you can come up 
> with a good way of debugging it. This usually seems to happen here after 
> an hour or two of not using the interface at all.

I saw this several times, too. I will start to debug this now.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] bcm43xx-d80211: Init, shutdown and restart fixes

2006-08-14 Thread Michael Buesch
Hi John,

Please apply this to wireless-dev.

--

This fixes various bugs in the init and shutdown code
that would lead to lockups and crashes.
This is best reproducable by receiving a timeout from
the netdev watchdog.

Signed-off-by: Michael Buesch <[EMAIL PROTECTED]>

Index: wireless-dev/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c
===
--- wireless-dev.orig/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c
2006-08-13 23:55:33.0 +0200
+++ wireless-dev/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c 
2006-08-14 18:19:14.0 +0200
@@ -593,6 +593,7 @@
return -EBUSY;
}
bcm43xx_interrupt_disable(bcm, BCM43xx_IRQ_ALL);
+   bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_MASK); /* flush */
spin_unlock_irqrestore(&bcm->irq_lock, flags);
bcm43xx_synchronize_irq(bcm);
 
@@ -2896,6 +2897,7 @@
/* Periodic work will take a long time, so we want it to
 * be preemtible.
 */
+   mutex_lock(&bcm->mutex);
netif_stop_queue(bcm->net_dev);
synchronize_net();
spin_lock_irqsave(&bcm->irq_lock, flags);
@@ -2904,7 +2906,6 @@
bcm43xx_pio_freeze_txqueues(bcm);
savedirqs = bcm43xx_interrupt_disable(bcm, BCM43xx_IRQ_ALL);
spin_unlock_irqrestore(&bcm->irq_lock, flags);
-   mutex_lock(&bcm->mutex);
bcm43xx_synchronize_irq(bcm);
} else {
/* Periodic work should take short time, so we want low
@@ -2918,13 +2919,11 @@
 
if (badness > BADNESS_LIMIT) {
spin_lock_irqsave(&bcm->irq_lock, flags);
-   if (likely(bcm43xx_status(bcm) == BCM43xx_STAT_INITIALIZED)) {
-   tasklet_enable(&bcm->isr_tasklet);
-   bcm43xx_interrupt_enable(bcm, savedirqs);
-   if (bcm43xx_using_pio(bcm))
-   bcm43xx_pio_thaw_txqueues(bcm);
-   bcm43xx_mac_enable(bcm);
-   }
+   tasklet_enable(&bcm->isr_tasklet);
+   bcm43xx_interrupt_enable(bcm, savedirqs);
+   if (bcm43xx_using_pio(bcm))
+   bcm43xx_pio_thaw_txqueues(bcm);
+   bcm43xx_mac_enable(bcm);
netif_wake_queue(bcm->net_dev);
}
mmiowb();
@@ -3622,13 +3621,16 @@
if (!active_core)
return -ESRCH; /* No such PHYTYPE on this board. */
 
+   /* Disable all network traffic. */
+   for (i = 0; i < bcm->ieee->queues; i++)
+   ieee80211_stop_queue(bcm->net_dev, i);
+   ieee80211_netif_oper(bcm->net_dev, NETIF_DETACH);
+
if (bcm->active_80211_core) {
/* We already selected a wl core in the past.
 * So first clean up everything.
 */
dprintk(KERN_INFO PFX "select_wireless_core: cleanup\n");
-   ieee80211_netif_oper(bcm->net_dev, NETIF_STOP);
-   ieee80211_netif_oper(bcm->net_dev, NETIF_DETACH);
bcm43xx_set_status(bcm, BCM43xx_STAT_INITIALIZED);
err = bcm43xx_disable_interrupts_sync(bcm);
assert(!err);
@@ -3700,10 +3702,8 @@
bcm43xx_security_init(bcm);
bcm43xx_measure_channel_change_time(bcm);
ieee80211_update_hw(bcm->net_dev, bcm->ieee);
-   ieee80211_start_queues(bcm->net_dev);
ieee80211_netif_oper(bcm->net_dev, NETIF_ATTACH);
-   ieee80211_netif_oper(bcm->net_dev, NETIF_START);
-   ieee80211_netif_oper(bcm->net_dev, NETIF_WAKE);
+   ieee80211_start_queues(bcm->net_dev);
 
/* Let's go! Be careful after enabling the IRQs.
 * Don't switch cores, for example.
@@ -3742,11 +3742,10 @@
err = bcm43xx_select_wireless_core(bcm, -1);
if (err)
goto err_crystal_off;
-
-   bcm43xx_periodic_tasks_setup(bcm);
err = bcm43xx_sysfs_register(bcm);
if (err)
goto err_wlshutdown;
+   bcm43xx_periodic_tasks_setup(bcm);
 out:
mutex_unlock(&bcm->mutex);
 
@@ -4028,9 +4027,13 @@
assert(0);
}
if (phy->type != new_phymode) {
+   spin_unlock_irqrestore(&bcm->irq_lock, flags);
+   bcm43xx_periodic_tasks_delete(bcm);
err = bcm43xx_select_wireless_core(bcm, new_phymode);
if (err)
-   goto out_unlock;
+   goto out_unlock_mutex;
+   bcm43xx_periodic_tasks_setup(bcm);
+   spin_lock_irqsave(&bcm->irq_lock, flags);
phy = bcm43xx_current_phy(bcm);
radio = bcm43xx_current_radio

[patch RFC 1/3] add Sonics Silicon Backplane module

2006-08-14 Thread Michael Buesch
Signed-off-by: Michael Buesch <[EMAIL PROTECTED]>

Index: wireless-dev/drivers/misc/Kconfig
===
--- wireless-dev.orig/drivers/misc/Kconfig  2006-08-14 15:31:50.0 
+0200
+++ wireless-dev/drivers/misc/Kconfig   2006-08-14 22:02:07.0 +0200
@@ -28,5 +28,16 @@
 
  If unsure, say N.
 
+config SONICS_SILICON_BACKPLANE
+   tristate "Sonics Silicon Backplane support"
+   depends on PCI && EXPERIMENTAL
+   default m
+   ---help---
+ The Sonics Silicon Backplane is used in some chips such
+ as the Broadcom 43xx and 44xx series.
+ The Backplane ties several chip-cores together.
+
+ If unsure, say M.
+
 endmenu
 
Index: wireless-dev/drivers/misc/Makefile
===
--- wireless-dev.orig/drivers/misc/Makefile 2006-08-14 15:31:50.0 
+0200
+++ wireless-dev/drivers/misc/Makefile  2006-08-14 22:02:07.0 +0200
@@ -3,5 +3,6 @@
 #
 obj- := misc.o # Dummy rule to force built-in.o to be made
 
-obj-$(CONFIG_IBM_ASM)  += ibmasm/
-obj-$(CONFIG_HDPU_FEATURES)+= hdpuftrs/
+obj-$(CONFIG_IBM_ASM)  += ibmasm/
+obj-$(CONFIG_HDPU_FEATURES)+= hdpuftrs/
+obj-$(CONFIG_SONICS_SILICON_BACKPLANE) += ssb.o
Index: wireless-dev/drivers/misc/ssb.c
===
--- /dev/null   1970-01-01 00:00:00.0 +
+++ wireless-dev/drivers/misc/ssb.c 2006-08-14 23:19:34.0 +0200
@@ -0,0 +1,502 @@
+/*
+ * Sonics Silicon Backplane backend.
+ *
+ * Copyright (C) 2005-2006 Michael Buesch <[EMAIL PROTECTED]>
+ * Copyright (C) 2005 Martin Langer <[EMAIL PROTECTED]>
+ * Copyright (C) 2005 Stefano Brivio <[EMAIL PROTECTED]>
+ * Copyright (C) 2005 Danny van Dyk <[EMAIL PROTECTED]>
+ * Copyright (C) 2005 Andreas Jaggi <[EMAIL PROTECTED]>
+ *
+ * Derived from the Broadcom 4400 device driver.
+ * Copyright (C) 2002 David S. Miller (davem@redhat.com)
+ * Fixed by Pekka Pietikainen ([EMAIL PROTECTED])
+ * Copyright (C) 2006 Broadcom Corporation.
+ *
+ * Licensed under the GNU/GPL. See COPYING for details.
+ */
+
+#include 
+#include 
+#include 
+
+#define SSB_DEBUG  1
+#define PFX"ssb: "
+
+#if SSB_DEBUG
+# define dprintk(f, x...)  do { printk(f ,##x); } while (0)
+#else
+# define dprintk(f, x...)  do { /* nothing */ } while (0)
+#endif
+
+static inline int ssb_pci_read_config32(struct ssb *ssb, int offset,
+   u32 *value)
+{
+   return pci_read_config_dword(ssb->pci_dev, offset, value);
+}
+
+static inline int ssb_pci_read_config16(struct ssb *ssb, int offset,
+   u16 *value)
+{
+   return pci_read_config_word(ssb->pci_dev, offset, value);
+}
+
+static inline int ssb_pci_write_config32(struct ssb *ssb, int offset,
+u32 value)
+{
+   return pci_write_config_dword(ssb->pci_dev, offset, value);
+}
+
+static inline u32 ssb_read32(struct ssb *ssb, u16 offset)
+{
+   return ioread32(ssb->mmio + offset + ssb_core_offset(ssb));
+}
+
+static inline void ssb_write32(struct ssb *ssb, u16 offset,
+  u32 value)
+{
+   iowrite32(value, ssb->mmio + offset + ssb_core_offset(ssb));
+}
+
+
+struct ssb * ssb_alloc(struct pci_dev *pci_dev,
+  void __iomem *mmio)
+{
+   struct ssb *ssb;
+
+   ssb = kzalloc(sizeof(*ssb), GFP_KERNEL);
+   if (!ssb)
+   return NULL;
+
+   ssb->pci_dev = pci_dev;
+   ssb->mmio = mmio;
+
+   return ssb;
+}
+EXPORT_SYMBOL(ssb_alloc);
+
+void ssb_free(struct ssb *ssb)
+{
+   kfree(ssb->cores);
+   kfree(ssb);
+}
+EXPORT_SYMBOL(ssb_free);
+
+static int do_switch_core(struct ssb *ssb, u8 coreidx)
+{
+   int err;
+   int attempts = 0;
+   u32 cur_core;
+
+   while (1) {
+   err = ssb_pci_write_config32(ssb, SSB_BAR0_WIN,
+(coreidx * 0x1000) + 0x1800);
+   if (unlikely(err))
+   goto error;
+   err = ssb_pci_read_config32(ssb, SSB_BAR0_WIN,
+   &cur_core);
+   if (unlikely(err))
+   goto error;
+   cur_core = (cur_core - 0x1800) / 0x1000;
+   if (cur_core == coreidx)
+   break;
+
+   if (unlikely(attempts++ > SSB_BAR0_MAX_RETRIES))
+   goto error;
+   udelay(10);
+   }
+#ifdef CONFIG_BCM947XX
+   ssb->current_core_offset = 0;
+   if (ssb->pci_dev->bus->number == 0)
+   ssb->current_core_offset = 0x1000 * coreidx;
+#endif /* CONFIG_BCM947XX */
+
+   return 0;
+error:
+   printk(KERN_ERR PF

[patch RFC 0/3] Sonics Silicon Backplane module

2006-08-14 Thread Michael Buesch
Hi.

This patch series creates a new Sonics Silicon Backplane
driver and converts the bcm43xx and b44 drivers to use it.
The bcm43xx patch is tested and it works. But the b44
patch is only compile tested, as I don't have the
hardware (yet?).

These patches apply against wireless-dev.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[patch RFC 3/3] b44: convert to ssb module

2006-08-14 Thread Michael Buesch
This is not runtime tested.

Signed-off-by: Michael Buesch <[EMAIL PROTECTED]>

Index: wireless-dev/drivers/net/Kconfig
===
--- wireless-dev.orig/drivers/net/Kconfig   2006-08-09 16:52:40.0 
+0200
+++ wireless-dev/drivers/net/Kconfig2006-08-14 23:29:25.0 +0200
@@ -1388,7 +1388,7 @@
 
 config B44
tristate "Broadcom 4400 ethernet support"
-   depends on NET_PCI && PCI
+   depends on NET_PCI && PCI && SONICS_SILICON_BACKPLANE
select MII
help
  If you have a network (Ethernet) controller of this type, say Y and
Index: wireless-dev/drivers/net/b44.c
===
--- wireless-dev.orig/drivers/net/b44.c 2006-08-09 16:52:40.0 +0200
+++ wireless-dev/drivers/net/b44.c  2006-08-15 00:06:58.0 +0200
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -177,85 +178,6 @@
return 0;
 }
 
-/* Sonics SiliconBackplane support routines.  ROFL, you should see all the
- * buzz words used on this company's website :-)
- *
- * All of these routines must be invoked with bp->lock held and
- * interrupts disabled.
- */
-
-#define SB_PCI_DMA 0x4000  /* Client Mode PCI memory 
access space (1 GB) */
-#define BCM4400_PCI_CORE_ADDR  0x18002000  /* Address of PCI core on 
BCM4400 cards */
-
-static u32 ssb_get_core_rev(struct b44 *bp)
-{
-   return (br32(bp, B44_SBIDHIGH) & SBIDHIGH_RC_MASK);
-}
-
-static u32 ssb_pci_setup(struct b44 *bp, u32 cores)
-{
-   u32 bar_orig, pci_rev, val;
-
-   pci_read_config_dword(bp->pdev, SSB_BAR0_WIN, &bar_orig);
-   pci_write_config_dword(bp->pdev, SSB_BAR0_WIN, BCM4400_PCI_CORE_ADDR);
-   pci_rev = ssb_get_core_rev(bp);
-
-   val = br32(bp, B44_SBINTVEC);
-   val |= cores;
-   bw32(bp, B44_SBINTVEC, val);
-
-   val = br32(bp, SSB_PCI_TRANS_2);
-   val |= SSB_PCI_PREF | SSB_PCI_BURST;
-   bw32(bp, SSB_PCI_TRANS_2, val);
-
-   pci_write_config_dword(bp->pdev, SSB_BAR0_WIN, bar_orig);
-
-   return pci_rev;
-}
-
-static void ssb_core_disable(struct b44 *bp)
-{
-   if (br32(bp, B44_SBTMSLOW) & SBTMSLOW_RESET)
-   return;
-
-   bw32(bp, B44_SBTMSLOW, (SBTMSLOW_REJECT | SBTMSLOW_CLOCK));
-   b44_wait_bit(bp, B44_SBTMSLOW, SBTMSLOW_REJECT, 10, 0);
-   b44_wait_bit(bp, B44_SBTMSHIGH, SBTMSHIGH_BUSY, 10, 1);
-   bw32(bp, B44_SBTMSLOW, (SBTMSLOW_FGC | SBTMSLOW_CLOCK |
-   SBTMSLOW_REJECT | SBTMSLOW_RESET));
-   br32(bp, B44_SBTMSLOW);
-   udelay(1);
-   bw32(bp, B44_SBTMSLOW, (SBTMSLOW_REJECT | SBTMSLOW_RESET));
-   br32(bp, B44_SBTMSLOW);
-   udelay(1);
-}
-
-static void ssb_core_reset(struct b44 *bp)
-{
-   u32 val;
-
-   ssb_core_disable(bp);
-   bw32(bp, B44_SBTMSLOW, (SBTMSLOW_RESET | SBTMSLOW_CLOCK | 
SBTMSLOW_FGC));
-   br32(bp, B44_SBTMSLOW);
-   udelay(1);
-
-   /* Clear SERR if set, this is a hw bug workaround.  */
-   if (br32(bp, B44_SBTMSHIGH) & SBTMSHIGH_SERR)
-   bw32(bp, B44_SBTMSHIGH, 0);
-
-   val = br32(bp, B44_SBIMSTATE);
-   if (val & (SBIMSTATE_IBE | SBIMSTATE_TO))
-   bw32(bp, B44_SBIMSTATE, val & ~(SBIMSTATE_IBE | SBIMSTATE_TO));
-
-   bw32(bp, B44_SBTMSLOW, (SBTMSLOW_CLOCK | SBTMSLOW_FGC));
-   br32(bp, B44_SBTMSLOW);
-   udelay(1);
-
-   bw32(bp, B44_SBTMSLOW, (SBTMSLOW_CLOCK));
-   br32(bp, B44_SBTMSLOW);
-   udelay(1);
-}
-
 static int ssb_core_unit(struct b44 *bp)
 {
 #if 0
@@ -281,12 +203,6 @@
return 0;
 }
 
-static int ssb_is_core_up(struct b44 *bp)
-{
-   return ((br32(bp, B44_SBTMSLOW) & (SBTMSLOW_RESET | SBTMSLOW_REJECT | 
SBTMSLOW_CLOCK))
-   == SBTMSLOW_CLOCK);
-}
-
 static void __b44_cam_write(struct b44 *bp, unsigned char *data, int index)
 {
u32 val;
@@ -1276,9 +1192,12 @@
 }
 
 /* bp->lock is held. */
-static void b44_chip_reset(struct b44 *bp)
+static int b44_chip_reset(struct b44 *bp)
 {
-   if (ssb_is_core_up(bp)) {
+   struct ssb *ssb = bp->ssb;
+   int err = 0;
+
+   if (ssb_core_is_enabled(ssb)) {
bw32(bp, B44_RCV_LAZY, 0);
bw32(bp, B44_ENET_CTRL, ENET_CTRL_DISABLE);
b44_wait_bit(bp, B44_ENET_CTRL, ENET_CTRL_DISABLE, 100, 1);
@@ -1291,12 +1210,13 @@
bw32(bp, B44_DMARX_CTRL, 0);
bp->rx_prod = bp->rx_cons = 0;
} else {
-   ssb_pci_setup(bp, (bp->core_unit == 0 ?
-  SBINTVEC_ENET0 :
-  SBINTVEC_ENET1));
+   err = ssb_cores_connect(ssb, 0x1);
+   if (err)
+   goto out;
+   err = ssb_switch_core(ssb, &(ssb->cores[

[patch RFC 2/3] bcm43xx-d80211: convert to ssb module

2006-08-14 Thread Michael Buesch
More #defines in bcm43xx.h can be deleted and the corresponding
#defines in ssh.h can be used instead. But I leave this alone for now...

Signed-off-by: Michael Buesch <[EMAIL PROTECTED]>

Index: wireless-dev/drivers/net/wireless/d80211/bcm43xx/bcm43xx.h
===
--- wireless-dev.orig/drivers/net/wireless/d80211/bcm43xx/bcm43xx.h 
2006-08-14 22:05:06.0 +0200
+++ wireless-dev/drivers/net/wireless/d80211/bcm43xx/bcm43xx.h  2006-08-14 
23:19:39.0 +0200
@@ -11,6 +11,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -186,25 +187,7 @@
 #define BCM43xx_PCTL_FORCE_PLL 0x1000
 #define BCM43xx_PCTL_DYN_XTAL  0x2000
 
-/* COREIDs */
-#define BCM43xx_COREID_CHIPCOMMON  0x800
-#define BCM43xx_COREID_ILINE20  0x801
-#define BCM43xx_COREID_SDRAM0x803
-#define BCM43xx_COREID_PCI 0x804
-#define BCM43xx_COREID_MIPS 0x805
-#define BCM43xx_COREID_ETHERNET 0x806
-#define BCM43xx_COREID_V90 0x807
-#define BCM43xx_COREID_USB11_HOSTDEV0x80a
-#define BCM43xx_COREID_IPSEC0x80b
-#define BCM43xx_COREID_PCMCIA  0x80d
-#define BCM43xx_COREID_EXT_IF   0x80f
-#define BCM43xx_COREID_80211   0x812
-#define BCM43xx_COREID_MIPS_33020x816
-#define BCM43xx_COREID_USB11_HOST   0x817
-#define BCM43xx_COREID_USB11_DEV0x818
-#define BCM43xx_COREID_USB20_HOST   0x819
-#define BCM43xx_COREID_USB20_DEV0x81a
-#define BCM43xx_COREID_SDIO_HOST0x81b
+// FIXME: deletes SSB duplicates.
 
 /* Core Information Registers */
 #define BCM43xx_CIR_BASE   0xf00
@@ -578,29 +561,9 @@
 
 #define BCM43xx_MAX_80211_CORES2
 
-#ifdef CONFIG_BCM947XX
-#define core_offset(bcm) (bcm)->current_core_offset
-#else
-#define core_offset(bcm) 0
-#endif
-
-/* Generic information about a core. */
-struct bcm43xx_coreinfo {
-   u8 available:1,
-  enabled:1,
-  initialized:1;
-   /** core_rev revision number */
-   u8 rev;
-   /** Index number for _switch_core() */
-   u8 index;
-   /** core_id ID number */
-   u16 id;
-   /** Core-specific data. */
-   void *priv;
-};
-
 /* Additional information for each 80211 core. */
-struct bcm43xx_coreinfo_80211 {
+struct bcm43xx_corepriv_80211 {
+   u8 initialized:1;
/* PHY device. */
struct bcm43xx_phyinfo phy;
/* Radio device. */
@@ -615,7 +578,7 @@
 
 /* Context information for a noise calculation (Link Quality). */
 struct bcm43xx_noise_calculation {
-   struct bcm43xx_coreinfo *core_at_start;
+   struct ssb_core *core_at_start;
u8 channel_at_start;
u8 calculation_running:1;
u8 nr_samples;
@@ -685,6 +648,8 @@
  */
 
 struct bcm43xx_private {
+   /* Pointer to the Sonics Silicon Backplane. */
+   struct ssb *ssb;
struct ieee80211_hw *ieee;
struct ieee80211_low_level_stats ieee_stats;
 
@@ -735,27 +700,19 @@
struct bcm43xx_led leds[BCM43xx_NR_LEDS];
spinlock_t leds_lock;
 
-   /* The currently active core. */
-   struct bcm43xx_coreinfo *current_core;
-#ifdef CONFIG_BCM947XX
-   /** current core memory offset */
-   u32 current_core_offset;
-#endif
-   struct bcm43xx_coreinfo *active_80211_core;
-   /* coreinfo structs for all possible cores follow.
-* Note that a core might not exist.
-* So check the coreinfo flags before using it.
+   /* Pointers to the available cores.
+* If a core is not available, this is NULL.
 */
-   struct bcm43xx_coreinfo core_chipcommon;
-   struct bcm43xx_coreinfo core_pci;
-   struct bcm43xx_coreinfo core_80211[ BCM43xx_MAX_80211_CORES ];
+   struct ssb_core *core_80211[ BCM43xx_MAX_80211_CORES ];
+   struct ssb_core *active_80211_core;//FIXME remove?
+   struct ssb_core *core_chipcommon;
+   struct ssb_core *core_pci;
+
/* Additional information, specific to the 80211 cores. */
-   struct bcm43xx_coreinfo_80211 core_80211_ext[ BCM43xx_MAX_80211_CORES ];
+   struct bcm43xx_corepriv_80211 corepriv_80211[ BCM43xx_MAX_80211_CORES ];
/* Number of available 80211 cores. */
int nr_80211_available;
 
-   u32 chipcommon_capabilities;
-
/* Reason code of the last interrupt. */
u32 irq_reason;
u32 dma_reason[4];
@@ -829,11 +786,11 @@
  * any of these functions.
  */
 static inline
-struct bcm43xx_coreinfo_80211 *
+struct bcm43xx_corepriv_80211 *
 bcm43xx_current_80211_priv(struct bcm43xx_private *bcm)
 {
-   assert(bcm->current_core->id == BCM43xx_COREID_80211);
-   return bcm->current_core->priv;
+   assert(bcm->ssb->current_core->cc == SSB_CC_80211);
+   return bcm->ssb->current_core->priv;
 }
 static inline
 struct bcm43xx_pio * bcm43xx_current_pio(struct bcm43xx_private *bc

Re: [patch RFC 3/3] b44: convert to ssb module

2006-08-15 Thread Michael Buesch
On Tuesday 15 August 2006 14:12, John W. Linville wrote:
> On Tue, Aug 15, 2006 at 12:18:39AM +0200, Michael Buesch wrote:
> 
> > @@ -2093,13 +2015,13 @@
> > bp->imask = IMASK_DEF;
> >  
> > bp->core_unit = ssb_core_unit(bp);
> > -   bp->dma_offset = SB_PCI_DMA;
> > +   bp->dma_offset = 0x4000; /* Client Mode PCI memory access space 
> > (1GB) */
> 
> Probably shouldn't change anything to a magic number...

Well, SB_PCI_DMA wasn't really a useful name, too IMHO :)
But you're right. We should use a define. And I know that.
This is only a patch for testing if it actually works with the
ssb module (I can not test this).

Besides that, we should probably get rid of dma_offset completely,
as it is not really an offset, but a routing bit.

You can take a look at the new DMA engine code of bcm43xx.
It's the same engine.

BTW: John, I see that the new DMA engine code is not applied
to wireless-dev (64-bit DMA and >1G support). But I think I
already sent the patch. Where did it get lost? Was it my fault?

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch RFC 3/3] b44: convert to ssb module

2006-08-15 Thread Michael Buesch
On Tuesday 15 August 2006 16:33, John W. Linville wrote:
> On Tue, Aug 15, 2006 at 04:14:16PM +0200, Michael Buesch wrote:
> 
> > You can take a look at the new DMA engine code of bcm43xx.
> > It's the same engine.
> > 
> > BTW: John, I see that the new DMA engine code is not applied
> > to wireless-dev (64-bit DMA and >1G support). But I think I
> > already sent the patch. Where did it get lost? Was it my fault?
> 
> http://marc.theaimsgroup.com/?l=linux-netdev&m=115056542006772&w=2

Hm, I _think_ that was the right one.
But I will dig out the right one and resend to make sure things don't
randomly break.

> Is this the patch?  I think this one is a victim of the "RFT" subject.
> Oh, and the patch seemed to not be totally working based on this post:
> 
>   http://marc.theaimsgroup.com/?l=linux-netdev&m=115075858220477&w=2

I think this oops comes from buggy (or too new) firmware.
I think we can ignore it for now.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] bcm43xx-d80211: Init, shutdown and restart fixes

2006-08-15 Thread Michael Buesch
On Tuesday 15 August 2006 17:23, Larry Finger wrote:
> Michael,
> 
> Michael Buesch wrote:
> > Hi John,
> > 
> > Please apply this to wireless-dev.
> > 
> > --
> > 
> > This fixes various bugs in the init and shutdown code
> > that would lead to lockups and crashes.
> > This is best reproducable by receiving a timeout from
> > the netdev watchdog.
> 
> Is a similar bug likely to be the cause of the netdev watchdog timeouts in 
> bcm43xx-softmac? If so, will there be a patch for wireless-2.6?

I don't know why the _timeouts_ itself happen, but maybe the
lockup and crash bugs are in bcm43xx-softmac, too.
I think it should be actually trivial to port this to bcm43xx-softmac.
Care to do this, Larry?

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: proposal for new wireless configuration API

2006-08-15 Thread Michael Buesch
On Tuesday 15 August 2006 18:29, Dan Williams wrote:
> o Separate attributes for channel and frequency

No, channel and freq is the same. It's just another name
for the same child. I would say we only want to deal with channel numbers
in the API. That's much easier, as we don't have to deal with this
fixed-point (or even floating point) mess. Look at WE for the
fixed-point mess.
The userspace tools can easily convert freq to channel and back.
And in the kernel, we can easily provide some small function
to convert from channel to khz and back, for example. But I would
like to see the fixed-point stuff in the API vanish.

> o Method of finding out channel <-> frequency mapping (not all drivers
> support this in the SIOCGIWRANGE handler now)

Yes, that would be a good idea.
Comes next to the conversion function (see above).

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: proposal for new wireless configuration API

2006-08-15 Thread Michael Buesch
On Tuesday 15 August 2006 20:14, Dan Williams wrote:
> On Tue, 2006-08-15 at 18:38 +0200, Michael Buesch wrote:
> > On Tuesday 15 August 2006 18:29, Dan Williams wrote:
> > > o Separate attributes for channel and frequency
> > 
> > No, channel and freq is the same. It's just another name
> > for the same child. I would say we only want to deal with channel numbers
> > in the API. That's much easier, as we don't have to deal with this
> > fixed-point (or even floating point) mess. Look at WE for the
> > fixed-point mess.
> 
> Right, I don't have a problem with only using one or the other; but we
> _HAVE_ to provide a function in the driver that allows userspace
> programs to convert channel <-> frequency both ways, like you suggest
> below.  Obviously the channel/frequency mapping isn't the same
> everywhere.
> 
> [ or is it?  I'd be very uncomfortable using the same channel #s
> everywhere unless some IEEE spec states exactly what the channel #s are
> for every frequency range that wireless stuff operates in ]

The channel<->freq mapping is stable.
What is unstable is the allowed channels map.
For APHY there are even many gaps in the map. For example
channels 6, 40 and 55 may be the only ones allowed

(6, 40, 55 are generated by my /brain/random device ;) )

The frequency range you mention is selected differently by
another config option. In d80211 we call it the PHYMODE.
Switching the PHYMODE changes the meaning of channel values
and changes the output of the two functions below.

So if I want to switch to channel 4 in the 2.4Ghz band, I would do:
select PHYMODE B or G,
select channel 4.

That would lead to the card tuning to 2427 MHz.

> > The userspace tools can easily convert freq to channel and back.
> > And in the kernel, we can easily provide some small function
> > to convert from channel to khz and back, for example. But I would
> > like to see the fixed-point stuff in the API vanish.
> 
> No argument here; as long as we provide the mapping function in the
> driver for each card.

Hm, I don't know if I understand this correctly.
You want to implement the mapping function in every driver
or in the d80211 stack?
It belongs into a central place in the 80211 stack. There where
regulatory domain stuff is managed, too.


We need functions like:

u32 ieee80211_channel_to_freq(struct ieee80211 *ieee, u8 channel);
u8 ieee80211_freq_to_channel(struct ieee80211 *ieee, u32 channel);

These would be used in drivers to convert values.
The validity of the channel value passed from userspace would be
checked in the _stack_. If the user passes an illegal value for
his country, it would never reach the driver.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: proposal for new wireless configuration API

2006-08-15 Thread Michael Buesch
On Tuesday 15 August 2006 21:27, Simon Barber wrote:
> A further complication happens in Japan with 802.11j, and now in the USA
> too - with 802.11y in the 3.65Ghz band - here there are some new channel
> widths that are possible. Normally 802.11 is 20 or 22Mhz wide (20Mhz for
> OFDM modulations - 11a/g, 22 for 11b). In Japan's 4.9Ghz band you can
> run the OFDM at half rate, giving a 10Mhz wide channel, or at quarter
> rate, giving a 5Mhz wide channel. Hence same frequency, different
> channel spec. Using a channel number is the way to go. If we need
> something to convert between the 2 it should probably be a library in
> user space (in hostapd or wpa_supplicant) - hostapd does have this
> today.
>
> It might be nice if other applications could access this data too - but
> I don't think it needs to be inside the kernel.

We need this conversion function, as most devices tune to frequencies,
not channels. So when a driver is instructed to tune to channel 2,
it must call back into the 80211 stack to ask for the frequency (based
on the current PHYMODE and the other parameters you mentioned above).
That call should IMO not result in a call to userspace. Userspace
should instead set flags _before_ in the stack and the conversion
callback would act on these flags.
That way userspace only has to tell the kernel once which frequency-band,
half, quater freq, or whatever it wants. The actual conversion
from channel number to freq (or the other way around) is trivial after
that, as it's only a few ifs and elses based on some cheap flags.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] bcm43xx: >1G and 64bit DMA support

2006-08-15 Thread Michael Buesch
Hi John,

Please apply this to wireless-2.6.

--

This is a rewrite of the bcm43xx DMA engine. It adds support
for >1G of memory (for chips that support the extension bits)
and 64-bit DMA (for chips that support it).

Signed-off-by: Michael Buesch <[EMAIL PROTECTED]>

Index: wireless-2.6/drivers/net/wireless/bcm43xx/bcm43xx.h
===
--- wireless-2.6.orig/drivers/net/wireless/bcm43xx/bcm43xx.h2006-08-15 
22:31:01.0 +0200
+++ wireless-2.6/drivers/net/wireless/bcm43xx/bcm43xx.h 2006-08-15 
23:25:55.0 +0200
@@ -33,14 +33,18 @@
 #define BCM43xx_PCICFG_ICR 0x94
 
 /* MMIO offsets */
-#define BCM43xx_MMIO_DMA1_REASON   0x20
-#define BCM43xx_MMIO_DMA1_IRQ_MASK 0x24
-#define BCM43xx_MMIO_DMA2_REASON   0x28
-#define BCM43xx_MMIO_DMA2_IRQ_MASK 0x2C
-#define BCM43xx_MMIO_DMA3_REASON   0x30
-#define BCM43xx_MMIO_DMA3_IRQ_MASK 0x34
-#define BCM43xx_MMIO_DMA4_REASON   0x38
-#define BCM43xx_MMIO_DMA4_IRQ_MASK 0x3C
+#define BCM43xx_MMIO_DMA0_REASON   0x20
+#define BCM43xx_MMIO_DMA0_IRQ_MASK 0x24
+#define BCM43xx_MMIO_DMA1_REASON   0x28
+#define BCM43xx_MMIO_DMA1_IRQ_MASK 0x2C
+#define BCM43xx_MMIO_DMA2_REASON   0x30
+#define BCM43xx_MMIO_DMA2_IRQ_MASK 0x34
+#define BCM43xx_MMIO_DMA3_REASON   0x38
+#define BCM43xx_MMIO_DMA3_IRQ_MASK 0x3C
+#define BCM43xx_MMIO_DMA4_REASON   0x40
+#define BCM43xx_MMIO_DMA4_IRQ_MASK 0x44
+#define BCM43xx_MMIO_DMA5_REASON   0x48
+#define BCM43xx_MMIO_DMA5_IRQ_MASK 0x4C
 #define BCM43xx_MMIO_STATUS_BITFIELD   0x120
 #define BCM43xx_MMIO_STATUS2_BITFIELD  0x124
 #define BCM43xx_MMIO_GEN_IRQ_REASON0x128
@@ -56,14 +60,27 @@
 #define BCM43xx_MMIO_XMITSTAT_10x174
 #define BCM43xx_MMIO_REV3PLUS_TSF_LOW  0x180 /* core rev >= 3 only */
 #define BCM43xx_MMIO_REV3PLUS_TSF_HIGH 0x184 /* core rev >= 3 only */
-#define BCM43xx_MMIO_DMA1_BASE 0x200
-#define BCM43xx_MMIO_DMA2_BASE 0x220
-#define BCM43xx_MMIO_DMA3_BASE 0x240
-#define BCM43xx_MMIO_DMA4_BASE 0x260
+
+/* 32-bit DMA */
+#define BCM43xx_MMIO_DMA32_BASE0   0x200
+#define BCM43xx_MMIO_DMA32_BASE1   0x220
+#define BCM43xx_MMIO_DMA32_BASE2   0x240
+#define BCM43xx_MMIO_DMA32_BASE3   0x260
+#define BCM43xx_MMIO_DMA32_BASE4   0x280
+#define BCM43xx_MMIO_DMA32_BASE5   0x2A0
+/* 64-bit DMA */
+#define BCM43xx_MMIO_DMA64_BASE0   0x200
+#define BCM43xx_MMIO_DMA64_BASE1   0x240
+#define BCM43xx_MMIO_DMA64_BASE2   0x280
+#define BCM43xx_MMIO_DMA64_BASE3   0x2C0
+#define BCM43xx_MMIO_DMA64_BASE4   0x300
+#define BCM43xx_MMIO_DMA64_BASE5   0x340
+/* PIO */
 #define BCM43xx_MMIO_PIO1_BASE 0x300
 #define BCM43xx_MMIO_PIO2_BASE 0x310
 #define BCM43xx_MMIO_PIO3_BASE 0x320
 #define BCM43xx_MMIO_PIO4_BASE 0x330
+
 #define BCM43xx_MMIO_PHY_VER   0x3E0
 #define BCM43xx_MMIO_PHY_RADIO 0x3E2
 #define BCM43xx_MMIO_ANTENNA   0x3E8
@@ -233,8 +250,14 @@
 #define BCM43xx_SBTMSTATELOW_FORCE_GATE_CLOCK  0x2
 
 /* sbtmstatehigh state flags */
-#define BCM43xx_SBTMSTATEHIGH_SERROR   0x1
-#define BCM43xx_SBTMSTATEHIGH_BUSY 0x4
+#define BCM43xx_SBTMSTATEHIGH_SERROR   0x0001
+#define BCM43xx_SBTMSTATEHIGH_BUSY 0x0004
+#define BCM43xx_SBTMSTATEHIGH_TIMEOUT  0x0020
+#define BCM43xx_SBTMSTATEHIGH_COREFLAGS0x1FFF
+#define BCM43xx_SBTMSTATEHIGH_DMA64BIT 0x1000
+#define BCM43xx_SBTMSTATEHIGH_GATEDCLK 0x2000
+#define BCM43xx_SBTMSTATEHIGH_BISTFAILED   0x4000
+#define BCM43xx_SBTMSTATEHIGH_BISTCOMPLETE 0x8000
 
 /* sbimstate flags */
 #define BCM43xx_SBIMSTATE_IB_ERROR 0x2
@@ -574,8 +597,11 @@
struct bcm43xx_dmaring *tx_ring1;
struct bcm43xx_dmaring *tx_ring2;
struct bcm43xx_dmaring *tx_ring3;
+   struct bcm43xx_dmaring *tx_ring4;
+   struct bcm43xx_dmaring *tx_ring5;
+
struct bcm43xx_dmaring *rx_ring0;
-   struct bcm43xx_dmaring *rx_ring1; /* only available on core.rev < 5 */
+   struct bcm43xx_dmaring *rx_ring3; /* only available on core.rev < 5 */
 };
 
 /* Data structures for PIO transmission, per 80211 core. */
@@ -739,7 +765,7 @@
 
/* Reason code of the last interrupt. */
u32 irq_reason;
-   u32 dma_reason[4];
+   u32 dma_reason[6];
/* saved irq enable/disable state bitfield. */
u32 irq_savedstate;
/* Link Quality calculation context. */
Index: wireless-2.6/drivers/net/wireless/bcm43xx/bcm43xx_dma.c
===
--- wireless-2.6.orig/drivers/net/wireless/bcm43xx/bcm43xx_dma.c
2006-08-15 22:31:01.0 +0200
+++ wireless-2.6/drivers/net/wireless/bcm43xx/bcm43xx_dma.c 2006-08-15 
23:25:55.0 +0200
@@ -4,7 +4,7 @@
 
   DMA ring

[PATCH] bcm43xx: re-add bcm43xx_rng_init() call

2006-08-15 Thread Michael Buesch
Hi John,

Please apply this to wireless-2.6.
Please also drop Adrian's patch (If you queued it somewhere):
[RFC: -mm patch] bcm43xx_main.c: remove 3 functions

--

Calls to bcm43xx_rng_init() and bcm43xx_rng_exit() got
lost due to merge trouble. Re-add them.

Signed-off-by: Michael Buesch <[EMAIL PROTECTED]>

Index: wireless-2.6/drivers/net/wireless/bcm43xx/bcm43xx_main.c
===
--- wireless-2.6.orig/drivers/net/wireless/bcm43xx/bcm43xx_main.c   
2006-08-15 23:27:13.0 +0200
+++ wireless-2.6/drivers/net/wireless/bcm43xx/bcm43xx_main.c2006-08-16 
00:14:00.0 +0200
@@ -3274,6 +3274,7 @@
 /* This is the opposite of bcm43xx_init_board() */
 static void bcm43xx_free_board(struct bcm43xx_private *bcm)
 {
+   bcm43xx_rng_exit(bcm);
bcm43xx_sysfs_unregister(bcm);
bcm43xx_periodic_tasks_delete(bcm);
 
@@ -3541,6 +3542,9 @@
err = bcm43xx_sysfs_register(bcm);
if (err)
goto err_wlshutdown;
+   err = bcm43xx_rng_init(bcm);
+   if (err)
+   goto err_sysfs_unreg;
 
/*FIXME: This should be handled by softmac instead. */
schedule_work(&bcm->softmac->associnfo.work);
@@ -3550,6 +3554,8 @@
 
return err;
 
+err_sysfs_unreg:
+   bcm43xx_sysfs_unregister(bcm);
 err_wlshutdown:
bcm43xx_shutdown_all_wireless_cores(bcm);
 err_crystal_off:


-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Add Larry Finger to bcm43xx MAINTAINERS

2006-08-15 Thread Michael Buesch
Hi,

I officially drop my maintainance of the bcm43xx-softmac
driver now with this mail. Larry will continue with my
work on the bcm43xx-softmac driver.

I will instead focus on the bcm43xx-d80211 driver and on
the implementation of the new v4 specs.

So if someone wants to send some bcm43xx-softmac patch
upstream, the person to send it to is Larry.
Larry will also take care of backporting fixes from
the bcm43xx-d80211 branch to bcm43xx-softmac.

So John, please apply the following patch to wireless-2.6.

--

Remove Michael Buesch and add Larry Finger in the
bcm43xx-softmac MAINTAINERS record.

Signed-off-by: Michael Buesch <[EMAIL PROTECTED]>

Index: wireless-2.6/MAINTAINERS
===
--- wireless-2.6.orig/MAINTAINERS   2006-08-15 16:51:43.0 +0200
+++ wireless-2.6/MAINTAINERS2006-08-16 00:32:46.0 +0200
@@ -449,9 +449,9 @@
 W: http://www.baycom.org/~tom/ham/ham.html
 S: Maintained
 
-BCM43XX WIRELESS DRIVER
-P: Michael Buesch
-M: [EMAIL PROTECTED]
+BCM43XX WIRELESS DRIVER (SOFTMAC BASED VERSION)
+P: Larry Finger
+M: [EMAIL PROTECTED]
 P: Stefano Brivio
 M: [EMAIL PROTECTED]
 W: http://bcm43xx.berlios.de/

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] add bcm43xx-d80211 MAINTAINERS entry

2006-08-16 Thread Michael Buesch
Hi John,

Please pull the patch to add Larry as bcm43xx-softmac
maintainer into wireless-dev and _after_ that please apply
the following patch to mark the d80211 branch explicitely.

--

Add MAINTAINERS for bcm43xx-d80211

Signed-off-by: Michael Buesch <[EMAIL PROTECTED]>

Index: wireless-dev/MAINTAINERS
===
--- wireless-dev.orig/MAINTAINERS   2006-08-16 11:26:18.0 +0200
+++ wireless-dev/MAINTAINERS2006-08-16 11:27:29.0 +0200
@@ -456,6 +456,14 @@
 W: http://www.baycom.org/~tom/ham/ham.html
 S: Maintained
 
+BCM43XX WIRELESS DRIVER (DEVICESCAPE BASED VERSION)
+P: Michael Buesch
+M: [EMAIL PROTECTED]
+P: Stefano Brivio
+M: [EMAIL PROTECTED]
+W: http://bcm43xx.berlios.de/
+S: Maintained
+
 BCM43XX WIRELESS DRIVER (SOFTMAC BASED VERSION)
 P: Larry Finger
 M: [EMAIL PROTECTED]


-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] bcm43xx-softmac: optimization of DMA bitfields.]

2006-08-16 Thread Michael Buesch
On Wednesday 16 August 2006 17:52, Larry Finger wrote:
> Johannes Berg wrote:
> > On Wed, 2006-08-16 at 10:36 -0500, Larry Finger wrote:
> >> +  /* Boolean. Is this a TX ring? */
> >> +  u8 tx
> >> +  /* Boolean. 64bit DMA if true, 32bit DMA otherwise. */
> >> +  u8 dma64;
> > 
> > does that compile?
> > 
> > johannes
> > 
> 
> Yes, it did here. Did you have a problem?

You removed a ; by mistake.
Look, it's there in my original patch (which was compile tested).

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: DEBUG_LOCKS_WARN_ON triggered by bcm43xx-SoftMAC

2006-08-16 Thread Michael Buesch
On Wednesday 16 August 2006 05:54, Larry Finger wrote:
> Using vanilla wireless-2.6 from Linville's tree as updated today, I
> get a warning generated by the statement
>  if (DEBUG_LOCKS_WARN_ON(!irqs_disabled()))
> in kernel.lockdep.c. This warning only occurs when my wireless
> interface is inserted and bcm43xx-softmac is activated. For that
> reason I have included the SoftMAC and bcm43xx messages.
> 
> kernel: SoftMAC: sent association request!
> kernel: SoftMAC: associated!
> kernel: bcm43xx: set security called, .active_key = 0, .level = 2,
> .enabled = 1, .encrypt = 1
> kernel: bcm43xx: set security called, .enabled = 1, .encrypt = 1
> kernel: BUG: warning at kernel/lockdep.c:1801/trace_hardirqs_on()
> kernel:  [] show_trace_log_lvl+0x197/0x1c0
> kernel:  [] show_trace+0x1b/0x20
> kernel:  [] dump_stack+0x26/0x30
> kernel:  [] trace_hardirqs_on+0x17f/0x190
> kernel:  [] do_general_protection+0xc5/0x230
> kernel:  [] error_code+0x39/0x40
> kernel:  [<38c7>] 0x38c7
> kernel:  [] show_trace+0x1b/0x20
> kernel:  [] dump_stack+0x26/0x30
> kernel:  [] trace_hardirqs_on+0x17f/0x190
> kernel:  [] do_general_protection+0xc5/0x230
> kernel:  [] error_code+0x39/0x40
> 
> I'm at a loss here. Can anyone explain how to interpret this dump? I
> think I see a general protection fault, but what to do from there is a
> mystery.

Hm, weird bug.
I can't reproduce this on i386 or PPC.
Could it be a bug in the lockdep code?

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: SSB: No is not an answer

2007-12-01 Thread Michael Buesch
On Saturday 01 December 2007 20:00:23 Arnaldo Carvalho de Melo wrote:
> Em Sat, Dec 01, 2007 at 12:45:32PM -0500, John W. Linville escreveu:
> > On Sat, Dec 01, 2007 at 03:17:44PM -0200, Arnaldo Carvalho de Melo wrote:
> > > Sonics Silicon Backplane support (SSB) [M/y/?] (NEW) n
> > > 
> > > Support for the Sonics Silicon Backplane bus.
> > > You only need to enable this option, if you are
> > > configuring a kernel for an embedded system with
> > > this bus.
> > > It will be auto-selected if needed in other
> > > environments.
> > > 
> > > The module will be called ssb.
> > > 
> > > If unsure, say N.
> > > 
> > > Sonics Silicon Backplane support (SSB) [M/y/?] (NEW)
> > 
> > I think this is OK -- it isn't really offering the choice to say
> > no anyway.  You must have turned-on B44 or B43(LEGACY) already?
> > 
> > So, your choice is merely whether to have it built-in or as a module.
> 
> Ok, so the comment on being unsure is wrong as we can't say N as
> suggested :-)

Oh, come on... Read the _whole_ comment.

-- 
Greetings Michael.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-2.6.24] introduce MAC_FMT/MAC_ARG

2007-08-29 Thread Michael Buesch
On Wednesday 29 August 2007 00:54:19 David Miller wrote:
> From: Michael Buesch <[EMAIL PROTECTED]>
> Date: Tue, 28 Aug 2007 16:48:44 +0200
> 
> > On Monday 27 August 2007 23:11:50 David Miller wrote:
> > > From: Joe Perches <[EMAIL PROTECTED]>
> > > Date: Mon, 27 Aug 2007 13:57:42 -0700
> > > 
> > > > On Mon, 2007-08-27 at 13:41 -0700, David Miller wrote:
> > > > > From: Johannes Berg <[EMAIL PROTECTED]>
> > > > > Date: Mon, 27 Aug 2007 12:54:09 +0200
> > > > > > #define MAC_FMT "%s"
> > > > > > #define MAC_ARG(a) ({char __buf[18]; print_mac(a, __buf); __buf;})
> > > > 
> > > > > I don't think this works.
> > > > 
> > > > $ cat test_fmt.c
> > > > #include 
> > > > #include 
> > > 
> > > You're just getting lucky in this test case.
> > > 
> > > The language does not allow what you are doing, so you're
> > > playing with fire.
> > 
> > What exactly to you think it invalid in this code?
> > I think it's fine (except that I'd chose an u8* for the mac
> > address (first arg in print_mac()).
> 
> The __buf[] is used out of scope, therefore it's stack space is
> fair game for the compiler to reuse.
> 
> When the compiler sees:
> 
>   printk(FMT, ({ char __buf[x]; print_mac(a, __buf); __buf;}));
> 
> It first all of the printk() argument expressions, first FMT and
> then it evaluates the ({ ... }) argument.
> 
> Now that the ({ ... }) expression is done, __buf[] is out of
> scope and illegal to reference.
> 
> printk() is now called, with a pointer to an out-of-scope buffer.
> This is illegal.
> 
> I don't know how else to explain this to you, I can learn how to
> describe the issue in German if this would help :-)

Oh, not needed. I see the bug and indeed, this is a ticking
timebomb.
I don't use the ({}) notation a lot, so I didn't see this here.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/5] Net: ath5k, license is GPLv2

2007-08-29 Thread Michael Buesch
On Wednesday 29 August 2007 21:33:43 Jon Smirl wrote:
> What if a patch spans both code that is pure GPL and code imported
> from BSD, how do you license it?

I think it's a valid assumption, if we say that the author
of the patch read the license header of a file and agreed with it.
So the patch is licensed to whatever the fileheader says. And if
there's none, it's licensed with the COPYING terms.
If a patch author likes some other license conditions, he must
explicitely add them with the patch to the file, saying that this
and that part have these and those conditions. Of course they must
be compatible with the original license.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: BUG: scheduling while atomic: ifconfig/0x00000002/4170

2007-09-07 Thread Michael Buesch
On Friday 07 September 2007, Johannes Berg wrote:
> On Thu, 2007-09-06 at 08:46 -0700, Paul E. McKenney wrote:
> 
> > Looks good to me from an RCU viewpoint.  I cannot claim familiarity with
> > this code.  I therefore especially like the indications of where RTNL
> > is held and not!!!
> 
> :)
> 
> > Some questions below based on a quick scan.  And a global question:
> > should the comments about RTNL being held be replaced by ASSERT_RTNL()?
> 
> I don't like ASSERT_RTNL() much because it actually tries to lock it.
> I'd be much happer if it was WARN_ON(!mutex_locked(&rtnl_mutex)) or
> something equivalent.

What's the problem with trying to lock it?
In the paths where you insert this assertion, you will be locked.
So the trylock will fail and not cause any blocking or something else.
It's basically not more expensive than your mutex_locked test.
And the !mutex_locked test might not work on UP (Not sure, about
the current implementation.)
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Please pull 'rt2x00' branch of wireless-2.6

2007-09-19 Thread Michael Buesch
On Wednesday 19 September 2007 20:47:59 Ivo van Doorn wrote:
> On Wednesday 19 September 2007, David Miller wrote:
> > From: Ivo van Doorn <[EMAIL PROTECTED]>
> > Date: Wed, 19 Sep 2007 20:31:19 +0200
> > 
> > > Because I am indeed not really happy with this early merge, but I'll do my
> > > best to resolve the last outstanding bugs as soon as possible.
> > 
> > Relax :-)
> 
> :)
> 
> > A merge upstream doesn't mean "bug free", it means "significantly
> > better than no driver at all" and that is undoubtedly the case
> > here.
> 
> Well I am happy rt2x00 is being considered good enough for upstream already,
> but I have the habbit of wanting things bug-free before pushing it further 
> upstream.
> 
> Well most users are happy with current rt2x00 version anyway,
> so the remaining issues probably aren't that big anyway. (Except for the 2 
> panics,
> which should be resolved soon anyway) ;)

Well, you have nothing to lose, basically.
It's impossible for you to create a regression.
So everything is fine, even if it's buggy and doesn't
work at all. ;)

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Please pull 'z1211' branch of wireless-2.6

2007-09-19 Thread Michael Buesch
On Wednesday 19 September 2007 20:59:03 David Miller wrote:
> From: "John W. Linville" <[EMAIL PROTECTED]>
> Date: Wed, 19 Sep 2007 14:10:05 -0400
> 
> > This patch adds z1211 (previously known as zd1211rw-mac80211),
> > a driver for Zydas ZD1211 hardware.  This driver has proven very
> > robust -- Fedora 7 uses this driver and I don't think I have any open
> > bugzillas for it.  Either the driver works well, or no one as using
> > it...given the common availability of the hardware, I have to presume
> > the former. :-)
> > 
> > This driver is mostly a port of zd1211rw to the mac80211
> > infrastructure.  In fact, most zd1211rw patches have been mirrored
> > directly to this driver for some time.  I considered merely updating
> > the existing driver with this code, but I think it is more prudent to
> > include this as a separate driver just to avoid confusion.  There is
> > some symbol clash between the two drivers, so I add a Kconfig hack
> > to ensure that only one or the other is built-in or that this one is
> > built as a module.
> > 
> > The maintainers have identified some (IMHO minor) issues with the
> > mac80211 port of this driver.  Some of them are summarized here:
> > 
> > http://linuxwireless.org/en/users/Drivers/zd1211rw/mac80211Issues
> > 
> > Still, I think we would be better-off having this driver upstream.
> 
> Agreed, merged into net-2.6.24, thanks John.


John, please also note that the following patch is not merged, yet.
http://bu3sch.de/patches/wireless-dev/20070915-1740/patches/006-zd-fix-tx-status.patch
Without it ratecontrol does not work. It does neither scale the rate
up nor down. This patch fixes both.
I think it got lost in a (really unrelated to the actual bug) discussion
about whether it's required or not to report status in mac80211 drivers.
(It turned out that it _is_ required).

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: net-2.6.24 - build failure

2007-09-21 Thread Michael Buesch
On Thursday 20 September 2007 00:17:12 David Miller wrote:
> From: Joe Perches <[EMAIL PROTECTED]>
> Date: Wed, 19 Sep 2007 14:53:22 -0700
> 
> > drivers/net/wireless/b43legacy/built-in.o: In function `tsf_read_file':
> > drivers/net/wireless/b43legacy/debugfs.c:80: multiple definition of 
> > `tsf_read_file'
> 
> Can one of the wireless folks fix b43legacy to not use the same
> global variable and function names as the b43 driver?

Fixes are already submitted.
This was only an accident (forgot static).

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Please pull 'z1211' branch of wireless-2.6

2007-09-22 Thread Michael Buesch
On Saturday 22 September 2007 11:48:00 Ulrich Kunitz wrote:
> A real high-quality driver will require Johannes' proposed
> mac80211 driver interface changes to be merged and TX
> confirmations handled in a way, that the semantics can really be
> supported by the driver. (Michael Buesh's patch is taping over the
> issue.)

No it is not. It is fixing the issue. It fixes the following issues:
* You must ignore the Txstat-requested bit in the driver.
* You must report bad frames with the excessive_retries set.

The issue you are (most likely) talking about is that we can not
reliably tell whether a frame was good in the driver. That is a different
issue completely seperate from the two points above, which my patch fixes.

With my patch rate-controlling correctly works. Without it does not.

If you find a way to fix the reliable-detection-of-good-TX issue, that's
another good fix. But I think it's not release critical, because the
device works with the current "guessing-around" code. But without the two
points above fixed, it does not correctly work at all (unless you manually
tune to the best rate each time you move the machine).

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Please pull 'z1211' branch of wireless-2.6

2007-09-22 Thread Michael Buesch
On Saturday 22 September 2007, Ulrich Kunitz wrote:
> Michael Buesch wrote:
> 
> > On Saturday 22 September 2007 11:48:00 Ulrich Kunitz wrote:
> > > A real high-quality driver will require Johannes' proposed
> > > mac80211 driver interface changes to be merged and TX
> > > confirmations handled in a way, that the semantics can really be
> > > supported by the driver. (Michael Buesh's patch is taping over the
> > > issue.)
> > 
> > No it is not. It is fixing the issue. It fixes the following issues:
> 
> > * You must ignore the Txstat-requested bit in the driver.
> 
> If that is really the case the flag should be removed from
> mac80211. There is no way for somebody looking at the code to know
> this.

It is used internally in mac80211.

> > * You must report bad frames with the excessive_retries set.
> 
> All bad frames or only those with actual excessive retries? Your patch
> set the excessive_retries flag for packets that couldn't be
> transmitted to the device because of an USB error. If the flag
> should be set for all kinds of errors it should be renamed.

Well, there is no better flag to set, currently.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] mac80211: Fix TX after monitor interface is converted to managed

2007-10-04 Thread Michael Buesch
On Thursday 04 October 2007 16:34:43 Michael Wu wrote:
> On Thursday 04 October 2007 07:33, Daniel Drake wrote:
> > Fix this by unsetting the hard_start_xmit handler in ieee80211_if_reinit.
> > It will then be reinitialised to the default (ieee80211_subif_start_xmit)
> > in ieee80211_if_set_type.
> >
> Well.. this kinda sucks, but we can clean up the logic here later.
> 
> > +   BUG_ON(netif_running(dev));
> This will never happen, so there's no point.

The reason why BUG_ON exists is to catch bugs that happen, although
they Should Never Happen (tm) ;)

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2.6] WE-22 : prevent information leak on 64 bit

2007-04-17 Thread Michael Buesch
On Tuesday 17 April 2007 19:08, Jean Tourrilhes wrote:
>   I'm sorry to have to write this e-mail. But this incident is
> completely opposed to the ideal of FreeSoftware/OpenSource and
> demonstrate some of the bad politics happening in Linux.
> 
>   First, I'm the current active maintainer of the
> wext-over-netlink interface, and nobody bothered to even 'inform' me
> about its removal, let alone consult with me.
>   This shows a complete lack of courtesy and a total disrespect
> to the concept of maintainer, basically some people are just second
> class citizens.
> 
>   Second, there is no technical justification to such decision,
> it's just plain politics. I would agree that for the vast majority of
> people, this API was useless, as any work in progress. But, it is
> maintained (by me), it is not causing any technical issue, for those
> people it's not compiled in (i.e. no bloat), it is not causing bugs
> and not preventing other code to be merged in the kernel.
>   Therefore a purely politic decision.

It is _only_ about replacing obsolete code by code that obsoleted it.
That happens all the time. Look at the process scheduler and compare
it to 2.4, for example.

We want to reduce the maintainance burden. Nothing more.
If we remove unused code (which WEXT-NL is), then we don't have to
write compatibility code to support it in future.
Why wait with removal until we can't anymore (when people use it)?

>   Now, I've got a problem with your attitude in this matter,
> Johannes. It's now the second time you remove features from code I
> maintain by pure fiat, and you have engaged in a long running FUD
> campain about my code. This is totally disgraceful of a Linux
> maintainer, and you should know it.
>   If the only way you have to promote your code is by actively
> destroying my code, then you have a real issue. Your code should stand
> on its own merit, without the need of attacking other people's work
> and playing political tricks.
>   I hope you will note that I never disparaged your code, I
> never prevented its inclusion in Linux and I never attempted to
> control the Linux Wireless space and left plenty of space for new
> developpers.
> 
>   You still have a lot to learn, like all of us. You still don't
> understand Wireless Extensions (as your FUD shows) and why it's still
> so popular despite all its warts. You don't get the value of not
> burning bridges with other developpers and professional conduct.

I'd say nobody but you does fully understand WEXT. Somebody might
call that either a design issue, or a documentation issue.
I personally make both issues responsible for this.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 7/9] wext: reduce inline abuse

2007-04-26 Thread Michael Buesch
On Thursday 26 April 2007 18:50:32 Jean Tourrilhes wrote:
> On Tue, Apr 24, 2007 at 08:07:39PM +0200, Johannes Berg wrote:
> > This patch removes a bunch of inline abuse from wext. Most functions
> > that were marked inline are only used once so the compiler will inline
> > them anyway, others are used multiple times but there's no requirement
> > for them to be inline since they aren't in any fast paths.
> > 
> > Signed-off-by: Johannes Berg <[EMAIL PROTECTED]>
> 
>   That's clearly not true of all compilers. All gcc versions
> before 4.0 need serious help to inline functions used only once. Our
> current minimal requirement for the kernel is gcc 3.2, therefore this
> code is still useful.
>   Note that this is a legitimate use of inline (tell the
> compiler to inline the function), not an abuse.

By my personal definition _every_ use of inline is abuse, if it's not
in an absolute fastpath and applied to a really tiny function.
Sure, other people have different opinions on that, but I think
with my approach we get smallest code with good speed.
In general I try to avoid inline whereever possible.
I think this patch is OK and should go in.

Often it's even desired to have out of line functions in fastpaths.
See spinlocks.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Merging SSB upstream

2007-05-05 Thread Michael Buesch
So, now that mac80211 is merged upstream, I think it's
time to merge SSB and the b44-ssb port upstream.
Note that bcm43xx-mac80211 is _not_ ready for upstream, yet.

What do you think? I'd like to merge ssb as-is, although
the embedded-device parts are not quite finished, yet.
But they don't interfere with the non-embedded parts used
by b44 and the bcm43xx PCI cards.
So we _could_ remove the ssb-mips code, but I don't like to
do that for better maintainability. It doesn't hurt anyone IMO.

Some opinions?

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Merging SSB upstream

2007-05-06 Thread Michael Buesch
On Sunday 06 May 2007 05:11:44 Jeff Garzik wrote:
> Michael Buesch wrote:
> > So, now that mac80211 is merged upstream, I think it's
> > time to merge SSB and the b44-ssb port upstream.
> > Note that bcm43xx-mac80211 is _not_ ready for upstream, yet.
> > 
> > What do you think? I'd like to merge ssb as-is, although
> > the embedded-device parts are not quite finished, yet.
> > But they don't interfere with the non-embedded parts used
> > by b44 and the bcm43xx PCI cards.
> > So we _could_ remove the ssb-mips code, but I don't like to
> > do that for better maintainability. It doesn't hurt anyone IMO.
> 
> What does Ralf (MIPS maintainer) and Gary (Broadcom maintainer) think?

I don't know. I wanted to get all these opinions with this mail. :)

> For my part, I'm not going to render even a tentative opinion without a 
> link to actual code.

All code is in wireless tree drivers/ssb
http://bu3sch.de/gitweb?p=wireless-dev.git;a=tree;f=drivers/ssb;h=681cd93bf166670efbdec471b78137d3d0f26537;hb=HEAD

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Merging SSB upstream

2007-05-06 Thread Michael Buesch
On Sunday 06 May 2007 04:00:51 John W. Linville wrote:
> On Sun, May 06, 2007 at 03:03:17AM +0200, Michael Buesch wrote:
> > So, now that mac80211 is merged upstream, I think it's
> > time to merge SSB and the b44-ssb port upstream.
> > Note that bcm43xx-mac80211 is _not_ ready for upstream, yet.
>  
> ACK, unfortunately.
> 
> > What do you think? I'd like to merge ssb as-is, although
> > the embedded-device parts are not quite finished, yet.
> > But they don't interfere with the non-embedded parts used
> > by b44 and the bcm43xx PCI cards.
> 
> How much testing have you (and others) done w/ b44?

Well, it works for me with my b44 card I got from broadcom.
There are really only 2 or 3 different b44 cards, so chances are damn
high it will work for all.

> I had to remove 
> the b44 ssb changes from fedora because a) users reported problems;

Which problems were that? The 2 compile issues?
Trivial to fix if that's the only issue. ;)

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Merging SSB upstream

2007-05-06 Thread Michael Buesch
On Sunday 06 May 2007 19:38:33 John W. Linville wrote:
> On Sun, May 06, 2007 at 11:44:27AM +0200, Michael Buesch wrote:
> > On Sunday 06 May 2007 04:00:51 John W. Linville wrote:
> 
> > > I had to remove 
> > > the b44 ssb changes from fedora because a) users reported problems;
> > 
> > Which problems were that? The 2 compile issues?
> > Trivial to fix if that's the only issue. ;)
> 
> I knew you would ask that... :-)

:P

> I don't think there was a bugzilla, but Dave Jones forwarded an email
> to me from "MASAO TAKAHASHI" in late February.  Takahashi-san (forgive
> me if I did that wrong) was complaining about tx timeouts after I
> had added the full wireless-dev patchset to rawhide.  Removing the
> b44 bits of the patch seemed to remove the problem.
> 
> That's all the info I have.  Perhaps Dave or Takahashi-san can add
> to the description?

Hm, interesting issue.
But I'm not convinced that it's caused by the SSB port, though.
What the SSB port essentially does is modifying small areas in the
init and exit paths. The modified things in the TX and RX hotpaths
are really tiny and trivial. TX timeout sounds like something in the
TX/RX paths is going wrong.
But anyway, maybe I got something wrong.
I'll run some burn-in tests on it now.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Merging SSB upstream

2007-05-07 Thread Michael Buesch
On Monday 07 May 2007 18:43:18 Gary Zambrano wrote:
> On Sat, 2007-05-05 at 23:11 -0400, Jeff Garzik wrote:
> > Michael Buesch wrote:
> > > So, now that mac80211 is merged upstream, I think it's
> > > time to merge SSB and the b44-ssb port upstream.
> > > Note that bcm43xx-mac80211 is _not_ ready for upstream, yet.
> > > 
> > > What do you think? I'd like to merge ssb as-is, although
> > > the embedded-device parts are not quite finished, yet.
> > > But they don't interfere with the non-embedded parts used
> > > by b44 and the bcm43xx PCI cards.
> > > So we _could_ remove the ssb-mips code, but I don't like to
> > > do that for better maintainability. It doesn't hurt anyone IMO.
> > 
> > What does Ralf (MIPS maintainer) and Gary (Broadcom maintainer) think?
> > 
> > For my part, I'm not going to render even a tentative opinion without a 
> > link to actual code.
> > 
> > Last I saw of the code, and descriptions in IRC, it sounded sane.
> > 
> > Jeff
> > 
> 
> I would like to put some test mileage behind the ssb.
> We had a hard time testing it a while back, so we will try the latest.

Ok, nice to hear. :)
I stresstested latest ssb on my b44 card and it works fine here.
No TX timeouts or something.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Please pull 'upstream-rtl8187' branch of wireless-2.6

2007-05-10 Thread Michael Buesch
On Thursday 10 May 2007 04:16:22 Michael Wu wrote:
> > > + rtl818x_iowrite8(priv, (u8 *)0xFE18, 0x10);
> > > + rtl818x_iowrite8(priv, (u8 *)0xFE18, 0x11);
> > > + rtl818x_iowrite8(priv, (u8 *)0xFE18, 0x00);
> > > + mdelay(200);
> >
> > ditto
> >
> > also, kill the magic numbers
> >
> I have no idea what that does so I don't see the point in moving the number 
> to 
> some define. However, the hardware does seem to work okay without this part 
> so I can remove it if you bothers you so much.

Nah, Jeff. Please don't go this path.
If we have to remove all magic numbers from drivers, we'd have to either
*) Drop reverse engineered drivers like bcm43xx completely.
*) Clutter them with completely useless defines such as
   #define RADIO_UNKNOWN97A_REG 0x97A

Better leave such magic registers/values in the code as-is
and probably convert them to good defines later, if reverse engineering
found out the meaning. That's what I'm doing for bcm43xx and it makes
a _whole_ lot more sense.

> > > + if (eeprom->reg_data_in)
> > > + reg |= RTL818X_EEPROM_CMD_WRITE;
> > > + if (eeprom->reg_data_out)
> > > + reg |= RTL818X_EEPROM_CMD_READ;
> > > + if (eeprom->reg_data_clock)
> > > + reg |= RTL818X_EEPROM_CMD_CK;
> > > + if (eeprom->reg_chip_select)
> > > + reg |= RTL818X_EEPROM_CMD_CS;
> > > +
> > > + rtl818x_iowrite8(priv, &priv->map->EEPROM_CMD, reg);
> > > + udelay(10);
> >
> > questionable delay
> >
> Most likely to prevent hitting the eeprom too fast.

Accessing eeproms is almost always some kind of critical
and timing dependent task on every device. That's because
of the electrical behavior of the eeprom and its bus.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: b44: regression in 2.6.22

2007-05-26 Thread Michael Buesch
On Saturday 26 May 2007 02:24:31 Stephen Hemminger wrote:
> Something is broken with the b44 driver in 2.6.22-rc1 or later. Now bisecting.
> The performance (with iperf) for receiving is normally 94Mbits or more.
> But something happened that dropped performance to less than 1Mbit,
> probably corrupted packets.
> 
> There is nothing obvious in the commit log for drivers/net/b44.c, so it
> probably is something more general.
> 
> 
> Looking at the code in b44_rx(), I see a couple unrelated of bugs:
> 1. In the small packet case it recycles the skb before copying data out... 
>Not good if new data arrives overwriting existing data.
> 
> 2. Macros like RX_PKT_BUF_SZ that depend on local variables are evil!!

Very interesting!
2.6.22 doesn't include ssb, does it?

Adding CCs to make reporters of another bugreport aware of this.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


  1   2   3   4   5   6   >