Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <11d4ef87-9724-dd61-e1b1-aef73afcef9f@intel.com>
Date:   Thu, 22 Jun 2023 17:03:53 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Betterbird/102.11.1
Subject: Re: [PATCH v6 18/27] cxl/hdm: Use stored Component Register mappings
 to map HDM decoder capability
To:     Terry Bowman <terry.bowman@amd.com>, <alison.schofield@intel.com>,
        <vishal.l.verma@intel.com>, <ira.weiny@intel.com>,
        <bwidawsk@kernel.org>, <dan.j.williams@intel.com>,
        <Jonathan.Cameron@huawei.com>, <linux-cxl@vger.kernel.org>
CC:     <rrichter@amd.com>, <linux-kernel@vger.kernel.org>,
        <bhelgaas@google.com>
References: <20230622035126.4130151-1-terry.bowman@amd.com>
 <20230622035126.4130151-19-terry.bowman@amd.com>
Content-Language: en-US
From:   Dave Jiang <dave.jiang@intel.com>
In-Reply-To: <20230622035126.4130151-19-terry.bowman@amd.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?N3hIQ2hjV3VXaWhXY1dMaGhBSWh0djdDNDRUd1YxN2xycUNGdWgyYkJUb1BC?=
 =?utf-8?B?QTZZZVpQeXJDVXlUK3laV2ptZlh2Wk1xNExFNWpmSjlVd051VVBBSUozejRZ?=
 =?utf-8?B?TTV4VGFDRENRY3BnaVJoZm1iWm9oSDk0eFBBbW9mbThKaUZ2WW9ERm12dS9C?=
 =?utf-8?B?NGQ3VHNndTVYcjA3N0ZvNGpjRVFsbmY1aHZnNWRTZFkySGZZclB4MXZPWktJ?=
 =?utf-8?B?SW9reHdNbENyMXl3cVg3aVlKekdrUzlXYXFld2thMDJZWnJFa1FDaU5tRFdS?=
 =?utf-8?B?eXAyVVdNR3RNSzBieUkxKy85VlNuaDZaUW5Pbm5Rby9LRjZyaDhocEx0UVhV?=
 =?utf-8?B?eXowMEc5bVFlYkpnZjdrMVFCOEdPT0NRTEVaTmZTMmFhSzUvNmREc2E0ZEhD?=
 =?utf-8?B?S1Q2R0Z0R1I4QXphOExYaDJsMUt6NkFjaTZnSDlUL1NCRi9tYytoblhUWXBz?=
 =?utf-8?B?Z2hLWXRrcjVxdmIxSkpXODNQMUhLVXIrU0gzRUZGei93dnV3VkxwekFISWhv?=
 =?utf-8?B?emdyZ2hMZUpVcGJIZjVteThaS2E1MHNyTDVtalBuTi9uSm02bjloaVhnN0Jq?=
 =?utf-8?B?c0pKTEpCeXdSdW1RVHFPMGt6VmYrY05RekNldnBvQkZUMEpmaCtYUklPZGFl?=
 =?utf-8?B?c0d5N3RMdUtIa1RRMmZ0N0ZkM0N1R1NUSWhEQnQvOVdVSWFsZk55eU5UTWtL?=
 =?utf-8?B?S3pMOXZtb2gxZ3RiK0dONjBQTm1ML2ViV1ZQaFJkaHU0RTZra1JKSll5T2h2?=
 =?utf-8?B?NFZyM2xOZC9ocnZJT3R0QUFJd0lLSEI3QTNVNmErek1KQW9pVTR3VnQrbzhV?=
 =?utf-8?B?V1hNVlhON3cwcWFJMTVick1IeXhsZWpXb014MUh6U096RGR3dzRpenpNaGor?=
 =?utf-8?B?M3RSdTI5VmZTYS80a2R0Zm9MRnBScDNaR2FTR3NybmhnZGQzTFNWK2t3MjMy?=
 =?utf-8?B?UWgrR082S1owd24wL0pzb0FNRmR4QzRxT1pYSnlxL282bHIvSFA5TlV1ZkxX?=
 =?utf-8?B?TURYWHRBdlc4TTIzd3poaUZleVp3WGliVTFPSU5jN29OUzZmTE5aREc3c3VR?=
 =?utf-8?B?YXgvenpRTTBjd0hjYWhzVzM5aW1OWHg4R1pNSWJkT1R3Zmx5Y0xGanUvM0lR?=
 =?utf-8?B?ZjRXVE9Gd2svVkpZQ1VITTA1K1ZoZVZLY2w5Nk4zL0JWSFVtRG83clUxa2pk?=
 =?utf-8?B?YmdXd2l0ZUxBblRrM2pIeDlKT0xzclYxcUY5RTN0REVoZ0tFRkQ2aHIzYkc1?=
 =?utf-8?B?K0VqMThBaGxub1BtYVVMUU43TUtxNFFxUXhHSDdmZDFFT3NqUko1S2xVYkFj?=
 =?utf-8?B?ZnYzTWE1UHZWZUpmNW5QZ3U1YWxaVmtRWnRIc1RVUjUrS3VPT1VMbnVZUmtl?=
 =?utf-8?B?Z2lHVEpLcmtmNkUrM3NjSkVUWW9MZkRjRFZXTDdTcTBxZWRpdGlVSS9YdEtk?=
 =?utf-8?B?QWh5TmxZc1JNSDNGRUE0Ry8zNGVTTFQyUzY1K04yK25ES3F3NDFJZjd6Qlds?=
 =?utf-8?B?L2F0S1ZCaThWNHJ3bnBEZ1R1SWtUM3hQL1RkcldNZGFyZUY0aWNlNVZLTEwy?=
 =?utf-8?B?MW5YVDFNVEI0SnllU29TOHdYWWRiQk9ETC8rWDVidzFhNUVOZG9aUVI2Vjg1?=
 =?utf-8?B?NTBsU0hqT2Rnc0FzRlBWb3g3SzIvaHlkSXlueGpDc1NxdFp1bytsK0tPdzZa?=
 =?utf-8?B?K0xDUzMzZzdzWXcwa0dRL2E0S3M3aVhjSnVPTkNGWC9MOWJoSk4yNCtMcmhl?=
 =?utf-8?B?eWc2amNkM3BMZEttZm96aFd1K1R2L245cDk2WjllSXNiSFZlMzhMUDJmODVj?=
 =?utf-8?B?V1ByZkVqUHYySmFyd2JWSGhvWm4ya1oyR3dnNEVTYS9oMzl1NzAzUWpDcHNq?=
 =?utf-8?B?THp1aDlTTDZZUFB6Q2lNb2NPUzgzc2lwazZlUHpWZVp1UnhaSVRoSVFseFph?=
 =?utf-8?B?bUMzUllmY2hpSlgxZ0FtdTAvT2FrOGp6TWpob2JqMU5PWUhXZzNBdXFKMjBp?=
 =?utf-8?B?UGdaQTlLbVd2eVZIUG5oTDRhdzdFbnNwNVczWFAycVk4YTI1WWxxMmZwQ2wr?=
 =?utf-8?B?RkY2TDhIT0V3VGNhU202QS9VYWxmODg2R1RqV3Z2M1ZFdW9yTjJhb1R0b2pT?=
 =?utf-8?Q?4o0jqT7q1S+PPXqRF5umN4dda?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 725a71ac-233b-4677-2e8d-08db737d57e6
