Re: [RFC PATCH 4/6] spi: ti-qspi: Implement the spi_mem interface

[Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org>]

[...]
>>>>>>     
>>>>>>> +static const struct spi_controller_mem_ops ti_qspi_mem_ops = {
>>>>>>> +   .exec_op = ti_qspi_exec_mem_op,      
>>>>>>
>>>>>>         .supports_op = ti_qspi_supports_mem_op,
>>>>>>
>>>>>> Its required as per spi_controller_check_ops() in Patch 1/6    
>>>>>     
>>>>> ->supports_op() is optional, and if it's missing, the core will do the    
>>>>> regular QuadSPI/DualSPI/SingleSPI check (see spi_mem_supports_op()
>>>>> implementation).     
>>>>
>>>> You might have overlooked spi_controller_check_ops() from Patch 1/6:
>>>> +static int spi_controller_check_ops(struct spi_controller *ctlr)
>>>> +{
>>>> +	/*
>>>> +	 * The controller can implement only the high-level SPI-memory
>>>> +	 * operations if it does not support regular SPI transfers.
>>>> +	 */
>>>> +	if (ctlr->mem_ops) {
>>>> +		if (!ctlr->mem_ops->supports_op ||
>>>> +		    !ctlr->mem_ops->exec_op)
>>>> +			return -EINVAL;
>>>> +	} else if (!ctlr->transfer && !ctlr->transfer_one &&
>>>> +		   !ctlr->transfer_one_message) {
>>>> +		return -EINVAL;
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>>
>>>> So if ->supports_op() is not populated by SPI controller driver, then
>>>> driver probe fails with -EINVAL. This is what I observed on my TI
>>>> hardware when testing this patch series.  
>>>
>>> Correct. Then I should fix spi_controller_check_ops() to allow empty
>>> ctlr->mem_ops->supports_op.
>>>   
>>>>  
>>>>> This being said, if you think a custom ->supports_op()
>>>>> implementation is needed for this controller I can add one.
>>>>>     
>>>>
>>>> spi_mem_supports_op() should suffice for now if above issue is fixed.  
>>>
>>> Cool. IIUC, you tested the series on a TI SoC. Does it work as
>>> expected? Do you see any perf regressions?
>>>   

No other functional failures or performance issues so far wrt TI QSPI.
Things look good :)


-- 
Regards
Vignesh
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html