From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753956AbbE1NZI (ORCPT ); Thu, 28 May 2015 09:25:08 -0400 Received: from mga03.intel.com ([134.134.136.65]:17548 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751207AbbE1NZD (ORCPT ); Thu, 28 May 2015 09:25:03 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,513,1427785200"; d="scan'208";a="701563401" Date: Thu, 28 May 2015 16:24:58 +0300 From: Heikki Krogerus To: Sasha Levin Cc: Sudip Mukherjee , Greg KH , Alan Stern , Tal Shorer , USB list , "" , David Cohen , Felipe Balbi , Lu Baolu Subject: Re: [PATCH] usb: ulpi: don't register drivers if bus doesn't exist Message-ID: <20150528132458.GA12315@kuha.fi.intel.com> References: <20150527083933.GA13844@kuha.fi.intel.com> <20150527152116.GA1972@kroah.com> <20150528053938.GB2944@sudip-PC> <55670BDE.5070103@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55670BDE.5070103@oracle.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, May 28, 2015 at 08:36:46AM -0400, Sasha Levin wrote: > On 05/28/2015 01:39 AM, Sudip Mukherjee wrote: > > diff --git a/drivers/base/driver.c b/drivers/base/driver.c > > index 4eabfe2..1acae5b 100644 > > --- a/drivers/base/driver.c > > +++ b/drivers/base/driver.c > > @@ -150,6 +150,11 @@ int driver_register(struct device_driver *drv) > > int ret; > > struct device_driver *other; > > > > + if (!drv->bus->registered) { > > + pr_err("Driver %s registration failed. bus not yet registered\n", > > + drv->name); > > + return -ENODEV; > > + } > > BUG_ON(!drv->bus->p); > > This is a design issue with the code in the layer above, there's no reason > driver_register() should be called with a bus that wasn't registered to > begin with. > > This is why there's a BUG_ON there to catch these issues - it's a bug, not > a desired behaviour. Unfortunately problems with the design are not the only cases why we could end up here before the bus has been registered. If the bus has failed to register, we definitely should not trigger a BUG here. The bus management driver has in that case already made the decision to not BUG. Or if the user is allowed to disable a bus somehow, for example with something like nousb parameter, but we still manage do get here, we should again not trigger BUG(). I don't think BUG_ON here is ever the correct thing to do. This function can see that the bus doesn't exist or possibly that something has gone wrong by checking the "p", but it does not know any details, nor should it. This function should trigger a warning in those cases and return failure, and not make any extra decisions. Thanks, -- heikki