From mboxrd@z Thu Jan 1 00:00:00 1970 From: miquel.raynal@bootlin.com (Miquel Raynal) Date: Fri, 23 Nov 2018 09:30:38 +0100 Subject: [PATCH 2/6] phy: add A3700 COMPHY support In-Reply-To: <20181122210420.14861-3-miquel.raynal@bootlin.com> References: <20181122210420.14861-1-miquel.raynal@bootlin.com> <20181122210420.14861-3-miquel.raynal@bootlin.com> Message-ID: <20181123093038.20278133@xps13> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hello, + Adding people concerned by this driver that I forgot when initially sending the driver, will update the Cc: list if there is a v2. One question below. Miquel Raynal wrote on Thu, 22 Nov 2018 22:04:16 +0100: > Add a driver to support COMPHY, a hardware block providing shared > serdes PHYs on Marvell Armada 3700. This driver uses SMC calls and > rely on having an up-to-date firmware. > > SATA, PCie and USB3 host mode have been tested successfully with an > ESPRESSObin. SGMII mode cannot be tested with this platform. > > Co-Developed-by: Evan Wang > Signed-off-by: Evan Wang > Signed-off-by: Miquel Raynal > --- > drivers/phy/marvell/Kconfig | 10 + > drivers/phy/marvell/Makefile | 1 + > drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 276 +++++++++++++++++++ > 3 files changed, 287 insertions(+) > create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-comphy.c [...] > + > +static int mvebu_a3700_comphy_power_on(struct phy *phy) > +{ > + struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy); > + u32 fw_param; > + int fw_mode; > + > + fw_mode = mvebu_a3700_comphy_get_fw_mode(lane->id, lane->port, > + lane->mode); > + if (fw_mode < 0) { > + dev_err(lane->dev, "invalid COMPHY mode\n"); > + return fw_mode; > + } > + > + switch (lane->mode) { > + case PHY_MODE_USB_HOST_SS: > + dev_dbg(lane->dev, "set lane %d to USB3 host mode\n", lane->id); > + fw_param = COMPHY_FW_MODE(fw_mode); > + break; > + case PHY_MODE_SATA: > + dev_dbg(lane->dev, "set lane %d to SATA mode\n", lane->id); > + fw_param = COMPHY_FW_MODE(fw_mode); > + break; > + case PHY_MODE_SGMII: > + dev_dbg(lane->dev, "set lane %d to SGMII mode\n", lane->id); > + fw_param = COMPHY_FW_NET(fw_mode, lane->port, > + COMPHY_FW_SPEED_1_25G); > + break; > + case PHY_MODE_PCIE: > + dev_dbg(lane->dev, "set lane %d to PCIe mode\n", lane->id); > + fw_param = COMPHY_FW_PCIE(fw_mode, lane->port, > + COMPHY_FW_SPEED_5G, > + phy->attrs.bus_width); > + break; > + default: > + dev_err(lane->dev, "unsupported PHY mode (%d)\n", lane->mode); > + return -ENOTSUPP; > + } > + > + return mvebu_a3700_comphy_smc(COMPHY_SIP_POWER_ON, lane->id, fw_param); This call might fail because the firmware is not up-to-date. In this case, I wonder what is the appropriate behavior. Here I just return the error; this means drivers may fail to probe when enabling their PHY. Shall I ignore a "not supported by the firmware" code and return 0 to the caller (with a printed warning) so it will continue probing? Doing so is lying as the PHY might not be configured as expected and S2RAM will always fail in this case. Thanks, Miqu?l From mboxrd@z Thu Jan 1 00:00:00 1970 From: Miquel Raynal Subject: Re: [PATCH 2/6] phy: add A3700 COMPHY support Date: Fri, 23 Nov 2018 09:30:38 +0100 Message-ID: <20181123093038.20278133@xps13> References: <20181122210420.14861-1-miquel.raynal@bootlin.com> <20181122210420.14861-3-miquel.raynal@bootlin.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Return-path: In-Reply-To: <20181122210420.14861-3-miquel.raynal@bootlin.com> Sender: linux-kernel-owner@vger.kernel.org To: Gregory Clement , Jason Cooper , Andrew Lunn , Sebastian Hesselbarth , Kishon Vijay Abraham I Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Rob Herring , Mark Rutland , Evan Wang , "Nadav Haklai (Marvell)" , Thomas Petazzoni , Antoine Tenart , Maxime Chevallier List-Id: devicetree@vger.kernel.org Hello, + Adding people concerned by this driver that I forgot when initially sending the driver, will update the Cc: list if there is a v2. One question below. Miquel Raynal wrote on Thu, 22 Nov 2018 22:04:16 +0100: > Add a driver to support COMPHY, a hardware block providing shared > serdes PHYs on Marvell Armada 3700. This driver uses SMC calls and > rely on having an up-to-date firmware. > > SATA, PCie and USB3 host mode have been tested successfully with an > ESPRESSObin. SGMII mode cannot be tested with this platform. > > Co-Developed-by: Evan Wang > Signed-off-by: Evan Wang > Signed-off-by: Miquel Raynal > --- > drivers/phy/marvell/Kconfig | 10 + > drivers/phy/marvell/Makefile | 1 + > drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 276 +++++++++++++++++++ > 3 files changed, 287 insertions(+) > create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-comphy.c [...] > + > +static int mvebu_a3700_comphy_power_on(struct phy *phy) > +{ > + struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy); > + u32 fw_param; > + int fw_mode; > + > + fw_mode = mvebu_a3700_comphy_get_fw_mode(lane->id, lane->port, > + lane->mode); > + if (fw_mode < 0) { > + dev_err(lane->dev, "invalid COMPHY mode\n"); > + return fw_mode; > + } > + > + switch (lane->mode) { > + case PHY_MODE_USB_HOST_SS: > + dev_dbg(lane->dev, "set lane %d to USB3 host mode\n", lane->id); > + fw_param = COMPHY_FW_MODE(fw_mode); > + break; > + case PHY_MODE_SATA: > + dev_dbg(lane->dev, "set lane %d to SATA mode\n", lane->id); > + fw_param = COMPHY_FW_MODE(fw_mode); > + break; > + case PHY_MODE_SGMII: > + dev_dbg(lane->dev, "set lane %d to SGMII mode\n", lane->id); > + fw_param = COMPHY_FW_NET(fw_mode, lane->port, > + COMPHY_FW_SPEED_1_25G); > + break; > + case PHY_MODE_PCIE: > + dev_dbg(lane->dev, "set lane %d to PCIe mode\n", lane->id); > + fw_param = COMPHY_FW_PCIE(fw_mode, lane->port, > + COMPHY_FW_SPEED_5G, > + phy->attrs.bus_width); > + break; > + default: > + dev_err(lane->dev, "unsupported PHY mode (%d)\n", lane->mode); > + return -ENOTSUPP; > + } > + > + return mvebu_a3700_comphy_smc(COMPHY_SIP_POWER_ON, lane->id, fw_param); This call might fail because the firmware is not up-to-date. In this case, I wonder what is the appropriate behavior. Here I just return the error; this means drivers may fail to probe when enabling their PHY. Shall I ignore a "not supported by the firmware" code and return 0 to the caller (with a printed warning) so it will continue probing? Doing so is lying as the PHY might not be configured as expected and S2RAM will always fail in this case. Thanks, Miquèl 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=-4.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED 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 E80D6C43441 for ; Fri, 23 Nov 2018 08:30:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B7AC120663 for ; Fri, 23 Nov 2018 08:30:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B7AC120663 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=bootlin.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2502282AbeKWTOK convert rfc822-to-8bit (ORCPT ); Fri, 23 Nov 2018 14:14:10 -0500 Received: from mail.bootlin.com ([62.4.15.54]:48125 "EHLO mail.bootlin.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2393613AbeKWTOK (ORCPT ); Fri, 23 Nov 2018 14:14:10 -0500 Received: by mail.bootlin.com (Postfix, from userid 110) id D004A20726; Fri, 23 Nov 2018 09:30:54 +0100 (CET) Received: from xps13 (aaubervilliers-681-1-94-205.w90-88.abo.wanadoo.fr [90.88.35.205]) by mail.bootlin.com (Postfix) with ESMTPSA id 65EE920DC4; Fri, 23 Nov 2018 09:30:38 +0100 (CET) Date: Fri, 23 Nov 2018 09:30:38 +0100 From: Miquel Raynal To: Gregory Clement , Jason Cooper , Andrew Lunn , Sebastian Hesselbarth , Kishon Vijay Abraham I Cc: , , linux-kernel@vger.kernel.org, Rob Herring , Mark Rutland , Evan Wang , "Nadav Haklai (Marvell)" , Thomas Petazzoni , Antoine Tenart , Maxime Chevallier Subject: Re: [PATCH 2/6] phy: add A3700 COMPHY support Message-ID: <20181123093038.20278133@xps13> In-Reply-To: <20181122210420.14861-3-miquel.raynal@bootlin.com> References: <20181122210420.14861-1-miquel.raynal@bootlin.com> <20181122210420.14861-3-miquel.raynal@bootlin.com> Organization: Bootlin X-Mailer: Claws Mail 3.17.1 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, + Adding people concerned by this driver that I forgot when initially sending the driver, will update the Cc: list if there is a v2. One question below. Miquel Raynal wrote on Thu, 22 Nov 2018 22:04:16 +0100: > Add a driver to support COMPHY, a hardware block providing shared > serdes PHYs on Marvell Armada 3700. This driver uses SMC calls and > rely on having an up-to-date firmware. > > SATA, PCie and USB3 host mode have been tested successfully with an > ESPRESSObin. SGMII mode cannot be tested with this platform. > > Co-Developed-by: Evan Wang > Signed-off-by: Evan Wang > Signed-off-by: Miquel Raynal > --- > drivers/phy/marvell/Kconfig | 10 + > drivers/phy/marvell/Makefile | 1 + > drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 276 +++++++++++++++++++ > 3 files changed, 287 insertions(+) > create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-comphy.c [...] > + > +static int mvebu_a3700_comphy_power_on(struct phy *phy) > +{ > + struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy); > + u32 fw_param; > + int fw_mode; > + > + fw_mode = mvebu_a3700_comphy_get_fw_mode(lane->id, lane->port, > + lane->mode); > + if (fw_mode < 0) { > + dev_err(lane->dev, "invalid COMPHY mode\n"); > + return fw_mode; > + } > + > + switch (lane->mode) { > + case PHY_MODE_USB_HOST_SS: > + dev_dbg(lane->dev, "set lane %d to USB3 host mode\n", lane->id); > + fw_param = COMPHY_FW_MODE(fw_mode); > + break; > + case PHY_MODE_SATA: > + dev_dbg(lane->dev, "set lane %d to SATA mode\n", lane->id); > + fw_param = COMPHY_FW_MODE(fw_mode); > + break; > + case PHY_MODE_SGMII: > + dev_dbg(lane->dev, "set lane %d to SGMII mode\n", lane->id); > + fw_param = COMPHY_FW_NET(fw_mode, lane->port, > + COMPHY_FW_SPEED_1_25G); > + break; > + case PHY_MODE_PCIE: > + dev_dbg(lane->dev, "set lane %d to PCIe mode\n", lane->id); > + fw_param = COMPHY_FW_PCIE(fw_mode, lane->port, > + COMPHY_FW_SPEED_5G, > + phy->attrs.bus_width); > + break; > + default: > + dev_err(lane->dev, "unsupported PHY mode (%d)\n", lane->mode); > + return -ENOTSUPP; > + } > + > + return mvebu_a3700_comphy_smc(COMPHY_SIP_POWER_ON, lane->id, fw_param); This call might fail because the firmware is not up-to-date. In this case, I wonder what is the appropriate behavior. Here I just return the error; this means drivers may fail to probe when enabling their PHY. Shall I ignore a "not supported by the firmware" code and return 0 to the caller (with a printed warning) so it will continue probing? Doing so is lying as the PHY might not be configured as expected and S2RAM will always fail in this case. Thanks, Miquèl