diff mbox

efikamx: reintroduce Genesi Efika MX Smarttop via device tree

Message ID 1344375978-29981-1-git-send-email-matt@genesi-usa.com
State New
Headers show

Commit Message

Matt Sealey Aug. 7, 2012, 9:46 p.m. UTC
This device tree only supports the final retail board ("TO3").

It is currently feature equivalent to the MX51 Babbage device tree. The
following features have been tested and work as well as can be expected:

* Serial port
* SD card support
* I2C bus
* SGTL5000 audio support via the internal speaker
* SDMA support (via audio)
* SPI bus
* NOR flash (at 25MHz bus speed)
* MC13892 PMIC Regulator and Realtime clock functions

Since the board requires some extra work on the PMIC to power off, the system
will instead just halt, however reboot works. Other missing features are due
to a lack of drivers or device tree bindings currently.

Signed-off-by: Matt Sealey <matt@genesi-usa.com>
Signed-off-by: Steev Klimaszewski <steev@genesi-usa.com>
---
 arch/arm/boot/dts/imx51-efikamx.dts |  247 +++++++++++++++++++++++++++++++++++
 arch/arm/mach-imx/Makefile.boot     |    2 +-
 2 files changed, 248 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/imx51-efikamx.dts

Comments

Shawn Guo Aug. 8, 2012, 3:15 p.m. UTC | #1
On Tue, Aug 07, 2012 at 04:46:18PM -0500, Matt Sealey wrote:
> This device tree only supports the final retail board ("TO3").
> 
> It is currently feature equivalent to the MX51 Babbage device tree. The
> following features have been tested and work as well as can be expected:
> 
> * Serial port
> * SD card support
> * I2C bus
> * SGTL5000 audio support via the internal speaker
> * SDMA support (via audio)
> * SPI bus
> * NOR flash (at 25MHz bus speed)
> * MC13892 PMIC Regulator and Realtime clock functions
> 
> Since the board requires some extra work on the PMIC to power off, the system
> will instead just halt, however reboot works. Other missing features are due
> to a lack of drivers or device tree bindings currently.
> 
> Signed-off-by: Matt Sealey <matt@genesi-usa.com>
> Signed-off-by: Steev Klimaszewski <steev@genesi-usa.com>
> ---
>  arch/arm/boot/dts/imx51-efikamx.dts |  247 +++++++++++++++++++++++++++++++++++
>  arch/arm/mach-imx/Makefile.boot     |    2 +-
>  2 files changed, 248 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boot/dts/imx51-efikamx.dts
> 
> diff --git a/arch/arm/boot/dts/imx51-efikamx.dts b/arch/arm/boot/dts/imx51-efikamx.dts
> new file mode 100644
> index 0000000..dd14f77
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx51-efikamx.dts
> @@ -0,0 +1,247 @@
> +/*
> + * Copyright 2011 Freescale Semiconductor, Inc.
> + * Copyright 2011 Linaro Ltd.
> + * Copyright 2012 Genesi USA, Inc.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +/dts-v1/;
> +/include/ "imx51.dtsi"
> +
> +/ {
> +	model = "Genesi Efika MX (Smarttop)";
> +	compatible = "genesi,imx51-efikamx", "fsl,imx51";
> +
> +	memory {
> +		reg = <0x90000000 0x20000000>;
> +	};
> +
> +	soc {
> +		aips@70000000 {
> +			spba@70000000 {
> +				esdhc@70004000 {

The pinctrl_provide_dummies() in imx51_dt_init() is something to be
removed.  Then any driver calling pinctrl API will require pinctrl
set up for the device here.  So please have the pinctrl setup for
those devices. 

> +					cd-gpios = <&gpio1 0 0>;
> +					wp-gpios = <&gpio1 1 0>;
> +					status = "okay";
> +				};
> +
> +				ssi2: ssi@70014000 {
> +				    fsl,mode = "i2s-slave";
> +				    status = "okay";
> +				};
> +
> +				ecspi@70010000 {
> +					fsl,spi-num-chipselects = <2>;
> +					cs-gpios = <&gpio4 24 0>, <&gpio4 25 0>;
> +					status = "okay";
> +
> +					pmic: mc13892@0 {
> +						#address-cells = <1>;
> +						#size-cells = <0>;
> +						compatible = "fsl,mc13892";
> +						spi-max-frequency = <6000000>;
> +						reg = <0>;
> +						interrupt-parent = <&gpio1>;
> +						interrupts = <6 0x4>;
> +						fsl,mc13xxx-uses-rtc;
> +
> +						regulators {
> +							sw1_reg: sw1 {
> +								regulator-min-microvolt = <600000>;
> +								regulator-max-microvolt = <1375000>;
> +								regulator-boot-on;
> +								regulator-always-on;
> +							};
> +
> +							sw2_reg: sw2 {
> +								regulator-min-microvolt = <900000>;
> +								regulator-max-microvolt = <1850000>;
> +								regulator-boot-on;
> +								regulator-always-on;
> +							};
> +
> +							sw3_reg: sw3 {
> +								regulator-min-microvolt = <1100000>;
> +								regulator-max-microvolt = <1850000>;
> +								regulator-boot-on;
> +								regulator-always-on;
> +							};
> +
> +							sw4_reg: sw4 {
> +								regulator-min-microvolt = <1100000>;
> +								regulator-max-microvolt = <1850000>;
> +								regulator-boot-on;
> +								regulator-always-on;
> +							};
> +
> +							vpll_reg: vpll {
> +								regulator-min-microvolt = <1050000>;
> +								regulator-max-microvolt = <1800000>;
> +								regulator-boot-on;
> +								regulator-always-on;
> +							};
> +
> +							vdig_reg: vdig {
> +								regulator-min-microvolt = <1650000>;
> +								regulator-max-microvolt = <1650000>;
> +								regulator-boot-on;
> +							};
> +
> +							vsd_reg: vsd {
> +								regulator-min-microvolt = <3150000>;
> +								regulator-max-microvolt = <3150000>;
> +							};
> +
> +							vusb2_reg: vusb2 {
> +								regulator-min-microvolt = <2400000>;
> +								regulator-max-microvolt = <2775000>;
> +								regulator-boot-on;
> +								regulator-always-on;
> +							};
> +
> +							vvideo_reg: vvideo {
> +								regulator-min-microvolt = <2775000>;
> +								regulator-max-microvolt = <2775000>;
> +								regulator-boot-on;
> +							};
> +
> +							vaudio_reg: vaudio {
> +								regulator-min-microvolt = <2300000>;
> +								regulator-max-microvolt = <3000000>;
> +							};
> +
> +							vcam_reg: vcam {
> +								regulator-min-microvolt = <2500000>;
> +								regulator-max-microvolt = <3000000>;
> +							};
> +
> +							vgen1_reg: vgen1 {
> +								regulator-min-microvolt = <1200000>;
> +								regulator-max-microvolt = <3150000>;
> +							};
> +
> +							vgen2_reg: vgen2 {
> +								regulator-min-microvolt = <3150000>;
> +								regulator-max-microvolt = <3150000>;
> +								regulator-always-on;
> +							};
> +
> +							vgen3_reg: vgen3 {
> +								regulator-min-microvolt = <1800000>;
> +								regulator-max-microvolt = <2900000>;
> +								regulator-always-on;
> +							};
> +						};
> +					};
> +
> +					flash: sst25vf032b@1 {
> +						#address-cells = <1>;
> +						#size-cells = <1>;
> +						compatible = "sst,sst25vf032b";
> +						spi-max-frequency = <25000000>;
> +						reg = <1>;
> +
> +						partition@0 {
> +							label = "firmware";
> +							reg = <0x0 0x200000>;
> +						};
> +
> +						partition@200000 {
> +							label = "user";
> +							reg = <0x200000 0x200000>;
> +						};
> +					};
> +				};
> +			};
> +
> +			wdog@73f98000 {
> +				status = "okay";
> +			};

Remove it.  I have queued a patch to enable wdog in <soc>.dtsi
by default.

> +
> +			iomuxc@73fa8000 {
> +				compatible = "fsl,imx51-iomuxc";
> +				reg = <0x73fa8000 0x4000>;
> +			};
> +
> +			uart1: serial@73fbc000 {
> +				fsl,uart-has-rtscts;
> +				status = "okay";
> +			};
> +
> +		};
> +
> +		aips@80000000 {
> +			sdma@83fb0000 {
> +				fsl,sdma-ram-script-name = "imx/sdma/sdma-imx51.bin";
> +			};

Remove it.  I have seen a patch to move this name into <soc>.dtsi
as default.

> +
> +			i2c@83fc4000 {
> +				status = "okay";
> +
> +				codec: sgtl5000@0a {
> +					compatible = "fsl,sgtl5000";
> +					reg = <0x0a>;
> +					clock-frequency = <12288000>;
> +					VDDA-supply = <&vdig_reg>;
> +					VDDIO-supply = <&vvideo_reg>;
> +				};
> +			};
> +
> +			audmux@83fd0000 {
> +				status = "okay";
> +			};
> +		};
> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +
> +		power {
> +			label = "Power Button";
> +			gpios = <&gpio2 31 0>;
> +			linux,code = <116>; /* KEY_POWER */
> +			gpio-key,wakeup;
> +		};
> +	};
> +
> +	gpio-leds {
> +		compatible = "gpio-leds";
> +
> +		red {
> +			label = "red";
> +			gpios = <&gpio3 13>;
> +			linux,default-trigger = "ide-disk";
> +		};
> +
> +		green {
> +			label = "green";
> +			gpios = <&gpio3 14>;
> +			linux,default-trigger = "default-on";
> +		};
> +
> +		blue {
> +			label = "blue";
> +			gpios = <&gpio3 15>;
> +			linux,default-trigger = "mmc0";
> +		};
> +	};
> +
> +	sound {
> +		compatible = "fsl,imx-audio-sgtl5000";
> +		model = "efikamx-sgtl5000";
> +		ssi-controller = <&ssi2>;
> +		audio-routing =
> +			"MIC_IN", "Mic Jack",
> +			"Mic Jack", "Mic Bias",
> +			"Ext Spk", "LINE_OUT";
> +		audio-codec = <&codec>;
> +		mux-int-port = <2>;
> +		mux-ext-port = <3>;
> +	};
> +};
> diff --git a/arch/arm/mach-imx/Makefile.boot b/arch/arm/mach-imx/Makefile.boot
> index 05541cf..3ed7c9d 100644
> --- a/arch/arm/mach-imx/Makefile.boot
> +++ b/arch/arm/mach-imx/Makefile.boot
> @@ -38,7 +38,7 @@ zreladdr-$(CONFIG_SOC_IMX6Q)	+= 0x10008000
>  params_phys-$(CONFIG_SOC_IMX6Q)	:= 0x10000100
>  initrd_phys-$(CONFIG_SOC_IMX6Q)	:= 0x10800000
>  
> -dtb-$(CONFIG_MACH_IMX51_DT) += imx51-babbage.dtb
> +dtb-$(CONFIG_MACH_IMX51_DT) += imx51-babbage.dtb imx51-efikamx.dtb

