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 C5545C433E1 for ; Tue, 18 Aug 2020 19:07:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A2A6A2076E for ; Tue, 18 Aug 2020 19:07:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1597777630; bh=55pVwpT0xEtPZVEROShTJ0Uc3arN8xwipdI+p/ChKAM=; h=References:In-Reply-To:From:Date:Subject:To:Cc:List-ID:From; b=N9MLg+OufCaz5KuOv6N37gGlkU5D3qq3PKmAbdT1cMEu6wRMfrPjZyQI4hg43cc13 rTGARZCxitS4lDtPXTtbkcgnluYLG9KCQD4Pv5t3FowidX0eusuIXacwpm17IwxSRY CLu5cJp7feuzAnAKNcmNriXH9jNwUe2bUU16Vd5s= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726701AbgHRTHI (ORCPT ); Tue, 18 Aug 2020 15:07:08 -0400 Received: from mail.kernel.org ([198.145.29.99]:54622 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726435AbgHRTHF (ORCPT ); Tue, 18 Aug 2020 15:07:05 -0400 Received: from mail-oi1-f181.google.com (mail-oi1-f181.google.com [209.85.167.181]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 72D1620772; Tue, 18 Aug 2020 19:07:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1597777624; bh=55pVwpT0xEtPZVEROShTJ0Uc3arN8xwipdI+p/ChKAM=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=quvDvkQpVKXStBOSmRMymOqkLwjWtVzNRoBo4xkbhmnR3elsXvxbmTGTA8NpwwlfS AbbVotQBwANhm/JVXvB5O2+9aGEBU8FJKQspBEEl88rUic9O2BlQY6SFLI1PRUq/Pr YNsu8AlgTVEui/hPehzU9/pe0Ye0hliUlPyxSUp4= Received: by mail-oi1-f181.google.com with SMTP id k4so18884839oik.2; Tue, 18 Aug 2020 12:07:04 -0700 (PDT) X-Gm-Message-State: AOAM531AhpRGDQ0xkKLA0of7So9qZLt/mYXwL/2cmLGd0sPpfwMVAKC9 Cmw+R62PenBORyZ6RYgQGOxpnJbkMwpOa1lxQA== X-Google-Smtp-Source: ABdhPJy0LflVo0MIWnhHUiGwo2npwXpKDh8efjVZ0OkgXzrN+b7fkI66kwoECSvpC/NEsckpFvqnjd3FkrDbH5HAa5Y= X-Received: by 2002:aca:190c:: with SMTP id l12mr1083286oii.147.1597777623829; Tue, 18 Aug 2020 12:07:03 -0700 (PDT) MIME-Version: 1.0 References: <20200815125112.462652-2-maz@kernel.org> <20200815232228.GA1325245@bjorn-Precision-5520> <87pn7qnabq.wl-maz@kernel.org> <72c10e43023289b9a4c36226fe3fd5d9@kernel.org> In-Reply-To: From: Rob Herring Date: Tue, 18 Aug 2020 13:06:51 -0600 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 1/2] PCI: rockchip: Work around missing device_type property in DT To: Marc Zyngier Cc: Saravana Kannan , 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 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 Tue, Aug 18, 2020 at 1:03 PM Marc Zyngier wrote: > > 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. LGTM. > 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 5DA65C433DF for ; Tue, 18 Aug 2020 19:07:17 +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 294182076E for ; Tue, 18 Aug 2020 19:07:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="ASO1Ndfr"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="quvDvkQp" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 294182076E 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-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=GKz5VN/b7ONGbr/6LFzgTuV0pOdM4TQfjY2upnL742Q=; b=ASO1NdfrJjQV7MmEN0WEvHp+3 PdGjzx2gPxudQvbSVH60aeOin7zruvbVQ4gIkWuMt2fB6ZPU6k6X5n9/n0DyvgLr0SVfD1tv4AE0G tsYxU0H0X2tjk9kwd4rXkD6XoOLdgsrcLdM6M/v0+tvxNeTHvq+C0RUhIIqgyWMst4rkn1ae+ef/T XoHS90zoMV8NB5BnImzYfY0hzuFxXAfOOF8NHy+eS1exo7JHKFJuDlABgDUsJaYQqrrZU6X/Y/ACa UrySKngzOsotrmW8izLT1UKOjKHuhcLCrNh4CGJ2O46caMUJmrNrDtjOpcsjPXmf/gOxx/Ee+eDFy xljIkoQvA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k86ww-0007F9-9v; Tue, 18 Aug 2020 19:07:10 +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 1k86wr-0007Di-Oe; Tue, 18 Aug 2020 19:07:06 +0000 Received: from mail-oi1-f179.google.com (mail-oi1-f179.google.com [209.85.167.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 7C1FE20882; Tue, 18 Aug 2020 19:07:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1597777624; bh=55pVwpT0xEtPZVEROShTJ0Uc3arN8xwipdI+p/ChKAM=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=quvDvkQpVKXStBOSmRMymOqkLwjWtVzNRoBo4xkbhmnR3elsXvxbmTGTA8NpwwlfS AbbVotQBwANhm/JVXvB5O2+9aGEBU8FJKQspBEEl88rUic9O2BlQY6SFLI1PRUq/Pr YNsu8AlgTVEui/hPehzU9/pe0Ye0hliUlPyxSUp4= Received: by mail-oi1-f179.google.com with SMTP id j7so18876209oij.9; Tue, 18 Aug 2020 12:07:04 -0700 (PDT) X-Gm-Message-State: AOAM533ujjEXUR2DMCYFp3oz8VQmGND9xs9dyZmsVg9yc6buchpSZmIw jATtQnt+jICpNWVml5DWFmfInPC3nbJB6AcKig== X-Google-Smtp-Source: ABdhPJy0LflVo0MIWnhHUiGwo2npwXpKDh8efjVZ0OkgXzrN+b7fkI66kwoECSvpC/NEsckpFvqnjd3FkrDbH5HAa5Y= X-Received: by 2002:aca:190c:: with SMTP id l12mr1083286oii.147.1597777623829; Tue, 18 Aug 2020 12:07:03 -0700 (PDT) MIME-Version: 1.0 References: <20200815125112.462652-2-maz@kernel.org> <20200815232228.GA1325245@bjorn-Precision-5520> <87pn7qnabq.wl-maz@kernel.org> <72c10e43023289b9a4c36226fe3fd5d9@kernel.org> In-Reply-To: From: Rob Herring Date: Tue, 18 Aug 2020 13:06:51 -0600 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 1/2] PCI: rockchip: Work around missing device_type property in DT To: Marc Zyngier X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200818_150706_009732_6BBF6826 X-CRM114-Status: GOOD ( 30.54 ) 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: Lorenzo Pieralisi , Heiko Stuebner , Saravana Kannan , 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-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+linux-rockchip=archiver.kernel.org@lists.infradead.org On Tue, Aug 18, 2020 at 1:03 PM Marc Zyngier wrote: > > 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. LGTM. > 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 1EDA8C433E1 for ; Tue, 18 Aug 2020 19:08:37 +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 DFA122076E for ; Tue, 18 Aug 2020 19:08:36 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="ilwJUhA3"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="quvDvkQp" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DFA122076E 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-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=SaRMO+lhPgcG0/i8cquAoIiosU9jbn8ep3JtoPjQ4dU=; b=ilwJUhA3OCVYO6DhofgRyENot snKq9hW4fQjzDZTB2wNykGisDnCbGBvt/7baLIq9p4GMXt/3ebCGjYhaxN582YhEaRDzfvN36sqlA kUDtMtXE/Uwi+cDFntYdgH90tGH79YQNLTFR90AfZmR+diiJODVHHlIBTIuBALM50bl6B66PVrmnf nAbv7hTh3uoVoM3Br+pf+ZG8O5E/M+waESBu4UE3hXPc9FIyQGmwnbul6yOE8f26ZZvyiEPgOq+l6 txZTKzhXfzcGBUABaGxhXMwIeMHrXeZWsTIOFwdPaIeaQVqRFl/Qylh5LrVb/GltiWFQF3i3eEjjs QpXHOgTwA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k86wu-0007Ed-Me; Tue, 18 Aug 2020 19:07:08 +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 1k86wr-0007Di-Oe; Tue, 18 Aug 2020 19:07:06 +0000 Received: from mail-oi1-f179.google.com (mail-oi1-f179.google.com [209.85.167.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 7C1FE20882; Tue, 18 Aug 2020 19:07:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1597777624; bh=55pVwpT0xEtPZVEROShTJ0Uc3arN8xwipdI+p/ChKAM=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=quvDvkQpVKXStBOSmRMymOqkLwjWtVzNRoBo4xkbhmnR3elsXvxbmTGTA8NpwwlfS AbbVotQBwANhm/JVXvB5O2+9aGEBU8FJKQspBEEl88rUic9O2BlQY6SFLI1PRUq/Pr YNsu8AlgTVEui/hPehzU9/pe0Ye0hliUlPyxSUp4= Received: by mail-oi1-f179.google.com with SMTP id j7so18876209oij.9; Tue, 18 Aug 2020 12:07:04 -0700 (PDT) X-Gm-Message-State: AOAM533ujjEXUR2DMCYFp3oz8VQmGND9xs9dyZmsVg9yc6buchpSZmIw jATtQnt+jICpNWVml5DWFmfInPC3nbJB6AcKig== X-Google-Smtp-Source: ABdhPJy0LflVo0MIWnhHUiGwo2npwXpKDh8efjVZ0OkgXzrN+b7fkI66kwoECSvpC/NEsckpFvqnjd3FkrDbH5HAa5Y= X-Received: by 2002:aca:190c:: with SMTP id l12mr1083286oii.147.1597777623829; Tue, 18 Aug 2020 12:07:03 -0700 (PDT) MIME-Version: 1.0 References: <20200815125112.462652-2-maz@kernel.org> <20200815232228.GA1325245@bjorn-Precision-5520> <87pn7qnabq.wl-maz@kernel.org> <72c10e43023289b9a4c36226fe3fd5d9@kernel.org> In-Reply-To: From: Rob Herring Date: Tue, 18 Aug 2020 13:06:51 -0600 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 1/2] PCI: rockchip: Work around missing device_type property in DT To: Marc Zyngier X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200818_150706_009732_6BBF6826 X-CRM114-Status: GOOD ( 30.54 ) 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: Lorenzo Pieralisi , Heiko Stuebner , Saravana Kannan , 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-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Aug 18, 2020 at 1:03 PM Marc Zyngier wrote: > > 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. LGTM. > 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