[PATCH 1/5] usb: dwc3-generic: implement Qualcomm wrapper

[Caleb Connolly]

Hi Marek,

On 06/02/2024 20:36, Marek Vasut wrote:
> On 1/31/24 15:57, Caleb Connolly wrote:
>> The Qualcomm specific dwc3 wrapper isn't hugely complicated, implemented
>> the missing initialisation for host and gadget mode.
>>
>> Signed-off-by: Caleb Connolly <caleb.connolly at linaro.org>
>> ---
>>   drivers/usb/dwc3/dwc3-generic.c | 99
>> ++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 98 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-generic.c
>> b/drivers/usb/dwc3/dwc3-generic.c
>> index 48da621ba966..1119cdecd26d 100644
>> --- a/drivers/usb/dwc3/dwc3-generic.c
>> +++ b/drivers/usb/dwc3/dwc3-generic.c
>> @@ -419,6 +419,99 @@ struct dwc3_glue_ops ti_ops = {
>>       .glue_configure = dwc3_ti_glue_configure,
>>   };
>>   +/* USB QSCRATCH Hardware registers */
>> +#define QSCRATCH_HS_PHY_CTRL 0x10
>> +#define UTMI_OTG_VBUS_VALID BIT(20)
>> +#define SW_SESSVLD_SEL BIT(28)
>> +
>> +#define QSCRATCH_SS_PHY_CTRL 0x30
>> +#define LANE0_PWR_PRESENT BIT(24)
>> +
>> +#define QSCRATCH_GENERAL_CFG 0x08
>> +#define PIPE_UTMI_CLK_SEL BIT(0)
>> +#define PIPE3_PHYSTATUS_SW BIT(3)
>> +#define PIPE_UTMI_CLK_DIS BIT(8)
>> +
>> +#define PWR_EVNT_IRQ_STAT_REG 0x58
>> +#define PWR_EVNT_LPM_IN_L2_MASK BIT(4)
>> +#define PWR_EVNT_LPM_OUT_L2_MASK BIT(5)
>> +
>> +#define SDM845_QSCRATCH_BASE_OFFSET 0xf8800
>> +#define SDM845_QSCRATCH_SIZE 0x400
>> +#define SDM845_DWC3_CORE_SIZE 0xcd00
> 
> Newline here please
> 
>> +static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset,
>> u32 val)
>> +{
>> +    u32 reg;
>> +
>> +    reg = readl(base + offset);
>> +    reg |= val;
>> +    writel(reg, base + offset);
> 
> Use setbits_le32() .
> 
>> +    /* ensure that above write is through */
>> +    readl(base + offset);
> 
> Is this needed ?

I honestly don't know, this is copied from the Linux driver and it seems
to be very defensively written. I doubt it's strictly necessary.
> 
>> +}
>> +
>> +static inline void dwc3_qcom_clrbits(void __iomem *base, u32 offset,
>> u32 val)
>> +{
>> +    u32 reg;
>> +
>> +    reg = readl(base + offset);
>> +    reg &= ~val;
>> +    writel(reg, base + offset);
> 
> clrbits_le32()
> 
>> +    /* ensure that above write is through */
>> +    readl(base + offset);
>> +}
>> +
>> +static void dwc3_qcom_vbus_override_enable(void __iomem
>> *qscratch_base, bool enable)
>> +{
>> +    if (enable) {
>> +        dwc3_qcom_setbits(qscratch_base, QSCRATCH_SS_PHY_CTRL,
>> +                  LANE0_PWR_PRESENT);
>> +        dwc3_qcom_setbits(qscratch_base, QSCRATCH_HS_PHY_CTRL,
>> +                  UTMI_OTG_VBUS_VALID | SW_SESSVLD_SEL);
>> +    } else {
>> +        dwc3_qcom_clrbits(qscratch_base, QSCRATCH_SS_PHY_CTRL,
>> +                  LANE0_PWR_PRESENT);
>> +        dwc3_qcom_clrbits(qscratch_base, QSCRATCH_HS_PHY_CTRL,
>> +                  UTMI_OTG_VBUS_VALID | SW_SESSVLD_SEL);
>> +    }
>> +}
>> +
>> +/* For controllers running without superspeed PHYs */
>> +static void dwc3_qcom_select_utmi_clk(void __iomem *qscratch_base)
>> +{
>> +    /* Configure dwc3 to use UTMI clock as PIPE clock not present */
>> +    dwc3_qcom_setbits(qscratch_base, QSCRATCH_GENERAL_CFG,
>> +              PIPE_UTMI_CLK_DIS);
>> +
>> +    udelay(500);
> 
> Isn't there some possibility to poll for completion instead of fixed
> delay ? If so, use wait_for_bit or some such .

Not that I'm aware of, no. I think this hardware just has a blanket
"writes take X bus cycles to complete" rule or something. It's totally
possible that this code was originally written this way to work around
some issues on an FPGA prototype or something. Everything seems to still
work if I remove the delays so I'll drop them...
> 
>> +    dwc3_qcom_setbits(qscratch_base, QSCRATCH_GENERAL_CFG,
>> +              PIPE_UTMI_CLK_SEL | PIPE3_PHYSTATUS_SW);
>> +
>> +    udelay(500);
>> +
>> +    dwc3_qcom_clrbits(qscratch_base, QSCRATCH_GENERAL_CFG,
>> +              PIPE_UTMI_CLK_DIS);
>> +}
>> +
>> +static void dwc3_qcom_glue_configure(struct udevice *dev, int index,
>> +                     enum usb_dr_mode mode)
>> +{
>> +    void __iomem *qscratch_base = (void __iomem *)dev_read_addr(dev);
> 
> Can this be NULL ? If yes, add check for != NULL .
Will do.
> 
>> +    if (dev_read_bool(dev, "qcom,select-utmi-as-pipe-clk"))
>> +        dwc3_qcom_select_utmi_clk(qscratch_base);
>> +
>> +    if (mode != USB_DR_MODE_HOST)
>> +        dwc3_qcom_vbus_override_enable(qscratch_base, true);
>> +}
>> +
>> +struct dwc3_glue_ops qcom_ops = {
>> +    .glue_configure = dwc3_qcom_glue_configure,
>> +};
>> +
>>   static int dwc3_rk_glue_get_ctrl_dev(struct udevice *dev, ofnode *node)
>>   {
>>       *node = dev_ofnode(dev);
>> @@ -506,6 +599,10 @@ static int dwc3_glue_reset_init(struct udevice *dev,
>>       else if (ret)
>>           return ret;
>>   +    if (device_is_compatible(dev, "qcom,dwc3")) {
>> +        reset_assert_bulk(&glue->resets);
>> +        udelay(500);
> 
> Why this delay here ?

According to the docs, the reset should be asserted for at least 6 sleep
clock cycles, that's ~200us on sdm845, but it can vary by platform.

> 
> [...]

-- 
// Caleb (they/them)