| 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
| ||
|
Message-ID: <CAA8EJpobfKD=WjapG7Vx0mMFrGYSbadsrsakWs65Lpt43z7wVg@mail.gmail.com>
Date: Thu, 27 Feb 2025 03:55:36 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Wesley Cheng <quic_wcheng@...cinc.com>
Cc: Melody Olvera <quic_molvera@...cinc.com>, Vinod Koul <vkoul@...nel.org>,
Kishon Vijay Abraham I <kishon@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Philipp Zabel <p.zabel@...gutronix.de>,
Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
Bjorn Andersson <andersson@...nel.org>, Konrad Dybcio <konradybcio@...nel.org>,
Satya Durga Srinivasu Prabhala <quic_satyap@...cinc.com>, Trilok Soni <quic_tsoni@...cinc.com>,
linux-arm-msm@...r.kernel.org, linux-phy@...ts.infradead.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-usb@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 5/7] phy: qcom: Add M31 based eUSB2 PHY driver
On Wed, 26 Feb 2025 at 22:23, Wesley Cheng <quic_wcheng@...cinc.com> wrote:
>
>
>
> On 1/14/2025 2:34 AM, Dmitry Baryshkov wrote:
> > On Mon, Jan 13, 2025 at 01:52:11PM -0800, Melody Olvera wrote:
> >> From: Wesley Cheng <quic_wcheng@...cinc.com>
> >>
> >> On SM8750, the eUSB2 PHY used is M31 based. Add the initialization
> >> sequences to bring it out of reset, and to initialize the associated eUSB2
> >> repeater as well.
> >
> > What does M31 mean? What is the relationship between the eUSB and USB
> > M31 PHYs?
> >
>
> M31 is the vendor. I'll reword this to make it a bit clearer. There's no
> relationship between eUSB2 and USB2 PHY drivers, as the eUSB2 based driver
> would require some additional components such as a USB repeater.
>
> >>
> >> Signed-off-by: Wesley Cheng <quic_wcheng@...cinc.com>
> >> Signed-off-by: Melody Olvera <quic_molvera@...cinc.com>
> >> ---
> >> drivers/phy/qualcomm/Kconfig | 12 +-
> >> drivers/phy/qualcomm/Makefile | 1 +
> >> drivers/phy/qualcomm/phy-qcom-m31-eusb2.c | 269 ++++++++++++++++++++++++++++++
> >> 3 files changed, 281 insertions(+), 1 deletion(-)
> >
> > Please run the patch through checkpatch.pl --strict
> >
> >>
> >> diff --git a/drivers/phy/qualcomm/phy-qcom-m31-eusb2.c b/drivers/phy/qualcomm/phy-qcom-m31-eusb2.c
> >> new file mode 100644
> >> index 0000000000000000000000000000000000000000..e15529673e358db914936a60fa605c872cd2511a
> >> --- /dev/null
> >> +++ b/drivers/phy/qualcomm/phy-qcom-m31-eusb2.c
> >> @@ -0,0 +1,269 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +/*
> >> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> >> + */
> >> +
> >> +#include <linux/clk.h>
> >> +#include <linux/delay.h>
> >> +#include <linux/err.h>
> >> +#include <linux/io.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/module.h>
> >> +#include <linux/of.h>
> >> +#include <linux/phy/phy.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/reset.h>
> >> +#include <linux/slab.h>
> >> +
> >> +#define USB_PHY_UTMI_CTRL0 (0x3c)
> >> +
> >> +#define USB_PHY_UTMI_CTRL5 (0x50)
> >> +
> >> +#define USB_PHY_HS_PHY_CTRL_COMMON0 (0x54)
> >> +#define FSEL (0x7 << 4)
> >
> > GENMASK()
> >
> >> +#define FSEL_38_4_MHZ_VAL (0x6 << 4)
> >
> > FIELD_PREP
> >
> >> +
> >> +#define USB_PHY_HS_PHY_CTRL2 (0x64)
> >> +
> >> +#define USB_PHY_CFG0 (0x94)
> >> +#define USB_PHY_CFG1 (0x154)
> >> +
> >> +#define USB_PHY_FSEL_SEL (0xb8)
> >> +
> >> +#define USB_PHY_XCFGI_39_32 (0x16c)
> >> +#define USB_PHY_XCFGI_71_64 (0x17c)
> >> +#define USB_PHY_XCFGI_31_24 (0x168)
> >> +#define USB_PHY_XCFGI_7_0 (0x15c)
> >> +
> >> +#define M31_EUSB_PHY_INIT_CFG(o, b, v) \
> >> +{ \
> >> + .off = o, \
> >> + .mask = b, \
> >> + .val = v, \
> >> +}
> >> +
> >> +struct m31_phy_tbl_entry {
> >> + u32 off;
> >> + u32 mask;
> >> + u32 val;
> >> +};
> >> +
> >> +struct m31_eusb2_priv_data {
> >> + const struct m31_phy_tbl_entry *setup_seq;
> >> + unsigned int setup_seq_nregs;
> >> + const struct m31_phy_tbl_entry *override_seq;
> >> + unsigned int override_seq_nregs;
> >> + const struct m31_phy_tbl_entry *reset_seq;
> >> + unsigned int reset_seq_nregs;
> >> + unsigned int fsel;
> >> +};
> >> +
> >> +static const struct m31_phy_tbl_entry m31_eusb2_setup_tbl[] = {
> >> + M31_EUSB_PHY_INIT_CFG(USB_PHY_CFG0, BIT(1), 1),
> >> + M31_EUSB_PHY_INIT_CFG(USB_PHY_UTMI_CTRL5, BIT(1), 1),
> >> + M31_EUSB_PHY_INIT_CFG(USB_PHY_CFG1, BIT(0), 1),
> >> + M31_EUSB_PHY_INIT_CFG(USB_PHY_FSEL_SEL, BIT(0), 1),
> >> +};
> >> +
> >> +static const struct m31_phy_tbl_entry m31_eusb_phy_override_tbl[] = {
> >> + M31_EUSB_PHY_INIT_CFG(USB_PHY_XCFGI_39_32, GENMASK(3, 2), 0),
> >> + M31_EUSB_PHY_INIT_CFG(USB_PHY_XCFGI_71_64, GENMASK(3, 0), 7),
> >> + M31_EUSB_PHY_INIT_CFG(USB_PHY_XCFGI_31_24, GENMASK(2, 0), 0),
> >> + M31_EUSB_PHY_INIT_CFG(USB_PHY_XCFGI_7_0, GENMASK(1, 0), 0),
> >> +};
> >> +
> >> +static const struct m31_phy_tbl_entry m31_eusb_phy_reset_tbl[] = {
> >> + M31_EUSB_PHY_INIT_CFG(USB_PHY_HS_PHY_CTRL2, BIT(3), 1),
> >> + M31_EUSB_PHY_INIT_CFG(USB_PHY_HS_PHY_CTRL2, BIT(2), 1),
> >> + M31_EUSB_PHY_INIT_CFG(USB_PHY_UTMI_CTRL0, BIT(0), 1),
> >> + M31_EUSB_PHY_INIT_CFG(USB_PHY_HS_PHY_CTRL_COMMON0, BIT(1), 1),
> >> + M31_EUSB_PHY_INIT_CFG(USB_PHY_HS_PHY_CTRL_COMMON0, BIT(2), 0),
> >> + M31_EUSB_PHY_INIT_CFG(USB_PHY_UTMI_CTRL5, BIT(1), 0),
> >> + M31_EUSB_PHY_INIT_CFG(USB_PHY_HS_PHY_CTRL2, BIT(3), 0),
> >> + M31_EUSB_PHY_INIT_CFG(USB_PHY_CFG0, BIT(1), 0),
> >> +};
> >> +
> >> +struct m31eusb2_phy {
> >> + struct phy *phy;
> >> + void __iomem *base;
> >> + const struct m31_eusb2_priv_data *data;
> >> +
> >> + struct regulator *vreg;
> >> + struct clk *clk;
> >> + struct reset_control *reset;
> >> +
> >> + struct phy *repeater;
> >> +};
> >> +
> >> +static void msm_m31_eusb2_write_readback(void __iomem *base, u32 offset,
> >> + const u32 mask, u32 val)
> >> +{
> >> + u32 write_val, tmp = readl_relaxed(base + offset);
> >> +
> >> + tmp &= ~mask;
> >> + write_val = tmp | val;
> >> +
> >> + writel_relaxed(write_val, base + offset);
> >> +
> >> + tmp = readl_relaxed(base + offset);
> >> + tmp &= mask;
> >> +
> >> + if (tmp != val)
> >> + pr_err("write: %x to offset: %x FAILED\n", val, offset);
> >> +}
> >> +
> >> +static void m31eusb2_phy_write_sequence(struct m31eusb2_phy *phy,
> >> + const struct m31_phy_tbl_entry *tbl,
> >> + int num)
> >> +{
> >> + int i;
> >> +
> >> + for (i = 0 ; i < num; i++, tbl++) {
> >> + dev_dbg(&phy->phy->dev, "Offset:%x BitMask:%x Value:%x",
> >> + tbl->off, tbl->mask, tbl->val);
> >> +
> >> + msm_m31_eusb2_write_readback(phy->base,
> >> + tbl->off, tbl->mask,
> >> + tbl->val << __ffs(tbl->mask));
> >
> > could you please check, what actually gets written? I suspect there
> > should be a -1 here.
> >
>
> The __ffs uses the ctz/ctzl built in, which counts leading zeros, so the
> -1 should already be accounted for. FIELD_PREP uses the ffs builtin
> directly, which would require the -1. Confirmed that the writes are being
> done as expected from the programming tables above.
Ack, thanks.
--
With best wishes
Dmitry
Powered by blists - more mailing lists