* [PATCH 0/2] clk: bcm2835: Propage rate change to PLLH @ 2016-12-01 21:00 ` Boris Brezillon 0 siblings, 0 replies; 16+ messages in thread From: Boris Brezillon @ 2016-12-01 21:00 UTC (permalink / raw) To: Mike Turquette, Stephen Boyd, linux-clk Cc: Florian Fainelli, Ray Jui, Scott Branden, bcm-kernel-feedback-list, Stephen Warren, Lee Jones, Eric Anholt, linux-rpi-kernel, linux-arm-kernel, Boris Brezillon Hello, The VEC (Video EnCoder) clock needs to be running at precisely 108Mhz for the VEC IP to work properly. Unfortunately, the current implementation does not propate peripheral clock rate change to their parents, which prevents us from getting the 108Mhz rate unless the PLLH and PLLH_AUX clocks are already configured (by the bootloader) to a multiple of 108Mhz. This series adds support for optional per-periph-clk rate change propagation and enables this feature on the VEC clock. Regards, Boris Boris Brezillon (2): clk: bcm: Support rate change propagation on bcm2835 clocks clk: bcm: Allow rate change propagation to PLLH_AUX on VEC clock drivers/clk/bcm/clk-bcm2835.c | 74 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 69 insertions(+), 5 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 0/2] clk: bcm2835: Propage rate change to PLLH @ 2016-12-01 21:00 ` Boris Brezillon 0 siblings, 0 replies; 16+ messages in thread From: Boris Brezillon @ 2016-12-01 21:00 UTC (permalink / raw) To: linux-arm-kernel Hello, The VEC (Video EnCoder) clock needs to be running at precisely 108Mhz for the VEC IP to work properly. Unfortunately, the current implementation does not propate peripheral clock rate change to their parents, which prevents us from getting the 108Mhz rate unless the PLLH and PLLH_AUX clocks are already configured (by the bootloader) to a multiple of 108Mhz. This series adds support for optional per-periph-clk rate change propagation and enables this feature on the VEC clock. Regards, Boris Boris Brezillon (2): clk: bcm: Support rate change propagation on bcm2835 clocks clk: bcm: Allow rate change propagation to PLLH_AUX on VEC clock drivers/clk/bcm/clk-bcm2835.c | 74 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 69 insertions(+), 5 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/2] clk: bcm: Support rate change propagation on bcm2835 clocks 2016-12-01 21:00 ` Boris Brezillon @ 2016-12-01 21:00 ` Boris Brezillon -1 siblings, 0 replies; 16+ messages in thread From: Boris Brezillon @ 2016-12-01 21:00 UTC (permalink / raw) To: Mike Turquette, Stephen Boyd, linux-clk Cc: Florian Fainelli, Ray Jui, Scott Branden, bcm-kernel-feedback-list, Stephen Warren, Lee Jones, Eric Anholt, linux-rpi-kernel, linux-arm-kernel, Boris Brezillon 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> --- Note: this approach is still fragile, since it does not prevent 2 clock users from stepping on each other's toes. We could use the clk rate constraints infrastructure, but it's only applicable to leaf clocks in the clock tree (when you apply a constraint, these constraints are not propagated to the clock tree). Another approach would be to lock a rate on a clock, thus preventing other users from changing it. Propagating rate locks would be easier than propagating rate range constraints (see this patch http://code.bulix.org/jwtzf8-107264), but it still requires that really care about a specific clock rate explicitly lock the rate (using clk_set_and_lock_rate()), and this means checking every drivers that have such requirements. --- drivers/clk/bcm/clk-bcm2835.c | 67 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 63 insertions(+), 4 deletions(-) diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c index 2acaa77ad482..df96fe6dadab 100644 --- a/drivers/clk/bcm/clk-bcm2835.c +++ b/drivers/clk/bcm/clk-bcm2835.c @@ -436,6 +436,9 @@ struct bcm2835_clock_data { const char *const *parents; int num_mux_parents; + /* Bitmap encoding which parents accept rate change propagation. */ + unsigned int set_rate_parent; + u32 ctl_reg; u32 div_reg; @@ -1017,10 +1020,60 @@ bcm2835_clk_is_pllc(struct clk_hw *hw) return strncmp(clk_hw_get_name(hw), "pllc", 4) == 0; } +static unsigned long bcm2835_clock_choose_div_and_prate(struct clk_hw *hw, + int parent_idx, + unsigned long rate, + u32 *div, + unsigned long *prate) +{ + struct bcm2835_clock *clock = bcm2835_clock_from_hw(hw); + struct bcm2835_cprman *cprman = clock->cprman; + const struct bcm2835_clock_data *data = clock->data; + unsigned long best_rate; + u32 curdiv, mindiv, maxdiv; + struct clk_hw *parent; + + parent = clk_hw_get_parent_by_index(hw, parent_idx); + + if (!(BIT(parent_idx) & data->set_rate_parent)) { + *prate = clk_hw_get_rate(parent); + *div = bcm2835_clock_choose_div(hw, rate, *prate, true); + + return bcm2835_clock_rate_from_divisor(clock, *prate, + *div); + } + + if (data->frac_bits) + dev_warn(cprman->dev, + "frac bits are not used when propagating rate change"); + + /* clamp to min divider of 2 if we're dealing with a mash clock */ + mindiv = data->is_mash_clock ? 2 : 1; + maxdiv = BIT(data->int_bits) - 1; + + /* TODO: Be smart, and only test a subset of the available divisors. */ + for (curdiv = mindiv; curdiv <= maxdiv; curdiv++) { + unsigned long tmp_rate; + + tmp_rate = clk_hw_round_rate(parent, rate * curdiv); + tmp_rate /= curdiv; + if (curdiv == mindiv || + (tmp_rate > best_rate && tmp_rate <= rate)) + best_rate = tmp_rate; + + if (best_rate == rate) + break; + } + + *div = curdiv << CM_DIV_FRAC_BITS; + *prate = curdiv * best_rate; + + return best_rate; +} + static int bcm2835_clock_determine_rate(struct clk_hw *hw, struct clk_rate_request *req) { - struct bcm2835_clock *clock = bcm2835_clock_from_hw(hw); struct clk_hw *parent, *best_parent = NULL; bool current_parent_is_pllc; unsigned long rate, best_rate = 0; @@ -1048,9 +1101,8 @@ static int bcm2835_clock_determine_rate(struct clk_hw *hw, if (bcm2835_clk_is_pllc(parent) && !current_parent_is_pllc) continue; - prate = clk_hw_get_rate(parent); - div = bcm2835_clock_choose_div(hw, req->rate, prate, true); - rate = bcm2835_clock_rate_from_divisor(clock, prate, div); + rate = bcm2835_clock_choose_div_and_prate(hw, i, req->rate, + &div, &prate); if (rate > best_rate && rate <= req->rate) { best_parent = parent; best_prate = prate; @@ -1262,6 +1314,13 @@ static struct clk_hw *bcm2835_register_clock(struct bcm2835_cprman *cprman, init.name = data->name; init.flags = data->flags | CLK_IGNORE_UNUSED; + /* + * Pass the CLK_SET_RATE_PARENT flag if we are allowed to propagate + * rate changes on at least of the parents. + */ + if (data->set_rate_parent) + init.flags |= CLK_SET_RATE_PARENT; + if (data->is_vpu_clock) { init.ops = &bcm2835_vpu_clock_clk_ops; } else { -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 1/2] clk: bcm: Support rate change propagation on bcm2835 clocks @ 2016-12-01 21:00 ` Boris Brezillon 0 siblings, 0 replies; 16+ messages in thread From: Boris Brezillon @ 2016-12-01 21:00 UTC (permalink / raw) To: linux-arm-kernel 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> --- Note: this approach is still fragile, since it does not prevent 2 clock users from stepping on each other's toes. We could use the clk rate constraints infrastructure, but it's only applicable to leaf clocks in the clock tree (when you apply a constraint, these constraints are not propagated to the clock tree). Another approach would be to lock a rate on a clock, thus preventing other users from changing it. Propagating rate locks would be easier than propagating rate range constraints (see this patch http://code.bulix.org/jwtzf8-107264), but it still requires that really care about a specific clock rate explicitly lock the rate (using clk_set_and_lock_rate()), and this means checking every drivers that have such requirements. --- drivers/clk/bcm/clk-bcm2835.c | 67 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 63 insertions(+), 4 deletions(-) diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c index 2acaa77ad482..df96fe6dadab 100644 --- a/drivers/clk/bcm/clk-bcm2835.c +++ b/drivers/clk/bcm/clk-bcm2835.c @@ -436,6 +436,9 @@ struct bcm2835_clock_data { const char *const *parents; int num_mux_parents; + /* Bitmap encoding which parents accept rate change propagation. */ + unsigned int set_rate_parent; + u32 ctl_reg; u32 div_reg; @@ -1017,10 +1020,60 @@ bcm2835_clk_is_pllc(struct clk_hw *hw) return strncmp(clk_hw_get_name(hw), "pllc", 4) == 0; } +static unsigned long bcm2835_clock_choose_div_and_prate(struct clk_hw *hw, + int parent_idx, + unsigned long rate, + u32 *div, + unsigned long *prate) +{ + struct bcm2835_clock *clock = bcm2835_clock_from_hw(hw); + struct bcm2835_cprman *cprman = clock->cprman; + const struct bcm2835_clock_data *data = clock->data; + unsigned long best_rate; + u32 curdiv, mindiv, maxdiv; + struct clk_hw *parent; + + parent = clk_hw_get_parent_by_index(hw, parent_idx); + + if (!(BIT(parent_idx) & data->set_rate_parent)) { + *prate = clk_hw_get_rate(parent); + *div = bcm2835_clock_choose_div(hw, rate, *prate, true); + + return bcm2835_clock_rate_from_divisor(clock, *prate, + *div); + } + + if (data->frac_bits) + dev_warn(cprman->dev, + "frac bits are not used when propagating rate change"); + + /* clamp to min divider of 2 if we're dealing with a mash clock */ + mindiv = data->is_mash_clock ? 2 : 1; + maxdiv = BIT(data->int_bits) - 1; + + /* TODO: Be smart, and only test a subset of the available divisors. */ + for (curdiv = mindiv; curdiv <= maxdiv; curdiv++) { + unsigned long tmp_rate; + + tmp_rate = clk_hw_round_rate(parent, rate * curdiv); + tmp_rate /= curdiv; + if (curdiv == mindiv || + (tmp_rate > best_rate && tmp_rate <= rate)) + best_rate = tmp_rate; + + if (best_rate == rate) + break; + } + + *div = curdiv << CM_DIV_FRAC_BITS; + *prate = curdiv * best_rate; + + return best_rate; +} + static int bcm2835_clock_determine_rate(struct clk_hw *hw, struct clk_rate_request *req) { - struct bcm2835_clock *clock = bcm2835_clock_from_hw(hw); struct clk_hw *parent, *best_parent = NULL; bool current_parent_is_pllc; unsigned long rate, best_rate = 0; @@ -1048,9 +1101,8 @@ static int bcm2835_clock_determine_rate(struct clk_hw *hw, if (bcm2835_clk_is_pllc(parent) && !current_parent_is_pllc) continue; - prate = clk_hw_get_rate(parent); - div = bcm2835_clock_choose_div(hw, req->rate, prate, true); - rate = bcm2835_clock_rate_from_divisor(clock, prate, div); + rate = bcm2835_clock_choose_div_and_prate(hw, i, req->rate, + &div, &prate); if (rate > best_rate && rate <= req->rate) { best_parent = parent; best_prate = prate; @@ -1262,6 +1314,13 @@ static struct clk_hw *bcm2835_register_clock(struct bcm2835_cprman *cprman, init.name = data->name; init.flags = data->flags | CLK_IGNORE_UNUSED; + /* + * Pass the CLK_SET_RATE_PARENT flag if we are allowed to propagate + * rate changes on at least of the parents. + */ + if (data->set_rate_parent) + init.flags |= CLK_SET_RATE_PARENT; + if (data->is_vpu_clock) { init.ops = &bcm2835_vpu_clock_clk_ops; } else { -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] clk: bcm: Support rate change propagation on bcm2835 clocks 2016-12-01 21:00 ` Boris Brezillon @ 2016-12-02 19:01 ` Eric Anholt -1 siblings, 0 replies; 16+ messages in thread From: Eric Anholt @ 2016-12-02 19:01 UTC (permalink / raw) To: Boris Brezillon, Mike Turquette, Stephen Boyd, linux-clk Cc: Florian Fainelli, Ray Jui, Scott Branden, bcm-kernel-feedback-list, Stephen Warren, Lee Jones, linux-rpi-kernel, linux-arm-kernel, Boris Brezillon [-- Attachment #1: Type: text/plain, Size: 1073 bytes --] 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. Still, this seems to be correct and fixes the bug. Both patches are: Reviewed-by: Eric Anholt <eric@anholt.net> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/2] clk: bcm: Support rate change propagation on bcm2835 clocks @ 2016-12-02 19:01 ` Eric Anholt 0 siblings, 0 replies; 16+ messages in thread From: Eric Anholt @ 2016-12-02 19:01 UTC (permalink / raw) To: linux-arm-kernel 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. Still, this seems to be correct and fixes the bug. Both patches are: Reviewed-by: Eric Anholt <eric@anholt.net> -------------- 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/54cee9b5/attachment.sig> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] clk: bcm: Support rate change propagation on bcm2835 clocks 2016-12-02 19:01 ` Eric Anholt @ 2016-12-02 19:50 ` Boris Brezillon -1 siblings, 0 replies; 16+ messages in thread From: Boris Brezillon @ 2016-12-02 19:50 UTC (permalink / raw) To: Eric Anholt Cc: Mike Turquette, Stephen Boyd, linux-clk, Florian Fainelli, Ray Jui, Scott Branden, bcm-kernel-feedback-list, Stephen Warren, Lee Jones, linux-rpi-kernel, linux-arm-kernel 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. This being said, I already had a similar issue with atmel clocks [1], where a peripheral requires a specific rate and the periph clk can take its source from 2 different PLLs: one that is widely used by other peripherals and which cannot be modified and the other which is not so widely used and can be customized to generate the rate we need. Maybe that's something we should address with a generic solution at some point: clk constraint propagation, clk rate lock or something else (Mike mentioned another approach here [1]). In the meantime, the patch here should do the trick for the bcm2835 platform. > Still, this seems to be correct and fixes the bug. Both patches are: > > Reviewed-by: Eric Anholt <eric@anholt.net> [1]https://patchwork.kernel.org/patch/6204221/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/2] clk: bcm: Support rate change propagation on bcm2835 clocks @ 2016-12-02 19:50 ` Boris Brezillon 0 siblings, 0 replies; 16+ messages in thread From: Boris Brezillon @ 2016-12-02 19:50 UTC (permalink / raw) To: linux-arm-kernel 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. This being said, I already had a similar issue with atmel clocks [1], where a peripheral requires a specific rate and the periph clk can take its source from 2 different PLLs: one that is widely used by other peripherals and which cannot be modified and the other which is not so widely used and can be customized to generate the rate we need. Maybe that's something we should address with a generic solution at some point: clk constraint propagation, clk rate lock or something else (Mike mentioned another approach here [1]). In the meantime, the patch here should do the trick for the bcm2835 platform. > Still, this seems to be correct and fixes the bug. Both patches are: > > Reviewed-by: Eric Anholt <eric@anholt.net> [1]https://patchwork.kernel.org/patch/6204221/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] clk: bcm: Support rate change propagation on bcm2835 clocks 2016-12-02 19:50 ` Boris Brezillon @ 2016-12-02 21:13 ` Eric Anholt -1 siblings, 0 replies; 16+ messages in thread From: Eric Anholt @ 2016-12-02 21:13 UTC (permalink / raw) To: Boris Brezillon Cc: Mike Turquette, Stephen Boyd, linux-clk, Florian Fainelli, Ray Jui, Scott Branden, bcm-kernel-feedback-list, Stephen Warren, Lee Jones, linux-rpi-kernel, linux-arm-kernel [-- 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 --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/2] clk: bcm: Support rate change propagation on bcm2835 clocks @ 2016-12-02 21:13 ` Eric Anholt 0 siblings, 0 replies; 16+ messages in thread From: Eric Anholt @ 2016-12-02 21:13 UTC (permalink / raw) To: linux-arm-kernel 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> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] clk: bcm: Support rate change propagation on bcm2835 clocks 2016-12-01 21:00 ` Boris Brezillon @ 2016-12-08 23:06 ` Stephen Boyd -1 siblings, 0 replies; 16+ messages in thread From: Stephen Boyd @ 2016-12-08 23:06 UTC (permalink / raw) To: Boris Brezillon Cc: Mike Turquette, linux-clk, Florian Fainelli, Ray Jui, Scott Branden, bcm-kernel-feedback-list, Stephen Warren, Lee Jones, Eric Anholt, linux-rpi-kernel, linux-arm-kernel On 12/01, Boris Brezillon wrote: > 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> > --- Applied to clk-next -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/2] clk: bcm: Support rate change propagation on bcm2835 clocks @ 2016-12-08 23:06 ` Stephen Boyd 0 siblings, 0 replies; 16+ messages in thread From: Stephen Boyd @ 2016-12-08 23:06 UTC (permalink / raw) To: linux-arm-kernel On 12/01, Boris Brezillon wrote: > 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> > --- Applied to clk-next -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/2] clk: bcm: Allow rate change propagation to PLLH_AUX on VEC clock 2016-12-01 21:00 ` Boris Brezillon @ 2016-12-01 21:00 ` Boris Brezillon -1 siblings, 0 replies; 16+ messages in thread From: Boris Brezillon @ 2016-12-01 21:00 UTC (permalink / raw) To: Mike Turquette, Stephen Boyd, linux-clk Cc: Florian Fainelli, Ray Jui, Scott Branden, bcm-kernel-feedback-list, Stephen Warren, Lee Jones, Eric Anholt, linux-rpi-kernel, linux-arm-kernel, Boris Brezillon The VEC clock requires needs to be set at exactly 108MHz. Allow rate change propagation on PLLH_AUX to match this requirement wihtout impacting other IPs (PLLH is currently only used by the HDMI encoder, which cannot be enabled when the VEC encoder is enabled). Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> --- drivers/clk/bcm/clk-bcm2835.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c index df96fe6dadab..eaf82f49dede 100644 --- a/drivers/clk/bcm/clk-bcm2835.c +++ b/drivers/clk/bcm/clk-bcm2835.c @@ -1861,7 +1861,12 @@ static const struct bcm2835_clk_desc clk_desc_array[] = { .ctl_reg = CM_VECCTL, .div_reg = CM_VECDIV, .int_bits = 4, - .frac_bits = 0), + .frac_bits = 0, + /* + * Allow rate change propagation only on PLLH_AUX which is + * assigned index 7 in the parent array. + */ + .set_rate_parent = BIT(7)), /* dsi clocks */ [BCM2835_CLOCK_DSI0E] = REGISTER_PER_CLK( -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/2] clk: bcm: Allow rate change propagation to PLLH_AUX on VEC clock @ 2016-12-01 21:00 ` Boris Brezillon 0 siblings, 0 replies; 16+ messages in thread From: Boris Brezillon @ 2016-12-01 21:00 UTC (permalink / raw) To: linux-arm-kernel The VEC clock requires needs to be set at exactly 108MHz. Allow rate change propagation on PLLH_AUX to match this requirement wihtout impacting other IPs (PLLH is currently only used by the HDMI encoder, which cannot be enabled when the VEC encoder is enabled). Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> --- drivers/clk/bcm/clk-bcm2835.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c index df96fe6dadab..eaf82f49dede 100644 --- a/drivers/clk/bcm/clk-bcm2835.c +++ b/drivers/clk/bcm/clk-bcm2835.c @@ -1861,7 +1861,12 @@ static const struct bcm2835_clk_desc clk_desc_array[] = { .ctl_reg = CM_VECCTL, .div_reg = CM_VECDIV, .int_bits = 4, - .frac_bits = 0), + .frac_bits = 0, + /* + * Allow rate change propagation only on PLLH_AUX which is + * assigned index 7 in the parent array. + */ + .set_rate_parent = BIT(7)), /* dsi clocks */ [BCM2835_CLOCK_DSI0E] = REGISTER_PER_CLK( -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] clk: bcm: Allow rate change propagation to PLLH_AUX on VEC clock 2016-12-01 21:00 ` Boris Brezillon @ 2016-12-08 23:06 ` Stephen Boyd -1 siblings, 0 replies; 16+ messages in thread From: Stephen Boyd @ 2016-12-08 23:06 UTC (permalink / raw) To: Boris Brezillon Cc: Mike Turquette, linux-clk, Florian Fainelli, Ray Jui, Scott Branden, bcm-kernel-feedback-list, Stephen Warren, Lee Jones, Eric Anholt, linux-rpi-kernel, linux-arm-kernel On 12/01, Boris Brezillon wrote: > The VEC clock requires needs to be set at exactly 108MHz. Allow rate > change propagation on PLLH_AUX to match this requirement wihtout > impacting other IPs (PLLH is currently only used by the HDMI encoder, > which cannot be enabled when the VEC encoder is enabled). > > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> > --- Applied to clk-next -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/2] clk: bcm: Allow rate change propagation to PLLH_AUX on VEC clock @ 2016-12-08 23:06 ` Stephen Boyd 0 siblings, 0 replies; 16+ messages in thread From: Stephen Boyd @ 2016-12-08 23:06 UTC (permalink / raw) To: linux-arm-kernel On 12/01, Boris Brezillon wrote: > The VEC clock requires needs to be set at exactly 108MHz. Allow rate > change propagation on PLLH_AUX to match this requirement wihtout > impacting other IPs (PLLH is currently only used by the HDMI encoder, > which cannot be enabled when the VEC encoder is enabled). > > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> > --- Applied to clk-next -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2016-12-08 23:06 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 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
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.