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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 15 May 2024 14:28:51 -0400
From: Frank Li <Frank.li@....com>
To: Conor Dooley <conor@...nel.org>
Cc: Shengjiu Wang <shengjiu.wang@...il.com>,
	Stephen Boyd <sboyd@...nel.org>,
	Shengjiu Wang <shengjiu.wang@....com>, abelvesa@...nel.org,
	peng.fan@....com, mturquette@...libre.com, robh@...nel.org,
	krzk+dt@...nel.org, conor+dt@...nel.org, shawnguo@...nel.org,
	s.hauer@...gutronix.de, kernel@...gutronix.de, festevam@...il.com,
	marex@...x.de, linux-clk@...r.kernel.org, imx@...ts.linux.dev,
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, p.zabel@...gutronix.de
Subject: Re: [PATCH v3 3/6] dt-bindings: clock: imx8mp: Add reset-controller
 sub-node

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@...nel.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