search for: gm20b_clk_new_speedo0

...he core? > If there's *any* error, it'll check the pointer, if it's non-NULL, > it'll call the destructor. If kzalloc() fails, the pointer will be > NULL, there's no double-free bug. *every* subdev is written this way > to avoid duplicating cleanup logic. Actually, gm20b_clk_new_speedo0() may have a bug here if kzalloc() fails as it doesn't overwrite the previous pointer from gm20b_clk_new(). That whole ctor() sequence is written a little strangely. > > Ben. > > > > Regards, > > Dinghao

> > > For security, I will release this pointer only on error paths in this function. > > Do you tend to release objects (which are referenced by pointers)? > I just found that clk is referenced by pclk in this function. When clk is freed, pclk will be allocated in gm20b_clk_new_speedo0(). Thus we should not release clk in this function and there is no bug here. Thank you for reminding me! Regards, Dinghao

> > I just found that clk is referenced by pclk in this function. When clk is freed, > > pclk will be allocated in gm20b_clk_new_speedo0(). Thus we should not release clk > > in this function and there is no bug here. > > Can there be a need to release a clock object after a failed gk20a_clk_ctor() call? > I think this mainly depends on pclk pointer. It seems that the caller of gm20b_clk_new() always expects pclk...

...() returns -ENOMEM, all of its callers (and callers of callers) will be influenced if there is a failed kzalloc inside which. In this case, maybe we should check the return value of gk20a_clk_ctor() and release clk if it returns -ENOMEM. And many other functions also have the same issue (e.g., gm20b_clk_new_speedo0). Do you have any idea about this problem? Regards, Dinghao

> I just found that clk is referenced by pclk in this function. When clk is freed, > pclk will be allocated in gm20b_clk_new_speedo0(). Thus we should not release clk > in this function and there is no bug here. Can there be a need to release a clock object after a failed gk20a_clk_ctor() call? Regards, Markus

> In this case, maybe we should check the return value of > gk20a_clk_ctor() and release clk if it returns -ENOMEM. All error situations (including failed memory allocations) can matter here. > And many other functions also have the same issue > (e.g. gm20b_clk_new_speedo0). I recommend to increase the error detection and improve the desired exception handling accordingly. Regards, Markus

> > > It's the same when gm20b_clk_new() returns from elsewhere following this call. > > I suggest to reconsider the interpretation of the software situation once more. > Can it be that the allocated clock object should be kept usable even after > a successful return from this function? > It's possible that we expect an usable clk pointer, though I could not

Hi Ben, > > When gk20a_clk_ctor() returns an error code, pointer "clk" > > should be released. It's the same when gm20b_clk_new() > > returns from elsewhere following this call. > This shouldn't be necessary. If a subdev constructor fails, and > returns a pointer, the core will call the destructor to clean things > up. > I'm not familiar with

This series adds support for GM20B PLL's Maxwell features, namely glitchless switch and (more importantly) DFS support. DFS lets the PLL lower its output speed according to input current variations, making the clock more stable and allowing it to run safely at lower voltage. All GM20B additions are done in the last patch, which consequently ends up being considerably big ; fortunately, it