[Intel-gfx] [PATCH v10 01/40] components: multiple components for a device

[Daniel Vetter]

On Thu, Jan 31, 2019 at 09:12:45AM +0100, Greg Kroah-Hartman wrote:
> On Thu, Jan 31, 2019 at 09:00:30AM +0100, Daniel Vetter wrote:
> > 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 ...
> 
> I thought I responded that I would love to have something for the new
> stuff at the very least.

Didn't see anything fly by ...

> And it's not nice to create new global functions without a single line
> of comment for them as to what they are supposed to be used for.  So I'm
> going to insist on that happening here at the very least.

I'll type the entire thing then. I get the "new stuff at least" drill to
fill gaps (use it plenty myself), but just doesn't make sense here at all
...

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch