All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Anholt <eric@anholt.net>
To: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Mike Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@codeaurora.org>,
	linux-clk@vger.kernel.org,
	Florian Fainelli <f.fainelli@gmail.com>,
	Ray Jui <rjui@broadcom.com>,
	Scott Branden <sbranden@broadcom.com>,
	bcm-kernel-feedback-list@broadcom.com,
	Stephen Warren <swarren@wwwdotorg.org>,
	Lee Jones <lee@kernel.org>,
	linux-rpi-kernel@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] clk: bcm: Support rate change propagation on bcm2835 clocks
Date: Fri, 02 Dec 2016 13:13:35 -0800	[thread overview]
Message-ID: <87y3zynif4.fsf@eliezer.anholt.net> (raw)
In-Reply-To: <20161202205021.074fd441@bbrezillon>

[-- Attachment #1: Type: text/plain, Size: 1689 bytes --]

Boris Brezillon <boris.brezillon@free-electrons.com> writes:

> On Fri, 02 Dec 2016 11:01:09 -0800
> Eric Anholt <eric@anholt.net> wrote:
>
>> Boris Brezillon <boris.brezillon@free-electrons.com> writes:
>> 
>> > Some peripheral clocks, like the VEC (Video EnCoder) clock need to be set
>> > to a precise rate (in our case 108MHz). With the current implementation,
>> > where peripheral clocks are not allowed to forward rate change requests
>> > to their parents, it is impossible to match this requirement unless the
>> > bootloader has configured things correctly, or a specific rate has been
>> > assigned through the DT (with the assigned-clk-rates property).
>> >
>> > Add a new field to struct bcm2835_clock_data to specify which parent
>> > clocks accept rate change propagation, and support set rate propagation
>> > in bcm2835_clock_determine_rate().
>> >
>> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>  
>> 
>> A possible simplification would be to limit VEC to only PLLH_AUX, since
>> that was how the HW designers intended it to be used.  Then you could
>> just have SET_RATE_PARENT flag, rather than the bitfield.
>> 
>
> I can rework the patches to do that if you prefer.

I'm fine with the patches as is, just throwing the idea out there.

In general, I think the only automatic parent switching I've heard of
being useful in the platform is I2S's clock being able to get a
non-MASHed clock when the rate divides evenly off of something.  Other
than that, automatic parent switching seems to just get us in trouble
(we've had bugs with PLLC, and we wouldn't want any non-VEC clock to end
up on PLLH_AUX either).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: eric@anholt.net (Eric Anholt)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] clk: bcm: Support rate change propagation on bcm2835 clocks
Date: Fri, 02 Dec 2016 13:13:35 -0800	[thread overview]
Message-ID: <87y3zynif4.fsf@eliezer.anholt.net> (raw)
In-Reply-To: <20161202205021.074fd441@bbrezillon>

Boris Brezillon <boris.brezillon@free-electrons.com> writes:

> On Fri, 02 Dec 2016 11:01:09 -0800
> Eric Anholt <eric@anholt.net> wrote:
>
>> Boris Brezillon <boris.brezillon@free-electrons.com> writes:
>> 
>> > Some peripheral clocks, like the VEC (Video EnCoder) clock need to be set
>> > to a precise rate (in our case 108MHz). With the current implementation,
>> > where peripheral clocks are not allowed to forward rate change requests
>> > to their parents, it is impossible to match this requirement unless the
>> > bootloader has configured things correctly, or a specific rate has been
>> > assigned through the DT (with the assigned-clk-rates property).
>> >
>> > Add a new field to struct bcm2835_clock_data to specify which parent
>> > clocks accept rate change propagation, and support set rate propagation
>> > in bcm2835_clock_determine_rate().
>> >
>> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>  
>> 
>> A possible simplification would be to limit VEC to only PLLH_AUX, since
>> that was how the HW designers intended it to be used.  Then you could
>> just have SET_RATE_PARENT flag, rather than the bitfield.
>> 
>
> I can rework the patches to do that if you prefer.

I'm fine with the patches as is, just throwing the idea out there.

In general, I think the only automatic parent switching I've heard of
being useful in the platform is I2S's clock being able to get a
non-MASHed clock when the rate divides evenly off of something.  Other
than that, automatic parent switching seems to just get us in trouble
(we've had bugs with PLLC, and we wouldn't want any non-VEC clock to end
up on PLLH_AUX either).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161202/32e5bcae/attachment.sig>

  reply	other threads:[~2016-12-02 21:13 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-01 21:00 [PATCH 0/2] clk: bcm2835: Propage rate change to PLLH Boris Brezillon
2016-12-01 21:00 ` Boris Brezillon
2016-12-01 21:00 ` [PATCH 1/2] clk: bcm: Support rate change propagation on bcm2835 clocks Boris Brezillon
2016-12-01 21:00   ` Boris Brezillon
2016-12-02 19:01   ` Eric Anholt
2016-12-02 19:01     ` Eric Anholt
2016-12-02 19:50     ` Boris Brezillon
2016-12-02 19:50       ` Boris Brezillon
2016-12-02 21:13       ` Eric Anholt [this message]
2016-12-02 21:13         ` Eric Anholt
2016-12-08 23:06   ` Stephen Boyd
2016-12-08 23:06     ` Stephen Boyd
2016-12-01 21:00 ` [PATCH 2/2] clk: bcm: Allow rate change propagation to PLLH_AUX on VEC clock Boris Brezillon
2016-12-01 21:00   ` Boris Brezillon
2016-12-08 23:06   ` Stephen Boyd
2016-12-08 23:06     ` Stephen Boyd

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=87y3zynif4.fsf@eliezer.anholt.net \
    --to=eric@anholt.net \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=boris.brezillon@free-electrons.com \
    --cc=f.fainelli@gmail.com \
    --cc=lee@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-rpi-kernel@lists.infradead.org \
    --cc=mturquette@baylibre.com \
    --cc=rjui@broadcom.com \
    --cc=sboyd@codeaurora.org \
    --cc=sbranden@broadcom.com \
    --cc=swarren@wwwdotorg.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.