> +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 devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html