Re: [PATCH 1/3] drm: introduce drm_connector_plug_all() helper

[Alexey Brodkin <Alexey.Brodkin@synopsys.com>]

Hi Daniel,

On Sat, 2016-03-19 at 11:02 +0100, Daniel Vetter wrote:
> On Fri, Mar 18, 2016 at 09:58:49PM +0000, Alexey Brodkin wrote:
> > 
> > Hi Daniel,
> > 
> > On Fri, 2016-03-18 at 19:06 +0100, Daniel Vetter wrote:
> > > 
> > > On Fri, Mar 18, 2016 at 01:01:42PM +0300, Alexey Brodkin wrote:
> > > > 
> > > > 
> > > > As a pair to already existing drm_connector_unplug_all() we're adding
> > > > generic implementation of what is already done in some drivers.
> > > > 
> > > > Once this helper is implemented we'll be ready to switch existing
> > > > driver-specific implementations with generic one.
> > > > 
> > > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > Cc: David Airlie <airlied@linux.ie>
> > > > ---
> > > >  drivers/gpu/drm/drm_crtc.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
> > > >  drivers/gpu/drm/drm_drv.c  |  3 ++-
> > > >  include/drm/drm_crtc.h     |  3 ++-
> > > >  3 files changed, 47 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > > > index 65258ac..ce27420 100644
> > > > --- a/drivers/gpu/drm/drm_crtc.c
> > > > +++ b/drivers/gpu/drm/drm_crtc.c
> > > > @@ -1080,6 +1080,46 @@ void drm_connector_unregister(struct drm_connector *connector)
> > > >  }
> > > >  EXPORT_SYMBOL(drm_connector_unregister);
> > > >  
> > > > +/**
> > > > + * drm_connector_plug_all - register connector userspace interfaces
> > > > + * @dev: drm device
> > > > + *
> > > > + * This function registers all connector userspace interfaces in sysfs. Should
> > > > + * be call when the device is disconnected, e.g. from an usb driver's
> > > Still talks about disconnect ;-) Please also mention that this just calls
> > > drm_connector_register() exactly like this including () to generate a
> > > kerneldoc hyperlink.
> > Well I intentionally left in description of drm_connector_register_all():
> > "Should be call when the device is disconnected, e.g. from an usb driver's
> >  ->connect callback."
> You use "disconnected" for connecting stuff. That doesn't make sense to me
> at all - register_all is for when you want to publish something, not for
> unpublishing when the device disappears. Or maybe this is a case of lost
> in translation, and you mean something else?

Let me try to explain what I meant.

We execute ->connect() callback of USB device when USB device is still in "disconnected" state
(well at least that was my thought). And only when that ->connect() callback succeeds we're
entering "connected" state.

Probably above sentence is not correct. Then could you please help me with correct wording of
comment to drm_connector_register_all()?

-Alexey