From: cw00.choi@samsung•com (Chanwoo Choi)
To: linux-arm-kernel@lists•infradead.org
Subject: [linux-sunxi] Re: [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG
Date: Thu, 11 Jun 2015 19:30:38 +0900 [thread overview]
Message-ID: <5579634E.9080004@samsung.com> (raw)
In-Reply-To: <5579572D.4070006@redhat.com>
Hi,
On 06/11/2015 06:38 PM, Hans de Goede wrote:
> Hi,
>
> On 11-06-15 11:33, Chanwoo Choi wrote:
>> Hi,
>>
>> On 06/11/2015 05:59 PM, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 11-06-15 10:29, Chanwoo Choi wrote:
>>>> Hi Hans,
>>>>
>>>> On 06/11/2015 05:21 PM, Hans de Goede wrote:
>>>>> Hi Chanwoo,
>>>>>
>>>>> Thanks for the quick review.
>>>>>
>>>>> On 11-06-15 10:07, Chanwoo Choi wrote:
>>>>>> Hi Hans,
>>>>>>
>>>>>> I add the comment about extcon-related code.
>>>>>>
>>>>>> Firstly,
>>>>>> I'd like you to implment the extcon driver for phy-sun4i-usb device
>>>>>> in drivers/extcon/ directoryby using MFD
>>>>>
>>>>> No, just no, this is not what the MFD framework is for, the usb-phy
>>>>> in question here is not a multifunction device. The MFD framework
>>>>> is intended for true multi-function devices like i2c attached
>>>>> PMICs which have regulators, gpios, pwm, input (power button),
>>>>> chargers, power-supply, etc. That is NOT the case here.
>>>>>
>>>>> Also moving this to the MFD framework would very likely requiring
>>>>> the devicetree binding for the usb-phy to change which we cannot
>>>>> do as that would break the devicetree ABI.
>>>>>
>>>>>> because there are both extcon
>>>>>> provider driver and extcon client driver. I think that all extcon
>>>>>> provider driver better to be included in drivers/extcon/ directory.
>>>>>> extcon_set_cable_state() function should be handled in extcon provider
>>>>>> driver which is incluced in drivers/extcon/ directory.
>>>>>
>>>>> I do not find this a compelling reason, there are plenty of subsystems
>>>>> where not all implementations of the subsystem class live in the subsystem
>>>>> directory, e.g. input and hwmon devices are often also found outside of
>>>>> the input and hwmon driver directories.
>>>>
>>>> There are difference on between input/hwmon and extcon.
>>>>
>>>> Because input and hwmon driver implement the only one type driver as provider driver.
>>>> But, extcon implement the two type driver of both extcon provider and extcon client driver.
>>>> The extcon is similiar with regulator and clock framework as resource.
>>>>
>>>> extcon provider driver to provider the event when the state of external connector is changed.
>>>> - devm_extcon_dev_register()
>>>> - e.g., almost extcon provider driver are included in 'drivers/extcon/' directory.
>>>
>>> I understand, but that does not change my first argument, that the usb-phy is not
>>> a MFD device. And although it may be desirable to keep extcon provider drivers
>>> in the drivers/extcon, there are no technical reasons to do so.
>>>
>>> The whole reason why Kishon asked me to start using the extcon framework is to avoid
>>> adding a private API to the phy-sun4i-usb code for notifying the musb-sunxi code
>>> about otg-id-pin status changes. Adding a separate driver for just the extcon bits
>>> means re-adding a private api to the phy-sun4i-usb code but this time for the
>>> extcon code, at which point we might just as well skip extcon and have the
>>> musb-sunxi glue code call directly into the phy-sun4i-usb code...
>>>
>>> Needing a private API for a separate extcn driver actually is a good argument to
>>> NOT have a separate extcon driver and keep the extcon code in the phy-sun4i-usb code,
>>> where as I see no technical arguments in favor of a separate extcon driver.
>>
>> There is one technical issue.
>>
>> The extcon_set_cable_state() should be handled by extcon provider driver.
>
> That is something which can be done regardless of where the extcon provider
> driver code is located in the kernel tree...
>
>> because extcon_set_cable_state() inform the extcon client driver of the event
>> when detecting the change of h/w line (gpio line) or register of peripheral device.
>>
>> But, extcon client driver can now get the instance of extcon_dev structure
>> by extcon_get_edev_by_phandle() and then can change the cable state by using the extcon_set_cable_state().
>>
>> I think that these issue have to be protected by framework level.
>
> Protecting this at the framework level would mean protecting it with code
> in drivers/extcon/extcon.c, that code will be used (and thus can protect
> things) regardless of where the extcon provider code lives.
>
> I really still see no technical reasons why all extcon provider code
> MUST be under drivers/extcon.
OK. As you said, this issue shold be prevented on framework.
I'm considering what is appropriate method to resolve this issue.
Thanks,
Chanwoo Choi
next prev parent reply other threads:[~2015-06-11 10:30 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-31 16:10 [PATCH v4 0/2] Remaining sunxi musb patches Hans de Goede
2015-05-31 16:10 ` [PATCH v4 1/2] phy-sun4i-usb: Add full support for usb0 phy / OTG Hans de Goede
2015-06-01 9:22 ` Maxime Ripard
2015-06-01 9:28 ` Hans de Goede
2015-06-02 15:22 ` Maxime Ripard
2015-06-11 5:48 ` Kishon Vijay Abraham I
2015-06-11 7:56 ` [linux-sunxi] " Hans de Goede
2015-06-11 8:28 ` Hans de Goede
2015-06-11 8:07 ` Chanwoo Choi
2015-06-11 8:21 ` [linux-sunxi] " Hans de Goede
2015-06-11 8:29 ` Chanwoo Choi
2015-06-11 8:59 ` Hans de Goede
2015-06-11 9:33 ` Chanwoo Choi
2015-06-11 9:37 ` Chanwoo Choi
2015-06-11 9:38 ` Hans de Goede
2015-06-11 10:30 ` Chanwoo Choi [this message]
2015-06-11 9:42 ` Kishon Vijay Abraham I
2015-06-11 9:53 ` Hans de Goede
2015-06-11 11:16 ` Kishon Vijay Abraham I
2015-06-11 12:35 ` [linux-sunxi] " Hans de Goede
2015-06-11 12:59 ` Kishon Vijay Abraham I
2015-07-15 10:55 ` Kishon Vijay Abraham I
2015-07-15 10:58 ` Kishon Vijay Abraham I
2015-05-31 16:10 ` [PATCH v4 2/2] musb: Add support for the Allwinner sunxi musb controller Hans de Goede
2015-06-02 15:02 ` Felipe Balbi
[not found] ` <79fc4306-a621-43b8-b347-e42a27beb134@googlegroups.com>
2015-06-01 18:34 ` [PATCH v4 0/2] Remaining sunxi musb patches Felipe Balbi
2015-06-02 7:14 ` Hans de Goede
2015-06-02 14:45 ` Felipe Balbi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5579634E.9080004@samsung.com \
--to=cw00.choi@samsung$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox