Re: [PATCH v3] USB: serial: ftdi_sio: Add support for CBUS GPIO
Hi Johan, On 4 September 2018 at 14:47, Johan Hovold wrote: > On Sat, Aug 04, 2018 at 12:17:15PM +0200, Loic Poulain wrote: >> Some FTDI devices like FTX or FT232R support CBUS Bit Bang mode on CBUS >> pins, allowing host to control them via simple USB control transfers. >> To make use of a CBUS pin in Bit Bang mode, the pin must be configured >> to I/O mode in the FTDI EEPROM. This mode perfectly coexists with regular >> USB to Serial function. >> >> In this implementation, a GPIO controller is registered on FTDI probe >> if at least one CBUS pin is configured for I/O mode. For now, only >> FTX devices are supported. >> >> This patch is based on previous Stefan Agner implementation tentative >> on LKML [1]. >> >> [1] Message-Id: 1434838377-8042-1-git-send-email-ste...@agner.ch >> >> Signed-off-by: Loic Poulain >> --- >> v2: Use message-id for LKML reference >> Rework read_eeprom according to Andy's comment and return read count >> Remove noisy messages >> Comment style alignment >> Add defines for magic values >> Cannot use devm, because gpiochip is linked to the port not to the udev >> v3: >>Fix typo in CBUS mask description comment > > First, thanks for looking into this; seems like we're finally about to > get support for the CBUS gpios. > > But now I get not one, but two, competing implementations for this > posted in one month: > > https://lkml.kernel.org/r/20180825204744.2307-1-pa...@pados.hu > > And it looks like you both have been considering at least some of the > earlier attempts, which is great. > > I've reviewed both patches and it seems to me that Karoly's version is > closer to what I'd like to see as the end-result. Specifically this > means having: > > - fixed offsets for the physical pins rather than having the offsets >depend on eeprom configuration > > - better type abstraction (we want to add support for ft232r and ft232h >eventually as well) > > The other patch is also more complete in that it: > > - considers the initial pin state > - implements a request() callback > - implements a get_direction() callback > > In contrast, with this implementation as it stands, we could end up with > a driven pin being reported as an input (corner case, but still). > > Implementation-wise, the other patch is also closer to what I'd like to > see (e.g. not using atomic bit ops, and getting the error handling right > from the start). > > There are some issues that needs to be addressed in the other patch as > well, and in doing so it would be wise to look at your patch for > inspiration (e.g. naming issues and adding an eeprom helper to only read > the couple of configuration words needed). > > In the end, going with the other patch means less work for me, even if > you both (by unfortunate timing) have forced me to do more than twice > the amount of review work already. Thanks for review, I'm fine with that and I'll review the other patch. Karoly can use chunks of my patch if useful. Regards, Loic
[PATCH v3 6/6] arm: dts: qcom: db410c: Enable USB OTG support
The Dragonboard-410c is able to act either as USB Host or Device.
The role can be determined at runtime via the USB_HS_ID pin which is
derived from the micro-usb port VBUS pin.
In Host role, SoC USB D+/D- are routed to the onboard USB 2.0 HUB.
In Device role, SoC USB D+/D- are routed to the USB 2.0 micro B port.
Routing is selected via USB_SW_SEL_PM gpio.
In device role USB HUB can be held in reset.
chipidea driver expects two extcon device pointers, one for the
EXTCON_USB event and one for the EXTCON_USB_HOST event. Since
the extcon-usb-gpio device is capable of generating both these
events, point two times to this extcon device.
Signed-off-by: Loic Poulain
---
v2: no change
v3: Point two times to the same extcon-usb-device
arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi | 20
arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 11 ++-
2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
b/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
index ec2f0de..99787cc 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
@@ -8,6 +8,16 @@
pinconf {
pins = "gpio3";
function = PMIC_GPIO_FUNC_NORMAL;
+ input-disable;
+ output-high;
+ };
+ };
+
+ usb_hub_reset_pm_device: usb_hub_reset_pm_device {
+ pinconf {
+ pins = "gpio3";
+ function = PMIC_GPIO_FUNC_NORMAL;
+ input-disable;
output-low;
};
};
@@ -22,6 +32,16 @@
};
};
+ usb_sw_sel_pm_device: usb_sw_sel_pm_device {
+ pinconf {
+ pins = "gpio4";
+ function = PMIC_GPIO_FUNC_NORMAL;
+ power-source = ;
+ input-disable;
+ output-low;
+ };
+ };
+
pm8916_gpios_leds: pm8916_gpios_leds {
pinconf {
pins = "gpio1", "gpio2";
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
index 78ce397..1f7dc1c 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
@@ -366,14 +366,15 @@
};
usb@78d9000 {
- extcon = <_id>;
+ extcon = <_id>, <_id>;
status = "okay";
adp-disable;
hnp-disable;
srp-disable;
- dr_mode = "host";
- pinctrl-names = "default";
- pinctrl-0 = <_sw_sel_pm>;
+ dr_mode = "otg";
+ pinctrl-names = "default", "device";
+ pinctrl-0 = <_sw_sel_pm _hub_reset_pm>;
+ pinctrl-1 = <_sw_sel_pm_device
_hub_reset_pm_device>;
ulpi {
phy {
v1p8-supply = <_l7>;
@@ -512,7 +513,7 @@
usb_id: usb-id {
compatible = "linux,extcon-usb-gpio";
- vbus-gpio = < 121 GPIO_ACTIVE_HIGH>;
+ id-gpio = < 121 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
pinctrl-0 = <_id_default>;
};
--
2.7.4
[PATCH v3 5/6] phy: qcom-usb-hs: Fix unbalanced notifier registration
Phy power on/off cycle can happen several times during device life.
We then need to balance the extcon notifier registration accordingly.
Fixes: f0b5c2c96370 ("phy: qcom-usb-hs: Replace the extcon API")
Signed-off-by: Loic Poulain
---
v2: don't use devres version (power-on always followed by power-off)
v3: no change
drivers/phy/qualcomm/phy-qcom-usb-hs.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
index abbbe75..7153dde 100644
--- a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
+++ b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
@@ -160,8 +160,8 @@ static int qcom_usb_hs_phy_power_on(struct phy *phy)
/* setup initial state */
qcom_usb_hs_phy_vbus_notifier(>vbus_notify, state,
uphy->vbus_edev);
- ret = devm_extcon_register_notifier(>dev, uphy->vbus_edev,
- EXTCON_USB, >vbus_notify);
+ ret = extcon_register_notifier(uphy->vbus_edev, EXTCON_USB,
+ >vbus_notify);
if (ret)
goto err_ulpi;
}
@@ -182,6 +182,11 @@ static int qcom_usb_hs_phy_power_off(struct phy *phy)
{
struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
+ if (uphy->vbus_edev) {
+ extcon_unregister_notifier(uphy->vbus_edev, EXTCON_USB,
+ >vbus_notify);
+ }
+
regulator_disable(uphy->v3p3);
regulator_disable(uphy->v1p8);
clk_disable_unprepare(uphy->sleep_clk);
--
2.7.4
[PATCH v3 4/6] usb: chipidea: Fix otg event handler
At OTG work running time, it's possible that several events need to be
addressed (e.g. ID and VBUS events). The current implementation handles
only one event at a time which leads to ignoring the other one. Fix it.
Signed-off-by: Loic Poulain
---
v2: v3: no change
drivers/usb/chipidea/otg.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index db4ceff..f25d482 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -203,14 +203,17 @@ static void ci_otg_work(struct work_struct *work)
}
pm_runtime_get_sync(ci->dev);
+
if (ci->id_event) {
ci->id_event = false;
ci_handle_id_switch(ci);
- } else if (ci->b_sess_valid_event) {
+ }
+
+ if (ci->b_sess_valid_event) {
ci->b_sess_valid_event = false;
ci_handle_vbus_change(ci);
- } else
- dev_err(ci->dev, "unexpected event occurs at %s\n", __func__);
+ }
+
pm_runtime_put_sync(ci->dev);
enable_irq(ci->irq);
--
2.7.4
[PATCH v3 1/6] usb: chipidea: Add dynamic pinctrl selection
Some hardware implementations require to configure pins differently
according to the USB role (host/device), this can be an update of the
pins routing or a simple GPIO value change.
This patch introduces new optional "host" and "device" pinctrls.
If these pinctrls are defined by the device, they are respectively
selected on host/device role start.
If a default pinctrl exist, it is restored on host/device role stop.
Signed-off-by: Loic Poulain
---
v2: includes ordering
v3: no change
drivers/usb/chipidea/core.c | 19 +++
drivers/usb/chipidea/host.c | 9 +
drivers/usb/chipidea/udc.c | 9 +
include/linux/usb/chipidea.h | 6 ++
4 files changed, 43 insertions(+)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 85fc6db..7bfcbb2 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -53,6 +53,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -723,6 +724,24 @@ static int ci_get_platdata(struct device *dev,
else
cable->connected = false;
}
+
+ platdata->pctl = devm_pinctrl_get(dev);
+ if (!IS_ERR(platdata->pctl)) {
+ struct pinctrl_state *p;
+
+ p = pinctrl_lookup_state(platdata->pctl, "default");
+ if (!IS_ERR(p))
+ platdata->pins_default = p;
+
+ p = pinctrl_lookup_state(platdata->pctl, "host");
+ if (!IS_ERR(p))
+ platdata->pins_host = p;
+
+ p = pinctrl_lookup_state(platdata->pctl, "device");
+ if (!IS_ERR(p))
+ platdata->pins_device = p;
+ }
+
return 0;
}
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 4638d9b..d858a82 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include "../host/ehci.h"
@@ -153,6 +154,10 @@ static int host_start(struct ci_hdrc *ci)
}
}
+ if (ci->platdata->pins_host)
+ pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_host);
+
ret = usb_add_hcd(hcd, 0, 0);
if (ret) {
goto disable_reg;
@@ -197,6 +202,10 @@ static void host_stop(struct ci_hdrc *ci)
}
ci->hcd = NULL;
ci->otg.host = NULL;
+
+ if (ci->platdata->pins_host && ci->platdata->pins_default)
+ pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_default);
}
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 9852ec5..829e947 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1965,6 +1966,10 @@ void ci_hdrc_gadget_destroy(struct ci_hdrc *ci)
static int udc_id_switch_for_device(struct ci_hdrc *ci)
{
+ if (ci->platdata->pins_device)
+ pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_device);
+
if (ci->is_otg)
/* Clear and enable BSV irq */
hw_write_otgsc(ci, OTGSC_BSVIS | OTGSC_BSVIE,
@@ -1983,6 +1988,10 @@ static void udc_id_switch_for_host(struct ci_hdrc *ci)
hw_write_otgsc(ci, OTGSC_BSVIE | OTGSC_BSVIS, OTGSC_BSVIS);
ci->vbus_active = 0;
+
+ if (ci->platdata->pins_device && ci->platdata->pins_default)
+ pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_default);
}
/**
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 07f9936..63758c3 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -77,6 +77,12 @@ struct ci_hdrc_platform_data {
struct ci_hdrc_cablevbus_extcon;
struct ci_hdrc_cableid_extcon;
u32 phy_clkgate_delay_us;
+
+ /* pins */
+ struct pinctrl *pctl;
+ struct pinctrl_state *pins_default;
+ struct pinctrl_state *pins_host;
+ struct pinctrl_state *pins_device;
};
/* Default offset of capability registers */
--
2.7.4
[PATCH v3 3/6] usb: chipidea: Prevent unbalanced IRQ disable
The ChipIdea IRQ is disabled before scheduling the otg work and
re-enabled on otg work completion. However if the job is already
scheduled we have to undo the effect of disable_irq int order to
balance the IRQ disable-depth value.
Fixes: be6b0c1bd0be ("usb: chipidea: using one inline function to cover queue
work operations")
Signed-off-by: Loic Poulain
---
v2: v3: no change
drivers/usb/chipidea/otg.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
index 7e7428e..4f8b817 100644
--- a/drivers/usb/chipidea/otg.h
+++ b/drivers/usb/chipidea/otg.h
@@ -17,7 +17,8 @@ void ci_handle_vbus_change(struct ci_hdrc *ci);
static inline void ci_otg_queue_work(struct ci_hdrc *ci)
{
disable_irq_nosync(ci->irq);
- queue_work(ci->wq, >work);
+ if (queue_work(ci->wq, >work) == false)
+ enable_irq(ci->irq);
}
#endif /* __DRIVERS_USB_CHIPIDEA_OTG_H */
--
2.7.4
[PATCH v3 2/6] doc: usb: ci-hdrc-usb2: Add pinctrl properties definition
Some hardware implementations require to configure pins differently according to the USB role (host/device), this can be an update of the pins routing or a simple GPIO value change. This patch introduces new optional "host" and "device" pinctrls. If these pinctrls are defined by the device, they are respectively selected on host/device role start. Signed-off-by: Loic Poulain --- v2: Add new pin modes documentation (host, device) v3: rebase on usb-next Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt index 2e93181..529e518 100644 --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt @@ -80,6 +80,8 @@ Optional properties: controller. It's expected that a mux state of 0 indicates device mode and a mux state of 1 indicates host mode. - mux-control-names: Shall be "usb_switch" if mux-controls is specified. +- pinctrl-names: Names for optional pin modes in "default", "host", "device" +- pinctrl-n: alternate pin modes i.mx specific properties - fsl,usbmisc: phandler of non-core register device, with one -- 2.7.4
Re: [PATCH v2 3/7] usb: chipidea: Support generic usb extcon
Hi Peter,
On 28 August 2018 at 08:33, Peter Chen wrote:
>
>> Add compatibility for extcon-usb-gpio which can handle more than one cable
>> per
>> instance, allowing coherency of USB cable states (USB/USB-HOST). These states
>> can be generated from ID or/and VBUS pins.
>>
>> In case only one extcon device is associated to the USB device, and this
>> device
>> supports USB and USB-HOST cable states, we now use it for both VBUS (USB) and
>> ID (USB-HOST) notifier.
>>
>> Signed-off-by: Loic Poulain
>> ---
>> v2: no change
>>
>> drivers/usb/chipidea/core.c | 11 +++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index
>> cdac778..afe85e2 100644
>> --- a/drivers/usb/chipidea/core.c
>> +++ b/drivers/usb/chipidea/core.c
>> @@ -702,6 +702,17 @@ static int ci_get_platdata(struct device *dev,
>> ext_id = extcon_get_edev_by_phandle(dev, 1);
>> if (IS_ERR(ext_id) && PTR_ERR(ext_id) != -ENODEV)
>> return PTR_ERR(ext_id);
>> +
>> + /*
>> + * Some extcon devices like extcon-usb-gpio have only one
>> + * instance for both USB and USB-HOST cable states.
>> + */
>> + if (!IS_ERR(ext_vbus) && IS_ERR(ext_id)) {
>> + if (extcon_get_state(ext_vbus, EXTCON_USB) >= 0 &&
>> + extcon_get_state(ext_vbus, EXTCON_USB_HOST) >= 0) {
>> + ext_id = ext_vbus;
>> + }
>> + }
>> }
>>
>
> Hi Loic,
>
> For your case, I suggest changing dts instead of changing source code, you
> would have both
> vbus and id phandle at your controller dts, and both point to the same extcon
> device.
Good point, will do.
Regards,
Loic
[PATCH v2 2/7] doc: usb: ci-hdrc-usb2: Add pinctrl properties definition
Some hardware implementations require to configure pins differently according to the USB role (host/device), this can be an update of the pins routing or a simple GPIO value change. This patch introduces new optional "host" and "device" pinctrls. If these pinctrls are defined by the device, they are respectively selected on host/device role start. Signed-off-by: Loic Poulain --- v2: Add new pin modes documentation (host, device) Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt index 0e03344..ea7033c 100644 --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt @@ -76,6 +76,8 @@ Optional properties: needs to make sure it does not send more than 90% maximum_periodic_data_per_frame. The use case is multiple transactions, but less frame rate. +- pinctrl-names: Names for optional pin modes in "default", "host", "device" +- pinctrl-n: alternate pin modes i.mx specific properties - fsl,usbmisc: phandler of non-core register device, with one -- 2.7.4
[PATCH v2 5/7] usb: chipidea: Fix otg event handler
At OTG work running time, it's possible that several events need to be
addressed (e.g. ID and VBUS events). The current implementation handles
only one event at a time which leads to ignoring the other one. Fix it.
Signed-off-by: Loic Poulain
---
v2: no change
drivers/usb/chipidea/otg.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index 10236fe..8bf4032 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -206,14 +206,17 @@ static void ci_otg_work(struct work_struct *work)
}
pm_runtime_get_sync(ci->dev);
+
if (ci->id_event) {
ci->id_event = false;
ci_handle_id_switch(ci);
- } else if (ci->b_sess_valid_event) {
+ }
+
+ if (ci->b_sess_valid_event) {
ci->b_sess_valid_event = false;
ci_handle_vbus_change(ci);
- } else
- dev_err(ci->dev, "unexpected event occurs at %s\n", __func__);
+ }
+
pm_runtime_put_sync(ci->dev);
enable_irq(ci->irq);
--
2.7.4
[PATCH v2 6/7] phy: qcom-usb-hs: Fix unbalanced notifier registration
Phy power on/off cycle can happen several times during device life.
We then need to balance the extcon notifier registration accordingly.
Fixes: f0b5c2c96370 ("phy: qcom-usb-hs: Replace the extcon API")
Signed-off-by: Loic Poulain
---
v2: don't use devres version (power-on always followed by power-off)
drivers/phy/qualcomm/phy-qcom-usb-hs.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
index 2d0c70b..18da6e2 100644
--- a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
+++ b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
@@ -159,8 +159,8 @@ static int qcom_usb_hs_phy_power_on(struct phy *phy)
/* setup initial state */
qcom_usb_hs_phy_vbus_notifier(>vbus_notify, state,
uphy->vbus_edev);
- ret = devm_extcon_register_notifier(>dev, uphy->vbus_edev,
- EXTCON_USB, >vbus_notify);
+ ret = extcon_register_notifier(uphy->vbus_edev, EXTCON_USB,
+ >vbus_notify);
if (ret)
goto err_ulpi;
}
@@ -181,6 +181,11 @@ static int qcom_usb_hs_phy_power_off(struct phy *phy)
{
struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
+ if (uphy->vbus_edev) {
+ extcon_unregister_notifier(uphy->vbus_edev, EXTCON_USB,
+ >vbus_notify);
+ }
+
regulator_disable(uphy->v3p3);
regulator_disable(uphy->v1p8);
clk_disable_unprepare(uphy->sleep_clk);
--
2.7.4
[PATCH v2 4/7] usb: chipidea: Prevent unbalanced IRQ disable
The ChipIdea IRQ is disabled before scheduling the otg work and
re-enabled on otg work completion. However if the job is already
scheduled we have to undo the effect of disable_irq int order to
balance the IRQ disable-depth value.
Fixes: be6b0c1bd0be ("usb: chipidea: using one inline function to cover queue
work operations")
Signed-off-by: Loic Poulain
---
v2: no change
drivers/usb/chipidea/otg.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
index 9ecb598..a5557c7 100644
--- a/drivers/usb/chipidea/otg.h
+++ b/drivers/usb/chipidea/otg.h
@@ -20,7 +20,8 @@ void ci_handle_vbus_change(struct ci_hdrc *ci);
static inline void ci_otg_queue_work(struct ci_hdrc *ci)
{
disable_irq_nosync(ci->irq);
- queue_work(ci->wq, >work);
+ if (queue_work(ci->wq, >work) == false)
+ enable_irq(ci->irq);
}
#endif /* __DRIVERS_USB_CHIPIDEA_OTG_H */
--
2.7.4
[PATCH v2 3/7] usb: chipidea: Support generic usb extcon
Add compatibility for extcon-usb-gpio which can handle more
than one cable per instance, allowing coherency of USB cable
states (USB/USB-HOST). These states can be generated from ID
or/and VBUS pins.
In case only one extcon device is associated to the USB device,
and this device supports USB and USB-HOST cable states, we now
use it for both VBUS (USB) and ID (USB-HOST) notifier.
Signed-off-by: Loic Poulain
---
v2: no change
drivers/usb/chipidea/core.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index cdac778..afe85e2 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -702,6 +702,17 @@ static int ci_get_platdata(struct device *dev,
ext_id = extcon_get_edev_by_phandle(dev, 1);
if (IS_ERR(ext_id) && PTR_ERR(ext_id) != -ENODEV)
return PTR_ERR(ext_id);
+
+ /*
+* Some extcon devices like extcon-usb-gpio have only one
+* instance for both USB and USB-HOST cable states.
+*/
+ if (!IS_ERR(ext_vbus) && IS_ERR(ext_id)) {
+ if (extcon_get_state(ext_vbus, EXTCON_USB) >= 0 &&
+ extcon_get_state(ext_vbus, EXTCON_USB_HOST) >= 0) {
+ ext_id = ext_vbus;
+ }
+ }
}
cable = >vbus_extcon;
--
2.7.4
[PATCH v2 7/7] arm: dts: qcom: db410c: Enable USB OTG support
The Dragonboard-410c is able to act either as USB Host or Device.
The role can be determined at runtime via the USB_HS_ID pin which is
derived from the micro-usb port VBUS pin.
In Host role, SoC USB D+/D- are routed to the onboard USB 2.0 HUB.
In Device role, SoC USB D+/D- are routed to the USB 2.0 micro B port.
Routing is selected via USB_SW_SEL_PM gpio.
In device role USB HUB can be held in reset.
Signed-off-by: Loic Poulain
---
v2: no change
arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi | 20
arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 9 +
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
b/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
index 16c1f1b..7229ad9 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
@@ -8,6 +8,16 @@
pinconf {
pins = "gpio3";
function = PMIC_GPIO_FUNC_NORMAL;
+ input-disable;
+ output-high;
+ };
+ };
+
+ usb_hub_reset_pm_device: usb_hub_reset_pm_device {
+ pinconf {
+ pins = "gpio3";
+ function = PMIC_GPIO_FUNC_NORMAL;
+ input-disable;
output-low;
};
};
@@ -22,6 +32,16 @@
};
};
+ usb_sw_sel_pm_device: usb_sw_sel_pm_device {
+ pinconf {
+ pins = "gpio4";
+ function = PMIC_GPIO_FUNC_NORMAL;
+ power-source = ;
+ input-disable;
+ output-low;
+ };
+ };
+
pm8916_gpios_leds: pm8916_gpios_leds {
pinconf {
pins = "gpio1", "gpio2";
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
index 4c3dda5..c619dea 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
@@ -221,9 +221,10 @@
adp-disable;
hnp-disable;
srp-disable;
- dr_mode = "host";
- pinctrl-names = "default";
- pinctrl-0 = <_sw_sel_pm>;
+ dr_mode = "otg";
+ pinctrl-names = "default", "device";
+ pinctrl-0 = <_sw_sel_pm _hub_reset_pm>;
+ pinctrl-1 = <_sw_sel_pm_device
_hub_reset_pm_device>;
ulpi {
phy {
v1p8-supply = <_l7>;
@@ -464,7 +465,7 @@
usb_id: usb-id {
compatible = "linux,extcon-usb-gpio";
- vbus-gpio = < 121 GPIO_ACTIVE_HIGH>;
+ id-gpio = < 121 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
pinctrl-0 = <_id_default>;
};
--
2.7.4
[PATCH v2 1/7] usb: chipidea: Add dynamic pinctrl selection
Some hardware implementations require to configure pins differently
according to the USB role (host/device), this can be an update of the
pins routing or a simple GPIO value change.
This patch introduces new optional "host" and "device" pinctrls.
If these pinctrls are defined by the device, they are respectively
selected on host/device role start.
If a default pinctrl exist, it is restored on host/device role stop.
Signed-off-by: Loic Poulain
---
v2: includes ordering
drivers/usb/chipidea/core.c | 19 +++
drivers/usb/chipidea/host.c | 9 +
drivers/usb/chipidea/udc.c | 9 +
include/linux/usb/chipidea.h | 6 ++
4 files changed, 43 insertions(+)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 43ea5fb..cdac778 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -56,6 +56,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -726,6 +727,24 @@ static int ci_get_platdata(struct device *dev,
else
cable->connected = false;
}
+
+ platdata->pctl = devm_pinctrl_get(dev);
+ if (!IS_ERR(platdata->pctl)) {
+ struct pinctrl_state *p;
+
+ p = pinctrl_lookup_state(platdata->pctl, "default");
+ if (!IS_ERR(p))
+ platdata->pins_default = p;
+
+ p = pinctrl_lookup_state(platdata->pctl, "host");
+ if (!IS_ERR(p))
+ platdata->pins_host = p;
+
+ p = pinctrl_lookup_state(platdata->pctl, "device");
+ if (!IS_ERR(p))
+ platdata->pins_device = p;
+ }
+
return 0;
}
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 18cb8e4..3729672 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include "../host/ehci.h"
@@ -164,6 +165,10 @@ static int host_start(struct ci_hdrc *ci)
}
}
+ if (ci->platdata->pins_host)
+ pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_host);
+
ret = usb_add_hcd(hcd, 0, 0);
if (ret) {
goto disable_reg;
@@ -208,6 +213,10 @@ static void host_stop(struct ci_hdrc *ci)
}
ci->hcd = NULL;
ci->otg.host = NULL;
+
+ if (ci->platdata->pins_host && ci->platdata->pins_default)
+ pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_default);
}
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index fe8a905..3f09a0d 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1964,6 +1965,10 @@ void ci_hdrc_gadget_destroy(struct ci_hdrc *ci)
static int udc_id_switch_for_device(struct ci_hdrc *ci)
{
+ if (ci->platdata->pins_device)
+ pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_device);
+
if (ci->is_otg)
/* Clear and enable BSV irq */
hw_write_otgsc(ci, OTGSC_BSVIS | OTGSC_BSVIE,
@@ -1982,6 +1987,10 @@ static void udc_id_switch_for_host(struct ci_hdrc *ci)
hw_write_otgsc(ci, OTGSC_BSVIE | OTGSC_BSVIS, OTGSC_BSVIS);
ci->vbus_active = 0;
+
+ if (ci->platdata->pins_device && ci->platdata->pins_default)
+ pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_default);
}
/**
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 07f9936..63758c3 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -77,6 +77,12 @@ struct ci_hdrc_platform_data {
struct ci_hdrc_cablevbus_extcon;
struct ci_hdrc_cableid_extcon;
u32 phy_clkgate_delay_us;
+
+ /* pins */
+ struct pinctrl *pctl;
+ struct pinctrl_state *pins_default;
+ struct pinctrl_state *pins_host;
+ struct pinctrl_state *pins_device;
};
/* Default offset of capability registers */
--
2.7.4
Re: [PATCH 1/6] usb: chipidea: Add dynamic pinctrl selection
On 23 August 2018 at 12:11, Andy Shevchenko wrote: > On Tue, Aug 21, 2018 at 4:57 PM Loic Poulain wrote: >> >> Some hardware implementations require to configure pins differently >> according to the USB role (host/device), this can be an update of the >> pins routing or a simple GPIO value change. >> >> This patch introduces new optional "host" and "device" pinctrls. >> If these pinctrls are defined by the device, they are respectively >> selected on host/device role start. >> >> If a default pinctrl exist, it is restored on host/device role stop. > >> #include >> #include >> #include >> +#include > > A nit: Even in this context it would be nice to keep *some* order. > Ok. >> #include >> #include >> #include > >> + p = pinctrl_lookup_state(platdata->pctl, "default"); >> + p = pinctrl_lookup_state(platdata->pctl, "host"); >> + p = pinctrl_lookup_state(platdata->pctl, "device"); > > I'm wondering if we have any rules applied to these names. I suppose i have document this in the chipidea DT binding doc. > >> #include >> #include >> #include >> +#include > > Ditto about ordering. > >> + if (ci->platdata->pins_host) >> + pinctrl_select_state(ci->platdata->pctl, >> +ci->platdata->pins_host); > > Hmm... Do we need to have a condition above? > Yes, since these pinctrls are optional and can be null. >> + if (ci->platdata->pins_host && ci->platdata->pins_default) >> + pinctrl_select_state(ci->platdata->pctl, >> +ci->platdata->pins_default); > > Ditto about conditional. > >> #include >> #include >> #include >> +#include > > Ditto about ordering. > >> + if (ci->platdata->pins_device) >> + pinctrl_select_state(ci->platdata->pctl, >> +ci->platdata->pins_device); > > >> + if (ci->platdata->pins_device && ci->platdata->pins_default) >> + pinctrl_select_state(ci->platdata->pctl, >> +ci->platdata->pins_default); > > Ditto about conditional. > > -- > With Best Regards, > Andy Shevchenko Regards, Loic
Re: [PATCH 5/6] phy: qcom-usb-hs: Fix unbalanced notifier registration
On 23 August 2018 at 19:27, Bjorn Andersson wrote:
> On Tue 21 Aug 06:55 PDT 2018, Loic Poulain wrote:
>
>> Phy power on/off cycle can happen several times during device life.
>> We then need to balance the extcon notifier registration accordingly.
>>
>> Fixes: f0b5c2c96370 ("phy: qcom-usb-hs: Replace the extcon API")
>> Signed-off-by: Loic Poulain
>> ---
>> drivers/phy/qualcomm/phy-qcom-usb-hs.c | 6 ++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
>> b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
>> index 2d0c70b..92e9d94 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
>> @@ -181,6 +181,12 @@ static int qcom_usb_hs_phy_power_off(struct phy *phy)
>> {
>> struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
>>
>> + if (uphy->vbus_edev) {
>> + devm_extcon_unregister_notifier(>ulpi->dev,
>> + uphy->vbus_edev, EXTCON_USB,
>> + >vbus_notify);
>
> As I presume the power_on should always be matched with a power_off I
> wonder if it really is appropriate to use the devres version of this
> API.
>
> Should we drop the devm_ from registration in power_on as well?
Yes, thanks, I'll drop the devres version.
Regards,
Loic
Re: [PATCH 1/6] usb: chipidea: Add dynamic pinctrl selection
Hi Bjorn, On 23 August 2018 at 16:53, Bjorn Andersson wrote: > On Tue 21 Aug 06:55 PDT 2018, Loic Poulain wrote: > >> Some hardware implementations require to configure pins differently >> according to the USB role (host/device), this can be an update of the >> pins routing or a simple GPIO value change. >> >> This patch introduces new optional "host" and "device" pinctrls. >> If these pinctrls are defined by the device, they are respectively >> selected on host/device role start. >> >> If a default pinctrl exist, it is restored on host/device role stop. >> > > The implementation looks reasonable, but we're actually just toggling a > gpio using pinctrl states. Do you see any reason not to control this mux > through the gpio api? > Actually, two gpios (including hub reset one), but you're right. Point is that these gpios are very specific to the Dragonboard layout and not related to USB controller itself (external mux), so I'm not sure it makes sense to control a 'mux' GPIO from the chipidea driver. That's why I used pinctrl which is more generic and hides the underlying operations (a simple GPIO toggling in our case). But let me know if there is a better way. Regards, Loic
Re: [PATCH 6/6] arm: dts: qcom: db410c: Enable USB OTG support
On 21 August 2018 at 18:09, Rob Herring wrote:
> On Tue, Aug 21, 2018 at 8:56 AM Loic Poulain wrote:
>>
>> The Dragonboard-410c is able to act either as USB Host or Device.
>> The role can be determined at runtime via the USB_HS_ID pin which is
>> derived from the micro-usb port VBUS pin.
>>
>> In Host role, SoC USB D+/D- are routed to the onboard USB 2.0 HUB.
>> In Device role, SoC USB D+/D- are routed to the USB 2.0 micro B port.
>> Routing is selected via USB_SW_SEL_PM gpio.
>>
>> In device role USB HUB can be held in reset.
>>
>> Signed-off-by: Loic Poulain
>> ---
>
>> @@ -512,7 +513,7 @@
>>
>> usb_id: usb-id {
>> compatible = "linux,extcon-usb-gpio";
>> - vbus-gpio = < 121 GPIO_ACTIVE_HIGH>;
>> + id-gpio = < 121 GPIO_ACTIVE_HIGH>;
>
> The GPIO has magically changed from being connected to Vbus to ID? The
> extcon binding is crap anyways...
Although the pin is derived from VBUS, it is really used as an ID pin (USB_HS_ID
on DB410C schematics [1]), so that DB410C switches to device role when a cable
is inserted to the micro B port.
> Ideally, it would be nice if this was moved to the usb connector binding.
Good suggestion, If you agree, I'll address that in a different patchset.
Regards,
Loic
[1]
https://www.96boards.org/documentation/consumer/dragonboard/dragonboard410c/hardware-docs/
[PATCH 6/6] arm: dts: qcom: db410c: Enable USB OTG support
The Dragonboard-410c is able to act either as USB Host or Device.
The role can be determined at runtime via the USB_HS_ID pin which is
derived from the micro-usb port VBUS pin.
In Host role, SoC USB D+/D- are routed to the onboard USB 2.0 HUB.
In Device role, SoC USB D+/D- are routed to the USB 2.0 micro B port.
Routing is selected via USB_SW_SEL_PM gpio.
In device role USB HUB can be held in reset.
Signed-off-by: Loic Poulain
---
arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi | 20
arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 9 +
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
b/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
index ec2f0de..99787cc 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
@@ -8,6 +8,16 @@
pinconf {
pins = "gpio3";
function = PMIC_GPIO_FUNC_NORMAL;
+ input-disable;
+ output-high;
+ };
+ };
+
+ usb_hub_reset_pm_device: usb_hub_reset_pm_device {
+ pinconf {
+ pins = "gpio3";
+ function = PMIC_GPIO_FUNC_NORMAL;
+ input-disable;
output-low;
};
};
@@ -22,6 +32,16 @@
};
};
+ usb_sw_sel_pm_device: usb_sw_sel_pm_device {
+ pinconf {
+ pins = "gpio4";
+ function = PMIC_GPIO_FUNC_NORMAL;
+ power-source = ;
+ input-disable;
+ output-low;
+ };
+ };
+
pm8916_gpios_leds: pm8916_gpios_leds {
pinconf {
pins = "gpio1", "gpio2";
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
index 9ff8487..661a7fd 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
@@ -371,9 +371,10 @@
adp-disable;
hnp-disable;
srp-disable;
- dr_mode = "host";
- pinctrl-names = "default";
- pinctrl-0 = <_sw_sel_pm>;
+ dr_mode = "otg";
+ pinctrl-names = "default", "device";
+ pinctrl-0 = <_sw_sel_pm _hub_reset_pm>;
+ pinctrl-1 = <_sw_sel_pm_device
_hub_reset_pm_device>;
ulpi {
phy {
v1p8-supply = <_l7>;
@@ -512,7 +513,7 @@
usb_id: usb-id {
compatible = "linux,extcon-usb-gpio";
- vbus-gpio = < 121 GPIO_ACTIVE_HIGH>;
+ id-gpio = < 121 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
pinctrl-0 = <_id_default>;
};
--
2.7.4
[PATCH 2/6] usb: chipidea: Support generic usb extcon
Add compatibility for extcon-usb-gpio which can handle more
than one cable per instance, allowing coherency of USB cable
states (USB/USB-HOST). These states can be generated from ID
or/and VBUS pins.
In case only one extcon device is associated to the USB device,
and this device supports USB and USB-HOST cable states, we now
use it for both VBUS (USB) and ID (USB-HOST) notifier.
Signed-off-by: Loic Poulain
---
drivers/usb/chipidea/core.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 03e52fc..c595718 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -699,6 +699,17 @@ static int ci_get_platdata(struct device *dev,
ext_id = extcon_get_edev_by_phandle(dev, 1);
if (IS_ERR(ext_id) && PTR_ERR(ext_id) != -ENODEV)
return PTR_ERR(ext_id);
+
+ /*
+* Some extcon devices like extcon-usb-gpio have only one
+* instance for both USB and USB-HOST cable states.
+*/
+ if (!IS_ERR(ext_vbus) && IS_ERR(ext_id)) {
+ if (extcon_get_state(ext_vbus, EXTCON_USB) >= 0 &&
+ extcon_get_state(ext_vbus, EXTCON_USB_HOST) >= 0) {
+ ext_id = ext_vbus;
+ }
+ }
}
cable = >vbus_extcon;
--
2.7.4
[PATCH 1/6] usb: chipidea: Add dynamic pinctrl selection
Some hardware implementations require to configure pins differently
according to the USB role (host/device), this can be an update of the
pins routing or a simple GPIO value change.
This patch introduces new optional "host" and "device" pinctrls.
If these pinctrls are defined by the device, they are respectively
selected on host/device role start.
If a default pinctrl exist, it is restored on host/device role stop.
Signed-off-by: Loic Poulain
---
drivers/usb/chipidea/core.c | 19 +++
drivers/usb/chipidea/host.c | 9 +
drivers/usb/chipidea/udc.c | 9 +
include/linux/usb/chipidea.h | 6 ++
4 files changed, 43 insertions(+)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 85fc6db..03e52fc 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -46,6 +46,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -723,6 +724,24 @@ static int ci_get_platdata(struct device *dev,
else
cable->connected = false;
}
+
+ platdata->pctl = devm_pinctrl_get(dev);
+ if (!IS_ERR(platdata->pctl)) {
+ struct pinctrl_state *p;
+
+ p = pinctrl_lookup_state(platdata->pctl, "default");
+ if (!IS_ERR(p))
+ platdata->pins_default = p;
+
+ p = pinctrl_lookup_state(platdata->pctl, "host");
+ if (!IS_ERR(p))
+ platdata->pins_host = p;
+
+ p = pinctrl_lookup_state(platdata->pctl, "device");
+ if (!IS_ERR(p))
+ platdata->pins_device = p;
+ }
+
return 0;
}
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index af45aa32..55dbd49 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include "../host/ehci.h"
@@ -150,6 +151,10 @@ static int host_start(struct ci_hdrc *ci)
}
}
+ if (ci->platdata->pins_host)
+ pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_host);
+
ret = usb_add_hcd(hcd, 0, 0);
if (ret) {
goto disable_reg;
@@ -194,6 +199,10 @@ static void host_stop(struct ci_hdrc *ci)
}
ci->hcd = NULL;
ci->otg.host = NULL;
+
+ if (ci->platdata->pins_host && ci->platdata->pins_default)
+ pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_default);
}
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 9852ec5..c04384e 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include "ci.h"
#include "udc.h"
@@ -1965,6 +1966,10 @@ void ci_hdrc_gadget_destroy(struct ci_hdrc *ci)
static int udc_id_switch_for_device(struct ci_hdrc *ci)
{
+ if (ci->platdata->pins_device)
+ pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_device);
+
if (ci->is_otg)
/* Clear and enable BSV irq */
hw_write_otgsc(ci, OTGSC_BSVIS | OTGSC_BSVIE,
@@ -1983,6 +1988,10 @@ static void udc_id_switch_for_host(struct ci_hdrc *ci)
hw_write_otgsc(ci, OTGSC_BSVIE | OTGSC_BSVIS, OTGSC_BSVIS);
ci->vbus_active = 0;
+
+ if (ci->platdata->pins_device && ci->platdata->pins_default)
+ pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_default);
}
/**
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 07f9936..63758c3 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -77,6 +77,12 @@ struct ci_hdrc_platform_data {
struct ci_hdrc_cablevbus_extcon;
struct ci_hdrc_cableid_extcon;
u32 phy_clkgate_delay_us;
+
+ /* pins */
+ struct pinctrl *pctl;
+ struct pinctrl_state *pins_default;
+ struct pinctrl_state *pins_host;
+ struct pinctrl_state *pins_device;
};
/* Default offset of capability registers */
--
2.7.4
[PATCH 5/6] phy: qcom-usb-hs: Fix unbalanced notifier registration
Phy power on/off cycle can happen several times during device life.
We then need to balance the extcon notifier registration accordingly.
Fixes: f0b5c2c96370 ("phy: qcom-usb-hs: Replace the extcon API")
Signed-off-by: Loic Poulain
---
drivers/phy/qualcomm/phy-qcom-usb-hs.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
index 2d0c70b..92e9d94 100644
--- a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
+++ b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
@@ -181,6 +181,12 @@ static int qcom_usb_hs_phy_power_off(struct phy *phy)
{
struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
+ if (uphy->vbus_edev) {
+ devm_extcon_unregister_notifier(>ulpi->dev,
+ uphy->vbus_edev, EXTCON_USB,
+ >vbus_notify);
+ }
+
regulator_disable(uphy->v3p3);
regulator_disable(uphy->v1p8);
clk_disable_unprepare(uphy->sleep_clk);
--
2.7.4
[PATCH 3/6] usb: chipidea: Prevent unbalanced IRQ disable
The ChipIdea IRQ is disabled before scheduling the otg work and
re-enabled on otg work completion. However if the job is already
scheduled we have to undo the effect of disable_irq int order to
balance the IRQ disable-depth value.
Fixes: be6b0c1bd0be ("usb: chipidea: using one inline function to cover queue
work operations")
Signed-off-by: Loic Poulain
---
drivers/usb/chipidea/otg.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
index 7e7428e..4f8b817 100644
--- a/drivers/usb/chipidea/otg.h
+++ b/drivers/usb/chipidea/otg.h
@@ -17,7 +17,8 @@ void ci_handle_vbus_change(struct ci_hdrc *ci);
static inline void ci_otg_queue_work(struct ci_hdrc *ci)
{
disable_irq_nosync(ci->irq);
- queue_work(ci->wq, >work);
+ if (queue_work(ci->wq, >work) == false)
+ enable_irq(ci->irq);
}
#endif /* __DRIVERS_USB_CHIPIDEA_OTG_H */
--
2.7.4
[PATCH 4/6] usb: chipidea: Fix otg event handler
At OTG work running time, it's possible that several events need to be
addressed (e.g. ID and VBUS events). The current implementation handles
only one event at a time which leads to ignoring the other one. Fix it.
Signed-off-by: Loic Poulain
---
drivers/usb/chipidea/otg.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index db4ceff..f25d482 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -203,14 +203,17 @@ static void ci_otg_work(struct work_struct *work)
}
pm_runtime_get_sync(ci->dev);
+
if (ci->id_event) {
ci->id_event = false;
ci_handle_id_switch(ci);
- } else if (ci->b_sess_valid_event) {
+ }
+
+ if (ci->b_sess_valid_event) {
ci->b_sess_valid_event = false;
ci_handle_vbus_change(ci);
- } else
- dev_err(ci->dev, "unexpected event occurs at %s\n", __func__);
+ }
+
pm_runtime_put_sync(ci->dev);
enable_irq(ci->irq);
--
2.7.4
Re: [PATCH v3] USB: serial: ftdi_sio: Add support for CBUS GPIO
Hi Johan, On 4 August 2018 at 12:17, Loic Poulain wrote: > Some FTDI devices like FTX or FT232R support CBUS Bit Bang mode on CBUS > pins, allowing host to control them via simple USB control transfers. > To make use of a CBUS pin in Bit Bang mode, the pin must be configured > to I/O mode in the FTDI EEPROM. This mode perfectly coexists with regular > USB to Serial function. > > In this implementation, a GPIO controller is registered on FTDI probe > if at least one CBUS pin is configured for I/O mode. For now, only > FTX devices are supported. > > This patch is based on previous Stefan Agner implementation tentative > on LKML [1]. > > [1] Message-Id: 1434838377-8042-1-git-send-email-ste...@agner.ch > > Signed-off-by: Loic Poulain Any comments on this v3? Regards, Loic
[PATCH v3] USB: serial: ftdi_sio: Add support for CBUS GPIO
Some FTDI devices like FTX or FT232R support CBUS Bit Bang mode on CBUS
pins, allowing host to control them via simple USB control transfers.
To make use of a CBUS pin in Bit Bang mode, the pin must be configured
to I/O mode in the FTDI EEPROM. This mode perfectly coexists with regular
USB to Serial function.
In this implementation, a GPIO controller is registered on FTDI probe
if at least one CBUS pin is configured for I/O mode. For now, only
FTX devices are supported.
This patch is based on previous Stefan Agner implementation tentative
on LKML [1].
[1] Message-Id: 1434838377-8042-1-git-send-email-ste...@agner.ch
Signed-off-by: Loic Poulain
---
v2: Use message-id for LKML reference
Rework read_eeprom according to Andy's comment and return read count
Remove noisy messages
Comment style alignment
Add defines for magic values
Cannot use devm, because gpiochip is linked to the port not to the udev
v3:
Fix typo in CBUS mask description comment
drivers/usb/serial/Kconfig| 9 ++
drivers/usb/serial/ftdi_sio.c | 238 ++
drivers/usb/serial/ftdi_sio.h | 83 +++
3 files changed, 330 insertions(+)
diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
index 533f127..64c9f2e 100644
--- a/drivers/usb/serial/Kconfig
+++ b/drivers/usb/serial/Kconfig
@@ -181,6 +181,15 @@ config USB_SERIAL_FTDI_SIO
To compile this driver as a module, choose M here: the
module will be called ftdi_sio.
+config USB_SERIAL_FTDI_SIO_CBUS_GPIO
+ bool "USB FDTI CBUS GPIO support"
+ depends on USB_SERIAL_FTDI_SIO
+ depends on GPIOLIB
+ help
+ Say yes here to add support for the CBUS bit-bang mode, allowing CBUS
+ pins to act as GPIOs. Note that pins must first be configured for GPIO
+ in the device's EEPROM. The FT232R and FT-X series support this mode.
+
config USB_SERIAL_VISOR
tristate "USB Handspring Visor / Palm m50x / Sony Clie Driver"
help
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index b5cef32..e90cae6 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -40,12 +41,21 @@
#include
#include
#include
+
#include "ftdi_sio.h"
#include "ftdi_sio_ids.h"
#define DRIVER_AUTHOR "Greg Kroah-Hartman , Bill Ryder
, Kuba Ober , Andreas Mohr, Johan Hovold
"
#define DRIVER_DESC "USB FTDI Serial Converters Driver"
+#define FTDI_CBUS_PIN_COUNT 4
+
+struct ftdi_gpiochip {
+ struct gpio_chip gc;
+ struct usb_serial_port *port;
+ unsigned int cbus_map[FTDI_CBUS_PIN_COUNT];
+ unsigned long cbus_mask;
+};
struct ftdi_private {
enum ftdi_chip_type chip_type;
@@ -72,6 +82,8 @@ struct ftdi_private {
unsigned int latency; /* latency setting in use */
unsigned short max_packet_size;
struct mutex cfg_lock; /* Avoid mess by parallel calls of config
ioctl() and change_speed() */
+
+ struct ftdi_gpiochip *fgc;
};
/* struct ftdi_sio_quirk is used by devices requiring special attention. */
@@ -1528,6 +1540,224 @@ static int get_lsr_info(struct usb_serial_port *port,
return 0;
}
+#ifdef CONFIG_USB_SERIAL_FTDI_SIO_CBUS_GPIO
+
+#define FTX_CBUS_MUX_EEPROM_ADDR 0x1a
+
+static int ftdi_read_eeprom(struct usb_device *udev, unsigned int off,
+ void *val, size_t bytes)
+{
+ unsigned int read = 0;
+
+ if (bytes % 2) /* 16-bit word eeprom */
+ bytes--;
+
+ while (read < bytes) {
+ int rv;
+
+ rv = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+FTDI_SIO_READ_EEPROM_REQUEST,
+FTDI_SIO_READ_EEPROM_REQUEST_TYPE,
+0, (off + read) / 2, val + read, 2,
+WDR_TIMEOUT);
+ if (rv < 0)
+ break;
+
+ read += 2;
+ }
+
+ return read;
+}
+
+static int ftdi_set_bitmode(struct usb_serial_port *port, u8 bitmask,
+ u8 bitmode)
+{
+ struct ftdi_private *priv = usb_get_serial_port_data(port);
+ __u16 urb_value = 0;
+
+ urb_value = bitmode << 8 | bitmask;
+
+ if (usb_control_msg(port->serial->dev,
+ usb_sndctrlpipe(port->serial->dev, 0),
+ FTDI_SIO_SET_BITMODE_REQUEST,
+ FTDI_SIO_SET_BITMODE_REQUEST_TYPE,
+ urb_value, priv->interface, NULL, 0,
+ WDR_SHORT_TIMEOUT) < 0) {
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int ftdi_read_pins(struct usb_serial_port *port, u8 *val)
+{
+
[PATCH v2] USB: serial: ftdi_sio: Add support for CBUS GPIO
Some FTDI devices like FTX or FT232R support CBUS Bit Bang mode on CBUS
pins, allowing host to control them via simple USB control transfers.
To make use of a CBUS pin in Bit Bang mode, the pin must be configured
to I/O mode in the FTDI EEPROM. This mode perfectly coexists with regular
USB to Serial function.
In this implementation, a GPIO controller is registered on FTDI probe
if at least one CBUS pin is configured for I/O mode. For now, only
FTX devices are supported.
This patch is based on previous Stefan Agner implementation tentative
on LKML [1].
[1] Message-Id: 1434838377-8042-1-git-send-email-ste...@agner.ch
Signed-off-by: Loic Poulain
---
v2: Use message-id for LKML reference
Rework read_eeprom according to Andy's comment and return read count
Remove noisy messages
Comment style alignment
Add defines for magic values
Cannot use devm, because gpiochip is linked to the port not to the udev
drivers/usb/serial/Kconfig| 9 ++
drivers/usb/serial/ftdi_sio.c | 238 ++
drivers/usb/serial/ftdi_sio.h | 83 +++
3 files changed, 330 insertions(+)
diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
index 533f127..64c9f2e 100644
--- a/drivers/usb/serial/Kconfig
+++ b/drivers/usb/serial/Kconfig
@@ -181,6 +181,15 @@ config USB_SERIAL_FTDI_SIO
To compile this driver as a module, choose M here: the
module will be called ftdi_sio.
+config USB_SERIAL_FTDI_SIO_CBUS_GPIO
+ bool "USB FDTI CBUS GPIO support"
+ depends on USB_SERIAL_FTDI_SIO
+ depends on GPIOLIB
+ help
+ Say yes here to add support for the CBUS bit-bang mode, allowing CBUS
+ pins to act as GPIOs. Note that pins must first be configured for GPIO
+ in the device's EEPROM. The FT232R and FT-X series support this mode.
+
config USB_SERIAL_VISOR
tristate "USB Handspring Visor / Palm m50x / Sony Clie Driver"
help
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index b5cef32..8592f39 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -40,12 +41,21 @@
#include
#include
#include
+
#include "ftdi_sio.h"
#include "ftdi_sio_ids.h"
#define DRIVER_AUTHOR "Greg Kroah-Hartman , Bill Ryder
, Kuba Ober , Andreas Mohr, Johan Hovold
"
#define DRIVER_DESC "USB FTDI Serial Converters Driver"
+#define FTDI_CBUS_PIN_COUNT 4
+
+struct ftdi_gpiochip {
+ struct gpio_chip gc;
+ struct usb_serial_port *port;
+ unsigned int cbus_map[FTDI_CBUS_PIN_COUNT];
+ unsigned long cbus_mask;
+};
struct ftdi_private {
enum ftdi_chip_type chip_type;
@@ -72,6 +82,8 @@ struct ftdi_private {
unsigned int latency; /* latency setting in use */
unsigned short max_packet_size;
struct mutex cfg_lock; /* Avoid mess by parallel calls of config
ioctl() and change_speed() */
+
+ struct ftdi_gpiochip *fgc;
};
/* struct ftdi_sio_quirk is used by devices requiring special attention. */
@@ -1528,6 +1540,224 @@ static int get_lsr_info(struct usb_serial_port *port,
return 0;
}
+#ifdef CONFIG_USB_SERIAL_FTDI_SIO_CBUS_GPIO
+
+#define FTX_CBUS_MUX_EEPROM_ADDR 0x1a
+
+static int ftdi_read_eeprom(struct usb_device *udev, unsigned int off,
+ void *val, size_t bytes)
+{
+ unsigned int read = 0;
+
+ if (bytes % 2) /* 16-bit word eeprom */
+ bytes--;
+
+ while (read < bytes) {
+ int rv;
+
+ rv = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+FTDI_SIO_READ_EEPROM_REQUEST,
+FTDI_SIO_READ_EEPROM_REQUEST_TYPE,
+0, (off + read) / 2, val + read, 2,
+WDR_TIMEOUT);
+ if (rv < 0)
+ break;
+
+ read += 2;
+ }
+
+ return read;
+}
+
+static int ftdi_set_bitmode(struct usb_serial_port *port, u8 bitmask,
+ u8 bitmode)
+{
+ struct ftdi_private *priv = usb_get_serial_port_data(port);
+ __u16 urb_value = 0;
+
+ urb_value = bitmode << 8 | bitmask;
+
+ if (usb_control_msg(port->serial->dev,
+ usb_sndctrlpipe(port->serial->dev, 0),
+ FTDI_SIO_SET_BITMODE_REQUEST,
+ FTDI_SIO_SET_BITMODE_REQUEST_TYPE,
+ urb_value, priv->interface, NULL, 0,
+ WDR_SHORT_TIMEOUT) < 0) {
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int ftdi_read_pins(struct usb_serial_port *port, u8 *val)
+{
+ struct ftdi_private *priv = usb_get
Re: [PATCH] USB: serial: ftdi_sio: Add support for CBUS GPIO
Thanks Andy,
On 1 August 2018 at 18:08, Andy Shevchenko wrote:
> On Wed, Aug 1, 2018 at 6:46 PM, Loic Poulain wrote:
>> Some FTDI devices like FTX or FT232R support CBUS Bit Bang mode on CBUS
>> pins, allowing host to control them via simple USB control transfers.
>> To make use of a CBUS pin in Bit Bang mode, the pin must be configured
>> to I/O mode in the FTDI EEPROM. This mode perfectly coexists with regular
>> USB to Serial function.
>>
>> In this implementation, a GPIO controller is registered on FTDI probe
>> if at least one CBUS pin is configured for I/O mode. For now, only
>> FTX devices are supported.
>>
>
>> This patch is based on previous Stefan Agner implementation tentative
>> on LKML ([PATCH 0/2] FTDI CBUS GPIO support).
>
> The best approach to refer to a mail is to put a message-id, something like
>
> (... [1].)
>
> [1]: Message-Id: <...message-id...>
>
>> +static int ftdi_read_eeprom(struct usb_device *udev, unsigned int off,
>> + void *val, size_t bytes)
>> +{
>
>> + if (bytes % 2) /* 16-bit eeprom */
>> + return -EINVAL;
>
> Why is it fatal?
> You may read one byte less (and provide an error code like -EIO, or
> whatever fits better) or more (and provide exact amount asked).
Yes, indeed.
>
>> + return 0;
>> +}
>
>> + rv = ftdi_read_pins(fgc->port, );
>> + if (rv)
>> + dev_err(>dev, "Unable to read CBUS GPIO pins, %d\n",
>> rv);
>
> Why not to return an error?
>
> (Message itself sounds like a noise. Perhaps, dev_dbg() or throw away.
Will do.
>
>> + rv = ftdi_set_bitmode(fgc->port, fgc->cbus_mask,
>> FTDI_SIO_BITMODE_CBUS);
>> + if (rv)
>> + dev_err(>dev, "Unable set CBUS Bit-Bang mode, %d\n",
>> rv);
>
> Ditto WRT message.
yes
>
>> +static int ftdi_register_cbus_gpiochip(struct usb_serial_port *port)
>> +{
>> + struct ftdi_private *priv = usb_get_serial_port_data(port);
>> + struct usb_device *udev = port->serial->dev;
>> + struct ftdi_gpiochip *fgc;
>> + int rv;
>> +
>
>> + fgc = kzalloc(sizeof(*fgc), GFP_KERNEL);
>> + if (!fgc)
>> + return -ENOMEM;
>
> devm_ ?
>
>> + cbus_mux = kmalloc(4, GFP_KERNEL);
>> + if (!cbus_mux)
>> + return -ENOMEM;
>
> Is it mandatory to alloc so small amount on heap in this case?
Yes, because it is used as USB transfer buffer (DMA...) which can not
be on the stack.
>
>> + /* CBUS pins are individually configurable via 8-bit MUX
>> control
>> +* value, living at 0x1a for CBUS0. cf application note
>> AN_201.
>> +*/
>
> Is it a comment style used in this file?
>
>> + rv = ftdi_read_eeprom(udev, 0x1a, cbus_mux, 4);
>
> 0x1a is a magic.
>
>> + rv = gpiochip_add_data(>gc, fgc);
>> + if (rv) {
>> + dev_err(>dev, "Unable to add gpiochip\n");
>> + kfree(fgc);
>> + return rv;
>> + }
>
> devm_ ?
>
>> + return 0;
>> +}
>> +
>> +static void ftdi_unregister_cbus_gpiochip(struct usb_serial_port *port)
>> +{
>> + struct ftdi_private *priv = usb_get_serial_port_data(port);
>> + struct ftdi_gpiochip *fgc = priv->fgc;
>> +
>
>> + if (fgc) {
>
> How you can be here when fgc == NULL?!
>
>> + gpiochip_remove(>gc);
>> + kfree(fgc);
>> + }
>> +}
>
> I guess complete function will go away when you switch to devm.
>
>
> --
> With Best Regards,
> Andy Shevchenko
Regards,
Loic
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: serial: ftdi_sio: Add support for CBUS GPIO
Some FTDI devices like FTX or FT232R support CBUS Bit Bang mode on CBUS
pins, allowing host to control them via simple USB control transfers.
To make use of a CBUS pin in Bit Bang mode, the pin must be configured
to I/O mode in the FTDI EEPROM. This mode perfectly coexists with regular
USB to Serial function.
In this implementation, a GPIO controller is registered on FTDI probe
if at least one CBUS pin is configured for I/O mode. For now, only
FTX devices are supported.
This patch is based on previous Stefan Agner implementation tentative
on LKML ([PATCH 0/2] FTDI CBUS GPIO support).
Signed-off-by: Loic Poulain
---
drivers/usb/serial/Kconfig| 9 ++
drivers/usb/serial/ftdi_sio.c | 222 ++
drivers/usb/serial/ftdi_sio.h | 83
3 files changed, 314 insertions(+)
diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
index 533f127..64c9f2e 100644
--- a/drivers/usb/serial/Kconfig
+++ b/drivers/usb/serial/Kconfig
@@ -181,6 +181,15 @@ config USB_SERIAL_FTDI_SIO
To compile this driver as a module, choose M here: the
module will be called ftdi_sio.
+config USB_SERIAL_FTDI_SIO_CBUS_GPIO
+ bool "USB FDTI CBUS GPIO support"
+ depends on USB_SERIAL_FTDI_SIO
+ depends on GPIOLIB
+ help
+ Say yes here to add support for the CBUS bit-bang mode, allowing CBUS
+ pins to act as GPIOs. Note that pins must first be configured for GPIO
+ in the device's EEPROM. The FT232R and FT-X series support this mode.
+
config USB_SERIAL_VISOR
tristate "USB Handspring Visor / Palm m50x / Sony Clie Driver"
help
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index b5cef32..3cfb5fd 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -40,12 +41,19 @@
#include
#include
#include
+
#include "ftdi_sio.h"
#include "ftdi_sio_ids.h"
#define DRIVER_AUTHOR "Greg Kroah-Hartman , Bill Ryder
, Kuba Ober , Andreas Mohr, Johan Hovold
"
#define DRIVER_DESC "USB FTDI Serial Converters Driver"
+struct ftdi_gpiochip {
+ struct gpio_chip gc;
+ unsigned int cbus_map[4];
+ unsigned long cbus_mask;
+ struct usb_serial_port *port;
+};
struct ftdi_private {
enum ftdi_chip_type chip_type;
@@ -72,6 +80,8 @@ struct ftdi_private {
unsigned int latency; /* latency setting in use */
unsigned short max_packet_size;
struct mutex cfg_lock; /* Avoid mess by parallel calls of config
ioctl() and change_speed() */
+
+ struct ftdi_gpiochip *fgc;
};
/* struct ftdi_sio_quirk is used by devices requiring special attention. */
@@ -1528,6 +1538,211 @@ static int get_lsr_info(struct usb_serial_port *port,
return 0;
}
+#ifdef CONFIG_USB_SERIAL_FTDI_SIO_CBUS_GPIO
+
+static int ftdi_read_eeprom(struct usb_device *udev, unsigned int off,
+ void *val, size_t bytes)
+{
+ if (bytes % 2) /* 16-bit eeprom */
+ return -EINVAL;
+
+ while (bytes) {
+ int rv;
+
+ rv = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+FTDI_SIO_READ_EEPROM_REQUEST,
+FTDI_SIO_READ_EEPROM_REQUEST_TYPE,
+0, off / 2, val, 2, WDR_TIMEOUT);
+ if (rv < 0)
+ return rv;
+
+ off += 2;
+ val += 2;
+ bytes -= 2;
+ }
+
+ return 0;
+}
+
+static int ftdi_set_bitmode(struct usb_serial_port *port, u8 bitmask,
+ u8 bitmode)
+{
+ struct ftdi_private *priv = usb_get_serial_port_data(port);
+ __u16 urb_value = 0;
+
+ urb_value = bitmode << 8 | bitmask;
+
+ if (usb_control_msg(port->serial->dev,
+usb_sndctrlpipe(port->serial->dev, 0),
+FTDI_SIO_SET_BITMODE_REQUEST,
+FTDI_SIO_SET_BITMODE_REQUEST_TYPE,
+urb_value, priv->interface,
+NULL, 0, WDR_SHORT_TIMEOUT) < 0) {
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int ftdi_read_pins(struct usb_serial_port *port, u8 *val)
+{
+ struct ftdi_private *priv = usb_get_serial_port_data(port);
+ unsigned char *buf;
+
+ buf = kmalloc(1, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ if (usb_control_msg(port->serial->dev,
+usb_rcvctrlpipe(port->serial->dev, 0),
+FTDI_SIO_READ_PINS_REQUEST,
+FTDI_SIO_READ_PINS_REQUEST_TYPE,
+0, p
Re: [PATCH v6] USB: serial: ftdi_sio: Add MTP NVM support
Hi Johan, Thanks for the review. On 10 July 2018 at 11:19, Johan Hovold wrote: > Hi, > > I finally found some time to dig into the ftdi eeprom handling. > > On Tue, Jun 26, 2018 at 02:54:48PM +0200, Loic Poulain wrote: >> Most of FTDI's devices have an EEPROM which records FTDI devices >> configuration setting (e.g. the VID, PID, I/O config...) and user >> data. For example, FT232R and FTX chips have 128-byte and 2048-byte >> internal EEPROM respectively. > > The "internal" qualifier here is crucial; only FT232R and FTX have > internal EEPROMs of a known size. Other device types use external > EEPROMs of varying sizes, and which need not even be populated. > > And judging from the heuristics used by libftdi, there's no good (known) > way to determine the size of the external EEPROMs, which means that this > cannot be generalised. Note that handling external EEPROMS also implies > dealing with explicit EEPROM erase commands, which does not seem to fit > the proposed interface. > >> This patch adds support for FTDI EEPROM read/write via USB control >> transfers and register a new nvm device to the nvmem core. >> >> This permits to expose the EEPROM as a sysfs file, allowing userspace >> to read/modify FTDI configuration and its user data without having to >> rely on a specific userspace USB driver. > > So I have doubts about the usefulness of all of this. > > - You cannot just write the EEPROM from user space without knowing the > device-specific layout. > > - You also need to make sure to recalculate and update the checksum > stored in the final word. Correct, at least for the 'non-user', area. That means having a userspace tool to manage this specific layout.This is not so complex thought. > > - And for that you obviously need to know the EEPROM size (known for R > and FTX). > > So in the end you still need something like libftdi to be able to do > anything useful with this. Also note that the layout and protocol for > handling the EEPROM is only available under NDA or is based on > reverse-engineering guess work. For example, it seems like for FT232R > you need to set a specific latency timer to be able to write the EEPROM > (reliably). And so on. Yes, there is a bit more public information for FT-X, the application note AN_201 describes the MTP memory map. > > And as the consequences of getting this wrong may result in a bricked > device, I'm even more sceptical about this. > >> Moreover, any upcoming new tentative to add CBUS GPIO support could >> integrate CBUS EEPROM configuration reading in order to determine >> which of the CBUS pins are available as GPIO. > > The CBUS configuration is stored in just a couple of words at a known > offset and could easily be retrieved without depending on the nvmem > subsystem when needed. > > As I assume fiddling with the EEPROM should be rare (e.g. done during > development or by hobbyists) and would still depend user-space tools > (e.g. for layouts, checksums and external EEPROMs), I think we should > just leave it all to user space to deal with. I'm fine with that, to be honest I'm interested in adding GPIO control but wanted to progress step by step, EEPROM access being the first one. I thought it could have been valuable to expose it via nvmem, but like you said, we can keep this internal for now. So, I'll come back with new patche(s) for CBUS GPIOs. Regards, Loic -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] USB: serial: ftdi_sio: Add MTP NVM support
Hi Johan, On 29 June 2018 at 21:52, Ajay Gupta wrote: > Hi Loic. > Thanks for fixing comments. Looks good to me now. > > On 6/26/18, Loic Poulain wrote: >> Most of FTDI's devices have an EEPROM which records FTDI devices >> configuration setting (e.g. the VID, PID, I/O config...) and user >> data. For example, FT232R and FTX chips have 128-byte and 2048-byte >> internal EEPROM respectively. >> >> This patch adds support for FTDI EEPROM read/write via USB control >> transfers and register a new nvm device to the nvmem core. >> >> This permits to expose the EEPROM as a sysfs file, allowing userspace >> to read/modify FTDI configuration and its user data without having to >> rely on a specific userspace USB driver. >> >> Moreover, any upcoming new tentative to add CBUS GPIO support could >> integrate CBUS EEPROM configuration reading in order to determine >> which of the CBUS pins are available as GPIO. >> >> Reviewed-by: Andy Shevchenko >> Signed-off-by: Loic Poulain > > Reviewed-by: Ajay Gupta Any other comments? Regards, Loic -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6] USB: serial: ftdi_sio: Add MTP NVM support
Most of FTDI's devices have an EEPROM which records FTDI devices
configuration setting (e.g. the VID, PID, I/O config...) and user
data. For example, FT232R and FTX chips have 128-byte and 2048-byte
internal EEPROM respectively.
This patch adds support for FTDI EEPROM read/write via USB control
transfers and register a new nvm device to the nvmem core.
This permits to expose the EEPROM as a sysfs file, allowing userspace
to read/modify FTDI configuration and its user data without having to
rely on a specific userspace USB driver.
Moreover, any upcoming new tentative to add CBUS GPIO support could
integrate CBUS EEPROM configuration reading in order to determine
which of the CBUS pins are available as GPIO.
Reviewed-by: Andy Shevchenko
Signed-off-by: Loic Poulain
---
v2: Use ifdef instead of IS_ENABLED
error message in case of nvmem registering failure
Fix space/tab in Kconfig
v3: Make nvmem a child of the usb dev instead of the serial port
Add macros defining eeprom sizes
Check read/write size is a nultiple of the eeprom word-size
Remove useless change in Kconfig
v4: Reword commit message
Remove default-yes from Kconfig
Change includes ordering
Use default linux size defines
Use get_unaligned_le16 helper
Prepend EEPROM functions with ftdi_
Error message in ftdi_eeprom_register()
v5: Fix missing linux/sizes header
v6: Ordering new headers insertion
Remove unecessary additional buf pointer from read/write_eeprom
drivers/usb/serial/Kconfig| 10
drivers/usb/serial/ftdi_sio.c | 118 ++
drivers/usb/serial/ftdi_sio.h | 28 ++
3 files changed, 156 insertions(+)
diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
index 533f127..5747562 100644
--- a/drivers/usb/serial/Kconfig
+++ b/drivers/usb/serial/Kconfig
@@ -181,6 +181,16 @@ config USB_SERIAL_FTDI_SIO
To compile this driver as a module, choose M here: the
module will be called ftdi_sio.
+config USB_SERIAL_FTDI_SIO_NVMEM
+ bool "FTDI MTP non-volatile memory support"
+ depends on USB_SERIAL_FTDI_SIO
+ select NVMEM
+ help
+ Say yes here to add support for the MTP non-volatile memory
+ present on FTDI. Most of FTDI's devices have an EEPROM which
+ records FTDI device's configuration setting as well as user
+ data.
+
config USB_SERIAL_VISOR
tristate "USB Handspring Visor / Palm m50x / Sony Clie Driver"
help
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 7ea221d..34daa8c 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -34,12 +34,17 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
#include
#include
#include
+
+#include
+
#include "ftdi_sio.h"
#include "ftdi_sio_ids.h"
@@ -73,6 +78,8 @@ struct ftdi_private {
unsigned int latency; /* latency setting in use */
unsigned short max_packet_size;
struct mutex cfg_lock; /* Avoid mess by parallel calls of config
ioctl() and change_speed() */
+
+ struct nvmem_device *eeprom;
};
/* struct ftdi_sio_quirk is used by devices requiring special attention. */
@@ -1529,6 +1536,110 @@ static int get_lsr_info(struct usb_serial_port *port,
return 0;
}
+#ifdef CONFIG_USB_SERIAL_FTDI_SIO_NVMEM
+
+static int ftdi_write_eeprom(void *priv, unsigned int off, void *val,
+size_t bytes)
+{
+ struct usb_serial_port *port = priv;
+ struct usb_device *udev = port->serial->dev;
+
+ if (bytes % 2) /* 16-bit eeprom */
+ return -EINVAL;
+
+ while (bytes) {
+ int rv;
+
+ rv = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+FTDI_SIO_WRITE_EEPROM_REQUEST,
+FTDI_SIO_WRITE_EEPROM_REQUEST_TYPE,
+get_unaligned_le16(val), off / 2, NULL,
+0, WDR_TIMEOUT);
+ if (rv < 0)
+ return rv;
+
+ off += 2;
+ val += 2;
+ bytes -= 2;
+ }
+
+ return 0;
+}
+
+static int ftdi_read_eeprom(void *priv, unsigned int off, void *val,
+ size_t bytes)
+{
+ struct usb_serial_port *port = priv;
+ struct usb_device *udev = port->serial->dev;
+
+ if (bytes % 2) /* 16-bit eeprom */
+ return -EINVAL;
+
+ while (bytes) {
+ int rv;
+
+ rv = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+FTDI_SIO_READ_EEPROM_REQUEST,
+FTDI_SIO_READ_EEPROM_REQUEST_TYPE,
+0, off / 2, val, 2, WDR_TIMEOUT);
+
Re: [PATCH v5] USB: serial: ftdi_sio: Add MTP NVM support
Hi Andy,
On 26 June 2018 at 13:02, Andy Shevchenko wrote:
> On Mon, Jun 25, 2018 at 3:35 PM, Loic Poulain wrote:
>> Most of FTDI's devices have an EEPROM which records FTDI devices
>> configuration setting (e.g. the VID, PID, I/O config...) and user
>> data. For example, FT232R and FTX chips have 128-byte and 2048-byte
>> internal EEPROM respectively.
>>
>> This patch adds support for FTDI EEPROM read/write via USB control
>> transfers and register a new nvm device to the nvmem core.
>>
>> This permits to expose the EEPROM as a sysfs file, allowing userspace
>> to read/modify FTDI configuration and its user data without having to
>> rely on a specific userspace USB driver.
>>
>> Moreover, any upcoming new tentative to add CBUS GPIO support could
>> integrate CBUS EEPROM configuration reading in order to determine
>> which of the CBUS pins are available as GPIO.
>>
>
> FWIW,
> Reviewed-by: Andy Shevchenko
>
> One nit below, though.
>
>> Signed-off-by: Loic Poulain
>> ---
>> v2: Use ifdef instead of IS_ENABLED
>> error message in case of nvmem registering failure
>> Fix space/tab in Kconfig
>> v3: Make nvmem a child of the usb dev instead of the serial port
>> Add macros defining eeprom sizes
>> Check read/write size is a nultiple of the eeprom word-size
>> Remove useless change in Kconfig
>> v4: Reword commit message
>> Remove default-yes from Kconfig
>> Change includes ordering
>> Use default linux size defines
>> Use get_unaligned_le16 helper
>> Prepend EEPROM functions with ftdi_
>> Error message in ftdi_eeprom_register()
>> v5: Fix missing linux/sizes header
>>
>> drivers/usb/serial/Kconfig| 10
>> drivers/usb/serial/ftdi_sio.c | 119
>> ++
>> drivers/usb/serial/ftdi_sio.h | 28 ++
>> 3 files changed, 157 insertions(+)
>>
>> diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
>> index 533f127..5747562 100644
>> --- a/drivers/usb/serial/Kconfig
>> +++ b/drivers/usb/serial/Kconfig
>> @@ -181,6 +181,16 @@ config USB_SERIAL_FTDI_SIO
>> To compile this driver as a module, choose M here: the
>> module will be called ftdi_sio.
>>
>> +config USB_SERIAL_FTDI_SIO_NVMEM
>> + bool "FTDI MTP non-volatile memory support"
>> + depends on USB_SERIAL_FTDI_SIO
>> + select NVMEM
>> + help
>> + Say yes here to add support for the MTP non-volatile memory
>> + present on FTDI. Most of FTDI's devices have an EEPROM which
>> + records FTDI device's configuration setting as well as user
>> + data.
>> +
>> config USB_SERIAL_VISOR
>> tristate "USB Handspring Visor / Palm m50x / Sony Clie Driver"
>> help
>> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
>> index 7ea221d..75a35a8 100644
>> --- a/drivers/usb/serial/ftdi_sio.c
>> +++ b/drivers/usb/serial/ftdi_sio.c
>> @@ -37,9 +37,13 @@
>> #include
>> #include
>> #include
>> +#include
>> +#include
>> #include
>> #include
>> #include
>> +#include
>> +
>
> I would put it rather as follows
>
> +#include
> ... // something else is still in order here?
> +#include
> #include
> #include
> #include
> #include
> #include
> #include
> +
> +#include
> +
>
> // also note another blank line as divider.
>
Ok !
>> #include "ftdi_sio.h"
>> #include "ftdi_sio_ids.h"
>>
>> @@ -73,6 +77,8 @@ struct ftdi_private {
>> unsigned int latency; /* latency setting in use */
>> unsigned short max_packet_size;
>> struct mutex cfg_lock; /* Avoid mess by parallel calls of config
>> ioctl() and change_speed() */
>> +
>> + struct nvmem_device *eeprom;
>> };
>>
>> /* struct ftdi_sio_quirk is used by devices requiring special attention. */
>> @@ -1529,6 +1535,112 @@ static int get_lsr_info(struct usb_serial_port *port,
>> return 0;
>> }
>>
>> +#ifdef CONFIG_USB_SERIAL_FTDI_SIO_NVMEM
>> +
>> +static int ftdi_write_eeprom(void *priv, unsigned int off, void *val,
>> +size_t bytes)
>> +{
>> + struct usb_serial_port *port = priv;
>> + struct usb_device *udev = port->serial->dev;
>
>> + unsigned
[PATCH v5] USB: serial: ftdi_sio: Add MTP NVM support
Most of FTDI's devices have an EEPROM which records FTDI devices
configuration setting (e.g. the VID, PID, I/O config...) and user
data. For example, FT232R and FTX chips have 128-byte and 2048-byte
internal EEPROM respectively.
This patch adds support for FTDI EEPROM read/write via USB control
transfers and register a new nvm device to the nvmem core.
This permits to expose the EEPROM as a sysfs file, allowing userspace
to read/modify FTDI configuration and its user data without having to
rely on a specific userspace USB driver.
Moreover, any upcoming new tentative to add CBUS GPIO support could
integrate CBUS EEPROM configuration reading in order to determine
which of the CBUS pins are available as GPIO.
Signed-off-by: Loic Poulain
---
v2: Use ifdef instead of IS_ENABLED
error message in case of nvmem registering failure
Fix space/tab in Kconfig
v3: Make nvmem a child of the usb dev instead of the serial port
Add macros defining eeprom sizes
Check read/write size is a nultiple of the eeprom word-size
Remove useless change in Kconfig
v4: Reword commit message
Remove default-yes from Kconfig
Change includes ordering
Use default linux size defines
Use get_unaligned_le16 helper
Prepend EEPROM functions with ftdi_
Error message in ftdi_eeprom_register()
v5: Fix missing linux/sizes header
drivers/usb/serial/Kconfig| 10
drivers/usb/serial/ftdi_sio.c | 119 ++
drivers/usb/serial/ftdi_sio.h | 28 ++
3 files changed, 157 insertions(+)
diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
index 533f127..5747562 100644
--- a/drivers/usb/serial/Kconfig
+++ b/drivers/usb/serial/Kconfig
@@ -181,6 +181,16 @@ config USB_SERIAL_FTDI_SIO
To compile this driver as a module, choose M here: the
module will be called ftdi_sio.
+config USB_SERIAL_FTDI_SIO_NVMEM
+ bool "FTDI MTP non-volatile memory support"
+ depends on USB_SERIAL_FTDI_SIO
+ select NVMEM
+ help
+ Say yes here to add support for the MTP non-volatile memory
+ present on FTDI. Most of FTDI's devices have an EEPROM which
+ records FTDI device's configuration setting as well as user
+ data.
+
config USB_SERIAL_VISOR
tristate "USB Handspring Visor / Palm m50x / Sony Clie Driver"
help
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 7ea221d..75a35a8 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -37,9 +37,13 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
+#include
+
#include "ftdi_sio.h"
#include "ftdi_sio_ids.h"
@@ -73,6 +77,8 @@ struct ftdi_private {
unsigned int latency; /* latency setting in use */
unsigned short max_packet_size;
struct mutex cfg_lock; /* Avoid mess by parallel calls of config
ioctl() and change_speed() */
+
+ struct nvmem_device *eeprom;
};
/* struct ftdi_sio_quirk is used by devices requiring special attention. */
@@ -1529,6 +1535,112 @@ static int get_lsr_info(struct usb_serial_port *port,
return 0;
}
+#ifdef CONFIG_USB_SERIAL_FTDI_SIO_NVMEM
+
+static int ftdi_write_eeprom(void *priv, unsigned int off, void *val,
+size_t bytes)
+{
+ struct usb_serial_port *port = priv;
+ struct usb_device *udev = port->serial->dev;
+ unsigned char *buf = val;
+
+ if (bytes % 2) /* 16-bit eeprom */
+ return -EINVAL;
+
+ while (bytes) {
+ int rv;
+
+ rv = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+FTDI_SIO_WRITE_EEPROM_REQUEST,
+FTDI_SIO_WRITE_EEPROM_REQUEST_TYPE,
+get_unaligned_le16(buf), off / 2, NULL,
+0, WDR_TIMEOUT);
+ if (rv < 0)
+ return rv;
+
+ off += 2;
+ buf += 2;
+ bytes -= 2;
+ }
+
+ return 0;
+}
+
+static int ftdi_read_eeprom(void *priv, unsigned int off, void *val,
+ size_t bytes)
+{
+ struct usb_serial_port *port = priv;
+ struct usb_device *udev = port->serial->dev;
+ unsigned char *buf = val;
+
+ if (bytes % 2) /* 16-bit eeprom */
+ return -EINVAL;
+
+ while (bytes) {
+ int rv;
+
+ rv = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+FTDI_SIO_READ_EEPROM_REQUEST,
+FTDI_SIO_READ_EEPROM_REQUEST_TYPE,
+0, off / 2, buf, 2, WDR_TIMEOUT);
+ if (rv < 0)
+ return rv;
+
+ off += 2;
+ buf += 2
[PATCH v4] USB: serial: ftdi_sio: Add MTP NVM support
Most of FTDI's devices have an EEPROM which records FTDI devices
configuration setting (e.g. the VID, PID, I/O config...) and user
data. For example, FT232R and FTX chips have 128-byte and 2048-byte
internal EEPROM respectively.
This patch adds support for FTDI EEPROM read/write via USB control
transfers and register a new nvm device to the nvmem core.
This permits to expose the EEPROM as a sysfs file, allowing userspace
to read/modify FTDI configuration and its user data without having to
rely on a specific userspace USB driver.
Moreover, any upcoming new tentative to add CBUS GPIO support could
integrate CBUS EEPROM configuration reading in order to determine
which of the CBUS pins are available as GPIO.
Signed-off-by: Loic Poulain
---
v2: Use ifdef instead of IS_ENABLED
error message in case of nvmem registering failure
Fix space/tab in Kconfig
v3: Make nvmem a child of the usb dev instead of the serial port
Add macros defining eeprom sizes
Check read/write size is a nultiple of the eeprom word-size
Remove useless change in Kconfig
v4: Reword commit message
Remove default-yes from Kconfig
Change includes ordering
Use default linux size defines
Use get_unaligned_le16 helper
Prepend EEPROM functions with ftdi_
Error message in ftdi_eeprom_register()
drivers/usb/serial/Kconfig| 10
drivers/usb/serial/ftdi_sio.c | 118 ++
drivers/usb/serial/ftdi_sio.h | 28 ++
3 files changed, 156 insertions(+)
diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
index 533f127..5747562 100644
--- a/drivers/usb/serial/Kconfig
+++ b/drivers/usb/serial/Kconfig
@@ -181,6 +181,16 @@ config USB_SERIAL_FTDI_SIO
To compile this driver as a module, choose M here: the
module will be called ftdi_sio.
+config USB_SERIAL_FTDI_SIO_NVMEM
+ bool "FTDI MTP non-volatile memory support"
+ depends on USB_SERIAL_FTDI_SIO
+ select NVMEM
+ help
+ Say yes here to add support for the MTP non-volatile memory
+ present on FTDI. Most of FTDI's devices have an EEPROM which
+ records FTDI device's configuration setting as well as user
+ data.
+
config USB_SERIAL_VISOR
tristate "USB Handspring Visor / Palm m50x / Sony Clie Driver"
help
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 7ea221d..3f2f2c7 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -37,9 +37,12 @@
#include
#include
#include
+#include
#include
#include
#include
+#include
+
#include "ftdi_sio.h"
#include "ftdi_sio_ids.h"
@@ -73,6 +76,8 @@ struct ftdi_private {
unsigned int latency; /* latency setting in use */
unsigned short max_packet_size;
struct mutex cfg_lock; /* Avoid mess by parallel calls of config
ioctl() and change_speed() */
+
+ struct nvmem_device *eeprom;
};
/* struct ftdi_sio_quirk is used by devices requiring special attention. */
@@ -1529,6 +1534,112 @@ static int get_lsr_info(struct usb_serial_port *port,
return 0;
}
+#ifdef CONFIG_USB_SERIAL_FTDI_SIO_NVMEM
+
+static int ftdi_write_eeprom(void *priv, unsigned int off, void *val,
+size_t bytes)
+{
+ struct usb_serial_port *port = priv;
+ struct usb_device *udev = port->serial->dev;
+ unsigned char *buf = val;
+
+ if (bytes % 2) /* 16-bit eeprom */
+ return -EINVAL;
+
+ while (bytes) {
+ int rv;
+
+ rv = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+FTDI_SIO_WRITE_EEPROM_REQUEST,
+FTDI_SIO_WRITE_EEPROM_REQUEST_TYPE,
+get_unaligned_le16(buf), off / 2, NULL,
+0, WDR_TIMEOUT);
+ if (rv < 0)
+ return rv;
+
+ off += 2;
+ buf += 2;
+ bytes -= 2;
+ }
+
+ return 0;
+}
+
+static int ftdi_read_eeprom(void *priv, unsigned int off, void *val,
+ size_t bytes)
+{
+ struct usb_serial_port *port = priv;
+ struct usb_device *udev = port->serial->dev;
+ unsigned char *buf = val;
+
+ if (bytes % 2) /* 16-bit eeprom */
+ return -EINVAL;
+
+ while (bytes) {
+ int rv;
+
+ rv = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+FTDI_SIO_READ_EEPROM_REQUEST,
+FTDI_SIO_READ_EEPROM_REQUEST_TYPE,
+0, off / 2, buf, 2, WDR_TIMEOUT);
+ if (rv < 0)
+ return rv;
+
+ off += 2;
+ buf += 2;
+ bytes -= 2;
+ }
+
+
Re: [PATCH v3] USB: serial: ftdi_sio: Add MTP NVM support
Hi Andy,
On 25 June 2018 at 09:36, Andy Shevchenko wrote:
> On Fri, Jun 22, 2018 at 5:22 PM, Loic Poulain wrote:
>> Most of FTDI's devices have an EEPROM which records FTDI devices
>> configuration setting (e.g. the VID, PID, I/O config...) and user
>> data.
>
>> FT230R chip integrates a 128-byte eeprom, FT230X a 2048-byte
>> eeprom...
>
> This sounds unfinished. What is the continuation of the sentence?
>
>>
>> This patch adds support for FTDI EEPROM read/write via USB control
>> transfers and register a new nvm device to the nvmem core.
>>
>> This permits to expose the eeprom as a sysfs file, allowing userspace
>> to read/modify FTDI configuration and its user data without having to
>> rely on a specific userspace USB driver.
>>
>> Moreover, any upcoming new tentative to add CBUS GPIO support could
>> integrate CBUS EEPROM configuration reading in order to determine
>> which of the CBUS pins are available as GPIO.
>
> In some cases you are using EEPROM, in the rest, eeprom. What the difference?
I'll unify this.
>
>> +config USB_SERIAL_FTDI_SIO_NVMEM
>> + bool "FTDI MTP non-volatile memory support"
>> + depends on USB_SERIAL_FTDI_SIO
>> + select NVMEM
>
>> + default y
>
> First of all, I didn't find an evidence why it should be y...
Yes, I'll fix this.
>
>> + help
>> + Say yes here to add support for the MTP non-volatile memory
>> + present on FTDI. Most of FTDI's devices have an EEPROM which
>> + records FTDI device's configuration setting as well as user
>> + data.
>
> ...second, the help semantics is inconsistent with default.
>
>> #include
>> #include
>> #include
>> +#include
>
> Perhaps squeeze it into most ordered part and keep there an order?
OK
>
>> +#define EEPROM_SZ_FTX 2048 /* cf FTDI AN_201 */
>> +#define EEPROM_SZ_FT232RL 128 /* cf FTDI AN_121 */
>
> These defines looks too particular, shouldn't be this a part of driver
> data / platform data / device properties?
I'm not sure where to move this since it depends on chip type which is
detected at runtime. I don't think ftdi_sio_quirk is the right place.
So maybe I could just skip the defines for now and come back to:
switch (priv->chip_type) {
case FTX:
nvmconf.size = SZ_2K;
break;
...
> Also, above I think is already defined in a common way in linux/sizes.h.
Correct
>
>> +static int write_eeprom(void *priv, unsigned int off, void *_val, size_t
>> bytes)
>
> Leading _ in the name looks weird here, since it's not a macro.
> (and inconsistent with below function definitions)
>
>> + val = buf[0] + (buf[1] << 8);
>
> get_unaligned_le16() ?
>
>
>> + if (register_eeprom(port)) {
>
>> + dev_err(>dev, "Unable to register FTDI EEPROM\n");
>> + /* non-fatal */
>
> Doesn't register_eeprom() have some error messaging already?
I can move this into register_eeprom for consistency.
> Btw, can we rename it to be less generic, like adding a prefix?
OK, I'll add ftdi_ prefix.
>
>> + }
>
> --
> With Best Regards,
> Andy Shevchenko
Thanks,
Loic
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] USB: serial: ftdi_sio: Add MTP NVM support
Most of FTDI's devices have an EEPROM which records FTDI devices
configuration setting (e.g. the VID, PID, I/O config...) and user
data. FT230R chip integrates a 128-byte eeprom, FT230X a 2048-byte
eeprom...
This patch adds support for FTDI EEPROM read/write via USB control
transfers and register a new nvm device to the nvmem core.
This permits to expose the eeprom as a sysfs file, allowing userspace
to read/modify FTDI configuration and its user data without having to
rely on a specific userspace USB driver.
Moreover, any upcoming new tentative to add CBUS GPIO support could
integrate CBUS EEPROM configuration reading in order to determine
which of the CBUS pins are available as GPIO.
Signed-off-by: Loic Poulain
---
v2: Use ifdef instead of IS_ENABLED
error message in case of nvmem registering failure
Fix space/tab in Kconfig
v3: Make nvmem a child of the usb dev instead of the serial port
Add macros defining eeprom sizes
Check read/write size is a nultiple of the eeprom word-size
Remove useless change in Kconfig
drivers/usb/serial/Kconfig| 11
drivers/usb/serial/ftdi_sio.c | 123 ++
drivers/usb/serial/ftdi_sio.h | 28 ++
3 files changed, 162 insertions(+)
diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
index 533f127..ee9230c 100644
--- a/drivers/usb/serial/Kconfig
+++ b/drivers/usb/serial/Kconfig
@@ -181,6 +181,17 @@ config USB_SERIAL_FTDI_SIO
To compile this driver as a module, choose M here: the
module will be called ftdi_sio.
+config USB_SERIAL_FTDI_SIO_NVMEM
+ bool "FTDI MTP non-volatile memory support"
+ depends on USB_SERIAL_FTDI_SIO
+ select NVMEM
+ default y
+ help
+ Say yes here to add support for the MTP non-volatile memory
+ present on FTDI. Most of FTDI's devices have an EEPROM which
+ records FTDI device's configuration setting as well as user
+ data.
+
config USB_SERIAL_VISOR
tristate "USB Handspring Visor / Palm m50x / Sony Clie Driver"
help
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 7ea221d..8183b02 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -40,6 +40,7 @@
#include
#include
#include
+#include
#include "ftdi_sio.h"
#include "ftdi_sio_ids.h"
@@ -73,6 +74,8 @@ struct ftdi_private {
unsigned int latency; /* latency setting in use */
unsigned short max_packet_size;
struct mutex cfg_lock; /* Avoid mess by parallel calls of config
ioctl() and change_speed() */
+
+ struct nvmem_device *eeprom;
};
/* struct ftdi_sio_quirk is used by devices requiring special attention. */
@@ -1529,6 +1532,116 @@ static int get_lsr_info(struct usb_serial_port *port,
return 0;
}
+#ifdef CONFIG_USB_SERIAL_FTDI_SIO_NVMEM
+
+#define EEPROM_SZ_FTX 2048 /* cf FTDI AN_201 */
+#define EEPROM_SZ_FT232RL 128 /* cf FTDI AN_121 */
+
+static int write_eeprom(void *priv, unsigned int off, void *_val, size_t bytes)
+{
+ struct usb_serial_port *port = priv;
+ struct usb_serial *serial = port->serial;
+ struct usb_device *udev = serial->dev;
+ unsigned char *buf = _val;
+
+ if (bytes % 2) /* 16-bit eeprom */
+ return -EINVAL;
+
+ while (bytes) {
+ uint16_t val;
+ int rv;
+
+ val = buf[0] + (buf[1] << 8);
+
+ rv = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+FTDI_SIO_WRITE_EEPROM_REQUEST,
+FTDI_SIO_WRITE_EEPROM_REQUEST_TYPE,
+val, off / 2, NULL, 0, WDR_TIMEOUT);
+ if (rv < 0)
+ return rv;
+
+ off += 2;
+ buf += 2;
+ bytes -= 2;
+ }
+
+ return 0;
+}
+
+static int read_eeprom(void *priv, unsigned int off, void *val, size_t bytes)
+{
+ struct usb_serial_port *port = priv;
+ struct usb_serial *serial = port->serial;
+ struct usb_device *udev = serial->dev;
+ unsigned char *buf = val;
+
+ if (bytes % 2) /* 16-bit eeprom */
+ return -EINVAL;
+
+ while (bytes) {
+ int rv;
+
+ rv = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+FTDI_SIO_READ_EEPROM_REQUEST,
+FTDI_SIO_READ_EEPROM_REQUEST_TYPE,
+0, off / 2, buf, 2, WDR_TIMEOUT);
+ if (rv < 0)
+ return rv;
+
+ off += 2;
+ buf += 2;
+ bytes -= 2;
+ }
+
+ return 0;
+}
+
+static int register_eeprom(struct usb_serial_port *port)
+{
+ struct ftdi_private *priv = usb_get_serial_port_data(port);
+
Re: [PATCH v2] USB: serial: ftdi_sio: Add MTP NVM support
Hi Ajay,
Thanks for the review.
On 21 June 2018 at 19:34, Ajay Gupta wrote:
> Hi Loic
>
> On 6/21/18, Loic Poulain wrote:
>> Most of FTDI's devices have an EEPROM which records FTDI devices
>> configuration setting (e.g. the VID, PID, I/O config...) and user
>> data. FT230R chip integrates a 128-byte eeprom, FT230X a 2048-byte
>> eeprom...
>>
>> This patch adds support for FTDI EEPROM read/write via USB control
>> transfers and register a new nvm device to the nvmem core.
>>
>> This permits to expose the eeprom as a sysfs file, allowing userspace
>> to read/modify FTDI configuration and its user data without having to
>> rely on a specific userspace USB driver.
>>
>> Moreover, any upcoming new tentative to add CBUS GPIO support could
>> integrate CBUS EEPROM configuration reading in order to determine
>> which of the CBUS pins are available as GPIO.
>>
>> Signed-off-by: Loic Poulain
>> ---
>> v2: Use ifdef instead of IS_ENABLED
>> error message in case of nvmem registering failure
>> Fix space/tab in Kconfig
>>
>> drivers/usb/serial/Kconfig| 13 -
>> drivers/usb/serial/ftdi_sio.c | 111
>> ++
>> drivers/usb/serial/ftdi_sio.h | 28 +++
>> 3 files changed, 151 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
>> index 533f127..f05af5f 100644
>> --- a/drivers/usb/serial/Kconfig
>> +++ b/drivers/usb/serial/Kconfig
>> @@ -153,7 +153,7 @@ config USB_SERIAL_CYPRESS_M8
>>
>> Supported microcontrollers in the CY4601 family are:
>> CY7C63741 CY7C63742 CY7C63743 CY7C64013
>> -
>> +
> There is no change here so please remove it.
>
>> To compile this driver as a module, choose M here: the
>> module will be called cypress_m8.
>>
>> @@ -181,6 +181,17 @@ config USB_SERIAL_FTDI_SIO
>> To compile this driver as a module, choose M here: the
>> module will be called ftdi_sio.
>>
>> +config USB_SERIAL_FTDI_SIO_NVMEM
>> + bool "FTDI MTP non-volatile memory support"
>> + depends on USB_SERIAL_FTDI_SIO
>> + select NVMEM
>> + default y
>> + help
>> + Say yes here to add support for the MTP non-volatile memory
>> + present on FTDI. Most of FTDI's devices have an EEPROM which
>> + records FTDI device's configuration setting as well as user
>> + data.
>> +
>> config USB_SERIAL_VISOR
>> tristate "USB Handspring Visor / Palm m50x / Sony Clie Driver"
>> help
>> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
>> index 7ea221d..9e242e8 100644
>> --- a/drivers/usb/serial/ftdi_sio.c
>> +++ b/drivers/usb/serial/ftdi_sio.c
>> @@ -40,6 +40,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include "ftdi_sio.h"
>> #include "ftdi_sio_ids.h"
>>
>> @@ -73,6 +74,8 @@ struct ftdi_private {
>> unsigned int latency; /* latency setting in use */
>> unsigned short max_packet_size;
>> struct mutex cfg_lock; /* Avoid mess by parallel calls of config
>> ioctl()
>> and change_speed() */
>> +
>> + struct nvmem_device *eeprom;
>> };
>>
>> /* struct ftdi_sio_quirk is used by devices requiring special attention.
>> */
>> @@ -1529,6 +1532,104 @@ static int get_lsr_info(struct usb_serial_port
>> *port,
>> return 0;
>> }
>>
>> +#ifdef CONFIG_USB_SERIAL_FTDI_SIO_NVMEM
>> +
>> +static int write_eeprom(void *priv, unsigned int off, void *_val, size_t
>> bytes)
>> +{
>> + struct usb_serial_port *port = priv;
>> + struct usb_serial *serial = port->serial;
>> + struct usb_device *udev = serial->dev;
>> + unsigned char *buf = _val;
>> +
>> + while (bytes) { /* bytes value is always a multiple of 2 */
>
> We should add check that 'bytes' is always multiple of 2 otherwise in
> case its not then there will be memory overrun due to buf[1] access
> below.
> while (bytes / 2) {
> }
>
This is in theory something which is guaranteed by the nvmem core since
we specified a 2-byte word size on registering. But looking at the code,
word-size is only checked on userspace access, not with kernel API
(e.g. nvmem_device_write). So yes It could be worth to double check here.
>> + uint16_t val;
>> + int rv;
>> +
>> +
[PATCH v2] USB: serial: ftdi_sio: Add MTP NVM support
Most of FTDI's devices have an EEPROM which records FTDI devices
configuration setting (e.g. the VID, PID, I/O config...) and user
data. FT230R chip integrates a 128-byte eeprom, FT230X a 2048-byte
eeprom...
This patch adds support for FTDI EEPROM read/write via USB control
transfers and register a new nvm device to the nvmem core.
This permits to expose the eeprom as a sysfs file, allowing userspace
to read/modify FTDI configuration and its user data without having to
rely on a specific userspace USB driver.
Moreover, any upcoming new tentative to add CBUS GPIO support could
integrate CBUS EEPROM configuration reading in order to determine
which of the CBUS pins are available as GPIO.
Signed-off-by: Loic Poulain
---
v2: Use ifdef instead of IS_ENABLED
error message in case of nvmem registering failure
Fix space/tab in Kconfig
drivers/usb/serial/Kconfig| 13 -
drivers/usb/serial/ftdi_sio.c | 111 ++
drivers/usb/serial/ftdi_sio.h | 28 +++
3 files changed, 151 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
index 533f127..f05af5f 100644
--- a/drivers/usb/serial/Kconfig
+++ b/drivers/usb/serial/Kconfig
@@ -153,7 +153,7 @@ config USB_SERIAL_CYPRESS_M8
Supported microcontrollers in the CY4601 family are:
CY7C63741 CY7C63742 CY7C63743 CY7C64013
-
+
To compile this driver as a module, choose M here: the
module will be called cypress_m8.
@@ -181,6 +181,17 @@ config USB_SERIAL_FTDI_SIO
To compile this driver as a module, choose M here: the
module will be called ftdi_sio.
+config USB_SERIAL_FTDI_SIO_NVMEM
+ bool "FTDI MTP non-volatile memory support"
+ depends on USB_SERIAL_FTDI_SIO
+ select NVMEM
+ default y
+ help
+ Say yes here to add support for the MTP non-volatile memory
+ present on FTDI. Most of FTDI's devices have an EEPROM which
+ records FTDI device's configuration setting as well as user
+ data.
+
config USB_SERIAL_VISOR
tristate "USB Handspring Visor / Palm m50x / Sony Clie Driver"
help
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 7ea221d..9e242e8 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -40,6 +40,7 @@
#include
#include
#include
+#include
#include "ftdi_sio.h"
#include "ftdi_sio_ids.h"
@@ -73,6 +74,8 @@ struct ftdi_private {
unsigned int latency; /* latency setting in use */
unsigned short max_packet_size;
struct mutex cfg_lock; /* Avoid mess by parallel calls of config
ioctl() and change_speed() */
+
+ struct nvmem_device *eeprom;
};
/* struct ftdi_sio_quirk is used by devices requiring special attention. */
@@ -1529,6 +1532,104 @@ static int get_lsr_info(struct usb_serial_port *port,
return 0;
}
+#ifdef CONFIG_USB_SERIAL_FTDI_SIO_NVMEM
+
+static int write_eeprom(void *priv, unsigned int off, void *_val, size_t bytes)
+{
+ struct usb_serial_port *port = priv;
+ struct usb_serial *serial = port->serial;
+ struct usb_device *udev = serial->dev;
+ unsigned char *buf = _val;
+
+ while (bytes) { /* bytes value is always a multiple of 2 */
+ uint16_t val;
+ int rv;
+
+ val = buf[0] + (buf[1] << 8);
+
+ rv = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+FTDI_SIO_WRITE_EEPROM_REQUEST,
+FTDI_SIO_WRITE_EEPROM_REQUEST_TYPE,
+val, off / 2, NULL, 0, WDR_TIMEOUT);
+ if (rv < 0)
+ return rv;
+
+ off += 2;
+ buf += 2;
+ bytes -= 2;
+ }
+
+ return 0;
+}
+
+static int read_eeprom(void *priv, unsigned int off, void *val, size_t bytes)
+{
+ struct usb_serial_port *port = priv;
+ struct usb_serial *serial = port->serial;
+ struct usb_device *udev = serial->dev;
+ unsigned char *buf = val;
+
+ while (bytes) { /* bytes value is always a multiple of 2 */
+ int rv;
+
+ rv = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+FTDI_SIO_READ_EEPROM_REQUEST,
+FTDI_SIO_READ_EEPROM_REQUEST_TYPE,
+0, off / 2, buf, 2, WDR_TIMEOUT);
+ if (rv < 0)
+ return rv;
+
+ off += 2;
+ buf += 2;
+ bytes -= 2;
+ }
+
+ return 0;
+}
+
+static int register_eeprom(struct usb_serial_port *port)
+{
+ struct ftdi_private *priv = usb_get_serial_port_data(port);
+ struct nvmem_config nvmconf = {};
+
+ switch (priv->
Re: [PATCH] USB: serial: ftdi_sio: Add MTP NVM support
Hi Johan, Srini, On 19 June 2018 at 15:06, Johan Hovold wrote: > On Tue, Jun 19, 2018 at 02:32:11PM +0200, Loic Poulain wrote: >> Hi Johan, Srini, >> >> On 18 June 2018 at 11:47, Srinivas Kandagatla >> wrote: >> > On 18/06/18 09:46, Johan Hovold wrote: > >> >> I'm not necessarily against the idea, but nvmem core needs to be fixed >> >> so that it can handle hotplugging before this can be considered for >> >> merging. >> >> >> >> Right now it just returns -EBUSY from nvmem_unregister(), which results >> >> in all kinds of memory leaks, use-after-frees and crashes when user >> >> space holds the character device open while the device is being >> >> unplugged. >> >> Correct me if I'm wrong, but nvmem is just exposed to userspace as a >> simple sysfs device attribute (nvmem), removing a device and its attribute(s) >> dynamically is well managed by sysfs, even if userspace has file open. > > My bad, I misremembered what interface nvmem was using and only saw the > deregistration refusal in nvmem_unregister() during a quick check. > >> The only risk here is when a kernel internal consumer still has a reference >> to >> the nvmem device on removal, which is not the case in our context. > > Right. > >> > I can also suggest you to try devm_nvmem_register(). >> >> Yes, I'm going to send a v2. > > I'll take a closer look soonish too. I changed my mind here, indeed, in order to avoid any potential use-after-free, we have to manually unregister/release the nvmem device before freeing of the private port structure (used as nvmem context). So I will not send a V2 with nvmem devm usage. You can consider this V1 for review. Regards, Loic -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: serial: ftdi_sio: Add MTP NVM support
Hi Johan, Srini, On 18 June 2018 at 11:47, Srinivas Kandagatla wrote: > > > On 18/06/18 09:46, Johan Hovold wrote: >> >> On Thu, Jun 14, 2018 at 10:08:46PM +0200, Loic Poulain wrote: >>> >>> Most of FTDI's devices have an EEPROM which records FTDI devices >>> configuration setting (e.g. the VID, PID, I/O config...) and user >>> data. FT230R chip integrates a 128-byte eeprom, FT230X a 2048-byte >>> eeprom... >>> >>> This patch adds support for FTDI EEPROM read/write via USB control >>> transfers and register a new nvm device to the nvmem core. >>> >>> This permits to expose the eeprom as a sysfs file, allowing userspace >>> to read/modify FTDI configuration and its user data without having to >>> rely on a specific userspace USB driver. >>> >>> Moreover, any upcoming new tentative to add CBUS GPIO support could >>> integrate CBUS EEPROM configuration reading in order to determine >>> which of the CBUS pins are available as GPIO. >> >> >> I'm not necessarily against the idea, but nvmem core needs to be fixed >> so that it can handle hotplugging before this can be considered for >> merging. >> >> Right now it just returns -EBUSY from nvmem_unregister(), which results >> in all kinds of memory leaks, use-after-frees and crashes when user >> space holds the character device open while the device is being >> unplugged. Correct me if I'm wrong, but nvmem is just exposed to userspace as a simple sysfs device attribute (nvmem), removing a device and its attribute(s) dynamically is well managed by sysfs, even if userspace has file open. The only risk here is when a kernel internal consumer still has a reference to the nvmem device on removal, which is not the case in our context. > I can also suggest you to try devm_nvmem_register(). Yes, I'm going to send a v2. Regards, Loic -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: serial: ftdi_sio: Add MTP NVM support
Most of FTDI's devices have an EEPROM which records FTDI devices
configuration setting (e.g. the VID, PID, I/O config...) and user
data. FT230R chip integrates a 128-byte eeprom, FT230X a 2048-byte
eeprom...
This patch adds support for FTDI EEPROM read/write via USB control
transfers and register a new nvm device to the nvmem core.
This permits to expose the eeprom as a sysfs file, allowing userspace
to read/modify FTDI configuration and its user data without having to
rely on a specific userspace USB driver.
Moreover, any upcoming new tentative to add CBUS GPIO support could
integrate CBUS EEPROM configuration reading in order to determine
which of the CBUS pins are available as GPIO.
Signed-off-by: Loic Poulain
---
drivers/usb/serial/Kconfig| 11 +
drivers/usb/serial/ftdi_sio.c | 108 ++
drivers/usb/serial/ftdi_sio.h | 28 +++
3 files changed, 147 insertions(+)
diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
index 533f127..2dd2f5d 100644
--- a/drivers/usb/serial/Kconfig
+++ b/drivers/usb/serial/Kconfig
@@ -181,6 +181,17 @@ config USB_SERIAL_FTDI_SIO
To compile this driver as a module, choose M here: the
module will be called ftdi_sio.
+config USB_SERIAL_FTDI_SIO_NVMEM
+bool "FTDI MTP non-volatile memory support"
+ depends on USB_SERIAL_FTDI_SIO
+select NVMEM
+default y
+help
+ Say yes here to add support for the MTP non-volatile memory
+ present on FTDI. Most of FTDI's devices have an EEPROM which
+ records FTDI device's configuration setting as well as user
+ data.
+
config USB_SERIAL_VISOR
tristate "USB Handspring Visor / Palm m50x / Sony Clie Driver"
help
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 7ea221d..03c9c75 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -40,6 +40,7 @@
#include
#include
#include
+#include
#include "ftdi_sio.h"
#include "ftdi_sio_ids.h"
@@ -73,6 +74,8 @@ struct ftdi_private {
unsigned int latency; /* latency setting in use */
unsigned short max_packet_size;
struct mutex cfg_lock; /* Avoid mess by parallel calls of config
ioctl() and change_speed() */
+
+ struct nvmem_device *eeprom;
};
/* struct ftdi_sio_quirk is used by devices requiring special attention. */
@@ -1529,6 +1532,104 @@ static int get_lsr_info(struct usb_serial_port *port,
return 0;
}
+#if IS_ENABLED(CONFIG_USB_SERIAL_FTDI_SIO_NVMEM)
+
+static int write_eeprom(void *priv, unsigned int off, void *_val, size_t bytes)
+{
+ struct usb_serial_port *port = priv;
+ struct usb_serial *serial = port->serial;
+ struct usb_device *udev = serial->dev;
+ unsigned char *buf = _val;
+
+ while (bytes) {
+ uint16_t val;
+ int rv;
+
+ val = buf[0] + (buf[1] << 8);
+
+ rv = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+FTDI_SIO_WRITE_EEPROM_REQUEST,
+FTDI_SIO_WRITE_EEPROM_REQUEST_TYPE,
+val, off / 2, NULL, 0, WDR_TIMEOUT);
+ if (rv < 0)
+ return rv;
+
+ off += 2;
+ buf += 2;
+ bytes -= 2;
+ }
+
+ return 0;
+}
+
+static int read_eeprom(void *priv, unsigned int off, void *val, size_t bytes)
+{
+ struct usb_serial_port *port = priv;
+ struct usb_serial *serial = port->serial;
+ struct usb_device *udev = serial->dev;
+ unsigned char *buf = val;
+
+ while (bytes) {
+ int rv;
+
+ rv = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+FTDI_SIO_READ_EEPROM_REQUEST,
+FTDI_SIO_READ_EEPROM_REQUEST_TYPE,
+0, off / 2, buf, 2, WDR_TIMEOUT);
+ if (rv < 0)
+ return rv;
+
+ off += 2;
+ buf += 2;
+ bytes -= 2;
+ }
+
+ return 0;
+}
+
+static int register_eeprom(struct usb_serial_port *port)
+{
+ struct ftdi_private *priv = usb_get_serial_port_data(port);
+ struct nvmem_config nvmconf = {};
+
+ switch (priv->chip_type) {
+ case FTX:
+ nvmconf.size = 2048;
+ break;
+ case FT232RL:
+ nvmconf.size = 128;
+ break;
+ default:
+ return 0;
+ }
+
+ nvmconf.word_size = 2;
+ nvmconf.stride = 2;
+ nvmconf.read_only = false;
+ nvmconf.priv = port;
+ nvmconf.dev = >dev;
+ nvmconf.reg_read = read_eeprom;
+ nvmconf.reg_write = write_eeprom;
+ nvmconf.owner = THIS_MODULE;
+
+
