linux-kernel - Re: [PATCH v2 2/2] extcon: Add extcon-regulator driver

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bc6595c8-5f05-ac2c-63e0-f442f9ec83be@samsung.com>
Date:   Tue, 17 May 2022 12:15:31 +0900
From:   Chanwoo Choi <cw00.choi@...sung.com>
To:     Zev Weiss <zev@...ilderbeest.net>, Mark Brown <broonie@...nel.org>
Cc:     Chanwoo Choi <cwchoi00@...il.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        MyungJoo Ham <myungjoo.ham@...sung.com>,
        linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        openbmc@...ts.ozlabs.org,
        Sebastian Reichel <sebastian.reichel@...labora.com>
Subject: Re: [PATCH v2 2/2] extcon: Add extcon-regulator driver

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