Received: by 2002:a05:6a10:8c0a:0:0:0:0 with SMTP id go10csp4004364pxb; Mon, 1 Feb 2021 09:55:06 -0800 (PST) X-Google-Smtp-Source: ABdhPJwufkkQOl4qNfTJWbQ2/KxCd4MNMgbNiC50bBw22kjtQteWzy70PwVd7ZjW3sXoVBs2C9ov X-Received: by 2002:a50:9f4e:: with SMTP id b72mr3588221edf.4.1612202106135; Mon, 01 Feb 2021 09:55:06 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1612202106; cv=none; d=google.com; s=arc-20160816; b=CNYraFbv1za1JWzMayYrJQEG6G2D2tb3jTL9jCGrM242rZxo+4U+4LkNWbr+0zI4N1 dSpZsu2lJNnRAMd1uWDowJWQts1QwKZqTvTFpNAc1aYZ+f/7745zR7bWf4TMCIeKotjr F3uiRiaCeUOJi8ux2eFe5aftfyAFSVbraUaYDqw3jxTBkgcYPqs0ziVglgug7PoOblPJ Og7wQdASv8BXlwGC0SEkvZAesULMtIv73xL8hRBUA5C/5hVTwZJtx4rfizi4dVULmNG3 liIj6WQgJ6e8yQYksRBlLC+spqyRwc0qr9TG+TvHPiz0dHzO2Tl21Zv4bWf7UuYIfc2c TVaw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:ironport-sdr :ironport-sdr; bh=nbOaMaKGNxZZbCdXUsOo7u/xNDDyrD4HUFuKAsk3mYo=; b=B1MeffELy3ZWE6jo0SGYQ4j68dt2FeOakVvqyRXwPFUC7YWsKqKRjEj+ShMpqN1tyt t/OQI+DJ652Pu1qkLcyqM53DMLBALKZicCvRSuF4BBD9d1FkrGi+TmXJBvfoajJ8Ai+l ChzBjatWLWKcmTnzU2Rp3InI61mCDsGa4zL1tyhMK6geNoPw6/g6JvdabUfkz3pltkBC Ie0ujsNvXk4YkO7O5SvYWwrDEgnctwulXvr+enjdPu7RKksSEVTxLTcqicJ1Qx0EhXju z28dHGkYsg8dgdpYceHuV//+zHth/CO3HQ+oImjRO51Hibkonl5lPb7zFIvHij/KXR8q kiVw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=intel.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id ly24si3151742ejb.67.2021.02.01.09.54.41; Mon, 01 Feb 2021 09:55:06 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231186AbhBARvg (ORCPT + 99 others); Mon, 1 Feb 2021 12:51:36 -0500 Received: from mga18.intel.com ([134.134.136.126]:1039 "EHLO mga18.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232434AbhBARv2 (ORCPT ); Mon, 1 Feb 2021 12:51:28 -0500 IronPort-SDR: o0ynsxVmxUQiox3lOqK6ZMvrb1mku85W86+fZTRH8hO+8QilqN8CySbtMRJbNVaCCRAAIj/79G kqOuQmlvmrNQ== X-IronPort-AV: E=McAfee;i="6000,8403,9882"; a="168405657" X-IronPort-AV: E=Sophos;i="5.79,393,1602572400"; d="scan'208";a="168405657" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Feb 2021 09:50:43 -0800 IronPort-SDR: vwkcuvc/K6/0HYC/dswoUfGzMUtTJ5RCPPD/dQMvTDarAcRp8pDLdNzcYlXlojKNViGybtDGXQ /w90aWaQLK/g== X-IronPort-AV: E=Sophos;i="5.79,393,1602572400"; d="scan'208";a="371637150" Received: from jambrizm-mobl1.amr.corp.intel.com (HELO intel.com) ([10.252.133.15]) by orsmga002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Feb 2021 09:50:42 -0800 Date: Mon, 1 Feb 2021 09:50:41 -0800 From: Ben Widawsky To: Konrad Rzeszutek Wilk Cc: linux-cxl@vger.kernel.org, linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, linux-nvdimm@lists.01.org, linux-pci@vger.kernel.org, Bjorn Helgaas , Chris Browy , Christoph Hellwig , Dan Williams , Ira Weiny , Jon Masters , Jonathan Cameron , Rafael Wysocki , Randy Dunlap , Vishal Verma , daniel.lll@alibaba-inc.com, "John Groves (jgroves)" , "Kelley, Sean V" Subject: Re: [PATCH 03/14] cxl/mem: Find device capabilities Message-ID: <20210201175041.qs56jk5tdbgn2zia@intel.com> References: <20210130002438.1872527-1-ben.widawsky@intel.com> <20210130002438.1872527-4-ben.widawsky@intel.com> <20210201174136.GF197521@fedora> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210201174136.GF197521@fedora> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 21-02-01 12:41:36, Konrad Rzeszutek Wilk wrote: > > +static int cxl_mem_setup_regs(struct cxl_mem *cxlm) > > +{ > > + struct device *dev = &cxlm->pdev->dev; > > + int cap, cap_count; > > + u64 cap_array; > > + > > + cap_array = readq(cxlm->regs + CXLDEV_CAP_ARRAY_OFFSET); > > + if (CXL_GET_FIELD(cap_array, CXLDEV_CAP_ARRAY_ID) != CXLDEV_CAP_ARRAY_CAP_ID) > > + return -ENODEV; > > + > > + cap_count = CXL_GET_FIELD(cap_array, CXLDEV_CAP_ARRAY_COUNT); > > + > > + for (cap = 1; cap <= cap_count; cap++) { > > + void __iomem *register_block; > > + u32 offset; > > + u16 cap_id; > > + > > + cap_id = readl(cxlm->regs + cap * 0x10) & 0xffff; > > + offset = readl(cxlm->regs + cap * 0x10 + 0x4); > > + register_block = cxlm->regs + offset; > > + > > + switch (cap_id) { > > + case CXLDEV_CAP_CAP_ID_DEVICE_STATUS: > > + dev_dbg(dev, "found Status capability (0x%x)\n", > > + offset); > > That 80 character limit is no longer a requirement. Can you just make > this one line? And perhaps change 'found' to 'Found' ? > Funny that. https://lore.kernel.org/linux-cxl/20201111073449.GA16235@infradead.org/ > > + cxlm->status.regs = register_block; > > + break; > > + case CXLDEV_CAP_CAP_ID_PRIMARY_MAILBOX: > > + dev_dbg(dev, "found Mailbox capability (0x%x)\n", > > + offset); > > + cxlm->mbox.regs = register_block; > > + break; > > + case CXLDEV_CAP_CAP_ID_SECONDARY_MAILBOX: > > + dev_dbg(dev, > > + "found Secondary Mailbox capability (0x%x)\n", > > + offset); > > + break; > > + case CXLDEV_CAP_CAP_ID_MEMDEV: > > + dev_dbg(dev, "found Memory Device capability (0x%x)\n", > > + offset); > > + cxlm->mem.regs = register_block; > > + break; > > + default: > > + dev_warn(dev, "Unknown cap ID: %d (0x%x)\n", cap_id, > > + offset); > > + break; > > + } > > + } > > + > > + if (!cxlm->status.regs || !cxlm->mbox.regs || !cxlm->mem.regs) { > > + dev_err(dev, "registers not found: %s%s%s\n", > > + !cxlm->status.regs ? "status " : "", > > + !cxlm->mbox.regs ? "mbox " : "", > > + !cxlm->mem.regs ? "mem" : ""); > > + return -ENXIO; > > + } > > + > > + return 0; > > +} > > + > > +static int cxl_mem_setup_mailbox(struct cxl_mem *cxlm) > > +{ > > + const int cap = cxl_read_mbox_reg32(cxlm, CXLDEV_MB_CAPS_OFFSET); > > + > > + cxlm->mbox.payload_size = > > + 1 << CXL_GET_FIELD(cap, CXLDEV_MB_CAP_PAYLOAD_SIZE); > > + > > I think the static analyzers are not going to be happy that you are not > checking the value of `cap` before using it. > > Perhaps you should check that first before doing the manipulations? > I'm not following the request. CXL_GET_FIELD is just doing the shift and mask on cap. Can you explain what you're hoping to see? > > + /* 8.2.8.4.3 */ > > + if (cxlm->mbox.payload_size < 256) { > > #define for 256?