In article <43FE217C.7000708@tadpole.com>
garrett_damore@tadpole.com:

> This looks good.  One obvious point, you misspelled "unable" in the
> error message below.  (search for "unbale").  :-)  Also there was an
> indent error in aupsc_attach (see the line that starts bus_space_write.)

More cosmetics:
- some more space/tab pasto
- use uintXX_t rather than u_intXX_t
- no parenthesis are needed for return values
  (even if other sources have them ;-)

> What exactly are you planning on locking, though?  It looks to me like
> the registers for each PSC are independent from each other, so you
> shouldn't need to lock them.

Agreed. If the lock is really needed, it could be done in
each lower device (smbus etc.).

> Hmm... I'm also not sure about the clock selection logic in general. 
> How do you know which clock to select?  That seems to be a board
> specific configuration option.  Maybe another set of board_info() ops? 
> Or another locator?  I don't know what the right option is.

Maybe we should think how much board specific clock variables
are needed:
- master clock frequency for internal clock source (peripheral clock?)
- PSCn_EXTCLK frequencies
- etc.
Or we should also have devprop_set() (or so) to define frequencies
and divider values for clock generater etc. in board specific
init functions?

> (In any
> case,  it seems likely that you need expose some aupsc operations to
> individual drivers -- e.g. aupsc_enable()/disable()/suspend(),
> aupsc_set_protocol(), aupsc_set_clock() maybe?

For now, we don't have to have enable/disable/suspend, and
set_protocol() and set_clock() can be done in attach functions
in children, I think.
---
Izumi Tsutsui