diff mbox

arm64: dts: add all hi6220 uart nodes

Message ID 1443558552-5734-1-git-send-email-tyler.baker@linaro.org
State Superseded
Headers show

Commit Message

Tyler Baker Sept. 29, 2015, 8:29 p.m. UTC
This patch adds all UART nodes for the Hi6220 SoC. Recently a board[1] has
been developed to standardize UART access across all the 96boards consumer
edition boards. To use this hardware on HiKey we must configure and enable
UART3. However, to ensure backward compatibility we must keep UART0 enabled
as well.

I have removed the hard coded clock index values in favor of using the ones
already defined in include/dt-bindings/clock/hi6220-clock.h.

This patch was boot tested on top of next-20150929, with both UART
configurations.

[1] http://www.seeedstudio.com/depot/96Boards-UART-p-2525.html?ref=newInBazaar

Signed-off-by: Tyler Baker <tyler.baker@linaro.org>
---
 arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts |  4 +++
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi      | 43 +++++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 1 deletion(-)

Comments

Arnd Bergmann Sept. 30, 2015, 8:24 a.m. UTC | #1
On Tuesday 29 September 2015 13:29:12 Tyler Baker wrote:
>         aliases {
>                 serial0 = &uart0;
> +               serial1 = &uart1;
> +               serial2 = &uart2;
> +               serial3 = &uart3;
> +               serial4 = &uart4;
>         };
> 

In the changelog you mention "both uarts", but here you have five of them.
Are they all accessible on the connector? If not, only provide aliases
for the ones that are, using numbering that makes most sense for given
how one would use the board.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Sept. 30, 2015, 5:31 p.m. UTC | #2
On Wed, Sep 30, 2015 at 10:24:56AM +0200, Arnd Bergmann wrote:
> On Tuesday 29 September 2015 13:29:12 Tyler Baker wrote:

> >         aliases {
> >                 serial0 = &uart0;
> > +               serial1 = &uart1;
> > +               serial2 = &uart2;
> > +               serial3 = &uart3;
> > +               serial4 = &uart4;
> >         };

> In the changelog you mention "both uarts", but here you have five of them.
> Are they all accessible on the connector? If not, only provide aliases
> for the ones that are, using numbering that makes most sense for given
> how one would use the board.

Unless I'm missing something there's only two UARTs brought out on the
low speed expansion connector (in addition to the one on the solder pads
which is currently supported).  We should also adjust the console
default to match whatever one of the low speed expansion connector UARTs
is being used by the bootloader.

While we're at it there was a recent talk which mentioned a fairly large
amount of functionality that's apparently already "upstream" for this
device but not included in the DT, assuming that means that driver
support is there it would be good to add the corresponding DT.
Tyler Baker Sept. 30, 2015, 6:12 p.m. UTC | #3
On 30 September 2015 at 10:31, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Sep 30, 2015 at 10:24:56AM +0200, Arnd Bergmann wrote:
>> On Tuesday 29 September 2015 13:29:12 Tyler Baker wrote:
>
>> >         aliases {
>> >                 serial0 = &uart0;
>> > +               serial1 = &uart1;
>> > +               serial2 = &uart2;
>> > +               serial3 = &uart3;
>> > +               serial4 = &uart4;
>> >         };
>
>> In the changelog you mention "both uarts", but here you have five of them.
>> Are they all accessible on the connector? If not, only provide aliases
>> for the ones that are, using numbering that makes most sense for given
>> how one would use the board.

Thanks for the comment Arnd. Mark's comment below is correct, there
are only two UARTs accessible on the LS connection in addition to the
one on the board (solder pad).

Is the following definition any clearer?

serial0 = &uart0; // Onboard UART0
serial1 = &uart2; // LS expansion UART0
serial2 = &uart3; // LS expansion UART1

If so, I'll respin this patch.

> Unless I'm missing something there's only two UARTs brought out on the
> low speed expansion connector (in addition to the one on the solder pads
> which is currently supported).  We should also adjust the console
> default to match whatever one of the low speed expansion connector UARTs
> is being used by the bootloader.

Your not missing anything, I should not have added the additional
aliases, it is confusing, will remove. The UART boards by default come
configured to use UART1 on the LS connector.

+ Peter as he has been submitting u-boot patches recently for the HiKey.

Obviously, both UEFI and u-boot can be configured to use either UART,
and at the moment u-boot defaults to using the on board UART. Whereas
UEFI is using UART1 on the LS connector. I'm fine with switching the
console default to use the UART1 on the LS connector as long as there
is agreement to do so.

> While we're at it there was a recent talk which mentioned a fairly large
> amount of functionality that's apparently already "upstream" for this
> device but not included in the DT, assuming that means that driver
> support is there it would be good to add the corresponding DT.

Indeed, I think this comment is targeted to toward the d410c though as
there are drivers for eMMC, and microsd drivers upstream but no DT
bindings. I do not believe currently this is the case for HiKey.

Cheers,

Tyler
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring Sept. 30, 2015, 7:03 p.m. UTC | #4
On Wed, Sep 30, 2015 at 1:12 PM, Tyler Baker <tyler.baker@linaro.org> wrote:
> On 30 September 2015 at 10:31, Mark Brown <broonie@kernel.org> wrote:
>> On Wed, Sep 30, 2015 at 10:24:56AM +0200, Arnd Bergmann wrote:
>>> On Tuesday 29 September 2015 13:29:12 Tyler Baker wrote:
>>
>>> >         aliases {
>>> >                 serial0 = &uart0;
>>> > +               serial1 = &uart1;
>>> > +               serial2 = &uart2;
>>> > +               serial3 = &uart3;
>>> > +               serial4 = &uart4;
>>> >         };
>>
>>> In the changelog you mention "both uarts", but here you have five of them.
>>> Are they all accessible on the connector? If not, only provide aliases
>>> for the ones that are, using numbering that makes most sense for given
>>> how one would use the board.
>
> Thanks for the comment Arnd. Mark's comment below is correct, there
> are only two UARTs accessible on the LS connection in addition to the
> one on the board (solder pad).
>
> Is the following definition any clearer?
>
> serial0 = &uart0; // Onboard UART0
> serial1 = &uart2; // LS expansion UART0
> serial2 = &uart3; // LS expansion UART1

Yes, but use C style comments.

What about the BT UART?

>
> If so, I'll respin this patch.
>
>> Unless I'm missing something there's only two UARTs brought out on the
>> low speed expansion connector (in addition to the one on the solder pads
>> which is currently supported).  We should also adjust the console
>> default to match whatever one of the low speed expansion connector UARTs
>> is being used by the bootloader.
>
> Your not missing anything, I should not have added the additional
> aliases, it is confusing, will remove. The UART boards by default come
> configured to use UART1 on the LS connector.

Also, the console default should be set in chosen stdout-path so we
can move away from having to set it on the kernel command line. Then
the aliases don't matter so much, but there may be some work to do to
setup a getty correctly.

> + Peter as he has been submitting u-boot patches recently for the HiKey.
>
> Obviously, both UEFI and u-boot can be configured to use either UART,
> and at the moment u-boot defaults to using the on board UART. Whereas
> UEFI is using UART1 on the LS connector. I'm fine with switching the
> console default to use the UART1 on the LS connector as long as there
> is agreement to do so.

BTW, u-boot can output to multiple serial ports. I think I gave Peter
details on how to do it.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Sept. 30, 2015, 7:18 p.m. UTC | #5
On Wed, Sep 30, 2015 at 11:12:58AM -0700, Tyler Baker wrote:
> On 30 September 2015 at 10:31, Mark Brown <broonie@kernel.org> wrote:

> + Peter as he has been submitting u-boot patches recently for the HiKey.

> Obviously, both UEFI and u-boot can be configured to use either UART,
> and at the moment u-boot defaults to using the on board UART. Whereas
> UEFI is using UART1 on the LS connector. I'm fine with switching the
> console default to use the UART1 on the LS connector as long as there
> is agreement to do so.

Given that UEFI is switching and the requirement for soldering to get to
the on board UART it does seem to make sense, though it will create some
pain for current users.

> > While we're at it there was a recent talk which mentioned a fairly large
> > amount of functionality that's apparently already "upstream" for this
> > device but not included in the DT, assuming that means that driver
> > support is there it would be good to add the corresponding DT.

> Indeed, I think this comment is targeted to toward the d410c though as
> there are drivers for eMMC, and microsd drivers upstream but no DT
> bindings. I do not believe currently this is the case for HiKey.

My understanding is that it applies to both with slightly different
driver sets, the slides aren't online yet (!) but if you look at 10:52
or so in the video we should have SD/eMMC on HiKey in v4.1 with thermal
and CPUfreq added in v4.2.  It also looks like there's just some DT
additions needed to enable reset, cpuidle and CPU hotplug using generic
code (mainly the constants for PSCI I expect).
Tyler Baker Sept. 30, 2015, 11:51 p.m. UTC | #6
On 30 September 2015 at 12:03, Rob Herring <robherring2@gmail.com> wrote:
> On Wed, Sep 30, 2015 at 1:12 PM, Tyler Baker <tyler.baker@linaro.org> wrote:
>> On 30 September 2015 at 10:31, Mark Brown <broonie@kernel.org> wrote:
>>> On Wed, Sep 30, 2015 at 10:24:56AM +0200, Arnd Bergmann wrote:
>>>> On Tuesday 29 September 2015 13:29:12 Tyler Baker wrote:
>>>
>>>> >         aliases {
>>>> >                 serial0 = &uart0;
>>>> > +               serial1 = &uart1;
>>>> > +               serial2 = &uart2;
>>>> > +               serial3 = &uart3;
>>>> > +               serial4 = &uart4;
>>>> >         };
>>>
>>>> In the changelog you mention "both uarts", but here you have five of them.
>>>> Are they all accessible on the connector? If not, only provide aliases
>>>> for the ones that are, using numbering that makes most sense for given
>>>> how one would use the board.
>>
>> Thanks for the comment Arnd. Mark's comment below is correct, there
>> are only two UARTs accessible on the LS connection in addition to the
>> one on the board (solder pad).
>>
>> Is the following definition any clearer?
>>
>> serial0 = &uart0; // Onboard UART0
>> serial1 = &uart2; // LS expansion UART0
>> serial2 = &uart3; // LS expansion UART1
>
> Yes, but use C style comments.

Ok.

> What about the BT UART?

According to the schematic it will use UART1, will add.

>> If so, I'll respin this patch.
>>
>>> Unless I'm missing something there's only two UARTs brought out on the
>>> low speed expansion connector (in addition to the one on the solder pads
>>> which is currently supported).  We should also adjust the console
>>> default to match whatever one of the low speed expansion connector UARTs
>>> is being used by the bootloader.
>>
>> Your not missing anything, I should not have added the additional
>> aliases, it is confusing, will remove. The UART boards by default come
>> configured to use UART1 on the LS connector.
>
> Also, the console default should be set in chosen stdout-path so we
> can move away from having to set it on the kernel command line. Then
> the aliases don't matter so much, but there may be some work to do to
> setup a getty correctly.

Ok.

Cheers,

Tyler
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tyler Baker Sept. 30, 2015, 11:54 p.m. UTC | #7
On 30 September 2015 at 12:18, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Sep 30, 2015 at 11:12:58AM -0700, Tyler Baker wrote:
>> On 30 September 2015 at 10:31, Mark Brown <broonie@kernel.org> wrote:
>
>> + Peter as he has been submitting u-boot patches recently for the HiKey.
>
>> Obviously, both UEFI and u-boot can be configured to use either UART,
>> and at the moment u-boot defaults to using the on board UART. Whereas
>> UEFI is using UART1 on the LS connector. I'm fine with switching the
>> console default to use the UART1 on the LS connector as long as there
>> is agreement to do so.
>
> Given that UEFI is switching and the requirement for soldering to get to
> the on board UART it does seem to make sense, though it will create some
> pain for current users.
>
>> > While we're at it there was a recent talk which mentioned a fairly large
>> > amount of functionality that's apparently already "upstream" for this
>> > device but not included in the DT, assuming that means that driver
>> > support is there it would be good to add the corresponding DT.
>
>> Indeed, I think this comment is targeted to toward the d410c though as
>> there are drivers for eMMC, and microsd drivers upstream but no DT
>> bindings. I do not believe currently this is the case for HiKey.
>
> My understanding is that it applies to both with slightly different
> driver sets, the slides aren't online yet (!) but if you look at 10:52
> or so in the video we should have SD/eMMC on HiKey in v4.1 with thermal
> and CPUfreq added in v4.2.  It also looks like there's just some DT
> additions needed to enable reset, cpuidle and CPU hotplug using generic
> code (mainly the constants for PSCI I expect).

