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  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:	Tue, 23 Jun 2015 08:54:09 +0200
From:	Krzysztof Opasiak <k.opasiak@...sung.com>
To:	Ruslan Bilovol <ruslan.bilovol@...il.com>, balbi@...com
Cc:	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	stern@...land.harvard.edu, peter.chen@...escale.com,
	gregkh@...uxfoundation.org, andrzej.p@...sung.com,
	maxime.ripard@...e-electrons.com
Subject: Re: [PATCH v5 2/5] usb: gadget: configfs: pass UDC name via
 usb_gadget_driver struct

Hello,

On 06/23/2015 12:01 AM, Ruslan Bilovol wrote:
> Now when udc-core supports binding to specific UDC by passing
> its name via 'udc_name' member of usb_gadget_driver struct,
> switch to this generic approach.
>
> Tested-by: Maxime Ripard <maxime.ripard@...e-electrons.com>
> Signed-off-by: Ruslan Bilovol <ruslan.bilovol@...il.com>
> ---
>   drivers/usb/gadget/configfs.c | 27 ++++++++++++++-------------
>   1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
> index c42765b..efad021 100644
> --- a/drivers/usb/gadget/configfs.c
> +++ b/drivers/usb/gadget/configfs.c
> @@ -54,7 +54,6 @@ struct gadget_info {
>   	struct list_head string_list;
>   	struct list_head available_func;
>
> -	const char *udc_name;
>   #ifdef CONFIG_USB_OTG
>   	struct usb_otg_descriptor otg;
>   #endif
> @@ -230,21 +229,21 @@ static ssize_t gadget_dev_desc_bcdUSB_store(struct gadget_info *gi,
>
>   static ssize_t gadget_dev_desc_UDC_show(struct gadget_info *gi, char *page)
>   {
> -	return sprintf(page, "%s\n", gi->udc_name ?: "");
> +	return sprintf(page, "%s\n", gi->composite.gadget_driver.udc_name ?: "");
>   }
>
>   static int unregister_gadget(struct gadget_info *gi)
>   {
>   	int ret;
>
> -	if (!gi->udc_name)
> +	if (!gi->composite.gadget_driver.udc_name)
>   		return -ENODEV;
>
>   	ret = usb_gadget_unregister_driver(&gi->composite.gadget_driver);
>   	if (ret)
>   		return ret;
> -	kfree(gi->udc_name);
> -	gi->udc_name = NULL;
> +	kfree(gi->composite.gadget_driver.udc_name);
> +	gi->composite.gadget_driver.udc_name = NULL;
>   	return 0;
>   }
>
> @@ -267,14 +266,16 @@ static ssize_t gadget_dev_desc_UDC_store(struct gadget_info *gi,
>   		if (ret)
>   			goto err;
>   	} else {
> -		if (gi->udc_name) {
> +		if (gi->composite.gadget_driver.udc_name) {

You are using this very long if condition in a few places, maybe it 
would be more suitable to define a macro or inline function for this? 
Something like gadget_dev_bound() or gadget_dev_is_active() or some 
other more suitable name.

Best regards,
-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists