linux-kernel - Re: [Patch V3 12/18] usb: gadget: tegra-xudc: support multiple device modes

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: <20200128181020.GJ2293590@ulmo>
Date:   Tue, 28 Jan 2020 19:10:20 +0100
From:   Thierry Reding <thierry.reding@...il.com>
To:     Nagarjuna Kristam <nkristam@...dia.com>
Cc:     balbi@...nel.org, gregkh@...uxfoundation.org, jonathanh@...dia.com,
        mark.rutland@....com, robh+dt@...nel.org, kishon@...com,
        devicetree@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [Patch V3 12/18] usb: gadget: tegra-xudc: support multiple
 device modes

On Mon, Dec 30, 2019 at 04:39:49PM +0530, Nagarjuna Kristam wrote:
> This change supports limited multiple device modes by:
> - At most 4 ports contains OTG/Device capability.
> - One port run as device mode at a time.
> 
> Signed-off-by: Nagarjuna Kristam <nkristam@...dia.com>
> ---
> V3:
>  - No changes in this version
> ---
> V2:
>  - Updated err variable on failure to get usbphy.
>  - Corrected identation after tegra_xudc_phy_get API call in tegra_xudc_probe.
> ---
>  drivers/usb/gadget/udc/tegra-xudc.c | 228 ++++++++++++++++++++++++++----------
>  1 file changed, 167 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/tegra-xudc.c b/drivers/usb/gadget/udc/tegra-xudc.c
> index 283c320..bf80fae 100644
> --- a/drivers/usb/gadget/udc/tegra-xudc.c
> +++ b/drivers/usb/gadget/udc/tegra-xudc.c
> @@ -483,14 +483,15 @@ struct tegra_xudc {
>  	bool device_mode;
>  	struct work_struct usb_role_sw_work;
>  
> -	struct phy *usb3_phy;
> -	struct phy *utmi_phy;
> +	struct phy **usb3_phy;
> +	struct phy **utmi_phy;
>  
>  	struct tegra_xudc_save_regs saved_regs;
>  	bool suspended;
>  	bool powergated;
>  
> -	struct usb_phy *usbphy;
> +	struct usb_phy **usbphy;
> +	int current_phy_index;

Can be unsigned int. It's also very long. It might be better to choose a
shorter name so that when you use it, the lines don't get excessively
long. Alternatively you could keep this field name and instead declare
local variables to reference the current PHY to make lines shorter.

Actually, looking at this a bit more, I don't see current_phy_index ever
used by itself (other than the assignment and one check to see if a PHY
has been selected). So why not just store a pointer to the current PHY
and avoid all the dereferencing?

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)