* [U-Boot] [PATCH 1/6] arm64: mvebu: Modify the A8K SPI and I2C config in DTS
2016-11-20 15:38 [U-Boot] [PATCH 0/6] arm64: mvebu: Armada-8K family patches kostap at marvell.com
@ 2016-11-20 15:38 ` kostap at marvell.com
2016-11-24 9:02 ` Stefan Roese
2016-11-20 15:38 ` [U-Boot] [PATCH 2/6] arm64: mvebu: Add bubt command for flash image burn kostap at marvell.com
` (4 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: kostap at marvell.com @ 2016-11-20 15:38 UTC (permalink / raw)
To: u-boot
From: Konstantin Porotchkin <kostap@marvell.com>
Align the Armada-8040-db SPI and I2C DTS settings with latest
A8040 DB settings:
- disable i2c0 and spi0 on AP (pins are reserved for eMMC)
- disable cps_i2c0 on CP1
- enable spi1 on CP1 (the new location of the boot flash)
Change-Id: I54698ce4dc8dbe6a2af14099f5bcc3ca3b21d7e1
Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
Cc: Stefan Roese <sr@denx.de>
Cc: Nadav Haklai <nadavh@marvell.com>
Cc: Neta Zur Hershkovits <neta@marvell.com>
Cc: Omri Itach <omrii@marvell.com>
Cc: Igal Liberman <igall@marvell.com>
Cc: Haim Boot <hayim@marvell.com>
Cc: Hanna Hawa <hannah@marvell.com>
---
arch/arm/dts/armada-8040-db.dts | 60 +++++++++++++++++------------------------
1 file changed, 25 insertions(+), 35 deletions(-)
diff --git a/arch/arm/dts/armada-8040-db.dts b/arch/arm/dts/armada-8040-db.dts
index 7fb674b..86666a1 100644
--- a/arch/arm/dts/armada-8040-db.dts
+++ b/arch/arm/dts/armada-8040-db.dts
@@ -57,7 +57,7 @@
aliases {
i2c0 = &cpm_i2c0;
- spi0 = &spi0;
+ spi0 = &cps_spi1;
};
memory at 00000000 {
@@ -66,38 +66,6 @@
};
};
-&i2c0 {
- status = "okay";
- clock-frequency = <100000>;
-};
-
-&spi0 {
- status = "okay";
-
- spi-flash at 0 {
- #address-cells = <1>;
- #size-cells = <1>;
- compatible = "jedec,spi-nor";
- reg = <0>;
- spi-max-frequency = <10000000>;
-
- partitions {
- compatible = "fixed-partitions";
- #address-cells = <1>;
- #size-cells = <1>;
-
- partition at 0 {
- label = "U-Boot";
- reg = <0 0x200000>;
- };
- partition at 400000 {
- label = "Filesystem";
- reg = <0x200000 0xce0000>;
- };
- };
- };
-};
-
/* Accessible over the mini-USB CON9 connector on the main board */
&uart0 {
status = "okay";
@@ -134,9 +102,31 @@
status = "okay";
};
-&cps_i2c0 {
+&cps_spi1 {
status = "okay";
- clock-frequency = <100000>;
+
+ spi-flash at 0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "jedec,spi-nor";
+ reg = <0>;
+ spi-max-frequency = <10000000>;
+
+ partitions {
+ compatible = "fixed-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ partition at 0 {
+ label = "U-Boot";
+ reg = <0 0x200000>;
+ };
+ partition at 400000 {
+ label = "Filesystem";
+ reg = <0x200000 0xce0000>;
+ };
+ };
+ };
};
/* CON4 on CP1 expansion */
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [U-Boot] [PATCH 1/6] arm64: mvebu: Modify the A8K SPI and I2C config in DTS
2016-11-20 15:38 ` [U-Boot] [PATCH 1/6] arm64: mvebu: Modify the A8K SPI and I2C config in DTS kostap at marvell.com
@ 2016-11-24 9:02 ` Stefan Roese
2016-11-24 9:22 ` Kostya Porotchkin
0 siblings, 1 reply; 16+ messages in thread
From: Stefan Roese @ 2016-11-24 9:02 UTC (permalink / raw)
To: u-boot
Hi Kosta,
On 20.11.2016 16:38, kostap at marvell.com wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
>
> Align the Armada-8040-db SPI and I2C DTS settings with latest
> A8040 DB settings:
> - disable i2c0 and spi0 on AP (pins are reserved for eMMC)
> - disable cps_i2c0 on CP1
> - enable spi1 on CP1 (the new location of the boot flash)
Thanks. I understand that the current SPI / I2C settings are
still valid for boards with earlier SoC revisions. Is this
correct? Would it make sense to move the old version into
a new file then, perhaps:
arch/arm/dts/armada-8040-db-revA.dts
?
This would be handy for users of this version at least for a
short period of time. This new file can be removed once its
not needed any more in a few months.
If you think this is a good idea, could you please add this
new file in a new patch to this patchset in v2?
> Change-Id: I54698ce4dc8dbe6a2af14099f5bcc3ca3b21d7e1
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Nadav Haklai <nadavh@marvell.com>
> Cc: Neta Zur Hershkovits <neta@marvell.com>
> Cc: Omri Itach <omrii@marvell.com>
> Cc: Igal Liberman <igall@marvell.com>
> Cc: Haim Boot <hayim@marvell.com>
> Cc: Hanna Hawa <hannah@marvell.com>
> ---
> arch/arm/dts/armada-8040-db.dts | 60 +++++++++++++++++------------------------
> 1 file changed, 25 insertions(+), 35 deletions(-)
>
> diff --git a/arch/arm/dts/armada-8040-db.dts b/arch/arm/dts/armada-8040-db.dts
> index 7fb674b..86666a1 100644
> --- a/arch/arm/dts/armada-8040-db.dts
> +++ b/arch/arm/dts/armada-8040-db.dts
> @@ -57,7 +57,7 @@
>
> aliases {
> i2c0 = &cpm_i2c0;
> - spi0 = &spi0;
> + spi0 = &cps_spi1;
> };
>
> memory at 00000000 {
> @@ -66,38 +66,6 @@
> };
> };
>
> -&i2c0 {
> - status = "okay";
> - clock-frequency = <100000>;
> -};
> -
> -&spi0 {
> - status = "okay";
> -
> - spi-flash at 0 {
> - #address-cells = <1>;
> - #size-cells = <1>;
> - compatible = "jedec,spi-nor";
> - reg = <0>;
> - spi-max-frequency = <10000000>;
> -
> - partitions {
> - compatible = "fixed-partitions";
> - #address-cells = <1>;
> - #size-cells = <1>;
> -
> - partition at 0 {
> - label = "U-Boot";
> - reg = <0 0x200000>;
> - };
> - partition at 400000 {
> - label = "Filesystem";
> - reg = <0x200000 0xce0000>;
> - };
> - };
> - };
> -};
> -
> /* Accessible over the mini-USB CON9 connector on the main board */
> &uart0 {
> status = "okay";
> @@ -134,9 +102,31 @@
> status = "okay";
> };
>
> -&cps_i2c0 {
> +&cps_spi1 {
> status = "okay";
> - clock-frequency = <100000>;
> +
> + spi-flash at 0 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "jedec,spi-nor";
> + reg = <0>;
> + spi-max-frequency = <10000000>;
> +
> + partitions {
> + compatible = "fixed-partitions";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + partition at 0 {
> + label = "U-Boot";
> + reg = <0 0x200000>;
> + };
> + partition at 400000 {
> + label = "Filesystem";
> + reg = <0x200000 0xce0000>;
> + };
> + };
> + };
> };
>
> /* CON4 on CP1 expansion */
>
Thanks,
Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread* [U-Boot] [PATCH 1/6] arm64: mvebu: Modify the A8K SPI and I2C config in DTS
2016-11-24 9:02 ` Stefan Roese
@ 2016-11-24 9:22 ` Kostya Porotchkin
2016-11-24 12:05 ` Stefan Roese
0 siblings, 1 reply; 16+ messages in thread
From: Kostya Porotchkin @ 2016-11-24 9:22 UTC (permalink / raw)
To: u-boot
Hi, Stefan,
The A0 SoC is not for production, it is engineering samples only.
Marvell has no plans for supporting this SoC in the future.
I believe that once you get a replacement for your board there will be no other customers using A0 SoC.
Regard
Kosta
________________________________________
From: Stefan Roese <sr@denx.de>
Sent: Thursday, November 24, 2016 11:02
To: Kostya Porotchkin; u-boot at lists.denx.de
Cc: Nadav Haklai; Neta Zur Hershkovits; Omri Itach; Igal Liberman; Haim Boot; Hanna Hawa
Subject: Re: [PATCH 1/6] arm64: mvebu: Modify the A8K SPI and I2C config in DTS
Hi Kosta,
On 20.11.2016 16:38, kostap at marvell.com wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
>
> Align the Armada-8040-db SPI and I2C DTS settings with latest
> A8040 DB settings:
> - disable i2c0 and spi0 on AP (pins are reserved for eMMC)
> - disable cps_i2c0 on CP1
> - enable spi1 on CP1 (the new location of the boot flash)
Thanks. I understand that the current SPI / I2C settings are
still valid for boards with earlier SoC revisions. Is this
correct? Would it make sense to move the old version into
a new file then, perhaps:
arch/arm/dts/armada-8040-db-revA.dts
?
This would be handy for users of this version at least for a
short period of time. This new file can be removed once its
not needed any more in a few months.
If you think this is a good idea, could you please add this
new file in a new patch to this patchset in v2?
> Change-Id: I54698ce4dc8dbe6a2af14099f5bcc3ca3b21d7e1
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Nadav Haklai <nadavh@marvell.com>
> Cc: Neta Zur Hershkovits <neta@marvell.com>
> Cc: Omri Itach <omrii@marvell.com>
> Cc: Igal Liberman <igall@marvell.com>
> Cc: Haim Boot <hayim@marvell.com>
> Cc: Hanna Hawa <hannah@marvell.com>
> ---
> arch/arm/dts/armada-8040-db.dts | 60 +++++++++++++++++------------------------
> 1 file changed, 25 insertions(+), 35 deletions(-)
>
> diff --git a/arch/arm/dts/armada-8040-db.dts b/arch/arm/dts/armada-8040-db.dts
> index 7fb674b..86666a1 100644
> --- a/arch/arm/dts/armada-8040-db.dts
> +++ b/arch/arm/dts/armada-8040-db.dts
> @@ -57,7 +57,7 @@
>
> aliases {
> i2c0 = &cpm_i2c0;
> - spi0 = &spi0;
> + spi0 = &cps_spi1;
> };
>
> memory at 00000000 {
> @@ -66,38 +66,6 @@
> };
> };
>
> -&i2c0 {
> - status = "okay";
> - clock-frequency = <100000>;
> -};
> -
> -&spi0 {
> - status = "okay";
> -
> - spi-flash at 0 {
> - #address-cells = <1>;
> - #size-cells = <1>;
> - compatible = "jedec,spi-nor";
> - reg = <0>;
> - spi-max-frequency = <10000000>;
> -
> - partitions {
> - compatible = "fixed-partitions";
> - #address-cells = <1>;
> - #size-cells = <1>;
> -
> - partition at 0 {
> - label = "U-Boot";
> - reg = <0 0x200000>;
> - };
> - partition at 400000 {
> - label = "Filesystem";
> - reg = <0x200000 0xce0000>;
> - };
> - };
> - };
> -};
> -
> /* Accessible over the mini-USB CON9 connector on the main board */
> &uart0 {
> status = "okay";
> @@ -134,9 +102,31 @@
> status = "okay";
> };
>
> -&cps_i2c0 {
> +&cps_spi1 {
> status = "okay";
> - clock-frequency = <100000>;
> +
> + spi-flash at 0 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "jedec,spi-nor";
> + reg = <0>;
> + spi-max-frequency = <10000000>;
> +
> + partitions {
> + compatible = "fixed-partitions";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + partition at 0 {
> + label = "U-Boot";
> + reg = <0 0x200000>;
> + };
> + partition at 400000 {
> + label = "Filesystem";
> + reg = <0x200000 0xce0000>;
> + };
> + };
> + };
> };
>
> /* CON4 on CP1 expansion */
>
Thanks,
Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread* [U-Boot] [PATCH 1/6] arm64: mvebu: Modify the A8K SPI and I2C config in DTS
2016-11-24 9:22 ` Kostya Porotchkin
@ 2016-11-24 12:05 ` Stefan Roese
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Roese @ 2016-11-24 12:05 UTC (permalink / raw)
To: u-boot
Hi Kosta,
On 24.11.2016 10:22, Kostya Porotchkin wrote:
> The A0 SoC is not for production, it is engineering samples
> only.
> Marvell has no plans for supporting this SoC in the future.
Of course.
> I believe that once you get a replacement for your board
> there will be no other customers using A0 SoC.
Okay. Then I will keep a copy of this file locally for testing
in the next weeks.
Thanks,
Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH 2/6] arm64: mvebu: Add bubt command for flash image burn
2016-11-20 15:38 [U-Boot] [PATCH 0/6] arm64: mvebu: Armada-8K family patches kostap at marvell.com
2016-11-20 15:38 ` [U-Boot] [PATCH 1/6] arm64: mvebu: Modify the A8K SPI and I2C config in DTS kostap at marvell.com
@ 2016-11-20 15:38 ` kostap at marvell.com
2016-11-24 8:58 ` Stefan Roese
2016-11-20 15:38 ` [U-Boot] [PATCH 3/6] arm64: mvebu: pinctrl: Add pin control driver for A8K family kostap at marvell.com
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: kostap at marvell.com @ 2016-11-20 15:38 UTC (permalink / raw)
To: u-boot
From: Konstantin Porotchkin <kostap@marvell.com>
Add support for mvebu bubt command for flash image
load, check and burn on boot device.
Change-Id: If2b971069ae44232761b601bbbcf0162567f5603
Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
Cc: Stefan Roese <sr@denx.de>
Cc: Nadav Haklai <nadavh@marvell.com>
Cc: Neta Zur Hershkovits <neta@marvell.com>
Cc: Omri Itach <omrii@marvell.com>
Cc: Igal Liberman <igall@marvell.com>
Cc: Haim Boot <hayim@marvell.com>
Cc: Hanna Hawa <hannah@marvell.com>
---
arch/arm/mach-mvebu/Kconfig | 30 ++
cmd/Kconfig | 3 +
cmd/Makefile | 2 +
cmd/mvebu/Kconfig | 10 +
cmd/mvebu/Makefile | 19 ++
cmd/mvebu/bubt.c | 699 ++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 763 insertions(+)
create mode 100644 cmd/mvebu/Kconfig
create mode 100644 cmd/mvebu/Makefile
create mode 100644 cmd/mvebu/bubt.c
diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
index 7248fd7..6de85d5 100644
--- a/arch/arm/mach-mvebu/Kconfig
+++ b/arch/arm/mach-mvebu/Kconfig
@@ -99,6 +99,36 @@ config TARGET_THEADORABLE
endchoice
+choice
+ prompt "Flash for image"
+ default MVEBU_SPI_BOOT
+ depends on (TARGET_MVEBU_DB_88F3720 || TARGET_MVEBU_ARMADA_8K)
+
+config MVEBU_NAND_BOOT
+ bool "NAND flash boot"
+ depends on NAND_PXA3XX
+ help
+ Enable boot from NAND
+
+config MVEBU_SPI_BOOT
+ bool "SPI flash boot"
+ depends on SPI_FLASH
+
+config MVEBU_MMC_BOOT
+ bool "eMMC flash boot"
+ depends on MVEBU_MMC
+ help
+ Enable boot from eMMC boot partition
+
+endchoice
+
+config MVEBU_UBOOT_DFLT_NAME
+ string "Default image name for bubt command"
+ default "flash-image.bin"
+ help
+ This option should contain a default file name to be used with
+ MVEBU "bubt" command if the source file name is omitted
+
config SYS_BOARD
default "clearfog" if TARGET_CLEARFOG
default "mvebu_db-88f3720" if TARGET_MVEBU_DB_88F3720
diff --git a/cmd/Kconfig b/cmd/Kconfig
index e339d86..39dd0a0 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -610,6 +610,9 @@ config CMD_QFW
This provides access to the QEMU firmware interface. The main
feature is to allow easy loading of files passed to qemu-system
via -kernel / -initrd
+
+source "cmd/mvebu/Kconfig"
+
endmenu
config CMD_BOOTSTAGE
diff --git a/cmd/Makefile b/cmd/Makefile
index 9c9a9d1..34bc544 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -163,3 +163,5 @@ obj-$(CONFIG_CMD_BLOB) += blob.o
# core command
obj-y += nvedit.o
+
+obj-$(CONFIG_ARCH_MVEBU) += mvebu/
diff --git a/cmd/mvebu/Kconfig b/cmd/mvebu/Kconfig
new file mode 100644
index 0000000..e7fbd20
--- /dev/null
+++ b/cmd/mvebu/Kconfig
@@ -0,0 +1,10 @@
+menu "MVEBU commands"
+depends on ARCH_MVEBU
+
+config CMD_MVEBU_BUBT
+ bool "bubt"
+ default n
+ help
+ bubt - Burn a u-boot image to flash
+
+endmenu
diff --git a/cmd/mvebu/Makefile b/cmd/mvebu/Makefile
new file mode 100644
index 0000000..b0817e3
--- /dev/null
+++ b/cmd/mvebu/Makefile
@@ -0,0 +1,19 @@
+#
+# ***************************************************************************
+# Copyright (C) 2015 Marvell International Ltd.
+# ***************************************************************************
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by the Free
+# Software Foundation, either version 2 of the License, or any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+# ***************************************************************************
+#
+
+obj-$(CONFIG_CMD_MVEBU_BUBT) += bubt.o
diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
new file mode 100644
index 0000000..6539063
--- /dev/null
+++ b/cmd/mvebu/bubt.c
@@ -0,0 +1,699 @@
+/*
+ * ***************************************************************************
+ * Copyright (C) 2015 Marvell International Ltd.
+ * ***************************************************************************
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation, either version 2 of the License, or any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ * ***************************************************************************
+ */
+
+#include <config.h>
+#include <common.h>
+#include <command.h>
+#include <vsprintf.h>
+#include <errno.h>
+#include <dm.h>
+
+#include <spi_flash.h>
+#include <spi.h>
+#include <nand.h>
+#include <usb.h>
+#include <fs.h>
+#include <mmc.h>
+#include <u-boot/sha1.h>
+#include <u-boot/sha256.h>
+
+#if defined(CONFIG_TARGET_MVEBU_ARMADA_8K)
+#define MAIN_HDR_MAGIC 0xB105B002
+
+struct mvebu_image_header {
+ uint32_t magic; /* 0-3 */
+ uint32_t prolog_size; /* 4-7 */
+ uint32_t prolog_checksum; /* 8-11 */
+ uint32_t boot_image_size; /* 12-15 */
+ uint32_t boot_image_checksum; /* 16-19 */
+ uint32_t rsrvd0; /* 20-23 */
+ uint32_t load_addr; /* 24-27 */
+ uint32_t exec_addr; /* 28-31 */
+ uint8_t uart_cfg; /* 32 */
+ uint8_t baudrate; /* 33 */
+ uint8_t ext_count; /* 34 */
+ uint8_t aux_flags; /* 35 */
+ uint32_t io_arg_0; /* 36-39 */
+ uint32_t io_arg_1; /* 40-43 */
+ uint32_t io_arg_2; /* 43-47 */
+ uint32_t io_arg_3; /* 48-51 */
+ uint32_t rsrvd1; /* 52-55 */
+ uint32_t rsrvd2; /* 56-59 */
+ uint32_t rsrvd3; /* 60-63 */
+};
+#elif defined(CONFIG_TARGET_MVEBU_DB_88F3720) /* A3700 */
+#define HASH_SUM_LEN 16
+#define IMAGE_VERSION_3_6_0 0x030600
+#define IMAGE_VERSION_3_5_0 0x030500
+
+struct common_tim_data {
+ u32 version;
+ u32 identifier;
+ u32 trusted;
+ u32 issue_date;
+ u32 oem_unique_id;
+ u32 reserved[5]; /* Reserve 20 bytes */
+ u32 boot_flash_sign;
+ u32 num_images;
+ u32 num_keys;
+ u32 size_of_reserved;
+};
+
+struct mvebu_image_info {
+ u32 image_id;
+ u32 next_image_id;
+ u32 flash_entry_addr;
+ u32 load_addr;
+ u32 image_size;
+ u32 image_size_to_hash;
+ u32 hash_algorithm_id;
+ u32 hash[HASH_SUM_LEN]; /* Reserve 512 bits for the hash */
+ u32 partition_number;
+ u32 enc_algorithm_id;
+ u32 encrypt_start_offset;
+ u32 encrypt_size;
+};
+#endif
+
+struct bubt_dev {
+ char name[8];
+ size_t (*read)(const char *file_name);
+ int (*write)(size_t image_size);
+ int (*active)(void);
+};
+
+static ulong get_load_addr(void)
+{
+ const char *addr_str;
+ unsigned long addr;
+
+ addr_str = getenv("loadaddr");
+ if (addr_str != NULL)
+ addr = simple_strtoul(addr_str, NULL, 16);
+ else
+ addr = CONFIG_SYS_LOAD_ADDR;
+
+ return addr;
+}
+
+/********************************************************************
+ * eMMC services
+ ********************************************************************/
+#ifdef CONFIG_DM_MMC
+static int mmc_burn_image(size_t image_size)
+{
+ struct mmc *mmc;
+ lbaint_t start_lba;
+ lbaint_t blk_count;
+ ulong blk_written;
+#ifdef CONFIG_SYS_MMC_ENV_DEV
+ const u8 mmc_dev_num = CONFIG_SYS_MMC_ENV_DEV;
+#else
+ const u8 mmc_dev_num = 0;
+#endif
+
+ mmc = find_mmc_device(mmc_dev_num);
+ if (!mmc) {
+ printf("No SD/MMC/eMMC card found\n");
+ return 1;
+ }
+
+ if (mmc_init(mmc)) {
+ printf("%s(%d) init failed\n", IS_SD(mmc) ? "SD" : "MMC", mmc_dev_num);
+ return 1;
+ }
+
+#ifdef CONFIG_SYS_MMC_ENV_PART
+ if (CONFIG_SYS_MMC_ENV_PART != mmc->part_num) {
+ if (mmc_switch_part(mmc_dev_num, CONFIG_SYS_MMC_ENV_PART)) {
+ printf("MMC partition switch failed\n");
+ return 1;
+ }
+ }
+#endif
+
+ /* SD reserves LBA-0 for MBR and boots from LBA-1, MMC/eMMC boots from LBA-0 */
+ start_lba = IS_SD(mmc) ? 1 : 0;
+ blk_count = image_size / mmc->block_dev.blksz;
+ if (image_size % mmc->block_dev.blksz)
+ blk_count += 1;
+
+ blk_written = mmc->block_dev.block_write(mmc_dev_num,
+ start_lba, blk_count, (void *)get_load_addr());
+ if (blk_written != blk_count) {
+ printf("Error - written %#lx blocks\n", blk_written);
+ return 1;
+ } else {
+ printf("Done!\n");
+ }
+
+#ifdef CONFIG_SYS_MMC_ENV_PART
+ if (CONFIG_SYS_MMC_ENV_PART != mmc->part_num)
+ mmc_switch_part(mmc_dev_num, mmc->part_num);
+#endif
+
+ return 0;
+}
+
+static size_t mmc_read_file(const char *file_name)
+{
+ loff_t act_read = 0;
+ int rc;
+ struct mmc *mmc;
+#ifdef CONFIG_SYS_MMC_ENV_DEV
+ const u8 mmc_dev_num = CONFIG_SYS_MMC_ENV_DEV;
+#else
+ const u8 mmc_dev_num = 0;
+#endif
+
+ mmc = find_mmc_device(mmc_dev_num);
+ if (!mmc) {
+ printf("No SD/MMC/eMMC card found\n");
+ return 0;
+ }
+
+ if (mmc_init(mmc)) {
+ printf("%s(%d) init failed\n", IS_SD(mmc) ? "SD" : "MMC", mmc_dev_num);
+ return 0;
+ }
+
+ /* Load from data partition (0) */
+ if (fs_set_blk_dev("mmc", "0", FS_TYPE_ANY)) {
+ printf("Error: MMC 0 not found\n");
+ return 0;
+ }
+
+ /* Perfrom file read */
+ rc = fs_read(file_name, get_load_addr(), 0, 0, &act_read);
+ if (rc)
+ return 0;
+
+ return act_read;
+}
+
+int is_mmc_active(void)
+{
+ return 1;
+}
+#else /* CONFIG_DM_MMC */
+#define mmc_burn_image 0
+#define mmc_read_file 0
+#define is_mmc_active 0
+#endif /* CONFIG_DM_MMC */
+
+
+/********************************************************************
+ * SPI services
+ ********************************************************************/
+#ifdef CONFIG_SPI_FLASH
+static int spi_burn_image(size_t image_size)
+{
+ int ret;
+ struct spi_flash *flash;
+ uint32_t erase_bytes;
+
+ /* Probe the SPI bus to get the flash device */
+ flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
+ CONFIG_SF_DEFAULT_SPEED, CONFIG_SF_DEFAULT_MODE);
+ if (!flash) {
+ printf("Failed to probe SPI Flash\n");
+ return 1;
+ }
+
+#ifdef CONFIG_SPI_FLASH_PROTECTION
+ spi_flash_protect(flash, 0);
+#endif
+ erase_bytes = image_size + (flash->erase_size - image_size % flash->erase_size);
+ printf("Erasing %d bytes (%d blocks) at offset 0 ...", erase_bytes, erase_bytes/flash->erase_size);
+ ret = spi_flash_erase(flash, 0, erase_bytes);
+ if (ret)
+ printf("Error!\n");
+ else
+ printf("Done!\n");
+
+ printf("Writing %d bytes from 0x%lx to offset 0 ...", (int)image_size, get_load_addr());
+ ret = spi_flash_write(flash, 0, image_size, (void *)get_load_addr());
+ if (ret)
+ printf("Error!\n");
+ else
+ printf("Done!\n");
+
+#ifdef CONFIG_SPI_FLASH_PROTECTION
+ spi_flash_protect(flash, 1);
+#endif
+
+ return ret;
+}
+
+int is_spi_active(void)
+{
+ return 1;
+}
+#else /* CONFIG_SPI_FLASH */
+#define spi_burn_image 0
+#define is_spi_active 0
+#endif /* CONFIG_SPI_FLASH */
+
+/********************************************************************
+ * NAND services
+ ********************************************************************/
+#ifdef CONFIG_CMD_NAND
+static int nand_burn_image(size_t image_size)
+{
+ int ret, block_size;
+ nand_info_t *nand;
+ int dev = nand_curr_device;
+
+ if ((dev < 0) || (dev >= CONFIG_SYS_MAX_NAND_DEVICE) ||
+ (!nand_info[dev].name)) {
+ puts("\nno devices available\n");
+ return 1;
+ }
+ nand = &nand_info[dev];
+ block_size = nand->erasesize;
+
+ /* Align U-Boot size to currently used blocksize */
+ image_size = ((image_size + (block_size - 1)) & (~(block_size-1)));
+
+ /* Erase the U-BOOT image space */
+ printf("Erasing 0x%x - 0x%x:...", 0, (int)image_size);
+ ret = nand_erase(nand, 0, image_size);
+ if (ret) {
+ printf("Error!\n");
+ goto error;
+ }
+ printf("Done!\n");
+
+ /* Write the image to flash */
+ printf("Writing image:...");
+ printf("&image_size = 0x%p\n", (void*)&image_size);
+ ret = nand_write(nand, 0, &image_size, (void *)get_load_addr());
+ if (ret)
+ printf("Error!\n");
+ else
+ printf("Done!\n");
+
+error:
+ return ret;
+}
+
+int is_nand_active(void)
+{
+ return 1;
+}
+#else /* CONFIG_CMD_NAND */
+#define nand_burn_image 0
+#define is_nand_active 0
+#endif /* CONFIG_CMD_NAND */
+
+/********************************************************************
+ * USB services
+ ********************************************************************/
+#if defined(CONFIG_USB_STORAGE) && defined (CONFIG_BLK)
+static size_t usb_read_file(const char *file_name)
+{
+ loff_t act_read = 0;
+ struct udevice *dev;
+ int rc;
+
+ usb_stop();
+
+ if (usb_init() < 0) {
+ printf("Error: usb_init failed\n");
+ return 0;
+ }
+
+ /* Try to recognize storage devices immediately */
+ blk_first_device(IF_TYPE_USB, &dev);
+ if (dev == NULL) {
+ printf("Error: USB storage device not found\n");
+ return 0;
+ }
+
+ /* Always load from usb 0 */
+ if (fs_set_blk_dev("usb", "0", FS_TYPE_ANY)) {
+ printf("Error: USB 0 not found\n");
+ return 0;
+ }
+
+ /* Perfrom file read */
+ rc = fs_read(file_name, get_load_addr(), 0, 0, &act_read);
+ if (rc)
+ return 0;
+
+ return act_read;
+}
+
+int is_usb_active(void)
+{
+ return 1;
+}
+#else /* defined(CONFIG_USB_STORAGE) && defined (CONFIG_BLK) */
+#define usb_read_file 0
+#define is_usb_active 0
+#endif /* defined(CONFIG_USB_STORAGE) && defined (CONFIG_BLK) */
+
+/********************************************************************
+ * Network services
+ ********************************************************************/
+#ifdef CONFIG_CMD_NET
+static size_t tftp_read_file(const char *file_name)
+{
+ /* update global variable load_addr before tftp file from network */
+ load_addr = get_load_addr();
+ return net_loop(TFTPGET);
+}
+
+int is_tftp_active(void)
+{
+ return 1;
+}
+#else
+#define tftp_read_file 0
+#define is_tftp_active 0
+#endif /* CONFIG_CMD_NET */
+
+
+enum bubt_devices {
+ BUBT_DEV_NET = 0,
+ BUBT_DEV_USB,
+ BUBT_DEV_MMC,
+ BUBT_DEV_SPI,
+ BUBT_DEV_NAND,
+
+ BUBT_MAX_DEV
+};
+
+struct bubt_dev bubt_devs[BUBT_MAX_DEV] = {
+ {"tftp", tftp_read_file, NULL, is_tftp_active},
+ {"usb", usb_read_file, NULL, is_usb_active},
+ {"mmc", mmc_read_file, mmc_burn_image, is_mmc_active},
+ {"spi", NULL, spi_burn_image, is_spi_active},
+ {"nand", NULL, nand_burn_image, is_nand_active},
+};
+
+static int bubt_write_file(struct bubt_dev *dst, size_t image_size)
+{
+ if (!dst->write) {
+ printf("Error: Write not supported on device %s\n", dst->name);
+ return 1;
+ }
+
+ return dst->write(image_size);
+}
+
+#if defined(CONFIG_TARGET_MVEBU_ARMADA_8K)
+uint32_t do_checksum32(uint32_t *start, int32_t len)
+{
+ uint32_t sum = 0;
+ uint32_t *startp = start;
+
+ do {
+ sum += *startp;
+ startp++;
+ len -= 4;
+ } while (len > 0);
+
+ return sum;
+}
+
+static int check_image_header(void)
+{
+ struct mvebu_image_header *hdr = (struct mvebu_image_header *)get_load_addr();
+ uint32_t header_len = hdr->prolog_size;
+ uint32_t checksum;
+ uint32_t checksum_ref = hdr->prolog_checksum;
+
+ /*
+ * For now compare checksum, and magic. Later we can
+ * verify more stuff on the header like interface type, etc
+ */
+ if (hdr->magic != MAIN_HDR_MAGIC) {
+ printf("ERROR: Bad MAGIC 0x%08x != 0x%08x\n", hdr->magic, MAIN_HDR_MAGIC);
+ return -ENOEXEC;
+ }
+
+ /* The checksum value is discarded from checksum calculation */
+ hdr->prolog_checksum = 0;
+
+ checksum = do_checksum32((uint32_t *)hdr, header_len);
+ if (checksum != checksum_ref) {
+ printf("Error: Bad Image checksum. 0x%x != 0x%x\n", checksum, checksum_ref);
+ return -ENOEXEC;
+ }
+
+ /* Restore the checksum before writing */
+ hdr->prolog_checksum = checksum_ref;
+ printf("Image checksum...OK!\n");
+
+ return 0;
+}
+#elif defined(CONFIG_TARGET_MVEBU_DB_88F3720) /* Armada 3700 */
+static int check_image_header(void)
+{
+ struct common_tim_data *hdr = (struct common_tim_data *)get_load_addr();
+ int image_num;
+ u8 hash_160_output[SHA1_SUM_LEN];
+ u8 hash_256_output[SHA256_SUM_LEN];
+ sha1_context hash1_text;
+ sha256_context hash256_text;
+ u8 *hash_output;
+ u32 hash_algorithm_id;
+ u32 image_size_to_hash;
+ u32 flash_entry_addr;
+ u32 *hash_value;
+ u32 internal_hash[HASH_SUM_LEN];
+ const uint8_t *buff;
+ u32 num_of_image = hdr->num_images;
+ u32 version = hdr->version;
+ u32 trusted = hdr->trusted;
+
+ /* bubt checksum validation only supports nontrusted images */
+ if (trusted == 1) {
+ printf("bypass image validation, only untrusted image is supported now\n");
+ return 0;
+ }
+ /* only supports image version 3.5 and 3.6 */
+ if (version != IMAGE_VERSION_3_5_0 && version != IMAGE_VERSION_3_6_0) {
+ printf("Error: Unsupported Image version = 0x%08x\n", version);
+ return -ENOEXEC;
+ }
+ /* validate images hash value */
+ for (image_num = 0; image_num < num_of_image; image_num++) {
+ struct mvebu_image_info *info = (struct mvebu_image_info *)(get_load_addr()
+ + sizeof(struct common_tim_data) + image_num * sizeof(struct mvebu_image_info));
+ hash_algorithm_id = info->hash_algorithm_id;
+ image_size_to_hash = info->image_size_to_hash;
+ flash_entry_addr = info->flash_entry_addr;
+ hash_value = info->hash;
+ buff = (const uint8_t *)(get_load_addr() + flash_entry_addr);
+
+ if (image_num == 0) {
+ /* The first image includes hash values in itself content. For hash calculation, we need
+ * to save original hash values to local variable that will be copied back for comparsion
+ * and set all zeros to replace orignal hash values to calculate its new hash value.
+ * First image original format : x...x (datum1) x...x(original hash values) x...x(datum2)
+ * Replaced first image format : x...x (datum1) 0...0(hash values) x...x(datum2)
+ */
+ memcpy(internal_hash, hash_value, sizeof(internal_hash));
+ memset(hash_value, 0, sizeof(internal_hash));
+ }
+ if (image_size_to_hash == 0) {
+ printf("Warning: Image_%d hash checksum is disable, skip the image validation.\n", image_num);
+ continue;
+ }
+ switch (hash_algorithm_id) {
+ case SHA1_SUM_LEN:
+ sha1_starts(&hash1_text);
+ sha1_update(&hash1_text, buff, image_size_to_hash);
+ sha1_finish(&hash1_text, hash_160_output);
+ hash_output = hash_160_output;
+ break;
+ case SHA256_SUM_LEN:
+ sha256_starts(&hash256_text);
+ sha256_update(&hash256_text, buff, image_size_to_hash);
+ sha256_finish(&hash256_text, hash_256_output);
+ hash_output = hash_256_output;
+ break;
+ default:
+ printf("Error: Unsupported hash_algorithm_id = %d\n", hash_algorithm_id);
+ return -ENOEXEC;
+ }
+ if (image_num == 0)
+ memcpy(hash_value, internal_hash, sizeof(internal_hash));
+ if (memcmp(hash_value, hash_output, hash_algorithm_id) != 0) {
+ printf("Error: Image_%d checksum is not correct\n", image_num);
+ return -ENOEXEC;
+ }
+ }
+ printf("Image checksum...OK!\n");
+ return 0;
+}
+#else
+static int check_image_header(void)
+{
+ printf("bubt cmd does not support this SoC device or family!\n");
+ return -ENOEXEC;
+}
+#endif
+
+static int bubt_verify(size_t image_size)
+{
+
+ /* Check a correct image header exists */
+ if (check_image_header()) {
+ printf("Error: Image header verification failed\n");
+ return 1;
+ }
+ return 0;
+}
+
+
+static int bubt_read_file(struct bubt_dev *src)
+{
+ size_t image_size;
+
+ if (!src->read) {
+ printf("Error: Read not supported on device \"%s\"\n", src->name);
+ return 0;
+ }
+
+ image_size = src->read(net_boot_file_name);
+ if (image_size <= 0) {
+ printf("Error: Failed to read file %s from %s\n", net_boot_file_name, src->name);
+ return 0;
+ }
+
+ return image_size;
+}
+
+static int bubt_is_dev_active(struct bubt_dev *dev)
+{
+ if (!dev->active) {
+ printf("Device \"%s\" not supported by U-BOOT image\n", dev->name);
+ return 0;
+ }
+
+ if (!dev->active()) {
+ printf("Device \"%s\" is inactive\n", dev->name);
+ return 0;
+ }
+
+ return 1;
+}
+
+struct bubt_dev *find_bubt_dev(char *dev_name)
+{
+ int dev;
+
+ for (dev = 0; dev < BUBT_MAX_DEV; dev++) {
+ if (strcmp(bubt_devs[dev].name, dev_name) == 0)
+ return &bubt_devs[dev];
+ }
+
+ return 0;
+}
+
+#define DEFAULT_BUBT_SRC "tftp"
+
+#ifndef DEFAULT_BUBT_DST
+#ifdef CONFIG_MVEBU_SPI_BOOT
+#define DEFAULT_BUBT_DST "spi"
+#elif defined(CONFIG_MVEBU_NAND_BOOT)
+#define DEFAULT_BUBT_DST "nand"
+#elif defined(CONFIG_MVEBU_MMC_BOOT)
+#define DEFAULT_BUBT_DST "mmc"
+else
+#define DEFAULT_BUBT_DST "error"
+#endif
+#endif /* DEFAULT_BUBT_DST */
+
+int do_bubt_cmd(cmd_tbl_t *cmdtp, int flag, int argc,
+ char * const argv[])
+{
+ struct bubt_dev *src, *dst;
+ size_t image_size;
+ char src_dev_name[8];
+ char dst_dev_name[8];
+ char *name;
+
+ if (argc < 2)
+ copy_filename(net_boot_file_name, CONFIG_MVEBU_UBOOT_DFLT_NAME, sizeof(net_boot_file_name));
+ else
+ copy_filename(net_boot_file_name, argv[1], sizeof(net_boot_file_name));
+
+ if (argc >= 3) {
+ strncpy(dst_dev_name, argv[2], 8);
+ } else {
+ name = DEFAULT_BUBT_DST;
+ strncpy(dst_dev_name, name, 8);
+ }
+
+ if (argc >= 4)
+ strncpy(src_dev_name, argv[3], 8);
+ else
+ strncpy(src_dev_name, DEFAULT_BUBT_SRC, 8);
+
+ /* Figure out the destination device */
+ dst = find_bubt_dev(dst_dev_name);
+ if (!dst) {
+ printf("Error: Unknown destination \"%s\"\n", dst_dev_name);
+ return 1;
+ }
+
+ if (!bubt_is_dev_active(dst))
+ return 1;
+
+ /* Figure out the source device */
+ src = find_bubt_dev(src_dev_name);
+ if (!src) {
+ printf("Error: Unknown source \"%s\"\n", src_dev_name);
+ return 1;
+ }
+
+ if (!bubt_is_dev_active(src))
+ return 1;
+
+ printf("Burning U-BOOT image \"%s\" from \"%s\" to \"%s\"\n", net_boot_file_name, src->name, dst->name);
+
+ image_size = bubt_read_file(src);
+ if (!image_size)
+ return 1;
+
+ if (bubt_verify(image_size))
+ return 1;
+
+ if (bubt_write_file(dst, image_size))
+ return 1;
+
+ return 0;
+}
+
+U_BOOT_CMD(
+ bubt, 4, 0, do_bubt_cmd,
+ "Burn a u-boot image to flash",
+ "[file-name] [destination [source]]\n"
+ "\t-file-name The image file name to burn. Default = u-boot.bin\n"
+ "\t-destination Flash to burn to [spi, nand, mmc]. Default = active boot device\n"
+ "\t-source The source to load image from [tftp, usb, mmc]. Default = tftp\n"
+ "Examples:\n"
+ "\tbubt - Burn flash-image.bin from tftp to active boot device\n"
+ "\tbubt flash-image-new.bin nand - Burn flash-image-new.bin from tftp to NAND flash\n"
+ "\tbubt backup-flash-image.bin mmc usb - Burn backup-flash-image.bin from usb to MMC\n"
+
+);
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [U-Boot] [PATCH 2/6] arm64: mvebu: Add bubt command for flash image burn
2016-11-20 15:38 ` [U-Boot] [PATCH 2/6] arm64: mvebu: Add bubt command for flash image burn kostap at marvell.com
@ 2016-11-24 8:58 ` Stefan Roese
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Roese @ 2016-11-24 8:58 UTC (permalink / raw)
To: u-boot
Hi Kosta,
a general comment: Please use scripts/checkpatch before sending
the patches to the list to make sure that all (mostly style
related) issues are resolved.
Please find some more mostly nitpicking comments below.
On 20.11.2016 16:38, kostap at marvell.com wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
>
> Add support for mvebu bubt command for flash image
> load, check and burn on boot device.
Could you please extent the patch (cmd) description here a bit?
Perhaps by adding an example on how this command can be used
to load and update image (loading via tftp, buring into SPI...).
> Change-Id: If2b971069ae44232761b601bbbcf0162567f5603
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Nadav Haklai <nadavh@marvell.com>
> Cc: Neta Zur Hershkovits <neta@marvell.com>
> Cc: Omri Itach <omrii@marvell.com>
> Cc: Igal Liberman <igall@marvell.com>
> Cc: Haim Boot <hayim@marvell.com>
> Cc: Hanna Hawa <hannah@marvell.com>
> ---
> arch/arm/mach-mvebu/Kconfig | 30 ++
> cmd/Kconfig | 3 +
> cmd/Makefile | 2 +
> cmd/mvebu/Kconfig | 10 +
> cmd/mvebu/Makefile | 19 ++
> cmd/mvebu/bubt.c | 699 ++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 763 insertions(+)
> create mode 100644 cmd/mvebu/Kconfig
> create mode 100644 cmd/mvebu/Makefile
> create mode 100644 cmd/mvebu/bubt.c
>
> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> index 7248fd7..6de85d5 100644
> --- a/arch/arm/mach-mvebu/Kconfig
> +++ b/arch/arm/mach-mvebu/Kconfig
> @@ -99,6 +99,36 @@ config TARGET_THEADORABLE
>
> endchoice
>
> +choice
> + prompt "Flash for image"
> + default MVEBU_SPI_BOOT
> + depends on (TARGET_MVEBU_DB_88F3720 || TARGET_MVEBU_ARMADA_8K)
> +
> +config MVEBU_NAND_BOOT
> + bool "NAND flash boot"
> + depends on NAND_PXA3XX
> + help
> + Enable boot from NAND
> +
> +config MVEBU_SPI_BOOT
> + bool "SPI flash boot"
> + depends on SPI_FLASH
> +
> +config MVEBU_MMC_BOOT
> + bool "eMMC flash boot"
> + depends on MVEBU_MMC
> + help
> + Enable boot from eMMC boot partition
> +
> +endchoice
> +
> +config MVEBU_UBOOT_DFLT_NAME
> + string "Default image name for bubt command"
> + default "flash-image.bin"
> + help
> + This option should contain a default file name to be used with
> + MVEBU "bubt" command if the source file name is omitted
> +
I'm not sure if these Kconfig options really below in this file - please
see below...
> config SYS_BOARD
> default "clearfog" if TARGET_CLEARFOG
> default "mvebu_db-88f3720" if TARGET_MVEBU_DB_88F3720
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index e339d86..39dd0a0 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -610,6 +610,9 @@ config CMD_QFW
> This provides access to the QEMU firmware interface. The main
> feature is to allow easy loading of files passed to qemu-system
> via -kernel / -initrd
> +
> +source "cmd/mvebu/Kconfig"
> +
> endmenu
>
> config CMD_BOOTSTAGE
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 9c9a9d1..34bc544 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -163,3 +163,5 @@ obj-$(CONFIG_CMD_BLOB) += blob.o
>
> # core command
> obj-y += nvedit.o
> +
> +obj-$(CONFIG_ARCH_MVEBU) += mvebu/
Do you plan to add move mvebu specific commands?
> diff --git a/cmd/mvebu/Kconfig b/cmd/mvebu/Kconfig
> new file mode 100644
> index 0000000..e7fbd20
> --- /dev/null
> +++ b/cmd/mvebu/Kconfig
> @@ -0,0 +1,10 @@
> +menu "MVEBU commands"
> +depends on ARCH_MVEBU
> +
> +config CMD_MVEBU_BUBT
> + bool "bubt"
> + default n
> + help
> + bubt - Burn a u-boot image to flash
> +
> +endmenu
Here you introduce a new Kconfig file. Wouldn't it be better, to
move all Kconfig option into this file instead of having most
of them in the mach-mvebu file?
> diff --git a/cmd/mvebu/Makefile b/cmd/mvebu/Makefile
> new file mode 100644
> index 0000000..b0817e3
> --- /dev/null
> +++ b/cmd/mvebu/Makefile
> @@ -0,0 +1,19 @@
> +#
> +# ***************************************************************************
> +# Copyright (C) 2015 Marvell International Ltd.
> +# ***************************************************************************
> +# This program is free software: you can redistribute it and/or modify it
> +# under the terms of the GNU General Public License as published by the Free
> +# Software Foundation, either version 2 of the License, or any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +# ***************************************************************************
> +#
As already mentioned by Simon, please move to using SPDX
license headers. You can and should of course keep your copyright
notice. Please fix this globally. For SPDX in U-Boot I suggest
to take a look at this page:
http://www.denx.de/wiki/U-Boot/Licensing
> +
> +obj-$(CONFIG_CMD_MVEBU_BUBT) += bubt.o
> diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
> new file mode 100644
> index 0000000..6539063
> --- /dev/null
> +++ b/cmd/mvebu/bubt.c
> @@ -0,0 +1,699 @@
> +/*
> + * ***************************************************************************
> + * Copyright (C) 2015 Marvell International Ltd.
> + * ***************************************************************************
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation, either version 2 of the License, or any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + * ***************************************************************************
> + */
> +
> +#include <config.h>
> +#include <common.h>
> +#include <command.h>
> +#include <vsprintf.h>
> +#include <errno.h>
> +#include <dm.h>
> +
> +#include <spi_flash.h>
> +#include <spi.h>
> +#include <nand.h>
> +#include <usb.h>
> +#include <fs.h>
> +#include <mmc.h>
> +#include <u-boot/sha1.h>
> +#include <u-boot/sha256.h>
> +
> +#if defined(CONFIG_TARGET_MVEBU_ARMADA_8K)
> +#define MAIN_HDR_MAGIC 0xB105B002
As already mentioned by Simon, please move to using lower
case characters for hex numbers.
> +struct mvebu_image_header {
> + uint32_t magic; /* 0-3 */
> + uint32_t prolog_size; /* 4-7 */
> + uint32_t prolog_checksum; /* 8-11 */
> + uint32_t boot_image_size; /* 12-15 */
> + uint32_t boot_image_checksum; /* 16-19 */
> + uint32_t rsrvd0; /* 20-23 */
> + uint32_t load_addr; /* 24-27 */
> + uint32_t exec_addr; /* 28-31 */
> + uint8_t uart_cfg; /* 32 */
> + uint8_t baudrate; /* 33 */
> + uint8_t ext_count; /* 34 */
> + uint8_t aux_flags; /* 35 */
> + uint32_t io_arg_0; /* 36-39 */
> + uint32_t io_arg_1; /* 40-43 */
> + uint32_t io_arg_2; /* 43-47 */
> + uint32_t io_arg_3; /* 48-51 */
> + uint32_t rsrvd1; /* 52-55 */
> + uint32_t rsrvd2; /* 56-59 */
> + uint32_t rsrvd3; /* 60-63 */
> +};
> +#elif defined(CONFIG_TARGET_MVEBU_DB_88F3720) /* A3700 */
> +#define HASH_SUM_LEN 16
> +#define IMAGE_VERSION_3_6_0 0x030600
> +#define IMAGE_VERSION_3_5_0 0x030500
> +
> +struct common_tim_data {
> + u32 version;
> + u32 identifier;
> + u32 trusted;
> + u32 issue_date;
> + u32 oem_unique_id;
> + u32 reserved[5]; /* Reserve 20 bytes */
> + u32 boot_flash_sign;
> + u32 num_images;
> + u32 num_keys;
> + u32 size_of_reserved;
> +};
> +
> +struct mvebu_image_info {
> + u32 image_id;
> + u32 next_image_id;
> + u32 flash_entry_addr;
> + u32 load_addr;
> + u32 image_size;
> + u32 image_size_to_hash;
> + u32 hash_algorithm_id;
> + u32 hash[HASH_SUM_LEN]; /* Reserve 512 bits for the hash */
> + u32 partition_number;
> + u32 enc_algorithm_id;
> + u32 encrypt_start_offset;
> + u32 encrypt_size;
> +};
> +#endif
> +
> +struct bubt_dev {
> + char name[8];
> + size_t (*read)(const char *file_name);
> + int (*write)(size_t image_size);
> + int (*active)(void);
> +};
> +
> +static ulong get_load_addr(void)
> +{
> + const char *addr_str;
> + unsigned long addr;
> +
> + addr_str = getenv("loadaddr");
> + if (addr_str != NULL)
> + addr = simple_strtoul(addr_str, NULL, 16);
> + else
> + addr = CONFIG_SYS_LOAD_ADDR;
> +
> + return addr;
> +}
> +
> +/********************************************************************
> + * eMMC services
> + ********************************************************************/
> +#ifdef CONFIG_DM_MMC
> +static int mmc_burn_image(size_t image_size)
> +{
> + struct mmc *mmc;
> + lbaint_t start_lba;
> + lbaint_t blk_count;
> + ulong blk_written;
> +#ifdef CONFIG_SYS_MMC_ENV_DEV
> + const u8 mmc_dev_num = CONFIG_SYS_MMC_ENV_DEV;
> +#else
> + const u8 mmc_dev_num = 0;
> +#endif
I suggest to move this one up in this file to make the code look a bit
"cleaner", like this:
#ifndef CONFIG_SYS_MMC_ENV_DEV
#define CONFIG_SYS_MMC_ENV_DEV 0
#endif
Then you can remove the #ifdef from the code here.
> +
> + mmc = find_mmc_device(mmc_dev_num);
> + if (!mmc) {
> + printf("No SD/MMC/eMMC card found\n");
> + return 1;
> + }
Does this bubt command only support eMMC or SD/MMC as well? If so,
please mention this in the Kconfig options too, as they only list
eMMC (IIRC).
> +
> + if (mmc_init(mmc)) {
> + printf("%s(%d) init failed\n", IS_SD(mmc) ? "SD" : "MMC", mmc_dev_num);
> + return 1;
> + }
> +
> +#ifdef CONFIG_SYS_MMC_ENV_PART
> + if (CONFIG_SYS_MMC_ENV_PART != mmc->part_num) {
> + if (mmc_switch_part(mmc_dev_num, CONFIG_SYS_MMC_ENV_PART)) {
> + printf("MMC partition switch failed\n");
> + return 1;
> + }
> + }
> +#endif
> +
> + /* SD reserves LBA-0 for MBR and boots from LBA-1, MMC/eMMC boots from LBA-0 */
> + start_lba = IS_SD(mmc) ? 1 : 0;
> + blk_count = image_size / mmc->block_dev.blksz;
> + if (image_size % mmc->block_dev.blksz)
> + blk_count += 1;
> +
> + blk_written = mmc->block_dev.block_write(mmc_dev_num,
> + start_lba, blk_count, (void *)get_load_addr());
> + if (blk_written != blk_count) {
> + printf("Error - written %#lx blocks\n", blk_written);
> + return 1;
> + } else {
> + printf("Done!\n");
> + }
> +
> +#ifdef CONFIG_SYS_MMC_ENV_PART
> + if (CONFIG_SYS_MMC_ENV_PART != mmc->part_num)
> + mmc_switch_part(mmc_dev_num, mmc->part_num);
> +#endif
> +
> + return 0;
> +}
> +
> +static size_t mmc_read_file(const char *file_name)
> +{
> + loff_t act_read = 0;
> + int rc;
> + struct mmc *mmc;
> +#ifdef CONFIG_SYS_MMC_ENV_DEV
> + const u8 mmc_dev_num = CONFIG_SYS_MMC_ENV_DEV;
> +#else
> + const u8 mmc_dev_num = 0;
> +#endif
Again, please use the construct suggested above.
> + mmc = find_mmc_device(mmc_dev_num);
> + if (!mmc) {
> + printf("No SD/MMC/eMMC card found\n");
> + return 0;
> + }
> +
> + if (mmc_init(mmc)) {
> + printf("%s(%d) init failed\n", IS_SD(mmc) ? "SD" : "MMC", mmc_dev_num);
> + return 0;
> + }
> +
> + /* Load from data partition (0) */
> + if (fs_set_blk_dev("mmc", "0", FS_TYPE_ANY)) {
> + printf("Error: MMC 0 not found\n");
> + return 0;
> + }
> +
> + /* Perfrom file read */
> + rc = fs_read(file_name, get_load_addr(), 0, 0, &act_read);
> + if (rc)
> + return 0;
> +
> + return act_read;
> +}
> +
> +int is_mmc_active(void)
> +{
> + return 1;
> +}
> +#else /* CONFIG_DM_MMC */
> +#define mmc_burn_image 0
> +#define mmc_read_file 0
> +#define is_mmc_active 0
Please use empty functions instead.
> +#endif /* CONFIG_DM_MMC */
> +
> +
> +/********************************************************************
> + * SPI services
> + ********************************************************************/
> +#ifdef CONFIG_SPI_FLASH
> +static int spi_burn_image(size_t image_size)
> +{
> + int ret;
> + struct spi_flash *flash;
> + uint32_t erase_bytes;
> +
> + /* Probe the SPI bus to get the flash device */
> + flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
> + CONFIG_SF_DEFAULT_SPEED, CONFIG_SF_DEFAULT_MODE);
> + if (!flash) {
> + printf("Failed to probe SPI Flash\n");
> + return 1;
> + }
> +
> +#ifdef CONFIG_SPI_FLASH_PROTECTION
> + spi_flash_protect(flash, 0);
> +#endif
> + erase_bytes = image_size + (flash->erase_size - image_size % flash->erase_size);
> + printf("Erasing %d bytes (%d blocks) at offset 0 ...", erase_bytes, erase_bytes/flash->erase_size);
> + ret = spi_flash_erase(flash, 0, erase_bytes);
> + if (ret)
> + printf("Error!\n");
> + else
> + printf("Done!\n");
> +
> + printf("Writing %d bytes from 0x%lx to offset 0 ...", (int)image_size, get_load_addr());
> + ret = spi_flash_write(flash, 0, image_size, (void *)get_load_addr());
> + if (ret)
> + printf("Error!\n");
> + else
> + printf("Done!\n");
> +
> +#ifdef CONFIG_SPI_FLASH_PROTECTION
> + spi_flash_protect(flash, 1);
> +#endif
> +
> + return ret;
> +}
> +
> +int is_spi_active(void)
> +{
> + return 1;
> +}
> +#else /* CONFIG_SPI_FLASH */
> +#define spi_burn_image 0
> +#define is_spi_active 0
Empty functions please.
> +#endif /* CONFIG_SPI_FLASH */
> +
> +/********************************************************************
> + * NAND services
> + ********************************************************************/
> +#ifdef CONFIG_CMD_NAND
> +static int nand_burn_image(size_t image_size)
> +{
> + int ret, block_size;
> + nand_info_t *nand;
> + int dev = nand_curr_device;
> +
> + if ((dev < 0) || (dev >= CONFIG_SYS_MAX_NAND_DEVICE) ||
> + (!nand_info[dev].name)) {
> + puts("\nno devices available\n");
> + return 1;
> + }
> + nand = &nand_info[dev];
> + block_size = nand->erasesize;
> +
> + /* Align U-Boot size to currently used blocksize */
> + image_size = ((image_size + (block_size - 1)) & (~(block_size-1)));
checkpatch will most likely complain about the missing space
before and after the "-" above.
> +
> + /* Erase the U-BOOT image space */
> + printf("Erasing 0x%x - 0x%x:...", 0, (int)image_size);
> + ret = nand_erase(nand, 0, image_size);
> + if (ret) {
> + printf("Error!\n");
> + goto error;
> + }
> + printf("Done!\n");
> +
> + /* Write the image to flash */
> + printf("Writing image:...");
> + printf("&image_size = 0x%p\n", (void*)&image_size);
> + ret = nand_write(nand, 0, &image_size, (void *)get_load_addr());
> + if (ret)
> + printf("Error!\n");
> + else
> + printf("Done!\n");
> +
> +error:
> + return ret;
> +}
> +
> +int is_nand_active(void)
> +{
> + return 1;
> +}
> +#else /* CONFIG_CMD_NAND */
> +#define nand_burn_image 0
> +#define is_nand_active 0
Empty functions please.
> +#endif /* CONFIG_CMD_NAND */
> +
> +/********************************************************************
> + * USB services
> + ********************************************************************/
> +#if defined(CONFIG_USB_STORAGE) && defined (CONFIG_BLK)
> +static size_t usb_read_file(const char *file_name)
> +{
> + loff_t act_read = 0;
> + struct udevice *dev;
> + int rc;
> +
> + usb_stop();
> +
> + if (usb_init() < 0) {
> + printf("Error: usb_init failed\n");
> + return 0;
> + }
> +
> + /* Try to recognize storage devices immediately */
> + blk_first_device(IF_TYPE_USB, &dev);
> + if (dev == NULL) {
> + printf("Error: USB storage device not found\n");
> + return 0;
> + }
> +
> + /* Always load from usb 0 */
> + if (fs_set_blk_dev("usb", "0", FS_TYPE_ANY)) {
> + printf("Error: USB 0 not found\n");
> + return 0;
> + }
> +
> + /* Perfrom file read */
> + rc = fs_read(file_name, get_load_addr(), 0, 0, &act_read);
> + if (rc)
> + return 0;
> +
> + return act_read;
> +}
> +
> +int is_usb_active(void)
> +{
> + return 1;
> +}
> +#else /* defined(CONFIG_USB_STORAGE) && defined (CONFIG_BLK) */
> +#define usb_read_file 0
> +#define is_usb_active 0
> +#endif /* defined(CONFIG_USB_STORAGE) && defined (CONFIG_BLK) */
> +
> +/********************************************************************
> + * Network services
> + ********************************************************************/
> +#ifdef CONFIG_CMD_NET
> +static size_t tftp_read_file(const char *file_name)
> +{
> + /* update global variable load_addr before tftp file from network */
> + load_addr = get_load_addr();
> + return net_loop(TFTPGET);
> +}
> +
> +int is_tftp_active(void)
> +{
> + return 1;
> +}
> +#else
> +#define tftp_read_file 0
> +#define is_tftp_active 0
> +#endif /* CONFIG_CMD_NET */
> +
> +
> +enum bubt_devices {
> + BUBT_DEV_NET = 0,
> + BUBT_DEV_USB,
> + BUBT_DEV_MMC,
> + BUBT_DEV_SPI,
> + BUBT_DEV_NAND,
> +
> + BUBT_MAX_DEV
> +};
> +
> +struct bubt_dev bubt_devs[BUBT_MAX_DEV] = {
> + {"tftp", tftp_read_file, NULL, is_tftp_active},
> + {"usb", usb_read_file, NULL, is_usb_active},
> + {"mmc", mmc_read_file, mmc_burn_image, is_mmc_active},
> + {"spi", NULL, spi_burn_image, is_spi_active},
> + {"nand", NULL, nand_burn_image, is_nand_active},
> +};
> +
> +static int bubt_write_file(struct bubt_dev *dst, size_t image_size)
> +{
> + if (!dst->write) {
> + printf("Error: Write not supported on device %s\n", dst->name);
> + return 1;
> + }
> +
> + return dst->write(image_size);
> +}
> +
> +#if defined(CONFIG_TARGET_MVEBU_ARMADA_8K)
> +uint32_t do_checksum32(uint32_t *start, int32_t len)
> +{
> + uint32_t sum = 0;
> + uint32_t *startp = start;
> +
> + do {
> + sum += *startp;
> + startp++;
> + len -= 4;
> + } while (len > 0);
> +
> + return sum;
> +}
> +
> +static int check_image_header(void)
> +{
> + struct mvebu_image_header *hdr = (struct mvebu_image_header *)get_load_addr();
> + uint32_t header_len = hdr->prolog_size;
> + uint32_t checksum;
> + uint32_t checksum_ref = hdr->prolog_checksum;
> +
> + /*
> + * For now compare checksum, and magic. Later we can
> + * verify more stuff on the header like interface type, etc
> + */
> + if (hdr->magic != MAIN_HDR_MAGIC) {
> + printf("ERROR: Bad MAGIC 0x%08x != 0x%08x\n", hdr->magic, MAIN_HDR_MAGIC);
> + return -ENOEXEC;
> + }
> +
> + /* The checksum value is discarded from checksum calculation */
> + hdr->prolog_checksum = 0;
> +
> + checksum = do_checksum32((uint32_t *)hdr, header_len);
> + if (checksum != checksum_ref) {
> + printf("Error: Bad Image checksum. 0x%x != 0x%x\n", checksum, checksum_ref);
> + return -ENOEXEC;
> + }
> +
> + /* Restore the checksum before writing */
> + hdr->prolog_checksum = checksum_ref;
> + printf("Image checksum...OK!\n");
> +
> + return 0;
> +}
> +#elif defined(CONFIG_TARGET_MVEBU_DB_88F3720) /* Armada 3700 */
> +static int check_image_header(void)
> +{
> + struct common_tim_data *hdr = (struct common_tim_data *)get_load_addr();
> + int image_num;
> + u8 hash_160_output[SHA1_SUM_LEN];
> + u8 hash_256_output[SHA256_SUM_LEN];
> + sha1_context hash1_text;
> + sha256_context hash256_text;
> + u8 *hash_output;
> + u32 hash_algorithm_id;
> + u32 image_size_to_hash;
> + u32 flash_entry_addr;
> + u32 *hash_value;
> + u32 internal_hash[HASH_SUM_LEN];
> + const uint8_t *buff;
> + u32 num_of_image = hdr->num_images;
> + u32 version = hdr->version;
> + u32 trusted = hdr->trusted;
> +
> + /* bubt checksum validation only supports nontrusted images */
> + if (trusted == 1) {
> + printf("bypass image validation, only untrusted image is supported now\n");
> + return 0;
> + }
> + /* only supports image version 3.5 and 3.6 */
> + if (version != IMAGE_VERSION_3_5_0 && version != IMAGE_VERSION_3_6_0) {
> + printf("Error: Unsupported Image version = 0x%08x\n", version);
> + return -ENOEXEC;
> + }
> + /* validate images hash value */
> + for (image_num = 0; image_num < num_of_image; image_num++) {
> + struct mvebu_image_info *info = (struct mvebu_image_info *)(get_load_addr()
> + + sizeof(struct common_tim_data) + image_num * sizeof(struct mvebu_image_info));
> + hash_algorithm_id = info->hash_algorithm_id;
> + image_size_to_hash = info->image_size_to_hash;
> + flash_entry_addr = info->flash_entry_addr;
> + hash_value = info->hash;
> + buff = (const uint8_t *)(get_load_addr() + flash_entry_addr);
> +
> + if (image_num == 0) {
> + /* The first image includes hash values in itself content. For hash calculation, we need
> + * to save original hash values to local variable that will be copied back for comparsion
> + * and set all zeros to replace orignal hash values to calculate its new hash value.
> + * First image original format : x...x (datum1) x...x(original hash values) x...x(datum2)
> + * Replaced first image format : x...x (datum1) 0...0(hash values) x...x(datum2)
> + */
Multicomment style is:
/*
*
*/
And please note the 80 colums limit.
> + memcpy(internal_hash, hash_value, sizeof(internal_hash));
> + memset(hash_value, 0, sizeof(internal_hash));
> + }
> + if (image_size_to_hash == 0) {
> + printf("Warning: Image_%d hash checksum is disable, skip the image validation.\n", image_num);
> + continue;
> + }
> + switch (hash_algorithm_id) {
> + case SHA1_SUM_LEN:
> + sha1_starts(&hash1_text);
> + sha1_update(&hash1_text, buff, image_size_to_hash);
> + sha1_finish(&hash1_text, hash_160_output);
> + hash_output = hash_160_output;
> + break;
> + case SHA256_SUM_LEN:
> + sha256_starts(&hash256_text);
> + sha256_update(&hash256_text, buff, image_size_to_hash);
> + sha256_finish(&hash256_text, hash_256_output);
> + hash_output = hash_256_output;
> + break;
> + default:
> + printf("Error: Unsupported hash_algorithm_id = %d\n", hash_algorithm_id);
> + return -ENOEXEC;
> + }
> + if (image_num == 0)
> + memcpy(hash_value, internal_hash, sizeof(internal_hash));
> + if (memcmp(hash_value, hash_output, hash_algorithm_id) != 0) {
> + printf("Error: Image_%d checksum is not correct\n", image_num);
> + return -ENOEXEC;
> + }
> + }
> + printf("Image checksum...OK!\n");
> + return 0;
> +}
> +#else
> +static int check_image_header(void)
> +{
> + printf("bubt cmd does not support this SoC device or family!\n");
> + return -ENOEXEC;
> +}
> +#endif
> +
> +static int bubt_verify(size_t image_size)
> +{
> +
> + /* Check a correct image header exists */
> + if (check_image_header()) {
> + printf("Error: Image header verification failed\n");
> + return 1;
> + }
> + return 0;
> +}
> +
> +
> +static int bubt_read_file(struct bubt_dev *src)
> +{
> + size_t image_size;
> +
> + if (!src->read) {
> + printf("Error: Read not supported on device \"%s\"\n", src->name);
> + return 0;
> + }
> +
> + image_size = src->read(net_boot_file_name);
> + if (image_size <= 0) {
> + printf("Error: Failed to read file %s from %s\n", net_boot_file_name, src->name);
> + return 0;
> + }
> +
> + return image_size;
> +}
> +
> +static int bubt_is_dev_active(struct bubt_dev *dev)
> +{
> + if (!dev->active) {
> + printf("Device \"%s\" not supported by U-BOOT image\n", dev->name);
> + return 0;
> + }
> +
> + if (!dev->active()) {
> + printf("Device \"%s\" is inactive\n", dev->name);
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> +struct bubt_dev *find_bubt_dev(char *dev_name)
> +{
> + int dev;
> +
> + for (dev = 0; dev < BUBT_MAX_DEV; dev++) {
> + if (strcmp(bubt_devs[dev].name, dev_name) == 0)
> + return &bubt_devs[dev];
> + }
> +
> + return 0;
> +}
> +
> +#define DEFAULT_BUBT_SRC "tftp"
> +
> +#ifndef DEFAULT_BUBT_DST
> +#ifdef CONFIG_MVEBU_SPI_BOOT
> +#define DEFAULT_BUBT_DST "spi"
> +#elif defined(CONFIG_MVEBU_NAND_BOOT)
> +#define DEFAULT_BUBT_DST "nand"
> +#elif defined(CONFIG_MVEBU_MMC_BOOT)
> +#define DEFAULT_BUBT_DST "mmc"
> +else
> +#define DEFAULT_BUBT_DST "error"
> +#endif
> +#endif /* DEFAULT_BUBT_DST */
> +
> +int do_bubt_cmd(cmd_tbl_t *cmdtp, int flag, int argc,
> + char * const argv[])
> +{
> + struct bubt_dev *src, *dst;
> + size_t image_size;
> + char src_dev_name[8];
> + char dst_dev_name[8];
> + char *name;
> +
> + if (argc < 2)
> + copy_filename(net_boot_file_name, CONFIG_MVEBU_UBOOT_DFLT_NAME, sizeof(net_boot_file_name));
> + else
> + copy_filename(net_boot_file_name, argv[1], sizeof(net_boot_file_name));
> +
> + if (argc >= 3) {
> + strncpy(dst_dev_name, argv[2], 8);
> + } else {
> + name = DEFAULT_BUBT_DST;
> + strncpy(dst_dev_name, name, 8);
> + }
> +
> + if (argc >= 4)
> + strncpy(src_dev_name, argv[3], 8);
> + else
> + strncpy(src_dev_name, DEFAULT_BUBT_SRC, 8);
> +
> + /* Figure out the destination device */
> + dst = find_bubt_dev(dst_dev_name);
> + if (!dst) {
> + printf("Error: Unknown destination \"%s\"\n", dst_dev_name);
> + return 1;
> + }
> +
> + if (!bubt_is_dev_active(dst))
> + return 1;
> +
> + /* Figure out the source device */
> + src = find_bubt_dev(src_dev_name);
> + if (!src) {
> + printf("Error: Unknown source \"%s\"\n", src_dev_name);
> + return 1;
> + }
> +
> + if (!bubt_is_dev_active(src))
> + return 1;
> +
> + printf("Burning U-BOOT image \"%s\" from \"%s\" to \"%s\"\n", net_boot_file_name, src->name, dst->name);
> +
> + image_size = bubt_read_file(src);
> + if (!image_size)
> + return 1;
> +
> + if (bubt_verify(image_size))
> + return 1;
> +
> + if (bubt_write_file(dst, image_size))
> + return 1;
> +
> + return 0;
> +}
> +
> +U_BOOT_CMD(
> + bubt, 4, 0, do_bubt_cmd,
> + "Burn a u-boot image to flash",
> + "[file-name] [destination [source]]\n"
> + "\t-file-name The image file name to burn. Default = u-boot.bin\n"
> + "\t-destination Flash to burn to [spi, nand, mmc]. Default = active boot device\n"
> + "\t-source The source to load image from [tftp, usb, mmc]. Default = tftp\n"
> + "Examples:\n"
> + "\tbubt - Burn flash-image.bin from tftp to active boot device\n"
> + "\tbubt flash-image-new.bin nand - Burn flash-image-new.bin from tftp to NAND flash\n"
> + "\tbubt backup-flash-image.bin mmc usb - Burn backup-flash-image.bin from usb to MMC\n"
Ah, here you have some nice examples. Please add at least one best
with a log from running on a board to the commit text as
mentioned above.
Thanks,
Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH 3/6] arm64: mvebu: pinctrl: Add pin control driver for A8K family
2016-11-20 15:38 [U-Boot] [PATCH 0/6] arm64: mvebu: Armada-8K family patches kostap at marvell.com
2016-11-20 15:38 ` [U-Boot] [PATCH 1/6] arm64: mvebu: Modify the A8K SPI and I2C config in DTS kostap at marvell.com
2016-11-20 15:38 ` [U-Boot] [PATCH 2/6] arm64: mvebu: Add bubt command for flash image burn kostap at marvell.com
@ 2016-11-20 15:38 ` kostap at marvell.com
2016-11-24 2:20 ` Simon Glass
2016-11-20 15:38 ` [U-Boot] [PATCH 4/6] arm64: mvebu: Add pin control nodes to A8K family DTS files kostap at marvell.com
` (2 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: kostap at marvell.com @ 2016-11-20 15:38 UTC (permalink / raw)
To: u-boot
From: Konstantin Porotchkin <kostap@marvell.com>
Add a port of Marvell pin control driver.
The A8K SoC family contains several silicone dies interconnected
in a single package. Every die is normally equuipped with its own
pin controller unit.
Since the UCLASS_PINCTRL device only calls the probe method for
the first detected pin controller, a trick similar to used with
comphy driver is required.
In order to bring up all pin controllers available in A8K SoC,
the arch_early_init_r() function sequentially calls the
uclass_get_device() function for each UCLASS_PINCTRL device.
Change-Id: Iff143827e8f1558a554d77173562c4b52ce179d7
Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Stefan Roese <sr@denx.de>
Cc: Nadav Haklai <nadavh@marvell.com>
Cc: Neta Zur Hershkovits <neta@marvell.com>
Cc: Omri Itach <omrii@marvell.com>
Cc: Igal Liberman <igall@marvell.com>
Cc: Haim Boot <hayim@marvell.com>
Cc: Hanna Hawa <hannah@marvell.com>
---
arch/arm/include/asm/arch-armada8k/soc-info.h | 45 +++++
arch/arm/mach-mvebu/arm64-common.c | 1 -
.../pinctrl/marvell,mvebu-pinctrl.txt | 113 ++++++++++++
drivers/pinctrl/Kconfig | 1 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/mvebu/Kconfig | 7 +
drivers/pinctrl/mvebu/Makefile | 17 ++
drivers/pinctrl/mvebu/pinctrl-mvebu.c | 195 +++++++++++++++++++++
drivers/pinctrl/mvebu/pinctrl-mvebu.h | 44 +++++
9 files changed, 423 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/include/asm/arch-armada8k/soc-info.h
create mode 100644 doc/device-tree-bindings/pinctrl/marvell,mvebu-pinctrl.txt
create mode 100644 drivers/pinctrl/mvebu/Kconfig
create mode 100644 drivers/pinctrl/mvebu/Makefile
create mode 100644 drivers/pinctrl/mvebu/pinctrl-mvebu.c
create mode 100644 drivers/pinctrl/mvebu/pinctrl-mvebu.h
diff --git a/arch/arm/include/asm/arch-armada8k/soc-info.h b/arch/arm/include/asm/arch-armada8k/soc-info.h
new file mode 100644
index 0000000..4640deb
--- /dev/null
+++ b/arch/arm/include/asm/arch-armada8k/soc-info.h
@@ -0,0 +1,45 @@
+/*
+ * ***************************************************************************
+ * Copyright (C) 2015 Marvell International Ltd.
+ * ***************************************************************************
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation, either version 2 of the License, or any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ * ***************************************************************************
+ */
+
+#ifndef _SOC_INFO_H_
+#define _SOC_INFO_H_
+
+/* General MPP definitions */
+#define MAX_MPP_OPTS 7
+#define MAX_MPP_ID 15
+
+#define MPP_BIT_CNT 4
+#define MPP_FIELD_MASK 0x7
+#define MPP_FIELD_BITS 3
+#define MPP_VAL_MASK 0xF
+
+#define MPPS_PER_REG (32 / MPP_BIT_CNT)
+#define MAX_MPP_REGS ((MAX_MPP_ID + MPPS_PER_REG) / MPPS_PER_REG)
+
+/* MPP pins and functions correcsponding to UART RX connections
+ This information is used for detection of recovery boot mode (boot from UART) */
+#define MPP_UART_RX_PINS { 3, 5 }
+#define MPP_UART_RX_FUNCTIONS { 1, 2 }
+
+/* Pin Ctrl driver definitions */
+#define BITS_PER_PIN 4
+#define PIN_FUNC_MASK ((1 << BITS_PER_PIN) - 1)
+#define PIN_REG_SHIFT 3
+#define PIN_FIELD_MASK ((1 << PIN_REG_SHIFT) - 1)
+
+#endif /* _SOC_INFO_H_ */
diff --git a/arch/arm/mach-mvebu/arm64-common.c b/arch/arm/mach-mvebu/arm64-common.c
index 1fc2ff2..78fe7a7 100644
--- a/arch/arm/mach-mvebu/arm64-common.c
+++ b/arch/arm/mach-mvebu/arm64-common.c
@@ -124,7 +124,6 @@ int arch_early_init_r(void)
if (ret)
break;
}
-
/* Cause the SATA device to do its early init */
uclass_first_device(UCLASS_AHCI, &dev);
diff --git a/doc/device-tree-bindings/pinctrl/marvell,mvebu-pinctrl.txt b/doc/device-tree-bindings/pinctrl/marvell,mvebu-pinctrl.txt
new file mode 100644
index 0000000..0973db8
--- /dev/null
+++ b/doc/device-tree-bindings/pinctrl/marvell,mvebu-pinctrl.txt
@@ -0,0 +1,113 @@
+The pinctrl driver enables Marvell Armada 8K SoCs to configure the multi-purpose
+pins (mpp) to a specific function.
+A Marvell SoC pin configuration node is a node of a group of pins which can
+be used for a specific device or function. Each node requires one or more
+mpp pins or group of pins and a mpp function common to all pins.
+
+Required properties for the pinctrl driver:
+- compatible: "marvell,mvebu-pinctrl",
+ "marvell,armada-ap806-pinctrl",
+ "marvell,a70x0-pinctrl",
+ "marvell,a80x0-cp0-pinctrl",
+ "marvell,a80x0-cp1-pinctrl"
+- bank-name: A string defining the pinc controller bank name
+- reg: A pair of values defining the pin controller base address
+ and the address space
+- pin-count: Numeric value defining the amount of multi purpose pins
+ included in this bank
+- max-func: Numeric value defining the maximum function value for
+ pins in this bank
+- pin-func: Array of pin function values for every pin in the bank.
+ When the function value for a specific pin equal 0xFF,
+ the pin configuration is skipped and a default function
+ value is used for this pin.
+
+The A8K is a hybrid SoC that contains several silicon dies interconnected in
+a single package. Each such die may have a separate pin controller.
+
+Example:
+/ {
+ ap806 {
+ config-space {
+ pinctl: pinctl@6F4000 {
+ compatible = "marvell,mvebu-pinctrl",
+ "marvell,armada-ap806-pinctrl";
+ bank-name ="apn-806";
+ reg = <0x6F4000 0x10>;
+ pin-count = <20>;
+ max-func = <3>;
+ /* MPP Bus:
+ SPI0 [0-3]
+ I2C0 [4-5]
+ UART0 [11,19]
+ */
+ /* 0 1 2 3 4 5 6 7 8 9 */
+ pin-func = < 3 3 3 3 3 3 0 0 0 0
+ 0 3 0 0 0 0 0 0 0 3>;
+ };
+ };
+ };
+
+ cp110-master {
+ config-space {
+ cpm_pinctl: pinctl at 44000 {
+ compatible = "marvell,mvebu-pinctrl",
+ "marvell,a70x0-pinctrl",
+ "marvell,a80x0-cp0-pinctrl";
+ bank-name ="cp0-110";
+ reg = <0x440000 0x20>;
+ pin-count = <63>;
+ max-func = <0xf>;
+ /* MPP Bus:
+ [0-31] = 0xff: Keep default CP0_shared_pins:
+ [11] CLKOUT_MPP_11 (out)
+ [23] LINK_RD_IN_CP2CP (in)
+ [25] CLKOUT_MPP_25 (out)
+ [29] AVS_FB_IN_CP2CP (in)
+ [32,34] SMI
+ [31] GPIO: push button/Wake
+ [35-36] GPIO
+ [37-38] I2C
+ [40-41] SATA[0/1]_PRESENT_ACTIVEn
+ [42-43] XSMI
+ [44-55] RGMII1
+ [56-62] SD
+ */
+ /* 0 1 2 3 4 5 6 7 8 9 */
+ pin-func = < 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
+ 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
+ 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
+ 0xff 0 7 0 7 0 0 2 2 0
+ 0 0 8 8 1 1 1 1 1 1
+ 1 1 1 1 1 1 0xE 0xE 0xE 0xE
+ 0xE 0xE 0xE>;
+ };
+ };
+ };
+
+ cp110-slave {
+ config-space {
+ cps_pinctl: pinctl at 44000 {
+ compatible = "marvell,mvebu-pinctrl",
+ "marvell,a80x0-cp1-pinctrl";
+ bank-name ="cp1-110";
+ reg = <0x440000 0x20>;
+ pin-count = <63>;
+ max-func = <0xf>;
+ /* MPP Bus:
+ [0-11] RGMII0
+ [27,31] GE_MDIO/MDC
+ [32-62] = 0xff: Keep default CP1_shared_pins:
+ */
+ /* 0 1 2 3 4 5 6 7 8 9 */
+ pin-func = < 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3
+ 0x3 0x3 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
+ 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0x8 0xff 0xff
+ 0xff 0x8 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
+ 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
+ 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
+ 0xff 0xff 0xff>;
+ };
+ };
+ };
+}
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 12be3cf..efcb4c0 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -181,5 +181,6 @@ source "drivers/pinctrl/meson/Kconfig"
source "drivers/pinctrl/nxp/Kconfig"
source "drivers/pinctrl/uniphier/Kconfig"
source "drivers/pinctrl/exynos/Kconfig"
+source "drivers/pinctrl/mvebu/Kconfig"
endmenu
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index f28b5c1..512112a 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -15,3 +15,4 @@ obj-$(CONFIG_PINCTRL_UNIPHIER) += uniphier/
obj-$(CONFIG_PIC32_PINCTRL) += pinctrl_pic32.o
obj-$(CONFIG_PINCTRL_EXYNOS) += exynos/
obj-$(CONFIG_PINCTRL_MESON) += meson/
+obj-$(CONFIG_PINCTRL_MVEBU) += mvebu/
diff --git a/drivers/pinctrl/mvebu/Kconfig b/drivers/pinctrl/mvebu/Kconfig
new file mode 100644
index 0000000..cf9c299
--- /dev/null
+++ b/drivers/pinctrl/mvebu/Kconfig
@@ -0,0 +1,7 @@
+config PINCTRL_MVEBU
+ depends on ARCH_MVEBU
+ bool
+ default y
+ help
+ Support pin multiplexing and pin configuration control on
+ Marvell's Armada-8K SoC.
diff --git a/drivers/pinctrl/mvebu/Makefile b/drivers/pinctrl/mvebu/Makefile
new file mode 100644
index 0000000..7db2b97
--- /dev/null
+++ b/drivers/pinctrl/mvebu/Makefile
@@ -0,0 +1,17 @@
+#* ***************************************************************************
+#* Copyright (C) 2016 Marvell International Ltd.
+#* ***************************************************************************
+#* This program is free software: you can redistribute it and/or modify it
+#* under the terms of the GNU General Public License as published by the Free
+#* Software Foundation, either version 2 of the License, or any later version.
+#*
+#* This program is distributed in the hope that it will be useful,
+#* but WITHOUT ANY WARRANTY; without even the implied warranty of
+#* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+#* GNU General Public License for more details.
+#*
+#* You should have received a copy of the GNU General Public License
+#* along with this program. If not, see <http://www.gnu.org/licenses/>.
+#* ***************************************************************************
+
+obj-$(CONFIG_PINCTRL_MVEBU) += pinctrl-mvebu.o
diff --git a/drivers/pinctrl/mvebu/pinctrl-mvebu.c b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
new file mode 100644
index 0000000..02fcd2f
--- /dev/null
+++ b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
@@ -0,0 +1,195 @@
+/*
+ * ***************************************************************************
+ * Copyright (C) 2016 Marvell International Ltd.
+ * ***************************************************************************
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation, either version 2 of the License, or any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ * ***************************************************************************
+ */
+
+#include <config.h>
+#include <common.h>
+#include <dm.h>
+#include <asm/system.h>
+#include <asm/io.h>
+#include <dm/pinctrl.h>
+#include <dm/root.h>
+#include <fdtdec.h>
+#include <errno.h>
+#include <asm/arch/fdt.h>
+#include <asm/arch-armada8k/soc-info.h>
+#include "pinctrl-mvebu.h"
+
+/*
+ * mvebu_pinctrl_set_state: configure pin functions.
+ * dev: the pinctrl device to be configured.
+ * config: the state to be configured.
+ */
+int mvebu_pinctrl_set_state(struct udevice *dev, struct udevice *config)
+{
+ const void *blob = gd->fdt_blob;
+ int node = config->of_offset;
+ struct mvebu_pinctrl_priv *priv;
+ u32 pin_arr[CONFIG_MAX_PINS_PER_BANK];
+ u32 function;
+ int i, pin_count;
+
+ priv = dev_get_priv(dev);
+ if (!priv) {
+ printf("%s: Failed to get private\n", __func__);
+ return -EINVAL;
+ }
+
+ pin_count = fdtdec_get_int_array_count(blob, node, "marvell,pins", pin_arr, CONFIG_MAX_PINS_PER_BANK);
+ if (pin_count <= 0) {
+ error("Failed reading pins array for pinconfig %s (%d)\n", config->name, pin_count);
+ return -EINVAL;
+ }
+
+ function = fdtdec_get_int(blob, node, "marvell,function", 0xFF);
+
+ for (i = 0; i < pin_count; i++) {
+ int reg_offset;
+ int field_offset;
+ u32 reg, mask;
+ int pin = pin_arr[i];
+
+ if (function > priv->max_func) {
+ error("Illegal function %d for pinconfig %s\n", function, config->name);
+ return -EINVAL;
+ }
+
+ /* Calculate register address and bit in register */
+ reg_offset = priv->reg_direction * 4 * (pin >> (PIN_REG_SHIFT));
+ field_offset = (BITS_PER_PIN) * (pin & PIN_FIELD_MASK);
+ mask = ~(PIN_FUNC_MASK << field_offset);
+
+ /* Clip value to field resolution */
+ function &= PIN_FUNC_MASK;
+
+ reg = readl(priv->base_reg + reg_offset);
+ reg = (reg & mask) | (function << field_offset);
+ writel(reg, priv->base_reg + reg_offset);
+ }
+
+ return 0;
+}
+
+/*
+ * mvebu_pinctrl_set_state_all: configure the entire bank pin functions.
+ * dev: the pinctrl device to be configured.
+ * config: the state to be configured.
+ */
+static int mvebu_pinctrl_set_state_all(struct udevice *dev, struct udevice *config)
+{
+ const void *blob = gd->fdt_blob;
+ int node = config->of_offset;
+ struct mvebu_pinctrl_priv *priv;
+ u32 func_arr[CONFIG_MAX_PINS_PER_BANK];
+ int pin, err;
+
+ priv = dev_get_priv(dev);
+ if (!priv) {
+ printf("%s: Failed to get private\n", __func__);
+ return -EINVAL;
+ }
+
+ err = fdtdec_get_int_array(blob, node, "pin-func", func_arr, priv->pin_cnt);
+ if (err) {
+ error("Failed reading pin functions for bank %s\n", priv->bank_name);
+ return -EINVAL;
+ }
+
+ for (pin = 0; pin < priv->pin_cnt; pin++) {
+ int reg_offset;
+ int field_offset;
+ u32 reg, mask;
+ u32 func = func_arr[pin];
+
+ /* Bypass pins with function 0xFF */
+ if (func == 0xFF) {
+ debug("Warning: pin %d value is not modified (kept as default\n", pin);
+ continue;
+ } else if (func > priv->max_func) {
+ error("Illegal function %d for pin %d\n", func, pin);
+ return -EINVAL;
+ }
+
+ /* Calculate register address and bit in register */
+ reg_offset = priv->reg_direction * 4 * (pin >> (PIN_REG_SHIFT));
+ field_offset = (BITS_PER_PIN) * (pin & PIN_FIELD_MASK);
+ mask = ~(PIN_FUNC_MASK << field_offset);
+
+ /* Clip value to field resolution */
+ func &= PIN_FUNC_MASK;
+
+ reg = readl(priv->base_reg + reg_offset);
+ reg = (reg & mask) | (func << field_offset);
+ writel(reg, priv->base_reg + reg_offset);
+ }
+
+ return 0;
+}
+
+int mvebu_pinctl_probe(struct udevice *dev)
+{
+ const void *blob = gd->fdt_blob;
+ int node = dev->of_offset;
+ struct mvebu_pinctrl_priv *priv;
+ fdt_addr_t base;
+
+ priv = dev_get_priv(dev);
+ if (!priv) {
+ printf("%s: Failed to get private\n", __func__);
+ return -EINVAL;
+ }
+
+ base = dev_get_addr(dev);
+ if (base == FDT_ADDR_T_NONE) {
+ printf("%s: Failed to get base address\n", __func__);
+ return -EINVAL;
+ }
+
+ priv->base_reg = (u8 *)base;
+ priv->pin_cnt = fdtdec_get_int(blob, node, "pin-count", CONFIG_MAX_PINS_PER_BANK);
+ priv->max_func = fdtdec_get_int(blob, node, "max-func", CONFIG_MAX_FUNC);
+ priv->bank_name = fdt_getprop(blob, node, "bank-name", NULL);
+
+ priv->reg_direction = 1;
+ if (fdtdec_get_bool(blob, node, "reverse-reg"))
+ priv->reg_direction = -1;
+
+ return mvebu_pinctrl_set_state_all(dev, dev);
+}
+
+
+static struct pinctrl_ops mvebu_pinctrl_ops = {
+ .set_state = mvebu_pinctrl_set_state
+};
+
+static const struct udevice_id mvebu_pinctrl_ids[] = {
+ { .compatible = "marvell,mvebu-pinctrl" },
+ { .compatible = "marvell,armada-ap806-pinctrl" },
+ { .compatible = "marvell,a70x0-pinctrl" },
+ { .compatible = "marvell,a80x0-cp0-pinctrl" },
+ { .compatible = "marvell,a80x0-cp1-pinctrl" },
+ { }
+};
+
+U_BOOT_DRIVER(pinctrl_mvebu) = {
+ .name = "mvebu_pinctrl",
+ .id = UCLASS_PINCTRL,
+ .of_match = mvebu_pinctrl_ids,
+ .priv_auto_alloc_size = sizeof(struct mvebu_pinctrl_priv),
+ .ops = &mvebu_pinctrl_ops,
+ .probe = mvebu_pinctl_probe
+};
diff --git a/drivers/pinctrl/mvebu/pinctrl-mvebu.h b/drivers/pinctrl/mvebu/pinctrl-mvebu.h
new file mode 100644
index 0000000..61c84ce
--- /dev/null
+++ b/drivers/pinctrl/mvebu/pinctrl-mvebu.h
@@ -0,0 +1,44 @@
+/*
+ * ***************************************************************************
+ * Copyright (C) 2016 Marvell International Ltd.
+ * ***************************************************************************
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation, either version 2 of the License, or any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ * ***************************************************************************
+ */
+
+ #ifndef __PINCTRL_MVEBU_H_
+ #define __PINCTRL_MVEBU_H_
+
+ #define CONFIG_MAX_PINCTL_BANKS 4
+ #define CONFIG_MAX_PINS_PER_BANK 100
+ #define CONFIG_MAX_FUNC 0xF
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * struct mvebu_pin_bank_data: mvebu-pinctrl bank data
+ * @base_reg: controller base address for this bank
+ * @pin_cnt: number of ping included in this bank
+ * @max_func: maximum configurable function value for pins in this bank
+ * @reg_direction:
+ * @bank_name: the pins bank name
+ */
+struct mvebu_pinctrl_priv {
+ u8 *base_reg;
+ u32 pin_cnt;
+ u32 max_func;
+ int reg_direction;
+ const char *bank_name;
+};
+
+#endif /* __PINCTRL_MVEBU_H_ */
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [U-Boot] [PATCH 3/6] arm64: mvebu: pinctrl: Add pin control driver for A8K family
2016-11-20 15:38 ` [U-Boot] [PATCH 3/6] arm64: mvebu: pinctrl: Add pin control driver for A8K family kostap at marvell.com
@ 2016-11-24 2:20 ` Simon Glass
2016-11-24 8:28 ` Kostya Porotchkin
0 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2016-11-24 2:20 UTC (permalink / raw)
To: u-boot
Hi,
On 20 November 2016 at 08:38, <kostap@marvell.com> wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
>
> Add a port of Marvell pin control driver.
> The A8K SoC family contains several silicone dies interconnected
> in a single package. Every die is normally equuipped with its own
> pin controller unit.
> Since the UCLASS_PINCTRL device only calls the probe method for
> the first detected pin controller, a trick similar to used with
> comphy driver is required.
> In order to bring up all pin controllers available in A8K SoC,
> the arch_early_init_r() function sequentially calls the
> uclass_get_device() function for each UCLASS_PINCTRL device.
>
> Change-Id: Iff143827e8f1558a554d77173562c4b52ce179d7
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Nadav Haklai <nadavh@marvell.com>
> Cc: Neta Zur Hershkovits <neta@marvell.com>
> Cc: Omri Itach <omrii@marvell.com>
> Cc: Igal Liberman <igall@marvell.com>
> Cc: Haim Boot <hayim@marvell.com>
> Cc: Hanna Hawa <hannah@marvell.com>
> ---
> arch/arm/include/asm/arch-armada8k/soc-info.h | 45 +++++
> arch/arm/mach-mvebu/arm64-common.c | 1 -
> .../pinctrl/marvell,mvebu-pinctrl.txt | 113 ++++++++++++
> drivers/pinctrl/Kconfig | 1 +
> drivers/pinctrl/Makefile | 1 +
> drivers/pinctrl/mvebu/Kconfig | 7 +
> drivers/pinctrl/mvebu/Makefile | 17 ++
> drivers/pinctrl/mvebu/pinctrl-mvebu.c | 195 +++++++++++++++++++++
> drivers/pinctrl/mvebu/pinctrl-mvebu.h | 44 +++++
> 9 files changed, 423 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm/include/asm/arch-armada8k/soc-info.h
> create mode 100644 doc/device-tree-bindings/pinctrl/marvell,mvebu-pinctrl.txt
> create mode 100644 drivers/pinctrl/mvebu/Kconfig
> create mode 100644 drivers/pinctrl/mvebu/Makefile
> create mode 100644 drivers/pinctrl/mvebu/pinctrl-mvebu.c
> create mode 100644 drivers/pinctrl/mvebu/pinctrl-mvebu.h
>
Generally looks good but I have a load of nits sorry.
> diff --git a/arch/arm/include/asm/arch-armada8k/soc-info.h b/arch/arm/include/asm/arch-armada8k/soc-info.h
> new file mode 100644
> index 0000000..4640deb
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-armada8k/soc-info.h
> @@ -0,0 +1,45 @@
> +/*
> + * ***************************************************************************
> + * Copyright (C) 2015 Marvell International Ltd.
> + * ***************************************************************************
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation, either version 2 of the License, or any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + * ***************************************************************************
Can you please use SPDX?
> + */
> +
> +#ifndef _SOC_INFO_H_
> +#define _SOC_INFO_H_
> +
> +/* General MPP definitions */
> +#define MAX_MPP_OPTS 7
> +#define MAX_MPP_ID 15
> +
> +#define MPP_BIT_CNT 4
> +#define MPP_FIELD_MASK 0x7
> +#define MPP_FIELD_BITS 3
> +#define MPP_VAL_MASK 0xF
> +
> +#define MPPS_PER_REG (32 / MPP_BIT_CNT)
> +#define MAX_MPP_REGS ((MAX_MPP_ID + MPPS_PER_REG) / MPPS_PER_REG)
> +
> +/* MPP pins and functions correcsponding to UART RX connections
> + This information is used for detection of recovery boot mode (boot from UART) */
/*
* MPP pins
* ...
* /
> +#define MPP_UART_RX_PINS { 3, 5 }
> +#define MPP_UART_RX_FUNCTIONS { 1, 2 }
> +
> +/* Pin Ctrl driver definitions */
> +#define BITS_PER_PIN 4
> +#define PIN_FUNC_MASK ((1 << BITS_PER_PIN) - 1)
> +#define PIN_REG_SHIFT 3
> +#define PIN_FIELD_MASK ((1 << PIN_REG_SHIFT) - 1)
> +
> +#endif /* _SOC_INFO_H_ */
> diff --git a/arch/arm/mach-mvebu/arm64-common.c b/arch/arm/mach-mvebu/arm64-common.c
> index 1fc2ff2..78fe7a7 100644
> --- a/arch/arm/mach-mvebu/arm64-common.c
> +++ b/arch/arm/mach-mvebu/arm64-common.c
> @@ -124,7 +124,6 @@ int arch_early_init_r(void)
> if (ret)
> break;
> }
> -
Unrelated change?
> /* Cause the SATA device to do its early init */
> uclass_first_device(UCLASS_AHCI, &dev);
>
> diff --git a/doc/device-tree-bindings/pinctrl/marvell,mvebu-pinctrl.txt b/doc/device-tree-bindings/pinctrl/marvell,mvebu-pinctrl.txt
> new file mode 100644
> index 0000000..0973db8
> --- /dev/null
> +++ b/doc/device-tree-bindings/pinctrl/marvell,mvebu-pinctrl.txt
> @@ -0,0 +1,113 @@
> +The pinctrl driver enables Marvell Armada 8K SoCs to configure the multi-purpose
> +pins (mpp) to a specific function.
> +A Marvell SoC pin configuration node is a node of a group of pins which can
> +be used for a specific device or function. Each node requires one or more
> +mpp pins or group of pins and a mpp function common to all pins.
> +
> +Required properties for the pinctrl driver:
> +- compatible: "marvell,mvebu-pinctrl",
> + "marvell,armada-ap806-pinctrl",
> + "marvell,a70x0-pinctrl",
> + "marvell,a80x0-cp0-pinctrl",
> + "marvell,a80x0-cp1-pinctrl"
> +- bank-name: A string defining the pinc controller bank name
> +- reg: A pair of values defining the pin controller base address
> + and the address space
> +- pin-count: Numeric value defining the amount of multi purpose pins
> + included in this bank
> +- max-func: Numeric value defining the maximum function value for
> + pins in this bank
> +- pin-func: Array of pin function values for every pin in the bank.
> + When the function value for a specific pin equal 0xFF,
> + the pin configuration is skipped and a default function
> + value is used for this pin.
> +
> +The A8K is a hybrid SoC that contains several silicon dies interconnected in
> +a single package. Each such die may have a separate pin controller.
> +
> +Example:
> +/ {
> + ap806 {
> + config-space {
> + pinctl: pinctl at 6F4000 {
> + compatible = "marvell,mvebu-pinctrl",
> + "marvell,armada-ap806-pinctrl";
> + bank-name ="apn-806";
> + reg = <0x6F4000 0x10>;
> + pin-count = <20>;
> + max-func = <3>;
> + /* MPP Bus:
> + SPI0 [0-3]
> + I2C0 [4-5]
> + UART0 [11,19]
> + */
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 3 3 3 3 3 3 0 0 0 0
> + 0 3 0 0 0 0 0 0 0 3>;
> + };
> + };
> + };
> +
> + cp110-master {
> + config-space {
> + cpm_pinctl: pinctl at 44000 {
> + compatible = "marvell,mvebu-pinctrl",
> + "marvell,a70x0-pinctrl",
> + "marvell,a80x0-cp0-pinctrl";
> + bank-name ="cp0-110";
> + reg = <0x440000 0x20>;
> + pin-count = <63>;
> + max-func = <0xf>;
> + /* MPP Bus:
> + [0-31] = 0xff: Keep default CP0_shared_pins:
> + [11] CLKOUT_MPP_11 (out)
> + [23] LINK_RD_IN_CP2CP (in)
> + [25] CLKOUT_MPP_25 (out)
> + [29] AVS_FB_IN_CP2CP (in)
> + [32,34] SMI
> + [31] GPIO: push button/Wake
> + [35-36] GPIO
> + [37-38] I2C
> + [40-41] SATA[0/1]_PRESENT_ACTIVEn
> + [42-43] XSMI
> + [44-55] RGMII1
> + [56-62] SD
> + */
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0 7 0 7 0 0 2 2 0
> + 0 0 8 8 1 1 1 1 1 1
> + 1 1 1 1 1 1 0xE 0xE 0xE 0xE
> + 0xE 0xE 0xE>;
> + };
> + };
> + };
> +
> + cp110-slave {
> + config-space {
> + cps_pinctl: pinctl at 44000 {
> + compatible = "marvell,mvebu-pinctrl",
> + "marvell,a80x0-cp1-pinctrl";
> + bank-name ="cp1-110";
> + reg = <0x440000 0x20>;
> + pin-count = <63>;
> + max-func = <0xf>;
> + /* MPP Bus:
> + [0-11] RGMII0
> + [27,31] GE_MDIO/MDC
> + [32-62] = 0xff: Keep default CP1_shared_pins:
> + */
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3
> + 0x3 0x3 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0x8 0xff 0xff
> + 0xff 0x8 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff>;
> + };
> + };
> + };
> +}
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 12be3cf..efcb4c0 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -181,5 +181,6 @@ source "drivers/pinctrl/meson/Kconfig"
> source "drivers/pinctrl/nxp/Kconfig"
> source "drivers/pinctrl/uniphier/Kconfig"
> source "drivers/pinctrl/exynos/Kconfig"
> +source "drivers/pinctrl/mvebu/Kconfig"
>
> endmenu
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index f28b5c1..512112a 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -15,3 +15,4 @@ obj-$(CONFIG_PINCTRL_UNIPHIER) += uniphier/
> obj-$(CONFIG_PIC32_PINCTRL) += pinctrl_pic32.o
> obj-$(CONFIG_PINCTRL_EXYNOS) += exynos/
> obj-$(CONFIG_PINCTRL_MESON) += meson/
> +obj-$(CONFIG_PINCTRL_MVEBU) += mvebu/
> diff --git a/drivers/pinctrl/mvebu/Kconfig b/drivers/pinctrl/mvebu/Kconfig
> new file mode 100644
> index 0000000..cf9c299
> --- /dev/null
> +++ b/drivers/pinctrl/mvebu/Kconfig
> @@ -0,0 +1,7 @@
> +config PINCTRL_MVEBU
> + depends on ARCH_MVEBU
> + bool
> + default y
> + help
> + Support pin multiplexing and pin configuration control on
> + Marvell's Armada-8K SoC.
> diff --git a/drivers/pinctrl/mvebu/Makefile b/drivers/pinctrl/mvebu/Makefile
> new file mode 100644
> index 0000000..7db2b97
> --- /dev/null
> +++ b/drivers/pinctrl/mvebu/Makefile
> @@ -0,0 +1,17 @@
> +#* ***************************************************************************
> +#* Copyright (C) 2016 Marvell International Ltd.
> +#* ***************************************************************************
> +#* This program is free software: you can redistribute it and/or modify it
> +#* under the terms of the GNU General Public License as published by the Free
> +#* Software Foundation, either version 2 of the License, or any later version.
> +#*
> +#* This program is distributed in the hope that it will be useful,
> +#* but WITHOUT ANY WARRANTY; without even the implied warranty of
> +#* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +#* GNU General Public License for more details.
> +#*
> +#* You should have received a copy of the GNU General Public License
> +#* along with this program. If not, see <http://www.gnu.org/licenses/>.
> +#* ***************************************************************************
> +
> +obj-$(CONFIG_PINCTRL_MVEBU) += pinctrl-mvebu.o
> diff --git a/drivers/pinctrl/mvebu/pinctrl-mvebu.c b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
> new file mode 100644
> index 0000000..02fcd2f
> --- /dev/null
> +++ b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
> @@ -0,0 +1,195 @@
> +/*
> + * ***************************************************************************
> + * Copyright (C) 2016 Marvell International Ltd.
> + * ***************************************************************************
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation, either version 2 of the License, or any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + * ***************************************************************************
> + */
> +
> +#include <config.h>
> +#include <common.h>
> +#include <dm.h>
> +#include <asm/system.h>
> +#include <asm/io.h>
> +#include <dm/pinctrl.h>
> +#include <dm/root.h>
> +#include <fdtdec.h>
> +#include <errno.h>
> +#include <asm/arch/fdt.h>
> +#include <asm/arch-armada8k/soc-info.h>
> +#include "pinctrl-mvebu.h"
Please check include-file ordering here:
http://www.denx.de/wiki/U-Boot/CodingStyle
It is close.
> +
> +/*
> + * mvebu_pinctrl_set_state: configure pin functions.
> + * dev: the pinctrl device to be configured.
> + * config: the state to be configured.
/*
* mvebu_pinctrl_set_state() - configure pin functions
* @dev: the pinctrl device to be configured.
* @config: the state to be configured.
* @return ...
*/
Please use that consistently.
> + */
> +int mvebu_pinctrl_set_state(struct udevice *dev, struct udevice *config)
> +{
> + const void *blob = gd->fdt_blob;
> + int node = config->of_offset;
> + struct mvebu_pinctrl_priv *priv;
> + u32 pin_arr[CONFIG_MAX_PINS_PER_BANK];
> + u32 function;
> + int i, pin_count;
> +
> + priv = dev_get_priv(dev);
> + if (!priv) {
> + printf("%s: Failed to get private\n", __func__);
debug()
> + return -EINVAL;
> + }
> +
> + pin_count = fdtdec_get_int_array_count(blob, node, "marvell,pins", pin_arr, CONFIG_MAX_PINS_PER_BANK);
Are you sure this passes checkpatch? That line seems to long.
> + if (pin_count <= 0) {
> + error("Failed reading pins array for pinconfig %s (%d)\n", config->name, pin_count);
debug() to avoid bloating the code? (globally)
> + return -EINVAL;
> + }
> +
> + function = fdtdec_get_int(blob, node, "marvell,function", 0xFF);
nit: Lower case hex throughout
> +
> + for (i = 0; i < pin_count; i++) {
> + int reg_offset;
> + int field_offset;
> + u32 reg, mask;
> + int pin = pin_arr[i];
> +
> + if (function > priv->max_func) {
> + error("Illegal function %d for pinconfig %s\n", function, config->name);
> + return -EINVAL;
> + }
> +
> + /* Calculate register address and bit in register */
> + reg_offset = priv->reg_direction * 4 * (pin >> (PIN_REG_SHIFT));
> + field_offset = (BITS_PER_PIN) * (pin & PIN_FIELD_MASK);
> + mask = ~(PIN_FUNC_MASK << field_offset);
> +
> + /* Clip value to field resolution */
> + function &= PIN_FUNC_MASK;
> +
> + reg = readl(priv->base_reg + reg_offset);
> + reg = (reg & mask) | (function << field_offset);
> + writel(reg, priv->base_reg + reg_offset);
You can use clrsetbits_le32()
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * mvebu_pinctrl_set_state_all: configure the entire bank pin functions.
> + * dev: the pinctrl device to be configured.
> + * config: the state to be configured.
> + */
> +static int mvebu_pinctrl_set_state_all(struct udevice *dev, struct udevice *config)
> +{
> + const void *blob = gd->fdt_blob;
> + int node = config->of_offset;
> + struct mvebu_pinctrl_priv *priv;
> + u32 func_arr[CONFIG_MAX_PINS_PER_BANK];
> + int pin, err;
> +
> + priv = dev_get_priv(dev);
> + if (!priv) {
> + printf("%s: Failed to get private\n", __func__);
This cannot happen if the device is probed. Add an assert() if you are
paranoid, but it has not benefit.
> + return -EINVAL;
> + }
> +
> + err = fdtdec_get_int_array(blob, node, "pin-func", func_arr, priv->pin_cnt);
> + if (err) {
> + error("Failed reading pin functions for bank %s\n", priv->bank_name);
> + return -EINVAL;
> + }
> +
> + for (pin = 0; pin < priv->pin_cnt; pin++) {
> + int reg_offset;
> + int field_offset;
> + u32 reg, mask;
> + u32 func = func_arr[pin];
> +
> + /* Bypass pins with function 0xFF */
> + if (func == 0xFF) {
> + debug("Warning: pin %d value is not modified (kept as default\n", pin);
> + continue;
> + } else if (func > priv->max_func) {
> + error("Illegal function %d for pin %d\n", func, pin);
> + return -EINVAL;
> + }
> +
> + /* Calculate register address and bit in register */
> + reg_offset = priv->reg_direction * 4 * (pin >> (PIN_REG_SHIFT));
> + field_offset = (BITS_PER_PIN) * (pin & PIN_FIELD_MASK);
> + mask = ~(PIN_FUNC_MASK << field_offset);
> +
> + /* Clip value to field resolution */
> + func &= PIN_FUNC_MASK;
> +
> + reg = readl(priv->base_reg + reg_offset);
> + reg = (reg & mask) | (func << field_offset);
> + writel(reg, priv->base_reg + reg_offset);
clrsetbits_le32()
> + }
> +
> + return 0;
> +}
> +
> +int mvebu_pinctl_probe(struct udevice *dev)
> +{
> + const void *blob = gd->fdt_blob;
> + int node = dev->of_offset;
> + struct mvebu_pinctrl_priv *priv;
> + fdt_addr_t base;
> +
> + priv = dev_get_priv(dev);
> + if (!priv) {
> + printf("%s: Failed to get private\n", __func__);
debug() - please fix globally - drivers should not print messages.
> + return -EINVAL;
> + }
> +
> + base = dev_get_addr(dev);
> + if (base == FDT_ADDR_T_NONE) {
> + printf("%s: Failed to get base address\n", __func__);
> + return -EINVAL;
> + }
> +
> + priv->base_reg = (u8 *)base;
Or use dev_get_addr_ptr() ?
> + priv->pin_cnt = fdtdec_get_int(blob, node, "pin-count", CONFIG_MAX_PINS_PER_BANK);
> + priv->max_func = fdtdec_get_int(blob, node, "max-func", CONFIG_MAX_FUNC);
> + priv->bank_name = fdt_getprop(blob, node, "bank-name", NULL);
> +
> + priv->reg_direction = 1;
> + if (fdtdec_get_bool(blob, node, "reverse-reg"))
> + priv->reg_direction = -1;
> +
> + return mvebu_pinctrl_set_state_all(dev, dev);
> +}
> +
> +
> +static struct pinctrl_ops mvebu_pinctrl_ops = {
> + .set_state = mvebu_pinctrl_set_state
> +};
> +
> +static const struct udevice_id mvebu_pinctrl_ids[] = {
> + { .compatible = "marvell,mvebu-pinctrl" },
> + { .compatible = "marvell,armada-ap806-pinctrl" },
> + { .compatible = "marvell,a70x0-pinctrl" },
> + { .compatible = "marvell,a80x0-cp0-pinctrl" },
> + { .compatible = "marvell,a80x0-cp1-pinctrl" },
> + { }
> +};
> +
> +U_BOOT_DRIVER(pinctrl_mvebu) = {
> + .name = "mvebu_pinctrl",
> + .id = UCLASS_PINCTRL,
> + .of_match = mvebu_pinctrl_ids,
> + .priv_auto_alloc_size = sizeof(struct mvebu_pinctrl_priv),
> + .ops = &mvebu_pinctrl_ops,
> + .probe = mvebu_pinctl_probe
> +};
> diff --git a/drivers/pinctrl/mvebu/pinctrl-mvebu.h b/drivers/pinctrl/mvebu/pinctrl-mvebu.h
> new file mode 100644
> index 0000000..61c84ce
> --- /dev/null
> +++ b/drivers/pinctrl/mvebu/pinctrl-mvebu.h
> @@ -0,0 +1,44 @@
> +/*
> + * ***************************************************************************
> + * Copyright (C) 2016 Marvell International Ltd.
> + * ***************************************************************************
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation, either version 2 of the License, or any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + * ***************************************************************************
> + */
> +
> + #ifndef __PINCTRL_MVEBU_H_
> + #define __PINCTRL_MVEBU_H_
> +
> + #define CONFIG_MAX_PINCTL_BANKS 4
> + #define CONFIG_MAX_PINS_PER_BANK 100
> + #define CONFIG_MAX_FUNC 0xF
Avoid using CONFIG_ prefixes since this looks like a new global CONFIG
option. Just dropping the CONFIG_ will do.
> +
> +DECLARE_GLOBAL_DATA_PTR;
Please put that in the C file that needs it.
> +
> +/*
> + * struct mvebu_pin_bank_data: mvebu-pinctrl bank data
* struct mvebu_pin_bank_data - mvebu-pinctrl bank data
> + * @base_reg: controller base address for this bank
> + * @pin_cnt: number of ping included in this bank
ping?
> + * @max_func: maximum configurable function value for pins in this bank
> + * @reg_direction:
?
> + * @bank_name: the pins bank name
pin's
> + */
> +struct mvebu_pinctrl_priv {
> + u8 *base_reg;
> + u32 pin_cnt;
> + u32 max_func;
Why are these u32? Can they be uint?
> + int reg_direction;
> + const char *bank_name;
> +};
> +
> +#endif /* __PINCTRL_MVEBU_H_ */
> --
> 2.7.4
>
Regards,
Simon
^ permalink raw reply [flat|nested] 16+ messages in thread* [U-Boot] [PATCH 3/6] arm64: mvebu: pinctrl: Add pin control driver for A8K family
2016-11-24 2:20 ` Simon Glass
@ 2016-11-24 8:28 ` Kostya Porotchkin
0 siblings, 0 replies; 16+ messages in thread
From: Kostya Porotchkin @ 2016-11-24 8:28 UTC (permalink / raw)
To: u-boot
Hello, Simon,
Thank you very much for your review.
I am preparing a second patch release, which will include the requested fixes.
For the license I have to check with the company legal department first.
I am also going to fix some functions values in DTS files following internal review.
Regards
Konstantin
________________________________________
From: sjg@google.com <sjg@google.com> on behalf of Simon Glass <sjg@chromium.org>
Sent: Thursday, November 24, 2016 04:20
To: Kostya Porotchkin
Cc: U-Boot Mailing List; Stefan Roese; Nadav Haklai; Neta Zur Hershkovits; Omri Itach; Igal Liberman; Haim Boot; Hanna Hawa
Subject: Re: [PATCH 3/6] arm64: mvebu: pinctrl: Add pin control driver for A8K family
Hi,
On 20 November 2016 at 08:38, <kostap@marvell.com> wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
>
> Add a port of Marvell pin control driver.
> The A8K SoC family contains several silicone dies interconnected
> in a single package. Every die is normally equuipped with its own
> pin controller unit.
> Since the UCLASS_PINCTRL device only calls the probe method for
> the first detected pin controller, a trick similar to used with
> comphy driver is required.
> In order to bring up all pin controllers available in A8K SoC,
> the arch_early_init_r() function sequentially calls the
> uclass_get_device() function for each UCLASS_PINCTRL device.
>
> Change-Id: Iff143827e8f1558a554d77173562c4b52ce179d7
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Nadav Haklai <nadavh@marvell.com>
> Cc: Neta Zur Hershkovits <neta@marvell.com>
> Cc: Omri Itach <omrii@marvell.com>
> Cc: Igal Liberman <igall@marvell.com>
> Cc: Haim Boot <hayim@marvell.com>
> Cc: Hanna Hawa <hannah@marvell.com>
> ---
> arch/arm/include/asm/arch-armada8k/soc-info.h | 45 +++++
> arch/arm/mach-mvebu/arm64-common.c | 1 -
> .../pinctrl/marvell,mvebu-pinctrl.txt | 113 ++++++++++++
> drivers/pinctrl/Kconfig | 1 +
> drivers/pinctrl/Makefile | 1 +
> drivers/pinctrl/mvebu/Kconfig | 7 +
> drivers/pinctrl/mvebu/Makefile | 17 ++
> drivers/pinctrl/mvebu/pinctrl-mvebu.c | 195 +++++++++++++++++++++
> drivers/pinctrl/mvebu/pinctrl-mvebu.h | 44 +++++
> 9 files changed, 423 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm/include/asm/arch-armada8k/soc-info.h
> create mode 100644 doc/device-tree-bindings/pinctrl/marvell,mvebu-pinctrl.txt
> create mode 100644 drivers/pinctrl/mvebu/Kconfig
> create mode 100644 drivers/pinctrl/mvebu/Makefile
> create mode 100644 drivers/pinctrl/mvebu/pinctrl-mvebu.c
> create mode 100644 drivers/pinctrl/mvebu/pinctrl-mvebu.h
>
Generally looks good but I have a load of nits sorry.
> diff --git a/arch/arm/include/asm/arch-armada8k/soc-info.h b/arch/arm/include/asm/arch-armada8k/soc-info.h
> new file mode 100644
> index 0000000..4640deb
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-armada8k/soc-info.h
> @@ -0,0 +1,45 @@
> +/*
> + * ***************************************************************************
> + * Copyright (C) 2015 Marvell International Ltd.
> + * ***************************************************************************
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation, either version 2 of the License, or any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + * ***************************************************************************
Can you please use SPDX?
> + */
> +
> +#ifndef _SOC_INFO_H_
> +#define _SOC_INFO_H_
> +
> +/* General MPP definitions */
> +#define MAX_MPP_OPTS 7
> +#define MAX_MPP_ID 15
> +
> +#define MPP_BIT_CNT 4
> +#define MPP_FIELD_MASK 0x7
> +#define MPP_FIELD_BITS 3
> +#define MPP_VAL_MASK 0xF
> +
> +#define MPPS_PER_REG (32 / MPP_BIT_CNT)
> +#define MAX_MPP_REGS ((MAX_MPP_ID + MPPS_PER_REG) / MPPS_PER_REG)
> +
> +/* MPP pins and functions correcsponding to UART RX connections
> + This information is used for detection of recovery boot mode (boot from UART) */
/*
* MPP pins
* ...
* /
> +#define MPP_UART_RX_PINS { 3, 5 }
> +#define MPP_UART_RX_FUNCTIONS { 1, 2 }
> +
> +/* Pin Ctrl driver definitions */
> +#define BITS_PER_PIN 4
> +#define PIN_FUNC_MASK ((1 << BITS_PER_PIN) - 1)
> +#define PIN_REG_SHIFT 3
> +#define PIN_FIELD_MASK ((1 << PIN_REG_SHIFT) - 1)
> +
> +#endif /* _SOC_INFO_H_ */
> diff --git a/arch/arm/mach-mvebu/arm64-common.c b/arch/arm/mach-mvebu/arm64-common.c
> index 1fc2ff2..78fe7a7 100644
> --- a/arch/arm/mach-mvebu/arm64-common.c
> +++ b/arch/arm/mach-mvebu/arm64-common.c
> @@ -124,7 +124,6 @@ int arch_early_init_r(void)
> if (ret)
> break;
> }
> -
Unrelated change?
> /* Cause the SATA device to do its early init */
> uclass_first_device(UCLASS_AHCI, &dev);
>
> diff --git a/doc/device-tree-bindings/pinctrl/marvell,mvebu-pinctrl.txt b/doc/device-tree-bindings/pinctrl/marvell,mvebu-pinctrl.txt
> new file mode 100644
> index 0000000..0973db8
> --- /dev/null
> +++ b/doc/device-tree-bindings/pinctrl/marvell,mvebu-pinctrl.txt
> @@ -0,0 +1,113 @@
> +The pinctrl driver enables Marvell Armada 8K SoCs to configure the multi-purpose
> +pins (mpp) to a specific function.
> +A Marvell SoC pin configuration node is a node of a group of pins which can
> +be used for a specific device or function. Each node requires one or more
> +mpp pins or group of pins and a mpp function common to all pins.
> +
> +Required properties for the pinctrl driver:
> +- compatible: "marvell,mvebu-pinctrl",
> + "marvell,armada-ap806-pinctrl",
> + "marvell,a70x0-pinctrl",
> + "marvell,a80x0-cp0-pinctrl",
> + "marvell,a80x0-cp1-pinctrl"
> +- bank-name: A string defining the pinc controller bank name
> +- reg: A pair of values defining the pin controller base address
> + and the address space
> +- pin-count: Numeric value defining the amount of multi purpose pins
> + included in this bank
> +- max-func: Numeric value defining the maximum function value for
> + pins in this bank
> +- pin-func: Array of pin function values for every pin in the bank.
> + When the function value for a specific pin equal 0xFF,
> + the pin configuration is skipped and a default function
> + value is used for this pin.
> +
> +The A8K is a hybrid SoC that contains several silicon dies interconnected in
> +a single package. Each such die may have a separate pin controller.
> +
> +Example:
> +/ {
> + ap806 {
> + config-space {
> + pinctl: pinctl at 6F4000 {
> + compatible = "marvell,mvebu-pinctrl",
> + "marvell,armada-ap806-pinctrl";
> + bank-name ="apn-806";
> + reg = <0x6F4000 0x10>;
> + pin-count = <20>;
> + max-func = <3>;
> + /* MPP Bus:
> + SPI0 [0-3]
> + I2C0 [4-5]
> + UART0 [11,19]
> + */
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 3 3 3 3 3 3 0 0 0 0
> + 0 3 0 0 0 0 0 0 0 3>;
> + };
> + };
> + };
> +
> + cp110-master {
> + config-space {
> + cpm_pinctl: pinctl at 44000 {
> + compatible = "marvell,mvebu-pinctrl",
> + "marvell,a70x0-pinctrl",
> + "marvell,a80x0-cp0-pinctrl";
> + bank-name ="cp0-110";
> + reg = <0x440000 0x20>;
> + pin-count = <63>;
> + max-func = <0xf>;
> + /* MPP Bus:
> + [0-31] = 0xff: Keep default CP0_shared_pins:
> + [11] CLKOUT_MPP_11 (out)
> + [23] LINK_RD_IN_CP2CP (in)
> + [25] CLKOUT_MPP_25 (out)
> + [29] AVS_FB_IN_CP2CP (in)
> + [32,34] SMI
> + [31] GPIO: push button/Wake
> + [35-36] GPIO
> + [37-38] I2C
> + [40-41] SATA[0/1]_PRESENT_ACTIVEn
> + [42-43] XSMI
> + [44-55] RGMII1
> + [56-62] SD
> + */
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0 7 0 7 0 0 2 2 0
> + 0 0 8 8 1 1 1 1 1 1
> + 1 1 1 1 1 1 0xE 0xE 0xE 0xE
> + 0xE 0xE 0xE>;
> + };
> + };
> + };
> +
> + cp110-slave {
> + config-space {
> + cps_pinctl: pinctl at 44000 {
> + compatible = "marvell,mvebu-pinctrl",
> + "marvell,a80x0-cp1-pinctrl";
> + bank-name ="cp1-110";
> + reg = <0x440000 0x20>;
> + pin-count = <63>;
> + max-func = <0xf>;
> + /* MPP Bus:
> + [0-11] RGMII0
> + [27,31] GE_MDIO/MDC
> + [32-62] = 0xff: Keep default CP1_shared_pins:
> + */
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3
> + 0x3 0x3 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0x8 0xff 0xff
> + 0xff 0x8 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff>;
> + };
> + };
> + };
> +}
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 12be3cf..efcb4c0 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -181,5 +181,6 @@ source "drivers/pinctrl/meson/Kconfig"
> source "drivers/pinctrl/nxp/Kconfig"
> source "drivers/pinctrl/uniphier/Kconfig"
> source "drivers/pinctrl/exynos/Kconfig"
> +source "drivers/pinctrl/mvebu/Kconfig"
>
> endmenu
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index f28b5c1..512112a 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -15,3 +15,4 @@ obj-$(CONFIG_PINCTRL_UNIPHIER) += uniphier/
> obj-$(CONFIG_PIC32_PINCTRL) += pinctrl_pic32.o
> obj-$(CONFIG_PINCTRL_EXYNOS) += exynos/
> obj-$(CONFIG_PINCTRL_MESON) += meson/
> +obj-$(CONFIG_PINCTRL_MVEBU) += mvebu/
> diff --git a/drivers/pinctrl/mvebu/Kconfig b/drivers/pinctrl/mvebu/Kconfig
> new file mode 100644
> index 0000000..cf9c299
> --- /dev/null
> +++ b/drivers/pinctrl/mvebu/Kconfig
> @@ -0,0 +1,7 @@
> +config PINCTRL_MVEBU
> + depends on ARCH_MVEBU
> + bool
> + default y
> + help
> + Support pin multiplexing and pin configuration control on
> + Marvell's Armada-8K SoC.
> diff --git a/drivers/pinctrl/mvebu/Makefile b/drivers/pinctrl/mvebu/Makefile
> new file mode 100644
> index 0000000..7db2b97
> --- /dev/null
> +++ b/drivers/pinctrl/mvebu/Makefile
> @@ -0,0 +1,17 @@
> +#* ***************************************************************************
> +#* Copyright (C) 2016 Marvell International Ltd.
> +#* ***************************************************************************
> +#* This program is free software: you can redistribute it and/or modify it
> +#* under the terms of the GNU General Public License as published by the Free
> +#* Software Foundation, either version 2 of the License, or any later version.
> +#*
> +#* This program is distributed in the hope that it will be useful,
> +#* but WITHOUT ANY WARRANTY; without even the implied warranty of
> +#* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +#* GNU General Public License for more details.
> +#*
> +#* You should have received a copy of the GNU General Public License
> +#* along with this program. If not, see <http://www.gnu.org/licenses/>.
> +#* ***************************************************************************
> +
> +obj-$(CONFIG_PINCTRL_MVEBU) += pinctrl-mvebu.o
> diff --git a/drivers/pinctrl/mvebu/pinctrl-mvebu.c b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
> new file mode 100644
> index 0000000..02fcd2f
> --- /dev/null
> +++ b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
> @@ -0,0 +1,195 @@
> +/*
> + * ***************************************************************************
> + * Copyright (C) 2016 Marvell International Ltd.
> + * ***************************************************************************
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation, either version 2 of the License, or any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + * ***************************************************************************
> + */
> +
> +#include <config.h>
> +#include <common.h>
> +#include <dm.h>
> +#include <asm/system.h>
> +#include <asm/io.h>
> +#include <dm/pinctrl.h>
> +#include <dm/root.h>
> +#include <fdtdec.h>
> +#include <errno.h>
> +#include <asm/arch/fdt.h>
> +#include <asm/arch-armada8k/soc-info.h>
> +#include "pinctrl-mvebu.h"
Please check include-file ordering here:
http://www.denx.de/wiki/U-Boot/CodingStyle
It is close.
> +
> +/*
> + * mvebu_pinctrl_set_state: configure pin functions.
> + * dev: the pinctrl device to be configured.
> + * config: the state to be configured.
/*
* mvebu_pinctrl_set_state() - configure pin functions
* @dev: the pinctrl device to be configured.
* @config: the state to be configured.
* @return ...
*/
Please use that consistently.
> + */
> +int mvebu_pinctrl_set_state(struct udevice *dev, struct udevice *config)
> +{
> + const void *blob = gd->fdt_blob;
> + int node = config->of_offset;
> + struct mvebu_pinctrl_priv *priv;
> + u32 pin_arr[CONFIG_MAX_PINS_PER_BANK];
> + u32 function;
> + int i, pin_count;
> +
> + priv = dev_get_priv(dev);
> + if (!priv) {
> + printf("%s: Failed to get private\n", __func__);
debug()
> + return -EINVAL;
> + }
> +
> + pin_count = fdtdec_get_int_array_count(blob, node, "marvell,pins", pin_arr, CONFIG_MAX_PINS_PER_BANK);
Are you sure this passes checkpatch? That line seems to long.
> + if (pin_count <= 0) {
> + error("Failed reading pins array for pinconfig %s (%d)\n", config->name, pin_count);
debug() to avoid bloating the code? (globally)
> + return -EINVAL;
> + }
> +
> + function = fdtdec_get_int(blob, node, "marvell,function", 0xFF);
nit: Lower case hex throughout
> +
> + for (i = 0; i < pin_count; i++) {
> + int reg_offset;
> + int field_offset;
> + u32 reg, mask;
> + int pin = pin_arr[i];
> +
> + if (function > priv->max_func) {
> + error("Illegal function %d for pinconfig %s\n", function, config->name);
> + return -EINVAL;
> + }
> +
> + /* Calculate register address and bit in register */
> + reg_offset = priv->reg_direction * 4 * (pin >> (PIN_REG_SHIFT));
> + field_offset = (BITS_PER_PIN) * (pin & PIN_FIELD_MASK);
> + mask = ~(PIN_FUNC_MASK << field_offset);
> +
> + /* Clip value to field resolution */
> + function &= PIN_FUNC_MASK;
> +
> + reg = readl(priv->base_reg + reg_offset);
> + reg = (reg & mask) | (function << field_offset);
> + writel(reg, priv->base_reg + reg_offset);
You can use clrsetbits_le32()
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * mvebu_pinctrl_set_state_all: configure the entire bank pin functions.
> + * dev: the pinctrl device to be configured.
> + * config: the state to be configured.
> + */
> +static int mvebu_pinctrl_set_state_all(struct udevice *dev, struct udevice *config)
> +{
> + const void *blob = gd->fdt_blob;
> + int node = config->of_offset;
> + struct mvebu_pinctrl_priv *priv;
> + u32 func_arr[CONFIG_MAX_PINS_PER_BANK];
> + int pin, err;
> +
> + priv = dev_get_priv(dev);
> + if (!priv) {
> + printf("%s: Failed to get private\n", __func__);
This cannot happen if the device is probed. Add an assert() if you are
paranoid, but it has not benefit.
> + return -EINVAL;
> + }
> +
> + err = fdtdec_get_int_array(blob, node, "pin-func", func_arr, priv->pin_cnt);
> + if (err) {
> + error("Failed reading pin functions for bank %s\n", priv->bank_name);
> + return -EINVAL;
> + }
> +
> + for (pin = 0; pin < priv->pin_cnt; pin++) {
> + int reg_offset;
> + int field_offset;
> + u32 reg, mask;
> + u32 func = func_arr[pin];
> +
> + /* Bypass pins with function 0xFF */
> + if (func == 0xFF) {
> + debug("Warning: pin %d value is not modified (kept as default\n", pin);
> + continue;
> + } else if (func > priv->max_func) {
> + error("Illegal function %d for pin %d\n", func, pin);
> + return -EINVAL;
> + }
> +
> + /* Calculate register address and bit in register */
> + reg_offset = priv->reg_direction * 4 * (pin >> (PIN_REG_SHIFT));
> + field_offset = (BITS_PER_PIN) * (pin & PIN_FIELD_MASK);
> + mask = ~(PIN_FUNC_MASK << field_offset);
> +
> + /* Clip value to field resolution */
> + func &= PIN_FUNC_MASK;
> +
> + reg = readl(priv->base_reg + reg_offset);
> + reg = (reg & mask) | (func << field_offset);
> + writel(reg, priv->base_reg + reg_offset);
clrsetbits_le32()
> + }
> +
> + return 0;
> +}
> +
> +int mvebu_pinctl_probe(struct udevice *dev)
> +{
> + const void *blob = gd->fdt_blob;
> + int node = dev->of_offset;
> + struct mvebu_pinctrl_priv *priv;
> + fdt_addr_t base;
> +
> + priv = dev_get_priv(dev);
> + if (!priv) {
> + printf("%s: Failed to get private\n", __func__);
debug() - please fix globally - drivers should not print messages.
> + return -EINVAL;
> + }
> +
> + base = dev_get_addr(dev);
> + if (base == FDT_ADDR_T_NONE) {
> + printf("%s: Failed to get base address\n", __func__);
> + return -EINVAL;
> + }
> +
> + priv->base_reg = (u8 *)base;
Or use dev_get_addr_ptr() ?
> + priv->pin_cnt = fdtdec_get_int(blob, node, "pin-count", CONFIG_MAX_PINS_PER_BANK);
> + priv->max_func = fdtdec_get_int(blob, node, "max-func", CONFIG_MAX_FUNC);
> + priv->bank_name = fdt_getprop(blob, node, "bank-name", NULL);
> +
> + priv->reg_direction = 1;
> + if (fdtdec_get_bool(blob, node, "reverse-reg"))
> + priv->reg_direction = -1;
> +
> + return mvebu_pinctrl_set_state_all(dev, dev);
> +}
> +
> +
> +static struct pinctrl_ops mvebu_pinctrl_ops = {
> + .set_state = mvebu_pinctrl_set_state
> +};
> +
> +static const struct udevice_id mvebu_pinctrl_ids[] = {
> + { .compatible = "marvell,mvebu-pinctrl" },
> + { .compatible = "marvell,armada-ap806-pinctrl" },
> + { .compatible = "marvell,a70x0-pinctrl" },
> + { .compatible = "marvell,a80x0-cp0-pinctrl" },
> + { .compatible = "marvell,a80x0-cp1-pinctrl" },
> + { }
> +};
> +
> +U_BOOT_DRIVER(pinctrl_mvebu) = {
> + .name = "mvebu_pinctrl",
> + .id = UCLASS_PINCTRL,
> + .of_match = mvebu_pinctrl_ids,
> + .priv_auto_alloc_size = sizeof(struct mvebu_pinctrl_priv),
> + .ops = &mvebu_pinctrl_ops,
> + .probe = mvebu_pinctl_probe
> +};
> diff --git a/drivers/pinctrl/mvebu/pinctrl-mvebu.h b/drivers/pinctrl/mvebu/pinctrl-mvebu.h
> new file mode 100644
> index 0000000..61c84ce
> --- /dev/null
> +++ b/drivers/pinctrl/mvebu/pinctrl-mvebu.h
> @@ -0,0 +1,44 @@
> +/*
> + * ***************************************************************************
> + * Copyright (C) 2016 Marvell International Ltd.
> + * ***************************************************************************
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation, either version 2 of the License, or any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + * ***************************************************************************
> + */
> +
> + #ifndef __PINCTRL_MVEBU_H_
> + #define __PINCTRL_MVEBU_H_
> +
> + #define CONFIG_MAX_PINCTL_BANKS 4
> + #define CONFIG_MAX_PINS_PER_BANK 100
> + #define CONFIG_MAX_FUNC 0xF
Avoid using CONFIG_ prefixes since this looks like a new global CONFIG
option. Just dropping the CONFIG_ will do.
> +
> +DECLARE_GLOBAL_DATA_PTR;
Please put that in the C file that needs it.
> +
> +/*
> + * struct mvebu_pin_bank_data: mvebu-pinctrl bank data
* struct mvebu_pin_bank_data - mvebu-pinctrl bank data
> + * @base_reg: controller base address for this bank
> + * @pin_cnt: number of ping included in this bank
ping?
> + * @max_func: maximum configurable function value for pins in this bank
> + * @reg_direction:
?
> + * @bank_name: the pins bank name
pin's
> + */
> +struct mvebu_pinctrl_priv {
> + u8 *base_reg;
> + u32 pin_cnt;
> + u32 max_func;
Why are these u32? Can they be uint?
> + int reg_direction;
> + const char *bank_name;
> +};
> +
> +#endif /* __PINCTRL_MVEBU_H_ */
> --
> 2.7.4
>
Regards,
Simon
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH 4/6] arm64: mvebu: Add pin control nodes to A8K family DTS files
2016-11-20 15:38 [U-Boot] [PATCH 0/6] arm64: mvebu: Armada-8K family patches kostap at marvell.com
` (2 preceding siblings ...)
2016-11-20 15:38 ` [U-Boot] [PATCH 3/6] arm64: mvebu: pinctrl: Add pin control driver for A8K family kostap at marvell.com
@ 2016-11-20 15:38 ` kostap at marvell.com
2016-11-24 9:13 ` Stefan Roese
2016-11-20 15:38 ` [U-Boot] [PATCH 5/6] arm64: mvebu: Enable BUBT command support in A8K default config kostap at marvell.com
2016-11-20 15:38 ` [U-Boot] [PATCH 6/6] arm64: mvebu: Enable pin control " kostap at marvell.com
5 siblings, 1 reply; 16+ messages in thread
From: kostap at marvell.com @ 2016-11-20 15:38 UTC (permalink / raw)
To: u-boot
From: Konstantin Porotchkin <kostap@marvell.com>
Add pin control nodes to APN806, CP-master, CP-slave and
Armada-7040 and Armada-8040 boards DTS files
Change-Id: I51522f33f23e3f9e94c6f5a5f9af19f5dc86e6b7
Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
Cc: Stefan Roese <sr@denx.de>
Cc: Nadav Haklai <nadavh@marvell.com>
Cc: Neta Zur Hershkovits <neta@marvell.com>
Cc: Omri Itach <omrii@marvell.com>
Cc: Igal Liberman <igall@marvell.com>
Cc: Haim Boot <hayim@marvell.com>
Cc: Hanna Hawa <hannah@marvell.com>
---
arch/arm/dts/armada-7040-db.dts | 38 +++++++++++++++++++++++
arch/arm/dts/armada-8040-db.dts | 57 +++++++++++++++++++++++++++++++++++
arch/arm/dts/armada-ap806.dtsi | 18 +++++++++++
arch/arm/dts/armada-cp110-master.dtsi | 32 ++++++++++++++++++++
arch/arm/dts/armada-cp110-slave.dtsi | 19 ++++++++++++
5 files changed, 164 insertions(+)
diff --git a/arch/arm/dts/armada-7040-db.dts b/arch/arm/dts/armada-7040-db.dts
index b8fe5a9..1bfb5a9 100644
--- a/arch/arm/dts/armada-7040-db.dts
+++ b/arch/arm/dts/armada-7040-db.dts
@@ -67,6 +67,8 @@
};
&i2c0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&cpm_i2c0_pins>;
status = "okay";
clock-frequency = <100000>;
};
@@ -98,6 +100,17 @@
};
};
+&ap_pinctl {
+ /* MPP Bus:
+ SDIO [0-10]
+ UART0 [11,19]
+ */
+ /* 0 1 2 3 4 5 6 7 8 9 */
+ pin-func = < 1 1 1 1 1 1 1 1 1 1
+ 1 3 0 0 0 0 0 0 0 3>;
+ status = "okay";
+};
+
&uart0 {
status = "okay";
};
@@ -112,8 +125,33 @@
clock-frequency = <100000>;
};
+&cpm_pinctl {
+ /* MPP Bus:
+ TDM [0-11]
+ SPI [13-16]
+ SATA1 [28]
+ UART0 [29-30]
+ SMI [32,34]
+ XSMI [35-36]
+ I2C [37-38]
+ RGMII1[44-55]
+ SD [56-62]
+ */
+ /* 0 1 2 3 4 5 6 7 8 9 */
+ pin-func = < 4 4 4 4 4 4 4 4 4 4
+ 4 4 0 3 3 3 3 0 0 0
+ 0 0 0 0 0 0 0 0 9 0xA
+ 0xA 0 7 0 7 7 7 2 2 0
+ 0 0 0 0 1 1 1 1 1 1
+ 1 1 1 1 1 1 0xE 0xE 0xE 0xE
+ 0xE 0xE 0xE>;
+ status = "okay";
+};
+
&cpm_spi1 {
status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&cpm_spi0_pins>;
spi-flash at 0 {
#address-cells = <0x1>;
diff --git a/arch/arm/dts/armada-8040-db.dts b/arch/arm/dts/armada-8040-db.dts
index 86666a1..30fd364 100644
--- a/arch/arm/dts/armada-8040-db.dts
+++ b/arch/arm/dts/armada-8040-db.dts
@@ -71,6 +71,42 @@
status = "okay";
};
+&ap_pinctl {
+ /* MPP Bus:
+ SPI0 [0-3]
+ I2C0 [4-5]
+ UART0 [11,19]
+ */
+ /* 0 1 2 3 4 5 6 7 8 9 */
+ pin-func = < 3 3 3 3 3 3 0 0 0 0
+ 0 3 0 0 0 0 0 0 0 3>;
+};
+
+&cpm_pinctl {
+ /* MPP Bus:
+ [0-31] = 0xff: Keep default CP0_shared_pins:
+ [11] CLKOUT_MPP_11 (out)
+ [23] LINK_RD_IN_CP2CP (in)
+ [25] CLKOUT_MPP_25 (out)
+ [29] AVS_FB_IN_CP2CP (in)
+ [32,34] SMI
+ [31] GPIO: push button/Wake
+ [35-36] GPIO
+ [37-38] I2C
+ [40-41] SATA[0/1]_PRESENT_ACTIVEn
+ [42-43] XSMI
+ [44-55] RGMII1
+ [56-62] SD
+ */
+ /* 0 1 2 3 4 5 6 7 8 9 */
+ pin-func = < 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
+ 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
+ 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
+ 0xff 0 7 0 7 0 0 2 2 0
+ 0 0 8 8 1 1 1 1 1 1
+ 1 1 1 1 1 1 0xE 0xE 0xE 0xE
+ 0xE 0xE 0xE>;
+};
/* CON5 on CP0 expansion */
&cpm_pcie2 {
@@ -78,6 +114,8 @@
};
&cpm_i2c0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&cpm_i2c0_pins>;
status = "okay";
clock-frequency = <100000>;
};
@@ -97,12 +135,31 @@
status = "okay";
};
+&cps_pinctl {
+ /* MPP Bus:
+ [0-11] RGMII0
+ [13-16] SPI1
+ [27,31] GE_MDIO/MDC
+ [32-62] = 0xff: Keep default CP1_shared_pins:
+ */
+ /* 0 1 2 3 4 5 6 7 8 9 */
+ pin-func = < 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3
+ 0x3 0x3 0xff 0x3 0x3 0x3 0x3 0xff 0xff 0xff
+ 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0x8 0xff 0xff
+ 0xff 0x8 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
+ 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
+ 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
+ 0xff 0xff 0xff>;
+};
+
/* CON5 on CP1 expansion */
&cps_pcie2 {
status = "okay";
};
&cps_spi1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&cps_spi1_pins>;
status = "okay";
spi-flash at 0 {
diff --git a/arch/arm/dts/armada-ap806.dtsi b/arch/arm/dts/armada-ap806.dtsi
index d315b29..efb383b 100644
--- a/arch/arm/dts/armada-ap806.dtsi
+++ b/arch/arm/dts/armada-ap806.dtsi
@@ -140,6 +140,24 @@
marvell,spi-base = <128>, <136>, <144>, <152>;
};
+ ap_pinctl: ap-pinctl at 6F4000 {
+ compatible = "marvell,armada-ap806-pinctrl";
+ bank-name ="apn-806";
+ reg = <0x6F4000 0x10>;
+ pin-count = <20>;
+ max-func = <3>;
+
+ ap_i2c0_pins: i2c-pins-0 {
+ marvell,pins = < 4 5 >;
+ marvell,function = <3>;
+ };
+ ap_emmc_pins: emmc-pins-0 {
+ marvell,pins = < 0 1 2 3 4 5 6 7
+ 8 9 10 >;
+ marvell,function = <1>;
+ };
+ };
+
xor at 400000 {
compatible = "marvell,mv-xor-v2";
reg = <0x400000 0x1000>,
diff --git a/arch/arm/dts/armada-cp110-master.dtsi b/arch/arm/dts/armada-cp110-master.dtsi
index 422d754..d637867 100644
--- a/arch/arm/dts/armada-cp110-master.dtsi
+++ b/arch/arm/dts/armada-cp110-master.dtsi
@@ -81,6 +81,38 @@
"cpm-usb3dev", "cpm-eip150", "cpm-eip197";
};
+ cpm_pinctl: cpm-pinctl at 440000 {
+ compatible = "marvell,mvebu-pinctrl",
+ "marvell,a70x0-pinctrl",
+ "marvell,a80x0-cp0-pinctrl";
+ bank-name ="cp0-110";
+ reg = <0x440000 0x20>;
+ pin-count = <63>;
+ max-func = <0xf>;
+
+ cpm_i2c0_pins: cpm-i2c-pins-0 {
+ marvell,pins = < 37 38 >;
+ marvell,function = <2>;
+ };
+ cpm_ge2_rgmii_pins: cpm-ge-rgmii-pins-0 {
+ marvell,pins = < 44 45 46 47 48 49 50 51
+ 52 53 54 55 >;
+ marvell,function = <1>;
+ };
+ pca0_pins: cpm-pca0_pins {
+ marvell,pins = <62>;
+ marvell,function = <0>;
+ };
+ cpm_sdhci_pins: cpm-sdhi-pins-0 {
+ marvell,pins = < 56 57 58 59 60 61 >;
+ marvell,function = <14>;
+ };
+ cpm_spi0_pins: cpm-spi-pins-0 {
+ marvell,pins = < 13 14 15 16 >;
+ marvell,function = <3>;
+ };
+ };
+
cpm_sata0: sata at 540000 {
compatible = "marvell,armada-8k-ahci";
reg = <0x540000 0x30000>;
diff --git a/arch/arm/dts/armada-cp110-slave.dtsi b/arch/arm/dts/armada-cp110-slave.dtsi
index a7f77b9..92ef55c 100644
--- a/arch/arm/dts/armada-cp110-slave.dtsi
+++ b/arch/arm/dts/armada-cp110-slave.dtsi
@@ -81,6 +81,25 @@
"cps-usb3dev", "cps-eip150", "cps-eip197";
};
+ cps_pinctl: cps-pinctl at 440000 {
+ compatible = "marvell,mvebu-pinctrl",
+ "marvell,a80x0-cp1-pinctrl";
+ bank-name ="cp1-110";
+ reg = <0x440000 0x20>;
+ pin-count = <63>;
+ max-func = <0xf>;
+
+ cps_ge1_rgmii_pins: cps-ge-rgmii-pins-0 {
+ marvell,pins = < 0 1 2 3 4 5 6 7
+ 8 9 10 11 >;
+ marvell,function = <3>;
+ };
+ cps_spi1_pins: cps-spi-pins-1 {
+ marvell,pins = < 13 14 15 16 >;
+ marvell,function = <3>;
+ };
+ };
+
cps_sata0: sata at 540000 {
compatible = "marvell,armada-8k-ahci";
reg = <0x540000 0x30000>;
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [U-Boot] [PATCH 4/6] arm64: mvebu: Add pin control nodes to A8K family DTS files
2016-11-20 15:38 ` [U-Boot] [PATCH 4/6] arm64: mvebu: Add pin control nodes to A8K family DTS files kostap at marvell.com
@ 2016-11-24 9:13 ` Stefan Roese
2016-11-24 14:09 ` Kostya Porotchkin
0 siblings, 1 reply; 16+ messages in thread
From: Stefan Roese @ 2016-11-24 9:13 UTC (permalink / raw)
To: u-boot
Hi Kosta,
On 20.11.2016 16:38, kostap at marvell.com wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
>
> Add pin control nodes to APN806, CP-master, CP-slave and
> Armada-7040 and Armada-8040 boards DTS files
>
> Change-Id: I51522f33f23e3f9e94c6f5a5f9af19f5dc86e6b7
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Nadav Haklai <nadavh@marvell.com>
> Cc: Neta Zur Hershkovits <neta@marvell.com>
> Cc: Omri Itach <omrii@marvell.com>
> Cc: Igal Liberman <igall@marvell.com>
> Cc: Haim Boot <hayim@marvell.com>
> Cc: Hanna Hawa <hannah@marvell.com>
> ---
> arch/arm/dts/armada-7040-db.dts | 38 +++++++++++++++++++++++
> arch/arm/dts/armada-8040-db.dts | 57 +++++++++++++++++++++++++++++++++++
> arch/arm/dts/armada-ap806.dtsi | 18 +++++++++++
> arch/arm/dts/armada-cp110-master.dtsi | 32 ++++++++++++++++++++
> arch/arm/dts/armada-cp110-slave.dtsi | 19 ++++++++++++
> 5 files changed, 164 insertions(+)
>
> diff --git a/arch/arm/dts/armada-7040-db.dts b/arch/arm/dts/armada-7040-db.dts
> index b8fe5a9..1bfb5a9 100644
> --- a/arch/arm/dts/armada-7040-db.dts
> +++ b/arch/arm/dts/armada-7040-db.dts
> @@ -67,6 +67,8 @@
> };
>
> &i2c0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&cpm_i2c0_pins>;
> status = "okay";
> clock-frequency = <100000>;
> };
> @@ -98,6 +100,17 @@
> };
> };
>
> +&ap_pinctl {
> + /* MPP Bus:
> + SDIO [0-10]
> + UART0 [11,19]
> + */
Please use the common multiline comment instead:
/*
* MPP Bus:
* SDIO [0-10]
* UART0 [11,19]
*/
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 1 1 1 1 1 1 1 1 1 1
> + 1 3 0 0 0 0 0 0 0 3>;
Is there any chance to not use those numeric values but some macros
instead to make it clearer, which function is selected?
> + status = "okay";
> +};
> +
> &uart0 {
> status = "okay";
> };
> @@ -112,8 +125,33 @@
> clock-frequency = <100000>;
> };
>
> +&cpm_pinctl {
> + /* MPP Bus:
> + TDM [0-11]
> + SPI [13-16]
> + SATA1 [28]
> + UART0 [29-30]
> + SMI [32,34]
> + XSMI [35-36]
> + I2C [37-38]
> + RGMII1[44-55]
> + SD [56-62]
> + */
Again, comment style please.
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 4 4 4 4 4 4 4 4 4 4
> + 4 4 0 3 3 3 3 0 0 0
> + 0 0 0 0 0 0 0 0 9 0xA
> + 0xA 0 7 0 7 7 7 2 2 0
> + 0 0 0 0 1 1 1 1 1 1
> + 1 1 1 1 1 1 0xE 0xE 0xE 0xE
> + 0xE 0xE 0xE>;
Lower case hex values please (globally).
> + status = "okay";
> +};
> +
> &cpm_spi1 {
> status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&cpm_spi0_pins>;
>
> spi-flash at 0 {
> #address-cells = <0x1>;
> diff --git a/arch/arm/dts/armada-8040-db.dts b/arch/arm/dts/armada-8040-db.dts
> index 86666a1..30fd364 100644
> --- a/arch/arm/dts/armada-8040-db.dts
> +++ b/arch/arm/dts/armada-8040-db.dts
> @@ -71,6 +71,42 @@
> status = "okay";
> };
>
> +&ap_pinctl {
> + /* MPP Bus:
> + SPI0 [0-3]
> + I2C0 [4-5]
> + UART0 [11,19]
> + */
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 3 3 3 3 3 3 0 0 0 0
> + 0 3 0 0 0 0 0 0 0 3>;
> +};
> +
> +&cpm_pinctl {
> + /* MPP Bus:
> + [0-31] = 0xff: Keep default CP0_shared_pins:
> + [11] CLKOUT_MPP_11 (out)
> + [23] LINK_RD_IN_CP2CP (in)
> + [25] CLKOUT_MPP_25 (out)
> + [29] AVS_FB_IN_CP2CP (in)
> + [32,34] SMI
> + [31] GPIO: push button/Wake
> + [35-36] GPIO
> + [37-38] I2C
> + [40-41] SATA[0/1]_PRESENT_ACTIVEn
> + [42-43] XSMI
> + [44-55] RGMII1
> + [56-62] SD
> + */
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0 7 0 7 0 0 2 2 0
> + 0 0 8 8 1 1 1 1 1 1
> + 1 1 1 1 1 1 0xE 0xE 0xE 0xE
> + 0xE 0xE 0xE>;
> +};
>
> /* CON5 on CP0 expansion */
> &cpm_pcie2 {
> @@ -78,6 +114,8 @@
> };
>
> &cpm_i2c0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&cpm_i2c0_pins>;
> status = "okay";
> clock-frequency = <100000>;
> };
> @@ -97,12 +135,31 @@
> status = "okay";
> };
>
> +&cps_pinctl {
> + /* MPP Bus:
> + [0-11] RGMII0
> + [13-16] SPI1
> + [27,31] GE_MDIO/MDC
> + [32-62] = 0xff: Keep default CP1_shared_pins:
> + */
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3
> + 0x3 0x3 0xff 0x3 0x3 0x3 0x3 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0x8 0xff 0xff
> + 0xff 0x8 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff>;
> +};
> +
> /* CON5 on CP1 expansion */
> &cps_pcie2 {
> status = "okay";
> };
>
> &cps_spi1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&cps_spi1_pins>;
> status = "okay";
>
> spi-flash at 0 {
> diff --git a/arch/arm/dts/armada-ap806.dtsi b/arch/arm/dts/armada-ap806.dtsi
> index d315b29..efb383b 100644
> --- a/arch/arm/dts/armada-ap806.dtsi
> +++ b/arch/arm/dts/armada-ap806.dtsi
> @@ -140,6 +140,24 @@
> marvell,spi-base = <128>, <136>, <144>, <152>;
> };
>
> + ap_pinctl: ap-pinctl at 6F4000 {
> + compatible = "marvell,armada-ap806-pinctrl";
> + bank-name ="apn-806";
> + reg = <0x6F4000 0x10>;
> + pin-count = <20>;
> + max-func = <3>;
> +
> + ap_i2c0_pins: i2c-pins-0 {
> + marvell,pins = < 4 5 >;
> + marvell,function = <3>;
So what are these marvell,pins/functions? They are not listed in the
DT bindings documentation.
> + };
> + ap_emmc_pins: emmc-pins-0 {
> + marvell,pins = < 0 1 2 3 4 5 6 7
> + 8 9 10 >;
> + marvell,function = <1>;
> + };
> + };
> +
> xor at 400000 {
> compatible = "marvell,mv-xor-v2";
> reg = <0x400000 0x1000>,
> diff --git a/arch/arm/dts/armada-cp110-master.dtsi b/arch/arm/dts/armada-cp110-master.dtsi
> index 422d754..d637867 100644
> --- a/arch/arm/dts/armada-cp110-master.dtsi
> +++ b/arch/arm/dts/armada-cp110-master.dtsi
> @@ -81,6 +81,38 @@
> "cpm-usb3dev", "cpm-eip150", "cpm-eip197";
> };
>
> + cpm_pinctl: cpm-pinctl at 440000 {
> + compatible = "marvell,mvebu-pinctrl",
> + "marvell,a70x0-pinctrl",
> + "marvell,a80x0-cp0-pinctrl";
> + bank-name ="cp0-110";
> + reg = <0x440000 0x20>;
> + pin-count = <63>;
> + max-func = <0xf>;
> +
> + cpm_i2c0_pins: cpm-i2c-pins-0 {
> + marvell,pins = < 37 38 >;
> + marvell,function = <2>;
> + };
> + cpm_ge2_rgmii_pins: cpm-ge-rgmii-pins-0 {
> + marvell,pins = < 44 45 46 47 48 49 50 51
> + 52 53 54 55 >;
> + marvell,function = <1>;
> + };
> + pca0_pins: cpm-pca0_pins {
> + marvell,pins = <62>;
> + marvell,function = <0>;
> + };
> + cpm_sdhci_pins: cpm-sdhi-pins-0 {
> + marvell,pins = < 56 57 58 59 60 61 >;
> + marvell,function = <14>;
> + };
> + cpm_spi0_pins: cpm-spi-pins-0 {
> + marvell,pins = < 13 14 15 16 >;
> + marvell,function = <3>;
> + };
> + };
> +
> cpm_sata0: sata at 540000 {
> compatible = "marvell,armada-8k-ahci";
> reg = <0x540000 0x30000>;
> diff --git a/arch/arm/dts/armada-cp110-slave.dtsi b/arch/arm/dts/armada-cp110-slave.dtsi
> index a7f77b9..92ef55c 100644
> --- a/arch/arm/dts/armada-cp110-slave.dtsi
> +++ b/arch/arm/dts/armada-cp110-slave.dtsi
> @@ -81,6 +81,25 @@
> "cps-usb3dev", "cps-eip150", "cps-eip197";
> };
>
> + cps_pinctl: cps-pinctl at 440000 {
> + compatible = "marvell,mvebu-pinctrl",
> + "marvell,a80x0-cp1-pinctrl";
> + bank-name ="cp1-110";
> + reg = <0x440000 0x20>;
> + pin-count = <63>;
> + max-func = <0xf>;
> +
> + cps_ge1_rgmii_pins: cps-ge-rgmii-pins-0 {
> + marvell,pins = < 0 1 2 3 4 5 6 7
> + 8 9 10 11 >;
> + marvell,function = <3>;
> + };
> + cps_spi1_pins: cps-spi-pins-1 {
> + marvell,pins = < 13 14 15 16 >;
> + marvell,function = <3>;
> + };
> + };
> +
> cps_sata0: sata at 540000 {
> compatible = "marvell,armada-8k-ahci";
> reg = <0x540000 0x30000>;
>
Thanks,
Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread* [U-Boot] [PATCH 4/6] arm64: mvebu: Add pin control nodes to A8K family DTS files
2016-11-24 9:13 ` Stefan Roese
@ 2016-11-24 14:09 ` Kostya Porotchkin
2016-11-24 16:00 ` Stefan Roese
0 siblings, 1 reply; 16+ messages in thread
From: Kostya Porotchkin @ 2016-11-24 14:09 UTC (permalink / raw)
To: u-boot
Hi, Stefan,
Thank you for your review!
I will put all required changes into second patch version.
Regarding the symbolic names for the pin controller functions and lack of documentation.
The problem is that same function number does not have the same meaning for different pins.
So if I want to put symbolic names instead of numbers, I have to add large structures defining symbolic names for each function on every pin as a platform data.
I think in this case I will loose the driver code compactness provided by the FDT usage.
I can create a documentation file with all pin function values taken from SoC HW manual and keep the numeric function IDs for the DTS usage.
Will it be good enough?
Best Regards
Konstantin
________________________________________
From: Stefan Roese <sr@denx.de>
Sent: Thursday, November 24, 2016 11:13
To: Kostya Porotchkin; u-boot at lists.denx.de
Cc: Nadav Haklai; Neta Zur Hershkovits; Omri Itach; Igal Liberman; Haim Boot; Hanna Hawa
Subject: Re: [PATCH 4/6] arm64: mvebu: Add pin control nodes to A8K family DTS files
Hi Kosta,
On 20.11.2016 16:38, kostap at marvell.com wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
>
> Add pin control nodes to APN806, CP-master, CP-slave and
> Armada-7040 and Armada-8040 boards DTS files
>
> Change-Id: I51522f33f23e3f9e94c6f5a5f9af19f5dc86e6b7
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Nadav Haklai <nadavh@marvell.com>
> Cc: Neta Zur Hershkovits <neta@marvell.com>
> Cc: Omri Itach <omrii@marvell.com>
> Cc: Igal Liberman <igall@marvell.com>
> Cc: Haim Boot <hayim@marvell.com>
> Cc: Hanna Hawa <hannah@marvell.com>
> ---
> arch/arm/dts/armada-7040-db.dts | 38 +++++++++++++++++++++++
> arch/arm/dts/armada-8040-db.dts | 57 +++++++++++++++++++++++++++++++++++
> arch/arm/dts/armada-ap806.dtsi | 18 +++++++++++
> arch/arm/dts/armada-cp110-master.dtsi | 32 ++++++++++++++++++++
> arch/arm/dts/armada-cp110-slave.dtsi | 19 ++++++++++++
> 5 files changed, 164 insertions(+)
>
> diff --git a/arch/arm/dts/armada-7040-db.dts b/arch/arm/dts/armada-7040-db.dts
> index b8fe5a9..1bfb5a9 100644
> --- a/arch/arm/dts/armada-7040-db.dts
> +++ b/arch/arm/dts/armada-7040-db.dts
> @@ -67,6 +67,8 @@
> };
>
> &i2c0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&cpm_i2c0_pins>;
> status = "okay";
> clock-frequency = <100000>;
> };
> @@ -98,6 +100,17 @@
> };
> };
>
> +&ap_pinctl {
> + /* MPP Bus:
> + SDIO [0-10]
> + UART0 [11,19]
> + */
Please use the common multiline comment instead:
/*
* MPP Bus:
* SDIO [0-10]
* UART0 [11,19]
*/
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 1 1 1 1 1 1 1 1 1 1
> + 1 3 0 0 0 0 0 0 0 3>;
Is there any chance to not use those numeric values but some macros
instead to make it clearer, which function is selected?
> + status = "okay";
> +};
> +
> &uart0 {
> status = "okay";
> };
> @@ -112,8 +125,33 @@
> clock-frequency = <100000>;
> };
>
> +&cpm_pinctl {
> + /* MPP Bus:
> + TDM [0-11]
> + SPI [13-16]
> + SATA1 [28]
> + UART0 [29-30]
> + SMI [32,34]
> + XSMI [35-36]
> + I2C [37-38]
> + RGMII1[44-55]
> + SD [56-62]
> + */
Again, comment style please.
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 4 4 4 4 4 4 4 4 4 4
> + 4 4 0 3 3 3 3 0 0 0
> + 0 0 0 0 0 0 0 0 9 0xA
> + 0xA 0 7 0 7 7 7 2 2 0
> + 0 0 0 0 1 1 1 1 1 1
> + 1 1 1 1 1 1 0xE 0xE 0xE 0xE
> + 0xE 0xE 0xE>;
Lower case hex values please (globally).
> + status = "okay";
> +};
> +
> &cpm_spi1 {
> status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&cpm_spi0_pins>;
>
> spi-flash at 0 {
> #address-cells = <0x1>;
> diff --git a/arch/arm/dts/armada-8040-db.dts b/arch/arm/dts/armada-8040-db.dts
> index 86666a1..30fd364 100644
> --- a/arch/arm/dts/armada-8040-db.dts
> +++ b/arch/arm/dts/armada-8040-db.dts
> @@ -71,6 +71,42 @@
> status = "okay";
> };
>
> +&ap_pinctl {
> + /* MPP Bus:
> + SPI0 [0-3]
> + I2C0 [4-5]
> + UART0 [11,19]
> + */
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 3 3 3 3 3 3 0 0 0 0
> + 0 3 0 0 0 0 0 0 0 3>;
> +};
> +
> +&cpm_pinctl {
> + /* MPP Bus:
> + [0-31] = 0xff: Keep default CP0_shared_pins:
> + [11] CLKOUT_MPP_11 (out)
> + [23] LINK_RD_IN_CP2CP (in)
> + [25] CLKOUT_MPP_25 (out)
> + [29] AVS_FB_IN_CP2CP (in)
> + [32,34] SMI
> + [31] GPIO: push button/Wake
> + [35-36] GPIO
> + [37-38] I2C
> + [40-41] SATA[0/1]_PRESENT_ACTIVEn
> + [42-43] XSMI
> + [44-55] RGMII1
> + [56-62] SD
> + */
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0 7 0 7 0 0 2 2 0
> + 0 0 8 8 1 1 1 1 1 1
> + 1 1 1 1 1 1 0xE 0xE 0xE 0xE
> + 0xE 0xE 0xE>;
> +};
>
> /* CON5 on CP0 expansion */
> &cpm_pcie2 {
> @@ -78,6 +114,8 @@
> };
>
> &cpm_i2c0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&cpm_i2c0_pins>;
> status = "okay";
> clock-frequency = <100000>;
> };
> @@ -97,12 +135,31 @@
> status = "okay";
> };
>
> +&cps_pinctl {
> + /* MPP Bus:
> + [0-11] RGMII0
> + [13-16] SPI1
> + [27,31] GE_MDIO/MDC
> + [32-62] = 0xff: Keep default CP1_shared_pins:
> + */
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3
> + 0x3 0x3 0xff 0x3 0x3 0x3 0x3 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0x8 0xff 0xff
> + 0xff 0x8 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff>;
> +};
> +
> /* CON5 on CP1 expansion */
> &cps_pcie2 {
> status = "okay";
> };
>
> &cps_spi1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&cps_spi1_pins>;
> status = "okay";
>
> spi-flash at 0 {
> diff --git a/arch/arm/dts/armada-ap806.dtsi b/arch/arm/dts/armada-ap806.dtsi
> index d315b29..efb383b 100644
> --- a/arch/arm/dts/armada-ap806.dtsi
> +++ b/arch/arm/dts/armada-ap806.dtsi
> @@ -140,6 +140,24 @@
> marvell,spi-base = <128>, <136>, <144>, <152>;
> };
>
> + ap_pinctl: ap-pinctl at 6F4000 {
> + compatible = "marvell,armada-ap806-pinctrl";
> + bank-name ="apn-806";
> + reg = <0x6F4000 0x10>;
> + pin-count = <20>;
> + max-func = <3>;
> +
> + ap_i2c0_pins: i2c-pins-0 {
> + marvell,pins = < 4 5 >;
> + marvell,function = <3>;
So what are these marvell,pins/functions? They are not listed in the
DT bindings documentation.
> + };
> + ap_emmc_pins: emmc-pins-0 {
> + marvell,pins = < 0 1 2 3 4 5 6 7
> + 8 9 10 >;
> + marvell,function = <1>;
> + };
> + };
> +
> xor at 400000 {
> compatible = "marvell,mv-xor-v2";
> reg = <0x400000 0x1000>,
> diff --git a/arch/arm/dts/armada-cp110-master.dtsi b/arch/arm/dts/armada-cp110-master.dtsi
> index 422d754..d637867 100644
> --- a/arch/arm/dts/armada-cp110-master.dtsi
> +++ b/arch/arm/dts/armada-cp110-master.dtsi
> @@ -81,6 +81,38 @@
> "cpm-usb3dev", "cpm-eip150", "cpm-eip197";
> };
>
> + cpm_pinctl: cpm-pinctl at 440000 {
> + compatible = "marvell,mvebu-pinctrl",
> + "marvell,a70x0-pinctrl",
> + "marvell,a80x0-cp0-pinctrl";
> + bank-name ="cp0-110";
> + reg = <0x440000 0x20>;
> + pin-count = <63>;
> + max-func = <0xf>;
> +
> + cpm_i2c0_pins: cpm-i2c-pins-0 {
> + marvell,pins = < 37 38 >;
> + marvell,function = <2>;
> + };
> + cpm_ge2_rgmii_pins: cpm-ge-rgmii-pins-0 {
> + marvell,pins = < 44 45 46 47 48 49 50 51
> + 52 53 54 55 >;
> + marvell,function = <1>;
> + };
> + pca0_pins: cpm-pca0_pins {
> + marvell,pins = <62>;
> + marvell,function = <0>;
> + };
> + cpm_sdhci_pins: cpm-sdhi-pins-0 {
> + marvell,pins = < 56 57 58 59 60 61 >;
> + marvell,function = <14>;
> + };
> + cpm_spi0_pins: cpm-spi-pins-0 {
> + marvell,pins = < 13 14 15 16 >;
> + marvell,function = <3>;
> + };
> + };
> +
> cpm_sata0: sata at 540000 {
> compatible = "marvell,armada-8k-ahci";
> reg = <0x540000 0x30000>;
> diff --git a/arch/arm/dts/armada-cp110-slave.dtsi b/arch/arm/dts/armada-cp110-slave.dtsi
> index a7f77b9..92ef55c 100644
> --- a/arch/arm/dts/armada-cp110-slave.dtsi
> +++ b/arch/arm/dts/armada-cp110-slave.dtsi
> @@ -81,6 +81,25 @@
> "cps-usb3dev", "cps-eip150", "cps-eip197";
> };
>
> + cps_pinctl: cps-pinctl at 440000 {
> + compatible = "marvell,mvebu-pinctrl",
> + "marvell,a80x0-cp1-pinctrl";
> + bank-name ="cp1-110";
> + reg = <0x440000 0x20>;
> + pin-count = <63>;
> + max-func = <0xf>;
> +
> + cps_ge1_rgmii_pins: cps-ge-rgmii-pins-0 {
> + marvell,pins = < 0 1 2 3 4 5 6 7
> + 8 9 10 11 >;
> + marvell,function = <3>;
> + };
> + cps_spi1_pins: cps-spi-pins-1 {
> + marvell,pins = < 13 14 15 16 >;
> + marvell,function = <3>;
> + };
> + };
> +
> cps_sata0: sata at 540000 {
> compatible = "marvell,armada-8k-ahci";
> reg = <0x540000 0x30000>;
>
Thanks,
Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread* [U-Boot] [PATCH 4/6] arm64: mvebu: Add pin control nodes to A8K family DTS files
2016-11-24 14:09 ` Kostya Porotchkin
@ 2016-11-24 16:00 ` Stefan Roese
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Roese @ 2016-11-24 16:00 UTC (permalink / raw)
To: u-boot
Hi Kosta,
On 24.11.2016 15:09, Kostya Porotchkin wrote:
> Thank you for your review!
> I will put all required changes into second patch version.
Thanks.
> Regarding the symbolic names for the pin controller functions
> and lack of documentation.
> The problem is that same function number does not have the
> same meaning for different pins.
> So if I want to put symbolic names instead of numbers, I have
> to add large structures defining symbolic names for each
> function on every pin as a platform data.
> I think in this case I will loose the driver code compactness
> provided by the FDT usage.
I suspected that something like this might be the reason for
the "plain numbers". But I also suspect that other SoCs might
suffer from the same problem. Did you take a look at other
pinctrl implementation (not only in U-Boot but also in Linux).
How is this solved for other SoCs (if this problem exists there
as well)?
> I can create a documentation file with all pin function values
> taken from SoC HW manual and keep the numeric function IDs for
> the DTS usage.
Is this something that you will create manually? Or can this
be created automatically from some documentation of internal
source? I'm asking, since manual creation always has the
potential problem of erroneous values.
> Will it be good enough?
This will help of course.
Thanks,
Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH 5/6] arm64: mvebu: Enable BUBT command support in A8K default config
2016-11-20 15:38 [U-Boot] [PATCH 0/6] arm64: mvebu: Armada-8K family patches kostap at marvell.com
` (3 preceding siblings ...)
2016-11-20 15:38 ` [U-Boot] [PATCH 4/6] arm64: mvebu: Add pin control nodes to A8K family DTS files kostap at marvell.com
@ 2016-11-20 15:38 ` kostap at marvell.com
2016-11-20 15:38 ` [U-Boot] [PATCH 6/6] arm64: mvebu: Enable pin control " kostap at marvell.com
5 siblings, 0 replies; 16+ messages in thread
From: kostap at marvell.com @ 2016-11-20 15:38 UTC (permalink / raw)
To: u-boot
From: Konstantin Porotchkin <kostap@marvell.com>
Enable mvebu "bubt" command support in the default configuration
file for Armada-7040 and Armada-8040 development boards
Change-Id: I9dba917baa68fc1e14007e66fda5d22d64bc94c1
Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
Cc: Stefan Roese <sr@denx.de>
Cc: Nadav Haklai <nadavh@marvell.com>
Cc: Neta Zur Hershkovits <neta@marvell.com>
Cc: Omri Itach <omrii@marvell.com>
Cc: Igal Liberman <igall@marvell.com>
Cc: Haim Boot <hayim@marvell.com>
Cc: Hanna Hawa <hannah@marvell.com>
---
configs/mvebu_db-88f7040_defconfig | 1 +
configs/mvebu_db-88f8040_defconfig | 1 +
2 files changed, 2 insertions(+)
diff --git a/configs/mvebu_db-88f7040_defconfig b/configs/mvebu_db-88f7040_defconfig
index f153b9c..ed61f78 100644
--- a/configs/mvebu_db-88f7040_defconfig
+++ b/configs/mvebu_db-88f7040_defconfig
@@ -27,6 +27,7 @@ CONFIG_CMD_EXT4=y
CONFIG_CMD_EXT4_WRITE=y
CONFIG_CMD_FAT=y
CONFIG_CMD_FS_GENERIC=y
+CONFIG_CMD_MVEBU_BUBT=y
CONFIG_BLOCK_CACHE=y
CONFIG_DM_I2C=y
CONFIG_SYS_I2C_MVTWSI=y
diff --git a/configs/mvebu_db-88f8040_defconfig b/configs/mvebu_db-88f8040_defconfig
index 61d58b5..fa2f597 100644
--- a/configs/mvebu_db-88f8040_defconfig
+++ b/configs/mvebu_db-88f8040_defconfig
@@ -27,6 +27,7 @@ CONFIG_CMD_EXT4=y
CONFIG_CMD_EXT4_WRITE=y
CONFIG_CMD_FAT=y
CONFIG_CMD_FS_GENERIC=y
+CONFIG_CMD_MVEBU_BUBT=y
CONFIG_BLOCK_CACHE=y
CONFIG_DM_I2C=y
CONFIG_SYS_I2C_MVTWSI=y
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [U-Boot] [PATCH 6/6] arm64: mvebu: Enable pin control support in A8K default config
2016-11-20 15:38 [U-Boot] [PATCH 0/6] arm64: mvebu: Armada-8K family patches kostap at marvell.com
` (4 preceding siblings ...)
2016-11-20 15:38 ` [U-Boot] [PATCH 5/6] arm64: mvebu: Enable BUBT command support in A8K default config kostap at marvell.com
@ 2016-11-20 15:38 ` kostap at marvell.com
5 siblings, 0 replies; 16+ messages in thread
From: kostap at marvell.com @ 2016-11-20 15:38 UTC (permalink / raw)
To: u-boot
From: Konstantin Porotchkin <kostap@marvell.com>
Enable mvebu pin control support in the default configuration
files for Armada-7040 and Armada-8040 development boards
Change-Id: Icc133c97c6f9fea374dd26ea5ab3f65fd66cc796
Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
Cc: Stefan Roese <sr@denx.de>
Cc: Nadav Haklai <nadavh@marvell.com>
Cc: Neta Zur Hershkovits <neta@marvell.com>
Cc: Omri Itach <omrii@marvell.com>
Cc: Igal Liberman <igall@marvell.com>
Cc: Haim Boot <hayim@marvell.com>
Cc: Hanna Hawa <hannah@marvell.com>
---
configs/mvebu_db-88f7040_defconfig | 1 +
configs/mvebu_db-88f8040_defconfig | 1 +
2 files changed, 2 insertions(+)
diff --git a/configs/mvebu_db-88f7040_defconfig b/configs/mvebu_db-88f7040_defconfig
index ed61f78..e3bdda6 100644
--- a/configs/mvebu_db-88f7040_defconfig
+++ b/configs/mvebu_db-88f7040_defconfig
@@ -51,3 +51,4 @@ CONFIG_USB_XHCI_HCD=y
CONFIG_USB_EHCI_HCD=y
CONFIG_USB_STORAGE=y
CONFIG_SMBIOS_MANUFACTURER=""
+CONFIG_PINCTRL=y
diff --git a/configs/mvebu_db-88f8040_defconfig b/configs/mvebu_db-88f8040_defconfig
index fa2f597..5d5be64 100644
--- a/configs/mvebu_db-88f8040_defconfig
+++ b/configs/mvebu_db-88f8040_defconfig
@@ -54,3 +54,4 @@ CONFIG_USB_XHCI_HCD=y
CONFIG_USB_EHCI_HCD=y
CONFIG_USB_STORAGE=y
CONFIG_SMBIOS_MANUFACTURER=""
+CONFIG_PINCTRL=y
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread