[PATCH] firmware: ti_sci: Parse all resource ranges even if some is not available

[PATCH] firmware: ti_sci: Parse all resource ranges even if some is not available

[Peter Ujfalusi]

Do not fail if any of the requested subtypes are not availabe, but set the
number of resources to 0 and continue parsing the resource ranges.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/firmware/ti_sci.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index af3ebcdeab18..5d13ed724ff0 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -2783,6 +2783,7 @@ devm_ti_sci_get_of_resource(const struct ti_sci_handle *handle,
 	struct ti_sci_resource *res;
 	u32 resource_subtype;
 	int i, ret;
+	bool valid_set = false;
 
 	res = devm_kzalloc(dev, sizeof(*res), GFP_KERNEL);
 	if (!res)
@@ -2813,13 +2814,16 @@ devm_ti_sci_get_of_resource(const struct ti_sci_handle *handle,
 		if (ret) {
 			dev_err(dev, "dev = %d subtype %d not allocated for this host\n",
 				dev_id, resource_subtype);
-			return ERR_PTR(ret);
+			res->desc[i].start = 0;
+			res->desc[i].num = 0;
+			continue;
 		}
 
 		dev_dbg(dev, "dev = %d, subtype = %d, start = %d, num = %d\n",
 			dev_id, resource_subtype, res->desc[i].start,
 			res->desc[i].num);
 
+		valid_set = true;
 		res->desc[i].res_map =
 			devm_kzalloc(dev, BITS_TO_LONGS(res->desc[i].num) *
 				     sizeof(*res->desc[i].res_map), GFP_KERNEL);
@@ -2828,7 +2832,10 @@ devm_ti_sci_get_of_resource(const struct ti_sci_handle *handle,
 	}
 	raw_spin_lock_init(&res->lock);
 
-	return res;
+	if (valid_set)
+		return res;
+
+	return ERR_PTR(-EINVAL);
 }
 
 static int tisci_reboot_handler(struct notifier_block *nb, unsigned long mode,
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] firmware: ti_sci: Parse all resource ranges even if some is not available

[Lokesh Vutla]

On 04/06/19 3:40 PM, Peter Ujfalusi wrote:
> Do not fail if any of the requested subtypes are not availabe, but set the
> number of resources to 0 and continue parsing the resource ranges.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  drivers/firmware/ti_sci.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index af3ebcdeab18..5d13ed724ff0 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -2783,6 +2783,7 @@ devm_ti_sci_get_of_resource(const struct ti_sci_handle *handle,
>  	struct ti_sci_resource *res;
>  	u32 resource_subtype;
>  	int i, ret;
> +	bool valid_set = false;

Minor nit: Can you maintain the reverse Christmas tree here? It looks good :) No
strong feelings though

Other than that:
Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>

Thanks and regards,
Lokesh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] firmware: ti_sci: Parse all resource ranges even if some is not available

[Peter Ujfalusi]

On 04/06/2019 13.13, Lokesh Vutla wrote:
> 
> 
> On 04/06/19 3:40 PM, Peter Ujfalusi wrote:
>> Do not fail if any of the requested subtypes are not availabe, but set the
>> number of resources to 0 and continue parsing the resource ranges.
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> ---
>>  drivers/firmware/ti_sci.c | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
>> index af3ebcdeab18..5d13ed724ff0 100644
>> --- a/drivers/firmware/ti_sci.c
>> +++ b/drivers/firmware/ti_sci.c
>> @@ -2783,6 +2783,7 @@ devm_ti_sci_get_of_resource(const struct ti_sci_handle *handle,
>>  	struct ti_sci_resource *res;
>>  	u32 resource_subtype;
>>  	int i, ret;
>> +	bool valid_set = false;
> 
> Minor nit: Can you maintain the reverse Christmas tree here? It looks good :) No
> strong feelings though

bool no_valid_sets = true;

and when we have at least one valid set flip it to false? That's equally
twisted if not worst. imho

