linux-kernel - Re: [PATCH v2] ethernet driver for the WIZnet W5300 chip

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: <4F6ACD82.2030505@permonline.ru>
Date:	Thu, 22 Mar 2012 12:58:10 +0600
From:	Mike Sinkovsky <msink@...monline.ru>
To:	Florian Fainelli <florian@...nwrt.org>
CC:	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	msink@...monline.ru
Subject: Re: [PATCH v2] ethernet driver for the WIZnet W5300 chip

21.03.2012 20:49, Florian Fainelli wrote:
>> +#if defined(CONFIG_WIZNET_BUS_DIRECT)
>> +#define detect_bus_mode(priv, mem_size) do {} while(0)
>> +#define read_reg_u16    read_u16_direct
>> +#define write_reg_u16    write_u16_direct
>> +
>> +#elif defined(CONFIG_WIZNET_BUS_INDIRECT)
>> +#define detect_bus_mode(priv, mem_size) do {} while(0)
>> +#define read_reg_u16    read_u16_indirect
>> +#define write_reg_u16    write_u16_indirect
> Looks like you don't have to make such decisions at compile-time. 
> Since it is a platform driver, better supply this through 
> platform_data instead.
Interface bus mode can be selected in .config (make menuconfig etc.)
If don't selected explicitly in .config, platform_device.resource[] is 
used in runtime to setup callbacks.
This hack was done for performance reasons, but may be not necessary, 
I'm not sure.


> Please implement phylib to properly report the link state to the 
> networking stack and ethtool.
This chip don't allow any access to phy interface, it completely hidden 
for host processor.
The only way I found - to wire pin LINK from W5300 to some GPIO, then 
ise RIGING/FALLING interrupt for tthat GPIO:
> +static irqreturn_t w5300_detect_link(int irq, void *ndev_instance)
> +{
> +        struct net_device *ndev = ndev_instance;
> +        struct w5300_private *priv = netdev_priv(ndev);
> +
> +        if (netif_running(ndev)) {
> +                if (gpio_get_value(priv->link) == 0)
> +                        netif_carrier_off(ndev);
> +                else
> +                        netif_carrier_on(ndev);
> +        }
> +
> +        return IRQ_HANDLED;
> +}
Isn't it enough for networking stack?


> Allow platform_data to pass a valid MAC address to this driver instead 
> of defaulting to random unconditionnaly.
>
Do you mean - create .h file in /include/linux/platform_data/ ?
Ok, will do in next version.

Thanks.

---
Mike


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/