Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define
Hi Wolfgang,
On Sat, Oct 15, 2011 at 4:36 AM, Wolfgang Denk w...@denx.de wrote:
Dear Lei Wen,
In message
CALZhoSSygHZ22N=odn4qv44a_1zxgupqlsrwa3pbbpnxbxj...@mail.gmail.com you
wrote:
And both the index and value arguments are never used in I/O
context, i. e. they are actual plain integer parameters. So just keep
it as
err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1);
Should I also keep the EXT_CSD_HS_TIMING like macro for previous
ext_csd parsing?
Or just add another ext_csd structure definition to the parse the
ext_csd, while keep macros
to be called by mmc_switch?
Let's try to understand what we are trying to acchieve. The purpose
of using C structs instead of a notation of base address + register
offset is that this way the struct entries that represent the
registers now have types, and the compiler can perform proper type
checking when using these data in I/O accessors.
So we use structs to describe the memory map of the hardware, the
mapping of device registers to addresses, and the data types give
information about the width of the register (8, 16, 32, ... bit).
Note that all the time we are talking about device registers and I/O
operations - that is situations where I/O accessors will be used.
On the other hand, when it comes to definitions of bit fields, like
here:
/*
* EXT_CSD field definitions
*/
#define EXT_CSD_CMD_SET_NORMAL (1 0)
#define EXT_CSD_CMD_SET_SECURE (1 1)
#define EXT_CSD_CMD_SET_CPSECURE (1 2)
...
or of indexes, like here:
/*
* EXT_CSD fields
*/
#define EXT_CSD_PART_CONF 179 /* R/W */
#define EXT_CSD_BUS_WIDTH 183 /* R/W */
#define EXT_CSD_HS_TIMING 185 /* R/W */
...
..then a struct makes no sense - we have plain integer constants
here.
To verify, please have a look at the code of mmc_switch():
int mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value)
{
struct mmc_cmd cmd;
int timeout = 1000;
int ret;
cmd.cmdidx = MMC_CMD_SWITCH;
cmd.resp_type = MMC_RSP_R1b;
cmd.cmdarg = (MMC_SWITCH_MODE_WRITE_BYTE 24) |
(index 16) |
(value 8);
...
ret = mmc_send_cmd(mmc, cmd, NULL);
As you can see, the arguments are ORed together to form an argument
to mmc_send_cmd() - they are not used in a I/O accessor or any
similar function.
In short: neither EXT_CSD_CMD_SET_NORMAL nor EXT_CSD_HS_TIMING
describe a device register that is used in any form of I/O
operations, so it is OK to leave these as simple #define's; the use
of a struct would not make sense here.
Thanks for your kindly explaination on the c structure usage in UBOOT.
So should I keep the V2 version of this patch, or anything else need
to be modified?
Thanks,
Lei
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define
Dear Lei Wen, In message calzhostndr9u9ajqku8f8mx_9jdpbbrhjlocyrpzudcb63d...@mail.gmail.com you wrote: Thanks for your kindly explaination on the c structure usage in UBOOT. So should I keep the V2 version of this patch, or anything else need to be modified? From my point of view this is OK, so: Acked-by: WOlfgang Denk w...@denx.de Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de There's no honorable way to kill, no gentle way to destroy. There is nothing good in war. Except its ending. -- Abraham Lincoln, The Savage Curtain, stardate 5906.5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define
Dear Lei Wen,
In message CALZhoSSygHZ22N=odn4qv44a_1zxgupqlsrwa3pbbpnxbxj...@mail.gmail.com
you wrote:
And both the index and value arguments are never used in I/O
context, i. e. they are actual plain integer parameters. So just keep
it as
err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1);
Should I also keep the EXT_CSD_HS_TIMING like macro for previous
ext_csd parsing?
Or just add another ext_csd structure definition to the parse the
ext_csd, while keep macros
to be called by mmc_switch?
Let's try to understand what we are trying to acchieve. The purpose
of using C structs instead of a notation of base address + register
offset is that this way the struct entries that represent the
registers now have types, and the compiler can perform proper type
checking when using these data in I/O accessors.
So we use structs to describe the memory map of the hardware, the
mapping of device registers to addresses, and the data types give
information about the width of the register (8, 16, 32, ... bit).
Note that all the time we are talking about device registers and I/O
operations - that is situations where I/O accessors will be used.
On the other hand, when it comes to definitions of bit fields, like
here:
/*
* EXT_CSD field definitions
*/
#define EXT_CSD_CMD_SET_NORMAL (1 0)
#define EXT_CSD_CMD_SET_SECURE (1 1)
#define EXT_CSD_CMD_SET_CPSECURE(1 2)
...
or of indexes, like here:
/*
* EXT_CSD fields
*/
#define EXT_CSD_PART_CONF 179 /* R/W */
#define EXT_CSD_BUS_WIDTH 183 /* R/W */
#define EXT_CSD_HS_TIMING 185 /* R/W */
...
..then a struct makes no sense - we have plain integer constants
here.
To verify, please have a look at the code of mmc_switch():
int mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value)
{
struct mmc_cmd cmd;
int timeout = 1000;
int ret;
cmd.cmdidx = MMC_CMD_SWITCH;
cmd.resp_type = MMC_RSP_R1b;
cmd.cmdarg = (MMC_SWITCH_MODE_WRITE_BYTE 24) |
(index 16) |
(value 8);
...
ret = mmc_send_cmd(mmc, cmd, NULL);
As you can see, the arguments are ORed together to form an argument
to mmc_send_cmd() - they are not used in a I/O accessor or any
similar function.
In short: neither EXT_CSD_CMD_SET_NORMAL nor EXT_CSD_HS_TIMING
describe a device register that is used in any form of I/O
operations, so it is OK to leave these as simple #define's; the use
of a struct would not make sense here.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Life would be so much easier if everyone read the manual.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define
Dear Lei Wen, In message CALButC+bZZwYhE0VT99Yjh_=p0lvjqnmm1yvzsex3inutn7...@mail.gmail.com Graeme Russ wrote: So do you means I should keep the EXT_CSD_HS_TIMING? Or I change like below? from err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1); to: err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, ext_csd.hs_timing - ext_csd, 1); I imagine the compiler will complain that the types for ext_csd.hs_timing and ext_csd are different. Try: struct ext_csd *ext_csd_ptr = 0; err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, ext_csd_ptr-hs_timing, 1); Umm... no. The signature of mmc_switch() [as used here - there is also a two argument version; what a mess!] is: int mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value) The set argument is bous, as the function never uses it anywhere. And both the index and value arguments are never used in I/O context, i. e. they are actual plain integer parameters. So just keep it as err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1); Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de That Microsoft, the Trabant of the operating system world, may be glancing over the Berlin Wall at the Audis and BMWs and Mercedes. In their own universe Trabants and Ladas were mainstream too... -- Evan Leibovitch ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define
Hi Wolfgang, On Fri, Oct 14, 2011 at 4:09 AM, Wolfgang Denk w...@denx.de wrote: Dear Lei Wen, In message CALButC+bZZwYhE0VT99Yjh_=p0lvjqnmm1yvzsex3inutn7...@mail.gmail.com Graeme Russ wrote: So do you means I should keep the EXT_CSD_HS_TIMING? Or I change like below? from err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1); to: err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, ext_csd.hs_timing - ext_csd, 1); I imagine the compiler will complain that the types for ext_csd.hs_timing and ext_csd are different. Try: struct ext_csd *ext_csd_ptr = 0; err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, ext_csd_ptr-hs_timing, 1); Umm... no. The signature of mmc_switch() [as used here - there is also a two argument version; what a mess!] is: int mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value) The set argument is bous, as the function never uses it anywhere. And both the index and value arguments are never used in I/O context, i. e. they are actual plain integer parameters. So just keep it as err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1); Should I also keep the EXT_CSD_HS_TIMING like macro for previous ext_csd parsing? Or just add another ext_csd structure definition to the parse the ext_csd, while keep macros to be called by mmc_switch? Thanks, Lei ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define
Hi Graeme, On Tue, Oct 11, 2011 at 9:09 AM, Graeme Russ graeme.r...@gmail.com wrote: Hi Lei Wen, On Tue, Oct 11, 2011 at 11:48 AM, Lei Wen adrian.w...@gmail.com wrote: Hi Wolfgang, On Tue, Oct 11, 2011 at 1:33 AM, Wolfgang Denk w...@denx.de wrote: Dear Lei Wen, In message CALZhoSQbvKj0MtqryeHX-4LkvQJR2=b9u_m4yjjfm1mjv2m...@mail.gmail.com you wrote: So macro may looks more concise and could parse from its meaning easily eno= ugh. We do not accept (typeless) register offset definitions. Please use a struct, so the compiler has a chance to perform type checking. I check the code again, and find there is a reason for previous defined macro to use. That is those register offset defined as macro may need later passing as a function parameter like: err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1); I don;t see that any register (which now would be an address in a struct insteadd of being an offset to be added to a base address) would be passed as parameter to mmc_switch(). So this is not an issue at all. So if the ext_csd change to structure, maybe the function call here don't looks like so concise as before... What do you think for this? The EXT_CSD field definitions are completely independent of the way how you access the registers. So do you means I should keep the EXT_CSD_HS_TIMING? Or I change like below? from err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1); to: err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, ext_csd.hs_timing - ext_csd, 1); I imagine the compiler will complain that the types for ext_csd.hs_timing and ext_csd are different. Try: struct ext_csd *ext_csd_ptr = 0; err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, ext_csd_ptr-hs_timing, 1); Yes, it looks more clear than mine. If have no other suggestion, I would do like this to format my patches. Thanks, Lei ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define
On Mon, Oct 10, 2011 at 8:12 PM, Graeme Russ graeme.r...@gmail.com wrote:
Hi Prafulla,
Hi Lei,
this is better, but what about structure-based access ?
struct somrthing {
u8 a1;
u8 a2;
...
};
Like this.
Also, CC Andy.
The ext_csd current usage in mmc.c is not too much, here I mean only few
of
the fields of the ext_csd is used, also fully definition of ext_csd
member would seems so huge a structure at its appearence...
So macro may looks more concise and could parse from its meaning easily
enough.
Anyway, more comments on this welcomes. :)
Dear Lei
Using c-struct is our strategy, may be full definition is huge, you may skip
them inserting padding in the c-struct.
I personally prefer the complete struct to be defined from day one. This
means you don't have to patch it when you end up needing to use another
member later on and risk fouling up the offsets
One possible issue with defining a struct for the ext CSD is that the
512-byte structure changes depending on the version. This will require
great care to make sure the unions are all set up properly to cover
the various specs.
Linux uses a character array with index offsets, but parses the ext
csd into a software structure in the card object.
I don't object to using a struct, but we may find it's a bit
heavyweight. I do think it's ugly to pass in pointer math to
mmc_switch. Let's hide it with a macro:
#define mmc_ext_csd_offset(x) (((struct ext_csd *)0)-x)
Which would remake that:
err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, mmc_ext_csd_offset(hs_timing), 1);
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define
Hi Wolfgang, On Fri, Oct 7, 2011 at 1:39 AM, Wolfgang Denk w...@denx.de wrote: Dear Lei Wen, In message calzhosrhbf2vmu5olp3hwh4yq4xfip19ajd24gn4sy-rm6b...@mail.gmail.com you wrote: The ext_csd current usage in mmc.c is not too much, here I mean only few of the fields of the ext_csd is used, also fully definition of ext_csd member would seems so huge a structure at its appearence... So macro may looks more concise and could parse from its meaning easily eno= ugh. We do not accept (typeless) register offset definitions. Please use a struct, so the compiler has a chance to perform type checking. I check the code again, and find there is a reason for previous defined macro to use. That is those register offset defined as macro may need later passing as a function parameter like: err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1); So if the ext_csd change to structure, maybe the function call here don't looks like so concise as before... What do you think for this? Thanks, Lei ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define
Dear Lei Wen, In message CALZhoSQbvKj0MtqryeHX-4LkvQJR2=b9u_m4yjjfm1mjv2m...@mail.gmail.com you wrote: So macro may looks more concise and could parse from its meaning easily eno= ugh. We do not accept (typeless) register offset definitions. Please use a struct, so the compiler has a chance to perform type checking. I check the code again, and find there is a reason for previous defined macro to use. That is those register offset defined as macro may need later passing as a function parameter like: err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1); I don;t see that any register (which now would be an address in a struct insteadd of being an offset to be added to a base address) would be passed as parameter to mmc_switch(). So this is not an issue at all. So if the ext_csd change to structure, maybe the function call here don't looks like so concise as before... What do you think for this? The EXT_CSD field definitions are completely independent of the way how you access the registers. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de This message was made from 100% recycled electrons. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define
Hi Wolfgang, On Tue, Oct 11, 2011 at 1:33 AM, Wolfgang Denk w...@denx.de wrote: Dear Lei Wen, In message CALZhoSQbvKj0MtqryeHX-4LkvQJR2=b9u_m4yjjfm1mjv2m...@mail.gmail.com you wrote: So macro may looks more concise and could parse from its meaning easily eno= ugh. We do not accept (typeless) register offset definitions. Please use a struct, so the compiler has a chance to perform type checking. I check the code again, and find there is a reason for previous defined macro to use. That is those register offset defined as macro may need later passing as a function parameter like: err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1); I don;t see that any register (which now would be an address in a struct insteadd of being an offset to be added to a base address) would be passed as parameter to mmc_switch(). So this is not an issue at all. So if the ext_csd change to structure, maybe the function call here don't looks like so concise as before... What do you think for this? The EXT_CSD field definitions are completely independent of the way how you access the registers. So do you means I should keep the EXT_CSD_HS_TIMING? Or I change like below? from err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1); to: err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, ext_csd.hs_timing - ext_csd, 1); Best regards, Lei ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define
Hi Lei Wen, On Tue, Oct 11, 2011 at 11:48 AM, Lei Wen adrian.w...@gmail.com wrote: Hi Wolfgang, On Tue, Oct 11, 2011 at 1:33 AM, Wolfgang Denk w...@denx.de wrote: Dear Lei Wen, In message CALZhoSQbvKj0MtqryeHX-4LkvQJR2=b9u_m4yjjfm1mjv2m...@mail.gmail.com you wrote: So macro may looks more concise and could parse from its meaning easily eno= ugh. We do not accept (typeless) register offset definitions. Please use a struct, so the compiler has a chance to perform type checking. I check the code again, and find there is a reason for previous defined macro to use. That is those register offset defined as macro may need later passing as a function parameter like: err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1); I don;t see that any register (which now would be an address in a struct insteadd of being an offset to be added to a base address) would be passed as parameter to mmc_switch(). So this is not an issue at all. So if the ext_csd change to structure, maybe the function call here don't looks like so concise as before... What do you think for this? The EXT_CSD field definitions are completely independent of the way how you access the registers. So do you means I should keep the EXT_CSD_HS_TIMING? Or I change like below? from err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1); to: err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, ext_csd.hs_timing - ext_csd, 1); I imagine the compiler will complain that the types for ext_csd.hs_timing and ext_csd are different. Try: struct ext_csd *ext_csd_ptr = 0; err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, ext_csd_ptr-hs_timing, 1); Regards, Graeme ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define
Hi Prafulla,
On Fri, Oct 7, 2011 at 7:36 PM, Prafulla Wadaskar prafu...@marvell.com wrote:
-Original Message-
From: u-boot-boun...@lists.denx.de [mailto:u-boot-boun...@lists.denx.de]
On Behalf Of Lei Wen
Sent: Thursday, October 06, 2011 8:41 PM
To: Marek Vasut
Cc: Lei Wen; u-boot@lists.denx.de; Andy Fleming
Subject: Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro
define
Hi Marek,
On Tue, Oct 4, 2011 at 8:07 PM, Marek Vasut marek.va...@gmail.com
wrote:
Hi Lei,
this is better, but what about structure-based access ?
struct somrthing {
u8 a1;
u8 a2;
...
};
Like this.
Also, CC Andy.
The ext_csd current usage in mmc.c is not too much, here I mean only few
of
the fields of the ext_csd is used, also fully definition of ext_csd
member would seems so huge a structure at its appearence...
So macro may looks more concise and could parse from its meaning easily
enough.
Anyway, more comments on this welcomes. :)
Dear Lei
Using c-struct is our strategy, may be full definition is huge, you may skip
them inserting padding in the c-struct.
I personally prefer the complete struct to be defined from day one. This
means you don't have to patch it when you end up needing to use another
member later on and risk fouling up the offsets
Regards,
Graeme
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define
On Fri, Oct 7, 2011 at 4:36 PM, Prafulla Wadaskar prafu...@marvell.com wrote:
-Original Message-
From: u-boot-boun...@lists.denx.de [mailto:u-boot-boun...@lists.denx.de]
On Behalf Of Lei Wen
Sent: Thursday, October 06, 2011 8:41 PM
To: Marek Vasut
Cc: Lei Wen; u-boot@lists.denx.de; Andy Fleming
Subject: Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro
define
Hi Marek,
On Tue, Oct 4, 2011 at 8:07 PM, Marek Vasut marek.va...@gmail.com
wrote:
On Tuesday, October 04, 2011 08:35:10 AM Lei Wen wrote:
Previous magic number is hard to parse its meaning, change it to
respective macro definition
Signed-off-by: Lei Wen lei...@marvell.com
[..]
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -145,13 +145,15 @@
/*
* EXT_CSD fields
*/
-
-#define EXT_CSD_PART_CONF 179 /* R/W */
-#define EXT_CSD_BUS_WIDTH 183 /* R/W */
-#define EXT_CSD_HS_TIMING 185 /* R/W */
-#define EXT_CSD_CARD_TYPE 196 /* RO */
-#define EXT_CSD_REV 192 /* RO */
-#define EXT_CSD_SEC_CNT 212 /* RO, 4 bytes */
+#define EXT_CSD_PARTITIONING_SUPPORT 160 /* RO */
+#define EXT_CSD_ERASE_GROUP_DEF 175 /* R/W */
+#define EXT_CSD_PART_CONF 179 /* R/W */
+#define EXT_CSD_BUS_WIDTH 183 /* R/W */
+#define EXT_CSD_HS_TIMING 185 /* R/W */
+#define EXT_CSD_REV 192 /* RO */
+#define EXT_CSD_CARD_TYPE 196 /* RO */
+#define EXT_CSD_SEC_CNT 212 /* RO, 4 bytes
*/
+#define EXT_CSD_HC_ERASE_GRP_SIZE 224 /* RO */
/*
* EXT_CSD field definitions
Hi Lei,
this is better, but what about structure-based access ?
struct somrthing {
u8 a1;
u8 a2;
...
};
Like this.
Also, CC Andy.
The ext_csd current usage in mmc.c is not too much, here I mean only few
of
the fields of the ext_csd is used, also fully definition of ext_csd
member would seems so huge a structure at its appearence...
So macro may looks more concise and could parse from its meaning easily
enough.
Anyway, more comments on this welcomes. :)
Dear Lei
Using c-struct is our strategy, may be full definition is huge, you may skip
them inserting padding in the c-struct.
Regards..
Prafulla . .
Ok... Got that.
I would send patch soon using the c style.
Thanks,
Lei
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define
-Original Message-
From: u-boot-boun...@lists.denx.de [mailto:u-boot-boun...@lists.denx.de]
On Behalf Of Lei Wen
Sent: Thursday, October 06, 2011 8:41 PM
To: Marek Vasut
Cc: Lei Wen; u-boot@lists.denx.de; Andy Fleming
Subject: Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro
define
Hi Marek,
On Tue, Oct 4, 2011 at 8:07 PM, Marek Vasut marek.va...@gmail.com
wrote:
On Tuesday, October 04, 2011 08:35:10 AM Lei Wen wrote:
Previous magic number is hard to parse its meaning, change it to
respective macro definition
Signed-off-by: Lei Wen lei...@marvell.com
[..]
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -145,13 +145,15 @@
/*
* EXT_CSD fields
*/
-
-#define EXT_CSD_PART_CONF 179 /* R/W */
-#define EXT_CSD_BUS_WIDTH 183 /* R/W */
-#define EXT_CSD_HS_TIMING 185 /* R/W */
-#define EXT_CSD_CARD_TYPE 196 /* RO */
-#define EXT_CSD_REV 192 /* RO */
-#define EXT_CSD_SEC_CNT 212 /* RO, 4 bytes */
+#define EXT_CSD_PARTITIONING_SUPPORT 160 /* RO */
+#define EXT_CSD_ERASE_GROUP_DEF 175 /* R/W */
+#define EXT_CSD_PART_CONF 179 /* R/W */
+#define EXT_CSD_BUS_WIDTH 183 /* R/W */
+#define EXT_CSD_HS_TIMING 185 /* R/W */
+#define EXT_CSD_REV 192 /* RO */
+#define EXT_CSD_CARD_TYPE 196 /* RO */
+#define EXT_CSD_SEC_CNT 212 /* RO, 4 bytes
*/
+#define EXT_CSD_HC_ERASE_GRP_SIZE 224 /* RO */
/*
* EXT_CSD field definitions
Hi Lei,
this is better, but what about structure-based access ?
struct somrthing {
u8 a1;
u8 a2;
...
};
Like this.
Also, CC Andy.
The ext_csd current usage in mmc.c is not too much, here I mean only few
of
the fields of the ext_csd is used, also fully definition of ext_csd
member would seems so huge a structure at its appearence...
So macro may looks more concise and could parse from its meaning easily
enough.
Anyway, more comments on this welcomes. :)
Dear Lei
Using c-struct is our strategy, may be full definition is huge, you may skip
them inserting padding in the c-struct.
Regards..
Prafulla . .
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define
Hi Marek,
On Tue, Oct 4, 2011 at 8:07 PM, Marek Vasut marek.va...@gmail.com wrote:
On Tuesday, October 04, 2011 08:35:10 AM Lei Wen wrote:
Previous magic number is hard to parse its meaning, change it to
respective macro definition
Signed-off-by: Lei Wen lei...@marvell.com
[..]
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -145,13 +145,15 @@
/*
* EXT_CSD fields
*/
-
-#define EXT_CSD_PART_CONF 179 /* R/W */
-#define EXT_CSD_BUS_WIDTH 183 /* R/W */
-#define EXT_CSD_HS_TIMING 185 /* R/W */
-#define EXT_CSD_CARD_TYPE 196 /* RO */
-#define EXT_CSD_REV 192 /* RO */
-#define EXT_CSD_SEC_CNT 212 /* RO, 4 bytes */
+#define EXT_CSD_PARTITIONING_SUPPORT 160 /* RO */
+#define EXT_CSD_ERASE_GROUP_DEF 175 /* R/W */
+#define EXT_CSD_PART_CONF 179 /* R/W */
+#define EXT_CSD_BUS_WIDTH 183 /* R/W */
+#define EXT_CSD_HS_TIMING 185 /* R/W */
+#define EXT_CSD_REV 192 /* RO */
+#define EXT_CSD_CARD_TYPE 196 /* RO */
+#define EXT_CSD_SEC_CNT 212 /* RO, 4 bytes */
+#define EXT_CSD_HC_ERASE_GRP_SIZE 224 /* RO */
/*
* EXT_CSD field definitions
Hi Lei,
this is better, but what about structure-based access ?
struct somrthing {
u8 a1;
u8 a2;
...
};
Like this.
Also, CC Andy.
The ext_csd current usage in mmc.c is not too much, here I mean only few of
the fields of the ext_csd is used, also fully definition of ext_csd
member would seems so huge a structure at its appearence...
So macro may looks more concise and could parse from its meaning easily enough.
Anyway, more comments on this welcomes. :)
Best regards,
Lei
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define
On Thursday, October 06, 2011 05:10:32 PM Lei Wen wrote:
Hi Marek,
On Tue, Oct 4, 2011 at 8:07 PM, Marek Vasut marek.va...@gmail.com wrote:
On Tuesday, October 04, 2011 08:35:10 AM Lei Wen wrote:
Previous magic number is hard to parse its meaning, change it to
respective macro definition
Signed-off-by: Lei Wen lei...@marvell.com
[..]
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -145,13 +145,15 @@
/*
* EXT_CSD fields
*/
-
-#define EXT_CSD_PART_CONF179 /* R/W */
-#define EXT_CSD_BUS_WIDTH183 /* R/W */
-#define EXT_CSD_HS_TIMING185 /* R/W */
-#define EXT_CSD_CARD_TYPE196 /* RO */
-#define EXT_CSD_REV 192 /* RO */
-#define EXT_CSD_SEC_CNT 212 /* RO, 4 bytes */
+#define EXT_CSD_PARTITIONING_SUPPORT 160 /* RO */
+#define EXT_CSD_ERASE_GROUP_DEF 175 /* R/W */
+#define EXT_CSD_PART_CONF179 /* R/W */
+#define EXT_CSD_BUS_WIDTH183 /* R/W */
+#define EXT_CSD_HS_TIMING185 /* R/W */
+#define EXT_CSD_REV 192 /* RO */
+#define EXT_CSD_CARD_TYPE196 /* RO */
+#define EXT_CSD_SEC_CNT 212 /* RO, 4 bytes */
+#define EXT_CSD_HC_ERASE_GRP_SIZE224 /* RO */
/*
* EXT_CSD field definitions
Hi Lei,
this is better, but what about structure-based access ?
struct somrthing {
u8 a1;
u8 a2;
...
};
Like this.
Also, CC Andy.
The ext_csd current usage in mmc.c is not too much, here I mean only few of
the fields of the ext_csd is used, also fully definition of ext_csd
member would seems so huge a structure at its appearence...
So macro may looks more concise and could parse from its meaning easily
enough.
Anyway, more comments on this welcomes. :)
Best regards,
Lei
Hi Lei,
let's see what Andy thinks of this approach.
Cheers
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define
Dear Lei Wen, In message calzhosrhbf2vmu5olp3hwh4yq4xfip19ajd24gn4sy-rm6b...@mail.gmail.com you wrote: The ext_csd current usage in mmc.c is not too much, here I mean only few of the fields of the ext_csd is used, also fully definition of ext_csd member would seems so huge a structure at its appearence... So macro may looks more concise and could parse from its meaning easily eno= ugh. We do not accept (typeless) register offset definitions. Please use a struct, so the compiler has a chance to perform type checking. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de An open mind has but one disadvantage: it collects dirt. - a saying at RPI ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define
On Tuesday, October 04, 2011 08:35:10 AM Lei Wen wrote:
Previous magic number is hard to parse its meaning, change it to
respective macro definition
Signed-off-by: Lei Wen lei...@marvell.com
[..]
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -145,13 +145,15 @@
/*
* EXT_CSD fields
*/
-
-#define EXT_CSD_PART_CONF179 /* R/W */
-#define EXT_CSD_BUS_WIDTH183 /* R/W */
-#define EXT_CSD_HS_TIMING185 /* R/W */
-#define EXT_CSD_CARD_TYPE196 /* RO */
-#define EXT_CSD_REV 192 /* RO */
-#define EXT_CSD_SEC_CNT 212 /* RO, 4 bytes */
+#define EXT_CSD_PARTITIONING_SUPPORT 160 /* RO */
+#define EXT_CSD_ERASE_GROUP_DEF 175 /* R/W */
+#define EXT_CSD_PART_CONF179 /* R/W */
+#define EXT_CSD_BUS_WIDTH183 /* R/W */
+#define EXT_CSD_HS_TIMING185 /* R/W */
+#define EXT_CSD_REV 192 /* RO */
+#define EXT_CSD_CARD_TYPE196 /* RO */
+#define EXT_CSD_SEC_CNT 212 /* RO, 4 bytes */
+#define EXT_CSD_HC_ERASE_GRP_SIZE224 /* RO */
/*
* EXT_CSD field definitions
Hi Lei,
this is better, but what about structure-based access ?
struct somrthing {
u8 a1;
u8 a2;
...
};
Like this.
Also, CC Andy.
Cheers
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
