Hi Daniel,
thank you for the review. I'll work in all your other comments, there's just one I'm not sure what to do about:
On Wed, 2020-12-09 at 17:05 +0100, Daniel Vetter wrote: [...]
+void *__drmm_encoder_alloc(struct drm_device *dev, size_t size, size_t offset,
const struct drm_encoder_funcs *funcs,
int encoder_type, const char *name, ...)
+{
- void *container;
- struct drm_encoder *encoder;
- va_list ap;
- int ret;
- if (WARN_ON(funcs && funcs->destroy))
return ERR_PTR(-EINVAL);
- container = drmm_kzalloc(dev, size, GFP_KERNEL);
- if (!container)
return ERR_PTR(-EINVAL);
- encoder = container + offset;
- va_start(ap, name);
- ret = __drm_encoder_init(dev, encoder, funcs, encoder_type, name, ap);
- va_end(ap);
- if (ret)
return ERR_PTR(ret);
- ret = drmm_add_action_or_reset(dev, drmm_encoder_alloc_release, encoder);
- if (ret)
return ERR_PTR(ret);
- return container;
+} +EXPORT_SYMBOL(__drmm_encoder_alloc);
[...]
- @encoder_type: user visible type of the encoder
- @name: printf style format string for the encoder name, or NULL for default name
- Allocates and initializes an encoder. Encoder should be subclassed as part of
- driver encoder objects. Cleanup is automatically handled through registering
- drm_encoder_cleanup() with drmm_add_action().
- The @drm_encoder_funcs.destroy hook must be NULL.
- Returns:
- Pointer to new encoder, or ERR_PTR on failure.
- */
+#define drmm_encoder_alloc(dev, type, member, funcs, encoder_type, name, ...) \
- ((type *)__drmm_encoder_alloc(dev, sizeof(type), \
Need to upcast with container_of or this breaks if the base class is in the wrong spot.
This is modeled after devm_drm_dev_alloc(). Like __devm_drm_dev_alloc(), __drmm_encoder_alloc() returns a pointer to the allocated container structure, not a pointer to the embedded struct drm_encoder. I think this direct cast is correct, unless you suggest that __drmm_encoder_alloc should return encoder instead of container?
regards Philipp