| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20141211225847.GB11666@earth.universe>
Date: Thu, 11 Dec 2014 23:58:48 +0100
From: Sebastian Reichel <sre@...nel.org>
To: NeilBrown <neilb@...e.de>
Cc: Grant Likely <grant.likely@...aro.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Mark Rutland <mark.rutland@....com>,
Jiri Slaby <jslaby@...e.cz>, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] TTY: add slave driver to power-on device via a
regulator.
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@...e.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@...e.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@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)
Powered by blists - more mailing lists