From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx.nabladev.com (mx.nabladev.com [178.251.229.89]) (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 D35743D5258; Tue, 14 Apr 2026 10:26:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=178.251.229.89 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776162415; cv=none; b=rn7bW8ybLWpEjV6tYnGb05xvk5klfevPcFpc99ufo2EG+otBG/AL/MUfyK5GinBatyQCXpRdJNe2a+HrLMZqtictGRc1+5ecnhM92vWJy68KEOeYjHnFLuzja91CseoWi/kxYWG4CdeejnUxP2uE9uGKE0c+RN3NnKVznvUMj0E= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776162415; c=relaxed/simple; bh=ZfFf+Xb9PptclsHbGofHnbDruAIXwhUSQ+T+mgmk3Qo=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=GYOdicx91ZebKe9z0wEQznAhqEBN+i8AWxULbruGIhFnolGbSUC6xhj4Dk3sjhS7o0Aoq60aS8Rn+KnZ2rkgjckeG75EGMxmz0HeVA6FdkUHpTEauWZ4vfuBLYzvQJGPc5RsvVLNIiu1fNK1s+JguHajAR2QBi5IAfCpkLmtCBE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=nabladev.com; spf=pass smtp.mailfrom=nabladev.com; dkim=pass (2048-bit key) header.d=nabladev.com header.i=@nabladev.com header.b=WkHVGH+j; arc=none smtp.client-ip=178.251.229.89 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=nabladev.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=nabladev.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=nabladev.com header.i=@nabladev.com header.b="WkHVGH+j" Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id 9296510CE86; Tue, 14 Apr 2026 12:26:42 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nabladev.com; s=dkim; t=1776162404; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:content-language:in-reply-to:references; bh=QUw1GVmNa8Tm46eOKkaFDELo/gPbvua9u5RkS9ks50Q=; b=WkHVGH+jj8fht7LcddhRDZghm6vajSldF7jCuPPld8XaJDmTaavEKLjPoyAyiSo++KUoeB yY6lRuBcEQxo02s6xWZd2oP88iW/C9ZeUYfQElpKoQq6jIbpSGEJXAWXpZUsVll3RdYw/K z6BusZnCnsxCW3891oKe2Dv+Yo02pJL3mhCPPQiCtgjPYkXCJ0sl/vWA8DNsQohqqACxN9 fAg1LL7BanNxdvRBXbnxQ3L2cfOENpERevQTsStSd+PtfepD8mRGDARaXkuRlCQjNkqjot j+m+QXBfJXMu5IQox2zqitdZoPEG+Xwzgp6Icb9hjcr2WZJ6fzL5Pm9hwanYjQ== Message-ID: Date: Tue, 14 Apr 2026 12:26:41 +0200 Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [net,PATCH v2] net: ks8851: Reinstate disabling of BHs around IRQ handler To: Sebastian Andrzej Siewior Cc: Jakub Kicinski , netdev@vger.kernel.org, stable@vger.kernel.org, "David S. Miller" , Andrew Lunn , Eric Dumazet , Nicolai Buchwitz , Paolo Abeni , Ronald Wahl , Yicong Hui , linux-kernel@vger.kernel.org, Thomas Gleixner References: <20260408162535.98108-1-marex@nabladev.com> <20260412090141.21bf1534@kernel.org> <2558832d-c821-436d-898d-b708c5e0a228@nabladev.com> <20260412105125.48f0c58f@kernel.org> <20260413125744.TVKkZcEK@linutronix.de> <16fdeec9-9208-4c9b-b228-d6c6e045e116@nabladev.com> <20260413160336.GQCaw-1d@linutronix.de> <20260414085556.SJSDwbpW@linutronix.de> Content-Language: en-US From: Marek Vasut In-Reply-To: <20260414085556.SJSDwbpW@linutronix.de> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Last-TLS-Session-Version: TLSv1.3 On 4/14/26 10:55 AM, Sebastian Andrzej Siewior wrote: > On 2026-04-13 18:03:38 [+0200], To Marek Vasut wrote: >> On 2026-04-13 17:31:34 [+0200], Marek Vasut wrote: >>>> I don't see why it needs to disable interrupts. >>> >>> Because when the lock is held, the PAR code shouldn't be interrupted by an >>> interrupt, otherwise it would completely mess up the state of the KS8851 >>> MAC. The spinlock does not protect only the IRQ handler, it protects also >>> ks8851_start_xmit_par() and ks8851_write_mac_addr() and >>> ks8851_read_mac_addr() and ks8851_net_open() and ks8851_net_stop() and other >>> sites which call ks8851_lock()/ks8851_unlock() which cannot be executed >>> concurrently, but where BHs can be enabled. >> >> I need check this once brain is at full power again. But which >> interrupt? Your interrupt is threaded. So that should be okay. > > I don't understand. There is no point in using spin_lock_irqsave() in > ks8851_lock_par(). You don't protect against interrupts because none of > the user actually run in an interrupt. As far as I can see, the > interrupt is threaded and the mdio phy link checks should come from the > workqueue. Ha, now that the IRQ handler is indeed only threaded, I can use spin_lock_bh() indeed. I will send a V3 like that. > What is wrong is that the ndo_start_xmit callback can be invoked from a > softirq and such you must disable BHs while acquiring a lock which can > be accessed from both contexts. Therefore spin_lock() is not sufficient, > it needs the _bh() and _irq() brings no additional value here.