I just skimmed the video and I see your point. I'll need to
investigate a bit but I'm up for sending some follow up patches to get
these drivers functioning. First lets get the UART working :)

Cheers,

Tyler
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Haojian Zhuang Oct. 1, 2015, 1:43 a.m. UTC | #8
On Thu, Oct 1, 2015 at 3:03 AM, Rob Herring <robherring2@gmail.com> wrote:
> On Wed, Sep 30, 2015 at 1:12 PM, Tyler Baker <tyler.baker@linaro.org> wrote:
>> On 30 September 2015 at 10:31, Mark Brown <broonie@kernel.org> wrote:
>>> On Wed, Sep 30, 2015 at 10:24:56AM +0200, Arnd Bergmann wrote:
>>>> On Tuesday 29 September 2015 13:29:12 Tyler Baker wrote:
>>>
>>>> >         aliases {
>>>> >                 serial0 = &uart0;
>>>> > +               serial1 = &uart1;
>>>> > +               serial2 = &uart2;
>>>> > +               serial3 = &uart3;
>>>> > +               serial4 = &uart4;
>>>> >         };
>>>
>>>> In the changelog you mention "both uarts", but here you have five of them.
>>>> Are they all accessible on the connector? If not, only provide aliases
>>>> for the ones that are, using numbering that makes most sense for given
>>>> how one would use the board.
>>
>> Thanks for the comment Arnd. Mark's comment below is correct, there
>> are only two UARTs accessible on the LS connection in addition to the
>> one on the board (solder pad).
>>
>> Is the following definition any clearer?
>>
>> serial0 = &uart0; // Onboard UART0
>> serial1 = &uart2; // LS expansion UART0
>> serial2 = &uart3; // LS expansion UART1
>
> Yes, but use C style comments.
>
> What about the BT UART?
>

Yes, all four uarts are used in hikey board. uar0 is onboard serial
uart. uart1 is
BT uart. uart2 & uart3 are connected to low speed expansion board. So Tyler
shouldn't decrease any uart from his current patch.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leo Yan Oct. 1, 2015, 3:15 a.m. UTC | #9
Hi Tyler, Mark,

On Wed, Sep 30, 2015 at 04:54:51PM -0700, Tyler Baker wrote:
> On 30 September 2015 at 12:18, Mark Brown <broonie@kernel.org> wrote:
> > On Wed, Sep 30, 2015 at 11:12:58AM -0700, Tyler Baker wrote:
> >> On 30 September 2015 at 10:31, Mark Brown <broonie@kernel.org> wrote:
> >
> >> + Peter as he has been submitting u-boot patches recently for the HiKey.
> >
> >> Obviously, both UEFI and u-boot can be configured to use either UART,
> >> and at the moment u-boot defaults to using the on board UART. Whereas
> >> UEFI is using UART1 on the LS connector. I'm fine with switching the
> >> console default to use the UART1 on the LS connector as long as there
> >> is agreement to do so.
> >
> > Given that UEFI is switching and the requirement for soldering to get to
> > the on board UART it does seem to make sense, though it will create some
> > pain for current users.
> >
> >> > While we're at it there was a recent talk which mentioned a fairly large
> >> > amount of functionality that's apparently already "upstream" for this
> >> > device but not included in the DT, assuming that means that driver
> >> > support is there it would be good to add the corresponding DT.
> >
> >> Indeed, I think this comment is targeted to toward the d410c though as
> >> there are drivers for eMMC, and microsd drivers upstream but no DT
> >> bindings. I do not believe currently this is the case for HiKey.
> >
> > My understanding is that it applies to both with slightly different
> > driver sets, the slides aren't online yet (!) but if you look at 10:52
> > or so in the video we should have SD/eMMC on HiKey in v4.1 with thermal
> > and CPUfreq added in v4.2.  It also looks like there's just some DT
> > additions needed to enable reset, cpuidle and CPU hotplug using generic
> > code (mainly the constants for PSCI I expect).
> 
> I just skimmed the video and I see your point. I'll need to
> investigate a bit but I'm up for sending some follow up patches to get
> these drivers functioning. First lets get the UART working :)

