Hi Gerhard,
On Thu, 2013-12-19 at 21:03 +0100, Gerhard Sittig wrote:
On Thu, Dec 19, 2013 at 08:46 -0600, Chin Liang See wrote:
Hi Gerhard,
On Thu, 2013-12-19 at 14:50 +0100, Gerhard Sittig wrote:
On Wed, Dec 18, 2013 at 14:05 -0600, Chin Liang See wrote:
To add the Cadence SPI driver support for Altera SOCFPGA. It required information such as clocks and timing from platform's configuration header file within include/configs folder
[ ... ]
Can you please add the information which header file is required? And do I get it right that this header file does not come with the source but is provided "externally" to the U-Boot project?
Oh actually its part of the header file within include/configs. For our case, it would be include/configs/socfpga_cyclone5.h. Its not external or generated file.
Ah, thank you for the explanation. So the board's configuration file is referenced as usual. That's OK.
But it would need macro in order to get some customization to the driver. Wonder would it be good we need to document the required macros? Or just the standard way where people normally just grep macro used in others platform header file (which I normally did).
Well, grepping sources may not always be as obvious as code authors may think. :) Some textual description with the complete set of possible options, their type and units would be nice. It's hard to guess for e.g. clocks whether a number is cycles or nanoseconds or a frequency or bit times or any other arbitrary thing that may need to get written to hardware with or without any further conversion.
Yup, I totally agree with you on this as I went through the pain before. To solve this, I will create a short documentation at doc/spi folder. I will resubmit my v2 patch with this documentation.
In current master's socfpga_cyclone5.h I can't see any SPI related defines, and your patch set does not update this file. So I'm still afraid that simply enabling the controller in the config won't result in successful build output. And waiting for compile errors is the only way to learn when settings are missing. And still you won't notice when settings are wrong (like booleans). This would be unsatisfying an experience. Users should not have to read and reverse engineer code just to find out how to enable and use it.
To cut it short, please provide example entries in the cyclone5 board configuration, and a either a text document or a comment in the driver source listing all options and their meaning.
Actually its not inside cyclone5 board configuration as there are few patches for the platform specific yet to be pushed. I am planning to submit once we cleared the pending patches. With that said, the documentation is a good suggestion as we can put more info there.
Thanks again for your helps and feedback
Chin Liang
virtually yours Gerhard Sittig