Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for UWP5661 SoC and IVY5661 board #20548

Open
wants to merge 7 commits into
base: master
from

Conversation

@Mani-Sadhasivam
Copy link
Collaborator

Mani-Sadhasivam commented Nov 9, 2019

Hello,

This is the second attempt on adding UNISOC UWP5661 SoC and 96Boards IVY5661 board support to zephyr. The first attempt was made by UNISOC (#10626) but the PR was closed and discontinued. Now, UNISOC has no intention to do the upstream by themselves but they are open for community contributions. Hence with their approval, I'm posting this PR and taken over the codebase from their previous PR. I've added my copyright for files I modified/created.

About SoC:

Unisoc UWP5661 is a highly integrated connectivity single chip which offers the lowest BOM in the industry for smart phone, PC, STB, OTT, IoT and automotive applications. This chip includes 2.4GHz and 5GHz WLAN IEEE 802.11 a/b/g/n/ac, Bluetooth 5.0 with supporting high power mode, Direction Finding and Long Range.

About Board:

96Boards IVY5661 is one of the IoT boards of the 96Boards family manufactured by UCRobotics. This board features UWP5661 SoC in 96Boards IoT form factor. More information about this board can be found in 96Boards website: https://www.96boards.org/product/ivy5661

The corresponding PR to add HAL is here: zephyrproject-rtos/hal_unisoc#1

@zephyrbot

This comment has been minimized.

Copy link
Collaborator

zephyrbot commented Nov 9, 2019

Some checks failed. Please fix and resubmit.

checkpatch (informational only, not a failure)

-:57: WARNING:UNDOCUMENTED_DT_STRING: DT compatible string "ucrobotics,ivy5661" appears un-documented -- check ./dts/bindings/
#57: FILE: boards/arm/96b_ivy5661/96b_ivy5661.dts:15:
+	compatible = "ucrobotics,ivy5661", "unisoc,uwp5661";

-:57: WARNING:UNDOCUMENTED_DT_STRING: DT compatible string "unisoc,uwp5661" appears un-documented -- check ./dts/bindings/
#57: FILE: boards/arm/96b_ivy5661/96b_ivy5661.dts:15:
+	compatible = "ucrobotics,ivy5661", "unisoc,uwp5661";

-:1191: WARNING:UNDOCUMENTED_DT_STRING: DT compatible string "serial-flash" appears un-documented -- check ./dts/bindings/
#1191: FILE: dts/arm/unisoc/uwp5661.dtsi:31:
+		compatible = "serial-flash";

- total: 0 errors, 3 warnings, 1427 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Nits issues


Please use this format for the header in 'boards/arm/96b_ivy5661/Kconfig.board' (see
https://docs.zephyrproject.org/latest/guides/kconfig/index.html#header-comments-and-other-nits):

    # <Overview of symbols defined in the file, preferably in plain English>
    (Blank line)
    # Copyright (c) 2019 ...
    # SPDX-License-Identifier: <License>
    (Blank line)
    (Kconfig definitions)

Skip the "Kconfig - " part of the first line, since it's clear that the comment
is about Kconfig from context. The "# Kconfig - " is what triggers this
failure.


Please use this format for the header in 'boards/arm/96b_ivy5661/Kconfig.defconfig' (see
https://docs.zephyrproject.org/latest/guides/kconfig/index.html#header-comments-and-other-nits):

    # <Overview of symbols defined in the file, preferably in plain English>
    (Blank line)
    # Copyright (c) 2019 ...
    # SPDX-License-Identifier: <License>
    (Blank line)
    (Kconfig definitions)

Skip the "Kconfig - " part of the first line, since it's clear that the comment
is about Kconfig from context. The "# Kconfig - " is what triggers this
failure.


Please use this format for the header in 'soc/arm/unisoc_uwp/Kconfig' (see
https://docs.zephyrproject.org/latest/guides/kconfig/index.html#header-comments-and-other-nits):

    # <Overview of symbols defined in the file, preferably in plain English>
    (Blank line)
    # Copyright (c) 2019 ...
    # SPDX-License-Identifier: <License>
    (Blank line)
    (Kconfig definitions)

Skip the "Kconfig - " part of the first line, since it's clear that the comment
is about Kconfig from context. The "# Kconfig - " is what triggers this
failure.


Please use this format for the header in 'soc/arm/unisoc_uwp/Kconfig.defconfig' (see
https://docs.zephyrproject.org/latest/guides/kconfig/index.html#header-comments-and-other-nits):

    # <Overview of symbols defined in the file, preferably in plain English>
    (Blank line)
    # Copyright (c) 2019 ...
    # SPDX-License-Identifier: <License>
    (Blank line)
    (Kconfig definitions)

Skip the "Kconfig - " part of the first line, since it's clear that the comment
is about Kconfig from context. The "# Kconfig - " is what triggers this
failure.


Please use this format for the header in 'soc/arm/unisoc_uwp/Kconfig.soc' (see
https://docs.zephyrproject.org/latest/guides/kconfig/index.html#header-comments-and-other-nits):

    # <Overview of symbols defined in the file, preferably in plain English>
    (Blank line)
    # Copyright (c) 2019 ...
    # SPDX-License-Identifier: <License>
    (Blank line)
    (Kconfig definitions)

Skip the "Kconfig - " part of the first line, since it's clear that the comment
is about Kconfig from context. The "# Kconfig - " is what triggers this
failure.


Please use this format for the header in 'soc/arm/unisoc_uwp/uwp566x/Kconfig.defconfig.series' (see
https://docs.zephyrproject.org/latest/guides/kconfig/index.html#header-comments-and-other-nits):

    # <Overview of symbols defined in the file, preferably in plain English>
    (Blank line)
    # Copyright (c) 2019 ...
    # SPDX-License-Identifier: <License>
    (Blank line)
    (Kconfig definitions)

Skip the "Kconfig - " part of the first line, since it's clear that the comment
is about Kconfig from context. The "# Kconfig - " is what triggers this
failure.


Please use this format for the header in 'soc/arm/unisoc_uwp/uwp566x/Kconfig.defconfig.uwp5661' (see
https://docs.zephyrproject.org/latest/guides/kconfig/index.html#header-comments-and-other-nits):

    # <Overview of symbols defined in the file, preferably in plain English>
    (Blank line)
    # Copyright (c) 2019 ...
    # SPDX-License-Identifier: <License>
    (Blank line)
    (Kconfig definitions)

Skip the "Kconfig - " part of the first line, since it's clear that the comment
is about Kconfig from context. The "# Kconfig - " is what triggers this
failure.


Please use this format for the header in 'soc/arm/unisoc_uwp/uwp566x/Kconfig.series' (see
https://docs.zephyrproject.org/latest/guides/kconfig/index.html#header-comments-and-other-nits):

    # <Overview of symbols defined in the file, preferably in plain English>
    (Blank line)
    # Copyright (c) 2019 ...
    # SPDX-License-Identifier: <License>
    (Blank line)
    (Kconfig definitions)

Skip the "Kconfig - " part of the first line, since it's clear that the comment
is about Kconfig from context. The "# Kconfig - " is what triggers this
failure.


Please use this format for the header in 'soc/arm/unisoc_uwp/uwp566x/Kconfig.soc' (see
https://docs.zephyrproject.org/latest/guides/kconfig/index.html#header-comments-and-other-nits):

    # <Overview of symbols defined in the file, preferably in plain English>
    (Blank line)
    # Copyright (c) 2019 ...
    # SPDX-License-Identifier: <License>
    (Blank line)
    (Kconfig definitions)

Skip the "Kconfig - " part of the first line, since it's clear that the comment
is about Kconfig from context. The "# Kconfig - " is what triggers this
failure.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

@Mani-Sadhasivam Mani-Sadhasivam force-pushed the Mani-Sadhasivam:uwp5661-upstream branch from cb50d99 to a0682c1 Nov 9, 2019
@Mani-Sadhasivam Mani-Sadhasivam force-pushed the Mani-Sadhasivam:uwp5661-upstream branch from a0682c1 to b85a825 Nov 13, 2019
@Mani-Sadhasivam Mani-Sadhasivam requested a review from dbkinder Nov 13, 2019
Copy link
Contributor

dbkinder left a comment

doc changes LGTM, thanks.

@pabigot

This comment has been minimized.

Copy link
Collaborator

pabigot commented Dec 23, 2019

I haven't looked closely at the port, but in preparation for doing so I have a couple questions:

  • The sole source for this processor appears to be 96Boards via Seeed, where it's out of stock. Is there reason to believe it will be possible for people to get their hands on this hardware?
  • #10626 does not include a reason for its being closed, but one blocker was absence of public documentation on the hardware. I don't see detailed information on the product page. Is hardware reference documentation going to become available?

Having recently spent time dealing with in-tree drivers for sensors that are so past end-of-life that the vendor doesn't even provide documentation for them anymore, I'm concerned about spending effort moving something into Zephyr---and taking on the upstream responsibility for basic maintenance---if no member companies have the hardware and are willing to devote resources to supporting it.

I personally would not want to approve a new platform without being able to verify for myself that it passes basic tests, but if the concerns above are mistaken and I can get hardware I'm happy to take a more in-depth look.

@Mani-Sadhasivam

This comment has been minimized.

Copy link
Collaborator Author

Mani-Sadhasivam commented Dec 24, 2019

I haven't looked closely at the port, but in preparation for doing so I have a couple questions:

  • The sole source for this processor appears to be 96Boards via Seeed, where it's out of stock.

I believe that this board is also available on Toabao. I'm not sure if it will be available on Seed studio, but I can check with board vendor.

Is there reason to believe it will be possible for people to get their hands on this hardware?

Please see below comment.

  • #10626 does not include a reason for its being closed, but one blocker was absence of public documentation on the hardware. I don't see detailed information on the product page. Is hardware reference documentation going to become available?

Yes, Unisoc is working on public documentation for this SoC and it will be available soon. The reason for closing #10626 is that Unisoc has no interest in doing the upstreaming by themselves. But they are happy if someone from the community does it.

Having recently spent time dealing with in-tree drivers for sensors that are so past end-of-life that the vendor doesn't even provide documentation for them anymore, I'm concerned about spending effort moving something into Zephyr---and taking on the upstream responsibility for basic maintenance---if no member companies have the hardware and are willing to devote resources to supporting it.

This statement doesn't make sense to me. Is Zephyr going to host only the SoC/boards of the member companies? If that's the case, it needs to be written somewhere so that people don't spend time on upstreaming the non-member platforms. And where does the community contribution comes here? This is purely a community contribution and even if the board is not going to be available in future, we should support the existing users. This is what Linux kernel is doing. The developer who submits the SoC/board/driver support takes the sole responsibility of testing regularly with releases and making sure that there is no regression present. Or even if someone reports some regression, then the developer is responsible for fixing it. If he fails to do so for a long time, then the platform can be considered stale and can be removed with notice. And for basic upstreaming maintenance, we only care about build check, didn't we?

I personally would not want to approve a new platform without being able to verify for myself that it passes basic tests, but if the concerns above are mistaken and I can get hardware I'm happy to take a more in-depth look.

Again, this is not going to work well. Is it a requirement now that the platform support has to be verified by you in order to be upstreamed?

@pabigot

This comment has been minimized.

Copy link
Collaborator

pabigot commented Dec 24, 2019

I'm just a contributor, so please don't take anything I say as being anything more than my opinion.

This statement doesn't make sense to me. Is Zephyr going to host only the SoC/boards of the member companies?

I didn't intend to imply that. I'm unaware of any clearly defined Zephyr policy and process for determining whether a new platform /device is acceptable or when an old platform/device can be removed.

And for basic upstreaming maintenance, we only care about build check, didn't we?

In practice as Zephyr evolves keeping something building means maintainers making source code changes to code in other areas when API or tooling changes, which is risky when the changes can't be verified because nobody has the hardware. I've been spending a lot of time recently doing this for sensors that use GPIO, so I'm possibly a little too sensitive to the costs and risks of untested code.

I personally would not want to approve a new platform without being able to verify for myself that it passes basic tests, but if the concerns above are mistaken and I can get hardware I'm happy to take a more in-depth look.

Again, this is not going to work well. Is it a requirement now that the platform support has to be verified by you in order to be upstreamed?

Somebody has to approve it; it certainly doesn't have to be me.

I was asked to give my review, and I believe that involves more than a cursory "looks like it might work", which is all I could say without hardware and documentation to evaluate it.

I have no problem with buying an interesting and inexpensive board like this in order to support Zephyr's capabilities, so I wanted to know whether it was going to be available so I could give this contribution the careful attention it deserves.

easonxiang and others added 7 commits Sep 20, 2018
Add SoC support for UNISOC UWP5661 SoC, which is a highly integrated
connectivity single chip.

This chip includes 2.4GHz and 5GHz WLAN IEEE 802.11 a/b/g/n/ac,
Bluetooth 5.0 with supporting high power mode, Direction Finding
and Long Range.

More information about this chip can be found in Unisoc's site:
https://unisoc.github.io/chips/UWP5661/

Signed-off-by: Dong Xiang <dong.xiang@unisoc.com>
Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
Add serial driver support for UNISOC UWP5661 SoC.

Signed-off-by: Dong Xiang <dong.xiang@unisoc.com>
Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
Add support for IVY5661 96Boards based on UNISOC UWP5661 SoC. This
board is one of the IoT Edition boards of the 96Boards family
manufactured by UCRobotics.

More information about this board can be found in 96Boards website:
https://www.96boards.org/product/ivy5661/

Signed-off-by: Dong Xiang <dong.xiang@unisoc.com>
Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
Add interrupt controller ICTL support for UWP5661 SoC. This interrupt
controller is a multi level interrupt controller chained to the
NVIC.

Signed-off-by: Dong Xiang <dong.xiang@unisoc.com>
Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
Add myself as the codeowner for all UNISOC stuffs.

Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
Add vendor prefix for UNISOC Communications, Inc.

Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
Add Unisoc HAL module.

Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
@galak galak force-pushed the Mani-Sadhasivam:uwp5661-upstream branch from b85a825 to 7482ea8 Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.