Re: [patch] ep(4) panic

Maxime Henrion wrote:

> Ruslan Ermilov wrote:
> 
>>On Sat, Jun 25, 2005 at 01:48:42PM +0200, Maxime Henrion wrote:
>>
>>>Ruslan Ermilov wrote:
>>>
>>>>On Fri, Jun 24, 2005 at 05:09:04PM -0700, Maksim Yevmenkin wrote:
>>>>
>>>>>Dear Hackers,
>>>>>
>>>>>my -current panics when i insert ep(4) pcmcia network adapter. following 
>>>>>is the patch to fix the panic.
>>>>>
>>>>>--- if_ep_pccard.c.orig Fri Jun 10 12:04:44 2005
>>>>>+++ if_ep_pccard.c      Fri Jun 24 16:51:12 2005
>>>>>_at__at_ -73,6 +73,7 _at__at_
>>>>>        struct ep_board *epb = &sc->epb;
>>>>>        const char *desc;
>>>>>        uint16_t result;
>>>>>+       u_char eaddr[6];
>>>>>        int error;
>>>>>
>>>>>        error = ep_alloc(dev);
>>>>>_at__at_ -120,7 +121,7 _at__at_
>>>>>         * Newer cards supported by this device need to have their
>>>>>         * MAC address set.
>>>>>         */
>>>>>-       error = ep_get_macaddr(sc, (u_char *)&IFP2ENADDR(sc->ifp));
>>>>>+       error = ep_get_macaddr(sc, eaddr);
>>>>>
>>>>>        ep_free(dev);
>>>>>        return (0);
>>>>>
>>>>
>>>>That's odd.  You're saving the MAC address nowhere (it's reused
>>>>later).
>>>
>>>But ep_get_macaddr() does a little bit more than what its name suggests.
>>>
>>
>>I fail to see what else it does.  My point, on the other hand, is
>>that we should be saving it in IFP2ENADDR here, for later reuse.
> 
> 
> Hmm, I apparently read the code wrong.  It would be interesting to know
> if just removing the call the ep_get_macaddr() fixes the panic as well
> and lets the card work OK.  The comment above the call to
> ep_get_macaddr() looks weird given that ep_get_macaddr() does nothing
> more than reading the MAC address and doesn't apparently set it.

the way i understand it: this code is in the "probe" routine and when 
its called ifp structure was not allocated/setup yet. the "attach" 
routine will call "ep_attach" later that will allocate/setup ifp and 
read/set mac address (once again). so, the card works just fine.

it is interesting that my 4.x has different comment

$FreeBSD: /repoman/r/ncvs/src/sys/dev/ep/if_ep_pccard.c,v 1.12.2.3 
2003/10/06 02:53:51 imp Exp $

	/*
	 * For some reason the 3c574 needs this.
	 */
	ep_get_macaddr(sc, (u_char *)&sc->arpcom.ac_enaddr);


perhaps the comment in -current should be changed as well? can anyone 
please shed some light on this?

thanks,
max