Subject: Re: [PATCH 2/5] [POWERPC] QE: add support for QE USB clocks routing

From: Timur Tabi <[email-masked]>
Date: Thu, 17 Apr 2008 14:56:42 -0500

Anton Vorontsov wrote:

> +config QE_USB
> +	bool
> +	help
> +	  QE USB Host Controller support

Don't you need something like "depends on USB"?  I would assume that you need
core USB support before any USB drivers can be loaded.

> --- /dev/null
> +++ b/arch/powerpc/sysdev/qe_lib/usb.c
> @@ -0,0 +1,57 @@
> +/*
> + * QE USB routines
> + *
> + * Copyright (c) Freescale Semicondutor, Inc. 2006.
> + *               Shlomi Gridish <[email-masked]>

Is this code from one of our BSPs?

> +int qe_usb_clock_set(enum qe_clock clk, int rate)
> +{
> +	struct qe_mux __iomem *mux = &qe_immr->qmx;
> +	unsigned long flags;
> +	const bool is_brg = clk < QE_CLK1;

You don't really need "is_brg", since it's only used once.

> +	spin_lock_irqsave(&cmxgcr_lock, flags);
> +
> +	clrsetbits_be32(&mux->cmxgcr, QE_CMXGCR_USBCS, val);

Would it be useful if I made the clrsetbits functions atomic, using lwarx/stwcx.?

-- 
Timur Tabi
Linux kernel developer at Freescale