From: Lee Jones <lee.jones@linaro.org>
To: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: alokc@codeaurora.org, andy.gross@linaro.org,
david.brown@linaro.org, wsa+renesas@sang-engineering.com,
linus.walleij@linaro.org, balbi@kernel.org,
gregkh@linuxfoundation.org, ard.biesheuvel@linaro.org,
jlhugo@gmail.com, linux-i2c@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-gpio@vger.kernel.org,
linux-usb@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 7/8] usb: dwc3: qcom: Start USB in 'host mode' on the SDM845
Date: Wed, 12 Jun 2019 06:57:36 +0100 [thread overview]
Message-ID: <20190612055736.GO4797@dell> (raw)
In-Reply-To: <20190611223349.GS4814@minitux>
On Tue, 11 Jun 2019, Bjorn Andersson wrote:
> On Mon 10 Jun 01:42 PDT 2019, Lee Jones wrote:
>
> > When booting with Device Tree, the current default boot configuration
> > table option, the request to boot via 'host mode' comes from the
> > 'dr_mode' property.
>
> As I said in my previous review, the default mode for SDM845 is OTG. For
> the MTP specifically we specify the default mode to be peripheral (was
> host).
>
>
> The remaining thing that worries me with this patch is that I do expect
> that at least one of the USB-C ports is OTG. But I am not able to
> conclude anything regarding this and host-only is a good default for
> this type of device, so I suggest that we merge this.
Right. So one thing to consider is that Qualcomm Mobile Dept. do not
use ACPI for Linux, so this patch-set only affects the Laptop
form factor devices, where 'host' is the expected mode.
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Thanks for taking the time to review this Bjorn.
Hopefully we can get Felipe's attention soon.
Felipe,
One thing to think about when taking Bjorn's Reviewed-by into
consideration; although we both work for Linaro, we actually operate
in different teams. Bjorn is on the Qualcomm Landing Team, and as an
ex-Qualcomm employee he is in an excellent position to review these
patches, thus his Review should carry more weight than the usual
co-worker review IMHO.
TIA.
> > A property of the same name can be used inside
> > ACPI tables too. However it is missing from the SDM845's ACPI tables
> > so we have to supply this information using Platform Device Properties
> > instead.
> >
> > This does not change the behaviour of any currently supported devices.
> > The property is only set on ACPI enabled platforms, thus for H/W
> > booting DT, unless a 'dr_mode' property is present, the default is
> > still OTG (On-The-Go) as per [0]. Any new ACPI devices added will
> > also be able to over-ride this implementation by providing a 'dr_mode'
> > property in their ACPI tables. In cases where 'dr_mode' is omitted
> > from the tables AND 'host mode' should not be the default (very
> > unlikely), then we will have to add some way of choosing between them
> > at run time - most likely by ACPI HID.
> >
> > [0] Documentation/devicetree/bindings/usb/generic.txt
> >
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> > drivers/usb/dwc3/dwc3-qcom.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> > index 1e1f12b7991d..55ba04254e38 100644
> > --- a/drivers/usb/dwc3/dwc3-qcom.c
> > +++ b/drivers/usb/dwc3/dwc3-qcom.c
> > @@ -444,6 +444,11 @@ static int dwc3_qcom_clk_init(struct dwc3_qcom *qcom, int count)
> > return 0;
> > }
> >
> > +static const struct property_entry dwc3_qcom_acpi_properties[] = {
> > + PROPERTY_ENTRY_STRING("dr_mode", "host"),
> > + {}
> > +};
> > +
> > static int dwc3_qcom_acpi_register_core(struct platform_device *pdev)
> > {
> > struct dwc3_qcom *qcom = platform_get_drvdata(pdev);
> > @@ -488,6 +493,13 @@ static int dwc3_qcom_acpi_register_core(struct platform_device *pdev)
> > goto out;
> > }
> >
> > + ret = platform_device_add_properties(qcom->dwc3,
> > + dwc3_qcom_acpi_properties);
> > + if (ret < 0) {
> > + dev_err(&pdev->dev, "failed to add properties\n");
> > + goto out;
> > + }
> > +
> > ret = platform_device_add(qcom->dwc3);
> > if (ret)
> > dev_err(&pdev->dev, "failed to add device\n");
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
next prev parent reply other threads:[~2019-06-12 5:57 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-10 8:42 [PATCH v3 1/8] i2c: i2c-qcom-geni: Provide support for ACPI Lee Jones
2019-06-10 8:42 ` [PATCH v3 2/8] i2c: i2c-qcom-geni: Signify successful driver probe Lee Jones
2019-06-11 19:40 ` Bjorn Andersson
2019-06-10 8:42 ` [PATCH v3 3/8] pinctrl: msm: Add ability for drivers to supply a reserved GPIO list Lee Jones
2019-06-12 7:25 ` Linus Walleij
2019-06-10 8:42 ` [PATCH v3 4/8] pinctrl: qcom: sdm845: Provide ACPI support Lee Jones
2019-06-10 8:46 ` Ard Biesheuvel
2019-06-10 8:55 ` Lee Jones
2019-06-10 9:03 ` Ard Biesheuvel
2019-06-10 9:22 ` Lee Jones
2019-06-10 10:20 ` Ard Biesheuvel
2019-06-11 18:39 ` Bjorn Andersson
2019-06-12 7:18 ` Linus Walleij
2019-06-11 18:40 ` Bjorn Andersson
2019-06-10 8:42 ` [PATCH v3 5/8] soc: qcom: geni: Add support for ACPI Lee Jones
2019-06-10 8:47 ` Ard Biesheuvel
2019-06-10 8:42 ` [PATCH v3 6/8] usb: dwc3: qcom: Add support for booting with ACPI Lee Jones
2019-06-10 8:42 ` [PATCH v3 7/8] usb: dwc3: qcom: Start USB in 'host mode' on the SDM845 Lee Jones
2019-06-11 22:33 ` Bjorn Andersson
2019-06-12 5:57 ` Lee Jones [this message]
2019-06-10 8:42 ` [PATCH v3 8/8] usb: dwc3: qcom: Improve error handling Lee Jones
2019-06-10 8:44 ` [PATCH v3 1/8] i2c: i2c-qcom-geni: Provide support for ACPI Ard Biesheuvel
2019-06-12 10:34 ` Wolfram Sang
2019-06-12 10:40 ` Lee Jones
2019-06-12 10:44 ` Wolfram Sang
2019-06-13 8:52 ` Lee Jones
2019-06-13 8:52 ` Lee Jones
2019-06-13 9:19 ` Wolfram Sang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190612055736.GO4797@dell \
--to=lee.jones@linaro.org \
--cc=alokc@codeaurora.org \
--cc=andy.gross@linaro.org \
--cc=ard.biesheuvel@linaro.org \
--cc=balbi@kernel.org \
--cc=bjorn.andersson@linaro.org \
--cc=david.brown@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=jlhugo@gmail.com \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=wsa+renesas@sang-engineering.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).