Re: [PATCH] w5300: add WIZnet W5300 Ethernet driver

[Taehun Kim]

2011/9/28 Francois Romieu <romieu@xxxxxxxxxxxxx>:

>> +/* 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 linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/