Linux-PHY Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] phy: ti: j721e-wiz: Fix device_node leak in wiz_get_lane_phy_types()
From: Felix Gu @ 2026-02-10 18:14 UTC (permalink / raw)
  To: Vinod Koul, Neil Armstrong, Kishon Vijay Abraham I, Jyri Sarha
  Cc: linux-phy, linux-kernel, Felix Gu

The 'serdes' device_node is obtained using of_get_child_by_name(),
which increments the reference count. However, it is never put,
leading to a memory leak.

Use the __free(device_node) attribute to automatically decrement
the reference count when the 'serdes' variable goes out of scope.

Fixes: 7ae14cf581f2 ("phy: ti: j721e-wiz: Implement DisplayPort mode to the wiz driver")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/phy/ti/phy-j721e-wiz.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
index 12a19bf2875c..904541e9138e 100644
--- a/drivers/phy/ti/phy-j721e-wiz.c
+++ b/drivers/phy/ti/phy-j721e-wiz.c
@@ -1407,9 +1407,8 @@ MODULE_DEVICE_TABLE(of, wiz_id_table);
 
 static int wiz_get_lane_phy_types(struct device *dev, struct wiz *wiz)
 {
-	struct device_node *serdes;
-
-	serdes = of_get_child_by_name(dev->of_node, "serdes");
+	struct device_node *serdes __free(device_node) =
+		of_get_child_by_name(dev->of_node, "serdes");
 	if (!serdes) {
 		dev_err(dev, "%s: Getting \"serdes\"-node failed\n", __func__);
 		return -EINVAL;

---
base-commit: 193579fe01389bc21aff0051d13f24e8ea95b47d
change-id: 20260204-wiz-9a67604a034f

Best regards,
-- 
Felix Gu <ustc.gu@gmail.com>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply related

* Re: [PATCH 4/8] phy: zynqmp: Calibrate ILL if necessary
From: Sean Anderson @ 2026-02-10 16:42 UTC (permalink / raw)
  To: Pandey, Radhey Shyam, Laurent Pinchart, Vinod Koul,
	linux-phy@lists.infradead.org
  Cc: Krzysztof Wilczyński, Lorenzo Pieralisi,
	linux-kernel@vger.kernel.org, Simek, Michal,
	linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org,
	Neil Armstrong, Rob Herring, Havalige, Thippeswamy,
	Manivannan Sadhasivam, Bjorn Helgaas
In-Reply-To: <CY1PR12MB9697F67210064132883611A8B762A@CY1PR12MB9697.namprd12.prod.outlook.com>

On 2/10/26 11:04, Pandey, Radhey Shyam wrote:
> [Public]
> 
>> -----Original Message-----
>> From: Sean Anderson <sean.anderson@linux.dev>
>> Sent: Tuesday, February 3, 2026 5:51 AM
>> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>; Vinod Koul
>> <vkoul@kernel.org>; linux-phy@lists.infradead.org
>> Cc: Krzysztof Wilczyński <kwilczynski@kernel.org>; Lorenzo Pieralisi
>> <lpieralisi@kernel.org>; Pandey, Radhey Shyam
>> <radhey.shyam.pandey@amd.com>; linux-kernel@vger.kernel.org; Simek, Michal
>> <michal.simek@amd.com>; linux-arm-kernel@lists.infradead.org; linux-
>> pci@vger.kernel.org; Neil Armstrong <neil.armstrong@linaro.org>; Rob Herring
>> <robh@kernel.org>; Havalige, Thippeswamy <thippeswamy.havalige@amd.com>;
>> Manivannan Sadhasivam <mani@kernel.org>; Bjorn Helgaas
>> <bhelgaas@google.com>; Sean Anderson <sean.anderson@linux.dev>
>> Subject: [PATCH 4/8] phy: zynqmp: Calibrate ILL if necessary
>>
>> init_serdes in psu_init_gpl is supposed to calibrate the ILL. However, this
>> may fail if the reference clock is not running, such as if the clock needs
>> to be configured on boot. To work around this, add support for ILL
>> calibration in U-Boot. If the ILL is already calibrated (any non-zero
>> value) we skip calibration.
>>
>> The algorithm is substantially the same as serdes_illcalib [1], but it has
>> been updated for readability (and to remove all the "if (lane0_active)"
>> conditions). Due to the amount of register fields, many of which are
>> undocumented (especially the chicken bits), I have mostly used defines only
>> for the register names. There are certainly areas where register writes are
>> superfluous, but I have left these in order to minimize deviation from the
>> procedure in serdes_illcalib.
> 
> Please consider splitting the patch in introducing calibrate ILL functions
> and then subsequently using them.

OK

> How did you validate the changes? functional testing is one part
> but I think better to match register configuration done in psu_init vs
> this series?

I've done this in the past, but I didn't do so with this version of the
driver. I'll make sure to do this comparison for v2. FWIW the ILL
values achieved through this procedure are fairly close to the values I
got from the firmware.

> Have we tried running on multiple designs having
> different GT lane and protocol combinations .

Currently tested with SATA on GTR2 (on [1]) and PCIe on GTR0/1 (custom
design). I've also tested SGMII and DP on the other lanes (although not
USB as mentioned in the cover letter). I'd definitely appreciate if you
(or anyone else) could test on some other boards.

[1] https://developer.seco.com/hardware/product/computer-on-modules/smarc-modules/som-smarc-zu-b71/

> There are multiple magic numbers. Consider renaming it to
> meaningful defines.

Some of the fields have very long names. E.g. L0_TX_ANA_TM_3 has two
fields that we set: TX_serializer_enable and
force_TX_serializer_enabled. Setting it would look like

			xpsgtr_write_lanes(gtr_dev, lanes, L0_TX_ANA_TM_3,
					   L0_TX_ANA_TM_3_TX_SERIALIZER_ENABLE |
					   L0_TX_ANA_TM_3_FORCE_TX_SERIALIZER_ENABLE);

and there's really no way to get that under 80 characters. This is
probably the area where I could most improve things. But I'm not a fan
of tripling the line count since I think it makes it harder to follow
the flow of the initialization.

Other fields have poor documentation. E.g. L0_TM_IQ_ILL8 has a single
field ill_bypass_iq_polytrim_val documented as "IQ ILL polytrim bypass
value." I have no idea what this field does or why it needs to be set to
0xf3 so I can't really define a meaningful constant for it.

Lastly, some fields are not documented at all, notably UPHY_SPARE0.

In these latter two cases, I have not guessed at the semantics, but I'd
more than welcome any additional info.

>>
>> [1] Example implementation; xpsgtr_phy_illcalib coresponds to
>>     serdes_illcalib_pcie_gen1:
>> https://source.denx.de/u-boot/u-boot/-/blob/v2026.01/board/xilinx/zynqmp/zynqmp-
>> zcu208-revA/psu_init_gpl.c?ref_type=tags#L710
>>
>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>> ---
>>
>>  drivers/phy/xilinx/phy-zynqmp.c | 421 +++++++++++++++++++++++++++++++-
>>  1 file changed, 420 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
>> index 152af1702bbd..854b0ea04648 100644
>> --- a/drivers/phy/xilinx/phy-zynqmp.c
>> +++ b/drivers/phy/xilinx/phy-zynqmp.c
>> @@ -12,6 +12,7 @@
>>   * PCIe should also work but that is experimental as of now.
>>   */
>>
>> +#include <linux/bitfield.h>
>>  #include <linux/clk.h>
>>  #include <linux/debugfs.h>
>>  #include <linux/delay.h>
>> @@ -31,6 +32,7 @@
>>   */
>>
>>  /* TX De-emphasis parameters */
>> +#define L0_TX_ANA_TM_3                       0x000c
>>  #define L0_TX_ANA_TM_18                      0x0048
>>  #define L0_TX_ANA_TM_118             0x01d8
>>  #define L0_TX_ANA_TM_118_FORCE_17_0  BIT(0)
>> @@ -50,16 +52,49 @@
>>  #define L0_TXPMD_TM_45_ENABLE_DP_POST2       BIT(5)
>>
>>  /* PCS control parameters */
>> +#define L0_TM_ANA_BYP_4                      0x1010
>> +#define L0_TM_ANA_BYP_7                      0x1018
>>  #define L0_TM_DIG_6                  0x106c
>> +#define L0_TM_DIG_22                 0x10ac
>>  #define L0_TM_DIS_DESCRAMBLE_DECODER 0x0f
>>  #define L0_TX_DIG_61                 0x00f4
>>  #define L0_TM_DISABLE_SCRAMBLE_ENCODER       0x0f
>> +#define L0_TM_AUX_0                  0x10cc
>> +#define L0_TM_MISC2                  0x189c
>> +#define L0_TM_MISC2_ILL_CAL_BYPASS   BIT(7)
>> +#define L0_TM_IQ_ILL1                        0x18f8
>> +#define L0_TM_IQ_ILL2                        0x18fc
> 
> #define L0_TM_IQ_ILL3                   0x1900
> 
> IQ_ILL(n) = 0x18f8 + (n - 1) * 4 ?
> 
> Similarly for others  (when applicable)?

OK

>> +#define L0_TM_ILL11                  0x198c
>> +#define L0_TM_ILL12                  0x1990
>> +#define L0_TM_E_ILL1                 0x1924
>> +#define L0_TM_E_ILL2                 0x1928
>> +#define L0_TM_IQ_ILL3                        0x1900
>> +#define L0_TM_E_ILL3                 0x192c
>> +#define L0_TM_IQ_ILL7                        0x1910
>> +#define L0_TM_E_ILL7                 0x193c
>> +#define L0_TM_ILL8                   0x1980
>> +#define L0_TM_IQ_ILL8                        0x1914
>> +#define L0_TM_IQ_ILL9                        0x1918
>> +#define L0_TM_EQ0                    0x194c
>> +#define L0_TM_EQ0_EQ_STG2_CTRL_BYP   BIT(5)
>> +#define L0_TM_EQ1                    0x1950
>> +#define L0_TM_EQ1_EQ_STG2_RL_PROG    GENMASK(1, 0)
>> +#define L0_TM_EQ1_EQ_STG2_PREAMP_MODE_VAL    BIT(2)
>> +#define L0_TM_E_ILL8                 0x1940
>> +#define L0_TM_E_ILL9                 0x1944
>> +#define L0_TM_ILL13                  0x1994
>> +#define L0_TM_CDR5                   0x1c14
>> +#define L0_TM_CDR5_FPHL_FSM_ACC_CYCLES       GENMASK(7, 5)
>> +#define L0_TM_CDR5_FFL_PH0_INT_GAIN  GENMASK(4, 0)
>> +#define L0_TM_CDR16                  0x1c40
>>
>>  /* PLL Test Mode register parameters */
>> +#define L0_TM_PLL_DIG_33             0x2084
>>  #define L0_TM_PLL_DIG_37             0x2094
>>  #define L0_TM_COARSE_CODE_LIMIT              0x10
>>
>>  /* PLL SSC step size offsets */
>> +#define L0_PLL_FBDIV_FRAC_3_MSB              0x2360
>>  #define L0_PLL_SS_STEPS_0_LSB                0x2368
>>  #define L0_PLL_SS_STEPS_1_MSB                0x236c
>>  #define L0_PLL_SS_STEP_SIZE_0_LSB    0x2370
>> @@ -69,6 +104,7 @@
>>  #define L0_PLL_STATUS_READ_1         0x23e4
>>
>>  /* SSC step size parameters */
>> +#define TM_FORCE_EN_FRAC             BIT(6)
>>  #define STEP_SIZE_0_MASK             0xff
>>  #define STEP_SIZE_1_MASK             0xff
>>  #define STEP_SIZE_2_MASK             0xff
>> @@ -76,6 +112,7 @@
>>  #define STEP_SIZE_SHIFT                      8
>>  #define FORCE_STEP_SIZE                      0x10
>>  #define FORCE_STEPS                  0x20
>> +#define TM_FORCE_EN                  BIT(7)
>>  #define STEPS_0_MASK                 0xff
>>  #define STEPS_1_MASK                 0x07
>>
>> @@ -84,6 +121,32 @@
>>  #define L0_REF_CLK_LCL_SEL           BIT(7)
>>  #define L0_REF_CLK_SEL_MASK          0x9f
>>
>> +/* Built-in self-test parameters */
>> +#define L0_BIST_CTRL_1                       0x3004
>> +#define L0_BIST_CTRL_2                       0x3008
>> +#define L0_BIST_RUN_LEN_L            0x300c
>> +#define L0_BIST_ERR_INJ_POINT_L              0x3010
>> +#define L0_BIST_RUNLEN_ERR_INJ_H     0x3014
>> +#define L0_BIST_IDLE_TIME            0x3018
>> +#define L0_BIST_MARKER_L             0x301c
>> +#define L0_BIST_IDLE_CHAR_L          0x3020
>> +#define L0_BIST_MARKER_IDLE_H                0x3024
>> +#define L0_BIST_LOW_PULSE_TIME               0x3028
>> +#define L0_BIST_TOTAL_PULSE_TIME     0x302c
>> +#define L0_BIST_TEST_PAT_1           0x3030
>> +#define L0_BIST_TEST_PAT_2           0x3034
>> +#define L0_BIST_TEST_PAT_3           0x3038
>> +#define L0_BIST_TEST_PAT_4           0x303c
>> +#define L0_BIST_TEST_PAT_MSBS                0x3040
>> +#define L0_BIST_PKT_NUM                      0x3044
>> +#define L0_BIST_FRM_IDLE_TIME                0x3048
>> +#define L0_BIST_PKT_CTR_L            0x304c
>> +#define L0_BIST_PKT_CTR_H            0x3050
>> +#define L0_BIST_ERR_CTR_L            0x3054
>> +#define L0_BIST_ERR_CTR_H            0x3058
>> +#define L0_BIST_FILLER_OUT           0x3068
>> +#define L0_BIST_FORCE_MK_RST         0x306c
>> +
>>  /* Calibration digital logic parameters */
>>  #define L3_TM_CALIB_DIG19            0xec4c
>>  #define L3_CALIB_DONE_STATUS         0xef14
>> @@ -139,6 +202,9 @@ static const char *const xpsgtr_icm_str[] = {
>>  #define TM_CMN_RST_SET                       0x2
>>  #define TM_CMN_RST_MASK                      0x3
>>
>> +#define LPBK_CTRL0                   0x10038
>> +#define LPBK_CTRL1                   0x1003c
>> +
>>  /* Bus width parameters */
>>  #define TX_PROT_BUS_WIDTH            0x10040
>>  #define RX_PROT_BUS_WIDTH            0x10044
>> @@ -148,9 +214,13 @@ static const char *const xpsgtr_icm_str[] = {
>>  #define PROT_BUS_WIDTH_SHIFT(n)              ((n) * 2)
>>  #define PROT_BUS_WIDTH_MASK(n)               GENMASK((n) * 2 + 1, (n) * 2)
>>
>> +#define UPHY_SPARE0                  0X10098
>> +
>>  /* Number of GT lanes */
>>  #define NUM_LANES                    4
>>
>> +#define SIOU_ECO_0                   0x1c
>> +
>>  /* SIOU SATA control register */
>>  #define SATA_CONTROL_OFFSET          0x0100
>>
>> @@ -338,6 +408,33 @@ static void xpsgtr_restore_lane_regs(struct xpsgtr_dev
>> *gtr_dev)
>>                            gtr_dev->saved_regs[i]);
>>  }
>>
>> +static inline void xpsgtr_write_lanes(struct xpsgtr_dev *gtr_dev,
>> +                                  unsigned long *lanes, u32 reg, u32 value)
>> +{
>> +     unsigned long lane;
>> +
>> +     for_each_set_bit(lane, lanes, NUM_LANES) {
>> +             void __iomem *addr = gtr_dev->serdes + lane * PHY_REG_OFFSET
>> +                                  + reg;
>> +
>> +             writel(value, addr);
>> +     }
>> +}
>> +
>> +static inline void xpsgtr_clr_set_lanes(struct xpsgtr_dev *gtr_dev,
>> +                                    unsigned long *lanes, u32 reg, u32 clr,
>> +                                    u32 set)
>> +{
>> +     unsigned long lane;
>> +
>> +     for_each_set_bit(lane, lanes, NUM_LANES) {
>> +             void __iomem *addr = gtr_dev->serdes + lane * PHY_REG_OFFSET
>> +                                  + reg;
>> +
>> +             writel((readl(addr) & ~clr) | set, addr);
>> +     }
>> +}
>> +
>>  /*
>>   * Hardware Configuration
>>   */
>> @@ -351,7 +448,7 @@ static int xpsgtr_wait_pll_lock(struct phy *phy)
>>       u8 protocol = gtr_phy->protocol;
>>       int ret;
>>
>> -     dev_dbg(gtr_dev->dev, "Waiting for PLL lock\n");
>> +     dev_vdbg(gtr_dev->dev, "Waiting for PLL lock\n");
> 
> Unrelated change.

We reconfigure the PLL many times during ILL training, and this debug
output makes it difficult to read the ILL calibration debug.

>>
>>       /*
>>        * For DP and PCIe, only the instance 0 PLL is used. Switch to that phy
>> @@ -520,6 +617,231 @@ static void xpsgtr_bypass_scrambler_8b10b(struct
>> xpsgtr_phy *gtr_phy)
>>       xpsgtr_write_phy(gtr_phy, L0_TX_DIG_61,
>> L0_TM_DISABLE_SCRAMBLE_ENCODER);
>>  }
>>
>> +/* Enable or disable loopback */
>> +static void xpsgtr_phy_set_loopback(struct xpsgtr_phy *gtr_phy, bool enabled)
>> +{
>> +     struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
>> +     u32 reg = gtr_phy->lane >= 2 ? LPBK_CTRL1 : LPBK_CTRL0;
>> +     u32 shift = gtr_phy->lane & 1 ? 4 : 0;
>> +
>> +     xpsgtr_clr_set(gtr_dev, reg, 7 << shift, (u32)enabled << shift);
>> +}
>> +
>> +static void xpsgtr_phy_set_ill(struct xpsgtr_phy *gtr_phy, u32 ill, bool gen2)
>> +{
>> +     u32 val = 4 + ill * 8;
>> +
>> +     if (gen2) {
>> +             xpsgtr_write_phy(gtr_phy, L0_TM_E_ILL2, val & 0xff);
>> +             xpsgtr_clr_set_phy(gtr_phy, L0_TM_ILL12, 0x0f,
>> +                                1 << (val >> 8));
>> +     } else {
>> +             xpsgtr_write_phy(gtr_phy, L0_TM_E_ILL1, val & 0xff);
>> +             xpsgtr_clr_set_phy(gtr_phy, L0_TM_ILL12, 0xf0,
>> +                                (val >> 4) & 0x10);
>> +     }
>> +}
>> +
>> +static bool xpsgtr_ill_calibrated(struct xpsgtr_phy *gtr_phy)
>> +{
>> +     u32 ill1 = xpsgtr_read_phy(gtr_phy, L0_TM_E_ILL1);
>> +     u32 ill2 = xpsgtr_read_phy(gtr_phy, L0_TM_E_ILL2);
>> +     u32 ill12 = xpsgtr_read_phy(gtr_phy, L0_TM_ILL12);
>> +
>> +     dev_dbg(gtr_phy->dev->dev, "lane %u gen1 ILL was %u gen2 ILL was
>> %u\n",
>> +             gtr_phy->lane, ill1 / 8 + (ill12 & 0x10 ? 32 : 0),
>> +             ill2 / 8 + (ill12 & 0x02 ? 32 : 0));
>> +     return ill1 || ill2 || ill12;
>> +}
>> +
>> +static void xpsgtr_init_ill(struct xpsgtr_phy *gtr_phy)
>> +{
>> +     struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
>> +     struct clk *clk = gtr_dev->clk[gtr_phy->refclk];
>> +     u32 ill123 = DIV_ROUND_CLOSEST(clk_get_rate(clk), 1000000);
>> +
>> +     xpsgtr_clr_set_phy(gtr_phy, L0_TM_MISC2, 0,
>> L0_TM_MISC2_ILL_CAL_BYPASS);
>> +     xpsgtr_write_phy(gtr_phy, L0_TM_IQ_ILL1, ill123);
>> +     xpsgtr_write_phy(gtr_phy, L0_TM_IQ_ILL2, ill123);
>> +     xpsgtr_write_phy(gtr_phy, L0_TM_IQ_ILL3, ill123);
>> +     xpsgtr_write_phy(gtr_phy, L0_TM_IQ_ILL7, 0xf3);
>> +     xpsgtr_write_phy(gtr_phy, L0_TM_E_ILL7, 0xf3);
>> +     xpsgtr_write_phy(gtr_phy, L0_TM_ILL8, 0xff);
>> +     xpsgtr_write_phy(gtr_phy, L0_TM_IQ_ILL8, 0xf3);
>> +     xpsgtr_write_phy(gtr_phy, L0_TM_E_ILL8, 0xf3);
>> +     xpsgtr_write_phy(gtr_phy, L0_TM_IQ_ILL9, 1);
>> +     xpsgtr_write_phy(gtr_phy, L0_TM_E_ILL9, 1);
>> +     xpsgtr_clr_set(gtr_dev, UPHY_SPARE0, BIT(5), 0);
>> +}
>> +
>> +static void xpsgtr_phy_illcalib(struct xpsgtr_dev *gtr_dev,
>> +                                  unsigned long *lanes, bool gen2)
>> +{
>> +     bool last_ok[NUM_LANES] = { 0 };
>> +     int pass[NUM_LANES] = { 0 }, altpass[NUM_LANES] = { 0 };
>> +     int best[NUM_LANES] = { 0 }, altbest[NUM_LANES] = { 0 };
>> +     unsigned long lane;
>> +     int i;
>> +
>> +     /* Initialize the BIST */
>> +     xpsgtr_clr_set_lanes(gtr_dev, lanes, L0_BIST_CTRL_1, 0xe0, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_FILLER_OUT, 1);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_FORCE_MK_RST, 1);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_DIG_22, 0x20);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_CTRL_2, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_RUN_LEN_L, 0xf4);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_ERR_INJ_POINT_L, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_RUNLEN_ERR_INJ_H, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_IDLE_TIME, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_MARKER_L, 0xfb);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_IDLE_CHAR_L, 0xff);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_MARKER_IDLE_H, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_LOW_PULSE_TIME, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TOTAL_PULSE_TIME, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_1, 0x4a);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_2, 0x4a);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_3, 0x4a);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_4, 0x4a);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_MSBS, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_PKT_NUM, 0x14);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_FRM_IDLE_TIME, 2);
>> +     xpsgtr_clr_set_lanes(gtr_dev, lanes, L0_BIST_CTRL_1, 0xe0, 0);
>> +
>> +     for (i = 0; i < 64; i++) {
>> +             bool ok[NUM_LANES];
>> +
>> +             for_each_set_bit(lane, lanes, NUM_LANES)
>> +                     xpsgtr_phy_set_ill(&gtr_dev->phys[lane], i, gen2);
>> +
>> +             /* Reset lanes */
>> +             xpsgtr_clr_set_lanes(gtr_dev, lanes, L0_TM_ANA_BYP_7, 0x20,
>> +                                  0x10);
>> +             xpsgtr_write(gtr_dev, UPHY_SPARE0, 0x00);
>> +             xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_ANA_BYP_4, 0x40);
>> +             xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_PLL_DIG_33, 0x80);
>> +             xpsgtr_write(gtr_dev, UPHY_SPARE0, 0x04);
>> +             udelay(50);
>> +             if (gen2)
>> +                     xpsgtr_write(gtr_dev, UPHY_SPARE0, 0x0e);
>> +             xpsgtr_write(gtr_dev, UPHY_SPARE0, 0x06);
>> +             if (gen2) {
>> +                     xpsgtr_write_lanes(gtr_dev, lanes, L0_TX_ANA_TM_3, 0x04);
>> +                     xpsgtr_write(gtr_dev, UPHY_SPARE0, 0x07);
>> +                     udelay(400);
>> +                     xpsgtr_write_lanes(gtr_dev, lanes, L0_TX_ANA_TM_3, 0x0c);
>> +                     udelay(15);
>> +                     xpsgtr_write(gtr_dev, UPHY_SPARE0, 0x0f);
>> +                     udelay(100);
>> +             }
>> +
>> +             if (xpsgtr_wait_pll_lock(gtr_dev->phys[0].phy)) {
>> +                     memset(last_ok, 0, sizeof(last_ok));
>> +                     continue;
>> +             }
>> +
>> +             udelay(50);
>> +             xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_ANA_BYP_4, 0xc0);
>> +             xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_ANA_BYP_4, 0x80);
>> +             xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_PLL_DIG_33, 0xc0);
>> +             udelay(50);
>> +             xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_PLL_DIG_33, 0x80);
>> +             udelay(50);
>> +             xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_ANA_BYP_4, 0);
>> +             xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_PLL_DIG_33, 0);
>> +             udelay(500);
>> +
>> +             /* Do the BIST */
>> +             for_each_set_bit(lane, lanes, NUM_LANES) {
>> +                     struct xpsgtr_phy *gtr_phy = &gtr_dev->phys[lane];
>> +                     u32 packets, errors;
>> +
>> +                     xpsgtr_phy_init_bus_width(gtr_phy, PROT_BUS_WIDTH_10);
>> +                     xpsgtr_phy_set_loopback(gtr_phy, true);
>> +                     xpsgtr_write_phy(gtr_phy, L0_TM_DIG_22, 0x20);
>> +                     xpsgtr_clr_set_phy(gtr_phy, L0_BIST_CTRL_1, 0, 1);
>> +
>> +                     udelay(200);
>> +                     xpsgtr_write_phy(gtr_phy, L0_BIST_CTRL_1, 0);
>> +                     packets = xpsgtr_read_phy(gtr_phy, L0_BIST_PKT_CTR_L);
>> +                     packets |= xpsgtr_read_phy(gtr_phy, L0_BIST_PKT_CTR_H)
>> << 8;
>> +                     errors = xpsgtr_read_phy(gtr_phy, L0_BIST_ERR_CTR_L);
>> +                     errors |= xpsgtr_read_phy(gtr_phy, L0_BIST_ERR_CTR_H)
>> << 8;
>> +                     ok[lane] = packets && !errors;
>> +
>> +                     dev_dbg(gtr_dev->dev,
>> +                             "lane %lu ILL %d packets %10u errors %10u\n",
>> +                             lane, i, packets, errors);
>> +             }
>> +
>> +             xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_CTRL_1, 0);
>> +             xpsgtr_write(gtr_dev, UPHY_SPARE0, 0x00);
>> +             xpsgtr_write(gtr_dev, UPHY_SPARE0, 0x02);
>> +
>> +             for_each_set_bit(lane, lanes, NUM_LANES) {
>> +                     pass[lane] += ok[lane] && last_ok[lane];
>> +                     if (pass[lane] < 4) {
>> +                             if (!ok[lane] && i > 2) {
>> +                                     if (altpass[lane] < pass[lane]) {
>> +                                             altpass[lane] = pass[lane];
>> +                                             altbest[lane] =
>> +                                                     (i - 1) - (pass[lane] + 1) / 2;
>> +                                     }
>> +                                     pass[lane] = 0;
>> +                             }
>> +                     } else if (!best[lane] && (!ok[lane] || i == 63) &&
>> +                                last_ok[lane]) {
>> +                             best[lane] = (i - 1) - (pass[lane] + 1) / 2;
>> +                     }
>> +             }
>> +
>> +             memcpy(last_ok, ok, sizeof(ok));
>> +     }
>> +
>> +     for_each_set_bit(lane, lanes, NUM_LANES) {
>> +             dev_dbg(gtr_dev->dev, "lane %lu ILL best %d alt best %d\n",
>> +                     lane, best[lane], altbest[lane]);
>> +
>> +             xpsgtr_phy_set_ill(&gtr_dev->phys[lane],
>> +                                best[lane] ?: altbest[lane], gen2);
>> +     }
>> +
>> +     /* Clean up */
>> +     xpsgtr_clr_set_lanes(gtr_dev, lanes, L0_TM_ANA_BYP_7, 0x30, 0);
>> +     xpsgtr_write(gtr_dev, UPHY_SPARE0, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_CTRL_1, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_CTRL_2, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_RUN_LEN_L, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_ERR_INJ_POINT_L, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_RUNLEN_ERR_INJ_H, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_IDLE_TIME, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_MARKER_L, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_IDLE_CHAR_L, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_MARKER_IDLE_H, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_LOW_PULSE_TIME, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TOTAL_PULSE_TIME, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_1, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_2, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_3, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_4, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_MSBS, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_PKT_NUM, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_FRM_IDLE_TIME, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_PKT_CTR_L, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_PKT_CTR_H, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_ERR_CTR_L, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_ERR_CTR_H, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_FILLER_OUT, 1);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_FORCE_MK_RST, 0);
>> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_DIG_22, 0);
>> +
>> +     for_each_set_bit(lane, lanes, NUM_LANES) {
>> +             struct xpsgtr_phy *gtr_phy = &gtr_dev->phys[lane];
>> +
>> +             xpsgtr_phy_init_bus_width(gtr_phy, PROT_BUS_WIDTH_20);
>> +             xpsgtr_phy_set_loopback(gtr_phy, false);
>> +     }
>> +}
>> +
>>  static int xpsgtr_common_init(struct xpsgtr_phy *gtr_phy)
>>  {
>>       int ret;
>> @@ -553,6 +875,37 @@ static void xpsgtr_phy_init_sata(struct xpsgtr_phy
>> *gtr_phy)
>>  {
>>       struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
>>
>> +     if (!xpsgtr_ill_calibrated(gtr_phy)) {
>> +             DECLARE_BITMAP(lanes, NUM_LANES) = { 0 };
>> +
>> +             xpsgtr_init_ill(gtr_phy);
>> +             xpsgtr_write_phy(gtr_phy, L0_TM_E_ILL3, 100);
>> +             xpsgtr_clr_set_phy(gtr_phy, L0_TM_ILL11, 0xf0, 0x20);
>> +
>> +             __set_bit(gtr_phy->lane, lanes);
>> +             xpsgtr_phy_illcalib(gtr_dev, lanes, false);
>> +             xpsgtr_phy_set_ill(gtr_phy, 7, true);
>> +     }
>> +
>> +     /* Disable SSC */
>> +     xpsgtr_write_phy(gtr_phy, L0_PLL_FBDIV_FRAC_3_MSB,
>> TM_FORCE_EN_FRAC);
>> +     xpsgtr_clr_set_phy(gtr_phy, L0_PLL_SS_STEP_SIZE_3_MSB, 0,
>> TM_FORCE_EN);
>> +
>> +     /* Disable Tx deemphasis */
>> +     xpsgtr_write_phy(gtr_phy, L0_TM_CDR5,
>> +                      FIELD_PREP(L0_TM_CDR5_FPHL_FSM_ACC_CYCLES,
>> 7) |
>> +                      FIELD_PREP(L0_TM_CDR5_FFL_PH0_INT_GAIN, 6));
>> +     xpsgtr_write_phy(gtr_phy, L0_TM_CDR16, 12);
>> +
>> +     /* Configure equalization */
>> +     xpsgtr_write_phy(gtr_phy, L0_TX_ANA_TM_118,
>> +                      L0_TX_ANA_TM_118_FORCE_17_0);
>> +     xpsgtr_clr_set_phy(gtr_phy, L0_TM_EQ0, 0,
>> L0_TM_EQ0_EQ_STG2_CTRL_BYP);
>> +     xpsgtr_clr_set_phy(gtr_phy, L0_TM_EQ1,
>> L0_TM_EQ1_EQ_STG2_RL_PROG,
>> +                        FIELD_PREP(L0_TM_EQ1_EQ_STG2_RL_PROG, 2) |
>> +                        L0_TM_EQ1_EQ_STG2_PREAMP_MODE_VAL);
>> +     xpsgtr_write_phy(gtr_phy, L0_TX_ANA_TM_18, 2); /* -3.5 dB deemphasis */
>> +
>>       xpsgtr_bypass_scrambler_8b10b(gtr_phy);
>>
>>       writel(gtr_phy->lane, gtr_dev->siou + SATA_CONTROL_OFFSET);
>> @@ -565,6 +918,64 @@ static void xpsgtr_phy_init_sgmii(struct xpsgtr_phy
>> *gtr_phy)
>>       xpsgtr_bypass_scrambler_8b10b(gtr_phy);
>>  }
>>
>> +/* PCIe-specific initialization. */
>> +static int xpsgtr_phy_init_pcie(struct xpsgtr_phy *gtr_phy)
>> +{
>> +     struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
>> +     DECLARE_BITMAP(lanes, NUM_LANES) = { 0 };
>> +     unsigned long lane;
>> +     bool calibrated = false;
>> +
>> +     xpsgtr_clr_set_phy(gtr_phy, L0_TM_AUX_0, 0, 0x20);
>> +
>> +     for (lane = 0; lane < NUM_LANES; lane++) {
>> +             struct xpsgtr_phy *gtr_phy = &gtr_dev->phys[lane];
>> +
>> +             if (gtr_phy->protocol != ICM_PROTOCOL_PCIE)
>> +                     continue;
>> +
>> +             __set_bit(lane, lanes);
>> +             calibrated = calibrated || xpsgtr_ill_calibrated(gtr_phy);
>> +     }
>> +
>> +     if (calibrated)
>> +             return 0;
>> +
>> +     /* Write default ILL config */
>> +     for_each_set_bit(lane, lanes, NUM_LANES) {
>> +             struct xpsgtr_phy *p = &gtr_dev->phys[lane];
>> +
>> +             if (lane != gtr_phy->lane) {
>> +                     int ret = xpsgtr_common_init(p);
>> +
>> +                     if (ret)
>> +                             return ret;
>> +             }
>> +
>> +             xpsgtr_init_ill(p);
>> +             xpsgtr_write_phy(p, L0_TM_E_ILL3, 0);
>> +             xpsgtr_clr_set_phy(p, L0_TM_MISC2, 0,
>> +                                L0_TM_MISC2_ILL_CAL_BYPASS);
>> +     }
>> +
>> +     /* Perform the ILL calibration procedure */
>> +     xpsgtr_phy_illcalib(gtr_dev, lanes, false);
>> +     xpsgtr_phy_illcalib(gtr_dev, lanes, true);
>> +
>> +     /* Disable PCIe ECO */
>> +     writel(1, gtr_dev->siou + SIOU_ECO_0);
>> +     return 0;
>> +}
>> +
>> +/* USB-specific initialization. */
>> +static void xpsgtr_phy_init_usb(struct xpsgtr_phy *gtr_phy)
>> +{
>> +     xpsgtr_clr_set_phy(gtr_phy, L0_TM_AUX_0, 0, 0x20);
>> +     xpsgtr_write_phy(gtr_phy, L0_TM_IQ_ILL8, 0xf3);
>> +     xpsgtr_write_phy(gtr_phy, L0_TM_E_ILL8, 0xf3);
>> +     xpsgtr_phy_set_ill(gtr_phy, 7, false);
>> +}
>> +
>>  /* Configure TX de-emphasis and margining for DP. */
>>  static void xpsgtr_phy_configure_dp(struct xpsgtr_phy *gtr_phy, unsigned int pre,
>>                                   unsigned int voltage)
>> @@ -710,6 +1121,10 @@ static int xpsgtr_phy_init(struct phy *phy)
>>               xpsgtr_phy_init_dp(gtr_phy);
>>               break;
>>
>> +     case ICM_PROTOCOL_PCIE:
>> +             ret = xpsgtr_phy_init_pcie(gtr_phy);
>> +             break;
>> +
>>       case ICM_PROTOCOL_SATA:
>>               xpsgtr_phy_init_sata(gtr_phy);
>>               break;
>> @@ -717,6 +1132,10 @@ static int xpsgtr_phy_init(struct phy *phy)
>>       case ICM_PROTOCOL_SGMII:
>>               xpsgtr_phy_init_sgmii(gtr_phy);
>>               break;
>> +
>> +     case ICM_PROTOCOL_USB:
>> +             xpsgtr_phy_init_usb(gtr_phy);
>> +             break;
>>       }
>>
>>  out:
>> --
>> 2.35.1.1320.gc452695387.dirty
> 

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* RE: [PATCH 4/8] phy: zynqmp: Calibrate ILL if necessary
From: Pandey, Radhey Shyam @ 2026-02-10 16:04 UTC (permalink / raw)
  To: Sean Anderson, Laurent Pinchart, Vinod Koul,
	linux-phy@lists.infradead.org
  Cc: Krzysztof Wilczyński, Lorenzo Pieralisi,
	linux-kernel@vger.kernel.org, Simek, Michal,
	linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org,
	Neil Armstrong, Rob Herring, Havalige, Thippeswamy,
	Manivannan Sadhasivam, Bjorn Helgaas
