Re: How to implement PSC driver on Au1550

Izumi Tsutsui <[email protected]>
Newsgroups gmane.os.netbsd.ports.evbmips
Message-ID <[email protected]>
In article <[email protected]>
[email protected]:

> 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.