Re: [PATCH] power: supply: ltc3651-charger.c: Fix typo in device name

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05-04-19 17:11, Sebastian Reichel wrote:
> Hi,
> 
> On Fri, Apr 05, 2019 at 02:58:24PM +0000, Hennerich, Michael wrote:
>>> -----Original Message-----
>>> From: Sebastian Reichel [mailto:sre@xxxxxxxxxx]
>>> Sent: Freitag, 5. April 2019 16:38
>>> To: Hennerich, Michael <Michael.Hennerich@xxxxxxxxxx>
>>> Cc: mike.looijmans@xxxxxxxx; robh+dt@xxxxxxxxxx; linux-pm@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; Getz, Robin
>>> <Robin.Getz@xxxxxxxxxx>
>>> Subject: Re: [PATCH] power: supply: ltc3651-charger.c: Fix typo in device name
>>>
>>> Hi,
>>>
>>> On Tue, Mar 26, 2019 at 02:57:15PM +0100, michael.hennerich@xxxxxxxxxx wrote:
>>>> From: Michael Hennerich <michael.hennerich@xxxxxxxxxx>
>>>>
>>>> There never was a device called LTC3651, it always was just LT3651.
>>>> This circumstance makes it pretty difficult to identify what this
>>>> driver is meant to control. This patch is a minimal invasive attempt
>>>> to fix this issue without affecting existing systems.
>>>>
>>>>
>>>> Signed-off-by: Michael Hennerich <michael.hennerich@xxxxxxxxxx>
>>>> ---
>>>
>>> Please change subject to "power: supply: ltc3651-charger: Fix device name"
>>
>> Sure
>>
>>> Also we probably should change the filenames and Kconfig name.
>>
>> If we go that far we can also fix the function prefixes as well.
> 
> sounds good to me.
> 
>> I'm a bit hesitant about the Kconfig name. It'll break existing kernel configs
>> for no good reason!? Please let me know what you prefer.
> 
> I suppose we can keep to the old Kconfig option, if this is being
> used already. Hopefully not many people search for their driver
> using raw Kconfig names :)

Well, if you don't change it now, it will never get fixed.

I'm actually in favor of completely fixing the name. It does mean that I'll 
have to update a few devicetrees and configs, but in the long run, I think 
it's the better choice.


> 
> -- Sebastian
> 
>>
>> Thanks for the review.
>>
>> -- Michael
>>
>>>
>>> -- Sebastian
>>>
>>>>   .../devicetree/bindings/power/supply/ltc3651-charger.txt         | 8 +++++---
>>>>   drivers/power/supply/Kconfig                                     | 6 +++---
>>>>   drivers/power/supply/ltc3651-charger.c                           | 9 +++------
>>>>   3 files changed, 11 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/power/supply/ltc3651-charger.txt
>>> b/Documentation/devicetree/bindings/power/supply/ltc3651-charger.txt
>>>> index 71f2840..f012449 100644
>>>> --- a/Documentation/devicetree/bindings/power/supply/ltc3651-charger.txt
>>>> +++ b/Documentation/devicetree/bindings/power/supply/ltc3651-charger.txt
>>>> @@ -1,7 +1,9 @@
>>>> -ltc3651-charger
>>>> +Analog Devices LT3651 Charger Power Supply bindings: ltc3651-charger
>>>>
>>>>   Required properties:
>>>> - - compatible: "lltc,ltc3651-charger"
>>>> +- compatible: Should contain one of the following:
>>>> + * "lltc,ltc3651-charger",
>>>> + * "lltc,lt3651-charger"
>>>>    - lltc,acpr-gpios: Connect to ACPR output. See remark below.
>>>>
>>>>   Optional properties:
>>>> @@ -20,7 +22,7 @@ attributes to detect changes.
>>>>   Example:
>>>>
>>>>   	charger: battery-charger {
>>>> -		compatible = "lltc,ltc3651-charger";
>>>> +		compatible = "lltc,lt3651-charger";
>>>>   		lltc,acpr-gpios = <&gpio0 68 GPIO_ACTIVE_LOW>;
>>>>   		lltc,fault-gpios = <&gpio0 64 GPIO_ACTIVE_LOW>;
>>>>   		lltc,chrg-gpios = <&gpio0 63 GPIO_ACTIVE_LOW>;
>>>> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
>>>> index e901b9879..6816a86 100644
>>>> --- a/drivers/power/supply/Kconfig
>>>> +++ b/drivers/power/supply/Kconfig
>>>> @@ -476,11 +476,11 @@ config CHARGER_MANAGER
>>>>             with help of suspend_again support.
>>>>
>>>>   config CHARGER_LTC3651
>>>> -	tristate "LTC3651 charger"
>>>> +	tristate "Analog Devices LT3651 charger"
>>>>   	depends on GPIOLIB
>>>>   	help
>>>> -	  Say Y to include support for the LTC3651 battery charger which reports
>>>> -	  its status via GPIO lines.
>>>> +	  Say Y to include support for the Analog Devices (Linear Technology)
>>>> +	  LT3651 battery charger which reports its status via GPIO lines.
>>>>
>>>>   config CHARGER_MAX14577
>>>>   	tristate "Maxim MAX14577/77836 battery charger driver"
>>>> diff --git a/drivers/power/supply/ltc3651-charger.c b/drivers/power/supply/ltc3651-charger.c
>>>> index eea63ff..07ed537 100644
>>>> --- a/drivers/power/supply/ltc3651-charger.c
>>>> +++ b/drivers/power/supply/ltc3651-charger.c
>>>> @@ -1,11 +1,7 @@
>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>>   /*
>>>> + *  Driver for Analog Devices (Linear Technology) LT3651 charger IC.
>>>>    *  Copyright (C) 2017, Topic Embedded Products
>>>> - *  Driver for LTC3651 charger IC.
>>>> - *
>>>> - *  This program is free software; you can redistribute it and/or modify it
>>>> - *  under  the terms of the GNU General  Public License as published by the
>>>> - *  Free Software Foundation;  either version 2 of the License, or (at your
>>>> - *  option) any later version.
>>>>    */
>>>>
>>>>   #include <linux/device.h>
>>>> @@ -190,6 +186,7 @@ static int ltc3651_charger_probe(struct platform_device *pdev)
>>>>
>>>>   static const struct of_device_id ltc3651_charger_match[] = {
>>>>   	{ .compatible = "lltc,ltc3651-charger" },
>>>> +	{ .compatible = "lltc,lt3651-charger" },
>>>>   	{ }
>>>>   };
>>>>   MODULE_DEVICE_TABLE(of, ltc3651_charger_match);
>>>> --
>>>> 2.7.4
>>>>





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux