Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller
On 06/07/2022 14:00, Tinghan Shen wrote:
> Hi Krzysztof,
>
> After discussing your message with our power team,
> we realized that we need your help to ensure we fully understand you.
>
> On Mon, 2022-07-04 at 14:38 +0200, Krzysztof Kozlowski wrote:
>> On 04/07/2022 12:00, Tinghan Shen wrote:
>>> Add power domains controller node for mt8195.
>>>
>>> Signed-off-by: Weiyi Lu
>>> Signed-off-by: Tinghan Shen
>>> ---
>>> arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++
>>> 1 file changed, 327 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> index 8d59a7da3271..d52e140d9271 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> @@ -10,6 +10,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>>
>>> / {
>>> compatible = "mediatek,mt8195";
>>> @@ -338,6 +339,332 @@
>>> #interrupt-cells = <2>;
>>> };
>>>
>>> + scpsys: syscon@10006000 {
>>> + compatible = "syscon", "simple-mfd";
>>
>> These compatibles cannot be alone.
>
> the scpsys sub node has the compatible of the power domain driver.
> do you suggest that the compatible in the sub node should move to here?
Not necessarily, depends. You have here device node representing system
registers. They need they own compatibles, just like everywhere in the
kernel (except the broken cases...).
Whether this should be compatible of power-domain driver, it depends
what this device node is. I don't know, I don't have your datasheets or
your architecture diagrams...
>
>>> + reg = <0 0x10006000 0 0x1000>;
>>> + #power-domain-cells = <1>;
>>
>> If it is simple MFD, then probably it is not a power domain provider.
>> Decide.
>
> this MFD device is the power controller on mt8195.
Then it is not a simple MFD but a power controller. Do not use
"simple-mfd" compatible.
> Some features need
> to do some operations on registers in this node. We think that implement
> the operation of these registers as the MFD device can provide flexibility
> for future use. We want to clarify if you're saying that an MFD device
> cannot be a power domain provider.
MFD device is Linuxism, so it has nothing to do here. I am talking only
about simple-mfd. simple-mfd is a simple device only instantiating
children and not providing anything to anyone. Neither to children. This
the most important part. The children do not depend on anything from
simple-mfd device. For example simple-mfd device can be shut down
(gated) and children should still operate. Being a power domain
controller, contradicts this usually.
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller
On 06/07/2022 15:41, Matthias Brugger wrote:
>
>
> On 04/07/2022 14:38, Krzysztof Kozlowski wrote:
>> On 04/07/2022 12:00, Tinghan Shen wrote:
>>> Add power domains controller node for mt8195.
>>>
>>> Signed-off-by: Weiyi Lu
>>> Signed-off-by: Tinghan Shen
>>> ---
>>> arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++
>>> 1 file changed, 327 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> index 8d59a7da3271..d52e140d9271 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> @@ -10,6 +10,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>>
>>> / {
>>> compatible = "mediatek,mt8195";
>>> @@ -338,6 +339,332 @@
>>> #interrupt-cells = <2>;
>>> };
>>>
>>> + scpsys: syscon@10006000 {
>>> + compatible = "syscon", "simple-mfd";
>>
>> These compatibles cannot be alone.
>>
>
> You mean we would need something like "mediatek,scpsys" as dummy compatible
> that's not bound to any driver?
Yes. syscon (and simple-mfd) must always come with a specific compatible.
>
>>> + reg = <0 0x10006000 0 0x1000>;
>>> + #power-domain-cells = <1>;
>>
>> If it is simple MFD, then probably it is not a power domain provider.
>> Decide.
>
> The SCPSYS IP block of MediaTek SoCs group several functionality, one is the
> power domain controller. Others are not yet implemented, but defining the
> scpsys
> as a MFD will give us the possibility to do so in the future.
No, quite the opposite. Having simple-mfd prevents you from implementing
it correctly later as a driver, because you cannot remove it. It would
be ABI break.
It's fine to have one block being a simple MFD having several children,
but then it's not a power controller. Children could be such power
controller, but not simple-mfd. Rob explained this several times:
https://lore.kernel.org/all/yxhine00hg6hb...@robh.at.kernel.org/
https://lore.kernel.org/all/20220701000959.ga3588170-r...@kernel.org/
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller
On 04/07/2022 14:38, Krzysztof Kozlowski wrote:
On 04/07/2022 12:00, Tinghan Shen wrote:
Add power domains controller node for mt8195.
Signed-off-by: Weiyi Lu
Signed-off-by: Tinghan Shen
---
arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++
1 file changed, 327 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 8d59a7da3271..d52e140d9271 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
/ {
compatible = "mediatek,mt8195";
@@ -338,6 +339,332 @@
#interrupt-cells = <2>;
};
+ scpsys: syscon@10006000 {
+ compatible = "syscon", "simple-mfd";
These compatibles cannot be alone.
You mean we would need something like "mediatek,scpsys" as dummy compatible
that's not bound to any driver?
+ reg = <0 0x10006000 0 0x1000>;
+ #power-domain-cells = <1>;
If it is simple MFD, then probably it is not a power domain provider.
Decide.
The SCPSYS IP block of MediaTek SoCs group several functionality, one is the
power domain controller. Others are not yet implemented, but defining the scpsys
as a MFD will give us the possibility to do so in the future.
Regards,
Matthias
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller
Hi Krzysztof,
After discussing your message with our power team,
we realized that we need your help to ensure we fully understand you.
On Mon, 2022-07-04 at 14:38 +0200, Krzysztof Kozlowski wrote:
> On 04/07/2022 12:00, Tinghan Shen wrote:
> > Add power domains controller node for mt8195.
> >
> > Signed-off-by: Weiyi Lu
> > Signed-off-by: Tinghan Shen
> > ---
> > arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++
> > 1 file changed, 327 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > index 8d59a7da3271..d52e140d9271 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > @@ -10,6 +10,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > / {
> > compatible = "mediatek,mt8195";
> > @@ -338,6 +339,332 @@
> > #interrupt-cells = <2>;
> > };
> >
> > + scpsys: syscon@10006000 {
> > + compatible = "syscon", "simple-mfd";
>
> These compatibles cannot be alone.
the scpsys sub node has the compatible of the power domain driver.
do you suggest that the compatible in the sub node should move to here?
> > + reg = <0 0x10006000 0 0x1000>;
> > + #power-domain-cells = <1>;
>
> If it is simple MFD, then probably it is not a power domain provider.
> Decide.
this MFD device is the power controller on mt8195. Some features need
to do some operations on registers in this node. We think that implement
the operation of these registers as the MFD device can provide flexibility
for future use. We want to clarify if you're saying that an MFD device
cannot be a power domain provider.
Best regards,
TingHan
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller
On 04/07/2022 12:00, Tinghan Shen wrote:
> Add power domains controller node for mt8195.
>
> Signed-off-by: Weiyi Lu
> Signed-off-by: Tinghan Shen
> ---
> arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++
> 1 file changed, 327 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index 8d59a7da3271..d52e140d9271 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -10,6 +10,7 @@
> #include
> #include
> #include
> +#include
>
> / {
> compatible = "mediatek,mt8195";
> @@ -338,6 +339,332 @@
> #interrupt-cells = <2>;
> };
>
> + scpsys: syscon@10006000 {
> + compatible = "syscon", "simple-mfd";
These compatibles cannot be alone.
> + reg = <0 0x10006000 0 0x1000>;
> + #power-domain-cells = <1>;
If it is simple MFD, then probably it is not a power domain provider.
Decide.
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller
Il 04/07/22 12:00, Tinghan Shen ha scritto: Add power domains controller node for mt8195. Signed-off-by: Weiyi Lu Signed-off-by: Tinghan Shen Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
