All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tomasz Figa <t.figa@samsung.com>
To: Sylwester Nawrocki <s.nawrocki@samsung.com>,
	Tushar Behera <tushar.behera@linaro.org>
Cc: linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	devicetree <devicetree@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>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Pankaj Dubey <pankaj.dubey@samsung.com>,
	Rahul Sharma <rahul.sharma@samsung.com>,
	Mark Brown <broonie@kernel.org>,
	Tomasz Figa <tomasz.figa@gmail.com>
Subject: Re: [PATCH 3/4] clk: samsung: Add driver to control CLKOUT line on Exynos SoCs
Date: Thu, 22 May 2014 14:32:33 +0200	[thread overview]
Message-ID: <537DEE61.2050606@samsung.com> (raw)
In-Reply-To: <537DECCE.8000301@samsung.com>



On 22.05.2014 14:25, Sylwester Nawrocki wrote:
> On 22/05/14 14:01, Tomasz Figa wrote:
>>>>>>>> [ ... ]
>>>>>>>>
>>>>>>>>>> +    clkout->clk_table[0] = clk_register_composite(NULL, "clkout",
>>>>>>>>>> +                            parent_names, parent_count, &clkout->mux.hw,
>>>>>>>>>> +                            &clk_mux_ops, NULL, NULL, &clkout->gate.hw,
>>>>>>>>>> +                            &clk_gate_ops, CLK_SET_RATE_PARENT
>>>>>>>>>> +                            | CLK_SET_RATE_NO_REPARENT);
>>>>>>>>
>>>>>>>> Would you please remove CLK_SET_RATE_NO_REPARENT flag from here? Let me
>>>>>>>> know if you have reservations against this.
>>>>>>
>>>>>> The problem with clock reparenting is that there are certain parent
>>>>>> clocks of CLKOUT, rate of which changes at runtime, e.g. clocks derived
>>>>>> from APLL or bus clocks, which can be reconfigured by cpufreq or devfreq.
>>>>>>
>>>>
>>>> +CC: Sylwester Nawrocki
>>>>
>>>> Okay. But in cases where there is only 1 valid parent clock provided
>>>> through DT (at the moment for Exynos5250/Exynos5420), would it be safe
>>>> to set that clock as the parent of CLKOUT?
>>
>> This is not something to rely on. I have simply omitted remaining CLKOUT
>> parents on Exynos 5 SoCs, as I don't have any board with them on which I
>> could test this. Eventually they will be added.
>>
>>>> Otherwise, this clock is
>>>> not usable ATM.
>>
>> On many boards it is already configured properly by the bootloader.
>> Although I don't see any reason why you couldn't reparent it in
>> (board-specific) sound card driver right now.
> 
> This would require passing the parent's clock specifier in 'clocks'
> property of the sound card device node, which I assume is not something
> we're generally expected to do in mainline. Although some drivers
> happen to be doing it already I think that's a bad example. It sounds
> like an abuse of the current clock bindings.

Well, an already board-specific driver might have some knowledge of the
SoC, so it might not be a huge problem, but I agree that this is not an
optimal solution.

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 3/4] clk: samsung: Add driver to control CLKOUT line on Exynos SoCs
Date: Thu, 22 May 2014 14:32:33 +0200	[thread overview]
Message-ID: <537DEE61.2050606@samsung.com> (raw)
In-Reply-To: <537DECCE.8000301@samsung.com>



