From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754624AbcFTQOe (ORCPT ); Mon, 20 Jun 2016 12:14:34 -0400 Received: from mail-wm0-f53.google.com ([74.125.82.53]:36061 "EHLO mail-wm0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753006AbcFTPms (ORCPT ); Mon, 20 Jun 2016 11:42:48 -0400 Subject: Re: [PATCH net-next] net: ena: Add a driver for Amazon Elastic Network Adapters (ENA) To: "Rosen, Rami" , "netdev@vger.kernel.org" , "davem@davemloft.net" , "linux-kernel@vger.kernel.org" References: <1465807573-11293-1-git-send-email-netanel@annapurnalabs.com> <9B0331B6EBBD0E4684FBFAEDA55776F92CD6858D@HASMSX110.ger.corp.intel.com> Cc: "zorik@annapurnalabs.com" , "saeed@annapurnalabs.com" , "alex@annapurnalabs.com" , "msw@amazon.com" , "aligouri@amazon.com" , "antoine.tenart@free-electrons.com" From: Netanel Belgazal Message-ID: <7f753555-eb8d-b902-ade8-efee286fe81b@annapurnalabs.com> Date: Mon, 20 Jun 2016 18:42:32 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <9B0331B6EBBD0E4684FBFAEDA55776F92CD6858D@HASMSX110.ger.corp.intel.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/20/2016 08:20 AM, Rosen, Rami wrote: > Hi all, > > A very limited review below. > > + > + /* get capabilities of particular feature */ > + ENA_ADMIN_GET_FEATURE = 8, > > Instead /* get capabilities SHOULD BE: /* set capabilities ..... > + > + /* get capabilities of particular feature */ > + ENA_ADMIN_SET_FEATURE = 9, > + > .. > > > +int ena_com_set_hash_ctrl(struct ena_com_dev *ena_dev) > +{ > + struct ena_com_admin_queue *admin_queue = &ena_dev->admin_queue; > ... > > You set ret=-EINVAL, but you do not use this ret as you immediately return 0 in the next line, which is the end of the method. Either return ret or return -EINVAL. > + if (unlikely(ret)) { > + ena_trc_err("Failed to set hash input. error: %d\n", ret); > + ret = -EINVAL; > + } > + > + return 0; > +} > + I will return ret. > > > + > +/* ena_com_set_mmio_read_mode - Enable/disable the mmio reg read mechanism > + * @ena_dev: ENA communication layer struct > > > Instead realess_supported: SHOULD BE: readless_supported ack > > + * @realess_supported: readless mode (enable/disable) > + */ > +void ena_com_set_mmio_read_mode(struct ena_com_dev *ena_dev, > + bool readless_supported); > + > > > > + > +/* ena_com_create_io_queue - Create io queue. > + * @ena_dev: ENA communication layer struct > > Instead ena_com_create_io_ctx SHOULD BE: @ena_com_create_io_ctx ack > > + * ena_com_create_io_ctx - create context structure > + * > + * Create the submission and the completion queues. > + * > + * @return - 0 on success, negative value on failure. > + */ > +int ena_com_create_io_queue(struct ena_com_dev *ena_dev, > + struct ena_com_create_io_ctx *ctx); > + > > > > > +/* ena_com_admin_destroy - Destroy IO queue with the queue id - qid. > + * @ena_dev: ENA communication layer struct > > Missing: @qid ack > > + */ > +void ena_com_destroy_io_queue(struct ena_com_dev *ena_dev, u16 qid); > + > > Regards, > Rami Rosen > Intel Corporation Thanks for the review