Message-ID: <05058013-0e9d-5422-2e1b-184fffe96503@suse.com>
Date:   Thu, 9 Mar 2023 11:34:13 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH v3 01/28] xhci: Add support to allocate several
 interrupters
Content-Language: en-US
To:     Wesley Cheng <quic_wcheng@quicinc.com>,
        srinivas.kandagatla@linaro.org, mathias.nyman@intel.com,
        perex@perex.cz, broonie@kernel.org, lgirdwood@gmail.com,
        krzysztof.kozlowski+dt@linaro.org, agross@kernel.org,
        Thinh.Nguyen@synopsys.com, bgoswami@quicinc.com,
        andersson@kernel.org, robh+dt@kernel.org,
        gregkh@linuxfoundation.org, tiwai@suse.com
Cc:     linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        alsa-devel@alsa-project.org, devicetree@vger.kernel.org,
        linux-usb@vger.kernel.org, quic_jackp@quicinc.com,
        quic_plai@quicinc.com,
        Mathias Nyman <mathias.nyman@linux.intel.com>
References: <20230308235751.495-1-quic_wcheng@quicinc.com>
 <20230308235751.495-2-quic_wcheng@quicinc.com>
From:   Oliver Neukum <oneukum@suse.com>
In-Reply-To: <20230308235751.495-2-quic_wcheng@quicinc.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?OUh3MldPWS9EakZHTHl1eCszdFdicmdXVGltVmpiM1VhMFVteDFCc1J2SEhI?=
 =?utf-8?B?TkVNNDljN0k3dUJoWHkxa210TWlWNXB4UTBWVGFHcFU0WlRCQW1PZmVacXF5?=
 =?utf-8?B?RUcyU3dnajJDb3RTdzdxQ1BsRnV3d0FLN1ZHSHA5OFYxUHpQdE1Ldk5FbkdC?=
 =?utf-8?B?eGp0WFFEQk1OQjY5YjJvUE50d2FmNmNjWXlHREYycnRWa05QN0dSTFBQT1Fl?=
 =?utf-8?B?S0JJL1YzckRJeTAxUng2cFNPSXNuNjd5aXJvRHUxMlQzcUtxZytqaERUWDBo?=
 =?utf-8?B?RUFUR01qMitJSWVpZFhNOEhidzhaR1ozRCs4SGowTlA1TjYwbG56dnE2R3Vh?=
 =?utf-8?B?RU9KbnpQTjF4ZVpkU1VWNWhCMGJGMkFRQkNyL1BtblNXN0YwYThVemRxY3Zi?=
 =?utf-8?B?Q0dzYkw1a1N5Y29FaXJxMmJYVVpneGRjczdrZXNqblA5Y0xnWHI2YWZPSERR?=
 =?utf-8?B?c1BSQTZMUkJGODltSE9ob2NSS1Z4dDR6NzVMRjhjSVVNR29mN0hMNXNVU3dW?=
 =?utf-8?B?ejRTTFRldmhsRzh6dit2c2RLenErQlkzRmp2TVB6TVlVNjQwSG53OW9EckI4?=
 =?utf-8?B?czlGcWdsdCs2dDl6TFNOeFMxazBoQkdPNDloRUg1Tlc3aDBFbmJHWUFUbi9v?=
 =?utf-8?B?TENrVUgvcWJJRXkwa0E1cW83WXA1Qm9MRHZaa3Z1V2JMeWJocGptTnc3eWVH?=
 =?utf-8?B?UHJSZ0tzNGNMSU5mMlZRMlFNUUNlbUp1aWxvUmt2V0RaMmIyaytMWmJodFVV?=
 =?utf-8?B?VnRQcTFFT1JVMDlBOW9haUNpOFIzKzQ1RTNIemM0UU9iMVlLMnFndWVGSVY2?=
 =?utf-8?B?TzYybzVnbmZpRFZWZ3NWU3lxYjYxekc5VnlBWHl3VEJrK0F1TktMais5Y1U2?=
 =?utf-8?B?SW5IK21LemRseE4xR0FkSXNvSnNBVllLZWxsSktVbFQxcXpOOExiODI4ZXpU?=
 =?utf-8?B?dUZ0MlZVUkY0dm0wR1FKbmVxTVc0RzJLdzNLMGdvYUMvejZJMTBIS2JPdWcr?=
 =?utf-8?B?NDZRemxiUkVTQU1ib2QxSDJWYVl1bjRNaUtlbzRDQ1dEK05kR2d3N0lpRW1P?=
 =?utf-8?B?MWxwNTdoQ0FHVXB1UzFiOGE2Nk5LY2pub1BYK3YrcFdVbWdhckNSOUNvZzlq?=
 =?utf-8?B?TERYV1FONXF4aE9neUlDZTFtVDFlaUV3TVhRSEsydnRzajNzc2xsRHhDNlV4?=
 =?utf-8?B?dHpKTHhWbDZjZ3ZXTm4yUjhSWmRLd3pNY2RlV1BGbmpteFUvdStoTGpMamZk?=
 =?utf-8?B?N0hBUk9FTWFhaW1ZUWNvdzRUeWZZVk5ZckR4OGI3ZXo2NlNWNDFmVy9pWmZu?=
 =?utf-8?B?alZtN2V1Z0MwblJneHVoME4yK09qd29RT2tXNHB1U2taM3pNamxqYzdhdGdJ?=
 =?utf-8?B?a2NFblRPbHB5ZXNydU9XL2NTZ0sxQXpGL0FxOHkvTlhFdTg3NzJVbEU0elli?=
 =?utf-8?B?Vkcrbyt1bXNEcEtuRU92WlRiWmVncXZRVkdYdkt4OWp3YVYrNFdHZjZPaW5a?=
 =?utf-8?B?d21qY2FIQThaNVdVcjFTMUlQY3k0T29OOEdxcmRzZmdod0VjU0plUkFSbFg0?=
 =?utf-8?B?MlplM2VBa0MwRW9rSE15RS9BdnpDa1F0R2UwVlFXYnl6UWY2elhKVG1tUllm?=
 =?utf-8?B?UTRacUtmNnJpaFU1MTFhR0RaMndPSjBOdlZnWkxWcElPNFhpdVNkM2NOZGd0?=
 =?utf-8?B?bk5salZ3K2grd0J1QkZGT3FIdHVENkI3dVpZN1RMcVA4ZDdxcjc1SkR3Ynp6?=
 =?utf-8?B?c09tcENoOWN5NDhoYzMxT2IrSm9SSGtid2d0NTRwVWVjZ3BJRWZORlBJK3ZJ?=
 =?utf-8?B?eXVVZzVIbDh6SHpMQlpSNGVabDRqSWJvcm1lRjN0MkNwS2xnRHo1cmp2R2pG?=
 =?utf-8?B?K0tlZkNyaWFHMDE4eDZvL3ZCMlF5bklORGhqTW5MOHE0MUUzaG5zRnhFUUp6?=
 =?utf-8?B?WmdlL0Q0akNWOW01ZFNjNVNWSGxlQnZseXhoS1hHTXZ1UUxIdWo4SzU3R21I?=
 =?utf-8?B?dTU4UjlHbzRFWmZSVXRiWkpET0R6SnFEM1NHandPa09DSDFQMmUrY3ZhUDM0?=
 =?utf-8?B?djBpOFFDL3hJMk9udXVoR0xjZk1VSmRVOWJQUnJ1QXhuemlKa2pqUmRBcFE3?=
 =?utf-8?B?WTJ3bityZkNXYUoxR3RHdXJGTW5pYTlCc0k1Q0NCYkFOdFRrcWZvc3dyZUFF?=
 =?utf-8?Q?OFZ0IJ+yQ4ZmlVIpLhcvSJK6a3YiYsVlxjMgaM/AAQnE?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 43547d43-5fd1-44cc-b853-08db2089d4cb
X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB7104.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Mar 2023 10:34:15.7986
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: PmQxwwGoilmeVo9M1YqHPy3BfXmfdZ+84eCsmKJTi946VEnZ24JKGM6LfzxuO7YxRpikmkKNjFlzc2SCIyJd8A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM9PR04MB8955
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 09.03.23 00:57, Wesley Cheng wrote:

> @@ -1865,8 +1889,15 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
>   
>   	cancel_delayed_work_sync(&xhci->cmd_timer);
>   
> -	xhci_free_interrupter(xhci, xhci->interrupter);
> -	xhci->interrupter = NULL;
> +	for (i = 1; i < xhci->max_interrupters; i++) {
> +		if (xhci->interrupters[i])
> +			xhci_remove_secondary_interrupter(xhci_to_hcd(xhci),
> +							  xhci->interrupters[i]);
> +	}
> +
> +	/* free the primary interrupter, interrupter number 0 */
> +	xhci_free_interrupter(xhci, xhci->interrupters[0]);
> +	xhci->interrupters[0] = NULL;
>   	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed primary event ring");

Any reason you are not just counting down to zero?

> +	if (idx > 0) {
> +		ir = xhci_alloc_interrupter(xhci, idx, GFP_KERNEL);
> +		if (!ir) {
> +			spin_unlock_irqrestore(&xhci->lock, flags);

Why use _irqrestore?

	Regards
		Oliver