Re: [PATCH 2/3] soc: amlogic: Add support for Secure power domains controller

From: Jianxin Pan
Date: Fri Sep 20 2019 - 03:57:47 EST


Hi Martin,

On 2019/9/20 4:03, Martin Blumenstingl wrote:
> Hi Jianxin,
>
> I added three comments below from a quick glance at this driver (I
> didn't have time for a complete review)
>
> On Thu, Sep 19, 2019 at 2:11 PM Jianxin Pan <jianxin.pan@xxxxxxxxxxx> wrote:
> [...]
>> + pm_genpd_init(&dom->base, NULL,
>> + (match->domains[i].get_power ?
>> + match->domains[i].get_power(dom) : true));
> .get_power is never NULL in this driver so the ": true" part is
> effectively a no-op
>
OK, I will remove it. Thanks for your time.
> [...]
>> +static const struct of_device_id meson_secure_pwrc_match_table[] = {
>> + {
>> + .compatible = "amlogic,meson-a1-pwrc",
>> + .data = &meson_secure_a1_pwrc_data,
>> + },
>> + { }
> many drivers use a /* sentinel */ comment inside { }
>
OK, I will add this comment line.
> [...]
>> +arch_initcall_sync(meson_secure_pwrc_init);
> why arch_initcall_sync instead of builtin_platform_driver?
> $ grep -R arch_initcall_sync drivers/soc/
> $
>
>
> Martin
>
The power-domain is depended by many other drivers, arch_initcall_sync is used to make power-domain probe earlier.
Maybe I need to switch back to builtin_platform_driver when use APIs from meson_sm.c.
> .
>