For Hikey's PM, related patches are ready for upstreaming [1].

Previously i'm waiting for merging mailbox driver, after that i plan
to send out all DT patches to enable cpuidle/cpufreq/thermal/suspend
in one going. Now pending on mailbox driver's merging.

If you think it's necessary, i could send out patches to enable cpuidle
firstly (actually now PSCI has been enabled yet); for cpufreq and
thermal features, need wait until mailbox driver has been merged.

[1] https://github.com/Leo-Yan/linux/commits/tracking-hikey-pm

Thanks,
Leo Yan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Oct. 1, 2015, 10:21 a.m. UTC | #10
On Thu, Oct 01, 2015 at 11:15:52AM +0800, Leo Yan wrote:
> On Wed, Sep 30, 2015 at 04:54:51PM -0700, Tyler Baker wrote:
> > On 30 September 2015 at 12:18, Mark Brown <broonie@kernel.org> wrote:

> > > My understanding is that it applies to both with slightly different
> > > driver sets, the slides aren't online yet (!) but if you look at 10:52
> > > or so in the video we should have SD/eMMC on HiKey in v4.1 with thermal
> > > and CPUfreq added in v4.2.  It also looks like there's just some DT
> > > additions needed to enable reset, cpuidle and CPU hotplug using generic
> > > code (mainly the constants for PSCI I expect).

> > I just skimmed the video and I see your point. I'll need to
> > investigate a bit but I'm up for sending some follow up patches to get
> > these drivers functioning. First lets get the UART working :)

> If you think it's necessary, i could send out patches to enable cpuidle
> firstly (actually now PSCI has been enabled yet); for cpufreq and
> thermal features, need wait until mailbox driver has been merged.

It seems sensible to get the DT bits for functionality that is already
supported on the driver side upstream, that way we get the maximum
functionality available as soon as possible and even if other bits get
problems in review users can take advantage of things that are OK.
Leo Yan Oct. 2, 2015, 2:28 p.m. UTC | #11
On Thu, Oct 01, 2015 at 11:21:36AM +0100, Mark Brown wrote:
> On Thu, Oct 01, 2015 at 11:15:52AM +0800, Leo Yan wrote:
> > On Wed, Sep 30, 2015 at 04:54:51PM -0700, Tyler Baker wrote:
> > > On 30 September 2015 at 12:18, Mark Brown <broonie@kernel.org> wrote:
> 
> > > > My understanding is that it applies to both with slightly different
> > > > driver sets, the slides aren't online yet (!) but if you look at 10:52
> > > > or so in the video we should have SD/eMMC on HiKey in v4.1 with thermal
> > > > and CPUfreq added in v4.2.  It also looks like there's just some DT
> > > > additions needed to enable reset, cpuidle and CPU hotplug using generic
> > > > code (mainly the constants for PSCI I expect).
> 
> > > I just skimmed the video and I see your point. I'll need to
> > > investigate a bit but I'm up for sending some follow up patches to get
> > > these drivers functioning. First lets get the UART working :)
> 
> > If you think it's necessary, i could send out patches to enable cpuidle
> > firstly (actually now PSCI has been enabled yet); for cpufreq and
> > thermal features, need wait until mailbox driver has been merged.
> 
> It seems sensible to get the DT bits for functionality that is already
> supported on the driver side upstream, that way we get the maximum
> functionality available as soon as possible and even if other bits get
> problems in review users can take advantage of things that are OK.

Okay, i will prepare patches for enabling CPUIdle ASAP.

Thanks,
Leo Yan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Oct. 2, 2015, 4:48 p.m. UTC | #12
On Fri, Oct 02, 2015 at 10:28:09PM +0800, Leo Yan wrote:
> On Thu, Oct 01, 2015 at 11:21:36AM +0100, Mark Brown wrote:

> > It seems sensible to get the DT bits for functionality that is already
> > supported on the driver side upstream, that way we get the maximum
> > functionality available as soon as possible and even if other bits get
> > problems in review users can take advantage of things that are OK.

> Okay, i will prepare patches for enabling CPUIdle ASAP.

Excellent, thanks!
Peter Griffin Oct. 2, 2015, 6 p.m. UTC | #13
Hi Tyler,

On Wed, 30 Sep 2015, Tyler Baker wrote:

> On 30 September 2015 at 10:31, Mark Brown <broonie@kernel.org> wrote:
> > On Wed, Sep 30, 2015 at 10:24:56AM +0200, Arnd Bergmann wrote:
> >> On Tuesday 29 September 2015 13:29:12 Tyler Baker wrote:
> >
> >> >         aliases {
> >> >                 serial0 = &uart0;
> >> > +               serial1 = &uart1;
> >> > +               serial2 = &uart2;
> >> > +               serial3 = &uart3;
> >> > +               serial4 = &uart4;
> >> >         };
> >
> >> In the changelog you mention "both uarts", but here you have five of them.
> >> Are they all accessible on the connector? If not, only provide aliases
> >> for the ones that are, using numbering that makes most sense for given
> >> how one would use the board.
> 
> Thanks for the comment Arnd. Mark's comment below is correct, there
> are only two UARTs accessible on the LS connection in addition to the
> one on the board (solder pad).
> 
> Is the following definition any clearer?
> 
> serial0 = &uart0; // Onboard UART0
> serial1 = &uart2; // LS expansion UART0
> serial2 = &uart3; // LS expansion UART1
> 
> If so, I'll respin this patch.
> 
> > Unless I'm missing something there's only two UARTs brought out on the
> > low speed expansion connector (in addition to the one on the solder pads
> > which is currently supported).  We should also adjust the console
> > default to match whatever one of the low speed expansion connector UARTs
> > is being used by the bootloader.
> 
> Your not missing anything, I should not have added the additional
> aliases, it is confusing, will remove. The UART boards by default come
> configured to use UART1 on the LS connector.
> 
> + Peter as he has been submitting u-boot patches recently for the HiKey.

Thanks :)

> 
> Obviously, both UEFI and u-boot can be configured to use either UART,
> and at the moment u-boot defaults to using the on board UART. Whereas
> UEFI is using UART1 on the LS connector. I'm fine with switching the
> console default to use the UART1 on the LS connector as long as there
> is agreement to do so.

Eek... "serial2 = uart3 //UART1"

I sent the following patch
http://lists.denx.de/pipermail/u-boot/2015-September/227465.html

which has been accepted which switches from using UART0 (onboard) to
UART3 (LS connector). This matched up with the change made in ATF i.e. both ATF
and u-boot were then both outputting on the same UART.

So u-boot has already been migrated over.

regards,

