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

[Tudor.Ambarus at microchip.com]

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.