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=-10.1 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 DA998C433E0 for ; Thu, 6 Aug 2020 01:25:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B0E4B2173E for ; Thu, 6 Aug 2020 01:25:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="zBjyQ7Fq" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726934AbgHFBZR (ORCPT ); Wed, 5 Aug 2020 21:25:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42328 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726845AbgHFBZE (ORCPT ); Wed, 5 Aug 2020 21:25:04 -0400 Received: from mail-ot1-x341.google.com (mail-ot1-x341.google.com [IPv6:2607:f8b0:4864:20::341]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AF983C061574 for ; Wed, 5 Aug 2020 18:25:03 -0700 (PDT) Received: by mail-ot1-x341.google.com with SMTP id h22so5709020otq.11 for ; Wed, 05 Aug 2020 18:25:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=k6pimq7S+VITQhnjcjjKsbMV9B3GCD09apLcTf0EFW4=; b=zBjyQ7FqB6dMiThk1hav7gsRnCLpxmI/Fy5uZOtG59LSQ2QsizkGeT9cxRcrrT5uuK IN7w9SlXJWWBuojgD/fhVAol+es3RGGs8AmAlc7M3oYZR77TgO837vZj7SBdpFIojDRw LTKRa9aTIHhvoQX+4vvpYLQ+yGi/UKZ8Y7EbrZD1x6XS4Bd5efeeHiRAUqKaCUK1vNQK lexkbnVoJkHVdOviBHjojnC4qCA6htQXfwn4PGdpQSF16KaqNDXBMqWYqst2HW96Hm7r 2/VNTA/3gvLGwRTGVyBp9ZBPbKUGDZdXhoY6xue1CEj/Gg4kd1FIAb4SqmnPuO8aJGbY 6hng== 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=k6pimq7S+VITQhnjcjjKsbMV9B3GCD09apLcTf0EFW4=; b=Zf4464ZqT/R4wth5w32zJ1pv4UhX4U3ScV7tLfkMM04S7P17gs0+wwgfCsmvQm6iPK jcnJGzZH4Sp72TxLd5biQ4W1dwvmopriVwU71e1s8K5dqA742ePp72m8dQ1HlMmPb5NW m/0BrtSmzJajG9AKxT/jj3+AqnNHG+zD0ZDi7S5z5+n0sPd1A1p48rdn/7ZPnbD41kng eviy7JEhmn1E8iRX7lrjsWIO9VNmvvh4sQUtqX7mH1T61Zj3ie37ORr/K90+wtvpb7MP l7CJsEjuPe02URQnxjrP7Ux+GJDzsUSR95iVdMyf7jfLvSU0MjDSDbO90o6Oj/bpiePL h1Sg== X-Gm-Message-State: AOAM530KufFAs7P6dHE6VgSVpziaV/CXOCsqaUilWXFsUcMEArAH+2Yd p5NZ4NBINbZlh5pkhVXrODZNN+dD8NHHHy+VVuV4BA== X-Google-Smtp-Source: ABdhPJwp1gy6ICxlR+QU+ROR+YJdoXDHLADEqbolalzpss7WqkKehLyLFZYdIvJ/S4ZjbwkZ1sedTzTnZwQb6ihZJxQ= X-Received: by 2002:a05:6830:3196:: with SMTP id p22mr5524618ots.102.1596677102717; Wed, 05 Aug 2020 18:25:02 -0700 (PDT) MIME-Version: 1.0 References: <20200718000637.3632841-1-saravanak@google.com> <20200718000637.3632841-3-saravanak@google.com> In-Reply-To: From: John Stultz Date: Wed, 5 Aug 2020 18:24:50 -0700 Message-ID: Subject: Re: [PATCH v3 2/4] irqchip/qcom-pdc: Switch to using IRQCHIP_PLATFORM_DRIVER helper macros To: Saravana Kannan Cc: Thomas Gleixner , Jason Cooper , Marc Zyngier , Matthias Brugger , Andy Gross , Bjorn Andersson , Android Kernel Team , lkml , linux-arm-kernel , "moderated list:ARM/Mediatek SoC support" , linux-arm-msm , Hanks Chen , CC Hwang , Loda Chou Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Aug 5, 2020 at 1:19 PM Saravana Kannan wrote: > On Wed, Aug 5, 2020 at 12:44 AM John Stultz wrote: > > On Fri, Jul 17, 2020 at 5:06 PM Saravana Kannan wrote: > > > > > > Switch the driver to use the helper macros. In addition to reducing the > > > number of lines, this also adds module unload protection (if the driver > > > is compiled as a module) by switching from module_platform_driver to > > > builtin_platform_driver. > > > > > > Signed-off-by: Saravana Kannan > > > --- > > > drivers/irqchip/qcom-pdc.c | 26 +++----------------------- > > > 1 file changed, 3 insertions(+), 23 deletions(-) > > > > > > diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c > > > index 5b624e3295e4..c1c5dfad57cc 100644 > > > --- a/drivers/irqchip/qcom-pdc.c > > > +++ b/drivers/irqchip/qcom-pdc.c > > > @@ -432,28 +432,8 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent) > > > return ret; > > > } > > > > > > -static int qcom_pdc_probe(struct platform_device *pdev) > > > -{ > > > - struct device_node *np = pdev->dev.of_node; > > > - struct device_node *parent = of_irq_find_parent(np); > > > - > > > - return qcom_pdc_init(np, parent); > > > -} > > > - > > > -static const struct of_device_id qcom_pdc_match_table[] = { > > > - { .compatible = "qcom,pdc" }, > > > - {} > > > -}; > > > -MODULE_DEVICE_TABLE(of, qcom_pdc_match_table); > > > - > > > -static struct platform_driver qcom_pdc_driver = { > > > - .probe = qcom_pdc_probe, > > > - .driver = { > > > - .name = "qcom-pdc", > > > - .of_match_table = qcom_pdc_match_table, > > > - .suppress_bind_attrs = true, > > > - }, > > > -}; > > > -module_platform_driver(qcom_pdc_driver); > > > +IRQCHIP_PLATFORM_DRIVER_BEGIN(qcom_pdc) > > > +IRQCHIP_MATCH("qcom,pdc", qcom_pdc_init) > > > +IRQCHIP_PLATFORM_DRIVER_END(qcom_pdc) > > > MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Power Domain Controller"); > > > MODULE_LICENSE("GPL v2"); > > > > > > So this is where I bashfully admit I didn't get a chance to try this > > patch series out, as I had success with a much older version of > > Saravana's macro magic. > > > > But unfortunately, now that this has landed in mainline, I'm seeing > > boot regressions on db845c. :( This is in the non-modular case, > > building the driver in. > > Does that mean the modular version is working? Or you haven't tried > that yet? I'll wait for your reply before I try to fix it. I don't > have the hardware, but it should be easy to guess this issue looking > at the code delta. I've not yet tested with modules with your patch. > The only significant change from what your probe function is doing is > this snippet. But it'd be surprising if this only affects the builtin > case. > > + if (par_np == np) > + par_np = NULL; > + > + /* > + * If there's a parent interrupt controller and none of the parent irq > + * domains have been registered, that means the parent interrupt > + * controller has not been initialized yet. it's not time for this > + * interrupt controller to initialize. So, defer probe of this > + * interrupt controller. The actual initialization callback of this > + * interrupt controller can check for specific domains as necessary. > + */ > + if (par_np && !irq_find_matching_host(np, DOMAIN_BUS_ANY)) > + return -EPROBE_DEFER; Yep. We're getting caught on the irq_find_matching_host() check. I'm a little lost as when I look at the qcom,pdc node in the dtsi its not under a parent controller (instead the soc node). Not sure if that's an issue in the dtsi or if par_np check needs to ignore the soc node and pass null? thanks -john