[U-Boot-Users] SPI support in U-boot
Hello,
It looks like the basic SPI I/O function, spi_xfer() has two different prototypes in U-boot.
1. in common/soft_spi.c, include/spi.h and cpu/nio/spi.c it is declared as
int spi_xfer(spi_chipsel_type chipsel, int bitlen, uchar *dout, uchar *din)
2. in SPI drivers for MPC CPUs (cpu/mpc{8xx, 8260, 5xxx}/spi.c) it is declared as
ssize_t spi_xfer (size_t count)
The code for "sspi" command (do_spi()) definitely uses the first prototype. Subsequently it doesn't work on the above mentioned CPUs (unless you use software SPI implementation).
On the other hand, the code for the "eeprom" command, which is another way to access an SPI EEPROM uses functions spi_read() and spi_write, which are implemented only in cpu/mpc{8xx, 8260, 5xxx}/spi.c. However, these functions are very basic. Fore example they do not allow you to control SPI chip select.
Can someone tell me what is going on. Is it that MPC drivers became outdated and should be modified? How do other people use them then? Or I am simply missing something?
Thanks, Vladimir
In message 43D44482.7030300@paulidav.org you wrote:
It looks like the basic SPI I/O function, spi_xfer() has two different prototypes in U-boot.
This is what happens when two groups of people solve the same problem independently.
On the other hand, the code for the "eeprom" command, which is another way to access an SPI EEPROM uses functions spi_read() and spi_write, which are implemented only in cpu/mpc{8xx, 8260, 5xxx}/spi.c. However, these functions are very basic. Fore example they do not allow you to control SPI chip select.
That's because they just deal with that: accessing a SPI EEPROM device.
Can someone tell me what is going on. Is it that MPC drivers became outdated and should be modified? How do other people use them then? Or I am simply missing something?
They are not "outdated". It's just a different (and incompatible) implementation. If you can come up with a patch tp cleanup please do so.
Best regards,
Wolfgang Denk
Hello Wolfgang,
Wolfgang Denk wrote:
They are not "outdated". It's just a different (and incompatible) implementation. If you can come up with a patch tp cleanup please do so.
I decided to do that (and it was pretty easy to do), but now I have even more questions...
The major issue is the way chip selects are controlled. Currently, do_spi() function that implements "sspi" command calls spi_xfer() this way:
spi_xfer(spi_chipsel[device], bitlen, dout, din)
where spi_chipsel is a global array of pointers to functions that are supposed to assert/de-assert chip selects for the specified target(s).
I looked at the code for the boards that use this mechanism, and I can see the array statically initialized, like (in board/sacsng/sacsng.c):
/* * The SPI command uses this table of functions for controlling the SPI * chip selects: it calls the appropriate function to control the SPI * chip selects. */ spi_chipsel_type spi_chipsel[] = { spi_adc_chipsel, spi_dac_chipsel }; int spi_chipsel_cnt = sizeof(spi_chipsel) / sizeof(spi_chipsel[0]);
My question is: where these addresses are relocated? My understanding is that relocation for this type of data should be done manually, but nowhere in the code can I see it. Not for a single board. That means that if people got lucky, they execute the copy of the code from the FLASH, not the relocated one.
Is that OK? I also noticed the same mechanism being used in the FPGA-related code.
And another question. The current implementation(s) of the "eeprom" command assume that there is only 1 SPI device and do not bothr themselves with the chip selects at all. That means, that if you try to execute "eeprom" command after you executed "sspi" (that will de-assert the chip-select at the end or can choose a different one), the results will be unpredictable. I have no problem modifying "eeprom" command for my board, but this will force other people to do modifications as well, so I am not sure what should we do.
Thanks, Vladimir
Dear Vladimir,
in message 43D733BF.4030207@paulidav.org you wrote:
I looked at the code for the boards that use this mechanism, and I can see the array statically initialized, like (in board/sacsng/sacsng.c):
...
My question is: where these addresses are relocated? My understanding is that relocation for this type of data should be done manually, but nowhere in the code can I see it. Not for a single board. That means that if people got lucky, they execute the copy of the code from the FLASH, not the relocated one.
Good catch!
Is that OK? I also noticed the same mechanism being used in the FPGA-related code.
I think your interpretation is correct.
And another question. The current implementation(s) of the "eeprom" command assume that there is only 1 SPI device and do not bothr themselves with the chip selects at all. That means, that if you try to execute "eeprom" command after you executed "sspi" (that will de-assert the chip-select at the end or can choose a different one), the results will be unpredictable. I have no problem modifying "eeprom" command for my board, but this will force other people to do modifications as well, so I am not sure what should we do.
It would be nice to preserve the currnt behaviour, so a patch should try to implement the required changes for all boiards. Testing will have to be performed by the respective board maintainers, of course.
Best regards,
Wolfgang Denk
Hello Wolfgang,
Wolfgang Denk wrote:
Good catch!
Is that OK? I also noticed the same mechanism being used in the FPGA-related code.
I think your interpretation is correct.
And? :)
OK, so here are the questions:
1. Should the code be left like that (i.e. some parts are executing from FLASH instead of RAM) because it "just works for me" or it is not acceptable to code like that? 2. What is the right way to do that: to use an uninitialized variable and initialize it from board_misc_init_r with something like
myfuncptr = &myfunc + bd->reloc_off
or to have initialized variable myfuncptr = myfunc and simply do myfuncptr += bd->reloc_off in board_misc_init_r? 3. Ironically enough, I noticed that fpga_init() is declared like extern void fpga_init( ulong reloc_off ), but all the implementations use void fpga_init(void)! 4. Wouldn't it make sense to add a small section to README, explaining how to use function pointer tables in U-boot (not sure how much sense it makes to begin with).
It would be nice to preserve the currnt behaviour, so a patch should try to implement the required changes for all boiards. Testing will have to be performed by the respective board maintainers, of course.
This seems to be difficult, since the current interface only allows you to assert/de-assert a particular chipselect, but not to query which one is currently active (so that "sspi" command could restore what was before it. I guess, I can do three things:
1. Extend sspi command, so that if there is only 1 arg, chip_select, it will simply assert it and do nothing else 2. Hope, that people who use "eeprom" command don't really care about "sspi" so far, otherwise it would be working! 3. Document the changes.
Thanks, Vladimir
participants (2)
-
Vladimir Gurevich -
Wolfgang Denk