LP8727 has two IRQ threads.
One is the I2C HW IRQ pin, the other is for delayed interrupt processing.
But this delayed processing can be handled without additional single thread
by using schedule_delayed_work() with jiffies time value.
Signed-off-by: Milo(Woogyom) Kim
---
Debounce time is configurable in the platform side.
If it is not defined, the default value is 270ms.
Platform data is msec unit, and this time is converted to jiffies internally.
The workqueue uses this jiffies time in the interrupt handling.
So debounce_jiffies is added in the private
LP8727 platform data is optional, so the driver should work even
the platform data is NULL.
To check the platform data, charging parameter data should be changed
to the pointer type.
Fix NULL point access problem when getting the battery properties.
When the data is NULL, just return as
If the lp8727_register_psy() gets failed, registered interrupt handler
should be freed, but this is not complete solution.
It has still problem.
Assume that the IRQ occurs while unregistering power supply devices.
Then the ISR will access to freed IRQ.
From Anton's opinion, it can be
Use devm_kzalloc() rather than kzalloc()/kfree() to
make allocating/freeing the private data simpler.
Signed-off-by: Milo(Woogyom) Kim
---
drivers/power/lp8727_charger.c |8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/power/lp8727_charger.c
Use devm_kzalloc() rather than kzalloc()/kfree() to
make allocating/freeing the private data simpler.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c |8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/power
- in reverse order of _probe()
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c | 20 +---
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/power/lp8727_charger.c b/drivers/power/lp8727_charger.c
index 0d3cb1d..b2df114 100644
as invalid value.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c | 28 +---
include/linux/platform_data/lp8727.h |7 ---
2 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/drivers/power/lp8727_charger.c b/drivers
.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c | 12 +---
include/linux/platform_data/lp8727.h |2 ++
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/power/lp8727_charger.c b/drivers/power/lp8727_charger.c
index
LP8727 has two IRQ threads.
One is the I2C HW IRQ pin, the other is for delayed interrupt processing.
But this delayed processing can be handled without additional single thread
by using schedule_delayed_work() with jiffies time value.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
, the driver should be operated.
In this case, just return as 0.
In additional function lp8727_release_irq(), the workqueue is canceled and
the allocated IRQ is released.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c | 37 -
1
To initialize the device, previous interrupts need to be cleared
while loading the driver.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c |8
1 file changed, 8 insertions(+)
diff --git a/drivers/power/lp8727_charger.c b/drivers/power
comparing raw register values.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c | 31 ++-
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/drivers/power/lp8727_charger.c b/drivers/power/lp8727_charger.c
index b5567bc..1a5f4b2
Enum lp8727_chg_state can be removed because only one charger status
is used - EOC(End Of Charge).
To check whether the EOC is reached or not, use simple comparison
rather than shift-operation.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c | 16
All definitions should be unique - not general name.
So the prefix LP8727_ are added.
Additionally, use BIT() macro for bit masks.
Remove unnecessary definitions such as SW_DM1_U1 and SW_DP2_U2.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c | 130
Add new LP8727_ICHG_SHIFT definition and replace a magic number.
Reuse definition for the size of interrupt register buffer.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c |9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git
Change return type as boolean.
Remove unnecessary check routine for NULL string.
(Power supply name is always valid when the function is executed.)
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c | 14 ++
1 file changed, 6 insertions(+), 8
Return as the result of writing register.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c |6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/power/lp8727_charger.c b/drivers/power/lp8727_charger.c
index 658518b..47cce41 100644
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/power/lp8727_charger.c b/drivers/power/lp8727_charger.c
index 47cce41..880bb7b 100644
--- a/drivers/power/lp8727_charger.c
+++ b
Charger has only one valid property - ONLINE.
If the property is not ONLINE, then just return quickly.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c |7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/power
If the charger is pulled out, just return as DISCHARGING.
Then no need for additional 'else' statement.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/power
Declare a variable at one line.
Just return when no charger exists to make code simpler.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c | 22 +-
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/power
Declare a variable at one line and align lines.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c |8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/power/lp8727_charger.c b/drivers/power/lp8727_charger.c
index a01059f
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/power/lp8727_charger.c b/drivers/power/lp8727_charger.c
index 62102c4..b3295a7 100644
--- a/drivers/power
This is really minor, but it improves the readability.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c |3 ++-
include/linux/platform_data/lp8727.h |8
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/power
Add description and relocate data.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/power/lp8727_charger.c b/drivers/power/lp8727_charger.c
index b3295a7..772e509 100644
Fix the warning on MODULE_AUTHOR.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/power/lp8727_charger.c b/drivers/power/lp8727_charger.c
index 772e509..005aaae 100644
LP8727 driver should be fixed for several reasons.
(a) Need to clean up _probe() and _remove()
(b) Not secure code when the platform data is NULL
(c) Improved the interrupt handling
(d) Lots of definitions should be unique.
(e) Others...
Patch v2.
These patches are submitted more separately.
> -Original Message-
> From: Anton Vorontsov [mailto:anton.voront...@linaro.org]
> Sent: Thursday, August 30, 2012 9:15 PM
> To: Kim, Milo
> Cc: linux-kernel@vger.kernel.org; David Woodhouse
> Subject: Re: [PATCH 0/8] lp8727_charger: cleanup code
>
> On Thu, Au
> > ERROR: Threaded IRQ with no primary handler requested without
> > IRQF_ONESHOT
> >
> > Make sure threaded IRQs without a primary handler are always request
> > with IRQF_ONESHOT
> >
> > Signed-off-by: Fengguang Wu
> > ---
> >
> > Note: I don't really know much about the situation, feel free
Best Regards,
Milo
> -Original Message-
> From: Devendra Naga [mailto:develkernel412...@gmail.com]
> Sent: Friday, August 24, 2012 1:52 AM
> To: Anton Vorontsov
> Cc: David Woodhouse; linux-kernel@vger.kernel.org; Kim, Milo
> Subject: Re: [PATCH 1/2] lp8727_char
(a) change the return type of lp8727_is_charger_attached()
(b) remove unnecessary name comparison in lp8727_is_charger_attached()
(c) return as the result of function in lp8727_init_device()
(d) make inline function for lp8727_ctrl_switch()
(e) use specific definition rather than magic number in
(a) add prefix LP8727_ for definitions
(b) replace lp8727_dev_id with lp8727_charger_type
(c) use one LP8727_STAT_EOC definition rather than enum type lp8727_chg_stat
: charger status definitions are not used except EOC status
(d) use LP8727_ICHG_SHIFT rather than magic number
Signed-off-by:
For better understanding, use specific function and name
rather than magic number
Signed-off-by: Milo(Woogyom) Kim
---
drivers/power/lp8727_charger.c | 31 ++-
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/drivers/power/lp8727_charger.c
use the mutex only when the write access to the registers
in the delayed work.
Signed-off-by: Milo(Woogyom) Kim
---
drivers/power/lp8727_charger.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/power/lp8727_charger.c
(a) add configurable debounce timer in the platform data
: if it is not defined, default time(270ms) is set.
(b) use schedule_delay_work() and remove unnecessary workqueue resource
: for delayed interrupt handling, use the schedule_delay_work()
(c) add lp8727_release_irq() for clearing the
Even the platform data is not defined, lp8727 driver should be run
(a) change charger platform data type to pointer
(b) name change: chg_parm -> chg_param
(c) fix NULL point access problem when getting the battery properties
Signed-off-by: Milo(Woogyom) Kim
---
drivers/power/lp8727_charger.c
make allocating/freeing the private data simpler
Signed-off-by: Milo(Woogyom) Kim
---
drivers/power/lp8727_charger.c |8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/power/lp8727_charger.c b/drivers/power/lp8727_charger.c
index 4e37b26..fb3d7ac 100644
---
(a) register the power supply prior to creating irq handler
After reordering the sequence, the interrupt issue can be gone
when the power supply registration gets failed
(b) remove unnecessary goto statements
(c) unregister the power supply after releasing irq resources
in reverse
LP8727 driver should be patched for several reasons.
(a) Need to clean up _probe()/_remove()
(b) Not secure code when the platform data is NULL
(c) Interrupt handling
Two threads are running for handling one IRQ.
One is for the IRQ pin, the other is used for delayed processing.
This
LP8727 driver should be patched for several reasons.
(a) Need to clean up _probe()/_remove()
(b) Not secure code when the platform data is NULL
(c) Interrupt handling
Two threads are running for handling one IRQ.
One is for the IRQ pin, the other is used for delayed processing.
This
order of _probe()
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c | 20 +---
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/power/lp8727_charger.c b/drivers/power/lp8727_charger.c
index fb3d7ac..4d51909 100644
--- a/drivers
make allocating/freeing the private data simpler
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c |8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/power/lp8727_charger.c b/drivers/power/lp8727_charger.c
index 4e37b26
Even the platform data is not defined, lp8727 driver should be run
(a) change charger platform data type to pointer
(b) name change: chg_parm - chg_param
(c) fix NULL point access problem when getting the battery properties
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power
the irq
(c) clear interrupts while loading the driver
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c | 70 +++---
include/linux/platform_data/lp8727.h |2 +
2 files changed, 50 insertions(+), 22 deletions(-)
diff --git
use the mutex only when the write access to the registers
in the delayed work.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/power/lp8727_charger.c b/drivers
For better understanding, use specific function and name
rather than magic number
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c | 31 ++-
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/drivers/power
-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c | 184 --
include/linux/platform_data/lp8727.h | 34 +++
2 files changed, 106 insertions(+), 112 deletions(-)
diff --git a/drivers/power/lp8727_charger.c b/drivers/power
in lp8727_delayed_func()
(f) make simple code in power_supply_get_properties
(g) make simple code in lp8727_charger_changed()
(h) fix checkpatch warning on MODULE_AUTHOR
(i) fit spaces for description in header
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/lp8727_charger.c
Best Regards,
Milo
-Original Message-
From: Devendra Naga [mailto:develkernel412...@gmail.com]
Sent: Friday, August 24, 2012 1:52 AM
To: Anton Vorontsov
Cc: David Woodhouse; linux-kernel@vger.kernel.org; Kim, Milo
Subject: Re: [PATCH 1/2] lp8727_charger: free_irq when
,
pchg);
}
Thanks for the update.
Acked-by: Milo(Woogyom) Kim milo@ti.com
Please refer to the patch below.
[PATCH 4/8] lp8727_charger: cleanup the interrupt handler code
This includes previous Fengguang' patch.
Best Regards,
Milo
--
To unsubscribe from this list: send
-Original Message-
From: Anton Vorontsov [mailto:anton.voront...@linaro.org]
Sent: Thursday, August 30, 2012 9:15 PM
To: Kim, Milo
Cc: linux-kernel@vger.kernel.org; David Woodhouse
Subject: Re: [PATCH 0/8] lp8727_charger: cleanup code
On Thu, Aug 30, 2012 at 11:37:16AM +, Kim
> ERROR: Threaded IRQ with no primary handler requested without
> IRQF_ONESHOT
>
> Make sure threaded IRQs without a primary handler are always request
> with IRQF_ONESHOT
>
> Signed-off-by: Fengguang Wu
> ---
>
> Note: I don't really know much about the situation, feel free to
> ignore it if
,
pchg);
}
Thanks for the update.
Acked-by: Milo(Woogyom) Kim milo@ti.com
Best Regards,
Milo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo
Turning off the brightness of each channel is required
when removing the driver.
So use flush_work() rather than cancel_work_sync() to execute
remaining brightness works.
Signed-off-by: Milo(Woogyom) Kim
---
drivers/leds/leds-lp5523.c |4 ++--
1 files changed, 2 insertions(+), 2
The name of each led channel is configurable.
If the name is NULL, just use the channel id for making the channel name
Signed-off-by: Milo(Woogyom) Kim
---
Documentation/leds/leds-lp5523.txt | 21 ++---
drivers/leds/leds-lp5523.c | 10 +++---
The name of each led channel is configurable.
If the name is NULL, just use the channel id for making the channel name
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
Documentation/leds/leds-lp5523.txt | 21 ++---
drivers/leds/leds-lp5523.c | 10
Turning off the brightness of each channel is required
when removing the driver.
So use flush_work() rather than cancel_work_sync() to execute
remaining brightness works.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/leds/leds-lp5523.c |4 ++--
1 files changed, 2 insertions
> Hmmm, I think we still should use cancel_work() here based on your
> idea. Please find the patch from Tejun and add him to this loop
> [PATCH 4/6] workqueue: deprecate flush[_delayed]_work_sync()
> ---
> Before this patchset,
>
> flush_work()
>
> flush the last queued instance of the
> > LP5523 can drive up to 9 channels. Leds can be controlled directly
> via
> > -the led class control interface. Channels have generic names:
> > +the led class control interface.
> > +The name of each channel is configurable in the platform data.
> > +If the name is NULL, channels have generic
(a) use LP5523_ENABLE rather than magic number 0x40
(b) use min_t() in lp5523_mux_parse()
(c) skip while loop and just return if invalid command
Signed-off-by: Milo(Woogyom) Kim
---
drivers/leds/leds-lp5523.c | 15 +++
1 files changed, 7 insertions(+), 8 deletions(-)
diff --git
The return value of this function is not handled any place, so
make it as void type.
And three if-statements are replaced with switch-statements.
Signed-off-by: Milo(Woogyom) Kim
---
drivers/leds/leds-lp5523.c | 24 +---
1 files changed, 13 insertions(+), 11 deletions(-)
Turning off the brightness of each channel is required
when removing the driver.
So use flush_work_sync() rather than cancel_work_sync() to wait for
unhandled brightness works.
Signed-off-by: Milo(Woogyom) Kim
---
drivers/leds/leds-lp5523.c |4 ++--
1 files changed, 2 insertions(+), 2
The name of each led channel is configurable.
If the name is NULL, just use the channel id for making the channel name
Signed-off-by: Milo(Woogyom) Kim
---
Documentation/leds/leds-lp5523.txt |7 +--
drivers/leds/leds-lp5523.c | 10 +++---
include/linux/leds-lp5523.h
is defined
So I would change the description as below.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
Documentation/leds/leds-lp5523.txt | 18 --
1 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/Documentation/leds/leds-lp5523.txt
b/Documentation/leds/leds
Hmmm, I think we still should use cancel_work() here based on your
idea. Please find the patch from Tejun and add him to this loop
[PATCH 4/6] workqueue: deprecate flush[_delayed]_work_sync()
---
Before this patchset,
flush_work()
flush the last queued instance of the work
The name of each led channel is configurable.
If the name is NULL, just use the channel id for making the channel name
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
Documentation/leds/leds-lp5523.txt |7 +--
drivers/leds/leds-lp5523.c | 10 +++---
include/linux/leds
Turning off the brightness of each channel is required
when removing the driver.
So use flush_work_sync() rather than cancel_work_sync() to wait for
unhandled brightness works.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/leds/leds-lp5523.c |4 ++--
1 files changed, 2
The return value of this function is not handled any place, so
make it as void type.
And three if-statements are replaced with switch-statements.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/leds/leds-lp5523.c | 24 +---
1 files changed, 13 insertions
(a) use LP5523_ENABLE rather than magic number 0x40
(b) use min_t() in lp5523_mux_parse()
(c) skip while loop and just return if invalid command
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/leds/leds-lp5523.c | 15 +++
1 files changed, 7 insertions(+), 8 deletions
> Yes, the goal is to remove all implementations of the old framework
> (HAVE_PWM) and replace it with PWM only implementations. I suppose we
> could in the meantime add #ifdef CONFIG_HAVE_PWM around the legacy
> functions and provide dummies in the !PWM case. That might work.
Sounds great,
> Maybe we should get this resolved somehow in the meantime. Resolving
> the
> other issues may take another cycle or two, so you may not want to wait
> that long.
Is that job also including HAVE_PWM configurations?
Some SoCs still set HAVE_PWMs and codes exist under arch/ directory.
As I far as
> In that case, I would recommend changing it from
>
> + /* if the pwm device exists, skip requesting the device */
> + if (drvdata->pwm)
> + return 0;
>
> to
>
> /* warn if the PWM was not released prior to reneabling it */
> WARN_ON(drvdata->pwm);
>
OK,
> > * Rather than having to do the #ifdef here, I think it would be
> better if
> > the PWM subsystem provided stub functions for pwm_request,
> pwm_config,
> > pwm_enable, pwm_disable and pwm_free that do nothing, so you can in
> effect
> > let the compiler optimize away the above code.
>
> * Rather than having to do the #ifdef here, I think it would be better
> if
> the PWM subsystem provided stub functions for pwm_request, pwm_config,
> pwm_enable, pwm_disable and pwm_free that do nothing, so you can in
> effect
> let the compiler optimize away the above code.
Thanks a lot
* Rather than having to do the #ifdef here, I think it would be better
if
the PWM subsystem provided stub functions for pwm_request, pwm_config,
pwm_enable, pwm_disable and pwm_free that do nothing, so you can in
effect
let the compiler optimize away the above code.
Thanks a lot for
* Rather than having to do the #ifdef here, I think it would be
better if
the PWM subsystem provided stub functions for pwm_request,
pwm_config,
pwm_enable, pwm_disable and pwm_free that do nothing, so you can in
effect
let the compiler optimize away the above code.
That's
In that case, I would recommend changing it from
+ /* if the pwm device exists, skip requesting the device */
+ if (drvdata-pwm)
+ return 0;
to
/* warn if the PWM was not released prior to reneabling it */
WARN_ON(drvdata-pwm);
OK, thanks for
Maybe we should get this resolved somehow in the meantime. Resolving
the
other issues may take another cycle or two, so you may not want to wait
that long.
Is that job also including HAVE_PWM configurations?
Some SoCs still set HAVE_PWMs and codes exist under arch/ directory.
As I far as
Yes, the goal is to remove all implementations of the old framework
(HAVE_PWM) and replace it with PWM only implementations. I suppose we
could in the meantime add #ifdef CONFIG_HAVE_PWM around the legacy
functions and provide dummies in the !PWM case. That might work.
Sounds great, thanks a
use generic pwm functions for changing the duty rather than the platform data
(a) add lm3530_pwm_xxx functions for supporting pwm control mode
(b) add pwm id and period in the platform data
(c) gather pwm platform data into one place
This device should be built even if the pwm mode is unused.
.
That's why CONFIG_PWM is defined in the source rather than in Kconfig file.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/leds/leds-lm3530.c | 95 ++--
include/linux/led-lm3530.h | 17 +++-
2 files changed, 89 insertions(+), 23 deletions
> > int iio_read_channel_scale(struct iio_channel *chan, int *val,
> >int *val2);
> >
> > +/**
> > + * iio_read_channel_offset() - read offset from a given channel
> > + * @channel: The channel being queried.
>
> the parameter is 'chan', not 'channel'
>
Patch v3.
(a) Delete unnecessary blank line of description
(b) Sort alphabetical order for header
(c) Replace udelay() with usleep_range()
(d) Change read_raw() in case of scale and offset
: result can be calculated as raw * (scaleint + scalepart * 100) +
offset.
(scale: micro unit)
(a) For better readability, replace 0 with IIO_CHAN_INFO_RAW.
(b) Make same line-format as other apis()
: iio_read_channel_scale() and iio_read_channel_offset()
Signed-off-by: Milo(Woogyom) Kim
---
drivers/iio/inkern.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff
This allows the iio consumer to get the offset of the channel.
The value of offset can be used when calculating the result such
as 'result = raw * scale + offset'.
Signed-off-by: Milo(Woogyom) Kim
---
drivers/iio/inkern.c | 21 +
include/linux/iio/consumer.h |9
This allows the iio consumer to get the offset of the channel.
The value of offset can be used when calculating the result such
as 'result = raw * scale + offset'.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/iio/inkern.c | 21 +
include/linux/iio
(a) For better readability, replace 0 with IIO_CHAN_INFO_RAW.
(b) Make same line-format as other apis()
: iio_read_channel_scale() and iio_read_channel_offset()
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/iio/inkern.c |6 --
1 files changed, 4 insertions(+), 2
)
(e) Add 'const' for static constant iio_chan_spec
(f) Fix wrong size of allocating iio private data
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/iio/adc/Kconfig |6 +
drivers/iio/adc/Makefile |1 +
drivers/iio/adc/lp8788_adc.c | 224
int iio_read_channel_scale(struct iio_channel *chan, int *val,
int *val2);
+/**
+ * iio_read_channel_offset() - read offset from a given channel
+ * @channel: The channel being queried.
the parameter is 'chan', not 'channel'
Oops, this is
> > + switch (mask) {
> > + case IIO_CHAN_INFO_RAW:
> > + *val = result;
> > + return IIO_VAL_INT;
> > + case IIO_CHAN_INFO_SCALE:
> > + *val = adc_const[id] * ((result * 1000 + 500) / 1000);
>
> This looks wrong. The IIO_CHAN_INFO_SCALE attribute is the factor
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ *val = result;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ *val = adc_const[id] * ((result * 1000 + 500) / 1000);
This looks wrong. The IIO_CHAN_INFO_SCALE attribute is the factor by
which
Patch v2.
(a) use irq domain for handling alarm IRQ
(b) replace module_init() and module_exit() with module_platform_driver()
(c) clean up code for _probe() and _remove()
Signed-off-by: Milo(Woogyom) Kim
---
drivers/rtc/Kconfig |6 +
drivers/rtc/Makefile |1 +
Patch v3.
(a) use irq domain for handling charger interrupts
(b) use scaled adc value rather than raw value
: replace iio_read_channel_raw() with iio_read_channel_scale()
(c) clean up charger-platform-data code
(d) remove goto statement in _probe()
(e) name change : from
Patch v3.
replace generic irq code with irq domain
(a) use linear irq domain
(b) remove irq_base from lp8788 platform data
(c) changes on lp8788-charger and rtc-lp8788 drivers
: mapping hwirq to linux IRQ number
(seperate patches will be sent)
Signed-off-by: Milo(Woogyom) Kim
---
Patch v3.
replace generic irq code with irq domain
(a) use linear irq domain
(b) remove irq_base from lp8788 platform data
(c) changes on lp8788-charger and rtc-lp8788 drivers
: mapping hwirq to linux IRQ number
(seperate patches will be sent)
Signed-off-by: Milo(Woogyom) Kim milo
() to lp8788_psy_unregister()
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/power/Kconfig |7 +
drivers/power/Makefile |1 +
drivers/power/lp8788-charger.c | 785
3 files changed, 793 insertions(+), 0 deletions
Patch v2.
(a) use irq domain for handling alarm IRQ
(b) replace module_init() and module_exit() with module_platform_driver()
(c) clean up code for _probe() and _remove()
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/rtc/Kconfig |6 +
drivers/rtc/Makefile |1
> > Patch v2.
> > (a) For interrupt handling, use generic irq rather than irq-domain
>
> This seems like a very substantial step backwards, why make this change?
> Using irqdomain solves a bunch of problems, especially around virq
> allocation, and is where we want all drivers to go longer term.
> This is mostly fine though things have gotten a little confused
> wrt to the handling iio_priv in the probe and remove so that
> needs cleaning up. A few other minor bits inline.
>
> Thanks,
>
> Jonathan
Thanks a lot for detailed review.
Patch v2 has been sent.
Title: [PATCH v2] iio: adc:
601 - 700 of 758 matches
Mail list logo
