[PATCH v7 1/3] mtd: spi-nor: spansion: Add support for Read/Write Any Register

Takahiro Kuwano tkuw584924 at gmail.com
Mon Jan 31 03:10:18 PST 2022


On 1/31/2022 4:50 PM, Tudor.Ambarus at microchip.com wrote:
> On 1/28/22 18:20, Pratyush Yadav wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 28/01/22 09:43AM, Tudor.Ambarus at microchip.com wrote:
>>> On 7/19/21 11:03, tkuw584924 at gmail.com wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>
>>>> From: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
>>>>
>>>> Some of Spansion/Cypress chips support Read/Write Any Register commands.
>>>> These commands are mainly used to write volatile registers.
>>>>
>>>> The Read Any Register instruction (65h) is followed by register address
>>>> and dummy cycles, then the selected register byte is returned.
>>>>
>>>> The Write Any Register instruction (71h) is followed by register address
>>>> and register byte to write.
>>>>
>>>> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
>>>> ---
>>>> Changes in v7:
>>>>   - No change
>>>>
>>>> Changes in v6:
>>>>   - Add helper functions for controller_ops
>>>>   - Add 'reg_addr_width' parameter to spansion_read/write_any_reg()
>>>>   - Remove spi_nor_write_enable() from spansion_write_any_reg() and modified
>>>>     function header comment
>>>>
>>>> Changes in v5:
>>>>   - Fix 'if (ret == 1)' to 'if (ret < 0)' in spansion_read_any_reg()
>>>>
>>>> Changes in v4:
>>>>   - Fix dummy cycle calculation in spansion_read_any_reg()
>>>>   - Modify comment for spansion_write_any_reg()
>>>>
>>>> Changes in v3:
>>>>   - Cleanup implementation
>>>>
>>>>  drivers/mtd/spi-nor/spansion.c | 142 +++++++++++++++++++++++++++++++++
>>>>  1 file changed, 142 insertions(+)
>>>>
>>>> +/**
>>>> + * spansion_read_any_reg() - Read Any Register.
>>>> + * @nor:               pointer to a 'struct spi_nor'
>>>> + * @reg_addr:          register address
>>>> + * @reg_addr_width:    number of address bytes
>>>> + * @reg_dummy:         number of dummy cycles for register read
>>>> + * @reg_val:           pointer to a buffer where the register value is copied
>>>> + *
>>>> + * Return: 0 on success, -errno otherwise.
>>>> + */
>>>> +static int spansion_read_any_reg(struct spi_nor *nor, u32 reg_addr,
>>>> +                                u8 reg_addr_width, u8 reg_dummy, u8 *reg_val)
>>>
>>> how about making this a generic core method? I'm sure there are other SPI NOR
>>> vendors that use registers indexed by address. How about introducing:
>>>
>>> struct spi_nor_reg {
>>>       u32 addr;
>>>       u8 addr_width;
>>>       u8 dummy;
>>>       u8 opcode;
>>>       u8 val;
>>> };
>>>
>>> and passing a pointer to this struct as argument.
>>
>> Wouldn't it be simpler if they fill it in a struct spi_mem_op, and then
>> just pass that in as a "template"?
>>
> sure, sounds fine. I would like a generic method with less function parameters,
> if possible. Let's see what Takahiro will propose.

So, I would propose...
In core, spi_nor_spimem_{read,write}_reg(struct spi_nor *, struct spi_mem_op *)
that calls spi_nor_spimem_setup() and spi_mem_exec_op().
In vendor specific, spi_mem_op is filled and passed to the core method.

Thank you for your comments,
Takahiro



More information about the linux-mtd mailing list