In-Reply-To: <20260203002128.935842-5-sean.anderson@linux.dev>

[Public]

> -----Original Message-----
> From: Sean Anderson <sean.anderson@linux.dev>
> Sent: Tuesday, February 3, 2026 5:51 AM
> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>; Vinod Koul
> <vkoul@kernel.org>; linux-phy@lists.infradead.org
> Cc: Krzysztof Wilczyński <kwilczynski@kernel.org>; Lorenzo Pieralisi
> <lpieralisi@kernel.org>; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; linux-kernel@vger.kernel.org; Simek, Michal
> <michal.simek@amd.com>; linux-arm-kernel@lists.infradead.org; linux-
> pci@vger.kernel.org; Neil Armstrong <neil.armstrong@linaro.org>; Rob Herring
> <robh@kernel.org>; Havalige, Thippeswamy <thippeswamy.havalige@amd.com>;
> Manivannan Sadhasivam <mani@kernel.org>; Bjorn Helgaas
> <bhelgaas@google.com>; Sean Anderson <sean.anderson@linux.dev>
> Subject: [PATCH 4/8] phy: zynqmp: Calibrate ILL if necessary
>
> init_serdes in psu_init_gpl is supposed to calibrate the ILL. However, this
> may fail if the reference clock is not running, such as if the clock needs
> to be configured on boot. To work around this, add support for ILL
> calibration in U-Boot. If the ILL is already calibrated (any non-zero
> value) we skip calibration.
>
> The algorithm is substantially the same as serdes_illcalib [1], but it has
> been updated for readability (and to remove all the "if (lane0_active)"
> conditions). Due to the amount of register fields, many of which are
> undocumented (especially the chicken bits), I have mostly used defines only
> for the register names. There are certainly areas where register writes are
> superfluous, but I have left these in order to minimize deviation from the
> procedure in serdes_illcalib.

Please consider splitting the patch in introducing calibrate ILL functions
and then subsequently using them.

How did you validate the changes? functional testing is one part
but I think better to match register configuration done in psu_init vs
this series? Have we tried running on multiple designs having
different GT lane and protocol combinations .

There are multiple magic numbers. Consider renaming it to
meaningful defines.

>
> [1] Example implementation; xpsgtr_phy_illcalib coresponds to
>     serdes_illcalib_pcie_gen1:
> https://source.denx.de/u-boot/u-boot/-/blob/v2026.01/board/xilinx/zynqmp/zynqmp-
> zcu208-revA/psu_init_gpl.c?ref_type=tags#L710
>
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
>
>  drivers/phy/xilinx/phy-zynqmp.c | 421 +++++++++++++++++++++++++++++++-
>  1 file changed, 420 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> index 152af1702bbd..854b0ea04648 100644
> --- a/drivers/phy/xilinx/phy-zynqmp.c
> +++ b/drivers/phy/xilinx/phy-zynqmp.c
> @@ -12,6 +12,7 @@
>   * PCIe should also work but that is experimental as of now.
>   */
>
> +#include <linux/bitfield.h>
>  #include <linux/clk.h>
>  #include <linux/debugfs.h>
>  #include <linux/delay.h>
> @@ -31,6 +32,7 @@
>   */
>
>  /* TX De-emphasis parameters */
> +#define L0_TX_ANA_TM_3                       0x000c
>  #define L0_TX_ANA_TM_18                      0x0048
>  #define L0_TX_ANA_TM_118             0x01d8
>  #define L0_TX_ANA_TM_118_FORCE_17_0  BIT(0)
> @@ -50,16 +52,49 @@
>  #define L0_TXPMD_TM_45_ENABLE_DP_POST2       BIT(5)
>
>  /* PCS control parameters */
> +#define L0_TM_ANA_BYP_4                      0x1010
> +#define L0_TM_ANA_BYP_7                      0x1018
>  #define L0_TM_DIG_6                  0x106c
> +#define L0_TM_DIG_22                 0x10ac
>  #define L0_TM_DIS_DESCRAMBLE_DECODER 0x0f
>  #define L0_TX_DIG_61                 0x00f4
>  #define L0_TM_DISABLE_SCRAMBLE_ENCODER       0x0f
> +#define L0_TM_AUX_0                  0x10cc
> +#define L0_TM_MISC2                  0x189c
> +#define L0_TM_MISC2_ILL_CAL_BYPASS   BIT(7)
> +#define L0_TM_IQ_ILL1                        0x18f8
> +#define L0_TM_IQ_ILL2                        0x18fc

#define L0_TM_IQ_ILL3                   0x1900

IQ_ILL(n) = 0x18f8 + (n - 1) * 4 ?

Similarly for others  (when applicable)?

> +#define L0_TM_ILL11                  0x198c
> +#define L0_TM_ILL12                  0x1990
> +#define L0_TM_E_ILL1                 0x1924
> +#define L0_TM_E_ILL2                 0x1928
> +#define L0_TM_IQ_ILL3                        0x1900
> +#define L0_TM_E_ILL3                 0x192c
> +#define L0_TM_IQ_ILL7                        0x1910
> +#define L0_TM_E_ILL7                 0x193c
> +#define L0_TM_ILL8                   0x1980
> +#define L0_TM_IQ_ILL8                        0x1914
> +#define L0_TM_IQ_ILL9                        0x1918
> +#define L0_TM_EQ0                    0x194c
> +#define L0_TM_EQ0_EQ_STG2_CTRL_BYP   BIT(5)
> +#define L0_TM_EQ1                    0x1950
> +#define L0_TM_EQ1_EQ_STG2_RL_PROG    GENMASK(1, 0)
> +#define L0_TM_EQ1_EQ_STG2_PREAMP_MODE_VAL    BIT(2)
> +#define L0_TM_E_ILL8                 0x1940
> +#define L0_TM_E_ILL9                 0x1944
> +#define L0_TM_ILL13                  0x1994
> +#define L0_TM_CDR5                   0x1c14
> +#define L0_TM_CDR5_FPHL_FSM_ACC_CYCLES       GENMASK(7, 5)
> +#define L0_TM_CDR5_FFL_PH0_INT_GAIN  GENMASK(4, 0)
> +#define L0_TM_CDR16                  0x1c40
>
>  /* PLL Test Mode register parameters */
> +#define L0_TM_PLL_DIG_33             0x2084
>  #define L0_TM_PLL_DIG_37             0x2094
>  #define L0_TM_COARSE_CODE_LIMIT              0x10
>
>  /* PLL SSC step size offsets */
> +#define L0_PLL_FBDIV_FRAC_3_MSB              0x2360
>  #define L0_PLL_SS_STEPS_0_LSB                0x2368
>  #define L0_PLL_SS_STEPS_1_MSB                0x236c
>  #define L0_PLL_SS_STEP_SIZE_0_LSB    0x2370
> @@ -69,6 +104,7 @@
>  #define L0_PLL_STATUS_READ_1         0x23e4
>
>  /* SSC step size parameters */
> +#define TM_FORCE_EN_FRAC             BIT(6)
>  #define STEP_SIZE_0_MASK             0xff
>  #define STEP_SIZE_1_MASK             0xff
>  #define STEP_SIZE_2_MASK             0xff
> @@ -76,6 +112,7 @@
>  #define STEP_SIZE_SHIFT                      8
>  #define FORCE_STEP_SIZE                      0x10
>  #define FORCE_STEPS                  0x20
> +#define TM_FORCE_EN                  BIT(7)
>  #define STEPS_0_MASK                 0xff
>  #define STEPS_1_MASK                 0x07
>
> @@ -84,6 +121,32 @@
>  #define L0_REF_CLK_LCL_SEL           BIT(7)
>  #define L0_REF_CLK_SEL_MASK          0x9f
>
> +/* Built-in self-test parameters */
> +#define L0_BIST_CTRL_1                       0x3004
> +#define L0_BIST_CTRL_2                       0x3008
> +#define L0_BIST_RUN_LEN_L            0x300c
> +#define L0_BIST_ERR_INJ_POINT_L              0x3010
> +#define L0_BIST_RUNLEN_ERR_INJ_H     0x3014
> +#define L0_BIST_IDLE_TIME            0x3018
> +#define L0_BIST_MARKER_L             0x301c
> +#define L0_BIST_IDLE_CHAR_L          0x3020
> +#define L0_BIST_MARKER_IDLE_H                0x3024
> +#define L0_BIST_LOW_PULSE_TIME               0x3028
> +#define L0_BIST_TOTAL_PULSE_TIME     0x302c
> +#define L0_BIST_TEST_PAT_1           0x3030
> +#define L0_BIST_TEST_PAT_2           0x3034
> +#define L0_BIST_TEST_PAT_3           0x3038
> +#define L0_BIST_TEST_PAT_4           0x303c
> +#define L0_BIST_TEST_PAT_MSBS                0x3040
> +#define L0_BIST_PKT_NUM                      0x3044
> +#define L0_BIST_FRM_IDLE_TIME                0x3048
> +#define L0_BIST_PKT_CTR_L            0x304c
> +#define L0_BIST_PKT_CTR_H            0x3050
> +#define L0_BIST_ERR_CTR_L            0x3054
> +#define L0_BIST_ERR_CTR_H            0x3058
> +#define L0_BIST_FILLER_OUT           0x3068
> +#define L0_BIST_FORCE_MK_RST         0x306c
> +
>  /* Calibration digital logic parameters */
>  #define L3_TM_CALIB_DIG19            0xec4c
>  #define L3_CALIB_DONE_STATUS         0xef14
> @@ -139,6 +202,9 @@ static const char *const xpsgtr_icm_str[] = {
>  #define TM_CMN_RST_SET                       0x2
>  #define TM_CMN_RST_MASK                      0x3
>
> +#define LPBK_CTRL0                   0x10038
> +#define LPBK_CTRL1                   0x1003c
> +
>  /* Bus width parameters */
>  #define TX_PROT_BUS_WIDTH            0x10040
>  #define RX_PROT_BUS_WIDTH            0x10044
> @@ -148,9 +214,13 @@ static const char *const xpsgtr_icm_str[] = {
>  #define PROT_BUS_WIDTH_SHIFT(n)              ((n) * 2)
>  #define PROT_BUS_WIDTH_MASK(n)               GENMASK((n) * 2 + 1, (n) * 2)
>
> +#define UPHY_SPARE0                  0X10098
> +
>  /* Number of GT lanes */
>  #define NUM_LANES                    4
>
> +#define SIOU_ECO_0                   0x1c
> +
>  /* SIOU SATA control register */
>  #define SATA_CONTROL_OFFSET          0x0100
>
> @@ -338,6 +408,33 @@ static void xpsgtr_restore_lane_regs(struct xpsgtr_dev
> *gtr_dev)
>                            gtr_dev->saved_regs[i]);
>  }
>
> +static inline void xpsgtr_write_lanes(struct xpsgtr_dev *gtr_dev,
> +                                  unsigned long *lanes, u32 reg, u32 value)
> +{
> +     unsigned long lane;
> +
> +     for_each_set_bit(lane, lanes, NUM_LANES) {
> +             void __iomem *addr = gtr_dev->serdes + lane * PHY_REG_OFFSET
> +                                  + reg;
> +
> +             writel(value, addr);
> +     }
> +}
> +
> +static inline void xpsgtr_clr_set_lanes(struct xpsgtr_dev *gtr_dev,
> +                                    unsigned long *lanes, u32 reg, u32 clr,
> +                                    u32 set)
> +{
> +     unsigned long lane;
> +
> +     for_each_set_bit(lane, lanes, NUM_LANES) {
> +             void __iomem *addr = gtr_dev->serdes + lane * PHY_REG_OFFSET
> +                                  + reg;
> +
> +             writel((readl(addr) & ~clr) | set, addr);
> +     }
> +}
> +
>  /*
>   * Hardware Configuration
>   */
> @@ -351,7 +448,7 @@ static int xpsgtr_wait_pll_lock(struct phy *phy)
>       u8 protocol = gtr_phy->protocol;
>       int ret;
>
> -     dev_dbg(gtr_dev->dev, "Waiting for PLL lock\n");
> +     dev_vdbg(gtr_dev->dev, "Waiting for PLL lock\n");

Unrelated change.

>
>       /*
>        * For DP and PCIe, only the instance 0 PLL is used. Switch to that phy
> @@ -520,6 +617,231 @@ static void xpsgtr_bypass_scrambler_8b10b(struct
> xpsgtr_phy *gtr_phy)
>       xpsgtr_write_phy(gtr_phy, L0_TX_DIG_61,
> L0_TM_DISABLE_SCRAMBLE_ENCODER);
>  }
>
> +/* Enable or disable loopback */
> +static void xpsgtr_phy_set_loopback(struct xpsgtr_phy *gtr_phy, bool enabled)
> +{
> +     struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
> +     u32 reg = gtr_phy->lane >= 2 ? LPBK_CTRL1 : LPBK_CTRL0;
> +     u32 shift = gtr_phy->lane & 1 ? 4 : 0;
> +
> +     xpsgtr_clr_set(gtr_dev, reg, 7 << shift, (u32)enabled << shift);
> +}
> +
> +static void xpsgtr_phy_set_ill(struct xpsgtr_phy *gtr_phy, u32 ill, bool gen2)
> +{
> +     u32 val = 4 + ill * 8;
> +
> +     if (gen2) {
> +             xpsgtr_write_phy(gtr_phy, L0_TM_E_ILL2, val & 0xff);
> +             xpsgtr_clr_set_phy(gtr_phy, L0_TM_ILL12, 0x0f,
> +                                1 << (val >> 8));
> +     } else {
> +             xpsgtr_write_phy(gtr_phy, L0_TM_E_ILL1, val & 0xff);
> +             xpsgtr_clr_set_phy(gtr_phy, L0_TM_ILL12, 0xf0,
> +                                (val >> 4) & 0x10);
> +     }
> +}
> +
> +static bool xpsgtr_ill_calibrated(struct xpsgtr_phy *gtr_phy)
> +{
> +     u32 ill1 = xpsgtr_read_phy(gtr_phy, L0_TM_E_ILL1);
> +     u32 ill2 = xpsgtr_read_phy(gtr_phy, L0_TM_E_ILL2);
> +     u32 ill12 = xpsgtr_read_phy(gtr_phy, L0_TM_ILL12);
> +
> +     dev_dbg(gtr_phy->dev->dev, "lane %u gen1 ILL was %u gen2 ILL was
> %u\n",
> +             gtr_phy->lane, ill1 / 8 + (ill12 & 0x10 ? 32 : 0),
> +             ill2 / 8 + (ill12 & 0x02 ? 32 : 0));
> +     return ill1 || ill2 || ill12;
> +}
> +
> +static void xpsgtr_init_ill(struct xpsgtr_phy *gtr_phy)
> +{
> +     struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
> +     struct clk *clk = gtr_dev->clk[gtr_phy->refclk];
> +     u32 ill123 = DIV_ROUND_CLOSEST(clk_get_rate(clk), 1000000);
> +
> +     xpsgtr_clr_set_phy(gtr_phy, L0_TM_MISC2, 0,
> L0_TM_MISC2_ILL_CAL_BYPASS);
> +     xpsgtr_write_phy(gtr_phy, L0_TM_IQ_ILL1, ill123);
> +     xpsgtr_write_phy(gtr_phy, L0_TM_IQ_ILL2, ill123);
> +     xpsgtr_write_phy(gtr_phy, L0_TM_IQ_ILL3, ill123);
> +     xpsgtr_write_phy(gtr_phy, L0_TM_IQ_ILL7, 0xf3);
> +     xpsgtr_write_phy(gtr_phy, L0_TM_E_ILL7, 0xf3);
> +     xpsgtr_write_phy(gtr_phy, L0_TM_ILL8, 0xff);
> +     xpsgtr_write_phy(gtr_phy, L0_TM_IQ_ILL8, 0xf3);
> +     xpsgtr_write_phy(gtr_phy, L0_TM_E_ILL8, 0xf3);
> +     xpsgtr_write_phy(gtr_phy, L0_TM_IQ_ILL9, 1);
> +     xpsgtr_write_phy(gtr_phy, L0_TM_E_ILL9, 1);
> +     xpsgtr_clr_set(gtr_dev, UPHY_SPARE0, BIT(5), 0);
> +}
> +
> +static void xpsgtr_phy_illcalib(struct xpsgtr_dev *gtr_dev,
> +                                  unsigned long *lanes, bool gen2)
> +{
> +     bool last_ok[NUM_LANES] = { 0 };
> +     int pass[NUM_LANES] = { 0 }, altpass[NUM_LANES] = { 0 };
> +     int best[NUM_LANES] = { 0 }, altbest[NUM_LANES] = { 0 };
> +     unsigned long lane;
> +     int i;
> +
> +     /* Initialize the BIST */
> +     xpsgtr_clr_set_lanes(gtr_dev, lanes, L0_BIST_CTRL_1, 0xe0, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_FILLER_OUT, 1);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_FORCE_MK_RST, 1);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_DIG_22, 0x20);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_CTRL_2, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_RUN_LEN_L, 0xf4);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_ERR_INJ_POINT_L, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_RUNLEN_ERR_INJ_H, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_IDLE_TIME, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_MARKER_L, 0xfb);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_IDLE_CHAR_L, 0xff);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_MARKER_IDLE_H, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_LOW_PULSE_TIME, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TOTAL_PULSE_TIME, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_1, 0x4a);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_2, 0x4a);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_3, 0x4a);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_4, 0x4a);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_MSBS, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_PKT_NUM, 0x14);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_FRM_IDLE_TIME, 2);
> +     xpsgtr_clr_set_lanes(gtr_dev, lanes, L0_BIST_CTRL_1, 0xe0, 0);
> +
> +     for (i = 0; i < 64; i++) {
> +             bool ok[NUM_LANES];
> +
> +             for_each_set_bit(lane, lanes, NUM_LANES)
> +                     xpsgtr_phy_set_ill(&gtr_dev->phys[lane], i, gen2);
> +
> +             /* Reset lanes */
> +             xpsgtr_clr_set_lanes(gtr_dev, lanes, L0_TM_ANA_BYP_7, 0x20,
> +                                  0x10);
> +             xpsgtr_write(gtr_dev, UPHY_SPARE0, 0x00);
> +             xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_ANA_BYP_4, 0x40);
> +             xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_PLL_DIG_33, 0x80);
> +             xpsgtr_write(gtr_dev, UPHY_SPARE0, 0x04);
> +             udelay(50);
> +             if (gen2)
> +                     xpsgtr_write(gtr_dev, UPHY_SPARE0, 0x0e);
> +             xpsgtr_write(gtr_dev, UPHY_SPARE0, 0x06);
> +             if (gen2) {
> +                     xpsgtr_write_lanes(gtr_dev, lanes, L0_TX_ANA_TM_3, 0x04);
> +                     xpsgtr_write(gtr_dev, UPHY_SPARE0, 0x07);
> +                     udelay(400);
> +                     xpsgtr_write_lanes(gtr_dev, lanes, L0_TX_ANA_TM_3, 0x0c);
> +                     udelay(15);
> +                     xpsgtr_write(gtr_dev, UPHY_SPARE0, 0x0f);
> +                     udelay(100);
> +             }
> +
> +             if (xpsgtr_wait_pll_lock(gtr_dev->phys[0].phy)) {
> +                     memset(last_ok, 0, sizeof(last_ok));
> +                     continue;
> +             }
> +
> +             udelay(50);
> +             xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_ANA_BYP_4, 0xc0);
> +             xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_ANA_BYP_4, 0x80);
> +             xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_PLL_DIG_33, 0xc0);
> +             udelay(50);
> +             xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_PLL_DIG_33, 0x80);
> +             udelay(50);
> +             xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_ANA_BYP_4, 0);
> +             xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_PLL_DIG_33, 0);
> +             udelay(500);
> +
> +             /* Do the BIST */
> +             for_each_set_bit(lane, lanes, NUM_LANES) {
> +                     struct xpsgtr_phy *gtr_phy = &gtr_dev->phys[lane];
> +                     u32 packets, errors;
> +
> +                     xpsgtr_phy_init_bus_width(gtr_phy, PROT_BUS_WIDTH_10);
> +                     xpsgtr_phy_set_loopback(gtr_phy, true);
> +                     xpsgtr_write_phy(gtr_phy, L0_TM_DIG_22, 0x20);
> +                     xpsgtr_clr_set_phy(gtr_phy, L0_BIST_CTRL_1, 0, 1);
> +
> +                     udelay(200);
> +                     xpsgtr_write_phy(gtr_phy, L0_BIST_CTRL_1, 0);
> +                     packets = xpsgtr_read_phy(gtr_phy, L0_BIST_PKT_CTR_L);
> +                     packets |= xpsgtr_read_phy(gtr_phy, L0_BIST_PKT_CTR_H)
> << 8;
> +                     errors = xpsgtr_read_phy(gtr_phy, L0_BIST_ERR_CTR_L);
> +                     errors |= xpsgtr_read_phy(gtr_phy, L0_BIST_ERR_CTR_H)
> << 8;
> +                     ok[lane] = packets && !errors;
> +
> +                     dev_dbg(gtr_dev->dev,
> +                             "lane %lu ILL %d packets %10u errors %10u\n",
> +                             lane, i, packets, errors);
> +             }
> +
> +             xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_CTRL_1, 0);
> +             xpsgtr_write(gtr_dev, UPHY_SPARE0, 0x00);
> +             xpsgtr_write(gtr_dev, UPHY_SPARE0, 0x02);
> +
> +             for_each_set_bit(lane, lanes, NUM_LANES) {
> +                     pass[lane] += ok[lane] && last_ok[lane];
> +                     if (pass[lane] < 4) {
> +                             if (!ok[lane] && i > 2) {
> +                                     if (altpass[lane] < pass[lane]) {
> +                                             altpass[lane] = pass[lane];
> +                                             altbest[lane] =
> +                                                     (i - 1) - (pass[lane] + 1) / 2;
> +                                     }
> +                                     pass[lane] = 0;
> +                             }
> +                     } else if (!best[lane] && (!ok[lane] || i == 63) &&
> +                                last_ok[lane]) {
> +                             best[lane] = (i - 1) - (pass[lane] + 1) / 2;
> +                     }
> +             }
> +
> +             memcpy(last_ok, ok, sizeof(ok));
> +     }
> +
> +     for_each_set_bit(lane, lanes, NUM_LANES) {
> +             dev_dbg(gtr_dev->dev, "lane %lu ILL best %d alt best %d\n",
> +                     lane, best[lane], altbest[lane]);
> +
> +             xpsgtr_phy_set_ill(&gtr_dev->phys[lane],
> +                                best[lane] ?: altbest[lane], gen2);
> +     }
> +
> +     /* Clean up */
> +     xpsgtr_clr_set_lanes(gtr_dev, lanes, L0_TM_ANA_BYP_7, 0x30, 0);
> +     xpsgtr_write(gtr_dev, UPHY_SPARE0, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_CTRL_1, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_CTRL_2, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_RUN_LEN_L, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_ERR_INJ_POINT_L, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_RUNLEN_ERR_INJ_H, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_IDLE_TIME, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_MARKER_L, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_IDLE_CHAR_L, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_MARKER_IDLE_H, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_LOW_PULSE_TIME, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TOTAL_PULSE_TIME, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_1, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_2, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_3, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_4, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_TEST_PAT_MSBS, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_PKT_NUM, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_FRM_IDLE_TIME, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_PKT_CTR_L, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_PKT_CTR_H, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_ERR_CTR_L, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_ERR_CTR_H, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_FILLER_OUT, 1);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_BIST_FORCE_MK_RST, 0);
> +     xpsgtr_write_lanes(gtr_dev, lanes, L0_TM_DIG_22, 0);
> +
> +     for_each_set_bit(lane, lanes, NUM_LANES) {
> +             struct xpsgtr_phy *gtr_phy = &gtr_dev->phys[lane];
> +
> +             xpsgtr_phy_init_bus_width(gtr_phy, PROT_BUS_WIDTH_20);
> +             xpsgtr_phy_set_loopback(gtr_phy, false);
> +     }
> +}
> +
>  static int xpsgtr_common_init(struct xpsgtr_phy *gtr_phy)
>  {
>       int ret;
> @@ -553,6 +875,37 @@ static void xpsgtr_phy_init_sata(struct xpsgtr_phy
> *gtr_phy)
>  {
>       struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
>
> +     if (!xpsgtr_ill_calibrated(gtr_phy)) {
> +             DECLARE_BITMAP(lanes, NUM_LANES) = { 0 };
> +
> +             xpsgtr_init_ill(gtr_phy);
> +             xpsgtr_write_phy(gtr_phy, L0_TM_E_ILL3, 100);
> +             xpsgtr_clr_set_phy(gtr_phy, L0_TM_ILL11, 0xf0, 0x20);
> +
> +             __set_bit(gtr_phy->lane, lanes);
> +             xpsgtr_phy_illcalib(gtr_dev, lanes, false);
> +             xpsgtr_phy_set_ill(gtr_phy, 7, true);
> +     }
> +
> +     /* Disable SSC */
> +     xpsgtr_write_phy(gtr_phy, L0_PLL_FBDIV_FRAC_3_MSB,
> TM_FORCE_EN_FRAC);
> +     xpsgtr_clr_set_phy(gtr_phy, L0_PLL_SS_STEP_SIZE_3_MSB, 0,
> TM_FORCE_EN);
> +
> +     /* Disable Tx deemphasis */
> +     xpsgtr_write_phy(gtr_phy, L0_TM_CDR5,
> +                      FIELD_PREP(L0_TM_CDR5_FPHL_FSM_ACC_CYCLES,
> 7) |
> +                      FIELD_PREP(L0_TM_CDR5_FFL_PH0_INT_GAIN, 6));
> +     xpsgtr_write_phy(gtr_phy, L0_TM_CDR16, 12);
> +
> +     /* Configure equalization */
> +     xpsgtr_write_phy(gtr_phy, L0_TX_ANA_TM_118,
> +                      L0_TX_ANA_TM_118_FORCE_17_0);
> +     xpsgtr_clr_set_phy(gtr_phy, L0_TM_EQ0, 0,
> L0_TM_EQ0_EQ_STG2_CTRL_BYP);
> +     xpsgtr_clr_set_phy(gtr_phy, L0_TM_EQ1,
> L0_TM_EQ1_EQ_STG2_RL_PROG,
> +                        FIELD_PREP(L0_TM_EQ1_EQ_STG2_RL_PROG, 2) |
> +                        L0_TM_EQ1_EQ_STG2_PREAMP_MODE_VAL);
> +     xpsgtr_write_phy(gtr_phy, L0_TX_ANA_TM_18, 2); /* -3.5 dB deemphasis */
> +
>       xpsgtr_bypass_scrambler_8b10b(gtr_phy);
>
>       writel(gtr_phy->lane, gtr_dev->siou + SATA_CONTROL_OFFSET);
> @@ -565,6 +918,64 @@ static void xpsgtr_phy_init_sgmii(struct xpsgtr_phy
> *gtr_phy)
>       xpsgtr_bypass_scrambler_8b10b(gtr_phy);
>  }
>
> +/* PCIe-specific initialization. */
> +static int xpsgtr_phy_init_pcie(struct xpsgtr_phy *gtr_phy)
> +{
> +     struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
> +     DECLARE_BITMAP(lanes, NUM_LANES) = { 0 };
> +     unsigned long lane;
> +     bool calibrated = false;
> +
> +     xpsgtr_clr_set_phy(gtr_phy, L0_TM_AUX_0, 0, 0x20);
> +
> +     for (lane = 0; lane < NUM_LANES; lane++) {
> +             struct xpsgtr_phy *gtr_phy = &gtr_dev->phys[lane];
> +
> +             if (gtr_phy->protocol != ICM_PROTOCOL_PCIE)
> +                     continue;
> +
> +             __set_bit(lane, lanes);
> +             calibrated = calibrated || xpsgtr_ill_calibrated(gtr_phy);
> +     }
> +
> +     if (calibrated)
> +             return 0;
> +
> +     /* Write default ILL config */
> +     for_each_set_bit(lane, lanes, NUM_LANES) {
> +             struct xpsgtr_phy *p = &gtr_dev->phys[lane];
> +
> +             if (lane != gtr_phy->lane) {
> +                     int ret = xpsgtr_common_init(p);
> +
> +                     if (ret)
> +                             return ret;
> +             }
> +
> +             xpsgtr_init_ill(p);
> +             xpsgtr_write_phy(p, L0_TM_E_ILL3, 0);
> +             xpsgtr_clr_set_phy(p, L0_TM_MISC2, 0,
> +                                L0_TM_MISC2_ILL_CAL_BYPASS);
> +     }
> +
> +     /* Perform the ILL calibration procedure */
> +     xpsgtr_phy_illcalib(gtr_dev, lanes, false);
> +     xpsgtr_phy_illcalib(gtr_dev, lanes, true);
> +
> +     /* Disable PCIe ECO */
> +     writel(1, gtr_dev->siou + SIOU_ECO_0);
> +     return 0;
> +}
> +
> +/* USB-specific initialization. */
> +static void xpsgtr_phy_init_usb(struct xpsgtr_phy *gtr_phy)
> +{
> +     xpsgtr_clr_set_phy(gtr_phy, L0_TM_AUX_0, 0, 0x20);
> +     xpsgtr_write_phy(gtr_phy, L0_TM_IQ_ILL8, 0xf3);
> +     xpsgtr_write_phy(gtr_phy, L0_TM_E_ILL8, 0xf3);
> +     xpsgtr_phy_set_ill(gtr_phy, 7, false);
> +}
> +
>  /* Configure TX de-emphasis and margining for DP. */
>  static void xpsgtr_phy_configure_dp(struct xpsgtr_phy *gtr_phy, unsigned int pre,
>                                   unsigned int voltage)
> @@ -710,6 +1121,10 @@ static int xpsgtr_phy_init(struct phy *phy)
>               xpsgtr_phy_init_dp(gtr_phy);
>               break;
>
> +     case ICM_PROTOCOL_PCIE:
> +             ret = xpsgtr_phy_init_pcie(gtr_phy);
> +             break;
> +
>       case ICM_PROTOCOL_SATA:
>               xpsgtr_phy_init_sata(gtr_phy);
>               break;
> @@ -717,6 +1132,10 @@ static int xpsgtr_phy_init(struct phy *phy)
>       case ICM_PROTOCOL_SGMII:
>               xpsgtr_phy_init_sgmii(gtr_phy);
>               break;
> +
> +     case ICM_PROTOCOL_USB:
> +             xpsgtr_phy_init_usb(gtr_phy);
> +             break;
>       }
>
>  out:
> --
> 2.35.1.1320.gc452695387.dirty


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* RE: [PATCH 3/8] phy: zynqmp: Refactor common phy initialization into a helper
From: Pandey, Radhey Shyam @ 2026-02-10 15:05 UTC (permalink / raw)
  To: Sean Anderson, Laurent Pinchart, Vinod Koul,
	linux-phy@lists.infradead.org
  Cc: Krzysztof Wilczyński, Lorenzo Pieralisi,
	linux-kernel@vger.kernel.org, Simek, Michal,
	linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org,
	Neil Armstrong, Rob Herring, Havalige, Thippeswamy,
	Manivannan Sadhasivam, Bjorn Helgaas
