diff mbox series

[v5,2/8] usb: dwc3: of-simple: bail probe if no dwc3 child node

Message ID 20210209192350.7130-2-jbx6244@gmail.com
State Superseded
Headers show
Series [v5,1/8] dt-bindings: usb: convert rockchip,dwc3.txt to yaml | expand

Commit Message

Johan Jonker Feb. 9, 2021, 7:23 p.m. UTC
For some of the dwc3-of-simple compatible SoCs we
don't want to bind this driver to a dwc3 node,
but bind that node to the 'snps,dwc3' driver instead.
The kernel has no logic to decide which driver to bind
to if there are 2 matching drivers, so bail probe if no
dwc3 child node.

Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---
 drivers/usb/dwc3/dwc3-of-simple.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Heiko Stuebner March 24, 2021, 4:41 p.m. UTC | #1
Hi Greg, Felipe,

Am Dienstag, 9. Februar 2021, 20:23:44 CET schrieb Johan Jonker:
> For some of the dwc3-of-simple compatible SoCs we

> don't want to bind this driver to a dwc3 node,

> but bind that node to the 'snps,dwc3' driver instead.

> The kernel has no logic to decide which driver to bind

> to if there are 2 matching drivers, so bail probe if no

> dwc3 child node.

> 

> Signed-off-by: Johan Jonker <jbx6244@gmail.com>


It looks like this patch fell through the cracks?

I.e. I can see patches 1+6 adding the devicetree bindings
in 5.12-rc but haven't found this patch there.

And looking at
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/log/drivers/usb/dwc3/dwc3-of-simple.c?h=usb-next
I also didn't find it.

From what I gathered that was somehow mandated from the Devicetree side
as the dwc3-subnode system merely is some Linux-specific thingy
and should not be enforced if not explicitly needed.

I guess Johan can provide pointers to the previous discussion.

So could you look at applying this patch to some usb-tree?


Thanks
Heiko


> ---

>  drivers/usb/dwc3/dwc3-of-simple.c | 4 ++++

>  1 file changed, 4 insertions(+)

> 

> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c

> index e62ecd22b..347b4d384 100644

> --- a/drivers/usb/dwc3/dwc3-of-simple.c

> +++ b/drivers/usb/dwc3/dwc3-of-simple.c

> @@ -38,6 +38,10 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)

>  

>  	int			ret;

>  

> +	/* Bail probe if no dwc3 child node. */

> +	if (!of_get_compatible_child(dev->of_node, "snps,dwc3"))

> +		return -ENODEV;

> +

>  	simple = devm_kzalloc(dev, sizeof(*simple), GFP_KERNEL);

>  	if (!simple)

>  		return -ENOMEM;

>
Johan Jonker March 24, 2021, 5:21 p.m. UTC | #2
On 3/24/21 5:41 PM, Heiko Stübner wrote:
> Hi Greg, Felipe,
> 
> Am Dienstag, 9. Februar 2021, 20:23:44 CET schrieb Johan Jonker:
>> For some of the dwc3-of-simple compatible SoCs we
>> don't want to bind this driver to a dwc3 node,
>> but bind that node to the 'snps,dwc3' driver instead.
>> The kernel has no logic to decide which driver to bind
>> to if there are 2 matching drivers, so bail probe if no
>> dwc3 child node.
>>
>> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
> 
> It looks like this patch fell through the cracks?
> 
> I.e. I can see patches 1+6 adding the devicetree bindings
> in 5.12-rc but haven't found this patch there.
> 
> And looking at
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/log/drivers/usb/dwc3/dwc3-of-simple.c?h=usb-next
> I also didn't find it.
> 
> From what I gathered that was somehow mandated from the Devicetree side
> as the dwc3-subnode system merely is some Linux-specific thingy
> and should not be enforced if not explicitly needed.
> 

> I guess Johan can provide pointers to the previous discussion.

Comment by Rob in version 3:
Presumably you are getting lucky here (with link order). The kernel
has no logic to decide which driver to bind to if there are 2 matching
drivers. If we did, it would probably be the opposite of what you want
here as we'd pick the most specific match. This driver should probably
bail probe if no dwc3 child node.

https://lore.kernel.org/linux-rockchip/20210205114011.10381-6-jbx6244@gmail.com/

https://lore.kernel.org/linux-rockchip/CAL_JsqJwNdUfoYM8SZmOgMG9iAyZkJ4-kzjjiDDm_mdmghTEOA@mail.gmail.com/

> 
> So could you look at applying this patch to some usb-tree?
> 
> 
> Thanks
> Heiko
> 
> 
>> ---
>>  drivers/usb/dwc3/dwc3-of-simple.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
>> index e62ecd22b..347b4d384 100644
>> --- a/drivers/usb/dwc3/dwc3-of-simple.c
>> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
>> @@ -38,6 +38,10 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
>>  
>>  	int			ret;
>>  
>> +	/* Bail probe if no dwc3 child node. */
>> +	if (!of_get_compatible_child(dev->of_node, "snps,dwc3"))
>> +		return -ENODEV;
>> +
>>  	simple = devm_kzalloc(dev, sizeof(*simple), GFP_KERNEL);
>>  	if (!simple)
>>  		return -ENOMEM;
>>
> 
> 
> 
>
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
index e62ecd22b..347b4d384 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -38,6 +38,10 @@  static int dwc3_of_simple_probe(struct platform_device *pdev)
 
 	int			ret;
 
+	/* Bail probe if no dwc3 child node. */
+	if (!of_get_compatible_child(dev->of_node, "snps,dwc3"))
+		return -ENODEV;
+
 	simple = devm_kzalloc(dev, sizeof(*simple), GFP_KERNEL);
 	if (!simple)
 		return -ENOMEM;