Re: [patch] add input_enable_device()

[Dmitry Torokhov <dtor_core@ameritech.net>]

--- Stas Sergeev <stsp@aknet.ru> wrote:

> Hello.
> 
> Dmitry Torokhov wrote:
> > I really believe that instead of shoving this into input core you need to
> > split pcspkr driver to allow concurrent access to the hardware.
> I split pcspkr and someone else will say that there is
> already enough of the midlayers to handle the like things,
> to not introduce another one just for the particular driver.
> Besides, I don't beleive people will be happy with having
> 2 modules for just handling the terminal beeps.
> The input midlayer looks like the best solution. It allows
> to deal with the modules as soon as they are loaded. It has
> enough of the information needed to precisely identify the
> module (I now use INPUT_DEVICE_ID_MATCH_BUS). The pcspkr *is*
> an "input driver" after all, so why not to deal with it via
> an input API?

pcspkr is still an input driver. Now you are adding another
driver to drive the same piece of hardware and you need
to mediate access to that hardware. It is not responsibility
of the input layer to do that. Like input core does not handle
PS/2 ports or USB hardware directly it should not do that for
the speaker either. Also, even now pcspkr implementation is
deficient - issuing SND_BELL will kill SND_TONE going at the
moment.

> If the input should not be used for anything
> related to the port IO,

Input core itself should not. It is positioned not at physical
device level. It gets and propagates events from individual
drivers which talk to hardware, maybe via port IO. 
  
>  then why it carries the information
> about the ports and the bus that are used by the device?
> Why does it have the INPUT_DEVICE_ID_MATCH_BUS after all?

For userspace benefits.

> The input API only lacks a very small piece of the functionality -
> disabling the device, which can easily be used by anything else
> in the future. Is there a reason not to include that functionality
> only because the snd-pcsp is going to use it, or is there any *other*
> reason?
>

See above. Your module wants to access hardware concurrently with
another module so implement it properly. While you are fine with
disabling beeps while music is playing otherpeoplr might still want
to hear them.
 
--
Dmitry