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

added support for Kimax U-25AWF #826

Closed
wants to merge 0 commits into from

Conversation

danielkucera
Copy link
Contributor

@pepe2k
Copy link
Member

pepe2k commented Feb 14, 2017

Please, fix your commit based on the information about submitting patches: here and here. In current shape it cannot be accepted.

u25awf)
set_wifi_led "u25awf:red:wifi"
ucidef_set_led_netdev "eth" "eth" "u25awf:green:lan" "eth0"
;;
rp-n53)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong alphabetical ordering

@@ -134,6 +134,7 @@ ramips_setup_interfaces()
atp-52b|\
awm002-evb|\
awm003-evb|\
u25awf|\
c20i|\
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong ordering and bad indenting

@@ -171,6 +171,9 @@ get_status_led() {
nw718)
status_led="$board:amber:cpu"
;;
u25awf)
status_led="u25awf:red:wifi"
;;
newifi-d1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong ordering and bad indenting

@@ -511,6 +511,9 @@ ramips_board_detect() {
*"WF-2881")
name="wf-2881"
;;
*"U-25AWF")
name="u25awf"
;;
*"WHR-1166D")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong ordering and bad indenting

@@ -151,6 +151,7 @@ platform_check_image() {
witi|\
wizfi630a|\
wl-330n|\
u25awf |\
wl-330n3g|\
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong ordering and bad indenting


palmbus@10000000 {
gpio0: gpio@600 {
status = "okay";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bad indenting

};

gpio1: gpio@638 {
status = "okay";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bad indenting

gpio3: gpio@688 {
status = "okay";
};

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the whole file is badly indented. please use tabs and not spaces

@danielkucera
Copy link
Contributor Author

Ok, I'll submit a new one.

@danielkucera
Copy link
Contributor Author

I just commit-ed a new version.

@bobafetthotmail
Copy link
Contributor

bobafetthotmail commented Feb 18, 2017

Looks good, but it lacks

Signed-off-by: myname mysurname <myemail@email.com>

Please add it.

@danielkucera
Copy link
Contributor Author

Oh, I forgot... What about now?

Copy link
Contributor

@mkresin mkresin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use git commit --amend to add changes to your commit. Use git push --force to overwrite your github repository/branch. The PR will afterwards update automatically.

poll-interval=<20>;
reset{
label="reset";
gpios=<&gpio231>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't work. there are a few whitespaces missing in <&gpio231>;. Should rather be <&gpio2 3 1>; or similar. Seam to apply to all gpios properties.

@@ -0,0 +1,114 @@
/dts-v1/;

/include/"mt7620n.dtsi"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#include "mt7620n.dtsi"

@@ -0,0 +1,114 @@
/dts-v1/;

/include/"mt7620n.dtsi"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include <dt-bindings/gpio/gpio.h> and <dt-bindings/input/input.h> here as well.

Use the GPIO_ACTIVE_LOW and GPIO_ACTIVE_HIGH macros afterwards in stead of 1 and 0 in the gpio parameters.

Use the Key macros instead of the hex values in linux,code.

Check the recent ramips board additions for examples.

m25p80@0{
#address-cells=<1>;
#size-cells=<1>;
compatible="mx25l12805d";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compatible = "jedec,spi-nor";

#address-cells=<1>;
#size-cells=<1>;
compatible="mx25l12805d";
reg=<00>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reg=<0>;

#size-cells=<1>;
compatible="mx25l12805d";
reg=<00>;
linux,modalias="m25p80","m25p128";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop this line. It was only required for kernels < 4.4


palmbus@10000000{
gpio0:gpio@600{
status="okay";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use whitespaces around the equal sign. Without the equal signs it is kinda hard to read.

The same applies to all {. They should have a leading whitespace.

The same applies to all :. They should have a trainling whitespace.

Have a look how it is done in the other dts files.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can drop the whole gpio0 node. it is already enabled in mt7620n.dtsi.

status="okay";
};

gpio1:gpio@638{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drop this node. as far as i can see, gpio1 isn't used in the dts.

@mkresin
Copy link
Contributor

mkresin commented Feb 19, 2017

Please provide a bootlog after you done all requested changes. Some of the spotted mistakes make it quite obvious that it isn't runtime tested and maybe even not compile tested.

@danielkucera
Copy link
Contributor Author

guys, I don't have serial access to my device (it's kinda hard to solder) so I'm not going to experiment with these changes.

@Mynzer
Copy link

Mynzer commented Oct 5, 2017

Hi, good day.

I'm interested in that this device gets full lede support in the future, currently my device is running a compiled lede 17.01.2 version with @danielkucera patch for openwrt

https://github.com/openwrt/openwrt/pull/380/files

I'm agree with @danielkucera that is very hard to solder serial access to this device, so in order to avoid brick my device, I've compiled a lede version with unlocked u-boot partition and I've change stock u-boot to breed bootloader for easy recovery this device.

in case that @danielkucera is interested, y could test any firmware that he compile and provide boot logs, or i could provide the steps and files that i use to change the stock u-boot so that @danielkucera could make any test in his device.

Greetings and thanks for all the hard work

@danielkucera
Copy link
Contributor Author

Hi Mynzer,

I already have my device in "production" so I'm not able to develop on in. But I can try to apply requested changes if you are willing to test them.

@danielkucera
Copy link
Contributor Author

Rebased against current master, new PR: #1402

@danielkucera
Copy link
Contributor Author

@Mynzer can you please test branch u25awf https://github.com/danielkucera/source/tree/u25awf and provide bootlog?

@Mynzer
Copy link

Mynzer commented Oct 6, 2017

Ok, when i try to compile from https://github.com/danielkucera/source/tree/u25awf branch, i've got the following error
CompileError.txt

@danielkucera
Copy link
Contributor Author

try to pull it now

@danielkucera
Copy link
Contributor Author

@Mynzer there were more errors, please try current version and respond to issue #1402 from now on.

@Mynzer
Copy link

Mynzer commented Oct 6, 2017

Ok, I'm in the middle of a previous compilation, I'm going to cancel all, pull again from your branch and report on #1402

@danielkucera
Copy link
Contributor Author

you don't need to make clean when testing updated version. just git pull and make

@Mynzer
Copy link

Mynzer commented Oct 6, 2017

Ok, I see this message to late, next updates will use git pull, by the way, I'm ready to provide bootlog when you apply the new change request

@danielkucera
Copy link
Contributor Author

danielkucera commented Oct 6, 2017 via email

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

Successfully merging this pull request may close these issues.

6 participants