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.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
