From: "Bjørn Mork" <bjorn@mork.no>
To: Daehwan Jung <dh10.jung@samsung.com>
Cc: Mathias Nyman <mathias.nyman@intel.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
Howard Yen <howardyen@google.com>,
Jack Pham <jackp@codeaurora.org>, Puma Hsu <pumahsu@google.com>,
"J . Avila" <elavila@google.com>,
sc.suh@samsung.com
Subject: Re: [PATCH v3 4/4] usb: host: add xhci-exynos driver
Date: Mon, 21 Mar 2022 16:45:01 +0100 [thread overview]
Message-ID: <87r16v9uo2.fsf@miraculix.mork.no> (raw)
In-Reply-To: <1647853194-62147-5-git-send-email-dh10.jung@samsung.com> (Daehwan Jung's message of "Mon, 21 Mar 2022 17:59:54 +0900")
Daehwan Jung <dh10.jung@samsung.com> writes:
> +++ b/drivers/usb/host/xhci-exynos.c
> @@ -0,0 +1,982 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * xhci-exynos.c - xHCI host controller driver platform Bus Glue for Exynos.
> + *
> + * Copyright (C) 2022 Samsung Electronics Incorporated - http://www.samsung.com
> + * Author: Daehwan Jung <dh10.jung@samsung.com>
> + *
> + * A lot of code borrowed from the Linux xHCI driver.
> + */
> +#include <linux/clk.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/usb/phy.h>
> +#include <linux/slab.h>
> +#include <linux/acpi.h>
> +#include <linux/usb/of.h>
> +
> +#include "xhci.h"
> +#include "xhci-plat.h"
> +#include "xhci-mvebu.h"
> +#include "xhci-rcar.h"
The xhci-plat.c file is Copyright (C) 2012 Texas Instruments Incorporated
You can't just steal it.
Besides, even if you could, this isn't about copying as much code as
posible from A to B. The point is to add as *little* code as possible
to support your hardware.
> +static int xhci_exynos_vendor_init(struct xhci_hcd *xhci)
> +{
> + /* TODO */
> + return 0;
> +}
And you didn't even add that?
> +static int xhci_exynos_wake_lock(struct xhci_hcd_exynos *xhci_exynos,
> + int is_main_hcd, int is_lock)
> +{
> + struct usb_hcd *hcd = xhci_exynos->hcd;
> + struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> + struct wakeup_source *main_wakelock, *shared_wakelock;
> +
> + main_wakelock = xhci_exynos->main_wakelock;
> + shared_wakelock = xhci_exynos->shared_wakelock;
Are these fields initialized anywhere?
> +
> + if (xhci->xhc_state & XHCI_STATE_REMOVING)
> + return -ESHUTDOWN;
> +
> + if (is_lock) {
bool?
> + if (is_main_hcd)
another bool?
> + __pm_stay_awake(main_wakelock);
> + else
> + __pm_stay_awake(shared_wakelock);
> + } else {
> + if (is_main_hcd)
> + __pm_relax(main_wakelock);
> + else
> + __pm_relax(shared_wakelock);
> + }
Looks interesting. Are you signalling relax/wakeups events to the PM
core on device suspend/resume? Why?
> +static int xhci_exynos_address_device(struct usb_hcd *hcd, struct usb_device *udev)
> +{
> + struct xhci_hcd *xhci;
> + int ret;
> +
> + ret = xhci_address_device(hcd, udev);
> + xhci = hcd_to_xhci(hcd);
> +
> + return ret;
> +}
What's left here if we drop the unused parts?
> +#ifdef CONFIG_OF
> +static const struct xhci_plat_priv xhci_plat_marvell_armada = {
> + .init_quirk = xhci_mvebu_mbus_init_quirk,
> +};
> +
> +static const struct xhci_plat_priv xhci_plat_marvell_armada3700 = {
> + .plat_setup = xhci_mvebu_a3700_plat_setup,
> + .init_quirk = xhci_mvebu_a3700_init_quirk,
> +};
Right...
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id usb_xhci_acpi_match[] = {
> + /* XHCI-compliant USB Controller */
> + { "PNP0D10", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, usb_xhci_acpi_match);
> +#endif
Nice one
There's no need to copy me if you plan to resend any of this. I'm just
a drive-by reader here anyway, and I've seen enough.
Good luck!
Bjørn
next prev parent reply other threads:[~2022-03-21 15:56 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20220321090202epcas2p1bfa78db059c1f6f6acbbb015e4bf991c@epcas2p1.samsung.com>
2022-03-21 8:59 ` [PATCH v3 0/4] support USB offload feature Daehwan Jung
[not found] ` <CGME20220321090204epcas2p31e39a4b8b6fc803ceecac5d19e6e39e9@epcas2p3.samsung.com>
2022-03-21 8:59 ` [PATCH v3 1/4] usb: host: export symbols for xhci hooks usage Daehwan Jung
2022-03-21 15:35 ` kernel test robot
2022-03-22 17:12 ` Krzysztof Kozlowski
2022-03-23 1:22 ` Jung Daehwan
[not found] ` <CGME20220321090204epcas2p3b2be5c6b131240e408d12d40c517395c@epcas2p3.samsung.com>
2022-03-21 8:59 ` [PATCH v3 2/4] usb: host: add xhci hooks for USB offload Daehwan Jung
2022-03-21 17:00 ` Mathias Nyman
2022-03-22 2:14 ` Jung Daehwan
[not found] ` <CGME20220321090205epcas2p4f3698a0aa49d251c0a8f008e85d968ba@epcas2p4.samsung.com>
2022-03-21 8:59 ` [PATCH v3 3/4] usb: host: add some to xhci overrides " Daehwan Jung
[not found] ` <CGME20220321090205epcas2p15ac16f281554b663062e0e31666defab@epcas2p1.samsung.com>
2022-03-21 8:59 ` [PATCH v3 4/4] usb: host: add xhci-exynos driver Daehwan Jung
2022-03-21 15:45 ` Bjørn Mork [this message]
2022-03-22 2:30 ` Jung Daehwan
2022-03-21 16:26 ` kernel test robot
2022-03-21 16:37 ` kernel test robot
2022-03-22 17:10 ` Krzysztof Kozlowski
2022-03-23 2:34 ` Jung Daehwan
2022-03-23 8:26 ` Krzysztof Kozlowski
2022-03-29 2:35 ` Jung Daehwan
2022-03-22 17:16 ` Krzysztof Kozlowski
2022-03-23 5:17 ` Jung Daehwan
2022-03-23 8:34 ` Krzysztof Kozlowski
2022-03-21 9:14 ` [PATCH v3 0/4] support USB offload feature Greg Kroah-Hartman
2022-03-21 9:24 ` Jung Daehwan
2022-03-21 9:32 ` Greg Kroah-Hartman
2022-03-21 10:06 ` Jung Daehwan
2022-03-21 10:16 ` Greg Kroah-Hartman
2022-03-22 2:17 ` Jung Daehwan
2022-03-22 17:05 ` Krzysztof Kozlowski
2022-03-23 1:31 ` Jung Daehwan
2022-03-23 8:25 ` Krzysztof Kozlowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87r16v9uo2.fsf@miraculix.mork.no \
--to=bjorn@mork.no \
--cc=dh10.jung@samsung.com \
--cc=elavila@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=howardyen@google.com \
--cc=jackp@codeaurora.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@intel.com \
--cc=pumahsu@google.com \
--cc=sc.suh@samsung.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).