Re: [U-Boot] [RFC PATCH 1/2] common: Add new clk command

[Simon Glass]

Hi Wolfgang,

On 29 December 2015 at 07:23, Wolfgang Denk  wrote:
> Dear Simon,
>
> In message 
>  you 
> wrote:
>>
>> >>>  create mode 100644 common/cmd_clk.c
>> >>>  create mode 100644 include/clk.h
> ...
>> 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [RFC PATCH 1/2] common: Add new clk command

[Michal Simek]

Hi Simon,

On 22.12.2015 05:21, Simon Glass wrote:
> Hi Michal,
> 
> On 22 January 2014 at 04:02, Michal Simek  wrote:
>> Command provides just dump subcommand for showing clock
>> frequencies in a soc.
>>
>> Signed-off-by: Michal Simek 
>> ---
>>
>>  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
>>
>> diff --git a/README b/README
>> index aea82be..0087649 100644
>> --- 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_CONSOLEconinfo
>> 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 000..6d3d46a
>> --- /dev/null
>> +++ b/common/cmd_clk.c
>> @@ -0,0 +1,51 @@
>> +/*
>> + * Copyright (C) 2013 Xilinx, Inc.
>> + *
>> + * SPDX-License-Identifier:GPL-2.0+
>> + */
>> +#include 
>> +#include 
>> +#include 
>> +
>> +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.

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

Cheers,
Michal


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [RFC PATCH 1/2] common: Add new clk command

[Simon Glass]

Hi Michal,

On 29 December 2015 at 01:33, Michal Simek  wrote:
> Hi Simon,
>
> On 22.12.2015 05:21, Simon Glass wrote:
>> Hi Michal,
>>
>> On 22 January 2014 at 04:02, Michal Simek  wrote:
>>> Command provides just dump subcommand for showing clock
>>> frequencies in a soc.
>>>
>>> Signed-off-by: Michal Simek 
>>> ---
>>>
>>>  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
>>>
>>> diff --git a/README b/README
>>> index aea82be..0087649 100644
>>> --- 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_CONSOLEconinfo
>>> 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 000..6d3d46a
>>> --- /dev/null
>>> +++ b/common/cmd_clk.c
>>> @@ -0,0 +1,51 @@
>>> +/*
>>> + * Copyright (C) 2013 Xilinx, Inc.
>>> + *
>>> + * SPDX-License-Identifier:GPL-2.0+
>>> + */
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +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.
>
> 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [RFC PATCH 1/2] common: Add new clk command

[Wolfgang Denk]

Dear Simon,

In message  
you wrote:
> 
> >>>  create mode 100644 common/cmd_clk.c
> >>>  create mode 100644 include/clk.h
...
> 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

-- 
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: w...@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 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [RFC PATCH 1/2] common: Add new clk command

[Simon Glass]

Hi Michal,

On 22 January 2014 at 04:02, Michal Simek  wrote:
> Command provides just dump subcommand for showing clock
> frequencies in a soc.
>
> Signed-off-by: Michal Simek 
> ---
>
>  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
>
> diff --git a/README b/README
> index aea82be..0087649 100644
> --- 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_CONSOLEconinfo
> 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 000..6d3d46a
> --- /dev/null
> +++ b/common/cmd_clk.c
> @@ -0,0 +1,51 @@
> +/*
> + * Copyright (C) 2013 Xilinx, Inc.
> + *
> + * SPDX-License-Identifier:GPL-2.0+
> + */
> +#include 
> +#include 
> +#include 
> +
> +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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [RFC PATCH 1/2] common: Add new clk command

[Stefano Babic]

Hi Michal,

On 30/01/2014 08:38, Michal Simek wrote:
 On 01/23/2014 08:53 AM, Michal Simek wrote:
 On 01/22/2014 08:44 PM, Gerhard Sittig wrote:
 On Wed, Jan 22, 2014 at 12:02 +0100, Michal Simek wrote:
 
 --- /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.
 
 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.

 Stefano: Can you give me your ACK?
 

Sure, go on. !


Regards,
Stefano

-- 
=
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: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [RFC PATCH 1/2] common: Add new clk command

[Michal Simek]

On 01/23/2014 08:53 AM, Michal Simek wrote:
 On 01/22/2014 08:44 PM, Gerhard Sittig wrote:
 On Wed, Jan 22, 2014 at 12:02 +0100, Michal Simek wrote:

 --- /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.
 
 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


-- 
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




signature.asc
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [RFC PATCH 1/2] common: Add new clk command

[Michal Simek]

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

Signed-off-by: Michal Simek michal.si...@xilinx.com
---

 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

diff --git a/README b/README
index aea82be..0087649 100644
--- 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_CONSOLEconinfo
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 000..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;
+}
+
+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, , ),
+};
+
+static int do_clk(cmd_tbl_t *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+   cmd_tbl_t *c;
+
+   if (argc  2)
+   return CMD_RET_USAGE;
+
+   /* Strip off leading 'clk' command argument */
+   argc--;
+   argv++;
+
+   c = find_cmd_tbl(argv[0], cmd_clk_sub[0], ARRAY_SIZE(cmd_clk_sub));
+
+   if (c)
+   return c-cmd(cmdtp, flag, argc, argv);
+   else
+   return CMD_RET_USAGE;
+}
+
+#ifdef CONFIG_SYS_LONGHELP
+static char clk_help_text[] =
+   dump - Print clock frequencies;
+#endif
+
+U_BOOT_CMD(clk, 2, 1, do_clk, CLK sub-system, clk_help_text);
diff --git a/include/clk.h b/include/clk.h
new file mode 100644
index 000..df4570c
--- /dev/null
+++ b/include/clk.h
@@ -0,0 +1,6 @@
+#ifndef _CLK_H_
+#define _CLK_H_
+
+int soc_clk_dump(void);
+
+#endif /* _CLK_H_ */
diff --git a/include/config_cmd_all.h b/include/config_cmd_all.h
index d847069..3e8983f 100644
--- a/include/config_cmd_all.h
+++ b/include/config_cmd_all.h
@@ -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



pgpGCp_vpnQkI.pgp
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [RFC PATCH 1/2] common: Add new clk command

[Stefano Babic]

Hi Michal,

On 22/01/2014 12:02, Michal Simek wrote:
 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.


 Signed-off-by: Michal Simek michal.si...@xilinx.com ---
 
 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
 
 diff --git a/README b/README index aea82be..0087649 100644 ---
 a/README +++ b/README @@ -887,6 +887,7 @@ The following options
 need to be configured: CONFIG_CMD_BSP * Board specific commands 
 CONFIG_CMD_BOOTDbootd 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 000..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; +} 
 + +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

-- 
=
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: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [RFC PATCH 1/2] common: Add new clk command

[Michal Simek]

Hi Stefano,

On 01/22/2014 01:46 PM, Stefano Babic wrote:
 Hi Michal,
 
 On 22/01/2014 12:02, Michal Simek wrote:
 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.


 
 Signed-off-by: Michal Simek michal.si...@xilinx.com ---

 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

 diff --git a/README b/README index aea82be..0087649 100644 ---
 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 000..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; +} 
 + +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

-- 
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




signature.asc
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [RFC PATCH 1/2] common: Add new clk command

[Gerhard Sittig]

On Wed, Jan 22, 2014 at 12:02 +0100, Michal Simek wrote:
 
 --- /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.


virtually yours
Gerhard Sittig
-- 
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: off...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [RFC PATCH 1/2] common: Add new clk command

[Michal Simek]

On 01/22/2014 08:44 PM, Gerhard Sittig wrote:
 On Wed, Jan 22, 2014 at 12:02 +0100, Michal Simek wrote:

 --- /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.

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

-- 
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




signature.asc
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot