Re: [PATCHv2 3/7] driver, misc: add Intel Stratix10 service layer driver

[Richard Gong <richard.gong@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1530 bytes --]

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


[-- Attachment #2: Type: text/html, Size: 2590 bytes --]