On Fri, 9 Sep 2016, Nicolai Stange wrote:
> @@ -339,17 +339,14 @@ static int sh_cmt_enable(struct sh_cmt_channel *ch,
> unsigned long *rate)
> sh_cmt_start_stop_ch(ch, 0);
>
> /* configure channel, periodic mode and maximum timeout */
> - if (ch->cmt->info->width == 16) {
> - *rate = clk_get_rate(ch->cmt->clk) / 512;
> + if (ch->cmt->info->width == 16)
> sh_cmt_write_cmcsr(ch, SH_CMT16_CMCSR_CMIE |
> SH_CMT16_CMCSR_CKS512);
> - } else {
> - *rate = clk_get_rate(ch->cmt->clk) / 8;
> + else
> sh_cmt_write_cmcsr(ch, SH_CMT32_CMCSR_CMM |
> SH_CMT32_CMCSR_CMTOUT_IE |
> SH_CMT32_CMCSR_CMR_IRQ |
> SH_CMT32_CMCSR_CKS_RCLK8);
> - }
Removing the braces here is just wrong.
if (foo)
bar()
else
rab();
Is perfectly fine because the lack of braces tells that there are single
line statements in both branches.
if (foo)
bar(foo->something, foo->somemore, foo->other,
CONST);)
else
rab();
Is NOT. Simply because the pattern of a 'if ()' condition without braces
suggests a single line statement to follow. Then the reading flow stops
because there is more than one line. While:
if (foo) {
bar(foo->something, foo->somemore, foo->other,
CONST);)
} else {
rab();
}
parses fine. With your 4 line single statement it's even worse.
> ret = sh_cmt_start(ch, FLAG_CLOCKSOURCE);
> - if (!ret) {
> - __clocksource_update_freq_hz(cs, ch->rate);
> + if (!ret)
> ch->cs_enabled = true;
> - }
This one is perfectly fine.
> @@ -824,6 +810,12 @@ static int sh_cmt_register_clockevent(struct
> sh_cmt_channel *ch,
> ced->suspend = sh_cmt_clock_event_suspend;
> ced->resume = sh_cmt_clock_event_resume;
>
> + /* TODO: calculate good shift from rate and counter bit width */
> + ced->shift = 32;
> + ced->mult = div_sc(ch->cmt->rate, NSEC_PER_SEC, ced->shift);
Errm. clockevents_calc_mult_shift()
> + ced->max_delta_ns = clockevent_delta2ns(ch->max_match_value, ced);
> + ced->min_delta_ns = clockevent_delta2ns(0x1f, ced);
> +
> dev_info(&ch->cmt->pdev->dev, "ch%u: used for clock events\n",
> ch->index);
> clockevents_register_device(ced);
Thanks,
tglx
