This driver supports power connectors supplied by a regulator, such as
outlets on a power distribution unit (PDU).
Its extcon functionality is currently quite limited, since the
hardware it's initially targeting is very simple and doesn't really
provide anything for the driver to interact with, but it can be
extended as required for hardware that might offer more for it to do
(e.g. a presence-detection mechanism).
Its sole feature is a read/write sysfs attribute allowing userspace to
switch the output on and off by enabling and disabling the supply
regulator.
Signed-off-by: Zev Weiss <[email protected]>
---
.../ABI/testing/sysfs-driver-extcon-regulator | 8 ++
MAINTAINERS | 8 ++
drivers/extcon/Kconfig | 8 ++
drivers/extcon/Makefile | 1 +
drivers/extcon/extcon-regulator.c | 133 ++++++++++++++++++
5 files changed, 158 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-extcon-regulator
create mode 100644 drivers/extcon/extcon-regulator.c
diff --git a/Documentation/ABI/testing/sysfs-driver-extcon-regulator b/Documentation/ABI/testing/sysfs-driver-extcon-regulator
new file mode 100644
index 000000000000..b2f3141a1c49
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-extcon-regulator
@@ -0,0 +1,8 @@
+What: /sys/bus/platform/drivers/extcon-regulator/*/state
+Date: May 2022
+KernelVersion: 5.18
+Contact: Zev Weiss <[email protected]>
+Description: When read, provides the current power state of the connector,
+ either "on" or "off". Either string may also be written to
+ set the power state of the connector.
+Users: OpenBMC <[email protected]>
diff --git a/MAINTAINERS b/MAINTAINERS
index edc96cdb85e8..c30b6cf95ff1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16740,6 +16740,14 @@ F: Documentation/devicetree/bindings/regmap/
F: drivers/base/regmap/
F: include/linux/regmap.h
+REGULATOR EXTCON DRIVER
+M: Zev Weiss <[email protected]>
+L: [email protected]
+S: Maintained
+F: Documentation/ABI/testing/sysfs-driver-extcon-regulator
+F: Documentation/devicetree/bindings/connector/regulator-connector.yaml
+F: drivers/extcon/extcon-regulator.c
+
REISERFS FILE SYSTEM
L: [email protected]
S: Supported
diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index 0d42e49105dd..19fe76da6c75 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -143,6 +143,14 @@ config EXTCON_QCOM_SPMI_MISC
Say Y here to enable SPMI PMIC based USB cable detection
support on Qualcomm PMICs such as PM8941.
+config EXTCON_REGULATOR
+ tristate "Regulator output extcon support"
+ depends on REGULATOR
+ help
+ Say y here to enable support for regulator-supplied external
+ power output connections, such as the outlets of a power
+ distribution unit (PDU).
+
config EXTCON_RT8973A
tristate "Richtek RT8973A EXTCON support"
depends on I2C
diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
index 1b390d934ca9..1a1c32d4b23e 100644
--- a/drivers/extcon/Makefile
+++ b/drivers/extcon/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_EXTCON_MAX8997) += extcon-max8997.o
obj-$(CONFIG_EXTCON_PALMAS) += extcon-palmas.o
obj-$(CONFIG_EXTCON_PTN5150) += extcon-ptn5150.o
obj-$(CONFIG_EXTCON_QCOM_SPMI_MISC) += extcon-qcom-spmi-misc.o
+obj-$(CONFIG_EXTCON_REGULATOR) += extcon-regulator.o
obj-$(CONFIG_EXTCON_RT8973A) += extcon-rt8973a.o
obj-$(CONFIG_EXTCON_SM5502) += extcon-sm5502.o
obj-$(CONFIG_EXTCON_USB_GPIO) += extcon-usb-gpio.o
diff --git a/drivers/extcon/extcon-regulator.c b/drivers/extcon/extcon-regulator.c
new file mode 100644
index 000000000000..eec1bb3f4c09
--- /dev/null
+++ b/drivers/extcon/extcon-regulator.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * extcon-regulator: extcon driver for regulator-supplied external power
+ * output connectors
+ *
+ * Copyright (C) 2022 Zev Weiss <[email protected]>
+ */
+
+#include <linux/extcon-provider.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+
+struct regulator_extcon_data {
+ struct extcon_dev *edev;
+ struct regulator *reg;
+ struct mutex lock;
+ unsigned int extcon_id;
+};
+
+static ssize_t state_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct regulator_extcon_data *data = dev_get_drvdata(dev);
+ int status = regulator_is_enabled(data->reg);
+
+ if (status < 0)
+ return status;
+
+ return sysfs_emit(buf, "%s\n", status ? "on" : "off");
+}
+
+static ssize_t state_store(struct device *dev, struct device_attribute *attr, const char *buf,
+ size_t count)
+{
+ int status, wantstate;
+ struct regulator_extcon_data *data = dev_get_drvdata(dev);
+ struct regulator *reg = data->reg;
+
+ if (sysfs_streq(buf, "on"))
+ wantstate = 1;
+ else if (sysfs_streq(buf, "off"))
+ wantstate = 0;
+ else
+ return -EINVAL;
+
+ mutex_lock(&data->lock);
+
+ status = regulator_is_enabled(reg);
+
+ /*
+ * We need to ensure our enable/disable calls don't get imbalanced, so
+ * bail if we can't determine the current state.
+ */
+ if (status < 0)
+ goto out;
+
+ /* Nothing further needed if we're already in the desired state */
+ if (!!status == wantstate) {
+ status = 0;
+ goto out;
+ }
+
+ if (wantstate)
+ status = regulator_enable(reg);
+ else
+ status = regulator_disable(reg);
+
+out:
+ mutex_unlock(&data->lock);
+
+ return status ? : count;
+}
+
+static DEVICE_ATTR_RW(state);
+
+static struct attribute *regulator_extcon_attrs[] = {
+ &dev_attr_state.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(regulator_extcon);
+
+static int regulator_extcon_probe(struct platform_device *pdev)
+{
+ int ret;
+ struct regulator_extcon_data *data;
+ struct device *dev = &pdev->dev;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->reg = devm_regulator_get_exclusive(&pdev->dev, "vout");
+ if (IS_ERR(data->reg))
+ return PTR_ERR(data->reg);
+
+ mutex_init(&data->lock);
+
+ /* No cables currently supported */
+ data->extcon_id = EXTCON_NONE;
+
+ data->edev = devm_extcon_dev_allocate(dev, &data->extcon_id);
+ if (IS_ERR(data->edev))
+ return PTR_ERR(data->edev);
+
+ ret = devm_extcon_dev_register(dev, data->edev);
+ if (ret < 0)
+ return ret;
+
+ platform_set_drvdata(pdev, data);
+
+ return 0;
+}
+
+static const struct of_device_id regulator_extcon_of_match_table[] = {
+ { .compatible = "regulator-connector" },
+ { },
+};
+
+static struct platform_driver regulator_extcon_driver = {
+ .driver = {
+ .name = "extcon-regulator",
+ .of_match_table = of_match_ptr(regulator_extcon_of_match_table),
+ .dev_groups = regulator_extcon_groups,
+ },
+ .probe = regulator_extcon_probe,
+};
+module_platform_driver(regulator_extcon_driver);
+
+MODULE_AUTHOR("Zev Weiss <[email protected]>");
+MODULE_DESCRIPTION("Regulator extcon driver");
+MODULE_LICENSE("GPL");
--
2.36.0
Hi Zev,
I checked this patch. But, it doesn't look like the extcon provider
driver. Because basically, extcon provider driver need the circuit
in order to detect the kind of external connector. But, there are
no any code for detection. Just add the specific sysfs attribute
for only this driver. It is not standard interface.
Thanks,
Chanwoo Choi
On 22. 5. 6. 08:25, Zev Weiss wrote:
> This driver supports power connectors supplied by a regulator, such as
> outlets on a power distribution unit (PDU).
>
> Its extcon functionality is currently quite limited, since the
> hardware it's initially targeting is very simple and doesn't really
> provide anything for the driver to interact with, but it can be
> extended as required for hardware that might offer more for it to do
> (e.g. a presence-detection mechanism).
>
> Its sole feature is a read/write sysfs attribute allowing userspace to
> switch the output on and off by enabling and disabling the supply
> regulator.
>
> Signed-off-by: Zev Weiss <[email protected]>
> ---
> .../ABI/testing/sysfs-driver-extcon-regulator | 8 ++
> MAINTAINERS | 8 ++
> drivers/extcon/Kconfig | 8 ++
> drivers/extcon/Makefile | 1 +
> drivers/extcon/extcon-regulator.c | 133 ++++++++++++++++++
> 5 files changed, 158 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-driver-extcon-regulator
> create mode 100644 drivers/extcon/extcon-regulator.c
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-extcon-regulator b/Documentation/ABI/testing/sysfs-driver-extcon-regulator
> new file mode 100644
> index 000000000000..b2f3141a1c49
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-extcon-regulator
> @@ -0,0 +1,8 @@
> +What: /sys/bus/platform/drivers/extcon-regulator/*/state
> +Date: May 2022
> +KernelVersion: 5.18
> +Contact: Zev Weiss <[email protected]>
> +Description: When read, provides the current power state of the connector,
> + either "on" or "off". Either string may also be written to
> + set the power state of the connector.
> +Users: OpenBMC <[email protected]>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index edc96cdb85e8..c30b6cf95ff1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16740,6 +16740,14 @@ F: Documentation/devicetree/bindings/regmap/
> F: drivers/base/regmap/
> F: include/linux/regmap.h
>
> +REGULATOR EXTCON DRIVER
> +M: Zev Weiss <[email protected]>
> +L: [email protected]
> +S: Maintained
> +F: Documentation/ABI/testing/sysfs-driver-extcon-regulator
> +F: Documentation/devicetree/bindings/connector/regulator-connector.yaml
> +F: drivers/extcon/extcon-regulator.c
> +
> REISERFS FILE SYSTEM
> L: [email protected]
> S: Supported
> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
> index 0d42e49105dd..19fe76da6c75 100644
> --- a/drivers/extcon/Kconfig
> +++ b/drivers/extcon/Kconfig
> @@ -143,6 +143,14 @@ config EXTCON_QCOM_SPMI_MISC
> Say Y here to enable SPMI PMIC based USB cable detection
> support on Qualcomm PMICs such as PM8941.
>
> +config EXTCON_REGULATOR
> + tristate "Regulator output extcon support"
> + depends on REGULATOR
> + help
> + Say y here to enable support for regulator-supplied external
> + power output connections, such as the outlets of a power
> + distribution unit (PDU).
> +
> config EXTCON_RT8973A
> tristate "Richtek RT8973A EXTCON support"
> depends on I2C
> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
> index 1b390d934ca9..1a1c32d4b23e 100644
> --- a/drivers/extcon/Makefile
> +++ b/drivers/extcon/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_EXTCON_MAX8997) += extcon-max8997.o
> obj-$(CONFIG_EXTCON_PALMAS) += extcon-palmas.o
> obj-$(CONFIG_EXTCON_PTN5150) += extcon-ptn5150.o
> obj-$(CONFIG_EXTCON_QCOM_SPMI_MISC) += extcon-qcom-spmi-misc.o
> +obj-$(CONFIG_EXTCON_REGULATOR) += extcon-regulator.o
> obj-$(CONFIG_EXTCON_RT8973A) += extcon-rt8973a.o
> obj-$(CONFIG_EXTCON_SM5502) += extcon-sm5502.o
> obj-$(CONFIG_EXTCON_USB_GPIO) += extcon-usb-gpio.o
> diff --git a/drivers/extcon/extcon-regulator.c b/drivers/extcon/extcon-regulator.c
> new file mode 100644
> index 000000000000..eec1bb3f4c09
> --- /dev/null
> +++ b/drivers/extcon/extcon-regulator.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * extcon-regulator: extcon driver for regulator-supplied external power
> + * output connectors
> + *
> + * Copyright (C) 2022 Zev Weiss <[email protected]>
> + */
> +
> +#include <linux/extcon-provider.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +
> +struct regulator_extcon_data {
> + struct extcon_dev *edev;
> + struct regulator *reg;
> + struct mutex lock;
> + unsigned int extcon_id;
> +};
> +
> +static ssize_t state_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct regulator_extcon_data *data = dev_get_drvdata(dev);
> + int status = regulator_is_enabled(data->reg);
> +
> + if (status < 0)
> + return status;
> +
> + return sysfs_emit(buf, "%s\n", status ? "on" : "off");
> +}
> +
> +static ssize_t state_store(struct device *dev, struct device_attribute *attr, const char *buf,
> + size_t count)
> +{
> + int status, wantstate;
> + struct regulator_extcon_data *data = dev_get_drvdata(dev);
> + struct regulator *reg = data->reg;
> +
> + if (sysfs_streq(buf, "on"))
> + wantstate = 1;
> + else if (sysfs_streq(buf, "off"))
> + wantstate = 0;
> + else
> + return -EINVAL;
> +
> + mutex_lock(&data->lock);
> +
> + status = regulator_is_enabled(reg);
> +
> + /*
> + * We need to ensure our enable/disable calls don't get imbalanced, so
> + * bail if we can't determine the current state.
> + */
> + if (status < 0)
> + goto out;
> +
> + /* Nothing further needed if we're already in the desired state */
> + if (!!status == wantstate) {
> + status = 0;
> + goto out;
> + }
> +
> + if (wantstate)
> + status = regulator_enable(reg);
> + else
> + status = regulator_disable(reg);
> +
> +out:
> + mutex_unlock(&data->lock);
> +
> + return status ? : count;
> +}
> +
> +static DEVICE_ATTR_RW(state);
> +
> +static struct attribute *regulator_extcon_attrs[] = {
> + &dev_attr_state.attr,
> + NULL,
> +};
> +ATTRIBUTE_GROUPS(regulator_extcon);
> +
> +static int regulator_extcon_probe(struct platform_device *pdev)
> +{
> + int ret;
> + struct regulator_extcon_data *data;
> + struct device *dev = &pdev->dev;
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->reg = devm_regulator_get_exclusive(&pdev->dev, "vout");
> + if (IS_ERR(data->reg))
> + return PTR_ERR(data->reg);
> +
> + mutex_init(&data->lock);
> +
> + /* No cables currently supported */
> + data->extcon_id = EXTCON_NONE;
> +
> + data->edev = devm_extcon_dev_allocate(dev, &data->extcon_id);
> + if (IS_ERR(data->edev))
> + return PTR_ERR(data->edev);
> +
> + ret = devm_extcon_dev_register(dev, data->edev);
> + if (ret < 0)
> + return ret;
> +
> + platform_set_drvdata(pdev, data);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id regulator_extcon_of_match_table[] = {
> + { .compatible = "regulator-connector" },
> + { },
> +};
> +
> +static struct platform_driver regulator_extcon_driver = {
> + .driver = {
> + .name = "extcon-regulator",
> + .of_match_table = of_match_ptr(regulator_extcon_of_match_table),
> + .dev_groups = regulator_extcon_groups,
> + },
> + .probe = regulator_extcon_probe,
> +};
> +module_platform_driver(regulator_extcon_driver);
> +
> +MODULE_AUTHOR("Zev Weiss <[email protected]>");
> +MODULE_DESCRIPTION("Regulator extcon driver");
> +MODULE_LICENSE("GPL");
On Mon, May 09, 2022 at 09:24:39PM +0900, Chanwoo Choi wrote:
> Hi Zev,
>
> I checked this patch. But, it doesn't look like the extcon provider
> driver. Because basically, extcon provider driver need the circuit
> in order to detect the kind of external connector. But, there are
> no any code for detection. Just add the specific sysfs attribute
> for only this driver. It is not standard interface.
OTOH it's something where if I look at the physical system with the
hardware there's a clearly visible external connector that I can point
to - it just happens to not support hotplug. It's not clear what other
system it would sit in, and it seems like an application that displays
external connections on a system in a UI would be able to do something
sensible with it.
Hi Mark, Zev,
On 5/17/22 10:03 AM, Zev Weiss wrote:
> [Adding Sebastian for drivers/power discussion]
>
> On Mon, May 09, 2022 at 08:24:16AM PDT, Mark Brown wrote:
>> On Mon, May 09, 2022 at 09:24:39PM +0900, Chanwoo Choi wrote:
>>> Hi Zev,
>>>
>>> I checked this patch. But, it doesn't look like the extcon provider
>>> driver. Because basically, extcon provider driver need the circuit
>>> in order to detect the kind of external connector. But, there are
>>> no any code for detection. Just add the specific sysfs attribute
>>> for only this driver. It is not standard interface.
>>
>> OTOH it's something where if I look at the physical system with the
>> hardware there's a clearly visible external connector that I can point
>> to - it just happens to not support hotplug. It's not clear what other
>> system it would sit in, and it seems like an application that displays
>> external connections on a system in a UI would be able to do something
>> sensible with it.
>
> Chanwoo, any further thoughts on Mark's reasoning above?
>
> I certainly understand the reluctance to add an extcon driver that
> doesn't really do anything with the extcon API, and I have no idea when
> we might end up enhancing it to do something more meaningful with that
> API (I don't know of any hardware at the moment that would need it).
>
> That said, as Mark points out, the hardware *is* ultimately an "external
> connector" (if a very simplistic one).
>
> Do you have any other ideas for where this functionality could go? Greg
> wasn't enthusiastic about a previous revision that had it in
> drivers/misc -- though now a fair amount of what was in that version is
> now going to be in the regulator core, so maybe that could be
> reconsidered?
>
> Or maybe something under drivers/power, though it's not really a supply
> or a reset device...drivers/power/output.c or something?
>
> Personally I don't have any terribly strong opinions on this, I'd just
> like to reach a mutually-agreeable consensus on a place for it to live.
>
After Mark's reply, I considered extcon provider driver without hotplug
feature. I think that extcon need to support the following something:
1. Need to specify the type of external connector instead of EXTCON_NONE.
2. extcon subsystem provides the standard sysfs interface
for non-hotplug extcon provider driver.
3. User can control the state of external connector via
the standard extcon sysfs attributes.
For example of extcon provider driver,
static const unsigned int supported_cables[] = {
EXTCON_USB,
EXTCON_NONE,
};
int extcon_usb_callback(int connector_id, int property_id, int state, void *data) {
struct extcon_dev *edev = data;
if (id == EXTCON_USB && property_id == EXTCON_NOT_HOTPLUG) {
regulator_enable() or regulator_disable()
}
return 0;
}
int extcon_provider_probe(...) {
edev = devm_extcon_dev_allocate(dev, supported_cables);
devm_extcon_dev_register(dev, edev);
extcon_set_property_capability(edev, EXTCON_USB, EXTCON_NOT_HOTPLUG);
extcon_set_property_callback(edev, EXTCON_USB, extcon_usb_callback);
...
}
And then user can change the state of each external connector
via '/sys/class/extcon/extcon0/cable.0/state'
if cable.0 contains the EXTCON_NOT_HOTPLUG property.
I'm designing this approach. But it has not yet decided
because try to check that this approach is right or not.
--
Best Regards,
Chanwoo Choi
Samsung Electronics
On 22. 5. 17. 19:07, Zev Weiss wrote:
> On Mon, May 16, 2022 at 08:15:31PM PDT, Chanwoo Choi wrote:
>> Hi Mark, Zev,
>>
>> On 5/17/22 10:03 AM, Zev Weiss wrote:
>>> [Adding Sebastian for drivers/power discussion]
>>>
>>> On Mon, May 09, 2022 at 08:24:16AM PDT, Mark Brown wrote:
>>>> On Mon, May 09, 2022 at 09:24:39PM +0900, Chanwoo Choi wrote:
>>>>> Hi Zev,
>>>>>
>>>>> I checked this patch. But, it doesn't look like the extcon provider
>>>>> driver. Because basically, extcon provider driver need the circuit
>>>>> in order to detect the kind of external connector. But, there are
>>>>> no any code for detection. Just add the specific sysfs attribute
>>>>> for only this driver. It is not standard interface.
>>>>
>>>> OTOH it's something where if I look at the physical system with the
>>>> hardware there's a clearly visible external connector that I can point
>>>> to - it just happens to not support hotplug. It's not clear what other
>>>> system it would sit in, and it seems like an application that displays
>>>> external connections on a system in a UI would be able to do something
>>>> sensible with it.
>>>
>>> Chanwoo, any further thoughts on Mark's reasoning above?
>>>
>>> I certainly understand the reluctance to add an extcon driver that
>>> doesn't really do anything with the extcon API, and I have no idea when
>>> we might end up enhancing it to do something more meaningful with that
>>> API (I don't know of any hardware at the moment that would need it).
>>>
>>> That said, as Mark points out, the hardware *is* ultimately an "external
>>> connector" (if a very simplistic one).
>>>
>>> Do you have any other ideas for where this functionality could go? Greg
>>> wasn't enthusiastic about a previous revision that had it in
>>> drivers/misc -- though now a fair amount of what was in that version is
>>> now going to be in the regulator core, so maybe that could be
>>> reconsidered?
>>>
>>> Or maybe something under drivers/power, though it's not really a supply
>>> or a reset device...drivers/power/output.c or something?
>>>
>>> Personally I don't have any terribly strong opinions on this, I'd just
>>> like to reach a mutually-agreeable consensus on a place for it to live.
>>>
>>
>> After Mark's reply, I considered extcon provider driver without hotplug
>> feature. I think that extcon need to support the following something:
>>
>> 1. Need to specify the type of external connector instead of EXTCON_NONE.
>> 2. extcon subsystem provides the standard sysfs interface
>> for non-hotplug extcon provider driver.
>> 3. User can control the state of external connector via
>> the standard extcon sysfs attributes.
>>
>>
>> For example of extcon provider driver,
>> static const unsigned int supported_cables[] = {
>> EXTCON_USB,
>> EXTCON_NONE,
>> };
>>
>> int extcon_usb_callback(int connector_id, int property_id, int state, void *data) {
>> struct extcon_dev *edev = data;
>>
>> if (id == EXTCON_USB && property_id == EXTCON_NOT_HOTPLUG) {
>> regulator_enable() or regulator_disable()
>> }
>>
>> return 0;
>> }
>>
>> int extcon_provider_probe(...) {
>> edev = devm_extcon_dev_allocate(dev, supported_cables);
>>
>> devm_extcon_dev_register(dev, edev);
>>
>> extcon_set_property_capability(edev, EXTCON_USB, EXTCON_NOT_HOTPLUG);
>> extcon_set_property_callback(edev, EXTCON_USB, extcon_usb_callback);
>>
>> ...
>> }
>>
>> And then user can change the state of each external connector
>> via '/sys/class/extcon/extcon0/cable.0/state'
>> if cable.0 contains the EXTCON_NOT_HOTPLUG property.
>>
>> I'm designing this approach. But it has not yet decided
>> because try to check that this approach is right or not.
>>
>
> Okay, so if I'm understanding correctly we'd be using the extcon
> subsystem's existing attached/detached state to model (and control) the
> on/off state of the power output?
The extcon provides the sysfs interface to control the state of each cable
and then passes the extra role to each extcon provider driver with using
the registered callback. The extcon core don't care the detailed operation
in registered callback. Just provide the interface from sysfs interface
to extcon provider driver and then handle the state of external connector.
In your case, you might enable/disable the regulator on the registered callback
like extcon_usb_callback in example.
>
> That could work for the particular hardware I'm dealing with at the
> moment, though I'd be a bit concerned that conflating the two might
> constrain things in the future if there's some similar but slightly more
> sophisticated hardware we'd want to extend the same driver to support.
> For example on a power connector with some capability for presence
> detection, we might want to be able to support "attached but powered
> off" as a valid state for it to be in -- would the above approach be
> able to do that?
Yes. As I mentioned above, the regulator control is extra role
for the specific extcon provider driver. Any extcon provider driver
without hotplug feature might control the audio volume in the registered
callback like extcon_usb_callback in example. The extra job depends
on the extcon provider driver.
>
>
> Thanks,
> Zev
>
--
Best Regards,
Samsung Electronics
Chanwoo Choi
[Adding Sebastian for drivers/power discussion]
On Mon, May 09, 2022 at 08:24:16AM PDT, Mark Brown wrote:
> On Mon, May 09, 2022 at 09:24:39PM +0900, Chanwoo Choi wrote:
> > Hi Zev,
> >
> > I checked this patch. But, it doesn't look like the extcon provider
> > driver. Because basically, extcon provider driver need the circuit
> > in order to detect the kind of external connector. But, there are
> > no any code for detection. Just add the specific sysfs attribute
> > for only this driver. It is not standard interface.
>
> OTOH it's something where if I look at the physical system with the
> hardware there's a clearly visible external connector that I can point
> to - it just happens to not support hotplug. It's not clear what other
> system it would sit in, and it seems like an application that displays
> external connections on a system in a UI would be able to do something
> sensible with it.
Chanwoo, any further thoughts on Mark's reasoning above?
I certainly understand the reluctance to add an extcon driver that
doesn't really do anything with the extcon API, and I have no idea when
we might end up enhancing it to do something more meaningful with that
API (I don't know of any hardware at the moment that would need it).
That said, as Mark points out, the hardware *is* ultimately an "external
connector" (if a very simplistic one).
Do you have any other ideas for where this functionality could go? Greg
wasn't enthusiastic about a previous revision that had it in
drivers/misc -- though now a fair amount of what was in that version is
now going to be in the regulator core, so maybe that could be
reconsidered?
Or maybe something under drivers/power, though it's not really a supply
or a reset device...drivers/power/output.c or something?
Personally I don't have any terribly strong opinions on this, I'd just
like to reach a mutually-agreeable consensus on a place for it to live.
Thanks,
Zev
On Mon, May 16, 2022 at 08:15:31PM PDT, Chanwoo Choi wrote:
> Hi Mark, Zev,
>
> On 5/17/22 10:03 AM, Zev Weiss wrote:
> > [Adding Sebastian for drivers/power discussion]
> >
> > On Mon, May 09, 2022 at 08:24:16AM PDT, Mark Brown wrote:
> >> On Mon, May 09, 2022 at 09:24:39PM +0900, Chanwoo Choi wrote:
> >>> Hi Zev,
> >>>
> >>> I checked this patch. But, it doesn't look like the extcon provider
> >>> driver. Because basically, extcon provider driver need the circuit
> >>> in order to detect the kind of external connector. But, there are
> >>> no any code for detection. Just add the specific sysfs attribute
> >>> for only this driver. It is not standard interface.
> >>
> >> OTOH it's something where if I look at the physical system with the
> >> hardware there's a clearly visible external connector that I can point
> >> to - it just happens to not support hotplug. It's not clear what other
> >> system it would sit in, and it seems like an application that displays
> >> external connections on a system in a UI would be able to do something
> >> sensible with it.
> >
> > Chanwoo, any further thoughts on Mark's reasoning above?
> >
> > I certainly understand the reluctance to add an extcon driver that
> > doesn't really do anything with the extcon API, and I have no idea when
> > we might end up enhancing it to do something more meaningful with that
> > API (I don't know of any hardware at the moment that would need it).
> >
> > That said, as Mark points out, the hardware *is* ultimately an "external
> > connector" (if a very simplistic one).
> >
> > Do you have any other ideas for where this functionality could go? Greg
> > wasn't enthusiastic about a previous revision that had it in
> > drivers/misc -- though now a fair amount of what was in that version is
> > now going to be in the regulator core, so maybe that could be
> > reconsidered?
> >
> > Or maybe something under drivers/power, though it's not really a supply
> > or a reset device...drivers/power/output.c or something?
> >
> > Personally I don't have any terribly strong opinions on this, I'd just
> > like to reach a mutually-agreeable consensus on a place for it to live.
> >
>
> After Mark's reply, I considered extcon provider driver without hotplug
> feature. I think that extcon need to support the following something:
>
> 1. Need to specify the type of external connector instead of EXTCON_NONE.
> 2. extcon subsystem provides the standard sysfs interface
> for non-hotplug extcon provider driver.
> 3. User can control the state of external connector via
> the standard extcon sysfs attributes.
>
>
> For example of extcon provider driver,
> static const unsigned int supported_cables[] = {
> EXTCON_USB,
> EXTCON_NONE,
> };
>
> int extcon_usb_callback(int connector_id, int property_id, int state, void *data) {
> struct extcon_dev *edev = data;
>
> if (id == EXTCON_USB && property_id == EXTCON_NOT_HOTPLUG) {
> regulator_enable() or regulator_disable()
> }
>
> return 0;
> }
>
> int extcon_provider_probe(...) {
> edev = devm_extcon_dev_allocate(dev, supported_cables);
>
> devm_extcon_dev_register(dev, edev);
>
> extcon_set_property_capability(edev, EXTCON_USB, EXTCON_NOT_HOTPLUG);
> extcon_set_property_callback(edev, EXTCON_USB, extcon_usb_callback);
>
> ...
> }
>
> And then user can change the state of each external connector
> via '/sys/class/extcon/extcon0/cable.0/state'
> if cable.0 contains the EXTCON_NOT_HOTPLUG property.
>
> I'm designing this approach. But it has not yet decided
> because try to check that this approach is right or not.
>
Okay, so if I'm understanding correctly we'd be using the extcon
subsystem's existing attached/detached state to model (and control) the
on/off state of the power output?
That could work for the particular hardware I'm dealing with at the
moment, though I'd be a bit concerned that conflating the two might
constrain things in the future if there's some similar but slightly more
sophisticated hardware we'd want to extend the same driver to support.
For example on a power connector with some capability for presence
detection, we might want to be able to support "attached but powered
off" as a valid state for it to be in -- would the above approach be
able to do that?
Thanks,
Zev