| 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
| ||
|
Message-ID: <BYAPR07MB4709E63A6B10AC2A894835E7DDC40@BYAPR07MB4709.namprd07.prod.outlook.com>
Date: Wed, 7 Nov 2018 08:42:25 +0000
From: Pawel Laszczak <pawell@...ence.com>
To: Roger Quadros <rogerq@...com>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
CC: "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Alan Douglas <adouglas@...ence.com>,
"jbergsagel@...com" <jbergsagel@...com>,
"peter.chen@....com" <peter.chen@....com>,
Pawel Jez <pjez@...ence.com>, Rahul Kumar <kurahul@...ence.com>
Subject: RE: [RFC PATCH v1 01/14] usb:cdns3: add pci to platform driver
wrapper.
Hi Roger,
>
>Hi Pawel,
>
>On 03/11/18 19:51, Pawel Laszczak wrote:
>> Patch adds PCI specific glue drivier that creaties and registers in
>
>s/drivier/driver
>s/creaties/creates
>s/in system/in-system
>
>> system cdns-usb3 platform device. Thanks to that we will be able to use
>> the cdns-usb3 platform driver for USBSS-DEV controller
>> build on PCI board
>>
>> Signed-off-by: Pawel Laszczak <pawell@...ence.com>
>> ---
>> drivers/usb/Kconfig | 2 +
>> drivers/usb/Makefile | 2 +
>> drivers/usb/cdns3/Kconfig | 24 +++++
>> drivers/usb/cdns3/Makefile | 3 +
>> drivers/usb/cdns3/cdns3-pci-wrap.c | 162 +++++++++++++++++++++++++++++
>> 5 files changed, 193 insertions(+)
>> create mode 100644 drivers/usb/cdns3/Kconfig
>> create mode 100644 drivers/usb/cdns3/Makefile
>> create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
>>
>> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
>> index 987fc5ba6321..5f9334019d04 100644
>> --- a/drivers/usb/Kconfig
>> +++ b/drivers/usb/Kconfig
>> @@ -112,6 +112,8 @@ source "drivers/usb/usbip/Kconfig"
>>
>> endif
>>
>> +source "drivers/usb/cdns3/Kconfig"
>> +
>> source "drivers/usb/mtu3/Kconfig"
>>
>> source "drivers/usb/musb/Kconfig"
>> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
>> index 7d1b8c82b208..82093a60ea2c 100644
>> --- a/drivers/usb/Makefile
>> +++ b/drivers/usb/Makefile
>> @@ -8,6 +8,8 @@
>> obj-$(CONFIG_USB) += core/
>> obj-$(CONFIG_USB_SUPPORT) += phy/
>>
>> +obj-$(CONFIG_USB_CDNS3) += cdns3/
>> +
>> obj-$(CONFIG_USB_DWC3) += dwc3/
>> obj-$(CONFIG_USB_DWC2) += dwc2/
>> obj-$(CONFIG_USB_ISP1760) += isp1760/
>> diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
>> new file mode 100644
>> index 000000000000..888458372adb
>> --- /dev/null
>> +++ b/drivers/usb/cdns3/Kconfig
>> @@ -0,0 +1,24 @@
>> +config USB_CDNS3
>> + tristate "Cadence USB3 Dual-Role Controller"
>> + depends on ((USB_XHCI_HCD && USB_GADGET) || (USB_XHCI_HCD && !USB_GADGET) || (!USB_XHCI_HCD && USB_GADGET)) &&
>HAS_DMA
>
>Why not depend on USB instead of USB_XHCI_HCD?
I will replace it with this:
Depend on USB_SUPPORT && (USB l| USB_GADGET) && HAS_DMA
>> + help
>> + Say Y here if your system has a cadence USB3 dual-role controller.
>> + It supports: dual-role switch Host-only, and Peripheral-only.
>
>Need a coma between switch and Host-only.
>
>> +
>> + If you choose to build this driver is a dynamically linked
>> + module, the module will be called cdns3.ko.
>> +
>> +if USB_CDNS3
>> +
>> +config USB_CDNS3_PCI_WRAP
>> + tristate "PCIe-based Platforms"
>> + depends on USB_PCI && ACPI
>> + default USB_CDNS3
>> + help
>> + If you're using the USBSS Core IP with a PCIe, please say
>> + 'Y' or 'M' here.
>> +
>> + If you choose to build this driver as module it will
>> + be dynamically linked and module will be called cdns3-pci.ko
>> +
>> +endif
>> diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
>> new file mode 100644
>> index 000000000000..dcdd62003c6a
>> --- /dev/null
>> +++ b/drivers/usb/cdns3/Makefile
>> @@ -0,0 +1,3 @@
>> +obj-$(CONFIG_USB_CDNS3_PCI_WRAP) += cdns3-pci.o
>> +
>> +cdns3-pci-y := cdns3-pci-wrap.o
>> diff --git a/drivers/usb/cdns3/cdns3-pci-wrap.c b/drivers/usb/cdns3/cdns3-pci-wrap.c
>> new file mode 100644
>> index 000000000000..6c229ab6dffb
>> --- /dev/null
>> +++ b/drivers/usb/cdns3/cdns3-pci-wrap.c
>> @@ -0,0 +1,162 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Cadence USBSS PCI Glue driver
>> + *
>> + * Copyright (C) 2018 Cadence.
>> + *
>> + * Author: Pawel Laszczak <pawell@...ence.com>
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/pci.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/dma-mapping.h>
>> +#include <linux/slab.h>
>> +
>> +struct cdns3_wrap {
>> + struct platform_device *plat_dev;
>> + struct pci_dev *hg_dev;
>> + struct resource dev_res[4];
>> +};
>> +
>> +struct cdns3_wrap wrap;
>> +
>> +#define RES_IRQ_ID 0
>> +#define RES_HOST_ID 1
>> +#define RES_DEV_ID 2
>> +#define RES_DRD_ID 3
>> +
>> +#define PCI_BAR_HOST 0
>> +#define PCI_BAR_DEV 2
>> +#define PCI_BAR_OTG 4
>> +
>> +#define PCI_DEV_FN_HOST_DEVICE 0
>> +#define PCI_DEV_FN_OTG 1
>> +
>> +#define PCI_DRIVER_NAME "cdns3-pci-usbss"
>> +#define PLAT_DRIVER_NAME "cdns-usb3"
>> +
>> +#define CDNS_VENDOR_ID 0x17cd
>> +#define CDNS_DEVICE_ID 0x0100
>> +
>> +/**
>> + * cdns3_pci_probe - Probe function for Cadence USB wrapper driver
>> + * @pdev: platform device object
>> + * @id: pci device id
>> + *
>> + * Returns 0 on success otherwise negative errno
>> + */
>> +static int cdns3_pci_probe(struct pci_dev *pdev,
>> + const struct pci_device_id *id)
>> +{
>> + struct platform_device_info plat_info;
>> + struct cdns3_wrap *wrap;
>> + struct resource *res;
>> + int err;
>> +
>> + /*
>> + * for GADGET/HOST PCI (devfn) function number is 0,
>> + * for OTG PCI (devfn) function number is 1
>> + */
>> + if (!id || pdev->devfn != PCI_DEV_FN_HOST_DEVICE)
>> + return -EINVAL;
>> +
>> + err = pcim_enable_device(pdev);
>> + if (err) {
>> + dev_err(&pdev->dev, "Enabling PCI device has failed %d\n", err);
>> + return err;
>> + }
>> +
>> + pci_set_master(pdev);
>> + wrap = devm_kzalloc(&pdev->dev, sizeof(*wrap), GFP_KERNEL);
>> + if (!wrap) {
>> + dev_err(&pdev->dev, "Failed to load PCI module\n");
>> + return -ENOMEM;
>> + }
>> +
>> + /* function 0: host(BAR_0) + device(BAR_1) + otg(BAR_2)). */
>> + memset(wrap->dev_res, 0x00,
>> + sizeof(struct resource) * ARRAY_SIZE(wrap->dev_res));
>> + dev_info(&pdev->dev, "Initialize Device resources\n");
>> + res = wrap->dev_res;
>> +
>> +#if IS_ENABLED(CONFIG_USB_CDNS3_GADGET)
>
>Why depend on Config options to populate resources? The resources should be there regardless.
>It is a lot simpler that way as it reflects the hardware as-is.
You're right. I will remove these Config options from this code.
>
>> + res[RES_DEV_ID].start = pci_resource_start(pdev, PCI_BAR_DEV);
>> + res[RES_DEV_ID].end = pci_resource_end(pdev, PCI_BAR_DEV);
>> + res[RES_DEV_ID].name = "cdns3-dev-regs";
>> + res[RES_DEV_ID].flags = IORESOURCE_MEM;
>> + dev_info(&pdev->dev, "USBSS-DEV physical base addr: %pa\n",
>> + &res[RES_DEV_ID].start);
>> +
>
>dev_dbg() for this and all occurrences below?
Ok , I replaced it.
>
>> +#endif
>> +
>> +#if IS_ENABLED(CONFIG_USB_CDNS3_HOST)
>> + res[RES_HOST_ID].start = pci_resource_start(pdev, PCI_BAR_HOST);
>> + res[RES_HOST_ID].end = pci_resource_end(pdev, PCI_BAR_HOST);
>> + res[RES_HOST_ID].name = "cdns3-xhci-regs";
>> + res[RES_HOST_ID].flags = IORESOURCE_MEM;
>> + dev_info(&pdev->dev, "USBSS-XHCI physical base addr: %pa\n",
>> + &res[RES_HOST_ID].start);
>> +#endif
>> +
>> + res[RES_DRD_ID].start = pci_resource_start(pdev, PCI_BAR_OTG);
>> + res[RES_DRD_ID].end = pci_resource_end(pdev, PCI_BAR_OTG);
>> + res[RES_DRD_ID].name = "cdns3-otg";
>> + res[RES_DRD_ID].flags = IORESOURCE_MEM;
>> + dev_info(&pdev->dev, "USBSS-DRD physical base addr: %pa\n",
>> + &res[RES_DRD_ID].start);
>> +
>> + /* Interrupt common for both device and XHCI */
>> + wrap->dev_res[RES_IRQ_ID].start = pdev->irq;
>> + wrap->dev_res[RES_IRQ_ID].name = "cdns3-irq";
>> + wrap->dev_res[RES_IRQ_ID].flags = IORESOURCE_IRQ;
>> +
>> + /* set up platform device info */
>> + memset(&plat_info, 0, sizeof(plat_info));
>> + plat_info.parent = &pdev->dev;
>> + plat_info.fwnode = pdev->dev.fwnode;
>> + plat_info.name = PLAT_DRIVER_NAME;
>> + plat_info.id = pdev->devfn;
>> + plat_info.res = wrap->dev_res;
>> + plat_info.num_res = ARRAY_SIZE(wrap->dev_res);
>> + plat_info.dma_mask = pdev->dma_mask;
>> +
>> + /* register platform device */
>> + wrap->plat_dev = platform_device_register_full(&plat_info);
>> + if (IS_ERR(wrap->plat_dev)) {
>> + err = PTR_ERR(wrap->plat_dev);
>> + return err;
>> + }
>> +
>> + pci_set_drvdata(pdev, wrap);
>> +
>> + return err;
>> +}
>> +
>> +void cdns3_pci_remove(struct pci_dev *pdev)
>> +{
>> + struct cdns3_wrap *wrap = (struct cdns3_wrap *)pci_get_drvdata(pdev);
>> +
>> + platform_device_unregister(wrap->plat_dev);
>> +}
>> +
>> +static const struct pci_device_id cdns3_pci_ids[] = {
>> + { PCI_DEVICE(CDNS_VENDOR_ID, CDNS_DEVICE_ID), },
>> + { 0, }
>> +};
>> +
>> +static struct pci_driver cdns3_pci_driver = {
>> + .name = PCI_DRIVER_NAME,
>> + .id_table = cdns3_pci_ids,
>> + .probe = cdns3_pci_probe,
>> + .remove = cdns3_pci_remove,
>> +};
>> +
>> +module_pci_driver(cdns3_pci_driver);
>> +MODULE_DEVICE_TABLE(pci, cdns3_pci_ids);
>> +
>> +MODULE_AUTHOR("Pawel Laszczak <pawell@...ence.com>");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("Cadence USBSS PCI wrapperr");
>> +
>>
>
>cheers,
>-roger
>--
>Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Thanks for all comments
Regards,
Pawel Laszczak
Powered by blists - more mailing lists