common: Add new clk command

RFC PATCH New Michal Simek 6 participants 13 messages Jan 22, 2014 – Jan 5, 2016

Subsystems (2)

Arm (32-bit) Arm/Zynq architecture

lore

Share & tools

lore

Michal Simek

author

2014-01-22 11:02 UTC #0 lore raw

RFC PATCH 1/2 common: Add new clk command

Command provides just dump subcommand for showing clock frequencies in a soc.

Signed-off-by Michal Simek <[email protected]>

---

 README                   |  1 +
 common/Makefile          |  1 +
 common/cmd_clk.c         | 51 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/clk.h            |  6 ++++++
 include/config_cmd_all.h |  1 +
 5 files changed, 60 insertions(+)
 create mode 100644 common/cmd_clk.c
 create mode 100644 include/clk.h

README +1 -0

index aea82be..0087649 100644
@@ -887,6 +887,7 @@ The following options need to be configured:
 		CONFIG_CMD_BSP		* Board specific commands
 		CONFIG_CMD_BOOTD	  bootd
 		CONFIG_CMD_CACHE	* icache, dcache
+		CONFIG_CMD_CLK   	* clock command support
 		CONFIG_CMD_CONSOLE	  coninfo
 		CONFIG_CMD_CRC32	* crc32
 		CONFIG_CMD_DATE		* support for RTC, date/time...

common/Makefile +1 -0

index d12cba5..a000e7d 100644
@@ -59,6 +59,7 @@ obj-$(CONFIG_CMD_BOOTLDR) += cmd_bootldr.o
 obj-$(CONFIG_CMD_BOOTSTAGE) += cmd_bootstage.o
 obj-$(CONFIG_CMD_CACHE) += cmd_cache.o
 obj-$(CONFIG_CMD_CBFS) += cmd_cbfs.o
+obj-$(CONFIG_CMD_CLK) += cmd_clk.o
 obj-$(CONFIG_CMD_CONSOLE) += cmd_console.o
 obj-$(CONFIG_CMD_CPLBINFO) += cmd_cplbinfo.o
 obj-$(CONFIG_DATAFLASH_MMC_SELECT) += cmd_dataflash_mmc_mux.o

common/cmd_clk.c new file +51 -0

51 lines not on this page · raw message

include/clk.h new file +6 -0

new file mode 100644
index 0000000..df4570c
@@ -0,0 +1,6 @@
+#ifndef _CLK_H_
+#define _CLK_H_
+
+int soc_clk_dump(void);
+
+#endif /* _CLK_H_ */

include/config_cmd_all.h +1 -0

index d847069..3e8983f 100644
@@ -23,6 +23,7 @@
 #define CONFIG_CMD_BSP		/* Board Specific functions	*/
 #define CONFIG_CMD_CACHE	/* icache, dcache		*/
 #define CONFIG_CMD_CDP		/* Cisco Discovery Protocol	*/
+#define CONFIG_CMD_CLK		/* Clock support		*/
 #define CONFIG_CMD_CONSOLE	/* coninfo			*/
 #define CONFIG_CMD_DATE		/* support for RTC, date/time...*/
 #define CONFIG_CMD_DHCP		/* DHCP Support			*/

--
1.8.2.3

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140122/19b81fe1/attachment.pgp>

Michal Simek

author

2014-01-22 11:02 UTC #1 lore raw

RFC PATCH 2/2 zynq: Implement dump clock command

From: Soren Brinkmann <[email protected]>

Enable and implement dump clock command which shows soc frequencies.

Signed-off-by Soren Brinkmann <[email protected]>

Signed-off-by Michal Simek <[email protected]>

---
On the top of Zynq clk patches I have sent.

All paches available for testing here
git://git.denx.de/u-boot-microblaze.git
xnext/clk-cmd branch.

For example:
zynq-uboot> clk dump
clk frequency

    armpll          1333333320
    ddrpll          1066666656
     iopll           999999990
 cpu_6or4x           666666660
 cpu_3or2x           333333330
    cpu_2x           222222220
    cpu_1x           111111110
    ddr_2x           355555552
    ddr_3x           533333328
       dci            10158730
     lqspi           199999998
       smc            22222222
      pcap           199999998
      gem0            25000000
      gem1            16666667
     fclk0            50000000
     fclk1            50000000
     fclk2            50000000
     fclk3            50000000
     sdio0            50000000
     sdio1            50000000
     uart0            50000000
     uart1            50000000
      spi0            15873016
      spi1            15873016
 usb0_aper           111111110
 usb1_aper           111111110
 gem0_aper           111111110
 gem1_aper           111111110
