mbox series

[0/5] convert ifc binding to yaml and drop "simple-bus"

Message ID 20211001000924.15421-1-leoyang.li@nxp.com
Headers show
Series convert ifc binding to yaml and drop "simple-bus" | expand

Message

Leo Li Oct. 1, 2021, 12:09 a.m. UTC
Convert the ifc binding to yaml schema, in the mean while remove the
"simple-bus" compatible from the binding to make sure ifc device probes
before any of the child devices.  Update the driver and existing DTSes
accordingly.

DTS changes should be merged together with the driver/binding changes
if DTS maintainer is ok with it or after the driver changes are applied.

Li Yang (5):
  dt-bindings: memory: fsl: convert ifc binding to yaml schema
  memory: fsl_ifc: populate child devices without relying on simple-bus
  ARM: dts: ls1021a: remove "simple-bus" compatible from ifc node
  arm64: dts: remove "simple-bus" compatible from ifc node
  powerpc/mpc85xx: remove "simple-bus" compatible from ifc node

 .../bindings/memory-controllers/fsl/ifc.txt   |  82 -----------
 .../bindings/memory-controllers/fsl/ifc.yaml  | 137 ++++++++++++++++++
 arch/arm/boot/dts/ls1021a.dtsi                |   2 +-
 .../arm64/boot/dts/freescale/fsl-ls1043a.dtsi |   2 +-
 .../arm64/boot/dts/freescale/fsl-ls1046a.dtsi |   2 +-
 .../arm64/boot/dts/freescale/fsl-ls1088a.dtsi |   2 +-
 .../arm64/boot/dts/freescale/fsl-ls208xa.dtsi |   2 +-
 arch/powerpc/boot/dts/fsl/b4si-post.dtsi      |   2 +-
 arch/powerpc/boot/dts/fsl/bsc9131si-post.dtsi |   2 +-
 arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi |   2 +-
 arch/powerpc/boot/dts/fsl/c293si-post.dtsi    |   2 +-
 arch/powerpc/boot/dts/fsl/p1010si-post.dtsi   |   2 +-
 arch/powerpc/boot/dts/fsl/t1023si-post.dtsi   |   2 +-
 arch/powerpc/boot/dts/fsl/t1040si-post.dtsi   |   2 +-
 arch/powerpc/boot/dts/fsl/t2081si-post.dtsi   |   2 +-
 arch/powerpc/boot/dts/fsl/t4240si-post.dtsi   |   2 +-
 drivers/memory/fsl_ifc.c                      |   9 ++
 17 files changed, 160 insertions(+), 96 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/fsl/ifc.yaml

Comments

Krzysztof Kozlowski Oct. 1, 2021, 9:45 a.m. UTC | #1
On 01/10/2021 02:09, Li Yang wrote:
> Convert the ifc binding to yaml schema, in the mean while remove the
> "simple-bus" compatible from the binding to make sure ifc device probes
> before any of the child devices.  Update the driver and existing DTSes
> accordingly.
> 
> DTS changes should be merged together with the driver/binding changes
> if DTS maintainer is ok with it or after the driver changes are applied.
> 

It's discouraged to merge DTS along with drivers (e.g. soc folks don't
accept such pull requests), so I propose to apply it in the next cycle.

Best regards,
Krzysztof
Leo Li Oct. 1, 2021, 7:51 p.m. UTC | #2
On Fri, Oct 1, 2021 at 4:46 AM Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
>
> On 01/10/2021 02:09, Li Yang wrote:
> > Convert the ifc binding to yaml schema, in the mean while remove the
> > "simple-bus" compatible from the binding to make sure ifc device probes
> > before any of the child devices.  Update the driver and existing DTSes
> > accordingly.
> >
> > DTS changes should be merged together with the driver/binding changes
> > if DTS maintainer is ok with it or after the driver changes are applied.
> >
>
> It's discouraged to merge DTS along with drivers (e.g. soc folks don't
> accept such pull requests), so I propose to apply it in the next cycle.

Ok.  Will separate the DTS changes in the next version.

>
> Best regards,
> Krzysztof
Rob Herring Oct. 4, 2021, 4:27 p.m. UTC | #3
On Thu, Sep 30, 2021 at 07:09:21PM -0500, Li Yang wrote:
> After we update the binding to not use simple-bus compatible for the
> controller, we need the driver to populate the child devices explicitly.
> 
> Signed-off-by: Li Yang <leoyang.li@nxp.com>
> ---
>  drivers/memory/fsl_ifc.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/memory/fsl_ifc.c b/drivers/memory/fsl_ifc.c
> index d062c2f8250f..251d713cd50b 100644
> --- a/drivers/memory/fsl_ifc.c
> +++ b/drivers/memory/fsl_ifc.c
> @@ -88,6 +88,7 @@ static int fsl_ifc_ctrl_remove(struct platform_device *dev)
>  {
>  	struct fsl_ifc_ctrl *ctrl = dev_get_drvdata(&dev->dev);
>  
> +	of_platform_depopulate(&dev->dev);
>  	free_irq(ctrl->nand_irq, ctrl);
>  	free_irq(ctrl->irq, ctrl);
>  
> @@ -285,6 +286,14 @@ static int fsl_ifc_ctrl_probe(struct platform_device *dev)
>  		}
>  	}
>  
> +	/* legacy dts may still use "simple-bus" compatible */
> +	if (!of_device_is_compatible(dev->dev.of_node, "simple-bus")) {
> +	        ret = of_platform_populate(dev->dev.of_node, NULL, NULL,
> +						&dev->dev);

There's no need to make this conditional. of_platform_populate() is safe 
to call multiple times. If that doesn't work, it's a bug.

Rob
Leo Li Oct. 9, 2021, 3:12 a.m. UTC | #4
On Mon, Oct 4, 2021 at 12:14 PM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Sep 30, 2021 at 07:09:21PM -0500, Li Yang wrote:
> > After we update the binding to not use simple-bus compatible for the
> > controller, we need the driver to populate the child devices explicitly.
> >
> > Signed-off-by: Li Yang <leoyang.li@nxp.com>
> > ---
> >  drivers/memory/fsl_ifc.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/memory/fsl_ifc.c b/drivers/memory/fsl_ifc.c
> > index d062c2f8250f..251d713cd50b 100644
> > --- a/drivers/memory/fsl_ifc.c
> > +++ b/drivers/memory/fsl_ifc.c
> > @@ -88,6 +88,7 @@ static int fsl_ifc_ctrl_remove(struct platform_device *dev)
> >  {
> >       struct fsl_ifc_ctrl *ctrl = dev_get_drvdata(&dev->dev);
> >
> > +     of_platform_depopulate(&dev->dev);
> >       free_irq(ctrl->nand_irq, ctrl);
> >       free_irq(ctrl->irq, ctrl);
> >
> > @@ -285,6 +286,14 @@ static int fsl_ifc_ctrl_probe(struct platform_device *dev)
> >               }
> >       }
> >
> > +     /* legacy dts may still use "simple-bus" compatible */
> > +     if (!of_device_is_compatible(dev->dev.of_node, "simple-bus")) {
> > +             ret = of_platform_populate(dev->dev.of_node, NULL, NULL,
> > +                                             &dev->dev);
>
> There's no need to make this conditional. of_platform_populate() is safe
> to call multiple times. If that doesn't work, it's a bug.

I think that it is probably an optimization to avoid re-populate the
bus for legacy device trees.  But it might be cleaner to just
re-populate anyway?

Regards,
Leo