RE: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4

From: <Mario.Limonciello@dell.com>
To: <mika.westerberg@linux.intel.com>
Cc: <linux-usb@vger.kernel.org>, <andreas.noever@gmail.com>,
	<michael.jamet@intel.com>, <YehezkelShB@gmail.com>,
	<rajmohan.mani@intel.com>,
	<nicholas.johnson-opensource@outlook.com.au>, <lukas@wunner.de>,
	<gregkh@linuxfoundation.org>, <stern@rowland.harvard.edu>,
	<anthony.wong@canonical.com>, <linux-kernel@vger.kernel.org>
Subject: RE: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
Date: Wed, 2 Oct 2019 15:04:33 +0000	[thread overview]
Message-ID: <46d927da41d9430196f9aed10ed33d91@AUSX13MPC105.AMER.DELL.COM> (raw)
In-Reply-To: <20191002083446.GF2714@lahna.fi.intel.com>

> -----Original Message-----
> From: Mika Westerberg <mika.westerberg@linux.intel.com>
> Sent: Wednesday, October 2, 2019 3:35 AM
> To: Limonciello, Mario
> Cc: linux-usb@vger.kernel.org; andreas.noever@gmail.com;
> michael.jamet@intel.com; YehezkelShB@gmail.com; rajmohan.mani@intel.com;
> nicholas.johnson-opensource@outlook.com.au; lukas@wunner.de;
> gregkh@linuxfoundation.org; stern@rowland.harvard.edu;
> anthony.wong@canonical.com; linux-kernel@vger.kernel.org
> Subject: Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
> 
> 
> [EXTERNAL EMAIL]
> 
> On Tue, Oct 01, 2019 at 05:05:09PM +0000, Mario.Limonciello@dell.com wrote:
> > > @@ -322,9 +398,21 @@ static int tb_switch_nvm_add(struct tb_switch *sw)
> > >  	u32 val;
> > >  	int ret;
> > >
> > > -	if (!sw->dma_port)
> > > +	if (!nvm_readable(sw))
> > >  		return 0;
> > >
> > > +	/*
> > > +	 * The NVM format of non-Intel hardware is not known so
> > > +	 * currently restrict NVM upgrade for Intel hardware. We may
> > > +	 * relax this in the future when we learn other NVM formats.
> > > +	 */
> > > +	if (sw->config.vendor_id != PCI_VENDOR_ID_INTEL) {
> > > +		dev_info(&sw->dev,
> > > +			 "NVM format of vendor %#x is not known, disabling
> NVM
> > > upgrade\n",
> > > +			 sw->config.vendor_id);
> > > +		return 0;
> > > +	}
> > > +
> >
> > Don't you actually have an attribute you can use here for this exact purpose
> that you
> > could  be setting rather than returning immediately?
> > sw->no_nvm_upgrade
> >
> > Then potentially you can at least let users "dump out" the nvm on !Intel but
> don't let
> > it be written until ready to relax further.
> 
> Problem is that we currently read NVM version and size from a known
> location in the NVM. If we don't know the format we can't do that so
> that would mean we need to expose active NVM with some size and hide
> nvm_version. I would rather have this support included in the kernel
> driver and expose standard set of attributes but no strong feelings from
> one way or another.

I agree with you; this is a safer and smarter approach to wait until details of format
for other vendors is known and export attributes only when it is.  This will also
encourage other vendors to open up their format if the only way to access NVM is
to document the headers.