From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 64260C433E7 for ; Tue, 13 Oct 2020 09:47:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1F54E2072D for ; Tue, 13 Oct 2020 09:47:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2404082AbgJMJri (ORCPT ); Tue, 13 Oct 2020 05:47:38 -0400 Received: from mail-oi1-f195.google.com ([209.85.167.195]:45914 "EHLO mail-oi1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727320AbgJMJri (ORCPT ); Tue, 13 Oct 2020 05:47:38 -0400 Received: by mail-oi1-f195.google.com with SMTP id j7so4276541oie.12; Tue, 13 Oct 2020 02:47:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=66uxKBaPY4Ovk1ktUOH3jIWixK/5hRqlez704bXhJTo=; b=gWGapRkLGR6bqyc2vCXxbhYwE209Hs8DI6fgs4gIhU9S5KM+522qE2cCd+PrzMybvO f3mmyBzakj6HKspZMwLMLQJPPwr978Q/2enYlZp4fkqEMao+P29wxILUld732Qgocc8B GHKhZwee8JwnZW7jZuzvZ9Osv+f1Z5ErMlfzfIHarxYdkbpMXVhJiNZkyPxVAs5vZsty /1UCjWMAUTaqmXbI+tzuAVqZjjxuWenlpjqlomJUZzWLiYdaQriqhTgxj7QBVvx8bxGX YmtbKp9BZsPTLhxMyWX7321VFPkthaP97/Qfsewvx09F63AWGHFIM/hllW3IsOHygIbB 4UDA== X-Gm-Message-State: AOAM530OvqcZ0R5HNUANCpTX/aYeiOQR6YWm/4oI38XRfdovVnp7mwah AtumV+D6cCIphkQGqex510tPDF3gBJ81jNURL1g= X-Google-Smtp-Source: ABdhPJwLN64T/gaMyS/i1LmR6BIkbfw+eTtQ5cF87nW2VWME3lCQ/eIpJO7UTvEJTaK46HDjetiRlRzz1vnmwuQ0rpY= X-Received: by 2002:aca:4441:: with SMTP id r62mr12567647oia.153.1602582456648; Tue, 13 Oct 2020 02:47:36 -0700 (PDT) MIME-Version: 1.0 References: <24ff92dd1b0ee1b802b45698520f2937418f8094.1598260050.git.viresh.kumar@linaro.org> In-Reply-To: From: Geert Uytterhoeven Date: Tue, 13 Oct 2020 11:47:25 +0200 Message-ID: Subject: Re: [PATCH V2 2/2] cpufreq: dt: Refactor initialization to handle probe deferral properly To: Viresh Kumar , Stephan Gerhold Cc: Ulf Hansson , "Rafael J. Wysocki" , Liam Girdwood , Mark Brown , Linux PM list , Vincent Guittot , Stephen Boyd , Nishanth Menon , nks@flawful.org, Georgi Djakov , Linux Kernel Mailing List , Wolfram Sang , Linux I2C , Linux-Renesas Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org Hi Viresh, Stephan, On Mon, Aug 24, 2020 at 11:12 AM Viresh Kumar wrote: > From: Stephan Gerhold > > cpufreq-dt is currently unable to handle -EPROBE_DEFER properly > because the error code is not propagated for the cpufreq_driver->init() > callback. Instead, it attempts to avoid the situation by temporarily > requesting all resources within resources_available() and releasing them > again immediately after. This has several disadvantages: > > - Whenever we add something like interconnect handling to the OPP core > we need to patch cpufreq-dt to request these resources early. > > - resources_available() is only run for CPU0, but other clusters may > eventually depend on other resources that are not available yet. > (See FIXME comment removed by this commit...) > > - All resources need to be looked up several times. > > Now that the OPP core can propagate -EPROBE_DEFER during initialization, > it would be nice to avoid all that trouble and just propagate its error > code when necessary. > > This commit refactors the cpufreq-dt driver to initialize private_data > before registering the cpufreq driver. We do this by iterating over > all possible CPUs and ensure that all resources are initialized: > > 1. dev_pm_opp_get_opp_table() ensures the OPP table is allocated > and initialized with clock and interconnects. > > 2. dev_pm_opp_set_regulators() requests the regulators and assigns > them to the OPP table. > > 3. We call dev_pm_opp_of_get_sharing_cpus() early so that we only > initialize the OPP table once for each shared policy. > > With these changes, we actually end up saving a few lines of code, > the resources are no longer looked up multiple times and everything > should be much more robust. > > Signed-off-by: Stephan Gerhold > [ Viresh: Use list_head structure for maintaining the list and minor > changes ] > Signed-off-by: Viresh Kumar Thanks for your patch, which is now commit dc279ac6e5b4e06e ("cpufreq: dt: Refactor initialization to handle probe deferral properly") in pm/linux-next, and to which I bisected a regression. Reverting this commit fixes the issue. On r8a7791/koelsch, during resume from s2ram: PM: suspend entry (deep) Filesystems sync: 0.000 seconds Freezing user space processes ... (elapsed 0.003 seconds) done. OOM killer disabled. Freezing remaining freezable tasks ... (elapsed 0.009 seconds) done. Disabling non-boot CPUs ... Enabling non-boot CPUs ... +i2c-sh_mobile e60b0000.i2c: Transfer request timed out +i2c-sh_mobile e60b0000.i2c: Transfer request timed out +i2c-sh_mobile e60b0000.i2c: Transfer request timed out +i2c-sh_mobile e60b0000.i2c: Transfer request timed out +i2c-sh_mobile e60b0000.i2c: Transfer request timed out +i2c-sh_mobile e60b0000.i2c: Transfer request timed out +i2c-sh_mobile e60b0000.i2c: Transfer request timed out +i2c-sh_mobile e60b0000.i2c: Transfer request timed out +i2c-sh_mobile e60b0000.i2c: Transfer request timed out +i2c-sh_mobile e60b0000.i2c: Transfer request timed out +i2c-sh_mobile e60b0000.i2c: Transfer request timed out +i2c-sh_mobile e60b0000.i2c: Transfer request timed out +cpu cpu0: OPP table can't be empty CPU1 is up rcar-pcie fe000000.pcie: PCIe x1: link up The cpufreq code tries to talk to the PMIC, while the I2C controller that hosts the PMIC is suspended, and thus any communication attempt times out. __i2c_check_suspended() fails to notice that, as the i2c_shmobile_i2c driver doesn't have a suspend callback calling i2c_mark_adapter_suspended() yet. After fixing that (will send a patch soon), the I2C core rightfully complains with: WARNING: CPU: 1 PID: 13 at drivers/i2c/i2c-core.h:54 __i2c_transfer+0x4a4/0x4e4 i2c i2c-6: Transfer while suspended CPU: 1 PID: 13 Comm: cpuhp/1 Not tainted 5.9.0-shmobile-09581-g05a3e5886c7615b1-dirty #718 Hardware name: Generic R-Car Gen2 (Flattened Device Tree) [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0x8c/0xac) [] (dump_stack) from [] (__warn+0xd0/0xe8) [] (__warn) from [] (warn_slowpath_fmt+0x70/0x9c) [] (warn_slowpath_fmt) from [] (__i2c_transfer+0x4a4/0x4e4) [] (__i2c_transfer) from [] (i2c_transfer+0xb0/0xf8) [] (i2c_transfer) from [] (regmap_i2c_read+0x54/0x88) [] (regmap_i2c_read) from [] (_regmap_raw_read+0x118/0x1f0) [] (_regmap_raw_read) from [] (_regmap_bus_read+0x44/0x68) [] (_regmap_bus_read) from [] (_regmap_read+0x84/0x110) [] (_regmap_read) from [] (regmap_read+0x40/0x58) [] (regmap_read) from [] (regulator_get_voltage_sel_regmap+0x28/0x74) [] (regulator_get_voltage_sel_regmap) from [] (regulator_get_voltage_rdev+0xa4/0x14c) [] (regulator_get_voltage_rdev) from [] (regulator_get_voltage+0x2c/0x60) [] (regulator_get_voltage) from [] (regulator_is_supported_voltage+0x30/0xd8) [] (regulator_is_supported_voltage) from [] (_opp_add+0x164/0x1b8) [] (_opp_add) from [] (_opp_add_v1+0x80/0xb8) [] (_opp_add_v1) from [] (dev_pm_opp_of_add_table+0x130/0x168) [] (dev_pm_opp_of_add_table) from [] (dev_pm_opp_of_cpumask_add_table+0x60/0xac) [] (dev_pm_opp_of_cpumask_add_table) from [] (cpufreq_init+0x94/0x1c4) [] (cpufreq_init) from [] (cpufreq_online+0x148/0x7ac) [] (cpufreq_online) from [] (cpuhp_cpufreq_online+0x8/0x10) [] (cpuhp_cpufreq_online) from [] (cpuhp_invoke_callback+0xf8/0x2e4) [] (cpuhp_invoke_callback) from [] (cpuhp_thread_fun+0xac/0x244) [] (cpuhp_thread_fun) from [] (smpboot_thread_fn+0x19c/0x1a8) [] (smpboot_thread_fn) from [] (kthread+0x104/0x110) [] (kthread) from [] (ret_from_fork+0x14/0x2c) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds