Discussion:
Another update for axe(4)
(too old to reply)
Izumi Tsutsui
2010-06-13 09:30:40 UTC
Permalink
My changes (for -current as of today) are attached. (I still need to
update the axe(4) man page.)
+#include <machine/bus.h>
We should use <sys/bus.h> nowadays,
but is this one actually required?
+#ifdef __NetBSD__
+ #define letoh16 htole16
+ #define letoh32 htole32
+#endif
I don't like these ifdefs.
We have had own le16toh(9) and le32toh(9) and
OpenBSD guys renamed them when they pulled them.
+void axe_ax88178_init(struct axe_softc *);
+void axe_ax88772_init(struct axe_softc *);
Why not Static?
Not in KNF, but I like one space before labels.
(to avoid diff(1) thinks it's a function name)
+ u_int16_t eeprom;
uintNN_t types are prefered, at least in new code.
+void
+axe_ax88772_init(struct axe_softc *sc)
+{
+ axe_cmd(sc, AXE_CMD_WRITE_GPIO, 0, 0x00b0, NULL);
+ usbd_delay_ms(sc->axe_udev, 40);
KNF says "Insert an empty line if the function has no local variables."
+ sc->axe_flags = axe_lookup(uaa->vendor, uaa->product)->axe_flags;
It looks sc->axe_flags should be set in axe_match()
where axe_lookup() is called first.
+ memcpy(&sc->init_eaddr, &eaddr, sizeof(sc->init_eaddr));
Do we really have to save MAC address into softc?
Can't we refer it by "CLLADDR(ifp->if_sadl)"?
- c->axe_buf = usbd_alloc_buffer(c->axe_xfer, AXE_BUFSZ);
+ c->axe_buf = usbd_alloc_buffer(c->axe_xfer,
+ sc->axe_bufsz);
KNF?
+ u_char *buf;
This should be uint8_t?
+ if ((pktlen % 2) != 0)
+ pktlen++;
roundup2(9)?
+struct axe_sframe_hdr {
+ u_int16_t len;
+ u_int16_t ilen;
+} __packed;
Is __packed actually required?

---
Izumi Tsutsui

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Izumi Tsutsui
2010-06-13 11:50:35 UTC
Permalink
Post by Izumi Tsutsui
+#include <machine/bus.h>
We should use <sys/bus.h> nowadays,
but is this one actually required?
Yes, it is really needed. I need to include usb/usbdivar.h to get the
usb speed data, and usbdivar.h references bus_dma_tag_t. I have changed
it to use <sys/bus.h>
Ok.
Post by Izumi Tsutsui
+ sc->axe_flags = axe_lookup(uaa->vendor, uaa->product)->axe_flags;
It looks sc->axe_flags should be set in axe_match()
where axe_lookup() is called first.
No, I don't think so. The results of the lookup in axe_match() are
discarded until the autoconfig stuff selects the best match. And
axe_match() doesn't even have a softc in which to store the results.
Ah, yes, we can only update attach args in aux, not softc. Ok.
Post by Izumi Tsutsui
+ memcpy(&sc->init_eaddr, &eaddr, sizeof(sc->init_eaddr));
Do we really have to save MAC address into softc?
Can't we refer it by "CLLADDR(ifp->if_sadl)"?
I'll be totally honest here - I really haven't dived very deeply into
the network interface architecture at all. So I have no idea if your
suggestion would work or not. Where does ifp->if_sadl get stored?
I think ifp is initialized once ether_ifattach() is called
and we can always pull ifp in the drive via &sc->ethercom.ec_if.
(USB drivers seem to use GET_IFP(sc) macro)

axe_init() is called only from ioctls so I think it will work.
(otherwise OpenBSD's ac_enaddr won't work either)
Post by Izumi Tsutsui
- c->axe_buf = usbd_alloc_buffer(c->axe_xfer, AXE_BUFSZ);
+ c->axe_buf = usbd_alloc_buffer(c->axe_xfer,
+ sc->axe_bufsz);
KNF?
:) Fixed (although I still prefer my own format here!)
There are some more similar lines.
Post by Izumi Tsutsui
+ u_char *buf;
This should be uint8_t?
This is from the OpenBSD code. buf is being assigned from
buf = c->axe_buf
where c->axe_buf is a (char *), so not sure if (uint8_t *) would be
correct.
(char *) might cause annoying gcc's signed/unsigned comparison warnings,
so unsigned is better for any xfer buffers, IMO.
Post by Izumi Tsutsui
+ if ((pktlen % 2) != 0)
+ pktlen++;
roundup2(9)?
Done.
Looks wrong arg order.

"pkglen = roundup2(pktlen, sizeof(uint16_t));"
might be better as roundup2(9) man page says :-)
Post by Izumi Tsutsui
+struct axe_sframe_hdr {
+ u_int16_t len;
+ u_int16_t ilen;
+} __packed;
Is __packed actually required?
Probably not, but the struct does reflect the format of data on the
physical media. Just to avoid any future issues on an architecture that
might want to align thing on 32-bit boundary, I would prefer to leave
__packed here.
Ok.

---
Izumi Tsutsui

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Paul Goyette
2010-06-13 14:25:45 UTC
Permalink
Post by Izumi Tsutsui
Post by Izumi Tsutsui
+ memcpy(&sc->init_eaddr, &eaddr, sizeof(sc->init_eaddr));
Do we really have to save MAC address into softc?
Can't we refer it by "CLLADDR(ifp->if_sadl)"?
I'll be totally honest here - I really haven't dived very deeply into
the network interface architecture at all. So I have no idea if your
suggestion would work or not. Where does ifp->if_sadl get stored?
I think ifp is initialized once ether_ifattach() is called
and we can always pull ifp in the drive via &sc->ethercom.ec_if.
(USB drivers seem to use GET_IFP(sc) macro)
axe_init() is called only from ioctls so I think it will work.
(otherwise OpenBSD's ac_enaddr won't work either)
OK, I changed it and it still works. (BTW, I need to make it
__UNCONST() for passing it as arg to axe_cmd() routine.)
Post by Izumi Tsutsui
Post by Izumi Tsutsui
- c->axe_buf = usbd_alloc_buffer(c->axe_xfer, AXE_BUFSZ);
+ c->axe_buf = usbd_alloc_buffer(c->axe_xfer,
+ sc->axe_bufsz);
KNF?
:) Fixed (although I still prefer my own format here!)
There are some more similar lines.
I think I have found all of them.
Post by Izumi Tsutsui
Post by Izumi Tsutsui
+ if ((pktlen % 2) != 0)
+ pktlen++;
roundup2(9)?
Done.
Looks wrong arg order.
"pkglen = roundup2(pktlen, sizeof(uint16_t));"
might be better as roundup2(9) man page says :-)
Ooops! Fixed.




-------------------------------------------------------------------------
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer | | pgoyette at netbsd.org |
-------------------------------------------------------------------------

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Izumi Tsutsui
2010-06-13 15:23:39 UTC
Permalink
Post by Paul Goyette
OK, I changed it and it still works. (BTW, I need to make it
__UNCONST() for passing it as arg to axe_cmd() routine.)
IMO, __UNCONST() could be problematic (actually -current kue(4)
panics due to abuse of __UNCONST()), so it's better to allocate
a tmpbuf[ETHER_ADDR_LEN] in stack and copy the const data into it
for generic APIs.
---
Izumi Tsutsui

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Izumi Tsutsui
2010-06-14 12:38:49 UTC
Permalink
I still looking for some guidance on whether or not my changes or
Fukaumi-san's changes (or neither?) should be committed?
Well, probably it's great to merge both changes ;-)

