Re: [PATCH v12 1/4] gadget: Introduce the usb charger framework

[Baolin Wang]

On 21 June 2016 at 18:25, Felipe Balbi <balbi@xxxxxxxxxx> wrote:
>
> Hi,
>
> Baolin Wang <baolin.wang@xxxxxxxxxx> writes:
>> This patch introduces the usb charger driver based on usb gadget that
>> makes an enhancement to a power driver. It works well in practice but
>> that requires a system with suitable hardware.
>>
>> The basic conception of the usb charger is that, when one usb charger
>> is added or removed by reporting from the usb gadget state change or
>> the extcon device state change, the usb charger will report to power
>> user to set the current limitation.
>>
>> The usb charger will register notifiees on the usb gadget or the extcon
>> device to get notified the usb charger state. It also supplies the
>> notification mechanism to userspace When the usb charger state is changed.
>>
>> Power user will register a notifiee on the usb charger to get notified
>> by status changes from the usb charger. It will report to power user
>> to set the current limitation when detecting the usb charger is added
>> or removed from extcon device state or usb gadget state.
>>
>> This patch doesn't yet integrate with the gadget code, so some functions
>> which rely on the 'gadget' are not completed, that will be implemented
>> in the following patches.
>>
>> Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxx>
>> Reviewed-by: Li Jun <jun.li@xxxxxxx>
>> Tested-by: Li Jun <jun.li@xxxxxxx>
>> ---
>>  drivers/usb/gadget/Kconfig       |    7 +
>>  drivers/usb/gadget/udc/Makefile  |    1 +
>>  drivers/usb/gadget/udc/charger.c |  770 ++++++++++++++++++++++++++++++++++++++
>>  include/linux/usb/charger.h      |  191 ++++++++++
>>  include/uapi/linux/usb/charger.h |   31 ++
>>  5 files changed, 1000 insertions(+)
>>  create mode 100644 drivers/usb/gadget/udc/charger.c
>>  create mode 100644 include/linux/usb/charger.h
>>  create mode 100644 include/uapi/linux/usb/charger.h
>>
>> diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
>> index 2057add..89f4e9b 100644
>> --- a/drivers/usb/gadget/Kconfig
>> +++ b/drivers/usb/gadget/Kconfig
>> @@ -134,6 +134,13 @@ config U_SERIAL_CONSOLE
>>       help
>>          It supports the serial gadget can be used as a console.
>>
>> +config USB_CHARGER
>> +     bool "USB charger support"
>
> you didn't build test all possibilities, did you?
>
> I have a feeling this won't link if USB_GADGET=m. Can you test that?

OK.

>
>> diff --git a/drivers/usb/gadget/udc/charger.c b/drivers/usb/gadget/udc/charger.c
> [...]
>> +struct class *usb_charger_class;
>
> We already have a UDC class, do we really, really need another class
> here?

We want to manage the usb charger devices by this class and one usb
charger device is not equal with one usb device which managed by UDC
class, so can we use UDC class to manage charger devices?
By the way, you also suggested to use the 'class' things instead of
'bus' in previous mail.

>
>> +subsys_initcall(usb_charger_class_init);
>
> this should always work as module_init(). Please make sure that's the
> case.

we should make sure the charger class has been allocated before user
try to add a new gadget to the udc class. So it should be issued
before 'module_init()' (many usb drivers are at module_init level).
Another hand this follows the UDC class allocation.

>
> --
> balbi



-- 
Baolin.wang
Best Regards