Please have the new entry on the new line like dtb-$(CONFIG_SOC_IMX6Q).
Yes, we will change dtb-$(CONFIG_MACH_IMX53_DT).

>  dtb-$(CONFIG_MACH_IMX53_DT) += imx53-ard.dtb imx53-evk.dtb \
>  			       imx53-qsb.dtb imx53-smd.dtb
>  dtb-$(CONFIG_SOC_IMX6Q)	+= imx6q-arm2.dtb \
> -- 
> 1.7.9.5
>
Matt Sealey Aug. 8, 2012, 4:55 p.m. UTC | #2
On Wed, Aug 8, 2012 at 10:15 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Tue, Aug 07, 2012 at 04:46:18PM -0500, Matt Sealey wrote:
>> This device tree only supports the final retail board ("TO3").
>>
>> It is currently feature equivalent to the MX51 Babbage device tree. The
>> following features have been tested and work as well as can be expected:
[snip]
>> +     soc {
>> +             aips@70000000 {
>> +                     spba@70000000 {
>> +                             esdhc@70004000 {
>
> The pinctrl_provide_dummies() in imx51_dt_init() is something to be
> removed.  Then any driver calling pinctrl API will require pinctrl
> set up for the device here.  So please have the pinctrl setup for
> those devices.

Absolutely not. Our pins are muxed in U-Boot as they should be. I
refuse to add pinmux entries
or any setup at all for this. What's stopping this right now is you
need a new U-Boot which we
didn't release or mainline because we are still testing it (old U-Boot
shipped on the boards
cannot boot device tree anyway). While the number of users of this is
limited to everyone in
the office here and a few trusted testers, actually the support here
is meaningless for everyone
else, but we feel it needs to go into the tree so we can track changes
when people changing
bindings and basically be future-thinking. We need the nitpicks, but
in this instance, I will
take a leaf from Russell's book and say I violently disagree with
requiring pinctrl to be set up.
There's no need on MX51 with the current state of the architecture and
we've successfully
tested all pad settings in mainline (and older kernels by stripping
muxing from the kernel)
just relying on gpio_direction and value sets.

We'll have a public U-Boot for this board by the end of next week
probably, and it will do it
right the first time.

>> +                     wdog@73f98000 {
>> +                             status = "okay";
>> +                     };
>
> Remove it.  I have queued a patch to enable wdog in <soc>.dtsi
> by default.

Okay.

>> +             aips@80000000 {
>> +                     sdma@83fb0000 {
>> +                             fsl,sdma-ram-script-name = "imx/sdma/sdma-imx51.bin";
>> +                     };
>
> Remove it.  I have seen a patch to move this name into <soc>.dtsi
> as default.

BTW I propose we make this somehow a bootloader-stage thing - at least, the SDMA
firmware should be in a location which is dictated not by the kernel
itself, certainly not
the device tree, but packaging and operating system dependent
features. It describes
the board, not the rootfs. As in, depending on the OS booted it may not be
imx/sdma/sdma-imx51.bin or anything like it. Remember device trees are NOT just
for Linux (or Android, which may still put it in a relative path with
that name, but it
may NOT depending on future changes!)

In a boot.scr for modern U-Boot you might just have

setenv bootargs ${bootargs} imx-sdma.firmware="imx/sdma/sdma-imx51.bin"

Or something similar. OS filesystem paths in device tree are absolutely wrong.

>> -dtb-$(CONFIG_MACH_IMX51_DT) += imx51-babbage.dtb
>> +dtb-$(CONFIG_MACH_IMX51_DT) += imx51-babbage.dtb imx51-efikamx.dtb
>
> Please have the new entry on the new line like dtb-$(CONFIG_SOC_IMX6Q).
> Yes, we will change dtb-$(CONFIG_MACH_IMX53_DT).

Okay.
Fabio Estevam Aug. 8, 2012, 5:19 p.m. UTC | #3
Matt,

On Wed, Aug 8, 2012 at 1:55 PM, Matt Sealey <matt@genesi-usa.com> wrote:

...
> or any setup at all for this. What's stopping this right now is you
> need a new U-Boot which we
> didn't release or mainline because we are still testing it (old U-Boot
> shipped on the boards
> cannot boot device tree anyway). While the number of users of this is

Actually you can boot a device tree kernel even on old bootloaders
that do not support dt.

You need to select:
CONFIG_ARM_APPENDED_DTB=y
CONFIG_ARM_ATAG_DTB_COMPAT=y

Then,

make -j4 zImage
make imx51-babbage.dtb
cat arch/arm/boot/zImage arch/arm/boot/ imx51-babbage.dtb  >
arch/arm/boot/zImage_dtb
mkimage -A arm -O linux -T kernel -C none -a 0x90008000 -e 0x90008000
-n Linux -d arch/arm/boot/zImage_dtb arch/arm/boot/uImage

and boot this generated uImage the same way as you used to do in the
non-dt case.

Regards,

Fabio Estevam
Mark Brown Aug. 9, 2012, 10:19 a.m. UTC | #4
On Tue, Aug 07, 2012 at 04:46:18PM -0500, Matt Sealey wrote:

Yay for indentation!  It'd be good to rewrite your DT so you could cut
down on that, at the minute it's not good for legibility.

> +							sw1_reg: sw1 {
> +								regulator-min-microvolt = <600000>;
> +								regulator-max-microvolt = <1375000>;
> +								regulator-boot-on;
> +								regulator-always-on;
> +							};

This and many of your other regulators have voltage ranges specified but
no consumers which doesn't make sense.  It looks awfully like you've
just typed in the maximum range supported by the regulator which is most
likely broken.

You're also specifying both boot_on and always_on which again doesn't
seem to make a lot of sense - boot_on mainly exists to help autoprobe,
using it quite this routinely isn't too clever.
Matt Sealey Aug. 9, 2012, 1:40 p.m. UTC | #5
On Thu, Aug 9, 2012 at 5:19 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Aug 07, 2012 at 04:46:18PM -0500, Matt Sealey wrote:
>
> Yay for indentation!  It'd be good to rewrite your DT so you could cut
> down on that, at the minute it's not good for legibility.
>
>> +                                                     sw1_reg: sw1 {
>> +                                                             regulator-min-microvolt = <600000>;
>> +                                                             regulator-max-microvolt = <1375000>;
>> +                                                             regulator-boot-on;
>> +                                                             regulator-always-on;
>> +                                                     };
>
> This and many of your other regulators have voltage ranges specified but
> no consumers which doesn't make sense.  It looks awfully like you've
> just typed in the maximum range supported by the regulator which is most
> likely broken.
>
> You're also specifying both boot_on and always_on which again doesn't
> seem to make a lot of sense - boot_on mainly exists to help autoprobe,
> using it quite this routinely isn't too clever.

The reason they're set like that is legacy - that's how they're set up
in a kernel
(pre-DT) that we know works. Most of those ranges are directly from the Babbage
reference and stay like that in the Babbage DT too - so there's another broken
one nobody noticed. I know what those voltages should be, but we're
leaving that for another patch that restricts the range of voltages
(it works right
now, since there are no consumers, nothing CHANGES the voltages as
configured at U-Boot time, and anything not boot-on is just not listed
in the DT anyway, but some of them really need to stay on)

There are few consumers because the primary ones out there are the display
controllers and USB hubs and some other things. MMC should be a consumer
but since on one board we share two MMC slots with one regulator we don't
want anyone to change the voltage (it breaks spec anyway, since we can't
provide more than 3.15V with FSL's PMIC and it should be 3.3V by default)
and since you can't coordinate between MMC hosts on what the lowest voltage
both cards can support actually is.. having someone change it would be
bad.
Mark Brown Aug. 9, 2012, 2:17 p.m. UTC | #6
On Thu, Aug 09, 2012 at 08:40:36AM -0500, Matt Sealey wrote:

> The reason they're set like that is legacy - that's how they're set up
> in a kernel
> (pre-DT) that we know works. Most of those ranges are directly from the Babbage
> reference and stay like that in the Babbage DT too - so there's another broken
> one nobody noticed. I know what those voltages should be, but we're
> leaving that for another patch that restricts the range of voltages
> (it works right
> now, since there are no consumers, nothing CHANGES the voltages as
> configured at U-Boot time, and anything not boot-on is just not listed
> in the DT anyway, but some of them really need to stay on)

Oh dear.  Well, no reason to propagate the breakage - if nothing else it
might well explode if we start doing more aggressive power saving with
the regulators (like having an option for dropping down to the lower end
of the voltage ranges in late init which I keep contemplating, it'd
explode with the boards doing this).

> There are few consumers because the primary ones out there are the display
> controllers and USB hubs and some other things. MMC should be a consumer
> but since on one board we share two MMC slots with one regulator we don't
> want anyone to change the voltage (it breaks spec anyway, since we can't
> provide more than 3.15V with FSL's PMIC and it should be 3.3V by default)
> and since you can't coordinate between MMC hosts on what the lowest voltage
> both cards can support actually is.. having someone change it would be
> bad.

It's not a problem to have fixed voltages, the problem is the
combination of specifying voltage ranges in conjunction with not having
anything there that wants to change the voltage.  Especially with things
like the audio supply, it's clear what that's for and it'd normally get
upset with the voltage changing.
Matt Sealey Aug. 9, 2012, 2:29 p.m. UTC | #7
On Wed, Aug 8, 2012 at 12:19 PM, Fabio Estevam <festevam@gmail.com> wrote:
> Matt,
>
> On Wed, Aug 8, 2012 at 1:55 PM, Matt Sealey <matt@genesi-usa.com> wrote:
>
> ...
>> or any setup at all for this. What's stopping this right now is you
>> need a new U-Boot which we
>> didn't release or mainline because we are still testing it (old U-Boot
>> shipped on the boards
>> cannot boot device tree anyway). While the number of users of this is
>
> Actually you can boot a device tree kernel even on old bootloaders
> that do not support dt.
>
> You need to select:
> CONFIG_ARM_APPENDED_DTB=y
> CONFIG_ARM_ATAG_DTB_COMPAT=y
>
> Then,
>
> make -j4 zImage
> make imx51-babbage.dtb
> cat arch/arm/boot/zImage arch/arm/boot/ imx51-babbage.dtb  >
> arch/arm/boot/zImage_dtb
> mkimage -A arm -O linux -T kernel -C none -a 0x90008000 -e 0x90008000
> -n Linux -d arch/arm/boot/zImage_dtb arch/arm/boot/uImage
>
> and boot this generated uImage the same way as you used to do in the
> non-dt case.

That's true, but we don't have our customers compile their own kernels
if they can help it, and appending a dtb to the end of a kernel isn't
part of distro packaging so it just doesn't get done yet... We'd need
to update a bunch of scripts, test them, and then deal with the
frightening scenario of appending a dtb to a kernel *and* passing the
address of the filesystem version (usually the one appended!) - hoping
either works. It's too much testing. We'd rather update everyone to a
new U-Boot that works, and deal with a single point on that end, that
can boot the legacy kernel (machine id matches, legacy boot works on
old kernel) and the new kernel alike.

All of this is coming from development branches we have here, and
we're pushing it to mainline as a convenience for everyone concerned.

What we've got on the test plan is;

1) Old U-Boot, Old Kernel. This works already for years.
2) New U-Boot, Old Kernel. This works, well tested.
3) New U-Boot, New Kernel+DTB. This works or we wouldn't be sending patches.

The Old U-Boot, New Kernel doesn't make sense for us if the New U-Boot
is all that's required to retain functionality.

The reason the new kernel depends on the new U-Boot is we're trying to
do all pinmux configuration in U-Boot (and we do in-house, and it
works). No pinctrl stuff in the kernel or device tree is required at
this point. The Old Kernel will remux a few things redundantly or
change drive strengths or whatever or add hysteresis to the UART port
which is not board-burning but is not really necessary, but it will
work. The new kernel will just be able to do what it does out of the
box, which is how it should be (hence why I object to adding pinctrl
setup...)

--
Matt Sealey <matt@genesi-usa.com>
Shawn Guo Aug. 10, 2012, 1:41 a.m. UTC | #8
On Thu, Aug 09, 2012 at 09:29:39AM -0500, Matt Sealey wrote:
> The reason the new kernel depends on the new U-Boot is we're trying to
> do all pinmux configuration in U-Boot (and we do in-house, and it
> works). No pinctrl stuff in the kernel or device tree is required at
> this point. The Old Kernel will remux a few things redundantly or
> change drive strengths or whatever or add hysteresis to the UART port
> which is not board-burning but is not really necessary, but it will
> work. The new kernel will just be able to do what it does out of the
> box, which is how it should be (hence why I object to adding pinctrl
> setup...)
> 
Then I will have to refuse to accept your patch, because I'm working on
a series to remove pinctrl_provide_dummies() from imx51_dt_init().
Matt Sealey Aug. 10, 2012, 1:36 p.m. UTC | #9
On Thu, Aug 9, 2012 at 8:41 PM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Thu, Aug 09, 2012 at 09:29:39AM -0500, Matt Sealey wrote:
>> The reason the new kernel depends on the new U-Boot is we're trying to
>> do all pinmux configuration in U-Boot (and we do in-house, and it
>> works). No pinctrl stuff in the kernel or device tree is required at
>> this point. The Old Kernel will remux a few things redundantly or
>> change drive strengths or whatever or add hysteresis to the UART port
>> which is not board-burning but is not really necessary, but it will
>> work. The new kernel will just be able to do what it does out of the
>> box, which is how it should be (hence why I object to adding pinctrl
>> setup...)
>
> Then I will have to refuse to accept your patch, because I'm working on
> a series to remove pinctrl_provide_dummies() from imx51_dt_init().

Requiring it breaks the entire concept of the device tree to describe running
hardware. It is not a configuration script. pinctrl should be optional
- built in
always, but not necessary to turn a board on if it's already configured.

What would happen if a board were designed that only used the default ALT
settings on i.MX53 or so? You'd have to redefine every default IOMUX pad
just to get it to boot. That's intolerable.
Shawn Guo Aug. 10, 2012, 2:04 p.m. UTC | #10
On Fri, Aug 10, 2012 at 08:36:02AM -0500, Matt Sealey wrote:
> Requiring it breaks the entire concept of the device tree to describe running
> hardware. It is not a configuration script. pinctrl should be optional
> - built in
> always, but not necessary to turn a board on if it's already configured.
> 
How would kernel know if it's already configured, correctly?

> What would happen if a board were designed that only used the default ALT
> settings on i.MX53 or so? You'd have to redefine every default IOMUX pad
> just to get it to boot. That's intolerable.
> 
Come on, even the pad configuration are all the default?  Even if that's
the case, yes, we still need to do it.  How do we know if firmware has
changed the settings or not.
Matt Sealey Aug. 10, 2012, 2:26 p.m. UTC | #11
On Fri, Aug 10, 2012 at 9:04 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Fri, Aug 10, 2012 at 08:36:02AM -0500, Matt Sealey wrote:
>> Requiring it breaks the entire concept of the device tree to describe running
>> hardware. It is not a configuration script. pinctrl should be optional
>> - built in
>> always, but not necessary to turn a board on if it's already configured.
>>
> How would kernel know if it's already configured, correctly?

Trust! When we release the new U-Boot, it will be already configured,
every pin in the schematic, every
pin from the old kernels (not mainline, some of that was wrong),
exactly the way it should be. There's
nothing new with the Efika MX.

If you try and boot it on the old Efika, it just won't work reliably
for any peripheral U-Boot didn't
already configure, but for the current version you'd get MMC, PATA,
serial port, SPI (NOR/PMIC)
which is all we configure in the DT right now anyway... this is only
going to be an issue once we
get to displays and USB (I2C isn't set up in the old one).

>> What would happen if a board were designed that only used the default ALT
>> settings on i.MX53 or so? You'd have to redefine every default IOMUX pad
>> just to get it to boot. That's intolerable.
>
> Come on, even the pad configuration are all the default?  Even if that's
> the case, yes, we still need to do it.  How do we know if firmware has
> changed the settings or not.

TRUST...

Maybe you can't rely on the development boards from Freescale, but we have to
do unit testing at every stage of operation for consumer devices. Once U-Boot
passes all tests, why bother re-testing the exact same configuration, now done
twice, in the kernel? I don't want to debug pad settings twice, and we shouldn't
need to.

If you really think it's necessary then fine, we'll do it.
Shawn Guo Aug. 10, 2012, 2:40 p.m. UTC | #12
On Fri, Aug 10, 2012 at 09:26:36AM -0500, Matt Sealey wrote:
> If you really think it's necessary then fine, we'll do it.
> 
Yes, please do.
Matt Sealey Aug. 10, 2012, 2:42 p.m. UTC | #13
By the way just as an example, a board with the following could be
configured on i.MX53 without touching any IOMUX settings at all
besides DDR (which would get done at boot rom time through the dcd);

* Keypad (KPP)
* 24-bit Parallel display on IPU DI0
* GPIO6&7 pins 22 through 31, GPIO4 10 through 14, and a couple others
* Parallel camera on CSI0
* NAND
* Certain configurations of Ethernet
* PATA
* SD1 and SD2
* ESAI audio
* EIM bus
* CLKIH CLKIL and CLKO clocks

With discrete power (no PMIC), bitbang I2C and SPI to control whatever
minimal peripherals you need out there, this is basically a Smarttop.
Sure, it's not as fun as using the real SPI, I2C buses and you don't
get a UART, but it's possible.
Matt Sealey Aug. 13, 2012, 3:42 p.m. UTC | #14
On Thu, Aug 9, 2012 at 5:19 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Aug 07, 2012 at 04:46:18PM -0500, Matt Sealey wrote:
>
> Yay for indentation!  It'd be good to rewrite your DT so you could cut
> down on that, at the minute it's not good for legibility.
>
>> +                                                     sw1_reg: sw1 {
>> +                                                             regulator-min-microvolt = <600000>;
>> +                                                             regulator-max-microvolt = <1375000>;
>> +                                                             regulator-boot-on;
>> +                                                             regulator-always-on;
>> +                                                     };
>
> This and many of your other regulators have voltage ranges specified but
> no consumers which doesn't make sense.  It looks awfully like you've
> just typed in the maximum range supported by the regulator which is most
> likely broken.

Okay I have a question about this; some of the regulators (SW1
especially) are obviously consumed by the CPU core complex so that
when DVFS gives us a hint we can clock down and reduce voltage. How on
earth do we implement that?

We can drop the maximum range to be better for the CPU (1.3V is too
high, I think this is legacy from when we may have had a sorted 1GHz
MX51 coming out) but I can't find any source for where this is hooked
in.
Mark Brown Aug. 13, 2012, 5:38 p.m. UTC | #15
On Mon, Aug 13, 2012 at 10:42:58AM -0500, Matt Sealey wrote:
> On Thu, Aug 9, 2012 at 5:19 AM, Mark Brown

> > This and many of your other regulators have voltage ranges specified but
> > no consumers which doesn't make sense.  It looks awfully like you've
> > just typed in the maximum range supported by the regulator which is most
> > likely broken.

> Okay I have a question about this; some of the regulators (SW1
> especially) are obviously consumed by the CPU core complex so that
> when DVFS gives us a hint we can clock down and reduce voltage. How on
> earth do we implement that?

> We can drop the maximum range to be better for the CPU (1.3V is too
> high, I think this is legacy from when we may have had a sorted 1GHz
> MX51 coming out) but I can't find any source for where this is hooked
> in.

DVFS doesn't use the device model in Linux (yet!) so there's no device
node and it's all a bit messy.  Shawn Guo is working on some generic DT
bindings for it which should solve that problem but for now in the
specific case of CPU bindings it's OK to not have an consumer, it's
understandable what's going on there.
Matt Sealey Aug. 13, 2012, 10:05 p.m. UTC | #16
On Mon, Aug 13, 2012 at 12:38 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Aug 13, 2012 at 10:42:58AM -0500, Matt Sealey wrote:
>> On Thu, Aug 9, 2012 at 5:19 AM, Mark Brown
>
>> > This and many of your other regulators have voltage ranges specified but
>> > no consumers which doesn't make sense.  It looks awfully like you've
>> > just typed in the maximum range supported by the regulator which is most
>> > likely broken.
>
>> Okay I have a question about this; some of the regulators (SW1
>> especially) are obviously consumed by the CPU core complex so that
>> when DVFS gives us a hint we can clock down and reduce voltage. How on
>> earth do we implement that?
>
>> We can drop the maximum range to be better for the CPU (1.3V is too
>> high, I think this is legacy from when we may have had a sorted 1GHz
>> MX51 coming out) but I can't find any source for where this is hooked
>> in.
>
> DVFS doesn't use the device model in Linux (yet!) so there's no device
> node and it's all a bit messy.  Shawn Guo is working on some generic DT
> bindings for it which should solve that problem but for now in the
> specific case of CPU bindings it's OK to not have an consumer, it's
> understandable what's going on there.

Going over the regulators, what I have so far is every regulator
properly specced with their fixed voltages where it never changes
(which is everything but SW1 and SW2).

There are still a lot of always-on and boot-on regulators since the
design is pretty poor - some pmic regulators that should have specific
consumers have been re-used in other places to supplement other
regulators or derive other voltages for parts of the board you would
usually have discrete logic to support. In this sense, a lot of the
board remains powered or it will just act weird, and a couple of very
specific ones can be turned on and off and don't need to be enabled
for the boot process, but they will be referenced by a metric f&%@ton
of devices as being a consumer so it's unlikely they'll ever actually
power down. In this case I found one always-on and boot-on that
actually doesn't need to be, but for the most part it stays as it is
with refined voltages.

I'm a little confused by the SW1 and SW2 ->VDDGP,VCC sourcing and what
that's actually supplying and what the voltage ranges SHOULD be - as I
think both need to be ranges. VDDGP should probably not be any more
flexible than 0.85V-1.1V.

That said, VCC seems to be only required to be a fixed 1.225V in the
datasheet? 0.9V to 1.85V seems excessive, but it's actually dependent
on load. I don't think anyone ever changes it, it was one of those
things your average board designer would sit down and say, this is
what it needs to be, and it might get bumped when it turns out
something on the board needs more power than the spec. I have NO idea
what this should be. I could just nail it to 1.225V and cover all
bases at the cost of possibly burning a ton of power. I'm looking into
what it's ACTUALLY running at in the old kernels.

It'll take some digging through the old Freescale code about where
they turn regulators on and off in the fixed board files to find this
out and what the original intent was. In the meantime I might be able
to refine the VDDGP/SW2 source. Shawn, any input? And does mainline
care about MX51 TO2 enough that we'd need to implement the voltage
bumps for that?
diff mbox

Patch

diff --git a/arch/arm/boot/dts/imx51-efikamx.dts b/arch/arm/boot/dts/imx51-efikamx.dts
new file mode 100644
index 0000000..dd14f77
--- /dev/null
+++ b/arch/arm/boot/dts/imx51-efikamx.dts
@@ -0,0 +1,247 @@ 
+/*
+ * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011 Linaro Ltd.
+ * Copyright 2012 Genesi USA, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+/dts-v1/;
+/include/ "imx51.dtsi"
+
+/ {
+	model = "Genesi Efika MX (Smarttop)";
+	compatible = "genesi,imx51-efikamx", "fsl,imx51";
+
+	memory {
+		reg = <0x90000000 0x20000000>;
+	};
+
+	soc {
+		aips@70000000 {
+			spba@70000000 {
+				esdhc@70004000 {
+					cd-gpios = <&gpio1 0 0>;
+					wp-gpios = <&gpio1 1 0>;
+					status = "okay";
+				};
+
+				ssi2: ssi@70014000 {
+				    fsl,mode = "i2s-slave";
+				    status = "okay";
+				};
+
+				ecspi@70010000 {
+					fsl,spi-num-chipselects = <2>;
+					cs-gpios = <&gpio4 24 0>, <&gpio4 25 0>;
+					status = "okay";
+
+					pmic: mc13892@0 {
+						#address-cells = <1>;
+						#size-cells = <0>;
+						compatible = "fsl,mc13892";
+						spi-max-frequency = <6000000>;
+						reg = <0>;
+						interrupt-parent = <&gpio1>;
+						interrupts = <6 0x4>;
+						fsl,mc13xxx-uses-rtc;
+
+						regulators {
+							sw1_reg: sw1 {
+								regulator-min-microvolt = <600000>;
+								regulator-max-microvolt = <1375000>;
+								regulator-boot-on;
+								regulator-always-on;
+							};
+
+							sw2_reg: sw2 {
+								regulator-min-microvolt = <900000>;
+								regulator-max-microvolt = <1850000>;
+								regulator-boot-on;
+								regulator-always-on;
+							};
+
+							sw3_reg: sw3 {
+								regulator-min-microvolt = <1100000>;
+								regulator-max-microvolt = <1850000>;
+								regulator-boot-on;
+								regulator-always-on;
+							};
+
+							sw4_reg: sw4 {
+								regulator-min-microvolt = <1100000>;
+								regulator-max-microvolt = <1850000>;
+								regulator-boot-on;
+								regulator-always-on;
+							};
+
+							vpll_reg: vpll {
+								regulator-min-microvolt = <1050000>;
+								regulator-max-microvolt = <1800000>;
+								regulator-boot-on;
+								regulator-always-on;
+							};
+
+							vdig_reg: vdig {
+								regulator-min-microvolt = <1650000>;
+								regulator-max-microvolt = <1650000>;
+								regulator-boot-on;
+							};
+
+							vsd_reg: vsd {
+								regulator-min-microvolt = <3150000>;
+								regulator-max-microvolt = <3150000>;
+							};
+
+							vusb2_reg: vusb2 {
+								regulator-min-microvolt = <2400000>;
+								regulator-max-microvolt = <2775000>;
+								regulator-boot-on;
+								regulator-always-on;
+							};
+
+							vvideo_reg: vvideo {
+								regulator-min-microvolt = <2775000>;
+								regulator-max-microvolt = <2775000>;
+								regulator-boot-on;
+							};
+
+							vaudio_reg: vaudio {
+								regulator-min-microvolt = <2300000>;
+								regulator-max-microvolt = <3000000>;
+							};
+
+							vcam_reg: vcam {
+								regulator-min-microvolt = <2500000>;
+								regulator-max-microvolt = <3000000>;
+							};
+
+							vgen1_reg: vgen1 {
+								regulator-min-microvolt = <1200000>;
+								regulator-max-microvolt = <3150000>;
+							};
+
+							vgen2_reg: vgen2 {
+								regulator-min-microvolt = <3150000>;
+								regulator-max-microvolt = <3150000>;
+								regulator-always-on;
+							};
+
+							vgen3_reg: vgen3 {
+								regulator-min-microvolt = <1800000>;
+								regulator-max-microvolt = <2900000>;
+								regulator-always-on;
+							};
+						};
+					};
+
+					flash: sst25vf032b@1 {
+						#address-cells = <1>;
+						#size-cells = <1>;
+						compatible = "sst,sst25vf032b";
+						spi-max-frequency = <25000000>;
+						reg = <1>;
+
+						partition@0 {
+							label = "firmware";
+							reg = <0x0 0x200000>;
+						};
+
+						partition@200000 {
+							label = "user";
+							reg = <0x200000 0x200000>;
+						};
+					};
+				};
+			};
+
+			wdog@73f98000 {
+				status = "okay";
+			};
+
+			iomuxc@73fa8000 {
+				compatible = "fsl,imx51-iomuxc";
+				reg = <0x73fa8000 0x4000>;
+			};
+
+			uart1: serial@73fbc000 {
+				fsl,uart-has-rtscts;
+				status = "okay";
+			};
+
+		};
+
+		aips@80000000 {
+			sdma@83fb0000 {
+				fsl,sdma-ram-script-name = "imx/sdma/sdma-imx51.bin";
+			};
+
+			i2c@83fc4000 {
+				status = "okay";
+
+				codec: sgtl5000@0a {
+					compatible = "fsl,sgtl5000";
+					reg = <0x0a>;
+					clock-frequency = <12288000>;
+					VDDA-supply = <&vdig_reg>;
+					VDDIO-supply = <&vvideo_reg>;
+				};
+			};
+
+			audmux@83fd0000 {
+				status = "okay";
+			};
+		};
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		power {
+			label = "Power Button";
+			gpios = <&gpio2 31 0>;
+			linux,code = <116>; /* KEY_POWER */
+			gpio-key,wakeup;
+		};
+	};
+
+	gpio-leds {
+		compatible = "gpio-leds";
+
+		red {
+			label = "red";
+			gpios = <&gpio3 13>;
+			linux,default-trigger = "ide-disk";
+		};
+
+		green {
+			label = "green";
+			gpios = <&gpio3 14>;
+			linux,default-trigger = "default-on";
+		};
+
+		blue {
+			label = "blue";
+			gpios = <&gpio3 15>;
+			linux,default-trigger = "mmc0";
+		};
+	};
+
+	sound {
+		compatible = "fsl,imx-audio-sgtl5000";
+		model = "efikamx-sgtl5000";
+		ssi-controller = <&ssi2>;
+		audio-routing =
+			"MIC_IN", "Mic Jack",
+			"Mic Jack", "Mic Bias",
+			"Ext Spk", "LINE_OUT";
+		audio-codec = <&codec>;
+		mux-int-port = <2>;
+		mux-ext-port = <3>;
+	};
+};
diff --git a/arch/arm/mach-imx/Makefile.boot b/arch/arm/mach-imx/Makefile.boot
index 05541cf..3ed7c9d 100644
--- a/arch/arm/mach-imx/Makefile.boot
+++ b/arch/arm/mach-imx/Makefile.boot
@@ -38,7 +38,7 @@  zreladdr-$(CONFIG_SOC_IMX6Q)	+= 0x10008000
 params_phys-$(CONFIG_SOC_IMX6Q)	:= 0x10000100
 initrd_phys-$(CONFIG_SOC_IMX6Q)	:= 0x10800000
 
-dtb-$(CONFIG_MACH_IMX51_DT) += imx51-babbage.dtb
+dtb-$(CONFIG_MACH_IMX51_DT) += imx51-babbage.dtb imx51-efikamx.dtb
 dtb-$(CONFIG_MACH_IMX53_DT) += imx53-ard.dtb imx53-evk.dtb \
 			       imx53-qsb.dtb imx53-smd.dtb
 dtb-$(CONFIG_SOC_IMX6Q)	+= imx6q-arm2.dtb \