From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qk1-f176.google.com (mail-qk1-f176.google.com [209.85.222.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F142A19ABC6 for ; Tue, 14 Jul 2026 00:25:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.222.176 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783988702; cv=none; b=Fld63IopV1OTGreEG5gk7gRuhI6yLXO2xu1gkZSkDoKlHi8yScPbFlrk5LdK8/3zMYXASph+RAaUDTs+tMXj5gQSQ7ZBKCrYWliLUdhkYznevMdagAZ2uYKkzDFBJq7DqCWrhNxxBmbZdYI/yY8jOvIgnvrU9nMG169hbiQCcWs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783988702; c=relaxed/simple; bh=/JhaswwRXUrVnZuSr9qRcn7rppTZCxzhNmG02h5b/IU=; h=Date:Message-ID:From:To:Cc:Subject:In-Reply-To:References: MIME-Version:Content-Type; b=SaqoJpliETCgXpEs3g4AnB2CkT7NabszNQUh588nud1zYmB8ivWwlL4f10rM56rDkZEMnYh6iiLn2NGRvyiVPDOt2Bmd+A/caACh2xTiTPtLVgognjEJiLEE9wCBS4Bb6s0dDBx0dWyiJZDobYjpe1bhEYhxHz31nE8kgdfYimM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=c127.dev; spf=fail smtp.mailfrom=c127.dev; dkim=pass (2048-bit key) header.d=c127-dev.20251104.gappssmtp.com header.i=@c127-dev.20251104.gappssmtp.com header.b=wWMnRsvi; arc=none smtp.client-ip=209.85.222.176 Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=c127.dev Authentication-Results: smtp.subspace.kernel.org; spf=fail smtp.mailfrom=c127.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=c127-dev.20251104.gappssmtp.com header.i=@c127-dev.20251104.gappssmtp.com header.b="wWMnRsvi" Received: by mail-qk1-f176.google.com with SMTP id af79cd13be357-92e4fd65b2bso34567485a.0 for ; Mon, 13 Jul 2026 17:25:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=c127-dev.20251104.gappssmtp.com; s=20251104; t=1783988700; x=1784593500; darn=vger.kernel.org; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:subject:cc:to:from:message-id:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=/JhaswwRXUrVnZuSr9qRcn7rppTZCxzhNmG02h5b/IU=; b=wWMnRsviOxJD6y6GSIAWaDYgH0lbX3Ihp0jKOC65q5hLEXJorAencb/Ie8hYxFwRGR dO6Sa9yB1A+LvVijqX1pXnVzjeqHVmeyHC6fIuuj77Wq3QbweBry2RRntOai4Q70Xdg/ wQ3eo1RppRwznu+1TM9V6SuOCnohJyLiE65spTpqRA5CUmK+YtxS3Azj1Vvvk21vCq73 kyUBllDoA4lqlHJdPzX2m9myxUCeC3Xgv/5WdxjbPMX46G90Nj7qmhLmHdUhw9q7/7Gh AExTFKZvjNirlYCSaI4uten1ZVMKLbUqUg7P/ws1EeKZP5JipdSrSz2YwP0MlQ/eLgid 2ZYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783988700; x=1784593500; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:subject:cc:to:from:message-id:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=/JhaswwRXUrVnZuSr9qRcn7rppTZCxzhNmG02h5b/IU=; b=bIaBGy3UDAHJsZJh1mgX81f0/Kk7d9MjQGFuKst7vQ/6bAfRtbAafk/1MKlA1tSFNZ qyEvfNNG1yoaJ97mXfO72BRvxLrk5yxVJQh+IjD4QS7KnESuuRWJQzxiyAaKUqqg4Bsj t1jJkYEWEoHORcwVQSG4tcAQy8MI+kHCGqImO5JJ96cm0LeEyhbObrlq8hp6c9xIfE6I z2bY6cQvzoXjWC5tfKsyMp5mpWQXDyDVB+yFo4r0ZcMhqlOt8Z1DRNQw3raQeTimGIyO q4vuw2qYTOENMG9eQFUUz5Jjh5MHO5cXp68luz2XvBey/P/WUt4Ru5/u4LdGEAHxJHjp 5aCA== X-Forwarded-Encrypted: i=1; AHgh+RpTdsFglEuz1PKvBcEsNcbp4BwRYim5PCu/PU8dRYzVY6hCp7ErJd98rZlQt82ogvcfs2eWxnyOcthnO+0=@vger.kernel.org X-Gm-Message-State: AOJu0YyJKVh2LafmhtZo5iFbYxwP2fg2tPGF5VN2I7Dv5a41UCJZijzZ sIEjpc1x50UgPdh8So3AlqhDck2eZUf2E4Pvimj+7bG9AAQzWwuz1UbQjEKIqbST5DX+ X-Gm-Gg: AfdE7ckmboMuZJ1tUn/FY+XPNkSjI5zrm2RDnncqvoSRwP6doumUyjFSl50T8Ruq4EJ GIInMwssHwsSUV3CwevEFGEIibZlw+nQs6sztAFHy1A/+ML46hZ67I47kqLdrhfalAyoNY6kevR aeriRTEU7WYGe4dEHjFGb0faakfAybPSojs5QLdycww5ec6lNlMGWoSOL2gpsgBoIcB0iAnS4h0 9+C+1ej96JxbKpri/IPNXTfLW4QhfUyb5szeZAuCzQPCFrkhzFn3L1B9EcLyC3GESBetdQNgyA8 RXzyqlV9osULKsZOfrtJthltmhUx5GsSkbXRySYSy9aXhu/0vtFMYXxRq9Z0MWzYYt2fVOFHbQ4 KDQGUV2DZDZSBN8YlhU+ZL74Z5Eml5tdJE+IenWNqVR0UQ+p9Qeff71EEO9fpGg7Zjjzi8EAKsh /0PlY9a7PzQv1APS9VyI8cqu+BShLzEWKcGqAHqA68zaCdWw== X-Received: by 2002:a05:620a:3949:b0:915:6c63:b9eb with SMTP id af79cd13be357-92ef2c1fc16mr1148399685a.47.1783988699839; Mon, 13 Jul 2026 17:24:59 -0700 (PDT) Received: from localhost (ec2-3-88-202-199.compute-1.amazonaws.com. [3.88.202.199]) by smtp.gmail.com with ESMTPSA id af79cd13be357-92ee5d30f38sm1210162085a.34.2026.07.13.17.24.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Jul 2026 17:24:59 -0700 (PDT) Date: Tue, 14 Jul 2026 00:24:59 +0000 Message-ID: <03309c370652c5ca36cf9972756efa5c@c127.dev> From: Johan Alvarado To: Mieczyslaw Nalewaj Cc: linusw@kernel.org, alsi@bang-olufsen.dk, andrew@lunn.ch, olteanv@gmail.com, kuba@kernel.org, davem@davemloft.net, edumazet@google.com, pabeni@redhat.com, linux@armlinux.org.uk, luizluca@gmail.com, maxime.chevallier@bootlin.com, kuncy7@gmail.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH net-next v6 2/2] net: dsa: realtek: rtl8365mb: add HSGMII support for RTL8367S In-Reply-To: <7b164d90-b57f-4746-8b7f-b5bb7fd7fe51@yahoo.com> References: <20260711-rtl8367s-sgmii-v6-0-88f7944ddca7@c127.dev> <20260711-rtl8367s-sgmii-v6-2-88f7944ddca7@c127.dev> <7b164d90-b57f-4746-8b7f-b5bb7fd7fe51@yahoo.com> 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=utf-8 Content-Transfer-Encoding: 8bit Hi Mieczyslaw, On 7/12/2026 8:05 PM, Mieczyslaw Nalewaj wrote: [...] > As discussed earlier in the thread: the rate limiter helper, as well > as pcs_config()/RTL8365MB_SDS_BYPASS_LINE_RATE_MASK, currently assume > the SerDes is always muxed to external interface 1 / port 6 > (RTL8365MB_SDS_EXT_INTERFACE_ID / _PORT). That's true for every chip > currently in rtl8365mb_chip_infos[] (RTL8367S and RTL8367SB both have > their SGMII/HSGMII-capable extint at { 6, 1, ... }), so nothing is > broken today. > > I still think it's worth guarding against this ahead of time though. > It isn't an architectural constant of the family, just something that > happens to hold for the two chips currently in the table, and there's > no guarantee a future chip won't mux its SerDes to a different port > or extint id. I see it the other way around: for the family this driver covers, the port 6 muxing is architectural. Every entry in rtl8365mb_chip_infos[] is chip id 0x6367, and on this silicon the SerDes mux is a pair of bits in SDS_MISC that select SGMII or HSGMII specifically for MAC8 - that is what the RTL8365MB_SDS_MISC_MAC8_SEL_* names in the driver reflect. Neither the register set nor the vendor driver offers a way to route the SerDes anywhere else on this chip id. A chip that muxes its SerDes to a different interface would not be a new table entry; it would be different silicon with a different SerDes datapath. Your draft illustrates this, I think: it makes the DIGITAL_INTERFACE_SELECT write follow mb->sds_id, but the SerDes mux write in pcs_config() still sets the MAC8_SEL bits unconditionally, because there is nothing else it could set. On the hypothetical future chip, the generalized path would program the interface mode for the cached id while still muxing the SerDes to MAC8 - as broken as before, only harder to see, because the code now looks as if it handled the case. The bypass mask has the same problem on a different axis. As came up in the v4 review, the bit layout of the line rate bypass register is not a function of the port number across the wider RTL8367 family; the BIT(port - 5) relationship in RTL8365MB_BYPASS_LINE_RATE_MASK() is the layout of this chip id specifically. The same goes for the SerDes tuning tables, the chip option probe and the rate limiter register addresses: all of them are tied to this silicon, not derivable from the extint table. Caching the extint port and id would generalize the two easiest parameters and leave every hard one behind. > Since rtl8365mb_sds_probe_option() already walks > chip_info->extints[] looking for the SGMII/HSGMII-capable one, I'd > like to have it cache the discovered port and id (mb->sds_port / > mb->sds_id) instead of relying on the fixed > RTL8365MB_SDS_EXT_INTERFACE_ID/_PORT. pcs_config() and the bypass > line-rate mask would then use the cached values, and > rtl8365mb_sds_raise_rate_limits() would check mb->sds_port == 6 and > warn+skip instead of assuming, since only the port 6 register > addresses are known/verified so far. The warn+skip in particular I think would be actively harmful: skipping the write means that hypothetical chip ships with the reset-default ~1.048 Gbps cap - the exact problem this patch exists to fix, reduced to a one-line boot warning. The MR85X report showed how easy that cap is to miss; it took multi-client throughput testing to even notice it. I would rather keep the assumption a hard, greppable constant next to a comment stating it, so that whoever adds such a chip trips over it while writing the chip_info entry and brings the SerDes path up against real hardware - which they will have to do anyway, for the mux, the tuning tables, the option probe and the limiter addresses alike. So I would prefer to keep the series as is. If a chip with the SerDes on another interface ever appears, its submitter will have a datasheet or vendor code in hand and can parametrize exactly what that silicon needs, with values that can be verified. Doing it now would generalize the two parameters we can test and guess at the rest. Best regards, Johan