linux-leds.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
To: Bryan Wu <cooloney@gmail.com>,
	Peter Meerwald <p.meerwald@bct-electronic.com>,
	Richard Purdie <rpurdie@rpsys.net>,
	Linux LED Subsystem <linux-leds@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Cc: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
Subject: [PATCH v4 3/5] leds-pca9633: Add mutex to the ledout register
Date: Wed, 14 Aug 2013 09:14:26 +0200	[thread overview]
Message-ID: <1376464470-6845-6-git-send-email-ricardo.ribalda@gmail.com> (raw)
In-Reply-To: <1376464470-6845-1-git-send-email-ricardo.ribalda@gmail.com>

To update an LED a register has to be read, updated and writen. If
another LED whas been updated at the same time, this could lead into
wrong updates.

This patch adds a common mutex to all the leds of the same chip to
protect the ledout register.

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/leds/leds-pca9633.c |   85 +++++++++++++++++++++++++++----------------
 1 file changed, 54 insertions(+), 31 deletions(-)

diff --git a/drivers/leds/leds-pca9633.c b/drivers/leds/leds-pca9633.c
index 9b7893c..097b098 100644
--- a/drivers/leds/leds-pca9633.c
+++ b/drivers/leds/leds-pca9633.c
@@ -93,9 +93,17 @@ enum pca9633_cmd {
 	BLINK_SET,
 };
 
