| Message ID | 53632AC5.1030702@denx.de |
|---|---|
| State | RFC |
| Delegated to: | Tom Rini |
| Headers | show |
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
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
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
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
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
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
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 _______________________________________________
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 ? U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot