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

[Michael Schmitz <schmitzmic@gmail.com>]

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