[PATCH v3 3/6] dt-bindings: clock: imx8mp: Add reset-controller sub-node

[Frank Li]

On Wed, May 15, 2024 at 05:04:48PM +0100, Conor Dooley wrote:
> On Wed, May 15, 2024 at 10:47:57AM +0800, Shengjiu Wang wrote:
> > On Wed, May 15, 2024 at 5:09 AM Stephen Boyd <sboyd at kernel.org> wrote:
> > >
> > > Quoting Conor Dooley (2024-05-14 11:06:14)
> > > > On Tue, May 14, 2024 at 05:33:27PM +0800, Shengjiu Wang wrote:
> > > > > diff --git a/Documentation/devicetree/bindings/clock/imx8mp-audiomix.yaml b/Documentation/devicetree/bindings/clock/imx8mp-audiomix.yaml
> > > > > index 0a6dc1a6e122..a403ace4d11f 100644
> > > > > --- a/Documentation/devicetree/bindings/clock/imx8mp-audiomix.yaml
> > > > > +++ b/Documentation/devicetree/bindings/clock/imx8mp-audiomix.yaml
> > > > > @@ -15,7 +15,10 @@ description: |
> > > > >
> > > > >  properties:
> > > > >    compatible:
> > > > > -    const: fsl,imx8mp-audio-blk-ctrl
> > > > > +    items:
> > > > > +      - const: fsl,imx8mp-audio-blk-ctrl
> > > > > +      - const: syscon
> > > > > +      - const: simple-mfd
> > > > >
> > > > >    reg:
> > > > >      maxItems: 1
> > > > > @@ -44,6 +47,11 @@ properties:
> > > > >        ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx8mp-clock.h
> > > > >        for the full list of i.MX8MP IMX8MP_CLK_AUDIOMIX_ clock IDs.
> > > > >
> > > > > +  reset-controller:
> > > > > +    type: object
> > > > > +    $ref: /schemas/reset/fsl,imx8mp-audiomix-reset.yaml#
> > > > > +    description: The child reset devices of AudioMIX Block Control.
> > > >
> > > > Why not just set #reset-cells = <1> in the existing node? IIRC it was
> > > > already suggested to you to do that and use auxdev to set up the reset
> > > > driver.
> > >
> > > Yes, do that.
> > 
> > Can I know why sub nodes can't be used? the relationship of parent and
> > child devices looks better with sub nodes.
> 
> That's pretty subjective. I don't think it looks better to have a clock
> node that is also a syscon with a reset child node as it is rather
> inconsistent.

I think it is multi function device syscon node. it should be like

mfd
{
	clock
	{
		...
	}

	reset
	{
		...
	}
}

clock and reset are difference device node with totally difference's
compatible string.

> > 
> > A further question is can I use the reset-ti-syscon? which is a generic reset
> > device for SoCs.  with it I don't even need to write a new reset device driver.
> > it is more simple.
> 
> That is for a TI SoC. You're working on an imx. I don't think that you
> should be using that...

I think this statement violate the linux basic reuse prinicple. If the
code logic are the same why need duplicate it just because it is difference
company. Of coures, if it is generic enough, it'd better to add a more
generic compatible string.

Frank