On Thu, Dec 1, 2022 at 5:39 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxx> > >>> --- > >>> Ref: https://lore.kernel.org/all/2ec80bd8-dfef-d2e6-eb41-6e6088043e33@xxxxxxxxxxxxx/ > >>> [Allen-KH Cheng <allen-kh.cheng@xxxxxxxxxxxx>] > >>> --- > >>> --- > >>> 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. 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 */ > >>> > >> > >
