linux-kernel - Re: [PATCH 04/10] USB: EHCI: make ehci-orion a separate driver

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: <5114D58A.1030903@openwrt.org>
Date:	Fri, 08 Feb 2013 11:38:02 +0100
From:	Florian Fainelli <florian@...nwrt.org>
To:	manjunath.goudar@...aro.org
CC:	linux-usb@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	patches@...aro.org, stern@...land.harvard.edu, arnd@...aro.org,
	gregkh@...uxfoundation.org, Greg KH <greg@...ah.com>,
	Jason Cooper <jason@...edaemon.net>,
	Andrew Lunn <andrew@...n.ch>,
	Russell King <linux@....linux.org.uk>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 04/10] USB: EHCI: make ehci-orion a separate driver

Hello Manjunath,

On 02/07/2013 06:34 PM, manjunath.goudar@...aro.org wrote:
> From: Manjunath Goudar <manjunath.goudar@...aro.org>
>
> Separate the Marvell Orion host controller driver from ehci-hcd host code
> into its own driver module.
>
[snip]

The changes you introduce in ehci-hcd.c would certainly deserve their 
own preliminary patch in this serie and not be squashed into this one imho.

> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -155,7 +155,7 @@ static inline unsigned ehci_read_frame_index(struct ehci_hcd *ehci)
>    * before driver shutdown. But it also seems to be caused by bugs in cardbus
>    * bridge shutdown:  shutting down the bridge before the devices using it.
>    */
> -static int handshake (struct ehci_hcd *ehci, void __iomem *ptr,
> +int handshake(struct ehci_hcd *ehci, void __iomem *ptr,
>   		      u32 mask, u32 done, int usec)
>   {
>   	u32	result;
> @@ -172,9 +172,9 @@ static int handshake (struct ehci_hcd *ehci, void __iomem *ptr,
>   	} while (usec > 0);
>   	return -ETIMEDOUT;
>   }
> -
> +EXPORT_SYMBOL_GPL(handshake);

This sounds way too generic as a name to be exported as is, so please 
namespace this with ehci_hcd_ at least.

>   /* check TDI/ARC silicon is in host mode */
> -static int tdi_in_host_mode (struct ehci_hcd *ehci)
> +static int tdi_in_host_mode(struct ehci_hcd *ehci)

Looks unrelated to the goals of your patches.

>   {
>   	u32		tmp;
>
> @@ -186,7 +186,7 @@ static int tdi_in_host_mode (struct ehci_hcd *ehci)
>    * Force HC to halt state from unknown (EHCI spec section 2.3).
>    * Must be called with interrupts enabled and the lock not held.
>    */
> -static int ehci_halt (struct ehci_hcd *ehci)
> +int ehci_halt(struct ehci_hcd *ehci)
>   {
>   	u32	temp;
>
> @@ -215,9 +215,9 @@ static int ehci_halt (struct ehci_hcd *ehci)
>   	return handshake(ehci, &ehci->regs->status,
>   			  STS_HALT, STS_HALT, 16 * 125);
>   }
> -
> +EXPORT_SYMBOL_GPL(ehci_halt);

I would rename this to ehci_hcd_halt to express the fact that it takes 
an ehci_hcd structure pointer as parameter, can be addressed later.

>   /* put TDI/ARC silicon into EHCI mode */
> -static void tdi_reset (struct ehci_hcd *ehci)
> +void tdi_reset(struct ehci_hcd *ehci)
>   {
>   	u32		tmp;
>
> @@ -231,12 +231,12 @@ static void tdi_reset (struct ehci_hcd *ehci)
>   		tmp |= USBMODE_BE;
>   	ehci_writel(ehci, tmp, &ehci->regs->usbmode);
>   }
> -
> +EXPORT_SYMBOL_GPL(tdi_reset);

Same here, way too generic to be exported as is as a symbol name.
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/