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]

Date:	Wed, 25 Jun 2014 12:34:23 +0200
From:	Daniele Forsi <dforsi@...il.com>
To:	Evgeny Boger <boger@...tactless.ru>
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.

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

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)

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