From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755616AbcCaIfO (ORCPT ); Thu, 31 Mar 2016 04:35:14 -0400 Received: from mail-yw0-f170.google.com ([209.85.161.170]:35433 "EHLO mail-yw0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755409AbcCaIfJ (ORCPT ); Thu, 31 Mar 2016 04:35:09 -0400 MIME-Version: 1.0 In-Reply-To: <87bn5vgiwx.fsf@intel.com> References: <11ce6df3eb8a95cfed26f3321f15c98a934db642.1458128215.git.baolin.wang@linaro.org> <87h9foqnur.fsf@intel.com> <87poubgnbh.fsf@intel.com> <87bn5vgiwx.fsf@intel.com> Date: Thu, 31 Mar 2016 16:35:03 +0800 Message-ID: Subject: Re: [PATCH v7 1/4] gadget: Introduce the usb charger framework From: Baolin Wang To: Felipe Balbi Cc: Greg KH , Sebastian Reichel , Dmitry Eremin-Solenikov , David Woodhouse , Peter Chen , Alan Stern , r.baldyga@samsung.com, Yoshihiro Shimoda , Lee Jones , Mark Brown , Charles Keepax , patches@opensource.wolfsonmicro.com, Linux PM list , USB , device-mainlining@lists.linuxfoundation.org, LKML Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 31 March 2016 at 16:18, Felipe Balbi wrote: > > Hi, > > Baolin Wang 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. > IMO ;-) > >>>>>> + >>>>>> +/* 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. > > -- > balbi -- Baolin.wang Best Regards