On 22.05.2014 14:25, Sylwester Nawrocki wrote:
> On 22/05/14 14:01, Tomasz Figa wrote:
>>>>>>>> [ ... ]
>>>>>>>>
>>>>>>>>>> +    clkout->clk_table[0] = clk_register_composite(NULL, "clkout",
>>>>>>>>>> +                            parent_names, parent_count, &clkout->mux.hw,
>>>>>>>>>> +                            &clk_mux_ops, NULL, NULL, &clkout->gate.hw,
>>>>>>>>>> +                            &clk_gate_ops, CLK_SET_RATE_PARENT
>>>>>>>>>> +                            | CLK_SET_RATE_NO_REPARENT);
>>>>>>>>
>>>>>>>> Would you please remove CLK_SET_RATE_NO_REPARENT flag from here? Let me
>>>>>>>> know if you have reservations against this.
>>>>>>
>>>>>> The problem with clock reparenting is that there are certain parent
>>>>>> clocks of CLKOUT, rate of which changes at runtime, e.g. clocks derived
>>>>>> from APLL or bus clocks, which can be reconfigured by cpufreq or devfreq.
>>>>>>
>>>>
>>>> +CC: Sylwester Nawrocki
>>>>
>>>> Okay. But in cases where there is only 1 valid parent clock provided
>>>> through DT (at the moment for Exynos5250/Exynos5420), would it be safe
>>>> to set that clock as the parent of CLKOUT?
>>
>> This is not something to rely on. I have simply omitted remaining CLKOUT
>> parents on Exynos 5 SoCs, as I don't have any board with them on which I
>> could test this. Eventually they will be added.
>>
>>>> Otherwise, this clock is
>>>> not usable ATM.
>>
>> On many boards it is already configured properly by the bootloader.
>> Although I don't see any reason why you couldn't reparent it in
>> (board-specific) sound card driver right now.
> 
> This would require passing the parent's clock specifier in 'clocks'
> property of the sound card device node, which I assume is not something
> we're generally expected to do in mainline. Although some drivers
> happen to be doing it already I think that's a bad example. It sounds
> like an abuse of the current clock bindings.

Well, an already board-specific driver might have some knowledge of the
SoC, so it might not be a huge problem, but I agree that this is not an
optimal solution.

Best regards,
Tomasz

  reply	other threads:[~2014-05-22 12:32 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-20 16:43 [PATCH 0/4] Add support for Exynos clock output configuration Tomasz Figa
2014-05-20 16:43 ` Tomasz Figa
2014-05-20 16:43 ` [PATCH 1/4] clk: samsung: exynos4: Add missing CPU/DMC clock hierarchy Tomasz Figa
2014-05-20 16:43   ` Tomasz Figa
2014-05-20 16:43 ` [PATCH 2/4] clk: samsung: exynos4: Add CLKOUT " Tomasz Figa
2014-05-20 16:43   ` Tomasz Figa
2014-05-20 16:43   ` Tomasz Figa
2014-05-20 16:43 ` [PATCH 3/4] clk: samsung: Add driver to control CLKOUT line on Exynos SoCs Tomasz Figa
2014-05-20 16:43   ` Tomasz Figa
2014-05-22  4:28   ` Tushar Behera
2014-05-22  4:28     ` Tushar Behera
2014-05-22 10:30     ` Tomasz Figa
2014-05-22 10:30       ` Tomasz Figa
2014-05-22  5:13   ` Tushar Behera
2014-05-22  5:13     ` Tushar Behera
2014-05-22 10:34     ` Tomasz Figa
2014-05-22 10:34       ` Tomasz Figa
2014-05-22 11:44       ` Tushar Behera
2014-05-22 11:44         ` Tushar Behera
2014-05-22 12:01         ` Tomasz Figa
2014-05-22 12:01           ` Tomasz Figa
2014-05-22 12:14           ` Tushar Behera
2014-05-22 12:14             ` Tushar Behera
2014-05-22 12:25           ` Sylwester Nawrocki
2014-05-22 12:25             ` Sylwester Nawrocki
2014-05-22 12:25             ` Sylwester Nawrocki
2014-05-22 12:32             ` Tomasz Figa [this message]
2014-05-22 12:32               ` Tomasz Figa
2014-05-22 19:20           ` Mark Brown
2014-05-22 19:20             ` Mark Brown
     [not found]         ` <CAHbNUh2gkpV-rt6S+-bUmFvYAuK_HNoXfyOStwYe29sZMJonUw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-05-22 12:10           ` Sylwester Nawrocki
2014-05-22 12:10             ` Sylwester Nawrocki
2014-05-22 12:10             ` Sylwester Nawrocki
2014-05-20 16:43 ` [PATCH 4/4] ARM: dts: exynos: Update PMU node with CLKOUT related data Tomasz Figa
2014-05-20 16:43   ` Tomasz Figa
2014-05-22  4:41   ` Tushar Behera
2014-05-22  4:41     ` 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=537DEE61.2050606@samsung.com \
    --to=t.figa@samsung.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --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=m.szyprowski@samsung.com \
    --cc=mark.rutland@arm.com \
    --cc=mturquette@linaro.org \
    --cc=pankaj.dubey@samsung.com \
    --cc=rahul.sharma@samsung.com \
    --cc=robh+dt@kernel.org \
    --cc=s.nawrocki@samsung.com \
    --cc=tomasz.figa@gmail.com \
    --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.