[PATCH linux v3 16/18] drivers/fsi: Set up CFAMs for communication

Christopher Bostic christopher.lee.bostic at gmail.com
Fri Oct 14 03:26:28 AEDT 2016


On Wed, Oct 12, 2016 at 8:28 PM, Jeremy Kerr <jk at ozlabs.org> wrote:
> Hi Chris,
>

>>>> diff --git a/drivers/fsi/fsi-slave.h b/drivers/fsi/fsi-slave.h
>>>> new file mode 100644
>>>> index 0000000..ea7a760
>>>> --- /dev/null
>>>> +++ b/drivers/fsi/fsi-slave.h
>>>> @@ -0,0 +1,62 @@
>>>> +/*
>>>> + * FSI slave engine definitions.
>>>> + *
>>>
>>> Why does this need to be in a header file? Isn't it only used by the FSI
>>> core?
>>
>> I suppose this is me not being familiar with the common coding practices
>> of the Linux community.  I would typically put constants and structs in an
>> associated header file even if there was no intent to have them be used
>> by anything outside the core.   Is it a requirement that this all be moved
>> into fsi-core.c as a means of limiting accessibility?
>
> It's not a requirement, but a strong convention. Only use headers for
> stuff that you need to share across other files; this gives future
> developers a definite indication of the scope of these macros, consts
> and structs.
>
> The less that is globally accessible, the better.
>

OK will move that all into fsi-core.c

Thanks,

Chris

> Regards,
>
>
> Jeremy


More information about the openbmc mailing list