| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 9 Jun 2023 17:07:03 +0800
From: zhuyinbo <zhuyinbo@...ngson.cn>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Minas Harutyunyan <hminas@...opsys.com>,
Alan Stern <stern@...land.harvard.edu>,
linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
Jianmin Lv <lvjianmin@...ngson.cn>, wanghongliang@...ngson.cn,
Liu Peibao <liupeibao@...ngson.cn>,
loongson-kernel@...ts.loongnix.cn, zhuyinbo@...ngson.cn
Subject: Re: [PATCH v2] usb: dwc2: add pci_device_id driver_data parse support
在 2023/6/9 下午2:13, Greg Kroah-Hartman 写道:
> On Fri, Jun 09, 2023 at 10:50:47AM +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@...ngson.cn>
>> ---
>> Change in v2:
>> 1. Move the dwc2 pci ID from pci_ids.h to params.c.
>> 2. Add some code logic to ensure that the current device is
>> a PCI device.
>> 3. Fix the compile issue when dwc2 pci driver as module.
>>
>> drivers/usb/dwc2/core.h | 1 +
>> drivers/usb/dwc2/params.c | 39 ++++++++++++++++++++++++++++++++++++++-
>> drivers/usb/dwc2/pci.c | 14 +-------------
>> 3 files changed, 40 insertions(+), 14 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..6b68a8830781 100644
>> --- a/drivers/usb/dwc2/params.c
>> +++ b/drivers/usb/dwc2/params.c
>> @@ -7,9 +7,14 @@
>> #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"
>>
>> +#define PCI_PRODUCT_ID_HAPS_HSOTG 0xabc0
>> +#define PCI_DEVICE_ID_LOONGSON_DWC2 0x7a04
>> +
>> static void dwc2_set_bcm_params(struct dwc2_hsotg *hsotg)
>> {
>> struct dwc2_core_params *p = &hsotg->params;
>> @@ -55,6 +60,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_PARTIAL;
>> +}
>> +
>> static void dwc2_set_x1600_params(struct dwc2_hsotg *hsotg)
>> {
>> struct dwc2_core_params *p = &hsotg->params;
>> @@ -281,6 +294,23 @@ 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);
>> +EXPORT_SYMBOL_GPL(dwc2_pci_ids);
>> +
>> static void dwc2_set_param_otg_cap(struct dwc2_hsotg *hsotg)
>> {
>> switch (hsotg->hw_params.op_mode) {
>> @@ -927,13 +957,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));
>
> At this point in time, how can you guarantee that the parent device
> really is a PCI one? This function is being called from a platform
> device callback, and platform devices should NEVER be a child of a PCI
> device, as that's not how PCI or platform devices work.
>
> So how is this even possible?
>
> confused,
>
Hi greg k-h,
My current considerations are based on that the dwc2 drivers support
three types of devices, they are Platform device / ACPI device / PCI
device, and The dwc2/platform.c can all cover the three type dwc2
device, no matter how it is registered.
So, when a dwc2 device wasn't platform device and acpi device and It
will be a PCI device, and the the dwc2/pci.c will register a dwc2
platform device that dwc2 device is a device that in dwc2/platform.c's
dwc2_driver_probe. the "&dwc2->dev->parent" is the "hsotg->dev->parent"
and it was also the &pci->dev.
So, We can use "to_pci_dev(hsotg->dev->parent)" to gain a dwc2 pci
device.
1) DWC2 PCI device driver (drivers/usb/dwc2/pci.c) :
static int dwc2_pci_probe(struct pci_dev *pci,
const struct pci_device_id *id)
{
...
struct platform_device *dwc2;
...
struct device *dev = &pci->dev;
...
dwc2 = platform_device_alloc("dwc2", PLATFORM_DEVID_AUTO);
//"dwc2" was used to match dwc2 platform driver
...
dwc2->dev.parent = dev;
...
}
2) DWC2 PLATFROM driver (drivers/usb/dwc2/platform.c) :
// This driver cover pci type device.
static const char dwc2_driver_name[] = "dwc2";
static struct platform_driver dwc2_platform_driver = {
.driver = {
.name = dwc2_driver_name,
//"dwc2" was used to match pci type dwc2 device
...
},
.probe = dwc2_driver_probe,
.remove = dwc2_driver_remove,
.shutdown = dwc2_driver_shutdown,
};
static int dwc2_driver_probe(struct platform_device *dev)
{
// The dev is the "dwc2" platform device in dwc2/pci.c driver.
struct dwc2_hsotg *hsotg;
...
hsotg->dev = &dev->dev;
// The hsotg->dev->parent or &dev->dev->parent is the &pci->dev in
// dwc2/pci.c driver.
...
}
Thanks,
Yinbo
Powered by blists - more mailing lists