Shigeyuki Fukushima wrote:
> Thanks, Garett and Tsutsui!
>
> Garrett D'Amore wrote:
>   
>> 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.)
>>     
>
> Thank you for your advice. I'll fix.
>
>   
>> 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.
>>     
>
> For now, we can't decide it. I'll remove at once.
>
>   
>> 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.
>>     
>
> I think so. We may need to prepare board-specific clock configuration
> if BIOS(ROM monitor) such as YAMON and so didn't/couldn't initialize
> registers related with clock.
>
>   
>> (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?
>>     
>
>
> Izumi Tsutsui wrote:
>   
> Thank you for your advice. I'll fix it.
>
>   
>> 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?
>>     
>
> Maybe OMS-AL400 uses internal clock source (pscn_intclk).
> On OMS-AL400, Initial value of PSC_SEL register is 0x0.
>   
Also on dbau1550.  However, I'm not convinced that this value was chosen
as the "correct"  solution rather than to just start the PSCs off in
some known disabled state.

It might be worth looking at the Linux code for them.

I liked the idea of devprop to configure this.  Maybe in the absence of
properties default to internal clocking?

    -- Garrett
>   
>>> (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.
>>     
>
I don't think you can put this problem off -- you need them as soon as
you implement the first ausmbus or whatever protocol.

-- 
Garrett D'Amore, Principal Software Engineer
Tadpole Computer / Computing Technologies Division,
General Dynamics C4 Systems
http://www.tadpolecomputer.com/
Phone: 951 325-2134  Fax: 951 325-2191