Re: [PATCH v2 2/2] PM / devfreq: mediatek: Introduce MediaTek CCI devfreq driver

Krzysztof Kozlowski
2022-04-08T08:21:40Z

On 08/04/2022 07:21, Johnson Wang wrote:
> We introduce a devfreq driver for the MediaTek Cache Coherent Interconnect
> (CCI) used by some MediaTek SoCs.
>
(...)

> index 87eb2b837e68..d985597f343f 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -120,6 +120,16 @@ config ARM_TEGRA_DEVFREQ
> It reads ACTMON counters of memory controllers and adjusts the
> operating frequencies and voltages with OPP support.
>
> +config ARM_MEDIATEK_CCI_DEVFREQ
> + tristate "MEDIATEK CCI DEVFREQ Driver"
> + depends on ARM_MEDIATEK_CPUFREQ

|| COMPILE_TEST? And check if it test compiles.

(...)

> +
> +static int mtk_ccifreq_remove(struct platform_device *pdev)
> +{
> + struct device *cci_dev = &pdev->dev;

s/cci_dev/dev/
Everywhere.

> + struct mtk_ccifreq_drv *drv;
> +
> + drv = platform_get_drvdata(pdev);
> +
> + dev_pm_opp_unregister_notifier(cci_dev, &drv->opp_nb);
> + dev_pm_opp_of_remove_table(cci_dev);
> + clk_disable_unprepare(drv->inter_clk);
> + clk_disable_unprepare(drv->cci_clk);
> + regulator_disable(drv->proc_reg);
> + if (!IS_ERR(drv->sram_reg))
> + regulator_disable(drv->sram_reg);
> +
> + return 0;
> +}
> +
> +static const struct mtk_ccifreq_platform_data mt8183_platform_data = {
> + .min_volt_shift = 100000,
> + .max_volt_shift = 200000,
> + .proc_max_volt = 1150000,
> + .sram_min_volt = 0,
> + .sram_max_volt = 1150000,
> +};
> +
> +static const struct mtk_ccifreq_platform_data mt8186_platform_data = {
> + .min_volt_shift = 100000,
> + .max_volt_shift = 250000,
> + .proc_max_volt = 1118750,
> + .sram_min_volt = 850000,
> + .sram_max_volt = 1118750,
> +};
> +
> +static const struct of_device_id mtk_ccifreq_machines[] = {
> + { .compatible = "mediatek,mt8183-cci", .data = &mt8183_platform_data },
> + { .compatible = "mediatek,mt8186-cci", .data = &mt8186_platform_data },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, mtk_ccifreq_machines);
> +
> +static struct platform_driver mtk_ccifreq_platdrv = {
> + .probe = mtk_ccifreq_probe,
> + .remove = mtk_ccifreq_remove,
> + .driver = {
> + .name = "mtk-ccifreq",
> + .of_match_table = of_match_ptr(mtk_ccifreq_machines),

You use of_match_ptr() so is it possible to build it without OF? If so,
then mtk_ccifreq_machines needs maybe_unused.

> + },
> +};
> +
> +static int __init mtk_ccifreq_platdrv_init(void)
> +{
> + return platform_driver_register(&mtk_ccifreq_platdrv);
> +}
> +module_init(mtk_ccifreq_platdrv_init)
> +
> +static void __exit mtk_ccifreq_platdrv_exit(void)
> +{
> + platform_driver_unregister(&mtk_ccifreq_platdrv);
> +}
> +module_exit(mtk_ccifreq_platdrv_exit)

Why not module_platform_driver()?

> +
> +MODULE_DESCRIPTION("MediaTek CCI devfreq driver");
> +MODULE_AUTHOR("Jia-Wei Chang <jia-wei.chang@mediatek.com>");
> +MODULE_LICENSE("GPL v2");

Best regards,
Krzysztof