Re: [PATCH v10 01/40] components: multiple components for a device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jan 31, 2019 at 08:50:20AM +0100, Greg Kroah-Hartman wrote:
> On Thu, Jan 31, 2019 at 12:29:17PM +0530, Ramalingam C wrote:
> > +void component_match_add_typed(struct device *master,
> > +	struct component_match **matchptr,
> > +	int (*compare_typed)(struct device *, int, void *), void *compare_data)
> > +{
> > +	__component_match_add(master, matchptr, NULL, NULL, compare_typed,
> > +			      compare_data);
> > +}
> > +EXPORT_SYMBOL(component_match_add_typed);
> 
> No comment at all as to what this new global function does?
> 
> > +int component_add_typed(struct device *dev, const struct component_ops *ops,
> > +	int subcomponent)
> > +{
> > +	if (WARN_ON(subcomponent == 0))
> > +		return -EINVAL;
> > +
> > +	return __component_add(dev, ops, subcomponent);
> > +}
> > +EXPORT_SYMBOL_GPL(component_add_typed);
> 
> Same here, no comments at all?
> 
> Please at the very least, document new things that you add, I thought I
> asked for this the last time this patch was posted :(

I replied and asked whether you insist on the docs for this or not, since
nothing has docs (and documenting these two alone is not going to explain
anything frankly). It's also defacto the drm component thing, other
subsystems didn't push their own solution into the core ...

There's also the bikeshed question for what exactly we should call these,
I'm not happy with the _typed suffix. Lockdep uses _class for this, but
kinda doesn't make, _subcomponent is a bit redundant and _sub also doesn't
convey much meaning.

Russell, any better ideas?

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux