devicetree: avoid duplicated matching code (was: Re: [PATCH 1/3] xhci: plat: adapt to unified device property interface)

devicetree: avoid duplicated matching code (was: Re: [PATCH 1/3] xhci: plat: adapt to unified device property interface)

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1294 bytes --]


Hi,

Heikki Krogerus <heikki.krogerus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> writes:
> @@ -197,7 +196,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  	}
>  
>  	xhci = hcd_to_xhci(hcd);
> -	match = of_match_node(usb_xhci_of_match, node);
> +	match = of_match_node(usb_xhci_of_match, pdev->dev.of_node);

Rob, it's weird that OF-based drivers have to redo the same matching
which was already done by drivers/base/platform.c::platform_match() just
to get match->data. If we know we matched, couldn't we just cache a
pointer to match->data in struct device_node.data ? Something like
below? (completely untested)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index b299de2b3afa..9b44caa38f7c 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -981,6 +981,8 @@ const struct of_device_id *of_match_node(const struct of_device_id *matches,
 
 	raw_spin_lock_irqsave(&devtree_lock, flags);
 	match = __of_match_node(matches, node);
+	if (match)
+		node->data = match->data;
 	raw_spin_unlock_irqrestore(&devtree_lock, flags);
 	return match;
 }

I tried to find users of device_node.data but couldn't find any. If this
patch is acceptable, we can remove 160 occurences of of_match_node with
some variance of node->data.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: devicetree: avoid duplicated matching code (was: Re: [PATCH 1/3] xhci: plat: adapt to unified device property interface)

[Rob Herring]

On Thu, Apr 21, 2016 at 6:20 AM, Felipe Balbi
<felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
>
> Hi,
>
> Heikki Krogerus <heikki.krogerus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> writes:
>> @@ -197,7 +196,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>       }
>>
>>       xhci = hcd_to_xhci(hcd);
>> -     match = of_match_node(usb_xhci_of_match, node);
>> +     match = of_match_node(usb_xhci_of_match, pdev->dev.of_node);
>
> Rob, it's weird that OF-based drivers have to redo the same matching
> which was already done by drivers/base/platform.c::platform_match() just
> to get match->data. If we know we matched, couldn't we just cache a
> pointer to match->data in struct device_node.data ? Something like
> below? (completely untested)

Yes, it is. AIUI, there is some sort of race condition in doing what
you suggest though. IIRC, Grant did that and reverted it if you look
at the git history.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: devicetree: avoid duplicated matching code (was: Re: [PATCH 1/3] xhci: plat: adapt to unified device property interface)

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1212 bytes --]


Hi,

Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:
> On Thu, Apr 21, 2016 at 6:20 AM, Felipe Balbi
> <felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
>>
>> Hi,
>>
>> Heikki Krogerus <heikki.krogerus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> writes:
>>> @@ -197,7 +196,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>>       }
>>>
>>>       xhci = hcd_to_xhci(hcd);
>>> -     match = of_match_node(usb_xhci_of_match, node);
>>> +     match = of_match_node(usb_xhci_of_match, pdev->dev.of_node);
>>
>> Rob, it's weird that OF-based drivers have to redo the same matching
>> which was already done by drivers/base/platform.c::platform_match() just
>> to get match->data. If we know we matched, couldn't we just cache a
>> pointer to match->data in struct device_node.data ? Something like
>> below? (completely untested)
>
> Yes, it is. AIUI, there is some sort of race condition in doing what
> you suggest though. IIRC, Grant did that and reverted it if you look
> at the git history.

looking at drivers/base/platform.c I can't find anything along these
lines. Adding Grant.

Grant, any memory left of this race ?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: devicetree: avoid duplicated matching code (was: Re: [PATCH 1/3] xhci: plat: adapt to unified device property interface)

[Rob Herring]

On Thu, Apr 21, 2016 at 8:57 AM, Felipe Balbi
<felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
>
> Hi,
>
> Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:
>> On Thu, Apr 21, 2016 at 6:20 AM, Felipe Balbi
>> <felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
>>>
>>> Hi,
>>>
>>> Heikki Krogerus <heikki.krogerus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> writes:
>>>> @@ -197,7 +196,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>>>       }
>>>>
>>>>       xhci = hcd_to_xhci(hcd);
>>>> -     match = of_match_node(usb_xhci_of_match, node);
>>>> +     match = of_match_node(usb_xhci_of_match, pdev->dev.of_node);
>>>
>>> Rob, it's weird that OF-based drivers have to redo the same matching
>>> which was already done by drivers/base/platform.c::platform_match() just
>>> to get match->data. If we know we matched, couldn't we just cache a
>>> pointer to match->data in struct device_node.data ? Something like
>>> below? (completely untested)
>>
>> Yes, it is. AIUI, there is some sort of race condition in doing what
>> you suggest though. IIRC, Grant did that and reverted it if you look
>> at the git history.
>
> looking at drivers/base/platform.c I can't find anything along these
> lines. Adding Grant.
>
> Grant, any memory left of this race ?

https://lkml.org/lkml/2011/5/18/384

tl;dr: matching and probe is not atomic.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: devicetree: avoid duplicated matching code (was: Re: [PATCH 1/3] xhci: plat: adapt to unified device property interface)

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1578 bytes --]


Hi,

Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:
> On Thu, Apr 21, 2016 at 8:57 AM, Felipe Balbi
> <felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
>>
>> Hi,
>>
>> Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:
>>> On Thu, Apr 21, 2016 at 6:20 AM, Felipe Balbi
>>> <felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
>>>>
>>>> Hi,
>>>>
>>>> Heikki Krogerus <heikki.krogerus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> writes:
>>>>> @@ -197,7 +196,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>>>>       }
>>>>>
>>>>>       xhci = hcd_to_xhci(hcd);
>>>>> -     match = of_match_node(usb_xhci_of_match, node);
>>>>> +     match = of_match_node(usb_xhci_of_match, pdev->dev.of_node);
>>>>
>>>> Rob, it's weird that OF-based drivers have to redo the same matching
>>>> which was already done by drivers/base/platform.c::platform_match() just
>>>> to get match->data. If we know we matched, couldn't we just cache a
>>>> pointer to match->data in struct device_node.data ? Something like
>>>> below? (completely untested)
>>>
>>> Yes, it is. AIUI, there is some sort of race condition in doing what
>>> you suggest though. IIRC, Grant did that and reverted it if you look
>>> at the git history.
>>
>> looking at drivers/base/platform.c I can't find anything along these
>> lines. Adding Grant.
>>
>> Grant, any memory left of this race ?
>
> https://lkml.org/lkml/2011/5/18/384
>
> tl;dr: matching and probe is not atomic.

cool, thanks

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]