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 Helios4 support (Target : default only) #812

Closed
wants to merge 3 commits into from

Conversation

@g-provost
Copy link
Member

@g-provost g-provost commented Nov 1, 2017

Add Helios4 board :(For now we focus only on default target : linux 4.4)

  • Add Helios4 DTS file
  • Arrange kernel specific patches in board folders.
  • Modified /config/sources/mvebu.conf to use u-boot helios4 repo instead of Solidrun one when compiling board helios4.
  • Add a helios4 patch folder for u-boot
  • Add ATA LEDS driver code + active in kernel config.
  • PWM Fan driver code is in but disable for now.

Note : We modified the repo README file to inform people this is only a dev repo and people should go to official armbian repo to get latest version. So do not merge README.md

@zador-blood-stained
Copy link
Member

@zador-blood-stained zador-blood-stained commented Nov 1, 2017

Some notes:

Arrange kernel specific patches in board folders.

Board specific patches work only for u-boot (and ATF). If you need to patch the kernel then changes have to apply to all board using this kernel configuration and sources. If you need to disable anything (like SPI flash) in DT you can use /delete-node/ syntax or reference node by label and set its status to "disabled".

Modified /config/sources/mvebu.conf to use u-boot helios4 repo instead of Solidrun one when compiling board helios4.

It would be simpler for us if you rebased your u-boot changes on top of SolidRun's clearfog sources (if it's possible). In this case we wouldn't need an extra u-boot sources and could just extract your changes as board specific patches.

And also it looks like we could reuse existing boot script (boot-marvell.cmd) since DT file name would come from compile-time environment patches and eMMC fix and extra MAC addresses just won't be used by u-boot.

PWM Fan driver code is in but disable for now.

Are there any problems with PWM support? Or why patches are currently disabled?

@g-provost
Copy link
Member Author

@g-provost g-provost commented Nov 1, 2017

@zador-blood-stained

Board specific patches work only for u-boot (and ATF). If you need to patch the kernel then changes have to apply to all board using this kernel configuration and sources. If you need to disable anything (like SPI flash) in DT you can use /delete-node/ syntax or reference node by label and set its status to "disabled".

Ok noted, no more board folder. I will revert to the way it was before and add helios4 kernel patches along the existing ones (clearfog).

It would be simpler for us if you rebased your u-boot changes on top of SolidRun's clearfog sources (if it's possible). In this case we wouldn't need an extra u-boot sources and could just extract your changes as board specific patches.

SolidRun u-boot repo has deviated too much from Marvell u-boot one. The way we added Helios4 support to Marvell u-boot is quite different than how SolidRun did. It's easier and cleaner for us to maintain helios4 u-boot code with our own repo.

And also it looks like we could reuse existing boot script (boot-marvell.cmd) since DT file name would come from compile-time environment patches and eMMC fix and extra MAC addresses just won't be used by u-boot.

Agree. Should we then cleanup the boot-marvell.cmd and remove the following lines?

# fdtfile should come from compile-time u-boot patches
if test -z "${fdtfile}"; then
	setenv fdtfile "armada-388-clearfog.dtb"
fi

Are there any problems with PWM support? Or why patches are currently disabled?

The PWM driver doesn't detect the PWM device. We are still working on it.

Even though is not yet a 100% full hardware support, I think it's good that we start to merge in Armbian at some point.

zador-blood-stained added a commit that referenced this pull request Nov 1, 2017
@zador-blood-stained
Copy link
Member

@zador-blood-stained zador-blood-stained commented Nov 1, 2017

I will revert to the way it was before and add helios4 kernel patches along the existing ones (clearfog).

Please note that I pulled most of your kernel changes already in 74b1acd. Since PWM patches are currently disabled I can't enable PWM options in the kernel config.

SolidRun u-boot repo has deviated too much from Marvell u-boot one. The way we added Helios4 support to Marvell u-boot is quite different than how SolidRun did.

That's unfortunate since it will need some rework in config/sources/mvebu.conf

Should we then cleanup the boot-marvell.cmd and remove the following lines?

This is a fallback case that should never happen on fresh images (DT will be set to armada-388-clearfog.dtb only if "fdtfile" variable is unset. I'm not sure whether this should be removed.

@g-provost
Copy link
Member Author

@g-provost g-provost commented Nov 1, 2017

Please note that I pulled most of your kernel changes already in 74b1acd. Since PWM patches are currently disabled I can't enable PWM options in the kernel config.

Ok noted, I pull what you have done first ;-)

That's unfortunate since it will need some rework in config/sources/mvebu.conf

The rework is already in my pull request.
cafa87d#diff-dba325fa979a7b6aabb1d839a869de8a

@zador-blood-stained
Copy link
Member

@zador-blood-stained zador-blood-stained commented Nov 1, 2017

The rework is already in my pull request.
cafa87d#diff-dba325fa979a7b6aabb1d839a869de8a

Which won't apply directly after aba730f
And by "rework" I mostly meant keeping it maintainable, so I'll think about the best way to pull these changes.

@g-provost
Copy link
Member Author

@g-provost g-provost commented Nov 1, 2017

Which won't apply directly after aba730f
And by "rework" I mostly meant keeping it maintainable, so I'll think about the best way to pull these changes.

Oups my bad I forgot to pull one more time the upstream before sending the pull request. I did not see your commit related to clearfog.
Ok thanks for the help.

g-provost added a commit to kobol-io/build that referenced this pull request Nov 1, 2017
@g-provost g-provost force-pushed the kobol-io:helios4 branch from 57907cc to 102eec9 Nov 1, 2017
g-provost added a commit to kobol-io/build that referenced this pull request Nov 1, 2017
zador-blood-stained added a commit that referenced this pull request Nov 1, 2017
@g-provost g-provost force-pushed the kobol-io:helios4 branch from 102eec9 to fcd0e5c Nov 2, 2017
@ThomasKaiser
Copy link
Contributor

@ThomasKaiser ThomasKaiser commented Nov 5, 2017

@g-provost Does this work for you: http://kaiser-edv.de/tmp/NumpU8/not_ready_now/OMV_3_0_91_Helios4_4.4.96.img.xz (new MD5 sum: 1063fd0381b18dcbeea9f8372ba5f7b7)

@g-provost
Copy link
Member Author

@g-provost g-provost commented Nov 6, 2017

@ThomasKaiser The build run ok on target :-)

image

But I had to disable NCQ otherwise I get some ATA error/timeout while re-syncing an RAID10 array and playing with the OMV web interface at the same time.

Helios4_OMV_ATA_errors.txt

Funnily these errors get triggered only when I browse the 'Storage/RAID Management' and 'Storage/File Systems' sub menu pages of the OMV web panel.

First time I see that even though I have done (and repeated today) a lot of stress test on Helios4 before where I fully load the CPU and I/O stress all the interfaces while doing iozone test on different RAID array configuration.

@g-provost
Copy link
Member Author

@g-provost g-provost commented Nov 6, 2017

@zador-blood-stained could you rename mvebu-u-boot-helios.inc to mvebu-u-boot-helios4.inc, just to stick the same naming everywhere. Thx.

@ThomasKaiser
Copy link
Contributor

@ThomasKaiser ThomasKaiser commented Nov 6, 2017

Funnily these errors get triggered only when I browse the 'Storage/RAID Management' and 'Storage/File Systems' sub menu pages of the OMV web panel.

That's important but the wrong place here to discuss these issues. Needs a closer look into OMV's backend (which commands get fired up, eg. there's a lot of devices throwing errors as soon as drive's are queried in a certain way --> eg smartctl -l devstat)

But let's better continue in Armbian forum and stop mis-using this PR :)

@g-provost
Copy link
Member Author

@g-provost g-provost commented Nov 7, 2017

@zador-blood-stained sorry forgot to say that also config/bootenv/helios-default.txt should be renamed to helios4-default.txt

Once this is done. Should we close the pull request ?

@ThomasKaiser Yup let's carry on OMV build discussion on the forum ;-)

@zador-blood-stained
Copy link
Member

@zador-blood-stained zador-blood-stained commented Nov 8, 2017

A rather interesting and possibly related change that affects all a38x devices, and, more importantly, both kernel branches (it was backported to 4.4 too): https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit?h=linux-4.4.y&id=4e351b8dd8b773669c3f0c5e50e4e61031f9e43e
Sorry, this concerns mostly heavy PCIe traffic so native SATA traffic shouldn't really count.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants