Re: [PATCH 1/2] mtd: spi-nor: Add Global Block Unlock command

[Tudor.Ambarus]

On 1/20/21 2:29 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Tudor,

Hi, Pratyush,

Thanks for reviewing this.

> 
> On 20/01/21 12:54PM, Tudor Ambarus wrote:
>> The Global Block Unlock command has different names depending
>> on the manufacturer, but always the same command value: 0x98.
>> Macronix's MX25U12835F names it Gang Block Unlock, Winbound's
>> W25Q128FV names it Global Block Unlock and Microchip's
>> SST26VF064B names it Global Block Protection Unlock.
>>
>> Used in the Individual Block Protection mode, which is mutual
> 
> Nitpick: s/mutual/mutually/
> 
>> exclusive with the Block Protection mode (BP0-3).
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxxxxx>
>> ---
>>  drivers/mtd/spi-nor/core.c  | 35 +++++++++++++++++++++++++++++++++++
>>  drivers/mtd/spi-nor/core.h  |  1 +
>>  include/linux/mtd/spi-nor.h |  1 +
>>  3 files changed, 37 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index 20df44b753da..cb275608cb7f 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -853,6 +853,41 @@ int spi_nor_wait_till_ready(struct spi_nor *nor)
>>                                                   DEFAULT_READY_WAIT_JIFFIES);
>>  }
>>
>> +/**
>> + * spi_nor_global_block_unlock() - Unlock Global Block Protection.
>> + * @nor:     pointer to 'struct spi_nor'.
>> + *
>> + * Return: 0 on success, -errno otherwise.
>> + */
>> +int spi_nor_global_block_unlock(struct spi_nor *nor)
>> +{
>> +     int ret;
>> +
>> +     ret = spi_nor_write_enable(nor);
>> +     if (ret)
>> +             return ret;
>> +
>> +     if (nor->spimem) {
>> +             struct spi_mem_op op =
>> +                     SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_GBULK, 1),
> 
> Set the buswidth to 0...
> 
>> +                                SPI_MEM_OP_NO_ADDR,
>> +                                SPI_MEM_OP_NO_DUMMY,
>> +                                SPI_MEM_OP_NO_DATA);
> 
> ... and run the op through spi_nor_spimem_setup_op().

Right, I thought that SPINOR_OP_GBULK is available just in single SPI
mode, but I see it can be issued in QPI mode as well. Will change.

Cheers,
ta
> 
> With this fixed,
> 
> Reviewed-by: Pratyush Yadav <p.yadav@xxxxxx>
> 
>> +
>> +             ret = spi_mem_exec_op(nor->spimem, &op);
>> +     } else {
>> +             ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_GBULK,
>> +                                                    NULL, 0);
>> +     }
>> +
>> +     if (ret) {
>> +             dev_dbg(nor->dev, "error %d on Global Block Unlock\n", ret);
>> +             return ret;
>> +     }
>> +
>> +     return spi_nor_wait_till_ready(nor);
>> +}
>> +
>>  /**
>>   * spi_nor_write_sr() - Write the Status Register.
>>   * @nor:     pointer to 'struct spi_nor'.
>> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
>> index d631ee299de3..eb26796db026 100644
>> --- a/drivers/mtd/spi-nor/core.h
>> +++ b/drivers/mtd/spi-nor/core.h
>> @@ -434,6 +434,7 @@ int spi_nor_write_disable(struct spi_nor *nor);
>>  int spi_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable);
>>  int spi_nor_write_ear(struct spi_nor *nor, u8 ear);
>>  int spi_nor_wait_till_ready(struct spi_nor *nor);
>> +int spi_nor_global_block_unlock(struct spi_nor *nor);
>>  int spi_nor_lock_and_prep(struct spi_nor *nor);
>>  void spi_nor_unlock_and_unprep(struct spi_nor *nor);
>>  int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor);
>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
>> index d13958de6d8a..a0d572855444 100644
>> --- a/include/linux/mtd/spi-nor.h
>> +++ b/include/linux/mtd/spi-nor.h
>> @@ -53,6 +53,7 @@
>>  #define SPINOR_OP_WREAR              0xc5    /* Write Extended Address Register */
>>  #define SPINOR_OP_SRSTEN     0x66    /* Software Reset Enable */
>>  #define SPINOR_OP_SRST               0x99    /* Software Reset */
>> +#define SPINOR_OP_GBULK              0x98    /* Global Block Unlock */
>>
>>  /* 4-byte address opcodes - used on Spansion and some Macronix flashes. */
>>  #define SPINOR_OP_READ_4B    0x13    /* Read data bytes (low frequency) */
> 
> --
> Regards,
> Pratyush Yadav
> Texas Instruments India
>