Re: [EXT] Re: [PATCH v3 4/4] soc: imx8m: change to use platform driver

From: Krzysztof Kozlowski
Date: Thu Nov 19 2020 - 04:42:08 EST


On Thu, Nov 19, 2020 at 07:32:17AM +0000, Alice Guo wrote:
>
>
> > -----Original Message-----
> > From: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
> > Sent: 2020年11月18日 22:11
> > To: Alice Guo <alice.guo@xxxxxxx>
> > Cc: robh+dt@xxxxxxxxxx; shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx;
> > dl-linux-imx <linux-imx@xxxxxxx>; Peng Fan <peng.fan@xxxxxxx>;
> > devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> > Subject: Re: [EXT] Re: [PATCH v3 4/4] soc: imx8m: change to use platform driver
> >
> > Caution: EXT Email
> >
> > On Wed, Nov 18, 2020 at 02:07:41PM +0000, Alice Guo wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
> > > > Sent: 2020年11月18日 18:42
> > > > To: Alice Guo <alice.guo@xxxxxxx>
> > > > Cc: robh+dt@xxxxxxxxxx; shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx;
> > > > dl-linux-imx <linux-imx@xxxxxxx>; Peng Fan <peng.fan@xxxxxxx>;
> > > > devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> > > > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> > > > Subject: Re: [EXT] Re: [PATCH v3 4/4] soc: imx8m: change to use
> > > > platform driver
> > > >
> > > > Caution: EXT Email
> > > >
> > > > On Wed, Nov 18, 2020 at 10:28:47AM +0000, Alice Guo wrote:
> > > > >
> > > > > > If it is properly explained and there is no other way then yes,
> > > > > > you could. Here, for old DTBs, I would prefer to use
> > > > > > of_platform_device_create() and bind to "soc" node (child of root).
> > > > > > This way you would always have device and exactly one entry
> > > > > > point for the probe.
> > > > > >
> > > > >
> > > > > static struct platform_driver imx8_soc_init_driver = {
> > > > > .probe = imx8_soc_init_probe,
> > > > > .driver = {
> > > > > .name = "soc@0",
> > > > > },
> > > > > };
> > > > > Can I use "soc@0" to match this driver? It will not use
> > > > > of_platform_device_create(). It will use of_find_property() to
> > > > > determine whether and nvmem-cells can be used. If there is no
> > > > > nvmem-cells,
> > > > it will use the old way, which supports old DTBS. There is no need
> > > > to add new compatible.
> > > >
> > > > No, the soc@0 is not a proper name for the driver.
> > >
> > > I have no good idea, please give suggestion. Should I still add new compatible?
> > > Should I still keep device_initcall? If use
> > > of_platform_device_create(), which node should I use?
> >
> > I mentioned my idea in the email before - of_platform_device_create() to bind
> > to the soc node. This will have to be in the initcall, you don't have a choice to
> > avoid it, since there was no compatible before.
> >
>
> node = of_find_node_by_path("/soc@0");
> if (!node)
> return -ENODEV;
>
> pdev = of_platform_device_create(node, "XXX", NULL);
> if (!pdev)
> return -ENODEV;
>
> Cannot use of_platform_device_create because "of_node_test_and_set_flag(np, OF_POPULATED)" returns true.
> of_platform_device_create is used to create platform device, but soc@0 is created by common code. I don't know how
> to bind to the soc node. The way I did in v3 seems not bad, it can work correctly and support old DTBs. Can I keep this way?

Indeed, it would require some more hacks and actually might not work at
all since bus device is already created. Keep the old way and fix other
pointed out issues.

Best regards,
Krzysztof