lkml.org 
[lkml]   [2022]   [Dec]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] arm64: dts: mt8192: Add adsp power domain controller
From
Il 01/12/22 11:05, Chen-Yu Tsai ha scritto:
> On Thu, Dec 1, 2022 at 5:39 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Il 01/12/22 10:10, Chen-Yu Tsai ha scritto:
>>> On Thu, Dec 1, 2022 at 5:07 PM AngeloGioacchino Del Regno
>>> <angelogioacchino.delregno@collabora.com> wrote:
>>>>
>>>> Il 01/12/22 08:33, Allen-KH Cheng ha scritto:
>>>>> Add adsp power domain controller node for mt8192 SoC.
>>>>>
>>>>> Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>
>>>>> ---
>>>>> Ref: https://lore.kernel.org/all/2ec80bd8-dfef-d2e6-eb41-6e6088043e33@collabora.com/
>>>>> [Allen-KH Cheng <allen-kh.cheng@mediatek.com>]
>>>>> ---
>>>>> ---
>>>>> arch/arm64/boot/dts/mediatek/mt8192.dtsi | 8 ++++++++
>>>>> include/dt-bindings/power/mt8192-power.h | 1 +
>>>>> 2 files changed, 9 insertions(+)
>>>>>
>>>>
>>>> Allen, thanks for this one, but it's incomplete...
>>>>
>>>> First of all, you must add the power domain on the driver itself, specifically,
>>>> in drivers/soc/mediatek/mt8192-pm-domains.h - otherwise this change will have no
>>>> effect!
>>>
>>> Actually it's worse. The driver doesn't know about the new power domain,
>>> and so it will fail to probe. We might need to make the power domain driver
>>> fail gracefully and skip unknown power domains.
>>>
>>
>> Right. It's worse. I don't know, though, if gracefully skipping unknown power
>> domains in the driver would be a good decision... as sometimes error messages
>> go unnoticed.
>>
>> When the platform "explodes" instead, you're forced to read that log carefully
>> and get it working again... Anyway, I'm only thinking out loud, nothing less and
>> nothing more than that :-)
>
> Me too. :)
>
>> By the way, we can probably expand on that topic a bit later, as it's outside of
>> the scope of this specific change.
>>
>> Back to topic, if we get one series containing both changes (devicetree, bindings
>> and driver) with the right Fixes tags and/or Cc stable, we shouldn't have such
>> issue on backports so we can probably ignore that potential issue, I think? :-)
>
> Everything goes through the soc tree, so they should appear in Linus's tree
> and get picked by stable at more or less the same time. I think we would
> want the driver change to appear before the dts change, for bisectability's
> sake (because of header dependencies and driver errors).
>
> So we probably want:
> 1. driver + binding header changes
> 2. dtsi changes
>
> And have these merged through fixes so that the history between them is linear.
>

Perfect, I fully agree.

>
> ChenYu
>
>> Cheers,
>> Angelo
>>
>>> ChenYu
>>>
>>>> ...Then, as Chen-Yu said, you should also add the power domain to the scp_adsp
>>>> clock node as that's solving the lockup issue...
>>>>
>>>> .......and last, but not least: we need a Fixes tag to backport this fix, here
>>>> and on the commit that adds the missing power domain in the driver.
>>>>
>>>> Thanks,
>>>> Angelo
>>>>
>>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
>>>>> index 424fc89cc6f7..e71afba871fc 100644
>>>>> --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
>>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
>>>>> @@ -514,6 +514,14 @@
>>>>> };
>>>>> };
>>>>> };
>>>>> +
>>>>> + power-domain@MT8192_POWER_DOMAIN_ADSP {
>>>>> + reg = <MT8192_POWER_DOMAIN_ADSP>;
>>>>> + clocks = <&topckgen CLK_TOP_ADSP_SEL>;
>>>>> + clock-names = "adsp";
>>>>> + mediatek,infracfg = <&infracfg>;
>>>>> + #power-domain-cells = <0>;
>>>>> + };
>>>>> };
>>>>> };
>>>>>
>>>>> diff --git a/include/dt-bindings/power/mt8192-power.h b/include/dt-bindings/power/mt8192-power.h
>>>>> index 4eaa53d7270a..63e81cd0d06d 100644
>>>>> --- a/include/dt-bindings/power/mt8192-power.h
>>>>> +++ b/include/dt-bindings/power/mt8192-power.h
>>>>> @@ -28,5 +28,6 @@
>>>>> #define MT8192_POWER_DOMAIN_CAM_RAWA 18
>>>>> #define MT8192_POWER_DOMAIN_CAM_RAWB 19
>>>>> #define MT8192_POWER_DOMAIN_CAM_RAWC 20
>>>>> +#define MT8192_POWER_DOMAIN_ADSP 21
>>>>>
>>>>> #endif /* _DT_BINDINGS_POWER_MT8192_POWER_H */
>>>>>
>>>>
>>
>>

--
AngeloGioacchino Del Regno
Software Engineer

Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales, no. 5513718

\
 
 \ /
  Last update: 2022-12-01 11:11    [W:0.058 / U:0.248 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site