On Wed, Jan 29, 2020 at 12:20:09PM +0530, Nagarjuna Kristam wrote:

On 28-01-2020 23:40, Thierry Reding wrote:

quoted

quoted

  	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

current_phy_index main purpose is to quickly get which index for USB 2 and 3
phy's to be used. This is used at mulitple functions. Based on your comment
above, I believe its good to use 2 pointers for UTMI and USB 3 phy's, which
are points to current phy index. This ensures to keep line length as less as
possible.

Yes, I think in general it's preferable to use the objects directly
rather than an index into an array to find the objects.

Thierry