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