On 04/04/2024 09:38, Anand Moon wrote: > Hi Krzysztof, > > On Thu, 4 Apr 2024 at 12:53, Krzysztof Kozlowski > <krzysztof.kozlowski@xxxxxxxxxx> wrote: >> >> On 04/04/2024 09:13, Anand Moon wrote: >>> Use devm_regulator_bulk_get_enable() instead of open coded >>> 'devm_regulator_get(), regulator_enable(), regulator_disable(). >> >> I fail to see how did you replace open-coded suspend/resume paths. >> >>> >>> Signed-off-by: Anand Moon <linux.amoon@xxxxxxxxx> >>> --- >>> V2: no changes, did not find any regression in pm suspend/resume. >> >> No, that's not equivalent code. No explanation in commit msg. >> >> You already got comments on this and nothing improved. You just entirely >> ignored received comments. That's not how it works. >> >> I don't think you understand the code and Linux driver model. This patch >> repeats several previous attempts with similar issues: no logic behind a >> change. >> >> NAK. > > devm_regulator_get_enable and devm_regulator_bulk_get_enable > both remove the dependency from the driver to handle the regulator_enabled > and regulator_disabled. ie this removes the regulator from the driver structure. Not true. Please do not paste some generic knowledge and assume reviewer knows it. Instead provide proof. > > Since these functions set devm_add_action to disable the regulator when the > resource is not used. > > ret = devm_add_action(dev, devm_regulator_bulk_disable, devres); > if (!ret) > return 0; Listen, you already got comments on this at v1. Address previous comments instead of repeating something unrelated. We should not have the same discussion twice. Best regards, Krzysztof