| Message ID | 8734eib0vx.wl-kuninori.morimoto.gx@renesas.com |
|---|---|
| State | New |
| Headers | show |
| Series | ASoC: add Renesas MSIOF sound driver | expand |
Hi Morimoto-san, On Wed, 9 Apr 2025 at 03:05, Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> wrote: > Renesas MSIOF (Clock-Synchronized Serial Interface with FIFO) can work as > both SPI and I2S. MSIOF-I2S will use Audio Graph Card/Card2 driver which > Of-Graph in DT. > > MSIOF-SPI/I2S are using same DT compatible properties. > MSIOF-I2S uses Of-Graph for Audio-Graph-Card/Card2, > MSIOF-SPI doesn't use Of-Graph. > > Check "port" node when driver probing > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Still, some comment below > --- a/drivers/spi/spi-sh-msiof.c > +++ b/drivers/spi/spi-sh-msiof.c > @@ -20,6 +20,7 @@ > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/of.h> > +#include <linux/of_graph.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > #include <linux/sh_dma.h> > @@ -1276,10 +1277,19 @@ static int sh_msiof_spi_probe(struct platform_device *pdev) > const struct sh_msiof_chipdata *chipdata; > struct sh_msiof_spi_info *info; > struct sh_msiof_spi_priv *p; > + struct device_node *port; If you would add "__free(device_node)", you could drop the of_node_put() below. > unsigned long clksrc; > int i; > int ret; > > + /* Check whether MSIOF is used as I2S mode or SPI mode by checking "port" node */ > + port = of_graph_get_next_port(pdev->dev.of_node, NULL); This is actually checking for both "ports" and "port". If you know the subnode is called "port", you could simplify to of_get_child_by_name(). > + if (port) { > + /* It was MSIOF-I2S */ > + of_node_put(port); > + return -ENODEV; > + } > + > chipdata = of_device_get_match_data(&pdev->dev); > if (chipdata) { > info = sh_msiof_spi_parse_dt(&pdev->dev); Gr{oetje,eeting}s, Geert
Hi Geert Thank you for your review > > Renesas MSIOF (Clock-Synchronized Serial Interface with FIFO) can work as > > both SPI and I2S. MSIOF-I2S will use Audio Graph Card/Card2 driver which > > Of-Graph in DT. > > > > MSIOF-SPI/I2S are using same DT compatible properties. > > MSIOF-I2S uses Of-Graph for Audio-Graph-Card/Card2, > > MSIOF-SPI doesn't use Of-Graph. > > > > Check "port" node when driver probing > > > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Thanks > > @@ -1276,10 +1277,19 @@ static int sh_msiof_spi_probe(struct platform_device *pdev) > > const struct sh_msiof_chipdata *chipdata; > > struct sh_msiof_spi_info *info; > > struct sh_msiof_spi_priv *p; > > + struct device_node *port; > > If you would add "__free(device_node)", you could drop the of_node_put() > below. Yes, indeed. will use it in v2 > > + /* Check whether MSIOF is used as I2S mode or SPI mode by checking "port" node */ > > + port = of_graph_get_next_port(pdev->dev.of_node, NULL); > > This is actually checking for both "ports" and "port". If you know the > subnode is called "port", you could simplify to of_get_child_by_name(). Current dt-bindings Doc is caring only "port" for now (because it will be more complicated if care both...), but sound might/can use "ports". Thank you for your help !! Best regards --- Kuninori Morimoto
diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c index 8a98c313548e..51415131eff1 100644 --- a/drivers/spi/spi-sh-msiof.c +++ b/drivers/spi/spi-sh-msiof.c @@ -20,6 +20,7 @@ #include <linux/kernel.h> #include <linux/module.h> #include <linux/of.h> +#include <linux/of_graph.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> #include <linux/sh_dma.h> @@ -1276,10 +1277,19 @@ static int sh_msiof_spi_probe(struct platform_device *pdev) const struct sh_msiof_chipdata *chipdata; struct sh_msiof_spi_info *info; struct sh_msiof_spi_priv *p; + struct device_node *port; unsigned long clksrc; int i; int ret; + /* Check whether MSIOF is used as I2S mode or SPI mode by checking "port" node */ + port = of_graph_get_next_port(pdev->dev.of_node, NULL); + if (port) { + /* It was MSIOF-I2S */ + of_node_put(port); + return -ENODEV; + } + chipdata = of_device_get_match_data(&pdev->dev); if (chipdata) { info = sh_msiof_spi_parse_dt(&pdev->dev);
Renesas MSIOF (Clock-Synchronized Serial Interface with FIFO) can work as both SPI and I2S. MSIOF-I2S will use Audio Graph Card/Card2 driver which Of-Graph in DT. MSIOF-SPI/I2S are using same DT compatible properties. MSIOF-I2S uses Of-Graph for Audio-Graph-Card/Card2, MSIOF-SPI doesn't use Of-Graph. Check "port" node when driver probing Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> --- drivers/spi/spi-sh-msiof.c | 10 ++++++++++ 1 file changed, 10 insertions(+)