| 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: Wed, 8 Jun 2022 21:08:56 +0530
From: Vinod Koul <vkoul@...nel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Bjorn Andersson <bjorn.andersson@...aro.org>,
Kishon Vijay Abraham I <kishon@...com>,
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 08-06-22, 02:52, Dmitry Baryshkov wrote:
> On Wed, 8 Jun 2022 at 02:02, Bjorn Andersson <bjorn.andersson@...aro.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@...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.
>
> 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...?
--
~Vinod
Powered by blists - more mailing lists