With a quick glance, these ones should be pulled from Fukaumi-san's one:
- more KNF cleanup (u_char -> uint8_t, no braces for return etc.)
(I don't like uWord though)
- make axe_init() return int to match our (*ifp->if_init)()
- change axe_stop() args to match our (*ifp->if_stop)()
- USETW()/UGETW() macro
- set ETHERCAP_VLAN_MTU
- callout_setfunc()/callout_schedule() rather than callout_reset()
- callout_destroy() is required on detach?
- one ether_ifdetach(ifp) is enough in axe_detach()
- it might be worth to try axe_start() in axe_tick_task()?
- IFQ_POLL() should be used to check TX mbuf and
IF_DEQUEUE() should be called after TX is actually ready in axe_start()
- probably needs callout_stop() (or axe_stop()?) in axe_init()
- AXE_RXCMD_MULTICAST in rxmode is required or not?
- AXE_RXCMD_PROMISC and AXE_RXCMD_BROADCAST should be set in axe_setmulti()?
- USBD_NO_COPY is better? (not sure if buffers are allocated by bus_dma(9))
- ioctl function is much simpler and seems correct (per other drivers)
- IFQ_IS_EMPTY() macro in axe_watchdog()

No idea for PHY/MII media options etc.
---
Izumi Tsutsui

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Paul Goyette
2010-06-14 12:51:33 UTC
Permalink
OK, I will grab a copy of Fukaumi-san's code and merge. This will take
a few days. :)
Post by Izumi Tsutsui
I still looking for some guidance on whether or not my changes or
Fukaumi-san's changes (or neither?) should be committed?
Well, probably it's great to merge both changes ;-)
- more KNF cleanup (u_char -> uint8_t, no braces for return etc.)
(I don't like uWord though)
- make axe_init() return int to match our (*ifp->if_init)()
- change axe_stop() args to match our (*ifp->if_stop)()
- USETW()/UGETW() macro
- set ETHERCAP_VLAN_MTU
- callout_setfunc()/callout_schedule() rather than callout_reset()
- callout_destroy() is required on detach?
- one ether_ifdetach(ifp) is enough in axe_detach()
- it might be worth to try axe_start() in axe_tick_task()?
- IFQ_POLL() should be used to check TX mbuf and
IF_DEQUEUE() should be called after TX is actually ready in axe_start()
- probably needs callout_stop() (or axe_stop()?) in axe_init()
- AXE_RXCMD_MULTICAST in rxmode is required or not?
- AXE_RXCMD_PROMISC and AXE_RXCMD_BROADCAST should be set in axe_setmulti()?
- USBD_NO_COPY is better? (not sure if buffers are allocated by bus_dma(9))
- ioctl function is much simpler and seems correct (per other drivers)
- IFQ_IS_EMPTY() macro in axe_watchdog()
No idea for PHY/MII media options etc.
---
Izumi Tsutsui
-------------------------------------------------------------------------
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer | | pgoyette at netbsd.org |
-------------------------------------------------------------------------

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
FUKAUMI Naoki
2010-06-15 08:35:58 UTC
Permalink
hi,

I'm sorry my work is incomplete.

I checked your patch.

- it should not ignore reported phy address. I have the device which can
read a value from a register via *invalid* phy address, it results
invalid phy address is used to access phy.

- it should add a quirk for devices which lies about phy address. but I
don't have such a device so I can't test it...

- in axe_setmulti(), "if (ifp->if_flags & IFF_ALLMULTI) {" must be "...
IFF_PROMISC ...".

- in axe_attach(), no need to read AXE_CMD_READ_PHYID twice.

- in axe_encap(), USBD_NO_COPY can be specified in usbd_setup_xfer().

- in axe_init(), splnet() should be called before using ifp->if_flags.

- in axe_init(), no need to set AXE_RXCMD_PROMISC and
AXE_RXCMD_BROADCAST individually.

- in axe_init(), usbd_setup_xfer(AXE_ENDPT_RX) should use c->axe_buf
instead of mtod(c->axe_mbuf). c->axe_mbuf seems a placeholder of unused
mbuf. received data should go into c->axe_buf. see axe_rxeof().

- in axe_ioctl(), call ifioctl_common() at first in SIOCSIFFLAGS case.

- in axe_watchdog(), "if (IFQ_IS_EMPTY(&ifp->if_snd))" must be
!IFQ_IS_EMPTY().

Regards,

--
FUKAUMI Naoki

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
FUKAUMI Naoki
2010-06-15 14:59:40 UTC
Permalink
hi,

thank you for your work!
Post by FUKAUMI Naoki
- in axe_init(), no need to set AXE_RXCMD_PROMISC and
AXE_RXCMD_BROADCAST individually.
Is the following correct?
/* If we want promiscuous mode, set the allframes bit. */
if (ifp->if_flags & (IFF_PROMISC | IFF_BROADCAST))
rxmode |= AXE_RXCMD_PROMISC |AXE_RXCMD_BROADCAST;
sorry, AXE_RXCMD_PROMISC is handled in axe_setmulti(), and
AXE_RXCMD_BROADCAST is handled few lines above in axe_init().
Post by FUKAUMI Naoki
- in axe_ioctl(), call ifioctl_common() at first in SIOCSIFFLAGS case.
Do you mean to call ether_ioctl()? Is the following correct?
see if_aue.c:aue_ioctl:SIOCSIFFLAGS for example.

Regards,

--
FUKAUMI Naoki

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Paul Goyette
2010-06-15 15:19:01 UTC
Permalink
Post by FUKAUMI Naoki
hi,
thank you for your work!
No problem - I needed to do it anyway because I needed to make my
AX88772 device work! Thanks for your work on the mii stuff - we should
be able to get that working fairly soon.
Post by FUKAUMI Naoki
Post by FUKAUMI Naoki
- in axe_init(), no need to set AXE_RXCMD_PROMISC and
AXE_RXCMD_BROADCAST individually.
Is the following correct?
/* If we want promiscuous mode, set the allframes bit. */
if (ifp->if_flags & (IFF_PROMISC | IFF_BROADCAST))
rxmode |= AXE_RXCMD_PROMISC |AXE_RXCMD_BROADCAST;
sorry, AXE_RXCMD_PROMISC is handled in axe_setmulti(), and
AXE_RXCMD_BROADCAST is handled few lines above in axe_init().
OK, so the following portion of axe_setmulti() is correct?

/* If we want promiscuous mode, set the allframes bit */
if (ifp->if_flags & IFF_PROMISC) {
rxmode |= AXE_RXCMD_PROMISC;
goto allmulti;
}
Post by FUKAUMI Naoki
Post by FUKAUMI Naoki
- in axe_ioctl(), call ifioctl_common() at first in SIOCSIFFLAGS case.
Do you mean to call ether_ioctl()? Is the following correct?
see if_aue.c:aue_ioctl:SIOCSIFFLAGS for example.
Ah, OK, got it!


-------------------------------------------------------------------------
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer | | pgoyette at netbsd.org |
-------------------------------------------------------------------------

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
FUKAUMI Naoki
2010-06-15 15:25:12 UTC
Permalink
Post by Paul Goyette
OK, so the following portion of axe_setmulti() is correct?
/* If we want promiscuous mode, set the allframes bit */
if (ifp->if_flags & IFF_PROMISC) {
rxmode |= AXE_RXCMD_PROMISC;
goto allmulti;
}
correct!

--
FUKAUMI Naoki

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
FUKAUMI Naoki
2010-06-16 11:17:02 UTC
Permalink
hi,

attached if_axe.c seems to be old.

please send latest one.

Regards,

--
FUKAUMI Naoki

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
FUKAUMI Naoki
2010-06-16 11:27:29 UTC
Permalink
thanks.

this is not objection, just a question.

what's happen if enabling mii stuff?

my patch enables it and working at least on my network.

--
FUKAUMI Naoki

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Paul Goyette
2010-06-16 11:39:39 UTC
Permalink
Post by FUKAUMI Naoki
thanks.
this is not objection, just a question.
what's happen if enabling mii stuff?
my patch enables it and working at least on my network.
In my case nothing happens. It appears that no packets ever get sent or
received. And ifconfig shows

media: Ethernet autoselect (none)

In my case, the phy is from vendor 0x007063 which is not even listed as
a valid OUI.


-------------------------------------------------------------------------
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer | | pgoyette at netbsd.org |
-------------------------------------------------------------------------

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Paul Goyette
2010-06-16 22:52:30 UTC
Permalink
Post by FUKAUMI Naoki
In my case nothing happens. It appears that no packets ever get sent or
received. And ifconfig shows
media: Ethernet autoselect (none)
In my case, the phy is from vendor 0x007063 which is not even listed as
a valid OUI.
it looks like lied/ghost phy problem. Is phy address correct in this case?
I think what you disabled is about media change/status functions. it may
cause media detect problem, but it should not cause register read problem.
I think the phy information is correct. I turned on AXE_DEBUG, and the
dmesg output is attached.




-------------------------------------------------------------------------
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer | | pgoyette at netbsd.org |
-------------------------------------------------------------------------
FUKAUMI Naoki
2010-06-17 00:29:36 UTC
Permalink
I think the phy information is correct. I turned on AXE_DEBUG, and the
dmesg output is attached.
ah, sorry. both vendor 0x007063 and phy addr 0x10 are same number as
disabled (working) case?

--
FUKAUMI Naoki

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Paul Goyette
2010-06-17 01:11:27 UTC
Permalink
I think the phy information is correct. I turned on AXE_DEBUG, and the
dmesg output is attached.
ah, sorry. both vendor 0x007063 and phy addr 0x10 are same number as disabled
(working) case?
The _entire_ output is the same from BOTH cases, including the phy info.



-------------------------------------------------------------------------
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer | | pgoyette at netbsd.org |
-------------------------------------------------------------------------

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
der Mouse
2010-06-16 12:53:13 UTC
Permalink
Post by Paul Goyette
And ifconfig shows
media: Ethernet autoselect (none)
That in itself is not necessarily a problem; I have a PCMCIA (or
similar - eg, I'm not sure of the difference between PCMCIA and
Cardbus) Ethernet that routinely shows media "none", even when it's in
actively carrying traffic. (Indeed, ifconfig -m on it lists nothing
but "none".)

Of course, in this case it may be more significant; I don't know axe.

/~\ The ASCII Mouse
\ / Ribbon Campaign
X Against HTML ***@rodents-montreal.org
/ \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
FUKAUMI Naoki
2010-06-16 14:04:28 UTC
Permalink
In my case nothing happens. It appears that no packets ever get sent or
received. And ifconfig shows
media: Ethernet autoselect (none)
In my case, the phy is from vendor 0x007063 which is not even listed as
a valid OUI.
it looks like lied/ghost phy problem. Is phy address correct in this case?

I think what you disabled is about media change/status functions. it may
cause media detect problem, but it should not cause register read problem.

--
FUKAUMI Naoki

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Izumi Tsutsui
2010-06-16 15:26:21 UTC
Permalink
My mistake - I intended to attach a diff file rather than the source
file.
I've got an axe(4) variant, and i386 kernel with DEBUG and DIAGNOSTIC gets:

---
axe0: at uhub5 port 4
axe0: ASIX Electronics AX88772 USB 2.0 10/100 Ethernet adapter, rev 2.00/0.01, addr 3
panic: kernel diagnostic assertion "mutex_owned(&sc->axe_mii_lock)" failed: \
file "../../../../dev/usb/if_axe.c", line 238
fatal brakepoint trap in supervisor mode
trap type 1 code 0 eip c01f25c4 cs 8 eflags 246 cr2 bbbd6924 ilevel 6
Stopped in pid 0.36 (system) at netbsd:breakpoint+0x4: popl %ebp
db{0}>
---

(have not investigated around the mutex)

---
Izumi Tsutsui

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Paul Goyette
2010-06-16 16:25:43 UTC
Permalink
I will investigate later today. My file/build server at home is down at
the moment, and I am at work.
Post by Izumi Tsutsui
My mistake - I intended to attach a diff file rather than the source
file.
---
axe0: at uhub5 port 4
axe0: ASIX Electronics AX88772 USB 2.0 10/100 Ethernet adapter, rev 2.00/0.01, addr 3
panic: kernel diagnostic assertion "mutex_owned(&sc->axe_mii_lock)" failed: \
file "../../../../dev/usb/if_axe.c", line 238
fatal brakepoint trap in supervisor mode
trap type 1 code 0 eip c01f25c4 cs 8 eflags 246 cr2 bbbd6924 ilevel 6
Stopped in pid 0.36 (system) at netbsd:breakpoint+0x4: popl %ebp
db{0}>
---
(have not investigated around the mutex)
---
Izumi Tsutsui
-------------------------------------------------------------------------
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer | | pgoyette at netbsd.org |
-------------------------------------------------------------------------

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
FUKAUMI Naoki
2010-06-16 17:46:42 UTC
Permalink
Post by Paul Goyette
I will investigate later today. My file/build server at home is down at
the moment, and I am at work.
in axe_attach(), axe_lock_mii() should be placed before
axe_cmd(AXE_CMD_READ_PHYID).

in axe_init(), lock/unlock should be added around
axe_cmd(AXE_178_CMD_WRITE_NODEID).


I think axe_{lock,unlock}_mii() can be reduced, but it can do after
commit. some part that you merged from my patch (incomplete, as I said;)
should be fixed too, but I don't think it's critial.

I have no objection to commit your code!

--
FUKAUMI Naoki

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Paul Goyette
2010-06-16 19:39:50 UTC
Permalink
Post by FUKAUMI Naoki
Post by Paul Goyette
I will investigate later today. My file/build server at home is down at
the moment, and I am at work.
in axe_attach(), axe_lock_mii() should be placed before
axe_cmd(AXE_CMD_READ_PHYID).
in axe_init(), lock/unlock should be added around
axe_cmd(AXE_178_CMD_WRITE_NODEID).
OK, I got my server back up and am building now.
Post by FUKAUMI Naoki
I think axe_{lock,unlock}_mii() can be reduced, but it can do after commit.
some part that you merged from my patch (incomplete, as I said;) should be
fixed too, but I don't think it's critial.
Yes, I agree that it is not critical.
Post by FUKAUMI Naoki
I have no objection to commit your code!
Well, I'm not sure it's "my" code - more likely it is "our" code, and
you will get credit in the commit message!

I should be able to commit by tomorrow.


-------------------------------------------------------------------------
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer | | pgoyette at netbsd.org |
-------------------------------------------------------------------------

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Izumi Tsutsui
2010-06-20 09:45:04 UTC
Permalink
Post by FUKAUMI Naoki
Post by Paul Goyette
I will investigate later today. My file/build server at home is down at
the moment, and I am at work.
in axe_attach(), axe_lock_mii() should be placed before
axe_cmd(AXE_CMD_READ_PHYID).
in axe_init(), lock/unlock should be added around
axe_cmd(AXE_178_CMD_WRITE_NODEID).
With these changes my AX88772 works:
---
axe0 at uhub4 port 2
axe0: ASIX Electronics AX88772 USB 2.0 10/100 Ethernet adapter, rev 2.00/0.01, addr 2
axe0: Ethernet address 00:90:cc:e8:15:7c
ukphy0 at axe0 phy 16: OUI 0x007063, model 0x0001, rev. 1
ukphy0: 10baseT, 10baseT-FDX, 100baseTX, 100baseTX-FDX, auto
---
Post by FUKAUMI Naoki
I think axe_{lock,unlock}_mii() can be reduced, but it can do after
commit. some part that you merged from my patch (incomplete, as I said;)
should be fixed too, but I don't think it's critial.
I'm not sure if we need splnet()/splx() in axe_attach(),
but anyway I think it's ready to commit.

---
Izumi Tsutsui

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
FUKAUMI Naoki
2010-06-20 15:31:49 UTC
Permalink
hi

sorry for late reply.
OK. Unless I hear any objections, I'll plan to commit the current
changes (see attached) later in the week.
I've tested my 3 axe(4)s with your latest patch.

1. planex UE-200TX-G2
axe0: vendor 0x0b95 product 0x7720, rev 2.00/0.01, addr 5
ukphy0 at axe0 phy 16: OUI 0x007063, model 0x0006, rev. 1
ukphy0: 10baseT, 10baseT-FDX, 100baseTX, 100baseTX-FDX, auto

2. planex UE-200TX-G
axe1: vendor 0x0b95 product 0x7720, rev 2.00/0.01, addr 6
ukphy1 at axe1 phy 16: OUI 0x007063, model 0x0001, rev. 1
ukphy1: 10baseT, 10baseT-FDX, 100baseTX, 100baseTX-FDX, auto

3. kuroutoshikou GbE-USB
axe2: vendor 0x0b95 product 0x1780, rev 2.00/0.01, addr 7
ukphy2 at axe2 phy 24: OUI 0x00c08f, model 0x000b, rev. 1
ukphy2: 10baseT, 10baseT-FDX, 100baseTX, 100baseTX-FDX, 1000baseT,
1000baseT-FDX, auto

all of them report correct media under "media autoselect" mode.
dhcpcd, ping, ssh are working.

no objection :)

--
FUKAUMI Naoki

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
FUKAUMI Naoki
2010-06-20 16:16:50 UTC
Permalink
panic() occurred while detaching.

callout_destroy() should be called *after* axe_stop() in axe_detach().

--
FUKAUMI Naoki

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Paul Goyette
2010-06-20 17:41:37 UTC
Permalink
Got it - I will include that when I commit in a couple of days.

Thanks!
Post by FUKAUMI Naoki
panic() occurred while detaching.
callout_destroy() should be called *after* axe_stop() in axe_detach().
--
FUKAUMI Naoki
-------------------------------------------------------------------------
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer | | pgoyette at netbsd.org |
-------------------------------------------------------------------------

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Loading...