Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1607673

Re: [PATCH v4 1/4] pinctrl: rockchip: remove unnecessary locking

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

Hello John-

One quick question below.  Apologies if this has been covered, but just
want to be sure.

On Thu, Mar 23, 2017 at 10:59:28AM +0000, John Keeping wrote:
> 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.
> 
> Signed-off-by: John Keeping <john@metanate.com>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> ---
> v3: unchanged
> v2.1:
> - Remove RK2928 locking in rockchip_set_pull()
> v2:
> - Also remove locking in rockchip_set_schmitt()
> ---
>  drivers/pinctrl/pinctrl-rockchip.c | 33 ++-------------------------------
>  1 file changed, 2 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index bd4b63f66220..6568c867bdcd 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
[..]
> @@ -1185,17 +1177,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);

Killing the lock here means the writes to to this pair of registers (reg
and reg + 4) can be observed non-atomically.  Have you convinced
yourself that this isn't a problem?

   Julia

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v4 0/4] pinctrl: rockchip: PREEMPT_RT_FULL fixes John Keeping <john@metanate.com> - 2017-03-23 12:10 +0100
  [PATCH v4 2/4] pinctrl: rockchip: convert to raw spinlock John Keeping <john@metanate.com> - 2017-03-23 12:10 +0100
  [PATCH v4 1/4] pinctrl: rockchip: remove unnecessary locking John Keeping <john@metanate.com> - 2017-03-23 12:10 +0100
    Re: [PATCH v4 1/4] pinctrl: rockchip: remove unnecessary locking Julia Cartwright <julia@ni.com> - 2017-03-23 17:20 +0100
      Re: [PATCH v4 1/4] pinctrl: rockchip: remove unnecessary locking John Keeping <john@metanate.com> - 2017-03-23 19:00 +0100
        Re: [PATCH v4 1/4] pinctrl: rockchip: remove unnecessary locking Heiko Stübner <heiko@sntech.de> - 2017-03-23 19:00 +0100
          Re: [PATCH v4 1/4] pinctrl: rockchip: remove unnecessary locking Julia Cartwright <julia@ni.com> - 2017-03-23 19:30 +0100
            Re: [PATCH v4 1/4] pinctrl: rockchip: remove unnecessary locking Heiko Stübner <heiko@sntech.de> - 2017-03-23 21:10 +0100
              Re: [PATCH v4 1/4] pinctrl: rockchip: remove unnecessary locking Julia Cartwright <julia@ni.com> - 2017-03-23 21:50 +0100

csiph-web