[PATCH resend] arm64: dts: mediatek: mt8192: Mark scp_adsp clock as broken

[PATCH resend] arm64: dts: mediatek: mt8192: Mark scp_adsp clock as broken

[Chen-Yu Tsai]

The scp_adsp clock controller is under the SCP_ADSP power domain. This
power domain is currently not supported nor defined.

Mark the clock controller as broken for now, to avoid the system from
trying to access it, and causing the CPU or bus to stall.

Fixes: 5d2b897bc6f5 ("arm64: dts: mediatek: Add mt8192 clock controllers")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 arch/arm64/boot/dts/mediatek/mt8192.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
index 6b20376191a7..ef91941848ae 100644
--- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
@@ -575,6 +575,8 @@ scp_adsp: clock-controller@10720000 {
 			compatible = "mediatek,mt8192-scp_adsp";
 			reg = <0 0x10720000 0 0x1000>;
 			#clock-cells = <1>;
+			/* power domain dependency not upstreamed */
+			status = "broken";
 		};
 
 		uart0: serial@11002000 {
-- 
2.37.3.968.ga6b4b080e4-goog

Re: [PATCH resend] arm64: dts: mediatek: mt8192: Mark scp_adsp clock as broken

[AngeloGioacchino Del Regno]

Il 30/11/22 04:17, Chen-Yu Tsai ha scritto:
> The scp_adsp clock controller is under the SCP_ADSP power domain. This
> power domain is currently not supported nor defined.
> 
> Mark the clock controller as broken for now, to avoid the system from
> trying to access it, and causing the CPU or bus to stall.
> 
> Fixes: 5d2b897bc6f5 ("arm64: dts: mediatek: Add mt8192 clock controllers")
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

....or we can add the ADSP power domain to actually fix this properly, which looks
like being a generally good idea :-)

Allen, can you please take care of that?

Thank you,
Angelo

> ---
>   arch/arm64/boot/dts/mediatek/mt8192.dtsi | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> index 6b20376191a7..ef91941848ae 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> @@ -575,6 +575,8 @@ scp_adsp: clock-controller@10720000 {
>   			compatible = "mediatek,mt8192-scp_adsp";
>   			reg = <0 0x10720000 0 0x1000>;
>   			#clock-cells = <1>;
> +			/* power domain dependency not upstreamed */
> +			status = "broken";
>   		};
>   
>   		uart0: serial@11002000 {
> 

Re: [PATCH resend] arm64: dts: mediatek: mt8192: Mark scp_adsp clock as broken

[Chen-Yu Tsai]

On Wed, Nov 30, 2022 at 7:10 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 30/11/22 04:17, Chen-Yu Tsai ha scritto:
> > The scp_adsp clock controller is under the SCP_ADSP power domain. This
> > power domain is currently not supported nor defined.
> >
> > Mark the clock controller as broken for now, to avoid the system from
> > trying to access it, and causing the CPU or bus to stall.
> >
> > Fixes: 5d2b897bc6f5 ("arm64: dts: mediatek: Add mt8192 clock controllers")
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>
> ....or we can add the ADSP power domain to actually fix this properly, which looks
> like being a generally good idea :-)

Sure, but that and any driver changes have to be backported, or anything
touching the clocks will still break the system.

There's no reason we can't have both. I think having this one merged and
backported to stable first, then adding the SCP_ADSP power domain, and tying
it to the clock controller as a follow up addition works best.

What do you think?

ChenYu

