Re: [PATCH V4 4/6] spi: mediatek: add spi memory support for ipm design

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 17/03/22 10:27, Leilk Liu ha scritto:
> On Tue, 2022-03-15 at 10:31 +0100, AngeloGioacchino Del Regno wrote:
>> Il 15/03/22 04:24, Leilk Liu ha scritto:
>>> this patch add the support of spi-mem for ipm design.
>>>
>>> Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
>>> ---
>>>    drivers/spi/spi-mt65xx.c | 349
>>> ++++++++++++++++++++++++++++++++++++++-
>>>    1 file changed, 348 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
>>> index 1a0b3208dfca..8958c3fa4fea 100644
>>> --- a/drivers/spi/spi-mt65xx.c
>>> +++ b/drivers/spi/spi-mt65xx.c
>>
>> ...snip...
>>
>>> +
>>> +static void of_mtk_spi_parse_dt(struct spi_master *master, struct
>>> device_node *nc)
>>> +{
>>> +	struct mtk_spi *mdata = spi_master_get_devdata(master);
>>> +	u32 value;
>>> +
>>> +	if (!of_property_read_u32(nc, "spi-tx-bus-width", &value)) {
>>
>> Hello Leilk,
>>
>> thanks for considering my advice about "spi-{tx,rx}-bus-width", but
>> there's
>> something that you have misunderstood about it.
>>
>> Simply, you don't need this function at all. Whatever you are doing
>> here is
>> already being performed in the Linux SPI framework: at the end of the
>> probe
>> function, this driver is calling the (legacy)
>> devm_spi_register_master(),
>> which calls devm_spi_register_controller().
>>
>> In drivers/spi/spi.c, function spi_register_controller(), will in
>> turn call
>> of_register_spi_devices(ctlr) -> of_register_spi_device(ctlr, nc)...
>> that
>> will end up finally calling function of_spi_parse_dt(ctlr, spi, nc).
>>
>> The last mentioned function already contains the logic and setup to
>> check
>> devicetree properties "spi-tx-bus-width" and "spi-rx-bus-width" (and
>> some
>> others, as well).
>>
>> This means that spi-mt65xx.c already probed these even before your
>> IPM
>> implementation, hence ***function of_mtk_spi_parse_dt() is not
>> needed***.
>>
>> Simply drop it and don't check for these properties: that's already
>> done.
>>
>>
>> Regards,
>> Angelo
>>
> Hi Angelo,
> 
> Thanks for your advice.
> 
> There are two spi controllor on MT7986. One supports single/dual mode,
> the other supports quad mode. Both of them can support spi memory
> framework(one's tx/rx bus width is 1/2, the other one's tx/rx bus width
> is 1/2/4).
> 
> Can I use of_mtk_spi_parse_dt() to parse the information? What's your
> suggestion?
> 
> Thanks!
> 

As I've already said, this does NOT require any devicetree handling in
spi-mt65xx.c, as setting the right mode_bits is already handled in
drivers/spi/spi.c - please follow the explaination that I have given
before to fully understand the situation.

Regards,
Angelo


> 
>>> +		switch (value) {
>>> +		case 1:
>>> +			break;
>>> +		case 2:
>>> +			master->mode_bits |= SPI_TX_DUAL;
>>> +			break;
>>> +		case 4:
>>> +			master->mode_bits |= SPI_TX_QUAD;
>>> +			break;
>>> +		default:
>>> +			dev_warn(mdata->dev,
>>> +				 "spi-tx-bus-width %d not supported\n",
>>> +				value);
>>> +			break;
>>> +		}
>>> +	}
>>> +
>>> +	if (!of_property_read_u32(nc, "spi-rx-bus-width", &value)) {
>>> +		switch (value) {
>>> +		case 1:

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek