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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <998aa6dd-6c21-48c8-a069-3b8cc8df2bb3@molgen.mpg.de>
Date:   Sun, 15 Oct 2023 21:18:06 +0200
From:   Paul Menzel <pmenzel@...gen.mpg.de>
To:     Tomer Maimon <tmaimon77@...il.com>
Cc:     peter.chen@...nel.org, gregkh@...uxfoundation.org,
        avifishman70@...il.com, tali.perry1@...il.com, joel@....id.au,
        venture@...gle.com, yuenn@...gle.com, benjaminfair@...gle.com,
        j.neuschaefer@....net, openbmc@...ts.ozlabs.org,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 3/3] usb: chipidea: Add support for NPCM

Dear Tomer,


Am 15.10.23 um 19:21 schrieb Tomer Maimon:

> On Fri, 13 Oct 2023 at 16:18, Paul Menzel <pmenzel@...gen.mpg.de> wrote:

>> Am 13.10.23 um 01:00 schrieb Tomer Maimon:
>>> Add Nuvoton NPCM BMC SoCs support to USB ChipIdea driver.
>>> NPCM SoC include ChipIdea IP block that used for USB device controller
>>
>> include*s*
>> “that *is* used” or just “… used for”
> Will do
>>
>>> mode.
>>
>> Please add a line, how you tested this patch.
> I don't recall "how you tested the patch" done in the commit message,
> I can add it in the cover letter

In my opinion, adding a separate paragraph to the commit message would 
be best.

>>> Signed-off-by: Tomer Maimon <tmaimon77@...il.com>
>>> Acked-by: Peter Chen <peter.chen@...nel.org>
>>> ---
>>>    drivers/usb/chipidea/Kconfig        |   4 +
>>>    drivers/usb/chipidea/Makefile       |   1 +
>>>    drivers/usb/chipidea/ci_hdrc_npcm.c | 114 ++++++++++++++++++++++++++++
>>>    3 files changed, 119 insertions(+)
>>>    create mode 100644 drivers/usb/chipidea/ci_hdrc_npcm.c
>>>
>>> diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig
>>> index c815824a0b2d..bab45bc62361 100644
>>> --- a/drivers/usb/chipidea/Kconfig
>>> +++ b/drivers/usb/chipidea/Kconfig
>>> @@ -43,6 +43,10 @@ config USB_CHIPIDEA_MSM
>>>        tristate "Enable MSM hsusb glue driver" if EXPERT
>>>        default USB_CHIPIDEA
>>>
>>> +config USB_CHIPIDEA_NPCM
>>> +     tristate "Enable NPCM hsusb glue driver" if EXPERT
>>> +     default USB_CHIPIDEA
>>> +
>>>    config USB_CHIPIDEA_IMX
>>>        tristate "Enable i.MX USB glue driver" if EXPERT
>>>        depends on OF
>>> diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
>>> index 71afeab97e83..718cb24603dd 100644
>>> --- a/drivers/usb/chipidea/Makefile
>>> +++ b/drivers/usb/chipidea/Makefile
>>> @@ -13,6 +13,7 @@ ci_hdrc-$(CONFIG_USB_OTG_FSM)               += otg_fsm.o
>>>
>>>    obj-$(CONFIG_USB_CHIPIDEA_GENERIC)  += ci_hdrc_usb2.o
>>>    obj-$(CONFIG_USB_CHIPIDEA_MSM)              += ci_hdrc_msm.o
>>> +obj-$(CONFIG_USB_CHIPIDEA_NPCM)              += ci_hdrc_npcm.o
>>>    obj-$(CONFIG_USB_CHIPIDEA_PCI)              += ci_hdrc_pci.o
>>>    obj-$(CONFIG_USB_CHIPIDEA_IMX)              += usbmisc_imx.o ci_hdrc_imx.o
>>>    obj-$(CONFIG_USB_CHIPIDEA_TEGRA)    += ci_hdrc_tegra.o
>>> diff --git a/drivers/usb/chipidea/ci_hdrc_npcm.c b/drivers/usb/chipidea/ci_hdrc_npcm.c
>>> new file mode 100644
>>> index 000000000000..37b64a3dbd96
>>> --- /dev/null
>>> +++ b/drivers/usb/chipidea/ci_hdrc_npcm.c
>>> @@ -0,0 +1,114 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +// Copyright (c) 2023 Nuvoton Technology corporation.
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/pm_runtime.h>
>>> +#include <linux/usb/chipidea.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/io.h>
>>> +#include <linux/reset-controller.h>
>>> +#include <linux/of.h>
>>> +
>>> +#include "ci.h"
>>> +
>>> +struct npcm_udc_data {
>>> +     struct platform_device  *ci;
>>> +     struct clk              *core_clk;
>>> +     struct ci_hdrc_platform_data pdata;
>>> +};
>>> +
>>> +static int npcm_udc_notify_event(struct ci_hdrc *ci, unsigned event)
>>> +{
>>> +     struct device *dev = ci->dev->parent;
>>> +
>>> +     switch (event) {
>>> +     case CI_HDRC_CONTROLLER_RESET_EVENT:
>>> +             /* clear all mode bits */
>>> +             hw_write(ci, OP_USBMODE, 0xffffffff, 0x0);
>>> +             break;
>>> +     default:
>>> +             dev_dbg(dev, "unknown ci_hdrc event\n");
>>
>> Please print it out.
> Do you mean instead dev_dbg to use dev_info? in that case, each event
> that didnt handled will cause a print.

Sorry for not being clear. I meant print out `event`.

>>> +             break;
>>> +     }
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int npcm_udc_probe(struct platform_device *pdev)
>>> +{
>>> +     int ret;
>>> +     struct npcm_udc_data *ci;
>>> +     struct platform_device *plat_ci;
>>> +     struct device *dev = &pdev->dev;
>>> +
>>> +     ci = devm_kzalloc(&pdev->dev, sizeof(*ci), GFP_KERNEL);
>>> +     if (!ci)
>>> +             return -ENOMEM;
>>> +     platform_set_drvdata(pdev, ci);
>>> +
>>> +     ci->core_clk = devm_clk_get_optional(dev, NULL);
>>> +     if (IS_ERR(ci->core_clk))
>>> +             return PTR_ERR(ci->core_clk);
>>> +
>>> +     ret = clk_prepare_enable(ci->core_clk);
>>> +     if (ret)
>>> +             return dev_err_probe(dev, ret, "failed to enable the clock: %d\n", ret);
>>> +
>>> +     ci->pdata.name = dev_name(dev);
>>> +     ci->pdata.capoffset = DEF_CAPOFFSET;
>>> +     ci->pdata.flags = CI_HDRC_REQUIRES_ALIGNED_DMA |
>>> +             CI_HDRC_FORCE_VBUS_ACTIVE_ALWAYS;
>>> +     ci->pdata.phy_mode = USBPHY_INTERFACE_MODE_UTMI;
>>> +     ci->pdata.notify_event = npcm_udc_notify_event;
>>> +
>>> +     plat_ci = ci_hdrc_add_device(dev, pdev->resource, pdev->num_resources,
>>> +                                  &ci->pdata);
>>> +     if (IS_ERR(plat_ci)) {
>>> +             ret = PTR_ERR(plat_ci);
>>> +             dev_err(dev, "failed to register HDRC NPCM device: %d\n", ret);
>>> +             goto clk_err;
>>> +     }
>>> +
>>> +     pm_runtime_no_callbacks(dev);
>>> +     pm_runtime_enable(dev);
>>> +
>>> +     return 0;
>>> +
>>> +clk_err:
>>> +     clk_disable_unprepare(ci->core_clk);
>>> +     return ret;
>>> +}
>>> +
>>> +static int npcm_udc_remove(struct platform_device *pdev)
>>> +{
>>> +     struct npcm_udc_data *ci = platform_get_drvdata(pdev);
>>> +
>>> +     pm_runtime_disable(&pdev->dev);
>>> +     ci_hdrc_remove_device(ci->ci);
>>> +     clk_disable_unprepare(ci->core_clk);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static const struct of_device_id npcm_udc_dt_match[] = {
>>> +     { .compatible = "nuvoton,npcm750-udc", },
>>> +     { .compatible = "nuvoton,npcm845-udc", },
>>> +     { }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, npcm_udc_dt_match);
>>> +
>>> +static struct platform_driver npcm_udc_driver = {
>>> +     .probe = npcm_udc_probe,
>>> +     .remove = npcm_udc_remove,
>>> +     .driver = {
>>> +             .name = "npcm_udc",
>>> +             .of_match_table = npcm_udc_dt_match,
>>> +     },
>>> +};
>>> +
>>> +module_platform_driver(npcm_udc_driver);
>>> +
>>> +MODULE_DESCRIPTION("NPCM USB device controller driver");
>>> +MODULE_AUTHOR("Tomer Maimon <tomer.maimon@...oton.com>");
>>
>> Should that address also be recorded as the patch author?
> I prefer to use this e-mail since this is my main e-mail.

Alright, if it’s fine with your employer that’s great.

>>> +MODULE_LICENSE("GPL v2");


Kind regards,

Paul

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