In-Reply-To: <20260203002128.935842-4-sean.anderson@linux.dev>

[Public]

> -----Original Message-----
> From: Sean Anderson <sean.anderson@linux.dev>
> Sent: Tuesday, February 3, 2026 5:51 AM
> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>; Vinod Koul
> <vkoul@kernel.org>; linux-phy@lists.infradead.org
> Cc: Krzysztof Wilczyński <kwilczynski@kernel.org>; Lorenzo Pieralisi
> <lpieralisi@kernel.org>; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; linux-kernel@vger.kernel.org; Simek, Michal
> <michal.simek@amd.com>; linux-arm-kernel@lists.infradead.org; linux-
> pci@vger.kernel.org; Neil Armstrong <neil.armstrong@linaro.org>; Rob Herring
> <robh@kernel.org>; Havalige, Thippeswamy <thippeswamy.havalige@amd.com>;
> Manivannan Sadhasivam <mani@kernel.org>; Bjorn Helgaas
> <bhelgaas@google.com>; Sean Anderson <sean.anderson@linux.dev>
> Subject: [PATCH 3/8] phy: zynqmp: Refactor common phy initialization into a helper
>
> All lanes undergoing ILL calibration must be initialized. Split off
> common phy initialization into a helper so that we can ensure all lanes
> are initialized before performing calibration.
>
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>

Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
Thanks!
> ---
>
>  drivers/phy/xilinx/phy-zynqmp.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> index 0d3c578d0f3f..152af1702bbd 100644
> --- a/drivers/phy/xilinx/phy-zynqmp.c
> +++ b/drivers/phy/xilinx/phy-zynqmp.c
> @@ -520,6 +520,21 @@ static void xpsgtr_bypass_scrambler_8b10b(struct
> xpsgtr_phy *gtr_phy)
>       xpsgtr_write_phy(gtr_phy, L0_TX_DIG_61,
> L0_TM_DISABLE_SCRAMBLE_ENCODER);
>  }
>
> +static int xpsgtr_common_init(struct xpsgtr_phy *gtr_phy)
> +{
> +     int ret;
> +
> +     /* Enable coarse code saturation limiting logic. */
> +     xpsgtr_write_phy(gtr_phy, L0_TM_PLL_DIG_37,
> L0_TM_COARSE_CODE_LIMIT);
> +
> +     ret = xpsgtr_configure_pll(gtr_phy);
> +     if (ret)
> +             return ret;
> +
> +     xpsgtr_lane_set_protocol(gtr_phy);
> +     return 0;
> +}
> +
>  /* DP-specific initialization. */
>  static void xpsgtr_phy_init_dp(struct xpsgtr_phy *gtr_phy)
>  {
> @@ -682,19 +697,14 @@ static int xpsgtr_phy_init(struct phy *phy)
>               gtr_dev->tx_term_fix = false;
>       }
>
> -     /* Enable coarse code saturation limiting logic. */
> -     xpsgtr_write_phy(gtr_phy, L0_TM_PLL_DIG_37,
> L0_TM_COARSE_CODE_LIMIT);
> -
>       /*
>        * Configure the PLL, the lane protocol, and perform protocol-specific
>        * initialization.
>        */
> -     ret = xpsgtr_configure_pll(gtr_phy);
> +     ret = xpsgtr_common_init(gtr_phy);
>       if (ret)
>               goto out;
>
> -     xpsgtr_lane_set_protocol(gtr_phy);
> -
>       switch (gtr_phy->protocol) {
>       case ICM_PROTOCOL_DP:
>               xpsgtr_phy_init_dp(gtr_phy);
> --
> 2.35.1.1320.gc452695387.dirty


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* RE: [PATCH 2/8] phy: zynqmp: Refactor bus width configuration into helper
From: Pandey, Radhey Shyam @ 2026-02-10 15:00 UTC (permalink / raw)
  To: Sean Anderson, Laurent Pinchart, Vinod Koul,
	linux-phy@lists.infradead.org
  Cc: Krzysztof Wilczyński, Lorenzo Pieralisi,
	linux-kernel@vger.kernel.org, Simek, Michal,
	linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org,
	Neil Armstrong, Rob Herring, Havalige, Thippeswamy,
	Manivannan Sadhasivam, Bjorn Helgaas
In-Reply-To: <20260203002128.935842-3-sean.anderson@linux.dev>

[Public]

> -----Original Message-----
> From: Sean Anderson <sean.anderson@linux.dev>
> Sent: Tuesday, February 3, 2026 5:51 AM
> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>; Vinod Koul
> <vkoul@kernel.org>; linux-phy@lists.infradead.org
> Cc: Krzysztof Wilczyński <kwilczynski@kernel.org>; Lorenzo Pieralisi
> <lpieralisi@kernel.org>; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; linux-kernel@vger.kernel.org; Simek, Michal
> <michal.simek@amd.com>; linux-arm-kernel@lists.infradead.org; linux-
> pci@vger.kernel.org; Neil Armstrong <neil.armstrong@linaro.org>; Rob Herring
> <robh@kernel.org>; Havalige, Thippeswamy <thippeswamy.havalige@amd.com>;
> Manivannan Sadhasivam <mani@kernel.org>; Bjorn Helgaas
> <bhelgaas@google.com>; Sean Anderson <sean.anderson@linux.dev>
> Subject: [PATCH 2/8] phy: zynqmp: Refactor bus width configuration into helper
>
> Split off the bus width configuration into a helper function for reuse.
>
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>

Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
Thanks!
> ---
>
>  drivers/phy/xilinx/phy-zynqmp.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> index fe6b4925d166..0d3c578d0f3f 100644
> --- a/drivers/phy/xilinx/phy-zynqmp.c
> +++ b/drivers/phy/xilinx/phy-zynqmp.c
> @@ -502,6 +502,17 @@ static void xpsgtr_lane_set_protocol(struct xpsgtr_phy
> *gtr_phy)
>       }
>  }
>
> +/* Set the bus width */
> +static void xpsgtr_phy_init_bus_width(struct xpsgtr_phy *gtr_phy, u32 width)
> +{
> +     struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
> +     u32 mask = PROT_BUS_WIDTH_MASK(gtr_phy->lane);
> +     u32 val = width << PROT_BUS_WIDTH_SHIFT(gtr_phy->lane);
> +
> +     xpsgtr_clr_set(gtr_dev, TX_PROT_BUS_WIDTH, mask, val);
> +     xpsgtr_clr_set(gtr_dev, RX_PROT_BUS_WIDTH, mask, val);
> +}
> +
>  /* Bypass (de)scrambler and 8b/10b decoder and encoder. */
>  static void xpsgtr_bypass_scrambler_8b10b(struct xpsgtr_phy *gtr_phy)
>  {
> @@ -535,14 +546,7 @@ static void xpsgtr_phy_init_sata(struct xpsgtr_phy
> *gtr_phy)
>  /* SGMII-specific initialization. */
>  static void xpsgtr_phy_init_sgmii(struct xpsgtr_phy *gtr_phy)
>  {
> -     struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
> -     u32 mask = PROT_BUS_WIDTH_MASK(gtr_phy->lane);
> -     u32 val = PROT_BUS_WIDTH_10 << PROT_BUS_WIDTH_SHIFT(gtr_phy-
> >lane);
> -
> -     /* Set SGMII protocol TX and RX bus width to 10 bits. */
> -     xpsgtr_clr_set(gtr_dev, TX_PROT_BUS_WIDTH, mask, val);
> -     xpsgtr_clr_set(gtr_dev, RX_PROT_BUS_WIDTH, mask, val);
> -
> +     xpsgtr_phy_init_bus_width(gtr_phy, PROT_BUS_WIDTH_10);
>       xpsgtr_bypass_scrambler_8b10b(gtr_phy);
>  }
>
> --
> 2.35.1.1320.gc452695387.dirty


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH v9 3/7] mux: add help text for MULTIPLEXER config option
From: Peter Rosin @ 2026-02-10 14:45 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Josua Mayer, Marc Kleine-Budde, Vincent Mailhol, Vinod Koul,
	Neil Armstrong, Aaro Koskinen, Andreas Kemnade, Kevin Hilman,
	Roger Quadros, Tony Lindgren, Janusz Krzysztofik, Vignesh R,
	Andi Shyti, Ulf Hansson, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, Wolfram Sang,
	Yazan Shhady, Jon Nettleton, Mikhail Anikin,
	linux-can@vger.kernel.org, linux-phy@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-i2c@vger.kernel.org, linux-mmc@vger.kernel.org,
	devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <CAMuHMdUEZZ+p+yaQDWhVCRNEQUygR2j=HpCp5LuzFU3E=qVNXg@mail.gmail.com>

Hi!

2026-02-10 at 08:50, Geert Uytterhoeven wrote:
> In the other thread, Josua pointed out that there are already several
> drivers that cannot be enabled if MULTIPLEXER is not selected by
> something else:
> 
> drivers/mux/Kconfig:
> 
>     menu "Multiplexer drivers"
>             depends on MULTIPLEXER
> 
>     config MUX_ADG792A
>             tristate "Analog Devices ADG792A/ADG792G Multiplexers"
>             depends on I2C
> 
>     config MUX_ADGS1408
>             tristate "Analog Devices ADGS1408/ADGS1409 Multiplexers"
>             depends on SPI
> 
>     config MUX_GPIO
>             tristate "GPIO-controlled Multiplexer"
>             depends on GPIOLIB || COMPILE_TEST
> 
>     config MUX_MMIO
>             tristate "MMIO/Regmap register bitfield-controlled Multiplexer"
>             depends on OF
> 
> While MUX_MMIO is selected by some/all(?) symbols that need it,
> the other three are not.  Are these three really dependent on another
> symbol selecting MULTIPLEXER?

I think you have the gist of it, yes. It's of course not ideal...

Cheers,
Peter

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH v9 3/7] mux: add help text for MULTIPLEXER config option
From: Peter Rosin @ 2026-02-10 14:42 UTC (permalink / raw)
  To: Josua Mayer, Marc Kleine-Budde, Vincent Mailhol, Vinod Koul,
	Neil Armstrong, Aaro Koskinen, Andreas Kemnade, Kevin Hilman,
	Roger Quadros, Tony Lindgren, Janusz Krzysztofik, Vignesh R,
	Andi Shyti, Ulf Hansson, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, Wolfram Sang
  Cc: Yazan Shhady, Jon Nettleton, Mikhail Anikin,
	linux-can@vger.kernel.org, linux-phy@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-i2c@vger.kernel.org, linux-mmc@vger.kernel.org,
	devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <de20903c-cae3-4f2e-8784-5b04421fdf4e@solid-run.com>

Hi!

2026-02-09 at 21:02, Josua Mayer wrote:
> Any suggestion how to name the new config symbol that can have
> a visible prompt?

MULTIPLEXER_CORE perhaps? Or maybe just MUX_CORE?

Cheers,
Peter

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH next] phy: renesas: rcar-gen3-usb2: Drop local devm_mux_state_get_optional()
From: Ulf Hansson @ 2026-02-10 13:34 UTC (permalink / raw)
  To: Vinod Koul, Geert Uytterhoeven
  Cc: Yoshihiro Shimoda, Neil Armstrong, Josua Mayer, Wolfram Sang,
	Tommaso Merciai, linux-phy, linux-mmc, linux-renesas-soc,
	linux-next, linux-kernel
In-Reply-To: <67c8c4f9bf9f09fd0c13daedef27b82ff389ddfb.1770720452.git.geert+renesas@glider.be>

On Tue, 10 Feb 2026 at 11:53, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
> Now the mux core provides devm_mux_state_get_optional():
>
>     drivers/phy/renesas/phy-rcar-gen3-usb2.c:944:1: error: static declaration of ‘devm_mux_state_get_optional’ follows non-static
>      declaration
>       944 | devm_mux_state_get_optional(struct device *dev, const char *mux_name)
>           | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>     In file included from drivers/phy/renesas/phy-rcar-gen3-usb2.c:20:
>     include/linux/mux/consumer.h:64:19: note: previous declaration of ‘devm_mux_state_get_optional’ with type ‘struct mux_state *(struct device *, const char *)’
>        64 | struct mux_state *devm_mux_state_get_optional(struct device *dev, const char *mux_name);
>           |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Fix this by dropping the temporary local wrapper.
>
> Fixes: ad314348ceb4fe1f ("mux: Add helper functions for getting optional and selected mux-state")
> Fixes: 8bb92fd7a0407792 ("phy: renesas: rcar-gen3-usb2: Use mux-state for phyrst management")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks Geert for helping out!

> ---
>   - ad314348ceb4fe1f is in mmc/next, and a PR has already been sent
>     https://lore.kernel.org/20260209133441.556464-1-ulf.hansson@linaro.org
>   - 8bb92fd7a0407792 is in phy/next

Vinod, do you want to pick up the $subject patch as a fix for 7.0-rc1
or do you prefer me to handle it?

Kind regards
Uffe

> ---
>  drivers/phy/renesas/phy-rcar-gen3-usb2.c | 10 ----------
>  1 file changed, 10 deletions(-)
>
> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> index cfc2a8d9028d58d0..65cbf330bd8fa10c 100644
> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> @@ -939,16 +939,6 @@ static int rcar_gen3_phy_usb2_vbus_regulator_register(struct rcar_gen3_chan *cha
>         return rcar_gen3_phy_usb2_vbus_regulator_get_exclusive_enable(channel, enable);
>  }
>
> -/* Temporary wrapper until the multiplexer subsystem supports optional muxes */
> -static inline struct mux_state *
> -devm_mux_state_get_optional(struct device *dev, const char *mux_name)
> -{
> -       if (!of_property_present(dev->of_node, "mux-states"))
> -               return NULL;
> -
> -       return devm_mux_state_get(dev, mux_name);
> -}
> -
>  static void rcar_gen3_phy_mux_state_deselect(void *data)
>  {
>         mux_state_deselect(data);
> --
> 2.43.0
>

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 1/3] phy: qcom: edp: Correct and clean up eDP/DP combo PHY configuration values
From: Dmitry Baryshkov @ 2026-02-10 12:33 UTC (permalink / raw)
  To: Yongxing Mou
  Cc: Vinod Koul, Neil Armstrong, linux-arm-msm, linux-phy,
	linux-kernel