-struct pca9633_led {
-	struct i2c_client *client;
+struct pca9633_led;
+
+struct pca9633 {
 	struct pca9633_chipdef *chipdef;
+	struct mutex mutex;
+	struct i2c_client *client;
+	struct pca9633_led *leds;
+};
+
+struct pca9633_led {
+	struct pca9633 *chip;
 	struct work_struct work;
 	enum led_brightness brightness;
 	struct led_classdev led_cdev;
@@ -108,52 +116,59 @@ struct pca9633_led {
 
 static void pca9633_brightness_work(struct pca9633_led *pca9633)
 {
-	u8 ledout_addr = pca9633->chipdef->ledout_base + (pca9633->led_num / 4);
+	u8 ledout_addr = pca9633->chip->chipdef->ledout_base
+		+ (pca9633->led_num / 4);
 	u8 ledout;
 	int shift = 2 * (pca9633->led_num % 4);
 	u8 mask = 0x3 << shift;
 
-	ledout = i2c_smbus_read_byte_data(pca9633->client, ledout_addr);
+	mutex_lock(&pca9633->chip->mutex);
+	ledout = i2c_smbus_read_byte_data(pca9633->chip->client, ledout_addr);
 	switch (pca9633->brightness) {
 	case LED_FULL:
-		i2c_smbus_write_byte_data(pca9633->client, ledout_addr,
+		i2c_smbus_write_byte_data(pca9633->chip->client, ledout_addr,
 			(ledout & ~mask) | (PCA9633_LED_ON << shift));
 		break;
 	case LED_OFF:
-		i2c_smbus_write_byte_data(pca9633->client, ledout_addr,
+		i2c_smbus_write_byte_data(pca9633->chip->client, ledout_addr,
 			ledout & ~mask);
 		break;
 	default:
-		i2c_smbus_write_byte_data(pca9633->client,
+		i2c_smbus_write_byte_data(pca9633->chip->client,
 			PCA9633_PWM_BASE + pca9633->led_num,
 			pca9633->brightness);
-		i2c_smbus_write_byte_data(pca9633->client, ledout_addr,
+		i2c_smbus_write_byte_data(pca9633->chip->client, ledout_addr,
 			(ledout & ~mask) | (PCA9633_LED_PWM << shift));
 		break;
 	}
+	mutex_unlock(&pca9633->chip->mutex);
 }
 
 static void pca9633_blink_work(struct pca9633_led *pca9633)
 {
-	u8 ledout_addr = pca9633->chipdef->ledout_base + (pca9633->led_num / 4);
-	u8 ledout = i2c_smbus_read_byte_data(pca9633->client, ledout_addr);
-	u8 mode2 = i2c_smbus_read_byte_data(pca9633->client, PCA9633_MODE2);
+	u8 ledout_addr = pca9633->chip->chipdef->ledout_base +
+		(pca9633->led_num / 4);
+	u8 ledout;
+	u8 mode2 = i2c_smbus_read_byte_data(pca9633->chip->client, PCA9633_MODE2);
 	int shift = 2 * (pca9633->led_num % 4);
 	u8 mask = 0x3 << shift;
 
-	i2c_smbus_write_byte_data(pca9633->client, pca9633->chipdef->grppwm,
-		pca9633->gdc);
+	i2c_smbus_write_byte_data(pca9633->chip->client,
+			pca9633->chip->chipdef->grppwm,	pca9633->gdc);
 
-	i2c_smbus_write_byte_data(pca9633->client, pca9633->chipdef->grpfreq,
-		pca9633->gfrq);
+	i2c_smbus_write_byte_data(pca9633->chip->client,
+			pca9633->chip->chipdef->grpfreq, pca9633->gfrq);
 
 	if (!(mode2 & PCA9633_MODE2_DMBLNK))
-		i2c_smbus_write_byte_data(pca9633->client, PCA9633_MODE2,
+		i2c_smbus_write_byte_data(pca9633->chip->client, PCA9633_MODE2,
 			mode2 | PCA9633_MODE2_DMBLNK);
 
+	mutex_lock(&pca9633->chip->mutex);
+	ledout = i2c_smbus_read_byte_data(pca9633->chip->client, ledout_addr);
 	if ((ledout & mask) != (PCA9633_LED_GRP_PWM << shift))
-		i2c_smbus_write_byte_data(pca9633->client, ledout_addr,
+		i2c_smbus_write_byte_data(pca9633->chip->client, ledout_addr,
 			(ledout & ~mask) | (PCA9633_LED_GRP_PWM << shift));
+	mutex_unlock(&pca9633->chip->mutex);
 }
 
 static void pca9633_work(struct work_struct *work)
@@ -318,6 +333,7 @@ pca9633_dt_init(struct i2c_client *client)
 static int pca9633_probe(struct i2c_client *client,
 					const struct i2c_device_id *id)
 {
+	struct pca9633 *pca9633_chip;
 	struct pca9633_led *pca9633;
 	struct pca9633_platform_data *pdata;
 	struct pca9633_chipdef *chip;
@@ -341,17 +357,30 @@ static int pca9633_probe(struct i2c_client *client,
 		return -EINVAL;
 	}
 
+	pca9633_chip = devm_kzalloc(&client->dev, sizeof(*pca9633_chip),
+								GFP_KERNEL);
+	if (!pca9633_chip)
+		return -ENOMEM;
 	pca9633 = devm_kzalloc(&client->dev, chip->n_leds * sizeof(*pca9633),
 								GFP_KERNEL);
 	if (!pca9633)
 		return -ENOMEM;
 
-	i2c_set_clientdata(client, pca9633);
+	i2c_set_clientdata(client, pca9633_chip);
+
+	mutex_init(&pca9633_chip->mutex);
+	pca9633_chip->chipdef = chip;
+	pca9633_chip->client = client;
+	pca9633_chip->leds = pca9633;
+
+	/* Turn off LEDs by default*/
+	i2c_smbus_write_byte_data(client, chip->ledout_base, 0x00);
+	if (chip->n_leds > 4)
+		i2c_smbus_write_byte_data(client, chip->ledout_base + 1, 0x00);
 
 	for (i = 0; i < chip->n_leds; i++) {
-		pca9633[i].client = client;
 		pca9633[i].led_num = i;
-		pca9633[i].chipdef = chip;
+		pca9633[i].chip = pca9633_chip;
 
 		/* Platform data can specify LED names and default triggers */
 		if (pdata && i < pdata->leds.num_leds) {
@@ -389,11 +418,6 @@ static int pca9633_probe(struct i2c_client *client,
 	if (pdata && pdata->outdrv == PCA9633_OPEN_DRAIN)
 		i2c_smbus_write_byte_data(client, PCA9633_MODE2, 0x01);
 
-	/* Turn off LEDs */
-	i2c_smbus_write_byte_data(client, chip->ledout_base, 0x00);
-	if (chip->n_leds > 4)
-		i2c_smbus_write_byte_data(client, chip->ledout_base + 1, 0x00);
-
 	return 0;
 
 exit:
@@ -407,14 +431,13 @@ exit:
 
 static int pca9633_remove(struct i2c_client *client)
 {
-	struct pca9633_led *pca9633 = i2c_get_clientdata(client);
+	struct pca9633 *pca9633 = i2c_get_clientdata(client);
 	int i;
 
-	for (i = 0; i < pca9633->chipdef->n_leds; i++)
-		if (pca9633[i].client != NULL) {
-			led_classdev_unregister(&pca9633[i].led_cdev);
-			cancel_work_sync(&pca9633[i].work);
-		}
+	for (i = 0; i < pca9633->chipdef->n_leds; i++) {
+		led_classdev_unregister(&pca9633->leds[i].led_cdev);
+		cancel_work_sync(&pca9633->leds[i].work);
+	}
 
 	return 0;
 }
-- 
1.7.10.4

  parent reply	other threads:[~2013-08-14  7:14 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-14  7:14 [PATCH v4 0/5] PCA9633: Add support to PCA9634 and fix some problems Ricardo Ribalda Delgado
2013-08-14  7:14 ` [PATCH v4 1/5] leds-pca9633: Add support for PCA9634 Ricardo Ribalda Delgado
2013-08-14 21:00   ` Bryan Wu
2013-08-14 21:05     ` Ricardo Ribalda Delgado
2013-08-14  7:14 ` [PATCH v4 1/4] leds-pca9633: Unique naming of the LEDs Ricardo Ribalda Delgado
2013-08-14  7:14 ` [PATCH v4 2/4] leds-pca9633: Add mutex to the ledout register Ricardo Ribalda Delgado
2013-08-14  7:14 ` [PATCH v4 2/5] leds-pca9633: Unique naming of the LEDs Ricardo Ribalda Delgado
2013-08-14  7:14 ` Ricardo Ribalda Delgado [this message]
2013-08-14  7:14 ` [PATCH v4 3/4] leds-pca9633: Rename to leds-pca963x Ricardo Ribalda Delgado
2013-08-14  7:14 ` [PATCH v4 4/5] " Ricardo Ribalda Delgado
2013-08-14  7:14 ` [PATCH v4 4/4] leds-pca963x: Fix device tree parsing Ricardo Ribalda Delgado
2013-08-14  7:14 ` [PATCH v4 5/5] " Ricardo Ribalda Delgado

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1376464470-6845-6-git-send-email-ricardo.ribalda@gmail.com \
    --to=ricardo.ribalda@gmail.com \
    --cc=cooloney@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=p.meerwald@bct-electronic.com \
    --cc=rpurdie@rpsys.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).