From mboxrd@z Thu Jan 1 00:00:00 1970 From: hayashi.kunihiko@socionext.com (Kunihiko Hayashi) Date: Wed, 11 Jul 2018 21:05:14 +0900 Subject: [PATCH 2/4] phy: socionext: add USB3 PHY driver for UniPhier SoC In-Reply-To: <20180709202319.1091.4A936039@socionext.com> References: <4b436a46-9046-25ad-ee6d-ad2cdb16e2ca@ti.com> <20180709202319.1091.4A936039@socionext.com> Message-ID: <20180711210514.BB32.4A936039@socionext.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Mon, 9 Jul 2018 20:23:19 +0900 wrote: > Hi Kishon, > Thank you for your comments. > > On Mon, 9 Jul 2018 10:49:50 +0530 wrote: > > > Hi, > > > > On Friday 29 June 2018 02:08 PM, Kunihiko Hayashi wrote: > > > Add a driver for PHY interface built into USB3 controller > > > implemented in UniPhier SoCs. > > > This driver supports High-Speed PHY and Super-Speed PHY. > > > > > > Signed-off-by: Kunihiko Hayashi > > > Signed-off-by: Motoya Tanigawa > > > Signed-off-by: Masami Hiramatsu > > > --- > > > drivers/phy/Kconfig | 1 + > > > drivers/phy/Makefile | 1 + > > > drivers/phy/socionext/Kconfig | 12 + > > > drivers/phy/socionext/Makefile | 6 + > > > drivers/phy/socionext/phy-uniphier-usb3hs.c | 422 ++++++++++++++++++++++++++++ > > > drivers/phy/socionext/phy-uniphier-usb3ss.c | 369 ++++++++++++++++++++++++ > > > 6 files changed, 811 insertions(+) > > > create mode 100644 drivers/phy/socionext/Kconfig > > > create mode 100644 drivers/phy/socionext/Makefile > > > create mode 100644 drivers/phy/socionext/phy-uniphier-usb3hs.c > > > create mode 100644 drivers/phy/socionext/phy-uniphier-usb3ss.c > > (snip...) > > > > --- /dev/null > > > +++ b/drivers/phy/socionext/phy-uniphier-usb3hs.c > > > @@ -0,0 +1,422 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * phy-uniphier-usb3hs.c - HS-PHY driver for Socionext UniPhier USB3 controller > > > + * Copyright 2015-2018 Socionext Inc. > > > + * Author: > > > + * Kunihiko Hayashi > > > + * Contributors: > > > + * Motoya Tanigawa > > > + * Masami Hiramatsu > > > + */ > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +#define HSPHY_CFG0 0x0 > > > +#define HSPHY_CFG0_HS_I_MASK GENMASK(31, 28) > > > +#define HSPHY_CFG0_HSDISC_MASK GENMASK(27, 26) > > > +#define HSPHY_CFG0_SWING_MASK GENMASK(17, 16) > > > +#define HSPHY_CFG0_SEL_T_MASK GENMASK(15, 12) > > > +#define HSPHY_CFG0_RTERM_MASK GENMASK(7, 6) > > > +#define HSPHY_CFG0_TRIMMASK (HSPHY_CFG0_HS_I_MASK \ > > > + | HSPHY_CFG0_SEL_T_MASK \ > > > + | HSPHY_CFG0_RTERM_MASK) > > > + > > > +#define HSPHY_CFG1 0x4 > > > +#define HSPHY_CFG1_DAT_EN BIT(29) > > > +#define HSPHY_CFG1_ADR_EN BIT(28) > > > +#define HSPHY_CFG1_ADR_MASK GENMASK(27, 16) > > > +#define HSPHY_CFG1_DAT_MASK GENMASK(23, 16) > > > + > > > +#define MAX_CLKS 3 > > > +#define MAX_RSTS 2 > > > +#define MAX_PHY_PARAMS 1 > > > + > > > +struct uniphier_u3hsphy_param { > > > + u32 addr; > > > + u32 mask; > > > + u32 val; > > > +}; > > > > I'd like to avoid configure the PHY this way, since it's impossible to know > > which register is being configured. > This way might be misunderstood. These HS-PHY and SS-PHY have "internal" registers, which are not memory-mapped. And to access these internal registers, we need to specify the number corresponding to the register. The "addr" in "uniphier_u3hsphy_param" is just the number of the register. The "mask" shows a bitfield of the register, that means one of PHY parameters. The "value" shows a parameter value to set to the bitfield. > I see. > In order to know which register is set, I'll add definitions for address > and mask. > And I think the driver might have functions for each SoC to configure > the registers instead of uniphier_u3hsphy_param. Then, I think it's sufficient to define a macro with a meaningful label for each bitfield, and PHY parameters are expressed like that. { 10, FOO_BAR_MODE, 0x2 }, /* set 2 to FOO_BAR_MODE in register 10 */ In this case, we need to shift "value" according to "mask". Thank you, --- Best Regards, Kunihiko Hayashi From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kunihiko Hayashi Subject: Re: [PATCH 2/4] phy: socionext: add USB3 PHY driver for UniPhier SoC Date: Wed, 11 Jul 2018 21:05:14 +0900 Message-ID: <20180711210514.BB32.4A936039@socionext.com> References: <4b436a46-9046-25ad-ee6d-ad2cdb16e2ca@ti.com> <20180709202319.1091.4A936039@socionext.com> Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20180709202319.1091.4A936039@socionext.com> Sender: linux-kernel-owner@vger.kernel.org To: Kishon Vijay Abraham I Cc: Rob Herring , Mark Rutland , Masahiro Yamada , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Masami Hiramatsu , Jassi Brar List-Id: devicetree@vger.kernel.org On Mon, 9 Jul 2018 20:23:19 +0900 wrote: > Hi Kishon, > Thank you for your comments. > > On Mon, 9 Jul 2018 10:49:50 +0530 wrote: > > > Hi, > > > > On Friday 29 June 2018 02:08 PM, Kunihiko Hayashi wrote: > > > Add a driver for PHY interface built into USB3 controller > > > implemented in UniPhier SoCs. > > > This driver supports High-Speed PHY and Super-Speed PHY. > > > > > > Signed-off-by: Kunihiko Hayashi > > > Signed-off-by: Motoya Tanigawa > > > Signed-off-by: Masami Hiramatsu > > > --- > > > drivers/phy/Kconfig | 1 + > > > drivers/phy/Makefile | 1 + > > > drivers/phy/socionext/Kconfig | 12 + > > > drivers/phy/socionext/Makefile | 6 + > > > drivers/phy/socionext/phy-uniphier-usb3hs.c | 422 ++++++++++++++++++++++++++++ > > > drivers/phy/socionext/phy-uniphier-usb3ss.c | 369 ++++++++++++++++++++++++ > > > 6 files changed, 811 insertions(+) > > > create mode 100644 drivers/phy/socionext/Kconfig > > > create mode 100644 drivers/phy/socionext/Makefile > > > create mode 100644 drivers/phy/socionext/phy-uniphier-usb3hs.c > > > create mode 100644 drivers/phy/socionext/phy-uniphier-usb3ss.c > > (snip...) > > > > --- /dev/null > > > +++ b/drivers/phy/socionext/phy-uniphier-usb3hs.c > > > @@ -0,0 +1,422 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * phy-uniphier-usb3hs.c - HS-PHY driver for Socionext UniPhier USB3 controller > > > + * Copyright 2015-2018 Socionext Inc. > > > + * Author: > > > + * Kunihiko Hayashi > > > + * Contributors: > > > + * Motoya Tanigawa > > > + * Masami Hiramatsu > > > + */ > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +#define HSPHY_CFG0 0x0 > > > +#define HSPHY_CFG0_HS_I_MASK GENMASK(31, 28) > > > +#define HSPHY_CFG0_HSDISC_MASK GENMASK(27, 26) > > > +#define HSPHY_CFG0_SWING_MASK GENMASK(17, 16) > > > +#define HSPHY_CFG0_SEL_T_MASK GENMASK(15, 12) > > > +#define HSPHY_CFG0_RTERM_MASK GENMASK(7, 6) > > > +#define HSPHY_CFG0_TRIMMASK (HSPHY_CFG0_HS_I_MASK \ > > > + | HSPHY_CFG0_SEL_T_MASK \ > > > + | HSPHY_CFG0_RTERM_MASK) > > > + > > > +#define HSPHY_CFG1 0x4 > > > +#define HSPHY_CFG1_DAT_EN BIT(29) > > > +#define HSPHY_CFG1_ADR_EN BIT(28) > > > +#define HSPHY_CFG1_ADR_MASK GENMASK(27, 16) > > > +#define HSPHY_CFG1_DAT_MASK GENMASK(23, 16) > > > + > > > +#define MAX_CLKS 3 > > > +#define MAX_RSTS 2 > > > +#define MAX_PHY_PARAMS 1 > > > + > > > +struct uniphier_u3hsphy_param { > > > + u32 addr; > > > + u32 mask; > > > + u32 val; > > > +}; > > > > I'd like to avoid configure the PHY this way, since it's impossible to know > > which register is being configured. > This way might be misunderstood. These HS-PHY and SS-PHY have "internal" registers, which are not memory-mapped. And to access these internal registers, we need to specify the number corresponding to the register. The "addr" in "uniphier_u3hsphy_param" is just the number of the register. The "mask" shows a bitfield of the register, that means one of PHY parameters. The "value" shows a parameter value to set to the bitfield. > I see. > In order to know which register is set, I'll add definitions for address > and mask. > And I think the driver might have functions for each SoC to configure > the registers instead of uniphier_u3hsphy_param. Then, I think it's sufficient to define a macro with a meaningful label for each bitfield, and PHY parameters are expressed like that. { 10, FOO_BAR_MODE, 0x2 }, /* set 2 to FOO_BAR_MODE in register 10 */ In this case, we need to shift "value" according to "mask". Thank you, --- Best Regards, Kunihiko Hayashi 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=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,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 C9BCDC5CFE7 for ; Wed, 11 Jul 2018 12:05:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 8944C20BF2 for ; Wed, 11 Jul 2018 12:05:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8944C20BF2 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=socionext.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 S1732661AbeGKMJR (ORCPT ); Wed, 11 Jul 2018 08:09:17 -0400 Received: from mx.socionext.com ([202.248.49.38]:23374 "EHLO mx.socionext.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726457AbeGKMJR (ORCPT ); Wed, 11 Jul 2018 08:09:17 -0400 Received: from unknown (HELO iyokan-ex.css.socionext.com) ([172.31.9.54]) by mx.socionext.com with ESMTP; 11 Jul 2018 21:05:15 +0900 Received: from mail.mfilter.local (m-filter-2 [10.213.24.62]) by iyokan-ex.css.socionext.com (Postfix) with ESMTP id E1A7560034; Wed, 11 Jul 2018 21:05:15 +0900 (JST) Received: from 172.31.9.53 (172.31.9.53) by m-FILTER with ESMTP; Wed, 11 Jul 2018 21:05:15 +0900 Received: from yuzu.css.socionext.com (yuzu [172.31.8.45]) by iyokan.css.socionext.com (Postfix) with ESMTP id 8358340220; Wed, 11 Jul 2018 21:05:15 +0900 (JST) Received: from [127.0.0.1] (unknown [10.213.132.48]) by yuzu.css.socionext.com (Postfix) with ESMTP id 504C5120423; Wed, 11 Jul 2018 21:05:15 +0900 (JST) Date: Wed, 11 Jul 2018 21:05:14 +0900 From: Kunihiko Hayashi To: Kishon Vijay Abraham I Subject: Re: [PATCH 2/4] phy: socionext: add USB3 PHY driver for UniPhier SoC Cc: Rob Herring , Mark Rutland , Masahiro Yamada , , , , Masami Hiramatsu , Jassi Brar In-Reply-To: <20180709202319.1091.4A936039@socionext.com> References: <4b436a46-9046-25ad-ee6d-ad2cdb16e2ca@ti.com> <20180709202319.1091.4A936039@socionext.com> Message-Id: <20180711210514.BB32.4A936039@socionext.com> MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Mailer: Becky! ver. 2.70 [ja] Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 9 Jul 2018 20:23:19 +0900 wrote: > Hi Kishon, > Thank you for your comments. > > On Mon, 9 Jul 2018 10:49:50 +0530 wrote: > > > Hi, > > > > On Friday 29 June 2018 02:08 PM, Kunihiko Hayashi wrote: > > > Add a driver for PHY interface built into USB3 controller > > > implemented in UniPhier SoCs. > > > This driver supports High-Speed PHY and Super-Speed PHY. > > > > > > Signed-off-by: Kunihiko Hayashi > > > Signed-off-by: Motoya Tanigawa > > > Signed-off-by: Masami Hiramatsu > > > --- > > > drivers/phy/Kconfig | 1 + > > > drivers/phy/Makefile | 1 + > > > drivers/phy/socionext/Kconfig | 12 + > > > drivers/phy/socionext/Makefile | 6 + > > > drivers/phy/socionext/phy-uniphier-usb3hs.c | 422 ++++++++++++++++++++++++++++ > > > drivers/phy/socionext/phy-uniphier-usb3ss.c | 369 ++++++++++++++++++++++++ > > > 6 files changed, 811 insertions(+) > > > create mode 100644 drivers/phy/socionext/Kconfig > > > create mode 100644 drivers/phy/socionext/Makefile > > > create mode 100644 drivers/phy/socionext/phy-uniphier-usb3hs.c > > > create mode 100644 drivers/phy/socionext/phy-uniphier-usb3ss.c > > (snip...) > > > > --- /dev/null > > > +++ b/drivers/phy/socionext/phy-uniphier-usb3hs.c > > > @@ -0,0 +1,422 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * phy-uniphier-usb3hs.c - HS-PHY driver for Socionext UniPhier USB3 controller > > > + * Copyright 2015-2018 Socionext Inc. > > > + * Author: > > > + * Kunihiko Hayashi > > > + * Contributors: > > > + * Motoya Tanigawa > > > + * Masami Hiramatsu > > > + */ > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +#define HSPHY_CFG0 0x0 > > > +#define HSPHY_CFG0_HS_I_MASK GENMASK(31, 28) > > > +#define HSPHY_CFG0_HSDISC_MASK GENMASK(27, 26) > > > +#define HSPHY_CFG0_SWING_MASK GENMASK(17, 16) > > > +#define HSPHY_CFG0_SEL_T_MASK GENMASK(15, 12) > > > +#define HSPHY_CFG0_RTERM_MASK GENMASK(7, 6) > > > +#define HSPHY_CFG0_TRIMMASK (HSPHY_CFG0_HS_I_MASK \ > > > + | HSPHY_CFG0_SEL_T_MASK \ > > > + | HSPHY_CFG0_RTERM_MASK) > > > + > > > +#define HSPHY_CFG1 0x4 > > > +#define HSPHY_CFG1_DAT_EN BIT(29) > > > +#define HSPHY_CFG1_ADR_EN BIT(28) > > > +#define HSPHY_CFG1_ADR_MASK GENMASK(27, 16) > > > +#define HSPHY_CFG1_DAT_MASK GENMASK(23, 16) > > > + > > > +#define MAX_CLKS 3 > > > +#define MAX_RSTS 2 > > > +#define MAX_PHY_PARAMS 1 > > > + > > > +struct uniphier_u3hsphy_param { > > > + u32 addr; > > > + u32 mask; > > > + u32 val; > > > +}; > > > > I'd like to avoid configure the PHY this way, since it's impossible to know > > which register is being configured. > This way might be misunderstood. These HS-PHY and SS-PHY have "internal" registers, which are not memory-mapped. And to access these internal registers, we need to specify the number corresponding to the register. The "addr" in "uniphier_u3hsphy_param" is just the number of the register. The "mask" shows a bitfield of the register, that means one of PHY parameters. The "value" shows a parameter value to set to the bitfield. > I see. > In order to know which register is set, I'll add definitions for address > and mask. > And I think the driver might have functions for each SoC to configure > the registers instead of uniphier_u3hsphy_param. Then, I think it's sufficient to define a macro with a meaningful label for each bitfield, and PHY parameters are expressed like that. { 10, FOO_BAR_MODE, 0x2 }, /* set 2 to FOO_BAR_MODE in register 10 */ In this case, we need to shift "value" according to "mask". Thank you, --- Best Regards, Kunihiko Hayashi