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@xxxxxxxxxxxx> > > --- > > 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! > > + 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,