tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: BCM5809S support in bnx(4) and brgphy(4)



> I have ported various modifications from OpenBSD and FreeBSD to bnx(4)
> and brgphy(4) (see attach).
 :
> --- sys/dev/mii/brgphy.c      27 Nov 2010 17:42:04 -0000      1.56
> +++ sys/dev/mii/brgphy.c      30 Nov 2010 23:58:49 -0000
 :
>  #include <dev/pci/if_bgereg.h>
> -#if 0
>  #include <dev/pci/if_bnxreg.h>
> -#endif
 :
> +             if (device_is_a(parent, "bnx")) {
> +                     struct bnx_softc *bnx_sc = device_private(parent);
 :
> +                     if (bnx_sc->bnx_phy_flags & BNX_PHY_2_5G_CAPABLE_FLAG) {

We should not refer struct bnx_softc in brgphy.c
because mii devices can be configured without PCI
and bnx_softc contains PCI specific stuff.
Your patch might break GENERIC on zaurus, which
has mii via on-chip USB but no PCI bus.

To pass bnx_foo_flag values from parent bnx(4) to child brgphy(4),
we need the following two changes

 - use device properties (proplib) to pass flag values
 - split if_bnxreg.h and move PCI specific stuff
   ("Device State Data", debug macro etc.) into (new) if_bnxvar.h

as done on past brgphy(4) changes for bge(4) by msaitoh@:
http://mail-index.NetBSD.org/current-users/2009/04/20/msg009101.html
http://mail-index.NetBSD.org/source-changes/2009/04/23/msg220278.html

> --- sys/dev/pci/if_bnxreg.h   19 Jan 2010 22:07:00 -0000      1.10
> +++ sys/dev/pci/if_bnxreg.h   30 Nov 2010 23:58:54 -0000
 :
> +#if BYTE_ORDER == BIG_ENDIAN
> +     u_int16_t tx_bd_vlan_tag;
> +     u_int16_t tx_bd_flags;
> +#else
>       u_int16_t tx_bd_flags;
>       u_int16_t tx_bd_vlan_tag;
> +#endif

It would be clearer to use one uint32_t member with shift ops
for bd_flags and bd_vlan_tag rather than two uint16_t members
with reorder #ifdef while the above one will also work...

---
Izumi Tsutusi


Home | Main Index | Thread Index | Old Index