Linux-Devicetree Archive mirror
 help / color / mirror / Atom feed
From: Frank Li <Frank.li@nxp.com>
To: Conor Dooley <conor@kernel.org>
Cc: Shengjiu Wang <shengjiu.wang@gmail.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Shengjiu Wang <shengjiu.wang@nxp.com>,
	abelvesa@kernel.org, peng.fan@nxp.com, mturquette@baylibre.com,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com, marex@denx.de,
	linux-clk@vger.kernel.org, imx@lists.linux.dev,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, p.zabel@pengutronix.de
Subject: Re: [PATCH v3 3/6] dt-bindings: clock: imx8mp: Add reset-controller sub-node
Date: Wed, 15 May 2024 14:28:51 -0400	[thread overview]
Message-ID: <ZkT+4yUgcUdB/i2t@lizhi-Precision-Tower-5810> (raw)
In-Reply-To: <20240515-unbundle-bubble-8623b495a4f1@spud>

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


  reply	other threads:[~2024-05-15 18:29 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-14  9:33 [PATCH v3 0/6] clk: imx: clk-audiomix: Improvement for audiomix Shengjiu Wang
2024-05-14  9:33 ` [PATCH v3 1/6] dt-bindings: reset: fsl,imx8mp-audiomix-reset: add bindings Shengjiu Wang
2024-05-14  9:33 ` [PATCH v3 2/6] reset: imx8mp-audiomix: Add AudioMix Block Control reset driver Shengjiu Wang
2024-05-14 14:23   ` Frank Li
2024-05-14  9:33 ` [PATCH v3 3/6] dt-bindings: clock: imx8mp: Add reset-controller sub-node Shengjiu Wang
2024-05-14 18:06   ` Conor Dooley
2024-05-14 21:09     ` Stephen Boyd
2024-05-15  2:47       ` Shengjiu Wang
2024-05-15  3:46         ` Shengjiu Wang
2024-05-15 16:04         ` Conor Dooley
2024-05-15 18:28           ` Frank Li [this message]
2024-05-16 17:15             ` Conor Dooley
2024-05-17  3:56               ` Frank Li
2024-05-17 16:21                 ` Conor Dooley
2024-05-17 17:10                   ` Frank Li
2024-05-17 17:13                     ` Conor Dooley
2024-05-17 19:24                       ` Frank Li
2024-05-17  0:39         ` Stephen Boyd
2024-05-14  9:33 ` [PATCH v3 4/6] clk: imx: clk-audiomix: Add CLK_SET_RATE_PARENT flags for clocks Shengjiu Wang
2024-05-14  9:33 ` [PATCH v3 5/6] clk: imx: clk-audiomix: Corrent parent clock for earc_phy and audpll Shengjiu Wang
2024-05-14  9:33 ` [PATCH v3 6/6] arm64: dts: imx8mp: Add reset-controller sub node for audio_blk_ctrl Shengjiu Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZkT+4yUgcUdB/i2t@lizhi-Precision-Tower-5810 \
    --to=frank.li@nxp.com \
    --cc=abelvesa@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=conor@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=festevam@gmail.com \
    --cc=imx@lists.linux.dev \
    --cc=kernel@pengutronix.de \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marex@denx.de \
    --cc=mturquette@baylibre.com \
    --cc=p.zabel@pengutronix.de \
    --cc=peng.fan@nxp.com \
    --cc=robh@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=sboyd@kernel.org \
    --cc=shawnguo@kernel.org \
    --cc=shengjiu.wang@gmail.com \
    --cc=shengjiu.wang@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).