| 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: <92b42216-c409-4e21-a33e-54c29fa5f8a0@quicinc.com>
Date: Wed, 26 Feb 2025 12:23:34 -0800
From: Wesley Cheng <quic_wcheng@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Melody Olvera
<quic_molvera@...cinc.com>
CC: 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 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.
Thanks
Wesley Cheng
Powered by blists - more mailing lists