> Allen, can you please take care of that?
>
> Thank you,
> Angelo
>
> > ---
> >   arch/arm64/boot/dts/mediatek/mt8192.dtsi | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> > index 6b20376191a7..ef91941848ae 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> > @@ -575,6 +575,8 @@ scp_adsp: clock-controller@10720000 {
> >                       compatible = "mediatek,mt8192-scp_adsp";
> >                       reg = <0 0x10720000 0 0x1000>;
> >                       #clock-cells = <1>;
> > +                     /* power domain dependency not upstreamed */
> > +                     status = "broken";
> >               };
> >
> >               uart0: serial@11002000 {
> >
>

Re: [PATCH resend] arm64: dts: mediatek: mt8192: Mark scp_adsp clock as broken

[AngeloGioacchino Del Regno]

Il 01/12/22 09:56, Chen-Yu Tsai ha scritto:
> On Wed, Nov 30, 2022 at 7:10 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Il 30/11/22 04:17, Chen-Yu Tsai ha scritto:
>>> The scp_adsp clock controller is under the SCP_ADSP power domain. This
>>> power domain is currently not supported nor defined.
>>>
>>> Mark the clock controller as broken for now, to avoid the system from
>>> trying to access it, and causing the CPU or bus to stall.
>>>
>>> Fixes: 5d2b897bc6f5 ("arm64: dts: mediatek: Add mt8192 clock controllers")
>>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>>
>> ....or we can add the ADSP power domain to actually fix this properly, which looks
>> like being a generally good idea :-)
> 
> Sure, but that and any driver changes have to be backported, or anything
> touching the clocks will still break the system.
> 

I agree.

> There's no reason we can't have both. I think having this one merged and
> backported to stable first, then adding the SCP_ADSP power domain, and tying
> it to the clock controller as a follow up addition works best.
> 
> What do you think?
> 

I think that one reason to not have both is that we'd have to revert this commit
after the SCP_ADSP power domain is added (with the appropriate Fixes tags and/or
Cc stable)...

I'd expect that entire addition to be no more than 3 commits, including the dtsi
one... and if it comes out as I expect, we'd be solving that issue on all of the
affected older versions of the kernel - the right way.

Can we wait for... let's say, a day or two to check how that works, before taking
a final decision on this commit?

Cheers,
Angelo

> ChenYu
> 
>> Allen, can you please take care of that?
>>
>> Thank you,
>> Angelo
>>
>>> ---
>>>    arch/arm64/boot/dts/mediatek/mt8192.dtsi | 2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
>>> index 6b20376191a7..ef91941848ae 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
>>> @@ -575,6 +575,8 @@ scp_adsp: clock-controller@10720000 {
>>>                        compatible = "mediatek,mt8192-scp_adsp";
>>>                        reg = <0 0x10720000 0 0x1000>;
>>>                        #clock-cells = <1>;
>>> +                     /* power domain dependency not upstreamed */
>>> +                     status = "broken";
>>>                };
>>>
>>>                uart0: serial@11002000 {
>>>
>>

Re: [PATCH resend] arm64: dts: mediatek: mt8192: Mark scp_adsp clock as broken

[Matthias Brugger]

On 01/12/2022 10:02, AngeloGioacchino Del Regno wrote:
> Il 01/12/22 09:56, Chen-Yu Tsai ha scritto:
>> On Wed, Nov 30, 2022 at 7:10 PM AngeloGioacchino Del Regno
>> <angelogioacchino.delregno@collabora.com> wrote:
>>>
>>> Il 30/11/22 04:17, Chen-Yu Tsai ha scritto:
>>>> The scp_adsp clock controller is under the SCP_ADSP power domain. This
>>>> power domain is currently not supported nor defined.
>>>>
>>>> Mark the clock controller as broken for now, to avoid the system from
>>>> trying to access it, and causing the CPU or bus to stall.
>>>>
>>>> Fixes: 5d2b897bc6f5 ("arm64: dts: mediatek: Add mt8192 clock controllers")
>>>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>>>
>>> ....or we can add the ADSP power domain to actually fix this properly, which 
>>> looks
>>> like being a generally good idea :-)
>>
>> Sure, but that and any driver changes have to be backported, or anything
>> touching the clocks will still break the system.
>>
> 
> I agree.
> 
>> There's no reason we can't have both. I think having this one merged and
>> backported to stable first, then adding the SCP_ADSP power domain, and tying
>> it to the clock controller as a follow up addition works best.
>>
>> What do you think?
>>
> 
> I think that one reason to not have both is that we'd have to revert this commit
> after the SCP_ADSP power domain is added (with the appropriate Fixes tags and/or
> Cc stable)...
> 
> I'd expect that entire addition to be no more than 3 commits, including the dtsi
> one... and if it comes out as I expect, we'd be solving that issue on all of the
> affected older versions of the kernel - the right way.
> 
> Can we wait for... let's say, a day or two to check how that works, before taking
> a final decision on this commit?
> 

