diff mbox series

[v6,1/4] usb: dwc3: of-simple: bail probe if no dwc3 child node

Message ID 20210401213652.14676-2-jbx6244@gmail.com
State New
Headers show
Series [v6,1/4] usb: dwc3: of-simple: bail probe if no dwc3 child node | expand

Commit Message

Johan Jonker April 1, 2021, 9:36 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

Greg Kroah-Hartman April 5, 2021, 7:02 a.m. UTC | #1
On Thu, Apr 01, 2021 at 11:36:49PM +0200, Johan Jonker wrote:
> 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(+)

> 

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

> index 71fd620c5..8d3baa5df 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;


Why is this part of the "convert to yaml" patch series?  Shouldn't this
be a separate, independant patch?

thanks,

greg k-h
Johan Jonker April 5, 2021, 12:06 p.m. UTC | #2
On 4/5/21 9:02 AM, Greg KH wrote:
> On Thu, Apr 01, 2021 at 11:36:49PM +0200, Johan Jonker wrote:
>> 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(+)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
>> index 71fd620c5..8d3baa5df 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;
> 

> Why is this part of the "convert to yaml" patch series?  Shouldn't this
> be a separate, independant patch?
independent


Hi Greg,

As pointed out by Rob in the v3 review process the YAML conversion has
some side effects for new dt files. A good habit is to fix things before
they become in effect. That's why this patch is placed before the dtsi
changes and why Heiko asked to have a look at it.

For the sake of completeness there are other less optimal options:
- remove rk3399 support from dwc3-of-simple.c
- unselect CONFIG_USB_DWC3_OF_SIMPLE for new rk3399 device trees

In that case at least have it on record why USB maintainers didn't apply
it in case someone else start to complain about it later when Heiko goes
ahead with it.

Johan Jonker

> 
> thanks,
> 
> greg k-h
>
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
index 71fd620c5..8d3baa5df 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;