> +w2sg0004 UART-attached GPS receiver
> +

I'm wondering why it's tied to the w2sg0004


> +struct w2sg_data {
> +     int             gpio;
> +     int             irq;    /* irq line from RX pin when pinctrl
> +                              * set to 'idle' */
> +     struct regulator *reg;
> +
> +     unsigned long   last_toggle;    /* jiffies when last toggle completed. 
> */
> +     unsigned long   backoff;        /* jiffies since last_toggle when
> +                                      * we try again
> +                                      */
> +     enum {Idle, Down, Up} state;    /* state-machine state. */
> +     bool            requested, is_on;
> +     bool            suspended;
> +     bool            reg_enabled;
> +
> +     struct delayed_work work;
> +     spinlock_t      lock;
> +     struct device   *dev;
> +
> +     struct rfkill   *rfkill;

So its
- a regulator (optional)
- an irq (optional)
- a gpio  (could be optional)
- an optional rfkill
- a pulse time (10ms fixed)
- a backoff time (1 second fixed)


It looks identical to half a dozen other widgets that are found in
Android phones. Would it perhaps be better to make the tiny tweaks to
make it generic

- make the timers configurable
- make the pulse time or high/low selectable for on/off
- make the gpio optional

and just have one driver with the right DT for all similar devices?

Am I missing some w2sg004 specific bits here ?


I think the general model is right, and there will be other slaves that
don't fit the pattern but I do think this one could be generalised.

Alan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to