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

[Chanwoo Choi <cw00.choi@samsung.com>]

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@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
>