sdio0_aper           111111110
sdio1_aper           111111110
 spi0_aper           111111110
 spi1_aper           111111110
 can0_aper           111111110
 can1_aper           111111110
 i2c0_aper           111111110
 i2c1_aper           111111110
uart0_aper           111111110
uart1_aper           111111110
 gpio_aper           111111110
lqspi_aper           111111110
  smc_aper           111111110

dbg_trc 66666666
dbg_apb 66666666

 arch/arm/cpu/armv7/zynq/clk.c | 21 +++++++++++++++++++++
 include/configs/zynq-common.h |  1 +
 2 files changed, 22 insertions(+)

arch/arm/cpu/armv7/zynq/clk.c +21 -0

index ea7a677..6a759a1 100644
@@ -6,6 +6,7 @@
  */
 #include <common.h>
 #include <errno.h>
+#include <clk.h>
 #include <asm/io.h>
 #include <asm/arch/hardware.h>
 #include <asm/arch/clk.h>

@@ -641,3 +642,23 @@ const char *zynq_clk_get_name(enum zynq_clk clk)
 {
 	return clks[clk].name;
 }
+
+/**
+ * soc_clk_dump() - Print clock frequencies
+ * Returns zero on success
+ *
+ * Implementation for the clk dump command.
+ */
+int soc_clk_dump(void)
+{
+	int i;
+
+	printf("clk\t\tfrequency\n");
+	for (i = 0; i < clk_max; i++) {
+		const char *name = zynq_clk_get_name(i);
+		if (name)
+			printf("%10s%20lu\n", name, zynq_clk_get_rate(i));
+	}
+
+	return 0;
+}

include/configs/zynq-common.h +1 -0

index 9666f88..52f1c66 100644
@@ -164,6 +164,7 @@
 #define CONFIG_BOARD_LATE_INIT
 #define CONFIG_SYS_LONGHELP
 #define CONFIG_CLOCKS
+#define CONFIG_CMD_CLK
 #define CONFIG_SYS_MAXARGS		15 /* max number of command args */
 #define CONFIG_SYS_CBSIZE		256 /* Console I/O Buffer Size */
 #define CONFIG_SYS_PBSIZE		(CONFIG_SYS_CBSIZE + \

--
1.8.2.3

Stefano Babic

2014-01-22 12:46 UTC #2 lore raw

RFC PATCH 1/2 common: Add new clk command

In reply to Michal Simek

Hi Michal,

Quoted 3 lines · Michal Simek

Command provides just dump subcommand for showing clock frequencies in a soc.

i.MXes has already an own command for this functionality - see command "clocks" in arch/arm. However, I like that we can have a common command for all SOCs.

Quoted 26 lines

Signed-off-by Michal Simek <[email protected]> ---
README | 1 + common/Makefile | 1 +
common/cmd_clk.c | 51
++++++++++++++++++++++++++++++++++++++++++++++++ include/clk.h | 6 ++++++ include/config_cmd_all.h | 1 + 5 files changed, 60 insertions(+) create mode 100644 common/cmd_clk.c create mode 100644 include/clk.h
README index aea82be..0087649 100644 --- renamed from README +0 -0
a/README +++ b/README @@ -887,6 +887,7 @@ The following options
need to be configured: CONFIG_CMD_BSP * Board specific commands
CONFIG_CMD_BOOTD bootd CONFIG_CMD_CACHE * icache, dcache +
CONFIG_CMD_CLK * clock command support CONFIG_CMD_CONSOLE
coninfo CONFIG_CMD_CRC32 * crc32 CONFIG_CMD_DATE * support for
RTC, date/time... diff --git a/common/Makefile b/common/Makefile index d12cba5..a000e7d 100644 --- a/common/Makefile +++ b/common/Makefile @@ -59,6 +59,7 @@ obj-$(CONFIG_CMD_BOOTLDR) += cmd_bootldr.o obj-$(CONFIG_CMD_BOOTSTAGE) += cmd_bootstage.o obj-$(CONFIG_CMD_CACHE) += cmd_cache.o obj-$(CONFIG_CMD_CBFS) += cmd_cbfs.o +obj-$(CONFIG_CMD_CLK) += cmd_clk.o obj-$(CONFIG_CMD_CONSOLE) += cmd_console.o obj-$(CONFIG_CMD_CPLBINFO) += cmd_cplbinfo.o obj-$(CONFIG_DATAFLASH_MMC_SELECT) += cmd_dataflash_mmc_mux.o diff --git a/common/cmd_clk.c b/common/cmd_clk.c new file mode 100644 index 0000000..6d3d46a --- /dev/null +++ b/common/cmd_clk.c @@ -0,0

Quoted 8 lines

+1,51 @@ +/* + * Copyright (C) 2013 Xilinx, Inc. + * + *
SPDX-License-Identifier: GPL-2.0+ + */ +#include <common.h>
+#include <command.h> +#include <clk.h> + +int __weak
soc_clk_dump(void) +{ + puts("Not implemented\n"); + return 1; +}
+ +static int do_clk_dump(cmd_tbl_t *cmdtp, int flag, int argc, +
char *const argv[]) +{ + return soc_clk_dump(); +} + +static
cmd_tbl_t cmd_clk_sub[] = { + U_BOOT_CMD_MKENT(dump, 1, 1,
do_clk_dump, "", ""), +}; +

Do you plan to extend the list with new functionalities ? IMHO we should do it when we really need, that is when we will need at least a second subcommand.

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

Michal Simek

2014-01-22 13:07 UTC #3 lore raw

RFC PATCH 1/2 common: Add new clk command

Hi Stefano,

Quoted 10 lines · Stefano Babic

Hi Michal,
Quoted 3 lines · Michal Simek
Command provides just dump subcommand for showing clock frequencies in a soc.
i.MXes has already an own command for this functionality - see command "clocks" in arch/arm. However, I like that we can have a common command for all SOCs.

Ah good to know. It seems to me better to have clock like command name and then subcommand. Unification will be good to have.

Quoted 31 lines

Quoted 31 lines
Signed-off-by Michal Simek <[email protected]> ---
README | 1 + common/Makefile | 1 +
common/cmd_clk.c | 51
++++++++++++++++++++++++++++++++++++++++++++++++ include/clk.h | 6 ++++++ include/config_cmd_all.h | 1 + 5 files changed, 60 insertions(+) create mode 100644 common/cmd_clk.c create mode 100644 include/clk.h
README index aea82be..0087649 100644 --- renamed from README +0 -0
a/README +++ b/README @@ -887,6 +887,7 @@ The following options
need to be configured: CONFIG_CMD_BSP * Board specific commands
CONFIG_CMD_BOOTD bootd CONFIG_CMD_CACHE * icache, dcache +
CONFIG_CMD_CLK * clock command support CONFIG_CMD_CONSOLE
coninfo CONFIG_CMD_CRC32 * crc32 CONFIG_CMD_DATE * support for
RTC, date/time... diff --git a/common/Makefile b/common/Makefile index d12cba5..a000e7d 100644 --- a/common/Makefile +++ b/common/Makefile @@ -59,6 +59,7 @@ obj-$(CONFIG_CMD_BOOTLDR) += cmd_bootldr.o obj-$(CONFIG_CMD_BOOTSTAGE) += cmd_bootstage.o obj-$(CONFIG_CMD_CACHE) += cmd_cache.o obj-$(CONFIG_CMD_CBFS) += cmd_cbfs.o +obj-$(CONFIG_CMD_CLK) += cmd_clk.o obj-$(CONFIG_CMD_CONSOLE) += cmd_console.o obj-$(CONFIG_CMD_CPLBINFO) += cmd_cplbinfo.o obj-$(CONFIG_DATAFLASH_MMC_SELECT) += cmd_dataflash_mmc_mux.o diff --git a/common/cmd_clk.c b/common/cmd_clk.c new file mode 100644 index 0000000..6d3d46a --- /dev/null +++ b/common/cmd_clk.c @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2013 Xilinx, Inc. + * + *
SPDX-License-Identifier: GPL-2.0+ + */ +#include <common.h>
+#include <command.h> +#include <clk.h> + +int __weak
soc_clk_dump(void) +{ + puts("Not implemented\n"); + return 1; +}

Quoted 8 lines

Quoted 4 lines
+ +static int do_clk_dump(cmd_tbl_t *cmdtp, int flag, int argc, +
char *const argv[]) +{ + return soc_clk_dump(); +} + +static
cmd_tbl_t cmd_clk_sub[] = { + U_BOOT_CMD_MKENT(dump, 1, 1,
do_clk_dump, "", ""), +}; +
Do you plan to extend the list with new functionalities ? IMHO we should do it when we really need, that is when we will need at least a second subcommand.

I tried to write it as generic as possible and currently we don't need any special clock functionality but it can happen in future.

Will be great if we can add all the clock handling to the one command, Currently we just use clock dump.

Thanks,
Michal

Signature · 14 lines

Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140122/436060aa/attachment.pgp>

Gerhard Sittig

2014-01-22 19:44 UTC #4 lore raw

RFC PATCH 1/2 common: Add new clk command

In reply to Michal Simek

Quoted 14 lines · Michal Simek

common/cmd_clk.c new file +10 -0

@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2013 Xilinx, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+#include <common.h>
+#include <command.h>
+#include <clk.h>
+
+int __weak soc_clk_dump(void)

common/cmd_clk.c line 20

Quoted 10 lines

common/cmd_clk.c new file +10 -0

+{
+	puts("Not implemented\n");
+	return 1;
+}
+
+static int do_clk_dump(cmd_tbl_t *cmdtp, int flag, int argc,
+		       char *const argv[])
+{
+	return soc_clk_dump();
+}

Is there a specific reason to not pass on the remaining (not yet consumed) command line arguments? Future implementations may want to take a clock item's name, or a clock group's name, or options related to the format or verbosity of the dump, et al.

virtually yours
Gerhard Sittig

Signature · 3 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de

Michal Simek

2014-01-23 07:53 UTC #5 lore raw

RFC PATCH 1/2 common: Add new clk command

Quoted 22 lines · Gerhard Sittig

Quoted 21 lines · Michal Simek

common/cmd_clk.c new file +17 -0

@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2013 Xilinx, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+#include <common.h>
+#include <command.h>
+#include <clk.h>
+
+int __weak soc_clk_dump(void)
+{
+	puts("Not implemented\n");
+	return 1;
+}
+
+static int do_clk_dump(cmd_tbl_t *cmdtp, int flag, int argc,
+		       char *const argv[])

Quoted 8 lines

common/cmd_clk.c line 20
Quoted 3 lines
common/cmd_clk.c new file +3 -0
+{
+	return soc_clk_dump();
+}
Is there a specific reason to not pass on the remaining (not yet consumed) command line arguments? Future implementations may want to take a clock item's name, or a clock group's name, or options related to the format or verbosity of the dump, et al.

Only one reason is that I don't need it for my zynq implementation. If this is necessary there is no problem to pass them because it is internal API. Also I prefer to pass just arguments which I need.

I have looked at i.MXes cases and they do in general what my zynq implementation. They can just include clk.h and change do_mx6_showclocks to soc_clk_dump.

Thanks,
Michal

Signature · 14 lines

Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140123/64de0add/attachment.pgp>

Michal Simek

2014-01-30 07:38 UTC #6 lore raw

RFC PATCH 1/2 common: Add new clk command

Quoted 32 lines · Michal Simek

Quoted 22 lines · Gerhard Sittig
Quoted 21 lines · Michal Simek
common/cmd_clk.c new file +17 -0
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2013 Xilinx, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+#include <common.h>
+#include <command.h>
+#include <clk.h>
+
+int __weak soc_clk_dump(void)
+{
+	puts("Not implemented\n");
+	return 1;
+}
+
+static int do_clk_dump(cmd_tbl_t *cmdtp, int flag, int argc,
+		       char *const argv[])
Quoted 8 lines
common/cmd_clk.c line 20
Quoted 3 lines
common/cmd_clk.c new file +3 -0
+{
+	return soc_clk_dump();
+}
Is there a specific reason to not pass on the remaining (not yet consumed) command line arguments? Future implementations may want to take a clock item's name, or a clock group's name, or options related to the format or verbosity of the dump, et al.

Quoted 8 lines

Only one reason is that I don't need it for my zynq implementation. If this is necessary there is no problem to pass them because it is internal API. Also I prefer to pass just arguments which I need.
I have looked at i.MXes cases and they do in general what my zynq implementation. They can just include clk.h and change do_mx6_showclocks to soc_clk_dump.

Stefano: Can you give me your ACK?

Tom: Do you have any problem with this new clk command?

Thanks,
Michal

Signature · 14 lines

Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140130/eaa74ce4/attachment.pgp>

Stefano Babic

2014-01-30 09:29 UTC #7 lore raw

RFC PATCH 1/2 common: Add new clk command

Hi Michal,

Quoted 19 lines · Michal Simek

Quoted 18 lines · Michal Simek
Quoted 16 lines · Gerhard Sittig
Quoted 9 lines · Michal Simek
--- /dev/null +++ b/common/cmd_clk.c @@ -0,0 +1,51 @@ +/* +
* Copyright (C) 2013 Xilinx, Inc. + * + *
SPDX-License-Identifier: GPL-2.0+ + */ +#include <common.h>
+#include <command.h> +#include <clk.h> + +int __weak
soc_clk_dump(void) +{ + puts("Not implemented\n"); + return
1; +} + +static int do_clk_dump(cmd_tbl_t *cmdtp, int flag,
int argc, + char *const argv[]) +{ + return
soc_clk_dump(); +}
Is there a specific reason to not pass on the remaining (not yet consumed) command line arguments? Future implementations may want to take a clock item's name, or a clock group's name, or options related to the format or verbosity of the dump, et al.

Quoted 9 lines

Quoted 8 lines
Only one reason is that I don't need it for my zynq implementation. If this is necessary there is no problem to pass them because it is internal API. Also I prefer to pass just arguments which I need.
I have looked at i.MXes cases and they do in general what my zynq implementation. They can just include clk.h and change do_mx6_showclocks to soc_clk_dump.

Agree.

Quoted 2 lines

Stefano: Can you give me your ACK?

Sure, go on. !

Regards,
Stefano

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

Simon Glass

2015-12-22 04:21 UTC #8 lore raw

RFC PATCH 1/2 common: Add new clk command

In reply to Michal Simek

Hi Michal,

Quoted 54 lines · Michal Simek

Command provides just dump subcommand for showing clock frequencies in a soc.

Signed-off-by Michal Simek <[email protected]>

---

 README                   |  1 +
 common/Makefile          |  1 +
 common/cmd_clk.c         | 51 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/clk.h            |  6 ++++++
 include/config_cmd_all.h |  1 +
 5 files changed, 60 insertions(+)
 create mode 100644 common/cmd_clk.c
 create mode 100644 include/clk.h

README +1 -0

index aea82be..0087649 100644
@@ -887,6 +887,7 @@ The following options need to be configured:
                CONFIG_CMD_BSP          * Board specific commands
                CONFIG_CMD_BOOTD          bootd
                CONFIG_CMD_CACHE        * icache, dcache
+               CONFIG_CMD_CLK          * clock command support
                CONFIG_CMD_CONSOLE        coninfo
                CONFIG_CMD_CRC32        * crc32
                CONFIG_CMD_DATE         * support for RTC, date/time...

common/Makefile +1 -0

index d12cba5..a000e7d 100644
@@ -59,6 +59,7 @@ obj-$(CONFIG_CMD_BOOTLDR) += cmd_bootldr.o
 obj-$(CONFIG_CMD_BOOTSTAGE) += cmd_bootstage.o
 obj-$(CONFIG_CMD_CACHE) += cmd_cache.o
 obj-$(CONFIG_CMD_CBFS) += cmd_cbfs.o
+obj-$(CONFIG_CMD_CLK) += cmd_clk.o
 obj-$(CONFIG_CMD_CONSOLE) += cmd_console.o
 obj-$(CONFIG_CMD_CPLBINFO) += cmd_cplbinfo.o
 obj-$(CONFIG_DATAFLASH_MMC_SELECT) += cmd_dataflash_mmc_mux.o

common/cmd_clk.c new file +9 -0

new file mode 100644
index 0000000..6d3d46a
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2013 Xilinx, Inc.
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+#include <common.h>
+#include <command.h>
+#include <clk.h>
+

common/cmd_clk.c line 19

Quoted 10 lines

common/cmd_clk.c new file +10 -0

