From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) (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 0DEB82E22B5 for ; Fri, 3 Apr 2026 09:35:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.42 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775208936; cv=none; b=aLNgs4fo1J/aVRzP2oiQuUqC/URfBSKj2Cjurl4zHyzUiVVxn6OSCLNOMJhYgbrvnnQIuqh413Z0KBTbqV0TjWq19k+ksO+nb+ym5lEkAQ12faX5tw2xz1OIOV/cUrzof42YhjK+X/IjelVRRXHBJzBnvqkb9ZrkUqsmKxRPMAg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775208936; c=relaxed/simple; bh=CGaTbTzIdYwuRlEh+YKKBHzWhKzKyqy2VXhXB7t6eBc=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=oIVeTmW6CLsFwPwvyLUg1/hXvvKwHEvu7n4YoGVJhhio+12ATPrZ0jsqDLO7RKtnZrSaeTJbcu9ncBd5a+wxzgacBA8mGpOGe9lxN0EmRF3lN3eHJ4UrRnccAHJ2On+cjqodWeE/HFxwsGWgRhe7II386+GnIzVxuLN/Lc0PrsY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=nW3JFyGr; arc=none smtp.client-ip=209.85.128.42 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="nW3JFyGr" Received: by mail-wm1-f42.google.com with SMTP id 5b1f17b1804b1-4887f49ec5aso23658225e9.1 for ; Fri, 03 Apr 2026 02:35:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1775208932; x=1775813732; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=b+7bh6JYJimFo01dLJuKHAMSI9ybhWnOBWq0Gz5gOhs=; b=nW3JFyGrzFRpkm/gQghHrjA282BtnDL2ZSyehbBk1sVw1nNcrIoey0HbrEuxFSgSMB 9BcFnOLeUUtDFJCdsat99H3hBRwd2EZp35YdP1ubIh2SNNt8I1x0ue8V7hf0Lmh/YLQT aGZjVsCRczSS3HRBsFHdgdC8NcUgjuEanlpzQy3EyapX1iUFBIJLAuAMOXuzZXEIjWZ6 HcHn/XlTu+vSkQxpoqg5HlIzKWCZ+tmI4uhWcwHRPER/knm7ym1v328r3aKTVAE5CfhH yz+odeWl2caLXz19LIiJ41l0UxKzLd37e5RRnHgMocLpYEVrTIX6R+npJ347Kz4jBVlI VjkA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1775208932; x=1775813732; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=b+7bh6JYJimFo01dLJuKHAMSI9ybhWnOBWq0Gz5gOhs=; b=jWSA5Y0tR+/E7UvTix2sDt4BpUUEfsPIc4s87LsiC6KlLNOmwk7TPQ63OvlAGQX6mq /13IoU2+RnuuilLXfdTfA5kQh7ry+bq1jItRYuPmDZqoEv0zYHxv8CQY3igftbNsW18e SLc0VmVfjqeZTctFkvRwqnfrqeRuykaKBh1aY4f/UpZ9CQ+jQo6NJF3+X9geB9TldpyU kBWa04TQBCVFtr3g5Y09H/OzYzVR0nKKDBTRf/+rCit98DU+MJrHZVZGthocaDgg4hY9 D2Z1y8B+NW+dTNZQ83WiSBs87rGjWZr/avj9tLq12F8ngcEwVemd8IRMg4hPFx/hs/en NJcg== X-Forwarded-Encrypted: i=1; AJvYcCX07RtmnyRYF5ldblCQqOrokbnc8waqprhgZ54UpHnl4Ibe4aC4DI0i+kdB85oh+kI2f5df8z50wPVs@vger.kernel.org X-Gm-Message-State: AOJu0YxE5jCJI98G49x31GfwrmEWWA5zjk//JtTmd2ghPiFIDtWuvoZ/ oljsNqiEaiCmgdvpsWYDDjSY4dSRT2FNz2BXooRz/Eh9QR03KDgNj4JC X-Gm-Gg: ATEYQzzHX7EFdEJ9oaNGw2+AY5uGPuTaHJkCJyEDoKWqkwfnaVj46F/gztEqoLuLTh+ CnN+pgcRvTU7YqTm69xqeh7oAjaKXAN3TMfygFmFhIX2MEavXFb+dfwcsUquEIG94VCEbAlgRub x7mUaRnzeJONiR+l3z8nmQjUGayiUKxQATrgUcRWfmEwTwNlbeoGPdShR1ZOfbJmJzMGaY4vQYK hl7uxm5dQxvisT7tHGyUxdnYdsB3p2wV2EW3YsqWNV4N59BQxGbszHh/ApegGT4lsFd5SRZ8tx8 acoDXqUt7fu7xtVA5Kb2QThBGoPU8hvrem4m4PpYIJmqyY2oWLgUIEP1oyYKWgMb2nXoWb8YTaf gA0TYAHZhgmJg8zUDzZJ+afkDLrZAoqNYUCTcZSt+kZpVqIkUbb26/w/auxW0i8EGT6QRlgj4nr NA9YXkn0Y6UvoHgcQEugLDGu3tA/6YNlGLODaEOIOyRz4J2lnS6JdeGAE9Bxixo1pISYjgiLISZ rUEjJr70eevTNv7YgpO X-Received: by 2002:a05:600c:698d:b0:488:7f49:eae5 with SMTP id 5b1f17b1804b1-48899780926mr36577785e9.16.1775208931761; Fri, 03 Apr 2026 02:35:31 -0700 (PDT) Received: from [10.1.4.108] (cust-east-par-46-193-119-166.cust.wifirst.net. [46.193.119.166]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-488981e8e66sm16662805e9.10.2026.04.03.02.35.30 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 03 Apr 2026 02:35:31 -0700 (PDT) Message-ID: Date: Fri, 3 Apr 2026 11:35:30 +0200 Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 3/3] net: dsa: microchip: implement KSZ87xx Module 3 low-loss cable errata To: Vladimir Oltean , Bastien Curutchet Cc: Woojung Huh , UNGLinuxDriver@microchip.com, Andrew Lunn , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Marek Vasut , Maxime Chevallier , netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Fidelio Lawson References: <20260326-ksz87xx_errata_low_loss_connections-v1-0-79a698f43626@exotec.com> <20260326-ksz87xx_errata_low_loss_connections-v1-0-79a698f43626@exotec.com> <20260326-ksz87xx_errata_low_loss_connections-v1-3-79a698f43626@exotec.com> <20260326-ksz87xx_errata_low_loss_connections-v1-3-79a698f43626@exotec.com> <20260326094211.hdaf4tz7lbjyjznn@skbuf> Content-Language: en-US From: Fidelio LAWSON In-Reply-To: <20260326094211.hdaf4tz7lbjyjznn@skbuf> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 3/26/26 10:42, Vladimir Oltean wrote: > On Thu, Mar 26, 2026 at 10:10:23AM +0100, Fidelio Lawson wrote: >> Implement the "Module 3: Equalizer fix for short cables" erratum from >> Microchip document DS80000687C for KSZ87xx switches. >> >> The issue affects short or low-loss cable links (e.g. CAT5e/CAT6), >> where the PHY receiver equalizer may amplify high-amplitude signals >> excessively, resulting in internal distortion and link establishment >> failures. >> >> Depending on the selected workaround (1 or 2), the driver writes a >> specific value to the indirect PHY register >> using the 6E/6F/A0 indirect access mechanism. >> >> The errata fix is applied during global switch initialization when >> enabled via device tree. >> >> Signed-off-by: Fidelio Lawson >> --- >> drivers/net/dsa/microchip/ksz8.c | 46 ++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 46 insertions(+) >> >> diff --git a/drivers/net/dsa/microchip/ksz8.c b/drivers/net/dsa/microchip/ksz8.c >> index 78b42cf50ce2..b6f3a1ce85fc 100644 >> --- a/drivers/net/dsa/microchip/ksz8.c >> +++ b/drivers/net/dsa/microchip/ksz8.c >> @@ -1901,6 +1901,41 @@ void ksz8_phylink_mac_link_up(struct phylink_config *config, >> ksz8_phy_port_link_up(dev, port, duplex, tx_pause, rx_pause); >> } >> >> +static int ksz8_handle_module3_errata(struct ksz_device *dev) >> +{ >> + int ret = 0; > > "ret" does not need to be zero-initialized. It is unconditionally > overwritten by ksz_write8(). > > And please sort lines with variable declarations in decreasing line > length order. Netdev calls this "reverse Christmas tree" ordering and is > the preferred coding style. > >> + const u16 *regs = dev->info->regs; >> + u16 indir_reg = 0x0000; >> + u8 indir_val = 0x00; >> + >> + switch (dev->low_loss_wa_mode) { >> + case KSZ_LOW_LOSS_WA_1: >> + indir_reg = 0x3C; >> + indir_val = 0x15; >> + break; >> + case KSZ_LOW_LOSS_WA_2: >> + indir_reg = 0x4C; >> + indir_val = 0x40; > > Do the 3c and 4c registers have any associated documentation? Do we know > what they are or what they do? We should have some macros for them, > instead of magic numbers. > >> + break; >> + default: >> + break; > > Is it expected that in the default case (no workaround), the code flow > writes indir_val = 0x00 to indir_reg = 0x0000? Or would it be better to > just exit early without making any change? > >> + } >> + >> + mutex_lock(&dev->alu_mutex); >> + >> + ret = ksz_write8(dev, regs[REG_IND_CTRL_0], 0xA0); >> + >> + if (!ret) >> + ret = ksz_write8(dev, 0x6F, indir_reg); >> + >> + if (!ret) >> + ret = ksz_write8(dev, regs[REG_IND_BYTE], indir_val); > > Is this sequence better suited for ksz8_ind_write8()? Perhaps wrapped in > another layer similar to ksz8_pme_write8(), once we know what the magic > numbers represent in terms of indirect table? > >> + >> + mutex_unlock(&dev->alu_mutex); >> + >> + return ret; >> +} >> + >> static int ksz8_handle_global_errata(struct dsa_switch *ds) >> { >> struct ksz_device *dev = ds->priv; >> @@ -1915,6 +1950,17 @@ static int ksz8_handle_global_errata(struct dsa_switch *ds) >> if (dev->info->ksz87xx_eee_link_erratum) >> ret = ksz8_ind_write8(dev, TABLE_EEE, REG_IND_EEE_GLOB2_HI, 0); >> >> + /* KSZ87xx Errata DS80000687C. >> + * Module 3: Equalizer fix for short cables >> + * The receiver of the embedded PHYs is tuned by default >> + * to support long cable length applications. >> + * Because of this, the equalizer in the PHY may amplify >> + * high amplitude receiver signals to the point that >> + * the signal is distorted internally >> + */ >> + if (!ret && dev->low_loss_wa_enable && ksz_is_ksz87xx(dev)) >> + ret = ksz8_handle_module3_errata(dev); >> + >> return ret; >> } >> >> >> -- >> 2.53.0 >> > > FYI, the driver is in a restructuring process. The ksz88xx_switch_ops > will be split out of the common ksz_switch_ops. This will conflict with > your series, so you should rebase on top of that much larger set. > You can coordinate with Bastien Curutchet to see what is the status: > https://lore.kernel.org/netdev/20260313153849.qkfzv5c2u6fepjku@skbuf Hi Vladimir, Thanks for the review and for the detailed feedback! I’ll apply your suggestions in the next version of the patch. Thanks again for the guidance. Best regards, Fidelio