Re: [PATCH 14/21] usb: chipidea: msm: Add proper clk and reset support
On Sun, Jun 26, 2016 at 12:28:31AM -0700, Stephen Boyd wrote:
> The msm chipidea controller uses two main clks, an AHB clk to
> read/write the MMIO registers and a core clk called the system
> clk that drives the controller itself. Add support for these clks
> as they're required in all designs.
>
> Also add support for an optional third clk that we need to turn
> on to read/write the ULPI phy registers. Some ULPI phys drive
> this clk themselves and so it isn't necessary to turn on to probe
> a ULPI device, but the HSIC phy doesn't provide one itself, so we
> must turn it on here.
>
> Cc: Peter Chen
> Cc: Greg Kroah-Hartman
> Signed-off-by: Stephen Boyd
> ---
> drivers/usb/chipidea/ci_hdrc_msm.c | 58
> +++---
> 1 file changed, 54 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c
> b/drivers/usb/chipidea/ci_hdrc_msm.c
> index 07cccd24a87f..40249b0e3e93 100644
> --- a/drivers/usb/chipidea/ci_hdrc_msm.c
> +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
> @@ -10,11 +10,19 @@
> #include
> #include
> #include
> +#include
> +#include
>
> #include "ci.h"
>
> #define HS_PHY_AHB_MODE 0x0098
>
> +struct ci_hdrc_msm {
> + struct platform_device *ci;
> + struct clk *core_clk;
> + struct clk *iface_clk;
> +};
> +
> static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
> {
> struct device *dev = ci->gadget.dev.parent;
> @@ -43,34 +51,76 @@ static struct ci_hdrc_platform_data ci_hdrc_msm_platdata
> = {
>
> static int ci_hdrc_msm_probe(struct platform_device *pdev)
> {
> + struct ci_hdrc_msm *ci;
> struct platform_device *plat_ci;
> + struct clk *clk;
> + struct reset_control *reset;
> + int ret;
>
> dev_dbg(&pdev->dev, "ci_hdrc_msm_probe\n");
>
> + ci = devm_kzalloc(&pdev->dev, sizeof(*ci), GFP_KERNEL);
> + if (!ci)
> + return -ENOMEM;
> + platform_set_drvdata(pdev, ci);
> +
> + reset = devm_reset_control_get(&pdev->dev, "core");
> + if (IS_ERR(reset))
> + return PTR_ERR(reset);
> +
> + ci->core_clk = clk = devm_clk_get(&pdev->dev, "core");
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + ci->iface_clk = clk = devm_clk_get(&pdev->dev, "iface");
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
You say it has three clocks in commit log, why it is only two in the code?
> +
> + reset_control_assert(reset);
> + usleep_range(1, 12000);
> + reset_control_deassert(reset);
> +
> + ret = clk_prepare_enable(ci->core_clk);
> + if (ret)
> + return ret;
> +
> + ret = clk_prepare_enable(ci->iface_clk);
> + if (ret)
> + goto err_iface;
> +
> plat_ci = ci_hdrc_add_device(&pdev->dev,
> pdev->resource, pdev->num_resources,
> &ci_hdrc_msm_platdata);
ci->plat_ci(or ci) = ...
Delete the local variable plat_ci
> if (IS_ERR(plat_ci)) {
> dev_err(&pdev->dev, "ci_hdrc_add_device failed!\n");
> - return PTR_ERR(plat_ci);
> + ret = PTR_ERR(plat_ci);
> + goto err_mux;
> }
>
> - platform_set_drvdata(pdev, plat_ci);
> + ci->ci = plat_ci;
[...]
--
Best Regards,
Peter Chen
--
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 4/7] reset: Supply *_shared variant calls when using *_optional APIs
Hi Lee,
Am Dienstag, den 28.06.2016, 09:56 +0100 schrieb Lee Jones:
> Philipp,
>
> I need this to go into the -rcs too.
>
> Can I add it with your Ack please?
I have already added your patches to my branch, and I intend to send a
pull request for it tomorrow. I am afraid applying this patch in another
branch would create a merge conflict with
git://git.pengutronix.de/git/pza/linux.git reset/next
in arm-soc. If that is not the case, go ahead. Otherwise I could provide
a tag at commit 2485394d9e0b ("reset: TRIVIAL: Add line break at same
place for similar APIs") for you to merge.
regards
Philipp
--
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 4/7] reset: Supply *_shared variant calls when using *_optional APIs
On Wed, 29 Jun 2016, Philipp Zabel wrote:
> Am Dienstag, den 28.06.2016, 09:56 +0100 schrieb Lee Jones:
> > Philipp,
> >
> > I need this to go into the -rcs too.
> >
> > Can I add it with your Ack please?
>
> I have already added your patches to my branch, and I intend to send a
> pull request for it tomorrow. I am afraid applying this patch in another
> branch would create a merge conflict with
> git://git.pengutronix.de/git/pza/linux.git reset/next
> in arm-soc. If that is not the case, go ahead. Otherwise I could provide
> a tag at commit 2485394d9e0b ("reset: TRIVIAL: Add line break at same
> place for similar APIs") for you to merge.
Yes please. If you could tag a branch containing commit:
reset: Supply *_shared variant calls when using *_optional APIs
... that would be perfect. Would you mind sending me the tag name
ASAP please, since I am ready to send my pull-request to Linus.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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: gadget: rndis: free response queue during REMOTE_NDIS_RESET_MSG
Amit Pundir writes: > From: xerox_lin Please sanitize name here. Then resend. thanks -- balbi signature.asc Description: PGP signature
RE: [PATCH 0/2] usb: renesas_usbhs: fix issues on specific situations
Yoshihiro Shimoda writes: > Hi Felipe, > > Would you review this patch set? both in my queue. -- balbi signature.asc Description: PGP signature
Re: [PATCH 15/21] usb: chipidea: msm: Mux over secondary phy at the right time
On Sun, Jun 26, 2016 at 12:28:32AM -0700, Stephen Boyd wrote:
> We need to pick the correct phy at runtime based on how the SoC
> has been wired onto the board. If the secondary phy is used, take
> it out of reset and mux over to it by writing into the TCSR
> register. Make sure to do this on reset too, because this
> register is reset to the default value (primary phy) after the
> RESET bit is set in USBCMD.
>
I am curious when you need the secondary phy?
> Cc: Peter Chen
> Cc: Greg Kroah-Hartman
> Signed-off-by: Stephen Boyd
> ---
> drivers/usb/chipidea/ci_hdrc_msm.c | 78
> +++---
> 1 file changed, 73 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c
> b/drivers/usb/chipidea/ci_hdrc_msm.c
> index 40249b0e3e93..df0f8b31db4f 100644
> --- a/drivers/usb/chipidea/ci_hdrc_msm.c
> +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
> @@ -8,30 +8,40 @@
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> +#include
> +#include
> +#include
>
> #include "ci.h"
>
> #define HS_PHY_AHB_MODE 0x0098
> +#define HS_PHY_SEC_CTRL 0x0278
> +# define HS_PHY_DIG_CLAMP_N BIT(16)
>
One space at the beginning, and keep alignment.
> struct ci_hdrc_msm {
> struct platform_device *ci;
> struct clk *core_clk;
> struct clk *iface_clk;
> + bool secondary_phy;
> + void __iomem *base;
> };
>
> static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
> {
> - struct device *dev = ci->gadget.dev.parent;
> + struct device *dev = ci->dev->parent;
> + struct ci_hdrc_msm *msm_ci = dev_get_drvdata(dev);
>
> switch (event) {
> case CI_HDRC_CONTROLLER_RESET_EVENT:
> dev_dbg(dev, "CI_HDRC_CONTROLLER_RESET_EVENT received\n");
> /* use AHB transactor, allow posted data writes */
> hw_write_id_reg(ci, HS_PHY_AHB_MODE, 0x, 0x8);
> + if (msm_ci->secondary_phy)
> + hw_write_id_reg(ci, HS_PHY_SEC_CTRL, HS_PHY_DIG_CLAMP_N,
> + HS_PHY_DIG_CLAMP_N);
> break;
> default:
> dev_dbg(dev, "unknown ci_hdrc event\n");
> @@ -49,12 +59,58 @@ static struct ci_hdrc_platform_data ci_hdrc_msm_platdata
> = {
> .notify_event = ci_hdrc_msm_notify_event,
> };
>
> +static int ci_hdrc_msm_mux_phy(struct ci_hdrc_msm *ci,
> +struct platform_device *pdev)
> +{
> + struct regmap *regmap;
> + struct device_node *syscon;
> + struct device *dev = &pdev->dev;
> + u32 off, val;
> + int ret;
> +
> + syscon = of_parse_phandle(dev->of_node, "phy-select", 0);
> + if (!syscon)
> + return 0;
> +
> + regmap = syscon_node_to_regmap(syscon);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> +
> + ret = of_property_read_u32_index(dev->of_node, "phy-select", 1, &off);
> + if (ret < 0) {
> + dev_err(dev, "no offset in syscon\n");
> + return -EINVAL;
> + }
> +
> + ret = of_property_read_u32_index(dev->of_node, "phy-select", 2, &val);
> + if (ret < 0) {
> + dev_err(dev, "no value in syscon\n");
> + return -EINVAL;
> + }
> +
> + ret = regmap_write(regmap, off, val);
> + if (ret)
> + return ret;
> +
> + ci->secondary_phy = !!val;
> + if (ci->secondary_phy) {
> + val = readl_relaxed(ci->base + HS_PHY_SEC_CTRL);
> + val |= HS_PHY_DIG_CLAMP_N;
> + writel_relaxed(val, ci->base + HS_PHY_SEC_CTRL);
> + }
> +
> + return 0;
> +}
> +
> static int ci_hdrc_msm_probe(struct platform_device *pdev)
> {
> struct ci_hdrc_msm *ci;
> struct platform_device *plat_ci;
> struct clk *clk;
> struct reset_control *reset;
> + struct resource *res;
> + void __iomem *base;
> + resource_size_t size;
> int ret;
>
> dev_dbg(&pdev->dev, "ci_hdrc_msm_probe\n");
> @@ -76,6 +132,15 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
> if (IS_ERR(clk))
> return PTR_ERR(clk);
>
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENODEV;
> +
> + size = resource_size(res);
> + ci->base = base = devm_ioremap(&pdev->dev, res->start, size);
> + if (!base)
> + return -ENOMEM;
> +
The core will do the ioremap too, you can't remap io address two times.
The offset larger than 0x200 is vendor specific, you can map it as
the second io region.
> reset_control_assert(reset);
> usleep_range(1, 12000);
> reset_control_deassert(reset);
> @@ -88,9 +153,12 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
> if (ret)
> goto err_iface;
>
> - plat_ci = ci_hdrc_add_device(&pdev->dev,
> -
Re: [PATCH v12 2/4] gadget: Support for the usb charger framework
Hi,
Baolin Wang writes:
>> Baolin Wang writes:
>>> For supporting the usb charger, it adds the usb_charger_init() and
>>> usb_charger_exit() functions for usb charger initialization and exit.
>>>
>>> It will report to the usb charger when the gadget state is changed,
>>> then the usb charger can do the power things.
>>>
>>> Signed-off-by: Baolin Wang
>>> Reviewed-by: Li Jun
>>> Tested-by: Li Jun
>>
>> Before anything, I must say that I really liked this patch. It's
>> minimaly invasive to udc core and does all the necessary changes. If it
>> wasn't for the extra charger class, this would've been perfect.
>>
>> Can't you just tie a charger to a UDC and avoid the charger class
>> completely?
>>
>>> static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned
>>> mA)
>>> {
>>> + if (gadget->charger)
>>
>> I guess you could do this check inside
>> usb_gadget_set_cur_limit_by_type() itself.
>
> We will access the 'gadget->charger->type' member when issuing
> usb_gadget_set_cur_limit_by_type(), so I think I should leave the
> check here in next new version.
Here's what I mean:
int usb_charger_set_cur_limit(struct usb_gadget *gadget, unsigned int mA)
{
struct usb_charger *charger;
enum usb_charger_type type;
if (!gadget->charger)
return 0;
charger = gadget->charger;
type = charger->type;
return __usb_charger_set_cur_limit(charger, type, mA);
}
static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA)
{
usb_charger_set_cur_limit(gadget, mA);
if (!gadget->ops->vbus_draw)
return -EOPNOTSUPP;
return gadget->ops->vbus_draw(gadget, mA);
}
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH v12 2/4] gadget: Support for the usb charger framework
Hi Felipe,
On 29 June 2016 at 16:20, Felipe Balbi wrote:
>
> Hi,
>
> Baolin Wang writes:
>>> Baolin Wang writes:
For supporting the usb charger, it adds the usb_charger_init() and
usb_charger_exit() functions for usb charger initialization and exit.
It will report to the usb charger when the gadget state is changed,
then the usb charger can do the power things.
Signed-off-by: Baolin Wang
Reviewed-by: Li Jun
Tested-by: Li Jun
>>>
>>> Before anything, I must say that I really liked this patch. It's
>>> minimaly invasive to udc core and does all the necessary changes. If it
>>> wasn't for the extra charger class, this would've been perfect.
>>>
>>> Can't you just tie a charger to a UDC and avoid the charger class
>>> completely?
>>>
static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget,
unsigned mA)
{
+ if (gadget->charger)
>>>
>>> I guess you could do this check inside
>>> usb_gadget_set_cur_limit_by_type() itself.
>>
>> We will access the 'gadget->charger->type' member when issuing
>> usb_gadget_set_cur_limit_by_type(), so I think I should leave the
>> check here in next new version.
>
> Here's what I mean:
>
> int usb_charger_set_cur_limit(struct usb_gadget *gadget, unsigned int mA)
> {
> struct usb_charger *charger;
> enum usb_charger_type type;
>
> if (!gadget->charger)
> return 0;
>
> charger = gadget->charger;
> type = charger->type;
>
> return __usb_charger_set_cur_limit(charger, type, mA);
> }
But that means we need to export both 'usb_charger_set_cur_limit()'
function and '__usb_charger_set_cur_limit()' function in charger.c
file. Cause some user may want to set the current limitation by one
charger type parameter (may be not from charger->type), like by
issuing '__usb_charger_set_cur_limit(charger, SDP_TYPE, mA)'. How do
you think about this situation? Thanks.
>
> static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA)
> {
> usb_charger_set_cur_limit(gadget, mA);
>
> if (!gadget->ops->vbus_draw)
> return -EOPNOTSUPP;
> return gadget->ops->vbus_draw(gadget, mA);
> }
>
> --
> balbi
--
Baolin.wang
Best Regards
--
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 16/21] usb: chipidea: msm: Restore wrapper settings after reset
On Sun, Jun 26, 2016 at 12:28:33AM -0700, Stephen Boyd wrote:
> When the RESET bit is set in the USBCMD register it resets quite
> a few of the wrapper's registers to their reset state. This
> includes the GENCONFIG and GENCONFIG2 registers. Currently this
> is done by the usb phy and ehci-msm drivers writing into the
> controller wrapper's MMIO address space. Let's consolidate the
> register writes into the wrapper driver instead so that we
> clearly split the wrapper from the phys.
>
> Cc: Peter Chen
> Cc: Greg Kroah-Hartman
> Signed-off-by: Stephen Boyd
> ---
> drivers/usb/chipidea/ci_hdrc_msm.c | 46
> ++
> 1 file changed, 46 insertions(+)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c
> b/drivers/usb/chipidea/ci_hdrc_msm.c
> index df0f8b31db4f..cc6f9b0df9d5 100644
> --- a/drivers/usb/chipidea/ci_hdrc_msm.c
> +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
> @@ -14,6 +14,8 @@
> #include
> #include
> #include
> +#include
> +#include
>
> #include "ci.h"
>
> @@ -21,11 +23,22 @@
> #define HS_PHY_SEC_CTRL 0x0278
> # define HS_PHY_DIG_CLAMP_N BIT(16)
>
> +#define HS_PHY_GENCONFIG 0x009c
> +# define HS_PHY_TXFIFO_IDLE_FORCE_DISBIT(4)
> +
> +#define HS_PHY_GENCONFIG_2 0x00a0
> +# define HS_PHY_SESS_VLD_CTRL_EN BIT(7)
> +# define HS_PHY_ULPI_TX_PKT_EN_CLR_FIX BIT(19)
> +
> +#define HSPHY_SESS_VLD_CTRL BIT(25)
> +
Keep alignment please.
> struct ci_hdrc_msm {
> struct platform_device *ci;
> struct clk *core_clk;
> struct clk *iface_clk;
> + struct extcon_dev *vbus_edev;
> bool secondary_phy;
> + bool hsic;
> void __iomem *base;
> };
>
> @@ -39,9 +52,26 @@ static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci,
> unsigned event)
> dev_dbg(dev, "CI_HDRC_CONTROLLER_RESET_EVENT received\n");
> /* use AHB transactor, allow posted data writes */
> hw_write_id_reg(ci, HS_PHY_AHB_MODE, 0x, 0x8);
> + /* workaround for rx buffer collision issue */
> + hw_write_id_reg(ci, HS_PHY_GENCONFIG,
> + HS_PHY_TXFIFO_IDLE_FORCE_DIS, 0);
> +
> if (msm_ci->secondary_phy)
> hw_write_id_reg(ci, HS_PHY_SEC_CTRL, HS_PHY_DIG_CLAMP_N,
> HS_PHY_DIG_CLAMP_N);
> +
> + if (!msm_ci->hsic)
> + hw_write_id_reg(ci, HS_PHY_GENCONFIG_2,
> + HS_PHY_ULPI_TX_PKT_EN_CLR_FIX, 0);
> +
> + if (msm_ci->vbus_edev) {
> + hw_write_id_reg(ci, HS_PHY_GENCONFIG_2,
> + HS_PHY_SESS_VLD_CTRL_EN,
> + HS_PHY_SESS_VLD_CTRL_EN);
> + hw_write(ci, OP_USBCMD, HSPHY_SESS_VLD_CTRL,
> + HSPHY_SESS_VLD_CTRL);
> +
> + }
> break;
> default:
> dev_dbg(dev, "unknown ci_hdrc event\n");
> @@ -112,6 +142,7 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
> void __iomem *base;
> resource_size_t size;
> int ret;
> + struct device_node *ulpi_node, *phy_node;
>
> dev_dbg(&pdev->dev, "ci_hdrc_msm_probe\n");
>
> @@ -141,6 +172,13 @@ static int ci_hdrc_msm_probe(struct platform_device
> *pdev)
> if (!base)
> return -ENOMEM;
>
> + ci->vbus_edev = extcon_get_edev_by_phandle(&pdev->dev, 0);
> + if (IS_ERR(ci->vbus_edev)) {
> + if (PTR_ERR(ci->vbus_edev) != -ENODEV)
> + return PTR_ERR(ci->vbus_edev);
> + ci->vbus_edev = NULL;
> + }
> +
Why not using ci->platdata->vbus_extcon directly?
> reset_control_assert(reset);
> usleep_range(1, 12000);
> reset_control_deassert(reset);
> @@ -157,6 +195,14 @@ static int ci_hdrc_msm_probe(struct platform_device
> *pdev)
> if (ret)
> goto err_mux;
>
> + ulpi_node = of_find_node_by_name(pdev->dev.of_node, "ulpi");
> + if (ulpi_node) {
> + phy_node = of_get_next_available_child(ulpi_node, NULL);
> + ci->hsic = of_device_is_compatible(phy_node,
> "qcom,usb-hsic-phy");
> + of_node_put(phy_node);
> + }
> + of_node_put(ulpi_node);
> +
Just confirm with you that ci->platdata->phy_mode is not enough?
> plat_ci = ci_hdrc_add_device(&pdev->dev, pdev->resource,
>pdev->num_resources,
> &ci_hdrc_msm_platdata);
> if (IS_ERR(plat_ci)) {
> --
> 2.9.0.rc2.8.ga28705d
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@v
Re: [PATCH v12 2/4] gadget: Support for the usb charger framework
Hi,
Baolin Wang writes:
> For supporting the usb charger, it adds the usb_charger_init() and
> usb_charger_exit() functions for usb charger initialization and exit.
>
> It will report to the usb charger when the gadget state is changed,
> then the usb charger can do the power things.
>
> Signed-off-by: Baolin Wang
> Reviewed-by: Li Jun
> Tested-by: Li Jun
Before anything, I must say that I really liked this patch. It's
minimaly invasive to udc core and does all the necessary changes. If it
wasn't for the extra charger class, this would've been perfect.
Can't you just tie a charger to a UDC and avoid the charger class
completely?
> static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget,
> unsigned mA)
> {
> + if (gadget->charger)
I guess you could do this check inside
usb_gadget_set_cur_limit_by_type() itself.
>>>
>>> We will access the 'gadget->charger->type' member when issuing
>>> usb_gadget_set_cur_limit_by_type(), so I think I should leave the
>>> check here in next new version.
>>
>> Here's what I mean:
>>
>> int usb_charger_set_cur_limit(struct usb_gadget *gadget, unsigned int mA)
>> {
>> struct usb_charger *charger;
>> enum usb_charger_type type;
>>
>> if (!gadget->charger)
>> return 0;
>>
>> charger = gadget->charger;
>> type = charger->type;
>>
>> return __usb_charger_set_cur_limit(charger, type, mA);
>> }
>
> But that means we need to export both 'usb_charger_set_cur_limit()'
> function and '__usb_charger_set_cur_limit()' function in charger.c
> file. Cause some user may want to set the current limitation by one
> charger type parameter (may be not from charger->type), like by
> issuing '__usb_charger_set_cur_limit(charger, SDP_TYPE, mA)'. How do
> you think about this situation? Thanks.
if we have that requirement, that's totally fine. Just rename
__usb_charger_set_cur_limit() back to
_usb_charger_set_cur_limit_by_type() and expose both. But
set_cur_limit_by_type can assume its arguments are valid at all times.
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH v12 2/4] gadget: Support for the usb charger framework
On 29 June 2016 at 16:34, Felipe Balbi wrote:
>
> Hi,
>
> Baolin Wang writes:
>> For supporting the usb charger, it adds the usb_charger_init() and
>> usb_charger_exit() functions for usb charger initialization and exit.
>>
>> It will report to the usb charger when the gadget state is changed,
>> then the usb charger can do the power things.
>>
>> Signed-off-by: Baolin Wang
>> Reviewed-by: Li Jun
>> Tested-by: Li Jun
>
> Before anything, I must say that I really liked this patch. It's
> minimaly invasive to udc core and does all the necessary changes. If it
> wasn't for the extra charger class, this would've been perfect.
>
> Can't you just tie a charger to a UDC and avoid the charger class
> completely?
>
>> static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget,
>> unsigned mA)
>> {
>> + if (gadget->charger)
>
> I guess you could do this check inside
> usb_gadget_set_cur_limit_by_type() itself.
We will access the 'gadget->charger->type' member when issuing
usb_gadget_set_cur_limit_by_type(), so I think I should leave the
check here in next new version.
>>>
>>> Here's what I mean:
>>>
>>> int usb_charger_set_cur_limit(struct usb_gadget *gadget, unsigned int mA)
>>> {
>>> struct usb_charger *charger;
>>> enum usb_charger_type type;
>>>
>>> if (!gadget->charger)
>>> return 0;
>>>
>>> charger = gadget->charger;
>>> type = charger->type;
>>>
>>> return __usb_charger_set_cur_limit(charger, type, mA);
>>> }
>>
>> But that means we need to export both 'usb_charger_set_cur_limit()'
>> function and '__usb_charger_set_cur_limit()' function in charger.c
>> file. Cause some user may want to set the current limitation by one
>> charger type parameter (may be not from charger->type), like by
>> issuing '__usb_charger_set_cur_limit(charger, SDP_TYPE, mA)'. How do
>> you think about this situation? Thanks.
>
> if we have that requirement, that's totally fine. Just rename
> __usb_charger_set_cur_limit() back to
> _usb_charger_set_cur_limit_by_type() and expose both. But
> set_cur_limit_by_type can assume its arguments are valid at all times.
Make sense. I'll fix this issue in v14 version. Thanks.
--
Baolin.wang
Best Regards
--
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: [PATCHv3 1/2] usb: USB Type-C connector class
On Mon, Jun 27, 2016 at 5:43 PM, Heikki Krogerus wrote: > Hi, > > On Mon, Jun 27, 2016 at 03:51:08PM +0530, Rajaram R wrote: >> May be I am missing user or usage of the driver.. I see this driver is >> providing limited information of the Type-C connectors or the port >> partner > > Yes, this interface can't provide directly information received from > PD commands like Discover Identity. We will have to present the > partners even when USB PD is not supported and in a consistent > fashion. Some details will be available in any case indirectly. Like > if there are modes, there will be devices presenting them, and the > product type in case of partners will be the partner type. Agree. What is the end use of this driver? IMO end use case will decide what attributes to be shared. Since we are terming this as a universal representation for user space we may need to expose details such as Discovery details say Vendor ID, Product ID, Super Speed support etc which are not related to alt mode. In the legacy drivers complete descriptors of the device is available for user space to build applications. > > But there are a couple of attributes I have been thinking about adding > for the partners: > > supported_data_roles > supports_usb_power_delivery > > The supported data roles would respond bits 30 and 31 of the ID Header > VDO. But when the partner does not support USB PD, we will have to > report "unknown" in it. > > Oliver, Guenter! How do you guys feel about those? Is there any use > for them? > > >> On Mon, Jun 27, 2016 at 3:21 PM, Heikki Krogerus >> wrote: >> > On Fri, Jun 24, 2016 at 07:54:12PM +0530, Rajaram R wrote: >> >> On Tue, Jun 21, 2016 at 8:21 PM, Heikki Krogerus >> >> wrote: >> >> > The purpose of USB Type-C connector class is to provide >> >> > unified interface for the user space to get the status and >> >> > basic information about USB Type-C connectors on a system, >> >> >> >> Since we are defining this is as a unified interface for user space, >> >> will the interface include identity details of local port and peer. >> >> Or am I over looking something ? >> > >> > By peer, do you mean the partners? Sorry but could you elaborate the >> > question? > > > Thanks, > > -- > heikki -- 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 v2] usb: gadget: rndis: free response queue during REMOTE_NDIS_RESET_MSG
From: Xerox Lin
When rndis data transfer is in progress, some Windows7 Host PC is not
sending the GET_ENCAPSULATED_RESPONSE command for receiving the response
for the previous SEND_ENCAPSULATED_COMMAND processed.
The rndis function driver appends each response for the
SEND_ENCAPSULATED_COMMAND in a queue. As the above process got corrupted,
the Host sends a REMOTE_NDIS_RESET_MSG command to do a soft-reset.
As the rndis response queue is not freed, the previous response is sent
as a part of this REMOTE_NDIS_RESET_MSG's reset response and the Host
block any more Rndis transfers.
Hence free the rndis response queue as a part of this soft-reset so that
the correct response for REMOTE_NDIS_RESET_MSG is sent properly during the
response command.
Signed-off-by: Rajkumar Raghupathy
Signed-off-by: Xerox Lin
[AmitP: Cherry-picked this patch and folded other relevant
fixes from Android common kernel android-4.4]
Signed-off-by: Amit Pundir
---
v2: Sanitized the name of original author. Dropped his email address from
the list of reviewers, since it is no longer valid.
v1: Cherry picked this usb tethering fix from AOSP kernel/common/android-4.4
tree and to make sure it doesn't get overlooked, submiting it for review
and comment.
drivers/usb/gadget/function/rndis.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/usb/gadget/function/rndis.c
b/drivers/usb/gadget/function/rndis.c
index 943c21a..ab6ac1b 100644
--- a/drivers/usb/gadget/function/rndis.c
+++ b/drivers/usb/gadget/function/rndis.c
@@ -680,6 +680,12 @@ static int rndis_reset_response(struct rndis_params
*params,
{
rndis_reset_cmplt_type *resp;
rndis_resp_t *r;
+ u8 *xbuf;
+ u32 length;
+
+ /* drain the response queue */
+ while ((xbuf = rndis_get_next_response(params, &length)))
+ rndis_free_response(params, xbuf);
r = rndis_add_response(params, sizeof(rndis_reset_cmplt_type));
if (!r)
--
2.7.4
--
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 20/21] phy: Add support for Qualcomm's USB HSIC phy
On 06/28/2016 11:58 PM, Stephen Boyd wrote: > Quoting Neil Armstrong (2016-06-28 01:49:37) >> On 06/26/2016 09:28 AM, Stephen Boyd wrote: >>> + uphy->cal_sleep_clk = clk = devm_clk_get(&ulpi->dev, "cal_sleep"); >>> + if (IS_ERR(clk)) >>> + return PTR_ERR(clk); >> >> Hi Stephen, >> >> In the bindings the cal_sleep is marked optional, and I think should be >> since AFAIK >> it's not present on MDM9615 for example. > > The cal_sleep clk is just the sleep clk then (should be a board clk in > DT). Sometimes there's a gate in GCC to allow us to turn it off, other > times there isn't. Either way, it's always wired there so I'll update > the binding to say it isn't optional. Sorry I don't understand ! What should I do if GCC does not provide a gate here ? And looking at the driver, it could be optional. > >> >> Also MDM9615 HSIC requires "core", "alt-core", "phy", "cal" and "iface" >> clocks. >> I assume "core" can be attributed to the main chipidea node, but I think >> "alt-core" and "iface" should be also optionnal. > > Looking at the downstream sources I see this: > > "core_clk" -> usb_hsic_sys_clk > "iface_clk"-> usb_hsic_p_clk > "alt_core_clk" -> usb_hsic_xcvr_clk > > "cal_clk" -> usb_hsic_hsio_cal_clk > "phy_clk" -> usb_hsic_clk > > "core_clk" would be the core clk in ci_hdrc_msm. "iface_clk" would be > the iface clk in ci_hdrc_msm. "cal_clk" would be the cal clk in the hsic > phy and "phy_clk" would be the phy clk in the hsic phy. > > That leaves alt_core_clk which seems to be a clock that needs to be on > during the reset assert/deassert and possibly for LPM and USB1.1 FS > modes. Sometimes it's referred to as the "housekeeping" clk. Due to the > way resets are done on msm8974 and later SoCs it looks like this clk was > removed. I can make this an optional clk in the ci_hdrc_msm driver, or > we can have two versions of the ci_hdrc_msm compatible string, one for a > device that has the housekeeping clk and one for the device that > doesn't. Having it optional would be the best solution I think. > >> >> Finally, it misses an optional reset line AFAIK mandatory on MDM9615. >> > > From what I can tell downstream, all those clks point to the same bit 0 > of HSIC_RESET register? So there isn't any phy reset, just the chipidea > controller wrapper reset bit, which should go into the wrapper node? Ok, makes sense. -- 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 4/7] reset: Supply *_shared variant calls when using *_optional APIs
Am Mittwoch, den 29.06.2016, 09:06 +0100 schrieb Lee Jones:
> On Wed, 29 Jun 2016, Philipp Zabel wrote:
> > Am Dienstag, den 28.06.2016, 09:56 +0100 schrieb Lee Jones:
> > > Philipp,
> > >
> > > I need this to go into the -rcs too.
> > >
> > > Can I add it with your Ack please?
> >
> > I have already added your patches to my branch, and I intend to send a
> > pull request for it tomorrow. I am afraid applying this patch in another
> > branch would create a merge conflict with
> > git://git.pengutronix.de/git/pza/linux.git reset/next
> > in arm-soc. If that is not the case, go ahead. Otherwise I could provide
> > a tag at commit 2485394d9e0b ("reset: TRIVIAL: Add line break at same
> > place for similar APIs") for you to merge.
>
> Yes please. If you could tag a branch containing commit:
>
> reset: Supply *_shared variant calls when using *_optional APIs
>
> ... that would be perfect. Would you mind sending me the tag name
> ASAP please, since I am ready to send my pull-request to Linus.
git://git.pengutronix.de/git/pza/linux.git tags/reset-shared-optional
regards
Philipp
--
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 4/7] reset: Supply *_shared variant calls when using *_optional APIs
On Wed, 29 Jun 2016, Philipp Zabel wrote:
> Am Mittwoch, den 29.06.2016, 09:06 +0100 schrieb Lee Jones:
> > On Wed, 29 Jun 2016, Philipp Zabel wrote:
> > > Am Dienstag, den 28.06.2016, 09:56 +0100 schrieb Lee Jones:
> > > > Philipp,
> > > >
> > > > I need this to go into the -rcs too.
> > > >
> > > > Can I add it with your Ack please?
> > >
> > > I have already added your patches to my branch, and I intend to send a
> > > pull request for it tomorrow. I am afraid applying this patch in another
> > > branch would create a merge conflict with
> > > git://git.pengutronix.de/git/pza/linux.git reset/next
> > > in arm-soc. If that is not the case, go ahead. Otherwise I could provide
> > > a tag at commit 2485394d9e0b ("reset: TRIVIAL: Add line break at same
> > > place for similar APIs") for you to merge.
> >
> > Yes please. If you could tag a branch containing commit:
> >
> > reset: Supply *_shared variant calls when using *_optional APIs
> >
> > ... that would be perfect. Would you mind sending me the tag name
> > ASAP please, since I am ready to send my pull-request to Linus.
>
> git://git.pengutronix.de/git/pza/linux.git tags/reset-shared-optional
Ah! If I rebase on that tag, I will also take in all of the patches
between it and v4.7-rc1, which is 14 commits. Is there any way you
can place it onto it's own branch, then we both merge it into our
respective trees?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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: otg-fsm: Cancel HNP polling work when not used
On Mon, Jun 27, 2016 at 06:18:27PM -0700, Stephen Boyd wrote:
> We setup the HNP polling worker, but we never stop it. The OTG
> state machine can go round and round and keep reinitializing the
> worker even while it's actively running. That's bad, and debug
> objects catches it. Fix this by canceling the work when we leave
> the A_HOST or B_HOST states.
>
> [otg_set_state] Set state: a_wait_bcon
> usb 2-1: USB disconnect, device number 2
> [otg_statemachine] quit statemachine, changed = 1
> [otg_set_state] Set state: a_host
>
> [otg_statemachine] quit statemachine, changed = 1
> usb 2-1: new low-speed USB device number 3 using ci_hdrc
> usb 2-1: New USB device found, idVendor=03f0, idProduct=134a
> usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0
> usb 2-1: Product: HP USB Optical Mouse
> usb 2-1: Manufacturer: PixArt
> input: PixArt HP USB Optical Mouse as
> /devices/platform/soc/f9a55000.usb/ci_hdrc.0/usb2/2-1/2-1:1.0/0003:03F0:134A.0002/input/input1
> hid-generic 0003:03F0:134A.0002: input: USB HID v1.11 Mouse [PixArt HP USB
> Optical Mouse] on usb-ci_hdrc.0-1/input0
> [otg_set_state] Set state: a_wait_bcon
> usb 2-1: USB disconnect, device number 3
> [otg_statemachine] quit statemachine, changed = 1
> [otg_set_state] Set state: a_host
>
> [ cut here ]
> WARNING: CPU: 2 PID: 95 at lib/debugobjects.c:263 debug_print_object+0x98/0xc0
> ODEBUG: init active (active state 0) object type: timer_list hint:
> delayed_work_timer_fn+0x0/0x2c
> Modules linked in: phy_qcom_usb_hsic phy_qcom_usb_hs ci_hdrc_msm ci_hdrc
> CPU: 2 PID: 95 Comm: kworker/u8:1 Not tainted
> 4.7.0-rc1-00043-g1f22f3b65c44-dirty #442
> Hardware name: Qualcomm (Flattened Device Tree)
> Workqueue: ci_otg ci_otg_work [ci_hdrc]
> [] (unwind_backtrace) from [] (show_stack+0x20/0x24)
> [] (show_stack) from [] (dump_stack+0x7c/0x9c)
> [] (dump_stack) from [] (__warn+0xe4/0x110)
> [] (__warn) from [] (warn_slowpath_fmt+0x48/0x50)
> [] (warn_slowpath_fmt) from []
> (debug_print_object+0x98/0xc0)
> [] (debug_print_object) from []
> (__debug_object_init+0xcc/0x3bc)
> [] (__debug_object_init) from []
> (debug_object_init+0x24/0x2c)
> [] (debug_object_init) from [] (init_timer_key+0x24/0x120)
> [] (init_timer_key) from []
> (otg_start_hnp_polling+0x7c/0xbc)
> [] (otg_start_hnp_polling) from []
> (otg_set_state+0x740/0xc20)
> [] (otg_set_state) from [] (otg_statemachine+0x47c/0x4ac)
> [] (otg_statemachine) from [] (ci_otg_fsm_work+0x48/0x1a0
> [ci_hdrc])
> [] (ci_otg_fsm_work [ci_hdrc]) from []
> (ci_otg_work+0xd4/0x218 [ci_hdrc])
> [] (ci_otg_work [ci_hdrc]) from []
> (process_one_work+0x154/0x4b4)
> [] (process_one_work) from [] (worker_thread+0x38/0x4d0)
> [] (worker_thread) from [] (kthread+0xe8/0x104)
> [] (kthread) from [] (ret_from_fork+0x14/0x3c)
>
> Cc: Li Jun
> Cc: Greg Kroah-Hartman
> Fixes: ae57e97a9521 ("usb: common: otg-fsm: add HNP polling support")
> Signed-off-by: Stephen Boyd
> ---
> drivers/usb/common/usb-otg-fsm.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/usb/common/usb-otg-fsm.c
> b/drivers/usb/common/usb-otg-fsm.c
> index 9059b7dc185e..73eec8c12235 100644
> --- a/drivers/usb/common/usb-otg-fsm.c
> +++ b/drivers/usb/common/usb-otg-fsm.c
> @@ -61,6 +61,18 @@ static int otg_set_protocol(struct otg_fsm *fsm, int
> protocol)
> return 0;
> }
>
> +static void otg_stop_hnp_polling(struct otg_fsm *fsm)
> +{
> + /*
> + * The memory of host_req_flag should be allocated by
> + * controller driver, otherwise, hnp polling is not started.
> + */
> + if (!fsm->host_req_flag)
> + return;
> +
> + cancel_delayed_work_sync(&fsm->hnp_polling_work);
> +}
> +
> /* Called when leaving a state. Do state clean up jobs here */
> static void otg_leave_state(struct otg_fsm *fsm, enum usb_otg_state
> old_state)
> {
> @@ -84,6 +96,7 @@ static void otg_leave_state(struct otg_fsm *fsm, enum
> usb_otg_state old_state)
> fsm->b_ase0_brst_tmout = 0;
> break;
> case OTG_STATE_B_HOST:
> + otg_stop_hnp_polling(fsm);
> break;
> case OTG_STATE_A_IDLE:
> fsm->adp_prb = 0;
> @@ -97,6 +110,7 @@ static void otg_leave_state(struct otg_fsm *fsm, enum
> usb_otg_state old_state)
> fsm->a_wait_bcon_tmout = 0;
> break;
> case OTG_STATE_A_HOST:
> + otg_stop_hnp_polling(fsm);
> otg_del_timer(fsm, A_WAIT_ENUM);
> break;
> case OTG_STATE_A_SUSPEND:
> --
It introduces circular locking after applying it, otg_statemachine calls
otg_leave_state, and otg_leave_state calls otg_statemachine again due
to flush work, see below dump:
I suggest moving initialization/flush hnp polling work to chipidea driver.
[ 183.086987] ==
[ 183.093183] [ INFO: possible circular locking dependency detected ]
[ 183.099471] 4.7.0-rc4-0001
Re: [PATCHv3 1/2] usb: USB Type-C connector class
On Wed, Jun 29, 2016 at 02:21:10PM +0530, Rajaram R wrote: > On Mon, Jun 27, 2016 at 5:43 PM, Heikki Krogerus > wrote: > > Hi, > > > > On Mon, Jun 27, 2016 at 03:51:08PM +0530, Rajaram R wrote: > >> May be I am missing user or usage of the driver.. I see this driver is > >> providing limited information of the Type-C connectors or the port > >> partner > > > > Yes, this interface can't provide directly information received from > > PD commands like Discover Identity. We will have to present the > > partners even when USB PD is not supported and in a consistent > > fashion. Some details will be available in any case indirectly. Like > > if there are modes, there will be devices presenting them, and the > > product type in case of partners will be the partner type. > > Agree. What is the end use of this driver? IMO end use case will > decide what attributes to be shared. Since we are terming this as a > universal representation for user space we may need to expose details > such as Discovery details say Vendor ID, Product ID, Super Speed > support etc which are not related to alt mode. In the legacy drivers > complete descriptors of the device is available for user space to > build applications. The details about the USB connection are out side the scope the this class, and in most cases the port driver will not even have them at their disposal. We can determine that the connector is in USB mode, and that's about it. But those details will in any case be exposed by the USB subsystem, so why should we duplicate them? The user space has been so far relying on getting the details from the normal interfaces the USB subsystem provides and that should not change. Thanks, -- heikki -- 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: [PATCHv3 1/2] usb: USB Type-C connector class
On Wed, Jun 29, 2016 at 4:00 PM, Heikki Krogerus wrote: > On Wed, Jun 29, 2016 at 02:21:10PM +0530, Rajaram R wrote: >> On Mon, Jun 27, 2016 at 5:43 PM, Heikki Krogerus >> wrote: >> > Hi, >> > >> > On Mon, Jun 27, 2016 at 03:51:08PM +0530, Rajaram R wrote: >> >> May be I am missing user or usage of the driver.. I see this driver is >> >> providing limited information of the Type-C connectors or the port >> >> partner >> > >> > Yes, this interface can't provide directly information received from >> > PD commands like Discover Identity. We will have to present the >> > partners even when USB PD is not supported and in a consistent >> > fashion. Some details will be available in any case indirectly. Like >> > if there are modes, there will be devices presenting them, and the >> > product type in case of partners will be the partner type. >> >> Agree. What is the end use of this driver? IMO end use case will >> decide what attributes to be shared. Since we are terming this as a >> universal representation for user space we may need to expose details >> such as Discovery details say Vendor ID, Product ID, Super Speed >> support etc which are not related to alt mode. In the legacy drivers >> complete descriptors of the device is available for user space to >> build applications. > > The details about the USB connection are out side the scope the this > class, and in most cases the port driver will not even have them at > their disposal. We can determine that the connector is in USB mode, > and that's about it. > > But those details will in any case be exposed by the USB subsystem, so > why should we duplicate them? The user space has been so far relying > on getting the details from the normal interfaces the USB subsystem > provides and that should not change. Apologize for bringing in USB example. I used it to as an example to say that complete device details are exposed to user space by other drivers. Sticking to the current topic/context more details of Type-C port/partner, a detailed information(with restrictions) will help build more user applications. > > > Thanks, > > -- > heikki -- 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: otg-fsm: Cancel HNP polling work when not used
Hi
> -Original Message-
> From: Peter Chen [mailto:hzpeterc...@gmail.com]
> Sent: Wednesday, June 29, 2016 5:44 PM
> To: Stephen Boyd
> Cc: Peter Chen ; Felipe Balbi ;
> linux-arm-ker...@lists.infradead.org; linux-ker...@vger.kernel.org; linux-
> u...@vger.kernel.org; Jun Li ; Greg Kroah-Hartman
>
> Subject: Re: [PATCH] usb: otg-fsm: Cancel HNP polling work when not used
>
> On Mon, Jun 27, 2016 at 06:18:27PM -0700, Stephen Boyd wrote:
> > We setup the HNP polling worker, but we never stop it. The OTG state
> > machine can go round and round and keep reinitializing the worker even
> > while it's actively running. That's bad, and debug objects catches it.
> > Fix this by canceling the work when we leave the A_HOST or B_HOST
> > states.
> >
> > [otg_set_state] Set state: a_wait_bcon usb 2-1: USB disconnect,
> > device number 2 [otg_statemachine] quit statemachine, changed = 1
> > [otg_set_state] Set state: a_host
> > [otg_statemachine] quit statemachine, changed = 1 usb 2-1: new
> > low-speed USB device number 3 using ci_hdrc usb 2-1: New USB device
> > found, idVendor=03f0, idProduct=134a usb 2-1: New USB device strings:
> > Mfr=1, Product=2, SerialNumber=0 usb 2-1: Product: HP USB Optical
> > Mouse usb 2-1: Manufacturer: PixArt
> > input: PixArt HP USB Optical Mouse as
> > /devices/platform/soc/f9a55000.usb/ci_hdrc.0/usb2/2-1/2-1:1.0/0003:03F
> > 0:134A.0002/input/input1 hid-generic 0003:03F0:134A.0002: input: USB
> > HID v1.11 Mouse [PixArt HP USB Optical Mouse] on
> > usb-ci_hdrc.0-1/input0 [otg_set_state] Set state: a_wait_bcon usb
> > 2-1: USB disconnect, device number 3 [otg_statemachine] quit
> > statemachine, changed = 1 [otg_set_state] Set state: a_host > Polling started> [ cut here ]
> > WARNING: CPU: 2 PID: 95 at lib/debugobjects.c:263
> > debug_print_object+0x98/0xc0
> > ODEBUG: init active (active state 0) object type: timer_list hint:
> > delayed_work_timer_fn+0x0/0x2c Modules linked in: phy_qcom_usb_hsic
> > phy_qcom_usb_hs ci_hdrc_msm ci_hdrc
> > CPU: 2 PID: 95 Comm: kworker/u8:1 Not tainted
> > 4.7.0-rc1-00043-g1f22f3b65c44-dirty #442 Hardware name: Qualcomm
> > (Flattened Device Tree)
> > Workqueue: ci_otg ci_otg_work [ci_hdrc] []
> > (unwind_backtrace) from [] (show_stack+0x20/0x24)
> > [] (show_stack) from [] (dump_stack+0x7c/0x9c)
> > [] (dump_stack) from [] (__warn+0xe4/0x110)
> > [] (__warn) from [] (warn_slowpath_fmt+0x48/0x50)
> > [] (warn_slowpath_fmt) from []
> > (debug_print_object+0x98/0xc0) [] (debug_print_object) from
> > [] (__debug_object_init+0xcc/0x3bc) []
> > (__debug_object_init) from [] (debug_object_init+0x24/0x2c)
> > [] (debug_object_init) from []
> > (init_timer_key+0x24/0x120) [] (init_timer_key) from
> > [] (otg_start_hnp_polling+0x7c/0xbc) []
> > (otg_start_hnp_polling) from [] (otg_set_state+0x740/0xc20)
> > [] (otg_set_state) from []
> > (otg_statemachine+0x47c/0x4ac) [] (otg_statemachine) from
> > [] (ci_otg_fsm_work+0x48/0x1a0 [ci_hdrc]) []
> > (ci_otg_fsm_work [ci_hdrc]) from [] (ci_otg_work+0xd4/0x218
> > [ci_hdrc]) [] (ci_otg_work [ci_hdrc]) from []
> > (process_one_work+0x154/0x4b4) [] (process_one_work) from
> > [] (worker_thread+0x38/0x4d0) [] (worker_thread)
> > from [] (kthread+0xe8/0x104) [] (kthread) from
> > [] (ret_from_fork+0x14/0x3c)
> >
> > Cc: Li Jun
> > Cc: Greg Kroah-Hartman
> > Fixes: ae57e97a9521 ("usb: common: otg-fsm: add HNP polling support")
> > Signed-off-by: Stephen Boyd
> > ---
> > drivers/usb/common/usb-otg-fsm.c | 14 ++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/usb/common/usb-otg-fsm.c
> > b/drivers/usb/common/usb-otg-fsm.c
> > index 9059b7dc185e..73eec8c12235 100644
> > --- a/drivers/usb/common/usb-otg-fsm.c
> > +++ b/drivers/usb/common/usb-otg-fsm.c
> > @@ -61,6 +61,18 @@ static int otg_set_protocol(struct otg_fsm *fsm, int
> protocol)
> > return 0;
> > }
> >
> > +static void otg_stop_hnp_polling(struct otg_fsm *fsm) {
> > + /*
> > +* The memory of host_req_flag should be allocated by
> > +* controller driver, otherwise, hnp polling is not started.
> > +*/
> > + if (!fsm->host_req_flag)
> > + return;
> > +
> > + cancel_delayed_work_sync(&fsm->hnp_polling_work);
> > +}
> > +
> > /* Called when leaving a state. Do state clean up jobs here */
> > static void otg_leave_state(struct otg_fsm *fsm, enum usb_otg_state
> > old_state) { @@ -84,6 +96,7 @@ static void otg_leave_state(struct
> > otg_fsm *fsm, enum usb_otg_state old_state)
> > fsm->b_ase0_brst_tmout = 0;
> > break;
> > case OTG_STATE_B_HOST:
> > + otg_stop_hnp_polling(fsm);
> > break;
> > case OTG_STATE_A_IDLE:
> > fsm->adp_prb = 0;
> > @@ -97,6 +110,7 @@ static void otg_leave_state(struct otg_fsm *fsm, enum
> usb_otg_state old_state)
> > fsm->a_wait_bcon_tmout = 0;
> > break;
> > case OTG_STATE_A_HOST:
> > + otg_stop_hnp_polling(fsm);
> > ot
Re: [PATCHv3 1/2] usb: USB Type-C connector class
On Wed, Jun 29, 2016 at 04:21:49PM +0530, Rajaram R wrote: > On Wed, Jun 29, 2016 at 4:00 PM, Heikki Krogerus > wrote: > > On Wed, Jun 29, 2016 at 02:21:10PM +0530, Rajaram R wrote: > >> On Mon, Jun 27, 2016 at 5:43 PM, Heikki Krogerus > >> wrote: > >> > Hi, > >> > > >> > On Mon, Jun 27, 2016 at 03:51:08PM +0530, Rajaram R wrote: > >> >> May be I am missing user or usage of the driver.. I see this driver is > >> >> providing limited information of the Type-C connectors or the port > >> >> partner > >> > > >> > Yes, this interface can't provide directly information received from > >> > PD commands like Discover Identity. We will have to present the > >> > partners even when USB PD is not supported and in a consistent > >> > fashion. Some details will be available in any case indirectly. Like > >> > if there are modes, there will be devices presenting them, and the > >> > product type in case of partners will be the partner type. > >> > >> Agree. What is the end use of this driver? IMO end use case will > >> decide what attributes to be shared. Since we are terming this as a > >> universal representation for user space we may need to expose details > >> such as Discovery details say Vendor ID, Product ID, Super Speed > >> support etc which are not related to alt mode. In the legacy drivers > >> complete descriptors of the device is available for user space to > >> build applications. > > > > The details about the USB connection are out side the scope the this > > class, and in most cases the port driver will not even have them at > > their disposal. We can determine that the connector is in USB mode, > > and that's about it. > > > > But those details will in any case be exposed by the USB subsystem, so > > why should we duplicate them? The user space has been so far relying > > on getting the details from the normal interfaces the USB subsystem > > provides and that should not change. > > Apologize for bringing in USB example. I used it to as an example to > say that complete device details are exposed to user space by other > drivers. > > Sticking to the current topic/context more details of Type-C > port/partner, a detailed information(with restrictions) will help > build more user applications. IMO we are exposing more or less all relevant information about the ports that the USB Type-C specification defines. And for control we should also be providing mechanisms for all that the spec. defines, so mainly role swapping, and entering/exiting the altenate modes. If there is something missing, please point it out. Otherwise, I think we are providing everything we can. About the end user of the interface, I think Oliver knows more about that. But I would imagine that the use cases will be something like, for example, on systems that need prefer sertain roles, perhaps Host for example on some server systems, need to have something like a udev script to set the preferred role and/or attempt role swap if the other role (device) is initially given to a port after connection. Br, -- heikki -- 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: otg-fsm: Cancel HNP polling work when not used
On Wed, Jun 29, 2016 at 10:56:42AM +, Jun Li wrote:
> > > }
> > >
> > > +static void otg_stop_hnp_polling(struct otg_fsm *fsm) {
> > > + /*
> > > + * The memory of host_req_flag should be allocated by
> > > + * controller driver, otherwise, hnp polling is not started.
> > > + */
> > > + if (!fsm->host_req_flag)
> > > + return;
> > > +
> > > + cancel_delayed_work_sync(&fsm->hnp_polling_work);
> > > +}
> > > +
> > > /* Called when leaving a state. Do state clean up jobs here */
> > > static void otg_leave_state(struct otg_fsm *fsm, enum usb_otg_state
> > > old_state) { @@ -84,6 +96,7 @@ static void otg_leave_state(struct
> > > otg_fsm *fsm, enum usb_otg_state old_state)
> > > fsm->b_ase0_brst_tmout = 0;
> > > break;
> > > case OTG_STATE_B_HOST:
> > > + otg_stop_hnp_polling(fsm);
> > > break;
> > > case OTG_STATE_A_IDLE:
> > > fsm->adp_prb = 0;
> > > @@ -97,6 +110,7 @@ static void otg_leave_state(struct otg_fsm *fsm, enum
> > usb_otg_state old_state)
> > > fsm->a_wait_bcon_tmout = 0;
> > > break;
> > > case OTG_STATE_A_HOST:
> > > + otg_stop_hnp_polling(fsm);
> > > otg_del_timer(fsm, A_WAIT_ENUM);
> > > break;
> > > case OTG_STATE_A_SUSPEND:
> > > --
> >
> > It introduces circular locking after applying it, otg_statemachine calls
> > otg_leave_state, and otg_leave_state calls otg_statemachine again due to
> > flush work, see below dump:
>
> How did you trigger this locking issue? I did some simple tests with
> this patch but no problems found.
>
Just do srp repeat at b side.
counter=0
while [ 1 ]
do
./srp.sh 0
sleep 5
./srp.sh 1
sleep 5
counter=$(( $counter + 1 ))
echo "the counter is $counter"
done
root@imx6qdlsolo:~# cat srp.sh
echo $1 > /sys/bus/platform/devices/ci_hdrc.0/inputs/b_bus_req
> >
> > I suggest moving initialization/flush hnp polling work to chipidea driver.
> >
>
> Since the HNP polling is done in common driver completely, so I think
> it's not proper to move some of it into controller driver.
>
If we can keep one-shot operation well, like initialization and remove,
I have no idea to keep it like current way.
Peter
>
> > [ 183.086987] ==
> > [ 183.093183] [ INFO: possible circular locking dependency detected ]
> > [ 183.099471] 4.7.0-rc4-00012-gf1f333f-dirty #856 Not tainted
> > [ 183.105059] ---
> > [ 183.111341] kworker/0:2/114 is trying to acquire lock:
> > [ 183.116492] (&ci->fsm.lock){+.+.+.}, at: [<806118dc>]
> > otg_statemachine+0x20/0x470
> > [ 183.124199]
> > [ 183.124199] but task is already holding lock:
> > [ 183.130052] ((&(&fsm->hnp_polling_work)->work)){+.+...}, at:
> > [<80140368>] process_one_work+0x128/0x418 [ 183.139568] [ 183.139568]
> > which lock already depends on the new lock.
> > [ 183.139568]
> > [ 183.147765]
> > [ 183.147765] the existing dependency chain (in reverse order) is:
> > [ 183.155265]
> > -> #1 ((&(&fsm->hnp_polling_work)->work)){+.+...}:
> > [ 183.161371][<8013e97c>] flush_work+0x44/0x234
> > [ 183.166469][<801411a8>] __cancel_work_timer+0x98/0x1c8
> > [ 183.172347][<80141304>] cancel_delayed_work_sync+0x14/0x18
> > [ 183.178570][<80610ef8>] otg_set_state+0x290/0xc54
> > [ 183.184011][<806119d0>] otg_statemachine+0x114/0x470
> > [ 183.189712][<8060a590>] ci_otg_fsm_work+0x40/0x190
> > [ 183.195239][<806054d0>] ci_otg_work+0xcc/0x1e4
> > [ 183.200430][<801403d4>] process_one_work+0x194/0x418
> > [ 183.206136][<8014068c>] worker_thread+0x34/0x4fc
> > [ 183.211489][<80146d08>] kthread+0xdc/0xf8
> > [ 183.216238][<80108ab0>] ret_from_fork+0x14/0x24
> > [ 183.221514]
> > -> #0 (&ci->fsm.lock){+.+.+.}:
> > [ 183.225880][<8016ff94>] lock_acquire+0x78/0x98
> > [ 183.231062][<80947c18>] mutex_lock_nested+0x54/0x3ec
> > [ 183.236773][<806118dc>] otg_statemachine+0x20/0x470
> > [ 183.242388][<80611df4>] otg_hnp_polling_work+0xc8/0x1a4
> > [ 183.248352][<801403d4>] process_one_work+0x194/0x418
> > [ 183.254055][<8014068c>] worker_thread+0x34/0x4fc
> > [ 183.259409][<80146d08>] kthread+0xdc/0xf8
> > [ 183.264154][<80108ab0>] ret_from_fork+0x14/0x24
> > [ 183.269424]
> > [ 183.269424] other info that might help us debug this:
> > [ 183.269424]
> > [ 183.277451] Possible unsafe locking scenario:
> > [ 183.277451]
> > [ 183.283389]CPU0CPU1
> > [ 183.287931]
> > [ 183.292473] lock((&(&fsm->hnp_polling_work)->work));
> > [ 183.297665]lock(&ci->fsm.lock);
> > [ 183.303639]
> > lock((&(&fsm->hnp_polling_work)->work));
> > [ 183.311347] lock(&ci->fsm.lock);
> > [ 183.314801]
Re: [PATCH 17/21] usb: chipidea: msm: Make platform data driver local instead of global
On Sun, Jun 26, 2016 at 12:28:34AM -0700, Stephen Boyd wrote:
> If two devices are probed with this same driver, they'll share
> the same platform data structure, while the chipidea core layer
> writes and modifies it. This can lead to interesting results
> especially if one device is an OTG type chipidea controller and
> another is a host. Let's create a copy of this structure per each
> device instance so that odd things don't happen.
>
> Cc: Peter Chen
> Cc: Greg Kroah-Hartman
> Signed-off-by: Stephen Boyd
> ---
> drivers/usb/chipidea/ci_hdrc_msm.c | 19 ---
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c
> b/drivers/usb/chipidea/ci_hdrc_msm.c
> index cc6f9b0df9d5..fb4340f02c16 100644
> --- a/drivers/usb/chipidea/ci_hdrc_msm.c
> +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
> @@ -37,6 +37,7 @@ struct ci_hdrc_msm {
> struct clk *core_clk;
> struct clk *iface_clk;
> struct extcon_dev *vbus_edev;
> + struct ci_hdrc_platform_data pdata;
> bool secondary_phy;
> bool hsic;
> void __iomem *base;
> @@ -79,16 +80,6 @@ static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci,
> unsigned event)
> }
> }
>
> -static struct ci_hdrc_platform_data ci_hdrc_msm_platdata = {
> - .name = "ci_hdrc_msm",
> - .capoffset = DEF_CAPOFFSET,
> - .flags = CI_HDRC_REGS_SHARED |
> - CI_HDRC_DISABLE_STREAMING |
> - CI_HDRC_OVERRIDE_AHB_BURST,
> -
> - .notify_event = ci_hdrc_msm_notify_event,
> -};
> -
> static int ci_hdrc_msm_mux_phy(struct ci_hdrc_msm *ci,
> struct platform_device *pdev)
> {
> @@ -151,6 +142,12 @@ static int ci_hdrc_msm_probe(struct platform_device
> *pdev)
> return -ENOMEM;
> platform_set_drvdata(pdev, ci);
>
> + ci->pdata.name = "ci_hdrc_msm";
> + ci->pdata.capoffset = DEF_CAPOFFSET;
> + ci->pdata.flags = CI_HDRC_REGS_SHARED | CI_HDRC_DISABLE_STREAMING |
> + CI_HDRC_OVERRIDE_AHB_BURST;
> + ci->pdata.notify_event = ci_hdrc_msm_notify_event;
> +
> reset = devm_reset_control_get(&pdev->dev, "core");
> if (IS_ERR(reset))
> return PTR_ERR(reset);
> @@ -204,7 +201,7 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
> of_node_put(ulpi_node);
>
> plat_ci = ci_hdrc_add_device(&pdev->dev, pdev->resource,
> - pdev->num_resources,
> &ci_hdrc_msm_platdata);
> + pdev->num_resources, &ci->pdata);
> if (IS_ERR(plat_ci)) {
> dev_err(&pdev->dev, "ci_hdrc_add_device failed!\n");
> ret = PTR_ERR(plat_ci);
You can do something like ci_hdrc_usb2.c, it looks simpler.
--
Best Regards,
Peter Chen
--
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 13/21] usb: chipidea: msm: Allow core to get usb phy
On Wed, Jun 29, 2016 at 02:48:11PM +0800, Peter Chen wrote:
> On Sun, Jun 26, 2016 at 12:28:30AM -0700, Stephen Boyd wrote:
> > The chipidea core gets the usb phy and initializes the phy at the
> > right point now so we don't need to get the phy in this driver.
> >
> > Cc: Peter Chen
> > Cc: Greg Kroah-Hartman
> > Signed-off-by: Stephen Boyd
> > ---
> > drivers/usb/chipidea/ci_hdrc_msm.c | 21 -
> > 1 file changed, 21 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c
> > b/drivers/usb/chipidea/ci_hdrc_msm.c
> > index 430856ef1be3..07cccd24a87f 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_msm.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
> > @@ -24,15 +24,6 @@ static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci,
> > unsigned event)
> > dev_dbg(dev, "CI_HDRC_CONTROLLER_RESET_EVENT received\n");
> > /* use AHB transactor, allow posted data writes */
> > hw_write_id_reg(ci, HS_PHY_AHB_MODE, 0x, 0x8);
> > - usb_phy_init(ci->usb_phy);
> > - break;
> > - case CI_HDRC_CONTROLLER_STOPPED_EVENT:
> > - dev_dbg(dev, "CI_HDRC_CONTROLLER_STOPPED_EVENT received\n");
> > - /*
> > -* Put the phy in non-driving mode. Otherwise host
> > -* may not detect soft-disconnection.
> > -*/
> > - usb_phy_notify_disconnect(ci->usb_phy, USB_SPEED_UNKNOWN);
> > break;
> > default:
> > dev_dbg(dev, "unknown ci_hdrc event\n");
> > @@ -53,21 +44,9 @@ static struct ci_hdrc_platform_data ci_hdrc_msm_platdata
> > = {
> > static int ci_hdrc_msm_probe(struct platform_device *pdev)
> > {
> > struct platform_device *plat_ci;
> > - struct usb_phy *phy;
> >
> > dev_dbg(&pdev->dev, "ci_hdrc_msm_probe\n");
> >
> > - /*
> > -* OTG(PHY) driver takes care of PHY initialization, clock management,
> > -* powering up VBUS, mapping of registers address space and power
> > -* management.
> > -*/
> > - phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0);
> > - if (IS_ERR(phy))
> > - return PTR_ERR(phy);
> > -
> > - ci_hdrc_msm_platdata.usb_phy = phy;
> > -
> > plat_ci = ci_hdrc_add_device(&pdev->dev,
> > pdev->resource, pdev->num_resources,
> > &ci_hdrc_msm_platdata);
> > --
>
Wait, how about the UTMI PHY? You don't have a platform which needs
to get PHY through the phandle?
--
Best Regards,
Peter Chen
--
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 18/21] usb: chipidea: msm: Add reset controller for PHY POR bit
On Sun, Jun 26, 2016 at 12:28:35AM -0700, Stephen Boyd wrote:
> The MSM chipidea wrapper has two bits that are used to reset the
> first or second phy. Add support for these bits via the reset
> controller framework, so that phy drivers can reset their
> hardware at the right time during initialization.
>
> Cc: Peter Chen
> Cc: Greg Kroah-Hartman
> Signed-off-by: Stephen Boyd
> ---
> drivers/usb/chipidea/ci_hdrc_msm.c | 43
> +-
> 1 file changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c
> b/drivers/usb/chipidea/ci_hdrc_msm.c
> index fb4340f02c16..7d191928e55b 100644
> --- a/drivers/usb/chipidea/ci_hdrc_msm.c
> +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
> @@ -14,14 +14,17 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> #include "ci.h"
>
> #define HS_PHY_AHB_MODE 0x0098
> +#define HS_PHY_CTRL 0x0240
> #define HS_PHY_SEC_CTRL 0x0278
> # define HS_PHY_DIG_CLAMP_N BIT(16)
> +# define HS_PHY_POR_ASSERT BIT(0)
>
> #define HS_PHY_GENCONFIG 0x009c
> # define HS_PHY_TXFIFO_IDLE_FORCE_DISBIT(4)
> @@ -38,11 +41,38 @@ struct ci_hdrc_msm {
> struct clk *iface_clk;
> struct extcon_dev *vbus_edev;
> struct ci_hdrc_platform_data pdata;
> + struct reset_controller_dev rcdev;
> bool secondary_phy;
> bool hsic;
> void __iomem *base;
> };
>
> +static int
> +ci_hdrc_msm_por_reset(struct reset_controller_dev *r, unsigned long id)
> +{
> + struct ci_hdrc_msm *ci_msm = container_of(r, struct ci_hdrc_msm, rcdev);
> + void __iomem *addr = ci_msm->base;
Like I mentioned at previous email, you can use vendor base for
0x200.
> + u32 val;
> +
> + if (id)
> + addr += HS_PHY_SEC_CTRL;
> + else
> + addr += HS_PHY_CTRL;
> +
> + val = readl_relaxed(addr);
> + val |= HS_PHY_POR_ASSERT;
> + writel_relaxed(val, addr);
> + udelay(12);
> + val &= ~HS_PHY_POR_ASSERT;
> + writel(val, addr);
> +
> + return 0;
> +}
> +
> +static const struct reset_control_ops ci_hdrc_msm_reset_ops = {
> + .reset = ci_hdrc_msm_por_reset,
> +};
> +
> static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
> {
> struct device *dev = ci->dev->parent;
> @@ -176,13 +206,21 @@ static int ci_hdrc_msm_probe(struct platform_device
> *pdev)
> ci->vbus_edev = NULL;
> }
>
> + ci->rcdev.owner = THIS_MODULE;
> + ci->rcdev.ops = &ci_hdrc_msm_reset_ops;
> + ci->rcdev.of_node = pdev->dev.of_node;
> + ci->rcdev.nr_resets = 2;
> + ret = reset_controller_register(&ci->rcdev);
> + if (ret)
> + return ret;
> +
> reset_control_assert(reset);
> usleep_range(1, 12000);
> reset_control_deassert(reset);
>
> ret = clk_prepare_enable(ci->core_clk);
> if (ret)
> - return ret;
> + goto err_core;
>
> ret = clk_prepare_enable(ci->iface_clk);
> if (ret)
> @@ -220,6 +258,8 @@ err_mux:
> clk_disable_unprepare(ci->iface_clk);
> err_iface:
> clk_disable_unprepare(ci->core_clk);
> +err_core:
> + reset_controller_unregister(&ci->rcdev);
> return ret;
> }
>
> @@ -232,6 +272,7 @@ static int ci_hdrc_msm_remove(struct platform_device
> *pdev)
> ci_hdrc_remove_device(ci->ci);
> clk_disable_unprepare(ci->iface_clk);
> clk_disable_unprepare(ci->core_clk);
> + reset_controller_unregister(&ci->rcdev);
>
> return 0;
> }
> --
> 2.9.0.rc2.8.ga28705d
>
> --
> 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
--
Best Regards,
Peter Chen
--
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 v12 2/4] gadget: Support for the usb charger framework
Hi,
Baolin Wang writes:
>>> For supporting the usb charger, it adds the usb_charger_init() and
>>> usb_charger_exit() functions for usb charger initialization and exit.
>>>
>>> It will report to the usb charger when the gadget state is changed,
>>> then the usb charger can do the power things.
>>>
>>> Signed-off-by: Baolin Wang
>>> Reviewed-by: Li Jun
>>> Tested-by: Li Jun
>>
>> Before anything, I must say that I really liked this patch. It's
>> minimaly invasive to udc core and does all the necessary changes. If it
>> wasn't for the extra charger class, this would've been perfect.
>>
>> Can't you just tie a charger to a UDC and avoid the charger class
>> completely?
>>
>>> static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget,
>>> unsigned mA)
>>> {
>>> + if (gadget->charger)
>>
>> I guess you could do this check inside
>> usb_gadget_set_cur_limit_by_type() itself.
>
> We will access the 'gadget->charger->type' member when issuing
> usb_gadget_set_cur_limit_by_type(), so I think I should leave the
> check here in next new version.
Here's what I mean:
int usb_charger_set_cur_limit(struct usb_gadget *gadget, unsigned int mA)
{
struct usb_charger *charger;
enum usb_charger_type type;
if (!gadget->charger)
return 0;
charger = gadget->charger;
type = charger->type;
return __usb_charger_set_cur_limit(charger, type, mA);
}
>>>
>>> But that means we need to export both 'usb_charger_set_cur_limit()'
>>> function and '__usb_charger_set_cur_limit()' function in charger.c
>>> file. Cause some user may want to set the current limitation by one
>>> charger type parameter (may be not from charger->type), like by
>>> issuing '__usb_charger_set_cur_limit(charger, SDP_TYPE, mA)'. How do
>>> you think about this situation? Thanks.
>>
>> if we have that requirement, that's totally fine. Just rename
>> __usb_charger_set_cur_limit() back to
>> _usb_charger_set_cur_limit_by_type() and expose both. But
>> set_cur_limit_by_type can assume its arguments are valid at all times.
>
> Make sense. I'll fix this issue in v14 version. Thanks.
there's one thing bothering me though:
gadget->charger is supposed to be "current detected charger" right? If
we have a single port tied to a single charger, in which case would we
*ever* need to change current limit of any charger type other than
"current charger" ?
IOW, why do we need _set_cur_limit_by_type() exported at all?
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH v12 2/4] gadget: Support for the usb charger framework
Hi Felipe,
On 29 June 2016 at 20:06, Felipe Balbi wrote:
>
> Hi,
>
> Baolin Wang writes:
For supporting the usb charger, it adds the usb_charger_init() and
usb_charger_exit() functions for usb charger initialization and exit.
It will report to the usb charger when the gadget state is changed,
then the usb charger can do the power things.
Signed-off-by: Baolin Wang
Reviewed-by: Li Jun
Tested-by: Li Jun
>>>
>>> Before anything, I must say that I really liked this patch. It's
>>> minimaly invasive to udc core and does all the necessary changes. If it
>>> wasn't for the extra charger class, this would've been perfect.
>>>
>>> Can't you just tie a charger to a UDC and avoid the charger class
>>> completely?
>>>
static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget,
unsigned mA)
{
+ if (gadget->charger)
>>>
>>> I guess you could do this check inside
>>> usb_gadget_set_cur_limit_by_type() itself.
>>
>> We will access the 'gadget->charger->type' member when issuing
>> usb_gadget_set_cur_limit_by_type(), so I think I should leave the
>> check here in next new version.
>
> Here's what I mean:
>
> int usb_charger_set_cur_limit(struct usb_gadget *gadget, unsigned int mA)
> {
> struct usb_charger *charger;
> enum usb_charger_type type;
>
> if (!gadget->charger)
> return 0;
>
> charger = gadget->charger;
> type = charger->type;
>
> return __usb_charger_set_cur_limit(charger, type, mA);
> }
But that means we need to export both 'usb_charger_set_cur_limit()'
function and '__usb_charger_set_cur_limit()' function in charger.c
file. Cause some user may want to set the current limitation by one
charger type parameter (may be not from charger->type), like by
issuing '__usb_charger_set_cur_limit(charger, SDP_TYPE, mA)'. How do
you think about this situation? Thanks.
>>>
>>> if we have that requirement, that's totally fine. Just rename
>>> __usb_charger_set_cur_limit() back to
>>> _usb_charger_set_cur_limit_by_type() and expose both. But
>>> set_cur_limit_by_type can assume its arguments are valid at all times.
>>
>> Make sense. I'll fix this issue in v14 version. Thanks.
>
> there's one thing bothering me though:
>
> gadget->charger is supposed to be "current detected charger" right? If
> we have a single port tied to a single charger, in which case would we
> *ever* need to change current limit of any charger type other than
> "current charger" ?
What I mean is user can set the current limit by charger type as what
they want at initial stage. As we know the default current of SDP (not
super speed) is 500 mA, but user can set the current limit of SDP as
450 mA if there are some special on their own platform by issuing
'__usb_charger_set_cur_limit(charger, SDP_TYPE, 450)'.
>
> IOW, why do we need _set_cur_limit_by_type() exported at all?
>
> --
> balbi
--
Baolin.wang
Best Regards
--
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 v12 2/4] gadget: Support for the usb charger framework
Hi,
Baolin Wang writes:
> For supporting the usb charger, it adds the usb_charger_init() and
> usb_charger_exit() functions for usb charger initialization and exit.
>
> It will report to the usb charger when the gadget state is changed,
> then the usb charger can do the power things.
>
> Signed-off-by: Baolin Wang
> Reviewed-by: Li Jun
> Tested-by: Li Jun
Before anything, I must say that I really liked this patch. It's
minimaly invasive to udc core and does all the necessary changes. If it
wasn't for the extra charger class, this would've been perfect.
Can't you just tie a charger to a UDC and avoid the charger class
completely?
> static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget,
> unsigned mA)
> {
> + if (gadget->charger)
I guess you could do this check inside
usb_gadget_set_cur_limit_by_type() itself.
>>>
>>> We will access the 'gadget->charger->type' member when issuing
>>> usb_gadget_set_cur_limit_by_type(), so I think I should leave the
>>> check here in next new version.
>>
>> Here's what I mean:
>>
>> int usb_charger_set_cur_limit(struct usb_gadget *gadget, unsigned int mA)
>> {
>> struct usb_charger *charger;
>> enum usb_charger_type type;
>>
>> if (!gadget->charger)
>> return 0;
>>
>> charger = gadget->charger;
>> type = charger->type;
>>
>> return __usb_charger_set_cur_limit(charger, type, mA);
>> }
>
> But that means we need to export both 'usb_charger_set_cur_limit()'
> function and '__usb_charger_set_cur_limit()' function in charger.c
> file. Cause some user may want to set the current limitation by one
> charger type parameter (may be not from charger->type), like by
> issuing '__usb_charger_set_cur_limit(charger, SDP_TYPE, mA)'. How do
> you think about this situation? Thanks.
if we have that requirement, that's totally fine. Just rename
__usb_charger_set_cur_limit() back to
_usb_charger_set_cur_limit_by_type() and expose both. But
set_cur_limit_by_type can assume its arguments are valid at all times.
>>>
>>> Make sense. I'll fix this issue in v14 version. Thanks.
>>
>> there's one thing bothering me though:
>>
>> gadget->charger is supposed to be "current detected charger" right? If
>> we have a single port tied to a single charger, in which case would we
>> *ever* need to change current limit of any charger type other than
>> "current charger" ?
>
> What I mean is user can set the current limit by charger type as what
> they want at initial stage. As we know the default current of SDP (not
> super speed) is 500 mA, but user can set the current limit of SDP as
> 450 mA if there are some special on their own platform by issuing
> '__usb_charger_set_cur_limit(charger, SDP_TYPE, 450)'.
Oh I see. Odd, but okay
--
balbi
signature.asc
Description: PGP signature
DONATION OF $ 1.5 MILLION DOLLARS!!!
My wife and I have awarded you with a donation of $ 1.5 million Dollars from part of our Jackpot Lottery of 161,653,000 Million Pounds,Send your name,Address, Phone for claims Now. To verify the genuineness of this email, check this web page; http://www.bbc.com/news/uk-scotland-glasgow-west-18801698 We await your earliest response and God Bless you. Best of luck. Colin & Chris Weir -- 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
[GIT PULL] usb patches for v4.8
Hi Greg, here's the big pull request for gadget API and related UDC drivers. Nothing really scary lately. Patches have been in linux-next for a while without outstanding reports. Let me know if you want any changes, but things seem to be calming down. I have, however, a few patches pending in my linux-usb inbox but I won't have time to really work on them in time for current merge window, so I decided to cut my tree short and send you a pull request. Anyway, here's the good stuff The following changes since commit 33688abb2802ff3a230bd2441f765477b94cc89e: Linux 4.7-rc4 (2016-06-19 21:30:02 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git tags/usb-for-v4.8 for you to fetch changes up to 15e4292a2d21e9997fdb2b8c014cc461b3f268f0: usb: renesas_usbhs: protect the CFIFOSEL setting in usbhsg_ep_enable() (2016-06-29 11:14:44 +0300) usb: patches for v4.8 merge window Here's the big pull request for Peripheral stack and all related drivers. This time around with 109 non-merge commits mostly concentrated on drivers/usb/gadget/udc (41.5%) and drivers/usb/dwc3 (28.1%). There's a big rework on dwc3's transfer handling which gave us almost 3x faster USB3 speeds with Mass Storage on a particular test scenario I measured. We are also removing platform_data from dwc3 after converting all users to built-in properties instead. For the Gadget API, we're just adding tracepoints to aid debugging activities. Other than these, there's the usual set of spelling fixes, minor bug fixes and sparse warnings cleanups. Alexandre Belloni (1): usb: gadget: udc: atmel: Also get regmap for at91sam9x5-pmc Arnd Bergmann (3): usb: pxa27x_udc: remove unused function argument usb: phy: move msm_hsusb.h into driver USB: dwc2-usb: add USB_GADGET dependency Baolin Wang (2): usb: dwc3: gadget: Add the suspend state checking when stopping gadget dwc3: gadget: Implement the suspend entry event handler Ben Dooks (1): usb: renesas_usbhs: make usbhs_write32() static Colin Ian King (1): usb: gadget: bdc: fix spelling mistake: "allocted" -> "allocated" Dan Carpenter (1): usb: gadget: f_fs: check for allocation failure Felipe Balbi (48): usb: gadget: storage: get rid of fsg_num_buffers_validate() usb: gadget: storage: increase maximum storage num buffers usb: dwc3: gadget: re-factor ->udc_start and ->udc_stop usb: dwc3: gadget: fix gadget suspend/resume usb: dwc3: core: get rid of DWC3_PM_OPS macro usb: dwc3: gadget: prepare TRBs on update transfers too usb: dwc3: gadget: simplify __dwc3_gadget_kick_transfer() usb: dwc3: gadget: rely on sg_is_last() and list_is_last() usb: dwc3: gadget: remove udelay(1) when sending ep cmds usb: dwc3: gadget: return 0 if we try to Wakeup in superspeed usb: dwc3: gadget: split __dwc3_gadget_kick_transfer() usb: dwc3: gadget: initialize NUMP based on RxFIFO Size usb: dwc3: gadget: pass dep as argument to endpoint command usb: dwc3: gadget: add a pointer to endpoint registers usb: dwc3: core: move fladj to dwc3 structure usb: dwc3: core: re-factor init and exit paths usb: dwc3: core: simplify suspend/resume operations usb: dwc3: gadget: hold gadget IRQ in dwc->irq_gadget usb: dwc3: gadget: only resume USB2 PHY in <=HIGHSPEED usb: dwc3: gadget: fix for possible endpoint disable race usb: dwc3: implement runtime PM usb: dwc3: pci: add Power Management dummy hooks usb: dwc3: gadget: add a per-endpoint request queue lock usb: dwc3: gadget: no more tracking endpoint type with its name usb: dwc3: trace: fully decode IRQ events usb: dwc3: gadget: fix trace output when command fails usb: dwc3: gadget: loop while (timeout) usb: dwc3: trace: print ep cmd status with a single trace usb: dwc3: gadget: single return point on generic commands usb: dwc3: gadget: remove udelay() from generic cmd usb: dwc3: gadget: improve gcmd trace usb: dwc3: remove trailing newline from dwc3_trace usb: dwc3: gadget: update transfer needs transfer resource usb: dwc3: gadget: keep track of allocated and queued reqs usb: dwc3: gadget: halt and stop based HWO bit usb: dwc3: gadget: use allocated/queued reqs for LST bit usb: dwc3: gadget: disable XFER_NOT_READY usb: dwc3: gadget: start Bulk endpoints more frequently usb: dwc3: gadget: decrement trbs_left for each sg entry usb: gadget: move gadget API functions to udc-core usb: gadget: add tracepoints to the gadget API usb: dwc3: gadget: rename 'ignore' argument to 'modify' usb: dwc3: pci: add dr-mode for Intel dwc3 usb: dwc3: core: fixup dr_mode fallback selection usb: dwc3: gadge
[PATCHv4 1/2] usb: USB Type-C connector class
The purpose of USB Type-C connector class is to provide unified interface for the user space to get the status and basic information about USB Type-C connectors on a system, control over data role swapping, and when the port supports USB Power Delivery, also control over power role swapping and Alternate Modes. Signed-off-by: Heikki Krogerus --- Documentation/ABI/testing/sysfs-class-typec | 236 + Documentation/usb/typec.txt | 103 +++ MAINTAINERS |9 + drivers/usb/Kconfig |2 + drivers/usb/Makefile|2 + drivers/usb/typec/Kconfig |7 + drivers/usb/typec/Makefile |1 + drivers/usb/typec/typec.c | 1277 +++ include/linux/usb/typec.h | 260 ++ 9 files changed, 1897 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-typec create mode 100644 Documentation/usb/typec.txt create mode 100644 drivers/usb/typec/Kconfig create mode 100644 drivers/usb/typec/Makefile create mode 100644 drivers/usb/typec/typec.c create mode 100644 include/linux/usb/typec.h diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec new file mode 100644 index 000..7cd40e5 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-typec @@ -0,0 +1,236 @@ +USB Type-C port devices (eg. /sys/class/typec/usbc0/) + +What: /sys/class/typec//current_data_role +Date: June 2016 +Contact: Heikki Krogerus +Description: + The current USB data role the port is operating in. This + attribute can be used for requesting data role swapping on the + port. + + Valid values: + - host + - device + +What: /sys/class/typec//current_power_role +Date: June 2016 +Contact: Heikki Krogerus +Description: + The current power role of the port. This attribute can be used + to request power role swap on the port when the port supports + USB Power Delivery. + + Valid values: + - source + - sink + +What: /sys/class/typec//current_vconn_role +Date: June 2016 +Contact: Heikki Krogerus +Description: + Shows the current VCONN role of the port. This attribute can be + used to request VCONN role swap on the port when the port + supports USB Power Delivery. + + Valid values are: + - source + - sink + +What: /sys/class/typec//power_operation_mode +Date: June 2016 +Contact: Heikki Krogerus +Description: + Shows the current power operational mode the port is in. + + Valid values: + - USB - Normal power levels defined in USB specifications + - BC1.2 - Power levels defined in Battery Charging Specification + v1.2 + - USB Type-C 1.5A - Higher 1.5A current defined in USB Type-C + specification. + - USB Type-C 3.0A - Higher 3A current defined in USB Type-C + specification. +- USB Power Delivery - The voltages and currents defined in USB + Power Delivery specification + +What: /sys/class/typec//preferred_role +Date: June 2016 +Contact: Heikki Krogerus +Description: + The user space can notify the driver about the preferred role. + It should be handled as enabling of Try.SRC or Try.SNK, as + defined in USB Type-C specification, in the port drivers. By + default there is no preferred role. + + Valid values: + - host + - device + - For example "none" to remove preference (anything else except + "host" or "device") + +What: /sys/class/typec//supported_accessory_modes +Date: June 2016 +Contact: Heikki Krogerus +Description: + Lists the Accessory Modes, defined in the USB Type-C + specification, the port supports. + +What: /sys/class/typec//supported_data_roles +Date: June 2016 +Contact: Heikki Krogerus +Description: + Lists the USB data roles the port is capable of supporting. + + Valid values: + - device + - host + - device, host (DRD as defined in USB Type-C specification v1.2) + +What: /sys/class/typec//supported_power_roles +Date: June 2016 +Contact: Heikki Krogerus +Description: + Lists the power roles the port is capable of supporting. + + Valid values: + - source
[PATCHv4 2/2] usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY
This adds driver for the USB Type-C PHY on Intel WhiskeyCove
PMIC which is available on some of the Intel Broxton SoC
based platforms.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/Kconfig | 14 ++
drivers/usb/typec/Makefile | 1 +
drivers/usb/typec/typec_wcove.c | 371
3 files changed, 386 insertions(+)
create mode 100644 drivers/usb/typec/typec_wcove.c
diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index b229fb9..7a345a4 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -4,4 +4,18 @@ menu "USB PD and Type-C drivers"
config TYPEC
tristate
+config TYPEC_WCOVE
+ tristate "Intel WhiskeyCove PMIC USB Type-C PHY driver"
+ depends on ACPI
+ depends on INTEL_SOC_PMIC
+ depends on INTEL_PMC_IPC
+ select TYPEC
+ help
+ This driver adds support for USB Type-C detection on Intel Broxton
+ platforms that have Intel Whiskey Cove PMIC. The driver can detect the
+ role and cable orientation.
+
+ To compile this driver as module, choose M here: the module will be
+ called typec_wcove
+
endmenu
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 1012a8b..b9cb862 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_TYPEC)+= typec.o
+obj-$(CONFIG_TYPEC_WCOVE) += typec_wcove.o
diff --git a/drivers/usb/typec/typec_wcove.c b/drivers/usb/typec/typec_wcove.c
new file mode 100644
index 000..c7c2d28
--- /dev/null
+++ b/drivers/usb/typec/typec_wcove.c
@@ -0,0 +1,371 @@
+/**
+ * typec_wcove.c - WhiskeyCove PMIC USB Type-C PHY driver
+ *
+ * Copyright (C) 2016 Intel Corporation
+ * Author: Heikki Krogerus
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* Register offsets */
+#define WCOVE_CHGRIRQ0 0x4e09
+#define WCOVE_PHYCTRL 0x5e07
+
+#define USBC_CONTROL1 0x7001
+#define USBC_CONTROL2 0x7002
+#define USBC_CONTROL3 0x7003
+#define USBC_CC1_CTRL 0x7004
+#define USBC_CC2_CTRL 0x7005
+#define USBC_STATUS1 0x7007
+#define USBC_STATUS2 0x7008
+#define USBC_STATUS3 0x7009
+#define USBC_IRQ1 0x7015
+#define USBC_IRQ2 0x7016
+#define USBC_IRQMASK1 0x7017
+#define USBC_IRQMASK2 0x7018
+
+/* Register bits */
+
+#define USBC_CONTROL1_MODE_DRP(r) ((r & ~0x7) | 4)
+
+#define USBC_CONTROL2_UNATT_SNKBIT(0)
+#define USBC_CONTROL2_UNATT_SRCBIT(1)
+#define USBC_CONTROL2_DIS_ST BIT(2)
+
+#define USBC_CONTROL3_PD_DIS BIT(1)
+
+#define USBC_CC_CTRL_VCONN_EN BIT(1)
+
+#define USBC_STATUS1_DET_ONGOING BIT(6)
+#define USBC_STATUS1_RSLT(r) (r & 0xf)
+#define USBC_RSLT_NOTHING 0
+#define USBC_RSLT_SRC_DEFAULT 1
+#define USBC_RSLT_SRC_1_5A 2
+#define USBC_RSLT_SRC_3_0A 3
+#define USBC_RSLT_SNK 4
+#define USBC_RSLT_DEBUG_ACC5
+#define USBC_RSLT_AUDIO_ACC6
+#define USBC_RSLT_UNDEF15
+#define USBC_STATUS1_ORIENT(r) ((r >> 4) & 0x3)
+#define USBC_ORIENT_NORMAL 1
+#define USBC_ORIENT_REVERSE2
+
+#define USBC_STATUS2_VBUS_REQ BIT(5)
+
+#define USBC_IRQ1_ADCDONE1 BIT(2)
+#define USBC_IRQ1_OVERTEMP BIT(1)
+#define USBC_IRQ1_SHORTBIT(0)
+
+#define USBC_IRQ2_CC_CHANGEBIT(7)
+#define USBC_IRQ2_RX_PDBIT(6)
+#define USBC_IRQ2_RX_HRBIT(5)
+#define USBC_IRQ2_RX_CRBIT(4)
+#define USBC_IRQ2_TX_SUCCESS BIT(3)
+#define USBC_IRQ2_TX_FAIL BIT(2)
+
+#define USBC_IRQMASK1_ALL (USBC_IRQ1_ADCDONE1 | USBC_IRQ1_OVERTEMP | \
+USBC_IRQ1_SHORT)
+
+#define USBC_IRQMASK2_ALL (USBC_IRQ2_CC_CHANGE | USBC_IRQ2_RX_PD | \
+USBC_IRQ2_RX_HR | USBC_IRQ2_RX_CR | \
+USBC_IRQ2_TX_SUCCESS | USBC_IRQ2_TX_FAIL)
+
+struct wcove_typec {
+ struct mutex lock; /* device lock */
+ struct device *dev;
+ struct regmap *regmap;
+ struct typec_port *port;
+ struct typec_capability cap;
+ struct typec_connection con;
+ struct typec_partner partner;
+};
+
+enum wcove_typec_func {
+ WCOVE_FUNC_DRIVE_VBUS = 1,
+ WCOVE_FUNC_ORIENTATION,
+ WCOVE_FUNC_ROLE,
+ WCOVE_FUNC_DRIVE_VCONN,
+};
+
+enum wcove_typec_orientation {
+ WCOVE_ORIENTATION_NORMAL,
+ WCOVE_ORIENTATION_REVERSE,
+};
+
+enum wcove_typec_role {
+ WCOVE
[PATCHv4 0/2] USB Type-C Connector class
Hi, The USB Type-C class is meant to provide unified interface to the userspace to present the USB Type-C ports in a system. Changes since v3: - Documentation cleanup as proposed by Roger Quadros - Setting partner altmodes member to NULL on removal and fixing a warning, as proposed by Guenter Roeck - Added the following attributes for partners and cables: * supports_usb_power_delivery * id_header_vdo - "id_header_vdo" is visible only when the partner or cable supports USB Power Delivery communication. - Partner attribute "accessory" is hidden when the partner type is not "Accessory". Changes since v2: - Notification on role and alternate mode changes - cleanups Changes since v1: - Completely rewrote alternate mode support - Patners, cables and cable plugs presented as devices. Heikki Krogerus (2): usb: USB Type-C connector class usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY Documentation/ABI/testing/sysfs-class-typec | 236 + Documentation/usb/typec.txt | 103 +++ MAINTAINERS |9 + drivers/usb/Kconfig |2 + drivers/usb/Makefile|2 + drivers/usb/typec/Kconfig | 21 + drivers/usb/typec/Makefile |2 + drivers/usb/typec/typec.c | 1277 +++ drivers/usb/typec/typec_wcove.c | 371 include/linux/usb/typec.h | 260 ++ 10 files changed, 2283 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-typec create mode 100644 Documentation/usb/typec.txt create mode 100644 drivers/usb/typec/Kconfig create mode 100644 drivers/usb/typec/Makefile create mode 100644 drivers/usb/typec/typec.c create mode 100644 drivers/usb/typec/typec_wcove.c create mode 100644 include/linux/usb/typec.h -- 2.8.1 -- 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 v5 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy
Hi,
On Friday 17 June 2016 10:16 PM, Heiko Stübner wrote:
> Hi Kishon,
>
> Am Freitag, 17. Juni 2016, 17:24:46 schrieb Kishon Vijay Abraham I:
>
>>> + ret = of_clk_add_provider(node, of_clk_src_simple_get, rphy->clk480m);
>>> + if (ret < 0)
>>> + goto err_clk_provider;
>>> +
>>> + ret = devm_add_action(rphy->dev, rockchip_usb2phy_clk480m_unregister,
>>> + rphy);
>>> + if (ret < 0)
>>> + goto err_unreg_action;
>>> +
>>> + return 0;
>>> +
>>> +err_unreg_action:
>>> + of_clk_del_provider(node);
>>> +err_clk_provider:
>>> + clk_unregister(rphy->clk480m);
>>> +err_register:
>>> + if (rphy->clk)
>>> + clk_put(rphy->clk);
>>> + return ret;
>>> +}
>>
>> I'm seeing lot of similarities specifically w.r.t to clock handling part in
>> drivers/phy/phy-rockchip-usb.c. Why not just re-use that driver?
>
> It's a completely different phy block (Designware vs. Innosilicon) and a lot
> of stuff also is handled differently.
>
> For one on the old block, each phy was somewhat independent and had for
> examle
> its own clock-supply, while on this one there is only one for both ports of
> the phy. Similarly with the clock getting fed back to the clock-controller
> (one clock per port on the old block, now one clock for the whole phy).
>
> Then as you can see, the handling for power-up and down is a bit different
> and
> I guess one big block might be the still missing special otg handling, Frank
> wrote about.
All right then.
>
>
> [...]
>
>>> + /*
>>> +* we don't need to rearm the delayed work when the phy port
>>> +* is suspended.
>>> +*/
>>> + mutex_unlock(&rport->mutex);
>>> + return;
>>> + default:
>>> + dev_dbg(&rport->phy->dev, "unknown phy state\n");
>>> + break;
>>> + }
>>> +
>>> +next_schedule:
>>> + mutex_unlock(&rport->mutex);
>>> + schedule_delayed_work(&rport->sm_work, SCHEDULE_DELAY);
>>
>> Why are you scheduling the work again? Interrupt handlers can invoke this
>> right?
>
> Frank said, that the phy is only able to detect the plug-in event via
> interrupts, not the removal, so once a plugged device is detected, this gets
> rescheduled until the device gets removed.
okay.
>
> [...]
>
>>> + /* find out a proper config which can be matched with dt. */
>>> + index = 0;
>>> + while (phy_cfgs[index].reg) {
>>> + if (phy_cfgs[index].reg == reg) {
>>
>> Why not pass these config values from dt? Moreover finding the config using
>> register offset is bound to break.
>
> As you have probably seen, this phy block is no stand-alone (mmio-)device,
> but
> gets controlled through special register/bits in the so called "General
> Register Files" syscon.
>
> The values stored and accessed here, are the location and layout of those
> control registers. Bits in those phy control registers at times move between
> phy-versions in different socs (rk3036, rk3228, rk3366, rk3368, rk3399) and
> some are even missing. So I don't really see a nice way to describe that in
> dt
> without describing the register and offset of each of those 22 used bits
> individually.
>
>
> I'm also not sure where you expect it to break? The reg-offset is the offset
> of the phy inside the GRF and the Designware-phy also already does something
> similar to select some appropriate values.
I'm concerned the phy can be placed in the different reg-offset within GRF
(like in the next silicon revision) and this driver can't be used.
Thanks
Kishon
>
--
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: Unable to wake Surface 3 from freeze with USB devices
On Tue, 28 Jun 2016, Stephen J wrote: > Hello, > > I'm currently having some difficulty with getting my Surface 3 device > to resume from the "freeze" PM state with USB devices. This device > uses an Intel Cherry Trail xHCI host, enumerated over PCI. (I am able > to resume with the device's hardware buttons, which are simply gpio > keys.) I experience the issue on all kernel versions that run on the > device, including usb-next. > > After a few days of playing around with this, I'm really not sure > what's going on - it's as if the system is not receiving any interrupt > from the USB host to wake it up, but I don't know how to verify this. > > I have already posted several system logs and other details on > bugzilla [1], and I've made sure that the /power/wakeup option was > enabled for the relevant USB peripherals. Which ones in particular? > At this point, I'm open to further suggestions that might help to pin > down the root cause of the problem, whether it be a bug in the xhci > driver, or simply user error on my part. > > [1] https://bugzilla.kernel.org/show_bug.cgi?id=120701 What do you get from "lspci -vv -s 00:14.0"? And what shows up in the /sys/bus/pci/device/:00:14.0/power/wakeup file? You can also try enabling usbcore dynamic debugging: echo 'module usbcore =p' >/sys/kernel/debug/dynamic_debug/control and then see what shows up in the dmesg log. Alan Stern -- 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
udev rule to reserve ftdi jtag instead of device description locked quirk
Hi there. I am seeking for a solution to make ftdi-sio.ko ignoring one of the ft2232h (and similar) channel, where it have to be free for jtag (and other) operations via libusb, libftdi, libftd2xx and so on. I have found in the sources, that there is a long list of device descriptors of devices with similar needs, but the particular "Lattice FTUSB Interface Cable" isn't here. I can append my device to source code and recompile the module myself, but that is not the way which may help other people. I also think, that the module quirk is not the best way to achieve this behavior of freeing sub-channel. In the eeprom connected to a ftdi chip, there is information if channel is "VCP" - serial port or "D2XX" - direct. Windows driver is checking it and makes not virtual COM port where VCP is not selected. My opinion is, that similar behavior should be provided by udev and the kernel module has to be configured in the time in runtime while registering a newly connected chip. The "quirked" devices list should be moved to udev/rules.d and user should be able to write his own rules for his devices, which also may be specific (homemade) hardware. The list of quirks in source code will be in principle always incomplete. I am not really experienced kernel driver hacker, but i would like to help with it. Many thanks Jakub Ladman -- 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 v5 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy
Hi Kishon,
Am Mittwoch, 29. Juni 2016, 19:44:52 schrieb Kishon Vijay Abraham I:
> On Friday 17 June 2016 10:16 PM, Heiko Stübner wrote:
> > Am Freitag, 17. Juni 2016, 17:24:46 schrieb Kishon Vijay Abraham I:
> > [...]
> >
> >>> + /* find out a proper config which can be matched with dt. */
> >>> + index = 0;
> >>> + while (phy_cfgs[index].reg) {
> >>> + if (phy_cfgs[index].reg == reg) {
> >>
> >> Why not pass these config values from dt? Moreover finding the config
> >> using register offset is bound to break.
> >
> > As you have probably seen, this phy block is no stand-alone
> > (mmio-)device, but gets controlled through special register/bits in the
> > so called "General Register Files" syscon.
> >
> > The values stored and accessed here, are the location and layout of
> > those
> > control registers. Bits in those phy control registers at times move
> > between phy-versions in different socs (rk3036, rk3228, rk3366, rk3368,
> > rk3399) and some are even missing. So I don't really see a nice way to
> > describe that in dt without describing the register and offset of each
> > of those 22 used bits individually.
> >
> >
> > I'm also not sure where you expect it to break? The reg-offset is the
> > offset of the phy inside the GRF and the Designware-phy also already
> > does something similar to select some appropriate values.
>
> I'm concerned the phy can be placed in the different reg-offset within GRF
> (like in the next silicon revision) and this driver can't be used.
That is something that has not happened with all Rockchip SoCs I have had in
my hands so far.
While the GRF is some sort-of wild-west area with settings for vastly
different blocks in there and always changes between different socs (rk3288
[4-core A17] <-> rk3366 <-> rk3368 [8-core A53] <-> rk3399 etc) there
haven't been such changes (different register offsets) between soc-revisions
of the same soc that I know off.
The GRF also includes such important things like the whole pinctrl handling,
so if there were such offset changes in the GRF a lot of drivers (and
maintainers) would get very unhappy - me included :-) .
Heiko
--
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
[no subject]
Hello LKML. I have had a small ASUS usb screen gadget that came with my motherboard. Its been sitting on my desk for about 7 years. Having exhausted my patience waiting for a nifty Linux app to drive it, I wrote one. If you have such a beast and want the app, go to https://github.com/mayorbobster/screenduo4linux If you aren't interested, please disregard this notice. Thank you. -- 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: [GIT PULL] usb patches for v4.8
On Wed, Jun 29, 2016 at 04:11:31PM +0300, Felipe Balbi wrote: > > Hi Greg, > > here's the big pull request for gadget API and related UDC drivers. > > Nothing really scary lately. Patches have been in linux-next for a while > without outstanding reports. > > Let me know if you want any changes, but things seem to be calming > down. I have, however, a few patches pending in my linux-usb inbox but I > won't have time to really work on them in time for current merge window, > so I decided to cut my tree short and send you a pull request. > > Anyway, here's the good stuff > > The following changes since commit 33688abb2802ff3a230bd2441f765477b94cc89e: > > Linux 4.7-rc4 (2016-06-19 21:30:02 -0700) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git > tags/usb-for-v4.8 I got a merge issue in drivers/usb/dwc3/host.c that I had to fix up by hand. Can you verify I got the merge correct? It builds for me here :) thanks, greg k-h -- 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 v5 resend 0/2] phy-sun4i-usb: peripheral-mode + a31 otg workaround
Hi Kishon, The "USB: Fix of_usb_get_dr_mode_by_phy with a shared phy block" patch on which this series depends is in usb-next now: https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=ce15ed4c5dfb3f7757e6611902aed5db253af977 Can you please merge this series? Regards, Hans -- 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 v5 resend 2/2] phy-sun4i-usb: Add workaround for missing Vbus det interrupts on A31
The A31 companion pmic (axp221) does not generate vbus change interrupts
when the board is driving vbus, so we must poll when using the pmic for
vbus-det _and_ we're driving vbus.
Signed-off-by: Hans de Goede
Acked-by: Kishon Vijay Abraham I
---
Changes in v2:
-No changes
Changes in v3:
-No changes
Changes in v4:
-No changes
Changes in v5:
-Rebase on top of latest linux-phy/fixes
-Added Kishon's Acked-by
---
drivers/phy/phy-sun4i-usb.c | 34 --
1 file changed, 24 insertions(+), 10 deletions(-)
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index f4d1178..8c7eb33 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -95,6 +95,7 @@
enum sun4i_usb_phy_type {
sun4i_a10_phy,
+ sun6i_a31_phy,
sun8i_a33_phy,
sun8i_h3_phy,
};
@@ -125,7 +126,6 @@ struct sun4i_usb_phy_data {
/* phy0 / otg related variables */
struct extcon_dev *extcon;
bool phy0_init;
- bool phy0_poll;
struct gpio_desc *id_det_gpio;
struct gpio_desc *vbus_det_gpio;
struct power_supply *vbus_power_supply;
@@ -353,6 +353,24 @@ static bool sun4i_usb_phy0_have_vbus_det(struct
sun4i_usb_phy_data *data)
return data->vbus_det_gpio || data->vbus_power_supply;
}
+static bool sun4i_usb_phy0_poll(struct sun4i_usb_phy_data *data)
+{
+ if ((data->id_det_gpio && data->id_det_irq <= 0) ||
+ (data->vbus_det_gpio && data->vbus_det_irq <= 0))
+ return true;
+
+ /*
+* The A31 companion pmic (axp221) does not generate vbus change
+* interrupts when the board is driving vbus, so we must poll
+* when using the pmic for vbus-det _and_ we're driving vbus.
+*/
+ if (data->cfg->type == sun6i_a31_phy &&
+ data->vbus_power_supply && data->phys[0].regulator_on)
+ return true;
+
+ return false;
+}
+
static int sun4i_usb_phy_power_on(struct phy *_phy)
{
struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
@@ -374,7 +392,7 @@ static int sun4i_usb_phy_power_on(struct phy *_phy)
phy->regulator_on = true;
/* We must report Vbus high within OTG_TIME_A_WAIT_VRISE msec. */
- if (phy->index == 0 && data->vbus_det_gpio && data->phy0_poll)
+ if (phy->index == 0 && sun4i_usb_phy0_poll(data))
mod_delayed_work(system_wq, &data->detect, DEBOUNCE_TIME);
return 0;
@@ -395,7 +413,7 @@ static int sun4i_usb_phy_power_off(struct phy *_phy)
* phy0 vbus typically slowly discharges, sometimes this causes the
* Vbus gpio to not trigger an edge irq on Vbus off, so force a rescan.
*/
- if (phy->index == 0 && data->vbus_det_gpio && !data->phy0_poll)
+ if (phy->index == 0 && !sun4i_usb_phy0_poll(data))
mod_delayed_work(system_wq, &data->detect, POLL_TIME);
return 0;
@@ -483,7 +501,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct
work_struct *work)
if (vbus_notify)
extcon_set_cable_state_(data->extcon, EXTCON_USB, vbus_det);
- if (data->phy0_poll)
+ if (sun4i_usb_phy0_poll(data))
queue_delayed_work(system_wq, &data->detect, POLL_TIME);
}
@@ -668,11 +686,6 @@ static int sun4i_usb_phy_probe(struct platform_device
*pdev)
}
data->id_det_irq = gpiod_to_irq(data->id_det_gpio);
- data->vbus_det_irq = gpiod_to_irq(data->vbus_det_gpio);
- if ((data->id_det_gpio && data->id_det_irq <= 0) ||
- (data->vbus_det_gpio && data->vbus_det_irq <= 0))
- data->phy0_poll = true;
-
if (data->id_det_irq > 0) {
ret = devm_request_irq(dev, data->id_det_irq,
sun4i_usb_phy0_id_vbus_det_irq,
@@ -684,6 +697,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
}
}
+ data->vbus_det_irq = gpiod_to_irq(data->vbus_det_gpio);
if (data->vbus_det_irq > 0) {
ret = devm_request_irq(dev, data->vbus_det_irq,
sun4i_usb_phy0_id_vbus_det_irq,
@@ -735,7 +749,7 @@ static const struct sun4i_usb_phy_cfg sun5i_a13_cfg = {
static const struct sun4i_usb_phy_cfg sun6i_a31_cfg = {
.num_phys = 3,
- .type = sun4i_a10_phy,
+ .type = sun6i_a31_phy,
.disc_thresh = 3,
.phyctl_offset = REG_PHYCTL_A10,
.dedicated_clocks = true,
--
2.7.4
--
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 v5 resend 1/2] phy-sun4i-usb: Add support for peripheral-only mode
Use the new of_usb_get_dr_mode_by_phy() function to get the dr_mode
from the musb controller node instead of assuming that having an id_det
gpio means otg mode, and not having one means host mode.
Implement peripheral-only mode by adding a sun4i_usb_phy0_get_id_det
helper which looks at the dr_mode, always registering our extcon and
always monitoring vbus.
If dr_mode is not specified in the dts, do not register phy0 as we then
do not know how to treat it. This is actually a good thing as this means
we will not be registering phy0 on devices where the otg controller is
not enabled in the devicetree.
Signed-off-by: Hans de Goede
Acked-by: Kishon Vijay Abraham I
---
Changes in v2:
-Adjust for of_usb_get_dr_mode_by_phy now taking an args0 parameter
Changes in v3:
-Only toggle the phy vbus-det bit on id-change on systems without vbus-det
when in otg mode
Changes in v4:
-No changes
Changes in v5:
-Added Kishon's Acked-by
---
drivers/phy/phy-sun4i-usb.c | 68 ++---
1 file changed, 46 insertions(+), 22 deletions(-)
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index de3101f..f4d1178 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -40,6 +40,7 @@
#include
#include
#include
+#include
#include
#define REG_ISCR 0x00
@@ -109,6 +110,7 @@ struct sun4i_usb_phy_cfg {
struct sun4i_usb_phy_data {
void __iomem *base;
const struct sun4i_usb_phy_cfg *cfg;
+ enum usb_dr_mode dr_mode;
struct mutex mutex;
struct sun4i_usb_phy {
struct phy *phy;
@@ -119,6 +121,7 @@ struct sun4i_usb_phy_data {
bool regulator_on;
int index;
} phys[MAX_PHYS];
+ int first_phy;
/* phy0 / otg related variables */
struct extcon_dev *extcon;
bool phy0_init;
@@ -285,16 +288,10 @@ static int sun4i_usb_phy_init(struct phy *_phy)
sun4i_usb_phy0_update_iscr(_phy, 0, ISCR_DPDM_PULLUP_EN);
sun4i_usb_phy0_update_iscr(_phy, 0, ISCR_ID_PULLUP_EN);
- if (data->id_det_gpio) {
- /* OTG mode, force ISCR and cable state updates */
- data->id_det = -1;
- data->vbus_det = -1;
- queue_delayed_work(system_wq, &data->detect, 0);
- } else {
- /* Host only mode */
- sun4i_usb_phy0_set_id_detect(_phy, 0);
- sun4i_usb_phy0_set_vbus_detect(_phy, 1);
- }
+ /* Force ISCR and cable state updates */
+ data->id_det = -1;
+ data->vbus_det = -1;
+ queue_delayed_work(system_wq, &data->detect, 0);
}
return 0;
@@ -319,6 +316,19 @@ static int sun4i_usb_phy_exit(struct phy *_phy)
return 0;
}
+static int sun4i_usb_phy0_get_id_det(struct sun4i_usb_phy_data *data)
+{
+ switch (data->dr_mode) {
+ case USB_DR_MODE_OTG:
+ return gpiod_get_value_cansleep(data->id_det_gpio);
+ case USB_DR_MODE_HOST:
+ return 0;
+ case USB_DR_MODE_PERIPHERAL:
+ default:
+ return 1;
+ }
+}
+
static int sun4i_usb_phy0_get_vbus_det(struct sun4i_usb_phy_data *data)
{
if (data->vbus_det_gpio)
@@ -414,7 +424,10 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct
work_struct *work)
struct phy *phy0 = data->phys[0].phy;
int id_det, vbus_det, id_notify = 0, vbus_notify = 0;
- id_det = gpiod_get_value_cansleep(data->id_det_gpio);
+ if (phy0 == NULL)
+ return;
+
+ id_det = sun4i_usb_phy0_get_id_det(data);
vbus_det = sun4i_usb_phy0_get_vbus_det(data);
mutex_lock(&phy0->mutex);
@@ -430,7 +443,8 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct
work_struct *work)
* without vbus detection report vbus low for long enough for
* the musb-ip to end the current device session.
*/
- if (!sun4i_usb_phy0_have_vbus_det(data) && id_det == 0) {
+ if (data->dr_mode == USB_DR_MODE_OTG &&
+ !sun4i_usb_phy0_have_vbus_det(data) && id_det == 0) {
sun4i_usb_phy0_set_vbus_detect(phy0, 0);
msleep(200);
sun4i_usb_phy0_set_vbus_detect(phy0, 1);
@@ -456,7 +470,8 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct
work_struct *work)
* without vbus detection report vbus low for long enough to
* the musb-ip to end the current host session.
*/
- if (!sun4i_usb_phy0_have_vbus_det(data) && id_det == 1) {
+ if (data->dr_mode == USB_DR_MODE_OTG &&
+ !sun4i_usb_phy0_have_vbus_det(data) && id_det == 1) {
mutex_lock(&phy0->mutex);
[PATCH v5 resend] usb: musb: sunxi: Simplify dr_mode handling
phy-sun4i-usb now has proper dr_mode handling, it always registers an
extcon, and sends a notify with the mode (even when in peripheral- /
host-only mode) at least once.
So we can simply the sunxi musb glue by always registering its extcon
notifier and relying on sunxi_musb_work() to enable vbus when in
host-only mode.
This also enables host- and peripheral-only mode with vbus monitoring.
Tested-by: Maxime Ripard
Signed-off-by: Hans de Goede
Acked-by: Bin Liu
---
Changes in v2:
-No changes
Changes in v3:
-No changes
Changes in v4:
-Add Maxime's Tested-by
-Add Bin Liu's Acked-by
Changes in v5:
-No changes
---
drivers/usb/musb/sunxi.c | 68 ++--
1 file changed, 25 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index 7650051..a17eb2a 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -256,12 +256,10 @@ static int sunxi_musb_init(struct musb *musb)
writeb(SUNXI_MUSB_VEND0_PIO_MODE, musb->mregs + SUNXI_MUSB_VEND0);
/* Register notifier before calling phy_init() */
- if (musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE) {
- ret = extcon_register_notifier(glue->extcon, EXTCON_USB_HOST,
- &glue->host_nb);
- if (ret)
- goto error_reset_assert;
- }
+ ret = extcon_register_notifier(glue->extcon, EXTCON_USB_HOST,
+ &glue->host_nb);
+ if (ret)
+ goto error_reset_assert;
ret = phy_init(glue->phy);
if (ret)
@@ -275,9 +273,8 @@ static int sunxi_musb_init(struct musb *musb)
return 0;
error_unregister_notifier:
- if (musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
- extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
- &glue->host_nb);
+ extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
+ &glue->host_nb);
error_reset_assert:
if (test_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags))
reset_control_assert(glue->rst);
@@ -301,9 +298,8 @@ static int sunxi_musb_exit(struct musb *musb)
phy_exit(glue->phy);
- if (musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
- extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
- &glue->host_nb);
+ extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
+ &glue->host_nb);
if (test_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags))
reset_control_assert(glue->rst);
@@ -315,25 +311,6 @@ static int sunxi_musb_exit(struct musb *musb)
return 0;
}
-static int sunxi_set_mode(struct musb *musb, u8 mode)
-{
- struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
- int ret;
-
- if (mode == MUSB_HOST) {
- ret = phy_power_on(glue->phy);
- if (ret)
- return ret;
-
- set_bit(SUNXI_MUSB_FL_PHY_ON, &glue->flags);
- /* Stop musb work from turning vbus off again */
- set_bit(SUNXI_MUSB_FL_VBUS_ON, &glue->flags);
- musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
- }
-
- return 0;
-}
-
static void sunxi_musb_enable(struct musb *musb)
{
struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
@@ -582,7 +559,6 @@ static const struct musb_platform_ops sunxi_musb_ops = {
.exit = sunxi_musb_exit,
.enable = sunxi_musb_enable,
.disable= sunxi_musb_disable,
- .set_mode = sunxi_set_mode,
.fifo_offset= sunxi_musb_fifo_offset,
.ep_offset = sunxi_musb_ep_offset,
.busctl_offset = sunxi_musb_busctl_offset,
@@ -638,10 +614,6 @@ static int sunxi_musb_probe(struct platform_device *pdev)
return -EINVAL;
}
- glue = devm_kzalloc(&pdev->dev, sizeof(*glue), GFP_KERNEL);
- if (!glue)
- return -ENOMEM;
-
memset(&pdata, 0, sizeof(pdata));
switch (usb_get_dr_mode(&pdev->dev)) {
#if defined CONFIG_USB_MUSB_DUAL_ROLE || defined CONFIG_USB_MUSB_HOST
@@ -649,15 +621,13 @@ static int sunxi_musb_probe(struct platform_device *pdev)
pdata.mode = MUSB_PORT_MODE_HOST;
break;
#endif
+#if defined CONFIG_USB_MUSB_DUAL_ROLE || defined CONFIG_USB_MUSB_GADGET
+ case USB_DR_MODE_PERIPHERAL:
+ pdata.mode = MUSB_PORT_MODE_GADGET;
+ break;
+#endif
#ifdef CONFIG_USB_MUSB_DUAL_ROLE
case USB_DR_MODE_OTG:
- glue->extcon = extcon_get_edev_by_phandle(&pdev->dev, 0);
- if (IS_ERR(glue->extcon)) {
- if (PTR_ERR(glue->extcon) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
-
[PATCH v5 resend 0/1] usb: musb: sunxi: Simplify dr_mode handling
Hi Bin Liu, The "USB: Fix of_usb_get_dr_mode_by_phy with a shared phy block" patch on which this patch indirectly depends is in usb-next now: https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=ce15ed4c5dfb3f7757e6611902aed5db253af977 Can you please merge this now? Regards, Hans -- 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 20/21] phy: Add support for Qualcomm's USB HSIC phy
Quoting Neil Armstrong (2016-06-29 02:16:51) > On 06/28/2016 11:58 PM, Stephen Boyd wrote: > > Quoting Neil Armstrong (2016-06-28 01:49:37) > >> On 06/26/2016 09:28 AM, Stephen Boyd wrote: > >>> + uphy->cal_sleep_clk = clk = devm_clk_get(&ulpi->dev, "cal_sleep"); > >>> + if (IS_ERR(clk)) > >>> + return PTR_ERR(clk); > >> > >> Hi Stephen, > >> > >> In the bindings the cal_sleep is marked optional, and I think should be > >> since AFAIK > >> it's not present on MDM9615 for example. > > > > The cal_sleep clk is just the sleep clk then (should be a board clk in > > DT). Sometimes there's a gate in GCC to allow us to turn it off, other > > times there isn't. Either way, it's always wired there so I'll update > > the binding to say it isn't optional. > > Sorry I don't understand ! > What should I do if GCC does not provide a gate here ? And looking at the > driver, it could be optional. You should set the property to point to &sleep_clk which should be under the "clocks" node at the root of the OF tree. For example, see the sleep_clk node in arch/arm/boot/dts/qcom-apq8064.dtsi. -- 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 3/3] dt-bindings: Document the STM32 USB OTG DWC2 core binding
On Tue, Jun 28, 2016 at 5:54 PM, Rob Herring wrote:
> On Fri, Jun 24, 2016 at 03:51:18PM -0300, Bruno Herrera wrote:
>> On Fri, Jun 24, 2016 at 12:41 PM, Rob Herring wrote:
>> > On Tue, Jun 21, 2016 at 11:25:49PM -0300, Bruno Herrera wrote:
>> >> Signed-off-by: Bruno Herrera
>> >> ---
>> >> Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
>> >> 1 file changed, 1 insertion(+)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt
>> >> b/Documentation/devicetree/bindings/usb/dwc2.txt
>> >> index 20a68bf..79e5370 100644
>> >> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>> >> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>> >> @@ -11,6 +11,7 @@ Required properties:
>> >>- "lantiq,arx100-usb": The DWC2 USB controller instance in Lantiq ARX
>> >> SoCs;
>> >>- "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX
>> >> SoCs;
>> >>- snps,dwc2: A generic DWC2 USB controller with default parameters.
>> >> + - st,stm32-fsotg: The DWC2 USB controller instance in STM32F4 SoCs in
>> >> FS mode;
>> >
>> > This should go above snps,dwc2.
>> >
>> Ok, tks!
>>
>> > What determines FS mode vs. HS?
>> >
>> Its more HW design decision.
>> STM32F429/439/469 has two OTG controllers, one that is FS (internal
>> phy) and other that is HS (but can also work in FS mode with
>> internal/external phy)
>> This bind work with both cores FS and HS working with the internal PHY.
>>
>> I tested the following configurations:
>> 1 - STM32F429I-DISCOv1 board (OTG HS working in FS mode internal PHY)
>> 2 - STM32F469I-DISCO board (OTG FS)
>>
>> I did not tested OTG HS core working in FS mode with external PHY (I2C).
>
> You shouldn't be setting the compatible string based on which mode you
> want. So for the HS block, you need a different compatible string than
> the FS block and set the speed in another way (not sure if we have a
> standard way). Or perhaps the phy should determine the speed.
I understand but I dont see how to fix it properly unless we add a
some specific STM32 properties in the DT or in the dwc2_core_params.
In fact there are two cores, and they are different in terms of
functionality (despite of the type of the PHY).
One core is for FS and other is for HS, so they could/should have
different compatible strings because they have different
configurations and are different piece of IP/Hardware(The buffer size
are different, the number of end points and so one, DMA)
But the problem is that the HS core can also support the the FS mode
and in this case is misleading to have a HS core with an FS compatible
string.
Something like that (real case for STM32F429I-DISCO):
&usbotg_hs {
compatible = "st,stm32-fsotg", "snps,dwc2";
dr_mode = "host";
pinctrl-0 = <&usbotg_fs_pins_b>;
pinctrl-names = "default";
status = "okay";
};
Even if the decision is phy based it would lead to a STM32 specific
logic and we would need to figure out how to represent the internal
PHY.
Bruno
>
> Rob
--
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 1/3] usb: dwc2: Add support for STM32F429/439/469 USB OTG in FS mode with internal PHY
On Mon, Jun 27, 2016 at 7:51 PM, John Youn wrote:
> On 6/21/2016 7:26 PM, Bruno Herrera wrote:
>> Signed-off-by: Bruno Herrera
>
> Please add a commit message describing the purpose of your changes,
> some information about the platform you're adding, and the special
> handling of the GGPIO.
>
Ok, no problem.
>> ---
>> drivers/usb/dwc2/core.c | 18 ++
>> drivers/usb/dwc2/core.h | 5 +
>> drivers/usb/dwc2/hcd.c | 12 +++-
>> drivers/usb/dwc2/hw.h | 2 ++
>> drivers/usb/dwc2/platform.c | 37 +
>> 5 files changed, 73 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
>> index 4135a5f..83fbed6 100644
>> --- a/drivers/usb/dwc2/core.c
>> +++ b/drivers/usb/dwc2/core.c
>> @@ -1276,6 +1276,23 @@ static void dwc2_set_param_hibernation(struct
>> dwc2_hsotg *hsotg,
>> hsotg->core_params->hibernation = val;
>> }
>>
>> +static void dwc2_set_param_stm32_powerdown(struct dwc2_hsotg *hsotg,
>> + int val)
>> +{
>> + if (DWC2_OUT_OF_BOUNDS(val, 0, 1)) {
>> + if (val >= 0) {
>> + dev_err(hsotg->dev,
>> + "'%d' invalid for parameter power down\n",
>> + val);
>> + dev_err(hsotg->dev, "power down must be 0 or 1\n");
>> + }
>> + val = 0;
>> + dev_dbg(hsotg->dev, "Setting power down to %d\n", val);
>> + }
>> +
>> + hsotg->core_params->stm32_powerdown = val;
>> +}
>> +
>> /*
>> * This function is called during module intialization to pass module
>> parameters
>> * for the DWC_otg core.
>> @@ -1323,6 +1340,7 @@ void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
>> dwc2_set_param_uframe_sched(hsotg, params->uframe_sched);
>> dwc2_set_param_external_id_pin_ctl(hsotg, params->external_id_pin_ctl);
>> dwc2_set_param_hibernation(hsotg, params->hibernation);
>> + dwc2_set_param_stm32_powerdown(hsotg, params->stm32_powerdown);
>> }
>>
>> /*
>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
>> index 3c58d63..d3e4fcb 100644
>> --- a/drivers/usb/dwc2/core.h
>> +++ b/drivers/usb/dwc2/core.h
>> @@ -386,6 +386,10 @@ enum dwc2_ep0_state {
>> * needed.
>> * 0 - No (default)
>> * 1 - Yes
>> + * @stm32_powerdown: Enable STM32 specific USB FS transceiver power down
>> + * control.
>> + * 0 = USB FS transceiver disabled (default)
>> + * 1 = USB FS transceiver enabled
>> *
>> * The following parameters may be specified when starting the module. These
>> * parameters define how the DWC_otg controller should be configured. A
>> @@ -426,6 +430,7 @@ struct dwc2_core_params {
>> int uframe_sched;
>> int external_id_pin_ctl;
>> int hibernation;
>> + int stm32_powerdown;
>> };
>>
>> /**
>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>> index 2df3d04..4f9bb93 100644
>> --- a/drivers/usb/dwc2/hcd.c
>> +++ b/drivers/usb/dwc2/hcd.c
>> @@ -118,7 +118,7 @@ static void dwc2_init_fs_ls_pclk_sel(struct dwc2_hsotg
>> *hsotg)
>>
>> static int dwc2_fs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
>> {
>> - u32 usbcfg, i2cctl;
>> + u32 usbcfg, usbgpio, i2cctl;
>
> The convention in this driver would be to just call 'usbgpio' -> 'ggpio'
OK.
>
>> int retval = 0;
>>
>> /*
>> @@ -142,6 +142,16 @@ static int dwc2_fs_phy_init(struct dwc2_hsotg *hsotg,
>> bool select_phy)
>> return retval;
>> }
>> }
>> +
>> + if (hsotg->core_params->stm32_powerdown > 0) {
>> + dev_dbg(hsotg->dev, "STM32 FS PHY enabling
>> transceiver\n");
>> + /* STM32 uses the GGPIO register as general core
>> + * configuration register.
>> + */
>> + usbgpio = dwc2_readl(hsotg->regs + GGPIO);
>> + usbgpio |= STM32_OTG_GCCFG_PWRDWN;
>> + dwc2_writel(usbgpio, hsotg->regs + GGPIO);
>> + }
>> }
>>
>> /*
>> diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h
>> index 281b57b..d5f9294 100644
>> --- a/drivers/usb/dwc2/hw.h
>> +++ b/drivers/usb/dwc2/hw.h
>> @@ -224,6 +224,8 @@
>>
>> #define GPVNDCTL HSOTG_REG(0x0034)
>> #define GGPIOHSOTG_REG(0x0038)
>> +#define STM32_OTG_GCCFG_PWRDWN (1 << 16)
>> +
>
> Also, bitfields for GGPIO should be named: GGPIO_xxx
Could it be -> GGPIO_STM32_OTG_GCCFG_PWRDWN
>From my understand the GGPIO are platform/vendor dependent so in the
future some other vendor could use the same bit for other purpose.
Should we keep it defined here or only in the scope it is used?
>
>
>> #define GUID HSOTG_REG(0x003c)
>> #d
Re: [PATCH 1/3] usb: dwc2: Add support for STM32F429/439/469 USB OTG in FS mode with internal PHY
On 6/29/2016 12:47 PM, Bruno Herrera wrote:
> On Mon, Jun 27, 2016 at 7:51 PM, John Youn wrote:
>> On 6/21/2016 7:26 PM, Bruno Herrera wrote:
>>> Signed-off-by: Bruno Herrera
>>
>> Please add a commit message describing the purpose of your changes,
>> some information about the platform you're adding, and the special
>> handling of the GGPIO.
>>
> Ok, no problem.
>
>>> ---
>>> drivers/usb/dwc2/core.c | 18 ++
>>> drivers/usb/dwc2/core.h | 5 +
>>> drivers/usb/dwc2/hcd.c | 12 +++-
>>> drivers/usb/dwc2/hw.h | 2 ++
>>> drivers/usb/dwc2/platform.c | 37 +
>>> 5 files changed, 73 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
>>> index 4135a5f..83fbed6 100644
>>> --- a/drivers/usb/dwc2/core.c
>>> +++ b/drivers/usb/dwc2/core.c
>>> @@ -1276,6 +1276,23 @@ static void dwc2_set_param_hibernation(struct
>>> dwc2_hsotg *hsotg,
>>> hsotg->core_params->hibernation = val;
>>> }
>>>
>>> +static void dwc2_set_param_stm32_powerdown(struct dwc2_hsotg *hsotg,
>>> + int val)
>>> +{
>>> + if (DWC2_OUT_OF_BOUNDS(val, 0, 1)) {
>>> + if (val >= 0) {
>>> + dev_err(hsotg->dev,
>>> + "'%d' invalid for parameter power down\n",
>>> + val);
>>> + dev_err(hsotg->dev, "power down must be 0 or 1\n");
>>> + }
>>> + val = 0;
>>> + dev_dbg(hsotg->dev, "Setting power down to %d\n", val);
>>> + }
>>> +
>>> + hsotg->core_params->stm32_powerdown = val;
>>> +}
>>> +
>>> /*
>>> * This function is called during module intialization to pass module
>>> parameters
>>> * for the DWC_otg core.
>>> @@ -1323,6 +1340,7 @@ void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
>>> dwc2_set_param_uframe_sched(hsotg, params->uframe_sched);
>>> dwc2_set_param_external_id_pin_ctl(hsotg,
>>> params->external_id_pin_ctl);
>>> dwc2_set_param_hibernation(hsotg, params->hibernation);
>>> + dwc2_set_param_stm32_powerdown(hsotg, params->stm32_powerdown);
>>> }
>>>
>>> /*
>>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
>>> index 3c58d63..d3e4fcb 100644
>>> --- a/drivers/usb/dwc2/core.h
>>> +++ b/drivers/usb/dwc2/core.h
>>> @@ -386,6 +386,10 @@ enum dwc2_ep0_state {
>>> * needed.
>>> * 0 - No (default)
>>> * 1 - Yes
>>> + * @stm32_powerdown: Enable STM32 specific USB FS transceiver power down
>>> + * control.
>>> + * 0 = USB FS transceiver disabled (default)
>>> + * 1 = USB FS transceiver enabled
>>> *
>>> * The following parameters may be specified when starting the module.
>>> These
>>> * parameters define how the DWC_otg controller should be configured. A
>>> @@ -426,6 +430,7 @@ struct dwc2_core_params {
>>> int uframe_sched;
>>> int external_id_pin_ctl;
>>> int hibernation;
>>> + int stm32_powerdown;
>>> };
>>>
>>> /**
>>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>>> index 2df3d04..4f9bb93 100644
>>> --- a/drivers/usb/dwc2/hcd.c
>>> +++ b/drivers/usb/dwc2/hcd.c
>>> @@ -118,7 +118,7 @@ static void dwc2_init_fs_ls_pclk_sel(struct dwc2_hsotg
>>> *hsotg)
>>>
>>> static int dwc2_fs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
>>> {
>>> - u32 usbcfg, i2cctl;
>>> + u32 usbcfg, usbgpio, i2cctl;
>>
>> The convention in this driver would be to just call 'usbgpio' -> 'ggpio'
>
> OK.
>>
>>> int retval = 0;
>>>
>>> /*
>>> @@ -142,6 +142,16 @@ static int dwc2_fs_phy_init(struct dwc2_hsotg *hsotg,
>>> bool select_phy)
>>> return retval;
>>> }
>>> }
>>> +
>>> + if (hsotg->core_params->stm32_powerdown > 0) {
>>> + dev_dbg(hsotg->dev, "STM32 FS PHY enabling
>>> transceiver\n");
>>> + /* STM32 uses the GGPIO register as general core
>>> + * configuration register.
>>> + */
>>> + usbgpio = dwc2_readl(hsotg->regs + GGPIO);
>>> + usbgpio |= STM32_OTG_GCCFG_PWRDWN;
>>> + dwc2_writel(usbgpio, hsotg->regs + GGPIO);
>>> + }
>>> }
>>>
>>> /*
>>> diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h
>>> index 281b57b..d5f9294 100644
>>> --- a/drivers/usb/dwc2/hw.h
>>> +++ b/drivers/usb/dwc2/hw.h
>>> @@ -224,6 +224,8 @@
>>>
>>> #define GPVNDCTL HSOTG_REG(0x0034)
>>> #define GGPIOHSOTG_REG(0x0038)
>>> +#define STM32_OTG_GCCFG_PWRDWN (1 << 16)
>>> +
>>
>> Also, bitfields for GGPIO should be named: GGPIO_xxx
>
> Could it be -> GGPIO_STM32_OTG_GCCFG_PWRDWN
> From my understand the GGPIO are platform/vendor dependent so in the
> future
Re: [PATCH v5 resend 0/1] usb: musb: sunxi: Simplify dr_mode handling
Hi, On Wed, Jun 29, 2016 at 08:16:39PM +0200, Hans de Goede wrote: > Hi Bin Liu, > > The "USB: Fix of_usb_get_dr_mode_by_phy with a shared phy block" > patch on which this patch indirectly depends is in usb-next now: > > https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=ce15ed4c5dfb3f7757e6611902aed5db253af977 > > Can you please merge this now? Sure. > > Regards, > > Hans Regards, -Bin. -- 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 4/7] reset: Supply *_shared variant calls when using *_optional APIs
Hi Lee,
On Wed, Jun 29, 2016 at 10:37:34AM +0100, Lee Jones wrote:
[...]
> > > > If that is not the case, go ahead. Otherwise I could provide
> > > > a tag at commit 2485394d9e0b ("reset: TRIVIAL: Add line break at same
> > > > place for similar APIs") for you to merge.
> > >
> > > Yes please. If you could tag a branch containing commit:
> > >
> > > reset: Supply *_shared variant calls when using *_optional APIs
> > >
> > > ... that would be perfect. Would you mind sending me the tag name
> > > ASAP please, since I am ready to send my pull-request to Linus.
> >
> > git://git.pengutronix.de/git/pza/linux.git tags/reset-shared-optional
>
> Ah! If I rebase on that tag, I will also take in all of the patches
> between it and v4.7-rc1, which is 14 commits. Is there any way you
> can place it onto it's own branch, then we both merge it into our
> respective trees?
I have now moved your patches into their own branch at
git://git.pengutronix.de/git/pza/linux.git tags/reset-explicit-api
and merged that back in.
regards
Philipp
--
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: ftdi_sio: overrun errors
Am 2016-05-27 20:02, schrieb Johan Hovold: On Fri, May 13, 2016 at 12:17:24PM +0200, mich...@walle.cc wrote: Hi, if the internal buffer is full, a read() returns a steady stream of zeros until one valid character is received. According to my experiments this happens if the FT232 receives characters while the device is not opened. After the 257th byte the device returns overrun errors, which are translated to zero bytes (with TTY_OVERRUN flag set). The 257 bytes makes sense because the internal RX FIFO is 256 bytes and there is room for one more byte in the receive register, before the overrun occurs. Unfortunately, this happens until one (valid?) character is received. The FT232RL seems to set the overrun flag and only clears it on the next received character. This flag is polled every $poll_interval microseconds and for each single poll there is one zero character placed in the tty buffer (and the overrun error is incremented). Before commit cc01f17d5 (USB: ftdi_sio: re-implement read processing) there was a similar note in the driver: /* if a parity error is detected you get status packets forever until a character is sent without a parity error. This doesn't work well since the application receives a never ending stream of bad data - even though new data hasn't been sent. Therefore I (bill) have taken this out. However - this might make sense for framing errors and so on so I am leaving the code in for now. */ So I guess this is still true for the parity error, but in this case there will be no character inserted into the tty buffer. Only the counter will be incremented indefinitely. I guess this is not the correct behaviour, either? The first byte of the status packet is compared to a saved "prev_status", maybe we should do the same for the second byte, too? Btw. this does not happen if, the adapter is plugged in for the first time. I see that the open causes issues a reset, I guess this doesn't reset this error condition. Thanks for reporting this. It sounds like we need to ignore the errors on status-only packets, possibly after checking for changes. What comes directly to my mind is the handling of the break condition. Can't look into that at the moment. But if break conditions are translated into status-only packets, how are two consecutive breaks recognized. Ie. if the break condition flag is sticky (until a new character is received) you can't determine if its one or more. But maybe we are lucky and the chip will send some different pattern. If only there were some specs ;) Might be a second issue since you say you can set the device in an error state which is not cleared when later opening the port. Well, I've already tried a soft reset with no success. So I doubt there is any solution. Seems like a soft reset doesn't reset the internal state entirely. Care to send patches to address this? yeah, in the near future. -michael -- 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 10/21] usb: chipidea: msm: Rely on core to override AHBBURST
On Wed, Jun 29, 2016 at 11:59:21AM -0700, Stephen Boyd wrote: > Quoting Peter Chen (2016-06-28 23:32:11) > > On Sun, Jun 26, 2016 at 12:28:27AM -0700, Stephen Boyd wrote: > > > The core framework already handles setting this parameter with a > > > platform quirk. Add the appropriate flag so that we always set > > > AHBBURST to 0. Technically DT should be doing this, but we always > > > do it for msm chipidea devices so setting the flag in the driver > > > works just as well. > > > > You still need to set AHB burst value at dts, this flag is just for > > override, see below: > > > > ahb-burst-config = <0x0>; > > Right, I have added that to dts now, but the CI_HDRC_OVERRIDE_AHB_BURST > flag allows us to specify it from the platdata structure in the > ci_hdrc_msm.c file. As the value is zero for msm type controllers, I > left it out of the static definition of platdata because all the > non-initialized members of that structure are going to be zero anyway. I > can explicitly set it to zero to make it more clear if you like. I suggest setting it explicitly at dts, at current code, it is set as zero explicitly too:) -- Best Regards, Peter Chen -- 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 19/21] usb: chipidea: msm: Be silent on probe defer errors
On Sun, Jun 26, 2016 at 12:28:36AM -0700, Stephen Boyd wrote:
> If something fails in ci_hdrc_add_device() due to probe defer, we
> shouldn't print an error message. Be silent in this case as we'll
> try probe again later.
>
> Cc: Peter Chen
> Cc: Greg Kroah-Hartman
> Signed-off-by: Stephen Boyd
> ---
> drivers/usb/chipidea/ci_hdrc_msm.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c
> b/drivers/usb/chipidea/ci_hdrc_msm.c
> index 7d191928e55b..2ed9a181f4b6 100644
> --- a/drivers/usb/chipidea/ci_hdrc_msm.c
> +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
> @@ -241,7 +241,8 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
> plat_ci = ci_hdrc_add_device(&pdev->dev, pdev->resource,
>pdev->num_resources, &ci->pdata);
> if (IS_ERR(plat_ci)) {
> - dev_err(&pdev->dev, "ci_hdrc_add_device failed!\n");
> + if (PTR_ERR(plat_ci) != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "ci_hdrc_add_device failed!\n");
> ret = PTR_ERR(plat_ci);
> goto err_mux;
> }
Why not let ret equals to PTR_ERR(plat_ci) first, and using ret to
compare?
--
Best Regards,
Peter Chen
--
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 06/21] usb: chipidea: Initialize and reinitialize phy later
On Wed, Jun 29, 2016 at 06:23:50PM -0700, Stephen Boyd wrote: > Quoting Peter Chen (2016-06-28 19:30:52) > > On Sun, Jun 26, 2016 at 12:28:23AM -0700, Stephen Boyd wrote: > > > The ULPI phy on qcom platforms needs to be initialized and > > > powered on after a USB reset and before we toggle the run/stop > > > bit. Otherwise, the phy locks up and doesn't work properly. > > > > This requirement is so strange, try to see if any other initialization > > sequences. > > I think the problem is that the reset bit also resets the phy because > the phy is part of the same clock domain as the controller. Just a guess > though. > > > > > Since this driver is multi-platforms, I can't accept this change for > > common, if you had to do that, would you please move your changes to > > msm glue layer using CI_HDRC_CONTROLLER_RESET_EVENT and > > CI_HDRC_CONTROLLER_STOPPED_EVENT? Besides, you need to add one flag > > at ci_hdrc_platform_data.flags for your case to avoid normal > > initialization. > > Ok, let me see if I can make this work properly in the glue layer. I > take it that you want me to add a flag for this specific case so that we > don't do any phy control in the core and leave it up to the glue layer > to handle, like CI_HDRC_DISABLE_PHY_CONTROL or something? Yes -- Best Regards, Peter Chen -- 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 08/21] usb: chipidea: Kick OTG state machine for AVVIS with vbus extcon
On Wed, Jun 29, 2016 at 06:19:59PM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-06-28 20:09:13)
> > On Sun, Jun 26, 2016 at 12:28:25AM -0700, Stephen Boyd wrote:
> > > Force the OTG state machine to go forward when we're using an
> > > extcon for vbus detection. In this case, the controller may never
> > > raise an interrupt for AVVIS, so we need to simulate the event by
> > > toggling the appropriate OTG fsm bits and kicking the state
> > > machine again.
> > >
> >
> > Well, I think you may misunderstand the OTG FSM and dual-role.
> > From my and Felipe's point, there are seldom users for USB FSM,
> > there are only OTG FSM spec and related OTG certification.
>
> Probably yes.
>
> >
> > The OTG FSM needs related SoC support, the vbus will be off at
> > several states, and the SRP should be supported by SoC.
> >
> > By default, the dts needs below properties for disabling it if you
> > choose otg fsm support at kernel configuration.
> >
> > &usbotg1 {
> > vbus-supply = <®_usb_otg1_vbus>;
> > srp-disable;
> > hnp-disable;
> > adp-disable;
> > status = "okay";
> > };
> >
> > See Documentation/devicetree/bindings/usb/generic.txt.
>
> Does this mean we should be setting all those properties if we're using
> an extcon for vbus and id?
It is not related to how we know vbus and id. If your controller is
otg-capable, and you don't want to enable OTG FSM (just want dual-role),
you should set them at dts since the zImage is multi-platforms, the
CONFIG_USB_OTG and CONFIG_USB_OTG_FSM may be chosen.
> I have noticed that vbus is powered off after
> some time when no device is connected and we're in A_HOST state because
> the timeout for a B device connection happens.
I think it is not you want, but it is OTG compliance.
--
Best Regards,
Peter Chen
--
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 12/21] usb: chipidea: msm: Keep device runtime enabled
On Wed, Jun 29, 2016 at 05:43:30PM -0700, Stephen Boyd wrote: > Quoting Peter Chen (2016-06-28 23:46:00) > > On Sun, Jun 26, 2016 at 12:28:29AM -0700, Stephen Boyd wrote: > > > Sometimes the usb wrapper device is part of a power domain that > > > needs to stay on as long as the device is active. Let's get and > > > put the device in driver probe/remove so that we keep the power > > > domain powered as long as the device is attached. We can fine > > > tune this later to handle wakeup interrupts, etc. for finer grain > > > power management later, but this is necessary to make sure we can > > > keep accessing the device right now. > > > > Since some of the controllers work abnormal if we enables runtime > > pm unconditionally, so I use one system flag CI_HDRC_SUPPORTS_RUNTIME_PM > > for it. I can't understand why you can't access device without enable > > parent's runtime pm, the controller will not enter runtime suspend > > without that flag. > > Correct, the child device of ci_hdrc_msm will be able to do runtime PM > and keep the parent enabled if the CI_HDRC_SUPPORTS_RUNTIME_PM flag is > set. But even if that flag isn't set, the ci_hdrc_msm driver is calling > pm_runtime_enable() on the same device that it would be called on if the > CI_HDRC_SUPPORTS_RUNTIME_PM flag was set. That allows runtime PM > transition of child devices such as the usb ports (usb1-port1 for > example) to propagate up all the way to the ci_hdrc_msm device and > disable any attached power domains. Sorry, I can't get you. If the chipidea core's runtime is disabled, the port under the controller will not be in runtime suspended, only the bus will be in suspended due to USB core enables runtime PM by default. > > Why don't we call runtime PM functions on the ci->dev for all cases of > ci->supports_runtime_pm? It seems like the glue drivers should be > managing their own device power states and the ci->dev should be managed > by core.c code. > This is current design. Chipidea core manages portsc.phcd and PHY's PM (through PHY's API), and glue layer manages its own clocks on the system bus for register visit (and data transfer if necessary). -- Best Regards, Peter Chen -- 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 13/21] usb: chipidea: msm: Allow core to get usb phy
On Wed, Jun 29, 2016 at 12:31:18PM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-06-29 04:34:11)
> > On Wed, Jun 29, 2016 at 02:48:11PM +0800, Peter Chen wrote:
> > > On Sun, Jun 26, 2016 at 12:28:30AM -0700, Stephen Boyd wrote:
> > > > @@ -53,21 +44,9 @@ static struct ci_hdrc_platform_data
> > > > ci_hdrc_msm_platdata = {
> > > > static int ci_hdrc_msm_probe(struct platform_device *pdev)
> > > > {
> > > > struct platform_device *plat_ci;
> > > > - struct usb_phy *phy;
> > > >
> > > > dev_dbg(&pdev->dev, "ci_hdrc_msm_probe\n");
> > > >
> > > > - /*
> > > > -* OTG(PHY) driver takes care of PHY initialization, clock
> > > > management,
> > > > -* powering up VBUS, mapping of registers address space and power
> > > > -* management.
> > > > -*/
> > > > - phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0);
> > > > - if (IS_ERR(phy))
> > > > - return PTR_ERR(phy);
> > > > -
> > > > - ci_hdrc_msm_platdata.usb_phy = phy;
> > > > -
> > > > plat_ci = ci_hdrc_add_device(&pdev->dev,
> > > > pdev->resource, pdev->num_resources,
> > > > &ci_hdrc_msm_platdata);
> > > > --
> > >
> >
> > Wait, how about the UTMI PHY? You don't have a platform which needs
> > to get PHY through the phandle?
>
> Sorry I don't understand the question. What is the UTMI PHY? We need to
> get the phy through phandles. The only boards that are using ci_hdrc_msm
> are DT enabled boards.
UTMI PHY is the PHY inside the SoC, I just want to confirm all your PHYs
are at ulpi bus, if they are, you can do that, else, you may need above
way to get the PHY through PHY node.
--
Best Regards,
Peter Chen
--
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 15/21] usb: chipidea: msm: Mux over secondary phy at the right time
On Wed, Jun 29, 2016 at 12:28:58PM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-06-29 01:08:52)
> > On Sun, Jun 26, 2016 at 12:28:32AM -0700, Stephen Boyd wrote:
> > > We need to pick the correct phy at runtime based on how the SoC
> > > has been wired onto the board. If the secondary phy is used, take
> > > it out of reset and mux over to it by writing into the TCSR
> > > register. Make sure to do this on reset too, because this
> > > register is reset to the default value (primary phy) after the
> > > RESET bit is set in USBCMD.
> > >
> >
> > I am curious when you need the secondary phy?
> >
> > > Cc: Peter Chen
> > > Cc: Greg Kroah-Hartman
> > > Signed-off-by: Stephen Boyd
> > > ---
> > > drivers/usb/chipidea/ci_hdrc_msm.c | 78
> > > +++---
> > > 1 file changed, 73 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c
> > > b/drivers/usb/chipidea/ci_hdrc_msm.c
> > > index 40249b0e3e93..df0f8b31db4f 100644
> > > --- a/drivers/usb/chipidea/ci_hdrc_msm.c
> > > +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
> > > @@ -8,30 +8,40 @@
> > > #include
> > > #include
> > > #include
> > > -#include
> > > #include
> > > #include
> > > #include
> > > +#include
> > > +#include
> > > +#include
> > >
> > > #include "ci.h"
> > >
> > > #define HS_PHY_AHB_MODE 0x0098
> > > +#define HS_PHY_SEC_CTRL 0x0278
> > > +# define HS_PHY_DIG_CLAMP_N BIT(16)
> > >
> >
> > One space at the beginning, and keep alignment.
> >
> > > struct ci_hdrc_msm {
> > > struct platform_device *ci;
> > > struct clk *core_clk;
> > > struct clk *iface_clk;
> > > + bool secondary_phy;
> > > + void __iomem *base;
> > > };
> > >
> > > static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
> > > {
> > > - struct device *dev = ci->gadget.dev.parent;
> > > + struct device *dev = ci->dev->parent;
> > > + struct ci_hdrc_msm *msm_ci = dev_get_drvdata(dev);
> > >
> > > switch (event) {
> > > case CI_HDRC_CONTROLLER_RESET_EVENT:
> > > dev_dbg(dev, "CI_HDRC_CONTROLLER_RESET_EVENT received\n");
> > > /* use AHB transactor, allow posted data writes */
> > > hw_write_id_reg(ci, HS_PHY_AHB_MODE, 0x, 0x8);
> > > + if (msm_ci->secondary_phy)
> > > + hw_write_id_reg(ci, HS_PHY_SEC_CTRL,
> > > HS_PHY_DIG_CLAMP_N,
> > > + HS_PHY_DIG_CLAMP_N);
> > > break;
> > > default:
> > > dev_dbg(dev, "unknown ci_hdrc event\n");
> > > @@ -49,12 +59,58 @@ static struct ci_hdrc_platform_data
> > > ci_hdrc_msm_platdata = {
> > > .notify_event = ci_hdrc_msm_notify_event,
> > > };
> > >
> > > +static int ci_hdrc_msm_mux_phy(struct ci_hdrc_msm *ci,
> > > +struct platform_device *pdev)
> > > +{
> > > + struct regmap *regmap;
> > > + struct device_node *syscon;
> > > + struct device *dev = &pdev->dev;
> > > + u32 off, val;
> > > + int ret;
> > > +
> > > + syscon = of_parse_phandle(dev->of_node, "phy-select", 0);
> > > + if (!syscon)
> > > + return 0;
> > > +
> > > + regmap = syscon_node_to_regmap(syscon);
> > > + if (IS_ERR(regmap))
> > > + return PTR_ERR(regmap);
> > > +
> > > + ret = of_property_read_u32_index(dev->of_node, "phy-select", 1,
> > > &off);
> > > + if (ret < 0) {
> > > + dev_err(dev, "no offset in syscon\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ret = of_property_read_u32_index(dev->of_node, "phy-select", 2,
> > > &val);
> > > + if (ret < 0) {
> > > + dev_err(dev, "no value in syscon\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ret = regmap_write(regmap, off, val);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ci->secondary_phy = !!val;
> > > + if (ci->secondary_phy) {
> > > + val = readl_relaxed(ci->base + HS_PHY_SEC_CTRL);
> > > + val |= HS_PHY_DIG_CLAMP_N;
> > > + writel_relaxed(val, ci->base + HS_PHY_SEC_CTRL);
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static int ci_hdrc_msm_probe(struct platform_device *pdev)
> > > {
> > > struct ci_hdrc_msm *ci;
> > > struct platform_device *plat_ci;
> > > struct clk *clk;
> > > struct reset_control *reset;
> > > + struct resource *res;
> > > + void __iomem *base;
> > > + resource_size_t size;
> > > int ret;
> > >
> > > dev_dbg(&pdev->dev, "ci_hdrc_msm_probe\n");
> > > @@ -76,6 +132,15 @@ static int ci_hdrc_msm_probe(struct platform_device
> > > *pdev)
> > > if (IS_ERR(clk))
> > > return PTR_ERR(clk);
> > >
> > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > + if (!r
RE: [PATCH 08/21] usb: chipidea: Kick OTG state machine for AVVIS with vbus extcon
Hi Stephen,
> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of Peter Chen
> Sent: Thursday, June 30, 2016 9:27 AM
> To: Stephen Boyd
> Cc: linux-usb@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> linux-ker...@vger.kernel.org; linux-arm-...@vger.kernel.org; Andy Gross
> ; Bjorn Andersson ;
> Neil Armstrong ; Arnd Bergmann ;
> Felipe Balbi ; Peter Chen ; Greg
> Kroah-Hartman
> Subject: Re: [PATCH 08/21] usb: chipidea: Kick OTG state machine for AVVIS
> with vbus extcon
>
> On Wed, Jun 29, 2016 at 06:19:59PM -0700, Stephen Boyd wrote:
> > Quoting Peter Chen (2016-06-28 20:09:13)
> > > On Sun, Jun 26, 2016 at 12:28:25AM -0700, Stephen Boyd wrote:
> > > > Force the OTG state machine to go forward when we're using an
> > > > extcon for vbus detection. In this case, the controller may never
> > > > raise an interrupt for AVVIS, so we need to simulate the event by
> > > > toggling the appropriate OTG fsm bits and kicking the state
> > > > machine again.
> > > >
> > >
> > > Well, I think you may misunderstand the OTG FSM and dual-role.
> > > From my and Felipe's point, there are seldom users for USB FSM,
> > > there are only OTG FSM spec and related OTG certification.
> >
> > Probably yes.
> >
> > >
> > > The OTG FSM needs related SoC support, the vbus will be off at
> > > several states, and the SRP should be supported by SoC.
> > >
> > > By default, the dts needs below properties for disabling it if you
> > > choose otg fsm support at kernel configuration.
> > >
> > > &usbotg1 {
> > > vbus-supply = <®_usb_otg1_vbus>;
> > > srp-disable;
> > > hnp-disable;
> > > adp-disable;
> > > status = "okay";
> > > };
> > >
> > > See Documentation/devicetree/bindings/usb/generic.txt.
> >
> > Does this mean we should be setting all those properties if we're
> > using an extcon for vbus and id?
>
> It is not related to how we know vbus and id. If your controller is otg-
> capable, and you don't want to enable OTG FSM (just want dual-role), you
> should set them at dts since the zImage is multi-platforms, the
> CONFIG_USB_OTG and CONFIG_USB_OTG_FSM may be chosen.
>
> > I have noticed that vbus is powered off after some time when no device
> > is connected and we're in A_HOST state because the timeout for a B
> > device connection happens.
>
> I think it is not you want, but it is OTG compliance.
For simple, if you don't want OTG(i.e HNP&SRP) at all, just needs
dual role, you may disable CONFIG_USB_OTG and CONFIG_USB_OTG_FSM
in your menuconfig, then you don't need touch all those properties.
Li Jun
>
> --
>
> Best Regards,
> Peter Chen
> --
> 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
--
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 v14 4/4] power: wm831x_power: Support USB charger current limit management
Integrate with the newly added USB charger interface to limit the current
we draw from the USB input based on the input device configuration
identified by the USB stack, allowing us to charge more quickly from high
current inputs without drawing more current than specified from others.
Signed-off-by: Mark Brown
Signed-off-by: Baolin Wang
Acked-by: Lee Jones
Acked-by: Charles Keepax
Acked-by: Peter Chen
Acked-by: Sebastian Reichel
---
drivers/power/wm831x_power.c | 69 ++
include/linux/mfd/wm831x/pdata.h |3 ++
2 files changed, 72 insertions(+)
diff --git a/drivers/power/wm831x_power.c b/drivers/power/wm831x_power.c
index 7082301..cef1812 100644
--- a/drivers/power/wm831x_power.c
+++ b/drivers/power/wm831x_power.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include
@@ -31,6 +32,8 @@ struct wm831x_power {
char usb_name[20];
char battery_name[20];
bool have_battery;
+ struct usb_charger *usb_charger;
+ struct notifier_block usb_notify;
};
static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
@@ -125,6 +128,43 @@ static enum power_supply_property wm831x_usb_props[] = {
POWER_SUPPLY_PROP_VOLTAGE_NOW,
};
+/* In milliamps */
+static const unsigned int wm831x_usb_limits[] = {
+ 0,
+ 2,
+ 100,
+ 500,
+ 900,
+ 1500,
+ 1800,
+ 550,
+};
+
+static int wm831x_usb_limit_change(struct notifier_block *nb,
+ unsigned long limit, void *data)
+{
+ struct wm831x_power *wm831x_power = container_of(nb,
+struct wm831x_power,
+usb_notify);
+ unsigned int i, best;
+
+ /* Find the highest supported limit */
+ best = 0;
+ for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
+ if (limit >= wm831x_usb_limits[i] &&
+ wm831x_usb_limits[best] < wm831x_usb_limits[i])
+ best = i;
+ }
+
+ dev_dbg(wm831x_power->wm831x->dev,
+ "Limiting USB current to %umA", wm831x_usb_limits[best]);
+
+ wm831x_set_bits(wm831x_power->wm831x, WM831X_POWER_STATE,
+ WM831X_USB_ILIM_MASK, best);
+
+ return 0;
+}
+
/*
* Battery properties
*/
@@ -607,8 +647,31 @@ static int wm831x_power_probe(struct platform_device *pdev)
}
}
+ if (wm831x_pdata && wm831x_pdata->usb_gadget) {
+ power->usb_charger =
+ usb_charger_find_by_name(wm831x_pdata->usb_gadget);
+ if (IS_ERR(power->usb_charger)) {
+ ret = PTR_ERR(power->usb_charger);
+ dev_err(&pdev->dev,
+ "Failed to find USB gadget: %d\n", ret);
+ goto err_bat_irq;
+ }
+
+ power->usb_notify.notifier_call = wm831x_usb_limit_change;
+
+ ret = usb_charger_register_notify(power->usb_charger,
+ &power->usb_notify);
+ if (ret != 0) {
+ dev_err(&pdev->dev,
+ "Failed to register notifier: %d\n", ret);
+ goto err_usb_charger;
+ }
+ }
+
return ret;
+err_usb_charger:
+ /* put_device on charger */
err_bat_irq:
--i;
for (; i >= 0; i--) {
@@ -637,6 +700,12 @@ static int wm831x_power_remove(struct platform_device
*pdev)
struct wm831x *wm831x = wm831x_power->wm831x;
int irq, i;
+ if (wm831x_power->usb_charger) {
+ usb_charger_unregister_notify(wm831x_power->usb_charger,
+ &wm831x_power->usb_notify);
+ /* Free charger */
+ }
+
for (i = 0; i < ARRAY_SIZE(wm831x_bat_irqs); i++) {
irq = wm831x_irq(wm831x,
platform_get_irq_byname(pdev,
diff --git a/include/linux/mfd/wm831x/pdata.h b/include/linux/mfd/wm831x/pdata.h
index dcc9631..5af8399 100644
--- a/include/linux/mfd/wm831x/pdata.h
+++ b/include/linux/mfd/wm831x/pdata.h
@@ -126,6 +126,9 @@ struct wm831x_pdata {
/** The driver should initiate a power off sequence during shutdown */
bool soft_shutdown;
+ /** dev_name of USB charger gadget to integrate with */
+ const char *usb_gadget;
+
int irq_base;
int gpio_base;
int gpio_defaults[WM831X_GPIO_NUM];
--
1.7.9.5
--
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.h
[PATCH v14 0/4] Introduce usb charger framework to deal with the usb gadget power negotation
Currently the Linux kernel does not provide any standard integration of this feature that integrates the USB subsystem with the system power regulation provided by PMICs meaning that either vendors must add this in their kernels or USB gadget devices based on Linux (such as mobile phones) may not behave as they should. Thus provide a standard framework for doing this in kernel. Now introduce one user with wm831x_power to support and test the usb charger, which is pending testing. Moreover there may be other potential users will use it in future. Changes since v13: - Remove the charger checking in usb_gadget_vbus_draw() function. - Rename some functions in charger.c file. - Rebase on git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git tags/usb-for-v4.8 Changes since v12: - Remove the class and device things. - Link usb charger to udc-core.ko. - Create one "charger" subdirectory which holds all charger-related attributes. Changes since v11: - Reviewed and tested by Li Jun. Changes since v10: - Introduce usb_charger_get_state() function to check charger state. - Remove the mutex lock in usb_charger_set_cur_limit_by_type() function in case will be issued in atomic context. Baolin Wang (4): usb: gadget: Introduce the usb charger framework usb: gadget: Support for the usb charger framework usb: gadget: Integrate with the usb gadget supporting for usb charger power: wm831x_power: Support USB charger current limit management drivers/power/wm831x_power.c | 69 drivers/usb/gadget/Kconfig |8 + drivers/usb/gadget/udc/Makefile |1 + drivers/usb/gadget/udc/charger.c | 743 ++ drivers/usb/gadget/udc/core.c| 17 + include/linux/mfd/wm831x/pdata.h |3 + include/linux/usb/charger.h | 164 + include/linux/usb/gadget.h |3 + include/uapi/linux/usb/charger.h | 31 ++ 9 files changed, 1039 insertions(+) create mode 100644 drivers/usb/gadget/udc/charger.c create mode 100644 include/linux/usb/charger.h create mode 100644 include/uapi/linux/usb/charger.h -- 1.7.9.5 -- 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 v14 2/4] usb: gadget: Support for the usb charger framework
For supporting the usb charger, it adds the usb_charger_init() and
usb_charger_exit() functions for usb charger initialization and exit.
It will report to the usb charger when the gadget state is changed,
then the usb charger can do the power things.
Signed-off-by: Baolin Wang
Reviewed-by: Li Jun
Tested-by: Li Jun
---
drivers/usb/gadget/udc/core.c | 17 +
include/linux/usb/gadget.h|3 +++
2 files changed, 20 insertions(+)
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index ff8685e..bdf1874 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -25,6 +25,7 @@
#include
#include
+#include
#include
#include
#include
@@ -578,12 +579,17 @@ EXPORT_SYMBOL_GPL(usb_gadget_vbus_connect);
* reporting how much power the device may consume. For example, this
* could affect how quickly batteries are recharged.
*
+ * It will also notify the USB charger how much power the device may
+ * consume if there is a USB charger linking with the gadget.
+ *
* Returns zero on success, else negative errno.
*/
int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA)
{
int ret = 0;
+ usb_charger_set_cur_limit_by_gadget(gadget, mA);
+
if (!gadget->ops->vbus_draw) {
ret = -EOPNOTSUPP;
goto out;
@@ -965,6 +971,9 @@ static void usb_gadget_state_work(struct work_struct *work)
struct usb_gadget *gadget = work_to_gadget(work);
struct usb_udc *udc = gadget->udc;
+ /* when the gadget state is changed, then report to USB charger */
+ usb_charger_plug_by_gadget(gadget, gadget->state);
+
if (udc)
sysfs_notify(&udc->dev.kobj, NULL, "state");
}
@@ -1134,6 +1143,10 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
if (ret)
goto err4;
+ ret = usb_charger_init(gadget);
+ if (ret)
+ goto err5;
+
usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED);
udc->vbus = true;
@@ -1154,6 +1167,9 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
return 0;
+err5:
+ device_del(&udc->dev);
+
err4:
list_del(&udc->list);
mutex_unlock(&udc_lock);
@@ -1262,6 +1278,7 @@ void usb_del_gadget_udc(struct usb_gadget *gadget)
kobject_uevent(&udc->dev.kobj, KOBJ_REMOVE);
flush_work(&gadget->work);
device_unregister(&udc->dev);
+ usb_charger_exit(gadget);
device_unregister(&gadget->dev);
}
EXPORT_SYMBOL_GPL(usb_del_gadget_udc);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 612dbdf..c864b51 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#define UDC_TRACE_STR_MAX 512
@@ -328,6 +329,7 @@ struct usb_gadget_ops {
* @in_epnum: last used in ep number
* @mA: last set mA value
* @otg_caps: OTG capabilities of this gadget.
+ * @charger: Negotiate the power with the usb charger.
* @sg_supported: true if we can handle scatter-gather
* @is_otg: True if the USB device port uses a Mini-AB jack, so that the
* gadget driver must provide a USB OTG descriptor.
@@ -385,6 +387,7 @@ struct usb_gadget {
unsignedin_epnum;
unsignedmA;
struct usb_otg_caps *otg_caps;
+ struct usb_charger *charger;
unsignedsg_supported:1;
unsignedis_otg:1;
--
1.7.9.5
--
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 v14 3/4] usb: gadget: Integrate with the usb gadget supporting for usb charger
When the usb gadget supporting for usb charger is ready, the usb charger
can implement the usb_charger_plug_by_gadget() function, usb_charger_exit()
function and dev_to_uchger() function by getting 'struct usb_charger' from
'struct gadget'.
Signed-off-by: Baolin Wang
Reviewed-by: Li Jun
Tested-by: Li Jun
---
drivers/usb/gadget/udc/charger.c | 70 --
1 file changed, 67 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/charger.c b/drivers/usb/gadget/udc/charger.c
index 7abecf3..45c0bf1b 100644
--- a/drivers/usb/gadget/udc/charger.c
+++ b/drivers/usb/gadget/udc/charger.c
@@ -35,7 +35,9 @@ static unsigned int __usb_charger_get_cur_limit(struct
usb_charger *uchger);
static struct usb_charger *dev_to_uchger(struct device *dev)
{
- return NULL;
+ struct usb_gadget *gadget = container_of(dev, struct usb_gadget, dev);
+
+ return gadget->charger;
}
/*
@@ -308,7 +310,13 @@ static int __usb_charger_set_cur_limit_by_type(struct
usb_charger *uchger,
int usb_charger_set_cur_limit_by_gadget(struct usb_gadget *gadget,
unsigned int cur_limit)
{
- return 0;
+ struct usb_charger *uchger = gadget->charger;
+
+ if (!uchger)
+ return 0;
+
+ return __usb_charger_set_cur_limit_by_type(uchger, uchger->type,
+ cur_limit);
}
EXPORT_SYMBOL_GPL(usb_charger_set_cur_limit_by_gadget);
@@ -562,11 +570,55 @@ usb_charger_plug_by_extcon(struct notifier_block *nb,
int usb_charger_plug_by_gadget(struct usb_gadget *gadget,
unsigned long state)
{
+ struct usb_charger *uchger = gadget->charger;
+ enum usb_charger_state uchger_state;
+
+ if (WARN(!uchger, "charger can not be NULL"))
+ return -EINVAL;
+
+ /*
+* Report event to power to setting the current limitation
+* for this usb charger when one usb charger state is changed
+* with detecting by usb gadget state.
+*/
+ if (uchger->old_gadget_state != state) {
+ uchger->old_gadget_state = state;
+
+ if (state >= USB_STATE_ATTACHED)
+ uchger_state = USB_CHARGER_PRESENT;
+ else if (state == USB_STATE_NOTATTACHED)
+ uchger_state = USB_CHARGER_REMOVE;
+ else
+ uchger_state = USB_CHARGER_DEFAULT;
+
+ usb_charger_notify_others(uchger, uchger_state);
+ }
+
return 0;
}
EXPORT_SYMBOL_GPL(usb_charger_plug_by_gadget);
/*
+ * usb_charger_unregister() - Unregister a usb charger.
+ * @uchger - the usb charger to be unregistered.
+ */
+static int usb_charger_unregister(struct usb_charger *uchger)
+{
+ if (WARN(!uchger, "charger can not be NULL"))
+ return -EINVAL;
+
+ ida_simple_remove(&usb_charger_ida, uchger->id);
+ sysfs_remove_groups(&uchger->gadget->dev.kobj, usb_charger_groups);
+
+ mutex_lock(&charger_lock);
+ list_del(&uchger->list);
+ mutex_unlock(&charger_lock);
+
+ kfree(uchger);
+ return 0;
+}
+
+/*
* usb_charger_register() - Register a new usb charger.
* @uchger - the new usb charger instance.
*/
@@ -649,6 +701,7 @@ int usb_charger_init(struct usb_gadget *ugadget)
/* register a notifier on a usb gadget device */
uchger->gadget = ugadget;
+ ugadget->charger = uchger;
uchger->old_gadget_state = USB_STATE_NOTATTACHED;
/* register a new usb charger */
@@ -671,7 +724,18 @@ fail:
int usb_charger_exit(struct usb_gadget *ugadget)
{
- return 0;
+ struct usb_charger *uchger = ugadget->charger;
+
+ if (WARN(!uchger, "charger can not be NULL"))
+ return -EINVAL;
+
+ ugadget->charger = NULL;
+ if (uchger->extcon_dev)
+ extcon_unregister_notifier(uchger->extcon_dev,
+ EXTCON_USB,
+ &uchger->extcon_nb.nb);
+
+ return usb_charger_unregister(uchger);
}
MODULE_AUTHOR("Baolin Wang ");
--
1.7.9.5
--
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 v14 1/4] usb: gadget: Introduce the usb charger framework
This patch introduces the usb charger driver based on usb gadget that
makes an enhancement to a power driver. It works well in practice but
that requires a system with suitable hardware.
The basic conception of the usb charger is that, when one usb charger
is added or removed by reporting from the usb gadget state change or
the extcon device state change, the usb charger will report to power
user to set the current limitation.
The usb charger will register notifiees on the usb gadget or the extcon
device to get notified the usb charger state. It also supplies the
notification mechanism to userspace When the usb charger state is changed.
Power user will register a notifiee on the usb charger to get notified
by status changes from the usb charger. It will report to power user
to set the current limitation when detecting the usb charger is added
or removed from extcon device state or usb gadget state.
This patch doesn't yet integrate with the gadget code, so some functions
which rely on the 'gadget' are not completed, that will be implemented
in the following patches.
Signed-off-by: Baolin Wang
Reviewed-by: Li Jun
Tested-by: Li Jun
---
drivers/usb/gadget/Kconfig |8 +
drivers/usb/gadget/udc/Makefile |1 +
drivers/usb/gadget/udc/charger.c | 679 ++
include/linux/usb/charger.h | 164 +
include/uapi/linux/usb/charger.h | 31 ++
5 files changed, 883 insertions(+)
create mode 100644 drivers/usb/gadget/udc/charger.c
create mode 100644 include/linux/usb/charger.h
create mode 100644 include/uapi/linux/usb/charger.h
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 3c3f31c..acea3f7 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -134,6 +134,14 @@ config U_SERIAL_CONSOLE
help
It supports the serial gadget can be used as a console.
+config USB_CHARGER
+ bool "USB charger support"
+ select EXTCON
+ help
+ The usb charger driver based on the usb gadget that makes an
+ enhancement to a power driver which can set the current limitation
+ when the usb charger is added or removed.
+
source "drivers/usb/gadget/udc/Kconfig"
#
diff --git a/drivers/usb/gadget/udc/Makefile b/drivers/usb/gadget/udc/Makefile
index 98e74ed..ede2351 100644
--- a/drivers/usb/gadget/udc/Makefile
+++ b/drivers/usb/gadget/udc/Makefile
@@ -2,6 +2,7 @@
CFLAGS_trace.o := -I$(src)
udc-core-y := core.o trace.o
+udc-core-$(CONFIG_USB_CHARGER) += charger.o
#
# USB peripheral controller drivers
diff --git a/drivers/usb/gadget/udc/charger.c b/drivers/usb/gadget/udc/charger.c
new file mode 100644
index 000..7abecf3
--- /dev/null
+++ b/drivers/usb/gadget/udc/charger.c
@@ -0,0 +1,679 @@
+/*
+ * charger.c -- USB charger driver
+ *
+ * Copyright (C) 2016 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DEFAULT_SDP_CUR_LIMIT 500
+#define DEFAULT_SDP_CUR_LIMIT_SS 900
+#define DEFAULT_DCP_CUR_LIMIT 1500
+#define DEFAULT_CDP_CUR_LIMIT 1500
+#define DEFAULT_ACA_CUR_LIMIT 1500
+
+static DEFINE_IDA(usb_charger_ida);
+static LIST_HEAD(charger_list);
+static DEFINE_MUTEX(charger_lock);
+
+static unsigned int __usb_charger_get_cur_limit(struct usb_charger *uchger);
+
+static struct usb_charger *dev_to_uchger(struct device *dev)
+{
+ return NULL;
+}
+
+/*
+ * charger_current_show() - Show the charger current limit.
+ */
+static ssize_t charger_current_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct usb_charger *uchger = dev_to_uchger(dev);
+
+ return sprintf(buf, "%u\n", __usb_charger_get_cur_limit(uchger));
+}
+static DEVICE_ATTR_RO(charger_current);
+
+/*
+ * charger_type_show() - Show the charger type.
+ *
+ * It can be SDP/DCP/CDP/ACA type, else for unknown type.
+ */
+static ssize_t charger_type_show(struct device *dev,
+struct device_attribute *attr,
+char *buf)
+{
+ struct usb_charger *uchger = dev_to_uchger(dev);
+ int cnt;
+
+ switch (uchger->type) {
+ case SDP_TYPE:
+ cnt = sprintf(buf, "%s\n", "SDP");
+ break;
+ case DCP_TYPE:
+ cnt = sprintf(buf, "%s\n", "DCP");
+ break;
+ case CDP_TYPE:
+ cnt = sprintf(buf, "%s\n", "CDP");
+ break;
+ case ACA_TYPE:
+ cnt = sprintf(buf, "%s\n", "ACA");
+ break;
+ default:
+ cnt = sprintf(buf, "%s
Re: [PATCH v5 resend 0/2] phy-sun4i-usb: peripheral-mode + a31 otg workaround
Hi, On Wednesday 29 June 2016 11:44 PM, Hans de Goede wrote: > Hi Kishon, > > The "USB: Fix of_usb_get_dr_mode_by_phy with a shared phy block" > patch on which this series depends is in usb-next now: > > https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=ce15ed4c5dfb3f7757e6611902aed5db253af977 > > Can you please merge this series? This is not applying cleanly against linux-phy -next. Can you rebase and send? Thanks Kishon -- 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
