Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
From: Sylwester Nawrocki <s.nawrocki@samsung.com>
To: cwchoi00@gmail.com, Lukasz Luba <l.luba@partner.samsung.com>
Cc: devicetree <devicetree@vger.kernel.org>,
	linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Linux PM list <linux-pm@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Chanwoo Choi <cw00.choi@samsung.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Kukjin Kim <kgene@kernel.org>,
	MyungJoo Ham <myungjoo.ham@samsung.com>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Marek Szyprowski <m.szyprowski@samsung.com>
Subject: Re: [PATCH v4 4/8] drivers: devfreq: add DMC driver for Exynos5422
Date: Wed, 06 Mar 2019 14:44:28 +0100
Message-ID: <94e169cf-4dc2-82ec-1385-852bc7853931@samsung.com> (raw)
In-Reply-To: <CAGTfZH0o6dHQj00vHwZBn05AT-Z_oAWHfBX0eATQ12NPmfw4fg@mail.gmail.com>

Hi,

On 2/3/19 13:23, Chanwoo Choi wrote:

> 2019년 2월 2일 (토) 오전 2:42, Lukasz Luba <l.luba@partner.samsung.com>님이 작성:

>> +/**
>> + * exynos5_dmc_pause_on_switching() - Controls a pause feature in DMC
>> + * @dmc:       device which is used for changing this feature
>> + * @set:       a boolean state passing enable/disable request
>> + *
>> + * There is a need of pausing DREX DMC when divider or MUX in clock tree
>> + * changes its configuration. In such situation access to the memory is blocked
>> + * in DMC automatically. This feature is used when clock frequency change
>> + * request appears and touches clock tree.
>> + */
>> +static int exynos5_dmc_pause_on_switching(struct exynos5_dmc *dmc, bool set)
> 
> Don't need to make it as the separate function. It is only used on
> probe() function.

It seems fine to me to have this functionality in a separate function, 
it's self-contained and it's now pretty well documented.

>> +{
>> +       unsigned int val;
>> +
>> +       val = readl(dmc->base_clk + DMC_PAUSE_CTRL);
>> +       if (set)
>> +               val |= DMC_PAUSE_ENABLE;
>> +       else
>> +               val &= ~DMC_PAUSE_ENABLE;
>> +       writel(val, dmc->base_clk + DMC_PAUSE_CTRL);
> 
> The dt-binding file doesn't explain the 'reg' property for 'base_clk'.
> You are missing.
> When I tried to find what are the base address, it is the register map
> of clock-controller.
> This driver accesed the register of clock controller without any
> functions of CCF
> (common clock framework). It is wrong.
> 
> If you need to get the some information of clock, must have to use the CCF.
We talked a little about this issue with Lukasz in person and it looks
like there are some DMC related registers in the clock controller register 
region, I'd say those registers are better handled by the DMC driver rather
than the clocks controller driver. Moreover, we should avoid abusing clk API
for not strictly clocks related functionality as it appears to be above.
It might be more appropriate to add in the dmc DT node a phandle to a regmap
exposed by the clock-controller node. It seems there will be even no single
register that would be shared between the DMC and the clock controller.

-- 
Regards,
Sylwester

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply index

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1549039612-28905-1-git-send-email-l.luba@partner.samsung.com>
     [not found] ` <CGME20190201164718eucas1p1bca3a199d60e109e1da654d8afff2a47@eucas1p1.samsung.com>
2019-02-01 16:46   ` [PATCH v4 1/8] clk: samsung: add needed IDs for DMC clocks in Exynos5420 Lukasz Luba
2019-02-13 21:07     ` Rob Herring
     [not found] ` <CGME20190201164719eucas1p2091c6d41a6cc21a3d36081daf4bc8267@eucas1p2.samsung.com>
2019-02-01 16:46   ` [PATCH v4 2/8] clk: samsung: add new clocks for DMC for Exynos5422 SoC Lukasz Luba
2019-02-03  9:56     ` Chanwoo Choi
2019-02-11 11:11       ` Lukasz Luba
2019-02-12  6:04         ` Chanwoo Choi
     [not found] ` <CGME20190201164719eucas1p106c8761eb4bff12a906b601a37bf58b5@eucas1p1.samsung.com>
2019-02-01 16:46   ` [PATCH v4 3/8] clk: samsung: add BPLL rate table for Exynos 5422 SoC Lukasz Luba
     [not found] ` <CGME20190201164720eucas1p13aac6550399d240b261a2cbe489d3dfc@eucas1p1.samsung.com>
2019-02-01 16:46   ` [PATCH v4 4/8] drivers: devfreq: add DMC driver for Exynos5422 Lukasz Luba
2019-02-03 12:23     ` Chanwoo Choi
2019-03-06 13:44       ` Sylwester Nawrocki [this message]
2019-03-07  1:10         ` Chanwoo Choi
2019-03-07  8:41           ` Sylwester Nawrocki
     [not found] ` <CGME20190201164720eucas1p1ae770f7981fa09016b69ca7265e820c7@eucas1p1.samsung.com>
2019-02-01 16:46   ` [PATCH v4 5/8] dt-bindings: devfreq: add Exynos5422 DMC device description Lukasz Luba
2019-02-03 12:28     ` Chanwoo Choi
     [not found] ` <CGME20190201164721eucas1p286976bab0cc9e06c2cf74a0eaa20144e@eucas1p2.samsung.com>
2019-02-01 16:46   ` [PATCH v4 6/8] DT: arm: exynos: add DMC device for exynos5422 Lukasz Luba
2019-02-08  8:14     ` Krzysztof Kozlowski
     [not found] ` <CGME20190201164722eucas1p1b619d939e0f93ddb9ee1af7306e7cf67@eucas1p1.samsung.com>
2019-02-01 16:46   ` [PATCH v4 7/8] drivers: devfreq: events: add Exynos PPMU new events Lukasz Luba
     [not found] ` <CGME20190201164723eucas1p1ec009489584e1c85fd0d62270796e003@eucas1p1.samsung.com>
2019-02-01 16:46   ` [PATCH v4 8/8] ARM: exynos_defconfig: enable DMC driver Lukasz Luba
2019-02-08  8:15     ` Krzysztof Kozlowski

Reply instructions:

You may reply publically 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=94e169cf-4dc2-82ec-1385-852bc7853931@samsung.com \
    --to=s.nawrocki@samsung.com \
    --cc=b.zolnierkie@samsung.com \
    --cc=cw00.choi@samsung.com \
    --cc=cwchoi00@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=kgene@kernel.org \
    --cc=krzk@kernel.org \
    --cc=kyungmin.park@samsung.com \
    --cc=l.luba@partner.samsung.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=myungjoo.ham@samsung.com \
    /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

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org
	public-inbox-index linux-arm-kernel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git