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=-15.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 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 E2DB5C2BB3F for ; Mon, 7 Dec 2020 01:19:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9D48022D06 for ; Mon, 7 Dec 2020 01:19:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728669AbgLGBTM (ORCPT ); Sun, 6 Dec 2020 20:19:12 -0500 Received: from foss.arm.com ([217.140.110.172]:37880 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728583AbgLGBTM (ORCPT ); Sun, 6 Dec 2020 20:19:12 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0800B11D4; Sun, 6 Dec 2020 17:18:26 -0800 (PST) Received: from [192.168.2.22] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 83E2C3F66B; Sun, 6 Dec 2020 17:18:23 -0800 (PST) Subject: Re: [RESEND PATCH 13/19] phy: sun4i-usb: add support for A100 USB PHY To: Frank Lee , tiny.windzz@gmail.com Cc: Randy Dunlap , linux-kernel@vger.kernel.org, Maxime Ripard , Kishon Vijay Abraham I , Vinod Koul , Krzysztof Kozlowski , Colin Ian King , Chen-Yu Tsai , linux-arm-kernel@lists.infradead.org, Shuosheng Huang References: From: =?UTF-8?Q?Andr=c3=a9_Przywara?= Organization: ARM Ltd. Message-ID: Date: Mon, 7 Dec 2020 01:18:09 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/11/2020 06:40, Frank Lee wrote: Hi, > From: Yangtao Li > > Add support for a100's usb phy, which with 2 PHYs. > > Signed-off-by: Yangtao Li > --- > drivers/phy/allwinner/phy-sun4i-usb.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c > index a6900495baa5..1a0e403131e7 100644 > --- a/drivers/phy/allwinner/phy-sun4i-usb.c > +++ b/drivers/phy/allwinner/phy-sun4i-usb.c > @@ -107,6 +107,7 @@ enum sun4i_usb_phy_type { > sun8i_r40_phy, > sun8i_v3s_phy, > sun50i_a64_phy, > + sun50i_a100_phy, > sun50i_h6_phy, > }; > > @@ -289,7 +290,13 @@ static int sun4i_usb_phy_init(struct phy *_phy) > } > > if (data->cfg->type == sun8i_a83t_phy || > + data->cfg->type == sun50i_a100_phy || > data->cfg->type == sun50i_h6_phy) { > + if (phy->pmu && data->cfg->enable_pmu_unk1) { > + val = readl(phy->pmu + REG_PMU_UNK1); > + writel(val & ~BIT(3), phy->pmu + REG_PMU_UNK1); > + } > + So having a closer look, this does not look right. We should not use this very same variable (enable_pmu_unk1) for a different bit. So what about changing "bool enable_pmu_unk1;" into "u32 pmu_phy_tune_mask;", and using this to mask bits in this PMU register, regardless of the PHY type (above this "if" statement)? We just check it for being 0 and possibly skip the R/M/W sequence. Then the newer SoCs get .pmu_phy_tune_mask = BIT(1), in their config below, and the A100 gets BIT(3). Older PHYs just omit this line at all, are initialised to 0, and are skipped. That would look more cleaner and might even be a bit future proof. Cheers, Andre > if (phy->index == 0) { > val = readl(data->base + data->cfg->phyctl_offset); > val |= PHY_CTL_VBUSVLDEXT; > @@ -339,6 +346,7 @@ static int sun4i_usb_phy_exit(struct phy *_phy) > > if (phy->index == 0) { > if (data->cfg->type == sun8i_a83t_phy || > + data->cfg->type == sun50i_a100_phy || > data->cfg->type == sun50i_h6_phy) { > void __iomem *phyctl = data->base + > data->cfg->phyctl_offset; > @@ -960,6 +968,16 @@ static const struct sun4i_usb_phy_cfg sun50i_a64_cfg = { > .phy0_dual_route = true, > }; > > +static const struct sun4i_usb_phy_cfg sun50i_a100_cfg = { > + .num_phys = 2, > + .type = sun50i_a100_phy, > + .disc_thresh = 3, > + .phyctl_offset = REG_PHYCTL_A33, > + .dedicated_clocks = true, > + .enable_pmu_unk1 = true, > + .phy0_dual_route = true, > +}; > + > static const struct sun4i_usb_phy_cfg sun50i_h6_cfg = { > .num_phys = 4, > .type = sun50i_h6_phy, > @@ -983,6 +1001,7 @@ static const struct of_device_id sun4i_usb_phy_of_match[] = { > { .compatible = "allwinner,sun8i-v3s-usb-phy", .data = &sun8i_v3s_cfg }, > { .compatible = "allwinner,sun50i-a64-usb-phy", > .data = &sun50i_a64_cfg}, > + { .compatible = "allwinner,sun50i-a100-usb-phy", .data = &sun50i_a100_cfg }, > { .compatible = "allwinner,sun50i-h6-usb-phy", .data = &sun50i_h6_cfg }, > { }, > }; > 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=-15.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable 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 DF56FC4361B for ; Mon, 7 Dec 2020 01:20:11 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 76D1222CF6 for ; Mon, 7 Dec 2020 01:20:11 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 76D1222CF6 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=AtFvjQCM+cxbZzCDMInNUEKoQGiOV0vmZHVNE0KE6PQ=; b=AEUQt2wXbPtnAekG/AXlyiuR/ yMR7N5kC08UVdqXBKWeZIO3YjGCoqDQNGQRDaPf82jcoNaCA71GJUNn+SLOXJL7ul3g0rwLUSTZlP ngDByXg+L1+471327OUJLp6IqqhJZv3or4o7HXO315i5GZy7v9Ix3hBPrh4/caF/j7D4fb9kjWGwd 57a4iW75A8F7cmJKKVfyWiFw0zg+CLVSWM0QVegY4wGgaVW74sx0dBw/WwFRzk20ujjfZM2V/jlqp hiT2vSIX6xUcQhl++C1RwKE2dKNUUW99/RRtyoGHqhyI++bdPZfWGXTXpPhmt/LzkWYL+XqDExDyE 6iCwA16Xw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1km5Ae-0007jU-JI; Mon, 07 Dec 2020 01:18:32 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1km5Ac-0007jB-D8 for linux-arm-kernel@lists.infradead.org; Mon, 07 Dec 2020 01:18:31 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0800B11D4; Sun, 6 Dec 2020 17:18:26 -0800 (PST) Received: from [192.168.2.22] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 83E2C3F66B; Sun, 6 Dec 2020 17:18:23 -0800 (PST) Subject: Re: [RESEND PATCH 13/19] phy: sun4i-usb: add support for A100 USB PHY To: Frank Lee , tiny.windzz@gmail.com References: From: =?UTF-8?Q?Andr=c3=a9_Przywara?= Organization: ARM Ltd. Message-ID: Date: Mon, 7 Dec 2020 01:18:09 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201206_201830_542723_13E52D16 X-CRM114-Status: GOOD ( 20.02 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Randy Dunlap , linux-kernel@vger.kernel.org, Krzysztof Kozlowski , Kishon Vijay Abraham I , Vinod Koul , Shuosheng Huang , Maxime Ripard , Colin Ian King , Chen-Yu Tsai , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 10/11/2020 06:40, Frank Lee wrote: Hi, > From: Yangtao Li > > Add support for a100's usb phy, which with 2 PHYs. > > Signed-off-by: Yangtao Li > --- > drivers/phy/allwinner/phy-sun4i-usb.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c > index a6900495baa5..1a0e403131e7 100644 > --- a/drivers/phy/allwinner/phy-sun4i-usb.c > +++ b/drivers/phy/allwinner/phy-sun4i-usb.c > @@ -107,6 +107,7 @@ enum sun4i_usb_phy_type { > sun8i_r40_phy, > sun8i_v3s_phy, > sun50i_a64_phy, > + sun50i_a100_phy, > sun50i_h6_phy, > }; > > @@ -289,7 +290,13 @@ static int sun4i_usb_phy_init(struct phy *_phy) > } > > if (data->cfg->type == sun8i_a83t_phy || > + data->cfg->type == sun50i_a100_phy || > data->cfg->type == sun50i_h6_phy) { > + if (phy->pmu && data->cfg->enable_pmu_unk1) { > + val = readl(phy->pmu + REG_PMU_UNK1); > + writel(val & ~BIT(3), phy->pmu + REG_PMU_UNK1); > + } > + So having a closer look, this does not look right. We should not use this very same variable (enable_pmu_unk1) for a different bit. So what about changing "bool enable_pmu_unk1;" into "u32 pmu_phy_tune_mask;", and using this to mask bits in this PMU register, regardless of the PHY type (above this "if" statement)? We just check it for being 0 and possibly skip the R/M/W sequence. Then the newer SoCs get .pmu_phy_tune_mask = BIT(1), in their config below, and the A100 gets BIT(3). Older PHYs just omit this line at all, are initialised to 0, and are skipped. That would look more cleaner and might even be a bit future proof. Cheers, Andre > if (phy->index == 0) { > val = readl(data->base + data->cfg->phyctl_offset); > val |= PHY_CTL_VBUSVLDEXT; > @@ -339,6 +346,7 @@ static int sun4i_usb_phy_exit(struct phy *_phy) > > if (phy->index == 0) { > if (data->cfg->type == sun8i_a83t_phy || > + data->cfg->type == sun50i_a100_phy || > data->cfg->type == sun50i_h6_phy) { > void __iomem *phyctl = data->base + > data->cfg->phyctl_offset; > @@ -960,6 +968,16 @@ static const struct sun4i_usb_phy_cfg sun50i_a64_cfg = { > .phy0_dual_route = true, > }; > > +static const struct sun4i_usb_phy_cfg sun50i_a100_cfg = { > + .num_phys = 2, > + .type = sun50i_a100_phy, > + .disc_thresh = 3, > + .phyctl_offset = REG_PHYCTL_A33, > + .dedicated_clocks = true, > + .enable_pmu_unk1 = true, > + .phy0_dual_route = true, > +}; > + > static const struct sun4i_usb_phy_cfg sun50i_h6_cfg = { > .num_phys = 4, > .type = sun50i_h6_phy, > @@ -983,6 +1001,7 @@ static const struct of_device_id sun4i_usb_phy_of_match[] = { > { .compatible = "allwinner,sun8i-v3s-usb-phy", .data = &sun8i_v3s_cfg }, > { .compatible = "allwinner,sun50i-a64-usb-phy", > .data = &sun50i_a64_cfg}, > + { .compatible = "allwinner,sun50i-a100-usb-phy", .data = &sun50i_a100_cfg }, > { .compatible = "allwinner,sun50i-h6-usb-phy", .data = &sun50i_h6_cfg }, > { }, > }; > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel