[U-Boot] [PATCH] spi/cadence: Adding Cadence SPI driver support for SOCFPGA

[Chin Liang See]

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