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

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

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

> +		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:
> +			break;
> +		case 2:
> +			master->mode_bits |= SPI_RX_DUAL;
> +			break;
> +		case 4:
> +			master->mode_bits |= SPI_RX_QUAD;
> +			break;
> +		case 8:
> +			master->mode_bits |= SPI_RX_OCTAL;
> +			break;
> +		default:
> +			dev_warn(mdata->dev,
> +				 "spi-rx-bus-width %d not supported\n",
> +				value);
> +			break;
> +		}
> +	}
> +}
> +
>   static int mtk_spi_probe(struct platform_device *pdev)
>   {
>   	struct spi_master *master;
> @@ -830,6 +1170,13 @@ static int mtk_spi_probe(struct platform_device *pdev)
>   	if (mdata->dev_comp->ipm_design)
>   		master->mode_bits |= SPI_LOOP;
>   
> +	if (mdata->dev_comp->ipm_design) {
> +		mdata->dev = &pdev->dev;
> +		master->mem_ops = &mtk_spi_mem_ops;
> +		of_mtk_spi_parse_dt(master, pdev->dev.of_node);
> +		init_completion(&mdata->spimem_done);
> +	}
> +
>   	if (mdata->dev_comp->need_pad_sel) {
>   		mdata->pad_num = of_property_count_u32_elems(
>   			pdev->dev.of_node,

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