Messages in this thread | | | From | Nicolas Boichat <> | Date | Tue, 27 Oct 2020 08:19:00 +0800 | Subject | Re: [PATCH v2 02/12] soc: mediatek: Add MediaTek SCPSYS power domains |
| |
Hi Enric,
On Mon, Oct 26, 2020 at 11:17 PM Enric Balletbo i Serra <enric.balletbo@collabora.com> wrote: > > Hi Nicolas, > > Many thanks for looking at this.
Thanks to you ,-)
[snip] > >> + if (id >= scpsys->soc_data->num_domains) { > >> + dev_err_probe(scpsys->dev, -EINVAL, "%pOFn: invalid domain id %d\n", node, id); > >> + return -EINVAL; > >> + } > >> + > >> + domain_data = &scpsys->soc_data->domains[id]; > >> + if (!domain_data) { > > > > Is that even possible at all? I mean, even if > > scpsys->soc_data->domains is NULL, as long as id != 0, this will no > > happen. > > > > I think could happen with a bad DT definition. I.e if for the definition of the > MT8173 domains you use a wrong value for the reg property, a value that is not > present in the SoC data. It is unlikely if you use the defines but could happen > if you hardcore the value. We cannot check this with the DT json-schema.
I wasn't clear in my explanation, and looking further there is more that looks wrong.
This expression &scpsys->soc_data->domains[id] is a pointer to element "id" of the array domains. So if you convert to integer arithmetic, it'll be something like `(long)scpsys->soc_data->domains + (sizeof(struct generic_pm_domain *)) * id`. The only way this can be NULL is if scpsys->soc_data->domains pointer is NULL, which, actually, can't really happen as it's the 5th element of a struct scpsys structure `(long)scpsys->soc_data + offset_of(domains, struct scpsys) + (sizeof(struct generic_pm_domain *)) * id`.
I think what you mean is either: domain_data = &scpsys->soc_data->domains[id]; if (!*domain_data) [but then domain_data type should be `struct generic_pm_domain **`? Does your code compile with warnings enabled?] or: domain_data = scpsys->soc_data->domains[id]; if (!domain_data) [then the test makes sense]
[snip]
| |