netdev - Re: [PATCH v2] Add support for GPIOs for SMSC LAN95xx chips.

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: <53AB0CB8.2090007@contactless.ru>
Date:	Wed, 25 Jun 2014 21:54:00 +0400
From:	Evgeny Boger <boger@...tactless.ru>
To:	Daniele Forsi <dforsi@...il.com>
CC:	Steve Glendinning <steve.glendinning@...well.net>,
	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	USB list <linux-usb@...r.kernel.org>
Subject: Re: [PATCH v2] Add support for GPIOs for SMSC LAN95xx chips.

06/25/2014 02:34 PM, Daniele Forsi пишет:
> 2014-06-25 6:46 GMT+02:00 Evgeny Boger:
>
>> There might be 11 GPIOs in total.
> do you mean "12 GPIOs"? You say later they are 0-based and the last one is "11"


>
>> Last three GPIOs  (offsets 8-11, 0-based) are shared with FDX, LNKA, SPD
>> LEDs respectively.
> so you mean the last "four"?
> and you may want to remove the extra space before the open parenthesis

My mistake. 11 GPIOs in total, eight of them (0-7) are normal GPIOs, 
while three other are multiplexed with link activity LEDs.
So it should read like "offset 8-10, 0-based".

Actually, the numbering scheme according to datasheets differs a bit 
between LAN9500 and LAN951x:

For LAN951x:
GPIOs with offsets 0-7 are named "GPIO3" - "GPIO7", offsets 8-10 are for 
"GPIO0" - "GPIO2" (these three are multiplexed with nFDX_LED, nLNKA_LED, 
nSPD_LED).

For LAN9500:
The datasheet name is the same as the corresponding offset, i.e. offsets 
0-10 are for "GPIO0"-"GPIO10".

What do you think, shoud I write some note about this numbering scheme? 
If so, where it would be better to place such a note?






>
> also there are still several unneeded newlines
>
>> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
>> @@ -68,6 +70,15 @@ struct smsc95xx_priv {
>>          spinlock_t mac_cr_lock;
>>          u8 features;
>>          u8 suspend_flags;
>> +
>> +       struct usbnet *dev;
>> +static int smsc95xx_gpio_request(struct gpio_chip *gpio, unsigned offset)
>> +       mutex_unlock(&pdata->gpio_lock);
>> +
>> +
>> +       return (ret < 0) ? ret : 0;
>> +}
>> +
>> +static void smsc95xx_gpio_free(struct gpio_chip *gpio, unsigned offset)
>> +       if (ret < 0)
>> +               netif_err(pdata->dev, ifdown, pdata->dev->net,
>> +                       "error freeing gpio %d\n", offset);
>> +
>> +}
> and in other places
>
>> +static void smsc95xx_gpio_set(struct gpio_chip *gpio, unsigned offset,
>> +                               int value)
>> +{
>> +       if (ret < 0) {
>> +               netif_err(pdata->dev, ifdown, pdata->dev->net,
>> +                       "error writing gpio %d=%d\n", offset, value);
>> +               return;
>> +       }
>> +}
> no need to put a "return" there at he end of the function (if it's
> defensive programming then you didn't put return in similar code in a
> previous function)
>


-- 
С уважением,
     Евгений Богер
     ООО Бесконтактные устройства
     http://contactless.ru
     +7 (919) 965 88 36

--
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