Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1397604 > unrolled thread

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

Started byChanwoo Choi <cw00.choi@samsung.com>
First post2016-05-10 03:00 +0200
Last post2016-05-12 12:30 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 0/3] power: Remove the deprecated extcon functions Chanwoo Choi <cw00.choi@samsung.com> - 2016-05-10 03:00 +0200
    Re: [PATCH v2 0/3] power: Remove the deprecated extcon functions Rob Herring <robh@kernel.org> - 2016-05-11 15:50 +0200
      Re: [PATCH v2 0/3] power: Remove the deprecated extcon functions Chanwoo Choi <cw00.choi@samsung.com> - 2016-05-12 12:30 +0200

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

FromChanwoo Choi <cw00.choi@samsung.com>
Date2016-05-10 03:00 +0200
SubjectRe: [PATCH v2 0/3] power: Remove the deprecated extcon functions
Message-ID<rx4bM-8tE-1@gated-at.bofh.it>
Ping.

Could you review this patch?

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(-)
>

[toc] | [next] | [standalone]

#1399100

FromRob Herring <robh@kernel.org>
Date2016-05-11 15:50 +0200
Message-ID<rxCGv-Pf-31@gated-at.bofh.it>
In reply to#1397604
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.

Rob

> 
> 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

[toc] | [prev] | [next] | [standalone]

#1399869

FromChanwoo Choi <cw00.choi@samsung.com>
Date2016-05-12 12:30 +0200
Message-ID<rxW2v-3Gh-17@gated-at.bofh.it>
In reply to#1399100
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
> 
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web