netdev - Re: [PATCH net-next v5 2/2] net/8390: apne.c - add 100 Mbit support to apne.c driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <458fe9a3-9a34-d35e-3559-7de498d8f28b@gmail.com>
Date:   Sat, 19 Jun 2021 15:13:35 +1200
From:   Michael Schmitz <schmitzmic@...il.com>
To:     Finn Thain <fthain@...ux-m68k.org>
Cc:     linux-m68k@...r.kernel.org, geert@...ux-m68k.org, alex@...ik.de,
        netdev@...r.kernel.org
Subject: Re: [PATCH net-next v5 2/2] net/8390: apne.c - add 100 Mbit support
 to apne.c driver

Hi Finn,

thanks for reviewing again!

Am 19.06.2021 um 12:56 schrieb Finn Thain:
>>
>> +	/* Reset card. Who knows what dain-bramaged state it was left in. */
>> +	{	unsigned long reset_start_time = jiffies;
>
> There's a missing line break here.

Straight copy from apne_probe1() below, but you're right about style of 
course.

>
>> +
>> +		outb(inb(IOBASE + NE_RESET), IOBASE + NE_RESET);
>> +
>> +		while ((inb(IOBASE + NE_EN0_ISR) & ENISR_RESET) == 0)
>> +			if (time_after(jiffies, reset_start_time + 2*HZ/100)) {
>
> You could use msecs_to_jiffies(20) here.
>
>> +				pr_info("Card not found (no reset ack).\n");
>> +				isa_type=ISA_TYPE_AG16;
>
> Whitespace is needed around the '='.
>
>> +			}
>
> Missing a break statement?

Ouch.

>
>> +
>> +		outb(0xff, IOBASE + NE_EN0_ISR);		/* Ack all intr. */
>> +	}
>> +
>>  	dev = alloc_ei_netdev();
>>  	if (!dev)
>>  		return ERR_PTR(-ENOMEM);
>> @@ -590,6 +613,16 @@ static int init_pcmcia(void)
>>  #endif
>>  	u_long offset;
>>
>> +	/* reset card (idea taken from CardReset by Artur Pogoda) */
>> +	if (isa_type == ISA_TYPE_AG16) {
>> +		u_char  tmp = gayle.intreq;
>> +
>
> Extra whitespace.
>
>> +		gayle.intreq = 0xff;
>> +		mdelay(1);
>> +		gayle.intreq = tmp;
>> +		mdelay(300);
>> +	}
>> +
>>  	pcmcia_reset();
>>  	pcmcia_program_voltage(PCMCIA_0V);
>>  	pcmcia_access_speed(PCMCIA_SPEED_250NS);
>>

Thanks, will fix in v6 (and correct the commit message). Might split off 
the autoprobe bit so that can be easily dropped if need be.

Cheers,

	Michael