Re: [dpdk-dev] [PATCH v3 4/8] raw/ioat: create device on probe and destroy on release

[Bruce Richardson <bruce.richardson@intel.com>]

On Thu, Jun 27, 2019 at 01:09:03PM +0100, Burakov, Anatoly wrote:
> On 27-Jun-19 11:40 AM, Bruce Richardson wrote:
> > Add the create/destroy driver functions so that we can actually allocate
> > a rawdev and destroy it when done. No rawdev API functions are actually
> > implemented at this point.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> 
> <snip>
> 
> >   ioat_rawdev_create(const char *name, struct rte_pci_device *dev)
> >   {
> > -	RTE_SET_USED(name);
> > -	RTE_SET_USED(dev);
> > +	static const struct rte_rawdev_ops ioat_rawdev_ops = {
> > +	};
> > +
> > +	struct rte_rawdev *rawdev = NULL;
> > +	struct rte_ioat_rawdev *ioat = NULL;
> > +	int ret = 0;
> > +	int retry = 0;
> > +
> > +	if (!name) {
> > +		IOAT_PMD_ERR("Invalid name of the device!");
> > +		ret = -EINVAL;
> > +		goto cleanup;
> > +	}
> 
> Is checking `dev` not necessary here?
> 

No, the only place it's called from is already using the PCI device
structure. I don't think the probe function can ever be called with a NULL
parameter.

> > +
> > +	/* Allocate device structure */
> > +	rawdev = rte_rawdev_pmd_allocate(name, sizeof(struct rte_ioat_rawdev),
> > +					 dev->device.numa_node);
> > +	if (rawdev == NULL) {
> > +		IOAT_PMD_ERR("Unable to allocate raw device");
> > +		ret = -EINVAL;
> > +		goto cleanup;
> 
> EINVAL is supposed to be used to indicate invalid arguments. Inability to
> allocate is not an "invalid arguments" condition. Does
> rte_rawdev_pmd_allocate() set its own errno value? If so, perhaps it would
> be worth passing it on? If not, perhaps -ENOMEM would be a better return
> value?
> 
Fixing in v4.