* [PATCH 1/3] MFD: MAX8997: IRQ Handling Bugfix @ 2011-06-30 1:31 MyungJoo Ham 2011-06-30 1:31 ` [PATCH 2/3] MFD: MAX8997: Support Wake-up from Suspend MyungJoo Ham 2011-06-30 1:31 ` [PATCH 3/3] MFD: MAX8997: IRQ definition moved to public header MyungJoo Ham 0 siblings, 2 replies; 14+ messages in thread From: MyungJoo Ham @ 2011-06-30 1:31 UTC (permalink / raw) To: linux-kernel Cc: Samuel Ortiz, Kyungmin Park, Mark Brown, Liam Girdwood, Donggeun Kim, myungjoo.ham Required platform information is not handed to max8997-irq.c properly. This patch enables to hand over such information to max8997-irq.c so that max8997-irq functions properly. Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> --- drivers/mfd/max8997.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c index 5d1fca0..f83103b 100644 --- a/drivers/mfd/max8997.c +++ b/drivers/mfd/max8997.c @@ -135,10 +135,13 @@ static int max8997_i2c_probe(struct i2c_client *i2c, max8997->dev = &i2c->dev; max8997->i2c = i2c; max8997->type = id->driver_data; + max8997->irq = i2c->irq; if (!pdata) goto err; + max8997->irq_base = pdata->irq_base; + max8997->ono = pdata->ono; max8997->wakeup = pdata->wakeup; mutex_init(&max8997->iolock); @@ -152,6 +155,8 @@ static int max8997_i2c_probe(struct i2c_client *i2c, pm_runtime_set_active(max8997->dev); + max8997_irq_init(max8997); + mfd_add_devices(max8997->dev, -1, max8997_devs, ARRAY_SIZE(max8997_devs), NULL, 0); -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] MFD: MAX8997: Support Wake-up from Suspend 2011-06-30 1:31 [PATCH 1/3] MFD: MAX8997: IRQ Handling Bugfix MyungJoo Ham @ 2011-06-30 1:31 ` MyungJoo Ham 2011-06-30 1:31 ` [PATCH 3/3] MFD: MAX8997: IRQ definition moved to public header MyungJoo Ham 1 sibling, 0 replies; 14+ messages in thread From: MyungJoo Ham @ 2011-06-30 1:31 UTC (permalink / raw) To: linux-kernel Cc: Samuel Ortiz, Kyungmin Park, Mark Brown, Liam Girdwood, Donggeun Kim, myungjoo.ham - Support wake-up from suspend-to-ram. - Handle pending interrupt after a resume. Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> --- drivers/mfd/max8997.c | 23 +++++++++++++++++++++++ 1 files changed, 23 insertions(+), 0 deletions(-) diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c index f83103b..4ae42c6 100644 --- a/drivers/mfd/max8997.c +++ b/drivers/mfd/max8997.c @@ -23,6 +23,7 @@ #include <linux/slab.h> #include <linux/i2c.h> +#include <linux/interrupt.h> #include <linux/pm_runtime.h> #include <linux/mutex.h> #include <linux/mfd/core.h> @@ -398,7 +399,29 @@ static int max8997_restore(struct device *dev) return 0; } +static int max8997_suspend(struct device *dev) +{ + struct i2c_client *i2c = container_of(dev, struct i2c_client, dev); + struct max8997_dev *max8997 = i2c_get_clientdata(i2c); + + if (max8997->wakeup && max8997->irq) + irq_set_irq_wake(max8997->irq, 1); + return 0; +} + +static int max8997_resume(struct device *dev) +{ + struct i2c_client *i2c = container_of(dev, struct i2c_client, dev); + struct max8997_dev *max8997 = i2c_get_clientdata(i2c); + + if (max8997->wakeup && max8997->irq) + irq_set_irq_wake(max8997->irq, 0); + return max8997_irq_resume(max8997); +} + const struct dev_pm_ops max8997_pm = { + .suspend = max8997_suspend, + .resume = max8997_resume, .freeze = max8997_freeze, .restore = max8997_restore, }; -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] MFD: MAX8997: IRQ definition moved to public header. 2011-06-30 1:31 [PATCH 1/3] MFD: MAX8997: IRQ Handling Bugfix MyungJoo Ham 2011-06-30 1:31 ` [PATCH 2/3] MFD: MAX8997: Support Wake-up from Suspend MyungJoo Ham @ 2011-06-30 1:31 ` MyungJoo Ham 2011-06-30 5:28 ` Mark Brown 1 sibling, 1 reply; 14+ messages in thread From: MyungJoo Ham @ 2011-06-30 1:31 UTC (permalink / raw) To: linux-kernel Cc: Samuel Ortiz, Kyungmin Park, Mark Brown, Liam Girdwood, Donggeun Kim, myungjoo.ham IRQ definitions are needed to be accessed by board support package codes as well. Because they are not only used by MAX8997 MFD device drivers, this patch pulls such definitions out of private header, which is meant for MAX8997 MFD device drivers. Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> --- include/linux/mfd/max8997-private.h | 46 ----------------------------------- include/linux/mfd/max8997.h | 46 +++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 46 deletions(-) diff --git a/include/linux/mfd/max8997-private.h b/include/linux/mfd/max8997-private.h index 5ff2400..4e50a62 100644 --- a/include/linux/mfd/max8997-private.h +++ b/include/linux/mfd/max8997-private.h @@ -265,52 +265,6 @@ enum max8997_irq_source { MAX8997_IRQ_GROUP_NR, }; -enum max8997_irq { - MAX8997_PMICIRQ_PWRONR, - MAX8997_PMICIRQ_PWRONF, - MAX8997_PMICIRQ_PWRON1SEC, - MAX8997_PMICIRQ_JIGONR, - MAX8997_PMICIRQ_JIGONF, - MAX8997_PMICIRQ_LOWBAT2, - MAX8997_PMICIRQ_LOWBAT1, - - MAX8997_PMICIRQ_JIGR, - MAX8997_PMICIRQ_JIGF, - MAX8997_PMICIRQ_MR, - MAX8997_PMICIRQ_DVS1OK, - MAX8997_PMICIRQ_DVS2OK, - MAX8997_PMICIRQ_DVS3OK, - MAX8997_PMICIRQ_DVS4OK, - - MAX8997_PMICIRQ_CHGINS, - MAX8997_PMICIRQ_CHGRM, - MAX8997_PMICIRQ_DCINOVP, - MAX8997_PMICIRQ_TOPOFFR, - MAX8997_PMICIRQ_CHGRSTF, - MAX8997_PMICIRQ_MBCHGTMEXPD, - - MAX8997_PMICIRQ_RTC60S, - MAX8997_PMICIRQ_RTCA1, - MAX8997_PMICIRQ_RTCA2, - MAX8997_PMICIRQ_SMPL_INT, - MAX8997_PMICIRQ_RTC1S, - MAX8997_PMICIRQ_WTSR, - - MAX8997_MUICIRQ_ADCError, - MAX8997_MUICIRQ_ADCLow, - MAX8997_MUICIRQ_ADC, - - MAX8997_MUICIRQ_VBVolt, - MAX8997_MUICIRQ_DBChg, - MAX8997_MUICIRQ_DCDTmr, - MAX8997_MUICIRQ_ChgDetRun, - MAX8997_MUICIRQ_ChgTyp, - - MAX8997_MUICIRQ_OVP, - - MAX8997_IRQ_NR, -}; - #define MAX8997_NUM_GPIO 12 struct max8997_dev { struct device *dev; diff --git a/include/linux/mfd/max8997.h b/include/linux/mfd/max8997.h index 0bbd13d..90e7731 100644 --- a/include/linux/mfd/max8997.h +++ b/include/linux/mfd/max8997.h @@ -119,4 +119,50 @@ struct max8997_platform_data { /* Flash: Not implemented */ }; +enum max8997_irq { + MAX8997_PMICIRQ_PWRONR, + MAX8997_PMICIRQ_PWRONF, + MAX8997_PMICIRQ_PWRON1SEC, + MAX8997_PMICIRQ_JIGONR, + MAX8997_PMICIRQ_JIGONF, + MAX8997_PMICIRQ_LOWBAT2, + MAX8997_PMICIRQ_LOWBAT1, + + MAX8997_PMICIRQ_JIGR, + MAX8997_PMICIRQ_JIGF, + MAX8997_PMICIRQ_MR, + MAX8997_PMICIRQ_DVS1OK, + MAX8997_PMICIRQ_DVS2OK, + MAX8997_PMICIRQ_DVS3OK, + MAX8997_PMICIRQ_DVS4OK, + + MAX8997_PMICIRQ_CHGINS, + MAX8997_PMICIRQ_CHGRM, + MAX8997_PMICIRQ_DCINOVP, + MAX8997_PMICIRQ_TOPOFFR, + MAX8997_PMICIRQ_CHGRSTF, + MAX8997_PMICIRQ_MBCHGTMEXPD, + + MAX8997_PMICIRQ_RTC60S, + MAX8997_PMICIRQ_RTCA1, + MAX8997_PMICIRQ_RTCA2, + MAX8997_PMICIRQ_SMPL_INT, + MAX8997_PMICIRQ_RTC1S, + MAX8997_PMICIRQ_WTSR, + + MAX8997_MUICIRQ_ADCError, + MAX8997_MUICIRQ_ADCLow, + MAX8997_MUICIRQ_ADC, + + MAX8997_MUICIRQ_VBVolt, + MAX8997_MUICIRQ_DBChg, + MAX8997_MUICIRQ_DCDTmr, + MAX8997_MUICIRQ_ChgDetRun, + MAX8997_MUICIRQ_ChgTyp, + + MAX8997_MUICIRQ_OVP, + + MAX8997_IRQ_NR, +}; + #endif /* __LINUX_MFD_MAX8998_H */ -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] MFD: MAX8997: IRQ definition moved to public header. 2011-06-30 1:31 ` [PATCH 3/3] MFD: MAX8997: IRQ definition moved to public header MyungJoo Ham @ 2011-06-30 5:28 ` Mark Brown 2011-06-30 5:56 ` MyungJoo Ham 0 siblings, 1 reply; 14+ messages in thread From: Mark Brown @ 2011-06-30 5:28 UTC (permalink / raw) To: MyungJoo Ham Cc: linux-kernel, Samuel Ortiz, Kyungmin Park, Liam Girdwood, Donggeun Kim, myungjoo.ham On Thu, Jun 30, 2011 at 10:31:47AM +0900, MyungJoo Ham wrote: > IRQ definitions are needed to be accessed by board support package codes > as well. Because they are not only used by MAX8997 MFD device drivers, > this patch pulls such definitions out of private header, which is meant > for MAX8997 MFD device drivers. Why is this needed? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] MFD: MAX8997: IRQ definition moved to public header. 2011-06-30 5:28 ` Mark Brown @ 2011-06-30 5:56 ` MyungJoo Ham 2011-06-30 5:57 ` Mark Brown 0 siblings, 1 reply; 14+ messages in thread From: MyungJoo Ham @ 2011-06-30 5:56 UTC (permalink / raw) To: Mark Brown Cc: linux-kernel, Samuel Ortiz, Kyungmin Park, Liam Girdwood, Donggeun Kim On Thu, Jun 30, 2011 at 2:28 PM, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote: > On Thu, Jun 30, 2011 at 10:31:47AM +0900, MyungJoo Ham wrote: >> IRQ definitions are needed to be accessed by board support package codes >> as well. Because they are not only used by MAX8997 MFD device drivers, >> this patch pulls such definitions out of private header, which is meant >> for MAX8997 MFD device drivers. > > Why is this needed? > In order to request MAX8997's IRQs, these IRQ enums are required by board files (such as /arch/arm/mach-exynos4/mach-*.c). So, these IRQ enums are no more "private" to max8997 device drivers. Without this patch, board files need to include max8997-private.h. It works properly with or without the patch. The patch is only for some aesthetics reasons. However, this max8997-private.h was meant to be included by device drivers of MAX8997-MFD (such as MAX8997-PMIC, MAX8997-RTC, MAX8997-IRQ, ...); thus, including that private header at board files (or any other non-max8997 device drivers) didn't look "proper". If these "private" MFD headers are to be included by non "subdevices" of the same MFD, it is meaningless to seperate into two headers (max8997.h and max8997-private.h) and we'd better merge public and private headers of MFDs. Cheers! - MyungJoo -- MyungJoo Ham (함명주), Ph.D. Mobile Software Platform Lab, Digital Media and Communications (DMC) Business Samsung Electronics cell: 82-10-6714-2858 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] MFD: MAX8997: IRQ definition moved to public header. 2011-06-30 5:56 ` MyungJoo Ham @ 2011-06-30 5:57 ` Mark Brown 2011-06-30 8:00 ` MyungJoo Ham 0 siblings, 1 reply; 14+ messages in thread From: Mark Brown @ 2011-06-30 5:57 UTC (permalink / raw) To: myungjoo.ham Cc: linux-kernel, Samuel Ortiz, Kyungmin Park, Liam Girdwood, Donggeun Kim On Thu, Jun 30, 2011 at 02:56:27PM +0900, MyungJoo Ham wrote: > In order to request MAX8997's IRQs, these IRQ enums are required by > board files (such as /arch/arm/mach-exynos4/mach-*.c). So, these IRQ > enums are no more "private" to max8997 device drivers. Without this > patch, board files need to include max8997-private.h. Why does the board file need to request these IRQs? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] MFD: MAX8997: IRQ definition moved to public header. 2011-06-30 5:57 ` Mark Brown @ 2011-06-30 8:00 ` MyungJoo Ham 2011-06-30 15:56 ` Mark Brown 0 siblings, 1 reply; 14+ messages in thread From: MyungJoo Ham @ 2011-06-30 8:00 UTC (permalink / raw) To: Mark Brown Cc: linux-kernel, Samuel Ortiz, Kyungmin Park, Liam Girdwood, Donggeun Kim On Thu, Jun 30, 2011 at 2:57 PM, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote: > On Thu, Jun 30, 2011 at 02:56:27PM +0900, MyungJoo Ham wrote: > >> In order to request MAX8997's IRQs, these IRQ enums are required by >> board files (such as /arch/arm/mach-exynos4/mach-*.c). So, these IRQ >> enums are no more "private" to max8997 device drivers. Without this >> patch, board files need to include max8997-private.h. > > Why does the board file need to request these IRQs? > It specifies which interrupts of MAX8997 are used. In our code: http://git.infradead.org/users/kmpark/linux-2.6-samsung/blob/eba500212699c0ad8d6bde0dc01c3ec7101f8154:/arch/arm/mach-exynos4/setup-charger.c around line 85, it specifies which interrupt is going to be a wakeup-source ("true"), how the power-managing S/W should interpret them. The interrupts not listed in the array are not used as a wakeup-source. -- MyungJoo Ham, Ph.D. Mobile Software Platform Lab, Digital Media and Communications (DMC) Business Samsung Electronics cell: 82-10-6714-2858 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] MFD: MAX8997: IRQ definition moved to public header. 2011-06-30 8:00 ` MyungJoo Ham @ 2011-06-30 15:56 ` Mark Brown 2011-07-01 0:43 ` MyungJoo Ham 0 siblings, 1 reply; 14+ messages in thread From: Mark Brown @ 2011-06-30 15:56 UTC (permalink / raw) To: MyungJoo Ham Cc: linux-kernel, Samuel Ortiz, Kyungmin Park, Liam Girdwood, Donggeun Kim On Thu, Jun 30, 2011 at 05:00:13PM +0900, MyungJoo Ham wrote: > In our code: > http://git.infradead.org/users/kmpark/linux-2.6-samsung/blob/eba500212699c0ad8d6bde0dc01c3ec7101f8154:/arch/arm/mach-exynos4/setup-charger.c > around line 85, it specifies which interrupt is going to be a > wakeup-source ("true"), how the power-managing S/W should interpret > them. This looks like charger specific configuration which should be done by the charger driver rather than by directly working with the IRQs? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] MFD: MAX8997: IRQ definition moved to public header. 2011-06-30 15:56 ` Mark Brown @ 2011-07-01 0:43 ` MyungJoo Ham 2011-07-04 17:16 ` Mark Brown 0 siblings, 1 reply; 14+ messages in thread From: MyungJoo Ham @ 2011-07-01 0:43 UTC (permalink / raw) To: Mark Brown Cc: linux-kernel, Samuel Ortiz, Kyungmin Park, Liam Girdwood, Donggeun Kim, myungjoo.ham On Fri, Jul 1, 2011 at 12:56 AM, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote: > On Thu, Jun 30, 2011 at 05:00:13PM +0900, MyungJoo Ham wrote: > >> In our code: >> http://git.infradead.org/users/kmpark/linux-2.6-samsung/blob/eba500212699c0ad8d6bde0dc01c3ec7101f8154:/arch/arm/mach-exynos4/setup-charger.c >> around line 85, it specifies which interrupt is going to be a >> wakeup-source ("true"), how the power-managing S/W should interpret >> them. > > This looks like charger specific configuration which should be done by > the charger driver rather than by directly working with the IRQs? > Well, the issue is that the charger driver just does not know what to do with its own interrupts. For example, each board has different regulator setting for DCIN depending on the specification of the board (some uses 450mA constantly, some uses 450mA and 700mA depending on the connection information, which is not known to charger driver, some uses 900mA unconditionally, and so on). Sometimes setting the attributes of a charger at its own interrupts depends on the status of another charger; when we have USB charger, wireless charger, and solar panels, which may be enabled independently and have their own device drivers. Scenarios that we are going to support include limiting the aggregated charging flow, stopping every charger for an error detected at a single charger (only for really critical ones that imply that the battery itself has a major issue) as usually only one of the chargers has sophiscated sensors and controls, and disabling a charger when others are active (e.g., when CC charging is finished and CV charging is going on with little current). In that charger specific configuration and its driver, we are trying to aggregate multiple chargers (although we specified only two independent chargers, not three, in that example). Thanks, - MyungJoo -- MyungJoo Ham, Ph.D. Mobile Software Platform Lab, Digital Media and Communications (DMC) Business Samsung Electronics cell: 82-10-6714-2858 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] MFD: MAX8997: IRQ definition moved to public header. 2011-07-01 0:43 ` MyungJoo Ham @ 2011-07-04 17:16 ` Mark Brown 2011-07-05 5:57 ` MyungJoo Ham 0 siblings, 1 reply; 14+ messages in thread From: Mark Brown @ 2011-07-04 17:16 UTC (permalink / raw) To: MyungJoo Ham Cc: linux-kernel, Samuel Ortiz, Kyungmin Park, Liam Girdwood, Donggeun Kim On Fri, Jul 01, 2011 at 09:43:31AM +0900, MyungJoo Ham wrote: > On Fri, Jul 1, 2011 at 12:56 AM, Mark Brown > > This looks like charger specific configuration which should be done by > > the charger driver rather than by directly working with the IRQs? > Well, the issue is that the charger driver just does not know what to > do with its own interrupts. > For example, each board has different regulator setting for DCIN > depending on the specification of the board (some uses 450mA > constantly, some uses 450mA and 700mA depending on the connection > information, which is not known to charger driver, some uses 900mA > unconditionally, and so on). That sounds like the charger driver just needs some platform data. > Sometimes setting the attributes of a charger at its own interrupts > depends on the status of another charger; when we have USB charger, > wireless charger, and solar panels, which may be enabled independently > and have their own device drivers. My understanding was that one of the goals of the power_supply subsystem was to support this sort of interaction? This (and your subsequent paragraphs) all sounds entirely sensible but it should be being dealt with at a higher level with the various charger drivers delivering events into a subsystem or board driver which coordinates them all. It seems like the driver should be doing the work of dealing with the actual interrupts. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] MFD: MAX8997: IRQ definition moved to public header. 2011-07-04 17:16 ` Mark Brown @ 2011-07-05 5:57 ` MyungJoo Ham 2011-07-05 6:19 ` Mark Brown 0 siblings, 1 reply; 14+ messages in thread From: MyungJoo Ham @ 2011-07-05 5:57 UTC (permalink / raw) To: Mark Brown Cc: linux-kernel, Samuel Ortiz, Kyungmin Park, Liam Girdwood, Donggeun Kim Hi! On Tue, Jul 5, 2011 at 2:16 AM, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote: > On Fri, Jul 01, 2011 at 09:43:31AM +0900, MyungJoo Ham wrote: >> On Fri, Jul 1, 2011 at 12:56 AM, Mark Brown > >> > This looks like charger specific configuration which should be done by >> > the charger driver rather than by directly working with the IRQs? > >> Well, the issue is that the charger driver just does not know what to >> do with its own interrupts. > >> For example, each board has different regulator setting for DCIN >> depending on the specification of the board (some uses 450mA >> constantly, some uses 450mA and 700mA depending on the connection >> information, which is not known to charger driver, some uses 900mA >> unconditionally, and so on). > > That sounds like the charger driver just needs some platform data. Right. And, that platform data includes IRQs: in this case, the IRQs of MAX8997, which have charger and USB connection information. MAX8997's IRQ handlers need to notify the charger driver and the charger driver may use other PMIC, USB switch (such as FSA9480), and any devices with external power connections. > >> Sometimes setting the attributes of a charger at its own interrupts >> depends on the status of another charger; when we have USB charger, >> wireless charger, and solar panels, which may be enabled independently >> and have their own device drivers. > > My understanding was that one of the goals of the power_supply subsystem > was to support this sort of interaction? This (and your subsequent > paragraphs) all sounds entirely sensible but it should be being dealt > with at a higher level with the various charger drivers delivering > events into a subsystem or board driver which coordinates them all. It > seems like the driver should be doing the work of dealing with the > actual interrupts. > Yes, I also think that it is supposed to read and update attributes of chargers. However, I don't see any ways to interconnect chargers and related devices with power_supply subsystem. If we let a user process interconnect the chargers and related devices, we need to allow userland to access (both read and write) related attributes including power_supply_class entries and regulators (charger on/off and adjusting charger speed). But, we don't support userland r/w access to regulators. Also, I feel a bit wierd to rely on user programs for specifying hardware dependencies between on-board chips (e.g., setting charger limit at 450mA if USB is attached, at 900mA if TA is attached, at 1.5A if high-speed TA is attached, or at 300mA if wireless charger is attached / disable charger B if charger A started CV charging, ...). Besides, we want charger control/monitoring while user tasks are frozen, which was the reason why we have been working on "suspend_again" callback for suspend: http://us.generation-nt.com/answer/patch-v5-pm-core-suspend-again-callback-suspend-ops-help-203628722.html . For this, we will need in-kernel code (but out of max8997 device driver code) to have information about the IRQs of max8997 (or any other similar chips). Anyway, even if we are going to implement charger control/monitoring service in userspace directly accessing the device drivers of PMIC, chargers, and switches, those user programs need to access IRQ (need to get notifications from the interrupts). Isn't including "non-private" kernel headers more preferable for user programs than including "private" kernel headers or hard coding IRQ numbers, is it? Thank you. Cheers! - MyungJoo. -- MyungJoo Ham, Ph.D. Mobile Software Platform Lab, Digital Media and Communications (DMC) Business Samsung Electronics cell: 82-10-6714-2858 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] MFD: MAX8997: IRQ definition moved to public header. 2011-07-05 5:57 ` MyungJoo Ham @ 2011-07-05 6:19 ` Mark Brown 2011-07-05 6:49 ` MyungJoo Ham 0 siblings, 1 reply; 14+ messages in thread From: Mark Brown @ 2011-07-05 6:19 UTC (permalink / raw) To: MyungJoo Ham Cc: linux-kernel, Samuel Ortiz, Kyungmin Park, Liam Girdwood, Donggeun Kim On Tue, Jul 05, 2011 at 02:57:54PM +0900, MyungJoo Ham wrote: > On Tue, Jul 5, 2011 at 2:16 AM, Mark Brown > > My understanding was that one of the goals of the power_supply subsystem > > was to support this sort of interaction? This (and your subsequent > > paragraphs) all sounds entirely sensible but it should be being dealt > > with at a higher level with the various charger drivers delivering > > events into a subsystem or board driver which coordinates them all. It > > seems like the driver should be doing the work of dealing with the > > actual interrupts. > Yes, I also think that it is supposed to read and update attributes of > chargers. However, I don't see any ways to interconnect chargers and > related devices with power_supply subsystem. Nor do I, but this is just software so we should be able to make it do what's needed here. > If we let a user process interconnect the chargers and related > devices, we need to allow userland to access (both read and write) I didn't say anything about userspace, and I wouldn't expect userspace to do anything except policy here. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] MFD: MAX8997: IRQ definition moved to public header. 2011-07-05 6:19 ` Mark Brown @ 2011-07-05 6:49 ` MyungJoo Ham 2011-07-05 19:53 ` Mark Brown 0 siblings, 1 reply; 14+ messages in thread From: MyungJoo Ham @ 2011-07-05 6:49 UTC (permalink / raw) To: Mark Brown Cc: linux-kernel, Samuel Ortiz, Kyungmin Park, Liam Girdwood, Donggeun Kim On Tue, Jul 5, 2011 at 3:19 PM, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote: > On Tue, Jul 05, 2011 at 02:57:54PM +0900, MyungJoo Ham wrote: >> On Tue, Jul 5, 2011 at 2:16 AM, Mark Brown > >> > My understanding was that one of the goals of the power_supply subsystem >> > was to support this sort of interaction? This (and your subsequent >> > paragraphs) all sounds entirely sensible but it should be being dealt >> > with at a higher level with the various charger drivers delivering >> > events into a subsystem or board driver which coordinates them all. It >> > seems like the driver should be doing the work of dealing with the >> > actual interrupts. > >> Yes, I also think that it is supposed to read and update attributes of >> chargers. However, I don't see any ways to interconnect chargers and >> related devices with power_supply subsystem. > > Nor do I, but this is just software so we should be able to make it do > what's needed here. > >> If we let a user process interconnect the chargers and related >> devices, we need to allow userland to access (both read and write) > > I didn't say anything about userspace, and I wouldn't expect userspace > to do anything except policy here. > Ah.. then, I misunderstood "a higher level" in your previous reply was userspace. Sorry, my bad. I was just trying to say that a) as long as we are connecting IRQ events of a device to another device, the IRQ information is going to be needed as non-private, and b) in the above case, we need to let the IRQ events of a device notify other devices' drivers. (defined in a form of platform_data in the case, specifying IRQ numbers for the notified devices requiring the IRQ definitions to be included) Anyway, I've been trying to implement some virtual device driver or psuedo framework in kernel to interconnect charger related devices, "charger manager". This code mentioned is to support that "charger manager". It is sort of at a higher level of charger related devices, taking device names of them as a platform data. The platform data taking the IRQ numbers is the one for that "charger manager". Thank you. - MyungJoo -- MyungJoo Ham, Ph.D. Mobile Software Platform Lab, Digital Media and Communications (DMC) Business Samsung Electronics cell: 82-10-6714-2858 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] MFD: MAX8997: IRQ definition moved to public header. 2011-07-05 6:49 ` MyungJoo Ham @ 2011-07-05 19:53 ` Mark Brown 0 siblings, 0 replies; 14+ messages in thread From: Mark Brown @ 2011-07-05 19:53 UTC (permalink / raw) To: MyungJoo Ham Cc: linux-kernel, Samuel Ortiz, Kyungmin Park, Liam Girdwood, Donggeun Kim On Tue, Jul 05, 2011 at 03:49:39PM +0900, MyungJoo Ham wrote: > Anyway, I've been trying to implement some virtual device driver or > psuedo framework in kernel to interconnect charger related devices, > "charger manager". This code mentioned is to support that "charger > manager". It is sort of at a higher level of charger related devices, > taking device names of them as a platform data. The platform data > taking the IRQ numbers is the one for that "charger manager". Right, so what I'm saying is that the interface between the two blocks should be changed so that the manager code isn't talking directly to the interrupt but is talking to something higher level. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2011-07-05 19:53 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-06-30 1:31 [PATCH 1/3] MFD: MAX8997: IRQ Handling Bugfix MyungJoo Ham 2011-06-30 1:31 ` [PATCH 2/3] MFD: MAX8997: Support Wake-up from Suspend MyungJoo Ham 2011-06-30 1:31 ` [PATCH 3/3] MFD: MAX8997: IRQ definition moved to public header MyungJoo Ham 2011-06-30 5:28 ` Mark Brown 2011-06-30 5:56 ` MyungJoo Ham 2011-06-30 5:57 ` Mark Brown 2011-06-30 8:00 ` MyungJoo Ham 2011-06-30 15:56 ` Mark Brown 2011-07-01 0:43 ` MyungJoo Ham 2011-07-04 17:16 ` Mark Brown 2011-07-05 5:57 ` MyungJoo Ham 2011-07-05 6:19 ` Mark Brown 2011-07-05 6:49 ` MyungJoo Ham 2011-07-05 19:53 ` Mark Brown
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.