| Message ID | 1374594138-17080-3-git-send-email-jonathan.austin@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 23, 2013 at 5:42 PM, Jonathan Austin <jonathan.austin@arm.com> wrote: > The order of arguments in the call to vco_set() for the ICST clocks appears to > have been switched in error, which results in the VCO not being initialised > correctly. This in turn stops the integrated LCD on things like Integrator/CP > from working correctly. > > This patch fixes the order and restores the expected functionality. > > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Mike Turquette <mturquette@linaro.org> > Signed-off-by: Jonathan Austin <jonathan.austin@arm.com> Good catch! Reviewed-by: Linus Walleij <linus.walleij@linaro.org> You should probably mark this for stable as well. What I don't understand is why my CLCD VGA output has been working all the time :-( I thought this VCO was necessary for that as well, but apparently it works with the defaults. Yours, Linus Walleij
Quoting Jonathan Austin (2013-07-23 08:42:18) > The order of arguments in the call to vco_set() for the ICST clocks appears to > have been switched in error, which results in the VCO not being initialised > correctly. This in turn stops the integrated LCD on things like Integrator/CP > from working correctly. > > This patch fixes the order and restores the expected functionality. > > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Mike Turquette <mturquette@linaro.org> > Signed-off-by: Jonathan Austin <jonathan.austin@arm.com> Thanks for the fix. Will send out with the next batch. Regards, Mike > --- > drivers/clk/versatile/clk-icst.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/versatile/clk-icst.c b/drivers/clk/versatile/clk-icst.c > index 67ccf4a..f5e4c21 100644 > --- a/drivers/clk/versatile/clk-icst.c > +++ b/drivers/clk/versatile/clk-icst.c > @@ -107,7 +107,7 @@ static int icst_set_rate(struct clk_hw *hw, unsigned long rate, > > vco = icst_hz_to_vco(icst->params, rate); > icst->rate = icst_hz(icst->params, vco); > - vco_set(icst->vcoreg, icst->lockreg, vco); > + vco_set(icst->lockreg, icst->vcoreg, vco); > return 0; > } > > -- > 1.7.9.5
On 24/07/13 20:32, Linus Walleij wrote: > On Tue, Jul 23, 2013 at 5:42 PM, Jonathan Austin > <jonathan.austin@arm.com> wrote: > >> The order of arguments in the call to vco_set() for the ICST clocks appears to >> have been switched in error, which results in the VCO not being initialised >> correctly. This in turn stops the integrated LCD on things like Integrator/CP >> from working correctly. >> >> This patch fixes the order and restores the expected functionality. >> >> Cc: Linus Walleij <linus.walleij@linaro.org> >> Cc: Mike Turquette <mturquette@linaro.org> >> Signed-off-by: Jonathan Austin <jonathan.austin@arm.com> > > Good catch! Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > You should probably mark this for stable as well. > Mike, will you add Cc: stable when you take the fix or shall I resubmit the patch somehow else? > What I don't understand is why my CLCD VGA output has > been working all the time :-( > No, this confuses me too - perhaps the display you've got it plugged in to is really lenient about modes and just made the best of it? > I thought this VCO was necessary for that as well, but apparently > it works with the defaults. Certainly I can't get anything out of the display on my integrator or via the VGA port without this fix and the other one in the series. Jonny > > Yours, > Linus Walleij >
On Thu, Jul 25, 2013 at 11:30 AM, Jonathan Austin <jonathan.austin@arm.com> wrote: >> What I don't understand is why my CLCD VGA output has >> been working all the time :-( >> > > No, this confuses me too - perhaps the display you've got it plugged in to > is really lenient about modes and just made the best of it? I think it's because it is set up into some mode by the boot monitor, as I get the boot monitor prompt on the VGA display. Then as the VCO params get written, it is never really unlocked so the registers are untouched. You may have a different boot monitor? Yours, Linus Walleij
On 25/07/13 10:33, Linus Walleij wrote: > On Thu, Jul 25, 2013 at 11:30 AM, Jonathan Austin > <jonathan.austin@arm.com> wrote: > >>> What I don't understand is why my CLCD VGA output has >>> been working all the time :-( >>> >> >> No, this confuses me too - perhaps the display you've got it plugged in to >> is really lenient about modes and just made the best of it? > > I think it's because it is set up into some mode by the boot > monitor, as I get the boot monitor prompt on the VGA > display. > > Then as the VCO params get written, it is never really unlocked > so the registers are untouched. > > You may have a different boot monitor? > Ahh, that seems highly likely. I don't ever see boot-monitor (boot straight in to uboot) but I don't see anything on the display until the kernel boots, so I don't think BM is setting up the VCO for me. Jonny
On Thu, Jul 25, 2013 at 11:36 AM, Jonathan Austin <jonathan.austin@arm.com> wrote: > On 25/07/13 10:33, Linus Walleij wrote: >> Then as the VCO params get written, it is never really unlocked >> so the registers are untouched. >> >> You may have a different boot monitor? >> > > Ahh, that seems highly likely. I don't ever see boot-monitor (boot straight > in to uboot) but I don't see anything on the display until the kernel boots, > so I don't think BM is setting up the VCO for me. True, if you boot directly into U-boot the boot monitor is never executed and the mode is never set up. Yours, Linus Walleij
Quoting Jonathan Austin (2013-07-25 02:30:29) > On 24/07/13 20:32, Linus Walleij wrote: > > On Tue, Jul 23, 2013 at 5:42 PM, Jonathan Austin > > <jonathan.austin@arm.com> wrote: > > > >> The order of arguments in the call to vco_set() for the ICST clocks appears to > >> have been switched in error, which results in the VCO not being initialised > >> correctly. This in turn stops the integrated LCD on things like Integrator/CP > >> from working correctly. > >> > >> This patch fixes the order and restores the expected functionality. > >> > >> Cc: Linus Walleij <linus.walleij@linaro.org> > >> Cc: Mike Turquette <mturquette@linaro.org> > >> Signed-off-by: Jonathan Austin <jonathan.austin@arm.com> > > > > Good catch! Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > You should probably mark this for stable as well. > > > > Mike, will you add Cc: stable when you take the fix or shall I resubmit > the patch somehow else? I'll add it. Regards, Mike
On 25/07/13 18:44, Mike Turquette wrote: > Quoting Jonathan Austin (2013-07-25 02:30:29) >> On 24/07/13 20:32, Linus Walleij wrote: >>> On Tue, Jul 23, 2013 at 5:42 PM, Jonathan Austin >>> <jonathan.austin@arm.com> wrote: >>> >>>> The order of arguments in the call to vco_set() for the ICST clocks appears to >>>> have been switched in error, which results in the VCO not being initialised >>>> correctly. This in turn stops the integrated LCD on things like Integrator/CP >>>> from working correctly. >>>> >>>> This patch fixes the order and restores the expected functionality. >>>> >>>> Cc: Linus Walleij <linus.walleij@linaro.org> >>>> Cc: Mike Turquette <mturquette@linaro.org> >>>> Signed-off-by: Jonathan Austin <jonathan.austin@arm.com> >>> >>> Good catch! Reviewed-by: Linus Walleij <linus.walleij@linaro.org> >>> You should probably mark this for stable as well. >>> >> >> Mike, will you add Cc: stable when you take the fix or shall I resubmit >> the patch somehow else? > > I'll add it. > Thanks very much. Linus or Mike: Should I put 1/2 in this series in to Russell's tree, or should that go in elsewise? Jonny
diff --git a/drivers/clk/versatile/clk-icst.c b/drivers/clk/versatile/clk-icst.c index 67ccf4a..f5e4c21 100644 --- a/drivers/clk/versatile/clk-icst.c +++ b/drivers/clk/versatile/clk-icst.c @@ -107,7 +107,7 @@ static int icst_set_rate(struct clk_hw *hw, unsigned long rate, vco = icst_hz_to_vco(icst->params, rate); icst->rate = icst_hz(icst->params, vco); - vco_set(icst->vcoreg, icst->lockreg, vco); + vco_set(icst->lockreg, icst->vcoreg, vco); return 0; }
The order of arguments in the call to vco_set() for the ICST clocks appears to have been switched in error, which results in the VCO not being initialised correctly. This in turn stops the integrated LCD on things like Integrator/CP from working correctly. This patch fixes the order and restores the expected functionality. Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Mike Turquette <mturquette@linaro.org> Signed-off-by: Jonathan Austin <jonathan.austin@arm.com> --- drivers/clk/versatile/clk-icst.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)