netdev - Re: [PATCH] w5300: add WIZnet W5300 Ethernet 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] [day] [month] [year] [list]

Message-ID: <CAOkfWjfujtrFLd8HFLVC_jAQ98o8BkZNg0zGSbceFoTt9Cw6fg@mail.gmail.com>
Date:	Mon, 3 Oct 2011 18:15:54 +0900
From:	Taehun Kim <kth3321@...il.com>
To:	Francois Romieu <romieu@...zoreil.com>
Cc:	netdev <netdev@...r.kernel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] w5300: add WIZnet W5300 Ethernet driver

2011/9/28 Francois Romieu <romieu@...zoreil.com>:

>> +/* Opening channels of W5300 */
>> +static int
>> +w5300_open(struct wiz_private *wp, u32 type)
>
> Name it w5300_channel_open or more adequately w5300_channel_0_open
> as it is hardwired to channel 0 ?

The W5300 Ethernet function is only available in channel 0. The other channels
(1~7) is used hardwired TCP/IP stack.
I was limited to the Ethernet function in this driver because hardwired TCP/IP
stack to use the TCP/IP stack in linux kernel be modified.

>> +     int s;
>> +
>> +     isr = w5300_read(wp, IR);
>> +
>> +     /* Completing all interrupts at a time. */
>> +     while (isr) {
>> +             w5300_write(wp, IR, isr);
>> +
>> +             /* Finding the channel to create the interrupt */
>> +             s = find_first_bit(&isr, sizeof(u16));
>> +             ssr = w5300_read(wp, Sn_IR(s));
>> +             /* socket interrupt is cleared. */
>> +             w5300_write(wp, Sn_IR(s), ssr);
>> +             DPRINTK("%s: ISR = %X, SSR = %X, s = %X\n",
>> +                     __func__, isr, ssr, s);
>> +             if (likely(!s)) {
>> +                     if (ssr & Sn_IR_RECV) {
>> +                             /* De-activation of interrupt */
>> +                             w5300_interrupt_disable(wp, 0);
>> +                             /* Receiving by polling method */
>> +                             napi_schedule(&wp->napi);
>> +                     }
>> +             }
>> +
>> +             /* Is there any interrupt to be processed? */
>> +             isr = w5300_read(wp, IR);
>> +     }
>> +
>> +     return IRQ_HANDLED;
>
> The hardware implementation is supposed to enforce that the 'while' is
> always entered, right ?
>

If isr is 0, it is to ensure that the W5300 is received an interrupt responses.
You mean that there is a need to the loop exit routine. right?

Thank you for your detailed feedback.

I will rewrite the driver code by applied your feedback. When the work is
completed, I will suggest the rewritten driver code to this thread.

Thank you.

T.K.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html