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

Daniel Vetter
2019-01-31T08:39:05Z

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