+int __weak soc_clk_dump(void)
+{
+       puts("Not implemented\n");
+       return 1;
+}
+
+static int do_clk_dump(cmd_tbl_t *cmdtp, int flag, int argc,
+                      char *const argv[])
+{
+       return soc_clk_dump();

This is not the way things should work in driver model. See how the gpio command works for an example. I suggest it iterates through the available clocks and then calls a driver function to obtain information about each clock, then prints it out. We should avoid weak functions as a method of connecting things together.

[snip]

Regards,
Simon

Michal Simek

author

2015-12-29 08:33 UTC #9 lore raw

RFC PATCH 1/2 common: Add new clk command

Hi Simon,

Quoted 65 lines · Simon Glass

Hi Michal,

Quoted 62 lines · Michal Simek

Command provides just dump subcommand for showing clock frequencies in a soc.

Signed-off-by Michal Simek <[email protected]>

---

 README                   |  1 +
 common/Makefile          |  1 +
 common/cmd_clk.c         | 51 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/clk.h            |  6 ++++++
 include/config_cmd_all.h |  1 +
 5 files changed, 60 insertions(+)
 create mode 100644 common/cmd_clk.c
 create mode 100644 include/clk.h

README +1 -0

index aea82be..0087649 100644
@@ -887,6 +887,7 @@ The following options need to be configured:
                CONFIG_CMD_BSP          * Board specific commands
                CONFIG_CMD_BOOTD          bootd
                CONFIG_CMD_CACHE        * icache, dcache
+               CONFIG_CMD_CLK          * clock command support
                CONFIG_CMD_CONSOLE        coninfo
                CONFIG_CMD_CRC32        * crc32
                CONFIG_CMD_DATE         * support for RTC, date/time...

common/Makefile +1 -0

index d12cba5..a000e7d 100644
@@ -59,6 +59,7 @@ obj-$(CONFIG_CMD_BOOTLDR) += cmd_bootldr.o
 obj-$(CONFIG_CMD_BOOTSTAGE) += cmd_bootstage.o
 obj-$(CONFIG_CMD_CACHE) += cmd_cache.o
 obj-$(CONFIG_CMD_CBFS) += cmd_cbfs.o
+obj-$(CONFIG_CMD_CLK) += cmd_clk.o
 obj-$(CONFIG_CMD_CONSOLE) += cmd_console.o
 obj-$(CONFIG_CMD_CPLBINFO) += cmd_cplbinfo.o
 obj-$(CONFIG_DATAFLASH_MMC_SELECT) += cmd_dataflash_mmc_mux.o

common/cmd_clk.c new file +17 -0

new file mode 100644
index 0000000..6d3d46a
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2013 Xilinx, Inc.
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+#include <common.h>
+#include <command.h>
+#include <clk.h>
+
+int __weak soc_clk_dump(void)
+{
+       puts("Not implemented\n");
+       return 1;
+}
+
+static int do_clk_dump(cmd_tbl_t *cmdtp, int flag, int argc,
+                      char *const argv[])

Quoted 8 lines

common/cmd_clk.c line 19
Quoted 2 lines
common/cmd_clk.c new file +2 -0
+{
+       return soc_clk_dump();
This is not the way things should work in driver model. See how the gpio command works for an example. I suggest it iterates through the available clocks and then calls a driver function to obtain information about each clock, then prints it out. We should avoid weak functions as a method of connecting things together.

This is patch which was sent in 2014. What are you trying to suggest?

Cheers,
Michal

Simon Glass

2015-12-29 12:23 UTC #10 lore raw

RFC PATCH 1/2 common: Add new clk command

Hi Michal,

Quoted 77 lines · Michal Simek

Hi Simon,

Quoted 65 lines · Simon Glass

Hi Michal,

Quoted 62 lines · Michal Simek

Command provides just dump subcommand for showing clock frequencies in a soc.

Signed-off-by Michal Simek <[email protected]>

---

 README                   |  1 +
 common/Makefile          |  1 +
 common/cmd_clk.c         | 51 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/clk.h            |  6 ++++++
 include/config_cmd_all.h |  1 +
 5 files changed, 60 insertions(+)
 create mode 100644 common/cmd_clk.c
 create mode 100644 include/clk.h

README +1 -0

index aea82be..0087649 100644
@@ -887,6 +887,7 @@ The following options need to be configured:
                CONFIG_CMD_BSP          * Board specific commands
                CONFIG_CMD_BOOTD          bootd
                CONFIG_CMD_CACHE        * icache, dcache
+               CONFIG_CMD_CLK          * clock command support
                CONFIG_CMD_CONSOLE        coninfo
                CONFIG_CMD_CRC32        * crc32
                CONFIG_CMD_DATE         * support for RTC, date/time...

common/Makefile +1 -0

index d12cba5..a000e7d 100644
@@ -59,6 +59,7 @@ obj-$(CONFIG_CMD_BOOTLDR) += cmd_bootldr.o
 obj-$(CONFIG_CMD_BOOTSTAGE) += cmd_bootstage.o
 obj-$(CONFIG_CMD_CACHE) += cmd_cache.o
 obj-$(CONFIG_CMD_CBFS) += cmd_cbfs.o
+obj-$(CONFIG_CMD_CLK) += cmd_clk.o
 obj-$(CONFIG_CMD_CONSOLE) += cmd_console.o
 obj-$(CONFIG_CMD_CPLBINFO) += cmd_cplbinfo.o
 obj-$(CONFIG_DATAFLASH_MMC_SELECT) += cmd_dataflash_mmc_mux.o

common/cmd_clk.c new file +17 -0

new file mode 100644
index 0000000..6d3d46a
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2013 Xilinx, Inc.
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+#include <common.h>
+#include <command.h>
+#include <clk.h>
+
+int __weak soc_clk_dump(void)
+{
+       puts("Not implemented\n");
+       return 1;
+}
+
+static int do_clk_dump(cmd_tbl_t *cmdtp, int flag, int argc,
+                      char *const argv[])

Quoted 8 lines

common/cmd_clk.c line 19
Quoted 2 lines
common/cmd_clk.c new file +2 -0
+{
+       return soc_clk_dump();
This is not the way things should work in driver model. See how the gpio command works for an example. I suggest it iterates through the available clocks and then calls a driver function to obtain information about each clock, then prints it out. We should avoid weak functions as a method of connecting things together.

Quoted 1 line

This is patch which was sent in 2014. What are you trying to suggest?

Sorry, I only just noticed it. I'll see if I can create a patch to get the clock info from an API call instead of using a weak function.

Regards,
Simon

Wolfgang Denk

2015-12-29 14:23 UTC #11 lore raw

RFC PATCH 1/2 common: Add new clk command

Dear Simon,

Quoted 3 lines · In message <CAPnjgZ0AxfXOiBwrjEPgs_Kd2A1x7qZ_z9gn_QAiprUS+48E_Q@mail.gmail.com>…

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
 create mode 100644 common/cmd_clk.c
 create mode 100644 include/clk.h

...

Quoted 2 lines

Sorry, I only just noticed it. I'll see if I can create a patch to get the clock info from an API call instead of using a weak function.

I don't get it. This patch appears to be identical (?) to commit 08d0d6f: "common: Add new clk command" which was applied in Nov 2013.

Best regards,

Wolfgang Denk

Signature · 8 lines

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I see that Microsoft's campaign  to  destroy  all  knowledge  of  any
operating   environment   but  its  own  environment-of-the-year  has
succeeded in creating a generation of users who don't understand  the
concept of a shell...
            -- L. Peter Deutsch in <[email protected]>

Simon Glass

2016-01-05 00:57 UTC #12 lore raw

RFC PATCH 1/2 common: Add new clk command

Hi Wolfgang,

Quoted 4 lines · Wolfgang Denk

Dear Simon,
Quoted 1 line · In message <CAPnjgZ0AxfXOiBwrjEPgs_Kd2A1x7qZ_z9gn_QAiprUS+48E_Q@mail.gmail.com>…

Quoted 8 lines

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
 create mode 100644 common/cmd_clk.c
 create mode 100644 include/clk.h
...
Quoted 2 lines
Sorry, I only just noticed it. I'll see if I can create a patch to get the clock info from an API call instead of using a weak function.
I don't get it. This patch appears to be identical (?) to commit 08d0d6f: "common: Add new clk command" which was applied in Nov 2013.

Yes, I did not realise how old this patch was, sorry. It was applied long ago. I'm going to create a vendor-specific command which uses driver model for now. We can revisit this if it becomes a problem. My idea is that we should have a command which displays the current clocks, no matter what the board type. But that can come later.

Regards,
Simon