| Message ID | 20190604101000.6741-1-peter.ujfalusi@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | firmware: ti_sci: Parse all resource ranges even if some is not available | expand |
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
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
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
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
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,
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(-)