| 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: <BYAPR07MB47090A16A7A8A85C6C3FD46ADDA80@BYAPR07MB4709.namprd07.prod.outlook.com>
Date: Wed, 5 Dec 2018 07:19:51 +0000
From: Pawel Laszczak <pawell@...ence.com>
To: Peter Chen <hzpeterchen@...il.com>
CC: "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"rogerq@...com" <rogerq@...com>,
lkml <linux-kernel@...r.kernel.org>,
Alan Douglas <adouglas@...ence.com>,
"jbergsagel@...com" <jbergsagel@...com>,
"nsekhar@...com" <nsekhar@...com>, "nm@...com" <nm@...com>,
Suresh Punnoose <sureshp@...ence.com>,
"peter.chen@....com" <peter.chen@....com>,
Pawel Jez <pjez@...ence.com>, Rahul Kumar <kurahul@...ence.com>
Subject: RE: [RFC PATCH v2 04/15] usb:cdns3: Driver initialization code.
Hi,
>> >>
>> >> Patch adds core.c and core.h file that implements initialization
>> >> of platform driver and adds function responsible for selecting,
>> >> switching and running appropriate Device/Host mode.
>> >>
>> >> Signed-off-by: Pawel Laszczak <pawell@...ence.com>
>> >> ---
>> >> drivers/usb/cdns3/Makefile | 2 +
>> >> drivers/usb/cdns3/core.c | 413 +++++++++++++++++++++++++++++++++++++
>> >> drivers/usb/cdns3/core.h | 100 +++++++++
>> >> 3 files changed, 515 insertions(+)
>> >> create mode 100644 drivers/usb/cdns3/core.c
>> >> create mode 100644 drivers/usb/cdns3/core.h
>> >>
>> >> diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
>> >> index dcdd62003c6a..02d25b23c5d3 100644
>> >> --- a/drivers/usb/cdns3/Makefile
>> >> +++ b/drivers/usb/cdns3/Makefile
>> >> @@ -1,3 +1,5 @@
>> >> +obj-$(CONFIG_USB_CDNS3) += cdns3.o
>> >> obj-$(CONFIG_USB_CDNS3_PCI_WRAP) += cdns3-pci.o
>> >>
>> >> +cdns3-y := core.o
>> >> cdns3-pci-y := cdns3-pci-wrap.o
>> >> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>> >> new file mode 100644
>> >> index 000000000000..f9055d4da67f
>> >> --- /dev/null
>> >> +++ b/drivers/usb/cdns3/core.c
>> >> @@ -0,0 +1,413 @@
>> >> +// SPDX-License-Identifier: GPL-2.0
>> >> +/*
>> >> + * Cadence USBSS DRD Driver.
>> >> + *
>> >> + * Copyright (C) 2018 Cadence.
>> >> + *
>> >
>> >Please add NXP copyright too.
>>
>> Ok, I don't know why I omitted this.
>> I know that you are the main author of this file
>> Sorry for that.
>>
>> One additional question. What year I should add in Copyright for NXP?.
>> The original year 2017 or I should modified all to 2018.
>>
>Please use below copyright, thanks.
>
>Copyright 2017-2018 NXP
I add this in all files modified or created by you.
>
>
>
>> >> + mutex_init(&cdns->mutex);
>> >> +
>> >> + cdns->phy = devm_phy_get(dev, "cdns3,usbphy");
>> >> + if (IS_ERR(cdns->phy)) {
>> >> + dev_info(dev, "no generic phy found\n");
>> >> + cdns->phy = NULL;
>> >> + /*
>> >> + * fall through here!
>> >> + * if no generic phy found, phy init
>> >> + * should be done under boot!
>> >> + */
>> >
>> >If the phy driver is defer-probed, it will be here, it is not an error.
>> >I think you could have a generic phy driver or usb generic phy driver
>> >(drivers/usb/phy/phy-generic.c) even you don't need any operations for
>> >PHY. It will be easy for other platforms.
>>
>> Yes, Roger ask me to modify this fragment. In next version it will look like:
>> cdns->phy = devm_phy_get(dev, "cdns3,usbphy");
>> if (IS_ERR(cdns->phy)) {
>> ret = PTR_ERR(cdns->phy);
>> if (ret == -ENOSYS || ret == -ENODEV) {
>> cdns->phy = NULL;
>> } else if (ret == -EPROBE_DEFER) {
>> return ret;
>> } else {
>> dev_err(dev, "no phy found\n");
>> goto err0;
>> }
>> }
>>
>> phy_init(cdns->phy);
>>
>> We are going to use phy driver. I don't know if it correct.
>> I don't have experience in this filed.
>> We need phy initialization but I don't have testing platform now.
>> In most usb drivers I see that there are used usb phy driverd instead phy dirverd.
>>
>
>At my CDNS3 platform, there are some USB PHY initialization for register setting
>and clock enable. You could add generic PHY driver under: drivers/phy/cadence/.
>
>Above PHY initialization code is OK for me.
It will be added as separate driver.
I think that Allan Douglas working on it.
I ask him to add you to -cc in patch for phy.
>
>
>> >> +static void __exit cdns3_driver_platform_unregister(void)
>> >> +{
>> >> + platform_driver_unregister(&cdns3_driver);
>> >> +}
>> >> +module_exit(cdns3_driver_platform_unregister);
>> >> +
>> >> +MODULE_ALIAS("platform:cdns3");
>> >> +MODULE_AUTHOR("Pawel Laszczak <pawell@...ence.com>");
>> >> +MODULE_LICENSE("GPL v2");
>> >> +MODULE_DESCRIPTION("Cadence USB3 DRD Controller Driver");
>> >> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
>> >> new file mode 100644
>> >> index 000000000000..7c8204fe4d3d
>> >> --- /dev/null
>> >> +++ b/drivers/usb/cdns3/core.h
>> >> @@ -0,0 +1,100 @@
>> >> +/* SPDX-License-Identifier: GPL-2.0 */
>> >> +/*
>> >> + * Cadence USBSS DRD Driver.
>> >> + *
>> >
>> >Header file
>> I don't understand. What is wrong ?
>> >
>
>The comment for this file
>
>Cadence USBSS DRD Core Header File
Ok, thanks
Cheers,
Pawel
Powered by blists - more mailing lists