| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 7 Jun 2022 16:04:51 -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 3/5] phy: qcom-qmp: Add USB4 5NM QMP combo PHY
registers
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@...aro.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
Powered by blists - more mailing lists