Hi Oleksij, > -----Original Message----- > From: Oleksij Rempel [mailto:o.rempel@xxxxxxxxxxxxxx] > Sent: Saturday, July 21, 2018 7:40 PM > To: Vladimir Zapolskiy <vladimir_zapolskiy@xxxxxxxxxx> > Cc: Shawn Guo <shawnguo@xxxxxxxxxx>; Fabio Estevam > <fabio.estevam@xxxxxxx>; Rob Herring <robh+dt@xxxxxxxxxx>; Mark > Rutland <mark.rutland@xxxxxxx>; A.s. Dong <aisheng.dong@xxxxxxx>; > kernel@xxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx> > Subject: Re: [PATCH v4 4/4] mailbox: Add support for i.MX7D messaging unit > [...] > > > + > > > +static int imx_mu_remove(struct platform_device *pdev) { > > > + struct imx_mu_priv *priv = platform_get_drvdata(pdev); > > > + > > > + mbox_controller_unregister(&priv->mbox); > > > + clk_disable_unprepare(priv->clk); > > > + > > > + return 0; > > > +} > > > + > > > +static void imx_mu_init_generic(struct imx_mu_priv *priv) { > > > + if (priv->side_b) > > > + return; > > > + > > > + /* Set default MU configuration */ > > > + imx_mu_write(priv, 0, IMX_MU_xCR); } > > > + > > > +static const struct imx_mu_cfg imx_mu_cfg_generic = { > > > + .chans = IMX_MU_MAX_CHANS, > > > > Here IMX_MU_MAX_CHANS macro name is questionable, see my top > comment. > > For clarity here 'MAX' is not expected, it shall be the exact > > controller specific value, something like s/MAX/NUM/ may be considered. > > this MU provide 4+4 interrupts for TX/RX registers and 4 general purpose > interrupts which can be reused for mailbox with shared memory or as irq > controller. > The NXP implementation of SCU on i.MX8 is using one MU as one channel. > > I'm not sure how many channels should be defined as it is endproduct > specific. So far MAX == 4 until other already existing implementations go > upstream. > 4 is ok to me currently. > > > + .init_hw = imx_mu_init_generic, > > > +}; > > > + > > > +static const struct of_device_id imx_mu_dt_ids[] = { > > > + { .compatible = "fsl,imx6sx-mu", .data = &imx_mu_cfg_generic }, > > > + { .compatible = "fsl,imx7d-mu", .data = &imx_mu_cfg_generic }, > > > + { .compatible = "fsl,imx7s-mu", .data = &imx_mu_cfg_generic }, > > > + { .compatible = "fsl,imx7ulp-mu", .data = &imx_mu_cfg_generic }, > > > + { .compatible = "fsl,imx8qm-mu", .data = &imx_mu_cfg_generic }, > > > + { .compatible = "fsl,imx8qxp-mu", .data = &imx_mu_cfg_generic }, > > > > It sounds like the list will be constantly extending. Is there any > > chance to introduce just one generic compatible in the driver, and > > describe the whole set of compatibles in documentation section only? > > Experience with iMX* IPs showed - it is worth do describe each IP in each SoC > with SoC specific name. See SPI, UART and other iMX driver. Why we should > make here an exception? > Even SPI, UART driver you mentioned here do not describe all the SoC compatible strings. New ones are usually added when we need different SoC specific .data. But here all SoC specific .data actually is the same one which seems like an unnecessary thing in driver. Regards Dong Aisheng > > > + { }, > > > +}; > > > +MODULE_DEVICE_TABLE(of, imx_mu_dt_ids); > > > + > > > +static struct platform_driver imx_mu_driver = { > > > + .probe = imx_mu_probe, > > > + .remove = imx_mu_remove, > > > + .driver = { > > > + .name = "imx_mu", > > > + .of_match_table = imx_mu_dt_ids, > > > + }, > > > +}; > > > +module_platform_driver(imx_mu_driver); > > > + > > > +MODULE_AUTHOR("Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>"); > > > +MODULE_DESCRIPTION("Message Unit driver for i.MX"); > > > +MODULE_LICENSE("GPL v2"); > > > > > > > -- > > Best wishes, > > Vladimir > > > > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
