On 3.11.2023 13:29, Bryan O'Donoghue wrote:
> Moving the location of the hooks to VFE power domains has several
> advantages.
>
> 1. Separation of concerns and functional decomposition.
> vfe.c should be responsible for and know best how manage
> power-domains for a VFE, excising from camss.c follows this
> principle.
>
> 2. Embedding a pointer to genpd in struct camss_vfe{} meas that we can
> dispense with a bunch of kmalloc array inside of camss.c.
>
> 3. Splitting up titan top gdsc from vfe/ife gdsc provides a base for
> breaking up magic indexes in dtsi.
>
> Suggested-by: Matti Lehtimäki <matti.lehtimaki@xxxxxxxxx>
> Tested-by: Matti Lehtimäki <matti.lehtimaki@xxxxxxxxx>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>
> ---
[...]
> +/*
> + * msm_vfe_genpd_cleanup - Cleanup VFE genpd linkages
> + * @vfe: VFE device
You can even give this an upgrade to kerneldoc! :)
[...]
> + /* count the # of VFEs which have flagged power-domain */
[...]
Personal peeve, but this comment seems a bit excessive
> + for (vfepd_num = i = 0; i < camss->vfe_total_num; i++) {
> + if (res->vfe_res[i].has_pd)
> + vfepd_num++;
> + }
>
> - camss->genpd_link = devm_kmalloc_array(dev, camss->genpd_num,
> - sizeof(*camss->genpd_link),
> - GFP_KERNEL);
> - if (!camss->genpd_link)
> - return -ENOMEM;
> + /*
> + * If the number of power-domains is greater than the number of VFEs
> + * then the additional power-domain is for the entire CAMSS block the
> + * 'top' power-domain.
the last 3 words seem out of place
> + */
> + if (camss->genpd_num <= vfepd_num)
> + return 0;
if (!(camss->genpd_num > vfepd_num))
would probably be easier to follow given your comment above
Konrad
