| Message ID | 20220607213543.4057620-1-bjorn.andersson@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | phy: qcom: Add SC8280XP UNI and COMBO USB phys | expand |
On 08/06/2022 00:35, Bjorn Andersson wrote: > The Qualcomm SC8280XP has two pairs of USB phys; a pair of combo phys and a > pair of uni phys. Introduce support for these. > > This is based ontop of Dmitry's refactoring of the QMP driver: > https://lore.kernel.org/all/20220602070909.1666068-1-dmitry.baryshkov@linaro.org/ > > A first version of this series was posted with only the UNI phy, this fixes a > few comments and add the combo phy as well. > > Bjorn Andersson (5): > dt-bindings: phy: qcom,qmp: Add compatible for SC8280XP USB phys > phy: qcom-qmp: Add USB3 5NM QMP UNI registers > phy: qcom-qmp: Add USB4 5NM QMP combo PHY registers I've noted, which symbols look close enough to be folded into existing namespaces. Could you please doublecheck my analysis and merge the tables? > phy: qcom-qmp: Add SC8280XP USB3 UNI phy > phy: qcom-qmp: Add sc8280xp USB/DP combo phys > > .../devicetree/bindings/phy/qcom,qmp-phy.yaml | 2 + > .../bindings/phy/qcom,qmp-usb3-dp-phy.yaml | 1 + > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 205 +++ > drivers/phy/qualcomm/phy-qcom-qmp-usb.c | 138 ++ > drivers/phy/qualcomm/phy-qcom-qmp.h | 13 + > .../phy/qualcomm/phy-qcom-usb3-5nm-qmp-uni.h | 617 +++++++ > .../qualcomm/phy-qcom-usb4-5nm-qmp-combo.h | 1547 +++++++++++++++++ > 7 files changed, 2523 insertions(+) > create mode 100644 drivers/phy/qualcomm/phy-qcom-usb3-5nm-qmp-uni.h > create mode 100644 drivers/phy/qualcomm/phy-qcom-usb4-5nm-qmp-combo.h >
On Tue 07 Jun 15:24 PDT 2022, Dmitry Baryshkov wrote: > On 08/06/2022 00:35, Bjorn Andersson wrote: > > Add all registers defines from qcom,usb4-5nm-qmp-combo.h of the msm-5.4 > > kernel. Offsets are adjusted to be relative to each sub-block, as we > > describe the individual pieces in the upstream kernel and "v5_5NM" are > > injected in the defines to not collide with existing constants. > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > --- > > > > Changes since v1: > > - New patch > > > > .../qualcomm/phy-qcom-usb4-5nm-qmp-combo.h | 1547 +++++++++++++++++ > > 1 file changed, 1547 insertions(+) > > create mode 100644 drivers/phy/qualcomm/phy-qcom-usb4-5nm-qmp-combo.h > > > > diff --git a/drivers/phy/qualcomm/phy-qcom-usb4-5nm-qmp-combo.h b/drivers/phy/qualcomm/phy-qcom-usb4-5nm-qmp-combo.h > > new file mode 100644 > > index 000000000000..7be8a50269ec > > --- /dev/null > > +++ b/drivers/phy/qualcomm/phy-qcom-usb4-5nm-qmp-combo.h > > @@ -0,0 +1,1547 @@ > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > +/* > > + * Copyright (c) 2021, The Linux Foundation. All rights reserved. > > + */ > > + > > +#ifndef PHY_QCOM_V5_5NM_QMP_COMBO_USB4_H > > +#define PHY_QCOM_V5_5NM_QMP_COMBO_USB4_H > > + > > +/* USB4-USB3-DP Combo PHY register offsets */ > > +/* Module: USB43DP_COM_USB43DP_COM_USB4_USB3_DP_COM */ > > +#define USB43DP_V5_5NM_COM_PHY_MODE_CTRL 0x00 > > +#define USB43DP_V5_5NM_COM_SW_RESET 0x04 > > +#define USB43DP_V5_5NM_COM_POWER_DOWN_CTRL 0x08 > > +#define USB43DP_V5_5NM_COM_SWI_CTRL 0x0c > > +#define USB43DP_V5_5NM_COM_TYPEC_CTRL 0x10 > > +#define USB43DP_V5_5NM_COM_TYPEC_PWRDN_CTRL 0x14 > > +#define USB43DP_V5_5NM_COM_DP_BIST_CFG_0 0x18 > > +#define USB43DP_V5_5NM_COM_RESET_OVRD_CTRL1 0x1c > > +#define USB43DP_V5_5NM_COM_RESET_OVRD_CTRL2 0x20 > > +#define USB43DP_V5_5NM_COM_DBG_CLK_MUX_CTRL 0x24 > > +#define USB43DP_V5_5NM_COM_TYPEC_STATUS 0x28 > > +#define USB43DP_V5_5NM_COM_PLACEHOLDER_STATUS 0x2c > > +#define USB43DP_V5_5NM_COM_REVISION_ID0 0x30 > > +#define USB43DP_V5_5NM_COM_REVISION_ID1 0x34 > > +#define USB43DP_V5_5NM_COM_REVISION_ID2 0x38 > > +#define USB43DP_V5_5NM_COM_REVISION_ID3 0x3c > > QPHY_V5_DP_COM_foo ? > My first version of the QMP patch used V5 defines and USB worked sometimes. So I hacked up a thing to dump the phy sequences of the downstream and upstream kernels, compared the magic numbers and then tried to fit suitable constants. But it obviously was a waste of time and I would have to make up a different naming scheme for the ones that doesn't match the existing constants - when we could just use the autogenerated files that exist in the downstream kernels. [..] > > +#define USB43DP_V5_5NM_QSERDES_TXA_DEBUG_BUS1 0xf0 > > +#define USB43DP_V5_5NM_QSERDES_TXA_DEBUG_BUS2 0xf4 > > +#define USB43DP_V5_5NM_QSERDES_TXA_DEBUG_BUS3 0xf8 > > +#define USB43DP_V5_5NM_QSERDES_TXA_TX_BKUP_RO_BUS 0xfc > > QSERDES_V5_20_TX_foo ? This looks compatible with the 4 registers that we > have in the header, but I can not verify the rest of registers > Exactly the point I was making in my reply to the other patch. Per the documentation this is version 5.0.0, but these register offsets happens to match the 5.20 defines that we have... > > + > > +/* Module: USB43DP_QSERDES_RXA_USB43DP_QSERDES_RXA_USB4_USB3_DP_QMP_RX */ [..] > > +#define USB43DP_V5_5NM_QSERDES_RXA_RX_BKUP_READ_BUS3_STATUS 0x3e8 And these, doesn't match either V5 or V5_20. [..] > > +#define USB43DP_V5_5NM_QSERDES_TXB_TX_BKUP_RO_BUS 0xfc > > What is the difference between _TXA_ and _TXB_ ? > Nothing, I just don't want us to mess around with these files if we can get them dumped from the register documentation. > > + [..] > > + > > +/* Module: USB3_PCS_MISC_USB3_PCS_MISC_USB3_PCS_MISC */ > > +#define USB3_V5_5NM_PCS_MISC_TYPEC_CTRL 0x00 > > +#define USB3_V5_5NM_PCS_MISC_TYPEC_PWRDN_CTRL 0x04 > > +#define USB3_V5_5NM_PCS_MISC_PCS_MISC_CONFIG1 0x08 > > +#define USB3_V5_5NM_PCS_MISC_CLAMP_ENABLE 0x0c > > +#define USB3_V5_5NM_PCS_MISC_TYPEC_STATUS 0x10 > > +#define USB3_V5_5NM_PCS_MISC_PLACEHOLDER_STATUS 0x14 > > QPHY_V4_PCS_MISC (or v5) > Perhaps, but then we're just making up those prefixes and hoping for the best. [..] > > +#define USB3_V5_5NM_PCS_EQ_CONFIG2 0x1e0 > > +#define USB3_V5_5NM_PCS_EQ_CONFIG3 0x1e4 > > +#define USB3_V5_5NM_PCS_EQ_CONFIG4 0x1E8 > > +#define USB3_V5_5NM_PCS_EQ_CONFIG5 0x1EC > > This looks like both QPHY_V4_PCS and QPHY_V5_PCS. Most probably we should > merge them together and add these defines. > Exactly, all these defines looks like defines we already have and if you pick the wrong one you end up with things not working - or in my case something that worked sometimes. > > + > > +/* Module: USB3_PCS_USB3_USB3_PCS_USB3_USB3_PCS_USB3 */ [..] > > +#define USB3_V5_5NM_PCS_USB3_RXTERMINATION_DLY_SEL 0x60 > > Again, QPHY_V5_PCS_USB w/o the 0x300 offset > Yeah, that extra region needs to be added to the binding and driver. Regards, Bjorn
On Wed, 8 Jun 2022 at 01:43, Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > On Tue 07 Jun 14:58 PDT 2022, Dmitry Baryshkov wrote: > > On 08/06/2022 00:35, Bjorn Andersson wrote: > [..] > > > +#define USB3_V5_5NM_UNI_QSERDES_COM_BIN_VCOCAL_CMP_CODE2_MODE0 0x1b0 > > > +#define USB3_V5_5NM_UNI_QSERDES_COM_BIN_VCOCAL_CMP_CODE1_MODE1 0x1b4 > > > +#define USB3_V5_5NM_UNI_QSERDES_COM_BIN_VCOCAL_CMP_CODE2_MODE1 0x1b8 > > > +#define USB3_V5_5NM_UNI_QSERDES_COM_BIN_VCOCAL_HSCLK_SEL 0x1bc > > > +#define USB3_V5_5NM_UNI_QSERDES_COM_RESERVED_1 0x1c0 > > > +#define USB3_V5_5NM_UNI_QSERDES_COM_MODE_OPERATION_STATUS 0x1c4 > > > > These defines look completely compatible with the existing ones in the > > QSERDES_V5_COM_ namespace. Please use them instead. > > > > Can you please confirm that all these constants are exactly the same as > the existing V5 entries? The only difference that I see is the phy-qcom-qmp.h defining QSERDES_V5_COM_CMN_MODE to 0x1a4, which should be 0x1a0. This is clearly a mistake on the upstream side (confirmed by the msm-5.10). Could you please send a patch for it? > > [..] > > > +/* Module: USB3_UNI_PCS_USB3_PCIE_USB3_UNI_PCS_USB3 */ > > > +#define USB3_V5_5NM_UNI_PCS_USB3_POWER_STATE_CONFIG1 0x00 > > > +#define USB3_V5_5NM_UNI_PCS_USB3_AUTONOMOUS_MODE_STATUS 0x04 > > > +#define USB3_V5_5NM_UNI_PCS_USB3_AUTONOMOUS_MODE_CTRL 0x08 > > > +#define USB3_V5_5NM_UNI_PCS_USB3_AUTONOMOUS_MODE_CTRL2 0x0c > > > +#define USB3_V5_5NM_UNI_PCS_USB3_LFPS_RXTERM_IRQ_SOURCE_STATUS 0x10 > > > +#define USB3_V5_5NM_UNI_PCS_USB3_LFPS_RXTERM_IRQ_CLEAR 0x14 > > > +#define USB3_V5_5NM_UNI_PCS_USB3_LFPS_DET_HIGH_COUNT_VAL 0x18 > > > +#define USB3_V5_5NM_UNI_PCS_USB3_LFPS_TX_ECSTART 0x1c > > > +#define USB3_V5_5NM_UNI_PCS_USB3_LFPS_PER_TIMER_VAL 0x20 > > > +#define USB3_V5_5NM_UNI_PCS_USB3_LFPS_TX_END_CNT_U3_START 0x24 > > > +#define USB3_V5_5NM_UNI_PCS_USB3_LFPS_CONFIG1 0x28 > > > +#define USB3_V5_5NM_UNI_PCS_USB3_RXEQTRAINING_LOCK_TIME 0x2c > > > +#define USB3_V5_5NM_UNI_PCS_USB3_RXEQTRAINING_WAIT_TIME 0x30 > > > +#define USB3_V5_5NM_UNI_PCS_USB3_RXEQTRAINING_CTLE_TIME 0x34 > > > +#define USB3_V5_5NM_UNI_PCS_USB3_RXEQTRAINING_WAIT_TIME_S2 0x38 > > > +#define USB3_V5_5NM_UNI_PCS_USB3_RXEQTRAINING_DFE_TIME_S2 0x3c > > > +#define USB3_V5_5NM_UNI_PCS_USB3_RCVR_DTCT_DLY_U3_L 0x40 > > > +#define USB3_V5_5NM_UNI_PCS_USB3_RCVR_DTCT_DLY_U3_H 0x44 > > > +#define USB3_V5_5NM_UNI_PCS_USB3_ARCVR_DTCT_EN_PERIOD 0x48 > > > +#define USB3_V5_5NM_UNI_PCS_USB3_ARCVR_DTCT_CM_DLY 0x4c > > > +#define USB3_V5_5NM_UNI_PCS_USB3_TXONESZEROS_RUN_LENGTH 0x50 > > > +#define USB3_V5_5NM_UNI_PCS_USB3_ALFPS_DEGLITCH_VAL 0x54 > > > +#define USB3_V5_5NM_UNI_PCS_USB3_SIGDET_STARTUP_TIMER_VAL 0x58 > > > +#define USB3_V5_5NM_UNI_PCS_USB3_TEST_CONTROL 0x5c > > > +#define USB3_V5_5NM_UNI_PCS_USB3_RXTERMINATION_DLY_SEL 0x60 > > > > These look like QPHY_V5_PCS_USB3, but without additional 0x300 offset. I'd > > suggest modifying qcom-qmp-phy-usb.c to allocate another register space for > > pcs_usb and updating QPHY_V4_PCS_USB3_foo / QPHY_V5_PCS_USB3_foo defines to > > remove this offset. > > > > Afterwards most if not all constants from this header can be merged into > > phy-qcom-qmp.h I do not think that it makes sense to split this header at > > this moment. The QSERDES_COM/_TX/_RX/_PCS defines are common to all PHY > > types. > > > > You might be right, but I spent considerable time debugging the combo > phy (which is version 5.0.0) and in the end it turned out that it's not > the same offsets. > > I really would prefer that we stop haphazardly try to fit things into > the phy-qcom-qmp.h with version numbers that we essentially make up > base, when Qualcomm dumps the register layout for each generation in > their downstream kernel. Well... Let's come up with a better versioning/naming scheme. But I don't think we should dump repeatable symbol headers, which are in reality common between different PHYs. This would make things harder to understand and harder to maintain.
On Wed, 8 Jun 2022 at 02:02, Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > On Tue 07 Jun 15:24 PDT 2022, Dmitry Baryshkov wrote: > > > On 08/06/2022 00:35, Bjorn Andersson wrote: > > > Add all registers defines from qcom,usb4-5nm-qmp-combo.h of the msm-5.4 > > > kernel. Offsets are adjusted to be relative to each sub-block, as we > > > describe the individual pieces in the upstream kernel and "v5_5NM" are > > > injected in the defines to not collide with existing constants. > > > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > > --- > > > > > > Changes since v1: > > > - New patch > > > > > > .../qualcomm/phy-qcom-usb4-5nm-qmp-combo.h | 1547 +++++++++++++++++ > > > 1 file changed, 1547 insertions(+) > > > create mode 100644 drivers/phy/qualcomm/phy-qcom-usb4-5nm-qmp-combo.h > > > > > > diff --git a/drivers/phy/qualcomm/phy-qcom-usb4-5nm-qmp-combo.h b/drivers/phy/qualcomm/phy-qcom-usb4-5nm-qmp-combo.h > > > new file mode 100644 > > > index 000000000000..7be8a50269ec > > > --- /dev/null > > > +++ b/drivers/phy/qualcomm/phy-qcom-usb4-5nm-qmp-combo.h > > > @@ -0,0 +1,1547 @@ > > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > > +/* > > > + * Copyright (c) 2021, The Linux Foundation. All rights reserved. > > > + */ > > > + > > > +#ifndef PHY_QCOM_V5_5NM_QMP_COMBO_USB4_H > > > +#define PHY_QCOM_V5_5NM_QMP_COMBO_USB4_H > > > + > > > +/* USB4-USB3-DP Combo PHY register offsets */ > > > +/* Module: USB43DP_COM_USB43DP_COM_USB4_USB3_DP_COM */ > > > +#define USB43DP_V5_5NM_COM_PHY_MODE_CTRL 0x00 > > > +#define USB43DP_V5_5NM_COM_SW_RESET 0x04 > > > +#define USB43DP_V5_5NM_COM_POWER_DOWN_CTRL 0x08 > > > +#define USB43DP_V5_5NM_COM_SWI_CTRL 0x0c > > > +#define USB43DP_V5_5NM_COM_TYPEC_CTRL 0x10 > > > +#define USB43DP_V5_5NM_COM_TYPEC_PWRDN_CTRL 0x14 > > > +#define USB43DP_V5_5NM_COM_DP_BIST_CFG_0 0x18 > > > +#define USB43DP_V5_5NM_COM_RESET_OVRD_CTRL1 0x1c > > > +#define USB43DP_V5_5NM_COM_RESET_OVRD_CTRL2 0x20 > > > +#define USB43DP_V5_5NM_COM_DBG_CLK_MUX_CTRL 0x24 > > > +#define USB43DP_V5_5NM_COM_TYPEC_STATUS 0x28 > > > +#define USB43DP_V5_5NM_COM_PLACEHOLDER_STATUS 0x2c > > > +#define USB43DP_V5_5NM_COM_REVISION_ID0 0x30 > > > +#define USB43DP_V5_5NM_COM_REVISION_ID1 0x34 > > > +#define USB43DP_V5_5NM_COM_REVISION_ID2 0x38 > > > +#define USB43DP_V5_5NM_COM_REVISION_ID3 0x3c > > > > QPHY_V5_DP_COM_foo ? > > > > My first version of the QMP patch used V5 defines and USB worked > sometimes. So I hacked up a thing to dump the phy sequences of the > downstream and upstream kernels, compared the magic numbers and then > tried to fit suitable constants. > > But it obviously was a waste of time and I would have to make up a > different naming scheme for the ones that doesn't match the existing > constants - when we could just use the autogenerated files that exist in > the downstream kernels. I decided that I should write more about it. My main issue with using downstream tables is that we end up with tons of repetitive defines. Each chip generation would bring 2-4 sets of tables, wouldn't it? This can easily become an unsupported beast. I'd propose to follow the opposite path. Let's split the existing tables on a per-generation, per-region basis. Yes, we'd end up with tens of the header files. However then when new generation arrives, we can split corresponding header files on a region-by-region basis, and compare each region with existing tables. If the region matches, use it. If it does not, create a new header. Yes, I can do this for the existing header as a continuation of the QMP split saga, if everybody agrees that this is a good path. You can ask, why do I suggest such a scheme? Because it looks like the lowest common scheme. If we check downstream, we have USB/USB+DP with huge autogenerated tables. Then comes UFS, which mostly follows naming of the phy-qcom-qmp.h. And the last one is a PCIe. I do not know about the sc8280xp, but for the rest of the platforms we do not have register names at all. When I was porting the SM8450 PCIe PHY support, I had to guess the correct generation beforehand. With just 5 QSERDES_COM_ namespaces, guessing is easy. If we had separate namespaces for the UFS and for several USB PHY instances, guessing would be next to impossible. And then creating a correct table would also be impossible. Well, as long as we do not accept tables without register names. Thus I think we should resort to using a single naming scheme rather than following downstream here. If you dislike existing QSERDES_Vn/QPHY_Vn, let's come up with something more sensible. -- With best wishes Dmitry
On 08-06-22, 02:52, Dmitry Baryshkov wrote: > On Wed, 8 Jun 2022 at 02:02, Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > > > On Tue 07 Jun 15:24 PDT 2022, Dmitry Baryshkov wrote: > > > > > On 08/06/2022 00:35, Bjorn Andersson wrote: > > > > Add all registers defines from qcom,usb4-5nm-qmp-combo.h of the msm-5.4 > > > > kernel. Offsets are adjusted to be relative to each sub-block, as we > > > > describe the individual pieces in the upstream kernel and "v5_5NM" are > > > > injected in the defines to not collide with existing constants. > > > > > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > > > --- > > > > > > > > Changes since v1: > > > > - New patch > > > > > > > > .../qualcomm/phy-qcom-usb4-5nm-qmp-combo.h | 1547 +++++++++++++++++ > > > > 1 file changed, 1547 insertions(+) > > > > create mode 100644 drivers/phy/qualcomm/phy-qcom-usb4-5nm-qmp-combo.h > > > > > > > > diff --git a/drivers/phy/qualcomm/phy-qcom-usb4-5nm-qmp-combo.h b/drivers/phy/qualcomm/phy-qcom-usb4-5nm-qmp-combo.h > > > > new file mode 100644 > > > > index 000000000000..7be8a50269ec > > > > --- /dev/null > > > > +++ b/drivers/phy/qualcomm/phy-qcom-usb4-5nm-qmp-combo.h > > > > @@ -0,0 +1,1547 @@ > > > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > > > +/* > > > > + * Copyright (c) 2021, The Linux Foundation. All rights reserved. > > > > + */ > > > > + > > > > +#ifndef PHY_QCOM_V5_5NM_QMP_COMBO_USB4_H > > > > +#define PHY_QCOM_V5_5NM_QMP_COMBO_USB4_H > > > > + > > > > +/* USB4-USB3-DP Combo PHY register offsets */ > > > > +/* Module: USB43DP_COM_USB43DP_COM_USB4_USB3_DP_COM */ > > > > +#define USB43DP_V5_5NM_COM_PHY_MODE_CTRL 0x00 > > > > +#define USB43DP_V5_5NM_COM_SW_RESET 0x04 > > > > +#define USB43DP_V5_5NM_COM_POWER_DOWN_CTRL 0x08 > > > > +#define USB43DP_V5_5NM_COM_SWI_CTRL 0x0c > > > > +#define USB43DP_V5_5NM_COM_TYPEC_CTRL 0x10 > > > > +#define USB43DP_V5_5NM_COM_TYPEC_PWRDN_CTRL 0x14 > > > > +#define USB43DP_V5_5NM_COM_DP_BIST_CFG_0 0x18 > > > > +#define USB43DP_V5_5NM_COM_RESET_OVRD_CTRL1 0x1c > > > > +#define USB43DP_V5_5NM_COM_RESET_OVRD_CTRL2 0x20 > > > > +#define USB43DP_V5_5NM_COM_DBG_CLK_MUX_CTRL 0x24 > > > > +#define USB43DP_V5_5NM_COM_TYPEC_STATUS 0x28 > > > > +#define USB43DP_V5_5NM_COM_PLACEHOLDER_STATUS 0x2c > > > > +#define USB43DP_V5_5NM_COM_REVISION_ID0 0x30 > > > > +#define USB43DP_V5_5NM_COM_REVISION_ID1 0x34 > > > > +#define USB43DP_V5_5NM_COM_REVISION_ID2 0x38 > > > > +#define USB43DP_V5_5NM_COM_REVISION_ID3 0x3c > > > > > > QPHY_V5_DP_COM_foo ? > > > > > > > My first version of the QMP patch used V5 defines and USB worked > > sometimes. So I hacked up a thing to dump the phy sequences of the > > downstream and upstream kernels, compared the magic numbers and then > > tried to fit suitable constants. > > > > But it obviously was a waste of time and I would have to make up a > > different naming scheme for the ones that doesn't match the existing > > constants - when we could just use the autogenerated files that exist in > > the downstream kernels. > > I decided that I should write more about it. My main issue with using > downstream tables is that we end up with tons of repetitive defines. > Each chip generation would bring 2-4 sets of tables, wouldn't it? This > can easily become an unsupported beast. > I'd propose to follow the opposite path. Let's split the existing > tables on a per-generation, per-region basis. Yes, we'd end up with > tens of the header files. However then when new generation arrives, we > can split corresponding header files on a region-by-region basis, and > compare each region with existing tables. If the region matches, use > it. If it does not, create a new header. Yes, I can do this for the > existing header as a continuation of the QMP split saga, if everybody > agrees that this is a good path. > > You can ask, why do I suggest such a scheme? Because it looks like the > lowest common scheme. If we check downstream, we have USB/USB+DP with > huge autogenerated tables. Then comes UFS, which mostly follows naming > of the phy-qcom-qmp.h. > > And the last one is a PCIe. I do not know about the sc8280xp, but for > the rest of the platforms we do not have register names at all. When I > was porting the SM8450 PCIe PHY support, I had to guess the correct > generation beforehand. With just 5 QSERDES_COM_ namespaces, guessing > is easy. If we had separate namespaces for the UFS and for several > USB PHY instances, guessing would be next to impossible. And then > creating a correct table would also be impossible. Well, as long as we > do not accept tables without register names. > > Thus I think we should resort to using a single naming scheme rather > than following downstream here. If you dislike existing > QSERDES_Vn/QPHY_Vn, let's come up with something more sensible. Bjorn has a valid point that we should not tinker with downstream auto-generated headers and use as is. But Dmitry also has a good argument of this becoming unmanageable mess. So which of the lesser devils should we deal with... Former is easy to do, latter involves a bit of work for kernel developers... TBH My personal taste would be latter as that keeps the code clean... We have seen the versions are getting managed terribly downstream.. Maybe splitting the headers up is a good idea in that direction... Thought...?