Re: [alsa-devel] [PATCH 15/27] ALSA: hda - Use timecounter_initialize interface

[Sagar Arun Kamble <sagar.a.kamble@intel.com>]

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