[PATCH 0/5] Support enabling interconnect path for GDSC for fixing Milos camcc

[PATCH 0/5] Support enabling interconnect path for GDSC for fixing Milos camcc

[Luca Weiss]

Some power domains like CAMSS_TOP_GDSC requires the enablement of the
multimedia NoC on newer SoCs like Milos.

Add support for getting an interconnect path as specified in the SoC
clock driver, and enabling/disabling that interconnect path when the
GDSC is being enabled/disabled.

Then specify that dependency in the camcc-milos driver and add it to the
dt-bindings and milos.dtsi.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
Luca Weiss (5):
      interconnect: Add devm_of_icc_get_by_index() as exported API for users
      dt-bindings: clock: qcom,milos-camcc: Document interconnect path
      clk: qcom: gdsc: Support enabling interconnect path for power domain
      clk: qcom: camcc-milos: Declare icc path dependency for CAMSS_TOP_GDSC
      arm64: dts: qcom: milos: Add interconnects to camcc

 .../devicetree/bindings/clock/qcom,milos-camcc.yaml  |  8 ++++++++
 arch/arm64/boot/dts/qcom/milos.dtsi                  |  3 +++
 drivers/clk/qcom/camcc-milos.c                       |  7 +++++++
 drivers/clk/qcom/gdsc.c                              | 19 +++++++++++++++++++
 drivers/clk/qcom/gdsc.h                              |  5 +++++
 drivers/interconnect/core.c                          | 20 ++++++++++++++++++++
 include/linux/interconnect.h                         |  6 ++++++
 7 files changed, 68 insertions(+)
---
base-commit: 48746fac3857845ce7ed4aeb56df58735208ffda
change-id: 20260116-milos-camcc-icc-d41bdff31ef8

Best regards,
-- 
Luca Weiss <luca.weiss@fairphone.com>

[PATCH 1/5] interconnect: Add devm_of_icc_get_by_index() as exported API for users

[Luca Weiss]

Users can use devm version of of_icc_get_by_index() to benefit from
automatic resource release.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 drivers/interconnect/core.c  | 20 ++++++++++++++++++++
 include/linux/interconnect.h |  6 ++++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index 6cc979b26151..2242554525df 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -443,6 +443,26 @@ struct icc_path *devm_of_icc_get(struct device *dev, const char *name)
 }
 EXPORT_SYMBOL_GPL(devm_of_icc_get);
 
