From: Rob Herring <robh@kernel.org>
To: Marek Vasut <marex@denx.de>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
linux-arm-kernel@lists.infradead.org,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Lucas Stach <l.stach@pengutronix.de>,
NXP Linux Team <linux-imx@nxp.com>,
Paul Elder <paul.elder@ideasonboard.com>,
Peng Fan <peng.fan@nxp.com>,
devicetree@vger.kernel.org
Subject: Re: [PATCH] dt-bindings: soc: imx8mp-media-blk-ctrl: Add LDB into DT example
Date: Thu, 30 Jun 2022 18:09:59 -0600 [thread overview]
Message-ID: <20220701000959.GA3588170-robh@kernel.org> (raw)
In-Reply-To: <247d48ae-d22f-4adc-07c0-74dbccfc9390@denx.de>
On Thu, Jun 30, 2022 at 09:50:31PM +0200, Marek Vasut wrote:
> On 6/30/22 21:31, Krzysztof Kozlowski wrote:
>
> Hi,
>
> [...]
>
> > > > > diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
> > > > > index b246d8386ba4a..05a19d3229830 100644
> > > > > --- a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
> > > > > +++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
> > > > > @@ -18,11 +18,18 @@ properties:
> > > > > compatible:
> > > > > items:
> > > > > - const: fsl,imx8mp-media-blk-ctrl
> > > > > + - const: simple-mfd
> > > >
> > > > Not really... simple-mfd means devices is really simple and you just use
> > > > it to instantiate children. However this is not simple - it's a power
> > > > domain controller with several clocks and power domains as input.
> > > >
> > > > It's not a simple MFD, but a regular device.
> > >
> > > I don't understand this comment. The LDB bridge is literally two
> > > registers with a few bits in this media block controller register area.
> > > Can you expand on why the simple-mfd is unsuitable and what should it be
> > > instead ?
> >
> > Looking at the bindings you have there 10 power domains, 10 input clocks
> > and a domain provider. The driver is also not that simple which is
> > another argument that this is not simple-mfd. Simply, it is not simple.
> >
> > What I meant, is that probably you should populate children from the
> > driver instead of adding simple-mfd compatible. Once you add simple-mfd,
> > you cannot remove it and children cannot use anything from the parent.
>
> No, I don't think so.
>
> The block controller provides those 10 power domains, those are separate
> things controlled by separate registers within the block control register
> space.
>
> This LDB bridge are two more completely unrelated registers which have
> nothing to do with those power domains . They are just in the same register
> block because they had to put those registers somewhere. And they are mixed
> literally in the middle of the register block, because there was space it
> seems. Hence the simple-mfd is I think the right thing here.
Was 'simple-mfd' missing or LDB bridge wasn't a child node? I though
this was a no functional change patch. Seems more than just update the
example.
'simple-mfd' is saying the child has 0 dependence on the parent. IMO,
'syscon' contradicts that, but that's an all to common pattern. You are
saying all the clocks (or any other resources) in the parent can be off
and the LDB bridge is still functional.
Rob
WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: Marek Vasut <marex@denx.de>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
linux-arm-kernel@lists.infradead.org,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Lucas Stach <l.stach@pengutronix.de>,
NXP Linux Team <linux-imx@nxp.com>,
Paul Elder <paul.elder@ideasonboard.com>,
Peng Fan <peng.fan@nxp.com>,
devicetree@vger.kernel.org
Subject: Re: [PATCH] dt-bindings: soc: imx8mp-media-blk-ctrl: Add LDB into DT example
Date: Thu, 30 Jun 2022 18:09:59 -0600 [thread overview]
Message-ID: <20220701000959.GA3588170-robh@kernel.org> (raw)
In-Reply-To: <247d48ae-d22f-4adc-07c0-74dbccfc9390@denx.de>
On Thu, Jun 30, 2022 at 09:50:31PM +0200, Marek Vasut wrote:
> On 6/30/22 21:31, Krzysztof Kozlowski wrote:
>
> Hi,
>
> [...]
>
> > > > > diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
> > > > > index b246d8386ba4a..05a19d3229830 100644
> > > > > --- a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
> > > > > +++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
> > > > > @@ -18,11 +18,18 @@ properties:
> > > > > compatible:
> > > > > items:
> > > > > - const: fsl,imx8mp-media-blk-ctrl
> > > > > + - const: simple-mfd
> > > >
> > > > Not really... simple-mfd means devices is really simple and you just use
> > > > it to instantiate children. However this is not simple - it's a power
> > > > domain controller with several clocks and power domains as input.
> > > >
> > > > It's not a simple MFD, but a regular device.
> > >
> > > I don't understand this comment. The LDB bridge is literally two
> > > registers with a few bits in this media block controller register area.
> > > Can you expand on why the simple-mfd is unsuitable and what should it be
> > > instead ?
> >
> > Looking at the bindings you have there 10 power domains, 10 input clocks
> > and a domain provider. The driver is also not that simple which is
> > another argument that this is not simple-mfd. Simply, it is not simple.
> >
> > What I meant, is that probably you should populate children from the
> > driver instead of adding simple-mfd compatible. Once you add simple-mfd,
> > you cannot remove it and children cannot use anything from the parent.
>
> No, I don't think so.
>
> The block controller provides those 10 power domains, those are separate
> things controlled by separate registers within the block control register
> space.
>
> This LDB bridge are two more completely unrelated registers which have
> nothing to do with those power domains . They are just in the same register
> block because they had to put those registers somewhere. And they are mixed
> literally in the middle of the register block, because there was space it
> seems. Hence the simple-mfd is I think the right thing here.
Was 'simple-mfd' missing or LDB bridge wasn't a child node? I though
this was a no functional change patch. Seems more than just update the
example.
'simple-mfd' is saying the child has 0 dependence on the parent. IMO,
'syscon' contradicts that, but that's an all to common pattern. You are
saying all the clocks (or any other resources) in the parent can be off
and the LDB bridge is still functional.
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-07-01 0:10 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-30 17:39 [PATCH] dt-bindings: soc: imx8mp-media-blk-ctrl: Add LDB into DT example Marek Vasut
2022-06-30 17:39 ` Marek Vasut
2022-06-30 19:11 ` Krzysztof Kozlowski
2022-06-30 19:11 ` Krzysztof Kozlowski
2022-06-30 19:16 ` Marek Vasut
2022-06-30 19:16 ` Marek Vasut
2022-06-30 19:31 ` Krzysztof Kozlowski
2022-06-30 19:31 ` Krzysztof Kozlowski
2022-06-30 19:50 ` Marek Vasut
2022-06-30 19:50 ` Marek Vasut
2022-07-01 0:09 ` Rob Herring [this message]
2022-07-01 0:09 ` Rob Herring
2022-07-01 8:06 ` Marek Vasut
2022-07-01 8:06 ` Marek Vasut
2022-07-04 8:00 ` Krzysztof Kozlowski
2022-07-04 8:00 ` Krzysztof Kozlowski
2022-07-08 15:32 ` Marek Vasut
2022-07-08 15:32 ` Marek Vasut
2022-07-11 9:22 ` Krzysztof Kozlowski
2022-07-11 9:22 ` Krzysztof Kozlowski
2022-06-30 23:28 ` Rob Herring
2022-06-30 23:28 ` Rob Herring
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=20220701000959.GA3588170-robh@kernel.org \
--to=robh@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=krzysztof.kozlowski@linaro.org \
--cc=l.stach@pengutronix.de \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-imx@nxp.com \
--cc=marex@denx.de \
--cc=paul.elder@ideasonboard.com \
--cc=peng.fan@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.