On Thu, 18 Jun 2015 15:55:28 +0800 Li Jun <b47624@xxxxxxxxxxxxx> wrote: > On Thu, Jun 18, 2015 at 10:36:50AM +0300, Roger Quadros wrote: > > Lin, > > > > You can use --in-reply-to "message id of v5 of this path" so that it appears together > > with the other patches in peoples mailboxes. > > > okay, I will try that next time, thanks. > > Li Jun > > On Thu, 18 Jun 2015 09:18:22 +0800 > > Li Jun <jun.li@xxxxxxxxxxxxx> wrote: > > > > > Change for v6: > > > UPdate otg caps by disable flags even if otg_rev is not passed, this is > > > needed in case user wants to disable whole OTG(or just want to ID pin > > > detect). > > > > > > Check property of usb hardware to get otg version and if SRP, HNP and ADP > > > are disabled. > > > > > > Signed-off-by: Li Jun <jun.li@xxxxxxxxxxxxx> > > > --- > > > drivers/usb/common/common.c | 27 +++++++++++++++++++++++++++ > > > include/linux/usb/of.h | 7 +++++++ > > > 2 files changed, 34 insertions(+) > > > > > > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c > > > index b530fd4..12f5d28 100644 > > > --- a/drivers/usb/common/common.c > > > +++ b/drivers/usb/common/common.c > > > @@ -154,6 +154,33 @@ bool of_usb_host_tpl_support(struct device_node *np) > > > return false; > > > } > > > EXPORT_SYMBOL_GPL(of_usb_host_tpl_support); > > > + > > > +/** > > > + * of_usb_get_set_caps - to set usb otg capabilities according to > > > > get or set? > > how about update instead? > > > Should be set, update? okay. > > Li Jun > > > + * the passed properties in DT. > > > + * @np: Pointer to the given device_node > > > + * @otg_caps: Pointer to the target usb_otg_caps to be set > > > + * > > > + * The function gets and sets the otg capabilities > > > + */ > > > +void of_usb_set_otg_caps(struct device_node *np, struct usb_otg_caps *otg_caps) > > > +{ > > > + u32 otg_rev; > > > + > > > + if (!otg_caps) > > > + return; > > > + > > > + if (!of_property_read_u32(np, "otg-rev", &otg_rev)) > > > + otg_caps->otg_rev = otg_rev; > > > > should we check if otg_rev is in correct format? > > anything non BCD and greater than 0x9999 is invalid. > > > Then that's not so simple, every number should be checked, > if ((otg_caps->otg_rev % 10) > 9) || > ((otg_caps->otg_rev >> 4) % 10) > 9) || > ((otg_caps->otg_rev >> 8) % 10) > 9) || > ((otg_caps->otg_rev >> 12) % 10) > 9)) > return error; > We really need this kind of format check? seems no this check > for bcdUSB. How about checking for released spec versions instead? switch (otg_rev) { case 0x0100: case 0x0110: case 0x0200: case 0x0300: break; default: dev_err(dev, "unsupported otg-rev: 0x%x\n", otg_rev); return error; } cheers, -roger > > > Also if otg-rev is not passed then we need to treat it as legacy > > platform right? How is this taken care of? > > > > > + if (of_find_property(np, "hnp-disable", NULL)) > > > + otg_caps->hnp_support = false; > > > + if (of_find_property(np, "srp-disable", NULL)) > > > + otg_caps->srp_support = false; > > > + if (of_find_property(np, "adp-disable", NULL)) > > > + otg_caps->adp_support = false; > > > > Don't we expect it to do basic sanity checks? > > e.g. if otg_rev < 0x200 then adp_support can't be true? > > > Okay, it's better we add a sanity check here. > > > > +} > > > +EXPORT_SYMBOL_GPL(of_usb_set_otg_caps); > > > + > > > #endif > > > > > > MODULE_LICENSE("GPL"); > > > diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h > > > index cfe0528..6339799 100644 > > > --- a/include/linux/usb/of.h > > > +++ b/include/linux/usb/of.h > > > @@ -15,6 +15,8 @@ > > > enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np); > > > enum usb_device_speed of_usb_get_maximum_speed(struct device_node *np); > > > bool of_usb_host_tpl_support(struct device_node *np); > > > +void of_usb_set_otg_caps(struct device_node *np, > > > + struct usb_otg_caps *otg_caps); > > > #else > > > static inline enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np) > > > { > > > @@ -30,6 +32,11 @@ static inline bool of_usb_host_tpl_support(struct device_node *np) > > > { > > > return false; > > > } > > > +static inline void of_usb_set_otg_caps(struct device_node *np, > > > + struct usb_otg_caps *otg_caps) > > > +{ > > > + > > > +} > > > #endif > > > > > > #if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_USB_SUPPORT) > > > -- > > > 1.9.1 > > > > > > > cheers, > > -roger -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html