X-MS-Exchange-CrossTenant-AuthSource: PH7PR11MB5984.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Jun 2023 00:03:58.7066
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: CpY8nbucsh4gnuhI2VtrH8y3Pz2AJyWhB812Rqv447Y/rn6UDa/1mZ/Ktla3iY/QJ3Sc+mqYHwFQ/BvOAdgV+Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB6654
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 6/21/23 20:51, Terry Bowman wrote:
> From: Robert Richter <rrichter@amd.com>
> 
> Now, that the Component Register mappings are stored, use them to
> enable and map the HDM decoder capabilities. The Component Registers
> do not need to be probed again for this, remove probing code.
> 
> The HDM capability applies to Endpoints, USPs and VH Host Bridges. The
> Endpoint's component register mappings are located in the cxlds and
> else in the port's structure. Provide a helper function
> cxl_port_get_comp_map() to locate the mappings depending on the
> component's type.
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>   drivers/cxl/core/hdm.c | 59 +++++++++++++++++++++---------------------
>   1 file changed, 30 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 5abfa9276dac..8dcd9f0b22d8 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -81,26 +81,6 @@ static void parse_hdm_decoder_caps(struct cxl_hdm *cxlhdm)
>   		cxlhdm->interleave_mask |= GENMASK(14, 12);
>   }
>   
> -static int map_hdm_decoder_regs(struct cxl_port *port, void __iomem *crb,
> -				struct cxl_component_regs *regs)
> -{
> -	struct cxl_register_map map = {
> -		.dev = &port->dev,
> -		.resource = port->component_reg_phys,
> -		.base = crb,
> -		.max_size = CXL_COMPONENT_REG_BLOCK_SIZE,
> -	};
> -
> -	cxl_probe_component_regs(&port->dev, crb, &map.component_map);
> -	if (!map.component_map.hdm_decoder.valid) {
> -		dev_dbg(&port->dev, "HDM decoder registers not implemented\n");
> -		/* unique error code to indicate no HDM decoder capability */
> -		return -ENODEV;
> -	}
> -
> -	return cxl_map_component_regs(&map, regs, BIT(CXL_CM_CAP_CAP_ID_HDM));
> -}
> -
>   static bool should_emulate_decoders(struct cxl_endpoint_dvsec_info *info)
>   {
>   	struct cxl_hdm *cxlhdm;
> @@ -145,6 +125,22 @@ static bool should_emulate_decoders(struct cxl_endpoint_dvsec_info *info)
>   	return true;
>   }
>   
> +static struct cxl_register_map *cxl_port_get_comp_map(struct cxl_port *port)
> +{
> +	/*
> +	 * HDM capability applies to Endpoints, USPs and VH Host
> +	 * Bridges. The Endpoint's component register mappings are
> +	 * located in the cxlds.
> +	 */
> +	if (is_cxl_endpoint(port)) {
> +		struct cxl_memdev *memdev = to_cxl_memdev(port->uport_dev);
> +
> +		return &memdev->cxlds->comp_map;
> +	}
> +
> +	return &port->comp_map;
> +}
> +
>   /**
>    * devm_cxl_setup_hdm - map HDM decoder component registers
>    * @port: cxl_port to map
> @@ -155,7 +151,7 @@ struct cxl_hdm *devm_cxl_setup_hdm(struct cxl_port *port,
>   {
>   	struct device *dev = &port->dev;
>   	struct cxl_hdm *cxlhdm;
> -	void __iomem *crb;
> +	struct cxl_register_map *comp_map;
>   	int rc;
>   
>   	cxlhdm = devm_kzalloc(dev, sizeof(*cxlhdm), GFP_KERNEL);
> @@ -164,19 +160,24 @@ struct cxl_hdm *devm_cxl_setup_hdm(struct cxl_port *port,
>   	cxlhdm->port = port;
>   	dev_set_drvdata(dev, cxlhdm);
>   
> -	crb = ioremap(port->component_reg_phys, CXL_COMPONENT_REG_BLOCK_SIZE);
> -	if (!crb && info && info->mem_enabled) {
> -		cxlhdm->decoder_count = info->ranges;
> -		return cxlhdm;
> -	} else if (!crb) {
> +	comp_map = cxl_port_get_comp_map(port);
> +
> +	if (!comp_map->component_map.hdm_decoder.valid) {
> +		dev_dbg(&port->dev, "HDM decoder registers not found\n");
> +		if (info && info->mem_enabled) {
> +			cxlhdm->decoder_count = info->ranges;
> +			return cxlhdm;
> +		}
>   		dev_err(dev, "No component registers mapped\n");
>   		return ERR_PTR(-ENXIO);
>   	}
>   
> -	rc = map_hdm_decoder_regs(port, crb, &cxlhdm->regs);
> -	iounmap(crb);
> -	if (rc)
> +	rc = cxl_map_component_regs(comp_map, &cxlhdm->regs,
> +				    BIT(CXL_CM_CAP_CAP_ID_HDM));
> +	if (rc) {
> +		dev_dbg(dev, "Failed to map HDM capability.\n");
>   		return ERR_PTR(rc);
> +	}
>   
>   	parse_hdm_decoder_caps(cxlhdm);
>   	if (cxlhdm->decoder_count == 0) {