linux-kernel - Re: [PATCH v4] USB: Support for LPC32xx SoC

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] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 8 Mar 2012 14:22:34 -0800
From:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:	Roland Stigge <stigge@...com.de>
Cc:	Alan Stern <stern@...land.harvard.edu>, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org, w.sang@...gutronix.de,
	kevin.wells@....com, linux-arm-kernel@...ts.infradead.org,
	arnd@...db.de
Subject: Re: [PATCH v4] USB: Support for LPC32xx SoC

On Thu, Mar 08, 2012 at 11:09:34PM +0100, Roland Stigge wrote:
> This patch adds OHCI support to the LPC32xx ARM platform
> 
> Signed-off-by: Roland Stigge <stigge@...com.de>
> 
> ---
>  Applies to v3.3-rc6
> 
>  Since v3, only the machine_is_*() dependencies have been pulled into
>  the machine dependent functions.

Better, but:

>  static void pnx4008_unset_usb_bits(void)
>  {
> -	start_int_mask(SE_USB_OTG_ATX_INT_N);
> -	start_int_mask(SE_USB_OTG_TIMER_INT);
> -	start_int_mask(SE_USB_I2C_INT);
> -	start_int_mask(SE_USB_INT);
> -	start_int_mask(SE_USB_NEED_CLK_INT);
> -	start_int_mask(SE_USB_AHB_NEED_CLK_INT);
> +	if (machine_is_pnx4008()) {

The function is called "pnx4008...", so why would any code path that is
not this hardware call this?

I think there's a disconnect between what you think a pnx4008 machine
is, and what I think it is, or what this code is running on and thinks
it is.

So, should this function just be called something else, for the type of
hardware (lpc32xx?), and then do this check within the function?

Again, consistancy is key.  Note how not all of the pnc4008_*()
functions make this check, which implies that something is not
consistant here...

thanks,

greg k-h
--
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/