On Wed, Jul 03, 2019 at 07:33:58PM +0300, Alexander Shishkin wrote:
> Greg Kroah-Hartman <gre...@linuxfoundation.org> writes:
> 
> >> +  /*
> >> +   * ->assign() called when buffer 'mode' is set to this driver
> >> +   *   (aka mode_store())
> >> +   * @device:     struct device * of the msc
> >> +   * @mode:       allows the driver to set HW mode (see the enum above)
> >> +   * Returns:     a pointer to a private structure associated with this
> >> +   *              msc or NULL in case of error. This private structure
> >> +   *              will then be passed into all other callbacks.
> >> +   */
> >> +  void    *(*assign)(struct device *dev, int *mode);
> >> +  /* ->unassign():        some other mode is selected, clean up */
> >> +  void    (*unassign)(void *priv);
> >> +  /*
> >> +   * ->alloc_window(): allocate memory for the window of a given
> >> +   *              size
> >> +   * @sgt:        pointer to sg_table, can be overridden by the buffer
> >> +   *              driver, or kept intact
> >> +   * Returns:     number of sg table entries <= number of pages;
> >> +   *              0 is treated as an allocation failure.
> >> +   */
> >> +  int     (*alloc_window)(void *priv, struct sg_table **sgt,
> >> +                          size_t size);
> >> +  void    (*free_window)(void *priv, struct sg_table *sgt);
> >> +  /* ->activate():        trace has started */
> >> +  void    (*activate)(void *priv);
> >> +  /* ->deactivate():      trace is about to stop */
> >> +  void    (*deactivate)(void *priv);
> >> +  /*
> >> +   * ->ready():   window @sgt is filled up to the last block OR
> >> +   *              tracing is stopped by the user; this window contains
> >> +   *              @bytes data. The window in question transitions into
> >> +   *              the "LOCKED" state, indicating that it can't be used
> >> +   *              by hardware. To clear this state and make the window
> >> +   *              available to the hardware again, call
> >> +   *              intel_th_msc_window_unlock().
> >> +   */
> >> +  int     (*ready)(void *priv, struct sg_table *sgt, size_t bytes);
> >> +};
> >
> > Why isn't this based off of 'struct driver'?
> 
> It's not a real driver, in a sense that there's no underlying
> device. None of the usual driver stuff applies.

Then do not call it a "driver", as in the kernel we have a very
well-defined and known definition of a driver.  Call it something else
please.  Yes, naming is hard, but don't try to overload onto an already
existing name.

> It's still a set of callbacks, though. Should this be an elaborate
> comment, should I replace the word "driver" with something else?

Yes.

> I'd really like to avoid shoehorning the whole 'struct device' +
> 'struct driver' here.

Why not?  If you have a driver, just make it a real one.  It not take
all that much boiler-plate code to do so and then you get all of the
things you will want in the end anyway (sysfs representation,
attributes, auto-loading of modules, etc.)

Try doing it "for real" and see what happens.

thanks,

greg k-h

Reply via email to