| Message ID | 20141211215944.4127.4186.stgit@notabene.brown |
|---|---|
| State | Needs Review / ACK, archived |
| Headers | show |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 1 errors, 0 warnings, 0 lines checked |
| robh/patch-applied | success |
Hi, On Fri, Dec 12, 2014 at 08:59:44AM +1100, NeilBrown wrote: > The regulator is identified in devicetree as 'vdd-supply' long description is kind of useless... > Signed-off-by: NeilBrown <neilb@suse.de> > --- > .../devicetree/bindings/serial/slave-reg.txt | 18 ++++ > drivers/tty/Kconfig | 2 > drivers/tty/Makefile | 1 > drivers/tty/slaves/Kconfig | 12 ++ > drivers/tty/slaves/Makefile | 2 > drivers/tty/slaves/tty-reg.c | 102 ++++++++++++++++++++ > 6 files changed, 137 insertions(+) > create mode 100644 Documentation/devicetree/bindings/serial/slave-reg.txt > create mode 100644 drivers/tty/slaves/Kconfig > create mode 100644 drivers/tty/slaves/Makefile > create mode 100644 drivers/tty/slaves/tty-reg.c > > diff --git a/Documentation/devicetree/bindings/serial/slave-reg.txt b/Documentation/devicetree/bindings/serial/slave-reg.txt > new file mode 100644 > index 000000000000..7896bce8dfe4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/serial/slave-reg.txt > @@ -0,0 +1,18 @@ > +Regulator powered UART-attached device > + > +Required properties: > +- compatible: "tty,regulator" > +- vdd-supply: regulator to power the device > + > + > +This is listed as a child node of a UART. When the > +UART is opened, the device is powered. > + > +Example: > + > +&uart1 { > + bluetooth { > + compatible = "tty,regulator"; > + vdd-supply = <&vaux4>; > + }; > +}; NACK. The compatible value should describe the connected device. You did not connect a regulator, but a bluetooth chip! DT should look like this: &uart1 { bluetooth { compatible = "vendor,bluetooth-chip"; vdd-supply = <&vaux4>; }; }; I think it would be ok to use your generic driver to handle the specific compatible value, though. Having the proper compatible value means, that there can be a more specific driver later, that other operating systems can expose the device as some kind of /dev/bluetooth instead of /dev/ttyXY, that userspace is able to know there is a bluetooth device connected to /dev/ttyXY by parsing the DT and results in easier-to-understand DTS. > diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig > index b24aa010f68c..ea3383c71701 100644 > --- a/drivers/tty/Kconfig > +++ b/drivers/tty/Kconfig > @@ -419,4 +419,6 @@ config DA_CONSOLE > help > This enables a console on a Dash channel. > > +source drivers/tty/slaves/Kconfig > + > endif # TTY > diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile > index 58ad1c05b7f8..45957d671e33 100644 > --- a/drivers/tty/Makefile > +++ b/drivers/tty/Makefile > @@ -13,6 +13,7 @@ obj-$(CONFIG_R3964) += n_r3964.o > obj-y += vt/ > obj-$(CONFIG_HVC_DRIVER) += hvc/ > obj-y += serial/ > +obj-$(CONFIG_TTY_SLAVE) += slaves/ > > # tty drivers > obj-$(CONFIG_AMIGA_BUILTIN_SERIAL) += amiserial.o > diff --git a/drivers/tty/slaves/Kconfig b/drivers/tty/slaves/Kconfig > new file mode 100644 > index 000000000000..2dd1acf80f8c > --- /dev/null > +++ b/drivers/tty/slaves/Kconfig > @@ -0,0 +1,12 @@ > +menuconfig TTY_SLAVE > + bool "TTY slave devices" > + help > + Devices which attach via a uart, but need extra > + driver support for power management etc. > + > +if TTY_SLAVE > + > +config TTY_SLAVE_REGULATOR > + tristate "TTY slave device powered by regulator" > + > +endif > diff --git a/drivers/tty/slaves/Makefile b/drivers/tty/slaves/Makefile > new file mode 100644 > index 000000000000..e636bf49f1cc > --- /dev/null > +++ b/drivers/tty/slaves/Makefile > @@ -0,0 +1,2 @@ > + > +obj-$(CONFIG_TTY_SLAVE_REGULATOR) += tty-reg.o > diff --git a/drivers/tty/slaves/tty-reg.c b/drivers/tty/slaves/tty-reg.c > new file mode 100644 > index 000000000000..613f8b10967c > --- /dev/null > +++ b/drivers/tty/slaves/tty-reg.c > @@ -0,0 +1,102 @@ > +/* > + * tty-reg: > + * Support for any device which needs a regulator turned on > + * when a tty is opened. > + */ > + > +#include <linux/module.h> > +#include <linux/slab.h> > +#include <linux/err.h> > +#include <linux/regulator/consumer.h> > +#include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > +#include <linux/tty.h> > + > +struct tty_reg_data { > + struct regulator *reg; > + bool reg_enabled; > +}; > + > +static int tty_reg_runtime_resume(struct device *slave) > +{ > + struct tty_reg_data *data = dev_get_drvdata(slave); > + if (!data->reg_enabled && > + regulator_enable(data->reg) == 0) { > + dev_dbg(slave, "power on\n"); > + data->reg_enabled = true; > + } > + return 0; > +} > + > +static int tty_reg_runtime_suspend(struct device *slave) > +{ > + struct tty_reg_data *data = dev_get_drvdata(slave); > + > + if (data->reg_enabled && > + regulator_disable(data->reg) == 0) { > + dev_dbg(slave, "power off\n"); > + data->reg_enabled = false; > + } > + return 0; > +} > + > +static int tty_reg_probe(struct platform_device *pdev) > +{ > + struct tty_reg_data *data; > + struct regulator *reg; > + int err; > + > + err = -ENODEV; > + if (pdev->dev.parent == NULL) > + goto out; > + reg = devm_regulator_get(&pdev->dev, "vdd"); > + err = PTR_ERR(reg); > + if (IS_ERR(reg)) > + goto out; > + > + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); > + err = -ENOMEM; > + if (!data) > + goto out; > + data->reg = reg; > + data->reg_enabled = false; > + pm_runtime_enable(&pdev->dev); > + platform_set_drvdata(pdev, data); > + err = 0; > +out: > + return err; > +} > + > +static struct of_device_id tty_reg_dt_ids[] = { > + { .compatible = "tty,regulator", }, > + {} > +}; > + > +static const struct dev_pm_ops tty_reg_pm_ops = { > + SET_RUNTIME_PM_OPS(tty_reg_runtime_suspend, > + tty_reg_runtime_resume, NULL) > +}; > + > +static struct platform_driver tty_reg_driver = { > + .driver.name = "tty-regulator", > + .driver.owner = THIS_MODULE, > + .driver.of_match_table = tty_reg_dt_ids, > + .driver.pm = &tty_reg_pm_ops, > + .probe = tty_reg_probe, > +}; > + > +static int __init tty_reg_init(void) > +{ > + return platform_driver_register(&tty_reg_driver); > +} > +module_init(tty_reg_init); > + > +static void __exit tty_reg_exit(void) > +{ > + platform_driver_unregister(&tty_reg_driver); > +} > +module_exit(tty_reg_exit); > + > +MODULE_AUTHOR("NeilBrown <neilb@suse.de>"); > +MODULE_DESCRIPTION("Serial port device which requires a regulator when open."); > +MODULE_LICENSE("GPL v2"); > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
On 12/11/2014 04:59 PM, NeilBrown wrote: > The regulator is identified in devicetree as 'vdd-supply' > > Signed-off-by: NeilBrown <neilb@suse.de> tty_* is only for tty core functions/data. Regards, Peter Hurley > --- > .../devicetree/bindings/serial/slave-reg.txt | 18 ++++ > drivers/tty/Kconfig | 2 > drivers/tty/Makefile | 1 > drivers/tty/slaves/Kconfig | 12 ++ > drivers/tty/slaves/Makefile | 2 > drivers/tty/slaves/tty-reg.c | 102 ++++++++++++++++++++ > 6 files changed, 137 insertions(+) > create mode 100644 Documentation/devicetree/bindings/serial/slave-reg.txt > create mode 100644 drivers/tty/slaves/Kconfig > create mode 100644 drivers/tty/slaves/Makefile > create mode 100644 drivers/tty/slaves/tty-reg.c > > diff --git a/Documentation/devicetree/bindings/serial/slave-reg.txt b/Documentation/devicetree/bindings/serial/slave-reg.txt > new file mode 100644 > index 000000000000..7896bce8dfe4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/serial/slave-reg.txt > @@ -0,0 +1,18 @@ > +Regulator powered UART-attached device > + > +Required properties: > +- compatible: "tty,regulator" > +- vdd-supply: regulator to power the device > + > + > +This is listed as a child node of a UART. When the > +UART is opened, the device is powered. > + > +Example: > + > +&uart1 { > + bluetooth { > + compatible = "tty,regulator"; > + vdd-supply = <&vaux4>; > + }; > +}; > diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig > index b24aa010f68c..ea3383c71701 100644 > --- a/drivers/tty/Kconfig > +++ b/drivers/tty/Kconfig > @@ -419,4 +419,6 @@ config DA_CONSOLE > help > This enables a console on a Dash channel. > > +source drivers/tty/slaves/Kconfig > + > endif # TTY > diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile > index 58ad1c05b7f8..45957d671e33 100644 > --- a/drivers/tty/Makefile > +++ b/drivers/tty/Makefile > @@ -13,6 +13,7 @@ obj-$(CONFIG_R3964) += n_r3964.o > obj-y += vt/ > obj-$(CONFIG_HVC_DRIVER) += hvc/ > obj-y += serial/ > +obj-$(CONFIG_TTY_SLAVE) += slaves/ > > # tty drivers > obj-$(CONFIG_AMIGA_BUILTIN_SERIAL) += amiserial.o > diff --git a/drivers/tty/slaves/Kconfig b/drivers/tty/slaves/Kconfig > new file mode 100644 > index 000000000000..2dd1acf80f8c > --- /dev/null > +++ b/drivers/tty/slaves/Kconfig > @@ -0,0 +1,12 @@ > +menuconfig TTY_SLAVE > + bool "TTY slave devices" > + help > + Devices which attach via a uart, but need extra > + driver support for power management etc. > + > +if TTY_SLAVE > + > +config TTY_SLAVE_REGULATOR > + tristate "TTY slave device powered by regulator" > + > +endif > diff --git a/drivers/tty/slaves/Makefile b/drivers/tty/slaves/Makefile > new file mode 100644 > index 000000000000..e636bf49f1cc > --- /dev/null > +++ b/drivers/tty/slaves/Makefile > @@ -0,0 +1,2 @@ > + > +obj-$(CONFIG_TTY_SLAVE_REGULATOR) += tty-reg.o > diff --git a/drivers/tty/slaves/tty-reg.c b/drivers/tty/slaves/tty-reg.c > new file mode 100644 > index 000000000000..613f8b10967c > --- /dev/null > +++ b/drivers/tty/slaves/tty-reg.c > @@ -0,0 +1,102 @@ > +/* > + * tty-reg: > + * Support for any device which needs a regulator turned on > + * when a tty is opened. > + */ > + > +#include <linux/module.h> > +#include <linux/slab.h> > +#include <linux/err.h> > +#include <linux/regulator/consumer.h> > +#include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > +#include <linux/tty.h> > + > +struct tty_reg_data { > + struct regulator *reg; > + bool reg_enabled; > +}; > + > +static int tty_reg_runtime_resume(struct device *slave) > +{ > + struct tty_reg_data *data = dev_get_drvdata(slave); > + if (!data->reg_enabled && > + regulator_enable(data->reg) == 0) { > + dev_dbg(slave, "power on\n"); > + data->reg_enabled = true; > + } > + return 0; > +} > + > +static int tty_reg_runtime_suspend(struct device *slave) > +{ > + struct tty_reg_data *data = dev_get_drvdata(slave); > + > + if (data->reg_enabled && > + regulator_disable(data->reg) == 0) { > + dev_dbg(slave, "power off\n"); > + data->reg_enabled = false; > + } > + return 0; > +} > + > +static int tty_reg_probe(struct platform_device *pdev) > +{ > + struct tty_reg_data *data; > + struct regulator *reg; > + int err; > + > + err = -ENODEV; > + if (pdev->dev.parent == NULL) > + goto out; > + reg = devm_regulator_get(&pdev->dev, "vdd"); > + err = PTR_ERR(reg); > + if (IS_ERR(reg)) > + goto out; > + > + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); > + err = -ENOMEM; > + if (!data) > + goto out; > + data->reg = reg; > + data->reg_enabled = false; > + pm_runtime_enable(&pdev->dev); > + platform_set_drvdata(pdev, data); > + err = 0; > +out: > + return err; > +} > + > +static struct of_device_id tty_reg_dt_ids[] = { > + { .compatible = "tty,regulator", }, > + {} > +}; > + > +static const struct dev_pm_ops tty_reg_pm_ops = { > + SET_RUNTIME_PM_OPS(tty_reg_runtime_suspend, > + tty_reg_runtime_resume, NULL) > +}; > + > +static struct platform_driver tty_reg_driver = { > + .driver.name = "tty-regulator", > + .driver.owner = THIS_MODULE, > + .driver.of_match_table = tty_reg_dt_ids, > + .driver.pm = &tty_reg_pm_ops, > + .probe = tty_reg_probe, > +}; > + > +static int __init tty_reg_init(void) > +{ > + return platform_driver_register(&tty_reg_driver); > +} > +module_init(tty_reg_init); > + > +static void __exit tty_reg_exit(void) > +{ > + platform_driver_unregister(&tty_reg_driver); > +} > +module_exit(tty_reg_exit); > + > +MODULE_AUTHOR("NeilBrown <neilb@suse.de>"); > +MODULE_DESCRIPTION("Serial port device which requires a regulator when open."); > +MODULE_LICENSE("GPL v2"); -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sebastian, >> diff --git a/Documentation/devicetree/bindings/serial/slave-reg.txt b/Documentation/devicetree/bindings/serial/slave-reg.txt >> new file mode 100644 >> index 000000000000..7896bce8dfe4 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/serial/slave-reg.txt >> @@ -0,0 +1,18 @@ >> +Regulator powered UART-attached device >> + >> +Required properties: >> +- compatible: "tty,regulator" >> +- vdd-supply: regulator to power the device >> + >> + >> +This is listed as a child node of a UART. When the >> +UART is opened, the device is powered. >> + >> +Example: >> + >> +&uart1 { >> + bluetooth { >> + compatible = "tty,regulator"; >> + vdd-supply = <&vaux4>; >> + }; >> +}; > > NACK. The compatible value should describe the connected device. You > did not connect a regulator, but a bluetooth chip! DT should look > like this: > > &uart1 { > bluetooth { > compatible = "vendor,bluetooth-chip"; > vdd-supply = <&vaux4>; > }; > }; > > I think it would be ok to use your generic driver to handle the > specific compatible value, though. > > Having the proper compatible value means, that there can be a more > specific driver later, that other operating systems can expose the > device as some kind of /dev/bluetooth instead of /dev/ttyXY, that > userspace is able to know there is a bluetooth device connected to > /dev/ttyXY by parsing the DT and results in easier-to-understand > DTS. I think that is up to udev to be able to make this a nice device node. However the device node name is pretty much irrelevant. What you want is enough meta data to tell that there is Bluetooth chip behind this TTY. From a Bluetooth stack perspective only hciattach needs to be run to get the N_HCI line discipline up and running for that chip. However what would be interesting for hciattach would be a possibility to expose the Bluetooth manufacturer. Since the init routine is different for each manufacturer. So if that could be encoded in the DT, then that would be certainly helpful. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Marcel, On Fri, Dec 12, 2014 at 01:46:33AM +0100, Marcel Holtmann wrote: > > NACK. The compatible value should describe the connected device. You > > did not connect a regulator, but a bluetooth chip! DT should look > > like this: > > > > &uart1 { > > bluetooth { > > compatible = "vendor,bluetooth-chip"; > > vdd-supply = <&vaux4>; > > }; > > }; > > > > I think it would be ok to use your generic driver to handle the > > specific compatible value, though. > > > > Having the proper compatible value means, that there can be a more > > specific driver later, that other operating systems can expose the > > device as some kind of /dev/bluetooth instead of /dev/ttyXY, that > > userspace is able to know there is a bluetooth device connected to > > /dev/ttyXY by parsing the DT and results in easier-to-understand > > DTS. > > I think that is up to udev to be able to make this a nice device > node. However the device node name is pretty much irrelevant. Ah that was not about Linux, but about $random-os using DT. Just ignore the device name its not important for the argument. > What you want is enough meta data to tell that there is Bluetooth > chip behind this TTY. Yes, that is what I tried to say :) > From a Bluetooth stack perspective only hciattach needs to be run > to get the N_HCI line discipline up and running for that chip. Yes, but with the metadata from Neil's proposed binding you do not know, that there is a bluetooth chip connected, so you need to do this manually. Not very nice, when it could be done automatically. > However what would be interesting for hciattach would be a > possibility to expose the Bluetooth manufacturer. Since the init > routine is different for each manufacturer. Yes and it may be, that a manufacturer breaks its init routine at some point. The problem is not much different from other devices and normally solved through the compatible string. > So if that could be encoded in the DT, then that would be > certainly helpful. All needed data should be encoded when specified as in the following example: &uart1 { bluetooth { compatible = "wi2wi,w2cbw003"; vdd-supply = <&vaux4>; }; }; Note: wi2wi is not yet in Documentation/devicetree/bindings/vendor-prefixes.txt -- Sebastian
On Thu, 11 Dec 2014 23:58:48 +0100 Sebastian Reichel <sre@kernel.org> wrote: > Hi, > > On Fri, Dec 12, 2014 at 08:59:44AM +1100, NeilBrown wrote: > > The regulator is identified in devicetree as 'vdd-supply' > > long description is kind of useless... > > > Signed-off-by: NeilBrown <neilb@suse.de> > > --- > > .../devicetree/bindings/serial/slave-reg.txt | 18 ++++ > > drivers/tty/Kconfig | 2 > > drivers/tty/Makefile | 1 > > drivers/tty/slaves/Kconfig | 12 ++ > > drivers/tty/slaves/Makefile | 2 > > drivers/tty/slaves/tty-reg.c | 102 ++++++++++++++++++++ > > 6 files changed, 137 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/serial/slave-reg.txt > > create mode 100644 drivers/tty/slaves/Kconfig > > create mode 100644 drivers/tty/slaves/Makefile > > create mode 100644 drivers/tty/slaves/tty-reg.c > > > > diff --git a/Documentation/devicetree/bindings/serial/slave-reg.txt b/Documentation/devicetree/bindings/serial/slave-reg.txt > > new file mode 100644 > > index 000000000000..7896bce8dfe4 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/serial/slave-reg.txt > > @@ -0,0 +1,18 @@ > > +Regulator powered UART-attached device > > + > > +Required properties: > > +- compatible: "tty,regulator" > > +- vdd-supply: regulator to power the device > > + > > + > > +This is listed as a child node of a UART. When the > > +UART is opened, the device is powered. > > + > > +Example: > > + > > +&uart1 { > > + bluetooth { > > + compatible = "tty,regulator"; > > + vdd-supply = <&vaux4>; > > + }; > > +}; > > NACK. The compatible value should describe the connected device. You > did not connect a regulator, but a bluetooth chip! DT should look > like this: > > &uart1 { > bluetooth { > compatible = "vendor,bluetooth-chip"; > vdd-supply = <&vaux4>; > }; > }; > > I think it would be ok to use your generic driver to handle the > specific compatible value, though. > > Having the proper compatible value means, that there can be a more > specific driver later, that other operating systems can expose the > device as some kind of /dev/bluetooth instead of /dev/ttyXY, that > userspace is able to know there is a bluetooth device connected to > /dev/ttyXY by parsing the DT and results in easier-to-understand > DTS. So the tty_reg_dt_ids array could conceivably grow a long list of compatible devices, starting with this one. I guess that makes sense. Thanks, NeilBrown
On Thu, 11 Dec 2014 18:32:52 -0500 Peter Hurley <peter@hurleysoftware.com> wrote: > On 12/11/2014 04:59 PM, NeilBrown wrote: > > The regulator is identified in devicetree as 'vdd-supply' > > > > Signed-off-by: NeilBrown <neilb@suse.de> > > tty_* is only for tty core functions/data. I think you are just objecting to the function and type names - is that correct? I suspect I can come up with something different. Thanks, NeilBrown
On 12/12/2014 12:27 AM, NeilBrown wrote: > On Thu, 11 Dec 2014 18:32:52 -0500 Peter Hurley <peter@hurleysoftware.com> > wrote: > >> On 12/11/2014 04:59 PM, NeilBrown wrote: >>> The regulator is identified in devicetree as 'vdd-supply' >>> >>> Signed-off-by: NeilBrown <neilb@suse.de> >> >> tty_* is only for tty core functions/data. > > I think you are just objecting to the function and type names - is that > correct? Yeah, exactly. > I suspect I can come up with something different. Thanks. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 12 Dec 2014 08:59:44 +1100 , NeilBrown <neilb@suse.de> wrote: > The regulator is identified in devicetree as 'vdd-supply' > > Signed-off-by: NeilBrown <neilb@suse.de> > --- > .../devicetree/bindings/serial/slave-reg.txt | 18 ++++ > drivers/tty/Kconfig | 2 > drivers/tty/Makefile | 1 > drivers/tty/slaves/Kconfig | 12 ++ > drivers/tty/slaves/Makefile | 2 > drivers/tty/slaves/tty-reg.c | 102 ++++++++++++++++++++ > 6 files changed, 137 insertions(+) > create mode 100644 Documentation/devicetree/bindings/serial/slave-reg.txt > create mode 100644 drivers/tty/slaves/Kconfig > create mode 100644 drivers/tty/slaves/Makefile > create mode 100644 drivers/tty/slaves/tty-reg.c > > diff --git a/Documentation/devicetree/bindings/serial/slave-reg.txt b/Documentation/devicetree/bindings/serial/slave-reg.txt > new file mode 100644 > index 000000000000..7896bce8dfe4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/serial/slave-reg.txt > @@ -0,0 +1,18 @@ > +Regulator powered UART-attached device > + > +Required properties: > +- compatible: "tty,regulator" > +- vdd-supply: regulator to power the device The binding should be specfic to the device. Trying to do a generic binding like this doesn't scale well. Do a specific binding for the bluetooth chipset and make that driver understand how to do PM operations on that device. That way each kind of tty slave device driver can have its own set of PM mechanism which may be completely different. > +This is listed as a child node of a UART. When the > +UART is opened, the device is powered. Another thought, what if I *don't* want the device to be powered merely because the uart is opened? > + > +Example: > + > +&uart1 { > + bluetooth { > + compatible = "tty,regulator"; > + vdd-supply = <&vaux4>; > + }; > +}; > diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig > index b24aa010f68c..ea3383c71701 100644 > --- a/drivers/tty/Kconfig > +++ b/drivers/tty/Kconfig > @@ -419,4 +419,6 @@ config DA_CONSOLE > help > This enables a console on a Dash channel. > > +source drivers/tty/slaves/Kconfig > + > endif # TTY > diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile > index 58ad1c05b7f8..45957d671e33 100644 > --- a/drivers/tty/Makefile > +++ b/drivers/tty/Makefile > @@ -13,6 +13,7 @@ obj-$(CONFIG_R3964) += n_r3964.o > obj-y += vt/ > obj-$(CONFIG_HVC_DRIVER) += hvc/ > obj-y += serial/ > +obj-$(CONFIG_TTY_SLAVE) += slaves/ > > # tty drivers > obj-$(CONFIG_AMIGA_BUILTIN_SERIAL) += amiserial.o > diff --git a/drivers/tty/slaves/Kconfig b/drivers/tty/slaves/Kconfig > new file mode 100644 > index 000000000000..2dd1acf80f8c > --- /dev/null > +++ b/drivers/tty/slaves/Kconfig > @@ -0,0 +1,12 @@ > +menuconfig TTY_SLAVE > + bool "TTY slave devices" > + help > + Devices which attach via a uart, but need extra > + driver support for power management etc. > + > +if TTY_SLAVE > + > +config TTY_SLAVE_REGULATOR > + tristate "TTY slave device powered by regulator" > + > +endif > diff --git a/drivers/tty/slaves/Makefile b/drivers/tty/slaves/Makefile > new file mode 100644 > index 000000000000..e636bf49f1cc > --- /dev/null > +++ b/drivers/tty/slaves/Makefile > @@ -0,0 +1,2 @@ > + > +obj-$(CONFIG_TTY_SLAVE_REGULATOR) += tty-reg.o > diff --git a/drivers/tty/slaves/tty-reg.c b/drivers/tty/slaves/tty-reg.c > new file mode 100644 > index 000000000000..613f8b10967c > --- /dev/null > +++ b/drivers/tty/slaves/tty-reg.c > @@ -0,0 +1,102 @@ > +/* > + * tty-reg: > + * Support for any device which needs a regulator turned on > + * when a tty is opened. > + */ > + > +#include <linux/module.h> > +#include <linux/slab.h> > +#include <linux/err.h> > +#include <linux/regulator/consumer.h> > +#include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > +#include <linux/tty.h> > + > +struct tty_reg_data { > + struct regulator *reg; > + bool reg_enabled; > +}; > + > +static int tty_reg_runtime_resume(struct device *slave) > +{ > + struct tty_reg_data *data = dev_get_drvdata(slave); > + if (!data->reg_enabled && > + regulator_enable(data->reg) == 0) { > + dev_dbg(slave, "power on\n"); > + data->reg_enabled = true; > + } > + return 0; > +} > + > +static int tty_reg_runtime_suspend(struct device *slave) > +{ > + struct tty_reg_data *data = dev_get_drvdata(slave); > + > + if (data->reg_enabled && > + regulator_disable(data->reg) == 0) { > + dev_dbg(slave, "power off\n"); > + data->reg_enabled = false; > + } > + return 0; > +} > + > +static int tty_reg_probe(struct platform_device *pdev) > +{ > + struct tty_reg_data *data; > + struct regulator *reg; > + int err; > + > + err = -ENODEV; > + if (pdev->dev.parent == NULL) > + goto out; > + reg = devm_regulator_get(&pdev->dev, "vdd"); > + err = PTR_ERR(reg); > + if (IS_ERR(reg)) > + goto out; > + > + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); > + err = -ENOMEM; > + if (!data) > + goto out; > + data->reg = reg; > + data->reg_enabled = false; > + pm_runtime_enable(&pdev->dev); > + platform_set_drvdata(pdev, data); > + err = 0; > +out: > + return err; > +} > + > +static struct of_device_id tty_reg_dt_ids[] = { > + { .compatible = "tty,regulator", }, > + {} > +}; > + > +static const struct dev_pm_ops tty_reg_pm_ops = { > + SET_RUNTIME_PM_OPS(tty_reg_runtime_suspend, > + tty_reg_runtime_resume, NULL) > +}; > + > +static struct platform_driver tty_reg_driver = { > + .driver.name = "tty-regulator", > + .driver.owner = THIS_MODULE, > + .driver.of_match_table = tty_reg_dt_ids, > + .driver.pm = &tty_reg_pm_ops, > + .probe = tty_reg_probe, > +}; Instead of creating a full driver that binds against the device, would it not be better to provide a library of stock PM operations that other device drivers (like the bluetooth driver) can use as-is instead of defining its own? > + > +static int __init tty_reg_init(void) > +{ > + return platform_driver_register(&tty_reg_driver); > +} > +module_init(tty_reg_init); > + > +static void __exit tty_reg_exit(void) > +{ > + platform_driver_unregister(&tty_reg_driver); > +} > +module_exit(tty_reg_exit); > + > +MODULE_AUTHOR("NeilBrown <neilb@suse.de>"); > +MODULE_DESCRIPTION("Serial port device which requires a regulator when open."); > +MODULE_LICENSE("GPL v2"); > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/serial/slave-reg.txt b/Documentation/devicetree/bindings/serial/slave-reg.txt new file mode 100644 index 000000000000..7896bce8dfe4 --- /dev/null +++ b/Documentation/devicetree/bindings/serial/slave-reg.txt @@ -0,0 +1,18 @@ +Regulator powered UART-attached device + +Required properties: +- compatible: "tty,regulator" +- vdd-supply: regulator to power the device + + +This is listed as a child node of a UART. When the +UART is opened, the device is powered. + +Example: + +&uart1 { + bluetooth { + compatible = "tty,regulator"; + vdd-supply = <&vaux4>; + }; +}; diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig index b24aa010f68c..ea3383c71701 100644 --- a/drivers/tty/Kconfig +++ b/drivers/tty/Kconfig @@ -419,4 +419,6 @@ config DA_CONSOLE help This enables a console on a Dash channel. +source drivers/tty/slaves/Kconfig + endif # TTY diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile index 58ad1c05b7f8..45957d671e33 100644 --- a/drivers/tty/Makefile +++ b/drivers/tty/Makefile @@ -13,6 +13,7 @@ obj-$(CONFIG_R3964) += n_r3964.o obj-y += vt/ obj-$(CONFIG_HVC_DRIVER) += hvc/ obj-y += serial/ +obj-$(CONFIG_TTY_SLAVE) += slaves/ # tty drivers obj-$(CONFIG_AMIGA_BUILTIN_SERIAL) += amiserial.o diff --git a/drivers/tty/slaves/Kconfig b/drivers/tty/slaves/Kconfig new file mode 100644 index 000000000000..2dd1acf80f8c --- /dev/null +++ b/drivers/tty/slaves/Kconfig @@ -0,0 +1,12 @@ +menuconfig TTY_SLAVE + bool "TTY slave devices" + help + Devices which attach via a uart, but need extra + driver support for power management etc. + +if TTY_SLAVE + +config TTY_SLAVE_REGULATOR + tristate "TTY slave device powered by regulator" + +endif diff --git a/drivers/tty/slaves/Makefile b/drivers/tty/slaves/Makefile new file mode 100644 index 000000000000..e636bf49f1cc --- /dev/null +++ b/drivers/tty/slaves/Makefile @@ -0,0 +1,2 @@ + +obj-$(CONFIG_TTY_SLAVE_REGULATOR) += tty-reg.o diff --git a/drivers/tty/slaves/tty-reg.c b/drivers/tty/slaves/tty-reg.c new file mode 100644 index 000000000000..613f8b10967c --- /dev/null +++ b/drivers/tty/slaves/tty-reg.c @@ -0,0 +1,102 @@ +/* + * tty-reg: + * Support for any device which needs a regulator turned on + * when a tty is opened. + */ + +#include <linux/module.h> +#include <linux/slab.h> +#include <linux/err.h> +#include <linux/regulator/consumer.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> +#include <linux/tty.h> + +struct tty_reg_data { + struct regulator *reg; + bool reg_enabled; +}; + +static int tty_reg_runtime_resume(struct device *slave) +{ + struct tty_reg_data *data = dev_get_drvdata(slave); + if (!data->reg_enabled && + regulator_enable(data->reg) == 0) { + dev_dbg(slave, "power on\n"); + data->reg_enabled = true; + } + return 0; +} + +static int tty_reg_runtime_suspend(struct device *slave) +{ + struct tty_reg_data *data = dev_get_drvdata(slave); + + if (data->reg_enabled && + regulator_disable(data->reg) == 0) { + dev_dbg(slave, "power off\n"); + data->reg_enabled = false; + } + return 0; +} + +static int tty_reg_probe(struct platform_device *pdev) +{ + struct tty_reg_data *data; + struct regulator *reg; + int err; + + err = -ENODEV; + if (pdev->dev.parent == NULL) + goto out; + reg = devm_regulator_get(&pdev->dev, "vdd"); + err = PTR_ERR(reg); + if (IS_ERR(reg)) + goto out; + + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); + err = -ENOMEM; + if (!data) + goto out; + data->reg = reg; + data->reg_enabled = false; + pm_runtime_enable(&pdev->dev); + platform_set_drvdata(pdev, data); + err = 0; +out: + return err; +} + +static struct of_device_id tty_reg_dt_ids[] = { + { .compatible = "tty,regulator", }, + {} +}; + +static const struct dev_pm_ops tty_reg_pm_ops = { + SET_RUNTIME_PM_OPS(tty_reg_runtime_suspend, + tty_reg_runtime_resume, NULL) +}; + +static struct platform_driver tty_reg_driver = { + .driver.name = "tty-regulator", + .driver.owner = THIS_MODULE, + .driver.of_match_table = tty_reg_dt_ids, + .driver.pm = &tty_reg_pm_ops, + .probe = tty_reg_probe, +}; + +static int __init tty_reg_init(void) +{ + return platform_driver_register(&tty_reg_driver); +} +module_init(tty_reg_init); + +static void __exit tty_reg_exit(void) +{ + platform_driver_unregister(&tty_reg_driver); +} +module_exit(tty_reg_exit); + +MODULE_AUTHOR("NeilBrown <neilb@suse.de>"); +MODULE_DESCRIPTION("Serial port device which requires a regulator when open."); +MODULE_LICENSE("GPL v2");
The regulator is identified in devicetree as 'vdd-supply' Signed-off-by: NeilBrown <neilb@suse.de> --- .../devicetree/bindings/serial/slave-reg.txt | 18 ++++ drivers/tty/Kconfig | 2 drivers/tty/Makefile | 1 drivers/tty/slaves/Kconfig | 12 ++ drivers/tty/slaves/Makefile | 2 drivers/tty/slaves/tty-reg.c | 102 ++++++++++++++++++++ 6 files changed, 137 insertions(+) create mode 100644 Documentation/devicetree/bindings/serial/slave-reg.txt create mode 100644 drivers/tty/slaves/Kconfig create mode 100644 drivers/tty/slaves/Makefile create mode 100644 drivers/tty/slaves/tty-reg.c -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html