[dpdk-dev] [PATCH v7 02/28] librte_ether: add fields from rte_pci_driver to rte_eth_dev_data

[Thomas Monjalon]

2015-11-02 16:44, Iremonger, Bernard:
> Hi Thomas,
> 
> <snip>
> 
> > Subject: Re: [dpdk-dev] [PATCH v7 02/28] librte_ether: add fields from
> > rte_pci_driver to rte_eth_dev_data
> > 
> > 2015-11-02 10:36, Iremonger, Bernard:
> > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > 2015-10-30 15:08, Bernard Iremonger:
> > > > > add dev_flags to rte_eth_dev_data, add macros for dev_flags.
> > > > > add kdrv to rte_eth_dev_data.
> > > > > add numa_node to rte_eth_dev_data.
> > > > > add drv_name to rte_eth_dev_data.
> > > >
> > > > A commit message should explain why things are done.
> > > >
> > > > > +	uint32_t dev_flags; /**< Flags controlling handling of device.
> > > > > +*/
> > > >
> > > > Where are defined this flags? What is the scope?
> > >
> > > These flags are defined in the following file:
> > >
> > > lib/librte_ether/rte_ethdev.h
> > >
> > > These flags are visible to all the vdevs and pdevs.
> > 
> > I mean it should be more explicit. Having an enum name will help.
> > Note: I understand your patch. I'm just asking the questions an user will ask
> > when trying to use your new API.
> > Thanks
> 
> I will try to be clearer in the commit message.
> I tried an enum for the dev_flags but it does not work well.
> There can be multiple flags set in dev_flags, it is intended to be a bit field similar to the pci flags.

Yes. You can assign some bits in an enum.
But if you prefer the defines, it's also OK if they are clearly identifiable:
with a prefix and good comments allowing to map the structure field and the flags.

> I have squashed down to 19 patches now, I don't want to over squash.

The proposal of 5 patches was good.