From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 590BCC433F5 for ; Sun, 9 Oct 2022 05:40:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:References:Cc:To:Subject:From: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Wo/isJZoTv+u3rbDHczM/7DTqU2M+1rNClVrIUOlHjo=; b=QkrXsDEvFnlvGG oN615YNGrLpetywjbGWMzajTfHiScNU+T13nEfEpM4j5nBVJ1pyeVYVBhJ+smd3OKrRjOnP7KySUP il71r3tBYvnYcicoGCNXVtu/TVlDlfDybtmXB+bIIaGvX3wVkkeaNr7tAG2h7DGzdjnPb0Fudb2IT DRKKXIziEKFUrhMrA85e4i6VQWycSAMdGyI85X2og+8u1PSMjCVf6mMrkrM4vZ4KZUtN0AtBL+LNR YOM4j6/JvXfBmzF8jyaQAVhe8zH/QGhLNzSORzpVqkK01Oe/lR8B/GcsIAXjeIiwNdcO2VDL2QPve zK9cQ2lr9ycyPCuj0FDA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ohP16-00Ew8h-RC; Sun, 09 Oct 2022 05:38:24 +0000 Received: from mail-qk1-x72d.google.com ([2607:f8b0:4864:20::72d]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1ohP14-00Ew7z-17 for linux-arm-kernel@lists.infradead.org; Sun, 09 Oct 2022 05:38:23 +0000 Received: by mail-qk1-x72d.google.com with SMTP id x25so1492310qki.2 for ; Sat, 08 Oct 2022 22:38:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:subject:from:user-agent:mime-version:date:message-id:from:to :cc:subject:date:message-id:reply-to; bh=10dt+raSrmwS6ADFu9fL6c/AYbOE0LPtaElD4X5UPAw=; b=onFuG7sTY3TRXA3CuIPpd/jcoWTdZDvZUIf1h1d8R6qx2+HR9H8i02Ik38pslqJqWw zPEbBEakv4txOd7hbkEym1Gyddh2Uu6mjWvm9eHEUYI8qz7/dfU8Oag/4HddPtsyHVzZ mO5g5V1veXnWL8eW+Zt4J4Zz9id+oPqitsgO4S5ffR9GfFw1dtBndQzpb6V3O5x6CF3T 5aIJNeU7XgjMrjG3SLjY5qU5I5ywSob9iO9L1n4G91whQUw04v8Rljf8U0YiH/YpnBR9 idhEhWK6WHLDb6iREzPR7MHPWo5CTV4j2qN+wHqaIZCZRE/tjkWB0kWsYzdHrDRfpwPr xFqg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:subject:from:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=10dt+raSrmwS6ADFu9fL6c/AYbOE0LPtaElD4X5UPAw=; b=NGsyz2V+tenALXy8MRFhrUqKIsOpAHVLfUgg2X9+f8oidTBEL53PTCt3YOHlyVR386 agNoqcvxZ9JP6Z1lOBM+We9qblkrtPLn0fzhkPiV0ampXBrYE9LJliLIgsAjhbThxIfo Ay3Cf7xNIk+TSXK23zlLABOwc9HT8i4n6D87jHQl3zo99gyjxpoGtDkP/ZctVeDbnzyT kp2ZV9aYSUkLTBGZ3ljigCTjd7lqBInSs/TS7zz5UXuuFSbsmAoTcaqSwG3oR3WthGnu SNXB2KaBoYU+X5Z+DXyYmebGfR7gITu9b23AoaciEYgE25zpPzza8twwIMS/U18DzIX2 JtyA== X-Gm-Message-State: ACrzQf0mWoJ47v8GkXUDXxnYubTTYU4l4vaEshz8W3ERRIB7kGyCfsIK TVDdOFJZnHdGI58Lef+VMUM= X-Google-Smtp-Source: AMsMyM7ygoGdAL64EXO5vMPoVdxGXWhnQZMB70k5pfE3ABkAPlE7jIgbrexbujK/UT0voebLoxGUpQ== X-Received: by 2002:a05:620a:671:b0:6ec:597a:cdf6 with SMTP id a17-20020a05620a067100b006ec597acdf6mr560648qkh.527.1665293896748; Sat, 08 Oct 2022 22:38:16 -0700 (PDT) Received: from [192.168.1.201] (pool-173-73-95-180.washdc.fios.verizon.net. [173.73.95.180]) by smtp.gmail.com with ESMTPSA id j19-20020a05620a289300b006bb87c4833asm6864623qkp.109.2022.10.08.22.38.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 08 Oct 2022 22:38:16 -0700 (PDT) Message-ID: <68b3dfbf-9bab-2554-254e-bffd280ba97e@gmail.com> Date: Sun, 9 Oct 2022 01:38:15 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 From: Sean Anderson Subject: Re: [PATCH net-next v3 3/5] net: pcs: add new PCS driver for altera TSE PCS To: Maxime Chevallier , davem@davemloft.net, Rob Herring Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, thomas.petazzoni@bootlin.com, Andrew Lunn , Jakub Kicinski , Eric Dumazet , Paolo Abeni , Florian Fainelli , Heiner Kallweit , Russell King , linux-arm-kernel@lists.infradead.org, Krzysztof Kozlowski , devicetree@vger.kernel.org References: <20220901143543.416977-1-maxime.chevallier@bootlin.com> <20220901143543.416977-4-maxime.chevallier@bootlin.com> Content-Language: en-US In-Reply-To: <20220901143543.416977-4-maxime.chevallier@bootlin.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221008_223822_113642_3D2E371C X-CRM114-Status: GOOD ( 39.35 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 9/1/22 10:35, Maxime Chevallier wrote: > The Altera Triple Speed Ethernet has a SGMII/1000BaseC PCS that can be > integrated in several ways. It can either be part of the TSE MAC's > address space, accessed through 32 bits accesses on the mapped mdio > device 0, or through a dedicated 16 bits register set. > > This driver allows using the TSE PCS outside of altera TSE's driver, > since it can be used standalone by other MACs. > > Signed-off-by: Maxime Chevallier > --- > V2->V3 : > - No changes > V1->V2 : > - Added a pcs_validate() callback to filter interface modes > - Added comments on the need for a soft reset at an_restart > > MAINTAINERS | 7 ++ > drivers/net/pcs/Kconfig | 6 ++ > drivers/net/pcs/Makefile | 1 + > drivers/net/pcs/pcs-altera-tse.c | 171 +++++++++++++++++++++++++++++++ > include/linux/pcs-altera-tse.h | 17 +++ > 5 files changed, 202 insertions(+) > create mode 100644 drivers/net/pcs/pcs-altera-tse.c > create mode 100644 include/linux/pcs-altera-tse.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index fe484e7d36dc..576c01576a5f 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -878,6 +878,13 @@ L: netdev@vger.kernel.org > S: Maintained > F: drivers/net/ethernet/altera/ > > +ALTERA TSE PCS > +M: Maxime Chevallier > +L: netdev@vger.kernel.org > +S: Supported > +F: drivers/net/pcs/pcs-altera-tse.c > +F: include/linux/pcs-altera-tse.h > + > ALTERA UART/JTAG UART SERIAL DRIVERS > M: Tobias Klauser > L: linux-serial@vger.kernel.org > diff --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig > index 6289b7c765f1..6e7e6c346a3e 100644 > --- a/drivers/net/pcs/Kconfig > +++ b/drivers/net/pcs/Kconfig > @@ -26,4 +26,10 @@ config PCS_RZN1_MIIC > on RZ/N1 SoCs. This PCS converts MII to RMII/RGMII or can be set in > pass-through mode for MII. > > +config PCS_ALTERA_TSE > + tristate > + help > + This module provides helper functions for the Altera Triple Speed > + Ethernet SGMII PCS, that can be found on the Intel Socfpga family. > + > endmenu > diff --git a/drivers/net/pcs/Makefile b/drivers/net/pcs/Makefile > index 0ff5388fcdea..4c780d8f2e98 100644 > --- a/drivers/net/pcs/Makefile > +++ b/drivers/net/pcs/Makefile > @@ -6,3 +6,4 @@ pcs_xpcs-$(CONFIG_PCS_XPCS) := pcs-xpcs.o pcs-xpcs-nxp.o > obj-$(CONFIG_PCS_XPCS) += pcs_xpcs.o > obj-$(CONFIG_PCS_LYNX) += pcs-lynx.o > obj-$(CONFIG_PCS_RZN1_MIIC) += pcs-rzn1-miic.o > +obj-$(CONFIG_PCS_ALTERA_TSE) += pcs-altera-tse.o > diff --git a/drivers/net/pcs/pcs-altera-tse.c b/drivers/net/pcs/pcs-altera-tse.c > new file mode 100644 > index 000000000000..ae7688ffc4d7 > --- /dev/null > +++ b/drivers/net/pcs/pcs-altera-tse.c > @@ -0,0 +1,171 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2022 Bootlin > + * > + * Maxime Chevallier > + */ > + > +#include > +#include > +#include > +#include > + > +/* SGMII PCS register addresses > + */ > +#define SGMII_PCS_SCRATCH 0x10 > +#define SGMII_PCS_REV 0x11 > +#define SGMII_PCS_LINK_TIMER_0 0x12 > +#define SGMII_PCS_LINK_TIMER_REG(x) (0x12 + (x)) Not used. > +#define SGMII_PCS_LINK_TIMER_1 0x13 > +#define SGMII_PCS_IF_MODE 0x14 > +#define PCS_IF_MODE_SGMII_ENA BIT(0) > +#define PCS_IF_MODE_USE_SGMII_AN BIT(1) > +#define PCS_IF_MODE_SGMI_SPEED_MASK GENMASK(3, 2) > +#define PCS_IF_MODE_SGMI_SPEED_10 (0 << 2) > +#define PCS_IF_MODE_SGMI_SPEED_100 (1 << 2) > +#define PCS_IF_MODE_SGMI_SPEED_1000 (2 << 2) You can use FIELD_PREP if you're so inclined. I assume SGMI is from the datasheet. > +#define PCS_IF_MODE_SGMI_HALF_DUPLEX BIT(4) > +#define PCS_IF_MODE_SGMI_PHY_AN BIT(5) > +#define SGMII_PCS_DIS_READ_TO 0x15 > +#define SGMII_PCS_READ_TO 0x16 > +#define SGMII_PCS_SW_RESET_TIMEOUT 100 /* usecs */ > + > +struct altera_tse_pcs { > + struct phylink_pcs pcs; > + void __iomem *base; > + int reg_width; > +}; > + > +static struct altera_tse_pcs *phylink_pcs_to_tse_pcs(struct phylink_pcs *pcs) > +{ > + return container_of(pcs, struct altera_tse_pcs, pcs); > +} > + > +static u16 tse_pcs_read(struct altera_tse_pcs *tse_pcs, int regnum) > +{ > + if (tse_pcs->reg_width == 4) > + return readl(tse_pcs->base + regnum * 4); > + else > + return readw(tse_pcs->base + regnum * 2); > +} > + > +static void tse_pcs_write(struct altera_tse_pcs *tse_pcs, int regnum, > + u16 value) > +{ > + if (tse_pcs->reg_width == 4) > + writel(value, tse_pcs->base + regnum * 4); > + else > + writew(value, tse_pcs->base + regnum * 2); > +} > + > +static int tse_pcs_reset(struct altera_tse_pcs *tse_pcs) > +{ > + int i = 0; > + u16 bmcr; > + > + /* Reset PCS block */ > + bmcr = tse_pcs_read(tse_pcs, MII_BMCR); > + bmcr |= BMCR_RESET; > + tse_pcs_write(tse_pcs, MII_BMCR, bmcr); > + > + for (i = 0; i < SGMII_PCS_SW_RESET_TIMEOUT; i++) { > + if (!(tse_pcs_read(tse_pcs, MII_BMCR) & BMCR_RESET)) > + return 0; > + udelay(1); > + } read_poll_timeout? > + > + return -ETIMEDOUT; > +} > + > +static int alt_tse_pcs_validate(struct phylink_pcs *pcs, > + unsigned long *supported, > + const struct phylink_link_state *state) > +{ > + if (state->interface == PHY_INTERFACE_MODE_SGMII || > + state->interface == PHY_INTERFACE_MODE_1000BASEX) > + return 1; > + > + return -EINVAL; > +} > + > +static int alt_tse_pcs_config(struct phylink_pcs *pcs, unsigned int mode, > + phy_interface_t interface, > + const unsigned long *advertising, > + bool permit_pause_to_mac) > +{ > + struct altera_tse_pcs *tse_pcs = phylink_pcs_to_tse_pcs(pcs); > + u32 ctrl, if_mode; > + > + ctrl = tse_pcs_read(tse_pcs, MII_BMCR); > + if_mode = tse_pcs_read(tse_pcs, SGMII_PCS_IF_MODE); > + > + /* Set link timer to 1.6ms, as per the MegaCore Function User Guide */ > + tse_pcs_write(tse_pcs, SGMII_PCS_LINK_TIMER_0, 0x0D40); > + tse_pcs_write(tse_pcs, SGMII_PCS_LINK_TIMER_1, 0x03); Shouldn't this be different for SGMII vs 1000BASE-X? > + > + if (interface == PHY_INTERFACE_MODE_SGMII) { > + if_mode |= PCS_IF_MODE_USE_SGMII_AN | PCS_IF_MODE_SGMII_ENA; I think PCS_IF_MODE_USE_SGMII_AN should be cleared if mode=MLO_AN_FIXED. > + } else if (interface == PHY_INTERFACE_MODE_1000BASEX) { > + if_mode &= ~(PCS_IF_MODE_USE_SGMII_AN | PCS_IF_MODE_SGMII_ENA); > + if_mode |= PCS_IF_MODE_SGMI_SPEED_1000; I don't think you need to set this for 1000BASE-X. > + } > + > + ctrl |= (BMCR_SPEED1000 | BMCR_FULLDPLX | BMCR_ANENABLE); BMCR_FULLDPLX is read-only, so you don't have to set it. Same for the speed. > + > + tse_pcs_write(tse_pcs, MII_BMCR, ctrl); > + tse_pcs_write(tse_pcs, SGMII_PCS_IF_MODE, if_mode); > + > + return tse_pcs_reset(tse_pcs); > +} > + > +static void alt_tse_pcs_get_state(struct phylink_pcs *pcs, > + struct phylink_link_state *state) > +{ > + struct altera_tse_pcs *tse_pcs = phylink_pcs_to_tse_pcs(pcs); > + u16 bmsr, lpa; > + > + bmsr = tse_pcs_read(tse_pcs, MII_BMSR); > + lpa = tse_pcs_read(tse_pcs, MII_LPA); > + > + phylink_mii_c22_pcs_decode_state(state, bmsr, lpa); > +} > + > +static void alt_tse_pcs_an_restart(struct phylink_pcs *pcs) > +{ > + struct altera_tse_pcs *tse_pcs = phylink_pcs_to_tse_pcs(pcs); > + u16 bmcr; > + > + bmcr = tse_pcs_read(tse_pcs, MII_BMCR); > + bmcr |= BMCR_ANRESTART; > + tse_pcs_write(tse_pcs, MII_BMCR, bmcr); > + > + /* This PCS seems to require a soft reset to re-sync the AN logic */ > + tse_pcs_reset(tse_pcs); This is kinda strange since c22 phys are supposed to reset the other registers to default values when BMCR_RESET is written. Good thing this is a PCS... > +} > + > +static const struct phylink_pcs_ops alt_tse_pcs_ops = { > + .pcs_validate = alt_tse_pcs_validate, > + .pcs_get_state = alt_tse_pcs_get_state, > + .pcs_config = alt_tse_pcs_config, > + .pcs_an_restart = alt_tse_pcs_an_restart, > +}; Don't you need link_up to set the speed/duplex for MLO_AN_FIXED? > + > +struct phylink_pcs *alt_tse_pcs_create(struct net_device *ndev, > + void __iomem *pcs_base, int reg_width) > +{ > + struct altera_tse_pcs *tse_pcs; > + > + if (reg_width != 4 && reg_width != 2) > + return ERR_PTR(-EINVAL); > + > + tse_pcs = devm_kzalloc(&ndev->dev, sizeof(*tse_pcs), GFP_KERNEL); > + if (!tse_pcs) > + return ERR_PTR(-ENOMEM); > + > + tse_pcs->pcs.ops = &alt_tse_pcs_ops; > + tse_pcs->base = pcs_base; > + tse_pcs->reg_width = reg_width; > + > + return &tse_pcs->pcs; > +} > +EXPORT_SYMBOL_GPL(alt_tse_pcs_create); > diff --git a/include/linux/pcs-altera-tse.h b/include/linux/pcs-altera-tse.h > new file mode 100644 > index 000000000000..92ab9f08e835 > --- /dev/null > +++ b/include/linux/pcs-altera-tse.h > @@ -0,0 +1,17 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2022 Bootlin > + * > + * Maxime Chevallier > + */ > + > +#ifndef __LINUX_PCS_ALTERA_TSE_H > +#define __LINUX_PCS_ALTERA_TSE_H > + > +struct phylink_pcs; > +struct net_device; > + > +struct phylink_pcs *alt_tse_pcs_create(struct net_device *ndev, > + void __iomem *pcs_base, int reg_width); > + > +#endif /* __LINUX_PCS_ALTERA_TSE_H */ --Sean _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel