[U-Boot] declaring and initializing variables

[U-Boot] declaring and initializing variables

[York Sun]

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

[U-Boot] declaring and initializing variables

[Timur Tabi]

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.

[U-Boot] declaring and initializing variables

[Kim Phillips]

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

[U-Boot] declaring and initializing variables

[York Sun]

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

[U-Boot] declaring and initializing variables

[Kim Phillips]

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