Am 20.03.2015 um 09:54 schrieb NeilBrown <neilb@xxxxxxx>: > On Fri, 20 Mar 2015 08:54:38 +0100 "Dr. H. Nikolaus Schaller" > <hns@xxxxxxxxxxxxx> wrote: > >> >> Am 18.03.2015 um 06:58 schrieb NeilBrown <neil@xxxxxxxxxx>: >> >>> If a platform has a particular device permanently attached to a UART, >>> there may be out-of-band signaling necessary to power the device >>> on and off. >>> >>> This driver controls that signalling for a number of different devices. >>> It can >>> - enable/disable a regulator >>> - toggle a GPIO >>> - register an 'rfkill' which can force the device to be off. >>> >>> When the rfkill is absent or unblocked, the device will be on when the >>> associated tty device is open, and closed otherwise. >>> >>> Signed-off-by: NeilBrown <neil@xxxxxxxxxx> >>> --- >>> .../bindings/tty_slave/wi2wi,w2cbw003.txt | 19 + >>> .../bindings/tty_slave/wi2wi,w2sg0004.txt | 37 + >>> .../devicetree/bindings/vendor-prefixes.txt | 1 >>> drivers/tty/slave/Kconfig | 14 + >>> drivers/tty/slave/Makefile | 2 >>> drivers/tty/slave/serial-power-manager.c | 510 ++++++++++++++++++++ >>> 6 files changed, 583 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/tty_slave/wi2wi,w2cbw003.txt >>> create mode 100644 Documentation/devicetree/bindings/tty_slave/wi2wi,w2sg0004.txt >>> create mode 100644 drivers/tty/slave/serial-power-manager.c >>> >>> diff --git a/Documentation/devicetree/bindings/tty_slave/wi2wi,w2cbw003.txt b/Documentation/devicetree/bindings/tty_slave/wi2wi,w2cbw003.txt >>> new file mode 100644 >>> index 000000000000..cfe6ee5e01e9 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/tty_slave/wi2wi,w2cbw003.txt >>> @@ -0,0 +1,19 @@ >>> +wi2wi bluetooth module >>> + >>> +This is accessed via a serial port and is largely controlled via that >>> +link. Extra configuration is needed to enable power on/off >>> + >>> +Required properties: >>> +- compatible: "wi2wi,w2cbw003" >>> +- vdd-supply: regulator used to power the device. >>> + >>> +The node for this device must be the child of a UART. >>> + >>> +Example: >>> + >>> +&uart1 { >>> + bluetooth { >>> + compatible = "wi2wi,w2cbw003"; >>> + vdd-supply = <&vaux4>; >>> + }; >>> +}; >> >> Wouldn’t it be easier to simply write >> >> &uart1 { >> vdd-suppy = <&vaux4>; >> } > > Easier to write: certainly. > Easier to justify? No. I just justified. > Easier to get merged upstream? Definitely not. Are you the maintainer? > After all, the uart itself doesn't require a power supply. > It is the device connected to the uart which requires the power supply. Yes. > > >> >>> diff --git a/Documentation/devicetree/bindings/tty_slave/wi2wi,w2sg0004.txt b/Documentation/devicetree/bindings/tty_slave/wi2wi,w2sg0004.txt >>> new file mode 100644 >>> index 000000000000..fdc52cf56533 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/tty_slave/wi2wi,w2sg0004.txt >>> @@ -0,0 +1,37 @@ >>> +wi2wi GPS device >>> + >>> +This is accessed via a serial port and is largely controlled via that >>> +link. Extra configuration is needed to enable power on/off >>> + >>> +Required properties: >>> +- compatible: "wi2wi,w2sg0004" >>> +- gpios: gpios used to toggle 'on/off' pin >>> +- interrupts: interrupt generated by RX pin when device >>> + should be off >>> + >>> +Optional properties: >>> +- vdd-supply: regulator used to power antenna >>> +- pinctrl: "default", "off" >>> + if "off" setting is provided it is imposed when device should >>> + be off. This can route the RX pin to a GPIO interrupt. >>> + >>> +The w2sg0004 uses a pin-toggle both to power-on and to >>> +power-off, so the driver needs to detect what state it is in. >>> +It does this by detecting characters on the RX line. >>> +When it should be off, these can optionally be detected by a GPIO. >>> + >>> +The node for this device must be the child of a UART. >>> + >>> +Example: >>> +&uart2 { >>> + gps { >>> + compatible = "wi2iw,w2sg0004"; >>> + vdd-supply = <&vsim>; >>> + gpios = <&gpio5 17 0>; /* GPIO_145 */ >>> + interrupts-extended = <&gpio5 19 0>; /* GPIO_147 */ >>> + /* When off, switch RX to be an interrupt */ >>> + pinctrl-names = "default", "off"; >>> + pinctrl-0 = <&uart2_pins>; >>> + pinctrl-1 = <&uart2_pins_rx_gpio>; >>> + }; >>> +}; >> >> If the wi2wi driver is a regulator driver one would write >> >> / { >> gps-regulator: gps { >> compatible = "wi2iw,w2sg0004"; >> vdd-supply = <&vsim>; >> gpios = <&gpio5 17 0>; /* GPIO_145 */ >> interrupts-extended = <&gpio5 19 0>; /* GPIO_147 */ >> /* When off, switch RX to be an interrupt */ >> pinctrl-names = "default", "off"; >> pinctrl-0 = <&uart2_pins>; >> pinctrl-1 = <&uart2_pins_rx_gpio>; >> }; >> } >> >> &uart2 { >> vdd-suppy = <&gps-regulator>; >> }; >> >> Which IMHO better describes that the uart controls power of a separate driver. > > But the uart doesn’t control the power. > An 'open' on the tty causes one driver to turn on a regulator, and another > driver to activate a uart so that the device represented by the tty can be > communicated with. Ok, that is a detail I have mixed up tty and uart. > >> >> And this pattern for writing a DT would IMHO be more flexible because you >> can „connect“ to any regulator, e.g. a regulator for a RS232 level shifter. > > I'm sure there are lots of ways we could find to make DT more flexible, only > then it wouldn't be DT any more - it would be something similar but different. > > There needs to be one device-node for each device, and that device-node needs > to be a child of the device-node for the device which is the primary > connection to the child device. Then please explain to me nodes like / { compatible = "ti,omap3-gta04", "ti,omap36xx", "ti,omap3"; cpus { cpu@0 { cpu0-supply = <&vcc>; }; }; According to the rule you apply here it should be something like cpu@0 { regulator { … } > That is how devicetree is structured - for > better or worse. > > > >> >> >>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt >>> index 389ca1347a77..81d259303710 100644 >>> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt >>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt >>> @@ -189,6 +189,7 @@ variscite Variscite Ltd. >>> via VIA Technologies, Inc. >>> virtio Virtual I/O Device Specification, developed by the OASIS consortium >>> voipac Voipac Technologies s.r.o. >>> +wi2wi wi2wi Inc. http://www.wi2wi.com/ >>> winbond Winbond Electronics corp. >>> wlf Wolfson Microelectronics >>> wm Wondermedia Technologies, Inc. >>> diff --git a/drivers/tty/slave/Kconfig b/drivers/tty/slave/Kconfig >>> index 3976760c2e28..05c5d966ae57 100644 >>> --- a/drivers/tty/slave/Kconfig >>> +++ b/drivers/tty/slave/Kconfig >>> @@ -5,3 +5,17 @@ menuconfig TTY_SLAVE >>> Devices which attach via a uart, but need extra >>> driver support for power management etc. >>> >>> +if TTY_SLAVE >>> + >>> +config SERIAL_POWER_MANAGER >>> + tristate "Power Management controller for serial-attached devices" >>> + default n >>> + help >>> + Some devices permanently attached via a UART can benefit from >>> + being power-managed when the tty device is opened or closed. >>> + This driver can support several such devices with simple >>> + power requirements such as enabling a regulator. >>> + >>> + If in doubt, say 'N' >>> + >>> +endif >>> diff --git a/drivers/tty/slave/Makefile b/drivers/tty/slave/Makefile >>> index 65669acb392e..a2f7d2847319 100644 >>> --- a/drivers/tty/slave/Makefile >>> +++ b/drivers/tty/slave/Makefile >>> @@ -1,2 +1,4 @@ >>> >>> obj-$(CONFIG_TTY_SLAVE) += tty_slave_core.o >>> + >>> +obj-$(CONFIG_SERIAL_POWER_MANAGER) += serial-power-manager.o >>> diff --git a/drivers/tty/slave/serial-power-manager.c b/drivers/tty/slave/serial-power-manager.c >>> new file mode 100644 >>> index 000000000000..662a526d8630 >>> --- /dev/null >>> +++ b/drivers/tty/slave/serial-power-manager.c >>> @@ -0,0 +1,510 @@ >>> +/* >>> + * Serial-power-manager >>> + * tty-slave device that intercepts open/close events on the tty, >>> + * and turns power on/off for the device which is connected. >>> + * >>> + * Currently supported devices: >>> + * wi2wi,w2sg0004 - GPS with on/off toggle on a GPIO >>> + * wi2wi,w2cbw003 - bluetooth port; powered by regulator. >>> + * >>> + * When appropriate, an RFKILL will be registered which >>> + * can power-down the device even when it is open. >>> + * >>> + * Device can be turned on either by >>> + * - enabling a regulator. Disable to turn off >>> + * - toggling a GPIO. Toggle again to turn off. This requires >>> + * that we know the current state. It is assumed to be 'off' >>> + * at boot, however if an interrupt can be generated when on, >>> + * such as by connecting RX to a GPIO, that can be used to detect >>> + * if the device is on when it should be off. >> >> Why does this driver mix both things? >> >> The only thing they have in common is that both are uart slaves >> and that they have a serial interface. But power control is very >> different. > > Because if I wrote two drivers, they would have more code in common than they > would have differences. I mostly can see the w2sg0004 part as an addition on top of a common boilerplate part. > > >> >> One driver per fundamentally different chip... >> >>> + */ >>> + >>> +#include <linux/module.h> >>> +#include <linux/slab.h> >>> +#include <linux/err.h> >>> +#include <linux/regulator/consumer.h> >>> +#include <linux/platform_device.h> >>> +#include <linux/of_device.h> >>> +#include <linux/tty.h> >>> +#include <linux/gpio.h> >>> +#include <linux/of_gpio.h> >>> +#include <linux/of_irq.h> >>> +#include <linux/interrupt.h> >>> +#include <linux/delay.h> >>> +#include <linux/rfkill.h> >>> + >>> +#include <linux/tty_slave.h> >>> + >>> +/* This is used for testing. Setting this module parameter >>> + * will simulate booting with the device "on" >>> + */ >>> +static bool toggle_on_probe = false; >>> +module_param(toggle_on_probe, bool, 0); >>> +MODULE_PARM_DESC(toggle_on_probe, "simulate power-on with devices active"); >>> + >>> +struct spm_config { >>> + int rfkill_type; /* type of rfkill to register */ >>> + int toggle_time; /* msec to pulse GPIO for on/off */ >>> + int toggle_gap; /* min msecs between toggles */ >>> + bool off_in_suspend; >>> +} >>> + simple_config = { >>> + .off_in_suspend = true, >>> + }, >>> + w2sg_config = { >>> + .rfkill_type = RFKILL_TYPE_GPS, >> >> The driver pretends to be generic by its name but incorporates a lot of specific >> knowledge about the w2sg chip, e.g. that it is a GPS chip. >> >>> + .toggle_time = 10, >>> + .toggle_gap = 500, >>> + .off_in_suspend = true, >>> + }; >>> + >>> +const static struct of_device_id spm_dt_ids[] = { >>> + { .compatible = "wi2wi,w2sg0004", .data = &w2sg_config}, >>> + { .compatible = "wi2wi,w2cbw003", .data = &simple_config}, >> >> Well, how large will this table become if other uart slave device types >> are added? > > When that becomes a problem it can trivially be solved. While it is not a > problem there is no value in solving it. thought-terminating argument? > > >> >>> + {} >>> +}; >>> + >>> +struct spm_data { >>> + const struct spm_config *config; >>> + struct gpio_desc *gpiod; >>> + int irq; /* irq line from RX pin when pinctrl >>> + * set to 'idle' */ >>> + struct regulator *reg; >>> + >>> + unsigned long toggle_time; >>> + unsigned long toggle_gap; >>> + 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. */ >>> + >>> + int open_cnt; >>> + bool requested, is_on; >>> + bool suspended; >>> + bool reg_enabled; >>> + >>> + struct pinctrl *pins; >>> + struct pinctrl_state *pins_off; >>> + >>> + struct delayed_work work; >>> + spinlock_t lock; >>> + struct device *dev; >>> + >>> + struct rfkill *rfkill; >>> + >>> + int (*old_open)(struct tty_struct * tty, struct file * filp); >>> + void (*old_close)(struct tty_struct * tty, struct file * filp); >>> + >>> +}; >>> + >>> +/* When a device is powered on/off by toggling a GPIO we perform >>> + * all the toggling via a workqueue to ensure only one toggle happens >>> + * at a time and to allow easy timing. >>> + * This is managed as a state machine which transitions >>> + * Idle -> Down -> Up -> Idle >>> + * The GPIO is held down for toggle_time and then up for toggle_time, >>> + * and then we assume the device has changed state. >>> + * We never toggle until at least toggle_gap has passed since the >>> + * last toggle. >>> + */ >>> +static void toggle_work(struct work_struct *work) >>> +{ >>> + struct spm_data *data = container_of( >>> + work, struct spm_data, work.work); >>> + >>> + if (data->gpiod == NULL) >>> + return; >>> + >>> + spin_lock_irq(&data->lock); >>> + switch (data->state) { >>> + case Up: >>> + data->state = Idle; >>> + if (data->requested == data->is_on) >>> + break; >>> + if (!data->requested) >>> + /* Assume it is off unless activity is detected */ >>> + break; >>> + /* Try again in a while unless we get some activity */ >>> + dev_dbg(data->dev, "Wait %dusec until retry\n", >>> + jiffies_to_msecs(data->backoff)); >>> + schedule_delayed_work(&data->work, data->backoff); >>> + break; >>> + case Idle: >>> + if (data->requested == data->is_on) >>> + break; >>> + >>> + /* Time to toggle */ >>> + dev_dbg(data->dev, "Starting toggle to turn %s\n", >>> + data->requested ? "on" : "off"); >>> + data->state = Down; >>> + spin_unlock_irq(&data->lock); >>> + gpiod_set_value_cansleep(data->gpiod, 1); >>> + schedule_delayed_work(&data->work, data->toggle_time); >>> + >>> + return; >>> + >>> + case Down: >>> + data->state = Up; >>> + data->last_toggle = jiffies; >>> + dev_dbg(data->dev, "Toggle completed, should be %s now.\n", >>> + data->is_on ? "off" : "on"); >>> + data->is_on = ! data->is_on; >>> + spin_unlock_irq(&data->lock); >>> + >>> + gpiod_set_value_cansleep(data->gpiod, 0); >>> + schedule_delayed_work(&data->work, data->toggle_time); >>> + >>> + return; >>> + } >>> + spin_unlock_irq(&data->lock); >>> +} >>> + >>> +static irqreturn_t spm_isr(int irq, void *dev_id) >>> +{ >>> + struct spm_data *data = dev_id; >>> + unsigned long flags; >>> + >>> + spin_lock_irqsave(&data->lock, flags); >>> + if (!data->requested && !data->is_on && data->state == Idle && >>> + time_after(jiffies, data->last_toggle + data->backoff)) { >>> + data->is_on = 1; >>> + data->backoff *= 2; >>> + dev_dbg(data->dev, "Received data, must be on. Try to turn off\n"); >>> + if (!data->suspended) >>> + schedule_delayed_work(&data->work, 0); >>> + } >>> + spin_unlock_irqrestore(&data->lock, flags); >>> + return IRQ_HANDLED; >>> +} >>> + >>> +static void spm_on(struct spm_data *data) >>> +{ >>> + if (!data->rfkill || !rfkill_blocked(data->rfkill)) { >>> + unsigned long flags; >>> + >>> + if (!data->reg_enabled && >>> + data->reg && >>> + regulator_enable(data->reg) == 0) >>> + data->reg_enabled = true; >>> + >>> + spin_lock_irqsave(&data->lock, flags); >>> + if (!data->requested) { >>> + dev_dbg(data->dev, "TTY open - turn device on\n"); >>> + data->requested = true; >>> + data->backoff = data->toggle_gap; >>> + if (data->irq > 0) { >>> + disable_irq(data->irq); >>> + pinctrl_pm_select_default_state(data->dev); >>> + } >>> + if (!data->suspended && data->state == Idle) >>> + schedule_delayed_work(&data->work, 0); >>> + } >>> + spin_unlock_irqrestore(&data->lock, flags); >>> + } >>> +} >>> + >>> +static int spm_open(struct tty_struct *tty, struct file *filp) >>> +{ >>> + struct spm_data *data = dev_get_drvdata(tty->dev->parent); >>> + >>> + data->open_cnt++; >>> + spm_on(data); >>> + if (data->old_open) >>> + return data->old_open(tty, filp); >>> +} >>> + >>> +static void spm_off(struct spm_data *data) >>> +{ >>> + unsigned long flags; >>> + >>> + if (data->reg && data->reg_enabled) >>> + if (regulator_disable(data->reg) == 0) >>> + data->reg_enabled = false; >>> + >>> + spin_lock_irqsave(&data->lock, flags); >>> + if (data->requested) { >>> + data->requested = false; >>> + data->backoff = data->toggle_gap; >>> + if (data->pins_off) { >>> + pinctrl_select_state(data->pins, >>> + data->pins_off); >>> + enable_irq(data->irq); >>> + } >>> + if (!data->suspended && data->state == Idle) >>> + schedule_delayed_work(&data->work, 0); >>> + } >>> + spin_unlock_irqrestore(&data->lock, flags); >>> +} >>> + >>> +static void spm_close(struct tty_struct *tty, struct file *filp) >>> +{ >>> + struct spm_data *data = dev_get_drvdata(tty->dev->parent); >>> + >>> + data->open_cnt--; >>> + if (!data->open_cnt) { >>> + dev_dbg(data->dev, "TTY closed - turn device off\n"); >>> + spm_off(data); >>> + } >>> + >>> + if (data->old_close) >>> + data->old_close(tty, filp); >>> +} >>> + >>> +static int spm_rfkill_set_block(void *vdata, bool blocked) >>> +{ >>> + struct spm_data *data = vdata; >>> + >>> + dev_dbg(data->dev, "rfkill_set_blocked %d\n", blocked); >>> + if (blocked) >>> + spm_off(data); >>> + >>> + if (!blocked && >>> + data->open_cnt) >>> + spm_on(data); >>> + >>> + return 0; >>> +} >>> + >>> +static struct rfkill_ops spm_rfkill_ops = { >>> + .set_block = spm_rfkill_set_block, >>> +}; >>> + >>> +static int spm_suspend(struct device *dev) >>> +{ >>> + /* Ignore incoming data and just turn device off. >>> + * we cannot really wait for a separate thread to >>> + * do things, so we disable that and do it all >>> + * here >>> + */ >>> + struct spm_data *data = dev_get_drvdata(dev); >>> + >>> + spin_lock_irq(&data->lock); >>> + data->suspended = true; >>> + spin_unlock_irq(&data->lock); >>> + if (!data->config->off_in_suspend) >>> + return 0; >>> + >>> + if (data->gpiod) { >>> + >>> + cancel_delayed_work_sync(&data->work); >>> + if (data->state == Down) { >>> + dev_dbg(data->dev, "Suspending while GPIO down - raising\n"); >>> + msleep(data->config->toggle_time); >>> + gpiod_set_value_cansleep(data->gpiod, 0); >>> + data->last_toggle = jiffies; >>> + data->is_on = !data->is_on; >>> + data->state = Up; >>> + } >>> + if (data->state == Up) { >>> + msleep(data->config->toggle_time); >>> + data->state = Idle; >>> + } >>> + if (data->is_on) { >>> + dev_dbg(data->dev, "Suspending while device on: toggling\n"); >>> + gpiod_set_value_cansleep(data->gpiod, 1); >>> + msleep(data->config->toggle_time); >>> + gpiod_set_value_cansleep(data->gpiod, 0); >>> + data->is_on = 0; >>> + } >>> + } >>> + >>> + if (data->reg && data->reg_enabled) >>> + if (regulator_disable(data->reg) == 0) >>> + data->reg_enabled = false; >>> + >>> + return 0; >>> +} >>> + >>> +static int spm_resume(struct device *dev) >>> +{ >>> + struct spm_data *data = dev_get_drvdata(dev); >>> + >>> + spin_lock_irq(&data->lock); >>> + data->suspended = false; >>> + spin_unlock_irq(&data->lock); >>> + schedule_delayed_work(&data->work, 0); >>> + >>> + if (data->open_cnt && >>> + (!data->rfkill || !rfkill_blocked(data->rfkill))) { >>> + if (!data->reg_enabled && >>> + data->reg && >>> + regulator_enable(data->reg) == 0) >>> + data->reg_enabled = true; >>> + } >>> + return 0; >>> +} >>> + >>> +static const struct dev_pm_ops spm_pm_ops = { >>> + SET_SYSTEM_SLEEP_PM_OPS(spm_suspend, spm_resume) >>> +}; >>> + >>> +static int spm_probe(struct device *dev) >>> +{ >>> + struct tty_slave *slave = container_of(dev, struct tty_slave, dev); >>> + struct spm_data *data; >>> + struct regulator *reg; >>> + int err; >>> + const struct of_device_id *id; >>> + const char *name; >>> + >>> + if (dev->parent == NULL) >>> + return -ENODEV; >>> + >>> + id = of_match_device(spm_dt_ids, dev); >>> + if (!id) >>> + return -ENODEV; >>> + >>> + if (dev->of_node && dev->of_node->name) >>> + name = dev->of_node->name; >>> + else >>> + name = "serial-power-manager"; >>> + >>> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); >>> + if (!data) >>> + return -ENOMEM; >>> + >>> + data->config = id->data; >>> + data->toggle_time = msecs_to_jiffies(data->config->toggle_time) + 1; >>> + data->toggle_gap = msecs_to_jiffies(data->config->toggle_gap) + 1; >>> + data->last_toggle = jiffies; >>> + data->backoff = data->toggle_gap; >>> + data->state = Idle; >>> + spin_lock_init(&data->lock); >>> + INIT_DELAYED_WORK(&data->work, toggle_work); >>> + >>> + /* If a regulator is provided, it is enabled on 'open' >>> + * and disabled on 'release' >>> + */ >>> + reg = devm_regulator_get(dev, "vdd"); >>> + if (IS_ERR(reg)) { >>> + err = PTR_ERR(reg); >>> + if (err != -ENODEV) >>> + goto out; >>> + } else >>> + data->reg = reg; >>> + >>> + /* If an irq is provided, any transitions are taken as >>> + * indication that the device is currently "on" >>> + */ >>> + data->irq = of_irq_get(dev->of_node, 0); >>> + if (data->irq < 0) { >>> + err = data->irq; >>> + if (err != -EINVAL) >>> + goto out; >>> + } else { >>> + dev_dbg(dev, "IRQ configured: %d\n", data->irq); >>> + >>> + irq_set_status_flags(data->irq, IRQ_NOAUTOEN); >>> + err = devm_request_irq(dev, data->irq, spm_isr, >>> + IRQF_TRIGGER_FALLING, >>> + name, data); >>> + >>> + if (err) >>> + goto out; >>> + >>> + } >> >> Up to here it is generic and makes no assumptions about a specific >> device. >> >>> + >>> + /* If a gpio is provided, then it is used to turn the device >>> + * on/off. >> >> You have a compatible record (for two well defined chips), but you >> do control which functions are really used by providing/not providing >> some DT parameters. >> >> Either one is redundant. >> >>> + * If toggle_time is zero, then the GPIO directly controls >>> + * the device. >> >> Very hidden and non-obvoius functionality. > > Yes, that should probably be documented more clearly, or removed. > > >> >>> If non-zero, then the GPIO must be toggled to >>> + * change the state of the device. >> >> All the following is very special logic for the w2sg0004 chip which should be >> divided out into a separate driver. >> >> Marek and me already had proposed such a chip specific driver (to be located >> in drivers/misc) some months ago. It would encapsulate everything w2sg0004 >> specific and present itself as a regulator (because that is its main purpose: >> control the LDO regulator inside the w2sg0004 chip). > > Presenting itself as a regulator would be wrong because it isn’t a regulator. It has a regulator that can be controlled by a gpio… Another example to think about: the twl4030 is also not a regulator. Nevertheless they present some regulator nodes. Or take the OMAP3 pbias_regulator. The OMAP3 isn’t a regulator as well but has an internal pbias_regulator that needs to be controlled. BR, Nikolaus > > > Thanks, > NeilBrown > > >> >> We can resubmit it as soon as this driver stabilizes and finds acceptance. >> >>> + */ >>> + data->gpiod = devm_gpiod_get(dev, NULL, GPIOD_OUT_LOW); >>> + if (IS_ERR(data->gpiod)) { >>> + err = PTR_ERR(data->gpiod); >>> + if (err != -ENOENT) >>> + goto out; >>> + data->gpiod = NULL; >>> + } else >>> + dev_dbg(dev, "GPIO configured: %d\n", >>> + desc_to_gpio(data->gpiod)); >>> + >>> + /* If an 'off' pinctrl state is defined, we apply that >>> + * when the device is assumed to be off. This is expected to >>> + * route the 'rx' line to the 'irq' interrupt. >>> + */ >>> + data->pins = devm_pinctrl_get(dev); >>> + if (data->pins && data->irq > 0) { >>> + data->pins_off = pinctrl_lookup_state(data->pins, "off"); >>> + if (IS_ERR(data->pins_off)) >>> + data->pins_off = NULL; >>> + } >>> + >>> + if (data->config->rfkill_type) { >>> + data->rfkill = rfkill_alloc(name, dev, >>> + data->config->rfkill_type, >>> + &spm_rfkill_ops, data); >>> + if (!data->rfkill) { >>> + err = -ENOMEM; >>> + goto out; >>> + } >>> + err = rfkill_register(data->rfkill); >>> + if (err) { >>> + dev_err(dev, "Cannot register rfkill device"); >>> + rfkill_destroy(data->rfkill); >>> + goto out; >>> + } >>> + } >>> + dev_set_drvdata(dev, data); >>> + data->dev = dev; >>> + data->old_open = slave->ops.open; >>> + data->old_close = slave->ops.close; >>> + slave->ops.open = spm_open; >>> + slave->ops.close = spm_close; >>> + tty_slave_finalize(slave); >>> + >>> + if (data->pins_off) >>> + pinctrl_select_state(data->pins, data->pins_off); >>> + if (data->irq > 0) >>> + enable_irq(data->irq); >>> + >>> + if (toggle_on_probe && data->gpiod) { >>> + dev_dbg(data->dev, "Performing initial toggle\n"); >>> + gpiod_set_value_cansleep(data->gpiod, 1); >>> + msleep(data->config->toggle_time); >>> + gpiod_set_value_cansleep(data->gpiod, 0); >>> + msleep(data->config->toggle_time); >>> + } >>> + err = 0; >>> +out: >>> + dev_dbg(data->dev, "Probed: err=%d\n", err); >>> + return err; >>> +} >>> + >>> +static int spm_remove(struct device *dev) >>> +{ >>> + struct spm_data *data = dev_get_drvdata(dev); >>> + >>> + if (data->rfkill) { >>> + rfkill_unregister(data->rfkill); >>> + rfkill_destroy(data->rfkill); >>> + } >>> + return 0; >>> +} >>> + >>> +static struct device_driver spm_driver = { >>> + .name = "serial-power-manager", >>> + .owner = THIS_MODULE, >>> + .of_match_table = spm_dt_ids, >>> + .probe = spm_probe, >>> + .remove = spm_remove, >>> +}; >>> + >>> +static int __init spm_init(void) >>> +{ >>> + return tty_slave_driver_register(&spm_driver); >>> +} >>> +module_init(spm_init); >>> + >>> +static void __exit spm_exit(void) >>> +{ >>> + driver_unregister(&spm_driver); >>> +} >>> +module_exit(spm_exit); >>> + >>> +MODULE_AUTHOR("NeilBrown <neil@xxxxxxxxxx>"); >>> +MODULE_DEVICE_TABLE(of, spm_dt_ids); >>> +MODULE_DESCRIPTION("Power management for Serial-attached device."); >>> +MODULE_LICENSE("GPL v2"); >>> >>> >>> _______________________________________________ >>> Gta04-owner mailing list >>> Gta04-owner@xxxxxxxxxxxxx >>> http://lists.goldelico.com/mailman/listinfo.cgi/gta04-owner >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ > -- 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