From: arno@natisbad.org (Arnaud Ebalard)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv2 7/8] arm: mvebu: add .dts file for Synology DS213j
Date: Mon, 17 Nov 2014 09:48:04 +0100 [thread overview]
Message-ID: <874mtyrz5n.fsf@natisbad.org> (raw)
In-Reply-To: <20141117012906.GA23536@lunn.ch> (Andrew Lunn's message of "Mon, 17 Nov 2014 02:29:06 +0100")
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
> On Sun, Nov 16, 2014 at 06:37:54PM +0100, Arnaud Ebalard wrote:
>> + sata1_pres_pin: sata1-pres-pin {
>> + marvell,pins = "mpp60";
>> + marvell,function = "gpio";
>> + };
>> +
>> + sata2_pres_pin: sata2-pres-pin {
>> + marvell,pins = "mpp48";
>> + marvell,function = "gpio";
>> + };
>
> Hi Arnaud
>
> Are they above correct?
>
> MPP_MODE(48,
> MPP_FUNCTION(0x0, "gpio", NULL),
> MPP_FUNCTION(0x1, "dev", "ad9"),
> MPP_FUNCTION(0x2, "uart0", "rts"),
> MPP_FUNCTION(0x3, "sd0", "cmd"),
> MPP_FUNCTION(0x4, "sata1", "prsnt"),
> MPP_FUNCTION(0x5, "spi0", "cs1")),
>
> and
>
> MPP_MODE(60,
> MPP_FUNCTION(0x0, "gpio", NULL),
> MPP_FUNCTION(0x1, "dev", "ale1"),
> MPP_FUNCTION(0x2, "uart1", "rxd"),
> MPP_FUNCTION(0x3, "sata0", "prsnt"),
> MPP_FUNCTION(0x4, "pcie", "rst-out"),
> MPP_FUNCTION(0x5, "audio", "sdi")),
>
> Seems like function sata[01] would be better than gpio.
>
> Also, i don't see you using these anywhere.
The main reason is that I have (still a draft version of) a patch that
adds a small feature to fixed regulator; the idea is to add an input
signal (gpio/interrupt) to be able to start/stop the regulator. I have
tested it on my RN102 and it works as expected.
ATM, disks on RN102, RN104 and RN2120 are powered by u-boot and I did
not add fixed regulators to have Linux kernel do it. This means if you
add a disk after boot which was not there at boot, it will not be
powered. If you remove a disk that was there at boot, power will still
be available.
With Synology NAS, it's a bit different: you need a fixed regulator to
power the disks because u-boot will not start them, AFAICT.
In the end, the sata presence pins are declared as gpio in order to be
able to use them as input signal for a modified fixed regulator. It's
also to document their existence.
I have nothing against changing the function to sata[01] but how will
those be used, by whom?
> There are a few files in /sys which you might find interesting.
>
> /sys/kernel/debug/pinctrl/f1018000.pinctrl/pinconf-groups shows you
> how pins are currently defined. These can be how Linux has set them,
> or if Linux has not touched them, how the boot loader set them.
>
> /sys/kernel/debug/pinctrl/f1018000.pinctrl/pinmux-pins shows you what
> has been claimed by Linux, either as a gpio or for a specific
> function.
Will take a look.
> There are two schools of thoughts for pinctl. One is to leave the
> bootloader to configure the pins, and Linux should use them as they
> are. The other is that Linux should not trust the bootloader and
> configure the pins itself. With kirkwood we have tried to configure
> everything in Linux. I also think for these two boards, we should
> configure everything. The reason being the broken bootloader. I
> suspect because of the saveenv corruption, more than average are going
> to install a new uboot, or barebox image. A generic uboot might not
> get the pinctl correct, and a barebox image will be using the dtb blob
> to configure the pins. So it would be good to see that all pins which
> are used and claimed by a driver.
I think I prefer second school. Relying on u-boot/barebox doing things
may break at next bootloader update.
Cheers,
a+
WARNING: multiple messages have this Message-ID (diff)
From: arno-LkuqDEemtHBg9hUCZPvPmw@public.gmane.org (Arnaud Ebalard)
To: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
Cc: Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>,
Gregory Clement
<gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>,
Sebastian Hesselbarth
<sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
Ben Peddell <klightspeed-aslSrjg9ejhWX4hkXwHRhw@public.gmane.org>,
Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>,
Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>,
Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>,
Ian Campbell
<ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>,
Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCHv2 7/8] arm: mvebu: add .dts file for Synology DS213j
Date: Mon, 17 Nov 2014 09:48:04 +0100 [thread overview]
Message-ID: <874mtyrz5n.fsf@natisbad.org> (raw)
In-Reply-To: <20141117012906.GA23536-g2DYL2Zd6BY@public.gmane.org> (Andrew Lunn's message of "Mon, 17 Nov 2014 02:29:06 +0100")
Hi Andrew,
Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> writes:
> On Sun, Nov 16, 2014 at 06:37:54PM +0100, Arnaud Ebalard wrote:
>> + sata1_pres_pin: sata1-pres-pin {
>> + marvell,pins = "mpp60";
>> + marvell,function = "gpio";
>> + };
>> +
>> + sata2_pres_pin: sata2-pres-pin {
>> + marvell,pins = "mpp48";
>> + marvell,function = "gpio";
>> + };
>
> Hi Arnaud
>
> Are they above correct?
>
> MPP_MODE(48,
> MPP_FUNCTION(0x0, "gpio", NULL),
> MPP_FUNCTION(0x1, "dev", "ad9"),
> MPP_FUNCTION(0x2, "uart0", "rts"),
> MPP_FUNCTION(0x3, "sd0", "cmd"),
> MPP_FUNCTION(0x4, "sata1", "prsnt"),
> MPP_FUNCTION(0x5, "spi0", "cs1")),
>
> and
>
> MPP_MODE(60,
> MPP_FUNCTION(0x0, "gpio", NULL),
> MPP_FUNCTION(0x1, "dev", "ale1"),
> MPP_FUNCTION(0x2, "uart1", "rxd"),
> MPP_FUNCTION(0x3, "sata0", "prsnt"),
> MPP_FUNCTION(0x4, "pcie", "rst-out"),
> MPP_FUNCTION(0x5, "audio", "sdi")),
>
> Seems like function sata[01] would be better than gpio.
>
> Also, i don't see you using these anywhere.
The main reason is that I have (still a draft version of) a patch that
adds a small feature to fixed regulator; the idea is to add an input
signal (gpio/interrupt) to be able to start/stop the regulator. I have
tested it on my RN102 and it works as expected.
ATM, disks on RN102, RN104 and RN2120 are powered by u-boot and I did
not add fixed regulators to have Linux kernel do it. This means if you
add a disk after boot which was not there at boot, it will not be
powered. If you remove a disk that was there at boot, power will still
be available.
With Synology NAS, it's a bit different: you need a fixed regulator to
power the disks because u-boot will not start them, AFAICT.
In the end, the sata presence pins are declared as gpio in order to be
able to use them as input signal for a modified fixed regulator. It's
also to document their existence.
I have nothing against changing the function to sata[01] but how will
those be used, by whom?
> There are a few files in /sys which you might find interesting.
>
> /sys/kernel/debug/pinctrl/f1018000.pinctrl/pinconf-groups shows you
> how pins are currently defined. These can be how Linux has set them,
> or if Linux has not touched them, how the boot loader set them.
>
> /sys/kernel/debug/pinctrl/f1018000.pinctrl/pinmux-pins shows you what
> has been claimed by Linux, either as a gpio or for a specific
> function.
Will take a look.
> There are two schools of thoughts for pinctl. One is to leave the
> bootloader to configure the pins, and Linux should use them as they
> are. The other is that Linux should not trust the bootloader and
> configure the pins itself. With kirkwood we have tried to configure
> everything in Linux. I also think for these two boards, we should
> configure everything. The reason being the broken bootloader. I
> suspect because of the saveenv corruption, more than average are going
> to install a new uboot, or barebox image. A generic uboot might not
> get the pinctl correct, and a barebox image will be using the dtb blob
> to configure the pins. So it would be good to see that all pins which
> are used and claimed by a driver.
I think I prefer second school. Relying on u-boot/barebox doing things
may break at next bootloader update.
Cheers,
a+
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2014-11-17 8:48 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-16 17:36 [PATCHv2 0/8] arm: mvebu: add Synology DS213j and DS414 .dts files Arnaud Ebalard
2014-11-16 17:36 ` Arnaud Ebalard
2014-11-16 17:36 ` [PATCHv2 1/8] of: add "micron" vendor prefix for Micron Arnaud Ebalard
2014-11-16 17:36 ` Arnaud Ebalard
2014-11-16 17:37 ` [PATCHv2 2/8] arm: mvebu: fix vendor prefix typo in kirkwood-synology.dtsi Arnaud Ebalard
2014-11-16 17:37 ` Arnaud Ebalard
2014-11-16 21:00 ` Andrew Lunn
2014-11-16 21:00 ` Andrew Lunn
2014-11-16 17:37 ` [PATCHv2 3/8] arm: mvebu: add uartX labels for Armada SoC serial nodes Arnaud Ebalard
2014-11-16 17:37 ` Arnaud Ebalard
2014-11-16 17:37 ` [PATCHv2 4/8] arm: mvebu: use recently introduced uart label for stdout-path Arnaud Ebalard
2014-11-16 17:37 ` Arnaud Ebalard
2014-11-16 21:02 ` Andrew Lunn
2014-11-16 21:02 ` Andrew Lunn
2014-11-16 17:37 ` [PATCHv2 5/8] arm: mvebu: add common uart0 and spi0 pintcrl entries for Armada 370 Arnaud Ebalard
2014-11-16 17:37 ` Arnaud Ebalard
2014-11-16 21:10 ` Andrew Lunn
2014-11-16 21:10 ` Andrew Lunn
2014-11-16 22:29 ` Arnaud Ebalard
2014-11-16 22:29 ` Arnaud Ebalard
2014-11-17 0:55 ` Andrew Lunn
2014-11-17 9:28 ` Sebastian Hesselbarth
2014-11-16 17:37 ` [PATCHv2 6/8] arm: mvebu: add common ge0/1 and spi0 pintcrl entries for Armada XP MV78230 Arnaud Ebalard
2014-11-16 17:37 ` Arnaud Ebalard
2014-11-16 17:37 ` [PATCHv2 7/8] arm: mvebu: add .dts file for Synology DS213j Arnaud Ebalard
2014-11-16 17:37 ` Arnaud Ebalard
2014-11-17 1:29 ` Andrew Lunn
2014-11-17 1:29 ` Andrew Lunn
2014-11-17 8:48 ` Arnaud Ebalard [this message]
2014-11-17 8:48 ` Arnaud Ebalard
2014-11-16 17:38 ` [PATCHv2 8/8] arm: mvebu: add .dts file for Synology DS414 Arnaud Ebalard
2014-11-16 17:38 ` Arnaud Ebalard
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=874mtyrz5n.fsf@natisbad.org \
--to=arno@natisbad.org \
--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.