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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DE0F8C433EF for ; Fri, 1 Jul 2022 08:11:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235408AbiGAILc (ORCPT ); Fri, 1 Jul 2022 04:11:32 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39922 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235028AbiGAIL2 (ORCPT ); Fri, 1 Jul 2022 04:11:28 -0400 Received: from mail-yb1-xb32.google.com (mail-yb1-xb32.google.com [IPv6:2607:f8b0:4864:20::b32]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1613A6F367 for ; Fri, 1 Jul 2022 01:11:26 -0700 (PDT) Received: by mail-yb1-xb32.google.com with SMTP id o19so2769705ybg.2 for ; Fri, 01 Jul 2022 01:11:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=hHlkuZMmnoq/Fcx5WJPgqnbubzspv6b5yJUWnWtjBFU=; b=ef3c0Q9NjuflyQ+MqqXmFYsu6VqqoBDzl3Rn9YOZ6BtoUOKK35qOxdgC8QR/fTDybG VqqyO8E0kSnvEPQLWxHMS1kOy7d31CPndv9IcUcD1PZLZYa4nQ8pJrAPz/ENotu6Fg+T cv1U55Rkpgay9b5N9Cz/uaM1OcWmVrSoIxTsrnEXfFYEuj/E7nl1nLRX5Jmy3jh1toiX m6K3sNRCdCrEU4AMi4upx2V2xs8eSYC2ucM/rUpwcUaeacdhm6JFJPOXHzwhz9f44Iz2 TiArY9oQLDmL0lT1Yba9o0E0/3CCha39GqTmMcxgRbms+W6WPbfZ6/kPtFAZ3gzJXLrq H1MQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=hHlkuZMmnoq/Fcx5WJPgqnbubzspv6b5yJUWnWtjBFU=; b=sy1Sj1tl0Ki0jYhRoLW2Z8+yxtFcwLUlRLXmo+Bv/bZruhgATnoceUpiXaW4wqnipn zPA8q7hbROZDa8nXxDXmmR1HbrLc8m9s6am/vb4M28pnkDWoo+ZeIl5f/S3U1iepoTSg qdoqy7hwKNd4TXoJgs4RdEcjefXyLI99YYL/btKOsWtd2eVPJJFINCMXY1G9+DfxfppL JUCBByiYoUMVTNPLU0i6A/jEn1/I1G6iCBK73UxL4/vljfY7XeNr9rSxJsCeb89CDnIS bMMoZEt1nkwqrO0oqQJ1DdsSHEKlVzK6RR92G3cB4LJQk6whbHC6PSNOmNdS1w9ydzer YWTw== X-Gm-Message-State: AJIora9RFeSpt9FuC3Hn0Pvw6gKK+eGzNeu7MWbyf1AWXe3xV/mPRP0P DQm+9+kYyJuYFAgu4o3LXEuFlL03FJX3Q9woi7ndKA== X-Google-Smtp-Source: AGRyM1vDgmfSwZquyidL3tNXvh2BQFGuYMc3TLjl4/SGBSIpusjC3N4lyjRUlXNEzsK9FZ7zCyuWpHbcQKPzr3oaKYo= X-Received: by 2002:a05:6902:1544:b0:66d:3948:deae with SMTP id r4-20020a056902154400b0066d3948deaemr14505784ybu.530.1656663085093; Fri, 01 Jul 2022 01:11:25 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Saravana Kannan Date: Fri, 1 Jul 2022 01:10:48 -0700 Message-ID: Subject: Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state() To: Tony Lindgren Cc: Rob Herring , Geert Uytterhoeven , Greg Kroah-Hartman , "Rafael J. Wysocki" , Kevin Hilman , Ulf Hansson , Len Brown , Pavel Machek , Joerg Roedel , Will Deacon , Andrew Lunn , Heiner Kallweit , Russell King , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Linus Walleij , Hideaki YOSHIFUJI , David Ahern , Android Kernel Team , "linux-kernel@vger.kernel.org" , "open list:THERMAL" , Linux IOMMU , netdev , "open list:GPIO SUBSYSTEM" , Alexander Stein Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jun 30, 2022 at 11:12 PM Tony Lindgren wrote: > > * Tony Lindgren [220701 08:33]: > > * Saravana Kannan [220630 23:25]: > > > On Thu, Jun 30, 2022 at 4:26 PM Rob Herring wrote: > > > > > > > > On Thu, Jun 30, 2022 at 5:11 PM Saravana Kannan wrote: > > > > > > > > > > On Mon, Jun 27, 2022 at 2:10 AM Tony Lindgren wrote: > > > > > > > > > > > > * Saravana Kannan [220623 08:17]: > > > > > > > On Thu, Jun 23, 2022 at 12:01 AM Tony Lindgren wrote: > > > > > > > > > > > > > > > > * Saravana Kannan [220622 19:05]: > > > > > > > > > On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren wrote: > > > > > > > > > > This issue is no directly related fw_devlink. It is a side effect of > > > > > > > > > > removing driver_deferred_probe_check_state(). We no longer return > > > > > > > > > > -EPROBE_DEFER at the end of driver_deferred_probe_check_state(). > > > > > > > > > > > > > > > > > > Yes, I understand the issue. But driver_deferred_probe_check_state() > > > > > > > > > was deleted because fw_devlink=on should have short circuited the > > > > > > > > > probe attempt with an -EPROBE_DEFER before reaching the bus/driver > > > > > > > > > probe function and hitting this -ENOENT failure. That's why I was > > > > > > > > > asking the other questions. > > > > > > > > > > > > > > > > OK. So where is the -EPROBE_DEFER supposed to happen without > > > > > > > > driver_deferred_probe_check_state() then? > > > > > > > > > > > > > > device_links_check_suppliers() call inside really_probe() would short > > > > > > > circuit and return an -EPROBE_DEFER if the device links are created as > > > > > > > expected. > > > > > > > > > > > > OK > > > > > > > > > > > > > > Hmm so I'm not seeing any supplier for the top level ocp device in > > > > > > > > the booting case without your patches. I see the suppliers for the > > > > > > > > ocp child device instances only. > > > > > > > > > > > > > > Hmmm... this is strange (that the device link isn't there), but this > > > > > > > is what I suspected. > > > > > > > > > > > > Yup, maybe it's because of the supplier being a device in the child > > > > > > interconnect for the ocp. > > > > > > > > > > Ugh... yeah, this is why the normal (not SYNC_STATE_ONLY) device link > > > > > isn't being created. > > > > > > > > > > So the aggregated view is something like (I had to set tabs = 4 space > > > > > to fit it within 80 cols): > > > > > > > > > > ocp: ocp { <========================= Consumer > > > > > compatible = "simple-pm-bus"; > > > > > power-domains = <&prm_per>; <=========== Supplier ref > > > > > > > > > > l4_wkup: interconnect@44c00000 { > > > > > compatible = "ti,am33xx-l4-wkup", "simple-pm-bus"; > > > > > > > > > > segment@200000 { /* 0x44e00000 */ > > > > > compatible = "simple-pm-bus"; > > > > > > > > > > target-module@0 { /* 0x44e00000, ap 8 58.0 */ > > > > > compatible = "ti,sysc-omap4", "ti,sysc"; > > > > > > > > > > prcm: prcm@0 { > > > > > compatible = "ti,am3-prcm", "simple-bus"; > > > > > > > > > > prm_per: prm@c00 { <========= Actual Supplier > > > > > compatible = "ti,am3-prm-inst", "ti,omap-prm-inst"; > > > > > }; > > > > > }; > > > > > }; > > > > > }; > > > > > }; > > > > > }; > > > > > > > > > > The power-domain supplier is the great-great-great-grand-child of the > > > > > consumer. It's not clear to me how this is valid. What does it even > > > > > mean? > > > > > > > > > > Rob, is this considered a valid DT? > > > > > > > > Valid DT for broken h/w. > > > > > > I'm not sure even in that case it's valid. When the parent device is > > > in reset (when the SoC is coming out of reset), there's no way the > > > descendant is functional. And if the descendant is not functional, how > > > is the parent device powered up? This just feels like an incorrect > > > representation of the real h/w. > > > > It should be correct representation based on scanning the interconnects > > and looking at the documentation. Some interconnect parts are wired > > always-on and some interconnect instances may be dual-mapped. Thanks for helping to debug this. Appreciate it. > > > > We have a quirk to probe prm/prcm first with pdata_quirks_init_clocks(). :'( I checked out the code. These prm devices just get populated with NULL as the parent. So they are effectively top level devices from the perspective of driver core. > > Maybe that also now fails in addition to the top level interconnect > > probing no longer producing -EPROBE_DEFER. As far as I can tell pdata_quirks_init_clocks() is just adding these prm devices (amongst other drivers). So I don't expect that to fail. > > > > > > So the domain must be default on and then simple-pm-bus is going to > > > > hold a reference to the domain preventing it from ever getting powered > > > > off and things seem to work. Except what happens during suspend? > > > > > > But how can simple-pm-bus even get a reference? The PM domain can't > > > get added until we are well into the probe of the simple-pm-bus and > > > AFAICT the genpd attach is done before the driver probe is even > > > called. > > > > The prm/prcm gets of_platform_populate() called on it early. :'( > The hackish patch below makes things boot for me, not convinced this > is the preferred fix compared to earlier deferred probe handling though. > Going back to the init level tinkering seems like a step back to me. The goal of fw_devlink is to avoid init level tinkering and it does help with that in general. But these kinds of quirks are going to need a few exceptions -- with them being quirks and all. And this change will avoid an unnecessary deferred probe (that used to happen even before my change). The other option to handle this quirk is to create the invalid (consumer is parent of supplier) fwnode_link between the prm device and its consumers when the prm device is populated. Then fw_devlink will end up creating a device link when ocp gets added. But I'm not sure if it's going to be easy to find and add all those consumers. I'd say, for now, let's go with this patch below. I'll see if I can get fw_devlink to handle these odd quirks without breaking the normal cases or making them significantly slower. But that'll take some time and I'm not sure there'll be a nice solution. Thanks, Saravana > Regards, > > Tony > > 8< ---------------- > diff --git a/drivers/soc/ti/omap_prm.c b/drivers/soc/ti/omap_prm.c > --- a/drivers/soc/ti/omap_prm.c > +++ b/drivers/soc/ti/omap_prm.c > @@ -991,4 +991,9 @@ static struct platform_driver omap_prm_driver = { > .of_match_table = omap_prm_id_table, > }, > }; > -builtin_platform_driver(omap_prm_driver); > + > +static int __init omap_prm_init(void) > +{ > + return platform_driver_register(&omap_prm_driver); > +} > +subsys_initcall(omap_prm_init); > -- > 2.36.1 > > -- > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com. >