Re: [RFC PATCH 0/3] introduce: Multistate Switch Class

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

Greg KH <gregkh@suse.de> wrote:

>On Sun, Nov 27, 2011 at 11:43:39PM +0100, Linus Walleij wrote:
>> Hi MyungJoo, Arnd
>> 
>> On Fri, Nov 25, 2011 at 3:02 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Thursday 24 November 2011, MyungJoo Ham wrote:
>> >> For switch ports, which may have different types of cables
>> >> (USB, TA, HDMI, Analog A/V, and others), we often have seperated
>device
>> >> drivers that detect the state changes at the port and device
>drivers that
>> >> do something according to the state changes.
>> >>
>> >> For example, when MAX8997-MUIC detects a Charger cable insertion,
>another
>> >> device driver (such as MAX8903 charger, MAX8997 charger, Charger
>Manager,
>> >> or board file) needs to set charger current limit accordingly and
>when
>> >> MAX8997-MUIC detects a HDMI cable insertion, multimedia device
>drivers
>> >> need to do some operations accordingly.
>> >>
>> >> This patchset supports the usage of notifier for passing such
>information
>> >> between device drivers.
>> >>
>> >> Another issue is that at a single switch port, there might be
>multiple
>> >> and heterogeneous cables attached at the same time. Besides, the
>state
>> >> (Attached or Detached) of each cable may alter independently.
>> >>
>> >> In order to address such issues, Android kernel's "Switch" class
>seems to
>> >> be a good basis and we have implemented "Multistate Switch Class"
>based on
>> >> it. The "Switch" class code of Android kernel is GPL as well.
>> >
>> > How does this relate to the new "pinmux" subsystem that Linus
>Walleij
>> > maintains? Would it be useful to integrate your driver into pinmux
>> > instead of starting a new subsystem?
>> 
>> Looks unrelated to pinmux but very useful.
>> 
>> And the uevent scheme from Arve seems like it's doing the
>> right thing to me, but see below on relation to <linux/input.h>.
>> 
>> The GPIO part has to be reviewed by Grant though.
>> 
>> Our charger code also needs to
>> know when the USB cable is inserted, currently we have a simple
>> cross-call in a header file:
>> static void ab8500_charger_usb_state_changed(u8 bm_usb_state, u16
>mA);
>> 
>> So there is certainly a growing need of subsystems that
>> need to notify each other about things that are happening,
>> the need comes naturally from mobile ICs I think, and
>> I also think they all have one or another custom mechanism
>> in place already.
>> 
>> But - and now we need Dmitry to check the concepts:
>> 
>> Some use this stuff from <linux/input.h> to talk to userspace
>> though the input subsystem:
>> 
>> /*
>>  * Switch events
>>  */
>> 
>> #define SW_LID                  0x00  /* set = lid shut */
>> #define SW_TABLET_MODE          0x01  /* set = tablet mode */
>> #define SW_HEADPHONE_INSERT     0x02  /* set = inserted */
>> #define SW_RFKILL_ALL           0x03  /* rfkill master switch, type
>"any"
>>                                          set = radio enabled */
>> #define SW_RADIO                SW_RFKILL_ALL   /* deprecated */
>> #define SW_MICROPHONE_INSERT    0x04  /* set = inserted */
>> #define SW_DOCK                 0x05  /* set = plugged into dock */
>> #define SW_LINEOUT_INSERT       0x06  /* set = inserted */
>> #define SW_JACK_PHYSICAL_INSERT 0x07  /* set = mechanical switch set
>*/
>> #define SW_VIDEOOUT_INSERT      0x08  /* set = inserted */
>> #define SW_CAMERA_LENS_COVER    0x09  /* set = lens covered */
>> #define SW_KEYPAD_SLIDE         0x0a  /* set = keypad slide out */
>> #define SW_FRONT_PROXIMITY      0x0b  /* set = front proximity sensor
>active */
>> #define SW_ROTATE_LOCK          0x0c  /* set = rotate locked/disabled
>*/
>> #define SW_LINEIN_INSERT        0x0d  /* set = inserted */
>> #define SW_MAX                  0x0f
>> #define SW_CNT                  (SW_MAX+1)
>> 
>> These are *also* switches. You could very well add USB
>> cable insertion to the list above.
>> 
>> So for the userspace part it seems to me that we need to make
>> up our mind about this stuff: is it going to be through input or
>> uevent like in this patch? Or ?both??
>
>Input please, uevent is not for things like switches that are "common",
>but for things that are "uncommon" and don't happen often.

Actually, please do not. I never liked audio-related switches added to input; ALSA guys just wore me down. These are usually not switches that user can flip, they are connections between components. Should we switch betide_carrier_*(), power supply state, etc, etc over to input? I think not.

I haven't looked at the patch yet, but a class that has an attribute that could be queried and emitting uneventful on state change seems like a good diluting for me.

Thanks.

-- 
Dmitry