Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 17 Feb 2013 11:53:53 -0700
From:      Warner Losh <imp@bsdimp.com>
To:        Tim Kientzle <tim@kientzle.com>
Cc:        freebsd-arm@freebsd.org, Damjan Marion <dmarion@freebsd.org>
Subject:   Re: Beaglebone Serial Ports Progress
Message-ID:  <9849AAD0-1C71-487D-A0C7-582FAD95FD41@bsdimp.com>
In-Reply-To: <73B8F088-EE5C-471D-AEED-52D4E45153DA@kientzle.com>
References:  <511E1A08.4020105@g7iii.net> <C54B5EBA-84A1-4BD9-AD7B-B61D7401E4F1@freebsd.org> <5120A7AD.40903@g7iii.net> <73B8F088-EE5C-471D-AEED-52D4E45153DA@kientzle.com>

next in thread | previous in thread | raw e-mail | index | archive | help

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

>=20
> On Feb 17, 2013, at 1:49 AM, Iain Young wrote:
>=20
>> Hi Damjan,
>>=20
>> On 15/02/13 16:46, You wrote:
>>=20
>>>>=20
>>>> 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.
>>>>=20
>>>> 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...)
>>>>=20
>>>> 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
>>>>=20
>>>> 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!
>>>>=20
>>>=20
>>> 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().
>>>=20
>>> Can you try to put following at the end of am335x_prcm_attach()?
>>>=20
>>> 	prcm_write_4(6c, 2);
>>>=20
>>>=20
>>> This should be register CM_PER_UART1_CLKCTRL.
>>=20
>> That indeed fixed it, and adding the other CLKCTRL registers in a
>> similar way enabled them as well.
>>=20
>> 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.
>>=20
>>=20
>> 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 ?
>=20
> It would require a little more work but I think the natural
> place to put this would be to put
>=20
>    uart1: serial@48022000 {
>       ...
>       clocks =3D "UART1";
>       clock-parent =3D <&PRCM>;
>    };
>=20
> 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.  ;-)
>=20
> This is the same issue that we've been discussing
> for pinmux, and the above is essentially the same solution
> being discussed for pinmux.
>=20
> 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.
>=20
> 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=



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?9849AAD0-1C71-487D-A0C7-582FAD95FD41>