[PATCH linux dev-4.7 07/12] mtd: spi-nor: aspeed: make locking more explicit

[Joel Stanley]

On Fri, Oct 14, 2016 at 11:07 PM, Cédric Le Goater <clg at kaod.org> wrote:
> The routine aspeed_smc_stop_user() and aspeed_smc_start_user() hide
> the fact that they lock an unlock the controller. Change that to make
> it obvious in the code.
>
> Signed-off-by: Cédric Le Goater <clg at kaod.org>
> ---
>  drivers/mtd/spi-nor/aspeed-smc.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
> index a47eecfdfda2..b22e07d3ef32 100644
> --- a/drivers/mtd/spi-nor/aspeed-smc.c
> +++ b/drivers/mtd/spi-nor/aspeed-smc.c
> @@ -276,8 +276,6 @@ static void aspeed_smc_start_user(struct spi_nor *nor)
>         struct aspeed_smc_per_chip *chip = nor->priv;
>         u32 ctl = chip->ctl_val[smc_base];
>
> -       mutex_lock(&chip->controller->mutex);
> -

This removes all locking from start_user and stop_user. There are some
accesses and register writes in both of these functions, is that okay?

What is the lock (supposed to be) protecting?

(I note that you effectively add it back in patch 9 where you
introduce DMA, by putting locks in aspeed_smc_read_user and write_user
that then calls start_user.)


>         ctl |= CONTROL_SPI_COMMAND_MODE_USER |
>                 CONTROL_SPI_CE_STOP_ACTIVE_CONTROL;
>         writel(ctl, chip->ctl);
> @@ -296,19 +294,21 @@ static void aspeed_smc_stop_user(struct spi_nor *nor)
>
>         writel(ctl2, chip->ctl);        /* stop user CE control */
>         writel(ctl, chip->ctl);         /* default to fread or read */
> -
> -       mutex_unlock(&chip->controller->mutex);
>  }
>
>  static int aspeed_smc_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
>  {
>         struct aspeed_smc_per_chip *chip = nor->priv;
>
> +       mutex_lock(&chip->controller->mutex);
> +
>         aspeed_smc_start_user(nor);
>         aspeed_smc_to_fifo(chip->base, &opcode, 1);
>         aspeed_smc_from_fifo(buf, chip->base, len);
>         aspeed_smc_stop_user(nor);
>
> +       mutex_unlock(&chip->controller->mutex);
> +
>         return 0;
>  }
>
> @@ -317,11 +317,15 @@ static int aspeed_smc_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf,
>  {
>         struct aspeed_smc_per_chip *chip = nor->priv;
>
> +       mutex_lock(&chip->controller->mutex);
> +
>         aspeed_smc_start_user(nor);
>         aspeed_smc_to_fifo(chip->base, &opcode, 1);
>         aspeed_smc_to_fifo(chip->base, buf, len);
>         aspeed_smc_stop_user(nor);
>
> +       mutex_unlock(&chip->controller->mutex);
> +
>         return 0;
>  }
>
> --
> 2.7.4
>