[Intel-wired-lan] [PATCH v3 02/11] igc: Add support for PF

[Shannon Nelson]

On 7/2/2018 1:57 AM, Neftin, Sasha wrote:
> On 6/27/2018 03:32, Shannon Nelson wrote:
>> On 6/24/2018 1:45 AM, Sasha Neftin wrote:
>>> This patch adds the basic defines and structures needed by the PF for
>>> operation.
>>

[...]

>>> +
>>> +/* forward declaration */
>>> +struct e1000_hw;
>>> +u32 igc_rd32(struct e1000_hw *hw, u32 reg);
>>
>> Can we do this without the forward decls?
>>
> Can you advice another place?

Yeah, this one is my mistake, putting the declarations of functions used 
in other files into the headerfile is fine.

[...]

>>> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c 
>>> b/drivers/net/ethernet/intel/igc/igc_main.c
>>> index 3e10d27752d5..ec3451dad91e 100644
>>> --- a/drivers/net/ethernet/intel/igc/igc_main.c
>>> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
>>> @@ -25,6 +25,69 @@ static const struct pci_device_id igc_pci_tbl[] = {
>>>   MODULE_DEVICE_TABLE(pci, igc_pci_tbl);
>>> +/* Forward declaration */
>>> +static int igc_sw_init(struct igc_adapter *);
>>
>> Can we do this without the forward decls?
>>
> Can you advice another place?

It's these in the .c file that bother me.  For the most part, you should 
be able to order the functions in the file such that anything used by 
one functions has already be defined above it, and then these are not 
needed.  This way, the starting or "main" functions e.g. probe and 
release, are at the bottom of the file, and simple little utility 
functions are near the top.  This is not an absolute standard, but a 
general custom.

sln