From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from vps0.lunn.ch (vps0.lunn.ch [156.67.10.101]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B999919149F; Sat, 16 Nov 2024 19:23:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=156.67.10.101 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731784998; cv=none; b=qm0rgR8XxyUhgtPbQ4aHga0FoKzZoBjNB+v3XaPyIGstZcGjv3BQimDyAcmNVZXpHl3zOVDoP3Z1h7G9lD+Ja4btD7LxB+7whU3B6Nd7Wopl/DUL942Lhn7Zmw7220sy1ldBdiAKOVLbvHj/+K6F+QuNbHgGc8qPPgFMtGzlGDw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731784998; c=relaxed/simple; bh=yfX/6TZQ1gd9+8AMgBgNPFETIp1Fe4WBrJJhmJztd/I=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=P1qN3RWW7epoR+V2Q6vRa2nmYbLa4Pm/nwn/m/4RwJbBeowuxkKm+cjzguqrpl3v6X+URPjXM2bqFzCqSQfLFW0RVHVw+I6bD53UQ5XtyEylRM7VO648RGfoiVQ0X9bOJG80DKuEtAHoniNJMsKqe+VuY8OQUKkWsISE2TEQWs8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=lunn.ch; spf=pass smtp.mailfrom=lunn.ch; dkim=pass (1024-bit key) header.d=lunn.ch header.i=@lunn.ch header.b=gdRfOcvT; arc=none smtp.client-ip=156.67.10.101 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=lunn.ch Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=lunn.ch Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=lunn.ch header.i=@lunn.ch header.b="gdRfOcvT" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Disposition:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:From:Sender:Reply-To:Subject: Date:Message-ID:To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Content-Disposition:In-Reply-To:References; bh=dn+2cDfRC1yH4NOcw8ysKYN7T3VzTAsD6jJX6tRQuE0=; b=gdRfOcvTKwxKkLKP+bQtMsvxeP Zcxq1PDSbPRXkd6PvRH478+Xa11lrRN3cdrqac8/EN9m0pGru+9CaGqImJwB9y2vem4kTJ9INuRjQ 3BNTlCTAlH6WrvWOK3XO5BHaKsjGnmQlg/tFjALWn9OFuc6IOvE4cK8R+h+z/XuTo7NM=; Received: from andrew by vps0.lunn.ch with local (Exim 4.94.2) (envelope-from ) id 1tCONh-00DXYh-Cb; Sat, 16 Nov 2024 20:22:53 +0100 Date: Sat, 16 Nov 2024 20:22:53 +0100 From: Andrew Lunn To: Parker Newman Cc: Paolo Abeni , Alexandre Torgue , Jose Abreu , Andrew Lunn , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Maxime Coquelin , Thierry Reding , Jonathan Hunter , netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org, Parker Newman Subject: Re: [PATCH v1 1/1] net: stmmac: dwmac-tegra: Read iommu stream id from device tree Message-ID: References: <20241115135940.5f898781.parker@finest.io> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20241115135940.5f898781.parker@finest.io> On Fri, Nov 15, 2024 at 01:59:40PM -0500, Parker Newman wrote: > On Fri, 15 Nov 2024 18:17:07 +0100 > Paolo Abeni wrote: > > > On 11/15/24 17:31, Parker Newman wrote: > > > From: Parker Newman > > > > > > Read the iommu stream id from device tree rather than hard coding to mgbe0. > > > Fixes kernel panics when using mgbe controllers other than mgbe0. > > > > It's better to include the full Oops backtrace, possibly decoded. > > > > Will do, there are many different ones but I can add the most common. > > > > Tested with Orin AGX 64GB module on Connect Tech Forge carrier board. > > > > Since this looks like a fix, you should include a suitable 'Fixes' tag > > here, and specify the 'net' target tree in the subj prefix. > > > > Sorry I missed the "net" tag. > > The bug has existed since dwmac-tegra.c was added. I can add a Fixes tag but > in the past I was told they aren't needed in that situation? > > > > @@ -241,6 +243,12 @@ static int tegra_mgbe_probe(struct platform_device *pdev) > > > if (IS_ERR(mgbe->xpcs)) > > > return PTR_ERR(mgbe->xpcs); > > > > > > + /* get controller's stream id from iommu property in device tree */ > > > + if (!tegra_dev_iommu_get_stream_id(mgbe->dev, &mgbe->iommu_sid)) { > > > + dev_err(mgbe->dev, "failed to get iommu stream id\n"); > > > + return -EINVAL; > > > + } > > > > I *think* it would be better to fallback (possibly with a warning or > > notice) to the previous default value when the device tree property is > > not available, to avoid regressions. > > > > I debated this as well... In theory the iommu must be setup for the > mgbe controller to work anyways. Doing it this way means the worst case is > probe() fails and you lose an ethernet port. New DT properties are always optional. Take the example of a board only using a single controller. It should happily work. It probably does not have this property because it is not needed. Your change is likely to cause a regression on such a board. Also, is a binding patch needed? Andrew