All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Keeping <john@metanate.com>
To: Heiko Stuebner <heiko@sntech.de>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	linux-gpio@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	John Keeping <john@metanate.com>
Subject: [RFC PATCH 1/4] pinctrl: rockchip: remove unnecessary locking
Date: Mon, 13 Mar 2017 18:38:10 +0000	[thread overview]
Message-ID: <20170313183813.3582-2-john@metanate.com> (raw)
In-Reply-To: <20170313183813.3582-1-john@metanate.com>

regmap_update_bits does its own locking and everything else accessed
here is a local variable so there is no need to lock around it.

I originally wrote this on to of v4.4 which doesn't have the split
registers for drive strength, where some additional locking might be
necessary.  But I don't think it can be "slock" given that the following
patches will convert that to a raw spinlock and regmap uses a normal
spinlock internally.

Signed-off-by: John Keeping <john@metanate.com>
---
 drivers/pinctrl/pinctrl-rockchip.c | 20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index a838c8bb3129..1defe83a5c4d 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -649,7 +649,6 @@ static int rockchip_set_mux(struct rockchip_pin_bank *bank, int pin, int mux)
 	int iomux_num = (pin / 8);
 	struct regmap *regmap;
 	int reg, ret, mask, mux_type;
-	unsigned long flags;
 	u8 bit;
 	u32 data, rmask;
 
@@ -698,15 +697,11 @@ static int rockchip_set_mux(struct rockchip_pin_bank *bank, int pin, int mux)
 	if (ctrl->iomux_recalc && (mux_type & IOMUX_RECALCED))
 		ctrl->iomux_recalc(bank->bank_num, pin, &reg, &bit, &mask);
 
-	spin_lock_irqsave(&bank->slock, flags);
-
 	data = (mask << (bit + 16));
 	rmask = data | (data >> 16);
 	data |= (mux & mask) << bit;
 	ret = regmap_update_bits(regmap, reg, rmask, data);
 
-	spin_unlock_irqrestore(&bank->slock, flags);
-
 	return ret;
 }
 
@@ -1132,7 +1127,6 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
 	struct rockchip_pinctrl *info = bank->drvdata;
 	struct rockchip_pin_ctrl *ctrl = info->ctrl;
 	struct regmap *regmap;
-	unsigned long flags;
 	int reg, ret, i;
 	u32 data, rmask, rmask_bits, temp;
 	u8 bit;
@@ -1160,8 +1154,6 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
 		return ret;
 	}
 
-	spin_lock_irqsave(&bank->slock, flags);
-
 	switch (drv_type) {
 	case DRV_TYPE_IO_1V8_3V0_AUTO:
 	case DRV_TYPE_IO_3V3_ONLY:
@@ -1182,17 +1174,14 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
 			rmask = BIT(15) | BIT(31);
 			data |= BIT(31);
 			ret = regmap_update_bits(regmap, reg, rmask, data);
-			if (ret) {
-				spin_unlock_irqrestore(&bank->slock, flags);
+			if (ret)
 				return ret;
-			}
 
 			rmask = 0x3 | (0x3 << 16);
 			temp |= (0x3 << 16);
 			reg += 0x4;
 			ret = regmap_update_bits(regmap, reg, rmask, temp);
 
-			spin_unlock_irqrestore(&bank->slock, flags);
 			return ret;
 		case 18 ... 21:
 			/* setting fully enclosed in the second register */
@@ -1200,7 +1189,6 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
 			bit -= 16;
 			break;
 		default:
-			spin_unlock_irqrestore(&bank->slock, flags);
 			dev_err(info->dev, "unsupported bit: %d for pinctrl drive type: %d\n",
 				bit, drv_type);
 			return -EINVAL;
@@ -1212,7 +1200,6 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
 		rmask_bits = RK3288_DRV_BITS_PER_PIN;
 		break;
 	default:
-		spin_unlock_irqrestore(&bank->slock, flags);
 		dev_err(info->dev, "unsupported pinctrl drive type: %d\n",
 			drv_type);
 		return -EINVAL;
@@ -1224,7 +1211,6 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
 	data |= (ret << bit);
 
 	ret = regmap_update_bits(regmap, reg, rmask, data);
-	spin_unlock_irqrestore(&bank->slock, flags);
 
 	return ret;
 }
