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 X-Spam-Level: X-Spam-Status: No, score=-7.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A3640C169C4 for ; Wed, 6 Feb 2019 21:57:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 565A6218B0 for ; Wed, 6 Feb 2019 21:57:15 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="uPKkx1Ya" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726480AbfBFV5O (ORCPT ); Wed, 6 Feb 2019 16:57:14 -0500 Received: from mail-wm1-f66.google.com ([209.85.128.66]:55579 "EHLO mail-wm1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726037AbfBFV5O (ORCPT ); Wed, 6 Feb 2019 16:57:14 -0500 Received: by mail-wm1-f66.google.com with SMTP id r17so342709wmh.5 for ; Wed, 06 Feb 2019 13:57:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=xhfvGomeHTA2jR/nSUZCfFevu1ppLiNSRxnqnD66mrw=; b=uPKkx1YaHtZYJ9ZEiaNYbRmA+TKW/+iv13g8lozth4+VIdaBDQWYsLsI10oBlOuTv5 XAcQFQQt+cS+LTQ3+3W6+FSI5K7mYYtM//OHSUZyiJ+xrVTxDTeaKtaD3g+NSiRlToye RK0AHy2+pbPVXTiIegBacIOd9Nj+EW/4ZTLLIE1w+r5W+RrkpzEuKlw2raCZF7RhhyVF meguBDr7iH2mcORyxmN5ZWzGWx2P6iXNP0DXr+75Rpqp9Yz3OH5G1X5lp1XvecalyiV8 ZxGXzT8FP+nPZXScd4eJ/2D1zH3eklKkPoyoRwg8ljihQNnqenT2nczodrz6lRHch2qh Qysw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=xhfvGomeHTA2jR/nSUZCfFevu1ppLiNSRxnqnD66mrw=; b=Yl6DNogilXXy2HLWJWTWLy82QIV7N1ECEcz/mn0LXgd1YCZixljgCylffiKjzCJMCz 4JGfZDrUXUX2H6coeEDZ52l6rzNuqEZwvsVakgrWdXKr5s719b7IWi0uN5ISY8ZZHZDk kYcRmYY30NcxlqAqOHrQv2xsi15PRlp/STPP6tWANalH8rWzhtd+hflOW3WayBv0TbzL VY8hcfWccrlh5VQJdrlQSozu0XVD9pFXinMh/aUe4xwwTjEqdpVlu7VGoa1rgLjGG84M hlvaLB7DSgt5lV0o9/EL/+m7GA1M6nvk6L7xTw/zVzD3Us42LqBlLZN3d5UUKbnPdSjg 8k9Q== X-Gm-Message-State: AHQUAubOgO1tO5YRIopIUeQ7DUJCN6T9vL8T8CwJlDDGheNkJlEKzh09 /Mf/LyRjFVxliZrL+4GH47g= X-Google-Smtp-Source: AHgI3IYDmIR5bjAdthOTYNYzvIv/vp/HfzI2eHHWU65mDYjYMhbjj3vUlANBL2prw/6lAfimb8Dw2A== X-Received: by 2002:a1c:2314:: with SMTP id j20mr4660581wmj.142.1549490231497; Wed, 06 Feb 2019 13:57:11 -0800 (PST) Received: from debian64.daheim (p4FD09AF4.dip0.t-ipconnect.de. [79.208.154.244]) by smtp.gmail.com with ESMTPSA id m21sm18691600wmi.43.2019.02.06.13.57.10 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 06 Feb 2019 13:57:10 -0800 (PST) Received: from localhost.daheim ([127.0.0.1] helo=debian64.localnet) by debian64.daheim with esmtp (Exim 4.92-RC5) (envelope-from ) id 1grVBs-0006qp-LX; Wed, 06 Feb 2019 22:57:08 +0100 From: Christian Lamparter To: Florian Fainelli Cc: Andrew Lunn , netdev@vger.kernel.org, Vivien Didelot Subject: Re: [PATCH v1] net: dsa: qca8k: implement DT-based ports <-> phy translation Date: Wed, 06 Feb 2019 22:57:07 +0100 Message-ID: <3897570.9hORRSCDvi@debian64> In-Reply-To: <699dfb8a-11d4-b27a-e762-ff4fdff1a3d7@gmail.com> References: <20190204213555.26054-1-chunkeey@gmail.com> <2061891.QOleSDBFsG@debian64> <699dfb8a-11d4-b27a-e762-ff4fdff1a3d7@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Tuesday, February 5, 2019 11:29:36 PM CET Florian Fainelli wrote: > On 2/5/19 2:12 PM, Christian Lamparter wrote: > > On Tuesday, February 5, 2019 10:29:34 PM CET Andrew Lunn wrote: > >>> For now, I added the DT binding update to the patch as well. > >>> But if this is indeed the way to go, it'll get a separate patch. > >> > >> Hi Christian > >> > >> You need to be careful with the DT binding. You need to keep backwards > >> compatible with it. An old DT blob needs to keep working. I don't > >> think this is true with this change. > > > > Do you mean because of the > > > > - switch0@0 { > > + switch@10 { > > compatible = "qca,qca8337"; > > #address-cells = <1>; > > #size-cells = <0>; > > > > - reg = <0>; > > + reg = <0x10>; > > > > change? > > > > or because I removed the phy-handles?> > > The reg = <0x10>; will be necessary regardless. Because this > > is really a bug in the existing binding example and if it is > > copied it will prevent the qca8k driver from loading. > > This is due to a resource conflict, because there will be > > already a "phy_port1: phy@0" registered at reg = <0>; > > So this never worked would have worked. > > That part is fine, it is the removal of the phy-handle properties that > is possibly a problem, but in hindsight, I do not believe it will be a > compatibility issue. Lack of "phy-handle" property within the core DSA > layer means: utilize the switch's internal MDIO bus (ds->slave_mii_bus) > instance, which you are not removing, you are just changing how the PHYs > map to port numbers. > Ok, thanks. I think I'm almost ready for v2. I have fully addressed the compatibility issue by forking off the qca8k_switch_ops depending on whenever a phy-handle property on one of the ports was found or not. If there was no phy-handle the driver adds the slave-bus accessors to the ops which tells DSA to allocate the slave bus and allows the phys can be enumerated. If the phy-handles are found the driver will not have the accessors and DSA will not setup a redundant/fake bus and this prevents the second/double/duplicated discovery and enumeration of the same PHYs again. Cheers, Christian Attached is a preview: --- commit 96bc70b4d6e290c450b9af728d9ca0f6db877f13 Author: Christian Lamparter Date: Fri Feb 1 22:54:32 2019 +0100 net: dsa: qca8k: extend slave-bus implementations This patch implements accessors for the QCA8337 MDIO access through the MDIO_MASTER register, which makes it possible to access the PHYs on slave-bus through the switch. In cases where the switch ports are already mapped via external "phy-phandles", the internal mdio-bus is disabled in order to prevent a duplicated discovery and enumeration of the same PHYs. Signed-off-by: Christian Lamparter --- Changes from v2: - Make it compatible with existing configurations Changes from v1: - drop DT port <-> phy mapping - added register definitions for the MDIO control register - implemented new slave-mdio bus accessors - DT-binding: fix switch's PSEUDO_PHY address. It's 0x10 not 0. diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c index a4b6cda38016..2f1b4b0a3507 100644 --- a/drivers/net/dsa/qca8k.c +++ b/drivers/net/dsa/qca8k.c @@ -613,19 +613,61 @@ qca8k_adjust_link(struct dsa_switch *ds, int port, struct phy_device *phy) } static int -qca8k_phy_read(struct dsa_switch *ds, int phy, int regnum) +qca8k_port_to_phy(int port) +{ + if (port < 1 || port > QCA8K_MDIO_MASTER_MAX_PORTS) + return -EINVAL; + + return port - 1; +} + +static int +qca8k_phy_write(struct dsa_switch *ds, int port, int regnum, u16 data) { struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv; + u32 val, phy; + + phy = qca8k_port_to_phy(port); + if (phy < 0 || (regnum < 0 || regnum >= QCA8K_MDIO_MASTER_MAX_REG)) + return -EINVAL; + + val = QCA8K_MDIO_MASTER_BUSY | QCA8K_MDIO_MASTER_EN | + QCA8K_MDIO_MASTER_WRITE | QCA8K_MDIO_MASTER_PHY_ADDR(phy) | + QCA8K_MDIO_MASTER_REG_ADDR(regnum) | + QCA8K_MDIO_MASTER_DATA(data); - return mdiobus_read(priv->bus, phy, regnum); + qca8k_write(priv, QCA8K_MDIO_MASTER_CTRL, val); + + return qca8k_busy_wait(priv, QCA8K_MDIO_MASTER_CTRL, + QCA8K_MDIO_MASTER_BUSY); } + static int -qca8k_phy_write(struct dsa_switch *ds, int phy, int regnum, u16 val) +qca8k_phy_read(struct dsa_switch *ds, int port, int regnum) { struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv; + u32 val, phy; + + phy = qca8k_port_to_phy(port); + if (phy < 0 || (regnum < 0 || regnum >= QCA8K_MDIO_MASTER_MAX_REG)) + return 0xffff; + + val = QCA8K_MDIO_MASTER_BUSY | QCA8K_MDIO_MASTER_EN | + QCA8K_MDIO_MASTER_READ | QCA8K_MDIO_MASTER_PHY_ADDR(phy) | + QCA8K_MDIO_MASTER_REG_ADDR(regnum); + + qca8k_write(priv, QCA8K_MDIO_MASTER_CTRL, val); - return mdiobus_write(priv->bus, phy, regnum, val); + if (qca8k_busy_wait(priv, QCA8K_MDIO_MASTER_CTRL, + QCA8K_MDIO_MASTER_BUSY)) { + return 0xffff; + } + + val = (qca8k_read(priv, QCA8K_MDIO_MASTER_CTRL) & + QCA8K_MDIO_MASTER_DATA_MASK); + + return val; } static void @@ -868,8 +910,6 @@ static const struct dsa_switch_ops qca8k_switch_ops = { .setup = qca8k_setup, .adjust_link = qca8k_adjust_link, .get_strings = qca8k_get_strings, - .phy_read = qca8k_phy_read, - .phy_write = qca8k_phy_write, .get_ethtool_stats = qca8k_get_ethtool_stats, .get_sset_count = qca8k_get_sset_count, .get_mac_eee = qca8k_get_mac_eee, @@ -884,6 +924,38 @@ static const struct dsa_switch_ops qca8k_switch_ops = { .port_fdb_dump = qca8k_port_fdb_dump, }; +/* Special code to detect DeviceTrees that use the phy-handle + * to map external PHYs to the ports. Only if no phy-handle is + * detected the slave bus accessors are getting enabled. + */ +static int qca8k_detect_slave_bus(struct qca8k_priv *priv) +{ + struct device_node *ports, *port; + bool found = false; + + ports = of_get_child_by_name(priv->dev->of_node, "ports"); + if (!ports) { + dev_err(priv->dev, "no ports child node found.\n"); + return -EINVAL; + } + + for_each_available_child_of_node(ports, port) { + if (of_property_read_bool(port, "phy-handle")) { + found = true; + break; + } + } + + if (found) { + dev_info(priv->dev, "uses external mdio-bus.\n"); + } else { + priv->ops.phy_read = qca8k_phy_read; + priv->ops.phy_write = qca8k_phy_write; + } + + return 0; +} + static int qca8k_sw_probe(struct mdio_device *mdiodev) { @@ -912,7 +984,12 @@ qca8k_sw_probe(struct mdio_device *mdiodev) return -ENOMEM; priv->ds->priv = priv; - priv->ds->ops = &qca8k_switch_ops; + priv->ops = qca8k_switch_ops; + if (qca8k_detect_slave_bus(priv)) + return -EINVAL; + + priv->ds->ops = &priv->ops; + mutex_init(&priv->reg_mutex); dev_set_drvdata(&mdiodev->dev, priv); diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h index 613fe5c50236..38d06661f0a8 100644 --- a/drivers/net/dsa/qca8k.h +++ b/drivers/net/dsa/qca8k.h @@ -48,6 +48,18 @@ #define QCA8K_MIB_FLUSH BIT(24) #define QCA8K_MIB_CPU_KEEP BIT(20) #define QCA8K_MIB_BUSY BIT(17) +#define QCA8K_MDIO_MASTER_CTRL 0x3c +#define QCA8K_MDIO_MASTER_BUSY BIT(31) +#define QCA8K_MDIO_MASTER_EN BIT(30) +#define QCA8K_MDIO_MASTER_READ BIT(27) +#define QCA8K_MDIO_MASTER_WRITE 0 +#define QCA8K_MDIO_MASTER_SUP_PRE BIT(26) +#define QCA8K_MDIO_MASTER_PHY_ADDR(x) ((x) << 21) +#define QCA8K_MDIO_MASTER_REG_ADDR(x) ((x) << 16) +#define QCA8K_MDIO_MASTER_DATA(x) (x) +#define QCA8K_MDIO_MASTER_DATA_MASK GENMASK(15, 0) +#define QCA8K_MDIO_MASTER_MAX_PORTS 5 +#define QCA8K_MDIO_MASTER_MAX_REG 32 #define QCA8K_GOL_MAC_ADDR0 0x60 #define QCA8K_GOL_MAC_ADDR1 0x64 #define QCA8K_REG_PORT_STATUS(_i) (0x07c + (_i) * 4) @@ -168,6 +180,7 @@ struct qca8k_priv { struct dsa_switch *ds; struct mutex reg_mutex; struct device *dev; + struct dsa_switch_ops ops; }; struct qca8k_mib_desc {