Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750847AbdLZHhk (ORCPT ); Tue, 26 Dec 2017 02:37:40 -0500 Received: from mga07.intel.com ([134.134.136.100]:27074 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750708AbdLZHhj (ORCPT ); Tue, 26 Dec 2017 02:37:39 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,459,1508828400"; d="scan'208";a="15180371" Subject: Re: [alsa-devel] [PATCH 15/27] ALSA: hda - Use timecounter_initialize interface To: Takashi Iwai , Richard Cochran Cc: linux-kernel@vger.kernel.org, alsa-devel@alsa-project.org, Vinod Koul , Thomas Gleixner References: <1513323522-15021-1-git-send-email-sagar.a.kamble@intel.com> <1513323522-15021-16-git-send-email-sagar.a.kamble@intel.com> <20171215165125.avkz25eek56i5md4@localhost> From: Sagar Arun Kamble Message-ID: Date: Tue, 26 Dec 2017 13:07:35 +0530 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1689 Lines: 42 On 12/15/2017 10:40 PM, Takashi Iwai wrote: > On Fri, 15 Dec 2017 17:51:25 +0100, > Richard Cochran wrote: >> On Fri, Dec 15, 2017 at 12:10:47PM +0100, Takashi Iwai wrote: >> >>>> - struct cyclecounter *cc = &azx_dev->tc.cc; >>>> - cc->read = azx_cc_read; >>>> - cc->mask = CLOCKSOURCE_MASK(32); >>>> - cc->mult = 125; /* saturation after 195 years */ >>>> - cc->shift = 0; >> I want to get away from this mess of open coded structure >> initialization and use a proper functional interface instead. > I agree that a proper functional interface would be better, too. > But not a form like foo(501, 21, 10, 499, 5678). > In C syntax, you may more easily pass a wrong value than open codes. > >>>> nsec = 0; /* audio time is elapsed time since trigger */ >>>> - timecounter_init(tc, nsec); >>>> + timecounter_initialize(tc, >>>> + azx_cc_read, >>>> + CLOCKSOURCE_MASK(32), >>>> + 125, /* saturation after 195 years */ >>>> + 0, >>>> + nsec); >>> Hmm, a function with so many arguments is difficult to remember and is >>> often error-prone. By this transition, it becomes harder to read >>> through. >> Please suggest a better way. > I have no good idea ATM, sorry. > > Or can we provide simpler versions for covering some defaults? At > least reducing the number of arguments would make things easier. Thought about specifying 1. cyclecounter read func 2. frequency 3. width of counter as parameters here which can get rid of mult, shift params. But this is not easy as most of the drivers do not specify cyclecounter frequency and instead hard-code the mult/shift factors. How about passing initialized cyclecounter struct? > > Takashi