From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757247AbdEVIkM (ORCPT ); Mon, 22 May 2017 04:40:12 -0400 Received: from mga05.intel.com ([192.55.52.43]:58737 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752433AbdEVIkL (ORCPT ); Mon, 22 May 2017 04:40:11 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,377,1491289200"; d="scan'208";a="1133355461" Date: Mon, 22 May 2017 11:40:05 +0300 From: Mika Westerberg To: Andreas Noever Cc: Greg Kroah-Hartman , Michael Jamet , Yehezkel Bernat , Lukas Wunner , Amir Levy , Andy Lutomirski , Mario.Limonciello@dell.com, Jared.Dominguez@dell.com, Andy Shevchenko , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH 02/24] thunderbolt: Do not try to read UID if DROM offset is read as 0 Message-ID: <20170522084005.GS8541@lahna.fi.intel.com> References: <20170518143914.60902-1-mika.westerberg@linux.intel.com> <20170518143914.60902-3-mika.westerberg@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, May 21, 2017 at 03:46:20PM +0200, Andreas Noever wrote: > On Thu, May 18, 2017 at 4:38 PM, Mika Westerberg > wrote: > > At least Falcon Ridge when in host mode does not have any kind of DROM > > available and reading DROM offset returns 0 for these. Do not try to > > read DROM any further in that case. > > > > Signed-off-by: Mika Westerberg > > Reviewed-by: Yehezkel Bernat > > Reviewed-by: Michael Jamet > > --- > > drivers/thunderbolt/eeprom.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > Hi Mika, > > nice work, it is nice to see Intel contribute to the Thunderbolt > driver (I can second Lukas's 'jaw drop' comment)! > > I will try to read through everything today, but maybe the last few > patches will get pushed back to next weekend. Thanks :) > > diff --git a/drivers/thunderbolt/eeprom.c b/drivers/thunderbolt/eeprom.c > > index 6392990c984d..e4e64b130514 100644 > > --- a/drivers/thunderbolt/eeprom.c > > +++ b/drivers/thunderbolt/eeprom.c > > @@ -276,6 +276,9 @@ int tb_drom_read_uid_only(struct tb_switch *sw, u64 *uid) > > if (res) > > return res; > > > > + if (drom_offset == 0) > > + return -ENODEV; > > + > I think that this will make tb_switch_resume bail out on the root > switch, which is not good. Since the uid is only used to detect > whether a different device was plugged in while the system was > suspended I think that we can safely ignore the uid on the root > switch: > - don't read it in tb_drom_read (route == 0 is already special cased anyways) > - add a special case for the root switch to tb_switch_resume and > don't read the uid - just assume that it did not change (should be > impossible anyways) > > What do you think? I think there actually is such check already in tb_switch_resume() where we special case the root switch ignoring its UID. Unless I'm missing something. I'm testing this on a Mac with Cactus Ridge and the root switch resume does not fail :)