On Wed, 8 Jun 2022 at 02:02, Bjorn Andersson <bjorn.andersson@xxxxxxxxxx> 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@xxxxxxxxxx> > > > --- > > > > > > 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
