Re: [Patch V2 03/18] phy: tegra: xusb: Add usb-role-switch support

Thierry Reding
2019-12-29T09:36:57Z

[-- Attachment #1: Type: text/plain, Size: 1164 bytes --]

On Fri, Dec 27, 2019 at 12:09:22PM +0530, Nagarjuna Kristam wrote:
> On 19-12-2019 18:56, Thierry Reding wrote:
> > On Wed, Dec 18, 2019 at 02:46:16PM +0530, Nagarjuna Kristam wrote:
[...]
> > > @@ -551,11 +556,42 @@ static const char *const modes[] = {
> > > [USB_DR_MODE_OTG] = "otg",
> > > };
> > > +static int tegra_xusb_role_sw_set(struct device *dev, enum usb_role role)
> > > +{
> > > + dev_dbg(dev, "%s calling notifier for role is %d\n", __func__, role);
> > I don't understand what "for role is %d" means here. I think perhaps you
> > meant to simply say "for role %d"? Also, perhaps add parentheses after
> > the "%s" to clarify that you're referring to a function.
> >
> Yes, intention is to print role, Will update as mentioned "for role %d"

It might be better to print the name of the role here because I suspect
very few people know the value of the role defines without looking them
up. I thought there was already a function to return the role name for
a given enum usb_role, but it seems like we only have name -> enum
conversion. Might be worth adding that, but doesn't have to be as part
of this series.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]