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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 81CBCC433F5 for ; Wed, 13 Oct 2021 13:23:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 69B7B610D0 for ; Wed, 13 Oct 2021 13:23:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233955AbhJMNZd (ORCPT ); Wed, 13 Oct 2021 09:25:33 -0400 Received: from mga17.intel.com ([192.55.52.151]:29501 "EHLO mga17.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233226AbhJMNZc (ORCPT ); Wed, 13 Oct 2021 09:25:32 -0400 X-IronPort-AV: E=McAfee;i="6200,9189,10135"; a="208223337" X-IronPort-AV: E=Sophos;i="5.85,371,1624345200"; d="scan'208";a="208223337" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Oct 2021 06:23:29 -0700 X-IronPort-AV: E=Sophos;i="5.85,371,1624345200"; d="scan'208";a="626341116" Received: from smile.fi.intel.com (HELO smile) ([10.237.72.159]) by fmsmga001-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Oct 2021 06:23:15 -0700 Received: from andy by smile with local (Exim 4.95) (envelope-from ) id 1maeDt-000LPO-Od; Wed, 13 Oct 2021 16:23:09 +0300 Date: Wed, 13 Oct 2021 16:23:09 +0300 From: Andy Shevchenko To: Bjorn Helgaas Cc: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= , linux-pci , Sascha Hauer , Alexander Duyck , Alexander Shishkin , Andrew Donnellan , Arnaldo Carvalho de Melo , Arnd Bergmann , Benjamin Herrenschmidt , Bjorn Helgaas , Borislav Petkov , Boris Ostrovsky , "David S. Miller" , Fiona Trahe , Frederic Barrat , Giovanni Cabiddu , Greg Kroah-Hartman , Herbert Xu , "H. Peter Anvin" , Ido Schimmel , Ingo Molnar , Jack Xu , Jakub Kicinski , Jesse Brandeburg , Jiri Olsa , Jiri Pirko , Juergen Gross , Konrad Rzeszutek Wilk , Marco Chiappero , Mark Rutland , Mathias Nyman , Michael Buesch , Michael Ellerman , Namhyung Kim , Oliver O'Halloran , Paul Mackerras , Peter Zijlstra , =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= , Russell Currey , Salil Mehta , Sathya Prakash , Simon Horman , Sreekanth Reddy , Stefano Stabellini , Suganath Prabu Subramani , Taras Chornyi , Thomas Gleixner , Tomaszx Kowalik , Vadym Kochan , Wojciech Ziemba , Yisen Zhuang , Zhou Wang , linux-crypto , Linux Kernel Mailing List , linux-perf-users@vger.kernel.org, "open list:LINUX FOR POWERPC PA SEMI PWRFICIENT" , linux-scsi , USB , "open list:TI WILINK WIRELES..." , MPT-FusionLinux.pdl@broadcom.com, netdev , oss-drivers@corigine.com, qat-linux@intel.com, "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" , xen-devel@lists.xenproject.org Subject: Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver Message-ID: References: <20211013113356.GA1891412@bhelgaas> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20211013113356.GA1891412@bhelgaas> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo Precedence: bulk List-ID: X-Mailing-List: linux-perf-users@vger.kernel.org On Wed, Oct 13, 2021 at 06:33:56AM -0500, Bjorn Helgaas wrote: > On Wed, Oct 13, 2021 at 12:26:42PM +0300, Andy Shevchenko wrote: > > On Wed, Oct 13, 2021 at 2:33 AM Bjorn Helgaas wrote: > > > On Mon, Oct 04, 2021 at 02:59:24PM +0200, Uwe Kleine-König wrote: ... > > It's a bit unusual. Other to_*_dev() are not NULL-aware IIRC. > > It is a little unusual. I only found three of 77 that are NULL-aware: > > to_moxtet_driver() > to_siox_driver() > to_spi_driver() > > It seems worthwhile to me because it makes the patch and the resulting > code significantly cleaner. I'm not objecting the change, just a remark. ... > > > + for (id = drv ? drv->id_table : NULL; id && id->vendor; id++) > > > + if (id->vendor == vendor && id->device == device) > > > > > + break; > > > > return true; > > > > > return id && id->vendor; > > > > return false; > > Good cleanup for a follow-up patch, but doesn't seem directly related > to the objective here. True. Maybe you can bake one while not forgotten? ... > > > + return drv && drv->resume ? > > > + drv->resume(pci_dev) : pci_pm_reenable_device(pci_dev); > > > > One line? > > I don't think I touched that line. Then why they are both in + section? ... > > > + struct pci_driver *drv = to_pci_driver(dev->dev.driver); > > > const struct pci_error_handlers *err_handler = > > > - dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL; > > > + drv ? drv->err_handler : NULL; > > > > Isn't dev->driver == to_pci_driver(dev->dev.driver)? > > Yes, I think so, but not sure what you're getting at here, can you > elaborate? Getting pointer from another pointer seems waste of resources, why we can't simply struct pci_driver *drv = dev->driver; ? ... > > Stray change? Or is it in a separate patch in your tree? > > Could be skipped. The string now fits on one line so I combined it to > make it more greppable. This is inconsistency in your changes, in one case you are objecting of doing something close to the changed lines, in the other you are doing unrelated change. -- With Best Regards, Andy Shevchenko