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

[Tudor.Ambarus at microchip.com]

On 1/31/22 13:10, Takahiro Kuwano wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> 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.
> 

how about the following?

static ssize_t spi_nor_spimem_read_reg(struct spi_nor *nor, struct spi_mem_op *op)                    
{                                                                                  
        bool usebouncebuf;                                                      
        ssize_t nbytes;                                                         
        int error;                                                              
                                                                                
        spi_nor_spimem_setup_op(nor, &op, nor->read_proto);                     
                                                                                
        /* convert the dummy cycles to the number of bytes */                   
        op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;            
        if (spi_nor_protocol_is_dtr(nor->read_proto))                           
                op.dummy.nbytes *= 2;                                           
                                                                                
        usebouncebuf = spi_nor_spimem_bounce(nor, &op);                         
                                                                                                                                          
        ret = spi_nor_spimem_exec_op(nor, &op);                       
        if (ret)                                                      
               return ret;                                           
        nbytes = op.data.nbytes;                                                                                                
                                                                               
        if (usebouncebuf && nbytes > 0)                                         
                memcpy(buf, op.data.buf.in, nbytes);                            
                                                                                
        return nbytes;                                                          
}  

ssize_t spi_nor_read_reg(struct spi_nor *nor, struct spi_mem_op *op)
{                                                                               
        if (nor->spimem)                                                        
                return spi_nor_spimem_read_reg(nor, from, len, buf);           
                                                                                
        return -EOPNOTSUPP;                  
}