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

[Thierry Reding]

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

Attachment: signature.asc
Description: PGP signature