Hi Angelo and Chen-Yu, On Thu, 2022-12-01 at 11:10 +0100, AngeloGioacchino Del Regno wrote: > 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@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. > > > > Perfect, I fully agree. > Thank you for your comments. I need to check internally with my coworkers for driver and will update v2. Best Regards, Allen > > > > 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_DOM > > > > > > AIN_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 */ > > > > > > > > > > > > > >
