Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1752853AbdHKLvA (ORCPT <rfc822;w@1wt.eu>);
        Fri, 11 Aug 2017 07:51:00 -0400
Received: from mga14.intel.com ([192.55.52.115]:64896 "EHLO mga14.intel.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1752403AbdHKLu7 (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 11 Aug 2017 07:50:59 -0400
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.41,357,1498546800"; 
   d="scan'208";a="888976721"
Date: Fri, 11 Aug 2017 14:50:56 +0300
From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Cc: Michal Suchanek <msuchanek@suse.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Peter Huewe <peterhuewe@gmx.de>, Marcel Selhorst <tpmdd@selhorst.net>,
        linux-kernel@vger.kernel.org, tpmdd-devel@lists.sourceforge.net
Subject: Re: [PATCH] Do not disable driver and bus shutdown hook when class
 shutdown hook is set.
Message-ID: <20170811115056.oegv6ziahja56cp4@linux.intel.com>
References: <20170809213420.2391-1-msuchanek@suse.de>
 <20170809215202.GA21867@obsidianresearch.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170809215202.GA21867@obsidianresearch.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
User-Agent: NeoMutt/20170609 (1.8.3)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1718
Lines: 40

On Wed, Aug 09, 2017 at 03:52:02PM -0600, Jason Gunthorpe wrote:
> On Wed, Aug 09, 2017 at 11:34:20PM +0200, Michal Suchanek wrote:
> > Disabling the driver hook by setting class hook is totally sound design
> > not prone to error as evidenced by the single implementation of the
> > class hook.
> 
> It was done this was for consistency, if you look at the full code:
> 
>                 if (dev->class && dev->class->shutdown) {
>                         if (initcall_debug)
>                                 dev_info(dev, "shutdown\n");
>                         dev->class->shutdown(dev);
>                 } else if (dev->bus && dev->bus->shutdown) {
>                         if (initcall_debug)
>                                 dev_info(dev, "shutdown\n");
>                         dev->bus->shutdown(dev);
>                 } else if (dev->driver && dev->driver->shutdown) {
>                         if (initcall_debug)
>                                 dev_info(dev, "shutdown\n");
>                         dev->driver->shutdown(dev);
>                 }
> 
> The bus disables the driver callback, on the expectation that the bus
> implementation will do it.
> 
> Existing bus implementations do properly chain to driver shutdown (eg
> look at mmc_bus_shutdown) and it appears to have been written like
> this so that the bus can insert code before and after calling the
> driver shutdown.
> 
> Making class act differently from bus seems very confusing, IHMO,
> which why the TPM patch was written to follow the existing pattern.
> 
> Jason

There's also more fundamental problem. There are Fixes tags but no
real regression. Even if this patch made sense I would not consider
it as a bug fix.

/Jarkko