In-Reply-To: <23e007dc-4145-4260-8810-1e98e533b1b5@oss.qualcomm.com>

On Tue, Feb 10, 2026 at 06:17:07PM +0800, Yongxing Mou wrote:
> 
> 
> On 2/9/2026 8:47 PM, Dmitry Baryshkov wrote:
> > On Mon, Feb 09, 2026 at 05:52:22PM +0800, Yongxing Mou wrote:
> > > 
> > > 
> > > On 2/7/2026 6:22 PM, Dmitry Baryshkov wrote:
> > > > On Thu, Feb 05, 2026 at 05:20:53PM +0800, Yongxing Mou wrote:
> > > > > According to the current HPG settings, most eDP/DP combo PHYs can reuse the
> > > > > same configuration values.
> > > > > DP mode:
> > > > > 	-sa8775p/sc7280/sc8280xp/x1e80100
> > > > > 	-glymur
> > > > > eDP mode(low vdiff):
> > > > > 	-glymur/sa8775p/sc8280xp/x1e80100
> > > > > 	-sc7280
> > > > > The current driver still keeps multiple versions of these tables and
> > > > > doesn't fully support every combo PHY mode. This patch removes the
> > > > 
> > > > See Documentation/process/submitting-patches.rst, "This patch".
> > > > 
> > > Sure. got it thanks, will remove "This patch".
> > > > > redundant configs and keeps only the sets we actually use, matching the
> > > > > platforms listed above.
> > > > 
> > > > Should it be combined with the 3rd patch? There you sort out all the
> > > > tables, it makes more sense to review all programming together.
> > > > 
> > > Hi, if this would help with your review work, I’m very happy to do it.
> > > My original intention was to do some cleanup in this patch—correct the table
> > > values so that more platforms can reuse the same table.
> > 
> > Fixed should come before cleanups (so that they can be backported to
> > stable kernels).
> > 
> Hi, do you mean the second LDO patch should come before the first patch?In

More like 3rd patch should be combined with the first and come before /
with LDO fixes.

> fact, maybe three patches should go into the stable branch. The LDO patch
> cannot be used on its own. On both sa8775p and hamoa, their eDP/DP modes do
> not match the tables they are currently using (hamoa should use the eDP
> table, while sa8775p should use the DP table). If only the LDO patch is
> applied, it may cause link training to fail.
> > >  From Konard’s comments, it seems that the HPG table may not be the most
> > > appropriate one either.
> > > Maybe shoud first determine what the correct table values should be.
> > 
> > Please follow the tables for each SKU separately (that's the 'fix'
> > part).
> > 
> Sure.
> 

-- 
With best wishes
Dmitry

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH next] phy: renesas: rcar-gen3-usb2: Drop local devm_mux_state_get_optional()
From: Tommaso Merciai @ 2026-02-10 11:26 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Yoshihiro Shimoda, Vinod Koul, Neil Armstrong, Josua Mayer,
	Ulf Hansson, Wolfram Sang, linux-phy, linux-mmc,
	linux-renesas-soc, linux-next, linux-kernel
In-Reply-To: <67c8c4f9bf9f09fd0c13daedef27b82ff389ddfb.1770720452.git.geert+renesas@glider.be>

Hi Geert,
Thanks for your patch!

On Tue, Feb 10, 2026 at 11:53:00AM +0100, Geert Uytterhoeven wrote:
> Now the mux core provides devm_mux_state_get_optional():
> 
>     drivers/phy/renesas/phy-rcar-gen3-usb2.c:944:1: error: static declaration of ‘devm_mux_state_get_optional’ follows non-static
>      declaration
>       944 | devm_mux_state_get_optional(struct device *dev, const char *mux_name)
> 	  | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>     In file included from drivers/phy/renesas/phy-rcar-gen3-usb2.c:20:
>     include/linux/mux/consumer.h:64:19: note: previous declaration of ‘devm_mux_state_get_optional’ with type ‘struct mux_state *(struct device *, const char *)’
>        64 | struct mux_state *devm_mux_state_get_optional(struct device *dev, const char *mux_name);
> 	  |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Fix this by dropping the temporary local wrapper.
> 
> Fixes: ad314348ceb4fe1f ("mux: Add helper functions for getting optional and selected mux-state")
> Fixes: 8bb92fd7a0407792 ("phy: renesas: rcar-gen3-usb2: Use mux-state for phyrst management")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>   - ad314348ceb4fe1f is in mmc/next, and a PR has already been sent
>     https://lore.kernel.org/20260209133441.556464-1-ulf.hansson@linaro.org
>   - 8bb92fd7a0407792 is in phy/next

Thanks for sharing :)

> ---
>  drivers/phy/renesas/phy-rcar-gen3-usb2.c | 10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> index cfc2a8d9028d58d0..65cbf330bd8fa10c 100644
> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> @@ -939,16 +939,6 @@ static int rcar_gen3_phy_usb2_vbus_regulator_register(struct rcar_gen3_chan *cha
>  	return rcar_gen3_phy_usb2_vbus_regulator_get_exclusive_enable(channel, enable);
>  }
>  
> -/* Temporary wrapper until the multiplexer subsystem supports optional muxes */
> -static inline struct mux_state *
> -devm_mux_state_get_optional(struct device *dev, const char *mux_name)
> -{
> -	if (!of_property_present(dev->of_node, "mux-states"))
> -		return NULL;
> -
> -	return devm_mux_state_get(dev, mux_name);
> -}
> -
>  static void rcar_gen3_phy_mux_state_deselect(void *data)
>  {
>  	mux_state_deselect(data);
> -- 
> 2.43.0

LGTM.
Tested on RZ/G3E.

Tested-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
Reviewed-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Kind Regards,
Tommaso

> 

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* [PATCH next] phy: renesas: rcar-gen3-usb2: Drop local devm_mux_state_get_optional()
From: Geert Uytterhoeven @ 2026-02-10 10:53 UTC (permalink / raw)
  To: Yoshihiro Shimoda, Vinod Koul, Neil Armstrong, Josua Mayer,
	Ulf Hansson, Wolfram Sang, Tommaso Merciai
  Cc: linux-phy, linux-mmc, linux-renesas-soc, linux-next, linux-kernel,
	Geert Uytterhoeven

Now the mux core provides devm_mux_state_get_optional():

    drivers/phy/renesas/phy-rcar-gen3-usb2.c:944:1: error: static declaration of ‘devm_mux_state_get_optional’ follows non-static
     declaration
      944 | devm_mux_state_get_optional(struct device *dev, const char *mux_name)
	  | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
    In file included from drivers/phy/renesas/phy-rcar-gen3-usb2.c:20:
    include/linux/mux/consumer.h:64:19: note: previous declaration of ‘devm_mux_state_get_optional’ with type ‘struct mux_state *(struct device *, const char *)’
       64 | struct mux_state *devm_mux_state_get_optional(struct device *dev, const char *mux_name);
	  |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~

Fix this by dropping the temporary local wrapper.

Fixes: ad314348ceb4fe1f ("mux: Add helper functions for getting optional and selected mux-state")
Fixes: 8bb92fd7a0407792 ("phy: renesas: rcar-gen3-usb2: Use mux-state for phyrst management")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
  - ad314348ceb4fe1f is in mmc/next, and a PR has already been sent
    https://lore.kernel.org/20260209133441.556464-1-ulf.hansson@linaro.org
  - 8bb92fd7a0407792 is in phy/next
---
 drivers/phy/renesas/phy-rcar-gen3-usb2.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
index cfc2a8d9028d58d0..65cbf330bd8fa10c 100644
--- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
+++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
@@ -939,16 +939,6 @@ static int rcar_gen3_phy_usb2_vbus_regulator_register(struct rcar_gen3_chan *cha
 	return rcar_gen3_phy_usb2_vbus_regulator_get_exclusive_enable(channel, enable);
 }
 
-/* Temporary wrapper until the multiplexer subsystem supports optional muxes */
-static inline struct mux_state *
-devm_mux_state_get_optional(struct device *dev, const char *mux_name)
-{
-	if (!of_property_present(dev->of_node, "mux-states"))
-		return NULL;
-
-	return devm_mux_state_get(dev, mux_name);
-}
-
 static void rcar_gen3_phy_mux_state_deselect(void *data)
 {
 	mux_state_deselect(data);
-- 
2.43.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply related

* Re: [PATCH 3/3] phy: qcom: edp: Add eDP phy mode switch support
From: Yongxing Mou @ 2026-02-10 10:29 UTC (permalink / raw)
  To: Konrad Dybcio, Vinod Koul, Neil Armstrong
  Cc: linux-arm-msm, linux-phy, linux-kernel
In-Reply-To: <af15838f-6a31-4017-ae5d-7b5e30f24aa4@oss.qualcomm.com>



On 2/6/2026 7:02 PM, Konrad Dybcio wrote:
> On 2/5/26 10:20 AM, Yongxing Mou wrote:
>> Add DP/eDP switch support by splitting the PHY swing/pre-emphasis tables
>> into separate DP and eDP configurations. This allows the driver to select
>> the correct table based on the is_edp flag.
>>
>> Add a dedicated table for the SC7280/glymur platforms, as they are not
>> compatible with the others.
>>
>> Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
>> ---
> 
> [...]
> 
>> +static const u8 edp_swing_hbr2_hbr3_v3[4][4] = {
>> +	{ 0x0b, 0x11, 0x16, 0x1b },
>> +	{ 0x0b, 0x19, 0x1f, 0xff },
>> +	{ 0x18, 0x1f, 0xff, 0xff },
>> +	{ 0x1f, 0xff, 0xff, 0xff }
>> +};
>> +
>> +static const u8 edp_pre_emp_hbr2_hbr3_v3[4][4] = {
>> +	{ 0x0c, 0x15, 0x19, 0x1e },
>> +	{ 0x09, 0x14, 0x19, 0xff },
>> +	{ 0x0f, 0x14, 0xff, 0xff },
>> +	{ 0x0d, 0xff, 0xff, 0xff }
>> +};
> 
> This is not quite in line with docs for kodiak. Now, if you have
> better/newer sequences than the HPG, I'm not objecting, but please
> cross-check
> 
> the rest of this patch I think looks fine
> 
> Konrad
Thanks for point that..
 >> +static const u8 edp_swing_hbr2_hbr3_v3[4][4] = {
 >> +	{ 0x0b, 0x11, 0x16, 0x1b },
here should be { 0x06, 0x11, 0x16, 0x1b }, arr[0][1]: 0x0b->0x06
does this looks fine? will check tables again..

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 2/3] phy: qcom: edp: Add per-version LDO configuration callback
From: Yongxing Mou @ 2026-02-10 10:21 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Vinod Koul, Neil Armstrong, linux-arm-msm, linux-phy,
	linux-kernel
In-Reply-To: <nxoukhntz3mn6m5qca2lk6wubhwlmxoldh6xpyxx63voseiw35@5rrwfryx6vww>



On 2/7/2026 6:17 PM, Dmitry Baryshkov wrote:
> On Thu, Feb 05, 2026 at 05:20:54PM +0800, Yongxing Mou wrote:
>> Introduce the com_ldo_config callback to support per‑PHY LDO
>> configuration.
> 
> Missing the why part. Is the driver programming incorrect values, or is
> it an optimisation? In the former case it needs Fixes, maybe cc:stable,
> maybe Reported-by, etc.
> 
Sure.. will add next version
>>
>> Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
>> ---
>>   drivers/phy/qualcomm/phy-qcom-edp.c | 86 ++++++++++++++++++++++++++++++++-----
>>   1 file changed, 76 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
>> index 10cbb7d9a8a0..388226dbad7f 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
>> @@ -81,6 +81,7 @@ struct phy_ver_ops {
>>   	int (*com_clk_fwd_cfg)(const struct qcom_edp *edp);
>>   	int (*com_configure_pll)(const struct qcom_edp *edp);
>>   	int (*com_configure_ssc)(const struct qcom_edp *edp);
>> +	int (*com_ldo_config)(const struct qcom_edp *edp);
>>   };
>>   
>>   struct qcom_edp_phy_cfg {
>> @@ -273,7 +274,7 @@ static int qcom_edp_set_voltages(struct qcom_edp *edp, const struct phy_configur
>>   	const struct qcom_edp_swing_pre_emph_cfg *cfg = edp->cfg->swing_pre_emph_cfg;
>>   	unsigned int v_level = 0;
>>   	unsigned int p_level = 0;
>> -	u8 ldo_config;
>> +	int ret;
>>   	u8 swing;
>>   	u8 emph;
>>   	int i;
>> @@ -300,13 +301,13 @@ static int qcom_edp_set_voltages(struct qcom_edp *edp, const struct phy_configur
>>   	if (swing == 0xff || emph == 0xff)
>>   		return -EINVAL;
>>   
>> -	ldo_config = edp->is_edp ? 0x0 : 0x1;
>> +	ret = edp->cfg->ver_ops->com_ldo_config(edp);
>> +	if (ret)
>> +		return ret;
>>   
>> -	writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
>>   	writel(swing, edp->tx0 + TXn_TX_DRV_LVL);
>>   	writel(emph, edp->tx0 + TXn_TX_EMP_POST1_LVL);
>>   
>> -	writel(ldo_config, edp->tx1 + TXn_LDO_CONFIG);
>>   	writel(swing, edp->tx1 + TXn_TX_DRV_LVL);
>>   	writel(emph, edp->tx1 + TXn_TX_EMP_POST1_LVL);
>>   
>> @@ -530,6 +531,52 @@ static int qcom_edp_com_configure_pll_v4(const struct qcom_edp *edp)
>>   	return 0;
>>   }
>>   
>> +static int qcom_edp_ldo_config_v3(const struct qcom_edp *edp)
>> +{
>> +	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
>> +	u32 ldo_config;
>> +
>> +	if (!edp->is_edp)
>> +		ldo_config = 0x0;
>> +	else if (dp_opts->link_rate <= 2700)
>> +		ldo_config = 0x81;
>> +	else
>> +		ldo_config = 0x41;
>> +
>> +	writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
>> +	writel(ldo_config, edp->tx1 + TXn_LDO_CONFIG);
>> +
>> +	return 0;
>> +}
>> +
>> +static int qcom_edp_ldo_config_v4(const struct qcom_edp *edp)
>> +{
>> +	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
>> +	u32 ldo_config;
>> +
>> +	if (!edp->is_edp)
>> +		ldo_config = 0x0;
>> +	else if (dp_opts->link_rate <= 2700)
>> +		ldo_config = 0xC1;
> 
> Lowercase hex
> 
Got it . will fix
>> +	else
>> +		ldo_config = 0x81;
>> +
>> +	writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
>> +	writel(ldo_config, edp->tx1 + TXn_LDO_CONFIG);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct phy_ver_ops qcom_edp_phy_ops_v3 = {
>> +	.com_power_on		= qcom_edp_phy_power_on_v4,
>> +	.com_resetsm_cntrl	= qcom_edp_phy_com_resetsm_cntrl_v4,
>> +	.com_bias_en_clkbuflr	= qcom_edp_com_bias_en_clkbuflr_v4,
>> +	.com_clk_fwd_cfg	= qcom_edp_com_clk_fwd_cfg_v4,
>> +	.com_configure_pll	= qcom_edp_com_configure_pll_v4,
>> +	.com_configure_ssc	= qcom_edp_com_configure_ssc_v4,
>> +	.com_ldo_config		= qcom_edp_ldo_config_v3,
>> +};
>> +
>>   static const struct phy_ver_ops qcom_edp_phy_ops_v4 = {
>>   	.com_power_on		= qcom_edp_phy_power_on_v4,
>>   	.com_resetsm_cntrl	= qcom_edp_phy_com_resetsm_cntrl_v4,
>> @@ -537,6 +584,7 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v4 = {
>>   	.com_clk_fwd_cfg	= qcom_edp_com_clk_fwd_cfg_v4,
>>   	.com_configure_pll	= qcom_edp_com_configure_pll_v4,
>>   	.com_configure_ssc	= qcom_edp_com_configure_ssc_v4,
>> +	.com_ldo_config		= qcom_edp_ldo_config_v4,
>>   };
>>   
>>   static const struct qcom_edp_phy_cfg sa8775p_dp_phy_cfg = {
>> @@ -550,7 +598,7 @@ static const struct qcom_edp_phy_cfg sa8775p_dp_phy_cfg = {
>>   static const struct qcom_edp_phy_cfg sc7280_dp_phy_cfg = {
>>   	.aux_cfg = edp_phy_aux_cfg_v4,
>>   	.vco_div_cfg = edp_phy_vco_div_cfg_v4,
>> -	.ver_ops = &qcom_edp_phy_ops_v4,
>> +	.ver_ops = &qcom_edp_phy_ops_v3,
> 
> This looks like an extra change. Is it intentional in this patch? If so,
> mention it in the commit message.
> 
Yes.. due to kodiak ldo func not compatable with other phys, will update 
the commit message.
>>   };
>>   
>>   static const struct qcom_edp_phy_cfg sc8280xp_dp_phy_cfg = {
> 


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 2/3] phy: qcom: edp: Add per-version LDO configuration callback
From: Yongxing Mou @ 2026-02-10 10:19 UTC (permalink / raw)
  To: Konrad Dybcio, Vinod Koul, Neil Armstrong, Dmitry Baryshkov
  Cc: linux-arm-msm, linux-phy, linux-kernel
In-Reply-To: <200b055b-7221-45b5-aea1-2462e07a3c1c@oss.qualcomm.com>



On 2/6/2026 6:52 PM, Konrad Dybcio wrote:
> On 2/5/26 10:20 AM, Yongxing Mou wrote:
>> Introduce the com_ldo_config callback to support per‑PHY LDO
>> configuration.
>>
>> Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
>> ---
> 
> [...]
> 
>> +static int qcom_edp_ldo_config_v4(const struct qcom_edp *edp)
>> +{
>> +	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
>> +	u32 ldo_config;
>> +
>> +	if (!edp->is_edp)
>> +		ldo_config = 0x0;
>> +	else if (dp_opts->link_rate <= 2700)
>> +		ldo_config = 0xC1;
> 
> lowercase hex, please
> 
Got it. will fix next version
>> +	else
>> +		ldo_config = 0x81;
>> +
>> +	writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
>> +	writel(ldo_config, edp->tx1 + TXn_LDO_CONFIG);
> 
> tx1 should be dp_ops->lanes ? 2 : ldo_config : 0x00, in all cases,
> I believe
> 
> Konrad
i check the HPG, yes , here need to consider dp_ops->lanes.. will fix it 
next patch.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 1/3] phy: qcom: edp: Correct and clean up eDP/DP combo PHY configuration values
From: Yongxing Mou @ 2026-02-10 10:17 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Vinod Koul, Neil Armstrong, linux-arm-msm, linux-phy,
	linux-kernel
In-Reply-To: <w3vmcgeh5bw7lfuyv7ziiw3oxpxtjyqmojddamjo5alz4nv7ld@ytgjwdhecfua>



On 2/9/2026 8:47 PM, Dmitry Baryshkov wrote:
> On Mon, Feb 09, 2026 at 05:52:22PM +0800, Yongxing Mou wrote:
>>
>>
>> On 2/7/2026 6:22 PM, Dmitry Baryshkov wrote:
>>> On Thu, Feb 05, 2026 at 05:20:53PM +0800, Yongxing Mou wrote:
>>>> According to the current HPG settings, most eDP/DP combo PHYs can reuse the
>>>> same configuration values.
>>>> DP mode:
>>>> 	-sa8775p/sc7280/sc8280xp/x1e80100
>>>> 	-glymur
>>>> eDP mode(low vdiff):
>>>> 	-glymur/sa8775p/sc8280xp/x1e80100
>>>> 	-sc7280
>>>> The current driver still keeps multiple versions of these tables and
>>>> doesn't fully support every combo PHY mode. This patch removes the
>>>
>>> See Documentation/process/submitting-patches.rst, "This patch".
>>>
>> Sure. got it thanks, will remove "This patch".
>>>> redundant configs and keeps only the sets we actually use, matching the
>>>> platforms listed above.
>>>
>>> Should it be combined with the 3rd patch? There you sort out all the
>>> tables, it makes more sense to review all programming together.
>>>
>> Hi, if this would help with your review work, I’m very happy to do it.
>> My original intention was to do some cleanup in this patch—correct the table
>> values so that more platforms can reuse the same table.
> 
> Fixed should come before cleanups (so that they can be backported to
> stable kernels).
> 
Hi, do you mean the second LDO patch should come before the first 
patch?In fact, maybe three patches should go into the stable branch. The 
LDO patch cannot be used on its own. On both sa8775p and hamoa, their 
eDP/DP modes do not match the tables they are currently using (hamoa 
should use the eDP table, while sa8775p should use the DP table). If 
only the LDO patch is applied, it may cause link training to fail.
>>  From Konard’s comments, it seems that the HPG table may not be the most
>> appropriate one either.
>> Maybe shoud first determine what the correct table values should be.
> 
> Please follow the tables for each SKU separately (that's the 'fix'
> part).
> 
Sure.


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH v9 3/7] mux: add help text for MULTIPLEXER config option
From: Geert Uytterhoeven @ 2026-02-10  7:50 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Josua Mayer, Marc Kleine-Budde, Vincent Mailhol, Vinod Koul,
	Neil Armstrong, Aaro Koskinen, Andreas Kemnade, Kevin Hilman,
	Roger Quadros, Tony Lindgren, Janusz Krzysztofik, Vignesh R,
	Andi Shyti, Ulf Hansson, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, Wolfram Sang,
	Yazan Shhady, Jon Nettleton, Mikhail Anikin,
	linux-can@vger.kernel.org, linux-phy@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-i2c@vger.kernel.org, linux-mmc@vger.kernel.org,
	devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <f4d8547d-3de3-0de5-da61-3de57d9af895@axentia.se>

Hi Peter,

On Mon, 9 Feb 2026 at 14:09, Peter Rosin <peda@axentia.se> wrote:
> 2026-02-09 at 13:07, Josua Mayer wrote:
> > On 09/02/2026 13:43, Peter Rosin wrote:
> >> 2026-02-09 at 12:31, Josua Mayer wrote:
> >>> On 09/02/2026 13:10, Peter Rosin wrote:
> >>>> 2026-02-08 at 16:38, Josua Mayer wrote:
> >>>>> Add prompt and help text for CONFIG_MULTIPLEXER to allow enabling this
> >>>>> option thorugh the kernel configuration without explicit "select" driver
> >>>>> dependencies.
> >>>>>
> >>>>> Select it by default when COMPILE_TEST is set for better coverage.
> >>>>>
> >>>>> Signed-off-by: Josua Mayer <josua@solid-run.com>
> >>>>> ---
> >>>>>    drivers/mux/Kconfig | 9 ++++++++-
> >>>>>    1 file changed, 8 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
> >>>>> index c68132e38138..4f7c6bb86fc6 100644
> >>>>> --- a/drivers/mux/Kconfig
> >>>>> +++ b/drivers/mux/Kconfig
> >>>>> @@ -4,7 +4,14 @@
> >>>>>    #
> >>>>>
> >>>>>    config MULTIPLEXER
> >>>>> - tristate
> >>>>> + tristate "Generic Multiplexer Support"
> >>>>> + default m if COMPILE_TEST
> >>>>> + help
> >>>>> +   This framework is designed to abstract multiplexer handling for
> >>>>> +   devices via various GPIO-, MMIO/Regmap or specific multiplexer
> >>>>> +   controller chips.
> >>>>> +
> >>>>> +   If unsure, say no.
> >>>>>
> >>>>>    menu "Multiplexer drivers"
> >>>>>           depends on MULTIPLEXER
> >>>>>
> >>>> I'm not comfortable with making MULTIPLEXER a visible symbol. It is meant to
> >>>> be selected when needed (and there are a dozen or so instances). The kbuild
> >>>> docs has this on the subject:
> >>>>
> >>>>    "In general use select only for non-visible symbols (no prompts
> >>>>     anywhere) and for symbols with no dependencies."
> >>> The patch description didn't make the decision logic clear,
> >>> and I plan to submit a standalone patch for this after v7.0-rc1.
> >>>
> >>> Basically existing drivers using mux core used "select" to enable it,
> >>> even though the core can function standalone with device-tree.
> >>>
> >>> Some of these users (phy-can-transceiver) function perfectly
> >>> perfectly fine without mux, and use it as an optional feature.
> >>>
> >>> Likely drivers only used "select" to avoid writing helper functions,
> >>> prompt, kconfig description and stubs - which this patch-set added.
> >>>
> >>> So I will argue that some existing users relying on "select" was wrong,
> >>> and that the mux framework is generally useful on its own.
> >> When I wrote the mux sub-system it was very much intentional and by
> >> design that drivers needing a mux should select MULTIPLEXER, and that
> >> MULTIPLEXER should not be a visible symbol.
> > Need is a strong word here, and doesn't address the optional case.
>
> "Need" was the correct verb up until you needed the subsystem to be
> optional. If you need the mux subsystem to be optional, you need to
> do it in a way that does not introduce headaches.

In the other thread, Josua pointed out that there are already several
drivers that cannot be enabled if MULTIPLEXER is not selected by
something else:

drivers/mux/Kconfig:

    menu "Multiplexer drivers"
            depends on MULTIPLEXER

    config MUX_ADG792A
            tristate "Analog Devices ADG792A/ADG792G Multiplexers"
            depends on I2C

    config MUX_ADGS1408
            tristate "Analog Devices ADGS1408/ADGS1409 Multiplexers"
            depends on SPI

    config MUX_GPIO
            tristate "GPIO-controlled Multiplexer"
            depends on GPIOLIB || COMPILE_TEST

    config MUX_MMIO
            tristate "MMIO/Regmap register bitfield-controlled Multiplexer"
            depends on OF

While MUX_MMIO is selected by some/all(?) symbols that need it,
the other three are not.  Are these three really dependent on another
symbol selecting MULTIPLEXER?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: phy: spacemit: add regulator support to K1 USB2 PHY
From: Chukun Pan @ 2026-02-10  7:00 UTC (permalink / raw)
  To: huang.ze
  Cc: amadeus, broonie, conor+dt, devicetree, dlan, krzk+dt, krzk,
	lgirdwood, linux-kernel, linux-phy, linux-riscv, neil.armstrong,
	robh, spacemit, vkoul
In-Reply-To: <DGB1P3K6I14J.1QCMDY84ZR7ZH@linux.dev>

Hi,

> > It appears Ze Huang described the vbus-supply property in
> > spacemit.k1-dwc3.yaml.
> >
> > So I should enable the regulator in the dwc3-generic-plat driver.
>
> No, we will enable regulator at port level.

Since dt-bindings (spacemit.k1-dwc3.yaml) already describes the
vbus-supply property, why can't it be implemented in the dwc3 driver?

> Previously, the VBUS regulator was defined as regulator-always-on
> in the device tree as a workaround.

I don't like this workaround because we can control the VBUS regulator
through the USB PHY driver or the USB Controller driver. These are widely
implemented in other similar drivers.

Regarding port level, few manufacturers use different VBUS supplies
for the ports coming out of the USB hub, such as the ArmSoM Sige1 board.
Most boards use the same VBUS for a single USB socket, so I don't quite
understand why we need to wait for port level implementation.

Thanks,
Chukun

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH v3 2/2] mmc: sdhci-of-arasan: add support on Axiado AX3000 SoC
From: Adrian Hunter @ 2026-02-10  6:38 UTC (permalink / raw)
  To: Tzu-Hao Wei, SriNavmani A, Prasad Bolisetty, Vinod Koul,
	Neil Armstrong, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Ulf Hansson, Michal Simek
  Cc: linux-phy, devicetree, linux-arm-kernel, linux-kernel, linux-mmc,
	openbmc
In-Reply-To: <20260206-axiado-ax3000-add-emmc-host-driver-support-v3-2-ef83b09325be@axiado.com>

On 06/02/2026 10:23, Tzu-Hao Wei wrote:
> From: SriNavmani A <srinavmani@axiado.com>
> 
> Axiado AX3000 SoC eMMC controller is based on Arasan eMMC 5.1 host
> controller IP.
> 
> Signed-off-by: SriNavmani A <srinavmani@axiado.com>
> Signed-off-by: Tzu-Hao Wei <twei@axiado.com>
> ---

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

>  drivers/mmc/host/sdhci-of-arasan.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index ab7f0ffe7b4f007a58eb0a26868b08b0b02b40f3..caf97238a58b487312d6cc2b7a868913ace60f22 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -1512,6 +1512,17 @@ static struct sdhci_arasan_of_data intel_keembay_sdio_data = {
>  	.clk_ops = &arasan_clk_ops,
>  };
>  
> +static const struct sdhci_pltfm_data sdhci_arasan_axiado_pdata = {
> +	.ops = &sdhci_arasan_ops,
> +	.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> +			SDHCI_QUIRK_BROKEN_CQE,
> +};
> +
> +static struct sdhci_arasan_of_data sdhci_arasan_axiado_data = {
> +	.pdata = &sdhci_arasan_axiado_pdata,
> +	.clk_ops = &arasan_clk_ops,
> +};
> +
>  static const struct of_device_id sdhci_arasan_of_match[] = {
>  	/* SoC-specific compatible strings w/ soc_ctl_map */
>  	{
> @@ -1538,6 +1549,10 @@ static const struct of_device_id sdhci_arasan_of_match[] = {
>  		.compatible = "intel,keembay-sdhci-5.1-sdio",
>  		.data = &intel_keembay_sdio_data,
>  	},
> +	{
> +		.compatible = "axiado,ax3000-sdhci-5.1-emmc",
> +		.data = &sdhci_arasan_axiado_data,
> +	},
>  	/* Generic compatible below here */
>  	{
>  		.compatible = "arasan,sdhci-8.9a",
> 


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 1/3] phy: qcom: edp: Correct and clean up eDP/DP combo PHY configuration values
From: Yongxing Mou @ 2026-02-10  6:13 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Konrad Dybcio, Vinod Koul, Neil Armstrong, linux-arm-msm,
	linux-phy, linux-kernel
In-Reply-To: <36ourp7swd36ptbal2gd7byrppownihbpkkxvocd44u7zddc4f@igo5h4yhbxs4>



On 2/9/2026 8:46 PM, Dmitry Baryshkov wrote:
> On Mon, Feb 09, 2026 at 05:00:03PM +0800, Yongxing Mou wrote:
>>
>>
>> On 2/6/2026 6:47 PM, Konrad Dybcio wrote:
>>> On 2/5/26 10:20 AM, Yongxing Mou wrote:
>>>> According to the current HPG settings, most eDP/DP combo PHYs can reuse the
>>>> same configuration values.
>>>
>>> Even across the various process nodes?
>>>
>> Emm,Currently, I have only checked the five platforms that already have eDP
>> PHY driver support enabled. The results are the same as stated in the commit
>> message: in DP mode and in eDP low‑vdiff mode, there are four platforms that
>> can reuse the same settings. The HPG I used was the one I found on IPCAT.
>> Regarding HRG, please move to next comment.
>>>> DP mode:
>>>> 	-sa8775p/sc7280/sc8280xp/x1e80100
>>>> 	-glymur
>>>> eDP mode(low vdiff):
>>>> 	-glymur/sa8775p/sc8280xp/x1e80100
>>>> 	-sc7280
>>>> The current driver still keeps multiple versions of these tables and
>>>> doesn't fully support every combo PHY mode. This patch removes the
>>>> redundant configs and keeps only the sets we actually use, matching the
>>>> platforms listed above.
>>>
>>> I see that e.g. eDP Low-Vdiff swing setting for RBR is:
>>>
>> Hi, do you mean emphasis settings, i don't see 0x11	0x12 in arr[0][1].
>>> 		hamoa	kodiak
>>> arr[0][1]	0x11	0x12
>>>
>>> It may be that this changed later during tuning but it's not reflected
>>> in the docs for kodiak
>> Emm, if that, where can i get the correct value for tables.. In this patch,
>> I’m indeed quite curious why the values before the modification differ from
>> those in the HPG. I’m not sure about the reason. The HPG I used was taken
>> directly from the current go/ipcat. Could you tell me where I can obtain the
>> final table that should be used?
> 
> Use the HPG which matches the chip generation / codename.
> 
Yes, the current modifications are made according to the chip's HPG.


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: phy: spacemit: add regulator support to K1 USB2 PHY
From: Ze Huang @ 2026-02-10  5:50 UTC (permalink / raw)
  To: Chukun Pan, krzk
  Cc: broonie, conor+dt, devicetree, dlan, huang.ze, krzk+dt, lgirdwood,
	linux-kernel, linux-phy, linux-riscv, neil.armstrong, robh,
	spacemit, vkoul
In-Reply-To: <20260209064016.1176056-1-amadeus@jmu.edu.cn>

On Mon Feb 9, 2026 at 2:40 PM CST, Chukun Pan wrote:
> Hi,
>
>> Plus, if this was a vbus regulator it would not be called "phy"...
>> Even more confusing.
>
> Sorry, I didn't consider that this naming is confusing. Other
> platforms, such as Rockchip's inno-usb2phy driver, use the
> "phy-supply" property. I continued with this bad naming.
>
>> Bindings are supposed to be complete regardless what driver does
>> (or does not).
>>
>> You must list all supplies with proper names.
>>
>> If this regulator was missing, then what else?
>
> (...)
>
>> Please start describing in the bindings your complete hardware, not
>> your drivers.
>
> It appears Ze Huang described the vbus-supply property in
> spacemit.k1-dwc3.yaml.
>

> So I should enable the regulator in the dwc3-generic-plat driver.

No, we will enable regulator at port level.

>
> Sorry for the noise.
>
> Thanks,
> Chukun


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH v2 1/4] dt-bindings: phy: axiado,ax3000-emmc-phy: add Axiado eMMC PHY
From: Rob Herring (Arm) @ 2026-02-10  1:30 UTC (permalink / raw)
  To: Tzu-Hao Wei
  Cc: openbmc, Conor Dooley, linux-arm-kernel, linux-kernel, Vinod Koul,
	Neil Armstrong, SriNavmani A, Prasad Bolisetty, linux-phy,
	devicetree, Krzysztof Kozlowski
In-Reply-To: <20260206-axiado-ax3000-add-emmc-phy-driver-support-v2-1-a2f59e97a92d@axiado.com>


On Fri, 06 Feb 2026 16:22:08 +0800, Tzu-Hao Wei wrote:
> From: SriNavmani A <srinavmani@axiado.com>
> 
> Axiado AX3000 SoC contains Arasan PHY which provides the interface to the
> HS200 eMMC host controller.
> 
> Signed-off-by: SriNavmani A <srinavmani@axiado.com>
> Signed-off-by: Tzu-Hao Wei <twei@axiado.com>
> ---
>  .../bindings/phy/axiado,ax3000-emmc-phy.yaml       | 37 ++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH v3 1/2] dt-bindings: mmc: arasan,sdhci: Add Axiado AX3000 SoC
From: Rob Herring (Arm) @ 2026-02-10  1:29 UTC (permalink / raw)
  To: Tzu-Hao Wei
  Cc: openbmc, linux-arm-kernel, Vinod Koul, Ulf Hansson, Michal Simek,
	Prasad Bolisetty, devicetree, linux-kernel, SriNavmani A,
	Conor Dooley, Neil Armstrong, linux-phy, linux-mmc,
	Krzysztof Kozlowski, Adrian Hunter
In-Reply-To: <20260206-axiado-ax3000-add-emmc-host-driver-support-v3-1-ef83b09325be@axiado.com>


On Fri, 06 Feb 2026 16:23:07 +0800, Tzu-Hao Wei wrote:
> From: SriNavmani A <srinavmani@axiado.com>
> 
> Add compatible strings for Axiado AX3000 SoC eMMC controller which
> is based on Arasan eMMC controller.
> 
> Signed-off-by: SriNavmani A <srinavmani@axiado.com>
> Signed-off-by: Tzu-Hao Wei <twei@axiado.com>
> ---
>  Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 1/4 v2] dt-bindings: serdes: s32g: Add NXP serdes subsystem
From: Rob Herring @ 2026-02-10  0:40 UTC (permalink / raw)
  To: Vincent Guittot
  Cc: vkoul, neil.armstrong, krzk+dt, conor+dt, ciprianmarian.costea,
	s32, p.zabel, linux, ghennadi.procopciuc, Ionut.Vicovan,
	linux-phy, devicetree, linux-kernel, linux-arm-kernel, netdev,
	horms, Frank.li
In-Reply-To: <20260203161917.1666696-2-vincent.guittot@linaro.org>

On Tue, Feb 03, 2026 at 05:19:14PM +0100, Vincent Guittot wrote:
> Describe the serdes subsystem available on the S32G platforms.
> 
> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> ---
>  .../bindings/phy/nxp,s32g-serdes.yaml         | 154 ++++++++++++++++++
>  1 file changed, 154 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/nxp,s32g-serdes.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/nxp,s32g-serdes.yaml b/Documentation/devicetree/bindings/phy/nxp,s32g-serdes.yaml
> new file mode 100644
> index 000000000000..fad34bee2a4f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/nxp,s32g-serdes.yaml
> @@ -0,0 +1,154 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/nxp,s32g-serdes.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP S32G2xxx/S32G3xxx SerDes PHY subsystem
> +
> +maintainers:
> +  - Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com>
> +
> +description: |
> +  The SerDes subsystem on S32G SoC Family includes two types of PHYs:
> +    - One PCIe PHY: Supports various PCIe operation modes
> +    - Two Ethernet Physical Coding Sublayer (XPCS) controllers
> +
> +  SerDes operation mode selects the enabled PHYs and speeds. Clock frequency
> +  must be adapted accordingly. Below table describes all possible operation
> +  modes.
> +
> +  Mode  PCIe	XPCS0		XPCS1		PHY clock	Description
> +                SGMII		SGMII		  (MHz)
> +  -------------------------------------------------------------------------
> +  0	Gen3	N/A		N/A		100		Single PCIe
> +  1	Gen2	1.25Gbps	N/A		100		PCIe/SGMII
> +  2	Gen2	N/A		1.25Gbps	100		PCIe/SGMII
> +  3	N/A	1.25Gbps	1.25Gbps	100,125		SGMII
> +  4	N/A	3.125/1.25Gbps	3.125/1.25Gbps 	125		SGMII
> +  5	Gen2	N/A	        3.125Gbps     	100		PCIe/SGMII

Mixed tabs and spaces. Drop the tabs.

What's not clear to me is do you have 2 or 4 lanes?

> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - nxp,s32g2-serdes
> +      - items:
> +          - const: nxp,s32g3-serdes
> +          - const: nxp,s32g2-serdes
> +
> +  reg:
> +    maxItems: 4
> +
> +  reg-names:
> +    items:
> +      - const: ss_pcie
> +      - const: pcie_phy
> +      - const: xpcs0
> +      - const: xpcs1
> +
> +  clocks:
> +    minItems: 4
> +    maxItems: 5
> +
> +  clock-names:
> +    items:
> +      - const: axi
> +      - const: aux
> +      - const: apb
> +      - const: ref
> +      - const: ext
> +    minItems: 4
> +
> +  resets:
> +    maxItems: 2
> +
> +  reset-names:
> +    items:
> +      - const: serdes
> +      - const: pcie
> +
> +  nxp,sys-mode:
> +    $ref: /schemas/types.yaml#/definitions/uint32

       maximum: 5

Though isn't this redundant with the child nodes? You could use the 
standard 'phy-mode' property in each child.

> +    description: |
> +      SerDes operational mode. See above table for possible values.
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +patternProperties:
> +  '^serdes[0,1]_lane@[0,1]$':

Do you need to support serdes0_lane@0 and serdes1_lane@0 (or similar 
with "@1")? That's illegal as you have 2 nodes with the same address.

> +    description:
> +      Describe a serdes lane.
> +    type: object
> +
> +    properties:
> +      compatible:
> +        enum:
> +          - nxp,s32g2-serdes-pcie-phy
> +          - nxp,s32g2-serdes-xpcs

Seems like phy-mode would be sufficient. Are these separate blocks from 
the parent?

> +
> +      reg:
> +        maxItems: 1

Just 'maximum: 1' instead.

> +
> +      '#phy-cells':
> +        const: 0
> +
> +    required:
> +      - reg
> +      - compatible
> +
> +    unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +  - nxp,sys-mode
> +  - '#address-cells'
> +  - '#size-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    bus {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +        serdes0: serdes@40480000 {
> +          compatible = "nxp,s32g3-serdes", "nxp,s32g2-serdes";
> +            reg = <0x0 0x40480000 0x0 0x108>,
> +                  <0x0 0x40483008 0x0 0x10>,
> +                  <0x0 0x40482000 0x0 0x800>,
> +                  <0x0 0x40482800 0x0 0x800>;
> +            reg-names = "ss_pcie", "pcie_phy", "xpcs0", "xpcs1";
> +            clocks = <&clks 1>,
> +                     <&clks 2>,
> +                     <&clks 3>,
> +                     <&clks 4>,
> +                     <&serdes_100_ext>;
> +            clock-names = "axi", "aux", "apb", "ref", "ext";
> +            resets = <&reset 9>,
> +                     <&reset 8>;
> +            reset-names = "serdes", "pcie";
> +            nxp,sys-mode = <1>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            phy_pcie0: serdes0_lane@0 {
> +              compatible = "nxp,s32g2-serdes-pcie-phy";
> +              #phy-cells = <0>;
> +              reg = <0>;
> +            };
> +            phy_xpcs0_0: serdes0_lane@1 {
> +              compatible = "nxp,s32g2-serdes-xpcs";
> +              reg = <0>;
> +            };
> +        };
> +    };
> -- 
> 2.43.0
> 

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 1/8] dt-bindings: pci: xilinx-nwl: Add resets
From: Rob Herring (Arm) @ 2026-02-10  0:24 UTC (permalink / raw)
  To: Sean Anderson
  Cc: Thippeswamy Havalige, Lorenzo Pieralisi, Michal Simek,
	Krzysztof Wilczyński, Laurent Pinchart, linux-phy,
	Vinod Koul, Neil Armstrong, Conor Dooley, devicetree, linux-pci,
	Bjorn Helgaas, Krzysztof Kozlowski, Radhey Shyam Pandey,
	Manivannan Sadhasivam, linux-arm-kernel, linux-kernel
In-Reply-To: <20260203002128.935842-2-sean.anderson@linux.dev>


On Mon, 02 Feb 2026 19:21:21 -0500, Sean Anderson wrote:
> Add resets so we can hold the bridge in reset while we perform phy
> calibration.
> 
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
> 
>  .../devicetree/bindings/pci/xlnx,nwl-pcie.yaml  | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH v9 3/7] mux: add help text for MULTIPLEXER config option
From: Josua Mayer @ 2026-02-09 20:02 UTC (permalink / raw)
  To: Peter Rosin, Marc Kleine-Budde, Vincent Mailhol, Vinod Koul,
	Neil Armstrong, Aaro Koskinen, Andreas Kemnade, Kevin Hilman,
	Roger Quadros, Tony Lindgren, Janusz Krzysztofik, Vignesh R,
	Andi Shyti, Ulf Hansson, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, Wolfram Sang
  Cc: Yazan Shhady, Jon Nettleton, Mikhail Anikin,
	linux-can@vger.kernel.org, linux-phy@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-i2c@vger.kernel.org, linux-mmc@vger.kernel.org,
	devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <f4d8547d-3de3-0de5-da61-3de57d9af895@axentia.se>

