Hi Jerome & Stephen, On Mon, Dec 18, 2017 at 12:06 PM, Jerome Brunet <jbru...@baylibre.com> wrote: > On Mon, 2017-12-18 at 11:03 -0800, Stephen Boyd wrote: >> On 12/18, Jerome Brunet wrote: >> > Nothing really prevents a provider from (trying to) register a clock >> > without providing the clock ops structure. >> > >> > We do check the individual fields before using them, but not the >> > structure pointer itself. This may have the usual nasty consequences when >> > the pointer is dereferenced, mostly likely when checking one the field >> > during the initialization. >> >> Yes, that nasty consequence should be a kernel oops, > > Precisely > >> and the >> developer should notice that before submitting the driver for >> inclusion. > > Agreed. But people may make mistakes, which is why (at least partly) we > do checks, isn't it ?
Agreed the developers should test before submitting, but procedurally generated clocks (e.g. registering clocks in a loop using a predictable register map, etc) could lead to a situation where a developer doesn't test every possible iteration. Hypothetical, but easy easy easy to fix with Jerome's patch. > >> I don't think we really care to return an error here >> if this happens. >> > > I don't understand why we would let a oops happen when can catch the error > properly ? > Agreed with Jerome on this one. Let's flip it on its head: any downside to this patch? If not I can merge. Regards, Mike