Re: [PATCH] dt-bindings: soc: imx8mp-media-blk-ctrl: Add LDB into DT example

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/1/22 02:09, Rob Herring wrote:
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.

In the local patch I have to imx8mp.dtsi , the simple-mfd had to be added for the ldb bridge child node to bind.

'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.

The bridge itself should work even if the block control is clocked off. The block controller apb (register) clock have to be enabled only when accessing these two LDB registers, which should be handled by runtime PM. Or what am I missing here ?



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux