* [PATCH] MFD: LP3974 PMIC support
@ 2010-08-02 3:54 Kyungmin Park
2010-08-19 5:08 ` Kyungmin Park
0 siblings, 1 reply; 6+ messages in thread
From: Kyungmin Park @ 2010-08-02 3:54 UTC (permalink / raw)
To: linux-kernel; +Cc: sameo, m.szyprowski, broonie, jy0922.shim
From: Kyungmin Park <kyungmin.park@samsung.com>
LP3974 PMIC support. It has same functionality with max8998.
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
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);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] MFD: LP3974 PMIC support
2010-08-02 3:54 [PATCH] MFD: LP3974 PMIC support Kyungmin Park
@ 2010-08-19 5:08 ` Kyungmin Park
2010-08-19 9:57 ` Jonathan Cameron
2010-08-20 22:09 ` Samuel Ortiz
0 siblings, 2 replies; 6+ messages in thread
From: Kyungmin Park @ 2010-08-19 5:08 UTC (permalink / raw)
To: linux-kernel, Samuel Ortiz; +Cc: m.szyprowski, broonie, jy0922.shim
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 <kmpark@infradead.org> wrote:
> From: Kyungmin Park <kyungmin.park@samsung.com>
>
> LP3974 PMIC support. It has same functionality with max8998.
>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> 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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MFD: LP3974 PMIC support
2010-08-19 9:57 ` Jonathan Cameron
@ 2010-08-19 9:56 ` Kyungmin Park
2010-08-19 10:08 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Kyungmin Park @ 2010-08-19 9:56 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-kernel, Samuel Ortiz, m.szyprowski, broonie, jy0922.shim
On Thu, Aug 19, 2010 at 6:57 PM, Jonathan Cameron
<kernel@jic23.retrosnub.co.uk> 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 <kmpark@infradead.org> wrote:
>>> From: Kyungmin Park <kyungmin.park@samsung.com>
>>>
>>> LP3974 PMIC support. It has same functionality with max8998.
>>>
>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>> ---
>>> 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 majordomo@vger.kernel.org
>>> 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 majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MFD: LP3974 PMIC support
2010-08-19 5:08 ` Kyungmin Park
@ 2010-08-19 9:57 ` Jonathan Cameron
2010-08-19 9:56 ` Kyungmin Park
2010-08-20 22:09 ` Samuel Ortiz
1 sibling, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2010-08-19 9:57 UTC (permalink / raw)
To: Kyungmin Park
Cc: linux-kernel, Samuel Ortiz, m.szyprowski, broonie, jy0922.shim
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 <kmpark@infradead.org> wrote:
>> From: Kyungmin Park <kyungmin.park@samsung.com>
>>
>> LP3974 PMIC support. It has same functionality with max8998.
>>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>> 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 majordomo@vger.kernel.org
>> 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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MFD: LP3974 PMIC support
2010-08-19 9:56 ` Kyungmin Park
@ 2010-08-19 10:08 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2010-08-19 10:08 UTC (permalink / raw)
To: Kyungmin Park
Cc: linux-kernel, Samuel Ortiz, m.szyprowski, broonie, jy0922.shim
On 08/19/10 10:56, Kyungmin Park wrote:
> On Thu, Aug 19, 2010 at 6:57 PM, Jonathan Cameron
> <kernel@jic23.retrosnub.co.uk> 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 <kmpark@infradead.org> wrote:
>>>> From: Kyungmin Park <kyungmin.park@samsung.com>
>>>>
>>>> LP3974 PMIC support. It has same functionality with max8998.
>>>>
>>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>> ---
>>>> 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 majordomo@vger.kernel.org
>>>> 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 majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at http://www.tux.org/lkml/
>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MFD: LP3974 PMIC support
2010-08-19 5:08 ` Kyungmin Park
2010-08-19 9:57 ` Jonathan Cameron
@ 2010-08-20 22:09 ` Samuel Ortiz
1 sibling, 0 replies; 6+ messages in thread
From: Samuel Ortiz @ 2010-08-20 22:09 UTC (permalink / raw)
To: Kyungmin Park; +Cc: linux-kernel, m.szyprowski, broonie, jy0922.shim
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/
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-08-20 22:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-08-02 3:54 [PATCH] MFD: LP3974 PMIC support Kyungmin Park
2010-08-19 5:08 ` Kyungmin Park
2010-08-19 9:57 ` Jonathan Cameron
2010-08-19 9:56 ` Kyungmin Park
2010-08-19 10:08 ` Jonathan Cameron
2010-08-20 22:09 ` Samuel Ortiz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).