Do I understand correctly that the correct way for now is to merge this patch 
until we have a fixed the power domain controller?

Regards,
Matthias

Re: [PATCH resend] arm64: dts: mediatek: mt8192: Mark scp_adsp clock as broken

[AngeloGioacchino Del Regno]

Il 16/12/22 14:17, Matthias Brugger ha scritto:
> 
> 
> On 01/12/2022 10:02, AngeloGioacchino Del Regno wrote:
>> Il 01/12/22 09:56, Chen-Yu Tsai ha scritto:
>>> On Wed, Nov 30, 2022 at 7:10 PM AngeloGioacchino Del Regno
>>> <angelogioacchino.delregno@collabora.com> wrote:
>>>>
>>>> Il 30/11/22 04:17, Chen-Yu Tsai ha scritto:
>>>>> The scp_adsp clock controller is under the SCP_ADSP power domain. This
>>>>> power domain is currently not supported nor defined.
>>>>>
>>>>> Mark the clock controller as broken for now, to avoid the system from
>>>>> trying to access it, and causing the CPU or bus to stall.
>>>>>
>>>>> Fixes: 5d2b897bc6f5 ("arm64: dts: mediatek: Add mt8192 clock controllers")
>>>>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>>>>
>>>> ....or we can add the ADSP power domain to actually fix this properly, which looks
>>>> like being a generally good idea :-)
>>>
>>> Sure, but that and any driver changes have to be backported, or anything
>>> touching the clocks will still break the system.
>>>
>>
>> I agree.
>>
>>> There's no reason we can't have both. I think having this one merged and
>>> backported to stable first, then adding the SCP_ADSP power domain, and tying
>>> it to the clock controller as a follow up addition works best.
>>>
>>> What do you think?
>>>
>>
>> I think that one reason to not have both is that we'd have to revert this commit
>> after the SCP_ADSP power domain is added (with the appropriate Fixes tags and/or
>> Cc stable)...
>>
>> I'd expect that entire addition to be no more than 3 commits, including the dtsi
>> one... and if it comes out as I expect, we'd be solving that issue on all of the
>> affected older versions of the kernel - the right way.
>>
>> Can we wait for... let's say, a day or two to check how that works, before taking
>> a final decision on this commit?
>>
> 
> Do I understand correctly that the correct way for now is to merge this patch until 
> we have a fixed the power domain controller?
> 
> Regards,
> Matthias

I thought that the proper fix would be going in v6.2, but apparently something
went wrong with it as it contains things that aren't upstream.

At this point, let's go with this one until the proper fix gets factored in,
which I expect to be ready for v6.3.

Cheers,
Angelo

Re: [PATCH resend] arm64: dts: mediatek: mt8192: Mark scp_adsp clock as broken

[Matthias Brugger]

On 30/11/2022 04:17, Chen-Yu Tsai wrote:
> The scp_adsp clock controller is under the SCP_ADSP power domain. This
> power domain is currently not supported nor defined.
> 
> Mark the clock controller as broken for now, to avoid the system from
> trying to access it, and causing the CPU or bus to stall.
> 
> Fixes: 5d2b897bc6f5 ("arm64: dts: mediatek: Add mt8192 clock controllers")
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
>   arch/arm64/boot/dts/mediatek/mt8192.dtsi | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> index 6b20376191a7..ef91941848ae 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> @@ -575,6 +575,8 @@ scp_adsp: clock-controller@10720000 {
>   			compatible = "mediatek,mt8192-scp_adsp";
>   			reg = <0 0x10720000 0 0x1000>;
>   			#clock-cells = <1>;
> +			/* power domain dependency not upstreamed */
> +			status = "broken";
>   		};
>   
>   		uart0: serial@11002000 {

Looking into the DT spec, "broken" is no valid value. I suppose we want to have 
"fail" here.

Regards,
Matthias