From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtpout-04.galae.net (smtpout-04.galae.net [185.171.202.116]) (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 E772E3A961F; Thu, 5 Mar 2026 14:46:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.171.202.116 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772721969; cv=none; b=GmoasorIo61jzQLp682yqYFoMhpcb+yg3t1jc4ZljzACEP6HY4YOjUSE1x9aUnjhHb8ZdQeSdDiVoILXAid9W6yAKmkUkTWwkVEFJr8ipWJEUgdDDPGxW/+w/BwQyiMo3gXUWEkIGbcjIgfHduOW0mdfhKvytqlRIrNMsvzYcHM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772721969; c=relaxed/simple; bh=7Fy8244GRUUip69C456QiN1ZM7elVxNq7hb0gn5Awbw=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=DFGfJJRFVeWQApPxAj+y+xF33/1ZalX08x4B7c050Y6secwfCMEwNPEfHrHtIzfnxeGCjcSzAoSDhRmHxgO69DZ/AdemR+xm+DtF/OcUxcB0obSZy2s84erT7VpO+/Wy/xegW9QYe6oXyof8KAG5eOHxc6gcVqZz4TGC9t2+8F8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=iHj2nspi; arc=none smtp.client-ip=185.171.202.116 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="iHj2nspi" Received: from smtpout-01.galae.net (smtpout-01.galae.net [212.83.139.233]) by smtpout-04.galae.net (Postfix) with ESMTPS id 96F4FC4041B; Thu, 5 Mar 2026 14:46:22 +0000 (UTC) Received: from mail.galae.net (mail.galae.net [212.83.136.155]) by smtpout-01.galae.net (Postfix) with ESMTPS id CDD995FDEB; Thu, 5 Mar 2026 14:46:03 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id 44B3B103697B5; Thu, 5 Mar 2026 15:45:58 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=dkim; t=1772721962; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:content-language:in-reply-to:references; bh=bIN4i+aSgD4SV8pfn3MjG18z5RHwGaMJsyLvyjMRK5M=; b=iHj2nspiTCO247d5OrTLhEpv7bpCnCZMIkufvLZQ1uOIFX3Sejf1XhsEzd9AQy5Rh+S4sq vpX4XzSjM5d+X6TLIciKp+2r89LzWv51RdJpv8WlXe83IadQaip58dFqgKkHn0tjhWc1zg TlAAt7F6MUPkqBJzGsB6p/WmI8i/dmenWxouQn0QjGpF13yJF5wHiKR1YjuXZ5QYDAT3jX qMcDXsYHnOUl032BTwrijiaO6ShqSrMttBSyAiX3VyFtaeRjuK6u6kHbCJmReusOT8C9lC OEuCMCszO2dmkOvcwtM0PDqb4Xz62V1KiskmNL9rZ97v6Ti8hls9O8qBm3CDbA== Message-ID: Date: Thu, 5 Mar 2026 15:45:57 +0100 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: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq To: Vladimir Oltean Cc: Woojung Huh , UNGLinuxDriver@microchip.com, Andrew Lunn , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Richard Cochran , Simon Horman , Pascal Eberhard , =?UTF-8?Q?Miqu=C3=A8l_Raynal?= , Thomas Petazzoni , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Maxime Chevallier , Tristram.Ha@microchip.com References: <20260304-ksz8463-ptp-v6-0-3f4c47954c71@bootlin.com> <20260304-ksz8463-ptp-v6-1-3f4c47954c71@bootlin.com> <20260305095656.vlyaztv6nbdqrmil@skbuf> <98944cef-0877-4fb9-83a0-92bbd3852f66@bootlin.com> <20260305125149.ejju5ptrkviqi3sm@skbuf> From: Bastien Curutchet Content-Language: en-US In-Reply-To: <20260305125149.ejju5ptrkviqi3sm@skbuf> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Last-TLS-Session-Version: TLSv1.3 On 3/5/26 1:51 PM, Vladimir Oltean wrote: > On Thu, Mar 05, 2026 at 01:39:29PM +0100, Bastien Curutchet wrote: >> Hi Vladimir, >> >> On 3/5/26 10:56 AM, Vladimir Oltean wrote: >>> On Wed, Mar 04, 2026 at 11:18:52AM +0100, Bastien Curutchet (Schneider Electric) wrote: >>>> @@ -2890,14 +2899,18 @@ static irqreturn_t ksz_irq_thread_fn(int irq, void *dev_id) >>>> unsigned int nhandled = 0; >>>> struct ksz_device *dev; >>>> unsigned int sub_irq; >>>> - u8 data; >>>> + u16 data; >>>> int ret; >>>> u8 n; >>>> dev = kirq->dev; >>>> - /* Read interrupt status register */ >>>> - ret = ksz_read8(dev, kirq->reg_status, &data); >>>> + /* >>>> + * Most of the KSZ switches have a 8-bits long interrupt status >>>> + * register, but the KSZ8463 has a 16-bits long one. The overread here >>>> + * is safe because we only iterate over kirq->nirqs in the below loop. >>> >>> FWIW, this isn't the only thing making an overread "safe". >>> If the adjacent register also has "clear on read" semantics, that's not >>> good. >>> >>> I can't tell whether that's the case here, though. There are just too >>> many hardware variations to check for. I just wanted to point out that >>> the reasoning is incomplete. >>> >> You're right, I hadn't thought about the 'clear on read' case. >> >> I'll use ksz_read16() only for the KSZ8463 to be 100% sure it won't break >> anything for other switches. > > I'm still on the fence on whether to say this or not, because I don't > really want to get so involved in internal driver bookkeeping, but... > this driver is just becoming a hell to review, even if I want to > concentrate exclusively on correct API use, like I try to do. > > Linus Walleij has added a new ks8995 driver, which has some overlap with > the common ksz driver for the KSZ8 family. Now he wants to remove the > overlapping device support: > https://lore.kernel.org/netdev/20260219-ks8995-fixups-v3-0-a7fc63fe1916@kernel.org/ > > Maybe we should go the other way around, migrate KSZ8 support to the > ks8995 driver instead? The common ksz driver is becoming just extremely > convoluted to handle all hardware variations. Would it help in any way > to maintain cleaner code paths, what do you think? I agree, this driver is extremely convoluted because of all the different hardware it handles. It wasn't easy to fit PTP support for the KSZ8463 into it. And I encountered the same kind of difficulties when adding periodic output support (I have another series ready for this once this PTP support will be merged). Regarding migrating the KSZ8 support into the ksz8995, I think we would quickly hit the same pain points than in ksz_common. Even inside the KSZ8 family we can find a quite big amount of differences between the switches. For instance, both KSZ8463 and KSZ8563 support PTP, they share lots of common registers but their interrupt scheme is very different. I've added Tristram in Cc:, who works at Microchip. Maybe, Tristram, you have some insights about which switches could share code if we decide to split the big ksz_common into several drivers ? Best regards, Bastien