[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Yp/VCg3UznbLKLuk@ripper>
Date: Tue, 7 Jun 2022 15:45:30 -0700
From: Bjorn Andersson <bjorn.andersson@...aro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Kishon Vijay Abraham I <kishon@...com>,
Vinod Koul <vkoul@...nel.org>,
Manu Gautam <mgautam@...eaurora.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
linux-arm-msm@...r.kernel.org, linux-phy@...ts.infradead.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/5] phy: qcom-qmp: Add USB3 5NM QMP UNI registers
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?
[..]
> > +/* 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.
Regards,
Bjorn
Powered by blists - more mailing lists