Dear Wolfgang Denk,
> In message <4e60b220.6010...@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
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
