Message ID | 20230518092240.8023-1-zhuyinbo@loongson.cn |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp365603vqo; Thu, 18 May 2023 02:50:46 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6Xq5PnoZroQnOU3d4J8Vb4HJRsCXiEt3li0tLNEzt4k3ADTNQlpG+m3NaCn6FkwrDf5b6Y X-Received: by 2002:a05:6a00:2287:b0:64b:205:dbf3 with SMTP id f7-20020a056a00228700b0064b0205dbf3mr3761109pfe.34.1684403445719; Thu, 18 May 2023 02:50:45 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1684403445; cv=none; d=google.com; s=arc-20160816; b=ASoPtNvTOh2gL8SyEGydmhTD69YETcyFeZviISGUzIyMKgtEDfBtQV1pLGnXQBSbbz gp2XkTOmdljioaerdf4bjDfczR026nrdkWGUvzTTXw+vr0jCMqNjz6yckhqo+EEpz+Tk GUlYhYAQ09UpjkqFvs2Kn2PwcfhcEZXDEtxDKOYy8NENwekMrXVzCPYcRQzs37mfrzqq WJoL5D7he2Mghw6f7p62ImORSizOExySJfGV5SZ6ltUokLd50De/V9SYybKtXQ9Wnmct 0ER+ZoRPaN1d9VX2/fDx4adl9w9usE8TYkWqLmpxSjLph36aWO4YrDumfWKrHQrEKCxP xL8A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from; bh=3bJ/Lps5SnwpwF3pIGma9IgI2go2apIyEi+ne9khHTk=; b=TPjYEOG5JHP6QVgUJdausvRjfvdytHlxCKdcnK67DxUcNLN5CnRRpa/96DnDEGRfnl OzAjgMVdL2HKP/a9dM8jrC6romG8JYcK3bv/du5RPt02JJfZU+NnVPsVXiZHuz0SYOJO BsPoEowBn2/9VtEajjRO9H31dkj6UEzhePLp014WEX2VmThokQHc0sXxDqGKD9rQzeDF jWsg41LSAAL73Zw2Xzt7rGlgpCZVO5Kgl9q/CMD1RUmu7WcTcTJ36jfPa4QPZZLBcpTm Odl4qDULhTy3uWufNpOypWcaHAqX6G5wmzxAGEbSBVFGwXH3xbyuAqroOdel/VvOK+8X /AQw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id y2-20020aa793c2000000b0064667c16883si1259933pff.100.2023.05.18.02.50.29; Thu, 18 May 2023 02:50:45 -0700 (PDT) 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; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230141AbjERJWv (ORCPT <rfc822;abdi.embedded@gmail.com> + 99 others); Thu, 18 May 2023 05:22:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49100 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229810AbjERJWu (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 18 May 2023 05:22:50 -0400 Received: from loongson.cn (mail.loongson.cn [114.242.206.163]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id A1378211B; Thu, 18 May 2023 02:22:47 -0700 (PDT) Received: from loongson.cn (unknown [10.20.42.35]) by gateway (Coremail) with SMTP id _____8BxRPBm7mVkM84JAA--.17280S3; Thu, 18 May 2023 17:22:46 +0800 (CST) Received: from user-pc.202.106.0.20 (unknown [10.20.42.35]) by localhost.localdomain (Coremail) with SMTP id AQAAf8BxWdJi7mVkSKFnAA--.45037S2; Thu, 18 May 2023 17:22:45 +0800 (CST) From: Yinbo Zhu <zhuyinbo@loongson.cn> To: Minas Harutyunyan <hminas@synopsys.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Bjorn Helgaas <bhelgaas@google.com>, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org Cc: Jianmin Lv <lvjianmin@loongson.cn>, wanghongliang@loongson.cn, Liu Peibao <liupeibao@loongson.cn>, loongson-kernel@lists.loongnix.cn, Yinbo Zhu <zhuyinbo@loongson.cn> Subject: [PATCH v1] usb: dwc2: add pci_device_id driver_data parse support Date: Thu, 18 May 2023 17:22:40 +0800 Message-Id: <20230518092240.8023-1-zhuyinbo@loongson.cn> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-CM-TRANSID: AQAAf8BxWdJi7mVkSKFnAA--.45037S2 X-CM-SenderInfo: 52kx5xhqerqz5rrqw2lrqou0/ X-Coremail-Antispam: 1Uk129KBjvJXoWxur4xWF13GFWrZry7Kr13XFb_yoW7Jr1DpF ZrZFW0yrWktFsxCw13CF4UAFy5Zan7J34UCa47Kw1S9FZ7Ar4rXF1jkr45Cr93t390ga12 vF17tw48CF47J37anT9S1TB71UUUUUJqnTZGkaVYY2UrUUUUj1kv1TuYvTs0mT0YCTnIWj qI5I8CrVACY4xI64kE6c02F40Ex7xfYxn0WfASr-VFAUDa7-sFnT9fnUUIcSsGvfJTRUUU bVAFc2x0x2IEx4CE42xK8VAvwI8IcIk0rVWrJVCq3wAFIxvE14AKwVWUXVWUAwA2ocxC64 kIII0Yj41l84x0c7CEw4AK67xGY2AK021l84ACjcxK6xIIjxv20xvE14v26r4j6ryUM28E F7xvwVC0I7IYx2IY6xkF7I0E14v26r4j6F4UM28EF7xvwVC2z280aVAFwI0_Gr1j6F4UJw A2z4x0Y4vEx4A2jsIEc7CjxVAFwI0_Cr1j6rxdM2kKe7AKxVWUXVWUAwAS0I0E0xvYzxvE 52x082IY62kv0487Mc804VCY07AIYIkI8VC2zVCFFI0UMc02F40EFcxC0VAKzVAqx4xG6I 80ewAv7VC0I7IYx2IY67AKxVWUXVWUAwAv7VC2z280aVAFwI0_Gr0_Cr1lOx8S6xCaFVCj c4AY6r1j6r4UM4x0Y48IcxkI7VAKI48JMxAIw28IcxkI7VAKI48JMxAIw28IcVCjz48v1s IEY20_WwCFx2IqxVCFs4IE7xkEbVWUJVW8JwCFI7km07C267AKxVWUXVWUAwC20s026c02 F40E14v26r1j6r18MI8I3I0E7480Y4vE14v26r106r1rMI8E67AF67kF1VAFwI0_Jw0_GF ylIxkGc2Ij64vIr41lIxAIcVC0I7IYx2IY67AKxVWUJVWUCwCI42IY6xIIjxv20xvEc7Cj xVAFwI0_Jr0_Gr1lIxAIcVCF04k26cxKx2IYs7xG6r1j6r1xMIIF0xvEx4A2jsIE14v26r 4j6F4UMIIF0xvEx4A2jsIEc7CjxVAFwI0_Gr0_Gr1UYxBIdaVFxhVjvjDU0xZFpf9x07j5 HUDUUUUU= X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS, SPF_PASS,T_SCC_BODY_TEXT_LINE 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 X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1766225027313153858?= X-GMAIL-MSGID: =?utf-8?q?1766225027313153858?= |
Series |
[v1] usb: dwc2: add pci_device_id driver_data parse support
|
|
Commit Message
Yinbo Zhu
May 18, 2023, 9:22 a.m. UTC
The dwc2 driver has everything we need to run in PCI mode except
for pci_device_id driver_data parse. With that to set Loongson
dwc2 element and added identified as PCI_VENDOR_ID_LOONGSON
and PCI_DEVICE_ID_LOONGSON_DWC2 in dwc2_pci_ids, the Loongson
dwc2 controller will work.
Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
---
drivers/usb/dwc2/core.h | 1 +
drivers/usb/dwc2/params.c | 33 +++++++++++++++++++++++++++++++--
drivers/usb/dwc2/pci.c | 14 +-------------
include/linux/pci_ids.h | 2 ++
4 files changed, 35 insertions(+), 15 deletions(-)
Comments
On Thu, May 18, 2023 at 05:22:40PM +0800, Yinbo Zhu wrote: > The dwc2 driver has everything we need to run in PCI mode except > for pci_device_id driver_data parse. With that to set Loongson > dwc2 element and added identified as PCI_VENDOR_ID_LOONGSON > and PCI_DEVICE_ID_LOONGSON_DWC2 in dwc2_pci_ids, the Loongson > dwc2 controller will work. > > Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn> > --- > drivers/usb/dwc2/core.h | 1 + > drivers/usb/dwc2/params.c | 33 +++++++++++++++++++++++++++++++-- > drivers/usb/dwc2/pci.c | 14 +------------- > include/linux/pci_ids.h | 2 ++ > 4 files changed, 35 insertions(+), 15 deletions(-) > > diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h > index 0bb4c0c845bf..c92a1da46a01 100644 > --- a/drivers/usb/dwc2/core.h > +++ b/drivers/usb/dwc2/core.h > @@ -1330,6 +1330,7 @@ irqreturn_t dwc2_handle_common_intr(int irq, void *dev); > /* The device ID match table */ > extern const struct of_device_id dwc2_of_match_table[]; > extern const struct acpi_device_id dwc2_acpi_match[]; > +extern const struct pci_device_id dwc2_pci_ids[]; > > int dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg); > int dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg); > diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c > index 21d16533bd2f..f7550d293c2d 100644 > --- a/drivers/usb/dwc2/params.c > +++ b/drivers/usb/dwc2/params.c > @@ -7,6 +7,8 @@ > #include <linux/module.h> > #include <linux/of_device.h> > #include <linux/usb/of.h> > +#include <linux/pci_ids.h> > +#include <linux/pci.h> > > #include "core.h" > > @@ -55,6 +57,14 @@ static void dwc2_set_jz4775_params(struct dwc2_hsotg *hsotg) > !device_property_read_bool(hsotg->dev, "disable-over-current"); > } > > +static void dwc2_set_loongson_params(struct dwc2_hsotg *hsotg) > +{ > + struct dwc2_core_params *p = &hsotg->params; > + > + p->phy_utmi_width = 8; > + p->power_down = DWC2_POWER_DOWN_PARAM_NONE; > +} > + > static void dwc2_set_x1600_params(struct dwc2_hsotg *hsotg) > { > struct dwc2_core_params *p = &hsotg->params; > @@ -281,6 +291,22 @@ const struct acpi_device_id dwc2_acpi_match[] = { > }; > MODULE_DEVICE_TABLE(acpi, dwc2_acpi_match); > > +const struct pci_device_id dwc2_pci_ids[] = { > + { > + PCI_DEVICE(PCI_VENDOR_ID_SYNOPSYS, PCI_PRODUCT_ID_HAPS_HSOTG), > + }, > + { > + PCI_DEVICE(PCI_VENDOR_ID_STMICRO, > + PCI_DEVICE_ID_STMICRO_USB_OTG), > + }, > + { > + PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, PCI_DEVICE_ID_LOONGSON_DWC2), > + .driver_data = (unsigned long)dwc2_set_loongson_params, > + }, > + { /* end: all zeroes */ } > +}; > +MODULE_DEVICE_TABLE(pci, dwc2_pci_ids); > + > static void dwc2_set_param_otg_cap(struct dwc2_hsotg *hsotg) > { > switch (hsotg->hw_params.op_mode) { > @@ -929,10 +955,13 @@ int dwc2_init_params(struct dwc2_hsotg *hsotg) > set_params(hsotg); > } else { > const struct acpi_device_id *amatch; > + const struct pci_device_id *pmatch; > > amatch = acpi_match_device(dwc2_acpi_match, hsotg->dev); > - if (amatch && amatch->driver_data) { > - set_params = (set_params_cb)amatch->driver_data; > + pmatch = pci_match_id(dwc2_pci_ids, to_pci_dev(hsotg->dev->parent)); Ick, this means this is not a "real" PCI driver, right? Why not? Please tie into the PCI device probe call, don't walk all PCI devices like this. How are you _sure_ that the parent is really a PCI device? That is very very fragile and will break. Do this properly instead. > + > + if ((amatch && amatch->driver_data) || (pmatch && pmatch->driver_data)) { > + set_params = (set_params_cb)pmatch->driver_data; > set_params(hsotg); > } > } > diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c > index b7306ed8be4c..f3a1e4232a31 100644 > --- a/drivers/usb/dwc2/pci.c > +++ b/drivers/usb/dwc2/pci.c > @@ -24,7 +24,7 @@ > #include <linux/platform_device.h> > #include <linux/usb/usb_phy_generic.h> > > -#define PCI_PRODUCT_ID_HAPS_HSOTG 0xabc0 > +#include "core.h" > > static const char dwc2_driver_name[] = "dwc2-pci"; > > @@ -122,18 +122,6 @@ static int dwc2_pci_probe(struct pci_dev *pci, > return ret; > } > > -static const struct pci_device_id dwc2_pci_ids[] = { > - { > - PCI_DEVICE(PCI_VENDOR_ID_SYNOPSYS, PCI_PRODUCT_ID_HAPS_HSOTG), > - }, > - { > - PCI_DEVICE(PCI_VENDOR_ID_STMICRO, > - PCI_DEVICE_ID_STMICRO_USB_OTG), > - }, > - { /* end: all zeroes */ } > -}; > -MODULE_DEVICE_TABLE(pci, dwc2_pci_ids); > - > static struct pci_driver dwc2_pci_driver = { > .name = dwc2_driver_name, > .id_table = dwc2_pci_ids, > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h > index e43ab203054a..6481f648695a 100644 > --- a/include/linux/pci_ids.h > +++ b/include/linux/pci_ids.h > @@ -157,6 +157,7 @@ > #define PCI_VENDOR_ID_PCI_SIG 0x0001 > > #define PCI_VENDOR_ID_LOONGSON 0x0014 > +#define PCI_DEVICE_ID_LOONGSON_DWC2 0x7a04 > > #define PCI_VENDOR_ID_SOLIDIGM 0x025e > > @@ -2356,6 +2357,7 @@ > #define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3_AXI 0xabce > #define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB31 0xabcf > #define PCI_DEVICE_ID_SYNOPSYS_EDDA 0xedda > +#define PCI_PRODUCT_ID_HAPS_HSOTG 0xabc0 Please read the top of this file for why you should not add new ids here. thanks, greg k-h
Hi Yinbo,
kernel test robot noticed the following build errors:
[auto build test ERROR on pci/next]
[also build test ERROR on pci/for-linus westeri-thunderbolt/next linus/master v6.4-rc2 next-20230518]
[cannot apply to usb/usb-testing usb/usb-next usb/usb-linus]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Yinbo-Zhu/usb-dwc2-add-pci_device_id-driver_data-parse-support/20230518-173721
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/r/20230518092240.8023-1-zhuyinbo%40loongson.cn
patch subject: [PATCH v1] usb: dwc2: add pci_device_id driver_data parse support
config: powerpc-allmodconfig
compiler: powerpc-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/3ff56448e1442fe8b1e72651a8d4d6e1086ece32
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Yinbo-Zhu/usb-dwc2-add-pci_device_id-driver_data-parse-support/20230518-173721
git checkout 3ff56448e1442fe8b1e72651a8d4d6e1086ece32
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305190105.O6ycxCti-lkp@intel.com/
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "dwc2_pci_ids" [drivers/usb/dwc2/dwc2_pci.ko] undefined!
在 2023/5/19 上午1:52, kernel test robot 写道: > Hi Yinbo, > > kernel test robot noticed the following build errors: > > [auto build test ERROR on pci/next] > [also build test ERROR on pci/for-linus westeri-thunderbolt/next linus/master v6.4-rc2 next-20230518] > [cannot apply to usb/usb-testing usb/usb-next usb/usb-linus] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: https://github.com/intel-lab-lkp/linux/commits/Yinbo-Zhu/usb-dwc2-add-pci_device_id-driver_data-parse-support/20230518-173721 > base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next > patch link: https://lore.kernel.org/r/20230518092240.8023-1-zhuyinbo%40loongson.cn > patch subject: [PATCH v1] usb: dwc2: add pci_device_id driver_data parse support > config: powerpc-allmodconfig > compiler: powerpc-linux-gcc (GCC) 12.1.0 > reproduce (this is a W=1 build): > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # https://github.com/intel-lab-lkp/linux/commit/3ff56448e1442fe8b1e72651a8d4d6e1086ece32 > git remote add linux-review https://github.com/intel-lab-lkp/linux > git fetch --no-tags linux-review Yinbo-Zhu/usb-dwc2-add-pci_device_id-driver_data-parse-support/20230518-173721 > git checkout 3ff56448e1442fe8b1e72651a8d4d6e1086ece32 > # save the config file > mkdir build_dir && cp config build_dir/.config > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc olddefconfig > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash > > If you fix the issue, kindly add following tag where applicable > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202305190105.O6ycxCti-lkp@intel.com/ > > All errors (new ones prefixed by >>, old ones prefixed by <<): > >>> ERROR: modpost: "dwc2_pci_ids" [drivers/usb/dwc2/dwc2_pci.ko] undefined! I test it was set dwc2 pci driver as built-in, so no error, this compile error was that dwc2_pci_ids not export when driver as module and I will add EXPORT_SYMBOL_GPL(dwc2_pci_ids) to fix that compile issue. Thanks. >
On Fri, May 19, 2023 at 03:13:20PM +0800, zhuyinbo wrote: > > > 在 2023/5/19 上午1:52, kernel test robot 写道: > > Hi Yinbo, > > > > kernel test robot noticed the following build errors: > > > > [auto build test ERROR on pci/next] > > [also build test ERROR on pci/for-linus westeri-thunderbolt/next linus/master v6.4-rc2 next-20230518] > > [cannot apply to usb/usb-testing usb/usb-next usb/usb-linus] > > [If your patch is applied to the wrong git tree, kindly drop us a note. > > And when submitting patch, we suggest to use '--base' as documented in > > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > > > url: https://github.com/intel-lab-lkp/linux/commits/Yinbo-Zhu/usb-dwc2-add-pci_device_id-driver_data-parse-support/20230518-173721 > > base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next > > patch link: https://lore.kernel.org/r/20230518092240.8023-1-zhuyinbo%40loongson.cn > > patch subject: [PATCH v1] usb: dwc2: add pci_device_id driver_data parse support > > config: powerpc-allmodconfig > > compiler: powerpc-linux-gcc (GCC) 12.1.0 > > reproduce (this is a W=1 build): > > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > > chmod +x ~/bin/make.cross > > # https://github.com/intel-lab-lkp/linux/commit/3ff56448e1442fe8b1e72651a8d4d6e1086ece32 > > git remote add linux-review https://github.com/intel-lab-lkp/linux > > git fetch --no-tags linux-review Yinbo-Zhu/usb-dwc2-add-pci_device_id-driver_data-parse-support/20230518-173721 > > git checkout 3ff56448e1442fe8b1e72651a8d4d6e1086ece32 > > # save the config file > > mkdir build_dir && cp config build_dir/.config > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc olddefconfig > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash > > > > If you fix the issue, kindly add following tag where applicable > > | Reported-by: kernel test robot <lkp@intel.com> > > | Closes: https://lore.kernel.org/oe-kbuild-all/202305190105.O6ycxCti-lkp@intel.com/ > > > > All errors (new ones prefixed by >>, old ones prefixed by <<): > > > > > > ERROR: modpost: "dwc2_pci_ids" [drivers/usb/dwc2/dwc2_pci.ko] undefined! > > > I test it was set dwc2 pci driver as built-in, so no error, this compile > error was that dwc2_pci_ids not export when driver as module and I will > add EXPORT_SYMBOL_GPL(dwc2_pci_ids) to fix that compile issue. Again, no, please do this properly, no one should ever be walking a pci id list by hand like this... thanks, greg k-h
Hi greg k-h, I'm sorry for giving you feedback so late, for your suggestion that I have a some analysis. 2023 May 18, 2023 at 6:32 PM, Greg Kroah-Hartman wrote: > On Thu, May 18, 2023 at 05:22:40PM +0800, Yinbo Zhu wrote: >> The dwc2 driver has everything we need to run in PCI mode except >> for pci_device_id driver_data parse. With that to set Loongson >> dwc2 element and added identified as PCI_VENDOR_ID_LOONGSON >> and PCI_DEVICE_ID_LOONGSON_DWC2 in dwc2_pci_ids, the Loongson >> dwc2 controller will work. >> >> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn> >> --- >> drivers/usb/dwc2/core.h | 1 + >> drivers/usb/dwc2/params.c | 33 +++++++++++++++++++++++++++++++-- >> drivers/usb/dwc2/pci.c | 14 +------------- >> include/linux/pci_ids.h | 2 ++ >> 4 files changed, 35 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h >> index 0bb4c0c845bf..c92a1da46a01 100644 >> --- a/drivers/usb/dwc2/core.h >> +++ b/drivers/usb/dwc2/core.h >> @@ -1330,6 +1330,7 @@ irqreturn_t dwc2_handle_common_intr(int irq, void *dev); >> /* The device ID match table */ >> extern const struct of_device_id dwc2_of_match_table[]; >> extern const struct acpi_device_id dwc2_acpi_match[]; >> +extern const struct pci_device_id dwc2_pci_ids[]; >> >> int dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg); >> int dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg); >> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c >> index 21d16533bd2f..f7550d293c2d 100644 >> --- a/drivers/usb/dwc2/params.c >> +++ b/drivers/usb/dwc2/params.c >> @@ -7,6 +7,8 @@ >> #include <linux/module.h> >> #include <linux/of_device.h> >> #include <linux/usb/of.h> >> +#include <linux/pci_ids.h> >> +#include <linux/pci.h> >> >> #include "core.h" >> >> @@ -55,6 +57,14 @@ static void dwc2_set_jz4775_params(struct dwc2_hsotg *hsotg) >> !device_property_read_bool(hsotg->dev, "disable-over-current"); >> } >> >> +static void dwc2_set_loongson_params(struct dwc2_hsotg *hsotg) >> +{ >> + struct dwc2_core_params *p = &hsotg->params; >> + >> + p->phy_utmi_width = 8; >> + p->power_down = DWC2_POWER_DOWN_PARAM_NONE; >> +} >> + >> static void dwc2_set_x1600_params(struct dwc2_hsotg *hsotg) >> { >> struct dwc2_core_params *p = &hsotg->params; >> @@ -281,6 +291,22 @@ const struct acpi_device_id dwc2_acpi_match[] = { >> }; >> MODULE_DEVICE_TABLE(acpi, dwc2_acpi_match); >> >> +const struct pci_device_id dwc2_pci_ids[] = { >> + { >> + PCI_DEVICE(PCI_VENDOR_ID_SYNOPSYS, PCI_PRODUCT_ID_HAPS_HSOTG), >> + }, >> + { >> + PCI_DEVICE(PCI_VENDOR_ID_STMICRO, >> + PCI_DEVICE_ID_STMICRO_USB_OTG), >> + }, >> + { >> + PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, PCI_DEVICE_ID_LOONGSON_DWC2), >> + .driver_data = (unsigned long)dwc2_set_loongson_params, >> + }, >> + { /* end: all zeroes */ } >> +}; >> +MODULE_DEVICE_TABLE(pci, dwc2_pci_ids); >> + >> static void dwc2_set_param_otg_cap(struct dwc2_hsotg *hsotg) >> { >> switch (hsotg->hw_params.op_mode) { >> @@ -929,10 +955,13 @@ int dwc2_init_params(struct dwc2_hsotg *hsotg) >> set_params(hsotg); >> } else { >> const struct acpi_device_id *amatch; >> + const struct pci_device_id *pmatch; >> >> amatch = acpi_match_device(dwc2_acpi_match, hsotg->dev); >> - if (amatch && amatch->driver_data) { >> - set_params = (set_params_cb)amatch->driver_data; >> + pmatch = pci_match_id(dwc2_pci_ids, to_pci_dev(hsotg->dev->parent)); > > Ick, this means this is not a "real" PCI driver, right? Why not? The params.c and platform.c was a part of pci dwc2 device driver. This pci.c was only play a role that register device resource but not operate dwc2 hardware. in other words, the params.c seems unrelated to the device type. Whether this device is a PCI device, platform device, or PCI device, it is best to use params.c for operational dwc2 elements. Failure to do so seems to break the original design. > > Please tie into the PCI device probe call, don't walk all PCI devices > like this. I learn about that you strongly disagree with using pci_match_id, May I ask you the reason ? actually, I use it was due to I noticed that xhci-pci.c, ehci-pci.c and ohci-pci.c was all use it. and I don't use it in dwc2/pci.c was considering set dwc2 element need dpend on elements.c and platform.c, and usb driver (ohci,echi,xhci) was a relatively indepent device driver when to operate usb controler. but dwc2 was not. If I fource the element setting of dwc2 element in dwc2/pci.c. It will be following case. This will cause problems with element-initial function or element-check function. 1. initial dwc2 element. 2. check the setting of dwc2 element whether was suitable 3. set dwc2 element. or 1. set dwc2 element. 2. initial dwc2 element. 3. check the setting of dwc2 element whether was suitable The corresponding code call process as follows: 1. dwc2_set_default_params(hsotg); 2. dwc2_get_device_properties(hsotg); 3. dwc2_check_params(hsotg); 4. dwc2_set_loongson_params; or 1. dwc2_set_loongson_params; 2. dwc2_set_default_params(hsotg); 3. dwc2_get_device_properties(hsotg); 4. dwc2_check_params(hsotg); But the platform dwc2 device or acpi dwc2 device was all following case and It seems was correct order. 1. dwc2_set_default_params(hsotg); 2. dwc2_get_device_properties(hsotg); 3. dwc2_set_loongson_params; 4. dwc2_check_params(hsotg); > > How are you _sure_ that the parent is really a PCI device? That is very > very fragile and will break. > > Do this properly instead. Thank you for your reminder. There was indeed an issue with my previous code, and the modified code is as follows, then it seems to ensure that device is a PCI device. @@ -927,13 +954,20 @@ int dwc2_init_params(struct dwc2_hsotg *hsotg) if (match && match->data) { set_params = match->data; set_params(hsotg); - } else { + } else if (!match) { const struct acpi_device_id *amatch; + const struct pci_device_id *pmatch = NULL; amatch = acpi_match_device(dwc2_acpi_match, hsotg->dev); if (amatch && amatch->driver_data) { set_params = (set_params_cb)amatch->driver_data; set_params(hsotg); + } else if (!amatch) + pmatch = pci_match_id(dwc2_pci_ids, to_pci_dev(hsotg->dev->parent)); + + if (pmatch && pmatch->driver_data) { + set_params = (set_params_cb)pmatch->driver_data; + set_params(hsotg); } > > >> + >> + if ((amatch && amatch->driver_data) || (pmatch && pmatch->driver_data)) { >> + set_params = (set_params_cb)pmatch->driver_data; >> set_params(hsotg); >> } >> } >> diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c >> index b7306ed8be4c..f3a1e4232a31 100644 >> --- a/drivers/usb/dwc2/pci.c >> +++ b/drivers/usb/dwc2/pci.c >> @@ -24,7 +24,7 @@ >> #include <linux/platform_device.h> >> #include <linux/usb/usb_phy_generic.h> >> >> -#define PCI_PRODUCT_ID_HAPS_HSOTG 0xabc0 >> +#include "core.h" >> >> static const char dwc2_driver_name[] = "dwc2-pci"; >> >> @@ -122,18 +122,6 @@ static int dwc2_pci_probe(struct pci_dev *pci, >> return ret; >> } >> >> -static const struct pci_device_id dwc2_pci_ids[] = { >> - { >> - PCI_DEVICE(PCI_VENDOR_ID_SYNOPSYS, PCI_PRODUCT_ID_HAPS_HSOTG), >> - }, >> - { >> - PCI_DEVICE(PCI_VENDOR_ID_STMICRO, >> - PCI_DEVICE_ID_STMICRO_USB_OTG), >> - }, >> - { /* end: all zeroes */ } >> -}; >> -MODULE_DEVICE_TABLE(pci, dwc2_pci_ids); >> - >> static struct pci_driver dwc2_pci_driver = { >> .name = dwc2_driver_name, >> .id_table = dwc2_pci_ids, >> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h >> index e43ab203054a..6481f648695a 100644 >> --- a/include/linux/pci_ids.h >> +++ b/include/linux/pci_ids.h >> @@ -157,6 +157,7 @@ >> #define PCI_VENDOR_ID_PCI_SIG 0x0001 >> >> #define PCI_VENDOR_ID_LOONGSON 0x0014 >> +#define PCI_DEVICE_ID_LOONGSON_DWC2 0x7a04 >> >> #define PCI_VENDOR_ID_SOLIDIGM 0x025e >> >> @@ -2356,6 +2357,7 @@ >> #define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3_AXI 0xabce >> #define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB31 0xabcf >> #define PCI_DEVICE_ID_SYNOPSYS_EDDA 0xedda >> +#define PCI_PRODUCT_ID_HAPS_HSOTG 0xabc0 > > Please read the top of this file for why you should not add new ids > here. okay, I will remove it. Thanks! Yinbo.
在 2023/5/19 下午5:49, Greg Kroah-Hartman 写道: > On Fri, May 19, 2023 at 03:13:20PM +0800, zhuyinbo wrote: >> >> >> 在 2023/5/19 上午1:52, kernel test robot 写道: >>> Hi Yinbo, >>> >>> kernel test robot noticed the following build errors: >>> >>> [auto build test ERROR on pci/next] >>> [also build test ERROR on pci/for-linus westeri-thunderbolt/next linus/master v6.4-rc2 next-20230518] >>> [cannot apply to usb/usb-testing usb/usb-next usb/usb-linus] >>> [If your patch is applied to the wrong git tree, kindly drop us a note. >>> And when submitting patch, we suggest to use '--base' as documented in >>> https://git-scm.com/docs/git-format-patch#_base_tree_information] >>> >>> url: https://github.com/intel-lab-lkp/linux/commits/Yinbo-Zhu/usb-dwc2-add-pci_device_id-driver_data-parse-support/20230518-173721 >>> base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next >>> patch link: https://lore.kernel.org/r/20230518092240.8023-1-zhuyinbo%40loongson.cn >>> patch subject: [PATCH v1] usb: dwc2: add pci_device_id driver_data parse support >>> config: powerpc-allmodconfig >>> compiler: powerpc-linux-gcc (GCC) 12.1.0 >>> reproduce (this is a W=1 build): >>> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross >>> chmod +x ~/bin/make.cross >>> # https://github.com/intel-lab-lkp/linux/commit/3ff56448e1442fe8b1e72651a8d4d6e1086ece32 >>> git remote add linux-review https://github.com/intel-lab-lkp/linux >>> git fetch --no-tags linux-review Yinbo-Zhu/usb-dwc2-add-pci_device_id-driver_data-parse-support/20230518-173721 >>> git checkout 3ff56448e1442fe8b1e72651a8d4d6e1086ece32 >>> # save the config file >>> mkdir build_dir && cp config build_dir/.config >>> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc olddefconfig >>> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash >>> >>> If you fix the issue, kindly add following tag where applicable >>> | Reported-by: kernel test robot <lkp@intel.com> >>> | Closes: https://lore.kernel.org/oe-kbuild-all/202305190105.O6ycxCti-lkp@intel.com/ >>> >>> All errors (new ones prefixed by >>, old ones prefixed by <<): >>> >>>>> ERROR: modpost: "dwc2_pci_ids" [drivers/usb/dwc2/dwc2_pci.ko] undefined! >> >> >> I test it was set dwc2 pci driver as built-in, so no error, this compile >> error was that dwc2_pci_ids not export when driver as module and I will >> add EXPORT_SYMBOL_GPL(dwc2_pci_ids) to fix that compile issue. > > Again, no, please do this properly, no one should ever be walking a pci > id list by hand like this... okay, I got it. But I don't seem to have found a good way to set dwc2 elements yet for pci device, in addition, I have some alalysis in another mail loop. please you check in your free time. Thanks! Yinbo.
Friendly ping ? 在 2023/5/20 上午11:24, zhuyinbo 写道: > > Hi greg k-h, > > I'm sorry for giving you feedback so late, for your suggestion that I > have a some analysis. > > 2023 May 18, 2023 at 6:32 PM, Greg Kroah-Hartman wrote: >> On Thu, May 18, 2023 at 05:22:40PM +0800, Yinbo Zhu wrote: >>> The dwc2 driver has everything we need to run in PCI mode except >>> for pci_device_id driver_data parse. With that to set Loongson >>> dwc2 element and added identified as PCI_VENDOR_ID_LOONGSON >>> and PCI_DEVICE_ID_LOONGSON_DWC2 in dwc2_pci_ids, the Loongson >>> dwc2 controller will work. >>> >>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn> >>> --- >>> drivers/usb/dwc2/core.h | 1 + >>> drivers/usb/dwc2/params.c | 33 +++++++++++++++++++++++++++++++-- >>> drivers/usb/dwc2/pci.c | 14 +------------- >>> include/linux/pci_ids.h | 2 ++ >>> 4 files changed, 35 insertions(+), 15 deletions(-) >>> >>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h >>> index 0bb4c0c845bf..c92a1da46a01 100644 >>> --- a/drivers/usb/dwc2/core.h >>> +++ b/drivers/usb/dwc2/core.h >>> @@ -1330,6 +1330,7 @@ irqreturn_t dwc2_handle_common_intr(int irq, >>> void *dev); >>> /* The device ID match table */ >>> extern const struct of_device_id dwc2_of_match_table[]; >>> extern const struct acpi_device_id dwc2_acpi_match[]; >>> +extern const struct pci_device_id dwc2_pci_ids[]; >>> int dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg); >>> int dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg); >>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c >>> index 21d16533bd2f..f7550d293c2d 100644 >>> --- a/drivers/usb/dwc2/params.c >>> +++ b/drivers/usb/dwc2/params.c >>> @@ -7,6 +7,8 @@ >>> #include <linux/module.h> >>> #include <linux/of_device.h> >>> #include <linux/usb/of.h> >>> +#include <linux/pci_ids.h> >>> +#include <linux/pci.h> >>> #include "core.h" >>> @@ -55,6 +57,14 @@ static void dwc2_set_jz4775_params(struct >>> dwc2_hsotg *hsotg) >>> !device_property_read_bool(hsotg->dev, >>> "disable-over-current"); >>> } >>> +static void dwc2_set_loongson_params(struct dwc2_hsotg *hsotg) >>> +{ >>> + struct dwc2_core_params *p = &hsotg->params; >>> + >>> + p->phy_utmi_width = 8; >>> + p->power_down = DWC2_POWER_DOWN_PARAM_NONE; >>> +} >>> + >>> static void dwc2_set_x1600_params(struct dwc2_hsotg *hsotg) >>> { >>> struct dwc2_core_params *p = &hsotg->params; >>> @@ -281,6 +291,22 @@ const struct acpi_device_id dwc2_acpi_match[] = { >>> }; >>> MODULE_DEVICE_TABLE(acpi, dwc2_acpi_match); >>> +const struct pci_device_id dwc2_pci_ids[] = { >>> + { >>> + PCI_DEVICE(PCI_VENDOR_ID_SYNOPSYS, PCI_PRODUCT_ID_HAPS_HSOTG), >>> + }, >>> + { >>> + PCI_DEVICE(PCI_VENDOR_ID_STMICRO, >>> + PCI_DEVICE_ID_STMICRO_USB_OTG), >>> + }, >>> + { >>> + PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, >>> PCI_DEVICE_ID_LOONGSON_DWC2), >>> + .driver_data = (unsigned long)dwc2_set_loongson_params, >>> + }, >>> + { /* end: all zeroes */ } >>> +}; >>> +MODULE_DEVICE_TABLE(pci, dwc2_pci_ids); >>> + >>> static void dwc2_set_param_otg_cap(struct dwc2_hsotg *hsotg) >>> { >>> switch (hsotg->hw_params.op_mode) { >>> @@ -929,10 +955,13 @@ int dwc2_init_params(struct dwc2_hsotg *hsotg) >>> set_params(hsotg); >>> } else { >>> const struct acpi_device_id *amatch; >>> + const struct pci_device_id *pmatch; >>> amatch = acpi_match_device(dwc2_acpi_match, hsotg->dev); >>> - if (amatch && amatch->driver_data) { >>> - set_params = (set_params_cb)amatch->driver_data; >>> + pmatch = pci_match_id(dwc2_pci_ids, >>> to_pci_dev(hsotg->dev->parent)); >> >> Ick, this means this is not a "real" PCI driver, right? Why not? > > > The params.c and platform.c was a part of pci dwc2 device driver. This > pci.c was only play a role that register device resource but not operate > dwc2 hardware. in other words, the params.c seems unrelated to the > device type. Whether this device is a PCI device, platform device, or > PCI device, it is best to use params.c for operational dwc2 elements. > Failure to do so seems to break the original design. > >> >> Please tie into the PCI device probe call, don't walk all PCI devices >> like this. > > > I learn about that you strongly disagree with using pci_match_id, May > I ask you the reason ? actually, I use it was due to I noticed that > xhci-pci.c, ehci-pci.c and ohci-pci.c was all use it. and I don't use it > in dwc2/pci.c was considering set dwc2 element need dpend on elements.c > and platform.c, and usb driver (ohci,echi,xhci) was a relatively > indepent device driver when to operate usb controler. but dwc2 was not. > > If I fource the element setting of dwc2 element in dwc2/pci.c. It will > be following case. This will cause problems with element-initial > function or element-check function. > > 1. initial dwc2 element. > 2. check the setting of dwc2 element whether was suitable > 3. set dwc2 element. > > or > > 1. set dwc2 element. > 2. initial dwc2 element. > 3. check the setting of dwc2 element whether was suitable > > The corresponding code call process as follows: > > 1. dwc2_set_default_params(hsotg); > 2. dwc2_get_device_properties(hsotg); > 3. dwc2_check_params(hsotg); > 4. dwc2_set_loongson_params; > > or > > 1. dwc2_set_loongson_params; > 2. dwc2_set_default_params(hsotg); > 3. dwc2_get_device_properties(hsotg); > 4. dwc2_check_params(hsotg); > > But the platform dwc2 device or acpi dwc2 device was all following case > and It seems was correct order. > > 1. dwc2_set_default_params(hsotg); > 2. dwc2_get_device_properties(hsotg); > 3. dwc2_set_loongson_params; > 4. dwc2_check_params(hsotg); > >> >> How are you _sure_ that the parent is really a PCI device? That is very >> very fragile and will break. >> >> Do this properly instead. > > > Thank you for your reminder. There was indeed an issue with my previous > code, and the modified code is as follows, then it seems to ensure that > device is a PCI device. > > @@ -927,13 +954,20 @@ int dwc2_init_params(struct dwc2_hsotg *hsotg) > if (match && match->data) { > set_params = match->data; > set_params(hsotg); > - } else { > + } else if (!match) { > const struct acpi_device_id *amatch; > + const struct pci_device_id *pmatch = NULL; > > amatch = acpi_match_device(dwc2_acpi_match, hsotg->dev); > if (amatch && amatch->driver_data) { > set_params = (set_params_cb)amatch->driver_data; > set_params(hsotg); > + } else if (!amatch) > + pmatch = pci_match_id(dwc2_pci_ids, > to_pci_dev(hsotg->dev->parent)); > + > + if (pmatch && pmatch->driver_data) { > + set_params = (set_params_cb)pmatch->driver_data; > + set_params(hsotg); > } > >> >> >>> + >>> + if ((amatch && amatch->driver_data) || (pmatch && >>> pmatch->driver_data)) { >>> + set_params = (set_params_cb)pmatch->driver_data; >>> set_params(hsotg); >>> } >>> } >>> diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c >>> index b7306ed8be4c..f3a1e4232a31 100644 >>> --- a/drivers/usb/dwc2/pci.c >>> +++ b/drivers/usb/dwc2/pci.c >>> @@ -24,7 +24,7 @@ >>> #include <linux/platform_device.h> >>> #include <linux/usb/usb_phy_generic.h> >>> -#define PCI_PRODUCT_ID_HAPS_HSOTG 0xabc0 >>> +#include "core.h" >>> static const char dwc2_driver_name[] = "dwc2-pci"; >>> @@ -122,18 +122,6 @@ static int dwc2_pci_probe(struct pci_dev *pci, >>> return ret; >>> } >>> -static const struct pci_device_id dwc2_pci_ids[] = { >>> - { >>> - PCI_DEVICE(PCI_VENDOR_ID_SYNOPSYS, PCI_PRODUCT_ID_HAPS_HSOTG), >>> - }, >>> - { >>> - PCI_DEVICE(PCI_VENDOR_ID_STMICRO, >>> - PCI_DEVICE_ID_STMICRO_USB_OTG), >>> - }, >>> - { /* end: all zeroes */ } >>> -}; >>> -MODULE_DEVICE_TABLE(pci, dwc2_pci_ids); >>> - >>> static struct pci_driver dwc2_pci_driver = { >>> .name = dwc2_driver_name, >>> .id_table = dwc2_pci_ids, >>> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h >>> index e43ab203054a..6481f648695a 100644 >>> --- a/include/linux/pci_ids.h >>> +++ b/include/linux/pci_ids.h >>> @@ -157,6 +157,7 @@ >>> #define PCI_VENDOR_ID_PCI_SIG 0x0001 >>> #define PCI_VENDOR_ID_LOONGSON 0x0014 >>> +#define PCI_DEVICE_ID_LOONGSON_DWC2 0x7a04 >>> #define PCI_VENDOR_ID_SOLIDIGM 0x025e >>> @@ -2356,6 +2357,7 @@ >>> #define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3_AXI 0xabce >>> #define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB31 0xabcf >>> #define PCI_DEVICE_ID_SYNOPSYS_EDDA 0xedda >>> +#define PCI_PRODUCT_ID_HAPS_HSOTG 0xabc0 >> >> Please read the top of this file for why you should not add new ids >> here. > > okay, I will remove it. > > Thanks! > Yinbo.
On Thu, Jun 08, 2023 at 03:54:36PM +0800, zhuyinbo wrote: > > Friendly ping ? For what? I thought a new series was going to be submitted, I don't have anything in my queue to review for this at the moment. thanks, greg k-h
在 2023/6/8 下午4:01, Greg Kroah-Hartman 写道: > On Thu, Jun 08, 2023 at 03:54:36PM +0800, zhuyinbo wrote: >> >> Friendly ping ? > > For what? I thought a new series was going to be submitted, I don't > have anything in my queue to review for this at the moment. I have some comments that need your consent or opinion. such as, I have a explanation why I using pci_match_id() in dwc2/params.c. Do you agree with my explanation ? Thanks, Yinbo
On Thu, Jun 08, 2023 at 04:57:04PM +0800, zhuyinbo wrote: > > > 在 2023/6/8 下午4:01, Greg Kroah-Hartman 写道: > > On Thu, Jun 08, 2023 at 03:54:36PM +0800, zhuyinbo wrote: > > > > > > Friendly ping ? > > > > For what? I thought a new series was going to be submitted, I don't > > have anything in my queue to review for this at the moment. > > > I have some comments that need your consent or opinion. > such as, I have a explanation why I using pci_match_id() in > dwc2/params.c. Do you agree with my explanation ? I have no context here, sorry. Remember, we review hundreds of patches a week, when you cut off all context that makes it impossible to remember. Submit your updated patchset based on what you feel is correct and we can review it from there. thanks, greg k-h
在 2023/6/8 下午5:11, Greg Kroah-Hartman 写道: > On Thu, Jun 08, 2023 at 04:57:04PM +0800, zhuyinbo wrote: >> >> >> 在 2023/6/8 下午4:01, Greg Kroah-Hartman 写道: >>> On Thu, Jun 08, 2023 at 03:54:36PM +0800, zhuyinbo wrote: >>>> >>>> Friendly ping ? >>> >>> For what? I thought a new series was going to be submitted, I don't >>> have anything in my queue to review for this at the moment. >> >> >> I have some comments that need your consent or opinion. >> such as, I have a explanation why I using pci_match_id() in >> dwc2/params.c. Do you agree with my explanation ? > > I have no context here, sorry. Remember, we review hundreds of patches > a week, when you cut off all context that makes it impossible to > remember. > > Submit your updated patchset based on what you feel is correct and we > can review it from there. okay, I got it. Thanks, Yinbo
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 0bb4c0c845bf..c92a1da46a01 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -1330,6 +1330,7 @@ irqreturn_t dwc2_handle_common_intr(int irq, void *dev); /* The device ID match table */ extern const struct of_device_id dwc2_of_match_table[]; extern const struct acpi_device_id dwc2_acpi_match[]; +extern const struct pci_device_id dwc2_pci_ids[]; int dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg); int dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg); diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c index 21d16533bd2f..f7550d293c2d 100644 --- a/drivers/usb/dwc2/params.c +++ b/drivers/usb/dwc2/params.c @@ -7,6 +7,8 @@ #include <linux/module.h> #include <linux/of_device.h> #include <linux/usb/of.h> +#include <linux/pci_ids.h> +#include <linux/pci.h> #include "core.h" @@ -55,6 +57,14 @@ static void dwc2_set_jz4775_params(struct dwc2_hsotg *hsotg) !device_property_read_bool(hsotg->dev, "disable-over-current"); } +static void dwc2_set_loongson_params(struct dwc2_hsotg *hsotg) +{ + struct dwc2_core_params *p = &hsotg->params; + + p->phy_utmi_width = 8; + p->power_down = DWC2_POWER_DOWN_PARAM_NONE; +} + static void dwc2_set_x1600_params(struct dwc2_hsotg *hsotg) { struct dwc2_core_params *p = &hsotg->params; @@ -281,6 +291,22 @@ const struct acpi_device_id dwc2_acpi_match[] = { }; MODULE_DEVICE_TABLE(acpi, dwc2_acpi_match); +const struct pci_device_id dwc2_pci_ids[] = { + { + PCI_DEVICE(PCI_VENDOR_ID_SYNOPSYS, PCI_PRODUCT_ID_HAPS_HSOTG), + }, + { + PCI_DEVICE(PCI_VENDOR_ID_STMICRO, + PCI_DEVICE_ID_STMICRO_USB_OTG), + }, + { + PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, PCI_DEVICE_ID_LOONGSON_DWC2), + .driver_data = (unsigned long)dwc2_set_loongson_params, + }, + { /* end: all zeroes */ } +}; +MODULE_DEVICE_TABLE(pci, dwc2_pci_ids); + static void dwc2_set_param_otg_cap(struct dwc2_hsotg *hsotg) { switch (hsotg->hw_params.op_mode) { @@ -929,10 +955,13 @@ int dwc2_init_params(struct dwc2_hsotg *hsotg) set_params(hsotg); } else { const struct acpi_device_id *amatch; + const struct pci_device_id *pmatch; amatch = acpi_match_device(dwc2_acpi_match, hsotg->dev); - if (amatch && amatch->driver_data) { - set_params = (set_params_cb)amatch->driver_data; + pmatch = pci_match_id(dwc2_pci_ids, to_pci_dev(hsotg->dev->parent)); + + if ((amatch && amatch->driver_data) || (pmatch && pmatch->driver_data)) { + set_params = (set_params_cb)pmatch->driver_data; set_params(hsotg); } } diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c index b7306ed8be4c..f3a1e4232a31 100644 --- a/drivers/usb/dwc2/pci.c +++ b/drivers/usb/dwc2/pci.c @@ -24,7 +24,7 @@ #include <linux/platform_device.h> #include <linux/usb/usb_phy_generic.h> -#define PCI_PRODUCT_ID_HAPS_HSOTG 0xabc0 +#include "core.h" static const char dwc2_driver_name[] = "dwc2-pci"; @@ -122,18 +122,6 @@ static int dwc2_pci_probe(struct pci_dev *pci, return ret; } -static const struct pci_device_id dwc2_pci_ids[] = { - { - PCI_DEVICE(PCI_VENDOR_ID_SYNOPSYS, PCI_PRODUCT_ID_HAPS_HSOTG), - }, - { - PCI_DEVICE(PCI_VENDOR_ID_STMICRO, - PCI_DEVICE_ID_STMICRO_USB_OTG), - }, - { /* end: all zeroes */ } -}; -MODULE_DEVICE_TABLE(pci, dwc2_pci_ids); - static struct pci_driver dwc2_pci_driver = { .name = dwc2_driver_name, .id_table = dwc2_pci_ids, diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index e43ab203054a..6481f648695a 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -157,6 +157,7 @@ #define PCI_VENDOR_ID_PCI_SIG 0x0001 #define PCI_VENDOR_ID_LOONGSON 0x0014 +#define PCI_DEVICE_ID_LOONGSON_DWC2 0x7a04 #define PCI_VENDOR_ID_SOLIDIGM 0x025e @@ -2356,6 +2357,7 @@ #define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3_AXI 0xabce #define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB31 0xabcf #define PCI_DEVICE_ID_SYNOPSYS_EDDA 0xedda +#define PCI_PRODUCT_ID_HAPS_HSOTG 0xabc0 #define PCI_VENDOR_ID_USR 0x16ec