Messages in this thread | | | Date | Wed, 5 Nov 2014 11:09:12 +0100 | From | Maxime Ripard <> | Subject | Re: [PATCH 1/6] clk: sunxi: Add support for sun9i a80 usb clocks and resets |
| |
On Wed, Nov 05, 2014 at 06:02:35PM +0800, Chen-Yu Tsai wrote: > >> +static void __init sunxi_usb_clk_setup(struct device_node *node, > >> + const struct usb_clk_data *data, > >> + spinlock_t *lock) > >> +{ > >> + struct clk_onecell_data *clk_data; > >> + struct usb_reset_data *reset_data; > >> + const char *clk_parent; > >> + const char *clk_name; > >> + void __iomem *reg; > >> + int qty; > >> + int i = 0; > >> + int j = 0; > >> + > >> + reg = of_iomap(node, 0); > > > > of_io_request_and_map? > > OK. About that, any recommended naming style for the 3rd argument? > Maybe the driver name "clk_sun9i_usb"? Or just a generic name like > "usb_clk"? > > I'm asking now as we'll likely be changing the existing drivers to > use it as well.
I don't really have a preference. Maybe the DT node name would be both the easier and better solution.
[...]
> >> +static void __init sun9i_a80_usb_mod_setup(struct device_node *node) > >> +{ > >> + /* AHB1 gate must be enabled to access registers */ > >> + struct clk *ahb = of_clk_get(node, 0); > >> + > >> + WARN_ON(IS_ERR(ahb)); > >> + clk_prepare_enable(ahb); > > > > Hmmmm. That look off. > > > > Why do you need the clock to be enabled all the time? Isn't the CCF > > already taking care of enabling the parent clock whenever it needs to > > access any register? > > There are also resets in the same block. That and I couldn't get it > working without enabling the clock beforehand.
Ah, right.
What happens if you just enable and disable the clocks in the reset_assert and reset_deassert right before and after accessing the registers?
Maxime
-- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com [unhandled content-type:application/pgp-signature] | |