On 31 March 2016 at 18:06, Felipe Balbi <ba...@kernel.org> wrote: > Baolin Wang <baolin.w...@linaro.org> writes: >> [ text/plain ] >> On 31 March 2016 at 16:18, Felipe Balbi <ba...@kernel.org> wrote: >>> >>> Hi, >>> >>> Baolin Wang <baolin.w...@linaro.org> writes: >>>>>>>> +#define DEFAULT_SDP_CUR_LIMIT (500 - DEFAULT_CUR_PROTECT) >>>>>>> >>>>>>> According to the spec we should always be talking about unit loads (1 >>>>>>> unit load is 100mA for HS/FS/LS and 150mA for SS). Also, this will not >>>>>>> work for SS capable ports and SS gadgets (we have quite a few of them, >>>>>>> actually). You're missing the opportunity of charging at 900mA. >>>>>> >>>>>> I follow the DCP/SDP/CDP/ACA type's default current limitation and >>>>>> user can set them what they want. >>>>> >>>>> no, the user CANNOT set it to what they want. If you get enumerated >>>>> @100mA and the user just decides to set it to 2000mA, s/he could even >>>>> melt the USB connector. The kernel _must_ prevent such cases. >>>>> >>>>> In any case, DEFAULT_SDP_CUR_LIMIT shouldn't be a constant, it should be >>>>> variable because if you enumerate in SS, you _can_ get up to 900mA. >>>> >>>> Make sense. But these are just default values. They can be changed >>>> safely by power driver with 'usb_charger_set_cur_limit_by_type()' >>>> function to set 900mA. >>> >>> oh okay. Still, the default value should be a function of gadget->speed, >> >> Sorry, I did not get your suggestion, could you give me an example? Thanks. > > int default_current_limit = 500; > > if (gadget->speed >= USB_SPEED_SUPER) > default_current_limit = 900;
Make sense. > >>>>>>>> + >>>>>>>> +/* USB charger state */ >>>>>>>> +enum usb_charger_state { >>>>>>>> + USB_CHARGER_DEFAULT, >>>>>>>> + USB_CHARGER_PRESENT, >>>>>>>> + USB_CHARGER_REMOVE, >>>>>>>> +}; >>>>>>> >>>>>>> userland really doesn't need these two ? >>>>>> >>>>>> We've reported to userspace by kobject_uevent in >>>>>> 'usb_charger_notify_others()' function. >>>>> >>>>> I mean as a type ;-) So userspace doesn't have to redefine these for >>>>> their applications. >>>> >>>> Make sense. I can introduce some sysfs files for userspace. Thanks for >>>> your comments. >>> >>> okay, my reply was a bit cryptic, but what I mean here is that enum >>> usb_charger_state could be moved your include/uapi header. My question >>> is, then, does userland need to have knowledge of enum >>> usb_charger_state ? >> >> I am not sure if userland need the enum usb_charger_state. But I >> remember you want to report the charger state to userland in previous >> email. > > right, which means this enumeration definition could be placed in the > UAPI header. Unless, of course, we're reporting strings, rather than > integers, in the sysfs file. OK. Thanks. > > -- > balbi -- Baolin.wang Best Regards -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html