Re: [PATCH v2] ARM: mach-shmobile: ap4evb: add fsib 44100Hz rate fixup

[Paul Mundt <lethal@linux-sh.org>]

On Tue, Nov 16, 2010 at 04:34:34PM +0900, Kuninori Morimoto wrote:
> FSIDIVB is used when HDMI sound is 48kHz,
> but it should be disabled when 44.1kHz.
> And fsidiv_set_rate do that if selected rate is same as parent rate.
> This patch select parent rate to fdiv_clk when 44.1kHz
> 
Ok, now that I now what this is doing, this is also totally bogus.

You have hard-coded some bizzare enable/disable logic in the middle of
the ->set_rate() op that completely blindsides the refcounting. This
might be ok in practice since you probably only have 1 user of the clock
at the moment, but consider what would happen if you had 2 users and one
just happens to randomly turn the clock off on the other. We do
refcounting and API abstraction for a reason. Any time you are deviating
from the API, you are probably doing something wrong.

Furthermore, what does any of this have to do with rate setting? And if
it has nothing to do with rate setting, why would you bother adding it to
the ->set_rate op?

Disabling of the parent clock will happen automatically by the clock
framework when the refcount on it drops. Disabling of the sibling clock
will happen automatically when its refcount drops, too. It seems that all
of your use cases here can be fixed up by simply using proper
clk_get/put() and enable/disable() refcounting.

It is never acceptable to bypass the refcount, under any situation.