@@ -1336,16 +1322,12 @@ static int rockchip_set_pull(struct rockchip_pin_bank *bank,
 			return ret;
 		}
 
-		spin_lock_irqsave(&bank->slock, flags);
-
 		/* enable the write to the equivalent lower bits */
 		data = ((1 << RK3188_PULL_BITS_PER_PIN) - 1) << (bit + 16);
 		rmask = data | (data >> 16);
 		data |= (ret << bit);
 
 		ret = regmap_update_bits(regmap, reg, rmask, data);
-
-		spin_unlock_irqrestore(&bank->slock, flags);
 		break;
 	default:
 		dev_err(info->dev, "unsupported pinctrl type\n");
-- 
2.12.0.377.gf910686b23.dirty


WARNING: multiple messages have this Message-ID (diff)
From: john@metanate.com (John Keeping)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 1/4] pinctrl: rockchip: remove unnecessary locking
Date: Mon, 13 Mar 2017 18:38:10 +0000	[thread overview]
Message-ID: <20170313183813.3582-2-john@metanate.com> (raw)
In-Reply-To: <20170313183813.3582-1-john@metanate.com>

regmap_update_bits does its own locking and everything else accessed
here is a local variable so there is no need to lock around it.

I originally wrote this on to of v4.4 which doesn't have the split
registers for drive strength, where some additional locking might be
necessary.  But I don't think it can be "slock" given that the following
patches will convert that to a raw spinlock and regmap uses a normal
spinlock internally.

Signed-off-by: John Keeping <john@metanate.com>
---
 drivers/pinctrl/pinctrl-rockchip.c | 20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index a838c8bb3129..1defe83a5c4d 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -649,7 +649,6 @@ static int rockchip_set_mux(struct rockchip_pin_bank *bank, int pin, int mux)
 	int iomux_num = (pin / 8);
 	struct regmap *regmap;
 	int reg, ret, mask, mux_type;
-	unsigned long flags;
 	u8 bit;
 	u32 data, rmask;
 
@@ -698,15 +697,11 @@ static int rockchip_set_mux(struct rockchip_pin_bank *bank, int pin, int mux)
 	if (ctrl->iomux_recalc && (mux_type & IOMUX_RECALCED))
 		ctrl->iomux_recalc(bank->bank_num, pin, &reg, &bit, &mask);
 
-	spin_lock_irqsave(&bank->slock, flags);
-
 	data = (mask << (bit + 16));
 	rmask = data | (data >> 16);
 	data |= (mux & mask) << bit;
 	ret = regmap_update_bits(regmap, reg, rmask, data);
 
-	spin_unlock_irqrestore(&bank->slock, flags);
-
 	return ret;
 }
 
@@ -1132,7 +1127,6 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
 	struct rockchip_pinctrl *info = bank->drvdata;
 	struct rockchip_pin_ctrl *ctrl = info->ctrl;
 	struct regmap *regmap;
-	unsigned long flags;
 	int reg, ret, i;
 	u32 data, rmask, rmask_bits, temp;
 	u8 bit;
@@ -1160,8 +1154,6 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
 		return ret;
 	}
 
-	spin_lock_irqsave(&bank->slock, flags);
-
 	switch (drv_type) {
 	case DRV_TYPE_IO_1V8_3V0_AUTO:
 	case DRV_TYPE_IO_3V3_ONLY:
@@ -1182,17 +1174,14 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
 			rmask = BIT(15) | BIT(31);
 			data |= BIT(31);
 			ret = regmap_update_bits(regmap, reg, rmask, data);
-			if (ret) {
-				spin_unlock_irqrestore(&bank->slock, flags);
+			if (ret)
 				return ret;
-			}
 
 			rmask = 0x3 | (0x3 << 16);
 			temp |= (0x3 << 16);
 			reg += 0x4;
 			ret = regmap_update_bits(regmap, reg, rmask, temp);
 
-			spin_unlock_irqrestore(&bank->slock, flags);
 			return ret;
 		case 18 ... 21:
 			/* setting fully enclosed in the second register */
@@ -1200,7 +1189,6 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
 			bit -= 16;
 			break;
 		default:
-			spin_unlock_irqrestore(&bank->slock, flags);
 			dev_err(info->dev, "unsupported bit: %d for pinctrl drive type: %d\n",
 				bit, drv_type);
 			return -EINVAL;
@@ -1212,7 +1200,6 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
 		rmask_bits = RK3288_DRV_BITS_PER_PIN;
 		break;
 	default:
-		spin_unlock_irqrestore(&bank->slock, flags);
 		dev_err(info->dev, "unsupported pinctrl drive type: %d\n",
 			drv_type);
 		return -EINVAL;
@@ -1224,7 +1211,6 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
 	data |= (ret << bit);
 
 	ret = regmap_update_bits(regmap, reg, rmask, data);
-	spin_unlock_irqrestore(&bank->slock, flags);
 
 	return ret;
 }
@@ -1336,16 +1322,12 @@ static int rockchip_set_pull(struct rockchip_pin_bank *bank,
 			return ret;
 		}
 
-		spin_lock_irqsave(&bank->slock, flags);
-
 		/* enable the write to the equivalent lower bits */
 		data = ((1 << RK3188_PULL_BITS_PER_PIN) - 1) << (bit + 16);
 		rmask = data | (data >> 16);
 		data |= (ret << bit);
 
 		ret = regmap_update_bits(regmap, reg, rmask, data);
-
-		spin_unlock_irqrestore(&bank->slock, flags);
 		break;
 	default:
 		dev_err(info->dev, "unsupported pinctrl type\n");
-- 
2.12.0.377.gf910686b23.dirty

  reply	other threads:[~2017-03-13 18:38 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-13 18:38 [RFC PATCH 0/4] pinctrl: rockchip: PREEMPT_RT_FULL fixes John Keeping
2017-03-13 18:38 ` John Keeping
2017-03-13 18:38 ` John Keeping [this message]
2017-03-13 18:38   ` [RFC PATCH 1/4] pinctrl: rockchip: remove unnecessary locking John Keeping
2017-03-15 16:25   ` Heiko Stuebner
2017-03-15 16:25     ` Heiko Stuebner
2017-03-15 16:25     ` Heiko Stuebner
2017-03-13 18:38 ` [RFC PATCH 2/4] pinctrl: rockchip: convert to raw spinlock John Keeping
2017-03-13 18:38   ` John Keeping
2017-03-15 16:28   ` Heiko Stuebner
2017-03-15 16:28     ` Heiko Stuebner
2017-03-15 16:28     ` Heiko Stuebner
2017-03-15 16:41     ` Heiko Stuebner
2017-03-15 16:41       ` Heiko Stuebner
2017-03-15 16:41       ` Heiko Stuebner
2017-03-15 16:50       ` Heiko Stuebner
2017-03-15 16:50         ` Heiko Stuebner
2017-03-15 16:59       ` John Keeping
2017-03-15 16:59         ` John Keeping
2017-03-15 17:12         ` Heiko Stuebner
2017-03-15 17:12           ` Heiko Stuebner
2017-03-13 18:38 ` [RFC PATCH 3/4] pinctrl: rockchip: split out verification of mux settings John Keeping
2017-03-13 18:38   ` John Keeping
2017-03-15 16:34   ` Heiko Stuebner
2017-03-15 16:34     ` Heiko Stuebner
2017-03-13 18:38 ` [RFC PATCH 4/4] pinctrl: rockchip: avoid hardirq-unsafe functions in irq_chip John Keeping
2017-03-13 18:38   ` John Keeping
     [not found]   ` <20170313183813.3582-5-john-HooS5bfzL4hWk0Htik3J/w@public.gmane.org>
2017-03-15 17:04     ` Heiko Stuebner
2017-03-15 17:04       ` Heiko Stuebner
2017-03-15 17:04       ` Heiko Stuebner
2017-03-15 13:12 ` [RFC PATCH 0/4] pinctrl: rockchip: PREEMPT_RT_FULL fixes Linus Walleij
2017-03-15 13:12   ` Linus Walleij
2017-03-15 18:17   ` Julia Cartwright
2017-03-15 18:17     ` Julia Cartwright
2017-03-15 18:17     ` Julia Cartwright
2017-03-15 17:09 ` Heiko Stuebner
2017-03-15 17:09   ` Heiko Stuebner

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=20170313183813.3582-2-john@metanate.com \
    --to=john@metanate.com \
    --cc=heiko@sntech.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    /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 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.