| Message ID | 1452101585-25933-1-git-send-email-swarren@wwwdotorg.org |
|---|---|
| State | Accepted |
| Delegated to: | Simon Glass |
| Headers | show |
On Thu, Jan 7, 2016 at 1:33 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: > From: Stephen Warren <swarren@nvidia.com> > > If a timer has a zero clock_rate, get_tbclk() will return zero for it, > which will cause tick_to_time() to perform a division-by-zero, which will > crash U-Boot. > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > --- > drivers/timer/timer-uclass.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c > index aca421bdea33..0771562c600d 100644 > --- a/drivers/timer/timer-uclass.c > +++ b/drivers/timer/timer-uclass.c > @@ -47,6 +47,16 @@ static int timer_pre_probe(struct udevice *dev) > return 0; > } > > +static int timer_post_probe(struct udevice *dev) > +{ > + struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev); > + > + if (!uc_priv->clock_rate) > + return -EINVAL; Should we just panic here? > + > + return 0; > +} > + > u64 timer_conv_64(u32 count) > { > /* increment tbh if tbl has rolled over */ > @@ -60,5 +70,6 @@ UCLASS_DRIVER(timer) = { > .id = UCLASS_TIMER, > .name = "timer", > .pre_probe = timer_pre_probe, > + .post_probe = timer_post_probe, > .per_device_auto_alloc_size = sizeof(struct timer_dev_priv), > }; > -- Regards, Bin
On 01/06/2016 10:29 PM, Bin Meng wrote: > On Thu, Jan 7, 2016 at 1:33 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >> From: Stephen Warren <swarren@nvidia.com> >> >> If a timer has a zero clock_rate, get_tbclk() will return zero for it, >> which will cause tick_to_time() to perform a division-by-zero, which will >> crash U-Boot. >> >> Signed-off-by: Stephen Warren <swarren@nvidia.com> >> --- >> drivers/timer/timer-uclass.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c >> index aca421bdea33..0771562c600d 100644 >> --- a/drivers/timer/timer-uclass.c >> +++ b/drivers/timer/timer-uclass.c >> @@ -47,6 +47,16 @@ static int timer_pre_probe(struct udevice *dev) >> return 0; >> } >> >> +static int timer_post_probe(struct udevice *dev) >> +{ >> + struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev); >> + >> + if (!uc_priv->clock_rate) >> + return -EINVAL; > > Should we just panic here? That would prevent the system operating correctly if multiple timers happened to be registered and the other one didn't have this issue. Still, it does seem reasonable to highlight this error. Simon, what do you think here?
Hi, On 6 January 2016 at 22:29, Bin Meng <bmeng.cn@gmail.com> wrote: > On Thu, Jan 7, 2016 at 1:33 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >> From: Stephen Warren <swarren@nvidia.com> >> >> If a timer has a zero clock_rate, get_tbclk() will return zero for it, >> which will cause tick_to_time() to perform a division-by-zero, which will >> crash U-Boot. >> >> Signed-off-by: Stephen Warren <swarren@nvidia.com> >> --- >> drivers/timer/timer-uclass.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) Acked-by: Simon Glass <sjg@chromium.org> >> >> diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c >> index aca421bdea33..0771562c600d 100644 >> --- a/drivers/timer/timer-uclass.c >> +++ b/drivers/timer/timer-uclass.c >> @@ -47,6 +47,16 @@ static int timer_pre_probe(struct udevice *dev) >> return 0; >> } >> >> +static int timer_post_probe(struct udevice *dev) >> +{ >> + struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev); >> + >> + if (!uc_priv->clock_rate) >> + return -EINVAL; > > Should we just panic here? I don't think so - there may be multiple timers, and this error should be reported. If you like you could use a strange error number so it is more obvious (e.g. -ETIME). > >> + >> + return 0; >> +} >> + >> u64 timer_conv_64(u32 count) >> { >> /* increment tbh if tbl has rolled over */ >> @@ -60,5 +70,6 @@ UCLASS_DRIVER(timer) = { >> .id = UCLASS_TIMER, >> .name = "timer", >> .pre_probe = timer_pre_probe, >> + .post_probe = timer_post_probe, >> .per_device_auto_alloc_size = sizeof(struct timer_dev_priv), >> }; >> -- > > Regards, > Bin Regards, Simon
On 11 January 2016 at 09:56, Simon Glass <sjg@chromium.org> wrote: > Hi, > > On 6 January 2016 at 22:29, Bin Meng <bmeng.cn@gmail.com> wrote: >> On Thu, Jan 7, 2016 at 1:33 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>> From: Stephen Warren <swarren@nvidia.com> >>> >>> If a timer has a zero clock_rate, get_tbclk() will return zero for it, >>> which will cause tick_to_time() to perform a division-by-zero, which will >>> crash U-Boot. >>> >>> Signed-off-by: Stephen Warren <swarren@nvidia.com> >>> --- >>> drivers/timer/timer-uclass.c | 11 +++++++++++ >>> 1 file changed, 11 insertions(+) > > Acked-by: Simon Glass <sjg@chromium.org> > >>> >>> diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c >>> index aca421bdea33..0771562c600d 100644 >>> --- a/drivers/timer/timer-uclass.c >>> +++ b/drivers/timer/timer-uclass.c >>> @@ -47,6 +47,16 @@ static int timer_pre_probe(struct udevice *dev) >>> return 0; >>> } >>> >>> +static int timer_post_probe(struct udevice *dev) >>> +{ >>> + struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev); >>> + >>> + if (!uc_priv->clock_rate) >>> + return -EINVAL; >> >> Should we just panic here? > > I don't think so - there may be multiple timers, and this error should > be reported. If you like you could use a strange error number so it is > more obvious (e.g. -ETIME). > >> >>> + >>> + return 0; >>> +} >>> + >>> u64 timer_conv_64(u32 count) >>> { >>> /* increment tbh if tbl has rolled over */ >>> @@ -60,5 +70,6 @@ UCLASS_DRIVER(timer) = { >>> .id = UCLASS_TIMER, >>> .name = "timer", >>> .pre_probe = timer_pre_probe, >>> + .post_probe = timer_post_probe, >>> .per_device_auto_alloc_size = sizeof(struct timer_dev_priv), >>> }; >>> -- Applied to u-boot-dm, thanks!
diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c index aca421bdea33..0771562c600d 100644 --- a/drivers/timer/timer-uclass.c +++ b/drivers/timer/timer-uclass.c @@ -47,6 +47,16 @@ static int timer_pre_probe(struct udevice *dev) return 0; } +static int timer_post_probe(struct udevice *dev) +{ + struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev); + + if (!uc_priv->clock_rate) + return -EINVAL; + + return 0; +} + u64 timer_conv_64(u32 count) { /* increment tbh if tbl has rolled over */ @@ -60,5 +70,6 @@ UCLASS_DRIVER(timer) = { .id = UCLASS_TIMER, .name = "timer", .pre_probe = timer_pre_probe, + .post_probe = timer_post_probe, .per_device_auto_alloc_size = sizeof(struct timer_dev_priv), };