On Wed, Jan 29, 2020 at 12:20:09PM +0530, Nagarjuna Kristam wrote: > > > On 28-01-2020 23:40, Thierry Reding wrote: > > > 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
Attachment:
signature.asc
Description: PGP signature