> 
> Other than that:
> Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>
> 
> Thanks and regards,
> Lokesh
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] firmware: ti_sci: Parse all resource ranges even if some is not available

[Lokesh Vutla]

On 04/06/19 3:51 PM, Peter Ujfalusi wrote:
> 
> 
> On 04/06/2019 13.13, Lokesh Vutla wrote:
>>
>>
>> On 04/06/19 3:40 PM, Peter Ujfalusi wrote:
>>> Do not fail if any of the requested subtypes are not availabe, but set the
>>> number of resources to 0 and continue parsing the resource ranges.
>>>
>>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>>> ---
>>>  drivers/firmware/ti_sci.c | 11 +++++++++--
>>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
>>> index af3ebcdeab18..5d13ed724ff0 100644
>>> --- a/drivers/firmware/ti_sci.c
>>> +++ b/drivers/firmware/ti_sci.c
>>> @@ -2783,6 +2783,7 @@ devm_ti_sci_get_of_resource(const struct ti_sci_handle *handle,
>>>  	struct ti_sci_resource *res;
>>>  	u32 resource_subtype;
>>>  	int i, ret;
>>> +	bool valid_set = false;
>>
>> Minor nit: Can you maintain the reverse Christmas tree here? It looks good :) No
>> strong feelings though
> 
> bool no_valid_sets = true;
> 
> and when we have at least one valid set flip it to false? That's equally
> twisted if not worst. imho

No no. I mean to add the change like below instead of adding the declaration at
the end.


--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -2383,6 +2383,7 @@ devm_ti_sci_get_of_resource(const struct ti_sci_handle
*handle,
 			    struct device *dev, u32 dev_id, char *of_prop)
 {
 	struct ti_sci_resource *res;
+	bool valid_set = false;
 	u32 resource_subtype;
 	int i, ret;

Thanks and regards,
Lokesh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] firmware: ti_sci: Parse all resource ranges even if some is not available

[Peter Ujfalusi]

On 04/06/2019 13.23, Lokesh Vutla wrote:
> 
> 
> On 04/06/19 3:51 PM, Peter Ujfalusi wrote:
>>
>>
>> On 04/06/2019 13.13, Lokesh Vutla wrote:
>>>
>>>
>>> On 04/06/19 3:40 PM, Peter Ujfalusi wrote:
>>>> Do not fail if any of the requested subtypes are not availabe, but set the
>>>> number of resources to 0 and continue parsing the resource ranges.
>>>>
>>>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>>>> ---
>>>>  drivers/firmware/ti_sci.c | 11 +++++++++--
>>>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
>>>> index af3ebcdeab18..5d13ed724ff0 100644
>>>> --- a/drivers/firmware/ti_sci.c
>>>> +++ b/drivers/firmware/ti_sci.c
>>>> @@ -2783,6 +2783,7 @@ devm_ti_sci_get_of_resource(const struct ti_sci_handle *handle,
>>>>  	struct ti_sci_resource *res;
>>>>  	u32 resource_subtype;
>>>>  	int i, ret;
>>>> +	bool valid_set = false;
>>>
>>> Minor nit: Can you maintain the reverse Christmas tree here? It looks good :) No
>>> strong feelings though
>>
>> bool no_valid_sets = true;
>>
>> and when we have at least one valid set flip it to false? That's equally
>> twisted if not worst. imho
> 
> No no. I mean to add the change like below instead of adding the declaration at
> the end.

Ah, sure. will send it asap

> 
> 
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -2383,6 +2383,7 @@ devm_ti_sci_get_of_resource(const struct ti_sci_handle
> *handle,
>  			    struct device *dev, u32 dev_id, char *of_prop)
>  {
>  	struct ti_sci_resource *res;
> +	bool valid_set = false;
>  	u32 resource_subtype;
>  	int i, ret;
> 
> Thanks and regards,
> Lokesh
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel