From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f169.google.com (mail-pl1-f169.google.com [209.85.214.169]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2380B39099C for ; Mon, 18 May 2026 20:30:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.169 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779136253; cv=none; b=e/5pOUjG+Y16LLCqajEAV/KVOwOq6ZJvQNrQfkVmtHoo8yE0GjtlDE2Fxoq3sq1Y6lYD5tUIF16+sRkoSIhzm213iJjHZQtIGXoPS7qrjnXmKnmY/2f+itVLGq54ee1fkFjuSs1Mc7SaVNZ6XxVUd+JWWngzzxHzVuiVK4eUvz8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779136253; c=relaxed/simple; bh=+AXWZ7VWZu++/J9Ox1DjhblJnRVzqBe9blE5RidmFX0=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=n7Kzz54tF5Jfb3vgRpXrPiZvx2RdSHCSV3LxNdJcOXzHsGBBYoeDs3kvhFARLkMpWF7wBlvXuDnFHAWEKgIG5U1HGSPOgvhbkVoQYWP5SrUCUl86urLPTtBKQTRCDHENtQrLXP/KtgLX9HQdnB+Y7M/k0OwaU8Z/D2rrKBi2I9w= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=JxZfAP/4; arc=none smtp.client-ip=209.85.214.169 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="JxZfAP/4" Received: by mail-pl1-f169.google.com with SMTP id d9443c01a7336-2b941cd869cso16013735ad.1 for ; Mon, 18 May 2026 13:30:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1779136252; x=1779741052; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=DlBfJe+9ZxBm7v8GCX+TON5gbGwp1e0878LuEL7rsgM=; b=JxZfAP/4U5fDo6s60PKFQHaFHBl+YMl8Uninfca6pyHv78ysYZBWfH+or94oSdEgZX GZGCn5UyLeefVm2gTHCSvCplLNp1lDG/0dydv9OjH4GOSzYk6xY9vTgPRTIrZLGNlqQB U8xdxdefwUbwh9MAe6PiFVc+0KZitzA4ZyDhFYbFd43cuNCMWFwFn/xUGGoTDd3uCHzQ iqtNJVMWRL5PLHypnsnfdR8EFoKLgodcTRDvRuGLBPxCOkSIGAdF2G7EIvcIpWL8DlxT nc8qgZcW5XQd47jwnb8CQI2W9pfW5ehvCRiVOnGXh0wp583XSswrNEZHH0jUjwnY89JW gAYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779136252; x=1779741052; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=DlBfJe+9ZxBm7v8GCX+TON5gbGwp1e0878LuEL7rsgM=; b=NcBIq7TyyVilztBZJjY2cyy4pqMTLf04XZ2hqUYGzM/GmCA58GV3zGsvm7bVi8k7oG CK7QVlCvmkc7JLbXpw9A0o4N5CZkn9ejlZiRAClfLUoUo2k6XQXf5CmONRHOJD5AoX99 KUhEKWz5RQt1sMeq/B7s4jwkNgn+/fKYrpaP/HTm4So2dR/TSsMRU36lP0uI9PqqElgY AAKEsRmMuEx4fc/XuExPQhz27aAP1pGQ0MAhHmXByQOlWC3hei+Mra/8hHLN50UINznp vVqchdAZj/SpljR6jrfWooz9YY7n61glb0RYnLfM2gBqgCUaC5LUEBkj6prD+aRNUA+Y AXzQ== X-Forwarded-Encrypted: i=1; AFNElJ8feNManePmx9rvdSMurGyZJdvIezCyCPiX5maxY/Yr660DKoyHCWgcmz4eRr1mkKAyTZ9aCOMBDwg=@vger.kernel.org X-Gm-Message-State: AOJu0YylxaE7mT3WZr34Fj9A74Ld4hag8X77kDZdVygN03170QjViI28 Ul2PPOkP/H+WabEtB3e+CG3uBt4dvg2BxOf/rEn5FtrKQ1LhxfF8qCfY X-Gm-Gg: Acq92OHmgHFJgUqFQFtxNROvbHDiy7Z6aZGzzGs8Hbf7DvxCJlTCTleUTIGTMkGDqDy iOIiOV4CEji0gflv+/xmNMq56J72L4hxuRMJJFL6l0OYmuSfQ1Qy2LtUQgETZCKEkuVcIumex0x /i8o6wl81/DOg66GCLq80W4G957yWT4JeapQktXFyhRLQSON7eStpXGAXJGlS5X2UxqB1s53p5J Bk7Shwevc6/JC0fbykbayR/3aQKffuTIu4EF7DSGW+Mezqlyk95QWCqYn8xOi12cs9XrCashalm 4Imb6KAmjpxZS33frdPFJ3tBMPnITEHjhKct4G/U5JjJidwCQF4wn02NrdzajKFvqF/ABXZpC0E ffunYsTKaNviwWFnn84bGA17RWoLRGDN6D/1n+gAiMfuM1JGAnY5QTUEbRxXeuga+qPAZeZF8zs CekijsEpXcarkjE1hl70AHPQbRXDT0R2Y= X-Received: by 2002:a17:902:d511:b0:2b0:6e60:9582 with SMTP id d9443c01a7336-2bd7e8cb160mr173923295ad.18.1779136251551; Mon, 18 May 2026 13:30:51 -0700 (PDT) Received: from [192.168.89.2] ([119.214.48.64]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-2bd5bd60275sm150934905ad.7.2026.05.18.13.30.48 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 18 May 2026 13:30:51 -0700 (PDT) Message-ID: <144ec61c-4cc1-4986-a16c-7c1b99f3a72e@gmail.com> Date: Tue, 19 May 2026 05:30:47 +0900 Precedence: bulk X-Mailing-List: linux-doc@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v6 1/2] usb: xhci-pci: add AMD Promontory 21 PCI glue To: Michal Pecio Cc: Greg Kroah-Hartman , Mathias Nyman , Guenter Roeck , Jonathan Corbet , Shuah Khan , Mario Limonciello , Basavaraj Natikar , linux-usb@vger.kernel.org, linux-hwmon@vger.kernel.org, linux-doc@vger.kernel.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, "Mario Limonciello (AMD)" , Yaroslav Isakov References: <20260517130407.795157-1-hurryman2212@gmail.com> <20260517130407.795157-2-hurryman2212@gmail.com> <20260517232147.34931718.michal.pecio@gmail.com> Content-Language: en-US From: Jihong Min In-Reply-To: <20260517232147.34931718.michal.pecio@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 5/18/26 06:21, Michal Pecio wrote: > Instead of the X86 heuristic, would it be possible to build glue > code if and only if SENSORS_PROM21_XHCI is enabled? > > This seems to work: > > config SENSORS_PROM21_XHCI > tristate "AMD Promontory 21 xHCI temperature sensor" > - depends on USB_XHCI_PCI_PROM21 > + depends on USB_XHCI_PCI > > config USB_XHCI_PCI_PROM21 > tristate > - depends on X86 > depends on USB_XHCI_PCI > - default USB_XHCI_PCI > + default USB_XHCI_PCI if SENSORS_PROM21_XHCI != 'n' > select AUXILIARY_BUS > > I don't know if it's the best way, perhaps it would be preferable for > the hwmon driver to select the glue, but then I'm not sure how to force > glue to become 'y' when xhci-pci is 'y'. I think I should keep the current hidden glue option for now. The PROM21 PCI glue is part of the PCI binding path for the xHCI controller when enabled, while SENSORS_PROM21_XHCI is only the optional user-visible hwmon driver. Tying the glue to the hwmon option would make the sensor option affect which driver binds the USB controller. As Guenter pointed out, that would be too strong; the USB controller should not depend on whether the optional hwmon driver is enabled. So I would prefer to keep USB_XHCI_PCI_PROM21 as hidden plumbing that follows USB_XHCI_PCI, and keep SENSORS_PROM21_XHCI as the user-visible sensor option. > +static int prom21_xhci_create_auxdev(struct pci_dev *pdev) > +{ > + struct prom21_xhci_auxdev *prom21_auxdev; > + struct usb_hcd *hcd = pci_get_drvdata(pdev); > + > + if (!hcd) > + return -ENODEV; > > Shouldn't be necessary after successful xhci_pci_common_probe(). Agreed. I removed the unnecessary NULL check from prom21_xhci_create_auxdev() locally for v7. > + prom21_auxdev->id = ida_alloc(&prom21_xhci_auxdev_ida, GFP_KERNEL); > + if (prom21_auxdev->id < 0) { > + int ret = prom21_auxdev->id; > + > + devres_free(prom21_auxdev); > + return ret; > + } > + > + prom21_auxdev->auxdev = auxiliary_device_create(&pdev->dev, > + KBUILD_MODNAME, "hwmon", > + &prom21_auxdev->pdata, > + prom21_auxdev->id); > + if (!prom21_auxdev->auxdev) { > + ida_free(&prom21_xhci_auxdev_ida, prom21_auxdev->id); > + devres_free(prom21_auxdev); > + return -ENOMEM; > > The usual "goto error" pattern could be used instead of increasingly > long sequences of xxx_free() calls. Agreed. I changed prom21_xhci_create_auxdev() to use a goto-based cleanup path locally for v7. > It seems that these three functions above are everything that you truly > want to add; the rest is boilerplate required by this two-module scheme > to work, plus ID tables which must be duplicated and kept in sync. > > I wonder if a separate module is really justified, as opposed to simply > linking this file into xhci_pci.ko when directed by Kconfig. > > The downside would be slightly higher memory usage on systems where the > hwmon driver is enabled but not needed. OTOH, same systems would likely > see reduced disk waste. I understand the concern. Linking the PROM21 auxiliary-device publisher into xhci_pci.ko would reduce some boilerplate and avoid the extra PCI driver, while still keeping the hwmon driver itself separate. The reason I kept the current split is that the earlier review direction was to keep the hwmon functionality out of xhci-pci and bind a drivers/hwmon driver through an auxiliary device. The current PROM21 PCI glue keeps the PROM21-specific auxiliary-device lifetime handling outside the common xhci-pci driver and leaves xhci-pci.c with only the PCI ID handoff, similar in spirit to the Renesas handoff path. That said, I agree this is a tradeoff. If Mathias or the USB maintainers prefer the PROM21 auxiliary-device publisher to be linked into xhci_pci.ko instead of being a separate PCI glue driver, I can rework it in that direction while still keeping the hwmon driver under drivers/hwmon and bound through the auxiliary bus. Sincerely, Jihong Min