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