diff mbox series

[v3,1/6] usb: gadget: udc: atmel: use of_find_matching_node_and_match

Message ID 20200722134421.190741-2-cristian.birsan@microchip.com (mailing list archive)
State Superseded
Headers show
Series usb: gadget: udc: atmel: add usb device support for SAM9x60 SoC | expand

Commit Message

Cristian Birsan July 22, 2020, 1:44 p.m. UTC 
From: Claudiu Beznea <claudiu.beznea@microchip.com>

Instead of trying to match every possible compatible use
of_find_matching_node_and_match() and pass the compatible array.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/usb/gadget/udc/atmel_usba_udc.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

Comments

Claudiu Beznea July 22, 2020, 2:43 p.m. UTC | #1 
On 22.07.2020 16:44, cristian.birsan@microchip.com wrote:
> From: Claudiu Beznea <claudiu.beznea@microchip.com>
> 
> Instead of trying to match every possible compatible use
> of_find_matching_node_and_match() and pass the compatible array.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  drivers/usb/gadget/udc/atmel_usba_udc.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
> index c5128c229c52..ee2b550aa400 100644
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> @@ -2112,6 +2112,13 @@ static const struct of_device_id atmel_udc_dt_ids[] = {
>  
>  MODULE_DEVICE_TABLE(of, atmel_udc_dt_ids);
>  
> +static const struct of_device_id atmel_pmc_dt_ids[] = {
> +	{ .compatible = "atmel,at91sam9g45-pmc" },
> +	{ .compatible = "atmel,at91sam9rl-pmc" },
> +	{ .compatible = "atmel,at91sam9x5-pmc" },
> +	{ /* sentinel */ }
> +};
> +
>  static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
>  						    struct usba_udc *udc)
>  {
> @@ -2128,13 +2135,17 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
>  
>  	udc_config = match->data;
>  	udc->errata = udc_config->errata;
> -	udc->pmc = syscon_regmap_lookup_by_compatible("atmel,at91sam9g45-pmc");
> -	if (IS_ERR(udc->pmc))
> -		udc->pmc = syscon_regmap_lookup_by_compatible("atmel,at91sam9rl-pmc");
> -	if (IS_ERR(udc->pmc))
> -		udc->pmc = syscon_regmap_lookup_by_compatible("atmel,at91sam9x5-pmc");
> -	if (udc->errata && IS_ERR(udc->pmc))
> -		return ERR_CAST(udc->pmc);
> +	if (udc->errata) {
> +		pp = of_find_matching_node_and_match(NULL, atmel_pmc_dt_ids,
> +						     NULL);
> +		if (!pp)
> +			return ERR_PTR(-ENODEV);
> +
> +		udc->pmc = syscon_node_to_regmap(pp);
> +		of_node_put(pp);
> +		if (IS_ERR(udc->pmc))
> +			return ERR_CAST(udc->pmc);
> +	}

This seems a bit not similar. I may had been wrong at the moment I wrote
this patch. Probably the best would be:

+	udc->pmc = ERR_PTR(-ENODEV);
+	pp = of_find_matching_node_and_match(NULL, atmel_pmc_dt_ids,
+					     NULL);
+	if (pp) {
+		udc->pmc = syscon_node_to_regmap(pp);
+		of_node_put(pp);
+	}
+
+	if ((!pp || IS_ERR(udc->pmc)) && udc->errata)
+		return ERR_CAST(udc->pmc);

Thank you,
Claudiu Beznea

>  
>  	udc->num_ep = 0;
>  
>
Claudiu Beznea July 23, 2020, 4:28 p.m. UTC | #2 
On 22.07.2020 17:43, Claudiu.Beznea@microchip.com wrote:
> 
> 
> On 22.07.2020 16:44, cristian.birsan@microchip.com wrote:
>> From: Claudiu Beznea <claudiu.beznea@microchip.com>
>>
>> Instead of trying to match every possible compatible use
>> of_find_matching_node_and_match() and pass the compatible array.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>>  drivers/usb/gadget/udc/atmel_usba_udc.c | 25 ++++++++++++++++++-------
>>  1 file changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
>> index c5128c229c52..ee2b550aa400 100644
>> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
>> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
>> @@ -2112,6 +2112,13 @@ static const struct of_device_id atmel_udc_dt_ids[] = {
>>  
>>  MODULE_DEVICE_TABLE(of, atmel_udc_dt_ids);
>>  
>> +static const struct of_device_id atmel_pmc_dt_ids[] = {
>> +	{ .compatible = "atmel,at91sam9g45-pmc" },
>> +	{ .compatible = "atmel,at91sam9rl-pmc" },
>> +	{ .compatible = "atmel,at91sam9x5-pmc" },
>> +	{ /* sentinel */ }
>> +};
>> +
>>  static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
>>  						    struct usba_udc *udc)
>>  {
>> @@ -2128,13 +2135,17 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
>>  
>>  	udc_config = match->data;
>>  	udc->errata = udc_config->errata;
>> -	udc->pmc = syscon_regmap_lookup_by_compatible("atmel,at91sam9g45-pmc");
>> -	if (IS_ERR(udc->pmc))
>> -		udc->pmc = syscon_regmap_lookup_by_compatible("atmel,at91sam9rl-pmc");
>> -	if (IS_ERR(udc->pmc))
>> -		udc->pmc = syscon_regmap_lookup_by_compatible("atmel,at91sam9x5-pmc");
>> -	if (udc->errata && IS_ERR(udc->pmc))
>> -		return ERR_CAST(udc->pmc);
>> +	if (udc->errata) {
>> +		pp = of_find_matching_node_and_match(NULL, atmel_pmc_dt_ids,
>> +						     NULL);
>> +		if (!pp)
>> +			return ERR_PTR(-ENODEV);
>> +
>> +		udc->pmc = syscon_node_to_regmap(pp);
>> +		of_node_put(pp);
>> +		if (IS_ERR(udc->pmc))
>> +			return ERR_CAST(udc->pmc);
>> +	}
> 
> This seems a bit not similar. I may had been wrong at the moment I wrote
> this patch. Probably the best would be:
> 
> +	udc->pmc = ERR_PTR(-ENODEV);
> +	pp = of_find_matching_node_and_match(NULL, atmel_pmc_dt_ids,
> +					     NULL);
> +	if (pp) {
> +		udc->pmc = syscon_node_to_regmap(pp);
> +		of_node_put(pp);
> +	}
> +
> +	if ((!pp || IS_ERR(udc->pmc)) && udc->errata)
> +		return ERR_CAST(udc->pmc);

Actually, the initial patch should be good and simpler than what I proposed
in previous email. Please ignore it.

Sorry for the noise!

> 
> Thank you,
> Claudiu Beznea
> 
>>  
>>  	udc->num_ep = 0;
>>  
>>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Cristian Birsan July 23, 2020, 4:33 p.m. UTC | #3 
On 7/23/20 7:28 PM, Claudiu Beznea - M18063 wrote:
> 
> 
> On 22.07.2020 17:43, Claudiu.Beznea@microchip.com wrote:
>>
>>
>> On 22.07.2020 16:44, cristian.birsan@microchip.com wrote:
>>> From: Claudiu Beznea <claudiu.beznea@microchip.com>
>>>
>>> Instead of trying to match every possible compatible use
>>> of_find_matching_node_and_match() and pass the compatible array.
>>>
>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>>> ---
>>>  drivers/usb/gadget/udc/atmel_usba_udc.c | 25 ++++++++++++++++++-------
>>>  1 file changed, 18 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
>>> index c5128c229c52..ee2b550aa400 100644
>>> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
>>> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
>>> @@ -2112,6 +2112,13 @@ static const struct of_device_id atmel_udc_dt_ids[] = {
>>>  
>>>  MODULE_DEVICE_TABLE(of, atmel_udc_dt_ids);
>>>  
>>> +static const struct of_device_id atmel_pmc_dt_ids[] = {
>>> +	{ .compatible = "atmel,at91sam9g45-pmc" },
>>> +	{ .compatible = "atmel,at91sam9rl-pmc" },
>>> +	{ .compatible = "atmel,at91sam9x5-pmc" },
>>> +	{ /* sentinel */ }
>>> +};
>>> +
>>>  static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
>>>  						    struct usba_udc *udc)
>>>  {
>>> @@ -2128,13 +2135,17 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
>>>  
>>>  	udc_config = match->data;
>>>  	udc->errata = udc_config->errata;
>>> -	udc->pmc = syscon_regmap_lookup_by_compatible("atmel,at91sam9g45-pmc");
>>> -	if (IS_ERR(udc->pmc))
>>> -		udc->pmc = syscon_regmap_lookup_by_compatible("atmel,at91sam9rl-pmc");
>>> -	if (IS_ERR(udc->pmc))
>>> -		udc->pmc = syscon_regmap_lookup_by_compatible("atmel,at91sam9x5-pmc");
>>> -	if (udc->errata && IS_ERR(udc->pmc))
>>> -		return ERR_CAST(udc->pmc);
>>> +	if (udc->errata) {
>>> +		pp = of_find_matching_node_and_match(NULL, atmel_pmc_dt_ids,
>>> +						     NULL);
>>> +		if (!pp)
>>> +			return ERR_PTR(-ENODEV);
>>> +
>>> +		udc->pmc = syscon_node_to_regmap(pp);
>>> +		of_node_put(pp);
>>> +		if (IS_ERR(udc->pmc))
>>> +			return ERR_CAST(udc->pmc);
>>> +	}
>>
>> This seems a bit not similar. I may had been wrong at the moment I wrote
>> this patch. Probably the best would be:
>>
>> +	udc->pmc = ERR_PTR(-ENODEV);
>> +	pp = of_find_matching_node_and_match(NULL, atmel_pmc_dt_ids,
>> +					     NULL);
>> +	if (pp) {
>> +		udc->pmc = syscon_node_to_regmap(pp);
>> +		of_node_put(pp);
>> +	}
>> +
>> +	if ((!pp || IS_ERR(udc->pmc)) && udc->errata)
>> +		return ERR_CAST(udc->pmc);
> 
> Actually, the initial patch should be good and simpler than what I proposed
> in previous email. Please ignore it.
> 
> Sorry for the noise!

I will rebase again the patchset on top of testing/next and send a V4 with your original version.

Regards,
Cristian Birsan 
> 
>>
>> Thank you,
>> Claudiu Beznea
>>
>>>  
>>>  	udc->num_ep = 0;
>>>  
>>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff mbox series

Patch

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index c5128c229c52..ee2b550aa400 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -2112,6 +2112,13 @@  static const struct of_device_id atmel_udc_dt_ids[] = {
 
 MODULE_DEVICE_TABLE(of, atmel_udc_dt_ids);
 
+static const struct of_device_id atmel_pmc_dt_ids[] = {
+	{ .compatible = "atmel,at91sam9g45-pmc" },
+	{ .compatible = "atmel,at91sam9rl-pmc" },
+	{ .compatible = "atmel,at91sam9x5-pmc" },
+	{ /* sentinel */ }
+};
+
 static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
 						    struct usba_udc *udc)
 {
@@ -2128,13 +2135,17 @@  static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
 
 	udc_config = match->data;
 	udc->errata = udc_config->errata;
-	udc->pmc = syscon_regmap_lookup_by_compatible("atmel,at91sam9g45-pmc");
-	if (IS_ERR(udc->pmc))
-		udc->pmc = syscon_regmap_lookup_by_compatible("atmel,at91sam9rl-pmc");
-	if (IS_ERR(udc->pmc))
-		udc->pmc = syscon_regmap_lookup_by_compatible("atmel,at91sam9x5-pmc");
-	if (udc->errata && IS_ERR(udc->pmc))
-		return ERR_CAST(udc->pmc);
+	if (udc->errata) {
+		pp = of_find_matching_node_and_match(NULL, atmel_pmc_dt_ids,
+						     NULL);
+		if (!pp)
+			return ERR_PTR(-ENODEV);
+
+		udc->pmc = syscon_node_to_regmap(pp);
+		of_node_put(pp);
+		if (IS_ERR(udc->pmc))
+			return ERR_CAST(udc->pmc);
+	}
 
 	udc->num_ep = 0;