| Message ID | 20220408052150.22536-1-johnson.wang@mediatek.com |
|---|---|
| Headers | show |
| Series | Introduce MediaTek CCI devfreq driver | expand |
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
Il 08/04/22 07:21, Johnson Wang ha scritto: > We introduce a devfreq driver for the MediaTek Cache Coherent Interconnect > (CCI) used by some MediaTek SoCs. > > In this driver, we use the passive devfreq driver to get target frequencies > and adjust voltages accordingly. In MT8183 and MT8186, the MediaTek CCI > is supplied by the same regulators with the little core CPUs. > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > --- > This patch depends on "devfreq-testing"[1]. > [1]https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing > --- > drivers/devfreq/Kconfig | 10 + > drivers/devfreq/Makefile | 1 + > drivers/devfreq/mtk-cci-devfreq.c | 479 ++++++++++++++++++++++++++++++ > 3 files changed, 490 insertions(+) > create mode 100644 drivers/devfreq/mtk-cci-devfreq.c > > diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig > 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 > + select DEVFREQ_GOV_PASSIVE > + help > + This adds a devfreq driver for MediaTek Cache Coherent Interconnect > + which is shared the same regulators with the cpu cluster. It can track > + buck voltages and update a proper CCI frequency. Use the notification > + to get the regulator status. > + > config ARM_RK3399_DMC_DEVFREQ > tristate "ARM RK3399 DMC DEVFREQ Driver" > depends on (ARCH_ROCKCHIP && HAVE_ARM_SMCCC) || \ > diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile > index 0b6be92a25d9..bf40d04928d0 100644 > --- a/drivers/devfreq/Makefile > +++ b/drivers/devfreq/Makefile > @@ -11,6 +11,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += governor_passive.o > obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o > obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ) += imx-bus.o > obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o > +obj-$(CONFIG_ARM_MEDIATEK_CCI_DEVFREQ) += mtk-cci-devfreq.o > obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o > obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ) += sun8i-a33-mbus.o > obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30-devfreq.o > diff --git a/drivers/devfreq/mtk-cci-devfreq.c b/drivers/devfreq/mtk-cci-devfreq.c > new file mode 100644 > index 000000000000..53a28e2c88bd > --- /dev/null > +++ b/drivers/devfreq/mtk-cci-devfreq.c > @@ -0,0 +1,479 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2022 MediaTek Inc. > + */ > + > +#include <linux/clk.h> > +#include <linux/devfreq.h> > +#include <linux/minmax.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/pm_opp.h> > +#include <linux/regulator/consumer.h> > + > +struct mtk_ccifreq_platform_data { > + int min_volt_shift; > + int max_volt_shift; > + int proc_max_volt; > + int sram_min_volt; > + int sram_max_volt; > +}; > + > +struct mtk_ccifreq_drv { > + struct device *cci_dev; > + struct devfreq *devfreq; > + struct regulator *proc_reg; > + struct regulator *sram_reg; > + struct clk *cci_clk; > + struct clk *inter_clk; > + int inter_voltage; > + int old_voltage; > + unsigned long old_freq; > + bool need_voltage_tracking; > + /* Avoid race condition for regulators between notify and policy */ > + struct mutex reg_lock; > + struct notifier_block opp_nb; > + const struct mtk_ccifreq_platform_data *soc_data; > +}; > + > +static int mtk_ccifreq_voltage_tracking(struct mtk_ccifreq_drv *drv, > + int new_voltage) > +{ > + const struct mtk_ccifreq_platform_data *soc_data = drv->soc_data; > + struct device *dev = drv->cci_dev; > + struct regulator *proc_reg = drv->proc_reg; > + struct regulator *sram_reg = drv->sram_reg; > + int old_voltage, old_vsram, new_vsram, vsram, voltage, ret; > + > + old_voltage = regulator_get_voltage(proc_reg); > + if (old_voltage < 0) { > + dev_err(dev, "invalid vproc value: %d\n", old_voltage); > + return old_voltage; > + } > + > + old_vsram = regulator_get_voltage(sram_reg); > + if (old_vsram < 0) { > + dev_err(dev, "invalid vsram value: %d\n", old_vsram); > + return old_vsram; > + } > + > + new_vsram = clamp(new_voltage + soc_data->min_volt_shift, > + soc_data->sram_min_volt, soc_data->sram_max_volt); > + > + do { > + if (old_voltage <= new_voltage) { > + vsram = clamp(old_voltage + soc_data->max_volt_shift, > + soc_data->sram_min_volt, new_vsram); > + ret = regulator_set_voltage(sram_reg, vsram, > + soc_data->sram_max_volt); > + if (ret) > + return ret; > + > + if (vsram == soc_data->sram_max_volt || > + new_vsram == soc_data->sram_min_volt) > + voltage = new_voltage; > + else > + voltage = vsram - soc_data->min_volt_shift; > + > + ret = regulator_set_voltage(proc_reg, voltage, > + soc_data->proc_max_volt); > + if (ret) { > + regulator_set_voltage(sram_reg, old_vsram, > + soc_data->sram_max_volt); > + return ret; > + } > + } else if (old_voltage > new_voltage) { > + voltage = max(new_voltage, > + old_vsram - soc_data->max_volt_shift); > + ret = regulator_set_voltage(proc_reg, voltage, > + soc_data->proc_max_volt); > + if (ret) > + return ret; > + > + if (voltage == new_voltage) > + vsram = new_vsram; > + else > + vsram = max(new_vsram, > + voltage + soc_data->min_volt_shift); > + > + ret = regulator_set_voltage(sram_reg, vsram, > + soc_data->sram_max_volt); > + if (ret) { > + regulator_set_voltage(proc_reg, old_voltage, > + soc_data->proc_max_volt); > + return ret; > + } > + } > + > + old_voltage = voltage; > + old_vsram = vsram; > + } while (voltage != new_voltage || vsram != new_vsram); Hello Johnson, are you extremely sure that there will *always* be a way out of this while loop? For safety purposes, I would set an iteration limit in order to avoid getting an infinite loop here. Probably, something like twice or thrice the expected number of iterations will also be fine. P.S.: Krzysztof's review also contains exactly all the rest of what I would also say here (thanks!). Regards, Angelo
Hi Krzysztof, On Fri, 2022-04-08 at 10:21 +0200, Krzysztof Kozlowski wrote: > 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. Thank you for your review. I will add COMPILE_TEST and see if this driver is compiled with it. > > (...) > > > + > > +static int mtk_ccifreq_remove(struct platform_device *pdev) > > +{ > > + struct device *cci_dev = &pdev->dev; > > s/cci_dev/dev/ > Everywhere. > Sure, I will modify it in the next version. > > + 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. > No, this driver must be built with OF due to our CPU arch. Should I add some dependencies in Kconfig to ensure OF is enabled? > > + }, > > +}; > > + > > +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()? I will try to use module_platform_driver() and update it in the next version. > > > + > > +MODULE_DESCRIPTION("MediaTek CCI devfreq driver"); > > +MODULE_AUTHOR("Jia-Wei Chang <jia-wei.chang@mediatek.com>"); > > +MODULE_LICENSE("GPL v2"); > > > Best regards, > Krzysztof BRs, Johnson Wang
On 12/04/2022 10:39, Johnson Wang wrote: >>> +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. >> > > No, this driver must be built with OF due to our CPU arch. COMPILE_TEST does not use your arch... There are stubs for most of of-like functions, so !OF should build anyway. > Should I add some dependencies in Kconfig to ensure OF is enabled? There are different ways to solve it and it depends on what goal do you want to achieve? One ways is to use maybe_unused and of_match_ptr. Other way is to do not use them (skip both). Best regards, Krzysztof
On Fri, 2022-04-08 at 13:51 +0200, AngeloGioacchino Del Regno wrote: > Il 08/04/22 07:21, Johnson Wang ha scritto: > > We introduce a devfreq driver for the MediaTek Cache Coherent > > Interconnect > > (CCI) used by some MediaTek SoCs. > > > > In this driver, we use the passive devfreq driver to get target > > frequencies > > and adjust voltages accordingly. In MT8183 and MT8186, the MediaTek > > CCI > > is supplied by the same regulators with the little core CPUs. > > > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > > --- > > This patch depends on "devfreq-testing"[1]. > > [1] > > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing__;!!CTRNKA9wMg0ARbw!zNCF7FI5uFmzlA1V1-Pp8ht2HtUk8_oRRDoqzzBcXm0Mo8JOOoVbPPqa5xg4WuYPnKNF$ > > > > --- > > drivers/devfreq/Kconfig | 10 + > > drivers/devfreq/Makefile | 1 + > > drivers/devfreq/mtk-cci-devfreq.c | 479 > > ++++++++++++++++++++++++++++++ > > 3 files changed, 490 insertions(+) > > create mode 100644 drivers/devfreq/mtk-cci-devfreq.c > > > > diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig > > 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 > > + select DEVFREQ_GOV_PASSIVE > > + help > > + This adds a devfreq driver for MediaTek Cache Coherent > > Interconnect > > + which is shared the same regulators with the cpu cluster. It > > can track > > + buck voltages and update a proper CCI frequency. Use the > > notification > > + to get the regulator status. > > + > > config ARM_RK3399_DMC_DEVFREQ > > tristate "ARM RK3399 DMC DEVFREQ Driver" > > depends on (ARCH_ROCKCHIP && HAVE_ARM_SMCCC) || \ > > diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile > > index 0b6be92a25d9..bf40d04928d0 100644 > > --- a/drivers/devfreq/Makefile > > +++ b/drivers/devfreq/Makefile > > @@ -11,6 +11,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += > > governor_passive.o > > obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o > > obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ) += imx-bus.o > > obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o > > +obj-$(CONFIG_ARM_MEDIATEK_CCI_DEVFREQ) += mtk-cci-devfreq.o > > obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o > > obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ) += sun8i-a33-mbus.o > > obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30-devfreq.o > > diff --git a/drivers/devfreq/mtk-cci-devfreq.c > > b/drivers/devfreq/mtk-cci-devfreq.c > > new file mode 100644 > > index 000000000000..53a28e2c88bd > > --- /dev/null > > +++ b/drivers/devfreq/mtk-cci-devfreq.c > > @@ -0,0 +1,479 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (C) 2022 MediaTek Inc. > > + */ > > + > > +#include <linux/clk.h> > > +#include <linux/devfreq.h> > > +#include <linux/minmax.h> > > +#include <linux/module.h> > > +#include <linux/of.h> > > +#include <linux/of_device.h> > > +#include <linux/platform_device.h> > > +#include <linux/pm_opp.h> > > +#include <linux/regulator/consumer.h> > > + > > +struct mtk_ccifreq_platform_data { > > + int min_volt_shift; > > + int max_volt_shift; > > + int proc_max_volt; > > + int sram_min_volt; > > + int sram_max_volt; > > +}; > > + > > +struct mtk_ccifreq_drv { > > + struct device *cci_dev; > > + struct devfreq *devfreq; > > + struct regulator *proc_reg; > > + struct regulator *sram_reg; > > + struct clk *cci_clk; > > + struct clk *inter_clk; > > + int inter_voltage; > > + int old_voltage; > > + unsigned long old_freq; > > + bool need_voltage_tracking; > > + /* Avoid race condition for regulators between notify and > > policy */ > > + struct mutex reg_lock; > > + struct notifier_block opp_nb; > > + const struct mtk_ccifreq_platform_data *soc_data; > > +}; > > + > > +static int mtk_ccifreq_voltage_tracking(struct mtk_ccifreq_drv > > *drv, > > + int new_voltage) > > +{ > > + const struct mtk_ccifreq_platform_data *soc_data = drv- > > >soc_data; > > + struct device *dev = drv->cci_dev; > > + struct regulator *proc_reg = drv->proc_reg; > > + struct regulator *sram_reg = drv->sram_reg; > > + int old_voltage, old_vsram, new_vsram, vsram, voltage, ret; > > + > > + old_voltage = regulator_get_voltage(proc_reg); > > + if (old_voltage < 0) { > > + dev_err(dev, "invalid vproc value: %d\n", old_voltage); > > + return old_voltage; > > + } > > + > > + old_vsram = regulator_get_voltage(sram_reg); > > + if (old_vsram < 0) { > > + dev_err(dev, "invalid vsram value: %d\n", old_vsram); > > + return old_vsram; > > + } > > + > > + new_vsram = clamp(new_voltage + soc_data->min_volt_shift, > > + soc_data->sram_min_volt, soc_data- > > >sram_max_volt); > > + > > + do { > > + if (old_voltage <= new_voltage) { > > + vsram = clamp(old_voltage + soc_data- > > >max_volt_shift, > > + soc_data->sram_min_volt, > > new_vsram); > > + ret = regulator_set_voltage(sram_reg, vsram, > > + soc_data- > > >sram_max_volt); > > + if (ret) > > + return ret; > > + > > + if (vsram == soc_data->sram_max_volt || > > + new_vsram == soc_data->sram_min_volt) > > + voltage = new_voltage; > > + else > > + voltage = vsram - soc_data- > > >min_volt_shift; > > + > > + ret = regulator_set_voltage(proc_reg, voltage, > > + soc_data- > > >proc_max_volt); > > + if (ret) { > > + regulator_set_voltage(sram_reg, > > old_vsram, > > + soc_data- > > >sram_max_volt); > > + return ret; > > + } > > + } else if (old_voltage > new_voltage) { > > + voltage = max(new_voltage, > > + old_vsram - soc_data- > > >max_volt_shift); > > + ret = regulator_set_voltage(proc_reg, voltage, > > + soc_data- > > >proc_max_volt); > > + if (ret) > > + return ret; > > + > > + if (voltage == new_voltage) > > + vsram = new_vsram; > > + else > > + vsram = max(new_vsram, > > + voltage + soc_data- > > >min_volt_shift); > > + > > + ret = regulator_set_voltage(sram_reg, vsram, > > + soc_data- > > >sram_max_volt); > > + if (ret) { > > + regulator_set_voltage(proc_reg, > > old_voltage, > > + soc_data- > > >proc_max_volt); > > + return ret; > > + } > > + } > > + > > + old_voltage = voltage; > > + old_vsram = vsram; > > + } while (voltage != new_voltage || vsram != new_vsram); > > Hello Johnson, > > are you extremely sure that there will *always* be a way out of this > while loop? > > For safety purposes, I would set an iteration limit in order to avoid > getting > an infinite loop here. > Probably, something like twice or thrice the expected number of > iterations will > also be fine. > > P.S.: Krzysztof's review also contains exactly all the rest of what I > would > also say here (thanks!). > > Regards, > Angelo Hello Angelo, Thanks for your suggestion! Actually, we are going to add an iteration limit inside the while loop. BRs, Johnson Wang