All of lore.kernel.org
 help / color / mirror / Atom feed
From: andrew@lunn.ch (Andrew Lunn)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] ARM: dts: add buffalo linkstation ls-wxl/wsxl
Date: Sat, 20 Jun 2015 00:14:48 +0200	[thread overview]
Message-ID: <20150619221448.GA1116@lunn.ch> (raw)
In-Reply-To: <1434713319-8691-2-git-send-email-rogershimizu@gmail.com>

Hi Roger

These two look quite good. Just some minor comments. Many of them
apply to both dts files.

> +/ {
> +	model = "Buffalo Linkstation LS-WXL/WSXL";
> +	compatible = "buffalo,linkstation,lswxl", "buffalo,lswxl", "marvell,kirkwood-88f6281", "marvell,kirkwood";

Compatibility strings don't normally have two , in them. I would drop
the first one.

> +			pmx_led_function_red: pmx-led-function_red {

The second _red should be -red.
> +		spi at 10600 {
> +			status = "okay";
> +
> +			m25p40 at 0 {
> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +				compatible = "m25p40";

There should be a vendor in this compatibility string, probably "st".

> +		button at 2 {
> +			label = "Power-on Switch";
> +			linux,code = <KEY_RESERVED>;
> +			linux,input-type = <5>;
> +			gpios = <&gpio1 42 GPIO_ACTIVE_LOW>;
> +		};

Why did you pick these values? <KEY_POWER> is a more common code for a
power button.

> +
> +		button at 3 {
> +			label = "Power-auto Switch";
> +			linux,code = <KEY_ESC>;
> +			linux,input-type = <5>;
> +			gpios = <&gpio1 43 GPIO_ACTIVE_LOW>;

Also a bit unusual. What is this button used for?

> +		led at 5 {
> +			label = "lswxl:red:func";
> +			gpios = <&gpio1 5 GPIO_ACTIVE_LOW>;
> +			default-state = "keep";
> +			linux,default-trigger = "heartbeat";
> +		};

I know of one maintainer who will not like the heartbeat here.  Also,
default-state and heartbeat at the same time?

> +&eth0 {
> +	status = "okay";
> +	reg = <0x76000 0x4000>;
> +	clocks = <&gate_clk 19>;
> +	ethernet0-port at 0 {
> +		phy-handle = <&ethphy1>;
> +		interrupts = <15>;
> +	};

Why reg, clocks and interrupts here? These values belong to eth1.

Thanks

    Andrew

  reply	other threads:[~2015-06-19 22:14 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-19 11:28 [PATCH 0/2] ARM: dts: add buffalo linkstation ls-wxl/wsxl & ls-wvl/vl Roger Shimizu
2015-06-19 11:28 ` [PATCH 1/2] ARM: dts: add buffalo linkstation ls-wxl/wsxl Roger Shimizu
2015-06-19 22:14   ` Andrew Lunn [this message]
2015-06-20 12:36     ` Roger Shimizu
2015-06-20 14:51       ` Andrew Lunn
2015-06-20 15:56         ` Roger Shimizu
2015-06-20 16:03           ` Andrew Lunn
2015-06-21 15:16             ` Roger Shimizu
2015-06-21 20:09               ` Andrew Lunn
2015-06-22 15:00   ` [PATCH v2 " Roger Shimizu
2015-06-29 13:22     ` Gregory CLEMENT
2015-06-29 13:38       ` Andrew Lunn
2015-07-09 13:12         ` Gregory CLEMENT
2015-07-09 14:06           ` Roger Shimizu
2015-06-19 11:28 ` [PATCH 2/2] ARM: dts: add buffalo linkstation ls-wvl/vl Roger Shimizu
2015-06-19 22:22   ` Andrew Lunn
2015-06-22 15:02   ` [PATCH v2 " Roger Shimizu
2015-06-29 13:22     ` Gregory CLEMENT
2015-06-29 13:43       ` Andrew Lunn
2015-06-29 13:52         ` Gregory CLEMENT
2015-07-09 13:13         ` Gregory CLEMENT

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150619221448.GA1116@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.