From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from cit-hm8-mail01.bmw-carit.de ([212.118.206.84]:33595 "EHLO cit-hm8-gw01.bmw-carit.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1757940AbcG1NKK (ORCPT ); Thu, 28 Jul 2016 09:10:10 -0400 Subject: Re: [RFC v0 4/8] Input: goodix: use firmware_stat instead of completion To: Bastien Nocera , Daniel Wagner , Bjorn Andersson , Dmitry Torokhov , Greg Kroah-Hartman , Johannes Berg , Kalle Valo , Ohad Ben-Cohen References: <1469692512-16863-1-git-send-email-wagi@monom.org> <1469692512-16863-5-git-send-email-wagi@monom.org> <1469704952.6761.142.camel@hadess.net> <1469708450.6761.156.camel@hadess.net> CC: , , , , , From: Daniel Wagner Message-ID: (sfid-20160728_151041_351809_3713B067) Date: Thu, 28 Jul 2016 15:10:04 +0200 MIME-Version: 1.0 In-Reply-To: <1469708450.6761.156.camel@hadess.net> Content-Type: text/plain; charset="utf-8"; format=flowed Sender: linux-wireless-owner@vger.kernel.org List-ID: > Looking at the API, I really don't like the mixing of namespaces. > Either it's fw_ or it's firmware_ but not a mix of both. I agree, that was not really clever. struct fw_loading { ... } __fw_loading_*() fw_loading_*() Would that make more sense? firmware_loading_ as prefix is a bit long IMO. > Also looks like fw_loading_start() would do nothing as the struct is > likely zero initialised, and FW_STATUS_LOADING == 0. Maybe you need an > UNSET enum member? Good point, I cut a corner here a bit :). In the spirit of making things more clear fw_loading_start() should be used. > FW_STATUS_ABORT <- FW_STATUS_ABORTED, to match the adjective suffixes > of the other members. Ditto fw_loading_abort() which doesn't abort > firmware loading but sets the status. Okay. Thanks a lot for the feedback. cheers, daniel From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Wagner Subject: Re: [RFC v0 4/8] Input: goodix: use firmware_stat instead of completion Date: Thu, 28 Jul 2016 15:10:04 +0200 Message-ID: References: <1469692512-16863-1-git-send-email-wagi@monom.org> <1469692512-16863-5-git-send-email-wagi@monom.org> <1469704952.6761.142.camel@hadess.net> <1469708450.6761.156.camel@hadess.net> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from cit-hm8-mail01.bmw-carit.de ([212.118.206.84]:33596 "EHLO cit-hm8-gw01.bmw-carit.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1757931AbcG1NKK (ORCPT ); Thu, 28 Jul 2016 09:10:10 -0400 In-Reply-To: <1469708450.6761.156.camel@hadess.net> Sender: linux-input-owner@vger.kernel.org List-Id: linux-input@vger.kernel.org To: Bastien Nocera , Daniel Wagner , Bjorn Andersson , Dmitry Torokhov , Greg Kroah-Hartman , Johannes Berg , Kalle Valo , Ohad Ben-Cohen Cc: linux-input@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org, irina.tirdea@intel.com, octavian.purdila@intel.com > Looking at the API, I really don't like the mixing of namespaces. > Either it's fw_ or it's firmware_ but not a mix of both. I agree, that was not really clever. struct fw_loading { ... } __fw_loading_*() fw_loading_*() Would that make more sense? firmware_loading_ as prefix is a bit long IMO. > Also looks like fw_loading_start() would do nothing as the struct is > likely zero initialised, and FW_STATUS_LOADING == 0. Maybe you need an > UNSET enum member? Good point, I cut a corner here a bit :). In the spirit of making things more clear fw_loading_start() should be used. > FW_STATUS_ABORT <- FW_STATUS_ABORTED, to match the adjective suffixes > of the other members. Ditto fw_loading_abort() which doesn't abort > firmware loading but sets the status. Okay. Thanks a lot for the feedback. cheers, daniel