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=-2.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=no 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 9B85EC43603 for ; Mon, 16 Dec 2019 23:27:38 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 6B27022464 for ; Mon, 16 Dec 2019 23:27:38 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="inPL5wH4"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="MMYzJp6U" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6B27022464 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=roeck-us.net Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-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=bombadil.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:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=BGoSC2khZmWWYT0wmhito9jUIstUS6AwsFv7woQL85Y=; b=inPL5wH4cqaLNR 9hU2SbmsFUPFe4OaBKBR0HEBKNPc46JctqpFbr9mVz+VrWunPvOQNYW0At2b1YaMdC/LU81a90jTU Z2z/htz84wKIjG8Nts/Uxq1qFy1Hdzb51lYrHXUkGRodMbETRJdRhgSSvt9F4PGLEuDaRy5XpP2cY Axati+hw0MJXm/Cj4jy48uOgeaUKzW/i5yz1kBetjOjorglX8N3hNNiwLkBGvKnEVA/qH2k4dUbFP LNMfxTV9CwGk4U7cpg+SlhI56+DShdHvcfI1r7FJmvqohZV2nkrNNrWIwSgEfbSLjNU3dsY14nBDL dga6KMirAmqAgtkpLOAQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1igzlw-0000r4-A8; Mon, 16 Dec 2019 23:27:28 +0000 Received: from mail-pl1-x643.google.com ([2607:f8b0:4864:20::643]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1igzls-0000qN-Ry for linux-arm-kernel@lists.infradead.org; Mon, 16 Dec 2019 23:27:26 +0000 Received: by mail-pl1-x643.google.com with SMTP id c13so5201088pls.0 for ; Mon, 16 Dec 2019 15:27:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=pP1hagqSYDwwA2YjZ9gfdWx6xDMgnbeyBHGfmn3+DE0=; b=MMYzJp6UxQUQNh58RYjrQGcY6a+hdQWcxSmKtC7oHVd0+x7KJpndeE6yPq3xBAl3jZ B/MX8003PV+SZ2nwo8AxyMJHjMHWpXKKFZsX3sWY/ZhgsSxWp3VW8rlFk3N5TVHYYYEX qpki9caXNC3/QO8osDQ43QRMuA/yh72mDh6umUSu4mOhWTdU8DaM6XUsO20xR7HH/TAY lE2AQsIq2ECvmaIM5v3+cWuHGSiTBIqbd/D1GSPw8a4IbTUmHJ1vwbN5KmsKT+zS7PBd VmO+ZH5dNK6HKl8B0m1c0D/SeFnNP8WfLnr7i9jiwoQ/Z3YF/c3IFNfjz1CggHZdxXJ0 rjqQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition :content-transfer-encoding:in-reply-to:user-agent; bh=pP1hagqSYDwwA2YjZ9gfdWx6xDMgnbeyBHGfmn3+DE0=; b=FvVIb42vkEocHm9S111riaKx9fcTnRpHPcVUlzddXOXrPSXLMTb71FMVx6lulP9Sme QDYOrYagp12EZfdOoKVBWQ6dttf4Yj/+4hOeGQjNQWvXA189x6nE2YRF+9GO4UmJUgW8 B4uJRQgmsmTnLxb+8ULerJ09euorr6gmsJ2zDRPsrSSSD1gFsXqLDH8u8UHQrdzJE8uZ p4GDZwAVLCQIXW2E5Klz7n0D0/eUqOq2S2dl8IMGHnXxmj8c4eHDTrWXseBkTxL5i2eo qrxiGy66hzCc5EP4q7e/jb9JMFBXJtnJF2U6zpZZ5reNz6NosMq55PLlJ48dz8OjO5rO wArw== X-Gm-Message-State: APjAAAX2QFgYvO/Vahb5rOiyImMrr6PtTEG3LFfGAdi0t+jOIAzhVxMz hg3zw8JSxPE/0MLCUtkjORI= X-Google-Smtp-Source: APXvYqxGQ8NGtMEivlA2YMXllL8k/BlL9MhGxi6roCjQUTeTt1nWxWnBdxfsHtWDSc0zFy7zNwVsAA== X-Received: by 2002:a17:90a:bd8f:: with SMTP id z15mr2370209pjr.54.1576538842771; Mon, 16 Dec 2019 15:27:22 -0800 (PST) Received: from localhost ([2600:1700:e321:62f0:329c:23ff:fee3:9d7c]) by smtp.gmail.com with ESMTPSA id k9sm593648pje.26.2019.12.16.15.27.20 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 16 Dec 2019 15:27:21 -0800 (PST) Date: Mon, 16 Dec 2019 15:27:20 -0800 From: Guenter Roeck To: Jae Hyun Yoo Subject: Re: [PATCH v11 14/14] hwmon: Add PECI dimmtemp driver Message-ID: <20191216232720.GA17398@roeck-us.net> References: <20191211194624.2872-1-jae.hyun.yoo@linux.intel.com> <20191211194624.2872-15-jae.hyun.yoo@linux.intel.com> <5ed9f292-e024-ffda-a1a8-870ba0f05c58@linux.intel.com> <20191216212120.GA12089@roeck-us.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20191216_152724_906576_82AE0728 X-CRM114-Status: GOOD ( 22.97 ) 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: Mark Rutland , Andrew Lunn , Randy Dunlap , Tomer Maimon , devicetree@vger.kernel.org, Frederic Barrat , Mauro Carvalho Chehab , Lee Jones , Jason M Biils , Eric Sandeen , Lorenzo Pieralisi , Jonathan Corbet , openbmc@lists.ozlabs.org, linux-doc@vger.kernel.org, Kishon Vijay Abraham I , Joel Stanley , "Bryant G . Ly" , Uwe Kleine-Konig , David Kershner , Wu Hao , linux-hwmon@vger.kernel.org, Jean Delvare , Arnd Bergmann , Philippe Ombredanne , Johan Hovold , Tomohiro Kusumi , Rob Herring , Stef van Os , Thomas Gleixner , Andy Shevchenko , Sagar Dharia , linux-arm-kernel@lists.infradead.org, Alan Cox , Juergen Gross , Cyrille Pitchen , Andrew Jeffery , Greg Kroah-Hartman , "Darrick J . Wong" , Stephen Boyd , Miguel Ojeda , Vinod Koul , Gustavo Pimentel , Andrew Morton , "David S . Miller" Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Dec 16, 2019 at 02:17:34PM -0800, Jae Hyun Yoo wrote: > [...] > = > > > > > +static int get_dimm_temp(struct peci_dimmtemp *priv, int dimm_no) > > > > > +{ > > > > > +=A0=A0=A0 int dimm_order =3D dimm_no % priv->gen_info->dimm_idx_= max; > > > > > +=A0=A0=A0 int chan_rank =3D dimm_no / priv->gen_info->dimm_idx_m= ax; > > > > > +=A0=A0=A0 struct peci_rd_pci_cfg_local_msg rp_msg; > > > > > +=A0=A0=A0 u8=A0 cfg_data[4]; > > > > > +=A0=A0=A0 int ret; > > > > > + > > > > > +=A0=A0=A0 if (!peci_sensor_need_update(&priv->temp[dimm_no])) > > > > > +=A0=A0=A0=A0=A0=A0=A0 return 0; > > > > > + > > > > > +=A0=A0=A0 ret =3D read_ddr_dimm_temp_config(priv, chan_rank, cfg= _data); > > > > > +=A0=A0=A0 if (ret) > > > > > +=A0=A0=A0=A0=A0=A0=A0 return ret; > > > > > + > > > > > +=A0=A0=A0 priv->temp[dimm_no].value =3D cfg_data[dimm_order] * 1= 000; > > > > > + > > > > > +=A0=A0=A0 switch (priv->gen_info->model) { > > > > > +=A0=A0=A0 case INTEL_FAM6_SKYLAKE_X: > > > > > +=A0=A0=A0=A0=A0=A0=A0 rp_msg.addr =3D priv->mgr->client->addr; > > > > > +=A0=A0=A0=A0=A0=A0=A0 rp_msg.bus =3D 2; > > > > > +=A0=A0=A0=A0=A0=A0=A0 /* > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 * Device 10, Function 2: IMC 0 channel = 0 -> rank 0 > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 * Device 10, Function 6: IMC 0 channel = 1 -> rank 1 > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 * Device 11, Function 2: IMC 0 channel = 2 -> rank 2 > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 * Device 12, Function 2: IMC 1 channel = 0 -> rank 3 > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 * Device 12, Function 6: IMC 1 channel = 1 -> rank 4 > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 * Device 13, Function 2: IMC 1 channel = 2 -> rank 5 > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 */ > > > > > +=A0=A0=A0=A0=A0=A0=A0 rp_msg.device =3D 10 + chan_rank / 3 * 2 + > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 (chan_rank % 3 = =3D=3D 2 ? 1 : 0); > > > > > +=A0=A0=A0=A0=A0=A0=A0 rp_msg.function =3D chan_rank % 3 =3D=3D 1= ? 6 : 2; > > > > > +=A0=A0=A0=A0=A0=A0=A0 rp_msg.reg =3D 0x120 + dimm_order * 4; > > > > > +=A0=A0=A0=A0=A0=A0=A0 rp_msg.rx_len =3D 4; > > > > > + > > > > > +=A0=A0=A0=A0=A0=A0=A0 ret =3D peci_command(priv->mgr->client->ad= apter, > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 PECI_CMD_= RD_PCI_CFG_LOCAL, &rp_msg); > > > > > +=A0=A0=A0=A0=A0=A0=A0 if (rp_msg.cc !=3D PECI_DEV_CC_SUCCESS) > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 ret =3D -EAGAIN; > > > > > +=A0=A0=A0=A0=A0=A0=A0 if (ret) > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return ret; > > > > > + > > > > > +=A0=A0=A0=A0=A0=A0=A0 priv->temp_max[dimm_no] =3D rp_msg.pci_con= fig[1] * 1000; > > > > > +=A0=A0=A0=A0=A0=A0=A0 priv->temp_crit[dimm_no] =3D rp_msg.pci_co= nfig[2] * 1000; > > > > > +=A0=A0=A0=A0=A0=A0=A0 break; > > > > > +=A0=A0=A0 case INTEL_FAM6_SKYLAKE_XD: > > > > > +=A0=A0=A0=A0=A0=A0=A0 rp_msg.addr =3D priv->mgr->client->addr; > > > > > +=A0=A0=A0=A0=A0=A0=A0 rp_msg.bus =3D 2; > > > > > +=A0=A0=A0=A0=A0=A0=A0 /* > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 * Device 10, Function 2: IMC 0 channel = 0 -> rank 0 > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 * Device 10, Function 6: IMC 0 channel = 1 -> rank 1 > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 * Device 12, Function 2: IMC 1 channel = 0 -> rank 2 > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 * Device 12, Function 6: IMC 1 channel = 1 -> rank 3 > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 */ > > > > > +=A0=A0=A0=A0=A0=A0=A0 rp_msg.device =3D 10 + chan_rank / 2 * 2; > > > > > +=A0=A0=A0=A0=A0=A0=A0 rp_msg.function =3D (chan_rank % 2) ? 6 : = 2; > > > > > +=A0=A0=A0=A0=A0=A0=A0 rp_msg.reg =3D 0x120 + dimm_order * 4; > > > > > +=A0=A0=A0=A0=A0=A0=A0 rp_msg.rx_len =3D 4; > > > > > + > > > > > +=A0=A0=A0=A0=A0=A0=A0 ret =3D peci_command(priv->mgr->client->ad= apter, > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 PECI_CMD_= RD_PCI_CFG_LOCAL, &rp_msg); > > > > > +=A0=A0=A0=A0=A0=A0=A0 if (rp_msg.cc !=3D PECI_DEV_CC_SUCCESS) > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 ret =3D -EAGAIN; > > > > > +=A0=A0=A0=A0=A0=A0=A0 if (ret) > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return ret; > > > > > + > > > > > +=A0=A0=A0=A0=A0=A0=A0 priv->temp_max[dimm_no] =3D rp_msg.pci_con= fig[1] * 1000; > > > > > +=A0=A0=A0=A0=A0=A0=A0 priv->temp_crit[dimm_no] =3D rp_msg.pci_co= nfig[2] * 1000; > > > > > +=A0=A0=A0=A0=A0=A0=A0 break; > > > > > +=A0=A0=A0 case INTEL_FAM6_HASWELL_X: > > > > > +=A0=A0=A0 case INTEL_FAM6_BROADWELL_X: > > > > > +=A0=A0=A0=A0=A0=A0=A0 rp_msg.addr =3D priv->mgr->client->addr; > > > > > +=A0=A0=A0=A0=A0=A0=A0 rp_msg.bus =3D 1; > > > > > +=A0=A0=A0=A0=A0=A0=A0 /* > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 * Device 20, Function 0: IMC 0 channel = 0 -> rank 0 > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 * Device 20, Function 1: IMC 0 channel = 1 -> rank 1 > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 * Device 21, Function 0: IMC 0 channel = 2 -> rank 2 > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 * Device 21, Function 1: IMC 0 channel = 3 -> rank 3 > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 * Device 23, Function 0: IMC 1 channel = 0 -> rank 4 > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 * Device 23, Function 1: IMC 1 channel = 1 -> rank 5 > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 * Device 24, Function 0: IMC 1 channel = 2 -> rank 6 > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 * Device 24, Function 1: IMC 1 channel = 3 -> rank 7 > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0 */ > > > > > +=A0=A0=A0=A0=A0=A0=A0 rp_msg.device =3D 20 + chan_rank / 2 + cha= n_rank / 4; > > > > > +=A0=A0=A0=A0=A0=A0=A0 rp_msg.function =3D chan_rank % 2; > > > > > +=A0=A0=A0=A0=A0=A0=A0 rp_msg.reg =3D 0x120 + dimm_order * 4; > > > > > +=A0=A0=A0=A0=A0=A0=A0 rp_msg.rx_len =3D 4; > > > > > + > > > > > +=A0=A0=A0=A0=A0=A0=A0 ret =3D peci_command(priv->mgr->client->ad= apter, > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 PECI_CMD_= RD_PCI_CFG_LOCAL, &rp_msg); > > > > > +=A0=A0=A0=A0=A0=A0=A0 if (rp_msg.cc !=3D PECI_DEV_CC_SUCCESS) > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 ret =3D -EAGAIN; > > > > > +=A0=A0=A0=A0=A0=A0=A0 if (ret) > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return ret; > > > > > + > > > > > +=A0=A0=A0=A0=A0=A0=A0 priv->temp_max[dimm_no] =3D rp_msg.pci_con= fig[1] * 1000; > > > > > +=A0=A0=A0=A0=A0=A0=A0 priv->temp_crit[dimm_no] =3D rp_msg.pci_co= nfig[2] * 1000; > > > > > +=A0=A0=A0=A0=A0=A0=A0 break; > > > > > +=A0=A0=A0 default: > > > > > +=A0=A0=A0=A0=A0=A0=A0 return -EOPNOTSUPP; > > > > = > > > > It looks like the sensors are created even on unsupported platforms, > > > > which would generate error messages whenever someone tries to read > > > > the attributes. > > > > = > > > > There should be some code early on checking this, and the driver > > > > should not even instantiate if the CPU model is not supported. > > > = > > > Actually, this 'default' case will not be happened because this driver > > > will be registered only when the CPU model is supported. The CPU model > > > checking code is in 'intel-peci-client.c' which is [11/14] of this > > > patch set. > > > = > > = > > That again assumes that both drivers will be modified in sync in the fu= ture. > > We can not make that assumption. > = > As you said, both drivers must be modified in sync in the future because > each Intel CPU family uses different way of reading DIMM temperature. > In case if supported CPU checking code updated without making sync with > it, this driver will return the error. > = ... and in that situation the driver should not instantiate in the first place. Its probe function should return -ENODEV. Guenter _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel