From: Kyungmin Park <[email protected]>
LP3974 PMIC support. It has same functionality with max8998.
Signed-off-by: Kyungmin Park <[email protected]>
---
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index db63d40..50383b1 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -303,6 +303,18 @@ config MFD_MAX8998
accessing the device, additional drivers must be enabled in order
to use the functionality of the device.
+config MFD_LP3974
+ bool "National Semiconductor LP3974 PMIC Support"
+ depends on I2C=y
+ select MFD_CORE
+ select MFD_MAX8998
+ help
+ Say yes here to support for National Semiconductor LP3974. This is
+ a Power Management IC. This driver provies common support for
+ accessing the device, additional drivers must be enabled in order
+ to use the functionality of the device.
+ Note that it has same functionality with max8998.
+
config MFD_WM8400
tristate "Support Wolfson Microelectronics WM8400"
select MFD_CORE
diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
index 0d68de2..cea9f48 100644
--- a/drivers/mfd/max8998.c
+++ b/drivers/mfd/max8998.c
@@ -30,6 +30,11 @@
#include <linux/mfd/max8998.h>
#include <linux/mfd/max8998-private.h>
+enum max8998_type {
+ TYPE_MAX8998,
+ TYPE_LP3974,
+};
+
static struct mfd_cell max8998_devs[] = {
{
.name = "max8998-pmic",
@@ -127,8 +132,8 @@ static int max8998_i2c_remove(struct i2c_client *i2c)
}
static const struct i2c_device_id max8998_i2c_id[] = {
- { "max8998", 0 },
- { }
+ { "max8998", TYPE_MAX8998 },
+ { "lp3974", TYPE_LP3974 },
};
MODULE_DEVICE_TABLE(i2c, max8998_i2c_id);
Any comments? I hope it's included the 2.6.36 if possible.
Thank you,
Kyungmin Park
On Mon, Aug 2, 2010 at 12:54 PM, Kyungmin Park <[email protected]> wrote:
> From: Kyungmin Park <[email protected]>
>
> LP3974 PMIC support. It has same functionality with max8998.
>
> Signed-off-by: Kyungmin Park <[email protected]>
> ---
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index db63d40..50383b1 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -303,6 +303,18 @@ config MFD_MAX8998
> ? ? ? ? ?accessing the device, additional drivers must be enabled in order
> ? ? ? ? ?to use the functionality of the device.
>
> +config MFD_LP3974
> + ? ? ? bool "National Semiconductor LP3974 PMIC Support"
> + ? ? ? depends on I2C=y
> + ? ? ? select MFD_CORE
> + ? ? ? select MFD_MAX8998
> + ? ? ? help
> + ? ? ? ? Say yes here to support for National Semiconductor LP3974. This is
> + ? ? ? ? a Power Management IC. This driver provies common support for
> + ? ? ? ? accessing the device, additional drivers must be enabled in order
> + ? ? ? ? to use the functionality of the device.
> + ? ? ? ? Note that it has same functionality with max8998.
> +
> ?config MFD_WM8400
> ? ? ? ?tristate "Support Wolfson Microelectronics WM8400"
> ? ? ? ?select MFD_CORE
> diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
> index 0d68de2..cea9f48 100644
> --- a/drivers/mfd/max8998.c
> +++ b/drivers/mfd/max8998.c
> @@ -30,6 +30,11 @@
> ?#include <linux/mfd/max8998.h>
> ?#include <linux/mfd/max8998-private.h>
>
> +enum max8998_type {
> + ? ? ? TYPE_MAX8998,
> + ? ? ? TYPE_LP3974,
> +};
> +
> ?static struct mfd_cell max8998_devs[] = {
> ? ? ? ?{
> ? ? ? ? ? ? ? ?.name = "max8998-pmic",
> @@ -127,8 +132,8 @@ static int max8998_i2c_remove(struct i2c_client *i2c)
> ?}
>
> ?static const struct i2c_device_id max8998_i2c_id[] = {
> - ? ? ? { "max8998", 0 },
> - ? ? ? { }
> + ? ? ? { "max8998", TYPE_MAX8998 },
> + ? ? ? { "lp3974", TYPE_LP3974 },
> ?};
> ?MODULE_DEVICE_TABLE(i2c, max8998_i2c_id);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>
On 08/19/10 06:08, Kyungmin Park wrote:
> Any comments? I hope it's included the 2.6.36 if possible.
One request for clarification below....
>
> Thank you,
> Kyungmin Park
>
> On Mon, Aug 2, 2010 at 12:54 PM, Kyungmin Park <[email protected]> wrote:
>> From: Kyungmin Park <[email protected]>
>>
>> LP3974 PMIC support. It has same functionality with max8998.
>>
>> Signed-off-by: Kyungmin Park <[email protected]>
>> ---
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index db63d40..50383b1 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -303,6 +303,18 @@ config MFD_MAX8998
>> accessing the device, additional drivers must be enabled in order
>> to use the functionality of the device.
>>
>> +config MFD_LP3974
>> + bool "National Semiconductor LP3974 PMIC Support"
>> + depends on I2C=y
>> + select MFD_CORE
>> + select MFD_MAX8998
>> + help
>> + Say yes here to support for National Semiconductor LP3974. This is
>> + a Power Management IC. This driver provies common support for
>> + accessing the device, additional drivers must be enabled in order
>> + to use the functionality of the device.
>> + Note that it has same functionality with max8998.
What is gained from adding a second kconfig option?
Numerous drivers throughout the kernel support very differently named parts, so why
not just change the text for the MFD_MAX8998 to say it supports this part as well?
>> +
>> config MFD_WM8400
>> tristate "Support Wolfson Microelectronics WM8400"
>> select MFD_CORE
>> diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
>> index 0d68de2..cea9f48 100644
>> --- a/drivers/mfd/max8998.c
>> +++ b/drivers/mfd/max8998.c
>> @@ -30,6 +30,11 @@
>> #include <linux/mfd/max8998.h>
>> #include <linux/mfd/max8998-private.h>
>>
>> +enum max8998_type {
>> + TYPE_MAX8998,
>> + TYPE_LP3974,
>> +};
>> +
>> static struct mfd_cell max8998_devs[] = {
>> {
>> .name = "max8998-pmic",
>> @@ -127,8 +132,8 @@ static int max8998_i2c_remove(struct i2c_client *i2c)
>> }
>>
>> static const struct i2c_device_id max8998_i2c_id[] = {
>> - { "max8998", 0 },
>> - { }
>> + { "max8998", TYPE_MAX8998 },
>> + { "lp3974", TYPE_LP3974 },
>> };
>> MODULE_DEVICE_TABLE(i2c, max8998_i2c_id);
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Thu, Aug 19, 2010 at 6:57 PM, Jonathan Cameron
<[email protected]> wrote:
> On 08/19/10 06:08, Kyungmin Park wrote:
>> Any comments? I hope it's included the 2.6.36 if possible.
> One request for clarification below....
>>
>> Thank you,
>> Kyungmin Park
>>
>> On Mon, Aug 2, 2010 at 12:54 PM, Kyungmin Park <[email protected]> wrote:
>>> From: Kyungmin Park <[email protected]>
>>>
>>> LP3974 PMIC support. It has same functionality with max8998.
>>>
>>> Signed-off-by: Kyungmin Park <[email protected]>
>>> ---
>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>>> index db63d40..50383b1 100644
>>> --- a/drivers/mfd/Kconfig
>>> +++ b/drivers/mfd/Kconfig
>>> @@ -303,6 +303,18 @@ config MFD_MAX8998
>>> ? ? ? ? ?accessing the device, additional drivers must be enabled in order
>>> ? ? ? ? ?to use the functionality of the device.
>>>
>>> +config MFD_LP3974
>>> + ? ? ? bool "National Semiconductor LP3974 PMIC Support"
>>> + ? ? ? depends on I2C=y
>>> + ? ? ? select MFD_CORE
>>> + ? ? ? select MFD_MAX8998
>>> + ? ? ? help
>>> + ? ? ? ? Say yes here to support for National Semiconductor LP3974. This is
>>> + ? ? ? ? a Power Management IC. This driver provies common support for
>>> + ? ? ? ? accessing the device, additional drivers must be enabled in order
>>> + ? ? ? ? to use the functionality of the device.
>>> + ? ? ? ? Note that it has same functionality with max8998.
> What is gained from adding a second kconfig option?
> Numerous drivers throughout the kernel support very differently named parts, so why
> not just change the text for the MFD_MAX8998 to say it supports this part as well?
I also consider that. If I got the LP3974 but can't find a config so
it's some confused.
Well, I will modify the MAX8998 comment. if you want
Thank you,
Kyungmin Park
>>> +
>>> ?config MFD_WM8400
>>> ? ? ? ?tristate "Support Wolfson Microelectronics WM8400"
>>> ? ? ? ?select MFD_CORE
>>> diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
>>> index 0d68de2..cea9f48 100644
>>> --- a/drivers/mfd/max8998.c
>>> +++ b/drivers/mfd/max8998.c
>>> @@ -30,6 +30,11 @@
>>> ?#include <linux/mfd/max8998.h>
>>> ?#include <linux/mfd/max8998-private.h>
>>>
>>> +enum max8998_type {
>>> + ? ? ? TYPE_MAX8998,
>>> + ? ? ? TYPE_LP3974,
>>> +};
>>> +
>>> ?static struct mfd_cell max8998_devs[] = {
>>> ? ? ? ?{
>>> ? ? ? ? ? ? ? ?.name = "max8998-pmic",
>>> @@ -127,8 +132,8 @@ static int max8998_i2c_remove(struct i2c_client *i2c)
>>> ?}
>>>
>>> ?static const struct i2c_device_id max8998_i2c_id[] = {
>>> - ? ? ? { "max8998", 0 },
>>> - ? ? ? { }
>>> + ? ? ? { "max8998", TYPE_MAX8998 },
>>> + ? ? ? { "lp3974", TYPE_LP3974 },
>>> ?};
>>> ?MODULE_DEVICE_TABLE(i2c, max8998_i2c_id);
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to [email protected]
>>> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at ?http://www.tux.org/lkml/
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at ?http://www.tux.org/lkml/
>
>
On 08/19/10 10:56, Kyungmin Park wrote:
> On Thu, Aug 19, 2010 at 6:57 PM, Jonathan Cameron
> <[email protected]> wrote:
>> On 08/19/10 06:08, Kyungmin Park wrote:
>>> Any comments? I hope it's included the 2.6.36 if possible.
>> One request for clarification below....
>>>
>>> Thank you,
>>> Kyungmin Park
>>>
>>> On Mon, Aug 2, 2010 at 12:54 PM, Kyungmin Park <[email protected]> wrote:
>>>> From: Kyungmin Park <[email protected]>
>>>>
>>>> LP3974 PMIC support. It has same functionality with max8998.
>>>>
>>>> Signed-off-by: Kyungmin Park <[email protected]>
>>>> ---
>>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>>>> index db63d40..50383b1 100644
>>>> --- a/drivers/mfd/Kconfig
>>>> +++ b/drivers/mfd/Kconfig
>>>> @@ -303,6 +303,18 @@ config MFD_MAX8998
>>>> accessing the device, additional drivers must be enabled in order
>>>> to use the functionality of the device.
>>>>
>>>> +config MFD_LP3974
>>>> + bool "National Semiconductor LP3974 PMIC Support"
>>>> + depends on I2C=y
>>>> + select MFD_CORE
>>>> + select MFD_MAX8998
>>>> + help
>>>> + Say yes here to support for National Semiconductor LP3974. This is
>>>> + a Power Management IC. This driver provies common support for
>>>> + accessing the device, additional drivers must be enabled in order
>>>> + to use the functionality of the device.
>>>> + Note that it has same functionality with max8998.
>> What is gained from adding a second kconfig option?
>> Numerous drivers throughout the kernel support very differently named parts, so why
>> not just change the text for the MFD_MAX8998 to say it supports this part as well?
>
> I also consider that. If I got the LP3974 but can't find a config so
> it's some confused.
> Well, I will modify the MAX8998 comment. if you want
Not my area of the kernel, so up to maintainers for what they would prefer.
Having worked on drivers supporting 10s of different parts I'm personally
anti this as I see it as bloat.
Actually, looking a little more closely...
Currently no use is being made of the enum.
You could turn this patch into a simple change to the Kconfig comment
and
{ "max8998", 0 },
+{ "lp3874", 0 },
{}
Unless there is a follow up patch using the enum, the usual principle
of don't add code that isn't used applies. Also, that makes the patch
even more trivial and so easier to merge post merge window.
Jonathan
>
> Thank you,
> Kyungmin Park
>
>>>> +
>>>> config MFD_WM8400
>>>> tristate "Support Wolfson Microelectronics WM8400"
>>>> select MFD_CORE
>>>> diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
>>>> index 0d68de2..cea9f48 100644
>>>> --- a/drivers/mfd/max8998.c
>>>> +++ b/drivers/mfd/max8998.c
>>>> @@ -30,6 +30,11 @@
>>>> #include <linux/mfd/max8998.h>
>>>> #include <linux/mfd/max8998-private.h>
>>>>
>>>> +enum max8998_type {
>>>> + TYPE_MAX8998,
>>>> + TYPE_LP3974,
>>>> +};
>>>> +
>>>> static struct mfd_cell max8998_devs[] = {
>>>> {
>>>> .name = "max8998-pmic",
>>>> @@ -127,8 +132,8 @@ static int max8998_i2c_remove(struct i2c_client *i2c)
>>>> }
>>>>
>>>> static const struct i2c_device_id max8998_i2c_id[] = {
>>>> - { "max8998", 0 },
>>>> - { }
>>>> + { "max8998", TYPE_MAX8998 },
>>>> + { "lp3974", TYPE_LP3974 },
>>>> };
>>>> MODULE_DEVICE_TABLE(i2c, max8998_i2c_id);
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>>> the body of a message to [email protected]
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>> Please read the FAQ at http://www.tux.org/lkml/
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at http://www.tux.org/lkml/
>>
>>
Hi Kyungmin,
On Thu, Aug 19, 2010 at 02:08:39PM +0900, Kyungmin Park wrote:
> Any comments? I hope it's included the 2.6.36 if possible.
It hopefully will be. I received the patch after the merge window opened
though.
Some comments on your patch:
> > +config MFD_LP3974
> > + ? ? ? bool "National Semiconductor LP3974 PMIC Support"
> > + ? ? ? depends on I2C=y
> > + ? ? ? select MFD_CORE
> > + ? ? ? select MFD_MAX8998
> > + ? ? ? help
> > + ? ? ? ? Say yes here to support for National Semiconductor LP3974. This is
> > + ? ? ? ? a Power Management IC. This driver provies common support for
> > + ? ? ? ? accessing the device, additional drivers must be enabled in order
> > + ? ? ? ? to use the functionality of the device.
> > + ? ? ? ? Note that it has same functionality with max8998.
As Jonathan mentioned, there really is no need for a Kconfig symbol. Just
change the existing Kconfig comments to add your device there.
> > ?config MFD_WM8400
> > ? ? ? ?tristate "Support Wolfson Microelectronics WM8400"
> > ? ? ? ?select MFD_CORE
> > diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
> > index 0d68de2..cea9f48 100644
> > --- a/drivers/mfd/max8998.c
> > +++ b/drivers/mfd/max8998.c
> > @@ -30,6 +30,11 @@
> > ?#include <linux/mfd/max8998.h>
> > ?#include <linux/mfd/max8998-private.h>
> >
> > +enum max8998_type {
> > + ? ? ? TYPE_MAX8998,
> > + ? ? ? TYPE_LP3974,
> > +};
Not used, so please remove that.
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/