Dear Wolfgang Denk,
In message 4E60B220.6010309@monstr.eu you wrote:
As I see there is still ugly board/xilinx/common folder and ancient enet driver and i2c driver.
Indeed, and improvementrs are more than welcome.
Actually even this is incorrect - AFAIK Device Control Registers (DCR) exist not on all PPC systems, but only on 4xx (and even there only on some models). So your code works on a few systems, silently does not do anything on others, and crashes on yet others with an illegal instruction.
That driver is not definitely for all ppc systems. That IP is available just for Xilinx FPGA where can be possible to use it with Microblaze and xilinx ppc440 (maybe ppc405). That DCR handling, which is implemented in this driver, fits to xilinx ppc440 implementation. Which means that none can add this IP to any other PPC system out of Xilinx FPGA.
So why not use something like CONFIG_440 in this test, and add an #error for anything else?
Do we actually need this m{f,t}dcr_local() then?
DCR handling is specific for Xilinx ppc440 which means that not all PPC440 can use it. As you see m{f,t}dcr_local setup handling for it that's why it is neeeded.
Sorry I can't see any problem to have driver for specific platform or even to have one driver which supports two completely different platform.
My issue is that this code silently breaks or crashes when certain (undocumented) conditions are not met. Preventing this seems not to bee too difficult: add a comment, make it depend on the right CONFIG_ settings, and bail out with a clear error message when conditions are not met.
Driver is protected by CONFIG_XILINX_LL_TEMAC option which means that any platform is not silently breaks. You can use it with Microblaze and PPC and configuration is done (xparameters.h and config.mk files) by u-boot BSP in connection to Xilinx EDK which check if DCR can be used or not.
As for the other part of the problem - you try to mix two different cases: one where you refer to an index, and one where you refer to an address.
In technical sense it is still address not index. It is different addressing mode.
I have done it because it is much better than a lot of ifdefs. It is more than that because ppc has dcr up to 4 DMAs but memory controller supports up to 8 DMAs that's why I think that it is better to support both modes and decide by configuration.
This obviously doesn't mix well. If there is no better way
of doing this, I'd still prefer deriving the index from the offset in a struct than deriving the address from an offset - the intention is to enable the compiler to perform type checking, which is impossible with a typeless base+offset address.
I understand the reasons for that but I can't see any elegant way how to do so.
I don't want to have this in mainline.
If this is your decision, I won't send any updated version.
Attempted extortion?
My god why do you think that it is extortion? If you don't want to add it to mainline because you think that this driver is bad/broken/anything, I can do nothing with it and make no sense for me to invest my time to it. And I am not going to disturb others with it.
Regards, Michal