Peter.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tyler Baker Oct. 6, 2015, 2:29 p.m. UTC | #14
On 2 October 2015 at 11:00, Peter Griffin <peter.griffin@linaro.org> wrote:
> Hi Tyler,
>
> On Wed, 30 Sep 2015, Tyler Baker wrote:
>
>> On 30 September 2015 at 10:31, Mark Brown <broonie@kernel.org> wrote:
>> > On Wed, Sep 30, 2015 at 10:24:56AM +0200, Arnd Bergmann wrote:
>> >> On Tuesday 29 September 2015 13:29:12 Tyler Baker wrote:
>> >
>> >> >         aliases {
>> >> >                 serial0 = &uart0;
>> >> > +               serial1 = &uart1;
>> >> > +               serial2 = &uart2;
>> >> > +               serial3 = &uart3;
>> >> > +               serial4 = &uart4;
>> >> >         };
>> >
>> >> In the changelog you mention "both uarts", but here you have five of them.
>> >> Are they all accessible on the connector? If not, only provide aliases
>> >> for the ones that are, using numbering that makes most sense for given
>> >> how one would use the board.
>>
>> Thanks for the comment Arnd. Mark's comment below is correct, there
>> are only two UARTs accessible on the LS connection in addition to the
>> one on the board (solder pad).
>>
>> Is the following definition any clearer?
>>
>> serial0 = &uart0; // Onboard UART0
>> serial1 = &uart2; // LS expansion UART0
>> serial2 = &uart3; // LS expansion UART1
>>
>> If so, I'll respin this patch.
>>
>> > Unless I'm missing something there's only two UARTs brought out on the
>> > low speed expansion connector (in addition to the one on the solder pads
>> > which is currently supported).  We should also adjust the console
>> > default to match whatever one of the low speed expansion connector UARTs
>> > is being used by the bootloader.
>>
>> Your not missing anything, I should not have added the additional
>> aliases, it is confusing, will remove. The UART boards by default come
>> configured to use UART1 on the LS connector.
>>
>> + Peter as he has been submitting u-boot patches recently for the HiKey.
>
> Thanks :)
>
>>
>> Obviously, both UEFI and u-boot can be configured to use either UART,
>> and at the moment u-boot defaults to using the on board UART. Whereas
>> UEFI is using UART1 on the LS connector. I'm fine with switching the
>> console default to use the UART1 on the LS connector as long as there
>> is agreement to do so.
>
> Eek... "serial2 = uart3 //UART1"

Yeah, I fixed this in v2 :)

> I sent the following patch
> http://lists.denx.de/pipermail/u-boot/2015-September/227465.html
>
> which has been accepted which switches from using UART0 (onboard) to
> UART3 (LS connector). This matched up with the change made in ATF i.e. both ATF
> and u-boot were then both outputting on the same UART.
>
> So u-boot has already been migrated over.

Excellent!

Cheers,

Tyler
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
index e36a539..34486bc 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
@@ -18,6 +18,10 @@ 
 
 	aliases {
 		serial0 = &uart0;
+		serial1 = &uart1;
+		serial2 = &uart2;
+		serial3 = &uart3;
+		serial4 = &uart4;
 	};
 
 	chosen {
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 3f03380..82d2488 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -5,6 +5,7 @@ 
  */
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/hi6220-clock.h>
 
 / {
 	compatible = "hisilicon,hi6220";
@@ -164,8 +165,48 @@ 
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x0 0xf8015000 0x0 0x1000>;
 			interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&ao_ctrl 36>, <&ao_ctrl 36>;
+			clocks = <&ao_ctrl HI6220_UART0_PCLK>,
+				 <&ao_ctrl HI6220_UART0_PCLK>;
 			clock-names = "uartclk", "apb_pclk";
 		};
+
+		uart1: uart@f7111000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x0 0xf7111000 0x0 0x1000>;
+			interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&sys_ctrl HI6220_UART1_PCLK>,
+				 <&sys_ctrl HI6220_UART1_PCLK>;
+			clock-names = "uartclk", "apb_pclk";
+			status = "disabled";
+		};
+
+		uart2: uart@f7112000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x0 0xf7112000 0x0 0x1000>;
+			interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&sys_ctrl HI6220_UART2_PCLK>,
+				 <&sys_ctrl HI6220_UART2_PCLK>;
+			clock-names = "uartclk", "apb_pclk";
+			status = "disabled";
+		};
+
+		uart3: uart@f7113000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x0 0xf7113000 0x0 0x1000>;
+			interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&sys_ctrl HI6220_UART3_PCLK>,
+				 <&sys_ctrl HI6220_UART3_PCLK>;
+			clock-names = "uartclk", "apb_pclk";
+		};
+
+		uart4: uart@f7114000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x0 0xf7114000 0x0 0x1000>;
+			interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&sys_ctrl HI6220_UART4_PCLK>,
+				 <&sys_ctrl HI6220_UART4_PCLK>;
+			clock-names = "uartclk", "apb_pclk";
+			status = "disabled";
+		};
 	};
 };