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=-3.8 required=3.0 tests=DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS 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 A88D8C3A5A9 for ; Mon, 4 May 2020 19:28:52 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 767792068E for ; Mon, 4 May 2020 19:28:52 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="Wm+BQom9"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="Ray7E97U" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 767792068E Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Ego2Jk28njolaVuMDPa9OvGcYTYYronZdSKm8Nolg8k=; b=Wm+BQom91GrbAR e7R9hlUqgMNFf+b8qXITBeF/CoanNyzTT6GB5b0ANDJ3gWvsitgpOylWe8QQDQOfnwLRL9qciMUEb lwv+Z8I3FFGY0fXFbto1Sndk//HHIdX2eCfqzVQD7lJFt9ICG6Gpj6OW7QQeP1tBp5L1zAt3dZLgC kfaN1gp9dkhEWX6GsiH/yzoUsQe4j0Q8GX4WjA9MvV/LFHrlBSNtkk5ryyaOsXkfuZMmxs+IEJL9p P1vftA8TYLBihHYphLMlGLwTLfRQ/hNVpKOlHFFN328h5a4B/ecZZWLkHr83Sj582ezlQHW7A4+jQ DSg30fTtCacPGNxM4CvQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jVgln-0003I5-Fo; Mon, 04 May 2020 19:28:51 +0000 Received: from mail-oi1-x243.google.com ([2607:f8b0:4864:20::243]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jVglj-0003Gc-Tc for linux-arm-kernel@lists.infradead.org; Mon, 04 May 2020 19:28:49 +0000 Received: by mail-oi1-x243.google.com with SMTP id r66so7713291oie.5 for ; Mon, 04 May 2020 12:28:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=65ayVsqDJjnjdKZdEBc4q9fksAu1k7B3hVdHXf6w4xA=; b=Ray7E97U6IyHegSOG4Ruo/s0VBkPr2xpmwYB1NPsl0+AD6p9kV8obryVwWSXC6LbaE gz5Tc1tuIzTw+HzG9scdSre9jzpDqVve7CzBYC39Q6u9Cnt4uEAKsW6Pkz3sMbA9PTCf IY9t5pkC4P8NxSmiLSWBdj2jvDVFcfU++zfnlKEHfLyVGB8cX3ZHyD52QFrF9bMX4/9t ns5ba1JiS9acZYSwvnnQ5kQxm9Nhb/wAwOkqRi3cF9EA6tI/DmMERPM7DycYETPDlQxI LvxzPQ8A+rLGHP+w2zNHWHZ4qfc5R+tyZuyP6tP7w3vpfjluYoSzzsM/muoxDWcc2D+w +ACg== 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=65ayVsqDJjnjdKZdEBc4q9fksAu1k7B3hVdHXf6w4xA=; b=s8ch+PJiDb9dle9IydriUV3Lt5k+JZceebjkdgOsli2gq/+2q/JETrNg81H7Zuhyxk xVPdtLBxfeffBwQ7yz+14eWfizNVYmdwh3lD9GXRZRwHRPF2F6YgaeLZeP18iuaEz6jH vNdbBfc/DTFGXlxf9LcHVkIve8nHXcFvXWLW7D2jjuxhMwN9UaKWuqAH/bMR+A0txChm TmF71gH177MvkUGhreaqdUhBYPpgY5I7xeaDyLoui8hfnP+i0P20wwzceUxKW1rzmS9k 08rEuBqG0NA0ZIHQHl1nIJbsNq1lUKZZNZMkhGqxKuGf9F07f006Zk0au6timSnM/sZf zggw== X-Gm-Message-State: AGi0PubB63e9tJR9JK+JMwahzSoslOK3EfgUCu4DgiKhkhp1X6wkTmTF QZ12p3FekwHbEMx9HUmJncUdannQct8AvxOZnbDr0g== X-Google-Smtp-Source: APiQypKx3Qry2LjUu+fZi00kuR8hErUpI+9R0/ofS/RvTKF780ixKpsNEJsNh3MMF9B4MtPo1A0aD+/VehAx0TW3DS4= X-Received: by 2002:aca:abc6:: with SMTP id u189mr5566oie.30.1588620524038; Mon, 04 May 2020 12:28:44 -0700 (PDT) MIME-Version: 1.0 References: <20200427212514.11219-1-robh@kernel.org> <733e20b1-9592-6941-766b-9f321ad2ace5@samsung.com> In-Reply-To: From: Saravana Kannan Date: Mon, 4 May 2020 12:28:07 -0700 Message-ID: Subject: Re: [PATCH] amba: Retry adding deferred devices at late_initcall To: Ulf Hansson X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200504_122847_978873_7E9BAAEF X-CRM114-Status: GOOD ( 33.23 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Rob Herring , Geert Uytterhoeven , Linus Walleij , Russell King , John Stultz , Nicolas Saenz Julienne , Sudeep Holla , Linux ARM , Marek Szyprowski Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, May 4, 2020 at 12:11 PM Ulf Hansson wrote: > > On Wed, 29 Apr 2020 at 09:33, Saravana Kannan wrote: > > > > On Tue, Apr 28, 2020 at 11:06 PM Marek Szyprowski > > wrote: > > > > > > Hi Linus, > > > > > > On 28.04.2020 22:39, Linus Walleij wrote: > > > > On Mon, Apr 27, 2020 at 11:25 PM Rob Herring wrote: > > > >> If amba bus devices defer when adding, the amba bus code simply retries > > > >> adding the devices every 5 seconds. This doesn't work well as it > > > >> completely unsynchronized with starting the init process which can > > > >> happen in less than 5 secs. Add a retry during late_initcall. If the > > > >> amba devices are added, then deferred probe takes over. If the > > > >> dependencies have not probed at this point, then there's no improvement > > > >> over previous behavior. To completely solve this, we'd need to retry > > > >> after every successful probe as deferred probe does. > > > >> > > > >> The list_empty() check now happens outside the mutex, but the mutex > > > >> wasn't necessary in the first place. > > > >> > > > >> This needed to use deferred probe instead of fragile initcall ordering > > > >> on 32-bit VExpress systems where the apb_pclk has a number of probe > > > >> dependencies (vexpress-sysregs, vexpress-config). > > > >> > > > >> Cc: John Stultz > > > >> Cc: Saravana Kannan > > > >> Cc: Linus Walleij > > > >> Cc: Sudeep Holla > > > >> Cc: Nicolas Saenz Julienne > > > >> Cc: Geert Uytterhoeven > > > >> Cc: Russell King > > > >> Signed-off-by: Rob Herring > > > > Makes sense to me, and the same approach is found > > > > in the generic code in drivers/base/dd.c so > > > > Reviewed-by: Linus Walleij > > > > > > > > The timer-based re-probe was added by Marek Szyprowski > > > > in commit a41980f2a3eb33ed7a2636e83498b47e95ceb05b > > > > do deal with power domains. I guess it mimics dd.c > > > > deferred probe at this point? > > > > > > > > There are a bit of other differences that have piled up, > > > > should we take a quick look at dd.c so there is not something > > > > else we're missing? I see some PM code for example. > > > > > > Well, late initcall based approach is what earlier version of my patch did: > > > > > > https://lkml.org/lkml/2016/4/12/414 > > > > > > but then it has been requested to solve the issue 'properly': > > > > > > https://lkml.org/lkml/2016/4/12/455 > > > > > > https://lkml.org/lkml/2016/4/14/875 > > > > > > For me it is fine to get back to late initcall based solution, though. > > > > > > > I haven't really dealt with a platform with AMBA devices and am not > > too familiar with it, so apologies in advance if any of my suggestions > > are silly. > > > > This whole "don't add a device before the clocks/resources needed to > > read the PID/CID" seems like something that can be solved by having a > > dummy device that "probes" when those resources are available. And > > during its probe, it adds the real amba device. That should avoid all > > the reinvention of deferred probing that amba/bus.c seems to be > > attempting. > > Sounds like a clever idea. Thanks > In principle we should then be able to rely on the regular defered > probe mechanism, just that it's the dummy device that is being defered > probed (if we fail to read PID/CID). > > > > > Any reason to not do something like that? I'd think that should clean > > up a whole lot of this code. Also, if we are primarily dealing with > > AMBA devices created from DT, then we might even be able to massage > > the fw_devlink feature to optimize this even more when fw_devlink=on. > > > > Just my 2 cents. > > Someone should try to implement this to see if it fits well. I don't mind taking a stab at this if people are actually okay with this approach and will test and merge it if it works. I have no platform to test this. I'll wait to hear what others think before I jump on this. -Saravana _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel