On Mon, Aug 25, 2014 at 9:35 AM, Emil Velikov <emil.l.veli...@gmail.com> wrote: > On 24/08/14 22:15, Martin Peres wrote: >> We will use this subdev to disable temperature reading on cards that did not >> get a sensor calibration in the factory. >> >> Signed-off-by: Martin Peres <martin.pe...@free.fr> >> --- >> configure.ac | 1 + >> drm/Kbuild | 4 ++ >> drm/core/include/subdev/fuse.h | 1 + >> drm/core/subdev/fuse/base.c | 1 + >> drm/core/subdev/fuse/g80.c | 1 + >> drm/core/subdev/fuse/gf100.c | 1 + >> drm/core/subdev/fuse/gm107.c | 1 + >> drm/core/subdev/fuse/priv.h | 1 + >> nvkm/engine/device/gm100.c | 2 + >> nvkm/engine/device/nv50.c | 15 ++++++++ >> nvkm/engine/device/nvc0.c | 10 +++++ >> nvkm/engine/device/nve0.c | 8 ++++ >> nvkm/include/core/device.h | 1 + >> nvkm/include/subdev/fuse.h | 30 +++++++++++++++ >> nvkm/subdev/Makefile.am | 3 +- >> nvkm/subdev/fuse/Makefile.am | 8 ++++ >> nvkm/subdev/fuse/base.c | 62 +++++++++++++++++++++++++++++++ >> nvkm/subdev/fuse/g80.c | 81 >> +++++++++++++++++++++++++++++++++++++++++ >> nvkm/subdev/fuse/gf100.c | 83 >> ++++++++++++++++++++++++++++++++++++++++++ >> nvkm/subdev/fuse/gm107.c | 66 +++++++++++++++++++++++++++++++++ >> nvkm/subdev/fuse/priv.h | 9 +++++ >> 21 files changed, 388 insertions(+), 1 deletion(-) >> create mode 120000 drm/core/include/subdev/fuse.h >> create mode 120000 drm/core/subdev/fuse/base.c >> create mode 120000 drm/core/subdev/fuse/g80.c >> create mode 120000 drm/core/subdev/fuse/gf100.c >> create mode 120000 drm/core/subdev/fuse/gm107.c >> create mode 120000 drm/core/subdev/fuse/priv.h >> create mode 100644 nvkm/include/subdev/fuse.h >> create mode 100644 nvkm/subdev/fuse/Makefile.am >> create mode 100644 nvkm/subdev/fuse/base.c >> create mode 100644 nvkm/subdev/fuse/g80.c >> create mode 100644 nvkm/subdev/fuse/gf100.c >> create mode 100644 nvkm/subdev/fuse/gm107.c >> create mode 100644 nvkm/subdev/fuse/priv.h >> > [snip] >> diff --git a/nvkm/subdev/fuse/base.c b/nvkm/subdev/fuse/base.c >> new file mode 100644 >> index 0000000..d249f2b >> --- /dev/null >> +++ b/nvkm/subdev/fuse/base.c >> @@ -0,0 +1,62 @@ >> +/* >> + * Copyright 2014 Martin Peres >> + * >> + * Permission is hereby granted, free of charge, to any person obtaining a >> + * copy of this software and associated documentation files (the >> "Software"), >> + * to deal in the Software without restriction, including without limitation >> + * the rights to use, copy, modify, merge, publish, distribute, sublicense, >> + * and/or sell copies of the Software, and to permit persons to whom the >> + * Software is furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice shall be included >> in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS >> OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL >> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR >> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, >> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR >> + * OTHER DEALINGS IN THE SOFTWARE. >> + * >> + * Authors: Martin Peres >> + */ >> + >> +#include <subdev/fuse.h> >> + >> +int >> +_nouveau_fuse_init(struct nouveau_object *object) >> +{ >> + struct nouveau_fuse *fuse = (void *)object; >> + int ret; >> + >> + ret = nouveau_subdev_init(&fuse->base); >> + if (ret) >> + return ret; >> + > If you want you can drop the check the extra variable. > >> + return 0; >> +} >> + >> +void >> +_nouveau_fuse_dtor(struct nouveau_object *object) >> +{ >> + struct nouveau_fuse *fuse = (void *)object; >> + nouveau_subdev_destroy(&fuse->base); >> +} >> + >> +int >> +nouveau_fuse_create_(struct nouveau_object *parent, >> + struct nouveau_object *engine, >> + struct nouveau_oclass *oclass, int length, void **pobject) >> +{ >> + struct nouveau_fuse *fuse; >> + int ret; >> + >> + ret = nouveau_subdev_create_(parent, engine, oclass, 0, "FUSE", >> + "fuse", length, pobject); > ^^^^^^^ > I think you want to use &fuse here ? > >> + fuse = *pobject; > Swap the assignment order and move it past the conditional ? > >> + if (ret) >> + return ret; >> + > > + *pobject = fuse; > > And perhaps return 0, to make it obvious and consistent with the second case > below. > >> + return ret; >> +} > [snip] >> diff --git a/nvkm/subdev/fuse/gm107.c b/nvkm/subdev/fuse/gm107.c >> new file mode 100644 >> index 0000000..4ade700 >> --- /dev/null >> +++ b/nvkm/subdev/fuse/gm107.c >> @@ -0,0 +1,66 @@ > [snip] >> +static int >> +gm107_fuse_ctor(struct nouveau_object *parent, struct nouveau_object >> *engine, >> + struct nouveau_oclass *oclass, void *data, u32 size, >> + struct nouveau_object **pobject) >> +{ >> + struct gm107_fuse_priv *priv; >> + int ret; >> + >> + ret = nouveau_fuse_create(parent, engine, oclass, &priv); >> + *pobject = nv_object(priv); > Believe the above assignment should go after the check ? Actually, no. It's correct how it is. If something further up fails, the pointer may still be valid and the destructors will get called automagically by the core to cleanup.
I have some stuff upcoming that removes the need for the massive amounts of explicit glue needed between the pieces, that'll make it far more obvious. Ben. > > Nice job :) > -Emil > >> + if (ret) >> + return ret; >> + >> + return 0; >> +} >> + > > _______________________________________________ > Nouveau mailing list > Nouveau@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/nouveau _______________________________________________ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau