[U-Boot] U-Boot, dm, ubi: struct device is declared twice

[U-Boot] U-Boot, dm, ubi: struct device is declared twice

[Heiko Schocher]

Hello Simon, Marek,

just updating to current mainline code and defining
CONFIG_SYS_GENERIC_BOARD pops up the following error:

  CC      common/board_r.o
In file included from include/linux/mtd/flashchip.h:21:0,
                  from include/linux/mtd/nand.h:31,
                  from include/nand.h:39,
                  from common/board_r.c:40:
include/ubi_uboot.h:202:8: error: redefinition of 'struct device'
  struct device {
         ^
In file included from include/dm.h:10:0,
                  from common/board_r.c:21:
include/dm/device.h:56:8: note: originally defined here
  struct device {
         ^
make[1]: *** [common/board_r.o] Fehler 1
make: *** [common] Fehler 2
pollux:u-boot hs [20140502] $

for a not yet mainlined imx6 board using UBI/UBIFS on nand. I am
currently sync current Linux MTD/UBI and UBIFS code to U-Boot, but
I think this error should pop up for all boards using DM and UBI ...

How to solve this double named struct? I do not want to change this
in Linux code as "struct device" is very much used, and this would
be a maintaining nightmare for future syncs with linux code.

Should we rename the DM "struct device" in include/dm/device.h ?

diff --git a/include/dm/device.h b/include/dm/device.h
index 4cd38ed..ba9f128 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -24,7 +24,7 @@ struct driver_info;
  #define DM_FLAG_ALLOC_PDATA    (2 << 0)

  /**
- * struct device - An instance of a driver
+ * struct dm_device - An instance of a driver
   *
   * This holds information about a device, which is a driver bound to a
   * particular port or peripheral (essentially a driver instance).
@@ -53,12 +53,12 @@ struct driver_info;
   * @sibling_node: Next device in list of all devices
   * @flags: Flags for this device DM_FLAG_...
   */
-struct device {
+struct dm_device {

What do you think?

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] U-Boot, dm, ubi: struct device is declared twice

[Wolfgang Denk]

Dear Heiko,

In message <53632AC5.1030702@denx.de> you wrote:
> 
> How to solve this double named struct? I do not want to change this
> in Linux code as "struct device" is very much used, and this would
> be a maintaining nightmare for future syncs with linux code.
> 
> Should we rename the DM "struct device" in include/dm/device.h ?
...
> - * struct device - An instance of a driver
> + * struct dm_device - An instance of a driver

I agree that we should keep U-Boot code as compatible with Linux as
possible, i. e. I would suggest to rename our U-Boot struct.

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: wd at denx.de
If I had to live my life again,  I'd  make  the  same  mistakes, only
sooner.                                          -- Tallulah Bankhead

[U-Boot] U-Boot, dm, ubi: struct device is declared twice

[Marek Vasut]

On Friday, May 02, 2014 at 07:19:01 AM, Heiko Schocher wrote:
> Hello Simon, Marek,
> 
> just updating to current mainline code and defining
> CONFIG_SYS_GENERIC_BOARD pops up the following error:
> 
>   CC      common/board_r.o
> In file included from include/linux/mtd/flashchip.h:21:0,
>                   from include/linux/mtd/nand.h:31,
>                   from include/nand.h:39,
>                   from common/board_r.c:40:
> include/ubi_uboot.h:202:8: error: redefinition of 'struct device'
>   struct device {
>          ^
> In file included from include/dm.h:10:0,
>                   from common/board_r.c:21:
> include/dm/device.h:56:8: note: originally defined here
>   struct device {
>          ^
> make[1]: *** [common/board_r.o] Fehler 1
> make: *** [common] Fehler 2
> pollux:u-boot hs [20140502] $
> 
> for a not yet mainlined imx6 board using UBI/UBIFS on nand. I am
> currently sync current Linux MTD/UBI and UBIFS code to U-Boot, but
> I think this error should pop up for all boards using DM and UBI ...

In the ideal case, we should use the same struct device for both UBI and DM, but 
we cannot do that. Thus, renaming the DM struct device would be the best option 
for now.

btw. are you syncing the UBI/UBIFS code from mainline into U-Boot now ? Can you 
keep me posted on patches please ?

Best regards,
Marek Vasut

[U-Boot] U-Boot, dm, ubi: struct device is declared twice

[Heiko Schocher]

Hello Marek,

Am 02.05.2014 08:06, schrieb Marek Vasut:
> On Friday, May 02, 2014 at 07:19:01 AM, Heiko Schocher wrote:
>> Hello Simon, Marek,
>>
>> just updating to current mainline code and defining
>> CONFIG_SYS_GENERIC_BOARD pops up the following error:
>>
>>    CC      common/board_r.o
>> In file included from include/linux/mtd/flashchip.h:21:0,
>>                    from include/linux/mtd/nand.h:31,
>>                    from include/nand.h:39,
>>                    from common/board_r.c:40:
>> include/ubi_uboot.h:202:8: error: redefinition of 'struct device'
>>    struct device {
>>           ^
>> In file included from include/dm.h:10:0,
>>                    from common/board_r.c:21:
>> include/dm/device.h:56:8: note: originally defined here
>>    struct device {
>>           ^
>> make[1]: *** [common/board_r.o] Fehler 1
>> make: *** [common] Fehler 2
>> pollux:u-boot hs [20140502] $
>>
>> for a not yet mainlined imx6 board using UBI/UBIFS on nand. I am
>> currently sync current Linux MTD/UBI and UBIFS code to U-Boot, but
>> I think this error should pop up for all boards using DM and UBI ...
>
> In the ideal case, we should use the same struct device for both UBI and DM, but

Yes, that was also my first thought ...

> we cannot do that. Thus, renaming the DM struct device would be the best option
> for now.

Ok. I prepare a patch for this.

> btw. are you syncing the UBI/UBIFS code from mainline into U-Boot now ? Can you
> keep me posted on patches please ?

Yes and Yes I can add you to cc.
(Currently MTD and UBI works on one board, now testing UBIFS ... after
this I want to test it on some more boards ...)

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] U-Boot, dm, ubi: struct device is declared twice

[Simon Glass]

Hi,

On 2 May 2014 01:16, Heiko Schocher <hs@denx.de> wrote:
> Hello Marek,
>
> Am 02.05.2014 08:06, schrieb Marek Vasut:
>
>> On Friday, May 02, 2014 at 07:19:01 AM, Heiko Schocher wrote:
>>>
>>> Hello Simon, Marek,
>>>
>>> just updating to current mainline code and defining
>>> CONFIG_SYS_GENERIC_BOARD pops up the following error:
>>>
>>>    CC      common/board_r.o
>>> In file included from include/linux/mtd/flashchip.h:21:0,
>>>                    from include/linux/mtd/nand.h:31,
>>>                    from include/nand.h:39,
>>>                    from common/board_r.c:40:
>>> include/ubi_uboot.h:202:8: error: redefinition of 'struct device'
>>>    struct device {
>>>           ^
>>> In file included from include/dm.h:10:0,
>>>                    from common/board_r.c:21:
>>> include/dm/device.h:56:8: note: originally defined here
>>>    struct device {
>>>           ^
>>> make[1]: *** [common/board_r.o] Fehler 1
>>> make: *** [common] Fehler 2
>>> pollux:u-boot hs [20140502] $
>>>
>>> for a not yet mainlined imx6 board using UBI/UBIFS on nand. I am
>>> currently sync current Linux MTD/UBI and UBIFS code to U-Boot, but
>>> I think this error should pop up for all boards using DM and UBI ...
>>
>>
>> In the ideal case, we should use the same struct device for both UBI and
>> DM, but
>
>
> Yes, that was also my first thought ...
>
>
>> we cannot do that. Thus, renaming the DM struct device would be the best
>> option
>> for now.
>
>
> Ok. I prepare a patch for this.

Linux also has struct device, so I wondered how it avoids this problem
and took a look.

This header file seems like a special thing for U-Boot - I wonder if
it would be better to use #define at the top of the C file for the
compatibility stuff (#define device ubi_device) rather than modify dm?
It does seem very strange to me, particularly as from what I can tell,
struct device is just a cut down version of the Linux struct.

Regards,
Simon

[U-Boot] U-Boot, dm, ubi: struct device is declared twice

[Heiko Schocher]

Hello Simon,

Am 02.05.2014 16:29, schrieb Simon Glass:
> Hi,
>
> On 2 May 2014 01:16, Heiko Schocher<hs@denx.de>  wrote:
>> Hello Marek,
>>
>> Am 02.05.2014 08:06, schrieb Marek Vasut:
>>
>>> On Friday, May 02, 2014 at 07:19:01 AM, Heiko Schocher wrote:
>>>>
>>>> Hello Simon, Marek,
>>>>
>>>> just updating to current mainline code and defining
>>>> CONFIG_SYS_GENERIC_BOARD pops up the following error:
>>>>
>>>>     CC      common/board_r.o
>>>> In file included from include/linux/mtd/flashchip.h:21:0,
>>>>                     from include/linux/mtd/nand.h:31,
>>>>                     from include/nand.h:39,
>>>>                     from common/board_r.c:40:
>>>> include/ubi_uboot.h:202:8: error: redefinition of 'struct device'
>>>>     struct device {
>>>>            ^
>>>> In file included from include/dm.h:10:0,
>>>>                     from common/board_r.c:21:
>>>> include/dm/device.h:56:8: note: originally defined here
>>>>     struct device {
>>>>            ^
>>>> make[1]: *** [common/board_r.o] Fehler 1
>>>> make: *** [common] Fehler 2
>>>> pollux:u-boot hs [20140502] $
>>>>
>>>> for a not yet mainlined imx6 board using UBI/UBIFS on nand. I am
>>>> currently sync current Linux MTD/UBI and UBIFS code to U-Boot, but
>>>> I think this error should pop up for all boards using DM and UBI ...
>>>
>>>
>>> In the ideal case, we should use the same struct device for both UBI and
>>> DM, but
>>
>>
>> Yes, that was also my first thought ...
>>
>>
>>> we cannot do that. Thus, renaming the DM struct device would be the best
>>> option
>>> for now.
>>
>>
>> Ok. I prepare a patch for this.
>
> Linux also has struct device, so I wondered how it avoids this problem
> and took a look.

Yes, the MTD/UBI and UBIFS subsystem is Code from Linux and it uses
the linux "struct device". I wonder, how DM and UBI compile together.

Is this tried somewhere?

(As I see drivers/usb/musb-new uses also "struct device" ...)

> This header file seems like a special thing for U-Boot - I wonder if

Yes, it includes some missing defines, structs for the UBI Subsystem,
so we could use Linux Code ... but we have also "include/linux/compat.h
for this ... Maybe it is worth to delete this include/ubi_uboot.h
and move the missing symbols to include/linux/compat.h?

> it would be better to use #define at the top of the C file for the

Yes, I thought about this too ...

The UBI subsystem in U-Boot has defined "UBI_LINUX" instead of
"__UBOOT__" ... which surprised me too.

  maybe I prepare such a change (delete "UBI_LINUX" and use
"__UBOOT__") for the new sync with current Linux MTD/UBI and UBIFS
layer?

> compatibility stuff (#define device ubi_device) rather than modify dm?

Hmm... I can try this ... but I am not really happy to have
such a define.

> It does seem very strange to me, particularly as from what I can tell,
> struct device is just a cut down version of the Linux struct.

Wouldn;t it be better to move this to include/linux/device.h then?

Looking in U-Boot:include/dm/device.h "struct device":

struct device {
*        struct driver *driver;
*        const char *name;
*        void *platdata;
*        int of_offset;
         struct device *parent;
*        void *priv;
*        struct uclass *uclass;
*        void *uclass_priv;
*        struct list_head uclass_node;
*        struct list_head child_head;
*        struct list_head sibling_node;
*        uint32_t flags;
};

All "*" are different to current mainline linux or not in it ...
so I prefer first to rename the DM "struct device" into for example
"struct u_device" ... Maybe we can sync them once with the linux
"struct device"

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] U-Boot, dm, ubi: struct device is declared twice

[Simon Glass]

Hi Heiko,

On 4 May 2014 23:55, Heiko Schocher <hs@denx.de> wrote:
> Hello Simon,
>
> Am 02.05.2014 16:29, schrieb Simon Glass:
>
>> Hi,
>>
>> On 2 May 2014 01:16, Heiko Schocher<hs@denx.de>  wrote:
>>>
>>> Hello Marek,
>>>
>>> Am 02.05.2014 08:06, schrieb Marek Vasut:
>>>
>>>> On Friday, May 02, 2014 at 07:19:01 AM, Heiko Schocher wrote:
>>>>>
>>>>>
>>>>> Hello Simon, Marek,
>>>>>
>>>>> just updating to current mainline code and defining
>>>>> CONFIG_SYS_GENERIC_BOARD pops up the following error:
>>>>>
>>>>>     CC      common/board_r.o
>>>>> In file included from include/linux/mtd/flashchip.h:21:0,
>>>>>                     from include/linux/mtd/nand.h:31,
>>>>>                     from include/nand.h:39,
>>>>>                     from common/board_r.c:40:
>>>>> include/ubi_uboot.h:202:8: error: redefinition of 'struct device'
>>>>>     struct device {
>>>>>            ^
>>>>> In file included from include/dm.h:10:0,
>>>>>                     from common/board_r.c:21:
>>>>> include/dm/device.h:56:8: note: originally defined here
>>>>>     struct device {
>>>>>            ^
>>>>> make[1]: *** [common/board_r.o] Fehler 1
>>>>> make: *** [common] Fehler 2
>>>>> pollux:u-boot hs [20140502] $
>>>>>
>>>>> for a not yet mainlined imx6 board using UBI/UBIFS on nand. I am
>>>>> currently sync current Linux MTD/UBI and UBIFS code to U-Boot, but
>>>>> I think this error should pop up for all boards using DM and UBI ...
>>>>
>>>>
>>>>
>>>> In the ideal case, we should use the same struct device for both UBI and
>>>> DM, but
>>>
>>>
>>>
>>> Yes, that was also my first thought ...
>>>
>>>
>>>> we cannot do that. Thus, renaming the DM struct device would be the best
>>>> option
>>>> for now.
>>>
>>>
>>>
>>> Ok. I prepare a patch for this.
>>
>>
>> Linux also has struct device, so I wondered how it avoids this problem
>> and took a look.
>
>
> Yes, the MTD/UBI and UBIFS subsystem is Code from Linux and it uses
> the linux "struct device". I wonder, how DM and UBI compile together.
>
> Is this tried somewhere?

Not yet, but it sounds like we are hitting the problem now.

>
> (As I see drivers/usb/musb-new uses also "struct device" ...)
>
>
>> This header file seems like a special thing for U-Boot - I wonder if
>
>
> Yes, it includes some missing defines, structs for the UBI Subsystem,
> so we could use Linux Code ... but we have also "include/linux/compat.h
> for this ... Maybe it is worth to delete this include/ubi_uboot.h
> and move the missing symbols to include/linux/compat.h?
>
>
>> it would be better to use #define at the top of the C file for the
>
>
> Yes, I thought about this too ...
>
> The UBI subsystem in U-Boot has defined "UBI_LINUX" instead of
> "__UBOOT__" ... which surprised me too.
>
>  maybe I prepare such a change (delete "UBI_LINUX" and use
> "__UBOOT__") for the new sync with current Linux MTD/UBI and UBIFS
> layer?
>
>
>> compatibility stuff (#define device ubi_device) rather than modify dm?
>
>
> Hmm... I can try this ... but I am not really happy to have
> such a define.

Me neither, but the justification is that we are trying to make
incompatible code work with minimal changes.

>
>
>> It does seem very strange to me, particularly as from what I can tell,
>> struct device is just a cut down version of the Linux struct.
>
>
> Wouldn;t it be better to move this to include/linux/device.h then?
>
> Looking in U-Boot:include/dm/device.h "struct device":
>
> struct device {
> *        struct driver *driver;
> *        const char *name;
> *        void *platdata;
> *        int of_offset;
>         struct device *parent;
> *        void *priv;
> *        struct uclass *uclass;
> *        void *uclass_priv;
> *        struct list_head uclass_node;
> *        struct list_head child_head;
> *        struct list_head sibling_node;
> *        uint32_t flags;
> };
>
> All "*" are different to current mainline linux or not in it ...
> so I prefer first to rename the DM "struct device" into for example
> "struct u_device" ... Maybe we can sync them once with the linux
> "struct device"

OK, I prefer not to do this, since we are renaming U-Boot files to
make Linux source work in the U-Boot tree. But this is not my decision
and it may be a more widespread problem in the future. So if you think
this is better, how about udevice? You will then need to rename all
the APIs.

Regards,
Simon