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@xxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxx>
>> Signed-off-by: Melody Olvera <quic_molvera@xxxxxxxxxxx>
>> ---
>> 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
