Am Donnerstag, den 23.02.2012, 13:22 +0100 schrieb Michal Simek:
Hi Stephan,
Hi Michal,
I will take your comments seriously and ar working on a solution without a complex conversion matrix and a ll_temac standard initialization. Give me some time to do so ...
Stephan
Stephan Linz wrote:
Hi Michal,
back to your requests and objections I've built up a new patch set for evaluation. Some of the changes relate to the Microblaze platform rather than the network driver.
- U-Boot configuration variables vs. xparameters.h
I've tried to find a compromise between the U-Boot coding and configuration style and your preference. But at least in the point of the many many CPP conditional directives I can not reduce the code. I can move the code only to another place. I need a converstion from the automaticaly generated content of xparameters.h to the U-Boot configuration space. Now you can find this conversion matrix in a new platform header, xconversions.h -- I've placed the header in board/xilinx/microblaze-generic/ I think, we can find a better place.
Sorry I don't like that idea in general. I think that make no sense to create infrastructure for several ll_temac driver in bootloader.
The same code in different location is the same solution which I won't like.
The conversion matrix avoids all the wrong configuration variables inside U-Boot code beginning with XILINX_ (and not CONFIG_) and we can expand the configuration to new software related options we never get from hardware (example special interface naming). This mechanism can also be used by other driver and platform code, for example arch/microblaze/cpu/cache.c and arch/microblaze/lib/bootm.c (use XILINX_USE_DCACHE instead of ex. CONFIG_XILINX_DCACHE), common/cmd_mfsl.c (use XILINX_FSL_NUMBER instead of ex. CONFIG_XILINX_FSL), serial_xuartlite.c (use XILINX_UARTLITE_BASEADDR, XILINX_UARTLITE_BASEADDR1, ...), and probably also in xilinx_emaclite.c and xilinx_axi_emac.c when we extand a multi-IP support as we have done for the LL TEMAC now. Sure we have to work on, but it's another case (I can make some patches if you want)
I agree that we should use the same variable name if has to start with CONFIG_ prefix. Based on my experience make more sense to move all functionality and configuration thing to BSP and keep them out of mainline code because none cares in community that we can change almost everything. That's why u-boot uses xparamters.h, linux dts and 5 kernel parameters.
Creating any conversions out of BSPs is just wrong way and I won't agree with that. If you want to talk about renaming some variables I am ok to do so and I can also change my bsp to cover backward compatibility.
Pretty soon the same situation will be with u-boot for Xilinx Zynq arm platform where we will have to use the same style to be acceptable.
Another but not currently existing feasibility would be to generate a U-Boot conform xparameters.h, for example CONFIG_XILINX_LL_TEMAC0_BASE_ADDR instead of XILINX_LLTEMAC_BASEADDR. You and I, we have it in hand. We can change the Xilinx BSP in the near future. You in PataLinux and I in my own home-brewed BSP generator TPOS.
As I wrote above. Suggest what we should change and we can do that.
- Standard initialization vs. multiple direct calls from platform
Returning to the topic "standard" initialization. The last patch will show the differences bwetween your prefered way to do all in board init and my applicable standard initializition (a single call to the driver). Please look at it in the code and decide for yourself.
IRC. I describe you how I would like to see that configuration. It should be the same as I use for axiemac and emaclite.
We need a forward-looking decision, what kind of initialization, we want to use. Do we want to copy and paste the standard initialization steps from ip core to ip core (board init) and from platform to platform (Microplace, PPC405, PPC440) or make a simple call to the driver?
My preferences is to call the driver from platform and let him run all the necessary steps, that I do not want knowing on platform side.
You have to know platform all the time. I want to keep microblaze-generic platform as simple as possible.
If you are not OK with my preferred way you can of course create your new platform with conversion tables as you like. In PetaLinux we uses separate platform because of configuration things which lie above of it.
Honestly if I look at patch v10 1/8 my eyes hurting me because the origin patch (v3 or so) had ~670 line of code and yours 1943 and do the same things. From my point of view I am OK to keep this driver completely out of mainline because it is based on plb which will be deprecated in near future. I know that people will still use it for a long time but the question is if make sense to push to mainline. I will definitely use v3 in PetaLinux and keep things simpler. Future is axi driver is in the mainline.
Regards, Michal