Re: [wip patch] allwinner: fix attachment of auxiliary UARTs

[Sujit K M <sjt.kar%gmail.com@localhost>]

On Wed, Jul 1, 2015 at 7:11 PM, Tobias Nygren <tnn%netbsd.org@localhost> wrote:
> Hi!
>
> I'm trying to get the physically routed aux com ports working on a
> Banana Pi board. The attached patch fixes com1 and com2 attachment and
> enables clock gating to the point that the ports are correctly probed
> and attached.

 I am looking at the attached patch. One thing that strikes me is you are not
using any refactoring. If you look at the code snippet to Clock Gating it has
an hardcoded if/else block.

> There are a few issues though:
>
> 1) I don't understand why the third argument to bus_space_subregion in
> awin_com_match needs to be divided by 4. It is already done in
> awin_com_attach and com ports are not correctly probed without it.

What are being probed in the awin_com_attach. Could you please clarify.

>
> 2) timeout waiting for BUSY ack. Probably need to unmask some interrupt
> somewhere. (Any clues?)

http://netbsd.gw.com/cgi-bin/man-cgi?plip+4+NetBSD-current

>
> 3) Should I bother to shut down the clocks in _match after probing and
> re-enable them in _attach?
>
> com0 at awinio0 port 0: ns16550a, working fifo
> com0: console
> com1 at awinio0 port 3: ns16550a, working fifo
> com2 at awinio0 port 7: ns16550a, working fifo

Do you believe in kernel programming is this a good practise or substantiate
why you are taking this approach?