Beaglebone Serial Ports Progress

[Warner Losh]

On Feb 17, 2013, at 11:21 AM, Tim Kientzle wrote:

> 
> On Feb 17, 2013, at 1:49 AM, Iain Young wrote:
> 
>> Hi Damjan,
>> 
>> On 15/02/13 16:46, You wrote:
>> 
>>>> 
>>>> I tried changing 0x1000 for 0x2000, as it seems the next reg is also
>>>> reserved for UART1. No more luck. So, thinking I needed to add it as an
>>>> alias (as UART0 is), I added that, but it still dumped me at the
>>>> debugger on boot.
>>>> 
>>>> The only other thing is reg-shift. I must confess, I am a bit blind
>>>> here. Not knowing what it actually did I left it as with UART0. I'm
>>>> hoping it essentially includes the next register up for UART1, as
>>>> while that's listed as "Reserved" in the memory map [Yes, I consulted
>>>> SPRUH73G :)] , it seems to be reserved for UART1, but I am just
>>>> guessing (Yes, I know, not good practice when kernel hacking...)
>>>> 
>>>> I've attached the latest version of my patch, the output from the
>>>> kernel until it blows up, as well as the trace. Patch is based on
>>>> r246610
>>>> 
>>>> Anyone able to point me in the right direction ? I can't be too far
>>>> away, and I can then add UART2-5, and submit an actual working patch!
>>>> 
>>> 
>>> It is very likely that clock is disabled for USART1. Problem is that usart uses
>>> standard serial driver which is not requesting clock to be enabled during the attach
>>> by invoking ti_prcm_clk_enable().
>>> 
>>> Can you try to put following at the end of am335x_prcm_attach()?
>>> 
>>> 	prcm_write_4(6c, 2);
>>> 
>>> 
>>> This should be register CM_PER_UART1_CLKCTRL.
>> 
>> That indeed fixed it, and adding the other CLKCTRL registers in a
>> similar way enabled them as well.
>> 
>> Have not been able to test fully, as my GPS units are 70 ft away in the
>> shed at the end of the garden, however, my radio clock receivers are now
>> happily twiddling CTS on UARTS3 and 4 (also tested on UART1), so I'm
>> pretty sure all UARTS are fine.
>> 
>> 
>> Before I send the full patch what are the FreeBSD standards wrt the
>> above lines in am335x_prcm_attach() ? Should I leave as is ? Create
>> a separate function, and call it from am335x_prcm_attach() ? Or do I
>> create a #define in the same file, and create functions for each UART,
>> as I see some other clocks do ?
> 
> It would require a little more work but I think the natural
> place to put this would be to put
> 
>    uart1: serial at 48022000 {
>       ...
>       clocks = "UART1";
>       clock-parent = <&PRCM>;
>    };
> 
> in the FDT (which is a straightforward way of
> saying "this device needs the PRCM to turn on
> "UART1" clocks) and then figure out how to
> actually support it.  ;-)
> 
> This is the same issue that we've been discussing
> for pinmux, and the above is essentially the same solution
> being discussed for pinmux.
> 
> It's not yet clear to me where/how this info should be acted on.
> It could actually be handled in the simplebus driver, I
> think, without modifying (in this case) the UART driver code
> at all.   That would need only a standard way for simplebus to
> communicate the clock-init string to the designated
> clock handler.

Yes. We also need to augment our clock support a bit too...  In linux land, which may prove illustrative, you create the clocks as part of the SoC, associate names in the FDT and then each device requests the proper clock from their FDT node. There is (or was a few months ago) a move afoot to make this more automatic by default.

> The current handling for interrupt and memory resources
> could also be used as a model, though that might
> require modifying each driver to manage pinmux and
> clock resources.
> 
> The nicest approach might be for simplebus to
> handle the clock-init key transparently in
> the many cases where there's a single clock and
> the driver doesn't need any special management
> and then provide a separate internal API for drivers
> that want to manage multiple clock modes or enable/disable
> clocks dynamically.

I rather like this idea, frankly...

Warner