[U-Boot] declaring and initializing variables
Kim, et al.,
I know I have asked this before. Pardon me as I don't consider myself a savy programmer.
I am cleaning up the DDR driver for mpc83xx, mpc85xx and mpc86xx. The question is the accetable formats of declaring and initializing variable at the same time. The variables are the ccsr register pointers. I have two formats here
struct ccsr_ddr __iomem *ddr = (void *) CONFIG_FOO_ADDR; struct ccsr_ddr __iomem *ddr = (struct ccsr_ddr __iomem *) CONFIG_FOO_ADDR;
You have told me the second format is preferred. I have been using this format since. But in practice, the second format is often too long and I have to wrap to next line. It's not a problem for new code. As I am trying to cleanup the existing code, I would have to make more changes. So I am back to this question. Is the first format (using void *) accetable in long term?
Regards,
York
On Mon, Sep 30, 2013 at 7:04 PM, York Sun yorksun@freescale.com wrote:
struct ccsr_ddr __iomem *ddr = (void *) CONFIG_FOO_ADDR; struct ccsr_ddr __iomem *ddr = (struct ccsr_ddr __iomem *) CONFIG_FOO_ADDR;
You have told me the second format is preferred. I have been using this format since. But in practice, the second format is often too long and I have to wrap to next line. It's not a problem for new code. As I am trying to cleanup the existing code, I would have to make more changes. So I am back to this question. Is the first format (using void *) accetable in long term?
I think you can blame me for the first version. I started doing that years ago exactly because the second version is too long.
I think the first version is preferred because the second version is unnecessarily complex. The reason you need to cast the macro to a pointer is because it's defined as an integer, which is the real "bug" here. All of these macros should be defined as pointers, but that's a huge change that causes problems elsewhere. Since you're already "cheating" by making them integers, you would just be pretending to do the right thing by using the "proper" cast. By using "(void *)", you're saying that you are "fixing" the integer by making it into a pointer.
I don't know if this makes any sense.
On Mon, 30 Sep 2013 17:04:33 -0700 York Sun yorksun@freescale.com wrote:
Kim, et al.,
I know I have asked this before. Pardon me as I don't consider myself a savy programmer.
I am cleaning up the DDR driver for mpc83xx, mpc85xx and mpc86xx. The question is the accetable formats of declaring and initializing variable at the same time. The variables are the ccsr register pointers. I have two formats here
struct ccsr_ddr __iomem *ddr = (void *) CONFIG_FOO_ADDR; struct ccsr_ddr __iomem *ddr = (struct ccsr_ddr __iomem *) CONFIG_FOO_ADDR;
You have told me the second format is preferred. I have been using this format since. But in practice, the second format is often too long and I have to wrap to next line. It's not a problem for new code. As I am trying to cleanup the existing code, I would have to make more changes. So I am back to this question. Is the first format (using void *) accetable in long term?
you're not running sparse, are you? :)
Use 'make C=1' or 'MAKEALL -C' when building u-boot.
Kim
On 10/07/2013 03:03 PM, Kim Phillips wrote:
On Mon, 30 Sep 2013 17:04:33 -0700 York Sun yorksun@freescale.com wrote:
Kim, et al.,
I know I have asked this before. Pardon me as I don't consider myself a savy programmer.
I am cleaning up the DDR driver for mpc83xx, mpc85xx and mpc86xx. The question is the accetable formats of declaring and initializing variable at the same time. The variables are the ccsr register pointers. I have two formats here
struct ccsr_ddr __iomem *ddr = (void *) CONFIG_FOO_ADDR; struct ccsr_ddr __iomem *ddr = (struct ccsr_ddr __iomem *) CONFIG_FOO_ADDR;
You have told me the second format is preferred. I have been using this format since. But in practice, the second format is often too long and I have to wrap to next line. It's not a problem for new code. As I am trying to cleanup the existing code, I would have to make more changes. So I am back to this question. Is the first format (using void *) accetable in long term?
you're not running sparse, are you? :)
Use 'make C=1' or 'MAKEALL -C' when building u-boot.
I see what you mean. We have so many issue with existing code. Is it practical to enforce?
York
On Mon, 14 Oct 2013 12:05:52 -0700 York Sun yorksun@freescale.com wrote:
On 10/07/2013 03:03 PM, Kim Phillips wrote:
On Mon, 30 Sep 2013 17:04:33 -0700 York Sun yorksun@freescale.com wrote:
Kim, et al.,
I know I have asked this before. Pardon me as I don't consider myself a savy programmer.
I am cleaning up the DDR driver for mpc83xx, mpc85xx and mpc86xx. The question is the accetable formats of declaring and initializing variable at the same time. The variables are the ccsr register pointers. I have two formats here
struct ccsr_ddr __iomem *ddr = (void *) CONFIG_FOO_ADDR; struct ccsr_ddr __iomem *ddr = (struct ccsr_ddr __iomem *) CONFIG_FOO_ADDR;
You have told me the second format is preferred. I have been using this format since. But in practice, the second format is often too long and I have to wrap to next line. It's not a problem for new code. As I am trying to cleanup the existing code, I would have to make more changes. So I am back to this question. Is the first format (using void *) accetable in long term?
you're not running sparse, are you? :)
Use 'make C=1' or 'MAKEALL -C' when building u-boot.
I see what you mean. We have so many issue with existing code. Is it practical to enforce?
incrementally, i.e., for new patches, yes. The notion of a separate i/o address space is valid for checking u-boot source. If existing users want to update their code, that's fine too, they get the added benefit.
Kim
participants (3)
-
Kim Phillips -
Timur Tabi -
York Sun