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

[Marek Vasut]

On 3/13/24 7:22 PM, Caleb Connolly wrote:

[...]

>>> +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.

Does git log indicate anything ?

I suspect this is some sort of barrier .

[...]

>>> +/* 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...

Could you possibly ask someone ?

[...]

>>>    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.

A comment in the code would be nice.

Sorry for the abysmal delay in replies.

btw. the new version of this series is still OK to go in, unless you 
want to fill in the comments. They can also go in in separate follow up 
patch.