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=-7.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI, 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 EF2DBC433E3 for ; Tue, 18 Aug 2020 19:03:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C054920772 for ; Tue, 18 Aug 2020 19:03:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1597777385; bh=7H5ZWWY6YLYiTiWY7a+UJBAlG+UhyF+KVtyPoiI+3DY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:List-ID:From; b=Zv2ZSRmtCS6LdAg9HlbSmftDrin3X6bNY5o9qumQtS6OFPs2MA/JX0xOMV/QCnWrK poywsAKvUqO2+iwRkgeOOEZIpRlzMIHZUAg/gfhuwui9BDstuWPr62JTgbMbd0OOJ2 /caH8b0aRgDbhCqxo4O7WqE2lkjWRYKX7mzVUjw0= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726715AbgHRTDE (ORCPT ); Tue, 18 Aug 2020 15:03:04 -0400 Received: from mail.kernel.org ([198.145.29.99]:44224 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726435AbgHRTDC (ORCPT ); Tue, 18 Aug 2020 15:03:02 -0400 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 447DE2076E; Tue, 18 Aug 2020 19:03:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1597777381; bh=7H5ZWWY6YLYiTiWY7a+UJBAlG+UhyF+KVtyPoiI+3DY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=lG6c7XfmiFTjSYHYrfxKb5hhYD6nIpqiB/Rpqaq0tdFteoQGT8DqDJlQ6shK6MgTI VpZFrJ//JYY431uMKgbwb/ByRcOPRzHDb0PYobp0Jm56cMrpp2LwSlpQ4TzZKVyT2A mLQU/svuX6rlC8PEFLK3Mjoj9w6cRfaxJwpgL4D0= Received: from disco-boy.misterjones.org ([51.254.78.96] helo=www.loen.fr) by disco-boy.misterjones.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1k86st-00406W-Q2; Tue, 18 Aug 2020 20:02:59 +0100 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Tue, 18 Aug 2020 20:02:59 +0100 From: Marc Zyngier To: Saravana Kannan Cc: Rob Herring , Bjorn Helgaas , PCI , "open list:ARM/Rockchip SoC..." , "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" , LKML , Lorenzo Pieralisi , Heiko Stuebner , Shawn Lin , Bjorn Helgaas , Android Kernel Team Subject: Re: [PATCH 1/2] PCI: rockchip: Work around missing device_type property in DT In-Reply-To: References: <20200815125112.462652-2-maz@kernel.org> <20200815232228.GA1325245@bjorn-Precision-5520> <87pn7qnabq.wl-maz@kernel.org> <72c10e43023289b9a4c36226fe3fd5d9@kernel.org> User-Agent: Roundcube Webmail/1.4.7 Message-ID: X-Sender: maz@kernel.org X-SA-Exim-Connect-IP: 51.254.78.96 X-SA-Exim-Rcpt-To: saravanak@google.com, robh@kernel.org, helgaas@kernel.org, linux-pci@vger.kernel.org, linux-rockchip@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, lorenzo.pieralisi@arm.com, heiko@sntech.de, shawn.lin@rock-chips.com, bhelgaas@google.com, kernel-team@android.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2020-08-18 18:48, Saravana Kannan wrote: > On Tue, Aug 18, 2020 at 10:34 AM Marc Zyngier wrote: [...] >> OK. So how about something like this? >> >> diff --git a/drivers/of/address.c b/drivers/of/address.c >> index 590493e04b01..a7a6ee599b14 100644 >> --- a/drivers/of/address.c >> +++ b/drivers/of/address.c >> @@ -134,9 +134,13 @@ static int of_bus_pci_match(struct device_node >> *np) >> * "pciex" is PCI Express >> * "vci" is for the /chaos bridge on 1st-gen PCI powermacs >> * "ht" is hypertransport >> + * >> + * If none of the device_type match, and that the node name is >> + * "pcie", accept the device as PCI (with a warning). >> */ >> return of_node_is_type(np, "pci") || of_node_is_type(np, >> "pciex") || >> - of_node_is_type(np, "vci") || of_node_is_type(np, >> "ht"); >> + of_node_is_type(np, "vci") || of_node_is_type(np, >> "ht") || >> + WARN_ON_ONCE(of_node_name_eq(np, "pcie")); > > I don't think we need the _ONCE. Otherwise, it'd warn only for the > first device that has this problem. Because probing devices doesn't necessarily occur once. Case in point, it takes *10 to 15* attempts for a rk3399 system such as mine to finally probe its PCIe device, thanks to the wonderful -EPROBE_DEFER. Do I want to see the same stack trace 10 (or more) times? No. > How about? > WARN(of_node_name_eq(np, "pcie"), "Missing device type in %pOF", np) > > That'll even tell them which node is bad. I explained my objections above. Spitting out the device node is useful, but there is no need to be exhaustive (if you're in a position to fix the DT, you can track all the broken instances for your device easily). I'm actually minded to tone it down even more, because the stack trace is meaningless to most users. See below for a revised patch. M. diff --git a/drivers/of/address.c b/drivers/of/address.c index 590493e04b01..b37bd9cc2810 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -128,15 +128,29 @@ static unsigned int of_bus_pci_get_flags(const __be32 *addr) * PCI bus specific translator */ +static bool of_node_is_pcie(struct device_node *np) +{ + bool is_pcie = of_node_name_eq(np, "pcie"); + + if (is_pcie) + pr_warn_once("%pOF: Missing device_type\n", np); + + return is_pcie; +} + static int of_bus_pci_match(struct device_node *np) { /* * "pciex" is PCI Express * "vci" is for the /chaos bridge on 1st-gen PCI powermacs * "ht" is hypertransport + * + * If none of the device_type match, and that the node name is + * "pcie", accept the device as PCI (with a warning). */ return of_node_is_type(np, "pci") || of_node_is_type(np, "pciex") || - of_node_is_type(np, "vci") || of_node_is_type(np, "ht"); + of_node_is_type(np, "vci") || of_node_is_type(np, "ht") || + of_node_is_pcie(np); } static void of_bus_pci_count_cells(struct device_node *np, -- Jazz is not dead. It just smells funny... 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=-7.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,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 7F61CC433E1 for ; Tue, 18 Aug 2020 19:03:13 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 5076620772 for ; Tue, 18 Aug 2020 19:03:13 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="1JjWqJZT"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="lG6c7Xfm" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5076620772 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-rockchip-bounces+linux-rockchip=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Message-ID:References:In-Reply-To:Subject:To:From: Date:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=AAl2mwe1f9cOI8NOEBQ42LV6rYGycd0RovFplb+Cne4=; b=1JjWqJZTgjfNm+kjN37tObWSx jxXpWENGjFHbAwPH+UkwxQoHJOYw98vrLED3GFW6em3rlMG99j2ZuM1nRjvcplY1dWN+jXtNIRSjf iPVkgsoDHVdzC3SNhGWRR4yZcS4MV49O9j/kFuUSwVzjafX+v4tbfFgoIfWrnDQ5J+UiTRYZe9Kqz Kw3ajDxthMOJkbqGJU3v7XKuvj/WVAK4I8SOvbkJDAt/wbATZqIk1qRGCB73Yymw1O8pt0FJ5hH1N Yel6VZqbfnpfCuGmxVKTt928wNEZHe5uP2yo3aS9VjqCV9lZxBIuh/mYx7I3HOsQYxlozqiLXqQ9f bSHneweww==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k86t0-0006iO-A1; Tue, 18 Aug 2020 19:03:06 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1k86sw-0006hK-FG; Tue, 18 Aug 2020 19:03:03 +0000 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 447DE2076E; Tue, 18 Aug 2020 19:03:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1597777381; bh=7H5ZWWY6YLYiTiWY7a+UJBAlG+UhyF+KVtyPoiI+3DY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=lG6c7XfmiFTjSYHYrfxKb5hhYD6nIpqiB/Rpqaq0tdFteoQGT8DqDJlQ6shK6MgTI VpZFrJ//JYY431uMKgbwb/ByRcOPRzHDb0PYobp0Jm56cMrpp2LwSlpQ4TzZKVyT2A mLQU/svuX6rlC8PEFLK3Mjoj9w6cRfaxJwpgL4D0= Received: from disco-boy.misterjones.org ([51.254.78.96] helo=www.loen.fr) by disco-boy.misterjones.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1k86st-00406W-Q2; Tue, 18 Aug 2020 20:02:59 +0100 MIME-Version: 1.0 Date: Tue, 18 Aug 2020 20:02:59 +0100 From: Marc Zyngier To: Saravana Kannan Subject: Re: [PATCH 1/2] PCI: rockchip: Work around missing device_type property in DT In-Reply-To: References: <20200815125112.462652-2-maz@kernel.org> <20200815232228.GA1325245@bjorn-Precision-5520> <87pn7qnabq.wl-maz@kernel.org> <72c10e43023289b9a4c36226fe3fd5d9@kernel.org> User-Agent: Roundcube Webmail/1.4.7 Message-ID: X-Sender: maz@kernel.org X-SA-Exim-Connect-IP: 51.254.78.96 X-SA-Exim-Rcpt-To: saravanak@google.com, robh@kernel.org, helgaas@kernel.org, linux-pci@vger.kernel.org, linux-rockchip@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, lorenzo.pieralisi@arm.com, heiko@sntech.de, shawn.lin@rock-chips.com, bhelgaas@google.com, kernel-team@android.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200818_150302_666929_587F4769 X-CRM114-Status: GOOD ( 22.19 ) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Rob Herring , Lorenzo Pieralisi , Heiko Stuebner , PCI , Shawn Lin , LKML , "open list:ARM/Rockchip SoC..." , Bjorn Helgaas , Bjorn Helgaas , Android Kernel Team , "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+linux-rockchip=archiver.kernel.org@lists.infradead.org On 2020-08-18 18:48, Saravana Kannan wrote: > On Tue, Aug 18, 2020 at 10:34 AM Marc Zyngier wrote: [...] >> OK. So how about something like this? >> >> diff --git a/drivers/of/address.c b/drivers/of/address.c >> index 590493e04b01..a7a6ee599b14 100644 >> --- a/drivers/of/address.c >> +++ b/drivers/of/address.c >> @@ -134,9 +134,13 @@ static int of_bus_pci_match(struct device_node >> *np) >> * "pciex" is PCI Express >> * "vci" is for the /chaos bridge on 1st-gen PCI powermacs >> * "ht" is hypertransport >> + * >> + * If none of the device_type match, and that the node name is >> + * "pcie", accept the device as PCI (with a warning). >> */ >> return of_node_is_type(np, "pci") || of_node_is_type(np, >> "pciex") || >> - of_node_is_type(np, "vci") || of_node_is_type(np, >> "ht"); >> + of_node_is_type(np, "vci") || of_node_is_type(np, >> "ht") || >> + WARN_ON_ONCE(of_node_name_eq(np, "pcie")); > > I don't think we need the _ONCE. Otherwise, it'd warn only for the > first device that has this problem. Because probing devices doesn't necessarily occur once. Case in point, it takes *10 to 15* attempts for a rk3399 system such as mine to finally probe its PCIe device, thanks to the wonderful -EPROBE_DEFER. Do I want to see the same stack trace 10 (or more) times? No. > How about? > WARN(of_node_name_eq(np, "pcie"), "Missing device type in %pOF", np) > > That'll even tell them which node is bad. I explained my objections above. Spitting out the device node is useful, but there is no need to be exhaustive (if you're in a position to fix the DT, you can track all the broken instances for your device easily). I'm actually minded to tone it down even more, because the stack trace is meaningless to most users. See below for a revised patch. M. diff --git a/drivers/of/address.c b/drivers/of/address.c index 590493e04b01..b37bd9cc2810 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -128,15 +128,29 @@ static unsigned int of_bus_pci_get_flags(const __be32 *addr) * PCI bus specific translator */ +static bool of_node_is_pcie(struct device_node *np) +{ + bool is_pcie = of_node_name_eq(np, "pcie"); + + if (is_pcie) + pr_warn_once("%pOF: Missing device_type\n", np); + + return is_pcie; +} + static int of_bus_pci_match(struct device_node *np) { /* * "pciex" is PCI Express * "vci" is for the /chaos bridge on 1st-gen PCI powermacs * "ht" is hypertransport + * + * If none of the device_type match, and that the node name is + * "pcie", accept the device as PCI (with a warning). */ return of_node_is_type(np, "pci") || of_node_is_type(np, "pciex") || - of_node_is_type(np, "vci") || of_node_is_type(np, "ht"); + of_node_is_type(np, "vci") || of_node_is_type(np, "ht") || + of_node_is_pcie(np); } static void of_bus_pci_count_cells(struct device_node *np, -- Jazz is not dead. It just smells funny... _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip 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=-7.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,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 1D6A8C433E1 for ; Tue, 18 Aug 2020 19:05:07 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 DD26D2076E for ; Tue, 18 Aug 2020 19:05:06 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="BsvvRwrw"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="lG6c7Xfm" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DD26D2076E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+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=merlin.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Message-ID:References:In-Reply-To:Subject:To:From: Date:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=d4y2FJfSAqOF34Sl1ahCD87bNecJcWY3LWP+oizdVvg=; b=BsvvRwrw4EelddqQ8IFeiaLmf 5pKSxZ8NP6mPgFiHfUr6NKVoFQF8I5F0rfgge2Dkmg3BK/ekAt4J4imiKzkwaKIq5uWzG+Ft2DXn0 rTwz/RWwxpj1izur0SbqbNDrqq0b/l6WJ9AaQdqISiVuurc/ZpTQblAdiQXsQ7/bnNugnIk13fLFZ 3Cu1XVjuETRvI3oGQeUba8/wsTOfR+UGu1LW6zxvSY4Qc9r8F732uEZGSfDSnhY+3avm3uq4xofsq 2jgy7VuR5tbPSY98pvvghTS6a9t5QE1rHtzTtiIxpB/sbpeYsC8s/AF38d4/5qBM4kSQkwaDG/hnt cA16kot/Q==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k86sz-0006hz-09; Tue, 18 Aug 2020 19:03:05 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1k86sw-0006hK-FG; Tue, 18 Aug 2020 19:03:03 +0000 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 447DE2076E; Tue, 18 Aug 2020 19:03:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1597777381; bh=7H5ZWWY6YLYiTiWY7a+UJBAlG+UhyF+KVtyPoiI+3DY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=lG6c7XfmiFTjSYHYrfxKb5hhYD6nIpqiB/Rpqaq0tdFteoQGT8DqDJlQ6shK6MgTI VpZFrJ//JYY431uMKgbwb/ByRcOPRzHDb0PYobp0Jm56cMrpp2LwSlpQ4TzZKVyT2A mLQU/svuX6rlC8PEFLK3Mjoj9w6cRfaxJwpgL4D0= Received: from disco-boy.misterjones.org ([51.254.78.96] helo=www.loen.fr) by disco-boy.misterjones.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1k86st-00406W-Q2; Tue, 18 Aug 2020 20:02:59 +0100 MIME-Version: 1.0 Date: Tue, 18 Aug 2020 20:02:59 +0100 From: Marc Zyngier To: Saravana Kannan Subject: Re: [PATCH 1/2] PCI: rockchip: Work around missing device_type property in DT In-Reply-To: References: <20200815125112.462652-2-maz@kernel.org> <20200815232228.GA1325245@bjorn-Precision-5520> <87pn7qnabq.wl-maz@kernel.org> <72c10e43023289b9a4c36226fe3fd5d9@kernel.org> User-Agent: Roundcube Webmail/1.4.7 Message-ID: X-Sender: maz@kernel.org X-SA-Exim-Connect-IP: 51.254.78.96 X-SA-Exim-Rcpt-To: saravanak@google.com, robh@kernel.org, helgaas@kernel.org, linux-pci@vger.kernel.org, linux-rockchip@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, lorenzo.pieralisi@arm.com, heiko@sntech.de, shawn.lin@rock-chips.com, bhelgaas@google.com, kernel-team@android.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200818_150302_666929_587F4769 X-CRM114-Status: GOOD ( 22.19 ) 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 , Lorenzo Pieralisi , Heiko Stuebner , PCI , Shawn Lin , LKML , "open list:ARM/Rockchip SoC..." , Bjorn Helgaas , Bjorn Helgaas , Android Kernel Team , "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2020-08-18 18:48, Saravana Kannan wrote: > On Tue, Aug 18, 2020 at 10:34 AM Marc Zyngier wrote: [...] >> OK. So how about something like this? >> >> diff --git a/drivers/of/address.c b/drivers/of/address.c >> index 590493e04b01..a7a6ee599b14 100644 >> --- a/drivers/of/address.c >> +++ b/drivers/of/address.c >> @@ -134,9 +134,13 @@ static int of_bus_pci_match(struct device_node >> *np) >> * "pciex" is PCI Express >> * "vci" is for the /chaos bridge on 1st-gen PCI powermacs >> * "ht" is hypertransport >> + * >> + * If none of the device_type match, and that the node name is >> + * "pcie", accept the device as PCI (with a warning). >> */ >> return of_node_is_type(np, "pci") || of_node_is_type(np, >> "pciex") || >> - of_node_is_type(np, "vci") || of_node_is_type(np, >> "ht"); >> + of_node_is_type(np, "vci") || of_node_is_type(np, >> "ht") || >> + WARN_ON_ONCE(of_node_name_eq(np, "pcie")); > > I don't think we need the _ONCE. Otherwise, it'd warn only for the > first device that has this problem. Because probing devices doesn't necessarily occur once. Case in point, it takes *10 to 15* attempts for a rk3399 system such as mine to finally probe its PCIe device, thanks to the wonderful -EPROBE_DEFER. Do I want to see the same stack trace 10 (or more) times? No. > How about? > WARN(of_node_name_eq(np, "pcie"), "Missing device type in %pOF", np) > > That'll even tell them which node is bad. I explained my objections above. Spitting out the device node is useful, but there is no need to be exhaustive (if you're in a position to fix the DT, you can track all the broken instances for your device easily). I'm actually minded to tone it down even more, because the stack trace is meaningless to most users. See below for a revised patch. M. diff --git a/drivers/of/address.c b/drivers/of/address.c index 590493e04b01..b37bd9cc2810 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -128,15 +128,29 @@ static unsigned int of_bus_pci_get_flags(const __be32 *addr) * PCI bus specific translator */ +static bool of_node_is_pcie(struct device_node *np) +{ + bool is_pcie = of_node_name_eq(np, "pcie"); + + if (is_pcie) + pr_warn_once("%pOF: Missing device_type\n", np); + + return is_pcie; +} + static int of_bus_pci_match(struct device_node *np) { /* * "pciex" is PCI Express * "vci" is for the /chaos bridge on 1st-gen PCI powermacs * "ht" is hypertransport + * + * If none of the device_type match, and that the node name is + * "pcie", accept the device as PCI (with a warning). */ return of_node_is_type(np, "pci") || of_node_is_type(np, "pciex") || - of_node_is_type(np, "vci") || of_node_is_type(np, "ht"); + of_node_is_type(np, "vci") || of_node_is_type(np, "ht") || + of_node_is_pcie(np); } static void of_bus_pci_count_cells(struct device_node *np, -- Jazz is not dead. It just smells funny... _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel