| Message ID | 1314877154-14536-1-git-send-email-monstr@monstr.eu |
|---|---|
| State | Changes Requested |
| Headers | show |
Dear Michal Simek, In message <1314877154-14536-1-git-send-email-monstr@monstr.eu> you wrote: > LL Temac driver can be used by microblaze, xilinx ppc405/440 > in sdma and fifo mode. DCR or XPS bus can be used. > > The driver uses and requires PHYLIB. > +static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val) > +{ > + if (priv->mode & DCR_BIT) > + mtdcr_local(priv->ctrl + offset, val); > + else > + out_be32((u32 *)(priv->ctrl + offset * 4), val); > +} > + > +static u32 sdma_in_be32(struct ll_priv *priv, u32 offset) > +{ > + if (priv->mode & DCR_BIT) > + return mfdcr_local(priv->ctrl + offset); > + > + return in_be32((u32 *)(priv->ctrl + offset * 4)); > +} Can we please get rid of these functions? As mentioned many, many times before, we discourage all use of "base address plus offset" to access any device registers etc. These functions here re-introduce such accesses, and this is something I will not accept. > + /* Soft reset the DMA. */ > + sdma_out_be32(priv, DMA_CONTROL_REG, DMA_CONTROL_RESET); > + while (sdma_in_be32(priv, DMA_CONTROL_REG) & DMA_CONTROL_RESET) > + ; I think this has been pointed out before: we do not accept such potentially endless loops. Please fix this (and integrate theother previously made review remarks). Thanks. > + do { > + flush_cache((u32)&tx_bd, sizeof(tx_bd)); > + } while (!(tx_bd.stat & BDSTAT_COMPLETED_MASK)); Ditto. Please fix globally. > + /* Read out the packet info and start the DMA > + onto the second buffer to enable the ethernet rx > + path to run in parallel with sw processing > + packets. */ Incorrect multiline comment style; please fix globally. Best regards, Wolfgang Denk
Wolfgang Denk wrote: > Dear Michal Simek, > > In message <1314877154-14536-1-git-send-email-monstr@monstr.eu> you wrote: >> LL Temac driver can be used by microblaze, xilinx ppc405/440 >> in sdma and fifo mode. DCR or XPS bus can be used. >> >> The driver uses and requires PHYLIB. > > >> +static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val) >> +{ >> + if (priv->mode & DCR_BIT) >> + mtdcr_local(priv->ctrl + offset, val); >> + else >> + out_be32((u32 *)(priv->ctrl + offset * 4), val); >> +} >> + >> +static u32 sdma_in_be32(struct ll_priv *priv, u32 offset) >> +{ >> + if (priv->mode & DCR_BIT) >> + return mfdcr_local(priv->ctrl + offset); >> + >> + return in_be32((u32 *)(priv->ctrl + offset * 4)); >> +} > > Can we please get rid of these functions? As mentioned many, many > times before, we discourage all use of "base address plus offset" to > access any device registers etc. > > These functions here re-introduce such accesses, and this is something > I will not accept. Ok. How to do it? For bus access it is necessary to use 4B offsets for DCR just 1B and one system can contains two MACs where the first use 4B offset and the second 1B. >> + /* Soft reset the DMA. */ >> + sdma_out_be32(priv, DMA_CONTROL_REG, DMA_CONTROL_RESET); >> + while (sdma_in_be32(priv, DMA_CONTROL_REG) & DMA_CONTROL_RESET) >> + ; > > I think this has been pointed out before: we do not accept such > potentially endless loops. Please fix this (and integrate theother > previously made review remarks). Thanks. > >> + do { >> + flush_cache((u32)&tx_bd, sizeof(tx_bd)); >> + } while (!(tx_bd.stat & BDSTAT_COMPLETED_MASK)); > > Ditto. Please fix globally. I forget. Sorry. Will fix it. > >> + /* Read out the packet info and start the DMA >> + onto the second buffer to enable the ethernet rx >> + path to run in parallel with sw processing >> + packets. */ > > Incorrect multiline comment style; please fix globally. Fixed and checked. Thanks, Michal
Dear Michal Simek, In message <4E5F85DA.4080403@monstr.eu> you wrote: > > >> +static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val) > >> +{ > >> + if (priv->mode & DCR_BIT) > >> + mtdcr_local(priv->ctrl + offset, val); > >> + else > >> + out_be32((u32 *)(priv->ctrl + offset * 4), val); > >> +} > >> + > >> +static u32 sdma_in_be32(struct ll_priv *priv, u32 offset) > >> +{ > >> + if (priv->mode & DCR_BIT) > >> + return mfdcr_local(priv->ctrl + offset); > >> + > >> + return in_be32((u32 *)(priv->ctrl + offset * 4)); > >> +} > > > > Can we please get rid of these functions? As mentioned many, many > > times before, we discourage all use of "base address plus offset" to > > access any device registers etc. > > > > These functions here re-introduce such accesses, and this is something > > I will not accept. > > Ok. How to do it? > > For bus access it is necessary to use 4B offsets for DCR just 1B > and one system can contains two MACs where the first use 4B offset and the second > 1B. I don't think your description here matches the code above. With "1 byte offsets" you would be doing all unaligned bus accesses. I think you should use an array of structs and an index to select the right one. Best regards, Wolfgang Denk
Dear Wolfgang Denk, Wolfgang Denk wrote: > Dear Michal Simek, > > In message <4E5F85DA.4080403@monstr.eu> you wrote: >>>> +static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val) >>>> +{ >>>> + if (priv->mode & DCR_BIT) >>>> + mtdcr_local(priv->ctrl + offset, val); >>>> + else >>>> + out_be32((u32 *)(priv->ctrl + offset * 4), val); >>>> +} >>>> + >>>> +static u32 sdma_in_be32(struct ll_priv *priv, u32 offset) >>>> +{ >>>> + if (priv->mode & DCR_BIT) >>>> + return mfdcr_local(priv->ctrl + offset); >>>> + >>>> + return in_be32((u32 *)(priv->ctrl + offset * 4)); >>>> +} >>> Can we please get rid of these functions? As mentioned many, many >>> times before, we discourage all use of "base address plus offset" to >>> access any device registers etc. >>> >>> These functions here re-introduce such accesses, and this is something >>> I will not accept. >> Ok. How to do it? >> >> For bus access it is necessary to use 4B offsets for DCR just 1B >> and one system can contains two MACs where the first use 4B offset and the second >> 1B. > > I don't think your description here matches the code above. With "1 > byte offsets" you would be doing all unaligned bus accesses. Let me show it on example. on MB or PPC system with bus: DMA is in memory controller on 32bit address + sdma DMA port offset. DMA : Sdma offset(www.xilinx.com/support/documentation/ip_documentation/mpmc.pdf page 54 and 55) 0 : 0x0 1 : 0x80 2 : 0x100 3 : 0x180 ... 7 : 0x380 Let me assume that MPMC is at 0x12340000 The first reg for DMA2 accessed through bus is at 0x12340100 offset the second at 0x12340104, the third 0x12340108 etc. On PPC system with DCR is special connection between memory controller through DCR bus. Handling is done with mfdcr_local and mtdcr_local functions. DMA : Sdma address ranges (www.xilinx.com/support/documentation/user_guides/ug200.pdf page 261 and 299) 0 : 0x80-0x90 1 : 0x98-0xA8 2 : 0xB0-0xC0 3 : 0xC8-0xD8 The first reg for DMA2 accessed trough DCR is at 0xB0, the second at 0xB1, etc.. Regards, Michal
Dear Michal Simek, In message <4E607A0B.3040608@monstr.eu> you wrote: > > >>>> +static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val) > >>>> +{ > >>>> + if (priv->mode & DCR_BIT) > >>>> + mtdcr_local(priv->ctrl + offset, val); > >>>> + else > >>>> + out_be32((u32 *)(priv->ctrl + offset * 4), val); > >>>> +} ... > On PPC system with DCR is special connection between memory controller through DCR bus. Handling is done > with mfdcr_local and mtdcr_local functions. > > DMA : Sdma address ranges (www.xilinx.com/support/documentation/user_guides/ug200.pdf page 261 and 299) > 0 : 0x80-0x90 > 1 : 0x98-0xA8 > 2 : 0xB0-0xC0 > 3 : 0xC8-0xD8 > > The first reg for DMA2 accessed trough DCR is at 0xB0, the second at 0xB1, etc.. This is indeed a good example, as it shows how terribly broken your code is. See function sdma_out_be32() above. It is suppose to write a 32 bit value ("u32 val") as a 32 bit entity in big endian mode ("_be32") to some device register - but the register addresses are (1) not aligned to 32 bit boundaries and (2) not even 32 bits apart. Oh! You are NOT writing 32 bit data? You are writing only 8 bit? But the function claims to be writing 32 bit!!! And your function mtdcr_local() is actually a NOOP for anything but PPC? Without even a warning? This code is obfuscated in multiple levels. I will not accept that. Best regards, Wolfgang Denk
Wolfgang Denk wrote: > Dear Michal Simek, > > In message <4E607A0B.3040608@monstr.eu> you wrote: >>>>>> +static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val) >>>>>> +{ >>>>>> + if (priv->mode & DCR_BIT) >>>>>> + mtdcr_local(priv->ctrl + offset, val); >>>>>> + else >>>>>> + out_be32((u32 *)(priv->ctrl + offset * 4), val); >>>>>> +} > ... >> On PPC system with DCR is special connection between memory controller through DCR bus. Handling is done >> with mfdcr_local and mtdcr_local functions. >> >> DMA : Sdma address ranges (www.xilinx.com/support/documentation/user_guides/ug200.pdf page 261 and 299) >> 0 : 0x80-0x90 >> 1 : 0x98-0xA8 >> 2 : 0xB0-0xC0 >> 3 : 0xC8-0xD8 >> >> The first reg for DMA2 accessed trough DCR is at 0xB0, the second at 0xB1, etc.. > > This is indeed a good example, as it shows how terribly broken your > code is. > > See function sdma_out_be32() above. It is suppose to write a 32 bit > value ("u32 val") as a 32 bit entity in big endian mode ("_be32") to > some device register - but the register addresses are (1) not aligned > to 32 bit boundaries and (2) not even 32 bits apart. I think you misunderstand what there is written. For DCR is register address with offset + 0x1 but still you are writing there 32bit values. It is not direct access. and it is not any unaligned access at all. But bus access it will be unaligned access but not for DCR. DCR is defined just for PPC right now because none wanted to do it for Microblaze. Regards, Michal
Dear Michal Simek, In message <4E609682.8030701@monstr.eu> you wrote: > > >>>>>> +static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val) > >>>>>> +{ > >>>>>> + if (priv->mode & DCR_BIT) > >>>>>> + mtdcr_local(priv->ctrl + offset, val); > >>>>>> + else > >>>>>> + out_be32((u32 *)(priv->ctrl + offset * 4), val); > >>>>>> +} > > ... ... > >> The first reg for DMA2 accessed trough DCR is at 0xB0, the second at 0xB1, etc.. > > > > This is indeed a good example, as it shows how terribly broken your > > code is. > > > > See function sdma_out_be32() above. It is suppose to write a 32 bit > > value ("u32 val") as a 32 bit entity in big endian mode ("_be32") to > > some device register - but the register addresses are (1) not aligned > > to 32 bit boundaries and (2) not even 32 bits apart. > > I think you misunderstand what there is written. Maybe. Maybe even I want to misunderstand it. The problem is that the code does not prevent such misunderstanding. There are many shortcomings of that code. > DCR is defined just for PPC right now because none wanted to do it for Microblaze. 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. How do you call code that exposes such behaviour? I don't want to have this in mainline. Best regards, Wolfgang Denk
Dear Wolfgang Denk, Wolfgang Denk wrote: > Dear Michal Simek, > > In message <4E609682.8030701@monstr.eu> you wrote: >>>>>>>> +static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val) >>>>>>>> +{ >>>>>>>> + if (priv->mode & DCR_BIT) >>>>>>>> + mtdcr_local(priv->ctrl + offset, val); >>>>>>>> + else >>>>>>>> + out_be32((u32 *)(priv->ctrl + offset * 4), val); >>>>>>>> +} >>> ... > ... >>>> The first reg for DMA2 accessed trough DCR is at 0xB0, the second at 0xB1, etc.. >>> This is indeed a good example, as it shows how terribly broken your >>> code is. >>> >>> See function sdma_out_be32() above. It is suppose to write a 32 bit >>> value ("u32 val") as a 32 bit entity in big endian mode ("_be32") to >>> some device register - but the register addresses are (1) not aligned >>> to 32 bit boundaries and (2) not even 32 bits apart. >> I think you misunderstand what there is written. > > Maybe. Maybe even I want to misunderstand it. The problem is that > the code does not prevent such misunderstanding. > > There are many shortcomings of that code. I think that this is the reason why we have review process, don't we? Especially this driver is 2-3 years old and it is used by many our customers. It is only my effort to clear xilinx drivers/platforms. As I see there is still ugly board/xilinx/common folder and ancient enet driver and i2c driver. > >> DCR is defined just for PPC right now because none wanted to do it for Microblaze. > > 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. > > How do you call code that exposes such behaviour? If I look at drivers/net folder there are a lot of examples like that. None expect that altera_tse/bfin_mac/tsec will be possible to use with all systems. Maybe you expect that they can be use on the same architecture but this is Xilinx FPGA. It is up to you how you want to compose your system. IRC tsec is used just for Freescale PPC and this ll_temac driver is just used for xilinx microblaze and xilinx ppc. 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. I saw that there are some drivers in arch/<cpu>/ folders but this is not that case because can be possible to use it for microblaze and xilinx ppc. This ll_temac driver is just another net IP like emaclite is. Emaclite driver is possible to use on Microblaze and xilinx ppc systems and in near future with arm on Xilinx zynq platform. > I don't want to have this in mainline. If this is your decision, I won't send any updated version. Regards, Michal
Dear Michal Simek, 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? > 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. 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. 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 don't want to have this in mainline. > > If this is your decision, I won't send any updated version. Attempted extortion? Best regards, Wolfgang Denk
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
Dear Michal Simek, In message <4E60DA47.4070301@monstr.eu> you wrote: > > >> 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? You did not answer this - why not using CONFIG_440 instead of CONFIG_PPC ? > > 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. Then maybe you should chose a better name for it, say m?dcr_xilinx() or so. > > 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. I can only offer you a solution that seems acceptable to me. > > 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. The Processor Core User's Manual says for example: The DCR number (DCRN) is specified by the Device Control Register Immediate Prefix Register (DCRIPR) and the DCRF field of the mfdcr instruction. To me, "number" translates much better into index than into address. Also, the DCR number are incremented by 1 - if these were addresses in the common sense, they could only point to 8 bit entities - but the registers are actually 32 bit wide. But again, I can only show you what I think could be an acceptable approach. If you don't like it, please feel free to develop a better one. In any case, I will not accept the current (V3) code. > 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. Well, I just proposed one way - not elegant indeed, but I'd be willing to swallow that. > 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. I only complain about a few details of the implementation, and I even lend you a helping hand by offering specific solutions. Feel free to take or to refuse it. I got better things to do myself, either. Wolfgang Denk
Dear Wolfgang Denk, > In message <4E60DA47.4070301@monstr.eu> you wrote: >>>> 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? > > You did not answer this - why not using CONFIG_440 instead of > CONFIG_PPC ? Probably the best is CONFIG_XILINX_440. > >>> 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. > > Then maybe you should chose a better name for it, say m?dcr_xilinx() > or so. np. > >>> 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. > > I can only offer you a solution that seems acceptable to me. seems? >>> 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. > > The Processor Core User's Manual says for example: > > The DCR number (DCRN) is specified by the Device Control > Register Immediate Prefix Register (DCRIPR) > and the DCRF field of the mfdcr instruction. > > To me, "number" translates much better into index than into address. DCRN is number but for my case I use 0 for register "address" and 1 for value. To be accurate - 0 is indirect dcr address reg, and 1 is indirect dcr access reg. (Have changed that magic value to macros according technical documentation). > Also, the DCR number are incremented by 1 - if these were addresses in > the common sense, they could only point to 8 bit entities - but the > registers are actually 32 bit wide. And agree that DCR number is incremented by 1. On www.xilinx.com/support/documentation/user_guides/ug200.pdf page 261 is written that it is address that's why I use address. > But again, I can only show you what I think could be an acceptable > approach. If you don't like it, please feel free to develop a better > one. > > In any case, I will not accept the current (V3) code. sure. > >> 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. > > Well, I just proposed one way - not elegant indeed, but I'd be willing > to swallow that. You mean that array of structs, right? > >> 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. > > I only complain about a few details of the implementation, and I even > lend you a helping hand by offering specific solutions. Feel free to > take or to refuse it. I got better things to do myself, either. Sure. Regards, Michal
Dear Michal Simek, In message <4E647722.9080602@monstr.eu> you wrote: > > > I can only offer you a solution that seems acceptable to me. > > seems? Well, yes. As mentioned, I'm not happy with the propsal myself, but as long as I cannot come up with any better one it would be unjust to demand it from you. > >> 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. > > > > Well, I just proposed one way - not elegant indeed, but I'd be willing > > to swallow that. > > You mean that array of structs, right? No. I mean the text above: I'd rather derive the index from the offset in a struct instead of deriving the address from an offset (but I have to admit that I don't see an elegant way to implement this yet). Best regards, Wolfgang Denk
diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 819b197..4541eaf 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -84,6 +84,7 @@ COBJS-$(CONFIG_TSI108_ETH) += tsi108_eth.o COBJS-$(CONFIG_ULI526X) += uli526x.o COBJS-$(CONFIG_VSC7385_ENET) += vsc7385.o COBJS-$(CONFIG_XILINX_EMACLITE) += xilinx_emaclite.o +COBJS-$(CONFIG_XILINX_LL_TEMAC) += xilinx_ll_temac.o COBJS := $(sort $(COBJS-y)) SRCS := $(COBJS:.o=.c) diff --git a/drivers/net/xilinx_ll_temac.c b/drivers/net/xilinx_ll_temac.c new file mode 100644 index 0000000..1639363 --- /dev/null +++ b/drivers/net/xilinx_ll_temac.c @@ -0,0 +1,666 @@ +/* + * Xilinx xps_ll_temac ethernet driver for u-boot + * + * Copyright (C) 2008 - 2011 Michal Simek <monstr@monstr.eu> + * Copyright (C) 2008 - 2011 PetaLogix + * + * Based on Yoshio Kashiwagi kashiwagi@co-nss.co.jp driver + * Copyright (C) 2008 Nissin Systems Co.,Ltd. + * March 2008 created + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +#include <config.h> +#include <common.h> +#include <net.h> +#include <malloc.h> +#include <asm/processor.h> +#include <asm/io.h> +#include <phy.h> +#include <miiphy.h> + +#undef ETH_HALTING + +#if !defined(CONFIG_PHYLIB) +# error LL_TEMAC requires PHYLIB +#endif + +#define XTE_EMMC_LINKSPEED_MASK 0xC0000000 /* Link speed */ +/* XTE_EMCFG_LINKSPD_MASK */ +#define XTE_EMMC_LINKSPD_10 0x00000000 /* for 10 Mbit */ +#define XTE_EMMC_LINKSPD_100 0x40000000 /* for 100 Mbit */ +#define XTE_EMMC_LINKSPD_1000 0x80000000 /* forr 1000 Mbit */ + +#define XTE_RSE_MIIM_RR_MASK 0x0002 +#define XTE_RSE_MIIM_WR_MASK 0x0004 +#define XTE_RSE_CFG_RR_MASK 0x0020 +#define XTE_RSE_CFG_WR_MASK 0x0040 + +/* XPS_LL_TEMAC indirect registers offset definition */ +#define RCW1 0x240 +#define TC 0x280 +#define EMMC 0x300 +#define MC 0x340 +#define UAW0 0x380 +#define UAW1 0x384 +#define AFM 0x390 +#define MIIMWD 0x3b0 +#define MIIMAI 0x3b4 + +#define CNTLREG_WRITE_ENABLE_MASK 0x8000 + +#define MDIO_ENABLE_MASK 0x40 +#define MDIO_CLOCK_DIV_100MHz 0x28 + +/* XPS_LL_TEMAC SDMA registers definition */ +#define TX_CURDESC_PTR 0x03 +#define TX_TAILDESC_PTR 0x04 +#define TX_CHNL_CTRL 0x05 +#define TX_IRQ_REG 0x06 +#define TX_CHNL_STS 0x07 +#define RX_NXTDESC_PTR 0x08 +#define RX_CURDESC_PTR 0x0b +#define RX_TAILDESC_PTR 0x0c +#define RX_CHNL_CTRL 0x0d +#define RX_IRQ_REG 0x0e +#define RX_CHNL_STS 0x0f +#define DMA_CONTROL_REG 0x10 + +/* DMA control bit */ +#define DMA_CONTROL_RESET 0x1 + +/* CDMAC descriptor status bit definitions */ +# define BDSTAT_STOP_ON_END_MASK 0x20 +# define BDSTAT_COMPLETED_MASK 0x10 +# define BDSTAT_SOP_MASK 0x08 +# define BDSTAT_EOP_MASK 0x04 + +# define CHNL_STS_ERROR_MASK 0x80 + +/* All interrupt enable bits */ +#define XLLDMA_CR_IRQ_ALL_EN_MASK 0x00000087 +/* All interrupt bits */ +#define XLLDMA_IRQ_ALL_MASK 0x0000001F +/* Disable error when 2 or 4 bit coalesce counter overflows */ +#define XLLDMA_DMACR_RX_OVERFLOW_ERR_DIS_MASK 0x00000010 +/* Disable error when 2 or 4 bit coalesce counter overflows */ +#define XLLDMA_DMACR_TX_OVERFLOW_ERR_DIS_MASK 0x00000008 +/* Enable use of tail pointer register */ +#define XLLDMA_DMACR_TAIL_PTR_EN_MASK 0x00000004 + +#define LL_FIFO_ISR_RC_COMPLETE 0x04000000 + +#define SDMA_BIT 1 +#define DCR_BIT 2 + +#define DMAALIGN 32 + +/* SDMA Buffer Descriptor */ +struct cdmac_bd_t { + struct cdmac_bd_t *next_p; + unsigned char *phys_buf_p; + unsigned long buf_len; + unsigned char stat; + unsigned char app1_1; + unsigned short app1_2; + unsigned long app2; + unsigned long app3; + unsigned long app4; + unsigned long app5; +}; + +static struct cdmac_bd_t tx_bd __attribute((aligned(DMAALIGN))); +static struct cdmac_bd_t rx_bd __attribute((aligned(DMAALIGN))); + +struct ll_fifo_s { + u32 isr; /* Interrupt Status Register 0x0 */ + u32 ier; /* Interrupt Enable Register 0x4 */ + u32 tdfr; /* Transmit data FIFO reset 0x8 */ + u32 tdfv; /* Transmit data FIFO Vacancy 0xC */ + u32 tdfd; /* Transmit data FIFO 32bit wide data write port 0x10 */ + u32 tlf; /* Write Transmit Length FIFO 0x14 */ + u32 rdfr; /* Read Receive data FIFO reset 0x18 */ + u32 rdfo; /* Receive data FIFO Occupancy 0x1C */ + u32 rdfd; /* Read Receive data FIFO 32bit wide data read port 0x20 */ + u32 rlf; /* Read Receive Length FIFO 0x24 */ + u32 llr; /* Read LocalLink reset 0x28 */ +}; + +static u8 tx_buffer[PKTSIZE_ALIGN] __attribute((aligned(DMAALIGN))); +static u8 rx_buffer[PKTSIZE_ALIGN] __attribute((aligned(DMAALIGN))); + +struct temac_reg { + u32 reserved[8]; + u32 msw; /* Hard TEMAC MSW Data Register */ + u32 lsw; /* Hard TEMAC LSW Data Register */ + u32 ctl; /* Hard TEMAC Control Register */ + u32 rdy; /* Hard TEMAC Ready Status */ +}; + +struct ll_priv { + u32 ctrl; + u32 mode; + int phyaddr; + + struct phy_device *phydev; + struct mii_dev *bus; +}; + +#define XILINX_INDIRECT_DCR_ADDRESS_REG 0 +#define XILINX_INDIRECT_DCR_ACCESS_REG 1 + +static void mtdcr_local(u32 reg, u32 val) +{ +#if defined(CONFIG_PPC) + mtdcr(XILINX_INDIRECT_DCR_ADDRESS_REG, reg); + mtdcr(XILINX_INDIRECT_DCR_ACCESS_REG, val); +#endif +} + +static u32 mfdcr_local(u32 reg) +{ + u32 val = 0; +#if defined(CONFIG_PPC) + mtdcr(XILINX_INDIRECT_DCR_ADDRESS_REG, reg); + val = mfdcr(XILINX_INDIRECT_DCR_ACCESS_REG); +#endif + return val; +} + +static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val) +{ + if (priv->mode & DCR_BIT) + mtdcr_local(priv->ctrl + offset, val); + else + out_be32((u32 *)(priv->ctrl + offset * 4), val); +} + +static u32 sdma_in_be32(struct ll_priv *priv, u32 offset) +{ + if (priv->mode & DCR_BIT) + return mfdcr_local(priv->ctrl + offset); + + return in_be32((u32 *)(priv->ctrl + offset * 4)); +} + +static void xps_ll_temac_check_status(struct temac_reg *regs, int mask) +{ + u32 timeout = 2000; + while (timeout && (!(in_be32(®s->rdy) & mask))) + timeout--; + + if (!timeout) + printf("%s: Timeout\n", __func__); +} + +#if defined(DEBUG) || defined(CONFIG_MII) || defined(CONFIG_CMD_MII) +/* undirect hostif write to ll_temac */ +static void xps_ll_temac_hostif_set(struct eth_device *dev, int emac, + int phy_addr, int reg_addr, int phy_data) +{ + struct temac_reg *regs = (struct temac_reg *)dev->iobase; + + out_be32(®s->lsw, phy_data); + out_be32(®s->ctl, CNTLREG_WRITE_ENABLE_MASK | MIIMWD); + out_be32(®s->lsw, (phy_addr << 5) | reg_addr); + out_be32(®s->ctl, CNTLREG_WRITE_ENABLE_MASK | MIIMAI | (emac << 10)); + xps_ll_temac_check_status(regs, XTE_RSE_MIIM_WR_MASK); +} +#endif + +/* undirect hostif read from ll_temac */ +static unsigned int xps_ll_temac_hostif_get(struct eth_device *dev, + int emac, int phy_addr, int reg_addr) +{ + struct temac_reg *regs = (struct temac_reg *)dev->iobase; + + out_be32(®s->lsw, (phy_addr << 5) | reg_addr); + out_be32(®s->ctl, MIIMAI | (emac << 10)); + xps_ll_temac_check_status(regs, XTE_RSE_MIIM_RR_MASK); + return in_be32(®s->lsw); +} + +/* undirect write to ll_temac */ +static void xps_ll_temac_indirect_set(struct eth_device *dev, + int emac, int reg_offset, int reg_data) +{ + struct temac_reg *regs = (struct temac_reg *)dev->iobase; + + out_be32(®s->lsw, reg_data); + out_be32(®s->ctl, + CNTLREG_WRITE_ENABLE_MASK | (emac << 10) | reg_offset); + xps_ll_temac_check_status(regs, XTE_RSE_CFG_WR_MASK); +} + +/* undirect read from ll_temac */ +static int xps_ll_temac_indirect_get(struct eth_device *dev, + int emac, int reg_offset) +{ + struct temac_reg *regs = (struct temac_reg *)dev->iobase; + + out_be32(®s->ctl, (emac << 10) | reg_offset); + xps_ll_temac_check_status(regs, XTE_RSE_CFG_RR_MASK); + return in_be32(®s->lsw); +} + +#ifdef DEBUG +/* read from phy */ +static void read_phy_reg(struct eth_device *dev, int phy_addr) +{ + int j, result; + debug("phy%d ", phy_addr); + for (j = 0; j < 32; j++) { + result = xps_ll_temac_hostif_get(dev, 0, phy_addr, j); + debug("%d: 0x%x ", j, result); + } + debug("\n"); +} +#endif + +/* setting ll_temac and phy to proper setting */ +static int xps_ll_temac_phy_ctrl(struct eth_device *dev) +{ + int i; + unsigned int temp, speed; + struct ll_priv *priv = dev->priv; + struct phy_device *phydev; + + u32 supported = SUPPORTED_10baseT_Half | + SUPPORTED_10baseT_Full | + SUPPORTED_100baseT_Half | + SUPPORTED_100baseT_Full | + SUPPORTED_1000baseT_Half | + SUPPORTED_1000baseT_Full; + + if (priv->phyaddr == -1) { + for (i = 31; i >= 0; i--) { + temp = xps_ll_temac_hostif_get(dev, 0, i, 1); + if ((temp & 0x0ffff) != 0x0ffff) { + debug("phy %x result %x\n", i, temp); + priv->phyaddr = i; + break; + } + } + } + + /* interface - look at tsec */ + phydev = phy_connect(priv->bus, priv->phyaddr, dev, 0); + + phydev->supported &= supported; + phydev->advertising = phydev->supported; + priv->phydev = phydev; + phy_config(phydev); + phy_startup(phydev); + + switch (phydev->speed) { + case 1000: + speed = XTE_EMMC_LINKSPD_1000; + break; + case 100: + speed = XTE_EMMC_LINKSPD_100; + break; + case 10: + speed = XTE_EMMC_LINKSPD_10; + break; + default: + return 0; + } + temp = xps_ll_temac_indirect_get(dev, 0, EMMC); + temp &= ~XTE_EMMC_LINKSPEED_MASK; + temp |= speed; + xps_ll_temac_indirect_set(dev, 0, EMMC, temp); + + return 1; +} + +static inline int xps_ll_temac_dma_error(struct eth_device *dev) +{ + int err; + struct ll_priv *priv = dev->priv; + + /* Check for TX and RX channel errrors. */ + err = sdma_in_be32(priv, TX_CHNL_STS) & CHNL_STS_ERROR_MASK; + err |= sdma_in_be32(priv, RX_CHNL_STS) & CHNL_STS_ERROR_MASK; + return err; +} + +static void xps_ll_temac_reset_dma(struct eth_device *dev) +{ + u32 r; + struct ll_priv *priv = dev->priv; + + /* Soft reset the DMA. */ + sdma_out_be32(priv, DMA_CONTROL_REG, DMA_CONTROL_RESET); + while (sdma_in_be32(priv, DMA_CONTROL_REG) & DMA_CONTROL_RESET) + ; + + /* Now clear the interrupts. */ + r = sdma_in_be32(priv, TX_CHNL_CTRL); + r &= ~XLLDMA_CR_IRQ_ALL_EN_MASK; + sdma_out_be32(priv, TX_CHNL_CTRL, r); + + r = sdma_in_be32(priv, RX_CHNL_CTRL); + r &= ~XLLDMA_CR_IRQ_ALL_EN_MASK; + sdma_out_be32(priv, RX_CHNL_CTRL, r); + + /* Now ACK pending IRQs. */ + sdma_out_be32(priv, TX_IRQ_REG, XLLDMA_IRQ_ALL_MASK); + sdma_out_be32(priv, RX_IRQ_REG, XLLDMA_IRQ_ALL_MASK); + + /* Set tail-ptr mode, disable errors for both channels. */ + sdma_out_be32(priv, DMA_CONTROL_REG, + XLLDMA_DMACR_TAIL_PTR_EN_MASK | + XLLDMA_DMACR_RX_OVERFLOW_ERR_DIS_MASK | + XLLDMA_DMACR_TX_OVERFLOW_ERR_DIS_MASK); +} + +/* bd init */ +static void xps_ll_temac_bd_init(struct eth_device *dev) +{ + struct ll_priv *priv = dev->priv; + memset(&tx_bd, 0, sizeof(tx_bd)); + memset(&rx_bd, 0, sizeof(rx_bd)); + + rx_bd.phys_buf_p = rx_buffer; + + rx_bd.next_p = &rx_bd; + rx_bd.buf_len = PKTSIZE_ALIGN; + flush_cache((u32)&rx_bd, sizeof(tx_bd)); + flush_cache((u32)rx_bd.phys_buf_p, PKTSIZE_ALIGN); + + sdma_out_be32(priv, RX_CURDESC_PTR, (u32)&rx_bd); + sdma_out_be32(priv, RX_TAILDESC_PTR, (u32)&rx_bd); + sdma_out_be32(priv, RX_NXTDESC_PTR, (u32)&rx_bd); /* setup first fd */ + + tx_bd.phys_buf_p = tx_buffer; + tx_bd.next_p = &tx_bd; + + flush_cache((u32)&tx_bd, sizeof(tx_bd)); + sdma_out_be32(priv, TX_CURDESC_PTR, (u32)&tx_bd); +} + +static int ll_temac_send_sdma(struct eth_device *dev, + volatile void *buffer, int length) +{ + struct ll_priv *priv = dev->priv; + + if (xps_ll_temac_dma_error(dev)) { + xps_ll_temac_reset_dma(dev); + xps_ll_temac_bd_init(dev); + } + + memcpy(tx_buffer, (void *)buffer, length); + flush_cache((u32)tx_buffer, length); + + tx_bd.stat = BDSTAT_SOP_MASK | BDSTAT_EOP_MASK | + BDSTAT_STOP_ON_END_MASK; + tx_bd.buf_len = length; + flush_cache((u32)&tx_bd, sizeof(tx_bd)); + + sdma_out_be32(priv, TX_CURDESC_PTR, (u32)&tx_bd); + sdma_out_be32(priv, TX_TAILDESC_PTR, (u32)&tx_bd); /* DMA start */ + + do { + flush_cache((u32)&tx_bd, sizeof(tx_bd)); + } while (!(tx_bd.stat & BDSTAT_COMPLETED_MASK)); + + return 0; +} + +static int ll_temac_recv_sdma(struct eth_device *dev) +{ + int length; + struct ll_priv *priv = dev->priv; + + if (xps_ll_temac_dma_error(dev)) { + xps_ll_temac_reset_dma(dev); + xps_ll_temac_bd_init(dev); + } + + flush_cache((u32)&rx_bd, sizeof(rx_bd)); + + if (!(rx_bd.stat & BDSTAT_COMPLETED_MASK)) + return 0; + + /* Read out the packet info and start the DMA + onto the second buffer to enable the ethernet rx + path to run in parallel with sw processing + packets. */ + length = rx_bd.app5 & 0x3FFF; /* max length mask */ + if (length > 0) + NetReceive(rx_bd.phys_buf_p, length); + + /* flip the buffer and re-enable the DMA. */ + flush_cache((u32)rx_bd.phys_buf_p, length); + + rx_bd.buf_len = PKTSIZE_ALIGN; + rx_bd.stat = 0; + rx_bd.app5 = 0; + + flush_cache((u32)&rx_bd, sizeof(rx_bd)); + sdma_out_be32(priv, RX_TAILDESC_PTR, (u32)&rx_bd); + + return length; +} + +#ifdef DEBUG +static void debugll(struct eth_device *dev, int count) +{ + struct ll_priv *priv = dev->priv; + struct ll_fifo_s *ll_fifo = (void *)priv->ctrl; + printf("%d fifo isr 0x%08x, fifo_ier 0x%08x, fifo_rdfr 0x%08x, " + "fifo_rdfo 0x%08x fifo_rlr 0x%08x\n", count, + in_be32(&ll_fifo->isr), in_be32(&ll_fifo->ier), + in_be32(&ll_fifo->rdfr), in_be32(&ll_fifo->rdfo), + in_be32(&ll_fifo->rlf)); +} +#endif + +static int ll_temac_send_fifo(struct eth_device *dev, + volatile void *buffer, int length) +{ + struct ll_priv *priv = dev->priv; + struct ll_fifo_s *ll_fifo = (void *)priv->ctrl; + u32 *buf = (u32 *)buffer; + u32 i; + + for (i = 0; i < length; i += 4) + out_be32(&ll_fifo->tdfd, *buf++); + + out_be32(&ll_fifo->tlf, length); + return 0; +} + +static int ll_temac_recv_fifo(struct eth_device *dev) +{ + struct ll_priv *priv = dev->priv; + struct ll_fifo_s *ll_fifo = (void *)priv->ctrl; + u32 i, len = 0; + u32 *buf = (u32 *)&rx_buffer; + + if (in_be32(&ll_fifo->isr) & LL_FIFO_ISR_RC_COMPLETE) { + out_be32(&ll_fifo->isr, 0xffffffff); /* reset isr */ + + /* while (ll_fifo->isr); */ + len = in_be32(&ll_fifo->rlf) & 0x7FF; + + for (i = 0; i < len; i += 4) + *buf++ = in_be32(&ll_fifo->rdfd); + +#ifdef DEBUG + debugll(dev, 1); +#endif + NetReceive((uchar *)&rx_buffer, len); + } + return len; +} + +/* setup mac addr */ +static int ll_temac_addr_setup(struct eth_device *dev) +{ + int val; + + /* set up unicast MAC address filter */ + val = ((dev->enetaddr[3] << 24) | (dev->enetaddr[2] << 16) | + (dev->enetaddr[1] << 8) | (dev->enetaddr[0])); + xps_ll_temac_indirect_set(dev, 0, UAW0, val); + val = (dev->enetaddr[5] << 8) | dev->enetaddr[4] ; + xps_ll_temac_indirect_set(dev, 0, UAW1, val); + + return 0; +} + +static int xps_ll_temac_init(struct eth_device *dev, bd_t *bis) +{ + struct ll_priv *priv = dev->priv; + struct ll_fifo_s *ll_fifo = (void *)priv->ctrl; + + if (priv->mode & SDMA_BIT) { + xps_ll_temac_reset_dma(dev); + xps_ll_temac_bd_init(dev); + } else { + out_be32(&ll_fifo->tdfr, 0x000000a5); /* Fifo reset key */ + out_be32(&ll_fifo->rdfr, 0x000000a5); /* Fifo reset key */ + out_be32(&ll_fifo->isr, 0xFFFFFFFF); /* Reset status register */ + out_be32(&ll_fifo->ier, 0); /* Disable all IRQs */ + } + + xps_ll_temac_indirect_set(dev, 0, MC, + MDIO_ENABLE_MASK | MDIO_CLOCK_DIV_100MHz); + + /* Promiscuous mode disable */ + xps_ll_temac_indirect_set(dev, 0, AFM, 0); + /* Enable Receiver - RX bit */ + xps_ll_temac_indirect_set(dev, 0, RCW1, 0x10000000); + /* Enable Transmitter - TX bit */ + xps_ll_temac_indirect_set(dev, 0, TC, 0x10000000); + return 0; +} + +/* halt device */ +static void ll_temac_halt(struct eth_device *dev) +{ +#ifdef ETH_HALTING + struct ll_priv *priv = dev->priv; + /* Disable Receiver */ + xps_ll_temac_indirect_set(dev, 0, RCW1, 0); + /* Disable Transmitter */ + xps_ll_temac_indirect_set(dev, 0, TC, 0); + + if (priv->mode & SDMA_BIT) { + sdma_out_be32(priv->ctrl, DMA_CONTROL_REG, DMA_CONTROL_RESET); + while (sdma_in_be32(priv->ctrl, DMA_CONTROL_REG) + & DMA_CONTROL_RESET) + ; + } +#endif +} + +static int ll_temac_init(struct eth_device *dev, bd_t *bis) +{ +#if DEBUG + int i; +#endif + xps_ll_temac_init(dev, bis); + + printf("%s: Xilinx XPS LocalLink Tri-Mode Ether MAC #%d at 0x%08X.\n", + dev->name, 0, dev->iobase); + +#if DEBUG + for (i = 0; i < 32; i++) + read_phy_reg(dev, i); +#endif + + if (!xps_ll_temac_phy_ctrl(dev)) { + ll_temac_halt(dev); + return -1; + } + + return 0; +} + +static int ll_temac_miiphy_read(const char *devname, uchar addr, + uchar reg, ushort *val) +{ + struct eth_device *dev = eth_get_dev(); + + *val = xps_ll_temac_hostif_get(dev, 0, addr, reg); /* emac = 0 */ + + debug("%s 0x%x, 0x%x, 0x%x\n", __func__, addr, reg, *val); + return 0; +} + +static int ll_temac_miiphy_write(const char *devname, uchar addr, + uchar reg, ushort val) +{ + struct eth_device *dev = eth_get_dev(); + debug("%s 0x%x, 0x%x, 0x%x\n", __func__, addr, reg, val); + + xps_ll_temac_hostif_set(dev, 0, addr, reg, val); + + return 0; +} + +static int ll_temac_bus_reset(struct mii_dev *bus) +{ + debug("Just bus reset\n"); + return 0; +} + +/* mode bits: 0bit - fifo(0)/sdma(1):SDMA_BIT, 1bit - no dcr(0)/dcr(1):DCR_BIT + * ctrl - control address for file/sdma */ +int xilinx_ll_temac_initialize(bd_t *bis, unsigned long base_addr, + int mode, unsigned long ctrl) +{ + struct eth_device *dev; + struct ll_priv *priv; + + dev = calloc(1, sizeof(*dev)); + if (dev == NULL) + return -1; + + dev->priv = calloc(1, sizeof(struct ll_priv)); + if (dev->priv == NULL) { + free(dev); + return -1; + } + + priv = dev->priv; + + sprintf(dev->name, "Xlltem.%lx", base_addr); + + dev->iobase = base_addr; + priv->ctrl = ctrl; + priv->mode = mode; + +#ifdef CONFIG_PHY_ADDR + priv->phyaddr = CONFIG_PHY_ADDR; +#else + priv->phyaddr = -1; +#endif + + dev->init = ll_temac_init; + dev->halt = ll_temac_halt; + dev->write_hwaddr = ll_temac_addr_setup; + + if (priv->mode & SDMA_BIT) { + dev->send = ll_temac_send_sdma; + dev->recv = ll_temac_recv_sdma; + } else { + dev->send = ll_temac_send_fifo; + dev->recv = ll_temac_recv_fifo; + } + + eth_register(dev); + +#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) + miiphy_register(dev->name, ll_temac_miiphy_read, ll_temac_miiphy_write); + priv->bus = miiphy_get_dev_by_name(dev->name); + priv->bus->reset = ll_temac_bus_reset; +#endif + return 1; +} diff --git a/include/netdev.h b/include/netdev.h index 26ce13d..e976659 100644 --- a/include/netdev.h +++ b/include/netdev.h @@ -92,6 +92,8 @@ int uec_standard_init(bd_t *bis); int uli526x_initialize(bd_t *bis); int xilinx_emaclite_initialize(bd_t *bis, unsigned long base_addr, int txpp, int rxpp); +int xilinx_ll_temac_initialize(bd_t *bis, unsigned long base_addr, + int mode, unsigned long ctrl); int sh_eth_initialize(bd_t *bis); int dm9000_initialize(bd_t *bis); int fecmxc_initialize(bd_t *bis);
LL Temac driver can be used by microblaze, xilinx ppc405/440 in sdma and fifo mode. DCR or XPS bus can be used. The driver uses and requires PHYLIB. Signed-off-by: Michal Simek <monstr@monstr.eu> --- v2: Remove helper function for access to temac Remove SDMA/FIFO/DCR macros and configure it in board Setup mac by write_hwaddr v3: Use helper functions for fifo mode Use helper functions for indirect accesses Code cleanup Add comments for MAGIC values Simplify code in fifo mode --- drivers/net/Makefile | 1 + drivers/net/xilinx_ll_temac.c | 666 +++++++++++++++++++++++++++++++++++++++++ include/netdev.h | 2 + 3 files changed, 669 insertions(+), 0 deletions(-) create mode 100644 drivers/net/xilinx_ll_temac.c