* [PATCH 1/5] MFD AB3100 accessor function cleanups
@ 2009-08-13 9:49 Linus Walleij
2009-08-16 22:56 ` Samuel Ortiz
0 siblings, 1 reply; 2+ messages in thread
From: Linus Walleij @ 2009-08-13 9:49 UTC (permalink / raw)
To: Samuel Ortiz, linux-kernel; +Cc: Linus Walleij
This adds the _interruptible suffix to the AB3100 accessor
functions on par with mutex_lock_interruptible() that's used
for blocking simultaneous calls to the AB3100 acessor functions.
Since these accesses are slow on a 100kHz I2C bus and may line
up waiting for the mutex, we need to handle interruption by
system shutdown or kill signals and may just as well denote that
in the function names.
Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
---
drivers/mfd/ab3100-core.c | 43 ++++++++++++++++++++++++-------------------
include/linux/mfd/ab3100.h | 8 ++++----
2 files changed, 28 insertions(+), 23 deletions(-)
diff --git a/drivers/mfd/ab3100-core.c b/drivers/mfd/ab3100-core.c
index 13e7d7b..ffb2af2 100644
--- a/drivers/mfd/ab3100-core.c
+++ b/drivers/mfd/ab3100-core.c
@@ -77,7 +77,7 @@ u8 ab3100_get_chip_type(struct ab3100 *ab3100)
}
EXPORT_SYMBOL(ab3100_get_chip_type);
-int ab3100_set_register(struct ab3100 *ab3100, u8 reg, u8 regval)
+int ab3100_set_register_interruptible(struct ab3100 *ab3100, u8 reg, u8 regval)
{
u8 regandval[2] = {reg, regval};
int err;
@@ -109,7 +109,8 @@ int ab3100_set_register(struct ab3100 *ab3100, u8 reg, u8 regval)
mutex_unlock(&ab3100->access_mutex);
return 0;
}
-EXPORT_SYMBOL(ab3100_set_register);
+EXPORT_SYMBOL(ab3100_set_register_interruptible);
+
/*
* The test registers exist at an I2C bus address up one
@@ -118,7 +119,7 @@ EXPORT_SYMBOL(ab3100_set_register);
* anyway. It's currently only used from this file so declare
* it static and do not export.
*/
-static int ab3100_set_test_register(struct ab3100 *ab3100,
+static int ab3100_set_test_register_interruptible(struct ab3100 *ab3100,
u8 reg, u8 regval)
{
u8 regandval[2] = {reg, regval};
@@ -148,7 +149,8 @@ static int ab3100_set_test_register(struct ab3100 *ab3100,
return err;
}
-int ab3100_get_register(struct ab3100 *ab3100, u8 reg, u8 *regval)
+
+int ab3100_get_register_interruptible(struct ab3100 *ab3100, u8 reg, u8 *regval)
{
int err;
@@ -202,9 +204,10 @@ int ab3100_get_register(struct ab3100 *ab3100, u8 reg, u8 *regval)
mutex_unlock(&ab3100->access_mutex);
return err;
}
-EXPORT_SYMBOL(ab3100_get_register);
+EXPORT_SYMBOL(ab3100_get_register_interruptible);
+
-int ab3100_get_register_page(struct ab3100 *ab3100,
+int ab3100_get_register_page_interruptible(struct ab3100 *ab3100,
u8 first_reg, u8 *regvals, u8 numregs)
{
int err;
@@ -258,9 +261,10 @@ int ab3100_get_register_page(struct ab3100 *ab3100,
mutex_unlock(&ab3100->access_mutex);
return err;
}
-EXPORT_SYMBOL(ab3100_get_register_page);
+EXPORT_SYMBOL(ab3100_get_register_page_interruptible);
-int ab3100_mask_and_set_register(struct ab3100 *ab3100,
+
+int ab3100_mask_and_set_register_interruptible(struct ab3100 *ab3100,
u8 reg, u8 andmask, u8 ormask)
{
u8 regandval[2] = {reg, 0};
@@ -328,7 +332,8 @@ int ab3100_mask_and_set_register(struct ab3100 *ab3100,
mutex_unlock(&ab3100->access_mutex);
return err;
}
-EXPORT_SYMBOL(ab3100_mask_and_set_register);
+EXPORT_SYMBOL(ab3100_mask_and_set_register_interruptible);
+
/*
* Register a simple callback for handling any AB3100 events.
@@ -371,7 +376,7 @@ static void ab3100_work(struct work_struct *work)
u32 fatevent;
int err;
- err = ab3100_get_register_page(ab3100, AB3100_EVENTA1,
+ err = ab3100_get_register_page_interruptible(ab3100, AB3100_EVENTA1,
event_regs, 3);
if (err)
goto err_event_wq;
@@ -435,7 +440,7 @@ static int ab3100_registers_print(struct seq_file *s, void *p)
seq_printf(s, "AB3100 registers:\n");
for (reg = 0; reg < 0xff; reg++) {
- ab3100_get_register(ab3100, reg, &value);
+ ab3100_get_register_interruptible(ab3100, reg, &value);
seq_printf(s, "[0x%x]: 0x%x\n", reg, value);
}
return 0;
@@ -515,7 +520,7 @@ static int ab3100_get_set_reg(struct file *file,
u8 reg = (u8) user_reg;
u8 regvalue;
- ab3100_get_register(ab3100, reg, ®value);
+ ab3100_get_register_interruptible(ab3100, reg, ®value);
dev_info(ab3100->dev,
"debug read AB3100 reg[0x%02x]: 0x%02x\n",
@@ -547,8 +552,8 @@ static int ab3100_get_set_reg(struct file *file,
return -EINVAL;
value = (u8) user_value;
- ab3100_set_register(ab3100, reg, value);
- ab3100_get_register(ab3100, reg, ®value);
+ ab3100_set_register_interruptible(ab3100, reg, value);
+ ab3100_get_register_interruptible(ab3100, reg, ®value);
dev_info(ab3100->dev,
"debug write reg[0x%02x] with 0x%02x, "
@@ -696,7 +701,7 @@ static int __init ab3100_setup(struct ab3100 *ab3100)
int i;
for (i = 0; i < ARRAY_SIZE(ab3100_init_settings); i++) {
- err = ab3100_set_register(ab3100,
+ err = ab3100_set_register_interruptible(ab3100,
ab3100_init_settings[i].abreg,
ab3100_init_settings[i].setting);
if (err)
@@ -705,14 +710,14 @@ static int __init ab3100_setup(struct ab3100 *ab3100)
/*
* Special trick to make the AB3100 use the 32kHz clock (RTC)
- * bit 3 in test registe 0x02 is a special, undocumented test
+ * bit 3 in test register 0x02 is a special, undocumented test
* register bit that only exist in AB3100 P1E
*/
if (ab3100->chip_id == 0xc4) {
dev_warn(ab3100->dev,
"AB3100 P1E variant detected, "
"forcing chip to 32KHz\n");
- err = ab3100_set_test_register(ab3100, 0x02, 0x08);
+ err = ab3100_set_test_register_interruptible(ab3100, 0x02, 0x08);
}
exit_no_setup:
@@ -852,8 +857,8 @@ static int __init ab3100_probe(struct i2c_client *client,
i2c_set_clientdata(client, ab3100);
/* Read chip ID register */
- err = ab3100_get_register(ab3100, AB3100_CID,
- &ab3100->chip_id);
+ err = ab3100_get_register_interruptible(ab3100, AB3100_CID,
+ &ab3100->chip_id);
if (err) {
dev_err(&client->dev,
"could not communicate with the AB3100 analog "
diff --git a/include/linux/mfd/ab3100.h b/include/linux/mfd/ab3100.h
index 7a3f316..56343b8 100644
--- a/include/linux/mfd/ab3100.h
+++ b/include/linux/mfd/ab3100.h
@@ -86,11 +86,11 @@ struct ab3100 {
bool startup_events_read;
};
-int ab3100_set_register(struct ab3100 *ab3100, u8 reg, u8 regval);
-int ab3100_get_register(struct ab3100 *ab3100, u8 reg, u8 *regval);
-int ab3100_get_register_page(struct ab3100 *ab3100,
+int ab3100_set_register_interruptible(struct ab3100 *ab3100, u8 reg, u8 regval);
+int ab3100_get_register_interruptible(struct ab3100 *ab3100, u8 reg, u8 *regval);
+int ab3100_get_register_page_interruptible(struct ab3100 *ab3100,
u8 first_reg, u8 *regvals, u8 numregs);
-int ab3100_mask_and_set_register(struct ab3100 *ab3100,
+int ab3100_mask_and_set_register_interruptible(struct ab3100 *ab3100,
u8 reg, u8 andmask, u8 ormask);
u8 ab3100_get_chip_type(struct ab3100 *ab3100);
int ab3100_event_register(struct ab3100 *ab3100,
--
1.6.2.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH 1/5] MFD AB3100 accessor function cleanups
2009-08-13 9:49 [PATCH 1/5] MFD AB3100 accessor function cleanups Linus Walleij
@ 2009-08-16 22:56 ` Samuel Ortiz
0 siblings, 0 replies; 2+ messages in thread
From: Samuel Ortiz @ 2009-08-16 22:56 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-kernel
Hi Linus,
On Thu, Aug 13, 2009 at 11:49:23AM +0200, Linus Walleij wrote:
> This adds the _interruptible suffix to the AB3100 accessor
> functions on par with mutex_lock_interruptible() that's used
> for blocking simultaneous calls to the AB3100 acessor functions.
> Since these accesses are slow on a 100kHz I2C bus and may line
> up waiting for the mutex, we need to handle interruption by
> system shutdown or kill signals and may just as well denote that
> in the function names.
Patches [1-4] applied, thanks a lot.
The build warning fixed by patch 5 was already fixed by a previous commit on
my for-next branch (commit 2cdc59dd59f3018eddfa09fbdba8a34a6bfc37d).
Cheers,
Samuel.
> Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
> ---
> drivers/mfd/ab3100-core.c | 43 ++++++++++++++++++++++++-------------------
> include/linux/mfd/ab3100.h | 8 ++++----
> 2 files changed, 28 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/mfd/ab3100-core.c b/drivers/mfd/ab3100-core.c
> index 13e7d7b..ffb2af2 100644
> --- a/drivers/mfd/ab3100-core.c
> +++ b/drivers/mfd/ab3100-core.c
> @@ -77,7 +77,7 @@ u8 ab3100_get_chip_type(struct ab3100 *ab3100)
> }
> EXPORT_SYMBOL(ab3100_get_chip_type);
>
> -int ab3100_set_register(struct ab3100 *ab3100, u8 reg, u8 regval)
> +int ab3100_set_register_interruptible(struct ab3100 *ab3100, u8 reg, u8 regval)
> {
> u8 regandval[2] = {reg, regval};
> int err;
> @@ -109,7 +109,8 @@ int ab3100_set_register(struct ab3100 *ab3100, u8 reg, u8 regval)
> mutex_unlock(&ab3100->access_mutex);
> return 0;
> }
> -EXPORT_SYMBOL(ab3100_set_register);
> +EXPORT_SYMBOL(ab3100_set_register_interruptible);
> +
>
> /*
> * The test registers exist at an I2C bus address up one
> @@ -118,7 +119,7 @@ EXPORT_SYMBOL(ab3100_set_register);
> * anyway. It's currently only used from this file so declare
> * it static and do not export.
> */
> -static int ab3100_set_test_register(struct ab3100 *ab3100,
> +static int ab3100_set_test_register_interruptible(struct ab3100 *ab3100,
> u8 reg, u8 regval)
> {
> u8 regandval[2] = {reg, regval};
> @@ -148,7 +149,8 @@ static int ab3100_set_test_register(struct ab3100 *ab3100,
> return err;
> }
>
> -int ab3100_get_register(struct ab3100 *ab3100, u8 reg, u8 *regval)
> +
> +int ab3100_get_register_interruptible(struct ab3100 *ab3100, u8 reg, u8 *regval)
> {
> int err;
>
> @@ -202,9 +204,10 @@ int ab3100_get_register(struct ab3100 *ab3100, u8 reg, u8 *regval)
> mutex_unlock(&ab3100->access_mutex);
> return err;
> }
> -EXPORT_SYMBOL(ab3100_get_register);
> +EXPORT_SYMBOL(ab3100_get_register_interruptible);
> +
>
> -int ab3100_get_register_page(struct ab3100 *ab3100,
> +int ab3100_get_register_page_interruptible(struct ab3100 *ab3100,
> u8 first_reg, u8 *regvals, u8 numregs)
> {
> int err;
> @@ -258,9 +261,10 @@ int ab3100_get_register_page(struct ab3100 *ab3100,
> mutex_unlock(&ab3100->access_mutex);
> return err;
> }
> -EXPORT_SYMBOL(ab3100_get_register_page);
> +EXPORT_SYMBOL(ab3100_get_register_page_interruptible);
>
> -int ab3100_mask_and_set_register(struct ab3100 *ab3100,
> +
> +int ab3100_mask_and_set_register_interruptible(struct ab3100 *ab3100,
> u8 reg, u8 andmask, u8 ormask)
> {
> u8 regandval[2] = {reg, 0};
> @@ -328,7 +332,8 @@ int ab3100_mask_and_set_register(struct ab3100 *ab3100,
> mutex_unlock(&ab3100->access_mutex);
> return err;
> }
> -EXPORT_SYMBOL(ab3100_mask_and_set_register);
> +EXPORT_SYMBOL(ab3100_mask_and_set_register_interruptible);
> +
>
> /*
> * Register a simple callback for handling any AB3100 events.
> @@ -371,7 +376,7 @@ static void ab3100_work(struct work_struct *work)
> u32 fatevent;
> int err;
>
> - err = ab3100_get_register_page(ab3100, AB3100_EVENTA1,
> + err = ab3100_get_register_page_interruptible(ab3100, AB3100_EVENTA1,
> event_regs, 3);
> if (err)
> goto err_event_wq;
> @@ -435,7 +440,7 @@ static int ab3100_registers_print(struct seq_file *s, void *p)
> seq_printf(s, "AB3100 registers:\n");
>
> for (reg = 0; reg < 0xff; reg++) {
> - ab3100_get_register(ab3100, reg, &value);
> + ab3100_get_register_interruptible(ab3100, reg, &value);
> seq_printf(s, "[0x%x]: 0x%x\n", reg, value);
> }
> return 0;
> @@ -515,7 +520,7 @@ static int ab3100_get_set_reg(struct file *file,
> u8 reg = (u8) user_reg;
> u8 regvalue;
>
> - ab3100_get_register(ab3100, reg, ®value);
> + ab3100_get_register_interruptible(ab3100, reg, ®value);
>
> dev_info(ab3100->dev,
> "debug read AB3100 reg[0x%02x]: 0x%02x\n",
> @@ -547,8 +552,8 @@ static int ab3100_get_set_reg(struct file *file,
> return -EINVAL;
>
> value = (u8) user_value;
> - ab3100_set_register(ab3100, reg, value);
> - ab3100_get_register(ab3100, reg, ®value);
> + ab3100_set_register_interruptible(ab3100, reg, value);
> + ab3100_get_register_interruptible(ab3100, reg, ®value);
>
> dev_info(ab3100->dev,
> "debug write reg[0x%02x] with 0x%02x, "
> @@ -696,7 +701,7 @@ static int __init ab3100_setup(struct ab3100 *ab3100)
> int i;
>
> for (i = 0; i < ARRAY_SIZE(ab3100_init_settings); i++) {
> - err = ab3100_set_register(ab3100,
> + err = ab3100_set_register_interruptible(ab3100,
> ab3100_init_settings[i].abreg,
> ab3100_init_settings[i].setting);
> if (err)
> @@ -705,14 +710,14 @@ static int __init ab3100_setup(struct ab3100 *ab3100)
>
> /*
> * Special trick to make the AB3100 use the 32kHz clock (RTC)
> - * bit 3 in test registe 0x02 is a special, undocumented test
> + * bit 3 in test register 0x02 is a special, undocumented test
> * register bit that only exist in AB3100 P1E
> */
> if (ab3100->chip_id == 0xc4) {
> dev_warn(ab3100->dev,
> "AB3100 P1E variant detected, "
> "forcing chip to 32KHz\n");
> - err = ab3100_set_test_register(ab3100, 0x02, 0x08);
> + err = ab3100_set_test_register_interruptible(ab3100, 0x02, 0x08);
> }
>
> exit_no_setup:
> @@ -852,8 +857,8 @@ static int __init ab3100_probe(struct i2c_client *client,
> i2c_set_clientdata(client, ab3100);
>
> /* Read chip ID register */
> - err = ab3100_get_register(ab3100, AB3100_CID,
> - &ab3100->chip_id);
> + err = ab3100_get_register_interruptible(ab3100, AB3100_CID,
> + &ab3100->chip_id);
> if (err) {
> dev_err(&client->dev,
> "could not communicate with the AB3100 analog "
> diff --git a/include/linux/mfd/ab3100.h b/include/linux/mfd/ab3100.h
> index 7a3f316..56343b8 100644
> --- a/include/linux/mfd/ab3100.h
> +++ b/include/linux/mfd/ab3100.h
> @@ -86,11 +86,11 @@ struct ab3100 {
> bool startup_events_read;
> };
>
> -int ab3100_set_register(struct ab3100 *ab3100, u8 reg, u8 regval);
> -int ab3100_get_register(struct ab3100 *ab3100, u8 reg, u8 *regval);
> -int ab3100_get_register_page(struct ab3100 *ab3100,
> +int ab3100_set_register_interruptible(struct ab3100 *ab3100, u8 reg, u8 regval);
> +int ab3100_get_register_interruptible(struct ab3100 *ab3100, u8 reg, u8 *regval);
> +int ab3100_get_register_page_interruptible(struct ab3100 *ab3100,
> u8 first_reg, u8 *regvals, u8 numregs);
> -int ab3100_mask_and_set_register(struct ab3100 *ab3100,
> +int ab3100_mask_and_set_register_interruptible(struct ab3100 *ab3100,
> u8 reg, u8 andmask, u8 ormask);
> u8 ab3100_get_chip_type(struct ab3100 *ab3100);
> int ab3100_event_register(struct ab3100 *ab3100,
> --
> 1.6.2.1
>
--
Intel Open Source Technology Centre
http://oss.intel.com/
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2009-08-16 22:54 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-13 9:49 [PATCH 1/5] MFD AB3100 accessor function cleanups Linus Walleij
2009-08-16 22:56 ` Samuel Ortiz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox