[PATCH] clk: qcom: regmap-phy-mux: Rework the implementation

[PATCH] clk: qcom: regmap-phy-mux: Rework the implementation

[Konrad Dybcio]

From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

The sole reason this hw exists is to let the branch clock downstream of
it keep running, with the PHY disengaged. This is not possible with the
current implementation, as the enabled status is hijacked to mean
"enabled" = "use fast/PHY source" and "disabled" = "use XO source".

This is an issue, since the mux enable state follows that of the child
branch, making the desired "child enabled, MUX @ XO" combination
impossible.

Solve that by implementing ratesetting. Because PHY clock rates may
change at runtime and aren't really deterministic from Linux, assume
ULONG_MAX as "fast clock" and 19.2 MHz as XO. All the branches in
question already set CLK_SET_RATE_PARENT, so everything works out.

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/clk/qcom/clk-regmap-phy-mux.c | 58 +++++++++++++++++++++++------------
 1 file changed, 38 insertions(+), 20 deletions(-)

diff --git a/drivers/clk/qcom/clk-regmap-phy-mux.c b/drivers/clk/qcom/clk-regmap-phy-mux.c
index 7b7243b7107d..b7d1c69d62f7 100644
--- a/drivers/clk/qcom/clk-regmap-phy-mux.c
+++ b/drivers/clk/qcom/clk-regmap-phy-mux.c
@@ -15,48 +15,66 @@
 #define PHY_MUX_PHY_SRC		0
 #define PHY_MUX_REF_SRC		2
 
+#define XO_RATE			19200000UL
+
 static inline struct clk_regmap_phy_mux *to_clk_regmap_phy_mux(struct clk_regmap *clkr)
 {
 	return container_of(clkr, struct clk_regmap_phy_mux, clkr);
 }
 
-static int phy_mux_is_enabled(struct clk_hw *hw)
+static unsigned long phy_mux_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 {
 	struct clk_regmap *clkr = to_clk_regmap(hw);
 	struct clk_regmap_phy_mux *phy_mux = to_clk_regmap_phy_mux(clkr);
-	unsigned int val;
+	u32 val;
 
 	regmap_read(clkr->regmap, phy_mux->reg, &val);
-	val = FIELD_GET(PHY_MUX_MASK, val);
 
-	WARN_ON(val != PHY_MUX_PHY_SRC && val != PHY_MUX_REF_SRC);
-
-	return val == PHY_MUX_PHY_SRC;
+	switch (FIELD_GET(PHY_MUX_MASK, val)) {
+	case PHY_MUX_PHY_SRC:
+		return ULONG_MAX;
+	case PHY_MUX_REF_SRC:
+		return XO_RATE;
+	default:
+		return 0;
+	}
 }
 
-static int phy_mux_enable(struct clk_hw *hw)
+static int phy_mux_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
+{
+	if (req->rate == XO_RATE || req->rate == ULONG_MAX)
+		return 0;
+
+	return -EINVAL;
+}
+
+static int phy_mux_set_rate(struct clk_hw *hw, unsigned long rate, unsigned long parent_rate)
 {
 	struct clk_regmap *clkr = to_clk_regmap(hw);
 	struct clk_regmap_phy_mux *phy_mux = to_clk_regmap_phy_mux(clkr);
+	u32 val;
 
-	return regmap_update_bits(clkr->regmap, phy_mux->reg,
-				  PHY_MUX_MASK,
-				  FIELD_PREP(PHY_MUX_MASK, PHY_MUX_PHY_SRC));
-}
-
-static void phy_mux_disable(struct clk_hw *hw)
-{
-	struct clk_regmap *clkr = to_clk_regmap(hw);
-	struct clk_regmap_phy_mux *phy_mux = to_clk_regmap_phy_mux(clkr);
+	switch (rate) {
+	case XO_RATE:
+		val = PHY_MUX_REF_SRC;
+		break;
+	case ULONG_MAX:
+		val = PHY_MUX_PHY_SRC;
+		break;
+	default:
+		return -EINVAL;
+	}
 
 	regmap_update_bits(clkr->regmap, phy_mux->reg,
 			   PHY_MUX_MASK,
-			   FIELD_PREP(PHY_MUX_MASK, PHY_MUX_REF_SRC));
+			   FIELD_PREP(PHY_MUX_MASK, val));
+
+	return 0;
 }
 
 const struct clk_ops clk_regmap_phy_mux_ops = {
-	.enable = phy_mux_enable,
-	.disable = phy_mux_disable,
-	.is_enabled = phy_mux_is_enabled,
+	.recalc_rate = phy_mux_recalc_rate,
+	.determine_rate = phy_mux_determine_rate,
+	.set_rate = phy_mux_set_rate,
 };
 EXPORT_SYMBOL_GPL(clk_regmap_phy_mux_ops);

---
base-commit: db7efce4ae23ad5e42f5f55428f529ff62b86fab
change-id: 20260409-topic-phy_fastclk-9258a40b75ed

Best regards,
-- 
Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Re: [PATCH] clk: qcom: regmap-phy-mux: Rework the implementation

[Bjorn Andersson]

On Thu, 09 Apr 2026 13:57:45 +0200, Konrad Dybcio wrote:
> The sole reason this hw exists is to let the branch clock downstream of
> it keep running, with the PHY disengaged. This is not possible with the
> current implementation, as the enabled status is hijacked to mean
> "enabled" = "use fast/PHY source" and "disabled" = "use XO source".
> 
> This is an issue, since the mux enable state follows that of the child
> branch, making the desired "child enabled, MUX @ XO" combination
> impossible.
> 
> [...]

Applied, thanks!

[1/1] clk: qcom: regmap-phy-mux: Rework the implementation
      commit: e108373c54fbc844b7f541c6fd7ecb31772afd3c

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

Re: [PATCH] clk: qcom: regmap-phy-mux: Rework the implementation

[Dmitry Baryshkov]

On Mon, Jun 08, 2026 at 10:09:55AM -0500, Bjorn Andersson wrote:
> 
> On Thu, 09 Apr 2026 13:57:45 +0200, Konrad Dybcio wrote:
> > The sole reason this hw exists is to let the branch clock downstream of
> > it keep running, with the PHY disengaged. This is not possible with the
> > current implementation, as the enabled status is hijacked to mean
> > "enabled" = "use fast/PHY source" and "disabled" = "use XO source".
> > 
> > This is an issue, since the mux enable state follows that of the child
> > branch, making the desired "child enabled, MUX @ XO" combination
> > impossible.
> > 
> > [...]
> 
> Applied, thanks!
> 
> [1/1] clk: qcom: regmap-phy-mux: Rework the implementation
>       commit: e108373c54fbc844b7f541c6fd7ecb31772afd3c

This breaks at PCIe at least on SM8350. The attached WiFi card is
not detected anymore. Rewerting the patch makes it work again.

> 
> Best regards,
> -- 
> Bjorn Andersson <andersson@kernel.org>

-- 
With best wishes
Dmitry

Re: [PATCH] clk: qcom: regmap-phy-mux: Rework the implementation

[Konrad Dybcio]

On 6/11/26 9:38 PM, Dmitry Baryshkov wrote:
> On Mon, Jun 08, 2026 at 10:09:55AM -0500, Bjorn Andersson wrote:
>>
>> On Thu, 09 Apr 2026 13:57:45 +0200, Konrad Dybcio wrote:
>>> The sole reason this hw exists is to let the branch clock downstream of
>>> it keep running, with the PHY disengaged. This is not possible with the
>>> current implementation, as the enabled status is hijacked to mean
>>> "enabled" = "use fast/PHY source" and "disabled" = "use XO source".
>>>
>>> This is an issue, since the mux enable state follows that of the child
>>> branch, making the desired "child enabled, MUX @ XO" combination
>>> impossible.
>>>
>>> [...]
>>
>> Applied, thanks!
>>
>> [1/1] clk: qcom: regmap-phy-mux: Rework the implementation
>>       commit: e108373c54fbc844b7f541c6fd7ecb31772afd3c
> 
> This breaks at PCIe at least on SM8350. The attached WiFi card is
> not detected anymore. Rewerting the patch makes it work again.

Hm, that's anticlimactic. Can you please dump:

gcc_pcie_0_pipe_clk_src: gcc+0x6b054
gcc_pcie_0_pipe_clk: gcc+0x52008

gcc_pcie_1_pipe_clk_src: gcc+0x8d054
gcc_pcie_1_pipe_clk: gcc+0x52000

Konrad

Re: [PATCH] clk: qcom: regmap-phy-mux: Rework the implementation

[Marek Szyprowski]

On 11.06.2026 21:38, Dmitry Baryshkov wrote:
> On Mon, Jun 08, 2026 at 10:09:55AM -0500, Bjorn Andersson wrote:
>> On Thu, 09 Apr 2026 13:57:45 +0200, Konrad Dybcio wrote:
>>> The sole reason this hw exists is to let the branch clock downstream of
>>> it keep running, with the PHY disengaged. This is not possible with the
>>> current implementation, as the enabled status is hijacked to mean
>>> "enabled" = "use fast/PHY source" and "disabled" = "use XO source".
>>>
>>> This is an issue, since the mux enable state follows that of the child
>>> branch, making the desired "child enabled, MUX @ XO" combination
>>> impossible.
>>>
>>> [...]
>> Applied, thanks!
>>
>> [1/1] clk: qcom: regmap-phy-mux: Rework the implementation
>>       commit: e108373c54fbc844b7f541c6fd7ecb31772afd3c
> This breaks at PCIe at least on SM8350. The attached WiFi card is
> not detected anymore. Rewerting the patch makes it work again.
Same on QCS6490-based RubikPi3, after this patch the XHCI USB host controller PCI
devive is no longer detected.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH] clk: qcom: regmap-phy-mux: Rework the implementation

[Marek Szyprowski]

On 26.06.2026 11:46, Marek Szyprowski wrote:
> On 11.06.2026 21:38, Dmitry Baryshkov wrote:
>> On Mon, Jun 08, 2026 at 10:09:55AM -0500, Bjorn Andersson wrote:
>>> On Thu, 09 Apr 2026 13:57:45 +0200, Konrad Dybcio wrote:
>>>> The sole reason this hw exists is to let the branch clock downstream of
>>>> it keep running, with the PHY disengaged. This is not possible with the
>>>> current implementation, as the enabled status is hijacked to mean
>>>> "enabled" = "use fast/PHY source" and "disabled" = "use XO source".
>>>>
>>>> This is an issue, since the mux enable state follows that of the child
>>>> branch, making the desired "child enabled, MUX @ XO" combination
>>>> impossible.
>>>>
>>>> [...]
>>> Applied, thanks!
>>>
>>> [1/1] clk: qcom: regmap-phy-mux: Rework the implementation
>>>       commit: e108373c54fbc844b7f541c6fd7ecb31772afd3c
>> This breaks at PCIe at least on SM8350. The attached WiFi card is
>> not detected anymore. Rewerting the patch makes it work again.
> Same on QCS6490-based RubikPi3, after this patch the XHCI USB host controller PCI
> devive is no longer detected.
A few more comments. I've checked the PCIe initialization code called on Rubik Pi3
board ("qcom_pcie_init_2_7_0()") and there is no call to set_rate(), which would 
change the MUX to PHY. The PCIe init code only calls clk_bulk_prepare_enable(),
but this became noop after this patch on the mentioned MUX.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH] clk: qcom: regmap-phy-mux: Rework the implementation

[Konrad Dybcio]

On 6/26/26 1:15 PM, Marek Szyprowski wrote:
> On 26.06.2026 11:46, Marek Szyprowski wrote:
>> On 11.06.2026 21:38, Dmitry Baryshkov wrote:
>>> On Mon, Jun 08, 2026 at 10:09:55AM -0500, Bjorn Andersson wrote:
>>>> On Thu, 09 Apr 2026 13:57:45 +0200, Konrad Dybcio wrote:
>>>>> The sole reason this hw exists is to let the branch clock downstream of
>>>>> it keep running, with the PHY disengaged. This is not possible with the
>>>>> current implementation, as the enabled status is hijacked to mean
>>>>> "enabled" = "use fast/PHY source" and "disabled" = "use XO source".
>>>>>
>>>>> This is an issue, since the mux enable state follows that of the child
>>>>> branch, making the desired "child enabled, MUX @ XO" combination
>>>>> impossible.
>>>>>
>>>>> [...]
>>>> Applied, thanks!
>>>>
>>>> [1/1] clk: qcom: regmap-phy-mux: Rework the implementation
>>>>       commit: e108373c54fbc844b7f541c6fd7ecb31772afd3c
>>> This breaks at PCIe at least on SM8350. The attached WiFi card is
>>> not detected anymore. Rewerting the patch makes it work again.
>> Same on QCS6490-based RubikPi3, after this patch the XHCI USB host controller PCI
>> devive is no longer detected.
> A few more comments. I've checked the PCIe initialization code called on Rubik Pi3
> board ("qcom_pcie_init_2_7_0()") and there is no call to set_rate(), which would 
> change the MUX to PHY. The PCIe init code only calls clk_bulk_prepare_enable(),
> but this became noop after this patch on the mentioned MUX.

Yeah, I sent a revert a couple days ago:

https://lore.kernel.org/linux-arm-msm/20260622-topic-phymux_revert-v1-1-f6ec85523840@oss.qualcomm.com/

The platform I originally tested this on seems to have different
defaults.

Konrad