Hi Greg, Thanks for review. On 03/15/2018 11:48 AM, Greg KH wrote: > On Thu, Mar 01, 2018 at 06:19:31PM -0600,richard.gong@linux.intel.com wrote: >> +EXPORT_SYMBOL_GPL(request_svc_channel_byname); > All of your global symbols should have the same prefix, not a verb. So > this should be: > svc_channel_request_by_name(), right? > >> +EXPORT_SYMBOL_GPL(free_svc_channel); > svc_channel_free()? > >> +EXPORT_SYMBOL_GPL(intel_svc_send); > Wait, why doesn't the first ones have "intel_svc" in them? > > What is the difference here? > > Should they all just have "intel_svc_" as the prefix? > > Stick to one thing for all exported functions/variables please, it's the > only way we can attempt to keep our namespace "sane". Sure, I will make changes so that all exported functions will have "intel_svc_" as the prefix. > Also, why would a 'misc' driver be exporting things that something else > uses? Why not just put this in the fpga directory next to the users of > this code? Intel Stratix10 service layer driver, running at privileged exception level (EL1), interfaces with the service providers and providers the services for FPGA configuration, QSPI, Crypto, ECC, and warm reset. Service layer driver also manages secure monitor call to communicate with secure monitor code running at EL3. FPGA manager is the first service provider. Later we will add several service providers, and extend service layer driver to provide the services for QSPI, Crypto, ECC and warm reset. Regards, Richard > thanks, > > greg k-h