On 2/9/26 15:08, Peter Rosin wrote:
> Hi!
>
> 2026-02-09 at 13:07, Josua Mayer wrote:
>> On 09/02/2026 13:43, Peter Rosin wrote:
>>> Hi!
>>>
>>> 2026-02-09 at 12:31, Josua Mayer wrote:
>>>> Hi Peter,
>>>>
>>>> On 09/02/2026 13:10, Peter Rosin wrote:
>>>>> Hi!
>>>>>
>>>>> 2026-02-08 at 16:38, Josua Mayer wrote:
>>>>>> Add prompt and help text for CONFIG_MULTIPLEXER to allow enabling this
>>>>>> option thorugh the kernel configuration without explicit "select" driver
>>>>>> dependencies.
>>>>>>
>>>>>> Select it by default when COMPILE_TEST is set for better coverage.
>>>>>>
>>>>>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>>>>>> ---
>>>>>>     drivers/mux/Kconfig | 9 ++++++++-
>>>>>>     1 file changed, 8 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
>>>>>> index c68132e38138..4f7c6bb86fc6 100644
>>>>>> --- a/drivers/mux/Kconfig
>>>>>> +++ b/drivers/mux/Kconfig
>>>>>> @@ -4,7 +4,14 @@
>>>>>>     #
>>>>>>     
>>>>>>     config MULTIPLEXER
>>>>>> -	tristate
>>>>>> +	tristate "Generic Multiplexer Support"
>>>>>> +	default m if COMPILE_TEST
>>>>>> +	help
>>>>>> +	  This framework is designed to abstract multiplexer handling for
>>>>>> +	  devices via various GPIO-, MMIO/Regmap or specific multiplexer
>>>>>> +	  controller chips.
>>>>>> +
>>>>>> +	  If unsure, say no.
>>>>>>     
>>>>>>     menu "Multiplexer drivers"
>>>>>>     	depends on MULTIPLEXER
>>>>>>
>>>>> I'm not comfortable with making MULTIPLEXER a visible symbol. It is meant to
>>>>> be selected when needed (and there are a dozen or so instances). The kbuild
>>>>> docs has this on the subject:
>>>>>
>>>>> 	"In general use select only for non-visible symbols (no prompts
>>>>> 	 anywhere) and for symbols with no dependencies."
>>>> The patch description didn't make the decision logic clear,
>>>> and I plan to submit a standalone patch for this after v7.0-rc1.
>>>>
>>>> Basically existing drivers using mux core used "select" to enable it,
>>>> even though the core can function standalone with device-tree.
>>>>
>>>> Some of these users (phy-can-transceiver) function perfectly
>>>> perfectly fine without mux, and use it as an optional feature.
>>>>
>>>> Likely drivers only used "select" to avoid writing helper functions,
>>>> prompt, kconfig description and stubs - which this patch-set added.
>>>>
>>>> So I will argue that some existing users relying on "select" was wrong,
>>>> and that the mux framework is generally useful on its own.
>>> When I wrote the mux sub-system it was very much intentional and by
>>> design that drivers needing a mux should select MULTIPLEXER, and that
>>> MULTIPLEXER should not be a visible symbol.
>> Need is a strong word here, and doesn't address the optional case.
> "Need" was the correct verb up until you needed the subsystem to be
> optional. If you need the mux subsystem to be optional, you need to
> do it in a way that does not introduce headaches.
>
>>> You say that it could be useful to have it visible, which is all fine
>>> I suppose. But, you fail to address that quote from the kbuild docs.
>>> Why is it OK to have the preexisting drivers select a visible symbol,
>>> when the kbuild documentation states that it should not be done that
>>> way?
>> It might have been okay for a transitional period.
> What would be ok for a transitional period? Introducing potentially
> problematic kbuild dependencies? I'd rather not...
>
>> My original patch-set had already exploded due to the request to
>> introduce general purpose devm_*_optional_* helpers,
>> and the fact phy-can-transceiver already had a local version of the same.
>>
>> So perhaps if I will submit a patch-set changing to visible symbol,
>> I shall also change the few drivers that are now using "select"?
> I think it would be simpler to introduce some new visible symbol
> that triggers select MULTIPLEXER,

Considering the large number of existing users, I tend to agree here:

drivers/gpu/drm/bridge/Kconfig: select MULTIPLEXER
drivers/i2c/busses/Kconfig:     select MULTIPLEXER
drivers/i2c/muxes/Kconfig:      select MULTIPLEXER
drivers/iio/multiplexer/Kconfig:        select MULTIPLEXER
drivers/media/platform/Kconfig: select MULTIPLEXER
drivers/mtd/hyperbus/Kconfig:   select MULTIPLEXER
drivers/mtd/maps/Kconfig:       select MULTIPLEXER
drivers/net/mdio/Kconfig:       select MULTIPLEXER
drivers/phy/ti/Kconfig: select MULTIPLEXER
drivers/phy/ti/Kconfig: select MULTIPLEXER
drivers/phy/Kconfig:    select MULTIPLEXER
drivers/spi/Kconfig:    select MULTIPLEXER
drivers/spi/Kconfig:    select MULTIPLEXER
sound/soc/codecs/Kconfig:       select MULTIPLEXER

> making it perfectly fine to
> leave all the existing select MULTIPLEXER users as-is?
However I think each of them should be reviewed as to whether
their use of mux is mandatory or optional (phy-can-transceiver).

A "depends" relationship might be clearer long-term,
and perhaps all users should be converted eventually.

It is rather frustrating that the consideration to make mux framework
an optional dependency to drivers, rather than mandatory,
came after so many uses were already established.

Any suggestion how to name the new config symbol that can have
a visible prompt?

- regards
Josua Mayer
-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox