Re: [PATCH v2 0/3] power: Remove the deprecated extcon functions

From: Chanwoo Choi
Date: Thu May 12 2016 - 06:27:04 EST


Hi Rob,

On 2016ë 05ì 11ì 22:47, Rob Herring wrote:
> On Tue, May 10, 2016 at 09:54:58AM +0900, Chanwoo Choi wrote:
>> Ping.
>>
>> Could you review this patch?
>
> I already did. The first problem is you are breaking compatibility here
> (a kernel with these changes won't work with a dtb without these
> changes). As I previously said, this binding in general is horribly
> designed and full on Linux driver specifics. The first clue is your
> driver changes are resulting in DT changes. If you are going to break
> compatibility here, it better be redoing this binding. The problems I
> see with this binding are:
>
> - Linux device name strings
> - "charger-manager" is not a chip or circuit. DT describes the h/w.
> - Current limits by type of USB connection are pointless. These are part
> of the spec.
> - Properties need standard unit suffixes.
> - A mixture of battery and charger properties.

As I mentioned already on previous reply, I agree about your opinion absolutely.
The current charger-manager is not proper DT binding as you mentioned.

Just I want to remove the deprecated EXTCON API on charger-manager.
I don't have any thinking about improving the charger-manager driver.

As you mentioned, "charger-manager" is not a chip or circuit.
So, I think that new charging framework should be implemented
on power-supply framework with deleting charger-manager driver.
But, the update to change the "charger-manager" dt binding style
may be huge task. I think that we have to divide this task from removing
the deprecated EXTCON API.

Dear Sebastian,
If possible, I'd like to receive the opinion from you.

Regards,
Chanwoo Choi

>
>>
>> Thanks,
>> Chanwoo Choi
>>
>> On 2016ë 04ì 21ì 18:55, Chanwoo Choi wrote:
>>> This patch-set removes the deprecated notifier API of extcon framework and
>>> then use the new extcon API[2] with the unique id[1] to indicate the each
>>> external connector. Alter deprecated API as following:
>>> - extcon_register_interest() -> extcon_register_notifier()
>>> - extcon_unregister_interest() -> extcon_unregister_notifier()
>>> - extcon_set_cable_state() -> extcon_set_cable_state_()
>>> - extcon_get_cable_state() -> extcon_get_cable_state_()
>>>
>>> And, extcon alters the name of USB charger connector in patch[3] as following:
>>> - EXTCON_CHG_USB_SDP /* Standard Downstream Port */
>>> - EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
>>> - EXTCON_CHG_USB_CDP /* Charging Downstream Port */
>>> - EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */
>>>
>>> [1] Commit 2a9de9c0f08d61
>>> - ("extcon: Use the unique id for external connector instead of string)
>>> [2] Commit 046050f6e623e4
>>> - ("extcon: Update the prototype of extcon_register_notifier() with enum extcon
>>> [3] Commit 11eecf910bd81d
>>> - ("extcon: Modify the id and name of external connector")
>>>
>>> Changes from v1:
>>> - Fix the typo (EXTCON_CHG_USB_SDP -> EXTCON_CHG_USB_CDP) on axp288_charger.c
>>>
>>> Chanwoo Choi (3):
>>> power: charger-manager: Replace deprecatd API of extcon
>>> power: axp288_charger: Replace deprecatd API of extcon
>>> extcon: Remove the deprecated extcon functions
>>>
>>> .../bindings/power_supply/charger-manager.txt | 4 +-
>>> drivers/extcon/extcon.c | 201 +++------------------
>>> drivers/power/axp288_charger.c | 77 +++++---
>>> drivers/power/charger-manager.c | 31 ++--
>>> include/linux/extcon.h | 59 ------
>>> include/linux/power/charger-manager.h | 4 +-
>>> 6 files changed, 101 insertions(+), 275 deletions(-)
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>