+struct icc_path *devm_of_icc_get_by_index(struct device *dev, int idx)
+{
+	struct icc_path **ptr, *path;
+
+	ptr = devres_alloc(devm_icc_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	path = of_icc_get_by_index(dev, idx);
+	if (!IS_ERR(path)) {
+		*ptr = path;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return path;
+}
+EXPORT_SYMBOL_GPL(devm_of_icc_get_by_index);
+
 /**
  * of_icc_get_by_index() - get a path handle from a DT node based on index
  * @dev: device pointer for the consumer device
diff --git a/include/linux/interconnect.h b/include/linux/interconnect.h
index 4b12821528a6..75a32ad0482e 100644
--- a/include/linux/interconnect.h
+++ b/include/linux/interconnect.h
@@ -47,6 +47,7 @@ struct icc_path *of_icc_get(struct device *dev, const char *name);
 struct icc_path *devm_of_icc_get(struct device *dev, const char *name);
 int devm_of_icc_bulk_get(struct device *dev, int num_paths, struct icc_bulk_data *paths);
 struct icc_path *of_icc_get_by_index(struct device *dev, int idx);
+struct icc_path *devm_of_icc_get_by_index(struct device *dev, int idx);
 void icc_put(struct icc_path *path);
 int icc_enable(struct icc_path *path);
 int icc_disable(struct icc_path *path);
@@ -79,6 +80,11 @@ static inline struct icc_path *of_icc_get_by_index(struct device *dev, int idx)
 	return NULL;
 }
 
+static inline struct icc_path *devm_of_icc_get_by_index(struct device *dev, int idx)
+{
+	return NULL;
+}
+
 static inline void icc_put(struct icc_path *path)
 {
 }

-- 
2.52.0

Re: [PATCH 1/5] interconnect: Add devm_of_icc_get_by_index() as exported API for users

[Dmitry Baryshkov]

On Fri, Jan 16, 2026 at 02:17:20PM +0100, Luca Weiss wrote:
> Users can use devm version of of_icc_get_by_index() to benefit from
> automatic resource release.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  drivers/interconnect/core.c  | 20 ++++++++++++++++++++
>  include/linux/interconnect.h |  6 ++++++
>  2 files changed, 26 insertions(+)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

> 
-- 
With best wishes
Dmitry

Re: [PATCH 1/5] interconnect: Add devm_of_icc_get_by_index() as exported API for users

[Konrad Dybcio]

On 1/16/26 2:17 PM, Luca Weiss wrote:
> Users can use devm version of of_icc_get_by_index() to benefit from
> automatic resource release.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

[PATCH 2/5] dt-bindings: clock: qcom,milos-camcc: Document interconnect path

[Luca Weiss]

Document an interconnect path for camcc that's required to enable
the CAMSS_TOP_GDSC power domain.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml
index f63149ecf3e1..707b25d2c11e 100644
--- a/Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml
@@ -25,6 +25,10 @@ properties:
       - description: Sleep clock source
       - description: Camera AHB clock from GCC
 
+  interconnects:
+    items:
+      - description: Interconnect path to enable the MultiMedia NoC
+
 required:
   - compatible
   - clocks
@@ -37,12 +41,16 @@ unevaluatedProperties: false
 examples:
   - |
     #include <dt-bindings/clock/qcom,milos-gcc.h>
+    #include <dt-bindings/interconnect/qcom,icc.h>
+    #include <dt-bindings/interconnect/qcom,milos-rpmh.h>
     clock-controller@adb0000 {
         compatible = "qcom,milos-camcc";
         reg = <0x0adb0000 0x40000>;
         clocks = <&bi_tcxo_div2>,
                  <&sleep_clk>,
                  <&gcc GCC_CAMERA_AHB_CLK>;
+        interconnects = <&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS
+                         &mmss_noc SLAVE_MNOC_HF_MEM_NOC QCOM_ICC_TAG_ALWAYS>;
         #clock-cells = <1>;
         #reset-cells = <1>;
         #power-domain-cells = <1>;

-- 
2.52.0

Re: [PATCH 2/5] dt-bindings: clock: qcom,milos-camcc: Document interconnect path

[Krzysztof Kozlowski]

On Fri, Jan 16, 2026 at 02:17:21PM +0100, Luca Weiss wrote:
> Document an interconnect path for camcc that's required to enable
> the CAMSS_TOP_GDSC power domain.

I find it confusing. Enabling GDSC power domains is done via power
domains, not via interconnects. Do not represent power domains as
interconnects, it's something completely different.

> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml
> index f63149ecf3e1..707b25d2c11e 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml
> @@ -25,6 +25,10 @@ properties:
>        - description: Sleep clock source
>        - description: Camera AHB clock from GCC
>  
> +  interconnects:
> +    items:
> +      - description: Interconnect path to enable the MultiMedia NoC

And since when clock controllers are part of interconnect path... Even
more questions....

Best regards,
Krzysztof

Re: [PATCH 2/5] dt-bindings: clock: qcom,milos-camcc: Document interconnect path

[Konrad Dybcio]

On 1/17/26 12:46 PM, Krzysztof Kozlowski wrote:
> On Fri, Jan 16, 2026 at 02:17:21PM +0100, Luca Weiss wrote:
>> Document an interconnect path for camcc that's required to enable
>> the CAMSS_TOP_GDSC power domain.
> 
> I find it confusing. Enabling GDSC power domains is done via power
> domains, not via interconnects. Do not represent power domains as
> interconnects, it's something completely different.

The name of the power domains is CAMSS_TOP_GDSC (seems you misread)

For the power domain to successfully turn on, the MNoC needs to be
turned on (empirical evidence). The way to do it is to request a
nonzero vote on this interconnect path

(presumably because the GDSC or its invisible providers require
something connected over that bus to carry out their enable sequences).

Taniya should be able to explain in more detail

Konrad

Re: [PATCH 2/5] dt-bindings: clock: qcom,milos-camcc: Document interconnect path

[Konrad Dybcio]

On 1/19/26 11:20 AM, Konrad Dybcio wrote:
> On 1/17/26 12:46 PM, Krzysztof Kozlowski wrote:
>> On Fri, Jan 16, 2026 at 02:17:21PM +0100, Luca Weiss wrote:
>>> Document an interconnect path for camcc that's required to enable
>>> the CAMSS_TOP_GDSC power domain.
>>
>> I find it confusing. Enabling GDSC power domains is done via power
>> domains, not via interconnects. Do not represent power domains as
>> interconnects, it's something completely different.
> 
> The name of the power domains is CAMSS_TOP_GDSC (seems you misread)
> 
> For the power domain to successfully turn on, the MNoC needs to be
> turned on (empirical evidence). The way to do it is to request a
> nonzero vote on this interconnect path
> 
> (presumably because the GDSC or its invisible providers require
> something connected over that bus to carry out their enable sequences).
> 
> Taniya should be able to explain in more detail

We'd have a better chance of her responding if I didn't forget to
extend the recipient list, fixing that

Konrad

Re: [PATCH 2/5] dt-bindings: clock: qcom,milos-camcc: Document interconnect path

[Luca Weiss]

Hi Taniya,

On Mon Jan 19, 2026 at 11:28 AM CET, Konrad Dybcio wrote:
> On 1/19/26 11:20 AM, Konrad Dybcio wrote:
>> On 1/17/26 12:46 PM, Krzysztof Kozlowski wrote:
>>> On Fri, Jan 16, 2026 at 02:17:21PM +0100, Luca Weiss wrote:
>>>> Document an interconnect path for camcc that's required to enable
>>>> the CAMSS_TOP_GDSC power domain.
>>>
>>> I find it confusing. Enabling GDSC power domains is done via power
>>> domains, not via interconnects. Do not represent power domains as
>>> interconnects, it's something completely different.
>> 
>> The name of the power domains is CAMSS_TOP_GDSC (seems you misread)
>> 
>> For the power domain to successfully turn on, the MNoC needs to be
>> turned on (empirical evidence). The way to do it is to request a
>> nonzero vote on this interconnect path
>> 
>> (presumably because the GDSC or its invisible providers require
>> something connected over that bus to carry out their enable sequences).

Do you have any details you can share for this discussion?

Regards
Luca

Re: [PATCH 2/5] dt-bindings: clock: qcom,milos-camcc: Document interconnect path

[Mike Tipton]

On Mon, Jan 19, 2026 at 11:28:07AM +0100, Konrad Dybcio wrote:
> 
> 
> On 1/19/26 11:20 AM, Konrad Dybcio wrote:
> > On 1/17/26 12:46 PM, Krzysztof Kozlowski wrote:
> >> On Fri, Jan 16, 2026 at 02:17:21PM +0100, Luca Weiss wrote:
> >>> Document an interconnect path for camcc that's required to enable
> >>> the CAMSS_TOP_GDSC power domain.
> >>
> >> I find it confusing. Enabling GDSC power domains is done via power
> >> domains, not via interconnects. Do not represent power domains as
> >> interconnects, it's something completely different.
> > 
> > The name of the power domains is CAMSS_TOP_GDSC (seems you misread)
> > 
> > For the power domain to successfully turn on, the MNoC needs to be
> > turned on (empirical evidence). The way to do it is to request a
> > nonzero vote on this interconnect path
> > 
> > (presumably because the GDSC or its invisible providers require
> > something connected over that bus to carry out their enable sequences).

The GDSC itself shouldn't depend on MMNOC in order to turn on properly.
It should turn on just fine without it. There *is* a dependency between
CAM_TOP_GDSC and MMNOC, but it's in the opposite direction.

For MMNOC to turn off properly when all BW votes are removed, the
CAM_TOP_GDSC must already be off. If CAM_TOP_GDSC is still on when MMNOC
starts turning off, then MMNOC will get stuck in its collapse sequence.
MMNOC waits for all HW clients to de-assert their active signals before
it'll allow itself to collapse. Most HW blocks assert this active signal
more dynamically than camera does rather than tying it to GDSC state.
The GDSC asserting active signals to RPMh that prevent NOC collapse is
unique to this particular camera GDSC.

If MMNOC BW is removed when CAM_TOP_GDSC is still enabled, then it
should reproduce as an icc_set_bw() failure on MMNOC rather than a GDSC
enable failure. The icc_set_bw(0) request would succeed because RPMh
immediately ACKs down requests, but the collapse sequence would get
stuck in the background. Later, when someone calls icc_set_bw() again
with a non-zero BW to enable MMNOC, then that request would fail because
MMNOC is still stuck in the prior collapse sequence.

Note I haven't explicitly confirmed the Milos behavior, but this has
been the HW dependency for at least several generations of chips now.
I've never seen this GDSC get stuck turning on because MMNOC if off, nor
would I be able to explain offhand why that would happen. But MMNOC
certainly does depend on this GDSC for the reasons stated above.

So, regardless of the originally stated rationale, CAM_TOP_GDSC voting
on MMNOC *is* a logical requirement to guarantee that MMNOC doesn't turn
off when the GDSC is still on. Otherwise, that requirement would be left
entirely up to consumers to understand and enforce.

Re: [PATCH 2/5] dt-bindings: clock: qcom,milos-camcc: Document interconnect path

[Konrad Dybcio]

On 3/24/26 3:48 AM, Mike Tipton wrote:
> On Mon, Jan 19, 2026 at 11:28:07AM +0100, Konrad Dybcio wrote:
>>
>>
>> On 1/19/26 11:20 AM, Konrad Dybcio wrote:
>>> On 1/17/26 12:46 PM, Krzysztof Kozlowski wrote:
>>>> On Fri, Jan 16, 2026 at 02:17:21PM +0100, Luca Weiss wrote:
>>>>> Document an interconnect path for camcc that's required to enable
>>>>> the CAMSS_TOP_GDSC power domain.
>>>>
>>>> I find it confusing. Enabling GDSC power domains is done via power
>>>> domains, not via interconnects. Do not represent power domains as
>>>> interconnects, it's something completely different.
>>>
>>> The name of the power domains is CAMSS_TOP_GDSC (seems you misread)
>>>
>>> For the power domain to successfully turn on, the MNoC needs to be
>>> turned on (empirical evidence). The way to do it is to request a
>>> nonzero vote on this interconnect path
>>>
>>> (presumably because the GDSC or its invisible providers require
>>> something connected over that bus to carry out their enable sequences).
> 
> The GDSC itself shouldn't depend on MMNOC in order to turn on properly.
> It should turn on just fine without it. There *is* a dependency between
> CAM_TOP_GDSC and MMNOC, but it's in the opposite direction.
> 
> For MMNOC to turn off properly when all BW votes are removed, the
> CAM_TOP_GDSC must already be off. If CAM_TOP_GDSC is still on when MMNOC
> starts turning off, then MMNOC will get stuck in its collapse sequence.
> MMNOC waits for all HW clients to de-assert their active signals before
> it'll allow itself to collapse. Most HW blocks assert this active signal
> more dynamically than camera does rather than tying it to GDSC state.
> The GDSC asserting active signals to RPMh that prevent NOC collapse is
> unique to this particular camera GDSC.
> 
> If MMNOC BW is removed when CAM_TOP_GDSC is still enabled, then it
> should reproduce as an icc_set_bw() failure on MMNOC rather than a GDSC
> enable failure. The icc_set_bw(0) request would succeed because RPMh
> immediately ACKs down requests, but the collapse sequence would get
> stuck in the background. Later, when someone calls icc_set_bw() again
> with a non-zero BW to enable MMNOC, then that request would fail because
> MMNOC is still stuck in the prior collapse sequence.
> 
> Note I haven't explicitly confirmed the Milos behavior, but this has
> been the HW dependency for at least several generations of chips now.
> I've never seen this GDSC get stuck turning on because MMNOC if off, nor
> would I be able to explain offhand why that would happen. But MMNOC
> certainly does depend on this GDSC for the reasons stated above.

In case that helps, I believe Luca originally observed that keeping the
display on, specifically voting on the:

&mmss_noc MASTER_MDP <-> &mc_virt SLAVE_EBI1

path made the GDSC power on successfully, lacking that made the problem
come back

The nodes that are hit on the way are:

qnm_mdp (mmss_noc)
qns_mem_noc_hf (mmss_noc)
qnm_mnoc_hf (gem_noc)
qns_llcc (gem_noc)
llcc_mc (mc_virt)
ebi (mc_virt)

That translates into

mmss_noc -> MM0, MM1
gem_noc -> SH0, SH1
mc_virt -> ACV, MC0

Notably, all of the foo_camnoc_bar nodes reside on mmss_noc as well.
May it be that voting on them ungates a clock that (perhaps recursively)
feeds the GDSC?

Because the platform is not suspended, the latter two buses are already
online, so I'd assume it's one of the MM BCMs that's causing something

I can't see any details about its inputs.. In case that's useful, the
GCC driver enables the following camera branches as always-on:

GCC_CAMERA_AHB_CLK
GCC_CAMERA_HF_XO_CLK
GCC_CAMERA_SF_XO_CLK

Konrad

Re: [PATCH 2/5] dt-bindings: clock: qcom,milos-camcc: Document interconnect path

[Luca Weiss]

Hi Mike,

On Tue Mar 24, 2026 at 3:48 AM CET, Mike Tipton wrote:
> On Mon, Jan 19, 2026 at 11:28:07AM +0100, Konrad Dybcio wrote:
>> 
>> 
>> On 1/19/26 11:20 AM, Konrad Dybcio wrote:
>> > On 1/17/26 12:46 PM, Krzysztof Kozlowski wrote:
>> >> On Fri, Jan 16, 2026 at 02:17:21PM +0100, Luca Weiss wrote:
>> >>> Document an interconnect path for camcc that's required to enable
>> >>> the CAMSS_TOP_GDSC power domain.
>> >>
>> >> I find it confusing. Enabling GDSC power domains is done via power
>> >> domains, not via interconnects. Do not represent power domains as
>> >> interconnects, it's something completely different.
>> > 
>> > The name of the power domains is CAMSS_TOP_GDSC (seems you misread)
>> > 
>> > For the power domain to successfully turn on, the MNoC needs to be
>> > turned on (empirical evidence). The way to do it is to request a
>> > nonzero vote on this interconnect path
>> > 
>> > (presumably because the GDSC or its invisible providers require
>> > something connected over that bus to carry out their enable sequences).
>
> The GDSC itself shouldn't depend on MMNOC in order to turn on properly.
> It should turn on just fine without it. There *is* a dependency between
> CAM_TOP_GDSC and MMNOC, but it's in the opposite direction.

I can personally just write from practical experience, as Qualcomm
doesn't share any relevant documentation with OEMs.

Without this patch the GDSC refuses to turn on.

[  291.055839] ------------[ cut here ]------------
[  291.055860] cam_cc_camss_top_gdsc status stuck at 'off'
[  291.055878] WARNING: drivers/clk/qcom/gdsc.c:178 at gdsc_toggle_logic+0x138/0x144, CPU#4: hexdump/1995

With the patch it turns on just fine, no issues seen.

As Konrad has written, originally I didn't see any issue because that
interconnect was being kept alive by simple-framebuffer where I've added
'interconnects' to keep the framebuffer alive. However when testing
without this, the GDSC would refuse to turn on, which led me to this
patch series.

Additionally you can see in downstream devicetree you can also see an
interconnect defined for the "cam_cc_camss_top_gdsc" node:

https://gerrit-public.fairphone.software/plugins/gitiles/platform/vendor/qcom/proprietary/devicetree/+/refs/heads/odm/rc/target/15/fp6/fps_overlay/volcano.dtsi#2943

Regards
Luca

Re: [PATCH 2/5] dt-bindings: clock: qcom,milos-camcc: Document interconnect path

[Mike Tipton]

Hi Luca,

On Mon, Mar 30, 2026 at 04:55:40PM +0200, Luca Weiss wrote:
> Hi Mike,
> 
> On Tue Mar 24, 2026 at 3:48 AM CET, Mike Tipton wrote:
> > On Mon, Jan 19, 2026 at 11:28:07AM +0100, Konrad Dybcio wrote:
> >> 
> >> 
> >> On 1/19/26 11:20 AM, Konrad Dybcio wrote:
> >> > On 1/17/26 12:46 PM, Krzysztof Kozlowski wrote:
> >> >> On Fri, Jan 16, 2026 at 02:17:21PM +0100, Luca Weiss wrote:
> >> >>> Document an interconnect path for camcc that's required to enable
> >> >>> the CAMSS_TOP_GDSC power domain.
> >> >>
> >> >> I find it confusing. Enabling GDSC power domains is done via power
> >> >> domains, not via interconnects. Do not represent power domains as
> >> >> interconnects, it's something completely different.
> >> > 
> >> > The name of the power domains is CAMSS_TOP_GDSC (seems you misread)
> >> > 
> >> > For the power domain to successfully turn on, the MNoC needs to be
> >> > turned on (empirical evidence). The way to do it is to request a
> >> > nonzero vote on this interconnect path
> >> > 
> >> > (presumably because the GDSC or its invisible providers require
> >> > something connected over that bus to carry out their enable sequences).
> >
> > The GDSC itself shouldn't depend on MMNOC in order to turn on properly.
> > It should turn on just fine without it. There *is* a dependency between
> > CAM_TOP_GDSC and MMNOC, but it's in the opposite direction.
> 
> I can personally just write from practical experience, as Qualcomm
> doesn't share any relevant documentation with OEMs.
> 
> Without this patch the GDSC refuses to turn on.
> 
> [  291.055839] ------------[ cut here ]------------
> [  291.055860] cam_cc_camss_top_gdsc status stuck at 'off'
> [  291.055878] WARNING: drivers/clk/qcom/gdsc.c:178 at gdsc_toggle_logic+0x138/0x144, CPU#4: hexdump/1995
> 
> With the patch it turns on just fine, no issues seen.

I haven't observed that behavior, and I just reconfirmed on a more
recent chip. I explicitly toggled this GDSC on/off while MMNOC is
collapsed and it turns on fine. And if I disable MMNOC while the GDSC is
still on, then MMNOC gets stuck entering collapse. But I haven't tried
on Milos, specifically. It's possible there's some behavior unique to it
that I'm not aware of.

Regardless, the correct solution for both issues (MMNOC getting stuck
turning off or the GDSC getting stuck turning on) is the same. Which is
to vote for MMNOC on behalf of the GDSC as your patch does. And is also
what we've done downstream.

> 
> As Konrad has written, originally I didn't see any issue because that
> interconnect was being kept alive by simple-framebuffer where I've added
> 'interconnects' to keep the framebuffer alive. However when testing
> without this, the GDSC would refuse to turn on, which led me to this
> patch series.
> 
> Additionally you can see in downstream devicetree you can also see an
> interconnect defined for the "cam_cc_camss_top_gdsc" node:
> 
> https://gerrit-public.fairphone.software/plugins/gitiles/platform/vendor/qcom/proprietary/devicetree/+/refs/heads/odm/rc/target/15/fp6/fps_overlay/volcano.dtsi#2943

Right, this logic was originally added to prevent MMNOC from getting
stuck in collapse, rather than to prevent the GDSC from getting stuck
turning on.

Mike

> 
> Regards
> Luca

[PATCH 3/5] clk: qcom: gdsc: Support enabling interconnect path for power domain

[Luca Weiss]

On newer SoCs like Milos the CAMSS_TOP_GDSC power domains requires the
enablement of the multimedia NoC, otherwise the GDSC will be stuck on
'off'.

Add support for getting an interconnect path as specified in the SoC
clock driver, and enabling/disabling that interconnect path when the
GDSC is being enabled/disabled.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
icc_enable()/icc_disable() seems like a nice API but doesn't work
without setting the bandwidth first, so it's not very useful for this
driver, at least I couldn't figure out how to use it correctly.
---
 drivers/clk/qcom/gdsc.c | 19 +++++++++++++++++++
 drivers/clk/qcom/gdsc.h |  5 +++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index 7deabf8400cf..ff1acaa3e008 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -7,6 +7,7 @@
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/export.h>
+#include <linux/interconnect.h>
 #include <linux/jiffies.h>
 #include <linux/kernel.h>
 #include <linux/ktime.h>
@@ -261,6 +262,8 @@ static int gdsc_enable(struct generic_pm_domain *domain)
 	struct gdsc *sc = domain_to_gdsc(domain);
 	int ret;
 
+	icc_set_bw(sc->icc_path, 1, 1);
+
 	if (sc->pwrsts == PWRSTS_ON)
 		return gdsc_deassert_reset(sc);
 
@@ -360,6 +363,8 @@ static int gdsc_disable(struct generic_pm_domain *domain)
 	if (sc->flags & CLAMP_IO)
 		gdsc_assert_clamp_io(sc);
 
+	icc_set_bw(sc->icc_path, 0, 0);
+
 	return 0;
 }
 
@@ -574,6 +579,20 @@ int gdsc_register(struct gdsc_desc *desc,
 	if (!data->domains)
 		return -ENOMEM;
 
+	for (i = 0; i < num; i++) {
+		if (!scs[i] || !scs[i]->needs_icc)
+			continue;
+
+		scs[i]->icc_path = devm_of_icc_get_by_index(dev, scs[i]->icc_path_index);
+		if (IS_ERR(scs[i]->icc_path)) {
+			ret = PTR_ERR(scs[i]->icc_path);
+			if (ret != -ENODEV)
+				return ret;
+
+			scs[i]->icc_path = NULL;
+		}
+	}
+
 	for (i = 0; i < num; i++) {
 		if (!scs[i] || !scs[i]->supply)
 			continue;
diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index dd843e86c05b..92ff6bcce7b1 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -9,6 +9,7 @@
 #include <linux/err.h>
 #include <linux/pm_domain.h>
 
+struct icc_path;
 struct regmap;
 struct regulator;
 struct reset_controller_dev;
@@ -74,6 +75,10 @@ struct gdsc {
 
 	const char 			*supply;
 	struct regulator		*rsupply;
+
+	bool				needs_icc;
+	unsigned int			icc_path_index;
+	struct icc_path			*icc_path;
 };
 
 struct gdsc_desc {

-- 
2.52.0

Re: [PATCH 3/5] clk: qcom: gdsc: Support enabling interconnect path for power domain

[Mike Tipton]

On Fri, Jan 16, 2026 at 02:17:22PM +0100, Luca Weiss wrote:
> On newer SoCs like Milos the CAMSS_TOP_GDSC power domains requires the
> enablement of the multimedia NoC, otherwise the GDSC will be stuck on
> 'off'.

As mentioned in another email, the dependency should actually be in the
other direction. Where MMNOC gets stuck turning off if CAM_TOP_GDSC is
still on.

> 
> Add support for getting an interconnect path as specified in the SoC
> clock driver, and enabling/disabling that interconnect path when the
> GDSC is being enabled/disabled.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
> icc_enable()/icc_disable() seems like a nice API but doesn't work
> without setting the bandwidth first, so it's not very useful for this
> driver, at least I couldn't figure out how to use it correctly.

Agreed. In cases where a driver only needs simple zero or non-zero
votes, then icc_enable()/icc_disable() don't really help vs. just using
icc_set_bw().

> ---
>  drivers/clk/qcom/gdsc.c | 19 +++++++++++++++++++
>  drivers/clk/qcom/gdsc.h |  5 +++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index 7deabf8400cf..ff1acaa3e008 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -7,6 +7,7 @@
>  #include <linux/delay.h>
>  #include <linux/err.h>
>  #include <linux/export.h>
> +#include <linux/interconnect.h>
>  #include <linux/jiffies.h>
>  #include <linux/kernel.h>
>  #include <linux/ktime.h>
> @@ -261,6 +262,8 @@ static int gdsc_enable(struct generic_pm_domain *domain)
>  	struct gdsc *sc = domain_to_gdsc(domain);
>  	int ret;
>  
> +	icc_set_bw(sc->icc_path, 1, 1);

Need to handle the error. If the BW request fails, then we shouldn't
proceed to enable the GDSC.

Additionally, setting BW here doesn't handle the case where the GDSC is
enabled as part of gdsc_init(). If we move the icc_set_bw() calls into
gdsc_toggle_logic(), then we don't have to care about how many places
could ultimately enable or disable it. Since it's a fundamental HW
dependency, then placing the BW votes in the common place where we
actually toggle the GDSC on/off seems to make the most sense.

> +
>  	if (sc->pwrsts == PWRSTS_ON)
>  		return gdsc_deassert_reset(sc);
>  
> @@ -360,6 +363,8 @@ static int gdsc_disable(struct generic_pm_domain *domain)
>  	if (sc->flags & CLAMP_IO)
>  		gdsc_assert_clamp_io(sc);
>  
> +	icc_set_bw(sc->icc_path, 0, 0);

Similar to above -- we should handle the error case and ideally move
into gdsc_toggle_logic().

> +
>  	return 0;
>  }
>  
> @@ -574,6 +579,20 @@ int gdsc_register(struct gdsc_desc *desc,
>  	if (!data->domains)
>  		return -ENOMEM;
>  
> +	for (i = 0; i < num; i++) {
> +		if (!scs[i] || !scs[i]->needs_icc)
> +			continue;
> +
> +		scs[i]->icc_path = devm_of_icc_get_by_index(dev, scs[i]->icc_path_index);

I generally prefer using string-based DT lookups rather than
index-based, i.e. using devm_of_icc_get(). I know our clock drivers have
switched to primarily using index-based lookups, but I still generally
prefer string lookups:

  1. They're self-documenting within DT rather than relying on magic indices.
  2. The property name in the driver being non-NULL can indicate whether a
     handle is expected rather than relying on things like "needs_icc".
  3. Would remove the need of adding the new devm_of_icc_get_by_index() API, in
     this case.

There's nothing fundamentally wrong with this and I won't argue hard against it
especially considering that it's consistent with how our clock handles are being
looked up on more recent targets, but I often find the string lookups to be
cleaner and more robust.

> +		if (IS_ERR(scs[i]->icc_path)) {
> +			ret = PTR_ERR(scs[i]->icc_path);
> +			if (ret != -ENODEV)
> +				return ret;
> +
> +			scs[i]->icc_path = NULL;
> +		}
> +	}
> +
>  	for (i = 0; i < num; i++) {
>  		if (!scs[i] || !scs[i]->supply)
>  			continue;
> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
> index dd843e86c05b..92ff6bcce7b1 100644
> --- a/drivers/clk/qcom/gdsc.h
> +++ b/drivers/clk/qcom/gdsc.h
> @@ -9,6 +9,7 @@
>  #include <linux/err.h>
>  #include <linux/pm_domain.h>
>  
> +struct icc_path;
>  struct regmap;
>  struct regulator;
>  struct reset_controller_dev;
> @@ -74,6 +75,10 @@ struct gdsc {
>  
>  	const char 			*supply;
>  	struct regulator		*rsupply;
> +
> +	bool				needs_icc;
> +	unsigned int			icc_path_index;
> +	struct icc_path			*icc_path;
>  };
>  
>  struct gdsc_desc {
> 
> -- 
> 2.52.0
> 
> 

[PATCH 4/5] clk: qcom: camcc-milos: Declare icc path dependency for CAMSS_TOP_GDSC

[Luca Weiss]

This GDSC requires an interconnect path to be enabled, otherwise the
GDSC will be stuck on 'off' and can't be enabled.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 drivers/clk/qcom/camcc-milos.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/clk/qcom/camcc-milos.c b/drivers/clk/qcom/camcc-milos.c
index 0077c9c9249f..45e68165a2f0 100644
--- a/drivers/clk/qcom/camcc-milos.c
+++ b/drivers/clk/qcom/camcc-milos.c
@@ -30,6 +30,11 @@ enum {
 	DT_IFACE,
 };
 
+/* Need to match the order of interconnects in DT binding */
+enum {
+	DT_ICC_TOP_GDSC,
+};
+
 enum {
 	P_BI_TCXO,
 	P_CAM_CC_PLL0_OUT_EVEN,
@@ -1971,6 +1976,8 @@ static struct gdsc cam_cc_camss_top_gdsc = {
 	},
 	.pwrsts = PWRSTS_OFF_ON,
 	.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
+	.needs_icc = true,
+	.icc_path_index = DT_ICC_TOP_GDSC,
 };
 
 static struct clk_regmap *cam_cc_milos_clocks[] = {

-- 
2.52.0

Re: [PATCH 4/5] clk: qcom: camcc-milos: Declare icc path dependency for CAMSS_TOP_GDSC

[Dmitry Baryshkov]

On Fri, Jan 16, 2026 at 02:17:23PM +0100, Luca Weiss wrote:
> This GDSC requires an interconnect path to be enabled, otherwise the
> GDSC will be stuck on 'off' and can't be enabled.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  drivers/clk/qcom/camcc-milos.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

[PATCH 5/5] arm64: dts: qcom: milos: Add interconnects to camcc

[Luca Weiss]

The CAMSS_TOP_GDSC inside camcc requires an interconnect path to be
enabled, otherwise the GDSC will be stuck on 'off' and can't be enabled.

Add the interconnect path so that this requirement can be satisfied by
the kernel.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 arch/arm64/boot/dts/qcom/milos.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/milos.dtsi b/arch/arm64/boot/dts/qcom/milos.dtsi
index 0f69deabb60c..58b4c2966df1 100644
--- a/arch/arm64/boot/dts/qcom/milos.dtsi
+++ b/arch/arm64/boot/dts/qcom/milos.dtsi
@@ -1660,6 +1660,9 @@ camcc: clock-controller@adb0000 {
 				 <&sleep_clk>,
 				 <&gcc GCC_CAMERA_AHB_CLK>;
 
+			interconnects = <&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS
+					 &mmss_noc SLAVE_MNOC_HF_MEM_NOC QCOM_ICC_TAG_ALWAYS>;
+
 			#clock-cells = <1>;
 			#reset-cells = <1>;
 			#power-domain-cells = <1>;

-- 
2.52.0

Re: [PATCH 5/5] arm64: dts: qcom: milos: Add interconnects to camcc

[Dmitry Baryshkov]

On Fri, Jan 16, 2026 at 02:17:24PM +0100, Luca Weiss wrote:
> The CAMSS_TOP_GDSC inside camcc requires an interconnect path to be
> enabled, otherwise the GDSC will be stuck on 'off' and can't be enabled.
> 
> Add the interconnect path so that this requirement can be satisfied by
> the kernel.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  arch/arm64/boot/dts/qcom/milos.dtsi | 3 +++
>  1 file changed, 3 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH 0/5] Support enabling interconnect path for GDSC for fixing Milos camcc

[Konrad Dybcio]

On 1/16/26 2:17 PM, Luca Weiss wrote:
> Some power domains like CAMSS_TOP_GDSC requires the enablement of the
> multimedia NoC on newer SoCs like Milos.
> 
> Add support for getting an interconnect path as specified in the SoC
> clock driver, and enabling/disabling that interconnect path when the
> GDSC is being enabled/disabled.
> 
> Then specify that dependency in the camcc-milos driver and add it to the
> dt-bindings and milos.dtsi.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---

+Mike could you please take a look?

Konrad