From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754067AbcHZT7K (ORCPT ); Fri, 26 Aug 2016 15:59:10 -0400 Received: from mail-db5eur01on0077.outbound.protection.outlook.com ([104.47.2.77]:38016 "EHLO EUR01-DB5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751449AbcHZT7I (ORCPT ); Fri, 26 Aug 2016 15:59:08 -0400 From: Yongcai Huang To: Russell King - ARM Linux CC: "linux-arm-kernel@lists.infradead.org" , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "shawnguo@kernel.org" , "kernel@pengutronix.de" , Fabio Estevam , "robh+dt@kernel.org" , "mark.rutland@arm.com" Subject: RE: [PATCH 2/3] ARM: imx: add gpcv2 support Thread-Topic: [PATCH 2/3] ARM: imx: add gpcv2 support Thread-Index: AQHR/0mm+KyRhm2buU6P2Mbsv6VvuaBbGGwAgAARbMA= Date: Fri, 26 Aug 2016 12:25:34 +0000 Message-ID: References: <1472209971-32469-1-git-send-email-Anson.Huang@nxp.com> <1472209971-32469-3-git-send-email-Anson.Huang@nxp.com> <20160826111758.GN1041@n2100.armlinux.org.uk> In-Reply-To: <20160826111758.GN1041@n2100.armlinux.org.uk> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=anson.huang@nxp.com; x-originating-ip: [199.59.231.64] x-ms-office365-filtering-correlation-id: 5356b04a-1b40-4511-2a9c-08d3cdac1408 x-microsoft-exchange-diagnostics: 1;AM5PR0402MB2737;6:dct4H3PW8V0NAA+ZzY2GpyMo/SgrxH+PSwWmn5ZvkIvPIAc3k4qLetzFbq7kVIFT2RAtxq1zz13o+5GVrKxHysjWKE0LkEmK9HvBSmLFDaiuCYC9HF/g8h8pYIVlvEHKdK+UGke3LySkIXF+93kTZaoDoEBby2Rf2mjbE051YT45tKg9AfhNNT/jHtO/9zsx1EUxgR561IcWyQMH5b954B/eIDyjj8ZlhjLys5aa5onMvHbb1wLE2uKILRMJWB7I0CE30JwApKVs8zf2BhnVKFTtWzyyXuJynviM/QSu3EglPBsT/ok0W3Db2+r5YKC2MAsowj++0DAs6BgeZ335KA==;5:kSxnfsNqYR+E9wnt8CfArbMMUgQHxtvfHvUYuEQZ78HvQRIJTecUXXySGopvDPjuoV5+kUMkxzmpvpCJLfUo+TR9thTzCm9GDg5+NttY+y5WHvVWvfEm1VusPcOTbrc74kvHDWs0I+o1Bb1y2aRajQ==;24:UYqEX+Y85SkN6NVtt4SxhPBkEOmPFL8z/4mrcihTVTbs+J7QtQz2djmvrActUjSclrbyUjcnFfkSR20kOIGutYINikFL1WhEkvMC3vbZhPw=;7:r32v6RTUyQ4fJNpd7JCZzC+NmmSutXPDlK2bvatkjtuv4pw7rIHE/Q3j0KibT2+l6OspqSURj3wIjwlQKeLyTa6nxIb0fwHmEeQEVcfxua9YTp5x4we0B1jPrmEFNXUBFaF2hckvJUd7J+sNNrSbghHIqKyrbfEwBMQCT8r4Vf9LWZziqiGF15jqzzMDAqH7rDZOnBDyBrNlLMv8Azz4nzpX0LBKAT4z5S2dz+tRQFsBshKep8415NFExeMuWXi1 x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AM5PR0402MB2737; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(250305191791016)(180628864354917)(22074186197030)(9452136761055)(185117386973197)(258649278758335); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6040176)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6055026);SRVR:AM5PR0402MB2737;BCL:0;PCL:0;RULEID:;SRVR:AM5PR0402MB2737; x-forefront-prvs: 00462943DE x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(6009001)(7916002)(199003)(189002)(24454002)(377454003)(377424004)(13464003)(8936002)(305945005)(76576001)(110136002)(92566002)(4326007)(74316002)(7696003)(81166006)(189998001)(5250100002)(66066001)(97736004)(9686002)(3660700001)(68736007)(81156014)(3280700002)(7846002)(5002640100001)(19580405001)(8676002)(5660300001)(54356999)(76176999)(86362001)(575784001)(7736002)(33656002)(2900100001)(2950100001)(106356001)(102836003)(19580395003)(101416001)(15975445007)(586003)(6116002)(3846002)(50986999)(2906002)(87936001)(106116001)(105586002);DIR:OUT;SFP:1101;SCL:1;SRVR:AM5PR0402MB2737;H:AM3PR04MB1315.eurprd04.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Aug 2016 12:25:34.2089 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0402MB2737 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id u7QJxE3b013105 Best Regards! Anson Huang > -----Original Message----- > From: Russell King - ARM Linux [mailto:linux@armlinux.org.uk] > Sent: 2016-08-26 7:18 PM > To: Yongcai Huang > Cc: linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; shawnguo@kernel.org; kernel@pengutronix.de; > Fabio Estevam ; robh+dt@kernel.org; > mark.rutland@arm.com > Subject: Re: [PATCH 2/3] ARM: imx: add gpcv2 support > > On Fri, Aug 26, 2016 at 07:12:50PM +0800, Anson Huang wrote: > > diff --git a/arch/arm/mach-imx/gpcv2.c b/arch/arm/mach-imx/gpcv2.c new > > file mode 100644 index 0000000..98577c4 > > --- /dev/null > > +++ b/arch/arm/mach-imx/gpcv2.c > > @@ -0,0 +1,66 @@ > > +/* > > + * Copyright 2016 Freescale Semiconductor, Inc. > > + * > > + * The code contained herein is licensed under the GNU General Public > > + * License. You may obtain a copy of the GNU General Public License > > + * Version 2 or later at the following locations: > > + * > > + * http://www.opensource.org/licenses/gpl-license.html > > + * http://www.gnu.org/copyleft/gpl.html > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include "common.h" > > + > > +#define GPC_CPU_PGC_SW_PUP_REQ 0xf0 > > +#define GPC_CPU_PGC_SW_PDN_REQ 0xfc > > +#define GPC_PGC_C1 0x840 > > + > > +#define BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7 0x2 > > +#define BM_GPC_PGC_PCG 0x1 > > + > > +static void __iomem *gpcv2_base; > > + > > +static void imx_gpcv2_set_m_core_pgc(bool enable, u32 offset) { > > + u32 val = readl_relaxed(gpcv2_base + offset) & (~BM_GPC_PGC_PCG); > > Unnecessary parens, and the code doesn't flow with the bit clearance here... > > > + > > + if (enable) > > + val |= BM_GPC_PGC_PCG; > > My first read of this lead me to think "okay, so what's the point of > enable=false". It would be clearer with: > > u32 val = readl_relaxed(gpcv2_base + offset); > > if (enable) > val |= BM_GPC_PGC_PCG; > else > val &= ~BM_GPC_PGC_PCG; > > here. Agree, will improve it in V2 patch. > > > + > > + writel_relaxed(val, gpcv2_base + offset); } > > + > > +void imx_gpcv2_set_core1_pdn_pup_by_software(bool pdn) { > > + u32 val = readl_relaxed(gpcv2_base + (pdn ? > > + GPC_CPU_PGC_SW_PDN_REQ : > GPC_CPU_PGC_SW_PUP_REQ)); > > + > > + imx_gpcv2_set_m_core_pgc(true, GPC_PGC_C1); > > + val |= BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7; > > + writel_relaxed(val, gpcv2_base + (pdn ? > > + GPC_CPU_PGC_SW_PDN_REQ : > GPC_CPU_PGC_SW_PUP_REQ)); > > + > > + while ((readl_relaxed(gpcv2_base + (pdn ? > > + GPC_CPU_PGC_SW_PDN_REQ : > GPC_CPU_PGC_SW_PUP_REQ)) & > > + BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7) != 0) > > + ; > > + imx_gpcv2_set_m_core_pgc(false, GPC_PGC_C1); } > > + > > +void __init imx_gpcv2_check_dt(void) > > +{ > > + struct device_node *np; > > + > > + np = of_find_compatible_node(NULL, NULL, "fsl,imx7d-gpc"); > > + if (WARN_ON(!np)) > > + return; > > + > > + gpcv2_base = of_iomap(np, 0); > > + WARN_ON(!gpcv2_base); > > What happens if gpcv2_base is NULL (apart from the obvious warning from the > above line)? I guess a bit later in the boot, > imx_gpcv2_set_core1_pdn_pup_by_software() oopses the kernel, probably > before the console has been initialised. Probably not nice behaviour. > Yes, normal console is NOT ready at this stage, unless early console is enabled. Could you please advise how to handle such case if gpcv2_base is NULL and console is NOT ready? Checking gpcv2_base everywhere before using it? Normally gpcv2_base should NOT be NULL. Thanks. Anson > > +} > > -- > > 1.9.1 > > > > -- > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > according to speedtest.net.