From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 95494A054F; Wed, 25 May 2022 11:01:20 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 85FCB40C35; Wed, 25 May 2022 11:01:20 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 4F6484067B for ; Wed, 25 May 2022 11:01:19 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id F03A78F; Wed, 25 May 2022 12:01:18 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru F03A78F DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1653469279; bh=/q2Nu3jzb68OdTmeYgAgTOtjW1p6Tu6eg6ntzv0EMtY=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=FS5TvYd+2SQVPLlMf7rmwA6Ywu+HAHBDKRlVudbbWiaJCk1s5s13lZOzERqM9ta8/ B2k7RfRGTxCYgesoptePsyUBJZR0iKsXOC0eO7LRPWdVaMg0aw69P8clY6HddFNSVO 06yIPk7roqMuwB5M3uSfNuhMnR/fQwAZjRsBCwOU= Message-ID: <2352ca6c-634a-0bc5-4361-04c1784dd335@oktetlabs.ru> Date: Wed, 25 May 2022 12:01:18 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH v8 5/5] ethdev: format module EEPROM for SFF-8636 Content-Language: en-US To: Robin Zhang , dev@dpdk.org Cc: thomas@monjalon.net, kevinx.liu@intel.com References: <20220524062442.194809-1-robinx.zhang@intel.com> <20220525031446.72578-1-robinx.zhang@intel.com> <20220525031446.72578-6-robinx.zhang@intel.com> From: Andrew Rybchenko Organization: OKTET Labs In-Reply-To: <20220525031446.72578-6-robinx.zhang@intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 5/25/22 06:14, Robin Zhang wrote: > This patch implements format module EEPROM information for > SFF-8636 Rev 2.7 > > Signed-off-by: Robin Zhang [snip] > + /* > + * There is no clear identifier to signify the existence of > + * optical diagnostics similar to SFF-8472. So checking existence > + * of page 3, will provide the gurantee for existence of alarms typo: gurantee -> guarantee (found by checkpatches.sh) > +const uint8_t rx_power_offset[MAX_CHANNEL_NUM] = { I think it should be "static". You don't need it outside of corresponding C file. Also, please, add a namespace prefix "sff_8636_" to make it easier in the code to understand that the symbol is global (not function/block local). > + SFF_8636_RX_PWR_1_OFFSET, TAB must be used to indent. checkpatches.sh complains on it. Please, run sanity checks yourself before sending patches upstream as contributor guidelines say. > + SFF_8636_RX_PWR_2_OFFSET, > + SFF_8636_RX_PWR_3_OFFSET, > + SFF_8636_RX_PWR_4_OFFSET, > +}; > +const uint8_t tx_power_offset[MAX_CHANNEL_NUM] = { > + SFF_8636_TX_PWR_1_OFFSET, > + SFF_8636_TX_PWR_2_OFFSET, > + SFF_8636_TX_PWR_3_OFFSET, > + SFF_8636_TX_PWR_4_OFFSET, > +}; > +const uint8_t tx_bias_offset[MAX_CHANNEL_NUM] = { > + SFF_8636_TX_BIAS_1_OFFSET, > + SFF_8636_TX_BIAS_2_OFFSET, > + SFF_8636_TX_BIAS_3_OFFSET, > + SFF_8636_TX_BIAS_4_OFFSET, > +}; It is wrong to define it in the header. You'll have copies if the header is included in many C files and linker will dislike it. So, it should be moved to sff_8636.c