RE: [U-Boot-Users] Proposed change; What do you think?
Jon Loeliger wrote:
I'd like to get your opinion on a proposed change to a few files that handle some aspects of the various enetaddr fields as found in asm-ppc/u-boot.h, common/cmd_bdinfo.c and lib_ppc/board.c.
In particular, I'd like to propose a shift from having these fields be present when certain boards are #defined to having these fields be present when CONFIG_ETH1ADDR symbols are defined.
I like the idea of using a common symbol, but I would prefer a new one, something like CONFIG_HAS_ETH1, so I can have bi_enet1addr in the kernel interface without putting a default value for it in the environment.
There are only a few boards for which this might make a difference. I'm the bit swizzler for most of them (MPC85xxXDS), but the others include the PN62, the PPCCHAMEONEVB, the 440GX, the GT_6426x. SXNI855T, SVM_SC8xx and possibly the DB64360, DB64460, CATcenter, and OCOTEA boards. The latter define CONFIG_ETH1ADDR but don't have their cases include in places like the coomon/cmd_bdinfo.c.
As the SXNI855T maintainer, I don't see any problems with using CONFIG_ETH1ADDR or CONFIG_HAS_ETH1.
Dave Ellis
On Thu, 2004-08-19 at 09:40, Dave Ellis wrote:
Jon Loeliger wrote:
I'd like to get your opinion on a proposed change to a few files that handle some aspects of the various enetaddr fields as found in asm-ppc/u-boot.h, common/cmd_bdinfo.c and lib_ppc/board.c.
In particular, I'd like to propose a shift from having these fields be present when certain boards are #defined to having these fields be present when CONFIG_ETH1ADDR symbols are defined.
I like the idea of using a common symbol, but I would prefer a new one, something like CONFIG_HAS_ETH1, so I can have bi_enet1addr in the kernel interface without putting a default value for it in the environment.
BTW, I am willing to make the change so that the code uses the symbols:
CONFIG_HAS_ETH CONFIG_HAS_ETH1 CONFIG_HAS_ETH2 and CONFIG_HAS_ETH3
as suggested. I like it.
However, now I need answers to the following question: Do you want me to retrofit code into all the Config files to #define CONFIG_HAS_ETHx where it currently also has CONFIG_ETHxADDR defined, or where the code has a board name even though a CONIG_ETHxADDR is not defined too?
Happy to do this, just realize that to be backwards compatible with existing config files, I'll have to change many config files. I can not test them all. I can test the 4 I have in front of me.
FYI, I am also willing to remove the #ifdef conditionality from the bd_t structure around these ETH addr fields as well, but with the caveat that it changes other people's bd_t structures and potentially messes up their Linux interfaces. Again, I can't test all that either...
Thanks, jdl
In message 1092945083.8297.24.camel@blarg.somerset.sps.mot.com you wrote:
However, now I need answers to the following question: Do you want me to retrofit code into all the Config files to #define CONFIG_HAS_ETHx where it currently also has CONFIG_ETHxADDR defined, or where the code has a board name even though a CONIG_ETHxADDR is not defined too?
Do you want to have your patch accepted?
Happy to do this, just realize that to be backwards compatible with existing config files, I'll have to change many config files. I can not test them all. I can test the 4 I have in front of me.
Please keep all files in a konsistent state.
FYI, I am also willing to remove the #ifdef conditionality from the bd_t structure around these ETH addr fields as well, but with the caveat that it changes other people's bd_t structures and potentially messes up their Linux interfaces. Again, I can't test all that either...
Don't put to many different things into a single patch. This last part has a chance of being rejected (depending on what you're going to do; I'm not sure I understand your intentions).
Best regards,
Wolfgang Denk
On Sun, 2004-08-22 at 18:38, Wolfgang Denk wrote:
In message 1092945083.8297.24.camel@blarg.somerset.sps.mot.com you wrote:
However, now I need answers to the following question: Do you want me to retrofit code into all the Config files to #define CONFIG_HAS_ETHx where it currently also has CONFIG_ETHxADDR defined, or where the code has a board name even though a CONIG_ETHxADDR is not defined too?
Do you want to have your patch accepted?
Happy to do this, just realize that to be backwards compatible with existing config files, I'll have to change many config files. I can not test them all. I can test the 4 I have in front of me.
Please keep all files in a konsistent state.
Oh, I'm happy to keep it in a consistent state. That's not the question I was trying to get answered. I'm seeking acceptance of the condition that I can not test all the affected boards even though the patch would necessarily touch many boards. Compile, sure. Test, no way.
FYI, I am also willing to remove the #ifdef conditionality from the bd_t structure around these ETH addr fields as well, but with the caveat that it changes other people's bd_t structures and potentially messes up their Linux interfaces. Again, I can't test all that either...
Don't put to many different things into a single patch. This last part has a chance of being rejected (depending on what you're going to do; I'm not sure I understand your intentions).
I never said it would all be one patch.
Furthermore, I was merely suggesting I'd be willing to work on the suggestion that Dan Malek had proposed. ACtually, just one aspect of it, specifically WRT the ethernet MAC address fields.
As I have it in my tree now, it has been left conditional, just the names of the #ifdef conditional has changed.
jdl
In message 1093270471.29614.7.camel@blarg.somerset.sps.mot.com you wrote:
Oh, I'm happy to keep it in a consistent state. That's not the question I was trying to get answered. I'm seeking acceptance of the condition that I can not test all the affected boards even though the patch would necessarily touch many boards. Compile, sure. Test, no way.
Submit a patch and hope that the board maintainers will test it. If they don't test and/or don't find any problems, there will be no complaints, and the patch will get added one day to the CVS tree.
Don't put to many different things into a single patch. This last part has a chance of being rejected (depending on what you're going to do; I'm not sure I understand your intentions).
I never said it would all be one patch.
Thanks. Maybe I misunderstood.
Furthermore, I was merely suggesting I'd be willing to work on the suggestion that Dan Malek had proposed. ACtually, just one aspect of it, specifically WRT the ethernet MAC address fields.
I think you misunderstood this. We already have different bd_info's for different processors, i. e. it looks different on a 8xx than it does on a 4xx. But within one class of processors iit should be fixed. Should ;-)
As I have it in my tree now, it has been left conditional, just the names of the #ifdef conditional has changed.
Thanks.
Best regards,
Wolfgang Denk
participants (3)
-
Dave Ellis -
Jon Loeliger -
Wolfgang Denk