On Tuesday 23 May 2006 18:27, Johannes Berg wrote:
> On Tue, 2006-05-23 at 08:26 +0200, Stefano Brivio wrote:
>
> > -static int bcm43xx_geo_init(struct bcm43xx_private *bcm)
> > +static void bcm43xx_geo_init(struct bcm43xx_private *bcm)
> > {
> > - struct ieee80211_geo *geo;
> > + struct ieee80211_geo geo;
>
> This doesn't look like PCI-E support to me, and I haven't seen it in
> your earlier patchset.
Yeah. He probably diffed against an old tree, as this reverts an
important fix.
> > - if (bcm->chip_rev < 5) {
> > + if (bcm->core_pci.rev <= 5 && bcm->core_pci.id !=
> > BCM43xx_COREID_PCIE) {
> > sbimconfiglow = bcm43xx_read32(bcm,
> > BCM43xx_CIR_SBIMCONFIGLOW);
> > sbimconfiglow &= ~ BCM43xx_SBIMCONFIGLOW_REQUEST_TOUT_MASK;
> > sbimconfiglow &= ~ BCM43xx_SBIMCONFIGLOW_SERVICE_TOUT_MASK;
> > - if (bcm->bustype == BCM43xx_BUSTYPE_PCI)
> > - sbimconfiglow |= 0x32;
> > - else if (bcm->bustype == BCM43xx_BUSTYPE_SB)
> > - sbimconfiglow |= 0x53;
> > - else
> > - assert(0);
> > + sbimconfiglow |= 0x32;
> > bcm43xx_write32(bcm, BCM43xx_CIR_SBIMCONFIGLOW,
> > sbimconfiglow);
> > }
>
> Are you sure we can remove those things for bus type SB? Because, afaik,
> that's how some of the WRT... devices are done. Not sure if we handle
> them that way though. I forgot who created the patch for those, please
> speak up!
>
> > + if (bcm->current_core->id == BCM43xx_COREID_PCI) {
> [...]
> > + } else {
>
> Shouldn't that be an else if .. PCIE? Or is it guaranteed that only
> those ever end up there? I'm not sure.
It can only be PCIE, but I would like to have a
assert(bcm->current_core->id == BCM43xx_COREID_PCIE);
in the else branch to assert it and to clarify it to the reader.
I also had to stop and think a minute here. ;)
> > /* Set the MAC address in the networking subsystem */
> > - if (is_valid_ether_addr(bcm->sprom.et1macaddr))
> > + if (bcm43xx_current_phy(bcm)->type == BCM43xx_PHYTYPE_A)
> > memcpy(bcm->net_dev->dev_addr, bcm->sprom.et1macaddr, 6);
> > else
> > memcpy(bcm->net_dev->dev_addr, bcm->sprom.il0macaddr, 6);
>
> I thought we wanted to simply try both mac addresses?
Diffed against old tree...
> > + bcm43xx_geo_init(bcm);
> > +
>
> Again, unrelated change.
dito.
> johannes
>
_______________________________________________
Bcm43xx-dev mailing list
[email protected]
http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
