From: Tomasz Figa <t.figa@samsung.com>
To: Rahul Sharma <rahul.sharma@samsung.com>,
Tushar Behera <tushar.behera@linaro.org>
Cc: Pankaj Dubey <pankaj.dubey@samsung.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
Mike Turquette <mturquette@linaro.org>,
Kukjin Kim <kgene.kim@samsung.com>,
Kumar Gala <galak@codeaurora.org>,
Ian Campbell <ijc+devicetree@hellion.org.uk>,
Mark Rutland <mark.rutland@arm.com>,
Pawel Moll <pawel.moll@arm.com>, Rob Herring <robh+dt@kernel.org>,
sunil joshi <joshi@samsung.com>
Subject: Re: [PATCH 1/4] clk: samsung: out: Add infrastructure to register CLKOUT
Date: Thu, 15 May 2014 16:07:24 +0200 [thread overview]
Message-ID: <5374CA1C.8000207@samsung.com> (raw)
In-Reply-To: <CAPdUM4N=k-pPxdhPQgVHh0fb+nqkgs7StwM2Pb6C8gcjx6dw-w@mail.gmail.com>
Hi Rahul, Tushar,
On 15.05.2014 15:44, Rahul Sharma wrote:
> Hi Tushar,
>
> Basically you are adding a new clock-type for Clkout. IMO clkout
> is not a special hardware. Existing clock types can be reused to
> support clkout. I see 3 major problem here:
>
> 1) Clkout -> (Mux + Gate). You clubbed mux and gate together, and
> exposing as a single clock which is something like a composite clock.
> IMO this is not a recommended way in CCF.
>
> 2) New Clock Type: Since clkout is just a combination of a simple
> mux and gate which are already supported, it is a unnecessary
> duplication.
>
> 3) Clkout registered along with CMU: which is not correct. Clkout is in PMU
> (Separate physical IP) and should be registered as a independent Clock
> provider which provides 1 mux and 1 gate clock (As if now). It should also be
> well connected with main CMU.
>
> I understand the challenge in using regmap interface for a clock provider. But
> we need to identify a clean solution. IMHO a independent clock provider with
> iomap, is relatively cleaner approach till CCF is not ready with regmap based
> reg access for clock registers.
>
> Experts!! please comment.
It's quite unfortunate that Tushar has duplicated the effort to create a
clkout driver, considering the fact that we did have such driver
internally at SRPOL and it was quite nice and simple.
I will post a cleaned-up version today, that is about 2 times smaller in
terms of lines of added code and provides the same functionality,
without introducing custom clock types. In addition, it models the
clkout properly as a feature of PMU, not CMU (CMU only provides outputs
of particular sub-blocks that are fed into the PMU).
Best regards,
Tomasz
WARNING: multiple messages have this Message-ID (diff)
From: t.figa@samsung.com (Tomasz Figa)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/4] clk: samsung: out: Add infrastructure to register CLKOUT
Date: Thu, 15 May 2014 16:07:24 +0200 [thread overview]
Message-ID: <5374CA1C.8000207@samsung.com> (raw)
In-Reply-To: <CAPdUM4N=k-pPxdhPQgVHh0fb+nqkgs7StwM2Pb6C8gcjx6dw-w@mail.gmail.com>
Hi Rahul, Tushar,
On 15.05.2014 15:44, Rahul Sharma wrote:
> Hi Tushar,
>
> Basically you are adding a new clock-type for Clkout. IMO clkout
> is not a special hardware. Existing clock types can be reused to
> support clkout. I see 3 major problem here:
>
> 1) Clkout -> (Mux + Gate). You clubbed mux and gate together, and
> exposing as a single clock which is something like a composite clock.
> IMO this is not a recommended way in CCF.
>
> 2) New Clock Type: Since clkout is just a combination of a simple
> mux and gate which are already supported, it is a unnecessary
> duplication.
>
> 3) Clkout registered along with CMU: which is not correct. Clkout is in PMU
> (Separate physical IP) and should be registered as a independent Clock
> provider which provides 1 mux and 1 gate clock (As if now). It should also be
> well connected with main CMU.
>
> I understand the challenge in using regmap interface for a clock provider. But
> we need to identify a clean solution. IMHO a independent clock provider with
> iomap, is relatively cleaner approach till CCF is not ready with regmap based
> reg access for clock registers.
>
> Experts!! please comment.
It's quite unfortunate that Tushar has duplicated the effort to create a
clkout driver, considering the fact that we did have such driver
internally at SRPOL and it was quite nice and simple.
I will post a cleaned-up version today, that is about 2 times smaller in
terms of lines of added code and provides the same functionality,
without introducing custom clock types. In addition, it models the
clkout properly as a feature of PMU, not CMU (CMU only provides outputs
of particular sub-blocks that are fed into the PMU).
Best regards,
Tomasz
next prev parent reply other threads:[~2014-05-15 14:07 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-09 13:00 [PATCH 0/4] Add framework to support clkout Tushar Behera
2014-05-09 13:00 ` Tushar Behera
2014-05-09 13:00 ` Tushar Behera
2014-05-09 13:00 ` [PATCH 1/4] clk: samsung: out: Add infrastructure to register CLKOUT Tushar Behera
2014-05-09 13:00 ` Tushar Behera
2014-05-10 3:51 ` Pankaj Dubey
2014-05-10 3:51 ` Pankaj Dubey
2014-05-12 4:46 ` Tushar Behera
2014-05-12 4:46 ` Tushar Behera
2014-05-15 13:44 ` Rahul Sharma
2014-05-15 13:44 ` Rahul Sharma
2014-05-15 14:07 ` Tomasz Figa [this message]
2014-05-15 14:07 ` Tomasz Figa
2014-05-15 14:14 ` Rahul Sharma
2014-05-15 14:14 ` Rahul Sharma
2014-05-19 3:30 ` Tushar Behera
2014-05-19 3:30 ` Tushar Behera
2014-05-19 10:44 ` Tomasz Figa
2014-05-19 10:44 ` Tomasz Figa
2014-05-09 13:00 ` [PATCH 2/4] clk: samsung: exynos5420: Add xclkout debug clock Tushar Behera
2014-05-09 13:00 ` Tushar Behera
2014-05-09 13:00 ` [PATCH 3/4] clk: samsung: exynos5250: " Tushar Behera
2014-05-09 13:00 ` Tushar Behera
2014-05-09 13:00 ` [PATCH 4/4] ARM: dts: Add pmu-syscon handle for Exynos5420/Exynos5250 clock Tushar Behera
2014-05-09 13:00 ` Tushar Behera
2014-05-10 3:39 ` [PATCH 0/4] Add framework to support clkout Pankaj Dubey
2014-05-10 3:39 ` Pankaj Dubey
2014-05-12 4:42 ` Tushar Behera
2014-05-12 4:42 ` Tushar Behera
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5374CA1C.8000207@samsung.com \
--to=t.figa@samsung.com \
--cc=devicetree@vger.kernel.org \
--cc=galak@codeaurora.org \
--cc=ijc+devicetree@hellion.org.uk \
--cc=joshi@samsung.com \
--cc=kgene.kim@samsung.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mturquette@linaro.org \
--cc=pankaj.dubey@samsung.com \
--cc=pawel.moll@arm.com \
--cc=rahul.sharma@samsung.com \
--cc=robh+dt@kernel.org \
--cc=tushar.behera@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.