[PATCH v4 4/5] usb: dwc3-generic: Add rk3568 support

[Marek Vasut]

On 7/31/23 00:45, Jonas Karlman wrote:
> On 2023-07-30 20:12, Marek Vasut wrote:
>> On 7/30/23 19:03, Jonas Karlman wrote:
>>> On 2023-07-28 16:02, Marek Vasut wrote:
>>>> On 7/28/23 14:40, Jonas Karlman wrote:
>>>>> RK3568 share glue and ctrl in a single node. Use glue_get_ctrl_dev to
>>>>> return the glue node as the ctrl node.
>>>>>
>>>>> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
>>>>> Reviewed-by: Jagan Teki <jagan at amarulasolutions.com>
>>>>> ---
>>>>> v4:
>>>>> - No change
>>>>> v3:
>>>>> - No change
>>>>> v2:
>>>>> - No change
>>>>> - Collect r-b tag
>>>>>
>>>>>     drivers/usb/dwc3/dwc3-generic.c | 17 +++++++++++++++++
>>>>>     1 file changed, 17 insertions(+)
>>>>>
>>>>> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
>>>>> index 2331ac453132..f6d087722c9f 100644
>>>>> --- a/drivers/usb/dwc3/dwc3-generic.c
>>>>> +++ b/drivers/usb/dwc3/dwc3-generic.c
>>>>> @@ -405,6 +405,22 @@ struct dwc3_glue_ops ti_ops = {
>>>>>     	.glue_configure = dwc3_ti_glue_configure,
>>>>>     };
>>>>>     
>>>>> +static int dwc3_rk_glue_get_ctrl_dev(struct udevice *dev, ofnode *node)
>>>>> +{
>>>>> +	if (!device_is_compatible(dev, "snps,dwc3"))
>>>>> +		return -EINVAL;
>>>>
>>>> Can this ever happen ?
>>>
>>> In normal case, probably not, in case someone e.g. try to use wrong
>>> compatible or a bad/strange device tree, sure it can.
>>>
>>> Added the check to make sure this ops only returns a valid ctrl node.
>>
>> The driver should already only bind to compatibles listed in
>> dwc3_glue_ids , so what kind of DT would that be ?
> 
> Not sure, hence I added the check to be extra safe the code would not
> break. I can however now see that bindings look like following, after
> a pending rk3588 compatible gets added.
> 
>    compatible:
>      items:
>        - enum:
>            - rockchip,rk3328-dwc3
>            - rockchip,rk3568-dwc3
>            - rockchip,rk3588-dwc3
>        - const: snps,dwc3
> 
> So I guess we can assume that for these compatible this check should
> always pass.
> 
> Do you want this check/part of the code removed?

Since the check seem to make little sense, yes please.